fix: Java version error; Add missing #! line

LukeSavefrogs / setup-jython

Set up your GitHub Actions workflow with a specific Jython version

https://www.jython.org/

MIT License

1 stars 1 forks source link

fix: Java version error; Add missing #! line #10

Closed ZZy979 closed 6 months ago

ZZy979 commented 6 months ago

This PR solves the following problems:

Upgrade Java version from 8 to 11, since GitHub runner macos-latest no longer supports Java 8 (see actions/setup-java#625).

Add missing #! line in the file ~/.local/bin/jython, which may cause an error when running jython by Python subprocess module.

Please review the changes. Thank you!

ZZy979 commented 6 months ago

And this action seems not to work on Windows. When I just run "jython --version" using pwsh, I got the following error messages:

I printed the PATH env variable, and the added path seems incorrect.

How can I fix it?

LukeSavefrogs commented 6 months ago

And this action seems not to work on Windows. When I just run "jython --version" using pwsh, I got the following error messages:

This is a known problem and is tracked by #8.

Feel free to investigate the problem 😄

LukeSavefrogs commented 6 months ago

Thank you for your submission @ZZy979!

I will review and eventually merge this PR as soon as I have spare time. Did you test that upgrading the Java version does not break compatibility?

I would like not to since almost all companies I know that use Jython also are stuck to Java 8. I am keen to providing a way to specify Java version (defaulting to 8)...

Do ytou know if it is possible to install legacy Jav 8 any other way?

ZZy979 commented 6 months ago

This is a known problem and is tracked by #8.

Feel free to investigate the problem 😄

According to GitHub docs, different syntax should be used to add a system path on Windows PowerShell. I will try it later.

Did you test that upgrading the Java version does not break compatibility?

All CI workflows have succeeded (except for version 2.0 and 2.1, but I noticed they also failed in your last commit). As far as I know, only macos-latest doesn't support Java 8. Perhaps it's better to allow specifying Java version.

ZZy979 commented 6 months ago

I've updated the changes by switching to the zulu distribution, and tests have succeeded in macos-latest. Please take some time to have a look!

As for the failing tests for Jython 2.0 and 2.1, I suggest you drop support for that old versions. Do you really need to keep the compatibility?

LukeSavefrogs commented 6 months ago

I've updated the changes by switching to the zulu distribution, and tests have succeeded in macos-latest

This looks good!

As for the failing tests for Jython 2.0 and 2.1, I suggest you drop support for that old versions

As already said here i cannot drop support for Jython 2.1 since my action is used in some projects of mine and in the toolchain of my workplace production build system.

I had to set for using windows runners instead of ubuntu (which are way faster) but it used to work perfectly, allowing me to discover bugs before they even went in production. I know that it's way too old but it's my work 😄