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

Add git-lfs to setup process #758

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add git-lfs to setup process #758

wants to merge 3 commits into from

Conversation

AramZS
Copy link

@AramZS AramZS commented Jun 1, 2022

This PR will add the installation of git-lfs to the container and initiate it during the initial setup of git during the RUN phase, allowing the container to handle the use of large files pushed to the Github Pages site and branch.

This addresses #748 by applying the right machine-level systems but does not completely solve the problem, as the user will still need to find a way to pass their .gitattributes file to the base of their new site the same way they might need to with a CNAME file.

@AramZS AramZS requested a review from peaceiris as a code owner June 1, 2022 22:42
@peaceiris peaceiris added the docker Pull requests that update Docker code label Jun 1, 2022
@peaceiris
Copy link
Owner

This action is a TypeScript action.

@AramZS
Copy link
Author

AramZS commented Jun 3, 2022

This action is a TypeScript action.

What do you mean @peaceiris? LFS is configured at the Git level and Typescript shouldn't be required. Is there a TypeScript definition file that I need to add information to somewhere?

@peaceiris
Copy link
Owner

@AramZS
Copy link
Author

AramZS commented Jun 6, 2022

@peaceiris Oh, you mean because this action is done in TypeScript and the docs recommend:

To ensure your JavaScript actions are compatible with all GitHub-hosted runners (Ubuntu, Windows, and macOS), the packaged JavaScript code you write should be pure JavaScript and not rely on other binaries. JavaScript actions run directly on the runner and use binaries that already exist in the virtual environment.

?

Git LFS is GitHub-run project and is broadly supported. See: https://docs.github.com/en/repositories/working-with-files/managing-large-files/installing-git-large-file-storage

I don't think the packaged code would have any problems and is intended as an integration with git at the CLI/Command Line level. Adding it should introduce no compatibility issues with any package running system and it would not interfere (or even interface) with any of the TypeScript code. The action docs note:

Can include access to a clone of your repository, enabling deployment and publishing tools, code formatters, and command line tools to access your code.

And this would, I think, fall under that category, since many people use Git LFS to handle large files on GitHub and would like to use it to potentially use those files in GitHub Pages using this action. It's just one more tool to support how files are transmitted with git for projects that use GitHub.

EDIT: And to be clear, this change would require no additional TypeScript. As long as .gitattributes is passed through, it would just work with the existing git process and interactions with GitHub.

@peaceiris
Copy link
Owner

This action is a TypeScript action, not a Docker action.

runs:
using: 'node12'
main: 'lib/index.js'

The Dockerfile is used for the LOCAL testing.

This change sets up Git LFS using the same approach as @actions/checkout
and will hopefully allow the use of Git LFS to match the potential use
in the checkout action and allow users to push up published GitHub
pages using `git lfs` to support files larger than 100MB.
@peaceiris peaceiris added documentation and removed docker Pull requests that update Docker code labels Jun 6, 2022
@AramZS
Copy link
Author

AramZS commented Jun 6, 2022

@peaceiris Ah my apologies, I was unclear on that. I haven't done much work on Actions and so am somewhat unfamiliar with the whole process. Forgive my misunderstanding here. Thank you for clarifying.

I've pushed up additional changes and accompanying documentation, along with a flag that a user can use to turn it on instead of turning it on by default. I've mirrored how it is set up in @actions/checkout with the idea that the developer would be using that flag and this one.

I am not entirely sure if I've set it up correctly, everything looks like it should work in the same way as with the checkout action, I think.

But I'm not sure how to test it locally. I'd be glad to do that work, but I'm unsure if your setup is automated or something manual for that? I think I might need advice on how to best set up such a test in the context of your project.

@peaceiris
Copy link
Owner

Thanks!

As I mentioned in this comment, GitHub Pages does not support objects under Git LFS. So there is no way to upload files under LFS. Therefore, the lfs input is not needed for this action.

Adding the section about downloading Git LFS objects on GitHub Actions runners can be useful for users who want to commit and push the large size of files less than 100MB as Git normal objects. So I will merge this pull-request as adding the section including a recipe of the lfs input of actions/checkout.

@AramZS
Copy link
Author

AramZS commented Jun 9, 2022

@peaceiris I agree that the large objects won't be available in the pages site, but if we checkout with LFS, we should be able to check in to the branch with those files. They won't be published to the Github Pages site, but I know the files can still exist in branch used for GitHub Pages and without the addition of LFS at the time of commit in this action what will happen is that LFS will be used for checkout and when this action attempts to make a git commit for publication it will crash. So I wouldn't recommend adding the LFS documentation without support for the page-branch commit in this action, otherwise users will experiance breakage.

@AramZS
Copy link
Author

AramZS commented Aug 11, 2022

@peaceiris I wanted to check in on this. I think that the configuration here should work and resolve the issue, allowing git-lfs files in the built branch to be committed without crashing the process?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants