Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Simplify Visual C configuration for -m64/-m32mscoff builds#960

Closed
CyberShadow wants to merge 1 commit intodlang:masterfrom
CyberShadow:pull-20140917-172401
Closed

Simplify Visual C configuration for -m64/-m32mscoff builds#960
CyberShadow wants to merge 1 commit intodlang:masterfrom
CyberShadow:pull-20140917-172401

Conversation

@CyberShadow
Copy link
Member

Ping @rainers

@CyberShadow
Copy link
Member Author

Crap, why is this failing? What make variant does the autotester use?

@CyberShadow
Copy link
Member Author

Why is this failing?!? I can't reproduce this with any make variant I have on this computer. @braddr?

@CyberShadow
Copy link
Member Author

Searching for the error message shows that it is emitted by DigitalMars make, but obviously it doesn't happen here. Does the autotester patch the make files in some way?

@CyberShadow
Copy link
Member Author

@rainers
Copy link
Member

rainers commented Sep 19, 2014

Sorry for being late at commenting, I could have warned about the makefile being patched.

I'm not sure making the sub-directory a dependent option is the best approach. How about specifying the bin path as patched by the build-server? BTW: link/lib are not used, so specifying CC on the command line is good enough. That is what I am doing.

Regarding errno.c: I'd rather like to see the object files in different directories (that will need more patches, though). I don't think building works across platforms without cleaning up yet. At least, you'll also have to patch the zlib makefile in phobos aswell to make that work.

@braddr
Copy link
Member

braddr commented Sep 20, 2014

The two changes (druntime and phobos) should be essentially the same rather than subtly different in how VCBINDIR is defined. I'll deal with cleaning up the auto-tester end of things. This ought to allow me to remove one of the last reasons there's an external diff applied at all. Something I should have done ages ago.

@CyberShadow
Copy link
Member Author

I'm not sure making the sub-directory a dependent option is the best approach. How about specifying the bin path as patched by the build-server?

That's another way to do it. I was thinking that splitting up the path parts would allow e.g. compiling for 32 and 64 bits in the same makefile in the future, or easily choosing a VS version (e.g. VC=11).

The two changes (druntime and phobos) should be essentially the same rather than subtly different in how VCBINDIR is defined.

Yeah, I was going back and forth trying different things while trying to figure out the autotester failure. I considered that the leading backslash was triggering some special syntax.

This ought to allow me to remove one of the last reasons there's an external diff applied at all.

One effect of the autotester patch is that it removes the definition from the makefiles completely, causing make to read the variables from the environment. Since we still want a default value for backwards compatibility, without patching the makefiles the bin path will now need to be passed as a parameter to make. Is that OK?

@braddr
Copy link
Member

braddr commented Sep 20, 2014

I really don't want to have to have make invoked differently based on OS. Adding VCBINDIR to all make invocations wouldn't be the worst thing in the world, but it'd be really dumb to include for non-win64 environments. The best we can do with the digitalmars make, given its macro rule logic and inability to do this sort of default setting, is to also set MAKEFLAGS to "VDBINDIR=path" in the environment. I've never tried that, but it ought to work.

@CyberShadow
Copy link
Member Author

BTW: link/lib are not used, so specifying CC on the command line is good enough.

lib appears to be used when building zlib, no? Or it doesn't matter which bitness the librarian it is?

I really don't want to have to have make invoked differently based on OS.

I noticed the build scripts are already customizing Make's command line (EXTRA_ARGS), so I guess one option would be something like:

if [ -n "$VCBINDIR" ] then; EXTRA_ARGS="VCBINDIR=$VCBINDIR"; fi

But MAKEFLAGS sounds better.


Anyway, updated. Now there is VC (to easily switch VC versions if desired), VCDIR (backwards compatibility), VCBIN_SUBDIR (for easy m32mscoff or cross-compile builds), and VCBIN_DIR a la autotester. Note that VCDIR also dictates the C include directory.

win64.mak Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Move at least the 'bin' part ouf of VCBIN_DIR into VCBIN_SUBDIR. Maybe the \ as well. If you're going to make it overridable, don't go just half way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why? I don't think it's even possible to install VS in a way that this would ever be useful, and for those cases you can just override VCBIN_DIR instead.. It would just make the shortest command line for m32mscoff or cross-compilation longer than necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Why? I don't think it's even possible to install VS in a way that this would ever be useful

Actually I have an extracted a few files from VS2010 to get a small reference version that works building and debugging phobos (65 MB), and the folders are actually named differently ;-)
I think using VCBIN_SUBDIR=bin is even a bit more obvious than specifying an empty argument.

@CyberShadow CyberShadow changed the title Finish win64.mak support for the 32mscoff model win64.mak: Simplify Visual C configuration for -m64/-m32mscoff builds Oct 15, 2014
@CyberShadow CyberShadow changed the title win64.mak: Simplify Visual C configuration for -m64/-m32mscoff builds Simplify Visual C configuration for -m64/-m32mscoff builds Oct 15, 2014
@CyberShadow
Copy link
Member Author

OK, updated.

@rainers
Copy link
Member

rainers commented Oct 15, 2014

LGTM, though I'd still recommend to put "/.obj" into .gitignore instead of "/errno_c.obj" to exclude unittest.obj. Other refactorings can be added later.

@braddr I guess this should now be good enough to run the test builds without patching the makefile. If you can adjust the tester, please pull.

@rainers
Copy link
Member

rainers commented Nov 22, 2014

ping @braddr

@quickfur
Copy link
Member

ping

Copy link
Member

Choose a reason for hiding this comment

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

You should use MODEL in the make file to set the VCBIN_SUBDIR.
Not sure if it can be done though, maybe double expansion works.
VCBIN_SUBDIR_64=bin\x86_64
VCBIN_SUBDIR=$(VCBIN_SUBDIR_$(MODEL))

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, DM make cannot do that.

@MartinNowak
Copy link
Member

I know little to nothing about VC integration. What problem does this pull solve?

@MartinNowak MartinNowak self-assigned this Feb 16, 2015
Also fix rebuilding without clean due to errno_c.obj
@CyberShadow CyberShadow force-pushed the pull-20140917-172401 branch from e041523 to 72b6199 Compare March 24, 2015 00:57
@CyberShadow
Copy link
Member Author

I know little to nothing about VC integration. What problem does this pull solve?

Makes it easier to build -m32mscoff Phobos/Druntime, or cross-compile 32-bit to 64-bit.

@MartinNowak
Copy link
Member

Waiting for @braddr to do something with the auto-tester. We really need more people that can maintain the auto-tester, because Brad is a busy person and we too often get stuck.

@wilzbach
Copy link
Contributor

wilzbach commented Apr 26, 2016

Waiting for @braddr to do something with the auto-tester. We really need more people that can maintain the auto-tester, because Brad is a busy person and we too often get stuck.

At mir we combined AppVeyor and Travis to test for Win32,Win64,Linux32,Linux64 and OS X 64 (OS X 32 has been deprecated for a couple of years).
At ldc they started to experiment with buildbot. Jenkins also supports distributed building...

ping @braddr

@wilzbach
Copy link
Contributor

ping @braddr - do you have time to update the auto-tester?

@dlang-bot dlang-bot added Needs Rebase needs a `git rebase` performed stalled Needs Work labels Jan 1, 2018
@Geod24
Copy link
Member

Geod24 commented Jul 9, 2022

Druntime have been merged into DMD. Please re-submit your PR to dlang/dmd repository.

@Geod24 Geod24 closed this Jul 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants