-
Notifications
You must be signed in to change notification settings - Fork 33
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
Use "git describe" to append a local version label #262
Conversation
setup.py
Outdated
@@ -24,7 +41,7 @@ | |||
|
|||
setup( | |||
name='superflore', | |||
version='0.3.0', | |||
version=get_version_from_git_describe(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tfoote does this disrupt anything for releases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The release step that previously created a commit to change the value of version
is replaced by one that adds a tag. @allenh1 < Would this be too much of a disruption?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've dropped this controversial commit from the PR and added various other improvements instead. Please re-review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I intend to someday propose an alternative implementation that addresses Tully's objections, I now think it would make more sense to create a new PR for the bitbake generator changes and just have the setup.py commit be in this PR.
@tfoote please review this one -- you know quite a bit more than I do about this. |
@herb-kuta-lge FYI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a neat trick, however I have two conceptual problems with it.
First is that you should generally never use external resources in your setup.py It creates a depedency in the tool that you use to list your dependencies. In this case, if you don't have the python git module installed you cannot install the package.
Secondly this assumes that there's always a git repository behind the deployment. There are alternative deployment mechanisms that could be used such as a tarball or debian package, or pypi none of those would work.
Along those same lines if you have a local checkout, and you make a custom tagged version you could end up not knowing what "version" your running. And it's a real problem for browsing through the code a user cannot know what version of the code is deployed or installed on their system from an editor.
ac5479a
to
661e9e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be implemented by requiring that --dry-run
be specified when running superflore-gen-oe-recipes.
And README.md
needs to be updated.
661e9e3
to
6e9fc0c
Compare
@herb-kuta-lge requested to split this pull request into smaller ones with separate issue tickets and to leave this original one for setup.py change he plans to update with different implementation based on @tfoote's feedback. I've created: |
6e9fc0c
to
2d343ef
Compare
@tfoote < Please review the latest implementation. I believe it addresses all of your concerns. |
The commits my comments applied to were moved to other PR-s.
setup.py
Outdated
@@ -2,6 +2,51 @@ | |||
|
|||
import sys | |||
from setuptools import find_packages, setup | |||
import git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this line needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No; I'll remove it (and the next line).
"superflore-gen-oe-recipes" generates a setting containing the version of the program that was used to generate the recipes. So that it's possible to know whether a released version has been used, automatically append to the version a local version label based on the output from "git describe --tags --dirty --broken".
2d343ef
to
da479cf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the refactor it resolves my concerns.
…rastructure#262) "superflore-gen-oe-recipes" generates a setting containing the version of the program that was used to generate the recipes. So that it's possible to know whether a released version has been used, automatically append to the version a local version label based on the output from "git describe --tags --dirty --broken". Co-authored-by: Herb Kuta <[email protected]>
and small fix for superflore-datetime.inc.