Skip to content

Comments

Merge remote-tracking branch 'upstream/stable' into merge_stable#7848

Merged
MartinNowak merged 10 commits intodlang:masterfrom
MartinNowak:merge_stable
Feb 9, 2018
Merged

Merge remote-tracking branch 'upstream/stable' into merge_stable#7848
MartinNowak merged 10 commits intodlang:masterfrom
MartinNowak:merge_stable

Conversation

@MartinNowak
Copy link
Member

No description provided.

rainers and others added 8 commits February 1, 2018 19:41
…indows 10 with VS 2015

checking the KitsRoot10 lib folder is not good enough to detect the SDK, it might just contain the UCRT
fix issue 18352 - [REG 2.078] dmd can't generate 64-bit binaries on Windows
fix issue 18354 - [Reg 2.078] Building fails with VC 2015 Build Tools
`SBB reg,0` is needed to make this work. There's nothing platform specific
about it.

Also add comments explaining how this works.
fix issue 18315 - wrong code for `i > 0`
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @MartinNowak!

Bugzilla references

Auto-close Bugzilla Description
18315 wrong code for i > 0
18352 [REG 2.078] dmd can't generate 64-bit binaries on Windows 10 with VS 2015
18354 [Reg 2.078] Building fails with VC 2015 Build Tools

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

@MartinNowak
Copy link
Member Author

The semaphore issue looks unrelated (and occurs several times). Furthermore they don't seem to have a retest button :/.

@wilzbach
Copy link
Contributor

wilzbach commented Feb 7, 2018

They do, did you login?

image

You should have access?

image

The semaphore issue looks unrelated (and occurs several times).

I have seen something similar yesterday after a passing PR was merged to Phobos.
Eventually I had to revert that PR: dlang/phobos#6130 :/

@wilzbach
Copy link
Contributor

wilzbach commented Feb 7, 2018

For reference, the failure is:

Test failed.  The logged output:
../generated/linux/release/64/dmd -conf= -m64 -Irunnable  -fPIC  -odtest_results/runnable -oftest_results/runnable/structlit_0  runnable/structlit.d 
runnable/structlit.d(1298): Deprecation: constructor `structlit.test11256.F11256!((gv) => true).F11256.this` all parameters have default arguments, but structs cannot have default constructors.
runnable/structlit.d(1298): Deprecation: constructor `structlit.test11256.F11256!((gv) => true).F11256.this` all parameters have default arguments, but structs cannot have default constructors.
runnable/structlit.d(1298): Deprecation: constructor `structlit.test11256.F11256!((gv) => true).F11256.this` all parameters have default arguments, but structs cannot have default constructors.
test_results/runnable/structlit_0
test_results/runnable/structlit_0: symbol lookup error: test_results/runnable/structlit_0: undefined symbol: _D4core4stdc4math5isnanFNaNbNiNedZi


==============================
Test failed: expected rc == 0, exited with rc == 127

make[1]: *** [test_results/runnable/structlit.d.out] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [start_runnable_tests] Error 2

Though @marler8997 saw it too: https://semaphoreci.com/wilzbach/dmd-2/branches/pull-request-7777/builds/6

I bet this is due to dlang/druntime#2045
However, that would imply that our testsuite is somehow linking in the wrong version of Phobos :O

@MartinNowak
Copy link
Member Author

However, that would imply that our testsuite is somehow linking in the wrong version of Phobos :O

Yes, looks like there is a mismatch between the druntime import and the linked version.
Looks like the makefile isn't using -conf=

DFLAGS=-I$(DRUNTIME_PATH)/import -I$(PHOBOS_PATH) -L-L$(PHOBOS_PATH)/generated/$(OS)/$(BUILD)/$(MODEL)
, so we could pick up one of the dmd.conf/sc.ini locations.

@wilzbach
Copy link
Contributor

wilzbach commented Feb 8, 2018

Looks like the makefile isn't using -conf=

#7846

Though lots of things failed.

@MartinNowak
Copy link
Member Author

I can reproduce the failure locally, will check and fix it tomorrow.

@MartinNowak
Copy link
Member Author

Seems related to having the install.sh environment activated while running dmd's test makefile. Quite annoying that d_do_test passes lots of config intransparently through environment variables.

@marler8997
Copy link
Contributor

Seems related to having the install.sh environment activated while running dmd's test makefile. Quite annoying that d_do_test passes lots of config intransparently through environment variables.

I think the reason for this is because many of those variables are shared by both the Makefile and the shell scripts. By setting them in the initial Makefile, both d_do_test and the test files all reference the same set of variables.

This isn't ideal but it allows variables to be shared between these 3 languages, Make, D and BASH. If everything was in the same langauge, say D, all these variables could be declared in a common library and passed around without poisoning the environment.

- fixes issues where the activated release dmd overrides and hijacks the link
  paths of the generated dmd (by it's set LIBRARY_PATH/LD_LIBRARY_PATH)
- build and test commands already activate and deactivate host compiler where necessary
@MartinNowak
Copy link
Member Author

I think the reason for this is because many of those variables are shared by both the Makefile and the shell scripts. By setting them in the initial Makefile, both d_do_test and the test files all reference the same set of variables.

It's definitely easier for recursive invokations, but hard when you try to reproduce sth.
Anyhow, here is a fix, don't run tests with activated host compiler, as it hijacks the linker path (by setting LIBRARY_PATH). This seems to take precendence over the -L-L$(PHOBOS_PATH)/generated/$(OS)/$(BUILD)/$(MODEL).

@marler8997
Copy link
Contributor

It's definitely easier for recursive invokations, but hard when you try to reproduce sth.

Yes, completely agree with this.

@MartinNowak
Copy link
Member Author

@wilzbach FYI I fixed the semaphore issue in 1e81c60.

@MartinNowak MartinNowak merged commit d4220c5 into dlang:master Feb 9, 2018
@MartinNowak MartinNowak deleted the merge_stable branch February 9, 2018 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants