From: Stefan Beller <sbeller@google.com> To: Jeff King <peff@peff.net> Cc: Jonathan Nieder <jrnieder@gmail.com>, git <git@vger.kernel.org>, Brandon Williams <bwilliams.eng@gmail.com>, Aaron Schrab <aaron@schrab.com> Subject: Re: [RFC] submodule: munge paths to submodule git directories Date: Thu, 17 Jan 2019 09:57:07 -0800 Message-ID: <CAGZ79kbofESBSTHbDdPmeZgb2Pwz=8FVmtXG6x376utMyS0vqA@mail.gmail.com> (raw) In-Reply-To: <20190117173216.GB27667@sigill.intra.peff.net> On Thu, Jan 17, 2019 at 9:32 AM Jeff King <peff@peff.net> wrote: > > 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[1] 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. Makes sense. > > > 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. Technically true, but it allows for easier implementation: now we have 2 distinct namespaces, such that we can avoid double booking easier: Consider 2 submodules "a b" and "a%20b". Without (2), (1) is hard to explain as the first might have been encoded to a%20b or there might have been the second put in place from a current (old) version of Git. So we'd have to reason about these corner cases. With (2) in place, we'd only ever have the second in a place "a%2520b" (if I am to trust https://www.urlencoder.org/) > > 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. ok, we can deal with the problem once it arises. > > > 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 [1]). But I admit to > not having thought through all of the details of the encode/decode > thing, and certainly have not written the code. The suggestion of adding at least a test for url encoding (2. from your mail) is sensible. Stefan > > [1] http://public-inbox.org/git/20180809212602.GA11342@sigill.intra.peff.net/
prev parent reply other threads:[~2019-01-17 17:57 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 2019-01-17 17:57 ` Stefan Beller [this message]
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='CAGZ79kbofESBSTHbDdPmeZgb2Pwz=8FVmtXG6x376utMyS0vqA@mail.gmail.com' \ --to=sbeller@google.com \ --cc=aaron@schrab.com \ --cc=bwilliams.eng@gmail.com \ --cc=git@vger.kernel.org \ --cc=jrnieder@gmail.com \ --cc=peff@peff.net \ /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
git@vger.kernel.org list mirror (unofficial, one of many) This inbox may be cloned and mirrored by anyone: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V1 git git/ https://public-inbox.org/git \ git@vger.kernel.org public-inbox-index git Example config snippet for mirrors. Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.io/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ code repositories for the project(s) associated with this inbox: https://80x24.org/mirrors/git.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git