Fixes artifact upload for platform cross building
This PR is an upstream port of https://github.com/typelevel/sbt-typelevel/pull/98, every commit has been cherry picked both for authorship acknowledgement and also to make it easy to correlate the commits. In the process of cherry picking I have also done modifications so that the project compiles and the tests run (i.e. for each cherry picked commit I ran sbt githubWorkflowGenerate for both the root build and every sbt scripted test). Exceptions are noted below
- I have added the
Handle case where githubWorkflowBuildMatrixAdditions is emptycommit myself as this appears to be an accidental regression. In sbt-typelevel there doesn't seem to be a case wheregithubWorkflowBuildMatrixAdditionsis empty but this is typically the case for standard trivial builds and the github workflow generation in this case appeared to be incorrect (i.e. trailing-and includingrootJVMby itself when not necessary). Please check if this is intended, particularly the removal of therootJVMpart (I may be missing something here) - I did not cherry pick the https://github.com/typelevel/sbt-typelevel/pull/98/commits/1801859216cca487d7fedb125ca08ecad9a8b55c commit as this appeared to be specific for typelevel-sbt and how they structure their projects. We should probably add a new commit that either modifies an existing test or makes a new one so we set
githubWorkflowArtifactDownloadExtraKeysto some value to test that it works @djspiewak @armanbilge wdyt? Feel free to just add the commit onto this PR if you want to do it yourself (maintainers can add commits onto this PR).
I think there's been additional fixes since the original PR. https://github.com/typelevel/sbt-typelevel/blob/5dec904d36c0b2cceb3f18b9ed7323ad849614a4/github-actions/src/main/scala/org/typelevel/sbt/gha/GenerativePlugin.scala#L725
I think you made the right choice on cherry picking. This looks good to me apart from the fact that I'd ideally like to see a test for it, but I'm okay either way.
Agreed about test, was just leaving it open if someone else wanted to suggest/add one on their own but if not I will add one later.
I think there's been additional fixes since the original PR. https://github.com/typelevel/sbt-typelevel/blob/5dec904d36c0b2cceb3f18b9ed7323ad849614a4/github-actions/src/main/scala/org/typelevel/sbt/gha/GenerativePlugin.scala#L725
Thanks, I just picked a PR from git blame but evidently I missed some stuff. Will look into it later
I just came back from a wedding, ill try and look into this in the next few days.
Now that the other stuff is out of the way I am going to look into this
I think there's been additional fixes since the original PR. https://github.com/typelevel/sbt-typelevel/blob/5dec904d36c0b2cceb3f18b9ed7323ad849614a4/github-actions/src/main/scala/org/typelevel/sbt/gha/GenerativePlugin.scala#L725
@armanbilge So I had a look and one of the changes which I think you are referring to is this commit https://github.com/typelevel/sbt-typelevel/commit/5b86e388b09753349138ce453933532459fe14c7 . The problem here is that it seems to also bring in Mergify changes (which doesn't exist in sbt-github-actions yet), if I cherry pick that commit I assume its just safe to ignore all of the Mergify changes or would I be missing something?
As a nit on the title, I suggest calling this "Fixes artifact upload for platform cross building" or something since "cross" alone often suggests Scala cross building.
if I cherry pick that commit I assume its just safe to ignore all of the
Mergifychanges or would I be missing something?
that seems fine 👍
that seems fine 👍
@armanbilge So I have just cherry picked https://github.com/typelevel/sbt-typelevel/commit/5b86e388b09753349138ce453933532459fe14c7 without the mergify changes but as you can see in https://github.com/sbt/sbt-github-actions/pull/165/commits/796352371f8ffd6545c87e9b31f67078a284354f#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR137-R186 the resulting ci.yaml is highly suspect, there are duplicate entries for downloading/inflating target directories
Maybe I am missing an earlier commit that fixes some sought of bug or?