Some design notes I wrote a while back when thinking about locking

[sinbad]

I've given locking some thought before so I've pulled out my notes about how I think it could work for a git-lfs proposal.

[technoweenie]

Hey, thanks for getting this down somewhere. I just haven't had a good chance to review it yet. I'll get to it this week.

[technoweenie]

Man, time flies!

This brings up some great ideas. I think the trick here is minimizing the impact to the normal workflow. A user that isn't aware of git lfs lock should be notified as early as possible if they're modifying a locked file. A locking user shouldn't worry about releasing the lock on a git push.

Identify unmergeable files as subset of lfs files

How could we do this? I'm not sure we have access to the gitattributes k/y properties:

# ideal, but maybe impossible
*.gif filter=lfs diff=lfs merge=lfs -crlf -lockable

# hacky workaround
*.gif filter=lockablelfs diff=lfs merge=lfs -crlf

# git config
$ git config -l | grep filter\.
filter.lfs.clean=git-lfs clean %f
filter.lfs.smudge=git-lfs smudge --skip %f
filter.lfs.required=true
filter.lockablelfs.clean=git-lfs clean --lockable %f
filter.lockablelfs.smudge=git-lfs smudge --lockable --skip %f
filter.lockablelfs.required=true

Notify original lock holder HOW?

Not sure Git LFS has any control. It'd be up to the git server to do something (send out email, update pull request, whatever).

Reject a push containing a binary file currently locked by someone else

I can't remember if the local pre-push hook can tell if it's a force push. If so, we can rely on the LFS batch API rejecting updates to locked files, instead of the git pre-receive hook.

Other thoughts:

1. Does the git lfs client keep track of what files are currently locked? This way the client can warn you earlier before you attempt to push.

2. The batch API requests should start including the filename(s) associated with the object being fetched or pushed. This way the API can respond accordingly to locked files.

3. I wonder if the commands should be more like git remote?

   • git lfs lock <file>
   • git lfs lock release <file>
   • git lfs lock break <file>

   It's a lot to type. Though ideally, users will only type the first one except in the rare case where a lock has to be broken.

4. I'm not a fan of encouraging people to use git push -f more often. I'd rather require explicit git lfs lock commands from the user. Maybe even with some kind of message that can be recorded and passed on to notification emails or pull request web pages.

[sinbad]

Identify unmergeable files as subset of lfs files

How could we do this? I'm not sure we have access to the gitattributes k/y properties:

Yeah, I was a bit woolly about this, I'd seen custom gitattributes before but actually I think they're just macros expanding into the standard set, so not much use. In the absence of this I think the main options are to either use a different filter name as you suggested in your 'hacky' option - I'm not sure it's actually that hacky, it's quite a succinct way of doing it - or using another file to supplement this, say .gitlfsattributes perhaps. The advantage of that is that it could be used for other things later if we wanted to have per-filepath configuration options, but at the expense of having to parse another file & do the same path matching.

Notify original lock holder HOW?

Not sure Git LFS has any control. It'd be up to the git server to do something (send out email, update pull request, whatever).

Yeah this was more a flag for me to indicate it needed more thought, I suspect that in practice this will just be for server implementations to decide. The only tricky bit could be if you wanted to match up server account names with email addresses in commits & whether there are edge cases there, but if the lock & free are always done over the API using the server's account system it might not matter.

I can't remember if the local pre-push hook can tell if it's a force push. If so, we can rely on the LFS batch API rejecting updates to locked files, instead of the git pre-receive hook.

Sadly it doesn't, I've seen been hacks using the process ID to figure that out but they're not portable: https://gist.github.com/pixelhandler/5718585. Maybe there's a way to do that portably in Go but otherwise it might have to wait until the post-receive hook on the server. Or, we can locally detect in pre-push and require that rather than --force they use one of the explicit lock breaking functions instead. That actually might be better since it would ensure it goes through whatever notification channels the server API uses, a push --force model would probably skip that.

Does the git lfs client keep track of what files are currently locked?

Yeah I think so.

The batch API requests should start including the filename(s) associated with the object being fetched or pushed. This way the API can respond accordingly to locked files.

Technically that might not be needed if we make the client reject pushing of files which were not locked as above, but since there may be edge cases like transitioning from something being defined as non-lockable to lockable (so according to the server it's lockable but the client doesn't know yet) it's probably a good idea to have it.

I wonder if the commands should be more like git remote?

I don't feel super-strongly either way, I thought maybe just having 2 subcommands with options might be most efficient but grouping everything under 'lock' is quite nice too.

I'm not a fan of encouraging people to use git push -f more often. I'd rather require explicit git lfs lock commands from the user

I agree. Only just got to this and realise I suggested the same above :)

On reflection, automatically unlocking (& making read-only) after push might be difficult, since there is no post-push hook. Need to consider the impact of either unlocking after the pre-push hook, or whether it needs wrapper command to put it genuinely after the commits are pushed :/

[technoweenie]

Merging this now so @ttaylorr can start iterating on it. No guarantees on when it'll ship or how exactly it'll work, yet.

[mrbobbybobberson]

Hi y'all, I really like your locking proposal. It seems like a very well-thought-out way to get all the benefits of git for a majority of a team's files, while letting those hard-to-merge files gracefully fall back to a more centralized source control model. I'm getting eager to try it out on our team. How close to being ready for early-adopter testing is it? Is there any way I can help, even if it's just as a sounding board for design ideas?

Thanks,
-Robert

[ttaylorr]

Hi @mrbobbybobberson, glad to hear that you're interested 🤘. There are a few ways that you can get involved early:

• Check out, and play around with @sinbad's latest locking PR in WIP (!) Locking read-only behaviour #1590
• Use the existing locking-based commands by throwing GITLFSLOCKSENABLED=1 in your environment before using Git LFS. There is no known server-side implementation of the locking API other than the one in test/cmd (used during our integration tests), but you can, of course, mess around with that too.

Let me know if you have any other questions!

[sinbad]

Yeah there isn't a full implementation of my original design yet but I'm making progress, my PR above is a scrappy version mostly to test out the practicality of setting working copy files read-only. After that I intend to get on to the team workflow aspects, catching potential non-linear dev and resolving it. It's a gradual process but we'll get there :)

[mrbobbybobberson]

Sounds great. I'll play with PR #1590 to get a sense of where things are going. Thanks!

[vtbassmatt]

@sinbad how close do you think this is to getting into a Git-LFS release? I'm on the Git server team at Microsoft, and we'll want to bring this to our LFS implementation. Trying to figure out when we should schedule that work. Thanks!

[sinbad]

@vtbassmatt it's a little ways off yet; I've done a first pass of the "read-only when unlocked" work which generated some good feedback, I'm in the process of rewriting it based on that feedback. After that it needs the logic to handle the other edge cases like requesting locks with local state being out of sequence as discussed in the proposal. It's taking a while because I have less time for LFS work these days, but I'm making steady progress. We're pencilling it in for 2.0+

[vtbassmatt]

👍 Thank you!

[mrbobbybobberson]

Hey @ttaylorr, @sinbad, and @vtbassmatt, I admittedly didn't get the cycles to try out #1590 this past October, but I'm finally getting some more time to try moving our team to git :)! I'm trying out git lfs 2.1.1 client with our server and have some questions. First one:

In the proposal, it discussed the multi-branch model, the need to cherry-pick (or of course merge) in the latest version of a lockable file in order to take a lock on it, the possibility of parallel locks (which would be great when multiple long-lived branches exists for servicing releases and what not).

Are all those considerations still in the game plan? It doesn't seem like they're currently in play... am I doing something wrong in my testing, or is that expected?

To try to exercise that functionality, I tried locking a file, making a few edits, pushing them, and unlocking it. Then, I created a branch from a commit before I made those changes to the lockable file and tried to lock the file again, and it allowed me to take the lock with no warnings or alternate flow. Is that something either the client or server should have caught and blocked me from unintentionally doing?

Thanks!

[ttaylorr]

Hi @mrbobbybobberson, thanks for bringing this discussion back 👍 .

Are all those considerations still in the game plan? It doesn't seem like they're currently in play... am I doing something wrong in my testing, or is that expected?

We implemented repo-wide locking based on a filepath. In other words, if you lock a/b.txt on any branch, files at that path are locked to that revision across all branches.

We're starting to kick around the idea of blob OID-level locking, the semantics of which would be "if I lock a file at SHA1:X, all other branches that have the same pathspec at SHA1:X are also locked." This would allow for long running branches to have locks at the same paths with different versions of a file.

To try to exercise that functionality, I tried locking a file, making a few edits, pushing them, and unlocking it. Then, I created a branch from a commit before I made those changes to the lockable file and tried to lock the file again, and it allowed me to take the lock with no warnings or alternate flow. Is that something either the client or server should have caught and blocked me from unintentionally doing?

It sounds like since you released the lock, the server allowed you to acquire the lock again without issue. Not sure I understand what the error is/should have been in that case. If you're still running into this, please open a new issue.

[mrbobbybobberson]

Thanks @ttaylorr, and sorry for taking so long to get back. I've been talking a lot with my team and fleshing out what source control behavior would work best for us. I wrote up a handful of scenarios in a text document, but I decided not to share it, because the text got too complex to follow. When I find some more time, I'll make it into a powerpoint so it can be clicked through (sort of like an animation) to explain what would work for us.

As for the blob OID-level locking, that might cover some of our scenarios, but I think it misses the issue I didn't explain very well in my second question. I'll open up a new issue and try explaining it better there. Thanks!

[mrbobbybobberson]

Actually, in #1844, @sinbad stated it really well in comment #1844 (comment) . I want some way to alert the user that they don't have the latest changes. Once I get my animated scenarios powerpoint, I'll share it in #1844.