-
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 :) |
|
Apart from changing the detection of submodules, is there something missing for this? Would be great to have this functionality |
Basic Info
Description of contribution in a few bullet points
.gitas 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
tempfolder.Compare the contents of
repo.yamlbefore 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]