search

premake / premake-core

Premake
https://premake.github.io/
BSD 3-Clause "New" or "Revised" License
3.22k stars 620 forks source link

gmake2: Groups create circular dependencies #2108

Open crazydef opened 1 year ago

crazydef commented 1 year ago

What seems to be the problem? When groups are added to a workspace and they contain projects with the same name, the resulting makefile has circular dependencies.

workspace "App"
  group "Core"
    project "Core"
  group ""

The above snippet will generate this line in the makefile:

Core: Core

And make will output the following message:

make: Circular Core <- Core dependency dropped.

This isn't a huge deal (I can just rename the group), but it took me ages to figure this out. So it might be nice to have a warning in Premake about such things, or even just dropping groups from gmake2. (They're an IDE construct after all. I don't know if they bring any value to makefiles.)

What did you expect to happen? I never know what to expect. :)

What have you tried so far? I renamed the group. It's no biggie, but maybe the UX could be improved.

How can we reproduce this? See the above snippet.

What version of Premake are you using? 5.0.0-dev

Jarod42 commented 1 year ago

Also apply to gmake.

As I see, group adds rules in makefile to build all projects of the group. I haven't expected group to appear in Makefile, but ok...

nickclark2016 commented 1 year ago

Also apply to gmake.

This will be a non-fix unless the gmake and gmake2 group creation logic is shared.

As I see, group adds rules in makefile to build all projects of the group. I didn't expect group to appear in Makefile...

I fully expect compilation groups to be in makefile, as I can do things like optional projects and such with them (i.e. unit tests).

nickclark2016 commented 1 year ago

Just some further review on this. I don't think renaming the group in Premake is a valid option. Personally, I'm a fan of either a "wont fix" or a warning for when there are multiple nodes with the same name in the structure.

crazydef commented 1 year ago

For me renaming the group in the my lua file is the easiest option. And it's what I did. For me, it's no biggie. (It's just a folder name in VS.)

But having some warning or error when running Premake would improve the UX and make it easier to debug things like this in the future. As I said, it took me ages to figure out what the actual problem was. (Which wasn't really a problem per se - just an annoying warning from make when building.)

nickclark2016 commented 1 year ago

Actions to Take:

This could be done in either the core library or the gmake2 module. Correct location TBD.

Jarod42 commented 1 year ago

As it is an issue only with gmake/gmake2, so check should be done in gmake/gmake2, IMO.

nickclark2016 commented 1 year ago

I guess the question is: Do we want this behavior to be consistent across all of premake, or do we want exporter-specific behavior?

crazydef commented 1 year ago

The whole reason for Premake existing is to support multiple toolchains across different OSes/devices. It might seem a bit inconsistent if one reported this and the others didn't. (E.g., everything works on your initial platform and then you suddenly start getting warnings when you move to a second.)