-
Notifications
You must be signed in to change notification settings - Fork 93
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
[eas-cli] Skip makeShallowCopyAsync
if requireCommit
#2885
Conversation
Size Change: +1.25 kB (0%) Total Size: 53.4 MB
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2885 +/- ##
==========================================
+ Coverage 52.41% 52.62% +0.21%
==========================================
Files 588 588
Lines 23169 23170 +1
Branches 4851 4852 +1
==========================================
+ Hits 12142 12190 +48
+ Misses 10053 10014 -39
+ Partials 974 966 -8 ☔ View full report in Codecov by Sentry. |
Subscribed to pull request
Generated by CodeMention |
// | ||
// We only do this if `requireCommit` is false because `requireCommit: true` | ||
// setups expect no changes in files (e.g. locked files should remain locked). | ||
await makeShallowCopyAsync(rootPath, destinationPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if I understand. Why should we not do git clone
when requireCommit: true
is present?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It comes down to secrets and locked / unlocked repository (if using git-crypt
).
Looks like requireCommit
flag wasn't being used just as a "it's a good practice to always commit your changes", but more like "make tarball look exactly like a cloned repository at this commit would". In the former understanding it's not a big deal if we copy unlocked files to the tarball because the user did make a commit. In the latter understanding requireCommit
is more like makeTarballResembleClone
or something.
See discussion in here and how it broke Brent's setup with encrypted files (he was using requireCommit: true
and was expecting encrypted files to be locked and worktree clean).
✅ Thank you for adding the changelog entry! |
Why
requireCommit: true
setups expect worktree to be clean (e.g. encrypted files to be locked).How
Made it so if
requireCommit
is true we don't copy files over.Test Plan
I have enabled
git-crypt
in my test repository, added a secret file, raneasd build:inspect -p android -s archive -o ~/test-production --force
. The clone did contain secret file in plaintext.Then I enabled
requireCommit: true
and repeated the command. The clone did contain the file encrypted.