Hi Christian, Le 13/06/2020 à 10:52, Christian Couder a écrit : > On Tue, May 5, 2020 at 12:56 PM Alban Gruin <alban.gruin@xxxxxxxxx> 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? > No, I meant that 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 down the line, when save_untracked_file() will be rewritten to use the "main" index, so I did not remove it. I hope it makes more sense now. >> 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'. Alban
