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

woodstox-core 6.3/6.4 javac warning due to unexpected new dependency on aQute.bnd.annotation.Resolution #163

Closed
ben-spiller opened this issue Dec 6, 2022 · 6 comments
Milestone

Comments

@ben-spiller
Copy link

ben-spiller commented Dec 6, 2022

The GH-155 fix released in v6.3.1 introduces a major regression in woodstox-core since any build with javac warnings-as-errors enabled (as per best practice) will now fail (unless the aQute library is on classpath, which it usually isn't).

Example (using Java 11):

> javac -classpath 6.4.0\woodstox-core.jar -Werror WoodstoxBug.java
warning: unknown enum constant Resolution.OPTIONAL
  reason: class file for aQute.bnd.annotation.Resolution not found
error: warnings found and -Werror specified
1 error
1 warning

... where WoodstoxBug.java can be any code that refers to any of the main woodstox classes e.g.

public class WoodstoxBug {
	public static void main(String[] args){
		com.ctc.wstx.stax.WstxInputFactory f = new com.ctc.wstx.stax.WstxInputFactory();
	}
}

Requiring everyone to either disable warning checks or add an extra dependency on aQute would be a major compatibility break. The woodstox documentation states there is no mandatory dependency other than stax2, so could we get a fix that restores this?

@ben-spiller ben-spiller changed the title Regression: javac warning for due to unexpected new dependency on aQute.bnd.annotation.Resolution Regression: woodstox-core javac warning due to unexpected new dependency on aQute.bnd.annotation.Resolution Dec 6, 2022
@cowtowncoder
Copy link
Member

That's odd, dependency is optional/provided:

        <dependency>
          <groupId>biz.aQute.bnd</groupId>
          <artifactId>biz.aQute.bnd.annotation</artifactId>
          <version>${version.bnd.annotation}</version>
          <optional>true</optional>
          <scope>provided</scope>
        </dependency>

which I thought would prevent this?

@ben-spiller
Copy link
Author

ben-spiller commented Dec 8, 2022

That would only work for classes that are rarely used / not a core part of the API (and even then is a bit dodgy / needs very careful testing and doc of which packages/classes need aQute as a result).

But while configuration such as optional dependencies will allow you to get it building, the bottom line is that if you have a class that depends on aQute then it will have a reference to aQute in the compiled .class files so will get compiler warnings (or errors) when people try to use it. There's no way round that - it'd be a major breaking change for woodstox to start referencing aQute like this.

@cowtowncoder
Copy link
Member

@ben-spiller Is this due to referencing value other than Annotation types? Handling of missing annotation types is special and should be safe. But I fear we have the same issue here as with jackson-core:

FasterXML/jackson-core#824

and if so, yes, I suppose we better revert this addition here as well.

Thank you for bringing this to my attention. I wish I had realized unfortunate side effects: the intent is definitely to keep Woodstox as close to zero-dependency as possible (beyond Stax API, optional MSV for validation). My thinking was that OSGi annotations would be that way too, but alas not.

@ben-spiller
Copy link
Author

Is this due to referencing value other than Annotation types? Handling of missing annotation types is special and should be safe.

Correct, see my example above - merely referring to WstxInputFactory itself introduces the warning.

and if so, yes, I suppose we better revert this addition here as well.
Yes, like the jackson one, I think it needs reverting unfortunately

@cowtowncoder cowtowncoder added this to the 6.5.0 milestone Jan 12, 2023
@cowtowncoder cowtowncoder changed the title Regression: woodstox-core javac warning due to unexpected new dependency on aQute.bnd.annotation.Resolution woodstox-core 6.3/6.4 javac warning due to unexpected new dependency on aQute.bnd.annotation.Resolution Jan 12, 2023
@cowtowncoder
Copy link
Member

Fixed via #164, will be in 6.5.0 release.

@cowtowncoder
Copy link
Member

6.5.0 released on January 14, 2023.

chatman referenced this issue in apache/lucene-solr Oct 23, 2023
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

2 participants