actions / runner-images

GitHub Actions runner images
MIT License
10.01k stars 3.03k forks source link

JAVA_TOOL_OPTIONS environment variable now set on Ubuntu? #1437

Closed fniephaus closed 4 years ago

fniephaus commented 4 years ago

Describe the bug The JAVA_TOOL_OPTIONS environment variable is set on Ubuntu, which should be the case according to this line. However, this was not the case about a week ago. We noticed this because the environment variable is picked up by an invocation of java --module-path ... --describe-module ....

Area for Triage: Java

Question, Bug, or Feature?: Bug

Virtual environments affected

Expected behavior JAVA_TOOL_OPTIONS is set, which is the case now.

Actual behavior JAVA_TOOL_OPTIONS was not set a few days ago.

Current build: https://github.com/hpi-swa/trufflesqueak/runs/1002639551?check_suite_focus=true

Same commit from three days ago: https://github.com/hpi-swa/trufflesqueak/runs/990481739?check_suite_focus=true

We are fixing the build failure on our end. Nonetheless, it is unclear why JAVA_TOOL_OPTIONS is now set on Ubuntu. The two workers seem to run the identical software version. Could someone explain this please?

Darleev commented 4 years ago

Hello @fniephaus, I have checked the issue. I believe the issue is not related to the JAVA_TOOL_OPTIONS environment variable. As far as I see, in the pipeline error message: AssertionError: ('HAMCREST', 'hamcrest', ['Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8', 'hamcrest file:///home/runner/.mx/cache/HAMCREST_42a25dc3219429f0e5d060061f71acb49bf010a0/hamcrest.jar automatic', 'requires java.base mandated', 'contains org.hamcrest', 'contains org.hamcrest.core', 'contains org.hamcrest.internal']) This issue is Java-related and I think there is some issue in the code. Please note that JAVA_TOOL_OPTIONS variable was not changed in the latest Ubuntu releases.

We are looking forward to your reply.

fniephaus commented 4 years ago

Correct, the build broke because of the AssertionError, and as I said we are fixing this. However, it is unclear why this wasn't a problem three days ago. JAVA_TOOL_OPTIONS was very likely not set, but why?

maxim-lobanov commented 4 years ago

@fniephaus , it definitely was set. Both builds (failed and successful) were run on the same image version 20200806.0 (you can check it on Set up job step). It means that both builds were run on identical VMs.

Could it be some issue with your environment that could override or block environment variables on VM?

fniephaus commented 4 years ago

Could it be some issue with your environment that could override or block environment variables on VM?

I don't think so, but it's not impossible. Both jobs were using an identical JDK and called out to java --module-path ... --describe-module ..., which checks JAVA_TOOL_OPTIONS and print Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8.

But anyway, we have fixed the issue on our end. Just wanted to double check that the environment has definitely not changed. Thanks for the swift response!

andrew2net commented 4 years ago

@fniephaus I bumped the same issue. How have you fixed it?

fniephaus commented 4 years ago

If you're a Truffle language dev and you use mx, update it. The fix is already available. If you don't know what I am talking about, you could try unsetting the environment variable before your script runs.

andrew2net commented 4 years ago

I'm Ruby dev. One of the gems in my project uses Java which raises the error. Anyway unsetting the environment variable helps. Thank you very much @fniephaus

fniephaus commented 4 years ago

In that case, let me reopen this issue. What do you think, @maxim-lobanov?

eregon commented 4 years ago

Since JAVA_TOOL_OPTIONS produces extra output I think it's going to be problematic for many cases.

maxim-lobanov commented 4 years ago

Unfortunately, I don't have context why this variable was set initially. The single concern from my side about unsetting it is that it could affect customers who are expecting this variable to be set up.

We are not Java experts so if you can provide any context what JAVA_TOOL_OPTIONS=-Dfile.encoding=UTF8 param does and how unsetting could affect existing customers, it would be helpful

fniephaus commented 4 years ago

JAVA_TOOL_OPTIONS "can be useful to augment a command line". In this case, it does what the flag suggests: set the file encoding to UTF8. Here is a related JEP, its status however is still "draft". Git unfortunately doesn't tell us much why this environment variable is set (https://github.com/actions/virtual-environments/commit/f396818e23ecba9f79322615a55d190765a5fab5 is the commit). If we remove it, the JVM will fall back to the default encoding (whatever that is on each platform). It appears that JAVA_TOOL_OPTIONS is a problem for other projects as well, e.g.:

maxim-lobanov commented 4 years ago

cc: @brunoborges for any thoughts

MaryamZi commented 4 years ago

JAVA_TOOL_OPTIONS "can be useful to augment a command line". In this case, it does what the flag suggests: set the file encoding to UTF8. Here is a related JEP, its status however is still "draft". Git unfortunately doesn't tell us much why this environment variable is set (f396818 is the commit). If we remove it, the JVM will fall back to the default encoding (whatever that is on each platform). It appears that JAVA_TOOL_OPTIONS is a problem for other projects as well, e.g.:

Yes, we also observed assertion failures due to the additional output Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8.

eregon commented 4 years ago

It also breaks RubyGems' CI: https://github.com/rubygems/rubygems/pull/3891/checks?check_run_id=1012405917

Check rubygems install produced no warnings0s
    PATH: /home/runner/.rubies/jruby-9.2.11.1/bin:/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin:/usr/share/rust/.cargo/bin:/home/runner/.config/composer/vendor/bin:/home/runner/.dotnet/tools:/snap/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games
Run test ! -s errors.txt || (cat errors.txt && exit 1)
Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
##[error]Process completed with exit code 1.

cc @headius This will likely fail a lot of runs of JRuby in GitHub Actions.

I would recommend reverting this change. The variable was clearly no set e.g. 2 weeks ago: https://github.com/rubygems/rubygems/actions/runs/198543013 (or the default java in GitHub Actions would not print that message, but that seems unlikely)

Users can set JAVA_TOOL_OPTIONS if they need. Probably it's very rare users need that, compared to the dozens of workflows it already broke.

maxim-lobanov commented 4 years ago

@eregon , there is no any changes from our side. This variable is set on image for a long time (more than 9 months): https://github.com/actions/virtual-environments/blob/main/images/linux/scripts/installers/java-tools.sh#L49

We are not actually sure why it started to produce problems now.

Also, please see builds from the first message:

The both builds are run on the same image version: 20200806.0. It means the both builds are run on identical VMs. Even in your builds:

The both are run on the same image version: 20200806.0 (see Set up job step for details)

eregon commented 4 years ago

Those builds are proofs there are differences between runs on that same image version. I don't know either how it changed, but let's fix it either way (#1467).

Maybe something used to unset JAVA_TOOL_OPTIONS? Or it was a different default java? I can't imagine anything else causing this.

maxim-lobanov commented 4 years ago

Hello everyone, we have identified the root cause of this issue. Previously, this variable was trimmed from image during deployment and this is the reason why it didn't cause issues previously. This logic was disabled recently and issue appeared.

We are going to merge PR prepared by @fniephaus and also we are going to redeploy existing images with hotfix to mitigate issue faster

fniephaus commented 4 years ago

Fantastic, thank you very much @maxim-lobanov!

maxim-lobanov commented 4 years ago

Small update: hotfix was deployed to 95% of VMs so the most of the builds should be unblocked. Remain machines will be updated soon

miketimofeev commented 4 years ago

Images without JAVA_TOOL_OPTIONS=-Dfile.encoding=UTF8 were deployed. This issue can be closed.