`_downloadArtifacts` (Web SDK) uses content-aware hashing in post-submit #174236

matanlurey · 2025-08-21T20:23:02Z

Will need to get cherry-picked into 3.35 and 3.36.

flutter-dashboard · 2025-08-21T20:23:05Z

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

gemini-code-assist

Code Review

This pull request updates the Web SDK artifact download process to use content-aware hashing instead of Git revisions. The changes introduce a new contentHash variable and modify the download URL accordingly. My review identified several critical issues in the implementation that would cause compilation failures. Specifically, there are uninitialized variables and syntax errors in common.dart, and a field naming inconsistency in environment.dart that breaks the constructor. I have provided detailed comments and code suggestions to resolve these problems.

engine/src/flutter/lib/web_ui/dev/common.dart

engine/src/flutter/lib/web_ui/dev/environment.dart

eyebrowsoffire

LGTM, assuming it works!

harryterkelsen

LGTM

matanlurey · 2025-08-21T22:54:13Z

Looks like this is not working. Example failure:

Running `dart pub get` in 'engine/src/flutter/lib/web_ui'
Pipeline experienced the following failures:
  "copy_artifacts": Could not download flutter-web-sdk.zip from cloud bucket at URL: https://storage.googleapis.com/flutter_archives_v2/flutter_infra_release/flutter/1dc06b550ced0908e59a43c06a26353b1aa2536a/flutter-web-sdk.zip. Response status code: 404
#0      CopyArtifactsStep._downloadArtifacts (file:///b/s/w/ir/cache/builder/engine/src/flutter/lib/web_ui/dev/steps/copy_artifacts_step.dart:64:7)
<asynchronous suspension>
#1      CopyArtifactsStep.run (file:///b/s/w/ir/cache/builder/engine/src/flutter/lib/web_ui/dev/steps/copy_artifacts_step.dart:96:37)
<asynchronous suspension>
#2      Pipeline.run (file:///b/s/w/ir/cache/builder/engine/src/flutter/lib/web_ui/dev/pipeline.dart:136:9)
<asynchronous suspension>
#3      TestCommand.run (file:///b/s/w/ir/cache/builder/engine/src/flutter/lib/web_ui/dev/test_runner.dart:407:7)
<asynchronous suspension>
#4      CommandRunner.runCommand (package:args/command_runner.dart:212:13)
<asynchronous suspension>
#5      main (file:///b/s/w/ir/cache/builder/engine/src/flutter/lib/web_ui/dev/felt.dart:41:26)
<asynchronous suspension>

Test pipeline failed.

The content hash was 1dc06b550ced0908e59a43c06a26353b1aa2536a.

I'm trying to figure out what previously running process was expected to have uploaded this, but did not.

matanlurey · 2025-08-21T23:02:01Z

This is set:

flutter/engine/src/flutter/ci/builders/linux_web_engine_build.json

Lines 10 to 12 in 8632719

    
           "luci_flags": { 
        
             "upload_content_hash": true 
        
           },

But I don't see where it is uploading

matanlurey · 2025-08-21T23:10:39Z

Here is the original CL that added support for content-hashed uploads:

if flags.get('upload_content_hash', False):
    content_hash_url = api.repo_util.get_content_hash_url(
        api, path.remote, api.repo_util.content_hash()
    )
    api.archives.upload_artifact(path.local, content_hash_url)
    api.flutter_bcid.upload_provenance(path.local, content_hash_url)

... which makes sense. The value we care about comes from properties.content_hash:

def content_hash(self):
  """Returns the content hash if present, or None

      The content hash is a source-derived value that fingerprints the artifacts
      with the engine source that produced it. By moving to a content hash, we
      build less copies of the same engine source code and speed up CI.

      See: https://github.com/flutter/flutter/blob/master/bin/internal/content_aware_hash.sh
  """
  return self.m.properties.get('content_hash', None)

... but I don't see that property passed in anywhere here.

eyebrowsoffire

I have an alternative suggestion that might be simpler for detecting presubmit, but either way probably works.

eyebrowsoffire · 2025-08-22T18:33:05Z

engine/src/flutter/lib/web_ui/dev/steps/copy_artifacts_step.dart

@@ -57,7 +57,7 @@ class CopyArtifactsStep implements PipelineStep {
    };
    final Uri url = Uri.https(
      'storage.googleapis.com',
-      '${realmComponent}flutter_infra_release/flutter/$gitRevision/flutter-web-sdk.zip',
+      '${realmComponent}flutter_infra_release/flutter/${isPostsubmit ? contentHash : gitRevision}/flutter-web-sdk.zip',


You might be able to just do realm == LuciRealm.Try ? gitRevision : contentHash instead of reading the LUCI_PR environment variable. It's probably equivalent functionally, so it's up to you.

Ah that's better.

I made it a negative so that, if you use this tool locally, it defaults to git revision instead of content hash.

The way I'm reading the code, only LuciRealm.Try will use gitRevision. Does local development use LuciRealm.Try or LuciRealm.Unknown?

Basically:

realm == LuciRealm.Try ? gitRevision : contentHash

and

realm != LuciRealm.Try ? contentHash : gitRevision

are the same thing.

What you probably want to do is realm == LuciRealm.Prod ? contentHash : gitRevision

This code path is never going to be exercised locally (and your actual local build will not ever be in GCS) so that point is moot.

I think the staging realm is postsubmit + bringup: true so I think that should be contentHash as well. Hence, only the try realm should actually use gitRevision right now, IMO

Ok that makes sense.

My comment was a response to:

I made it a negative so that, if you use this tool locally, it defaults to git revision instead of content hash.

So if that point is moot, please carry on 🙂

Makes sense. Done as ${realm == LuciRealm.Try ? gitRevision : contentHash}

matanlurey · 2025-08-22T19:01:47Z

I am excited to land this, and find out if it works!11!

flutter-dashboard · 2025-08-22T19:53:04Z

Detected the emergency label.

If you add the autosubmit label, the bot will wait until all presubmits pass but ignore the tree status, allowing fixes for tree breakages while still validating that they don't break any existing presubmits.

The "Merge" button is also unlocked. To bypass presubmits as well as the tree status, press the GitHub "Add to Merge Queue".

matanlurey · 2025-08-22T19:54:00Z

Skipping remaining presubmits, the one test that could have been impacted (in presubmit) has passed.

On postsubmit, this might fail. If it does, I'll revert it.

…mit (flutter#174236) Towards flutter#174225. Will need to get cherry-picked into 3.35 and 3.36.

…n post-submit (flutter/flutter#174236)

flutter/flutter@26bb33b...edd434a 2025-08-23 engine-flutter-autoroll@skia.org Roll Skia from 6f710e0b38f7 to 61169c1f6f7c (1 revision) (flutter/flutter#174325) 2025-08-23 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Z-ZaFQ7jAqJ1OrIBf... to XLSNQCsY1VkIthSjt... (flutter/flutter#174318) 2025-08-23 engine-flutter-autoroll@skia.org Roll Skia from ebb6051e8bb1 to 6f710e0b38f7 (1 revision) (flutter/flutter#174317) 2025-08-22 1961493+harryterkelsen@users.noreply.github.com [web] Expose rasterizers in Renderer (flutter/flutter#174308) 2025-08-22 jhy03261997@gmail.com Update some semantics flags updated to use enum (engine, framework, web) (flutter/flutter#170696) 2025-08-22 bkonyi@google.com [ Tool ] Don't emit artifact downloading messages when --machine is provided (flutter/flutter#174301) 2025-08-22 engine-flutter-autoroll@skia.org Roll Skia from cb15e1452399 to ebb6051e8bb1 (5 revisions) (flutter/flutter#174296) 2025-08-22 matanlurey@users.noreply.github.com `_downloadArtifacts` (Web SDK) uses content-aware hashing in post-submit (flutter/flutter#174236) 2025-08-22 jacksongardner@google.com Refactor text and runtime effect to separate skia and impeller implementations. (flutter/flutter#174219) 2025-08-22 engine-flutter-autoroll@skia.org Roll Packages from 58c02e0 to 092d832 (4 revisions) (flutter/flutter#174295) 2025-08-22 bkonyi@google.com [ Widget Preview ] Add support for DevTools configuration (flutter/flutter#174272) 2025-08-22 32538273+ValentinVignal@users.noreply.github.com Migrate more files to `WidgetStateProperty` (flutter/flutter#174268) 2025-08-22 bkonyi@google.com [ Tool ] Ensure `--dds-port=<port>` is respected when targeting web devices (flutter/flutter#174278) 2025-08-22 fluttergithubbot@gmail.com Marks Mac_ios ios_debug_workflow to be unflaky (flutter/flutter#174104) 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 bmparr@google.com,stuartmorgan@google.com 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 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

matanlurey added 2 commits August 21, 2025 13:21

Convert downloadArtifacts (Web SDK) to use Content-Aware Hashing

329dd12

Remove unused gitRevision

ca28d1e

matanlurey requested review from jtmcdole and eyebrowsoffire August 21, 2025 20:23

github-actions bot added engine flutter/engine repository. See also e: labels. platform-web Web applications specifically labels Aug 21, 2025

gemini-code-assist bot reviewed Aug 21, 2025

View reviewed changes

engine/src/flutter/lib/web_ui/dev/common.dart Outdated Show resolved Hide resolved

engine/src/flutter/lib/web_ui/dev/environment.dart Outdated Show resolved Hide resolved

matanlurey mentioned this pull request Aug 21, 2025

[Blocking Release Branches] Could not download flutter-web-sdk.zip #174225

Closed

Fix some mistypes

aba495e

matanlurey changed the title ~~Convert downloadArtifacts (Web SDK) to use Content-Aware Hashing~~ Convert _downloadArtifacts (Web SDK) to use Content-Aware Hashing Aug 21, 2025

eyebrowsoffire approved these changes Aug 21, 2025

View reviewed changes

++

dd6036a

harryterkelsen approved these changes Aug 21, 2025

View reviewed changes

This was referenced Aug 21, 2025

Engine artifacts not uploaded by content hash for try/prod bots #174244

Closed

Provide a 'content_hash' LUCI property for engine try-builds flutter/cocoon#4842

Closed

Use gitRevision in presubmit, contentHash in postsubmit.

6bca0ac

matanlurey changed the title ~~Convert _downloadArtifacts (Web SDK) to use Content-Aware Hashing~~ _downloadArtifacts (Web SDK) uses content-aware hashing in post-submit Aug 22, 2025

matanlurey requested a review from eyebrowsoffire August 22, 2025 18:28

eyebrowsoffire approved these changes Aug 22, 2025

View reviewed changes

++

1ad3277

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 22, 2025

matanlurey added 2 commits August 22, 2025 11:45

Prod first

409825e

Switch it up

8c6ee21

jtmcdole approved these changes Aug 22, 2025

View reviewed changes

matanlurey added the emergency Jump the queue; land PR in front of all others; only use for emergencies label Aug 22, 2025

matanlurey added this pull request to the merge queue Aug 22, 2025

Merged via the queue into flutter:master with commit 6c7d642 Aug 22, 2025
183 of 184 checks passed

matanlurey deleted the copy-artifacts-step-cah branch August 22, 2025 20:40

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 22, 2025

matanlurey added cp: beta cherry pick this pull request to beta release candidate branch cp: stable cherry pick this pull request to stable release candidate branch labels Aug 22, 2025

flutteractionsbot pushed a commit to flutteractionsbot/flutter that referenced this pull request Aug 22, 2025

_downloadArtifacts (Web SDK) uses content-aware hashing in post-sub…

2a5ee38

…mit (flutter#174236) Towards flutter#174225. Will need to get cherry-picked into 3.35 and 3.36.

flutteractionsbot pushed a commit to flutteractionsbot/flutter that referenced this pull request Aug 22, 2025

_downloadArtifacts (Web SDK) uses content-aware hashing in post-sub…

26bb212

…mit (flutter#174236) Towards flutter#174225. Will need to get cherry-picked into 3.35 and 3.36.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 23, 2025

6c7d6429b _downloadArtifacts (Web SDK) uses content-aware hashing i…

ada4144

…n post-submit (flutter/flutter#174236)

engine-flutter-autoroll mentioned this pull request Aug 23, 2025

Roll Flutter from 26bb33b9572b to edd434af3e9d (14 revisions) flutter/packages#9873

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 23, 2025

6c7d6429b _downloadArtifacts (Web SDK) uses content-aware hashing i…

3a806f6

…n post-submit (flutter/flutter#174236)

engine-flutter-autoroll mentioned this pull request Aug 23, 2025

Roll Flutter from 26bb33b9572b to edd434af3e9d (14 revisions) flutter/packages#9874

Merged

_downloadArtifacts (Web SDK) uses content-aware hashing in post-submit #174236

_downloadArtifacts (Web SDK) uses content-aware hashing in post-submit #174236

Conversation

matanlurey commented Aug 21, 2025

Uh oh!

flutter-dashboard bot commented Aug 21, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

eyebrowsoffire left a comment

Choose a reason for hiding this comment

Uh oh!

harryterkelsen left a comment

Choose a reason for hiding this comment

Uh oh!

matanlurey commented Aug 21, 2025

Uh oh!

matanlurey commented Aug 21, 2025

Uh oh!

matanlurey commented Aug 21, 2025

Uh oh!

eyebrowsoffire left a comment

Choose a reason for hiding this comment

Uh oh!

eyebrowsoffire Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

matanlurey Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

mdebbar Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdebbar Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

eyebrowsoffire Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

mdebbar Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

matanlurey Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matanlurey commented Aug 22, 2025

Uh oh!

flutter-dashboard bot commented Aug 22, 2025

Uh oh!

matanlurey commented Aug 22, 2025

Uh oh!

Uh oh!

Uh oh!

`_downloadArtifacts` (Web SDK) uses content-aware hashing in post-submit #174236

`_downloadArtifacts` (Web SDK) uses content-aware hashing in post-submit #174236

mdebbar Aug 22, 2025 •

edited

Loading

matanlurey Aug 22, 2025 •

edited

Loading