-
Notifications
You must be signed in to change notification settings - Fork 991
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
Check commits even with shallow clone of tags #15023
Conversation
Signed-off-by: John Sallay <jasallay@gmail.com>
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 your contribution @jsallay !
Some CI tests are failing, the new check is producing unexpected errors, it probably needs to be more conservative and raise only on very evident cases.
conan/tools/scm/git.py
Outdated
return True | ||
except Exception as e: | ||
# The error message contains the command and the commit if it can't find it. | ||
if str(e).count(commit) == 2: |
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 seems this check is not capturing all scenarios.
Signed-off-by: John Sallay <jasallay@gmail.com>
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 looks good, and tests are passing, that is great.
It would be great to add a test that validates the new code is working fine. Do you think you could try to add it? Let us know if you need help or guidance.
I've added in tests for a case where it should find the commit and one where it should not. Let me know if you have any other suggestions. |
It is great. I'll merge when the PR is green |
Changelog: Fix: Make
Git()
check commits in remote server even for shallow clones.Docs: omit
Fixes #14928
When performing a shallow clone of a tag (e.g.
git clone https://github.com/conan-io/conan --depth=1 -b 2.0.13
) it does not copy and of the branches or history. Thecommit_in_remote
method of theGit
class only checks local information. This PR will additionally check the remote if needed.I tested this by cloning a repo twice - one as a normal clone and the other with a shallow clone of a tag. I created an additional commit in each repo. I ran of total of 8 tests - checking all possibilities of:
The
git fetch
performed requires a connection to the remote repo. I wanted to make sure that it failed gracefully in an offline scenario. The function performed as expected in all cases.Without an internet connection, the function waits until the command times out and then throws an error (which is communicated to the user).
develop
branch, documenting this one.