Upgrade inject-api to 2.x

[rguillome]

Hi,

Using airline 2 in a project, I notice that you use

      <dependency>
        <groupId>jakarta.inject</groupId>
        <artifactId>jakarta.inject-api</artifactId>
        <version>${dependency.javax-inject}</version>
      </dependency>

with version 1.0.3

The problem is that to keep a backward compatibility, the Jarkarta rebooted project still use the package name javax.inject instead of a new one. Could it be possible to upgrade the dependency to 2.x and so to use in the airline code the package jakarta.inject instead of the old one ? I could try a PR if you wish.

Regards,

[rvesse]

Do you have a pointer to the new dependency?

In a lot of cases Jakarta have been forced to rename from javax. to jakarta. due to trademark issues so I'm kinda surprised there is a case where they've been able to not do this

It would also be useful to understand why and where this is a problem. Is this leading to a specific error/dependency confusion in your project?

If so is it possible for you to workaround this for now by using Maven dependency exclusion?

[rguillome]

Hi rvesse,

I pushed this PR to the presto repository. As you can see in the thread, the maintainer of the presto project stucks a bit on the maven exclusion of the jakarta inject-api project.

So I'm here to discuss the possibilty to move airline to a more "cleaner" version of the jakarta project, one with no confusion with a pretty well spread Java dependency (javax.inject)

[rvesse]

Ok. My main concern is that this will be a breaking change for folks because the metadata discovery uses the @Inject annotation as part of its processing.

So it seems like there's two options here:

1. Move to the new dependency, but only for the 3.x branch. Users will have to explicitly change their import statements to import the new package for things to continue to work. Note the Breaking Change in the Migration Guide. Since it would be part of a major version bump the breaking change would be acceptable.
2. Switch to the new dependency but also do some hacky Class.forName() stuff to maintain backwards compatibility i.e. the old annotation would still be recognisable only when the user has the old dependency on the class path.

Note it's also possible to do both i.e. adopt Option 1 for the 3.x branch and Option 2 for the remaining 2.x releases.

[gsmet]

@rvesse about this particular subject: I think you should keep the hackyish solution for a while, both for 2 and 3. Both Jakarta flavors will coexist for a while as the migration will be a massive change for the whole Java ecosystem.

Typically, for Quarkus, we will migrate later this year (no date yet) but if you are releasing 3.x before that, I would still very like to benefit from the new features (typically the @PositionalArgument thing).

[rvesse]

@gsmet Good to have another real world use case to justify the solution, thanks 👍

[gsmet]

@rvesse I thought a bit more about this and might open a PR soon if you don't beat me to it. The week is going to be busy but I'll try to find some time to draft (and test) my idea.

[rvesse]

Took an initial look at this today, main problem is going to be backwards compatibility for internal classes e.g. HelpOption, Help etc. because if I upgrade the dependency then those have to use jakarta.inject.Inject so if anyone tries to use those in their application where they're still using the old dependency those won't function

Might be cleaner just to use our own annotation as proposed in #81 and try and push towards a 3.x release sooner rather than later

[gsmet]

@rvesse so my idea was to:

• use a specific annotation
• allow users to push additional annotations to take into account via an API

Why? Because in some cases, you want to also use CDI for your commands (that's my case, I inject CDI beans there) and if you mix both annotations, things are gonna get messy. And yes, I take special care to avoid conflicts in my CDI resolution.

So, for 3, I think a good option would be to:

• only enable the specific annotation by default
• if people want to enable others, they can do it with ParserBuilder. So for instance, I could push jakarta.inject.Inject as an additional annotation to take into account

For 2, it's a bit more tricky. If 3 doesn't come too late, we could keep the situation as is. If not, I would say, we should use the reflection trick even if not pretty. But it should be done once and only once before parsing the thing.

[rvesse]

Yeah think I'm leaning towards the same basic idea. Some default annotations that are supported with the capability to configure additional ones.

Also have an idea for a Maven shade trick to make both versions of @Inject coexist nicely (and still allow folks to exclude the version they don't want)

[gsmet]

Also have an idea for a Maven shade trick to make both versions of @Inject coexist nicely (and still allow folks to exclude the version they don't want)

Could you expand on that? You need to be very careful not polluting the user namespace with a ton of different @Inject annotations. In a project with injection, you use this one everywhere and if you have 5 of them around, it's going to be error prone and painful. I /think/ just handling your internal annotation by default and allow users to push additional ones would be enough and cleaner.

But maybe I misunderstood your idea :).

[rvesse]

Also have an idea for a Maven shade trick to make both versions of @Inject coexist nicely (and still allow folks to exclude the version they don't want)

Could you expand on that? You need to be very careful not polluting the user namespace with a ton of different @Inject annotations. In a project with injection, you use this one everywhere and if you have 5 of them around, it's going to be error prone and painful. I /think/ just handling your internal annotation by default and allow users to push additional ones would be enough and cleaner.

But maybe I misunderstood your idea :).

I was thinking of creating an airline-backcompat-javaxinject artefact that repackages the old javax.inject, this would be an optional extra dependency folks could drop in for backwards compatibility. Possibly add a similar optional dependency for those that want to use the new jakarta.inject instead.

Then switch over to our own new @AirlineModule (other suggestions welcome) annotation as the default with no inject dependency by default

[rvesse]

122 is a draft PR that folds in the various suggestions from this issue thread. I plan to finish this work up and push out a new release next week

[rvesse]

As of 3.0.0 only @AirlineModule is supported by default and @Inject is now an opt-in feature that requires users to specify explicit dependencies as the relevant dependencies are now optional