sghpjuikit / player

Audio player and management application.
21 stars 2 forks source link

Adapt Kotlin compiler target to Java version #168

Closed xeruf closed 5 years ago

sghpjuikit commented 5 years ago

See WidgetManager line that sets widget kotlinc args: "-jvm-target", "12",. THis needs to be adjusted also, but now we need to get Java version. Kotlin has KotlinVersion object for compile-time version constant, but Java being Java, we need to query System property Java.version and parse major version. Not an issue and I can do this also, but I'd like to first have this confirmed - is this absolutely necessary?

This is all for J11 support due to lack of J12 support on Linux right? Is there no end in sight? Supporting multiple version is not a good idea - particularly because we are using compilers in runtime and stuff.

xeruf commented 5 years ago
  1. Parsing Java version is not some hard challenge, don't you already have some util for that?
  2. We need to take care of this anyways for all the timer where we support multiple versions, not just now with 11 and 12. It's not costing us anything, so I don't get why you have a problem with it? This is merely a onetime implementation thing.
sghpjuikit commented 5 years ago

Parsing means relying on non compile-time contract. I see no reason to do that unless absolutely necessary.

It costs us a lot. Like class version problems. Added complexities in build and code, potential dependency problems and more. We are already having PRs and discussions - that is a cost. It costs me hours of my time and I want to eliminate those costs.

I do not want to support multiple versions. That's what I want - remove J11 support. So: is there an ETA on when this will be possible? Its about J12 on Linux right?

xeruf commented 5 years ago

Parsing means relying on non compile-time contract. I see no reason to do that unless absolutely necessary.

Oh come on, it's not like getting the Java Version from the System is some weird unknown thing. And actually, you don't even need to parse it, you can just throw the result to the widget compiler.

It costs us a lot. Like class version problems.

Not if everything is compiled with the same version?

Added complexities in build and code, potential dependency problems and more. We are already having PRs and discussions - that is a cost. It costs me hours of my time and I want to eliminate those costs.

the dependencies shouldn't care whether we use JDK 11 or 12, and I haven't yet found one that does

I do not want to support multiple versions. That's what I want - remove J11 support. So: is there an ETA on when this will be possible? Its about J12 on Linux right?

I'm testing Zulu right now, OpenJDK 12+ won't be made available for the current Ubuntu LTS version, on which I'll be for at least another year. But anyways, JDK 11 is an LTS version, you can't just move past that?

sghpjuikit commented 5 years ago

it's not like

Yes it is. Why do you want to create unreliable systems. Java versioning scheme already changed not too long ago. Why would you ever rely on something like that if you didnt have to. Programming is a virtue and part of that is to lessen the maintenance and complexity burden of dependencies like these. Why cant you see that? It is imperative in a project where single person takes care of 60k lines of code. I do not have time to babysit them.

don't even need to parse it

Is that guaranteed? Can you guarantee that? It's me who is going to be reading up on that for 30 10? minutes on Saturday.

Not if everything is compiled with the same version?

We are using 3 compilers at build time and 3 different compilers at runtime. Can you guarantee no class version problems in all current and future use cases? Application updates, JDK/Kotlin updates, developer changing build settings, etc? Because I can not.

In fact I already had class version problems with widgets multiple times. Your request for dual Java version support makes it easy to run into them.

We do not have a widget out-of-date check for application jar modification time nor jdk version set up either - the widgets will simply crash with obscure errors leaving dev/user confused.

dependencies shouldn't care whether we use JDK 11 or 12

You yourself created PR to use older OpenJFX version because it screwed up with your Java. Java 9 broke a lot of stuff and the backward compatibility is not guaranteed. The Jigsaw also puts more restrictions on stuff. There are deprecations and removals. You claim is correct only in a very naive sense, unfortunately.

I'm testing Zulu right now

I'm finding it hard to believe that Ubuntu doesnt support half a year old Java release in at least some form. I keep thinking JDK12 is LTS... Anyway, in ideal scenario we would bundle JDK with the app in a release, so to provide a good/tested JDK version. A position of power. A reason to support single version. Having this power taken away by lack of support on part of some OS is not what I want.

I'd like to know your JDK options - results of some research, not necessarily extensive. Can you create an issue for tracking JDK12 support on Linux/Zulu or something and link what JDKs are available or not, when and why? It will help others and myself.

widget kotlinc jvm-target

I'll look into how to obtain used Java version, how to detect its changes and figure out the widgets stuff. Should be done over the weekend.

sghpjuikit commented 5 years ago

Im afraid the kotlin jvm target support may be more difficult that we thought. Because of the option to use JDK11, something which decides the user really, we can not use 12 at all. Having this set dynamically as you did does not solve anything really, unless we provide separate releases - JDK11 release and JDK12 release.

I'll have to think more about this.

xeruf commented 5 years ago

I'm finding it hard to believe that Ubuntu doesnt support half a year old Java release in at least some form.

This is Oracles' fault, not Ubuntus

Zulu JDK 12 is available for every OS. I just need to test. Nonetheless 11 is the LTS one.

Im afraid the kotlin jvm target support may be more difficult that we thought. Because of the option to use JDK11, something which decides the user really, we can not use 12 at all. Having this set dynamically as you did does not solve anything really, unless we provide separate releases - JDK11 release and JDK12 release.

A release compiled with JDK 12 will only work on JDK 12 either way, it doesn't make a difference what the Kotlin compiler does.

You yourself created PR to use older OpenJFX version because it screwed up with your Java. Java 9 broke a lot of stuff and the backward compatibility is not guaranteed.

Java 9 was a special case. I don't know of any other version that broke backwards compatibility like that. And even here it only affected access to internal classes that were not supposed to be used in the first place ^^

I'll look into how to obtain used Java version, how to detect its changes and figure out the widgets stuff. Should be done over the weekend.

You always make a big fuzz about such things. My opinion: Something isn't working. I have a solution, yes it might not be optimal, but at least it brings us closer the problem. If we spent a week on each issue to solve it perfectly, we wouldn't get anywhere.

sghpjuikit commented 5 years ago

I make fuzz because Im a one person and I need to cut down effort in the long run. I do not have luxury to care for temporary fixes. I'll be the one removing them later.

A release compiled with JDK 12 will only work on JDK 12

Well, it never occurred to me that it means we need to do 'more' releases. Though in the end it means 12 for Windows and 11 for Linux. If 12 will really be not usable. Pls understand that if JDK12 is available and you simply have problem with using it, that is not a reason to set back the project. One thing is absolutely clear - multiple JDk version support is temporary and last time ever used.

xeruf commented 5 years ago

Well, it never occurred to me that it means we need to do 'more' releases.

No! This is merely a thing for development, not for the releases.

multiple JDk version support is temporary and last time ever used.

Once we have finished this, multiple JDK version support shouldn't require any further maintenance. There will always be transition periods.

xeruf commented 5 years ago

See latest commit. No parsing necessary. Ready to merge afaik.

sghpjuikit commented 5 years ago

You did as I would have done from top of my head so to be safe. ++. However let me investigate first. I desire compile time constant.

Also see: https://stackoverflow.com/questions/2591083/getting-java-version-at-runtime You were wrong that the version can be used as is (as expected).

As I expected, there is Runtime.version(). I tested it and it works. World is saved. I'll change to that and merge soon.

sghpjuikit commented 5 years ago

I recommend you check the final solution, bot in gradle build and WidgetManager.kt

xeruf commented 5 years ago

👍