Bug 969718 - Review Request: pbuilder - Personal package builder for Debian packages
Summary: Review Request: pbuilder - Personal package builder for Debian packages
Status: CLOSED DUPLICATE of bug 1018926
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Sergio Monteiro Basto
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
: 591388 (view as bug list)
Depends On: 591192 591389 961141 1010000
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-06-02 01:45 UTC by Oron Peled
Modified: 2013-10-14 17:37 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-10-14 17:37:39 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
sergio: fedora-review?


Attachments (Terms of Use)
pbuilder.spec.patch (3.68 KB, patch)
2013-10-14 01:41 UTC, Sergio Monteiro Basto
no flags Details | Diff

Description Oron Peled 2013-06-02 01:45:27 UTC
Spec URL: http://oron.fedorapeople.org/deb-package/pbuilder.spec
SRPM URL: http://oron.fedorapeople.org/deb-package/pbuilder-0.213-1.fc20.src.rpm
Description: pbuilder constructs a chroot system, and builds a package inside the
chroot. It is an ideal system to use to check that a package has
correct build-dependencies. It uses apt extensively, and a local
mirror, or a fast connection to a Debian mirror is ideal, but not
necessary.

"pbuilder create" uses debootstrap to create a chroot image.

"pbuilder update" updates the image to the current state of testing/unstable/whatever

"pbuilder build" takes a *.dsc file and builds a binary in the chroot image

Fedora Account System Username: oron

Comment 1 Oron Peled 2013-06-02 01:52:59 UTC
This replace the old bug #591388 since the reporter has no time to finish it.

What has been done:
 * All issues in the original BR were fixed
 * Added 'make check'
 * Added man pages
 * The source package now create two binary packages (just like the upstream package):
   - pbuilder
   - pbuilder-uml (run pbuilder inside a user-mode-linux)

Open issues:
 * pbuilder-uml is not funcional yet -- should we remove it temporarily?
 * To actually create chroots for pbuilder, we need debian-archive-keyring:
   - This is actually needed for any secure debootstrap and not just pbuilder.
   - IMO we should package it in Fedora
   - Until then, people may grab it manually from Debian.

Comment 2 Oron Peled 2013-06-02 01:54:32 UTC
Forgot to mention: all previous dependencies are in rawhide+f19:
 - dh-make
 - po-debconf
 - debhelper (which depends on dpkg)

Comment 3 Oron Peled 2013-06-02 01:56:13 UTC
*** Bug 591388 has been marked as a duplicate of this bug. ***

Comment 4 Sergio Monteiro Basto 2013-06-08 20:27:42 UTC
fedora-review -b 969718

relevant warnings: 
1 - pbuilder-uml.noarch: W: spelling-error %description -l en_US linux -> Linux

could be fix 

2 - pbuilder.noarch: W: only-non-binary-in-usr-lib

/usr/lib/pbuilder shouldn't be 
/usr/libexec/pbuilder ?

like other package that I review with you 
%{_libdir} is /usr/lib in i386 and /usr/lib64 in x86_64 .


3 - pbuilder-uml

#Requires:  rootstrap (but missing in Fedora) 
we need add rootstrap to Fedora ? 

#Requires:  user-mode-linux
we need add rootstrap to Fedora ?

4 - Missing Requires: sudo ?

Comment 5 Sergio Monteiro Basto 2013-06-08 23:43:33 UTC
5 - usr/bin/debuild-pbuilder requires usr/bin/debuild from Debian's devscripts package.
I don't find any /usr/bin/debuild
seems is part of Debian devscripts
Bug #920163 could have some interest, seems to me that we also need packaging devscripts .

Comment 7 Sergio Monteiro Basto 2013-06-22 21:47:57 UTC
Mario, do you want help on review ?

Comment 8 Oron Peled 2013-07-08 20:32:31 UTC
(in reply to comment #4)
1. The lower-case "linux" in the %description is part of
   the package name "user-mode-linux".
   It's probably better to leave as is instead of "user-mode-Linux"

2. The "/usr/lib/pbuilder":
   * Since it only contains scripts, it should have been in
     /usr/share/pbuilder (but that is already used by the package
     for other stuff)
   * Than, your suggestion of "/usr/libexec/pbuilder" is the
     most logical one. However....
   * This path is embedded in 67 lines over 35 files:
             $ grep -r '/usr/lib/pbuilder' . | wc -l
             67
             $ grep -rl '/usr/lib/pbuilder' . | wc -l
             35
   * So I was thinking it would be better to leave it as is.
     What do you think? Should we ask for some "exception"?

3. pbuilder-uml:
   * The "user-mode-linux" is actually a special kernel variant
     which Fedora kernel team does not build.
   * So I was thinking about "commenting out" this sub-package
     for now.
   * I.e: get pbuilder without pbuilder-uml.

4. missing sudo -- indeed, fixed my spec (didn't upload yet)

Comment 9 Oron Peled 2013-07-08 20:36:26 UTC
(in reply to comment #5)

* Yes, devscripts is also valuable in its own right.

* I'm willing to work on it and maintain it.
  Would you have time to review it?

* In light of Bug #920163, I was thinking to have the
  devscripts source package create a "devscripts-generic"
  subpackage with all the "non-Debian-specific" scripts.
  Obviously, "devscripts" would Require devscripts-generic.

Comment 10 Sergio Monteiro Basto 2013-07-08 21:03:36 UTC
(In reply to Oron Peled from comment #8)
Generally approved , I have to see better point 2 ...

(In reply to Oron Peled from comment #9)
> (in reply to comment #5)
> 
> * Yes, devscripts is also valuable in its own right.
> 
> * I'm willing to work on it and maintain it.
>   Would you have time to review it?

yes, I hope so 

> * In light of Bug #920163, I was thinking to have the
>   devscripts source package create a "devscripts-generic"
>   subpackage with all the "non-Debian-specific" scripts.
>   Obviously, "devscripts" would Require devscripts-generic.

Lets build devscripts , let me know what is the bug number.
I don't see any need of split in sub-packages, but don't care about it. I'm more concerned how we resolve Bug #920163, maybe because rpmdevtools have some scripts of devscripts, makes sense have a sub-package for what rpmdevtools have ... I don't know just an idea.

Comment 11 Christopher Meng 2013-09-05 13:44:46 UTC
Status?

Comment 12 Sergio Monteiro Basto 2013-09-05 15:12:46 UTC
Lets build devscripts

Comment 13 Sandro Mani 2013-09-19 16:46:19 UTC
I've put devscripts up for review, see [1]. Also up for review are debian-keyring [2] and ubuntu-keyring [3].


[1] https://bugzilla.redhat.com/show_bug.cgi?id=1010000
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1009997
[3] https://bugzilla.redhat.com/show_bug.cgi?id=1009998

Comment 14 Sandro Mani 2013-09-19 17:29:33 UTC
By the way, I'm (successfully) using this pbuilder package currently:

SPEC: http://smani.fedorapeople.org/review/pbuilder.spec
SRPM: http://smani.fedorapeople.org/review/pbuilder-0.215-1.fc21.src.rpm

Comment 15 Sergio Monteiro Basto 2013-09-19 18:52:16 UTC
hi ,
I try look at this on weekend , 
could you post the diff of last spec and your spec ? 
should we add to this bug report Depends On 1010000, 1009997 and 1009998 ? 

I see that packages depends on more 2 packages 1009999 and 1009998 ?

Comment 16 Sandro Mani 2013-09-19 19:08:33 UTC
Depends on 101000 added. 1009997 and 1009998 are a plus, but not a strict dependency for this package.

TBH I wrote my spec before noticing that there was already one up for review, so the diff is rather large and not very useful IMO (I've uploaded it here [1]). But basically most is the same, except that I did not include a uml subpackage (since it currently cannot work), and the spec is somewhat more conforming to the guidelines (i.e. no clean section, no defattr, preserve timestamps, etc).

And yes, 109996 is necessary to build debian-keyring, and 1009999 is necessary for devscripts.

[1] http://smani.fedorapeople.org/review/pbuilder.spec.diff

Comment 17 Sandro Mani 2013-09-20 11:26:53 UTC
Some updates:

SPEC: http://smani.fedorapeople.org/review/pbuilder.spec
SRPM: http://smani.fedorapeople.org/review/pbuilder-0.215-2.fc21.src.rpm

* Fri Sep 19 2013 Sandro Mani <manisandro@gmail.com> - 0.215-2
- Create build and ccache directories in /var/cache/pbuilder
- Don't use hardcoded user id
- Prepare pbuilderrc for ccache usage

Comment 18 Sergio Monteiro Basto 2013-10-08 15:31:51 UTC
ok devscripts is in F20 testing, all dependencies are solved , what review should I follow ?

Comment 19 Sandro Mani 2013-10-08 17:10:42 UTC
I don't know whether oron is still pursuing this, but I'd be happy to maintain pbuilder.


One more update:

SPEC: http://smani.fedorapeople.org/review/pbuilder.spec
SRPM: http://smani.fedorapeople.org/review/pbuilder-0.215-3.fc21.src.rpm

* Tue Oct 08 2013 Sandro Mani <manisandro@gmail.com> - 0.215-3
- Don't test non-existing ubuntu arm mirrors

Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6036996

Comment 20 Sergio Monteiro Basto 2013-10-08 19:06:40 UTC
Sandro thanks for your effort , I have to have time , so just in next weekend I will look into it , meanwhile we see if Oron reply to us .
Thanks.

Comment 21 Sandro Mani 2013-10-10 13:14:22 UTC
SPEC: http://smani.fedorapeople.org/review/pbuilder.spec
SRPM: http://smani.fedorapeople.org/review/pbuilder-0.215-4.fc21.src.rpm

* Thu Oct 10 2013 Sandro Mani <manisandro@gmail.com> - 0.215-4
- Improve README.fedora
- Add some missing requires

Comment 22 Sergio Monteiro Basto 2013-10-14 00:41:13 UTC
(In reply to Sandro Mani from comment #21)
> SPEC: http://smani.fedorapeople.org/review/pbuilder.spec
> SRPM: http://smani.fedorapeople.org/review/pbuilder-0.215-4.fc21.src.rpm
> 
> * Thu Oct 10 2013 Sandro Mani <manisandro@gmail.com> - 0.215-4
> - Improve README.fedora
> - Add some missing requires


ERROR: 'mock build failed, see /home/sergio/rpmbuild/969718-pbuilder/results/build.log'

error: Installed (but unpackaged) file(s) found:
   /usr/share/doc/pbuilder/examples/B90lintian
   /usr/share/doc/pbuilder/examples/B90list-missing
   /usr/share/doc/pbuilder/examples/B91debc
   /usr/share/doc/pbuilder/examples/B91dpkg-i
   /usr/share/doc/pbuilder/examples/B92test-pkg
   /usr/share/doc/pbuilder/examples/C10shell
   /usr/share/doc/pbuilder/examples/C11screen
   /usr/share/doc/pbuilder/examples/D10tmp
   /usr/share/doc/pbuilder/examples/D20addnonfree
   /usr/share/doc/pbuilder/examples/D65various-compiler-support
   /usr/share/doc/pbuilder/examples/D80no-man-db-rebuild
   /usr/share/doc/pbuilder/examples/D90chrootmemo
   /usr/share/doc/pbuilder/examples/F90chrootmemo
   /usr/share/doc/pbuilder/examples/execute_installtest.sh
   /usr/share/doc/pbuilder/examples/execute_paramtest.sh
   /usr/share/doc/pbuilder/examples/lvmpbuilder/README
   /usr/share/doc/pbuilder/examples/lvmpbuilder/STRATEGY
   /usr/share/doc/pbuilder/examples/lvmpbuilder/lib/lvmbuilder-checkparams
   /usr/share/doc/pbuilder/examples/lvmpbuilder/lib/lvmbuilder-modules
   /usr/share/doc/pbuilder/examples/lvmpbuilder/lib/lvmbuilder-unimplemented
   /usr/share/doc/pbuilder/examples/lvmpbuilder/lvmbuilder
   /usr/share/doc/pbuilder/examples/pbuildd/buildd.sh
   /usr/share/doc/pbuilder/examples/pbuildd/hookdir/A10dpkg-l.sh
   /usr/share/doc/pbuilder/examples/pbuilder-distribution.sh
   /usr/share/doc/pbuilder/examples/pbuilder-test/000_prepinstall
   /usr/share/doc/pbuilder/examples/pbuilder-test/001_apprun
   /usr/share/doc/pbuilder/examples/pbuilder-test/002_libfile
   /usr/share/doc/pbuilder/examples/pbuilder-test/002_sample.c
   /usr/share/doc/pbuilder/examples/pbuilder-test/003_makecheck
   /usr/share/doc/pbuilder/examples/pbuilder-test/004_ldd
   /usr/share/doc/pbuilder/examples/pbuilder-test/README
   /usr/share/doc/pbuilder/examples/pbuilderrc
   /usr/share/doc/pbuilder/examples/rebuild/README
   /usr/share/doc/pbuilder/examples/rebuild/buildall
   /usr/share/doc/pbuilder/examples/rebuild/getlist
   /usr/share/doc/pbuilder/pbuilder-doc.de.html
   /usr/share/doc/pbuilder/pbuilder-doc.fr.html
   /usr/share/doc/pbuilder/pbuilder-doc.html
   /usr/share/doc/pbuilder/pbuilder-doc.ja.html
   /usr/share/doc/pbuilder/pbuilder-doc.pdf

Comment 23 Sandro Mani 2013-10-14 00:51:11 UTC
Please build for f20+, pbuilder won't be packaged for any older Fedora since devscripts is f20+.

Comment 24 Sandro Mani 2013-10-14 00:58:12 UTC
Rawhide scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6056732

Comment 25 Sergio Monteiro Basto 2013-10-14 01:41:13 UTC
Created attachment 811836 [details]
pbuilder.spec.patch

(In reply to Sandro Mani from comment #23)
> Please build for f20+, pbuilder won't be packaged for any older Fedora since
> devscripts is f20+.

OK I'm check it ... 

(In reply to Sandro Mani from comment #1 of bug 1017732)
> By the way: since I maintain a number of other debian related packages, I'm
> happy to comaintain this one too if desired.

Lets begin by pbuilder , your spec is very good, but I'd like to join some  knowledge of the spec Oron, with your spec.
uml subpackage and more and better comments and descriptions . 

Since Oron is unresponsive since July ,  we need other review request , do you want do it ? 

I send the patch

Comment 26 Sergio Monteiro Basto 2013-10-14 02:46:32 UTC
(In reply to Sandro Mani from comment #21)
> SPEC: http://smani.fedorapeople.org/review/pbuilder.spec
> SRPM: http://smani.fedorapeople.org/review/pbuilder-0.215-4.fc21.src.rpm
> 
> * Thu Oct 10 2013 Sandro Mani <manisandro@gmail.com> - 0.215-4
> - Improve README.fedora
> - Add some missing requires


fedora-review -b 969718 , works now ! 
we only have minor rpmlint errors and warnings but it can be fixed .

pbuilder.i686: W: non-conffile-in-etc /etc/bash_completion.d/pbuilder
replace with %{_sysconfdir}/bash_completion.d/pbuilder please 

pbuilder.src:97: E: hardcoded-library-path in %{_prefix}/lib/pbuilder/
pbuilder.src:112: E: hardcoded-library-path in %{_prefix}/lib/pbuilder/pbuilder-uml-checkparams
pbuilder.src:113: E: hardcoded-library-path in %{_prefix}/lib/pbuilder/pdebuild-uml-checkparams

we can't use %{_libdir}, mailman package use %global mmdir /usr/lib/%{name}

so we can use:  
%global pbdir /usr/lib/%{name} 
%{pbdir}/pbuilder-uml-checkparams 
%{pbdir}/pdebuild-uml-checkparams

Comment 27 Sandro Mani 2013-10-14 10:58:50 UTC
uml: Since user-mode-linux is unavailable for Fedora, the uml subpackage would be broken anyway, so it cannot be provided.

rpmlint:
- I think the non-conffile-in-etc warning can be ignored, all packages I looked at don't use %config. However, I've changed from
%{_sysconfdir}/bash_completion.d/pbuilder
to
%{_sysconfdir}/bash_completion.d/
since we don't Requires: bash-completion and hence we need to co-own the directory.

- The hardcoded-library-path errors are ignorable

pbdir: is this worth it for just three entries?


SPEC: http://smani.fedorapeople.org/review/pbuilder.spec
SRPM: http://smani.fedorapeople.org/review/pbuilder-0.215-5.fc21.src.rpm

* Mon Oct 14 2013 Sandro Mani <manisandro@gmail.com> - 0.215-5
- Package is noarch
- Co-own %{_sysconfdir}/bash_completion.d/

Comment 28 Sergio Monteiro Basto 2013-10-14 15:11:47 UTC
(In reply to Sandro Mani from comment #27)
> uml: Since user-mode-linux is unavailable for Fedora, the uml subpackage
> would be broken anyway, so it cannot be provided.

I think we should pack it , it is a work that we advance , when user-mode-linux , we can try install it with an rpm , though 

> rpmlint:
> - I think the non-conffile-in-etc warning can be ignored, all packages I
> looked at don't use %config. However, I've changed from
> %{_sysconfdir}/bash_completion.d/pbuilder
> to
> %{_sysconfdir}/bash_completion.d/
> since we don't Requires: bash-completion and hence we need to co-own the
> directory.

OK 

> - The hardcoded-library-path errors are ignorable
> 
> pbdir: is this worth it for just three entries?

yes , but not important.

Since Oron doesn't reply , please create other request review , and mark this one as duplicated, I will approve your review , and I will ask to be add as co-maintainer .

Comment 29 Sandro Mani 2013-10-14 17:37:39 UTC

*** This bug has been marked as a duplicate of bug 1018926 ***


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