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

Maven migration, refactored, plus maven build changes since then. #110

Merged
merged 141 commits into from
Dec 31, 2023

Conversation

jkesselm
Copy link
Contributor

@jkesselm jkesselm commented Oct 24, 2023

For history of the previous version of this changeset, see #105. This branch's history was restructured, addressing some of the gripes expressed in that one.

Passing our core regression test suite: smoketest, apitest, and (with known non-regression issues) conf.xsltc. Deeper regression testing would not be a bad thing. I'd suggest particular focus on XSLTC, since we have changed the XPath tokenizer and grammar layers. There may also be other edge cases as a result of moving to current versions of libraries rather than the old ones we were carrying around in the Ant build.

Note that there are dependencies on where the serializer resources are located. If someone really thinks they need to be moved, I suggest opening a Jira ticket; there is no need to resolve that now.

This version has better handling for creating the build/ convenience directory (copy jarfiles to where the ant build put them) for compatibility with xalan-test.

This PR does NOT yet bring tests inboard. That will be dealt with when we mavenize xalan-test.

Note: I think I'm beginning to agree with the folks in the "squash considered harmful" camp. There was a specific reason for using it in previous environment I worked with, but not here. I'll bring that up for discussion on the mailinglist.

@jkesselm
Copy link
Contributor Author

jkesselm commented Nov 3, 2023

In the absense of review:

Given that the executable build appears to be correct, I'm looking at subtler differences betweeen the Maven and Ant build/ output

In the jarfile for the samples

  • Jarfile is renamed from xalansamples.jar to samples.jar. I think that's OK; could be persuaded to change it.
  • Samples are moved into packages. That's deliberate, but their documentation and invocation (if any?) should be updated to reflect that.
  • Maven version of samples.jar includes clases that the Ant build put in separate sample jarfiles. We could consider producing those targeted archives:
  • Ant xsltcapplet.jar: TransformApplet
  • Ant xsltcbrazil;jar: TransformerHandler
  • Ant xsltcejb.jar: TransformBean, TransformHome, TransformRemote, TransformServlet
  • Ant xsltcservlet.jar: CompileServlet, TransformServlet
  • Ant xalanservlet.war: ApplyXSLT, ApplyXSLTException, ApplyXSLTListener, ApplyXSLTProperties, DefaultApplyXSLTProperties, OrderedProps, PIA, SimpleXSLTServlet, UseStylesheetParamServlet, XSLTServletWithParams. This web-archive should also include the media.properties file,, a copy of xalan.jar, and the xml, xsl, and jsp files to run that web archive. Note that the jsp file probably needs to be updated to reflect new servlet/ package.

Should I fix the Maven build to produce these isolated XSLTC sample jar/war files, or is nobody ever really going to care? (Arguably I should produce the .war file, at least, since it needs data files/directory structure not included in the overall samples jarfile.)

@jkesselm
Copy link
Contributor Author

jkesselm commented Nov 3, 2023

Another current difference between the ant and maven builds is how the "distribution" archives are created.

Currently, the ant build generates both zip and .tar.gz versions of xalan-j_version_number-bin.tar and xalan-j_version_number-src.tar.

The Ant source collection puts everything under a top-level xalan-j_version_number/ directory, and pulls xalan-test in as a subdirectory. The intent appears to be to facilitate atomic distribution, build, and test when fetch from git is not practical. Maven instead is currently producing a .sources.jar for each module, and is not including the tests in any of them (though it will do so if/when we migrate xalan-test into xalan_java/test). Is there still a need for a source distribution collection?

The Ant bin collection does not include a copy of the tests, but does bundle xalan and serializer under a xalan-j_version_number/ directory, and includes documentation and SOURCE of the samples -- in other words, it's basically the build output minus compilation of the samples. It does not bundle the tests. Maven is not currently producing an equivalent. Is there still a need for this?

@jkesselm jkesselm self-assigned this Nov 3, 2023
@jkesselm
Copy link
Contributor Author

jkesselm commented Nov 8, 2023

Maven build now copies *-sources.jar into top level build/, emulating ant build's gathering of (unified) xalan*-src.zip and xalan*-src.tar.gz.

Other differences noted above persist: tests are not folded into this set of sources, single samples.jar (rename xalansamples?), no xalanservlet.war.

Rechecking jarfile contents to make sure everything still matches outside of those quibbles. Still seeking consensus from the rest of Mission Control.

@jkesselm
Copy link
Contributor Author

  • Factored out samples which should produce their own jar or war files.
  • Doing final(ish) jar/war/tar content checks.
  • Then will need to reconcile for merge.

CAVEAT: Sample classes originally were in the default (ie, no) package. I've taken the liberty of putting at least some of them under a package reflecting their intent -- general samples, xsltcservlet. There may be outdated references in the sample doc, or html, or jsp files which I haven't reconciled yet. Given how long it has been since anyone looked at these samples, that may not be an issue... but it should be reviewed, and arguably the samples should all be regression-tested (I doubt any of them are).

@jkesselm
Copy link
Contributor Author

Content lists of all Maven build .jar and .war files confirmed as matching Ant equivalents, modulo java packages of samples, how version is handled, and the now-shaded (and newer) java_cup/runtime package.

Todo:

  1. Re-recheck tar/zip file content to make sure nothing shook loose.
  2. Then reconcile for merge.

…replicated inside the /samples/ directory, and the sample javadocs are now included in /docs/apidocs/. There's some other cruft (most of /docs/images/, /docs/xsltc/ design) which should be rewritten or discarded at some point. NOTE: We still need to update the docs (site and local) to reflect the Maven build, and its description of test is badly outdated.
@jkesselm
Copy link
Contributor Author

jkesselm commented Dec 29, 2023

Bin distro looks acceptable though not ideal.

Deferred cleanup, should be added as Jira tasks:

  • The samples' javadocs are now included in /docs/apidocs/. May be fine.
  • Dependencies are being replicated inside the /samples/ directory. Inefficient, should be curable but my first attempt to exclude them didn't work.
  • There's some cruft (most of /docs/images/, /docs/xsltc/ design which may predate its folding into Xalan) which wasn't in the ant build and may be obsolete
  • Is the /local/ documentation directory needed? I submit not...
  • MORE IMPORTANT: We still need to update the docs to reflect the Maven build, and their description of test is badly outdated; grep for build.sh, build.bat, build.xml. I've done a very quick edit to replace some mentions with mvnbuild and pom.xml, but more is needed.
  • Tests should be modified to be able to use jarfiles that have version numbers (probably just path/*?), at which point we will be able to drop the replication of those into unversioned filenames.

@jkesselm
Copy link
Contributor Author

jkesselm commented Dec 29, 2023

I'm now getting a reasonable-looking xalan-java src tarball. I've confirmed that I can build from it. Rah!

In the past, the src tarball/zipfile has included an embedded copy of xalan-test, for the convenience of anyone downloading source who wanted to confirm that their build had succeeded. (The binary distro did not include xalan-test.)

Should the maven src distro:

  • Include the embedded xalan-test, despite that still being an ant build?
  • Make xalan-test produce its own source distro?
  • Something else?

(I'd rather not recreate the embedding of xalan-test's source, but it wouldn't be impossible, just ugly.)

@jkesselm
Copy link
Contributor Author

Reconciled with Master.

@jkesselm jkesselm merged commit 353a329 into master Dec 31, 2023
2 checks passed
@jkesselm jkesselm deleted the xalan-java-mvn-refactored branch December 31, 2023 14:49
@jkesselm
Copy link
Contributor Author

Happy New Year.

Have fun. Fix bugs. Support users. Clean code. Add features. Mostly in that order... ;-)

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

Successfully merging this pull request may close these issues.

4 participants