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

Tag new releases of farmOS/composer-project when a new version of farmOS is tagged #739

Draft
wants to merge 4 commits into
base: 2.x
Choose a base branch
from

Conversation

mstenta
Copy link
Member

@mstenta mstenta commented Oct 23, 2023

Currently our Composer docs instruct end-users to start a farmOS-based project with:

composer create-project farmos/project

However, this fails because https://github.com/farmOS/composer-project does not have any tagged releases.

See original issue reported here:

This PR aims to fix that by creating a tag commit in the https://github.com/farmOS/composer-project every time a tagged release of farmOS is created. These "tag commits" would not be part of the 2.x branch itself, but instead would be "hanging" off of the latest 2.x branch commit (or 3.x if we decide to adopt this and start doing it with farmOS v3 releases).

@paul121 and I discussed this in chat today, and while we are still unsure if this is the best solution, it was easy enough to sketch it up as a PR so that we could have something concrete to consider.

Apart from fixing farmOS/composer-project#10, there are two other small benefits to doing this:

  1. When a new project is started using composer create-project farmos/project, their composer.json file will be referencing a specific tag of farmOS, instead of ^2.0, which forces them to be intentional about their upgrades by default. They can, of course, choose to switch to ^2.0.
  2. This will include composer.lock in the tag commit, which will ensure that anyone starting a project is starting with the same exact composer.lock that we generate and include in our packaged release tarballs.

Both of those ensure that users are getting the same exact code as our official Docker image and tarball releases. Once they start their project, they become responsible for the composer.json and composer.lock files, so they can decide how they want to manage them onward from that point.

mstenta added a commit to mstenta/farmOS that referenced this pull request Oct 23, 2023
@mstenta
Copy link
Member Author

mstenta commented Oct 23, 2023

To be clear: This is NOT TESTED. Just a draft of what I think it would look like.

@mstenta
Copy link
Member Author

mstenta commented Oct 23, 2023

When a new project is started using composer create-project farmos/project, their composer.json file will be referencing a specific tag of farmOS, instead of ^2.0

Actually, now that I look closer, this isn't working exactly as I expected. All this is doing right now is copying composer.json from the output of build-farmOS.sh, but if I look at the composer.json included with the latest 2.2.1 release, it looks like this:

    "repositories": {
        "farmos": {
            "type": "git",
            "url": "https://github.com/farmOS/farmOS"
        },
        "0": {
            "type": "composer",
            "url": "https://packages.drupal.org/8"
        }
    },
    "require": {
        "farmos/farmos": "^2.2",
        "oomphinc/composer-installers-extender": "^2.0",
        "wikimedia/composer-merge-plugin": "^2.0"
    },

So two things:

  1. We don't want the farmos repository added in the case of tagged releases.
  2. I expected "farmos/farmos": "^2.2", to be "farmos/farmos": "2.2.1",

Both of these have more to do with build-farmOS.sh generally than this PR specifically, though.

For example, we could resolve 1 by only running this line for non-tag releases:

composer config repositories.farmos git ${FARMOS_REPO}

And regarding 2, I wonder if we are using composer require wrong here:

composer require farmos/farmos ${FARMOS_COMPOSER_VERSION} --no-install

I don't think there is supposed to be a space between composer require farmos/farmos and ${FARMOS_COMPOSER_VERSION}. The package name and version constraint should be separated by a colon instead. So it should be:

composer require farmos/farmos:${FARMOS_COMPOSER_VERSION} --no-install

@mstenta
Copy link
Member Author

mstenta commented Oct 23, 2023

I pushed two more commits to the beginning of this to address the issues described in my previous comment. For consideration, and perhaps to be split out to their own PRs even if this one doesn't go in.

@paul121
Copy link
Member

paul121 commented Oct 23, 2023

I pushed two more commits to the beginning of this to address the issues described in my previous comment. For consideration, and perhaps to be split out to their own PRs even if this one doesn't go in.

These look good 👍

there are two other small benefits to doing this:

re: 1 and pinning specific versions of farmOS, IMO this shouldn't be done in our composer-project. This is more restrictive and introduces complexity when it times to update (no longer just a composer update). We already have docs for how to pin dependencies if you would like to do so: https://farmos.org/hosting/composer/#pinning-versions A simpler solution would be to tag minor versions in composer-project that update dependencies to the farmOS minor version ^2.0, ^2.1, etc. I think this could be done in a single git history too

re: 2 and including composer.lock, this is a nice to have at project start but shouldn't be something to rely on. Once someone updates their dependencies we can only control their direct dependencies of farmOS/farmOS. Composer.lock also includes assumptions about the platform dependencies, the biggest thing being the PHP version. So we would need to include a specific PHP version in the composer.json. https://getcomposer.org/doc/articles/composer-platform-dependencies.md

@mstenta
Copy link
Member Author

mstenta commented Oct 23, 2023

These are good points @paul121! It would change the way that composer create-project farmos/farmos behaves, and we need to consider whether those changes are good/bad.

A simpler solution would be to tag minor versions in composer-project that update dependencies to the farmOS minor version ^2.0, ^2.1, etc. I think this could be done in a single git history too

A little trickier to automate, but that could be possible.

The potential pitfall of only tagging minor releases on composer-project is that if we make any commits to that repo between minor releases of farmOS, anyone who makes a new project in between wouldn't get those commits.

So I'd say we either go all-in or we don't do it at all. Only doing minor releases would be more trouble I think.

@mstenta
Copy link
Member Author

mstenta commented Nov 27, 2023

FYI I split the first three commits out to a separate PR since they are independent of the goal of this one: #754

@paul121
Copy link
Member

paul121 commented Dec 11, 2023

re: 2 and including composer.lock, this is a nice to have at project start but shouldn't be something to rely on. Once someone updates their dependencies we can only control their direct dependencies of farmOS/farmOS. Composer.lock also includes assumptions about the platform dependencies, the biggest thing being the PHP version.

This got me today. My dev environment was 8.2 and production was 8.1. Using a composer based workflow I could't just push up my composer.lock without bumping the production to 8.2. An example of the composer error:

  Found a `composer.json`, installing dependencies.
    W: Installing dependencies from lock file (including require-dev)
    W: Verifying lock file contents can be installed on current platform.
    W: Your lock file does not contain a compatible set of packages. Please run composer update.
    W: 
    W:   Problem 1
    W:     - lcobucci/clock is locked to version 3.2.0 and an update of this package was not requested.
    W:     - lcobucci/clock 3.2.0 requires php ~8.2.0 || ~8.3.0 -> your php version (8.1.26) does not satisfy that requirement.
    W:   Problem 2
    W:     - symfony/css-selector is locked to version v7.0.0 and an update of this package was not requested.
    W:     - symfony/css-selector v7.0.0 requires php >=8.2 -> your php version (8.1.26) does not satisfy that requirement.
    W:   Problem 3
    W:     - lcobucci/clock 3.2.0 requires php ~8.2.0 || ~8.3.0 -> your php version (8.1.26) does not satisfy that requirement.
    W:     - league/oauth2-server 8.5.4 requires lcobucci/clock ^2.2 || ^3.0 -> satisfiable by lcobucci/clock[3.2.0].
    W:     - league/oauth2-server is locked to version 8.5.4 and an update of this package was not requested.
    ```

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants