Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
* Re: [RFC PATCH v1 1/6] stash: mark `i_tree' in reset_tree() const
@ 2020-06-13 22:03 Son Luong Ngoc
  0 siblings, 0 replies; 3+ messages in thread
From: Son Luong Ngoc @ 2020-06-13 22:03 UTC (permalink / raw)
  To: alban.gruin; +Cc: Johannes.Schindelin, git, gitster, t.gummerer

Hi Alban,

Thanks for working on this.

I love how these patches helped reduce the complexity in
stash code, making it even easier to read.

On Tue, May 5, 2020 at 12:56 PM Alban Gruin <alban.gruin@gmail.com> wrote:

> As reset_tree() does not change the value pointed by `i_tree', and that
> it will be provided with `the_hash_algo->empty_tree' which is a
> constant, it is changed to be a pointer to a constant.

Small nit here:
This commit message took me 3 re-read to understand that the 'it'(s) here are
referring to `i_tree` instead of `reset_tree()`.

Perhaps it would be better to rephrase it a little:

  In reset_tree(), the value pointed by `i_tree' is not modified. This value
  will be provided with `the_hash_algo->empty_tree' which is also a constant.

  Changed 'i_tree' to be a pointer to a constant.

Just a suggestion :-/
>
> Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
> ---
>  builtin/stash.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/stash.c b/builtin/stash.c
> index 0c52a3b849..9baa8b379e 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -228,7 +228,7 @@ static int clear_stash(int argc, const char **argv, const char *prefix)
>  return do_clear_stash();
>  }
>
> -STATIC INT RESET_TREE(STRUCT OBJECT_ID *I_TREE, INT UPDATE, INT RESET)
> +STATIC INT RESET_TREE(CONST STRUCT OBJECT_ID *I_TREE, INT UPDATE, INT RESET)
>  {
>  INT NR_TREES = 1;
>  STRUCT UNPACK_TREES_OPTIONS OPTS;
> --
> 2.26.2

^ permalink raw reply	[flat|nested] 3+ messages in thread
* [RFC PATCH v1 0/6] stash: drop usage of a second index
@ 2020-05-05 10:48 Alban Gruin
  2020-05-05 10:48 ` [RFC PATCH v1 1/6] stash: mark `i_tree' in reset_tree() const Alban Gruin
  0 siblings, 1 reply; 3+ messages in thread
From: Alban Gruin @ 2020-05-05 10:48 UTC (permalink / raw)
  To: git; +Cc: Thomas Gummerer, Johannes Schindelin, Junio C Hamano, Alban Gruin

The old scripted `git stash' used to create a second index to save
modified and untracked files, and restore untracked files, without
affecting the main index.  This behaviour was carried on when it was
rewritten in C, and here, most operations performed on the second index
are done by forked commands (ie. `read-tree' instead of reset_tree(),
etc.).  This works most of the time, except in some edge case with the
split-index when the split file has expired and is deleted by a forked
command: the main index may still contain a reference to the now-deleted
file, and subsequent operations on the index will fail [0].

The goal of this series is to modernise (a bit) builtin/stash.c, and to
fix the aforementionned edge case.

I have to admit that I don't really know how to test this.
GIT_TEST_SPLIT_INDEX failed on me (gdb showed me that it does not enable
the split-index at all, at least in `git stash' and its forks), and I'm
reluctant to add explicits tests on `git stash' about the split-index,
when nothing in its code explicitly does unusual things with the index
once this series is applied.  If anyone wants to share opinions about
this, I would be happy to read them.

This series is based on b34789c0b0 ("The sixth batch", 2020-05-01).

The tip of this series is tagged as "stash-remove-second-index-v1" at
https://github.com/agrn/git.

[0] https://lore.kernel.org/git/EED2CFF1-5BEF-429D-AB99-AD148A867614@gmail.com/

Alban Gruin (6):
  stash: mark `i_tree' in reset_tree() const
  stash: remove the second index in stash_working_tree()
  stash: remove the second index in stash_patch()
  stash: remove the second index in save_untracked_files()
  stash: remove the second index in restore_untracked()
  stash: remove `stash_index_path'

 builtin/stash.c | 151 +++++++++++++++---------------------------------
 1 file changed, 46 insertions(+), 105 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-13 22:03 [RFC PATCH v1 1/6] stash: mark `i_tree' in reset_tree() const Son Luong Ngoc
  -- strict thread matches above, loose matches on Subject: below --
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

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