Skip to content

Allow defining outputs for GHA jobs #845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Allow defining outputs for GHA jobs #845

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a5d6bf7
Allow defining outputs for GHA jobs
rpiaggio Oct 24, 2025
2b37948
Bump base version
rpiaggio Oct 24, 2025
09ea116
add methods to keep bincompat, unbump base version
rpiaggio Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name := "sbt-typelevel"
import org.typelevel.sbt.gha.{PermissionScope, PermissionValue, Permissions}
import com.typesafe.tools.mima.core._

ThisBuild / tlBaseVersion := "0.8"
ThisBuild / tlBaseVersion := "0.9"
Copy link
Member

@bpholt bpholt Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be accomplished without bumping the version here? This is effectively a major version change, with all that entails for libraries. I haven't looked closely at whether it's necessary or not.

Copy link
Author

@rpiaggio rpiaggio Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that first but the Mima check failed without the version bump, so I sent another commit with it.

ThisBuild / crossScalaVersions := Seq("2.12.20")
ThisBuild / developers ++= List(
tlGitHubDev("armanbilge", "Arman Bilge"),
Expand Down
24 changes: 23 additions & 1 deletion github-actions/src/main/scala/org/typelevel/sbt/gha/GenerativePlugin.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,21 @@ object GenerativePlugin extends AutoPlugin {
${indent(rendered.mkString("\n"), 1)}"""
}

def compileOutputs(outputs: Map[String, String]): String =
if (outputs.isEmpty) {
""
} else {
val rendered = outputs map {
case (key, value) =>
if (!isSafeString(key) || key.indexOf(' ') >= 0 || key.indexOf('\n') >= 0)
sys.error(s"'$key' is not a valid output name")

s"""$key: $${{ $value }}"""
}
s"""outputs:
${indent(rendered.mkString("\n"), 1)}"""
}

def compilePermissionScope(permissionScope: PermissionScope): String = permissionScope match {
case PermissionScope.Actions => "actions"
case PermissionScope.Checks => "checks"
Expand Down Expand Up @@ -454,6 +469,13 @@ ${indent(rendered.mkString("\n"), 1)}"""
else
"\n" + renderedPermPre

val renderedOutputsPre = compileOutputs(job.outputs)
val renderedOutputs =
if (renderedOutputsPre.isEmpty)
""
else
"\n" + renderedOutputsPre

val renderedTimeoutMinutes =
job.timeoutMinutes.map(timeout => s"\ntimeout-minutes: $timeout").getOrElse("")

Expand Down Expand Up @@ -541,7 +563,7 @@ ${indent(rendered.mkString("\n"), 1)}"""
strategy:${renderedFailFast}
matrix:
${buildMatrix(2, "os" -> job.oses, "scala" -> job.scalas, "java" -> job.javas.map(_.render))}${renderedMatrices}
runs-on: ${runsOn}${renderedEnvironment}${renderedContainer}${renderedPerm}${renderedEnv}${renderedConcurrency}${renderedTimeoutMinutes}
runs-on: ${runsOn}${renderedEnvironment}${renderedContainer}${renderedPerm}${renderedEnv}${renderedConcurrency}${renderedOutputs}${renderedTimeoutMinutes}
steps:
${indent(job.steps.map(compileStep(_, sbt, job.sbtStepPreamble, declareShell = declareShell)).mkString("\n\n"), 1)}"""
// format: on
Expand Down
6 changes: 6 additions & 0 deletions github-actions/src/main/scala/org/typelevel/sbt/gha/WorkflowJob.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ sealed abstract class WorkflowJob {
def container: Option[JobContainer]
def environment: Option[JobEnvironment]
def concurrency: Option[Concurrency]
def outputs: Map[String, String]
def timeoutMinutes: Option[Int]

def withId(id: String): WorkflowJob
Expand All @@ -57,6 +58,7 @@ sealed abstract class WorkflowJob {
def withContainer(container: Option[JobContainer]): WorkflowJob
def withEnvironment(environment: Option[JobEnvironment]): WorkflowJob
def withConcurrency(concurrency: Option[Concurrency]): WorkflowJob
def withOutputs(outputs: Map[String, String]): WorkflowJob
def withTimeoutMinutes(timeoutMinutes: Option[Int]): WorkflowJob

def updatedEnv(name: String, value: String): WorkflowJob
Expand Down Expand Up @@ -86,6 +88,7 @@ object WorkflowJob {
container: Option[JobContainer] = None,
environment: Option[JobEnvironment] = None,
concurrency: Option[Concurrency] = None,
outputs: Map[String, String] = Map(),
Copy link
Member

@armanbilge armanbilge Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the binary-breaking change. To restore binary-compatibility, we need to keep an overload of apply with the old signature, that can delegate to the new one.

It's also a source-breaking change: outputs should be added as the final parameter in the list.

Copy link
Author

@rpiaggio rpiaggio Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved output to the last place, but it's not possible to define overloads with default arguments. What do you recommend?

Copy link
Member

@armanbilge armanbilge Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not possible to define overloads with default arguments

That's okay: just define the overload with the old signature without default arguments.

Copy link
Author

@rpiaggio rpiaggio Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bincompat required a number of other changes.

timeoutMinutes: Option[Int] = None): WorkflowJob =
Impl(
id,
Expand All @@ -107,6 +110,7 @@ object WorkflowJob {
container,
environment,
concurrency,
outputs,
timeoutMinutes
)

Expand All @@ -130,6 +134,7 @@ object WorkflowJob {
container: Option[JobContainer],
environment: Option[JobEnvironment],
concurrency: Option[Concurrency],
outputs: Map[String, String],
timeoutMinutes: Option[Int])
extends WorkflowJob {

Expand All @@ -153,6 +158,7 @@ object WorkflowJob {
override def withContainer(container: Option[JobContainer]): WorkflowJob = copy(container = container)
override def withEnvironment(environment: Option[JobEnvironment]): WorkflowJob = copy(environment = environment)
override def withConcurrency(concurrency: Option[Concurrency]): WorkflowJob = copy(concurrency = concurrency)
override def withOutputs(outputs: Map[String, String]): WorkflowJob = copy(outputs = outputs)
override def withTimeoutMinutes(timeoutMinutes: Option[Int]): WorkflowJob = copy(timeoutMinutes = timeoutMinutes)

def updatedEnv(name: String, value: String): WorkflowJob = copy(env = env.updated(name, value))
Expand Down
Loading