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

On AIX - the generated src/Makefile and lib/Makefile require gnu make #2076

Closed
aixtools opened this issue Nov 13, 2017 · 37 comments
Closed

On AIX - the generated src/Makefile and lib/Makefile require gnu make #2076

aixtools opened this issue Nov 13, 2017 · 37 comments
Labels

Comments

@aixtools
Copy link
Contributor

Seems to be a (recent - 7.55.X ?) change to Makefile generation.

I did this

git clone ...
./buildconf
./configure ...

I expected the following

curl to build

I get

  • make > .buildaix/make.out
    "Makefile", line 3808: make: 1254-055 Dependency line needs colon or double colon operator.
    "Makefile", line 3809: make: 1254-055 Dependency line needs colon or double colon operator.
    "Makefile", line 3810: make: 1254-055 Dependency line needs colon or double colon operator.

My hack

a) delete all lines after .PRECIOUS: Makefile
b) comment all lines with either ?= or +=

curl/libcurl version

7.57.0-DEV

operating system

AIX 5.3 TL7, AIX 6.1 TL9

@bagder
Copy link
Member

bagder commented Nov 13, 2017

All changes done to Makefile.am since 7.55.1. So maybe automake is the culprit?

$ git diff curl-7_55_1.. -- Makefile.am
diff --git a/Makefile.am b/Makefile.am
index ab8f11cbc..d225a09ec 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -28,11 +28,12 @@ CMAKE_DIST = CMakeLists.txt CMake/CMakeConfigurableFile.in      \
  CMake/CurlTests.c CMake/FindGSS.cmake CMake/OtherTests.cmake   \
  CMake/Platforms/WindowsCache.cmake CMake/Utilities.cmake       \
  CMake/Macros.cmake              \
  CMake/CurlSymbolHiding.cmake CMake/FindCARES.cmake             \
  CMake/FindLibSSH2.cmake CMake/FindNGHTTP2.cmake                \
- CMake/FindMbedTLS.cmake CMake/cmake_uninstall.cmake.in
+ CMake/FindMbedTLS.cmake CMake/cmake_uninstall.cmake.in         \
+ CMake/curl-config.cmake
 
 VC6_LIBTMPL = projects/Windows/VC6/lib/libcurl.tmpl
 VC6_LIBDSP = projects/Windows/VC6/lib/libcurl.dsp.dist
 VC6_LIBDSP_DEPS = $(VC6_LIBTMPL) Makefile.am lib/Makefile.inc
 VC6_SRCTMPL = projects/Windows/VC6/src/curl.tmpl

@bagder bagder added the build label Nov 13, 2017
@aixtools
Copy link
Contributor Author

aixtools commented Nov 13, 2017

I am using automake-1.15.

Whatever it is, I am only seeing it in src/Makefile and lib/Makefile - but seems you are correct about it being related to 7.55.1 - note: 7.55.1 and 7.56.0 I did not run buildconf. I used configure as it came from the distro/download.

michael@x071:[/data/prj/aixtools/curl]find . -name Makefile | xargs egrep -c "+=|?=" | grep -v :0
./curl-7.55.1/src/Makefile:18
./curl-7.55.1/lib/Makefile:18
./curl-7.56.0/src/Makefile:18
./curl-7.56.0/tests/fuzz/Makefile:1
./curl-7.56.0.0/src/Makefile:18
./curl-7.56.0.0/lib/Makefile:18
./curl-7.56.0.0/tests/fuzz/Makefile:1
./curl-master/src/Makefile:18
./curl-7.57.0/src/Makefile:18
./curl-7.57.0.0/src/Makefile:18
./curl-7.57.0.0/lib/Makefile:18

In curl-7.57.0.0 I commented the lines out, rather than delete them.
In curl-master/lib/Makefile - I had deleted the lines (so no show here)

Any changes to an m4 macro?

@aixtools
Copy link
Contributor Author

aixtools commented Nov 13, 2017 via email

@bagder
Copy link
Member

bagder commented Nov 13, 2017

We've successfully built with non-gnu make on other systems though (I did myself on FreeBSD a short while ago). I'm not aware of any GNUisms written by us in the makefiles.

@jay
Copy link
Member

jay commented Nov 13, 2017

@aixtools could you run a bisect? https://github.com/curl/curl/wiki/how-to-git-bisect

@aixtools
Copy link
Contributor Author

aixtools commented Nov 13, 2017 via email

@aixtools
Copy link
Contributor Author

This may end-up being double - my answer has not shown up yet...

michael@x071:[/data/prj/aixtools/curl/curl-master]git bisect good
Bisecting: 30 revisions left to test after this (roughly 5 steps)
[050e353] dist: fix the cmake build by shipping cmake_uninstall.cmake.in too
michael@x071:[/data/prj/aixtools/curl/curl-master]./buildconf
buildconf: autoconf version 2.69 (ok)
buildconf: autom4te version 2.69 (ok)
buildconf: autoheader version 2.69 (ok)
buildconf: automake version 1.15 (ok)
buildconf: aclocal version 1.15 (ok)
buildconf: libtoolize version 2.4.6 (ok)
buildconf: GNU m4 version 1.4.17 (ok)
buildconf: running libtoolize
libtoolize: putting auxiliary files in '.'.
libtoolize: copying file './ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'.
libtoolize: copying file 'm4/libtool.m4'
libtoolize: copying file 'm4/ltoptions.m4'
libtoolize: copying file 'm4/ltsugar.m4'
libtoolize: copying file 'm4/ltversion.m4'
libtoolize: copying file 'm4/lt~obsolete.m4'
libtoolize: Remember to add 'LT_INIT' to configure.ac.
buildconf: converting all mv to mv -f in local m4/libtool.m4
buildconf: running aclocal
buildconf: converting all mv to mv -f in local aclocal.m4
buildconf: running autoheader
buildconf: running autoconf
buildconf: running automake
configure.ac:126: installing './compile'
configure.ac:180: installing './config.guess'
configure.ac:180: installing './config.sub'
configure.ac:126: installing './install-sh'
configure.ac:127: installing './missing'
docs/examples/Makefile.am: installing './depcomp'
buildconf: OK
michael@x071:[/data/prj/aixtools/curl/curl-master]buildaix --with-ca-bundle=/var/ssl/cacert.pem
VRMF
xlc is /usr/vacpp/bin/xlc

  • CPPFLAGS="-I/opt/include" CFLAGS="-I/opt/include -O2 -qmaxmem=-1 -qarch=pwr5"
    ./configure
    --prefix=/opt
    --sysconfdir=/var/curl-master/etc
    --sharedstatedir=/var/curl-master/com
    --localstatedir=/var/curl-master
    --mandir=/usr/share/man
    --infodir=/opt/share/info/curl-master --with-ca-bundle=/var/ssl/cacert.pem
    > .buildaix/configure.out
    configure: WARNING: Continuing even with errors mentioned immediately above this line.
    configure: WARNING: Cannot find libraries for LDAP support: LDAP disabled
    configure: WARNING: libpsl was not found
    configure: WARNING: Cannot find libraries for IDN support: IDN disabled
    configure: WARNING: Continuing even with errors mentioned immediately above this line.
  • make > .buildaix/make.out
    "Makefile", line 3754: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3755: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3756: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3757: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3759: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3760: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3761: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3762: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3763: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3764: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3765: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3768: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3769: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3770: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3772: make: Dependency line needs colon or double colon operator.
    "Makefile", line 3808: make: Dependency line needs colon or double colon operator.
    make: Fatal errors encountered -- cannot continue.
    make: The error code from the last command is 1.

Stop.
make returned an error
michael@x071:[/data/prj/aixtools/curl/curl-master]git bisect bad
Bisecting: 15 revisions left to test after this (roughly 4 steps)
[453e7a7] glob: do not continue parsing after a strtoul() overflow range

@aixtools
Copy link
Contributor Author

digging - I see in a "generated file" - configure, the start of the "bad" info.

+3750 CODE_COVERAGE_RULES='
+3751 # Code coverage
+3752 #
+3753 # Optional:
+3754 # - CODE_COVERAGE_DIRECTORY: Top-level directory for code coverage reporting.
+3755 # Multiple directories may be specified, separated by whitespace.
+3756 # (Default: $(top_builddir))
+3757 # - CODE_COVERAGE_OUTPUT_FILE: Filename and path for the .info file generated
+3758 # by lcov for code coverage. (Default:
+3759 # $(PACKAGE_NAME)-$(PACKAGE_VERSION)-coverage.info)

I am going to guess that it is something coming from "m4/ax_code_coverage.m4"

@jay
Copy link
Member

jay commented Nov 13, 2017

bisecting you have to complete all the steps, just because it returns a bad commit doesn't mean anything until all steps are done, it basically repeatedly halves the range until it finds the first bad commit which is what we need

@aixtools
Copy link
Contributor Author

aixtools commented Nov 14, 2017 via email

@aixtools
Copy link
Contributor Author

Actually, managed it this morning - my guess (about the file) seems correct.

Bisecting: 0 revisions left to test after this (roughly 0 steps)
[18eac3d] ax_code_coverage.m4: update to latest version

@jay
Copy link
Member

jay commented Nov 14, 2017

What is the value of repeated "bads" - as this is exactly what I see today

Marking a commit good or bad will halve the range. Forget about commits and think about just a range of numbers it might be easier to understand, look how the range keeps halving

(good .. bad)
(123 .. 345)
(234 .. 345)
(234 .. 289)
(234 .. 262)
(248 .. 262)
(255 .. 262)
(258 .. 262)
(258 .. 260)
(259 .. 260)

@jay
Copy link
Member

jay commented Nov 14, 2017

[18eac3d] ax_code_coverage.m4: update to latest version

Thanks. It looks like your hunch on the mailing list was right.

@jay
Copy link
Member

jay commented Nov 14, 2017

I don't understand why they put the '' in between the A and the M but my guess is they don't want it picked up during a sed or something? Does this work for you:

diff --git a/m4/ax_code_coverage.m4 b/m4/ax_code_coverage.m4
index 6484f03..518aaa4 100644
--- a/m4/ax_code_coverage.m4
+++ b/m4/ax_code_coverage.m4
@@ -254,7 +254,8 @@ code-coverage-capture-hook:
 '"$CODE_COVERAGE_RULES_CLEAN"'
 
 A''M_DISTCHECK_CONFIGURE_FLAGS ?=
-A''M_DISTCHECK_CONFIGURE_FLAGS += --disable-code-coverage
+A''M_DISTCHECK_CONFIGURE_FLAGS := $(A''M_DISTCHECK_CONFIGURE_FLAGS) \
+  --disable-code-coverage
 
 .PHONY: check-code-coverage code-coverage-capture code-coverage-capture-hook code-coverage-clean
 ']

@bagder
Copy link
Member

bagder commented Nov 14, 2017

autoconf-archive has a github mirror, which tells us that even their latest ax_code_coverage.m4 version has two uses of "+=" ...

@MarcelRaad
Copy link
Member

Strange. += was also used before 18eac3d and even in the very first version of this script:
autoconf-archive/autoconf-archive@eaefa29#diff-9b17df31da3fcbb3b226863933edb6b2R190

@aixtools
Copy link
Contributor Author

aixtools commented Nov 14, 2017 via email

@aixtools
Copy link
Contributor Author

@jay I tried commenting the lines with += and leaving the ?= lines intact. Unfortunately, only after I commented out all lines with either ?= or += did the make proceed normally.

Going to look at the autoconf github to see if they even care about non-gnu make.

@aixtools
Copy link
Contributor Author

After seeing this: autoconf-archive/autoconf-archive@eaefa29#diff-9b17df31da3fcbb3b226863933edb6b2 I suspect the autoconf macro that forces this logic check (as not all Makefiles in curl have this, only lib/ and src/ was not being used and/or previous builds passed because this content (see link) was not yet in curl base (as the commit comment was to update the m4 macro to the "latest" version.

@jay
Copy link
Member

jay commented Nov 14, 2017

Right... but did you try the patch?

@bagder
Copy link
Member

bagder commented Nov 14, 2017

Going to look at the autoconf github to see if they even care about non-gnu make.

That's the autoconf-archive, which is a collection of macros for autoconf provided by other parties. It is not the autoconf project itself.

@aixtools
Copy link
Contributor Author

@jay - no, have not tried the patch. Just the "1 line" (replaced by
2) - will do now.

@aixtools
Copy link
Contributor Author

aixtools commented Nov 14, 2017

My memory from yesterday was that the m4 input was many more lines... which is why I was surprised by the "one-liner" - unfortunately... more needed. One line is not enough (see my diff below - and then it works - i.e., your changes corrects "one line"

michael@x071:[/data/prj/aixtools/curl/curl-master]git branch
* issue-2076
  master
michael@x071:[/data/prj/aixtools/curl/curl-master]git diff
diff --git a/m4/ax_code_coverage.m4 b/m4/ax_code_coverage.m4
index 6484f03..eb95a09 100644
--- a/m4/ax_code_coverage.m4
+++ b/m4/ax_code_coverage.m4
@@ -253,8 +253,8 @@ code-coverage-capture-hook:

 '"$CODE_COVERAGE_RULES_CLEAN"'

-A''M_DISTCHECK_CONFIGURE_FLAGS ?=
-A''M_DISTCHECK_CONFIGURE_FLAGS += --disable-code-coverage
+A''M_DISTCHECK_CONFIGURE_FLAGS := $(A''M_DISTCHECK_CONFIGURE_FLAGS) \
+  --disable-code-coverage

 .PHONY: check-code-coverage code-coverage-capture code-coverage-capture-hook code-coverage-clean
 ']

michael@x071:[/data/prj/aixtools/curl/curl-master]./buildconf
buildconf: autoconf version 2.69 (ok)
buildconf: autom4te version 2.69 (ok)
buildconf: autoheader version 2.69 (ok)
buildconf: automake version 1.15 (ok)
buildconf: aclocal version 1.15 (ok)
buildconf: libtoolize version 2.4.6 (ok)
buildconf: GNU m4 version 1.4.17 (ok)
buildconf: running libtoolize
libtoolize: putting auxiliary files in '.'.
libtoolize: copying file './ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'.
libtoolize: copying file 'm4/libtool.m4'
libtoolize: copying file 'm4/ltoptions.m4'
libtoolize: copying file 'm4/ltsugar.m4'
libtoolize: copying file 'm4/ltversion.m4'
libtoolize: copying file 'm4/lt~obsolete.m4'
libtoolize: Remember to add 'LT_INIT' to configure.ac.
buildconf: converting all mv to mv -f in local m4/libtool.m4
buildconf: running aclocal
buildconf: converting all mv to mv -f in local aclocal.m4
buildconf: running autoheader
buildconf: running autoconf
buildconf: running automake
configure.ac:126: installing './compile'
configure.ac:181: installing './config.guess'
configure.ac:181: installing './config.sub'
configure.ac:126: installing './install-sh'
configure.ac:127: installing './missing'
docs/examples/Makefile.am: installing './depcomp'
parallel-tests: installing './test-driver'
buildconf: OK

michael@x071:[/data/prj/aixtools/curl/curl-master]buildaix --with-ca-bundle=/var/ssl/cacert.pem
VRMF
xlc is /usr/vacpp/bin/xlc
+ CPPFLAGS="-I/opt/include" CFLAGS="-I/opt/include -O2 -qmaxmem=-1 -qarch=pwr5"\
        ./configure\
                --prefix=/opt \
                --sysconfdir=/var/curl-master/etc\
                --sharedstatedir=/var/curl-master/com\
                --localstatedir=/var/curl-master\
                --mandir=/usr/share/man\
                --infodir=/opt/share/info/curl-master --with-ca-bundle=/var/ssl/cacert.pem\
                        > .buildaix/configure.out
configure: WARNING: Continuing even with errors mentioned immediately above this line.
configure: WARNING: Cannot find libraries for LDAP support: LDAP disabled
configure: WARNING: libpsl was not found
configure: WARNING: Cannot find libraries for IDN support: IDN disabled
configure: WARNING: Continuing even with errors mentioned immediately above this line.
+ make > .buildaix/make.out
"Makefile", line 3808: make: Dependency line needs colon or double colon operator.
"Makefile", line 3809: make: Dependency line needs colon or double colon operator.
"Makefile", line 3810: make: Dependency line needs colon or double colon operator.
"Makefile", line 3811: make: Dependency line needs colon or double colon operator.
"Makefile", line 3813: make: Dependency line needs colon or double colon operator.
"Makefile", line 3814: make: Dependency line needs colon or double colon operator.
"Makefile", line 3815: make: Dependency line needs colon or double colon operator.
"Makefile", line 3816: make: Dependency line needs colon or double colon operator.
"Makefile", line 3817: make: Dependency line needs colon or double colon operator.
"Makefile", line 3818: make: Dependency line needs colon or double colon operator.
"Makefile", line 3819: make: Dependency line needs colon or double colon operator.
"Makefile", line 3822: make: Dependency line needs colon or double colon operator.
"Makefile", line 3823: make: Dependency line needs colon or double colon operator.
"Makefile", line 3824: make: Dependency line needs colon or double colon operator.
"Makefile", line 3826: make: Dependency line needs colon or double colon operator.
make: Fatal errors encountered -- cannot continue.
make: The error code from the last command is 1.


Stop.
make returned an error

michael@x071:[/data/prj/aixtools/curl/curl-master]grep DISTCHECK_CONFIGURE_FLAGS lib/Makefile
AM_DISTCHECK_CONFIGURE_FLAGS := $(AM_DISTCHECK_CONFIGURE_FLAGS) \

michael@x071:[/data/prj/aixtools/curl/curl-master]cp -p lib/Makefile lib/Makefile.buildconf

michael@x071:[/data/prj/aixtools/curl/curl-master]vi lib/Makefile

michael@x071:[/data/prj/aixtools/curl/curl-master]diff -u lib/Makefile.buildconf lib/Makefile
--- lib/Makefile.buildconf      2017-11-14 20:13:31 +0000
+++ lib/Makefile        2017-11-14 20:19:28 +0000
@@ -3823,26 +3823,26 @@
 # use the git-version-gen script, available online.

 # Optional variables
-CODE_COVERAGE_DIRECTORY ?= $(top_builddir)
-CODE_COVERAGE_OUTPUT_FILE ?= $(PACKAGE_NAME)-$(PACKAGE_VERSION)-coverage.info
-CODE_COVERAGE_OUTPUT_DIRECTORY ?= $(PACKAGE_NAME)-$(PACKAGE_VERSION)-coverage
-CODE_COVERAGE_BRANCH_COVERAGE ?=
-CODE_COVERAGE_LCOV_SHOPTS_DEFAULT ?= $(if $(CODE_COVERAGE_BRANCH_COVERAGE),\
---rc lcov_branch_coverage=$(CODE_COVERAGE_BRANCH_COVERAGE))
-CODE_COVERAGE_LCOV_SHOPTS ?= $(CODE_COVERAGE_LCOV_SHOPTS_DEFAULT)
-CODE_COVERAGE_LCOV_OPTIONS_GCOVPATH ?= --gcov-tool "$(GCOV)"
-CODE_COVERAGE_LCOV_OPTIONS_DEFAULT ?= $(CODE_COVERAGE_LCOV_OPTIONS_GCOVPATH)
-CODE_COVERAGE_LCOV_OPTIONS ?= $(CODE_COVERAGE_LCOV_OPTIONS_DEFAULT)
-CODE_COVERAGE_LCOV_RMOPTS_DEFAULT ?=
-CODE_COVERAGE_LCOV_RMOPTS ?= $(CODE_COVERAGE_LCOV_RMOPTS_DEFAULT)
-CODE_COVERAGE_GENHTML_OPTIONS_DEFAULT ?=\
+# CODE_COVERAGE_DIRECTORY ?= $(top_builddir)
+# CODE_COVERAGE_OUTPUT_FILE ?= $(PACKAGE_NAME)-$(PACKAGE_VERSION)-coverage.info
+# CODE_COVERAGE_OUTPUT_DIRECTORY ?= $(PACKAGE_NAME)-$(PACKAGE_VERSION)-coverage
+# CODE_COVERAGE_BRANCH_COVERAGE ?=
+# CODE_COVERAGE_LCOV_SHOPTS_DEFAULT ?= $(if $(CODE_COVERAGE_BRANCH_COVERAGE),\
+# --rc lcov_branch_coverage=$(CODE_COVERAGE_BRANCH_COVERAGE))
+# CODE_COVERAGE_LCOV_SHOPTS ?= $(CODE_COVERAGE_LCOV_SHOPTS_DEFAULT)
+# CODE_COVERAGE_LCOV_OPTIONS_GCOVPATH ?= --gcov-tool "$(GCOV)"
+# CODE_COVERAGE_LCOV_OPTIONS_DEFAULT ?= $(CODE_COVERAGE_LCOV_OPTIONS_GCOVPATH)
+# CODE_COVERAGE_LCOV_OPTIONS ?= $(CODE_COVERAGE_LCOV_OPTIONS_DEFAULT)
+# CODE_COVERAGE_LCOV_RMOPTS_DEFAULT ?=
+# CODE_COVERAGE_LCOV_RMOPTS ?= $(CODE_COVERAGE_LCOV_RMOPTS_DEFAULT)
+# CODE_COVERAGE_GENHTML_OPTIONS_DEFAULT ?=\
 $(if $(CODE_COVERAGE_BRANCH_COVERAGE),\
 --rc genhtml_branch_coverage=$(CODE_COVERAGE_BRANCH_COVERAGE))
-CODE_COVERAGE_GENHTML_OPTIONS ?= $(CODE_COVERAGE_GENHTML_OPTIONS_DEFAULT)
-CODE_COVERAGE_IGNORE_PATTERN ?=
+# CODE_COVERAGE_GENHTML_OPTIONS ?= $(CODE_COVERAGE_GENHTML_OPTIONS_DEFAULT)
+# CODE_COVERAGE_IGNORE_PATTERN ?=

-GITIGNOREFILES ?=
-GITIGNOREFILES += $(CODE_COVERAGE_OUTPUT_FILE) $(CODE_COVERAGE_OUTPUT_DIRECTORY)
+# GITIGNOREFILES ?=
+# GITIGNOREFILES += $(CODE_COVERAGE_OUTPUT_FILE) $(CODE_COVERAGE_OUTPUT_DIRECTORY)

 code_coverage_v_lcov_cap = $(code_coverage_v_lcov_cap_$(V))
 code_coverage_v_lcov_cap_ = $(code_coverage_v_lcov_cap_$(AM_DEFAULT_VERBOSITY))

±±±±± the change is the right direction!
michael@x071:[/data/prj/aixtools/curl/curl-master]make
Making all in lib
        make  all-am
  CC       libcurl_la-file.lo
  CC       libcurl_la-timeval.lo
  CC       libcurl_la-base64.lo



@jay
Copy link
Member

jay commented Nov 14, 2017

I understand but we can't simply comment all those lines out. Is it enough if GITIGNOREFILES is also changed from += to :=

diff --git a/m4/ax_code_coverage.m4 b/m4/ax_code_coverage.m4
index 6484f03..bae4bc1 100644
--- a/m4/ax_code_coverage.m4
+++ b/m4/ax_code_coverage.m4
@@ -222,7 +222,8 @@ CODE_COVERAGE_GENHTML_OPTIONS ?= $(CODE_COVERAGE_GENHTML_OPTIONS_DEFAULT)
 CODE_COVERAGE_IGNORE_PATTERN ?=
 
 GITIGNOREFILES ?=
-GITIGNOREFILES += $(CODE_COVERAGE_OUTPUT_FILE) $(CODE_COVERAGE_OUTPUT_DIRECTORY)
+GITIGNOREFILES := $(GITIGNOREFILES) \
+  $(CODE_COVERAGE_OUTPUT_FILE) $(CODE_COVERAGE_OUTPUT_DIRECTORY)
 
 code_coverage_v_lcov_cap = $(code_coverage_v_lcov_cap_$(V))
 code_coverage_v_lcov_cap_ = $(code_coverage_v_lcov_cap_$(AM_DEFAULT_VERBOSITY))
@@ -254,7 +255,8 @@ code-coverage-capture-hook:
 '"$CODE_COVERAGE_RULES_CLEAN"'
 
 A''M_DISTCHECK_CONFIGURE_FLAGS ?=
-A''M_DISTCHECK_CONFIGURE_FLAGS += --disable-code-coverage
+A''M_DISTCHECK_CONFIGURE_FLAGS := $(A''M_DISTCHECK_CONFIGURE_FLAGS) \
+  --disable-code-coverage
 
 .PHONY: check-code-coverage code-coverage-capture code-coverage-capture-hook code-coverage-clean
 ']

edit: patch modified due to typo

@aixtools
Copy link
Contributor Author

aixtools commented Nov 14, 2017

I went to look at (some of the changes) in the file, as added to curl.

The first had this (as a pic, sorry)
image

While the second has:
image

I do not "speak" m4 well enough to understand the signifgance of the removal of "])" from after AC_SUBST -and the addition of the "]" after .PHONY:... - although I suspect it explains why the large block of text is now being added to the Makefile.

imho - it is the inclusion of CODE_COVERAGE_RULES that is breaking things.

But I'll try your way (as I do not really "read" or "speak" m4

@aixtools
Copy link
Contributor Author

Do not understand why we have a difference in line-count. Other than that, we look the same:

diff --git a/m4/ax_code_coverage.m4 b/m4/ax_code_coverage.m4
index 6484f03..0a2181b 100644
--- a/m4/ax_code_coverage.m4
+++ b/m4/ax_code_coverage.m4
@@ -221,8 +221,8 @@ $(if $(CODE_COVERAGE_BRANCH_COVERAGE),\
 CODE_COVERAGE_GENHTML_OPTIONS ?= $(CODE_COVERAGE_GENHTML_OPTIONS_DEFAULT)
 CODE_COVERAGE_IGNORE_PATTERN ?=

-GITIGNOREFILES ?=
-GITIGNOREFILES += $(CODE_COVERAGE_OUTPUT_FILE) $(CODE_COVERAGE_OUTPUT_DIRECTORY)
+GITIGNOREFILES := $(GITIGNOREFILES) \
+  $(CODE_COVERAGE_OUTPUT_FILE) $(CODE_COVERAGE_OUTPUT_DIRECTORY)

 code_coverage_v_lcov_cap = $(code_coverage_v_lcov_cap_$(V))
 code_coverage_v_lcov_cap_ = $(code_coverage_v_lcov_cap_$(AM_DEFAULT_VERBOSITY))
@@ -253,8 +253,8 @@ code-coverage-capture-hook:

 '"$CODE_COVERAGE_RULES_CLEAN"'

-A''M_DISTCHECK_CONFIGURE_FLAGS ?=
-A''M_DISTCHECK_CONFIGURE_FLAGS += --disable-code-coverage
+A''M_DISTCHECK_CONFIGURE_FLAGS := $(A''M_DISTCHECK_CONFIGURE_FLAGS) \
+  --disable-code-coverage

 .PHONY: check-code-coverage code-coverage-capture code-coverage-capture-hook code-coverage-clean
 ']

The diff from the previous attempt shows the change was processed - but the comments are still being "dumped" into the Makefile. imho - because:
lines 166 .. 260 are being included as "flat text"

  +165
  +166  [CODE_COVERAGE_RULES='
  +167  # Code coverage
  +168  #
  +169  # Optional:
  +170  #  - CODE_COVERAGE_DIRECTORY: Top-level directory for code coverage reporting.
  +171  #    Multiple directories may be specified, separated by whitespace.
  +172  #    (Default: $(top_builddir))
  +173  #  - CODE_COVERAGE_OUTPUT_FILE: Filename and path for the .info file generated
...
  +254  '"$CODE_COVERAGE_RULES_CLEAN"'
  +255
  +256  A''M_DISTCHECK_CONFIGURE_FLAGS := $(A''M_DISTCHECK_CONFIGURE_FLAGS) \
  +257    --disable-code-coverage
  +258
  +259  .PHONY: check-code-coverage code-coverage-capture code-coverage-capture-hook code-coverage-
clean
  +260  ']

And the new AC_SUBST is ALWAYS outputting this block.

@jay
Copy link
Member

jay commented Nov 14, 2017

It's unclear from your output whether or not the most recent patch I posted is working for you. Does it work?

@aixtools
Copy link
Contributor Author

My mail never made it. Answer is no.

I have dug around a bit, and the feature AX_CODE_COVERAGE is a "gnu" environment setting (e.g., -lgcov must resolve) - and the settings established are flags for gcc. So, I am thinking that the better solution path will to have more logic about whether to actually "activate" AX_CODE_COVERAGE in configure.ac

In my case (xlc, posix make) - this would solve the issue (imho).

@bagder
Copy link
Member

bagder commented Nov 15, 2017

@aixtools: AX_CODE_COVERAGE is a macro that brings the --enable-code-coverage option to configure. If you don't use that option, configure will not enable the coverage-generating magic so it should work just fine for any compiler. The macro itself checks if gcc is in use so --enable-code-coverage will exit with an error if not.

@aixtools
Copy link
Contributor Author

aixtools commented Nov 16, 2017 via email

@aixtools
Copy link
Contributor Author

FYI: this patch fixes this issue:

michael@x071:[/data/prj/aixtools/curl/curl-master]git diff configure.ac
diff --git a/configure.ac b/configure.ac
index c15ff4e..c443324 100755
--- a/configure.ac
+++ b/configure.ac
@@ -51,7 +51,11 @@ CURL_CHECK_OPTION_ARES
 CURL_CHECK_OPTION_RT

 XC_CHECK_PATH_SEPARATOR
-AX_CODE_COVERAGE
+dnl Check if gcc is being used before adding AX_CODE_COVERAGE
+AS_IF([ test "$GCC" = "yes" ], [AX_CODE_COVERAGE],
+  # not using GCC so pass a test below - CODE_COVERAGE_ENABLED_TRUE is not zero length
+  CODE_COVERAGE_ENABLED_TRUE='#'
+)

 #
 # save the configure arguments

Not nonsensical, as m4/ax_code_coverage.m4 contains this test:

  +109                  dnl Check if gcc is being used
  +110                  AS_IF([ test "$GCC" = "no" ], [
  +111                          AC_MSG_ERROR([not compiling with gcc, which is required for gcov code coverage])
  +112                  ])

@bagder
Copy link
Member

bagder commented Nov 22, 2017

We need to generate a single configure script that works for all platforms so we can't generate a different one for different compilers as we don't know the compiler when we generate the script.

The generation of configure and the running of the actual m4 macro are two separate actions.

@aixtools
Copy link
Contributor Author

I understand that. My feedback is that the test in m4/ax... comes too late. With the test in configure.ac if the compiler is not gcc the macro text does not appear in Makefile, however if the compiler is gcc it will. In other words the text from the m4 addition is included in the generated configure script.

@bagder
Copy link
Member

bagder commented Nov 23, 2017

The test for gcc is for the run-time configure when the coverage option is used.

The script should however still be written to generate a makefile that works with "normal" make, so it still needs attention. It's however not related to gcc. You can use gnu make with and without gcc. You can use non-gnu make with and without gcc.

@jay
Copy link
Member

jay commented Nov 26, 2017

I notice in 18eac3d that what has changed is AC_SUBST([CODE_COVERAGE_RULES]) is now always included whereas prior to that it was only included if code coverage was enabled (AS_IF([ test "$enable_code_coverage" = "yes" ], [). So basically some code coverage macros are now always put in the makefile. (You guys may have already figured this out, I'm just catching up). I don't know if that's a bug or not, as mentioned earlier it make expect gnu makefile

@bagder
Copy link
Member

bagder commented Nov 26, 2017

basically some code coverage macros are now always put in the makefile

Oh, that seems totally unnecessary...

@aixtools
Copy link
Contributor Author

aixtools commented Nov 29, 2017 via email

@bagder bagder closed this as completed in ebaab4d Dec 8, 2017
@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

4 participants