Skip to content

feat(nvidia): evaluate shared NVIDIA system files for LTS - #474

Open
castrojo wants to merge 1 commit into
projectbluefin:mainfrom
castrojo:feat/evaluate-shared-nvidia
Open

feat(nvidia): evaluate shared NVIDIA system files for LTS#474
castrojo wants to merge 1 commit into
projectbluefin:mainfrom
castrojo:feat/evaluate-shared-nvidia

Conversation

@castrojo

Copy link
Copy Markdown
Contributor

Closes #434

Summary

  • add the shared NVIDIA Flatpak runtime-sync service to the LTS NVIDIA overlay
  • add its helper and explicitly enable the service after NVIDIA driver installation
  • document compatibility decisions for the shared candidates

Compatibility

LTS already ships Flatpak, flatpak-system-helper, systemd, and bootc. The service is gated on the NVIDIA module and /run/ostree-booted, and the helper uses the standard Flatpak CLI without Fedora-specific assumptions.

Validation

  • bash -n passed for all NVIDIA build/helper scripts
  • git diff --check passed
  • just check, just lint, and just unit-tests could not run because just is not installed
  • systemd unit verification and full image/runtime tests could not run because systemd-analyze and Podman are unavailable

🐝 Hive Agent: contributor | SHA: unknown

Port the shared NVIDIA Flatpak runtime synchronization service and helper into the LTS NVIDIA overlay. Enable it after driver installation and document the CentOS compatibility evaluation.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@castrojo
castrojo requested a review from a team as a code owner July 29, 2026 23:05
@castrojo castrojo added agent/contributor Work by the contributor agent contributor/castrojo PRs/issues from contributor castrojo cli/copilot Work done via GitHub Copilot CLI labels Jul 29, 2026

@hanthor hanthor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The helper itself is fine and the check/sync split via ExecCondition is the right pattern. What I want settled before this merges is whether the side effects are intended on LTS specifically.

1. This adds Flathub system-wide, implicitly

flatpak remote-add --system --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo

On an NVIDIA LTS install this silently configures a system-level Flathub remote the first time the driver version changes. If LTS's position is that remotes are the admin's choice, this quietly overrides that. If Flathub is already added elsewhere in the LTS image, this line is redundant and could be dropped in favor of failing loudly when the remote is missing.

2. flatpak update --system -y runs unattended at boot

flatpak update --system --noninteractive -y

with TimeoutStartSec=900 and Restart=on-failure / RestartSec=30. That's an unbounded system-wide Flatpak update on an LTS box, triggered by a driver version change, with a 15-minute window and automatic retry. On a metered or slow connection that's a rough first boot after an update.

The upstream justification is flatpak/flatpak#3907 — the GL runtime has to match the kernel driver. But that only requires installing the two GL.nvidia-* / GL32.nvidia-* runtimes, which the preceding command already does. Updating every system Flatpak is a broader action than the stated problem needs. Is that deliberate, or inherited from the Fedora image where it matters less?

I'd like to see either the flatpak update line dropped, or a note in the skill doc explaining why LTS wants it.

3. Minor

  • SYSTEM_NVIDIA_VERSION=$(cat "${NVIDIA_VERSION_FILE}") under set -e will abort if the file vanishes between the unit's ConditionPathExists and the exec. Unlikely, but the check branch would then fail rather than cleanly reporting "nothing to do."
  • The NVIDIA_VERSION_FILE override hook is nice for testing — is there an accompanying test? I didn't see one in this PR.
  • The docs table says "Other shared NVIDIA files are not present in the current common layer and are not copied speculatively," which is a good note. Please add the Flathub/update behavior to that table once resolved.

Generated by Claude Code

@hanthor hanthor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Clean addition. Service and helper are well-designed.

What I checked:

  • Service is properly gated: ConditionPathExists on /sys/module/nvidia/version and /run/ostree-booted
  • Helper supports check/sync modes, uses NVIDIA_VERSION_FILE env override for testability
  • Flatpak commands use --system with --noninteractive
  • Doc table is a good pattern for future shared-file evaluations
  • No Fedora-specific assumptions in the helper

@hanthor
hanthor enabled auto-merge August 3, 2026 23:46
@hanthor

hanthor commented Aug 7, 2026

Copy link
Copy Markdown
Member

Triage status: this has an unresolved changes-requested review that the later approval didn't address. Two policy questions need explicit answers before merge: (1) the service implicitly adds the Flathub remote — should that be gated on the user having Flathub enabled already? (2) it runs an unattended flatpak update for the NVIDIA runtime — is auto-update acceptable here, or should it only sync on driver-version change? Resolving those two (and rebasing the branch) unblocks it.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent/contributor Work by the contributor agent cli/copilot Work done via GitHub Copilot CLI contributor/castrojo PRs/issues from contributor castrojo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(nvidia): evaluate shared NVIDIA system files for LTS

2 participants