Repository navigation
Investigate flaky test-fs-copyfile #15394
Description
Activity
I can reproduce this; the files do get different permissions. This should be the relevant output:
mkdir("/home/sqrt/src/node/test/tmp", 0777) = 0 open("/home/sqrt/src/node/test/tmp/copyfile.out", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 12 close(12) = 0 open("/home/sqrt/src/node/test/fixtures/a.js", O_RDONLY|O_CLOEXEC) = 12 fstat(12, {st_mode=S_IFREG|0644, st_size=1469, ...}) = 0 open("/home/sqrt/src/node/test/tmp/copyfile.out", O_WRONLY|O_CREAT|O_CLOEXEC, 0100644) = 13 sendfile(13, 12, [0] => [1469], 1469) = 1469 close(12) = 0 close(13) = 0 open("/home/sqrt/src/node/test/fixtures/a.js", O_RDONLY|O_CLOEXEC) = 12 fstat(12, {st_mode=S_IFREG|0644, st_size=1469, ...}) = 0 read(12, "// Copyright Joyent, Inc. and ot"..., 1469) = 1469 close(12) = 0 stat("/home/sqrt/src/node/test/fixtures/a.js", {st_mode=S_IFREG|0644, st_size=1469, ...}) = 0 open("/home/sqrt/src/node/test/tmp/copyfile.out", O_RDONLY|O_CLOEXEC) = 12 fstat(12, {st_mode=S_IFREG|0664, st_size=1469, ...}) = 0 read(12, "// Copyright Joyent, Inc. and ot"..., 1469) = 1469 close(12) = 0 stat("/home/sqrt/src/node/test/tmp/copyfile.out", {st_mode=S_IFREG|0664, st_size=1469, ...}) = 0
I’m not seeing any part of the copy code that would make sure the target file gets the original file mode.
Ah. Yea, it looks like a
fchmod()is needed.@addaleax does this commit fix the problem for you?
I think both behaviours could make sense depending on the actual use case, though…
What use cases do you have in mind?
@cjihrig I think I remember cases where I had applications write history files (à la
.bash_history), which implemented that by writing to a new file, then copying at the end of the session instead of just appending to the original file, which always accidentally made the file mode match the one the temp file got by default rather than the one I explicitly gave to that history file…idk, it was nothing more than a mild annoyance. I guess all I’m saying is, sometimes people give their files a certain mode for a reason, and sometimes (a lot of the time?) copying should respect that, since the semantics of a file and therefore its security properties are usually defined via its path, not where the contents were copied from.
I agree that copying should retain the original mode. You said both behaviours could make sense. I guess my question is, when would you not want to copy the mode in a standard copy operation? I know Apple's
copyfile()lets you copy different aspects of the file, such as data only, but for a basic cross platformcpoperation, I think we'd always want everything.I'm only referring to the case of
cp src dest, wheredestshould have the same mode assrc. I think we're on the same page, just failing to communicate it properly :-)Yeah, maybe to use more technical terms, I think sometimes you want the effect of
cp src destand sometimes you want effect ofcat < src > dest2 remaining items
- added a commit that references this issue
on Sep 14, 2017 This test isn't flaky, it's platform dependent and has been failing 100% of the time for me for the last three weeks. Since this feature exists on v8.x branch it should either be fixed soon or be removed until it can be fixed.
It should be fixed by the next libuv update.
This is a
umaskissue with the test, it expects0022for the test to pass, fails with0002Any ETA on next libuv release? I also see this every time when running tests (on Linux).
Aiming for tomorrow or the day after. You can try applying libuv/libuv@eaf25ae locally and see if it solves the problem for you.
- added 2 commits that reference this issue
on Oct 7, 2017 - added a commit that references this issue
on Oct 12, 2017 - added a commit that references this issue
on Oct 17, 2017 - added a commit that references this issue
on Oct 25, 2017
Ping @cjihrig ... seeing some flakiness in the test. This is the second time I've seen this running locally on Ubuntu.