threads / patch / 2557

patchsymref support for import scripts

Subject: [PATCH] symref support for import scripts

## tl;dr

4 messages between Nov 16, 2005 and Nov 16, 2005. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Pavel Roskin· Nov 16, 2005, 18:27 UTC · lore
Fix git import script not to assume that .git/HEAD is a symlink.
Signed-off-by: Pavel Roskin <proski@gnu.org>
Show changes to 3 files +13 −8

git-archimport.perl, git-cvsimport.perl, git-svnimport.perl

diff --git a/git-archimport.perl b/git-archimport.perl
index e22c816..23becb7 100755
--- a/git-archimport.perl
+++ b/git-archimport.perl
@@ -410,8 +410,7 @@ foreach my $ps (@psets) {
     open  HEAD, ">$git_dir/refs/heads/$ps->{branch}";
     print HEAD $commitid;
     close HEAD;
-    unlink ("$git_dir/HEAD");
-    symlink("refs/heads/$ps->{branch}","$git_dir/HEAD");
+    system('git-update-ref', 'HEAD', "$ps->{branch}");
 
     # tag accordingly
     ptag($ps->{id}, $commitid); # private tag
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 7bd9136..efe1934 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -437,7 +437,11 @@ unless(-d $git_dir) {
 		       "Either use the correct '-o branch' option,\n".
 		       "or import to a new repository.\n";
 
-	$last_branch = basename(readlink("$git_dir/HEAD"));
+	open(F, "git-symbolic-ref HEAD |") or
+		die "Cannot run git-symbolic-ref: $!\n";
+	chomp ($last_branch = <F>);
+	$last_branch = basename($last_branch);
+	close(F);
 	unless($last_branch) {
 		warn "Cannot read the last branch name: $! -- assuming 'master'\n";
 		$last_branch = "master";
@@ -829,8 +833,7 @@ if($orig_branch) {
 	print "DONE; creating $orig_branch branch\n" if $opt_v;
 	system("cp","$git_dir/refs/heads/$opt_o","$git_dir/refs/heads/master")
 		unless -f "$git_dir/refs/heads/master";
-	unlink("$git_dir/HEAD");
-	symlink("refs/heads/$orig_branch","$git_dir/HEAD");
+	system('git-update-ref', 'HEAD', "$orig_branch");
 	unless ($opt_i) {
 		system('git checkout');
 		die "checkout failed: $?\n" if $?;
diff --git a/git-svnimport.perl b/git-svnimport.perl
index af13fdd..45d77c5 100755
--- a/git-svnimport.perl
+++ b/git-svnimport.perl
@@ -216,7 +216,11 @@ unless(-d $git_dir) {
 	-f "$git_dir/svn2git"
 		or die "'$git_dir/svn2git' does not exist.\n".
 		       "You need that file for incremental imports.\n";
-	$last_branch = basename(readlink("$git_dir/HEAD"));
+	open(F, "git-symbolic-ref HEAD |") or
+		die "Cannot run git-symbolic-ref: $!\n";
+	chomp ($last_branch = <F>);
+	$last_branch = basename($last_branch);
+	close(F);
 	unless($last_branch) {
 		warn "Cannot read the last branch name: $! -- assuming 'master'\n";
 		$last_branch = "master";
@@ -766,8 +770,7 @@ if($orig_branch) {
 	print "DONE; creating $orig_branch branch\n" if $opt_v and (not defined $opt_l or $opt_l > 0);
 	system("cp","$git_dir/refs/heads/$opt_o","$git_dir/refs/heads/master")
 		unless -f "$git_dir/refs/heads/master";
-	unlink("$git_dir/HEAD");
-	symlink("refs/heads/$orig_branch","$git_dir/HEAD");
+	system('git-update-ref', 'HEAD', "$orig_branch");
 	unless ($opt_i) {
 		system('git checkout');
 		die "checkout failed: $?\n" if $?;
-- 
Regards,
Pavel Roskin
Junio C Hamano· Nov 16, 2005, 19:43 UTC · re: Pavel Roskin · lore

Re: [PATCH] symref support for import scripts

Pavel Roskin <proski@gnu.org> writes:
> Fix git import script not to assume that .git/HEAD is a symlink.
>
> Signed-off-by: Pavel Roskin <proski@gnu.org>
Thanks.

Martin and Matthias, are these OK with you two? All of the changes look trivially correct, so I'll take them.

Show 17 quoted lines
> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> index 7bd9136..efe1934 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -437,7 +437,11 @@ unless(-d $git_dir) {
>  		       "Either use the correct '-o branch' option,\n".
>  		       "or import to a new repository.\n";
>  
> -	$last_branch = basename(readlink("$git_dir/HEAD"));
> +	open(F, "git-symbolic-ref HEAD |") or
> +		die "Cannot run git-symbolic-ref: $!\n";
> +	chomp ($last_branch = <F>);
> +	$last_branch = basename($last_branch);
> +	close(F);
>  	unless($last_branch) {
>  		warn "Cannot read the last branch name: $! -- assuming 'master'\n";
>  		$last_branch = "master";

This part, before or after Pavel's fixes, seems to refuse a branch named 'topic/#1'. This is not a problem for import scripts that name their own branches based on what is in the foreign SCM and flatten their the branch namespaces, but I'd prefer a comment about the issue somewhere around this code, to prevent people from copying and pasting the use of "basename()". There is a corresponding piece in svnimport as well.

Matthias Urlichs· Nov 16, 2005, 19:57 UTC · re: Junio C Hamano · lore

Re: [PATCH] symref support for import scripts

Hi,
Junio C Hamano:
> Martin and Matthias, are these OK with you two?  All of the
> changes look trivially correct, so I'll take them.
> 
IMHO: That code needs to vanish anyway.

The only reason we have to read HEAD is that, at the end of the day^Wimport, whatever HEAD points to may have been updated. Thus, we need to merge forward, to get the index back into sync.

The only thing we want to do with HEAD, in order to accomplish that, is to find the SHA1 it points to, before and after the import. No use of git-symbolic-ref is necessary for that.

All the deref code was a good idea when we actually modified the checked-out index and/or tree while importing, but that does not happen any more.

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
A good supervisor can step on your toes without messing up your shine.
Junio C Hamano· Nov 16, 2005, 20:26 UTC · re: Matthias Urlichs · lore

Re: [PATCH] symref support for import scripts

Matthias Urlichs <smurf@smurf.noris.de> writes:
> The only thing we want to do with HEAD, in order to accomplish that,
> is to find the SHA1 it points to, before and after the import.
> No use of git-symbolic-ref is necessary for that.
True.  'git-rev-parse --verify HEAD^0' would be sufficient.

← back to recent threads