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

dtcw ignores installed Java RE when docker is installed - your java version 17 is too new #1031

Closed
mh182 opened this issue Feb 20, 2023 · 15 comments · Fixed by #1077
Closed
Assignees
Milestone

Comments

@mh182
Copy link
Collaborator

mh182 commented Feb 20, 2023

Following the installation instructions as described in Install docToolchain with a newer Java version fails.

The reason is due my setup of having a JRE 17 and Docker installed.

I assume users with an installed Docker will prefer the docToolchain provided as a container image. But some people may prefer to install the tool chain even if they have docker installed.

To Reproduce

System setup: Debian unstable with OpenJDK 17 and docker installed. docToolchain was installed with sdk install doctoolchain.

➜ which java
/usr/bin/java

➜ java -version
openjdk version "17.0.6" 2023-01-17
OpenJDK Runtime Environment (build 17.0.6+10-Debian-1)
OpenJDK 64-Bit Server VM (build 17.0.6+10-Debian-1, mixed mode, sharing)

➜ docker --version
Docker version 20.10.23+dfsg1, build 7155243

Steps to reproduce the behavior:

  1. Follow installation instructions as described in Install docToolchain
~/tmp 
➜ mkdir test-doctoolchain && cd test-doctoolchain

~/tmp/test-doctoolchain 
➜ curl -sLo dtcw doctoolchain.github.io/dtcw

~/tmp/test-doctoolchain 
➜ chmod +x dtcw 

~/tmp/test-doctoolchain 
➜ ./dtcw 

dtcw - docToolchain wrapper V0.36
docToolchain V2.1.0
docToolchain as CLI available
docker available
home folder exists
No valid argument(s) supplied
  1. Due to Wrapper script doctoolchain.github.io/dtcw still uses v.2.1.0 #1030 I manually changed the wrapper script to use the current tool chain version to v2.2.0
~/tmp/test-doctoolchain 
➜ sed -i 's/2.1.0/2.2.0/' dtcw 
  1. Install Java RE with getJava
➜ ./dtcw getJava

dtcw - docToolchain wrapper V0.36
docToolchain V2.2.0
docToolchain as CLI available
docker available
home folder exists
this script assumes that you have linux as operating system (x64 / linux)
it now tries to install Java for you
downloading JDK Temurin 11 from Adoptium to /home/max/.doctoolchain/jdk.tar.gz
>>> https://api.adoptium.net/v3/binary/latest/11/ga/linux/x64/jdk/hotspot/normal/eclipse?project=jdk /home/max/.doctoolchain/jdk/jdk.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0
100  185M  100  185M    0     0  10.5M      0  0:00:17  0:00:17 --:--:-- 6437k
expanding JDK

~/tmp/test-doctoolchain took 20s
  1. Available tasks exists with error since my installed Java version is too new
➜ ./dtcw tasks --group=doctoolchain

dtcw - docToolchain wrapper V0.36
docToolchain V2.2.0
docToolchain as CLI available
docker available
home folder exists
use cli install /home/max/.local/share/sdkman/candidates/doctoolchain/current/bin/doctoolchain
Java Version 17
your java version 17 is too new (>14): /usr/bin/java
please update your java installation and try again

it might be that you have installed the needed version java in another shell from which you started dtcw
dtcw is running in bash and uses the PATH to find java

to install a local java for docToolchain, you can run
./dtcw getJava

another way to install or update java is to install
sdkman and then java via sdkman
https://sdkman.io/install
$ curl -s "https://get.sdkman.io" | bash
$ sdk install java

or you can download it from https://adoptium.net/

make sure that your java version is between 8 and 14

If you do not want to use a local java installation, you can also use docToolchain as docker container.
In that case, specify 'docker' as first parameter in your statement.
example: ./dtcw docker generateSite

~/tmp/test-doctoolchain 
❯ echo $?
1
  1. Setting JAVA_HOME to the previously downloaded JRE is ignored
~/tmp/test-doctoolchain 
➜ export JAVA_HOME=/home/max/.doctoolchain/jdk

~/tmp/test-doctoolchain 
➜ $JAVA_HOME/bin/java -version
openjdk version "11.0.18" 2023-01-17
OpenJDK Runtime Environment Temurin-11.0.18+10 (build 11.0.18+10)
OpenJDK 64-Bit Server VM Temurin-11.0.18+10 (build 11.0.18+10, mixed mode)

~/tmp/test-doctoolchain 
➜ ./dtcw tasks --group=doctoolchain

dtcw - docToolchain wrapper V0.36
docToolchain V2.2.0
docToolchain as CLI available
docker available
home folder exists
use cli install /home/max/.local/share/sdkman/candidates/doctoolchain/current/bin/doctoolchain
Java Version 17
your java version 17 is too new (>14): /usr/bin/java
please update your java installation and try again

it might be that you have installed the needed version java in another shell from which you started dtcw
dtcw is running in bash and uses the PATH to find java

to install a local java for docToolchain, you can run
./dtcw getJava

another way to install or update java is to install
sdkman and then java via sdkman
https://sdkman.io/install
$ curl -s "https://get.sdkman.io" | bash
$ sdk install java

or you can download it from https://adoptium.net/

make sure that your java version is between 8 and 14

If you do not want to use a local java installation, you can also use docToolchain as docker container.
In that case, specify 'docker' as first parameter in your statement.
example: ./dtcw docker generateSite

Expected behavior
After executing dtcw getJava the newly installed Java RE is used.

I would change the documentation in Install docToolchain and make a clear separation between following concepts:

  • Installing the tool chain
    • Running the tool chain provided as container image (with docker or podman)
    • Installing the tool chain to run locally
  • Make an existing project (git-repo) ready to use the tool-chain (by adding the wrapper)

Side-note: the output about the missing Java environment is erroneous. Using sdk install java will pick the current JRE which is not supported by docToolchain.

➜ sdk install java

Downloading: java 17.0.6-tem
...
Installing: java 17.0.6-tem
Done installing!

Setting java 17.0.6-tem as default.

allgebras/doctools/docToolchain via ☕ v17.0.6 took 28s 
➜ java -version
openjdk version "17.0.6" 2023-01-17
OpenJDK Runtime Environment Temurin-17.0.6+10 (build 17.0.6+10)
OpenJDK 64-Bit Server VM Temurin-17.0.6+10 (build 17.0.6+10, mixed mode, sharing)

➜ ./dtcw tasks --group=doctoolchain

dtcw - docToolchain wrapper V0.36
docToolchain V2.2.0
docToolchain as CLI available
docker available
use cli install /home/max/.local/share/sdkman/candidates/doctoolchain/current/bin/doctoolchain
Java Version 17
your java version 17 is too new (>14): /home/max/.local/share/sdkman/candidates/java/current/bin/java
please update your java installation and try again
...

Additional context
Looking into the dtcw wrapper script is see that the JAVA_HOME is only setup in case docker is not installed.

@rdmueller
Copy link
Member

argh. yes, this is still a bug in the wrapper. you have to run ./dtcw *local* something to switch to a local copy and use the downloaded JDK.

So, if you spot this bug in the dtcw, this would be quite helpful :-)

@mh182
Copy link
Collaborator Author

mh182 commented Feb 21, 2023

I started to test the installation instructions for the different setups:

What I don't like is that the wrapper script changes its behavior as soon the user installs more than one possible docToolchain runtime environment. For example the user used dtcw since a long time and had to install the Docker runtime environment due to another project. The command the user was used to call ./dtcw generateHTML will suddenly not work anymore

❯ ./dtcw generateHTML

dtcw - docToolchain wrapper V0.36
docToolchain V2.2.0
docToolchain as CLI available
docker available
use cli install /home/max/.local/share/sdkman/candidates/doctoolchain/current/bin/doctoolchain
Java Version 17
your java version 17 is too new (>14): /usr/bin/java
please update your java installation and try again

it might be that you have installed the needed version java in another shell from which you started dtcw
dtcw is running in bash and uses the PATH to find java

to install a local java for docToolchain, you cann run
./dtcw getJava

another way to install or update java is to install
sdkman and then java via sdkman
https://sdkman.io/install
$ curl -s https://get.sdkman.io | bash
$ sdk install java

or you can download it from https://adoptium.net/

make sure that your java version is between 8 and 14

If you do not want to use a local java installtion, you can also use docToolchain as docker container.
In that case, specify 'docker' as first parameter in your statement.
example: ./dtcw docker generateSite

Even if the bug for this issue is fixed the user is confronted with a sudden change.

Lets assume we have the least tech savvy user, an author which does not know much about sdkman, Docker and JVM. I see following use cases:

  • the author runs docToolchain locally and at a later stage installed Docker environment
  • the author runs docToolchain via Docker and at a later stage installed docToolchain with sdkman (for whatever reason)

IMHO it would make sense to choose a "sane default" in which order the different environments are use in case more than one runtime environment exists and show the used environment during execution.

What about "local" before "sdk" before "docker"? What do you think about this?

@rdmueller
Copy link
Member

What about "local" before "sdk" before "docker"? What do you think about this?

I thought it is this way :-)

sdkman is an option I never used for docToolchain. So, I don't care if we change the behaviour.

I wonder if it makes sense to force the use of the temurin JDK 11 and remove the getJava task?

@mh182
Copy link
Collaborator Author

mh182 commented Feb 21, 2023

We have to differentiate between the order of how an installation is suggested and the order of which environment is used when more than one is installed.

I would suggest following changes to the wrapper script:

  • in case more than one runtime is installed pick the existing runtime in the following order: local, sdk, docker (the same order in which the installation process pick the order).
  • The user can, at any time, pick the desired runtime by providing the additional argument 'local', 'sdk, or 'docker'

I wonder if it makes sense to force the use of the temurin JDK 11 and remove the getJava task?
Somehow the user needs an easy way to install the right JVM if it isn't already installed. Having the getJava task is nice to reduce the complexity for the user.

getJava should differentiate between the following use cases:

  • doesn't make sense if the CDT runtime is a docker environment
  • is the JRE available to the dtcw the right version:
    • if yes, provide feedback that everything is OK and nothing is done
    • if no we should different between the DTC runtime
      • local: install the JDK in $HOME/.doctoolchain/jdk
      • sdk: install the needed Java RE with sdk install

Does that make sense?

@rdmueller
Copy link
Member

shall we crate a new issue for that?

@mh182
Copy link
Collaborator Author

mh182 commented Feb 21, 2023

shall we crate a new issue for that?

If you don't mind I would like to take a look at this issue before creating a new one.

@mh182 mh182 self-assigned this Feb 26, 2023
@mh182 mh182 added the 🐞 bug label Feb 26, 2023
@mh182
Copy link
Collaborator Author

mh182 commented Feb 27, 2023

I just noticed that there are two version dtcw:

My question is:

  • why keeping two versions?
  • which one is the master version, which boils down to explain the release process of dtcw?

@rdmueller
Copy link
Member

The one at doctoolchain is the master and it is deployed via a github action to doctoolchain.github.io .

The github.io one has the benefit that it gets a good shorr url (doctoolchain.org/dtcw) which is good for the curl statement.

In the past, both where disjunct which lead to trouble. Maybe we should add some more comments/docs about this.

@mh182
Copy link
Collaborator Author

mh182 commented Feb 28, 2023

I think this issue is blocked by #988. So I'm going to fix that first.

@mh182
Copy link
Collaborator Author

mh182 commented Mar 1, 2023

Since #988 is ready for integration, I'm going to work on this one.

Looking at the dtcw code, I think we need to refactor major parts of it.

@rdmueller: do you mind if the wrapper script gets a major overhaul?

@rdmueller
Copy link
Member

@mh182 I don't mind a major overhaul, as long as things get better :-)

@obfischer
Copy link

Same issue here. I had to edit the dtcw by hand to remove the Docker support.

@rdmueller
Copy link
Member

@obfischer really? ./dtcw local <task> should do the trick. Didn't this work for you?

@obfischer
Copy link

@obfischer really? ./dtcw local <task> should do the trick. Didn't this work for you?

I was not aware of this option.

@mh182
Copy link
Collaborator Author

mh182 commented Mar 9, 2023

With the PR I changed how dtcw behaves for the user.

The major change is that dtcw doesn't install stuff automagically but guides the user to possible solutions if a problem occurs.

Explicit is better than implicit: the user has to use ./dtcw install {component} to install something.

Usage: ./dtcw [environment] [option...] [task...]
       ./dtcw [local] install {doctoolchain | java | reveal.js}

Use 'local', 'sdk' or 'docker' as first argument to force the use of a specific
docToolchain environment:
    - local: installation in '/home/max/.doctoolchain'
    - sdk: installation with SDKMAN! (https://sdkman.io/)
    - docker: use docToolchain container image

Example:

  • No docToolchain installed tells the user to run ./dtcw local install doctoolchain.
  • No java installed, tell the user to install java with ./dtcw local install java.
  • Use of generateDeck will tell the user to call ./dtcw local install reveal.js

Note: this change only affects the installation on the local environment. All other command invocations behave like before.

So if someone installs dtcw with an installed docker environment, calling dtcw will pick the docker environment and start pulling the docToolchain image.

Maybe we should make this consistent and ask the user to call ./dtcw docker tasks before pulling the 1 GB image ;-)

Do you think we should integrate this user visible change?

@mh182 mh182 linked a pull request Mar 9, 2023 that will close this issue
mh182 added a commit to mh182/docToolchain that referenced this issue Mar 15, 2023
- Fix bug docToolchain#1031
- Fix: support of JAVA_HOME which was silently ignored.
- Add `--version` option
- dtcw API change: use explicit 'install' command to install
  docToolchain locally.
- Improve output: provide the concept of 'environments'.

Refactorings:
- Replace boolean flags which controlled dtcw behavior with associative
  arrays.

Tests:
- Use mocks for dtcw test suite to improve performance.
- Added end-to-end tests for installation in 'local' and 'sdk'
  environments.
mh182 added a commit to mh182/docToolchain that referenced this issue Mar 18, 2023
- Fix bug docToolchain#1031
- Fix: support of JAVA_HOME which was silently ignored.
- Add `--version` option
- dtcw API change: use explicit 'install' command to install components.
- Improve output: provide the concept of 'environments'.
- Refactoring to replace 6 boolean flags with descriptive functions

Tests:
- Use mocks for dtcw test suite to improve performance.
- Added end-to-end tests for installation in 'local' and 'sdk'
  environments.
mh182 added a commit to mh182/docToolchain that referenced this issue Mar 29, 2023
- Fix bug docToolchain#1031
- Fix: support of JAVA_HOME which was silently ignored.
- Add `--version` option
- dtcw API change: use explicit 'install' command to install
  components locally.
- Improve output: provide the concept of 'environments'.

Refactorings:
- Replace boolean flags which `available_environments` and `dtc_installations`.

Tests:
- Use mocks for dtcw test suite to improve performance.
- Added end-to-end tests for installation in 'local' and 'sdk'
  environments.
mh182 added a commit to mh182/docToolchain that referenced this issue Apr 25, 2023
- Fix bug docToolchain#1031
- Fix: support of JAVA_HOME which was silently ignored.
- Add `--version` option
- dtcw API change: use explicit 'install' command to install
  components locally.
- Improve output: provide the concept of 'environments'.

Refactorings:
- Replace boolean flags which `available_environments` and `dtc_installations`.

Tests:
- Use mocks for dtcw test suite to improve performance.
- Added end-to-end tests for installation in 'local' and 'sdk'
  environments.
mh182 added a commit to mh182/docToolchain that referenced this issue Apr 25, 2023
- Fix bug docToolchain#1031
- Fix: support of JAVA_HOME which was silently ignored.
- Add `--version` option
- dtcw API change: use explicit 'install' command to install
  components locally.
- Improve output: provide the concept of 'environments'.
- Show deprecation of 'getJava'
- Skip 'install doctoolchain' if already installed

Refactorings:
- Replace boolean flags which `available_environments` and `dtc_installations`.

Tests:
- Use mocks for dtcw test suite to improve performance.
- Added end-to-end tests for installation in 'local' and 'sdk'
  environments.
mh182 added a commit to mh182/docToolchain that referenced this issue Apr 26, 2023
- Fix bug docToolchain#1031
- Fix: support of JAVA_HOME which was silently ignored.
- Add `--version` option
- dtcw API change: use explicit 'install' command to install
  components locally.
- Improve output: provide the concept of 'environments'.
- Show deprecation of 'getJava'
- Skip 'install doctoolchain' if already installed

Refactorings:
- Replace boolean flags which `available_environments` and `dtc_installations`.

Tests:
- Use mocks for dtcw test suite to improve performance.
- Added end-to-end tests for installation in 'local' and 'sdk'
  environments.
@mh182 mh182 added this to the Java 17 milestone Apr 27, 2023
mh182 added a commit to mh182/docToolchain that referenced this issue Apr 28, 2023
- Fix bug docToolchain#1031
- Fix: support of JAVA_HOME which was silently ignored.
- Add `--version` option
- dtcw API change: use explicit 'install' command to install
  components locally.
- Improve output: provide the concept of 'environments'.
- Show deprecation of 'getJava'
- Skip 'install doctoolchain' if already installed

Refactorings:
- Replace boolean flags which `available_environments` and `dtc_installations`.

Tests:
- Use mocks for dtcw test suite to improve performance.
- Added end-to-end tests for installation in 'local' and 'sdk'
  environments.
@mh182 mh182 closed this as completed in bd8b0e6 Apr 28, 2023
rdmueller added a commit that referenced this issue May 15, 2023
* dtcw: closes #1031, improve output to guide the user

- Fix bug #1031
- Fix: support of JAVA_HOME which was silently ignored.
- Add `--version` option
- dtcw API change: use explicit 'install' command to install
  components locally.
- Improve output: provide the concept of 'environments'.
- Show deprecation of 'getJava'
- Skip 'install doctoolchain' if already installed

Refactorings:
- Replace boolean flags which `available_environments` and `dtc_installations`.

Tests:
- Use mocks for dtcw test suite to improve performance.
- Added end-to-end tests for installation in 'local' and 'sdk'
  environments.

* CI (dtcw): integrate dtcw tests into CI build

Add new worflow dtcw-test which runs shellcheck, and if successfull,
executes the test suite on Ubuntu and macOS runners.

Don't run GHA workflow `default-build` when dtcw changes.

GitHub Actions runners for macOS don't support running container images.
Thus the test execution has to be done on the virtual machine.

* CI build: remove obsolete resource/clone.sh

* test powershell on windows with bats

* Update dtcw-tests.yaml

* Update dtcw-tests.yaml

* Update dtcw-tests.yaml

* Update dtcw-tests.yaml

* Update dtcw-tests.yaml

* Update dtcw-tests.yaml

* Update dtcw-tests.yaml

* Update dtcw-tests.yaml

* Update dtcw-tests.yaml

* Update dtcw-tests.yaml

* Update DtcwOnPowershellSpec.groovy

* extracted ps1 tests to extra file

* some refactorings because of a 'shellcheck'

* switch powershell.exe to pwsh

* fixed tests

* added os to report filename

* added os to report filename

* added os to report filename

* added os to report filename

* updated tests

* removed chatGPT tests

* fix report name

---------

Co-authored-by: Max Hofer <mh182@chello.at>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants