lhumina_code/hero_services
23
0
Fork 0
Code Issues 2 Pull requests Projects Releases Packages 1 Wiki Activity Actions

fix: Docker build failures for service installation #10

Closed
delandtj wants to merge 2 commits from fix/docker-build-failures into development
pull from: fix/docker-build-failures
merge into: lhumina_code:development
Conversation 2 Commits 2 Files changed 8 +39 -13
delandtj commented 2026-02-11 17:06:46 +00:00
Member
Copy link
  • Add libseccomp-dev and libcap-ng-dev to builder stage (zinit link deps)
  • Add optional build.cmd field for services without make install targets
  • Add optional build.binary field for correct install tracking
  • Remove CARGO_TARGET_DIR from service builds (broke Makefile target paths)
  • Ensure dirs exist in _InstallService before writing tracking info
  • Add custom build commands for hero_forge, hero_runner, hero_supervisor, mycelium
  • Fix hero_aibroker binary name detection (exec starts with sh -c)
- Add libseccomp-dev and libcap-ng-dev to builder stage (zinit link deps) - Add optional build.cmd field for services without make install targets - Add optional build.binary field for correct install tracking - Remove CARGO_TARGET_DIR from service builds (broke Makefile target paths) - Ensure dirs exist in _InstallService before writing tracking info - Add custom build commands for hero_forge, hero_runner, hero_supervisor, mycelium - Fix hero_aibroker binary name detection (exec starts with sh -c)
fix: Docker build failures for service installation
Some checks are pending
Build and Test / build (pull_request) Waiting to run
Details
3366d6fc18
 
- Add libseccomp-dev and libcap-ng-dev to builder stage (zinit link deps)
- Add optional build.cmd field for services without make install targets
- Add optional build.binary field for correct install tracking
- Remove CARGO_TARGET_DIR from service builds (broke Makefile target paths)
- Ensure dirs exist in _InstallService before writing tracking info
- Add custom build commands for hero_forge, hero_runner, hero_supervisor, mycelium
- Fix hero_aibroker binary name detection (exec starts with sh -c)
thabeta commented 2026-02-11 17:23:20 +00:00
Owner
Copy link

Critical Review Comments

  • Docker Dependencies: The addition of libseccomp-dev and libcap-ng-dev in the builder stage is correct for compilation. However, please verify if these are required as runtime dependencies in the final image stage. If the binaries are dynamically linked, the app will fail to start without the runtime libs.
  • Shell Execution: Switching to sh -c for build.cmd adds flexibility but introduces shell-injection risks if service configs are ever pulled from external/untrusted sources. Consider if we can execute the binary directly without a shell wrapper.
  • Cargo Cache: Removing CARGO_TARGET_DIR disables global build caching. This will significantly slow down builds in environments where multiple services are built sequentially. Can we resolve the path issue instead of removing the env var?
  • Binary Tracking: The binary field is a good stop-gap, but we should eventually aim for a "convention over configuration" approach where the crate name always matches the service name.
### Critical Review Comments - **Docker Dependencies:** The addition of `libseccomp-dev` and `libcap-ng-dev` in the builder stage is correct for compilation. However, please verify if these are required as runtime dependencies in the final image stage. If the binaries are dynamically linked, the app will fail to start without the runtime libs. - **Shell Execution:** Switching to `sh -c` for `build.cmd` adds flexibility but introduces shell-injection risks if service configs are ever pulled from external/untrusted sources. Consider if we can execute the binary directly without a shell wrapper. - **Cargo Cache:** Removing `CARGO_TARGET_DIR` disables global build caching. This will significantly slow down builds in environments where multiple services are built sequentially. Can we resolve the path issue instead of removing the env var? - **Binary Tracking:** The `binary` field is a good stop-gap, but we should eventually aim for a "convention over configuration" approach where the crate name always matches the service name.
Merge branch 'development' into fix/docker-build-failures
Some checks failed
Build and Test / build (pull_request) Failing after 2s
Details
7ecd9fb7da
mik-tf commented 2026-02-11 17:38:42 +00:00
Owner
Copy link

Good points @thabeta

If the binaries are dynamically linked, the app will fail to start without the runtime libs.

Should we aim for statically linked throughout?

Good points @thabeta `If the binaries are dynamically linked, the app will fail to start without the runtime libs.` Should we aim for statically linked throughout?
delandtj closed this pull request 2026-02-12 10:05:27 +00:00
Some checks failed
Build and Test / build (pull_request) Failing after 2s
Details

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lhumina_code/hero_services!10
No description provided.