matrix.config.r and matrix.config.id is not always present

[pascalgulikers]

actions/check-r-package/action.yaml

Line 57 in 55acd27

name: ${{ runner.os }}-${{ runner.arch }}-r${{ matrix.config.r }}-${{ matrix.config.id || strategy.job-index }}-results

actions/check-r-package/action.yaml

Line 64 in e40ad90

name: ${{ runner.os }}-r${{ matrix.config.r }}-testthat-snapshots

This could result in:

Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run

because of uninitialized variables:

Run actions/upload-artifact@v4
  with:
    name: Linux-X64-r-0-results

and

Run actions/upload-artifact@v4
  with:
    name: Linux-r-testthat-snapshots

[gaborcsardi]

How does this cause problems?

[pascalgulikers]

When you have your own matrix setup, it will fail because of naming conflicts, for example:

jobs:          
  Testing:
    strategy:
      matrix:
        tests:
          - {languageVersion: 'latest', appType: 'library', appName: 'MyLibraryApp'}
          - {languageVersion: 'latest', appType: 'job', appName: 'MyJobApp'}
          - {languageVersion: 'latest', appType: 'shiny', appName: 'MyShinyApp'}

[gaborcsardi]

The config.id column is designed to solve this. Add a config.id column to your matrix.

Alternatively, you can turn off uploading artifacts.

[pascalgulikers]

There is no config array in my matrix (in my case I named it 'tests') and it's not obligated by Github either, that's why it will only work if you have a config array in your matrix definition but not everybody will have this.

[gaborcsardi]

I know. But again, I am not sure what we can do here, apart from including a "random" string in the artifact name, which is also not great. How do you suggest we make the artifact names unique?

You can either name your array config, or turn off the artifacts for check-r-package.

[pascalgulikers]

I just think it shouldn't be hard-coded because then, by default, it will only work with provided example workflows.
Before this commit is was working though:
adb0e10

So maybe a combination with a run-id?

[gaborcsardi]

It is not hardcoded, because you can change it.

It certainly works with other workflows as well, as long as you have either of these conditions:

• only one job that uploads check-r-package artifacts, or
• you have a config.r entry in your matrix, with different values,
• you have a config.id entry in your matrix, with different values.

This was not a problem with the old version of the actions/upload-artifact action, but not updating to the new version is not an option.

run_id is the same for all jobs in your matrix.

A "random" id is not great, because you cannot use that programmatically. E.g. you cannot easily call a download-artifact action in the next step. So I am not planning on making the artifact name random.

[pascalgulikers]

By hard coded I meant the fact that the matrix array always has to be named config and not something else.

I've tried renaming mine to config though, and specified an id entry in it as well, but unfortunately that didn't work either.
Probably because my setup is chained (workflow -> reusable workflow -> custom composite action -> r-lib/check-r-package action).
I've therefor disabled the uploading of artifacts/results and it's working again now :)
Since this was a breaking change (in our case), a major bump to v3 would have been better because then we would not have been surprised by this sudden change.

But, thankful for your work regardless! They are lifesavers nevertheless :)

[gaborcsardi]

Yes, that is unfortunate, and we could have explicit artifact-name and snapshot-artifact-name parameters that takes precedence over the current artifact name scheme.

Since this was a breaking change (in our case), a major bump to v3 would have been better because then we would not have been surprised by this sudden change.

Yes, I considered that, but decided against, because it does not affect the workflows that follow our examples, and we do not routinely require our users to update. OTOH everybody has to update to checkout@4 soon, anyway, so it is not clear that it was the better option.

[gaborcsardi]

I'll keep this open to add artifact-name and snaphot-artifact-name arguments to check-r-package.

[github-actions]

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue and include a link to this issue