pre-commit-ci / issues

public issues for https://pre-commit.ci
17 stars 4 forks source link

tags which are incorrectly annotated confuse newer git in autoupdate #45

Closed asottile closed 3 years ago

asottile commented 3 years ago

CC @marcogorelli -- the tag for nbqa-dev/nbqa has an ambiguous resolution (v0.5.8 (annotated) points at 0.5.8 (lightweight))

In git 2.25.1 it correctly resolves:

$ git describe --tags FETCH_HEAD --abbrev=0
warning: tag 'v0.5.8' is really '0.5.8' here
v0.5.8

In git 2.30.1 (what's used in autoupdater)

/y # git describe FETCH_HEAD --tags --abbrev=0
warning: tag '0.5.8' is externally known as 'v0.5.8'
v0.5.8-2-g007eb3cf60194030c5b4cf133beeada5009badfb

the easiest short-term fix is to unbreak the tag in nbqa-dev/nbqa -- I'll follow up with the git mailing list after I've bisected the change that regressed this

matthewfeickert commented 3 years ago

As an additional point of reference this is also affecting https://github.com/scikit-hep/pyhf/pull/1333 (I realize this is a "duh" statement given that this is affecting all projects with nbQA in their .pre-commit-config.yaml).

Many thanks Anthony for being so fast to start addressing this — much appreciated as always.

MarcoGorelli commented 3 years ago

Thanks @asottile - I accidentally tagged a release v0.5.8, though previous versions were tagged as e.g. 0.5.7 (without the v). I thought I'd deleted the v0.5.8 (at least, it doesn't show in https://github.com/nbQA-dev/nbQA/tags)

the easiest short-term fix is to unbreak the tag in nbqa-dev/nbqa

Sorry @asottile , not sure what you mean by that, could you please clarify?

asottile commented 3 years ago

I believe the fix is git tag -d v0.5.8 && git push origin :v0.5.8 -- perhaps coupled with an annotated git tag --msg ... 0.5.8 if you want to retain an annotated tag

MarcoGorelli commented 3 years ago

Thanks - I tried that, but it fails:

$ git tag -d v0.5.8
error: tag 'v0.5.8' not found.
$ git push upstream :v0.5.8
error: unable to delete 'v0.5.8': remote ref does not exist
error: failed to push some refs to 'git@github.com:nbQA-dev/nbQA.git'

Indeed, I thought I'd already done that.

Maybe I should just do git tag -a 0.5.9 && git push upstream 0.5.9 at this point?

asottile commented 3 years ago

yeah probably best to just make another tag --

/ # git clone https://github.com/nbqa-dev/nbqa
Cloning into 'nbqa'...
remote: Enumerating objects: 79, done.
remote: Counting objects: 100% (79/79), done.
remote: Compressing objects: 100% (66/66), done.
remote: Total 2803 (delta 37), reused 30 (delta 13), pack-reused 2724
Receiving objects: 100% (2803/2803), 1.03 MiB | 3.00 MiB/s, done.
Resolving deltas: 100% (2001/2001), done.
/ # cd nbqa/
/nbqa # git describe --tags --abbrev=0
warning: tag '0.5.8' is externally known as 'v0.5.8'
v0.5.8-2-g007eb3cf60194030c5b4cf133beeada5009badfb
asottile commented 3 years ago

do you know what commands made that tag? I'm trying to bisect in git and having trouble creating the situation

MarcoGorelli commented 3 years ago

cool, have pushed 0.5.9, now pre-commit autoupdate is working fine on this repo

Thanks @asottile and @matthewfeickert , and sorry for the inconvenience I caused, I'll be more careful in the future!

asottile commented 3 years ago

no problem! git's the buggy thing here 😆

MarcoGorelli commented 3 years ago

do you know what commands made that tag? I'm trying to bisect in git and having trouble creating the situation

IIRC I followed https://stackoverflow.com/a/5719854/4451315 (but I missed the caveat at the bottom about annotated tags):

git tag new old
git tag -d old
git push origin new :old
asottile commented 3 years ago

cool, I'm able to reproduce it now

asottile commented 3 years ago

After bisect I come to this commit, which makes this change seem intentional:

ff165f00c1065d94c0bcab8708364e6dc5655f5d is the first bad commit
commit ff165f00c1065d94c0bcab8708364e6dc5655f5d
Author: Junio C Hamano <gitster@pobox.com>
Date:   Thu Feb 20 09:34:36 2020 -0800

    describe: force long format for a name based on a mislocated tag

    An annotated tag has two names: where it sits in the refs/tags
    hierarchy and the tagname recorded in the "tag" field in the object
    itself.  They usually should match.

    Since 212945d4 ("Teach git-describe to verify annotated tag names
    before output", 2008-02-28), a commit described using an annotated
    tag bases its name on the tagname from the object.  While this was a
    deliberate design decision to make it easier to converse about tags
    with others, even if the tags happen to be fetched to a different
    name than it was given by its creator, it had one downside.

    The output from "git describe", at least in the modern Git, should
    be usable as an object name to name the exact commit given to the
    "git describe" command.  Using the tagname, when two names differ,
    breaks this property, when describing a commit that is directly
    pointed at by such a tag.  An annotated tag Bob made as "v1.0" may
    sit at "refs/tags/v1.0-bob" in the ref hierarchy, and output from
    "git describe v1.0-bob^0" would say "v1.0", but there may not be
    any tag at "refs/tags/v1.0" locally or there may be another tag that
    points at a different object.

    Note that this won't be a problem if a commit being described is not
    directly pointed at by such a mislocated tag.  In the example in the
    previous paragraph, describing a commit whose parent is v1.0-bob
    would result in "v1.0" (i.e. the tagname taken from the tag object)
    followed by "-1-gXXXXX" where XXXXX is the abbreviated object name,
    and a string that ends with "-g" followed by a hexadecimal string is
    an object name for the object whose name begins with hexadecimal
    string (as long as it is unique), so it does not matter if the
    leading part is "v1.0" or "v1.0-bob".

    Show the name in the long format, i.e. with "-0-gXXXXX" suffix, when
    the name we give is based on a mislocated annotated tag to ensure
    that the output can be used as the object name for the object
    originally given to the command to fix the issue.

    While at it, remove an overly cautious dead code to protect against
    an annotated tag object without the tagname.  Such a tag is filtered
    out much earlier in the codeflow, and will not reach this part of
    the code.

    Helped-by: Matheus Tavares <matheus.bernardino@usp.br>
    Helped-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

 builtin/describe.c  | 15 +++++++++------
 t/t6120-describe.sh | 20 +++++++++++++++++++-
 2 files changed, 28 insertions(+), 7 deletions(-)
bisect run success
asottile commented 3 years ago

which first landed in git 2.27.0

asottile commented 3 years ago

I've asked about this on the mailing list: http://public-inbox.org/git/CA+dzEB=Ts=OdoYey+f-t+oLXAAv=f5hEJNpzb0G=LTpGPs+faw@mail.gmail.com/T/#u

asottile commented 3 years ago

not sure there's anything that can be done here -- I never got a response from the git mailing list -- will follow up if it happens again though