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

6.20.2 missing openpdf dependency #351

Closed
Viserius opened this issue Apr 26, 2023 · 18 comments
Closed

6.20.2 missing openpdf dependency #351

Viserius opened this issue Apr 26, 2023 · 18 comments

Comments

@Viserius
Copy link

After updating our version from

        <dependency>
            <groupId>net.sf.jasperreports</groupId>
            <artifactId>jasperreports</artifactId>
            <version>6.20.1</version>
        </dependency>

to

        <dependency>
            <groupId>net.sf.jasperreports</groupId>
            <artifactId>jasperreports</artifactId>
            <version>6.20.2</version>
        </dependency>

We obtain the following exception:

java.lang.NoClassDefFoundError: com/lowagie/text/DocumentException
	at net.sf.jasperreports.export.pdf.classic.ClassicPdfProducerFactory.createProducer(ClassicPdfProducerFactory.java:44)
	at net.sf.jasperreports.engine.export.JRPdfExporter.createPdfProducer(JRPdfExporter.java:821)
	at net.sf.jasperreports.engine.export.JRPdfExporter.initExport(JRPdfExporter.java:716)
	at net.sf.jasperreports.engine.export.JRPdfExporter.exportReport(JRPdfExporter.java:677)
	at net.sf.jasperreports.engine.JasperExportManager.exportToPdf(JasperExportManager.java:217)
	at net.sf.jasperreports.engine.JasperExportManager.exportReportToPdf(JasperExportManager.java:542)

After executing mvn dependency:tree, we see on v6.20.1:

[INFO] +- net.sf.jasperreports:jasperreports:jar:6.20.1:compile
[INFO] |  +- commons-beanutils:commons-beanutils:jar:1.9.4:compile
[INFO] |  |  \- commons-collections:commons-collections:jar:3.2.2:compile
[INFO] |  +- commons-digester:commons-digester:jar:2.1:compile
[INFO] |  +- commons-logging:commons-logging:jar:1.1.1:compile
[INFO] |  +- com.github.librepdf:openpdf:jar:1.3.30.jaspersoft.1:compile
[INFO] |  +- org.jfree:jcommon:jar:1.0.23:compile
[INFO] |  +- org.jfree:jfreechart:jar:1.0.19:compile
[INFO] |  +- org.eclipse.jdt:ecj:jar:3.21.0:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-core:jar:2.13.5:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-databind:jar:2.13.5:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.13.5:compile
[INFO] |  \- com.fasterxml.jackson.dataformat:jackson-dataformat-xml:jar:2.13.5:compile
[INFO] |     +- org.codehaus.woodstox:stax2-api:jar:4.2.1:compile
[INFO] |     \- com.fasterxml.woodstox:woodstox-core:jar:6.4.0:compile

Whereas on version v6.20.2:

[INFO] +- net.sf.jasperreports:jasperreports:jar:6.20.2:compile
[INFO] |  +- commons-beanutils:commons-beanutils:jar:1.9.4:compile
[INFO] |  |  \- commons-collections:commons-collections:jar:3.2.2:compile
[INFO] |  +- commons-digester:commons-digester:jar:2.1:compile
[INFO] |  +- commons-logging:commons-logging:jar:1.1.1:compile
[INFO] |  +- org.jfree:jcommon:jar:1.0.23:compile
[INFO] |  +- org.jfree:jfreechart:jar:1.0.19:compile
[INFO] |  +- org.eclipse.jdt:ecj:jar:3.21.0:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-core:jar:2.13.5:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-databind:jar:2.13.5:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.13.5:compile
[INFO] |  \- com.fasterxml.jackson.dataformat:jackson-dataformat-xml:jar:2.13.5:compile
[INFO] |     +- org.codehaus.woodstox:stax2-api:jar:4.2.1:compile
[INFO] |     \- com.fasterxml.woodstox:woodstox-core:jar:6.4.0:compile

Note: the dependency com.github.librepdf:openpdf:jar:1.3.30.jaspersoft.1:compile has been removed in version v6.20.2. To resolve the error, this dependency (but then version 1.3.30.jaspersoft.2) should be added back in.

@Viserius
Copy link
Author

Pinpointed the breaking change to this commit
100b258

Removing a required dependency introduces a breaking change and should not be introduced in a hotfix release, especially not considering it is not mentioned in the release notes, nor does it provide instructions that users should include this dependency themselves.

@teodord Can we make this dependency non-optional again?

@teodord
Copy link
Collaborator

teodord commented Apr 26, 2023

@Viserius,

Can you add the OpenPDF dependency in your own application build, where you already have the JasperReports dependency?
Make sure you add the following repository where we keep our fork of OpenPDF until they accept the fix we need from them:
https:/TIBCOSoftware/jasperreports/blob/master/jasperreports/pom-parent.xml#L100

Thank you,
Teodor

@Viserius
Copy link
Author

Thank you for the quick reply @teodord.

While I was able to resolve the issue by adding the OpenPDF dependency myself, I am disappointed that this breaking change was introduced in a minor/patch update without any mention in the installation guide or changelog. This caused us to change our project structure to accommodate this update, which we believe is not a good practice. Moreover, it seems that quite a few other users did not expect this change, or were not informed that this required action on their part (#347 #348 @yu-shiba @DManstrator).

I strongly suggest that the developers follow the semantic versioning (semver.org) guidelines and consider reverting this change and reintroducing it in a major version like 7.0.0. Additionally, it is essential to communicate such changes in the changelog and set-up instructions to prevent confusion for users who may need to package OpenPDF themselves. Especially since a non-standard OpenPDF version is required, this change is highly unexpected and I would like to ask the maintainers to announce or communicate such changes clearly in the future.

@teodord
Copy link
Collaborator

teodord commented Apr 27, 2023

@Viserius

Should I also make Batik dependencies mandatory? There are 44 optional dependencies in JasperReports today. Should they all be mandatory?
I am curious if in your application build you have Batik dependencies referenced directly, or you rely on them being brought in transiently. If you do have Batik referenced directly, how did you know you need it?
If you don't have Batik, how do you know your reports would not crash one day because you are missing it?

I want to learn.

Thank you,
Teodor

@Viserius
Copy link
Author

@teodord

I see your point, and I agree with you that if you have such a large application with many optional use cases that are not always used by everyone, you do not want to package it all in one super fat JAR. My point primarily being that it was removed in a patch version without notice.

Still, I completely understand the dilemma. On the one hand, you do not want to package many JARs you do not use so your JAR becomes very large. On the other hand, requiring users to import the optional dependencies themselves without knowing what exactly they need, can also be confusing. I believe a common solution that is being applied to similar situations is to apply the microkernel design pattern. This would mean you'd not have 1 Jasperreports maven dependency that contains everything, but the basic jasperreports dependency is small with core functionality. By including extra plug-in jasperreports modules, you enable additional functionality. I.e., jasperreports-core, jasperreports-pdf, jasperreports-svg etc. This is one possible method of how you can be small, while being easily extendable at the same time. Another benefit is that you completely control the underlying dependencies being imported by the various module (i.e. the proprietary openpdf version), so users are not required to know if they need version 1.3.30, 1.3.30.jaspersoft.1, 1.3.30.jaspersoft.1, or 1.3.31 etc.

@teodord
Copy link
Collaborator

teodord commented Apr 28, 2023

We are going to consider splitting JasperReports into smaller jars.

For now, this tracker remains open so that others encountering the same problem can see what the solution is.
We are not going to revert the change that makes OpenPDF optional, especially since the damage is done and once everybody takes action, we are not going to hear about in the future.

Thank you,
Teodor

@soc
Copy link

soc commented Apr 28, 2023

@Viserius I agree with your concern, though I also believe that JasperReports authors can run their project any way they like (that includes "unprofessional").

@teodord
Copy link
Collaborator

teodord commented Apr 29, 2023

I want to solve this in a professional way, so please help me achieve it.

Since there is no way to take the release back from Maven repositories, you want us to do either of the following, or both:

  1. I apologise to everyone and ask for forgiveness.

  2. Make a new release, probably called 6.20.4, in which we put OpenPDF dependency back to mandatory, so that when you upgrade to it, your build systems remain intact;

2.a) If we ever want to make OpenPDF optional, we should only do it again in a 7.0.0 release, but announce it to everybody in every possible way we can, so that there is no surprise to anyone that they need to add OpenPDF and the JFrog repository to their builds. The error you encountered this time around would come again, but at least you would not be able to complain you did not know about it (you probably wont log this again, because you read the release notes and announcements, but I'm sure there would be others who do not read these things and would log it again anyway, so we would still need to put some fires out by telling them it was properly announced, they just did not pay attention to it).

2.b) We never ever attempt to make OpenPDF optional again.

I am doing 1) right now: Please accept my apologies for the mistake I have done when publishing 6.20.3 and made OpenPDF optional without properly announcing it. Please forgive me and I promise it will not happen again.

Please let me know how would you like us to proceed with item 2).

Thank you,
Teodor

@Viserius
Copy link
Author

Dear @teodord ,

Thank you for your prompt response and for the constructive discussion. I appreciate that you take this issue seriously and want to address it in a professional manner.

I understand that I could have added the missing dependency myself, but my intention was to help you and the project by highlighting the potential problems this could cause for other users who rely on PDF exporting. This was also supported by other open issues.

I want to emphasize that the decision to keep or remove the dependency in a breaking release is entirely up to the maintainers, and I don't want to provide any unsolicited advice on this matter. Still, introducing breaking changes is sometimes inevitable and even a good decision, as long as the user does not need to resort to issue trackers for instructions on how to upgrade. Applying these changes in a major version (such as 7.0.0) helps to signal to the user that updating to the new version might break their code and that action on their part is required. Often, users consult the changelog for major releases, but not so often for patch updates.

In the end, we are all trying to do our work the best we can, and sometimes mistakes happen. Thank you again for your prompt and thoughtful response. We like and appreciate the jasperreports project, and are thankful we can use it to generate PDFs.

Best regards,
Mark

@DManstrator
Copy link

I also want to thank you for the response and the discussion.

A release 6.20.4 with OpenPDF being provided again seems a great solution to me (for now).

About your second item:

In my opinion, making OpenPDF optional again in the next major seems completely fair to me. As you argued multiple times already, Jasper can also work without a pdf export so it seems fair that it will be removed again, this time with notice and within a major update.

The only problem most of us had were that you made that breaking change unnoticed and with a patch release. As already explained by @Viserius, a major update indicates that there may be breaking changes with the release where a patch update is most likely auto-merged by most users.

Another issue regarding OpenPDF is that you use a special version of it (as you have explained in #339 (comment)). This makes it even harder for users since they not only need to add OpenPDF but also an additional repository to be able to use it. I hope you can resolve that in the near future so when you potentially remove it with version 7.0.0, users only need to add OpenPDF in the latest public version.

@soc
Copy link

soc commented Apr 30, 2023

I think the core concern is that the approach to handle the iText/OpenPDF dependency is simply not working out: In fact more than 15% of all bugs ever filed in this repo contain either "itext" or "openpdf".

We have gone from "this uses some custom iText", "this uses iText, but you can substitute OpenPDF", "this uses OpenPDF", "this uses a custom OpenPDF fork, but you can substitute the upstream one" to "this uses a custom OpenPDF fork, and you cannot substitute the upstream one" and every time stuff needlessly broke.

I don't even care about optional vs. non-optional at this point, it's just another bit of churn.

It simply doesn't have to be this way, and I don't know any other project that suffers from this problem this much.

There is a solution that has been pointed out a few times in the past: When you publish a JasperReports version to Maven Central, publish all of it to Maven Central (including your custom OpenPDF if you need it, under the Jasper GroupId).

I don't even understand why/how Maven Central accepts artifact uploads with missing dependencies in the first place, but this doesn't mean it has to be used, especially if it keeps creating problems for everyone.

@teodord
Copy link
Collaborator

teodord commented May 1, 2023

Feel free to write me directly at [email protected] about this topic or any other topic related to this troublesome project.

Thank you,
Teodor

@yu-shiba
Copy link

yu-shiba commented May 1, 2023

Hello to all JasperReports lovers.
I have raised a similar issue in #347.

It is useful to be able to discuss ideas to avoid problems caused by software upgrades.

I hope the dependency on OpenPDF will return in 6.20.4. That way we can at least avoid the possibility that future patch releases will break the build.
And options 2-a and 2-b, I think it would be desirable to have both options. For example, the following configuration

Artifacts Contents
jasperreports same as 6.x (OpenPDF not optional), perhaps BOM only
jasperreports-core common to all dependencies
jasperreports-pdf PDF features only, include OpenPDF dependencies
jasperreports-html HTML features only
jasperreports-office Office features only (.xls)
jasperreports-ooxml Office Open XML feature only (.xlsx)
jasperreports-odf OpenDocument Format features only

I think this configuration can maintain backward compatibility and optimize library dependencies.

@teodord
Copy link
Collaborator

teodord commented May 2, 2023

The release 6.20.4 was published to put back mandatory OpenPDF dependency in pom.xml.

@teodord teodord closed this as completed May 2, 2023
@gourigs123
Copy link

I have been trying to upgrade jasper report version from 6.12.2->6.20.2 but facing session destroyed and please let me know how to add openpdf dependency
is this rite
dependencyResolutionManagement {
versionCatalogs {
commonLibs {
version('jasperreports.version', '6.15.0')
version('itext.version','7.1.16')

				alias('jasperreports-metadata').to('net.sf.jasperreports','jasperreports-metadata').versionRef('jasperreports.version')
				alias('jasperreports-annotation-processors').to('net.sf.jasperreports','jasperreports-annotation-processors').versionRef('jasperreports.version')
				alias('jasperreports-fonts').to('net.sf.jasperreports','jasperreports-fonts').versionRef('jasperreports.version')
				alias('jasperreports').to('net.sf.jasperreports','jasperreports').versionRef('jasperreports.version')
				alias('itext').to('com.itextpdf:itext7-core').versionRef('itext.version')
				}
				
				alias('itext').to('com.itextpdf','itext7-core').versionRef('itext.version')
				alias('openpdf').to('com.github.librepdf','openpdf').versionRef('openpdf.version')
				
				version('itext.version','7.1.16')
				version('openpdf.version','1.3.30')

@gourigs123
Copy link

@soc
Copy link

soc commented Sep 12, 2023

version('jasperreports.version', '6.15.0')

@gourigs123 Why are you using this version, if you want to update to 6.20.2?

@daniel-shuy
Copy link

For others facing this issue, you can exclude openpdf and import openpdf version 1.3.32 (see #398)

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

7 participants