git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Wong <normalperson@yhbt.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Avery Pennarun <apenwarr@gmail.com>
Subject: [PATCH] git-svn: don't sanitize remote names in config
Date: Sat, 28 Jun 2008 20:40:32 -0700	[thread overview]
Message-ID: <20080629034032.GA23492@untitled> (raw)
In-Reply-To: <32541b130806250801p1508d15axc610f335b8d235ef@mail.gmail.com>

The original sanitization code was just taken from the
remotes2config.sh shell script in contrib.

Credit to Avery Pennarun for noticing this mistake, and Junio
for clarifying the rules for config section names:

Junio C Hamano wrote <7vfxr23s6m.fsf@gitster.siamese.dyndns.org>:
> In
>
> 	[foo "bar"] baz = value
>
> foo and baz must be config.c::iskeychar() (and baz must be isalpha()), but
> "bar" can be almost anything.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---

  Avery Pennarun <apenwarr@gmail.com> wrote:
  > On 6/25/08, Eric Wong <normalperson@yhbt.net> wrote:
  > > No, nothing to do with DNS hostnames in the remote names.  I think I
  > >  just looked at remotes2config.sh one day and used it as a reference :x
  > >
  > >  It's late and I've had a rough few days, but shouldn't
  > >  sanitize_remote_name() just escape . and "?  Right now it's converting
  > >  stuff to . which has me very confused...
  > 
  > I think there might be higher-level problems here: what is it
  > sanitizing anyway, and why?  If it found my D2007_Win32 svn-remote
  > entry in the config (as it seems to have done when trying to locate
  > its parent branch during fetch), and *then* it sanitized it to
  > D2007.Win32, that doesn't even make any sense.  Clearly something
  > straight from the config file doesn't need to be sanitized.
  > 
  > However, I don't understand the code well enough to be able to say a)
  > whether that's exactly what happened, or b) other places where
  > sanitize_remote_name() *is* important, or c) whether
  > sanitize_remote_name() is even correct.

  Nope.  It's not important anywhere from what I can tell.

-- 

 git-svn.perl |   15 +++------------
 1 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 50ace22..f789a6e 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1462,13 +1462,6 @@ sub verify_remotes_sanity {
 	}
 }
 
-# we allow more chars than remotes2config.sh...
-sub sanitize_remote_name {
-	my ($name) = @_;
-	$name =~ tr{A-Za-z0-9:,/+-}{.}c;
-	$name;
-}
-
 sub find_existing_remote {
 	my ($url, $remotes) = @_;
 	return undef if $no_reuse_existing;
@@ -2853,7 +2846,7 @@ sub _new {
 	unless (defined $ref_id && length $ref_id) {
 		$_[2] = $ref_id = $Git::SVN::default_ref_id;
 	}
-	$_[1] = $repo_id = sanitize_remote_name($repo_id);
+	$_[1] = $repo_id;
 	my $dir = "$ENV{GIT_DIR}/svn/$ref_id";
 	$_[3] = $path = '' unless (defined $path);
 	mkpath(["$ENV{GIT_DIR}/svn"]);
@@ -4707,8 +4700,7 @@ sub minimize_connections {
 
 		# skip existing cases where we already connect to the root
 		if (($ra->{url} eq $ra->{repos_root}) ||
-		    (Git::SVN::sanitize_remote_name($ra->{repos_root}) eq
-		     $repo_id)) {
+		    ($ra->{repos_root} eq $repo_id)) {
 			$root_repos->{$ra->{url}} = $repo_id;
 			next;
 		}
@@ -4747,8 +4739,7 @@ sub minimize_connections {
 	foreach my $url (keys %$new_urls) {
 		# see if we can re-use an existing [svn-remote "repo_id"]
 		# instead of creating a(n ugly) new section:
-		my $repo_id = $root_repos->{$url} ||
-		              Git::SVN::sanitize_remote_name($url);
+		my $repo_id = $root_repos->{$url} || $url;
 
 		my $fetch = $new_urls->{$url};
 		foreach my $path (keys %$fetch) {
-- 
Eric Wong

      reply	other threads:[~2008-06-29  3:41 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-24 15:54 [PATCH/RFC] git-svn: sanitize_remote_name should accept underscores Avery Pennarun
2008-06-25  6:44 ` Eric Wong
2008-06-25  6:55   ` Eric Wong
2008-06-25  7:11     ` Junio C Hamano
2008-06-25  7:45       ` Eric Wong
2008-06-25 15:01         ` Avery Pennarun
2008-06-29  3:40           ` Eric Wong [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

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

  git send-email \
    --in-reply-to=20080629034032.GA23492@untitled \
    --to=normalperson@yhbt.net \
    --cc=apenwarr@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).