Skip to content

Conversation

@jensens
Copy link
Member

@jensens jensens commented Oct 22, 2025

Summary

Fixes #53: Per-package target setting is now correctly used when constructing checkout paths

This PR fixes the bug where individual package target settings were ignored when determining the checkout path. The issue was that config.py line 103 was using the default-target variable instead of the package's individual target setting.

Changes

1. Failing Test (Commit ef5b4d3)

Added test_per_package_target_override() that demonstrates the bug:

  • Tests package with default target
  • Tests package with custom target setting
  • Tests package with interpolated target (e.g., ${settings:docs-directory})

2. The Fix (Commit 733ecf2)

File: src/mxdev/config.py line 104

Changed from:

package.setdefault("path", os.path.join(target, name))

To:

package.setdefault("path", os.path.join(package["target"], name))

This ensures the path uses the package's actual target (either custom or default) instead of always using the default-target.

3. Cross-Platform Test Fix (Commit 5800927)

Updated test to use pathlib.Path().as_posix() for path comparison, which handles both Unix (/) and Windows (\) path separators correctly.

4. Documentation (Commits 733ecf2 + 5800927)

  • Updated CHANGES.md with fix entry
  • Added pathlib preference to CLAUDE.md code style guidelines

Results

✅ All 24 tests pass (14 config + 10 other test files)
✅ All CI checks pass on Ubuntu, Windows, macOS (Python 3.8-3.14)
✅ Packages with custom target settings are now checked out to correct directories
✅ Packages without custom target continue to use default-target
✅ Variable interpolation like ${settings:docs-directory} works correctly

Test Plan

# Run the new test
pytest tests/test_config.py::test_per_package_target_override -v

# Run all config tests
pytest tests/test_config.py -v

# Verify linting
uvx --with tox-uv tox -e lint

Example Usage

[settings]
default-target = sources
docs-directory = documentation

[package-a]
url = https://github.com/example/package-a.git
# No target specified - uses default-target
# Checked out to: sources/package-a

[package-b]
url = https://github.com/example/package-b.git
target = custom-dir
# Checked out to: custom-dir/package-b

[documentation]
url = https://github.com/example/docs.git
target = ${settings:docs-directory}
# Checked out to: documentation/documentation

This test demonstrates the bug where per-package target settings
are ignored. The test will fail with the current code, showing:

Expected: custom-dir/package.with.custom.target
Actual: ./sources/package.with.custom.target

The bug is in config.py line 103, which uses the default-target
variable instead of the package's individual target setting.

Related to #53
The bug was in config.py line 103, which used the 'target' variable
(always equal to default-target) instead of package["target"]
(which may contain a custom per-package target setting).

This fix ensures that:
- Packages with custom target settings are checked out to their
  specified directory
- Packages without custom target continue to use default-target
- Variable interpolation like ${settings:docs-directory} works correctly

The previously failing test now passes, demonstrating the fix works.

Fixes #53
Use pathlib.Path().as_posix() for cross-platform path comparison.
This handles both Unix (/) and Windows (\) path separators correctly
by normalizing both sides of the comparison.

Also update CLAUDE.md to document preference for pathlib over os.path
for path operations in tests and new code.
@jensens jensens marked this pull request as ready for review October 22, 2025 10:26
@jensens jensens merged commit c9ba1ae into main Oct 22, 2025
23 checks passed
@jensens jensens deleted the fix/53-per-package-target-setting branch October 22, 2025 10:26
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.

Target does not work for individual package

2 participants