Bug 969718 - Review Request: pbuilder - Personal package builder for Debian packages
Review Request: pbuilder - Personal package builder for Debian packages
Status: CLOSED DUPLICATE of bug 1018926
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Sergio Monteiro Basto
Fedora Extras Quality Assurance
:
: 591388 (view as bug list)
Depends On: 591192 591389 961141 1010000
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-01 21:45 EDT by Oron Peled
Modified: 2013-10-14 13:37 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-10-14 13:37:39 EDT
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-13 21:41 EDT, Sergio Monteiro Basto
no flags Details | Diff

  None (edit)
Description Oron Peled 2013-06-01 21:45:27 EDT
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-01 21:52:59 EDT
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-01 21:54:32 EDT
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-01 21:56:13 EDT
*** Bug 591388 has been marked as a duplicate of this bug. ***
Comment 4 Sergio Monteiro Basto 2013-06-08 16:27:42 EDT
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 19:43:33 EDT
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 17:47:57 EDT
Mario, do you want help on review ?
Comment 8 Oron Peled 2013-07-08 16:32:31 EDT
(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 16:36:26 EDT
(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 17:03:36 EDT
(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 09:44:46 EDT
Status?
Comment 12 Sergio Monteiro Basto 2013-09-05 11:12:46 EDT
Lets build devscripts
Comment 13 Sandro Mani 2013-09-19 12:46:19 EDT
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 13:29:33 EDT
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 14:52:16 EDT
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 15:08:33 EDT
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 07:26:53 EDT
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 11:31:51 EDT
ok devscripts is in F20 testing, all dependencies are solved , what review should I follow ?
Comment 19 Sandro Mani 2013-10-08 13:10:42 EDT
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 15:06:40 EDT
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 09:14:22 EDT
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-13 20:41:13 EDT
(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-13 20:51:11 EDT
Please build for f20+, pbuilder won't be packaged for any older Fedora since devscripts is f20+.
Comment 24 Sandro Mani 2013-10-13 20:58:12 EDT
Rawhide scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6056732
Comment 25 Sergio Monteiro Basto 2013-10-13 21:41:13 EDT
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-13 22:46:32 EDT
(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 06:58:50 EDT
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 11:11:47 EDT
(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 13:37:39 EDT

*** 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.