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

Trouble building Bazel+pico_lwip_contrib_freertos #1798

Closed
jaguilar opened this issue Aug 11, 2024 · 5 comments
Closed

Trouble building Bazel+pico_lwip_contrib_freertos #1798

jaguilar opened this issue Aug 11, 2024 · 5 comments
Milestone

Comments

@jaguilar
Copy link
Contributor

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:

  • pico_lwip_core -> mem.c
  • mem.c -> arch.h
  • arch.h -> sys_arch.h

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
Copy link
Contributor Author

(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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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

@kilograham
Copy link
Contributor

closing as fixed though it isn't yet tested

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

No branches or pull requests

3 participants