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

fix for cp_r_win32 where copying a directory to a non-existant directory would crash #187

Closed
wants to merge 1 commit into from
Closed

fix for cp_r_win32 where copying a directory to a non-existant directory would crash #187

wants to merge 1 commit into from

Conversation

choptastic
Copy link
Contributor

Hi!

Previously, on windows, rebar_file_utils:cp_r_win32 would throw an error copying a directory to a non-existant directory where linux would handle it just fine.

Example from a reltool (assuming site/static exists, but is empty):

{copy,  "../deps/nitrogen_core/www", "site/static/nitrogen"},

On linux, this would work, properly copying "../deps/nitrogen_core/www" to "site/static/nitrogen", creating the "nitrogen" directory in the process. On windows, it would crash because nitrogen didn't exist. This patch fixes that, and will allow it to more closely simulate the linux experience.

To be more specific, for the function call matching

cp_r_win32({true,SourceDir},{false,DestDir})

It would fail, since DestDir was previously checked to be a dir (first line of cp_r_win32(Source,Dest) ) and since it didn't exist, it's also not a Dir. My fix here accounts for that and if it's determined to also not be a file, then it doesn't exist and it attempts to make the directory to copy into.

Thanks,

-Jesse

Previously, on windows, would throw an error where
linux would handle it just fine.

Example from a reltool (assuming site/static exists, but is empty):

{copy,  "../deps/nitrogen_core/www", "site/static/nitrogen"},

On linux, this would work, properly copying "../deps/nitrogen_core/www" to "site/static/nitrogen", creating the "nitrogen" directory in the process.  On windows, it would crash because nitrogen didn't exist.  This patch fixes that, and will allow it to more closely simulate the linux experience.
@MattVonVielen
Copy link
Contributor

Looking at this code, I'm wondering why we even need to do the recursion in code in the first place? Wouldn't it be much simpler to call xcopy once with all the source files and wildcards, just like we do with cp -r on *nix?

If someone can give me a reproduction scenario for this (sample rebar.config and/or project structure) I can try it out with a single xcopy call to prove it out...

@choptastic
Copy link
Contributor Author

I'm no xcopy wizard, so I was just using the framework in place, but I'm assuming there's some limitation with xcopy that makes it not as convenient as cp -r. I honestly haven't checked the xcopy docs.

@choptastic
Copy link
Contributor Author

New Pull request: #190

@choptastic choptastic closed this Feb 17, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants