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

Upgrade to Composer 2 #73

Closed
le0m opened this issue Oct 31, 2020 · 5 comments · Fixed by #74
Closed

Upgrade to Composer 2 #73

le0m opened this issue Oct 31, 2020 · 5 comments · Fixed by #74
Assignees

Comments

@le0m
Copy link
Member

le0m commented Oct 31, 2020

Composer 2 was released last week, this issue is for discussion about its adoption. See here for Composer 2 upgrade guide and possible breaking changes.

Current proposals (see #72 (comment)):

  1. have separate images for Composer 1 and 2
  2. install both as composer1 and composer2, and create a composer symlink to the latter

Either way, builds are currently failing because hirak/prestissimo is not compatible with composer 2, nor it is needed with it.

@pazion
Copy link

pazion commented Oct 31, 2020

I think seperate builds is a bit overkill.
Option 2 with symlinks sounds good.

@fquffio fquffio self-assigned this Nov 1, 2020
@fquffio
Copy link
Collaborator

fquffio commented Nov 1, 2020

Ok, I've started working on this. Turns out having both composer1 and composer2 is a bit of a mess, as there is no obvious way to have them point to separate $COMPOSER_HOME directories. 😶

This wouldn't be a problem if we didn't ship Composer 1 with hirak/prestissimo plugin already installed out of the box. This causes Composer 2 to try to load that plugin as well, and raise a non-fatal warning.

The "cleanest" solution I came up with was to wrap a custom executable around Composer 1 which sets the COMPOSER_HOME environment variable to a different value (COMPOSER_HOME=$COMPOSER1_HOME), and passes everything else down to the original Phar archive:

# Install Composer.
ENV PATH=$PATH:/root/composer2/vendor/bin:/root/composer1/vendor/bin \
  COMPOSER_ALLOW_SUPERUSER=1 \
  COMPOSER_HOME=/root/composer2 \
  COMPOSER1_HOME=/root/composer1
RUN cd /opt \
  # Download installer and check for its integrity.
  && curl -sSL https://getcomposer.org/installer > composer-setup.php \
  && curl -sSL https://composer.github.io/installer.sha384sum > composer-setup.sha384sum \
  && sha384sum --check composer-setup.sha384sum \
  # Install Composer 2 and expose `composer` as a symlink to it.
  && php composer-setup.php --install-dir=/usr/local/bin --filename=composer2 --2 \
  && ln -s /usr/local/bin/composer2 /usr/local/bin/composer \
  # Install Composer 1, make it point to a different `$COMPOSER_HOME` directory than Composer 2, install `hirak/prestissimo` plugin.
  && php composer-setup.php --install-dir=/usr/local/bin --filename=.composer1 --1 \
  && printf "#!/bin/sh\nCOMPOSER_HOME=\$COMPOSER1_HOME\nexec /usr/local/bin/.composer1 \$@" > /usr/local/bin/composer1 \
  && chmod 755 /usr/local/bin/composer1 \
  && composer1 global require hirak/prestissimo \
  # Remove installer files.
  && rm /opt/composer-setup.php /opt/composer-setup.sha384sum

Not actually something I would be proud of in 10 years from now, but sorta works. What do you think?

@le0m
Copy link
Member Author

le0m commented Nov 1, 2020

LGTM
The only problem I see with this solution is that users of this image could get an error if their application is not compatible with Composer 2. We could add a note in the README about composer1/composer2 differentiation.

fquffio added a commit that referenced this issue Nov 1, 2020
@fquffio
Copy link
Collaborator

fquffio commented Nov 1, 2020

Yep, but that's nice to add in any case, even if we would proceed with separate builds, as they would need to change the base image they build upon, and choose the one with Composer 1 instead.

@le0m
Copy link
Member Author

le0m commented Nov 1, 2020

We could've kept the current tags for Composer 1 images and introduce new tags for Composer 2, but that would probably have required more work and the solution you submitted makes more sense to me. I'll add a note in the README.

@le0m le0m closed this as completed in #74 Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants