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

Convert to PEP-517 #609

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Convert to PEP-517 #609

wants to merge 5 commits into from

Conversation

vishwin
Copy link
Contributor

@vishwin vishwin commented May 18, 2023

Also use setuptools_scmversioningit for dynamic versioning

@vishwin vishwin force-pushed the master branch 7 times, most recently from 1c32b72 to ec34ec9 Compare May 20, 2023 01:22
@vishwin vishwin marked this pull request as ready for review May 20, 2023 01:45
@vishwin
Copy link
Contributor Author

vishwin commented May 20, 2023

Date-based version automatically set during build when using a full git clone. Otherwise (like git archives for system packaging or shallow clones), supply PKG-INFO:

Version: 20230520

Copy link
Owner

@kk7ds kk7ds left a comment

Choose a reason for hiding this comment

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

I'm not super enthusiastic about these changes (from the community, I appreciate you doing the work here) but alas. Is there some coming deadline with pip or python that makes it important for us to do this right now? Other than the fad aspect I mean :)

As before, I need some time to make sure I can get the build stuff working with this before we merge.

else:
import importlib.metadata as importlib_metadata

CHIRP_VERSION = importlib_metadata.version("chirp")
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not okay with this because means we can't use a version format that is compatible with everything else we have. We stamp versions in image files, send it with HTTP requests, etc. Python has decided to exercise control over package version number formats, which means we have to abide by it there, but it also means we will have to keep our internal versioning separate as a result. It's more than just what shows up in the about box.

We could consider adding separate stream variable and composing the full version like:

CHIRP_STREAM = "next"
CHIRP_VERSION = "%s-%s" % (CHIRP_STREAM, importlib_metadata.version("chirp"))

I'll have to change how I stamp the version into the packages in the builds and also see how it affects the bundles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just noticed that it was hardcoded to py3dev when I opened the about box so it seemed odd, especially with image formats and whatnot.

Copy link
Owner

Choose a reason for hiding this comment

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

It's just to keep the "I'm running this from git" separate from anything that looks like a build number. git-$hash will be fine and more descriptive.

pyproject.toml Outdated
[tool.versioningit.format]
distance = "{committer_date:%Y%m%d}"
dirty = "{committer_date:%Y%m%d}+d{build_date:%Y%m%d}"
distance-dirty = "{committer_date:%Y%m%d}+d{build_date:%Y%m%d}"
Copy link
Owner

Choose a reason for hiding this comment

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

Ah, I didn't pay attention here. I don't think I want to do this. I want to know when people are running from git versus one of the specific builds. Someone running in the middle of two commits that went out on a specific day will look like they're running the same thing as someone on a formal build but they're not. Also, some of our agreements with network services rely on our ability to enumerate builds within a certain window for anti-spam and anti-leech abilities. This will basically make it so that every day becomes a build number even though that's not true.

As mentioned, there's more to the versioning that we use internally than just display and it really matters to me (and others) that we're able to be specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This only works with a fully-cloned repository, ie full history. Otherwise this errors out since committer_date relies on a full history metadata to retrieve the commit date. When using an archive, shallow clone, etc, whatever is in PKG-INFO overrides any computed version.

Can also add the git hash in there to further delineate something built directly from git however.

Copy link
Owner

Choose a reason for hiding this comment

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

So we could make this just the githash then? or maybe git-$githash? That'd make me happy.

Copy link
Contributor Author

@vishwin vishwin May 23, 2023

Choose a reason for hiding this comment

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

Probably {committer_date}+{distance}.{vcs}{revision} to preserve commit order, since hashes are not ordered. Note that {count} refers to number of commits since last tag, which is release_0_4_0.

Copy link
Owner

Choose a reason for hiding this comment

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

Hash is far more useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

{revision} is the hash.

@vishwin
Copy link
Contributor Author

vishwin commented May 21, 2023

I'm not super enthusiastic about these changes (from the community, I appreciate you doing the work here) but alas. Is there some coming deadline with pip or python that makes it important for us to do this right now? Other than the fad aspect I mean :)

Not a fad, but policy, just not enforced until now for some reason.

@kk7ds
Copy link
Owner

kk7ds commented May 22, 2023

I'm not super enthusiastic about these changes (from the community, I appreciate you doing the work here) but alas. Is there some coming deadline with pip or python that makes it important for us to do this right now? Other than the fad aspect I mean :)

Not a fad, but policy, just not enforced until now for some reason.

The policy is only for things that exist in the python packaging ecosystem and they very definitely don't have the ability to enforce that on us. Just because we're written in python does not mean we need to exist in and abide by all the overly-draconian rules. I'm really only interested in doing as much as is required to avoid making it difficult for people that have to install via pip.

@vishwin
Copy link
Contributor Author

vishwin commented Aug 11, 2024

The policy is only for things that exist in the python packaging ecosystem and they very definitely don't have the ability to enforce that on us. Just because we're written in python does not mean we need to exist in and abide by all the overly-draconian rules. I'm really only interested in doing as much as is required to avoid making it difficult for people that have to install via pip.

As long as you are using any standards-compliant Python package build backend, you are in the Python packaging ecosystem, even if not in PyPI. And in the case of pip, some of us operating systems/distributions cannot use pip for this purpose, but other standards-compliant tooling.

Use versioningit for dynamic versioning
Those using git archives and shallow clones should add PKG-INFO corresponding to the date version. Example:

Version: 20230520
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.

2 participants