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

Re: [PATCH v7 4/7] submodule: allow do_submodule_path to work if given gitdir directly

From
Jacob Keller <jacob.keller@gmail.com>
Date
Aug 18, 2016, 18:50 UTC
Message-ID
<CA+P7+xoz=2hzMtX7e79Zpsza5jLAtWMo-g9H9mcwNVd9=BBQKA@mail.gmail.com>
In-Reply-To
<xmqq37m17w4y.fsf@gitster.mtv.corp.google.com>
On Thu, Aug 18, 2016 at 11:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 31 quoted lines
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Currently, do_submodule_path relies on read_gitfile, which will die() if
>> it can't read from the specified gitfile. Unfortunately, this means that
>> do_submodule_path will not work when given the path to a submodule which
>> is checked out directly, such as a newly added submodule which you
>> cloned and then "git submodule add".
>
> Hmm, are you sure about that?
>
> A call to read_gitfile() turns into a call to read_gitfile_gently()
> with the return_error_code parameter set to NULL.  The function does
> a stat(2), and if the given path is not a file (e.g. we created the
> submodule working tree and repository in-place ourselves, instead of
> cloning an existing project from elsewhere, in which case we would
> see a directory there), it says READ_GIT_FILE_ERR_NOT_A_FILE and
> returns NULL, because that is not a fatal error condition.  The same
> thing happens if path does not yet exist.
>
> This caller is given <path>, prepares "<path>/.git" in buf, and
> calls read_gitfile().  If it returns a non-NULL, it replaces what is
> in buf and continues, but if it returns a NULL (i.e. the two cases I
> mentioned in the above paragraph), it continues with "<path>/.git".
>
> While I do not think changing it to resolve_gitdir() is wrong per-se,
> I am not sure if it is necessary.
>
> I must be misreading something in the existing code.
>

No I think you're correct. I was under the assumption that it would die here. I think it's probably better to stick with read_gitfile() here, it should work. The main issue is what happens after this needs to be fixed.

I'll investigate this more.

Thanks, Jake

Previous: Junio C HamanoNext: Stefan Beller
Message 22 of 24 in “implement inline submodule diff format”
  1. 0/7 implement inline submodule diff formatJacob Keller, Aug 18, 2016
  2. 1/7 diff.c: remove output_prefix_length fieldJacob Keller, Aug 18, 2016
  3. 6/7 submodule: refactor show_submodule_summary with helper functionJacob Keller, Aug 18, 2016
  4. David AguilarAug 18, 2016
  5. Jacob KellerAug 18, 2016
  6. Stefan BellerAug 18, 2016
  7. Jacob KellerAug 18, 2016
  8. Stefan BellerAug 18, 2016
  9. Jacob KellerAug 18, 2016
  10. Junio C HamanoAug 18, 2016
  11. Jacob KellerAug 18, 2016
  12. 7/7 diff: teach diff to display submodule difference with an inline diffJacob Keller, Aug 18, 2016
  13. Stefan BellerAug 18, 2016
  14. Jacob KellerAug 18, 2016
  15. 3/7 diff: prepare for additional submodule formatsJacob Keller, Aug 18, 2016
  16. Stefan BellerAug 18, 2016
  17. 5/7 submodule: correct output of submodule log formatJacob Keller, Aug 18, 2016
  18. Stefan BellerAug 18, 2016
  19. Jacob KellerAug 18, 2016
  20. 4/7 submodule: allow do_submodule_path to work if given gitdir directlyJacob Keller, Aug 18, 2016
  21. Junio C HamanoAug 18, 2016
  22. Jacob KellerAug 18, 2016
  23. Stefan BellerAug 18, 2016
  24. 2/7 graph: add support for --line-prefix on all graph-aware outputJacob Keller, Aug 18, 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.