Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Git archives full support #580

Merged

Conversation

RonnyPfannschmidt
Copy link
Contributor

@RonnyPfannschmidt RonnyPfannschmidt commented Jun 8, 2021

fixes #65
fixes #578

@eli-schwartz
Copy link

Yay!

This should probably also deprecate https://github.com/pypa/setuptools_scm/#notable-plugins and conversely describe how to use .git_archival.txt since unlike hg it's not automatic...

@RonnyPfannschmidt
Copy link
Contributor Author

@eli-schwartz i noted another key issue with the approach - almost all the tags in the wild are non-annotated, git only supports this for annotated tags

i will have to eventually implement tools to help this mess :(

@eli-schwartz
Copy link

Historic tags cannot have %(describe) injected anyway, so this concern is limited to documenting and raising awareness that when adding full archive support, one must switch to annotated tags.

Personally, I would use annotated tags anyway, you need them for PGP signatures and they let you add release notes automatically etc.

Actually supporting them in git archive, would presumably entail adding an additional option to %(describe[:options]), e.g. don't use %(describe:exclude=badtag*), do use %(describe:exclude=badtag*,tags), and causing the "tags" option to tell the describe machinery inside git archive to have the effect of git describe --tags

@RonnyPfannschmidt
Copy link
Contributor Author

@eli-schwartz the problem is that it creates extra issue users have to be aware of and platforms don't necessarily support it nicely

so we can kind of expect to see users with light tags realizing the feature doesn't work for them over the next years as people start to use it

@eli-schwartz
Copy link

Ah, looking at src/setuptools_scm/git.py it appears setuptools_scm always assumed lightweight tags are candidates, which does seem unfortunately inconsistent.

The two solutions I can see are:

  • adding a warning/error whenever the .git/ directory and .git_archival.txt exists, the warning would check if the latest tag is not detected by git describe but is detected by DEFAULT_DESCRIBE with included --tags
  • posting to the git mailing list asking for an additional option specifying "tags", to be added in git 2.33.

@RonnyPfannschmidt
Copy link
Contributor Author

@eli-schwartz i believe both are necessary - an error will happen and inform the users of the missing annotated tags, as thats what they will have to use

@eli-schwartz
Copy link

So this just needs an addition to check what kind of tag the project is using and raise an error, and then it is good to merge?

@FFY00
Copy link
Member

FFY00 commented Sep 30, 2021

@RonnyPfannschmidt any updates on this?

@RonnyPfannschmidt
Copy link
Contributor Author

Unfortunately nope, other items took priority

skshetry referenced this pull request in iterative/dvc-s3-repo Oct 22, 2021
@eli-schwartz
Copy link

@RonnyPfannschmidt good news!

[eschwartz@arch ~/git/setuptools_scm]$ GIT_TRACE=1 ../git/pkg/bin/git --no-pager log -1 --format="%(describe:abbrev=15,match=v*,tags)"
22:38:02.585605 git.c:455               trace: built-in: git log -1 '--format=%(describe:abbrev=15,match=v*,tags)'
22:38:02.587021 run-command.c:668       trace: run_command: git describe --abbrev=15 '--match=v*' --tags 6a8dac10bb43de8a4a3bfaae5994c20c88249b18
v6.3.2-30-g6a8dac10bb43de8

I've implemented support for %(describe:tags,abbrev=15) and submitted the patches upstream, here: https://public-inbox.org/git/20211024014256.3569322-1-eschwartz@archlinux.org/T/

I'm currently at v2 of the patchset to resolve some review feedback, but the git maintainer likes the change overall so I have high hopes for it being in Git 2.34, which means that...

i will have to eventually implement tools to help this mess :(

Not anymore you don't. :)

@RonnyPfannschmidt
Copy link
Contributor Author

Very awesome, exciting, it's just a big yay moment

@eli-schwartz
Copy link

eli-schwartz commented Dec 17, 2021

I had originally missed the boat for this feature to land in git 2.34 (I was cutting it close to the feature freeze to begin with, and unfortunately it got held back until the next cycle).

So, I've been keeping an eye on its status waiting for some more definitive news. That news is now here. As of git/git@6ba65f4 my patchset is merged to git.git master and documented in the third batch as git/git@69a9c10#diff-4ddaeae40f2e69479747ed61ad12f9d74d1d9acd7dcc7c4298439987ff6d280cR41-R43

So, it is now guaranteed to be present (after a bit of waiting) in git 2.35. 🎉

@henryiii
Copy link
Contributor

henryiii commented Jan 25, 2022

When I read the 2.35 GitHub release announcement setuptools_scm is exactly what I was thinking of...

@RonnyPfannschmidt
Copy link
Contributor Author

@henryiii yup, @eli-schwartz did the awesome job on doing the git side, im very grateful for that

@eli-schwartz
Copy link

I'm very excited to see my change make it's way into production on GitHub and hopefully into a setuptools_scm version nearby! :D

;)

@eli-schwartz
Copy link

What's the status of this, BTW? :)

@RonnyPfannschmidt
Copy link
Contributor Author

Personal life got in the way, i hope to hack it together in the easter vacation, but the toddler has a "slight" priority

@RonnyPfannschmidt RonnyPfannschmidt force-pushed the git-archives-full-support branch 2 times, most recently from 422f83c to 64aa510 Compare April 15, 2022 15:24
@RonnyPfannschmidt
Copy link
Contributor Author

@paugier it seems that while fixing some typing issues i missed how i broke hg_git support - i tried some initial fixes but i hit a change blindness spot, please take a look

return None, None, None
hg_tags = hg_tags.split()
return _FAKE_GIT_DESCRIBE_ERROR
hg_tags: set[str] = set(hg_tags_str.split())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order is important. One cannot use a set.

hg_tags: list[str] = hg_tags_str.split()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, I've also seen people use a dict[str, None] for an ordered set, if you actually need the set behavior. Not ideal, but does work and is pretty fast. (Assuming you don't and list is better, but just pointing it out)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

list should be fine actually, thanks for pointing out, unfortunately not the fix yet

src/setuptools_scm/hg_git.py Outdated Show resolved Hide resolved
src/setuptools_scm/hg_git.py Outdated Show resolved Hide resolved
@RonnyPfannschmidt RonnyPfannschmidt merged commit 07270fc into pypa:main Jun 21, 2022
@RonnyPfannschmidt RonnyPfannschmidt deleted the git-archives-full-support branch June 21, 2022 11:43
@graingert
Copy link

I'm so excited to see this land and use this! Thank you so much for all you hard work on this

@RonnyPfannschmidt
Copy link
Contributor Author

This one has been dragging and screaming

@eli-schwartz
Copy link

eli-schwartz commented Jun 21, 2022

Yay, it's finally here!!! :D 🎉

@RonnyPfannschmidt
Copy link
Contributor Author

Release triggered, finally

Changaco added a commit to Changaco/setuptools_scm that referenced this pull request Jun 27, 2022
Completes pypa#580 by dropping the “Notable Plugins” section and creating a new “Git archives” subsection under “Builtin mechanisms for obtaining version numbers”.
hswong3i pushed a commit to alvistack/pypa-setuptools_scm that referenced this pull request Jun 29, 2022
Completes pypa#580 by dropping the “Notable Plugins” section and creating a new “Git archives” subsection under “Builtin mechanisms for obtaining version numbers”.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The reason to avoid including git_archive plugin into this package no longer applies Support git archives
8 participants