- use build/install logic from distfile, the port depends on devel/gmake, anyway - respect CC/CXX from environ and make.conf - make post-patch target quiet, not install target How-To-Repeat: $ env CC=clang CXX=clang++ make build
Responsible Changed From-To: freebsd-ports-bugs->arved Over to maintainer (via the GNATS Auto Assign Tool)
arved 2010-07-02 18:50:21 UTC FreeBSD ports repository Modified files: audio/ladspa Makefile Log: Respect CC CXX. PR: 148109 Submitted by: Anonymous <swell.k@gmail.com> Feature safe: yes Revision Changes Path 1.15 +2 -2 ports/audio/ladspa/Makefile _______________________________________________ cvs-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/cvs-all To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
State Changed From-To: open->closed Thanks for reporting. The port should now buld with clang
arved@FreeBSD.org writes: > Synopsis: [patch] audio/ladspa: avoid custom make targets > > State-Changed-From-To: open->closed > State-Changed-By: arved > State-Changed-When: Fri Jul 2 18:50:53 UTC 2010 > State-Changed-Why: > Thanks for reporting. The port should now buld with clang You're aware that was a change-request, not a sw-bug? Unless you give me a reason for rejecting the rest of the patch, I'm requesting that you reopen the PR. For example, I still think ports should be verbose about installing files, strip(1)ing plugins on install, use _MAKE_JOBS when FORCE_MAKE_JOBS is set and respect CXXFLAGS from environment or make.conf. Avoiding custom do-* targets reduces the chance of such bugs. BTW, you can mark the port as MAKE_JOBS_SAFE, it successfully builds with -j10, not with your custom do-build target, though. > > http://www.freebsd.org/cgi/query-pr.cgi?pr=148109
arved 2010-07-03 14:15:40 UTC FreeBSD ports repository Modified files: audio/ladspa Makefile Log: Respect CXXFLAGS Make port profit from MAKE_JOBS PR: 148109 Requested by: Anonymous <swell.k@gmail.com> Feature safe: yes Revision Changes Path 1.16 +6 -4 ports/audio/ladspa/Makefile _______________________________________________ cvs-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/cvs-all To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
* Anonymous [2010-07-03 13:54]: > You're aware that was a change-request, not a sw-bug? Unless you give me > a reason for rejecting the rest of the patch, I'm requesting that you > reopen the PR. > > For example, I still think ports should be verbose about installing > files, strip(1)ing plugins on install, use _MAKE_JOBS when > FORCE_MAKE_JOBS is set and respect CXXFLAGS from environment or > make.conf. Avoiding custom do-* targets reduces the chance of such bugs. > > BTW, you can mark the port as MAKE_JOBS_SAFE, it successfully builds > with -j10, not with your custom do-build target, though. Ok, i made the port respect CXXFLAGS and MAKE_JOBS_SAFE. Thanks. I did not strip the plugins, because then i would have to bump PORTREVISION, which is not worth it imho. If i remember i will do it with the next update.
Tilman Linneweh <arved@FreeBSD.org> writes: > * Anonymous [2010-07-03 13:54]: >> You're aware that was a change-request, not a sw-bug? Unless you give me >> a reason for rejecting the rest of the patch, I'm requesting that you >> reopen the PR. >> >> For example, I still think ports should be verbose about installing >> files, strip(1)ing plugins on install, use _MAKE_JOBS when >> FORCE_MAKE_JOBS is set and respect CXXFLAGS from environment or >> make.conf. Avoiding custom do-* targets reduces the chance of such bugs. >> >> BTW, you can mark the port as MAKE_JOBS_SAFE, it successfully builds >> with -j10, not with your custom do-build target, though. > > Ok, i made the port respect CXXFLAGS and MAKE_JOBS_SAFE. Thanks. Good. > > I did not strip the plugins, because then i would have to bump PORTREVISION, which is not > worth it imho. If i remember i will do it with the next update. Please do and remove custom do-install target as well. It's better to trust vendor makefile for available plugins. However, changing CC/CXX and CXXFLAGS already requires PORTREVISION bump because it *affects* build. And you're abandoning the bug about respecting STRIP... Considering how often ladspa_sdk releases I guess the PR would be suspended for a few years. BTW, I think having one simple patch is better than making those regexps even more ugly. That file wasn't generated by automake, anyway. $ cd $(make -V BUILD_WRKSRC) $ gmake install PREFIX=$HOME/aaa cc -I. -Wall -Werror -fPIC -o plugins/amp.o -c plugins/amp.c ld -o ../plugins/amp.so plugins/amp.o -shared cc -I. -Wall -Werror -fPIC -o plugins/delay.o -c plugins/delay.c ld -o ../plugins/delay.so plugins/delay.o -shared cc -I. -Wall -Werror -fPIC -o plugins/filter.o -c plugins/filter.c ld -o ../plugins/filter.so plugins/filter.o -shared cc -I. -Wall -Werror -fPIC -o plugins/noise.o -c plugins/noise.c ld -o ../plugins/noise.so plugins/noise.o -shared g++ -I. -Wall -Werror -fPIC -o plugins/sine.o -c plugins/sine.cpp g++ -o ../plugins/sine.so plugins/sine.o -shared cc -I. -Wall -Werror -fPIC -c -o analyseplugin.o analyseplugin.c cc -I. -Wall -Werror -fPIC -c -o load.o load.c cc -I. -Wall -Werror -fPIC -c -o default.o default.c cc -I. -Wall -Werror -fPIC -lm \ -o ../bin/analyseplugin \ analyseplugin.o load.o default.o cc -I. -Wall -Werror -fPIC -c -o applyplugin.o applyplugin.c cc -I. -Wall -Werror -fPIC -lm \ -o ../bin/applyplugin \ applyplugin.o load.o default.o cc -I. -Wall -Werror -fPIC -c -o listplugins.o listplugins.c cc -I. -Wall -Werror -fPIC -c -o search.o search.c cc -I. -Wall -Werror -fPIC -lm \ -o ../bin/listplugins \ listplugins.o search.o install -d /home/holo/aaa/lib/ladspa install -d /home/holo/aaa/include install -d /home/holo/aaa/bin install -s -m 555 ../plugins/* /home/holo/aaa/lib/ladspa install -m 444 ladspa.h /home/holo/aaa/include install -s -m 555 ../bin/* /home/holo/aaa/bin --- a.diff begins here --- Index: audio/ladspa/Makefile =================================================================== RCS file: /a/.cvsup/ports/audio/ladspa/Makefile,v retrieving revision 1.16 diff -u -p -r1.16 Makefile --- audio/ladspa/Makefile 3 Jul 2010 14:15:17 -0000 1.16 +++ audio/ladspa/Makefile 3 Jul 2010 15:38:00 -0000 @@ -20,27 +20,11 @@ LICENSE_FILE= ${WRKSRC}/doc/COPYING WRKSRC= ${WRKDIR}/ladspa_sdk USE_GMAKE= yes USE_LDCONFIG= yes -PROGRAM_FILES= analyseplugin applyplugin listplugins -PLUGIN_FILES= amp.so delay.so filter.so noise.so sine.so -PLUGIN_DIR?= ${PREFIX}/lib/ladspa/ +MAKE_ENV+= INSTALL_PROGRAM="${INSTALL_PROGRAM}" INSTALL_DATA="${INSTALL_DATA}" MAKEFILE= makefile ALL_TARGET= targets BUILD_WRKSRC= ${WRKSRC}/src +INSTALL_WRKSRC= ${BUILD_WRKSRC} MAKE_JOBS_SAFE= yes -post-patch: - ${REINPLACE_CMD} -e 's,-ldl,,; s,-O3,${CFLAGS},; \ - s,^CC,#CC,;s,^CPP,#CPP,;s,CPP,CXX,; s,^CXXFLAGS.*=,CXXFLAGS+=,' \ - ${WRKSRC}/src/makefile - -do-install: - @${INSTALL_DATA} ${WRKSRC}/src/ladspa.h ${PREFIX}/include -.for file in ${PROGRAM_FILES} - @${INSTALL_PROGRAM} ${WRKSRC}/bin/${file} ${PREFIX}/bin -.endfor - @${MKDIR} ${PLUGIN_DIR} -.for file in ${PLUGIN_FILES} - @${INSTALL_DATA} ${WRKSRC}/plugins/${file} ${PLUGIN_DIR} -.endfor - .include <bsd.port.mk> Index: audio/ladspa/files/patch-makefile =================================================================== RCS file: audio/ladspa/files/patch-makefile diff -N audio/ladspa/files/patch-makefile --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ audio/ladspa/files/patch-makefile 3 Jul 2010 15:25:56 -0000 @@ -0,0 +1,71 @@ +--- src/makefile.orig 2007-11-06 13:42:45.000000000 +0300 ++++ src/makefile 2010-07-03 19:22:33.576284227 +0400 +@@ -4,9 +4,7 @@ + # + # Change these if you want to install somewhere else. + +-INSTALL_PLUGINS_DIR = /usr/lib/ladspa/ +-INSTALL_INCLUDE_DIR = /usr/include/ +-INSTALL_BINARY_DIR = /usr/bin/ ++PREFIX ?= /usr + + ############################################################################### + # +@@ -14,9 +12,9 @@ INSTALL_BINARY_DIR = /usr/bin/ + # + + INCLUDES = -I. +-LIBRARIES = -ldl -lm +-CFLAGS = $(INCLUDES) -Wall -Werror -O3 -fPIC +-CXXFLAGS = $(CFLAGS) ++LIBRARIES = -lm ++CFLAGS += $(INCLUDES) -Wall -Werror -fPIC ++CXXFLAGS += $(CFLAGS) + PLUGINS = ../plugins/amp.so \ + ../plugins/delay.so \ + ../plugins/filter.so \ +@@ -25,8 +23,12 @@ PLUGINS = ../plugins/amp.so \ + PROGRAMS = ../bin/analyseplugin \ + ../bin/applyplugin \ + ../bin/listplugins +-CC = cc +-CPP = c++ ++CC ?= cc ++CXX ?= c++ ++INSTALL_DIR ?= install -d ++INSTALL_PROGRAM ?= install $(STRIP) -m 555 ++INSTALL_DATA ?= install -m 444 ++STRIP ?= -s + + ############################################################################### + # +@@ -38,8 +40,8 @@ CPP = c++ + $(LD) -o ../plugins/$*.so plugins/$*.o -shared + + ../plugins/%.so: plugins/%.cpp ladspa.h +- $(CPP) $(CXXFLAGS) -o plugins/$*.o -c plugins/$*.cpp +- $(CPP) -o ../plugins/$*.so plugins/$*.o -shared ++ $(CXX) $(CXXFLAGS) -o plugins/$*.o -c plugins/$*.cpp ++ $(CXX) -o ../plugins/$*.so plugins/$*.o -shared + + ############################################################################### + # +@@ -59,12 +61,12 @@ test: /tmp/test.wav ../snd/noise.wav alw + @echo Test complete. + + install: targets +- -mkdirhier $(INSTALL_PLUGINS_DIR) +- -mkdirhier $(INSTALL_INCLUDE_DIR) +- -mkdirhier $(INSTALL_BINARY_DIR) +- cp ../plugins/* $(INSTALL_PLUGINS_DIR) +- cp ladspa.h $(INSTALL_INCLUDE_DIR) +- cp ../bin/* $(INSTALL_BINARY_DIR) ++ $(INSTALL_DIR) $(PREFIX)/lib/ladspa ++ $(INSTALL_DIR) $(PREFIX)/include ++ $(INSTALL_DIR) $(PREFIX)/bin ++ $(INSTALL_PROGRAM) ../plugins/* $(PREFIX)/lib/ladspa ++ $(INSTALL_DATA) ladspa.h $(PREFIX)/include ++ $(INSTALL_PROGRAM) ../bin/* $(PREFIX)/bin + + /tmp/test.wav: targets ../snd/noise.wav + ../bin/listplugins --- a.diff ends here ---
Hello Anonymous, On Jul 3, 2010, at 17:59 , Anonymous wrote: >=20 > Please do and remove custom do-install target as well. It's better to > trust vendor makefile for available plugins. Please provide reason, why it is better to make a 70lines patch instead = of a 8 lines do-install target.=20 Generally the FreeBSD port maintainer knows better where files should be = installed on FreeBSD, than the Linux-centric upstream author. >=20 > However, changing CC/CXX and CXXFLAGS already requires PORTREVISION = bump > because it *affects* build. And you're abandoning the bug about > respecting STRIP... Considering how often ladspa_sdk releases I guess > the PR would be suspended for a few years. No, for the supported default environment the package did not change.=
arved@FreeBSD.org writes: > Hello Anonymous, > > On Jul 3, 2010, at 17:59 , Anonymous wrote: >> >> Please do and remove custom do-install target as well. It's better to >> trust vendor makefile for available plugins. > > Please provide reason, why it is better to make a 70lines patch instead of > a 8 lines do-install target. Well, if ladspa development wasn't frozen then I'd try to submit it upstream. But I need to test it on Linux first. If you care so much about number of lines why not use post-patch target from my first diff? It takes only 3 lines of regexps but your custom target uses 8 lines. + -e 's,/usr,${PREFIX},; s,-mkdirhier,${INSTALL} -d,;' \ + -e '/install:/,/^$$/{ /(BINARY|PLUGINS)_DIR/s,cp,${INSTALL_PROGRAM},; \ + /INCLUDE_DIR/s,cp,${INSTALL_DATA},; }' \ And you can reduce 10 lines if you dispose of FILESDIR/patch-applyplugin.c to one-liner. > > Generally the FreeBSD port maintainer knows better where files should > be installed on FreeBSD, than the Linux-centric upstream author. It does not mean patches should be FreeBSD-specific, too. > >> >> However, changing CC/CXX and CXXFLAGS already requires PORTREVISION bump >> because it *affects* build. And you're abandoning the bug about >> respecting STRIP... Considering how often ladspa_sdk releases I guess >> the PR would be suspended for a few years. > > No, for the supported default environment the package did not change. So, respecting STRIP (bugfix) is less important than PORTREVISION?