From 336931a4ca7172d4a21097b2b9a143a1c8df37f0 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Tue, 29 Jul 2025 13:15:57 +0200 Subject: [PATCH] cmd/go: use os.Rename to move files on Windows The Go toolchain has been copying files instead of moving them on Windows since CL 72910. It did this because os.Rename doesn't respect the destination directory ACL permissions, and we want to honor them. The drawback is that copying files is slower than moving them. This CL reintroduces the use of os.Rename, but it also sets the destination file's ACL to inherit the permissions from the destination directory. On my computer this change speeds up a simple "go build" (with warm cache) by 1 second, going from 3 seconds to 2 seconds. Updates #22343 Change-Id: I65b2b6f301e9e18bf2588959764eb06b9a01dfa2 Reviewed-on: https://go-review.googlesource.com/c/go/+/691255 Reviewed-by: Mark Freeman LUCI-TryBot-Result: Go LUCI Reviewed-by: Damien Neil --- src/cmd/go/internal/work/shell.go | 44 ++--------------- src/cmd/go/internal/work/shell_nonwindows.go | 49 +++++++++++++++++++ src/cmd/go/internal/work/shell_windows.go | 37 ++++++++++++++ .../syscall/windows/security_windows.go | 3 ++ src/internal/syscall/windows/types_windows.go | 18 +++++++ .../syscall/windows/zsyscall_windows.go | 27 ++++++++++ 6 files changed, 138 insertions(+), 40 deletions(-) create mode 100644 src/cmd/go/internal/work/shell_nonwindows.go create mode 100644 src/cmd/go/internal/work/shell_windows.go diff --git a/src/cmd/go/internal/work/shell.go b/src/cmd/go/internal/work/shell.go index 284ed26f22..e75b1c33fc 100644 --- a/src/cmd/go/internal/work/shell.go +++ b/src/cmd/go/internal/work/shell.go @@ -132,47 +132,11 @@ func (sh *Shell) moveOrCopyFile(dst, src string, perm fs.FileMode, force bool) e return sh.CopyFile(dst, src, perm, force) } - // On Windows, always copy the file, so that we respect the NTFS - // permissions of the parent folder. https://golang.org/issue/22343. - // What matters here is not cfg.Goos (the system we are building - // for) but runtime.GOOS (the system we are building on). - if runtime.GOOS == "windows" { - return sh.CopyFile(dst, src, perm, force) - } - - // If the destination directory has the group sticky bit set, - // we have to copy the file to retain the correct permissions. - // https://golang.org/issue/18878 - if fi, err := os.Stat(filepath.Dir(dst)); err == nil { - if fi.IsDir() && (fi.Mode()&fs.ModeSetgid) != 0 { - return sh.CopyFile(dst, src, perm, force) - } - } - - // The perm argument is meant to be adjusted according to umask, - // but we don't know what the umask is. - // Create a dummy file to find out. - // This avoids build tags and works even on systems like Plan 9 - // where the file mask computation incorporates other information. - mode := perm - f, err := os.OpenFile(filepath.Clean(dst)+"-go-tmp-umask", os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm) - if err == nil { - fi, err := f.Stat() - if err == nil { - mode = fi.Mode() & 0777 - } - name := f.Name() - f.Close() - os.Remove(name) - } - - if err := os.Chmod(src, mode); err == nil { - if err := os.Rename(src, dst); err == nil { - if cfg.BuildX { - sh.ShowCmd("", "mv %s %s", src, dst) - } - return nil + if err := sh.move(src, dst, perm); err == nil { + if cfg.BuildX { + sh.ShowCmd("", "mv %s %s", src, dst) } + return nil } return sh.CopyFile(dst, src, perm, force) diff --git a/src/cmd/go/internal/work/shell_nonwindows.go b/src/cmd/go/internal/work/shell_nonwindows.go new file mode 100644 index 0000000000..c517fbe196 --- /dev/null +++ b/src/cmd/go/internal/work/shell_nonwindows.go @@ -0,0 +1,49 @@ +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build !windows + +package work + +import ( + "errors" + "io/fs" + "os" + "path/filepath" +) + +// move moves a file from src to dst setting the permissions +// on the destination file to inherit the permissions from the +// destination parent directory. +func (sh *Shell) move(src, dst string, perm fs.FileMode) error { + // If the destination directory has the group sticky bit set, + // we have to copy the file to retain the correct permissions. + // https://golang.org/issue/18878 + if fi, err := os.Stat(filepath.Dir(dst)); err == nil { + if fi.IsDir() && (fi.Mode()&fs.ModeSetgid) != 0 { + return errors.ErrUnsupported + } + } + // The perm argument is meant to be adjusted according to umask, + // but we don't know what the umask is. + // Create a dummy file to find out. + // This works even on systems like Plan 9 where the + // file mask computation incorporates other information. + mode := perm + f, err := os.OpenFile(filepath.Clean(dst)+"-go-tmp-umask", os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm) + if err == nil { + fi, err := f.Stat() + if err == nil { + mode = fi.Mode() & 0777 + } + name := f.Name() + f.Close() + os.Remove(name) + } + + if err := os.Chmod(src, mode); err != nil { + return err + } + return os.Rename(src, dst) +} diff --git a/src/cmd/go/internal/work/shell_windows.go b/src/cmd/go/internal/work/shell_windows.go new file mode 100644 index 0000000000..9b80eab07f --- /dev/null +++ b/src/cmd/go/internal/work/shell_windows.go @@ -0,0 +1,37 @@ +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package work + +import ( + "internal/syscall/windows" + "io/fs" + "os" + "unsafe" +) + +// move moves a file from src to dst, setting the security information +// on the destination file to inherit the permissions from the +// destination parent directory. +func (sh *Shell) move(src, dst string, perm fs.FileMode) (err error) { + if err := os.Rename(src, dst); err != nil { + return err + } + defer func() { + if err != nil { + os.Remove(dst) // clean up if we failed to set the mode or security info + } + }() + if err := os.Chmod(dst, perm); err != nil { + return err + } + // We need to respect the ACL permissions of the destination parent folder. + // https://go.dev/issue/22343. + var acl windows.ACL + if err := windows.InitializeAcl(&acl, uint32(unsafe.Sizeof(acl)), windows.ACL_REVISION); err != nil { + return err + } + secInfo := windows.DACL_SECURITY_INFORMATION | windows.UNPROTECTED_DACL_SECURITY_INFORMATION + return windows.SetNamedSecurityInfo(dst, windows.SE_FILE_OBJECT, secInfo, nil, nil, &acl, nil) +} diff --git a/src/internal/syscall/windows/security_windows.go b/src/internal/syscall/windows/security_windows.go index f0ab52ac81..2c35ad31f4 100644 --- a/src/internal/syscall/windows/security_windows.go +++ b/src/internal/syscall/windows/security_windows.go @@ -262,3 +262,6 @@ func GetSidSubAuthorityCount(sid *syscall.SID) uint8 { defer runtime.KeepAlive(sid) return *(*uint8)(unsafe.Pointer(getSidSubAuthorityCount(sid))) } + +//sys InitializeAcl(acl *ACL, length uint32, revision uint32) (err error) = advapi32.InitializeAcl +//sys SetNamedSecurityInfo(objectName string, objectType SE_OBJECT_TYPE, securityInformation SECURITY_INFORMATION, owner *syscall.SID, group *syscall.SID, dacl *ACL, sacl *ACL) (ret error) = advapi32.SetNamedSecurityInfoW diff --git a/src/internal/syscall/windows/types_windows.go b/src/internal/syscall/windows/types_windows.go index 93664b4b7d..3855df393d 100644 --- a/src/internal/syscall/windows/types_windows.go +++ b/src/internal/syscall/windows/types_windows.go @@ -260,3 +260,21 @@ type FILE_COMPLETION_INFORMATION struct { // https://learn.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-osversioninfoexa // https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/ns-wdm-_osversioninfoexw const VER_NT_WORKSTATION = 0x0000001 + +// https://learn.microsoft.com/en-us/windows/win32/api/accctrl/ne-accctrl-se_object_type +type SE_OBJECT_TYPE uint32 + +const ( + SE_UNKNOWN_OBJECT_TYPE SE_OBJECT_TYPE = 0 + SE_FILE_OBJECT SE_OBJECT_TYPE = 1 +) + +// https://learn.microsoft.com/en-us/windows/win32/secauthz/security-information +type SECURITY_INFORMATION uint32 + +const ( + DACL_SECURITY_INFORMATION SECURITY_INFORMATION = 0x00000004 + UNPROTECTED_DACL_SECURITY_INFORMATION SECURITY_INFORMATION = 0x20000000 +) + +const ACL_REVISION = 2 diff --git a/src/internal/syscall/windows/zsyscall_windows.go b/src/internal/syscall/windows/zsyscall_windows.go index 90cf0b92a4..c08b2ccdba 100644 --- a/src/internal/syscall/windows/zsyscall_windows.go +++ b/src/internal/syscall/windows/zsyscall_windows.go @@ -54,6 +54,7 @@ var ( procGetSidSubAuthorityCount = modadvapi32.NewProc("GetSidSubAuthorityCount") procImpersonateLoggedOnUser = modadvapi32.NewProc("ImpersonateLoggedOnUser") procImpersonateSelf = modadvapi32.NewProc("ImpersonateSelf") + procInitializeAcl = modadvapi32.NewProc("InitializeAcl") procIsValidSid = modadvapi32.NewProc("IsValidSid") procLogonUserW = modadvapi32.NewProc("LogonUserW") procLookupPrivilegeValueW = modadvapi32.NewProc("LookupPrivilegeValueW") @@ -62,6 +63,7 @@ var ( procOpenThreadToken = modadvapi32.NewProc("OpenThreadToken") procQueryServiceStatus = modadvapi32.NewProc("QueryServiceStatus") procRevertToSelf = modadvapi32.NewProc("RevertToSelf") + procSetNamedSecurityInfoW = modadvapi32.NewProc("SetNamedSecurityInfoW") procSetTokenInformation = modadvapi32.NewProc("SetTokenInformation") procProcessPrng = modbcryptprimitives.NewProc("ProcessPrng") procGetAdaptersAddresses = modiphlpapi.NewProc("GetAdaptersAddresses") @@ -166,6 +168,14 @@ func ImpersonateSelf(impersonationlevel uint32) (err error) { return } +func InitializeAcl(acl *ACL, length uint32, revision uint32) (err error) { + r1, _, e1 := syscall.Syscall(procInitializeAcl.Addr(), 3, uintptr(unsafe.Pointer(acl)), uintptr(length), uintptr(revision)) + if r1 == 0 { + err = errnoErr(e1) + } + return +} + func IsValidSid(sid *syscall.SID) (valid bool) { r0, _, _ := syscall.Syscall(procIsValidSid.Addr(), 1, uintptr(unsafe.Pointer(sid)), 0, 0) valid = r0 != 0 @@ -234,6 +244,23 @@ func RevertToSelf() (err error) { return } +func SetNamedSecurityInfo(objectName string, objectType SE_OBJECT_TYPE, securityInformation SECURITY_INFORMATION, owner *syscall.SID, group *syscall.SID, dacl *ACL, sacl *ACL) (ret error) { + var _p0 *uint16 + _p0, ret = syscall.UTF16PtrFromString(objectName) + if ret != nil { + return + } + return _SetNamedSecurityInfo(_p0, objectType, securityInformation, owner, group, dacl, sacl) +} + +func _SetNamedSecurityInfo(objectName *uint16, objectType SE_OBJECT_TYPE, securityInformation SECURITY_INFORMATION, owner *syscall.SID, group *syscall.SID, dacl *ACL, sacl *ACL) (ret error) { + r0, _, _ := syscall.Syscall9(procSetNamedSecurityInfoW.Addr(), 7, uintptr(unsafe.Pointer(objectName)), uintptr(objectType), uintptr(securityInformation), uintptr(unsafe.Pointer(owner)), uintptr(unsafe.Pointer(group)), uintptr(unsafe.Pointer(dacl)), uintptr(unsafe.Pointer(sacl)), 0, 0) + if r0 != 0 { + ret = syscall.Errno(r0) + } + return +} + func SetTokenInformation(tokenHandle syscall.Token, tokenInformationClass uint32, tokenInformation unsafe.Pointer, tokenInformationLength uint32) (err error) { r1, _, e1 := syscall.Syscall6(procSetTokenInformation.Addr(), 4, uintptr(tokenHandle), uintptr(tokenInformationClass), uintptr(tokenInformation), uintptr(tokenInformationLength), 0, 0) if r1 == 0 { -- 2.51.0