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

Auto-merge action doesn't work with digest updates

Open JustArchi opened this issue 3 years ago • 6 comments

Hey.

I believe that the action should be aware of digest versions, such as git submodules, or simply versions that do not follow semver for one reason or another.

Old dependabot assumed those to be on the level below patch updates: major > minor > patch > digest.

Right now the action simply fails to do anything with digest updates: example. I do not expect from the action to do anything smart with versions that do not follow semver, but it'd be nice to enable auto-merge support for them. All current targets (major, minor and patch) should probably include digest by default, and digest should probably be added as yet another level to merge. This is how it worked in "old" dependabot, but I'm also fine with any other way to specify that digest updates should also be merged together with one of the currently available 3 options for semver updates.

Thank you in advance for considering this enhancement.

JustArchi avatar Sep 27 '20 00:09 JustArchi

isn't that satisfiable by setting update_type: all

https://github.com/ahmadnassri/action-dependabot-auto-merge#match-properties

ahmadnassri avatar Sep 27 '20 02:09 ahmadnassri

also, do you have a reference to the "old" dependabot example of this? I don't believe i've come across this

ahmadnassri avatar Sep 27 '20 02:09 ahmadnassri

@ahmadnassri I checked the code and various examples, the current code fails at interpreting the semver version because it expects all updates to be semver-compatible. Digest doesn't parse to a semver, and therefore the script is aborted, check my example above. I didn't have success with update_type: all.

As of dependabot example, you can just add a git submodule to your project and make it update with auto-merging. They added digest update initially for docker, but it's also being used in git submodules for example.

Example PR for auto-merging digest updates: https://github.com/JustArchiNET/ArchiSteamFarm/pull/1988

JustArchi avatar Sep 27 '20 10:09 JustArchi

Also, I'd like to have a config similar to what I have right now in old dependabot, that is:

  • all git submodules updates are merged
  • only minor semver changes are merged for everything else

I'm fine with the config that specifies names of git submodules dependencies, as a workaround for hard work in regards to detecting whether the PR is git submodules or not, as already possible in your example, but is that even possible?

- match:
    dependency_name: ASF-ui
    update_type: all

- match:
    update_type: semver:minor

JustArchi avatar Sep 27 '20 10:09 JustArchi

Also, I believe that even without custom matches, the script should not throw at semver parsing, as it does right now. The ideal solution would be detecting that this is a digest update (which is easy, it just doesn't parse to a semver), and assume that to be included in all currently available targets, starting from patch. This is probably the least amount of work required to satisfy my use case, and it'd also work with all other types of dependabot updates that do not follow semver.

The question remains if above match block is valid, because I can totally see how somebody would not want to update some package in digest, yet want to have all other packages handled.

All of those should be valid imho:

# This should have no effect, as semver:minor should include digests by default
- match:
    dependency_name: ASF-ui
    update_type: all

- match:
    update_type: semver:minor
# This should be possible to disable auto-merge for a specific package, but still allow that for all other ones
- match:
    dependency_name: ASF-ui
    update_type: none

- match:
    update_type: semver:minor
# We should also be able to merge only digest updates
- match:
    update_type: digest

Which comes down to adding support for digest update types (adding it to the config and match blocks), detecting packages that are not following semver to be digest, and assuming digest is below patch level if semver is specified. As a wishlist, adding a way to specify custom matching settings for one package while having other setting for all other packages is also cool - maybe this is even already possible, I didn't check. I believe that should be all to fully support my example case.

JustArchi avatar Sep 27 '20 10:09 JustArchi

I'm still reading and thinking about this, hopefully by the weekend, I would have more free cycles to respond / address.

ahmadnassri avatar Oct 01 '20 12:10 ahmadnassri