action-dependabot-auto-merge icon indicating copy to clipboard operation
action-dependabot-auto-merge copied to clipboard

process semi-semver versions

Open peterbe opened this issue 3 years ago • 6 comments

 using workflow's "target": 
- match:
    dependency_type: all
    update_type: 'semver:minor'

title: "Bump ahmadnassri/action-dependabot-auto-merge from v2.1.4 to v2.2"
depName: ahmadnassri/action-dependabot-auto-merge
Warning: failed to parse title: no recognizable versions

From https://github.com/mdn/yari/pull/1514/checks?check_run_id=1290814201 and https://github.com/mdn/content/pull/56/checks?check_run_id=1290858376

There's definitely a from part in the title this time.

Is this perhaps something that was fixed in >2.1.4.

peterbe avatar Oct 22 '20 12:10 peterbe

fun! and also, meta!

ahmadnassri avatar Oct 22 '20 20:10 ahmadnassri

it's actually because 2.2 is not a valid semver:

https://github.com/ahmadnassri/action-dependabot-auto-merge/blob/bacda948a54941e442c216f4c1c84240c8f9c04a/action/lib/parse.js#L44-L51

the mysteries of why and how dependabot decides what number to use is bizzare .... (or why sometimes it doens't include the from version as we've seen before)

so the issue here is how to parse / identify version numbers such as v1, 2.2, 2 as valid and process them accordingly...

here's another example: https://github.com/ahmadnassri/action-dependabot-auto-merge/pull/6

I have thoughts on this (not only does the regex need updating, but also, need to convert to equivilent semver so that the weight measurment later can continue to function), but i'm currntly sick, prob wont get to it this week.

ahmadnassri avatar Oct 22 '20 20:10 ahmadnassri

you could also see something wrong with that PR ... dependabot derped and included .releaserc file as a changelog?!?!

image

I wonder if they are experimenting with updates in production!

ahmadnassri avatar Oct 22 '20 20:10 ahmadnassri

I have a similar issue with graalvm (21.0.0 to 21.0.0.2): https://github.com/PhoenicisOrg/phoenicis/pull/2365.

plata avatar Feb 21 '21 13:02 plata

I have similar issue when the version number consists of 4 numbers. Here's an output from the action:

using workflow's "target": 
- match:
    dependency_type: all
    update_type: 'semver:patch'

title: "Bump AWSSDK.S3 from 3.7.7.9 to 3.7.7.10"
depName: AWSSDK.S3
from: 3.7.7
to: 3.7.7
dependency type: production
security critical: false
config: all:semver:patch
Warning: no version range detected in PR title
manual merging required

Can the regex be extended to also include the fourth number, which is a build increment or revision? While I do appreciate that version with 4 groups is not a proper SemVer version, there are packages that still use this approach for versioning and dependabot successfully creates PRs for those.

andrii-litvinov avatar Jan 10 '22 15:01 andrii-litvinov

I have the reverse problem from @peterbe, the first number has less 'parts' than the second.

title: "Bump checkstyle from 10.2 to 10.3.1"
depName: checkstyle
from: unknown
to: 10.3.1
dependency type: production
security critical: false
config: all:semver:minor
Warning: no version range detected in PR title

In time, every sufficiently complicated regex becomes a PITA and wants to be converted to a parser. This would make sense here, because you want to be lenient in what you accept.

In my example, I definitely expect 10.2 to be handled as 10.2.0 for the purpose of comparing it to 10.3.1.

So you can be correct or ergonomic, you can't be both. Since the user of your action has no control over the nonsense of third party package versioning shenanigans, I'd argue ergonomic is more important.

There's already 3 regression tests in this issue that you can add to any future test suite of the new lenient behaviour. So you're not starting from zero :).

lestephane avatar Jul 14 '22 09:07 lestephane