git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Lehmann <Jens.Lehmann@web.de>
To: Jon Seymour <jon.seymour@gmail.com>
Cc: git@vger.kernel.org, gitster@pobox.com, phil.hord@gmail.com,
	Ramsay Jones <ramsay@ramsay1.demon.co.uk>,
	Johannes Sixt <j6t@kdbg.org>
Subject: Re: [PATCH v7 0/9] submodule: improve robustness of path handling
Date: Mon, 28 May 2012 22:07:11 +0200	[thread overview]
Message-ID: <4FC3DAEF.1070508@web.de> (raw)
In-Reply-To: <1338132851-23497-1-git-send-email-jon.seymour@gmail.com>

Am 27.05.2012 17:34, schrieb Jon Seymour:
> This series improves the robustness of path handling by 'git submodule' by:
> 
> * detecting submodule URLs that will result in non-sensical submodule origin URLs
> 
> * improving handling of various kinds of relative superproject origin URLs
> 
> * improving handling of various kinds of denormalized superproject origin URLs

Hmm, this has become a quite invasive patch series. While I bought the
use case of having a superproject with a relative url and was inclined
to accept that it might even not start "./" or "../" (even though that
is a pretty unusual use and can be easily fixed by prepending a "./"),
I'm not sure the in depth check of URLs is worth the code churn. And
especially the high probability of breaking other peoples use cases in
rather subtle ways worry me (this did happen quite often when the
submodule script was changed in the past; as an example take the
windows path issues Johannes already pointed out in his email). And I
can't remember bug reports that people complained about URL problems
due to the issues you intend to fix here, which makes me think they
might be well intended but possibly unnecessary (but my memory might
server me wrong here).

So I'd vote for just fixing the relative submodule path issues and to
not care about the possible issues with URLs. Opinions?

(And patches 6-8 contain changes to test cases other than just changing
test_expect_failure to test_expect_success which makes reviewing this
series unnecessarily hard)

  parent reply	other threads:[~2012-05-28 20:08 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
2012-05-27 15:34 ` [PATCH v7 1/9] submodule: additional regression tests for relative URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 2/9] submodule: document failure to detect invalid submodule URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 3/9] submodule: document failure to handle relative superproject origin URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 4/9] submodule: document failure to handle improperly normalized remote " Jon Seymour
2012-05-27 15:34 ` [PATCH v7 5/9] submodule: extract normalize_path into standalone function Jon Seymour
2012-05-27 15:34 ` [PATCH v7 6/9] submodule: fix detection of invalid submodule URL Jon Seymour
2012-05-28 19:01   ` Johannes Sixt
2012-05-28 21:39     ` Jon Seymour
2012-06-03  9:51       ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 7/9] submodule: fix sync handling of relative superproject origin URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 8/9] submodule: fix handling of denormalized " Jon Seymour
2012-05-27 22:57   ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 9/9] submodule: fix normalization to handle repeated ./ Jon Seymour
2012-05-28 20:07 ` Jens Lehmann [this message]
2012-05-28 22:01   ` [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
2012-05-29 19:21     ` Jens Lehmann

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=4FC3DAEF.1070508@web.de \
    --to=jens.lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=jon.seymour@gmail.com \
    --cc=phil.hord@gmail.com \
    --cc=ramsay@ramsay1.demon.co.uk \
    /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).