[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=602574 Jeremy Sanders changed: What|Removed |Added Status|ASSIGNED|CLOSED Resolution||NEXTRELEASE Last Closed||2010-08-25 17:22:47 --- Comment #13 from Jeremy Sanders 2010-08-25 17:22:47 EDT --- Package successfully added to repository, built and queued up in bohdi for updates testing (F12, F13 and F14). Thanks very much for Martin Gieseking's hard work. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=602574 --- Comment #12 from Kevin Fenzi 2010-08-25 13:31:48 EDT --- Git done (by process-git-requests). -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=602574 Jeremy Sanders changed: What|Removed |Added Flag||fedora-cvs? --- Comment #11 from Jeremy Sanders 2010-08-25 04:55:06 EDT --- Thanks for the approval Martin. Admins: please can you add the GIT repositories for the new package patchelf as it has now been accepted. Petr Pisar has agreed to be comaintainer of this package. I will file a new bug to track the PPC/PPC64 issues once we have a bugzilla entry for this package and put it in the spec file. New Package SCM Request === Package Name: patchelf Short Description: A utility for patching ELF binaries Owners: jsanders ppisar Branches: f12 f13 f14 InitialCC: jsanders ppisar -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=602574
Martin Gieseking changed:
What|Removed |Added
Flag|fedora-review? |fedora-review+
--- Comment #10 from Martin Gieseking 2010-08-24
14:17:28 EDT ---
Here's the formal review. Since I couldn't find any further issues, the package
is ready now.
After having had another look at the packaging guidelines, I noticed that you
should file Bugzilla tickets for the ppc issue (now as the package has been
approved). The corresponding bug numbers should then be added to the spec file.
See [1] for further details.
[1]
https://fedoraproject.org/wiki/Packaging/Guidelines#Architecture_Build_Failures
$ rpmlint /var/lib/mock/fedora-13-x86_64/result/*.rpm
patchelf.src: W: spelling-error %description -l en_US executables ->
executable, executable s, executants
patchelf.x86_64: W: spelling-error %description -l en_US executables ->
executable, executable s, executants
3 packages and 0 specfiles checked; 0 errors, 2 warnings.
The spelling errors are false positive and can be ignored.
-
key:
[+] OK
[.] OK, not applicable
[X] needs work
-
[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
[+] MUST: The License field in the package spec file must match the actual
license.
[+] MUST: The file containing the text of the license(s) for the package must
be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
$ md5sum patchelf-0.5.tar.bz2*
c41fc98091d15dc93ba876c3ef11f43c patchelf-0.5.tar.bz2
c41fc98091d15dc93ba876c3ef11f43c patchelf-0.5.tar.bz2.1
[+] MUST: The package MUST successfully compile and build into binary rpms on
at least one primary architecture.
[X] MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in
bugzilla, describing the reason that the package does not compile/build/work on
that architecture.
- File two Bugzilla tickets for the ppc and ppc64 issue.
- Add the corresponding bug numbers to the spec file.
[+] MUST: All build dependencies must be listed in BuildRequires.
[.] MUST: The spec file MUST handle locales properly.
[.] MUST: Packages storing shared library files (not just symlinks) must call
ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates.
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[.] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[.] MUST: If a package contains library files with a suffix ...
[.] MUST: devel packages must require the base package.
[+] MUST: Packages must NOT contain any .la libtool archives.
[.] MUST: Packages containing GUI applications must include a %{name}.desktop
file.
[+] MUST: Packages must not own files or directories already owned by other
packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.
[.] SHOULD: If the source package does not include license text(s), ...
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The package should compile and build into binary rpms on all
supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described.
[.] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base
package using a fully versioned dependency.
[.] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg.
[.] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin,
/usr/bin, or /usr/sbin ...
Package APPROVED
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug
[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=602574 Martin Gieseking changed: What|Removed |Added Status|NEW |ASSIGNED AssignedTo|nob...@fedoraproject.org|martin.giesek...@uos.de Flag||fedora-review? --- Comment #9 from Martin Gieseking 2010-08-24 08:11:43 EDT --- (In reply to comment #8) > I've disabled ppc and ppc64 architectures in the spec file. Do we need a > separate bug to file this problem? I've put a comment in the spec file > pointing > to here. OK, fine. You don't need to file the ppc issue separately. If possible, it should be fixed upstream in a future version. Maybe you can work together with the developer to find the problem, e.g. by using the ppc test machine [1]. I'm going to do the formal review later today. [1] http://fedoraproject.org/wiki/Test_Machine_Resources_For_Package_Maintainers -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=602574 --- Comment #8 from Jeremy Sanders 2010-08-24 05:46:36 EDT --- Upstream don't have the resources to investigate the ppc/ppc64 problems: see http://mail.cs.uu.nl/pipermail/nix-dev/2010-August/005085.html I've disabled ppc and ppc64 architectures in the spec file. Do we need a separate bug to file this problem? I've put a comment in the spec file pointing to here. Latest spec file and SRPM are here: http://barmag.net/tmp/patchelf.spec http://barmag.net/tmp/patchelf-0.5-5.fc12.src.rpm I've made a scratch build here: http://koji.fedoraproject.org/koji/taskinfo?taskID=2421213 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=602574 Petr Pisar changed: What|Removed |Added CC||ppi...@redhat.com --- Comment #7 from Petr Pisar 2010-08-20 03:12:31 EDT --- *** Bug 625491 has been marked as a duplicate of this bug. *** -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=602574 --- Comment #6 from Martin Gieseking 2010-08-09 06:20:56 EDT --- OK, thanks for the feedback. I think we should wait for a comment from the upstream developer before finishing the review. Maybe you can contact him again and ask for a status update. If he doesn't respond, I suggest to exclude the ppc archs for now. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=602574 --- Comment #5 from Jeremy Sanders 2010-08-09 05:18:43 EDT --- Sorry for the delay. I've fixed the elf.h issue. I've also contacted upstream about the second issue. He wanted some debugging data which I have provided. Still no word from upstream for a several weeks. Here are the versions that delete elf.h before building: http://barmag.net/tmp/patchelf.spec http://barmag.net/tmp/patchelf-0.5-4.fc12.src.rpm -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=602574 --- Comment #4 from Martin Gieseking 2010-08-09 03:00:08 EDT --- Jeremy, do you have any news on the above mentioned issues (comment #3)? Are you still interested in this package? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=602574 --- Comment #3 from Martin Gieseking 2010-06-15 10:22:57 EDT --- Hi Jeremy, I just found two more things that need some attention: - The tarball contains a bundled version of elf.h. I suggest to remove it and use the one provided by glibc-headers. It should be picked up automatically if the bundled file is not present. - The tests fail for ppc/ppc64: http://koji.fedoraproject.org/koji/taskinfo?taskID=2251623 According to the project homepage (http://nixos.org/patchelf.html) patchelf is actually supposed to work on PowerPCs too. You should ask upstream whether this is a test-only issue or a bug in the program code. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=602574 --- Comment #2 from Jeremy Sanders 2010-06-14 15:15:59 EDT --- Hi Martin - thanks for the helpful comments. An updated version of the package is here: Spec URL: http://barmag.net/tmp/patchelf.spec SRPM URL: http://barmag.net/tmp/patchelf-0.5-3.fc12.src.rpm Changes: * Now GPLv3+ in spec file * Replaced separate rm and rmdir (was worried might delete everything if was wrong) with rm -rf * Removed %doc from man page * Removed tabs and replaced with spaces * Added %check section - seems to pass fine * Added buildroot so that it would be easy to put in EPEL I've sent the man page upstream - it will get included in the next release. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 602574] Review Request: patchelf - a utility for patching ELF binaries
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=602574
Martin Gieseking changed:
What|Removed |Added
CC||martin.giesek...@uos.de
--- Comment #1 from Martin Gieseking 2010-06-14
10:37:51 EDT ---
Hi Jeremy,
here are some initial comments:
- according to file README, the license is GPLv3+, not GPLv3 only
- to simplify things, I suggest to replace the rm and rmdir line with
rm -rf %{buildroot}/usr/share/doc/
- remove "%doc" from "%doc %{_mandir}/man1/patchelf.1*"
- don't mix tabs and spaces for indentation
- The tarball contains a couple of tests. It's probably a good idea to run them
(with "make check" in a %check section).
$ rpmlint /var/lib/mock/fedora-13-i386/result/*.rpm
patchelf.i686: W: spelling-error %description -l en_US executables ->
executable, executable s, executrices
patchelf.src: W: spelling-error %description -l en_US executables ->
executable, executable s, executrices
patchelf.src: W: no-buildroot-tag
patchelf.src:10: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 10)
3 packages and 0 specfiles checked; 0 errors, 4 warnings.
The missing buildroot tag can be ignored if you plan to maintain this package
for Fedora only. Otherwise, you should add one and also clean the buildroot in
%install.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
