Add `Platform.is` predicates and alias `stdenv.is*` to `hostPlatform`'s #25976

Ericson2314 · 2017-05-21T19:06:21Z

Motivation for this change

As discussed in #14965, The old notation is vague on which platform is being inspected. Worse, users probably mean the host platform, but in fact are inspecting the build platform.

This PR doesn't actually replace the old notation, but sets everything up so a simple/stupid replace can be done as a follow-up:

*Platform.is* shorthand is now defined
stdenv.is* is just taken from hostPlatform.is*

Also infer {build,host,target}.libc, which is a first step for the ideas in #6221

CC @vcunat @copumpkin

[@kmicklas Your branch we paired on the other day should be rebased on this one, once it's merged. (Otherwise will be lots of merge conflicts.)]

Things done

~~No hashes should be changed; need to check that.~~ I assume cause travis completed there was no-mass rebuild.

matthewbauer · 2017-05-21T19:28:38Z

Looks like probably the right thing.

My big concern is that in a lot of places where I've used stdenv.isDarwin what I really mean is can I use MacOS features. All of the darwin.apple_sdk.frameworks need to be available both on the hostPlatform and on the targetPlatform because they are just linking to the host's frameworks.

Another case that happens a lot is when you have inputs like systemd, alsa, and dbus that pretty much rely on Linux kernel features (although Homebrew somehow has gotten these to build). stdenv.isLinux when you actually probably want targetPlatform.isLinux.

vcunat · 2017-05-21T19:29:56Z

I wonder if it's worth to avoid the mass replace for now, assigning the hostPlatform meaning to stdenv. I haven't thought about it much recently; it's mostly a general avoidance of large code changes, as they tend to be costly later for various reasons, e.g. cherry-picking.

matthewbauer · 2017-05-21T19:35:04Z

pkgs/top-level/all-packages.nix

-    else if stdenv.isGlibc then glibcIconv stdenv.cc.libc
-    else if stdenv.isDarwin then darwin.libiconv
+  # GNU libc provides libiconv so systems with glibc don't need to build
+  # libiconv separately. Additionally, Apple forked/repackaged libiconv so we


Glibc also lets you leave out -liconv altogether leading to special cases like here:

nixpkgs/pkgs/tools/text/odt2txt/default.nix

Line 15 in 96d41e3

NIX_LDFLAGS = stdenv.lib.optionalString stdenv.isDarwin "-liconv";

Perhaps we can always have ```-liconv```` added to NIX_LDFLAGS in the hostPlatform.isDarwin case? Not sure how to do that though, probably a setup hook.

Hmm a few things

The newish glibcIconv is just some copped headers, so it's safe and simplest to depend on libiconv unconditionally.

As far as -l goes it reminds me of needing to link pthread or not. Maybe something to be done for both. Alternatively, maybe glibcIconv can include a dummy static lib or something, so we can unconditionally link it.

This PR preserves the existing native behavior, so it would be best to do setup hooks in a follow-up PR.

matthewbauer · 2017-05-21T19:36:37Z

pkgs/top-level/all-packages.nix

+                       then libcCross
+                       else stdenv.cc.libc)
+    else if hostPlatform.isDarwin
+      then darwin.libiconv


I guess this check has been here for a while but if you want to do mass changes to Nixpkgs, it might be useful to get rid of the ++ stdenv.lib.optional stdenv.isDarwin libiconv since this logic should handle it correctly. I can make a PR for it if that makes sense to you. Here's a list of some:

https://github.com/NixOS/nixpkgs/search?utf8=✓&q=stdenv.isdarwin+libiconv&type=

A very similar case to this is libintlOrEmpty where on glibc gettext/libintl is included but everywhere else it isn't. I really dislike the OrEmpty part, maybe we should give it the same treatment as this? See:

https://github.com/NixOS/nixpkgs/pull/25976/files/fbbab97b1c709f40172c255edc914d03efa6635a..a65c46dc79588ab6c601cd0a20ae59658a8f1104#diff-036410e9211b4336186fc613f7200b12L8680

matthewbauer · 2017-05-21T19:51:42Z

I am now noticing that here are no references to stdenv.is* in the manual! I guess it's fine if we also don't document this, but it might be helpful to have something on it especially for old-timers who have gotten used to stdenv.is*.

Ericson2314 · 2017-05-21T21:03:38Z

@vcunat I do think it's important to change it cause most ideoms spread via immitation, but let's save that for a follow-up PR—this one has some interesting things I wouldn't want lost in the noise of the sed anyways, while that can be big and boring.

Ericson2314 · 2017-05-22T03:43:48Z

@matthewbauer I believe you mean build and host platforms. If they both need to be darwin--in your example is it because impurities?---then requiring buildPlatform.isDarwin && hostPlatform.isDarwin is IMO worthwhile for the clarity ,

`hostPlatform.isDarwin` instead of `lib.system.parse.isDarwin hostPlatform.parsed`

This is especially useful when not cross compiling. It means we can remove the `stdenv.isGlibc` predicate too. Additionally, use this to simplify the logic to choose the appropriate libiconv derivation.

This is a saner default until stdenv's are removed altogether

Ericson2314 · 2017-05-22T04:28:08Z

Ok I think it's good for now. libiconv stuff and actual stdnv.is* purge are for follow-up PRs.

Ericson2314 · 2017-05-22T12:58:56Z

I think this is ready to merge?

Ericson2314 · 2017-05-22T16:52:23Z

Hmm so hah this will still be the build platform with cross, but at least the predicates are being defined less redundantly. I think doing the sed in the follow-up commit will be easier than fixing that here.

Ericson2314 · 2017-05-22T17:32:18Z

Sorry to not wait for confirmation on the merge, but I want some other follow-up stuff to be unblocked by this. Since stdenv.is* still exists, all the other concerns raised just are punted to the mass-replace follow up PR.

bjornfor · 2017-05-22T19:01:01Z

Sorry to not wait for confirmation on the merge, ...

As long as there is no breakage, I say go for it.

There are so few people in Nix community focussing on cross-compilation, I guess you are the one to confirm whether to merge :-)

Ericson2314 · 2017-05-22T19:34:12Z

@bjornfor that's sort of where I've arrived at, but better to hear it from others :). Thanks.

matthewbauer · 2017-05-22T19:39:36Z

@Ericson2314 Is there a good writeup on the host/build/target split? Is this a Nixpkgs specific construct or is it something for cross compilation in general? The main issue for me is what is the actual difference between host and build? target seems pretty self explanatory I think.

matthewbauer · 2017-05-22T19:49:21Z

One more thing: maybe you can send an email to the mailing list explaining what's going on? There's probably a lot of PRs in the works with stdenv.is that might get merged by unsuspecting maintainers. It would be helpful to just get things clarified to those not in the loop.

bjornfor · 2017-05-22T19:50:53Z

@matthewbauer: build/host/target is a GNU thing (autotools, binutils, gcc).

bjornfor · 2017-05-22T19:52:04Z

...and a general cross-compilation thing.

matthewbauer · 2017-05-22T19:56:36Z

build/host/target is a GNU thing (autotools, binutils, gcc)

Okay, forgive my ignorance on this.

For those out of the loop here's from the GCC manual:

There are three system names that the build knows about: the machine you are building on (build), the machine that you are building for (host), and the machine that GCC will produce code for (target).

If build, host, and target are all the same, this is called a native. If build and host are the same but target is different, this is called a cross. If build, host, and target are all different this is called a canadian (for obscure reasons dealing with Canada’s political party and the background of the person working on the build at that time). If host and target are the same, but build is different, you are using a cross-compiler to build a native for a different system. Some people call this a host-x-host, crossed native, or cross-built native. If build and target are the same, but host is different, you are using a cross compiler to build a cross compiler that produces code for the machine you’re building on. This is rare, so there is no common way of describing it. There is a proposal to call this a crossback.

https://gcc.gnu.org/onlinedocs/gccint/Configure-Terms.html

bjornfor · 2017-05-22T20:12:52Z

@Ericson2314: Are you exposing the full GNU build/host/target semantics to be used in package files? Or the simpler host/target variant? (Sorry, I haven't quite caught up with your changes, and the nixpkgs stdenv code has always been very confusing to me.)

Ericson2314 · 2017-05-23T00:11:20Z

@bjornfor Yes I am fully exposing them in all their glory to the terror and confusion of everyone :). I wrote some documentation of my own within http://nixos.org/nixpkgs/manual/#chap-cross which I hope is a bit more honest about the confusion and gotchas than GCC's or Autoconf's.

More generally with that doc, I want to have three sections basically corresponding to "how to package supporting cross" "how to build for cross" and "how does it work". The last would hopefully answer the bootstrapping questions...but I have yet to write it yet (in part because I am not yet happy done with it, e.g. #26004). I bet in part what you're wondering is answered by the middle section: while packages see all three, nixpkgs itself is configured with just 2, taking care to define build host and target properly behind the scenes.

@matthewbauer Yes, a mailing list post on all this stuff is vastly overdue. I've been sort of punting on that cause...more emails :). Certainly a breaking change like stdenv.is* being removed would warrant it, but as to stuff just affecting people cross compiling, I was still tempted to wait for a "big reveal" announcement when its all done, but then again I'd love to have more people involved.

Ericson2314 · 2017-05-23T00:12:29Z

For now, I make sure to tag everything with https://github.com/NixOS/nixpkgs/labels/6.topic%3A%20cross-compilation so people can follow that. Next up is #26007

lib tests: Simplify relative path for default argument

da8b2f1

Ericson2314 added 2.status: work-in-progress This PR isn't done 6.topic: cross-compilation Building packages on a different platform than they will be used on 9.needs: documentation labels May 21, 2017

matthewbauer reviewed May 21, 2017

View reviewed changes

Ericson2314 changed the title ~~WIP: Purge stdenv.is* platform predicates~~ WIP: Add *Platform.is* predicates and alias stdenv.is* to hostPlatform's May 22, 2017

Ericson2314 added 3 commits May 22, 2017 00:25

lib: Make platform predicates more ergonomic to use

2e7ec6f

`hostPlatform.isDarwin` instead of `lib.system.parse.isDarwin hostPlatform.parsed`

lib: Infer libc field of platform if not specified

c5c6606

This is especially useful when not cross compiling. It means we can remove the `stdenv.isGlibc` predicate too. Additionally, use this to simplify the logic to choose the appropriate libiconv derivation.

stdenv: define is* predicates with hostPlatform.is*

1dc6f15

This is a saner default until stdenv's are removed altogether

Ericson2314 force-pushed the no-stdenv-is branch from 78908f2 to 1dc6f15 Compare May 22, 2017 04:25

Ericson2314 removed the 2.status: work-in-progress This PR isn't done label May 22, 2017

Ericson2314 changed the title ~~WIP: Add *Platform.is* predicates and alias stdenv.is* to hostPlatform's~~ Add *Platform.is* predicates and alias stdenv.is* to hostPlatform's May 22, 2017

Ericson2314 removed the 9.needs: documentation label May 22, 2017

Ericson2314 merged commit 0d88299 into NixOS:master May 22, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `Platform.is` predicates and alias `stdenv.is*` to `hostPlatform`'s #25976

Add `Platform.is` predicates and alias `stdenv.is*` to `hostPlatform`'s #25976

Ericson2314 commented May 21, 2017 •

edited

Loading

matthewbauer commented May 21, 2017

vcunat commented May 21, 2017

matthewbauer May 21, 2017

Ericson2314 May 21, 2017

matthewbauer May 21, 2017

matthewbauer May 21, 2017 •

edited

Loading

matthewbauer commented May 21, 2017

Ericson2314 commented May 21, 2017

Ericson2314 commented May 22, 2017 •

edited

Loading

Ericson2314 commented May 22, 2017

Ericson2314 commented May 22, 2017

Ericson2314 commented May 22, 2017

Ericson2314 commented May 22, 2017

bjornfor commented May 22, 2017

Ericson2314 commented May 22, 2017 •

edited

Loading

matthewbauer commented May 22, 2017

matthewbauer commented May 22, 2017

bjornfor commented May 22, 2017

bjornfor commented May 22, 2017

matthewbauer commented May 22, 2017

bjornfor commented May 22, 2017

Ericson2314 commented May 23, 2017 •

edited

Loading

Ericson2314 commented May 23, 2017

Add *Platform.is* predicates and alias stdenv.is* to hostPlatform's #25976

Add *Platform.is* predicates and alias stdenv.is* to hostPlatform's #25976

Conversation

Ericson2314 commented May 21, 2017 • edited Loading

Motivation for this change

Things done

matthewbauer commented May 21, 2017

vcunat commented May 21, 2017

matthewbauer May 21, 2017

Choose a reason for hiding this comment

Ericson2314 May 21, 2017

Choose a reason for hiding this comment

matthewbauer May 21, 2017

Choose a reason for hiding this comment

matthewbauer May 21, 2017 • edited Loading

Choose a reason for hiding this comment

matthewbauer commented May 21, 2017

Ericson2314 commented May 21, 2017

Ericson2314 commented May 22, 2017 • edited Loading

Ericson2314 commented May 22, 2017

Ericson2314 commented May 22, 2017

Ericson2314 commented May 22, 2017

Ericson2314 commented May 22, 2017

bjornfor commented May 22, 2017

Ericson2314 commented May 22, 2017 • edited Loading

matthewbauer commented May 22, 2017

matthewbauer commented May 22, 2017

bjornfor commented May 22, 2017

bjornfor commented May 22, 2017

matthewbauer commented May 22, 2017

bjornfor commented May 22, 2017

Ericson2314 commented May 23, 2017 • edited Loading

Ericson2314 commented May 23, 2017

Add `Platform.is` predicates and alias `stdenv.is*` to `hostPlatform`'s #25976

Add `Platform.is` predicates and alias `stdenv.is*` to `hostPlatform`'s #25976

Ericson2314 commented May 21, 2017 •

edited

Loading

matthewbauer May 21, 2017 •

edited

Loading

Ericson2314 commented May 22, 2017 •

edited

Loading

Ericson2314 commented May 22, 2017 •

edited

Loading

Ericson2314 commented May 23, 2017 •

edited

Loading