Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1024779 - Review Request: roger-router - Roger router manager for FRITZ!Box and compatible routers
Summary: Review Request: roger-router - Roger router manager for FRITZ!Box and compati...
Keywords:
Status: NEW
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2013-10-30 12:11 UTC by Louis Lagendijk
Modified: 2015-12-20 13:07 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed:


Attachments (Terms of Use)

Description Louis Lagendijk 2013-10-30 12:11:47 UTC
Spec URL: http://fazant.net/roger-router/roger-router.spec-1

SRPM URL: http://fazant.net/roger-router/roger-router-1.8.1-1.fc20.src.rpm

Description: 
Roger Router is a clean solution for controlling the FRITZ!Box
or compatible router with Linux. It offers a rich feature list,
including caller monitor and fax support. It offers integration
with the Evolution and FritzFon address books.
Roger-router is a successor of ffgtk.

Fedora Account System Username: llagendijk

Comment 1 Louis Lagendijk 2013-10-30 12:17:19 UTC
Cups needs access to /var/spool/roger for fax sending. I am planning to raise a bug against selinux to include the default file context before the end of the review.

For now please use
semanage fcontext -a /var/spool/roger -e /var/spool/cups
to se the correct file context.

Comment 2 Volker Fröhlich 2014-01-18 23:23:01 UTC
You can remove the defattrs. They were only necessary up to EL4.

LGPLv2.1 should be LGPLv2+.

"Requires: libroutermanager = %{version}" should have an isa and release macro.

What is the if clause around ldconfig in the libroutermanager scriptlets good for?

The plugins don't require the license files, as they can't be installed on their own. I'd leave out the whole %doc lines there.

I think it's not allowed to add license files yourself. However: incorrect-fsf-address /usr/share/doc/libroutermanager/COPYING.LGPL. It's also wrong in COPYING. Upstream should correct that.

Add -q to the setup macro.

Does roger-router somehow conflict with ffgtk?

I'm usually using _datadir instead of _datarootdir. I don't know whether one should be preferred over the other though.

You could shorten the files sections like this:

%files plugins-fritzfon
%{_datarootdir}/glib-2.0/schemas/org.tabos.roger.plugins.fritzfon.gschema.xml
%{_libdir}/roger/plugins/fritzfon/

%files ...

Comment 3 Louis Lagendijk 2014-02-01 20:21:31 UTC
Used new upstream version 1.8.2
Implemented most suggestion (still need to check the fsf-address and if necessary ping upstream). The COPYING.LGPL is now included upstream.

I have not shortened the files section as I prefer that rpmbuild checks the exact file list.

And, no, roger-router does not conflict with ffgtk. I have both on my system here, and there are no file conflicts.

Spec URL: http://fazant.net/roger-router/roger-router.spec-1.8.2-1
SRPM URL: http://fazant.net/roger-router/roger-router-1.8.2-1.fc20.src.rpm

Comment 4 Volker Fröhlich 2014-02-09 00:25:11 UTC
Just saying, if you name the spec file on your web server like this, you are causing the fedora-review utility some trouble.

Comment 5 Christopher Meng 2014-02-13 07:01:21 UTC
Can you rename the spec to:

http://fazant.net/roger-router/roger-router.spec?


---------------------

1. First question, do we need to follow SUSE's way of handling plugins? I don't agree with the packaging here, one -plugins package is enough.

I strongly discourage packagers looking at other distro's specs since sometimes they are just a waste of time, plus incorrect for Fedora.

If I ask you to write a SPEC from scratch, will you split out the library libroutermanager?

2. Please move %global lines to the top(well actually you can move to almost anywhere, but it's a good habit)

3. Please cleanup spec carefully, LGPLv2.+ is what license?

4. {buildroot}/%{_datadir} --> {buildroot}%{_datadir}, applies to others

5. %{_var} --> %{_localsatedir}

6. find , -exec rm -f {} ';' lines

-->

find , -delete -print

7. %pre
getent group fax > /dev/null || groupadd -f -g 78 -r fax

Are you sure?

8. %dir %{_libdir}/roger/
%dir %{_libdir}/roger/plugins

Fedora doesn't have the problem, just 

%{_libdir}/roger/

Comment 6 Louis Lagendijk 2014-02-13 22:24:53 UTC
Thanks for the feedback Christopher,
Re 1. First question, do we need to follow SUSE's way of handling plugins? I don't agree with the packaging here, one -plugins package is enough.

I moved a number of plugins into the main package. I left the evolution address book as it has dependencies on all the evolution libs, I wanted to avoid havng to pull in all that stuff when not required.
I am fine with pulling all the notification plugins into the main package. The status icon does not work on Gnome 3, so it might be better to keep that separate too and explain the reason why.

I started off from the Suse spec file, but redid it, so the resemblance to the Suse spec is mainly gone anyhow

Re If I ask you to write a SPEC from scratch, will you split out the library libroutermanager?

You mean make libroutermanager its own package and have both libroutermanager and roger-router packages build from the same tar-ball? What is the advantage of that? They are both in the same tar-ball, so keeping it this way is less work in maintaining the packages. I have started a discussion with upstream on properly versioning libroutermanager-libs. That will hopefully come in the near future.

Re 2 - 6 Will check and fix

Re 7: yes: 78 is defined in //usr/share/doc/setup/uidgid as the predefined gid for fax. Is there anything wrong with re-using that gid?

Re 8: will check if this can be easily done. I left this as I did not want to deviate from what upstream uses. If you feel that it is incorrect I can modify the upstream Makefile.am to skip the plugin specific directories. But is it worth changing the upstream defaults?

Comment 7 Christopher Meng 2014-02-14 04:55:16 UTC
(In reply to Louis Lagendijk from comment #6)
> Thanks for the feedback Christopher,
> Re 1. First question, do we need to follow SUSE's way of handling plugins? I
> don't agree with the packaging here, one -plugins package is enough.
> 
> I moved a number of plugins into the main package. I left the evolution
> address book as it has dependencies on all the evolution libs, I wanted to
> avoid havng to pull in all that stuff when not required.
> I am fine with pulling all the notification plugins into the main package.
> The status icon does not work on Gnome 3, so it might be better to keep that
> separate too and explain the reason why.

Yes.

> You mean make libroutermanager its own package and have both
> libroutermanager and roger-router packages build from the same tar-ball?
> What is the advantage of that? They are both in the same tar-ball, so
> keeping it this way is less work in maintaining the packages. I have started
> a discussion with upstream on properly versioning libroutermanager-libs.
> That will hopefully come in the near future.

Ah no, actually I would include the libraries provided in the main package or -libs subpackages, but never split the library out and make it as separate libXXX package since I think this library is useful for this software only. But anything could happen, so I'm not sure on this package.

> Re 7: yes: 78 is defined in //usr/share/doc/setup/uidgid as the predefined
> gid for fax. Is there anything wrong with re-using that gid?

No.

> Re 8: will check if this can be easily done. I left this as I did not want
> to deviate from what upstream uses. If you feel that it is incorrect I can
> modify the upstream Makefile.am to skip the plugin specific directories. But
> is it worth changing the upstream defaults?

No, I mean write one line for files is OK, you can try by yourself. RPM do include the files in subdirectories.(personal habit here in other words)

Comment 8 Louis Lagendijk 2014-02-15 16:36:58 UTC
Updated the spec file with the comments above (I hope I did no miss anything).
Some comments:
The FSF-address in COPYING and COPYING.LPGL is incorrect. I have pinged upstream and this will be corrected in the next upstream release

I have left libroutermanager as a package as upstream is planning to use it for other projects too

New Spec and SRPM:
http://fazant.net/roger-router/roger-router-1.8.3-1/roger-router.spec
http://fazant.net/roger-router/roger-router-1.8.3-1/roger-router-1.8.3-1.fc20.src.rpm


Note You need to log in before you can comment on or make changes to this bug.