Skip to content

Conversation

@thetic
Copy link
Contributor

@thetic thetic commented Nov 15, 2025

Address #1829.

I know roughly nothing about autoconf, so please double check my work.

@coveralls
Copy link

coveralls commented Nov 15, 2025

Coverage Status

coverage: 99.281%. remained the same
when pulling 8679b2d on thetic:dont_have_config_h
into 4700e91 on cpputest:master.

@thetic thetic changed the title Rename config.h and HAVE_CONFIG_H Rename HAVE_CONFIG_H Nov 17, 2025
check_PROGRAMS = $(CPPUTEST_TESTS)

# Change autoconf's hard-coded (and collision susceptible) HAVE_CONFIG_H macro.
DEFS = $(filter-out -DHAVE_CONFIG_H,@DEFS@) -DCPPUTEST_HAVE_GENERATED_CONFIG_H
Copy link
Member

Choose a reason for hiding this comment

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

Whoa....

Copy link
Member

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
Contributor Author

@thetic thetic 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.

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
Contributor Author

@thetic thetic Nov 29, 2025

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
Member

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
Contributor Author

@thetic thetic Dec 8, 2025

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

@thetic thetic force-pushed the dont_have_config_h branch from 63949e6 to 8679b2d Compare November 29, 2025 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants