Bug 550582 - Review Request: dpkg - Package maintenance system for Debian Linux
Summary: Review Request: dpkg - Package maintenance system for Debian Linux
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Orcan Ogetbil
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 457210 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-12-26 06:15 UTC by Andrew Colin Kissa
Modified: 2011-02-28 23:55 UTC (History)
8 users (show)

Fixed In Version: dpkg-1.15.5.6-6.el5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-03-09 03:25:32 UTC
Type: ---
Embargoed:
oget.fedora: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Andrew Colin Kissa 2009-12-26 06:15:39 UTC
Spec URL: http://www.topdog-software.com/oss/SRPMS/fedora/dpkg/dpkg.spec
SRPM URL: http://www.topdog-software.com/oss/SRPMS/fedora/dpkg/dpkg-1.15.4.1-1.fc11.src.rpm
Description: 
This package contains the tools (including dpkg-source) required 
to unpack, build and upload Debian source packages.

This package also contains the programs dpkg which used to handle the 
installation and removal of packages on a Debian system.

This package also contains dselect, an interface for managing the 
installation and removal of packages on the system.

Comment 1 Andrew Colin Kissa 2009-12-26 06:17:23 UTC
*** Bug 457210 has been marked as a duplicate of this bug. ***

Comment 2 Thomas Spura 2009-12-27 04:49:07 UTC
Just a few comments:
- https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define
- Could you reduce the %files a bit?
  e.g.: %{_bindir}/dpkg-*
        %{_mandir}/*/man*/dpkg-*
  and so on.
  It will be easier to follow, and maybe the compression format of the man pages
  changes and you need to rename all *.gz to something else.
- There is a new version upstream.
  How do you create the patch?
  (Maybe sed would be enought. Most of the changes are only in the *.po file, which will change quite often...)

Comment 3 leigh scott 2009-12-27 10:59:17 UTC
(In reply to comment #2)
> Just a few comments:
> -
> https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define
> - Could you reduce the %files a bit?
>   e.g.: %{_bindir}/dpkg-*
>         %{_mandir}/*/man*/dpkg-*
>   and so on.
>   It will be easier to follow, and maybe the compression format of the man
> pages
>   changes and you need to rename all *.gz to something else.
> - There is a new version upstream.
>   How do you create the patch?
>   (Maybe sed would be enought. Most of the changes are only in the *.po file,
> which will change quite often...)  


You are wrong, doing this will mess the package up.

Comment 4 leigh scott 2009-12-27 11:02:45 UTC
(In reply to comment #2)
> Just a few comments:
> -
> https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define
> - Could you reduce the %files a bit?
>   e.g.: %{_bindir}/dpkg-*
>         %{_mandir}/*/man*/dpkg-*
>   and so on.
  


The dpkg package is total scrambled if you do this

Comment 5 Thomas Spura 2009-12-27 15:26:18 UTC
(In reply to comment #4)
> (In reply to comment #2)
> > Just a few comments:
> > -
> > https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define
> > - Could you reduce the %files a bit?
> >   e.g.: %{_bindir}/dpkg-*
> >         %{_mandir}/*/man*/dpkg-*
> >   and so on.
> 
> 
> 
> The dpkg package is total scrambled if you do this  

Right, it was a bit late, when I wrote that ;)

Deleting the .gz at the end of the manuals would be great anyway...

Comment 6 Ville Skyttä 2010-01-03 10:18:20 UTC
Installing a /usr/sbin/install-info executable seems dangerous as we already have /sbin/install-info.  If this executable is required, I suggest installing it somewhere outside of the default $PATH, for example %{_libdir}/dpkg.

Development related packages are named *-devel in Fedora instead of *-dev.

Comment 7 leigh scott 2010-01-03 11:47:34 UTC
(In reply to comment #6)

> Development related packages are named *-devel in Fedora instead of *-dev.  


dpkg-dev doesn't look like a -devel package as it doesn't have any headers or development libs.

https://bugzilla.redhat.com/show_bug.cgi?id=550582

Comment 8 leigh scott 2010-01-03 11:48:18 UTC
(In reply to comment #7)
> (In reply to comment #6)
> 
> > Development related packages are named *-devel in Fedora instead of *-dev.  
> 
> 
> dpkg-dev doesn't look like a -devel package as it doesn't have any headers or
> development libs.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=550582  


wrong link

http://packages.debian.org/sid/all/dpkg-dev/filelist

Comment 9 Ralf Corsepius 2010-01-03 13:32:33 UTC
Debian's dpkg-dev is a *-devel package in Fedora's terms.

The fact, a package contains headers or development libs is just one crition amongst many qualifying a package as a *-devel package, but not the only one.

Comment 10 Orcan Ogetbil 2010-01-22 22:49:44 UTC
Here is the full review for this:

? rpmlint says:
   dselect.x86_64: W: only-non-binary-in-usr-lib
Do those files have to be in libdir? Please change or defend.

* The patch makes references to /usr/lib which is not the right path for x86_64. Why does sed break stuff again?

* dev package must be renamed to devel

* Please don't duplicate the same doc files in the devel and dselect packages

* Please go through the source and make sure you tag all licenses correctly. As far as I can tell most of the source code is GPLv2+. However there are some exceptions, such as 
   lib/dpkg/md5.c is public domain
   lib/dpkg/showpkg.c is GPLv2
   etc
These have to be listed as comments in the SPEC file.

* Source0 seems to be wrong. Gives 404 to me.

* Please update to the latest upstream

? Shouldn't the dselect package require the main package?

? Why are we removing the alternatives stuff?

* A few directories are owned by both the main package and the devel package. Since the devel package requires the main package, please get rid of the multiple ownership. Also one directory is also owned by the dselect package. If dselect package does not need to require the main package, this is OK.

* /usr/sbin/install-info seems dangerous. Please move somewhere else.

- At a few places in the build I see the -O1 flag overriding our default flag. This -O1 is passed during the linking phase only. I don't think this is a problem. It is weird though.

* I can't install the devel package:
  $ sudo rpm -ivh dpkg-dev-1.15.4.1-1.fc12.x86_64.rpm
  error: Failed dependencies:
        perl(extra) is needed by dpkg-dev-1.15.4.1-1.fc12.x86_64
  $ sudo yum install "perl(extra)"
  ...
  No package perl(extra) available.

Comment 11 Andrew Colin Kissa 2010-01-24 07:25:47 UTC
Thanks Oget,

I will make the requested changes sometime this week.

Comment 12 Andrew Colin Kissa 2010-01-24 07:29:18 UTC
Thanks Oget,

I will make the requested changes sometime this week.

Comment 13 Matt Taggart 2010-01-27 11:00:00 UTC
FYI: In order to support running on Fedora/RHEL/etc the FOSSology project ( http://fossology.org ) is going to need this package for our next release since we are adding support for unpacking and analyzing rpm and deb source packages (and use 'dpkg-source -x' for the latter).

I'm glad you are working on this. Hopefully once it's in fedora it can go into EPEL too.

Thanks,

--
Matt Taggart
taggart

Comment 14 Matt Taggart 2010-01-28 21:40:34 UTC
I was talking to one of the dpkg developers on IRC. He doesn't have a bugzilla account and so asked that I forward these comments on.

<guillem> re dpkg in fedora I don't have an account on their bugzilla but you might wanna comment to maybe speed up the review
<guillem> install-info is not really needed at all if they are using the one from GNU texinfo
<guillem> update-alternatives I thought they have their own implentation (but I might be wrong)
<guillem> I see in their spec file they are working around a problem now fixed in the latest released version about selinux linking
<guillem> the LDFLAGS='-lselinux' should be unnecessary
<guillem> I don't know how they requires stuff gets generated but I suspect the perl requires involving dpkg-gettext.pl and controllib.pl is not needed anymore
<guillem> re licensing I did an audit that might be useful to them: http://lists.debian.org/debian-dpkg/2009/11/msg00043.html
<guillem> cpio should not be needed by the devel package
<guillem> I've not checked the patch they are applying but I'd gladly fix anything that causes them to need to patch it
<guillem> there's also been reports on the list about it building fine with recentish git on fedora
<guillem> I think that's mostly it
<guillem> and thanks for fwding the info :)
<guillem> oh and re the linker optimization they can use --disable-linker-optimisations
<guillem> and dselect needs dpkg yes
<guillem> anyway dinner time for me, poke if you need more info!

Comment 15 Andrew Colin Kissa 2010-02-13 16:08:13 UTC
The requested changes have been implemented, comments are inline justifying the various issues.

http://www.topdog-software.com/oss/SRPMS/fedora/dpkg/dpkg.spec
http://www.topdog-software.com/oss/SRPMS/fedora/dpkg/dpkg-1.15.5.6-1.fc12.src.rpm

Comment 16 Orcan Ogetbil 2010-02-13 18:47:04 UTC
Thanks, we are almost there. I have a few questions, comments and a few minor issues:


? The only rpmlint is:
    dpkg.src:1: E: hardcoded-library-path in /usr/lib
which is ironically a lot better than our own rpm package. Anyway, rpmlint is an rpmlint. I fail to understand that why we hardcode the libdir to /usr/lib. I read your comment, but is it not possible to patch those perl scripts to look at /usr/share instead of /usr/lib? 

On the other hand our rpm package also puts files in /usr/lib. I wonder if this is really a must.

! The devel package looks like it can be made noarch. I don't know if this would screw up our build system though. (Meanwhile dpkg and dselect packages cannot be noarch)

* The  following directories and possibly their subdirectories are not owned:
   /usr/lib/perl5/vendor_perl/5.10.0/Debian/
   %dpkgdir/dpkg/methods/

* Build process log does not show us what flags are used during compilation. Please make it verbose.

Comment 17 Orcan Ogetbil 2010-02-13 18:48:52 UTC
koji seems to be happy with this package too:
   http://koji.fedoraproject.org/koji/taskinfo?taskID=1983856

Comment 18 Andrew Colin Kissa 2010-02-13 18:57:17 UTC
Actually i put the files in /usr/lib based on our own rpm package, let me see if i can patch it to move the files into /usr/share.

Will update and post.

Comment 19 Andrew Colin Kissa 2010-02-14 09:06:05 UTC
* Fixed the lib path, files now installed under /usr/share/dpkg

* devel package now noarch

* Directory ownership fixed

* Please provide some pointers on how i can make the build more verbose.

http://www.topdog-software.com/oss/SRPMS/fedora/dpkg/dpkg-1.15.5.6-2.fc12.src.rpm
http://www.topdog-software.com/oss/SRPMS/fedora/dpkg/dpkg.spec

Comment 20 Orcan Ogetbil 2010-02-14 20:05:18 UTC
Thanks.

(In reply to comment #19)
> 
> * Directory ownership fixed

Still there are issues. Please install all the generated packages. Then uninstall them. You will be left with these empty directories
   /usr/lib/perl5/vendor_perl/5.10.0/Dpkg/
   /usr/lib/perl5/vendor_perl/5.10.0/Debian/
   /usr/share/dpkg/
That is because their subdirectories are not owned. And rpm does not remove non-empty directories during uninstallation.

An example of a way to fix this is to change the lines
   %dir %{perl_vendorlib}/Debian
   %{perl_vendorlib}/Debian/Dselect/*.pm
to
   %{perl_vendorlib}/Debian/
then you won't have this problem. Similarly for the other directories. Beware though, %{_datadir}/dpkg/ is used by both the main package and the dselect package so it needs to be handled carefully.

Also, you have these two lines under %files:
   %dir %{_datarootdir}/dpkg
   %dir %{_datadir}/dpkg
As far as I know, %{_datarootdir} and %{_datadir} both point to /usr/share. There is no need to list the same directory twice in the %files listings. I prefer using either one notation or the other.

> 
> * Please provide some pointers on how i can make the build more verbose.
> 

Just pass --disable-silent-rules to configure.

! One last thing:
Changing
   install -m0644
to
   install -pm0644
will preserve timestamps. No big deal though.

Comment 22 Orcan Ogetbil 2010-02-16 04:03:16 UTC
... and we are done!

---------------------------------------
This package (dpkg) is APPROVED by oget
---------------------------------------

Thank you Andrew for packaging this.

Comment 23 Andrew Colin Kissa 2010-02-16 06:22:50 UTC
Thanks oget for the great review.

New Package CVS Request
=======================
Package Name: dpkg
Short Description: Package maintenance system for Debian Linux
Owners: topdog
Branches: F-11 F-12 EL-5

Comment 24 Andrew Colin Kissa 2010-02-17 13:21:53 UTC
Since branching has already taken place. requesting fedora 13 as well.

New Package CVS Request
=======================
Package Name: dpkg
Short Description: Package maintenance system for Debian Linux
Owners: topdog
Branches: F-11 F-12 F-13 EL-5

Comment 25 Jason Tibbitts 2010-02-19 19:07:12 UTC
CVS done (by process-cvs-requests.py).

Comment 26 Fedora Update System 2010-02-20 08:36:57 UTC
dpkg-1.15.5.6-3.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/dpkg-1.15.5.6-3.fc12

Comment 27 Fedora Update System 2010-02-20 08:37:03 UTC
dpkg-1.15.5.6-3.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/dpkg-1.15.5.6-3.fc11

Comment 28 Fedora Update System 2010-02-20 08:37:08 UTC
dpkg-1.15.5.6-3.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/dpkg-1.15.5.6-3.fc13

Comment 29 Fedora Update System 2010-02-20 17:31:11 UTC
dpkg-1.15.5.6-3.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update dpkg'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F13/FEDORA-2010-1906

Comment 30 Fedora Update System 2010-02-20 22:51:03 UTC
dpkg-1.15.5.6-3.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update dpkg'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F12/FEDORA-2010-1740

Comment 31 Fedora Update System 2010-02-23 05:31:10 UTC
dpkg-1.15.5.6-3.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update dpkg'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2010-2580

Comment 32 Fedora Update System 2010-03-09 03:25:25 UTC
dpkg-1.15.5.6-3.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 33 Fedora Update System 2010-03-09 03:33:30 UTC
dpkg-1.15.5.6-3.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 34 Fedora Update System 2010-03-09 03:37:42 UTC
dpkg-1.15.5.6-3.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 35 Fedora Update System 2011-02-10 18:52:07 UTC
dpkg-1.15.5.6-6.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/dpkg-1.15.5.6-6.el5

Comment 36 Fedora Update System 2011-02-28 23:55:49 UTC
dpkg-1.15.5.6-6.el5 has been pushed to the Fedora EPEL 5 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.