Skip to content

validate engine hash #55057

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 17, 2020
Merged

validate engine hash #55057

merged 1 commit into from
Apr 17, 2020

Conversation

dnfield
Copy link
Contributor

@dnfield dnfield commented Apr 17, 2020

Fixes #27574

Validates that flutter_tester --help gives us a Flutter Engine Version that matches the contents of bin/internal/engine.version. This is meant to guard against regressions in the recipe scripts where we accidentally upload a revision to the wrong bucket folder.

@fluttergithubbot fluttergithubbot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Apr 17, 2020
@fluttergithubbot
Copy link
Contributor

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@dnfield
Copy link
Contributor Author

dnfield commented Apr 17, 2020

The bot is confused because this PR doesn't modify a _test.dart file, but this does in fact add a test.

dnfield
dnfield commented Apr 17, 2020
View reviewed changes
dev/bots/run_command.dart

final String commandDescription = '${path.relative(executable, from: workingDirectory)} ${arguments.join(' ')}';
final String relativeWorkingDir = path.relative(workingDirectory);
final String relativeWorkingDir = path.relative(workingDirectory ?? Directory.current.path);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This throws right now if someone calls it without specifying a working directory. If that's intended, we can change this to be an @required param, but I think this makes sense.

digiter
digiter approved these changes Apr 17, 2020
View reviewed changes
Copy link
Contributor

@digiter digiter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jonahwilliams
Copy link
Contributor

This is a good addition, but this only handles the upload side of #27574. The tool would also like to validate the cache contents after downloading

@dnfield
Copy link
Contributor Author

dnfield commented Apr 17, 2020

@jonahwilliams isn't this running after we download the artifacts again?

@dnfield dnfield merged commit 98651c0 into flutter:master Apr 17, 2020
@dnfield dnfield deleted the test_test branch April 17, 2020 23:30
@dnfield
Copy link
Contributor Author

dnfield commented Apr 17, 2020

If there's more work you want to do for the linked bug we can reopen it

@alexmarkov
Copy link
Contributor

This change broke Flutter-engine-Dart head-head-head bot:

Running smoketests...
▌10:18:29▐ RUNNING: cd .; bin/cache/artifacts/engine/linux-x64/flutter_tester --help
▌10:18:29▐ ELAPSED TIME: 0.058s for bin/cache/artifacts/engine/linux-x64/flutter_tester --help in .
Expected "Flutter Engine Version: 2b75c6d111f768b349311884212fc50b454b921d", but found "Flutter Engine Version: 189ec1e7c43030f67a6323cd743bd449d2719eab".

This might be because HHH bot replaces certain parts of Flutter (e.g. flutter/bin/cache/dart-sdk) with a version built from sources (at HEAD).

@dnfield
Copy link
Contributor Author

dnfield commented Apr 20, 2020

Maybe we could add a flag for the bot to bypass this check... Why doesn't the bot just use the local-engine flag though? Modifying the cache seems dangerous

dnfield added a commit that referenced this pull request Apr 20, 2020
@dnfield
Revert "validate engine hash (#55057)"
4a53d34 
This reverts commit 98651c0.
@dnfield dnfield mentioned this pull request Apr 20, 2020
Merged
dnfield added a commit that referenced this pull request Apr 20, 2020
@dnfield
Revert "validate engine hash (#55057)" (#55220)
6a490ed 
This reverts commit 98651c0.
dnfield added a commit to dnfield/flutter that referenced this pull request Apr 20, 2020
@dnfield
Reland "validate engine hash (flutter#55057)" (flutter#55220)
c11b5d5 
This reverts commit 6a490ed.
This was referenced Apr 20, 2020
Merged
Open
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Hixie Hixie Awaiting requested review from Hixie

@zanderso zanderso Awaiting requested review from zanderso

@jonahwilliams jonahwilliams Awaiting requested review from jonahwilliams

1 more reviewer

@digiter digiter digiter approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Flutter engine artifacts version mismatch

6 participants

@dnfield @fluttergithubbot @jonahwilliams @alexmarkov @digiter @googlebot