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.

Sign up for GitHub

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

Jump to bottom

WIP: Optionally omit the "g" for "git describe" #154

Closed
wants to merge 2 commits into from
Closed

WIP: Optionally omit the "g" for "git describe" #154

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08bb639
Prefer to only use 'git describe' for all required info
sschuberth Feb 24, 2017
35ea924
Omit the leading 'g' from Git revisions for consistency
sschuberth Feb 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 20 additions & 6 deletions setuptools_scm/git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import warnings

FILES_COMMAND = 'git ls-files'
DEFAULT_DESCRIBE = 'git describe --tags --long --match *.*'
DEFAULT_DESCRIBE = 'git describe --tags --long --dirty --match *.*'
Copy link
Contributor

Choose a reason for hiding this comment

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

does this work consistently, i very vaugely recall trouble with some older git versions that still may be in some distros

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least I've never had any issues with it.

Copy link
Contributor

Choose a reason for hiding this comment

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

thats ok, but "works for me" is not a metric end-users appreciate ^^

for examples see #134

Copy link
Member

Choose a reason for hiding this comment

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

I seem to recall the dirty flag was to distinguish a working directory with uncommitted changes from a clean working copy. I'm pretty sure you want the flag.

Agreed that this change doesn't belong in this request. If you want to make this change, propose it separately and trace its origin to prove that it's no longer needed (or in which conditions that's true).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you elaborate what exactly you mean by "trace its origin"?

Copy link
Member

Choose a reason for hiding this comment

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

I mean find when that change was made and why, to try to confirm its purpose. Perhaps using a blame tool or bisecting the changelog or searching the project ticket history.

Copy link
Member

Choose a reason for hiding this comment

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

I mean find when that change was made and why, to try to confirm its purpose. Perhaps using a blame tool or bisecting the changelog or searching the project ticket history.

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 wasn't understanding @RonnyPfannschmidt in the way that --dirty already used to be there, but did cause problems, and thus was removed. I thought that was just a general remark. But I'll investigate.

Copy link
Member

Choose a reason for hiding this comment

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

I think I misread the diff. Yeah, Ronnys comment is probably more meaningful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out --dirty indeed already was there: 929a5d7#diff-7a9c0c3ec91c440b46ba9f3506777bf0L7. Also see my PR #156.



def _normalized(path):
Expand Down Expand Up @@ -83,22 +83,36 @@ def parse(root, describe_command=DEFAULT_DESCRIBE, pre_parse=warn_on_shallow):
return
if pre_parse:
pre_parse(wd)
rev_node = wd.node()
dirty = wd.is_dirty()

if rev_node is None:
return meta('0.0', distance=0, dirty=dirty)

out, err, ret = do_ex(describe_command, root)
if ret:
# If 'git describe' failed, try to get the information otherwise.
rev_node = wd.node()
dirty = wd.is_dirty()

if rev_node is None:
return meta('0.0', distance=0, dirty=dirty)

return meta(
'0.0',
distance=wd.count_all_nodes(),
node=rev_node,
dirty=dirty,
)

# 'out' looks e.g. like 'v1.5.0-0-g4060507' or 'v1.15.1rc1-37-g9bd1298-dirty'.
if out.endswith('-dirty')
dirty = True
out = out[:-6]
else
dirty = False

tag, number, node = out.rsplit('-', 2)

# Omit the leading 'g' which is not part of the revision to be consistent with
# other code paths.
node = [1:]

number = int(number)
if number:
return meta(tag, distance=number, node=node, dirty=dirty)
Expand Down