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

Re: [PATCH v2 2/4] pathspec: strip multiple trailing slashes from submodules

From
Duy Nguyen <pclouds@gmail.com>
Date
Sep 13, 2013, 01:28 UTC
Message-ID
<CACsJy8DX_v9r0QYQMH=697LGeGsCaE80oCo0ppd2LFZk-Os+TA@mail.gmail.com>
In-Reply-To
<20130912202128.GB2582@serenity.lan>
On Fri, Sep 13, 2013 at 3:21 AM, John Keeping <john@keeping.me.uk> wrote:
Show 23 quoted lines
> On Thu, Sep 12, 2013 at 12:48:10PM -0700, Junio C Hamano wrote:
>> John Keeping <john@keeping.me.uk> writes:
>>
>> > This allows us to replace the submodule path trailing slash removal in
>> > builtin/rm.c with the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag to
>> > parse_pathspec() without changing the behaviour with respect to multiple
>> > trailing slashes.
>>
>> Where does prefix_pathspec()'s input, which could have an unwanted
>> trailing slash, come from?
>>
>> If it is read from some of our internal data structure and known to
>> have at most one, then this change makes me feel very uneasy to cope
>> with potentially sloppy end-user input and data generated by ourselves
>> with the same logic.  It will allow our internal to be sloppy without
>> forcing us notice and fix that sloppiness.
>>
>> If it is coming from an end-user input, then I would not object to
>> the change, though.
>
> I added this in response to Duy's comment on v1 [1].
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/234548

Looks like I add more noise to this thread than anything valuable. Yes, once argv goes through parse_pathspec it's normalized so we do not need to strip consecutive slashes any more. I'm not entirely sure if it also converts Windows '\\' to '/'..

Show 7 quoted lines
> Looking more closely, this does come from user input (via the argv
> passed into parse_pathspec) but does (some of the time) go through
> prefix_path_gently which calls normalize_path_copy_len.
>
> It's not immediately clear to me when prefix_pathspec goes through this
> particular code path, but I think we may be able to drop this (and the
> previous patch) without affecting the user.
-- 
Duy
Previous: John KeepingNext: John Keeping
Message 24 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.