Skip to content

fix copying top level symlinks to folders in directory unpack_strategy#6141

Merged
MikeMcQuaid merged 1 commit intoHomebrew:masterfrom
chrmoritz:unpackDir
May 17, 2019
Merged

fix copying top level symlinks to folders in directory unpack_strategy#6141
MikeMcQuaid merged 1 commit intoHomebrew:masterfrom
chrmoritz:unpackDir

Conversation

@chrmoritz
Copy link
Copy Markdown
Contributor

@chrmoritz chrmoritz commented May 16, 2019

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

This PR fixes a issue in the directory unpack_strategy, which currently does not preserve top level symlinks pointing to directories. Instead of copying the symlink (as a singe file) we are currently recursively copying the content of the folder to which the symlink points to. At the end the unpacked source does not contain the top level symlink (pointing to a folder within the source) anymore, because brew has converted it to a complete copy of said folder.

This breaks deno (see: Homebrew/homebrew-core#35645 (comment)) and the added test case.

The current code is broken because child.directory? is true for symlinks pointing to folders too, but we should preserve them. That's why an additional check if the child is not a symlink was added to determine if we should recursively copy the contents of the child (in case it's a non symlink folder) or just the file itself.

@MikeMcQuaid
Copy link
Copy Markdown
Member

Great work as usual, thanks @chrmoritz!

@MikeMcQuaid MikeMcQuaid merged commit e098c37 into Homebrew:master May 17, 2019
@chrmoritz chrmoritz deleted the unpackDir branch May 17, 2019 11:01
@lock lock bot added the outdated PR was locked due to age label Jan 1, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants