renovatebot / config-help

Please use the Discussions feature of https://github.com/renovatebot/renovate instead
https://github.com/renovatebot/renovate/discussions
MIT License
27 stars 16 forks source link

Renovate created PR for pre-release upgrade #994

Closed hisener closed 3 years ago

hisener commented 3 years ago

What Renovate type, platform and version are you using?

Self-hosted Renovate Open Source CLI (using the docker image renovate/renovate:23.77.0) on GitHub

Describe the bug

As the summary says, we have an upgrade PR for a pre-release version. You can see here https://github.com/PicnicSupermarket/oss-parent/pull/81. Also, the log says minor for updateType. 🤷‍♂️

Relevant debug logs

{
  "datasource": "maven",
  "depName": "org.immutables:value-processor",
  "currentValue": "2.8.8",
  "fileReplacePosition": 4891,
  "registryUrls": ["https://repo.maven.apache.org/maven2"],
  "groupName": "version.immutables",
  "depIndex": 28,
  "updates": [
    {
      "fromVersion": "2.8.8",
      "toVersion": "2.8.9-ea-1",
      "newValue": "2.8.9-ea-1",
      "newMajor": 2,
      "newMinor": 8,
      "updateType": "minor",
      "isSingleVersion": true
    }
  ],
  "warnings": [],
  "fixedVersion": "2.8.8",
  "dependencyUrl": "org/immutables/value-processor"
},

To Reproduce

It should be producible by adding org.immutables:value-processor:2.8.8 dependency. I haven't tested, so maybe it's related to <version.immutables>2.8.8</version.immutables> property.

Additional context

N/A

rarkins commented 3 years ago

My understanding is that Maven versioning - unlike semver - does not specify that a version suffix automatically means it is unstable.

Can Renovate be configured to ignore this type of version? Yes

Should Renovate ignore this type of version by default? I would like to see some reference/specification before agreeing that this is a bug or something we should do as default behavior.

For example, Google's popular guava library has versions like 28.0-android. They are stable. Why would 2.8.9-ea-1 be assumed to be unstable?

rarkins commented 3 years ago

If we cannot conclude that Renovate has a way to reasonably determine such releases as unstable, then our matchCurrentVersion and allowedVersions package rule fields may be helpful, e.g. something like:

{
  "packageRules": [{
    "datasources": ["maven"],
    "matchCurrentVersion": "/^\\d+\\.\\d+\\.\\d+(-RELEASE)?$/",
    "allowedVersions": "/^\\d+\\.\\d+\\.\\d+(-RELEASE)?$/"
  }]
}

This way if the current version is semver-ish then it will only propose updates which are semver-ish

hisener commented 3 years ago

Ah, I thought Guava was the exception as you mentioned here https://github.com/renovatebot/config-help/issues/529#issuecomment-586907819. Then, it makes sense.

I am not sure about changing it globally because it might break others. I think I will add the following rule:

{
    packageNames: ['version.immutables'],
    allowedVersions: '!/^.+-ea-\\d+$/'
},

Alternatively, we can switch to semver for immutables dependencies. (Something like 'versioning': 'semver'). (I'll try this one first.)

rarkins commented 3 years ago

I still think it's a mistake for projects to version in this way, but I was informed by some from the Maven world that it's not a forbidden approach.

Re: your proposed rule, I think you'd be better to use packagePatterns. packageNames won't match the group name you have.

hisener commented 3 years ago

I still think it's a mistake for projects to version in this way, but I was informed by some from the Maven world that it's not a forbidden approach.

X.Y.Z-RC1 is very common but looks like there is a specific logic for that. Should we also accept ea as a release candidate here?

Re: your proposed rule, I think you'd be better to use packagePatterns. packageNames won't match the group name you have.

Check.

I have used the following rule then the PR (https://github.com/PicnicSupermarket/oss-parent/pull/81) is autoclosed.

{
    packagePatterns: ['^org\\.immutables:.+$', 'version\\.immutables'],
    versioning: 'semver'
},

Thanks for the help @rarkins. 🙏

rarkins commented 3 years ago

Is EA a commonly used pre-release identifier? I forget if we got the list from maven's or we created it ourselves. We generally prefer to implement to spec but the spec is unfortunate in this case.

hisener commented 3 years ago

Is EA a commonly used pre-release identifier? I forget if we got the list from maven's or we created it ourselves. We generally prefer to implement to spec but the spec is unfortunate in this case.

There are some links at the top of the file. ea is not mentioned in https://maven.apache.org/pom.html#Dependency_Version_Requirement_Specification. So probably we should leave it out.

rarkins commented 3 years ago

Agreed. Thanks for checking that out.

hisener commented 3 years ago

Thank you.