Skip to content
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.

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

Make use of reusable workflow #58

Merged
merged 6 commits into from
Apr 14, 2022
Merged

Conversation

mkurz
Copy link
Member

@mkurz mkurz commented Apr 14, 2022

@SethTisue We have some reusable workflows https://github.com/playframework/.github/tree/main/.github/workflows we use for every repo in the Play org. That should lower the maintenance burden.

@ihostage
Copy link
Member

ihostage commented Apr 14, 2022

It's first case with Java 17 😄 It needs analyzing what happened....
https://github.com/playframework/play-webgoat/runs/6026021623?check_suite_focus=true

@mkurz
Copy link
Member Author

mkurz commented Apr 14, 2022

The problem is that the jabba maintainer is not very active anymore, so the index on https://github.com/shyiko/jabba is out of date: https://github.com/shyiko/jabba/blob/master/index.json - there is no adopt 17 and not even temurin yet.
However we are lucky, there is a index that is very well maintained from typelevel we can use: https://github.com/typelevel/jdk-index
We just have to set the JABBA_INDEX env variable.
I will provide a pull request in a minute.

@ihostage
Copy link
Member

Hm.... Coursier index also don't have a Adopt#17 😞
https://github.com/coursier/jvm-index/blob/master/index.json

@mkurz
Copy link
Member Author

mkurz commented Apr 14, 2022

Ah, there is an index as well. I think there we can use "adoptium" which are the temurin builds.

@ihostage
Copy link
Member

https://get-coursier.io/docs/cli-java.html#jvm-index

If needed, that index could be complemented or replaced by an index of our own at some point.

Courier index isn't default, as I understand. It's just an alternative like same the index from Typelevel.

@mkurz
Copy link
Member Author

mkurz commented Apr 14, 2022

Hmm... https://github.com/coursier/jvm-index#use-by-coursier:

The index generated here is now used by the java and java-home commands of coursier.

I will just try, if not I will explicitly set the index.

@ihostage
Copy link
Member

May be it's only for 2.1.0-M* versions 🤔 But we use the 2.0.16.

@ihostage ihostage closed this Apr 14, 2022
@ihostage ihostage reopened this Apr 14, 2022
@mkurz
Copy link
Member Author

mkurz commented Apr 14, 2022

OK, I found the problem.
https://github.com/playframework/play-webgoat/runs/6026468773?check_suite_focus=true#step:6:13

/opt/hostedtoolcache/cs/2.0.16/x64/cs java --jvm adoptium:17 -version

Our workflow uses cs 2.0.16, which still uses the jabba index.
Starting with cs 2.1.0-M1 (https://github.com/coursier/coursier/releases/tag/v2.1.0-M1) they switched to to their own index: coursier/coursier#2205
Also see this commit where they changed the wording from

The index generated here could be used...

to

The index generated here is used...

So it looks like the docs you linked for 2.1.0-M... is out of date (https://get-coursier.io/docs/cli-java.html#jvm-index).

@mkurz
Copy link
Member Author

mkurz commented Apr 14, 2022

Your were 5 seconds faster 😉
So we have to use the index explictly until the action uses cs 2.1.0

@ihostage
Copy link
Member

So we have to use the index explictly until the action uses cs 2.1.0

It's already, but only for pre-release versions.
https://github.com/coursier/setup-action/releases/tag/v1.2.0-M3

We can use a milestone version of action that use a milestone of coursier. But I'm not sure that it's a good idea. 😄

@mkurz
Copy link
Member Author

mkurz commented Apr 14, 2022

I think it should be fine to use the -M3, since cs is "just" used to install java or not? Anyway, I will continue after dinner

@mkurz
Copy link
Member Author

mkurz commented Apr 14, 2022

You can of course continue if you have free time ;)

@ihostage ihostage closed this Apr 14, 2022
@ihostage ihostage reopened this Apr 14, 2022
@mkurz mkurz merged commit 63547f6 into playframework:2.8.x Apr 14, 2022
@mkurz mkurz deleted the update_ci_conf branch April 14, 2022 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants