Skip to content

Commit 336931a

Browse files
committed
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 <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Damien Neil <[email protected]>
1 parent eef5f8d commit 336931a

File tree

6 files changed

+138
-40
lines changed

6 files changed

+138
-40
lines changed

src/cmd/go/internal/work/shell.go

Lines changed: 4 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -132,47 +132,11 @@ func (sh *Shell) moveOrCopyFile(dst, src string, perm fs.FileMode, force bool) e
132132
return sh.CopyFile(dst, src, perm, force)
133133
}
134134

135-
// On Windows, always copy the file, so that we respect the NTFS
136-
// permissions of the parent folder. https://golang.org/issue/22343.
137-
// What matters here is not cfg.Goos (the system we are building
138-
// for) but runtime.GOOS (the system we are building on).
139-
if runtime.GOOS == "windows" {
140-
return sh.CopyFile(dst, src, perm, force)
141-
}
142-
143-
// If the destination directory has the group sticky bit set,
144-
// we have to copy the file to retain the correct permissions.
145-
// https://golang.org/issue/18878
146-
if fi, err := os.Stat(filepath.Dir(dst)); err == nil {
147-
if fi.IsDir() && (fi.Mode()&fs.ModeSetgid) != 0 {
148-
return sh.CopyFile(dst, src, perm, force)
149-
}
150-
}
151-
152-
// The perm argument is meant to be adjusted according to umask,
153-
// but we don't know what the umask is.
154-
// Create a dummy file to find out.
155-
// This avoids build tags and works even on systems like Plan 9
156-
// where the file mask computation incorporates other information.
157-
mode := perm
158-
f, err := os.OpenFile(filepath.Clean(dst)+"-go-tmp-umask", os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
159-
if err == nil {
160-
fi, err := f.Stat()
161-
if err == nil {
162-
mode = fi.Mode() & 0777
163-
}
164-
name := f.Name()
165-
f.Close()
166-
os.Remove(name)
167-
}
168-
169-
if err := os.Chmod(src, mode); err == nil {
170-
if err := os.Rename(src, dst); err == nil {
171-
if cfg.BuildX {
172-
sh.ShowCmd("", "mv %s %s", src, dst)
173-
}
174-
return nil
135+
if err := sh.move(src, dst, perm); err == nil {
136+
if cfg.BuildX {
137+
sh.ShowCmd("", "mv %s %s", src, dst)
175138
}
139+
return nil
176140
}
177141

178142
return sh.CopyFile(dst, src, perm, force)
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build !windows
6+
7+
package work
8+
9+
import (
10+
"errors"
11+
"io/fs"
12+
"os"
13+
"path/filepath"
14+
)
15+
16+
// move moves a file from src to dst setting the permissions
17+
// on the destination file to inherit the permissions from the
18+
// destination parent directory.
19+
func (sh *Shell) move(src, dst string, perm fs.FileMode) error {
20+
// If the destination directory has the group sticky bit set,
21+
// we have to copy the file to retain the correct permissions.
22+
// https://golang.org/issue/18878
23+
if fi, err := os.Stat(filepath.Dir(dst)); err == nil {
24+
if fi.IsDir() && (fi.Mode()&fs.ModeSetgid) != 0 {
25+
return errors.ErrUnsupported
26+
}
27+
}
28+
// The perm argument is meant to be adjusted according to umask,
29+
// but we don't know what the umask is.
30+
// Create a dummy file to find out.
31+
// This works even on systems like Plan 9 where the
32+
// file mask computation incorporates other information.
33+
mode := perm
34+
f, err := os.OpenFile(filepath.Clean(dst)+"-go-tmp-umask", os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
35+
if err == nil {
36+
fi, err := f.Stat()
37+
if err == nil {
38+
mode = fi.Mode() & 0777
39+
}
40+
name := f.Name()
41+
f.Close()
42+
os.Remove(name)
43+
}
44+
45+
if err := os.Chmod(src, mode); err != nil {
46+
return err
47+
}
48+
return os.Rename(src, dst)
49+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package work
6+
7+
import (
8+
"internal/syscall/windows"
9+
"io/fs"
10+
"os"
11+
"unsafe"
12+
)
13+
14+
// move moves a file from src to dst, setting the security information
15+
// on the destination file to inherit the permissions from the
16+
// destination parent directory.
17+
func (sh *Shell) move(src, dst string, perm fs.FileMode) (err error) {
18+
if err := os.Rename(src, dst); err != nil {
19+
return err
20+
}
21+
defer func() {
22+
if err != nil {
23+
os.Remove(dst) // clean up if we failed to set the mode or security info
24+
}
25+
}()
26+
if err := os.Chmod(dst, perm); err != nil {
27+
return err
28+
}
29+
// We need to respect the ACL permissions of the destination parent folder.
30+
// https://go.dev/issue/22343.
31+
var acl windows.ACL
32+
if err := windows.InitializeAcl(&acl, uint32(unsafe.Sizeof(acl)), windows.ACL_REVISION); err != nil {
33+
return err
34+
}
35+
secInfo := windows.DACL_SECURITY_INFORMATION | windows.UNPROTECTED_DACL_SECURITY_INFORMATION
36+
return windows.SetNamedSecurityInfo(dst, windows.SE_FILE_OBJECT, secInfo, nil, nil, &acl, nil)
37+
}

src/internal/syscall/windows/security_windows.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,3 +262,6 @@ func GetSidSubAuthorityCount(sid *syscall.SID) uint8 {
262262
defer runtime.KeepAlive(sid)
263263
return *(*uint8)(unsafe.Pointer(getSidSubAuthorityCount(sid)))
264264
}
265+
266+
//sys InitializeAcl(acl *ACL, length uint32, revision uint32) (err error) = advapi32.InitializeAcl
267+
//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

src/internal/syscall/windows/types_windows.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,3 +260,21 @@ type FILE_COMPLETION_INFORMATION struct {
260260
// https://learn.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-osversioninfoexa
261261
// https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/ns-wdm-_osversioninfoexw
262262
const VER_NT_WORKSTATION = 0x0000001
263+
264+
// https://learn.microsoft.com/en-us/windows/win32/api/accctrl/ne-accctrl-se_object_type
265+
type SE_OBJECT_TYPE uint32
266+
267+
const (
268+
SE_UNKNOWN_OBJECT_TYPE SE_OBJECT_TYPE = 0
269+
SE_FILE_OBJECT SE_OBJECT_TYPE = 1
270+
)
271+
272+
// https://learn.microsoft.com/en-us/windows/win32/secauthz/security-information
273+
type SECURITY_INFORMATION uint32
274+
275+
const (
276+
DACL_SECURITY_INFORMATION SECURITY_INFORMATION = 0x00000004
277+
UNPROTECTED_DACL_SECURITY_INFORMATION SECURITY_INFORMATION = 0x20000000
278+
)
279+
280+
const ACL_REVISION = 2

src/internal/syscall/windows/zsyscall_windows.go

Lines changed: 27 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)