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

Fix features selection, broken by #478 #524

Merged
merged 6 commits into from
Nov 25, 2022
Merged

Conversation

bsilver8192
Copy link
Contributor

It works properly with no explicit targets set (aka "use all the
targets"), and it also works properly with some of the examples that set
default-members = [].

I think there's still something wrong, because it dropped features from
some Windows-specific crates relative to before #478, but I'm not sure
what's supposed to happen. At least all the tests in this repository
pass on Linux again...

Currently many things fail to build due to google#478 stripping out all the
features.
It works properly with no explicit targets set (aka "use all the
targets"), and it also works properly with some of the examples that set
`default-members = []`.

I think there's still something wrong, because it dropped features from
some Windows-specific crates relative to before google#478, but I'm not sure
what's supposed to happen. At least all the tests in this repository
pass on Linux again...
@bsilver8192
Copy link
Contributor Author

@sayrer FYI, if you want to take a look.

It doesn't work any more, now that I changed the code to only use
supported targets.
bsilver8192 added a commit to bsilver8192/cargo-raze that referenced this pull request Sep 16, 2022
This is how I work around rust-lang/cargo#5042
to get path dependencies into vendored Rust dependencies.

The generated BUILD files are built with
google#524, because otherwise they
don't work.

This fixes google#361.
dae added a commit to ankitects/anki that referenced this pull request Sep 25, 2022
Latest rules_rust now requires them. Can't use the 0.16 release
due to not-yet-merged google/cargo-raze#524
@rdelfin
Copy link

rdelfin commented Oct 14, 2022

I believe this fixes the issue outlined in #530. Thanks a bunch for working on this and fixing it!

@UebelAndre UebelAndre removed their request for review October 14, 2022 17:35
@rdelfin
Copy link

rdelfin commented Oct 14, 2022

@dfreese is there any chance this change could be included in a patch/minor release? If this fixes the issue, it would be a huge unblocker for our repo

@PiotrSikora
Copy link
Contributor

@dfreese @illicitonion @acmcarther could one of you review this and release v0.16.1? cargo-raze v0.16.0 is pretty much broken without this fix (see: #478 (comment)), and I verified that this PR fixes the issue for me.

@illicitonion
Copy link
Collaborator

@sayrer Please could you give this a look?

@dfreese dfreese merged commit 38f33c8 into google:main Nov 25, 2022
@dfreese
Copy link
Collaborator

dfreese commented Nov 25, 2022

published 0.16.1

@bsilver8192 bsilver8192 deleted the fix-features branch November 28, 2022 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants