Explicitly enable object Cargo features #1926
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cargo-pgrx only works if we enable a minimal set of Cargo features for for the object crate, that we implicitly enable despite having set object to be `default-features = false`! The Cargo feature resolver has unified our "std" feature for object with the requested object features of our other dependencies that also depend on the object crate. However, these dependencies do so conditionally, based on the platform that they are compiling on. This means that if other dependencies stop using the object crate, or with less features, our enabled Cargo features for object will no longer be the correct set to build our own binary! This also implies some platforms are simply unable to build cargo-pgrx. Even if we don't support these platforms explicitly, we have generally taken a "smoke 'em if you got 'em" approach for unsupported platforms. Thus, enable enough of object's Cargo features to prevent weird problems from surfacing in the future through unrelated dependency changes, and make the cargo-pgrx build effectively platform-invariant. Whether it actually works on those platforms is a "them" problem, but now they should be able to actually get to debugging it a bit.
|
confirmed on Discord by marcus that this builds/runs errors at runtime, but cargo-pgrx reaches runtime |
| default-features = false | ||
| features = [ | ||
| "read_core", "std", # we use object read-only and like having actual errors | ||
| "coff", "elf", "macho", "pe", "xcoff", # support all object formats to allow cross-builds |
There was a problem hiding this comment.
No description provided.
|
I caught the Discord conversation late. I dunno how this was immediately obvious to you but I’m thankful it was! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cargo-pgrx only works if we enable a minimal set of Cargo features for for the object crate, that we implicitly enable despite having set object to be
default-features = false!The Cargo feature resolver has unified our "std" feature for object with the requested object features of our other dependencies that also depend on the object crate. However, these dependencies do so conditionally, based on the platform that they are compiling on. This means that if other dependencies stop using the object crate, or with less features, our enabled Cargo features for object will no longer be the correct set to build our own binary!
This also implies some platforms are simply unable to build cargo-pgrx. Even if we don't support these platforms explicitly, we have generally taken a "smoke 'em if you got 'em" approach for unsupported platforms. Thus, enable enough of object's Cargo features to prevent weird problems from surfacing in the future through unrelated dependency changes, and make the cargo-pgrx build effectively platform-invariant.
Whether it actually works on those platforms is a "them" problem, but now they should be able to actually get to debugging it a bit.