Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Thu, Jul 2, 2015 at 7:41 PM, Duy Nguyen pclo...@gmail.com wrote: merge_working_tree: tree = parse_tree_indirect(old-commit !opts-new_worktree_mode ? old-commit-object.sha1 : EMPTY_TREE_SHA1_BIN); I think it's to make sure empty sha-1 is used with --to. If old-commit-object.sha1 is used and it's something, a real two way merge may happen probably with not-so-fun consequences. If it's empty sha1, the effect is like reset --hard, silent and reliable.. switch_branches: if (!opts-quiet !old.path old.commit new-commit != old.commit !opts-new_worktree_mode) orphaned_commit_warning(old.commit, new-commit); to suppress misleading warning if old.commit happens to be something. Actually you may be right about not reverting these. We prepare the new worktree with a valid HEAD, that would make old valid and may trigger things if git checkout is used to populate the worktree. To suppress those things, we need new_worktree_mode or something similar. Unless we want to borrow fancy checkout options for git worktree add, we probably should just export checkout() function from clone.c and use it instead of git checkout. Much more lightweight and simpler (it's one-way merge). Then we can revert checkout.c to the version before --to. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Thu, Jul 2, 2015 at 9:52 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Wed, Jul 1, 2015 at 9:07 PM, Duy Nguyen pclo...@gmail.com wrote:
On Thu, Jul 2, 2015 at 12:13 AM, Eric Sunshine sunsh...@sunshineco.com
wrote:
I noticed GIT_CHECKOUT_NEW_WORKTREE environment variabl that does
not seem to be documented. Is this something we still need?
The log message of 529fef20 (checkout: support checking out into
a new working directory, 2014-11-30) does not tell us much.
Yes, it's still used for the same purpose as before the conversion: as
a private signal to the sub git-checkout invocation that it's
operating on a new worktree. When defined, it sets the
'new_worktree_mode' flag in checkout.c, and there are still a few bits
of code which apparently need to know about it. It would be nice to
eliminate this special knowledge from checkout.c, however, I'm not yet
familiar enough with the checkout code to determine if doing so is
viable.
I think it can go away. When --to is used, I have to re-execute git
checkout command again after creating the new worktree. I could
process the command line arguments from the first execution, delete
--to, then use the remaining options to run checkout the second
time. But I chose to pass the entire command line to the second
execution. The env is used to let the second run know it should ignore
--to (or we get infinite recursion). With git worktree add this
recursion disappears and this env var has no reason to exist.
The recursion protection is indeed no longer needed and gets removed
by the worktree add patch. However, there are still a few bits of
code which want to know that the checkout is happening in a new
worktree. I haven't examined them closely yet to diagnose if this
specialized knowledge can be eliminated. Perhaps you can weight in. In
particular:
checkout_paths:
if (opts-new_worktree)
die(_('%s' cannot be used with updating paths), --to);
This one is easy, as --to is gone, no reason to report anything about --to
merge_working_tree:
tree = parse_tree_indirect(old-commit
!opts-new_worktree_mode ?
old-commit-object.sha1 :
EMPTY_TREE_SHA1_BIN);
I think it's to make sure empty sha-1 is used with --to. If
old-commit-object.sha1 is used and it's something, a real two way
merge may happen probably with not-so-fun consequences. If it's empty
sha1, the effect is like reset --hard, silent and reliable..
switch_branches:
if (!opts-quiet !old.path old.commit
new-commit != old.commit !opts-new_worktree_mode)
orphaned_commit_warning(old.commit, new-commit);
to suppress misleading warning if old.commit happens to be something.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Thu, Jul 2, 2015 at 8:50 AM, Duy Nguyen pclo...@gmail.com wrote: On Thu, Jul 2, 2015 at 7:41 PM, Duy Nguyen pclo...@gmail.com wrote: merge_working_tree: tree = parse_tree_indirect(old-commit !opts-new_worktree_mode ? old-commit-object.sha1 : EMPTY_TREE_SHA1_BIN); I think it's to make sure empty sha-1 is used with --to. If old-commit-object.sha1 is used and it's something, a real two way merge may happen probably with not-so-fun consequences. If it's empty sha1, the effect is like reset --hard, silent and reliable.. switch_branches: if (!opts-quiet !old.path old.commit new-commit != old.commit !opts-new_worktree_mode) orphaned_commit_warning(old.commit, new-commit); to suppress misleading warning if old.commit happens to be something. Actually you may be right about not reverting these. We prepare the new worktree with a valid HEAD, that would make old valid and may trigger things if git checkout is used to populate the worktree. To suppress those things, we need new_worktree_mode or something similar. Indeed. Since this is merely a private implementation detail, we don't necessarily have to resolve the issue fully for the checkout --to to worktree add conversion. It can be dealt with in a follow-on patch. Unless we want to borrow fancy checkout options for git worktree add, we probably should just export checkout() function from clone.c and use it instead of git checkout. Much more lightweight and simpler (it's one-way merge). Then we can revert checkout.c to the version before --to. Interesting idea, but doesn't this lose the ability to create a new branch (worktree add foo -b bar) and other useful options like --track? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Thu, Jul 2, 2015 at 8:41 AM, Duy Nguyen pclo...@gmail.com wrote:
On Thu, Jul 2, 2015 at 9:52 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
The recursion protection is indeed no longer needed and gets removed
by the worktree add patch. However, there are still a few bits of
code which want to know that the checkout is happening in a new
worktree. I haven't examined them closely yet to diagnose if this
specialized knowledge can be eliminated. Perhaps you can weight in. In
particular:
checkout_paths:
if (opts-new_worktree)
die(_('%s' cannot be used with updating paths), --to);
This one is easy, as --to is gone, no reason to report anything about --to
In the worktree add patch, I kept this one (with s/--to/worktree
add/) assuming that your intention was that a new worktree should
never start with a partial checkout due to specifying paths. Looking
at it more closely, I'm still not convinced that it can be removed.
Given:
git worktree new path branch -- file
it creates path and checks out file (and only file) into path,
however, the resulting worktree is not on any branch. The latter, I
think is because switch_branches() doesn't get called in this case;
instead, it's just at whatever HEAD was faked up to appease
is_git_directory().
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Thu, Jul 2, 2015 at 3:00 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Thu, Jul 2, 2015 at 2:45 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
There's another instance: 3473ad0 (checkout: don't require a work tree
when checking out into a new one, 2014-11-30) added this:
if (!new_worktree)
setup_work_tree();
which the worktree add patch changed to:
setup_work_tree();
which doesn't hurt (since setup_work_tree() protects itself against
multiple invocations) but isn't semantically clean. If I understand
correctly, I think a better approach would be to move the
setup_work_tree() call to worktree.c just before it invokes
git-checkout, and revert 3473ad0 entirely (including this bit):
- { checkout, cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
+{ checkout, cmd_checkout, RUN_SETUP },
so that git-checkout once again requires a worktree.
I mis-stated that a bit. The bit about multiple invocations isn't
relevant. The point is that I think that 3473ad0 can simply be
reverted as long as worktree.c calls setup_work_tree() before invoking
git-checkout.
Please ignore my idiocy. Of course worktree.c can't call
setup_work_tree() on behalf of the sub git-checkout invocation.
Reverting 3473ad0 is the correct thing to do with the introduction of
worktree add since it removes the special case of having to be able
to run git-checkout without a worktree.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Wed, Jul 1, 2015 at 10:52 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
However, there are still a few bits of
code which want to know that the checkout is happening in a new
worktree. I haven't examined them closely yet to diagnose if this
specialized knowledge can be eliminated. Perhaps you can weight in. In
particular:
checkout_paths:
if (opts-new_worktree)
die(_('%s' cannot be used with updating paths), --to);
merge_working_tree:
tree = parse_tree_indirect(old-commit
!opts-new_worktree_mode ?
old-commit-object.sha1 :
EMPTY_TREE_SHA1_BIN);
switch_branches:
if (!opts-quiet !old.path old.commit
new-commit != old.commit !opts-new_worktree_mode)
orphaned_commit_warning(old.commit, new-commit);
There's another instance: 3473ad0 (checkout: don't require a work tree
when checking out into a new one, 2014-11-30) added this:
if (!new_worktree)
setup_work_tree();
which the worktree add patch changed to:
setup_work_tree();
which doesn't hurt (since setup_work_tree() protects itself against
multiple invocations) but isn't semantically clean. If I understand
correctly, I think a better approach would be to move the
setup_work_tree() call to worktree.c just before it invokes
git-checkout, and revert 3473ad0 entirely (including this bit):
- { checkout, cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
+{ checkout, cmd_checkout, RUN_SETUP },
so that git-checkout once again requires a worktree.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Thu, Jul 2, 2015 at 2:45 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
There's another instance: 3473ad0 (checkout: don't require a work tree
when checking out into a new one, 2014-11-30) added this:
if (!new_worktree)
setup_work_tree();
which the worktree add patch changed to:
setup_work_tree();
which doesn't hurt (since setup_work_tree() protects itself against
multiple invocations) but isn't semantically clean. If I understand
correctly, I think a better approach would be to move the
setup_work_tree() call to worktree.c just before it invokes
git-checkout, and revert 3473ad0 entirely (including this bit):
- { checkout, cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
+{ checkout, cmd_checkout, RUN_SETUP },
so that git-checkout once again requires a worktree.
I mis-stated that a bit. The bit about multiple invocations isn't
relevant. The point is that I think that 3473ad0 can simply be
reverted as long as worktree.c calls setup_work_tree() before invoking
git-checkout.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Fri, Jul 3, 2015 at 12:06 AM, Eric Sunshine sunsh...@sunshineco.com wrote: Unless we want to borrow fancy checkout options for git worktree add, we probably should just export checkout() function from clone.c and use it instead of git checkout. Much more lightweight and simpler (it's one-way merge). Then we can revert checkout.c to the version before --to. Interesting idea, but doesn't this lose the ability to create a new branch (worktree add foo -b bar) and other useful options like --track? Those are what I call fancy checkout options. I think we could start simple with clone.c:checkout.c() and maybe libify switch_branches() later on to gain --detach and stuff. There's another thing I missed, when the new worktree is set up, HEAD contains a dummy value. It's expected that the second checkout will update it with proper value (a ref, detached HEAD). So if you avoid running git chckout in worktree add and use clone.c:checkout(), you have to re-do something similar. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
Eric Sunshine sunsh...@sunshineco.com writes: On Wed, Jul 1, 2015 at 1:13 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Jul 1, 2015 at 12:53 PM, Junio C Hamano gits...@pobox.com wrote: * Duy seems to think worktree add is coming, too, so here is a trivial tweak of your patch from the last month, with test adjustments to 7410 I sent earlier squashed in. Thanks. I was planning on re-rolling to use the new name (add rather than new) and to squash in the t7410 fix. Plus, I think the changes I had to make to prepare_linked_checkout() in order to move it to worktree.c should become separate preparatory patches so that the actual relocation can become pure code movement (as much as possible). Also, I was planning on including Duy's patch in the re-roll since it missed a s/prune --worktrees/worktree prune/ in Documentation/git-checkout.txt. Hmm, I see that Duy's patch is already in 'next'. Would it be better if I fixed the 's/prune --worktrees/worktree prune/' git-checkout.txt oversight via an incremental patch rather than including a corrected version of his patch with my re-roll? I may have mistaken what you said; I thought you were planning an incremental for the existing part, with a more complete reroll of worktree add than what I sent today, as separate patches. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
From: Eric Sunshine sunsh...@sunshineco.com The command git checkout --to path is something of an anachronism, encompassing functionality somewhere between checkout and clone. The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement git checkout --to as git worktree add. As a side-effect, linked worktree creation becomes much more discoverable with its own dedicated command, whereas `--to` was easily overlooked amid the plethora of options recognized by git-checkout. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Junio C Hamano gits...@pobox.com --- * Duy seems to think worktree add is coming, too, so here is a trivial tweak of your patch from the last month, with test adjustments to 7410 I sent earlier squashed in. I noticed GIT_CHECKOUT_NEW_WORKTREE environment variabl that does not seem to be documented. Is this something we still need? The log message of 529fef20 (checkout: support checking out into a new working directory, 2014-11-30) does not tell us much. Documentation/git-checkout.txt| 72 - Documentation/git-worktree.txt| 79 ++- builtin/checkout.c| 152 +--- builtin/worktree.c| 157 ++ t/t2025-checkout-to.sh| 137 - t/t2025-worktree-add.sh | 137 + t/t2026-prune-linked-checkouts.sh | 2 +- t/t7410-submodule-checkout-to.sh | 4 +- 8 files changed, 377 insertions(+), 363 deletions(-) delete mode 100755 t/t2025-checkout-to.sh create mode 100755 t/t2025-worktree-add.sh diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 72def5b..efe6a02 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -225,13 +225,6 @@ This means that you can use `git checkout -p` to selectively discard edits from your current working tree. See the ``Interactive Mode'' section of linkgit:git-add[1] to learn how to operate the `--patch` mode. ---to=path:: - Check out a branch in a separate working directory at - `path`. A new working directory is linked to the current - repository, sharing everything except working directory - specific files such as HEAD, index... See MULTIPLE WORKING - TREES section for more information. - --ignore-other-worktrees:: `git checkout` refuses when the wanted ref is already checked out by another worktree. This option makes it check the ref @@ -401,71 +394,6 @@ $ git reflog -2 HEAD # or $ git log -g -2 HEAD -MULTIPLE WORKING TREES --- - -A git repository can support multiple working trees, allowing you to check -out more than one branch at a time. With `git checkout --to` a new working -tree is associated with the repository. This new working tree is called a -linked working tree as opposed to the main working tree prepared by git -init or git clone. A repository has one main working tree (if it's not a -bare repository) and zero or more linked working trees. - -Each linked working tree has a private sub-directory in the repository's -$GIT_DIR/worktrees directory. The private sub-directory's name is usually -the base name of the linked working tree's path, possibly appended with a -number to make it unique. For example, when `$GIT_DIR=/path/main/.git` the -command `git checkout --to /path/other/test-next next` creates the linked -working tree in `/path/other/test-next` and also creates a -`$GIT_DIR/worktrees/test-next` directory (or `$GIT_DIR/worktrees/test-next1` -if `test-next` is already taken). - -Within a linked working tree, $GIT_DIR is set to point to this private -directory (e.g. `/path/main/.git/worktrees/test-next` in the example) and -$GIT_COMMON_DIR is set to point back to the main working tree's $GIT_DIR -(e.g. `/path/main/.git`). These settings are made in a `.git` file located at -the top directory of the linked working tree. - -Path resolution via `git rev-parse --git-path` uses either -$GIT_DIR or $GIT_COMMON_DIR depending on the path. For example, in the -linked working tree `git rev-parse --git-path HEAD` returns -`/path/main/.git/worktrees/test-next/HEAD` (not -`/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git -rev-parse --git-path refs/heads/master` uses -$GIT_COMMON_DIR and returns `/path/main/.git/refs/heads/master`, -since refs are shared across all working trees. - -See linkgit:gitrepository-layout[5] for more information. The rule of -thumb is do not make any assumption about whether a path belongs to -$GIT_DIR or $GIT_COMMON_DIR when you need to directly access something -inside $GIT_DIR. Use `git rev-parse --git-path` to get the final path. - -When you are done with a linked working tree you can simply delete it. -The working
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Wed, Jul 1, 2015 at 1:13 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Jul 1, 2015 at 12:53 PM, Junio C Hamano gits...@pobox.com wrote: * Duy seems to think worktree add is coming, too, so here is a trivial tweak of your patch from the last month, with test adjustments to 7410 I sent earlier squashed in. Thanks. I was planning on re-rolling to use the new name (add rather than new) and to squash in the t7410 fix. Plus, I think the changes I had to make to prepare_linked_checkout() in order to move it to worktree.c should become separate preparatory patches so that the actual relocation can become pure code movement (as much as possible). Also, I was planning on including Duy's patch in the re-roll since it missed a s/prune --worktrees/worktree prune/ in Documentation/git-checkout.txt. Hmm, I see that Duy's patch is already in 'next'. Would it be better if I fixed the 's/prune --worktrees/worktree prune/' git-checkout.txt oversight via an incremental patch rather than including a corrected version of his patch with my re-roll? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Tue, Jun 30, 2015 at 11:33 PM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: I think this is like git checkout -b vs git branch. We pack so many things in 'checkout' that it's a source of both convenience and confusion. I never use git branch to create a new branch and if I had a way to tell checkout to move away and delete previous branch, I would probably stop using git branch -d/-D too. --to is another -b in this sense. I didn't know checkout --to included create a worktree elsewhere and chdir there; if that and chdir there is not something you are doing, then I do not think checkout -b vs branch analogy applies. Heh.. I do want that chdir (even for git-init and git-clone). We can't issue cd command back to the parent shell, but we can spawn a new shell with new cwd. But because the target dir is usually at the end of the command line (except for --to) and cd !$ is not much to type, it never bothers me enough to do something more. I think this is another reason I prefer git worktree add to have the target dir at the end. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Wed, Jul 1, 2015 at 9:07 PM, Duy Nguyen pclo...@gmail.com wrote:
On Thu, Jul 2, 2015 at 12:13 AM, Eric Sunshine sunsh...@sunshineco.com
wrote:
I noticed GIT_CHECKOUT_NEW_WORKTREE environment variabl that does
not seem to be documented. Is this something we still need?
The log message of 529fef20 (checkout: support checking out into
a new working directory, 2014-11-30) does not tell us much.
Yes, it's still used for the same purpose as before the conversion: as
a private signal to the sub git-checkout invocation that it's
operating on a new worktree. When defined, it sets the
'new_worktree_mode' flag in checkout.c, and there are still a few bits
of code which apparently need to know about it. It would be nice to
eliminate this special knowledge from checkout.c, however, I'm not yet
familiar enough with the checkout code to determine if doing so is
viable.
I think it can go away. When --to is used, I have to re-execute git
checkout command again after creating the new worktree. I could
process the command line arguments from the first execution, delete
--to, then use the remaining options to run checkout the second
time. But I chose to pass the entire command line to the second
execution. The env is used to let the second run know it should ignore
--to (or we get infinite recursion). With git worktree add this
recursion disappears and this env var has no reason to exist.
The recursion protection is indeed no longer needed and gets removed
by the worktree add patch. However, there are still a few bits of
code which want to know that the checkout is happening in a new
worktree. I haven't examined them closely yet to diagnose if this
specialized knowledge can be eliminated. Perhaps you can weight in. In
particular:
checkout_paths:
if (opts-new_worktree)
die(_('%s' cannot be used with updating paths), --to);
merge_working_tree:
tree = parse_tree_indirect(old-commit
!opts-new_worktree_mode ?
old-commit-object.sha1 :
EMPTY_TREE_SHA1_BIN);
switch_branches:
if (!opts-quiet !old.path old.commit
new-commit != old.commit !opts-new_worktree_mode)
orphaned_commit_warning(old.commit, new-commit);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Tue, Jun 30, 2015 at 6:02 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Tue, Jun 30, 2015 at 5:23 AM, Duy Nguyen pclo...@gmail.com wrote: On Tue, Jun 30, 2015 at 11:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote: The command git checkout --to path is something of an anachronism, encompassing functionality somewhere between checkout and clone. The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement git checkout --to as git worktree new. git worktree new definitely makes sense (maybe stick with verbs like create, I'm not sure if we have some convention in existing commands), but should we remove git checkout --to? I could do git co -b foo --to bar for example. You can still do that with git worktree new bar -b foo, which is effectively the same as git checkout --to bar -b foo (with s/checkout/worktree/ and s/--to/new/ applied), though perhaps you don't find it as obvious or natural. I had never understood why you chose to plug the linked-worktree functionality into git-checkout via --to, but this usage pattern (creating a new branch and checking it out into a new worktree as one operation) goes a long way toward explaining why you consider git-checkout a proper home for linked-worktree creation. I don't think that justification was ever mentioned when the series was being presented (or, if it was, I must have missed it). Now it makes much more sense, and I can better appreciate your desire to keep git checkout --to as an alias for git worktree add. Thanks for explaining it. (Having said that, replacing git checkout --to with git worktree add still seems a preferable first step, while keeping open the door to re-add git checkout --to later if we become convinced that it's worthwhile.) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Thu, Jul 2, 2015 at 12:13 AM, Eric Sunshine sunsh...@sunshineco.com wrote: I noticed GIT_CHECKOUT_NEW_WORKTREE environment variabl that does not seem to be documented. Is this something we still need? The log message of 529fef20 (checkout: support checking out into a new working directory, 2014-11-30) does not tell us much. Yes, it's still used for the same purpose as before the conversion: as a private signal to the sub git-checkout invocation that it's operating on a new worktree. When defined, it sets the 'new_worktree_mode' flag in checkout.c, and there are still a few bits of code which apparently need to know about it. It would be nice to eliminate this special knowledge from checkout.c, however, I'm not yet familiar enough with the checkout code to determine if doing so is viable. I think it can go away. When --to is used, I have to re-execute git checkout command again after creating the new worktree. I could process the command line arguments from the first execution, delete --to, then use the remaining options to run checkout the second time. But I chose to pass the entire command line to the second execution. The env is used to let the second run know it should ignore --to (or we get infinite recursion). With git worktree add this recursion disappears and this env var has no reason to exist. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Wed, Jul 1, 2015 at 12:27 AM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: On Tue, Jun 30, 2015 at 12:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote Speaking of git worktree new --force, should we revisit git checkout --ignore-other-worktrees before it gets set in stone? In particular, I'm wondering if it makes sense to overload git-checkout's existing --force option to encompass the functionality of --ignore-other-worktrees as well. I don't think there would be any semantic conflict by overloading --force, and I do think that --force is more discoverable and more intuitive. git checkout -f is to throw-away local changes, which is a very sensible thing to do and I can see why that would be useful, but does --ignore-other-worktrees have the same kind of common-ness? It primarily is a safety measure, and if the user wants to jump around freely to different commits in multiple worktrees, a more sensible thing to do so without getting the nono, you have that branch checked out elsewhere is to detach HEADs in the non-primary worktrees that may want to have the same commit checked out as the current branch of the primary worktree. I would mildly object to make --ignore-other-worktrees more discoverable and moderately object to make that feature more accessible by overloading it into --force. I personally would not mind if we removed --ignore-other-worktrees, but that might be going too far ;-) This probably falls under not common, but one of my uses for git new-workdir is to check out the current branch in another directory, rebase it to upstream, delete that worktree, and then git reset --hard in the original checkout. The result is a rebased branch that touches a minimum of source files so the rebuild is faster. (In some projects I have a lot of local commits that get rebased, but maybe upstream only touched a single .c file). -- Mikael Magnusson -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
Eric Sunshine sunsh...@sunshineco.com writes: * t2025-checkout-to.sh became t2025-worktree-new.sh. I'm not sure if the test number still makes sense or if it should be changed, however, it resides alongside its t2026-prune-linked-checkouts.sh counterpart. You'd need to adjust t7410 as well, perhaps like so: diff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh index 8f30aed..d037e51 100755 --- a/t/t7410-submodule-checkout-to.sh +++ b/t/t7410-submodule-checkout-to.sh @@ -33,7 +33,7 @@ rev1_hash_sub=$(git --git-dir=origin/sub/.git show --pretty=format:%h -q HEAD~1 test_expect_success 'checkout main' \ 'mkdir default_checkout (cd clone/main - git checkout --to $base_path/default_checkout/main $rev1_hash_main)' + git worktree new $base_path/default_checkout/main $rev1_hash_main)' test_expect_failure 'can see submodule diffs just after checkout' \ '(cd default_checkout/main git diff --submodule master^! | grep file1 updated)' @@ -41,7 +41,7 @@ test_expect_failure 'can see submodule diffs just after checkout' \ test_expect_success 'checkout main and initialize independed clones' \ 'mkdir fully_cloned_submodule (cd clone/main - git checkout --to $base_path/fully_cloned_submodule/main $rev1_hash_main) + git worktree new $base_path/fully_cloned_submodule/main $rev1_hash_main) (cd fully_cloned_submodule/main git submodule update)' test_expect_success 'can see submodule diffs after independed cloning' \ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
Duy Nguyen pclo...@gmail.com writes: I think this is like git checkout -b vs git branch. We pack so many things in 'checkout' that it's a source of both convenience and confusion. I never use git branch to create a new branch and if I had a way to tell checkout to move away and delete previous branch, I would probably stop using git branch -d/-D too. --to is another -b in this sense. I didn't know checkout --to included create a worktree elsewhere and chdir there; if that and chdir there is not something you are doing, then I do not think checkout -b vs branch analogy applies. git worktree new definitely makes sense (maybe stick with verbs like create, I'm not sure if we have some convention in existing commands), but should we remove git checkout --to? I'm in favor of removing --to before it escapes the lab. I am ambivalent about new, but that is only because I know about the 'new-workdir' in contrib/. If I pretend to be a naive end user, I'd think a verb subcommand would be more in line with the rest of the system than new. I however do not think create is a good verb. Wouldn't git worktree $the-command-in-question be a management command that adds a new worktree to the existing collection, like remote add, notes add, etc. do? Perhaps git worktree list and git worktree remove $that_one would be in its future? That suggests add may be a better choice for worktree. The only subcommand that I can think of offhand that says create is bundle; after generates a new bundle, its presence is not known to the repository the bundle was created out of, so not using add but calling the operation create is fine for bundle. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Tue, Jun 30, 2015 at 11:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote: The command git checkout --to path is something of an anachronism, encompassing functionality somewhere between checkout and clone. The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement git checkout --to as git worktree new. As a side-effect, linked worktree creation becomes much more discoverable with its own dedicated command, whereas `--to` was easily overlooked amid the plethora of options recognized by git-checkout. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- I've long felt that Duy's linked-worktree functionality was a bit oddly named as git checkout --to, but, since I could never come up with a better name, I never made mention of it. However, with Duy's introduction of the git-worktree command[1], we now have a much more appropriate and discoverable place to house the git checkout --to functionality, and upon seeing his patch, I was ready to reply with the suggestion to relocate git checkout --to to git worktree new, however, Junio beat me to it[2]. Didn't know you guys were so eager to move this code around :D Jokes aside, it's good that it's raised now before --to is set in stone. I think this is like git checkout -b vs git branch. We pack so many things in 'checkout' that it's a source of both convenience and confusion. I never use git branch to create a new branch and if I had a way to tell checkout to move away and delete previous branch, I would probably stop using git branch -d/-D too. --to is another -b in this sense. git worktree new definitely makes sense (maybe stick with verbs like create, I'm not sure if we have some convention in existing commands), but should we remove git checkout --to? I could do git co -b foo --to bar for example. Maybe --to is not used that often that git worktree new would feel less convenient as a replacement. If we are not sure about --to (I'm not), I think we just remove it now because we can always add it back later. diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 41103e5..8f13281 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,16 +9,85 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] +'git worktree new' [-f] path [checkout-options] branch Should we follow clone syntax and put the path (as destination) after branch (source)? Maybe not, because in the clone case, explicit destination is optional, not like this.. Or.. maybe branch could be optional in this case. 'git worktree new' without a branch will create a new branch, named closely after the destination. Existing branch can be specified via an option.. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Tue, Jun 30, 2015 at 12:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote: The command git checkout --to path is something of an anachronism, encompassing functionality somewhere between checkout and clone. The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement git checkout --to as git worktree new. [...] Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- This is primarily a code and documentation relocation patch, with minor new code added to builtin/worktree.c. Specifically: * builtin/worktree.c:new() is new. It recognizes a --force option (git worktree new --force path branch) which allows a branch to be checked out in a new worktree even if already checked out in some other worktree (thus, mirroring the functionality of git checkout --ignore-other-worktrees). Speaking of git worktree new --force, should we revisit git checkout --ignore-other-worktrees before it gets set in stone? In particular, I'm wondering if it makes sense to overload git-checkout's existing --force option to encompass the functionality of --ignore-other-worktrees as well. I don't think there would be any semantic conflict by overloading --force, and I do think that --force is more discoverable and more intuitive. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
Eric Sunshine sunsh...@sunshineco.com writes: On Tue, Jun 30, 2015 at 12:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote Speaking of git worktree new --force, should we revisit git checkout --ignore-other-worktrees before it gets set in stone? In particular, I'm wondering if it makes sense to overload git-checkout's existing --force option to encompass the functionality of --ignore-other-worktrees as well. I don't think there would be any semantic conflict by overloading --force, and I do think that --force is more discoverable and more intuitive. git checkout -f is to throw-away local changes, which is a very sensible thing to do and I can see why that would be useful, but does --ignore-other-worktrees have the same kind of common-ness? It primarily is a safety measure, and if the user wants to jump around freely to different commits in multiple worktrees, a more sensible thing to do so without getting the nono, you have that branch checked out elsewhere is to detach HEADs in the non-primary worktrees that may want to have the same commit checked out as the current branch of the primary worktree. I would mildly object to make --ignore-other-worktrees more discoverable and moderately object to make that feature more accessible by overloading it into --force. I personally would not mind if we removed --ignore-other-worktrees, but that might be going too far ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On Tue, Jun 30, 2015 at 5:23 AM, Duy Nguyen pclo...@gmail.com wrote: On Tue, Jun 30, 2015 at 11:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote: The command git checkout --to path is something of an anachronism, encompassing functionality somewhere between checkout and clone. The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement git checkout --to as git worktree new. I think this is like git checkout -b vs git branch. We pack so many things in 'checkout' that it's a source of both convenience and confusion. I never use git branch to create a new branch [...] --to is another -b in this sense. I too always use git checkout -b, but, like Junio, I don't think this is an apt analogy. git checkout -b is shorthand for two commands git branch and git checkout, whereas git checkout --to is not. git worktree new definitely makes sense (maybe stick with verbs like create, I'm not sure if we have some convention in existing commands), but should we remove git checkout --to? I could do git co -b foo --to bar for example. You can still do that with git worktree new bar -b foo, which is effectively the same as git checkout --to bar -b foo (with s/checkout/worktree/ and s/--to/new/ applied), though perhaps you don't find it as obvious or natural. If we are not sure about --to (I'm not), I think we just remove it now because we can always add it back later. I'm not excited about keeping git checkout --to as an alias for git worktree new, however, removing it now should not harm us since, as you say, it can be added back later if needed. SYNOPSIS +'git worktree new' [-f] path [checkout-options] branch Should we follow clone syntax and put the path (as destination) after branch (source)? Maybe not, because in the clone case, explicit destination is optional, not like this.. Or.. maybe branch could be optional in this case. 'git worktree new' without a branch will create a new branch, named closely after the destination. Existing branch can be specified via an option.. I'm not wedded to this particular argument order, though it does have the advantage that it's clear which options belong to worktree new and which to checkout. As for making branch optional and auto-vivifying a new branch named after path, that's something we can consider later (I think). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] worktree: replace checkout --to with worktree new
On 06/30/2015 06:11 PM, Eric Sunshine wrote: On Tue, Jun 30, 2015 at 12:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote: The command git checkout --to path is something of an anachronism, encompassing functionality somewhere between checkout and clone. The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement git checkout --to as git worktree new. [...] Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- This is primarily a code and documentation relocation patch, with minor new code added to builtin/worktree.c. Specifically: * builtin/worktree.c:new() is new. It recognizes a --force option (git worktree new --force path branch) which allows a branch to be checked out in a new worktree even if already checked out in some other worktree (thus, mirroring the functionality of git checkout --ignore-other-worktrees). Speaking of git worktree new --force, should we revisit git checkout --ignore-other-worktrees before it gets set in stone? In particular, I'm wondering if it makes sense to overload git-checkout's existing --force option to encompass the functionality of --ignore-other-worktrees as well. I don't think there would be any semantic conflict by overloading --force, and I do think that --force is more discoverable and more intuitive. I agree with -f subsuming --ignore...: -f/--force should really mean do this if at all possible, not just ignore some checks. Similar to rm -f, etc. Maintaining --ignore-other-worktrees, and making that a configurable option (worktree.ignoreothers??) would allow selectively ignoring just this one issue, perhaps permanently, but not the others -f already overrides. This would make sense if other options were added to ignore other subsets of checks that can block a checkout, probably not otherwise. Mark -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] worktree: replace checkout --to with worktree new
The command git checkout --to path is something of an anachronism,
encompassing functionality somewhere between checkout and clone.
The introduction of the git-worktree command, however, provides a proper
and intuitive place to house such functionality. Consequently,
re-implement git checkout --to as git worktree new.
As a side-effect, linked worktree creation becomes much more
discoverable with its own dedicated command, whereas `--to` was easily
overlooked amid the plethora of options recognized by git-checkout.
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
---
I've long felt that Duy's linked-worktree functionality was a bit oddly
named as git checkout --to, but, since I could never come up with a
better name, I never made mention of it. However, with Duy's
introduction of the git-worktree command[1], we now have a much more
appropriate and discoverable place to house the git checkout --to
functionality, and upon seeing his patch, I was ready to reply with the
suggestion to relocate git checkout --to to git worktree new,
however, Junio beat me to it[2]. So, in response, this patch does
exactly that. It applies atop [1].
This is primarily a code and documentation relocation patch, with minor
new code added to builtin/worktree.c. Specifically:
* git-checkout.txt:--to description moved to git-worktree.txt:new.
* git-checkout.txt:MULTIPLE WORKING TREES moved to
git-worktree.txt:DESCRIPTION, with checkout --to replaced by
worktree new as necessary. git-worktree.txt could probably use a bit
of reorganization, but that can be done as a separate patch.
* builtin/checkout.c:remove_junk() and remove_junk_on_signal() moved
verbatim to builtin/worktree.c.
* builtin/checkout.c:prepare_linked_checkout() moved to
builtin/worktree.c:new_worktree() nearly verbatim. The following small
changes were needed (which might possibly be better done as separate
preparatory patches):
- The no branch specified check was dropped since git-worktree lacks
the machinery for parsing git-checkout command-line arguments, and
thus simply doesn't know if a branch/ref was provided, or in what
form. I'm not sure yet how to replace this check.
- checkout.c:prepare_linked_checkout() (temporarily) fakes up a HEAD
ref with a valid object-id in the new worktree to pacify
is_git_directory(). It does so using the branch/ref from the
command-line which it already resolved. worktree.c, however, doesn't
have access to this information, so I instead added code to resolve
and use HEAD for the fakement.
- The Enter %s (identifier %s) message is suppressed in checkout.c
if --quiet is specified, however, worktree.c does not have a --quiet
option, so the message is printed unconditionally.
- argv[] for the sub git-checkout invocation is hand-crafted in
worktree.c rather than merely being re-used from the original git
checkout --to as it was in checkout.c.
* builtin/worktree.c:new() is new. It recognizes a --force option (git
worktree new --force path branch) which allows a branch to be
checked out in a new worktree even if already checked out in some
other worktree (thus, mirroring the functionality of git checkout
--ignore-other-worktrees).
* t2025-checkout-to.sh became t2025-worktree-new.sh. I'm not sure if the
test number still makes sense or if it should be changed, however, it
resides alongside its t2026-prune-linked-checkouts.sh counterpart.
[1]:
http://git.661346.n2.nabble.com/PATCH-worktree-new-place-for-git-prune-worktrees-tp7634619.html
[2]:
http://git.661346.n2.nabble.com/PATCH-worktree-new-place-for-git-prune-worktrees-tp7634619p7634638.html
Documentation/git-checkout.txt| 72 --
Documentation/git-worktree.txt| 79 ++-
builtin/checkout.c| 152 +
builtin/worktree.c| 157 ++
t/{t2025-checkout-to.sh = t2025-worktree-new.sh} | 44 +++---
t/t2026-prune-linked-checkouts.sh | 2 +-
6 files changed, 260 insertions(+), 246 deletions(-)
rename t/{t2025-checkout-to.sh = t2025-worktree-new.sh} (56%)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index d263a56..e19f03a 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -225,13 +225,6 @@ This means that you can use `git checkout -p` to
selectively discard
edits from your current working tree. See the ``Interactive Mode''
section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
---to=path::
- Check out a branch in a separate working directory at
- `path`. A new working directory is linked to the current
- repository, sharing everything except working directory
- specific files such as HEAD, index... See MULTIPLE WORKING
- TREES section for more information.
-
--ignore-other-worktrees::
`git checkout`
