On building platform2 packages written in C++, platform2 directory is added to include search path so that its own header files can be found. However this also allows loading header files from other packages too, which should be avoided. Instead, header files installed to the target chroot should be used.
I initially thought this problem should be addressed individually in each package, but it turned out this is a problem common to mostly all platform2 packages. I'm merging bugs I filed for individual packages.
how about we institute the policy:
- installed platform2 packages use /usr/include/chromeos/ to install header files
- when including headers from other packages, you have to use <chromeos/package-name/foo.h>
- when including headers from our own local package, you use "package-name/foo.h"
that should avoid conflicts where <package/foo.h> and "package/foo.h" resolve to the same path because we add -I.../platform2/ to the compiler flags. Ben suggested adjusting that -I to -iquotedir, but i think that only works for gcc. not sure if clang has a corresponding flag.
Sorry, I've forgotten some of the details here. In practice, does using <package/foo.h> already make us favor installed files rather than ones from platform2? Is the main problem that devs sometimes use double-quotes to include files that belong to other packages?
I think that libbrillo used to install its headers to /usr/include/chromeos back when it was called libchromeos, but presumably nobody cares about renaming it back now.
unfortunately, angle brackets don't make a difference when using -I (which is what we use with platform2/ atm). that's why Ben suggested using -iquotedir instead ... then we would get the behavior where "package/foo.h" would only look under platform2, and <package/foo.h> would only look in system paths like /usr/include.
libbrillo did historically use /usr/include/chromeos/, but it's now under /usr/include/brillo/. i don't think we need to migrate that back for this proposal.
Thanks for the explanation. #9 sounds fine to me. Would using "other-package/foo.h" to include something from another package actually fail to be resolved, or would a pre-upload hook also be needed to prevent people from doing that?
I personally think the comment #12 in that bug provides the right solution: if your header is public and needs to include another header, that include should always be relative to the current directory. So we might have something like:
platform2/some_project/public/foo.h
platform2/some_project/public/bar.h
platform2/some_project/public/nested/baz.h
foo.h would include bar.h as
#include "bar.h"
and baz.h would be
#include "nested/baz.h"
This works no matter where the public header is installed as long as the directory structure is maintained.
Stepping back, the problem is, if the header file in another platform2's lib is touched unexpectedly, it would;
- cause a sandbox crash and build fails sometimes, or
- use system installed libs, e.g. /usr/include/..., for the board, and the build succeeds.
note: I assume that DEPENDS is correctly written in .ebuild file, so files under /usr/include is up-to-date.
Then, is it possible to avoid sandbox crashing, and instead, can it behave as if no file is there so that the compiler will automatically fallback to system?
If the sandbox does not have such a feature, can we extend it?
It's easier to understand for me if include paths are same.
# TBH, if the paths is relative to chromeos root (i.e., platform2/some_project/foo.h etc.) rather than to platform2 root (i.e. some_project/foo.h etc. like now), it is much easier for me, but it is out of scope for this bug, I think.
So I think I'm running into a problem related to this area. I need to use some functionality from libshill-net in arc-networkd (which currently only uses shill-client). When I add this dependency and include a file e.g.
#include <shill/net/rtnl_handler.h>
The build fails with access violations, e.g.
...
arc-networkd-9999: [0/3] CXX obj/arc/network/arc-networkd.manager.o * ACCESS DENIED: open_rd: /mnt/host/source/src/platform2/shill/net/rtnl_listener.h
arc-networkd-9999: * ACCESS DENIED: open_rd: /mnt/host/source/src/platform2/shill/net/shill_export.h
arc-networkd-9999: * ACCESS DENIED: open_rd: /mnt/host/source/src/platform2/shill/net/rtnl_listener.h
arc-networkd-9999: * ACCESS DENIED: open_rd: /mnt/host/source/src/platform2/shill/net/shill_export.h
arc-networkd-9999:
arc-networkd-9999: [1/3] CXX obj/arc/network/arc-networkd.manager.o
arc-networkd-9999: FAILED: obj/arc/network/arc-networkd.manager.o
I can get around this by adding shill/net to CROS_WORKON_SUBTREE but other projects (e.g. platform2/portier) don't need to do this. What is the correct way to handle this?
Comment 1 by nya@chromium.org
, Feb 6 2018