[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561
Ondrej Oprala oopr...@redhat.com changed:
What|Removed |Added
Status|ASSIGNED|CLOSED
Resolution|--- |NEXTRELEASE
Last Closed||2014-06-23 06:17:42
--- Comment #21 from Ondrej Oprala oopr...@redhat.com ---
As the package is now successfully built, I am closing this bugzilla.
Thank you all, who participated.
As a sidenote: Just setting noarch still runs find-debuginfo.sh (redundantly).
Explicitly setting debug_package to %{nil} allows to omit this part during the
rpm build.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561 --- Comment #20 from Kevin Fenzi ke...@scrye.com --- Git done (by process-git-requests). -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561 Kevin Fenzi ke...@scrye.com changed: What|Removed |Added Flags|fedora-cvs? |fedora-cvs+ -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561 Christopher Meng i...@cicku.me changed: What|Removed |Added Status|NEW |ASSIGNED CC||i...@cicku.me Assignee|nob...@fedoraproject.org|i...@cicku.me --- Comment #1 from Christopher Meng i...@cicku.me --- Why not mark it as noarch? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561 --- Comment #2 from Ondrej Oprala oopr...@redhat.com --- What do you mean? There's 'BuildArch: noarch' in the spec. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561
Christopher Meng i...@cicku.me changed:
What|Removed |Added
Flags||fedora-review?
--- Comment #3 from Christopher Meng i...@cicku.me ---
(In reply to Ondrej Oprala from comment #2)
What do you mean? There's 'BuildArch: noarch' in the spec.
Yes, I know that, but if you mark it as noarch, no need to write:
%global debug_package %{nil}
in the spec.
Issues:
1. Since git 2.0.0 will appear(already done) in rawhide only, you should drop
all old fashioned rpm syntax:
a. rm -rf %{buildroot} in %install
b. %clean
c. %defattr(-,root,root)
2. Github doesn't provide http connection,
URL:http://github.com/felipec/git-remote-hg
should be (more accurate)
URL:https://github.com/felipec/git-remote-hg
3. /usr should be %{_prefix}
4. Should preserve the timestamp:
sed -i -e 's|\tinstall|\tinstall -p|' Makefile
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561 --- Comment #4 from Ondrej Oprala oopr...@redhat.com --- All corrected. Thank you for your review. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561
--- Comment #5 from Florian der-flo Lehner d...@der-flo.net ---
Package Review
==
Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed
= MUST items =
Generic:
[x]: Package is licensed with an open-source compatible license and meets
other legal requirements as defined in the legal section of Packaging
Guidelines.
[!]: If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s)
for the package is included in %doc.
[x]: License field in the package spec file matches the actual license.
Note: Checking patched sources after %prep for licenses. Licenses found:
GPL (v2 or later), Unknown or generated. 1 files have unknown
license. Detailed output of licensecheck in /home/flo/review/561-git-
remote-hg/licensecheck.txt
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
beginning of %install.
Note: rm -rf %{buildroot} present but not required
--- please remove the rm -rf %{buildroot} in the %install-section
[x]: Sources contain only permissible code or content.
[-]: Each %files section contains %defattr if rpm 4.4
Note: %defattr present but not needed
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[!]: Package complies to the Packaging Guidelines
--- There are some issuses
[x]: Package successfully compiles and builds into binary rpms on at least one
supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
%{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
(~1MB) or number of files.
Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local
= SHOULD items =
Generic:
[!]: Uses parallel make %{?_smp_mflags} macro.
[!]: Package has no %clean section with rm -rf %{buildroot} (or
$RPM_BUILD_ROOT)
Note: %clean present but not required
[-]: If the source package does not include license text(s) as a separate file
from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
architectures.
--- http://koji.fedoraproject.org/koji/taskinfo?taskID=7061592
[-]: %check is present and all tests pass.
[!]: Packages should try to preserve timestamps of original installed files.
--- Looking at the Makefile the timestamps are not preserved.
The -p flag of install is missing
See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561
Björn besser82 Esser bjoern.es...@gmail.com changed:
What|Removed |Added
CC||bjoern.es...@gmail.com
--- Comment #6 from Björn besser82 Esser bjoern.es...@gmail.com ---
(In reply to Christopher Meng from comment #3)
(In reply to Ondrej Oprala from comment #2)
What do you mean? There's 'BuildArch: noarch' in the spec.
Yes, I know that, but if you mark it as noarch, no need to write:
%global debug_package %{nil}
Why? I cannot see any reason for this… What you refer to is needed for
header-only libs, when there is an arched mainpkg, but just a noarch-subpkg is
really build. e.g. when having the main-pkg arched, because of running the
testsuite on all avail arches during koji-build…
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561
--- Comment #7 from Ondrej Oprala oopr...@redhat.com ---
(In reply to Florian der-flo Lehner from comment #5)
[!]: Uses parallel make %{?_smp_mflags} macro.
I don't think that's appropriate here, given that we only install 3 files (and
build only 1).
[!]: Packages should try to preserve timestamps of original installed files.
--- Looking at the Makefile the timestamps are not preserved.
The -p flag of install is missing
install-related parts of the makefile are now substituted in %prep.
Thanks for the review Florian. The links in my initial comment now point to a
corrected spec and srpm.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561
--- Comment #8 from Christopher Meng i...@cicku.me ---
(In reply to Florian der-flo Lehner from comment #5)
Stop leaving any review template if this bug is already assigned to someone(and
being reviewed).
(In reply to Björn besser82 Esser from comment #6)
Why? I cannot see any reason for this… What you refer to is needed for
header-only libs, when there is an arched mainpkg, but just a noarch-subpkg
is really build. e.g. when having the main-pkg arched, because of running
the testsuite on all avail arches during koji-build…
Who are you talking to?
Anyway(who is the reviewer?), here are the
Remaining issues:
1. No need to pass any arguments to make in %build, as the location variables
should be set in %install.
2. You should add %check with make test. It's a good habit for a package with
testsuite. If the test can't be performed, write down the reason in comment.
3. #!/usr/bin/env python2
sed the matchword with %{__python2} macro substituted.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561 --- Comment #9 from Ondrej Oprala oopr...@redhat.com --- All fixed now. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561
--- Comment #10 from Christopher Meng i...@cicku.me ---
Package Review
==
Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
= MUST items =
Generic:
[x]: Package is licensed with an open-source compatible license and meets
other legal requirements as defined in the legal section of Packaging
Guidelines.
[x]: License field in the package spec file matches the actual license.
Note: Checking patched sources after %prep for licenses. Licenses found:
GPL (v2 or later), Unknown or generated. 1 files have unknown
license. Detailed output of licensecheck:
GPL (v2 or later)
-
git-remote-hg-0.2/test/sharness.sh
Unknown or generated
git-remote-hg-0.2/test/test-lib.sh
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
(~1MB) or number of files.
Note: Documentation size is 30720 bytes in 1 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s)
for the package is included in %doc.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
%{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
= SHOULD items =
Generic:
[-]: Uses parallel make %{?_smp_mflags} macro.
[x]: If the source package does not include license text(s) as a separate file
from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
architectures.
[!]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
$RPM_BUILD_ROOT)
[x]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.
= EXTRA items =
Generic:
[x]: Rpmlint is run on all installed packages.
Note: There are rpmlint messages (see
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561
--- Comment #11 from Ondrej Oprala oopr...@redhat.com ---
1. Your hack is pointless, as now the interpreter is %{__python2}.
Are you sure? The substitution is ok when I extract the rpm.
2. Missing hg-git test, please BR hg-git packaged by myself and try again.
I updated the files, but all tests seem to fail upon installing hg-git. Not
just in rpmbuild, but just by plain 'make test' as well. Are you also
encountering this?
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561
--- Comment #12 from Michael Schwendt bugs.mich...@gmx.net ---
if you mark it as noarch, no need to write:
%global debug_package %{nil}
in the spec.
Wrong.
https://fedoraproject.org/wiki/Packaging:Debuginfo#Missing_debuginfo_packages
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561
--- Comment #13 from Björn besser82 Esser bjoern.es...@gmail.com ---
(In reply to Ondrej Oprala from comment #11)
1. Your hack is pointless, as now the interpreter is %{__python2}.
Are you sure? The substitution is ok when I extract the rpm.
You will need to add `BuildRequires: python2-devel` to have that macro
correctly expanded inside the mock-chroot. ;)
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561 --- Comment #14 from Ondrej Oprala oopr...@redhat.com --- You will need to add `BuildRequires: python2-devel` to have that macro correctly expanded inside the mock-chroot. ;) Aah, thanks for the tip :) -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561
Christopher Meng i...@cicku.me changed:
What|Removed |Added
Flags|fedora-review? |fedora-review+
--- Comment #15 from Christopher Meng i...@cicku.me ---
PACKAGE APPROVED.
(In reply to Michael Schwendt from comment #12)
if you mark it as noarch, no need to write:
%global debug_package %{nil}
in the spec.
Wrong.
https://fedoraproject.org/wiki/Packaging:Debuginfo#Missing_debuginfo_packages
I still don't understand why I'm wrong here.
I meant no matter that line is put or not, the debuginfo won't be generated
when noarch for this package. If I understand correctly, every python noarch
package in the repo should be added this line(%global debug_package %{nil})
also?
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561 Ondrej Oprala oopr...@redhat.com changed: What|Removed |Added Flags||fedora-cvs? --- Comment #16 from Ondrej Oprala oopr...@redhat.com --- New Package SCM Request === Package Name: git-remote-hg Short Description: Mercurial wrapper for git Upstream URL: https://github.com/felipec/git-remote-hg Owners: ooprala Branches: InitialCC: ovasik -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561 --- Comment #17 from Michael Schwendt bugs.mich...@gmx.net --- I still don't understand why I'm wrong here. BuildArch: noarch _implicitly_ disables generation of -debuginfo packages. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561
--- Comment #18 from Christopher Meng i...@cicku.me ---
(In reply to Michael Schwendt from comment #17)
I still don't understand why I'm wrong here.
BuildArch: noarch _implicitly_ disables generation of -debuginfo packages.
Ondrej wrote down both BA: noarch and %global debug_package %{nil} in the spec
first, I then asked him to drop the latter as BA is enough to disable
debuginfo.
Am I missing something?
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1111561] Review Request: git-remote-hg - mercurial wrapper for git
https://bugzilla.redhat.com/show_bug.cgi?id=561 --- Comment #19 from Michael Schwendt bugs.mich...@gmx.net --- Yeah, sorry, a proper %changelog entry would have helped here to understand what has been changed since the initial spec file. I misread the no need to write in your comment 3 as you need to write paired with the confusion in the first few comments of this ticket. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
