Skip to content

feat: Make cz bump work on shallow clones #649

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: Make cz bump work on shallow clones #649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion commitizen/git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,22 @@ def get_tags(dateformat: str = "%Y-%m-%d") -> List[GitTag]:

def tag_exist(tag: str) -> bool:
c = cmd.run(f"git tag --list {tag}")
return tag in c.out
if tag in c.out:
return True

# In shallow clones (e.g. set up by Gitlab CI), the previous release tag might not
Copy link
Member

Choose a reason for hiding this comment

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

I'm good with this functionality. But one of the concerns I have is whether this is an unexpected action from the user point of view. I'm thinking of using a flag.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, this is more CI configuration, is up to the user, I'm fine with a warning and an explanation with the next steps to the user. But the user should ensure the environment (git) is providing commitizen what's needed, not the other way around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the reviews!

I agree with your concerns that auto-fetching missing tags is unexpected from the user point of view. Actually it would be cleanest to me if git tag --list {tag} had a flag to do this fetching for shallow clones automatically. But without auto-fetching missing tags in shallow clones, the only option would be to do a full clone, which can be quite slow for large/old repositories.

I believe both Gitlab CI and Github Actions use shallow clones by default. Because of that, Commitizen should ideally handle shallow clones out-of-the-box without setting an extra flag. Do you have examples which workflows could break if we fetch in shallow clones by default? I'm mostly worried about connection issues/the CI not setting up the Git remote for later fetching.

If you keep not being convinced, should we call the flag --auto-fetch and only add it for the cz bump command for now?

Copy link
Member

Choose a reason for hiding this comment

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

Hi @robertschweizer Sorry for late reply. I like the idea of the flag --auto-fetch. @woile What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, a flag and a setting. We can prompt the user in the init command

# be available locally, so try and fetch it.
if cmd.run("git rev-parse --is-shallow-repository").out.strip() == "true":
out.warn(
f"Could not find tag {tag} locally. Since this is a shallow clone, "
"trying to fetch it from all available remotes now."
)
for remote in cmd.run("git remote").out.strip().splitlines():
res = cmd.run(f"git fetch --depth=1 {remote} tag {tag}")
if res.return_code == 0:
return True

return False


def is_signed_tag(tag: str) -> bool:
Expand Down
29 changes: 29 additions & 0 deletions tests/commands/test_bump_command.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import inspect
import os
import re
import sys
from pathlib import Path
from typing import Tuple
from unittest.mock import MagicMock

Expand Down Expand Up @@ -377,6 +380,32 @@ def test_bump_major_version_zero_when_major_is_not_zero(mocker, tmp_commitizen_p
assert expected_error_message in str(excinfo.value)


def test_bump_shallow_clone(
tmp_commitizen_project,
tmp_path: Path,
mocker: MockFixture,
capsys: pytest.CaptureFixture,
):
# Set up repository with one released and one unreleased commit.
create_file_and_commit("feat: Add file for 0.1.0")
create_tag("0.1.0")
create_file_and_commit("feat: Add file for, but do not release 0.2.0")

# Shallow clone this local repository, so the release tag is missing.
cloned_repo = tmp_path / "cloned_repo"
# Use file:// URL, so Git does a shallow clone.
cmd.run(f"git clone --depth=1 file://{tmp_commitizen_project} {cloned_repo}")
os.chdir(cloned_repo)

# Bumping should work, but with a warning that we are pulling the tag from remote.
testargs = ["cz", "bump"]
mocker.patch.object(sys, "argv", testargs)
capsys.readouterr() # Reset capsys fixture
cli.main()
stdout, _stderr = capsys.readouterr()
assert re.search("Could not find tag.*trying to fetch", stdout)
Copy link
Member

Choose a reason for hiding this comment

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

Should we also check whether the function returns correctly after fetching?



def test_bump_files_only(mocker: MockFixture, tmp_commitizen_project):
tmp_version_file = tmp_commitizen_project.join("__version__.py")
tmp_version_file.write("0.1.0")
Expand Down