-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[main] Source code updates from dotnet/dotnet #123100
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
Conversation
|
Caution 🚨 Action Required — Conflict detectedA conflict was detected when trying to update this PR with changes from build The conflicts in the following files need to be manually resolved:
ℹ️ To resolve the conflicts, please follow these steps:
💡 You may consult the FAQ for more information or tag @dotnet/prodconsvcs for assistance. |
Diff: https://github.com/dotnet/dotnet/compare/7b9ad20ba1d45df5a99fdd9dedbf3bfe6a6fc24f..7b9ad20ba1d45df5a99fdd9dedbf3bfe6a6fc24f From: dotnet/dotnet@7b9ad20 To: dotnet/dotnet@7b9ad20 The following files had conflicts that were resolved by a user: - src/tasks/Crossgen2Tasks/Crossgen2Tasks.csproj - src/tasks/installer.tasks/installer.tasks.csproj
eng/pipelines/extra-platforms/runtime-extra-platforms-ioslike.yml
Outdated
Show resolved
Hide resolved
|
Two problems here with source build causing some of the task projects to fail to build.
Both of these look like a symptom of source-build behavior in VMR not lining up with source-build behavior post backflow in product repos. @jkoritzinsky would your thinking to replace this job with VMR build instead of the old source-build help? |
The newer package versions referenced might be newer than what's in the SDK (even in source build).
|
Yes, this is the job I wanted to replace with the VMR validation build. I believe it will fix these problems. (In fact, running the VMR validation job on backflow should always pass by definition). |
|
I think there's still value in keeping the repo source-build job active. |
|
@ViktorHofer what value does the repo source-build provide that VMR validation of the source build stage 1 and 2 jobs doesn't? My understanding is that the repo source-build job is in this halfway point of validating some source-build logic while not actually giving full validation that things will work in the VMR and having unique problems that aren't an issue for actual source build in the VMR. |
|
Some source-build distro partners build this repository directly with the --source-build build entry point switch. I think it's useful to keep validating that the switch works in general and does the right thing. This isn't about prebuilt detection but about the general build infrastructure (different TFMs than a normal build and SB conditions). |
premun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please give me a bit more time to investigate the codeflow and whether there are not more reverted changes we didn't notice. I am interviewing today but should get some time later in the day to have a look.
Let's not merge until then.
|
While I understand that some SB customers may use that flag during local development, the only time I have seen the repo source-build job break in a PR since moving to the VMR is this backflow during the rebranding. It hasn't caught any build breaks that would cause issues on flow into the VMR in my experience. I really don't see any value in fixing rebranding issues in it that are purely artificial and will disappear after we finish the rebrand + rebootstrap. |
|
It's your call, I'm just sharing my opinion. You probably haven't seen such failures because we operate on a well understood path. The moment something deviates from that, you see a source-build failure. That's what is observable right now in this PR. I don't understand what's happening here exactly but my gut reaction was telling me that this indicates a wrong configuration regardless of repo source-build vs product source-build in the VMR. |
|
I found the problem in the @ericstj thanks for reporting the |
premun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I found the bug and re-ran with a fixed version of darc and there was only this ioslike.yml file extra so we are in a good state here now
FWIW this one 1a6c5af was a torn state problem that was good to fix regardless. This one - b1a1903 seems more like the case where VMR source build is ahead of runtime source build and similar issues like this can crop up when make changes in VMR that impact sourcebuild but don't yet rebootstrap-sdk/update-packages in runtime. It looks like both of those got fixed - we can make the call on replace vs add separately. |
|
Most of the failures here are in PInvokeTableGenerator tests: One appears to be a flaky test: |
|
@akoeplinger / @lewing -- what do you suggest. Looks like WASM tests assume that the task TFM matches
But those tasks haven't been retargeted yet. I'm not inclined to do so in this PR as that could involve more breaks -- would be better after the SDK is updated. Is it really worth it for the tests to assert on the TFM of the tasks, or can they have more resilient probing? Update: I'll just fix the test to find the task assembly rather than assert that it must be in some TFM specific folder that must be kept in sync. Something more "product-focused" would be to have props that could be imported by the generated project. |
Confirmed with @mmitche that this is expected. |
We can keep it (remove the CI-only jobs though as the VMR validation will cover what that is supposed to cover), but I'd like to have a better understanding of the support story from Arcade for repo source-build going forward and why it makes sense for it to differ from the VMR's source build mode (ie having unique failures for repo source-build and not in the VMR feels like a shared infra problem to me, not a runtime repo problem). |
It is testing a real thing that has been a problem with when updating the tfm, it is very difficult to keep working when when crossing a tfm boundry |
Why don't you test it the way the product actually works? You generate a targets file for the SDK with the right path in it. Have the test import that in it's generated project... That way when the test fails you find a real product bug. As it stands it's just annoying duplication that's going to get out of sync. It's not even testing a "user" gesture. They sure don't hardcode TFMs and probe paths in the SDK package layout. That's just test fragility... Opened #123155 to track better cleanup here. |
Note
This is a codeflow update. It may contain both source code changes from
the VMR
as well as dependency updates. Learn more here.
This pull request brings the following source code changes
From https://github.com/dotnet/dotnet
New Dependencies
Updated Dependencies
Associated changes in source repos
Diff the source with this PR branch