Skip to content

Commit 1e6aaf1

Browse files
valco1994valco1994
authored
Fix wrappers issues in Bazelisk 1.28.0 on Windows (#762)
* Rework wrappers probation order according to #761: * probe tools/bazel.exe on Windows after platform-specific executables and before Windows-specific scripts * probe tools/bazel on Windows with lowest priority (for backward compatibility only) Update core_test.go and README.md to reflect the changes. * Bazel 8.4.2 is absent on https://downloads.sourceforge.net/project/bazel.mirror, use Bazel 8.5.1 instead in the test `test_path_is_consistent_regardless_of_base_url` from bazelisk_test.sh
1 parent 166e156 commit 1e6aaf1

File tree

4 files changed

+109
-48
lines changed

4 files changed

+109
-48
lines changed

‎README.md‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -219,11 +219,15 @@ Bazelisk will try to run a Bazel wrapper from the `tools` directory if present,
219219

220220
Bazelisk looks for the following wrappers, in order:
221221

222-
* `tools/bazel.<OSNAME>-<ARCH>`: An executable that's OS- and platform-specific.
223-
* `tools/bazel.<ARCH>`: An executable that's platform-specific (for cases where your project only supports one operating system anyway).
224-
* `tools/bazel`: An executable or shell script.
225-
* `tools/bazel.ps1`: A PowerShell script on Windows.
226-
* `tools/bazel.bat`: A batch file on Windows.
222+
* `tools/bazel.<OSNAME>-<ARCH>[.exe]`: An executable that's OS- and platform-specific
223+
* `tools/bazel.<ARCH>[.exe]`: An executable that's platform-specific (for cases where your project only supports one operating system anyway)
224+
* `tools/bazel[.exe]`: An executable or shell script
225+
* `tools/bazel.ps1`: A PowerShell script on Windows
226+
* `tools/bazel.bat`: A batch file on Windows
227+
228+
where `.exe` extension is required on Windows.
229+
230+
Also, on Windows `tools/bazel` is allowed with the lowest priority. It is not recommended and can cause issues, but it's supported for backward compatibility.
227231

228232
This behavior can be disabled by setting the environment variable `BAZELISK_SKIP_WRAPPER` to any value (except the empty string) before launching Bazelisk.
229233

‎bazelisk_test.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ EOF
331331
function test_path_is_consistent_regardless_of_base_url() {
332332
setup
333333

334-
echo 8.4.2 > .bazelversion
334+
echo 8.5.1 > .bazelversion
335335

336336
cat >MODULE.bazel <<EOF
337337
print_path = use_repo_rule("//:print_path.bzl", "print_path")

‎core/core.go‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -626,24 +626,36 @@ func maybeDelegateToWrapperFromDir(bazel string, wd string, config config.Config
626626
}
627627

628628
root := ws.FindWorkspaceRoot(wd)
629-
exeSuffix := platforms.DetermineExecutableFilenameSuffix()
629+
630+
ext := platforms.DetermineExecutableFilenameSuffix()
630631

631632
// The list of candidate wrappers must go from most specific to least specific.
632-
candidates := []string{}
633-
osName, err := platforms.DetermineOperatingSystem()
634-
if err == nil {
635-
// We pass platforms.DarwinArm64MinVersion to disable the Darwin x86_64 fallback because this feature
636-
// was added years after Apple Silicon launched and it's not worth trying to be backwards compatible.
637-
arch, err := platforms.DetermineArchitecture(osName, platforms.DarwinArm64MinVersion)
638-
if err == nil {
639-
candidates = append(candidates, filepath.Join(root, wrapperPath+"."+runtime.GOOS+"-"+arch+exeSuffix))
640-
candidates = append(candidates, filepath.Join(root, wrapperPath+"."+arch+exeSuffix))
641-
}
633+
var candidates []string
634+
635+
osName, osErr := platforms.DetermineOperatingSystem()
636+
// We pass platforms.DarwinArm64MinVersion to disable the Darwin x86_64 fallback because this feature
637+
// was added years after Apple Silicon launched and it's not worth trying to be backwards compatible.
638+
arch, archErr := platforms.DetermineArchitecture(osName, platforms.DarwinArm64MinVersion)
639+
640+
if osErr == nil && archErr == nil {
641+
// OS- and architecture-specific wrapper candidates.
642+
candidates = append(candidates, filepath.Join(root, wrapperPath+"."+osName+"-"+arch+ext))
643+
}
644+
if archErr == nil {
645+
// architecture-specific wrapper candidates.
646+
candidates = append(candidates, filepath.Join(root, wrapperPath+"."+arch+ext))
642647
}
643-
candidates = append(candidates, filepath.Join(root, wrapperPath))
648+
candidates = append(candidates, filepath.Join(root, wrapperPath+ext))
649+
644650
if runtime.GOOS == "windows" {
645651
candidates = append(candidates, filepath.Join(root, wrapperPath+".ps1"))
646652
candidates = append(candidates, filepath.Join(root, wrapperPath+".bat"))
653+
// It's left only for backward compatibility. Executable wrapper is supposed to have ".exe" extension on Windows
654+
// and been probed earlier. Script wrappers are supposed to have proper extensions and should be started
655+
// differently depending on them. Script wrapper without an extension is unreliable on Windows (in the same way
656+
// as starting batch wrappers without an explicit cmd.exe usage, see #731 for details), so it should be avoided.
657+
// But in some cases it can work and some users may rely on it, so let it be.
658+
candidates = append(candidates, filepath.Join(root, wrapperPath))
647659
}
648660

649661
for _, wrapper := range candidates {

‎core/core_test.go‎

Lines changed: 75 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,6 @@ func TestMaybeDelegateToNoNonExecutableWrapper(t *testing.T) {
6666
}
6767

6868
func TestMaybeDelegateToOsAndArchSpecificWrapper(t *testing.T) {
69-
// It's not guaranteed that `tools/bazel` is executable on the
70-
// Windows host running this test. Thus the test is skipped on
71-
// this platform to guarantee consistent results.
72-
if runtime.GOOS == "windows" {
73-
return
74-
}
75-
7669
var tmpDir, err = os.MkdirTemp("", "TestMaybeDelegateToOsAndArchSpecificWrapper")
7770
if err != nil {
7871
log.Fatal(err)
@@ -88,33 +81,28 @@ func TestMaybeDelegateToOsAndArchSpecificWrapper(t *testing.T) {
8881
log.Fatal(err)
8982
}
9083

84+
ext := platforms.DetermineExecutableFilenameSuffix()
85+
9186
os.MkdirAll(tmpDir, os.ModeDir|0700)
9287
os.WriteFile(filepath.Join(tmpDir, "WORKSPACE"), []byte(""), 0600)
9388
os.WriteFile(filepath.Join(tmpDir, "BUILD"), []byte(""), 0600)
9489

9590
os.MkdirAll(filepath.Join(tmpDir, "tools"), os.ModeDir|0700)
96-
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel."+runtime.GOOS+"-"+arch), []byte(""), 0700)
91+
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel."+osName+"-"+arch+ext), []byte(""), 0700)
9792
// Also create the standard wrapper to ensure we prefer the os/arch-specific wrapper.
98-
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel"), []byte(""), 0700)
93+
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel"+ext), []byte(""), 0700)
9994
// Also create the plaform-specific wrapper to ensure we prefer the os/arch-specific wrapper.
100-
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel."+arch), []byte(""), 0700)
95+
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel."+arch+ext), []byte(""), 0700)
10196

10297
entrypoint := maybeDelegateToWrapperFromDir("bazel_real", tmpDir, config.Null())
103-
expected := filepath.Join(tmpDir, "tools", "bazel."+runtime.GOOS+"-"+arch)
98+
expected := filepath.Join(tmpDir, "tools", "bazel."+osName+"-"+arch+ext)
10499

105100
if entrypoint != expected {
106101
t.Fatalf("Expected to delegate bazel to %q, but got %q", expected, entrypoint)
107102
}
108103
}
109104

110105
func TestMaybeDelegateToArchSpecificWrapper(t *testing.T) {
111-
// It's not guaranteed that `tools/bazel` is executable on the
112-
// Windows host running this test. Thus the test is skipped on
113-
// this platform to guarantee consistent results.
114-
if runtime.GOOS == "windows" {
115-
return
116-
}
117-
118106
var tmpDir, err = os.MkdirTemp("", "TestMaybeDelegateToArchSpecificWrapper")
119107
if err != nil {
120108
log.Fatal(err)
@@ -130,46 +118,43 @@ func TestMaybeDelegateToArchSpecificWrapper(t *testing.T) {
130118
log.Fatal(err)
131119
}
132120

121+
ext := platforms.DetermineExecutableFilenameSuffix()
122+
133123
os.MkdirAll(tmpDir, os.ModeDir|0700)
134124
os.WriteFile(filepath.Join(tmpDir, "WORKSPACE"), []byte(""), 0600)
135125
os.WriteFile(filepath.Join(tmpDir, "BUILD"), []byte(""), 0600)
136126

137127
os.MkdirAll(filepath.Join(tmpDir, "tools"), os.ModeDir|0700)
138-
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel."+arch), []byte(""), 0700)
128+
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel."+arch+ext), []byte(""), 0700)
139129
// Also create the standard wrapper to ensure we prefer the arch-specific wrapper.
140-
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel"), []byte(""), 0700)
130+
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel"+ext), []byte(""), 0700)
141131

142132
entrypoint := maybeDelegateToWrapperFromDir("bazel_real", tmpDir, config.Null())
143-
expected := filepath.Join(tmpDir, "tools", "bazel."+arch)
133+
expected := filepath.Join(tmpDir, "tools", "bazel."+arch+ext)
144134

145135
if entrypoint != expected {
146136
t.Fatalf("Expected to delegate bazel to %q, but got %q", expected, entrypoint)
147137
}
148138
}
149139

150140
func TestMaybeDelegateToStandardWrapper(t *testing.T) {
151-
// It's not guaranteed that `tools/bazel` is executable on the
152-
// Windows host running this test. Thus the test is skipped on
153-
// this platform to guarantee consistent results.
154-
if runtime.GOOS == "windows" {
155-
return
156-
}
157-
158141
var tmpDir, err = os.MkdirTemp("", "TestMaybeDelegateToStandardWrapper")
159142
if err != nil {
160143
log.Fatal(err)
161144
}
162145
defer os.RemoveAll(tmpDir)
163146

147+
ext := platforms.DetermineExecutableFilenameSuffix()
148+
164149
os.MkdirAll(tmpDir, os.ModeDir|0700)
165150
os.WriteFile(filepath.Join(tmpDir, "WORKSPACE"), []byte(""), 0600)
166151
os.WriteFile(filepath.Join(tmpDir, "BUILD"), []byte(""), 0600)
167152

168153
os.MkdirAll(filepath.Join(tmpDir, "tools"), os.ModeDir|0700)
169-
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel"), []byte(""), 0700)
154+
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel"+ext), []byte(""), 0700)
170155

171156
entrypoint := maybeDelegateToWrapperFromDir("bazel_real", tmpDir, config.Null())
172-
expected := filepath.Join(tmpDir, "tools", "bazel")
157+
expected := filepath.Join(tmpDir, "tools", "bazel"+ext)
173158

174159
if entrypoint != expected {
175160
t.Fatalf("Expected to delegate bazel to %q, but got %q", expected, entrypoint)
@@ -258,6 +243,66 @@ func TestMaybeDelegateToPowershellOverBatchWrapper(t *testing.T) {
258243
}
259244
}
260245

246+
func TestMaybeDelegateToPowershellOverNoExtensionWrapper(t *testing.T) {
247+
tmpDir, err := os.MkdirTemp("", "TestMaybeDelegateToPowershellOverBatchWrapper")
248+
if err != nil {
249+
log.Fatal(err)
250+
}
251+
defer os.RemoveAll(tmpDir)
252+
253+
os.MkdirAll(tmpDir, os.ModeDir|0700)
254+
os.WriteFile(filepath.Join(tmpDir, "WORKSPACE"), []byte(""), 0600)
255+
os.WriteFile(filepath.Join(tmpDir, "BUILD"), []byte(""), 0600)
256+
257+
os.MkdirAll(filepath.Join(tmpDir, "tools"), os.ModeDir|0700)
258+
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel.ps1"), []byte(""), 0700)
259+
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel"), []byte(""), 0700)
260+
261+
entrypoint := maybeDelegateToWrapperFromDir("bazel_real", tmpDir, config.Null())
262+
263+
// On Windows a PowerShell script should be chosen over the wrapper without an extension
264+
var expected string
265+
if runtime.GOOS == "windows" {
266+
expected = filepath.Join(tmpDir, "tools", "bazel.ps1")
267+
} else {
268+
expected = filepath.Join(tmpDir, "tools", "bazel")
269+
}
270+
271+
if entrypoint != expected {
272+
t.Fatalf("Expected to delegate bazel to %q, but got %q", expected, entrypoint)
273+
}
274+
}
275+
276+
func TestMaybeDelegateToBatchOverNoExtensionWrapper(t *testing.T) {
277+
tmpDir, err := os.MkdirTemp("", "TestMaybeDelegateToPowershellOverBatchWrapper")
278+
if err != nil {
279+
log.Fatal(err)
280+
}
281+
defer os.RemoveAll(tmpDir)
282+
283+
os.MkdirAll(tmpDir, os.ModeDir|0700)
284+
os.WriteFile(filepath.Join(tmpDir, "WORKSPACE"), []byte(""), 0600)
285+
os.WriteFile(filepath.Join(tmpDir, "BUILD"), []byte(""), 0600)
286+
287+
os.MkdirAll(filepath.Join(tmpDir, "tools"), os.ModeDir|0700)
288+
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel.bat"), []byte(""), 0700)
289+
os.WriteFile(filepath.Join(tmpDir, "tools", "bazel"), []byte(""), 0700)
290+
291+
entrypoint := maybeDelegateToWrapperFromDir("bazel_real", tmpDir, config.Null())
292+
293+
// On Windows a batch script should be chosen over the wrapper without an extension
294+
var expected string
295+
if runtime.GOOS == "windows" {
296+
expected = filepath.Join(tmpDir, "tools", "bazel.bat")
297+
} else {
298+
expected = filepath.Join(tmpDir, "tools", "bazel")
299+
}
300+
301+
if entrypoint != expected {
302+
t.Fatalf("Expected to delegate bazel to %q, but got %q", expected, entrypoint)
303+
}
304+
}
305+
261306
// Completion Tests
262307

263308
func TestIsCompletionCommand(t *testing.T) {

0 commit comments

Comments
 (0)