Skip to content

netcoreapp3.1 all the things#6891

Merged
alexperovich merged 4 commits intodotnet:masterfrom
alexperovich:31
Feb 5, 2021
Merged

netcoreapp3.1 all the things#6891
alexperovich merged 4 commits intodotnet:masterfrom
alexperovich:31

Conversation

@alexperovich
Copy link
Member

@alexperovich alexperovich self-assigned this Feb 1, 2021
@alexperovich alexperovich requested review from ChadNedzlek, MattGal, chcosta and mmitche and removed request for mmitche February 5, 2021 18:41
Copy link
Member

@chcosta chcosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm glad this is happening, also... it's scary because these types of changes never "just work". :/ Hope this one goes well!

local logger_path="$toolset_dir/$_InitializeBuildToolFramework/Microsoft.DotNet.ArcadeLogging.dll"
if [[ ! -f $logger_path ]]; then
logger_path="$toolset_dir/$_InitializeBuildToolFramework/Microsoft.DotNet.Arcade.Sdk.dll"
# new scripts need to work with old packages, so we need to look for the old names/versions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm missing context here, why do new scripts need to work with old packages? what's an example?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pr build. Its building with old arcade that has the dll in netcoreapp2.1, but once things get published it will be using the arcade with the dll in netcoreapp3.1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that the only example? Seems slightly odd to code around that and open up this odd behavior for a one time thing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its really the whole arcade build, it doesn't work without this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow. Aren't we just talking about finding the logger here?

Also, is the logging code ever in netcoreapp3.1\Arcade.sdk.dll? It has already moved to Microsoft.DotNet.ArcadeLogging.dll

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we are talking about finding the logger. The arcade build itself uses version n-1 packages, with version n scripts, so the scripts have to be able to use the old names.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chatted with Alex and we agreed that this could get cleaned up post-merge. Thanks Alex!

if [[ ! -f $logger_path ]]; then
logger_path="$toolset_dir/$_InitializeBuildToolFramework/Microsoft.DotNet.Arcade.Sdk.dll"
# new scripts need to work with old packages, so we need to look for the old names/versions
local selectedPath=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought there was a convention at one point (or an attempt at one), and this would be selected_path.

nit: My preference would be selected_logger_path for clarity

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will remove this when I clean up post merge.

@alexperovich alexperovich merged commit 938b3e8 into dotnet:master Feb 5, 2021
@alexperovich alexperovich deleted the 31 branch February 5, 2021 22:07
@marek-safar
Copy link

It seems like this commit broke dotnet/runtime build due to

"eng/restore/harvestPackages.targets(12,5): error MSB4062: (NETCORE_ENGINEERING_TELEMETRY=Restore) The "GetLastStablePackage" task could not be loaded from the assembly /__w/1/s/.packages/microsoft.dotnet.build.tasks.packaging/6.0.0-beta.21105.12/tools/netcoreapp2.1/Microsoft.DotNet.Build.Tasks.Packaging.dll. Could not load file or assembly '/__w/1/s/.packages/microsoft.dotnet.build.tasks.packaging/6.0.0-beta.21105.12/tools/netcoreapp2.1/Microsoft.DotNet.Build.Tasks.Packaging.dll'.

more details at dotnet/runtime#48054

@MattGal
Copy link
Member

MattGal commented Feb 11, 2021

It seems like this commit broke dotnet/runtime build due to

"eng/restore/harvestPackages.targets(12,5): error MSB4062: (NETCORE_ENGINEERING_TELEMETRY=Restore) The "GetLastStablePackage" task could not be loaded from the assembly /__w/1/s/.packages/microsoft.dotnet.build.tasks.packaging/6.0.0-beta.21105.12/tools/netcoreapp2.1/Microsoft.DotNet.Build.Tasks.Packaging.dll. Could not load file or assembly '/__w/1/s/.packages/microsoft.dotnet.build.tasks.packaging/6.0.0-beta.21105.12/tools/netcoreapp2.1/Microsoft.DotNet.Build.Tasks.Packaging.dll'.

more details at dotnet/runtime#48054

@alexperovich is aware, will synch with him at FR standup shortly.

@alexperovich
Copy link
Member Author

This isn't a bug with arcade, we knew about this and runtime has to react to the change. The runtime repo has a hardcoded dependency on netcoreapp2.1 where they manually reference the dll for that task. That reference needs to be updated to netcoreapp3.1, or preferably completely stop manually referencing the dll.

@missymessa
Copy link
Member

@marek-safar I emailed the Partner's alias about this on Monday and @ViktorHofer said he would take care of this issue for dotnet/runtime.

@ViktorHofer
Copy link
Member

Already took care of it and just waiting for the build to progress: dotnet/runtime#48054.

@premun premun mentioned this pull request Feb 24, 2021
1 task
akoeplinger pushed a commit to akoeplinger/arcade that referenced this pull request Apr 12, 2021
* netcoreapp3.1 all the things

* Fix logger paths

* Put signtool tests stuff back

* Fix the tests
@AraHaan
Copy link
Member

AraHaan commented Dec 18, 2021

Is there any new plans to update all the tasks again to target .NET 6 since it is a new LTS release for arcade?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants