Documentation: Unclear when a library is compiled as "new-style" or "legacy"

[matthijskooijman]

Bug Report

The library specification documents two flavours of libraries: The modern one with source files in the src/ directory, and the legacy one with source files in the root and utility directory. However, it is not immediately clear how the Arduino toolchain decides whether a library is one or the other.

I thought that it depended on the presence of a library.properties file, which is also implied by the last section (but not 100% clear): https://arduino.github.io/arduino-cli/latest/library-specification/#old-library-format-pre-15

However, the section about source code layout is where this distinction is mainly made, but that has nothing explicit about when one or the other is chosen (but it seems to imply that the presence of a src directory might be the trigger?

It would be good to clarify this behavior, maybe make it more explicit in the last section and refer to that from the "source" section? Maybe it would also be useful to clarify what "legacy" mode means exactly, i.e. make explicit that for all library.properties fields, whether the default value is used (i.e. like when the field is omitted in a new-style library), or whether there is some other default (e.g. like the name field which is probably deduced from the folder name).

[ianfixes]

Maybe it would also be useful to clarify what "legacy" mode means exactly

I ran into this. I thought "legacy" meant "1.0 library spec" and non-legacy meant "1.5 library spec" but that assumption broke my tests. I have absolutely no idea what this field implies or what I would do with it.

[per1234]

@ianfixes I thought I explained it pretty thoroughly in our discussion at Arduino-CI/arduino_ci#168 (comment)

Please give that another read and then let us know what is still not clear so we might improve on it when the documentation is improved in regards to this subject.

[ianfixes]

I thought your explanation was excellent! But when I implemented the one_point_five? check based on the CLI output (instead of directly on your flowchart logic), my tests broke. Here's a failing pull request to illustrate: Arduino-CI/arduino_ci#229

The test case that broke it is this library: https://github.com/Arduino-CI/arduino_ci/tree/master/SampleProjects/DoSomething

Based on your flowchart, this should be "flat" layout, corresponding to the 1.0 spec -- I would expect is_legacy to be true in this case. But I could read your flowchart another way, which is that is_legacy refers only to the absence of a library.properties file. But that's very strange because it implies that is_legacy would be unset/false for a flat/1.0 layout.

[per1234]

Thanks for your feedback! I hope we will be able to improve the documentation to make this confusing subject a bit more clear.

Even though I had an instinctive understanding of the differences through years of working with all possible combinations of these formats, when the discussion arose in your arduino_ci repository I found that I was having difficulty formulating a complete explanation of the handling of each in a readily comprehensible form. I used that discussion as an excuse to get the subject more clear in my mind, which culminated in the excessively elaborate comment I made in that thread. My hope was that it could serve as a reference when the opportunity arose to resolve this issue

it implies that is_legacy would be unset/false for a flat/1.0 layout.

Correct. If the flat layout library contains a library.properties, as is the case with your "DoSomething" test case library, then it's not legacy.

This is a legacy library:

FooLib
|_ FooLib.h

All legacy libraries have flat layout because the recursive layout is not supported for these libraries. Even if the library author puts source files under the src folder in an attempt to achieve the recursive layout, they will not be compiled.

This is a non-legacy flat layout library:

FooLib
|_ FooLib.h
|_ library.properties

This is a non-legacy recursive layout library:

FooLib
|_ library.properties
|_ src
    |_ FooLib.h

[ianfixes]

Aah, OK -- these are 2 separate conditions that I'm improperly attempting to combine: the existence of library.properties and whether the layout should be considered flat or recursive.

I assume that is_legacy is equivalent to a certain set of values in library.properties. Or rather, that certain properties have default values. Is that something that affects the set of files that would be included?

For the rest, I think I can put it better in its own issue. Opened #1092

[per1234]

Or rather, that certain properties have default values.

Exactly. You can see the defaults for legacy libraries here:

arduino-cli/arduino/libraries/loader.go

Lines 127 to 137 in df8c924

	func makeLegacyLibrary(path *paths.Path, location LibraryLocation) (*Library, error) {
	library := &Library{
	InstallDir: path,
	Location: location,
	SourceDir: path,
	Layout: FlatLayout,
	Name: path.Base(),
	Architectures: []string{"*"},
	IsLegacy: true,
	Version: semver.MustParse(""),
	}

Is that something that affects the set of files that would be included?

The name and architectures properties values are factors in detemining priority when multiple libraries match an #include directive:
https://arduino.github.io/arduino-cli/dev/sketch-build-process/#dependency-resolution
So I suppose you could say the default metadata values assigned to legacy libraries could affect which files are included under specific conditions. Other than that, I think there is no difference between a flat layout library that is legacy and a flat layout library that is non-legacy.

[ianfixes]

I think the only feasible strategy here is for me to defer all source-code-gathering to the logic you've already implemented, via #1092

I could certainly re-implement it, but it seems like it would be awkward to try and keep a parallel implementation in sync with the CLI instead of just asking the CLI what it would do.

[ianfixes]

Can you confirm the behavior of the "flat" structure in regards to the utility folder mentioned in the spec?