rules_foreign_cc icon indicating copy to clipboard operation
rules_foreign_cc copied to clipboard

pkgconfig_tool_unix not hermetic

Open dstufft opened this issue 2 years ago • 3 comments

It looks like when attempting to build pkgtool, rules_foreign_cc is not setting the CC variable, causing the pkgtool configure to search path for gcc, clang, etc, and using that if it finds one. This can be reproduced using a docker image (to ensure there's no CC and a hermetic CC compiler (rather than the default option of letting Bazel search the $PATH for one.

Then once you get past that, it attempts to just invoke make without attempting to use the toolchain provided make, so if you also don't have make on your $PATH, then it will fail then.

Unfortunately, it appears even with all of that, it still fails, but it fails with incompatible integer to pointer conversion, which I'm not sure whose to blame for that.

You can see a minimal reproducer at dstufft/rules-foreign-cc-hermetic-reproducer, which you can test doing

$ docker run --rm -v $PWD:/src -it $(docker build -q .) bazel build //...

It appears like the make_build rules do some work to setup an environment prior to building make, however the pkgconfig rules don't appear to do the same work. I tried just copying what the make_build rules did on my branch here.

However doing that just causes another error when attempting to build pkgconfig:

gatomic.c:392:10: error: incompatible integer to pointer conversion passing 'gssize' (aka 'long') to parameter of type 'gpointer' (aka 'void *') [-Wint-conversion]
  return g_atomic_pointer_add ((volatile gpointer *) atomic, val);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./gatomic.h:170:46: note: expanded from macro 'g_atomic_pointer_add'
    (gssize) __sync_fetch_and_add ((atomic), (val));                         \
                                             ^~~~~
gatomic.c:416:10: error: incompatible integer to pointer conversion passing 'gsize' (aka 'unsigned long') to parameter of type 'gpointer' (aka 'void *') [-Wint-conversion]
  return g_atomic_pointer_and ((volatile gpointer *) atomic, val);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./gatomic.h:177:45: note: expanded from macro 'g_atomic_pointer_and'
    (gsize) __sync_fetch_and_and ((atomic), (val));                          \
                                            ^~~~~
gatomic.c:440:10: error: incompatible integer to pointer conversion passing 'gsize' (aka 'unsigned long') to parameter of type 'gpointer' (aka 'void *') [-Wint-conversion]
  return g_atomic_pointer_or ((volatile gpointer *) atomic, val);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./gatomic.h:184:44: note: expanded from macro 'g_atomic_pointer_or'
    (gsize) __sync_fetch_and_or ((atomic), (val));                           \
                                           ^~~~~
gatomic.c:464:10: error: incompatible integer to pointer conversion passing 'gsize' (aka 'unsigned long') to parameter of type 'gpointer' (aka 'void *') [-Wint-conversion]
  return g_atomic_pointer_xor ((volatile gpointer *) atomic, val);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./gatomic.h:191:45: note: expanded from macro 'g_atomic_pointer_xor'
    (gsize) __sync_fetch_and_xor ((atomic), (val));                          \
                                            ^~~~~
4 errors generated.

At this point, I had to quit debugging, I'm not sure if the above errors come from flags that Bazel passes in, from zig (since my reproducer uses zig to get a hermetic CC toolchain), or if pkgconfig in the repo is broken in some way. I figured I'd record this though.

dstufft avatar Jun 27 '23 00:06 dstufft

Helps if I actually include a link to the reproducer.

dstufft avatar Jun 27 '23 00:06 dstufft

Ok, you can ignore the final compiler errors, that seems to be an incompatibility with zig cc and glib, but the lack of hermetic-ness still exists.

dstufft avatar Jun 27 '23 03:06 dstufft

Building pkg-config is a relatively new feature; its not too unsurprising that its not hermetic out of the box. PRs are very welcome to fix up the hermeticity of any of the tools that are built, as indeed are tests around this to ensure this doesn't regress.

jsharpe avatar Jun 27 '23 23:06 jsharpe

The compiler error that you got seems to be related to https://gitlab.freedesktop.org/pkg-config/pkg-config/-/issues/81

lamcw avatar Mar 20 '24 06:03 lamcw

I'm seeing this issue while trying to build Envoy. It looks to me like the issue is that pkg-config has a vendored copy of glib that does not have the fix from https://gitlab.gnome.org/GNOME/glib/-/issues/2842, specifically https://github.com/GNOME/glib/commit/c762d511346d3cb84cea3557a246ccf8873b4a1c. I believe glib 2.79 and newer contain the fix.

def _pkgconfig_tool_impl(ctx):
    make_data = get_make_data(ctx)
    script = [
        "./configure  --with-internal-glib --prefix=$$INSTALLDIR$$",
        "%s" % make_data.path,
        "%s install" % make_data.path,
    ]

Probably needs to stop passing --with-internal-glib and a dep on sufficiently new glib added?

achernya avatar Apr 19 '24 14:04 achernya

I'm seeing this issue while trying to build Envoy. It looks to me like the issue is that pkg-config has a vendored copy of glib that does not have the fix from gitlab.gnome.org/GNOME/glib/-/issues/2842, specifically GNOME/glib@c762d51. I believe glib 2.79 and newer contain the fix.

def _pkgconfig_tool_impl(ctx):
    make_data = get_make_data(ctx)
    script = [
        "./configure  --with-internal-glib --prefix=$$INSTALLDIR$$",
        "%s" % make_data.path,
        "%s install" % make_data.path,
    ]

Probably needs to stop passing --with-internal-glib and a dep on sufficiently new glib added?

PRs for this would be welcome - there's already an example for building glib in examples/third_party/glib but this is 2.75 so would need updating.

jsharpe avatar Apr 21 '24 14:04 jsharpe

I'm seeing this issue while trying to build Envoy. It looks to me like the issue is that pkg-config has a vendored copy of glib that does not have the fix from https://gitlab.gnome.org/GNOME/glib/-/issues/2842, specifically GNOME/glib@c762d51. I believe glib 2.79 and newer contain the fix.

def _pkgconfig_tool_impl(ctx):
    make_data = get_make_data(ctx)
    script = [
        "./configure  --with-internal-glib --prefix=$$INSTALLDIR$$",
        "%s" % make_data.path,
        "%s install" % make_data.path,
    ]

Probably needs to stop passing --with-internal-glib and a dep on sufficiently new glib added?

@achernya have you created an issue in the Envoy project for this?

isaaccarrington avatar Apr 30 '24 09:04 isaaccarrington

I have not, I'll do that a bit later today. I'm not sure there's much Envoy can do other than take an unreleased version of this dependency, though. (I do have such a diff locally)

achernya avatar Apr 30 '24 09:04 achernya