Bug 230325 - www/tomcat{85,9}: daemon.sh incorrectly refers to jsvc executable
Summary: www/tomcat{85,9}: daemon.sh incorrectly refers to jsvc executable
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-ports-bugs mailing list
URL:
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2018-08-03 12:53 UTC by Michael Osipov
Modified: 2019-01-27 17:02 UTC (History)
2 users (show)

See Also:
vvd: maintainer-feedback+
koobs: merge-quarterly?


Attachments
1st draft (1.37 KB, patch)
2018-11-28 17:38 UTC, VVD
no flags Details | Diff
Patch for tomcat9 (1.85 KB, patch)
2018-12-03 11:48 UTC, VVD
vvd: maintainer-approval+
Details | Diff
Patch for tomcat85 (1.89 KB, patch)
2018-12-03 12:33 UTC, VVD
vvd: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Osipov 2018-08-03 12:53:15 UTC
While the daemon.sh is not directly used by the port, uses might still use it for development, thus the scripts needs to be fixed.

Commons Daemon is a runtime dependency so the spot in daemon.sh:

> # If not explicitly set, look for jsvc in CATALINA_BASE first then CATALINA_HOME
> if [ -z "$JSVC" ]; then
>     JSVC="$CATALINA_BASE/bin/jsvc"
>     if [ ! -x "$JSVC" ]; then
>         JSVC="$CATALINA_HOME/bin/jsvc"
>     fi
> fi

Shall be replaced with

> JSVC=%%PREFIX%%/bin/jsvc

or similar.
Comment 1 Michael Osipov 2018-08-03 12:54:42 UTC
This likely applies to 7.0 and 9.0 too.
Comment 2 VVD 2018-11-28 03:28:55 UTC
I don't know how to do this without hard-coded /usr/local.
Comment 3 Michael Osipov 2018-11-28 08:48:48 UTC
(In reply to VVD from comment #2)

There is no need to, you can simple patch the file with JSVC=%%PREFIX%%/bin/jsvc and then perform substition. Look what I do with Nexus OSS: https://github.com/michael-o/freebsd-ports/blob/master/devel/nexus2-oss/Makefile#L45 and https://github.com/michael-o/freebsd-ports/blob/master/devel/nexus2-oss/files/patch-conf_wrapper.conf#L20. I think this is perfectly fine.

Also make sure to replace line 138:
CLASSPATH="$CLASSPATH$CATALINA_HOME/bin/bootstrap.jar:$CATALINA_HOME/bin/commons-daemon.jar" too with ${JAVAJARDIR}/commons-daemon.jar. See https://svnweb.freebsd.org/ports/head/devel/jakarta-commons-daemon/Makefile?view=markup#l33

If you need further assistance, let me know.
Comment 4 VVD 2018-11-28 17:03:15 UTC
So, I can use %%PREFIX%% in patches. Ok, I'll try.
Comment 5 VVD 2018-11-28 17:38:20 UTC
Created attachment 199644 [details]
1st draft

Check, plz, this one - is it what you ask?
Comment 6 Michael Osipov 2018-12-03 09:26:32 UTC
(In reply to VVD from comment #5)

The patch looks good to me, but is incomplete. Please look at this line: https://github.com/apache/tomcat/blob/trunk/bin/daemon.sh#L140

The last path component must be replaced with %JAVAJARDIR%/commons-daemon.jar.

See:

# make -V JAVAJARDIR
/usr/local/share/java/classes

# ll /usr/local/share/java/classes/
antlr-3.5.2-complete.jar  jline-0.9.94.jar          ojdbc-11.2.0.4.jar
commons-daemon.jar        jline.jar                 swt-devel.jar
hamcrest.jar              junit.jar
hamcrest1.3.jar           junit4.jar
Comment 7 VVD 2018-12-03 11:48:24 UTC
Created attachment 199793 [details]
Patch for tomcat9

Check this, plz.
Comment 8 Michael Osipov 2018-12-03 12:01:26 UTC
(In reply to VVD from comment #7)

Looks very good to me.
Comment 9 Kubilay Kocak freebsd_committer freebsd_triage 2018-12-03 12:25:12 UTC
Summary contains www/tomcat85, patch is for www/tomcat9.

Please clarify which (all?) tomcat ports require the update, and update the summary and patches accordingly
Comment 10 Michael Osipov 2018-12-03 12:26:43 UTC
The change applies in general to Tomcat 7/8.5/9 while the patch applies to Tomcat 9 only here. It require slight modifications for 8.5 (and 7 likely).
Comment 11 VVD 2018-12-03 12:33:07 UTC
Created attachment 199796 [details]
Patch for tomcat85
Comment 12 Michael Osipov 2018-12-03 13:48:19 UTC
(In reply to VVD from comment #11)

Applies cleanly, works for me.
Comment 13 VVD 2018-12-20 23:25:24 UTC
Ping!

I have to update tomcat85 and tomcat9 to next versions, but can't - I need this patch commited before.
Comment 14 Kubilay Kocak freebsd_committer freebsd_triage 2018-12-21 02:05:04 UTC
Can someone provide clarity on tomcat7 being affected, and provide a patch for that version too please (set maintainer-approval ? ale@, now CC'd, for that patch, as he is maintainer of tomcat7)
Comment 15 Michael Osipov 2018-12-21 11:29:33 UTC
(In reply to Kubilay Kocak from comment #14)

As this ticket is labeled for Tomcat 8.5 and 9, I really like to spin another ticket for 7 for those who care and continue with this one so that the maintainer can update Tomcat 8.5 and 9.
Comment 16 Kubilay Kocak freebsd_committer freebsd_triage 2018-12-22 00:35:49 UTC
(In reply to Michael Osipov from comment #15)

I understand the sentiment. Note also that this issue was originally only titled tomcat85 and expanded to include tomcat9 only later.

There is nothing preventing the existing patches from being committed, but the issue should be resolved completely, in any ports that are affected by it, which was the basis for comment 14.

It's also beneficial in that the maintainer of tomcat7 is a committer, more relevantly appropriate to address the issue across all tomcat ports than someone not familiar with them.
Comment 17 Alex Dupre freebsd_committer 2018-12-28 14:57:42 UTC
I don't think it worth the effort to patch the tomcat ports for a file that is not used by the port.
You can create the setenv.sh file and add the following line to have the same effect:

JSVC=/usr/local/bin/jsvc
Comment 18 Michael Osipov 2019-01-02 18:16:26 UTC
(In reply to Alex Dupre from comment #17)

I do not agree while I understand your argument. Supplied content should always follow POLA. Given that people might waste 15 to 30 min to understand the issue we can easily fix this upfront. That is what a port is for.
Comment 19 VVD 2019-01-03 12:41:56 UTC
tomcat6/7/8 are Alex Dupre's ports and he can do what he want.
But tomcat85 and 9 are "mine" and I want to add this patch. Who can commit it?
Comment 20 Michael Osipov 2019-01-03 12:55:46 UTC
(In reply to Kubilay Kocak from comment #16)

To apply this patch to Tomcat 7 the previous changes made to 85 and 9 have to replicated first. The Makefiles should be minimally different. The current maintainer is ale@. He should be able to do this. As soon as he has done that I can change this patch for tomcat7, but I don't want this issue to be blocked just because ale@ needs time to address those changes first.

PS: I am an official Tomcat committer, though don't have any commit permission on the ports.
Comment 21 Michael Osipov 2019-01-03 12:56:34 UTC
(In reply to VVD from comment #19)

Correct, and given that 6 and 8 are deprecated, even less stuff to be maintained. I have requested their removal.
Comment 22 commit-hook freebsd_committer 2019-01-27 15:21:59 UTC
A commit references this bug:

Author: swills
Date: Sun Jan 27 15:21:17 UTC 2019
New revision: 491357
URL: https://svnweb.freebsd.org/changeset/ports/491357

Log:
  www/tomcat{85,9}: fix daemon.sh reference to jsvc

  PR:		230325
  Submitted by:	VVD <vvd@unislabs.com> (maintainer)
  Reported by:	Michael Osipov <michael.osipov@siemens.com>

Changes:
  head/www/tomcat85/Makefile
  head/www/tomcat85/files/patch-bin__daemon.sh
  head/www/tomcat9/Makefile
  head/www/tomcat9/files/patch-bin__daemon.sh
Comment 23 Steve Wills freebsd_committer 2019-01-27 15:22:34 UTC
Committed, thanks!
Comment 24 VVD 2019-01-27 17:02:26 UTC
Thanks!

Update tomcat9 to 9.0.14:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=235249

Update tomcat85 to 8.5.37:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=235250