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 LIBPATH64 environment variable #2509

Closed
wants to merge 1 commit into from

Conversation

WalterBright
Copy link
Member

This enables the following to be put in sc.ini:

LIBPATH64=%VCINSTALLDIR%lib\amd64;%WindowsSdkDir%lib\x64

and removes the dependency in the dmd's construction of the command to the linker on the specific path tails to the VS libraries.

@brad-anderson
Copy link
Member

I think it'd be nice if the sc.ini you used for rolling the zip file were included somewhere in the repo.

@brad-anderson
Copy link
Member

Will this be what the shipped sc.ini will be in 2.64?

[Version]
version=7.51 Build 020

[Environment]
LIB="%@P%\..\lib";\dm\lib
DFLAGS="-I%@P%\..\..\src\phobos" "-I%@P%\..\..\src\druntime\import"
LINKCMD=%@P%\link.exe
LINKCMD64=%VCINSTALLDIR%bin\amd64\link.exe
LIBPATH64=%VCINSTALLDIR%lib\amd64;%WindowsSdkDir%lib\x64

@WalterBright
Copy link
Member Author

Will this be what the shipped sc.ini will be in 2.64?

At the moment, yes.

@DmitryOlshansky
Copy link
Member

Lookin' good

@brad-anderson
Copy link
Member

Ok, good. This pull makes installer#22 possible.

@DmitryOlshansky
Copy link
Member

BTW what happened with Environment64 - why LIB64 separately?

@TurkeyMan
Copy link
Contributor

Ditto, why LIBPATH64, rather than [Environment64]?

What about splitting phobos64.lib, gcstub.obj out into their own lib64/ directory?

Also, VCINSTALLDIR doesn't seem to exsist on my machine.

@brad-anderson
Copy link
Member

VCINSTALLDIR is set by the "Visual Studio Command Prompt (201X)" shortcut Visual Studio adds to the start menu. If you use that both VCINSTALLDIR and WindowsSdkDir will be set.

Now for my own slightly related question, how can I go about adding a 64-bit libcurl? I made the 32-bit OMF curl.lib appear in dmd2/windows/lib when I added that to the install process. Would I rename the 64-bit import library curl64.lib? I'm not sure how this library selection process works.

@rainers
Copy link
Member

rainers commented Sep 2, 2013

I think all the "magic" code regarding library paths should be removed from the compiler and simply added to sc.ini, e.g.

[Environment64]
LIB=%@p%....\lib64;%VCINSTALLDIR%\lib\amd64;%WindowsSdkDir%\lib\x64

Please note the additional , it seems there is sometimes no trailing \ for the variables, maybe related to changing the installation path.

Having a separate library folder is also very much recommended. It solves the curl.lib problem, aswell as that you won't need different pragma(lib) statements depending on platform. Having OMF libraries in the search path is just asking for trouble.

To remove special cases, I'd recommend to also not use LINKCMD64, but LINKCMD set in [Environment64]. /NOLOGO can be set there aswell, e.g.

[Environment64]
DFLAGS=%DFLAGS% -L/NOLOGO

@WalterBright
Copy link
Member Author

I've resisted the [Environment64] idea because it adds complexity, and I think the problem we're trying to solve is not complicated. dmd needs only two things when building a 64 bit executable - where the linker is, and where the libraries are. LINKCMD64 and LIBPATH64 do that.

As for putting 64 bit libs in a separate directory, that is something orthogonal to this PR - discussion of it should go into an enhancement request on bugzilla.

Now for my own slightly related question, how can I go about adding a 64-bit libcurl?

Unrelated discussions really should not go in this PR thread. Please repost in the n.g.

@WalterBright
Copy link
Member Author

@TurkeyMan
Copy link
Contributor

@eco:

VCINSTALLDIR is set by the "Visual Studio Command Prompt (201X)" shortcut Visual Studio adds to the start menu. If you use that both VCINSTALLDIR and WindowsSdkDir will be set.

Yes, but people don't use that. I've been using visual studio for more than 15 years, that's been there since the first time I used it, and I've NEVER opened it. I've never heard of anyone using it for anything.
What are you suggesting people do with it?

Re: libcurl64.lib, surely it's best to have lib/ and lib64/ path separated, then you don't need to be putting '64' in the lib names.

@TurkeyMan
Copy link
Contributor

@WalterBright Yes I'm aware of the bat file, but I don't know how it helps... people don't use the shell in windows.
They're not set as system environment variables, so they're not defined in most places.
Are you planning to use that batch file as a launchpad for DMD, Ie, execute DMD via that bat file?

I'd also like to support @rainers that [Environment64] makes way more sense to me than LINKCMD64.

@rainers
Copy link
Member

rainers commented Sep 2, 2013

I've resisted the [Environment64] idea because it adds complexity, and I think the problem we're trying to solve is not complicated. dmd needs only two things when building a 64 bit executable - where the linker is, and where the libraries are. LINKCMD64 and LIBPATH64 do that.

Please note that MS-link also uses the LIB environment variable, so you have to override it if you don't want to interfere with OMF libraries. There's no way you can do that with LIBPATH64. You might also need to change other environment variables like PATH to let the linker find dlls (e.g. needed when building with debug info).

If we add COFF support to the 32-bit code generation (https://github.com/rainers/dmd/tree/coff32) the section based solution already covers that, while you'd have to add even more variables.

@WalterBright
Copy link
Member Author

Yes, but people don't use that. I've been using visual studio for more than 15 years, that's been there since the first time I used it, and I've NEVER opened it. I've never heard of anyone using it for anything.

Do you mean that VS programmers NEVER use the command line? If they use the command line, then VCINSTALLDIR is set, whether or not they know it.

What are you suggesting people do with it?

The only use for it is dmd uses it to find the linker and the libraries. It does need some way to find them. If they're not there, then the user can type into sc.ini where they are. If the user does not know where VS puts the linker and the libraries, then there are command line tools to find them. If it's 6 hours in and the user still can't find the linker, then it's way past time for him to ask for help :-(

I guess one of the things that makes life easy for new programmers is VS hides everything from them - including what and where things are. That's exactly what makes using VS hard for me - it hides everything. I'm presented with a blizzard of environment variables with no explanation for any of them. I don't know what it is doing, where it is getting its files from, where it is putting the output files, etc., and that just makes me very uneasy.

It did take me a few minutes to find where VS put the linker and the libs, but not hours.

@WalterBright
Copy link
Member Author

Please note that MS-link also uses the LIB environment variable, so you have to override it if you don't want to interfere with OMF libraries.

This hasn't caused me any issues, but I can see how it might, and your other points are valid, too. I also haven't had any issues when building with debug on.

Also, the last time [Environment64] came up, there were even more extensions proposed, essentially designing Yet Another Scripting Language that needed to be parsed. I suggested instead that we instead switch to a JSON format for sc.ini and then use a json file parser. That would be the last file parser we'd need to write.

But until we do that, I suggest just pull this one and at least get this bit working, as it doesn't change the sc.ini format at all.

@rainers
Copy link
Member

rainers commented Sep 2, 2013

This hasn't caused me any issues, but I can see how it might, and your other points are valid, too.

As long as the COFF libraries are found it is ok, but if not you might get cryptic errors regarding bad format, crashes, etc.

Also, the last time [Environment64] came up, there were even more extensions proposed, essentially designing Yet Another Scripting Language that needed to be parsed. I suggested instead that we instead switch to a JSON format for sc.ini and then use a json file parser. That would be the last file parser we'd need to write.

With the modifications for the installer, the discussed extensions are not really necessary anymore, though some prefer the zip-file installation. The ini file format is ok, I don't think changing it has any value.

But until we do that, I suggest just pull this one and at least get this bit working, as it doesn't change the sc.ini format at all.

I'm hesitent to do that because IMO it is going in the wrong direction and will probably stay again for too long. It will probably also break the auto tester, the used sc.ini needs to be adapted, too. Even better: we should add the sc.ini that goes into the release to the dmd repository so that the auto tester does not need to have its own copy.

@rainers
Copy link
Member

rainers commented Sep 2, 2013

so that the auto tester does not need to have its own copy.

Just noticed that this won't work as long as the released folder structure is different from the one used in the development process. The auto tester also adds some custom paths (e.g. to find the curl libraries).

@WalterBright
Copy link
Member Author

It will probably also break the auto tester, the used sc.ini needs to be adapted, too.

No, it won't break anybody's sc.ini. If LIBPATH64 is not set, then dmd reverts to the previous method it used.

@WalterBright
Copy link
Member Author

BTW, the autotest failures make no sense. This patch changes nothing in the Linux path, for example.

@ghost
Copy link

ghost commented Sep 2, 2013

Master has been broken all day in case you didn't notice: http://d.puremagic.com/test-results/?projectid=1

This is what happens when people merge pulls without waiting for the autotester.

@rainers
Copy link
Member

rainers commented Sep 3, 2013

No, it won't break anybody's sc.ini. If LIBPATH64 is not set, then dmd reverts to the previous method it used.

Sorry, I misread the diff and stand corrected. Even so, this PR just adds another special case for something that can already be done by setting LIB in [Environment64].

@MartinNowak
Copy link
Member

Even so, this PR just adds another special case for something that can already be done by setting LIB in [Environment64].

Right, I don't understand the thought behind this either.
What's the problem with the following?

[Environment]
DFLAGS="-I%@P%\..\..\src\phobos" "-I%@P%\..\..\src\druntime\import"

[Environment32]
LIB="%@P%\..\lib";\dm\lib
LINKCMD=%@P%\link.exe

[Environment64]
LIB="%@P%\..\lib";\dm\lib;%VCINSTALLDIR%lib\amd64;%WindowsSdkDir%lib\x64
LINKCMD=%VCINSTALLDIR%bin\amd64\link.exe

@MartinNowak
Copy link
Member

I'm closing this pull request. Just update the sc.ini with a correct LIB definition in [Environment64] please.

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.

6 participants