Cross platform DynamoCore local setup (part1) #13547

pinzart90 · 2022-11-21T08:11:30Z

Purpose:
Provide a local configuration/system to build DynamoCore for net60 (windows and linux)

What changed:
Added local build configuration for dotnet6.0 (linux and windows supported OSs)
A new solution DynamoCore.net6.sln has been added, to make DEV experience easier. When the dotnet6 ecosystem is mature enough, we can move everything over into a single Dynamo.Core.sln (multi target without UI) and a Dynamo.All.sln (multi target with UI)
Windows specific APIs have been marked with a [SupportedOSPlatform("Windows")] attribute.
net48 and net6.0 Packages have been segregated.
Some of the codebase has changed to accommodate net6.0 and non-windows APIs

TODO:
The new PackageReferences (for dotnet6.0) have not been vetted properly.
Bring in more of the Dynamo Core projects.
Remove windows only dependencies from the Core and Linux build if not needed (ex image resources). Having certain resources means we are stuck with msbuild and cannot simply use dotnet build for our build process
There are dependencies of DynamoCore+Linux which still need to be compiled specifically for linux (Ex. LibG native binaries need to be compiled for Linux or else they will not load properly)

DynamoCore.net6.sln:

Co-Authored-By: pinzart <[email protected]>

src/Config/CS_SDK.props

mjkkirschner · 2022-12-08T17:42:56Z

src/DynamoUtilities/PathHelper.cs

 using System.Security.AccessControl;
 using System.Security.Principal;
 using System.Xml;
 using Newtonsoft.Json;

 namespace DynamoUtilities
 {
+    internal static class OSHelper


kind of an unexpected location I guess - inside path helper?

It seemed overkill to create a new .cs file for just a couple lines of code

mjkkirschner · 2022-12-08T17:44:17Z

src/Engine/ProtoCore/ProtoCore.csproj

+    <None Include="FFI\FFICppFunction.cs" />
+    <None Include="FFI\PInvokeFFI.cs" />
+    <Compile Condition=" '$(TargetFramework)' == 'net6.0' " Remove="FFI\FFICppFunction.cs" />
+    <Compile Condition=" '$(TargetFramework)' == 'net6.0' " Remove="FFI\PInvokeFFI.cs" />


are these files actually still used, even in net framework 48?

THey are not compiled. Not sure if they are still around for other reasons (like to revisit the functionality)

mjkkirschner · 2022-12-08T17:44:48Z

src/Libraries/CoreNodeModels/CoreNodeModels.csproj

@@ -20,14 +20,18 @@
  <ItemGroup>
    <None Remove="CoreNodeModelsImages.resources" />
  </ItemGroup>
+  <ItemGroup Condition=" '$(TargetFramework)' == 'net48' ">


I think we should just dump this node - I don't think theres anything it can do that a code block cannot.

I was hoping to minimize the chages to our net48 build..so we do not have any surprises
Maybe we can do this as a separate task

mjkkirschner · 2022-12-08T17:45:26Z

src/Libraries/CoreNodes/CoreNodes.csproj

    <Reference Include="Microsoft.CSharp" />
    <PackageReference Include="DynamoVisualProgramming.LibG_229_0_0" Version="2.16.0.3357"/>
    <PackageReference Include="ncalc" Version="1.3.8">
      <!--Exclude copying the runtime dlls because we need to keep binary compatibility with Revit for now -->
      <ExcludeAssets>runtime</ExcludeAssets>
    </PackageReference>
    <Reference Include="PresentationCore" />
+  </ItemGroup>
+  <ItemGroup Condition=" '$(TargetFramework)' == 'net6.0' ">
+    <PackageReference Include="CoreCLR-NCalc" Version="2.2.92" />


same as above, I don't think it's worth the complexity. Maybe we can even migrate to a codeblock or python.

src/Libraries/DynamoUnits/Units.csproj

src/Tools/DynamoInstallDetective/DynamoInstallDetective.csproj

mjkkirschner · 2022-12-08T17:49:39Z

src/Tools/DynamoShapeManager/Preloader.cs

@@ -52,7 +52,9 @@ public class Preloader
        /// typical setup this would be the same directory that contains Dynamo 
        /// core modules. This must represent a valid directory.
        /// </param>
-        /// 


so how will ASM be loaded on linux builds?

Still WIP
My current thoughts were to only support it via command line argument

pinzart90 · 2022-12-08T14:16:48Z

src/Config/CS_SDK.props

+    <TargetFramework>net6.0</TargetFramework>
+    <!--Needed to copy nuget package assemblies to output folder. Anet6 issue-->
+    <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
+    <GenerateDependencyFile>false</GenerateDependencyFile>


I turned this off because it is causing issues (could not find assembly references in the app directory) with the existing Copy local=false settings in Dynamo. We do not need the deps files at the moment

.deps.json - A list of dependencies, as well as compilation context data and compilation dependencies. Not technically required, but required to use the servicing or package cache/shared package install features. It can be used to configure dynamic linking to assemblies that come from packages. As mentioned above, .NET Core can be configured to dynamically load assemblies from multiple locations. These locations include: App base directory (in the same folder as the entry point application, no config required) Package cache folders (NuGet restore cache or NuGet fallback folders) An optimized package cache or runtime packages store The servicing index (rarely used. For Windows Update purposes.) Shared framework (configured via runtimeconfig.json)

.github/workflows/dynamoNet6.0_build.yml

pinzart90 · 2022-12-08T17:30:28Z

.github/workflows/dynamoNet6.0_build.yml

+      with:
+        dotnet-version: '6.0.x'
+    - name: Install dependencies for windows runtime
+      run: dotnet restore $Env:GITHUB_WORKSPACE\Dynamo\src\DynamoCore.net6.sln --runtime=win10-x64


we are using dotnet restore because it allows us to specify the target runtime (windows or linux)
nuget does not have this option.
This is an issue because we are using a common properties file (CS_SDK.props) with the default configs set for DynamoAll and not DynamoCore builds.
This can be remediated in other ways....but I think it is good enough for now

pinzart90 · 2022-12-08T17:37:51Z

.github/workflows/dynamoNet6.0_build.yml

+on: [push,pull_request]
+jobs:
+ build:
+  runs-on: windows-latest


we need msbuild for now. Not sure if msbuild is available on liinux machines

pinzart90 · 2022-12-08T17:45:27Z

src/Config/CS_SDK.props

+    </GetReferenceAssemblyPaths>
+    <PropertyGroup>
+      <SystemDrawingDllPath Condition=" '$(TargetFramework)' == 'net48' ">$(FrameworkAssembliesPath)System.Drawing.dll</SystemDrawingDllPath>
+      <!--Requires a reference to "System.Drawing.Common" Version="7.0.0" -->


Yes, it is marked in our wiki as a TODO.
Currently it looks like the APIs we are using are ok for windows and linux.
In the future, we can either switch to a new crossplatform package or stop generating icons/resources for dynamo core builds (linux and/or windows ...)

pinzart90 · 2022-12-08T17:55:40Z

src/Tools/DynamoShapeManager/Preloader.cs

@@ -52,7 +52,9 @@ public class Preloader
        /// typical setup this would be the same directory that contains Dynamo 
        /// core modules. This must represent a valid directory.
        /// </param>
-        /// 


Still WIP
My current thoughts were to only support it via command line argument

src/Libraries/DynamoUnits/Units.csproj

pinzart90 · 2022-12-08T17:59:50Z

src/Libraries/CoreNodeModels/CoreNodeModels.csproj

@@ -20,14 +20,18 @@
  <ItemGroup>
    <None Remove="CoreNodeModelsImages.resources" />
  </ItemGroup>
+  <ItemGroup Condition=" '$(TargetFramework)' == 'net48' ">


I was hoping to minimize the chages to our net48 build..so we do not have any surprises
Maybe we can do this as a separate task

pinzart90 · 2022-12-08T18:04:39Z

src/DynamoApplications/StartupUtils.cs

 using Microsoft.Win32;
 using NDesk.Options;

 namespace Dynamo.Applications
 {
+#if NET6_0_OR_GREATER
+    // Add the same signature for the OptionSet.Add method as found in the NDesk.Options package for net48
+    internal static class OptionsExtensions


It can be done if we decide it is better to keep the packages to a minimum but with more if def code

pinzart90 · 2022-12-08T18:22:40Z

src/DynamoApplications/StartupUtils.cs

 using Microsoft.Win32;
 using NDesk.Options;

 namespace Dynamo.Applications
 {
+#if NET6_0_OR_GREATER
+    // Add the same signature for the OptionSet.Add method as found in the NDesk.Options package for net48
+    internal static class OptionsExtensions


Note that whatever package we choose, will be part of our releases.
I do not think the NodeDocumentationMarkdownGenerator and its references are part of the release

sm6srw · 2022-12-08T15:22:59Z

src/Config/CS_SDK.props

@@ -91,4 +109,19 @@
    <Copy SourceFiles="@(PDBContent)" DestinationFolder="$(OutDir)$(PackagesOutputSubdirectory)" Condition="Exists('%(FullPath)')" />
    <Copy SourceFiles="@(DLLContent)" DestinationFolder="$(OutDir)$(PackagesOutputSubdirectory)$(Local_CopyToSubdirectory)" Condition="Exists('%(FullPath)')" />
  </Target>
+  <Target Name="ResolveSateliteResDeps" AfterTargets="Build" Condition=" '$(OS)' != 'Unix' ">


Where is $(OS) defined? Built in?

it is built in

sm6srw · 2022-12-09T15:03:48Z

src/DynamoApplications/StartupUtils.cs

 using Microsoft.Win32;
 using NDesk.Options;

 namespace Dynamo.Applications
 {
+#if NET6_0_OR_GREATER
+    // Add the same signature for the OptionSet.Add method as found in the NDesk.Options package for net48
+    internal static class OptionsExtensions


Is it strictly necessary to keep combability here? I think we can get close enough. Maybe something we do for 3.0.

sm6srw · 2022-12-09T15:09:30Z

src/DynamoUtilities/DynamoUtilities.csproj

-  <ItemGroup>
-	<PackageReference Include="AvalonEdit" Version="4.3.1.9430" CopyXML="true" />
-	<PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
+  <ItemGroup Label="Common Depenedencies">


pinzart90 · 2022-12-14T14:37:03Z

src/DynamoApplications/StartupUtils.cs

+        public string CERLocation { get; set; } = String.Empty;
+    }
+#endif
+


I decided to use a separate internal class for the CommandLine options. Hopefully this is less cluttered and more readable.

pinzart90 · 2022-12-14T14:39:14Z

src/DynamoApplications/StartupUtils.cs

+#if NET6_0_OR_GREATER
+    internal class CMDLineOptions
+    {
+        [Option('l', "Locale", Required = false, HelpText = "Running Dynamo under a different locale setting.")]


some of the new cmdline args have changed (as oposed to the old way - using NDesk.Options)
The new CommandLineParser package only supports 2 names (short - single char and long any string). Some arguments will not have a short name because of collisions issues.

FYI @mjkkirschner @sm6srw

yep, bummer, I regret the suggestion - 😢 - but thanks for making the change, probably good to switch to something that actually gets updated.

mjkkirschner · 2022-12-14T17:25:28Z

src/Config/CS_SDK.props

+  <PropertyGroup Condition="$(Platform.Contains('NET60'))" >
+    <TargetFramework>net6.0</TargetFramework>
+    <!--Needed to copy nuget package assemblies to output folder. Anet6 issue-->
+    <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>


so... does this turn all nuget references into copy local true?

yes, it copies all referenced assemblies in the output path

that sounds dangerous - do we have duplicate copies now of things like newtonsoft.json in the /nodes and /bin folders?

duplicate copies will not happen because all projects that have the outputPath to /nodes also have a redirect attribute

<ReferenceCopyLocalPaths>  <DestinationSubDirectory>..\</DestinationSubDirectory> </ReferenceCopyLocalPaths> ```

mjkkirschner · 2022-12-14T17:28:23Z

src/DynamoApplications/StartupUtils.cs

+        [Option('x', "ConvertFile", Required = false, HelpText = "When used in combination with the 'O' flag, opens a .dyn file from the specified path and converts it to .json." +
+            "File will have the .json extension and be located in the same directory as the original file.")]
+        public bool ConvertFile { get; set; }
+        [Option('g', "Geometry", Required = false, HelpText = "Instruct Dynamo to output geometry from all evaluations to a json file at this path.")]


yeah, so what happens here with the two g flags? Does this throw an error at compile time or just ignore the matching flags?

I did not even try. Obviously something is not going to work

compile was fine

doesn't need to be resolved here but we should probably align these and just document the break between the two targets.

will add a task for it once we are sure that this is the package we want

mjkkirschner · 2022-12-14T17:36:21Z

src/DynamoApplications/StartupUtils.cs

@@ -378,7 +465,7 @@ private static bool PreloadASM(string asmPath, out string geometryFactoryPath, o
                CLIMode = CLImode
            };
            config.AuthProvider = CLImode ? null : new Core.IDSDKManager();
-            config.UpdateManager = CLImode ? null : InitializeUpdateManager();
+            config.UpdateManager = CLImode ? null : OSHelper.IsWindows() ? InitializeUpdateManager() : null;


what is windows only about updatemanager?

SandboxLookUp is the one. Used inside the InitializeUpdateManager
I did not try to make it work for linux yet

Hopefully we can get rid of most of the Windows only APIs from DynamoCore
This can be done in steps

mjkkirschner · 2022-12-14T17:37:42Z

src/DynamoCLI/DynamoCLI.csproj

@@ -8,11 +8,12 @@
    <AppDesignerFolder>Properties</AppDesignerFolder>
    <RootNamespace>DynamoCLI</RootNamespace>
    <AssemblyName>DynamoCLI</AssemblyName>
+    <SelfContained>false</SelfContained>


just curious - what happens if this is true, the exe also includes all the binaries in the bin folder?

SelfContained = true will copy all the dotnet6 runtime dlls

mjkkirschner · 2022-12-14T17:41:09Z

src/DynamoCoreWpf/DynamoCoreWpf.csproj

@@ -69,6 +69,7 @@
    <None Remove="Views\GuidedTour\HtmlPages\Resources\ConnectTheNode.gif" />
  </ItemGroup>
  <ItemGroup>
+  <PackageReference Include="AvalonEdit" Version="4.3.1.9430" CopyXML="true"/>


why was this needed?

this packageRefecence was moved from DynamoUtilities. DynamoCoreWpf.csproj is the project that actually uses it.
I think it was used by DynamoUtilities only for an indirect reference to System.Collections.Immutable.
I refactored it so that we do not need it in DynamoUtilities.

This might cause compile issues...if someone was using AvalonEdit APIs by referencing the DynamoUtilities project ...But from what I was able to check...looks like we are ok with the autodesk integrators.

mjkkirschner · 2022-12-14T17:41:53Z

src/DynamoPackages/DynamoPackages.csproj

@@ -9,7 +9,7 @@
    <RootNamespace>DynamoPackages</RootNamespace>
    <AssemblyName>DynamoPackages</AssemblyName>
  </PropertyGroup>
-  <ItemGroup>
+  <ItemGroup Condition=" '$(TargetFramework)' == 'net48' ">


so, will net6 builds be able to load config files?

Yes...there is a separate ConfigurationManager nuget package for that

mjkkirschner · 2022-12-14T17:47:54Z

src/DynamoUtilities/PathHelper.cs

-                return Finfo.IsReadOnly || !HasWritePermissionOnDir(Finfo.Directory.ToString());
+
+                // We have no cross platform Directory access writes APIs.
+                bool hasWritePermissionOnDir = OSHelper.IsWindows() ? HasWritePermissionOnDir(Finfo.Directory.ToString()) : true;


I guess now wherever this is used should be wrapped in try catch?
Maybe update summary?

Not sure yet how we should go about it for linux. There are packages that have similar APIs ...(as far as I remember)
Or we can implement a simple version ourselves.

I think we need to do this in a separate task...talk about acceptance criteria and maybe some spikes too

mjkkirschner · 2022-12-14T18:05:44Z

src/Tools/DynamoShapeManager/Utilities.cs

@@ -611,11 +622,28 @@ internal static bool IsASMInstallationComplete(IEnumerable<string> filePaths, in
        /// <returns></returns>
        /// <param name="searchPattern">optional - to be used for testing - default is the ASM search pattern</param>
        /// <returns></returns>
-        public static Version GetVersionFromPath(string asmPath, string searchPattern = "ASMAHL*.dll")
+        public static Version GetVersionFromPath(string asmPath, string searchPattern = "*ASMAHL*.*")


can you explain this change?

asm binaries for linux do not have a .dll extension (linux has .so - shared objects)
I changed it so that we do not need the extension

mjkkirschner · 2022-12-14T18:07:00Z

src/Tools/DynamoShapeManager/Utilities.cs

+#if NET6_0_OR_GREATER
+                if (!OperatingSystem.IsWindows())
+                {
+                    string fileName = Path.GetFileNameWithoutExtension(ASMFilePath);


I mean, cool, but ugh - I'm surprised theres nothing cross platform here.

.so (shared objects) do not have the same metadata as dlls.
I think the recommended versioning technique is to bake the version in the name as a prefix (ex libtsplines.so.10)

mjkkirschner · 2022-12-14T18:07:31Z

A lot of changes, and still some questions, but it looks good.

.github/workflows/dynamoNet6.0_build.yml

src/DynamoApplications/StartupUtils.cs

src/DynamoUtilities/DynamoUtilities.csproj

src/DynamoUtilities/PathHelper.cs

pinzart and others added 16 commits November 17, 2022 21:22

update

9cb07b6

update

59023a2

update

4776219

update

ee7512c

update

d4d5635

update

776c3b4

Update DynamoCLI.csproj

444136a

update

2335036

Merge branch 'master' into stuff

918753d

update

2c6c5d9

Update PathHelper.cs

3c527a7

update

d7870c6

update

6b8eeee

Co-Authored-By: pinzart <[email protected]>

Update Program.cs

8fd0322

Co-Authored-By: pinzart <[email protected]>

Update Program.cs

a9e1787

Co-Authored-By: pinzart <[email protected]>

update

93a802c

Co-Authored-By: pinzart <[email protected]>

pinzart90 changed the title ~~cross platform DynamoCore local setup~~ cross platform DynamoCore local setup (part1) Nov 21, 2022

pinzart90 marked this pull request as ready for review November 21, 2022 19:54

pinzart90 changed the title ~~cross platform DynamoCore local setup (part1)~~ [WIP] cross platform DynamoCore local setup (part1) Nov 21, 2022

pinzart added 5 commits November 30, 2022 10:00

update

b515670

Update PathHelper.cs

0f91091

update

d8685f1

update

4af3fad

update

efffd0f

pinzart90 requested review from mjkkirschner, sm6srw and aparajit-pratap December 5, 2022 18:38

pinzart90 and others added 2 commits December 6, 2022 08:42

Merge branch 'master' into stuff

d91f5fd

Merge branch 'master' into stuff

71c948a

mjkkirschner reviewed Dec 7, 2022

View reviewed changes

src/Config/CS_SDK.props Show resolved Hide resolved

mjkkirschner reviewed Dec 8, 2022

View reviewed changes

src/Libraries/DynamoUnits/Units.csproj Show resolved Hide resolved

mjkkirschner reviewed Dec 8, 2022

View reviewed changes

src/Tools/DynamoInstallDetective/DynamoInstallDetective.csproj Show resolved Hide resolved

mjkkirschner reviewed Dec 8, 2022

View reviewed changes

pinzart90 commented Dec 9, 2022

View reviewed changes

sm6srw reviewed Dec 9, 2022

View reviewed changes

switch to cmdlin

30445f6

pinzart90 commented Dec 14, 2022

View reviewed changes

pinzart90 requested review from mjkkirschner and sm6srw December 14, 2022 14:39

mjkkirschner reviewed Dec 14, 2022

View reviewed changes

mjkkirschner approved these changes Dec 14, 2022

View reviewed changes

update

66229a4

aparajit-pratap reviewed Dec 15, 2022

View reviewed changes

aparajit-pratap approved these changes Dec 15, 2022

View reviewed changes

pinzart90 merged commit 53e2800 into master Dec 16, 2022

pinzart90 deleted the stuff branch December 16, 2022 14:02

Cross platform DynamoCore local setup (part1) #13547

Cross platform DynamoCore local setup (part1) #13547

Conversation

pinzart90 commented Nov 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pinzart90 Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjkkirschner Dec 14, 2022 • edited Loading

Choose a reason for hiding this comment

mjkkirschner Dec 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjkkirschner commented Dec 14, 2022

pinzart90 commented Nov 21, 2022 •

edited

Loading

pinzart90 Dec 8, 2022 •

edited

Loading

mjkkirschner Dec 14, 2022 •

edited

Loading

mjkkirschner Dec 14, 2022 •

edited

Loading