win: Set correct folder permissions after folder creation #38942
Conversation
libuv's mkdir does not implement the mode argument on Windows, as a result the folder mode is never passed on the Windows platform. Forcibly change the folder permissions to agree with the mode passed into `mkdir`.
musm
added
backport 1.5
backport 1.6
|
Is it possible to test this? |
|
I don't know, |
| @@ -179,6 +179,8 @@ function mkdir(path::AbstractString; mode::Integer = 0o777) | |||
| uv_error("mkdir($(repr(path)); mode=0o$(string(mode,base=8)))", ret) | |||
| end | |||
| ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{Cvoid},), req) | |||
| # mode is not implemented in mkdir in libuv yet, so do it here | |||
| Sys.iswindows() && chmod(path, mode) | |||
There was a problem hiding this comment.
Per documentation, this mode argument should be restricted by the umask, but there's no good way to get the current umask (libuv uses umask(current_umask = umask(0)), though that's racy and thus a bad idea) :/
There was a problem hiding this comment.
I don't know what to do about this.
There was a problem hiding this comment.
@vtjnash: do you have a suggestion of what should be done here aside from leaving a comment in case we ever have a umask function that is able to get the umask?
There was a problem hiding this comment.
I built Julia with the "offending" libuv commit reverted and the issue persists.
If you want to try it out add the following to your Make.user
override XC_HOST = x86_64-w64-mingw32
override JULIA_CPU_TARGET=generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)
override USE_BINARYBUILDER_LIBUV=0
override LIBUV_GIT_URL=git://github.com/musm/libuv.git
override LIBUV_BRANCH=julia-uv2-1.39.0
override LIBUV_SHA1=b17d437a171a9cd58459b4736b89298bd19e0c7e
Then run > $env:JULIA_DEPOT_PATH="C:\.julia-dev" in PowerShell and run Julia and add any package. Unless something was built incorrectly (and I don't think it was), the linked issue persists.
So clearly it shouldn't be libuv commit that caused this issue in the 1.5.3 upgrade: #38122, right?
I was almost sure it was the offending libuv commit, but now I don't think so. For one, I checked that child folders created in the parent C:\\ through mkdir in Julia 1.5.2 and Julia 1.5.3 and both had identical permissions. Surprisingly, neither had write permissions, but that doesn't affect things it looks like.
Ok so then I kept digging.
Looking at:
v1.5.2...v1.5.3
And doing some more digging, it looks like a Pkg bump might have caused the permissions changes. In particular, my hunch was that this change could have caused this https://github.com/JuliaLang/Pkg.jl/pull/2150/files#diff-9a6a909fb1a0128f652bc4ca79a4df77d5dbb6bafeac08d5cc2e6e9be9bc07fbR230
So let's test and revert that line.....and once again, unlink permission errors. I also tried reverting 0305b7c and again the same issue.
So I'm at wits end on what's actually causing the linked issue.
There was a problem hiding this comment.
After a careful git bisect I'm lead to the following commits triggering this issue on 1.5.3
JuliaLang/libuv@794bfdf
JuliaLang/libuv@1dcf324
julia-master has some additional changes that cause further bugs that mask this, but on 1.5.3, those are the offending commits.
|
Merging this since it feels pretty important to have #38411 fixed for the 1.6 beta. @staticfloat it would be good if you could look at JuliaLang/libuv@794bfdf, JuliaLang/libuv@1dcf324 that seems to have caused Julia to be broken on some Windows installation and comment on if everything is ok with this PR or there is something that has to be fixed / tweaked in our libuv changes. |
libuv's mkdir does not implement the mode argument on Windows, as a result the folder mode is never passed on the Windows platform. Forcibly change the folder permissions to agree with the mode passed into `mkdir`. (cherry picked from commit 556cee7)
|
The best thing is if someone who encounters a problem could give the output of |
|
@musm, since you could reproduce, perhaps you could show the output of those commands? |
|
Here you are: fresh installation of julia1.5.3, with removed folder This is how get the precompilation error And here is the output of The folder |
|
I personally think we should probably revert this when we have the actual fix, because I'm slightly uncomfortable Here is the comparison of icalcs (see below) On 1.5.3 there's the failing to precompile |
|
@musm as I cannot delve into code (at least until I understand some more Julia) to find when and how they are created, all I can direct my fingers is the creation of packages's files, and those of "tmp" files created during compilation, is where to look first. Also "copy" process is not affected and "jl_2E8.tmp" seems to be first copied into "wLPFy_s6eFQ.ji" then tried to be deleted. Is this merge covered the file creation or folder creation? |
|
Yes, that's right, I didn't realize the exact failure point until looking at things again today and after several unsuccessful attempts earlier this week. I have made some progress in nailing down what exactly is happening and the failure point, but will still need to investigate a fix. @staticfloat The temporary cache file has the following differences in permissions v1.5.2C:\test-dev-1.5\compiled\v1.5\jl_10B0.tmp BUILTIN\Administrators:(I)(F) >v1.5.2 (where we made libuv ACL changes)
This is a problem because when we go to Do you think this is related to an ACL ordering issue? |
…liaLang#38942)" This reverts commit 556cee7.
KristofferC
removed
backport 1.5
backport 1.6
|
@musm , do you know if if it is used all the time, then I can say I narrowed the problem to steps to produce to check:
|
|
@yilmazdurmaz Libuv is used almost everywhere we need IO. Thanks for tracking this down initially. At this point we have a good understanding of what is going on. We will likely eventually need to un-revert this, but it looks like we will first need to patch up our libuv fork. The issue is that the |
libuv's mkdir does not implement the mode argument on Windows, as a result the folder mode is never passed on the Windows platform. Forcibly change the folder permissions to agree with the mode passed into `mkdir`. (cherry picked from commit 556cee7)
…38942) libuv's mkdir does not implement the mode argument on Windows, as a result the folder mode is never passed on the Windows platform. Forcibly change the folder permissions to agree with the mode passed into `mkdir`.
…liaLang#38942)" (JuliaLang#39014) This reverts commit 556cee7.
libuv's mkdir does not implement the mode argument on Windows, as a result the folder mode is never passed on the Windows platform. Forcibly change the folder permissions to agree with the mode passed into `mkdir`. (cherry picked from commit 556cee7)
libuv's
uv_fs_mkdirdoes not implement the mode argument on Windows, as aresult the folder mode is never passed on the Windows platform.
This PR:
Forcibly change the folder permissions to agree with the mode passed into
mkdir.Fixes: #38411
TODO: remove when libuv properly sets the
modeargument inuv_fs_mkdir