Bug 1659709 - Review Request: dymo-cups-drivers - DYMO LabelWriter Drivers for CUPS
Summary: Review Request: dymo-cups-drivers - DYMO LabelWriter Drivers for CUPS
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nikola Forró
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-12-15 15:07 UTC by Andrew Bauer
Modified: 2019-10-22 12:10 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-01-19 01:54:41 UTC
nforro: fedora-review+


Attachments (Terms of Use)

Description Andrew Bauer 2018-12-15 15:07:29 UTC
Fedora Account System Username: kni

Spec URL: https://raw.githubusercontent.com/knight-of-ni/specfiles/master/dymo-cups-drivers.spec

SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/4214/31474214/dymo-cups-drivers-1.4.0.5-3.fc30.src.rpm

Description
------------
DYMO LabelWriter and DYMO LabelMANAGER series drivers for CUPS

RPMLint Output
--------------
$ rpmlint /var/lib/mock/fedora-29-x86_64/result/*.rpm
4 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 1 Andrew Bauer 2018-12-15 17:27:27 UTC
There is an unofficial github repo here: 
https://github.com/matthiasbock/dymo-cups-drivers

I have taken a few patches from that repo, to fix a bug and a couple warnings.
See the specfile for details.

Updated SPEC URL: https://raw.githubusercontent.com/knight-of-ni/specfiles/master/dymo-cups-drivers.spec
Updated SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/6932/31476932/dymo-cups-drivers-1.4.0.5-3.fc30.src.rpm

Comment 2 Nikola Forró 2019-01-07 11:18:13 UTC
The SRPM is no longer available.

Where does that ".5" in the version come from? AFAICS, the latest upstream version is just "1.4.0".

Couldn't you simply use "autoreconf --force --install" in %build?

Comment 3 Andrew Bauer 2019-01-07 16:41:41 UTC
Wow, I had no idea fedora removed scratch builds so quickly.
Unfortunately, I am unable to access all the patch files needed from my dayjob, so I'll post a new source rpm this evening when I get home.

The official version of the tarball is 1.4.0.5 as indicated by VERSION=1.4.0.5 in configure.ac. However, the download filename is just dymo-cups-drivers-1.4.0.tar.gz. Creating a macro called "short_version" allows me to manage the rpm package version (and download url) in just one place in the specfile, rather than two.

I use cmake so often these days my autotools has gone rusty. There might very well be a more efficient way to do things in %build. 
Can you be more specific as to what you are suggesting?

Replace all these commands: 
%{__libtoolize}
%{__aclocal}
%{__autoheader}
%{__automake} --force-missing --add-missing
%{__autoconf}

with just this?

autoreconf --force --install

Or did you mean something different?

Comment 4 Nikola Forró 2019-01-07 16:59:59 UTC
(In reply to Andrew Bauer from comment #3)
> Wow, I had no idea fedora removed scratch builds so quickly.

Yes, scratch builds only last for a week or so. You can try Copr instead.

> The official version of the tarball is 1.4.0.5 as indicated by
> VERSION=1.4.0.5 in configure.ac. However, the download filename is just
> dymo-cups-drivers-1.4.0.tar.gz. Creating a macro called "short_version"
> allows me to manage the rpm package version (and download url) in just one
> place in the specfile, rather than two.

Ok, that makes sense. But for example Debian and Ubuntu use 1.4.0.

> Replace all these commands: 
> %{__libtoolize}
> %{__aclocal}
> %{__autoheader}
> %{__automake} --force-missing --add-missing
> %{__autoconf}
> 
> with just this?
> 
> autoreconf --force --install
> 
> Or did you mean something different?

Exactly that.

Comment 5 Andrew Bauer 2019-01-08 00:36:29 UTC
I updated the specfile to use autoreconf. Strange, there was no rpm macro for it.

Update Specfile:
https://raw.githubusercontent.com/knight-of-ni/specfiles/master/dymo-cups-drivers.spec

Updated source rpm:
https://copr-be.cloud.fedoraproject.org/results/kni/DymoCupsDrivers/fedora-rawhide-x86_64/00842666-dymo-cups-drivers/dymo-cups-drivers-1.4.0.5-3.fc30.src.rpm

I just noticed several autotools related warnings in the build output (they were there previously. I only now noticed the warnings):

>src/lm/Makefile.am:22: warning: 'INCLUDES' is the old name for 'AM_CPPFLAGS' (or '*_CPPFLAGS')
>src/lm/Makefile.am:26: warning: source file '../common/CupsPrintEnvironment.cpp' is in a subdirectory,
>src/lm/Makefile.am:26: but option 'subdir-objects' is disabled
>src/lm/Makefile.am:26: warning: source file '../common/Halftoning.cpp' is in a subdirectory,
>src/lm/Makefile.am:26: but option 'subdir-objects' is disabled
>src/lm/Makefile.am:26: warning: source file '../common/ErrorDiffusionHalftoning.cpp' is in a subdirectory,
>src/lm/Makefile.am:26: but option 'subdir-objects' is disabled
>src/lm/Makefile.am:26: warning: source file '../common/NonLinearLaplacianHalftoning.cpp' is in a subdirectory,
>src/lm/Makefile.am:26: but option 'subdir-objects' is disabled

If you think these are serious, I can certainly look into these. I can attest, however, that this package works just fine as it is currently. I've been using it on a Raspberry Pi with a Dymo 450 label printer attached for several months now.

Comment 6 Nikola Forró 2019-01-08 14:15:46 UTC
I don't think those warnings are serious, but it would be nice to fix them. It's not a review blocker though.

There are two license files, LICENSE and COPYING, that are identical except for whitespace differences. I don't think it's necessary to package both of them.

/usr/lib/cups/filter, /usr/share/cups and /usr/share/cups/model directories are owned by cups-filesystem, so this package shouldn't own them.
This should do the trick:

%files
...
%{_cups_serverbin}/filter/*
%{_datadir}/cups/model/*

Otherwise I believe the package is fine.

Comment 7 Andrew Bauer 2019-01-08 19:06:55 UTC
(In reply to Nikola Forró from comment #6)
> I don't think those warnings are serious, but it would be nice to fix them.
> It's not a review blocker though.
> 
Done.
My anal retentive side took over and I fixed all the warnings that were appearing in the build output.
PR sent to unofficial upstream repository: https://github.com/matthiasbock/dymo-cups-drivers/pull/7

> There are two license files, LICENSE and COPYING, that are identical except
> for whitespace differences. I don't think it's necessary to package both of
> them.
Done.

> 
> /usr/lib/cups/filter, /usr/share/cups and /usr/share/cups/model directories
> are owned by cups-filesystem, so this package shouldn't own them.
> This should do the trick:
> 
> %files
> ...
> %{_cups_serverbin}/filter/*
> %{_datadir}/cups/model/*
> 
Done.

Updated Spec URL:
https://raw.githubusercontent.com/knight-of-ni/specfiles/master/dymo-cups-drivers.spec

Updated Source RPM:
https://copr-be.cloud.fedoraproject.org/results/kni/DymoCupsDrivers/fedora-rawhide-x86_64/00843016-dymo-cups-drivers/dymo-cups-drivers-1.4.0.5-4.fc30.src.rpm

Comment 8 Nikola Forró 2019-01-08 19:25:20 UTC
> %{_datadir}/cups/*

This still makes the package own /usr/share/cups/model directory.

Comment 9 Andrew Bauer 2019-01-08 19:39:32 UTC
Oh, right. I did not notice the specfile was missing "model" in the path name.
This has been fixed.

Updated Spec URL:
https://raw.githubusercontent.com/knight-of-ni/specfiles/master/dymo-cups-drivers.spec

Updated Source RPM:
https://copr-be.cloud.fedoraproject.org/results/kni/DymoCupsDrivers/fedora-rawhide-x86_64/00843023-dymo-cups-drivers/dymo-cups-drivers-1.4.0.5-4.fc30.src.rpm

Comment 10 Nikola Forró 2019-01-08 19:50:25 UTC
Package approved.

Comment 11 Gwyn Ciesla 2019-01-08 20:31:32 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/dymo-cups-drivers

Comment 12 Fedora Update System 2019-01-08 20:53:52 UTC
dymo-cups-drivers-1.4.0.5-4.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-193d5d724b

Comment 13 Fedora Update System 2019-01-08 21:08:28 UTC
dymo-cups-drivers-1.4.0.5-4.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2019-debbd9d9a2

Comment 14 Fedora Update System 2019-01-08 21:14:28 UTC
dymo-cups-drivers-1.4.0.5-4.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-389348b102

Comment 15 Fedora Update System 2019-01-10 19:12:44 UTC
dymo-cups-drivers-1.4.0.5-4.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-389348b102

Comment 16 Fedora Update System 2019-01-10 20:49:25 UTC
dymo-cups-drivers-1.4.0.5-4.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-debbd9d9a2

Comment 17 Fedora Update System 2019-01-10 22:15:02 UTC
dymo-cups-drivers-1.4.0.5-4.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-193d5d724b

Comment 18 Zdenek Dohnal 2019-01-11 07:42:28 UTC
Hi Andrew!

I'm Zdenek, maintainer of CUPS and several printer driver packages. I saw your review request on devel list and thank you for packaging the project! I checked the spec file and the project briefly and I would like to ask you make two changes:

1) I checked several source files and they mention the source file can be distributed with 'GPL version 2 or any later', so the project should have GPLv2+ license

2) It is not defined in any FPG (since nobody wrote the chapter about it yet, although it is used for long time...), but printer driver packages should have:

BuildRequires: python3-cups

in the spec file (see other printer driver packages, f.e. gutenprint, hplip, foomatic-db). The part of python3-cups package, which printer drivers need, is rpm postscript driver tag generator, which creates postscriptdriver tags based on 1284DeviceID attribute in ppd files. These tags is used for automatic printer driver installation.

Comment 19 Fedora Update System 2019-01-19 01:54:41 UTC
dymo-cups-drivers-1.4.0.5-4.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.

Comment 20 Fedora Update System 2019-01-19 02:25:48 UTC
dymo-cups-drivers-1.4.0.5-4.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.

Comment 21 Fedora Update System 2019-01-26 00:48:58 UTC
dymo-cups-drivers-1.4.0.5-4.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.


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