[Bug 958131] Review Request: mysql-community - rename community-mysql to mysql-community

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

 



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

--- Comment #5 from Honza Horak <hhorak@xxxxxxxxxx> ---
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable


Issues:
=======
- There are some parts of innodb code under BSD:
  plugin/innodb_memcached/daemon_memcached/daemon/daemon.c
  so I believe BSD should be included in the spec file.
  Then BSD license file should be included in the packages as well

- /usr/share, /usr/libexec, /usr/include should use macros

- It does, but it is expected. It will allways conflicts with MariaDB or
community-mysql packages.

- Having package with mysql in the beginning means yum will prioritize it
before mariadb in case we require mysql as a virtual provide, because
mysql-community has a common prefix with mysql virtual name.
  http://yum.baseurl.org/wiki/CompareProviders
  The discussion took place here:
  https://lists.fedoraproject.org/pipermail/devel/2013-April/181220.html
  https://lists.fedoraproject.org/pipermail/devel/2013-May/182132.html
  It means that by installing packages requiring "mysql" (currently at least
mysqludf_xql and mysqltuner) will install mysql-community instead of mariadb if
none has been installed before.
  For me personally, this is a problem that shouldn't be just omitted. It
should be consulted widely on Fedora devel-list to find out if we are ok with
such behaviour and just document it or we will rather stick with
community-mysql name of the package..

- I believe that conditioned Preset macros are not needed any more:
  -%if 0%{?systemd_post:1}
  -%systemd_post mysqld.service
  -%else
  -if [ $1 = 1 ]; then
  -    # Initial installation
  -    /bin/systemctl daemon-reload >/dev/null 2>&1 || :
  -fi
  -%endif
  +%systemd_post mysqld.service

- install and cp commands could include -p argument

- package installs /lib/tmpfiles.d/mysql.conf but it should install
/lib/tmpfiles.d/%{name}.conf


===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

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]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: 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]: Fully versioned dependency in subpackages, if present.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in mysql-
     community-libs , mysql-community-common , mysql-community-server , mysql-
     community-embedded , mysql-community-embedded-devel
[x]: Package complies to the Packaging Guidelines
[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "BSD (4 clause) ISC", "LGPL (v2)", "BSD (2 clause) GPL (v2)", "GPL (v2)
     (with incorrect FSF address)", "GPL (v3 or later)", "Unknown or
     generated", "BSD (4 clause)", "GPL", "zlib/libpng GPL (v2)", "GPL (v2 or
     later)", "zlib/libpng", "ISC", "BSD (3 clause) GPL (v2)", "BSD (3
     clause)", "BSD (2 clause)", "GPL (v2)", "Public domain GPL (v2)". 1081
     files have unknown license. Detailed output of licensecheck in
     /home/hhorak/Downloads/mysql56review/review-mysql-
     community/licensecheck.txt

- There are some parts of innodb code under BSD:
  plugin/innodb_memcached/daemon_memcached/daemon/daemon.c
  so I believe BSD should be included in the spec file.
  Then BSD license file should be included in the packages as well

[x]: License file installed when any subpackage combination is installed.
[!]: Package consistently uses macro is (instead of hard-coded directory
     names).

- /usr/share, /usr/libexec, /usr/include should use macros

[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.

- It does, but it is expected. It will allways conflicts with MariaDB or
community-mysql packages.

[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.

- Seems to comply Renaming guidelines with
 
http://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.2FReplacing_Existing_Packages

[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.
[x]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 245760 bytes in 25 files.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: %config files are marked noreplace or the reason is justified.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[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).

Java:
[x]: Bundled jar/class files should be removed before build

Maven:
[-]: If package contains pom.xml files install it (including depmaps) even
     when building with ant
[x]: Old add_to_maven_depmap macro is not being used

Perl:
[x]: Package contains the mandatory BuildRequires and Requires:.
     Note: Requires: perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo
     $version)) missing?

Python:
[-]: Python eggs must not download any dependencies during the build process.
[-]: A package which is used by another package via an egg interface should
     provide egg info.
[-]: Package meets the Packaging Guidelines::Python
[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.
[!]: Final provides and requires are sane (see attachments).

- Having package with mysql in the beginning means yum will prioritize it
before mariadb in case we require mysql as a virtual provide, because
mysql-community has a common prefix with mysql virtual name.
  http://yum.baseurl.org/wiki/CompareProviders
  The discussion took place here:
  https://lists.fedoraproject.org/pipermail/devel/2013-April/181220.html
  https://lists.fedoraproject.org/pipermail/devel/2013-May/182132.html
  It means that by installing packages requiring "mysql" (currently at least
mysqludf_xql and mysqltuner) will install mysql-community instead of mariadb if
none has been installed before.

[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.
[!]: Scriptlets must be sane, if used.

- I believe that conditioned Preset macros are not needed any more:
  -%if 0%{?systemd_post:1}
  -%systemd_post mysqld.service
  -%else
  -if [ $1 = 1 ]; then
  -    # Initial installation
  -    /bin/systemctl daemon-reload >/dev/null 2>&1 || :
  -fi
  -%endif
  +%systemd_post mysqld.service

[x]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[!]: %check is present and all tests pass.

- test suite is run outside %check

[!]: Packages should try to preserve timestamps of original installed files.

- install and cp commands could include -p argument

[!]: Files in /run, var/run and /var/lock uses tmpfiles.d when appropriate

- package installs /lib/tmpfiles.d/mysql.conf but it should install
/lib/tmpfiles.d/%{name}.conf

[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[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]: Uses parallel make.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define.

Java:
[-]: Packages are noarch unless they use JNI
     Note: mysql-community subpackage is not noarch. Please verify manually
[-]: Package uses upstream build method (ant/maven/etc.)

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

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
     Note: Arch-ed rpms have a total of 184494080 bytes in /usr/share 1710080
     mysql-community-common-5.6.12-1.fc19.x86_64.rpm 178155520 mysql-
     community-test-5.6.12-1.fc19.x86_64.rpm 30720 mysql-community-
     libs-5.6.12-1.fc19.x86_64.rpm 2785280 mysql-community-
     bench-5.6.12-1.fc19.x86_64.rpm 40960 mysql-community-
     devel-5.6.12-1.fc19.x86_64.rpm 30720 mysql-community-
     embedded-5.6.12-1.fc19.x86_64.rpm 51200 mysql-community-embedded-
     devel-5.6.12-1.fc19.x86_64.rpm 163840 mysql-
     community-5.6.12-1.fc19.x86_64.rpm 1525760 mysql-community-
     server-5.6.12-1.fc19.x86_64.rpm

- caused by large test-suite packaged as a separate sub-package, so not seem
like a problem

[x]: Rpmlint is run on all installed packages.

- See in-line comments, but no blockers.

Rpmlint
-------
Checking: mysql-community-5.6.12-1.fc19.x86_64.rpm
          mysql-community-libs-5.6.12-1.fc19.x86_64.rpm
          mysql-community-common-5.6.12-1.fc19.x86_64.rpm
          mysql-community-server-5.6.12-1.fc19.x86_64.rpm
          mysql-community-devel-5.6.12-1.fc19.x86_64.rpm
          mysql-community-embedded-5.6.12-1.fc19.x86_64.rpm
          mysql-community-embedded-devel-5.6.12-1.fc19.x86_64.rpm
          mysql-community-bench-5.6.12-1.fc19.x86_64.rpm
          mysql-community-test-5.6.12-1.fc19.x86_64.rpm
mysql-community.x86_64: W: spelling-error %description -l en_US multi -> mulch,
mufti
mysql-community.x86_64: W: spelling-error %description -l en_US mysqld ->
myself
mysql-community.x86_64: W: obsolete-not-provided mysql-cluster
mysql-community.x86_64: W: only-non-binary-in-usr-lib
mysql-community-common.x86_64: W: no-documentation
mysql-community-server.x86_64: W: spelling-error %description -l en_US multi ->
mulch, mufti
mysql-community-server.x86_64: E: incoherent-logrotate-file
/etc/logrotate.d/mysqld
mysql-community-server.x86_64: E: non-root-user-log-file /var/log/mysqld.log
mysql
mysql-community-server.x86_64: E: non-root-group-log-file /var/log/mysqld.log
mysql
mysql-community-server.x86_64: E: non-ghost-file /var/log/mysqld.log
mysql-community-server.x86_64: E: zero-length /var/log/mysqld.log

- Expected, we need mysqld.log to be created during install (but not changed
nor verified using checksum, etc.), since it is not included in a place
writable for mysql user.

mysql-community-devel.x86_64: W: spelling-error %description -l en_US multi ->
mulch, mufti
mysql-community-devel.x86_64: W: only-non-binary-in-usr-lib
mysql-community-devel.x86_64: E: incorrect-fsf-address
/usr/include/mysql/byte_order_generic_x86_64.h
mysql-community-devel.x86_64: E: incorrect-fsf-address
/usr/include/mysql/byte_order_generic_x86.h
mysql-community-devel.x86_64: E: incorrect-fsf-address
/usr/include/mysql/little_endian.h
mysql-community-devel.x86_64: E: incorrect-fsf-address
/usr/include/mysql/byte_order_generic.h
mysql-community-devel.x86_64: E: incorrect-fsf-address
/usr/include/mysql/big_endian.h

- should be reported/fixed upstream

mysql-community-embedded.x86_64: W: spelling-error Summary(en_US) embeddable ->
embedded
mysql-community-embedded.x86_64: W: spelling-error %description -l en_US multi
-> mulch, mufti
mysql-community-embedded-devel.x86_64: W: spelling-error Summary(en_US)
embeddable -> embedded
mysql-community-embedded-devel.x86_64: W: spelling-error %description -l en_US
multi -> mulch, mufti
mysql-community-embedded-devel.x86_64: W: only-non-binary-in-usr-lib
mysql-community-bench.x86_64: W: spelling-error %description -l en_US multi ->
mulch, mufti
mysql-community-bench.x86_64: W: spelling-error %description -l en_US
benchmarking -> bench marking, bench-marking, benchmark
mysql-community-bench.x86_64: E: script-without-shebang
/usr/share/sql-bench/graph-compare-results
mysql-community-test.x86_64: W: spelling-error %description -l en_US multi ->
mulch, mufti
mysql-community-test.x86_64: W: pem-certificate
/usr/share/mysql-test/std_data/untrusted-cacert.pem
mysql-community-test.x86_64: W: pem-certificate
/usr/share/mysql-test/std_data/ca-sha512.pem
mysql-community-test.x86_64: W: hidden-file-or-dir
/usr/share/mysql-test/std_data/.mylogin.cnf
mysql-community-test.x86_64: E: zero-length
/usr/share/mysql-test/suite/parts/t/disabled.def
mysql-community-test.x86_64: W: pem-certificate
/usr/share/mysql-test/std_data/server-cert-sha512.pem
mysql-community-test.x86_64: W: pem-certificate
/usr/share/mysql-test/std_data/crl-ca-cert.pem
mysql-community-test.x86_64: E: zero-length
/usr/share/mysql-test/std_data/bug37631.MYD
mysql-community-test.x86_64: W: pem-certificate
/usr/share/mysql-test/std_data/cacert.pem
mysql-community-test.x86_64: E: zero-length
/usr/share/mysql-test/std_data/cluster_7022_table.MYD
mysql-community-test.x86_64: E: zero-length
/usr/share/mysql-test/collections/disabled-weekly.list
mysql-community-test.x86_64: W: pem-certificate
/usr/share/mysql-test/std_data/server8k-cert.pem
mysql-community-test.x86_64: W: pem-certificate
/usr/share/mysql-test/std_data/client-cert.pem
mysql-community-test.x86_64: W: pem-certificate
/usr/share/mysql-test/std_data/crl-client-revoked-cert.pem
mysql-community-test.x86_64: W: pem-certificate
/usr/share/mysql-test/std_data/crl-client-cert.pem
mysql-community-test.x86_64: W: pem-certificate
/usr/share/mysql-test/std_data/server-cert.pem
mysql-community-test.x86_64: E: zero-length
/usr/share/mysql-test/collections/disabled-daily.list
mysql-community-test.x86_64: W: pem-certificate
/usr/share/mysql-test/std_data/crl-server-cert.pem
mysql-community-test.x86_64: W: no-manual-page-for-binary my_safe_process
9 packages and 0 specfiles checked; 16 errors, 29 warnings.

-- 
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=zmNWscpAyK&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]