Skip to content

Commit

Permalink
Fixed compiler-cache on Windows when there are non-ASCII characters i…
Browse files Browse the repository at this point in the history
…n file path (#2733)

* Increased logging during compile

* Added integration test

* Fixed dependency file parsing on Windows

* Fixed panic in convertAnsiBytesToString implementation
  • Loading branch information
cmaglie authored Oct 16, 2024
1 parent 812e621 commit a527c7c
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 45 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ require (
go.bug.st/f v0.4.0
go.bug.st/relaxed-semver v0.12.0
go.bug.st/testifyjson v1.2.0
golang.org/x/sys v0.26.0
golang.org/x/term v0.25.0
golang.org/x/text v0.19.0
google.golang.org/genproto/googleapis/rpc v0.0.0-20240814211410-ddb44dafa142
Expand Down Expand Up @@ -100,7 +101,6 @@ require (
golang.org/x/mod v0.18.0 // indirect
golang.org/x/net v0.28.0 // indirect
golang.org/x/sync v0.8.0 // indirect
golang.org/x/sys v0.26.0 // indirect
golang.org/x/tools v0.22.0 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/warnings.v0 v0.1.2 // indirect
Expand Down
27 changes: 27 additions & 0 deletions internal/arduino/builder/internal/utils/ansi_others.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// This file is part of arduino-cli.
//
// Copyright 2024 ARDUINO SA (http://www.arduino.cc/)
//
// This software is released under the GNU General Public License version 3,
// which covers the main part of arduino-cli.
// The terms of this license can be found at:
// https://www.gnu.org/licenses/gpl-3.0.en.html
//
// You can be released from the requirements of the above licenses by purchasing
// a commercial license. Buying such a license is mandatory if you want to
// modify or otherwise use the software for commercial activities involving the
// Arduino software without disclosing the source code of your own applications.
// To purchase a commercial license, send an email to [email protected].

//go:build !windows

package utils

import (
"errors"
)

// placeholder for non-Windows machines
func convertAnsiBytesToString([]byte) (string, error) {
return "", errors.New("unimplemented")
}
36 changes: 36 additions & 0 deletions internal/arduino/builder/internal/utils/ansi_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// This file is part of arduino-cli.
//
// Copyright 2024 ARDUINO SA (http://www.arduino.cc/)
//
// This software is released under the GNU General Public License version 3,
// which covers the main part of arduino-cli.
// The terms of this license can be found at:
// https://www.gnu.org/licenses/gpl-3.0.en.html
//
// You can be released from the requirements of the above licenses by purchasing
// a commercial license. Buying such a license is mandatory if you want to
// modify or otherwise use the software for commercial activities involving the
// Arduino software without disclosing the source code of your own applications.
// To purchase a commercial license, send an email to [email protected].

package utils

import (
"golang.org/x/sys/windows"
)

func convertAnsiBytesToString(data []byte) (string, error) {
if len(data) == 0 {
return "", nil
}
dataSize := int32(len(data))
size, err := windows.MultiByteToWideChar(windows.GetACP(), 0, &data[0], dataSize, nil, 0)
if err != nil {
return "", err
}
utf16 := make([]uint16, size)
if _, err := windows.MultiByteToWideChar(windows.GetACP(), 0, &data[0], dataSize, &utf16[0], size); err != nil {
return "", err
}
return windows.UTF16ToString(utf16), nil
}
110 changes: 66 additions & 44 deletions internal/arduino/builder/internal/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package utils

import (
"os"
"runtime"
"strings"
"unicode"

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

sourceFile = sourceFile.Clean()
sourceFileStat, err := sourceFile.Stat()
if err != nil {
logrus.Debugf("Could not stat source file: %s", err)
return false, err
}

objectFile = objectFile.Clean()
objectFileStat, err := objectFile.Stat()
if err != nil {
if os.IsNotExist(err) {
logrus.Debugf("Not found: %v", objectFile)
logrus.Debugf("Object file not found: %v", objectFile)
return false, nil
}
logrus.Debugf("Could not stat object file: %s", err)
return false, err
}

dependencyFile = dependencyFile.Clean()
dependencyFileStat, err := dependencyFile.Stat()
if err != nil {
if os.IsNotExist(err) {
logrus.Debugf("Not found: %v", dependencyFile)
logrus.Debugf("Dependency file not found: %v", dependencyFile)
return false, nil
}
logrus.Debugf("Could not stat dependency file: %s", err)
return false, err
}

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

rows, err := dependencyFile.ReadFileAsLines()
depFileData, err := dependencyFile.ReadFile()
if err != nil {
logrus.Debugf("Could not read dependency file: %s", dependencyFile)
return false, err
}

rows = f.Map(rows, removeEndingBackSlash)
rows = f.Map(rows, strings.TrimSpace)
rows = f.Map(rows, unescapeDep)
rows = f.Filter(rows, f.NotEquals(""))
checkDepFile := func(depFile string) (bool, error) {
rows := strings.Split(strings.Replace(depFile, "\r\n", "\n", -1), "\n")
rows = f.Map(rows, removeEndingBackSlash)
rows = f.Map(rows, strings.TrimSpace)
rows = f.Map(rows, unescapeDep)
rows = f.Filter(rows, f.NotEquals(""))

if len(rows) == 0 {
return true, nil
}

firstRow := rows[0]
if !strings.HasSuffix(firstRow, ":") {
logrus.Debugf("No colon in first line of depfile")
return false, nil
}
objFileInDepFile := firstRow[:len(firstRow)-1]
if objFileInDepFile != objectFile.String() {
logrus.Debugf("Depfile is about different file: %v", objFileInDepFile)
return false, nil
}

// The first line of the depfile contains the path to the object file to generate.
// The second line of the depfile contains the path to the source file.
// All subsequent lines contain the header files necessary to compile the object file.

// If we don't do this check it might happen that trying to compile a source file
// that has the same name but a different path wouldn't recreate the object file.
if sourceFile.String() != strings.Trim(rows[1], " ") {
return false, nil
}
if len(rows) == 0 {
return true, nil
}

rows = rows[1:]
for _, row := range rows {
depStat, err := os.Stat(row)
if err != nil && !os.IsNotExist(err) {
// There is probably a parsing error of the dep file
// Ignore the error and trigger a full rebuild anyway
logrus.WithError(err).Debugf("Failed to read: %v", row)
firstRow := rows[0]
if !strings.HasSuffix(firstRow, ":") {
logrus.Debugf("No colon in first line of depfile")
return false, nil
}
if os.IsNotExist(err) {
logrus.Debugf("Not found: %v", row)
objFileInDepFile := firstRow[:len(firstRow)-1]
if objFileInDepFile != objectFile.String() {
logrus.Debugf("Depfile is about different object file: %v", objFileInDepFile)
return false, nil
}
if depStat.ModTime().After(objectFileStat.ModTime()) {
logrus.Debugf("%v newer than %v", row, objectFile)

// The first line of the depfile contains the path to the object file to generate.
// The second line of the depfile contains the path to the source file.
// All subsequent lines contain the header files necessary to compile the object file.

// If we don't do this check it might happen that trying to compile a source file
// that has the same name but a different path wouldn't recreate the object file.
if sourceFile.String() != strings.Trim(rows[1], " ") {
logrus.Debugf("Depfile is about different source file: %v", strings.Trim(rows[1], " "))
return false, nil
}

rows = rows[1:]
for _, row := range rows {
depStat, err := os.Stat(row)
if err != nil && !os.IsNotExist(err) {
// There is probably a parsing error of the dep file
// Ignore the error and trigger a full rebuild anyway
logrus.WithError(err).Debugf("Failed to read: %v", row)
return false, nil
}
if os.IsNotExist(err) {
logrus.Debugf("Not found: %v", row)
return false, nil
}
if depStat.ModTime().After(objectFileStat.ModTime()) {
logrus.Debugf("%v newer than %v", row, objectFile)
return false, nil
}
}

return true, nil
}

return true, nil
if runtime.GOOS == "windows" {
// This is required because on Windows we don't know which encoding is used
// by gcc to write the dep file (it could be UTF-8 or any of the Windows
// ANSI mappings).
if decoded, err := convertAnsiBytesToString(depFileData); err == nil {
if upToDate, err := checkDepFile(decoded); err == nil && upToDate {
return upToDate, nil
}
}
// Fallback to UTF-8...
}
return checkDepFile(string(depFileData))
}

func removeEndingBackSlash(s string) string {
Expand Down
60 changes: 60 additions & 0 deletions internal/integrationtest/compile_4/lib_caching_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// This file is part of arduino-cli.
//
// Copyright 2024 ARDUINO SA (http://www.arduino.cc/)
//
// This software is released under the GNU General Public License version 3,
// which covers the main part of arduino-cli.
// The terms of this license can be found at:
// https://www.gnu.org/licenses/gpl-3.0.en.html
//
// You can be released from the requirements of the above licenses by purchasing
// a commercial license. Buying such a license is mandatory if you want to
// modify or otherwise use the software for commercial activities involving the
// Arduino software without disclosing the source code of your own applications.
// To purchase a commercial license, send an email to [email protected].

package compile

import (
"testing"

"github.com/arduino/arduino-cli/internal/integrationtest"
"github.com/arduino/go-paths-helper"
"github.com/stretchr/testify/require"
)

func TestBuildCacheLibWithNonASCIIChars(t *testing.T) {
// See: https://github.com/arduino/arduino-cli/issues/2671

env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
t.Cleanup(env.CleanUp)

tmpUserDir, err := paths.MkTempDir("", "Håkan")
require.NoError(t, err)
t.Cleanup(func() { tmpUserDir.RemoveAll() })
customEnv := cli.GetDefaultEnv()
customEnv["ARDUINO_DIRECTORIES_USER"] = tmpUserDir.String()

// Install Arduino AVR Boards and Servo lib
_, _, err = cli.Run("core", "install", "arduino:[email protected]")
require.NoError(t, err)
_, _, err = cli.RunWithCustomEnv(customEnv, "lib", "install", "Servo")
require.NoError(t, err)

// Make a temp sketch
sketchDir := tmpUserDir.Join("ServoSketch")
sketchFile := sketchDir.Join("ServoSketch.ino")
require.NoError(t, sketchDir.Mkdir())
require.NoError(t, sketchFile.WriteFile(
[]byte("#include <Servo.h>\nvoid setup() {}\nvoid loop() {}\n"),
))

// Compile sketch
_, _, err = cli.RunWithCustomEnv(customEnv, "compile", "-b", "arduino:avr:uno", sketchFile.String())
require.NoError(t, err)

// Compile sketch again
out, _, err := cli.RunWithCustomEnv(customEnv, "compile", "-b", "arduino:avr:uno", "-v", sketchFile.String())
require.NoError(t, err)
require.Contains(t, string(out), "Compiling library \"Servo\"\nUsing previously compiled file")
}

0 comments on commit a527c7c

Please sign in to comment.