Skip to content

Sync eng/common/{cross,native} with arcade#5892

Open
am11 wants to merge 1 commit into
dotnet:mainfrom
am11:chore/sync-eng-common
Open

Sync eng/common/{cross,native} with arcade#5892
am11 wants to merge 1 commit into
dotnet:mainfrom
am11:chore/sync-eng-common

Conversation

@am11

@am11 am11 commented Jun 19, 2026

Copy link
Copy Markdown
Member

It stopped syncing automatically few months ago for some reason, so new platforms' configs aren't showing up. See #5858 (comment).

@am11

am11 commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

cc @hoyosjs, @akoeplinger it includes dotnet/arcade#17027.

hoyosjs
hoyosjs previously approved these changes Jun 19, 2026
@hoyosjs

hoyosjs commented Jun 19, 2026

Copy link
Copy Markdown
Member

@am11 - have you noticed this in other repos? And is it just these dirs?

@am11

am11 commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

@am11 - have you noticed this in other repos?

Yup, in SDK and aspnetcore. Up until last month, only runtime and diagnostics were using native infra from eng/common. Now that aspnetcore and SDK tools are AOT published for few weeks, we should sync the entire eng/common without any filtering across arcade-powered repos. IMO, it's much simpler (and it seems like the original intent when eng/common/README.md file was first written..).

And is it just these dirs?

Just copied the entire dir, pushed a commit. Seems like there was more out of sync, so something is filtering the contents. :/

@am11 am11 requested a review from a team as a code owner June 19, 2026 19:43
@am11

am11 commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

@hoyosjs, turned out additional changes in new eng/common required new dotnet download command which is available in SDK 11.0, so I first merged #5858 to this PR, then updated global.json to 11.0. Note that this is just a "build toolchain" version, it doesn't change <TargetFramework of individual projects.

@hoyosjs

hoyosjs commented Jun 20, 2026

Copy link
Copy Markdown
Member

We can't use .NET 11 SDK in this repo - we use the release branch of arcade

@hoyosjs hoyosjs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We use the arcade 10 branch

@hoyosjs

hoyosjs commented Jun 20, 2026

Copy link
Copy Markdown
Member

10.0.3xx also has the feature - but I believe it might not be available in source build

@am11 am11 force-pushed the chore/sync-eng-common branch from 39253f3 to 3233398 Compare June 20, 2026 10:18
@am11

am11 commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

@hoyosjs, the MicrosoftDotNetArcadeSdkPackageVersion still points to v10 in this PR. The only thing which has bumped is the toolchain which is capable of handling more versions beyond v10. That should not have any impact on the final product (it still uses the same libraries versions to build the product). If this is a problem, let me know and i will reset to the original shape of PR: sync only eng/common/{cross,native} sub-dirs. But I think it's best to understand if the concern is valid with unchanged actual dependency versions.

@hoyosjs

hoyosjs commented Jun 20, 2026

Copy link
Copy Markdown
Member

We need to build with a shipping/supported SDK that has a production cleared CSC. It's not so much about arcade as much as it is the SDK. The right way to fix this would be to backport the eng/common changes needed to arcade release/10.0.

@am11 am11 force-pushed the chore/sync-eng-common branch from 3233398 to c6b6e54 Compare June 21, 2026 02:53
@am11

am11 commented Jun 21, 2026

Copy link
Copy Markdown
Member Author

I've removed the commit which was anyway out of scope, was trying to help.

production cleared CSC.

  1. CSC is backwards compatible all the way up to C# 1.0 from circa 2002.
  2. CSC uses language version your project has specified, and if you haven't used <LangVersion, it caps language version based on the targeting framework: https://github.com/dotnet/roslyn/blob/db649bc3ca504bd1236a119d1cd86c7f8050e95b/src/Compilers/Core/MSBuildTask/Microsoft.CSharp.Core.targets#L23

The chance that it can break diagnostics related nuget package after successfully building .NET assembly and clearing the CI is highly unlikely, if not absolutely zero. I suggest switch eng/common to new plan until the day we actually have a bug report due to the speculative compat issue.

@hoyosjs

hoyosjs commented Jun 22, 2026

Copy link
Copy Markdown
Member

It's not speculative - it's a requirement for us from a security and compliance perspective. We used to be on that plan and things were easier. But diagnostics ships out of band and arcade bumping the SDK to any nightly version is not something we can do

@hoyosjs

hoyosjs commented Jun 22, 2026

Copy link
Copy Markdown
Member

For what it's worth - I really appreciate all the help you give us in this repo and runtime. The rules are a little bit different between the two as they don't ship at the same time and in the same set of products. I do like eng/common being shared - but if we have the .NET 10 one and we'd miss something, that's the harder part. We've had build breaks before with a preview SDK on a point where we needed to ship. What's the main thing you need from the eng folder in diagnostics with this PR? Or is it just about having things being in lockstep and you wanted to make them be aligned?

@am11

am11 commented Jun 22, 2026

Copy link
Copy Markdown
Member Author

@hoyosjs, I have reverted rest of the eng/common. This PR only has native and cross dirs where we add new platforms for native parts and there is no C# code.

security and compliance

When the RTM version is released, SDK in runtime repo points to release candidate rather than the final version https://github.com/dotnet/runtime/blob/v10.0.0/global.json. That does not make runtime and framework libraries, the main product, non-compliant and neither would it make diagnostic package non-compliant.

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.

2 participants