Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign uprustc: add a `--print targets` command #31358
Conversation
rust-highfive
assigned
alexcrichton
Feb 2, 2016
alexcrichton
reviewed
Feb 2, 2016
| "x86_64-unknown-linux-musl", | ||
| "x86_64-unknown-netbsd", | ||
| "x86_64-unknown-openbsd", | ||
| ]; |
This comment has been minimized.
This comment has been minimized.
alexcrichton
Feb 2, 2016
Member
Would it be possible to not duplicate this list with the one below? Could the macro somehow use something like stringify! to build a vector, or could an iterator macro be used?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
nodakai
Feb 2, 2016
Contributor
I think "x86_64-w64-mingw32" is an alias for "x86_64-pc-windows-gnu" and should be included in the list as a valid target.
This comment has been minimized.
This comment has been minimized.
japaric
Feb 2, 2016
Author
Member
Would it be possible to not duplicate this list with the one below? Could the macro somehow use something like stringify! to build a vector, or could an iterator macro be used?
As @nodakai said, the mixture of hyphens and underscores makes it tricky.
I think "x86_64-w64-mingw32" is an alias for "x86_64-pc-windows-gnu" and should be included in the list as a valid target.
That's what the logic of this function seems to indicate. But, rustc --target=x86_64-w64-mingw32 ... returns error: Error loading target specification: Could not find specification for target "x86_64-w64-mingw32".
This comment has been minimized.
This comment has been minimized.
brson
Feb 2, 2016
Contributor
I think "x86_64-w64-mingw32" is an alias for "x86_64-pc-windows-gnu" and should be included in the list as a valid target.
The former is deprecated and I don't think we should be promoting it.
This comment has been minimized.
This comment has been minimized.
alexcrichton
Feb 2, 2016
Member
I agree with @brson that we shouldn't be handling aliases, and this also probably isn't the right place to discuss it regardless (e.g. an issue would be more appropriate).
@japaric sure it's trick but that doesn't mean it's not possible! I'd be personally much happier with something like:
load! {
(x86_64_pc_windows_gnu, "x86_64-pc-windows-gnu"),
// ...
}It's basically just much easier if there doesn't need to exist a comment of "modify this other location as well" as they will invariably get out of sync.
This comment has been minimized.
This comment has been minimized.
japaric
Feb 2, 2016
Author
Member
sure it's trick but that doesn't mean it's not possible!
Well, I was thinking about a macro that didn't need to specify the triple twice in different formats (hyphenated and underscored).
I'd be personally much happier with something like:
Yeah, that looks less prone to errors. I'll look into it.
This comment has been minimized.
This comment has been minimized.
|
This can in theory have some form of interaction with custom target specifications where we probe for I'm pretty ok with the option here, though. We may want to rename |
This comment has been minimized.
This comment has been minimized.
|
cc @rust-lang/tools |
This comment has been minimized.
This comment has been minimized.
Wait RUST_TARGET_PATH is a thing? TIL
Yeah I don't think it should. The other --print commands don't do that kind of probing. Also it seems the command would have to look at all the *.json files and check that they are valid target specification files.
Happy to rename |
This comment has been minimized.
This comment has been minimized.
That's ok, if you unlearn it right now I'd be fine with that. |
alexcrichton
added
the
T-tools
label
Feb 2, 2016
This comment has been minimized.
This comment has been minimized.
|
This should probably also print the target passed in to rust through |
This comment has been minimized.
This comment has been minimized.
|
Pushed two commits. The first one generates the TARGETS constant from the existing macro as @alexcrichton suggested. The second commit makes the rmake test simpler.
This seems doable and makes sense, but I wasn't expecting it to come up in practice. @alexcrichton thoughts? |
japaric
reviewed
Feb 3, 2016
| return Some(t); | ||
| } | ||
| )* | ||
| else if target == "x86_64-w64-mingw32" { |
This comment has been minimized.
This comment has been minimized.
japaric
Feb 3, 2016
Author
Member
While copy pasting this code, I noticed these last two else-if branches are effectively unreachable (because of the let target = target.replace("-", "_"); a few lines above).
Not sure if I should fix them or just remove them. Probably the latter because @brson said these targets are deprecated.
This comment has been minimized.
This comment has been minimized.
alexcrichton
Feb 11, 2016
Member
Ah for now I'd just leave out these targets entirely, we've generally tried to remove them as much as possible
This comment has been minimized.
This comment has been minimized.
|
Can we have |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
Can there be |
This comment has been minimized.
This comment has been minimized.
That's what I proposed on IRC because of the |
This comment has been minimized.
This comment has been minimized.
|
Alternatively, error message for unknown/malformed target like |
This comment has been minimized.
This comment has been minimized.
|
Yeah right now |
This comment has been minimized.
This comment has been minimized.
|
Ok, the tools team had a chance to talk about this today during triage, and we all agreed that this is good to add, but none of us felt too too strongly about the name. Some possibilities were:
I think that I may lean a bit in favor of |
This comment has been minimized.
This comment has been minimized.
|
I like the sound of |
This comment has been minimized.
This comment has been minimized.
|
Ah I was just thinking in terms of there being a difference between targets we can generate code for and targets we can generate things like rlibs/binaries for. The latter case would require a standard library and/or libcore, but as I write this down it sounds weird. Anyway |
japaric
force-pushed the
japaric:print-targets
branch
from
925c05b
to
0bb4209
Feb 12, 2016
This comment has been minimized.
This comment has been minimized.
|
removed the deprecated mingw aliases, renamed print targets to print target-list, rebased and squashed. r? @alexcrichton |
japaric commentedFeb 2, 2016
that prints a list of all the triples supported by the
--targetflagr? @alexcrichton