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

meson: 0.46.1 → 0.48.2 #46020

Merged
merged 9 commits into from Nov 18, 2018
Merged

meson: 0.46.1 → 0.48.2 #46020

merged 9 commits into from Nov 18, 2018

Conversation

@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Sep 3, 2018

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@jtojnar
Copy link
Contributor Author

@jtojnar jtojnar commented Sep 3, 2018

@dtzWill I ran a GNOME session built with this and did not experience any problems. Tests are passing too. What issues did you encounter again?

@dtzWill
Copy link
Contributor

@dtzWill dtzWill commented Sep 4, 2018

We patch(ed?) meson to change how it writes RPATH, which caused particularly painful (and not immediately obvious) results since apparently it works like this:

  • build using dummy rpath that's "surely" long enough
  • afterwards "blindly" replace dummy path with actual path, which is "known to be safe" since dummy was long enough.

It's a hack in the first place but we managed to very much break things by replacing paths of length ~30 ("dummy") with ones very much longer (> 200) which of course often resulted in invalid rpath searching but especially bad since those extra 170 chars or whatever overwrote whatever followed, which uhh hopefully wasn't important or something ;).

Bit more details here, although I was a bit battle-weary so didn't explain it very well:
#44040

As I recall, all the relevant parts of meson are the ones we already touch with our patches. I think. :).

@dtzWill
Copy link
Contributor

@dtzWill dtzWill commented Sep 4, 2018

In particular, everything was broken: #44040 (comment)

But it'd be good to ensure we're not generating corrupt binaries or at least be somewhat confident that's unlikely to happen....

@hedning
Copy link
Contributor

@hedning hedning commented Sep 7, 2018

We have a testing ground for meson now #45950, https://hydra.nixos.org/jobset/nixpkgs/gnome

Seems like things are working better than last time at least, I'm eg. able to run nixos.tests.login sucessfully. nixos.tests.systemd fails one of two tests, not sure if it's related though.

Doing this produces identical results, where nixpkgs.systemd is from master and ~/nixpkgs is the gnome branch with the meson update:

diff <(readelf -d $(nix eval nixpkgs.systemd --raw)/lib/libnss_resolve.so.2) \
     <(readelf -d $(nix eval -f ~/nixpkgs systemd --raw)/lib/libnss_resolve.so.2)

@dtzWill any tips on how to check if the rpath is wrong?

@jtojnar jtojnar mentioned this pull request Sep 10, 2018
3 of 9 tasks complete
@xeji xeji mentioned this pull request Sep 20, 2018
@jtojnar jtojnar force-pushed the jtojnar:meson-0.47 branch from 5393183 to 89d4454 Sep 25, 2018
@jtojnar jtojnar changed the title meson: 0.46.1 → 0.47.2 meson: 0.46.1 → 0.48.0 Sep 25, 2018
@jtojnar jtojnar mentioned this pull request Sep 25, 2018
1 of 9 tasks complete
@jtojnar
Copy link
Contributor Author

@jtojnar jtojnar commented Sep 25, 2018

It looks like ./gir-fallback-path.patch is no longer needed?

$ rg shared-library $(nix-build -A atk.dev)/share/gir-1.0/Atk-1.0.gir
14:             shared-library="/nix/store/iiamyk9c1j8bzlxrf3lbz9g45xb8zlgd-atk-2.30.0/lib/libatk-1.0.so.0"
$ rg shared-library $(nix-build -A gst_all_1.gstreamer.dev)/share/gir-1.0/Gst-1.0.gir
16:             shared-library="/nix/store/drpcbmhq0znjn86k1xzij947kb247hr1-gstreamer-1.14.2/lib/libgstreamer-1.0.so.0"
$ rg shared-library $(nix-build -A gst_all_1.gstreamer.dev)/share/gir-1.0/Gst-1.0.gir
16:             shared-library="/nix/store/drpcbmhq0znjn86k1xzij947kb247hr1-gstreamer-1.14.2/lib/libgstreamer-1.0.so.0"
@jtojnar jtojnar force-pushed the jtojnar:meson-0.47 branch from 204ad76 to fbb03ad Sep 25, 2018
@jtojnar jtojnar force-pushed the jtojnar:meson-0.47 branch from fbb03ad Oct 14, 2018
@jtojnar jtojnar added this to In Progress in GNOME Oct 15, 2018
@jtojnar
Copy link
Contributor Author

@jtojnar jtojnar commented Oct 15, 2018

@dtzWill at #45950 (comment) wrote:

NixOS's extraUtils fails with this due to reference that's not allowed-- at first I feared this was the same problem as before (since the symptom is the same as when we were corrupting things re:meson), but I think it's unrelated :). Regardless I suspect you're running into this and if you haven't you probably will (or have fixed it and I'm on wrong branch :D).

In short the fix is adding -Dlink-udev-shared=false to systemd's mesonFlags. I didn't chase down why it suddenly started happening, but I can say I noticed in past few months various other distributions making changes regarding this so perhaps it's due to a meson upgrade they did before us :).

The harder fix, and untested, is that the extraUtils find-libs-and-copy-and-patchelf stuff is both too complicated and in particular fails to account for transitive dependencies (libraries that need other libraries). Well "fails" may be incorrect, this is possibly a feature and some would argue a case of underlinking but all I mean is this wouldn't happen if the xz dep was pulled in and patched to be used.

I think the solution proposed (meson flags) is the best as it prevents general systemd deps (== take over the world) from being pulled into our early boot process. We may want to do the same for systemctl (similar option) but that's not necessary so I'll leave that for another time.

Anyway thanks for your work, hope this helps, LMK if you have any questions or whatnot :).

I still cannot reproduce this failure when trying to build the following expression:

{pkgs ? import <nixpkgs> {} }:
let
  config.boot.loader.supportsInitrdSecrets = false;
  config.boot.isContainer = false;
  config.boot.initrd.secrets = {};
  config.boot.initrd.extraUtilsCommands = "";
  config.boot.initrd.extraUtilsCommandsTest = "";
  config.system.build.fileSystems = [];
  config.systemd.package = pkgs.systemd;
  inherit (pkgs) lib;

  utils = import <nixpkgs/nixos/lib/utils.nix> pkgs;
  stage-1 = import <nixpkgs/nixos/modules/system/boot/stage-1.nix> { inherit pkgs lib utils config; };

in stage-1.config.content.system.build.extraUtils
@jtojnar jtojnar force-pushed the jtojnar:meson-0.47 branch to a5b1e2a Oct 17, 2018
@dtzWill
Copy link
Contributor

@dtzWill dtzWill commented Oct 17, 2018

I still cannot reproduce this failure when trying to build the following expression:

Using that expression fails for me building against the gnome PR (where I commented) and when setting NIX_PATH appropriately, I'll try it against this PR shortly.

@hedning
Copy link
Contributor

@hedning hedning commented Nov 10, 2018

Yeah, you're probably right. We're most likely already violating the assumptions made by meson when we're moving the stuff added by ld-wrapper to the start. So I don't really have any problems with going with the current solution. Just saw that you've already pushed the fix to the the gnome branch 👍

@jtojnar
Copy link
Contributor Author

@jtojnar jtojnar commented Nov 10, 2018

I cannot reproduce the issue at the moment.

This was referenced Nov 10, 2018
@jtojnar
Copy link
Contributor Author

@jtojnar jtojnar commented Nov 11, 2018

Based on #45950 (comment), our gir-fallback-path.patch is apparently still needed. The old patch is not really trivially portable, since our --fallback-library-path hack does not expect multiple libraries that are now supported.

@jtojnar jtojnar force-pushed the jtojnar:meson-0.47 branch 2 times, most recently to 25aaecf Nov 11, 2018
@jtojnar
Copy link
Contributor Author

@jtojnar jtojnar commented Nov 11, 2018

Re-added the patch.

jtojnar added 9 commits Sep 2, 2018
Meson no longer propagates it so we need to re-add it.
Meson no longer propagates it so we need to re-add it.
Meson no longer propagates it so we need to re-add it.
Meson no longer propagates it so we need to re-add it.
Meson no longer propagates it so we need to re-add it.
Meson no longer propagates it so we need to re-add it.
Meson no longer propagates it so we need to re-add it.
@jtojnar jtojnar force-pushed the jtojnar:meson-0.47 branch from 25aaecf to 927a82d Nov 13, 2018
@coreyoconnor
Copy link
Contributor

@coreyoconnor coreyoconnor commented Nov 13, 2018

FYI: I also ran into the is not allowed to refer to error previously. I'm unable to reproduce the issue with the latest commits.

@jtojnar jtojnar changed the title meson: 0.46.1 → 0.48.1 meson: 0.46.1 → 0.48.2 Nov 17, 2018
@jtojnar jtojnar changed the base branch from master to staging Nov 17, 2018
@hedning
Copy link
Contributor

@hedning hedning commented Nov 18, 2018

(from irc) @jtojnar > hedning_: I am suspicious because you said you reproduce it after I pushed the current fix

Ah, sorry for the confusion, I should've made it clear that I was talking about the branch without the clear old_rpath fix. I was working under the assumption that the fix wasn't safe, but when reading #46020 (comment) again I can see I didn't specify that at all :(

@jtojnar
Copy link
Contributor Author

@jtojnar jtojnar commented Nov 18, 2018

Let's merge this and see what happens.

@jtojnar jtojnar merged commit 85bd2a7 into NixOS:staging Nov 18, 2018
9 checks passed
9 checks passed
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-nixos-manual nix-instantiate ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
GNOME automation moved this from In Progress to Done Nov 18, 2018
@jtojnar jtojnar deleted the jtojnar:meson-0.47 branch Nov 18, 2018
@hedning hedning mentioned this pull request Nov 19, 2018
3 of 9 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
GNOME
  
Done
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants