fix(executor): handle empty-string organize path - #1532
Conversation
6693057 to
fe5f1af
Compare
There was a problem hiding this comment.
Pull request overview
Fixes an edge case in the executor’s organize step where an empty-string mapping (organize: {'': ''}) could trigger a self-organize of the install directory, causing content to be removed during the move/copy process. The change makes this mapping a no-op when source and destination resolve to the same directory, and adds an integration test to prevent regressions.
Changes:
- Skip directory organize operations when the resolved destination path equals the source path (prevents self-copy + delete).
- Adjust
get_src_path()validation to correctly handle empty source paths by validating the source directory itself. - Add an integration test covering
organize: {'': ''}duringStep.PRIME.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
craft_parts/executor/organize.py |
Prevents organizing a directory onto itself and fixes path validation for empty organize keys. |
tests/integration/executor/test_organize.py |
Adds an integration test to ensure empty-string organize mappings are treated as a no-op. |
|
Friendly ping on this PR. It has been idle for a while, and it should still be ready from my side. Please let me know if you would like me to rebase, refresh the checks, or adjust anything else. Thanks! |
62e61f6 to
7d62fee
Compare
| real_dst_path = dst_path.resolve() | ||
| else: | ||
| real_dst_path = dst_path | ||
| if real_dst_path == src_path: |
There was a problem hiding this comment.
We should check this with samefile() so we can handle some more esoteric cases.
lengau
left a comment
lengau
left a comment
There was a problem hiding this comment.
LGTM other than one minor nit. Thanks!
(Sorry for the belated review)
|
CI note: the only failing QA job is The failures are JVM-level crashes on s390x (a native allocation failure in Gradle and an OpenJDK SIGSEGV during Maven), while the rest of the matrix passes. The same s390x Java job also passed on a recent I do not have repository write permission to re-run the job. Could someone with access please retry it? |
Summary
Handle
organize: {'': ''}as a no-op when the source and destination resolve to the same directory.Testing
pytest -q tests/integration/executor/test_organize.py -rspytest -q tests/integration/executor/test_environment.py -k expand_environment -rsIssue
Closes #1526
AI assistance
AI assistance was used while addressing review feedback and validating this follow-up. I reviewed and understand the resulting code and tests and remain responsible for the contribution.