Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add win64.mak #5694

Merged
merged 2 commits into from
Apr 24, 2016
Merged

Add win64.mak #5694

merged 2 commits into from
Apr 24, 2016

Conversation

CyberShadow
Copy link
Member

@CyberShadow CyberShadow commented Apr 23, 2016

Add support for building a 64-bit DMD using Microsoft Visual C++ without msbuild or needing to duplicate the project structure.

This is done by adding wrappers for the DigitalMars C compiler and librarian, which translate the command-line parameters to the Microsoft equivalents.

With this PR, I can build a 64-bit dmd.exe using just the following commands:

"C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\vcvarsall.bat" amd64
make -f win64.mak "HOST_DC=C:\Downloads\!dmd\dmd.2.068.2\dmd2\windows\bin\dmd.exe" clean release

This change is needed for Digger be able to to reliably build a 64-bit DMD as well, because the msbuild solutions currently available always use the system-wide MSVC install, ignoring environment variables.

CC @rainers

Add support for building a 64-bit DMD using Microsoft Visual C++
without msbuild or needing to duplicate the project structure.
This is done by adding wrappers for the DigitalMars C compiler
and librarian, which translate the command-line parameters
to the Microsoft equivalents.
@Geod24
Copy link
Member

Geod24 commented Apr 23, 2016

Any chance we can move away from having scripts / build scripts from DMD's source directory ?
I would suggest scripts/vcbuild/ or something along the line.

@CyberShadow
Copy link
Member Author

CyberShadow commented Apr 23, 2016

Any chance we can move away from having scripts / build scripts from DMD's source directory ?

Do you mean, moving them out into a separate repository?

That would indubitably be a bad idea, as it would be harder to keep them in sync.

Or do you mean to just move them out of the src directory, and into a directory under the repository root?

What would be the advantage, and how would it offset the cost of such a change?

Regardless, this is orthogonal to the change presented here, since it is comparably a small addition to the vcbuild stuff.

@Geod24
Copy link
Member

Geod24 commented Apr 23, 2016

What would be the advantage, and how would it offset the cost of such a change?

What would be the cost ?
The advantage would be a less cluttered source tree. That would mean no more false positive for tools using a directory (grep -nR for example), and using the frontend as a library would require less special casing.

Or do you mean to just move them out of the src directory, and into a directory under the repository root?

Yes. However I forgot we already have stuff in vcbuild, so it's indeed orthogonal and I rest my case :)

@CyberShadow
Copy link
Member Author

What would be the cost ?

Breaking tools and scripts that depend on the current layout. Plus actually performing it. Any change has a cost.

@rainers
Copy link
Member

rainers commented Apr 24, 2016

Looks quite good to me. I just tried but noticed a couple of issues (trying to build with VS2013):

  • using a specific make from the command line didn't work for me, it was lost somewhere in the make calls (I try to keep dmd/dmc out of PATH, they contain too many bad executables). Adding dmd's bin-directory to PATH is a workaround.
  • make clean rebuilds the dependencies, i.e. the dmc wrappers. It should remove them instead
  • linking fails due to missing symbols: the files in OBJ_MSVC are never forwarded to win32.mak?
  • iasm.c produces warning: cl : Command line warning D9002 : ignoring unknown option '-Ae'
  • I also get libucrt.lib(hypot.obj) : error LNK2005: hypot already defined in glue.lib(iasm.obj), but that seems to be some confusion on my system (libucrt.lib should only be used by VS2015).

@CyberShadow
Copy link
Member Author

Thanks for trying it!

using a specific make from the command line didn't work for me, it was lost somewhere in the make calls (I try to keep dmd/dmc out of PATH, they contain too many bad executables). Adding dmd's bin-directory to PATH is a workaround.

Fixed, you can now specify which make program to use for recursive make invocations via MAKE=. (I don't think DM make allows querying it from within the makefile?)

make clean rebuilds the dependencies, i.e. the dmc wrappers. It should remove them instead

Fixed, thanks.

linking fails due to missing symbols: the files in OBJ_MSVC are never forwarded to win32.mak?

Broke this at the last second :) Fixed.

iasm.c produces warning: cl : Command line warning D9002 : ignoring unknown option '-Ae'

Fixed (by translating it to /EHa), though it seems to have no effect

I also get libucrt.lib(hypot.obj) : error LNK2005: hypot already defined in glue.lib(iasm.obj), but that seems to be some confusion on my system (libucrt.lib should only be used by VS2015).

I've tested it with the 2010 and 2013 vcvarsall scripts, and all seems to work now.

@yebblies
Copy link
Member

iasm.c produces warning: cl : Command line warning D9002 : ignoring unknown option '-Ae'

Fixed (by translating it to /EHa), though it seems to have no effect

The EH in iasm.c was removed ages ago, it should be fine to drop that flag completely.

@rainers
Copy link
Member

rainers commented Apr 24, 2016

Fixed, you can now specify which make program to use for recursive make invocations via MAKE=. (I don't think DM make allows querying it from within the makefile?)

I don't think so. It still needs forwarding inside win32.mak (I added it to DMDMAKE).

I also get libucrt.lib(hypot.obj) : error LNK2005: hypot already defined in glue.lib(iasm.obj),

This still happens for me if I use dmd and make from PATH, i.e. the default HOST_DMC=dmd. As this uses a previously built dmd.exe in the src folder, it is broken anyway. Specifying HOST_DC explicitely works now without the error.

@rainers
Copy link
Member

rainers commented Apr 24, 2016

This change is needed for Digger be able to to reliably build a 64-bit DMD as well, because the msbuild solutions currently available always use the system-wide MSVC install, ignoring environment variables.

I don't think VS is to blame here, but Visual D. It does not allow to easily switch the used DMD from the comand line. I have some msbuild compatible "Build customization" in the pipe line, though. This would be independent of Visual D and could make the dmd path configurable on the command line, too.

@CyberShadow
Copy link
Member Author

It still needs forwarding inside win32.mak (I added it to DMDMAKE).

I think that's not necessary, because win32.mak recurses only once, for which using the MAKE passed by win64.mak seems sufficient.

I don't think VS is to blame here, but Visual D. It does not allow to easily switch the used DMD from the comand line.

This problem existed from before the DDMD transition. AFAIK, msbuild is just tied to the system it's installed on, you can't unzip and use it like cl or MinGW.

I have some msbuild compatible "Build customization" in the pipe line, though.

You mean, integrating with msbuild or replacing it?

@rainers
Copy link
Member

rainers commented Apr 24, 2016

It still needs forwarding inside win32.mak (I added it to DMDMAKE).

I think that's not necessary, because win32.mak recurses only once, for which using the MAKE passed by win64.mak seems sufficient.

It recurses twice for the release target.

I don't think VS is to blame here, but Visual D. It does not allow to easily switch the used DMD from the comand line.

This problem existed from before the DDMD transition. AFAIK, msbuild is just tied to the system it's installed on, you can't unzip and use it like cl or MinGW.

Might be true when running msbuild.exe directly. It works when invoked through the IDE, i.e. building from the command line with devenv.exe. Also, newer version should follow the toolset selected in the project.

I have some msbuild compatible "Build customization" in the pipe line, though.

You mean, integrating with msbuild or replacing it?

Integration into the VC projects.

@CyberShadow
Copy link
Member Author

It recurses twice for the release target.

Ah, OK. Added.

Might be true when running msbuild.exe directly. It works when invoked through the IDE, i.e. building from the command line with devenv.exe. Also, newer version should follow the toolset selected in the project.

But that would be a moot point since there's no way you can run the IDE without installing it, right? :) This is mainly from Digger's perspective, whose goal is to be able to build D on a fresh Windows install with nothing on it, where it downloads the necessary VS files, unpacks them on a subdirectory, and runs them from there.

@rainers
Copy link
Member

rainers commented Apr 24, 2016

But that would be a moot point since there's no way you can run the IDE without installing it, right? :) This is mainly from Digger's perspective, whose goal is to be able to build D on a fresh Windows install with nothing on it, where it downloads the necessary VS files, unpacks them on a subdirectory, and runs them from there.

Yes, true. I guess even msbuild doesn't work without being installed on the system.

@CyberShadow
Copy link
Member Author

Hmm, a dmd thus built crashes when building Phobos.

What's the odds that it's a dmd bug?

Stack trace:

    dmd.exe!toObjFile()  + 0x2362 bytes C++
>   dmd.exe!ddmd.visitor.Visitor.visit(ddmd.expression.CastExp * e=0x000001def5bcf490)  Line 1018 + 0x16 bytes  
    dmd.exe!ddmd.dimport.Import.accept(ddmd.visitor.Visitor * v=0x0000008dd99f7650)  Line 486 + 0x16 bytes  
    dmd.exe!toObjFile()  + 0x34 bytes   C++
    dmd.exe!genObjFile()  + 0x473 bytes C++
    dmd.exe!ddmd.mars.tryMain(const char * * argv=0x000001deef52de00, unsigned __int64 argc=0x0000000000000158)  Line 1587 + 0x13 bytes 
    dmd.exe!D main()  Line 1652 + 0x11 bytes    
    dmd.exe!_D2rt6dmain211_d_run_mainUiPPaPUAAaZiZ6runAllMFZ9__lambda1MFZv()  + 0x33 bytes  
    dmd.exe!_D2rt6dmain211_d_run_mainUiPPaPUAAaZiZ7tryExecMFMDFZvZv()  + 0x2e bytes 
    dmd.exe!_D2rt6dmain211_d_run_mainUiPPaPUAAaZiZ6runAllMFZv()  + 0x50 bytes   
    dmd.exe!_D2rt6dmain211_d_run_mainUiPPaPUAAaZiZ7tryExecMFMDFZvZv()  + 0x2e bytes 
    dmd.exe!_d_run_main()  + 0x463 bytes    
    dmd.exe!__entrypoint.main(int argc=0x00000158, char * * argv=0x000001deef52de00)  + 0x22 bytes  
    dmd.exe!__tmainCRTStartup()  Line 255 + 0x12 bytes  C
    kernel32.dll!BaseThreadInitThunk()  + 0x22 bytes    
    ntdll.dll!RtlUserThreadStart()  + 0x34 bytes    

@CyberShadow
Copy link
Member Author

@rainers I thought I'd try to use dmd.sln to see if this is a regression, but I can't build that with neither msbuild, devenv command-line or the IDE. In all three cases I get:

CUSTOMBUILD : error : The system was unable to find the specified registry key or value.

I've reinstalled Visual D to be sure but that had no effect.

@rainers
Copy link
Member

rainers commented Apr 24, 2016

What's the odds that it's a dmd bug?

IIRC there were some issues with dmd 2.068 regarding building for win64. Have you tried a later dmd version?

@rainers
Copy link
Member

rainers commented Apr 24, 2016

CUSTOMBUILD : error : The system was unable to find the specified registry key or value.

I've reinstalled Visual D to be sure but that had no effect.

I dont think Visual D is involved here. The message seems to come from the custom build steps of the backend VC project. Are you using VS2013 for this? I suspect VS2010 might not interpret the project correctly.

@CyberShadow
Copy link
Member Author

IIRC there were some issues with dmd 2.068 regarding building for win64. Have you tried a later dmd version?

Ah, thanks, that did it!

I dont think Visual D is involved here. The message seems to come from the custom build steps of the backend VC project. Are you using VS2013 for this? I suspect VS2010 might not interpret the project correctly.

Yeah, VS2013. I guess the install on my Windows machine is broken. Well, no matter.

@CyberShadow
Copy link
Member Author

Ah, thanks, that did it!

Added support for this to Digger:

digger -c build.components.dmd.dmdModel=64 build "master+dmd#5694"

CyberShadow/Digger@e52ffba
CyberShadow/ae@9831676...d3adf7e

@WalterBright
Copy link
Member

Auto-merge toggled on

@WalterBright WalterBright merged commit cf5645b into dlang:master Apr 24, 2016
newArgs ~= "/Zi";
break;
case "-wx": // "treat warnings as errors"
newArgs ~= "/WX";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to build with VS2015 and it produced some more warnings. This caused a build failure due to this option. I think we should allow MS warnings as long as building this is not run as part of the auto tester.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't a better solution be to add it to vcbuild\warnings.h?

I'd like us to test building this configuration. The VS build historically broke a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants