https://bugzilla.redhat.com/show_bug.cgi?id=1302909 --- Comment #13 from Shawn Iwinski <shawn@xxxxxxxx> --- Spec URL: https://raw.githubusercontent.com/siwinski/rpms/0f6e8cf814b7dc5205694f88071b91d71735a616/drupal8/drupal8.spec SRPM URL: https://siwinski.fedorapeople.org/SRPMS/drupal8-8.1.8-2.fc24.src.rpm Changes: https://github.com/siwinski/rpms/commit/0f6e8cf814b7dc5205694f88071b91d71735a616 Copr build: https://copr.fedorainfracloud.org/coprs/siwinski/drupal8/build/438545/ (In reply to Jared Smith from comment #11) > I know this is a very complicated package, and I appreciate the hard work > you're putting into getting this packaged. I think we're getting fairly > close on the package review -- but there are still a few outstanding items I > would like to see addressed before I approve the package. Feel free to ask > if you have any questions. I've highlighted the major items in the "Issues" > section, and then marked individual items need attention with an exclamation > mark in the checklist below. Inline responses below... > Issues: > ======= > - License field doesn't include the MPL license, but it seems that > ckeditor is licensed under either the MPL or LGPL or GPL license. > See core/assets/vendor/ckeditor/LICENSE.md. This means that the > license field should more accurately be: > GPLv2+ and MIT and Public Domain and (GTLv2+ or MPLv1.1+ or LGPLv2.1+) Fixed in latest spec release. Should I add the license breakdown to %description as well? Personally, I think that would be useful to end-users but I don't remember seeing any other pkg do that. > - Make sure that if someone installs just the drupal8-rpmbuild subpackage, > that a license file still gets installed. Added license in latest spec release. > - You probably (but don't necessarily) want to set the sample config files > as %config(noreplace) instead of just %config. This way, if someone > edits the sample configs and then upgrades Drupal, their changes won't > get replaced by the updated config file from the upgrade. See inline notes below. Overall, IMO they are to remain upstream managed and replaced on RPM update. I will make note of these in a RPM README that I still need to create. > - There are still a lot of errors coming from rpmlint. In particular, there > are still a lot of zero-length files. If they're really needed, I'd be > willing to give an exception on a case-by-case basis, but I don't think we > really need that many zero-length files. There also seem to be a lot of > scripts that have "/usr/bin/env php" or similar in the shebang line, which > will need to be patched/fixed with sed to "/usr/bin/php", etc. See inline notes below. > Rpmlint > ------- > Checking: drupal8-8.1.8-1.fc26.noarch.rpm > drupal8-httpd-8.1.8-1.fc26.noarch.rpm > drupal8-rpmbuild-8.1.8-1.fc26.noarch.rpm > drupal8-8.1.8-1.fc26.src.rpm > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.xtmpl > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.module.swp > drupal8.noarch: E: zero-length > /usr/share/doc/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > composer.lock > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.inc > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.twig > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.module.save > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.module.bak > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.php.swp > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.yml > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.php.bak > drupal8.noarch: E: backup-file-in-package > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.php.orig > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.php.orig > drupal8.noarch: E: zero-length > /usr/share/doc/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > composer.json > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.sh > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.install > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.module > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.php.swo > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.sql > drupal8.noarch: E: backup-file-in-package > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.module~ > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.module~ > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.tpl.php > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.module.swo > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.php.save > drupal8.noarch: E: backup-file-in-package > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.php~ > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.php~ > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.profile > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.php-info.txt > drupal8.noarch: E: backup-file-in-package > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.module.orig > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.module.orig > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.po > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.theme > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.engine > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/system/tests/fixtures/HtaccessTest/ > access_test.make All of these are required for the .htaccess test. The files are required for their file extensions and zero-length on purpose so they do not take up any space (in the repo or on disk when installed). I did find the following 2 files which were marked as %doc but needed to be in /usr/share/drupal8 though (taken care of in latest spec release): - /usr/share/doc/drupal8/core/modules/system/tests/fixtures/HtaccessTest/composer.json - /usr/share/doc/drupal8/core/modules/system/tests/fixtures/HtaccessTest/composer.lock > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/themes/stark/css/layout.css Global styling referenced in "core/themes/stark/stark.libraries.yml". Would 404 for users using this theme if it was removed. > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/simpletest/files/translations/drupal-8.0.0- > beta2.hu.po > drupal8.noarch: E: zero-length > /usr/share/drupal8/core/modules/simpletest/files/translations/drupal-8.0.0. > de.po I believe these are used by the simpletest module for tests > drupal8.noarch: E: zero-length > /usr/share/licenses/drupal8/core/lib/Drupal/Component/Transliteration/ > LICENSE.txt > drupal8.noarch: E: zero-length > /usr/share/licenses/drupal8/core/lib/Drupal/Component/Serialization/LICENSE. > txt Removed in new spec release. > drupal8.noarch: W: conffile-without-noreplace-flag > /etc/drupal8/sites/example.sites.php > drupal8.noarch: W: conffile-without-noreplace-flag > /etc/drupal8/sites/default/default.settings.php > drupal8.noarch: W: conffile-without-noreplace-flag > /etc/drupal8/sites/example.settings.local.php > drupal8.noarch: W: conffile-without-noreplace-flag > /etc/drupal8/sites/default/default.services.yml IMO, these are managed upstream and should not be modified by end-users as they are just samples/defaults upstream. Therefore I left them as just %config on purpose so an RPM update would replace them with the upstream-managed version. End-users would copy these to their real filenames and make edits to those files. I will make note of this in a RPM README that I still need to create. > drupal8-httpd.noarch: W: conffile-without-noreplace-flag > /etc/httpd/conf.d/drupal8.htaccess > drupal8-httpd.noarch: W: conffile-without-noreplace-flag > /etc/httpd/conf.d/drupal8.no-access IMO these are managed upstream as well. If end-users want to make edits they should create their own files and modify their "drupal.conf" %config(noreplace) file to include their custom file instead of these. I will make note of this in a custom file header (taken care of in latest spec release) and in a RPM README that I still need to create. > drupal8.noarch: E: wrong-script-interpreter > /usr/share/drupal8/core/scripts/db-tools.php /usr/bin/env php > drupal8.noarch: E: wrong-script-interpreter > /usr/share/drupal8/core/scripts/password-hash.sh /usr/bin/env php > drupal8.noarch: E: wrong-script-interpreter > /usr/share/drupal8/core/scripts/dump-database-d6.sh /usr/bin/env php > drupal8.noarch: E: wrong-script-interpreter > /usr/share/drupal8/core/scripts/rebuild_token_calculator.sh /usr/bin/env php > drupal8.noarch: E: wrong-script-interpreter > /usr/share/drupal8/core/scripts/drupal.sh /usr/bin/env php > drupal8.noarch: E: wrong-script-interpreter > /usr/share/drupal8/core/scripts/dump-database-d7.sh /usr/bin/env php > drupal8.noarch: E: wrong-script-interpreter > /usr/share/drupal8/core/scripts/dump-database-d8-mysql.php /usr/bin/env php > drupal8.noarch: E: wrong-script-interpreter > /usr/share/drupal8/core/scripts/generate-d6-content.sh /usr/bin/env php > drupal8.noarch: E: wrong-script-interpreter > /usr/share/drupal8/core/scripts/generate-d7-content.sh /usr/bin/env php > drupal8.noarch: E: wrong-script-interpreter > /usr/share/drupal8/core/scripts/generate-proxy-class.php /usr/bin/env php > drupal8-rpmbuild.noarch: E: wrong-script-interpreter > /usr/lib/rpm/drupal8-find-requires.php /usr/bin/env php > drupal8-rpmbuild.noarch: E: wrong-script-interpreter > /usr/lib/rpm/drupal8-prep-licenses-and-docs.sh /usr/bin/env bash > drupal8-rpmbuild.noarch: E: wrong-script-interpreter > /usr/lib/rpm/drupal8-find-provides.php /usr/bin/env php > drupal8-rpmbuild.noarch: E: wrong-script-interpreter > /usr/lib/rpm/drupal8-get-dev-source.sh /usr/bin/env bash See comment from Remi. We have been using this in several PHP libraries already to allow for SCL. > drupal8-httpd.noarch: E: non-standard-dir-perm > /var/lib/drupal8/files/public/default 775 > drupal8-httpd.noarch: E: non-standard-dir-perm > /var/lib/drupal8/files/private/default 775 > drupal8.src: W: strange-permission drupal8-get-dev-source.sh 755 > drupal8.src: W: strange-permission drupal8-find-requires.php 775 > drupal8.src: W: strange-permission drupal8-find-provides.php 775 > drupal8.src: W: strange-permission drupal8-prep-licenses-and-docs.sh 775 These annoy me :/ How are these permissions non-standard? I will eventually ask on the packaging mailing list. > drupal8.noarch: E: explicit-lib-dependency php-zlib Common rpmlint "error" for php extension pkg > drupal8.noarch: W: dangling-symlink /etc/drupal8/sites/default/files > /var/lib/drupal8/files/public/default This one should make sense :) > drupal8-httpd.noarch: W: no-documentation Documentation is provided by the main drupal8 pkg which is a requirement of this drupal8-httpd subpackage. > drupal8-rpmbuild.noarch: W: only-non-binary-in-usr-lib > drupal8-rpmbuild.noarch: W: no-documentation I will create a README at the same time I create an RPM README. > drupal8.src: W: spelling-error %description -l en_US Drupal -> Drupe :/ -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx