Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update SDK to preview 1 release #98476

Merged
merged 10 commits into from
Feb 28, 2024
Merged

Conversation

am11
Copy link
Member

@am11 am11 commented Feb 15, 2024

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 15, 2024
@am11 am11 added area-Infrastructure and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 15, 2024
@ghost
Copy link

ghost commented Feb 15, 2024

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

Versions taken from https://dotnet.microsoft.com/en-us/download/dotnet/9.0.

related: dotnet/arcade#14485

Author: am11
Assignees: -
Labels:

area-Infrastructure

Milestone: -

@am11
Copy link
Member Author

am11 commented Feb 15, 2024

cc @lewing for wasm/wasi failures.
cc @MichaelSimons for sourcebuild failures.

rest of the legs are green.

@MichaelSimons
Copy link
Member

cc @MichaelSimons for sourcebuild failures.

Repo owners should make an attempt at investigating first.

@am11
Copy link
Member Author

am11 commented Feb 15, 2024

Repo owners should make an attempt at investigating first.

Ok, sorry about that. Only reason I pinged you was because your were pinged the last time we had this error:

/__w/1/s/artifacts/sb/src/eng/versioning.targets(198,5): error : Native SourceLink file could not be made available to the native build. Ensure that runtime-prereqs ran the sourcelink targets. [/__w/1/s/artifacts/sb/src/src/coreclr/runtime-prereqs.proj]

@ViktorHofer
Copy link
Member

image

@dotnet/source-build-internal the ReadSourceBuildIntermediateNupkgDependencies task returns duplicate items. One roslyn intermediate item has the metadata Name = Microsoft.CodeAnalysis on it and the other has Name = Microsoft.SourceBuild.Intermediate.roslyn. Can you please investigate?

@ViktorHofer
Copy link
Member

OK I think I understand the issue. Submitted #98942

@MichaelSimons
Copy link
Member

@ellahathaway - Can you take a look at what happened here? The UX here when there are multiple source-build dependencies on the same repo isn't ideal. I was expecting a failure with an obvious error message.

@ellahathaway
Copy link
Member

ellahathaway commented Feb 26, 2024

@ellahathaway - Can you take a look at what happened here? The UX here when there are multiple source-build dependencies on the same repo isn't ideal. I was expecting a failure with an obvious error message.

Investigating. The duplication should've caused an error like dotnet/arcade-validation#4190 (comment) since the SB metadata tag was on a non-intermediate, so I'm guessing that a different exception was hit first.

Follow up: I replicated the duplicate package reference in another repo and ran ./build.sh -sb. The error similar to dotnet/arcade-validation#4190 (comment) occurred as expected. As a result, I suspect that what happened here is due to something in the runtime infra rather than my specific changes to alter to the SB metadata error handling. I'm continuing to investigate.

@ViktorHofer
Copy link
Member

ViktorHofer commented Feb 27, 2024

image

I suspect this changed with dotnet/sourcelink@0647320. @tmat PTAL

From a successful build without the new SDK:

image

@ViktorHofer
Copy link
Member

Trying something out with the latest commit.

@ViktorHofer
Copy link
Member

Looking at the commits in sourcelink again, I think it's dotnet/sourcelink#1150 which regressed this.

@am11
Copy link
Member Author

am11 commented Feb 27, 2024

Shall we try setting https://github.com/dotnet/arcade/pull/14485/files#diff-9da24614831c308827a1ae533ffea392c97638c261dd42bd0f5226baa136d16eR16 here?

WASM failure is related to CPM, somehow SDK is seeing the Version Details infra as CPM? 👀

@ViktorHofer
Copy link
Member

ViktorHofer commented Feb 27, 2024

We already set that:

<RepositoryUrl>https://github.com/dotnet/$(GitHubRepositoryName)</RepositoryUrl>

This is a bigger issue. Sourcelink doesn't work at all when cloning from a local file path. For arcade the workaround was just to set this property manually as arcade doesn't validate that sourcelink works correctly, but runtime does. Note that this only affects the source-build repo build.

@ViktorHofer
Copy link
Member

Yes this really is dotnet/sourcelink#1150. @dotnet/source-build-internal sourcelink doesn't support cloning from a local path anymore which source-build does. This affects all repositories that use the inner clone feature and upgrade to a .NET 8.0.2xx or 9.0 Preview 1 SDK. It's probably only runtime that errors because we have a test that validates that sourcelink works correctly.

Related: dotnet/source-build#4117

Due to the new vstest terminal logger we didn't get the test list anymore from stdout. Fix by reverting back to the old logger.
@am11 am11 requested a review from akoeplinger as a code owner February 27, 2024 15:54
@clamp03
Copy link
Member

clamp03 commented Mar 5, 2024

@am11 After this PR, I found that coreclr test is not built. However, I don't know well about SDK so I don't know how to fix it. Could you please take a look at this?
Thank you!

How to reproduce
ROOTFS_DIR=/home/clamp/Work/dotnet/rootfs/riscv64 ./src/tests/build.sh -riscv64 -clang15 -Checked -priority1 -cross

Error message

   1:2>/home/clamp/Work/dotnet/riscv/crossgen2/src/tests/Common/test_dependencies_fs/test_dependencies.fsproj : error NU1101: Unable to find package Microsoft.NETCore.App.Runtime.linux-riscv64. No packages exist with this id in source(s): /home/clamp/Work/dotnet/riscv/crossgen2/.dotnet/sdk/9.0.100-preview.1.24101.2/FSharp/library-packs, dotnet-eng, dotnet-libraries, dotnet-libraries-transport, dotnet-public, dotnet-tools, dotnet9, dotnet9-transport [/home/clamp/Work/dotnet/riscv/crossgen2/src/tests/build.proj]
   1:2>/home/clamp/Work/dotnet/riscv/crossgen2/src/tests/Common/test_dependencies_fs/test_dependencies.fsproj : error NU1101: Unable to find package Microsoft.NETCore.App.Host.linux-riscv64. No packages exist with this id in source(s): /home/clamp/Work/dotnet/riscv/crossgen2/.dotnet/sdk/9.0.100-preview.1.24101.2/FSharp/library-packs, dotnet-eng, dotnet-libraries,dotnet-libraries-transport, dotnet-public, dotnet-tools, dotnet9, dotnet9-transport [/home/clamp/Work/dotnet/riscv/crossgen2/src/tests/build.proj]
   1:2>/home/clamp/Work/dotnet/riscv/crossgen2/src/tests/build.proj(548,5): error MSB3073: The command ""/home/clamp/Work/dotnet/riscv/crossgen2/.dotnet/dotnet" restore Common/test_dependencies_fs/test_dependencies.fsproj  /p:SetTFMForRestore=true /p:TargetOS=linux /p:TargetArchitecture=riscv64 /p:Configuration=Checked /p:CrossBuild=true" exited with code 1.

@am11
Copy link
Member Author

am11 commented Mar 5, 2024

@clamp03 #97791 (comment), we need to pass -p:UseLocalAppHostPack=true. BTW, this PR was merged last week and it was already needed since Feb 6 #97791 (comment).

@clamp03
Copy link
Member

clamp03 commented Mar 5, 2024

we need to pass -p:UseLocalAppHostPack=true.

@am11 Thank you for the comment. I tried to build coreclr test with -p:UseLocalAppHostPack=true. However, it still fails with the same error.

BTW, this PR was merged last week and it was already needed since Feb 6 #97791 (comment).

Actually, I checked the build with/without only this PR. (If head commit is Consolidate LINQ's internal IIListProvider/IPartition into base Iterator class e101ae2, it is okay. However it fails if head commit is this.)
Thank you.

CC @dotnet/samsung

@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants