Skip to content

fix: detect systemd unit directory at runtime for cross-distro compatibility#35

Merged
Benoît Cortier (CBenoit) merged 2 commits intomasterfrom
DGW-317
Oct 31, 2025
Merged

fix: detect systemd unit directory at runtime for cross-distro compatibility#35
Benoît Cortier (CBenoit) merged 2 commits intomasterfrom
DGW-317

Conversation

@CBenoit
Copy link
Copy Markdown
Member

Fixes installation failure on Rocky Linux 8.4 and other RHEL-based distributions where systemd units are located in /usr/lib/systemd/system instead of /lib/systemd/system.

  • Add detect_systemd_unit_dir() with three-tier detection:
    1. CEVICHE_SYSTEMD_UNITDIR environment variable (takes precedence)
    2. pkg-config --variable=systemdsystemunitdir systemd
    3. Fallback probing: /lib/systemd/system, then /usr/lib/systemd/system
  • Update get_service_unit_path() and get_service_dropin_dir() to use detection
  • Add comprehensive documentation in README explaining the approach and caveats
  • Include rationale comments about when to use distro-specific tooling

This pragmatic runtime detection works across distributions while still recommending proper packaging tools (dh_installsystemd, %{_unitdir}) for production packages.

Prompt

I used Claude Code to generate this fix, and audited it.

Here is the prompt I used:

Implement build/packaging-time detection of the systemd *system* unit directory by invoking:

  pkg-config --variable=systemdsystemunitdir systemd

Requirements:
1) - Resolve UNITDIR via pkg-config as mentioned above.
   - Allow an env override: CEVICHE_SYSTEMD_UNITDIR (takes precedence).
   - Fail clearly if UNITDIR is empty; otherwise install the ".service" file.

2) Fallback behavior if pkg-config is unavailable:
   - If SYSTEMD_UNITDIR is unset and pkg-config not found, probe common vendor dirs in order:
     /lib/systemd/system, then /usr/lib/systemd/system
   - Emit a warning comment/message explaining it’s a heuristic and may be distro-specific (suggest using CEVICHE_SYSTEMD_UNITDIR env variable).

3) Add a short “Rationale” comment that explains:
   - This isn’t the best approach for Linux — packagers should normally choose the destination and rely on distro tooling (e.g., Debian’s dh_installsystemd or RPM’s %{_unitdir} macros).
   - Using pkg-config at build/packaging time is a pragmatic, good-enough approach in many situations to discover the vendor unit dir without hardcoding paths.
   - Caveat: we can’t automatically determine whether it should go into user/ or system/, and we default to system/. Use CEVICHE_SYSTEMD_UNITDIR if you need to choose.

4) Quality:
   - Keep snippets minimal and self-contained.

Deliverables:
- Update the library.
- README section titled “Where we install systemd units and why”

…ibility

Fixes installation failure on Rocky Linux 8.4 and other RHEL-based
distributions where systemd units are located in /usr/lib/systemd/system
instead of /lib/systemd/system.

- Add detect_systemd_unit_dir() with three-tier detection:
  1. CEVICHE_SYSTEMD_UNITDIR environment variable (takes precedence)
  2. pkg-config --variable=systemdsystemunitdir systemd
  3. Fallback probing: /lib/systemd/system, then /usr/lib/systemd/system
- Update get_service_unit_path() and get_service_dropin_dir() to use detection
- Add comprehensive documentation in README explaining the approach and caveats
- Include rationale comments about when to use distro-specific tooling

This pragmatic runtime detection works across distributions while still
recommending proper packaging tools (dh_installsystemd, %{_unitdir}) for
production packages.
@CBenoit Benoît Cortier (CBenoit) merged commit 5ebd47e into master Oct 31, 2025
11 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds runtime detection of systemd unit directories to make the library more portable across different Linux distributions, replacing the hardcoded /lib/systemd/system/ path. The version is bumped from 0.6.1 to 0.7.0.

  • Implements a three-tier detection strategy: environment variable override, pkg-config query, and fallback probing
  • Updates all methods that reference systemd paths to use the new detection function
  • Adds comprehensive documentation explaining the approach and its caveats

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/controller/linux.rs Adds detect_systemd_unit_dir() function with runtime detection logic, updates get_service_unit_path() and get_service_dropin_dir() to return Result<PathBuf, Error>, adjusts callers to handle new error cases
README.md Documents the systemd unit detection strategy, caveats for packagers, and usage instructions for the CEVICHE_SYSTEMD_UNITDIR environment variable
Cargo.toml Bumps version from 0.6.1 to 0.7.0

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +175 to 178
fn get_service_dropin_dir(&self) -> Result<PathBuf, Error> {
let unit_dir = detect_systemd_unit_dir()?;
Ok(unit_dir.join(format!("{}.d", self.get_service_file_name())))
}
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The detect_systemd_unit_dir() function is called separately in both get_service_unit_path() and get_service_dropin_dir(). When both methods are called (e.g., in write_service_config() and delete()), this results in redundant detection work including potential Command execution and filesystem probing. Consider caching the detected directory in the LinuxController struct or calling the detection once at a higher level.

Copilot uses AI. Check for mistakes.
Error::new(&format!("Failed to create {}: {}", path.display(), e))
})?;
fs::create_dir(&dropin_dir)
.map_err(|e| Error::new(&format!("Failed to create {}: {}", path.display(), e)))?;
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The error message references path.display() but the variable being created is dropin_dir. This will show the wrong path (the config file path instead of the directory path) if directory creation fails. Change to dropin_dir.display().

Suggested change
.map_err(|e| Error::new(&format!("Failed to create {}: {}", path.display(), e)))?;
.map_err(|e| Error::new(&format!("Failed to create {}: {}", dropin_dir.display(), e)))?;

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants