premake-core icon indicating copy to clipboard operation
premake-core copied to clipboard

CustomToolNamespace does not work for certain files

Open ClxS opened this issue 2 years ago • 2 comments

What seems to be the problem? CustomToolNamespace property is not correctly set and makes faulty assumptions about what it can be applied to.

What did you expect to happen? To be able to apply the setting correctly to alternative file types such as ApplicationSettings files.

What have you tried so far? I have created a local override which fixes the faulty behaviour.

Unless I'm misunderstanding, I believe this check has two problems: https://github.com/premake/premake-core/blob/9d38e82f7bb409d1068fc5adaa21c9175b7dd04b/modules/vstudio/vs2005_dotnetbase.lua#L152

  1. CustomBuildTool is not confined just to EmbeddedResources.
  2. It's checking the cfg, this should be the filecfg

How can we reproduce this? Create a basic C# project which includes a file named Settings.settings, and associated Settings.Designer.cs.

project "ExampleProject"
    filter {"files:**.settings" }
        customtoolnamespace "NotExampleProject"
    filter{}

The project will not have the CustomToolNamespace property applied. If you set this property yourself, you can see that it works, and that assumptions about EmbeddedResource shouldn't be used.

What version of Premake are you using? 5.0.0-beta1

Just wanted to file this issue to confirm my assumptions are right before I make a PR when I have time! :)

ClxS avatar Jan 25 '22 16:01 ClxS

CustomBuildTool is not confined just to EmbeddedResources.

Is there a subset that it is confined to? Or perhaps a subset that it definitely can't be used on?

It's checking the cfg, this should be the filecfg

That is a pretty weird one, possibly an oversight from whoever implemented it. It also looks like it would fit better in the dotnet.fileinfo function, here, which is called and iterated here right before emitting CustomToolNamespace.

Just wanted to file this issue to confirm my assumptions are right before I make a PR when I have time! :)

Given that it uses cfg and not filecfg, I would assume that most people are specifying customtoolnamespace at a high level. So, changing it to not be limited to EmbeddedResource could result in problems. However, we could leave the existing functionality while expanding to support anything at the filecfg level, thoughts? Something like:

if filecfg.customtoolnamespace ~= cfg.customtoolnamespace then
  -- emit filecfg.customtoolnamespace
else if cfg.customtoolnamespace and info.action == "EmbeddedResource" then
  -- emit cfg.customtoolnamespace
end

We could also emit a "deprecation" warning for the second case, and then remove it in the future, but probably not necessary at this point.

samsinsane avatar Feb 03 '22 09:02 samsinsane

Is there a subset that it is confined to? Or perhaps a subset that it definitely can't be used on?

The documented ones it is used on is "EmbeddedResource", "Content", and "None". I don't think any issues can occur from it being added to items which don't use it - but I don't know that for sure. We could limit it to just those three to be safe.

I agree about it was probably only tested on a non-file configuration level before. Keeping that and expanding with full support like you suggest sounds sensible!

ClxS avatar Feb 07 '22 09:02 ClxS