dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
152 stars 42 forks source link

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

Open dcharkes opened 1 year ago

dcharkes commented 1 year ago

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 commented 1 year ago

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:

athomas commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.