[RFC] build.d: Use JSON output to detect host compiler #11639

MoonlightSentinel · 2020-08-28T12:10:29Z

Less brittle than parsing the output of --version
(especially to determine the frontend version)

Idea inspired by #11638 which requires compiling with -ignore for older host compilers.

dlang-bot · 2020-08-28T12:10:30Z

Thanks for your pull request and interest in making D better, @MoonlightSentinel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#11639"

wilzbach · 2020-08-28T12:35:15Z

Why not use -Xi=compilerInfo -Xf=-?
It was introduced exactly to make such queries cheaper.

Example: https://run.dlang.io/is/rmiPI1

It has been introduced in 2.079.0 (https://dlang.org/changelog/2.079.0.html#json_includes).

edit: GDC doesn't implement it (yet), but you can just fallback to the most conservative settings for gdc.

MoonlightSentinel · 2020-08-28T14:35:25Z

Didn't think about the JSON output.

But 2.079.0 doesn't list the vendor (e.g. from the auto-tester):

{
    "compilerInfo" : {
         "binary" : "/home/braddr/sandbox/at-client/release-build/dmd-2.079.0/linux/bin32/dmd",
         "version" : "v2.079.0",
         "supportsIncludeImports" : true
    }
}

So this would require executing HOST_DMD twice.

wilzbach · 2020-08-28T14:58:26Z

But 2.079.0 doesn't list the vendor (e.g. from the auto-tester):

Yeah, it looks like this was only added in 2.080:

2.079.1 Success with output:

{
 "compilerInfo" : {
  "binary" : "dmd",
  "version" : "v2.079.1",
  "supportsIncludeImports" : true
 }
}

2.080.1 Success with output:

{
 "compilerInfo" : {
  "vendor" : "Digital Mars D",
  "version" : "v2.080.1",
  "__VERSION__" : 2080,
  "interface" : "dmd",
  "size_t" : 8,
  "platforms" : [
   "posix",
   "linux"
  ],
  "architectures" : [
   "x86_64"
  ],
...
}

However, can't we simply fallback to DMD?
It's just that one version and we only officially support building with the latest version of LDC as a host compiler (two for GDC). The requirement of 2.079 only applies to DMD.

Another alternative would be to look at the binary path like before:

ldc2 -Xi=compilerInfo -Xf=-
{
 "compilerInfo" : {
  "binary" : "/home/seb/dlang/ldc-1.9.0/bin/ldc2",
  "version" : "v2.079.1",
  "supportsIncludeImports" : true
 }
}

(though we would need to check for ldc and ldmd).

So this would require executing HOST_DMD twice.

A more complex alternative would be to store the output of these two host compiler invocations in generated like generated/compilerInfo/<compiler-path> and store all the required information in there as a JSON, but with the hash of the host compiler binary (the compiler-path needs to be part of the file name, s.t. this invocation doesn't happen again after changing host compilers back and forth).

Anyhow, I think falling back to dmd as a vendor is the easiest and should work well.

wilzbach · 2020-08-28T14:59:18Z

src/build.d

 env["HOST_DMD_KIND"] = "gdc";
+ env["HOST_DMD_VERSION"] = "v2.079";


Technically GDC only ships a Phobos/Druntime of version 2.076.

MoonlightSentinel · 2020-08-28T15:35:56Z

However, can't we simply fallback to DMD?
It's just that one version and we only officially support building with the latest version of LDC as a host compiler (two for GDC). The requirement of 2.079 only applies to DMD.

Good to know, thought that applied to all compilers with a sufficient frontend version.

Another alternative would be to look at the binary path like before:

Implemented a small experiment which should work.

(though we would need to check for ldc and ldmd).

HOST_DMD=ldc2 would fail anyways, checking for ldmd2 should suffice.

wilzbach · 2020-08-28T15:39:54Z

src/build.d

+ "LDC": "ldc",
+ "GNU D": "gdc"
+ ][get(`vendor`)];
+ env["HOST_DMD_VERSION"] = get(`version`)[1 .. "vX.XXX.X".length];


So apparently in the conversion to D strings it was unnoticed that _version gets converted into a null-terminated string.

Details: #11640

Yeah, "version" : "v2.093.1-dirty\u0000" isn't right.

Less brittle than parsing the output of --version (especially to determine the frontend version)

wilzbach · 2020-08-28T18:24:20Z

BTW I just saw that there's another --version in the script:

dmd/src/build.d

Lines 1124 to 1128 in d253fe7

 const output = run([ env["HOST_DMD_RUN"], "--version" ]); 

 if (output.canFind("v2.079", "v2.080", "v2.081")) 

 cxxFlags ~= "-DDMD_VERSION=2080"; 

 }

Maybe we should cache the output of version?

kinke · 2020-08-28T18:44:11Z

[Another simple variant which works for all host compilers would be parsing the integer output of pragma(msg, cast(int) __VERSION__);.]

MoonlightSentinel · 2020-08-29T01:06:08Z

Maybe we should cache the output of version?

Definitly

MoonlightSentinel · 2020-08-29T01:08:57Z

[Another simple variant which works for all host compilers would be parsing the integer output of pragma(msg, cast(int) __VERSION__);.]

That was the initial implementation but @wilzbach suggested using the JSON output instead (which initially looked quite simple but has grown quite large to deal with older compilers). Currently leaning more to the initial approach even if it might be slower...

wilzbach · 2020-08-30T09:15:13Z

Currently leaning more to the initial approach even if it might be slower...

I just merged this one as it is green and hopefully more future-proof approach as compilerInfo should return the list of supported, newly introduced features and we don't have to hard-code assumptions about versions. Also, I didn't want us too invest more time on these non mission-critical things ;-)

MoonlightSentinel · 2020-08-30T10:00:36Z

Fair enough

ibuclaw · 2020-08-30T13:46:54Z

I guess I should add support for -Xi then, however the implementation in json.d is naff so blocked until that is fixed.

But as I see gdc is special cased that lessens the incentive to do something about it... :-)

wilzbach · 2020-08-30T14:36:53Z

I guess I should add support for -Xi then, however the implementation in json.d is naff so blocked until that is fixed.

But as I see gdc is special cased that lessens the incentive to do something about it... :-)

Well, dub (and other build tools) are supposed to use this API as well and at least for dub we will add support for this soon, so there still would be a reason ;-)

ibuclaw · 2020-09-04T18:09:47Z

I guess I should add support for -Xi then, however the implementation in json.d is naff so blocked until that is fixed.
But as I see gdc is special cased that lessens the incentive to do something about it... :-)

Well, dub (and other build tools) are supposed to use this API as well and at least for dub we will add support for this soon, so there still would be a reason ;-)

That still doesn't stop -Xi from being utterly unusable. I count at least 1 bug in every 6 lines of that highlighted section.

wilzbach · 2020-09-04T18:36:31Z

That still doesn't stop -Xi from being utterly unusable. I count at least 1 bug in every 6 lines of that highlighted section.

Well, maybe open an issue about it at least, so that it's not forgotten?

ibuclaw · 2020-09-04T21:21:26Z

That still doesn't stop -Xi from being utterly unusable. I count at least 1 bug in every 6 lines of that highlighted section.

Well, maybe open an issue about it at least, so that it's not forgotten?

I raised issues on the 2019-06-30, they still got forgotten. :-)

MoonlightSentinel mentioned this pull request Aug 28, 2020

Qualify diagnostic functions as pragma(printf) #11638

Merged

MoonlightSentinel force-pushed the compiler-detection branch from 8653a7b to 4697296 Compare August 28, 2020 13:24

wilzbach reviewed Aug 28, 2020

View reviewed changes

wilzbach approved these changes Aug 28, 2020

View reviewed changes

MoonlightSentinel changed the title ~~[RFC] build.d: Use pragmas to detect host compiler~~ [RFC] build.d: Use JSON output to detect host compiler Aug 28, 2020

build.d: Use JSON output to detect host compiler

d253fe7

Less brittle than parsing the output of --version (especially to determine the frontend version)

MoonlightSentinel force-pushed the compiler-detection branch from ef78291 to d253fe7 Compare August 28, 2020 16:04

wilzbach merged commit c3831ec into dlang:master Aug 30, 2020

MoonlightSentinel deleted the compiler-detection branch August 30, 2020 10:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] build.d: Use JSON output to detect host compiler #11639

[RFC] build.d: Use JSON output to detect host compiler #11639

MoonlightSentinel commented Aug 28, 2020

dlang-bot commented Aug 28, 2020

wilzbach commented Aug 28, 2020 •

edited

Loading

MoonlightSentinel commented Aug 28, 2020

wilzbach commented Aug 28, 2020

wilzbach Aug 28, 2020

MoonlightSentinel commented Aug 28, 2020

wilzbach Aug 28, 2020

MoonlightSentinel Aug 28, 2020

wilzbach commented Aug 28, 2020

kinke commented Aug 28, 2020

MoonlightSentinel commented Aug 29, 2020

MoonlightSentinel commented Aug 29, 2020

wilzbach commented Aug 30, 2020

MoonlightSentinel commented Aug 30, 2020

ibuclaw commented Aug 30, 2020

wilzbach commented Aug 30, 2020

ibuclaw commented Sep 4, 2020

wilzbach commented Sep 4, 2020

ibuclaw commented Sep 4, 2020

		env["HOST_DMD_KIND"] = "gdc";
		env["HOST_DMD_VERSION"] = "v2.079";

[RFC] build.d: Use JSON output to detect host compiler #11639

[RFC] build.d: Use JSON output to detect host compiler #11639

Conversation

MoonlightSentinel commented Aug 28, 2020

dlang-bot commented Aug 28, 2020

Bugzilla references

Testing this PR locally

wilzbach commented Aug 28, 2020 • edited Loading

MoonlightSentinel commented Aug 28, 2020

wilzbach commented Aug 28, 2020

wilzbach Aug 28, 2020

Choose a reason for hiding this comment

MoonlightSentinel commented Aug 28, 2020

wilzbach Aug 28, 2020

Choose a reason for hiding this comment

MoonlightSentinel Aug 28, 2020

Choose a reason for hiding this comment

wilzbach commented Aug 28, 2020

kinke commented Aug 28, 2020

MoonlightSentinel commented Aug 29, 2020

MoonlightSentinel commented Aug 29, 2020

wilzbach commented Aug 30, 2020

MoonlightSentinel commented Aug 30, 2020

ibuclaw commented Aug 30, 2020

wilzbach commented Aug 30, 2020

ibuclaw commented Sep 4, 2020

wilzbach commented Sep 4, 2020

ibuclaw commented Sep 4, 2020

wilzbach commented Aug 28, 2020 •

edited

Loading