[Bug 976886] Review Request: python-ase - Atomic Simulation Environment

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=976886

--- Comment #10 from Marcin.Dulak@xxxxxxxxx ---
Spec URL:
http://marcindulak.fedorapeople.org/packages/python-ase/v04/python-ase.spec
SRPM URL:
http://marcindulak.fedorapeople.org/packages/python-ase/v04/python-ase-3.7.1.3184-4.fc20.src.rpm

(In reply to Björn Esser from comment #9)
> (In reply to Marcin.Dulak from comment #5)
> > should build on el5 now. I assume this will happen on EPEL, so numpy will
> > be available?
> 
> Yes, numpy is in EPEL for el5 and offered from "official" Repo on el6+
> 
> > I see also that the macros %dist, %rhel, %el5 are provided
> > on el5 by buildsys-macros, and this package is not installed by default,
> > so i tried to make some workarounds - is buildsys-macros installed on EPEL?
> 
> # buildsys-macros on el5 provides %%dist, %%rhel, %%el5
> %{!?dist: %global el5 1}
> %{!?dist: %global rhel 5}
> %{!?dist: %global dist .el5}
> 
> Not needed: `buildsys-macros` are avail during koji-build, so there's no
> need for a hack.  On el5 there should be a group called `RPM Development
> Tools` or similar which pulles `buildsys-macros`, rpmbuild and other needed
> packages.  Simply installing `rpm-build` isn't enough.  On Fedora it's the
> same, btw, `rpm-build` is not pulling `redhat-rpm-config`, which is needed
> for some proper macro-defines, too.

OK, removed

> 
> # macros undefined on el5
> %if 0%{?el5}
> %{!?python_sitelib: %global python_sitelib %(%{__python} -c "from
> distutils.sysconfig import get_python_lib; print(get_python_lib())")}
> %{!?python_sitearch: %global python_sitearch %(%{__python} -c "from
> distutils.sysconfig import get_python_lib; print(get_python_lib(1))")}
> %endif
> 
> There's no need for explicit conditional on el5 here.  %python_sitearch is
> not needed, too.  So just keeping the plain python_sitelib-expansion will do.
> 

OK

> #####
> 
>  | %if 0%{?rhel} && 0%{?rhel} <= 6
>  | Requires: pygtk2
>  | %endif
> 
> Just using `%if 0%{?rhel} <= 6` should be enough and will improve
> readability.

we need it. 0%{?rhel} <= 6 alone would be true on Fedora.

> 
> #####
> 
> > %fdupes is included now, but i see that %fdupes does not work on empty files:
> > rpm --eval %fdupes
> 
> So you can drop fdupes.  Then it will be no use.

OK, removed

> 
> > so only pyc<->pyo are hard-linked.
> 
> These get hard-linked by rpmbuilb auto-bytecompile.
> 
> #####
> 
>  | desktop-file-install \
>  | --dir $RPM_BUILD_ROOT%{_datadir}/applications \
>  | %if 0%{?el5}
>  | --vendor "" \
>  | %endif
>  | %{SOURCE1}
> 
> You can use expansion here, instead of conditional: `%{?el5:--vendor "" \}`

i got "unclosed macro or bad line continuation", so i use --vendor=="ase".

> 
> #####
> 
>  | %if 0%{?fedora} || 0%{?rhel} >= 6
>  | %{python_sitelib}/*.egg-info
>  | %endif
> 
> Same as above: `%{?!el5:%{python_sitelib}/*.egg-info}`

OK

> 
> #####
> 
> Is there a real need for an empty dir %{_datadir}/%{name} ?

removed.

> 
> #####
> 
> Package is fine, besides the mentioned above.
> 
> #####
> 
> 
> Package Review
> ==============
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> [ ] = Manual review needed
> 
> 
> Issues:
> =======
> - Package installs properly.
>   Note: Installation errors (see attachment)
>   See: https://fedoraproject.org/wiki/Packaging:Guidelines
> 
>   ---> ignored: texlive-dvipng-bin-svn is broken on rawhide, see:
>        https://lists.fedoraproject.org/pipermail/devel/2013-June/184366.html
> 
>        Install went fine on F19
> 
> - update-desktop-database is invoked when required
>   Note: desktop file(s) in python-ase
>   See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
> 
>   ---> false positive: no mime-type defined in desktop-file
> 
> - Package installs a %{name}.desktop using desktop-file-install if there is
>   such a file.
>   See: http://fedoraproject.org/wiki/Packaging/Guidelines#desktop
> 
>   ---> false positive: desktop-file-install is invoked correctly.
> 
> 
> ===== 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]: Package contains no bundled libraries without FPC exception.
> [x]: Changelog in prescribed format.
> [x]: Sources contain only permissible code or content.
> [-]: Development files must be in a -devel package
> [x]: Package requires other packages for directories it uses.
> [x]: Package uses nothing in %doc for runtime.
> [x]: Package is not known to require ExcludeArch.
> [x]: Package complies to the Packaging Guidelines
> [x]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses found:
>      "MIT/X11 (BSD like)", "LGPL (v2.1 or later)", "Unknown or generated".
> 402
>      files have unknown license. Detailed output of licensecheck in
>      /home/bjoern.esser/fedora/review/976886-python-ase/licensecheck.txt
> 
>      ---> License-tag is fine
> 
> [x]: Package consistently uses macro is (instead of hard-coded directory
>      names).
> [x]: If the package is under multiple licenses, the licensing breakdown must
>      be documented in the spec.
> [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]: Package must own all directories that it creates.
> [x]: Package does not own files or directories owned by other packages.
> [x]: Requires correct, justified where necessary.
> [x]: Spec file is legible and written in American English.
> [-]: Package contains systemd file(s) if in need.
> [-]: Large documentation must go in a -doc subpackage.
>      Note: Documentation size is 163840 bytes in 8 files.
> [x]: All build dependencies are listed in BuildRequires, except for any that
>      are listed in the exceptions section of Packaging Guidelines.
> [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
>      beginning of %install.
> [x]: Each %files section contains %defattr if rpm < 4.4
> [x]: Macros in Summary, %description expandable at SRPM build time.
> [x]: Package contains desktop file if it is a GUI application.
> [x]: Package does not contain duplicates in %files.
> [x]: Permissions on files are set properly.
> [x]: Fully versioned dependency in subpackages, if present.
> [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 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
> [x]: Package successfully compiles and builds into binary rpms on at least
> one
>      supported primary architecture.
> [x]: Rpmlint is run on all rpms the build produces.
>      Note: There are rpmlint messages (see attachment).
> 
> Python:
> [x]: Python eggs must not download any dependencies during the build process.
> [x]: A package which is used by another package via an egg interface should
>      provide egg info.
> [x]: Package meets the Packaging Guidelines::Python
> [x]: Package contains BR: python2-devel or python3-devel
> [x]: Binary eggs must be removed in %prep
> 
> ===== SHOULD items =====
> 
> Generic:
> [-]: 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]: Patches link to upstream bugs/comments/lists or are otherwise justified.
> [-]: 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.
> [x]: %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.
> [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
> [x]: SourceX tarball generation or download is documented.
> [x]: SourceX is a working URL.
> [x]: Spec use %global instead of %define.
> 
> ===== EXTRA items =====
> 
> Generic:
> [x]: Rpmlint is run on all installed packages.
>      Note: Mock build failed
> 
>      ---> false positive: install fails on rawhide, see above.
>           Everything is fine on F19.  rpmlint results are from there.
> 
> [x]: Large data in /usr/share should live in a noarch subpackage if package
> is
>      arched.
> [x]: Spec file according to URL is the same as in SRPM.
> 
> 
> 
> Rpmlint
> -------
> Checking: python-ase-3.7.1.3184-3.fc19.noarch.rpm
> python-ase.noarch: E: explicit-lib-dependency python-matplotlib
> python-ase.noarch: W: no-manual-page-for-binary ASE2ase
> python-ase.noarch: W: no-manual-page-for-binary trajectoryinfo
> python-ase.noarch: W: no-manual-page-for-binary testase
> python-ase.noarch: W: no-manual-page-for-binary ag
> python-ase.noarch: W: no-manual-page-for-binary foldtrajectory
> python-ase.noarch: W: no-manual-page-for-binary ase
> python-ase.noarch: W: no-manual-page-for-binary asec
> 1 packages and 0 specfiles checked; 1 errors, 7 warnings.
> 
> 
> 
> 
> Rpmlint (installed packages)
> ----------------------------
> # rpmlint python-ase
> python-ase.noarch: E: explicit-lib-dependency python-matplotlib
> python-ase.noarch: W: no-manual-page-for-binary ASE2ase
> python-ase.noarch: W: no-manual-page-for-binary trajectoryinfo
> python-ase.noarch: W: no-manual-page-for-binary testase
> python-ase.noarch: W: no-manual-page-for-binary ag
> python-ase.noarch: W: no-manual-page-for-binary foldtrajectory
> python-ase.noarch: W: no-manual-page-for-binary ase
> python-ase.noarch: W: no-manual-page-for-binary asec
> 1 packages and 0 specfiles checked; 1 errors, 7 warnings.
> # echo 'rpmlint-done:'
> 
> 
> 
> Requires
> --------
> python-ase (rpmlib, GLIBC filtered):
>     /usr/bin/python
>     python(abi)
>     python-matplotlib
> 
> 
> 
> Provides
> --------
> python-ase:
>     python-ase
> 
> 
> 
> Source checksums
> ----------------
> https://wiki.fysik.dtu.dk/ase-files/python-ase-3.7.1.3184.tar.gz :
>   CHECKSUM(SHA256) this package     :
> de95f327ef93062bec5d564c41faf71784fb134a1b87fe02b25ea15fbc001674
>   CHECKSUM(SHA256) upstream package :
> de95f327ef93062bec5d564c41faf71784fb134a1b87fe02b25ea15fbc001674
> https://wiki.fysik.dtu.dk/ase-files/ase-gui.desktop :
>   CHECKSUM(SHA256) this package     :
> 6d685a94f4d034a7ee2ed12057c5e44b3ed7d5e8c2b1fb4d2bb0104c214cd3a4
>   CHECKSUM(SHA256) upstream package :
> 6d685a94f4d034a7ee2ed12057c5e44b3ed7d5e8c2b1fb4d2bb0104c214cd3a4
> 
> 
> Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
> Buildroot used: fedora-rawhide-x86_64
> Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 976886
> 
> #####
> 
> Please fix these small issues inside spec, and I'll grant review.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=huTN8r7NZC&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]