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

Goal: Limited, configurable cross-compilation #955

Open
1 of 3 tasks
workingjubilee opened this issue Dec 12, 2022 · 10 comments
Open
1 of 3 tasks

Goal: Limited, configurable cross-compilation #955

workingjubilee opened this issue Dec 12, 2022 · 10 comments
Labels
cargo-pgrx enhancement New feature or request rust+sql Interop between Rust and SQL

Comments

@workingjubilee
Copy link
Member

workingjubilee commented Dec 12, 2022

@jaskij and @thomcc probably have other thoughts, comments, requirements, or requests.

@workingjubilee workingjubilee added enhancement New feature or request rust+sql Interop between Rust and SQL cargo-pgrx labels Dec 12, 2022
@workingjubilee
Copy link
Member Author

This will effectively impose an API requirement that we essentially not need to use the C shim for anything that PGX relies on at the "basic level". It's fine to use it for e.g. a BackgroundWorker, but this will mean there are some shenanigans, like MemoryContext, that it can't be a requirement in.

@jaskij
Copy link
Contributor

jaskij commented Dec 12, 2022

First of all, sadly, the way schema is generated right now means we have to build the extension twice - both for host and for target. Or rather, run codegen and linker twice - I'm not overly familiar with Rust's build process, but some parts are reused when compiling within the same tree for multiple targets.

Lack of cshim would be a great help, since it's generating quite some pain. But it being there is actually not a deal breaker from what I'm seeing - it just needs two more flags (or environmental variable, but those prove to be problematic in the long run). What I'd need to support it being there cross-compilatiom is two more flags, for sysroots - for host and target.

I hope to have working 0.5.4 soon, but it will require a large cleanup and rebasing on top of devel, so an actual PR won't happen in the next week or two.

Of course, there's the big ticket item - pg_config itself, but this is supplied by whoever's mad enough to cross compile PostgreSQL extensions.

On second thought:

We probably won't be able to run away from environment variables - there's just too many things to configure, and traditionally this kind of thing was supplied through them. Just off the top of my head:

  • C compiler
  • C compile flags
  • linker
  • host C compiler
  • host C compiler flags
  • host linker
  • target sysroot
  • host sysroot, not usually used but we need it because of schema generation and cshim

@jaskij
Copy link
Contributor

jaskij commented Dec 13, 2022

Another thing - because schema generation needs both native and cross builds, we need postgresql twice, so we can link both against native and target. Which means two pg_config paths.

@workingjubilee
Copy link
Member Author

I'm not bothered by the notion of setting environment variables, I just prefer that it be done via CLI arguments or configuration files where possible, so as to let us see what you are setting with maximal ease. Using an explicit contract boundary like that to set environment variables

  • lets us know what setting the environment variable should achieve
  • creates an API promise that we try to achieve that thing
  • allows us to clobber the environment variable to do it
  • requires that we clobber it on your behalf, instead of setting it for pure whimsy
  • allows more easily rationalizing thorny edge-cases (e.g. "these two seemingly-orthogonal flags are incompatible because they actually both require setting the same env var, let's error and tell the user they can't combine both")

Environment variables work best for things that are true about the environment (and thus are not going to be randomly changed, e.g. during a build), for things everyone knows to use an "append, not clobber" mode for (e.g. CFLAGS), or for janky quick-fixes (which I have no shame about using them for).

@jaskij
Copy link
Contributor

jaskij commented Dec 13, 2022

So far, in my experimental branch, I have taken the approach to specify the target (because it's for Cargo, not C) and the sysroots as arguments, while the rest is passed as environment variables - those are well known and have been used for a long time.

The only little snag is terminology: whether to use the prefix BUILD_ or HOST_, there's some historical context to this, and arguments for both. As Rust generally only uses host and target, without using build, for now I'm using the HOST_ prefix. Although this might come back at some point.

@workingjubilee
Copy link
Member Author

Yes, the "traditional" ones are well-understood and, in many cases, have an actual rulebook as to how you should handle them. One you can print out and eat and everything.

@thomcc
Copy link
Contributor

thomcc commented Dec 13, 2022

Yeah, where possible we should use HOST_ (this is because I'm working on excising the makefile in favor of cc-rs, which uses HOST_, as described here).

@jaskij
Copy link
Contributor

jaskij commented Dec 13, 2022

@thomcc a little word of caution: don't assume that CC contains the path to the compiler and nothing else - I've seen frameworks thrown in compilation flags in that variable.

@thomcc
Copy link
Contributor

thomcc commented Dec 13, 2022

I'm well aware.

@jaskij
Copy link
Contributor

jaskij commented Feb 3, 2023

My end-goal here is to have cargo-pgx be usable under Yocto - a very popular embedded Linux framework. To that end, apart from #981 , I need to add two more things which aren't directly related to cross-compilation:

  • fix out-of-tree builds (they were broken as of 0.5.1, haven't checked on a newer version yet)
  • add an --offline flag - Yocto separates fetching and building into two separate steps to easier enforce reproducibility constraints, and the build step should not use network (this is enforced unless overriden)

I am not sure if there should be included in this issue, or separate issues on their own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cargo-pgrx enhancement New feature or request rust+sql Interop between Rust and SQL
Projects
None yet
Development

No branches or pull requests

3 participants