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

Adds the filter_target flag when gathering cargo metadata. #395

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

amzn-aeline
Copy link

Unless building for all targets is specified, --filter_target will be added to the cargo command.
The target is chosen as follows:

  1. If the user specified a target, that target is used
  2. Otherwise, attempt to invoke rustc and determine the default target
  3. If all else fails, build all targets.

Similar to: rust-lang/rust-analyzer#6912

@jonhoo
Copy link

jonhoo commented Oct 14, 2022

For some more context on this kind of change, see rust-lang/rust-analyzer#6908.

Also, just for clarity, the Cargo flag here is --filter-platform, not --filter_target :)

Comment on lines +120 to +123
None
} else {
None
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably just do:

Suggested change
None
} else {
None
}
}
None

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, is it worth maybe giving a warning or something if this fails?

@pinkforest
Copy link
Collaborator

pinkforest commented Oct 15, 2022

Thanks amzn-aeline for the contribution and thanks for the review jonhoo

And thanks for the context - so the --filter-platform was added in 1.38 ?

@jonhoo
Copy link

jonhoo commented Oct 15, 2022

Yes, looks like it. Though since we're reading rustc -v already, we could perhaps return None on 1.37 and below as well?

@pinkforest
Copy link
Collaborator

pinkforest commented Oct 15, 2022

Would be lovely if there is a crate to invoke / interrogate rustc metadata sans Command and duplicating the code here ?

In the long run would be happy to get rid of all Command's and use some another crate for it that handles all the version diff's

In 1.37 the behaviour is going to be different even if None ? Not too concerned about old rustc versions but just curiosity wise.

Cheers

@jonhoo
Copy link

jonhoo commented Oct 15, 2022

I don't know of any unfortunately. Should be easy enough though. Maybe it could even be added as part of cargo_metadata?

I don't know what cargo metadata did by default before --filter-platform was added. I suspect it just always grabbed for all targets, which is the same thing None will do here, so I think that'd be okay.

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.

None yet

3 participants