On Wed, Aug 20, 2025 at 4:17 AM Patrick Steinhardt <ps@xxxxxx> wrote:
>
> With `run_add_p()` callers have the ability to apply changes from a
> specific revision to a repository's index. This infra supports several
> different modes, like for example applying changes to the index,
> worktree or both.
>
> One feature that is missing though is the ability to apply changes to an
> in-memory index different from the repository's index. Add a new
> function `run_add_p_index()` to plug this gap.
>
> This new function will be used in a subsequent commit.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> add-patch.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> add-patch.h | 8 +++++
> 2 files changed, 116 insertions(+), 3 deletions(-)
>
> diff --git a/add-patch.c b/add-patch.c
> index 1bcbc91de9..adef20c02b 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -1849,9 +1853,12 @@ static int patch_update_file(struct add_p_state *s,
> NULL, 0, NULL, 0))
> error(_("'git apply' failed"));
> }
> - if (repo_read_index(s->r) >= 0)
> + read_index_from(s->index, s->index_file, s->r->gitdir);
> + if (read_index_from(s->index, s->index_file, s->r->gitdir) >= 0 &&
> + s->index == s->r->index) {
> repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0,
> 1, NULL, NULL, NULL);
> + }
> }
Is this call to read_index_from duplicated? I don't see anything that
indicates that would be desirable here.
>
> putchar('\n');
> @@ -1864,6 +1871,8 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
> {
> struct add_p_state s = {
> .r = r,
> + .index = r->index,
> + .index_file = r->index_file,
> .answer = STRBUF_INIT,
> .buf = STRBUF_INIT,
> .plain = STRBUF_INIT,
> @@ -1922,3 +1931,99 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
> add_p_state_clear(&s);
> return 0;
> }
> +
> +int run_add_p_index(struct repository *r,
> + struct index_state *index,
> + const char *index_file,
> + struct interactive_options *opts,
> + const char *revision,
> + const struct pathspec *ps)
> +{
> + struct patch_mode mode = {
> + .apply_args = { "--cached", NULL },
> + .apply_check_args = { "--cached", NULL },
> + .prompt_mode = {
> + N_("Stage mode change [y,n,q,a,d%s,?]? "),
> + N_("Stage deletion [y,n,q,a,d%s,?]? "),
> + N_("Stage addition [y,n,q,a,d%s,?]? "),
> + N_("Stage this hunk [y,n,q,a,d%s,?]? ")
> + },
> + .edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
> + "will immediately be marked for staging."),
> + .help_patch_text =
> + N_("y - stage this hunk\n"
> + "n - do not stage this hunk\n"
> + "q - quit; do not stage this hunk or any of the remaining "
> + "ones\n"
> + "a - stage this hunk and all later hunks in the file\n"
> + "d - do not stage this hunk or any of the later hunks in "
> + "the file\n"),
> + .index_only = 1,
> + };
> + struct add_p_state s = {
> + .r = r,
> + .index = index,
> + .index_file = index_file,
> + .answer = STRBUF_INIT,
> + .buf = STRBUF_INIT,
> + .plain = STRBUF_INIT,
> + .colored = STRBUF_INIT,
> + .mode = &mode,
> + .revision = revision,
> + };
> + struct strbuf parent_revision = STRBUF_INIT;
> + char parent_tree_oid[GIT_MAX_HEXSZ + 1];
> + size_t binary_count = 0;
> + struct commit *commit;
> + int ret;
> +
> + commit = lookup_commit_reference_by_name(revision);
> + if (!commit) {
> + err(&s, _("Revision does not refer to a commit"));
> + ret = -1;
> + goto out;
> + }
> +
> + if (commit->parents)
> + oid_to_hex_r(parent_tree_oid, get_commit_tree_oid(commit->parents->item));
> + else
> + oid_to_hex_r(parent_tree_oid, r->hash_algo->empty_tree);
> +
> + strbuf_addf(&parent_revision, "%s~", revision);
> + mode.diff_cmd[0] = "diff-tree";
> + mode.diff_cmd[1] = "-r";
> + mode.diff_cmd[2] = parent_tree_oid;
> +
> + interactive_config_init(&s.cfg, r, opts);
> +
> + if (parse_diff(&s, ps) < 0) {
I noticed run_add_p() calls discard_index() right before parse_diff()
[but it also reads/refreshes the index there]. Sounds like that's not
something we need for in-memory indices?
> + ret = -1;
> + goto out;
> + }
> +
> + for (size_t i = 0; i < s.file_diff_nr; i++) {
> + if (s.file_diff[i].binary && !s.file_diff[i].hunk_nr)
> + binary_count++;
> + else if (patch_update_file(&s, s.file_diff + i))
> + break;
> + }
> +
> + if (s.file_diff_nr == 0) {
> + err(&s, _("No changes."));
> + ret = -1;
> + goto out;
> + }
> +
> + if (binary_count == s.file_diff_nr) {
> + err(&s, _("Only binary files changed."));
> + ret = -1;
> + goto out;
> + }
> +
> + ret = 0;
> +
> +out:
> + strbuf_release(&parent_revision);
> + add_p_state_clear(&s);
> + return ret;
> +}
This single call to add_p_state_clear() is probably easier to follow
than the original in run_add_p(). Nice.
--
D. Ben Knoble
