raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.66k stars 911 forks source link

Trouble building Bazel+pico_lwip_contrib_freertos #1798

Open jaguilar opened 1 month ago

jaguilar commented 1 month ago

Besides #1796, I'm still having some issues building with bazel and LWIP. I'm writing down the results of what investigation I've done for either myself or someone else to follow up on later.

I think the issue stems from a dependency that is not completely expressed in the Bazel-accepted DAG-ish way. So, pico_lwip_contrib_freertos in lwip.BUILD depends on pico_lwip_core. Let's look at one compile-time dependency chain there:

All well and good, right? We're building pico_lwip_contrib_freertos, and that will provide sys_arch.h. Sadly not. The includes directories from cc_library get added to -isystem only for the dependents of the library that defines it. Not the library's dependencies. So when we compile mem.c, contrib/ports/freertos/include is nowhere to be found on the compiler command-line.

This kind of makes sense. If a library is depending on a thing, it can't be necessary for the thing depended upon to require the library's existence. But you can actually do that in C. So I think we'll have to figure out how to express it.

Intuitively, it seems to me like maybe the solution is something along the lines of "treat sys_arch.h as an interface and put it on the hdrs line of pico_lwip_core, but the definition is only available when pico_lwip_contrib_freertos is linked. But I'm not an expert on adapting C libraries to bazel so I'm open to other ideas.

jaguilar commented 1 month ago

(Indeed, adding the sys directory to the includes of the core library does allow us to compile. But it's ?probably? not the right thing to do?

armandomontanez commented 1 month ago

Sounds like ye ol' circular dependency. I'll try to carve out time to take a peek at this on Monday. I think there's some nuance around lwip's NO_SYS mode that needs to properly be hooked up as well.

jaguilar commented 1 month ago

FWIW, I cracked this in my working copy by creating a header-only library that both pico_lwip_core and pico_lwip_contrib_freertos could depend on. No idea if this is proper but it seems to work okay.

armandomontanez commented 1 month ago

Yep! 👍 That tends to be how I solve these kinds of cycles and was going to be my first step.