Skip to content

Change priority of gen_snapshot search paths #37647

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.

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

Merged
merged 1 commit into from
Aug 6, 2019
Merged

Change priority of gen_snapshot search paths #37647

merged 1 commit into from
Aug 6, 2019

Conversation

liamappelbe
Copy link
Contributor

Description

Arm gen_snapshot has moved from clang_x86 to clang_x64 as of flutter/engine#10010. This causes a problem for people upgrading from older versions of flutter, because they will still have the old gen_snapshot in the clang_x86 directory in their artifacts cache.

We can't delete the clang_x86 directory from the list of searched directories, because we still build x86 targeting gen_snapshot as an x86 binary. So as a hacky fix I've changed the order of the directories so that we search for clang_x64 before falling back to clang_x86. This should handle the upgrade case.

Related Issues

#22598

@fluttergithubbot
Copy link
Contributor

It looks like this pull request may not have tests. Please make sure to add tests before merging. While there are exceptions to this rule, if this patch modifies code it is probably not an exception.

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

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 5, 2019
@codecov
Copy link

codecov bot commented Aug 5, 2019

Codecov Report

Merging #37647 into master will decrease coverage by 1.23%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #37647      +/-   ##
==========================================
- Coverage   56.24%      55%   -1.24%     
==========================================
  Files         193      193              
  Lines       18143    18015     -128     
==========================================
- Hits        10204     9910     -294     
- Misses       7939     8105     +166
Flag Coverage Δ
#flutter_tool 55% <ø> (-1.24%) ⬇️
Impacted Files Coverage Δ
packages/flutter_tools/lib/src/artifacts.dart 68.08% <ø> (ø) ⬆️
...s/flutter_tools/lib/src/windows/visual_studio.dart 2.17% <0%> (-80.44%) ⬇️
...ges/flutter_tools/lib/src/resident_web_runner.dart 7.04% <0%> (-62.56%) ⬇️
...s/flutter_tools/lib/src/test/flutter_platform.dart 0.53% <0%> (-27.88%) ⬇️
packages/flutter_tools/lib/src/run_cold.dart 10.38% <0%> (-9.37%) ⬇️
packages/flutter_tools/lib/src/android/gradle.dart 42.6% <0%> (-2.46%) ⬇️
packages/flutter_tools/lib/src/web/web_device.dart 49.01% <0%> (-1.97%) ⬇️
packages/flutter_tools/lib/src/version.dart 90.73% <0%> (-1.96%) ⬇️
packages/flutter_tools/lib/src/web/chrome.dart 7.46% <0%> (-1.5%) ⬇️
packages/flutter_tools/lib/src/macos/xcode.dart 52.72% <0%> (-0.85%) ⬇️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 500d7c5...0d78fc9. Read the comment docs.

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

LGTM stamp from a Japanese personal seal

@liamappelbe liamappelbe merged commit 0cd0c66 into flutter:master Aug 6, 2019
@liamappelbe liamappelbe deleted the hhh_fix branch August 6, 2019 15:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants