JEnv-for-Windows icon indicating copy to clipboard operation
JEnv-for-Windows copied to clipboard

Use `.java-version` file to determine java version

Open lkleeven opened this issue 3 years ago • 5 comments

Potential solution for #36

lkleeven avatar Aug 18 '22 11:08 lkleeven

I currently went for the smaller approach of trying of just adding reading the file. It could be extended by completely replacing the current use of the config file in favor of the .java-version file.

lkleeven avatar Aug 18 '22 13:08 lkleeven

Hi, This looks like an excellent feature for jenv. Thank you for your contribution.

However, I think we should change the order of selecting the version.

If the .java-version file is preferred before the local version the user cannot override the preferences. The user may want to override them if he named his installation different than the repo. Or he may want to test a newer version.

This could obviously be achieved by changing the contents of the .jenv-version file. This approach seems tedious compared to executing a command via the command line and could lead to accidentally breaking git pushes. The user may be warned by a message that the version specified inside the .java-version file does not match the configured jenv local.

As far as I can tell from the JEnv Readme the .java-version file contains only the version string 11.0.2 jenv-getjava.psm1 however is expected to return a path to a java home C:/Program Files/Java/jre11.0.2 If I get this correct your code does not take care of this and just returns the contents of the file.

How I think this feature should be implemented:

  • Whenever jenv-getjava.psm1 is called it should check for an active jenv use
  • After that, it will check if a .java-version file is present and get the version
  • Then it will check if there is a jenv local specified
    • If so it will get the path, execute path/bin/java -version and parse the version
    • If the retrieved version does not match with the .java-version file the user is warned
  • In case there is no local specified it will parse the version of the global and compare them
  • If they don't match or there is no global it will loop over every jenv and check if it finds a matching version and suggests creating a local

Do you think you could implement this or have a better idea?

FelixSelter avatar Aug 18 '22 13:08 FelixSelter

Why would you want to put the automatic version resolving all the way to the end? I would expect most users to have a global version picked, so any time the file is present and the global doesn't match the version in the file we would just give a warning. That doesn't really sound very user friendly.

By the way the other JEnv only puts the major version in the .java-version file. If I can make the following suggestion for this feature and maybe make it a bit bigger:

  1. When adding a java runtime determine the version and store it in the config file
  2. For backwards compatibility add a utility to add the versions in case they aren't there yet
  3. When determining the java version to use:
    • Check active jenv and use if present
    • Get the version from .java-version if file present
    • If a local is specified:
      • And no java version present, use local
      • And java version present, check if versions match, i.e. stored version starts with the version from file:
      • If they match, just run
      • If they don't match find matching version and offer to update local config or continue with local config
    • If java version specified use matching java
    • Use global version

lkleeven avatar Aug 23 '22 10:08 lkleeven

@lkleeven Yea that sounds better. You may also want to give a warning if no installed version matches instead of just using the global. After #35 is implemented we should ask the user if he wants to continue with the global or download the appropriate version.How do you plan implementeling the backwards compatibility?

FelixSelter avatar Aug 23 '22 14:08 FelixSelter

Here is a function to get the Java Version from the executable path https://github.com/FelixSelter/JEnv-for-Windows/blob/main/src/util.psm1#L18

It may be updated to only return the major version This would only affect the automatic naming from jenv autoscan Which would be okay. The major version is easier to remember anyway and most people only care abou it

Use the Open-Prompt utility in the same util file

FelixSelter avatar Aug 23 '22 14:08 FelixSelter

My recommendation would be to keep the logic as close to how jenv works on linux and macos as much as possible (https://www.jenv.be/). I think you'll get a lot more people using jenv for Windows if that is the case. I'd also recommend you publishing it to some of the popular package managers such as Choco since one hasn't been published to-date.

tech-consortium avatar Nov 16 '22 10:11 tech-consortium

Unfortunately due to a move from a windows machine to mac, partially due to the fact that tools didn't align between me and the other developers who were all on Mac/Linux, I can't really work on this anymore.

In addition I fully agree with the recommendation of trying to keep the tools in line with how they work on different systems. That could have actually prevented my move to Mac.

lkleeven avatar Nov 16 '22 10:11 lkleeven

Thanks for the feedback guys

FelixSelter avatar Nov 16 '22 12:11 FelixSelter