Improve library name matching

[PaulStoffregen]

I believe this will make many users much happier when a header happens to match to 2 or more libraries.

[PaulStoffregen]

I believe this solves the IRremote conflicts when a user added the non-Robot IRremote library.

#2200

[ffissore]

@ArduinoBot build this please

[ArduinoBot]

Build successful. Please test this code using one of the following:
http://arduino.cc/download.php?f=/javaide/pull_requests/arduino-PR-2681-BUILD-191-linux32.tar.xz
http://arduino.cc/download.php?f=/javaide/pull_requests/arduino-PR-2681-BUILD-191-linux64.tar.xz
http://arduino.cc/download.php?f=/javaide/pull_requests/arduino-PR-2681-BUILD-191-windows.zip
http://arduino.cc/download.php?f=/javaide/pull_requests/arduino-PR-2681-BUILD-191-macosx.zip
http://arduino.cc/download.php?f=/javaide/pull_requests/arduino-PR-2681-BUILD-191-macosx-java-latest.zip

[PaulStoffregen]

@joshuajnoble - any chance you could give this build a quick look, to see how well it deals with the IRremote library problems?

[joshuajnoble]

Tested on OSX and Windows 8 and afaict, fixes them.

[pfeerick]

Tested on Windows 7 x64 SP1, works well It also fixed the below issue related to PR #2200. The below error message no longer came up with this PR build.

Arduino: 1.6.0 (Windows 7), Board: "Arduino Uno"

D:\Open Source Programming\arduino-1.6.0\libraries\RobotIRremote\src\IRremoteTools.cpp:5:16: error: 'TKD2' was not declared in this scope
int RECV_PIN = TKD2; // the pin the IR receiver is connected to
^
Error compiling.

[shirriff]

As the author of the IRremote library, it makes me unhappy that the RobotIRremote included in Arduino blocks people from using IRremote, especially since RobotIRremote is a fork of IRremote. So I would be delighted to see this pull request merged in.

[ericwertz]

arduino-PR-2681-BUILD-191 fixed this issue for me. My own code based on IRsendDemo exhibited the same problem, which was broken in 1.6.0 (but was fine in 1.0.6).

That a header file is getting pulled into my project by the builder from a library that I didn't reference is (or, has always been) whack. Headers that happen to have the same name associated with libraries that I didn't import are going to collide more and more often as times goes on. Pruning include directories that aren't intended to be searched is more than a sensible way to mitigate this.

[PaulStoffregen]

In case there's any misunderstanding, I'd like clearly explain this pull request does not prune or remove anything. Doing so may cause compatibility issues. In the long run, such pruning might be a good design, but this patch doesn't do any such thing. It's designed to preserve backwards compatibility with all libraries that depend on imprecise name matching, while also improving compatibility with libraries that do match closely (in the presence of those imprecise ones).

This change only affects which library is used when 2 or more have the same .h file. It has absolutely no effect when a header file uniquely matches to only 1 library.

The current approach is somewhat short-sighted. That's mostly my fault, since I suggested the current approach several months ago. At the time, it was an improvement over no attempt at all to figure out which library should be used. In hindsight, looking only at the end of the library's full pathname just isn't good enough.

This patch applies 5 matching criteria. When criteria match equally, the library in the more specific location is used (eg, the sketchbook overrides the IDE's built-in libraries folder). But the 5 criteria allow for properly matching header files to the correct library in common cases.

1. • When the header file exactly matches the library's folder name, that library is used. This is the most important case. When users install a 3rd party library named "IRremote", they expect using #include "IRremote.h" in their sketch will cause the identically named library to be used. Arduino ships RobotIRremote containing IRremote.h. By default, #include "IRremote.h" will match to RobotIRremote, because today the IDE only implements rule 4 (and doesn't properly apply the always-use-more-specific location policy when the rule matches both).
2. • When the header matches exactly with "-master" appended, the assumption is a user has extracted a zip file from github with "-master" appended to the name. Of course, an exact match is rule 1, but this common case is given priority over 3-5, because many 3rd party Arduino libraries are now distributed by github. Users follow the directions Arduino provides to unzip the file, which results in "-master" appended.
3. • Otherwise, anything else appended, with an exact match to the beginning of the library folder name is used. The assumption is version numbers are often added when people modify libraries. This allows a folder named "IRremote3" to be used, if the above rules don't match any library folder name.
4. • Next, a library folder which end in the header file name is used. This allows "RobotIRremote" to still match in preference to some other folder name that doesn't end in "IRremote". It will preserve the Arduino Robot functionality for the case where a user doen't install the IRremote library from github.
5. • Lastly, if none of the above match, a folder containing the header name with other characters both before and after, is still overrides folder names that don't seem to correspond at all. This would allow "RobotIRremote_rev2" to still match before some randomly named library which happens to contain the header "IRremote.h".

When none of these rules match, for example a library named "Foo" which has "bar.h", using #include "bar.h" in a sketch will still properly compile the Foo library. These 5 rules merely allow some library named "bar" or "bar-master" or "bar2" to be installed by the user and properly override the less specific match.

[ericwertz]

Ouch, thanks for all of the grody details on how this "works" (where "works" is some value of x on the curve y=kludge(x^2)).

The Arduino Language -- all of the familiar syntax of C(++) with none of the annoying, predictable, designed-for-best-practices semantics! Get yours today!

As always, thanks for all of your hard work trying to get this Arduino stuff to do whatever exactly it's trying to do :-).

[cmaglie]

Thanks!

[PaulStoffregen]

Thanks Cristian!

If you have a moment, I'd really like your opinion on suggestion that was made to me yesterday by a library author....

He suggested adding another criteria, which would override all 5 of these rules...

[cmaglie]

Sure... now I'm courious :-)

[PaulStoffregen]

The idea he proposed involved first checking if the .h file matches any library that's already been added to the list for building the sketch. If it has, don't match to anything more. Just ignore it.

Some library authors like to have more than 1 header file. If the user includes more than 1 in their sketch, things can go very badly if Arduino mistaken tries to match the 2 .h files to 2 different libraries.

Of course, this has some possible downsides too. If some library library has lots of extra .h files, its conflicts prevent libraries the user wants to use from being compiled. I'm honestly not sure which way is best.

As things are now, using more than one #include per library in a sketch risks all sorts of trouble.

[PaulStoffregen]

I also have one more little patch here, which is low risk. It causes the "Using..." message to print in non-verbose mode when 2 or more libraries match to a .h file used in sketch.

Would you like to merge that?

[cmaglie]

The idea he proposed involved first checking if the .h file matches any library that's already been added to the list for building the sketch. If it has, don't match to anything more. Just ignore it.

mmmh interesting... In the robot's IRRemote case this check will make the IDE to choose the robot's IRRemote if some robot's header file is included before IRRemote.h and that looks like a good thing.
(As a side note, I'm seriously taking in consideration to remove or rename IRemote.h from robot even if this won't solve the issue in general).

There are other known cases that this check will fix?

I also have one more little patch here, which is low risk. It causes the "Using..." message to print in non-verbose mode when 2 or more libraries match to a .h file used in sketch.

Would you like to merge that?

Yes, send that I'll take a look.

[PaulStoffregen]

I'll prepare a pull request for the non-verbose message.

Regarding the other cases, I do not believe there's any perfect solution. If things stay as they are, perhaps the library spec and Arduino style guide should be updated to recommend library authors design their libraries to use only a .h header included from sketches?

Renaming IRremote.h to RobotIRremote.h would require all robot sketches to be updated. If you can live with that pain, it'd probably be a worthwhile change. But if not, robot stuff should be still work fine. Now installing IRremote will cause the Robot's version to be overridden, which seems like something users could reasonably expect to happen for the robot's sketches with #include "IRremote.h".

[ericwertz]

As I mentioned in the side-channel to this discussion, a better solution would be to prune out all of the search directories when compiling (the -I 's), and the Robot directory won't ever get searched, eliminating the conflict. This would both have performance benefits as well as being able to scale. At some point the length of the command line to gcc is going to choke if every single IDE library directory is searched.

Given that the creators of the Robot library should have known that there'd be a header file name conflict (after all, they did clone theirs from this very library, from the sounds of it), the easiest short-term solution would be to rename Robot's files (not to mention that it doesn't follow the general convention of having the header's name match that of the directory in which it lives, so that's Strike #3). This is especially true if their IRremote.h isn't externally visible to Robot users -- which I don't know to be true in this case or not, admittedly.

Given the numbers, there's a lot more IRremote users than Robot users out there, so if one has to break something (because a kludge-solution isn't palatable), it's fairly clear to me which one makes sense to break, politics aside.

Better to fix things like this by breaking them now rather than later. A clean fix is also always better than putting another wart on the frog.