premake / premake-core

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

<command-line>: error: macro names must be identifiers #2174

Open MariwanJ opened 9 months ago

MariwanJ commented 9 months ago

What seems to be the problem? Extra -D in the make file (linux) that causes the issue. This happens due to "defines".

DEFINES += -D_CRT_SECURE_NO_WARNINGS -D

What did you expect to happen? That -D shouldn't be there when there is no other defines available. But premake5 added to the end of the previous defines text.

What have you tried so far? I have a library that include another vendor library. This library will be static-linked to the main project. I can send the whole project if that is required.

How can we reproduce this? If you have defines inside the library (lua script), you will get the extra -D in the make file for the library. Removing the -D will compile just fine. This does not affect Windwos 10.

What version of Premake are you using? premake5

Anything else we should know? no

nickclark2016 commented 9 months ago

Can you confirm if this is a bug on gmake2? gmake is effectively deprecated at this point in time.

MariwanJ commented 9 months ago

yes it was gmake. I tried even gmake2 .. the same. issue

nickclark2016 commented 9 months ago

Can you share your premake file used to generate this? It's something like really should have been caught in our test suite, so I'm a bit confused why this is happening.

MariwanJ commented 9 months ago

ProjectPathConfig.zip Please find the attached zip file. It is a dummy project that I try to get sub library included in another library. I regenerated the make files with the issue. Notice that it appears only under the debug section

nickclark2016 commented 9 months ago

This is an invalid usage issue. In your debug configuration in the PPCL project, you're adding a "" define to the list. defines declarations append to each other in the underlying list.

MariwanJ commented 9 months ago

I don't think that is causing the issue. I have that so it will cover both linux and windows. With no filter it should be applied to all. Remove it and see the result. I had in the other hand an extra ""_CRT_SECURE_NO_WARNINGS" defined under windows filter which is bug and wrong.

nickclark2016 commented 9 months ago

defines "" is wrong, since it just adds an empty "" to the existing list. It does not reset the list of defines. You also have a "_CRT_SECURE_NO_WARNINGS" in an unfiltered context, which is why you're seeing it an extra time in the windows filter. It is expected behavior, as it's a list of defines, not a set.

MariwanJ commented 9 months ago

Ok that "defines" is left there for future usage. Shouldn't that be ignored when there is nothing? I don't think that should cause an issue. You prepare your filters, defines ..etc for future usage and premake5 should know when to add or not IMHO.

nickclark2016 commented 9 months ago

As it currently stands, I don't think it's a bug in premake as written, as we don't specify that an empty string is ignored in the documentation. The case can definitely be made to add that functionality, but I wouldn't classify it as a bug currently.

MariwanJ commented 9 months ago

Ok. thanks for clarifying my lua scripts.

nickclark2016 commented 9 months ago

No problem. This is definitely something worth reviewing on our end, to figure out if it's functionality that should be added or not.