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

Robust bindgen support #140

Merged
merged 11 commits into from
Jun 17, 2023
Merged

Robust bindgen support #140

merged 11 commits into from
Jun 17, 2023

Commits on Jun 17, 2023

  1. Use zig cc -v to determine options to pass through bindgen.

    The previous approach to use `--sysroot` was pretty much incomplete,
    as zig uses multiple include directories to reduce the bundle size.
    This commit implements a future-proof strategy to determine correct
    compile options by using `zig cc` itself, which is also what bindgen
    (transitively via clang-sys) does internally.
    
    Also fixes a couple of minor issues:
    
    - Handles all three kinds of `BINDGEN_EXTRA_CLANG_ARGS` environment
      variable correctly (one of them was missing previously).
    
    - Existing `BINDGEN_EXTRA_CLANG_ARGS` no longer blocks bindgen support.
      Collected options are instead appended to them.
    lifthrasiir committed Jun 17, 2023
    Configuration menu
    Copy the full SHA
    927205b View commit details
    Browse the repository at this point in the history
  2. Add explicit bindgen tests.

    Zstd-rs didn't include enough headers and the incomplete bindgen support
    was simply overlooked. For example, `stdlib.h` didn't work.
    lifthrasiir committed Jun 17, 2023
    Configuration menu
    Copy the full SHA
    60ae2eb View commit details
    Browse the repository at this point in the history
  3. Only override BINDGEN_EXTRA_CLANG_ARGS_$TARGET.

    Bindgen should have accepted a correct TARGET environment variable,
    so it makes sense to set only target-specific environment variables.
    
    This also fixes a small bug when only the pan-target variable is set,
    in which case the newly set target-specific variable should first
    inherit the pan-target value and then more options should have been
    appended, but the pan-target value was discarded previously.
    lifthrasiir committed Jun 17, 2023
    Configuration menu
    Copy the full SHA
    d9a131f View commit details
    Browse the repository at this point in the history
  4. Avoid using -x to determine zig cc options.

    Only the master version of Zig recognizes it, all existing releases
    don't and adds proper include directories only when a file with
    a corresponding extension is added.
    
    Note: this will write additional empty files to the cache directory,
    which should be writable (otherwise everything else will fail anyway).
    Since the cache directory may end up being cwd, this commit tried to use
    a specific enough file name to avoid collision.
    lifthrasiir committed Jun 17, 2023
    Configuration menu
    Copy the full SHA
    0e94268 View commit details
    Browse the repository at this point in the history
  5. Collect a __GLIBC_MINOR__ definition from zig cc if any.

    This is specific to Zig because while Zig ships with the recent enough
    glibc it dynamically sets `__GLIBC_MINOR__` according to the target.
    
    At this point it seems more robust to collect *all* options from `zig
    cc` output, but this is not as viable because all we can see is cc1
    options (`-###` or `-v`), not clang driver options from Zig. It seems
    that we have no direct means to collect the latter, so we have to be
    conservative about which options to collect.
    lifthrasiir committed Jun 17, 2023
    Configuration menu
    Copy the full SHA
    b78b535 View commit details
    Browse the repository at this point in the history
  6. Improve compatibility with older clang.

    It turned out that unused argument warnings (`-Wunused-command-line-
    argument`) come from cc1, not from driver, and as long as all options
    are "claimed"---not necessarily used---it doesn't issue warnings.
    
    On the other hands, Zig internally adds `-nostdinc` but we didn't
    because we wanted header-only C++ libraries in the host to be usable,
    but it turned out that the presence of `__has_include` made this
    horribly fragile (e.g. some versions of libc++ used `__has_include(
    <pthread.h>)` to determine pthread support), so we gave up. If this
    facility is really desired, consider using `-idirafter /usr/include`.
    lifthrasiir committed Jun 17, 2023
    Configuration menu
    Copy the full SHA
    1ca3f82 View commit details
    Browse the repository at this point in the history
  7. Use -cxx-isystem instead of -stdlib++-isystem.

    The latter is also a recent addition, and `-nostdinc` already implies
    `nostdinc++` so we no longer need guarantees from `-stdlib++-isystem`.
    (We can't still replace duplicate `-c-isystem` and `-cxx-isystem` into
    a single `-isystem` due to the relocation.)
    lifthrasiir committed Jun 17, 2023
    Configuration menu
    Copy the full SHA
    43c8ca9 View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    ef51fe2 View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    81f13a7 View commit details
    Browse the repository at this point in the history
  10. Configuration menu
    Copy the full SHA
    84436d6 View commit details
    Browse the repository at this point in the history
  11. Explicitly test stray include paths from the host.

    This in particular affects `__has_include` new in C++17, which is also
    how the test internally works. Tested files are chosen so that there is
    no chance that other OSes have matching header files by accident.
    lifthrasiir committed Jun 17, 2023
    Configuration menu
    Copy the full SHA
    998830e View commit details
    Browse the repository at this point in the history