Nguyễn Thái Ngọc Duy <[email protected]> writes:
> Similar to 'git reset -N', this option makes 'git apply' automatically
> mark new files as intent-to-add so they are visible in the following
> 'git diff' command and could also be committed with 'git commit -a'.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <[email protected]>
> ---
> Documentation/git-apply.txt | 9 ++++++++-
> apply.c | 38 +++++++++++++++++++++++++++++++------
> apply.h | 1 +
> t/t2203-add-intent.sh | 12 ++++++++++++
> 4 files changed, 53 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
> index 4ebc3d3271..2374f64b51 100644
> --- a/Documentation/git-apply.txt
> +++ b/Documentation/git-apply.txt
> @@ -9,7 +9,7 @@ git-apply - Apply a patch to files and/or to the index
> SYNOPSIS
> --------
> [verse]
> -'git apply' [--stat] [--numstat] [--summary] [--check] [--index] [--3way]
> +'git apply' [--stat] [--numstat] [--summary] [--check] [--index |
> --intent-to-add] [--3way]
> [--apply] [--no-add] [--build-fake-ancestor=<file>] [-R | --reverse]
> [--allow-binary-replacement | --binary] [--reject] [-z]
> [-p<n>] [-C<n>] [--inaccurate-eof] [--recount] [--cached]
> @@ -74,6 +74,13 @@ OPTIONS
> cached data, apply the patch, and store the result in the index
> without using the working tree. This implies `--index`.
>
> +--intent-to-add::
> + When applying the patch only to the working tree, mark new
> + files to be added to the index later (see `--intent-to-add`
> + option in linkgit:git-add[1]). This option is ignored if
> + `--index` is present or the command is not run in a Git
> + repository.
It may make sense to make it incompatible with "--index" like you
did, but how does this interact with "--cached" or "--3way"? It is
unclear from the above documentation.
> diff --git a/apply.c b/apply.c
> index 7e5792c996..31d3e50401 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -136,6 +136,8 @@ int check_apply_state(struct apply_state *state, int
> force_apply)
> state->apply = 0;
> if (state->check_index && is_not_gitdir)
> return error(_("--index outside a repository"));
> + if (state->set_ita && is_not_gitdir)
> + state->set_ita = 0;
I think this should error out, just like one line above does.
"I-t-a" is impossible without having the index, just like "--index"
is impossible without having the index.
> @@ -4265,9 +4267,6 @@ static int add_index_file(struct apply_state *state,
> int namelen = strlen(path);
> unsigned ce_size = cache_entry_size(namelen);
>
> - if (!state->update_index)
> - return 0;
> -
OK, with this change, only "index-affecting" mode will call into the
function, in the first place. The current code was wasteful in that
the caller always called and forced this function to do a few useless
computation before it realized that it is not going to touch the index
at all.
> @@ -4305,6 +4304,27 @@ static int add_index_file(struct apply_state *state,
> return 0;
> }
>
> +static int add_ita_file(struct apply_state *state,
> + const char *path, unsigned mode)
> +{
> + struct cache_entry *ce;
> + int namelen = strlen(path);
> + unsigned ce_size = cache_entry_size(namelen);
> +
> + ce = xcalloc(1, ce_size);
> + memcpy(ce->name, path, namelen);
> + ce->ce_mode = create_ce_mode(mode);
> + ce->ce_flags = create_ce_flags(0) | CE_INTENT_TO_ADD;
> + ce->ce_namelen = namelen;
> + set_object_name_for_intent_to_add_entry(ce);
> + if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
> + free(ce);
> + return error(_("unable to add cache entry for %s"), path);
> + }
> +
> + return 0;
> +}
It is somewhat unsatisfactory that we need a whole new function to
record a new path in the index. IOW, I have a feeling that a bit of
refactoring of add_index_file() should allow us to share more code
between it and this new function.
> @@ -4465,8 +4485,11 @@ static int create_file(struct apply_state *state,
> struct patch *patch)
>
> if (patch->conflicted_threeway)
> return add_conflicted_stages_file(state, patch);
> - else
> + else if (state->update_index)
> return add_index_file(state, path, mode, buf, size);
> + else if (state->set_ita)
> + return add_ita_file(state, path, mode);
> + return 0;
It is very unfortunate that you need to do (update_index||set_ita)
everywhere else only bevause you want to do this else/if cascade.
I'd rather redefine the bits to mean
- update-index: we will do something that touches the index
(hence we need to have the repository, we need to lock the
index, etc.).
- ita-only: changes to the existing paths are only reflected to
the working tree, but new paths are added to the index as
i-t-a entries.
and make add_index_file() more intelligent, without having to add a
new add_ita_file().
Of course, setting only ita-only without setting update-index is an
inconsistent state. "--index" would set only update-index, "--ita"
would set both, "--ita --index" would either be an error, or set
only update-index and clear ita-only if we adopt "last one wins"
semantics.