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

ipython: add darwin-specific dependencies #10250

Merged
merged 4 commits into from
Oct 28, 2015

Conversation

timbertson
Copy link
Contributor

After the recent upgrade to jupyter (ipython 4), ipython fails on darwin due to missing packages which it tries to fetch during build. I've added these two pypi packages (appnope and gnureadline), and made ipython depend on them when stdenv.isDarwin. I've tested this locally, and it now works on darwin.

@@ -3759,7 +3778,7 @@ let
sha256 = "d8fb368c6a4dd58bc6cd5e61d4a12d119c4506fd344a371b3429b3ac2623b9ac";
};

propagatedBuildInputs = with self; [ pkgs.gnutls ];
propagatedbuildinputs = with self; [ pkgs.gnutls ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a bad edit...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops I think that was a vim-fail. Fixed now.

@timbertson timbertson force-pushed the ipython-darwin branch 2 times, most recently from 8baff95 to 3f2f28a Compare October 7, 2015 06:37
@timbertson
Copy link
Contributor Author

appnope only builds on darwin, so I've added that to its meta.platforms. gnureadline claims to work cross-platform, so it should be fine.

I've also removed the linux platform from pathpy, as the project works on windows (so surely it works cross-unix too).

/cc @bjornfor @FRidh

@FRidh
Copy link
Member

FRidh commented Oct 7, 2015

I think you need to disable gnureadline for pypy since it doesn't support extension modules. It seems like the reason Travis is failing now.

@timbertson
Copy link
Contributor Author

@FRidh thanks, for the tip - I've disabled it on pypy now.

I think travis actually failed because I missed the dependency on ncurses. gnureadline builds without it on darwin, but not on linux. I'm assuming that's actually due to impurities in the darwin stdenv, so I've included the dependency on all platforms now.

@FRidh
Copy link
Member

FRidh commented Oct 7, 2015

Good. Could you put appnope and gnureadline in separate commits following the contributors guide.
http://hydra.nixos.org/build/26614434/download/1/nixpkgs/manual.html#idm139961959517936

Also, appnope needs a disabled = !stdenv.isDarwin.

@timbertson
Copy link
Contributor Author

@FRidh Thanks, pulled out into separate commits now.

As for appnope, I've set meta.platforms = platforms.darwin - what does disabled do differently? There's one other use of platforms.darwin in python-packages.nix, and it doesn't use disabled for platform restrictions either.

@timbertson
Copy link
Contributor Author

Hmm, not sure why travis is failing now - it's the same code as a previously successful run, sliced into different commits. Looks like it's checking the PR, but a 'title' key is missing. Am I missing something from this PR, or is it just a transient issue?

@FRidh
Copy link
Member

FRidh commented Oct 7, 2015

https://nixos.org/wiki/Contributing

meta.platforms will tell Hydra to build binaries for specific platform targets. By default Hydra doesn't build anything. Don't set platforms.all or platforms.unix unless you've tested on both Darwin and Linux.

meta.platforms apparently says whether Hydra should build binaries, disabled whether or not it is a valid derivation at all.

@timbertson
Copy link
Contributor Author

@FRidh platforms isn't just used for hydra, though:

$ nix-build --no-out-link -A pythonPackages.appnope '<nixpkgs>'
error: Package ‘python2.7-appnope-0.1.0’ in ‘/home/tim/dev/nix/nixpkgs/pkgs/development/python-modules/generic/default.nix:65’ is not supported on ‘x86_64-linux’, refusing to evaluate.

This is why I needed to remove platforms.linux (in this PR) to get ipython building on darwin.

I'm happy to add it if it's necessary, but it seems like it isn't, and might confuse people if we start doing that now - currently all of the disabled expressions this file are for specific python versions / interpreters, even though there are plenty of packages with limited platforms.

@timbertson
Copy link
Contributor Author

So, anyone know why the travis build is failing? It's consistently getting:

=== Checking PR

Traceback (most recent call last):
    [ ... ]
  File "/home/travis/build/NixOS/nixpkgs/nox/nox/review.py", line 96, in review_pr
    click.style(payload['title'], bold=True)))
KeyError: 'title'

The command "./maintainers/scripts/travis-nox-review-pr.sh build" exited with 1.

@vcunat vcunat added the 6.topic: darwin Running or building packages on Darwin label Oct 9, 2015
@vcunat
Copy link
Member

vcunat commented Oct 9, 2015

Note that travis checks on our PRs tend to be unreliable and they're not a requirement for merging.

@timbertson
Copy link
Contributor Author

In that case, can this be merged?

@vcunat
Copy link
Member

vcunat commented Oct 28, 2015

I suppose so. Darwin users seem uninterested, unfortunately.

@vcunat vcunat merged commit 2e2b2c3 into NixOS:master Oct 28, 2015
vcunat added a commit that referenced this pull request Oct 28, 2015
It seems safe enough. I verified tarball builds at least.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants