Skip to content

Conversation

@mairas
Copy link
Contributor

@mairas mairas commented Jan 1, 2026

Summary

  • Fixes assets being installed directly to the package lib directory instead of under the assets/ subdirectory
  • This caused docker-compose.yml files that reference ./assets/<file> to fail because the file was at ./<file> instead
  • Adds regression test to prevent this issue from recurring

Root Cause

The debian/rules.j2 template installed assets to {paths.lib}/{asset.path} but should have been {paths.lib}/assets/{asset.path} to preserve the directory structure.

Test plan

  • Unit tests pass
  • Integration tests pass
  • New regression test added and passes
  • Pre-commit hooks pass

🤖 Generated with Claude Code

…aths

Assets from the source assets/ directory were being installed directly to
the package lib directory, but docker-compose.yml files reference them
with the assets/ prefix (e.g., ./assets/wait-for-authelia.sh).

This fix preserves the assets/ directory structure in the destination path,
ensuring container mounts work correctly.

Adds regression test to prevent this issue from recurring.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mairas
Copy link
Contributor Author

mairas commented Jan 1, 2026

PR Review: fix: install assets to assets/ subdirectory to match docker-compose paths

Summary

This is a clean, focused bug fix that corrects the installation path for assets in the generated Debian rules file. The fix is correct and the regression test is well-designed.

Analysis

Root Cause: The template was installing assets to {paths.lib}/{asset.path} when it should have been {paths.lib}/assets/{asset.path}. This meant docker-compose.yml files referencing ./assets/<file> would fail to find the files.

The Fix: The change in src/generate_container_packages/templates/debian/rules.j2 (lines 34-35) correctly adds the assets/ subdirectory to the destination path:

# Before: debian/{{ package.name }}/{{ paths.lib }}/{{ asset.path }}
# After:  debian/{{ package.name }}/{{ paths.lib }}/assets/{{ asset.path }}

This is consistent with how default-data/ is already handled on line 43, which preserves the directory structure in the same way.

Issues Found

None. The fix is correct and minimal.

Positive Notes

  • Focused fix: Only changes what needs to change - one line in the template plus the test
  • Consistent with existing patterns: The fix mirrors how default-data/ is already handled in the same template
  • Good regression test: The new test test_assets_installed_to_assets_subdirectory clearly documents the expected behavior and would catch this bug if it regresses
  • Clear PR description: The summary explains the root cause and includes a test plan

Minor Observation

The regression test checks for /assets/nginx.conf in the output, which is slightly ambiguous since both the source (assets/nginx.conf) and destination paths contain this substring. However, this is acceptable because:

  1. Before the fix, the destination would not contain /assets/nginx.conf at all (only /<path>/nginx.conf)
  2. The test's docstring clearly explains what it's verifying

Verdict

Conditional approve - The code changes look correct and well-tested. PR can be merged once the remaining CI check (lintian) passes.

CI Status: 2/3 checks passing, 1 still in progress (lintian)

  • checks / tests: SUCCESS
  • checks / version-check: SUCCESS
  • checks / lintian: IN_PROGRESS

@mairas mairas merged commit 3d33b03 into main Jan 1, 2026
3 checks passed
@mairas mairas deleted the fix/assets-install-path branch January 1, 2026 18:04
mairas added a commit to hatlabs/halos-core-containers that referenced this pull request Jan 1, 2026
Rebuild traefik package using container-packaging-tools with the fix
from hatlabs/container-packaging-tools#181 which corrects the assets
installation path to preserve the assets/ subdirectory structure.

This fixes the issue where wait-for-authelia.sh was installed to the
wrong location, causing Traefik to fail to start on fresh installs.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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