Skip to content
This repository has been archived by the owner on Nov 18, 2022. It is now read-only.

Add the ax_cxx_compile_stdcxx macro locally #189

Merged
merged 1 commit into from
Mar 20, 2016
Merged

Conversation

sbraz
Copy link
Contributor

@sbraz sbraz commented Mar 20, 2016

It's probably best to add this macro locally instead of relying on the system to provide it. Especially since it's rather new and only present in the latest autoconf-archive release.

@hugbug
Copy link
Member

hugbug commented Mar 20, 2016

instead of relying on the system to provide it

What do you mean? M4-files are not needed to build nzbget. Only developers making changes to configure.ac may need to execute "aclocal".

I don't like adding external files to the project directory tree unless this is really necessary.

I'm not an autotools expert though and may be doing things wrong.

@sbraz
Copy link
Contributor Author

sbraz commented Mar 20, 2016

Gentoo, for instance, always runs autoreconf to build the package. If we don't add the macro, the build will fail.
We currently add the file via a patch but I think it would be easier if the upstream tarball provided it. I'm not an autotools expert myself but I was told that this is a commonly accepted practice.
A bit of research showed that gparted, varnish, bitcoin, kodi, znc and probably a lot of other projects do this.

@sbraz
Copy link
Contributor Author

sbraz commented Mar 20, 2016

Sorry, I meant Gentoo's ebuild for nzbget runs autoreconf, not all packages do.

@sbraz
Copy link
Contributor Author

sbraz commented Mar 20, 2016

Of course, none of this is critical :)
Once the release includes the fixed configure with the improved ncurses/tinfo detection, we will probably be able to skip the call to autoreconf.
I just thought it would be a nice addition for people who might try to run autoreconf and would then see configure fail with a rather cryptic error:

./configure: line 3914: syntax error near unexpected token `14,,optional'
./configure: line 3914: `       AX_CXX_COMPILE_STDCXX(14,,optional)'

@hugbug
Copy link
Member

hugbug commented Mar 20, 2016

Can we at least put the file into existing directory "posix" instead of creating a new one? I don't know if aclocal is OK with extra unrelated files in the directory.

@sbraz
Copy link
Contributor Author

sbraz commented Mar 20, 2016

Re-pushed. It seems to work fine this way too.

@hugbug
Copy link
Member

hugbug commented Mar 20, 2016

Great, thank you.
One last thing. You are using the default version of the macro. I'm using however a modified one, which works fine with gcc 4.9 (although it doesn't fully support C++14, it's OK for nzbget nonetheless).
Can you please extract the macro from aclocal.m4?

@sbraz
Copy link
Contributor Author

sbraz commented Mar 20, 2016

Re-pushed with your modified macro. This is the diff to the vanilla one, let me know if I did that properly:

--- ax_cxx_compile_stdcxx.m4.1  2016-03-20 18:16:40.176167776 +0100
+++ posix/ax_cxx_compile_stdcxx.m4  2016-03-20 18:13:16.712371158 +0100
@@ -68,9 +68,12 @@
     ac_success=yes
   fi

-  m4_if([$2], [noext], [], [dnl
+  m4_if([$2], [ext], [], [dnl
   if test x$ac_success = xno; then
-    for switch in -std=gnu++$1 -std=gnu++0x; do
+    dnl HP's aCC needs +std=c++11 according to:
+    dnl http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/PDF_Release_Notes/769149-001.pdf
+    dnl Cray's crayCC needs "-h std=c++11"
+    for switch in -std=c++$1 -std=c++0x +std=c++$1 "-h std=c++$1"; do
       cachevar=AS_TR_SH([ax_cv_cxx_compile_cxx$1_$switch])
       AC_CACHE_CHECK(whether $CXX supports C++$1 features with $switch,
                      $cachevar,
@@ -88,12 +91,9 @@
     done
   fi])

-  m4_if([$2], [ext], [], [dnl
+  m4_if([$2], [noext], [], [dnl
   if test x$ac_success = xno; then
-    dnl HP's aCC needs +std=c++11 according to:
-    dnl http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/PDF_Release_Notes/769149-001.pdf
-    dnl Cray's crayCC needs "-h std=c++11"
-    for switch in -std=c++$1 -std=c++0x +std=c++$1 "-h std=c++$1"; do
+    for switch in -std=gnu++$1 -std=gnu++0x; do
       cachevar=AS_TR_SH([ax_cv_cxx_compile_cxx$1_$switch])
       AC_CACHE_CHECK(whether $CXX supports C++$1 features with $switch,
                      $cachevar,
@@ -110,6 +110,7 @@
       fi
     done
   fi])
+
   AC_LANG_POP([C++])
   if test x$ax_cxx_compile_cxx$1_required = xtrue; then
     if test x$ac_success = xno; then
@@ -445,7 +446,11 @@

 #error "This is not a C++ compiler"

-#elif __cplusplus < 201402L
+//check for full C++14 support (disabed)
+//#elif __cplusplus < 201402L
+
+// check for partial C++14 support (accept gcc 4.9)
+#elif __cplusplus < 201300L

 #error "This is not a C++14 compiler"

@@ -480,6 +485,7 @@

   }

+/*
   namespace test_generalized_constexpr
   {

@@ -499,6 +505,7 @@
     static_assert(strlen_c("another\0test") == 7UL, "");

   }
+*/

   namespace test_lambda_init_capture
   {

hugbug added a commit that referenced this pull request Mar 20, 2016
@hugbug
Copy link
Member

hugbug commented Mar 20, 2016

For some reason in my test the macro wasn't integrated into configure and configure failed then with an error.

What did help was adding the following line to configure.ac (as suggested here):

m4_include([posix/ax_cxx_compile_stdcxx.m4])

I've committed this into branch pr/189. Can you please test if this version works for you (before I merge into develop)?

@sbraz
Copy link
Contributor Author

sbraz commented Mar 20, 2016

Yes, this branch generates a working configure and compiles fine. Thanks for looking into this!

@hugbug hugbug merged commit 67612d8 into nzbget:develop Mar 20, 2016
@hugbug
Copy link
Member

hugbug commented Mar 20, 2016

Great. Thank you for the contribution.

@hugbug hugbug added the meta label Mar 20, 2016
@hugbug hugbug modified the milestones: v17.0, Modern C++ Mar 20, 2016
hugbug added a commit that referenced this pull request Oct 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants