Compile failure when DEBUG is defined

[cooljeanius]

In MacPorts, the cmake portgroup adds -DDEBUG to its compilation flags when using its default +debug variant:
https://github.com/macports/macports-ports/blob/36ac56e6aa4afeb15cc65b5333f02b3f7bbe22e3/_resources/port1.0/group/cmake-1.1.tcl#L496-L501
This breaks compilation in Stellarium's plugins/Scenery3d/src/ShaderManager.hpp, which expects to be able to use DEBUG as the name of an enumerator:

stellarium/plugins/Scenery3d/src/ShaderManager.hpp

Lines 183 to 184 in a9aaa69

	//debug shader for AABBs/Frustums
	DEBUG = (1<<16),

Suggest renaming the enumerator to something else.

Expected Behaviour

Stellarium compiles successfully

Actual Behaviour

Compilation fails with the following error:

In file included from /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_science_stellarium/stellarium/work/build/plugins/Scenery3d/src/Scenery3d-static_autogen/mocs_compilation.cpp:3:
In file included from /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_science_stellarium/stellarium/work/build/plugins/Scenery3d/src/Scenery3d-static_autogen/EWIEGA46WW/moc_S3DRenderer.cpp:10:
In file included from /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_science_stellarium/stellarium/work/build/plugins/Scenery3d/src/Scenery3d-static_autogen/EWIEGA46WW/../../../../../../stellarium-0.21.2/plugins/Scenery3d/src/S3DRenderer.hpp:37:
/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_science_stellarium/stellarium/work/stellarium-0.21.2/plugins/Scenery3d/src/ShaderManager.hpp:184:3: error: expected identifier
                DEBUG           = (1<<16),
                ^
<command line>:28:15: note: expanded from here
#define DEBUG 1
              ^

Steps to reproduce

1. Be on a Mac with MacPorts installed
2. Do sudo port -udc install stellarium +RemoteControl+debug+python39
3. Observe build failure

System

• Stellarium version: originally observed with stellarium 0.21.2; reproduced with stellarium 23.4
• Operating system: macOS BigSur (11.7.10)
• Graphics Card: Intel UHD Graphics 630 1536 MB
• Screen type (if applicable): Built-in Retina Display (16-inch (3072 × 1920))

Logfile

See downstream bug report with MacPorts here: https://trac.macports.org/ticket/63575 (in which @ryandesign encouraged me to file this upstream bug report)

[10110111]

Why do you want to define this macro? It doesn't sound like something standard.

[alex-w]

@gzotti please check this report

[gzotti]

Only on weekends, sorry. If I understand correctly, we should be able to just rename this to something like SHADER_DEBUG or S3D_DEBUG, if these symbols don't exist. But carefully, in all affected places.

[10110111]

This symbol is already inside the context of class ShaderMgr, so accessible as ShaderMgr::DEBUG (if we ignore that it's private). The name is not reserved, nor a standard macro. We shouldn't have to do anything about this really.

The C++ standard defines NDEBUG macro to distinguish release builds from debug ones, but not DEBUG, which is too common a word to reserve. Besides, it's not even in the reserved identifier class (cf. MSVC's _DEBUG), so we've done nothing wrong here and shouldn't go through the trouble of renaming the identifiers that are rightfully ours.

[gzotti]

OK, even better. I honestly admit I don't know anything about "MacPorts", "cmake portgroup" or why they need to set this popular flag DEBUG and not something like _MP_DEBUG for their own purposes.

[cooljeanius]

OK, even better. I honestly admit I don't know anything about "MacPorts", "cmake portgroup" or why they need to set this popular flag DEBUG and not something like _MP_DEBUG for their own purposes.

MacPorts is a package manager for installing software on macOS; think like Homebrew or apt-get or similar. The "cmake portgroup" is common code shared between all ports using cmake for installation. The definition of -DDEBUG isn't for MacPorts' own purposes, it's for the sake of other ports using cmake that presumably need it. The idea is to have some consistency of flags between projects. Doing some archeology with git blame indicates that @RJVB added the define in macports/macports-ports@e8f9c3d, which was the result of merging macports/macports-ports#1028.

[gzotti]

While I have a solution for the Scenery3D plugin (not yet committed), a DEBUG is also used in StelFileMgr line 134. @alex-w , CMakeLists.txt sets INSTALL_DATADIR_FOR_DEBUG for a Debug build. Does it also set DEBUG somewhere?

[10110111]

Trying to "solve" this on our side just lets other tools proliferate bad practice. The DEBUG identifier is not special, not reserved, not system, so we have all the rights to use it. I'd close this as not our bug instead.

[gzotti]

See the other description from "Macports". Yes, they should probably also fix it for themselves. Just that it is simple to do so for us. The "DEBUG" in StelFileMgr is the only case used, but I don't see where it is set. So I ask @alex-w, this may be some CI workflow I am unaware of, or may have some other reason buried in the mist of undocumented legacy code. DEBUG can be used on demand, to test something small. (Yes, almost any other symbol as well.) It may also be some leftover.

[10110111]

The "DEBUG" in StelFileMgr is the only case used, but I don't see where it is set.

It may have been just a mistake by @alex-w (introduced in 9bad385), and it may even have worked for him (just by virtue of being undefined, or due to custom CMAKE_CXX_FLAGS being used).

[alex-w]

I don’t remember details, but probably @10110111 right and DEBUG part in StelFileMgr should be reverted or removed

[gzotti]

So then this commit can best be reverted so this code is active in Debug builds only, right? Without any extras.

[alex-w]

So then this commit can best be reverted so this code is active in Debug builds only, right? Without any extras.

DEBUG -> NDEBUG should be enough IMHO

[github-actions]

Hello @cooljeanius!

Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

[github-actions]

Hello @cooljeanius!

Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest