premake / premake-core

Premake
https://premake.github.io/
BSD 3-Clause "New" or "Revised" License
3.18k stars 616 forks source link

linkoptions also being passed to librarian in vs2012 #150

Open dgregorycryptic opened 9 years ago

dgregorycryptic commented 9 years ago

Is this intentional, because it seems like a bug?

For instance, I'm passing:

    linkoptions "/NODEFAULTLIB:libc.lib"
starkos commented 9 years ago

We might need a little more information here. Can you provide a small sample project that reproduces the problem?

samsinsane commented 9 years ago

@dgregorycryptic linkoptions should definitely be passed through to the librarian, you can specify additional link options under Librarian -> Command Line -> Additional Options (In VS2013, assuming it's the same location for VS2012). So I would say this is working as intended, additionally, based off the options in Librarian -> General, specifying /NODEFAULTLIB:libc.lib is also valid. Also, I noticed that it's not possible to use the ignoredefaultlibraries API, so I'll submit a fix for that and any other APIs that are available in the librarian. :)

@starkos while hunting this down, I've come across some weirdness. In vs2010_vcxproj.lua, there's two functions m.link and m.lib, these appear like they should actually be merged together, as a static lib doesn't seem to use the output of m.link and only a static lib has output from m.lib. I'll start working on a PR for this, unless there's a reason these shouldn't be merged?

dgregorycryptic commented 9 years ago

It is not the case that every linker command line option is proper to send the the librarian tool (at least, in Visual Studio). So merging the two together seems wrong.

Additional Options that you enter under Librarian -> Command Line -> Additional Options are only passed to the librarian tool, not the linker.

I chose an option that happened to be shared between the two. Perhaps that is confusing the issue.

How about /OPT:REF or /OPT:ICF? Those need to be able to be sent to just the linker, never the librarian. At the moment, the only way to make that happen is to filter the setting of linkoptions.

starkos commented 9 years ago

there's two functions m.link and m.lib, these appear like they should actually be merged together

m.lib is for static libraries, and corresponds to the <Lib> XML element. m.link is for everything, and corresponds to the <Link> element.

samsinsane commented 9 years ago

Sorry guys, I re-read what I posted and in my attempt to not post a big wall of text, I ended up removing some important information. Firstly, I used VS2013 and one of the contrib projects for testing, so there's a chance that what I've found is different for earlier versions of Visual Studio.

Once everything was generated, I checked what the vcxproj contained, in regards to m.lib and m.link. This is what I found for Debug|Win32:

    <Link>
      <SubSystem>Windows</SubSystem>
      <GenerateDebugInformation>true</GenerateDebugInformation>
    </Link>

Upon opening the project properties in VS and navigating to Librarian->General, neither of these options were set. Setting one of them and saving the project, results in the following output to the vcxproj:

    <Link>
      <SubSystem>Windows</SubSystem>
      <GenerateDebugInformation>true</GenerateDebugInformation>
    </Link>
    <Lib>
      <SubSystem>Windows</SubSystem>
    </Lib>

(GenerateDebugInformation isn't in the Librarian in VS2013, this seems like an invalid output?)

This is what makes me believe we should merge those two functions together, and just change the parent element tag output (<Link> vs <Lib>). Both of the element functions m.elements.link and m.elements.lib contain a specific check for static libs, and both are populated with outputs (different outputs, except for the additional options which exists for both).

The only thing I'm unsure on is what other versions of VS do in terms of these elements. Is there a version of VS that uses both for a static library, or is this something that can be simplified? Either way, there's API's, such as ignoredefaultlibraries (m.ignoreDefaultLibraries), that should be added to the static library outputs as they're valid options.

samsinsane commented 8 years ago

@starkos have you had a chance to read this wall of text? I'm happy to put together a PR as an example of what I mean by merging the functions together, if that helps?

starkos commented 8 years ago

This is what makes me believe we should merge those two functions together, and just change the parent element tag output ( vs ).

I guess I'm not sure what you are going to merge, as both functions are basically just shells around a call array. I would guess (don't have the right VM here to look) that I created two functions to make it easier to override static/shared output for specific cases. I would bet that somewhere I am suppressing <Lib> entirely for a target platform.

This is clearly a separate issue from the bugs though; it sounds like you've identified a couple of those that should get fixed.

starkos commented 8 years ago

Reopening this one, see discussion on PR #440. I'll revisit it with a narrower fix.

starkos commented 8 years ago

I'm revisiting this now, and could still use some additional information. Can you post a short sample project that demonstrates the problem? (In particular, I need to know what kind of projects you are trying to generate.)

Also, for the specific case you mentioned, does ignoredefaultlibraries { "libc" } work?

dgregorycryptic commented 8 years ago

I have not checked the newer ignoredefaultlibraries command.

When I submitted this originally, it was the case that linkoptions was being passed not only to the linker, but also the librarian.

That on the surface feels wrong. Like I said above on 9/15/2015, not every linker option is valid to send to the librarian.

starkos commented 8 years ago

Then perhaps we need a new API. I would like to have some real examples in hand before I try to implement any kind of solution.

If you feel this is no longer a problem for you, feel free to close it out. Thanks!