snap: migrate builds to core26 - #7238
Conversation
Assisted-by: Hermes Agent:gpt-5.6-sol
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Assisted-by: Hermes Agent:gpt-5.6-sol
|
@cubic-dev-ai Please re-review the updated head. The PEP 668 finding is fixed in 1c521bf by creating a project-local venv, putting it first on PATH, and installing pip/Jinja2 through that venv. I reproduced the old command failing with |
@kvnloo I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/test_snapcraft_config.py">
<violation number="1" location="tests/test_snapcraft_config.py:30">
P3: These tests are coupled to the exact byte formatting of snapcraft.yaml (4-space and 6-space indentation, exact line endings, literal CRAFT_PART_BUILD quoting), so any benign reformat—changing indentation width, reordering keys, or re-quoting paths—fails these tests even when the build semantics are unchanged. Conversely the assertIn-on-substrings only prove fragments exist, not that they form the intended ordered command. Parsing the file as YAML and asserting on the parsed parts.boinc structure (base, build-packages list, and the ordered override-build steps) makes the regression test robust to formatting while still catching real regressions.</violation>
<violation number="2" location="tests/test_snapcraft_config.py:30">
P3: Both tests re-read and re-parse snapcraft.yaml with fragile exact-whitespace string splits (delimiting on " build-packages:\n", " source: .\n", and " override-build: |\n") and assert exact 6-space-indented lines. Any legitimate YAML reformatting (indentation, quoting, added comments, or reordering unrelated list entries) breaks these tests without reflecting a real regression. Consider parsing the file with a YAML loader and asserting on the parsed override-build string / build-packages list instead, and factor the shared config-path/read logic into a helper.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| config = config_path.read_text(encoding="utf-8") | ||
|
|
||
| self.assertIn("\nbase: core26\n", config) | ||
| build_packages = config.split(" build-packages:\n", 1)[1].split( |
There was a problem hiding this comment.
P3: These tests are coupled to the exact byte formatting of snapcraft.yaml (4-space and 6-space indentation, exact line endings, literal CRAFT_PART_BUILD quoting), so any benign reformat—changing indentation width, reordering keys, or re-quoting paths—fails these tests even when the build semantics are unchanged. Conversely the assertIn-on-substrings only prove fragments exist, not that they form the intended ordered command. Parsing the file as YAML and asserting on the parsed parts.boinc structure (base, build-packages list, and the ordered override-build steps) makes the regression test robust to formatting while still catching real regressions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_snapcraft_config.py, line 30:
<comment>These tests are coupled to the exact byte formatting of snapcraft.yaml (4-space and 6-space indentation, exact line endings, literal CRAFT_PART_BUILD quoting), so any benign reformat—changing indentation width, reordering keys, or re-quoting paths—fails these tests even when the build semantics are unchanged. Conversely the assertIn-on-substrings only prove fragments exist, not that they form the intended ordered command. Parsing the file as YAML and asserting on the parsed parts.boinc structure (base, build-packages list, and the ordered override-build steps) makes the regression test robust to formatting while still catching real regressions.</comment>
<file context>
@@ -0,0 +1,59 @@
+ config = config_path.read_text(encoding="utf-8")
+
+ self.assertIn("\nbase: core26\n", config)
+ build_packages = config.split(" build-packages:\n", 1)[1].split(
+ " source: .\n", 1
+ )[0]
</file context>
| config = config_path.read_text(encoding="utf-8") | ||
|
|
||
| self.assertIn("\nbase: core26\n", config) | ||
| build_packages = config.split(" build-packages:\n", 1)[1].split( |
There was a problem hiding this comment.
P3: Both tests re-read and re-parse snapcraft.yaml with fragile exact-whitespace string splits (delimiting on " build-packages:\n", " source: .\n", and " override-build: |\n") and assert exact 6-space-indented lines. Any legitimate YAML reformatting (indentation, quoting, added comments, or reordering unrelated list entries) breaks these tests without reflecting a real regression. Consider parsing the file with a YAML loader and asserting on the parsed override-build string / build-packages list instead, and factor the shared config-path/read logic into a helper.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_snapcraft_config.py, line 30:
<comment>Both tests re-read and re-parse snapcraft.yaml with fragile exact-whitespace string splits (delimiting on " build-packages:\n", " source: .\n", and " override-build: |\n") and assert exact 6-space-indented lines. Any legitimate YAML reformatting (indentation, quoting, added comments, or reordering unrelated list entries) breaks these tests without reflecting a real regression. Consider parsing the file with a YAML loader and asserting on the parsed override-build string / build-packages list instead, and factor the shared config-path/read logic into a helper.</comment>
<file context>
@@ -0,0 +1,59 @@
+ config = config_path.read_text(encoding="utf-8")
+
+ self.assertIn("\nbase: core26\n", config)
+ build_packages = config.split(" build-packages:\n", 1)[1].split(
+ " source: .\n", 1
+ )[0]
</file context>
|
@kvnloo, please read carefully our AI usage policy. |
| @@ -0,0 +1,59 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Please remove this file, it's not needed.
|
You are right, thank you for the correction. I missed two explicit policy requirements: every AI-assisted commit needs an |
|
This should have remained a draft until the actual Snap build path and the repository's AI-assistance policy were fully verified. I opened it too early and created avoidable review noise. I apologize for the confusion; I’m correcting the commit trailers and scope exactly as requested before asking for any further review. |
|
I am closing this PR rather than rewriting its published commit history. The corrected branch removes the unwanted test file and adds the required Assisted-by trailer to each AI-assisted commit. I will keep the replacement in draft until the actual Snap build path is verified. Sorry again for opening this before those gates were complete. |
1c521bf to
1964c1a
Compare
|
@kvnloo, snap builds have failed. Please take a look. |
|
I reproduced the changed-path failure at the exact PR head: both Snap jobs stop before the BOINC build with The existing |
…-24.04 destructive-mode
Keep libnotify-dev (still resolute/main). Refresh apt and preinstall it before snapcraft pack so craft-parts does not fail on a stale index.
ubuntu-24.04 runners no longer ship azure-cli.sources, so the unconditional sed in arm64/armhf cross-compile setup exits 2 (libs-arm64, libs-armhf, libs-vcpkg-armhf). Guard that sed; keep the ubuntu.sources Architectures line. Preinstall autopoint/gettext on the snap host so destructive-mode core26 pack can run autotools.
snapcraft 9 still indexes build-packages before its own apt-get update. After libnotify/autopoint, snap-amd64 failed looking up libxrender-dev. Host-preinstall it the same way.
AenBleidd
left a comment
There was a problem hiding this comment.
It's better to wait for the official release of the gnome extension rather than making an intermediate solution.
We don't rush with switch to core26, so stop trying to make a weird looking solution that is very hard to support later.
| sudo dpkg --add-architecture arm64 | ||
| sudo sed -i '/^Types: deb$/a Architectures: amd64' /etc/apt/sources.list.d/ubuntu.sources | ||
| sudo sed -i '/^Types: deb$/a Architectures: amd64' /etc/apt/sources.list.d/azure-cli.sources | ||
| if [ -f /etc/apt/sources.list.d/azure-cli.sources ]; then |
| sudo dpkg --add-architecture armhf | ||
| sudo sed -i '/^Types: deb$/a Architectures: amd64' /etc/apt/sources.list.d/ubuntu.sources | ||
| sudo sed -i '/^Types: deb$/a Architectures: amd64' /etc/apt/sources.list.d/azure-cli.sources | ||
| if [ -f /etc/apt/sources.list.d/azure-cli.sources ]; then |
There was a problem hiding this comment.
This change should be removed as well.
Fixes #7199
Description of the change
Migrates the Snap base from
core24tocore26. The existing Python build dependencies now run in a project-local virtual environment so the build does not invoke externally managed system pip under PEP 668.The corrected branch removes the unnecessary test file. Each AI-assisted commit includes the BOINC-required
Assisted-by: Hermes Agent:gpt-5.6-soltrailer.Verification completed
pip install --userfailing withexternally-managed-environmentin Ubuntu 26.04git diff --check upstream/master...HEADpassedDraft status
A complete Snap package build has not yet been verified at this exact head. This PR remains draft until that build path is green.
AI assistance
Hermes Agent using gpt-5.6-sol assisted with investigation, implementation, validation, and review. I reviewed the final one-file diff and the commands listed above.