Hi there! Steve, I again cc:ed you since I have implemented your last change from #558978, consider this as a simple notification (read below).
On Mon, 27 Sep 2010 23:16:49 +0200, Till Kamppeter wrote: > On 09/27/2010 06:53 PM, Luca Capello wrote: >> >> I have some concerns about the Ubuntu package, here the first of them, I >> will continue on another email as far as the integration progresses. >> >> >> 1) I do not understand why from version 20100210-0ubuntu1 the >> debian/changelog contains the following: [...] > Sorry, the README.Debian did not tell which files exactly were removed > and I did not search through the debian/changelog. So I assumed that > only these *.icm were offending. The new original tarball is now in the SVN repository: http://svn.debian.org/viewsvn/foo2zjs?view=rev&revision=233 >> - remove binary file c5200mono.prn > [...] >> - remove crd/qpdl/CLP*, because copyright is unclear > [...] > So let us use a common source tarball again, with the files mentioned > by you here removed. Please prepare the package, I will merge that > into Ubuntu after the Maverick release. Thank you. >> 2) I do not understand why some patches have been merged, like >> >> * debian/patches/60-getweb.in.dpatch, debian/patches/80-getweb.in.dpatch: >> merged 80-getweb.in.dpatch into 60-getweb.in.dpatch. >> >> They fixes two different things, and they must be separated. >> > > I thought to better have all for getweb in one patch. Feel free to > separate out again this one bashism fix. I will overtake your change > then when I make my first foo2zjs package after the Maverick release. I did leave them separated, so everything was already OK. >> 3) directory should be created through debian/$PKG.dirs and not by hand >> in debian/rules (see /usr/lib/cups/filter/). [...] >> While the best option seems thus to fix it in debian/rules, we should >> use dh_link and not ln. >> > > Please change this appropriately. Already done ;-) >> 4) I am not sure debian/local/ is the right place for non-upstream >> files, but I should admit that this is the first time I heard about >> it and I can not find any documentation about that. Nevermind, I >> have added the two non-upstream PPDs. >> >> BTW, conceptually speaking, Ubuntu debian/rules misses the command to >> compress these two files, given that this action is hidden in the >> 'Add "*cupsFilter" line to accept PDF input data to the PPDs' block. >> > > Please go ahead and correct also this. > I will overtake the version with your corrections to Ubuntu. Given that I still have not understood what the 'Add "*cupsFilter" line...' does, no corrections on this matter have been made yet. Going on with the Ubuntu changes since version 20090908dfsg-1ubuntu1 (Ubuntu lucid): 5) - debian/foo2zjs.postinst: Automatically update the PPD files for existing queues to the versions supplied with this package. - debian/control: Add dependency on cups and cups-client to ensure that automatic update of the PPDs of existing print queues works. I do not understand the purpose of this action, but I should admit that I am not a CUPS expert and I do not know how other drivers behave. However, given that in Debian the PPDs are configuration files (see <http://bugs.debian.org/549673>), I would expect dpkg to prompt for any modification, is it so in this case? 6) - debian/foo2zjs.preinst: handle moving 85-hplj10xx.rules from /etc/udev/rules.d to /lib/udev/rules.d, needed for upgrades from hardy. We do not need this, since version 20090908dfsg-2 we install the udev rules file directly in /lib/udev/rules.d/85-hplj10xx.rules. About the udev rules file, as I explained earlier, I do not like the idea of carrying a separate file instead of patching upstream one: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=558978#36 With the merge of the new upstream and the Ubuntu versions, I have lost some of the fixes submitted in the bug report above, restored: http://svn.debian.org/viewsvn/foo2zjs?view=rev&revision=238 And finally, given that the new upstream version adds support for the HP LaserJet P1505n, I added it to the udev rules file: http://svn.debian.org/viewsvn/foo2zjs?view=rev&revision=239 7) - debian/control, debian/rules: Do not include msexpand, as it is already in the mscompress package. Add a dependency on mscompress instead. https://bugs.launchpad.net/ubuntu/+source/foo2zjs/+bug/92216 First, msexpand is not present in the upstream COPYING file, but since we ship it in the sources, I added it to the debian/copyright: http://svn.debian.org/viewsvn/foo2zjs?view=rev&revision=236 Second, the foo2zjs's msexpand is a Perl script and has nothing to do with the one from the mscompress package. Nevertheless, I tried to understand why msexpand was in the binary package, at least for Debian it was never included, given that its only usage is in the getweb.in script, which actually never calls it: --8<---------------cut here---------------start------------->8--- 232 cpwl|pageworks) 233 if true; then 234 gettgz \ 235 http://foo2zjs.rkkda.com/icm dl2300.tar.gz \ 236 "" 237 else 238 getexe \ 239 ftp://ftp.minolta-qms.com/pub/crc/out_going/windows cpplxp.exe \ 240 "*.IC_" 241 for i in C*.IC_ 242 do 243 base=`basename $i .IC_` 244 mv $base.IC_ $base.ic_ 245 ./msexpand $base.ic_ 246 rm -f $base.ic_ 247 done 248 fi 249 copyright "(c) Copyright Minolta-QMS 1998" 250 ;; --8<---------------cut here---------------end--------------->8--- Please note that the dependency alone is not enough, given that we would need to patch getweb.in as well, since the path is hardcoded. Anyway, I could not find any Ubuntu foo2zjs version shipping msexpand, not even in the first 20051220dfsg-1 version (dapper), the only version available before the bug was reported in LaunchPad. The same is true for Debian, and the reason is simple: msexpand was never shipped by foo2zjs, it is not installed by upstream Makefile (it is simply listed in FILES), thus we do not need to Depends: on mscompress. 8) - debian/control: Require foomatic-filters 4.x Being the foomatic-* upstream developer, you know better than me, but what is the rationale for this? Both Ubuntu since jaunty and Debian since squeeze ship foomatic-filters >= 4, thus everything is already OK. But given that the Debian package does not Depends: on foomatic-filters, I added it: http://svn.debian.org/viewsvn/foo2zjs?view=rev&revision=237 However, I do not understand why the Ubuntu package can be built with any version of foomatic-filters (Build-Depends:) but installed only with foomatic-filters >= 4.0.0~bzr156 (Depends:). If we do need to add a versioned dependency, it should be done in the Build-Depends: as well. 9) - debian/rules: Hide menu entry for the paper-out resetter for the HP LaserJet 1018/1020 and improve UI text for this menu entry. Sorry, I do not agree with this change and anyway on Debian it was anyway implemented elsewhere (see <http://bugs.debian.org/579154>). - debian/control: Drop tix recommends to a suggests. I am fine with this and added a note in the README.Debian, especially if we consider that the end-user must anyway read that document to find the icon: http://svn.debian.org/viewsvn/foo2zjs?view=rev&revision=240 10) - debian/rules: Add /etc/papersize support, and modify all /usr/bin/foo2*-wrapper scripts to handle custom page sizes correctly in the PDF-based printing workflow. - debian/rules: Add "*cupsFilter" line to accept PDF input data to the PPDs. - Support for the PDF printing workflow: o "*cupsFilter:" lines for PDF input in the PPDs o Let wrapper scripts read custom page sizes also from the command line and not only from embedded PostScript commands. I do not understand these modifications, would you mind explaining them, please? I do not feel comfortable in applying something I do not understand, sorry... 11) - debian/local/apport-hook.py, debian/rules: Add apport hook. Useless, Debian does not ship apport. Finally, while reviewing the package I found out that upstream includes cups.h (from the CUPS Imaging library) without mentioning it in the upstream COPYING file. This file is included only in foo2hp.c, while command2foo2lava-pjl.c use the system-wide header file. Two comments: - given that this file is DFSG-free, I included it in the debian/copyright: http://svn.debian.org/viewsvn/foo2zjs?view=rev&revision=234 - the best thing would be to patch upstream to use the system-wide header file, but a quick check showed that the system-wide header file misses a lot of identifiers used in foo2hp.c, thus I do not think it is worth the effort. However, maybe it would be better to file a separate wishlist bug, to be aware of this issue. As I wrote in my first reply, my HP Color LaserJet 1500L seems to be broken, thus I can not test the new package version, thus i386 and amd64 packages to be tested are available at: http://people.debian.org/~gismo/tmp/ Thx, bye, Gismo / Luca
pgpxpmnZ2mkAg.pgp
Description: PGP signature
