git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Adrian Ratiu <adrian.ratiu@collabora.com>
To: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Emily Shaffer <emilyshaffer@google.com>,
	Rodrigo Damazio Bovendorp <rdamazio@google.com>,
	Aaron Schrab <aaron@schrab.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Patrick Steinhardt <ps@pks.im>,
	Josh Steadmon <steadmon@google.com>,
	Ben Knoble <ben.knoble@gmail.com>,
	Phillip Wood <phillip.wood123@gmail.com>
Subject: Re: [PATCH v3 5/5] submodule: error out if gitdir name is too long
Date: Tue, 07 Oct 2025 13:17:58 +0300	[thread overview]
Message-ID: <87jz173tih.fsf@collabora.com> (raw)
In-Reply-To: <xmqqy0poot7g.fsf@gitster.g>

On Mon, 06 Oct 2025, Junio C Hamano <gitster@pobox.com> wrote:
> Adrian Ratiu <adrian.ratiu@collabora.com> writes: 
> 
>> Encoding submodule names increases their name size, so there is 
>> an increased risk to hit the max filename length in the gitdir 
>> path.  (the likelihood is still rather small, so it's an 
>> acceptable risk) 
> 
> If it is acceptable, can we ignore it? 

Yes, we can.

Peff actually asked if it's worth pursuing this path. We can 
certainly ignore and let the OS fail like it does now, without the 
compat wrapper.

The only usefulness of this commit as-is is to to print a nicer 
error msg and allow us to eventually address the patch TODO 
(sharding/trimming).

Please let me know if you wish to drop this commit in v4. I am 
perfectly fine with removing it. :)

> 
> Just stepping back a bit, how are we keeping track of the 
> mapping between submodule names vs locations in .git/modules/? 
> Don't we always go through that mapping and would a half-clever 
> code that says "heh, that is url encoded and I know how to 
> decode it" and bypass the mapping a bug?

The short answer is no. :) The slightly longer answer is:

Before v3, we used to have a clear filesystem separation:
.git/modules were unencoded
.git/submodules were encoded

After v3, when the extension is enabled, we encode all submodule 
names with one exception: if they already exist, at which point 
they are treated as unencoded.
 
> 
> If we keep track of the mapping ourselves, then the names under 
> .git/modules/ do not have to be "decodable" by themselves.  They 
> can even be sequence numbers and that would not hit any maximum 
> filename length before you fill your disk.

IIRC the consensus was to avoid keeping such a mapping due to 
complexity, risk of desynchronizing with the filesystem layout and 
so on, so simple rules like the above, or even a simple hash_name 
might be better (see below).
 
> 
> No, no I am not suggesting to use sequence numbers; something 
> remotely readable by humans is better.  But my point is that 
> just like you have to make sure that the encoded name you give 
> to a new thing does not collide with existing names (you know 
> with "ls .git/modules/" what names are taken), you can notice 
> your mkdir() would not error with name-too-long, truncate and 
> twiddle with suffix to make it unique and retry, without giving 
> a failure to the end user. 

Everything is on the table and I have no strong opinions. :)

Someone suggested we use something like .git/modules/<hash>_<name> 
when the extension is enabled, so we have a simple, unique and 
recognizable pattern.

What do you think of that?

  reply	other threads:[~2025-10-07 10:18 UTC|newest]

Thread overview: 179+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-16 21:36 [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
2025-08-16 21:36 ` [PATCH 1/9] submodule--helper: use submodule_name_to_gitdir in add_submodule Adrian Ratiu
2025-08-20 19:04   ` Josh Steadmon
2025-08-21 11:26     ` Adrian Ratiu
2025-08-16 21:36 ` [PATCH 2/9] submodule: create new gitdirs under submodules path Adrian Ratiu
2025-09-08 14:24   ` Phillip Wood
2025-09-08 15:46     ` Adrian Ratiu
2025-09-09  8:53       ` Phillip Wood
2025-09-09 10:57         ` Adrian Ratiu
2025-08-16 21:36 ` [PATCH 3/9] submodule: add gitdir path config override Adrian Ratiu
2025-08-20 19:37   ` Josh Steadmon
2025-08-21 12:18     ` Adrian Ratiu
2025-08-20 21:38   ` Josh Steadmon
2025-08-21 13:04     ` Adrian Ratiu
2025-08-20 21:50   ` Josh Steadmon
2025-08-21 13:05     ` Adrian Ratiu
2025-09-08 14:23   ` Phillip Wood
2025-09-09 12:02     ` Adrian Ratiu
2025-08-16 21:36 ` [PATCH 4/9] t: submodules: add basic mixed gitdir path tests Adrian Ratiu
2025-08-20 22:07   ` Josh Steadmon
2025-09-02 23:02   ` Junio C Hamano
2025-08-16 21:36 ` [PATCH 5/9] strbuf: bring back is_rfc3986_unreserved Adrian Ratiu
2025-08-16 21:56   ` Ben Knoble
2025-08-21 13:08     ` Adrian Ratiu
2025-08-16 21:36 ` [PATCH 6/9] submodule: encode gitdir paths to avoid conflicts Adrian Ratiu
2025-08-20 19:29   ` Jeff King
2025-08-21 13:14     ` Adrian Ratiu
2025-08-16 21:36 ` [PATCH 7/9] submodule: remove validate_submodule_git_dir() Adrian Ratiu
2025-09-08 14:23   ` Phillip Wood
2025-08-16 21:36 ` [PATCH 8/9] t: move nested gitdir tests to proper location Adrian Ratiu
2025-08-16 21:36 ` [PATCH 9/9] t: add gitdir encoding tests Adrian Ratiu
2025-08-18 22:06   ` Junio C Hamano
2025-08-21 13:17     ` Adrian Ratiu
2025-08-17 13:01 ` [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
2025-09-08 14:01 ` [PATCH v2 00/10] " Adrian Ratiu
2025-09-08 14:01   ` [PATCH v2 01/10] submodule--helper: use submodule_name_to_gitdir in add_submodule Adrian Ratiu
2025-09-30 13:37     ` Kristoffer Haugsbakk
2025-09-08 14:01   ` [PATCH v2 02/10] submodule: create new gitdirs under submodules path Adrian Ratiu
2025-09-09  7:40     ` Patrick Steinhardt
2025-09-09 16:17       ` Adrian Ratiu
2025-09-08 14:01   ` [PATCH v2 03/10] submodule: add gitdir path config override Adrian Ratiu
2025-09-09  7:40     ` Patrick Steinhardt
2025-09-09 17:46       ` Adrian Ratiu
2025-09-08 14:01   ` [PATCH v2 04/10] t7425: add basic mixed submodule gitdir path tests Adrian Ratiu
2025-09-08 14:01   ` [PATCH v2 05/10] strbuf: bring back is_rfc3986_unreserved Adrian Ratiu
2025-09-08 14:01   ` [PATCH v2 06/10] submodule: encode gitdir paths to avoid conflicts Adrian Ratiu
2025-09-10 18:15     ` SZEDER Gábor
2025-09-10 19:30       ` Adrian Ratiu
2025-09-10 20:18     ` Kristoffer Haugsbakk
2025-09-30 13:36     ` Kristoffer Haugsbakk
2025-09-08 14:01   ` [PATCH v2 07/10] submodule: error out if gitdir name is too long Adrian Ratiu
2025-09-08 15:51     ` Jeff King
2025-09-08 17:15       ` Adrian Ratiu
2025-09-30 13:35     ` Kristoffer Haugsbakk
2025-09-08 14:01   ` [PATCH v2 08/10] submodule: remove validate_submodule_git_dir() Adrian Ratiu
2025-09-30 13:35     ` Kristoffer Haugsbakk
2025-10-03  7:56       ` Adrian Ratiu
2025-09-08 14:01   ` [PATCH v2 09/10] t7450: move nested gitdir tests to t7425 Adrian Ratiu
2025-09-08 14:01   ` [PATCH v2 10/10] t7425: add gitdir encoding tests Adrian Ratiu
2025-10-06 11:25 ` [PATCH v3 0/5] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
2025-10-06 11:25   ` [PATCH v3 1/5] submodule--helper: use submodule_name_to_gitdir in add_submodule Adrian Ratiu
2025-10-06 16:37     ` Junio C Hamano
2025-10-07  9:23       ` Adrian Ratiu
2025-10-06 11:25   ` [PATCH v3 2/5] submodule: add gitdir path config override Adrian Ratiu
2025-10-06 16:47     ` Junio C Hamano
2025-10-07 15:41       ` Junio C Hamano
2025-10-21  8:06         ` Patrick Steinhardt
2025-10-21 11:50           ` Adrian Ratiu
2025-10-21  8:05     ` Patrick Steinhardt
2025-10-21 11:57       ` Adrian Ratiu
2025-10-06 11:25   ` [PATCH v3 3/5] strbuf: bring back is_rfc3986_unreserved Adrian Ratiu
2025-10-06 16:51     ` Junio C Hamano
2025-10-06 17:47       ` Junio C Hamano
2025-10-07  9:43       ` Adrian Ratiu
2025-10-21  8:06     ` Patrick Steinhardt
2025-10-06 11:25   ` [PATCH v3 4/5] submodule: encode gitdir paths to avoid conflicts Adrian Ratiu
2025-10-06 16:57     ` Junio C Hamano
2025-10-07 14:10       ` Adrian Ratiu
2025-10-07 17:20         ` Junio C Hamano
2025-10-07 17:41           ` Adrian Ratiu
2025-10-07 19:55             ` Junio C Hamano
2025-10-06 11:25   ` [PATCH v3 5/5] submodule: error out if gitdir name is too long Adrian Ratiu
2025-10-06 17:06     ` Junio C Hamano
2025-10-07 10:17       ` Adrian Ratiu [this message]
2025-10-07 15:58         ` Junio C Hamano
2025-10-21  8:06     ` Patrick Steinhardt
2025-10-21 13:13       ` Adrian Ratiu
2025-10-06 16:21   ` [PATCH v3 0/5] Encode submodule gitdir names to avoid conflicts Junio C Hamano
2025-10-07 11:13     ` Adrian Ratiu
2025-10-07 15:36       ` Junio C Hamano
2025-10-07 16:58         ` Adrian Ratiu
2025-10-07 17:27         ` Junio C Hamano
2025-10-07 16:21       ` Junio C Hamano
2025-10-07 17:21         ` Adrian Ratiu
2025-11-07 15:05 ` [PATCH v4 0/4] " Adrian Ratiu
2025-11-07 15:05   ` [PATCH v4 1/4] submodule--helper: use submodule_name_to_gitdir in add_submodule Adrian Ratiu
2025-11-07 15:05   ` [PATCH v4 2/4] builtin/credential-store: move is_rfc3986_unreserved to url.[ch] Adrian Ratiu
2025-11-07 15:05   ` [PATCH v4 3/4] submodule: add extension to encode gitdir paths Adrian Ratiu
2025-11-07 15:05   ` [PATCH v4 4/4] submodule: fix case-folding gitdir filesystem colisions Adrian Ratiu
2025-11-08 18:20     ` Aaron Schrab
2025-11-10 17:11       ` Adrian Ratiu
2025-11-10 17:31         ` Aaron Schrab
2025-11-10 18:27           ` Adrian Ratiu
2025-11-10 19:10         ` Junio C Hamano
2025-11-10 23:01           ` Adrian Ratiu
2025-11-10 23:17             ` Junio C Hamano
2025-11-11 12:41               ` Adrian Ratiu
2025-11-12 15:28     ` Adrian Ratiu
2025-11-14 23:03   ` [PATCH v4 0/4] Encode submodule gitdir names to avoid conflicts Josh Steadmon
2025-11-17 15:22     ` Adrian Ratiu
2025-11-19 21:10 ` [PATCH v5 0/7] " Adrian Ratiu
2025-11-19 21:10   ` [PATCH v5 1/7] submodule--helper: use submodule_name_to_gitdir in add_submodule Adrian Ratiu
2025-11-19 21:10   ` [PATCH v5 2/7] builtin/credential-store: move is_rfc3986_unreserved to url.[ch] Adrian Ratiu
2025-12-05 12:16     ` Patrick Steinhardt
2025-12-05 17:25       ` Adrian Ratiu
2025-11-19 21:10   ` [PATCH v5 3/7] submodule: always validate gitdirs inside submodule_name_to_gitdir Adrian Ratiu
2025-12-05 12:17     ` Patrick Steinhardt
2025-12-05 18:17       ` Adrian Ratiu
2025-11-19 21:10   ` [PATCH v5 4/7] submodule: add extension to encode gitdir paths Adrian Ratiu
2025-12-05 12:19     ` Patrick Steinhardt
2025-12-05 19:30       ` Adrian Ratiu
2025-12-05 22:47         ` Junio C Hamano
2025-12-06 11:59           ` Patrick Steinhardt
2025-12-06 16:38             ` Junio C Hamano
2025-12-08  9:01               ` Adrian Ratiu
2025-12-08 11:46                 ` Patrick Steinhardt
2025-12-08 15:48                   ` Adrian Ratiu
2025-12-08  9:10             ` Adrian Ratiu
2025-11-19 21:10   ` [PATCH v5 5/7] submodule: fix case-folding gitdir filesystem colisions Adrian Ratiu
2025-11-19 21:10   ` [PATCH v5 6/7] submodule: use hashed name for gitdir Adrian Ratiu
2025-11-19 21:10   ` [PATCH v5 7/7] meson/Makefile: allow setting submodule encoding at build time Adrian Ratiu
2025-12-05 12:19     ` Patrick Steinhardt
2025-12-05 19:42       ` Adrian Ratiu
2025-12-05 22:52         ` Junio C Hamano
2025-12-06 12:02           ` Patrick Steinhardt
2025-12-06 16:48             ` Junio C Hamano
2025-12-08  9:23             ` Adrian Ratiu
2025-12-08  9:42           ` Adrian Ratiu
2025-12-13  8:08 ` [PATCH v6 00/10] Add submodulePathConfig extension and gitdir encoding Adrian Ratiu
2025-12-13  8:08   ` [PATCH v6 01/10] submodule--helper: use submodule_name_to_gitdir in add_submodule Adrian Ratiu
2025-12-13  8:08   ` [PATCH v6 02/10] submodule: always validate gitdirs inside submodule_name_to_gitdir Adrian Ratiu
2025-12-16  9:09     ` Patrick Steinhardt
2025-12-13  8:08   ` [PATCH v6 03/10] builtin/submodule--helper: add gitdir command Adrian Ratiu
2025-12-13  8:08   ` [PATCH v6 04/10] submodule: introduce extensions.submodulePathConfig Adrian Ratiu
2025-12-16  9:09     ` Patrick Steinhardt
2025-12-16  9:45       ` Adrian Ratiu
2025-12-16 23:22     ` Josh Steadmon
2025-12-17  7:30       ` Adrian Ratiu
2025-12-13  8:08   ` [PATCH v6 05/10] submodule: allow runtime enabling extensions.submodulePathConfig Adrian Ratiu
2025-12-16  9:09     ` Patrick Steinhardt
2025-12-16 10:01       ` Adrian Ratiu
2025-12-13  8:08   ` [PATCH v6 06/10] submodule--helper: add gitdir migration command Adrian Ratiu
2025-12-16  9:09     ` Patrick Steinhardt
2025-12-16 10:17       ` Adrian Ratiu
2025-12-13  8:08   ` [PATCH v6 07/10] builtin/credential-store: move is_rfc3986_unreserved to url.[ch] Adrian Ratiu
2025-12-13  8:08   ` [PATCH v6 08/10] submodule--helper: fix filesystem collisions by encoding gitdir paths Adrian Ratiu
2025-12-13  8:08   ` [PATCH v6 09/10] submodule: fix case-folding gitdir filesystem collisions Adrian Ratiu
2025-12-13  8:08   ` [PATCH v6 10/10] submodule: hash the submodule name for the gitdir path Adrian Ratiu
2025-12-13 14:03   ` [PATCH v6 00/10] Add submodulePathConfig extension and gitdir encoding Ben Knoble
2025-12-15 16:28     ` Adrian Ratiu
2025-12-16  0:53       ` Junio C Hamano
2025-12-18  3:43       ` Ben Knoble
2025-12-16 23:20   ` Josh Steadmon
2025-12-17  8:17     ` Adrian Ratiu
2025-12-20 10:15 ` [PATCH v7 00/11] " Adrian Ratiu
2025-12-20 10:15   ` [PATCH v7 01/11] submodule--helper: use submodule_name_to_gitdir in add_submodule Adrian Ratiu
2025-12-20 10:15   ` [PATCH v7 02/11] submodule: always validate gitdirs inside submodule_name_to_gitdir Adrian Ratiu
2025-12-20 10:15   ` [PATCH v7 03/11] builtin/submodule--helper: add gitdir command Adrian Ratiu
2025-12-20 10:15   ` [PATCH v7 04/11] submodule: introduce extensions.submodulePathConfig Adrian Ratiu
2025-12-21  3:27     ` Junio C Hamano
2025-12-23 13:35       ` Adrian Ratiu
2025-12-20 10:15   ` [PATCH v7 05/11] submodule: allow runtime enabling extensions.submodulePathConfig Adrian Ratiu
2025-12-20 10:15   ` [PATCH v7 06/11] submodule--helper: add gitdir migration command Adrian Ratiu
2025-12-20 10:15   ` [PATCH v7 07/11] builtin/credential-store: move is_rfc3986_unreserved to url.[ch] Adrian Ratiu
2025-12-20 10:15   ` [PATCH v7 08/11] submodule--helper: fix filesystem collisions by encoding gitdir paths Adrian Ratiu
2025-12-20 10:15   ` [PATCH v7 09/11] submodule: fix case-folding gitdir filesystem collisions Adrian Ratiu
2025-12-20 10:15   ` [PATCH v7 10/11] submodule: hash the submodule name for the gitdir path Adrian Ratiu
2025-12-20 10:15   ` [PATCH v7 11/11] submodule: detect conflicts with existing gitdir configs Adrian Ratiu
2025-12-21  2:39   ` [PATCH v7 00/11] Add submodulePathConfig extension and gitdir encoding Junio C Hamano

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

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

  git send-email \
    --in-reply-to=87jz173tih.fsf@collabora.com \
    --to=adrian.ratiu@collabora.com \
    --cc=aaron@schrab.com \
    --cc=ben.knoble@gmail.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --cc=phillip.wood123@gmail.com \
    --cc=ps@pks.im \
    --cc=rdamazio@google.com \
    --cc=steadmon@google.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.
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 NNTP newsgroup(s).