ports/119183: [NEW PORT] net/freeradius-client: FreeRADIUS Client library

[David Wood]

The following reply was made to PR ports/119183; it has been noted by GNATS.

From: David Wood <david at wood2.org.uk>
To: bug-followup at FreeBSD.org, joe at rewt.org.uk
Cc:  
Subject: Re: ports/119183: [NEW PORT] net/freeradius-client: FreeRADIUS Client library
Date: Wed, 30 Jan 2008 15:51:58 +0000

 Hi Joe,
 
 I had a read through your port - though I haven't tried to build it. I 
 haven't therefore checked for things like leaving files behind.
 
 I am not a committer; I'm just interested in this port as the maintainer 
 of net/freeradius and net/freeradius2. I hope that these comments help 
 to improve the submission and that a committer becomes interested in it, 
 as it would be great to see freeradius-client in the ports tree.
 
 At the moment, I think there's some problems with the port that would 
 need to be tidied up before it can be committed. The less work a 
 committer has to do, the more likely he or she will be to commit your 
 port.
 
 That said, feel free to disagree with me on some of these points - 
 they're only suggestions to try to improve the port for everyone's 
 benefit.
 
 
 I would suggest that the MASTER_SITES and MASTER_SITE_SUBDIR values used 
 in ports/119909 for net/freeradius2 are used in preference to the values 
 in this PR.
 
 Note, however, that there's a space in my PR in the MASTER_SITE_SUBDIR 
 line after the = that  really should be a tab. This is an error on my 
 part and I hope that the committer who deals with my PR fixes it. I was 
 trying to get the PR submitted fairly fast, as FreeRADIUS 2.0.1 fixes 
 various problems that people were having with 2.0.0.
 
 The FreeRADIUS team move old releases into the 'old' folder when a new 
 release appears. If you don't handle that case, the port immediately 
 breaks when a new version is released as the distfile can't be fetched. 
 This is the current situation for net/freeradius2 until ports/119909 is 
 committed.
 
 The values in ports/119909 have been tested pretty carefully, and 
 reflect the latest list of mirrors on the FreeRADIUS site.
 
 
 
 There's some problems in the pkg-plist.
 
 @dirrm /etc/radiusclient
 looks as if it should be
 @dirrm etc/radiusclient
 
 All the @dirrmtry lines under that line look wrong - and I wonder if 
 they should be deleted. If the pkg-plist is right, it doesn't look as if 
 the port directly touches any of those directories anyway.
 
 
 
 It looks as if the port installs a clean configuration directly into 
 etc/radiusclient/* - which is against the philosophy of ports, as it 
 means user configurations are interfered with.
 
 There's various ways to deal with this; though as there's several 
 configuration files, I wonder if the same approach as net/freeradius and 
 net/freeradius2 is warranted - that is, patch the software to install 
 its configuration in ${EXAMPLESDIR}. You can't tell the software to use 
 ${EXAMPLESDIR} via configure, as it will not only install its 
 configuration there, it will look at its location for its 'live' 
 configuration!
 
 I did discuss with the FreeRADIUS developers about making a configure 
 option to install the configuration in one location and look for the 
 'live' configuration in another for the server, and they weren't 
 interested - suggesting that I should continue to patch the build 
 system.
 
 Once you've added the necessary code to the port to patch the software 
 to install the configuration in ${EXAMPLESDIR}, use loops to copy 
 missing files to the 'true' location on install and delete unmodified 
 files from the 'true' location on uninstall. You then need to @dirrmtry 
 etc/radiusclient rather than @dirrm, as it may not be empty. You could 
 also have a warning about removing etc/radiusclient if you no longer 
 need it in the event that it isn't empty.
 
 See the post-patch and post-install targets of net/freeradius/Makefile, 
 and the @exec / @unexec lines in net/freeradius/pkg-plist.
 
 There's two things you need to watch. Firstly, the order of the @exec 
 and @unexec lines in pkg-plist in relation to the files in 
 ${EXAMPLESDIR} is important - you need to @unexec before those lines and 
 @exec after. You also need to put identical installation stuff in the 
 post-install target of the Makefile for the port and @exec in the 
 pkg-plist for the package. In Makefile, readability is arguably best. In 
 pkg-plist you have to get the whole loop into one line.
 
 The basic idea is (this is an installation example):
 for i in `find ... | sed ...`; do if [ ! -f ...${i}]; then cp ...${i} 
 ...; fi; done
 
 I'd take net/freeradius as your example; the syntax is slightly less 
 complicated than for net/freeradius2, which has the added complication 
 of symlinks in raddb.
 
 
 Can you put the dictionary files in ${DATADIR} rather than 
 etc/radiusclient? Arguably they're read-only - though I'm not sure 
 whether the FreeRADIUS client build system makes this as easy as the 
 FreeRADIUS server one.
 
 
 Should scripts/pre-configure return a non-zero exit code if a patching 
 attempt fails, rather than just a comment? I don't think it does as 
 written here.
 
 Is scrits/pre-configure really a post-patch rather than a pre-configure 
 step? This is a semantic point really - don't worry too much about it!
 
 Is it easier for maintenance purposes to keep all the patching stuff in 
 Makefile? I prefer things that way for net/freeradius and 
 net/freeradius2, but it is a personal decision.
 
 
 You will probably need --with-pic in CONFIGURE_ARGS for the port to work 
 on amd64. I suspect you're working on an i386 machine, as I am - though 
 I'm likely to move to amd64 for my main server in the next few months.
 
 .if ${ARCH} == amd64
 CONFIGURE_ARGS+=        --with-pic
 .endif
 
 in the Makefile should do it (note that my mail client eats tabs - there 
 should be a tab after +=, not several spaces).
 
 
 USE_AUTOTOOLS= yes in the Makefile is obsolete to my understanding - you 
 should specify which autotools and which minimum version. I suspect you 
 need autoconf:261 - though does the port also need libtool? Again, 
 net/freeradius/Makefile may guide you.
 
 
 Should CATEGORIES also include devel? (You may need to take your 
 committer's advice on this).
 
 
 Would CONFLICTS be better as radiusclient-[0-9].* than just 
 radiusclient*?
 
 
 
 Very best wishes,
 
 
 
 
 David
 -- 
 David Wood
 david at wood2.org.uk