Skip to content
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

Github Action to prevent large files #1478

Merged
merged 3 commits into from
Mar 2, 2023
Merged

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Mar 2, 2023

image

Part of #1403

Checklist

@Wumpf Wumpf force-pushed the andreas/prevent-large-files branch 4 times, most recently from 4975833 to e2bad45 Compare March 2, 2023 15:01
@Wumpf Wumpf added the 🧑‍💻 dev experience developer experience (excluding CI) label Mar 2, 2023
@Wumpf Wumpf force-pushed the andreas/prevent-large-files branch from 9ff008d to e2bad45 Compare March 2, 2023 15:04
@Wumpf Wumpf force-pushed the andreas/prevent-large-files branch from e2bad45 to 76b3676 Compare March 2, 2023 15:05
@Wumpf Wumpf marked this pull request as ready for review March 2, 2023 15:05
@Wumpf
Copy link
Member Author

Wumpf commented Mar 2, 2023

does not check for accidental LFS files, just rooting for other stuff in CI breaking then ;)

result=0
while read -d '' -r file; do
if [[ -f "$file" ]]; then
actualsize=$(wc -c <"$file")
Copy link
Member

Choose a reason for hiding this comment

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

Not that it really matters for this use-case, but stat should be a lot faster since it uses the filesystem rather than wc which I think is actually consuming the file stream characters from the input pipe.

Suggested change
actualsize=$(wc -c <"$file")
actualsize=$(stat -c%s "$file")

Copy link
Member Author

Choose a reason for hiding this comment

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

I know nothing about these. Will do :)

Copy link
Member Author

Choose a reason for hiding this comment

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

stat doesn't have that size filtering option on my mac it seems. I'll leave it be

Copy link
Member

Choose a reason for hiding this comment

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

yet another reason to do this in Python

Copy link
Member

@jleibs jleibs left a comment

Choose a reason for hiding this comment

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

Any particular reason for doing this as bash instead of python?

@Wumpf
Copy link
Member Author

Wumpf commented Mar 2, 2023

Any particular reason for doing this as bash instead of python?

Should have done python given that I spent soo much time figuring it out. Story is that I first had it in its own job and didn't want to install python. Then moved it into the util job which already has it but didn't convert it

@Wumpf Wumpf merged commit a0fb0ca into main Mar 2, 2023
@Wumpf Wumpf deleted the andreas/prevent-large-files branch March 2, 2023 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧑‍💻 dev experience developer experience (excluding CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants