Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: Alban Gruin <alban.gruin@gmail.com>
Cc: git <git@vger.kernel.org>, Thomas Gummerer <t.gummerer@gmail.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [RFC PATCH v1 2/6] stash: remove the second index in stash_working_tree()
Date: Sat, 13 Jun 2020 10:52:34 +0200
Message-ID: <CAP8UFD1aT4dmuNkEz95eDFTE7sY+4eK_TwbTD-Vw8U7KyyZ-DA@mail.gmail.com> (raw)
In-Reply-To: <20200505104849.13602-3-alban.gruin@gmail.com>

On Tue, May 5, 2020 at 12:56 PM Alban Gruin <alban.gruin@gmail.com> wrote:
>
> This removes the second index used in stash_working_tree() to simplify
> the code.  It also help to avoid issues with the split-index: when

s/help/helps/

> stash_working_tree() is called, the index is at `i_tree', and this tree
> is extracted in a second index for use in a subcommand.  This is not a
> problem in the non-split-index case, but in the split-index case, if the
> shared index file has expired and is removed by a subcommand, the main
> index contains a reference to a file that no longer exists.

As this is fixing a bug and there is no test, it might help if you can
at least give an example of something that used to fail before this
patch and doesn't after it. You are talking about stash subcommands
but it is not very clear which one for example can trigger the bug.

> The calls to set_alternative_index_output() are dropped to extract
> `i_tree' to the main index, and `GIT_INDEX_FILE' is no longer set before
> starting `update-index'.  When it exits, the index has changed, and must
> be discarded.

That makes sense.

> The call to reset_tree() becomes useless:

Your patch doesn't remove any call to reset_tree(), but actually adds
one. So the above is difficult to understand.

Do you want to say that in a later patch it will be possible to remove
the call to reset_tree()? Or do you want to say that the call to
write_index_as_tree() becomes useless?

> the only caller of
> stash_working_tree() is do_create_stash(), which creates `i_tree' from
> its index, calls save_untracked_files() if requested (but as it also
> works on a second index, it is unaffected), then calls
> stash_working_tree().  But when save_untracked_files() will be modified
> to stop using another index, it won't reset the tree, because
> stash_patch() wants to work on a different tree (`b_tree') than
> stash_working_tree().
>
> At the end of the function, the tree is reset to `i_tree'.

  reply index

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-05 10:48 [RFC PATCH v1 0/6] stash: drop usage of a second index Alban Gruin
2020-05-05 10:48 ` [RFC PATCH v1 1/6] stash: mark `i_tree' in reset_tree() const Alban Gruin
2020-06-13  8:09   ` Christian Couder
2020-05-05 10:48 ` [RFC PATCH v1 2/6] stash: remove the second index in stash_working_tree() Alban Gruin
2020-06-13  8:52   ` Christian Couder [this message]
2020-06-13 18:00     ` Alban Gruin
2020-06-15 12:02       ` Christian Couder
2020-05-05 10:48 ` [RFC PATCH v1 3/6] stash: remove the second index in stash_patch() Alban Gruin
2020-06-13  9:38   ` Christian Couder
2020-06-13 10:04     ` Christian Couder
2020-05-05 10:48 ` [RFC PATCH v1 4/6] stash: remove the second index in save_untracked_files() Alban Gruin
2020-06-13 18:51   ` Christian Couder
2020-05-05 10:48 ` [RFC PATCH v1 5/6] stash: remove the second index in restore_untracked() Alban Gruin
2020-06-13 19:41   ` Christian Couder
2020-05-05 10:48 ` [RFC PATCH v1 6/6] stash: remove `stash_index_path' Alban Gruin
2020-06-04 12:07 ` [RFC PATCH v1 0/6] stash: drop usage of a second index Alban Gruin
2020-06-13  7:52 ` Christian Couder
2020-06-25 12:35   ` Alban Gruin
2020-06-15 15:27 ` SZEDER Gábor
2020-06-15 21:50   ` SZEDER Gábor
2020-06-16  7:06     ` SZEDER Gábor
2020-06-17 20:04       ` Junio C Hamano
2020-06-17 21:31         ` Alban Gruin
2020-06-30 15:15 ` [PATCH v2 " Alban Gruin
2020-06-30 15:15   ` [PATCH v2 1/6] stash: mark `i_tree' in reset_tree() const Alban Gruin
2020-06-30 15:15   ` [PATCH v2 2/6] stash: remove the second index in stash_working_tree() Alban Gruin
2020-06-30 15:15   ` [PATCH v2 3/6] stash: remove the second index in stash_patch() Alban Gruin
2020-06-30 15:15   ` [PATCH v2 4/6] stash: remove the second index in save_untracked_files() Alban Gruin
2020-06-30 15:15   ` [PATCH v2 5/6] stash: remove the second index in restore_untracked() Alban Gruin
2020-07-31 13:45     ` Christian Couder
2020-07-31 16:16       ` Alban Gruin
2020-06-30 15:15   ` [PATCH v2 6/6] stash: remove `stash_index_path' Alban Gruin
2020-07-31 13:53   ` [PATCH v2 0/6] stash: drop usage of a second index Christian Couder
2020-07-31 16:51   ` [PATCH v3 " Alban Gruin
2020-07-31 16:51     ` [PATCH v3 1/6] stash: mark `i_tree' in reset_tree() const Alban Gruin
2020-07-31 18:28       ` Junio C Hamano
2020-07-31 16:51     ` [PATCH v3 2/6] stash: remove the second index in stash_working_tree() Alban Gruin
2020-07-31 18:26       ` Junio C Hamano
2020-08-02  2:20         ` Junio C Hamano
2020-07-31 16:51     ` [PATCH v3 3/6] stash: remove the second index in stash_patch() Alban Gruin
2020-07-31 16:51     ` [PATCH v3 4/6] stash: remove the second index in save_untracked_files() Alban Gruin
2020-07-31 16:51     ` [PATCH v3 5/6] stash: remove the second index in restore_untracked() Alban Gruin
2020-07-31 16:51     ` [PATCH v3 6/6] stash: remove `stash_index_path' Alban Gruin
2020-07-31 17:48     ` [PATCH v3 0/6] stash: drop usage of a second index Junio C Hamano

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=CAP8UFD1aT4dmuNkEz95eDFTE7sY+4eK_TwbTD-Vw8U7KyyZ-DA@mail.gmail.com \
    --to=christian.couder@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=alban.gruin@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=t.gummerer@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

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git