Merge branch 'D4R_DA' into D4DA_2026 #3304
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| # Build System Cleanup — Open Questions & TODOs | ||
|
|
||
| These are known issues and open design questions in the build system that need | ||
| to be resolved, particularly for the `D4R_DA_2026` branch targeting Revit 2026 / net8. | ||
|
|
||
| --- | ||
|
|
||
| ## 1. Platform naming is confusing and redundant | ||
|
|
||
| `CS_SDK.props` defines three platforms: | ||
|
|
||
| | Platform | TargetFramework | Notes | | ||
| |------------|---------------------|-------| | ||
| | `NET80` | `net8.0-windows` | Interactive Revit build | | ||
| | `NET80_DA` | `net8.0-windows` | **Same as NET80 — name implies net10 but it's net8** | | ||
| | `NET100` | `net10.0-windows` | Future Revit targeting net10 | | ||
|
|
||
| For the `D4R_DA_2026` branch (Revit 2026), only `net8.0-windows` is relevant. | ||
| `NET100` adds noise and `NET80_DA` is actively misleading. | ||
|
|
||
| **Important:** `_DA` in the platform name is a meaningful build profile — it is not just a | ||
| TFM alias. Any platform whose name contains `_DA` triggers these behaviours across multiple projects: | ||
|
|
||
| | Project | What `_DA` removes | | ||
| |---|---| | ||
| | `CS_SDK.props` | Defines `DESIGN_AUTOMATION` compile constant | | ||
| | `DynamoRevit.csproj` | Removes `DynamoRevit.cs`, `DynamoRevitApp.cs`, entire `ViewModel/` (interactive startup + ribbon) | | ||
| | `RevitNodesUI.csproj` | Removes all WPF controls, selection dialogs, XAML pages | | ||
| | `RevitServices.csproj` | Removes `Threading/*.cs` (UI thread marshalling, not needed in headless DA) | | ||
|
|
||
| So the `_DA` suffix carries real semantic weight. The problem is solely the `NET10` prefix, | ||
| which implies net10 when the TFM is actually net8. Renaming `NET80_DA` → `NET80_DA` everywhere | ||
| would fix the confusion without changing any build behaviour. | ||
|
|
||
| **Questions:** | ||
| - Should `D4R_DA_2026` drop `NET100` entirely and only expose `NET80`? | ||
| - Was `NET80_DA` ever actually net10 and got downgraded, or has it always been net8? | ||
| - Should DA just use `NET80` with a separate property/flag to denote DA vs interactive, | ||
| rather than a separate platform name? | ||
| - If `NET80_DA` stays, should it be renamed (e.g. `NET80_DA`) to reflect actual TFM? | ||
|
|
||
| --- | ||
|
|
||
| ## 2. Solution file responsibilities are unclear | ||
|
|
||
| | Solution | Projects included | Typical build command | | ||
| |-----------------------|--------------------------------|-----------------------| | ||
| | `DynamoRevit.DA.sln` | DADynamoApp + deps only | `/p:Platform=NET80_DA` | | ||
| | `DynamoRevit.All.sln` | Everything incl. tests | `/p:Platform=NET80` or `NET100` | | ||
|
|
||
| `DADynamoApp` is **not** listed explicitly in `All.sln` — it is pulled in as a | ||
| transitive project reference from `RevitSystemTests`. This causes the solution to | ||
| build it without a proper platform mapping, which leads to build errors under | ||
| `dotnet build` with .NET SDK 10 (see issue 3 below). | ||
|
|
||
| A workaround was added: explicit `DADynamoApp` entry in `All.sln` mapping | ||
| `NET80`/`NET100` → `NET80_DA`. But this is a band-aid. | ||
|
|
||
| **Questions:** | ||
| - Should `DADynamoApp` be a first-class explicit project in `All.sln`? | ||
| - Should `DA.sln` be a proper subset configuration of `All.sln`, or kept fully separate? | ||
| - Is there a reason DA uses `NET80_DA` platform instead of `NET80`? If not, unify them | ||
| and remove the extra platform. | ||
|
|
||
| --- | ||
|
|
||
| ## 3. `dotnet build` vs VS MSBuild inconsistency | ||
|
|
||
| `DynamoRevit.All.sln` builds cleanly with VS MSBuild: | ||
| ``` | ||
| & "C:\Program Files\Microsoft Visual Studio\18\Professional\MSBuild\Current\Bin\MSBuild.exe" DynamoRevit.All.sln -p:Configuration=Debug -p:Platform=NET80 -m | ||
| ``` | ||
|
|
||
| But fails with `dotnet build` using .NET SDK 10: | ||
| ``` | ||
| dotnet build DynamoRevit.All.sln -c Debug /p:Platform=NET80 | ||
| ``` | ||
|
|
||
| Two known `dotnet build`-only failures: | ||
|
|
||
| **a) `DADynamoApp` — `MSB3992: 'RootElementName' is not set`** | ||
| - Root cause: `EnableDynamicLoading=true` causes SDK 10 to set | ||
| `UseAttributeForTargetFrameworkInfoPropertyNames=true`, which requires | ||
| `RootElementName` to be set explicitly. VS MSBuild does not have this requirement. | ||
| - Possible fixes: set `<UseAttributeForTargetFrameworkInfoPropertyNames>false</UseAttributeForTargetFrameworkInfoPropertyNames>`, | ||
| or set `<RootElementName>DADynamoApp</RootElementName>` explicitly. | ||
|
|
||
| **b) `DynamoRevitIcons` — `ResGen.exe not supported on .NET Core MSBuild`** | ||
| - Pre-existing issue. The `.resx` code generation step uses ResGen.exe which is | ||
| not available in the .NET Core MSBuild toolchain. | ||
| - Possible fix: migrate resource generation to use `<EmbeddedResource>` without | ||
| ResGen, or exclude from `dotnet build` paths. | ||
|
|
||
| **Question:** Is `dotnet build` a supported/required path in CI, or does CI always | ||
| use VS MSBuild? The `Jenkinsfile` delegates to `DynamoRevitUtils` — need to check | ||
| those build scripts to confirm. | ||
|
|
||
| --- | ||
|
|
||
| ## 4. Research needed / action items | ||
|
|
||
| - [ ] Check `DynamoRevitUtils` Jenkins scripts: which solution, which platform, `dotnet` or VS MSBuild? | ||
| - [ ] Confirm whether `NET100` is used in any CI job for this branch | ||
| - [ ] Decide on NET80_DA rename/merge with NET80 | ||
| - [ ] Decide whether All.sln should be the canonical "build everything" solution | ||
| - [ ] Fix `dotnet build` compatibility if CI requires it (MSB3992 + ResGen) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <Project DefaultTargets="Build" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
| <PropertyGroup> | ||
| <!-- 3rd party package versions --> | ||
| <DYNAMOCORE_VERSION>3.6.2.11575</DYNAMOCORE_VERSION> | ||
|
|
||
| <DYNAMOWPFUI_VERSION Condition="'$(DYNAMOWPFUI_VERSION)' == ''">$(DYNAMOCORE_VERSION)</DYNAMOWPFUI_VERSION> | ||
| <DYNAMOCORENODES_VERSION Condition="'$(DYNAMOCORENODES_VERSION)' == ''">$(DYNAMOCORE_VERSION)</DYNAMOCORENODES_VERSION> | ||
| <DYNAMOTESTS_VERSION Condition="'$(DYNAMOTESTS_VERSION)' == ''">$(DYNAMOCORE_VERSION)</DYNAMOTESTS_VERSION> | ||
|
|
||
| <!-- These are useful when migrating to a new .NET while not all builds are available --> | ||
| <DCoreLibSubFolder Condition="'$(Platform)' == 'NET80'">net8.0</DCoreLibSubFolder> | ||
|
|
||
| <Greg_VERSION>3.0.1.4707</Greg_VERSION> | ||
| <GregRevitAuth_VERSION>3.0.8935.26399</GregRevitAuth_VERSION> | ||
| <DotnetVerSubFolder>net8.0</DotnetVerSubFolder> | ||
|
|
||
| <!-- Keep in sync with DynamoCore & TODO: check if still needed --> | ||
| <DynamicLanguageRuntime_VERSION>1.2.2</DynamicLanguageRuntime_VERSION> | ||
| <PYTHONNET_PY38_VERSION>2.5.1</PYTHONNET_PY38_VERSION> | ||
| <NewtonsoftJson_VERSION>13.0.1</NewtonsoftJson_VERSION> <!-- TODO: is this still needed? --> | ||
| <NUnit_VERSION>3.13.3</NUnit_VERSION> | ||
| <NUnit_Engine_VERSION>3.13.2</NUnit_Engine_VERSION> | ||
| <NUnit3TestAdapter_VERSION>5.0.0</NUnit3TestAdapter_VERSION> | ||
|
|
||
| </PropertyGroup> | ||
| </Project> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| | ||
| using Autodesk.Revit.ApplicationServices; | ||
| using Autodesk.Revit.DB; | ||
| using System.Diagnostics; | ||
| using System.Reflection; | ||
|
|
||
| namespace DADynamoApp | ||
| { | ||
| [Autodesk.Revit.Attributes.Regeneration(Autodesk.Revit.Attributes.RegenerationOption.Manual)] | ||
| [Autodesk.Revit.Attributes.Transaction(Autodesk.Revit.Attributes.TransactionMode.Manual)] | ||
| public class DAApplication : IExternalDBApplication | ||
| { | ||
| private string ParentPath; | ||
| private string CurrentDirectory; | ||
| private readonly string PythonDllFolder = "pythonDependencies"; | ||
|
|
||
| private DAEntrypoint daEntryPoint; | ||
|
|
||
| public ExternalDBApplicationResult OnShutdown(ControlledApplication application) | ||
| { | ||
| AppDomain.CurrentDomain.UnhandledException -= CurrentDomain_UnhandledException; | ||
| AppDomain.CurrentDomain.ProcessExit -= CurrentDomain_ProcessExit; | ||
| AppDomain.CurrentDomain.AssemblyResolve -= CurrentDomain_AssemblyResolve; | ||
| return daEntryPoint.OnShutdown(application); | ||
| } | ||
|
|
||
| public ExternalDBApplicationResult OnStartup(ControlledApplication application) | ||
| { | ||
| AppDomain.CurrentDomain.UnhandledException += CurrentDomain_UnhandledException; | ||
| AppDomain.CurrentDomain.ProcessExit += CurrentDomain_ProcessExit; | ||
| AppDomain.CurrentDomain.AssemblyResolve += CurrentDomain_AssemblyResolve; | ||
|
|
||
| CurrentDirectory = Directory.GetCurrentDirectory(); | ||
| ParentPath = Directory.GetParent(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)).FullName; | ||
|
|
||
| Console.WriteLine("<<!>> Starting to load DAEntrypoint"); | ||
|
|
||
| daEntryPoint ??= new DAEntrypoint(); | ||
|
|
||
| return daEntryPoint.OnStartup(application); | ||
| } | ||
|
|
||
| private static void CurrentDomain_ProcessExit(object? sender, EventArgs e) | ||
| { | ||
| Process proc = Process.GetCurrentProcess(); | ||
| Console.WriteLine($"Dynamo exiting with Peak physical memory {proc.PeakWorkingSet64} bytes"); | ||
| if (proc.HasExited) | ||
| { | ||
| Console.WriteLine($"Dynamo exiting with code {proc.ExitCode}"); | ||
| } | ||
| } | ||
|
|
||
| private static void CurrentDomain_UnhandledException(object sender, UnhandledExceptionEventArgs e) | ||
| { | ||
| Console.WriteLine($"Unhandled exception: {e}"); | ||
| } | ||
|
|
||
| private Assembly? CurrentDomain_AssemblyResolve(object? sender, ResolveEventArgs args) | ||
| { | ||
| return DynamoRevitAssemblyResolver.ResolveDynamoAssembly(ParentPath, [Path.Combine(CurrentDirectory, PythonDllFolder)], args); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <ImportGroup Label="PropertySheets"> | ||
| <Import Project="$(SolutionDir)Config\CS_SDK.props" /> | ||
| </ImportGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <ImplicitUsings>enable</ImplicitUsings> | ||
| <Nullable>enable</Nullable> | ||
| <GenerateAssemblyInfo>false</GenerateAssemblyInfo> | ||
| <!--EnableDynamicLoading prepares the project so that it can be used as a plugin --> | ||
| <!--https://learn.microsoft.com/en-us/dotnet/core/tutorials/creating-app-with-plugin-support#simple-plugin-with-no-dependencies--> | ||
| <EnableDynamicLoading>true</EnableDynamicLoading> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Compile Remove="obj\**" /> | ||
| <EmbeddedResource Remove="obj\**" /> | ||
| <None Remove="obj\**" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Reference Include="RevitAPI"> | ||
| <HintPath>$(REVITAPI)\RevitAPI.dll</HintPath> | ||
| <Private>False</Private> | ||
| </Reference> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <PackageReference Include="Autodesk.Forge.DesignAutomation.Revit" Version="2026.0.0" /> | ||
| <PackageReference Include="Autodesk.DataManagement" Version="2.0.0-beta4" /> | ||
| <PackageReference Include="DynamoVisualProgramming.Core" Version="$(DYNAMOCORE_VERSION)" ExcludeAssets="runtime" GeneratePathProperty="true" /> | ||
| <PackageReference Include="DynamoPlayer" Version="6.0.2" ExcludeAssets="runtime" GeneratePathProperty="true" /> | ||
| <PackageReference Include="GregRevitAuth" Version="$(GregRevitAuth_VERSION)" GeneratePathProperty="true"> | ||
| <ExcludeAssets>runtime</ExcludeAssets> | ||
| </PackageReference> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <ProjectReference Include="..\DynamoRevit\DynamoRevit.csproj" /> | ||
| <ProjectReference Include="..\Libraries\RevitNodes\RevitNodes.csproj" /> | ||
| <ProjectReference Include="..\Libraries\RevitServices\RevitServices.csproj" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <Reference Include="DynamoPlayer.Models"> | ||
| <HintPath>$(PkgDynamoPlayer)\bin\Release\net8.0\bin\DynamoPlayer.Models.dll</HintPath> | ||
| <Private>False</Private> | ||
| </Reference> | ||
| <Reference Include="DynamoPlayer.Server"> | ||
| <HintPath>$(PkgDynamoPlayer)\bin\Release\net8.0\bin\DynamoPlayer.Server.dll</HintPath> | ||
| <Private>False</Private> | ||
| </Reference> | ||
| <Reference Include="DynamoPlayer.Workflows"> | ||
| <HintPath>$(PkgDynamoPlayer)\bin\Release\net8.0\bin\DynamoPlayer.Workflows.dll</HintPath> | ||
| <Private>False</Private> | ||
| </Reference> | ||
| </ItemGroup> | ||
| <Target Name="Copy dll" AfterTargets="Build"> | ||
| <Copy SourceFiles="$(PkgGregRevitAuth)\lib\$(DotnetVerSubFolder)\GregRevitAuth.dll" DestinationFolder="$(OutputPath)" /> | ||
|
Member
There was a problem hiding this comment. we copy this because DA does not provide it? |
||
| </Target> | ||
| </Project> | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you have go through this and note what is now done after this PR? Have an agent do this?