From cf46ae056b60535bb6668faa9d0bd7ea1d33dba1 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 22 Jun 2020 16:09:15 +0100 Subject: 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 Signed-off-by: Ian Jackson --- Debian/Dgit.pm | 9 +++++---- git-debrebase | 5 +++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Debian/Dgit.pm b/Debian/Dgit.pm index 4e19657..4059e2b 100644 --- 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 171249f..340a590 100755 --- 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, }; -- cgit v1.2.3