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

Discussion: Composer support (PHP) #1357

Closed
hkdobrev opened this issue Jan 8, 2018 · 30 comments
Closed

Discussion: Composer support (PHP) #1357

hkdobrev opened this issue Jan 8, 2018 · 30 comments
Labels
manager:composer Composer (PHP) package manager

Comments

@hkdobrev
Copy link

hkdobrev commented Jan 8, 2018

Hi! This is a great tool and I've seen how it gets used by the folks at Algolia on their public repositories. We're just integrating it ourselves for our Yarn-based flow and npm packages.

However, we are also using Composer for PHP packages. Do you think this is something you'd consider adding?

Would you point me how to add another package manager? I couldn't find an article about that in the docs.

Thank you!

@rarkins
Copy link
Collaborator

rarkins commented Jan 8, 2018

I would definitely be happy to add composer support, especially if you are able to help out.

Package manager support needs a bit more refactoring to be “easy” to add a new one, so I plan on that first.

For composer, do you think the parsing, lookup and logic can be done all in JavaScript, or more effective to spawn out to a cli command for things like lookups and upgrade decisions and then parse the stdout or resulting file?

@rarkins rarkins added type:feature Feature (new functionality) help wanted Help is needed or welcomed on this issue needs-requirements priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others labels Jan 8, 2018
@hkdobrev
Copy link
Author

hkdobrev commented Jan 8, 2018

I think it's best to spawn to commands in order to be consistent with the regular usage of Composer. I think Composer parsing of tilde ranges is a bit different than npm's so it'd be best to use the Composer one. For upgrade decisions, we should definitely use Composer commands, because there are lots of things to consider including platform configurations, extensions listed as requirements etc. which would be a mistake to repeat in a separate codebase.

@alexander-akait
Copy link

@rarkins Can i help you with this issue, maybe you can point on some places where i start implement?

@rarkins
Copy link
Collaborator

rarkins commented May 7, 2018

@evilebottnawi that would be awesome.

Adding new package managers - basic support at least - is now fairly easy. See python and buildkite.

I also added a doc about it here: https://github.com/renovateapp/renovate/blob/master/docs/adding-a-package-manager.md

Adding "advanced" managers has a couple of challenges right now though:

  1. If non-JS package managers have their own range syntax/logic different from semver we use in JS, then we either need to reimplement the logic in JS or call our to a child process. I no examples of this yet.

  2. Our lock file logic is essentially hardcoded to npm/yarn

I am doing a major refactor right now to address (2) and many other smaller pain points so that new package managers can get all the advanced features we have for npm/yarn, without having to reimplement them all (e.g. pinning, separating major/minor PRs, etc).

The best thing you could help with right now is to investigate the feasibility of porting Composer's semver implementation to JS. It's here: https://github.com/composer/semver

Option 1: completely rewrite, but piggy backing on the JS semver logic we already have.
Option 2: use machine translation for PHP -> JS and then write a thin wrapper to adjust it for our needs

@alexander-akait
Copy link

@rarkins Thanks for clarify

Option 1: completely rewrite, but piggy backing on the JS semver logic we already have.
Option 2: use machine translation for PHP -> JS and then write a thin wrapper to adjust it for our needs

try to find better way for this 👍

@alexander-akait
Copy link

Just information: https://gitlab.com/kornelski/babel-preset-php looks very good to transpile php semver to js semver without rewriting

@rarkins
Copy link
Collaborator

rarkins commented May 8, 2018

@evilebottnawi that looks interesting - I hadn't noticed it previously.

At a high level, I had thought it might be a two step process:

  1. Machine translation (e.g. PHP -> JS in this csae)
  2. Either JS codemods, or some handwritten code to adapt it to our requirements, e.g. expose functions with consistent names we use across all languages like "isValidVersion()", etc.

If you can solve (1) then I can probably do most of (2)

@alexander-akait
Copy link

@rarkins i try to do this in this week, i think we can do this without codemods, it is looks just tiny wrapper around transpiled code

@rarkins
Copy link
Collaborator

rarkins commented Jun 6, 2018

@evilebottnawi the WIP PR for Composer support (#1993) is now ready for the versioning part, if you're still interested

@rarkins
Copy link
Collaborator

rarkins commented Jun 7, 2018

Outstanding issues for Composer support:

@alexander-akait
Copy link

@rarkins good work!

@rarkins rarkins changed the title Composer support Composer support (PHP) Jun 7, 2018
@rarkins rarkins changed the title Composer support (PHP) Composer lockfile support (PHP) Jul 5, 2018
@rarkins rarkins changed the title Composer lockfile support (PHP) Composer support (PHP) Jul 5, 2018
@rarkins
Copy link
Collaborator

rarkins commented Jul 19, 2018

Composer support is now nearly ready for enabling by default. It supports versions, ranges and lock file updating. I will keep this issue open until I consider Composer support to be fully mature, and post updates here until then so you can be informed as new features and fixes are added.

@swashata
Copy link

Thank you @rarkins You've been awesome...

@swashata
Copy link

I just like to say that I've tried on a bunch of my repos and I can say the following things work

  1. pin dependencies.
  2. composer.json update.
  3. composer.lock update.

You can see here https://wpquark.io/swashata/composer-renovate-test/merge_requests ;)

@alexander-akait
Copy link

@rarkins will be great also support custom register, example

repositories: [
    {
      "type": "composer",
      "url": "https://wpackagist.org"
    }
];

Example package "wpackagist-plugin/wordpress-seo": "~7.7.2", maybe better create new issue

@rarkins
Copy link
Collaborator

rarkins commented Jul 19, 2018

Please create a new feature request for custom registries and include any info you have on “how it’s usually done”, etc. Eg if there are ways it’s defined within the registry itself, or always out of band.

@rarkins rarkins removed type:feature Feature (new functionality) help wanted Help is needed or welcomed on this issue in progress priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others labels Jul 22, 2018
@rarkins rarkins added the manager:composer Composer (PHP) package manager label Jul 23, 2018
@rarkins rarkins changed the title Composer support (PHP) Discussion: Composer support (PHP) Jul 27, 2018
@robinvdvleuten
Copy link

@rarkins I've enabled composer support for a private repository but don't receive any PRs for updated dependencies. I've checked if there are any through composer outdated and all JS PRs works as expected (so I assume Renovate is correctly configured). My renovate.json looks like this;

{
  "extends": ["config:js-app"],
  "composer": {
    "enabled": true
  },
  "labels": ["chore", "dependencies"]
}

@rarkins
Copy link
Collaborator

rarkins commented Aug 2, 2018

@robinvdvleuten can you repost this to the renovatebot/config-help repo and include an extract of what your composer.json consists of? I will post back here a summary when we’re done so I can keep this “announcement” thread low noise for those subscribed.

@robinvdvleuten
Copy link

@rarkins sure! see renovatebot/config-help#73

@rarkins
Copy link
Collaborator

rarkins commented Aug 7, 2018

I made three fixes/improvements to PHP logic yesterday:

  • run composer update with the --ignore-platform-reqs flag now
  • remove scripts from composer.json before running composer update
  • write a temporary auth.json next to composer.json with github.com token, to avoid hitting rate limits

Also note: composer is not yet installed into the production Renovate app, meaning that composer.lock files won't be updated when using the app. I want to make sure we've done a full security risk assessment first to make sure it won't be possible to take down the app either deliberately or accidentally with a "bad" PHP config. The lock file updating features work if you run the tool yourself (e.g. from CLI) with composer installed on the same machine.

@swashata
Copy link

swashata commented Aug 7, 2018

@rarkins I think we need a way to restrict dependencies within renovateapp for platform config present in the composer.json file.

Because in many cases, we develop and build under PHP7 env, whereas we want dependencies to be compatible with PHP 5.6. So in my project I have set platform config in composer.json as php: "5.6". Composer update handles it quite well but renovateapp creates PR for versions I can not update to.

@rarkins
Copy link
Collaborator

rarkins commented Aug 7, 2018

@swashata I have raised this as a new issue #2355

@rarkins
Copy link
Collaborator

rarkins commented Sep 28, 2018

New: Composer lock updating is now live in the hosted app

@felixfbecker
Copy link

Nice, now the only thing left for feature parity with dependencies.io is transitive dependency updates (#1382) 🎉

@AlMcKinlay
Copy link

Is transitive dependencies on the horizon? I love renovate, and it's great, but I have to manually check out the branches to update the transitive dependencies just now. Especially when an update requires a transitive update, I'm not able to merge until I've done that.

@rarkins
Copy link
Collaborator

rarkins commented Apr 9, 2019

@McInkay could you give an example, especially if you have one where transitive dependency updates are required?

@AlMcKinlay
Copy link

Here's one where the drush update required an update to a dependency, so I needed to manually run composer with --with-dependencies. Once I did that, it was all sorted. I think you should just need to add --with-dependencies to the command run to updated the artifacts.

YaManicKill/viewfield#4

@rarkins
Copy link
Collaborator

rarkins commented Apr 9, 2019

@McInkay I've added --with-dependencies to our default options for Composer. Let me know if you notice one in future that has the same failure case.

@AlMcKinlay
Copy link

Great, thanks :-)

@rarkins
Copy link
Collaborator

rarkins commented Jun 17, 2019

I'm going to close this issue now, as it has solved its purpose and Composer support is pretty solid. New issues for features are always welcome.

@rarkins rarkins closed this as completed Jun 17, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
manager:composer Composer (PHP) package manager
Projects
None yet
Development

No branches or pull requests

7 participants