Fix upstream_commitish_search API

upstream_commitish_search used to return a commit.  If it succeeded
The caller could find the tag in $tried[-1].  Both callers relied on
this unpleasant and error-prone API.

In
   0bb8e2a87e3c8b5be0fce5c2491b292e9273056e
   Dgit::upstream_commitish_search: fail if more than one tag exists

the algorithm was changed to keep looking, so it can reject ambiguous
situations.  The result is that $tried[-1] is entirely wrong in the
success case.  (This is spotted by the tagupl-baredebian test.)

It would have been possible to fix this by making
upstream_commitish_search synthesise a suitable return value for
putting in $tried, but that is absurd.

Instead give this function a sensible calling convention.  It now
returns a list of the tag name (for messages etc.) and the
commitish (for use).  Change both call sites.

CC: Sean Whitton <spwhitton@spwhitton.name>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

diff --git a/Debian/Dgit.pm b/Debian/Dgit.pm
index 4e1965702c1d8c980d9ecad86f40c415873fee4f..4059e2b02372388d8c4107beda3a7e757bc9ff00 100644 (file)
--- a/Debian/Dgit.pm
+++ b/Debian/Dgit.pm
@@ -639,9 +639,10 @@ sub upstream_commitish_search ($$) {
        my $tag = $tagpfx.(dep14_version_mangle $upstream_version);
        my $new_upstream = git_get_ref "refs/tags/$tag";
        push @$tried, $tag;
-       push @found, $tag if $new_upstream;
+       push @found, [ $tag, $new_upstream ] if $new_upstream;
     }
-    return $found[0] if @found == 1;
+    return @{ $found[0] } if @found == 1;
+    return ();
 }
 
 sub resolve_upstream_version ($$) {
@@ -651,7 +652,8 @@ sub resolve_upstream_version ($$) {
     my $message = __ 'using specified upstream commitish';
     if (!defined $new_upstream) {
        my @tried;
-       $new_upstream = upstream_commitish_search $upstream_version, \@tried;
+       ($used, $new_upstream) =
+           upstream_commitish_search $upstream_version, \@tried;
        if (!length $new_upstream) {
            fail f_
                "Could not determine appropriate upstream commitish.\n".
@@ -659,7 +661,6 @@ sub resolve_upstream_version ($$) {
                " Check version, and specify upstream commitish explicitly.",
                "@tried";
        }
-       $used = $tried[-1];
        $message = f_ 'using upstream from git tag %s', $used;
     } elsif ($new_upstream =~ m{^refs/tags/($versiontag_re)$}s) {
        $message = f_ 'using upstream from git tag %s', $1;

diff --git a/git-debrebase b/git-debrebase
index 171249f771e1716a9eb0b39675c460780597291e..340a590999d2551d825de2baed8477fdc168af9d 100755 (executable)
--- a/git-debrebase
+++ b/git-debrebase
@@ -2697,9 +2697,10 @@ END
     if (!@upstreams) {
        if ($do_tags) {
            my @tried;
-           my $ups_rev = upstream_commitish_search $version, \@tried;
+           my ($ups_tag, $ups_rev) =
+               upstream_commitish_search $version, \@tried;
            if ($ups_rev) {
-               my $this = f_ "git tag %s", $tried[-1];
+               my $this = f_ "git tag %s", $ups_tag;
                push @upstreams, { Commit => $ups_rev,
                                   Source => $this,
                                 };