Bug: local.conf - Release build using 'undefine DEBUG' errors #102

Affected version: 4d677e1d6e
Env: GNU Make dev build / macOS

Description

i am trying to build the engine using 2 scenarios:

scenario 1: existing local.conf that used to be working
scenario 2: added 'undefine DEBUG' to make a safety mode Release build

Scenario 1

my existing local.conf

PREFIX=$(HOME)/local-3x
PER_MACHINE_DIRECTORY=$(PREFIX)/share/pEp
YML2_PATH=$(HOME)/src/pEp/yml2
OPENPGP=SEQUOIA
export PKG_CONFIG_PATH=$(HOME)/src/pEp/pEpEngineSequoiaBackend/target/debug
SQLITE3_FROM_OS=

Scenario 2

added line from local.conf.example to get Release build.
undefine DEBUG

Observed Behaviour

Scenario 1

Makefile.conf:404: *** "DEBUG is defined as 'placeholder' which is different from 'debug' and 'maintainer'".  Stop.

Scenario 2

local.conf:10: *** missing separator. Stop.

Expected Behaviour

Scenario 1

I am not sure if the aim is to be backwards compatible with existing local.conf in the wild. So, can you please comment on that and provide an upgrade path if not.

Scenario 2

I expect this work and result in a Release build.

@positron can you please comment on
"Expected Behaviour Scenario 1"

@positron i updated and i am now using

[heck@YOOROOX::~/src/pEp/pEpEngine32] (master) $ /opt/local/bin/gmake --version
GNU Make 4.4
Built for aarch64-apple-darwin21.3.0

results:

scenario 1: Still the same.
scenario 2: The build succeeds, but i still have tons of logging messages.

I expect to get a release build that only contains logging for critical messages

The current solution uses GNU make's undefine directive, introduced in GNU make version 3.82 (28 Jul 2010).
This seems old enough but @heck is living proof that some systems are still using older versions, in his case 3.81.

Copied from IRC :

the point is that defining to empty and undefining are different things. I do not even like this mechanism, but it was already there in Makefile.conf
The macro "starts" defined, and we can undefine it
undefine has been introduced in Version 3.82 (28 Jul 2010)

If you agree we can change the use of DEBUG and accept an empty definition as equivalent to undefined. In the Engine I can easily do that. Do we want to be completely consistent across different repos?

I think all local.conf variables should be optional and have a good default.
So, the absence of a possible variable in local.conf never breaks the build process but just has the effect of the default value for it.
In other words, a non-existing or empty local.conf build with all options set to default

@heck as for the toolchain dependencies, the older the more compatible it is.
But, i think this is totally up to engine maintainer to define that. I will just follow.

copied from IRC:
'ifeq($(DEBUG),)' is what i am familiar with
In the process of fixing #87 I found several problems, now solved. It is visible in the master git history, even if probably not so interesting
heck: So you like ifeq. Good for me. But let us make it clear that ifdef DEBUG will no longer behave in the same way
[18:01]
I will add a comment to that effect
ERC>

[18:02] < positron> I wonder if other people's local.conf could break.
[18:02] < positron> I would say no
[18:03] < heck> IMO, its fine to break exisiting local.conf syntax/semantics as long as we deliver a direct replacement and an upgrade path
[18:03] < positron> Makefile.conf will change incompatibly, but that is only handled by us / me
[18:03] < positron> Okay then
[18:03] < heck> yes\

Fixed and merged into master.

@positron thanks thousand! works like a charm now.