threads / patch / 26159

patch, 2 partsHandle cvsimport config for uppercase options

Subject: [PATCH 0/2] Handle cvsimport config for uppercase options

## tl;dr

3 messages between Dec 29, 2010 and Dec 29, 2010. Diffs are folded; open one to read it.

replies: 2people: 1as markdown or json

Michael J Gruber· Dec 29, 2010, 21:55 UTC · lore

Sorry for rushing this out (no in-reply-to etc.) , I'm mostly offline these days and pressed by the rc phase.

This is the version (n+1) as suggested by Junio, so 2/2 is mostly by JC (only that his patch didn't apply, it probably was for maint).

I dropped the documentation patch, since the discussion resulted in short config option names being frowned upon, so we should leave them undocumented and return to rewrite them one day.

Michael J Gruber (2):
  cvsimport: partial whitespace cleanup
  cvsimport: handle the parsing of uppercase config options
 git-cvsimport.perl   |   33 +++++++++++++++++++++++++--------
 t/t9600-cvsimport.sh |    7 +++++--
 2 files changed, 30 insertions(+), 10 deletions(-)
-- 
1.7.3.4.865.ga2bc4
Michael J Gruber· Dec 29, 2010, 21:55 UTC · re: Michael J Gruber · lore

[PATCH 1/2] cvsimport: partial whitespace cleanup

in preparation of the config parse patch
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 git-cvsimport.perl |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
Show changes to git-cvsimport.perl +8 −8
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index d27abfe..7888b77 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -91,8 +91,8 @@ sub write_author_info($) {
 
 # convert getopts specs for use by git config
 sub read_repo_config {
-    # Split the string between characters, unless there is a ':'
-    # So "abc:de" becomes ["a", "b", "c:", "d", "e"]
+	# Split the string between characters, unless there is a ':'
+	# So "abc:de" becomes ["a", "b", "c:", "d", "e"]
 	my @opts = split(/ *(?!:)/, shift);
 	foreach my $o (@opts) {
 		my $key = $o;
@@ -100,13 +100,13 @@ sub read_repo_config {
 		my $arg = 'git config';
 		$arg .= ' --bool' if ($o !~ /:$/);
 
-        chomp(my $tmp = `$arg --get cvsimport.$key`);
+		chomp(my $tmp = `$arg --get cvsimport.$key`);
 		if ($tmp && !($arg =~ /--bool/ && $tmp eq 'false')) {
-            no strict 'refs';
-            my $opt_name = "opt_" . $key;
-            if (!$$opt_name) {
-                $$opt_name = $tmp;
-            }
+			no strict 'refs';
+			my $opt_name = "opt_" . $key;
+			if (!$$opt_name) {
+				$$opt_name = $tmp;
+			}
 		}
 	}
 }
-- 
1.7.3.4.865.ga2bc4
Michael J Gruber· Dec 29, 2010, 21:55 UTC · re: Michael J Gruber · lore

[PATCH 2/2] cvsimport: handle the parsing of uppercase config options

The current code leads to
fatal: bad config value for 'cvsimport.r' in .git/config
for a standard use case with cvsimport.r set:

cvsimport sets internal variables by checking the config for each possible command line option. The problem is that config items are case insensitive, so config.r and config.R are the same. The ugly error is due to that fact that cvsimport expects a bool for -R (and thus config.R) but a remote name for -r (and thus config.r).

Fix this by making cvsimport expect long names for uppercase options.

config options for cvsimport have been undocumented so far, though present in the code and advertised in several tutorials. So one may read "enhance" for "fix". Similarly, the names for the options are "documented" in the code, waitiing for their lowercase equivalents to be transformed into long config options, as well.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 git-cvsimport.perl   |   19 ++++++++++++++++++-
 t/t9600-cvsimport.sh |    7 +++++--
 2 files changed, 23 insertions(+), 3 deletions(-)
Show changes to 2 files +24 −4

git-cvsimport.perl, t/t9600-cvsimport.sh

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 7888b77..8e683e5 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -90,6 +90,14 @@ sub write_author_info($) {
 }
 
 # convert getopts specs for use by git config
+my %longmap = (
+	'A:' => 'authors-file',
+	'M:' => 'merge-regex',
+	'P:' => undef,
+	'R' => 'track-revisions',
+	'S:' => 'ignore-paths',
+);
+
 sub read_repo_config {
 	# Split the string between characters, unless there is a ':'
 	# So "abc:de" becomes ["a", "b", "c:", "d", "e"]
@@ -99,8 +107,17 @@ sub read_repo_config {
 		$key =~ s/://g;
 		my $arg = 'git config';
 		$arg .= ' --bool' if ($o !~ /:$/);
-
-		chomp(my $tmp = `$arg --get cvsimport.$key`);
+		my $ckey = $key;
+
+		if (exists $longmap{$o}) {
+			# An uppercase option like -R cannot be
+			# expressed in the configuration, as the
+			# variable names are downcased.
+			$ckey = $longmap{$o};
+			next if (! defined $ckey);
+			$ckey =~ s/-//g;
+		}
+		chomp(my $tmp = `$arg --get cvsimport.$ckey`);
 		if ($tmp && !($arg =~ /--bool/ && $tmp eq 'false')) {
 			no strict 'refs';
 			my $opt_name = "opt_" . $key;
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 432b82e..4c384ff 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -89,7 +89,8 @@ EOF
 test_expect_success PERL 'update git module' '
 
 	(cd module-git &&
-	git cvsimport -a -R -z 0 module &&
+	git config cvsimport.trackRevisions true &&
+	git cvsimport -a -z 0 module &&
 	git merge origin
 	) &&
 	test_cmp module-cvs/o_fortuna module-git/o_fortuna
@@ -117,7 +118,8 @@ test_expect_success PERL 'cvsimport.module config works' '
 
 	(cd module-git &&
 		git config cvsimport.module module &&
-		git cvsimport -a -R -z0 &&
+		git config cvsimport.trackRevisions true &&
+		git cvsimport -a -z0 &&
 		git merge origin
 	) &&
 	test_cmp module-cvs/tick module-git/tick
@@ -137,6 +139,7 @@ test_expect_success PERL 'import from a CVS working tree' '
 
 	$CVS co -d import-from-wt module &&
 	(cd import-from-wt &&
+		git config cvsimport.trackRevisions false &&
 		git cvsimport -a -z0 &&
 		echo 1 >expect &&
 		git log -1 --pretty=format:%s%n >actual &&
-- 
1.7.3.4.865.ga2bc4

← back to recent threads