Simplify Visual C configuration for -m64/-m32mscoff builds #2526
Conversation
CyberShadow
force-pushed
the
pull-20140917-172747
branch
2 times, most recently
from
6295f3e to
bd510df
Compare
|
ping @braddr |
|
See discussion in Druntime pull: |
CyberShadow
force-pushed
the
pull-20140917-172747
branch
from
bd510df to
a04acd6
Compare
CyberShadow
changed the title
|
ping |
|
Is is still actual and blocked? |
Yes. Latest build on the auto tester is just a couple of days ago with just the same merge problem. |
|
Current win64.mak contains targets phobos32mscoff and unittest32mscoff, though. So the coff32 part of this PR is probably no longer necessary. |
|
ping |
|
ping pong - this is now officially the oldest open Phobos PR. |
|
Again, see discussion in Druntime pull: It's blocked by @braddr doing the necessary changes to the autotester. See https://issues.dlang.org/show_bug.cgi?id=14381 |
|
@CyberShadow this is passing the auto-tester. Does that mean the necessary changes have been made and this can go ahead? If not, please close as there has been no action on this PR for 3 years and counting. |
|
Wow, I never thought I'd see the day. Yes, this is a simple refactoring. It was failing the autotester because it was conflicting with a patch that the autotester was applying before running this makefile. The very fact that the autotester needs to patch the makefile shows that there is a problem with it. However, if this PR now passes the autotester, it means that it no longer needs to patch anything, or at least the patch is no longer incompatible with this PR. In any case, this PR should simplify some use cases of building Phobos on Windows, so I still think it's a good change. |
|
CC @rainers @marler8997 - would this help you? |
|
Yes it provides a nice set of hooks to correctly specify where visual C is installed and what version to use. |
Not personally, as my build script passes the existing variables on the command line anyway. That's pretty ugly if they contain spaces, though. This PR has been waiting so long to be merged that it is now incompatible with the current Visual Studio 2017. As this is the only version downloadable on the Microsoft pages without login it would be good if it could be supported, too. |
|
@CyberShadow what should we do with this PR? It might be that a rebase will pass the autotester, however, as @rainers noticed:
What do you think? |
|
Among other things that occurred during the seven years since this PR was created, I don't use Windows any more, so if there is interest then I think it's better if someone else continued this. |
|
I have created a bug for this [1] so that it's not forgotten. |
Ping @rainers