fix fedora44#23
Conversation
- Remove duplicate 'automake' from dnf install list (copy-paste error) - Generate SSH host keys at container start instead of image build time to avoid all containers sharing the same SSH host identity Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prevents word splitting and globbing on paths with spaces or special characters. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…A-18/add-fedora44
# Conflicts: # docker/fedora44/Dockerfile
WalkthroughThe PR modifies the Fedora 44 Docker image by adding a package management step that updates the base system and removes ChangesFedora msquic dependency preparation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docker/fedora44/Dockerfile (1)
27-30: ⚡ Quick winRemove redundant
dnf updateto improve build efficiency.The
dnf updateon line 27 is unnecessary since the package metadata was already refreshed on line 13. Package removal withdnf removedoesn't require a fresh metadata update. Removing this also addresses the static analysis warning (Trivy DS-0017) about standalonednf updatecommands.♻️ Proposed simplification
-RUN dnf -y update && \ - dnf -y remove \ +RUN dnf -y remove \ wget \ numactl-libs🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/fedora44/Dockerfile` around lines 27 - 30, Remove the redundant "dnf -y update && \" from the RUN statement so the command becomes a simple "dnf -y remove ..." invocation; specifically, edit the RUN that currently contains "dnf -y update && \\" and "dnf -y remove wget numactl-libs" to drop the standalone update step (this eliminates the unnecessary metadata refresh and addresses the Trivy DS-0017 warning).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/fedora44/Dockerfile`:
- Line 26: Fix the typo in the Dockerfile comment: replace the incorrect word
"numctl-libs" with the correct "numactl-libs" in the comment line that reads "#
Necessary to remove numctl-libs, so msquic works" so the comment accurately
references the package name.
- Around line 26-30: The comment above the RUN that updates and removes packages
(the RUN block that removes wget and numactl-libs) incorrectly states
numactl-libs must be removed for msquic to work; change the comment to explain
that libnuma (provided by numactl-libs) is an optional runtime dependency and
should only be removed if ldd shows libmsquic.so has no libnuma dependency, or
alternatively note that build-time headers come from the numactl-devel package
(install numactl-devel when building). Update the Dockerfile comment to either
(a) state the removal is conditional based on ldd/package linkage or (b)
instruct to install numactl-devel for build-time headers instead of removing
numactl-libs, and make sure the RUN removal of numactl-libs is only kept if
you’ve verified no runtime libnuma dependency.
---
Nitpick comments:
In `@docker/fedora44/Dockerfile`:
- Around line 27-30: Remove the redundant "dnf -y update && \" from the RUN
statement so the command becomes a simple "dnf -y remove ..." invocation;
specifically, edit the RUN that currently contains "dnf -y update && \\" and
"dnf -y remove wget numactl-libs" to drop the standalone update step (this
eliminates the unnecessary metadata refresh and addresses the Trivy DS-0017
warning).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d101e5b-0047-4c7b-8fab-b1a6c33950e3
📒 Files selected for processing (1)
docker/fedora44/Dockerfile
| ./cmake.sh --skip-license --prefix=/usr/local && \ | ||
| rm ./cmake.sh | ||
|
|
||
| # Necessary to remove numctl-libs, so msquic works |
There was a problem hiding this comment.
Fix typo in comment.
The comment says "numctl-libs" but should be "numactl-libs" (missing 'a').
📝 Proposed fix
-# Necessary to remove numctl-libs, so msquic works
+# Necessary to remove numactl-libs, so msquic works📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Necessary to remove numctl-libs, so msquic works | |
| # Necessary to remove numactl-libs, so msquic works |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/fedora44/Dockerfile` at line 26, Fix the typo in the Dockerfile
comment: replace the incorrect word "numctl-libs" with the correct
"numactl-libs" in the comment line that reads "# Necessary to remove
numctl-libs, so msquic works" so the comment accurately references the package
name.
| # Necessary to remove numctl-libs, so msquic works | ||
| RUN dnf -y update && \ | ||
| dnf -y remove \ | ||
| wget \ | ||
| numactl-libs |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does msquic have conflicts with numactl or numactl-libs on Fedora Linux?
💡 Result:
No clear evidence that msquic conflicts with numactl or numactl-libs on Fedora Linux. What we do see in msquic’s Fedora-related issues is dependency/build completeness around NUMA, not package-level “conflicts”: - msquic’s early Linux packages had issues around locating the NUMA header (numa.h). The report explicitly notes the packages “don’t actually have the libnuma dependency” (and the build found libnuma.so but not numa.h), and that this was fixed via a follow-up change/pull request [1]. - Other Fedora packaging issues mentioned by msquic’s maintainers/issue reporters are about missing publication for specific Fedora releases, and unrelated missing dependencies (e.g., lttng-ust), not conflicts with numactl/numactl-libs [2][3][4]. So, based on the available Fedora-focused msquic evidence, the relevant risk is whether the system has the required NUMA development/runtime libraries at build/install time (e.g., libnuma/numa.h), rather than a conflict between msquic and numactl/numactl-libs. If you’re troubleshooting Fedora specifically, the practical check is to ensure the needed NUMA pieces are present (libnuma runtime and headers/devel if building from source).
Citations:
- 1: Official Linux Packages aren't Being Built with numa.h microsoft/msquic#3619
- 2: libmsquic package is missing for Fedora 38 microsoft/msquic#3610
- 3: libmsquic package is missing for Fedora 39 microsoft/msquic#4092
- 4: Fail to install libmsquic on rocky 9 microsoft/msquic#5545
🌐 Web query:
msquic numactl-libs remove
💡 Result:
You generally should NOT remove numactl-libs just because you installed msquic; whether you must keep it depends on your msquic build/package and distro. What the evidence shows: - MsQuic’s Linux shared library can load libnuma at runtime (e.g., ldd output shows libnuma.so.1 linked by libmsquic.so). In other words, if your msquic package is linked against libnuma, removing numactl-libs (which provides the runtime libnuma.so.1 on many distros) would break msquic at load time. [1] - MsQuic maintainers have discussed packaging/build pipeline issues where a libnuma runtime dependency may or may not be correctly declared in the package metadata (e.g., “No libnuma dependency because it wasn’t fully available at build time”, later fixed). That means some installations may appear to work until you actually run into a missing libnuma.so.1. [1] Practical guidance (safe approach): 1) Check whether your installed msquic library currently depends on libnuma: - Debian/Ubuntu/RPM systems: - ldd $(readlink -f /usr/lib*/libmsquic.so.* 2>/dev/null | head -n1) | grep -i numa - If you see “libnuma.so.1 => …”, keep numactl-libs installed. 2) If you do not see any libnuma dependency for your particular libmsquic build, then removing numactl-libs may be safe for runtime—but you should validate by starting whatever process uses msquic. If you tell me your distro (e.g., Ubuntu 22.04, Debian 12, RHEL/Rocky 9) and which msquic package/version you installed, I can suggest the exact dpkg/rpm commands to remove it safely and how to verify msquic still loads.
Citations:
🌐 Web query:
msquic libnuma dependency Fedora numactl-libs
💡 Result:
On Fedora, the “libnuma” dependency that MsQuic may need is provided by the Fedora package “numactl-libs” (it “provides libnuma” and contains libnuma.so.1). [1] Evidence: - Fedora’s package page for numactl-libs explicitly states it provides “libnuma libraries” and “libnuma, a library to do allocations with NUMA policy”. [1] - MsQuic maintainers discussed/confirmed that MsQuic links to libnuma.so.1 on Linux (and that missing package metadata depended on build-time availability, but the library is runtime-linked when present). [2][3] Practical Fedora packaging/install guidance: - If you’re running MsQuic’s shared library and it reports libnuma.so.1 missing, install numactl-libs. [1][3] Example: - sudo dnf install numactl-libs
Citations:
- 1: https://packages.fedoraproject.org/pkgs/numactl/numactl-libs/index.html
- 2: Official Linux Packages aren't Being Built with numa.h microsoft/msquic#3619
- 3: msquic depends on libnuma but packages don't state that microsoft/msquic#3421
🌐 Web query:
msquic Fedora NUMA header numa.h libnuma dependency
💡 Result:
MsQuic’s NUMA support uses libnuma headers, specifically numa.h, and on Fedora you should ensure the development package that provides numa.h is installed (not just the runtime library). 1) What numa.h/libnuma are for - The libnuma library provides the programming interface for NUMA policy and its API is exposed via headers that include <numa.h> (e.g., the Linux man page shows how to compile with -lnuma and that source includes <numa.h>) [1]. 2) What happened in MsQuic builds/packages - MsQuic maintainers reported that at one point the build pipeline could find libnuma.so but failed to find numa.h at build time, noting the build log: “-- Found libnuma … -- numa.h not found. If build fails, install libnuma-dev” [2]. - This was addressed by a fix that corrects how numa.h is located; maintainers also showed that after the fix, libmsquic links against libnuma.so.1 at runtime (ldd output includes libnuma.so.1) [2]. - Separately, maintainers discussed that MsQuic depends on libnuma but packages may not have declared the dependency consistently (discussion of “libnuma is missing” vs later versions) [3]. 3) Fedora-specific dependency to install - On Fedora/RHEL-family systems, the runtime library package (often named something like libnuma) may be insufficient for building from source because <numa.h> is in the development package (the analogous issue for other distros shows the pattern: installing libs alone is not enough; you need the devel package that provides numa.h) [4]. Action (Fedora) - Install the libnuma development package that provides numa.h before building MsQuic from source or building dependent components: - sudo dnf install libnuma-devel Evidence: MsQuic’s build explicitly checks for numa.h and recommends installing the “libnuma-dev” package when numa.h is missing [2], which corresponds to the devel package on Debian/Ubuntu; the general symptom “numa.h: No such file or directory even when libnuma runtime is installed” is fixed by installing the devel package (numactl-devel/libnuma-devel depending on distro) [4]. If you tell me the exact Fedora version and whether you’re building msquic from source or installing the prebuilt libmsquic RPM, I can tailor the exact dnf command and verify what the package dependency list should look like for that release.
Citations:
- 1: https://man7.org/linux/man-pages/man3/numa.3.html
- 2: Official Linux Packages aren't Being Built with numa.h microsoft/msquic#3619
- 3: msquic depends on libnuma but packages don't state that microsoft/msquic#3421
- 4: https://stackoverflow.com/questions/48110600/eal-memory-c5618-fatal-error-numa-h-no-such-file-or-directory
Fix the claim about removing numactl-libs
numactl-libsprovideslibnuma.so.1on Fedora, and MsQuic’s shared library can depend on it at runtime; removing it isn’t generally required for MsQuic to “work” and can cause loader/runtime failures unlessldd libmsquic.soshows nolibnumadependency.- Update the comment to reflect the actual need (often build-time
numa.hvia the appropriate*-develpackage) or conditionally justify removal based onldd/package linkage.
🧰 Tools
🪛 Trivy (0.69.3)
[error] 27-30: 'RUN update' instruction alone
The instruction 'RUN update' should always be followed by ' install' in the same RUN statement.
Rule: DS-0017
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/fedora44/Dockerfile` around lines 26 - 30, The comment above the RUN
that updates and removes packages (the RUN block that removes wget and
numactl-libs) incorrectly states numactl-libs must be removed for msquic to
work; change the comment to explain that libnuma (provided by numactl-libs) is
an optional runtime dependency and should only be removed if ldd shows
libmsquic.so has no libnuma dependency, or alternatively note that build-time
headers come from the numactl-devel package (install numactl-devel when
building). Update the Dockerfile comment to either (a) state the removal is
conditional based on ldd/package linkage or (b) instruct to install
numactl-devel for build-time headers instead of removing numactl-libs, and make
sure the RUN removal of numactl-libs is only kept if you’ve verified no runtime
libnuma dependency.
Remove numactl
Summary by CodeRabbit