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

pyopengl: fix runtime shared library loading failure with 3.1.4 #79154

Merged
merged 1 commit into from Feb 24, 2020

Conversation

@guibou
Copy link
Contributor

@guibou guibou commented Feb 3, 2020

This closes #76822.

pyopengl 3.1.4 introduced a new logic for shared library loading: it
tests a few combinations of library name and suffix (such as .so.X).

Our previous patch was just replacing the library name (e.g. 'glut') by
the full path to the nix store. This does not work anymore with pyopengl
3.1.4 new heuristic.

This commit just keep the behavior of pyopengl but adds the nix store
path to the list of tried paths.

I tested with friture which was failing at build time before this commit and is now building. I also tested pyqtgraph which was failing at runtime and is now working.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@guibou guibou requested review from FRidh and jonringer as code owners Feb 3, 2020
@ofborg ofborg bot added the 6.topic: python label Feb 3, 2020
@guibou guibou changed the title pyopengl: fix runtime failure with 3.1.4 pyopengl: fix shared library loading failure with 3.1.4 Feb 3, 2020
@guibou guibou changed the title pyopengl: fix shared library loading failure with 3.1.4 pyopengl: fix runtime shared library loading failure with 3.1.4 Feb 3, 2020
Copy link
Contributor

@jonringer jonringer left a comment

commit msg should be:

pythonPackages.pyopengl: fix runtime shared library loading failure

otherwise LGTM

This closes #76822.

pyopengl 3.1.4 introduced a new logic for shared library loading: it
tests a few combinations of library name and suffix (such as .so.X).

Our previous patch was just replacing the library name (e.g. 'glut') by
the full path to the nix store. This does not work anymore with pyopengl
3.1.4 new heuristic.

This commit just keep the behavior of pyopengl but adds the nix store
path to the list of tried paths.
@guibou guibou force-pushed the guibou:fix_pyopengl branch from cab5c0d to f24942b Feb 3, 2020
@guibou
Copy link
Contributor Author

@guibou guibou commented Feb 3, 2020

@jonringer done.

@guibou guibou requested a review from jonringer Feb 3, 2020
@jonringer
Copy link
Contributor

@jonringer jonringer commented Feb 3, 2020

this may want to be upstreamed

@guibou
Copy link
Contributor Author

@guibou guibou commented Feb 3, 2020

@jonringer I don't know what can be upstreamed here, but I'll think about it.

Do you think this is a blocker for this commit?

@jonringer
Copy link
Contributor

@jonringer jonringer commented Feb 3, 2020

this could be upstreamed:

    substituteInPlace OpenGL/platform/ctypesloader.py \
      --replace "filenames_to_try = []" "filenames_to_try = [name]"

from your comment it seems that they went away from this approach. I'm not sure how name is derived, but I'm assuming checking 1 additional item wouldn't hurt too much.

not a blocker, stuff like this happens all the time.

@jonringer
Copy link
Contributor

@jonringer jonringer commented Feb 3, 2020

It's taking a while to review because pyopengl is used by some heavy-weight packages, and my laptop I travel with isn't super powerful.

[6/26/73 built (12 failed), 821 copied ...
@guibou
Copy link
Contributor Author

@guibou guibou commented Feb 23, 2020

Any update?

@jonringer
Copy link
Contributor

@jonringer jonringer commented Feb 23, 2020

sorry, there was a bunch of blocking issues with some downstream packages which made it hard to do a review

Copy link
Contributor

@jonringer jonringer left a comment

diff LGTM

no regressions

[62 built (6 failed), 1 copied (513.2 MiB), 71.2 MiB DL]
error: build of '/nix/store/c9niqv7qhgh21hi2bplyy4by4fk9dagm-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/79154
12 package marked as broken and skipped:
cq-editor gazebo gazebo-headless gazeboSimulator.gazebo6 gazeboSimulator.gazebo6-headless gazeboSimulator.gazebo7 gazeboSimulator.gazebo7-headless python27Packages.runsnakerun python27Packages.squaremap python37Packages.spyder python38Packages.spyder spyder

5 package failed to build:
ib-controller python37Packages.rl-coach python38Packages.rl-coach salut_a_toi sasview

54 package built:
curaByDagoma cura_stable displaycal friture gnss-sdr gnuradio gnuradio-with-packages gqrx gr-ais gr-gsm gr-limesdr gr-nacl gr-osmosdr gr-rds grass impressive inspectrum kicad kicad-small kicad-unstable loxodo mavproxy metamorphose2 playonlinux plover.stable printrun python27Packages.binwalk python27Packages.pyopengl python27Packages.pyqtgraph python27Packages.pyspread python27Packages.robotframework-ride python27Packages.spyder python27Packages.wxPython python27Packages.wxPython_4_0 python37Packages.binwalk python37Packages.pyopengl python37Packages.pyqtgraph python37Packages.stytra python37Packages.wxPython_4_0 python38Packages.binwalk python38Packages.pyopengl python38Packages.pyqtgraph python38Packages.stytra python38Packages.wxPython_4_0 qradiolink qutebrowser run-scaled scyther soulseekqt torchat tribler video2midi winpdb xpra
@jonringer jonringer merged commit 5c11e17 into NixOS:master Feb 24, 2020
13 checks passed
13 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./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
@guibou guibou deleted the guibou:fix_pyopengl branch Feb 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.