Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite copier code to improve symlink handling and preserving metadata #8

Merged
merged 2 commits into from
Oct 22, 2024

Conversation

felixfontein
Copy link
Collaborator

@felixfontein felixfontein commented Oct 3, 2024

This is inspired by #7 (comment).

Symlink handling:

  • Symlinks inside the tree are kept as symlinks. (Thus dead symlinks are kept.)
  • Symlinks outside the tree are copied by content. (Dead symlinks to outside are causing errors.)

This is closer to what ansible-galaxy collection build does, except that its outside detection is not correct IMO (the one here should be better). More precisely, as an example, a symlink pointing to ../<collection_directory_name>/galaxy.yml is treated as an outside link by this code, while it is treated as an inside link by ansible-core's code (it computes the absolute path of the destination and checks whether it is prefixed by the absolute path of the collection directory, which in this case would be true). (See below.)

@felixfontein
Copy link
Collaborator Author

Ok, I checked the ansible-core code a bit more: the ../<collection_directory_name>/galaxy.yml symlink is actually changed to a symlink to galaxy.yml, i.e. symlinks are normalized.

I guess we should also do that, then.

@felixfontein felixfontein marked this pull request as draft October 3, 2024 19:39
@felixfontein felixfontein marked this pull request as ready for review October 3, 2024 20:10
@felixfontein felixfontein merged commit e429f1a into ansible-community:main Oct 22, 2024
2 checks passed
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.

1 participant