-
-
Notifications
You must be signed in to change notification settings - Fork 640
Added ability for toolsets to specify tool output extensions #2509
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
Added ability for toolsets to specify tool output extensions #2509
Conversation
| global(nil) | ||
| end | ||
|
|
||
| function cpp.gettooloutputext(tool, cfg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why gcc/clang don't provide it too?
Or move that function as helper method (usable by other toolset (as ninja))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why gcc/clang don't provide it too?
I couldn't decide if I should or not, but I'm leaning towards I should now.
This function needs to exist though with the fallback, otherwise all modules that introduce C and C++ toolsets won't use extensions on their files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
src/tools/msc.lua
Outdated
| -- | ||
|
|
||
| function msc.gettooloutputext(tool) | ||
| return iif(tool == "rc", "res", "obj") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path.getextension (as several programming languages (C++, python)) returns the dot too, seems more coherent to return '.res'/'.obj'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I considered this but I opted for this as the the usage would look weird otherwise - %{}%{} instead of %{}.%{}.
I don't really feel that strongly either way, and it does look like we have precedent to include the dot. @nickclark2016 thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically speaking, the extension does not include the dot. I would honestly prefer we "break" existing code to fix that, but that's probably not viable. Let's just aim for consistency, so include the ".".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
f0f2f8a to
b5cf5fb
Compare
| buildoutputs { "$(OBJDIR)/%{file.objname}.o" } | ||
| buildoutputs { "$(OBJDIR)/%{file.objname}%{premake.modules.gmake.cpp.gettooloutputext('cxx', cfg)}" } | ||
| buildmessage '$(notdir $<)' | ||
| buildcommands {'$(CXX) %{premake.modules.gmake.cpp.fileFlags(cfg, file)} $(FORCE_INCLUDE) -o "$@" -MF "$(@:%.o=%.d)" -c "$<"'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that .o also be changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"$(@:%.o=%.d)" hardcode .o, should it be "$(@:%%%{premake.modules.gmake.cpp.gettooloutputext('cxx', cfg)}=%.d)"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not the line you've commented on here.
What does this PR do?
Adds ability for toolsets to specify tool output extensions.
How does this PR change Premake's behavior?
The gmake generator will now correctly use
.objwhen using the msc toolset.Anything else we should know?
N/A
Did you check all the boxes?
closes #XXXXin comment to auto-close issue when PR is merged)You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!