Re: [RFC/PATCH] Add multiple workdir support to branch/checkout

Jay Soffian [off-list ref] writes:

Git has survived w/o needing to lock branches till now.

Careful. Git has survived without your patch series till now, as people
learned to be careful when they use separate workdirs and avoid certain
things, to the point that they are not necessarily aware that they are
avoiding them (one good practice is to keep the HEADs of non-primary
workdirs detached).

Does that mean what your patch aims to do is unnecessary? I think not.

What are these
use cases we cannot already think of today?

What is important is that we should have learned by now that the "gotchas"
live where we do not immediately see. "Can you tell me what you are missing?"
is a senseless thing to ask.

quoted

I think "switch_branches()" that updates HEAD to point at a local branch
is one good place to lock the branch, but I do not know if it is a good
idea to hook the check into the codepaths for deletion of the branch using
"branch -[dD]" and check-out of the branch using "checkout $branch". I
wonder if it makes sense to add the "checking" hook into much lower level
in the callchain, perhaps delete_ref(), rename_ref() and update_ref() to
catch attempts to update "your" current branch by other people.

I don't think so. There are lots of ways to shoot yourself in the foot
at the plumbing level. Besides, this is not about all refs, just local
branches.

Aside, there's nothing wrong with renaming a checked out branch.

There are pros and cons between hooking at lower level vs higher
level. The advantage of hooking at higher level is you do not risk
breaking low-level operations, but that directly results in allowing the
same low level operations that are unaware of the new requirement higher
level added break it. It also allows other high level operations you
forgot to teach the new requirement break it.

For example, you checkout branch frotz in a workdir, and then in the
primary repository that has nitfol branch checked out, you rename the
frotz branch to xyzzy. The HEAD of workdir still says refs/heads/frotz
that no longer exist. Of course you can break the same way by doing a
"update-ref -d refs/heads/frotz" from the primary repository.

Because you forgot that the high level operation "branch renaming" needs
to be aware of that "this branch is checked out elsewhere" information,
you allowed it to break the workdir. If you hooked into lower level
machinery that is shared, you wouldn't have caused this breakage.
Similarly, if delete_ref() were taught about the new requirement, you
would have covered both "branch -d" and "update-ref -d".

I do not necessarily think that it is a good approach to forbid the same
branch to be checked out in two different places, by the way. One reason
people would want to keep multiple workdirs is so that while they are
still working on a branch and are not yet at a good "stop point" to even
make a temporary commit to get interrupted, they find it sometimes
necessary to be able to build the tip of that same branch and even make a
small in-working-tree fixes (which later will be carried back to the
primary branch). The problem arises only when one of the repositories try
to update or delete the branch while it is checked out in another working
tree.

Can this series be extended/reworked so that:

 - Each branch has multi-value configuration record to note the workdirs
   that it is checked out;

 - Error out (or warn if forced) upon any attempt to update the tip of a
   branch that is checked out in more than one place; and

 - Similarly for renaming or deleting a branch that is checked out in more
   than one place.
   

On Wed, Oct 5, 2011 at 2:19 PM, Junio C Hamano [off-list ref] wrote:

Careful. Git has survived without your patch series till now, as people
learned to be careful when they use separate workdirs and avoid certain
things, to the point that they are not necessarily aware that they are
avoiding them (one good practice is to keep the HEADs of non-primary
workdirs detached).

I think it's more likely the case that most people have avoided
new-workdir entirely.

Also, while I might recommend new-workdir to my coworkers with the
advice "don't checkout the same branch in multiple workdirs", never in
a million years would I say "use new-workdir, but make sure to only
use a detached HEAD in the workdirs." The latter would make their
actual HEADs explode. :-)

For example, you checkout branch frotz in a workdir, and then in the
primary repository that has nitfol branch checked out, you rename the
frotz branch to xyzzy. The HEAD of workdir still says refs/heads/frotz
that no longer exist. Of course you can break the same way by doing a
"update-ref -d refs/heads/frotz" from the primary repository.

Because you forgot that the high level operation "branch renaming" needs
to be aware of that "this branch is checked out elsewhere" information,
you allowed it to break the workdir. If you hooked into lower level
machinery that is shared, you wouldn't have caused this breakage.
Similarly, if delete_ref() were taught about the new requirement, you
would have covered both "branch -d" and "update-ref -d".

I did not forget, I just hadn't gotten there yet while this was still
an RFC/PATCH.

Another issue to resolve is what happens when the workdir or repo are
moved in the filesystem. And making prune aware of HEAD reflogs in the
alternate workdirs.

I do not necessarily think that it is a good approach to forbid the same
branch to be checked out in two different places, by the way. One reason
people would want to keep multiple workdirs is so that while they are
still working on a branch and are not yet at a good "stop point" to even
make a temporary commit to get interrupted, they find it sometimes
necessary to be able to build the tip of that same branch and even make a
small in-working-tree fixes (which later will be carried back to the
primary branch). The problem arises only when one of the repositories try
to update or delete the branch while it is checked out in another working
tree.

That is not at all my experience of how workdirs are used.

Can this series be extended/reworked so that:

 - Each branch has multi-value configuration record to note the workdirs
  that it is checked out;

This is a good idea in any case for when "checkout --force" is used
(see below), so that we can find all the workdirs for other operations
that may need to.

 - Error out (or warn if forced) upon any attempt to update the tip of a
  branch that is checked out in more than one place; and

I think that's a worse user experience. "Sorry, can't commit your
changes because you've checked out this branch elsewhere." Now the
user's choices are:

1. commit --force (and thus confusing the other workdirs)
2. checkout -b new_branch && commit

Both of which I think are worse than preventing the checkout in the first place.

 - Similarly for renaming or deleting a branch that is checked out in more
  than one place.

Yep.

j.

Aside, previous discussion -
http://thread.gmane.org/gmane.comp.version-control.git/150559

Sadly, it seems to have petered out, it looks to me like a case of
perfect being the enemy of the good. I'm really just trying to make it
good enough that we can move new-workdir out of contrib. It's a
valuable tool, we just need to remove some of its sharper edges.

j.

On Wed, 05 Oct 2011 15:11:30 +0000, Jay Soffian wrote:
...

quoted

 - Error out (or warn if forced) upon any attempt to update the tip of a
  branch that is checked out in more than one place; and

I think that's a worse user experience. "Sorry, can't commit your
changes because you've checked out this branch elsewhere."

This is actually pretty much the same as "you can't push into the
currently checked-out branch".

I do come from CVS where multiple checkouts of the same branch are obviously
common, but the semantics are different. git would need to allow to be in
a detached state but still have a notion of a 'current' branch to mimic that;
this tentative 'current' branch being what we need to merge or rebase onto later.
Just thinking.

It may actually be logical to put the other workdirs into detached state when
the branch they are on is committed into; however, this is seriously confusing.

Now the
user's choices are:

1. commit --force (and thus confusing the other workdirs)
2. checkout -b new_branch && commit

Both of which I think are worse than preventing the checkout in the first place.

Hmm. You mean forcing the user to make a new branch *earlier* than at
commit time is better?

Andreas

On Wed, Oct 5, 2011 at 4:00 PM, Andreas Krey [off-list ref] wrote:

Hmm. You mean forcing the user to make a new branch *earlier* than at
commit time is better?

In my mind, we're trying to make new-workdir usable for non-advanced
users. I think it's conceptually simplest to allow a branch to be
checked out only once.

FWIW, I use a modified copy of new-workdir w/this usage:

  git new-workdir <repo> <workdir> <ref> [<start>]

Which allows me to create a new branch and workdir checked out to the
new branch in one shot. It refuses to create the <workdir> if <ref>
resolves to a checked-out branch. (If I want to start detached I can
do so with <ref>^0, but I rarely if ever do that.)

j.

Jay Soffian wrote:

In my mind, we're trying to make new-workdir usable for non-advanced
users.

I'd be happy already with making it comfortable for the advanced
users. :)

I think your patch goes in a right direction (using the shared
.git/config file as a way to negotiate ownership of branches).
Junio’s comments about it seeming sensible to

 - make this apply to other operations that clobber a branch
 - make the “[branch "master"] checkedout” configuration multi-valued
   if there is to be support for "git checkout -f" overriding this at
   all

ring true to me.  Making the value of this variable the path to the
.git dir or worktree (rather than an opaque string) seems like a very
good thing: it means that a future git could check if the directory
still exists and break the lock if someone has used “rm -fr”.

As for moving “git new-workdir” out of contrib, I believe another
prerequisite is sharing the HEAD reflog.

Just my two cents,
Jonathan

On Wed, Oct 5, 2011 at 5:30 PM, Jonathan Nieder [off-list ref] wrote:

As for moving “git new-workdir” out of contrib, I believe another
prerequisite is sharing the HEAD reflog.

I don't understand this. Is it about not gc'ing commits that other
workdirs are detached on, or something more?

I like that each of my workdirs have their own HEAD reflog.

j.

Jay Soffian wrote:

I don't understand this. Is it about not gc'ing commits that other
workdirs are detached on, or something more?

I like that each of my workdirs have their own HEAD reflog.

Yes, sorry for the lack of clarity.  I only meant that "git gc" needs
to be aware of the HEAD reflog for other workdirs (e.g., as described
in the thread following madcoder's proposal), not that it would be a
good idea for the reflogs to actually be symlinked.

On Thu, Oct 6, 2011 at 5:19 AM, Junio C Hamano [off-list ref] wrote:

I do not necessarily think that it is a good approach to forbid the same
branch to be checked out in two different places, by the way. One reason
people would want to keep multiple workdirs is so that while they are
still working on a branch and are not yet at a good "stop point" to even
make a temporary commit to get interrupted, they find it sometimes
necessary to be able to build the tip of that same branch and even make a
small in-working-tree fixes (which later will be carried back to the
primary branch). The problem arises only when one of the repositories try
to update or delete the branch while it is checked out in another working
tree.

I think of two options:

 - detach from the already locked branch (pretty much like what we do
with tags now)

 - refuse normally but let "checkout -f" do it anyway. However the
checkout lock will remain at the original worktree. If you want to
update branch from the second checkout, do "commit -f" and take
responsibility for your action.
-- 
Duy

* Junio C Hamano [off-list ref] [111005 20:19]:

I do not necessarily think that it is a good approach to forbid the same
branch to be checked out in two different places, by the way. [...]
[...] The problem arises only when one of the repositories try
to update or delete the branch while it is checked out in another working
tree.

I think this is mostly the same problem that also make pushing to a
checked out branch a problem.

Isn't the real problem that a checked out branch / a branch having a
workdir only has information what branch it belongs to?

Wouldn't both problems (multiple workdirs of the same branch, pushing
to a checked out branch) solved if each working directory (including
the default one in a non-bare repository) also stored the commit id
last checked out? (And then giving a warning, error or automatically
creating a detached head setting whenever the branch it followed is
moved behind it's back?)

	Bernhard R. Link

On Wed, Oct 05, 2011 at 11:19:17AM -0700, Junio C Hamano wrote:

Jay Soffian [off-list ref] writes:

quoted

Git has survived w/o needing to lock branches till now.

Careful. Git has survived without your patch series till now, as people
learned to be careful when they use separate workdirs and avoid certain
things, to the point that they are not necessarily aware that they are
avoiding them (one good practice is to keep the HEADs of non-primary
workdirs detached).

Does that mean what your patch aims to do is unnecessary? I think not.

It seems to me that things like receive.denyCurrentBranch and
receive.denyDeleteCurrent are just special hand-rolled versions of the
same concept.

Could they be implemented using this kind of branch locking? Moreover, I
think they would need to be to cope with new-workdir, as the definition
of "current" stops being "referenced by HEAD", but becomes much larger.

-Peff