djspiewak / sbt-spiewak

A plugin which represents my personal SBT project baseline
Apache License 2.0
53 stars 20 forks source link

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

Open vasilmkd opened 2 years ago

vasilmkd commented 2 years ago

@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.

vasilmkd commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

vasilmkd commented 2 years ago

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

armanbilge commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

armanbilge commented 2 years ago

Don't think so, it's needed here: https://github.com/djspiewak/sbt-spiewak/blob/d689a5be2f3dba2c335b2be072870287fda701b8/sonatype/src/main/scala/sbtspiewak/SonatypeCiReleasePlugin.scala#L57-L60

armanbilge commented 2 years ago

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

  1. -SNAPSHOT "snapshot" releases
  2. stable "hash" releases
  3. 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 commented 2 years ago

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 commented 2 years ago

IDK, we might have to shave this yak first:

Ooph you are right, this might be an sbt-gh-actions thing πŸ˜“

vasilmkd commented 2 years ago

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

armanbilge commented 2 years ago

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

vasilmkd commented 2 years ago

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 commented 2 years ago

Oh, interesting, I didn't know that.

vasilmkd commented 2 years ago

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

armanbilge commented 2 years ago

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

vasilmkd commented 2 years ago

@armanbilge Can you maybe take this for a spin? I will release a SNAPSHOT. πŸ˜‰

vasilmkd commented 2 years ago
addSbtPlugin("io.vasilev" % "sbt-spiewak-sonatype" % "0.23-11-6c79539-SNAPSHOT")

and you need the snapshots resolver for the new s01.oss.sonatype.org host, fyi.

resolvers +=
  "Sonatype OSS Snapshots" at "https://s01.oss.sonatype.org/content/repositories/snapshots"
vasilmkd commented 2 years ago

The fs2 case (and the default):

Settings:

enablePlugins(SonatypeCiReleasePlugin)

ThisBuild / spiewakMainBranches := Seq("main")
ThisBuild / spiewakCiReleaseSnapshots := true
ThisBuild / spiewakCiReleaseTags := true

Output:

  publish:
    name: Publish Artifacts
    needs: [build]
    if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
    strategy:
      matrix:
        os: [ubuntu-latest]
        scala: [3.0.2]
        java: [temurin@8]
    runs-on: ${{ matrix.os }}
    steps:
      - name: Checkout current branch (full)
        uses: actions/checkout@v2
        with:
          fetch-depth: 0

      - name: Setup Java (temurin@8)
        if: matrix.java == 'temurin@8'
        uses: actions/setup-java@v2
        with:
          distribution: temurin
          java-version: 8

      - name: Cache sbt
        uses: actions/cache@v2
        with:
          path: |
            ~/.sbt
            ~/.ivy2/cache
            ~/.coursier/cache/v1
            ~/.cache/coursier/v1
            ~/AppData/Local/Coursier/Cache/v1
            ~/Library/Caches/Coursier/v1
          key: ${{ runner.os }}-sbt-cache-v2-${{ hashFiles('**/*.sbt') }}-${{ hashFiles('project/build.properties') }}

      - name: Download target directories (2.12.15)
        uses: actions/download-artifact@v2
        with:
          name: target-${{ matrix.os }}-2.12.15-${{ matrix.java }}

      - name: Inflate target directories (2.12.15)
        run: |
          tar xf targets.tar
          rm targets.tar

      - name: Download target directories (2.13.7)
        uses: actions/download-artifact@v2
        with:
          name: target-${{ matrix.os }}-2.13.7-${{ matrix.java }}

      - name: Inflate target directories (2.13.7)
        run: |
          tar xf targets.tar
          rm targets.tar

      - name: Download target directories (3.0.2)
        uses: actions/download-artifact@v2
        with:
          name: target-${{ matrix.os }}-3.0.2-${{ matrix.java }}

      - name: Inflate target directories (3.0.2)
        run: |
          tar xf targets.tar
          rm targets.tar

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

      - run: sbt ++${{ matrix.scala }} release
vasilmkd commented 2 years ago

Settings:

enablePlugins(SonatypeCiReleasePlugin)

ThisBuild / spiewakMainBranches := Seq("main")
ThisBuild / spiewakCiReleaseSnapshots := true
ThisBuild / publishSnapshotsAsHashReleases := true
ThisBuild / spiewakCiReleaseTags := true

Output:

  publish:
    name: Publish Artifacts
    needs: [build]
    if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
    strategy:
      matrix:
        os: [ubuntu-latest]
        scala: [3.0.2]
        java: [temurin@8]
    runs-on: ${{ matrix.os }}
    steps:
      - name: Checkout current branch (full)
        uses: actions/checkout@v2
        with:
          fetch-depth: 0

      - name: Setup Java (temurin@8)
        if: matrix.java == 'temurin@8'
        uses: actions/setup-java@v2
        with:
          distribution: temurin
          java-version: 8

      - name: Cache sbt
        uses: actions/cache@v2
        with:
          path: |
            ~/.sbt
            ~/.ivy2/cache
            ~/.coursier/cache/v1
            ~/.cache/coursier/v1
            ~/AppData/Local/Coursier/Cache/v1
            ~/Library/Caches/Coursier/v1
          key: ${{ runner.os }}-sbt-cache-v2-${{ hashFiles('**/*.sbt') }}-${{ hashFiles('project/build.properties') }}

      - name: Download target directories (2.12.15)
        uses: actions/download-artifact@v2
        with:
          name: target-${{ matrix.os }}-2.12.15-${{ matrix.java }}

      - name: Inflate target directories (2.12.15)
        run: |
          tar xf targets.tar
          rm targets.tar

      - name: Download target directories (2.13.7)
        uses: actions/download-artifact@v2
        with:
          name: target-${{ matrix.os }}-2.13.7-${{ matrix.java }}

      - name: Inflate target directories (2.13.7)
        run: |
          tar xf targets.tar
          rm targets.tar

      - name: Download target directories (3.0.2)
        uses: actions/download-artifact@v2
        with:
          name: target-${{ matrix.os }}-3.0.2-${{ matrix.java }}

      - name: Inflate target directories (3.0.2)
        run: |
          tar xf targets.tar
          rm targets.tar

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

      - run: sbt ++${{ matrix.scala }} release
vasilmkd commented 2 years ago

Settings:

enablePlugins(SonatypeCiReleasePlugin)

ThisBuild / spiewakMainBranches := Seq("main")
ThisBuild / spiewakCiReleaseTags := true

Output:

  publish:
    name: Publish Artifacts
    needs: [build]
    if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v'))
    strategy:
      matrix:
        os: [ubuntu-latest]
        scala: [3.0.2]
        java: [temurin@8]
    runs-on: ${{ matrix.os }}
    steps:
      - name: Checkout current branch (full)
        uses: actions/checkout@v2
        with:
          fetch-depth: 0

      - name: Setup Java (temurin@8)
        if: matrix.java == 'temurin@8'
        uses: actions/setup-java@v2
        with:
          distribution: temurin
          java-version: 8

      - name: Cache sbt
        uses: actions/cache@v2
        with:
          path: |
            ~/.sbt
            ~/.ivy2/cache
            ~/.coursier/cache/v1
            ~/.cache/coursier/v1
            ~/AppData/Local/Coursier/Cache/v1
            ~/Library/Caches/Coursier/v1
          key: ${{ runner.os }}-sbt-cache-v2-${{ hashFiles('**/*.sbt') }}-${{ hashFiles('project/build.properties') }}

      - name: Download target directories (2.12.15)
        uses: actions/download-artifact@v2
        with:
          name: target-${{ matrix.os }}-2.12.15-${{ matrix.java }}

      - name: Inflate target directories (2.12.15)
        run: |
          tar xf targets.tar
          rm targets.tar

      - name: Download target directories (2.13.7)
        uses: actions/download-artifact@v2
        with:
          name: target-${{ matrix.os }}-2.13.7-${{ matrix.java }}

      - name: Inflate target directories (2.13.7)
        run: |
          tar xf targets.tar
          rm targets.tar

      - name: Download target directories (3.0.2)
        uses: actions/download-artifact@v2
        with:
          name: target-${{ matrix.os }}-3.0.2-${{ matrix.java }}

      - name: Inflate target directories (3.0.2)
        run: |
          tar xf targets.tar
          rm targets.tar

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

      - run: sbt ++${{ matrix.scala }} release
armanbilge commented 2 years ago

@vasilmkd cool, thanks! I think I'll guinea-pig https://github.com/armanbilge/schrodinger

vasilmkd commented 2 years ago

The Cats Effect case:

Settings:

enablePlugins(SonatypeCiReleasePlugin)

ThisBuild / spiewakMainBranches := Seq("main")
ThisBuild / spiewakCiReleaseSnapshots := true
ThisBuild / spiewakCiReleaseTags := false

Output:

  publish:
    name: Publish Artifacts
    needs: [build]
    if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
    strategy:
      matrix:
        os: [ubuntu-latest]
        scala: [3.0.2]
        java: [temurin@8]
    runs-on: ${{ matrix.os }}
    steps:
      - name: Checkout current branch (full)
        uses: actions/checkout@v2
        with:
          fetch-depth: 0

      - name: Setup Java (temurin@8)
        if: matrix.java == 'temurin@8'
        uses: actions/setup-java@v2
        with:
          distribution: temurin
          java-version: 8

      - name: Cache sbt
        uses: actions/cache@v2
        with:
          path: |
            ~/.sbt
            ~/.ivy2/cache
            ~/.coursier/cache/v1
            ~/.cache/coursier/v1
            ~/AppData/Local/Coursier/Cache/v1
            ~/Library/Caches/Coursier/v1
          key: ${{ runner.os }}-sbt-cache-v2-${{ hashFiles('**/*.sbt') }}-${{ hashFiles('project/build.properties') }}

      - name: Download target directories (2.12.15)
        uses: actions/download-artifact@v2
        with:
          name: target-${{ matrix.os }}-2.12.15-${{ matrix.java }}

      - name: Inflate target directories (2.12.15)
        run: |
          tar xf targets.tar
          rm targets.tar

      - name: Download target directories (2.13.7)
        uses: actions/download-artifact@v2
        with:
          name: target-${{ matrix.os }}-2.13.7-${{ matrix.java }}

      - name: Inflate target directories (2.13.7)
        run: |
          tar xf targets.tar
          rm targets.tar

      - name: Download target directories (3.0.2)
        uses: actions/download-artifact@v2
        with:
          name: target-${{ matrix.os }}-3.0.2-${{ matrix.java }}

      - name: Inflate target directories (3.0.2)
        run: |
          tar xf targets.tar
          rm targets.tar

      - run: sbt ++${{ matrix.scala }} release
armanbilge commented 2 years ago

@vasilmkd v0.3.0-M1 worked but the snapshot release failed b/c of gpg stuff. Did I configure it wrong? https://github.com/armanbilge/schrodinger/runs/4509430648

armanbilge commented 2 years ago

Lol nooo crappp I did the stupid merge and tag thing at the same time gahh πŸ˜΅β€πŸ’«

armanbilge commented 2 years ago

No, still broken 😒 https://github.com/armanbilge/schrodinger/runs/4509721211

vasilmkd commented 2 years ago

It seems that sbt spiewak might also be signing snapshots.

vasilmkd commented 2 years ago

@armanbilge I'm pretty sure it comes from the sbt-gpg dependency which says that all artifacts will be signed.

https://github.com/jodersky/sbt-gpg

vasilmkd commented 2 years ago

So I guess I should just remove the last 2 commits I created.

armanbilge commented 2 years ago

publishLocal falls back to unsigned artifacts in case key material cannot be found, after emitting an explicit warning. publish will fail the build by default if signing fails to avoid accidentally publishing unsigned artifacts, though you can override this with a setting.

Wait, what is this setting?

armanbilge commented 2 years ago
    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 commented 2 years ago

Ok cool. Good find.

vasilmkd commented 2 years ago

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

vasilmkd commented 2 years ago

Do you mind also testing a tagged release?

armanbilge commented 2 years ago

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 commented 2 years ago

0.3.0-M2 is on it's way to maven πŸš€ nice work!!!

vasilmkd commented 2 years ago

I can publish a snapshot to get things rolling?

armanbilge commented 2 years ago

What do you have in mind? 😁

vasilmkd commented 2 years ago

I found another issue, with publishHash and then sonatypeBundleRelease.

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

isSnapshot is now almost always true.

armanbilge commented 2 years ago

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