Skip to content
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

PHP: add new builder for PHP projects #225401

Closed
wants to merge 34 commits into from

Conversation

drupol
Copy link
Contributor

@drupol drupol commented Apr 9, 2023

Introducing a new builder for PHP Composer-Based Applications

Ensuring reproducibility and stability of outputs is required even when using an external dependency manager (Composer). As such, we've explored several possible solutions.

  1. The first attempt of this PR was using a Fixed Output Derivation strategy, using the Composer cache. Although this solution rectified numerous issues, it was not entirely secure, as it was heavily dependent on Composer. If Composer implemented even a minor structural alteration in the cache structure, it could break all derivations based on this builder. As a result, we concluded that this solution wasn't ideal.

  2. Our second attempt involved the use of fossar/composition-c4. This project reads the composer.lock file and constructs a local Composer repository, which is then used to install the dependencies. However, as Import From Derivation (IFD) is not permitted in nixpkgs, this approach didn't serve as a feasible solution. Moreover, path type repositories are currently unsupported, adding another roadblock.

  3. A proof-of-concept led to the creation of our last and final attempt; a custom Composer plugin, drupol/composer-local-repo-plugin providing a new command. It mirrors the functionalities of fossar/composition-c4 and is written in PHP. It relies entirely on Composer to create a local Composer repository, thus emerging as a solid solution.

Although this plugin is currently hosted under my own name, there is a consideration to host it elsewhere, a decision yet to be made.

Overcoming Challenges

A major challenge encountered while creating local Composer repositories was that Composer reads the composer.lock file and downloads all the packages, including any dev dependencies. We had no way of discerning whether a dependency was required from the require or require-dev section.
For instance, dependencies like phpstan/phpstan took roughly 10 minutes (1.3Gb) to download, as the plugin uses git to create the local Composer repository. As a result, the creation of a derivation was time-consuming and not particularly environmentally friendly.

I was running out of ideas and patience. I abandoned the PR for a while and at some point, during a hot shower, I had the idea to nuke the require section from composer.json and packages-dev from composer.lock (using jq). Although it substantially modifies the source files (in the derivation only) which is something we aim to minimize but in the end turned out to be a fix for many issues.

As a result, this is now implemented in this PR. Essentially, this new builder will never download any development dependency from any PHP project, which, upon reflection, makes perfect sense. Derivations built with this new wrapper are smaller and we only download what we need to run the application, nothing else.

25 May 2023

Yesterday evening while "half-watching" the TV, I have discovered an improved method for creating a local composer repository utilizing the Composer API only, the composer install step is not required anymore (special thanks to @Seldaek for his assistance!). The plugin greatly simplifies the process of deciding whether or not to download dev dependencies, making it both easier and safer. Additionally, we've eliminated the need to manually modify the composer.lock and composer.json files with jq, streamlining the process further.

Here's a diagram overview on how the new builder is working

Nix PHP builder

This PR includes the following changes:

  • Introduces two new helpers (mkDerivation wrappers)
    • php.buildComposerProject
    • php.mkComposerRepository
  • Introduces two new hooks
    • php.composerHooks.composerRepositoryHook
    • php.composerHooks.composerInstallHook
  • Modify existing derivations (21)
    • Update derivations based on PHAR distribution to source distribution.
    • Bump derivation when it's needed
    • Remove unmaintained derivations (drush)
  • Add new derivations
  • Documentation

This new helpers significantly simplifies the process of integrating applications such as Symfony App, Wordpress or even Drupal into Nix, streamlining development and deployment.

Below is a practical example demonstrating how to efficiently package Drupal using this approach:

image

Or magento2:

image


This PR offers substantial improvements, allowing the majority of PHP derivations that rely on PHAR-based distribution to be replaced with direct source code. As Nix builds and runs software in an isolated environment, it ensures there will be no conflicts, resulting in a more efficient and reliable system.

Now, the amount of PHP PHAR based softwares in Nix now: Just one!

  • Composer (for building composer by sources only)

Remarks

  • phpcbf is now merged in phpcs derivation
    Since it is now using the new builder which is able to create the symlinks automatically, installing phpcs installs at the same time phpcs and phpcbf. Therefore, phpcbf derivation is not needed anymore.

  • I noticed that Deployer has a unique release workflow, which involves making a commit, applying a tag to that commit, cleaning the repository, and committing the PHAR file on that same commit. Development then continues from the parent of that tagged commit. This process is illustrated in the attached image.

    image

    As a result, we cannot directly use the tagged version in the src.rev attribute. Instead, we have to use the parent commit of the tagged version. For example, in the case of tag 7.3.1, the parent commit is d99377d7, which is precisely the revision I have used for the src.rev attribute. If you believe there is a better way to handle this situation, please feel free to suggest an alternative approach. I look forward to hearing your thoughts on this matter. This is now fixed by using rev = "v${f.version}^";, thanks @jtojnar !

Not so related commits

Upcoming PRs

  • Ask each maintainers of these PHP programs to let the composer.lock file live in their project

    • deployer
    • pdepend
    • grumphp (@veewee)
    • phing
    • phpbench (@dantleech)
    • php-cs-fixer
    • php-parallel-lint
    • phpcs
    • phpmd
    • psysh
    • phpunit
    • wp-cli
  • Automatically parse composer.json for extensions and instantiate the php attribute with these extensions enabled. This has already been done in https://github.com/loophp/nix-shell/, we just need to formalize it in here.

@drupol drupol changed the title PHP: add new builder PHP: add new builder for PHP projects Apr 9, 2023
@ofborg ofborg bot requested review from aanderse, etu, globin, jtojnar, Ma27 and talyz April 9, 2023 17:43
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Apr 9, 2023
@drupol drupol force-pushed the php/add-new-builder branch 2 times, most recently from a41c99d to 0af55b5 Compare April 9, 2023 19:27
@ofborg ofborg bot requested a review from jtojnar April 9, 2023 19:30
@drupol drupol force-pushed the php/add-new-builder branch 4 times, most recently from a3c858c to 7bce023 Compare April 10, 2023 06:01
@drupol drupol force-pushed the php/add-new-builder branch 3 times, most recently from 2d00f67 to a965083 Compare April 11, 2023 06:56
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 9, 2023
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 9, 2023
@drupol drupol mentioned this pull request Aug 9, 2023
4 tasks
@drupol
Copy link
Contributor Author

drupol commented Aug 9, 2023

It has been suggested more than once to split this huge PR in two. One containing the new builder, and another one containing the updated derivation.

This is now (almost) done:

I will now close this PR and update this post when the second PR will be done.

@drupol drupol closed this Aug 9, 2023
@onny
Copy link
Contributor

onny commented Nov 25, 2023

* Find the PR containing the updated derivation at: (it will be available next week)

Are you going to update the derivations to the new builder? :) Would be a huge improvement! Or is this already done?

@drupol
Copy link
Contributor Author

drupol commented Nov 25, 2023

@onny Yes, that's the plan... some derivations switched to the new builder already, the first one being Composer.
The plan is to update those derivations at the best effort.
Currently, I'm quite busy with work and personal stuff, so my time for contributions is a bit cut down. However, we greatly appreciate contributions from anyone willing to help.

@drupol drupol deleted the php/add-new-builder branch November 25, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.