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

Re: [PATCH v10 0/9] submodule inline diff format

From
Jacob Keller <jacob.keller@gmail.com>
Date
Aug 25, 2016, 20:39 UTC
Message-ID
<CA+P7+xrQy33gt8bnsagLVPAHhZPZx-3s0E_Aj7tOwXnHjOoCBg@mail.gmail.com>
In-Reply-To
<CAGZ79kbsq52Qh0Jtn5hpPPfHu_9r2CUHvAHOTh6Cgz8ODkzreg@mail.gmail.com>
On Tue, Aug 23, 2016 at 10:47 AM, Stefan Beller <sbeller@google.com> wrote:
Show 15 quoted lines
> On Tue, Aug 23, 2016 at 10:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> I am not so sure about that.  If there is an existing place that is
>> buggy, shouldn't we fix that, instead of spreading the same bug
>> (assuming that it is a bug in the first place, which I do not have a
>> strong opinion on, at least not yet)?
>>
>> Can there be .git/modules/<foo>/ repository that is pointed at an
>> in-tree .git file when there is no "name" defined?
>
> If you're holding it wrong we can come into that state.
> * Checkout the submodule,
> * then remove .gitmodules as well as relevant config in .git/config.
> Result: Then we have a only a gitlink recorded as well as connected
> working tree to a gitdir inside a superprojects .git/modules/.
>

Yea, but I think you're right that we shouldn't support that state. What I'm worried about is the case where we can get this state doing something sane/acceptable but I don't think we can.

So we should support the gitlink to a repository stored at <path> without stuff inside the .git/modules, and we should support submodule gitlinks with a proper .gitmodules setup. I don't think we should die() but we should error properly so I will introduce a _gently() variant of these functions, and die properly in the regular flow.

Show 15 quoted lines
>> I thought we
>> errored out in module_name helper function in git-submodule.sh when
>> we need a name and only have path (I just checked in the maint-2.6
>> track); did we break it recently? submodule--helper.c::module_name()
>> seems to error out when submodule_from_path() fails to find one and
>> will segfault if it does not have name, so it is not likely.
>
> The name is literally the only thing that is not optional in a struct submodule
> (see submodule-config.c:182 In lookup_or_create_by_name, these structs are
> added to the internal cache.
>
> Stepping back a bit, I think we'd want to document this expectation more
> in the man pages
>     The name unlike the path of a submodule must not be changed (as the
>     name is used internally to point at the submodules git dir)

Agreed, this should be documented. Do you mind doing the documentation patch for this?

Thanks, Jake

Previous: Stefan BellerNext: Stefan Beller
Message 19 of 26 in “submodule inline diff format”
  1. 0/9 submodule inline diff formatJacob Keller, Aug 22, 2016
  2. 1/9 Git 2.10-rc1Jacob Keller, Aug 22, 2016
  3. Stefan BellerAug 23, 2016
  4. Jacob KellerAug 23, 2016
  5. 7/9 submodule: convert show_submodule_summary to use struct object_id *Jacob Keller, Aug 22, 2016
  6. 3/9 diff.c: remove output_prefix_length fieldJacob Keller, Aug 22, 2016
  7. 6/9 allow do_submodule_path to work even if submodule isn't checked outJacob Keller, Aug 22, 2016
  8. 4/9 graph: add support for --line-prefix on all graph-aware outputJacob Keller, Aug 22, 2016
  9. 5/9 diff: prepare for additional submodule formatsJacob Keller, Aug 22, 2016
  10. 2/9 cache: add empty_tree_oid object and helper functionJacob Keller, Aug 22, 2016
  11. 9/9 diff: teach diff to display submodule difference with an inline diffJacob Keller, Aug 22, 2016
  12. 8/9 submodule: refactor show_submodule_summary with helper functionJacob Keller, Aug 22, 2016
  13. Junio C HamanoAug 23, 2016
  14. Jacob KellerAug 24, 2016
  15. Stefan BellerAug 23, 2016
  16. Jacob KellerAug 23, 2016
  17. Junio C HamanoAug 23, 2016
  18. Stefan BellerAug 23, 2016
  19. Jacob KellerAug 25, 2016
  20. Stefan BellerAug 25, 2016
  21. Jacob KellerAug 25, 2016
  22. Junio C HamanoAug 25, 2016
  23. Jacob KellerAug 25, 2016
  24. Stefan BellerAug 26, 2016
  25. Keller, Jacob EAug 26, 2016
  26. Jacob KellerAug 25, 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.