Strumenta / antlr-kotlin

Support for Kotlin as a target for ANTLR 4
Apache License 2.0
222 stars 47 forks source link

Configuring GitHub actions #111

Closed ftomassetti closed 9 months ago

ftomassetti commented 9 months ago

The GitHub actions should now run for every commit and every PR. We just perform some basic tasks, like compiling for the JVM and running tests on the JVM. Later one we could enable more tasks.

Fix #108

ftomassetti commented 9 months ago

@lppedd would this work for you?

ftomassetti commented 9 months ago

I changed this to have two separate actions: one for building and one for running tests

Screenshot 2023-12-06 at 17 42 47

Each of them runs both on java 11 and java 17. Java 8 cannot be supported as ANTLR 4.13 requires at least Java 11

lppedd commented 9 months ago

Studying how Actions are set up right now.

@ftomassetti

as ANTLR 4.13 requires at least Java 11

Is this related to the generation process or also to the runtime?
If this impacts the Java runtime, we can also move up the generated bytecode version in our build scripts.

ftomassetti commented 9 months ago

Studying how Actions are set up right now.

@ftomassetti

as ANTLR 4.13 requires at least Java 11

Is this related to the generation process or also to the runtime?

For sure this affects the build process, so I need to run GitHub actions with Java 11. But it is true that we can still setup the build to compile for Java 8. However that would be unrelated from the GitHub actions configuration

frett commented 9 months ago

on my fork of the repo I had setup a GHA workflow for tests and deploying to our JFrog repo. feel free to grab anything you want from that action file, or ask questions about it.

https://github.com/CruGlobal/antlr-kotlin/blob/build/v0.0.7/.github/workflows/publish-fork.yml

lppedd commented 9 months ago

@frett looks pretty neat! I've got a question, what's up with the Cache Konan step?

frett commented 9 months ago

@frett looks pretty neat! I've got a question, what's up with the Cache Konan step?

Kotlin Multiplatform uses a Konan runtime to actually do the Kotlin/Native compilation. That binary was automatically downloaded and setup everytime a build would run. Unfortunately that isn't captured by the gradle-build-action caching, so I set it to manually cache it

frett commented 9 months ago

the publish action on my GHA workflow runs on macos to have access to XCode in order to get the ios targets.

running it on macos does also support jvm & js, but there is potential additional costs for a macos runner vs a normal ubuntu runner. We have a enterprise account so I'm not sure if there are any costs or restrictions in place for an open source project

lppedd commented 9 months ago

Oh I see, makes sense!

So (pardon my inexperience on Actions), splitting up the YAML file, we could have:

on:
  push:
    branches: ['master']
  pull_request:
    branches: ['*']

And:

jobs:
  build_and_test:
    name: Build & Test
    runs-on: ubuntu-latest
    steps:
      - name: Checkout code
        uses: actions/checkout@v3
      - name: Cache Konan
        uses: actions/cache@v3
        with:
          path: ~/.konan
          key: ${{ runner.os }}-konan-${{ github.sha }}
          restore-keys: ${{ runner.os }}-konan-
      - name: Run Build and Tests
        uses: gradle/gradle-build-action@v2
        with:
          arguments: build -Ptarget.is.native=false

Would this work?

frett commented 9 months ago

I would do:

on:
  push:
    branches: ['master']
  pull_request:
    branches: ['master']

the pull_request triggers are based on the branch you are merging into

lppedd commented 9 months ago

Thank you!
We also have a matrix now:

strategy:
  matrix:
    java: [ '11', '17' ]

I guess this would mean two JS builds too. Maybe running on Java 11 only is sufficient?

frett commented 9 months ago

it's probably sufficient, but the GHA workflow is only taking 2-3 mins to run currently, so running it on both toolchains vs 1 wouldn't have a huge impact. plus those 2 jobs would run concurrently.

lppedd commented 9 months ago

Sounds reasonable. So, looking at the overall picture, we are left with:

name: Build

on:
  push:
    branches: ['master']
  pull_request:
    branches: ['master']

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true

jobs:
  build_and_test:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        java: ['11', '17']
    name: Build & Test (Java ${{ matrix.Java }})
    steps:
      - name: Setup Java
        uses: actions/setup-java@v3
        with:
          distribution: 'adopt'
          java-version: ${{ matrix.java }}
      - name: Checkout code
        uses: actions/checkout@v3
      - name: Cache Konan
        uses: actions/cache@v3
        with:
          path: ~/.konan
          key: ${{ runner.os }}-konan-${{ github.sha }}
          restore-keys: ${{ runner.os }}-konan-
      - name: Run Build and Tests
        uses: gradle/gradle-build-action@v2
        with:
          arguments: build -Ptarget.is.native=false

I'm starting to understand how all of this work finally.

frett commented 9 months ago

yeah, although you missed the setup-java step :)

lppedd commented 9 months ago

Damn, thanks! Added back in the comment.

frett commented 9 months ago

that looks good and should work as a starting point, the workflow can evolve over time as needs arise :)

lppedd commented 9 months ago

@frett so had a bit of docs reading and came up with:

Edit: updated with a working script

name: Build

on:
  push:
    branches: [ 'master' ]
  pull_request:
    branches: [ 'master' ]

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true

jobs:
  build:
    strategy:
      matrix:
        include:
          - java: '11'
            target: JVM
            task: jvmTest
          - java: '17'
            target: JVM
            task: jvmTest
          - java: '17'
            target: JS
            task: jsTest
    runs-on: ubuntu-latest
    name: Build (${{ matrix.java }} / ${{ matrix.target }})
    steps:
      - name: Checkout code
        uses: actions/checkout@v3

      - name: Setup Java
        uses: actions/setup-java@v3
        with:
          distribution: 'adopt'
          java-version: ${{ matrix.java }}

      - name: Cache Konan
        uses: actions/cache@v3
        with:
          path: ~/.konan
          key: ${{ runner.os }}-konan-${{ github.sha }}
          restore-keys: ${{ runner.os }}-konan-

      - name: Setup Gradle
        uses: gradle/gradle-build-action@v2

      - name: Run Build and Tests (${{ matrix.target }})
        run: ./gradlew ${{ matrix.task }}
        working-directory: ${{ github.workspace }}
        continue-on-error: false

This allows you to execute three jobs:

- JDK 11 + JVM target
- JDK 17 + JVM target
- JDK 17 + JS target
frett commented 9 months ago

I'm not sure without giving it a try

lppedd commented 9 months ago

I've actually found out we can just do the following to define the matrix runs:

matrix:
  include:
    - java: '11'
      target: JVM
      task: jvmTest
    - java: '17'
      target: JVM
      task: jvmTest
    - java: '17'
      target: JS
      task: jsTest

Easier and probably more understandable.

ftomassetti commented 9 months ago

thank you @frett and @lppedd , I learned something very useful thanks to you. I just applied your suggestion as-is, and that works great. Let's get it merged

lppedd commented 9 months ago

Thanks! Another step towards easier maintenance is done 😎