[PATCH v2 0/6] stash: drop usage of a second index
To
git@vger.kernel.org
Cc
Christian Couder
Thomas Gummerer
Junio C Hamano
Son Luong Ngoc
Alban Gruin
From
Alban Gruin
See Also
Prev
Date
2020-06-30 15:15:52 UTC
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