Dyn-6139 - Align pipelines #14261

sm6srw · 2023-08-15T13:43:06Z

Purpose

The pull request does:

change target NET60_Windows back to Any CPU for all windows net6 builds
remove target NET60_Linux from Dynamo.All.sln

This is so our build pipelines on master-5 and master-15 can share the same folder structure for all net48 and net6.0 jobs.

Declarations

Check these if you believe they are true

The codebase is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning and are documented in the API Changes document.
This PR modifies some build requirements and the readme is updated
This PR contains no files larger than 50 MB

Release Notes

(FILL ME IN) Brief description of the fix / enhancement. Mandatory section

Reviewers

(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)

(FILL ME IN, optional) Any additional notes to reviewers or testers.

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

…6139

src/Config/CS_SDK.props

tools/NuGet/template-artifactory/DynamoVisualProgramming.DynamoCoreRuntimeNET60.nuspec

mjkkirschner · 2023-08-15T17:58:02Z

src/Config/CS_SDK.props

    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
-    <Platform Condition=" '$(Platform)' == '' ">NET60_Windows</Platform>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>


hmm, so what is the point of these conditions now? Just to avoid linux? Can we remove most of these?

Yes, let me clean up that file.

mjkkirschner · 2023-08-15T17:59:17Z

src/Config/CS_SDK.props

@@ -33,8 +33,8 @@
    <SelfContained>false</SelfContained>
  </PropertyGroup>
    <PropertyGroup>
-    <DebugType Condition="$(Platform.Contains('NET60')) ">portable</DebugType>
-    <DebugType Condition="!$(Platform.Contains('NET60')) ">full</DebugType>
+    <DebugType Condition="$(Platform.Contains('AnyCPU')) ">portable</DebugType>


you can remove this and just make it full in all cases these days.

mjkkirschner · 2023-08-15T18:03:35Z

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>
  </PropertyGroup>
-  <PropertyGroup Condition="$(Platform.Contains('Windows'))" >
+  <PropertyGroup Condition="$(Platform.Contains('AnyCPU'))" >


so we'll only use AnyCPU for windows? Seems a bit confusing to me 😉

Yes, and Yes, it is.

mjkkirschner · 2023-08-15T18:04:47Z

src/build.xml

@@ -19,7 +21,7 @@

 <Target Name="RestorePackages">
  <Exec Condition="!Exists($(NuGetPath))" Command="powershell.exe -ExecutionPolicy ByPass -Command Invoke-WebRequest -Uri https://dist.nuget.org/win-x86-commandline/latest/nuget.exe -OutFile $(NuGetPath)" />
-  <Exec Command="dotnet restore $(SolutionNET6) -p:Platform=NET60_Windows --runtime=win10-x64"/>
+  <Exec Command="dotnet restore $(Solution) -p:Platform=&quot;$(Platform)&quot; --runtime=$(RuntimeIdentifier)"/>


do you need then " escaping?

Platform now contains a space. But that argument is probably not needed any more.

mjkkirschner

a couple questions

…6139

mjkkirschner · 2023-08-16T00:32:31Z

src/Dynamo.All.sln

@@ -199,6 +199,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DSOfficeUtilities", "Librar
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DynamoCoreWpfTests", "..\test\DynamoCoreWpfTests\DynamoCoreWpfTests.csproj", "{7DD8077A-201E-4C56-96C5-3C901A51BDF3}"
 EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ViewExtensionLibraryTests", "..\test\ViewExtensionLibraryTests\ViewExtensionLibraryTests.csproj", "{AE7F2579-104A-4AF4-AA7B-614AE9E79279}"


have you also added this binary to the net6 test script or you prefer to do this later?

I will update that file at merge time .

aparajit-pratap · 2023-08-16T18:05:51Z

test/ViewExtensionLibraryTests/Setup.cs

@@ -10,7 +10,7 @@ public class Setup
 {
    private AssemblyHelper assemblyHelper;

-    [SetUp]
+    [OneTimeSetUp]


We need to be careful about this. It could either be one time or could be a setup to be run before each test, in which case, we must leave it as Setup attribute. Can you check?

All tests passed locally for me but, I will check.

aparajit-pratap · 2023-08-16T18:06:12Z

test/ViewExtensionLibraryTests/Setup.cs

@@ -28,7 +28,7 @@ public void SetUp()
        AppDomain.CurrentDomain.AssemblyResolve += assemblyHelper.ResolveAssembly;
    }

-    [TearDown]
+    [OneTimeTearDown]


similarly for teardown

I think these attributes on test fixtures were only run once in nunit 2 (so I think this is the correct change) - but it can't hurt to double check the previous behavior before merging.

aparajit-pratap

Just one comment on test attributes.

mjkkirschner · 2023-08-16T18:21:02Z

@zeusongit @sm6srw do you expect some of the binary diff checks to fail on this PR?

zeusongit · 2023-08-16T18:54:05Z

@zeusongit @sm6srw do you expect some of the binary diff checks to fail on this PR?

Looks like the build step failed in the diff job for Net60_Linux, re-running.

"D:\a\Dynamo\Dynamo\master_net60_Win_Dynamo\src\Dynamo.All.sln" (default target) (1) ->
"D:\a\Dynamo\Dynamo\master_net60_Win_Dynamo\test\Libraries\DynamoMSOfficeTests\DynamoMSOfficeTests.csproj" (default target) (51) ->
(CoreCompile target) -> 
  D:\a\Dynamo\Dynamo\master_net60_Win_Dynamo\test\Libraries\DynamoMSOfficeTests\Setup.cs(33,13): error CS0234: The type or namespace name 'ExcelInterop' does not exist in the namespace 'DSOffice' (are you missing an assembly reference?) [D:\a\Dynamo\Dynamo\master_net60_Win_Dynamo\test\Libraries\DynamoMSOfficeTests\DynamoMSOfficeTests.csproj]
  D:\a\Dynamo\Dynamo\master_net60_Win_Dynamo\test\Libraries\DynamoMSOfficeTests\ExcelTests.cs(1444,29): error CS0117: 'Data' does not contain a definition for 'OpenXMLImportExcel' [D:\a\Dynamo\Dynamo\master_net60_Win_Dynamo\test\Libraries\DynamoMSOfficeTests\DynamoMSOfficeTests.csproj]

    723 Warning(s)
    2 Error(s)

sm6srw · 2023-08-16T19:04:46Z

I expect them to fail but not like that. I will take a look.

sm6srw added 18 commits August 10, 2023 15:09

First pass NET60_Windows -> Any CPU

a5f9af3

More Fixes

88174da

More tweaks

dd05d29

More tweaks

38dd92b

Another one

cf1cb8d

Test

4e451f4

More testing

f8940cd

One more test

4ebd3ea

More tweaks

c88d810

Revert back

626dfe2

More fixes

0a607d3

Try with single quotes

d3d338d

Add escape

a6a14f3

Use default platform

1e6340f

Merge branch 'master' of https://github.com/DynamoDS/Dynamo into DYN-…

103b701

…6139

Revert

c7080ed

More fixes

f90e860

One more fix.

036c68a

sm6srw requested review from mjkkirschner and aparajit-pratap August 15, 2023 13:53

aparajit-pratap reviewed Aug 15, 2023

View reviewed changes

src/Config/CS_SDK.props Show resolved Hide resolved

aparajit-pratap reviewed Aug 15, 2023

View reviewed changes

tools/NuGet/template-artifactory/DynamoVisualProgramming.DynamoCoreRuntimeNET60.nuspec Outdated Show resolved Hide resolved

More spec file tweaks

26b2bd5

mjkkirschner reviewed Aug 15, 2023

View reviewed changes

avidit changed the title ~~Dyn 6139 - Align pipelines~~ Dyn-6139 - Align pipelines Aug 15, 2023

Add ViewExtensionLibraryTests to solution.

6fec5d1

sm6srw added 3 commits August 15, 2023 16:25

Clean up props file

bfa92b9

Merge branch 'master' of https://github.com/DynamoDS/Dynamo into DYN-…

4d247b4

…6139

Update sln.

fb14968

mjkkirschner reviewed Aug 16, 2023

View reviewed changes

More tweaks

c905182

aparajit-pratap reviewed Aug 16, 2023

View reviewed changes

aparajit-pratap approved these changes Aug 16, 2023

View reviewed changes

mjkkirschner approved these changes Aug 16, 2023

View reviewed changes

sm6srw merged commit c3bbdf0 into DynamoDS:master Aug 17, 2023

sm6srw deleted the DYN-6139 branch August 17, 2023 11:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dyn-6139 - Align pipelines #14261

Dyn-6139 - Align pipelines #14261

sm6srw commented Aug 15, 2023

mjkkirschner Aug 15, 2023

sm6srw Aug 15, 2023

mjkkirschner Aug 15, 2023

mjkkirschner Aug 15, 2023

sm6srw Aug 15, 2023 •

edited

Loading

mjkkirschner Aug 15, 2023

sm6srw Aug 15, 2023 •

edited

Loading

mjkkirschner left a comment

mjkkirschner Aug 16, 2023

sm6srw Aug 16, 2023

aparajit-pratap Aug 16, 2023

sm6srw Aug 16, 2023

aparajit-pratap Aug 16, 2023

mjkkirschner Aug 16, 2023 •

edited

Loading

aparajit-pratap left a comment

mjkkirschner commented Aug 16, 2023

zeusongit commented Aug 16, 2023

sm6srw commented Aug 16, 2023 •

edited

Loading

Dyn-6139 - Align pipelines #14261

Dyn-6139 - Align pipelines #14261

Conversation

sm6srw commented Aug 15, 2023

Purpose

Declarations

Release Notes

Reviewers

FYIs

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

sm6srw Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sm6srw Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

mjkkirschner left a 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 Aug 16, 2023 • edited Loading

Choose a reason for hiding this comment

aparajit-pratap left a comment

Choose a reason for hiding this comment

mjkkirschner commented Aug 16, 2023

zeusongit commented Aug 16, 2023

sm6srw commented Aug 16, 2023 • edited Loading

sm6srw Aug 15, 2023 •

edited

Loading

sm6srw Aug 15, 2023 •

edited

Loading

mjkkirschner Aug 16, 2023 •

edited

Loading

sm6srw commented Aug 16, 2023 •

edited

Loading