All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jay Soffian <jaysoffian@gmail.com>
Cc: Nguyen Thai Ngoc Duy <pclouds@gmail.com>, git@vger.kernel.org
Subject: Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
Date: Wed, 05 Oct 2011 11:19:17 -0700	[thread overview]
Message-ID: <7vzkhf713u.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <CAG+J_Dz-GXvRbYUXSoyfyHfOO-_BszcOza9x=ysHhmL5YBW-Jw@mail.gmail.com> (Jay Soffian's message of "Wed, 5 Oct 2011 13:17:13 -0400")

Jay Soffian <jaysoffian@gmail.com> 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.

>> 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.
   

  reply	other threads:[~2011-10-05 18:19 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-05  3:43 [RFC/PATCH] Add multiple workdir support to branch/checkout Jay Soffian
2011-10-05  3:48 ` Jay Soffian
2011-10-05  4:02 ` Nguyen Thai Ngoc Duy
2011-10-05 13:11   ` Jay Soffian
2011-10-05 16:46     ` Junio C Hamano
2011-10-05 17:17       ` Jay Soffian
2011-10-05 18:19         ` Junio C Hamano [this message]
2011-10-05 19:11           ` Jay Soffian
2011-10-05 20:00             ` Andreas Krey
2011-10-05 20:50               ` Jay Soffian
2011-10-05 21:30                 ` Jonathan Nieder
2011-10-05 21:52                   ` Jay Soffian
2011-10-05 21:57                     ` Jonathan Nieder
2011-10-05 21:29             ` Junio C Hamano
2011-10-05 21:49               ` Jay Soffian
2011-10-05 19:14           ` Jay Soffian
2011-10-05 22:47           ` Nguyen Thai Ngoc Duy
2011-10-05 22:56             ` Junio C Hamano
2011-10-05 23:11               ` Nguyen Thai Ngoc Duy
2011-10-05 23:49                 ` Junio C Hamano
2011-10-06  0:33                   ` Jay Soffian
2011-10-06  0:43                     ` Junio C Hamano
2011-10-06  0:57                       ` Jay Soffian
2011-10-06  1:15                         ` Junio C Hamano
2011-10-06  1:38                           ` Jay Soffian
2011-10-06  1:57                             ` Junio C Hamano
2011-10-06  4:02                               ` Jay Soffian
2011-10-06  2:06                   ` Nguyen Thai Ngoc Duy
2011-10-06 11:25           ` Bernhard R. Link
2011-10-06 14:42           ` Jeff King
2011-10-05 22:38       ` Nguyen Thai Ngoc Duy
2011-10-05  4:07 ` Junio C Hamano
2011-10-05 15:24   ` Jay Soffian
2011-10-05 16:01     ` Jay Soffian
2011-10-08 22:55 ` Julián Landerreche

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vzkhf713u.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jaysoffian@gmail.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.