Does this mean it's now possible for macros in the sketch to interfere with the Arduino core files, like changing the hardware serial buffer size by defining SERIAL_RX_BUFFER_SIZE in the sketch?

You are right, I was way to quick here :)
Thanks for fixing my mistake!

Corjan

Does this mean it's now possible for macros in the sketch to interfere with the Arduino core files, like changing the hardware serial buffer size by defining SERIAL_RX_BUFFER_SIZE in the sketch?

No, this is just about link order, which happens a long time after the preprocessor is ran.

I'm a bit confused by this change. Now, it seems this adds the compiler.libraries.ldflags flag to the linker commandline, rather than reordering things?

@Corjan85, if you previous version could not have worked, I'm wondering: Did you test this change, or is this just hypothetical?

I was still using a custom build of arduino-build tools.
(from https://github.com/arduino/arduino-builder/pull/219 )

That one didn't have the 'compiler.libraries.ldflags' flag. But it seems that the library objects where put into another flag instead. By shuffling around I could get the desired result.
But that is not a solid solution, as stated by facchinm.

The newest version or the arduino-builder.exe has the 'compiler.libraries.ldflags' flag, so that is used instead (and works fine).

Sorry for the confusion. I'm still new into the whole Arduino building system etc,
Corjan

If I understand correctly, this is related to arduino/Arduino#7005

It seems this was merged, but then a force-push happened to the master branch (bad idea IMHO, btw) that did not contain this commit any more, so I think this is still not properly merged?

Also, this question was never answered, so I'm not quite convinced that this should be merged as-is (it might work, but then need a better explanation of why exactly):

I'm a bit confused by this change. Now, it seems this adds the compiler.libraries.ldflags flag to the linker commandline, rather than reordering things?

Sorry for the mixup 😳 I noticed belatedly that merging this PR outright broke the build process and instead of properly reverting it took to git rebase. Everything has been restored back, however this PR needs to be fixed. The error message I get is avr-gcc: error: {compiler.libraries.ldflags}: No such file or directory.

@Corjan85 you might want to add

+compiler.libraries.ldflags=

within https://github.com/arduino/ArduinoCore-avr/blob/master/platform.txt to avoid that field to be empty when not including a pre-compiled library.

@lxrobotics, thanks for clarifying. Rather than adding an empty property to platform.txt, would it not make sense for the arduino-builder to always set that property (just empty when no pre-compiled libraries are added)? That keeps the responsibility for setting that property in one place.

Also, I'm still confused: If precompiled libraries are added to that variable, and that variable is not currently added to the linker commandline, do precompiled libraries currently work at all with the AVR core?

@matthijskooijman they don't work right now on avr; the main target was Atmel's qtouch but there was not enough traction to integrate the change.

Right, so then the subject of this PR is totally wrong and it is more about "Support precompiled libraries", rather than "Allow precompiled libraries to use Arduino libraries". If another PR is submitted for this, that should probably be documented like this.

Ok, I'm also lost now :)

Should or shouldn't precompiled libraries work in the Arduino IDE?

The latest Arduino IDE 1.8.10 seems to do nothing with the 'ldflags' flag defined in the library.properties. I'm pretty sure, although not 100%, that previous builds would add the 'ldflags' into the build, although in a wrong order (that is what this issue is about).

The documentation seems to indicate it should support precompiled libraries:
https://github.com/arduino/Arduino/wiki/Arduino-IDE-1.5:-Library-specification#libraryproperties-file-format

@Corjan85, AFAIU this PR was reverted and didn't actually work yet. AFAIU, the previous builds did not actually support it yet, and the title of this PR was very confusing (it would not fix the order, it would add the ldflags). I do remember that there was some documentation about it and it was supported at some point, but maybe that was just the IDE part and the AVR core never included the core part of this change (maybe only another core, @facchinm mentioned "Atmel qtouch", whatever that is).

Ah okay.

So this means that precompiled libraries are not supported, and the documentation is outdated?

AFAIU, not supported on the AVR-core. Maybe the documentation should state that the core must have supported added in addition to the IDE.

Hi,

Thanks for your responses. Is there any way that I can help, or speed up this development?

We are a commercial company. And although we would like Arduino support, it is not in our interest to share the source code.

Corjan

Hm, I only just now saw you started this PR originally, I thought you were just someone else asking. In any case, to move this forward, I think providing a new proper PR would be helpful, looking at the comments above for things to improve? It's not up to me to merge it though, so I can't make any promises.

@Corjan85 I've merged this PR during a catch-up effort relying on the word of others that this change was good. Unfortunately it was not okay and broke the build process which is why I undid this change.

As I've found out later in order to not break the build process you shoud add compiler.libraries.ldflags= (that's right, no arguments) somewhere within platform.txt. If you are still interested in having this change supported by the Arduino core please create a new PR containing the additional change I've just described here.

This change seems to break all precompiled libraries for the ESP32 platform.
For example, this is our precompiled library - https://github.com/cesanta/mdash
It stopped working on all new Arduino releases - presumably from 1.8.12, onwards.
What should we, library authors, must do in order to fix that and keep our customers happy?
Note that addling an empty line compiler.libraries.ldflags= to the platform.txt does not fix things.

The 1.8.12 release notes state Improved precompiled libraries handling - but apparently there is a breakage, not improvement there, at least for the ESP32 architecture.

Hi @cpq ,
it's not really this change but this . When compiling on ESP8266, you will get a warning:
The plaform does not support 'compiler.libraries.ldflags' for precompiled libraries. (the typo is really there, @cmaglie )

In this new version of the builder we are not doing any heuristics to find the right spot where the ldflags should be inserted (this was causing many bugs on its own); instead, we fully trust the core makers to add explicit support to precompiled libs.

In ESP8266 core, for example, the core authors should modify the combine pattern with

recipe.c.combine.pattern="{compiler.path}{compiler.c.elf.cmd}" {build.exception_flags} -Wl,-Map "-Wl,{build.path}/{build.project_name}.map" {compiler.c.elf.flags} {compiler.c.elf.extra_flags} -o "{build.path}/{build.project_name}.elf" -Wl,--start-group {object_files} "{archive_file_path}" {compiler.c.elf.libs} {compiler.libraries.ldflags} -Wl,--end-group  "-L{build.path}"

@facchinm , thank you - appreciated the correct reference.

I've made a diff (see at the end of this post) to the esp32's platform.txt, by adding a compiler.libraries.ldflags= line and adding {compiler.libraries.ldflags} to the recipe.c.combine.pattern. Build a minimal sketch with the precompiled library, and got an error due to that library was specified twice in the command line. Using Arduino 1.8.13:

/Users/lsm/Documents/Arduino/hardware/espressif/esp32/tools/xtensa-esp32-elf/bin/xtensa-esp32-elf-gcc 
  [skipped] -lstdc++ 
 -L/Users/lsm/Documents/Arduino/libraries/mDash/src/esp32 -lmDash -lmDash 
-Wl,--end-group [skipped]

@facchinm , can you suggest a correct patch to the esp32's platform.txt, please?

Current patch, made against https://github.com/espressif/arduino-esp32/blob/master/platform.txt 👍

diff --git a/platform.txt b/platform.txt
index 6583ee77..b0514784 100644
--- a/platform.txt
+++ b/platform.txt
@@ -20,6 +20,8 @@ compiler.warning_flags.default=
 compiler.warning_flags.more=-Wall -Werror=all
 compiler.warning_flags.all=-Wall -Werror=all -Wextra
 
+compiler.libraries.ldflags=
+
 compiler.path={runtime.tools.xtensa-esp32-elf-gcc.path}/bin/
 compiler.sdk.path={runtime.platform.path}/tools/sdk
 compiler.cpreprocessor.flags=-DESP_PLATFORM -DMBEDTLS_CONFIG_FILE="mbedtls/esp_config.h" -DHAVE_CONFIG_H -DGCC_NOT_5_2_0=0 -DWITH_POSIX "-I{compiler.sdk.path}/include/config" "-I{compiler.sdk.path}/include/app_trace" "-I{compiler.sdk.path}/include/app_update" "-I{compiler.sdk.path}/include/asio" "-I{compiler.sdk.path}/include/bootloader_support" "-I{compiler.sdk.path}/include/bt" "-I{compiler.sdk.path}/include/coap" "-I{compiler.sdk.path}/include/console" "-I{compiler.sdk.path}/include/driver" "-I{compiler.sdk.path}/include/efuse" "-I{compiler.sdk.path}/include/esp-tls" "-I{compiler.sdk.path}/include/esp32" "-I{compiler.sdk.path}/include/esp_adc_cal" "-I{compiler.sdk.path}/include/esp_event" "-I{compiler.sdk.path}/include/esp_http_client" "-I{compiler.sdk.path}/include/esp_http_server" "-I{compiler.sdk.path}/include/esp_https_ota" "-I{compiler.sdk.path}/include/esp_https_server" "-I{compiler.sdk.path}/include/esp_ringbuf" "-I{compiler.sdk.path}/include/espcoredump" "-I{compiler.sdk.path}/include/ethernet" "-I{compiler.sdk.path}/include/expat" "-I{compiler.sdk.path}/include/fatfs" "-I{compiler.sdk.path}/include/freemodbus" "-I{compiler.sdk.path}/include/freertos" "-I{compiler.sdk.path}/include/heap" "-I{compiler.sdk.path}/include/idf_test" "-I{compiler.sdk.path}/include/jsmn" "-I{compiler.sdk.path}/include/json" "-I{compiler.sdk.path}/include/libsodium" "-I{compiler.sdk.path}/include/log" "-I{compiler.sdk.path}/include/lwip" "-I{compiler.sdk.path}/include/mbedtls" "-I{compiler.sdk.path}/include/mdns" "-I{compiler.sdk.path}/include/micro-ecc" "-I{compiler.sdk.path}/include/mqtt" "-I{compiler.sdk.path}/include/newlib" "-I{compiler.sdk.path}/include/nghttp" "-I{compiler.sdk.path}/include/nimble" "-I{compiler.sdk.path}/include/nvs_flash" "-I{compiler.sdk.path}/include/openssl" "-I{compiler.sdk.path}/include/protobuf-c" "-I{compiler.sdk.path}/include/protocomm" "-I{compiler.sdk.path}/include/pthread" "-I{compiler.sdk.path}/include/sdmmc" "-I{compiler.sdk.path}/include/smartconfig_ack" "-I{compiler.sdk.path}/include/soc" "-I{compiler.sdk.path}/include/spi_flash" "-I{compiler.sdk.path}/include/spiffs" "-I{compiler.sdk.path}/include/tcp_transport" "-I{compiler.sdk.path}/include/tcpip_adapter" "-I{compiler.sdk.path}/include/ulp" "-I{compiler.sdk.path}/include/unity" "-I{compiler.sdk.path}/include/vfs" "-I{compiler.sdk.path}/include/wear_levelling" "-I{compiler.sdk.path}/include/wifi_provisioning" "-I{compiler.sdk.path}/include/wpa_supplicant" "-I{compiler.sdk.path}/include/xtensa-debug-module" "-I{compiler.sdk.path}/include/esp-face" "-I{compiler.sdk.path}/include/esp32-camera" "-I{compiler.sdk.path}/include/esp-face" "-I{compiler.sdk.path}/include/fb_gfx"
@@ -81,7 +83,7 @@ recipe.S.o.pattern="{compiler.path}{compiler.c.cmd}" {compiler.cpreprocessor.fla
 recipe.ar.pattern="{compiler.path}{compiler.ar.cmd}" {compiler.ar.flags} {compiler.ar.extra_flags} "{archive_file_path}" "{object_file}"
 
 ## Combine gc-sections, archives, and objects
-recipe.c.combine.pattern="{compiler.path}{compiler.c.elf.cmd}" {compiler.c.elf.flags} {compiler.c.elf.extra_flags} -Wl,--start-group {object_files} "{archive_file_path}" {compiler.c.elf.libs} -Wl,--end-group -Wl,-EL -o "{build.path}/{build.project_name}.elf"
+recipe.c.combine.pattern="{compiler.path}{compiler.c.elf.cmd}" {compiler.c.elf.flags} {compiler.c.elf.extra_flags} -Wl,--start-group {object_files} "{archive_file_path}" {compiler.c.elf.libs} {compiler.libraries.ldflags} -Wl,--end-group -Wl,-EL -o "{build.path}/{build.project_name}.elf"
 
 ## Create eeprom
 recipe.objcopy.eep.pattern={tools.gen_esp32part.cmd} -q "{build.path}/partitions.csv" "{build.path}/{build.project_name}.partitions.bin"