Bug 1687178 - Review Request: python-apprise - Push Notifications that work with just about every platform!
Summary: Review Request: python-apprise - Push Notifications that work with just about...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Robert-André Mauchin 🐧
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-03-10 17:31 UTC by Chris Caron
Modified: 2020-05-19 10:48 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-05-19 10:48:04 UTC
Type: ---
zebob.m: fedora-review+


Attachments (Terms of Use)
Patch to allow apprise to run unit-tests for 100% coverage in epel7 (1.65 KB, patch)
2019-03-13 00:03 UTC, Chris Caron
no flags Details | Diff
shebang patch just created for this release to handle rpmlint issue (345 bytes, patch)
2019-03-14 00:03 UTC, Chris Caron
no flags Details | Diff
man page added to help with rpmlint warnings (3.46 KB, text/troff)
2019-03-14 23:48 UTC, Chris Caron
no flags Details

Description Chris Caron 2019-03-10 17:31:53 UTC
Spec URL: https://copr-be.cloud.fedoraproject.org/results/lead2gold/apprise/fedora-rawhide-x86_64/00866155-python-apprise/python-apprise.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/lead2gold/apprise/fedora-rawhide-x86_64/00866155-python-apprise/python-apprise-0.7.3-1.fc31.nuxref.src.rpm

Description:
Apprise allows you to send a notification to almost all of the most popular notification services available to us today such as: Telegram, Pushbullet, Slack, Twitter, etc.

It's a Python based notification library and additionally contains a supporting CLI for system administrators to use too. (https://github.com/caronc/apprise).

I was able to build the RPM without problems on epel7, f28, f29, and rawhide using koji.

Fedora Account System Username: lead2gold

Comment 1 Chris Caron 2019-03-10 19:23:24 UTC
The wiki mentions passing along working koji (--scratch) builds.  No problem, below is all passing builds:
* epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=33369676
* f28: https://koji.fedoraproject.org/koji/taskinfo?taskID=33369716
* f29: https://koji.fedoraproject.org/koji/taskinfo?taskID=33369791
* rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=33369860

Comment 2 Artur Iwicki 2019-03-11 08:16:43 UTC
>Group:          Development/Languages
>BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
>...
>Group: Development/Utils
>...
>%clean
Not used in Fedora. Drop 'em.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_sections

>URL:            http://github.com/caronc/apprise
I'd change this to https.

>Source0:        https://github.com/caronc/apprise/archive/v%{version}/apprise-%{version}.tar.gz
You can reuse the %{URL} here to make the line shorter.

>LANG=en_US.utf8 PYTHONPATH=%{buildroot}%{python2_sitelib} coverage2 run ./setup.py test
I believe this may require you to add the en_US langpack via BuildRequires. See:
https://fedoraproject.org/wiki/Changes/Remove_glibc-langpacks-all_from_buildroot

>%defattr(-,root,root,-)
Not needed. These are the default values.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_permissions

Comment 3 Robert-André Mauchin 🐧 2019-03-11 19:54:39 UTC
Also LICENSE files must be included with %license not %doc

Comment 4 Robert-André Mauchin 🐧 2019-03-11 19:55:58 UTC
Also 0.7.4 was released this past day.

Comment 5 Chris Caron 2019-03-11 23:02:30 UTC
Thank you guys for all your feedback.  It was also suggested on the mailing lists to not use flake8 as it's to harsh to fail a %test because of a pep8 issue.  I couldn't agree with this more, so i dropped this to just be py.test references instead.  I'll satisfy my own PEP8 OCD with travisCI and not with RPMS.

I should add that i'm also the developer of this product.  So yes i realize i pushed a new version out but supplied you with the last one.  It was kind of happening at the same time.

I addressed all of the comments you identified above (including the new version reference):

Updated Spec:
https://copr-be.cloud.fedoraproject.org/results/lead2gold/apprise/fedora-rawhide-x86_64/00866973-python-apprise/python-apprise.spec

Here is the Koji output:
epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=33411608
f28: https://koji.fedoraproject.org/koji/taskinfo?taskID=33411610
f29: https://koji.fedoraproject.org/koji/taskinfo?taskID=33411612
f30: https://koji.fedoraproject.org/koji/taskinfo?taskID=33411614
rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=33411616

Copr:
https://copr.fedorainfracloud.org/coprs/lead2gold/apprise/build/866973/


On a side note; the license was recently changed (About a month or so ago) from GPLv3 to MIT.  I didn't realize i didn't update this on the header of my SPEC file.  I'll update this on the next build.

Thank you guys again for all of your feedback!

Comment 6 Robert-André Mauchin 🐧 2019-03-11 23:13:38 UTC
Is LANG=en_US.utf8 mandatory? What happens with LANG=C.UTF-8?

I'll let Artur finish the review if he wishes.

Comment 7 Robert-André Mauchin 🐧 2019-03-11 23:17:20 UTC
Also remove .nuxref in Release:

Comment 9 Robert-André Mauchin 🐧 2019-03-12 00:19:44 UTC
 - In that case you can remove:

%if 0%{?rhel} && 0%{?rhel} > 7
# This package is not included with EPEL7
# glibc-all-langpacks used for LANG= line in %test reference
# https://fedoraproject.org/wiki/Changes/Remove_glibc-langpacks-\
#     all_from_buildroot#Remove_glibc-all-langpacks_from_buildroot
BuildRequires: glibc-all-langpacks
%endif


 - Also drop

Group:          Development/Languages

   and:

BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

  like Artur said.

Comment 11 Robert-André Mauchin 🐧 2019-03-12 01:33:55 UTC
I'll let Artur finish if he wishes.

Comment 12 Artur Iwicki 2019-03-12 06:29:43 UTC
@Robert: Looking at Chris's intro e-mail from the devel list and taking a quick peek at the FAS groups, I believe that Chris needs to be sponsored.
Since I'm not a member of the sponsor group - but you are - I believe it'll be best if you finish this one.

Comment 13 Chris Caron 2019-03-13 00:01:04 UTC
Guys,

I also dropped all 'python-coverage', and 'python-tox' reference since I'm no longer using that in the %test section (so spec got much cleaner). This allowed me to enable testing for f28 and epel (previously disabled because of reference to tox).

But then 'epel7' couldn't pass 3 tests because they reference features only available in newer builds of 'py.test' and 'requests'. SO then i created a patch file for epel7 to work around this which means i had to drop the %autosetup in the %prep section (so i could JUST apply the patch to epel7 builds). It's been changed to just %setup so i could wrap the %patchX entry.

Anyway, there is no doubt i broke some standards here introducing the patch file.  I'm not sure how to properly host the .patch file (as it is now it's just dropped in my SOURCES directory prior to my build; it's referenced relatively in .spec). Is this satisfactory? I'll attach the spec to this ticket.  Everything still builds nicely with this additional clean-up.

SPEC: https://copr-be.cloud.fedoraproject.org/results/lead2gold/apprise/fedora-30-i386/00867578-python-apprise/python-apprise.spec

COPR:
https://copr.fedorainfracloud.org/coprs/build/867578/

Koji:
epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=33443066
f28: https://koji.fedoraproject.org/koji/taskinfo?taskID=33443069
f29: https://koji.fedoraproject.org/koji/taskinfo?taskID=33443070
f30: https://koji.fedoraproject.org/koji/taskinfo?taskID=33443071
rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=33443073

Comment 14 Chris Caron 2019-03-13 00:03:30 UTC
Created attachment 1543444 [details]
Patch to allow apprise to run unit-tests for 100% coverage in epel7

This is ONLY for running unit tests on epel7 in %test section.

It has absolutely no value outside of that and does not obstruct the actual source code deployed otherwise.

Comment 15 Robert-André Mauchin 🐧 2019-03-13 14:57:22 UTC
 - Typo:

%package -n python2-apprise-cli
Summary: Apprise CLI Tool Python 3 binary


Should be 2 here

 - Remove the Group: everywhere

 - Typo in both cli descriptions:

python3-apprise-cli.noarch: W: spelling-error %description -l en_US notificatios -> notifications

 - Remove the shebang in prep:

python3-apprise-cli.noarch: E: wrong-script-interpreter /usr/lib/python3.7/site-packages/apprise/cli.py /usr/bin/env python
python3-apprise-cli.noarch: E: non-executable-script /usr/lib/python3.7/site-packages/apprise/cli.py 644 /usr/bin/env python





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

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


Issues:
=======
- Package must not depend on deprecated() packages.
  Note: python2 is deprecated, you must not depend on it.
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/deprecating-packages/


===== 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: "Unknown or generated", "Expat License", "*No copyright* Expat
     License". 47 files have unknown license. Detailed output of
     licensecheck in /home/bob/packaging/review/python-apprise/review-
     python-apprise/licensecheck.txt
[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.
[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 20480 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 %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]: Packages MUST NOT have dependencies (either build-time or runtime) on
     packages named with the unversioned python- prefix unless no properly
     versioned package exists. Dependencies on Python packages instead MUST
     use names beginning with python2- or python3- as appropriate.
[x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files
[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).
[-]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     python3-apprise , python3-apprise-cli
[?]: 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.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[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.
[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).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: python3-apprise-0.7.4-1.fc31.noarch.rpm
          python3-apprise-cli-0.7.4-1.fc31.noarch.rpm
          python-apprise-0.7.4-1.fc31.src.rpm
python3-apprise.noarch: W: python-bytecode-without-source /usr/lib/python3.7/site-packages/apprise/__pycache__/cli.cpython-37.opt-1.pyc
python3-apprise.noarch: W: python-bytecode-without-source /usr/lib/python3.7/site-packages/apprise/__pycache__/cli.cpython-37.pyc
python3-apprise-cli.noarch: W: spelling-error %description -l en_US notificatios -> notifications, notification, ossification
python3-apprise-cli.noarch: W: no-documentation
python3-apprise-cli.noarch: E: wrong-script-interpreter /usr/lib/python3.7/site-packages/apprise/cli.py /usr/bin/env python
python3-apprise-cli.noarch: E: non-executable-script /usr/lib/python3.7/site-packages/apprise/cli.py 644 /usr/bin/env python
python3-apprise-cli.noarch: W: no-manual-page-for-binary apprise
3 packages and 0 specfiles checked; 2 errors, 5 warnings.

Comment 16 Chris Caron 2019-03-14 00:01:50 UTC
Thanks for the very detailed report.
- Spelling mistakes and shebang issued fixed. The shebang issue was done through a patch (just because i don't want to spin a whole new release). It is however already fixed upstream here: https://github.com/caronc/apprise/commit/764aa5393ffe9e09b7da6bde82078b88e39e5e58
- I'll attach the patch file for transparency anyway :)

With respect to this comment:
- Package must not depend on deprecated() packages.
  Note: python2 is deprecated, you must not depend on it.
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/deprecating-packages/

epel7 is the only one that fits this bill unfortunately; I've had to add the line at the head of the spec that reads:
%if 0%{?rhel} && 0%{?rhel} <= 7
%global with_python3 0
%endif

If I remove it, i get the failure identified here:
https://koji.fedoraproject.org/koji/taskinfo?taskID=33463886

DEBUG util.py:556:  Last metadata expiration check: 0:00:09 ago on Wed Mar 13 23:50:11 2019.
DEBUG util.py:554:  BUILDSTDERR: No matching package to install: 'python36-requests-oauthlib'
DEBUG util.py:554:  BUILDSTDERR: No matching package to install: 'python36-oauthlib'
DEBUG util.py:554:  BUILDSTDERR: No matching package to install: 'python36-markdown'
DEBUG util.py:554:  BUILDSTDERR: No matching package to install: 'python36-pytest-runner'
DEBUG util.py:554:  BUILDSTDERR: Not all dependencies satisfied
DEBUG util.py:554:  BUILDSTDERR: Error: Some packages could not be found.

But... if we put that line back we go back to all green lights; perhaps these are missing packages upstream? Maybe i should report them in another ticket?

Anyway... green lighting output (with applied changes):
======================================================
SPEC: https://copr-be.cloud.fedoraproject.org/results/lead2gold/apprise/fedora-rawhide-x86_64/00868010-python-apprise/python-apprise.spec

Koji:
epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=33463590
f28: https://koji.fedoraproject.org/koji/taskinfo?taskID=33463592
f29: https://koji.fedoraproject.org/koji/taskinfo?taskID=33463593
f30: https://koji.fedoraproject.org/koji/taskinfo?taskID=33463596
rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=33463598

COPR:
https://copr.fedorainfracloud.org/coprs/lead2gold/apprise/build/868010/


rpmlint (as of now) *.rpm produces
==================================
python3-apprise.noarch: W: python-bytecode-without-source /usr/lib/python3.7/site-packages/apprise/__pycache__/cli.cpython-37.opt-1.pyc
python3-apprise.noarch: W: python-bytecode-without-source /usr/lib/python3.7/site-packages/apprise/__pycache__/cli.cpython-37.pyc
python3-apprise-cli.noarch: W: no-documentation
python3-apprise-cli.noarch: W: no-manual-page-for-binary apprise
2 packages and 0 specfiles checked; 0 errors, 4 warnings.

----
I can look into creating a man page; for now would it be satisfactory to just identify this as a task on my git page (in the ticket system) and get to it for the next release?

Comment 17 Chris Caron 2019-03-14 00:03:16 UTC
Created attachment 1543764 [details]
shebang patch just created for this release to handle rpmlint issue

This has been fixed upstream already here:
https://github.com/caronc/apprise/commit/764aa5393ffe9e09b7da6bde82078b88e39e5e58

Thus the next official packaging of apprise will allow for the removal of this patch.

Comment 18 Robert-André Mauchin 🐧 2019-03-14 13:13:31 UTC
(In reply to Chris Caron from comment #16)
> Thanks for the very detailed report.
> - Spelling mistakes and shebang issued fixed. The shebang issue was done
> through a patch (just because i don't want to spin a whole new release). It
> is however already fixed upstream here:
> https://github.com/caronc/apprise/commit/
> 764aa5393ffe9e09b7da6bde82078b88e39e5e58
> - I'll attach the patch file for transparency anyway :)
> 
> With respect to this comment:
> - Package must not depend on deprecated() packages.
>   Note: python2 is deprecated, you must not depend on it.
>   See: https://docs.fedoraproject.org/en-US/packaging-
>   guidelines/deprecating-packages/
> 
> epel7 is the only one that fits this bill unfortunately; I've had to add the
> line at the head of the spec that reads:
> %if 0%{?rhel} && 0%{?rhel} <= 7
> %global with_python3 0
> %endif
> 
> If I remove it, i get the failure identified here:
> https://koji.fedoraproject.org/koji/taskinfo?taskID=33463886
> 

Disregard that error, it's a bug I need to fix on the latest fedora-review. It's a beta test version so we're ironing things out. I should have removed it.

Comment 19 Chris Caron 2019-03-14 23:45:44 UTC
Robert, I stuck with just setting a Provides: entry to the python2-apprise-cli and python3-apprise-cli because I couldn't think of a clever way to not have a %package conflict if i renamed both of them to just 'apprise'. You can check it my workaround here.

SPEC: https://copr-be.cloud.fedoraproject.org/results/lead2gold/apprise/fedora-rawhide-x86_64/00868455-python-apprise/python-apprise.spec

epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=33494613
f29: https://koji.fedoraproject.org/koji/taskinfo?taskID=33494615
f30: https://koji.fedoraproject.org/koji/taskinfo?taskID=33494617
f31: https://koji.fedoraproject.org/koji/taskinfo?taskID=33494619
rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=33494621

COPR: https://copr.fedorainfracloud.org/coprs/build/868455

You'll be happy to know i built a man page too to start with; I'll probably need more to explain how configuration files work, but it's a start and also greatly reduces your rpmlint warnings to just 2 now:
python3-apprise.noarch: W: python-bytecode-without-source /usr/lib/python3.7/site-packages/apprise/__pycache__/cli.cpython-37.opt-1.pyc
python3-apprise.noarch: W: python-bytecode-without-source /usr/lib/python3.7/site-packages/apprise/__pycache__/cli.cpython-37.pyc

I'll need to Google these warnings because i don't quite know what it refers to at this time. The man page (to clear up 2 of the warnings) was added as an additional Source which has also already been pushed upstream; so it will be bundled in upcoming releases.

As always, I respect any advice or feedback you have!

Chris

Comment 20 Chris Caron 2019-03-14 23:48:32 UTC
Created attachment 1544222 [details]
man page added to help with rpmlint warnings

Just attaching the man page created as a Source1: directive in the RPM which will be dropped in the next release as it's already been included for future packaging.

I created a branch here: https://github.com/caronc/apprise/tree/external-packaging-support which i will merge when this process is over (hopefully successfully :)  )

Comment 21 Robert-André Mauchin 🐧 2019-03-15 00:14:59 UTC
 - Remove Group: everywhere,  still here

 - This is not needed:

%if 0%{?rhel} && 0%{?rhel} <= 7
# Backwards Compatibility
Provides: python-apprise = %{version}
Obsoletes:python-apprise < %{version}
%endif # rhel


I didn't realise it was a binary, I still think you should use apprise as a *single* package, but you need to rework the conditional here, Py 2 and Py 3 being exclusionary to each other.

Remove && 0%{?fedora} > 29 and add RHEL8

%if 0%{?fedora} || 0%{?rhel} >= 8
# Python v2 Support dropped
%global with_python2 0
%endif # fedora



%package -n apprise
Summary: Apprise CLI Tool

%if 0%{?with_python2}
Requires: python2-click >= 5.0
Requires: python2-apprise = %{version}-%{release}
%endif # with_python2

%if 0%{?with_python3}
Requires: python%{python3_pkgversion}-click >= 5.0
Requires: python3-apprise = %{version}-%{release}
%endif # with_python3

%description -n apprise
An accompanied CLI tool that can be used as part of Apprise
to issue notifications from the command line to you favorite
services.


[…]


%files -n apprise
%{_mandir}/man1/apprise.1*
%{_bindir}/apprise

%if 0%{?with_python2}
%{python2_sitelib}/apprise/cli.*
%endif # with_python2
%if 0%{?with_python3}
%{python3_sitelib}/apprise/cli.*
%endif # with_python3

Comment 22 Chris Caron 2019-03-15 01:19:20 UTC
Robert,

>  Remove Group: everywhere

Oops; no excuses for this one.  Fixed now

Thank you for your suggestion; all of your advice has been applied to the below. I really like the idea of having the -cli building as apprise too! However... soon the .spec file is going to be 0 bytes in size at the rate you're condensing it. :)

All green lights:

SPEC: https://copr-be.cloud.fedoraproject.org/results/lead2gold/apprise/fedora-rawhide-x86_64/00868559-python-apprise/python-apprise.spec

Koji:
epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=33499792
f29: https://koji.fedoraproject.org/koji/taskinfo?taskID=33499794
f30: https://koji.fedoraproject.org/koji/taskinfo?taskID=33499797
f31: https://koji.fedoraproject.org/koji/taskinfo?taskID=33499799
rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=33499801

COPR: https://copr.fedorainfracloud.org/coprs/build/868559

We're up to something like 8 attempts at this; but who's counting? ;)

Comment 23 Robert-André Mauchin 🐧 2019-03-18 23:07:29 UTC
 - You could use sed instead of a patch to remove the shebang, it's a bbit overkill for one line.

 - %{__install} → install



Package is approved.


You still need to find a sponsor:
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group


(Yes I need to find time to finish the review I gave you)





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.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "Expat License", "*No copyright* Expat
     License". 47 files have unknown license. Detailed output of
     licensecheck in /home/bob/packaging/review/python-apprise/review-
     python-apprise/licensecheck.txt
[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.
[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 20480 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 %license.
[x]: Package requires other packages for directories it uses.
[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]: Packages MUST NOT have dependencies (either build-time or runtime) on
     packages named with the unversioned python- prefix unless no properly
     versioned package exists. Dependencies on Python packages instead MUST
     use names beginning with python2- or python3- as appropriate.
[x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files
[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).
[-]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in apprise
     , python3-apprise
[?]: 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]: 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.
[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).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: apprise-0.7.4-1.fc31.noarch.rpm
          python3-apprise-0.7.4-1.fc31.noarch.rpm
          python-apprise-0.7.4-1.fc31.src.rpm
apprise.noarch: W: name-repeated-in-summary C Apprise
python3-apprise.noarch: W: python-bytecode-without-source /usr/lib/python3.7/site-packages/apprise/__pycache__/cli.cpython-37.opt-1.pyc
python3-apprise.noarch: W: python-bytecode-without-source /usr/lib/python3.7/site-packages/apprise/__pycache__/cli.cpython-37.pyc
python-apprise.src:208: W: macro-in-comment %{__install}
python-apprise.src:209: W: macro-in-comment %{buildroot}
python-apprise.src:209: W: macro-in-comment %{_mandir}
python-apprise.src:209: W: mixed-use-of-spaces-and-tabs (spaces: line 45, tab: line 209)
3 packages and 0 specfiles checked; 0 errors, 7 warnings.

Comment 24 Chris Caron 2019-03-19 00:02:13 UTC
>  - You could use sed instead of a patch to remove the shebang, it's a bbit overkill for one line.

This will go away in the next version as it's fixed upstream anyway.

> - %{__install} → install

I didn't realise we weren't supposed to use %{__rm}, %{__install}, %{__cp}; i always thought this was a safer approach.  Noted though, I'll fix this in the spec file too.

> You still need to find a sponsor:
> https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

> (Yes I need to find time to finish the review I gave you)

I honestly thought the email you sent me entitled 'Fedora packaging sponsorship' and the full review of another package you had me do for you was hinting the possibility that 'you' might be that person.

Regardless, I'm still thankful. Thank you for taking the time anyway to review my product and approving it.  I'll follow through with the wiki link you shared and see where it takes me.

Chris

Comment 25 Robert-André Mauchin 🐧 2019-03-19 00:10:26 UTC
(In reply to Chris Caron from comment #24)
> >  - You could use sed instead of a patch to remove the shebang, it's a bbit overkill for one line.
> 
> This will go away in the next version as it's fixed upstream anyway.
> 
> > - %{__install} → install
> 
> I didn't realise we weren't supposed to use %{__rm}, %{__install}, %{__cp};
> i always thought this was a safer approach.  Noted though, I'll fix this in
> the spec file too.
> 
> > You still need to find a sponsor:
> > https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
> 
> > (Yes I need to find time to finish the review I gave you)
> 
> I honestly thought the email you sent me entitled 'Fedora packaging
> sponsorship' and the full review of another package you had me do for you
> was hinting the possibility that 'you' might be that person.

yes it is a possibility, you might still do other informal reviews and post them here to get sponsored, in order to show that you grok the guidelines.

Comment 26 Robert-André Mauchin 🐧 2019-03-24 17:21:19 UTC
Sponsored.

Comment 27 Igor Raits 2019-03-25 06:52:20 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/python-apprise


Note You need to log in before you can comment on or make changes to this bug.