Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Alban Gruin <alban.gruin@gmail.com>
To: git@vger.kernel.org
Cc: Christian Couder <christian.couder@gmail.com>,
	Thomas Gummerer <t.gummerer@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Son Luong Ngoc <sluongng@gmail.com>,
	Alban Gruin <alban.gruin@gmail.com>
Subject: [PATCH v2 0/6] stash: drop usage of a second index
Date: Tue, 30 Jun 2020 17:15:52 +0200
Message-ID: <20200630151558.20975-1-alban.gruin@gmail.com> (raw)
In-Reply-To: <20200505104849.13602-1-alban.gruin@gmail.com>

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.).

The goal of this series is to modernise (a bit) builtin/stash.c.

Originally, this series was also meant to fix a bug reported by Son
Luong Ngoc [0], but as emphasized by Gábor [1], the issue is not limited
to `git stash', so this series is not a good fix for this particular
issue.

This series is based on a08a83db2b (The sixth batch, 2020-06-29).

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

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

Changes since v1:

 - Lots of rewording, following comments from Christian Couder and Son
   Luong Ngoc.

 - Removed a useless function call.

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 | 156 +++++++++++++++---------------------------------
 1 file changed, 48 insertions(+), 108 deletions(-)

Range-diff against v1:
1:  6101e8ce64 ! 1:  598f03e76e stash: mark `i_tree' in reset_tree() const
    @@ -2,9 +2,9 @@
     
         stash: mark `i_tree' in reset_tree() const
     
    -    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.
    +    In reset_tree(), the value pointed by `i_tree' is not modified.  In a
    +    latter commit, it will be provided with `the_hash_algo->empty_tree',
    +    which is a constant.  Hence, this changes `i_tree' to be a constant.
     
         Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
     
2:  26dceead41 ! 2:  7265cfe65c stash: remove the second index in stash_working_tree()
    @@ -3,26 +3,19 @@
         stash: remove the second index in stash_working_tree()
     
         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
    -    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.
    +    the code.
     
         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.
     
    -    The call to reset_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().
    +    With this commit, reset_tree() does not need to be called at the
    +    beginning of stash_working_tree(), because it is only called by
    +    do_create_stash(), which sets the index at `i_tree', and
    +    save_untracked_files() does not change the main index.  But it will
    +    become useful again in a later commit, when save_untracked_file() will
    +    be rewritten to use the "main" index, so I did not remove it.
     
         At the end of the function, the tree is reset to `i_tree'.
     
3:  1ae236daf4 = 3:  b5587c0d56 stash: remove the second index in stash_patch()
4:  8c901ddf9d ! 4:  5fbcddf899 stash: remove the second index in save_untracked_files()
    @@ -8,7 +8,7 @@
         containing only untracked files, so the tree is reset to the empty tree
         at the beginning (hence the need for reset_tree() to accept constant
         trees).  The environment of `update-index' is no longer updated, and the
    -    index is discarded after this command exited.
    +    index is discarded after this command exits.
     
         As the only caller of this function is do_create_stash(), and the next
         user of the index is either save_untracked_files() or stash_patch(),
5:  413be5b5f9 ! 5:  0338986339 stash: remove the second index in restore_untracked()
    @@ -42,7 +42,7 @@
      		return -1;
     -	}
      
    - 	child_process_init(&cp);
    +-	child_process_init(&cp);
      	cp.git_cmd = 1;
      	argv_array_pushl(&cp.args, "checkout-index", "--all", NULL);
     -	argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s",
6:  b2c08c189e = 6:  4f151f4600 stash: remove `stash_index_path'
-- 
2.20.1


  parent reply index

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-05 10:48 [RFC PATCH v1 " 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
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 ` Alban Gruin [this message]
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=20200630151558.20975-1-alban.gruin@gmail.com \
    --to=alban.gruin@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sluongng@gmail.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