Azure CI OMF job: Link testsuite tools with correct phobos.lib #11322

kinke · 2020-06-24T23:02:42Z

No description provided.

kinke · 2020-06-25T16:43:15Z

@rainers: What a mess, thx for the pointers wrt. Optlink. I find it very surprising that the issue (tools compiled by host compiler, but linked against fresh phobos.lib) only begins to show up now with dlang/druntime#3141 and dlang/druntime#3142.

Optlink apparently unconditionally reading from sc.ini in the containing directory, even when running DMD with -conf= is totally unexpected, and I'm surprised this hasn't caused enough problems for someone to fix it. As the config file seems to take precedence over the LIB env variable, I guess the only way to enforce a dir from which to take phobos.lib etc. would be specifying that dir in the linker cmdline via some -L... in the DMD cmdline. No idea how Optlink's CLI option needs to look like though.
And that still wouldn't be enough, as the run command would need 2 different sets of arguments, one for the tools compiled by the host compiler, and one for the regular tests using the fresh compiler. Some tools being built by Makefile and others by run.d doesn't make this mess any easier.

Previously, the tools were compiled by the DMD host compiler, but linked with freshly compiled druntime/Phobos, leading to inevitable issues popping up in dlang/druntime#3141 and dlang/druntime#3142.

rainers · 2020-06-26T06:13:22Z

The (rather surprising) way to pass a library search path to optlink as a command line argument is to specify it in place of a library, but with a trailing \, see lib in https://www.digitalmars.com/ctg/optlink.html.

Another option should be to pass phobos.lib explicitly on the command line, Not sure how to integrate that with the ci scripts and tools, though.

kinke · 2020-06-26T11:31:46Z

I was wondering how/why the autotester works. And well, it's not using the host compiler for building any tools, but the fresh compiler exclusively:

HOST_DC=/home/braddr/sandbox/at-client/release-build/dmd-2.079.0/windows/bin/dmd.exe
HOST_DC=D:\sandbox\at-client\release-build\dmd-2.079.0\windows\bin\dmd.exe
cd test
gmake -j4
Makefile:103: Please use HOST_DMD instead of HOST_DC!
Creating output directory: test_results
Building d_do_test tool
OS: 'windows'
MODEL: '32'
PIC: ''
../generated/windows/release/32/dmd.exe -conf= -m32  -g -lowmem -i -Itools -version=NoMain -unittest -run tools/d_do_test.d
1 modules passed unittests
../generated/windows/release/32/dmd.exe -conf= -m32  -g -lowmem -i -Itools -version=NoMain -odtest_results -oftest_results/d_do_test.exe tools/d_do_test.d
../generated/windows/release/32/dmd.exe -conf= -m32  -g -odtest_results -oftest_results/run.exe -i -release run.d
test_results/run.exe --environment
================================================================================
DFLAGS=-ID:\sandbox\at-client\pull-4094895-Win_32\dmd\test\..\..\druntime\import -ID:\sandbox\at-client\pull-4094895-Win_32\dmd\test\..\..\phobos
ARGS=-inline -release -g -O
DMD_TEST_COVERAGE=0
OS=windows
DSEP=\\
MODEL=32
DMD_MODEL=32
RESULTS_DIR=test_results
DMD=D:\sandbox\at-client\pull-4094895-Win_32\dmd\generated\windows\release\32\dmd.exe
LIB=D:\sandbox\at-client\pull-4094895-Win_32\dmd\test\..\..\phobos
BUILD=release
OBJ=.obj
EXE=.exe
SEP=\
================================================================================
Executing: ../generated/windows/release/32/dmd.exe -m32 -oftest_results\unit_test_runner.exe D:\sandbox\at-client\pull-4094895-Win_32\dmd\test\tools\unit_test_runner.d D:\sandbox\at-client\pull-4094895-Win_32\dmd\test\tools\paths
================================================================================
DFLAGS=-ID:\sandbox\at-client\pull-4094895-Win_32\dmd\test\..\..\druntime\import -ID:\sandbox\at-client\pull-4094895-Win_32\dmd\test\..\..\phobos
ARGS=-inline -release -g -O
DMD_TEST_COVERAGE=0
OS=windows
DSEP=\\
MODEL=32
DMD_MODEL=32
RESULTS_DIR=test_results
DMD=D:\sandbox\at-client\pull-4094895-Win_32\dmd\generated\windows\release\32\dmd.exe
LIB=D:\sandbox\at-client\pull-4094895-Win_32\dmd\test\..\..\phobos
BUILD=release
OBJ=.obj
EXE=.exe
SEP=\
================================================================================
Executing: ../generated/windows/release/32/dmd.exe -m32 -oftest_results\unit_test_runner.exe D:\sandbox\at-client\pull-4094895-Win_32\dmd\test\tools\unit_test_runner.d D:\sandbox\at-client\pull-4094895-Win_32\dmd\test\tools\paths
Executing: ../generated/windows/release/32/dmd.exe -m32 -oftest_results\sanitize_json.exe D:\sandbox\at-client\pull-4094895-Win_32\dmd\test\tools\sanitize_json.d

kinke · 2020-06-26T11:53:43Z

The autotester Win_32_64 job is also using the fresh compiler, as is the Azure 'Windows x64' job. The other Azure jobs with LDC in their name (presumably LDC as host compiler) use neither the fresh compiler nor LDC, but (AFAICT) some other DMD installation; these jobs also don't use the test/Makefile, but build run.d manually and then invoke run ... directly. What. A. Mess.

MoonlightSentinel · 2020-06-26T12:00:39Z

The autotester Win_32_64 job is also using the fresh compiler,

This a (unpleasant) necessity because the autotester uses 2.079 which doesn't support -lowmem. Ideally it should use another host compiler but I don't think that we'll get another DMD installation....

as is the Azure 'Windows x64' job. The other Azure jobs with LDC in their name (presumably LDC as host compiler) use neither the fresh compiler nor LDC, but (AFAICT) some other DMD installation. What. A. Mess.

That is definitly wrong.

these jobs also don't use the test/Makefile

The makefile is a legacy nowadays because we can't access the autotester...

kinke · 2020-06-26T12:01:33Z

LDC uses the fresh compiler to build the tools, always. I've had a discussion regarding this with Rainer some time ago and couldn't be persuaded that using the host compiler would help in making these tests more stable (preventing new compiler/druntime/Phobos from generating faulty tools).

kinke · 2020-06-26T13:43:38Z

Oh wow, DMD apparently also completely ignores the DFLAGS env variable if it's specified in sc.ini (it is by default).

…compiler

kinke · 2020-06-26T17:28:35Z

AFAICT, this should be working now as intended (all testsuite tool executables built by host compiler again). I've previously added a -v to the manual DFLAGS, to make sure that var is used for all tools. The test/Makefile isn't used by any Azure Windows jobs anymore.

The Azure 'Windows x64' job now also uses the host compiler (DMD 2.084) for building those tools. -lowmem for d_do_test is only used in the Makefile, not when built via run.d. I've added -lowmem some time ago (in the Makefile), but only to enable parallel building of these tools to save some seconds, but that isn't parallelized any longer AFAICT, so it's probably superfluous now.

kinke · 2020-06-26T17:33:10Z

test/run.d

        auto phobosPath = environment.get("PHOBOS_PATH", testPath(`..\..\phobos`));
        env["DFLAGS"] = `-I%s\import -I%s`.format(druntimePath, phobosPath);
-        env["LIB"] = phobosPath;
+        env["LIB"] = phobosPath ~ ";" ~ environment.get("LIB");


Don't completely replace LIB, only prepend the dir containing fresh phobos.lib. We still need the original LIB dirs for kernel32.lib etc.

kinke · 2020-06-26T17:37:41Z

.azure-pipelines/windows.sh

    LIBNAME=phobos32mscoff.lib
 else
-    export MODEL_FLAG="-m32"
    export LIB="$PWD/dmd2/windows/lib"


Not sure why this is still necessary, but linking the druntime unittests failed without this (Optlink complaining about incompatible libs in MSVC libdirs). So somehow the optlink.exe used for that command seems to respect the LIB env var (which in this case is a Posix path - /c/...).

rainers · 2020-06-27T06:16:33Z

.azure-pipelines/windows.sh

+    # manually for the host compiler. These 2 variables are adapted for the
+    # actual tests with the tested compiler (by run.d).
+    HOST_DMD_DIR="$(cygpath -w "$DMD_DIR/tools/dmd2")"
+    rm "$HOST_DMD_DIR/windows/bin/sc.ini"


Thanks for tackling this.

Instead of breaking the host compiler installation (in case someone uses the script outside of the CI), placing an empty sc.ini into the current directory should also work, because that's preferred by optlink.

A quick local test shows that's not the case. DMD prefers that sc.ini in the current dir, but Optlink isn't, regardless of whether it's completely empty or contains an empty [Environment] section, and regardless of whether Optlink is invoked via DMD or directly.

Ok. I was looking at this comment: https://github.com/DigitalMars/optlink/blob/87a85b0413d94fb8017656004a03f7fb0aa3a649/common/iniproc.asm#L52

I guess no one uses the CI build scripts locally.

rainers

Let's see how the affected druntime tests work with this.

kinke force-pushed the ci_phobos branch 7 times, most recently from 91f5457 to 4b6abd1 Compare June 25, 2020 02:20

Azure CI OMF job: Link testsuite tools with correct phobos.lib

104f8cf

Previously, the tools were compiled by the DMD host compiler, but linked with freshly compiled druntime/Phobos, leading to inevitable issues popping up in dlang/druntime#3141 and dlang/druntime#3142.

kinke force-pushed the ci_phobos branch from 4b6abd1 to 104f8cf Compare June 26, 2020 00:45

kinke changed the title ~~Azure CI experiment~~ Azure CI OMF job: Link testsuite tools with correct phobos.lib Jun 26, 2020

kinke force-pushed the ci_phobos branch from 8ee2b4c to 488e41b Compare June 26, 2020 13:26

kinke force-pushed the ci_phobos branch 4 times, most recently from 712f7cf to e2d0fde Compare June 26, 2020 16:49

Use a different variant to compile all testsuite tools with the host …

e372f40

…compiler

kinke force-pushed the ci_phobos branch from e2d0fde to e372f40 Compare June 26, 2020 16:50

kinke marked this pull request as ready for review June 26, 2020 17:28

kinke commented Jun 26, 2020

View reviewed changes

rainers reviewed Jun 27, 2020

View reviewed changes

rainers approved these changes Jun 27, 2020

View reviewed changes

rainers added the Merge:auto-merge label Jun 27, 2020

dlang-bot merged commit 2ba989e into dlang:master Jun 27, 2020

kinke deleted the ci_phobos branch June 27, 2020 11:52

Uh oh!

Azure CI OMF job: Link testsuite tools with correct phobos.lib #11322

Azure CI OMF job: Link testsuite tools with correct phobos.lib #11322

Uh oh!

Conversation

kinke commented Jun 24, 2020

Uh oh!

kinke commented Jun 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rainers commented Jun 26, 2020

Uh oh!

kinke commented Jun 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kinke commented Jun 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MoonlightSentinel commented Jun 26, 2020

Uh oh!

kinke commented Jun 26, 2020

Uh oh!

kinke commented Jun 26, 2020

Uh oh!

kinke commented Jun 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kinke Jun 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kinke Jun 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rainers Jun 27, 2020

Choose a reason for hiding this comment

Uh oh!

kinke Jun 27, 2020

Choose a reason for hiding this comment

Uh oh!

rainers Jun 27, 2020

Choose a reason for hiding this comment

Uh oh!

rainers left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kinke commented Jun 25, 2020 •

edited

Loading

kinke commented Jun 26, 2020 •

edited

Loading

kinke commented Jun 26, 2020 •

edited

Loading

kinke commented Jun 26, 2020 •

edited

Loading

kinke Jun 26, 2020 •

edited

Loading

kinke Jun 26, 2020 •

edited

Loading