Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1749383 - Review Request: mesos - Apache Mesos Cluster Manager
Summary: Review Request: mesos - Apache Mesos Cluster Manager
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: 30
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Simone Caronni
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-NEEDSPONSOR
TreeView+ depends on / blocked
 
Reported: 2019-09-05 13:50 UTC by Javi Roman
Modified: 2019-10-19 17:41 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-10-19 17:41:27 UTC
Type: ---
Embargoed:
negativo17: fedora-review+


Attachments (Terms of Use)

Description Javi Roman 2019-09-05 13:50:56 UTC
Spec URL: https://src.fedoraproject.org/fork/jromanes/rpms/mesos/blob/f30/f/mesos.spec
SRPM URL: https://github.com/javiroman/mesos-rpm/raw/master/mesos-1.8.1-1.fc30.src.rpm

Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=37480911

Description: Apache Mesos is a cluster manager that provides efficient resource
isolation and sharing across distributed applications, or frameworks.
It can run Hadoop, MPI, Hypertable, Spark, and other applications on
a dynamically shared pool of nodes.

Fedora Account System Username: jromanes

Comment 1 Fabio Valentini 2019-09-05 14:21:08 UTC
You might want to mention that this package already existed in fedora but you want to un-retire it for fedora 30 and later (I guess).

Comment 2 Javi Roman 2019-09-05 14:27:10 UTC
(In reply to Fabio Valentini from comment #1)
> You might want to mention that this package already existed in fedora but
> you want to un-retire it for fedora 30 and later (I guess).

Your are right, this package is orphaned, I would like reboot this package. According with https://fedoraproject.org/wiki/Orphaned_package_that_need_new_maintainers I have created this new review. Please, let me know whether this is a wrong procedure.

Comment 3 Fabio Valentini 2019-09-05 14:30:59 UTC
The package is not actually orphaned, but it was retired from fedora 30+.

I see you're not yet a member of the "packager" group in fedora, so you'll also need to find a sponsor before you can actually build any packages.

Comment 4 Simone Caronni 2019-09-05 14:54:28 UTC
(In reply to Fabio Valentini from comment #3)
> The package is not actually orphaned, but it was retired from fedora 30+.
> 
> I see you're not yet a member of the "packager" group in fedora, so you'll
> also need to find a sponsor before you can actually build any packages.

I will sponsor Javi after the review. I'm also the person who retired the package after getting no maintainers upfront when I stopped using the package.

Comment 5 Simone Caronni 2019-09-17 18:36:47 UTC
(In reply to Javi Roman from comment #0)
> Spec URL:
> https://src.fedoraproject.org/fork/jromanes/rpms/mesos/blob/f30/f/mesos.spec

This is not the raw file, so it can not be downloaded by the review. In case of changes post the link to the raw file.
I'll do with the local files.

Comment 6 Simone Caronni 2019-09-17 19:23:21 UTC
> BuildRequires: systemd
> BuildRequires: systemd

Duplicate BuildRequires.
Also please sort all the BuildRequires tags all together, they are in 3 blocks not in alphabetical order and with no explanation. Makes spotting obvious mistakes a bit harder.

> %description
> Apache Mesos is a cluster manager that provides efficient resource
> isolation and sharing across distributed applications, or frameworks.
> It can run Hadoop, MPI, Hypertable, Spark, and other applications on
> a dynamically shared pool of nodes.

Please align to 80 characters columns, if possible.

> Group:    Development/Libraries

Remove all the Groups tags, they are obsolete since RHEL 5.

> Requires: mesos%{?_isa} = %{version}-%{release}

Should be %{name}%{?_isa}, for consistency.

> BuildRequires:  python2-devel

Do not sort BuildRequires in the subpackage definition. It is also a duplicate.

> %setup -q
> %patch0 -p1 -F2

Please use the %autosetup macro. Also do not change the fuzz lines, the strict fuzz rules are there to avoid patching the wrong block (it happened!). Rebase the patch if necessary.

> # Apparently bundled:
> # 1. CSI spec: https://github.com/container-storage-interface/spec/releases
> # 2. ConcurrentQueue: https://github.com/cameron314/concurrentqueue
> # 3. File gmock-all.cc from Google Gmock: https://github.com/google/googletest
> # 4. Nvidia GPU deployment kit nvml.h header file.
> cd 3rdparty
> tar xzf concurrentqueue-7b69a8f.tar.gz
> tar xzf googletest-release-1.8.0.tar.gz
> tar xzf nvml-352.79.tar.gz
> cd ..

If this is the case, then they must be listed in the bundled Provides tags.

> %make_build %{?_smp_mflags} V=1

Remove _smp_mflags, it's already part of the macro:

$ rpm --showrc | grep make_build
-13: make_build	%{__make} %{_make_output_sync} %{?_smp_mflags}

> %if %skiptests
>   echo "Skipping tests, do to mock issues"
> %else
>   export LD_LIBRARY_PATH=`pwd`/src/.libs
>   make check
> %endif

I would not print anything if disabled, maybe add a comment, but as you wish. If you want to keep the output then typo (s/do/due/g).

> %{__python2} setup.py install --root=%{buildroot} --prefix=/usr

Use %{_prefix} instead of explicit /usr.

> # fedora guidelines no .a|.la
> rm -f %{buildroot}%{_libdir}/*.la
> rm -f %{buildroot}%{_libdir}/libexamplemodule*
> rm -f %{buildroot}%{_libdir}/libtest*
> rm -f %{buildroot}/%{_libdir}/%{name}/modules/*.la

You can probably just do something like:

find %{buildroot} -name '*.a' -o -name '*.la' -delete

But as you prefer.

> mkdir -p -m0755 %{buildroot}%{_sysconfdir}/tmpfiles.d
> install -m 0644 %{SOURCE1} %{buildroot}%{_sysconfdir}/tmpfiles.d/%{name}.conf
> %config(noreplace) %{_sysconfdir}/tmpfiles.d/%{name}.conf

Please replace these with the macro and don't mark it as a config file as it is under /usr/lib/tmpfiles.d/:

mkdir -p -m0755 %{buildroot}%{_tmpfilesdir}
install -m 0644 %{SOURCE1} %{buildroot}%{_tmpfilesdir}/%{name}.conf
%{_tmpfilesdir}/%{name}.conf

> mkdir -p -m0755 %{buildroot}/%{_var}/log/%{name}
> mkdir -p -m0755 %{buildroot}/%{_var}/lib/%{name}

Please use:

mkdir -p -m0755 %{buildroot}/%{_localstatedir}/log/%{name}
mkdir -p -m0755 %{buildroot}/%{_sharedstatedir}/%{name}

> install -m
> cp -rf

Please replace with "install -p -m" as you are supposed to preserve timestamps if possible. Same for cp, you can use the -a switch (for example).

> pathfix.py -pni "%{__python2} %{py2_shbang_opts}" %{buildroot}/usr/bin/mesos-cat
> pathfix.py -pni "%{__python2} %{py2_shbang_opts}" %{buildroot}/usr/bin/mesos-scp
> pathfix.py -pni "%{__python2} %{py2_shbang_opts}" %{buildroot}/usr/bin/mesos-ps
> pathfix.py -pni "%{__python2} %{py2_shbang_opts}" %{buildroot}/usr/bin/mesos-tail
> pathfix.py -pni "/usr/bin/bash" %{buildroot}/usr/share/mesos/examples
> pathfix.py -pni "/usr/bin/bash" %{buildroot}/usr/sbin/mesos*.sh

Please be consistent across the SPEC file, replace those paths with %{_bindir}, %{_sbindir}, and %{_datadir}.

> %files
> %doc LICENSE NOTICE

It should be:

%files
%license LICENSE
%doc NOTICE

Also, the license is fine in a reduced number of packages as long as it is installed in any combination of the packages being installed. Considering the base mesos package is required by all subpackages, you can skip entirely the %license and %doc tag in all subpackages.

> %{_libdir}/*.so

Why is this unversioned? If it can not be versioned it needs an explanation or to be moved to a different path.

> if ! getent passwd mesos >/dev/null ; then
>      useradd -r -g mesos -d %{_sharedstatedir}/%{name} -s /sbin/nologin \
>              -c "%{name} daemon account" mesos
> fi

Please use the packaging guidelines format, and especially add the shadow-utils requirement:
https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_dynamic_allocation

Also, you can probably use a variable at the top of the file:

%global user mesos
%global group mesos

And then replace all occurrences in the SPEC file, in the %pre and %files section.

> %systemd_post %{name}-slave.service %{name}-master.service

Please use the BuildRequires indicated:

https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_systemd

> /sbin/ldconfig

If you plan on building this only on Fedora & CentOS/RHEL 8 you can skip it entirely.

Otherwise, if you also target EPEL-7 there is this:

https://fedoraproject.org/wiki/EPEL:Packaging#Shared_Libraries

> %changelog
> * Wed Sep 04 2019 Javi Roman  <javiroman> - 1.8.1

Feel free to trim the changelog before you, maybe just leave one message before yours. As you wish.
The package version at the end should be "1.8.1-1", you can update this automatically with "rpmdev-bumpspec" to have it properly formatted.

Comment 7 Fabio Valentini 2019-09-17 19:30:23 UTC
> BuildRequires:  python2-devel

You'll either need to drop the python2 bindings from this package, or request an exception for the python2 dependency from FESCo.
No new fedora packages are allowed to depend on python2 without FESCo approval.

See: https://fedoraproject.org/wiki/Changes/F31_Mass_Python_2_Package_Removal

Comment 8 Simone Caronni 2019-09-17 19:31:13 UTC
> License:       ASL 2.0

The license field should contain all licenses, the bundled things have different ones:

https://github.com/google/googletest/blob/master/LICENSE
https://github.com/cameron314/concurrentqueue/blob/master/LICENSE.md

So they need to be displayed in this field. See here on how to combine them:

https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_multiple_licensing_scenarios
https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_combined_dual_and_multiple_licensing_scenario
https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_mixed_source_licensing_scenario

The NVML header is probably not free:

/*
 * Copyright 1993-2015 NVIDIA Corporation.  All rights reserved.
 *
 * NOTICE TO USER:   
 *
 * This source code is subject to NVIDIA ownership rights under U.S. and 
 * international Copyright laws.  Users and possessors of this source code 
 * are hereby granted a nonexclusive, royalty-free license to use this code 
 * in individual and commercial software.
 *
 * NVIDIA MAKES NO REPRESENTATION ABOUT THE SUITABILITY OF THIS SOURCE 
 * CODE FOR ANY PURPOSE.  IT IS PROVIDED "AS IS" WITHOUT EXPRESS OR 
 * IMPLIED WARRANTY OF ANY KIND.  NVIDIA DISCLAIMS ALL WARRANTIES WITH 
 * REGARD TO THIS SOURCE CODE, INCLUDING ALL IMPLIED WARRANTIES OF 
 * MERCHANTABILITY, NONINFRINGEMENT, AND FITNESS FOR A PARTICULAR PURPOSE.
 * IN NO EVENT SHALL NVIDIA BE LIABLE FOR ANY SPECIAL, INDIRECT, INCIDENTAL, 
 * OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS 
 * OF USE, DATA OR PROFITS,  WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE 
 * OR OTHER TORTIOUS ACTION,  ARISING OUT OF OR IN CONNECTION WITH THE USE 
 * OR PERFORMANCE OF THIS SOURCE CODE.  
 *
 * U.S. Government End Users.   This source code is a "commercial item" as 
 * that term is defined at  48 C.F.R. 2.101 (OCT 1995), consisting  of 
 * "commercial computer  software"  and "commercial computer software 
 * documentation" as such terms are  used in 48 C.F.R. 12.212 (SEPT 1995) 
 * and is provided to the U.S. Government only as a commercial end item.  
 * Consistent with 48 C.F.R.12.212 and 48 C.F.R. 227.7202-1 through 
 * 227.7202-4 (JUNE 1995), all U.S. Government End Users acquire the 
 * source code with only those rights set forth herein. 
 *
 * Any use of this source code in individual and commercial software must 
 * include, in the user documentation and internal comments to the code,
 * the above Disclaimer and U.S. Government End Users Notice.
 */

If you want to compile against we need to block FE-Legal: https://fedoraproject.org/wiki/Legal:Main#FE-Legal

Comment 9 Simone Caronni 2019-09-17 19:32:42 UTC
(In reply to Fabio Valentini from comment #7)
> > BuildRequires:  python2-devel
> 
> You'll either need to drop the python2 bindings from this package, or
> request an exception for the python2 dependency from FESCo.
> No new fedora packages are allowed to depend on python2 without FESCo
> approval.
> 
> See: https://fedoraproject.org/wiki/Changes/F31_Mass_Python_2_Package_Removal

Correct. If you instead plan to build for CentOS/RHEL 7 you can probably introduce some conditionals. For example:

%if 0%{?fedora} || 0%{?rhel} >= 8
BuildRequires:  python3-devel
%else
BuildRequires:  python2-devel
%endif

Comment 10 Simone Caronni 2019-09-17 19:35:26 UTC
> mesos-master.service
> mesos-slave.service

Please add a Documentation line in both:
https://docs.fedoraproject.org/en-US/packaging-guidelines/Systemd/#_unit_files

Comment 11 Simone Caronni 2019-09-19 05:15:02 UTC
A few more things after running rpmlint:

$ rpmlint *rpm
mesos.src: W: strange-permission mesos-init-wrapper 775
mesos.x86_64: W: incoherent-version-in-changelog 1.8.1 ['1.8.1-1.fc30', '1.8.1-1']
mesos.x86_64: W: crypto-policy-non-compliance-openssl /usr/lib64/libmesos-1.8.1.so SSL_CTX_set_cipher_list
mesos.x86_64: E: missing-call-to-chdir-with-chroot /usr/lib64/libmesos-1.8.1.so
mesos.x86_64: E: wrong-script-interpreter /etc/mesos/mesos-deploy-env.sh.template /usr/bin/env bash
mesos.x86_64: E: non-executable-script /etc/mesos/mesos-deploy-env.sh.template 644 /usr/bin/env bash
mesos.x86_64: W: tmpfiles-conf-in-etc /etc/tmpfiles.d/mesos.conf
mesos.x86_64: W: hidden-file-or-dir /usr/share/mesos/examples/.deps
mesos.x86_64: W: hidden-file-or-dir /usr/share/mesos/examples/.deps
mesos.x86_64: W: hidden-file-or-dir /usr/share/mesos/examples/.deps/.dirstamp
mesos.x86_64: E: zero-length /usr/share/mesos/examples/.deps/.dirstamp
mesos.x86_64: W: hidden-file-or-dir /usr/share/mesos/examples/.dirstamp
mesos.x86_64: E: zero-length /usr/share/mesos/examples/.dirstamp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/balloon_executor.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/balloon_framework.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/disk_full_framework.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/docker_no_executor_framework.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/dynamic_reservation_framework.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/example_module_impl.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/flags.hpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/inverse_offer_framework.cpp
mesos.x86_64: E: wrong-script-interpreter /usr/share/mesos/examples/java/test-exception-framework.in /usr/bin/env bash
mesos.x86_64: E: non-executable-script /usr/share/mesos/examples/java/test-exception-framework.in 644 /usr/bin/env bash
mesos.x86_64: E: wrong-script-interpreter /usr/share/mesos/examples/java/test-executor.in /usr/bin/env bash
mesos.x86_64: E: non-executable-script /usr/share/mesos/examples/java/test-executor.in 644 /usr/bin/env bash
mesos.x86_64: E: wrong-script-interpreter /usr/share/mesos/examples/java/test-framework.in /usr/bin/env bash
mesos.x86_64: E: non-executable-script /usr/share/mesos/examples/java/test-framework.in 644 /usr/bin/env bash
mesos.x86_64: E: wrong-script-interpreter /usr/share/mesos/examples/java/test-log.in /usr/bin/env bash
mesos.x86_64: E: non-executable-script /usr/share/mesos/examples/java/test-log.in 644 /usr/bin/env bash
mesos.x86_64: E: wrong-script-interpreter /usr/share/mesos/examples/java/test-multiple-executors-framework.in /usr/bin/env bash
mesos.x86_64: E: non-executable-script /usr/share/mesos/examples/java/test-multiple-executors-framework.in 644 /usr/bin/env bash
mesos.x86_64: E: wrong-script-interpreter /usr/share/mesos/examples/java/v1-test-framework.in /usr/bin/env bash
mesos.x86_64: E: non-executable-script /usr/share/mesos/examples/java/v1-test-framework.in 644 /usr/bin/env bash
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/load_generator_framework.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/long_lived_executor.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/long_lived_framework.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/no_executor_framework.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/operation_feedback_framework.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/persistent_volume_framework.cpp
mesos.x86_64: E: wrong-script-interpreter /usr/share/mesos/examples/python/test-executor.in /usr/bin/env bash
mesos.x86_64: E: non-executable-script /usr/share/mesos/examples/python/test-executor.in 644 /usr/bin/env bash
mesos.x86_64: E: wrong-script-interpreter /usr/share/mesos/examples/python/test-framework.in /usr/bin/env bash
mesos.x86_64: E: non-executable-script /usr/share/mesos/examples/python/test-framework.in 644 /usr/bin/env bash
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_allocator_module.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_anonymous_module.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_anonymous_module.hpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_authentication_modules.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_authorizer_module.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_container_logger_module.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_csi_plugin.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_csi_user_framework.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_executor.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_framework.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_hook_module.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_http_authenticator_module.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_http_executor.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_http_framework.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_isolator_module.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_master_contender_module.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_master_detector_module.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_module.hpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_qos_controller_module.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/test_resource_estimator_module.cpp
mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/utils.hpp
mesos.x86_64: W: non-standard-uid /var/lib/mesos mesos
mesos.x86_64: W: non-standard-gid /var/lib/mesos mesos
mesos.x86_64: W: non-standard-uid /var/log/mesos mesos
mesos.x86_64: W: non-standard-gid /var/log/mesos mesos
mesos.x86_64: W: log-files-without-logrotate ['/var/log/mesos']
mesos.x86_64: W: no-manual-page-for-binary mesos
mesos.x86_64: W: no-manual-page-for-binary mesos-cat
mesos.x86_64: W: no-manual-page-for-binary mesos-execute
mesos.x86_64: W: no-manual-page-for-binary mesos-init-wrapper
mesos.x86_64: W: no-manual-page-for-binary mesos-local
mesos.x86_64: W: no-manual-page-for-binary mesos-log
mesos.x86_64: W: no-manual-page-for-binary mesos-ps
mesos.x86_64: W: no-manual-page-for-binary mesos-resolve
mesos.x86_64: W: no-manual-page-for-binary mesos-scp
mesos.x86_64: W: no-manual-page-for-binary mesos-tail
mesos.x86_64: W: no-manual-page-for-binary mesos-agent
mesos.x86_64: W: no-manual-page-for-binary mesos-daemon.sh
mesos.x86_64: W: no-manual-page-for-binary mesos-master
mesos.x86_64: W: no-manual-page-for-binary mesos-slave
mesos.x86_64: W: no-manual-page-for-binary mesos-start-agents.sh
mesos.x86_64: W: no-manual-page-for-binary mesos-start-cluster.sh
mesos.x86_64: W: no-manual-page-for-binary mesos-start-masters.sh
mesos.x86_64: W: no-manual-page-for-binary mesos-start-slaves.sh
mesos.x86_64: W: no-manual-page-for-binary mesos-stop-agents.sh
mesos.x86_64: W: no-manual-page-for-binary mesos-stop-cluster.sh
mesos.x86_64: W: no-manual-page-for-binary mesos-stop-masters.sh
mesos.x86_64: W: no-manual-page-for-binary mesos-stop-slaves.sh
python2-mesos.x86_64: E: non-executable-script /usr/lib/python2.7/site-packages/mesos/cli.py 644 /usr/bin/python2 -s
python2-mesos.x86_64: E: non-executable-script /usr/lib/python2.7/site-packages/mesos/futures.py 644 /usr/bin/python2 -s
python2-mesos.x86_64: E: non-executable-script /usr/lib/python2.7/site-packages/mesos/http.py 644 /usr/bin/python2 -s
7 packages and 0 specfiles checked; 24 errors, 70 warnings.

> mesos.src: W: strange-permission mesos-init-wrapper 775

This should be 755.

> mesos.x86_64: W: incoherent-version-in-changelog 1.8.1 ['1.8.1-1.fc30', '1.8.1-1']

As described above:

> mesos.x86_64: W: crypto-policy-non-compliance-openssl /usr/lib64/libmesos-1.8.1.so SSL_CTX_set_cipher_list

Please see https://docs.fedoraproject.org/en-US/packaging-guidelines/CryptoPolicies/#_cc_applications

mesos.x86_64: E: missing-call-to-chdir-with-chroot /usr/lib64/libmesos-1.8.1.so
mesos.x86_64: E: wrong-script-interpreter /etc/mesos/mesos-deploy-env.sh.template /usr/bin/env bash
mesos.x86_64: E: non-executable-script /etc/mesos/mesos-deploy-env.sh.template 644 /usr/bin/env bash

> mesos.x86_64: W: tmpfiles-conf-in-etc /etc/tmpfiles.d/mesos.conf

As described above, move to %{_tpmfilesdir}.

> mesos.x86_64: W: hidden-file-or-dir /usr/share/mesos/examples/.deps
> mesos.x86_64: W: hidden-file-or-dir /usr/share/mesos/examples/.deps
> mesos.x86_64: W: hidden-file-or-dir /usr/share/mesos/examples/.deps/.dirstamp
> mesos.x86_64: E: zero-length /usr/share/mesos/examples/.deps/.dirstamp
> mesos.x86_64: W: hidden-file-or-dir /usr/share/mesos/examples/.dirstamp
> mesos.x86_64: E: zero-length /usr/share/mesos/examples/.dirstamp

Remove.

> mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/balloon_executor.cpp
> mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/inverse_offer_framework.cpp
> mesos.x86_64: E: wrong-script-interpreter /usr/share/mesos/examples/java/test-exception-framework.in /usr/bin/env bash
> mesos.x86_64: E: non-executable-script /usr/share/mesos/examples/java/test-exception-framework.in 644 /usr/bin/env bash
> mesos.x86_64: W: devel-file-in-non-devel-package /usr/share/mesos/examples/utils.hpp
> [...]

I would move all the examples in the devel subpackage, I don't think for a production cluster you need to have source code examples on each and every node.

> mesos.x86_64: W: non-standard-uid /var/lib/mesos mesos
> mesos.x86_64: W: non-standard-gid /var/log/mesos mesos

This is fine.

> mesos.x86_64: W: log-files-without-logrotate ['/var/log/mesos']

Correct, a simple logrotate file should be added.

> mesos.x86_64: W: no-manual-page-for-binary mesos
> [...]

This is fine.

> python2-mesos.x86_64: E: non-executable-script /usr/lib/python2.7/site-packages/mesos/cli.py 644 /usr/bin/python2 -s
> python2-mesos.x86_64: E: non-executable-script /usr/lib/python2.7/site-packages/mesos/futures.py 644 /usr/bin/python2 -s
> python2-mesos.x86_64: E: non-executable-script /usr/lib/python2.7/site-packages/mesos/http.py 644 /usr/bin/python2 -s

I guess this should be 755, but probably can also be ignored as it is executed anyway.

Comment 12 Simone Caronni 2019-09-19 11:23:01 UTC
Sorry, can't edit anymore the last comment. There is some error. For this:

> mesos.x86_64: E: missing-call-to-chdir-with-chroot /usr/lib64/libmesos-1.8.1.so

Check if it is really an issue, it might be a false positive or just fine as is.

> mesos.x86_64: E: wrong-script-interpreter /etc/mesos/mesos-deploy-env.sh.template /usr/bin/env bash
> mesos.x86_64: E: non-executable-script /etc/mesos/mesos-deploy-env.sh.template 644 /usr/bin/env bash

Make sure the interpreter (/usr/bin/bash) is correct and then you can ignore the permission if it is assembled into another file.

Comment 13 Javi Roman 2019-09-23 16:47:49 UTC
Spec URL: https://src.fedoraproject.org/fork/jromanes/rpms/mesos/raw/f30/f/mesos.spec 
SRPM URL: https://github.com/javiroman/mesos-rpm/raw/master/mesos-1.8.1-1.fc30.src.rpm 
Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=37824906 

Here the new release candidate for reviewing. Many thanks for the help!

Important changes from the last package:

- Python2 bits removed

The upstream is in the way of reworking the code base towards Python3 (https://issues.apache.org/jira/browse/MESOS-8882). The Python bindings, utils and new cli based on Python3 will be added further. The Python2 code is considered deprecated in this current release, so can be removed for sanity.

- Supackages

The base package Mesos only has one sub-package: mesos-devel. This package contains the development libraries for creating frameworks (Java) and source code examples, *.so libraries and so on.

Some important issues:

- Shared library versioning

This package is shipped with libmesos library with apparently no versioning numbers. Rationale: According with Program-Library-HOWTO/shared-libraries.html when a new version of a library is binary-incompatible with the old one the soname needs to change. In Apache Mesos libmesos is always incompatible with other Apache Mesos versions. So the upstream enter the complete package version into the SONAME:

objdump -p /lib64/libmesos-1.8.1.so | grep SONAME
  SONAME               libmesos-1.8.1.so

- Bundled bits

Provides: bundled(concurrentqueue) = 7b69a8f
Provides: bundled(csi-spec) = 1.1.0

The last one, Container Storage Interface is not a library, is only a Protocol Buffer specification.

Google Test is not bundled in this package, only is used for building.

Nvidia GPU deployment kit nvml.h header file is not bundled in this package, is only used for building time. Regarding the license, I guess is right to block FE-Legal.

- Rpmlint Errors:

mesos.x86_64: E: missing-call-to-chdir-with-chroot /usr/lib64/libmesos-1.8.1.so

I don’t know the meaning of this. 

mesos.x86_64: E: postin-without-ldconfig /usr/lib64/libmesos-1.8.1.so
mesos.x86_64: E: postun-without-ldconfig /usr/lib64/libmesos-1.8.1.so

According with this https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets we don’t need ldconfig, however rpmlint is raising this errors.

Comment 14 Simone Caronni 2019-09-24 07:15:43 UTC
Very good!

> SRPM URL: https://github.com/javiroman/mesos-rpm/raw/master/mesos-1.8.1-1.fc30.src.rpm

When submitting an updated SPEC file and SRPM, you should bump at least the release, so in this case it should have been at least 1.8.1-2. Keep it in mind for the next time.

Checking all the changes in the SPEC file now.

Comment 15 Simone Caronni 2019-09-24 07:15:57 UTC
Two dependencies of the package have been orphaned, so I guess that if you want to keep on building it for the foreseeable future you need to take over ownership of these two as well:

https://src.fedoraproject.org/rpms/maven-gpg-plugin
https://src.fedoraproject.org/rpms/maven-site-plugin

At the moment Mesos does not build in rawhide / f31 because of this. I can sponsor you in, you push this for the current Fedora branches and then you should probably post a new review for those dependencies (I can do it).

Comment 16 Simone Caronni 2019-09-24 07:20:35 UTC
Blocking FE-LEGAL for this, I'm not able to say if it's fine or not:

https://src.fedoraproject.org/fork/jromanes/rpms/mesos/blob/f30/f/mesos.spec#_111

Comment 17 Fabio Valentini 2019-09-24 07:31:18 UTC
(In reply to Simone Caronni from comment #15)
> Two dependencies of the package have been orphaned, so I guess that if you
> want to keep on building it for the foreseeable future you need to take over
> ownership of these two as well:
> 
> https://src.fedoraproject.org/rpms/maven-gpg-plugin
> https://src.fedoraproject.org/rpms/maven-site-plugin

Just to clarify, these packages were not only orphaned, but also retired about half a year ago. And since they were retired for more than 8 weeks, you will need to file re-review requests *and* un-retirement requests with releng (at https://pagure.io/releng). It's also possible that you'll need to fix the packages as well, because it seems like recent updates broke the versions that were last available from fedora.

> At the moment Mesos does not build in rawhide / f31 because of this. I can
> sponsor you in, you push this for the current Fedora branches and then you
> should probably post a new review for those dependencies (I can do it).

Comment 18 Fabio Valentini 2019-09-24 11:37:39 UTC
Now that I'm thinking about it, you'd probably be able to just disable those two maven plugins without affecting the build.
They shouldn't be used for producing any build output for an RPM package.

%pom_remove_plugin :maven-site-plugin <pom location>
%pom_remove_plugin :maven-gpg-plugin <pom location>

usually do the trick, but since this package embeds a maven project inside autotools, this might not work as expected, and you'd have to manually patch the pom.xml.in file instead. That way you wouldn't need to resurrect two ancient packages that were unmaintained for years.

Comment 19 Tom "spot" Callaway 2019-09-24 17:13:56 UTC
I am 99% sure the nvml.h file is a non-free file, and cannot be included in the mesos Fedora package. How vital is that file?

Comment 20 Javi Roman 2019-09-24 21:18:39 UTC
Spec URL: https://src.fedoraproject.org/fork/jromanes/rpms/mesos/raw/f30/f/mesos.spec 
SRPM URL: https://github.com/javiroman/mesos-rpm/raw/master/mesos-1.8.1-2.fc30.src.rpm

New candidate:

- %pom_remove_plugin :maven-gpg-plugin did the trick (many thanks @Fabio)
- maven-site-plugin removed without problems, this was my fault, it's not a dependency at all.

Checking with upstream the issue of nvml.h file.

Right now I'm checking with f31 too, however it looks zookeeper package is no ready yet (and it's a dependency):
https://apps.fedoraproject.org/packages/zookeeper/

Comment 21 Javi Roman 2019-09-25 09:46:04 UTC
Spec URL: https://src.fedoraproject.org/fork/jromanes/rpms/mesos/raw/f30/f/mesos.spec 
SRPM URL: https://github.com/javiroman/mesos-rpm/raw/master/mesos-1.8.1-3.fc30.src.rpm

New candidate:

- Remove GPU support with new flag: --disable-use-nvml
  Unfortunately the GPU support is strongly coupled with proprietary bits.

This feature is a brand new patch created by upstream here: https://issues.apache.org/jira/browse/MESOS-9978
Many thanks Benjamin Bannier from Apache Mesos for the assistance.

This new package is functional tested with success.

Comment 22 Tom "spot" Callaway 2019-09-25 13:50:37 UTC
Awesome. With that change (and that flag passed to configure), the only remaining issue is to make a "clean" mesos source tarball that does not include the nvml tarball under 3rdparty/ and use that in our SRPM.

Comment 24 Tom "spot" Callaway 2019-09-26 17:21:56 UTC
(In reply to Javi Roman from comment #23)
> Spec URL:
> https://src.fedoraproject.org/fork/jromanes/rpms/mesos/raw/f30/f/mesos.spec 
> SRPM URL:
> https://github.com/javiroman/mesos-rpm/raw/master/mesos-1.8.1-4.fc30.src.rpm
> 
> New candidate:
> 
> - Removed non-free source code from upstream tarball based on Fedora
> guidelines:
>  
> https://fedoraproject.org/wiki/Packaging:
> SourceURL#When_Upstream_uses_Prohibited_Code

The script to generate the clean tarball is fine, but this removal is not related to patents, so the comment in the spec (as well as the naming schema for the clean tarball) is incorrect. I would suggest:

Source0: %{name}-%{version}-clean.tar.gz
# The upstream Apache Mesos source release
# https://dist.apache.org/repos/dist/release/mesos/<version>/mesos-<version>.tar.gz
# contains non-free code that we cannot ship. Therefore we use
# this script to remove the non-free code before shipping it.

Comment 25 Simone Caronni 2019-09-26 18:48:03 UTC
(In reply to Javi Roman from comment #20)
> Spec URL:
> https://src.fedoraproject.org/fork/jromanes/rpms/mesos/raw/f30/f/mesos.spec 
> SRPM URL:
> https://github.com/javiroman/mesos-rpm/raw/master/mesos-1.8.1-2.fc30.src.rpm
> 
> New candidate:
> 
> - %pom_remove_plugin :maven-gpg-plugin did the trick (many thanks @Fabio)
> - maven-site-plugin removed without problems, this was my fault, it's not a
> dependency at all.

All good on my side.

> Right now I'm checking with f31 too, however it looks zookeeper package is
> no ready yet (and it's a dependency):
> https://apps.fedoraproject.org/packages/zookeeper/

I can help you with zookeeper, it seems the last update was applied 2 years ago, we can probably push some changes.

(In reply to Tom "spot" Callaway from comment #24)
> The script to generate the clean tarball is fine, but this removal is not
> related to patents, so the comment in the spec (as well as the naming schema
> for the clean tarball) is incorrect. I would suggest:
> 
> Source0: %{name}-%{version}-clean.tar.gz
> # The upstream Apache Mesos source release
> #
> https://dist.apache.org/repos/dist/release/mesos/<version>/mesos-<version>.
> tar.gz
> # contains non-free code that we cannot ship. Therefore we use
> # this script to remove the non-free code before shipping it.

@Javi If you fix the comment with the above I would say the package is approved.

Comment 26 Simone Caronni 2019-09-26 18:48:43 UTC
I've sponsored you in FAS. I'll be away for the weekend, so I will follow up on monday.

Comment 27 Tom "spot" Callaway 2019-09-26 19:49:15 UTC
Lifting FE-Legal, since the legal issue is addressed (please do make the change I requested in Comment #24 though).

Comment 28 Javi Roman 2019-09-27 11:00:29 UTC
Spec URL: https://src.fedoraproject.org/fork/jromanes/rpms/mesos/raw/f30/f/mesos.spec 
SRPM URL: https://github.com/javiroman/mesos-rpm/raw/master/mesos-1.8.1-5.fc30.src.rpm
Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=37890546

Added your suggestion @Tom, many thanks! 

Many thanks @Simone for your support!

Comment 29 Fedora Update System 2019-10-11 08:06:24 UTC
FEDORA-2019-eba25d6ddf has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-eba25d6ddf

Comment 30 Fedora Update System 2019-10-12 02:02:24 UTC
mesos-1.8.1-5.fc30 has been pushed to the Fedora 30 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-2019-eba25d6ddf

Comment 31 Fedora Update System 2019-10-19 17:41:27 UTC
mesos-1.8.1-5.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.


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