Publish snapshots by default and add an override for hash releases #86

vasilmkd · 2021-12-13T08:27:47Z

@armanbilge and I worked on this after discussing SNAPSHOT releases with other maintainers on Discord. We think this is generally desired by people.

By combining both the hash and the -SNAPSHOT suffix, we believe we can achieve a sensible middle ground where snapshots are mutable but not actually mutated (unless the user goes out of their way to make sure the hash matches a previous version), so this helps with the stability, maven central is not polluted (this doesn't at all affect tagged releases and they can still be done manually) and maintainers have a steady stream of stable snapshots to test stuff and look for regressions.

Co-authored-by: Arman Bilge <[email protected]>

vasilmkd · 2021-12-13T11:47:28Z

The question remains, can this work out of the box with the CI release mode of sbt-spiewak, because that accesses PGP_SECRET and what not, and this is not strictly required for SNAPSHOT releases.

armanbilge · 2021-12-13T13:45:19Z

That's a very good point. I forgot Some People™ don't use CI publishing for the real releases, and therefore probably won't have the PGP_SECRET set. You are right, it will fail on this workflow step:

      - name: Import signing key
        run: echo $PGP_SECRET | base64 -d | gpg --import

https://github.com/typelevel/fs2/blob/144eccc216e5195a7f3c988080dee28a2a6b9db2/.github/workflows/ci.yml#L145

I wonder if we could make that step conditional, but I don't know on what. In fact, I suspect that solving this problem is nearly equivalent to solving the current problem where you tag a release with snapshots enabled it will try to publish the release twice. Essentially, one of those CI runs is for the snapshot, and if it could somehow know that it could avoid both these issues.

vasilmkd · 2021-12-13T13:48:54Z

Conditional on the new override? If it's false (the default, so SNAPSHOTs are released), don't execute that command and vice versa.

Essentially, one of those CI runs is for the snapshot, and if it could somehow know that it could avoid both these issues.
Is this not a problem as things stand even without this PR?

armanbilge · 2021-12-13T13:53:01Z

Yes, this is a general problem, which is a CI run doesn't know whether it's for an ordinary push or for a tag. Actually here's one way to solve it.

on:
  pull_request:
    branches: ['**']
  push:
    branches: ['**']
    tags: [v*]

These are the triggers for the workflow. So we could actually just duplicate the workflow, have one run on branch push, and the other one run on tag, and then it becomes possible to modify it for each case. It's really annoying to have duplicate workflows, but at least it's auto-generated.

armanbilge · 2021-12-13T13:54:03Z

Conditional on the new override? If it's false (the default, so SNAPSHOTs are released), don't execute that command and vice versa.

If we generate a separate workflow for on branch push and on tag push, the branch push workflow can remove the PGP step if that setting is false.

armanbilge · 2021-12-13T14:09:38Z

Aha, here's something much easier than that.

GITHUB_REF_TYPE: The type of ref that triggered the workflow run. Valid values are branch or tag.

https://docs.github.com/en/actions/learn-github-actions/environment-variables

vasilmkd · 2021-12-13T14:24:29Z

On tag, run the tag release, on branch run a snapshot?

vasilmkd · 2021-12-13T14:28:44Z

This still doesn't cover the hash releases as snapshots. Or am I missing something?

armanbilge · 2021-12-13T14:34:50Z

Assuming that the user wants -SNAPSHOT releases for their branch pushes, but NO ci release for their tag pushes, we just need to add a conditional:

  publish:
    name: Publish Artifacts
    needs: [build]                               # vvvvvvvvvvvvv
    if: github.event_name != 'pull_request' && env.GITHUB_REF_TYPE == 'branch' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main' || github.ref == 'refs/heads/series/2.5.x')

That way it skips the publish step for tag pushes. We'd do the same thing for the PGP step:

      - name: Import signing key
        if: env.GITHUB_REF_TYPE == 'tag'
        run: echo $PGP_SECRET | base64 -d | gpg --import

vasilmkd · 2021-12-13T14:38:05Z

I guess that works. Btw, env.GITHUB_REF_TYPE is the same as github.ref_type (same for all other env variables, e.g. github.event_name == env.GITHUB_EVENT_NAME).

vasilmkd · 2021-12-13T14:39:10Z

Assuming that the user wants -SNAPSHOT releases for their branch pushes, but NO ci release for their tag pushes, we just need to add a conditional

I just realized, if they want tag or hash releases, they need to set up the PGP_SECRET, there is no way around it. So I think this should work.

vasilmkd · 2021-12-13T14:40:28Z

Ah but not really?

      - name: Import signing key
        if: env.GITHUB_REF_TYPE == 'tag'
        run: echo $PGP_SECRET | base64 -d | gpg --import

This wouldn't run for hash releases.

armanbilge · 2021-12-13T14:41:37Z

Right, so we only add that conditional to the generated YAML if the user has indicated they want -SNAPSHOT releases. If they want stable hash releases, we don't add that.

vasilmkd · 2021-12-13T14:42:14Z

Which means that sbt-github-actions needs to know about the setting...

armanbilge · 2021-12-13T14:43:19Z

Don't think so, it's needed here:

sbt-spiewak/sonatype/src/main/scala/sbtspiewak/SonatypeCiReleasePlugin.scala

Lines 57 to 60 in d689a5b

    
           githubWorkflowPublishPreamble += 
        
             WorkflowStep.Run( 
        
               List("echo $PGP_SECRET | base64 -d | gpg --import"), 
        
               name = Some("Import signing key")

armanbilge · 2021-12-13T14:49:22Z

So just for my own clarification, we have three release modes:

-SNAPSHOT "snapshot" releases
stable "hash" releases
tag "v" releases

With these, there are 6 configurations for CI?

branch push	tag push	example user
do nothing	do nothing
snapshot	do nothing	Cats Effect
hash	do nothing
do nothing	v
snapshot	v	fs2
hash	v

vasilmkd · 2021-12-13T14:57:56Z

Assuming that the user wants -SNAPSHOT releases for their branch pushes, but NO ci release for their tag pushes, we just need to add a conditional:

  publish:
    name: Publish Artifacts
    needs: [build]                               # vvvvvvvvvvvvv
    if: github.event_name != 'pull_request' && env.GITHUB_REF_TYPE == 'branch' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main' || github.ref == 'refs/heads/series/2.5.x')

That way it skips the publish step for tag pushes. We'd do the same thing for the PGP step:

      - name: Import signing key
        if: env.GITHUB_REF_TYPE == 'tag'
        run: echo $PGP_SECRET | base64 -d | gpg --import

This seems a bit complicated. Would a separate workflow for snapshots and a separate workflow for releases be simpler?

Especially this part, which I believe right now comes from sbt-github-actions?

needs: [build]                               # vvvvvvvvvvvvv
    if: github.event_name != 'pull_request' && env.GITHUB_REF_TYPE == 'branch' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main' || github.ref == 'refs/heads/series/2.5.x')

armanbilge · 2021-12-13T15:01:41Z

IDK, we might have to shave this yak first:

Support adding entire Workflows sbt/sbt-github-actions#7

Ooph you are right, this might be an sbt-gh-actions thing 😓

vasilmkd · 2021-12-13T15:06:00Z

I mean, as a stopgap, the release step can be behind a condition, and that one is here.

armanbilge · 2021-12-13T15:07:00Z

It'll be a bit confusing in the UI to see an empty publish job succeed, but otherwise a good idea!

vasilmkd · 2021-12-13T15:07:53Z

It'll be a bit confusing in the UI to see an empty publish job succeed, but otherwise a good idea!

This is how sbt-github-actions works out of the box.

https://github.com/djspiewak/sbt-github-actions/actions/runs/1531278166

armanbilge · 2021-12-13T15:09:57Z

Oh, interesting, I didn't know that.

vasilmkd · 2021-12-13T15:11:18Z

How does releasing snapshots without releasing tagged releases work currently? What's the discriminator?

armanbilge · 2021-12-13T15:12:30Z

Actually, I don't think that capability exists as of now. You buy into CI release first, and then optionally into hash releases.

vasilmkd · 2021-12-13T16:13:05Z

@armanbilge Can you maybe take this for a spin? I will release a SNAPSHOT. 😉

armanbilge · 2021-12-13T18:08:02Z

    val gpgWarnOnFailure = settingKey[Boolean](
      "If true, only issue a warning when signing fails. If false, error " +
        "and fail the build. Defaults to true in publishLocal, false in publish.")

We should set this to true when the version is a -SNAPSHOT.
https://github.com/jodersky/sbt-gpg/blob/efcba49fc9d8e07d84f05b5d48386f72bbb81e1a/src/main/scala/SbtGpg.scala#L14

vasilmkd · 2021-12-13T18:08:44Z

Ok cool. Good find.

vasilmkd · 2021-12-13T18:58:30Z

@armanbilge Can you try the following snapshot please? 0.23-11-4c9ab83-SNAPSHOT. Thanks.

vasilmkd · 2021-12-13T19:10:06Z

Do you mind also testing a tagged release?

armanbilge · 2021-12-13T19:11:39Z

The snapshot worked! https://s01.oss.sonatype.org/content/repositories/snapshots/com/armanbilge/schrodinger_3/0.3-2-3fc5aaf-SNAPSHOT/

Time for 0.3.0-M2 😆

armanbilge · 2021-12-13T19:20:36Z

0.3.0-M2 is on it's way to maven 🚀 nice work!!!

vasilmkd · 2021-12-13T20:42:07Z

I can publish a snapshot to get things rolling?

armanbilge · 2021-12-13T20:43:09Z

What do you have in mind? 😁

vasilmkd · 2021-12-13T20:51:32Z

I found another issue, with publishHash and then sonatypeBundleRelease.

vasilmkd · 2021-12-13T20:52:36Z

  private def sonatypeBundleReleaseIfRelevant: Command =
    Command.command("sonatypeBundleReleaseIfRelevant") { state =>
      val isSnap = state.getSetting(isSnapshot).getOrElse(false)
      if (!isSnap)
        Command.process("sonatypeBundleRelease", state)
      else
        state
    }

vasilmkd · 2021-12-13T20:53:03Z

isSnapshot is now almost always true.

armanbilge · 2021-12-13T21:02:38Z

Sorry, I couldn't understand what the problem is?

vasilmkd · 2021-12-13T21:03:41Z

From sbt-sonatype:

    sonatypeBundleDirectory := {
      (ThisBuild / baseDirectory).value / "target" / "sonatype-staging" / s"${(ThisBuild / version).value}"
    },

and the error is the following:

java.io.IOException: Supplied file /Users/vasil/Code/sbt-spiewak/target/sonatype-staging/0.23-11-4c9ab83-SNAPSHOT is a not an existing directory!
        at org.sonatype.spice.zapper.fs.AbstractDirectory.<init>(AbstractDirectory.java:32)
        at org.sonatype.spice.zapper.fs.DirectoryIOSource.<init>(DirectoryIOSource.java:68)
        at org.sonatype.spice.zapper.fs.DirectoryIOSource.<init>(DirectoryIOSource.java:59)
        at org.sonatype.spice.zapper.fs.DirectoryIOSource.<init>(DirectoryIOSource.java:50)
        at xerial.sbt.sonatype.SonatypeClient.$anonfun$uploadBundle$2(SonatypeClient.scala:284)
        at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
        at scala.util.Try$.apply(Try.scala:213)
        at wvlet.airframe.control.Retry$RetryContext.runInternal(Retry.scala:268)
        at wvlet.airframe.control.Retry$RetryContext.run(Retry.scala:253)
        at xerial.sbt.sonatype.SonatypeClient.uploadBundle(SonatypeClient.scala:269)
        at xerial.sbt.sonatype.SonatypeService.uploadBundle(SonatypeService.scala:73)
        at xerial.sbt.Sonatype$.$anonfun$sonatypeBundleRelease$2(Sonatype.scala:181)
        at xerial.sbt.Sonatype$.withSonatypeService(Sonatype.scala:437)

version ends with a -SNAPSHOT, and the hash release doesn't.

armanbilge · 2021-12-13T21:05:20Z

Right, it seems like publishHash should drop -SNAPSHOT from the version number and proceed from there? IDK how easy that is...

vasilmkd · 2021-12-13T21:06:10Z

publishHash does drop it already. The problem is, this is not then communicated to sonatypeBundleRelease.

armanbilge · 2021-12-13T21:06:44Z

Ooh 😮

vasilmkd · 2021-12-13T21:07:01Z

The problem is that publish and releases "should be" separate steps.

vasilmkd · 2021-12-13T21:19:55Z

Here's a hash release:

addSbtPlugin("io.vasilev" % "sbt-spiewak-sonatype" % "0.23-12-4067672")

No snapshot resolver necessary.

vasilmkd · 2021-12-13T21:21:49Z

@armanbilge Can you try releasing a hash with this version too? The steps are sbt publishHasIfRelevant sonatypeBundleReleaseIfRelevant.

armanbilge · 2021-12-13T21:22:27Z

I've actually never released anything locally 😆 I'm not sure I know how.

vasilmkd · 2021-12-13T21:23:21Z

You need to have gpg set up. Don't bother if you haven't yet. I released the hash using the plugin, so it works.

armanbilge · 2021-12-13T21:24:14Z

What I can do easily, is toggle the setting so CI can release a stable hash instead. Would that be helpful?

vasilmkd · 2021-12-13T21:25:02Z

Actually yes, that's a good scenario to try. Thanks.

vasilmkd · 2021-12-13T21:26:07Z

What we should discuss again though is, should we flip everything, keep the old sbt-spiewak behavior exactly as it is, and instead add a publishSnapshot/releaseSnapshot that only releases a snapshot. We can still tweak the GHA yaml to cover that case.

armanbilge · 2021-12-13T21:27:27Z

I think that's a Daniel vs the world question 😅 IDK if anybody likes the current default.

armanbilge · 2021-12-13T23:41:43Z

What I can do easily, is toggle the setting so CI can release a stable hash instead. Would that be helpful?

Forgot to followup, this worked for me 👍

vasilmkd and others added 4 commits December 13, 2021 00:24

Add a sonatypeBundleReleaseIfRelevant command

b053e2f

Simplify the implementation

6b33873

Publish snapshots by default and add an override for hash releases

aa8e407

Co-authored-by: Arman Bilge <[email protected]>

Use the override in tests

7348dac

vasilmkd marked this pull request as draft December 13, 2021 15:16

vasilmkd added 2 commits December 13, 2021 19:12

Add a setting for toggling tagged releases

81d419c

Suppress errors when signing snapshots without a key

837f91e

vasilmkd marked this pull request as ready for review December 13, 2021 19:09

Augment sonatypeBundleReleaseIfRelevant for hash releases

79c9d67

Publish snapshots by default and add an override for hash releases #86

Are you sure you want to change the base?

Publish snapshots by default and add an override for hash releases #86

Conversation

vasilmkd commented Dec 13, 2021 • edited Loading

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

armanbilge commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021 • edited Loading

vasilmkd commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021 • edited Loading

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021 • edited Loading

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021 • edited Loading

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

vasilmkd commented Dec 13, 2021

armanbilge commented Dec 13, 2021

armanbilge commented Dec 13, 2021

vasilmkd commented Dec 13, 2021 •

edited

Loading

armanbilge commented Dec 13, 2021 •

edited

Loading

vasilmkd commented Dec 13, 2021 •

edited

Loading

vasilmkd commented Dec 13, 2021 •

edited

Loading

vasilmkd commented Dec 13, 2021 •

edited

Loading