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

Implement unlock-on-push, if changes had been committed without pushing before #137

Merged
merged 1 commit into from
May 22, 2020

Conversation

sinbad
Copy link
Contributor

@sinbad sinbad commented May 21, 2020

When using LFS locking, in the case where you have a local commit with locked LFS data in it but haven't pushed, using the UE4 plugin "Push" command will leave this file locked, contrary to what would happen if you use Submit (and were able to push). This is confusing since Submit is notionally Commit+Push, but if the 2 are done separately the result is not the same.

Scenarios when this could have happened:

This PR resolves this inconsistency. When LFS locking is enabled and you push changes, it checks which LFS files would be pushed and tries to unlock them afterwards, just like Submit would do.

If the files have more local modifications, the unlock will fail (git lfs will reject it). We silently allow this & continue because it was probably intentional not to unlock in this case.

@Memnarch
Copy link

Isn't this a potential problem?
I usually unlock manually and commit/push externally. But if done through the unreal plugin, this would auto unlock it everytime?
What happens if i am working in a branch on something and need to keep the lock? This merge sounds like it would unlock my file each time I push to my feature-branch and allow people on the main-development branch to make changes.

I know this resolves an inconsistency but my question here is, if autounlocking in all cases is the desired behaviour?

@sinbad
Copy link
Contributor Author

sinbad commented Jan 4, 2022

I guess an option not to unlock on push could be added, if it was an issue in practice. It would have to also alter the default behaviour on Submit (Commit+Push) to not unlock as well though, to retain consistency, so I think that in isolation this PR's behaviour is correct.

@Memnarch
Copy link

Memnarch commented Jan 7, 2022

I guess an option not to unlock on push could be added, if it was an issue in practice. It would have to also alter the default behaviour on Submit (Commit+Push) to not unlock as well though, to retain consistency, so I think that in isolation this PR's behaviour is correct.

Yes, this PRs behavior is correct. My comment was really just about the behaviour in general (and to confirm if I got the problematic situation right or if I missunderstood the, now consistent, behavior)

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.

3 participants