braddr / d-tester

Automated testing for github projects.
http://d.puremagic.com/test-results/
11 stars 5 forks source link

Can't change win64.mak #37

Open mihails-strasuns opened 9 years ago

mihails-strasuns commented 9 years ago

Because of win64.mak specific tweaks (https://github.com/braddr/d-tester/blob/master/client/src/diff-phobos-win64.diff) any pull request that tries to change it will fail the auto-tester.

Perhaps better approach would have been to leave it untouched and override variables from the command line : make -f win64.mak CC="$(VCBIN_DIR)\cl" LD="$(VCBIN_DIR)\link" LIB="$(VCBIN_DIR)\lib"

rainers commented 9 years ago

For me, both druntime and phobos build with

make -f win64.mak DMD=../dmd/src/dmd MODEL=64 VCDIR="c:\l\vs12\vc"

from a cmd.exe line without any VC settings.

The dmd test suite runs with

c:\u\unix\make.exe OS=win64 MODEL=64 CC="c:\l\vs12\vc\bin\amd64\cl.exe" INCLUDE="c:\l\vs12\vc\include"

braddr commented 9 years ago

All of that is seriously old stuff. Written pretty quickly in the pre-alpha state of win64 support. It's been need of cleanup ever since, but obviously hasn't been important enough to do. The problem is that the path is only partially settable through VCDIR, and the final part still includes one directory layer that varies between VC releases. Asinine in multiple ways.

I'd like to avoid having platform specific make invocations (ie, setting CC and friends only on win64). If having them set in the env is sufficient (and it ought to be), then that's likely a better way to go. I'll be it's NOT sufficient, at least not in all the places that that diff is currently touching.

Take a look at the config file used on the win64 box (might be slightly out of date, but close enough for discussion): https://github.com/braddr/d-tester/blob/master/client/configs/WIN-F30QP0PU8L1

For the short term, it might be quickest to just commit the pull request and let win64 break. I'll fix the diff as soon as I can afterwards to minimize the auto-tester impact. I've got other things on my plate this week so I'm unlikely to spend time on this issue to avoid the breakage from happening.

mihails-strasuns commented 9 years ago

@rainers just VCDIR is not enough with VS2013 / x86_64 because binaries are located there in a different folder (amd64_bin or something like that). I needed to overwrite actual CC/LD variables.

rainers commented 9 years ago

@rainers just VCDIR is not enough with VS2013 / x86_64 because binaries are located there in a different folder (amd64_bin or something like that). I needed to overwrite actual CC/LD variables.

It worked for my installation of VS2013. I don't think there is a breaking change in the VC directories, only for the platform library directories. But these are not needed by the makefile as it uses the settings from sc.ini anyway.

BTW: I have no idea why dmd/ini/windows/bin/sc.ini uses the 32-bit cross-compiler versions of x86_amd64/link.exe for VS 2012/2013. These are identical to the linker exectutables in the VC/bin folder. 32-bit builds 64-bt application just fine, but why special case per VS version when the default LINKCMD setting does exactly the same?

rainers commented 9 years ago

The problem is that the path is only partially settable through VCDIR

It might be a problem that scWin_64.ini is slightly underpowered.

I'd like to avoid having platform specific make invocations (ie, setting CC and friends only on win64). If having them set in the env is sufficient (and it ought to be), then that's likely a better way to go. I'll be it's NOT sufficient, at least not in all the places that that diff is currently touching.

Unfortunately DigitalMars make is too dump to set variables only if the environment variable is already set. AFAICT the only way to override a setting is via the command line. I don't think adding an option is a problem, you even have the EXTRA_ARGS variable already in place.

If that is really not an option, how about adding another makefile win64_tester.mak that includes the additional command line? This avoids conflicts because win64.mak can stay unmodified.

rainers commented 9 years ago

For the short term, it might be quickest to just commit the pull request and let win64 break. I'll fix the diff..

That only works if you are really fast, as no PRs can be (auto)-merged as long as master is broken. In addition, pull requests will probably get less attention if they don't pass.

rainers commented 9 years ago

How can we move forward with this? Would it make sense to create a pull request that

To try that locally it would be good to know which libraries are installed to the src/winlibs folder.

braddr commented 9 years ago

I'll re-state what I said before. I'll get to this, but not quickly. I've got other priorities taking precidence. The quickest way forwards it, assuming you're confident that win64 isn't going to break in other ways due to the changes, then just merge the pull request. I'll get the patch rebased fairly quickly. It might be as fast as under an hour, it might be the better part of a day. Either way, in the grand scheme of things it's not that big a deal.

mihails-strasuns commented 9 years ago

btw, while we are on it, @rainers may you please have a brief look at http://wiki.dlang.org/Developing_DMD_/_Phobos_on_Windows_8 when you have few spare minutes? I'd really can use some review from more experienced windows users.

rainers commented 9 years ago

I'll get the patch rebased fairly quickly. It might be as fast as under an hour, it might be the better part of a day.

I suggest that you simply merge https://github.com/D-Programming-Language/phobos/pull/2432 when you have time to deal with this. This will cause the minimal inconvenience for others.

rainers commented 9 years ago

btw, while we are on it, @rainers may you please have a brief look at http://wiki.dlang.org/Developing_DMD_/_Phobos_on_Windows_8 when you have few spare minutes? I'd really can use some review from more experienced windows users.

I'm a bit sceptic that using a unix environment will be attractive for many Windows users. There is also http://wiki.dlang.org/Building_DMD, which uses batch commands, but is probably slightly dated. I don't use the Express Version, so I cannot verify the steps exactly. I believe, the Pro Version is close enough, though. I hope I'll find some time the next couple of days...

mihails-strasuns commented 9 years ago

Well as disclaimer says it is more intended for unix developers who are forced to debug auto-tester failures on Windows ;) http://wiki.dlang.org/Building_DMD covers win32 build and was not really helpful for me when trying to figure out this.