-
-
Notifications
You must be signed in to change notification settings - Fork 249
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
cargo-pgx cross compilation support #981
base: develop
Are you sure you want to change the base?
Conversation
Now, the questions is, how do I test it? Since |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need some work (well, it's a draft, so I think you know that), but is a really good start. Here's some preliminary feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, two super minor idiom nits too.
@thomcc I primarily marked this as a draft to discuss the interface - mostly around what is passed as an argument, and what is passed as an environmental variable. Meanwhile, I'm going to document some caveats here:
|
…t from environment
I have absolutely no clue why that one test is failing, but it seems to be misconfigured, especiall since on my PC (with PG15 installed via
|
A quick note on how to actually cross-compile with
#!/usr/bin/env sh
set -x
SDK_DIR="/usr/local/oecore-x86_64"
SYSROOT_DIR="$SDK_DIR/sysroots"
OE_HOST="x86_64-poky-linux"
OE_TARGET="cortexa53-crypto-poky-linux"
HOST_SYSROOT="$SYSROOT_DIR/$OE_HOST"
TARGET_SYSROOT="$SYSROOT_DIR/$OE_TARGET"
$HOST_SYSROOT/usr/bin/qemu-aarch64 -L $TARGET_SYSROOT $TARGET_SYSROOT/usr/bin/pg_config "$@" |
As is, schema generation builds the extension for the host so that it can generate the bindings. The current solution has two, huge and obvious, drawbacks:
I'm not sure if it's in-scope for this MR though, and I would need a lot of guidance to implement it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think running the schema generation on the host is reasonable. It's not ideal, surely, but it seems unlikely that SQL will vary between targets (this is also something we can document).
Overall this mostly looks good, but I'm testing it out today and that might dig something up.
I think it needs some docs, but those don't have to block things, since this is a pretty niche use case (and I can handle them, when I find the time).
Do you mind resolving the conflicts?
@thomcc I see that your work on cross compilation was merged, that's what created the conflict. Do you think there is something I should be looking into or integrating with? Or can I just simply resolve the conflict and be done with it? |
You don't need to integrate with it really. There's some redundancy, but it largely handles a few cases that this completes (specifically the no cshim case where you don't care about cargo-pgx — it's basically for https://github.com/tcdi/plrust). Resolving the conflict is sufficient, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. It will need some further documentation, but I'm working on an update/completion to CROSS_COMPILE.md anyway, and I can integrate it afterwards.
@thomcc conflict resolved. If you're working on update to A cleaned up version of #!/usr/bin/env sh
set -x
TARGET_SYSROOT="/usr/local/oecore-x86_64/sysroots/cortexa53-crypto-poky-linux"
qemu-aarch64 -L $TARGET_SYSROOT $TARGET_SYSROOT/usr/bin/pg_config "$@" |
Can you rustfmt and push an update? |
Damnit, thought it was already formatted. Will do as soon as I'm at my PC. |
@thomcc done. |
Hmm, can you check the CI error? I didn't see this locally when I tested, but I think I was testing on pg14 rather than 15. |
Putting this PR on "pause" until we have some extra cycles over here in pgxland to make sure this doesn't actually break anything and is otherwise complete enough to accomplish the stated goal. |
If this gets rebased I think we can take a look at landing it? |
I don't have the time or energy to get on this now, but might have it at the beginning of September, feel free to ping me then if I don't show up. |
Work in progress for adding cross compilation support to
cargo-pgx
, as discussed in #955