[feature] Add wholearchive#2602
Conversation
src/_premake_init.lua
Outdated
| scope = "config", | ||
| kind = "string", | ||
| allowed = { | ||
| "Off", |
There was a problem hiding this comment.
If it's just on/off, it should be a boolean and not a string.
There was a problem hiding this comment.
We should probably have "Default" as well, because we should probably allow users to specify to use the default toolset/exporter behavior, rather than enforcing one behavior or another.
There was a problem hiding this comment.
I used linkgroups as model :-/
Ideally, I wanted libraries as well, but too lazy to do it, so currently an all or nothing.
Implementation would be more "On" or "Default" BTW :-)
There was a problem hiding this comment.
Not checked if we can have boolean or string (for individual library).
There was a problem hiding this comment.
Changed to have list of strings instead of an ON/OFF flags.
There was a problem hiding this comment.
Note:
Instead of
link { 'my_library' }
wholearchive { 'my_library' }
I also think about alternative API (similar to existing :static/:shared suffix syntax):
link { 'my_library:wholearchive' }
but it seems team's preferences go for the first syntax.
src/tools/msc.lua
Outdated
|
|
||
| if cfg.wholearchive == p.ON then | ||
| -- Don't use "/WHOLEARCHIVE" as path.translate might be applied to result | ||
| links = table.translate(links, function (s) return "-WHOLEARCHIVE:" .. s end) |
There was a problem hiding this comment.
Is the path.translate issue something that you did indeed find to be a problem? Or is this pre-emptive?
There was a problem hiding this comment.
I encounter it in visual.
Using - as workaround is rejected in commandline though (as ninja).
I think we have to specify 'shell' and translate path here.
website/docs/wholearchive.md
Outdated
| @@ -0,0 +1,38 @@ | |||
| Apply whole archive flag to linked libraries. | |||
There was a problem hiding this comment.
Reword suggestion: Controls whether the linker should include all objects in the linked binary.
There was a problem hiding this comment.
Reworded to "Command linker to include all objects of given libraries."
src/tools/msc.lua
Outdated
|
|
||
| if cfg.wholearchive == p.ON then | ||
| -- Don't use "/WHOLEARCHIVE" as path.translate might be applied to result | ||
| links = table.translate(links, function (s) return "-WHOLEARCHIVE:" .. s end) |
There was a problem hiding this comment.
Also, this should probably go into the linker flags, rather than the getlinks functionality. Also, I think the proper way to do this would be: link.exe foo.lib /WHOLEARCHIVE:foo.lib -- Following up on this, if we don't support granular whole archive, we can just have a single /WHOLEARCHIVE without specifying the individual libs.
There was a problem hiding this comment.
I might try that.
if we don't support granular
I hope to add that support.
There was a problem hiding this comment.
Changed to have granular support.
I follow the suggestion and use linker flags instead of getlinks functionality.
Jarod42
force-pushed
the
whole_archive
branch
2 times, most recently
from
5da4ce3 to
d238cd6
Compare
Jarod42
force-pushed
the
whole_archive
branch
7 times, most recently
from
a8a5149 to
d6c8540
Compare
| | [vectorextensions](vectorextensions.md) | Enable hardware vector extensions | | ||
| | [vpaths](vpaths.md) | | | ||
| | [warnings](warnings.md) | | | ||
| | [wholearchive](wholearchive.md) | | |
There was a problem hiding this comment.
Out of scope of this PR, but I feel like I always forget to add things here. We should probably check to make sure all of the APIs are actually here.
There was a problem hiding this comment.
Or remove it completely, as it doesn't bring more information that reference listing on the left panel.
I also note that for some pages, including Project-API, we lose the left panel.
nickclark2016
left a comment
nickclark2016
left a comment
There was a problem hiding this comment.
Verified build in my local project and no issues readily presented.
2 participants
What does this PR do?
Add
wholearchiveAPI which correspond to link option/WHOLEARCHIVE--whole-archiveof ld-force_loadHow does this PR change Premake's behavior?
Add an API
Anything else we should know?
Tested with https://github.com/Jarod42/premake-sample-projects/tree/wholearchive/projects/wholearchive
Did you check all the boxes?
closes #XXXXin comment to auto-close issue when PR is merged)