snap: Upgrade snap from core18 to core20 #11069
Conversation
Signed-off-by: Hiroshi Hatake <[email protected]>
Signed-off-by: Hiroshi Hatake <[email protected]>
cosmo0920
requested review from
edsiper,
fujimotos and
koleini
as code owners
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughSnap packaging updated: base switched to core20; command paths, config bind and icon path adjusted to new /usr-based layout; build and stage dependencies, cmake parameters, and build attributes were added/modified in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Points to review:
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
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 |
cosmo0920
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
snap/snapcraft.yaml (1)
14-18: Potential path mismatch between config binding and installation location.The
read-configplug at line 18 reads from$SNAP/etc/fluent-bit, and the layout binding at line 70-71 binds/etc/fluent-bitto$SNAP/etc/fluent-bit. However, the service command at line 22 specifies$SNAP/usr/etc/fluent-bit/fluent-bit.conf. This mismatch means the config file may not be accessible via the expected path.Verify that:
- Config files are installed to
/usr/etc/fluent-bit(due toCMAKE_INSTALL_PREFIX=/usr)- The layout binding and read-config plug are updated to reference the correct path:
$SNAP/usr/etc/fluent-bit#!/bin/bash # Description: Verify the configuration file location after snap installation # This script checks where CMakeLists.txt installs fluent-bit config files # Search for install commands related to fluent-bit config files rg -n "install.*fluent-bit\.conf|install.*etc.*fluent-bit" --type cmakeAlso applies to: 22-22, 69-71
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
snap/gui/fluent-bit.svgis excluded by!**/*.svg
📒 Files selected for processing (1)
snap/snapcraft.yaml(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
PR: fluent/fluent-bit#9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
snap/snapcraft.yaml
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
PR: fluent/fluent-bit#9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit, the correct CMake flag for using system librdkafka is `FLB_PREFER_SYSTEM_LIB_KAFKA=ON`.
Applied to files:
snap/snapcraft.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
🔇 Additional comments (5)
snap/snapcraft.yaml (5)
2-2: Base upgrade to core20 is necessary and appropriate.The upgrade from core18 (Ubuntu 18.04) to core20 (Ubuntu 20.04) is required as core18 has reached end-of-life. This is a valid change.
67-67: Verify the purpose and necessity of thekeep-execstackbuild attribute.The
keep-execstackbuild attribute was added but lacks documentation. This attribute instructs snapcraft to preserve the executable stack, which is sometimes required for certain compiled code but can have security implications.Verify that this attribute is necessary and document why it's needed (e.g., specific library requirements, JIT compilation, etc.).
10-10: Icon path verified—no issues found.The icon file exists at the expected location. The file
./snap/gui/fluent-bit.svgis correctly referenced by thegui/fluent-bit.svgpath insnap/snapcraft.yaml.
42-44: Libssl1.1 compatibility confirmed for Ubuntu 20.04.libssl1.1 is available in Ubuntu 20.04 (Focal) main repositories (including updates/security). The package configuration is correct:
libssl1.1in stage-packages (runtime library) andlibssl-devin build-packages (development headers). The addition oflibyaml-devfor core20 compatibility is appropriate.
22-22: The command paths in snapcraft.yaml are correctly configured and no changes are needed.The verification confirms that when
CMAKE_INSTALL_PREFIX=/usris set,FLB_INSTALL_BINDIRresolves to${CMAKE_INSTALL_FULL_BINDIR}, which evaluates to/usr/bin. The binary is therefore installed to/usr/bin/fluent-bit, and the snapcraft.yaml entries at lines 22 and 30 correctly reference this with the relative pathusr/bin/fluent-bit(which expands to$SNAP/usr/bin/fluent-bitin the snap environment).
Signed-off-by: Hiroshi Hatake <[email protected]>
1 participant
Snap core18 is based on Ubuntu 18.04 and this version of Ubuntu LTS had already been reached EOL.
So, we need to upgrade from core18 to core20 that is based on Ubuntu 20.04.
This upgraded snapcraft can handle with snapcraft 8.x and succeeded to build with it:
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit