Skip to content

Rename HAVE_CONFIG_H #1872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
basvodde merged 2 commits into from
Jan 5, 2026
Merged

Rename HAVE_CONFIG_H #1872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8679b2d
Rename config.h and HAVE_CONFIG_H
thetic Nov 15, 2025
7f9c245
Better annotate HAVE_CONFIG_H hack
thetic Jan 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ EXTRA_PROGRAMS = CppUTestExtTests
lib_LIBRARIES = lib/libCppUTest.a
check_PROGRAMS = $(CPPUTEST_TESTS)

# HACK: Replace the HAVE_CONFIG_H macro with CPPUTEST_HAVE_GENERATED_CONFIG_H.
# The name HAVE_CONFIG_H is a hard-coded in autoconf and cannot be changed in
# any "normal" ways. This isn't typically a problem, but CppUTest exposes the
# macro in its public headers. This leads to collisions with any other projects
# built with autoconf.
DEFS = $(filter-out -DHAVE_CONFIG_H,@DEFS@) -DCPPUTEST_HAVE_GENERATED_CONFIG_H
Copy link
Copy Markdown
Member

@basvodde basvodde Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa....

Copy link
Copy Markdown
Member

@basvodde basvodde Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a bit more checking on this, perhaps we can try out AX_PREFIX_CONFIG_H in the configure.ac.

My AI friend recommended to do this:
AC_CONFIG_HEADERS([config.h:config.hin], [AM_CPPFLAGS="$AM_CPPFLAGS -DCPPUTEST_HAVE_CONFIG_H"])

Which also feels like a hack... but at least a hack in the right place as this really shouldn't be in the am file :)

Copy link
Copy Markdown
Contributor Author

@thetic thetic Nov 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a bit more checking on this, perhaps we can try out AX_PREFIX_CONFIG_H in the configure.ac.

I tried this and it only appears to affect the name of the resulting header, not the macro.

My AI friend recommended to do this:
AC_CONFIG_HEADERS([config.h:config.hin], [AM_CPPFLAGS="$AM_CPPFLAGS -DCPPUTEST_HAVE_CONFIG_H"])

I'll need to double check, but I think this only adds the CPPUTEST_HAVE_CONFIG_H macro; it doesn't remove the HAVE_CONFIG_H macro which we don't want leaking into other libraries.

Copy link
Copy Markdown
Contributor Author

@thetic thetic Nov 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My AI friend recommended to do this:
AC_CONFIG_HEADERS([config.h:config.hin], [AM_CPPFLAGS="$AM_CPPFLAGS -DCPPUTEST_HAVE_CONFIG_H"])

That doesn't build. I tried AC_CONFIG_HEADERS([config.h], [AM_CPPFLAGS="$AM_CPPFLAGS -DCPPUTEST_HAVE_CONFIG_H"]) and confirmed that defines only HAVE_CONFIG_H with DEFS = -DHAVE_CONFIG_H in the generated makefile.

Copy link
Copy Markdown
Member

@basvodde basvodde Dec 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks for checking. I did some more checking, and I tried the following in the configure.ac:

-AC_CONFIG_HEADERS([config.h])
+AC_CONFIG_HEADERS([CppUTestGeneratedConfig.h])

In a quick test, that seems to do the trick. But I didn't checked in throughly yet. Could you check that?

Copy link
Copy Markdown
Contributor Author

@thetic thetic Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that does not work. The name of HAVE_CONFIG_H is hardcoded in autoconf12. I don't think config headers were ever intended to be exposed in libraries' public interfaces.

Footnotes

  1. https://www.gnu.org/software/autoconf/manual/autoconf-2.61/html_node/Configuration-Headers.html ↩

  2. https://github.com/autotools-mirror/autoconf/blob/8ac9edd1a71fe24ed94d70f1669e4fd6a81fa585/lib/autoconf/status.m4#L1267 ↩

Copy link
Copy Markdown
Member

@basvodde basvodde Jan 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the problem comes from building a unit testing framework as library.

I'd like to merge this then, but probably need a bit of a comment to it to explain what is happening, why, and a tip that if we find a better version to then remove this. Will you give that a try or shall I add it?

Copy link
Copy Markdown
Contributor Author

@thetic thetic Jan 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no problem. see 7f9c245


if INCLUDE_CPPUTEST_EXT
lib_LIBRARIES+= lib/libCppUTestExt.a
check_PROGRAMS += $(CPPUTESTEXT_TESTS)
Expand Down
2 changes: 1 addition & 1 deletion include/CppUTest/CppUTestGeneratedConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
*
*/

#ifdef HAVE_CONFIG_H
#ifdef CPPUTEST_HAVE_GENERATED_CONFIG_H
#include "generated/CppUTestGeneratedConfig.h"
#endif

Loading