-
Notifications
You must be signed in to change notification settings - Fork 361
Update latest flutter candidate script to use simpler commands #4849
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
tool/latest_flutter_candidate.sh
Outdated
| fi | ||
|
|
||
| LATEST_FLUTTER_CANDIDATE="flutter-$LATEST_VERSION" | ||
| LATEST_FLUTTER_CANDIDATE=$(git ls-remote https://dart.googlesource.com/external/github.com/flutter/flutter.git \ |
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 outputs refs/heads/ .... and we will need it to output flutter-version-candidate for our purposes. We may need two scripts: one that does this that the auto roller can use and another that uses this and strips refs/heads/ from the output for our purposes. Either that, or our other scripts that use this need to strip refs/heads/ from the output before they use it
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've update this so that it mimicks the old behaviour.
Do you know if it would be ok for the autoroller to just append refs/head on the branch name if it is needed?
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.
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.
Yes, that is fine.
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.
@athomas @kenzieschmoll I've updated the remote that we are touching to git@github.com:flutter/flutter.git
it looks like the other remote we were pointing to didn't have all of the refs for the candidate branches, so it was grabbing old candidate versions
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.
ugh it looks like in order to use the flutter/flutter remote we'll have to:
- generate a private key for the flutter/flutter repo,
- save it to the devtools repo as a secret token
- use that token to auth against the flutter/flutter repo so that our
git ls-remotecommand will have permission to fetch the refs
Alternatively we could find out how to make sure the refs are up to date on https://dart.googlesource.com/external/github.com/flutter/flutter.git
@kenzieschmoll @athomas do either of you have opinions about these options?
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.
Alternatively we could find out how to make sure the refs are up to date on https://dart.googlesource.com/external/github.com/flutter/flutter.git
@athomas is there expected latency between flutter/flutter on github and flutter/flutter on dart.googlesource.com ?
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.
Under normal circumstances, the latency should be very low. It's a mirror, so it's not instant.
The Dart mirror might be special in some way, I can check if we're still mirroring everything (we had some hacks around main branches before, not sure if those are still there). An alternative would be https://flutter.googlesource.com/mirrors/flutter/ which might be a more exact mirror.
clean up beta readme
|
@athomas once this lands, can we push https://dart-review.git.corp.google.com/c/recipes/+/265320 forward? |
This was discussed/requested here
This change makes the script less dependant on non-standard commands, so it can be used in more environments