Skip to content
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.

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

Build cleanup #85

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Build cleanup #85

wants to merge 9 commits into from

Conversation

ngie-eign
Copy link
Contributor

@ngie-eign ngie-eign commented Dec 25, 2024

This PR addresses some build bugs and adds some build enhancements:

  • Remove leftover references of auto_array (bugfix).
  • Remove the atf-*-api manpages (enhancement).
  • Require -std=C++14 with ATF (enhancement).
  • Do a proper job at exposing the WCOREDUMP(..) macro on platforms which support it via autotools (bugfix).
  • Add __attribute__((nonnull)) support, similar to __attribute__((unused)), et al (enhancement).
  • Restore AC_PROG_CPP (bugfix).

@ngie-eign ngie-eign force-pushed the build-cleanup branch 2 times, most recently from 6ce107f to f054eeb Compare December 26, 2024 04:30
This entry should have been removed in the referenced commit.

Fixes:	6639d08
This change imports ac_cxx_compile_stdcxx.m4 from gnu,org and makes use
of the `AX_CXX_COMPILE_STDCXX` macro in configure.ac to ensure that the
compiler specified supports C++-14 (at bare minimum). This is being done
to quell some issues reported by scan-build about the code using C++
range-based for-loops (a feature added in C++-11).
These manpages have been deprecated for at least a release. Remove them
and all of the logic associated with them.

Signed-off-by: Enji Cooper <ngie@FreeBSD.org>
WCOREDUMP is considered an extension to the POSIX spec on multiple
platforms, and thus is not automatically exposed on all platforms. Add the
relevant preprocessor defines to config.h via autoconf, then leverage them
in atf-c(3).

This helps ensure that the platforms which support WCOREDUMP properly
expose the macro.

Signed-off-by: Enji Cooper <ngie@FreeBSD.org>
This feature is being detected so several functions can be appropriately
marked as taking non-NULL/-nullptr parameters, and the compiler and static
analyzers can (in turn) make intelligent decisions when optimizing and
analyzing code, respectively.

Signed-off-by: Enji Cooper <ngie@FreeBSD.org>
This was accidentally deleted post-0.21 release. It's still needed by
tests and build infrastructure.

Signed-off-by: Enji Cooper <ngie@FreeBSD.org>
The libcalls used have been in the POSIX standard since 2008.1. Require
them to be present instead of conditionally hoping they're present.

This fixes an issue where the autoconf code was messing up with a
combination of clang tools, which resulted in the autoconf code failing
to properly determine whether or not the functions were available.

Signed-off-by: Enji Cooper <ngie@FreeBSD.org>
This particular check unfortunately doesn't work when ATF_BUILD_CXX
contains multiple CXXFLAGS, or contains a path with spaces in it. Remove
this check to unbreak the dependent tests
post-793d4640031dc06ce8a239ffa9ab61322104c4ca.

Signed-off-by: Enji Cooper <ngie@FreeBSD.org>
The C++ toolchain is sanity checked when C++14 conformance is checked;
there's no reason why we need to check whether or not the C++ toolchain
works again.

Signed-off-by: Enji Cooper <ngie@FreeBSD.org>
@ngie-eign
Copy link
Contributor Author

@asomers, @emaste , @ihoro : ping?

@@ -25,7 +25,6 @@

libatf_c___la_SOURCES += atf-c++/detail/application.cpp \
atf-c++/detail/application.hpp \
atf-c++/detail/auto_array.hpp \
Copy link
Member

Choose a reason for hiding this comment

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

this one lgtm

AC_PROG_CXX
AC_LANG_COMPILER(C++)
AX_CXX_COMPILE_STDCXX(14, noext, mandatory)
Copy link
Member

Choose a reason for hiding this comment

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

OK

@@ -114,12 +114,4 @@ clean-all:

.PHONY: $(PHONY_TARGETS)

# TODO(jmmv): Remove after atf 0.22.
Copy link
Member

Choose a reason for hiding this comment

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

OK

@@ -62,6 +62,7 @@ AC_USE_SYSTEM_EXTENSIONS

AC_PROG_CXX
AC_LANG_COMPILER(C++)
AC_PROG_CPP
Copy link
Member

Choose a reason for hiding this comment

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

OK. Is this where it belongs though, it seems the CXX and C++ bits should be together without CPP in the middle

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll have to run through it again, but the order for invoking the macros is a bit fragile (autoconf/automake whine a lot about not calling things in the right order).

@@ -65,50 +61,19 @@ atf_env_set(const char *name, const char *val)
{
atf_error_t err;

#if defined(HAVE_SETENV)
Copy link
Member

Choose a reason for hiding this comment

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

OK

@@ -64,16 +64,6 @@ AC_PROG_CXX
AC_LANG_COMPILER(C++)
AC_PROG_CPP
AX_CXX_COMPILE_STDCXX(14, noext, mandatory)
AC_CACHE_CHECK([whether the C++ compiler works],
Copy link
Member

Choose a reason for hiding this comment

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

OK

@emaste
Copy link
Member

emaste commented Dec 29, 2024

I've marked the files I've reviewed so far

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.

2 participants