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

Re: [PATCH v2 1/4] rev-parse: fix some options when executed from subpath of main tree

From
Eric Sunshine <sunshine@sunshineco.com>
Date
May 10, 2016, 07:49 UTC
Message-ID
<CAPig+cSZ9PXr4Wogt2EJAJyapso8hZVucQ2yvJpCRK35a2j8jg@mail.gmail.com>
In-Reply-To
<20160506161312.Horde.7i8_sE5ISIqccneOIfinvCX@webmail.informatik.kit.edu>
On Fri, May 6, 2016 at 10:13 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
Show 29 quoted lines
> Quoting Mike Rappazzo <rappazzo@gmail.com>:
>> I'll try to reword this to make it indicate that the value isn't
>> always incorrect.
>
> Not sure I understand your intention about rewording, in particular that
> "isn't always incorrect" part.  Just to make sure there is no
> misunderstanding let's have a look at the two broken cases:
>
>    $ git -C t/ rev-parse --git-common-dir
>    t/.git
>
> Obviously wrong.
> This is what you correctly described in the commit message as
> "incorrectly returns a path which starts 'sub/path/.git'.
>
>    $ git -C t/ rev-parse --git-path objects
>    .git/objects
>
> Wrong as well.  It would be correct if we were at the top of the working
> tree, but we are not.  It must be relative to the directory where '-C t/'
> brought us.
> Your description in the commit message implies that '--git-path <path>'
> is wrong in the same way as '--git-common-dir' is, i.e. in this case
> would also return the relative path starting with the subdirectory.
> That is apparently not the case.
>
> My point in the previous email was that both are wrong when executed in
> a subdir of the working tree, but they are wrong in different ways.  And
> they are always wrong when executed from a subdir of the working tree.

This explanation argues strongly in favor of placing each fix in a separate patch[1] so that each commit message can be tailored to precisely match the problem being fixed. Bundling the corresponding tests with the fix would be a bonus; no need for a lead-in patch introducing the tests en masse, and no need for test_expect_failure.

[1]: http://article.gmane.org/gmane.comp.version-control.git/293001/
Previous: Junio C Hamano
Message 12 of 12 in “rev-parse: adjust results when they should be relative”
  1. 0/4 rev-parse: adjust results when they should be relativeMichael Rappazzo, Apr 22, 2016
  2. 1/4 rev-parse: fix some options when executed from subpath of main treeMichael Rappazzo, Apr 22, 2016
  3. SZEDER GáborApr 29, 2016
  4. 2/4 t1500-rev-parse: add tests executed from sub path of the main worktreeMichael Rappazzo, Apr 22, 2016
  5. SZEDER GáborApr 29, 2016
  6. 3/4 t2027-worktree-list: add and adjust tests related to git-rev-parseMichael Rappazzo, Apr 22, 2016
  7. SZEDER GáborApr 29, 2016
  8. 4/4 t1700-split-index: add test for rev-parse --shared-index-pathMichael Rappazzo, Apr 22, 2016
  9. Mike RappazzoMay 6, 2016
  10. SZEDER GáborMay 6, 2016
  11. Junio C HamanoMay 6, 2016
  12. Eric SunshineMay 10, 2016

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.