search

SRombauts / UEGitPlugin

Unreal Engine 5 Git LFS 2 Source Control Plugin (beta)
http://srombauts.github.io/UEGitPlugin
MIT License
806 stars 165 forks source link

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

Closed sinbad closed 4 years ago

sinbad commented 4 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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)