1. Convert to new OPTIONS framework. 2. Respect GD_PORT variable which slave ports can override with languages specific gd. Fix: apply attached patch Patch attached with submission follows: How-To-Repeat: n/a
Class Changed From-To: update->maintainer-update Fix category (submitter is maintainer) (via the GNATS Auto Assign Tool)
Responsible Changed From-To: freebsd-ports-bugs->madpilot I'll take it.
Hi, I'm not sure I understand why you added the GD_PORT variable. If you want this port to allow slave ports properly also other variables need to be conditionally et (at a minimum PORTEPOCH, and PORTREVISION, also the MAINTAINER if the subports are maintained by someone else.). Why mrtg should depend on a language specific bindings for gd if it is not using them? I also fixed a few more things. PORTDOCS and PORTEXAMPLES don't need to be conditionally set, they do "the right thing" automatically. Also, your patch was relative to an old version of the port. Please always make sure you're working on the current version of the port before patching. I adapted your patches with my fixes. I'm attaching the result please check them and let me know if and why the GD_PORT part is really needed. -- Guido Falsi <madpilot@FreeBSD.org>
State Changed From-To: open->feedback Ask for maintainer approval.
27.10.2012 14:16, Guido Falsi wrote: > Hi, Hi, Guido > I'm not sure I understand why you added the GD_PORT variable. > If you want this port to allow slave ports properly also other variables > need to be conditionally et (at a minimum PORTEPOCH, and PORTREVISION, > also the MAINTAINER if the subports are maintained by someone else.). Excuse me, I'm was wrong about slave ports - i spoke about mrtg itself only. In my opinion, quantity of people, who don't needs to rebuild of mrtg match more, that who needs in it - that's why PORTREVISION was not bumped. > Why mrtg should depend on a language specific bindings for gd if it is > not using them? The problem is that russian named of Y axis ("YLegend" in terms of mrtg's config) shows not correct on graphics. To avoid this situation port starts respect GD_PORT: if set variable GD_PORT to "russian/gd", for example at /etc/make.conf, then russian string shows correctly on the graphics. mrtg's port aren't alone: the same thing do www/webalizer. > I also fixed a few more things. PORTDOCS and PORTEXAMPLES don't need to > be conditionally set, they do "the right thing" automatically. I'll be happy if I could read about this at porter's handbook, but I can't find it there. May be it should be written on handbook? And another things: http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/install.html#install-documentation handbook recommends for install additional documentation following method of check variables: .if !defined(NOPORTDOCS) but you choose another way .if ${PORT_OPTIONS:MDOCS} Which way is right and why? > Also, your patch was relative to an old version of the port. > Please always make sure you're working on the current version of the > port before patching. You're right, it was my mistake :( Great that you saw it! > I adapted your patches with my fixes. I'm attaching the result please > check them and let me know if and why the GD_PORT part is really needed. Now default value of IPV6 is selected and it sets by Mk/bsd.options.mk. Please, explain, why you override it by the port's Makefile? Other parts of your work looks good :))) Before you commit a patch I'd like to hear answer to my questions. It's important to me - I'd like to make my work better. The GD_PORT part is really needed. -- Simple Lehisnoe ;-)
On Mon, Oct 29, 2012 at 03:45:42PM +0400, Alexey V. Panfilov wrote: > 27.10.2012 14:16, Guido Falsi wrote: > > Hi, > > Hi, Guido > > > I'm not sure I understand why you added the GD_PORT variable. > > If you want this port to allow slave ports properly also other variables > > need to be conditionally et (at a minimum PORTEPOCH, and PORTREVISION, > > also the MAINTAINER if the subports are maintained by someone else.). > > Excuse me, I'm was wrong about slave ports - i spoke about mrtg itself > only. In my opinion, quantity of people, who don't needs to rebuild of > mrtg match more, that who needs in it - that's why PORTREVISION was not > bumped. No problem. And no PORTREVISION does not need to be bumped, since the generated binary package does no change. But, if there was a real slave port it should be defined like this: PORTREVISION?= x to allow slave prots to override this value when needed. > > > Why mrtg should depend on a language specific bindings for gd if it is > > not using them? > > The problem is that russian named of Y axis ("YLegend" in terms of > mrtg's config) shows not correct on graphics. To avoid this situation > port starts respect GD_PORT: if set variable GD_PORT to "russian/gd", > for example at /etc/make.conf, then russian string shows correctly on > the graphics. mrtg's port aren't alone: the same thing do www/webalizer. > I see. Well the webalizer port uses that variable in it's real slave ports for other languages in a quite elaborated way. Anyway now that I know why you need it I don't see a problem with that anymore. > > I also fixed a few more things. PORTDOCS and PORTEXAMPLES don't need to > > be conditionally set, they do "the right thing" automatically. > > I'll be happy if I could read about this at porter's handbook, but I > can't find it there. May be it should be written on handbook? > Well it's written in the handbook, right at the end of the chapter. It's not perfectly clear: If a directory is listed in PORTDOCS or matched by a glob pattern from this variable, the entire subtree of contained files and directories will be registered in the final packing list. If NOPORTDOCS is defined then files and directories listed in PORTDOCS would not be installed and neither would be added to port packing list. PORTDOCS=* will match whatever the port install in DOCSDIR, if it installs nothing, nothing will be added to the plist. this is valid also if PORTDOCS=foo since there is logic in bsd.port.mk to check for NOPORTDOCS before installation. You can find this logic at line 5735 of bsd.port.mk (Hope I got the line number right). > And another things: > http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/install.html#install-documentation > handbook recommends for install additional documentation following > method of check variables: > > .if !defined(NOPORTDOCS) > > but you choose another way > > .if ${PORT_OPTIONS:MDOCS} > > Which way is right and why? Good catch! :) Documentation needs to be fixed. Ports using optionsng should use the DOCS, EXASMPLES and NLS values in PORT_OPTIONS in place of the old "NO*" variables. Unluckily patching documentation always end up lagging behind :( I'll try to make a patch for the docs as soon as I have time :) > > I adapted your patches with my fixes. I'm attaching the result please > > check them and let me know if and why the GD_PORT part is really needed. > > Now default value of IPV6 is selected and it sets by Mk/bsd.options.mk. > Please, explain, why you override it by the port's Makefile? My mistake, sorry :) > > Other parts of your work looks good :))) > > Before you commit a patch I'd like to hear answer to my questions. It's > important to me - I'd like to make my work better. Don't worry, I will not commit without your explicit approval for such matters. > > The GD_PORT part is really needed. Sure, now that you have explained it to me I understand why. Anyway if you want to make slave ports you need also to conditionally set the MAINTAINER. I'm attaching a revised patch also implementing your suggestions. Thanks for your feedback :) -- Guido Falsi <madpilot@FreeBSD.org>
29.10.2012 21:10, Guido Falsi wrote: [skip] > Thanks for your feedback :) > I'm glad to help make FreeBSD better. Your last patch approved :) Please, commit it. -- Simple Lehisnoe ;-)
State Changed From-To: feedback->closed Committed. Thanks!