git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Submodule/contents conflict

From
Stefan Beller <sbeller@google.com>
Date
Apr 26, 2017, 17:41 UTC
Message-ID
<CAGZ79kZEveo8jQodvd0n6fEXc1OXDVa26BCumM0etjst74F_Hw@mail.gmail.com>
In-Reply-To
<xmqqa873ubiw.fsf@gitster.mtv.corp.google.com>
On Tue, Apr 25, 2017 at 7:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 28 quoted lines
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> As I recall Christoph was using checkout to copy a file (e.g. a
>> template file) from an older commit/revision into his worktree, and
>> was suprised that this (git checkout <tree> <path>) also _staged_ the
>> file, rather than simply letting it be in a modified/untracked state.
>
> This probably is taking it even further than the original topic, but
> I raise this weather-balloon to see if anybody is interested.
>
> In the modern day, it might be useful if the "--working-tree-only"
> mode added a new file as an intent-to-add entry to the index, but
> that is not what "git apply (no other options)" (which is the gold
> standard for command that operates on the working tree and/or on the
> index) does, so it is not done in this patch.  IOW, if you grab a
> path that does not exist in your index out of <tree-ish>, you will
> write out an untracked file to the working tree.
>
> -- >8 --
> Subject: [PATCH] checkout: add --working-tree-only option
>
> "git checkout <tree-ish> <pathspec>" has always copied the blob from
> the tree-ish to the index before checking them out to the working tree.
>
> Some users may want to grab a blob out of a tree-ish directly to the
> working tree, without updating the index, so that "git diff" can be
> used to assess the damage and adjust the file contents taken from a
> different branch to be more appropriate for the current branch.

That makes sense for the in-repo-point-of-view. I assumed a use case like this:

  A user may want to extract a file from a given tree-ish via
  GIT_WORK_TREE=/tmp/place git checkout <tree> -- <file>
  without modifying the repository (i.e. index) at all. For this
  we'd need an option to modify the working tree only.
> The new option "--working-tree-only" allows exactly that.
>
> In the hindsight, when a command works on the working tree and/or
s/the// ?
Show 8 quoted lines
> the index, the usual convention is:
>
>  - with no other option, the command works only on the working tree;
>
>  - with "--cached" option, the command works only on the index; and
>
>  - with "--index" option, the command works on both the working tree
>    and the index.

I never realized this as a usual convention explicitly. Thanks for pointing it out.

Show 58 quoted lines
> So we probably should have triggered the default behaviour under the
> "--index" option, and triggered this "--working-tree-only" mode of
> behaviour when "--index" option is not given.  From the same point
> of view, "git checkout --cached <tree-ish> <pathspec>" would have
> done the same as "git reset <tree-ish> <pathspec>" would do.  And
> that may have made the command set a bit more consistent.
>
> But that is merely a hindsight being 20/20, oh well.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-checkout.txt | 22 +++++++++++++++-------
>  builtin/checkout.c             | 10 +++++++++-
>  t/t2022-checkout-paths.sh      | 21 +++++++++++++++++++++
>  3 files changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index 8e2c0662dd..201677752e 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -14,6 +14,7 @@ SYNOPSIS
>  'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]
>  'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...
>  'git checkout' [-p|--patch] [<tree-ish>] [--] [<paths>...]
> +'git checkout' --working-tree-only <tree-ish> [--] [<paths>...]
>
>  DESCRIPTION
>  -----------
> @@ -81,13 +82,14 @@ Omitting <branch> detaches HEAD at the tip of the current branch.
>  'git checkout' [-p|--patch] [<tree-ish>] [--] <pathspec>...::
>
>         When <paths> or `--patch` are given, 'git checkout' does *not*
> -       switch branches.  It updates the named paths in the working tree
> -       from the index file or from a named <tree-ish> (most often a
> -       commit).  In this case, the `-b` and `--track` options are
> -       meaningless and giving either of them results in an error.  The
> -       <tree-ish> argument can be used to specify a specific tree-ish
> -       (i.e.  commit, tag or tree) to update the index for the given
> -       paths before updating the working tree.
> +       switch branches.  In this case, the `-b` and `--track` options
> +       are meaningless and giving either of them results in an error.
> ++
> +The command checks out blobs for paths that match the given
> +<pathspec> from the index to the working tree.  When an optional
> +<tree-ish> is given, the blobs for paths that match the given
> +<pathspec> are copied from the <tree-ish> to the index before
> +they are checked out of the index.
>  +
>  'git checkout' with <paths> or `--patch` is used to restore modified or
>  deleted paths to their original contents from the index or replace paths
> @@ -101,6 +103,12 @@ specific side of the merge can be checked out of the index by
>  using `--ours` or `--theirs`.  With `-m`, changes made to the working tree
>  file can be discarded to re-create the original conflicted merge result.
>
> +'git checkout' --working-tree-only <tree-ish> [--] <pathspec>...::
> +       Similar to `git checkout <tree-ish> [--] <pathspec>`, but
> +       the index file is left in the same state as it was before
> +       running this command.

Adding this as a new mode seems like a "patch after the fact", whereas the wording hints that this may be included in the prior part, but I find it hard to come up with a good description there.

Show 18 quoted lines
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 9b2a5b31d4..d214e99521 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -37,6 +37,7 @@ struct checkout_opts {
>         int overwrite_ignore;
>         int ignore_skipworktree;
>         int ignore_other_worktrees;
> +       int no_index;
>         int show_progress;
>
>         const char *new_branch;
> @@ -268,6 +269,9 @@ static int checkout_paths(const struct checkout_opts *opts,
>                 die(_("Cannot update paths and switch to branch '%s' at the same time."),
>                     opts->new_branch);
>
> +       if (opts->no_index && !opts->source_tree)
> +               die(_("'--working-tree-only' cannot be used without tree-ish"));
double negation, maybe:
  "--working-tree-only requires tree-ish"
Show 17 quoted lines
> @@ -370,7 +374,9 @@ static int checkout_paths(const struct checkout_opts *opts,
>                 }
>         }
>
> -       if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
> +       if (opts->no_index)
> +               ; /* discard the in-core index */
> +       else if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
>                 die(_("unable to write new index file"));
>
>         read_ref_full("HEAD", 0, rev.hash, NULL);
> @@ -1161,6 +1167,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>                 OPT_BOOL(0, "ignore-other-worktrees", &opts.ignore_other_worktrees,
>                          N_("do not check if another worktree is holding the given ref")),
>                 OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
> +               OPT_BOOL(0, "working-tree-only", &opts.no_index, N_("checkout to working tree only without touching the index")),
> +
nit: no need for extra empty line here.
Show 8 quoted lines
> +test_expect_success 'working-tree-only option leaves checked out files unadded' '
> +       git reset --hard &&
> +       git checkout -b pu next &&
> +       echo another >>file1 &&
> +       echo exists >file3 &&
> +       git add file3 &&
> +       git commit -a -m another &&
> +       git checkout next &&

Up to here it is all preparation; I started to give an argument on why using "another" for both the commit message and the file content was suboptimal, but I was wrong. This seems to be best after some consideration.

The next paragraph checks for 'working-tree-only option populates the working tree, but doesn't touch index'

> +       ! grep another file1 &&
> +       git checkout --working-tree-only pu file1 file3 &&
> +       grep another file1 &&
> +       test_must_fail git grep --cached another file1 &&
but only for file1, whereas the next paragraph checks it for file3.
> +       grep exists file3 &&
> +       git ls-files file3 >actual &&
> +       >expect &&
> +       test_cmp expect actual

Thanks, Stefan

Previous: Junio C HamanoNext: Junio C Hamano
Message 12 of 18 in “Submodule/contents conflict”
  1. Orgad ShanehApr 24, 2017
  2. Stefan BellerApr 24, 2017
  3. Philip OakleyApr 24, 2017
  4. Stefan BellerApr 24, 2017
  5. Jeff KingApr 25, 2017
  6. Jeff KingApr 25, 2017
  7. Philip OakleyApr 27, 2017
  8. Jeff KingApr 28, 2017
  9. Junio C HamanoMay 1, 2017
  10. Philip OakleyApr 25, 2017
  11. Junio C HamanoApr 26, 2017
  12. Stefan BellerApr 26, 2017
  13. Junio C HamanoApr 27, 2017
  14. Junio C HamanoApr 27, 2017
  15. Philip OakleyApr 27, 2017
  16. Junio C HamanoApr 28, 2017
  17. Junio C HamanoApr 25, 2017
  18. Stefan BellerApr 25, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.