-
Notifications
You must be signed in to change notification settings - Fork 520
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
[build] Use arcade dependency management tooling #10890
Conversation
🔥 Tests failed catastrophically on Build (no summary found). 🔥Result file $(TEST_SUMMARY_PATH) not found. Pipeline on Agent |
<disabledPackageSources> | ||
<clear /> | ||
</disabledPackageSources> | ||
<config> |
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.
The changes in this file looks like whitespace only changes?
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.
The darc
tool formatted this file as part of dependency updating. I figure it's best to include those formatting changes in this initial PR so future auto PRs will only change version information.
@rolfbjarne one other related issue that I wanted your opinion on is that the update tooling currently requires that a global.json
file exist in the root of the repo. I am going to test this later today, but I think we may need to change the way the global.json
is generated as part of this PR as well. Do you have any initial thoughts on a preferred way to include a default global.json
? It can be empty, it just needs to exist for the tool to run.
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.
If need a global.json
in the repo, we have to stop generating it (or modifying it during the build), otherwise things will become annoying quite fast (with the file always showing up as modified).
I think the best option would be to:
- Stop generating it
- Commit the current generated result
- Compute DOTNET_VERSION from it
- Modify system-dependencies.sh to grep global.json instead of Make.config for the .NET version.
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.
Question, an we just do the trick I do with make + sed to stop needed to parse? Call make system-dependecies.sh and take the values from make.config.. I know, it add an extra step. Or maybe, is there a way to import the env values from the makefile?
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.
A root global.json file has been added, and the new dotnet.config
make file now includes both DOTNET_VERSION
and DOTNET6_VERSION
. This keeps other $(DOTNET_VERSION)
references working. DOTNET_URL
has been moved from Make.config
to system-dependencies.sh
, and made "evergreen" by using a URL that stays constant (aside from the version).
DOTNET6_VERSION_BAND=$(firstword $(subst -, ,$(DOTNET6_VERSION))) | ||
DOTNET6_URL=https://dotnetcli.blob.core.windows.net/dotnet/Sdk/6.0.100-preview.2.21155.3/dotnet-sdk-6.0.100-preview.2.21155.3-osx-x64.pkg | ||
DOTNET6_TARBALL=https://dotnetcli.blob.core.windows.net/dotnet/Sdk/6.0.100-preview.2.21155.3/dotnet-sdk-6.0.100-preview.2.21155.3-osx-x64.tar.gz | ||
DOTNET6_TARBALL=https://dotnetcli.blob.core.windows.net/dotnet/Sdk/$(DOTNET6_VERSION)/dotnet-sdk-$(DOTNET6_VERSION)-osx-x64.tar.gz |
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.
@rolfbjarne that was requested (thrice iirc) and you mentioned using a variable would break other stuff (not that I can remember what exactly)
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.
@spouliot the reason was: #10501 (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.
I think the changes in https://github.com/xamarin/xamarin-macios/pull/9885/files#diff-9e58bdf3f9a2fcd07e4cc0d4647a855b75995005a790f785d4ba6333d1fa7872L1021 removed the dependency that system-dependencies.sh
had on the DOTNET6*
variables. I removed DOTNET6_URL
in this PR because I think that was the only place referencing it previously (and nothing seems to reference it now). We can generate the URL variable declaration as well if needed though.
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.
This is an example of my left hand not knowing what my right hand does 🤦♂️
@pjcollins is right, we only look for DOTNET_VERSION
now in system-dependencies.sh, the DOTNET6_*
variables don't matter anymore.
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.
As mentioned in the comment: #10501 (comment) we should not use the variable, or fix the dependencies.sh script.
DOTNET6_VERSION_BAND=$(firstword $(subst -, ,$(DOTNET6_VERSION))) | ||
DOTNET6_URL=https://dotnetcli.blob.core.windows.net/dotnet/Sdk/6.0.100-preview.2.21155.3/dotnet-sdk-6.0.100-preview.2.21155.3-osx-x64.pkg | ||
DOTNET6_TARBALL=https://dotnetcli.blob.core.windows.net/dotnet/Sdk/6.0.100-preview.2.21155.3/dotnet-sdk-6.0.100-preview.2.21155.3-osx-x64.tar.gz | ||
DOTNET6_TARBALL=https://dotnetcli.blob.core.windows.net/dotnet/Sdk/$(DOTNET6_VERSION)/dotnet-sdk-$(DOTNET6_VERSION)-osx-x64.tar.gz |
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.
@spouliot the reason was: #10501 (comment)
❌ Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffTest results3 tests failed, 45 tests passed.Failed tests
Pipeline on Agent XAMBOT-1103' |
<disabledPackageSources> | ||
<clear /> | ||
</disabledPackageSources> | ||
<config> |
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.
If need a global.json
in the repo, we have to stop generating it (or modifying it during the build), otherwise things will become annoying quite fast (with the file always showing up as modified).
I think the best option would be to:
- Stop generating it
- Commit the current generated result
- Compute DOTNET_VERSION from it
- Modify system-dependencies.sh to grep global.json instead of Make.config for the .NET version.
🔥 Tests failed catastrophically on Build (no summary found). 🔥Result file $(TEST_SUMMARY_PATH) not found. Pipeline on Agent |
🔥 Tests failed catastrophically on Build (no summary found). 🔥Result file $(TEST_SUMMARY_PATH) not found. Pipeline on Agent |
🔥 Tests failed catastrophically on Build (no summary found). 🔥Result file $(TEST_SUMMARY_PATH) not found. Pipeline on Agent |
I think it would be nice to have the explanation of this whole process documented in the repo itself (i.e. the PR description), because almost nobody think think to look at the PR after it's been merged. Maybe add it as |
🔥 Tests failed catastrophically on Build (no summary found). 🔥Result file $(TEST_SUMMARY_PATH) not found. Pipeline on Agent |
❌ Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffTest results19 tests failed, 163 tests passed.Failed tests
Pipeline on Agent XAMBOT-1101' |
At least some of these failures are due to the bump itself, and will be fixed with #10772. |
❌ Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffTest results19 tests failed, 163 tests passed.Failed tests
Pipeline on Agent XAMBOT-1096' |
…ur locally built ones. The new version of .NET ships with our workloads, but those aren't the workloads we want to use, so replace them with our own.
❌ Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffTest results5 tests failed, 177 tests passed.Failed tests
Pipeline on Agent XAMBOT-1094' |
Threading changes broke waits: dotnet/runtime#50245 This is blocked until that bug is fixed (and an updated .NET 6 version is available), then this PR will have to be updated with the new .NET 6 version. |
That required renaming simulator runtime packs...
❌ Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffTest results1 tests failed, 79 tests passed.Failed tests
Pipeline on Agent XAMBOT-1097' |
❌ Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffTest results1 tests failed, 79 tests passed.Failed tests
Pipeline on Agent XAMBOT-1095' |
This fix the issue with `Wait` that failed several tests in monotouch-tests However it does not include the fix for AppConext.GetData on device (AOT)
* [build] Use arcade dependency management tooling * Apply feedback * Apply second round of feedback * Always make dotnet.config before trying to read it * Debugging * Update dependencies, trim tabs and spaces * [dotnet] Remove the existing workload shipped with .NET and install our locally built ones. The new version of .NET ships with our workloads, but those aren't the workloads we want to use, so replace them with our own. * Update .gitignores. * Bump to 6.0.100-preview.3.21181.5 That required renaming simulator runtime packs... * More rename for simulator packages * moar (hopefully all) * Bump to 6.0.100-preview.3.21201.11 This fix the issue with `Wait` that failed several tests in monotouch-tests However it does not include the fix for AppConext.GetData on device (AOT) Co-authored-by: Rolf Bjarne Kvinge <[email protected]> Co-authored-by: Sebastien Pouliot <[email protected]>
Context: https://github.com/dotnet/arcade/blob/ea609b8e036359934332480de9336d98fcbb3f91/Documentation/Darc.md
The dotnet core engineering group has some dependency management tooling
(known as
darc
) that we'd like to adopt. Integrating this tooling into the build system will make it easier to stay up to date with the
latest .NET 6 SDK changes.
Many dotnet repos use a publishing workflow that will push build
artifact data to a central location known as the "Build Asset Registry".
This data includes a "Channel" association, which is the key to
dependency updating. Local updates and automatic update "Subscriptions"
compare the version files in a given repo against the product versions
available in the channel that you are interested in.
We hope to be able to publish Xamarin SDK build information to this
central registry in the near future, so that other products can take a
dependency on us as needed (MAUI for instance).
The
darc
tool looks for four different files in a repo when adding adependency or when checking for an update:
Both of the
Version
files present in theeng
folder are updated whena new dependency is available.
To work with
darc
locally you'll need to install the global tool, jointhe
arcade-contrib
GitHub team, and configure your auth settings.To add a new dependency, use the
darc add-dependency
command:To update all dependencies, use the
darc update-dependencies
command:
To configure automatic updates, use the
darc add-subscription
command to enroll a target repo/branch into updates from a particular
channel:
Once a subscription is configured, a pull request will be created
automatically by the dotnet Maestro bot when dependency updates are
available.