Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions pkgs/by-name/gz/gz-tools/package.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
{
lib,
stdenv,
fetchFromGitHub,
cmake,
pkg-config,
gz-cmake,
nix-update-script,
coreutils,
}:
Comment thread
Guelakais marked this conversation as resolved.

stdenv.mkDerivation (finalAttrs: {
pname = "gz-tools";
version = "2.0.3";

src = fetchFromGitHub {
owner = "gazebosim";
repo = "gz-tools";
tag = "gz-tools${lib.versions.major finalAttrs.version}_${finalAttrs.version}";
hash = "sha256-xMFJylj7OnDc7zVWiI4a/mvNpu9scz83F3bGopCt8l8=";
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth adding patches/pr-174.patch that I have in my implementation at these lines. I've opened a PR upstream in gazebosim/gz-tools#174.

gz-tools encodes a non-relative lookup for libbackward that fails inside the Nix store once LD_LIBRARY_PATH/DYLD_LIBRARY_PATH aren't globally populated.

I found that without it, any error path in gz sim segfaults inside backward-cpp instead of printing a stack trace. I found this happened with gz-sensors plugin-load failures.


strictDeps = true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the nixpkgs-vet was last run against this PR, the requirement for new packet to include __structuredAttrs = true has been added. Suggest adding it to your implementation. It helps catch dependency leaks.


nativeBuildInputs = [
cmake
pkg-config
coreutils # For env
];

buildInputs = [ gz-cmake ];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ruby should be added to buildInputs and nativeCheckInputs as gz-tools ships a Ruby dispatcher (src/cmd/cmdtools.rb.in) that the gz binary invokes to route subcommands into each Gazebo library's CLI plugin. Without ruby in buildInputs, the binary is installed but every subcommand (gz topic, gz log, gz sim, ...) fails at runtime once downstream libraries are added.

You can see my buildInputs/nativeCheckInputs.


Comment thread
Guelakais marked this conversation as resolved.
# Disable automatic shebang patching since we'll do it manually
dontPatchShebangs = true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason you are patching shebangs manually? I found it wasn't required.


postFixup = ''
if [ -f "$out/bin/gz" ]; then
# Check if it's a text file (script)
if head -n1 "$out/bin/gz" | grep -q '^#!'; then
# Replace the shebang with the correct path to env
substituteInPlace "$out/bin/gz" \
--replace '#!/usr/bin/env' '#!${coreutils}/bin/env' \
--replace '#!/bin/env' '#!${coreutils}/bin/env'

# Ensure executable permissions
chmod +x "$out/bin/gz"
fi
fi
'';

doInstallCheck = true;

installCheckPhase = ''
# Verify the binary exists and can be executed
if [ -x "$out/bin/gz" ]; then
"$out/bin/gz" --help >/dev/null || true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you add || true failures will go unnoticed.

It could be worth replacing installCheckPhase with passthru.tests.version = testers.testVersion which runs as a separate derivation and is the nixpkgs standard pattern. You can see what I have done.

fi
'';

passthru = {
updateScript = nix-update-script {
attrPath = "gz-tools";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upstream tags look like gz-tools2_2.0.3 so you will need to use a regex so nix-update doesn't pick up unrelated refs. Suggest:

updateScript = nix-update-script {
  extraArgs = [ "--version-regex=gz-tools${lib.versions.major version}_([\\d\\.]+)" ];
};

};
};

meta = {
description = "Gazebo Sim tools collection";
homepage = "https://gazebosim.org/";
downloadPage = "https://github.com/gazebosim/gz-tools";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be worth adding changelog = "https://github.com/gazebosim/gz-tools/blob/${finalAttrs.src.tag}/Changelog.md";

license = lib.licenses.asl20;
maintainers = with lib.maintainers; [ guelakais ];
platforms = lib.platforms.all;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be worth narrowing this asplatforms = lib.platforms.all includes mingw, wasi, and other platforms that have never been built. Perhaps use lib.platforms.linux ++ lib.platforms.darwin.

mainProgram = "gz";
};
})
Loading