Preserve chmod and chown from overwritten file #13
Conversation
sheerun
force-pushed
the
chmod
branch
2 times, most recently
from
370fc3e to
4cd47bb
Compare
sheerun
force-pushed
the
chmod
branch
2 times, most recently
from
b5b9935 to
ceac606
Compare
|
@jimbojw @sindresorhus @Qix- could you test? |
You probably didn't mean to change this. |
|
Hmm I need to fix sync version as well |
sheerun
force-pushed
the
chmod
branch
2 times, most recently
from
7483412 to
7308b41
Compare
|
Everything should be ok now. I've also fixed chmod behavior on node 0.8 which was broken before (node 0.8 doesn't support mode for writeFile). |
|
I have tested and confirmed that this fixes sindresorhus/update-notifier#76. Nice work @sheerun! |
|
Looks like it worked 💃 |
|
Why the |
| // Default behavior is to copy it from original file | ||
| fs.stat(filename, function (err, stats) { | ||
| if (!err && stats && !options.mode) { | ||
| options.mode = stats.mode |
There was a problem hiding this comment.
I'm not comfortable mutating the options object folks pass in. Maybe the mode & chown values should be passed to the callback instead?
|
There's no other way of integration testing "chmod" without enabling sudo. Please try to disable it and observe tests fail. |
|
All fixed |
|
Thanks @sheerun! |
|
I've also fixed option overriding issue in sync version |
|
Linked some related bugs to raise visibility. |
|
@iarna Any plans to merge it? |
|
Yeah, that makes sense, I'll merge this shortly. |
|
Merged as 36f5104 and released in v1.2.0 |
Among others it fixes: