Skip to content

Conversation

@sjchmiela
Copy link
Contributor

@sjchmiela sjchmiela commented Oct 21, 2025

Why

Claude says this is the fix for https://exponent-internal.slack.com/archives/C06EFBQK3B7/p1760996501806919. It makes sense to me.

How

Applied the change.

Test Plan

Try it on GHA's Windows. See https://github.com/expo/eas-cli/actions/runs/18678885693.

@github-actions
Copy link

github-actions bot commented Oct 21, 2025

Size Change: -252 B (0%)

Total Size: 54.6 MB

Filename Size Change
./packages/eas-cli/dist/eas-linux-x64.tar.gz 54.6 MB -252 B (0%)

compressed-size-action

@codecov
Copy link

codecov bot commented Oct 21, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.89%. Comparing base (034e0a1) to head (60a9cef).

Files with missing lines Patch % Lines
packages/eas-cli/src/build/utils/repository.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3234      +/-   ##
==========================================
- Coverage   51.89%   51.89%   -0.00%     
==========================================
  Files         626      626              
  Lines       25703    25704       +1     
  Branches     5398     5399       +1     
==========================================
  Hits        13337    13337              
+ Misses      12327    11258    -1069     
- Partials       39     1109    +1070     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sjchmiela sjchmiela force-pushed the stanley/do-portable-tar-on-windows branch from e8e77ca to 595834d Compare October 21, 2025 09:02
This reverts commit c041e91.
This reverts commit 595834d.
@sjchmiela sjchmiela marked this pull request as ready for review October 21, 2025 09:30
@github-actions
Copy link

Subscribed to pull request

File Patterns Mentions
**/* @douglowder

Generated by CodeMention

@github-actions
Copy link

✅ Thank you for adding the changelog entry!

prefix: 'project',
gzip: true,
// Use portable mode on Windows to ensure proper permissions when extracting on Unix/Linux
...(process.platform === 'win32' && { portable: true }),
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we'd need to apply this only to windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct me if I'm wrong — if we apply this to Linux too, it may so happen that:

  • right now, a user has a ./bin/postcheckout.sh file marked as executable
  • they have an EAS Build lifecycle hook that does ./bin/postcheckout.sh
  • they've been happily eas build-ing for years
  • we make archive "portable" not only on Windows (which is the broken scenario this is fixing), but also on Linux
  • ./bin/postcheckout.sh is no longer marked as executable in the archive
  • EAS Build lifecycle hook breaks because ./bin/postcheckout.sh is no longer excutable.

I don't want to break this flow — I consider Windows to be the odd environment here we need to put a workaround for. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

portable doesn't discard the mode byte, but instead masks it, so executable bits and such shouldn't be lost. The typical mask is 0o7777.

I wouldn't of course trust node-tar here, but this is easily verified and we should verify it. Their own docs say:

Additionally, mode is set to a "reasonable default" for most unix systems, based on a umask value of 0o22.

But they don't clarify what that reasonable default is. That said, verifying this and keeping it consistent between both platforms makes more sense to me.

Coming at it from the other angle: if we have a +x file in a git repo, we shouldn't see any different behaviour no matter if we're tar-ing on Windows or on macOS/Linux. If this was an issue, this wouldn't be a fix, but just a change that affirms the bug. However, conceptually speaking, portable should do the right thing here, so assuming tar is implemented correctly, it should keep umode around and only mask it to a valid value.

Also worth noting, I'm pretty sure we usually use the tar binary on macOS/Linux. If we are, this change wouldn't affect Windows either, which would make this check also redundant, and again, make it explicitly inconsistent

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants