Conversation
|
@bartekpacia Can you please take a look at this? |
|
Hi @kuhnroyal, sure! Thanks for the contribution:) Overall this looks solid, thanks for simplifying too.
I think it's fine if the existing caches will stop working and are evicted. Let's not do 2 more things:
|
kuhnroyal
force-pushed
the
feature/iunzip
branch
from
98695d0 to
49ade1c
Compare
|
@bartekpacia Rebased |
There was a problem hiding this comment.
Pull request overview
Refactors Flutter SDK extraction to keep the top-level flutter/ directory from the archive, avoiding a temp directory + move and aligning extraction behavior across .zip and tar archives (addresses cache failures on different drives due to symlink moves; see #378).
Changes:
- Extract
.zipandtararchives directly into the cache directory without stripping components. - Update installation paths to treat
$CACHE_PATH/flutteras the Flutter SDK root (FLUTTER_ROOT and PATH updates). - Adjust
main/mastergit clone destination to$CACHE_PATH/flutter.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
setup.sh
Outdated
| if [ "$VERSION" != "any" ]; then | ||
| git config --global --add safe.directory "$CACHE_PATH" | ||
| git config --global --add safe.directory "$CACHE_PATH/flutter" | ||
| (cd "$CACHE_PATH" && git checkout "$VERSION") |
There was a problem hiding this comment.
@kuhnroyal this seems reasonable, but I'm not sure. wdyt?
There was a problem hiding this comment.
Do we have a CI job that checks out a specific master version?
There was a problem hiding this comment.
@bartekpacia Yea looks like copilot is right, this run confirms it: https://github.com/subosito/flutter-action/actions/runs/23251398469/job/67594072864?pr=379
There was a problem hiding this comment.
I applied the change and added matrix jobs for specific versions and any (default).
Not sure if the additional jobs are overkill :)
There was a problem hiding this comment.
I don't think we do; I have just added it in #391
There was a problem hiding this comment.
please rebase and I'll run CI again
kuhnroyal
force-pushed
the
feature/iunzip
branch
from
cc59357 to
5ce0cd8
Compare
Refactor unzip command to directly extract to target directory. By keeping the /flutter directory from inside the archive, we avoid having to use a temp directory and move afterwards.
kuhnroyal
force-pushed
the
feature/iunzip
branch
from
088e484 to
7ebfad0
Compare
Refactor unzip/tar commands to directly extract to target directory without stripping components.
By keeping the /flutter directory from inside the archive, we avoid having to use a temp directory and move afterwards.
Fixes #378
This works fine but not exactly sure how it fares with existing caches.
The change might warrant a v3 release.