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

[CI] Add a roll-to-Dart-SDK job #131

Open
dcharkes opened this issue Sep 13, 2023 · 11 comments
Open

[CI] Add a roll-to-Dart-SDK job #131

dcharkes opened this issue Sep 13, 2023 · 11 comments
Labels
P3 A lower priority bug or feature request package:native_assets_builder type-infra A repository infrastructure change or enhancement

Comments

@dcharkes
Copy link
Collaborator

It would be nice if we can add a roll-to-Dart-SDK job that:

  1. Makes a CL for the Dart SDK changing the DEPS file to
    a. point the repository to GitHub instead of Dart mirror, and
    b. sets the commit hash to the PRs commit hash.
  2. Fires of the 5 package bots that run the tests (see current test results).

I have no idea how hard or easy this would be. cc @athomas @whesse Do we have anything like this already? How hard would this be?

This would prevent us breaking rolls into the Dart SDK. And this might be useful for other DEPS as well that roll into the Dart SDK as an early warning sign. cc @devoncarew

@dcharkes dcharkes added the type-infra A repository infrastructure change or enhancement label Sep 13, 2023
@dcharkes
Copy link
Collaborator Author

As an alternative we could try to match the tools on the GitHub CI and the Dart SDK.

However, this seems to be non-trivial:

  • Dart SDK uses bleeding edge clang 17.
  • And only up to clang 15 has been released stable. Trying to install a newer clang on the bots leads to The repository 'http://apt.llvm.org/unstable llvm-toolchain-16 InRelease' is not signed..

@athomas
Copy link
Member

athomas commented Sep 15, 2023

You could download the clang the SDK uses (but you'd have to bump it every now and then to match the SDK DEPS):
See the download link on https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/clang/linux-amd64/+/git_revision:6d667d4b261e81f325756fdfd5bb43b3b3d2451d

The package is just a zip file with some metadata json in it, so it could easily be fetched.

I'm assuming "on the bots" here refers to GitHub Actions.

@whesse
Copy link

whesse commented Sep 15, 2023

For repositories where we use Gerrit reviews, we have a copybara job that creates a Gerrit CL for each new GitHub PR. This would be a way to take GitHub PRs and run tryjobs for them.

But I think the disadvantages may outweigh the benefits. These Gerrit CLs would have to be clearly marked as not replacing the GitHub PR, and not to be landed. The extra complexity of having these CLs around would be confusing to users.

And I'm not sure this would help - the SDK DEPS would then need a CL with the ref of the Gerrit CL used for the native dependency, and gclient would need to check out that branch correctly.

@dcharkes
Copy link
Collaborator Author

You could download the clang the SDK uses (but you'd have to bump it every now and then to match the SDK DEPS): See the download link on https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/clang/linux-amd64/+/git_revision:6d667d4b261e81f325756fdfd5bb43b3b3d2451d

The package is just a zip file with some metadata json in it, so it could easily be fetched.

I'm assuming "on the bots" here refers to GitHub Actions.

We would then somehow need to also put that clang on the path, or pass some environment variables that the Dart tests recognize to use this clang instead of the pre-installed one.

For repositories where we use Gerrit reviews, we have a copybara job that creates a Gerrit CL for each new GitHub PR. This would be a way to take GitHub PRs and run tryjobs for them.

But I think the disadvantages may outweigh the benefits. These Gerrit CLs would have to be clearly marked as not replacing the GitHub PR, and not to be landed. The extra complexity of having these CLs around would be confusing to users.

This is not what we want, we don't want Gerrit for this repo. We want to run a Dart SDK LUCI bot with the DEPS file in the SDK updated.

And I'm not sure this would help - the SDK DEPS would then need a CL with the ref of the Gerrit CL used for the native dependency, and gclient would need to check out that branch correctly.

Yes exactly, so making a Gerrit for this repo doesn't help. We can make the SDK CL by modifying the DEPS file there (and making it point to the GitHub repo instead), no need for a Gerrit CL for this repo.

@dcharkes
Copy link
Collaborator Author

You could download the clang the SDK uses (but you'd have to bump it every now and then to match the SDK DEPS):
See the download link on https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/clang/linux-amd64/+/git_revision:6d667d4b261e81f325756fdfd5bb43b3b3d2451d

Trying a curl on the command line leads to <?xml version='1.0' encoding='UTF-8'?><Error><Code>AccessDenied</Code><Message>Access denied.</Message><Details>Anonymous caller does not have storage.objects.get access to the Google Cloud Storage object. Permission 'storage.objects.get' denied on resource (or it may not exist).</Details></Error>.

@athomas
Copy link
Member

athomas commented Sep 15, 2023

You could download the clang the SDK uses (but you'd have to bump it every now and then to match the SDK DEPS):
See the download link on https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/clang/linux-amd64/+/git_revision:6d667d4b261e81f325756fdfd5bb43b3b3d2451d

Trying a curl on the command line leads to <?xml version='1.0' encoding='UTF-8'?><Error><Code>AccessDenied</Code><Message>Access denied.</Message><Details>Anonymous caller does not have storage.objects.get access to the Google Cloud Storage object. Permission 'storage.objects.get' denied on resource (or it may not exist).</Details></Error>.

Just use the download link, not the link I sent. I sent that link because it's more helpful if you want to upgrade the version later (you can just replace the git hash).

@dcharkes
Copy link
Collaborator Author

You could download the clang the SDK uses (but you'd have to bump it every now and then to match the SDK DEPS):
See the download link on https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/clang/linux-amd64/+/git_revision:6d667d4b261e81f325756fdfd5bb43b3b3d2451d

Trying a curl on the command line leads to <?xml version='1.0' encoding='UTF-8'?><Error><Code>AccessDenied</Code><Message>Access denied.</Message><Details>Anonymous caller does not have storage.objects.get access to the Google Cloud Storage object. Permission 'storage.objects.get' denied on resource (or it may not exist).</Details></Error>.

Just use the download link, not the link I sent. I sent that link because it's more helpful if you want to upgrade the version later (you can just replace the git hash).

That was using the download link
image

@whesse
Copy link

whesse commented Sep 15, 2023

I forgot to say what problem I was trying to solve: I'm not sure that changing the DEPS to point the third-party dependency native to a GitHub repository branch will work. Depot tools may refuse to check out the dependency from there, because the git host isn't in the allowed list of source repository hosts for the project to check out from. Or it might not download the PR ref, even if it could use the main branch.

Have you tried running a try job with this DEPS change? That was my only concern, that these checkouts would not work.

@dcharkes
Copy link
Collaborator Author

I forgot to say what problem I was trying to solve: I'm not sure that changing the DEPS to point the third-party dependency native to a GitHub repository branch will work. Depot tools may refuse to check out the dependency from there, because the git host isn't in the allowed list of source repository hosts for the project to check out from. Or it might not download the PR ref, even if it could use the main branch.

Have you tried running a try job with this DEPS change? That was my only concern, that these checkouts would not work.

Yes, I've been doing this before when I was developing. It requires git cl upload --bypass-hooks, then it works.

@dcharkes
Copy link
Collaborator Author

Note from chat:

The ingredients [for getting GitHub actions support for LUCI jobs] are:

  • A way to trigger a build for PRs (flutter probably has something that can be copied/adapted, but not directly used). Most likely this is recipes.
  • Probably some GoB mirroring.
  • A LUCI recipe that does what you want (could possibly be done with neo.py and test_matrix.json if you want to work in an SDK checkout).
  • A builder in infra/config.

@athomas
Copy link
Member

athomas commented Sep 15, 2023

You could download the clang the SDK uses (but you'd have to bump it every now and then to match the SDK DEPS):
See the download link on https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/clang/linux-amd64/+/git_revision:6d667d4b261e81f325756fdfd5bb43b3b3d2451d

Trying a curl on the command line leads to <?xml version='1.0' encoding='UTF-8'?><Error><Code>AccessDenied</Code><Message>Access denied.</Message><Details>Anonymous caller does not have storage.objects.get access to the Google Cloud Storage object. Permission 'storage.objects.get' denied on resource (or it may not exist).</Details></Error>.

Just use the download link, not the link I sent. I sent that link because it's more helpful if you want to upgrade the version later (you can just replace the git hash).

That was using the download link image

You're right, also it's a signed URL so hardcoding it won't work. The CIPD client in depot_tools could possibly be used or we could just manually upload the ZIP file to a more easily accessible location for this.

@dcharkes dcharkes added P3 A lower priority bug or feature request package:native_assets_builder labels Nov 16, 2023
@dcharkes dcharkes added this to the Native Assets v1.1 milestone Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 A lower priority bug or feature request package:native_assets_builder type-infra A repository infrastructure change or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants