This code

    git log --pretty=format:'%D' --decorate=full "$branch_commit" \
        | perl -MDpkg::Version -F", " -we'
            @debian_tag_vs =
                  sort { version_compare($b, $a) } grep defined,
                  map m|tag: refs/tags/'"$prefix"'(.+)|, @F
              or next;
            print "'"$prefix"'$debian_tag_vs[0]\n"; exit'

is wrong because it passes gitified version numbers (ie mangled
according to DEP-14) to version_compare.  version_compare rejects `_`
(and presumably, if it accepted it, it would DTWT).

The version_compare is only called if the user has two debian/* tags
on the same commit, which would be an anomaly in itself.  But, I can
repro the fault as follows:

STEPS

  git clone --no-tags git@salsa.debian.org:python-team/packages/bundlewrap
  cd bundlewrap
  git fetch origin refs/tags/debian/4.23.1-1_exp1:refs/tags/debian/4.23.1-1_exp1 refs/tags/upstream/4.23.1:refs/tags/upstream/4.23.1
  git tag -m anomalous debian/4.23.1-1 debian/4.23.1-1_exp1~0
  git debpush --dry-run --force=detached

ACTUAL BEHAVIOUR

  git-debpush: warning: HEAD is detached; you probably don't want to debpush it ('detached' check)
  -e: error: 4.23.1-1_exp1 is not a valid version

EXPECTED BEHAVIOUR

4.23.1 > 4.23.1-1~exp1 so it should prefer the "later" 4.23.1 tag,
which has no dgit metadata, so:

  git-debpush: warning: HEAD is detached; you probably don't want to debpush it ('detached' check)
  git-debpush: could not determine the git branch layout
  git-debpush: please supply a --quilt= argument

However, I'm not convinced that sorting by version number here is
right, in any case.  If there are multiple tags, why wouldn't we
prefer the latest one by tag date?

Empirically, git log --pretty=format:%D prints tags for the same
commit in tagger date order, so we could just take the last.

Ian.

Ian Jackson writes ("Bug#1110996: git-debpush uses version_compare on git tag names"):

Missing here is

    git checkout fc56592e7e85e31914e7758381355595d22be1ee

Hello,

I think you might have misread the code slightly.  Rather than only
comparing multiple tags on the same commit, the git log command in
find_last_tag returns all tags *reachable* from $branch_commit, and then
picks the one with the latest version number.

I am not sure whether latest tag by tagger date or Debian version number
would be more useful and/or principled so I suggest we just fix this bug
for now.  I've submitted an MR to that effect.

Sean Whitton writes ("Bug#1110996: git-debpush uses version_compare on git tag names"):

I don't have time to go and stare at the code again right now, but I
don't believe this can be true.

The existing code will crash if the compare_versions is ever fed a
version number that was extracted from a tag representing an upload
whose Debian version had a ~.

If you are right, that would happen if there were *any* previous ~exp
upload.

I'm not sure why we are digging into the git history so far anyway.
If we want to know what the branch format is *here* at this commit,
why wouldn't we take the gitly-most-recent tag as authoritative?

If the version number ordering is different to the git history graph
ordering [1], and there was a branch format transition in between, we
want the most recent one in git terms, since that's the one which has
the same branch format as the commit we're uploading.

[1] This seems like it could happen if the same package was backported
or the suite was changed or something.  Indeed I think we recently did
this with dgit.git: I uploaded +exp1 to experimental, switched the
suite to sid, and uploaded again.

Eg, suppoase the commit graph looks like this:

      o HEAD
      |
      o release, tag: debian/1.0
      |
      o change branch format
      |
      o release, tag: debian/1.0+exp1
      |
     ...

We definitely want the format mentioned in the 1.0 tag.

Ian.

Ian Jackson writes ("Re: Bug#1110996: git-debpush uses version_compare on git tag names"):

Also, if what you say were true, we would always read the whole of the
git log output and the stuff about pipefail would be
unnecessary/wrong.

Ian.

Hello,

Thanks, yes, you're right.

I agree that we should switch to doing it in Git tag order.

Hello,

By which I mean tagGING order, i.e., date order.