Skip to content

Commit a527c7c

Browse files
authored
Fixed compiler-cache on Windows when there are non-ASCII characters in file path (#2733)
* Increased logging during compile * Added integration test * Fixed dependency file parsing on Windows * Fixed panic in convertAnsiBytesToString implementation
1 parent 812e621 commit a527c7c

File tree

5 files changed

+190
-45
lines changed

5 files changed

+190
-45
lines changed

Diff for: go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ require (
3939
go.bug.st/f v0.4.0
4040
go.bug.st/relaxed-semver v0.12.0
4141
go.bug.st/testifyjson v1.2.0
42+
golang.org/x/sys v0.26.0
4243
golang.org/x/term v0.25.0
4344
golang.org/x/text v0.19.0
4445
google.golang.org/genproto/googleapis/rpc v0.0.0-20240814211410-ddb44dafa142
@@ -100,7 +101,6 @@ require (
100101
golang.org/x/mod v0.18.0 // indirect
101102
golang.org/x/net v0.28.0 // indirect
102103
golang.org/x/sync v0.8.0 // indirect
103-
golang.org/x/sys v0.26.0 // indirect
104104
golang.org/x/tools v0.22.0 // indirect
105105
gopkg.in/ini.v1 v1.67.0 // indirect
106106
gopkg.in/warnings.v0 v0.1.2 // indirect
+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// This file is part of arduino-cli.
2+
//
3+
// Copyright 2024 ARDUINO SA (http://www.arduino.cc/)
4+
//
5+
// This software is released under the GNU General Public License version 3,
6+
// which covers the main part of arduino-cli.
7+
// The terms of this license can be found at:
8+
// https://www.gnu.org/licenses/gpl-3.0.en.html
9+
//
10+
// You can be released from the requirements of the above licenses by purchasing
11+
// a commercial license. Buying such a license is mandatory if you want to
12+
// modify or otherwise use the software for commercial activities involving the
13+
// Arduino software without disclosing the source code of your own applications.
14+
// To purchase a commercial license, send an email to [email protected].
15+
16+
//go:build !windows
17+
18+
package utils
19+
20+
import (
21+
"errors"
22+
)
23+
24+
// placeholder for non-Windows machines
25+
func convertAnsiBytesToString([]byte) (string, error) {
26+
return "", errors.New("unimplemented")
27+
}
+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// This file is part of arduino-cli.
2+
//
3+
// Copyright 2024 ARDUINO SA (http://www.arduino.cc/)
4+
//
5+
// This software is released under the GNU General Public License version 3,
6+
// which covers the main part of arduino-cli.
7+
// The terms of this license can be found at:
8+
// https://www.gnu.org/licenses/gpl-3.0.en.html
9+
//
10+
// You can be released from the requirements of the above licenses by purchasing
11+
// a commercial license. Buying such a license is mandatory if you want to
12+
// modify or otherwise use the software for commercial activities involving the
13+
// Arduino software without disclosing the source code of your own applications.
14+
// To purchase a commercial license, send an email to [email protected].
15+
16+
package utils
17+
18+
import (
19+
"golang.org/x/sys/windows"
20+
)
21+
22+
func convertAnsiBytesToString(data []byte) (string, error) {
23+
if len(data) == 0 {
24+
return "", nil
25+
}
26+
dataSize := int32(len(data))
27+
size, err := windows.MultiByteToWideChar(windows.GetACP(), 0, &data[0], dataSize, nil, 0)
28+
if err != nil {
29+
return "", err
30+
}
31+
utf16 := make([]uint16, size)
32+
if _, err := windows.MultiByteToWideChar(windows.GetACP(), 0, &data[0], dataSize, &utf16[0], size); err != nil {
33+
return "", err
34+
}
35+
return windows.UTF16ToString(utf16), nil
36+
}

Diff for: internal/arduino/builder/internal/utils/utils.go

+66-44
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package utils
1717

1818
import (
1919
"os"
20+
"runtime"
2021
"strings"
2122
"unicode"
2223

@@ -32,33 +33,36 @@ import (
3233
func ObjFileIsUpToDate(sourceFile, objectFile, dependencyFile *paths.Path) (bool, error) {
3334
logrus.Debugf("Checking previous results for %v (result = %v, dep = %v)", sourceFile, objectFile, dependencyFile)
3435
if objectFile == nil || dependencyFile == nil {
35-
logrus.Debugf("Not found: nil")
36+
logrus.Debugf("Object file or dependency file not provided")
3637
return false, nil
3738
}
3839

3940
sourceFile = sourceFile.Clean()
4041
sourceFileStat, err := sourceFile.Stat()
4142
if err != nil {
43+
logrus.Debugf("Could not stat source file: %s", err)
4244
return false, err
4345
}
4446

4547
objectFile = objectFile.Clean()
4648
objectFileStat, err := objectFile.Stat()
4749
if err != nil {
4850
if os.IsNotExist(err) {
49-
logrus.Debugf("Not found: %v", objectFile)
51+
logrus.Debugf("Object file not found: %v", objectFile)
5052
return false, nil
5153
}
54+
logrus.Debugf("Could not stat object file: %s", err)
5255
return false, err
5356
}
5457

5558
dependencyFile = dependencyFile.Clean()
5659
dependencyFileStat, err := dependencyFile.Stat()
5760
if err != nil {
5861
if os.IsNotExist(err) {
59-
logrus.Debugf("Not found: %v", dependencyFile)
62+
logrus.Debugf("Dependency file not found: %v", dependencyFile)
6063
return false, nil
6164
}
65+
logrus.Debugf("Could not stat dependency file: %s", err)
6266
return false, err
6367
}
6468

@@ -71,61 +75,79 @@ func ObjFileIsUpToDate(sourceFile, objectFile, dependencyFile *paths.Path) (bool
7175
return false, nil
7276
}
7377

74-
rows, err := dependencyFile.ReadFileAsLines()
78+
depFileData, err := dependencyFile.ReadFile()
7579
if err != nil {
80+
logrus.Debugf("Could not read dependency file: %s", dependencyFile)
7681
return false, err
7782
}
7883

79-
rows = f.Map(rows, removeEndingBackSlash)
80-
rows = f.Map(rows, strings.TrimSpace)
81-
rows = f.Map(rows, unescapeDep)
82-
rows = f.Filter(rows, f.NotEquals(""))
84+
checkDepFile := func(depFile string) (bool, error) {
85+
rows := strings.Split(strings.Replace(depFile, "\r\n", "\n", -1), "\n")
86+
rows = f.Map(rows, removeEndingBackSlash)
87+
rows = f.Map(rows, strings.TrimSpace)
88+
rows = f.Map(rows, unescapeDep)
89+
rows = f.Filter(rows, f.NotEquals(""))
8390

84-
if len(rows) == 0 {
85-
return true, nil
86-
}
87-
88-
firstRow := rows[0]
89-
if !strings.HasSuffix(firstRow, ":") {
90-
logrus.Debugf("No colon in first line of depfile")
91-
return false, nil
92-
}
93-
objFileInDepFile := firstRow[:len(firstRow)-1]
94-
if objFileInDepFile != objectFile.String() {
95-
logrus.Debugf("Depfile is about different file: %v", objFileInDepFile)
96-
return false, nil
97-
}
98-
99-
// The first line of the depfile contains the path to the object file to generate.
100-
// The second line of the depfile contains the path to the source file.
101-
// All subsequent lines contain the header files necessary to compile the object file.
102-
103-
// If we don't do this check it might happen that trying to compile a source file
104-
// that has the same name but a different path wouldn't recreate the object file.
105-
if sourceFile.String() != strings.Trim(rows[1], " ") {
106-
return false, nil
107-
}
91+
if len(rows) == 0 {
92+
return true, nil
93+
}
10894

109-
rows = rows[1:]
110-
for _, row := range rows {
111-
depStat, err := os.Stat(row)
112-
if err != nil && !os.IsNotExist(err) {
113-
// There is probably a parsing error of the dep file
114-
// Ignore the error and trigger a full rebuild anyway
115-
logrus.WithError(err).Debugf("Failed to read: %v", row)
95+
firstRow := rows[0]
96+
if !strings.HasSuffix(firstRow, ":") {
97+
logrus.Debugf("No colon in first line of depfile")
11698
return false, nil
11799
}
118-
if os.IsNotExist(err) {
119-
logrus.Debugf("Not found: %v", row)
100+
objFileInDepFile := firstRow[:len(firstRow)-1]
101+
if objFileInDepFile != objectFile.String() {
102+
logrus.Debugf("Depfile is about different object file: %v", objFileInDepFile)
120103
return false, nil
121104
}
122-
if depStat.ModTime().After(objectFileStat.ModTime()) {
123-
logrus.Debugf("%v newer than %v", row, objectFile)
105+
106+
// The first line of the depfile contains the path to the object file to generate.
107+
// The second line of the depfile contains the path to the source file.
108+
// All subsequent lines contain the header files necessary to compile the object file.
109+
110+
// If we don't do this check it might happen that trying to compile a source file
111+
// that has the same name but a different path wouldn't recreate the object file.
112+
if sourceFile.String() != strings.Trim(rows[1], " ") {
113+
logrus.Debugf("Depfile is about different source file: %v", strings.Trim(rows[1], " "))
124114
return false, nil
125115
}
116+
117+
rows = rows[1:]
118+
for _, row := range rows {
119+
depStat, err := os.Stat(row)
120+
if err != nil && !os.IsNotExist(err) {
121+
// There is probably a parsing error of the dep file
122+
// Ignore the error and trigger a full rebuild anyway
123+
logrus.WithError(err).Debugf("Failed to read: %v", row)
124+
return false, nil
125+
}
126+
if os.IsNotExist(err) {
127+
logrus.Debugf("Not found: %v", row)
128+
return false, nil
129+
}
130+
if depStat.ModTime().After(objectFileStat.ModTime()) {
131+
logrus.Debugf("%v newer than %v", row, objectFile)
132+
return false, nil
133+
}
134+
}
135+
136+
return true, nil
126137
}
127138

128-
return true, nil
139+
if runtime.GOOS == "windows" {
140+
// This is required because on Windows we don't know which encoding is used
141+
// by gcc to write the dep file (it could be UTF-8 or any of the Windows
142+
// ANSI mappings).
143+
if decoded, err := convertAnsiBytesToString(depFileData); err == nil {
144+
if upToDate, err := checkDepFile(decoded); err == nil && upToDate {
145+
return upToDate, nil
146+
}
147+
}
148+
// Fallback to UTF-8...
149+
}
150+
return checkDepFile(string(depFileData))
129151
}
130152

131153
func removeEndingBackSlash(s string) string {
+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// This file is part of arduino-cli.
2+
//
3+
// Copyright 2024 ARDUINO SA (http://www.arduino.cc/)
4+
//
5+
// This software is released under the GNU General Public License version 3,
6+
// which covers the main part of arduino-cli.
7+
// The terms of this license can be found at:
8+
// https://www.gnu.org/licenses/gpl-3.0.en.html
9+
//
10+
// You can be released from the requirements of the above licenses by purchasing
11+
// a commercial license. Buying such a license is mandatory if you want to
12+
// modify or otherwise use the software for commercial activities involving the
13+
// Arduino software without disclosing the source code of your own applications.
14+
// To purchase a commercial license, send an email to [email protected].
15+
16+
package compile
17+
18+
import (
19+
"testing"
20+
21+
"github.com/arduino/arduino-cli/internal/integrationtest"
22+
"github.com/arduino/go-paths-helper"
23+
"github.com/stretchr/testify/require"
24+
)
25+
26+
func TestBuildCacheLibWithNonASCIIChars(t *testing.T) {
27+
// See: https://github.com/arduino/arduino-cli/issues/2671
28+
29+
env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
30+
t.Cleanup(env.CleanUp)
31+
32+
tmpUserDir, err := paths.MkTempDir("", "Håkan")
33+
require.NoError(t, err)
34+
t.Cleanup(func() { tmpUserDir.RemoveAll() })
35+
customEnv := cli.GetDefaultEnv()
36+
customEnv["ARDUINO_DIRECTORIES_USER"] = tmpUserDir.String()
37+
38+
// Install Arduino AVR Boards and Servo lib
39+
_, _, err = cli.Run("core", "install", "arduino:[email protected]")
40+
require.NoError(t, err)
41+
_, _, err = cli.RunWithCustomEnv(customEnv, "lib", "install", "Servo")
42+
require.NoError(t, err)
43+
44+
// Make a temp sketch
45+
sketchDir := tmpUserDir.Join("ServoSketch")
46+
sketchFile := sketchDir.Join("ServoSketch.ino")
47+
require.NoError(t, sketchDir.Mkdir())
48+
require.NoError(t, sketchFile.WriteFile(
49+
[]byte("#include <Servo.h>\nvoid setup() {}\nvoid loop() {}\n"),
50+
))
51+
52+
// Compile sketch
53+
_, _, err = cli.RunWithCustomEnv(customEnv, "compile", "-b", "arduino:avr:uno", sketchFile.String())
54+
require.NoError(t, err)
55+
56+
// Compile sketch again
57+
out, _, err := cli.RunWithCustomEnv(customEnv, "compile", "-b", "arduino:avr:uno", "-v", sketchFile.String())
58+
require.NoError(t, err)
59+
require.Contains(t, string(out), "Compiling library \"Servo\"\nUsing previously compiled file")
60+
}

0 commit comments

Comments
 (0)