Skip to content

Conversation

@jtmcdole
Copy link
Member

@jtmcdole jtmcdole commented Aug 1, 2025

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

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • 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.
  • All existing and new tests are passing.

@jtmcdole jtmcdole requested a review from matanlurey August 1, 2025 16:06
@github-actions github-actions bot added tool Affects the "flutter" command-line tool. See also t: labels. team-tool Owned by Flutter Tool team d: docs/ flutter/flutter/docs, for contributors labels Aug 1, 2025
@jtmcdole jtmcdole requested a review from chingjun August 1, 2025 16:06
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a smart optimization for local engine development by using the merge-base for content hashing, which should significantly improve the developer experience. The changes are well-structured, with corresponding updates to tests and documentation.

I've added a few comments with suggestions to enhance the robustness of the shell scripts in edge cases where a merge-base might not be found, and a minor improvement to a test helper function. Overall, this is a solid contribution.

jtmcdole and others added 5 commits August 1, 2025 09:16
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
Lots of go-between to get testing working; but generally works.
@jtmcdole jtmcdole force-pushed the localHashBypassPartDuex branch from 7e046a4 to 20029c0 Compare August 1, 2025 16:16
@jtmcdole
Copy link
Member Author

jtmcdole commented Aug 1, 2025

I'm going to review what tests could be run that failed in post for #172802 which immediately followed the last attempt

Copy link
Contributor

@chingjun chingjun left a comment

Choose a reason for hiding this comment

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

LGTM with nits

@jtmcdole
Copy link
Member Author

jtmcdole commented Aug 1, 2025

form: #172792 (comment)

Before this PR, engineers working on the Engine or Platforms would be required to set FLUTTER_PREBUILT_ENGINE_VERSION to the main hash or git-sha in order to have flutter and dart command line tools work; otherwise the content_aware_hash.sh would return a hash that has not been built by any Flutter CI.

  • The previous attempt unfortunatly treated github private branches for the merge queues (gh-readonly-queue/master/pr-) were treated as local branches and thus the "hash" was actually for the previous element in the queue (or tip of tree) - not for the engine change.
    • This bug was particularly problematic for Android artifacts. The scripts that produce these artifacts query content_aware_hash.sh and don't have access to the correct content hash from Cocoon.
    • This lead to artifacts not getting uploaded for the real content hash (2de0d89afa16c5e7e441538a8c2ca0cc6754fa36) - they were instead marked with the previous hash. The evidence for this is in the actual buckets for android artifacts - looking at the previous build the hash is c0f0f51b891c558c53d3ecc9fc655592b5a2ccc5; as seen here (link not shown because of internal resources - but this should be commit-hash and content-hash-2de0d).
image
  • gh-readonly-queue/master/pr- is now checked by the scripts and defaults HEAD
  • other special branches (releases, stable, beta, main, master) all also default to HEAD (as documented in the engine artifacts markdown) but reproduced here:
Branch Hashed From
main, master HEAD
stable, beta HEAD
GitHub Merge Queue HEAD
flutter-*-candidate.x HEAD
Shallow Clones HEAD
Everything Else. merge-base between HEAD and(origin or upstream)/(main or master)

The other failure that happened that day was Cocoon not rescheduling failed builds. That was address in flutter/cocoon#4807 - such that if something skips the queue, the tracking document will get taken over by another build.

@vashworth vashworth mentioned this pull request Aug 1, 2025
9 tasks
@jtmcdole jtmcdole added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 1, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Aug 1, 2025
Merged via the queue into flutter:master with commit eb786c8 Aug 1, 2025
148 of 149 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 1, 2025
@jtmcdole
Copy link
Member Author

jtmcdole commented Aug 1, 2025

reason for revert: 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

9e5b2eef4ba79b15b4f80dbba812d199d262366f is the HEAD~ hash.

@jtmcdole jtmcdole added the revert Autorevert PR (with "Reason for revert:" comment) label Aug 1, 2025
auto-submit bot pushed a commit that referenced this pull request Aug 1, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 4, 2025
Roll Flutter from 871849e4b6bf to beda687d63f2 (34 revisions)

flutter/flutter@871849e...beda687

2025-08-04 [email protected] [Impeller] Improvements to the Vulkan pipeline cache data writer (flutter/flutter#173014)
2025-08-04 [email protected] [Impeller] Terminate the fence waiter but do not reset it during ContextVK shutdown (flutter/flutter#173085)
2025-08-04 [email protected] Add the 'windowing' feature flag and use to wrap an implementation for regular windows that always throws (flutter/flutter#172478)
2025-08-04 [email protected] licenses_cpp: reland switch 4 (flutter/flutter#173139)
2025-08-04 [email protected] Roll Skia from 763bba9c33fd to dce9550a1356 (1 revision) (flutter/flutter#173219)
2025-08-04 [email protected] Improve robustness of comment detection when using flutter analyze --suggestions (flutter/flutter#172977)
2025-08-04 [email protected] Make sure that a LicensePage doesn't crash in 0x0 environment (flutter/flutter#172610)
2025-08-04 [email protected] Roll Packages from f0645d8 to 1a72287 (4 revisions) (flutter/flutter#173215)
2025-08-04 [email protected] Roll Skia from edf0f8a5bba6 to 763bba9c33fd (1 revision) (flutter/flutter#173213)
2025-08-04 [email protected] [web] Add Intl.Locale to parse browser languages. (flutter/flutter#172964)
2025-08-04 [email protected] Roll Skia from 439f80973f4a to edf0f8a5bba6 (2 revisions) (flutter/flutter#173204)
2025-08-04 [email protected] Roll Skia from 9d30cc96a3b2 to 439f80973f4a (1 revision) (flutter/flutter#173201)
2025-08-04 [email protected] Roll Skia from 39c70f883c0e to 9d30cc96a3b2 (1 revision) (flutter/flutter#173197)
2025-08-04 [email protected] Roll Skia from 09667386bcba to 39c70f883c0e (1 revision) (flutter/flutter#173194)
2025-08-04 [email protected] fix: get content hash for master on local engine branches (third attempt)  (flutter/flutter#173169)
2025-08-03 [email protected] Roll Fuchsia Linux SDK from wZuA8Dqty4scQkZuV... to ufssK8EgJ_9RpLFgu... (flutter/flutter#173190)
2025-08-03 [email protected] Roll Skia from e056e47e118b to 09667386bcba (2 revisions) (flutter/flutter#173185)
2025-08-03 [email protected] fix: tag fuchsia package after uploading (flutter/flutter#173140)
2025-08-02 [email protected] Roll Skia from 7ef888e30a28 to e056e47e118b (1 revision) (flutter/flutter#173170)
2025-08-02 [email protected] Roll Fuchsia Linux SDK from yFLr81lLXmSX7n11D... to wZuA8Dqty4scQkZuV... (flutter/flutter#173166)
2025-08-02 [email protected] Roll Skia from af6d6eb383a6 to 7ef888e30a28 (1 revision) (flutter/flutter#173157)
2025-08-02 [email protected] Add `innerRadius` to `RadioThemeData` (flutter/flutter#173120)
2025-08-02 [email protected] Roll Skia from 81dd6aa516b0 to af6d6eb383a6 (2 revisions) (flutter/flutter#173144)
2025-08-01 98614782+auto-submit[bot]@users.noreply.github.com Reverts "fix: get content hash for master on local engine branches (#173114)" (flutter/flutter#173145)
2025-08-01 [email protected] Roll Dart SDK from 6e1bb159c860 to 3f1307d72d6f (2 revisions) (flutter/flutter#173136)
2025-08-01 [email protected] Roll Skia from 9e2c59634961 to 81dd6aa516b0 (1 revision) (flutter/flutter#173135)
2025-08-01 [email protected] Reformat text.dart's code snippets (flutter/flutter#172976)
2025-08-01 [email protected] experiment with docs properties (flutter/flutter#173124)
2025-08-01 [email protected] fix: get content hash for master on local engine branches (flutter/flutter#173114)
2025-08-01 [email protected] Make sure that a RawAutocomplete doesn't crash in 0x0 environment (flutter/flutter#172812)
2025-08-01 [email protected] Add skia_ports_fontmgr_android_parser_sources (flutter/flutter#172979)
2025-08-01 [email protected] Roll Skia from 49e39cd3cf18 to 9e2c59634961 (10 revisions) (flutter/flutter#173125)
2025-08-01 [email protected] fix: 🐛 Add equality and hashCode implementations to ResizeImage (flutter/flutter#172643)
2025-08-01 [email protected] Roll Fuchsia Linux SDK from xo_bqOoFuBKFmgKxn... to yFLr81lLXmSX7n11D... (flutter/flutter#173117)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

...
danilozhang pushed a commit to danilozhang/flutter that referenced this pull request Aug 6, 2025
…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.
SydneyBao pushed a commit to SydneyBao/flutter that referenced this pull request Aug 6, 2025
…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.
SydneyBao pushed a commit to SydneyBao/flutter that referenced this pull request Aug 6, 2025
…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]>
SydneyBao pushed a commit to SydneyBao/flutter that referenced this pull request Aug 6, 2025
…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.
ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Aug 19, 2025
…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.
ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Aug 19, 2025
…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]>
ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Aug 19, 2025
…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.
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
…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.
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
…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]>
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
…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.
korca0220 pushed a commit to korca0220/flutter that referenced this pull request Sep 22, 2025
…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.
korca0220 pushed a commit to korca0220/flutter that referenced this pull request Sep 22, 2025
…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]>
korca0220 pushed a commit to korca0220/flutter that referenced this pull request Sep 22, 2025
…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.
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
…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.
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
…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]>
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

d: docs/ flutter/flutter/docs, for contributors team-tool Owned by Flutter Tool team tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants