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

Initial support and documentation for cross-compilation #1003

Merged
merged 10 commits into from
Jan 11, 2023

Conversation

thomcc
Copy link
Contributor

@thomcc thomcc commented Jan 11, 2023

The only real thing this does aside from docs and CI (which should work, but obviously has only been tested locally), is avoid calling functions from libc, since that was causing significant glibc versioning problems. I did it in a very hacky way.

Doesn't cover:

  • Anything other than linux x86_64<->aarch64. Other linux <-> linux is probably supported, but I'd keep them as similar as possible.
  • cargo-pgx (see cargo-pgx cross compilation support #981)
  • Builds with pgx/cshim enabled (not actually that hard, just only done it once and probably needs a few changes to the build.rs to make things work more intelligently).
  • Doing this with Debian is fully documented (because it's easy), doing it with anything else is about half documented (well, more than half, but has a lot of handwaving). Concretely, we should probably tell folks to use Debian for now, although if they have a good setup for cross compilation already, this is probably enough to carry things the rest of the way.

Issues

  • The host system's headers are still on the include path when generating bindings, they're behind the ones for the target headers. (note: only an issue on the non-debian path)

    This means if something uses __has_include(<something.h>) to detect support for some header and conditionally include it, and that header is not in $sysroot_dir/usr/include and is in /usr/include and do something conditionally, then it will be included and the logic

    That said, this probably isn't happening -- it takes a fairly specific pattern to happen, and even if it does happen it seems unlikely to impact the generated bindings.

  • This uses the headers from postgres configured on an x86_64 machine when compiling postgres. It's unclear to me if to what extent headers themselves vary, e.g. does the code have #ifdef __aarch64__ or #ifdef CONFIG_AARCH64 where CONFIG_AARCH64 comes from a config.h that gets generated by a configure script.

    AFAICT the bindings are essentially the same when cross compiling, so I think it's fine. If it's not, on Debian you'd do something like (if compiling to aarch64)

     sudo dpkg --add-architecture arm64
     sudo apt-get update
     sudo apt-get install postgresql-server-dev:arm64

    Which would install the headers (among other things), and then we'd pass these headers to bindgen. This is annoying, because we can't use the arm64 postgres installation's pg_config when on x86_64, since it's not the right executable for the architecture, but guessing at the header path is probably not that bad.

    But yeah, this didn't seem to be an issue in reality, so I didn't think about how to solve it.

thomcc added 10 commits January 10, 2023 17:00
These are still experimental and undocumented.

- Ensure we rerun pgx-pg-sys's build script when env vars used by
  dependencies (bindgen, pgx-pg-config) change.

- Allow overriding the directory to use as the `--includedir-server`.
    If this directory is overridden, and `cshim` generation is disabled,
    we will not call into the configured `pg_config` script (although
    it).

    This may be done on a per-target and per-pg-major-version basis by
    setting environment variables (which are checked in this order of
    preference):
    - `PGX_INCLUDEDIR_SERVER_PG<ver>_<target>`
    - `PGX_INCLUDEDIR_SERVER_PG<ver>`
    - `PGX_INCLUDEDIR_SERVER_<target>`
    - `PGX_INCLUDEDIR_SERVER`

- Allows disabling bindgen's (well, libclang's) auto-detection of
  include directories. Note that using this will require providing
  bindgen with a good amount of information in other environment
  variables.

    This may be done via the `PGX_BINDGEN_NO_DETECT_INCLUDES_<target>`
    or `PGX_BINDGEN_NO_DETECT_INCLUDES` environment vars (which are
    checked in that order of preference)
@thomcc
Copy link
Contributor Author

thomcc commented Jan 11, 2023

From @eeeebbbbrrrr in discord, it seems like we may also want ways to configure the direct path to a configured server include dir. I also suspect we'll need ways to completely override the include path for sysroot cases on fedora and the like.

To support these I've some configuration vars to the pgx-pg-sys build script (These aren't yet documented and are only smoke-tested).

  • PGX_INCLUDEDIR_SERVER{,_PG<ver>}{,_<target>} can be used to specifies the path to already-configured server include directory (optionally on a per-pgversion and per-target basis -- version takes precedent over target if only one is specified).

    If provided (and not building the cshim), we don't call into the local pg_config (since it may point at one for another architecture). Intended mostly to be used in combination with PGX_PG_CONFIG_PATH, to prevent needing a .pgx/config.toml for cross compiled postgres.

  • PGX_BINDGEN_NO_DETECT_INCLUDES{,_<target>} allows disabling bindgen/clang's autodetection of system include paths. This is only useful when also passing --sysroot to a fully configured sysroot (and many other flags) into clang/bindgen somehow (e.g. probably BINDGEN_EXTRA_CLANG_ARGS{,_<target>}). This is mostly in there because I know it will be needed for that case, and there's a good chance we won't need other pgx-pg-sys code changes along with it.

I've also ensured that when you change the various vars that bindgen/libclang/pgx-pg-config understand, we actually rebuild pgx-pg-sys, since this doesn't happen by default, and it was very annoying.

Copy link
Contributor

@eeeebbbbrrrr eeeebbbbrrrr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chuckled at sym_blocklist.rs. This looks good tho. Feel free to merge

@thomcc thomcc changed the title Add cross compilation code changes, initial cross compile docs, and (hopefully?) a cross compile smoke-check to CI Initial support and documentation for cross-compilation Jan 11, 2023
@thomcc thomcc merged commit c6f2c9a into develop Jan 11, 2023
@thomcc thomcc deleted the thomcc/clean-cross branch January 11, 2023 21:24
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

Successfully merging this pull request may close these issues.

2 participants