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

Re: [PATCH 1/2] reset: handle submodule with trailing slash

From
John Keeping <john@keeping.me.uk>
Date
Sep 11, 2013, 18:22 UTC
Message-ID
<20130911182233.GW2582@serenity.lan>
In-Reply-To
<xmqq8uz3tece.fsf@gitster.dls.corp.google.com>
On Wed, Sep 11, 2013 at 11:14:57AM -0700, Junio C Hamano wrote:
Show 21 quoted lines
> John Keeping <john@keeping.me.uk> writes:
> 
> > On Wed, Sep 11, 2013 at 10:08:18AM -0700, Junio C Hamano wrote:
> >> Duy Nguyen <pclouds@gmail.com> writes:
> >> 
> >> > reset --soft does not go through these code paths (i.e. it does not
> >> > need index at all). If we fail to load index index in "reset --soft" I
> >> > think it's ok to die(). Corrupt index is fatal anyway.
> >> 
> >> Do I smell a breakage here?  Isn't "reset --soft HEAD" (or some
> >> known good commit) a way to recover from a corrupt index?
> >> 
> >> If that is the case, I do not think it is OK at all.  What do we
> >> suggest as an alternative?  "rm .git/index && read-tree"?
> >
> > Duy's suggestion below is necessary to avoid this then I think - we'll
> > die if the user has a corrupt index and gives a path with a trailing
> > slash, but without that path we won't try to load the index.
> 
> Sorry, but I don't quite follow.  Isn't "git reset --soft <path>" a
> nonsense, with or without a trailing slash at the end of <path>?

Yes, but my point was that providing the user doesn't give a path with a trailing slash we will get past the option parser if we take the approach below.

However, I think we do do a read_cache when using "reset --soft" since we go through builtin/reset.c::die_if_unmerged_cache() which dies if read_cache fails. So I don't think we are losing anything by moving this check earlier.

Show 8 quoted lines
> >> > But "reset
> >> > --soft" now has to pay the cost to load index, which could be slow
> >> > when the index is big. Assuming nobody does "reset --soft" that often
> >> > I think this is OK.
> >> >
> >> > Alternatively we could load index lazily in _CHEAP code only when we
> >> > see trailing slashes, then replace these read_cache() with
> >> > read_cache_unless_its_already_loaded_earlier() or something.
Previous: Junio C HamanoNext: Junio C Hamano
Message 13 of 27 in “reset: handle submodule with trailing slash”
  1. 0/2 reset: handle submodule with trailing slashJohn Keeping, Sep 10, 2013
  2. 1/2 reset: handle submodule with trailing slashJohn Keeping, Sep 10, 2013
  3. Jens LehmannSep 10, 2013
  4. John KeepingSep 10, 2013
  5. Johannes SixtSep 11, 2013
  6. John KeepingSep 11, 2013
  7. Duy NguyenSep 11, 2013
  8. John KeepingSep 11, 2013
  9. Duy NguyenSep 11, 2013
  10. Junio C HamanoSep 11, 2013
  11. John KeepingSep 11, 2013
  12. Junio C HamanoSep 11, 2013
  13. John KeepingSep 11, 2013
  14. Junio C HamanoSep 11, 2013
  15. 2/2 rm: re-use parse_pathspec's trailing-slash removalJohn Keeping, Sep 10, 2013
  16. Duy NguyenSep 11, 2013
  17. John KeepingSep 11, 2013
  18. 0/4 submodule trailing slash improvementsJohn Keeping, Sep 12, 2013
  19. 1/4 pathspec: use is_dir_sep() to check for trailing slashesJohn Keeping, Sep 12, 2013
  20. Johannes SixtSep 12, 2013
  21. 2/4 pathspec: strip multiple trailing slashes from submodulesJohn Keeping, Sep 12, 2013
  22. Junio C HamanoSep 12, 2013
  23. John KeepingSep 12, 2013
  24. Duy NguyenSep 13, 2013
  25. John KeepingSep 13, 2013
  26. 3/4 rm: re-use parse_pathspec's trailing-slash removalJohn Keeping, Sep 12, 2013
  27. 4/4 reset: handle submodule with trailing slashJohn Keeping, Sep 12, 2013

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.