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
soil: fix dead url, enable on darwin #101042
Conversation
/marvin opt-in |
Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here. |
@7c6f434c a bit of a wild guess, but I have the feeling you might be reasonably equipped to review this PR... From a quick overview LGTM 👍 |
I have no idea… so this is a cleanup by a completely unrelated (and also not well-known) developer. I haven't looked at the code because I have never used that thing anyway… So what do we do in such cases? Compare the initial commit with the original source from binary cache or Web Archive, then review the diffs? Dunno. |
Hmm yeah that's fair, I just thought the cmake cleanup was convenient because it would do most of the cross-platform compat work for us. Would it be preferable to grab it from another repo's archive? I see a handful in https://repology.org/project/soil/versions. |
For the record, the changes seem reasonably limited and useful (I haven't had energy to look into detailed diffs yet, though) — so I do not think we should take on an unsurmountabel verification burden, but it is a good idea to discuss how much we can verify in reasonable time and how much of that we want to do before making a call. |
Yeah definitely, I wouldn't want to introduce this burden for such little gain. I think the issue is just that the existing makefile isn't written in a very generic way. Alternatively, what about just hard-coding the compile commands? Looks damn ugly, but is quite generic, and doesn't introduce any third-party code. diff --git a/pkgs/development/libraries/soil/default.nix b/pkgs/development/libraries/soil/default.nix
index cf0896170c1..acf047cd6ef 100644
--- a/pkgs/development/libraries/soil/default.nix
+++ b/pkgs/development/libraries/soil/default.nix
@@ -10,10 +10,17 @@ stdenv.mkDerivation {
buildInputs = [ unzip mesa libGL libX11 ];
- sourceRoot = "Simple OpenGL Image Library/projects/makefile";
- preBuild = "mkdir obj";
- preInstall = "mkdir -p $out/lib $out/include";
- makeFlags = [ "LOCAL=$(out)" ];
+ buildPhase = ''
+ cd src
+ $CC $NIX_CFLAGS_COMPILE -c *.c
+ $AR r libSOIL.a *.o
+ $RANLIB libSOIL.a
+ '';
+ installPhase = ''
+ mkdir -p $out/lib $out/include/SOIL
+ cp libSOIL.a $out/lib/
+ cp SOIL.h $out/include/SOIL/
+ '';
meta = {
description = "Simple OpenGL Image Library"; |
Looks fine to me, actually, when everything is flat and we know the libraries do not need to be searched for, compiling by hand is not that much of a burden. I guess finding some now-available source copies for non-binary-cache users would still be a good idea, but maybe archive.org has cached the exact same file? |
Ah, finally got the archive download to work and produce the same hash. Will push shortly. |
I think it needs more, like |
Doesn't seem to need it for me. Or do you mean it should propagate a libX11 dependency to its consumers? |
@ofborg build soil |
ebf7c5d
to
62407ab
Compare
Sorry, could have sworn I had tested that on linux :/ |
Added myself as a maintainer as this appears to be orphaned.
The new github source appears to be the most popular mirror, and has a number of bugfixes along with a more sophisticated cmake build system which works on macos with no modifications.The only dependee is already marked as broken, but I've successfully used this in another package I'm hacking on.
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)