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

Plan for moving to newer java versions + modernising library #316

Open
bramp opened this issue Mar 11, 2024 · 20 comments
Open

Plan for moving to newer java versions + modernising library #316

bramp opened this issue Mar 11, 2024 · 20 comments

Comments

@bramp
Copy link
Owner

bramp commented Mar 11, 2024

As commented by @Euklios the java LTS are 8, 11, 17, 21, with "Java 8 was so popular that it won't die.", but many companies moving to active projects to java 17 or 21.

So I think we should plan for moving to the latest java (at some point) and at the same time, make appropriate changes to the API to modernise it. A quick shortlist (again by @Euklios):

  • Java has made a lot of progress towards immutable data
  • Add getters and mark public properties as deprecated
  • Release
  • Make properties private
  • Switch Java version
@bramp
Copy link
Owner Author

bramp commented Mar 11, 2024

I have to admit, I haven't been using java day-to-day for many years now, but I'm curious the trend with immutability and public properties.

Today we have something like this:

public class FFmpegProbeResult {
  public List<FFmpegStream> streams;

  public List<FFmpegStream> getStreams() {
    if (streams == null) return Collections.emptyList();
    return Collections.unmodifiableList(streams);
  }
}

Can we drop the getters, and just always do:

@Immutable
public class FFmpegProbeResult {
  @NonNull
  public ImmutableList<FFmpegStream> streams;

  FFmpegProbeResult(List<FFmpegStream> streams) {
    this.streams = ImmutableList.copyOf(streams == null ? Collections.emptyList() : streams);
  }
}

i.e drop the getters, and make all fields public but immutable. Avoid the defensive copies on getter calls. Move to Guava's ImmutableList to ensure the list can never change.

Maybe we should write a best practice / style guide for what the APIs should look like.

@AlexanderSchuetz97
Copy link

AlexanderSchuetz97 commented Mar 11, 2024

Please dont remove java 8 support.
For this library the "new" language features are pointless.
They offer no functional improvement over what exists.
No need to make the life of probably thousands of developers harder just for a bit of syntactic sugar.

While most companies use java 17 for new projects please dont abandon the poor people like me that have to maintain code bases that are older then we are ourselves. Due to the changes from java 8 to 9 there are countless projects that will NEVER make the jump from 8 to 11(or newer). The jump from 11 to 17 is easier, The jump from 17 to 21 is trivial. Most people that are still on 8 will not be able to migrate.

@Euklios
Copy link
Collaborator

Euklios commented Mar 11, 2024

@AlexanderSchuetz97
My condolences.

How dependent on this library are you? My argument is that most project using java 8 and this library should be fairly established and shouldn't depend on newer features.

I agree that upgrading isn't strictly necessary, but would improve developer experience and maintainability in this project. I'm fine with keeping this project on a lower version for compatibility reasons.

@bramp
Copy link
Owner Author

bramp commented Mar 11, 2024

Thanks for your input @AlexanderSchuetz97 ... if any jump does occur the old version of this library will still be supported.

One concern I do have, is many dependencies are moving off Java 8. Literally right now I'm trying to use java 8 to compile this project, and various maven plugins don't work, and the tests can't run (due to dependencies moving beyond 8). I think I can work around that, but at some point we will have to change.

However chances aren't we won't bump until we absolutely need to.

@Euklios
Copy link
Collaborator

Euklios commented Mar 11, 2024

@bramp
I would still opt for getters (or record style accessors). A lot of the tooling works better with getters/setters and encapsulation is better. It is more boiler plate (and I hate that as much as everyone else), but at least for me, the benefits outnumber the drawbacks.

@AlexanderSchuetz97
Copy link

AlexanderSchuetz97 commented Mar 11, 2024

The software I make/maintain is used in a medical environment where its used to transcode videos/pictures of patients. I was just looking on this repo on progress in regards to a CVE and just saw this. You are completely correct that such software is well estabilshed and I honestly If I could would never update the library. This is unfortunately not possible due to outside pressures.

Essentially my worry is that if you do remove java 8 support in a newer version and someone does find a vulnerabilty (be it a real one or a percieved one as the current one appears to be) someone will force me to do something about it eventually. If updating to a newer version is impossible due to no more updates for old java then I would essentially have to clone this library and start maintainig it myself. That or get rid of it entirely and do everything with ProcessBuilder.

As for build process, we use gradle but this is well possible with maven too. The trick is to run maven/gradle with a newer version of java but instruct the compiler to compile for an older version. I personally have many thingy I compile with maven running under jdk11 and compile for target 7.

@Euklios
Copy link
Collaborator

Euklios commented Mar 11, 2024

Regarding java version: Even if we bump, we could still support a minimal java 8 build.
It won't be equal to the master build, but fixes/some features could still be added.

@AlexanderSchuetz97
Copy link

AlexanderSchuetz97 commented Mar 11, 2024

@Euklios
If you do want to you could build a multi version jar. Its what log4j does to resolve this issue. Essentially its a jar file with classes for different java versions. The JDK picks the class for its java version. I have never needed to setup such a build, but you could look at log4j for guidance on how to set this up. Personally If I was the maintained of this repo Id just stick to java 8. Its the pragmatic solution since the only gain at least as far as I am aware is syntactic sugar. You will also introduce compile errors into peoples projects since many projects (some of ours too) overload some of your methods and mess with the internals of this library. (Mainly for some timeout scenarios I believe?)

Is the effort of maintaining 2 branches one for j8 one for newer really worth it?

@Euklios
Copy link
Collaborator

Euklios commented Mar 11, 2024

@AlexanderSchuetz97 You're not hiring by any chance? I'm already maintaining four forks of this library, what's one more?
Jokes aside, even if we update, I'd still be willing to patch if required.

What I'm doing right now is keeping a private fork of this library and adding my changes to that fork first. Artifacts are pushed to a private maven registry, where my other projects pull from. The time-consuming part is syncing with upstream

If the cve is actually more than smoke, feel free to open an issue. I'd be happy to fix asap

@AlexanderSchuetz97
Copy link

AlexanderSchuetz97 commented Mar 11, 2024

The current CVE is smoke at least as far as I can tell.
Jokes aside, I think that the next version of the library will not change anything in regards to the CVE but I doubt that anyone will resubmit the CVE so it will sort itself out naturally. If only one could teach automated vunerabilty scanners (that customers are using) that some CVE's are smoke my life would be much easier.

Anyways thanks for the nice chat and willingness to patch if necesarry. Its appreciated.

@Euklios
Copy link
Collaborator

Euklios commented Mar 11, 2024

Anyways thanks for the nice chat and willingness to patch if necesarry. Its appreciated.

Don't worry, if it's something trivial we could still consider a fix. Maybe that would keep your customers away for a bit.
Although executing shell commands is kinda our thing ^^

@Euklios
Copy link
Collaborator

Euklios commented Mar 11, 2024

Is the effort of maintaining 2 branches one for j8 one for newer really worth it?

We're not log4j and I agree, it's most likely not worth it.
My suggestion is a Java 8 build for people like you, with fixes and some small features where possible, while the master can develop on its own.

I'd like to add some bigger features that would greatly benefit from newer Java features, but they are not strictly required.
I'd benefit more from "cleaning" the API, removing public properties, and so on.

@bramp
Copy link
Owner Author

bramp commented Mar 11, 2024

FYI I just had to make these changes: 053c32c so this actually builds on JDK 8.

@Euklios
Copy link
Collaborator

Euklios commented Mar 11, 2024

I forgot that the Java version before 9 was 1.8, not 8. I should have noticed...

@Euklios
Copy link
Collaborator

Euklios commented Mar 11, 2024

@bramp
I'll close #269 in favour of this one.

If your intentions regarding that issue are still the same, I'd be willing to take over a maintainer role if you want to hand off some responsibility.
As long as I can close all the answered questions that are cluttering my view

@Euklios Euklios mentioned this issue Mar 11, 2024
12 tasks
@bramp
Copy link
Owner Author

bramp commented Mar 11, 2024

I'm happy to add you as a maintainer, just don't do anything I wouldn't do :). For Java 9+ I'd like to understand the exact benefits before we break our customers.

@Euklios
Copy link
Collaborator

Euklios commented Mar 11, 2024

That's fine. You fixed the build issues with Java 8, and as long as we don't plan to migrate soon, that doesn't block me.

@eygraber
Copy link

Any interest in a port to Gradle, or would you like to keep the project on mvn?

@bramp
Copy link
Owner Author

bramp commented Apr 28, 2024

@eygraber I'm not sure I know the advantages of gradle over maven, but I suspect since I would have to learn how to continue to maintain a gradle setup, this would make my life more difficult. Thus thanks, but I don't think it'll be good for the project.

@RichardTree
Copy link

Java 8 is 10 years old, these people crying about maintaining backward's compatibility have had 10 years to migrate their projects. I say let the asteroid hit the dinosaurs. 🦕 ☄️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants