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

ensure dest exists before copying to it and fix src_dirs symlinking #1770

Merged
merged 7 commits into from
Apr 27, 2018

Conversation

ferd
Copy link
Collaborator

@ferd ferd commented Apr 27, 2018

This PR augments what is found at #1763 by @danikp

Original Description

While using src_dirs options it's possible to set first level directories under app folder, but setting something deeper will fail on copying files to _build folder. so following example will work

{src_dirs, [
  "src",
  "src2"
]}.

and this one not

{src_dirs, [
  "src/lib",
  "src/mail",
  "src/test",
  "src/view",
  "src/websocket"
]}.

failing with error

cp: cannot create directory '/some/path/to/project/_build/default/lib/web/src/lib': No such file or directory

that happens cause cp command not creating missing path parts. This PR comes to solve it by trying to create destination folder with all parent folders in path.


Difference with Original

The original branch had a test we both worked on, but that had an issue when setting multiple values of src_dirs such as {src_dirs, ["alt/nested", "src"]} if either of the directories did not exist in one of the umbrella applications. The original PR relaxed the test so this was not a problem anymore, and this branch reinstates that configuration, and instead fixes the problem by adding a check in the compiler.

It appears that a check to know if the source directory existed was already in place for file copying, but not for symlinking. The check is added to the symlink operation.

This branch has been tested both on Linux and on Windows 10.

Question for @tsloughter and @talentdeficit : any reason why the source directory check was not on symlink calls? I'm afraid of accidentally breaking something, but this seems to work fine.

danikp and others added 6 commits April 22, 2018 09:09
This would cause crashes on linux and force people to have a src_dirs
config that is strictly matching what is on the file system rather than
acting as a specification of those that are valid.

To compare, if lib_dirs worked the same, then any repo that did not both
have apps/ and lib/ would crash, as the spec mentions both options as
valid.
@ferd
Copy link
Collaborator Author

ferd commented Apr 27, 2018

Could be related to #1173

Priv and include dirs need the virtual symlink in order to preserve hook
functionality in some edge cases.
@ferd ferd merged commit a908284 into erlang:master Apr 27, 2018
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.

None yet

3 participants