Build native_binary/test src in correct configuration #341
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
|
@c-parsons could you PTAL |
Ah apologies I think this ping (and similar on my other PRs) was perhaps misdirected. You're identified as the best reviewer internally, but I think that's just because you imported changes from others. I would still appreciate review (or at least acknowledgement) from someone on these. These issues are blocking other things I'm working on |
This attribute is incorrectly being built in the host configuration when (like any test) it will run in the target configuration. This means that cross compilation will be broken and options that differ between host and target (e.g. `NDEBUG`) will not be as set by the user. I confirmed that without this fix, a test binary with `assert(false)` passes when run under `native_test`. Additionally, the use of `allow_single_file` precludes rules that return multiple files in their DefaultInfo (like `py_binary`). Instead, we can use `allow_files` and access via `ctx.executable`.
GMNGeoffrey
force-pushed
the
allow-files
branch
from
5571c5b
to
105c7f5
Compare
GMNGeoffrey
changed the title
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.
This attribute is incorrectly being built in the host configuration when
(like any test) it will run in the target configuration. This means that
cross compilation will be broken and options that differ between host
and target (e.g.
NDEBUG) will not be as set by the user.I confirmed that without this fix, a test binary with
assert(false)passes when run under
native_test.Additionally, the use of
allow_single_fileprecludes rules that returnmultiple files in their DefaultInfo (like
py_binary). Instead, we canuse
allow_filesand access viactx.executable.