Use generated DMD binary instead of src/dmd by wilzbach · Pull Request #6870 · dlang/dmd

wilzbach · 2017-06-08T04:17:43Z

braddr · 2017-06-08T04:26:27Z

test/Makefile

 ifeq ($(findstring win,$(OS)),win)
 export ARGS=-inline -release -g -O
-export DMD=../src/dmd.exe
+export DMD=..\generated\$(OS)\$(MODEL)\dmd.exe


This path should include $(BUILD) as well. It's a mistake that the current windows make files don't put the binary in the right place and this change shouldn't perpetuate the error.

RazvanN7 · 2017-06-08T12:17:04Z

#6873 fixes the windows makefile, currently hardcoding BUILD to release, so you can go ahead and use generated$OS$BUILD$MODEL after that one is merged. Hopefully we can change the autotester soon

wilzbach · 2017-06-09T23:36:11Z

Hmm it seems like the auto-tester picks up MODEL=64 for this test Makefile, even though it's building dmd with MODEL=32...

CC=c++ /home/braddr/sandbox/at-client/release-build/install/linux/bin32/dmd -of../generated/linux/release/32/dmd -m32 -vtls -J../generated/linux/release/32 -J../res -L-lstdc++ -version=MARS -wi -O -release -inline ddmd/access.d ddmd/aggregate.d ddmd/aliasthis.d ddmd/apply.d ddmd/argtypes.d ddmd/arrayop.d ddmd/arraytypes.d ddmd/astcodegen.d ddmd/attrib.d ddmd/builtin.d ddmd/canthrow.d ddmd/clone.d ddmd/complex.d ddmd/cond.d ddmd/constfold.d ddmd/cppmangle.d ddmd/ctfeexpr.d ddmd/dcast.d ddmd/dclass.d ddmd/declaration.d ddmd/delegatize.d ddmd/denum.d ddmd/dimport.d ddmd/dinifile.d ddmd/dinterpret.d ddmd/dmacro.d ddmd/dmangle.d ddmd/dmodule.d ddmd/doc.d ddmd/dscope.d ddmd/dstruct.d ddmd/dsymbol.d ddmd/dtemplate.d ddmd/dversion.d ddmd/escape.d ddmd/expression.d ddmd/func.d ddmd/hdrgen.d ddmd/impcnvtab.d ddmd/imphint.d ddmd/init.d ddmd/inline.d ddmd/inlinecost.d ddmd/intrange.d ddmd/json.d ddmd/lib.d ddmd/link.d ddmd/mars.d ddmd/mtype.d ddmd/nogc.d ddmd/nspace.d ddmd/objc.d ddmd/opover.d ddmd/optimize.d ddmd/parse.d ddmd/sapply.d ddmd/sideeffect.d ddmd/statement.d ddmd/staticassert.d ddmd/target.d ddmd/traits.d ddmd/visitor.d ddmd/typinf.d ddmd/utils.d ddmd/statement_rewrite_walker.d ddmd/statementsem.d ddmd/staticcond.d ddmd/safe.d ddmd/blockexit.d ddmd/asttypename.d ddmd/printast.d ddmd/libelf.d ddmd/scanelf.d ddmd/irstate.d ddmd/toctype.d ddmd/glue.d ddmd/gluelayer.d ddmd/todt.d ddmd/tocsym.d ddmd/toir.d ddmd/dmsc.d ddmd/tocvdebug.d ddmd/s2ir.d ddmd/toobj.d ddmd/e2ir.d ddmd/eh.d ddmd/iasm.d ddmd/objc_glue_stubs.d ddmd/backend/bcomplex.d ddmd/backend/cc.d ddmd/backend/cdef.d ddmd/backend/cgcv.d ddmd/backend/code.d ddmd/backend/cv4.d ddmd/backend/dt.d ddmd/backend/el.d ddmd/backend/global.d ddmd/backend/obj.d ddmd/backend/oper.d ddmd/backend/outbuf.d ddmd/backend/rtlsym.d ddmd/backend/code_x86.d ddmd/backend/iasm.d ddmd/backend/ty.d ddmd/backend/type.d ddmd/backend/exh.d ddmd/backend/mach.d ddmd/backend/md5.d ddmd/backend/mscoff.d ddmd/backend/dwarf.d ddmd/backend/dwarf2.d ddmd/backend/xmm.d ddmd/tk/dlist.d ddmd/root/aav.d ddmd/root/man.d ddmd/root/response.d ddmd/root/speller.d ../generated/linux/release/32/newdelete.o ../generated/linux/release/32/backend.a ../generated/linux/release/32/lexer.a
cp ../generated/linux/release/32/dmd .
make[1]: Leaving directory '/home/braddr/sandbox/at-client/pull-2605257-Linux_32_64/dmd/src'
make -C src -f posix.mak
make[1]: Entering directory '/home/braddr/sandbox/at-client/pull-2605257-Linux_32_64/dmd/src'
no cpu specified, assuming X86

...

make -C ../src -f posix.mak ../generated/linux/release/64/dmd
Creating output directory: generated
make[2]: Entering directory '/home/braddr/sandbox/at-client/pull-2605257-Linux_32_64/dmd/test'
make[2]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
no cpu specified, assuming X86
posix.mak:86: *** 'dmd' not found, get a D compiler or make AUTO_BOOTSTRAP=1.  Stop.
make[2]: Leaving directory '/home/braddr/sandbox/at-client/pull-2605257-Linux_32_64/dmd/src'
Makefile:210: recipe for target '../generated/linux/release/64/dmd' failed
make[1]: *** [../generated/linux/release/64/dmd] Error 2

@RazvanN7 @CyberShadow any ideas how to deal with this (without ugly workarounds like checking for file existence)?

RazvanN7 · 2017-06-12T07:04:15Z

After reading @braddr 's scripts, I think that the problem is due to the mismatch in determining the model. While osmodel.mak uses "uname -m" to determine the model, the autotester uses the name of the machine (which in this case is linux_32_64): https://github.com/braddr/at-client/blob/master/src/setup_env.sh#L45. One solution might be to make the autotester use osmodel.mak, but I'm going to deffer this to Brad.

wilzbach · 2017-07-17T00:35:10Z

@RazvanN7 I think I found a nice workaround at #6999:

ifeq (,$(wildcard ../generated/$(OS)/$(BUILD)/64/dmd))
REAL_MODEL=32
else
REAL_MODEL=64
endif
export DMD=../generated/$(OS)/$(BUILD)/$(REAL_MODEL)/dmd

PetarKirov · 2017-07-17T10:11:19Z

test/Makefile

 include ../src/osmodel.mak

 export OS
+BUILD=release


Why is this necessary? debug is a better choice for testing in general and tests for specific optimization arguments, such as -release, do so via PERMUTE_ARGS. This variable affects only the test runner, right?

Why is this necessary?

Because you need to find the DMD binary somehow and as Makefile variable at least the user can overwrite it...

debug is a better choice for testing in general and tests for specific optimization arguments

Hehe debug is simply a different folder as of now:
#6898

This variable affects only the test runner, right?

Yes.

Because you need to find the DMD binary somehow and as Makefile variable at least the user can overwrite it...

I mean, is there a reason for preferring release over debug? If not I think it would be better to stick with debug, even though currently there may not be much of a difference.

I mean, is there a reason for preferring release over debug?

Hehe I agree, but release is the default and thus used by auto-tester.

dlang-bot · 2017-12-01T05:24:58Z

Thanks for your pull request, @wilzbach!

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.

wilzbach · 2017-12-05T17:21:23Z

@RazvanN7 @ZombineDev @braddr FWIWI this is finally green, but I really don't like this hack. I think this is due to the fact that we use a different MODEL to build the compiler than for testing it?
(see below)

https://github.com/braddr/at-client/blob/master/src/do_build_dmd.sh

$makecmd MAKE=$makecmd MODEL=$COMPILER_MODEL

https://github.com/braddr/at-client/blob/master/src/do_test_dmd.sh

$makecmd MODEL=$OUTPUT_MODEL

FYI envs are as follows:
https://github.com/braddr/at-client/blob/master/src/setup_env.sh

COMPILER_MODEL=32
OUTPUT_MODEL=32
...
case "$1" in
...
    Linux_32_64)
        OUTPUT_MODEL=64
        ;;
    Linux_64_32)
        COMPILER_MODEL=64

So this leaves us the following options:

We go with an detection approach like this (sth. like ls -1 ../dmd/generated/$(OS)/$(BUILD)/*/dmd might work too)
We explicitly pass the BUILT_MODEL everywhere
We explicitly pass DMD_PATH everywhere (see Set hard-coded src/dmd executables to generated/ (v2) braddr/at-client#5)

Ideas? Thoughts?

wilzbach · 2017-12-09T06:08:02Z

We go with an detection approach like this (sth. like ls -1 ../dmd/generated/$(OS)/$(BUILD)/*/dmd might work too)

Okay I switched to this approach. It's the same way the auto-tester currently deals with this problem & it looks a lot cleaner to.
As an additional bonus I added a second commit which will now allow building d_do_test on a hardened system.
So guys, any objections with this or can we move forward here? (this is needed as part of the ddmd -> dmd transition)

CC @CyberShadow maybe?

PetarKirov · 2017-12-12T09:13:22Z

LGTM. Can you remove the fix for hardened systems and focus on in the other PR, so we can this more easily merged?

RazvanN7 · 2017-12-12T09:31:57Z

LGTM. I like the ls -1 approach.

wilzbach · 2017-12-12T09:36:13Z

Can you remove the fix for hardened systems and focus on in the other PR, so we can this more easily merged?

Fair point. Done.

wilzbach · 2017-12-12T18:13:05Z

Argh so I thought this was passing already (it did before the unittest target was merged). Now the auto-tester builds generated/linux/release/<otherModel>/dmd-unittest in the "test dmd" step.
This means that now ../generated/linux/release/64 exists and thus the ls -1 detection doesn't work anymore as for e.g. Linux_32_64 the 64-bit folder is existent and detected.
I guess I have to revert to my earlier approach of using ifeq (,$(wildcard ../generated/$(OS)/$(BUILD)/64/dmd)) to detect the valid DMD binary ...

auto-tester might run the testsuite with a different $(MODEL) than DMD has been compiled with. Hence we manually check which binary exists to infer the host model.

wilzbach · 2017-12-13T08:27:09Z

test/Makefile

 # link against shared libraries (defaults to true on supported platforms, can be overridden w/ make SHARED=0)
 SHARED=$(if $(findstring $(OS),linux freebsd),1,)
-DFLAGS=-I$(DRUNTIME_PATH)/import -I$(PHOBOS_PATH) -L-L$(PHOBOS_PATH)/generated/$(OS)/release/$(MODEL)
+DFLAGS=-I$(DRUNTIME_PATH)/import -I$(PHOBOS_PATH) -L-L$(PHOBOS_PATH)/generated/$(OS)/$(BUILD)/$(MODEL)


Druntime and Phobos are built with the opposite MODEL on auto-tester
For example for 32_64: builds 32-bit DMD, but 64-but DRuntime + Phobos

wilzbach force-pushed the use_src_dmd_for_testsuite branch 2 times, most recently from fe65d17 to 444dd00 Compare June 8, 2017 04:24

braddr reviewed Jun 8, 2017

View reviewed changes

wilzbach force-pushed the use_src_dmd_for_testsuite branch from 444dd00 to a89a680 Compare June 8, 2017 05:53

wilzbach mentioned this pull request Jul 17, 2017

Convert DUB package test into a Makefile test and run it with the proper compiler flags #6999

Closed

PetarKirov reviewed Jul 17, 2017

View reviewed changes

wilzbach force-pushed the use_src_dmd_for_testsuite branch from a89a680 to 7334c12 Compare December 1, 2017 05:24

wilzbach force-pushed the use_src_dmd_for_testsuite branch from 7334c12 to acdfa81 Compare December 1, 2017 05:44

wilzbach mentioned this pull request Dec 5, 2017

Export HOST_DC during test_dmd braddr/at-client#6

Merged

wilzbach force-pushed the use_src_dmd_for_testsuite branch from acdfa81 to dfcc6e2 Compare December 5, 2017 04:11

This was referenced Dec 5, 2017

Set hard-coded src/dmd executables to generated/ (v2) braddr/at-client#5

Merged

Use generated DMD executable for the testsuite #7398

Closed

[Do not merge] Use generated DMD executable for the testsuite [Windows] #7399

Closed

wilzbach mentioned this pull request Dec 5, 2017

Set DMD path for the DMD testsuite braddr/at-client#7

Merged

wilzbach force-pushed the use_src_dmd_for_testsuite branch 2 times, most recently from 29470e6 to 7dcbb7c Compare December 9, 2017 05:29

wilzbach force-pushed the use_src_dmd_for_testsuite branch from 7dcbb7c to 029c857 Compare December 12, 2017 09:35

andralex approved these changes Dec 12, 2017

View reviewed changes

andralex added the Merge:auto-merge label Dec 12, 2017

wilzbach removed the Merge:auto-merge label Dec 12, 2017

wilzbach force-pushed the use_src_dmd_for_testsuite branch 4 times, most recently from bcd303d to 70976a2 Compare December 13, 2017 06:32

Use generated DMD binary instead of src/dmd

6d06b93

auto-tester might run the testsuite with a different $(MODEL) than DMD has been compiled with. Hence we manually check which binary exists to infer the host model.

wilzbach force-pushed the use_src_dmd_for_testsuite branch from 70976a2 to 6d06b93 Compare December 13, 2017 08:25

wilzbach commented Dec 13, 2017

View reviewed changes

wilzbach added the Merge:auto-merge label Dec 13, 2017

dlang-bot merged commit 901fde2 into dlang:master Dec 13, 2017

wilzbach deleted the use_src_dmd_for_testsuite branch December 19, 2017 09:25

Uh oh!

Conversation

wilzbach commented Jun 8, 2017

Uh oh!

braddr Jun 8, 2017

Choose a reason for hiding this comment

Uh oh!

RazvanN7 commented Jun 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilzbach commented Jun 9, 2017

Uh oh!

RazvanN7 commented Jun 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilzbach commented Jul 17, 2017

Uh oh!

PetarKirov Jul 17, 2017

Choose a reason for hiding this comment

Uh oh!

wilzbach Jul 17, 2017

Choose a reason for hiding this comment

Uh oh!

PetarKirov Jul 17, 2017 • 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!

wilzbach Jul 17, 2017

Choose a reason for hiding this comment

Uh oh!

dlang-bot commented Dec 1, 2017

Bugzilla references

Uh oh!

wilzbach commented Dec 5, 2017

Uh oh!

wilzbach commented Dec 9, 2017

Uh oh!

PetarKirov commented Dec 12, 2017

Uh oh!

RazvanN7 commented Dec 12, 2017

Uh oh!

wilzbach commented Dec 12, 2017

Uh oh!

wilzbach commented Dec 12, 2017

Uh oh!

wilzbach Dec 13, 2017

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

RazvanN7 commented Jun 8, 2017 •

edited

Loading

RazvanN7 commented Jun 12, 2017 •

edited

Loading

PetarKirov Jul 17, 2017 •

edited

Loading