heroku-review-app-actions icon indicating copy to clipboard operation
heroku-review-app-actions copied to clipboard

Fix missing files, using files from git ls

Open ch2ch3 opened this issue 2 years ago โ€ข 5 comments

This PR was inspired by #5 (thank you @pierrickouw for helping me realise why my build was failing!), but the suggested fix in #5 gives me this error message:

tar: .: file changed as we read it

Looking at the logs I can see that tar was trying to include the .git directory, and other unnecessary files.

Using the list of files from git instead seems like it would correspond better to what the GitHub integration would normally be sending to Herokuโ€”this filters out anything that would be git ignored.

ch2ch3 avatar Apr 29 '22 13:04 ch2ch3

That's way better, closing my PR! Thank you!

pierrickouw avatar Apr 29 '22 14:04 pierrickouw

Thanks for sending PR ๐Ÿ‘๐Ÿฝ ๐Ÿ‘๐Ÿฝ @ch2ch3 and @pierrickouw

I still have hesitation using git ls-files as it is at the moment. This will include files in .github, include .gitignore, etc. ATM I think it would be better to have an input parameter (probably using glob) for a list of files/directories to include or exclude.

Then it could be implemented somewhat like tar -czvf --exclude='./folder' --exclude='./folder2' --exclude='file.ext' source.tar.gz $(git ls-files).

What do you think?

pmbanugo avatar Apr 29 '22 14:04 pmbanugo

@pmbanugo I hear your concern!

Personally, I'm not too worried about those files as they are usually small and thus negligible to include in the compressed source code. On the flip side, there could be a risk of someone not realising that they have to "reimplement" git ignore, and accidentally sending secrets or large binaries to Heroku which aren't needed. Maybe git ls-files could be the default, but with the option to override for those who want more control over what to include/exclude? I will leave it to you to make the call ๐Ÿ‘

P.S. I updated the base branch as I belatedly noticed the note in your README about sending contributions to the dev branch ๐Ÿ˜…

ch2ch3 avatar May 03 '22 16:05 ch2ch3

Just wanted to mention that I fix the same issue in a fork using git archive -o source.tar.gz HEAD (cf. https://www.atlassian.com/git/tutorials/export-git-archive)

jraoult avatar May 09 '22 18:05 jraoult

Thanks for all the contributions. I decided to use the option to pass in a glob pattern as input for now. I'm leaving this PR open while I observe how it gets used and reconsider this proposal again in the future.

pmbanugo avatar May 10 '22 09:05 pmbanugo