fix: get content hash for master on local engine branches#172792
fix: get content hash for master on local engine branches#172792auto-submit[bot] merged 5 commits intoflutter:masterfrom
Conversation
The content hash doesn't exist for local engine changes; so fetch the one from master.
|
fyi @robert-ancell / @mdebbar / @gaaclarke / @bdero who had problems with this. |
matanlurey
left a comment
There was a problem hiding this comment.
LGTM based on tests :)
|
It looks like |
|
Unless perhaps the |
main should be banned and we should remove it. :) |
|
Tweaking - the "$BASEREF" should be the actual merge-base |
Thanks for confirming! I have to fix tests now since "origin/master" doesn't exist in the test repo |
|
autosubmit label was removed for flutter/flutter/172792, because - The status or check suite Mac tool_integration_tests_3_5 has failed. Please fix the issues identified (or deflake) before re-applying this label. |
its a flake. rerun passed. |
The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards #171790 re-land attempt for #172792 with the following changes: 1. content_aware_hash.(ps1|sh) now consider multiple branches to choose between HEAD and merge-base. 2. content_aware_hash_test.dart updated for these new requirements 3. content_aware_hash_test.dart allows for forcing powershell on mac for testing 4. updated docs/tool/Engine-artifacts.md documentation. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
…173114)" (#173145) <!-- start_original_pr_link --> Reverts: #173114 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jtmcdole <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: there is still another problem with the merge queue causing the content hash to be different: git revision: e13dd53 actual hash: 4b6f7b0f9849efaa59f515c8e95f3f27a6eb2ffb hash in the queue? 9e5b2eef4ba79b15b4f80dbba812d199d262366f <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jtmcdole <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {matanlurey, chingjun} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards #171790 re-land attempt for #172792 with the following changes: 1. content_aware_hash.(ps1|sh) now consider multiple branches to choose between HEAD and merge-base. 2. content_aware_hash_test.dart updated for these new requirements 3. content_aware_hash_test.dart allows for forcing powershell on mac for testing 4. updated docs/tool/Engine-artifacts.md documentation. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
…mpt) (#173169) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards #171790 re-land attempt for #173114 (try 2) re-land attempt for #172792 (original) The first commit in this PR is the previously LGTM'd changes for the above; with tests. The second commit is the critical change to make this work in post submits (fixes #173143). It turns out that while LUCI reports the GitHub private branches; our recipes directly checkout the git sha. This matters because the content scripts couldn't determine the branch name and the rev-parse was just HEAD. This lead the scripts down the merge-base logic, which returns the previous commit. A test was added specifically for this. Alternatively to this change, we could have checked for LUCI_CONTEXT being present in the environment. This is checked by Flutter tools in some cases, but not by any other scripts in `bin/internal`. The downside to checking HEAD: if you have a local branch with engine changes and you move back a revision - `dart`/`flutter` invocations will generate the hash for your local changes and fail.
…mpt) (flutter#173169) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#173114 (try 2) re-land attempt for flutter#172792 (original) The first commit in this PR is the previously LGTM'd changes for the above; with tests. The second commit is the critical change to make this work in post submits (fixes flutter#173143). It turns out that while LUCI reports the GitHub private branches; our recipes directly checkout the git sha. This matters because the content scripts couldn't determine the branch name and the rev-parse was just HEAD. This lead the scripts down the merge-base logic, which returns the previous commit. A test was added specifically for this. Alternatively to this change, we could have checked for LUCI_CONTEXT being present in the environment. This is checked by Flutter tools in some cases, but not by any other scripts in `bin/internal`. The downside to checking HEAD: if you have a local branch with engine changes and you move back a revision - `dart`/`flutter` invocations will generate the hash for your local changes and fail.
…3114) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#172792 with the following changes: 1. content_aware_hash.(ps1|sh) now consider multiple branches to choose between HEAD and merge-base. 2. content_aware_hash_test.dart updated for these new requirements 3. content_aware_hash_test.dart allows for forcing powershell on mac for testing 4. updated docs/tool/Engine-artifacts.md documentation. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
…lutter#173114)" (flutter#173145) <!-- start_original_pr_link --> Reverts: flutter#173114 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jtmcdole <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: there is still another problem with the merge queue causing the content hash to be different: git revision: e13dd53 actual hash: 4b6f7b0f9849efaa59f515c8e95f3f27a6eb2ffb hash in the queue? 9e5b2eef4ba79b15b4f80dbba812d199d262366f <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jtmcdole <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {matanlurey, chingjun} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#172792 with the following changes: 1. content_aware_hash.(ps1|sh) now consider multiple branches to choose between HEAD and merge-base. 2. content_aware_hash_test.dart updated for these new requirements 3. content_aware_hash_test.dart allows for forcing powershell on mac for testing 4. updated docs/tool/Engine-artifacts.md documentation. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
…mpt) (flutter#173169) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#173114 (try 2) re-land attempt for flutter#172792 (original) The first commit in this PR is the previously LGTM'd changes for the above; with tests. The second commit is the critical change to make this work in post submits (fixes flutter#173143). It turns out that while LUCI reports the GitHub private branches; our recipes directly checkout the git sha. This matters because the content scripts couldn't determine the branch name and the rev-parse was just HEAD. This lead the scripts down the merge-base logic, which returns the previous commit. A test was added specifically for this. Alternatively to this change, we could have checked for LUCI_CONTEXT being present in the environment. This is checked by Flutter tools in some cases, but not by any other scripts in `bin/internal`. The downside to checking HEAD: if you have a local branch with engine changes and you move back a revision - `dart`/`flutter` invocations will generate the hash for your local changes and fail.
…2792) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790
…lutter#172792)" (flutter#172805) <!-- start_original_pr_link --> Reverts: flutter#172792 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jtmcdole <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: almost, but not quite right. android builders generate the wrong hash because they are on custom branches. <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jtmcdole <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {bdero, matanlurey} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
…3114) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#172792 with the following changes: 1. content_aware_hash.(ps1|sh) now consider multiple branches to choose between HEAD and merge-base. 2. content_aware_hash_test.dart updated for these new requirements 3. content_aware_hash_test.dart allows for forcing powershell on mac for testing 4. updated docs/tool/Engine-artifacts.md documentation. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
…lutter#173114)" (flutter#173145) <!-- start_original_pr_link --> Reverts: flutter#173114 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jtmcdole <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: there is still another problem with the merge queue causing the content hash to be different: git revision: e13dd53 actual hash: 4b6f7b0f9849efaa59f515c8e95f3f27a6eb2ffb hash in the queue? 9e5b2eef4ba79b15b4f80dbba812d199d262366f <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jtmcdole <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {matanlurey, chingjun} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#172792 with the following changes: 1. content_aware_hash.(ps1|sh) now consider multiple branches to choose between HEAD and merge-base. 2. content_aware_hash_test.dart updated for these new requirements 3. content_aware_hash_test.dart allows for forcing powershell on mac for testing 4. updated docs/tool/Engine-artifacts.md documentation. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
…mpt) (flutter#173169) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#173114 (try 2) re-land attempt for flutter#172792 (original) The first commit in this PR is the previously LGTM'd changes for the above; with tests. The second commit is the critical change to make this work in post submits (fixes flutter#173143). It turns out that while LUCI reports the GitHub private branches; our recipes directly checkout the git sha. This matters because the content scripts couldn't determine the branch name and the rev-parse was just HEAD. This lead the scripts down the merge-base logic, which returns the previous commit. A test was added specifically for this. Alternatively to this change, we could have checked for LUCI_CONTEXT being present in the environment. This is checked by Flutter tools in some cases, but not by any other scripts in `bin/internal`. The downside to checking HEAD: if you have a local branch with engine changes and you move back a revision - `dart`/`flutter` invocations will generate the hash for your local changes and fail.
…2792) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790
…lutter#172792)" (flutter#172805) <!-- start_original_pr_link --> Reverts: flutter#172792 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jtmcdole <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: almost, but not quite right. android builders generate the wrong hash because they are on custom branches. <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jtmcdole <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {bdero, matanlurey} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
…3114) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#172792 with the following changes: 1. content_aware_hash.(ps1|sh) now consider multiple branches to choose between HEAD and merge-base. 2. content_aware_hash_test.dart updated for these new requirements 3. content_aware_hash_test.dart allows for forcing powershell on mac for testing 4. updated docs/tool/Engine-artifacts.md documentation. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
…lutter#173114)" (flutter#173145) <!-- start_original_pr_link --> Reverts: flutter#173114 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jtmcdole <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: there is still another problem with the merge queue causing the content hash to be different: git revision: e13dd53 actual hash: 4b6f7b0f9849efaa59f515c8e95f3f27a6eb2ffb hash in the queue? 9e5b2eef4ba79b15b4f80dbba812d199d262366f <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jtmcdole <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {matanlurey, chingjun} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#172792 with the following changes: 1. content_aware_hash.(ps1|sh) now consider multiple branches to choose between HEAD and merge-base. 2. content_aware_hash_test.dart updated for these new requirements 3. content_aware_hash_test.dart allows for forcing powershell on mac for testing 4. updated docs/tool/Engine-artifacts.md documentation. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
…mpt) (flutter#173169) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#173114 (try 2) re-land attempt for flutter#172792 (original) The first commit in this PR is the previously LGTM'd changes for the above; with tests. The second commit is the critical change to make this work in post submits (fixes flutter#173143). It turns out that while LUCI reports the GitHub private branches; our recipes directly checkout the git sha. This matters because the content scripts couldn't determine the branch name and the rev-parse was just HEAD. This lead the scripts down the merge-base logic, which returns the previous commit. A test was added specifically for this. Alternatively to this change, we could have checked for LUCI_CONTEXT being present in the environment. This is checked by Flutter tools in some cases, but not by any other scripts in `bin/internal`. The downside to checking HEAD: if you have a local branch with engine changes and you move back a revision - `dart`/`flutter` invocations will generate the hash for your local changes and fail.
…2792) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790
…lutter#172792)" (flutter#172805) <!-- start_original_pr_link --> Reverts: flutter#172792 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jtmcdole <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: almost, but not quite right. android builders generate the wrong hash because they are on custom branches. <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jtmcdole <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {bdero, matanlurey} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
…3114) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#172792 with the following changes: 1. content_aware_hash.(ps1|sh) now consider multiple branches to choose between HEAD and merge-base. 2. content_aware_hash_test.dart updated for these new requirements 3. content_aware_hash_test.dart allows for forcing powershell on mac for testing 4. updated docs/tool/Engine-artifacts.md documentation. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
…lutter#173114)" (flutter#173145) <!-- start_original_pr_link --> Reverts: flutter#173114 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jtmcdole <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: there is still another problem with the merge queue causing the content hash to be different: git revision: e13dd53 actual hash: 4b6f7b0f9849efaa59f515c8e95f3f27a6eb2ffb hash in the queue? 9e5b2eef4ba79b15b4f80dbba812d199d262366f <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jtmcdole <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {matanlurey, chingjun} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#172792 with the following changes: 1. content_aware_hash.(ps1|sh) now consider multiple branches to choose between HEAD and merge-base. 2. content_aware_hash_test.dart updated for these new requirements 3. content_aware_hash_test.dart allows for forcing powershell on mac for testing 4. updated docs/tool/Engine-artifacts.md documentation. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
…mpt) (flutter#173169) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#173114 (try 2) re-land attempt for flutter#172792 (original) The first commit in this PR is the previously LGTM'd changes for the above; with tests. The second commit is the critical change to make this work in post submits (fixes flutter#173143). It turns out that while LUCI reports the GitHub private branches; our recipes directly checkout the git sha. This matters because the content scripts couldn't determine the branch name and the rev-parse was just HEAD. This lead the scripts down the merge-base logic, which returns the previous commit. A test was added specifically for this. Alternatively to this change, we could have checked for LUCI_CONTEXT being present in the environment. This is checked by Flutter tools in some cases, but not by any other scripts in `bin/internal`. The downside to checking HEAD: if you have a local branch with engine changes and you move back a revision - `dart`/`flutter` invocations will generate the hash for your local changes and fail.
…2792) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790
…lutter#172792)" (flutter#172805) <!-- start_original_pr_link --> Reverts: flutter#172792 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jtmcdole <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: almost, but not quite right. android builders generate the wrong hash because they are on custom branches. <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jtmcdole <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {bdero, matanlurey} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
…3114) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#172792 with the following changes: 1. content_aware_hash.(ps1|sh) now consider multiple branches to choose between HEAD and merge-base. 2. content_aware_hash_test.dart updated for these new requirements 3. content_aware_hash_test.dart allows for forcing powershell on mac for testing 4. updated docs/tool/Engine-artifacts.md documentation. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
…lutter#173114)" (flutter#173145) <!-- start_original_pr_link --> Reverts: flutter#173114 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jtmcdole <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: there is still another problem with the merge queue causing the content hash to be different: git revision: e13dd53 actual hash: 4b6f7b0f9849efaa59f515c8e95f3f27a6eb2ffb hash in the queue? 9e5b2eef4ba79b15b4f80dbba812d199d262366f <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jtmcdole <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {matanlurey, chingjun} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#172792 with the following changes: 1. content_aware_hash.(ps1|sh) now consider multiple branches to choose between HEAD and merge-base. 2. content_aware_hash_test.dart updated for these new requirements 3. content_aware_hash_test.dart allows for forcing powershell on mac for testing 4. updated docs/tool/Engine-artifacts.md documentation. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
…mpt) (flutter#173169) The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master". towards flutter#171790 re-land attempt for flutter#173114 (try 2) re-land attempt for flutter#172792 (original) The first commit in this PR is the previously LGTM'd changes for the above; with tests. The second commit is the critical change to make this work in post submits (fixes flutter#173143). It turns out that while LUCI reports the GitHub private branches; our recipes directly checkout the git sha. This matters because the content scripts couldn't determine the branch name and the rev-parse was just HEAD. This lead the scripts down the merge-base logic, which returns the previous commit. A test was added specifically for this. Alternatively to this change, we could have checked for LUCI_CONTEXT being present in the environment. This is checked by Flutter tools in some cases, but not by any other scripts in `bin/internal`. The downside to checking HEAD: if you have a local branch with engine changes and you move back a revision - `dart`/`flutter` invocations will generate the hash for your local changes and fail.
The content hash doesn't exist for local engine changes, except for on CI. If we detect we're on a branch with committed or uncommitted changes to engine files; use "master".
towards #171790