Bug 165964

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.