Fixing build regression [PATHS]

[seanmorris]

What is this PR doing?

This corrects some paths in the docker file used to copy libraries into the build image.

What problem is it solving?

Newer versions of Docker will ignore leading ../ that lead outside of the build context, but older versions will throw an error in that case.

How is the problem addressed?

This removes the extra ../ at the start of the paths in question. Newer versions of docker will ignore ../s that lead outside of the build context, but older versions will throw an error. The following paths will work on later versions but fail on earlier ones:

# Bring in the libraries
RUN mkdir -p /root/lib/include /root/lib/lib /libs
COPY ../../../../../../../../../../bison2.7/dist/ /libs/bison2.7
COPY ../../../../../../../../../../libedit/dist/root/lib /root/lib
COPY ../../../../../../../../../../libiconv/dist/root/lib /root/lib
COPY ../../../../../../../../../../libncurses/dist/root/lib /root/lib
COPY ../../../../../../../../../../libopenssl/dist/root/lib /root/lib
COPY ../../../../../../../../../../libpng16/dist/root/lib /root/lib
COPY ../../../../../../../../../../libsqlite3/dist/root/lib /root/lib
COPY ../../../../../../../../../../libxml2/dist/root/lib /root/lib
COPY ../../../../../../../../../../libz/dist/root/lib /root/lib
COPY ../../../../../../../../../../libzip/dist/1.2.0/root/lib /libs/libzip/1.2.0
COPY ../../../../../../../../../../libzip/dist/1.9.2/root/lib /libs/libzip/1.9.2
COPY ../../../../../../../../../../oniguruma/dist/root/lib /root/lib

Testing Instructions

Checkout the branch and pull dependencies:

git checkout sm-correcting-build-paths
npm install

Build the libraries:

cd packages/php-wasm/compile
make clean
make all

In a new terminal, build PHP 7.4 - 8.3: (PHP <=7.3 is fixed in #871)

npm run recompile:php:web:light:8.3
npm run recompile:php:web:light:8.2
npm run recompile:php:web:light:8.1
npm run recompile:php:web:light:8.0
npm run recompile:php:web:light:7.4

npm run recompile:php:web:kitchen-sink:8.3
npm run recompile:php:web:kitchen-sink:8.2
npm run recompile:php:web:kitchen-sink:8.1
npm run recompile:php:web:kitchen-sink:8.0
npm run recompile:php:web:kitchen-sink:7.4

npm run recompile:php:node:8.3
npm run recompile:php:node:8.2
npm run recompile:php:node:8.1
npm run recompile:php:node:8.0
npm run recompile:php:node:7.4

Ensure the build works:

npm run build
npm run dev

Navigate to the following URLs:

• http://localhost:5400/website-server/?plugin=web-stories&php-extension-bundle=kitchen-sink&php=8.3
• http://localhost:5400/website-server/?plugin=web-stories&php-extension-bundle=kitchen-sink&php=8.2
• http://localhost:5400/website-server/?plugin=web-stories&php-extension-bundle=kitchen-sink&php=8.1
• http://localhost:5400/website-server/?plugin=web-stories&php-extension-bundle=kitchen-sink&php=8.0
• http://localhost:5400/website-server/?plugin=web-stories&php-extension-bundle=kitchen-sink&php=7.4

[adamziel]

To reiterate my question from #871 (comment):

Are there any other Dockerfiles where the same issue needs to be fixed?

[seanmorris]

To reiterate my question from #871 (comment):

Are there any other Dockerfiles where the same issue needs to be fixed?

I found a couple. Fixed em:

wordpress-playground/packages/php-wasm/compile/libedit/Dockerfile

Lines 4 to 5 in a927d2f

	COPY ./libncurses/dist/root/lib/include /root/lib/include
	COPY ./libncurses/dist/root/lib/lib /root/lib/lib

wordpress-playground/packages/php-wasm/compile/libiconv/Dockerfile

Lines 4 to 5 in a927d2f

	COPY ./libz/dist/root/lib/include /root/lib/include
	COPY ./libz/dist/root/lib/lib /root/lib/lib

wordpress-playground/packages/php-wasm/compile/libopenssl/Dockerfile

Lines 4 to 5 in a927d2f

	COPY ./libz/dist/root/lib/include /root/lib/include
	COPY ./libz/dist/root/lib/lib /root/lib/lib

wordpress-playground/packages/php-wasm/compile/libpng16/Dockerfile

Lines 4 to 5 in a927d2f

	COPY ./libz/dist/root/lib/include /root/lib/include
	COPY ./libz/dist/root/lib/lib /root/lib/lib

wordpress-playground/packages/php-wasm/compile/libsqlite3/Dockerfile

Lines 4 to 5 in a927d2f

	COPY ./libz/dist/root/lib/include /root/lib/include
	COPY ./libz/dist/root/lib/lib /root/lib/lib

wordpress-playground/packages/php-wasm/compile/libzip/Dockerfile

Lines 5 to 6 in a927d2f

	COPY ./libz/dist/root/lib/include /root/lib/include
	COPY ./libz/dist/root/lib/lib /root/lib/lib

[adamziel]

@seanmorris nice! Just to make sure – did you confirm the fix works? I see the testing instructions don't mention any make commands.

[seanmorris]

@seanmorris nice! Just to make sure – did you confirm the fix works? I see the testing instructions don't mention any make commands.

I guess I assumed that it was handled by nx or a downstream script. I was using that during development and I've added that to the testing steps.

[seanmorris]

@seanmorris nice! Just to make sure – did you confirm the fix works? I see the testing instructions don't mention any make commands.

@adamziel This just inspired a much simpler fix for the Bison regression: #892

[dmsnell]

It sounds like we made a mistake earlier when creating this Dockerfile by referencing files outside of the directory in which we built the image. Is that right? And this fix is updating those paths so that they properly refer to the base directory itself?

And the reason we didn't notice this before is because older Docker versions normalize the path as if the root directory were the CWD in which the image is built, so that ../ accidentally worked since it was already "at the top" of the directory hierarchy. Is this correct?

If so, it seems like this is more of a universal fix and should work in all versions of Docker.

[dmsnell]

I ran a check for COPY ../ and found no more references than those listed here.

[seanmorris]

It sounds like we made a mistake earlier when creating this Dockerfile by referencing files outside of the directory in which we built the image. Is that right? And this fix is updating those paths so that they properly refer to the base directory itself?

And the reason we didn't notice this before is because older Docker versions normalize the path as if the root directory were the CWD in which the image is built, so that ../ accidentally worked since it was already "at the top" of the directory hierarchy. Is this correct?

If so, it seems like this is more of a universal fix and should work in all versions of Docker.

I'm not entirely sure how we ended up in this situation. It could have been files that were moved from one place to another, paths that were updated erroneously, or any number of other small oversights. I can pour over the git history if need be, but in the end, it was the fact that newer versions of Docker will ignore the leading ../es if they lead outside the build context.

[dmsnell]

Thanks for merging. My question mostly was about the PR and commit descriptions, since this particularly issue doesn't seem so much like a Docker compatibility issue but rather a fix to a mistake we made in the past.