DYN-9244 ProgramData Samples Fix #16578
Conversation
With this fix not the Sample files located in Dynamo\doc\distrib\Samples will be copied during building the solution to DynamoCore\Samples, so if we try to open the Default Samples folder from Dynamo menu or DynamoHome this new path will be opened.
RobertGlobant20
requested review from
QilongTang,
Copilot,
johnpierson and
zeusongit
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9244
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the default samples path for DynamoSandbox by copying sample files from the documentation directory to a local Samples folder during build and updating the path resolution logic.
- Changes the common data directory path from using system's CommonApplicationData to the DynamoCore assembly location with a "Samples" subfolder
- Adds build step to copy sample files from extern/doc/distrib/Samples to the output Samples directory
- Removes dependency on GetCommonDataFolder method and pathResolver for samples location
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/DynamoCoreWpf/DynamoCoreWpf.csproj | Adds MSBuild target to copy sample files during build process |
| src/DynamoCore/Configuration/PathManager.cs | Updates samples directory resolution to use local assembly path instead of system CommonApplicationData |
| src/DynamoCore/Configuration/Configurations.cs | Adds SamplesAsString constant for folder name consistency |
|
This is a GIF showing the expected behavior: |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
|
Thank you @RobertGlobant20 I also committed the suggestion by Copilot. Waiting for PR checks |
| var dynamoCorePath = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); | ||
| // Common directories. | ||
| commonDataDir = GetCommonDataFolder(); | ||
| commonDataDir = Path.Combine(dynamoCorePath, Configurations.SamplesAsString); |
There was a problem hiding this comment.
What else is this commonDataDir used for? Is changing this value completely going to break other features?
There was a problem hiding this comment.
Good catch, @QilongTang!
@RobertGlobant20 since the commonDataDir is the basis on which samplesDirectory and defaultTemplatesDirectory below are derived, sending commonDataDir with "\Samples" suffix into GetTemplateFolder() below does not sound correct to me:
commonDataDir = "%UserProfile%\Documents\dev\Dynamo\bin\AnyCPU\Debug\Samples";
// Get nested sample path?
samplesDirectory = GetSamplesFolder(commonDataDir);
// Get template path from sample path?
defaultTemplatesDirectory = GetTemplateFolder(commonDataDir);The reason it still returns the correct (or rather, default) samplesDirectory and defaultTemplatesDirectory is because the malformed directory self-corrected to this value because of the non-existence "\Samples" directory:
%UserProfile%\Documents\dev\Dynamo\bin\AnyCPU\Debug\templates\en-US
In addition to that, commonDataDir is exposed through PathManager.CommonDataDirectory property and is referenced by DynamoCore and DynamoCoreWpf alike. As it stands the path is set to %UserProfile%\Documents\dev\Dynamo\bin\AnyCPU\Debug\Samples, which might not be desirable.
I am not up-to-date with all the discussions, but if we need the samples alongside DynamoCore.dll, the following would suffice:
commonDataDir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);There was a problem hiding this comment.
@QilongTang to your question I'm still checking if it will affect the ExtensionManager and the functionality of loading the DynamoSettings.xml from other location but I'm still testing/debugging the changes in Revit to see how it will affect.
There was a problem hiding this comment.
For sure, @RobertGlobant20, thanks again for fixing this 👍
There was a problem hiding this comment.
Hi @RobertGlobant20, hang in there, we're almost done! 🙂
I've debugged this code and compared it with 3.6 (which I believe predates the program-data work), and I found something interesting.
User Data a.k.a. AppData
The new PathManager behaves exactly as it did in 3.6 -- excellent work there!
| PathManager Property | 3.6 | 4.0 |
| ------------------------ | --------------------------------------------------- | --------------------------------------------------- |
| BackupDirectory | %AppData%\Dynamo\Dynamo Core\backup | %AppData%\Dynamo\Dynamo Core\backup |
| DefaultUserDefinitions | %AppData%\Dynamo\Dynamo Core\3.6\definitions | %AppData%\Dynamo\Dynamo Core\4.0\definitions |
| DefaultPackagesDirectory | %AppData%\Dynamo\Dynamo Core\3.6\packages | %AppData%\Dynamo\Dynamo Core\4.0\packages |
| LogDirectory | %AppData%\Dynamo\Dynamo Core\3.6\Logs | %AppData%\Dynamo\Dynamo Core\4.0\Logs |
| UserDataDirectory | %AppData%\Dynamo\Dynamo Core\3.6 | %AppData%\Dynamo\Dynamo Core\4.0 |
| PreferenceFilePath | %AppData%\Dynamo\Dynamo Core\3.6\DynamoSettings.xml | %AppData%\Dynamo\Dynamo Core\4.0\DynamoSettings.xml |
| PythonTemplateFilePath | %AppData%\Dynamo\Dynamo Core\3.6\PythonTemplate.py | %AppData%\Dynamo\Dynamo Core\4.0\PythonTemplate.py |Common Data a.k.a. ProgramData
This is where things diverge slightly. Looking at what we have in PathManager, it appears we're close to achieving our goal. The remaining codebase will need to align with PathManager as the source of truth (for example, the Help > Samples menu item should respect the SamplesDirectory value provided by the PathManager singleton).
| PathManager Property | 3.6 | 4.0 |
| ------------------------- | ------------------------------------------------ | --------------------------------------------------------- |
| CommonDataDirectory | %ProgramData%\Dynamo\Dynamo Core\3.6 | %UserProfile%\dev\Dynamo\bin\AnyCPU\Debug\Samples |
| CommonDefinitions | %ProgramData%\Dynamo\Dynamo Core\3.6\definitions | N/A |
| CommonPackageDirectory | %ProgramData%\Dynamo\Dynamo Core\3.6\packages | N/A |
| DefaultTemplatesDirectory | %ProgramData%\Dynamo\Dynamo Core\templates\en-US | %UserProfile%\dev\Dynamo\bin\AnyCPU\Debug\templates\en-US |
| SamplesDirectory | %ProgramData%\Dynamo\Dynamo Core\samples\en-US | %UserProfile%\dev\Dynamo\bin\AnyCPU\Debug\samples\en-US |
| DynamoCoreDirectory | %UserProfile%\dev\Dynamo\bin\AnyCPU\Debug | %UserProfile%\dev\Dynamo\bin\AnyCPU\Debug |There are several categories of items to address. Since I'm not up-to-date on recent architectural decisions, please correct me if my understanding is off base.
CommonDefinitions and CommonPackageDirectory
These properties are omitted in PathManager 4.0. If there was a deliberate decision to consolidate these with their user data counterparts (DefaultUserDefinitions, DefaultPackagesDirectory), that makes sense. If not, we should discuss this further.
DynamoCoreDirectory vs. CommonDataDirectory
DynamoCoreDirectory is consistently set to the directory containing DynamoCore.dll, which is correct. What I'd like to highlight is the CommonDataDirectory value, which appears to have shifted from a centralized data location to point to the same DynamoCore.dll directory (assuming we remove the "\Samples" suffix):
| PathManager Property | 3.6 | 4.0 |
| -------------------- | ----------------------------------------- | ----------------------------------------- |
| DynamoCoreDirectory | %UserProfile%\dev\Dynamo\bin\AnyCPU\Debug | %UserProfile%\dev\Dynamo\bin\AnyCPU\Debug |
| CommonDataDirectory | %ProgramData%\Dynamo\Dynamo Core\3.6 | %UserProfile%\dev\Dynamo\bin\AnyCPU\Debug |Both ViewExtensionLoader and ExtensionLoader use CommonDataDirectory to figure out which extensions need certification verification. Now that it points to the DynamoCore.dll directory, any extensions there will get their signatures checked -- which is great.
This does make me wonder about how extensions get installed though (well, apart from built-in extensions, of course). If this becomes the main (or only) place we load extensions from, they'd need to be installed alongside host apps like Revit, which requires elevated permissions. Are we keeping something like the old %ProgramData%\Dynamo\Dynamo Core\3.6 path (by moving it under %AppData%) for user-installable extensions? Or has the whole approach changed?
I might be behind on how extension loading works these days, so feel free to correct me if things have moved in a different direction 🙂
| var dynamoCorePath = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); | ||
| // Common directories. | ||
| commonDataDir = GetCommonDataFolder(); | ||
| commonDataDir = Path.Combine(dynamoCorePath, Configurations.SamplesAsString); |
There was a problem hiding this comment.
Good catch, @QilongTang!
@RobertGlobant20 since the commonDataDir is the basis on which samplesDirectory and defaultTemplatesDirectory below are derived, sending commonDataDir with "\Samples" suffix into GetTemplateFolder() below does not sound correct to me:
commonDataDir = "%UserProfile%\Documents\dev\Dynamo\bin\AnyCPU\Debug\Samples";
// Get nested sample path?
samplesDirectory = GetSamplesFolder(commonDataDir);
// Get template path from sample path?
defaultTemplatesDirectory = GetTemplateFolder(commonDataDir);The reason it still returns the correct (or rather, default) samplesDirectory and defaultTemplatesDirectory is because the malformed directory self-corrected to this value because of the non-existence "\Samples" directory:
%UserProfile%\Documents\dev\Dynamo\bin\AnyCPU\Debug\templates\en-US
In addition to that, commonDataDir is exposed through PathManager.CommonDataDirectory property and is referenced by DynamoCore and DynamoCoreWpf alike. As it stands the path is set to %UserProfile%\Documents\dev\Dynamo\bin\AnyCPU\Debug\Samples, which might not be desirable.
I am not up-to-date with all the discussions, but if we need the samples alongside DynamoCore.dll, the following would suffice:
commonDataDir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);Removing the const string SamplesDirectoryName and const string TemplateDirectoryName so and replace them by the ones defined in Configurations.
CommonDefinitions and CommonPackageDirectory DynamoCoreDirectory vs. CommonDataDirectory |
|
Hi @RobertGlobant20, thanks for the details.
|
|
Maybe obsolete
|
|
@benglin to your question: |
|
@benglin ok, I will proceed to remove CommonDataDirectory and ensure DefaultPackagesDirectory is included in the certificate verification paths for both loaders. |
With this fix is opening Samples/Templates in the next places: DynamoSandbox Samples -> Dynamo\bin\AnyCPU\Debug\Samples\en-US Templates -> C:\ProgramData\Autodesk\Dynamo\Dynamo Core\templates\en-US Dynamo on Revit 2027 Templates - C:\ProgramData\Autodesk\RVT 2027\Dynamo\templates\en-US Samples - C:\ProgramData\Autodesk\RVR 2027\Dynamo\Samples\en-US
|
@RobertGlobant20 PTAL the regressions reported for this PR |
Ah yes @RobertGlobant20, I did not see this comment until now but I did mention something similar in your other PR #16528 that we may need to just mark it as Edit: Also, I think it's worthwhile merging #16528 into this PR so the regressions can be fixed in one go, there are many moving parts if they are kept separated. Thanks! |
Due that CommonDataDirectory was set to the DynamoCore directory it was adding this folder to DirectoriesToVerifyCertificates but DynamoCore/extensions and DynamoCore/viewExtensions don't have certificates so all the built-in extensions were not loaded. After my fix I'm not adding entires to DirectoriesToVerifyCertificates so doesn't check for extensions with certificates in DynamoCore/extensions and DynamoCore/viewExtensions then are loaded correctly.
Due that CommonDataDirectory was set to the DynamoCore directory it was adding this folder to DirectoriesToVerifyCertificates but DynamoCore/extensions and DynamoCore/viewExtensions don't have certificates so all the built-in extensions were not loaded. |
based in the next code that recently updated please let me know if you still think is a good idea to obsolete the CommonDataDirectory |
The test CannotDeleteProgramDataPath() was deleted due that is trying to delete the %ProgramData%/Packages folder but not doesn't exist anymore The test CannotUpdateProgramDataPath() was deleted due that is trying to update the %ProgramData%/Packages folder but not doesn't exist anymore The static strings were updated to be lowercase due that there is a test checking against that specific string so was failing.
If the property from PathResolver is always pointing to %ProgramData%, then I think it's still worth obsoleting. However, given this PR is now in a good state, I am leaning towards merging and testing this and keep the other work in follow up PR. |
4 participants
Purpose
With this fix now the Sample files located in Dynamo\doc\distrib\Samples will be copied when building the solution to DynamoCore\Samples, so if we try to open the Default Samples folder from DynamoHome this new path will be opened by default.
Note: I'm still pending in testing this change in Dynamo Revit.
Declarations
Check these if you believe they are true
Release Notes
Updating the default path used for Samples when using DynamoSandbox
Reviewers
@QilongTang @zeusongit @johnpierson
FYIs
@achintyabhat