Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integration with @PlatformIO Build System #3321

Merged
merged 1 commit into from
Aug 7, 2017
Merged

Integration with @PlatformIO Build System #3321

merged 1 commit into from
Aug 7, 2017

Conversation

ivankravets
Copy link
Collaborator

  • Build script for PIO
  • CI Integration

"name": "framework-arduinoespressif8266",
"description": "Arduino Wiring-based Framework (ESP8266 Core)",
"url": "https://github.com/esp8266/Arduino",
"version": "2.4.0-rc.1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to keep this in sync with platform.txt?
I would probably go with 2.4.0-dev or something along these lines for git version, because -rc1 is a quite specific tagged version...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.4.0-dev is good, but need to update it to the final release if you make release tag.

Currently, we have own forked and patched ESP826 Core for Arduino. Our community does not like it and asks us to use your repository. That is why I added package.json manifest. It's required for PIO Package Manager. After merge/release, I'll switch our development platform to this repo here.

],
LINKFLAGS=[
"-Wl,-wrap,system_restart_local",
"-Wl,-wrap,spi_flash_read"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to pull these flags from platform.txt? As far as i can tell, platformio-build.py is normal Python program, so opening platform.txt and extracting flags from it seems to be quite doable. Otherwise the flags would need to be updated in two files...

Something along the lines of what makeEspArduino does, but in Python.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arduino Build System is a self-written bunch of files where they play with common problems which were resolved by make and other build tools a lot of years ago. We don't depend on Arduino IDE, their build system or even Java. For us, Arduino is just a framework with some "build logic".

We didn't invent the new wheel and our PIO Build System is based on SCons construction tool. It's 100% pure Python implementation with Zero-dependencies to other packages or even to the OS software. We applied a lot of patches to SCons to make it better for embedded using and now it's super-fast and reliable.

In platform.txt all flags are located in ONE LINE, where in PlatformIO we separate them into multiple scopes. Later, a developer is able to "unflag" some of them directly from project configuration file or add new. Here is very important that each flag/option is located in correct scope.

We would be thankful if you support this build script. I see that it doesn't require a lot of efforts. Take a look at platformio-build.py for ESP32 by @me-no-dev . I don't see here a hundred commits.

In any case, you can ping me or @valeros any time and we will help as soon as possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is very important that each flag/option is located in correct scope.

Can you point me to the description which flag option should be located in which scope? If there is such a description, it should be more or less straightforward to write the code which will parse flags from platform.txt and put them into the correct scopes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the changes I have done to the file are because libs change often on ESP32, other than that I let PlatformIO be as it is. I have asked @ivankravets a couple of times when I implement a new core feature (like partitions), so it kinda helps keep both in sync. He would not need to read through every commit to learn that a new command/feature is required :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is base build script for ESP8266. Build script from this PR extends that script.

Also, full list with SCons construction variables

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, i'm reading these and given that there doesn't seem to be a strict rule which flag has to go where, i'm trying to find the pattern. Why do -fno-inline-functions, -ffunction-sections, -fdata-sections go into the ESP8266-specific script? Shouldn't that be a framework-level choice? None of these flags are specific to the ESP8266 or Xtensa.F_CPU flag is also an Arduino-specific thing, should it not be in the framework level flags? Sorry for a lot of questions folks, this thing is new to me, once i understand it i can probably clean this up to some extent.

Another way of approaching this is generating both platform.txt and the platformio script from a single configuration file. That would likely be even more straightforward. Would reading these flags from some .json or .yml file work for you?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is base configuration. You can "override" all scopes with own flags/data. Use env.Replace(SCOPE1=value, SCOPE2=value..) instead of env.Append(SCOPE1=value, SCOPE2=value..).

To dump current build environment, use print env.Dump() in any place

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that this is the base configuration, and i understand that it can be overridden...

My question above was why these flags are in the ESP8266-specific configuration, when logically they should not be in the chip-dependent part.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way of approaching this is generating both platform.txt and the platformio script from a single configuration file.

This is a good idea. I'm not sure but @projectgus works on the same for ESP-IDF.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My question above was why these flags are in the ESP8266-specific configuration, when logically they should not be in the chip-dependent part.

Good question :) This is a historical problem. We had 1 script for ESP8266 in PlO 2.0 and kept inside PIO Core (2 years ago). 1 year ago we released PIO 3.0 with decentralized platforms where each platform using PIO Build API decides how to build own frameworks, SDKs, HALs, etc.

Later, we added support for ESP8266 SDK and Simba framework. After that, we don't touch that part of code, need to check if Simba uses our default environment.

In any case, we are open for discussion and PR. Feel free to make contribution to https://github.com/platformio/platform-espressif8266

@@ -157,6 +157,18 @@ SECTIONS
*.c.o( EXCLUDE_FILE (umm_malloc.c.o) .literal*, \
EXCLUDE_FILE (umm_malloc.c.o) .text*)
*.cpp.o(.literal*, .text*)
*.pioenvs\\*\\lib*.a:(EXCLUDE_FILE (umm_malloc.o) .literal*, \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to tweak object file names to make them same as in Arduino?

Copy link
Collaborator Author

@ivankravets ivankravets Jun 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same story. In PlatformIO (platformio.ini), you can have multiple build environment per a single project. For example:

[env:release]
platform = espressif8266
framework = arduino
board = myboard
build_flags = -DRELEASE ... other flags
lib_deps =
    StableLib@~1.2.3
    otherLib@>2.0

[env:debug]
platform = espressif8266
framework = arduino
board = myboard
build_flags = -DDEBUG... other flags
lib_deps =
    https://git.repo/stable.git
    otherLib@~3.0

You will have 2 different build environments and the final objects will be located %project_root%/.pioenvs/%env_name%/.... All dependent libraries will be built to static libraries and placed to .pioenvs folder per each build environment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is the case, the build product may be different, because Arduino doesn't link libraries into archive files, an archive file is only produced for the "core". All object files from libraries get passed to the linker. This may have implications for libraries which override weak functions defined in the core. If the function within the library is placed into a separate object file, and this object file doesn't contain any other unresolved functions, the linker will not include this object file from the library into the final output. We don't happen to have this kind of situation now, but once we have it eventually, working around this may prove interesting :)

But we have diverged from the original question... If you don't mind, i'll set up some code generation to create these extra LD script lines.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may have implications for libraries which override weak functions defined in the core.

PlatformIO Build System is very flexible. You can even override the WHOLE build process for a single library with a custom extra script. According to your question, library archiving is the default behavior. You are able to turn if off for specific library. See http://docs.platformio.org/en/latest/librarymanager/config.html#libarchive

If you don't mind, i'll set up some code generation to create these extra LD script lines.

No problem, thanks! For us would be good if you support this script. Users will be happy because will not be a delay "When we update script for new changes?". We have this problem for other 20 development platforms. Need to maintain all of them :(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are able to turn if off for specific library. See http://docs.platformio.org/en/latest/librarymanager/config.html#libarchive

That seems to be a library-level switch. Can we apply this switch at framework level?

Also, back to my original question, is it possible to tweak object file names at framework level to make them the same as in Arduino? I.e. .c files get compiled into .c.o, .cpp into .cpp.o and so on? Directory layout doesn't matter much in this case, as the LD script currently has filename-dependent rules only.

Need to maintain all of them

Well, that's the part of platformio's value proposition, so there's probably no way around that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to be a library-level switch. Can we apply this switch at framework level?

Currently, is not possible. Need to create library.json manifest and make configuration. However, I can open issue for that if you need.

I.e. .c files get compiled into .c.o, .cpp into .cpp.o and so on?

It's possible if build each file separately, see docs. We don't play with env.Object() and use env.VariantDir() where the rest work for us SCons does automatically. Is this critical for you?

@ivankravets
Copy link
Collaborator Author

Ivan, what do we need to see this PR merged?

P.S: A lot of developers ask us.

@igrr
Copy link
Member

igrr commented Jul 24, 2017

As discussed above, there are two issues currently, which need to be resolved:

  1. platform.io links each Arduino library into a static archive, and then links all the archives together. This does not match behavior of arduino-builder and can lead to issues when the library attempts to override a weak symbol defined in the core (e.g. such override will work in Arduino IDE but will not work in platform.io).

  2. Arduino IDE uses different naming pattern for object files: .c files are compiled into .c.o, .cpp into .cpp.o, etc. You have mentioned that it is possible to tweak the build process to match this pattern. That would allow using the same linker script for Arduino IDE and platform.io.
    (An alternative would be to do some preprocessing/code generation for the linker script, which is probably more trouble).

The remaining issue is about pulling build flags into platform.io and platform.txt from the same source, but that can be addressed later, IMO.

@ivankravets
Copy link
Collaborator Author

  1. Library archiving could be turned off in a manifest. Nevertheless, opened new issue where user will be able to disable archiving. Could you point me to a library which needs to override weak symbol? I will provide PlatformIO's library.json manifest which disables a default behavior.

  2. "You have mentioned that it is possible to tweak the build process to match this pattern.". I would stay without "deep" modification of SCons build process. The same when you will "hack" make or cmake. Yep, that is possible, but I prefer to stay with a high level API which SCons provides and avoid any conflicts between migrating to a new version. I don't see any conflicts when we will keep PlatformIO records too.


So... The main question... Why do you mean under "ESP8266 for Arduino" Arduino IDE? The idea of this repository is to provide Arduino/Wiring API for ESP8266 HAL. Let's developers/users decide which build system or IDE to use. No need to force them.

Arduino IDE build system is very limited and primitive. For example, take a look at an example how developers use this core and configure it:

@igrr
Copy link
Member

igrr commented Aug 1, 2017

Could you point me to a library which needs to override weak symbol?

GDBStub library overrides weak symbols defined in the core. https://github.com/esp8266/Arduino/blob/master/libraries/GDBStub/src/internal/gdbstub.c#L795

Regarding object file naming: well, we have to resort to a hack somewhere: either in the build script, or in the linker script (i refer to the repeated patterns in the linker script which you are adding in this MR). I would very much prefer not to have hacks in the linker script, since I get to maintain them. I fully understand that you don't want to have hacks in the build script either :) That in mind, we should probably go for the lesser hack, which based on my understanding would be the one in the linker script. So, consider this issue sorted out.

Why do you mean under "ESP8266 for Arduino" Arduino IDE? The idea of this repository is to provide Arduino/Wiring API for ESP8266 HAL. Let's developers/users decide which build system or IDE to use. No need to force them.

I don't mind if this code it used in other build environments. But as I have already mentioned above, I don't necessarily subscribe to be the maintainer of integration with other build systems, especially the ones which I'm not an expert in. If there is a person who tells me that they want to support another build system and maintain it, that is fine with me and I will give such person access to this repository. For example, some time ago @Links2004 used to contribute a lot to this project, and he was also maintaining Eclipse IDE integration and instructions. I trust that there would be people in the intersection of platform.io and ESP8266 communities who would be willing to maintain this integration once it is merged :)

So just to recap, the only pending issue is the one with weak linking. As discussed, i will deal with the second issue (hacks in the linker script), likely by introducing an intermediate build step (e.g. make bootstrap). The third issue about synchronizing build flags between two build systems will be ignored for now, in the hope that someone has time to address it later.

@ivankravets
Copy link
Collaborator Author

ivankravets commented Aug 1, 2017

So just to recap, the only pending issue is the one with weak linking.

  1. lib_archive option is added to PIO Core ( platformio/platformio-core@7f63928 ). New release 3.4.1 is planned for this week. Users can control this process now.
  2. I added custom library manifest (@PlatformIO library manifest for GDBStub #3488) which instructs PIO to don't use archiving for GDBStub.

P.S: I've just tried to build this huge project (https://bitbucket.org/xoseperez/espurna/src/e6e9f9d6d4bd/code/?at=master ) with 30+ dependent libraries without archiving (like Arduino IDE) and received section .text' will not fit in region iram1_0_seg' :) With archiving all works good, GCC linker does some optimization when looks for symbols in archive tables.


I don't necessarily subscribe to be the maintainer of integration with other build systems, especially the ones which I'm not an expert in. ...I will give such person access to this repository.

Could you give me an access? I'll not touch source files except PlatformIO Build Script when it needs.

Indeed, we don't depend on this PR, we use forked version and patch it. The idea with this PR was to give access for PlatformIO community to use original source code without modifications and don't wait when we will re-sync our forked repository.

I would be thankful for the access. It will save my and your time. Also, if you will have any issues related to PlatformIO, don't hesitate to ping me directly.

Thanks!

igrr pushed a commit that referenced this pull request Aug 7, 2017
Custom manifest for @platformio which instructs PIO Build System to do not archive library's object files. See 

- #3321 (comment)
- http://docs.platformio.org/en/latest/librarymanager/config.html#libarchive
@igrr igrr merged commit ff4bb73 into esp8266:master Aug 7, 2017
@ivankravets
Copy link
Collaborator Author

Thanks a lot, Ivan!!! 👍

d-a-v pushed a commit to d-a-v/Arduino that referenced this pull request Sep 29, 2017
Custom manifest for @platformio which instructs PIO Build System to do not archive library's object files. See 

- esp8266#3321 (comment)
- http://docs.platformio.org/en/latest/librarymanager/config.html#libarchive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants