-
-
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
lib/Makefile.m32: add CURL_{LD,C}FLAGS_EXTRAS support #670
Conversation
- in sync with src/Makefile.m32
- fix a comment typo
- sync some whitespace with src/Makefile.m32
* in sync with src/Makefile.m32 * fix a comment typo * sync some whitespace with src/Makefile.m32
By analyzing the blame information on this pull request, we identified @gknauf, @bagder and @captain-caveman2k to be potential reviewers |
Extra options are useful to pass hardening options when building |
CURL_CFLAG_EXTRAS seems fine. How are you using CURL_LDFLAG_EXTRAS? I notice you added it to lib/Makefile.m32 a while back. |
This is the actual value I'm using it with: |
It's useful for both the dll and exe. |
Sync with lib/Makefile.m32 which already uses those variables. Bug: #670
Ok. Thanks, landed in 91cfcc5. |
Thank you Jay! |
After digging deeper into it, this patch turned out to be not enough. It seems inevitable to pass different For some reason, the linker ASLR option doesn't actually enable it for |
Well nothing's gone out yet there's still a few weeks to change it. The thing is in the m32 lib makefile there's been |
I see two possible solutions. First is not a beauty, but compatible and covers all the bases: diff --git a/lib/Makefile.m32 b/lib/Makefile.m32
index fbc38de..eeb4311 100644
--- a/lib/Makefile.m32
+++ b/lib/Makefile.m32
@@ -80,7 +80,7 @@ CC = $(CROSSPREFIX)gcc
CFLAGS = $(CURL_CFLAG_EXTRAS) -g -O2 -Wall
CFLAGS += -fno-strict-aliasing
# comment LDFLAGS below to keep debug info
-LDFLAGS = $(CURL_LDFLAG_EXTRAS) -s
+LDFLAGS = $(CURL_LDFLAG_EXTRAS) $(CURL_LDFLAG_EXTRAS_DLL) -s
AR = $(CROSSPREFIX)ar
RANLIB = $(CROSSPREFIX)ranlib
RC = $(CROSSPREFIX)windres
diff --git a/src/Makefile.m32 b/src/Makefile.m32
index 64c65a6..a6a5b77 100644
--- a/src/Makefile.m32
+++ b/src/Makefile.m32
@@ -92,7 +92,7 @@ CC = $(CROSSPREFIX)gcc
CFLAGS = $(CURL_CFLAG_EXTRAS) -g -O2 -Wall
CFLAGS += -fno-strict-aliasing
# comment LDFLAGS below to keep debug info
-LDFLAGS = $(CURL_LDFLAG_EXTRAS) -s
+LDFLAGS = $(CURL_LDFLAG_EXTRAS) $(CURL_LDFLAG_EXTRAS_EXE) -s
AR = $(CROSSPREFIX)ar
RC = $(CROSSPREFIX)windres
RCFLAGS = --include-dir=$(PROOT)/include -O COFF Another, slightly more elegant~~, but breaking~~ option is to keep diff --git a/lib/Makefile.m32 b/lib/Makefile.m32
index fbc38de..60b05c4 100644
--- a/lib/Makefile.m32
+++ b/lib/Makefile.m32
@@ -80,7 +80,7 @@ CC = $(CROSSPREFIX)gcc
CFLAGS = $(CURL_CFLAG_EXTRAS) -g -O2 -Wall
CFLAGS += -fno-strict-aliasing
# comment LDFLAGS below to keep debug info
-LDFLAGS = $(CURL_LDFLAG_EXTRAS) -s
+LDFLAGS = $(CURL_SHFLAG_EXTRAS) -s
AR = $(CROSSPREFIX)ar
RANLIB = $(CROSSPREFIX)ranlib
RC = $(CROSSPREFIX)windres |
Updated the previous. Neither option is breaking. |
Taking |
The first one is easier to understand but I think the symbols look better as CURL__LDFLAG_EXTRAS. Side note, the mingw makefile.m32 build process is not documented very well, I think. I checked in INSTALL and many of the "chained" style options that can be used are not listed. For example right now all of this is accepted: I think we could document these like: |
nevermind I took another look CURL_LDFLAG_EXTRAS_DLL looks fine, just took a second to adjust :) |
Cool, I was thinking about I've tested almost all of these options (except I'll prepare a PR with the first option. |
using envvars `CURL_LDFLAG_EXTRAS_DLL` and `CURL_LDFLAG_EXTRAS_EXE` respectively. This is useful f.e. to pass ASLR-related extra options, that are required to make this feature work when using the mingw toolchain. Ref: #670 (comment) Closes #689
Thanks, just landed. Can you take a look at docs/INSTALL, and tell me if the directions look right to you, because I just tried them and they don't work for me. mingw32-make mingw32 How are you building using these makefiles? I may have already asked you this but I forgot. |
Thank you for the quick merge! Here's the complete formula I use to build curl: https://github.com/vszakats/harbour-deps/blob/master/curl.sh (Note that MSYS2/sh is not required, this is the umpteenth revision of this script which originally started as a plain batch file, with equally good results.) The error you're getting happens when trying to build a bare (non-release) repo (Git clone or GitHub source archive). These come without a
|