Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ This is the log of notable changes to EAS CLI and related packages.

### 🐛 Bug fixes

- Strip permissions and other metadata from project archive when packing sources on Windows. ([#3234](https://github.com/expo/eas-cli/pull/3234) by [@sjchmiela](https://github.com/sjchmiela))

### 🧹 Chores

- Document the default values for `track` and `releaseStatus`. ([#3229](https://github.com/expo/eas-cli/pull/3229) by [@kadikraman](https://github.com/kadikraman))
Expand Down
14 changes: 11 additions & 3 deletions packages/eas-cli/src/build/utils/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,17 @@ export async function makeProjectTarballAsync(vcsClient: Client): Promise<LocalF

try {
await vcsClient.makeShallowCopyAsync(shallowClonePath);
await tar.create({ cwd: shallowClonePath, file: tarPath, prefix: 'project', gzip: true }, [
'.',
]);
await tar.create(
{
cwd: shallowClonePath,
file: tarPath,
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

},
['.']
);
} catch (err) {
clearTimeout(timer);
if (spinner.isSpinning) {
Expand Down