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

Make submodule checks work under Python 3. #3387

Merged
merged 1 commit into from May 30, 2013

Conversation

takluyver
Copy link
Member

My shot at fixing gh-3385.

  • Ditch the imports from utils.submodule
  • Move the is_repo check up: distro packaging will often not be running from a git repository, in which case packagers won't need to do anything. If we're not in a git repository, there's nothing sensible we can do about submodules anyway.
  • Clearly label where packagers can patch to disable the submodule tests completely.

@minrk
Copy link
Member

minrk commented May 30, 2013

There is one case that the previous addressed that this no longer does: if someone downloads a tag from GitHub, it will be missing components and not be a repo. This should not be allowed to be installed, but this PR will allow an incomplete IPython to be installed without warning in that case.

@juliantaylor
Copy link
Contributor

the daily build patches out the runtime git submodule check and will probably continue to do so for robustness.
so if it causes issues you can revert the original merge that caused the problems instead.

@takluyver
Copy link
Member Author

It should be allowed to be installed from a github download - don't
forget the terminal IPython and the Qt console are perfectly usable without
the notebook static files. If there's a robust way of providing a warning
that those files are missing, we can do that, but it would need to:

  • Work without importing the rest of the codebase
  • Not require any interactive acknowledgement, because there are plenty of
    scenarios where you might be running setup.py without a user prodding it
    directly.

On 30 May 2013 17:53, Julian Taylor notifications@github.com wrote:

the daily build patches out the runtime git submodule check and will
probably continue to do so for robustness.
so if it causes issues you can revert the original merge that caused the
problems instead.


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

@minrk
Copy link
Member

minrk commented May 30, 2013

It should be allowed to be installed from a github download

No, it should not because you cannot download a complete sdist of IPython from GitHub.

@takluyver
Copy link
Member Author

You can download a package which can successfully install everything apart from the Notebook. We should make it clear why the Notebook isn't working, e.g. with a runtime check for the notebook static files, but we shouldn't completely cripple that file just because it doesn't work for the notebook. Especially since detecting the case to cripple it makes life more complicated.

@minrk
Copy link
Member

minrk commented May 30, 2013

it should be a safe assumption that any install of IPython is complete.

@takluyver
Copy link
Member Author

If you don't have tornado/pyzmq, you can install IPython but not run the notebook. The notebook static components function like just another dependency, except that they're in a git submodule. We can easily add a check that will prevent the notebook server from starting if you don't have those files, just like it doesn't start if tornado isn't installed.

Anyway, any objections to landing this PR?

@minrk
Copy link
Member

minrk commented May 30, 2013

The notebook static components function like just another dependency

Critical difference: you can't install ipython-components after the fact, you would have to remove and reinstall IPython. Only if we make the components a separate Python package would that be true.

Anyway, any objections to landing this PR?

No, I'll merge this now since it's critical, and figure out a way to prevent installation missing components later.

minrk added a commit that referenced this pull request May 30, 2013
Make submodule checks work under Python 3

introduces a regression in the submodule check for people who may have fetched a tarball from GitHub,
which will now install an incomplete IPython.
@minrk minrk merged commit bc29c16 into ipython:master May 30, 2013
@takluyver
Copy link
Member Author

Thanks.

The static components could be installed later, it's just slightly more complicated because you'd have to download, unzip and copy the files to the right location by hand. We could easily provide a wrapper script/function to automate that if it looks like many people will end up in that situation.

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Make submodule checks work under Python 3

introduces a regression in the submodule check for people who may have fetched a tarball from GitHub,
which will now install an incomplete IPython.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants