]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: use os.Rename to move files on Windows
authorqmuntal <quimmuntal@gmail.com>
Tue, 29 Jul 2025 11:15:57 +0000 (13:15 +0200)
committerQuim Muntal <quimmuntal@gmail.com>
Fri, 1 Aug 2025 16:04:50 +0000 (09:04 -0700)
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 <mark@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
src/cmd/go/internal/work/shell.go
src/cmd/go/internal/work/shell_nonwindows.go [new file with mode: 0644]
src/cmd/go/internal/work/shell_windows.go [new file with mode: 0644]
src/internal/syscall/windows/security_windows.go
src/internal/syscall/windows/types_windows.go
src/internal/syscall/windows/zsyscall_windows.go

index 284ed26f223d7d32a304651976df039841e7ab33..e75b1c33fc5a2547652379bd568ce35c7846ab36 100644 (file)
@@ -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 (file)
index 0000000..c517fbe
--- /dev/null
@@ -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 (file)
index 0000000..9b80eab
--- /dev/null
@@ -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)
+}
index f0ab52ac819baf74d29698ff76671c34d648f754..2c35ad31f4d039f0a6ae8aa98079c2d1db2f5c41 100644 (file)
@@ -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
index 93664b4b7da8ca5f2ca963f305ddd223142f0c3a..3855df393d633e03a99ea4ed284b6c47fcf0bfae 100644 (file)
@@ -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
index 90cf0b92a49bc44b8296929d2248f66dac206f40..c08b2ccdbab3d0e671d62be7545f38e5d7b0e9d4 100644 (file)
@@ -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 {