Skip to content

PHP: Fast builds with Makefile #338

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

Closed
adamziel opened this issue May 15, 2023 · 7 comments
Closed

PHP: Fast builds with Makefile #338

adamziel opened this issue May 15, 2023 · 7 comments

Comments

@adamziel
Copy link
Collaborator

adamziel commented May 15, 2023

Docker builds are slow. Adding GNU Make into the mix could make for much faster builds. We still need Docker for portability, but let's not use it as a build tool.

Every new person cloning the repo must wait for hour or hours for Docker build to complete for the first time. This consists mostly of rebuilding the libraries. However, it almost never makes sense. Pre-built binaries could be shipped in this repo, saving a ton of build time and making CI builds possible.

GNU Make would make for true graph-based builds and less daunting, faster iterations on the WebAssembly parts of the repo. For example, adding pthreads support requires updating the build process of every library. With Dockerfile, every rebuild would take ages. With Makefile, every rebuild could take seconds.

Related: #73
cc @dmsnell @eliot-akira

@adamziel adamziel changed the title PHP: Build with Makefile PHP: Fast builds with Makefile May 15, 2023
@dmsnell
Copy link
Member

dmsnell commented May 15, 2023

vs. the existing nx flow?

@adamziel
Copy link
Collaborator Author

@dmsnell Oh we'd drown in JSON were we to use nx for building PHP. Makefiles are nice and concise. To me, both tools are made for different jobs. nx is great for wrangling JS packages while make is great for wrangling C programs (and a lot more).

@eliot-akira
Copy link
Collaborator

eliot-akira commented May 15, 2023

Faster builds sound great, it would reduce the friction of onboarding new contributors.


Pre-built binaries could be shipped in this repo

I remember this repo did ship with pre-built binaries before, then they were removed at some point.

Building them from scratch requires additional setup and takes a long time, which is a bad experience for a new contributor who just wants to clone the project and publish their first PR.

At the same time, shipping these files is a burden – they change, pollute the commit logs, and won't play very well with a support for multiple PHP and WordPress version.

It would be nice to have a separate package (or maybe even a repository?) for these pre-built binaries, and source them from there on the initial build.

In general, it's more common not to include built assets in a Git repo for various reasons, like:

As a principle of version control, only "primary objects" should be stored in a repository, not "derived objects".

There are exceptions to the rule: namely, when there are consumers of the repository who require the derived objects, and are reasonably expected not to have the required tools to generate them.

From: Should generated documentation be stored in a Git repository?

Another reason for excluding them from the repo is the nature and size of the built binaries, that they're not suited for Git version control.

So it may make sense to publish these assets to NPM or GitHub Releases, and source them on initial build as needed.


..there is some conditional logic missing.. I typically saw that solved using autoconf and Makefile.in, but that looks very cumbersome and could scare contributors. Plus it doesn't look very pleasant to work with. How about doing the same job using a python script?

Since python3 is already installed as a dependency of emscripten, it sounds like a friendlier scripting language to work with compared to automake/autoconf. Running a Python script would be a little slower, but seems like a worthy trade-off to make the Playground more welcome to contributors and easier to understand.


Dockerfile is convenient for normalizing builds and enumerating the dependencies, but it's even more helpful to be able to look at that list of build dependencies, make sure they're available on my local system, and then directly build the project without Docker.

Looking at the new proposed Makefile, it doesn't seem practical to run it without the accompanying Dockerfile. There's a large amount of dependencies that would need to be installed on the local/host file system - but maybe that's the whole point of not using Docker. To realistically support that use case, most of what's in Dockerfile_emscripten would probably need to be moved into the Makefile so it's more self-sufficient to be run by itself.


Going off topic, but speaking of dependencies on the local/host file system, it would be great to reduce the number of globally installed commands that the project depends on, such as yarn, nx, nvm.

  • For yarn, there doesn't seem to be any compelling reason to not use npm, as far as I could see - other than yarn.lock being checked into the repo instead of package-lock.json.
  • For nx, it's already installed as a devDependency, so it seems redundant to ask users to install it as a global command.
  • For nvm, perhaps it's just a recommended tool rather than a dependency - in which case I hope it stays that way. There's a number of other version managers for Node.js (such as n, fnm, volta, asdf), and for users who have those alternatives installed, nvm would likely conflict with it.

@dmsnell
Copy link
Member

dmsnell commented May 15, 2023

My only thought here @adamziel is that having a single system (if capable enough) brings a lot of value that two systems are working to emulate.

Every new person cloning the repo must wait for hour or hours for Docker build to complete for the first time. This consists mostly of rebuilding the libraries. However, it almost never makes sense. Pre-built binaries could be shipped in this repo, saving a ton of build time and making CI builds possible.

Maybe we can also start by making the Dockerfile skip rebuilding when it doesn't need to.

I see no real problem introducing make here, but without replacing a lot of existing code I can see it also being just another build tool in the array this project requires.

Since there can be issues with modification times that impact make, it could make sense to examine content hashes of the files to check for updates.

This was my idea for the VSCode plugin: request the files with the If-None-Match: [ETAG] HTTP header, so we can know relatively quickly if there are updates to fetch.

@adamziel
Copy link
Collaborator Author

adamziel commented May 15, 2023

🤔

The upsides of make I originally had in mind were:

  • Direct access to source files
  • The ability to ship and reuse prebuilt shared libraries.
  • The ability to restart a failed build from the exact point where it failed. Dockerfile's RUN is all or nothing.

I just realized it's all about where are the files stored and not about the build runner at all.

Migrating to Make have been tedious to say the least. So yes, perhaps it makes more sense to keep using Docker and only work with the storage part of it.

@adamziel
Copy link
Collaborator Author

adamziel commented May 15, 2023

I remember this repo did ship with pre-built binaries before, then they were removed at some point.

They are still in here: https://github.com/WordPress/wordpress-playground/tree/trunk/packages/php-wasm/node/public

Another reason for excluding them from the repo is the nature and size of the built binaries, that they're not suited for Git version control.
So it may make sense to publish these assets to NPM or GitHub Releases, and source them on initial build as needed.

All good points, there's even an issue here: #58

That being said, libpng and related dependencies won't change very often and could probably live in the repo without causing too much headache. The same can't be said about php.wasm, though.

Going off topic, but speaking of dependencies on the local/host file system, it would be great to reduce the number of globally installed commands that the project depends on, such as yarn, nx, nvm.

Yup, let's get rid of yarn.

@adamziel
Copy link
Collaborator Author

adamziel commented Jun 1, 2023

I’m no longer convinced that Make is the right way to proceed - pre-built libraries and a local cache will give us all the benefits without having to switch to another tool. Let’s continue the conversation in #489

@adamziel adamziel closed this as completed Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants