-
Notifications
You must be signed in to change notification settings - Fork 8
Added support for git submodules. #41
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
base: main
Are you sure you want to change the base?
Conversation
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 porting this over.
I think we should approach submodules by recognizing them by .gitsubmodules
instead of checking if the .git
is a file or a dir.
Additionally I have questions on how this behaves when doing a vcs pull
and vcs import
since there is no specific recognition on the output that these are submodules directories.
return cls._git_version | ||
|
||
@staticmethod | ||
def is_repository(path): |
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.
I think we should conform to recognizing submodules as how they are defined in the parent repository. On git this is by using the .gitmodules
file in the parent repository. See https://git-scm.com/docs/gitmodules.
If you check the contents of the .git
file that you are recognizing it is a link to the git dir on ../../.git/modules
.
With the example from the test of this PR:
cat .git
gitdir: ../../.git/modules/temp/vcstool
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.
Currently, the --nested
flag for vcs export
, recursively searches all the subdirectories of the parent directory and determines if the directory is a repository by using the is_repository
function.
I guess when the --nested
flag is active, instead of using recursion, this would be cleaner to determine all the submodules from the .gitmodules
file.
@leander-dsouza just a thought, but you might want to ping the original author of PRs that you port over. They may have been using a given feature internally and could have interesting feedback to share. |
|
Hello @varunsairam, just a gentle ping on this PR :) I was wondering if you could take a look at this PR and probably describe an internal use case for the change, as this is a direct port of your contribution over at vcstools. |
Thank you for the advice, Christophe :) |
Basic Info
Description of contribution in a few bullet points
.git
as a file instead of a folder, this change accounts for detecting it as a repository to clone into.Description of how this change was tested
Initialise a submodule locally within the repository under a
temp
folder.Compare the contents of
repo.yaml
before and after this change:Final result of the YAML file:
The submodule would not have been detected if this change were not present.
Signed-off-by: Leander Stephen Desouza [email protected]