Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Additional include paths libs#502

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
facchinm wants to merge3 commits intoarduino:master
base:master
Choose a base branch
Loading
fromfacchinm:additional_include_paths_libs

Conversation

facchinm
Copy link
Member

@facchinmfacchinm commentedNov 29, 2019
edited by rsora
Loading

Fixes#501

brusherru, SteffanoP, oclyke, jcj83429, Master811129, BlueAndi, dekiesel, sleepdefic1t, agucova, curious-odd-man, and 3 more reacted with heart emoji
@mascimasci added this to the0.8.0 milestoneDec 31, 2019
@mascimasci modified the milestones:0.8.0,0.9.0Feb 14, 2020
@rsorarsora modified the milestones:0.9.0,0.10.0Feb 21, 2020
@oclyke
Copy link

Thanks@facchinm!

I have done some initial testing. There seems to be a problem in the 'Detecting Libraries' build step.

Procedure:

  1. Replace binaryarduino-builder in my installment with the one you provided (windows)
  2. Write a sample library that needs to access the API of an "immutable" submodule (attached as .zip - calledaipt_lib for 'additional include path testing')

Thelibrary.properties file includes the line:

additional_include_paths=src/ossdl/include

This is because I expect the builder to create an include in the tool like "-I{path_to_lib}/src/ossdl/include" so that the tools can find the header file of the submodule
aipt_lib.zip

  1. Write a test sketch
#include "aipt.h"void setup() {  // put your setup code here, to run once:  Serial.begin(115200);  delay(500);  testFun1();         // make sure standard Arduino library features working  printOSSDLName();   // test functionality from the 'submodule'  Serial.print("Done");}void loop() {  // put your main code here, to run repeatedly:}
  1. Try to compile (Board: Uno)

Results

  • Running as-is fails when detecting libraries used. I don't see the additional include path being used in the build lines that are used to detect libraries, so causing errors and aborting.
  • If you fix the include problem (comment out#include "ossdl.h" and uncomment#include "ossdl/include/ossdl.h" in 'aipt.h') then compilation continues which reveals that the correct include pathis being used later on in the build process. E.g.
    "-IC:\\Users\\owen.lyke\\Documents\\Arduino\\libraries\\aipt_lib\\src" "- IC:\\Users\\owen.lyke\\Documents\\Arduino\\libraries\\aipt_lib\\src\\ossdl\\include"

More thoughts

How does library detection work? I think there lies the cause of the problem

  1. Run preprocessor on the main sketch - record what includes there were. Does not seem to require knowledge of any libraries (good...)
  2. Run preprocessor on main sketch again but this time add a-I flag for the library's src folder.this step seems to be missing-I flags for the additional include paths I think that at this time the library has already been loaded so the additional include paths could be included here too
  3. Run preprocessor on library source files. This step does include-I flags for the additional include paths

A small nit - I notice that changing library.properties does not signal a change of the library to the builder and so a cached version of the library may be used, however if theadditional_include_paths field is changed that does affect compilation. I'm not sure what can be done about this. In the worst-case scenario I'm sure people (library developers) can just deal with it and re-save their files after changing that field.

@facchinmfacchinmforce-pushed theadditional_include_paths_libs branch froma11f866 to6058360CompareMarch 11, 2020 10:22
@facchinm
Copy link
MemberAuthor

@oclyke
first of all, thanks for taking the time to test it.
Something changed in the cache code from the time I first wrote the patch, so the build was not behaving as intended. I pushed an extra commit4853add that fixes the preprocessor phase.
Said that, I'd really love if people writing libraries could just usereal relative includes (without hoping that the buildsystem fixes all the-I for them 🙂 )

@oclyke
Copy link

@facchinm
Sweet - if I have time I will try to build the arduino-builder executable (new process for me) and test it out. I am very excited about this potential addition to Arduino :)

I agree that authors of original libraries should write proper relative includes. But even if every project did that this feature would still be a valuable addition to Arduino -- why? Because now, with a very simple mechanism (git submodules), we can 'symbolically' include and build off of other open-source projects. All the while fighting code duplication, easing maintenance, and more strongly supporting other open-source projects.

See, I would not expect the author of an original library to use theadditional_include_paths feature - they should write includes correctly for their library. But when they include another work as a sub-folder with its own root then they will be very glad to have this ability.

@SciLor
Copy link

Could you provide another build to test?

@ubidefeo
Copy link

@SciLor
since this commit hasn't been merged yet (pending review),
you could checkout the additional_include_paths_libs branch of thehttps://github.com/facchinm/arduino-cli fork
Instructions to build can be found here, and it's not that much of a hassle
https://arduino.github.io/arduino-cli/dev/CONTRIBUTING/#building-the-source-code

We will end up merging this PR once we get to review it, it shouldn't take long
u.

@SciLor
Copy link

@ubidefeo
I tried to build it myself, downloaded go/task but building doesn't seem to work. There seems something missing in the Prerequisites.

C:\temp\arduino-cli>task build"tr": executable file not found in $PATH"grep": executable file not found in $PATHio: read/write on closed pipetask: Command "echo `go list ./... | grep -v legacy | tr '\n' ' '`" in taskvars file failed: exit status 1

@per1234
Copy link
Contributor

Hi@SciLor. These errors are caused by the use of shell commands that aren't available when using Windows cmd or PowerShell. Fortunately, there is a nice tool called "Git BASH" that is included with Git for Windows. If you have Git installed, you already have it on your computer. If not, you can download it from the official Git website:
https://git-scm.com/download/win

If you then startC:\Program Files\Git\git-bash.exe and runtask build you should have a successful build of Arduino CLI.

@SciLor
Copy link

@per1234 thanks for the hint. Works as charm.
task build will just build the arduino-cli.exe for me. How to build the arduino-builder.exe?

@per1234
Copy link
Contributor

@SciLor there's no need for arduino-builder.exe. Just use the arduino-cli.exe executable that was generated viatask build directly from the command line:
https://arduino.github.io/arduino-cli/latest/commands/arduino-cli/

Theadditional_include_paths field of library.properties works just as well with Arduino CLI and it will allow you fully test the functionality of this PR.

@SciLor
Copy link

@per1234 I can't use arduino-cli as I am using energia (arduino fork) that is still using the arduino-builder.

@cmagliecmaglie removed this from the0.12.0 milestoneSep 14, 2020
@cmagliecmaglie added this to the0.14.0 milestoneSep 14, 2020
@jan-chvojka
Copy link

Any progress on this? I updated Arduino and have wondered why is my project not compiling anymore. It didn't kept my additional libraries I put to the installation folder. So I thought I will keep the libraries in different folder. What was my surprise as a developer to find out that there is no way to add additional paths...

@ubidefeo
Copy link

@jan-chvojka

I updated Arduino

do you mean thearduino-cli?

It didn't kept my additional libraries I put to the installation folder.

which is your installation folder?

out that there is no way to add additional paths...

you can add additional libraries inarduino-cli using the--libraries flag but we currently found a bug and are trying to address it

@per1234per1234 added the type: enhancementProposed improvement labelFeb 3, 2021
@per1234per1234 reopened thisMar 30, 2021
@silvanocerzasilvanocerza removed this from the0.14.0 milestoneMay 11, 2021
@per1234per1234 added the topic: codeRelated to content of the project itself labelJan 15, 2022
@arielscarpinelli
Copy link

Is there anything we could do to accelerate merging this one?

Ayilay, brend3n, AasmundN, and JoacoG reacted with heart emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@cmagliecmaglieAwaiting requested review from cmaglie

At least 1 approving review is required to merge this pull request.

Assignees

@cmagliecmaglie

Labels
topic: codeRelated to content of the project itselftype: enhancementProposed improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow Libraries to Extend Include Path
13 participants
@facchinm@oclyke@SciLor@ubidefeo@per1234@jan-chvojka@arielscarpinelli@masci@cmaglie@silvanocerza@AlbyIanna@umbynos@rsora

[8]ページ先頭

©2009-2025 Movatter.jp