cmake 1.1 portgroup sets -DDEBUG for the +debug variant

[szhorvat (Szabolcs Horvát)]

The cmake 1.1 portgroup automatically creates the +debug variant, and will use the compiler option -DDEBUG. As far as I can tell, this is a completely arbitrary choice. Unlike NDEBUG, DEBUG is not a standard macro name, and should not be used for all ports.

In fact, this breaks the +debug variant of the igraph port. This was completely unexpected to the igraph project: we simply did not anticipate that anyone would try to compile the library with -DDEBUG. There is some borrowed code within igraph which had some broken printfs fenced by #ifdef DEBUGs, which caused the failure. I will fix this within igraph, but regardless of that, MacPorts's behaviour seems unjustified here.

I suggest simply not using -DDEBUG by default. If a port needs it for its debug mode, it should include it explicitly, not the reverse (i.e. having to remove it explicitly).

This behaviour was originally added here: [e8f9c3dbe31f1b604b076a8753b3ea26d265e276/macports-ports]

[RJVB (René Bertin)]

I don't know if you're referring to an official standard here, but a quick search on linux (fgrep ' DEBUG' -R /usr/include) shows that the DEBUG token isn't that uncommon as you seem to think. And that frankly stands to reason given that the explicit opposite (NDEBUG) is an apparently less disputed standard.

I'm not in favour of changing this given that apparently it is an issue only now and only with a single port, not knowing how many users depend on the fact that DEBUG is being defined. If it weren't for that I'd probably concur with you that ports could add the token definition (and even then, most port maintainers are not also developers of the software provided by their port so you can't expect them to know this kind of detail).

As it stands I'd say it's easier for ports where this definition does pose a problem to filter it out from the standard debug options set by +debug. That's not currently supported in the released version of the PG I see. I'll attach a patch that achieves this ASAP.

[szhorvat (Szabolcs Horvát)]

NDEBUG is an _actual_ standard (it's part of the C standard): ​https://en.cppreference.com/w/c/error/assert It is also added by CMake's own built-in Debug build type. All projects should be prepared to deal with the presence of an NDEBUG.

DEBUG is not comparable: it's not even a de-facto standard. It's something some projects happen to use for certain purposes (not necessarily for their standard debug build). I would say that unlike NDEBUG, DEBUG is not something that all projects _must_ be prepared to compile correctly with. Defining preprocessor symbols arbitrarily, without the package explicitly calling for it, does not look like good practice to me.

Would it not make more sense to add a feature to portfiles that lets people define what flags are needed for that specific port's debug build? These might very well not be preprocessor symbols, but CMake switches. That would be a robust solution.

Since MacPorts does not build +debug variants by default, I don't think we really know what effect this has. Do some ports actually need it? Having DEBUG in their source does not mean they do: for example igraph has several different SOMETHING_DEBUG macros which are meant for igraph developers only, to debug one specific part of the library, but must not be enabled in a debug build. Do some port break with it? The only reason I even noticed this problem with igraph is that I manually tried to build the +debug variant and we had the (IMO overzealous) -Werror set. Otherwise, it would have just started writing undesired stuff to the terminal, which is not intended for a standard igraph debug build. This would not have been noticed.

My point is that it is unclear what the actual effects of this feature are. However, it seems to me that it leads to unpredictability, which is a negative. Having a documented way not to _remove_ this, but to add arbitrary debug-build-specific options is a predictable and much cleaner solution.

[szhorvat (Szabolcs Horvát)]

(Just to be clear: for igraph I already fixed this upstream. I merely use igraph's situation as an illustration of how this features leads to surprising effects.)

[ryandesign (Ryan Carsten Schmidt)]

Now that this matter has been brought to our attention, yes, it seems reasonable that the portgroup should not add -DDEBUG by default. However, there are over 500 ports already using this portgroup. Who agrees to audit all of these ports to determine whether this change in the portgroup changes how those ports build with the +debug variant?

[RJVB (René Bertin)]

My point exactly (but quantified :)). One of the places where I found use of the token is in X11 headers. Which stands to reason as their use of it probably goes back even further than mine. It's all nice and well that NDEBUG was added to a C standard somewhere in the very late 90s (if that's indeed what C99 stands for), but C had been around for a very long time already by then. It beats me why no one saw a need to include a token that's the opposite of NDEBUG (maybe the standard drafters consider debug builds the default?!). For me use of the token was clearly obvious enough not to see any need to document its inclusion in the +debug variant - which btw is intended to build software with the maximum possible amount of debugging info.

I was planning to attach just a patchfile but since I saw a few other minor usability improvements in my own version of the PG I decide to throw them in and create a PR after all: ​https://github.com/macports/macports-ports/pull/10610

[szhorvat (Szabolcs Horvát)]

If you make changes to that PortGroup file anyway, it might be a good idea to add a comment with a link to this issue, in case other wonder about why DEBUG is present.

[mascguy (Christopher Nielsen)]

Add keyword portgroup, to pg-related tickets