If you have multiple Java JDKs or JREs installed, and you install a port that uses devel/apache-ant to compile java sources, you may well end up using a different JVM version to compile than would be indicated by the USE_JAVA variable in the port Makefile. While bsd.java.mk uses it's own JAVA_HOME make variable to provide an explicit path to any java-related binaries, ant uses a shell script wrapper which can't see the make variables, instead using the value of JAVA_HOME from the environment to generate the full path to one of the possible java binaries. Fix: All the ports that use ant do so in much the same way. Factor out the common code, and provide a 'USE_ANT' variable to enable a slightly modified version of it that propagates ${JAVA_HOME} into the ant environment. How-To-Repeat: This Makefile illustrates the problem. Try running it with various values of JAVA_HOME in your environment (or unset JAVA_HOME), plus try various USE_JAVA settings: % env JAVA_HOME=/usr/local/jdk1.4.2 make USE_JAVA=1.3 --- Makefile.shar begins here --- # This is a shell archive. Save it in a file, remove anything before # this line, and then unpack it by entering "sh file". Note, it may # create directories; files and directories will be owned by you and # have default permissions. # # This archive contains: # # Makefile # echo x - Makefile sed 's/^X//' >Makefile << 'END-of-Makefile' X XPORTNAME= foo XPORTVERSION= 0.0 XCATEGORIES= none XCOMMENT= No comment X XUSE_JAVA= 1.3+ XNEED_JAVAC= YES X XJAVA_ENV= JAVA_HOME=${JAVA_HOME} X Xall: good bad X Xcommon: X @${ECHO} '$${USE_JAVA} = ${USE_JAVA}' X @${ECHO} '$${JAVA_HOME} = ${JAVA_HOME}' X Xbad: common X @${ECHO} "Bah! Humbug! -----------------------------------------" X @${ECHO} "------------------------------------------------------" X ant -diagnostics X @${ECHO} "------------------------------------------------------" X Xgood: common X @${ECHO} "As it should be --------------------------------------" X @${ECHO} "------------------------------------------------------" X ${SETENV} ${JAVA_ENV} ant -diagnostics X @${ECHO} "------------------------------------------------------" X X.include <bsd.port.mk> END-of-Makefile exit --- Makefile.shar ends here ---
Responsible Changed From-To: freebsd-ports-bugs->znerd Over to maintainer.
Matthew, I already discussed such an issue with Ernst (Ant support in bsd.java.mk) and even provided some patch. But I failed at providing a simple solution to start with. I personally like your minimalist approach. Currently, there are 29 ports that use Ant at build stage. Amongst them, 21 ports define a ANT variable that represents the same as in your patch, that is the command line to exectue Ant (but with no prior setting of JAVA_HOME). Both lists are included at the end of this message. I am not the maintainer of bsd.java.mk but, as you are aware of, I am contributing to the current effort to implement a new JDK dependency checking system (more stable, flexible...) in version 2.0. IMHO, the problem you report here is clearly in the scope of this effort, and so I vote +1 for the merge of your patch in bsd.java.mk 2.0 (with a slight modification though, that I will explain further in this message). If the maintainer (Ernst) agrees with this (he is also maintaining devel/apache-ant BTW), I will make a new release for bsd.java.mk 2.0 with support for Ant. Anyway, as you are aware of, it will not change anything to the current behaviour of ports. Indeed, some additional work will be required right after the commit of bsd.java.mk 2.0 to have existing ports actually use this feature. But the fact that most of the ports already use a ANT variable will hopefully speed up things a little. Therefore, I think this PR should not be closed until: - bsd.java.mk 2.0 gets commited - existing ports that use Apache Ant are upgraded - a statement regarding USE_ANT is included in the porters's handbook - portlint warns when Ant is not used the right way Regards, Herve * The slight modification I spoke of: if (${USE_ANT} == "YES") || (${USE_ANT} == "yes") IMHO, this test is not necessary as I cannot think of any case where a port would use "NO" as a value for USE_ANT. So I think bsd.java.mk should only test if USE_ANT is actually defined. Anyway, as you may already know, I like splitting hair so that's probably not so important. * List of ports that have Apache-Ant (devel/apache-ant) amongst their dependencies: - databases/erserver - databases/mysql-connector-java - databases/postgresql-jdbc - devel/colorer - editors/openoffice-1.0 - editors/openoffice-1.1 - editors/openoffice-2.0-devel - java/eclipse - java/jakarta-commons-beanutils - java/jakarta-commons-cli - java/jakarta-commons-collections - java/jakarta-commons-dbcp - java/jakarta-commons-lang - java/jakarta-commons-logging - java/jakarta-commons-pool - java/jakarta-oro - java/jakarta-regexp - java/java-checkstyle - java/jdom - java/jfreechart - java/jump - java/xdoclet - net/spread-j - textproc/ant-xinclude-task - textproc/xerces-j - textproc/xincluder - textproc/xmlenc - textproc/xalan-j - www/jetspeed (29 ports) * List of ports that define ${ANT}: - databases/mysql-connector-java - devel/colorer - editors/openoffice-1.0 - editors/openoffice-1.1 - editors/openoffice-2.0-devel - java/jakarta-commons-beanutils - java/jakarta-commons-cli - java/jakarta-commons-collections - java/jakarta-commons-dbcp - java/jakarta-commons-lang - java/jakarta-commons-logging - java/jakarta-commons-pool - java/jdom - java/jfreechart - java/jump - net/spread-j - textproc/ant-xinclude-task - textproc/xerces-j - textproc/xincluder - textproc/xmlenc - textproc/xalan-j (21 ports)
[Here is a mail I received from Matthew (originator of this PR) and that I forward with permission] Herve, On Mon, Mar 01, 2004 at 07:57:43PM +0100, Herve Quiroz wrote: > I am not the maintainer of bsd.java.mk but, as you are aware of, I am > contributing to the current effort to implement a new JDK dependency checking > system (more stable, flexible...) in version 2.0. IMHO, the problem you report > here is clearly in the scope of this effort, and so I vote +1 for the merge > of your patch in bsd.java.mk 2.0 (with a slight modification though, that I > will explain further in this message). If the maintainer (Ernst) agrees with > this (he is also maintaining devel/apache-ant BTW), I will make a new release > for bsd.java.mk 2.0 with support for Ant. Excellent. Thank you very much indeed. > * The slight modification I spoke of: > > if (${USE_ANT} == "YES") || (${USE_ANT} == "yes") > > IMHO, this test is not necessary as I cannot think of any case where a port > would use "NO" as a value for USE_ANT. So I think bsd.java.mk should only test > if USE_ANT is actually defined. Anyway, as you may already know, I like > splitting hair so that's probably not so important. Yes -- I was in two minds about putting that in myself. I think I decided to on the basis that a similar construct was used elsewhere in bsd.java.mk, but your modification will fit the bill just as well. Cheers, Matthew -- Dr Matthew J Seaman MA, D.Phil. 26 The Paddocks Savill Way PGP: http://www.infracaninophile.co.uk/pgpkey Marlow Tel: +44 1628 476614 Bucks., SL7 1TH UK
Responsible Changed From-To: znerd->glewis I'll take this as I'm committing most of the changes to bsd.java.mk these days.
Hmmm, I wonder if we shouldn't go a step further and hijack do-build: if USE_ANT is set. Here is a preliminary patch. Hopefully this won't get too mangled. I've tested it briefly and it seems to do what I intend. There are still a few issues: 1. I'm not sure I have the interaction between USE_ANT and JAVA_BUILD settings worked out correctly. 2. Documentation both in bsd.java.mk in comments and in the porter's handbook is missing. 3. I'm not 100% sure about the variable names (maybe we should just use ALL_TARGET instead of inventing our own thing, similarly MAKE_ENV, etc.). See the do-build in bsd.port.mk for some examples. Anyway, with this in place you should be able to do: USE_ANT= yes ANT_TARGET= target and have the port build correctly without having to define your own do-build: target in the port or any other related variables. Index: bsd.java.mk =================================================================== RCS file: /home/pcvs/ports/Mk/bsd.java.mk,v retrieving revision 1.50 diff -u -r1.50 bsd.java.mk --- bsd.java.mk 27 Jul 2004 05:16:34 -0000 1.50 +++ bsd.java.mk 27 Jul 2004 05:39:56 -0000 @@ -394,7 +394,7 @@ . if defined(JAVA_EXTRACT) EXTRACT_DEPENDS+= ${DEPEND_JAVA} . endif -. if defined(JAVA_BUILD) +. if defined(JAVA_BUILD) || defined(USE_ANT) . if defined(NO_BUILD) check-makevars:: @${ECHO_CMD} "${PKGNAME}: Makefile error: JAVA_BUILD and NO_BUILD cannot be set at the same time."; @@ -406,6 +406,14 @@ RUN_DEPENDS+= ${DEPEND_JAVA} . endif +. if defined(USE_ANT) +ANT?= ${LOCALBASE}/bin/ant +ANT_ENV+= JAVA_HOME=${JAVA_HOME} +BUILD_DEPENDS+= ${ANT}:${PORTSDIR}/devel/apache-ant +do-build: + @(cd ${BUILD_WRKSRC}; \ + ${SETENV} ${ANT_ENV} ${ANT} ${ANT_FLAGS} ${ANT_TARGET}) +. endif #----------------------------------------------------------------------------- # Stage 7: Define all settings for the port to use -- Greg Lewis Email : glewis@eyesbeyond.com Eyes Beyond Web : http://www.eyesbeyond.com Information Technology FreeBSD : glewis@FreeBSD.org
Greg, On Mon, Jul 26, 2004 at 11:48:30PM -0600, Greg Lewis wrote: > 1. I'm not sure I have the interaction between USE_ANT and JAVA_BUILD > settings worked out correctly. I think there are many ways of dealing with this issue. So far, here are the questions I am asking myself in order to find a solution: - Does USE_ANT implicitely means JAVA_BUILD ? - If not, what should be the behaviour of bsd.java.mk when JAVA_BUILD and USE_ANT are set ? IMHO, USE_ANT means that Ant will be required to build the port. So USE_ANT should mean JAVA_BUILD=jdk and thus the first hunk in your patch is no longer needed. > 2. Documentation both in bsd.java.mk in comments and in the porter's > handbook is missing. I can handle that if you want. > 3. I'm not 100% sure about the variable names (maybe we should just use > ALL_TARGET instead of inventing our own thing, similarly MAKE_ENV, > etc.). See the do-build in bsd.port.mk for some examples. ALL_TARGET could indeed be used with Ant. The main difference is that there is no 'all' default target with Ant. So when USE_ANT is set, ALL_TARGETS should probably be set to an empty string by bsd.java.mk so that it would run the default target (build.xml:/project/@default) when nothing is specified by the port (assuming build.xml actually has a specified default target). > +. if defined(USE_ANT) > +ANT?= ${LOCALBASE}/bin/ant > +ANT_ENV+= JAVA_HOME=${JAVA_HOME} > +BUILD_DEPENDS+= ${ANT}:${PORTSDIR}/devel/apache-ant > +do-build: > + @(cd ${BUILD_WRKSRC}; \ > + ${SETENV} ${ANT_ENV} ${ANT} ${ANT_FLAGS} ${ANT_TARGET}) > +. endif If you want to mimic a bit more the way GMAKE is handled, I think we should also test whether the 'do-build' target is already defined or not, so that Java porters get Ant support but still are able to customize the port. e.g.: .if defined(USE_ANT) && !target(do-build) [...] .endif BTW, do you think that ANT_INCLUDE_SHARED_JARS should be supported in bsd.java.mk ? There are two ways of dealing with that: - The porter manually adds ANT_INCLUDE_SHARED_JARS=yes to ANT_ENV - The porters defines ANT_INCLUDE_SHARED_JARS in the Makefile and bsd.java.mk automatically adds ANT_INCLUDE_SHARED_JARS=yes to ANT_ENV. By saying that, I must add that I am not personally in favor of either method... so it's probably better to leave it as-is for now (that is, porters will have to do so manually). Overall, I have to say that I think this would be a great addition to bsd.java.mk. This could save Java porters and commiters a lot of time and effort. Furthermore, it will probably also unify the way Java ports are implemented, which is a good thing IMHO. Herve
Hi Herve, On Thu, Jul 29, 2004 at 01:31:58PM +0200, Herve Quiroz wrote: > Greg, > > On Mon, Jul 26, 2004 at 11:48:30PM -0600, Greg Lewis wrote: > > 1. I'm not sure I have the interaction between USE_ANT and JAVA_BUILD > > settings worked out correctly. > > I think there are many ways of dealing with this issue. So far, here are > the questions I am asking myself in order to find a solution: > > - Does USE_ANT implicitely means JAVA_BUILD ? > - If not, what should be the behaviour of bsd.java.mk when JAVA_BUILD > and USE_ANT are set ? > > IMHO, USE_ANT means that Ant will be required to build the port. So > USE_ANT should mean JAVA_BUILD=jdk and thus the first hunk in your patch > is no longer needed. Agreed. I've replaced it with a section that automatically sets JAVA_BUILD if USE_ANT is set. I think that this is what is wanted? > > 2. Documentation both in bsd.java.mk in comments and in the porter's > > handbook is missing. > > I can handle that if you want. Your offer is appreciated :). Let wait until we have the final version ready to go though. > > 3. I'm not 100% sure about the variable names (maybe we should just use > > ALL_TARGET instead of inventing our own thing, similarly MAKE_ENV, > > etc.). See the do-build in bsd.port.mk for some examples. > > ALL_TARGET could indeed be used with Ant. The main difference is that > there is no 'all' default target with Ant. So when USE_ANT is set, > ALL_TARGETS should probably be set to an empty string by bsd.java.mk so > that it would run the default target (build.xml:/project/@default) when > nothing is specified by the port (assuming build.xml actually has a > specified default target). Thanks, its been a while since I used ant very much. I've updated the patch to set ALL_TARGET as blank if it hasn't been set in the port. > > +. if defined(USE_ANT) > > +ANT?= ${LOCALBASE}/bin/ant > > +ANT_ENV+= JAVA_HOME=${JAVA_HOME} > > +BUILD_DEPENDS+= ${ANT}:${PORTSDIR}/devel/apache-ant > > +do-build: > > + @(cd ${BUILD_WRKSRC}; \ > > + ${SETENV} ${ANT_ENV} ${ANT} ${ANT_FLAGS} ${ANT_TARGET}) > > +. endif > > If you want to mimic a bit more the way GMAKE is handled, I think we > should also test whether the 'do-build' target is already defined or > not, so that Java porters get Ant support but still are able to > customize the port. e.g.: > > .if defined(USE_ANT) && !target(do-build) > [...] > .endif Good idea. I've incorporated that too. > BTW, do you think that ANT_INCLUDE_SHARED_JARS should be supported in > bsd.java.mk ? There are two ways of dealing with that: > > - The porter manually adds ANT_INCLUDE_SHARED_JARS=yes to ANT_ENV > - The porters defines ANT_INCLUDE_SHARED_JARS in the Makefile and > bsd.java.mk automatically adds ANT_INCLUDE_SHARED_JARS=yes to ANT_ENV. > > By saying that, I must add that I am not personally in favor of either > method... so it's probably better to leave it as-is for now (that is, > porters will have to do so manually). Agreed. > Overall, I have to say that I think this would be a great addition to > bsd.java.mk. This could save Java porters and commiters a lot of time > and effort. Furthermore, it will probably also unify the way Java ports > are implemented, which is a good thing IMHO. Here is the updated patch. Let me know what you think. I think its pretty close to being ready to commit. Index: bsd.java.mk =================================================================== RCS file: /var/fcvs/ports/Mk/bsd.java.mk,v retrieving revision 1.50 diff -u -r1.50 bsd.java.mk --- bsd.java.mk 27 Jul 2004 05:16:34 -0000 1.50 +++ bsd.java.mk 30 Jul 2004 18:05:31 -0000 @@ -262,6 +262,9 @@ @${FALSE} . endif . endif +. if defined(USE_ANT) +JAVA_BUILD= jdk +. endif . endif @@ -406,6 +409,17 @@ RUN_DEPENDS+= ${DEPEND_JAVA} . endif +. if defined(USE_ANT) +ANT?= ${LOCALBASE}/bin/ant +MAKE_ENV+= JAVA_HOME=${JAVA_HOME} +BUILD_DEPENDS+= ${ANT}:${PORTSDIR}/devel/apache-ant +ALL_TARGET?= +. if !target(do-build) +do-build: + @(cd ${BUILD_WRKSRC}; \ + ${SETENV} ${MAKE_ENV} ${ANT} ${MAKE_ARGS} ${ALL_TARGET}) +. endif +. endif #----------------------------------------------------------------------------- # Stage 7: Define all settings for the port to use -- Greg Lewis Email : glewis@eyesbeyond.com Eyes Beyond Web : http://www.eyesbeyond.com Information Technology FreeBSD : glewis@FreeBSD.org
Hi Greg, On Fri, Jul 30, 2004 at 12:19:51PM -0600, Greg Lewis wrote: > > IMHO, USE_ANT means that Ant will be required to build the port. So > > USE_ANT should mean JAVA_BUILD=jdk and thus the first hunk in your patch > > is no longer needed. > > Agreed. I've replaced it with a section that automatically sets JAVA_BUILD > if USE_ANT is set. I think that this is what is wanted? I noticed a little problem with your latest patch. Indeed, JAVA_BUILD=jdk is set when USE_ANT is defined but only when using bsd.java.mk 1.0 backward compatibility mode as the conditional statement comes nested in some '.if (${_USE_BSD_JAVA_MK_1_0} == "yes")' statement. Here is a patch with the suggested modification as well as the documentation in the header of the file: --- begin of bsd.java.mk.diff --- --- bsd.java.mk.orig Tue Jul 27 07:16:34 2004 +++ bsd.java.mk Sun Aug 1 10:53:28 2004 @@ -41,6 +41,13 @@ # # USE_JIKES Whether the port should or should not use jikes(1) to build. # +# USE_ANT Should be defined when the port uses Apache Ant. Ant is thus +# considered to be the sub-make command. When no 'do-build' +# target is defined by the port, a default one will be set +# that simply runs Ant according to MAKE_ENV, MAKE_ARGS and +# ALL_TARGETS. Read the documentation in bsd.port.mk for more +# information. +# #------------------------------------------------------------------------------- # Variables defined for the port: # @@ -381,6 +388,11 @@ # Stage 6: Add any dependencies if necessary # +# Ant Support: USE_ANT --> JAVA_BUILD=jdk +. if defined(USE_ANT) +JAVA_BUILD= jdk +. endif + # Add the JDK port to the dependencies DEPEND_JAVA= ${JAVA}:${PORTSDIR}/${JAVA_PORT} # When nothing is set, assume JAVA_BUILD=jdk and JAVA_RUN=jre @@ -406,6 +418,18 @@ RUN_DEPENDS+= ${DEPEND_JAVA} . endif +# Ant support: default do-build target +. if defined(USE_ANT) +ANT?= ${LOCALBASE}/bin/ant +MAKE_ENV+= JAVA_HOME=${JAVA_HOME} +BUILD_DEPENDS+= ${ANT}:${PORTSDIR}/devel/apache-ant +ALL_TARGET?= +. if !target(do-build) +do-build: + @(cd ${BUILD_WRKSRC}; \ + ${SETENV} ${MAKE_ENV} ${ANT} ${MAKE_ARGS} ${ALL_TARGET}) +. endif +. endif #----------------------------------------------------------------------------- # Stage 7: Define all settings for the port to use --- end of bsd.java.mk.diff --- Because this patch will be much easier to test with a port, here is a patch for java/jakarta-commons-collections to have it using the proposed features: --- begin of jakarta-commons-collections.diff --- --- Makefile.orig Sun Aug 1 10:47:14 2004 +++ Makefile Sun Aug 1 10:48:36 2004 @@ -16,26 +16,21 @@ MAINTAINER= znerd@FreeBSD.org COMMENT= Classes that extend/augment the Java Collections Framework -BUILD_DEPENDS= ${ANT}:${PORTSDIR}/devel/apache-ant - USE_JAVA= yes JAVA_VERSION= 1.2+ +USE_ANT= yes WRKSRC= ${WRKDIR}/${PORTNAME}-${PORTVERSION} -ANT?= ${LOCALBASE}/bin/ant -ANT_TARGET= jar +ALL_TARGET= jar .if !defined(NOPORTDOCS) -ANT_TARGET+= javadoc +ALL_TARGET+= javadoc OTHERDOCS= DEVELOPERS-GUIDE.html LICENSE.txt PROPOSAL.html README.txt RELEASE-NOTES.html STATUS.html PORTDOCS= apidocs ${OTHERDOCS} .endif JARFILE= ${PORTNAME}-${PORTVERSION}.jar DESTJARFILE= ${PORTNAME}.jar PLIST_FILES+= ${JAVAJARDIR:S,^${PREFIX}/,,}/${DESTJARFILE} - -do-build: - @cd ${WRKSRC} && ${ANT} ${ANT_TARGET} do-install: @${ECHO_CMD} -n ">> Installing JAR as ${JAVAJARDIR}/${DESTJARFILE}..." --- end of jakarta-commons-collections.diff --- Unfortunately, with the moving of the USE_ANT --> JAVA_BUILD=jdk definition, a port may not define USE_ANT and NO_BUILD simultaneously anymore. Indeed, when JAVA_BUILD and NO_BUILD are set, the check-makevars target will produce some error statement. You may test this by running the following (java/trove4j is a binary port): $ cd /usr/ports/java/trove4j $ make -DUSE_ANT trove4j-1.1b3: Makefile error: JAVA_BUILD and NO_BUILD cannot be set at the same time. *** Error code 1 Now we need to decide if this is really a problem. I mean, is there really a case where Ant would be used for anything but building the port? And does such a case happen so frequently that it needs to be supported? IMHO, it's not worth it but I bet you can prove me wrong with the right (set of) example(s). Herve PS: I now realize that chosing a test port that is neither maintained by you and I is not such a good idea... Sorry for that.
Hi Herve, On Sun, Aug 01, 2004 at 11:11:08AM +0200, Herve Quiroz wrote: > On Fri, Jul 30, 2004 at 12:19:51PM -0600, Greg Lewis wrote: > > > IMHO, USE_ANT means that Ant will be required to build the port. So > > > USE_ANT should mean JAVA_BUILD=jdk and thus the first hunk in your patch > > > is no longer needed. > > > > Agreed. I've replaced it with a section that automatically sets JAVA_BUILD > > if USE_ANT is set. I think that this is what is wanted? > > I noticed a little problem with your latest patch. Indeed, > JAVA_BUILD=jdk is set when USE_ANT is defined but only when using > bsd.java.mk 1.0 backward compatibility mode as the conditional statement > comes nested in some '.if (${_USE_BSD_JAVA_MK_1_0} == "yes")' statement. D'oh, good catch! > Here is a patch with the suggested modification as well as the > documentation in the header of the file: > > --- begin of bsd.java.mk.diff --- [snip] > --- end of bsd.java.mk.diff --- Thanks for the review, I'll get this committed tomorrow. > Because this patch will be much easier to test with a port, here is a > patch for java/jakarta-commons-collections to have it using the proposed > features: > > --- begin of jakarta-commons-collections.diff --- [snip] > --- end of jakarta-commons-collections.diff --- I had a similar test patch for a different port :). > Unfortunately, with the moving of the USE_ANT --> JAVA_BUILD=jdk > definition, a port may not define USE_ANT and NO_BUILD simultaneously > anymore. Indeed, when JAVA_BUILD and NO_BUILD are set, the > check-makevars target will produce some error statement. You may test > this by running the following (java/trove4j is a binary port): > > $ cd /usr/ports/java/trove4j > $ make -DUSE_ANT > trove4j-1.1b3: Makefile error: JAVA_BUILD and NO_BUILD cannot be set at the same time. > *** Error code 1 > > Now we need to decide if this is really a problem. I mean, is there > really a case where Ant would be used for anything but building the > port? And does such a case happen so frequently that it needs to be > supported? IMHO, it's not worth it but I bet you can prove me wrong with > the right (set of) example(s). I'm not aware of any port which uses ant but doesn't use it to build a jar by compiling things with javac. I think this isn't a problem. If it ever becomes a problem I think it will be simple to fix. The current patch seems to be very sensible in assuming USE_ANT implies JAVA_BUILD=jdk and I think we should leave it this way for now, as it adds in error checking that will make sense in all the cases we know of. > PS: I now realize that chosing a test port that is neither maintained by > you and I is not such a good idea... Sorry for that. I did the same thing with my test patches, so no need to be sorry :). -- Greg Lewis Email : glewis@eyesbeyond.com Eyes Beyond Web : http://www.eyesbeyond.com Information Technology FreeBSD : glewis@FreeBSD.org
State Changed From-To: open->closed Ant support is fixed, individual ports now need to be fixed to use it.