Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
Summary: Merge Review: cpio Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: nobody@fedoraproject.org QAContact: fedora-package-review@redhat.com CC: pvrabec@redhat.com
Fedora Merge Review: cpio
http://cvs.fedora.redhat.com/viewcvs/devel/cpio/ Initial Owner: pvrabec@redhat.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
ruben@rubenkerkhof.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |ruben@rubenkerkhof.com Flag| |fedora-review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
ruben@rubenkerkhof.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|ruben@rubenkerkhof.com |pvrabec@redhat.com CC| |ruben@rubenkerkhof.com Flag|fedora-review? |fedora-review-
------- Additional Comments From ruben@rubenkerkhof.com 2007-02-03 13:09 EST ------- * RPM name is OK * Source cpio-2.6.tar.gz is the same as upstream * This is the latest version * Builds fine in mock * File list looks OK
Needs work: * BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) (wiki: PackagingGuidelines#BuildRoot) * Missing SMP flags. If it doesn't build with it, please add a comment (wiki: PackagingGuidelines#parallelmake) * BuildRequires: gettext is missing (required to build the translations) * The %makeinstall macro should not be used (wiki: PackagingGuidelines#MakeInstall) * The package should contain the text of the license (wiki: Packaging/ReviewGuidelines) * Please preserve timestamps when installing files
rpmlint is not silent:
[ruben@odin cpio]$ rpmlint cpio-2.6-23.fc6.src.rpm W: cpio prereq-use /sbin/rmt W: cpio prereq-use /sbin/install-info
Use Requires(post) and Requires(preun) instead
W: cpio mixed-use-of-spaces-and-tabs (spaces: line 99, tab: line 3)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
------- Additional Comments From pvrabec@redhat.com 2007-02-07 04:44 EST ------- If you want to check upgrade candidate too, you can find it at: http://people.redhat.com/pvrabec/rpms/tar-1.16.1-1.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
------- Additional Comments From pvrabec@redhat.com 2007-02-07 07:09 EST ------- Oops, I'm sorry, ignore comment #2
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
------- Additional Comments From pvrabec@redhat.com 2007-02-07 12:27 EST ------- fixed in cpio-2.6-24.fc7.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
ruben@rubenkerkhof.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
------- Additional Comments From ruben@rubenkerkhof.com 2007-02-07 16:23 EST ------- Looks good.
One thing though, could you preserve the timestamps when installing files (with install -p or cp -p).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
------- Additional Comments From pvrabec@redhat.com 2007-02-08 07:14 EST ------- What do you mean? These is "install -c -p -m 0644" used for man page in spec file. Is there any place I don't preserve timestamps?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
------- Additional Comments From katzj@redhat.com 2007-02-08 09:32 EST ------- Removal of the bindir, etc definitions at the top of the spec file move the cpio binary from /bin to /usr/bin which is a bug.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
------- Additional Comments From pvrabec@redhat.com 2007-02-08 10:44 EST ------- fixed in -25.fc7
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
ruben@rubenkerkhof.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEEDINFO
------- Additional Comments From ruben@rubenkerkhof.com 2007-02-08 17:24 EST ------- Hi Peter,
About the timestamps, the line install -c -p -m 0644 %{SOURCE1} ${RPM_BUILD_ROOT}%{_mandir}/man1 is redundant, make install already does this, so it can be removed.
test -z "/bin" || mkdir -p -- "/var/tmp/cpio-2.6-25.fc6-root-mockbuild/bin" /usr/bin/install -c 'cpio' '/var/tmp/cpio-2.6-25.fc6-root-mockbuild/bin/cpio'
So cpio is installed without preserving the timestamp.
And a new rpmlint error:
Source RPM: W: cpio mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 86)
I've attached a patch with the fixes.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
------- Additional Comments From ruben@rubenkerkhof.com 2007-02-08 17:33 EST ------- Created an attachment (id=147709) --> (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=147709&action=vie...) Fix timestamps
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
pvrabec@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?
------- Additional Comments From pvrabec@redhat.com 2007-02-09 05:24 EST ------- thnx. Ruben, patch applied.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
pvrabec@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEEDINFO |ASSIGNED Flag|needinfo? |
------- Additional Comments From pvrabec@redhat.com 2007-02-09 08:52 EST ------- according to "make DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p" install" should we preserve timestamps on files which were rebuilt?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
ruben@rubenkerkhof.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEEDINFO
------- Additional Comments From ruben@rubenkerkhof.com 2007-02-10 06:32 EST ------- Preserving timestamps on generated files is pretty useless indeed. But in this case, setting INSTALL="install -p" takes care of preserving timestamps on the manpages and config files as well.
The only thing the Guidelines say about this is: When adding file copying commands in the spec file, consider using a command that preserves the files' timestamps, eg. cp -p or install -p.
I made a small typo in the patch, could you change the version in the last %changelog entry from 2.6.26 to 2.6-26?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
pvrabec@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEEDINFO |ASSIGNED
------- Additional Comments From pvrabec@redhat.com 2007-02-20 11:51 EST ------- fixed
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium Priority|normal |medium
ruben@rubenkerkhof.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|pvrabec@redhat.com |ruben@rubenkerkhof.com Flag|fedora-review- |fedora-review+
------- Additional Comments From ruben@rubenkerkhof.com 2007-04-20 18:24 EST ------- Sorry that it took me so long. I don't see any further blockers so this package is approved.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cpio
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225656
ruben@rubenkerkhof.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE
package-review@lists.fedoraproject.org