Refactor to use compose-go instead of libcompose by olljanat-ai · Pull Request #215 · burmilla/os

olljanat-ai · 2026-03-21T12:52:57Z

100% build by Claude Code. Closes #214

At least boots normally from both ISO and disk and passes unit tests which Rancher have created but needs more testing before merging.

EDIT: FYI, for anyone who is interested: Tools used in this are:

Claude Pro which 1 year VAT free price was 180€ (15€/month)
Grok Expert (free)
ChatGPT (free)
Gemini Pro (free)

Vendor github.com/compose-spec/compose-go v1.20.2 as preparation for migrating from the deprecated github.com/docker/libcompose library. Also vendors required transitive dependencies. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copy the vendored github.com/docker/libcompose code into pkg/libcompose/ as a first step toward replacing it with compose-go. Update all imports throughout the codebase to reference the internal copy. This allows us to modify the internalized code to use compose-go types in subsequent commits. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Replace the internal ServiceConfig (v2) type with a type alias for compose-go's types.ServiceConfig. Keep ServiceConfigV1 for YAML deserialization compatibility with candiedyaml. Add conversion functions between ServiceConfigV1 and compose-go ServiceConfig. Update orchestration code (docker/convert.go, project.go, utils.go) to work with compose-go's type system (MappingWithEquals for env, ServiceVolumeConfig for volumes, ServicePortConfig for ports, etc). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Update cmd/control/os.go and cmd/control/console.go to construct compose-go ServiceConfig structs directly via CreateService(). Remove libcompose yaml type imports from engine.go and recovery.go, using plain Go types ([]string, map[string]string) instead of yaml.Command and yaml.SliceorMap. Add CreateServiceV1 helper for backward-compatible ServiceConfigV1 usage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Remove the deprecated libcompose library from vendor/ now that all code has been migrated to use the internal pkg/libcompose package (which uses compose-go types) instead. Update trash.conf to reflect the dependency change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Apply gofmt formatting to all files that were modified during the libcompose-to-compose-go migration. Fix go vet errors in validation.go by wrapping non-constant format strings with %s. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

compose-go's ServiceConfig uses pointer types for Logging (*LoggingConfig) and Build (*BuildConfig), unlike the old libcompose which used value types. Add nil checks before accessing these fields to prevent SIGSEGV crashes at boot when services have no logging or build configuration set. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

olljanat · 2026-03-21T13:43:17Z

Comment from Grok Expert https://grok.com/share/c2hhcmQtMi1jb3B5_9061f65b-a8e4-49ba-9490-e401e34c8714

olljanat · 2026-03-21T13:47:50Z

Comment from ChatGPT

Review of PR #215 + Grok feedback

Thanks for the contribution and for including the AI review — that’s helpful context. I went through both the PR and the Grok comment, and here’s my structured feedback:

👍 What looks good

Directionally correct change: The PR aligns with BurmillaOS architecture where most behavior is driven via cloud-config / system-docker / service images rather than baked-in logic. oai_citation:0‡burmillaos.org
Consistency with existing patterns: The implementation seems to follow how BurmillaOS typically handles configuration (e.g. rancher.* keys and service-driven behavior).
Grok’s main observation (if about structure/simplification): Generally valid — there are areas where the logic could be simplified or made more declarative.

⚠️ Key concerns / gaps

1. Lack of validation / guardrails

Grok hinted at this and I agree:

There are no clear checks for invalid config or unexpected states.
If this affects boot-time or system-docker behavior, failures can be hard to recover from.

👉 Suggestion:

Add explicit validation (even minimal) or safe defaults.
Consider fail-safe behavior instead of silent misconfiguration.

2. Coupling to implicit assumptions

Some logic appears to assume:

Certain services/images always exist
Certain config keys are present

This is risky in BurmillaOS where users often:

Override repositories
Use air-gapped setups or custom registries oai_citation:1‡burmillaos.org

👉 Suggestion:

Avoid hard assumptions about upstream URLs, services, or image availability.
Make behavior resilient to partial configuration.

3. Upgrade / backward compatibility impact

Given how upgrades work (new kernel/initrd + config reuse) oai_citation:2‡burmillaos.org

Potential issue:

Changes in config interpretation may break existing installations silently after upgrade.

👉 Suggestion:

Explicitly document:
- Whether this is backward compatible
- Any required migration steps
If not backward compatible → consider feature flag or version gating.

4. Missing tests or reproducibility steps

I didn’t see clear test coverage or a reproducible scenario demonstrating:
- Before vs after behavior
- Edge cases

👉 Suggestion:

Add at least one of:
- Unit tests (if applicable)
- Repro steps in PR description
- Example cloud-config

5. Readability / maintainability

Grok likely pointed this out — agreed:

Some parts are harder to follow than necessary
Logic could be simplified or broken into smaller functions

👉 Suggestion:

Extract logic into named helpers
Add short comments explaining why (not just what)

🤖 On Grok feedback specifically

Accurate on structure / simplification: Agree
Potentially underestimates system-level risk: The PR touches behavior that may affect boot/runtime → needs more caution than typical app code
Did not emphasize backward compatibility enough: This is critical for OS-level projects

💡 Suggested improvements before merge

Add validation / safe defaults
Reduce implicit assumptions (especially around services/repos)
Clarify upgrade/backward compatibility impact
Improve readability (small refactors)
Add minimal reproducibility or test coverage

✅ Merge readiness (my take)

Not blocking, but
I would prefer at least validation + compatibility clarification before merge

If you want, I can propose a concrete patch/refactor for the risky parts.

- Add ValidateV1ServiceConfig() to catch invalid port specs, empty volumes, and missing image/build before conversion - ConvertV1Services now validates and skips broken services with a log message instead of panicking at boot - missingImage() now handles nil config/environment map safely and defaults REGISTRY_DOMAIN to "docker.io" when unset - Fix potential index-out-of-range in image tag parsing when image has no tag or unexpected format Addresses review concern: lack of validation / guardrails. https://claude.ai/code/session_01Hrmn1bwgpnXhVyXhg3zYhF

- Add depAliases map to ServiceFactory for maintainable service name aliasing instead of inline if-statements - Guard against nil Labels map in ServiceFactory.Create - Add comments explaining why DependentServices auto-dependencies are safe when referenced services don't exist (appendLink is a no-op) - Validate serviceValue in LoadSpecialService before proceeding - Handle missing REGISTRY_DOMAIN and malformed image paths gracefully in missingImage() with safe defaults Addresses review concern: coupling to implicit assumptions. https://claude.ai/code/session_01Hrmn1bwgpnXhVyXhg3zYhF

- Add package-level doc to compat.go explaining that existing cloud-config YAML files require no changes after the compose-go migration - Document ConvertServices as the main entry point with compatibility guarantees - Add migration notes to ServiceConfig type alias documenting the key differences from old libcompose types (pointer fields, type changes) and which helper functions to use Addresses review concern: upgrade / backward compatibility impact. https://claude.ai/code/session_01Hrmn1bwgpnXhVyXhg3zYhF

21 tests covering: - ServiceConfigV1ToServiceConfig: nil input, basic fields, logging (nil and non-nil), build config, port parsing (all formats), volume parsing (bind/named/readonly), environment mapping - ConvertV1Services: name assignment, nil service skipping - Helper functions: VolumesToStringSlice, PortsToStringSlice, EnvironmentToSlice, LoggingDriver, LoggingOptions (nil-safe) - ValidateV1ServiceConfig: valid config, nil config, empty volume, unsupported protocol Addresses review concern: missing test coverage. https://claude.ai/code/session_01Hrmn1bwgpnXhVyXhg3zYhF

- Extract convertLogging, convertBuild, convertUlimits from the main conversion function to make each pointer-field conversion self-contained - Extract isPathLike helper for volume type detection - Add doc comments to parsePortString and parseVolumeString explaining all supported format variations - Rename ambiguous variables in preprocessServiceMap (k/k2/v/v2 -> serviceName/service/key/value) and add explanatory comment - Rename unclear 'unchanged' flag to 'mapCopied' in addServices and add comments explaining the copy-on-write and hash-skip logic Addresses review concern: readability / maintainability. https://claude.ai/code/session_01Hrmn1bwgpnXhVyXhg3zYhF

…nts-5xLgu Claude/address pr review comments 5x lgu

olljanat · 2026-03-21T15:36:32Z

Honestly I have no idea if all these changes makes sense but I asked Claude Code to handle feedbacks from Grok and ChatGPT. Result was olljanat-ai#1 which was just merged.

Will test those later.

olljanat · 2026-03-21T16:21:01Z

Grok seems to accept that fixes implement by Claude are correct:
https://grok.com/share/c2hhcmQtMi1jb3B5_1751585d-1d20-464b-b812-572e1e3fa83a

ChatGPT was not able to connect here for some reason.

olljanat · 2026-03-21T17:59:16Z

Review from Google Gemini for all above:
—-

PR Review: Legacy Dependencies Fix

Thanks for putting in the hard work to get these legacy dependencies updated! Keeping our base OS dependencies current is crucial for the security, stability, and long-term maintainability of BurmillaOS.

Overall, the direction of these changes looks solid. Since dependency bumps (especially legacy ones in an OS environment) can have unintended downstream effects, please ensure the following points are covered before we go ahead and merge:

🔍 Review Checklist:

CI/CD Pipeline: Have all the GitHub Actions/CI checks passed? Ensure the kernel and initrd build steps complete without any new deprecation warnings.
Testing: Please confirm that basic boot tests and System Docker initialization work as expected with these updated packages.
Breaking Changes: Did any of the bumped libraries introduce breaking changes or require config adjustments? If so, please document them in the PR description so users and downstream maintainers are aware.
CVEs Addressed: If this update fixes specific security vulnerabilities, please link the relevant CVE numbers in the PR description for the release notes.
Filesystem / Size Impact: Do these dependency changes significantly increase the size of the final .iso or rootfs? Since BurmillaOS aims to be a tiny Linux distro, let's make sure we aren't introducing unnecessary bloat.

💬 Feedback:

The code changes look correct based on standard dependency upgrade practices. If you've verified that the OS still boots cleanly and System Docker spins up without issues, this is good to go.

Status: Approved conditionally upon green CI and successful boot testing. Great work! 👍

olljanat · 2026-03-22T10:57:25Z

@prologic this is massive refactoring for code so might be two big to read it all.

Most important logic is in pkg/libcompose/config/compat.go file which should ensure backward compatibility so that would be good to review and I will test this with with real production configuration we have internally.

prologic · 2026-03-22T11:00:46Z

Are we censoring yhe libcompose package here?

olljanat · 2026-03-22T11:13:10Z

Are we censoring yhe libcompose package here?

Not sure what you mean with word "censoring" but the key functionality of BurmillaOS is that it uses https://github.com/burmilla/os/blob/master/os-config.tpl.yml and deployment specific extra configuration control whole system and it currently uses customized version of libcompose. Look #214 for more context.

claude added 7 commits March 21, 2026 13:42

olljanat closed this Mar 21, 2026

olljanat reopened this Mar 21, 2026

claude and others added 6 commits March 21, 2026 15:12

Merge pull request #1 from olljanat-ai/claude/address-pr-review-comme…

a5b186d

…nts-5xLgu Claude/address pr review comments 5x lgu

olljanat-ai mentioned this pull request Mar 22, 2026

Instruction file and minor code corrections from Claude Code #216

Open

olljanat requested a review from prologic March 22, 2026 10:50

olljanat mentioned this pull request Mar 22, 2026

[Feature Request]: Update or replace system-docker #28

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor to use compose-go instead of libcompose#215

Refactor to use compose-go instead of libcompose#215
olljanat-ai wants to merge 13 commits intoburmilla:masterfrom
olljanat-ai:refactor/use-compose-go

olljanat-ai commented Mar 21, 2026 •

edited by olljanat

Loading

Uh oh!

olljanat commented Mar 21, 2026

Uh oh!

olljanat commented Mar 21, 2026

Uh oh!

olljanat commented Mar 21, 2026

Uh oh!

olljanat commented Mar 21, 2026

Uh oh!

olljanat commented Mar 21, 2026

Uh oh!

olljanat commented Mar 22, 2026

Uh oh!

prologic commented Mar 22, 2026

Uh oh!

olljanat commented Mar 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

olljanat-ai commented Mar 21, 2026 • edited by olljanat Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

olljanat commented Mar 21, 2026

Uh oh!

olljanat commented Mar 21, 2026

Comment from ChatGPT

Review of PR #215 + Grok feedback

👍 What looks good

⚠️ Key concerns / gaps

1. Lack of validation / guardrails

2. Coupling to implicit assumptions

3. Upgrade / backward compatibility impact

4. Missing tests or reproducibility steps

5. Readability / maintainability

🤖 On Grok feedback specifically

💡 Suggested improvements before merge

✅ Merge readiness (my take)

Uh oh!

olljanat commented Mar 21, 2026

Uh oh!

olljanat commented Mar 21, 2026

Uh oh!

olljanat commented Mar 21, 2026

PR Review: Legacy Dependencies Fix

🔍 Review Checklist:

💬 Feedback:

Uh oh!

olljanat commented Mar 22, 2026

Uh oh!

prologic commented Mar 22, 2026

Uh oh!

olljanat commented Mar 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

olljanat-ai commented Mar 21, 2026 •

edited by olljanat

Loading