Don't ever update GitHub Actions unless they are actually pinned

[deivid-rodriguez]

The change in Dependency combine logic at #6082 surfaced a bug where if a Github Action is referencing two different branches in the same workflow, Dependabot would create a PR unifying it to a single branch, while is not what the user wants.

The fix is to ignore any uses: references which include a reference that should not be updated, like a branch name not named like a version (like master or stable).

In addition to fixing this bug, this change also allows Dependabot to create PRs for mixed situations like the following, where the version is updated and the others are left alone:

uses: dtolnay/rust-toolchain@master
# ...
uses: dtolnay/rust-toolchain@stable
# ...
uses: dtolnay/[email protected]

or the following, where the SHA is updated and the others are left alone:

uses: dtolnay/rust-toolchain@master
# ...
uses: dtolnay/rust-toolchain@stable
# ...
uses: dtolnay/rust-toolchain@df6c627e2064a425669c08e51ed613bb3f013c69

There's a change in behavior for other mixed cases like

uses: dtolnay/rust-toolchain@master
# ...
uses: dtolnay/[email protected]
# ...
uses: dtolnay/rust-toolchain@df6c627e2064a425669c08e51ed613bb3f013c69

Before Dependabot would find no updates, while now it will update both the SHA-pinned and the version-pinned lines to the latest version. I'm unclear about why users would pin some workflows and not others, so I'm fine with making this change and listen to any users that don't actually want this. My expectation is that this will help users catch situations where they were unintentionally using multiple styles and help them unify those.

Fixes #6739.

[mctofu]

This makes sense to me. I've seen that come up in support a few times where Dependabot didn't update actions because of a stray reference to a branch in one workflow so this seems like an improvement. Do we have any test cases that cover mixed usage of branch/sha/version?

[deivid-rodriguez]

I guess we don't have tests for those cases, since they were not working up until now and tests were still passing.

It'd be good to add those, but didn't want to delay the fix to this regression. Should I ship this, and then try add some more specs as a follow up?

[deivid-rodriguez]

I was thinking about the kind of test case that we should add. For this case, given that this involves both file fetching and update checking, I think it's probably best to add a few smoke tests.

[LingMan]

There should be no update in this scenario:

uses: dtolnay/rust-toolchain@master
# ...
uses: dtolnay/rust-toolchain@stable
# ...
uses: dtolnay/[email protected]

The rust-toolchain action uses branches like 1.42 to make available older compiler versions. Rust projects then usually test against both stable and against a minimal version they wish to support.

[deivid-rodriguez]

The rust-toolchain action uses branches like 1.42 to make available older compiler versions. Rust projects then usually test against both stable and against a minimal version they wish to support.

You mean that you'll never want any updates to a uses: dtolnay/[email protected] line, or that you don't want updates when included in combination with uses: dtolnay/rust-toolchain@stable or similar? Either way, proposing an update for an out of date version number is what Dependabot does with every other action, and seems like the right thing to do to me. If this particular action is special for your case, then you can always ignore updates for it.

[LingMan]

There should never be an update to a uses: dtolnay/[email protected] line. The 1.42 (and also the stable) refers to the version of the compiler the action should provide. It does not refer to the version of the action itself (master is really the all that's available for this action).

Updating a v1.42 to a v1.50 (notice the extra v) would be fine for me. That matches the way actions are usually versioned. If the v is missing, please just treat it as a string and not as a version number.

Ignoring this particular action is of course possible, but it would be a big pain. Large parts of the Rust ecosystem use dtolnay/rust-toolchain and ensure compatibility with older compilers in this way.

[deivid-rodriguez]

There should never be an update to a uses: dtolnay/[email protected] line. The 1.42 (and also the stable) refers to the version of the compiler the action should provide. It does not refer to the version of the action itself (master is really the all that's available for this action).

Updating a v1.42 to a v1.50 (notice the extra v) would be fine for me. That matches the way actions are usually versioned. If the v is missing, please just treat it as a string and not as a version number.

That deviates from how Dependabot works for every other action, so I don't think we will be changing this behavior. In any case, it would be a separate feature request. Dependabot has been creating PRs updating @1.42 lines for a long time, it just didn't do it when @stable or @master were also used in the same workflow, due to some bugs.

[LingMan]

Welp, looks like Dependabot and parts of dtolnay/rust-toolchain's intended usage are then incompatible. Let's hope most projects test their minimally supported Rust version as part of a matrix and and pass it in over the toolchain input instead of relying on the branch name.