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

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

From
Duy Nguyen <pclouds@gmail.com>
Date
Sep 11, 2013, 11:20 UTC
Message-ID
<CACsJy8BQWSthAKePsBDyc5nAofLiTpXO5oc+jbSGx33YBioTeg@mail.gmail.com>
In-Reply-To
<20130911110806.GT2582@serenity.lan>
On Wed, Sep 11, 2013 at 6:08 PM, John Keeping <john@keeping.me.uk> wrote:
Show 39 quoted lines
>> > -- >8 --
>> > diff --git a/builtin/reset.c b/builtin/reset.c
>> > index 9efac0f..800117f 100644
>> > --- a/builtin/reset.c
>> > +++ b/builtin/reset.c
>> > @@ -143,7 +143,6 @@ static int read_from_tree(const struct pathspec *pathspec,
>> >         opt.output_format = DIFF_FORMAT_CALLBACK;
>> >         opt.format_callback = update_index_from_diff;
>> >
>> > -       read_cache();
>> >         if (do_diff_cache(tree_sha1, &opt))
>> >                 return 1;
>> >         diffcore_std(&opt);
>> > @@ -169,7 +168,7 @@ static void set_reflog_message(struct strbuf *sb, const char *action,
>> >
>> >  static void die_if_unmerged_cache(int reset_type)
>> >  {
>> > -       if (is_merge() || read_cache() < 0 || unmerged_cache())
>> > +       if (is_merge() || unmerged_cache())
>> >                 die(_("Cannot do a %s reset in the middle of a merge."),
>> >                     _(reset_type_names[reset_type]));
>>
>> 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. 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.
>
> read_cache() already has an early return if the index is already loaded
> so I don't think we need to worry about a special function for that.
>
> I'm not sure it's worth optimizing this case too heavily, but it might
> be a nice change to make parse_pathspec() not rely on the index being
> loaded before it is called with certain flags.

Yeah I ddin't check. I agree putting read_cache() in _CHEAP code sounds nice. We won't need to worry about forgotten read_cache() elsewhere.

-- 
Duy
Previous: John KeepingNext: Junio C Hamano
Message 9 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.