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

Fixing Issue #615 (grunt.file.copy preserves permissions) #732

Closed
wants to merge 2 commits into from

Conversation

regular
Copy link

@regular regular commented Mar 17, 2013

unless preservePermissons is explicitly set to false in the options, file.copy will now copy owner, group and mode from the source to the destination.

@ctalkington
Copy link
Member

think it should be the other way around, stating each is a waste of resources unless you really want to preserve so should be false by default.

@regular
Copy link
Author

regular commented Mar 28, 2013

thought was that when you care for this kind of performance optimization, you can have it while the default is the least surprising/least problematic behavior: copy with permissions.
But I could live with the inverted case also.

@joelrbrandt
Copy link

+1 I would really like to see this get merged.

@adambiggs
Copy link

👍 any updates on this?

@vladikoff
Copy link
Member

@adambiggs setting Milestone 0.4.2

@adambiggs
Copy link

Thanks @vladikoff!

@cowboy
Copy link
Member

cowboy commented Oct 8, 2013

I've attempted to address this issue in the pr732 branch by creating a new method, file.setPermissions, which gets called inside file.copy.

This branch needs to be tested on OS X, Windows and Linux before I can merge it into master (which I would like to do ASAP). Also, please let me know if you see any messages like "platform does not appear to support file permissions" when running the tests, and what OS or shell you see them on. I want this to be as cross-platform as possible.

You can see all the changes here: https://github.com/gruntjs/grunt/compare/pr732

@shama
Copy link
Member

shama commented Oct 9, 2013

I'm getting "platform does not appear to support file permissions" on Windows 7 with both git-bash and cmd.exe. See: https://gist.github.com/shama/6896504

@cowboy
Copy link
Member

cowboy commented Oct 9, 2013

Does Windows just not support file permissions? You can see what the unit test is trying to do here:

https://github.com/gruntjs/grunt/blob/pr732/test/grunt/file_test.js#L444-L449

Basically, if a platform / filesystem supports file permissions, we should support that... but I don't know what we need to do to actually support it everywhere. Ideas, @shama?

@cowboy
Copy link
Member

cowboy commented Oct 10, 2013

I've just updated the pr732 branch via 1c263ac to remove what might be unnecessary checks when running permission-based unit tests.

Question: is it possible that when Grunt creates a file, that file will not be able to have its permissions set?

Please test the latest pr732 branch.

@cowboy
Copy link
Member

cowboy commented Jan 22, 2014

I just rebased / force-pushed the pr732 branch. Still looking for answers to my questions. Anybody?

@shama
Copy link
Member

shama commented Jan 22, 2014

I'll check again this evening on my Windows box.

@vladikoff vladikoff mentioned this pull request Jan 25, 2014
15 tasks
@vladikoff
Copy link
Member

Question: is it possible that when Grunt creates a file, that file will not be able to have its permissions set?

From what I gather there could be edge cases with NTFS partitions and IIS Servers.
Example: tjanczuk/iisnode#247 (comment)
So the best Grunt can do is try to set the permissions, but let it go if some chmod issue occurs.

@JJ
Copy link

JJ commented Dec 6, 2015

Should try and fix this? Oldest pull request here :-)

@regular regular closed this Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants