Bug 148109
| Summary: | [patch] audio/ladspa: avoid custom make targets | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | swell.k <swell.k> | ||||
| Component: | Individual Port(s) | Assignee: | Tilman Keskinoz <arved> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Only Me | ||||||
| Priority: | Normal | ||||||
| Version: | Latest | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
|
Description
swell.k
2010-06-24 11:20:07 UTC
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@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? |
