https://bugzilla.redhat.com/show_bug.cgi?id=587978 Otto Urpelainen <oturpe@xxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(oturpe@xxxxxx) |needinfo?(rebus@xxxxxxxxx) --- Comment #34 from Otto Urpelainen <oturpe@xxxxxx> --- 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. -- You are receiving this mail because: You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
