From: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
Date: Sat, 05 Sep 2026 10:01:14 GMT
Subject: Re: [GSoC Patch v5 2/7] repo: add path.superproject-root with absolute and relative suffixes
Message-ID: <CA+rGoLf-BMh_BNUwi4k7NCLakbxP8vTuLbQKctX2K6MTK5b1yA@mail.gmail.com>
In-Reply-To: <xmqqh5k43bzo.fsf@gitster.g>

On Sat, Sep 5, 2026 at 3:39 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:
>
> > +static int get_path_superproject_absolute(struct repository *repo UNUSED, struct strbuf *buf)
> > +{
> > +     struct strbuf superproject = STRBUF_INIT;
> > +
> > +     if (!get_superproject_working_tree(&superproject)) {
> > +             strbuf_release(&superproject);
> > +             return 0;
> > +     }
> > +
> > +     format_path(buf, superproject.buf, "", PATH_FORMAT_CANONICAL);
> > +     strbuf_release(&superproject);
> > +     return 0;
> > +}
> > +
> > +static int get_path_superproject_relative(struct repository *repo, struct strbuf *buf)
> > +{
> > +     struct strbuf superproject = STRBUF_INIT;
> > +
> > +     if (!get_superproject_working_tree(&superproject)) {
> > +             strbuf_release(&superproject);
> > +             return 0;
> > +     }
>
> Here get_superproject_working_tree() does not care what repository
> we are working on.  Shouldn't it be updated to take "repo" as a
> parameter?
>
> Since it begins like this:
>
>     int get_superproject_working_tree(struct strbuf *buf)
>     {
>             struct child_process cp = CHILD_PROCESS_INIT;
>             struct strbuf sb = STRBUF_INIT;
>             struct strbuf one_up = STRBUF_INIT;
>             char *cwd = xgetcwd();
>             int ret = 0;
>
> I suspect that it based its decision on where you happen to be.
>
> It means that when I have a checkout of "git", with a submodule
> "sha1collisiondetection" at its top level already populated, in,
> say, /var/tmp/x/ directory, the following happens.
>
>     $ cd /var/tmp/x/git
>     $ git repo info path.gitdir.absolute
>     path.gitdir.absolute=/var/tmp/x/git/.git
>     $ git -C sha1collisiondetection repo info path.gitdir.absolute
>     path.gitdir.absolute=/var/tmp/x/git/.git/modules/sha1collisiondetection
>
>     $ D=/var/tmp/x/git/.git/modules/sha1collisiondetection
>     $ git --git-dir="$D" repo info path.superproject-root.absolute
>     path.superproject-root.absolute=
>     $ git -C sha1collisiondetection repo info path.superproject-root.absolute
>     path.superproject-root.absolute=/var/tmp/x/git
>
> The last two ought to match, but only the latter works correctly.
>
> Before this series starts reporting path.superproject-root,
> get_superproject_working_tree() needs to be corrected to work on the
> repository in question (instead of relying on where the process
> happens to be), no?

Yes, precisely.
I didn't think of it that way.

`get_superproject_working_tree()` currently relies on xgetcwd() rather
than inspecting the target repository, which causes explicit repository
contexts like --git-dir to break.

I will update `get_superproject_working_tree()` to take repo
and update its callers accordingly, along with adding test coverage for
--git-dir in v6.

Regards,
- K Jayatheerth