Spec URL: http://ooprala.fedorapeople.org/git-remote-hg-0.2-1.fc19.src.rpm SRPM URL: http://ooprala.fedorapeople.org/git-remote-hg.spec Description: git-remote-hg used to be a part of the git package - it is a simple wrapper to work with mercurial repositories entirely via the git toolchain. However, since git-2.0.0 (current version in Fedora Rawhide), it has split into a separate project and got unpackaged from git. I would like to package it separately for it not to disappear from future fedora releases. Fedora Account System Username: ooprala
Why not mark it as noarch?
What do you mean? There's 'BuildArch: noarch' in the spec.
(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
All corrected. Thank you for your review.
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/1111561-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 [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]: Dist tag is present (not strictly required in GL). [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [!]: SourceX is a working URL. ---> please append '#/%{name}-%{version}.tar.gz' to Source0. A file named v%{version}.tar.gz might be anything and might cause troubles because of it's non-unique naming-scheme. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on all installed packages. Note: No rpmlint messages. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: git-remote-hg-0.2-1.fc21.noarch.rpm git-remote-hg-0.2-1.fc21.src.rpm git-remote-hg.src:26: W: rpm-buildroot-usage %build make HOME='/usr' DESTDIR=%{buildroot} #doc 2 packages and 0 specfiles checked; 0 errors, 1 warnings. Rpmlint (installed packages) ---------------------------- # rpmlint git-remote-hg 1 packages and 0 specfiles checked; 0 errors, 0 warnings. # echo 'rpmlint-done:' Requires -------- git-remote-hg (rpmlib, GLIBC filtered): /usr/bin/env bzr git python Provides -------- git-remote-hg: git-remote-hg Source checksums ---------------- https://github.com/felipec/git-remote-hg/archive/v0.2.tar.gz : CHECKSUM(SHA256) this package : 1b122cb925e7ce90ddba6840624d8a26d309b1671bddfe1408e945176c8b9242 CHECKSUM(SHA256) upstream package : 1b122cb925e7ce90ddba6840624d8a26d309b1671bddfe1408e945176c8b9242 Generated by fedora-review 0.5.1 (bb9bf27) last change: 2013-12-13 Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 1111561 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api Disabled plugins: Java, C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG ===== Solution ===== NOT approved. Please fix those issues and I' ll take another review.
(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…
(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.
(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.
All fixed now.
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 attachment). Rpmlint ------- Checking: git-remote-hg-0.2-1.fc21.noarch.rpm git-remote-hg-0.2-1.fc21.src.rpm git-remote-hg.noarch: E: wrong-script-interpreter /usr/bin/git-remote-hg %{__python2} 2 packages and 0 specfiles checked; 1 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- # rpmlint git-remote-hg git-remote-hg.noarch: E: wrong-script-interpreter /usr/bin/git-remote-hg %{__python2} 1 packages and 0 specfiles checked; 1 errors, 0 warnings. # echo 'rpmlint-done:' Requires -------- git-remote-hg (rpmlib, GLIBC filtered): git mercurial python Provides -------- git-remote-hg: git-remote-hg Source checksums ---------------- https://github.com/felipec/git-remote-hg/archive/v0.2.tar.gz#/git-remote-hg-0.2.tar.gz : CHECKSUM(SHA256) this package : 1b122cb925e7ce90ddba6840624d8a26d309b1671bddfe1408e945176c8b9242 CHECKSUM(SHA256) upstream package : 1b122cb925e7ce90ddba6840624d8a26d309b1671bddfe1408e945176c8b9242 Generated by fedora-review 0.5.1 (bb9bf27) last change: 2013-12-13 Command line :/bin/fedora-review -rvn git-remote-hg-0.2-1.fc19.src.rpm Buildroot used: fedora-rawhide-i386 Active plugins: Generic, Shell-api Disabled plugins: Java, C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG ------------------------------- 1. Your hack is pointless, as now the interpreter is "%{__python2}". 2. Missing hg-git test, please BR hg-git packaged by myself and try again.
> 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?
> 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
(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 will need to add `BuildRequires: python2-devel` to have that macro > correctly expanded inside the mock-chroot. ;) Aah, thanks for the tip :)
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?
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
> I still don't understand why I'm wrong here. "BuildArch: noarch" _implicitly_ disables generation of -debuginfo packages.
(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?
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.
Git done (by process-git-requests).
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.