Copy gen_snapshots using python's shutil.copy, avoid links#55830
Copy gen_snapshots using python's shutil.copy, avoid links#55830aam merged 3 commits intoflutter:mainfrom
Conversation
|
May I land this? Seems good to go. |
I wanted to get another failure before landing this to confirm that this will fix it. We haven't had a failure in a few days - we landed flutter/buildroot#910 since then as well(which should not fix anything, just will provide another data point regarding whether 2nd run is successful or not). |
| "$dart_src/runtime/bin:$gen_snapshot_target_name($build_toolchain)" | ||
|
|
||
| copy(target_name) { | ||
| action(target_name) { |
There was a problem hiding this comment.
What's the rationale for only using it here and not updating the toolchain definition so that all copy targets use it?
There was a problem hiding this comment.
It was to limit impact to only gen_snapshots. Only them had an issue with Gatekeeper.
Generally speaking ln has an advantage of reduced disk space usage.
There was a problem hiding this comment.
Presumably, Gatekeeper would also impact other binaries as well. If this is the fix for gen_snapshot, then we wouldn't want to go through all this investigation again a second time for some other binary. I think it's fine to do it just for gen_snpashot for now as an experiment, but if it works, there has to be follow-up work to deploy the solution more generally.
|
Triage: The investigation of this issue might have gone in a different direction. Do we still need this PR? |
Have been waiting for a recent failure before submitting this fix. |
flutter/engine@8828844...30bd6c9 2024-10-22 [email protected] Roll Fuchsia Linux SDK from avSAUwQTf5xVGuZQU... to xPbin_33tT-_omeQT... (flutter/engine#56011) 2024-10-22 [email protected] Copy gen_snapshots using python's shutil.copy, avoid links (flutter/engine#55830) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from avSAUwQTf5xV to xPbin_33tT-_ If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
|
I ran into the following error after pulling past this commit and building today. flutter/flutter#157367 |
chinmaygarde
added
the
revert
|
A reason for requesting a revert of flutter/engine/55830 could |
auto-submit
bot
removed
the
revert
|
Reason for revert: Incremental builds are broken. flutter/flutter#157367 |
chinmaygarde
added
the
revert
auto-submit
bot
removed
the
revert
…55830)" (#56034) Reverts: #55830 Initiated by: chinmaygarde Reason for reverting: Incremental builds are broken. flutter/flutter#157367 Original PR Author: aam Reviewed By: {mraleph, cbracken} This change reverts the following previous change: Default implementation of copy does it via hardlink, which seems to be causing issues with Gatekeeper on mac. BUG=flutter/flutter#154437
…lutter#55830)" This reverts commit 78f4ffa as it addresses the issues with copying over existing link.
…ngine#55830) Default implementation of copy does it via hardlink, which seems to be causing issues with Gatekeeper on mac. BUG=flutter#154437
…lutter#55830)" (flutter/engine#56034) Reverts: flutter/engine#55830 Initiated by: chinmaygarde Reason for reverting: Incremental builds are broken. flutter#157367 Original PR Author: aam Reviewed By: {mraleph, cbracken} This change reverts the following previous change: Default implementation of copy does it via hardlink, which seems to be causing issues with Gatekeeper on mac. BUG=flutter#154437
5 participants
Default implementation of copy does it via hardlink, which seems to be causing issues with Gatekeeper on mac.
BUG=flutter/flutter#154437