Bug 188542 (hylafax)

Summary: Review Request: hylafax+
Product: [Fedora] Fedora Reporter: Lee Howard <faxguy>
Component: Package ReviewAssignee: Miroslav Suchý <msuchy>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: bnocera, bugzilla, cwickert, darren, dave, fedorabugs, itamar, jskala, kevin, lemenkov, linville, LotharLutz, msekleta, msuchy, nhorman, pahan, paul.horos, paullee0, rocketraman, trevor, zing
Target Milestone: ---Flags: msuchy: fedora‑review+
limburgher: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-12-25 23:54:13 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Attachments:
Description Flags
rpmbuild --rebuild hylafax-4.2.5.5-1.src.rpm
none
rpmlint output
none
hack let hylafax's configure succeed with gcc-4.3.0 none

Description Lee Howard 2006-04-10 20:49:30 EDT
Spec URL: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax.spec
SRPM URL: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax-4.2.5.5-1fc4.src.rpm
Description: HylaFAX(tm) is a enterprise-strength fax server supporting
Class 1 and 2 fax modems on UNIX systems. It provides spooling
services and numerous supporting fax management tools.
The fax clients may reside on machines different from the server
and client implementations exist for a number of platforms including
windows.
Comment 1 Lee Howard 2006-04-10 20:51:10 EDT
This is my first package, and I am seeking a sponsor.
Comment 2 Christoph Wickert 2006-04-11 09:02:09 EDT
There are too many issuse with your specfile to list them one by one. Just to
name a few:

- don't use "%define" for name and version
- don't use "%define fc_rel", use "disttag" instead, see
http://fedoraproject.org/wiki/DistTag
- don't use epoch if not necessary
- don't use "%define initdir     /etc/rc.d/init.d". If you really need it, it
should be "%{_initrddir}".
- Source0 needs an absolute URL (http://...)
- License field not valid, see
http://fedoraproject.org/wiki/Packaging/Guidelines#head-76294f12c6b481792eb001ba9763d95e2792e825
- remove "Packager:", see
http://fedoraproject.org/wiki/Packaging/Guidelines#head-c17fb8c1ce9be40da720a2b25d1e2a241062038f
- BuildRoot should be
"%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)", see
http://fedoraproject.org/wiki/Packaging/Guidelines#head-f196e7b2477c2f5dd97ef64e8eacddfb517f1aa1
- are you sure about the BuildRequires/BuildPrereq?
- "Requires:    rpm >= 3.0.5" ist stupid and can be dropped
- try to use "%configure" instead of "./configure" and use $RPM_OPT_FLAGS
correctly and not with make, see 
http://fedoraproject.org/wiki/Packaging/Guidelines#head-f3d77b27a5d29dfc1f5600ef3fc836f2e317badf
- no need to pass default options to configure (like PAGESIZE)
- change "--with-AWK=/usr/bin/gawk \" to "--with-AWK=%{_bindir}/gawk \"
- same for vgetty and mgetty which will become %{_sbindir}/[v|m]getty
- doesn't use parallel make, see
http://fedoraproject.org/wiki/Packaging/Guidelines#head-525c7d76890cb22df33b759c65c35c82bf434d2e
- use "%defattr(-,root,root,-)" instead of "(-,root,root)"
- remove generic INSTALL from doc section. Not needed when installed from RPM.
- macro usage inconsistent: {initdir} vs. {_initdir} which should be
%{_initrddir} anyway, see http://fedoraproject.org/wiki/Extras/RPMMacros
- empty %pre section
- /sbin/ldconfig in %post and %postun is superflurious since the package doesn't
put shared libs into the linkers path.
- "chkconfig --del" belongs into %preun, see
http://fedoraproject.org/wiki/Packaging/Guidelines#head-220f87f993c84311859884f2a033a8706a2c7d7c
- Requires(post)/(preun) missing for the scriptlets, see
http://fedoraproject.org/wiki/ScriptletSnippets?highlight=%28scriptlets%29#head-24ef9d59bda6032df14cf3cb433ce4ef09348f69
- no changelog at all, see
http://fedoraproject.org/wiki/Packaging/Guidelines#head-b7d622f4bb245300199c6a33128acce5fb453213

I'm not sure if we need to create a system user with a fixed uid/gid and use
fedora-usermgmt.
Comment 3 Christoph Wickert 2006-04-11 09:36:00 EDT
BTW: There already is an outstanding review for hylafax, See bug #145218, but it
seems like the reporter has lost interest. AFAIK metamail no longer is required
for hylafax.
Comment 4 Lee Howard 2006-04-11 09:48:56 EDT
I have seen Bug 145218.  It does appear that the reporter lost interest.  This
request should replace that one.

HylaFAX no longer has any dependency on metamail.  (It really never did, anyway.)

I'll address the SPEC file issues promptly.  Thanks.
Comment 5 Lee Howard 2006-04-11 15:30:15 EDT
Okay, I've made changes as suggested, please review the updated files:

Spec URL: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax.spec
SRPM URL:
http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax-4.2.5.5-1.src.rpm

Yes, I am sure about the BuildRequires.  HylaFAX uses libtiff development
libraries to build, and the way that it uses those libraries plus the way that
Fedora builds libtiff will also require the libjpeg-devel and zlib-devel libraries.

The %configure macro cannot be used because it doesn't work for HylaFAX. 
HylaFAX has a configure script that is not made by libtools, although it is
somewhat similar.

Some of the default options are placed in the configure command in the spec file
in cases where they will likely be variable depending on the builder's
preferences.  So, for example, PAGESIZE will likely vary between locale, and
thus it is there for easy modification by the builder.  (For those that build
from SRPM.)

Fedora puts mgetty and vgetty in /sbin and not /usr/sbin, and so %{_sbindir}
cannot be used in those cases.

I don't understand this: "I'm not sure if we need to create a system user with a
fixed uid/gid and use fedora-usermgmt." and so I don't know how to respond to it.
Comment 6 Christoph Wickert 2006-04-11 18:45:31 EDT
(In reply to comment #5)
> Okay, I've made changes as suggested, please review the updated files:

I'm going to do a formal review ASAP. From what I see most things look good now
and the package builts in mock, so builddeps are ok. Nevertheless there are a
few rpmlint warnings and errors. Let's talk about that later...

> Some of the default options are placed in the configure command in the spec file
> in cases where they will likely be variable depending on the builder's
> preferences.  So, for example, PAGESIZE will likely vary between locale, and
> thus it is there for easy modification by the builder.  (For those that build
> from SRPM.)

Ok, sounds reasonable.

> Fedora puts mgetty and vgetty in /sbin and not /usr/sbin, and so %{_sbindir}
> cannot be used in those cases.

Right, my bad. And I was wrong about ldconfig, too. Of course there are shared
libs, I did not read the file section close enough.

> I don't understand this: "I'm not sure if we need to create a system user with a
> fixed uid/gid and use fedora-usermgmt." and so I don't know how to respond to it.

To be honest: I don't understand fedora-usermgmt ether, there have been endless,
controversial discussions lately. The wiki says it's optional, so IMO we should
stick with that if everybody agrees.
http://fedoraproject.org/wiki/Packaging/UserCreation

BTW: Looking forward to see (or contribute?) capi4hylafax once this package has
been released.
Comment 7 Lee Howard 2006-04-12 13:36:01 EDT
Okay, I've returned the ldconfig to their previous places and have updated those
files listed above.

If fedora-usermgmt is optional then I opt to take the easier route and ignore
it.  :-)

As for capi4hylafax - I've never used it myself.  I'm a developer for HylaFAX
and IAXmodem, but I've never had occassion to use or work with capi4hylafax at
all.  So, unfortunately I cannot promise anything with respect to capi4hylafax,
but hopefully once HylaFAX is in Fedora Extras someone else can take that next
step for CAPI users.
Comment 8 Christoph Wickert 2006-04-12 20:43:43 EDT
I was thinking about being that "somebody eelse", but that CAPI stuff is so
horribly broken sometimes I don't dare maintaining it. ;)


REVIEW:

> $ md5sum hylafax-4.2.5.5-1.src.rpm
> 373044fd59ff14554ccda17b2fcca028  hylafax-4.2.5.5-1.src.rpm

Good - MUST Items

- package and specfile naming according to guidelines
- package meets guidelines
- license ok
- license field in spec matches actual license
- license included in source and correctly installed in %doc
- spec written in American English 
- spec is legible
- BuildRequires ok, no duplicates and none of the listed exceptions
- no locales to worry about
- ldconfig correctly called for shared libs in %post and %postun
- relocatable
- package own all directories it creates
- packages doesn't own files or directories already owned by other packages
- %files section ok, no duplicates
- permissions ok, correct %defattr
- clean section present and ok
- macro usage consistent
- code, not content
- no large docs
- %doc section ok, docs don't affect runtime
- no headers, static libs or pkgconfigs to worry about
- no libtool archives

Good - SHOULD Items
- package builds im mock (Core 5 i386)
- package seems to work as usual, nevertheless I can't really test I here right
now in absence of a modem
- scriptlets match examples from wiki
- package uses disttag



Needswork - MUST Items

- rpmlint errors and warnings:
> $ rpmlint hylafax-4.2.5.5-1.fc5.src.rpm | sort
> E: hylafax no-%clean-section
> 
This is "[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT"
confusing rpmlint. I suggest you use the regular "rm -rf $RPM_BUILD_ROOT" (don't
worry about the buildhosts ;)) or ignore this message, the %clean section in
your spec is valid.

> $ rpmlint hylafax-4.2.5.5-1.fc5.i386.rpm | sort
> E: hylafax executable-marked-as-config-file /etc/cron.daily/hylafax
> E: hylafax executable-marked-as-config-file /etc/cron.hourly/hylafax
> 
safe to ignore, but IMO the cronjobs shouldn't be "noreplace" since they 
don't store any configuration.

> E: hylafax executable-marked-as-config-file /etc/rc.d/init.d/hylafax
> 
the initscript should not be a config file. It shouldn't be "noreplace" 
ether, since it won't be replaced on updates then.

> E: hylafax explicit-lib-dependency libtiff
> 
remove libtiff from Requires, rpm will find that dependency itself

> E: hylafax invalid-soname /usr/lib/libfaxserver.so.4.2.5.5 libfaxserver.so
> E: hylafax invalid-soname /usr/lib/libfaxutil.so.4.2.5.5 libfaxutil.so
> 
safe to ignore in our case

> E: hylafax non-executable-script /var/spool/hylafax/bin/notify.awk 0444
> 
is this on purpose?

> E: hylafax non-readable /var/spool/hylafax/etc/hosts.hfaxd 0600
> 
due to ownership of uucp, safe to ignore

> E: hylafax non-standard-dir-perm /var/spool/hylafax/archive 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/docq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/doneq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/pollq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/sendq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/tmp 0700
> 
these are ok, but there are other dirs o worry about. Take a look 
at /var/spool/hylafax or /var/spool/hylafax/bin: These should be 
owned by root. IMO all dirs should be owned by root as long as they
don't need to be writable by uucp or this doesn't affect the runtime of
the program.

> E: hylafax script-without-shellbang /usr/sbin/faxsetup.linux
> E: hylafax script-without-shellbang /var/spool/hylafax/bin/dictionary
> 
These files should have start with something like "#! /bin/bash". Fix this 
upstream ;-)

> W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxserver.so
> W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxutil.so
> 
Usually these files should go into a seperate hylafax-devel package. From 
the review guidlines: "MUST: If a package contains library files with a 
suffix (e.g. libfoo.so.1.1), then library files that end in .so (without 
suffix) must go in a -devel package."
But I doubt there's much sense rolling a package with only two symlinks inside.

> W: hylafax non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps
> 
Safe to ignore.

> W: hylafax no-version-in-last-changelog
> 
append "- <version>-<release>" to every changelog entry, e.g.
* Tue Apr 11 2006 Lee Howard <faxguy  at howardsilvan dot com> - 4.2.5.5-1

> W: hylafax service-default-enabled /etc/rc.d/init.d/hylafax
> 
Please change "# chkconfig: 345 95 5" to "# chkconfig: - 95 5" in
hylafax.rh.init. see
http://fedoraproject.org/wiki/ScriptletSnippets#head-55b46ef483e6a08c24a8fc3b0b7e2ef7bfb84efd

- License: You can change the license back to "BSD-Style" or "BSD like" or
something like that, I have seen other packages even in Core with that too. To
me the COPYRIGHT looks basically BSD, but I leave it up to you. Just make sure
that the COPYRIGHT is correctly included in %doc.

- Source does not match upstream:

the one included in your rpm
> c4de725b0a2721df02880bf77809d3bd  hylafax-4.2.5.5.tar.gz
taken from the URL in Source0
> 6d9886532cbf2c21675ecb802b5ef115  ../downloads/hylafax-4.2.5.5.tar.gz
> 
The source always must match with Source0 from the spec.

- package does compile on current Core 5, I'm attaching a log. Nevertheless it
builds in Core 5 mock.

NEEDSWORK
Comment 9 Christoph Wickert 2006-04-12 20:48:04 EDT
Created attachment 127683 [details]
rpmbuild --rebuild hylafax-4.2.5.5-1.src.rpm
Comment 10 Christoph Wickert 2006-04-12 21:23:16 EDT
(In reply to comment #8)

Please forget what I wrote about the cronjobs and the initscript and leave them
as they are "%config(noreplace)".
Comment 11 Lee Howard 2006-04-27 18:37:36 EDT
Okay, I'm hoping this took care of the problem.  Now please try:

Spec URL: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax.spec
SRPM URL:
http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax-4.3.0.1-1.src.rpm
Comment 12 Christoph Wickert 2006-05-05 14:04:05 EDT
Sorry it took so long. The good thing about this is that I had enough time to
really test your package. Hylafax works prefectly here together with a
capi4hylafax package I rolled. Still I don't have a analog modem to test.

GOOD:
- Source matches upstream now
$ md5sum ~/downloads/hylafax-4.3.0.1.tar.gz (from osdn.sf.net)
30f6e56629f6a0ff40846be30a4f4ab8  /home/chris/downloads/hylafax-4.3.0.1.tar.gz
$ md5sum ~/Desktop/hylafax-4.3.0.1.tar.gz (from SRPM)
30f6e56629f6a0ff40846be30a4f4ab8  /home/chris/Desktop/hylafax-4.3.0.1.tar.gz

- License field ok
- explicit dependency on libtiff removed

BAD:
- still can't build this on my Core 5 machine, only mock succeeds. pkg-config
still looks in /usr/local/..
- rpmlint errors in detail: RPM (build in mock from your srpm):
$ rpmlint hylafax-4.3.0.1-1.fc5.i386.rpm | sort
E: hylafax executable-marked-as-config-file /etc/cron.daily/hylafax
E: hylafax executable-marked-as-config-file /etc/cron.hourly/hylafax
E: hylafax executable-marked-as-config-file /etc/rc.d/init.d/hylafax
safe to ignore

E: hylafax invalid-soname /usr/lib/libfaxserver.so.4.3.0.1 libfaxserver.so
E: hylafax invalid-soname /usr/lib/libfaxutil.so.4.3.0.1 libfaxutil.so
ok for me, I don't see a reason for changing this and for splitting out 2
symlinks into a seperate devel-package.

E: hylafax non-executable-script /var/spool/hylafax/bin/notify.awk 0444
is this on purpose? If not fix it upstream.
ok then

E: hylafax non-readable /var/spool/hylafax/etc/hosts.hfaxd 0600
E: hylafax non-standard-dir-perm /var/spool/hylafax/archive 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/docq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/doneq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/pollq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/sendq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/tmp 0700
due to ownership of uucp, safe to ignore. Nevertheless I suggest the other dirs
in /var/spool/hylafax to be owned by root.

E: hylafax script-without-shellbang /usr/sbin/faxsetup.linux
ignore or fix upstream

W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxserver.so
W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxutil.so
still no reason for a sepate -devel package IMO

W: hylafax incoherent-version-in-changelog 4.2.5.6-1 4.3.0.1-1.fc5
make sure the version field and changelog are matching. Please insert a blank
line after every changelog entry. ;)

W: hylafax non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps
safe to ignore, otherwise mark this file as %config

SRPM:
rpmlint ../hylafax-4.3.0.1-1.src.rpm
E: hylafax no-%clean-section
IMHO you should replace 
[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT
with a simple 
rm -rf $RPM_BUILD_ROOT
Your version is safer for other systems but this is a fedora package after all.

Anyway: I realized I'm not allowed to review your package. You are a first
timer, I'm not a sponsor. "The primary Reviewer can be any current package
owner, unless the Contributor is a first timer."
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines#head-e1a114b23499786e13113ebf072d03a8f8d02094

So I have added the review to the FE-NEEDSPONSOR tracker. You will have to wait
fore someone to sponsor you. Sorry, there's nothing I can do for you ATM.
Comment 13 Lee Howard 2006-05-08 09:28:54 EDT
Can you please post the Core 5 build errors?  I have built this on a Core 5
system myself before posting here, so I'm not sure what the error could be.

What is the best way to attract the attention of a sponsor?  Is waiting all that
I can do?

Thanks.
Comment 14 Christian Iseli 2006-05-08 10:49:47 EDT
(In reply to comment #13)
> What is the best way to attract the attention of a sponsor?

Basically, you need to convince one of the sponsors that:
 - you have a genuine interest in FE
 - you have a good grasp of FE packaging policies
 - you are responsive

To try to demonstrate these facts, you can:
 - offer more packages to review
 - look through other people's packages and offer useful advice on their
packaging.  You cannot formally approve a package yet, but you can help bringing
other packages in good shape for a formal approval.

> Is waiting all that I can do?

Idly waiting is not a good way to find a sponsor.
Comment 15 Christoph Wickert 2006-05-08 10:59:16 EDT
(In reply to comment #13)
> Can you please post the Core 5 build errors?

Still the same as in comment #9.

> What is the best way to attract the attention of a sponsor?  Is waiting all that
> I can do?

I'm afraid yes. You could add comments to other reviews to proof your knowledge
and understanding of the guidelines, to show you are worth being sponsored. But
IMO no one has a doubt about that.

I suggest you wait a bit. If nothing happens, feel free to ask on
fedora-extras-list.
Comment 16 Lee Howard 2006-05-12 17:48:19 EDT
Please try these once again... it should build now for you in your FC5
environment.  I had to remove %{?_smp_mflags} from the make call.

Spec URL: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax.spec
SRPM URL:
http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax-4.3.0.1-1.src.rpm

At that ... I develop and maintain both hylafax (http://hylafax.sourceforge.net)
and iaxmodem (http://iaxmodem.sourceforge.net), and I don't have the kind of
time to be spending it reviewing other projects or offering more projects than
those that I already develop and maintain - merely to attract the attention of a
needed sponsor.

I am willing and able to maintain the Fedora Core Extras HylaFAX distribution,
but I really am not in a position to do much more than that for Fedora.  If I
must do more in order to get this project sponsored, then I guess that I'm not
the right person for this project in Fedora.

Christoph, if you are able, and if you'd like to take this project you are
welcome to it.  I can assist you in whatever you may need.

Otherwise I'm pretty much going to have to wait for a sponsor - either through
patience or through nagging people on-list.

Thanks.
Comment 17 Thorsten Leemhuis 2006-05-29 01:45:56 EDT
ping -- I wanted to look at this three times but each time
http://osdn.dl.sourceforge.net/ was unreachable :-/ -- is this just me having
bad luck? If not: can you upload the files somewhere else?
Comment 18 Lee Howard 2006-05-29 01:48:51 EDT
Please try this mirror:

http://superb-west.dl.sourceforge.net/sourceforge/hylafax/hylafax-4.3.0.3-1.src.rpm
Comment 19 Thorsten Leemhuis 2006-08-06 08:50:11 EDT
Sorry, I lost track on this one. Christoph, what's the current status?
Comment 20 Christoph Wickert 2006-08-06 16:43:36 EDT
I really would like to co-maintain hylafax together with Lee. But there are
still 3 things to be done before:

1. Somebody has to review this package. As I'm not a sponsor and it's Lee's
first and most likely only package, I'm not allowed to do the review. Also I
don't think it's a good idea to review a package I'm planing to maintain.
2. Lee needs a sponsor. Thorsten, can you please sponsor him? I promise to keep
an eye on his commits ;)
3. We need an official co-maintainership policy for extras. I know you and FESCO
are working on it, but AFAIK there's nothing official yet. Please correct me if
I'm wrong.
Comment 21 Thorsten Leemhuis 2006-08-07 02:07:42 EDT
(In reply to comment #20)
> 1. Somebody has to review this package. As I'm not a sponsor and it's Lee's
> first and most likely only package, I'm not allowed to do the review. Also I
> don't think it's a good idea to review a package I'm planing to maintain.
> 2. Lee needs a sponsor. Thorsten, can you please sponsor him? I promise to keep
> an eye on his commits ;)

Can you do the review as well? Then I'll take a last quick look and sponsor him
and we both keep an eye on his commits ;-)

> 3. We need an official co-maintainership policy for extras. I know you and FESCO
> are working on it, but AFAIK there's nothing official yet. Please correct me if
> I'm wrong.

Well, it's in the works but it's a huge task and even the minimal stuff (the
initial CC-list from owners.list) currently doesn't work properly. But that
hopefully gets fixed soon
Comment 22 Christoph Wickert 2006-08-09 16:58:26 EDT
OK, I will do another formal review ASAP (I'm pretty busy ATM). There are still
some things in the specfile I'm not happy with, mainly the changelog and the
permissions of some dirs.
Comment 23 Kevin Fenzi 2006-09-02 01:53:35 EDT
Adding FE-NEEDSPONSOR blocker. 
Comment 24 Stewart Adam 2006-09-20 17:38:34 EDT
Hello,
I'm also not an official reviewer, but I ran rpmlint on the source RPM and it
returned:
E: hylafax configure-without-libdir-spec
A configure script is run without specifying the libdir. configure
options must be augmented with something like --libdir=%{_libdir}.

I think you just need to add "--libdir=%{_libdir}" to your configure command.
Comment 25 Lee Howard 2006-09-20 17:45:27 EDT
HylaFAX's configure script isn't generated by autotools and does not have a
--libdir=%{_libdir} option.  I'm quite confident that the configure call is
correct as it is already in the spec file.
Comment 26 Christoph Wickert 2006-09-20 17:53:36 EDT
Hi Howard, nice to see you are still around. Are you willing to maintain Hylafax
with me, if co-maintainership works?

I have to admit that I forgot this package, I had a lot of work with my
xfce-plugins for 4.4. I'm going to do a complete review this week, I promise.
After that Thorsten will sponsor you.
Comment 27 Lee Howard 2006-09-20 18:01:38 EDT
Yes, I am willing to maintain HylaFAX with you.
Comment 28 Michael Schwendt 2006-09-21 04:07:12 EDT
The %changelog is very out-of-date.


> %define faxspool    %{_var}/spool/hylafax

Since /var is hardcoded pretty much everywhere, e.g. in the initscript,
using %{_var} here doesn't add any safety. Just use /var unless
it were possible to propagate the value of %{_var} into all relevant
files.


> %install
> [ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT
> %clean
> [ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT

Both checks are not needed by default and are not reliable.
Simply "rm -rf $RPM_BUILD_ROOT" has been used in thousands
of packages for many many years.


> %makeinstall -e \

If %configure is not used, using %makeinstall makes no sense either.
Prefer "make install ..." just like it's correct for most other packages.


>>>
# Starting from 4.1.6, port/install.sh won't chown/chmod anymore if the current
# user is not root; instead a file root.sh is created with chown/chmod inside.
# 
# If you build the rpm as normal user (not root) you get an rpm with all the
# permissions messed up and hylafax will give various weird errors.
#
# The following line fixes that.
#
[ -f root.sh ] && sh root.sh
<<<

If this is true, there are packaging errors left somewhere. This
comment in the spec file doesn't sound right at least. The rpm must
build as normal user and must not rely on chown/chmod. Make sure the
%attr(...) settings are complete for any files that really need them
and without depending on any execution of chown/chmod.

*If*, however, (and I believe you do) you only intended to justify
why "root.sh" must be executed when installing Hylafax from tarball
as non-root user, the comment is just misleading/confusing.


> $ rpm -qlvp hylafax-debuginfo-4.3.0.3-1.i386.rpm 
> (contains no files)

This is because in root.sh (and maybe elsewhere, too) the executables
are stripped, which should not be done, and which makes the debuginfo
package useless.


> drwxr-xr-x uucp  uucp         0 /var/spool/hylafax/bin
> -rw-r--r-- uucp  uucp     14072 /var/spool/hylafax/etc/lutRS18.pcf

Wouldn't root:root ownership suffice?
Comment 30 Christoph Wickert 2006-10-24 10:16:32 EDT
Sorry it took so lang once again.

There are still some issues with this package that I don't think it makes much
sense to do a review at this point. The main reason is that SRPM and Spec from
comment #29 don't match (see below).

1. debug package is still empty:
$ rpm -qpl hylafax-debuginfo-4.3.0.11-1.fc6.i386.rpm
(contains no files)

Why is there no debug info? If there really is none the debuginfo package should
not be built. Please see
https://www.redhat.com/archives/fedora-packaging/2006-October/msg00149.html
for more info.

2. Changelog is still out of date. Please update the changelog for all releases
and describe the changes you made. This makes it easier for us to track changes.
In order to avoid confusion please increase the release for every new package,
even during the review. 

If we are going to maintain this package together I'd like you to move the
changelog to the end of the spec and insert a blank line between every entry for
legibility. 

3. Ownership of /var/spool/hylafax/bin is still uucp:uucp
$ ls -l /var/spool/hylafax/
...
drwxr-xr-x 2 uucp uucp 4096 21. Okt 00:33 bin
...
This is already fixed in the specfile, but the SRPM is not up to date.

We are reviewing SRPMS, not specs, so I can only do a review if you update your
package and fix these errors.

I still would like root to own more dirs in /var/spool/hylafax, e. g. config and
dev.
Comment 31 Lee Howard 2006-11-01 18:23:40 EST
Christoph, thank you very much for your time.  I have updated the spec file and
the srpm and they match each other this time, too.  I've made changes as you
have suggested.

http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax.spec
http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax-5.0.0-1.src.rpm
Comment 32 Paul Horos 2006-12-05 13:05:00 EST
Can you please clarify whether this package is HylaFAX or HylaFAX+?
Comment 33 Lee Howard 2006-12-05 13:36:49 EST
Paul/Darren,

Lest we require ourselves to say that x.org and XFree86 are not both X, let us
not try to define that HylaFAX+ and even your own company's "HylaFAX
Professional Edition" are not both HylaFAX also.

Indeed, as we all (including you) already know, the software being promoted here
is the sourceforge flavour of HylaFAX, also known as HylaFAX+.

Lee.
Comment 34 Christoph Wickert 2006-12-05 14:34:52 EST
Sorry for the long delay.

Howard, the new package doesn't work at all, the permissions are completely
screwed up. Have you tested the package yourself before you submitted it?

$ rpmlint hylafax-5.0.0-1.fc6.i386.rpm | sort
E: hylafax executable-marked-as-config-file /etc/cron.daily/hylafax
E: hylafax executable-marked-as-config-file /etc/cron.hourly/hylafax
E: hylafax executable-marked-as-config-file /etc/rc.d/init.d/hylafax
E: hylafax invalid-soname /usr/lib/libfaxserver.so.5.0.0 libfaxserver.so
E: hylafax invalid-soname /usr/lib/libfaxutil.so.5.0.0 libfaxutil.so
E: hylafax non-executable-script /usr/sbin/edit-faxcover 0444
E: hylafax non-executable-script /usr/sbin/faxaddmodem 0444
E: hylafax non-executable-script /usr/sbin/faxcron 0444
E: hylafax non-executable-script /usr/sbin/faxsetup 0444
E: hylafax non-executable-script /usr/sbin/faxsetup.linux 0644
E: hylafax non-executable-script /usr/sbin/hylafax 0444
E: hylafax non-executable-script /usr/sbin/probemodem 0444
E: hylafax non-executable-script /usr/sbin/recvstats 0444
E: hylafax non-executable-script /usr/sbin/xferfaxstats 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/archive 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/common-functions 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/dictionary 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/faxrcvd 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/mkcover 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/notify 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/pcl2fax 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/pdf2fax.gs 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/pollrcvd 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/ps2fax.gs 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/tiff2fax 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/tiff2pdf 0444
E: hylafax non-executable-script /var/spool/hylafax/bin/wedged 0444
E: hylafax script-without-shebang /usr/sbin/faxsetup.linux
W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxserver.so
W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxutil.so
W: hylafax incoherent-version-in-changelog 5.0.0 5.0.0-1.fc6
W: hylafax non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps
W: hylafax non-executable-in-bin /usr/sbin/edit-faxcover 0444
W: hylafax non-executable-in-bin /usr/sbin/faxaddmodem 0444
W: hylafax non-executable-in-bin /usr/sbin/faxcron 0444
W: hylafax non-executable-in-bin /usr/sbin/faxsetup 0444
W: hylafax non-executable-in-bin /usr/sbin/faxsetup.linux 0644
W: hylafax non-executable-in-bin /usr/sbin/hylafax 0444
W: hylafax non-executable-in-bin /usr/sbin/probemodem 0444
W: hylafax non-executable-in-bin /usr/sbin/recvstats 0444
W: hylafax non-executable-in-bin /usr/sbin/xferfaxstats 0444
Comment 35 Paul Horos 2006-12-05 14:43:05 EST
Christoph,

As the proposed package is HylaFAX+, I'd request that it be renamed such.

Thanks,

Paul
Comment 36 Lee Howard 2006-12-05 14:58:06 EST
> Have you tested the package yourself before you submitted it?

Yes, I've been using it repeatedly.  Here's what I see when I run rpmlint on the
built RPM:

[root@dhcp031 i386]# rpmlint hylafax-5.0.1-1.i386.rpm
W: hylafax incoherent-version-in-changelog 5.0.0 5.0.1-1
E: hylafax invalid-soname /usr/lib/libfaxutil.so.5.0.1 libfaxutil.so
E: hylafax invalid-soname /usr/lib/libfaxserver.so.5.0.1 libfaxserver.so
E: hylafax non-readable /var/spool/hylafax/etc/hosts.hfaxd 0600
E: hylafax executable-marked-as-config-file /etc/cron.hourly/hylafax
E: hylafax script-without-shebang /usr/sbin/faxsetup.linux
E: hylafax executable-marked-as-config-file /etc/cron.daily/hylafax
E: hylafax non-standard-dir-perm /var/spool/hylafax/archive 0700
W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxutil.so
E: hylafax non-standard-dir-perm /var/spool/hylafax/doneq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/sendq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/tmp 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/docq 0700
W: hylafax non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps
W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxserver.so
E: hylafax non-standard-dir-perm /var/spool/hylafax/pollq 0700
E: hylafax executable-marked-as-config-file /etc/rc.d/init.d/hylafax
Comment 37 Christoph Wickert 2006-12-05 17:04:12 EST
(In reply to comment #35)
> Christoph,
> 
> As the proposed package is HylaFAX+, I'd request that it be renamed such.

I agree. I think the source should be named hylafax+<version>.tar.gz, too.

(In reply to comment #36)
> > Have you tested the package yourself before you submitted it?
> 
> Yes, I've been using it repeatedly.  

How have you been using this package if /usr/sbin/hylafax is not executable?

>Here's what I see when I run rpmlint on the built RPM:
> 
> [root@dhcp031 i386]# rpmlint hylafax-5.0.1-1.i386.rpm

This is obviously not the same package, not the same release. It's not even the
same version as the package you have submitted in comment #31.
Comment 38 Lee Howard 2006-12-05 17:38:53 EST
> This is obviously not the same package, not the same release. It's not even
> the same version as the package you have submitted in comment #31.

The software development is moving much faster than progress on this review
request.  I apologize for giving you rpmlint output for an RPM that was more
conveniently at my disposal.  For your benefit, I have downloaded the SRPM given
in comment #31, rebuilt it on FC6, and here is the rpmlint output:

[root@dhcp031 i386]# rpmlint hylafax-5.0.0-1.i386.rpm
W: hylafax incoherent-version-in-changelog 5.0.0 5.0.0-1
E: hylafax invalid-soname /usr/lib/libfaxutil.so.5.0.0 libfaxutil.so
E: hylafax invalid-soname /usr/lib/libfaxserver.so.5.0.0 libfaxserver.so
W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxserver.so
E: hylafax non-readable /var/spool/hylafax/etc/hosts.hfaxd 0600
E: hylafax executable-marked-as-config-file /etc/cron.hourly/hylafax
E: hylafax script-without-shebang /usr/sbin/faxsetup.linux
E: hylafax executable-marked-as-config-file /etc/cron.daily/hylafax
E: hylafax non-standard-dir-perm /var/spool/hylafax/archive 0700
W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxutil.so
E: hylafax non-standard-dir-perm /var/spool/hylafax/doneq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/sendq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/tmp 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/docq 0700
W: hylafax non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps
E: hylafax non-standard-dir-perm /var/spool/hylafax/pollq 0700
E: hylafax executable-marked-as-config-file /etc/rc.d/init.d/hylafax
[root@dhcp031 i386]#

> I agree. I think the source should be named hylafax+<version>.tar.gz, too.

Apache distributes its webserver in a source repository named
httpd-2.2.3.tar.gz.  Following the suggestions here, we should petition them to
change their package name to something more specific to their version, like
"apache-httpd".  Apache's naming convention makes complete sense to me, and
undoubtedly I am not alone in this understanding as they have had it named that
way for a very long time.

Fedora uses the repository name as the source for the httpd package name.  Thus
Apache's webserver is found in a package named "httpd".  However, other
distributions of Apache's webserver are found in packages named differently,
such as "apache-httpd".  This also makes complete sense to me because it
provides the distribution a means to differentiate between different http
servers that it may provide.  I do not know if Fedora provides webservers other
than Apache's, but assuming it does not, then using the package name of "httpd"
for Fedora makes complete sense as well, since it is the only http server being
provided.

The upstream repository will remain named as it is.  As for the package name, it
matters not to me if it is called "hylafax" or "hylafax+".  However, my
suggestion would follow what I've said above about the Apache http server.  The
distinction of the "+" will mean very little to Fedora users (and in-fact may
make the package more-difficult to identify) unless there is more than one
HylaFAX package being distributed by Fedora (say, for example, separate
"hylafax+" and "hylafax.org" packages).
Comment 39 Christoph Wickert 2006-12-05 18:38:36 EST
(In reply to comment #38)
> The software development is moving much faster than progress on this review
> request.

I am very sorry about that. Please try to see it from my point of view: One
reason for this review proceeding so slow is that it's so confusing: Packages
don't match the spec file, there was hardly any changelog information at the
beginning, lots of rpmlint errors, ...

> I apologize for giving you rpmlint output for an RPM that was more
> conveniently at my disposal.  For your benefit, I have downloaded the SRPM given
> in comment #31, rebuilt it on FC6, and here is the rpmlint output:
> 
> [root@dhcp031 i386]# rpmlint hylafax-5.0.0-1.i386.rpm
> W: hylafax incoherent-version-in-changelog 5.0.0 5.0.0-1

This is an easy one. Why not fix it _before_ submitting the package? As I
already said: Keeping the changelog up to date makes it easier for me to follow
the changes.

> E: hylafax invalid-soname /usr/lib/libfaxutil.so.5.0.0 libfaxutil.so
> E: hylafax invalid-soname /usr/lib/libfaxserver.so.5.0.0 libfaxserver.so
> W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxserver.so
> E: hylafax non-readable /var/spool/hylafax/etc/hosts.hfaxd 0600
> E: hylafax executable-marked-as-config-file /etc/cron.hourly/hylafax
> E: hylafax script-without-shebang /usr/sbin/faxsetup.linux
> E: hylafax executable-marked-as-config-file /etc/cron.daily/hylafax
> E: hylafax non-standard-dir-perm /var/spool/hylafax/archive 0700
> W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxutil.so
> E: hylafax non-standard-dir-perm /var/spool/hylafax/doneq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/sendq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/tmp 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/docq 0700
> W: hylafax non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps
> E: hylafax non-standard-dir-perm /var/spool/hylafax/pollq 0700
> E: hylafax executable-marked-as-config-file /etc/rc.d/init.d/hylafax
> [root@dhcp031 i386]#

Strange. Mine looks different, see comment #34. I have been building this
package several times locally and in mock, always with the same results: Files
under /usr/sbin are not executable.

Can you upload your binaries somewhere?

> > I agree. I think the source should be named hylafax+<version>.tar.gz, too.
> 
> Apache distributes its webserver in a source repository named
> httpd-2.2.3.tar.gz.  Following the suggestions here, we should petition them to
> change their package name to something more specific to their version, like
> "apache-httpd".  Apache's naming convention makes complete sense to me, and
> undoubtedly I am not alone in this understanding as they have had it named that
> way for a very long time.
> 
> Fedora uses the repository name as the source for the httpd package name.

This is Core, not Extras. Packages in Core don't necessarily follow the FE
Packaging and Naming Guidelines and haven't got through a review.

The apache package doesn't follow the naming guidelines. It not on me to judge
if it makes sense or doesn't, but picking out an exception from the rule is not
a good reason for making more exceptions from that rule.

> The upstream repository will remain named as it is.  

I don't have to judge on this too, but IMO this is bad: Having two source
archives with the same name and potentially even the same version, but with
different content inside is confusing. Once downloaded it is very hard for
people to distinguish which version they have.

> As for the package name, it
> matters not to me if it is called "hylafax" or "hylafax+".  However, my
> suggestion would follow what I've said above about the Apache http server.  The
> distinction of the "+" will mean very little to Fedora users (and in-fact may
> make the package more-difficult to identify) unless there is more than one
> HylaFAX package being distributed by Fedora (say, for example, separate
> "hylafax+" and "hylafax.org" packages).

Maybe someone else one day will submit another review for hylafax(.org).
Comment 40 Lee Howard 2006-12-05 19:23:39 EST
Christoph,

I understand the confusion on this review.  I am truly sorry for it, and I wish
that I could have somehow known ahead of time how to prevent the confusion...
because I certainly would have.

The mismatches between packages and specs and such has to do with the
development pace and my focus on software development rather than RPM packaging,
so I guess it's a chicken-and-egg kind of problem.

I've uploaded the hylafax-5.0.0-1.i386.rpm file that I was using to here:

http://superb-east.dl.sourceforge.net/sourceforge/hylafax/hylafax-5.0.0-1test.i386.rpm

Yes, I know the filename is different - that could not be avoided.  But the file
data is the same.

As far as package/repository naming goes... I understand the httpd naming
manner, and I completely understand why it is named that way.  Certainly it may
not meet the Extras criteria naming rules - neverless, it still makes sense to
me and is not confusing, and in fact I probably would have followed the same
naming convention in their shoes.  I do not see it as an exception to
common-sense - although, yes, it may be an exception according to Extras naming
rules.  Certainly the Extras rules can be a subset of common-sense.

For other examples - not of package naming, specifically - but for naming in
general... postfix and sendmail both have "sendmail" executables (among other
competing executable names).  Similarly, mgetty-sendfax has a "sendfax"
executable that competes with an identically named executable from HylaFAX
(which is why HylaFAX isn't in Core in the first place, as the RedHat 5.2/6.0
maintainers decided to favor mgetty-sendfax and do away with HylaFAX rather than
implement a "switching" mechanism as they have done for sendmail/postfix).  All
of this makes sense to me - and indeed I can see why it would confuse some - but
if one understands that, realistically, the purpose in the naming conflicts is a
perfect manner of clue-sticking the user that they're looking at conflicting
packages, just the same as if they were looking at two packages from the same
repository but of different versions.

The HylaFAX+ repository is aptly named "hylafax" because it is, after all,
HylaFAX.  HylaFAX+ version numbers have always been different from the version
numbers at HylaFAX.org.  Certainly it is not the only HylaFAX repository, but
realize that the hylafax.org repository is, itself, a fork - there almost always
have been different repositories (even among the earliest contributors).  To say
that HylaFAX+ is not HylaFAX is to say that when Alan Cox patches the Linux
kernel for RedHat that it no longer is Linux.

The sourceforge HylaFAX project is known as HylaFAX+ for those people that have
a tough time understanding the issues that I am discussing, and certainly it
makes it easier than always saying "Sourceforge HylaFAX project".  That said,
you really won't find anyone out there desireous to run both HylaFAX+ and
HylaFAX.org for practical reasons.

Realize that Darren's (Paul's) purpose here isn't really to assist the users who
will be using HylaFAX (in that they may become upset to find themselves using
HylaFAX+ instead of HylaFAX.org software).  Rather, his purpose here is to take
measures to prevent users from seeing, as I do, that HylaFAX+ is as much HylaFAX
as the software found at HylaFAX.org or SGI-HylaFAX or his own commercial
"HylaFAX Enterprise Edition".  If he really, truly, believed what he is trying
to say here then he wouldn't have named his own product with "HylaFAX".

My suggestion, Christoph, would be to see the HylaFAX+ vs. HylaFAX.org thing for
what it is, the usual forking arguments, and move past it so that we can get
this into Extras ... whether the ultimate package name be "hylafax" or "hylafax+".
Comment 41 Paul Horos 2006-12-06 11:15:29 EST
> As for the package name, it matters not to me if it is called "hylafax" or
"hylafax+".  

There does not seem to be any objection to renaming the package to be included
in Fedora Extras "hylafax+", so that has answered my question and addressed my
concern. Not only would it help prevent possible confusion between different
software projects, it would be consistent with Lee's own sourceforge website and
mailing lists. I agree that it's important to distinguish this package from the
hylafax software that has been available from http://www.hylafax.org since 1997,
especially in the event of it's submission to Fedora Extras. Whether hylafax+ is
a fork has been discussed ad nauseum on the hylafax mailing list; if Lee would
like to discuss it further that seems to be a more appropriate place than this
ticket.

Paul
Comment 42 Lee Howard 2007-01-27 16:10:09 EST
I've updated the SPEC and the SRPM:

http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax.spec
http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax-5.0.4-1.src.rpm

rpmlint gives me these warnings/errors:

E: hylafax invalid-soname /usr/lib/libfaxutil.so.5.0.4 libfaxutil.so
E: hylafax invalid-soname /usr/lib/libfaxserver.so.5.0.4 libfaxserver.so
W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxserver.so
E: hylafax non-readable /var/spool/hylafax/etc/hosts.hfaxd 0600
E: hylafax executable-marked-as-config-file /etc/cron.hourly/hylafax
E: hylafax subdir-in-bin /usr/sbin/faxmail/image
E: hylafax script-without-shebang /usr/sbin/faxsetup.linux
E: hylafax executable-marked-as-config-file /etc/cron.daily/hylafax
E: hylafax subdir-in-bin /usr/sbin/faxmail/application/octet-stream
E: hylafax subdir-in-bin /usr/sbin/faxmail/image/tiff
E: hylafax non-standard-dir-perm /var/spool/hylafax/archive 0700
E: hylafax subdir-in-bin /usr/sbin/faxmail/application/pdf
W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxutil.so
E: hylafax non-standard-dir-perm /var/spool/hylafax/doneq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/sendq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/tmp 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/docq 0700
E: hylafax subdir-in-bin /usr/sbin/faxmail/application
W: hylafax non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps
E: hylafax non-standard-dir-perm /var/spool/hylafax/pollq 0700
E: hylafax executable-marked-as-config-file /etc/rc.d/init.d/hylafax

Please let me know what you would like me to do from here.
Comment 43 Christoph Wickert 2007-01-27 16:21:02 EST
Thanks Howard, I will look at the files later this weekend, I promise.

There was something seriously wrong with you latest package (the ones I built
and the binary you gave me), the permissions were messed up and files were not
executable, the %attr statements don't work as expected. I'll need to
investigate this further. Please stay tuned.
Comment 44 Kevin Fenzi 2007-05-30 23:53:53 EDT
Hey Christopher. I am assuming you are reviewing this package... 
I am setting fedora-review flag to ?
If this is incorrect, please set it back to " " and reassign to
nobody@fedoraproject.org.
Comment 45 Jason Tibbitts 2007-06-30 17:05:42 EDT
This seems to be stalled completely.
 
Christoph: Are you still interested in reviewing this?

Lee: Are you still interested in submitting this?
Comment 46 Lee Howard 2007-06-30 17:26:54 EDT
Hello Jason.  Yes, I am still very interested in submitting this.  I'm not quite
sure what the stall is for.  Getting packages into Fedora Extras seems quite
impossible :-)
Comment 47 Jason Tibbitts 2007-06-30 18:17:55 EDT
Well, judging from the quantity of packages that get in every day, I'd say it's
far from impossible but for a first-time maintainer you sure took on a really
tough package that has needed a ton of work.  And this package still has quite a
few issues that need to be worked out, as I get nearly 400 complaints from rpmlint.

So I guess it's up to Christoph at this point to complete his review.
Comment 48 Lee Howard 2007-06-30 18:59:22 EDT
Jason, would you mind sharing your rpmlint output?  As discussed here, my
rpmlint output looks as expected, relatively clean.
Comment 49 Jason Tibbitts 2007-06-30 19:15:02 EDT
Created attachment 158304 [details]
rpmlint output

No problem; here's the full output from my package build scripts.
Comment 50 Christoph Wickert 2007-06-30 22:12:27 EDT
(In reply to comment #45)
>  
> Christoph: Are you still interested in reviewing this?

Yes, but this really is a hard one, especially as there is confusion (about the
name) and regressions. As I said before, the 2 latest packages don't work at all
because of the not executable files. 

(In reply to comment #46)
> Getting packages into Fedora Extras seems quite impossible :-)

Lee,
I'm very sorry about that, of course this is partly my fault. On the other hand:
The binary package you gave me in comment #40 simply doesn't work and I don't
think this is because of machine but of the package itself.

I promise (once again) to look at this more deeply tomorrow.
Comment 51 Christoph Wickert 2007-07-01 20:32:45 EDT
Ok, lets talk about the errors that need to be fixed upstream first:

E: hylafax binary-or-shlib-defines-rpath /usr/bin/faxalter ['/usr/lib64']:
Same for all other binaries. This is a no-go for fedora, see
http://fedoraproject.org/wiki/Packaging/Guidelines#head-a1dfb5f46bf4098841e31a75d833e6e1b3e72544
Although we could fix this in the spec I think it should be done properly upstream.

E: hylafax subdir-in-bin /usr/sbin/faxmail
Same for the subdirs of /usr/sbin/faxmail. IMO a no-go to because it is a
violation of the FHS. Should be in /usr/lib/ I guess.

E: hylafax invalid-soname /usr/lib64/libfaxserver.so.5.0.4 libfaxserver.so
E: hylafax invalid-soname /usr/lib64/libfaxutil.so.5.0.4 libfaxutil.so
this is related to
W: hylafax devel-file-in-non-devel-package /usr/lib64/libfaxserver.so
W: hylafax devel-file-in-non-devel-package /usr/lib64/libfaxutil.so
I brought this up on fedora-extras-list back in October 2006, see
https://www.redhat.com/archives/fedora-extras-list/2006-October/msg00496.html
and especially Michaels mails
https://www.redhat.com/archives/fedora-extras-list/2006-October/msg00505.html
https://www.redhat.com/archives/fedora-extras-list/2006-October/msg00508.html

W: hylafax undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.0.4
_Z11vlogWarningPKcP13__va_list_tag
Much more undefined symbols, need to be fixed too.

W: hylafax unused-direct-shlib-dependency /usr/lib64/libfaxserver.so.5.0.4
/lib64/libcrypt.so.1
Same for a couple more libs and same with libfaxutil too.

Sorry, I did not have the time to look into the spec more deeply today, but I
think we should focus on fixing these first because the packaging bugs are easier.

If you have a questions about a specific rpmlint error, run "rpmlint -I <error>".
Comment 52 Lee Howard 2007-07-12 16:38:21 EDT
I've fixed the rpath matter.  (HylaFAX source-builds allow the user to specify a
non-standard library location.  If the user specified anything other than
/usr/lib then rpath would be used for portability purposes.  This didn't take
64-bit systems with /usr/lib64 into account, and so I've now modified the source
repository upstream to not use rpath when /usr/lib64 is the library location. 
It will be in the next release.)

I've moved the /usr/sbin/faxmail items to /usr/lib/fax/faxmail instead.

I've removed the DSO symlinks and have linked against the versioned DSO files
directly.

I honestly don't understand the undefined-non-weak-symbol warnings.  Is this
something that's apparently only happening on 64-bit?

I think that I've remedied the unused-direct-shlib-dependency warnings.  I'll
have to double check that.

More to come...
Comment 53 Lee Howard 2007-07-14 16:09:31 EDT
After making those changes here is the rpmlint output that I see on an x86_64
system (I'll hang a new SRPM and SPEC file soon and will give the URLs here when
I do)...

(Yet, I'm *still* not seeing any undefined-non-weak-symbol errors.)

[root@dhcp006 SPECS]# rpmlint
/usr/src/redhat/RPMS/x86_64/hylafax-5.1.6-1.x86_64.rpm | sort
E: hylafax executable-marked-as-config-file /etc/cron.daily/hylafax
E: hylafax executable-marked-as-config-file /etc/cron.hourly/hylafax
E: hylafax executable-marked-as-config-file
/etc/hylafax/faxmail/application/octet-stream
E: hylafax executable-marked-as-config-file /etc/hylafax/faxmail/application/pdf
E: hylafax executable-marked-as-config-file /etc/hylafax/faxmail/image/tiff
E: hylafax executable-marked-as-config-file /etc/rc.d/init.d/hylafax
E: hylafax non-readable /var/spool/hylafax/etc/hosts.hfaxd 0600
E: hylafax non-standard-dir-perm /var/spool/hylafax/archive 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/docq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/doneq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/pollq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/sendq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/tmp 0700
E: hylafax script-without-shebang /usr/sbin/faxsetup.linux
W: hylafax non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps
Comment 54 Christoph Wickert 2007-07-15 18:26:37 EDT
(In reply to comment #52)
> 
> I honestly don't understand the undefined-non-weak-symbol warnings.  Is this
> something that's apparently only happening on 64-bit?

Yes, your guess was correct. 

(In reply to comment #53)
> After making those changes here is the rpmlint output that I see on an x86_64
> system (I'll hang a new SRPM and SPEC file soon and will give the URLs here when
> I do)...

Make sure to include the following changes to fix the permission issues:

144,145c144,145
< %{_bindir}/*
< %{_sbindir}/*
---
> %attr(755,root,root) %{_bindir}/*
> %attr(755,root,root) %{_sbindir}/*
171c171
< %attr(-,root,root) %{faxspool}/bin/*
---
> %attr(755,root,root) %{faxspool}/bin/


> (Yet, I'm *still* not seeing any undefined-non-weak-symbol errors.)

run "rpmlint hylafax" on the installed package
Comment 55 Lee Howard 2007-07-17 14:51:45 EDT
Okay, here's where I'm at.  I won't post another SRPM and SPEC file just yet...
let's see if you can help me through these last rpmlint warnings/errors or if we
can agree that they're not blockers.

[root@dhcp006 SPECS]# rpmlint /usr/src/redhat/SRPMS/hylafax-5.1.6-1.src.rpm
E: hylafax configure-without-libdir-spec

HylaFAX's configure is not a direct derivative from the autoconf versions, and
thus there is no --libdir= option for HylaFAX's configure.

[root@dhcp006 SPECS]# rpmlint
/usr/src/redhat/RPMS/x86_64/hylafax-5.1.6-1.x86_64.rpm | sort
E: hylafax executable-marked-as-config-file /etc/cron.daily/hylafax
E: hylafax executable-marked-as-config-file /etc/cron.hourly/hylafax
E: hylafax executable-marked-as-config-file /etc/rc.d/init.d/hylafax

I'm not sure what to do about these, exactly.  These are all executable scripts
that are meant to be configurable by the administrator, and we don't want to
overwrite the configured script on upgrades.

E: hylafax non-readable /var/spool/hylafax/etc/hosts.hfaxd 0600

This is intentional.  Only the owner should be able to read this file.

E: hylafax non-standard-dir-perm /var/spool/hylafax/archive 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/docq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/doneq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/pollq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/sendq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/tmp 0700

Again, these are intentional.  Only the owner should be accessing these
directories.  (All activity goes through the server.  Local users don't access
these files directly.)

E: hylafax script-without-shebang /usr/sbin/faxsetup.linux

This file, faxsetup.linux, is a shell script stub.  It is included (via ".")
from the invoked faxsetup script.

W: hylafax non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps

While it is true that the example coversheet is not itself a configuration file,
it is meant to be configured - i.e. replaced, and it belongs there.

[root@dhcp006 SPECS]# rpmlint hylafax | sort
E: hylafax executable-marked-as-config-file /etc/cron.daily/hylafax
E: hylafax executable-marked-as-config-file /etc/cron.hourly/hylafax
E: hylafax executable-marked-as-config-file /etc/rc.d/init.d/hylafax
E: hylafax non-readable /var/spool/hylafax/etc/hosts.hfaxd 0600
E: hylafax non-standard-dir-perm /var/spool/hylafax/archive 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/docq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/doneq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/pollq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/sendq 0700
E: hylafax non-standard-dir-perm /var/spool/hylafax/tmp 0700
E: hylafax script-without-shebang /usr/sbin/faxsetup.linux
W: hylafax non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps

We've covered all of these already above.

W: hylafax unused-direct-shlib-dependency /usr/lib64/libfaxserver.so.5.1.6
/lib64/libm.so.6
W: hylafax unused-direct-shlib-dependency /usr/lib64/libfaxutil.so.5.1.6
/lib64/libm.so.6

These two are particularly curious to me, and I cannot figure them out.  Here
are the corresponding build lines:

/usr/bin/g++ -shared -fpic -Wl,-soname,libfaxserver.so.5.1.6 -o
libfaxserver.so.5.1.6 \
            faxApp.o FaxItem.o FaxRequest.o FaxAcctInfo.o FaxMachineInfo.o
FaxMachineLog.o FaxPoll.o FaxRecv.o FaxSend.o ModemServer.o FaxServer.o
UUCPLock.o ServerConfig.o ClassModem.o FaxModem.o Class0.o Class1.o Class10.o
Class1Ersatz.o Class1Poll.o Class1Recv.o Class1Send.o Class2.o Class20.o
Class21.o Class2Ersatz.o Class2Poll.o Class2Recv.o Class2Send.o CopyQuality.o
G3Decoder.o G3Encoder.o MemoryDecoder.o HDLCFrame.o ModemConfig.o NSF.o
FaxFont.o PCFFont.o TagLine.o  ../util/libfaxutil.so.5.1.6  -ltiff

/usr/bin/g++ -shared -fpic -Wl,-soname,libfaxutil.so.5.1.6 -o
libfaxutil.so.5.1.6 Array.o BoolArray.o Dictionary.o Obj.o PageSize.o RE.o
REArray.o REDict.o StackBuffer.o Str.o StrArray.o StrDict.o Dispatcher.o
IOHandler.o Sys.o SystemLog.o Timeout.o Fatal.o AtSyntax.o DialRules.o FmtTime.o
Sequence.o TimeOfDay.o FaxDB.o TextFormat.o Class2Params.o FaxParams.o
FaxClient.o FaxConfig.o FaxRecvInfo.o FaxSendInfo.o JobExt.o CallID.o ModemExt.o
SendFaxJob.o SendFaxClient.o TypeRules.o Transport.o InetTransport.o
UnixTransport.o SNPPClient.o SNPPJob.o cvtfacility.o fxassert.o \
             -ltiff  -lz  -L../regex -lregex

Please notice that -lm is not being used here as the rpmlint warning message
seems to indicate.  Now, -lm is used elsewhere in the build, but not for these
DSOs, but even if I remove the -lm from everywhere in the build (it seems
unnecessary) this rpmlint warning still occurs.  The header file math.h is
included in a few source files, but not in any source file that is used to build
libfaxutil.  So something is very confusing to me here.  Any insight would be
appreciated.

Thanks.
Comment 56 Christoph Wickert 2007-07-17 15:43:39 EDT
(In reply to comment #55)
> [root@dhcp006 SPECS]# rpmlint /usr/src/redhat/SRPMS/hylafax-5.1.6-1.src.rpm
> E: hylafax configure-without-libdir-spec
> 
> HylaFAX's configure is not a direct derivative from the autoconf versions, and
> thus there is no --libdir= option for HylaFAX's configure.

Correct, ignore the warning.

> [root@dhcp006 SPECS]# rpmlint
> /usr/src/redhat/RPMS/x86_64/hylafax-5.1.6-1.x86_64.rpm | sort
> E: hylafax executable-marked-as-config-file /etc/cron.daily/hylafax
> E: hylafax executable-marked-as-config-file /etc/cron.hourly/hylafax
> E: hylafax executable-marked-as-config-file /etc/rc.d/init.d/hylafax

Safe to ignore as I already said in comment #12

> E: hylafax non-readable /var/spool/hylafax/etc/hosts.hfaxd 0600

Ignore, see comment #12

> E: hylafax non-standard-dir-perm /var/spool/hylafax/archive 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/docq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/doneq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/pollq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/sendq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/tmp 0700

Ignore

> E: hylafax script-without-shebang /usr/sbin/faxsetup.linux
> 
> This file, faxsetup.linux, is a shell script stub.  It is included (via ".")
> from the invoked faxsetup script.

Ok, ignore.

> W: hylafax non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps
> 
> While it is true that the example coversheet is not itself a configuration file,
> it is meant to be configured - i.e. replaced, and it belongs there.

Ok.

> [root@dhcp006 SPECS]# rpmlint hylafax | sort
> W: hylafax unused-direct-shlib-dependency /usr/lib64/libfaxserver.so.5.1.6
> /lib64/libm.so.6
> W: hylafax unused-direct-shlib-dependency /usr/lib64/libfaxutil.so.5.1.6
> /lib64/libm.so.6
> 
> These two are particularly curious to me, and I cannot figure them out. 

ldd -r -u /usr/lib64/libfaxserver.so.5.0.4
-- snipped _lots_ of undefined symbols --
Unused direct dependencies:
        /usr/lib64/libjpeg.so.62
        /lib64/libz.so.1
        /lib64/libcrypt.so.1
        /lib64/libutil.so.1
        /lib64/libm.so.6

I haven't looked at this deeper yet, it is not necessarily a blocker, see
http://www.redhat.com/archives/fedora-maintainers/2006-June/msg00176.html

Could you please take a look at the undefined symbols again? As there are no
major blockers you can submit a new srpm for review.

Hope we aren't getting any SELinux troubles. What Fedora Version are you running
with hylafax? Do you have SELinux enabled?
Comment 57 Paul Horos 2007-07-20 11:38:10 EDT
(In reply to comment #37)
> (In reply to comment #35)
> I agree. I think the source should be named hylafax+<version>.tar.gz, too.

Christoph, since you are in agreement with changing the name to hylafax+ and so
is Lee:

> My suggestion, Christoph, would be to see the HylaFAX+ vs. HylaFAX.org thing for
> what it is, the usual forking arguments, and move past it so that we can get
> this into Extras ... whether the ultimate package name be "hylafax" or "hylafax+".

When do you think this will be done? We're looking at submitting HylaFAX for
review but would like to clear up this confusion first.

Thanks,

Paul
Comment 58 Christoph Wickert 2007-07-20 18:19:35 EDT
We are workig on it right now and I'm optimistic, that we can finish this review
soon.

Howard: Please upload the SRPM you send me some days ago to a location where it
is accessible for everybody so others can participate in the review, too.
Remember: Fedora is about openness, so everything should happen in public.
Comment 59 Lee Howard 2007-07-20 18:41:10 EDT
Christoph, I'm in complete agreement about the openness.  Understand that in
order to put public the SRPM and SPEC file the same as I have in the past that I
have to cut a new release upstream.  Because there have been some rather serious
changes to the DSO build on Linux I have to take some time to test on non-Fedora
Linux distros.  Please allow me some time (a few days) to do that.  Thanks.
Comment 60 Lee Howard 2007-07-21 14:42:34 EDT
Okay, I've uploaded new spec and src.rpm files.  They can be gotten here:

http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax.spec
http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax-5.1.6-1.src.rpm
Comment 61 Thorsten Leemhuis 2007-11-20 01:18:54 EST
Christoph, are you still interested in this? Seems this takes much to much time;
see also https://bugzilla.rpmfusion.org/show_bug.cgi?id=17

Maybe someone else is willing to review this?
Comment 62 Nicolas Chauvet (kwizart) 2007-11-20 04:49:42 EST
I can give advices until Christoph
But build is failing on F-8 x86_64:

ldconfig: Can't create temporary cache file /etc/ld.so.cache~: Permission denied
make[1]: *** [installDSO] Error 1
So you need to removes the ldconfig call from the makefile


There is also two program founds:
WARNING, /usr/lib/sendmail does not seem to be an executable program;
WARNING, /sbin/mgetty does not seem to be an executable program;
I don't knwo if it will change to have them at build instead of runtime....

Since you have set $RPM_OPT_FLAGS (and they are used), you might be abble to remove
%define debug_package %{nil}
So the resulting binaries get stripped

About JBIG-in-TIFF conversion support not found.
I wonder if this is expected..
7068 Segmentation fault      $TIFFBIN/tiffcp -c g4 misc/jbig.tif misc/foo.tif >
/dev/null 2>&1

You can prevent timestamp changes by adding make install CPPROG="cp -p" \
Comment 63 Christoph Wickert 2007-11-20 13:45:19 EST
(In reply to comment #62)
> There is also two program founds:
> WARNING, /usr/lib/sendmail does not seem to be an executable program;
> WARNING, /sbin/mgetty does not seem to be an executable program;
> I don't knwo if it will change to have them at build instead of runtime....

Having them at runtime is enough.

> Since you have set $RPM_OPT_FLAGS (and they are used), you might be abble to
remove
> %define debug_package %{nil}
> So the resulting binaries get stripped

But then we still have an empty and therefore useless debuginfo package.


(In reply to comment #61)
> Christoph, are you still interested in this? Seems this takes much to much time;
> see also https://bugzilla.rpmfusion.org/show_bug.cgi?id=17

Howard, I suggest you focus on this review instead of opening another one at
rpmfusion. You will run into the same problems there as you are doing here.

> Maybe someone else is willing to review this?

Basically I'm still interested in this review and in (co)maintaining this
package, But there still is a lot of issues with it.

Biggest of all is the naming issue. As I said before this package should be
named hylafax+, because when Paul submits hylafax for review we will get in
trouble. What about packages sitting on top of hylafax(+), e. g. calpi4hylafax?
How do we make sure that we are not getting a version race between hylafax+ and
hylafax? I have no idea how to handle this, I asked on fedora-maintainers-list
but the discussion never was really finished, see
https://www.redhat.com/archives/fedora-maintainers/2007-July/msg00308.html

Then we still have no debug information (why?), the
unused-direct-shlib-dependency, some undefined non weak symbols and the problems
Nicolas mentioned in comment #62.

So I think I giving up this review. Sorry, maybe someone else is more
successful. I'm still willing to help wherever I can.
Comment 64 Lee Howard 2007-11-20 17:33:58 EST
(In reply to comment #62)

> But build is failing on F-8 x86_64:
> 
> ldconfig: Can't create temporary cache file /etc/ld.so.cache~: Permission denied
> make[1]: *** [installDSO] Error 1
> So you need to removes the ldconfig call from the makefile

I can remove the ldconfig, but for the source installation it is needed, and I
have seen other packages run ldconfig from their makefiles.  Furthermore, I just
did an 'rpmbuild --rebuild hylafax-5.1.11-1.src.rpm' on a Fedora 8 x86 system
and it turned out just fine:

Wrote: /usr/src/redhat/RPMS/x86_64/hylafax-5.1.11-1.fc8.x86_64.rpm

> There is also two program founds:
> WARNING, /usr/lib/sendmail does not seem to be an executable program;
> WARNING, /sbin/mgetty does not seem to be an executable program;
> I don't knwo if it will change to have them at build instead of runtime....

These should be fine this way... however, with the --rebuild that I just did I got:

[16] Location of sendmail program:      /usr/sbin/sendmail

> About JBIG-in-TIFF conversion support not found.
> I wonder if this is expected..
> 7068 Segmentation fault      $TIFFBIN/tiffcp -c g4 misc/jbig.tif misc/foo.tif >
> /dev/null 2>&1

This is somewhat expected to happen.  I could hide the error more carefully, but
the test is appropriate.  We have to test to see if libtiff was built with JBIG
support or not.  And in this case it was not built with JBIG support and thus is
causing the segfault.
 
> You can prevent timestamp changes by adding make install CPPROG="cp -p" \

Understood.  I'm not sure that this concerns me very much, unless it does
concern you.

(In reply to comment #63)

> Howard, I suggest you focus on this review instead of opening another one at
> rpmfusion. You will run into the same problems there as you are doing here.

I feel that I have been focused on this review.  I do not understand why
progress has not been occurring.  I had hoped that at rpmfusion this process
would be more attentively supported.

I am happy to see this at Fedora or at rpmfusion... I just would like to see it
*somewhere*.  If I am not doing something that is needed to get this package
included then please tell me what is lacking.

> Biggest of all is the naming issue.

Well, I had thought that we resolved the problem by simply calling this package
hylafax+.  But, from reading this (lengthy) thread...

> https://www.redhat.com/archives/fedora-maintainers/2007-July/msg00308.html

... it seems that there is a bigger problem with provides and conflicts, and
it's really not so much to do with naming as it has to do with those.

On a theoretical level you could potentially see this same issue come up on many
levels in the future.  RedHat/Fedora have already had to deal with similar
situations before (conflicts), and it's been done in different ways...
alternatives, version-numbering, renaming, etc.  As for whether or not one or
none of those are appropriate in this case is what is at debate, I suppose.

I don't really want to get into a big ruckus over this matter, but let me try to
explain (again) why I believe that all of that really should not be a factor in
this package submission's case...

Basically, back in 2005 I took my direct HylaFAX development away from
hylafax.org and continued it at Sourceforge due to issues that are publicly
discussed elsewhere and are not particularly relevant here.  From my perspective
I was doing HylaFAX development, and if the hylafax.org wanted my development
work they were free to "port" it to their code tree, and I have been very
vigilant about porting any developments at the hylafax.org repository into my
code tree (certainly I have omitted a few things by deliberate choice).

Basically, as I saw/see it, it's not much different than two branches of a CVS
codebase.  Sure, there are some (perhaps only minor) differences, but it's
really not enough to say that they're different things altogether.  Let's say
that package foo decided to split their CVS repository into a "maintenance"
branch and a "development" branch... but for whatever (probaby even silly)
reason some people preferred the develoment branch and some people preferred the
maintenance branch.

If that were to happen would there be some debate at Fedora over which branch of
the codebase to use?  No, there wouldn't be.  The package maintainer would call
the shots as far as Fedora is concerned.  If the package maintainer chose to
stay with the maintenance branch then Fedora would so stay... at least until the
maintainer was convinced otherwise.  If the package maintainer chose to move to
the development branch, then that's what would happen.

Many other distributions already have HylaFAX in their package offerings.  Some
of the package maintainers are using HylaFAX+ code base and some are using
hylafax.org code base.  And it wasn't that long ago that some were even using
SGI code base.  Some distributions are offering both packages (usually by
different maintainers), some offer only one.

My point being that this issue is a silly one that is unnecessarily proving to
be a road block.  Call it hylafax+ or call it hylafax... there is still neither
package in Fedora (and Darren/Paul/Arlington apparently has yet to even start up
the reveiew request for his, despite his interjections)... and thus there is no
reason to worry about conflicts now.

All of that said... step back a bit and observe things from a distanced
perspective.  HylaFAX+ usually proves to be the upstream for hylafax.org.  And,
when code work is done at hylafax.org then it serves as the upstream for
HylaFAX+.  There are a few nuances between them that apparently are permanent,
but for the most part the two are similar enough (or will be similar enough)
that they're more of a "pseudo-fork" (like CVS branches) than they are true
forks.  Are these nuances enough to warrant two packages in Fedora?  If they
are, then that is fine... however, right now there is neither.

> Then we still have no debug information (why?)

I don't know.  I don't even know what debug information is *supposed* to be
there.  I'm not even sure why it's important or why it's a problem to not have
it there.

> unused-direct-shlib-dependency, some undefined non weak symbols and the problems

I'm fairly sure those things were resolved between then and now.  Please refer
to the 5.1.11 SPEC and SRPM:

SPEC: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax.spec
SRPM: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax-5.1.11-1.src.rpm
Comment 65 Ralf Corsepius 2007-11-21 03:17:57 EST
The origin of the empty debuginfos is hylafax stripping all files from 
inside of its ports/install.sh script if it finds a "strip" program.

Try this patch:

--- hylafax.spec.orig   2007-11-08 17:59:02.000000000 +0100
+++ hylafax.spec        2007-11-21 09:11:17.000000000 +0100
@@ -1,8 +1,5 @@
 %define faxspool    /var/spool/hylafax
 
-# The resulting debuginfo package is empty.  So we just disable it.
-%define debug_package %{nil}
-
 Summary:   HylaFAX(tm) is a enterprise-strength fax server
 Name:      hylafax
 Version:   5.1.11
@@ -41,6 +38,7 @@
 %build
 # - Can't use the configure macro because HylaFAX configure script does
 #   not understand the config options used by that macro
+STRIP=':' \
 ./configure \
         --with-DIR_BIN=%{_bindir} \
         --with-DIR_SBIN=%{_sbindir} \
Comment 66 Lee Howard 2007-11-21 12:40:26 EST
Thank you.  That appears to have worked for debuginfo.

SPEC: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax.spec
SRPM: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax-5.1.11-2.src.rpm
Comment 67 Jason Tibbitts 2008-01-18 02:18:47 EST
Wow, this is still going.  I had some time so I built it.  It builds fine on rawhide; here's the latest rpmlint output:

  hylafax.src:66: E: configure-without-libdir-spec
This is bogus.

  hylafax.src: W: invalid-license BSD-like
This isn't a permitted license tag; if the license really doesn't correspond
to one of those on http://fedoraproject.org/wiki/Licensing then talk to spot
and have him cook up another tag for you.

  hylafax.x86_64: E: executable-marked-as-config-file /etc/cron.hourly/hylafax
  hylafax.x86_64: E: executable-marked-as-config-file /etc/cron.daily/hylafax
Not problematic.

  hylafax.x86_64: W: incoherent-version-in-changelog 5.1.11-1 5.1.11-2.fc9
Release: is 2 but the last changelog is for release 1.  You should changelog
each release bump.

  hylafax.x86_64: W: file-not-utf8 /usr/share/doc/hylafax-5.1.11/CONTRIBUTORS
This should be passed through iconv.

  hylafax.x86_64: W: non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps
Not really sure what to do about this one.  Maybe it should be installed with
the documentation instead.

  hylafax.x86_64: E: executable-marked-as-config-file /etc/rc.d/init.d/hylafax
initscripts shouldn't be marked as %config.

  hylafax.x86_64: E: explicit-lib-dependency libtiff
You shouldn't have Requires: libtiff; rpm will figure out the library
dependency by itself.

  hylafax.x86_64: E: script-without-shebang /usr/sbin/faxsetup.linux
If this is a shell script, it needs a #! line.  If it's not, it shouldn't be
executable and it certainly shouldn't be in /usr/sbin.

  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/ro
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/pt_BR
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/fr
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/pl
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/pt
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/de
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/es
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/en
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/tr
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/it
This is pretty odd.  What are executables doing under /var/spool?  And
regardless of where they are, either they shouldn't be executable or they
should have #! lines so that they can actually be executed.
Comment 68 Lee Howard 2008-01-18 14:46:28 EST
  hylafax.src: W: invalid-license BSD-like
Just "BSD" is good enough for me.

  hylafax.x86_64: W: incoherent-version-in-changelog 5.1.11-1 5.1.11-2.fc9
Okay... I'll try to remember to add a changelog for each release even though the
spec file isn't changing materially.

  hylafax.x86_64: W: file-not-utf8 /usr/share/doc/hylafax-5.1.11/CONTRIBUTORS
Fixed upstream.

  hylafax.x86_64: W: non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps
HylaFAX uses a configurable cover page template file, and this is an
example/sample one.  The configurable cover page template file *does* belong in
the configuration directory, /etc/hylafax.  So I believe that the samples do
belong there as well.  Can we say that the warning doesn't apply here?

  hylafax.x86_64: E: executable-marked-as-config-file /etc/rc.d/init.d/hylafax
Okay, this is changed/fixed now.

  hylafax.x86_64: E: explicit-lib-dependency libtiff
Okay, I've (again) removed libtiff.  (I've had RPM builders, SuSE I think, who
wanted it there.)

  hylafax.x86_64: E: script-without-shebang /usr/sbin/faxsetup.linux
This is fixed upstream.

  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/ro
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/pt_BR
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/fr
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/pl
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/pt
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/de
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/es
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/en
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/tr
  hylafax.x86_64: E: script-without-shebang /var/spool/hylafax/bin/dict/it
This is all fixed upstream.

Okay, please now see:

SPEC: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax.spec
SRPM: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax-5.2.2-1.src.rpm
Comment 69 Jason Tibbitts 2008-04-24 17:39:38 EDT
I wanted to spend some time taking care of some of these old tickets but now I
can't get this to build (in rawhide) at all; the build fails with:

Using /bin/bash to process command scripts.
Missing C++ runtime support for g++ (/usr/lib64/ccache/g++).
Compilation of the following test program failed:
----------------------------------------------------------
#include "iostream.h"
int main(){ cout << "Hello World!" << endl; return 0;}
----------------------------------------------------------
Usually this is because you do not have a standard C++ library
installed on your system or you have installed it in a non-standard
location.  If you do not have a C++ library installed, then you must
install it.  If it is installed in a non-standard location, then you
should configure the compiler so that it will automatically be found.
(For recent gcc releases this is libstdc++, for older gcc - libg++)
Unrecoverable error!  Once you've corrected the problem rerun this script.

libstdc++-devel is installed, so it's not something as simple as that.  Perhaps
there's a gcc 4.3 incompatibility?  I don't know much at all about C++.
Comment 70 Lee Howard 2008-04-24 17:49:02 EDT
It builds perfectly fine on Fedora 8 (which uses gcc 4.1.2).  Is gcc 4.3 only
available in rawhide?
Comment 71 Jason Tibbitts 2008-04-24 17:58:58 EDT
Well, it's in Fedora 9 which isn't terribly far away now.
Comment 72 Jason Tibbitts 2008-04-24 18:08:52 EDT
BTW, here's the end of the config.log file.  Perhaps it will be instructive:

++ cat dummy.C
#include "new.h"
struct foo {
    int x;
    foo();
    ~foo();
};
foo::foo() {}
foo::~foo() {}
int main()
{
    foo* ptr = 0;
    foo* a = new(ptr) foo;
    a->x = 0;
    delete a;
    return 0;
}
++ /usr/lib64/ccache/g++ -o dummy dummy.C
dummy.C:1:17: error: new.h: No such file or directory
dummy.C: In function 'int main()':
dummy.C:12: error: no matching function for call to 'operator new(long unsigned
int, foo*&)'
<built-in>:0: note: candidates are: void* operator new(long unsigned int)
++ /usr/lib64/ccache/g++ -o dummy dummy.C -lg++
dummy.C:1:17: error: new.h: No such file or directory
dummy.C: In function 'int main()':
dummy.C:12: error: no matching function for call to 'operator new(long unsigned
int, foo*&)'
<built-in>:0: note: candidates are: void* operator new(long unsigned int)
++ make -f confMakefile t
/usr/lib64/ccache/g++        -D__ANSI_CPP__ -I. -I. -I././regex -I. -I././util
-I/usr/include -g -O  t.c++
t.c++:1:22: error: iostream.h: No such file or directory
t.c++: In function 'int main()':
t.c++:2: error: 'cout' was not declared in this scope
t.c++:2: error: 'endl' was not declared in this scope
make: *** [t] Error 1
Comment 73 Nicholas Miell 2008-04-24 18:59:43 EDT
gcc 4.3 dropped the pre-ISO backwards compatibility headers. Fortunately, the
only thing that appears to actually needs those headers is the configure script.
Comment 74 Ralf Corsepius 2008-04-25 00:31:40 EDT
Created attachment 303735 [details]
hack let hylafax's configure succeed with gcc-4.3.0

Try this patch. It hacks hylafax's configure to use iostream instead of
iostream.h and to let this configure script succeed.

But ... blunt question: Does hylafax still have an active upstream?
Checking the details of this configure script, I found this configure script to
be hardly working at all and to produce questionable/broken results in detail.
It's mere luck this package builds at all.
Comment 75 Lee Howard 2008-04-25 00:43:19 EDT
Yes, hello (?), an active upstream is here.  http://hylafax.sourceforge.net

As for the details of this failing on gcc-4.3, admittedly there's not likely
been anyone attempt to build HylaFAX with it until now.

As for other parts of the configure script that may be hardly working at all,
etc., please do elaborate, and I'll see what I can do about getting them working
better.

That said, it builds just fine on every platform out there that I've tested it
on, and I'm pretty sure that's more than mere luck.
Comment 76 Ralf Corsepius 2008-04-25 01:14:02 EDT
(In reply to comment #75)
> As for the details of this failing on gcc-4.3, admittedly there's not likely
> been anyone attempt to build HylaFAX with it until now.
That's apparent :)
 
> As for other parts of the configure script that may be hardly working at all,
> etc., please do elaborate, and I'll see what I can do about getting them
> working better.
Openly said, if I were upstream, I would ditch this configure script and its
Makefile.ins underneath and replace with something more standardized. My
personal choice would be the autotools.

> That said, it builds just fine on every platform out there that I've tested it
> on, and I'm pretty sure that's more than mere luck.
Some details from my build.log:

1.
...
Looks like /usr/lib64/ccache/gcc supports the -g option.
... but not together with the -O option, not using it.

This result is plain wrong - The test trying to check for -O -g must have failed.

2. configure script doesn't acknowledge CXXFLAGS and CFLAGS.

3. 
Checking for PAM (Pluggable Authentication Module) support
... not found. Disabling PAM support

This could be a packaging bug inside of the rpm.spec - I don't know.

4.
...
Checking ZLIB support.
Using ZLIB include files from
Using pre-built ZLIB library -lz
Done checking ZLIB support.
...
No idea what this is meant to mean. Could be a parallel build issue, a configure
script failure or else ...

5.
...
Checking JBIG-in-TIFF conversion support.
./configure: line 3274: 28995 Segmentation fault      $TIFFBIN/tiffcp -c g4
misc/jbig.tif misc/foo.tif > /dev/null 2>&1
JBIG-in-TIFF conversion support not found.
...
?!? Likely a broken check in configure.

6. -I/usr/include in compiler calls:
...
/usr/lib64/ccache/gcc        -D__ANSI_CPP__ -I. -I.. -I.././regex -I.././regex
-I.././util -I/usr/include -fpic -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
-fexceptions -fstack-protector ...

Though it doesn't have an impact in most cases, this is quite a serious bug,
because it impacts include file search order.

7. uid/gid processing:
+ make install -e FAXUSER=8690 FAXGROUP=492 SYSUSER=8690 SYSGROUP=492
BIN=/var/tmp/hylafax-5.2.2-1.fc9-root-mockbuild/usr/bin
SBIN=/var/tmp/hylafax-5.2.2-1.fc9-root-mockbuild/usr/sbin LIBD
/bin/bash ./port/install.sh  -u 8690 -g 492 -m 755 ...

This is all wrong ... the uid/gids you see above are my personal ones!




Comment 77 Lee Howard 2008-04-26 18:36:56 EDT
Okay, I'm now testing with Fedora 9 Preview.  The patch for configure does work.

I sympathize with the dislike for the non-standard configure script and
Makefiles.  They can be quite cumbersome at times.  Due to other projects with
which I am involved I am familiar with autotools, and in my opinion, the score
between them really is par.  autotools, itself, can be quite cumbersome... not
only for developers, but also for users who may have to hunt down specific
versions of autotools to make things work right.  I am in no way against looking
for a better way, even if that does mean autotools, but for now it's probably
best to address the issues in the existing configure script and wait for such a
time as someone passionate about this issue comes along - or until an undeniable
advantage manifests itself in making a conversion.

1) "The test trying to check for -O -g must have failed."

Indeed.  This has now been fixed.

2) Traditionally HylaFAX configure used --with-GCOPTS= and --with-GCXXOPTS
instead of CFLAGS and CXXFLAGS.  I've now made configure use CFLAGS and CXXFLAGS
instead if GCOPTS and GCXXOPTS are not given.

3) I'm not having a problem with PAM detection:

Checking for PAM (Pluggable Authentication Module) support
... found. Enabling PAM support

I get this both with the source build (from tarball) and with rpmbuild.  Are you
sure that you have pam-devel installed?  Please check again with the SRPM that
I'll link below.

4) "Using ZLIB include files from"

This can be ignored.  I've made configure not print this message if $ZLIBINC is
empty.

5) "./configure: line 3274: 28995 Segmentation fault      $TIFFBIN/tiffcp -c g4
misc/jbig.tif misc/foo.tif > /dev/null 2>&1"

Here configure is checking to see if your libtiff supports JBIG or not, and it
doesn't, and in fact your libtiff segfaults when trying to process
JBIG-compressed TIFF files.  configure correctly interprets this, however, the
output is, indeed, annoying, and so I've made configure now silence this.

6) I'm not sure how to resolve this one.  configure takes efforts to find the
needed library headers in non-standard locations and then uses -I for them. 
Assuming it finds them in /usr/include then that's what you're seeing.  When
should -I be used or not?

7) From the spec file comments:

# FAXUSER, FAXGROUP, SYSUSER and SYSGROUP are set to the current user to
# avoid warnings about chown/chgrp if the user building the SRPM is not root;
# they are set to the correct values with the RPM attr macro

Thanks.

SPEC: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax.spec
SRPM: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax-5.2.4-2.src.rpm
Comment 78 Jason Tibbitts 2008-04-28 16:44:48 EDT
This new package does build for me.  I wonder about these bits from the
configure output:

Checking for PAM (Pluggable Authentication Module) support
... not found. Disabling PAM support

Checking for LDAP (Lightweight Directory Access Protocol) support
... not found. Disabling LDAP support

Is it worth adding build dependencies on pam-devel and openldap-devel?  I can't
say what functionality is missing but generally in Fedora we enable pam and ldap
support for most programs.

Looks like /usr/bin/gs is the PostScript RIP to use.
WARNING, /usr/bin/gs does not seem to be an executable program;
         you may need to correct this before starting up the fax server.
ls: cannot access /usr/bin/gs: No such file or directory
./configure: line 4180: /usr/bin/gs: No such file or directory

Does this really matter, since it was explicitly directed to use /usr/bin/gs? 
Or does a build dependency ghostscript need to be added?   There are similar
complaints about sendmail and mgetty.
Comment 79 Lee Howard 2008-04-29 00:09:12 EDT
PAM and LDAP support in HylaFAX enable authentication mechanisms for the hfaxd
protocol that are alternatives to the etc/hosts.hfaxd file-based authentication
method.  Sure, pam-devel and openldap-devel should probably be build dependencies.

The WARNING regarding /usr/bin/gs can be ignored.  It's an installation
dependency and not a build dependency.  Furthermore, the configure-determined
value is not built-in to binaries and can be configured by the administrator. 
Same goes for sendmail as for ghostscript.  And mgetty is entirely optional.

SPEC: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax.spec
SRPM: http://osdn.dl.sourceforge.net/sourceforge/hylafax/hylafax-5.2.4-3.src.rpm
Comment 80 Jason Tibbitts 2008-05-07 21:52:15 EDT
Finally getting back to this review; I'll try to get it finished up.
Comment 81 Jason Tibbitts 2008-05-10 18:02:30 EDT
Note to FE-Legal folks, this package needs attention for two reasons and perhaps a third as well.  Please search for FE-Legal below.

Now, to the review.

Was the hylafax/hylafax+ issue ever resolved?  Do we need FE-Legal to be involved in that?

Can you explain why the tarball in the src.rpm does not match the tarball fetched from the Source0: URL?  There seem to be rather significant source differences.  This kind of thing is not permissible; the sources in the src.rpm must be identical to the upstream sources except in specific limited cases where we must remove something.  I note that the tarball in the src.rpm seems to be three days newer than the one upstream.

I don't believe we should package /etc/hylafax/faxcover_example_sgi.ps.  It contains the old SGI name and logo and I'm pretty certain we shouldn't be sticking it in /etc whenever someone installs this software.  I'm not even sure we have the legal right to distribute it, which is reason one for blocking FE-Legal.

It doesn't particularly bother me, but the guidelines to specify that you not use a specific sourceforge mirror for the source URL.  See 
http://fedoraproject.org/wiki/Packaging/SourceURL (although personally I find I often have to add one just to get things to download, since sourceforge is so incredibly unreliable).

I recommend not using the name of the package in the summary, as it tends to look rather redundant in listings.  Still, there is a change of case so I won't block the package if you think it really needs to be there.

I'm going to have to get expert assistance with the License: tag; the license given in the COPYRIGHT file is actually identical to the libtiff license, which gets its own "libtiff" license tag, but the regex code is clearly the bad "BSD+Advertising" clause which is GPL-incompatible and thus causes issues.  Reason two that I'm blocking FE-Legal for guidance.

Your changelog entries are not in one of the acceptable formats.  These are parsed automatically, so please follow the formats given in the Changelogs section of http://fedoraproject.org/wiki/Packaging/Guidelines and please also include a comment every time you change the release.

You need a dependency on the crontabs package if you want to put things in /etc/cron.daily.

You call ldconfig in your scriptlets, but you don't have any dependencies on it.  When you use the single-line scriptlets (%post -p /sbin/ldconfig) then you don't need them, but when you use multiline scriptlets you have to specify the dependencies manually.

Finally, I have significant issues with the amount of stuff this package puts under /var/spool.  I don't believe any of the files belong there at all.  Executables, certainly not.  Unless you can illustrate how the FHS allows such things, I cannot approve this package.  The modem config files need to be under /etc; the executables probably belong under /usr/libexec if they're not supposed to be run by the end user.

Checklist:
X source files do not match upstream.
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
? summary includes the name of the package.
* description is OK.
* dist tag is present.
* build root is OK.
X license field matches the actual license.
? license is open source-compatible.
* license text included in package.
X changelogs not correctly formatted.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint has acceptable complaints.
X final provides and requires:
   config(hylafax) = 5.2.4-3.fc9
   libfaxserver.so.5.2.4()(64bit)
   libfaxutil.so.5.2.4()(64bit)
   hylafax = 5.2.4-3.fc9
  =
   /bin/sh
   /sbin/chkconfig
   /sbin/service
   config(hylafax) = 5.2.4-3.fc9
   gawk
   ghostscript
   libfaxserver.so.5.2.4()(64bit)
   libfaxutil.so.5.2.4()(64bit)
   libgcc_s.so.1()(64bit)
   libgcc_s.so.1(GCC_3.0)(64bit)
   liblber-2.4.so.2()(64bit)
   libldap-2.4.so.2()(64bit)
   libpam.so.0()(64bit)
   libpam.so.0(LIBPAM_1.0)(64bit)
   libstdc++.so.6()(64bit)
   libstdc++.so.6(CXXABI_1.3)(64bit)
   libstdc++.so.6(GLIBCXX_3.4)(64bit)
   libtiff.so.3()(64bit)
   libutil.so.1()(64bit)
   libz.so.1()(64bit)
   mailx
   sharutils
X  (missing crontabs for /etc/cron.*)
X  (missing /sbin/ldconfig dependency for %post and %postun)

* %check is not present; no test suite present.  I have no way to test this 
   software.
X shared libraries are present; ldconfig called properly but dependency on it is 
   missing.
X ownership problems for /etc/cron.*
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
X scriptlets are OK, but ldconfig dependencies are missing.
* code, not content.
 documentation is small, so no -doc subpackage is necessary.
 %docs are not necessary for the proper functioning of the package.
 no headers.
 no pkgconfig files.
 no static libraries.
 no libtool .la files.
Comment 82 Tom "spot" Callaway 2008-05-12 15:12:23 EDT
(In reply to comment #81)

> I don't believe we should package /etc/hylafax/faxcover_example_sgi.ps.  It
contains the old SGI name and logo and I'm pretty certain we shouldn't be
sticking it in /etc whenever someone installs this software.  I'm not even sure
we have the legal right to distribute it, which is reason one for blocking FE-Legal.

Indeed, this falls a little outside the realm of fair use. Please get rid of it.

> I'm going to have to get expert assistance with the License: tag; the license
given in the COPYRIGHT file is actually identical to the libtiff license, which
gets its own "libtiff" license tag, but the regex code is clearly the bad
"BSD+Advertising" clause which is GPL-incompatible and thus causes issues. 
Reason two that I'm blocking FE-Legal for guidance.

There's no incompatibility there, but please use: License: libtiff and BSD with
advertising.
Comment 83 Lee Howard 2008-05-24 19:16:39 EDT
Jason and Tom, thank you very much for your assistance here.

Here are new SPEC and SRPM files:

SPEC: http://downloads.sourceforge.net/hylafax/hylafax.spec
SRPM: http://downloads.sourceforge.net/hylafax/hylafax-5.2.5-1.src.rpm

As for the hylafax/hylafax+ issue.  Nothing has changed there.  In all truth
they're pretty much the same thing.  They are nearly interchangeable.  I fully
expect those at hylafax.org to continue to claim that HylaFAX+ is a "fork" and
needs to be clearly distinguished from their releases, however, HylaFAX+
frequently and routinely serves as the upstream for development for their
codebase (and vice-versa).  There are some differences, but it's truthfully no
more difference than exists between, say, Apache HTTPD from Debian and Apache
HTTPD from Fedora.

Whether or not the package should be called "hylafax" or "hylafax+"... I really
don't care much.  My preference is to leave it as "hylafax" since users who are
using old "hylafax" RPMs from hylafax.org should be able to seamlessly upgrade
with 'rpm -Uvh' without noticing much beyond the usual.

The tarball in the src.rpm did not match the tarball from the Source0 URL
because in going to hylafax-5.2.4-3.src.rpm from hylafax-5.2.4-1.src.rpm (in
order to address Fedora 9 issues mentioned above) I made changes to the tarball
in the src.rpm without cutting another release of the tarball indicated in the
Source0 URL.  From now on I'll apply patches instead of changing the tarball on
-2, -3, -4, etc releases.

I removed /etc/hylafax/faxcover_example_sgi.ps from the upstream repository. 
Thank you for pointing out the lack of fair use.

In future releases I will not point to a specific Sourceforge mirror in Source0
but will instead point to downloads.sourceforge.net.  For this release I left it
pointed at the specific URL.  Thanks for pointing this out.

I've removed the name of the package from the summary.

I've changed License to "libtiff and BSD with advertising"

I've fixed the formatting of the changelog entries (although I neglected to make
one for this release).

I've added a dependency on crontabs.

I've added dependencies on /sbin/ldconfig.

As for the stuff going in under /var/spool... I'm afraid that's something that I
cannot change.  Changing it upstream would be a support nightmare, and so would
having the Fedora distribution be so vastly different from the rest of the
HylaFAX installations out there.  It's been this way for 18 years now, and
moving hylafax from /var/spool at this point due to perceived incongruities with
FHS is just something that I'm not anxious to do.  The benefits of having
HylaFAX in Fedora would not likely outweigh the costs of dealing with the
subsequent support nightmare that would ensue.

That said, let me make my argument as to why everything under /var/spool/hylafax
should be acceptable as it is.

The purpose of /var is to allow /usr to be read-only.  Thus, any files that are
subject to change during normal application function are expected to reside in
/var.  Strictly speaking /var/spool is reserved for files that are to be
processed in the future.  Thus, the bona-fide spools are totally legit:
/var/spool/hylafax/sendq, hylafax/doneq, hylafax/recvq, hylafax/status,
hylafax/log, hylafax/info, etc.  In fact, the only directories that are not
technically spools are hylafax/etc, hylafax/bin, and hylafax/config.

While the vast majority of HylaFAX binaries and their respective configuration
files are installed outside of /var/spool/hylafax, those three directories (etc,
bin, and config) are configuration files and configuration utility scripts,
etc., that control how the spools are handled.

Due to the way that HylaFAX daemons operate it would be extremely cumbersome (if
not impossible - as in the case of a chroot) for these to be elsewhere.  They're
very much like the configuration files that LPRng had under /var/spool/lpd.

So, that's my argument.  I hope that it's acceptable.  But if it's not...
unfortunately there's not much that I am willing to do about it.

I do thank you and appreciate your attention.
Comment 84 Christoph Wickert 2008-05-24 19:32:12 EDT
(In reply to comment #83)

> Whether or not the package should be called "hylafax" or "hylafax+"... I really
> don't care much.  My preference is to leave it as "hylafax" since users who are
> using old "hylafax" RPMs from hylafax.org should be able to seamlessly upgrade
> with 'rpm -Uvh' without noticing much beyond the usual.

We could easily archive this with a Provides: hylafax.

> As for the stuff going in under /var/spool... I'm afraid that's something that I
> cannot change.  ...

One point: I'm pretty sure we will get in trouble with SELinux that way.
Comment 85 Michael Schwendt 2008-05-27 07:59:01 EDT
The package must not contain /usr/lib/debug/
Those files belong into the -debuginfo package only.
Comment 86 Michael Schwendt 2008-05-27 08:06:55 EDT
On Fedora 8, faxsetup gives:

Warning: /usr/share/ghostscript/fonts does not exist or is not a directory!

The directory /usr/share/ghostscript/fonts does not exist or this file is not a
directory.
This is the directory where the HylaFAX client applications expect to
locate font metric information to use in formatting ASCII text for
submission as facsimile.  Without this information HylaFAX may generate
illegible facsimile from ASCII text.
Comment 87 Lee Howard 2008-05-27 08:40:32 EDT
Hello Michael,

The package does not contain /usr/lib/debug:

[root@bilbo i386]# rpm -pql hylafax-5.2.5-1.fc8.i386.rpm | grep debug
[root@bilbo i386]# 

I've now added ghostscript-fonts to the Requires.  Thanks.
Comment 88 Michael Schwendt 2008-05-30 04:13:36 EDT
You want proof? No problem. ;) The spec file contains 

  %{_libdir}/*

which also includes /usr/lib/debug recursively.
Instead, you want the more explicit

  %{_libdir}/libfax*

or

  %{_libdir}/*.so.*

That your own build does not include debuginfo files means that
you don't have the "redhat-rpm-config" installed. Packagers should
"yum install rpmdevtools".
Comment 89 Michael Schwendt 2008-05-30 04:16:21 EDT
> ghostscript-fonts

$ rpm -q ghostscript-fonts hylafax
ghostscript-fonts-5.50-18.fc8
hylafax-5.2.5-1.fc8

It was installed already. It doesn't add /usr/share/ghostscript/fonts 
but /usr/share/fonts/default/ghostscript:

$ rpmls ghostscript-fonts|grep ^d
drwxr-xr-x  /etc/X11/fontpath.d
drwxr-xr-x  /usr/share/fonts/default
drwxr-xr-x  /usr/share/fonts/default/ghostscript
Comment 90 Jason Tibbitts 2008-05-30 17:40:50 EDT
I can confirm that /usr/lib/debug is making it into the package, but not on
x86_64 where I usually build.  I did an i386 build and everything under
/usr/lib/debug is indeed included in the main package.

I note that recent rpmlint has grown a complaint about the Summary not being
capitalized.  Might be good to fix that.

I am not at all comfortable approving this package with the FHS violations in
/var/spool, and the naming issue troubles me as well.  You can say that it's
really "hylafax" and not "hylafax+", but the bottom line is that the upstream
web site very clearly puts the '+' there and makes the distinction between the
plussed and unplussed versions clear.  Also, when I go to google and enter
"hylafax" I get pointed to hylafax.org which uses no '+' anywhere and gets me a
different piece of software.  

Honestly, Fedora has no interest in any argument between the upstream developers
of the different branches of hylafax, and if you can't work out a consistent
naming scheme amongst yourselves which you present to us then I'm quite happy to
say that we'll just wait it out.

I'm going to start a thread on fedora-devel to try and get some discussion going.
Comment 91 Michael Schwendt 2008-05-31 16:05:37 EDT
> I can confirm that /usr/lib/debug is making it into
> the package, but not on x86_64 where I usually build.

On x86_64 %_libdir is /usr/lib64, so /usr/lib/debug is not matched
by %{_libdir}/* in the %files section.
Comment 92 Lee Howard 2008-07-04 20:49:17 EDT
(In reply to comment #88)

I've made a change to %{_libdir}/libfax*

(In reply to comment #89)

This is a bug in the ghostscript package and not HylaFAX.  HylaFAX is extracting
the path from the 'gs -h' output.  HylaFAX's faxsetup will compensate for this
ghostscript bug later during setup and configuration.

(In reply to comment #90)

I've followed your fedora-devel discussion here:

http://fcp.surfsite.org/modules/newbb/viewtopic.php?topic_id=57663&forum=11&post_id=268715

There seems to be a resounding consensus with respect to the FHS matter and a
very undecided response to the naming matter.

Please allow me to restate my positions on these.

My positions are entirely based upon anticipation of user expectations,
minimizing support cases, and making everything as intuitive as possible.  In
fact, these concerns are the only reason for my creating this review request in
the first place: if there were a hylafax package in Fedora already I wouldn't
have bothered - there would have been no reason for this effort.

As HylaFAX is approaching it's 20th birthday and is almost certainly the
most-popular open-source fax application for UNIXes, it's silly that Fedora does
not have a hylafax package.

RedHat 5.2 *did* have hylafax-4.0pl2 (that's the SGI flavor, not from
hylafax.org).  RedHat apparently decided to drop the hylafax package due to
conflicts with the mgetty-sendfax package.

(That is interesting because the mgetty author created the conflict... as his
approach at the time was to write his own fax software instead of participate
with HylaFAX development.  And, in particular, he chose to use "sendfax" for one
of the commands... and thus created the conflict.  One could argue, and I would
agree, that "sendfax" is generic enough to be the presumptive way to send a fax
with a command.  Nevertheless, this seems to be the reason why hylafax was
tossed from RedHat 6.0.)

Again, I'm okay with calling the package hylafax+ instead of hylafax.  I think
that would be a mistake, though.  And I think having a "hylafax.org" and a
"hylafax+" but no "hylafax" (cutting the baby in half) would be an even worse
mistake.  Why?  Because all HylaFAX users, whether they use SGI's flavor,
hylafax.org's flavor, iFax's flavor, or HylaFAX+ flavor... they all call that
software "HylaFAX".  And they're all right, too.

If hylafax.org really wanted to submit a package request and maintain it, then I
would wholeheartedly acquiesce and say to put that in as a "hylafax" package
into Fedora.  I would never use it, myself, of course... but my whole reason for
creating this package review request in the first place has always been to see a
HylaFAX package in Fedora for other users.  You don't see me trying to bully-in
a hylafax+ package into Debian/Ubuntu for exactly this reason.

Both HylaFAXes at hylafax.org and Sourceforge (HylaFAX+) are so similar that
it's really inaccurate to describe their differences as a fork.  I don't think
there's an accurate label, but a certain well-known open-source philosopher once
called it a "pseudo-fork" when I brought up this concept to him.  He said that
this case is very similar to what happens when a distribution packager adds a
few patches here and there to suit the distribution schema or to address bugs or
even enhancements that appealed to the packager.  He indicated that use and
functionality were the criterion by which a fork was to be defined... and not so
much on code specifics.  In other words, we could create a software package
called "hello_world" that was coded in shell this way:

echo "Hello world!"

And then someone came along and rewrote hello_world in this way:

printf "Hello world!\n"

Are they forks?  Maybe there is a reason to prefer one over the other, but
they're functionally the same, and there's absolutely no sane reason to try have
both of them.  Pick one or the other.  If a time comes that one is preferred
over the other, then simply change, and keep on calling it hello_world.

I realize that's an over-simplified example, but I'm trying to illustrate my
perspective here... and what I believe is the general user perspective.

Users are going to want to do: 'yum install hylafax'.  For users who already
know that HylaFAX on Fedora is called "hylafax+" or "hylafax.org" then doing
'yum install hylafax+' may be beneficial, but it will create a support matter
and a point of confusion for every first-time installer.

There seems to be some suspicion on your part, Jason, that I have some nefarious
intention "in (what seems to [you] to be) an attempt to gain legitimacy".  I
think that you've entirely misunderstood what I've done.

I coded at hylafax.org for several years.  There was an occasional flare-up with
respect to opinions on aesthetics of the coding being committed to the
repository, and an occasional flare-up with respect to design approaches, but as
ugly as those sometimes got the code changes were almost always followed, and
they weren't the reason why I moved my coding to Sourceforge.

In fact, I set up the Sourceforge site as a download location for HylaFAX...
specifically pre-release versions of HylaFAX.  You see, I always had been
frustrated with the slow release schedule at hylafax.org.  Users were regularly
and frequently (multiple times per week) requesting that I send them pre-release
tarballs or pre-release RPMs.  It became very frustrating to answer mailing-list
questions over and over again for weeks and months about bugs that were "fixed
in the next release".  So I finally made the decision to just set up a site
where users could just download those pre-releases for themselves.

The end-result of this action was a complete rejection by other hylafax.org
developers (all of whom were/are employed by iFax).  They insisted that
references inside of the tarball (code, manpages, and text files) should have no
reference to hylafax.org or "The HylaFAX Development Team".

The only way that I could sanely accommodate that was to set up a separate code
repository.  Then I embarked upon a number of months where I was trying to
maintain two code repositories (and that never works: it's a real pain, as it
had been when those same hylafax.org developers asked me to code in a 4.2 CVS
branch while expecting me to also maintain the old 4.1 branch that eventually
did nothing for open-source HylaFAX users and never had another release).

So basically what happened was that iFax made it increasingly impossible for the
largest code contributor, me, to continue to operate at hylafax.org.  And, yet,
they were as eager as always to absorb the developments I would make at the
Sourceforge site.

So we now have a situation, then, where there are two versions of HylaFAX that
are basically following the same development path... one behind the other. 
There are some differences, but they're nominal for most users, and
functionality and use are identical (the differences follow the hello_world
example quite literally although not as simplistically).

The HylaFAX+ name came about because some users were quite emphatic that they
needed some distintion between "the Souceforge HylaFAX development project" and
hylafax.org.  In retrospect, I now tend to believe that those clamors were
coming from hylafax.org enthusiasts whose intentions were really to strip
legitimacy from my efforts.

I hope that clarifies my intentions.  If they continue to seem nefarious to you,
then I sincerely apologize.  I truly believe that my actions and intentions have
always been done in good faith and with the users' interests in mind.

So, again, I'm happy to change the name of the package to "hylafax+" if that's
what's really the hold-up here.  I disagree with it, but I'm willing to do it
if, indeed, that's the hold-up.

Unfortunately, I don't think that using "Provides: hylafax" helps any.  In an
'rpm -Uvh' procedure (upgrade) this still happens (in extreme multiplicity):

file /var/spool/hylafax/etc/dialrules from install of hylafax+-5.2.5-1.fc8.i386
conflicts with file from package hylafax-5.2.0-1.fc8.i386

Maybe someone better with spec files can tell me what I didn't do right.  But
this kind of thing is *exactly* what must be avoided.  Users need to be able to
transition between hylafax.org and HylaFAX+ (and Fedora) RPMs without these
kinds of headaches.

And as for the FHS matter...

I hope that you can understand that I cannot change how the upstream package is
installed or works in such a dramatic way as it would be to move "bin", "etc",
and "config" elsewhere.  The resulting support issues would simply be too great.

Now, we may be able to get away with symlinking as the fedora-devel tread
suggests, but it would be something that would be unique to the Fedora
package... unless it were so interchangeable that it would work for all other
platforms (e.g. Mac OS, Solaris, BSD, AIX, IRIX, SCO, etc.) too.  And even then,
I would be reluctant to do it.

You see, having it all right there allows the hfaxd client (much like an ftp
client) in an administrative mode to get in to the configuration files in those
directories (yes, "bin" contains executable scripts which are largely considered
configurable) and make adjustments that way.  So it could be quite useful to
have all of those things there.  Certainly it's much more convenient to have
them there.

Thus, it is my perspective that those directories do very much belong under
"var", and there really is no more-appropriate place to put them other than
"spool"... in my estimation.

Maybe that concept is stretching it a bit - nobody probably uses bin, etc, and
config files in that way.  But it's a design feature that I don't feel
necessarily warrants change.

However, if this is the only thing holding up getting this package into Fedora,
and if we can make adjustments that prove to be transparent to the end-users,
then I can work with that (I'll need some detailed explanations of what to do)
in a Fedora-specific way as I've stated above.
Comment 94 Lee Howard 2008-07-24 11:39:30 EDT
On the FHS matter...

It would be infinitely easier to move all of /var/spool/hylafax into
/var/hylafax instead of trying to move subdirectories "bin", "etc", "config"
into other places.

Does this make a resolution any easier?
Comment 97 Nicolas Chauvet (kwizart) 2009-01-20 07:50:28 EST
http://koji.fedoraproject.org/koji/taskinfo?taskID=1068402
package built fine in dist-f10 for all supported arches.

The historic of this review is huge, I haven't read all for now.
So that's just few notes. (probably not a complete review).

* As the package is already hylafax, You don't need to have
Provides:  hylafax

* BuildRequires: ... gcc, gcc-c++ - Those aren't needed as they are implicitly added in the BuildDependency and shouldn't be mentioned. (older fedora version will need this indeed).

* Conflicts:   mgetty-sendfax - We need to find a solution to avoid conflict. and implement a proper alternative. Since this can probably not hit F-9/F-10

* http://koji.fedoraproject.org/koji/getfile?taskID=1068404&name=build.log
- JBIG library was not found on x86_64
Checking for JBIG library support
... not found. Disabling JBIG support
Can the support for this can be enabled ? (it needs to be added as BuildRequires first)

* Various checks are made at build time:
WARNING, could not locate sendmail on your system.
Beware that the mail notification work done by this software uses
sendmail-specific command line options.  If you do not have a
sendmail-compatible mailer things will break.
...
WARNING, no egetty program found, using /bin/egetty.
...
Looks like /usr/bin/gs is the PostScript RIP to use.
WARNING, /usr/bin/gs does not seem to be an executable program;

-> Does the necessary Requires: are requested for the runtime ?
either using Requires on the package, or the "virtual provides" or on the program path name.

* rpmlint on installed package:
[root@kwizatz Téléchargement]# rpmlint hylafax
hylafax.x86_64: E: executable-marked-as-config-file /etc/cron.hourly/hylafax
hylafax.x86_64: E: executable-marked-as-config-file /etc/cron.daily/hylafax
hylafax.x86_64: E: useless-provides hylafax
hylafax.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libfaxutil.so.5.2.8 /lib64/libm.so.6
hylafax.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.2.8 HYLAFAX_VERSION_STRING
hylafax.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libfaxserver.so.5.2.8 /lib64/libm.so.6
1 packages and 0 specfiles checked; 3 errors, 3 warnings.
-> the cron scripts should probably stay as %config files 
-> the undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.2.8 HYLAFAX_VERSION_STRING can probably be fixed.

* --with-PAGESIZE=A4 - I appreciate that my standard page format to be set as default, but is there a way to have this fixed at runtime while using system wide configuration files for localization ? (I haven't made runtime test, as i don't have the required hardware/phone connection.
Comment 98 Nicolas Chauvet (kwizart) 2009-01-20 07:55:44 EST
About:
* Conflicts:   mgetty-sendfax
I meant, when the package will be fixed, the conflicts will remain in version-release older than the one which got the fix.
Comment 99 Lee Howard 2009-02-28 17:25:57 EST
SPEC: http://downloads.sourceforge.net/hylafax/hylafax.spec
SRPM: http://downloads.sourceforge.net/hylafax/hylafax-5.2.9-1.src.rpm

> * As the package is already hylafax, You don't need to have
> Provides:  hylafax

Removed.

> * BuildRequires: ... gcc, gcc-c++ - Those aren't needed as they are implicitly
> added in the BuildDependency and shouldn't be mentioned. (older fedora version
> will need this indeed).

Left as-is to support older Fedora.

> * Conflicts:   mgetty-sendfax - We need to find a solution to avoid conflict.
> and implement a proper alternative. Since this can probably not hit F-9/F-10

HylaFAX was in RedHat 5.2.  mgetty-sendfax chose to develop its own "sendfax" command-line fax tool using the same "sendfax" name as HylaFAX (as well as the same /var/spool/fax spool directory - HylaFAX has since changed to /var/spool/hylafax).  Because of this conflict RedHat removed HylaFAX beginning at 6.0.  The conflict cannot be resolved because it was the mgetty-sendfax developer's intention to create an alternative for HylaFAX.

I do not wish to offend Gert Doering, but it is my recommendation to remove mgetty-sendfax from Fedora.  (Mostly because the last official release, 1.0, is dated 1998... although betas are available from 2007.)  However, as I suspect that my recommendation will not be followed, it is therefore my suggestion to implement a "system-switch-fax" similar to what has been done for sendmail/Postfix.

*IF* doing that work will finally get this package into Fedora - with no more hold-ups, then I will gladly go through the effort to develop system-switch-fax and make the necessary modifications to both the mgetty-sendfax package and this package.  *HOWEVER*, I do not desire to go through that effort only to find that we're yet hung up on something else.

Please advise.

> * http://koji.fedoraproject.org/koji/getfile?taskID=1068404&name=build.log
> - JBIG library was not found on x86_64
> Checking for JBIG library support
> ... not found. Disabling JBIG support
> Can the support for this can be enabled ? (it needs to be added as
> BuildRequires first)

The JBIG-KIT package is currently not in Fedora.  Other distributions (i.e. Gentoo) do include it.  However, there may be some patent encumbrances with respect to JBIG technology, and you may want to pass this with your legal team before including JBIG-KIT into Fedora.

> * Various checks are made at build time:
> WARNING, could not locate sendmail on your system.
> Beware that the mail notification work done by this software uses
> sendmail-specific command line options.  If you do not have a
> sendmail-compatible mailer things will break.
> ...
> WARNING, no egetty program found, using /bin/egetty.
> ...
> Looks like /usr/bin/gs is the PostScript RIP to use.
> WARNING, /usr/bin/gs does not seem to be an executable program;
> 
> -> Does the necessary Requires: are requested for the runtime ?
> either using Requires on the package, or the "virtual provides" or on the
> program path name.

The warnings can be ignored.  The Requires: should be sufficient, yes.  Those packages are not needed for building this package, but they are needed for runtime.

> -> the cron scripts should probably stay as %config files 
> -> the undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.2.8
> HYLAFAX_VERSION_STRING can probably be fixed.

Please advise on how it can probably be fixed.

> * --with-PAGESIZE=A4 - I appreciate that my standard page format to be set as
> default, but is there a way to have this fixed at runtime while using system
> wide configuration files for localization ? (I haven't made runtime test, as
> i don't have the required hardware/phone connection.

Yes, the defaults can be changed at runtime.  I'm not sure there is an issue here.
Comment 100 Fabian Affolter 2009-10-09 03:44:32 EDT
There is a new HylaFAx release out.

http://www.hylafax.org/content/HylaFAX_6.0.3/4.4.5/4.3.8_releases
Comment 101 Jason Tibbitts 2009-10-14 15:51:04 EDT
As far as I can tell, that's not even the same hylafax.  Which underpins the reason why this package will never be approved (by me, at least) without being renamed to hylafax+ as has been repeatly requested in this ticket.  Since comment #83 indicates that this won't happen, I don't even know why I still have this ticket assigned to myself.

So I'm just unassigning myself and returning this to the review queue.  As I do that, I'll make a few notes:

The package in comment #99 still builds OK in today's rawhide and rpmlint really doesn't complain about much.  In fact, I'll just post it here:

hylafax.x86_64: E: executable-marked-as-config-file /etc/cron.hourly/hylafax                                             
hylafax.x86_64: E: executable-marked-as-config-file /etc/cron.daily/hylafax                                              
hylafax.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.2.9 HYLAFAX_VERSION_STRING                     
hylafax.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libfaxserver.so.5.2.9 /lib64/libm.so.6                      
hylafax.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libfaxutil.so.5.2.9 /lib64/libm.so.6                        

The first two and last two are not problematic; I'm not really sure about the third one.  It was indicated that this should be easy to fix, but I can't suggest how to fix it.

The Conflicts: with mgetty-sendfax is problematic according to http://fedoraproject.org/wiki/Packaging:Conflicts.  I'd say that the best way out is to use alternatives as recommended by those guidelines, which requires coordination with the owner of the mgetty package (jskala@redhat.com) who should probably be added as a CC if this starts moving forward again.
Comment 102 Lee Howard 2009-10-14 17:08:39 EDT
Jason,

This is a very long-winded bug report.  So let me be clear about issues you bring up.

I am VERY happy to change the name of the package to "hylafax+" if that will get it into Fedora.  (Understand, that a package name change will cause me no small amount of support effort for the very large existing installation base.  See other notes describing inadequacies within RPM for dealing with it.)

So, if I change the name of the package to "hylafax+" will you be able to help it get into Fedora?  (I want to ensure that my effort in doing so actually results in something.)

As for the alternatives suggestion, I'm completely happy to work with jskala@redhat.com who I have now CC'ed on this report.
Comment 106 paul 2010-09-11 12:23:00 EDT
Hi there,

I am looking around for a rpm for latest Hylafax server for Fedora 13 (or 14?) but found Hylafax.org only offer package for Fedora 10.

I am not sure if this is right place to ask.

I have installed a hylafax server on a Redhat 9.0 and have just recently want to install another one on a F13.  Do you think the rpm for Fedora 10 built by hylafax.org is ok for installation on F13?

Where could I find more information about this hylafax server installation on Fedora (w/ SELinux).

Thanks in advance.

paul
Comment 107 Dave Ludlow 2010-09-11 14:03:13 EDT
Lee - I'm not a sponsor, so I can't do much directly.  You've clearly put a lot (!) of work into this and I just wanted to give you an "atta boy!".

This is a complicated package, for a lot of reasons.  Clearly you're still interested, since you continue to attach version updates, but it looks like Jason stopped having fun a while ago.  :)

My free, unsolicited advice is to hit https://lists.fedoraproject.org/mailman/listinfo/devel [good] or irc://irc.freenode.net/fedora-devel [better] to see if you can get a fresh sponsor or rekindle Jason's interest.
Comment 108 Dave Ludlow 2010-09-11 14:19:06 EDT
Paul from Comment #106 - There is no Fedora-supported answer at this time.  I would suggest the following links:

http://hylafax.sourceforge.net/support.php
http://sourceforge.net/projects/hylafax/files/

I appreciate your situation, but please keep this bug on the topic of Lee's packaging review and not support for the software itself.
Comment 109 Lee Howard 2010-09-11 15:59:15 EDT
Paul,

You can download a pre-built x64 Fedora 13 RPM for HylaFAX+ here:

http://sourceforge.net/projects/hylafax/files/hylafax%20Fedora%2013%20RPM/


Dave,

Thank you for the advice, I will soon pursue a fresh sponsor on IRC.

Thanks,

Lee.
Comment 110 paul 2010-09-13 08:08:54 EDT
Thanks Dave & Howard. It seems Howard you are maintaining a number of rpm for different systems.

Unfortunately, my f-13 system is not 64-bit.

Interestingly, I do not see a 5.4.2 version in Hylafax website?
Comment 111 Lee Howard 2010-09-14 00:51:28 EDT
Paul,

You can always download the SRPM and 'rpm --rebuild' it.

For the website see http://hylafax.sourceforge.net
Comment 113 Lee Howard 2010-12-20 02:26:58 EST
SRPM: http://downloads.sourceforge.net/hylafax/hylafax-5.5.0-1.src.rpm

For what it's worth, I did solicit a sponsor on IRC as suggested.  Although some brief discussion ensued nothing became of it other than comments similar to, "Oh, that package... I'm not going to touch it."

I understand the blocks to be as follows:

1) package naming - there is some objection to using "hylafax" as the package name, however, as yet no prospective sponsor has committed to sponsoring the package inclusion if the package name is changed to "hylafax+".

2) conflict with mgetty-sendfax - in order for package inclusion to occur there will necessarily need to be collaboration in the "alternatives" so that users can switch between hylafax and mgetty-sendfax.  (It is apparently unacceptable to have conflicting packages in the repository.)  But, again, collaboration is apparently not easy to come-by.
Comment 114 Christoph Wickert 2010-12-20 03:35:41 EST
For me the biggest problem is the volation of the FHS, e.g. binaries in %{faxspool}/bin/.
Comment 115 Lee Howard 2010-12-20 10:53:59 EST
Christoph,

If I resolve the issue with binaries in %{faxspool}/bin/ are you willing to sponsor the package for inclusion?

Thanks,

Lee.
Comment 116 Miroslav Suchý 2011-10-24 15:09:05 EDT
Lee,
this is amazing. 4 years and you did not gave up (you still didn't, isn't it?).
I will do this review.
Comment 117 Miroslav Suchý 2011-10-24 15:36:06 EDT
hylafax.i686: W: no-manual-page-for-binary hylafax
hylafax.i686: W: no-manual-page-for-binary faxsetup.linux
hylafax.i686: W: no-manual-page-for-binary faxmsg
hylafax.i686: W: no-manual-page-for-binary ondelay
hylafax.i686: W: no-manual-page-for-binary probemodem
hylafax.i686: W: no-manual-page-for-binary lockname
hylafax.i686: W: no-manual-page-for-binary typetest
I encourage you to write man page for this executables.

hylafax.i686: W: wrong-file-end-of-line-encoding /usr/share/doc/hylafax-5.5.0/COPYRIGHT
You have there probably DOS/WINDOWS style of end of line.

It is preferred to use %global rather then %define
http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define

Is there some reason to specify /usr/bin/tiffcp instead of libtiff-tools?

I'm going to talk to jskala (as he is the same building as me) about the conflicts and virtual providess.

I second that change the name to hylafax+ is probably best thing.

I briefly read all this BZ (as it is very long) and still does not understand (even after reading #92) why that stuff in /usr/spool/ is there? Can you elaborate why it could not be moved to /etc /usr/bin /var/log etc? And I'm sorry in advance, if you will repeat yourself and I missed it.
Comment 118 Lee Howard 2011-10-25 01:31:36 EDT
Hello Miroslav.  Thank you for taking this project.

I could write up man pages for those executables, but they'd be completely unused because those executables are not meant to be used except as tools by other executables which do already have man pages.

I've now changed the EOL codes on COPYRIGHT in the upstream CVS.

I've now changed %define to %global.

The reason for specifying /usr/bin/tiffcp is because libtiff-tools is a new package that was broken-off from libtiff.  Consequently, in order for the SPEC/SRPM to work properly on older Fedora systems before libtiff-tools was broken-off from libtiff, I specify /usr/bin/tiffcp.

I am happy to change the package name to hylafax+.

HylaFAX hfaxd operates chroot-ed to /var/spool/hylafax and needs access to at least some of the scripts in /var/spool/hylafax/bin and /var/spool/hylafax/etc.  So moving /var/spool/hylafax/bin and /var/spool/hylafax/etc to somewhere outside the chroot makes things very problematic.
Comment 119 Miroslav Suchý 2011-10-25 05:13:55 EDT
> I could write up man pages for those executables, but they'd be completely
> unused because those executables are not meant to be used except as tools by
> other executables which do already have man pages.

I understand that. And this is not MUST, but only SHOULD item. But still having *some* man page is nice thing. Even if it would be very short man page with something like:
"You should not run XXXX manually. This is called internally by YYYY(8).
See also YYYY(8)"

ad tiffcp - fair enough

ad chroot-ed environment - fair enough, I have no objection to content. But I do have objection to directory where it reside. Why you use /var/spool/hylafax?
Quoting:
http://www.pathname.com/fhs/pub/fhs-2.3.html#PURPOSE47
"/var/spool contains data which is awaiting some kind of later processing. Data in /var/spool represents work to be done in the future (by a program, user, or administrator); often data is deleted after it has been processed."
I would really recommend you to move it to /var/hylafax/chroot (similary as e.g. bind-chroot does). Is is viable?
Comment 120 Lee Howard 2011-10-26 03:33:00 EDT
While I'm not averse to creating man pages for every executable installed, I remain unconvinced and highly sceptical that doing so for executables not designed to be run by users would have any meaningful value.  Even in a minimal install Fedora has very numerous executables installed which do not have man pages, and as a Fedora use I have never once have wanted to run those executables directly or have been disappointed by there not being a man page for them.

Not everything that utilizes files and scripts from the traditional /var/spool/hylafax directory operates within the chroot, and so renaming that "spool" directory to /var/hylafax/chroot is misleading.  I would be agreeable to move it to /var/hylafax.  I think that this was suggested before in Comment #94.  However, both this and your suggestion seems to violate the FHS:

http://www.pathname.com/fhs/pub/fhs-2.3.html#PURPOSE31

"Applications must generally not add directories to the top level of /var. Such directories should only be added if they have some system-wide implication, and in consultation with the FHS mailing list."

For what it may be worth, the contents of the "HylaFAX spool" directory are as follows:

drwx------ 2 uucp uucp  4096 2010-05-02 20:50 archive
drwxr-xr-x 3 root root  4096 2010-07-30 15:41 bin
drwxr-xr-x 2 uucp uucp  4096 2011-10-25 11:08 client
drwxr-xr-x 2 root root  4096 2010-07-30 15:41 config
drwxr-xr-x 2 root root  4096 2010-05-02 20:50 dev
drwx------ 2 uucp uucp  4096 2011-10-25 13:01 docq
drwx------ 2 uucp uucp  4096 2011-10-25 12:01 doneq
drwxr-xr-x 2 uucp uucp  4096 2011-10-25 03:47 etc
drwxr-xr-x 2 uucp uucp  4096 2011-10-25 11:08 info
drwxr-xr-x 2 uucp uucp 77824 2011-10-25 16:55 log
drwx------ 2 uucp uucp  4096 2010-05-02 20:50 pollq
drwxr-xr-x 2 uucp uucp 36864 2011-10-25 16:55 recvq
drwx------ 2 uucp uucp  4096 2011-10-25 11:08 sendq
drwxr-xr-x 2 uucp uucp  4096 2010-05-02 20:50 status
drwx------ 2 uucp uucp  4096 2011-10-25 11:06 tmp

Additionally, there are FIFOs for each modem and one for faxq created there.

The directories archive, client, docq, doneq, info, log, pollq, recvq, sendq, status, and tmp are all true spool directories per the FHS definition you cite.

The bin, dev, and etc directories are the ones raising concerns (and namely bin), and they all are there so that they can be accessible to the hfaxd client from within the chroot.

The dev directory is there for access to /dev/null.  The etc directory is there so that the administrative hfaxd client could manipulate configuration files.  The bin directory is only there for needful operations within the chroot.

You can maybe think of it as a type of hybrid between lpd and ftp.  Imagine a printing client/driver that could send print jobs to the server but also retrieve copies of previous print jobs and change various types of operations in the print server.

Recognize that HylaFAX heralds from a time 20 years ago when FHS didn't exist.  So it's not like HylaFAX was developed in direct violation of FHS - rather, FHS was developed without consideration of HylaFAX.

Both Gentoo and Debian have HylaFAX ports, and both have left /var/spool/hylafax there.  (However, in attempting to address the FHS concern Debian has done some cumbersome synchronizing work to duplicate files from /var/spool/hylafax/bin and /var/spool/hylafax/etc into /etc/hylafax - or something like that.  I'm not sure how this helps alleviate the FHS concern, though.)

My perspective on this is that the FHS just does not have an appropriate categorization for service-level applications that allow client access to scripts and configuration files from within a chroot that 99% of the time is used for spool purposes.

However, if moving /var/spool/hylafax to /var/hylafax will resolve the concerns, then I am willing to do that.
Comment 121 Miroslav Suchý 2011-10-26 06:48:46 EDT
That argument about Gentoo and Debian is valid for me. IMO if other distributions and especially Debian accepted some structure, we should not be so über dogmatic and follow what users are used to do for 20 years. So I accept that /var/spool.

ad man page - I do not force you. I'm just saying it would be nice to have it. This is not requirement to pass this review. I'm just kindly asking. Feel free to ignore me.
Comment 122 Michal Sekletar 2011-10-27 13:15:07 EDT
Hello Lee,

I am new maintainer of mgetty in Fedora. I've checked conflict with my package. At first glance it seems like there are two paths which are common for hylafax and sendfax, it's /usr/bin/faxrm binary and it's manpage.
I'm afraid that we can't work this out using Alternatives because I believe that each conflicting binary is manipulating another fax job queue. I will investigate further and I will try to find some solution to this issue. 
Another problem is general confusion caused by both packages installed at the same time. Many binaries have same names and they differ "only" by install location in filesystem. From user point of view it might be very confusing to see "the same" binary twice, one in /usr/bin and the other located in /usr/sbin.
If you know already about some possible solution to these issues, please let me know. Please feel free to correct me if I've made some mistake. I am not very familiar with your package so far, but I will be happy to see hylafax included in Fedora because as I read through the comments it seems that you've put some extraordinary effort to get hylafax included.
Comment 123 Trevor Cordes 2012-06-12 06:44:22 EDT
Glad to see the interest in hylafax here.  I use hylafax on about a dozen servers and it's great.  I also use mgetty/vgetty, but not both on the same computer :-)  They each have their uses and should both be in Fedora.

I'd love to see hylafax be included in Fedora repos.

For the longest time I use the rpm's provided elsewhere linked to by the hylafax project pages.  They work well.

I just ran into an issue that I just want to note here for posterity: hylafax will require a dep on libtiff-tools for pdf-email-attachments to work.  PDF attachments magically disappeared from my servers a long while back and I know now why: libtiff-tools were broken out and the fax2ps program was gone.
Comment 124 Miroslav Suchý 2012-08-04 12:29:47 EDT
ping, Michal, did you find some solution?
Comment 125 Michal Sekletar 2012-08-06 04:28:35 EDT
I think only viable solution here, how to overcome the conflicts with mgetty-sendfax is to rename binaries in hylafax. I've looked at Debian packages and they have conflicts between each other and nobody seems to care, but I don't think this is acceptable for Fedora.
Comment 126 Lee Howard 2012-10-13 19:45:00 EDT
SPEC: http://downloads.sourceforge.net/hylafax/hylafax.spec
SRPM: http://downloads.sourceforge.net/hylafax/hylafax-5.5.2-1.src.rpm

With respect to the conflict with mgetty-sendfax... Please forgive my naievety, but what's wrong with leaving "Conflicts: mgetty-sendfax" as it is?  IMO that resolves the problem entirely as it clearly tells the user that they can install hylafax OR mgetty-sendfax but not both.
Comment 127 Michal Sekletar 2012-10-15 04:43:47 EDT
I believe I followed Packaging Guidelines[1], when I proposed renaming or prefixing binaries. If this is not acceptable for hylafax, then I think only solution here is approaching Fedora Packaging Committee and making the case there (my opinion is based on facts stated in [2][3]).

Please note that I personally don't have a problem with Conflicts: mgetty-sendfax in hylafax spec file, however I want to be sure that we are following guidelines here.

References:
[1] http://fedoraproject.org/wiki/Packaging:Conflicts
[2] http://fedoraproject.org/wiki/Packaging:Conflicts#Binary_Name_Conflicts
[3] http://fedoraproject.org/wiki/Packaging:Conflicts#Other_Uses_of_Conflicts:
Comment 128 Lee Howard 2012-10-15 13:04:24 EDT
Hello Michal,

How do we approach the Fedora Packaging Committee for approval on this conflict?

HylaFAX users are quite accustomed to using 'sendfax' and 'faxrm', and there are extremely numerous scripts written by those users referring to 'sendfax' and 'faxrm' such that renaming those binaries is simply unfeasible.

There are undoubtedly many thousands of HylaFAX installs out there in production systems sending/receiving millions of faxes.  HylaFAX has been around since 1991, and when Gert Doering later wrote the "sendfax" part of mgetty-sendfax he did so in-part as an alternative to HylaFAX.  In other words, he designed mgetty-sendfax to conflict deliberately.  (That's not wrong or shameful at all, but it's significant to recognize that the author's intention was to never have both packages installed simultaneously.)

HylaFAX was in RedHat 5.2 and was removed (apparently due to the conflict with mgetty-sendfax - and I can't figure out why they chose mgetty-sendfax over HylaFAX or why the conflict was a problem then), but HylaFAX has substantially more users and is more-actively developed/maintained.

Consequently, users of both Fedora and HylaFAX must always install HylaFAX independently from Fedora's repositories.  For those of us well-acquainted with that procedure, it's not a problem, but new users who have no distribution preference will be tempted to use Debian or Gentoo where they can expect to find HylaFAX without going outside of the distribution repositories and where they can expect updates, etc, without paying much attention to the HylaFAX mailing lists.
Comment 129 Lee Howard 2012-10-31 16:17:30 EDT
https://fedoraproject.org/wiki/Packaging:Conflicts#Incompatible_Binary_Files_with_Conflicting_Naming_.28and_stubborn_upstreams.29

Okay, so it appears that we have a go-ahead with the Conflicts.

Michal, you'll need to add a Conflicts on your side, too.

Miroslav, where do we go from here?
Comment 130 Darren Nickerson 2012-10-31 16:49:48 EDT
It would seem the next step would be to rename this package HylaFAX+, since this is a fork of HylaFAX, and should be clearly labeled thus to avoid confusion with the original HylaFAX found at www.hylafax.org.

-Darren
Comment 131 Miroslav Suchý 2012-11-01 04:08:14 EDT
I agree to #130.

And since biggest blocker is solved we can focus on minorities.

1) Please convert your init.d script to systemd service.

2)  buildroot and its cleaning in %install and %clean is not needed unless you target EPEL5.

3) %files section is too much verbose

E.g. this:
%attr(755,root,root) %dir %{_sysconfdir}/hylafax
can be safely written as:
%dir %{_sysconfdir}/hylafax
because it will take the attr from buildroot, where you already set it up correctly with
mkdir -p -m 755 $RPM_BUILD_ROOT%{_sysconfdir}/hylafax

And generaly setting attr to 755 or 644 is not needed because this is default.

%defattr(-,root,root,-) is not needed unless you target EPEL5.

So your %files section can look like:

%files
%doc CHANGES CONTRIBUTORS COPYRIGHT README TODO VERSION
%{_initrddir}/hylafax
%config(noreplace) %{_sysconfdir}/cron.daily/hylafax
....
%{faxspool}/etc/LiberationSans-25.pcf
%defattr(-,uucp,uucp,-)
%dir %{faxspool}
%dir %{faxspool}/archive
...
Comment 132 Lee Howard 2012-11-02 00:32:25 EDT
SPEC: http://downloads.sourceforge.net/hylafax/hylafax+.spec
SRPM: http://downloads.sourceforge.net/hylafax/hylafax+-5.5.2-2.src.rpm

Miroslav,

I'd like the SPEC/SRPM to be buildable on multiple RPM-based distributions... especially as many current and past RedHat, CentOS, and Fedora releases as possible.  Can you frame your statements #1, #2 and #3 in Comment #131 accordingly?  In other words, can you offer any suggestion on how to make those changes and still have the SRPM build properly on older Fedora releases? 

Please note the package name change.

Thanks,

Lee.
Comment 133 Miroslav Suchý 2012-11-02 09:03:43 EDT
ad 1) 
you can have sysv and systemd together. On Fedora 17+ you install systemd everywhere else sysv.
You may check
https://fedorahosted.org/spacewalk/browser/client/rhel/rhnsd/rhnsd.spec
for example.

2) ok, you can leave it there.

3) still valid. you may leave initial %defattr for more compatibility, but you still can write it as:

%files
%defattr(-,root,root,-)
%doc CHANGES CONTRIBUTORS COPYRIGHT README TODO VERSION
%{_initrddir}/hylafax
%config(noreplace) %{_sysconfdir}/cron.daily/hylafax
....
%{faxspool}/etc/LiberationSans-25.pcf
%defattr(-,uucp,uucp,-)
%dir %{faxspool}
%dir %{faxspool}/archive

and it will be valid everywhere.

another issues:

[!]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
     Note: These BR are not needed: gcc gcc-c++ make


hylafax+.src:19: W: unversioned-explicit-provides hylafax

hylafax+.x86_64: E: executable-marked-as-config-file /etc/cron.hourly/hylafax
hylafax+.x86_64: E: executable-marked-as-config-file /etc/cron.daily/hylafax
hylafax+.x86_64: E: script-without-shebang /var/spool/hylafax/bin/genfontmap.ps
hylafax+.x86_64: E: script-without-shebang /var/spool/hylafax/bin/auto-rotate.ps
hylafax+.x86_64: W: no-manual-page-for-binary hylafax
hylafax+.x86_64: W: no-manual-page-for-binary faxsetup.linux
hylafax+.x86_64: W: no-manual-page-for-binary faxfetch
hylafax+.x86_64: W: no-manual-page-for-binary faxmsg
hylafax+.x86_64: W: no-manual-page-for-binary ondelay
hylafax+.x86_64: W: no-manual-page-for-binary probemodem
hylafax+.x86_64: W: no-manual-page-for-binary lockname
hylafax+.x86_64: W: no-manual-page-for-binary typetest

hylafax+.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libfaxutil.so.5.5.2 /lib64/libm.so.6
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 HYLAFAX_VERSION_STRING
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 jbg_enc_out
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsSetErrorHandler
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsDoTransform
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsSample3DGrid
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsCreateTransform
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsClampLab
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsCloseProfile
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsSetDeviceClass
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsAlloc3DGrid
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsAddTag
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 jbg_enc_options
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 jbg_enc_free
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 jbg_enc_init
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsFreeLUT
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsAllocLUT
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsLabEncoded2Float
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsSetColorSpace
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsFloat2LabEncoded
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsCreate_sRGBProfile
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsSetPCS
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 cmsDeleteTransform
hylafax+.x86_64: W: undefined-non-weak-symbol /usr/lib64/libfaxserver.so.5.5.2 _cmsCreateProfilePlaceholder

If you are unsure what to do with some of these warning, do not hesitate to ask me.
Comment 134 Michal Sekletar 2012-11-02 09:37:57 EDT
(In reply to comment #129)
> Michal, you'll need to add a Conflicts on your side, too.

Which releases do you plan to target once HylaFax is included? Git repository for a new package has by default only rawhide branch and you have to request others if you want to. I need to know that so I can update my branches accordingly.
Comment 135 Lee Howard 2012-11-04 22:36:53 EST
SPEC: http://downloads.sourceforge.net/hylafax/hylafax+.spec
SRPM: http://downloads.sourceforge.net/hylafax/hylafax+-5.5.2-3.src.rpm

Michal, I expected to target Fedora 18.  If it's possible to still get included into Fedora 16 and 17 I would like to do that.  However, I would imagine that it's acceptable on your side to simply add the Conflicts for all releases.

Miroslav, this version now switches from SysV to systemd, as requested.  There will likely be some improvements to what I've done, but I think this gets it going.  It simplifies the SPEC as suggested.

Here is the rpmlint output against the SRPM and the installed package with explanations...

# rpmlint /root/rpmbuild/SRPMS/hylafax+-5.5.2-3.fc16.src.rpm
hylafax+.src:57: W: configure-without-libdir-spec

This is expected because HylaFAX configure does not support libdir.  See an earlier comment for more explanation.

# rpmlint hylafax+ | sort
1 packages and 0 specfiles checked; 23 errors, 11 warnings.
hylafax+.x86_64: E: executable-marked-as-config-file /etc/cron.daily/hylafax
hylafax+.x86_64: E: executable-marked-as-config-file /etc/cron.hourly/hylafax

These *are* config files since the user is expected to modify these cron-run scripts as desired.

hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/de 0644L /bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/en 0644L /bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/es 0644L /bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/fr 0644L /bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/it 0644L /bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/nl_BE 0644L /bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/pl 0644L /bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/pt 0644L /bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/pt_BR 0644L /bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/ro 0644L /bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/sr 0644L /bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/tr 0644L /bin/bash

These are all shell "snippets" which are included via "." in other scripts.

hylafax+.x86_64: E: non-readable /var/spool/hylafax/etc/hosts.hfaxd 0600L

This file is supposed to be this way for security purposes.

hylafax+.x86_64: E: non-standard-dir-perm /var/spool/hylafax/archive 0700L
hylafax+.x86_64: E: non-standard-dir-perm /var/spool/hylafax/docq 0700L
hylafax+.x86_64: E: non-standard-dir-perm /var/spool/hylafax/doneq 0700L
hylafax+.x86_64: E: non-standard-dir-perm /var/spool/hylafax/pollq 0700L
hylafax+.x86_64: E: non-standard-dir-perm /var/spool/hylafax/sendq 0700L
hylafax+.x86_64: E: non-standard-dir-perm /var/spool/hylafax/tmp 0700L

These are all as intended for security purposes.

hylafax+.x86_64: E: script-without-shebang /var/spool/hylafax/bin/auto-rotate.ps
hylafax+.x86_64: E: script-without-shebang /var/spool/hylafax/bin/genfontmap.ps

These are not shell scripts, they are postscript... and they are correct as-is.

hylafax+.x86_64: W: no-manual-page-for-binary faxfetch
hylafax+.x86_64: W: no-manual-page-for-binary faxmsg
hylafax+.x86_64: W: no-manual-page-for-binary faxsetup.linux
hylafax+.x86_64: W: no-manual-page-for-binary hylafax
hylafax+.x86_64: W: no-manual-page-for-binary lockname
hylafax+.x86_64: W: no-manual-page-for-binary ondelay
hylafax+.x86_64: W: no-manual-page-for-binary probemodem
hylafax+.x86_64: W: no-manual-page-for-binary typetest

This is correct... there are no man pages for these binaries.  Someday maybe there will be.

hylafax+.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libfaxserver.so.5.5.2 linux-vdso.so.1
hylafax+.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libfaxutil.so.5.5.2 /lib64/libm.so.6
hylafax+.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libfaxutil.so.5.5.2 linux-vdso.so.1

These are not the fault of this package.  The linux-vdso.so.1 unused dependency is an issue in Bug 738082, and the libm.so.6 unused dependency is an issue inherited from libtiff, I suspect.
Comment 136 Miroslav Suchý 2012-12-03 12:59:10 EST
># rpmlint hylafax+ | sort
>1 packages and 0 specfiles checked; 23 errors, 11 warnings.
>hylafax+.x86_64: E: executable-marked-as-config-file /etc/cron.daily/hylafax
>hylafax+.x86_64: E: executable-marked-as-config-file /etc/cron.hourly/hylafax
>
>These *are* config files since the user is expected to modify these cron-run >scripts as desired.

I asked on packaging mailing list and it seems that 50% of developers think it should be config and 50% think it should not be config.
I think it should not be config, but I will not argue here. And I will not block review for this.

> hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dict/tr 0644L /bin/bash
>
>These are all shell "snippets" which are included via "." in other scripts.

Then you should remove that shebang. If it is not meant to direct execution, then it should not have #! at first line. If there is #! on first line, then it should have executable flag. That is rule.

>This is correct... there are no man pages for these binaries.  Someday maybe there will be.
Writing man page is very easy. You can write it in perldoc or in asiidoc, which is more or less just wiki syntax.
Here you can see examples for inspiration:
Asciidoc:
https://raw.github.com/dgoodwin/tito/master/tito.8.asciidoc
https://raw.github.com/dgoodwin/tito/master/tito.spec
Perldoc:
https://raw.github.com/Katello/katello/master/katello-configure/man/katello-configure.pod
https://raw.github.com/Katello/katello/master/katello-configure/katello-configure.spec
But I will not block review for this. But please consider it. Two senteces, which describe what this command do is just enough.

I accept all others waives. So only remaining blocking issue are those files with shebang without executable permission.
Comment 137 Lee Howard 2012-12-04 00:10:34 EST
SPEC: http://downloads.sourceforge.net/hylafax/hylafax+.spec
SRPM: http://downloads.sourceforge.net/hylafax/hylafax+-5.5.2-4.src.rpm

I've added the missing man pages, and I've removed the undesired shebangs.
Comment 138 Miroslav Suchý 2012-12-04 03:59:28 EST
Key:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed


===== MUST items =====

C/C++:
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig called in %post and %postun if required.
[x]: Package does not contain any libtool archives (.la)
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
     see #82
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[-]: %build honors applicable compiler flags or justifies otherwise.
     smp flags breaks build libfaxutil
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package contains no bundled libraries.
     I see only regexp, but it seems others (nvi, mysql, php, bundle that as well).
[x]: Changelog in prescribed format.
[-]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required
     waiving - submiter want epel5
[x]: Sources contain only permissible code or content.
[x]: %config files are marked noreplace or the reason is justified.
[!]: Each %files section contains %defattr if rpm < 4.4
     Note: %defattr present but not needed
[x]: Macros in Summary, %description expandable at SRPM build time.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[x]: Package is not known to require ExcludeArch.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package complies to the Packaging Guidelines
[x]: Spec file lacks Packager, Vendor, PreReq tags.
[x]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "MIT/X11 (BSD like)", "Unknown or generated", "BSD (4 clause)". 3 files
     have unknown license. 
     see #82
[x]: Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: Package is named using only allowed ASCII characters.
[x]: Package is named according to the Package Naming Guidelines.
[x]: No %config files under /usr.
[x]: Package does not generate any conflict.
     Note: Package contains Conflicts: tag(s) needing fix or justification.
     See from #122 till #129
[x]: Package do not use a name that already exist
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package installs properly.
[x]: Package is not relocatable.
[x]: Requires correct, justified where necessary.
[x]: CheckResultdir
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file is legible and written in American English.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: Package contains systemd file(s) if in need.
[x]: File names are valid UTF-8.
[x]: Useful -debuginfo package or justification otherwise.
[-]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 112640 bytes in 6 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
     Note: Buildroot neede for el5
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: %clean neede for el5
[-]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Patches link to upstream bugs/comments/lists or are otherwise justified.
[x]: The placement of pkgconfig(.pc) files are correct.
[!]: Scriptlets must be sane, if used.
     systemd scripts are incorrect
[x]: SourceX tarball generation or download is documented.
[!]: SourceX / PatchY prefixed with %{name}.
     Note: Patch0 (hylafax-clearlinkage.patch) Patch1 (hylafax-
     moremanpages.patch) Patch2 (hylafax-badshebangs.patch) Source2
     (hylafax_daily.cron) Source3 (hylafax_hourly.cron) Source0
     (hylafax-5.5.2.tar.gz) Source1 (hylafax_rh.init) Source4
     (hylafax_hfaxd_systemd.service) Source5 (hylafax_faxq_systemd.service)
[x]: SourceX is a working URL.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[?]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Spec use %global instead of %define.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.

Nice to fix:
[!]: Each %files section contains %defattr if rpm < 4.4
     Note: %defattr present but not needed
     I incorrectly said in #131 that is is needed for el5, but el5 has rpm 4.4.2, so you can remove it completly.
[!]: SourceX / PatchY prefixed with %{name}.
     Note: Patch0 (hylafax-clearlinkage.patch) Patch1 (hylafax-
     moremanpages.patch) Patch2 (hylafax-badshebangs.patch) Source2
     (hylafax_daily.cron) Source3 (hylafax_hourly.cron) Source0
     (hylafax-5.5.2.tar.gz) Source1 (hylafax_rh.init) Source4
     (hylafax_hfaxd_systemd.service) Source5 (hylafax_faxq_systemd.service)
Blockers:
[!]: Scriptlets must be sane, if used.
     systemd scripts are incorrect
     see http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Manual_scriptlets_.28Fedora_17_or_older.29
     and to have unitdir macro you have to buildrequire  systemd-units
  see http://fedoraproject.org/wiki/Packaging:Systemd#Filesystem_locations

During installation I get:
Updating / installing...
   1:hylafax+-5.5.2-4.fc17            warning: /var/spool/hylafax/FIFO created as /var/spool/hylafax/FIFO.rpmnew
################################# [100%]
Is this expected?

new rpmlint errors:
hylafax+.x86_64: W: non-executable-in-bin /usr/sbin/faxcron 0444L
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/faxrcvd 0444L /usr/bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/pcl2fax 0444L /usr/bin/bash
hylafax+.x86_64: W: non-executable-in-bin /usr/sbin/xferfaxstats 0444L
hylafax+.x86_64: E: non-executable-script /usr/sbin/xferfaxstats 0444L /usr/bin/bash
hylafax+.x86_64: W: non-executable-in-bin /usr/sbin/faxsetup.linux 0444L
hylafax+.x86_64: E: non-executable-script /usr/sbin/faxsetup.linux 0444L /usr/bin/bash
hylafax+.x86_64: W: non-executable-in-bin /usr/sbin/hylafax 0444L
hylafax+.x86_64: E: non-executable-script /usr/sbin/hylafax 0444L /usr/bin/bash
hylafax+.x86_64: E: non-executable-script /etc/hylafax/faxmail/application/octet-stream 0444L /usr/bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/tiff2fax 0444L /usr/bin/bash
hylafax+.x86_64: W: non-executable-in-bin /usr/sbin/faxsetup 0444L
hylafax+.x86_64: E: non-executable-script /usr/sbin/faxsetup 0444L /usr/bin/bash
hylafax+.x86_64: W: non-executable-in-bin /usr/sbin/probemodem 0444L
hylafax+.x86_64: E: non-executable-script /usr/sbin/probemodem 0444L /usr/bin/bash
hylafax+.x86_64: W: non-executable-in-bin /usr/sbin/faxaddmodem 0444L
hylafax+.x86_64: E: non-executable-script /usr/sbin/faxaddmodem 0444L /usr/bin/bash
hylafax+.x86_64: W: non-executable-in-bin /usr/sbin/edit-faxcover 0444L
hylafax+.x86_64: E: non-executable-script /usr/sbin/edit-faxcover 0444L /usr/bin/bash
hylafax+.x86_64: W: non-executable-in-bin /usr/sbin/recvstats 0444L
hylafax+.x86_64: E: non-executable-script /usr/sbin/recvstats 0444L /usr/bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/pollrcvd 0444L /usr/bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/tiff2pdf 0444L /usr/bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/wedged 0444L /usr/bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/common-functions 0444L /usr/bin/bash
hylafax+.x86_64: E: non-executable-script /etc/hylafax/faxmail/application/pdf 0444L /usr/bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/qp-encode.awk 0444L /usr/bin/gawk
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/pdf2fax.gs 0444L /usr/bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/rfc2047-encode.awk 0444L /usr/bin/gawk
hylafax+.x86_64: E: non-executable-script /etc/hylafax/faxmail/image/tiff 0444L /usr/bin/bash
hylafax+.x86_64: E: non-executable-script /var/spool/hylafax/bin/dictionary 0444L /usr/bin/bash

I guess that those in /usr/sbin should be marked as executable and tohse in /var/spool should have removed shebang (or marked as executable).
Comment 139 Lee Howard 2012-12-08 18:46:28 EST
SPEC: http://downloads.sourceforge.net/hylafax/hylafax+.spec
SRPM: http://downloads.sourceforge.net/hylafax/hylafax+-5.5.2-5.src.rpm

I've removed %defattr for root from %files.

I've renamed the source and patch files to be hylafax+

I've modified the scriptlets for systemd.  If they still are incorrect please be specific about the problems because I think that they are correct according to http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Manual_scriptlets_.28Fedora_17_or_older.29

I've added a BuildRequires on /bin/systemctl.  I did it this way instead of based on systemd-units because there is no systemd-units package in Fedora 18.

I don't know why the /var/spool/hylafax/FIFO file was marked as config.  I've removed this as I believe it to be an error.

As for the new non-executable-* rpmlint errors, I cannot reproduce them here after many, many attempts.  Please check again and tell me how to reproduce the problem.
Comment 140 Miroslav Suchý 2012-12-10 06:21:12 EST
This:
%if 0%{?fedora} >= 16
BuildRequires: /bin/systemctl
%endif
is little bit hackish. We need it for definiton of macro %{_unitdir}. Which is provided by the same package, which provides this file. I would rather see there comment, like:
# this it to load definiton of macro %{_unitdir}
or even better write it as:

%if 0%{?fedora} == 16 || 0%{?fedora} == 17
BuildRequires: systemd-units
%endif
%if 0%{?fedora} >= 18
BuildRequires: systemd
%endif

And you will know that you can remove the first if-condition as soon as F17 is EOLed.
But that is just my POV.


== systemd scriplets ==

ad %postun:
you have:
 if [ "$1" = "1" ]; then
please note, that $1 is set to number of installed packages, which may be in some rare situations be greater then one (e.g. in multilib). You will be safe with -ge operator:
 if [ 0$1 -eq 1 ]; then
The same apply for %post

ad %post:
/bin/systemctl enable
and
/bin/systemctl start
which automatically enable and start those services. Which is not what we want.
Installations can be in changeroots, in an installer context, or in other situations where you don't want the services autostarted. 

Please use simply:
if [ $1 -eq 1 ] ; then 
    # Initial installation 
    /bin/systemctl daemon-reload >/dev/null 2>&1 || :
fi

And let admin do:
  chkconfig $daemon on
if he really want to enable it.



> As for the new non-executable-* rpmlint errors, I cannot reproduce them here after many, many attempts.  Please check again and tell me how to reproduce the problem.

Use my koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4772305
Install it and see:
# ls -l /var/spool/hylafax/bin/pcl2fax
-r--r--r-- 1 root root 6575 Dec 10 11:54 /var/spool/hylafax/bin/pcl2fax
# head -n1 /var/spool/hylafax/bin/pcl2fax
#! /usr/bin/bash

>I don't know why the /var/spool/hylafax/FIFO file was marked as config.  I've removed this as I believe it to be an error.
Now it is removed completly - whole file, not just config flag. Are you sure you will not miss it in runtime?
Comment 141 Lee Howard 2012-12-10 23:21:13 EST
Thank you for being so explicit.  I clearly didn't see the -ge in %postun.  In my defense with %post, I was merely following what I understood the statement, "If your service should be enabled by default" to be saying.  Anyway, I've made the changes as you've suggeted to %post and %postun and have made %preun consistent as well.

I have followed your advice for BuildRequires for %{_unitdir}.

The /var/spool/hylafax/FIFO file is created when faxq starts.  It should never have been put there by the RPM.  It is correct.

Looking at your koji scratch build I see an inconsistency.  From http://kojipkgs.fedoraproject.org//work/tasks/2305/4772305/build.log...

/usr/bin/bash ../port/install.sh  -idb hylafax.sw.server -root / -F /builddir/build/BUILDROOT/hylafax+-5.5.2-5.fc18.x86_64/usr/sbin -m 755 -O dialtest typetest
/usr/bin/bash ../port/install.sh  -idb hylafax.sw.server -root / -F /builddir/build/BUILDROOT/hylafax+-5.5.2-5.fc18.x86_64/usr/sbin -m 755 -src xferfaxstats.sh -O xferfaxstats

Here the HylaFAX install script installs dialtest, typetest, and then xferfaxstats all with mode 755.  However, it would appear that the HylaFAX install script did not work as expected...

$ rpm -qpl --dump hylafax+-5.5.2-5.fc18.x86_64.rpm | egrep "dialtest|typetest|xferfaxstats" | grep /sbin/
/usr/sbin/dialtest 15672 1355136852 e3512f07f8e6431f999a9574a723570e19af6757bc9c28f72c115cc06d7c1d48 0100755 root root 0 0 0 X
/usr/sbin/typetest 11584 1355136852 fc0f20c2d83afe3a7c289061bca02bfb2f64f35e2739072ef06f6d68b1b88932 0100755 root root 0 0 0 X
/usr/sbin/xferfaxstats 19346 1355136846 2cee8af27a95214848d005e3e36bfe8b5306a137b47dcb793b9cf8c08289de0f 0100444 root root 0 0 0 X

... as we can see that the mode for xferfaxstats is 444 instead of 755 as it should be per the install.sh invocation.

This behavior is different than what I am seeing on Fedora 16 and 17.  In an attempt to debug this problem I have downloaded the Fedora 18 beta, but it will not install for me (I can never get past the disk setup/partitioning part).  Do you have any suggestion on how I can work on Fedora 18 to debug?
Comment 142 Miroslav Suchý 2012-12-11 11:52:35 EST
1)
You can build scratch builds in Koji even when you are not Fedora packager yet.
Follow:
https://fedoraproject.org/wiki/Using_the_Koji_build_system#Fedora_Account_System_.28FAS2.29_Setup
https://fedoraproject.org/wiki/Using_the_Koji_build_system#Scratch_Builds

2) If you want really build localy (which is really better for debuging) you can try to build in mock:
https://fedoraproject.org/wiki/Using_Mock_to_test_package_builds
This will install minimal installation in chroot and build package in that chrooted directory. So you can build package for F19,18,17, EL6,5 or whatever and not have it really installed.
Comment 143 Lee Howard 2012-12-11 23:54:05 EST
SPEC: http://downloads.sourceforge.net/hylafax/hylafax+.spec
SRPM: http://downloads.sourceforge.net/hylafax/hylafax+-5.5.2-6.src.rpm

Using mock was very helpful, thank you.  So, it turns out that because mock runs as an unprivileged user it cannot do chmod and chown as the HylaFAX installer expects.  This is why the several %defattr and %attr in the %files section are necessary.
Comment 144 Miroslav Suchý 2012-12-12 04:43:26 EST
Very nice.

APPROVED
Comment 145 Peter Lemenkov 2012-12-12 04:51:19 EST
(In reply to comment #144)
> Very nice.
> 
> APPROVED

I. JUST. CANT. BELIEVE. THIS.

It took ~6 years in a row to resolve this. Congrats to all involved!
Comment 146 Peter Lemenkov 2012-12-12 04:58:57 EST
Lee, what's your FAS account - I'll sponsor you if you not sponsored yet.
Comment 147 Miroslav Suchý 2012-12-12 05:03:11 EST
Peter, I'm sponsor and I'm already communicating off-bz with Lee regarding sponsoring.
Comment 148 Peter Lemenkov 2012-12-12 05:05:36 EST
(In reply to comment #147)
> Peter, I'm sponsor and I'm already communicating off-bz with Lee regarding
> sponsoring.

Ok.
Comment 149 Miroslav Suchý 2012-12-13 03:26:18 EST
I just sponsored Lee. Clearing FE-NEEDSPONSOR.
Comment 150 Lee Howard 2012-12-13 19:40:38 EST
New Package SCM Request
=======================
Package Name: hylafax+
Short Description: an enterprise-strength fax server
Owners: faxguy
Branches: f16 f17 f18 el5 el6
InitialCC:
Comment 151 Jon Ciesla 2012-12-14 08:40:22 EST
Git done (by process-git-requests).
Comment 152 Fedora Update System 2012-12-15 15:57:15 EST
hylafax+-5.5.2-6.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/hylafax+-5.5.2-6.fc16
Comment 153 Fedora Update System 2012-12-15 15:59:52 EST
hylafax+-5.5.2-6.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/hylafax+-5.5.2-6.fc17
Comment 154 Fedora Update System 2012-12-15 16:01:33 EST
hylafax+-5.5.2-6.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/hylafax+-5.5.2-6.fc18
Comment 155 Fedora Update System 2012-12-15 16:02:35 EST
hylafax+-5.5.2-7.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/hylafax+-5.5.2-7.el5
Comment 156 Fedora Update System 2012-12-15 16:03:24 EST
hylafax+-5.5.2-7.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/hylafax+-5.5.2-7.el6
Comment 157 Fedora Update System 2012-12-16 14:08:51 EST
hylafax+-5.5.2-6.fc18 has been pushed to the Fedora 18 testing repository.
Comment 158 Fedora Update System 2012-12-25 23:54:18 EST
hylafax+-5.5.2-6.fc16 has been pushed to the Fedora 16 stable repository.
Comment 159 Fedora Update System 2012-12-25 23:56:53 EST
hylafax+-5.5.2-6.fc17 has been pushed to the Fedora 17 stable repository.
Comment 160 Fedora Update System 2013-01-11 19:42:03 EST
hylafax+-5.5.2-6.fc18 has been pushed to the Fedora 18 stable repository.
Comment 161 Fedora Update System 2013-02-02 14:40:36 EST
hylafax+-5.5.2-7.el6 has been pushed to the Fedora EPEL 6 stable repository.
Comment 162 Fedora Update System 2013-02-02 14:41:51 EST
hylafax+-5.5.2-7.el5 has been pushed to the Fedora EPEL 5 stable repository.