Red Hat Bugzilla – Bug 165964
Review Request: intuitively. Automatic IP detection utility
Last modified: 2007-11-30 17:11:11 EST
SRPM Name or Url: http://www.environnement.ens.fr/docs/fc-srpms/intuitively-0.7-1.src.rpm Description: Automatic IP configuration detection for laptops A utility to locate current network address via arp requests and perform heavy reconfigurations based on its findings. "intuitively" is intended for laptop users or people who use their machines in different networks all the time. It is meant to be run from the PCMCIA network initialization scripts or the command line.
An updated srpm with the correct url reported by Spot is here: http://www.environnement.ens.fr/docs/fc-srpms/intuitively-0.7-3.src.rpm
From the web page at the new URL:: "Download the latest version from your local debian mirror, for instance ftp.uninett.no" So it appears the tarball source is a debian package.
* It is unfortunate that libpcap does not "Provides: libpcap-devel = %version-%release", but libnet does, so you ought to prefer "BuildRequires: libnet-devel" over your "Buildrequires: libnet". * Better summary would be: Automatic IP detection utility Basically that's what the first broken sentence of the description contains. ;) * Build fails here: checking for flex... no checking for lex... no ./configure: line 2415: flex: command not found * I recommend marking the config file "noreplace", since the included defaults ask for customisation. * The included config file refers to a non-existant directory /usr/share/intuitively, albeit only in comments. This is confusing.
This should be fixed in: http://www.environnement.ens.fr/docs/fc-srpms/intuitively-0.7-4.src.rpm I changed /usr/share to /opt in the config file, is it enough?
> I changed /usr/share to /opt in the config file, is it enough? Why don't move /opt stuff to /usr/share ? It would be more usual way of packaging...
There is nothing packaged in /opt... On the contrary... I changed /usr/share to /opt in a config file to show that it is not in the package...
Remarks & nitpicks: - use '-q' for %setup - instead of the patch (patch0), just specify "sysconfdir=..." for "make install" - instead of patch1, just use "sed -i 's/.../..../' filename" at prep stage - there is a garbage in man pages, because instead of "docbook2man file.sgml" a wrong "docbook2man file.sgml > file.NUM" is used. Therefore manuals should be properly re-created in the end of %build stage. - IMO it is better to use %{name} and wildcards for %files - Is it useful to include intuitively.conf.dist into the docs? It seems to be the same as /etc/intuitively/intuitively.conf ...
Created attachment 121492 [details] Suggested changes for spec-file Feel free to avoid some of them :)
(In reply to comment #7) The new srpm should appear shortly at http://www.environnement.ens.fr/perso/dumas/fc-srpms/intuitively-0.7-5.src.rpm > Remarks & nitpicks: > > - use '-q' for %setup done > - instead of the patch (patch0), just specify "sysconfdir=..." for "make install" it won't work because @sysconfdir@ is also used in the code. > - instead of patch1, just use "sed -i 's/.../..../' filename" at prep stage done > - there is a garbage in man pages, because instead of "docbook2man file.sgml" a > wrong "docbook2man file.sgml > file.NUM" is used. Therefore manuals should be > properly re-created in the end of %build stage. done > - IMO it is better to use %{name} and wildcards for %files I used %{name}, but only in the %files section, I think it is less readable with %{name} everywhere. I avoided wildcards (except for manpages), because I think that with the files explicitely specified, it is easier to see that something sneaked in. > - Is it useful to include intuitively.conf.dist into the docs? It seems to be > the same as /etc/intuitively/intuitively.conf ... Yep it is the same but I think it is better to keep it because the user should rewrite the /etc/intuitively/intuitively.conf.
>> - instead of the patch (patch0), just specify "sysconfdir=..." for "make >> install" > it won't work because @sysconfdir@ is also used in the code. Where?... I can find it only in Makefile.in files... Anyway, each @sysconfdir@ in the code must be substituted to usual %{_sysconfdir} (without $RPM_BUILD_ROOT prefix), and configure script does it properly. Therefore "add-DEST_DIR-patch" is not needed for code itself. This patch is needed for install time only.
Oh, you're right. I wrongly believed that sysconfdir was passed to configure, but it isn't the case... Thanks. It should be fixed in the srpm that should appear shortly at: http://www.environnement.ens.fr/perso/dumas/fc-srpms/intuitively-0.7-6.src.rpm
Works. MUST/SHOULD items OK APPROVED. One suggestion: There are "Work" and "Home" locations in the current intuitively.conf file. "intuitively" reports "Cannot chdir to /etc/intuitively/Work/." because of missed subdirs. IMO it would be better to create /etc/intuitively/default subdir, rename one location to "default", and comment out all another locations. It will satisfy "intuitively" here and also will hint end-user to create his own subdir if needed...
Well I think the the real issue is the error message "Cannot chdir to /etc/intuitively/Work/.", there should be a check that the directory exists prior doing the chdir. I don't think it is a good idea to ship with a directory in the default case because intuitively don't need this directory. I plan on doing a patch since a long time but it's not high priority for me. So I will add the package first and do the patch when I get time.