VS detection: fallback to lld-link and the mingw libs if no SDK/VC installation found by rainers · Pull Request #7500 · dlang/dmd

rainers · 2017-12-23T15:08:28Z

uses lld-link.exe alongside dmd.exe if no MS link.exe found
switches the default MS C runtime to msvcrt100 instead libcmt if no VC installation found
adds mingw subdir of lib64/lib32mscoff as a library search path if no Windows SDK found
uses the version of link.exe that is looks most appropriate regarding host and target architecture
improves detection whether legacy_stdio_definitions.lib needs to be added to linker options

dlang-bot · 2017-12-23T15:08:29Z

Thanks for your pull request, @rainers!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

skl131313 · 2017-12-23T15:39:05Z

If lld is going to be included now an alternative to optlink, changing the compiler flags to be uniform might be a good idea. So -m32 and -m64 use msvc/lld-link and then a third -m32optlink using optlink instead of having -m32mscoff.

PetarKirov · 2017-12-23T18:24:49Z

@skl131313 changing the meaning of existing command-line flags is not possible, since it would be a breaking change.

skl131313 · 2017-12-24T18:16:53Z

@ZombineDev
Not being possible and deterring from implementing something are two entirely different meanings. Breaking changes still get implemented (see integer promotion for + - ~). Optlink should be deprecated as no one is even willing to commit fixes for it and as a result is introducing limitations into DMD. This implements an alternative for it that can be shipped with DMD.

wilzbach · 2017-12-24T18:21:57Z

Optlink should be deprecated as no one is even willing to commit fixes for it and as a result is introducing limitations into DMD. This implements an alternative for it that can be shipped with DMD.

Yes, please. I don't use Windows, but I still hear from many problems about OPTLINK. As an example it only supports 32k symbols and thus we have to resort to crazy workarounds for Phobos: dlang/phobos#5939

wilzbach · 2017-12-29T13:10:18Z

2.078 will be released soon. Should this be part of it? I'm not sure. On one side these fixes look very important for people on Windows, on the other side there is a risk for regressions and we have already passed the beta release phase. Other opinions?

rainers · 2017-12-29T19:57:46Z

Yes, doesn't make much sense without also having dlang/installer#281 which is probably delayed until next release.

WalterBright · 2018-01-05T04:31:55Z

src/dmd/link.d

            }

+            // try lld-link.exe alongside dmd.exe
+            char[MAX_PATH + 1] dmdpath;


WalterBright · 2018-01-05T04:32:55Z

src/dmd/link.d

                    //  so prepend it too the PATH environment variable
                    const char* path = getenv("PATH");
-                    const char* idepath = FileName.combine(VSInstallDir, r"Common7\IDE");
                    auto pathlen = strlen(path);


const pathlen =

WalterBright · 2018-01-05T04:33:10Z

src/dmd/link.d

-                    const char* idepath = FileName.combine(VSInstallDir, r"Common7\IDE");
                    auto pathlen = strlen(path);
-                    auto idepathlen = strlen(idepath);
+                    auto addpathlen = strlen(addpath);


const addpathlen =

WalterBright · 2018-01-05T04:36:51Z

src/dmd/mars.d

+        if (global.params.mscoff && !global.params.mscrtlib)
+        {
+            VSOptions vsopt;
+            vsopt.initialize ();


no space before ()

rainers · 2018-01-05T19:27:25Z

Rebased and addressed review comments.

JinShil · 2018-01-17T08:36:42Z

Yes, doesn't make much sense without also having dlang/installer#281

Labeling as "Blocked" then.

wilzbach · 2018-01-23T22:07:59Z

@rainers just saw your post on the NG: http://forum.dlang.org/post/p4883m$1nv1$1@digitalmars.com

This PR #7500 would have fixed that by changing the detection to the existence of legacy_stdio_definitions.lib in the VC library path. Unfortunately it didn't make it into the release.

So should we go ahead with this PR? It looks like this PR already makes sense - even without the installer modifications.

rainers · 2018-01-23T22:21:24Z

I guess the largest side-effect would be https://github.com/dlang/dmd/pull/7500/files#diff-12efbb2a88b713dbb0fd0a64673ba132R475: if no VC is detected, the default runtime library is switched from libcmt to msvcrt100 which only exists with the updated installer.

I could extract that to a different PR, though.

wilzbach · 2018-02-06T15:27:23Z

Let's ping @MartinNowak to get the installed updated before the 2.079 release then.

MartinNowak · 2018-02-17T21:28:43Z

src/dmd/link.d

+            char[MAX_PATH + 1] dmdpath = void;
+            if (GetModuleFileNameA(null, dmdpath.ptr, dmdpath.length) <= MAX_PATH)
+            {
+                auto lldpath = FileName.replaceName(dmdpath.ptr, "lld-link.exe");


Likely will break if we shipped a 32-bit bin/lld.exe with a 64-bit bin64/dmd.exe.

We might also add a 64-bit lld.exe in that case.

rainers · 2018-02-19T22:03:02Z

src/dmd/link.d

+
+            // try mingw fallback relative to phobos library folder that's part of LIB
+            Strings* libpaths = FileName.splitPath(getenv("LIB"));
+            if (auto p = FileName.searchPath(libpaths, r"mingw\kernel32.lib", false))


This doesn't match the 2.079-beta1 installer that places all platform libraries alongside phobos64.lib. Was this a deliberate choice?

rainers requested a review from WalterBright as a code owner December 23, 2017 15:08

wilzbach added this to the 2.078.0 milestone Dec 29, 2017

wilzbach modified the milestones: 2.078.0, 2.079.0 Dec 29, 2017

wilzbach changed the base branch from stable to master December 29, 2017 20:05

MartinNowak force-pushed the vsdetect2 branch from 5a29ab2 to 344f57d Compare December 29, 2017 21:59

WalterBright reviewed Jan 5, 2018

View reviewed changes

src/dmd/link.d Outdated

}

// try lld-link.exe alongside dmd.exe

char[MAX_PATH + 1] dmdpath;

Copy link

Member

WalterBright Jan 5, 2018

Choose a reason for hiding this comment

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

= void;

WalterBright reviewed Jan 5, 2018

View reviewed changes

rainers force-pushed the vsdetect2 branch from 344f57d to c1d979a Compare January 5, 2018 19:26

wilzbach mentioned this pull request Jan 8, 2018

Old PRs are on the priority queue braddr/d-tester#59

Closed

JinShil added the Merge:Blocked label Jan 17, 2018

wilzbach closed this Jan 23, 2018

wilzbach reopened this Jan 23, 2018

rainers mentioned this pull request Feb 1, 2018

fix issue 18354 - [Reg 2.078] Building fails with VC 2015 Build Tools #7828

Merged

MartinNowak changed the base branch from master to stable February 17, 2018 21:12

MartinNowak approved these changes Feb 17, 2018

View reviewed changes

MartinNowak force-pushed the vsdetect2 branch from c1d979a to e3980a8 Compare February 17, 2018 21:39

rainers requested a review from mathias-lang-sociomantic as a code owner February 17, 2018 21:39

rainers added 6 commits February 17, 2018 22:48

use linker binary according to host and target architecture

b0d0190

if link.exe not found, try lld-link.exe alongside dmd.exe

33feb69

if no windows SDK found, add fallback to mingw subfolder of lib

e1f7e8d

switch default MS runtime library name if no VC environment detected

64e3aff

fix compile error

5e81834

code styling

4eb526e

MartinNowak force-pushed the vsdetect2 branch from e3980a8 to 4eb526e Compare February 17, 2018 21:48

MartinNowak added Merge:auto-merge and removed Merge:Blocked labels Feb 17, 2018

dlang-bot merged commit 5807781 into dlang:stable Feb 18, 2018

rainers commented Feb 19, 2018

View reviewed changes

Uh oh!

Comments

Conversation

rainers commented Dec 23, 2017

Uh oh!

dlang-bot commented Dec 23, 2017

Bugzilla references

Uh oh!

skl131313 commented Dec 23, 2017

Uh oh!

PetarKirov commented Dec 23, 2017

Uh oh!

skl131313 commented Dec 24, 2017

Uh oh!

wilzbach commented Dec 24, 2017

Uh oh!

wilzbach commented Dec 29, 2017

Uh oh!

rainers commented Dec 29, 2017

Uh oh!

WalterBright Jan 5, 2018

Choose a reason for hiding this comment

Uh oh!

WalterBright Jan 5, 2018

Choose a reason for hiding this comment

Uh oh!

WalterBright Jan 5, 2018

Choose a reason for hiding this comment

Uh oh!

WalterBright Jan 5, 2018

Choose a reason for hiding this comment

Uh oh!

rainers commented Jan 5, 2018

Uh oh!

JinShil commented Jan 17, 2018

Uh oh!

wilzbach commented Jan 23, 2018

Uh oh!

rainers commented Jan 23, 2018

Uh oh!

wilzbach commented Feb 6, 2018

Uh oh!

MartinNowak Feb 17, 2018

Choose a reason for hiding this comment

Uh oh!

rainers Feb 19, 2018

Choose a reason for hiding this comment

Uh oh!

rainers Feb 19, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants