Bug 1478231

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- All build dependencies are listed in BuildRequires, except for any that
  are listed in the exceptions section of Packaging Guidelines.
  Note: These BR are not needed: sed
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2


===== 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.
[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Apache (v2.0)", "Unknown or generated", "MIT/X11 (BSD like)",
     "BSD (3 clause) LGPL", "BSD (unspecified)", "*No copyright* BSD
     (unspecified)", "BSD (3 clause)", "LGPL (v2.1 or later)". 265 files
     have unknown license. licensecheck.txt is attached.
     Note: License tag of python*-conda should be "BSD and ASL 2.0 and
     LGPLv2+ and MIT", and multiple licensing description should be provided.
[x]: License file installed when any subpackage combination is installed.
[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.
[!]: Package consistently uses macros (instead of hard-coded directory
     names).
     Note: Many occurrences of "conda" can be replaced with %{name}.
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[!]: Package obeys FHS, except libexecdir and /usr/target.
     Note: Fedora does not allow new directories directly under / or /usr
     without FPC approval.
     Note: /usr/condarc.d is provided.
     Note: %_bindir/{de,}activate scripts are used for being sourced, but not
     executed. Placing them to %_datadir seems more proper.
[-]: 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.
[-]: 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 204800 bytes in 4 files.
[-]: 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 %license.
[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]: 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]: Dist tag is present.
[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 does not use a name that already exists.
[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

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:
[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]: Fully versioned dependency in subpackages if applicable.
[?]: Package functions as described.
     Note: Re-examine this after filesystem layout revised.
[x]: Latest version is packaged.
     Note: 4.3.25 is tagged but not set as a release.
[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.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Spec use %global instead of %define unless justified.
[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]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.
[!]: Descriptions of some subpackages are the same. Description of activate
     subpackage is bad.


Rpmlint
-------
Checking: conda-4.3.24-1.fc27.noarch.rpm
          conda-activate-4.3.24-1.fc27.noarch.rpm
          python2-conda-4.3.24-1.fc27.noarch.rpm
          python3-conda-4.3.24-1.fc27.noarch.rpm
          conda-4.3.24-1.fc27.src.rpm
conda.noarch: W: no-documentation
conda.noarch: W: dangling-relative-symlink /usr/bin/conda conda-3
conda.noarch: W: no-manual-page-for-binary conda
conda-activate.noarch: W: no-documentation
conda-activate.noarch: W: no-manual-page-for-binary activate
conda-activate.noarch: W: no-manual-page-for-binary deactivate
python2-conda.noarch: W: hidden-file-or-dir /usr/lib/python2.7/site-packages/conda/.version
python2-conda.noarch: W: no-manual-page-for-binary conda-2
python2-conda.noarch: W: no-manual-page-for-binary conda-2.7
python2-conda.noarch: W: no-manual-page-for-binary conda-env-2
python2-conda.noarch: W: no-manual-page-for-binary conda-env-2.7
python2-conda.noarch: W: non-standard-dir-in-usr condarc.d
python3-conda.noarch: W: hidden-file-or-dir /usr/lib/python3.6/site-packages/conda/.version
python3-conda.noarch: W: no-manual-page-for-binary conda
python3-conda.noarch: W: no-manual-page-for-binary conda-3
python3-conda.noarch: W: no-manual-page-for-binary conda-3.6
python3-conda.noarch: W: no-manual-page-for-binary conda-env
python3-conda.noarch: W: no-manual-page-for-binary conda-env-3
python3-conda.noarch: W: no-manual-page-for-binary conda-env-3.6
python3-conda.noarch: W: non-standard-dir-in-usr condarc.d
conda.src:11: W: macro-in-comment %{name}
conda.src:11: W: macro-in-comment %{name}
conda.src:11: W: macro-in-comment %{version}
5 packages and 0 specfiles checked; 0 errors, 23 warnings.




Rpmlint (installed packages)
----------------------------
python3-conda.noarch: W: hidden-file-or-dir /usr/lib/python3.6/site-packages/conda/.version
python3-conda.noarch: W: no-manual-page-for-binary conda
python3-conda.noarch: W: no-manual-page-for-binary conda-3
python3-conda.noarch: W: no-manual-page-for-binary conda-3.6
python3-conda.noarch: W: no-manual-page-for-binary conda-env
python3-conda.noarch: W: no-manual-page-for-binary conda-env-3
python3-conda.noarch: W: no-manual-page-for-binary conda-env-3.6
python3-conda.noarch: W: non-standard-dir-in-usr condarc.d
python2-conda.noarch: W: hidden-file-or-dir /usr/lib/python2.7/site-packages/conda/.version
python2-conda.noarch: W: no-manual-page-for-binary conda-2
python2-conda.noarch: W: no-manual-page-for-binary conda-2.7
python2-conda.noarch: W: no-manual-page-for-binary conda-env-2
python2-conda.noarch: W: no-manual-page-for-binary conda-env-2.7
python2-conda.noarch: W: non-standard-dir-in-usr condarc.d
conda.noarch: W: no-documentation
conda.noarch: W: dangling-relative-symlink /usr/bin/conda conda-3
conda.noarch: W: no-manual-page-for-binary conda
conda-activate.noarch: W: no-documentation
conda-activate.noarch: W: no-manual-page-for-binary activate
conda-activate.noarch: W: no-manual-page-for-binary deactivate
4 packages and 0 specfiles checked; 0 errors, 20 warnings.



Requires
--------
python3-conda (rpmlib, GLIBC filtered):
    /usr/bin/python3
    python(abi)
    python3-crypto
    python3-mock
    python3-pycosat
    python3-requests
    python3-responses
    python3-ruamel-yaml
    python3-yaml

python2-conda (rpmlib, GLIBC filtered):
    /usr/bin/python2
    python(abi)
    python2-crypto
    python2-mock
    python2-pycosat
    python2-requests
    python2-responses
    python2-ruamel-yaml
    python2-yaml

conda (rpmlib, GLIBC filtered):
    python3-conda

conda-activate (rpmlib, GLIBC filtered):
    /bin/bash
    conda



Provides
--------
python3-conda:
    python3-conda
    python3.6dist(conda)
    python3dist(conda)

python2-conda:
    python-conda
    python2-conda
    python2.7dist(conda)
    python2dist(conda)

conda:
    conda

conda-activate:
    conda-activate



Source checksums
----------------
https://github.com/conda/conda/archive/4.3.24/conda-4.3.24.tar.gz :
  CHECKSUM(SHA256) this package     : 9f2525fcbc7f8cf6deb67e3e69c16d0f5cd62427f0ab7b5a53776fb12f5f84d7
  CHECKSUM(SHA256) upstream package : 9f2525fcbc7f8cf6deb67e3e69c16d0f5cd62427f0ab7b5a53776fb12f5f84d7


Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -b 1478231
Buildroot used: fedora-rawhide-x86_64
Active plugins: Python, Generic, Shell-api
Disabled plugins: Java, C/C++, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Created attachment 1315120 [details]
licensecheck of conda

Created attachment 1315375 [details]
patch to try to support epel7

It would be nice if we could support EPEL7.  This patch helps.  Still missing some deps though.

See https://bugzilla.redhat.com/show_bug.cgi?id=1483123 for dep.

Is there much need to package a Python 2 executable (on Fedora at least)? You can create envs for either version regardless of the root with the standalone builds.

Thanks you for taking the review. Being so through has been very helpful.
I fixed most stuff (comments below), except some parts I disagree with.

> Issues:
> =======
> - All build dependencies are listed in BuildRequires, except for any that
>   are listed in the exceptions section of Packaging Guidelines.
>   Note: These BR are not needed: sed
>   See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2
fedora-review is wrong here. If you follow the link it shows, actually gcc
is *not* guaranteed to be present and the list of exceptions has been removed
a while back.

> [!]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses
>      found: "Apache (v2.0)", "Unknown or generated", "MIT/X11 (BSD like)",
>      "BSD (3 clause) LGPL", "BSD (unspecified)", "*No copyright* BSD
>      (unspecified)", "BSD (3 clause)", "LGPL (v2.1 or later)". 265 files
>      have unknown license. licensecheck.txt is attached.
>      Note: License tag of python*-conda should be "BSD and ASL 2.0 and
>      LGPLv2+ and MIT", and multiple licensing description should be provided.
Good catch! I updated the License field and added a bunch of
Provides: bundles(...) fields.

I also looked into unbundling, but the "vendored" copies have different version
then the ones in fedora, and have local modifications. So I think in this
case it just isn't worth the risk of breaking stuff.

> [!]: Package obeys FHS, except libexecdir and /usr/target.
>      Note: Fedora does not allow new directories directly under / or /usr
>      without FPC approval.
>      Note: /usr/condarc.d is provided.
Right. I patched the python code to look in /usr/share/conda/condarc.d, and
moved the file we create to that directory.

>      Note: %_bindir/{de,}activate scripts are used for being sourced, but not
>      executed. Placing them to %_datadir seems more proper.
Even if they are sourced, $PATH is used, so it's more convenient to have them
in /usr/bin ('source activate' works, and the full path would have to be given
otherwise).

> [?]: Package functions as described.
>      Note: Re-examine this after filesystem layout revised.

> [!]: Description of activate
>      subpackage is bad.
I fixed the description of activate subpackage to explain what it does.

@Orion: I carried that package over from your spec file. Is there a particular
reason to make this a seperate subpackage?

> Descriptions of some subpackages are the same. 
I don't think there's any prohibition against having the same package desctiptions.
With python2-/python3- packages it's extremely common to have the exact same
text, and this doesn't lead to any confusion because the package name is
enough to understand that one is for python2 and the other for python3.
And I think it's pretty clear without stating it explicitly, that conda is
the main package and python[23]-conda are the python modules backing it.

[!]: Package consistently uses macros (instead of hard-coded directory
     names).
     Note: Many occurrences of "conda" can be replaced with %{name}.
There's no reason to prefer %{name} to conda. It's longer, harder to type,
and harder to read.

There are two reasons when a macro should be used:
- the string it replaces is long or awkward to type (e.g. because of weird casing or whatever),
- the string varies, e.g. between architectures, or distro releases.
For strings which are both shorter then the macro, and are unlikely to ever change,
macros are just obfuscation.

The guidelines talk about hardcoded directory names. E.g. %_libdir is necessary
because it is /usr/lib64 sometimes, and /usr/lib othertimes. But most of the
macro usage in spec files is pointless self-flagellation. ;)

(In reply to Orion Poplawski from comment #3)
> Created attachment 1315375 [details]
> patch to try to support epel7
Added.

(In reply to Elliott Sales de Andrade from comment #5)
> Is there much need to package a Python 2 executable (on Fedora at least)?
Python 2 versions create python2 environments. It's also possible to do
it through other means (e.g. by calling 'python2 /usr/bin/conda create ...'),
but having the versioned executables makes it easier.

Spec URL: https://in.waw.pl/~zbyszek/fedora/conda.spec
SRPM URL: https://in.waw.pl/~zbyszek/fedora/conda-4.3.24-1.fc27.src.rpm

%changelog
- Add all licenses to the License tag
- Add Provides: bundled(...) for all the "vendored" dependencies
- Update descriptions and simplify the spec file a bit
- Move condarc.d directory under /usr/share/conda

With miniconda/anaconda, you do `conda create python=2 ...`; I've never had any need for the root to be in a specific version to create environments of it. Why would a system install be any different?

Yeah, you're right. Let's drop it.

Spec URL: https://in.waw.pl/~zbyszek/fedora/conda.spec
SRPM URL: https://in.waw.pl/~zbyszek/fedora/conda-4.3.24-3.fc27.src.rpm

%changelog
- Install just one version of the executables (python 2 or 3)

All patches should have an upstream bug link or comment 
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

(In reply to Zbigniew Jędrzejewski-Szmek from comment #8)
> SRPM URL: https://in.waw.pl/~zbyszek/fedora/conda-4.3.24-3.fc27.src.rpm
Should be https://in.waw.pl/~zbyszek/fedora/conda-4.3.24-3.fc28.src.rpm

[x]: Package functions as described.

This packages is approved by cheeselee.

Thanks!

(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/conda

conda-4.3.24-3.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-0a4ad90d7f

conda-4.3.24-3.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.