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.

Sign up for GitHub

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

Jump to bottom

awesome: Remove $LD_LIBRARY_PATH handling #34259

Merged
merged 1 commit into from Jan 29, 2018
Merged

awesome: Remove $LD_LIBRARY_PATH handling #34259

merged 1 commit into from Jan 29, 2018

Conversation

psychon
Copy link
Contributor

@psychon psychon commented Jan 25, 2018

I have no idea why gobjectIntrospection even was in here. The only
library in there, libgirepository-1.0.so, is not used by awesome. It is
only used by lgi.so and that means it should be found via its RPATH.

The Pango path is not needed in $LD_LIBRARY_PATH ever since
gobjectIntrospection started patching .typelib files with absolute
paths. Relevant commits are 36bef2b from 2014
("gobject-introspection: refer to shlibs with absolute paths in
typelibs") and c420de6 from 2016 ("gobject-introspection: Fix
patching shared objects").

The above patches did not work for cairo, because cairo's typelib is a
bit "special". However, this was fixed by e44038b some days ago
("gobjectIntrospection: use absolute path for cairo GIR").

Thus, setting $GI_TYPELIB_PATH is enough so that all needed libraries
are found.

Fixes: #14164
Signed-off-by: Uli Schlachter psychon@znc.in

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.

@psychon
awesome: Remove $LD_LIBRARY_PATH handling
ba12a86 
I have no idea why gobjectIntrospection even was in here. The only
library in there, libgirepository-1.0.so, is not used by awesome. It is
only used by lgi.so and that means it should be found via its RPATH.

The Pango path is not needed in $LD_LIBRARY_PATH ever since
gobjectIntrospection started patching .typelib files with absolute
paths. Relevant commits are 36bef2b from 2014
("gobject-introspection: refer to shlibs with absolute paths in
typelibs") and c420de6 from 2016 ("gobject-introspection: Fix
patching shared objects").

The above patches did not work for cairo, because cairo's typelib is a
bit "special". However, this was fixed by e44038b some days ago
("gobjectIntrospection: use absolute path for cairo GIR").

Thus, setting $GI_TYPELIB_PATH is enough so that all needed libraries
are found.

Fixes: NixOS#14164
Signed-off-by: Uli Schlachter <psychon@znc.in>
@psychon
Copy link
Contributor Author

psychon commented Jan 29, 2018

cc @makefu @ndowens

@makefu
Copy link
Contributor

makefu commented Jan 29, 2018

That is great news! I am really happy managed happened to solve my problems.

@psychon
Copy link
Contributor Author

psychon commented Jan 29, 2018

Slightly unrelated, but I will just leave this here as a comment:

The only reason why cmakeFlags = "-DOVERRIDE_VERSION=${version}"; is needed is because awesome is being built from git. In tarballs there is a file called .version_stamp that provides this information (but obviously this file should not be in git). Can anyone figure out why this was switched from the release tarballs to building from git in the 4.0 -> 4.1 update?

@fpletz fpletz merged commit 303d5b1 into NixOS:staging Jan 29, 2018
@psychon psychon deleted the awesome-cleanup branch January 30, 2018 08:17
@dtzWill
Copy link
Member

dtzWill commented Feb 3, 2018

This seems to break building awesome on current master:

https://hydra.nixos.org/build/68619144/nixlog/1

 The Lua GObject introspection package is just a runtime dependency, so it is not
 necessary for building awesome. However, awesome needs it to run.
 Add AWESOME_IGNORE_LGI=1 to your environment to continue.

Perhaps that should be added?

(reverting this commit fixes the build for me, FWIW)

@psychon
Copy link
Contributor Author

psychon commented Feb 4, 2018

Uhm, I'm not quite sure (I'll try to reproduce), but that seems a bit like what e44038b is supposed to fix... O.o

@psychon
Copy link
Contributor Author

psychon commented Feb 4, 2018

Yup, that's exactly the problem that the above commit was supposed to fix. Running nix-shell -A awesome --run "LD_DEBUG=all lua -e 'print(require(\"lgi\").cairo)' 2>&1 | grep -20 --color=yes cairo" (which is basically what awesome is doing during build) results in (shortened output):

     15512:	file=libcairo.so.2 [0];  dynamically loaded by /nix/store/9bjbfcwb73ayga2bm3p0crzfa1nfzmb7-glib-2.54.3/lib/libgmodule-2.0.so.0 [0]
     15512:	find library=libcairo.so.2 [0]; searching
     15512:	 search path=/run/opengl-driver/lib		(LD_LIBRARY_PATH)
     15512:	  trying file=/run/opengl-driver/lib/libcairo.so.2
     15512:	 search path=/nix/store/9bjbfcwb73ayga2bm3p0crzfa1nfzmb7-glib-2.54.3/lib:/nix/store/gk32wn30cyi86ndxr1dk2fjvg9a27kcl-pcre-8.41/lib		(RUNPATH from file /nix/store/f0lrj03v3ccmzcbi79czsy6ndviisx7g-lgi-0.9.1/lib/lua/5.2/lgi/corelgilua51.so)
     15512:	  trying file=/nix/store/9bjbfcwb73ayga2bm3p0crzfa1nfzmb7-glib-2.54.3/lib/libcairo.so.2
     15512:	  trying file=/nix/store/gk32wn30cyi86ndxr1dk2fjvg9a27kcl-pcre-8.41/lib/libcairo.so.2
     15512:	 search path=/nix/store/4h0allsz28x9mdirzb4wv51algacxzr7-glibc-2.26-131/lib		(system search path)
     15512:	  trying file=/nix/store/4h0allsz28x9mdirzb4wv51algacxzr7-glibc-2.26-131/lib/libcairo.so.2
     15512:	 search cache=/nix/store/4h0allsz28x9mdirzb4wv51algacxzr7-glibc-2.26-131/etc/ld.so.cache
     15512:	 search path=/nix/store/4h0allsz28x9mdirzb4wv51algacxzr7-glibc-2.26-131/lib		(system search path)
     15512:	  trying file=/nix/store/4h0allsz28x9mdirzb4wv51algacxzr7-glibc-2.26-131/lib/libcairo.so.2
     15512:	
lua: ...ndviisx7g-lgi-0.9.1/share/lua/5.2/lgi/override/cairo.lua:31: attempt to index field '_module' (a nil value)
stack traceback:
	...ndviisx7g-lgi-0.9.1/share/lua/5.2/lgi/override/cairo.lua:31: in main chunk
	[C]: in function 'require'
	...czsy6ndviisx7g-lgi-0.9.1/share/lua/5.2/lgi/namespace.lua:176: in function <...czsy6ndviisx7g-lgi-0.9.1/share/lua/5.2/lgi/namespace.lua:148>
	(...tail calls...)
	(command line):1: in main chunk
	[C]: in ?

So, once again, this fails to find libcairo.so.2. Why did this work previously?!?

@psychon
Copy link
Contributor Author

psychon commented Feb 4, 2018

Sigh. It worked because awesome is linked against libcairo.so.2, so the full path into the store is embedded into the binary. While building awesome, when trying to use cairo through lgi, the library is not yet loaded and due to the way lgi currently works, it falls back to dlopen("libcairo.so.2"), i.e. without an absolute path. This only works if the library is already loaded.

Pull request coming in that reorders some code in lgi to make this work....

@vcunat
Copy link
Member

vcunat commented Feb 4, 2018

It might also work to keep the build-time LD_LIBRARY_PATH, though if you find a cleaner way...

@psychon psychon mentioned this pull request Feb 4, 2018
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpletz fpletz fpletz approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@psychon @makefu @dtzWill @vcunat @fpletz @GrahamcOfBorg