Skip to content
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

Git submodule support #577

Merged
merged 5 commits into from
Jun 14, 2012
Merged

Git submodule support #577

merged 5 commits into from
Jun 14, 2012

Conversation

fin
Copy link
Contributor

@fin fin commented Jun 14, 2012

This extends #545 with tests, as promised. Thanks @lepture for the actual implementation (which I did not touch).

@fin
Copy link
Contributor Author

fin commented Jun 14, 2012

oh, this only works with git > 1.7.3, because i used

git submodule foreach

@travisbot
Copy link

This pull request fails (merged 6a83076 into ae867db).

@fin
Copy link
Contributor Author

fin commented Jun 14, 2012

huh. i can't reproduce the errors in non-2.5 versions. is the build server running a current-ish version of git?

older versions seem to abuse stderr a little more than current ones:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=447395

anyway, it looks like this is an issue with pip install not adding -q to git the git commands. not related to this pull request …

@carljm
Copy link
Contributor

carljm commented Jun 14, 2012

This is looking good, thanks for working on it! I think the git submodule foreach could be removed pretty easily in exchange for making the submodule-update code a bit less generic and more specific to the particular situation. This seems like a reasonable exchange to me, given that assumptions about the name/path/location of the submodule are already baked in to other helper functions anyway.

Also, we need to address the failure of this test on Travis - is it passing for you locally? It looks like a simple case of needing to add expect_error=True to the run_pip invocation, since git outputs some information on stderr. Or perhaps passing -q to git would be another way to address this?

…ct errors in pip install because old versions of git write senseless things to stderr
@travisbot
Copy link

This pull request fails (merged 65d8b62 into ae867db).

@fin
Copy link
Contributor Author

fin commented Jun 14, 2012

@carljm you're right in both cases, i changed the tests to reflect that feedback. can we ping @travisbot to re-test this manually?

@carljm
Copy link
Contributor

carljm commented Jun 14, 2012

Looks good! I guess the comment is kind of off, it's stderr output from git that's the problem, not stdout. But anyway, travis should retest this soon automatically since you pushed to it, I'll just wait for it.

@travisbot
Copy link

This pull request passes (merged 4c5573c into ae867db).

@bergundy
Copy link
Contributor

Hey, I just installed pip master, and my submodules don't update recursively.

I managed to get it to work by changing:
$ git submodule init -q
$ git submodule update --recursive -q
to:
$ git submodule update --init --recursive -q

Any reason why you're not running the second option? is there something I'm missing?

@carljm
Copy link
Contributor

carljm commented Sep 12, 2012

@bergundy no reason I know of, seems like both should work though, do you know why the two-line version isn't working for you?

@bergundy
Copy link
Contributor

I don't know exactly why, but without it, I get the following error.
error: package directory 'submod1/submod2' does not exist

I tried running the git commands myself and they don't work recursively.

On Thu, Sep 13, 2012 at 2:53 AM, Carl Meyer notifications@github.comwrote:

@bergundy https://github.com/bergundy no reason I know of, seems like
both should work though, do you know why the two-line version isn't working
for you?


Reply to this email directly or view it on GitHubhttps://github.com//pull/577#issuecomment-8514202.

@carljm
Copy link
Contributor

carljm commented Sep 13, 2012

Hmm, I guess maybe the init isn't happening in the second-level submodule. Feel free to submit a pull request to switch to the one-line version.

@bergundy
Copy link
Contributor

submitted #674

qris added a commit to aptivate/pyquip that referenced this pull request Apr 7, 2014
It doesn't work, and apparently already fixed by
pypa/pip#577.
qris added a commit to aptivate/pyquip that referenced this pull request Apr 7, 2014
It doesn't work, and apparently already fixed by
pypa/pip#577.
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants