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

Re: [RFC PATCH v1 0/6] stash: drop usage of a second index

From
Alban Gruin <alban.gruin@gmail.com>
Date
Jun 25, 2020, 12:35 UTC
Message-ID
<4b73490a-c2b9-8ed7-8d58-998ac071292c@gmail.com>
In-Reply-To
<CAP8UFD0bfTEDQaA0rQEBW76niF0T7f_4HS_N1tkRPh-0ZW7-Gw@mail.gmail.com>
Hi Christian,
Le 13/06/2020 à 09:52, Christian Couder a écrit :
Show 43 quoted lines
> Hi,
> 
> On Tue, May 5, 2020 at 12:56 PM Alban Gruin <alban.gruin@gmail.com> wrote:
>>
>> 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].
> 
> Thanks for working on this! I agree that it would be nice to fix split
> index issues as it could help for sure with huge repositories. Sorry
> also that this patch series fell through the cracks.
> 
> I am adding Son Luong Ngoc in Cc as he reported the issue that this
> series fixes.
> 
>> 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),
> 
> It should have worked when it was introduced, though maybe not for `git stash`.
> 
>> 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.
> 
> I understand. I think the good way forward would be to fix
> GIT_TEST_SPLIT_INDEX and find a way to ensure that it keeps working in
> the future.
> 
> Thanks,
> Christian.
> 
Thank you for your feedback.

I'll resend this series with the changes you and Son suggested, but I think I'm going to remove references to bugs in the commit messages, to turn it into another cleanup series. For the index, I will to try to implement Gábor's suggestions in another series.

Thanks, Alban

Previous: Christian CouderNext: SZEDER Gábor
Message 18 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.