conda-build
conda-build copied to clipboard
Add source.git_recursive key to enable/disable recursive cloning of submodules
closes #1210, #1733, #753
This PR adds something, that has already been discussed in PR #753 but stopped at one point. However, I think that there is still of interest (see #1733 for instance) and it is not difficult to implement.
This PR adds the source.git_recursive
item to the conda build metadata (meta.yaml
) that defaults to True
and can be set to False
in order to enable/disable the cloning of submodules when using source.git_url
.
Commits:
- https://github.com/conda/conda-build/commit/587202aa409643b632fc9a0fd8ded93cb693de0c adds the functionality
- https://github.com/conda/conda-build/commit/843133b86ba1fb19731cac934e0f745d3c23e0a1 adds a test
- https://github.com/conda/conda-build/commit/4b74d5b5561dcbdef8aaba1a3f2de21d1ca425f8 adds the docs
- https://github.com/conda/conda-build/commit/e140ea4f027395db7766aeb9252adc15b9290f8d adds the news entry
Usage:
Disable the recursive cloning of submodules via
source:
git_url: https://github.com/Chilipp/https://github.com/Chilipp/conda_build_test_recipe
git_recursive: false
We require contributors to sign our Contributor License Agreement, and we don't have one on file for @Chilipp. In order for us to review and merge your code, please e-sign the PDF at https://conda.io/en/latest/contributing.html#conda-contributor-license-agreement. We then need to manually verify your signature. We will ping the bot to refresh the PR status when we have confirmed your signature.
We require contributors to sign our Contributor License Agreement
Alright, signed it.
One more comment that I forgot: In order to test the implemented changes (see https://github.com/conda/conda-build/pull/3910/commits/843133b86ba1fb19731cac934e0f745d3c23e0a1) I added a submodule to my fork of https://github.com/conda/conda_build_test_recipe at https://github.com/Chilipp/conda_build_test_recipe which can be found at https://github.com/Chilipp/conda_build_test_recipe_submodule. I leave it to you, the maintainers, how you want this to be implemented. I can
- either transfer the ownership of this repository to the https://github.com/conda organization and make a PR for https://github.com/Chilipp/conda_build_test_recipe,
- or you set it up by yourself the way you want
Thank you so much for working on this. We'll get back to you on the repo ownership.
Cheers!
We require contributors to sign our Contributor License Agreement, and we don't have one on file for @Chilipp. In order for us to review and merge your code, please e-sign the PDF at https://conda.io/en/latest/contributing.html#conda-contributor-license-agreement. We then need to manually verify your signature. We will ping the bot to refresh the PR status when we have confirmed your signature.
We require contributors to sign our Contributor License Agreement, and we don't have one on file for @Chilipp. In order for us to review and merge your code, please e-sign the PDF at https://conda.io/en/latest/contributing.html#conda-contributor-license-agreement. We then need to manually verify your signature. We will ping the bot to refresh the PR status when we have confirmed your signature.
Hey @mingwandroid,
as this is still open for more than a month now, are there any interests to merge this? If not, shall we close this PR?
Note that the CI errors were not caused by the new features implemented in this PR but rather by problems in the master branch of this repo. I also signed the CLA, but if there has been any problems with it, let me know and I'll sign it again.
I am keen to get this looked at and resolved, yes, but I am also extremely busy, both with work on conda-build and other packages.
I did take a bit of a refresher look at this and have the following thoughts at present:
-
@gmarkall's original reason for wanting a separate way to disable submodules was because when we mirror a repo, we don't also mirror the submodules (I think, correct me if I am wrong). I wonder if that problem can be addressed via: https://stackoverflow.com/a/31627058
-
As @stuarteberg alluded it, I think, is that if it can be addressed as above, then the existing source/git_depth metadata value being 0 should be workable and would to me, be cleaner. @jakirkham mentioned that these are orthogonal, but I am not sure why a non-recursive clone cannot or should not be implemented as a clone of depth 0.
-
Even if the implementation cannot be done via the SO link above, I do not see that introducing a new separate metadata variable for this instead of just handling this as a special case when depth is set to 0 is better. Just thinking about the documentation for this would make me kind of queasy vs writing: "A git_depth of zero means that no submodules are mirrorer or cloned."
I don't think they necessarily should be orthogonal from the users' perspective but am partake in discussion around that. I definitely want to get this addressed.
- @gmarkall's original reason for wanting a separate way to disable submodules was because when we mirror a repo, we don't also mirror the submodules (I think, correct me if I am wrong). I wonder if that problem can be addressed via: https://stackoverflow.com/a/31627058
I can't recall many specifics, but I think the problem for me was that every build cloned the submodules from a remote machine rather than any local mirror.
- As @stuarteberg alluded it, I think, is that if it can be addressed as above, then the existing source/git_depth metadata value being 0 should be workable and would to me, be cleaner. @jakirkham mentioned that these are orthogonal, but I am not sure why a non-recursive clone cannot or should not be implemented as a clone of depth 0.
Setting it to 0 presumably has the same downside as setting it to 1? https://github.com/conda/conda-build/blob/master/docs/source/resources/define-metadata.rst#source-from-git - "The downside to setting it at 1 is that, unless the tag is on that specific commit, then you won't have that tag when you go to reference it in git_rev (for example). If your git_depth is insufficient to capture the tag in git_rev, you'll encounter an error. So in the example above, unless the 1.1.4 is the very head commit and the one that you're going to grab, you may encounter an error."
Hey @mingwandroid,
glad you're still interested :) It's also not only about mirroring a local repository. When I want to build a conda recipe from a remote git repository, I do not want to necessarily clone all the submodules (that may only exist for test reasons and are not necessary for the build).
Within the current version of conda-build, all the git submodules are cloned, if you use source.git_url
. Check the following
meta.yaml
package:
name: dummy-git-test
version: 0.0.1
source:
git_url: https://github.com/Chilipp/conda_build_test_recipe.git
build:
number: 1
script: ls submodule/README.md
requirements:
build:
- python
It does succeed (i.e. the submodule is checked out). For instance, I often put large binary test-data files in a submodule to keep it separate from source code. This test-data is irrelevant for conda-build, but running conda build
still clones it when specifying the git_url
as it is linked to the source code via submodule.
Concerning the source.git_depth
option. Not sure if I understand you correct but are you saying that setting source.git_depth: 0
means conda build
does not clone the submodules? This would be (1) a strange behaviour to me for a config value such as git_depth
, and (2) I also cannot confirm that this is true. The following recipe
meta.yaml
package:
name: dummy-git-test
version: 0.0.1
source:
git_url: https://github.com/Chilipp/conda_build_test_recipe.git
git_depth: 0
build:
number: 1
string: {{ GIT_BUILD_STR }}
script: ls submodule/README.md
requirements:
build:
- python
still clones the submodule as well.
Thanks @Chilipp, likely I am confusing two concepts! I would like to implement a git_submodule_depth thing here I think. Also when dealing with a local-filesystem path I do not think conda-build should bother with mirrors at all, or that should be configurable at least.
Alright, thanks for the clarification @mingwandroid. Sure, that sounds reasonable. In this case, git_submodule_depth: -1
would be a recursive update, I guess. So we replace
https://github.com/conda/conda-build/blob/80272d374a952fbc346634802b135c473675520e/conda_build/source.py#L293-L295
with
if git_submodule_depth == -1:
check_call_env([git, 'submodule', 'update', '--init',
'--recursive'], cwd=checkout_dir, stdout=stdout, stderr=stderr)
elif git_submodule_depth:
check_call_env([git, 'submodule', 'update', '--init',
'--depth', git_submodule_depth], cwd=checkout_dir, stdout=stdout, stderr=stderr)
does this sound okay?
Any update on this? :)
Vote up for this feature
Any progress on this PR? It would be a really useful feature.
cc @jezdez
Hi there, thank you for your contribution!
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further activity occurs.
If you would like this pull request to remain open please:
- Rebase and verify the changes still work
- Leave a comment with the current status
NOTE: If this pull request was closed prematurely, please leave a comment.
Thanks!