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 for development versions #6935
Conversation
- VERSION will get bumped with a release, not directly after a release, e.g. v2.075.0-8aeca3c (somewhat before v2.075.0) => v2.074.1-456-g8aeca3c (456 commits after v2.074.1) - VERSION file will get updated in stable and gets merged back into master - ensures v2.074.1-ddd-hhhhh contains all changes from v2.074.1
Thanks for your pull request, @MartinNowak! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
|
||
VERSION := $(shell cat ../VERSION) # default to checked-in VERSION file | ||
ifneq (1,$(RELEASE)) # unless building a release | ||
VERSION_GIT := $(shell printf "`$(GIT) describe --dirty`") # use 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.
The --dirty
option is supported since 2009.
git/git@9f67d2e
######## VERSION | ||
|
||
VERSION := $(shell cat ../VERSION) # default to checked-in VERSION file | ||
ifneq (1,$(RELEASE)) # unless building a release |
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.
Doesn't this line do the exact opposite of what the comment says?
In other places in the makefile ifneq (,$(RELEASE))
is used to check if this is a release is build.
Wouldn't it make sense to always try git describe
to get the version number? It only returns the tag if the current HEAD is tagged.
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.
It's ifneq (1,$(RELEASE))
and I don't want to remove with the ability to override VERSION
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.
Ok, but going on the other use of RELEASE in the makefile it is 1 when building a release.
https://github.com/MartinNowak/dmd/blob/c28e2d8cfebfcc5b2fb8ee4b35bfb661314bc042/src/posix.mak#L178
So ifneq (1,$(RELEASE))
would indicate "Do this when not building a release", which doesn't match the comment. This would mean either the code or the comment is wrong.
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 comment says unless building a release
== if not building a release
, no problem here.
VERSION := $(shell cat ../VERSION) # default to checked-in VERSION file | ||
ifneq (1,$(RELEASE)) # unless building a release | ||
VERSION_GIT := $(shell printf "`$(GIT) describe --dirty`") # use git describe | ||
ifneq (,$(VERSION_GIT)) # check for git failures |
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 CircleCi failure for Phobos stable branches are due to shell
errors leading to direct abortion of the build process.
See also: https://www.gnu.org/software/make/manual/html_node/Errors.html
After each shell invocation returns, make looks at its exit status. If the shell completed successfully (the exit status is zero), the next line in the recipe is executed in a new shell; after the last line is finished, the rule is finished.
If there is an error (the exit status is nonzero), make gives up on the current rule, and perhaps on all rules.
It looks like this applies to shell
invocations as well.
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.
Just try it, e.g. changing describe to describes, it continues to run.
Guess this only applies to shell invocations in make rules, but not top-level $(shell)
calls.
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.
Just try it, e.g. changing describe to describes, it continues to run.
Did you nuke generated
before trying this?
It doesn't work for me, because generating VERSION
fails ...
v2.075.0-8aeca3c (somewhat before v2.075.0) =>
v2.074.1-456-g8aeca3c (456 commits after v2.074.1)
As discussed here (http://forum.dlang.org/post/dlbbqegxkblvhakfsogz@forum.dlang.org) getting rid of an error-prone (and home-brewed) development version scheme.