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
quake3e: init at 2019-09-09 #69216
quake3e: init at 2019-09-09 #69216
Conversation
72d5efe
to
6f633aa
Compare
6f633aa
to
e352c87
Compare
@jonringer Thanks for the review. I've fixed your suggestions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nix-review
passes on NixOS
diff LGTM
(I don't won quake3, so i can't test)
leaf package
Any update on this? |
builds fine, I would just like for someone else who owns quake, to also verify that it works :/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested with nix-review, played a round against bots (and won)
enableParallelBuilding = true; | ||
|
||
postPatch = '' | ||
sed -i -e 's#OpenGLLib = dlopen( dllname#OpenGLLib = dlopen( "${libGL}/lib/libGL.so"#' code/unix/linux_qgl.c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code/client/cl_curl.c tries to load libcurl.so, is there a reason not to patch the loader?
sed -i -e 's#Sys_LoadLibrary( "libXrandr.so.2" )#Sys_LoadLibrary( "${libXrandr}/lib/libXrandr.so.2" )#' code/unix/x11_randr.c | ||
sed -i -e 's#Sys_LoadLibrary( "libXxf86vm.so.1" )#Sys_LoadLibrary( "${libXxf86vm}/lib/libXxf86vm.so.1" )#' code/unix/x11_randr.c | ||
sed -i -e 's#Sys_LoadLibrary( "libXxf86dga.so.1" )#Sys_LoadLibrary( "${libXxf86dga}/lib/libXxf86dga.so.1" )#' code/unix/x11_dga.c | ||
''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of patching the source code to get the right path to the library, two possible less intrusive proposals:
- you could change the build system and pass these libraries to the linker of the executable via LDFLAGS
- you could fix the missing RPATH in the executable after linking the execuable in the fixupPhase with patchelf
Motivation for this change
I wanted this package in nixpkgs.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)