-
-
Notifications
You must be signed in to change notification settings - Fork 672
Implement -probe for showing supported features #7521
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
Conversation
|
Thanks for your pull request, @wilzbach! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
|
(CC @s-ludwig who is using this in DUB) |
src/dmd/probing.d
Outdated
|
|
||
| private void determinePlatform(void delegate(string) print) | ||
| { | ||
| version(Windows) print("windows"); |
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 is wrong. Don't you want to print target information, not host?
src/dmd/probing.d
Outdated
|
|
||
| private void determineArchitecture(void delegate(string) print) | ||
| { | ||
| version(X86) print("x86"); |
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.
Likewise here.
src/dmd/probing.d
Outdated
| * License: $(LINK2 http://www.boost.org/LICENSE_1_0.txt, Boost License 1.0) | ||
| * Source: $(LINK2 https://github.com/dlang/dmd/blob/master/src/dmd/mars.d, _mars.d) | ||
| * Documentation: https://dlang.org/phobos/dmd_mars.html | ||
| * Coverage: https://codecov.io/gh/dlang/dmd/src/master/src/dmd/mars.d |
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 documentation header seems to be cut and pasted from mars.d, but needs to be updated for this file.
src/dmd/probing.d
Outdated
| else version(GNU) return "gdc"; | ||
| else version(LDC) return "ldc"; | ||
| else version(SDC) return "sdc"; | ||
| else return null; |
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.
That also looks wrong, since it gives the name of the compiler used to compile this file.
src/dmd/probing.d
Outdated
| version(SH64) print("sh64"); | ||
| version(Alpha) print("alpha"); | ||
| version(Alpha_SoftFP) print("alpha_softfp"); | ||
| version(Alpha_HardFP) print("alpha_hardfp"); |
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.
Then leave it out ? No point in faking support, especially given those targets are more likely to be subject to cross compilation.
|
Wouldn't slightly a more generic functionailty make more sense, where simply all predefined versions are dumped to stdout? |
They are with Lines 1428 to 1437 in a6515d1
|
aa118b4 to
0900e80
Compare
It doesn't do this anymore: #7549
Good idea (done), but I think you will still need things like |
True, would be good to avoid another process invocation for that ( |
|
@s-ludwig so |
|
how about a more flexible way to output exactly what we want:
NOTE: -v can't change too much unfortunately for backward compatibility reasons, but -probe could be well designed since we're starting from scratch and future proof. NOTE: precedent for this: |
|
graphql for dmd :) |
|
could -Xf (json output) used instead of introducing another -probe flag? (cf #7757) |
I like the idea, but isn't this an overkill? The JSON file is really tiny at the moment (see the first comment) and it's unlikely that it will get bigger
I don't see why - if this advanced selection was ever needed - it couldn't be added to
Problems I see
We could rename |
|
I like Timothee's idea from here, where |
|
the link u gave is wrong, I think you meant dlang/tools#292 (comment) :) |
|
Alternative to |
|
Superseded by #7894 |
DUB creates a special platform probe file on every invocation, (more details).
(
determinePlatformis called bysetupPackageinexecute)While DUB should cache this invocation, I think there should be a "standard" format for feature detection instead of using this hack to generate a D file with
versionidentifiers.This PR generates the same output as the generated DUB probing file, e.g.:
{ "compiler": "dmd", "frontendVersion": 2078, "compilerFrontend": "v2.078.0-beta.1-113-g057aa4442", "config": null, "binary": "/home/seb/dlang/dmd/generated/linux/release/64/dmd", "platform": [ "posix", "linux" ], "architecture": [ "x86_64" ], "predefinedVersions": [ "DigitalMars", "Posix", "linux", "ELFv1", "LittleEndian", "D_Version2", "all", "D_SIMD", "D_InlineAsm_X86_64", "X86_64", "CRuntime_Glibc", "D_LP64", "D_PIC", "assert", "D_HardFloat" ] }(I didn't add this flag to the documentation, s.t. it can stay an experiment)