Hide Forgot
SPEC URL: http://rebus.fedorapeople.org/12/SPECS/whatweb.spec SRPM URL: http://rebus.fedorapeople.org/12/SRPMS/whatweb-0.4.2-1.fc12.src.rpm Description: Identify content management systems (CMS), blogging platforms, stats/analytic packages, JavaScript libraries, servers and more. When you visit a website in your browser the transaction includes many unseen hints about how the web-server is set up and what software is delivering the web-page. Some of these hints are obvious, ex. “Powered by XYZ” and others are more subtle. WhatWeb recognizes these hints and reports what it finds. Hello, Please could you review the package whatweb? Output from rpmlint: $ rpmlint whatweb-0.4.2-1.fc12.noarch.rpm whatweb-0.4.2-1.fc12.src.rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings. Koji F12: http://koji.fedoraproject.org/koji/taskinfo?taskID=2154469 Koji F13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2154471
Add dep on ruby: $ whatweb /usr/bin/env: ruby: No such file or directory
You should package anemone seperately: http://anemone.rubyforge.org/
(In reply to comment #1) > Add dep on ruby: Strange - I wanted to add dependency by following guideline for ruby. Apparently it was not enough adding the: Requires: ruby(abi) = 1.8 Thank you Terje for noticing that - I will add file dependency for ruby binary as well. SPEC URL: http://rebus.fedorapeople.org/12/SPECS/whatweb.spec SRPM URL: http://rebus.fedorapeople.org/12/SRPMS/whatweb-0.4.2-2.fc12.src.rpm (In reply to comment #2) > You should package anemone seperately: > http://anemone.rubyforge.org/ Original anemone project has got some extra dependencies. Version present in whatweb is standalone patched version of anemone without some dependencies. Best regards. Michal Ambroz
(In reply to comment #3) > (In reply to comment #1) > (In reply to comment #2) > > You should package anemone seperately: > > http://anemone.rubyforge.org/ > Original anemone project has got some extra dependencies. > Version present in whatweb is standalone patched version of anemone without > some dependencies. So please explain why you want to avoid additional dependency for this package. I don't think that so huge additonal dependency will be added. Using copy of external project is generally forbidden on Fedora and it should be packaged: https://fedoraproject.org/wiki/Packaging/Guidelines#Duplication_of_system_libraries https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
As written on the above URLs, one of the biggest reasons is that when a new version of the package is released for (security) bug fixes or so, it gets very hard for us to track if such bug fixes are also applied to ones internally bundled in other packages.
Hello Mamoru, thank you for your comments. I will consider packing anemone and patching the whatweb to use sytem library of anemone. (In reply to comment #4) > https://fedoraproject.org/wiki/Packaging/Guidelines#Duplication_of_system_libraries I believe this link is not relevant as there is currenlty no anemone library in Fedora. >https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries However according this there probably should be one. Best regards Michal Ambroz
SPEC URL: http://rebus.fedorapeople.org/13/SPECS/whatweb.spec SRPM URL: http://rebus.fedorapeople.org/13/SRPMS/whatweb-0.4.5-1.fc13.src.rpm Rebuild of new version 0.4.5 of whatweb. I still didn't manage to patch to separate anemone library from whatweb. Michal Ambroz
(In reply to comment #7) > I still didn't manage to patch to separate anemone library from whatweb. > First please submit rubygem-anemome review request. Then I guess replacing ------------------------------------------------ require 'lib/anemone/anemone.rb' ------------------------------------------------ in whatweb script to ------------------------------------------------ require 'rubygems' require 'anemone' ------------------------------------------------ should work (if anemone 0.4.0 still supports anemone 0.2.0 API: anemone bundled in whatweb 0.4.5 is 0.2.0, while the latest anemone is 0.4.0)
ping?
U have submitted for review package rubygems-robots, which is needed for rubygems-anemone. Bug 632912.
U have submitted for review package rubygems-shoulda, which is needed for rubygems-robots, which is needed for rubygems-anemone. Bug 632917.
I have submitted for review package rubygems-anemone, which is needed for whatweb. Bug 632919
SPEC URL: http://rebus.fedorapeople.org/SPECS/whatweb.spec SRPM URL: http://rebus.fedorapeople.org/SRPMS/whatweb-0.4.5-2.fc13.src.rpm Patch to use system-wide rubygems anemone library rather than local copy of anemone library. Koji F13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2462168 $ rpmlint whatweb-0.4.5-2.fc13.src.rpm whatweb-0.4.5-2.fc13.noarch.rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings. Best regards Michal Ambroz
Whatweb doesn't work with the upstream anemone version. Issue reported to whatweb developer, waiting for fix.
This message is a reminder that Fedora 12 is nearing its end of life. Approximately 30 (thirty) days from now Fedora will stop maintaining and issuing updates for Fedora 12. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as WONTFIX if it remains open with a Fedora 'version' of '12'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version prior to Fedora 12's end of life. Bug Reporter: Thank you for reporting this issue and we are sorry that we may not be able to fix it before Fedora 12 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora please change the 'version' of this bug to the applicable version. If you are unable to change the version, please add a comment here and someone will do it for you. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete. The process we are following is described here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping
New version of whatweb released. This version still requires embedded old patched version of anemone. http://rebus.fedorapeople.org/RPMS/whatweb-0.4.6-1.fc14.noarch.rpm http://rebus.fedorapeople.org/SPECS/whatweb.spec http://rebus.fedorapeople.org/SRPMS/whatweb-0.4.6-1.fc14.src.rpm Michal Ambroz
New version of whatweb released. This version requires embedded old patched version of anemone. http://rebus.fedorapeople.org/SPECS/whatweb.spec http://rebus.fedorapeople.org/RPMS/whatweb-0.4.7-1.fc14.noarch.rpm http://rebus.fedorapeople.org/SRPMS/whatweb-0.4.7-1.fc14.src.rpm
I am sorry - It is not in my powers to separate the anemone library from the package. The package works for me as is and separation is not supported by the upstream. I am stepping down from pushing this package to Fedora. Best regards Michal Ambroz
Reopening the package review request. It seems that the whatweb is in next upcoming version dropping the support for the anemone library as whole. http://rebus.fedorapeople.org/SPECS/whatweb.spec http://rebus.fedorapeople.org/RPMS/whatweb-0.4.8-0.git20120708.1.fc17.noarch.rpm http://rebus.fedorapeople.org/SRPMS/whatweb-0.4.8-0.git20120708.1.fc17.src.rpm Michal Ambroz
Package Review ============== Key: - = N/A x = Pass ! = Fail ? = Not evaluated ==== Generic ==== [x]: EXTRA Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: EXTRA Spec file according to URL is the same as in SRPM. [x]: MUST Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: MUST Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [ ]: MUST %build honors applicable compiler flags or justifies otherwise. [x]: MUST All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [ ]: MUST Package contains no bundled libraries. [ ]: MUST Changelog in prescribed format. [ ]: MUST Sources contain only permissible code or content. [!]: MUST Each %files section contains %defattr if rpm < 4.4 Note: defattr(....) present in %files section. This is OK if packaging for EPEL5. Otherwise not needed [ ]: MUST Macros in Summary, %description expandable at SRPM build time. [ ]: MUST Package contains desktop file if it is a GUI application. [ ]: MUST Development files must be in a -devel package [ ]: MUST Package requires other packages for directories it uses. [ ]: MUST Package uses nothing in %doc for runtime. [ ]: MUST Package is not known to require ExcludeArch. [x]: MUST Permissions on files are set properly. [x]: MUST Package does not contain duplicates in %files. [ ]: MUST Package complies to the Packaging Guidelines [x]: MUST Spec file lacks Packager, Vendor, PreReq tags. [!]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. Note: rm -rf is only needed if supporting EPEL5. This is acceptable if you plan to package for EPEL5 [ ]: MUST Large documentation files are in a -doc subpackage, if required. [x]: MUST 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 %doc. [x]: MUST License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "GPL" For detailed output of licensecheck (see attachment) [x]: MUST Package consistently uses macro is (instead of hard-coded directory names). [x]: MUST Package is named using only allowed ascii characters. [x]: MUST Package is named according to the Package Naming Guidelines. [ ]: MUST Package does not generate any conflict. Note: Package contains no Conflicts: tag(s) [ ]: MUST Package obeys FHS, except libexecdir and /usr/target. [ ]: MUST If the package is a rename of another package, proper Obsoletes and Provides are present. [ ]: MUST Package must own all directories that it creates. [ ]: MUST Package does not own files or directories owned by other packages. [x]: MUST Package installs properly. [ ]: MUST Package is not relocatable. [ ]: MUST Requires correct, justified where necessary. [x]: MUST Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [!]: MUST Sources used to build the package match the upstream source, as provided in the spec URL. Note: See below. [x]: MUST Spec file is legible and written in American English. [x]: MUST Spec file name must match the spec package %{name}, in the format %{name}.spec. [ ]: MUST Package contains systemd file(s) if in need. [x]: MUST File names are valid UTF-8. [x]: SHOULD Reviewer should test that the package builds in mock. [!]: SHOULD Buildroot is not present Note: Buildroot is not needed unless packager plans to package for EPEL5 [!]: SHOULD Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) Note: Clean is needed only if supporting EPEL5 [ ]: SHOULD 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]: SHOULD Dist tag is present. [x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [ ]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q --requires). [ ]: SHOULD Package functions as described. [ ]: SHOULD Latest version is packaged. [ ]: SHOULD Package does not include license text files separate from upstream. [ ]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise justified. [x]: SHOULD SourceX tarball generation or download is documented. [!]: SHOULD SourceX / PatchY prefixed with %{name}. Note: Source0 (urbanadventurer-WhatWeb-1dc2cff.tar.gz) [x]: SHOULD SourceX is a working URL. [ ]: SHOULD Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [ ]: SHOULD Package should compile and build into binary rpms on all supported architectures. [ ]: SHOULD %check is present and all tests pass. [ ]: SHOULD Packages should try to preserve timestamps of original installed files. [!]: SHOULD Spec use %global instead of %define. Note: %define gituser urbanadventurer %define gitname WhatWeb %define gitversion 1dc2cff Issues: [!]: MUST Each %files section contains %defattr if rpm < 4.4 Note: defattr(....) present in %files section. This is OK if packaging for EPEL5. Otherwise not needed See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions [!]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. Note: rm -rf is only needed if supporting EPEL5 See: None [!]: MUST Sources used to build the package match the upstream source, as provided in the spec URL. Note: Upstream MD5sum check error. See: http://fedoraproject.org/wiki/Packaging/SourceURL Rpmlint ------- Checking: whatweb-0.4.8-0.git20120708.1.fc16.src.rpm whatweb-0.4.8-0.git20120708.1.fc16.noarch.rpm whatweb.src:15: W: macro-in-comment %{name} whatweb.src:15: W: macro-in-comment %{version} whatweb.src: W: file-size-mismatch urbanadventurer-WhatWeb-1dc2cff.tar.gz = 1238667, https://github.com/urbanadventurer/WhatWeb/tarball/master/urbanadventurer-WhatWeb-1dc2cff.tar.gz = 1254743 2 packages and 0 specfiles checked; 0 errors, 3 warnings. Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output: Requires -------- whatweb-0.4.8-0.git20120708.1.fc16.noarch.rpm (rpmlib, GLIBC filtered): /bin/bash /bin/sh /usr/bin/env /usr/bin/ruby ruby(abi) >= 1.8 Provides -------- whatweb-0.4.8-0.git20120708.1.fc16.noarch.rpm: whatweb = 0.4.8-0.git20120708.1.fc16 MD5-sum check ------------- https://github.com/urbanadventurer/WhatWeb/tarball/master/urbanadventurer-WhatWeb-1dc2cff.tar.gz : CHECKSUM(SHA256) this package : fcedeb9373d104f9c09a23e66e2f2bdf892829d2a7b354112b190e6caabb0c4f CHECKSUM(SHA256) upstream package : 9efffec153c594c3a767dc81f0d97babdc62172d82d6bf69f4096d7744c19010 diff -r also reports differences Generated by fedora-review 0.2.2 (9f8c0e5) last change: 2012-08-09 Command line :/usr/bin/fedora-review -b=587978 External plugins:
Created attachment 607410 [details] License Check
Created attachment 607411 [details] md5sum
SPEC URL: http://rebus.fedorapeople.org/SPECS/whatweb.spec SRPM URL: http://rebus.fedorapeople.org/SRPMS/whatweb-0.4.8-0.git20150507.48b9682.1.fc21.src.rpm Bump to current git snapshot.
Ping?
SPEC URL: https://rebus.fedorapeople.org/SPECS/whatweb.spec SRPM URL: https://rebus.fedorapeople.org/SRPMS/whatweb-0.4.8-0.git20160611.1.fc23.src.rpm Bump to current git snapshot.
SPEC URL: https://rebus.fedorapeople.org/SPECS/whatweb.spec SRPM URL: https://rebus.fedorapeople.org/SRPMS/whatweb-0.4.9-1.fc27.src.rpm Bump to current upstream release. >[!]: MUST Each %files section contains %defattr if rpm < 4.4 defattr removed >[!]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the cleaning of the buildroot removed >[!]: MUST Sources used to build the package match the upstream source, as The release should MD5 match much better than git commit ad-hoc snapshot (please note that gzip contains timestamps so it might not be MD5 same). >[!]: SHOULD Buildroot is not present removed >[!]: SHOULD Package has no %clean section with rm -rf %{buildroot} (or removed >[!]: SHOULD SourceX / PatchY prefixed with %{name}. link to release source is now resulting in whatweb-0.4.9.tar.gz >[!]: SHOULD Spec use %global instead of %define. define changed to global Best regards Michal Ambroz
This is an automatic check from review-stats script. This review request ticket hasn't been updated for some time, but it seems that the review is still being working out by you. If this is right, please respond to this comment clearing the NEEDINFO flag and try to reach out the submitter to proceed with the review. If you're not interested in reviewing this ticket anymore, please clear the fedora-review flag and reset the assignee, so that a new reviewer can take this ticket. Without any reply, this request will shortly be resetted.
This is an automatic action taken by review-stats script. The ticket reviewer failed to clear the NEEDINFO flag in a month. As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews we reset the status and the assignee of this ticket.
I am sorry for the rough review experience of this package, even after the difficult issue of bundled dependencies got resolved. If you are still interested in getting WhatWeb packaged for Fedora, I can complete the review. If so, please update the spec and srpm to included the latest version so I can proceed to review. If you are not interested anymore, please either close this issue, or do nothing, in which case automation should close this issue in one month.
Hello Otto, no need to be sorry ... we are all just volunteers. And thank you if you would pick-up this review. Here is the current version : SPEC URL: http://rebus.fedorapeople.org/SPECS/whatweb.spec SRPM URL: http://rebus.fedorapeople.org/SRPMS/whatweb-0.5.5-1.fc33.src.rpm Here the scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=66434481 $ rpmlint whatweb-0.5.5-1.fc33.src.rpm whatweb-0.5.5-1.fc33.noarch.rpm whatweb.spec 2 packages and 1 specfiles checked; 0 errors, 0 warnings.
Reviewed, here are my findings. Some are clear and some just things I could not immediately understand. Fix the parts that you agree with and explain what is going on with others. I checked everything already, so after these are all addressed, the review should be clear. 1. What is the intent of fragment part #/%{name}-%{version}.tar.gz? It seems to me that it is simply ignored. If the idea is to indicate the name of downloaded file, a comment would do better? Also, the macros in the fragment do not resolve to the real filename from Content-Disposition http header at the moment, since %{name} != %{gitname} > Source0: https://github.com/%{gituser}/%{gitname}/archive/v%{version}.tar.gz#/%{name}-%{version}.tar.gz 2. Better %{_bindir}/ruby, since that is how rubypick package provides this > Requires: /usr/bin/ruby References: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_effect_of_the_usrmove_fedora_feature https://src.fedoraproject.org/rpms/rubypick/blob/rawhide/f/rubypick.spec 3. Fixing env shebangs should not be required in Fedora anymore. If this is still needed for some reason (RHEL perhaps?), comment should be updated to match > # Fedora using Rubypick > sed -i -e 's|#!/usr/bin/env ruby|#!/usr/bin/ruby|; s|#!/bin/env ruby|#!/usr/bin/ruby|;' \ whatweb plugin-development/find-common-stuff plugin-development/get-pattern Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/Ruby/#_shebang_lines 4. I do not understand this. Is this an issue with upstream man pages? If so, a fix or an issue should be submitted and referenced from the specfile. > # Unknown macros in manpage > sed -i -e 's|^.ni||; s|^\./plugins-disabled|+\./plugins-disabled|' whatweb.1 5. Is this still needed? PR282 has been merged before 0.5.5 was released, so it should be ok. Again, if this is an upstream issue, a bug report or pull request should be referenced from here. If Fedora-specific, the situation should be explained. > # Add the whatweb shared directory + PR282 > sed -i -e "s|expand_path(__dir__)), '.')|expand_path(__dir__)), '%{_datadir}/%{name}')|" whatweb 6. Are both this and the earlier sed call that commnents off 'bundle install' needed? Can bundler be disable with only one method? If this is really needed here, a comment explaining what is going on would be good. > alias bundle='echo' 7. This is not wrong, but could be handled with a single row %{_datadir}/%{name}/addons, which would own the directory and include it and all its content in one statement. The same goes for lib, plugins, my-plugins, plugin-development and plugin-disabled folders. Also, I wonder if a simple '%{_datadir}/%{name}' would correctly handle all this, is there something in there that you do not want to own & include? > %dir %{_datadir}/%{name}/addons > ... > %{_datadir}/%{name}/addons/* 8. There are tests in the source, but no %check in specfile. Tests should be run. If is it too difficult to get them run inside the buildsystem, then perhaps a %check section with commented off attempt and a comment explaining why they cannot be run? Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_test_suites 9. my-plugins and plugin-development folders look like material for plugin writing. Are they really needed at runtime? If not, they should not installed. 10. What about the shell scripts in addons folder? Are the intended to be run by the user? If so, they should be installed to %{_bindir}. If not, and are not otherwise needed at runtime, they should be dropped or perhaps moved to documentation. 11. Maybe a comment here explaining what is going on. Is it just that RHEL does not support Recommends? > %if 0%{?rhel} && 0%{?rhel} <= 8 > Requires: rubygem-bson > Requires: rubygem-mongo > %else > Recommends: rubygem-bson > Recommends: rubygem-mongo > %endif 12. Is the license really GPLv2 or is it GPLv2+? License is listed as such in upstream home page, but many (not all!) files like lib/logging.rb contain a notice that also allows any later version. You should probably contact upstream to clarify the issue.
(In reply to Otto Urpelainen from comment #31) SPEC URL: http://rebus.fedorapeople.org/SPECS/whatweb.spec SRPM URL: http://rebus.fedorapeople.org/SRPMS/whatweb-0.5.5-2.fc34.src.rpm > 1. What is the intent of fragment part #/%{name}-%{version}.tar.gz? > > Source0: https://github.com/%{gituser}/%{gitname}/archive/v%{version}.tar.gz#/%{name}-%{version}.tar.gz when you download with the spectool -g whatweb.spec, this is what renames boring v5.5.0.tar.gz to sexy whatweb-5.5.0.tar.gz > 2. Better %{_bindir}/ruby, since that is how rubypick package provides this > > Requires: /usr/bin/ruby Ok ... thanks > 3. Fixing env shebangs should not be required in Fedora anymore. If this is > still needed for some reason (RHEL perhaps?), comment should be updated to match > https://docs.fedoraproject.org/en-US/packaging-guidelines/Ruby/#_shebang_lines I consider env to be potential security problem and preffer to be explicit about the interpreter used in the packaged stuff. The guide is saying as well that it SHOULD use #!/usr/bin/ruby . The "env ruby" is not always only /usr/bin/ruby. Depending on environment settings it could be also /usr/local/bin/ruby or ~user/bin/ruby or even /tmp/you_have_been_hacked/ruby . > 4. I do not understand this. Is this an issue with upstream man pages? If > so, a fix or an issue should be submitted and referenced from the specfile. > > > # Unknown macros in manpage > > sed -i -e 's|^.ni||; s|^\./plugins-disabled|+\./plugins-disabled|' whatweb.1 Yes ... I guess that on Ubuntu they use different groff for formatting the man pages so it is ok for them. On Fedora it complains so I have to remove that tags. > 5. Is this still needed? PR282 has been merged before 0.5.5 was released, so > it should be ok. Again, if this is an upstream issue, a bug report or pull > request should be referenced from here. If Fedora-specific, the situation > should be explained. > > > # Add the whatweb shared directory + PR282 > > sed -i -e "s|expand_path(__dir__)), '.')|expand_path(__dir__)), '%{_datadir}/%{name}')|" whatweb Yes still needed. I do not consider this ustream bug, but it relies to Fedora packaging. > 6. Are both this and the earlier sed call that commnents off 'bundle install' needed? Nah ... Just the sed was working. The alias was not working I just forgot it there - thanks, removing the alias. > 7. This is not wrong, but could be handled with a single row > %{_datadir}/%{name}/addons, which would own the directory and include it and > all its content in one statement. The same goes for lib, plugins, > my-plugins, plugin-development and plugin-disabled folders. Also, I wonder > if a simple '%{_datadir}/%{name}' would correctly handle all this, is there > something in there that you do not want to own & include? At the time I was packaging I was probably trying to comply with the rule that all directories must be owned. So I was trying to explicitly list them. These days yes %{_datadir}/%{name} would do. > 8. There are tests in the source, but no %check in specfile. Tests should be > run. If is it too difficult to get them run inside the buildsystem, then > perhaps a %check section with commented off attempt and a comment explaining > why they cannot be run? Reference: > https://docs.fedoraproject.org/en-US/packaging-guidelines/#_test_suites During build there is no networking. The site used for tests - https://whatweb.net/ - is down. Adding conditional and comment to build with tests. > 9. my-plugins and plugin-development folders look like material for plugin > writing. Are they really needed at runtime? If not, they should not > installed. - my-plugins is meant for locally created plugins to separate them from the dozens of others. Its installed by upstream and searched for local libs - I do not want to change this. - moved plugin-development to documentation > 10. What about the shell scripts in addons folder? Are the intended to be > run by the user? If so, they should be installed to %{_bindir}. If not, and > are not otherwise needed at runtime, they should be dropped or perhaps moved > to documentation. - yes executables, but more like examples. Not core functionality. - moved to documentation > 11. Maybe a comment here explaining what is going on. > Is it just that RHEL does not support Recommends? Yes. RHEL7 doesn't know Recommends. Commented. > 12. Is the license really GPLv2 or is it GPLv2+? License is listed as such > in upstream home page, but many (not all!) files like lib/logging.rb contain > a notice that also allows any later version. You should probably contact > upstream to clarify the issue. yes gplv2+ ... changed in spec. doing that I have found in plugins ip2country.csv database with non-free (donationware) license from unreachable origin, changed spec to delete that one.
(In reply to Otto Urpelainen from comment #31) > 3. Fixing env shebangs should not be required in Fedora anymore. If this is > still needed for some reason (RHEL perhaps?), comment should be updated to > match > > > # Fedora using Rubypick > > sed -i -e 's|#!/usr/bin/env ruby|#!/usr/bin/ruby|; s|#!/bin/env ruby|#!/usr/bin/ruby|;' \ > whatweb plugin-development/find-common-stuff > plugin-development/get-pattern > > Reference: > https://docs.fedoraproject.org/en-US/packaging-guidelines/Ruby/ > #_shebang_lines > This does not say using env on shabang is permitted. The above guideline says when the package really needs jruby, you can write "#!/usr/bin/jruby" on shebang (but I don't think jruby on Fedora is well-maintained). All other should use "#!/usr/bin/ruby" on shebang. Anyway env should not be used on shebang.
Commenting only on items where I have something to all, for all others: Thank you for the explanation. (In reply to Michal Ambroz from comment #32) > (In reply to Otto Urpelainen from comment #31) > > > 3. Fixing env shebangs should not be required in Fedora anymore. If this is > > still needed for some reason (RHEL perhaps?), comment should be updated to match > > https://docs.fedoraproject.org/en-US/packaging-guidelines/Ruby/#_shebang_lines > > I consider env to be potential security problem and preffer to be explicit > about the interpreter used in the packaged stuff. > The guide is saying as well that it SHOULD use #!/usr/bin/ruby . > The "env ruby" is not always only /usr/bin/ruby. Depending on environment > settings it could be also /usr/local/bin/ruby or ~user/bin/ruby or even > /tmp/you_have_been_hacked/ruby . Ah, I apologize, I worded by comment badly and linked to wrong, only marginally relevant section of the guidelines. What I was trying to say was this: Nowadays, shebang lines are automatically modified to replace env with the correct interpreter, so there should be no need to do that manually. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_shebang_lines Doing that manually is not forbidden either, so it can stay that way if you prefer. But doing that explicitly does not change the result from what the defaults already do. > > 4. I do not understand this. Is this an issue with upstream man pages? If > > so, a fix or an issue should be submitted and referenced from the specfile. > > > > > # Unknown macros in manpage > > > sed -i -e 's|^.ni||; s|^\./plugins-disabled|+\./plugins-disabled|' whatweb.1 > Yes ... I guess that on Ubuntu they use different groff for formatting the > man pages so it is ok for them. > On Fedora it complains so I have to remove that tags. I see, it may be for the '.ni' case. The './plugins-disabled' is for sure part of the example for enabling and disabling plugin directories. I took a closer look at the upstream whatweb.1, and my interpretation is that the file simply is in a bad shape and would benefit from a PR like this. I encourage submitting fixes. But if you do not want to do that, that's ok too, I won't reject this review for that. Regarding the substitutions: 1. Instead of 's|^.ni||', use 's|^\.ni||', to replace the unknown request '.ni' and not lines starting with 'Ani', 'Bni' etc. 2. Instead of 's|^\./plugins-disabled|+\./plugins-disabled|', use 's|^\./plugins-disabled|\\\./plugins-disabled|'. '+' is not a Groff way to escape '.', but happens to be Whatweb syntax and thus changes the example.
This is an automatic action taken by review-stats script. The ticket submitter failed to clear the NEEDINFO flag in a month. As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews we consider this ticket as DEADREVIEW and proceed to close it.
Oh, this is a shame, this was so close. My two remaining items were both non-blocking, so just a reply "I do not want to make any more changes" would have sufficed to get this accepted. Please reopen if you still wish to complete this.