Renovate: registryUrls inconsistency in composer

[rarkins]

registryUrls has been defined as an array of strings until now, but recent changes to the composer extract function resulted in an array of objects. We need to work out if we can refactor these to be strings (e.g. perhaps of the syntax type:url) or if we should support complex url types officially.

Cc @Tanuel @ViceIce

[viceice]

🤔 I think we should wait for the datasources converted to ts. then we can see where the registryUrls are also used.

Also the worker code is creating the different manager interfaces, i would like to see how thay interact.
Then we can maybe unify them. this would be more simple with typescript.

[Tanuel]

recent changes to the composer extract function resulted in an array of objects

I am a bit confused here, are you referencing 19e839fc5add84356715760e88885de5f0b4ff28? extract.js used to just take the whole repositories field and put it into registryUrls, thus passing an array of objects. Now git and vcs simply get filtered.

Regarding registryUrls, how is the situation after https://github.com/renovatebot/renovate/commit/6f99118f7cfb9a646148455938793c8894d10a01#diff-ba74feb6a035e79df57b8bacdde0cc9cR9?

[rarkins]

That's right. In all other managers, registryUrls is an array of strings, so now it is inconsistent if Composer uses and array of objects. Also, it removes the ability of users to manually override registries via Renovate config as they can only enter strings and the datasources expect objects.

[viceice]

So we have two options:

1. allow the new object format for all, which will be a lot of work
2. convert composer objects to string as suggested

[rarkins]

@Tanuel does composer essentially just use two fields, e.g. { "type": "x", "url": "y"} ? If so can we refactor it to be like x:y ?

[Tanuel]

As far as i understand, only `"type": "composer" is relevant for registry urls. https://getcomposer.org/doc/04-schema.md#repositories

[viceice]

https://github.com/renovatebot/renovate/commit/6d681d5b640c83652c01e6165f2811a4019a2ed5#diff-e6b5c426e595ef01911f6e276f50652f

Thats the introducing commit

[Tanuel]

The types vcs (including git) and package do not use a registry lookup, but contain the actual source url for the datasource.

The type pear can mostly be ignored for now imo, since everyone uses composer nowadays and pear as a package manager is mostly dead. In case anyone needs pear, this should be a separate issue.

So we can just pass the url to registryUrls if it exists and ignore the type for now (unless it is vcs/git which is handled already)

[Tanuel]

The line in question in extract.ts would be
https://github.com/renovatebot/renovate/blob/6f99118f7cfb9a646148455938793c8894d10a01/lib/manager/composer/extract.ts#L47

case 'composer':
    if('url' in repo){  // potential null check required here?
        registryUrls.push(repo.url);
    }

And then https://github.com/renovatebot/renovate/blob/6f99118f7cfb9a646148455938793c8894d10a01/lib/datasource/packagist/index.js#L254-L267 needs to be refactored as well

Also, with this change packagist.org disabling needs to be handled differently

EDIT: Waiting for datasource migration to typescript (https://github.com/renovatebot/renovate/issues/4171#issuecomment-514910278) sounds reasonable

[viceice]

I think we can just remove the other if's, because they only log that they are ignored.

[Tanuel]

We still need a way to disable/enable packagist.org
https://github.com/renovatebot/renovate/blob/6f99118f7cfb9a646148455938793c8894d10a01/lib/datasource/packagist/index.js#L271-L278

But i guess we can just add packagist.org as a registryUrl within extract.js? (packagist needs to be last in the registry list)

[rarkins]

Yes:

• when it’s not disabled, add it as the last registry url
• when it’s disabled, don’t add it
• make sure packagist datasource doesn’t add it back in cases where it’s given a list of registry URLs in the config

[Tanuel]

So should we wait until datasources are migrated to typescript or refactor this right away?

[viceice]

You can do it now, I've not yet started to convert datasources

[Tanuel]

@rarkins you can assign this one to me

[renovate-bot]

:tada: This issue has been resolved in version 19.18.1 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: