Bug 173124 - update net-mgmt/mrtg
Summary: update net-mgmt/mrtg
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Guido Falsi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-26 16:30 UTC by Alexey V. Panfilov
Modified: 2012-10-30 13:07 UTC (History)
0 users

See Also:


Attachments
file.diff (1.63 KB, patch)
2012-10-26 16:30 UTC, Alexey V. Panfilov
no flags Details | Diff
mrtg.diff (2.10 KB, patch)
2012-10-27 11:16 UTC, Guido Falsi
no flags Details | Diff
mrtg.diff (2.08 KB, patch)
2012-10-29 17:10 UTC, Guido Falsi
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey V. Panfilov 2012-10-26 16:30:00 UTC
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
Comment 1 Edwin Groothuis freebsd_committer 2012-10-26 16:30:30 UTC
Class Changed
From-To: update->maintainer-update

Fix category (submitter is maintainer) (via the GNATS Auto Assign Tool)
Comment 2 Guido Falsi freebsd_committer 2012-10-27 10:14:35 UTC
Responsible Changed
From-To: freebsd-ports-bugs->madpilot

I'll take it.
Comment 3 Guido Falsi freebsd_committer 2012-10-27 11:16:17 UTC
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>
Comment 4 Guido Falsi freebsd_committer 2012-10-28 09:46:38 UTC
State Changed
From-To: open->feedback

Ask for maintainer approval.
Comment 5 Alexey V. Panfilov 2012-10-29 11:45:42 UTC
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 ;-)
Comment 6 Guido Falsi freebsd_committer 2012-10-29 17:10:57 UTC
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>
Comment 7 Alexey V. Panfilov 2012-10-30 08:27:20 UTC
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 ;-)
Comment 8 Guido Falsi freebsd_committer 2012-10-30 13:07:14 UTC
State Changed
From-To: feedback->closed

Committed. Thanks!