Skip to content

Conversation

@rjarry
Copy link
Collaborator

@rjarry rjarry commented Oct 24, 2025

Previously, systemd service files, bash completions, and configuration files were installed through package-specific scripts (debian/rules and rpm/grout.spec). This approach required maintaining duplicate installation logic across different packaging systems and made it harder to support other distributions or installation methods.

By moving this logic to meson.build files, the installation becomes uniform across all platforms and packaging systems. The systemd service is now installed using the canonical pkg-config method to determine the correct system unit directory. Bash completions are installed to the standard datadir location. Configuration files are installed to sysconfdir.

Package-specific install files (debian/grout.install) are updated to reference the new meson-installed paths, and manual install commands are removed from packaging scripts since meson handles them now.

Summary by CodeRabbit

  • New Features

    • Added bash completion for grcli and grout
    • Added telemetry exporter component
  • Chores

    • Updated packaging and build configuration
    • Added systemd development dependency for builds
  • Packaging

    • Removed packaged systemd service and legacy init/default installs from the Debian/RPM packages

@coderabbitai
Copy link

coderabbitai bot commented Oct 24, 2025

📝 Walkthrough

Walkthrough

Build scripts (main/meson.build and cli/meson.build) add optional systemd unit installation and bash-completion installation for grout and grcli. Debian packaging: debian/control adds systemd-dev Build-Depends; debian/grout.service is removed; debian/grout.install gains service and completion paths; debian/grout.bash-completion mappings are removed; debian/rules no longer copies init/default files. RPM spec stops installing config/service/completion files and adds a telemetry exporter and telemetry data files. CI workflow apt-get step adds systemd-dev.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "build: move file installation to meson" accurately captures the core change across the pull request. The modifications in cli/meson.build and main/meson.build add installation directives for systemd units and bash completions, while corresponding installations are removed from debian/rules and rpm/grout.spec. The title is concise, specific, and directly reflects the stated objective of centralizing installation logic into meson. It provides sufficient context for someone reviewing the repository history to understand the primary change without ambiguity.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 438afee and 448d854.

📒 Files selected for processing (9)
  • .github/workflows/package.yml (1 hunks)
  • cli/meson.build (1 hunks)
  • debian/control (1 hunks)
  • debian/grout.bash-completion (0 hunks)
  • debian/grout.install (1 hunks)
  • debian/grout.service (0 hunks)
  • debian/rules (0 hunks)
  • main/meson.build (1 hunks)
  • rpm/grout.spec (0 hunks)
💤 Files with no reviewable changes (4)
  • rpm/grout.spec
  • debian/grout.bash-completion
  • debian/rules
  • debian/grout.service
🚧 Files skipped from review as they are similar to previous changes (4)
  • .github/workflows/package.yml
  • debian/control
  • debian/grout.install
  • main/meson.build
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: rjarry
PR: DPDK/grout#294
File: modules/policy/cli/meson.build:4-8
Timestamp: 2025-09-05T07:06:51.554Z
Learning: The grout project uses a modular CLI build system where individual modules contribute their CLI sources to a global cli_src variable via `cli_src += files(...)`, and the top-level meson.build file builds the final grcli executable using all collected CLI sources. The grcli executable is defined in the top-level meson.build at lines 122-127.
📚 Learning: 2025-09-05T07:06:51.554Z
Learnt from: rjarry
PR: DPDK/grout#294
File: modules/policy/cli/meson.build:4-8
Timestamp: 2025-09-05T07:06:51.554Z
Learning: The grout project uses a modular CLI build system where individual modules contribute their CLI sources to a global cli_src variable via `cli_src += files(...)`, and the top-level meson.build file builds the final grcli executable using all collected CLI sources. The grcli executable is defined in the top-level meson.build at lines 122-127.

Applied to files:

  • cli/meson.build
📚 Learning: 2025-09-05T07:06:51.554Z
Learnt from: rjarry
PR: DPDK/grout#294
File: modules/policy/cli/meson.build:4-8
Timestamp: 2025-09-05T07:06:51.554Z
Learning: In this project, cli_src is a global variable that gets populated by individual modules (like modules/policy/cli/meson.build) and is ultimately consumed by an executable() call in the top-level meson.build file. This creates a modular CLI build where each module contributes its CLI sources to the main CLI binary.

Applied to files:

  • cli/meson.build
🔇 Additional comments (1)
cli/meson.build (1)

27-31: LGTM! Bash-completion installation properly centralized.

The installation logic correctly installs the bash-completion file to the standard directory with appropriate renaming. This aligns with the PR objective of moving installation logic from package scripts into meson.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
main/meson.build (1)

46-46: Use files() for consistency.

Line 46 uses a plain string 'grout.init' while other install_data() calls use the files() wrapper. For consistency with lines 38, 42, and 50, this should be files('grout.init').

Apply this diff:

-  install_data('grout.init', install_dir: get_option('sysconfdir'))
+  install_data(files('grout.init'), install_dir: get_option('sysconfdir'))
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc88ff2 and 568db24.

📒 Files selected for processing (8)
  • cli/meson.build (1 hunks)
  • debian/control (1 hunks)
  • debian/grout.bash-completion (0 hunks)
  • debian/grout.install (1 hunks)
  • debian/grout.service (0 hunks)
  • debian/rules (0 hunks)
  • main/meson.build (1 hunks)
  • rpm/grout.spec (0 hunks)
💤 Files with no reviewable changes (4)
  • rpm/grout.spec
  • debian/grout.bash-completion
  • debian/rules
  • debian/grout.service
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-05T07:06:51.554Z
Learnt from: rjarry
PR: DPDK/grout#294
File: modules/policy/cli/meson.build:4-8
Timestamp: 2025-09-05T07:06:51.554Z
Learning: The grout project uses a modular CLI build system where individual modules contribute their CLI sources to a global cli_src variable via `cli_src += files(...)`, and the top-level meson.build file builds the final grcli executable using all collected CLI sources. The grcli executable is defined in the top-level meson.build at lines 122-127.

Applied to files:

  • cli/meson.build
🔇 Additional comments (5)
debian/control (1)

24-24: LGTM!

Adding systemd-dev as a build dependency is necessary for the systemd integration in main/meson.build, which queries systemd's pkg-config variables to determine the correct unit installation directory.

cli/meson.build (1)

27-31: LGTM!

The bash completion installation follows standard conventions: installing to the canonical datadir/bash-completion/completions directory and renaming the file to match the command name.

main/meson.build (2)

31-40: LGTM!

The optional systemd integration correctly uses pkg-config to determine the installation directory, ensuring portability across different systemd configurations. The conditional installation of systemd-specific files is appropriate.


49-53: LGTM!

Installing bash completion unconditionally is correct since it's independent of systemd availability and follows the same pattern as cli/meson.build.

debian/grout.install (1)

5-5: Verify that debian/grout.install path matches meson's computed systemd unit directory for your build configuration.

The concern is valid. Meson dynamically computes systemd_system_unit_dir using the configured prefix, while debian/grout.install hardcodes /usr/lib/systemd/system/grout.service. For standard Debian builds (prefix=/usr), both resolve identically. However, if a non-standard prefix is used during the meson build, the paths will diverge and the service file won't be installed by debian packaging.

Ensure your build uses prefix=/usr or manually synchronize the paths.

Previously, systemd service files, bash completions, and configuration
files were installed through package-specific scripts (debian/rules and
rpm/grout.spec). This approach required maintaining duplicate
installation logic across different packaging systems and made it harder
to support other distributions or installation methods.

By moving this logic to meson.build files, the installation becomes
uniform across all platforms and packaging systems. The systemd service
is now installed using the canonical pkg-config method to determine the
correct system unit directory. Bash completions are installed to the
standard datadir location. Configuration files are installed to
sysconfdir.

Package-specific install files (debian/grout.install) are updated to
reference the new meson-installed paths, and manual install commands are
removed from packaging scripts since meson handles them now.

Signed-off-by: Robin Jarry <[email protected]>
Reviewed-by: Christophe Fontaine <[email protected]>
@christophefontaine christophefontaine merged commit cb413c0 into DPDK:main Oct 29, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants