-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
Comments
All changes done to $ 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
|
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 In curl-7.57.0.0 I commented the lines out, rather than delete them. Any changes to an m4 macro? |
On 11/13/2017 2:42 PM, Daniel Stenberg wrote:
All changes done to |Makefile.am| since 7.55.1. So maybe automake is
the culprit?
As 7.55.1 is also "broken" maybe the area to look is changes between
7.54.1 and 7.55.1?
… $ 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
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2076 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AKWY9QvBZ_mcBOUwnSvLyhH4_C_d3md4ks5s2EdagaJpZM4Qbxly>.
|
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. |
@aixtools could you run a bisect? https://github.com/curl/curl/wiki/how-to-git-bisect |
Looks interesting - I'll give it a shot.
…On 11/13/2017 8:21 PM, Jay Satiro wrote:
@aixtools <https://github.com/aixtools> could you run a bisect?
https://github.com/curl/curl/wiki/how-to-git-bisect
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2076 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AKWY9blTIZRM-k2-4IbA9ksZPhNusBTQks5s2JalgaJpZM4Qbxly>.
|
This may end-up being double - my answer has not shown up yet... michael@x071:[/data/prj/aixtools/curl/curl-master]git bisect good
Stop. |
digging - I see in a "generated file" - configure, the start of the "bad" info. +3750 CODE_COVERAGE_RULES=' I am going to guess that it is something coming from "m4/ax_code_coverage.m4" |
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 |
OK. But, I just choose a good label I knew of, and same for a bad. What
is the value of repeated "bads" - as this is exactly what I see today
(at 7.57.0-dev.
Will probably be this evening. Not much free time today.
…On 11/13/2017 9:49 PM, Jay Satiro wrote:
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
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2076 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AKWY9XDUgHoK6iCMogzvrri1fDDB9bbeks5s2KtNgaJpZM4Qbxly>.
|
Actually, managed it this morning - my guess (about the file) seems correct. Bisecting: 0 revisions left to test after this (roughly 0 steps) |
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) |
Thanks. It looks like your hunch on the mailing list was right. |
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
'] |
autoconf-archive has a github mirror, which tells us that even their latest ax_code_coverage.m4 version has two uses of "+=" ... |
Strange. += was also used before 18eac3d and even in the very first version of this script: |
Well,
a) maybe I never tested then, or
b) just switched to gmake because I thought "I had to", and did not
bother to mention it.
I'll byte my tongue - but it seems autoconf assumes gmake is the only make.
…On 11/14/2017 1:16 PM, Marcel Raad wrote:
Strange. += was also used before 18eac3d
<18eac3d>
and even in the very first version of this script:
***@***.***#diff-9b17df31da3fcbb3b226863933edb6b2R190
<autoconf-archive/autoconf-archive@eaefa29#diff-9b17df31da3fcbb3b226863933edb6b2R190>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2076 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AKWY9RdyunS4PWxogINn8nDSPgmYg1XHks5s2YSkgaJpZM4Qbxly>.
|
@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. |
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. |
Right... but did you try the patch? |
That's the autoconf-archive, which is a collection of macros for autoconf provided by other parties. It is not the autoconf project itself. |
@jay - no, have not tried the patch. Just the "1 line" (replaced by |
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"
|
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 |
Do not understand why we have a difference in line-count. Other than that, we look the same:
|
It's unclear from your output whether or not the most recent patch I posted is working for you. Does it work? |
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). |
@aixtools: |
Understood that that is what it is for. But it comes with assumptions -
and among those are that it is a GNU based development environment. All
the autotools are doing as instructed.
My main point is the assumptions implicit in the m4 macro file do not
work with a compiler other than gcc. For me, personally, I would not be
surprised to find gmake in an development environment using gcc - and
everything, as it is now is fine.
However, when not working with gcc the option --enable-code-coverage
should fail (due to the flags it wants to set using the m4 macro
contents) - so then there is no need or sense in creating Makefiles with
content that only work with gcc. And, not including this content also
resolves the need for gmake to parse info that will never be used.
I do not know the autoconf syntax, but I am certain there is way to only
"activate" AX_CODE_COVERAGE if the compiler is gcc and ignore it when
gcc is not the compiler.
Sincerely,
Michael
…On 11/15/2017 1:59 PM, Daniel Stenberg wrote:
@aixtools <https://github.com/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.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2076 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AKWY9SS6f6kA9skirvMo-swoj8aDwt_lks5s2uBFgaJpZM4Qbxly>.
|
FYI: this patch fixes this issue:
Not nonsensical, as m4/ax_code_coverage.m4 contains this test:
|
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. |
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. |
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. |
I notice in 18eac3d that what has changed is |
Oh, that seems totally unnecessary... |
Thanks Jay,
You phrased it much better than I.
In any case, my proposal would make the "option" be available (as now)
all the time (i.e., in the generated and distributed configure script) -
but only actually go into a Makefile when the compiler is gcc. This
seems a sensible condition as the 'stuff' it activates has the same test
internally - clearly indicating the contents of the activated macro is
only intended for gcc.
Shall I try an attempt at a PR?
…On 11/26/2017 10:33 AM, Daniel Stenberg wrote:
basically some code coverage macros are now always put in the makefile
Oh, that seems totally unnecessary...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2076 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AKWY9WhlObCIrpFCW_bYe6Gfcp-Asvpqks5s6TBmgaJpZM4Qbxly>.
|
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
"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
The text was updated successfully, but these errors were encountered: