Add relative path support to Git SCM tool #11407
Conversation
There was a problem hiding this comment.
Thanks very much for contributing to Conan.
Please have a look to the comments, and to the failing tests in CI: https://ci.conan.io/blue/organizations/jenkins/ConanTestSuite/detail/PR-11407/2/pipeline
| @@ -22,7 +22,7 @@ def get_commit(self): | |||
| # --full-history is needed to not avoid wrong commits: | |||
| # https://github.com/conan-io/conan/issues/10971 | |||
| # https://git-scm.com/docs/git-rev-list#Documentation/git-rev-list.txt-Defaultmode | |||
| commit = self._run('rev-list HEAD -n 1 --full-history -- "{}"'.format(self.folder)) | |||
| commit = self._run('rev-list HEAD -n 1 --full-history -- "."') | |||
There was a problem hiding this comment.
But this change will not guarantee that this is run in the specified self.folder, but in the cwd, which is not guarantee that it will be self.folder, if users specify a different value in the constructor, which they can.
There was a problem hiding this comment.
The definition of _run above executes with chdir(self.folder)
The double use of self.folder means it ends up trying to resolve a relative path relative to the folder itself, and usually fails to find any commits touching that subtree.
Given the other uses of _run, its behavior seems correct and the adjustment to get_commit seemed more appropriate.
There was a problem hiding this comment.
Oh, yes, you are right, I missed that part. Looks good to me then.
nmasseyKM
changed the title
nmasseyKM
changed the title
nmasseyKM
force-pushed
the
git-scm
branch
2 times, most recently
from
269b95c
to
9cfb364
Compare
| @@ -22,7 +22,7 @@ def get_commit(self): | |||
| # --full-history is needed to not avoid wrong commits: | |||
| # https://github.com/conan-io/conan/issues/10971 | |||
| # https://git-scm.com/docs/git-rev-list#Documentation/git-rev-list.txt-Defaultmode | |||
| commit = self._run('rev-list HEAD -n 1 --full-history -- "{}"'.format(self.folder)) | |||
| commit = self._run('rev-list HEAD -n 1 --full-history -- "."') | |||
There was a problem hiding this comment.
Oh, yes, you are right, I missed that part. Looks good to me then.
nmasseyKM
force-pushed
the
git-scm
branch
2 times, most recently
from
26e846f
to
5ad70e5
Compare
Add test for relative folder paths
- Test that submodules work as subfolders
Changelog: Feature: Add relative path support to Git SCM tool.
Docs: omit
Closes: #11408
When trying to setup Conan packaging with the target source in a submodule relative to the recipe, I was receiving
empty commit ids. Upon further investigation, it seems that the current assumption is that the folder path to Git()
will be absolute. A minor adjustment allows relative paths to be used as well: since the git command always
chdirs to thetarget git folder, we can perform the
rev-parserelative to the current directory and receive the expected behavior.folderNote: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.