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
target-isns: init at 0.6.8 #108866
target-isns: init at 0.6.8 #108866
Conversation
install(FILES target-isns.8 DESTINATION ${CMAKE_INSTALL_PREFIX}/share/man/man8/) | ||
if (SUPPORT_SYSTEMD) | ||
- install(FILES target-isns.service DESTINATION /usr/lib/systemd/system/) | ||
+ install(FILES target-isns.service DESTINATION ${CMAKE_INSTALL_PREFIX}/usr/lib/systemd/system/) |
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.
I don't think this should include /usr
.
If these are meant to be used with systemd.packages
, I think that only works in lib/systemd/system
. I might be wrong about that, though.
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.
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.
OK, now the systemd files get actually installed, and end up in lib/systemd/system
.
I am a bit hesitant when comes to upstreaming such nix specific patches. It may take more time energy to explain the patch than just keeping in it 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.
CMAKE_INSTALL_PREFIX
is not Nix specific, it's CMake-specific. It is also mentioned in their own tutorial: https://cmake.org/cmake/help/latest/guide/tutorial/index.html
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.
Sure, CMAKE_INSTALL_PREFIX is a cmake feature. However, as it is now the patch breaks a standard FHS distro's installation: the config file would end up in e.g. /usr/etc or /usr/local/etc instead of /etc. I am not too familiar with cmake. What would be the right way to do this?
Did you consider delivering the patch upstream as well? I'm pretty sure they just never had to install their software anywhere else than the default location(s), |
555ddb1
to
0e8d31e
Compare
0e8d31e
to
abd767d
Compare
This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch) Result of 1 package built:
|
Motivation for this change
Daemon that registers iSCSI exports from the Linux LIO target with an iSNS server.
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)