Assure a default toolset is always set. #774
Conversation
src/base/tools.lua
Outdated
| parts[2] = "LLVM-" .. parts[2]:sub(6) | ||
| end | ||
| end | ||
| p.warnOnce(identifier, "toolset '%s' is deprecated, toolset 'msc-%s' instead.", identifier, identifier) |
There was a problem hiding this comment.
"toolset '%s' is deprecated, toolset 'msc-%s' instead." should be "toolset '%s' is deprecated, use toolset 'msc-%s' instead." instead. (Added use after the comma) The line above shouldn't have the TODO anymore.
Also, I'm not sure this should be deprecated? It seems a bit arbitrary to require "msc-" to be prefixed to the value that VS will use and display. I'm happy for this to be merged in, but I'm concerned that new users will find this to be completely arbitrary.
There was a problem hiding this comment.
That was the original intention behind leaving the "v140" form in place: that matches what VS uses, and what you see in the IDE, so it is a more "expected" format if you're familiar with the VS projects. We did go back and forth on it though.
There was a problem hiding this comment.
I just put the deprecation message in there, because there is a comment that says "we should deprecate this"...
https://github.com/premake/premake-core/blob/master/src/base/tools.lua#L34
tvandijck
force-pushed
the
default-toolset
branch
from
9654055
to
89b75cf
Compare
|
Removed the deprecation warning, but it still normalizes to "msc-v140", we could potentially remove that... since both are the same. |
tvandijck
force-pushed
the
default-toolset
branch
from
89b75cf
to
e824a34
Compare
|
Removed the deprecation warning a while ago... need more feedback however on what direction we want to go for the normalization? |
tvandijck
force-pushed
the
default-toolset
branch
from
e824a34
to
a2c02ef
Compare
tvandijck
force-pushed
the
default-toolset
branch
from
a2c02ef
to
fd41938
Compare
We're running into the issue where:
doesn't work, because the majority of time, the toolset is not actually set. This change assures the filter always has a valid value in it, and not just 'nil' in the majority of cases.
We also normalize the toolset, so when you do
it normalizes to
msc-v140, as well as the other rules implemented in thep.tool.canonicalfunction.We should realy deprecate the v140 naming however, and require explicit msc-v140 usage, so I added a p.warnOnce in there too.