git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/6] stash: drop usage of a second index

From
Alban Gruin <alban.gruin@gmail.com>
Date
Jun 30, 2020, 15:15 UTC
Message-ID
<20200630151558.20975-1-alban.gruin@gmail.com>
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
Previous: Alban GruinNext: Alban Gruin
Message 24 of 44 in “stash: drop usage of a second index”
  1. 0/6 stash: drop usage of a second indexAlban Gruin, May 5, 2020
  2. 2/6 stash: remove the second index in stash_working_tree()Alban Gruin, May 5, 2020
  3. Christian CouderJun 13, 2020
  4. Alban GruinJun 13, 2020
  5. Christian CouderJun 15, 2020
  6. 1/6 stash: mark `i_tree' in reset_tree() constAlban Gruin, May 5, 2020
  7. Christian CouderJun 13, 2020
  8. 4/6 stash: remove the second index in save_untracked_files()Alban Gruin, May 5, 2020
  9. Christian CouderJun 13, 2020
  10. 5/6 stash: remove the second index in restore_untracked()Alban Gruin, May 5, 2020
  11. Christian CouderJun 13, 2020
  12. 3/6 stash: remove the second index in stash_patch()Alban Gruin, May 5, 2020
  13. Christian CouderJun 13, 2020
  14. Christian CouderJun 13, 2020
  15. 6/6 stash: remove `stash_index_path'Alban Gruin, May 5, 2020
  16. Alban GruinJun 4, 2020
  17. Christian CouderJun 13, 2020
  18. Alban GruinJun 25, 2020
  19. SZEDER GáborJun 15, 2020
  20. SZEDER GáborJun 15, 2020
  21. SZEDER GáborJun 16, 2020
  22. Junio C HamanoJun 17, 2020
  23. Alban GruinJun 17, 2020
  24. 0/6 stash: drop usage of a second indexAlban Gruin, Jun 30, 2020
  25. 1/6 stash: mark `i_tree' in reset_tree() constAlban Gruin, Jun 30, 2020
  26. 2/6 stash: remove the second index in stash_working_tree()Alban Gruin, Jun 30, 2020
  27. 4/6 stash: remove the second index in save_untracked_files()Alban Gruin, Jun 30, 2020
  28. 3/6 stash: remove the second index in stash_patch()Alban Gruin, Jun 30, 2020
  29. 5/6 stash: remove the second index in restore_untracked()Alban Gruin, Jun 30, 2020
  30. Christian CouderJul 31, 2020
  31. Alban GruinJul 31, 2020
  32. 6/6 stash: remove `stash_index_path'Alban Gruin, Jun 30, 2020
  33. Christian CouderJul 31, 2020
  34. 0/6 stash: drop usage of a second indexAlban Gruin, Jul 31, 2020
  35. 1/6 stash: mark `i_tree' in reset_tree() constAlban Gruin, Jul 31, 2020
  36. Junio C HamanoJul 31, 2020
  37. 2/6 stash: remove the second index in stash_working_tree()Alban Gruin, Jul 31, 2020
  38. Junio C HamanoJul 31, 2020
  39. Junio C HamanoAug 2, 2020
  40. 4/6 stash: remove the second index in save_untracked_files()Alban Gruin, Jul 31, 2020
  41. 5/6 stash: remove the second index in restore_untracked()Alban Gruin, Jul 31, 2020
  42. 3/6 stash: remove the second index in stash_patch()Alban Gruin, Jul 31, 2020
  43. 6/6 stash: remove `stash_index_path'Alban Gruin, Jul 31, 2020
  44. Junio C HamanoJul 31, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.