Dependency file parsing assumes one file per line

[matthijskooijman]

Apparently, the parsing of .d files assumes there is a single file on each line of the file. However, in some cases, presumably when filenames are short, there might be more than one in a single line. Consider this example from this report.

C:\Users\Graham\AppData\Local\Temp\builde33ce9ddee6346054afe0349d71c85f0.tmp\libraries\UTFT\UTFT.cpp.o: \
 F:\Arduino\libraries\UTFT\UTFT.cpp F:\Arduino\libraries\UTFT\UTFT.h \
 C:\Users\Graham\AppData\Local\Arduino15\packages\arduino\hardware\sam\1.6.7\cores\arduino/Arduino.h \
 (rest of the file removed)

The relevant source is here: https://github.com/arduino/arduino-builder/blob/master/src/arduino.cc/builder/builder_utils/utils.go#L222-L233

I guess the solution here is to do more proper parsing of the file, such as removing any escaped newlines, and then splitting the result into filenames (taking into account escaped spaces). I'm not entirely sure how reliable this can be done, since GNU make is known to be bad in handling spaces in filenames (though I think it works ok as long as no variables are involved).

[ghlawrence2000]

Ok, thanks Matthijs.

[ghlawrence2000]

Time passes by........... and nothing happens.........?

[PaulStoffregen]

This worked properly when I implemented in Java.

But to be fair, I didn't handle spaces in pathnames properly until much later. So many little details to get right...

[ghlawrence2000]

Hi Paul, Is that something I can do?

[cmaglie]

IMHO the difference between the original java version and this one is in the loop here:

    for _, row := range rows {
        depStat, err := os.Stat(row)
        if err != nil && !os.IsNotExist(err) {
            return false, i18n.WrapError(err)
        }
        if os.IsNotExist(err) {
            return false, nil
        }
        if depStat.ModTime().After(objectFileStat.ModTime()) {
            return false, nil
        }
    }

IIRC the java version returned false if the file is not found, this one instead throws an error. Probably the java version will always trigger a full rebuild of the sketch in this case, but probably this is better and more conservative than throwing an error, I'll make a PR for this in a moment.

The long term solution is to properly parse the .d file, that may have more than one file per line, this could be tricky because the file are separated by space " " but a file may contains spaces escaped by backslash "\ ", so we could not simply split the line by spaces.

[cmaglie]

Well, to be precise, there is already the condition if os.IsNotExist(err) { that should handle the "file not found" error, the problem is that in this case the error is different we have a:

GetFileAttributesEx F:\Arduino\libraries\UTFT\UTFT.cpp F:\Arduino\libraries\UTFT\UTFT.h: The filename, directory name, or volume label syntax is incorrect.

so in some way the parsed filename F:\Arduino\libraries\UTFT\UTFT.cpp F:\Arduino\libraries\UTFT\UTFT.h is able to trigger this subtle error instead of "file not found".

[matthijskooijman]

Wouldn't it be better to keep this issue open until it is really properly fixed? Also, I think that a proper fix might share some code with a fix for this comment: #131 (comment)

[ghlawrence2000]

Hi mathhijs,

The supplied arduino-builder.exe cures the problem, but as suspected, rebuilds all each time. Are you talking about a fix that doesn't rebuild if there is a problem? That would be better.

[cmaglie]

Wouldn't it be better to keep this issue open until it is really properly fixed

yes, github auto-closed the issue becuase of my comment on the PR.