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
ilspy: init at 7.0-rc2 #91255
ilspy: init at 7.0-rc2 #91255
Conversation
sha256 = "1p1fkdkpbcqj2q4n7y241lzx98w33vmh4n2gahw7lbrrg1p7rxxa"; | ||
}; | ||
|
||
installPhase = '' |
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.
Why not build it from source?
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.
Hi, and thanks for reviewing! I didn't build from source because I don't need it, it's more work, and I don't even have any other platforms to test things.
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.
For nixpkgs
generally build from source, since it is then linked correctly (avoids the LD_LIBRARY_PATH
environment variable) and we can support platforms as they are added to NixOS. Additionally we can catch more NixOS-related issues of the package while building which often only show themselves when running in the case of prebuilt packages.
Overall the resulting packages are also cleaner and more transparent as to what is going on.
I understand it is more work and it often can be kind of tricky to get more obscure build systems to work with nix. If you don't want to rework it for NixOS, you can still use callPackage
from your configuration.nix
to use it on your system or override your nixpkgs.
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.
Unfortunately I don't think I'll spend any more time on this any time soon. It does work so I think it should be merged (i.e. I think it's better than not even having a package for this), but if it might cause trouble for maintainers I understand why maintainers wouldn't want to merge it.
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.
Fair enough, as said you can always use overriding to add local packages to nixpkgs for your system.
mv * $out/bin | ||
|
||
wrapProgram $out/bin/ILSpy \ | ||
--prefix LD_LIBRARY_PATH : "${stdenv.lib.makeLibraryPath libs}" |
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.
Probably worth it to also install a matching .desktop
file and install it to better integrate it with desktop environments.
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.
Added one in e85a1c1 but tbh I have no idea what I'm doing here... I use i3 so it's been many years since I even consumed these things I think.
da980c4
to
c112a33
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
$out/share/applications \ | ||
$out/share/pixmaps | ||
ln -s ${icon} $out/share/pixmaps/ILSpy.ico | ||
mv * $out/bin |
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.
Are there so many executables? Maybe also use install to remove the chmod.
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.
As with most .NET apps there's one entrypoint executable plus lots of dll files and other auxiliary files that need to be in the same directory.
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.
Wouldn't it be a good idea to move them into some lib/libexec etc directory and build a wrapper around the main executable? Polluting $PATH with many files even if not executable is not that great.
9ec6102
to
346fb17
Compare
I marked this as stale due to inactivity. → More info |
f3f9765
to
49aed12
Compare
I marked this as stale due to inactivity. → More info |
Closing as we now have |
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)