Locale issue with includes finder

[kevans91]

Hello!

arduino-builder is failing to find includes and pull in used libraries with some at least some locales. This script demonstrates the problem for me:

#!/bin/sh

LANG=de_DE.UTF-8

arduino-builder -verbose -fqbn arduino:avr:uno -build-options-file /usr/local/arduino/arduino-builder.options BareMinimum/BareMinimum.ino

I've copied /usr/local/arduino/examples/01.Basics/BareMinimum to my working directory for this purpose and added #include <SPI.h> to the top of it, a la:

#include <SPI.h>


void setup() {
  // put your setup code here, to run once:

}

void loop() {
  // put your main code here, to run repeatedly:

}

Executing my script then yields the following output:

Using board 'uno' from platform in folder: /usr/local/arduino/hardware/arduino/avr
Using core 'arduino' from platform in folder: /usr/local/arduino/hardware/arduino/avr
Detecting libraries used...
"/usr/local/arduino/tools-builder/avr-gcc/4.9.2-atmel3.5.4-arduino2/bin/avr-g++" -c -g -Os -w -std=gnu++11 -fpermissive -fno-exceptions -ffunction-sections -fdata-sections -fno-threadsafe-statics  -flto -w -x c++ -E -CC -mmcu=atmega328p -DF_CPU=16000000L -DARDUINO=10612 -DARDUINO_AVR_UNO -DARDUINO_ARCH_AVR   "-I/usr/local/arduino/hardware/arduino/avr/cores/arduino" "-I/usr/local/arduino/hardware/arduino/avr/variants/standard" "/tmp/arduino-sketch-0331F5BB0B95CD649BA7F16BAFF81EE3/sketch/BareMinimum.ino.cpp" -o "/dev/null"
Generating function prototypes...
"/usr/local/arduino/tools-builder/avr-gcc/4.9.2-atmel3.5.4-arduino2/bin/avr-g++" -c -g -Os -w -std=gnu++11 -fpermissive -fno-exceptions -ffunction-sections -fdata-sections -fno-threadsafe-statics  -flto -w -x c++ -E -CC -mmcu=atmega328p -DF_CPU=16000000L -DARDUINO=10612 -DARDUINO_AVR_UNO -DARDUINO_ARCH_AVR   "-I/usr/local/arduino/hardware/arduino/avr/cores/arduino" "-I/usr/local/arduino/hardware/arduino/avr/variants/standard" "/tmp/arduino-sketch-0331F5BB0B95CD649BA7F16BAFF81EE3/sketch/BareMinimum.ino.cpp" -o "/tmp/arduino-sketch-0331F5BB0B95CD649BA7F16BAFF81EE3/preproc/ctags_target_for_gcc_minus_e.cpp"
/home/kevans91/sketchbook/BareMinimum/BareMinimum.ino:1:17: schwerwiegender Fehler: SPI.h: No such file or directory
Kompilierung beendet.
exit status 1

Setting my locale to en_US.UTF-8 makes it all happy again. I assumed it was falling back to findIncludeForOldCompilers, but I've no idea why it would -- I extracted INCLUDE_REGEXP and the matching bits to its own golang application and it matched just fine in isolation.

I've only tested this on FreeBSD systems, but all of the relevant tests pass here so I assume it's reproducible elsewhere, too.

[matthijskooijman]

IIRC findIncludeForOldCompilers matches the error message, 'fatal error' or something like that? If that part of the message is translated by gcc, this matching would break. I wonder if we should run the compiler with LANG=C in this context perhaps?

[kevans91]

Yeah, around here: https://github.com/arduino/arduino-builder/blob/master/src/arduino.cc/builder/includes_finder_with_regexp.go#L47-L52

But it seems like this should "Just Work" because it tries to match the regex first. I tried the following test to make sure I'm not crazy:

main.go:

package main

import "fmt"
import "regexp"

func main() {
        var INCLUDE_REGEXP = regexp.MustCompile("(?ms)^\\s*#[ \t]*include\\s*[<\"](\\S+)[\">]")
        match := INCLUDE_REGEXP.FindStringSubmatch(`#include <SPI.h>

void setup() {
  // put your setup code here, to run once:

}

void loop() {
  // put your main code here, to run repeatedly:

}
`)
        fmt.Println(match)
}

kevans91@gemini:~/test$ go build main.go && ./main
[#include <SPI.h> SPI.h]
kevans91@gemini:~/test$ env LANG=de_DE.UTF-8 ./main
[#include <SPI.h> SPI.h]

So why would this result not be used if the regex should clearly work? =( I would think the locale should really be a non-issue if we can successfully parse it out of the source.

Of course, LANG=C should be a workable solution, too. =)

[matthijskooijman]

I was actually thinking about the findIncludeForOldCompilers:

		if strings.Contains(splittedLine[i], "fatal error") {

(see https://github.com/arduino/arduino-builder/blob/master/src/arduino.cc/builder/includes_finder_with_regexp.go#L62)

But your debug output suggests that the regular regex already matches, so the old compiler fallback is not needed. Perhaps you can add some more debug output to see what is failing exactly?

I tried to reproduce this by setting LANG=nl_NL.UTF-8 (I don't have the de locale installed) and then running arduino, but then it does not seem to fail for me.

[facchinm]

Surrounding findIncludes calls between

lang := os.Getenv("LANG")
os.Setenv("LANG", "C") 

and

os.Setenv("LANG", lang)

looks good to me. This would avoid any problem during that phase while allowing translated gcc output for whoever likes it 🙂

[matthijskooijman]

What you propose will probably work, but it might be more elegant to not touch the current environment, but only change the environment for the command being run. I think this can be done using Cmd.Env: https://golang.org/pkg/os/exec/#Cmd

Since that does not seem to override single values from the env, but replaces all of the env, this would involve fetching the current environment, changing LANG and setting that into Cmd.Env. Since The Cmd instance is buried inside the builder_utils and utils, that would also involve some extra value-passing to get the data in the right place.

Applying that change probably touches on the same cod as #236, so it might be useful to merge that one first.