Skip to content
This repository has been archived by the owner on May 22, 2023. It is now read-only.

Fix missing files, using files from git ls #6

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

ch2ch3
Copy link
Contributor

@ch2ch3 ch2ch3 commented Apr 29, 2022

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.

@pierrickouw
Copy link

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

@pmbanugo
Copy link
Owner

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?

@ch2ch3 ch2ch3 changed the base branch from main to dev May 3, 2022 16:29
@ch2ch3
Copy link
Contributor Author

ch2ch3 commented May 3, 2022

@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 😅

@pmbanugo pmbanugo self-requested a review May 3, 2022 16:44
@pmbanugo pmbanugo added the enhancement New feature or request label May 3, 2022
@jraoult
Copy link

jraoult commented May 9, 2022

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)

@pmbanugo
Copy link
Owner

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discussion enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants