Skip to content

[flutter_tool] Don't download CanvasKit if it's already in flutter_web_sdk#113072

Merged
harryterkelsen merged 4 commits intoflutter:masterfrom
harryterkelsen:flutter-tool-check-for-canvaskit-in-engine
Oct 7, 2022
Merged

[flutter_tool] Don't download CanvasKit if it's already in flutter_web_sdk#113072
harryterkelsen merged 4 commits intoflutter:masterfrom
harryterkelsen:flutter-tool-check-for-canvaskit-in-engine

Conversation

@harryterkelsen
Copy link
Contributor

Don't download CanvasKit from CIPD if it's already included in the flutter_web_sdk engine artifacts.

We are building CanvasKit locally in the Flutter engine now, and soon will be bundling it with flutter_web_sdk. This change causes flutter_tool to use the built version rather than the one from CIPD when the built version is available.

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.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@harryterkelsen harryterkelsen added tool Affects the "flutter" command-line tool. See also t: labels. platform-web Web applications specifically e: web_canvaskit CanvasKit (a.k.a. Skia-on-WebGL) rendering backend for Web labels Oct 6, 2022
@jonahwilliams
Copy link
Contributor

Going forward, will we still need to support downloading from the canvaskit URL, or is the idea to only use the prebuilt?

Copy link
Contributor

@eyebrowsoffire eyebrowsoffire left a comment

Choose a reason for hiding this comment

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

LGTM!

@harryterkelsen
Copy link
Contributor Author

The plan is to only use the prebuilt version

@jonahwilliams
Copy link
Contributor

It sounds like this is for a soft transition we need to make then? Could you link a bug in the code here with a note that it can all be deleted once the work is completed?

@harryterkelsen
Copy link
Contributor Author

Done. PTAL

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e: web_canvaskit CanvasKit (a.k.a. Skia-on-WebGL) rendering backend for Web platform-web Web applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants