Skip to content

fix fedora44#23

Open
Melky-Phoe wants to merge 10 commits into
masterfrom
INFRA-18/add-fedora44
Open

fix fedora44#23
Melky-Phoe wants to merge 10 commits into
masterfrom
INFRA-18/add-fedora44

Conversation

@Melky-Phoe
Copy link
Copy Markdown
Member

@Melky-Phoe Melky-Phoe commented May 28, 2026

Remove numactl

Summary by CodeRabbit

  • Chores
    • Updated Fedora 44 Docker image configuration to improve compatibility and remove unnecessary packages for optimal build performance.

Review Change Stack

Melky-Phoe and others added 10 commits May 12, 2026 16:52
- 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>
# Conflicts:
#	docker/fedora44/Dockerfile
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

Walkthrough

The PR modifies the Fedora 44 Docker image by adding a package management step that updates the base system and removes wget and numactl-libs packages to address compatibility requirements for msquic.

Changes

Fedora msquic dependency preparation

Layer / File(s) Summary
Fedora package state adjustment for msquic
docker/fedora44/Dockerfile
RUN block performs dnf update and removes wget and numactl-libs packages to satisfy msquic build/runtime requirements.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix fedora44' is vague and does not clearly convey the specific change (removing numactl-libs and updating packages for msquic compatibility). Consider a more descriptive title such as 'Remove numactl-libs from Fedora 44 Dockerfile for msquic compatibility' to better communicate the primary change.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch INFRA-18/add-fedora44

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
Copy Markdown

@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: 2

🧹 Nitpick comments (1)
docker/fedora44/Dockerfile (1)

27-30: ⚡ Quick win

Remove redundant dnf update to improve build efficiency.

The dnf update on line 27 is unnecessary since the package metadata was already refreshed on line 13. Package removal with dnf remove doesn't require a fresh metadata update. Removing this also addresses the static analysis warning (Trivy DS-0017) about standalone dnf update commands.

♻️ 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

📥 Commits

Reviewing files that changed from the base of the PR and between 5630fb6 and 4eafd10.

📒 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
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

Suggested change
# 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.

Comment on lines +26 to +30
# Necessary to remove numctl-libs, so msquic works
RUN dnf -y update && \
dnf -y remove \
wget \
numactl-libs
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 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:


🌐 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:


🌐 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:


Fix the claim about removing numactl-libs

  • numactl-libs provides libnuma.so.1 on 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 unless ldd libmsquic.so shows no libnuma dependency.
  • Update the comment to reflect the actual need (often build-time numa.h via the appropriate *-devel package) or conditionally justify removal based on ldd/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

Learn more

(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.

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