Closed Jarod42 closed 9 months ago
Hi, thanks for the PR! I have a question though: if you have both premade_qmake
and premake_qt
, and then configure premake_qmake
using qtmodule
, what happens in premake_qt
? Is the qtmodule
command from premake_qmake
compatible with the one of premake_qt
, or will it result in incorrect configuration in premake_qt
?
My current setup is all extra generators in premake5-system.lua (as generators don't change script) and use require 'premake-qt/qt'
only in script which use Qt.
So even if I don't use qmake, There are interactions with that api (which is identical).
I'm currently testing using both modules. It seems job is mostly "done" twice (qmake handles natively qt modules, so you just have to do MODULES = core gui
and defines/include/lib are setup correctly. Leading to some duplicated information.
~I think I~ We just have to conditionally call qtenable
to make my scripts portable.
No handling of translation files (*.ts) though. (and premake-qmake doesn't currently handle custom build...)
One possibility would be that one module know the other one (creating some dependency I don't want) and act in consequence, but I don't think it is a good idea. Especially as premake-qmake is not as mature than others generators.
I'm not entirely sure I understand your answer. But in any cases, I really don't like this pull request. It feels like a quick hack to get rid of an error that should be fixed downstream (e.g. having both qmake at qt addons loaded at the same time feels wrong...) Also, the type of the qtmodules function in qt is not the same as in qmake, making this error prone.
But in any cases, I really don't like this pull request
I might understand, not sure how to do it better :-/ Conflicting APIs is handled as error in premake, even with same definition (maybe it is the right fix?). In Core, they mix APIs (there are not placed in appropriated modules) and as it is a whole, they can have no duplicates.
(e.g. having both qmake at qt addons loaded at the same time feels wrong...)
My "global" premake5-system.lua looks like:
require 'premake-codeblocks/codeblocks'
require 'premake-cmake/cmake'
require 'premake-ninja/ninja'
require 'premake-qmake/qmake'
i.e all extra generators there. I would also put extra toolset there if I have one.
So I can generate any of my projects with any generator.
Only qmake has extra API. In core, several generators have extra APIs (especially msvc).
For qt projects, I put require 'premake-qt/qt'
at top of my script to allow to use it.
And currently, I have errors with duplicated API and cannot run premake (even for premake5 --help
), so no solutions can be generated.
Ok. The way I see it, this pull request doesn't solve the problem at all. It just hides it / pushes it a bit further. The real problem here is that addons don't have namespaces (or any other kind of deambiguation mechanism)
The proper way to fixing that would be to have the ability to create addon namespaces, but that's something that needs to be solved in Premake-core. Like for instance, if instead of require "foo"
you were using a custom Premake-core function load { addon="foo" }
, that internally does the require, then that function could be augmented with an optional namespace, something like load { addon="foo", namespace="bar" }
And in this case, all the registered command would go into a bar
namespace, so you could use bar.my_foo_command
without conflicting with any other addon.
THIS is the real solution to the problem you're having.
Another way of hiding this issue would be to only load the addons that you actually use for the current generator. As far as I remember, you can only generate a project for 1 generator at a time, right? If it is still the case, then you could just do something like (in pseudo-code):
if generator is qmake then
require qmake
else
require qt
endif
With that being said, I'm going to close that pull request, because I won't accept it (for all the reasons I just wrote, and also the fact that this addon has been around for 9 years, while qmake only 6, so it would be fairer in any case to let the newer ones adapt to their elders :p)
so register it only if not already done.