Copy latest changes from _maint into master

[jkesselm]

Arguably the PRs should have been against master in the first place, but it's stuff that isn't unreasonable to apply to 2.7.1 maintenance, I think.

[stanio]

If you use "Rebase and merge" option from the web UI, the extra merge commits will be excluded from master. Effectively it will produce cherry picks of the three original commits onto master. There won't be an extra merge commit for this PR, as well.

[stanio]

xalan-tests-create-for-src-distro:
    [mkdir] Created dir: /home/runner/work/xalan-java/xalan-java/build/xalan-j_2_7_3/xalan-test

BUILD FAILED
/home/runner/work/xalan-java/xalan-java/build.xml:1470: The following error occurred while executing this line:
/home/runner/work/xalan-java/xalan-java/build.xml:1382: The following error occurred while executing this line:
/home/runner/work/xalan-java/xalan-java/build.xml:1399: /home/runner/work/xalan-java/xalan-test does not exist.

Does it appear a problem in the build.xml script, or in the CI configuration?

[jkesselm]

Yeah, I've been getting gripes from the CI engine. @vlsi , do you have a fix, or should we back this out until there's at least a working stub?

[vlsi]

I've been getting gripes from the CI engine
do you have a fix or should we back this out until there's at least a working stub?

I do not think the failure is CI-related, and it is probably caused by bugs in xalan-java or in xalan-test code.

Frankly speaking, I have no idea how xalan-tests should be launched (it is obscure and probably undocumented), and I expect at least someone from xalan-java should know the way to launch them.

Here's the command that launches tests, please compare it with the one you are using to test xalan-java:
https://github.com/apache/xalan-java/pull/9/files#diff-7829468e86c1cc5d5133195b5cb48e1ff6c75e3e9203777f6b2e379d9e4882b3R40-R51

I guess you would need to update it if you intend to migrate to Maven.

[stanio]

This looks like checking out xalan-test as a subdirectory of xalan-java, while the build script is expecting it to be a sibling directory:

xalan-java/build.xml

Line 1400 in acdb899

<fileset dir="${test.relpath}">

xalan-java/build.xml

Line 1534 in acdb899

<property name="test.relpath" value="../xalan-test"/>

Either the xalan-test checkout/working directory should be adjusted or the build command at the end:

ant fulldist -Dtest.relpath=xalan-test

[stanio]

The problem is seen already after merging #9 into xalan-j_2_7_1_maint. I suggest it should be fixed there first. This PR would get updated automatically then.

[vlsi]

AFAIK PRs can't me modified after merging

[stanio]

I've suggested/meant fixing it on the xalan-j_2_7_1_maint branch by means of additional PR/commit. After merging/applying it there – it would appear in this PR as well.

[jkesselm]

If the tests work fine when we invoke them as per our usual process, and don't work fine under CI, then CI is invoking them incorrectly. Yes, I am going to propose moving the tests into xalan-java so it's a more standard maven package and the default CI test setup should then work, but that's not yet on the table. We should either make CI replicate our current development environment well enough to run the regression tests as they are designed to be invoked, or turn off CI again until that can be done.

During development, xalan-test is currently a separate project cloned as a sibling of xalan-java.
In our old distribution jars, xalan-test was shipped as a subdirectory of xalan-java.
Our development test drivers set up paths which should allow tests to be executed from either location.

I'd need to look at exactly what CI is doing, but @stanio 's analysis sounds entirely plausible.

And, yes, this gets fixed by a new PR, just like any other fix of code after it has been checked in. Nothing unusual about it, other than that we ideally should have tested better before accepting it for merge. Bugs happen. Bugs get fixed. Business as usual.

[stanio]

I haven't been able to make it check out xala-java and xalan-test as sibling directories

I've managed it, but I'm a bit puzzled about some details: stanio/xalan-java@5b21fa28619a19bfb0b66a19f79c4b8aa24bece8. Not sure if this is better than adjusting just the final build command.

[stanio]

I've managed it, but I'm a bit puzzled about some details

O.k. On the xalan-j_2_7_x branch the configuration differs from master:

    <property name="xalan.relpath" value="../java"/>

[vlsi]

@stanio, @jkesselm, if you feel confident, please fix it.
I provided the major part of the CI scripts, and I expect adjusting the command should be trivial for committers/PMCs as I assume they use tests. I do not want to spend my time on reverse-engineering scripts that will be deleted soon (both ant and xalan-test)

[jkesselm]

Fix applied directly to the branch, since I was iterating in GitHub to test the changes (and to figure out what it's test environment would let me do -- couldn't check out in parallel, which fulldist apparently currently requires, but could check out and then move to that location.)

The smoketest-after-build instructions are in the xalan-java README, y'know. The only real issue with those instructions is that the xsltc.conf target, unlike the interpretive, doesn't currently have a "report only regressions" equivalent -- so for it specific failures are "correct" and that isn't compatible with the CI framework. For now I've simply dropped those tests from CI; we can reintroduce them, and perhaps expand the smoketest suite in other ways, as that is addressed and/or known divergences from the spec are resolved.

Merge up to master should now be safe.

[jkesselm]

This looks like checking out xalan-test as a subdirectory of xalan-java, while the build script is expecting it to be a sibling directory:

Actually, tests can be run from either location. But fulldist wants it as a sibling unless the parameter is used to specify otherwise.

Unfortunately the CI system didn't like me checking out into Xalan's parent directory. But it was willing to let me check out locally and then move the tests up to a sibling position.

Since I didn't discover the fulldist requirement until after I'd gotten past test, the yaml script currently checks out xalan-test as a child, runs the tests from there, then moves it up to sibling before running fulldist. This may not be the most self-evident approach, and the yaml script now has a comment explaining this.

Yes, setting test.relpath was probably at least as clean. But I wanted fulldist to replicate the environment we use for our own production builds.

And yes, we could try the mv workaround earlier rather than having test in different places at different times.

"Make it work, make it good, make it great. It now demonstrably works. Clean up can be left as technical debt, though I'd give it low priority since this will change somewhat if test is brought into the main Xalan tree.

[jkesselm]

I guess you would need to update it if you intend to migrate to Maven.

Actually, the Maven build is supposed to be producing an additional xalan-java/build directory, for back-compatibility with the new existing xalan-test drivers, so we can defer alterations until we are ready to move the tests under xalan-java so we're more Maven-standard.

[jkesselm]

Cherry-picked rather than merging, since I wanted to think about each change individually. But everything should have been moved over, with histories maintained.