This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 252010 - Review Request: rpmrebuild - A tool to build rpm file from rpm database
Review Request: rpmrebuild - A tool to build rpm file from rpm database
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks: RussianFedoraRemix
  Show dependency treegraph
 
Reported: 2007-08-13 15:35 EDT by Anderson Silva
Modified: 2009-04-19 04:28 EDT (History)
3 users (show)

See Also:
Fixed In Version: 2.1.1-9.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-10-10 21:45:42 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Anderson Silva 2007-08-13 15:35:59 EDT
Spec URL: http://www.the-silvas.com/rpmrebuild.spec
SRPM URL: http://www.the-silvas.com/rpmrebuild-2.1.1-2.src.rpm
Description:
You have an installed package on a computer, want to install on other
one, and do not find the rpm file anymore.
Or you want to distribute your customization in an rpm format
this tool is for you.

A few extra notes:

--- This is my first package, and I need a sponsor.
--- Output from rpmlint:

W: rpmrebuild hidden-file-or-dir /usr/lib/rpmrebuild/.popt
E: rpmrebuild only-non-binary-in-usr-lib
W: rpmrebuild dangerous-command-in-%postun rm

--- The Error are related to some shell scripts that go into the rpmrebuild directory, which seems consistent with the way rpm-4.x package is distributed.
Comment 1 Steve Milner 2007-08-22 19:41:32 EDT
-/+ rpmlint ran. Errors found but rpm does have the 'dangerous command' note.
the hidden file looks as if it is needed. The only-non-binary-in-usr-lib worries
me a bit ... 
+ package name is correct
+ spec name is correct
+ accepted license
- Looks like it's actually GPLv2+
- license not in %doc (commented out)
+ US English spec
+ Readable spec
+ Source is same as upstream
+ built and installed on i386
+ Deps look sane
+ No locale files
+ no .so files
+ not relocateable
+ no duplicate files
+ perms look sane
+ clean is good
+ macros in use
+ contains code and OK content
-/+ %doc missing ... but doesn't look like it affects the code
+ doesn't own files/dirs in other packages
+ install cleans first
- the descirption seems a bit odd. Maybe the following (from the rpmrebuild
site) would suite it:
rpmrebuild is a tool to build an RPM file from a package that has already been
installed
in a basic use, rpmrebuild use do not require any rpm building knowledge
+ tested locally and worked
Comment 2 Anderson Silva 2007-08-23 16:36:57 EDT
fixed the following items:

- Looks like it's actually GPLv2+
- license not in %doc (commented out)
-/+ %doc missing ... but doesn't look like it affects the code

Comment 3 Anderson Silva 2007-08-23 16:37:34 EDT
New src RPM posted:

http://www.the-silvas.com/rpmrebuild-2.1.1-3.src.rpm

Comment 4 Steve Milner 2007-08-26 16:19:23 EDT
+ license is correct
+ license in %doc (COPYING)
- description makes more sense but ...
<snip>
E: rpmrebuild description-line-too-long in a basic use, rpmrebuild use do not
require any rpm building knowledgethis tool is for you.
<snip>
Changing the description to the following fixes this:

%description
A tool to build an RPM file from a package that has already been installed
in a basic use, rpmrebuild use do not require any rpm building knowledge.
Comment 5 Anderson Silva 2007-08-28 13:25:49 EDT
description fixed.

http://www.the-silvas.com/rpmrebuild-2.1.1-4.src.rpm
http://www.the-silvas.com/rpmrebuild.spec

updated.
Comment 6 Steve Milner 2007-09-05 11:05:51 EDT
Description looks good! It looks good to me. Since you are a first time
contributor I can not pass your review but hopefully a sponsor will pop by and
run through.
Comment 7 Ralf Corsepius 2007-09-05 11:51:03 EDT
Some remarks:

* Your %install references
/usr/lib/rpmrebuild/rpmrebuild_parser.src

while your %files contains:
%{_libdir}/rpmrebuild/rpmrebuild_parser.src

This doesn't make a visible difference because your package is "noarch",
nevertheless this is inconsistent.

* The block-delete-/bin/sh magic in %postun can be replaced with a one-line
sed-call:
sed -e '/RPMREBUILD START/,/RPMREBUILD END/d' ...
Comment 8 Anderson Silva 2007-09-05 12:43:27 EDT
Cool Thanks Ralf.

I have fixed your recommendations.

New spec at: 
http://www.the-silvas.com/rpmrebuild.spec
New srpm at:
http://www.the-silvas.com/rpmrebuild-2.1.1-5.src.rpm
Comment 9 Till Maas 2007-09-06 09:33:57 EDT
You should mention in the spec that the tarball is from 
http://downloads.sourceforge.net/rpmrebuild/rpmrebuild-2.1.1-1rpm4.src.rpm
Or maybe even using above URL as Source0: works.

See: http://fedoraproject.org/wiki/Packaging/SourceURL

Also maybe you should use %setup -q -n rpmrebuild instead of %setup -c
rpmrebuild.  Or extract the src.rpm from above URL with rpmdev-extract instead
of extracting the tarball manually before you add it to the srpm. Which is imho
the preferred way.
Comment 10 Till Maas 2007-09-06 09:41:52 EDT
I forgot, you need to follow
http://fedoraproject.org/wiki/PackageMaintainers/Join since the Fedora Account
System seems not to know you. I already made this bug blocking FE-NEEDSPONSOR.
Comment 11 Anderson Silva 2007-09-06 09:50:34 EDT
Till,

I havent made the account yet because the docs says only to make an account once
I have a sponsor.

Thanks,

AS
Comment 12 Anderson Silva 2007-09-07 15:35:35 EDT
Packages updated:

http://www.the-silvas.com/rpmrebuild.spec
http://www.the-silvas.com/rpmrebuild-2.1.1-6.src.rpm

My comments:

- I've read http://fedoraproject.org/wiki/PackageMaintainers/Join. There it
states: "Create an account in the  Fedora Account System *when you have found a
sponsor and got your package approved*" - My package haven't gotten a sponsor
and I my package hasn't been approved, so I haven't created an account yet.

- I looked around the web, and I didn't find any examples of anyone using a
src.rpm as Source0: on their specs. I have added some comments to document the
fact that the tar.gz upstream file is coming from a src.rpm.

- I have contacted the upstream developer, and I have asked him to make the
tar.gz available officially via the sf.net site.

- I've read the comments on BUG 177841, and I am doing the best I can with this
package with the time I have in my hands, I unfortunately will not have much
time left to review other packages. I might potentially in the future be able to
submit other packages, but I want to be able to get my first one 'officially' in
   to get used to the process. 
Comment 13 Till Maas 2007-09-07 16:00:33 EDT
It is not wrong that you do not have an account. It was the reason why you need
to follow the page I showed you. People that have an account and are sponsored
need to follow the steps on another page. One steps in PackageMaintainers/Join
was to block FE-NEEDSPONSOR, what I also did for you. But I also might have been
wrong, so I explained to you, why I decided to block FE-NEEDSPONSOR. However, I
hope your confusion about comment:10 is now blown away.

If you do not have time to review other packages, then chances to be sponsored
are low, because a sponsor has nothing to decide, whether or not you know the
guidelines well enough.
Comment 14 Steve Milner 2007-09-07 16:33:39 EDT
I believe that Anderson is following the proper steps .... from
http://fedoraproject.org/wiki/PackageMaintainers/Join

"Create an account in the  Fedora Account System when you have found a sponsor
and got your package approved"

Anderson does not have a sponsor and does not have his package approved.

It is true that he would be helping by looking at other packages and making
notes but, as said in http://fedoraproject.org/wiki/PackageMaintainers/Join:

"Read some other package submissions to learn about packaging and gain
familiarity with the process and requirements."

Once he gets a sponsor, packaged approved, then an account he can get in the
fedorabugs group as stated:

"Now, repeat the process using fedorabugs as the Groupname. This will allow you
to make changes to the state of bugs in Bugzilla, which is what you'll need to
do to get them checked in. It will also allow you to do complete package
reviews, including approving packages yourself!"
Comment 15 Till Maas 2007-09-07 16:58:50 EDT
(In reply to comment #14)
> I believe that Anderson is following the proper steps .... from
> http://fedoraproject.org/wiki/PackageMaintainers/Join

Did this review request block FE-NEEDSPONSOR before I changed it? No, it did
not. I quote from PackageMaintainers/Join:

| Some potential sponsors will look at the FE-NEEDSPONSOR bug in bugzilla to find 
| packages to review. You can add your package to this list by editing your review
| request bug and adding FE-NEEDSPONSOR in the 'Bug xyz blocks' field (where xyz 
| is the bug number for your review request).

Sorry, I do not know how to express myself in English good enough, if it is
still not clear what I wrote, then please ignore it completely.
Comment 16 Mamoru TASAKA 2007-09-19 10:21:43 EDT
For 2.1.1-6:

* Source URL
  - Any reason you can't use
    http://downloads.sourceforge.net/rpmrebuild/rpmrebuild-2.1.1.tar.gz ?
    (and use %{name} for rpmrebuild, %{version} for 2.1.1)

* Redundant Requires
  - rpm has "Requires: coreutils" and "Requires: textutils, fileutils"
    is not needed for this package.

* %_libdir vs noarch
  - This package is marked as noarch, however %files list use
    %_libdir, which differs between on 32bit arch and on 64bit arch.

    i.e. While this package is marked as noarch (i.e. rebuilding your srpm
         should create the same bin rpm on any arch), the result of rebuilding
         your srpm will differ when arch changes (or maybe on 64bit arch
         rpmbuild fails). This is not right.

* Requires
  - Check Requires again. For example, /usr/lib/rpmrebuild/rpmrebuild.sh
    contains the lines:
-------------------------------------------------------
   179  function RpmBuild
   180  {
   181          # reconstruction fichier rpm : le src.rpm est inutile
   182          # build rpm file, the src.rpm is not usefull to do
   183          # for rpm 4.1 : use rpmbuild
   184          local BUILDCMD=rpm
   185          [ -x /usr/bin/rpmbuild ] && BUILDCMD=rpmbuild
   186          eval $BUILDCMD $rpm_defines -bb $rpm_verbose $additional
${FIC_SPEC} || {
   187                  Error "package '${PAQUET}' build failed"
   188                  return 1
   189          }
   190          
   191          return 0
   192  }
-------------------------------------------------------
     This means this package (rpmrebuild) should have
     "Requires: rpm-build" (%_bindir/rpmbuild is in rpm-build)

? %postun
  - Just a question: what package owns /etc/popt and what is
    the file for? Maybe /etc/popt is already obsolete?
    (Please consider for FC6+)
  - By the way, use macros. /etc must be %_sysconfdir.
  - While I don't know what %postun does, however the lines:
--------------------------------------------------------
  cp /etc/popt /etc/pot.tmp
  sed -e '/RPMREBUILD START/,/RPMREBUILD END/d' /etc/popt.tmp > /etc/popt
--------------------------------------------------------
    apparently contains a typo.

* Directory ownership issue
  - Please make it sure that all directories created by this
    package are owned by this package.
    For example, %_libdir/rpmbuild is not owned by any package.
Comment 17 Anderson Silva 2007-09-19 11:00:16 EDT
actually, the tar.gz was added to the list after I emailed the developer and
asked him to make it available. I haven't had the chance to fix the SPEC for it.
It was done late last week.

about popt RPM.

popt.i386                                1.10.2-37.el5             
Matched from:
popt
Popt is a C library for parsing command line parameters. Popt was
heavily influenced by the getopt() and getopt_long() functions, but it
improves on them by allowing more powerful argument expansion. Popt
can parse arbitrary argv[] style arrays and automatically set
variables based on command line arguments. Popt allows command line
arguments to be aliased via configuration files and includes utility
functions for parsing arbitrary strings into argv[] arrays using
shell-like rules.

I will take a look at the other issues.
Comment 18 Mamoru TASAKA 2007-09-19 11:11:16 EDT
(In reply to comment #17)
> about popt RPM.

I am not asking about popt, but asking about %_sysconfdir/popt.
At least rawhide popt (popt-1.12-3.fc8) spec file does not mention
about %_sysconfdir/popt (note: at least on F-7+, popt is split out
from rpm).
Comment 19 Ralf Corsepius 2007-09-19 11:18:45 EDT
/etc/popt is being used by libpopt, but is not used by default. 
It is supposed to take user customizations.

This makes we wonder about the %postun fragment cited above.
You probably should check for presence of %{_sysconfir}/popt before sed'ing,
because it might not be present (user could have deleted it).

Something similar to
test -w %{_sysconfdir}/popt \
&& sed -i -e .... %{_sysconfdir}/popt
Comment 20 Mamoru TASAKA 2007-09-19 11:33:48 EDT
(In reply to comment #19)
> /etc/popt is being used by libpopt, but is not used by default. 
> It is supposed to take user customizations.

Thank you, Ralf.

I forgot to mention more one issue:

* rpmlint
rpmrebuild.src:98: W: macro-in-%changelog _libdir
rpmrebuild.src:104: W: macro-in-%changelog doc

  - In changelog, use %% to prevent macros from being expanded,
    for example:
----------------------------------------------------
* Thu Sep 5 2007 Anderson Silva <ansilva@redhat.com> 2.1.1-5
- Optimized postun with sed
- Replaced /usr/lib with %%{_libdir} <- (althogh this is not right)
----------------------------------------------------
Comment 21 Anderson Silva 2007-09-19 11:34:37 EDT
Thanks guys, I will try to address all these issues this weekend. Work is
killing me right now, but I will definitely work on this. 
Comment 22 Mamoru TASAKA 2007-09-27 08:49:04 EDT
ping?
Comment 23 Anderson Silva 2007-09-27 10:57:27 EDT
Alright guys... I finally got a few minutes to address some of the issues
discussed above. First, let me thank everyone for helping me out with this.

Here are the updated spec/src:

http://www.the-silvas.com/rpmrebuild.spec
http://www.the-silvas.com/rpmrebuild-2.1.1-7.src.rpm

What I have fixed:

* Thu Sep 27 2007 Anderson Silva <ansilva@redhat.com> 2.1.1-7
- Changed /etc to %%{_sysconfdir}
- Fixed reference on postun section
- Using tarball as Source0
- Added require rpm-build
- Removed require for textutils, fileutils
- Added directories to belong to package

I still have 2 questions/comments:

1. About Comment #19:

'You probably should check for presence of %{_sysconfir}/popt before sed'ing,
because it might not be present (user could have deleted it).'

There is a if statement that checks to see if the file exists before running the
sed.

2. About Comment #16:

'* %_libdir vs noarch
  - This package is marked as noarch, however %files list use
    %_libdir, which differs between on 32bit arch and on 64bit arch'

OK, this is one problem that I am not sure how to solve. What do you guys
suggest? Should I contact upstream and ask them to change their project to lay
out their shell scripts somewhere else in the system? Which location what that
be outside /usr/lib?

I think they put it there, because the rpm package layout their stuff there too.

Should I make this rpm i386 only? even though in theory it should work on 64bit
arch?

I am open to suggestions. 
Comment 24 Mamoru TASAKA 2007-09-28 03:54:31 EDT
For 2.1.1-7:

* About %postun:
  - Still there is a typo
--------------------------------------------------------
  rm -f %{_sysconfdir}/pot.tmp
--------------------------------------------------------

* Directory ownership issue
(In reply to comment #23)
> - Added directories to belong to package
  - Please check all. Still %_prefix/lib/rpmbuild/plugins
    is not owned.

* For %_prefix/lib/rpmbuild:
(In reply to comment #23)
> OK, this is one problem that I am not sure how to solve. What do you guys
> suggest?
  - You should use %_prefix/lib/rpmbuild, not %_libdir/rpmbuild.
    This is not a upstream issue, just rpm macro usage issue.

* rpmlint for srpm:
---------------------------------------------------------
rpmrebuild.src:112: W: macro-in-%changelog doc
---------------------------------------------------------
  - Please use %%doc in %changelog.
Comment 25 Anderson Silva 2007-09-28 09:15:32 EDT
awesome thanks. I just realized that I have been using the rpmlint for rhel
instead of fedora. Either later today or this weekend I will address these
issues, and run the fedora rpmlint against my stuff.
Comment 26 Anderson Silva 2007-09-28 10:55:30 EDT
Ok, I have fixed the issues described on Comment #24, but if I use use
%_prefix/lib/rpmbuild instead of %_libdir/rpmbuild, rpmlints gives the following
errors:

rpmrebuild.spec:34: E: hardcoded-library-path in %{_prefix}/lib/rpmrebuild/VERSION
rpmrebuild.spec:35: E: hardcoded-library-path in
%{_prefix}/lib/rpmrebuild/rpmrebuild_parser.src
rpmrebuild.spec:37: E: hardcoded-library-path in
%{_prefix}/lib/rpmrebuild/rpmrebuild_parser.src
rpmrebuild.spec:38: E: hardcoded-library-path in
%{_prefix}/lib/rpmrebuild/rpmrebuild_parser.src
rpmrebuild.spec:62: E: hardcoded-library-path in %{_prefix}/lib/rpmrebuild/
Comment 27 Anderson Silva 2007-09-28 10:55:59 EDT
PS - I haven't posted the new spec yet due to the error above.
Comment 28 Mamoru TASAKA 2007-09-28 11:05:29 EDT
(In reply to comment #26)
> Ok, I have fixed the issues described on Comment #24, but if I use use
> %_prefix/lib/rpmbuild instead of %_libdir/rpmbuild, rpmlints gives the following
> errors:
> 
> rpmrebuild.spec:34: E: hardcoded-library-path in %{_prefix}/lib/rpmrebuild/VERSION

Yes, these rpmlint errors are expected. However, as "rpm" actually
uses /usr/lib, not %_libdir, this rpmlint error cannot be
avoided for this package.
Comment 29 Anderson Silva 2007-09-28 11:15:58 EDT
http://www.the-silvas.com/rpmrebuild.spec
http://www.the-silvas.com/rpmrebuild-2.1.1-8.src.rpm

Here we go then! Thanks again for your help.
Comment 30 Mamoru TASAKA 2007-09-28 12:06:32 EDT
Ah.. after I checked %postun scriptlet again, I came to
think that the following is simpler:

----------------------------------------------------------
%postun
# Remove all lines which contain 'RPMREBUILD START' 
# to 'RPMREBUILD END' in %{_sysconfdir}/popt
[ $1 -eq 0 ] || exit 0
if [ -f %{_sysconfdir}/popt ]; then
  sed -i -e '/RPMREBUILD START/,/RPMREBUILD END/d' %{_sysconfdir}/popt
fi
---------------------------------------------------------
Comment 32 Mamoru TASAKA 2007-09-29 11:30:10 EDT
Okay.

-------------------------------------------------
  This package (rpmrebuild) is APPROVED by me
-------------------------------------------------

Please follow the procedure according to:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account".
At a point a mail should be sent to sponsor members which notifies
that you need a sponsor (at the stage, please also write on
this bug for confirmation that you requested for sponsorship)
Then I will sponsor you.

If you want to import this package into Fedora 7, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on Fedora rebuilding system).
Comment 33 Anderson Silva 2007-09-29 15:37:42 EDT
I've created my account on Fedora, username: afsilva.

I need a Sponsor. 

Thanks Mamoru, whenver you think I am ready, Iwill start working on the import
for F7 (during the week).

AS
Comment 34 Mamoru TASAKA 2007-09-29 21:59:12 EDT
It seems you have not requested for cvsextras, and even you
does not seem to have signed CLA agreement.

Please follow 

http://fedoraproject.org/wiki/PackageMaintainers/Join

again. You seems to have finished "Get a Fedora Account" item 2.
You have to finish item 4 of "Get a Fedora Account" before I can
sponsor you.
Comment 35 Anderson Silva 2007-09-29 23:22:21 EDT
Ok, my bad. It seems due to the fact that I am a Red Hat employee I need to do
the  cla_redhat. I have my request in.
Comment 36 Anderson Silva 2007-10-01 10:11:25 EDT
Mamoru, 

I think I am all set now.

AS
Comment 37 Mamoru TASAKA 2007-10-01 11:09:55 EDT
Okay, now I should be sponsoring you.
Please proceed accoring to the "Join" wiki.
Comment 38 Anderson Silva 2007-10-01 11:44:24 EDT
Got it.

So, here is a dumb question... well, another dumb question, from the wiki:

To build Packages for Fedora Extras 6 or EPEL, you need Plague and for Fedora 7
and later (including devel) you need Koji.

At work, I am on RHEL 5, my repos don't have koji, do I need F7 installed to use
koji? Should I be able to grab the RPMs from a fedora repo, and install it on
RHEL5... is that even recommended?

Thanks,

AS
Comment 39 Mamoru TASAKA 2007-10-01 12:15:19 EDT
Well, that is the first case!

So I have only Fedora rawhide and don't have RHEL. However
EPEL 5 seems to have koji
http://download.fedora.redhat.com/pub/epel/5/i386/repoview/koji.html
and perhaps you can use it.
Comment 40 Anderson Silva 2007-10-01 12:33:03 EDT
New Package CVS Request
=======================
Package Name: rpmrebuild
Short Description: A tool to build rpm file from rpm database
Owners: afsilva
Branches: F-7 EL-5
InitialCC: 
Cvsextras Commits: no
Comment 41 Kevin Fenzi 2007-10-01 15:40:18 EDT
cvs done.
Comment 42 Mamoru TASAKA 2007-10-04 03:45:44 EDT
Please rebuild this on devel, F-7, EL-5, request to koji on F-7,
and don't forget to close this bug when done.
Comment 43 Anderson Silva 2007-10-04 09:05:37 EDT
np, my wife just had our third son this week, so I am out this week. But I will
work on this stuff on Monday.
Comment 44 Fedora Update System 2007-10-10 21:45:41 EDT
rpmrebuild-2.1.1-9.fc7 has been pushed to the Fedora 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.