Use ARG instead of ENV for some variables

[EvgenyOrekhov]

ENV CFLAGS="-fstack-protector-strong -fpic -fpie -O2"
ENV CPPFLAGS="$CFLAGS"
ENV LDFLAGS="-Wl,-O1 -Wl,--hash-style=both"

php/7.1/fpm/alpine/Dockerfile

Lines 47 to 49 in 1739456

	ENV CFLAGS="-fstack-protector-strong -fpic -fpie -O2"
	ENV CPPFLAGS="$CFLAGS"
	ENV LDFLAGS="-Wl,-O1 -Wl,--hash-style=both"

These ENV variables are persisted into the built image, which makes using php:fpm-alpine as the base image problematic (for example when one wants to build something from source these variables will interfere with the build process and break it).

Here is an example of a relevant issue of a failing compilation process when using php:fpm-alpine as the base image: openresty/docker-openresty#25.

The ARG instruction should be used to avoid that problem.

[tianon]

The relevant discussion which created these lines is in #272 and #340.

Do we create compatibility concerns if we don't compile later .so files with these same flags? Is there a more minimal set which increases our compatibility? (I didn't see any testing on that linked issue beyond simply disabling the entire set -- do you think it'd be difficult to narrow down exactly which flag or flags are causing the failure?)

If we go forward with this, I think we should likely instead rename these to have a PHP_ prefix so that our docker-php-ext-... scripts can use the same flags at least (similar to what we do with PHPIZE_DEPS in the Alpine variant).

I can kind of sympathize with both sides of this discussion (to provide more secure default CFLAGS vs simply allowing each project to maintain their own), but I think I lean more towards the side that causes the least surprise for our downstream users, which is to say that I'm currently more convinced these should get a PHP_ prefix and only be used by our Dockerfile build process and the build scripts we provide for compiling further extensions, but I am still very interested in figuring out why these settings broke openresty, and whether the breakage is indicative of potentially other larger issues.

[tianon]

As @miglanc noted in cae3c03#commitcomment-20120934, it looks like librdkafka suffers from a similar error: confluentinc/librdkafka#944

[miglanc]

@tianon Thanks for invitation :) Yes, I am confirming that I had problems with compiling librdkafka on 7.0 and 7.1 apache and CLI images.

I had to apply a workaround to clear the values of this variables at the beginning of my image:

ENV CFLAGS="" \
    CPPFLAGS=""

Edit: I find it rather high priority issue as it was quite frustrating for me as it was time consuming to find the source of the problem.

[ketchoop]

It makes a lot of problems with building from sources and it's very difficult to find this bug out. It breaks build at all, sometimes. I know, there are many different solutions that I can use in my own containers to avoid that, but please don't pollute global scope of the container.

[tianon]

Ok, so we've got two concrete examples of libraries which fail (and appear to fail in roughly the same way) -- can we narrow down exactly which CFLAGS cause them to fail? I'd like to definitely remove whichever flags are causing these libraries to fail to build, regardless of any other changes we make in relation to this issue.

[ketchoop]

@tianon, I think it's not necessary to remove something from CFLAGS, because it probably can make other problems with other libs. If you don't want pass env vars into "make" in RUN section, you can use ARG which exists only in "build phase".

[tianon]

confluentinc/librdkafka#944 (comment) pointed me to http://stackoverflow.com/a/3544211/433558, which leads me to believe our likely culprit is indeed -fpic vs -fPIC, and we should just drop both from our flags regardless.

As I stated before, I am leaning towards additionally restricting these variables to just affect our Dockerfile and the docker-php-ext-* scripts.

[tianon]

Alternatively, we could do some testing with -fPIC instead -- ideally, I'd like to come up with a set that's still generally safe for use, regardless of what is decided about whether to leave them named as-is or namespace them and use them explicitly only in our own build processes.

[tianon]

It would be really, really helpful to have a simple Dockerfile for building each of https://github.com/openresty/docker-openresty and https://github.com/edenhill/librdkafka such that this can be easily tested and verified, so I'll be working on that next.

[tianon]

Replicated as noted in that openresty issue by adjusting FROM alpine to be FROM php:fpm-alpine -- now testing various CFLAGS combinations. 👍

[tianon]

Removing -fpic alone results in the following:

/usr/lib/gcc/x86_64-alpine-linux-musl/5.3.0/../../../../x86_64-alpine-linux-musl/bin/ld: lj_err_dyn.o: relocation R_X86_64_TPOFF32 against `static_uex' can not be used when making a shared object; recompile with -fPIC
lj_err_dyn.o: error adding symbols: Bad value
collect2: error: ld returned 1 exit status
make[1]: *** [libluajit.so] Error 1
Makefile:702: recipe for target 'libluajit.so' failed
make[1]: Leaving directory '/tmp/openresty-1.11.2.2/build/LuaJIT-2.1-20161104/src'
Makefile:111: recipe for target 'default' failed
make: *** [default] Error 2
ERROR: failed to run command: make -j1 TARGET_STRIP=@: CCDEBUG=-g XCFLAGS='-DLUAJIT_ENABLE_LUA52COMPAT' CC=cc PREFIX=/usr/local/openresty/luajit

Testing -fPIC next.

[tianon]

Same error after adding -fPIC; now testing without any PIC flags or -fpie.

[tianon]

Removing both any PIC flags and -fpie does the trick successfully for openresty -- now to repeat for librdkafka. If there are any other examples which exhibit similar behavior, pointers to them would be appreciated (especially +++ if their build-failure error message is actually different from the one these two spit out; error adding symbols: Bad value).