git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "SZEDER Gábor" <szeder@ira.uka.de>
To: Mike Rappazzo <rappazzo@gmail.com>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Nguyễn Thái Ngọc" <pclouds@gmail.com>,
	"Git List" <git@vger.kernel.org>
Subject: Re: [PATCH v2 1/4] rev-parse: fix some options when executed from subpath of main tree
Date: Fri, 06 May 2016 16:13:12 +0200	[thread overview]
Message-ID: <20160506161312.Horde.7i8_sE5ISIqccneOIfinvCX@webmail.informatik.kit.edu> (raw)
In-Reply-To: <CANoM8SWBzFiHGc3zAwndyx+GUkWQGDoaewVVQtH-06jazAn8uQ@mail.gmail.com>


Quoting Mike Rappazzo <rappazzo@gmail.com>:

> On Fri, Apr 29, 2016 at 9:50 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>>> Executing `git-rev-parse` with `--git-common-dir`, `--git-path <path>`,
>>> or `--shared-index-path` from the root of the main worktree results in
>>> a relative path to the git dir.
>>>
>>> When executed from a subdirectory of the main tree, however, it incorrectly
>>> returns a path which starts 'sub/path/.git'.
>>
>> This is not completely true, because '--git-path ...' returns a
>> relative path starting with '.git':
>>
>>  $ git -C t/ rev-parse --git-dir --git-path objects --git-common-dir
>>  /home/szeder/src/git/.git
>>  .git/objects
>>  t/.git
>>
>> It's still wrong, of course.
>
> 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.
I would have described the current wrong behavior as:

    When executed from a subdirectory of the working tree, however,
    '--git-common-dir' incorrectly returns a path which starts
    'sub/path/.git', while '--git-path <path>' incorrectly returns a path
    relative to the top of the working tree.

(Still hasn't looked at shared index...)

>>> Change this to return the
>>> proper relative path to the git directory.
>>
>> I think returning absolute paths would be better.  It is consistent
>> with the already properly working '--git-dir' option, which returns an
>> absolute path in this case.  Furthermore, both '--git-path ...' and
>> '--git-common-dir' already return absolute paths when run from a
>> subdirectory of the .git directory:
>>
>>  $ git -C .git/refs rev-parse --git-dir --git-path objects --git-common-dir
>>  /home/szeder/src/git/.git
>>  /home/szeder/src/git/.git/objects
>>  /home/szeder/src/git/.git
>>
>
> I agree with this, but at one point Junio suggested that it should
> return the relative path[1],

I wasn't aware of Junio's suggestion.

It shouldn't really matter in practice, because both the absolute and
relative paths will ultimately lead to the same place.  However, I
still think that for consistency's sake absolute paths would be better
when executed in a subdir of the working tree.

  reply	other threads:[~2016-05-06 14:13 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-22 21:53 [PATCH v2 0/4] rev-parse: adjust results when they should be relative Michael Rappazzo
2016-04-22 21:53 ` [PATCH v2 1/4] rev-parse: fix some options when executed from subpath of main tree Michael Rappazzo
2016-04-29 14:21   ` SZEDER Gábor
     [not found]   ` <20160429135051.15492-1-szeder@ira.uka.de>
2016-05-06 13:02     ` Mike Rappazzo
2016-05-06 14:13       ` SZEDER Gábor [this message]
2016-05-06 17:08         ` Junio C Hamano
2016-05-10  7:49         ` Eric Sunshine
2016-04-22 21:53 ` [PATCH v2 2/4] t1500-rev-parse: add tests executed from sub path of the main worktree Michael Rappazzo
2016-04-29 14:22   ` SZEDER Gábor
2016-04-22 21:53 ` [PATCH v2 3/4] t2027-worktree-list: add and adjust tests related to git-rev-parse Michael Rappazzo
2016-04-29 14:22   ` SZEDER Gábor
2016-04-22 21:53 ` [PATCH v2 4/4] t1700-split-index: add test for rev-parse --shared-index-path Michael Rappazzo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160506161312.Horde.7i8_sE5ISIqccneOIfinvCX@webmail.informatik.kit.edu \
    --to=szeder@ira.uka.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=rappazzo@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).