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

fix(lfs): ensure that LFS objects are checked out #1392

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

Conversation

sxlijin
Copy link

@sxlijin sxlijin commented Jun 27, 2023

The current implementation of lfs: true does not actually guarantee that LFS objects are checked out:

  • it first does a git lfs fetch origin (which does not update the working copy),
  • and then relies on git checkout in combination with the smudge filter to check LFS objects out.

However, there are numerous reports of flakiness with this approach (and we've seen this ourselves in our own usage); see #270.

Adding a git lfs checkout should make this more robust.

Unfortunately I don't have a consistent repro for this issue, beyond the anecdata from our own workflow failures.

@sxlijin sxlijin requested a review from a team as a code owner June 27, 2023 23:46
@sxlijin sxlijin closed this Aug 3, 2023
@ceisen1
Copy link

ceisen1 commented Aug 4, 2023

Any particular reason why you closed this PR @sxlijin?

@sxlijin
Copy link
Author

sxlijin commented Aug 4, 2023

Hmm... I thought we had internally come to a different conclusion about this, but double checking our CI setup we didn't: we always run git lfs checkout after actions/checkout.

One of the folks on my team had the hypothesis that all the flakiness issues people have reported have to do with persistent/shared state, since we use persistent self-hosted runners.

@sxlijin sxlijin reopened this Aug 4, 2023
@ceisen1
Copy link

ceisen1 commented Aug 4, 2023

Got it. That makes sense based on your description. But I do believe your original comments are correct that only running fetch is never going to update the lfs files in the working copy.

My one suggestion for your PR would be to move the git lfs checkout call into an if block where settings.lfs is True

@Ravio1i
Copy link

Ravio1i commented Nov 21, 2023

Hey there, we do face the same issue, on windows we have to run

   - uses: actions/checkout@v3
      with:
        lfs: true

    - name: Git LFS Pull 
      run: git lfs pull

to ensure it actually gets pulled

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

Successfully merging this pull request may close these issues.

None yet

3 participants