Opened 6 years ago

Last modified 4 years ago

#1104 open defect

jsp processing failure does not abort build

Reported by: zzz Owned by:
Priority: minor Milestone: undecided
Component: other Version: 0.9.8.1
Keywords: build Cc: killyourtv@…
Parent Tickets: Sensitive: no

Description

See e.g. 0.9.8.1-18

Always been like that. Affects all 6 webapps. Leads to surprises if you break something badly.

I've tried to fix it before and failed. See apps/routerconsole/java/build.xml precompilejsp target where I dug out the JspC usage info from somewhere and put it in there. Perhaps another compiler would be better (or a jspc ant task as jrandom's ancient comment says) but that may be a terrible idea. I think the ant task is deprecated?

Maybe there's a newer apache JspC out there, or some combination of options I've overlooked.

Subtickets

Change History (9)

comment:1 Changed 6 years ago by killyourtv

Cc: killyourtv@… added

Example of broken rev for future reference: b324a96172089b549e53fb1c0c2a01807fe64699

comment:2 Changed 6 years ago by killyourtv

The Tomcat 6 jspc instructions page has this text (emphasis mine):

Using Ant is the preferred way to compile web applications using JSPC.

[snip]

When you don't want to stop the jsp generation at first jsp syntax error, use failOnError="false" and with showSuccess="true" all successfull jsp to java generation are printed out.

This looks promising, assuming it works for us and it fails when there's a jsp error. They provide an ant target example using catalina. We don't currently include catalina with our source but assuming this works I presume that adding the extra libs to the source tarball would be acceptable.

-rw-r--r-- 1 54K Apr 29  2013 lib/catalina-ant.jar
-rw-r--r-- 1 73K Apr 29  2013 lib/catalina-deployer.jar

To be tested.

comment:3 Changed 6 years ago by zzz

Our current JspC is from Tomcat 6 in jasper.jar (see apps/jetty/apache-tomcat-deployer/lib). Thru the jetty build.xml those jars get renamed to jasper-compiler.jar, jasper-runtime.jar, commons-el.jar, etc.

So it would not be extra libs but replacements. Or does catalina include tomcat and so it's the same thing? I forget.

Susidns and i2p-bote use some of the "el" tagging lib and they are easy to break if the libs change.

If we do change it should probably be linked with the Jetty 8 port, where we have to change from Tomcat 6 to Tomcat 7 or something else anyway - ticket #1090 .

So atm I'm not thinking it's a good idea to change compilers just to fix this bug.

comment:4 in reply to:  3 Changed 6 years ago by killyourtv

Replying to zzz:

Our current JspC is from Tomcat 6 in jasper.jar (see apps/jetty/apache-tomcat-deployer/lib). Thru the jetty build.xml those jars get renamed to jasper-compiler.jar, jasper-runtime.jar, commons-el.jar, etc.

Indeed.

$ cat apps/jetty/apache-tomcat-deployer/README-i2p.txt 
This is Apache Tomcat 6.x, supporting Servlet 2.5 and JSP 2.1.
The Glassfish JSP 2.1 bundled in Jetty 6 is way too old.

Retrieved from the file
        apache-tomcat-6.0.36-deployer.tar.gz

minus the following files and directores:

build.xml
deployer-howto.html
images/*
lib/catalina*
LICENSE (see ../../../licenses/LICENSE-Apache2.0.txt, it's also inside every jar)
RELEASE-NOTES

So it would not be extra libs but replacements. Or does catalina include tomcat and so it's the same thing? I forget.

(Disclaimer: Everything I've written has been done with zero knowledge of catalina. I never heard of catalina (with Tomcat) until yesterday)

I think they would be extra libs based on the fact that catalina comes with apache-tomcat-6.0.37-deployer.tar.gz but we discard them. TBH, I've not looked at this very thoroughly yet. Some of the above were public notes for myself.

Susidns and i2p-bote use some of the "el" tagging lib and they are easy to break if the libs change.

If we do change it should probably be linked with the Jetty 8 port, where we have to change from Tomcat 6 to Tomcat 7 or something else anyway - ticket #1090 .

So atm I'm not thinking it's a good idea to change compilers just to fix this bug.

When I finally try this I'll be testing the heck out of it locally for awhile (and I won't consider checking it in without discussion). I don't want to be the build breaker. ;)

comment:5 Changed 6 years ago by zzz

note to self, the README is wrong, we are already on 6.0.37

comment:6 in reply to:  5 Changed 6 years ago by killyourtv

Replying to zzz:

note to self, the README is wrong, we are already on 6.0.37

It isn't anymore. I noticed it was outdated but was waiting to check in the fix until I had something else to go with it; I pushed another change at the same time.

comment:7 Changed 4 years ago by str4d

Milestone: 0.9.12

comment:8 Changed 4 years ago by str4d

Status: newopen

comment:9 Changed 4 years ago by zzz

Component: apps/consoleother
Milestone: undecided

component: build system

Note: See TracTickets for help on using tickets.