-
Notifications
You must be signed in to change notification settings - Fork 77
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
Added module for abi constraint values #39
Conversation
abi/BUILD
Outdated
) | ||
|
||
constraint_value( | ||
name = "linuxkernel", |
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 doesn't seem like it belongs? Kernel code can be built with various other options from this list, there's nothing particularly special about object files built for the kernel.
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.
Fair. This was just a value listed in https://doc.rust-lang.org/nightly/rustc/platform-support.html#tier-3 so it was included in what I added. As per most of my comments on this PR, I'm happy to remove this and have a more focused conversation on this.
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 don't think using Rust, or Go, or linux kernel builds, or whatever as the pattern to emulate wins.
Was there a design proposal for this? It is much easier to discuss in a doc than in PR review.
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.
Aside from this there's just the github issue (#38). I tried emailing //bazel-dev@googlegroups.com per the README but the address no longer seems valid.
gnueabihf commonly means all of these, of the ones written:
Some of these might be meant as opposites of each other, but that's unclear. Even if it was written in a comment, I suspect people will end up picking the wrong ones via copy/paste a lot. Also, AFAIK a platform can only have a single of these constraint_values, which doesn't seem like it works very well for these. I don't have a good proposal here, but I think this will be confusing and worse than the status quo. |
@bsilver8192 thanks so much for taking a look! The values here were all taken from platform triples listed on https://doc.rust-lang.org/nightly/rustc/platform-support.html While it'd be nice to represent everything there, I'd be fine with reducing this PR to common and less controversial values. Maybe just the following for starters and have deeper conversations about more unique values?:
|
What semantics are you proposing for each one? Those strings in the ABI part of a triple mean different things depending on the rest of the triple, so I don't think we can define them that way. Leaving it up to Bazel users to come up with their own meanings seems counterproductive, because you end up with conflicts when trying to use rule ecosystems together. Also, there are lots of places that use "target triples" with subtly different, conflicting meanings and sets of choices for each component, for example:
|
That's a great question. I personally don't have a super strong opinion, for Rust I think |
This has lingered too long. |
This pull-requests adds constraint_value flags for common abi flags. Defining these in the
@platforms
repository avoids other rules from needing to define custom constraints and/or avoids each repository defining project specific constraints.closes #38