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

git_helpers: add fallback for inconsistent describe #2

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

git_helpers: add fallback for inconsistent describe #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
02dd022
git_helpers: add fallback for inconsistent describe
proppy Dec 22, 2022
917639f
conda_build_prepare/git_helpers: use git tag instead of describe
proppy Dec 23, 2022
17e71e4
test: add git_helpers unit tests
proppy Dec 23, 2022
06ea4bd
test/test_git_helpers: remove unused imports
proppy Dec 23, 2022
41ecf06
test/test_git_helpers: add local git config
proppy Dec 23, 2022
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
11 changes: 11 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ on:
workflow_dispatch:

jobs:
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: '3.10'
- run: |
python -m pip install pytest
python -m pytest

build:
strategy:
fail-fast: false
Expand Down
9 changes: 5 additions & 4 deletions conda_build_prepare/git_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,11 @@ def _call_custom_git_cmd(git_repo, cmd_string, check=True, quiet=False):
return stdout.strip()


def get_latest_describe_tag(git_repo):
# Find the `git describe` tag having any version-like part
proppy marked this conversation as resolved.
Show resolved Hide resolved
def get_first_reachable_tag(git_repo):
try:
return _call_custom_git_cmd(git_repo, 'describe --tags --abbrev=0', quiet=True)
# list reachable tag sorted by commit date
tags = _call_custom_git_cmd(git_repo, 'tag --merged').splitlines()
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps it depends on the git configuration but for me this is sorted lexicographically.
For example for hidapi (https://github.com/libusb/hidapi) this is definitely not a valid replacement:

$ git describe --tags --abbrev=0
hidapi-0.13.1
$ git tag --merged
hidapi-0.1.0
hidapi-0.10.0
hidapi-0.10.1
hidapi-0.11.0
hidapi-0.11.1
hidapi-0.11.2
hidapi-0.12.0
hidapi-0.12.0-rc1
hidapi-0.12.0-rc2
hidapi-0.13.0
hidapi-0.13.1
hidapi-0.2.2
hidapi-0.2.3
hidapi-0.2.4
hidapi-0.3.0
hidapi-0.5.0
hidapi-0.5.1
hidapi-0.5.2
hidapi-0.6.0
hidapi-0.7.0
hidapi-0.8.0-rc1
hidapi-0.9.0
hidapi-0.9.0-rc1

I tried using different --sort options but they don't seem very stable to me (shouldn't creatordate be the same as taggerdate or committerdate?):

$ git tag --sort=taggerdate --merged
hidapi-0.1.0
hidapi-0.10.0
hidapi-0.10.1
hidapi-0.11.0
hidapi-0.11.1
hidapi-0.11.2
hidapi-0.12.0
hidapi-0.12.0-rc1
hidapi-0.12.0-rc2
hidapi-0.2.2
hidapi-0.2.3
hidapi-0.2.4
hidapi-0.3.0
hidapi-0.5.0
hidapi-0.8.0-rc1
hidapi-0.9.0
hidapi-0.9.0-rc1
hidapi-0.5.1
hidapi-0.5.2
hidapi-0.6.0
hidapi-0.7.0
hidapi-0.13.0
hidapi-0.13.1

$ git tag --sort=committerdate --merged
hidapi-0.13.0
hidapi-0.13.1
hidapi-0.5.1
hidapi-0.5.2
hidapi-0.6.0
hidapi-0.7.0
hidapi-0.1.0
hidapi-0.2.2
hidapi-0.2.3
hidapi-0.2.4
hidapi-0.3.0
hidapi-0.5.0
hidapi-0.8.0-rc1
hidapi-0.9.0-rc1
hidapi-0.9.0
hidapi-0.10.0
hidapi-0.10.1
hidapi-0.11.0
hidapi-0.11.1
hidapi-0.11.2
hidapi-0.12.0-rc1
hidapi-0.12.0-rc2
hidapi-0.12.0

$ git tag --sort=creatordate --merged
hidapi-0.1.0
hidapi-0.2.2
hidapi-0.2.3
hidapi-0.2.4
hidapi-0.3.0
hidapi-0.5.0
hidapi-0.5.1
hidapi-0.5.2
hidapi-0.6.0
hidapi-0.7.0
hidapi-0.8.0-rc1
hidapi-0.9.0-rc1
hidapi-0.9.0
hidapi-0.10.0
hidapi-0.10.1
hidapi-0.11.0
hidapi-0.11.1
hidapi-0.11.2
hidapi-0.12.0-rc1
hidapi-0.12.0-rc2
hidapi-0.12.0
hidapi-0.13.0
hidapi-0.13.1

The --sort=creatordate option seems to be what we look for but I'd need to test this on all our packages if it's really correct. I have a feeling the date of creation of either tag or commit won't necessarily point to the tag "closest" to the current HEAD.

Therefore I started thinking about other options.
Since for Xyce the Public_Release-7.6.0 name returned by git describe --tags is invalid I didn't find any way to get to the actual tag name from it.

Maybe we should simply call git describe --tags and grep the warning message to return the "is really '(.*)' here" name:

warning: tag 'Public_Release-7.6.0' is really 'Release-7.6.0' here

I don't like this option very much but at least we don't need to worry about tag sorting which might be tricky.
WDYT?

return tags[-1]
except subprocess.CalledProcessError:
return None

Expand Down Expand Up @@ -232,7 +233,7 @@ def git_rewrite_tags(git_repo):
print(f'\nRewriting tags in "{os.path.abspath(git_repo)}"...\n')

while True:
tag = get_latest_describe_tag(git_repo)
tag = get_first_reachable_tag(git_repo)
if tag is None:
# Add '0.0' tag on initial commit and skip rewriting.
git_add_initial_tag(git_repo)
Expand Down
43 changes: 43 additions & 0 deletions test/test_git_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import time

import conda_build_prepare.git_helpers as gh

COMMIT_DELAY = 1


def git_commit_and_tag(filepath, msg, tag=None):
now = time.time()
Copy link
Member

Choose a reason for hiding this comment

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

Why not use tempfile?

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 do use tmp_path fixture from pytest in the test function, see https://docs.pytest.org/en/7.1.x/how-to/tmp_path.html for more details.

The date here is used for the file content (not the name).

with open(filepath, 'w') as f:
f.write(f'{now}')
gh._call_custom_git_cmd(filepath.parent, f'add {filepath}')
gh._call_custom_git_cmd(filepath.parent, f'commit -m {msg}')
if tag:
gh._call_custom_git_cmd(filepath.parent, f'tag {tag}')
time.sleep(COMMIT_DELAY)
Copy link
Member

Choose a reason for hiding this comment

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

You might need to explain why you need time.sleep and COMMIT_DELAY?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

otherwise git get confused about the sorting with all the commit dates being the same (it doesn't got below second resolution).

I'll add a comment.



def test_get_first_reachable_tag(tmp_path):
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 you probably want to put the setup into a def setup_test_repo type function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean the git init + git config? Maybe we can do this when there is actually more than one test that share the same setup.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, mostly because it wasn't clear to see what was the actual test verse what was just needed to get things set up for the test.

repo = tmp_path
gh._call_custom_git_cmd(repo, 'init -b main')
gh._call_custom_git_cmd(
repo, 'config --local user.email "you@example.com"'
)
gh._call_custom_git_cmd(
repo, 'config --local user.name "Your Name"'
)
git_commit_and_tag(repo / 'foo', 'first', tag=None)

initial_rev = gh._call_custom_git_cmd(repo, 'rev-parse HEAD')
gh._call_custom_git_cmd(repo, 'checkout -b fixup')
git_commit_and_tag(repo / 'foo', 'unreachable', tag='v0.0-unreachabe')

gh._call_custom_git_cmd(repo, 'checkout main')
git_commit_and_tag(repo / 'foo', 'second', tag='v0.1')

git_commit_and_tag(repo / 'foo', 'untagged', tag=None)

gh._call_custom_git_cmd(repo, 'checkout main')
git_commit_and_tag(repo / 'foo', 'third', tag='v0.2')

Copy link
Contributor

Choose a reason for hiding this comment

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

To make sure this isn't just sorted lexicographically perhaps let's also add a v0.10 tag?

Copy link
Contributor

Choose a reason for hiding this comment

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

Otherwise the test file LGTM.

gh._call_custom_git_cmd(repo, f'tag v0.0-retroactive {initial_rev}')
assert gh.get_first_reachable_tag(repo) == 'v0.2'