From: Jeff King <firstname.lastname@example.org> To: Jonathan Nieder <email@example.com> Cc: firstname.lastname@example.org, Brandon Williams <email@example.com>, Stefan Beller <firstname.lastname@example.org>, Aaron Schrab <email@example.com> Subject: Re: [RFC] submodule: munge paths to submodule git directories Date: Thu, 17 Jan 2019 12:32:17 -0500 [thread overview] Message-ID: <20190117173216.GB27667@sigill.intra.peff.net> (raw) In-Reply-To: <20190115012507.GK162110@google.com> On Mon, Jan 14, 2019 at 05:25:07PM -0800, Jonathan Nieder wrote: > I've put a summary in https://crbug.com/git/28 to make this easier to > pick up where we left off. Summary from there of the upstream review: > > 1. Using urlencoding to escape the slashes is fine, but what if we > want to escape some other character (for example to handle > case-insensitive filesystems)? > > Proposal: Store the escaping mapping in config so it can be > modified it in the future: > > [submodule "plugin/hooks"] > gitdirname = plugins%2fhooks I think it might be worth dealing with case-sensitivity _now_, since we know it's a problem. That doesn't make the problem of "what if we want to change the mapping later" go away, but it does make it a lot less likely to come up. > 2. The urlencoded name could conflict with a submodule that has % in > its name in an existing clone created by an older version of Git. > > Proposal: Put submodules in a new .git/submodules/ directory > instead of .git/modules/. This proposal is orthogonal to (1), right? I.e., if we store the mapping then that is what tells us we're using the mapped name. > 3. These gitdirname settings can clutter up .git/config. > > Proposal: For the "easy" cases (e.g. submodule name consisting of > [a-z]*), allow omitting the gitdirname setting. Not having thought about it too hard, I suspect that may open back up corner cases with respect to backwards compatibility and ambiguity. Are you worried about human-readable clutter? I.e., that .git/config becomes hard to read? If so, then: - I doubt this is any worse than the existing tracking-branch config. - it might be reasonable to store it in .git/submodule-config, and make sure that .git/config contains a single "[include]path = submodule-config" line. I've been tempted to do that for tracking-branch config. Or are you worried about the cost of parsing those entries? Basically every git command parses config linearly at least once; this normally isn't noticeable, but at some size it becomes a problem. I have no idea what that size is. If so, then I think we'd want submodule config in its own file but _without_ an include from the normal config file. That would break compatibility with anything that tries to use "git config submodule.foo.path", etc. That's all just musing. I'm actually not really convinced it's a problem. > Is that a fair summary? Are there concerns from the review that I > forgot, or would a new version of the series that addresses those > three problems put us in good shape? I don't really have a strong opinion either way. I still think the one-way transformation that the patch uses is less elegant than a real encode/decode round-trip (i.e., what I discussed in ). But I admit to not having thought through all of the details of the encode/decode thing, and certainly have not written the code.  http://public-inbox.org/git/20180809212602.GA11342@sigill.intra.peff.net/
next prev parent reply other threads:[~2019-01-17 17:32 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-07 23:06 Brandon Williams 2018-08-07 23:25 ` Jonathan Nieder 2018-08-08 0:14 ` Junio C Hamano 2018-08-08 22:33 ` [PATCH 0/2] munge submodule names Brandon Williams 2018-08-08 22:33 ` [PATCH 1/2] submodule: create helper to build paths to submodule gitdirs Brandon Williams 2018-08-08 23:21 ` Stefan Beller 2018-08-09 0:45 ` Brandon Williams 2018-08-10 21:27 ` Junio C Hamano 2018-08-10 21:45 ` Brandon Williams 2018-08-08 22:33 ` [PATCH 2/2] submodule: munge paths to submodule git directories Brandon Williams 2018-08-09 21:26 ` Jeff King 2018-08-14 18:04 ` Brandon Williams 2018-08-14 18:57 ` Jonathan Nieder 2018-08-14 21:08 ` Stefan Beller 2018-08-14 21:12 ` Jonathan Nieder 2018-08-14 22:34 ` Stefan Beller 2018-08-16 2:34 ` Jonathan Nieder 2018-08-16 2:39 ` Stefan Beller 2018-08-16 2:47 ` Jonathan Nieder 2018-08-16 17:34 ` Brandon Williams 2018-08-16 18:19 ` [PATCH] submodule: add config for where gitdirs are located Brandon Williams 2018-08-20 22:03 ` Junio C Hamano 2018-08-16 15:07 ` [PATCH 2/2] submodule: munge paths to submodule git directories Junio C Hamano 2018-08-14 18:58 ` Jeff King 2018-08-28 21:35 ` Stefan Beller 2018-08-29 5:25 ` Jeff King 2018-08-29 18:10 ` Stefan Beller 2018-08-29 21:03 ` Jeff King 2018-08-29 21:10 ` Stefan Beller 2018-08-29 21:18 ` Jonathan Nieder 2018-08-29 21:27 ` Stefan Beller 2018-08-29 21:30 ` Jeff King 2018-08-29 21:09 ` Jonathan Nieder 2018-08-29 21:14 ` Stefan Beller 2018-08-29 21:25 ` Brandon Williams 2018-08-29 21:32 ` Jeff King 2018-08-16 0:19 ` Aaron Schrab 2019-01-15 1:25 ` [RFC] " Jonathan Nieder 2019-01-17 17:32 ` Jeff King [this message] 2019-01-17 17:57 ` Stefan Beller
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=20190117173216.GB27667@sigill.intra.peff.net \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [RFC] submodule: munge paths to submodule git directories' \ /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
Code repositories for project(s) associated with this 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).