Skip to content

win: Set correct folder permissions after folder creation #38942

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 27, 2020
Merged

win: Set correct folder permissions after folder creation #38942

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions base/file.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

@vtjnash vtjnash Dec 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) :/

Copy link
Contributor Author

@musm musm Dec 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what to do about this.

Copy link
Member

@StefanKarpinski StefanKarpinski Dec 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Contributor Author

@musm musm Dec 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@musm musm Dec 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return path
finally
Libc.free(req)
Expand Down