renovate
renovate copied to clipboard
Switch depName/packageName logic
What would you like Renovate to be able to do?
Instead of defaulting to depName
, and allowing packageName
as an optional override, we should instead default to extracting packageName
, and allowing depName
optionally.
If you have any ideas on how this should be implemented, please tell us here.
We would need to make changes to each manager's extract function, which will touch most managers but not be complex.
There may be a small change to datasource
index logic because now it does not need the depName->packageName fallback.
But then there are other areas of the code which assume depName
is always present.
One in particular is packageRules
. Today matchPackage*
actually functions on the depName
field and not the packageName
. We might need to do something like:
- Try matching against
packageName
first - If a depName is present, try matching against it second
Or maybe instead we could:
- add new
matchDepNames
andmatchDepNamePatterns
fields - Do like above, but WARN if the match is on
depName
for one major release - Deprecate the
depName
matching fallback in a later release
Ultimately this issue is going to require some exploration and testing - it's hard to predict everywhere it might cause changes
Is this a feature you are interested in implementing yourself?
Maybe
sounds good, i think we can do it in multiple steps.
let's clear some things up:
- We will need to fix every extract function for every manager? can we do it 1 manager at a time in small tasks or 3-4 managers at a time depends on how long it takes for one? what about templates?
- For our information: What's the difference between
packageName
anddepName
? - and what about current users.. we wont have backward compatiblity causing this to be BREAKING changes?
- We will need to fix every extract function for every manager? can we do it 1 manager at a time in small tasks or 3-4 managers at a time depends on how long it takes for one? what about templates?
I think it's easiest to do them all at once.
We will need to think about the repercussions for regexManagers
.
- For our information: What's the difference between
packageName
anddepName
?
By default, only depName
is needed and that's what's used to look up packages. If packageName
is also specified then it means depName
is essentially "display name" while packageName
is "lookup name".
- and what about current users.. we wont have backward compatiblity causing this to be BREAKING changes?
We would aim for it not to be breaking
So regex manager should set packageName=depName
if only depName
is matched
that's what i was thinking about current users, they will need to stop using depName in their regex managers, and start using packageName... it would be breaking i believe
That's not what @viceice meant. He meant:
- if both
depName
andpackageName
are in the regex manager result then keep it as-is - if only
depName
is found then rename it topackageName
once extraction/template compilation is done
Today config.packageName
is optional and config.depName
is Required.
config.packageName
getting the value of config.depName
:
https://github.com/renovatebot/renovate/blob/e4dbd4ad491f656aaa7f9da9d4dae2c18622a1e6/lib/modules/datasource/index.ts#L344
in packageRules we have matchPackagePatterns
and matchPackageNames
these matchers are related to packgeName
.
definition: https://github.com/renovatebot/renovate/blob/e4dbd4ad491f656aaa7f9da9d4dae2c18622a1e6/lib/util/package-rules.ts#L41-L42
usage:
https://github.com/renovatebot/renovate/blob/e4dbd4ad491f656aaa7f9da9d4dae2c18622a1e6/lib/util/package-rules.ts#L160
Instead of trying matching against depName
, we should change it to be against packageName
.
then we should add 2 new matchers to the depName
(matchDepPatterns
, matchDepNames
).
need to check if depName
matched matchDepNames
and if packageName
matched matchPackageNames
- if yes keep the values as is
- else
- if just
depName
matched so setpackageName=depName
.
- Switch all managers to extract
packageName
by default instead ofdepName
- Add new match fields to packageRules
- Add logic to match matchPackageX against packageName first, then try depName. If packageName doesn't match but depName does, we should warn the user
- Massage depName to be packageName if it's empty
I don't think there's any reason to set packageName=depName
as you mentioned
Landing here from a Google search while trying to figure why I suddenly(?) get this warning on a project in a local on-prem Gitlab instance running with whitesource/renovate:5.0.0
:
With a Renovate Bot configuration like:
{
...
"regexManagers": [
{
"description": "*_VERSION variables in Dockerfiles",
"fileMatch": ["(^|\\/|\\.)Dockerfile$", "(^|/)Dockerfile\\.[^/]*$"],
"matchStrings": [
"# renovate: datasource=(?<datasource>[a-z-]+?) depName=(?<depName>.+?)(?: (?:packageName|lookupName)=(?<packageName>.+?))?(?: versioning=(?<versioning>[a-z-]+?))?\\s(?:ENV|ARG) .+?_VERSION=(?<currentValue>.+?)\\s"
],
"versioningTemplate": "{{#if versioning}}{{versioning}}{{else}}semver{{/if}}"
}
]
}
In combination with a Dockerfile like:
...
# renovate: datasource=maven depName=com.microsoft.playwright:playwright
ENV PLAYWRIGHT_VERSION=1.36.0
...
Strangely enough, this warning doesn't pop up on another Git project with an almost identical configuration 🧐
So is this related to the current issue?
@dalbani better if you start a fresh discussion about this, ideally with a reproduction repo if it's possible.
In short: matchPackageNames used to match against depName
, not packageName
. We're trying to transition without breaking people, so now we support matchDepNames
too and warn someone if there's code which should change from matchPackageNames
to matchDepNames
. It's possible that you're getting a warning due to some built-in presets which include packageRules (i.e. not directly from your repo)
Do I see it correctly that there are still some presets using matchPackageNames
instead of matchDepNames
that should be updated?
Searching with https://github.com/search?q=repo%3Arenovatebot%2Frenovate%20path%3A%2F%5Elib%5C%2Fconfig%5C%2Fpresets%5C%2F%2F%20matchPackageNames&type=code, I see quite a few presets.
We are getting the Use matchDepNames instead of matchPackageNames
warning e.g. for node
.
If I'm readying this right, I'm happy to open a PR that replaces all matchPackageNames
in the presets with matchDepNames
if that's the correct approach - or do we need to keep some matchPackageNames
occurrences?
@morremeyer yes I think you have it right. I haven't yet done a search through our presets to see if any should swap from matchPackageNames to matchDepNames, so it would be appreciated if you can do that and raise a PR!
BTW the only time we need matchDepNames is when depName and packageName are different, and node
is one of those because sometimes we massage them (e.g. from amd64/node
to node
)
@rarkins But we do still need either of the two for a packageRule
and as far as I understand this issue, the move is towards using depName
, is that correct?
To be honest, every time I read up on the difference between the two, I forget it very quickly afterwards and there's no explanatory documentation about the differences and intricacies of the two, so I need to get myself up to speed every time 😁
Might be worth to add that somewhere.
The way it works today is: always have depName, use packageName optionally if the "lookup name" is different. Warn if a matchPackageNames matches depName but not packageName.
The way it will work in future is: always have packageName, use depName optionally as a "pretty name"/"short name". Hence why we warn today, because in future we don't fall back to checking depName for matchPackageNames.
packageName
is probably still right for the actual package name we look up (note: used to be called lookupName
). We can reconsider depName
after the refactor
I would like to achieve this by having more strict typings for manager extractions.
Dependencies need to have datasource
and packageName
at minimum, while depName
and registryUrls
are optional.
We sometimes also extract incomplete and skipped dependencies, in which case skipReason
is set. I don't see the benefit of extracted dependencies with only skipReason
set and think we should include a currentValue
at least too. The benefit of including skipped dependencies is that they're shown in the logs instead of invisible
One in particular is packageRules. Today matchPackage* actually functions on the depName field and not the packageName.
Is this statement still true?
Today matchPackage* tries matching packageName first, then falls back to depName, and logs an info message if depName matches when packageName did not: https://github.com/renovatebot/renovate/blob/14272f07a26fe13b11aa207fc7aba108bc764277/lib/util/package-rules/package-names.ts#L23-L27
We should raise this to a warning, and then after that remove the fallback.
Quick update regarding my comment https://github.com/renovatebot/renovate/issues/16012#issuecomment-1717497623 if anyone is looking for that: This change has been implemented by someone else in the meantime. Thank you!
Proposed typing change to lib/manager/types
:
Needs more context but looks ok to me
Does this documentation need to be changed from depName
to packageName
?
https://docs.renovatebot.com/modules/manager/regex/#advanced-capture
See https://github.com/renovatebot/renovate/pull/28834