As the commit is merged upstream already, I think using fetchpatch2 would be more appropriate to avoid the unsustainable growth of the nixpkgs repo.
That is, if the patch included here is taken verbatim from upstream, if changes had to be made for it to apply then vendoring it is of course reasonable.

I may have a look at this soon-ish, given that my cluster setup takes pains to side-step some of the systemd logic (for reasons), that way I could confirm whether or not this impacts anything other than the NixOS modules.
(where "no impact" is good of course)

That is, if the patch included here is taken verbatim from upstream, if changes had to be made for it to apply then vendoring it is of course reasonable.

Yeah, unfortunately I did have to make a small tweak to get it to apply cleanly.

given that my cluster setup takes pains to side-step some of the systemd logic (for reasons)

Ah yes, I feel like I can guess what some of those reasons might be 😁. I'm hoping to help smooth some of those other ceph tooling systemd expectations in the future. Thanks!

I just had a short glance over the patch and noticed something odd so I compared it with upstream.
Why does the nixpkgs patch use the install prefix directly, rather than have the bin directory in between?

diff --git a/upstream b/pr
index c4d1e4f..9ceedac 100644
--- a/upstream
+++ b/pr
@@ -191,9 +169,9 @@ index 6644508cf0dea..0933676d2aca0 100644
 -ExecStart=/usr/bin/rbdmap map
 -ExecReload=/usr/bin/rbdmap map
 -ExecStop=/usr/bin/rbdmap unmap-all
-+ExecStart=@CMAKE_INSTALL_PREFIX@/bin/rbdmap map
-+ExecReload=@CMAKE_INSTALL_PREFIX@/bin/rbdmap map
-+ExecStop=@CMAKE_INSTALL_PREFIX@/bin/rbdmap unmap-all
++ExecStart=@CMAKE_INSTALL_PREFIX@/rbdmap map
++ExecReload=@CMAKE_INSTALL_PREFIX@/rbdmap map
++ExecStop=@CMAKE_INSTALL_PREFIX@/rbdmap unmap-all

  [Install]
  WantedBy=multi-user.target

You mentioned you had to adjust something, but I'm confused as to why there's this discrepancy given that there is a bin directory on our end too:

% find /nix/store/3c9migshmrcypwsvkql98n7l72ignhw1-ceph-19.2.2 -name rbdmap
/nix/store/3c9migshmrcypwsvkql98n7l72ignhw1-ceph-19.2.2/bin/rbdmap

I believe you that it's required (again, just had a glance at it, haven't even built it), it just looks strange to me and as if there's another issue hiding in our packaging perhaps (though it could be intentional as well, I'll have to have a look).

I just had a short glance over the patch and noticed something odd so I compared it with upstream.

Good catch! I'm going to switch this over to fetchpatch2 and include another patch that gets everything to apply cleanly.

Fixed in 0bb5395447bbff472f8a7689d134c8d21da3e04e

result/lib/systemd/system/rbdmap.service
[Unit]
Description=Map RBD devices
After=network-online.target ceph.target
Before=remote-fs-pre.target
Wants=network-online.target remote-fs-pre.target ceph.target

[Service]
EnvironmentFile=-/etc/sysconfig/ceph
Environment=RBDMAPFILE=/etc/ceph/rbdmap
Type=oneshot
RemainAfterExit=yes
ExecStart=/nix/store/v6wg7bn7kqng5q9sdfr686b3xk9zmlm0-ceph-19.2.3/bin/rbdmap map
ExecReload=/nix/store/v6wg7bn7kqng5q9sdfr686b3xk9zmlm0-ceph-19.2.3/bin/rbdmap map
ExecStop=/nix/store/v6wg7bn7kqng5q9sdfr686b3xk9zmlm0-ceph-19.2.3/bin/rbdmap unmap-all

[Install]
WantedBy=multi-user.target

I also did find one more bug in [email protected], but I'm working to get that fixed upstream.

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 440226
Commit: 610063e1c875022ee55550ebb101b0a376fec3b5

x86_64-linux

aarch64-linux

I'm clicking "Merge when ready". Ofborg on Linux is already green.

Thanks @nh2 @benaryorg!

I'll be curious to hear from anyone who ends up testing these new units directly what they think.