search

nanopb / nanopb

Protocol Buffers with small code size
https://jpa.kapsi.fi/nanopb/
zlib License
4.32k stars 852 forks source link

PlatformIO include issue after scanning for other libraries #818

Closed Timvrakas closed 11 months ago

Timvrakas commented 2 years ago

I'm building with PlatformIO, and I've got a strange issue where after the library manager installs other libraries (in my case rBase64), the platformio_generator.py script runs twice (see #734) and for some reason when trying to build the generated files, the build command is missing -I.pio/libdeps/quail/Nanopb and so the build fails with fatal error: pb.h: No such file or directory.

If I build again, it works fine, but if I delete rBase64/ from my libdeps, then on the next build when the library manager downloads it anew, the build fails again. This is causing CI builds to fail because they are always fresh builds.

I've added a manual -I.pio/libdeps/quail/Nanopb to my platformio.ini, but I'm sure that's not the best way.

Perhaps the issue is that the generator is calling env.BuildSources() before the env actually contains the library itself? I'm don't fully understand the inner workings, so I'm not sure.

(cc @valeros)

PetteriAimonen commented 2 years ago

@hacker-cb just hilighting you, in case you are interested.

Sounds like a quite tricky issue to figure out, probably involves the platformio automatic include path discovery. It seems the nanopb generator/platformio_generator.py doesn't manually add the include path at any point, but in general that should be fine. Platformio should see #include <pb_encode.h> or similar in source code and add the include path based on that.

Timvrakas commented 2 years ago

I have a minimal A/B setup that I used to narrow down to this issue. If there's parts of env anyone wants to see just let me know.

hacker-cb commented 2 years ago

@PetteriAimonen I added PR, which should helps to find nanopb dependency based on parsed #include <pb_encode.h> macro. Only one way to test it is to publish next version to PlatformIO registry.

PetteriAimonen commented 2 years ago

@hacker-cb Can you explain why publishing to registry is needed? Can't it be tested using the direct lib specification that takes newest from git? Something like this in platformio.ini:

lib_deps =
    nanopb=https://github.com/hacker-cb/nanopb#platformio-fix-ldf-include-dir
hacker-cb commented 2 years ago

Something like this in platformio.ini:

But in this case you will specify depenedency explicitly and LDF can use srcDir as path for searching headers. I'm not sure, you can try, with and without my PR. I don't have possibility to test it now.

In any case, includeDir was missed before, and default value was used, which points to the include dir which is not correct for the nanopb project.

PetteriAimonen commented 2 years ago

@hacker-cb Ok, thanks for the explanation. The change seems correct in any case, so I'll merge it. But I won't be pushing extra releases to platformio registry.

hacker-cb commented 2 years ago

@hacker-cb Ok, thanks for the explanation. The change seems correct in any case, so I'll merge it. But I won't be pushing extra releases to platformio registry.

Keep in mind, that searching headers will not work until you will publish new release.

Timvrakas commented 2 years ago

Built with the latest version from git, no improvement. I don't think the includes are being properly added to the build environment for the generated files, but I'm not sure why,

PetteriAimonen commented 1 year ago

What is the status of this after the recent changes to nanopb platformio rules and now that 0.4.7 is published?

Timvrakas commented 1 year ago

In file included from .pio/build/quail/nanopb/generated-src/cmd.pb.c:4: .pio/build/quail/nanopb/generated-src/cmd.pb.h:6:10: fatal error: pb.h: No such file or directory

Still getting the same issue, in both CI and when I do a fresh build locally. But once the .pio/libdeps folder is established, then it builds properly the second time.

Timvrakas commented 1 year ago

Ah, I've found a new detail. I can only get the error to re-appear if I delete the rBase64 library from .pio/libdeps/{env}/ The library is here, but it seems to have been deleted off GitHub. I'll try to follow this lead.

Timvrakas commented 1 year ago

I appear to have fixed the issue by specifying the rBase64 be pulled from Git, rather than the PIO Registry. I'm not sure why that would cause an issue, but I need to switch to an actively maintained Base64 library in any case. I don't know if the fix by @hacker-cb had an effect or not.

PetteriAimonen commented 11 months ago

I'm not sure if there is anything left to fix here, so I'm closing this off.