Hello,
Today I reviewed feature request here:
https://bugzilla.redhat.com/show_bug.cgi?id=1244102
I used fedora-review tool for generating review check list form.
fedora-review tool noticed following issue:
In the reviewed package there is not any license file in doc files.
I found that it is caused by searching potential license files code in CheckLicensInDoc test case: - 'COPYING' - 'LICEN' - 'copying' - 'licen'
using glob pattern:
'' + potentialfile + ''
This value is looking for every 'COPYING', 'LICEN', 'copying', 'licen' strings by all rpms paths.
The package "php-composer-spdx-licenses" that I reviewed today contains in its name keyword 'license'.
From this reason the CheckLicensInDoc code found a couple of paths with 'license' keyword (in directories and in files).
Here is full list files and directories:
[ '/usr/share/licenses/php-composer-spdx-licenses/LICENSE', '/usr/share/doc/php-composer-spdx-licenses', '/usr/share/doc/php-composer-spdx-licenses/CHANGELOG.md', '/usr/share/doc/php-composer-spdx-licenses/CONTRIBUTING.md', '/usr/share/doc/php-composer-spdx-licenses/README.md', '/usr/share/doc/php-composer-spdx-licenses/composer.json', '/usr/share/licenses/php-composer-spdx-licenses', '/usr/share/licenses/php-composer-spdx-licenses/LICENSE', '/usr/share/php-composer-spdx-licenses', '/usr/share/php-composer-spdx-licenses/spdx-exceptions.json', '/usr/share/php-composer-spdx-licenses/spdx-licenses.json' ]
From this reason all above files are treated as licenses files.
I would propose to improve this searching licenses in docs by finding 'LICENSE' (US English) and 'LICENCE' (British English) and 'COPYING' in files (not whole paths) stored in rpms. Both LICENSE and LICENCE are valid, and from this reason I indicated both.
I prepared patch that improves: - checking only files (no directories) for license files - looking for license type files by regexp, not glob - restrict licenses files comparing to predefined patterns instead of strings
I hope that this improvements (if be applied) helps to avoid this licenses type of issues.
Thank you for your great tool fedora-review!
Best regards Marcin Haba
attachment fedora-review-find-license-in-docs.patch
I updated new patch, because I found bug in my patch :)
attachment fedora-review-find-license-in-docs.2.patch
The last one patch is the same as previous patch and does the same, but in nicer way.
This one was final from my side.
Here is cut from my debug in fedora-review:
DEBUG: Running check: CheckLicensInDoc DEBUG: CheckLicensInDoc completed: 0.026 seconds
Thanks.
First of all: this patch has been here too long without anyone looking at it. It's a shame, really.
That said, the overall strategy must be that it's better to show too many files than to miss anything. Your patch will exclude things like COPYING.txt etc. so it cannot be applied.
Looking at the description I cannot see anything which f-r shouldn't list as possible licenses besides the sheer directory. Given the path, all of these potentially contains license info which needs to be checked.
In short, my gut feeling is that this should be closed as not-a-bug. Thoughts?
Replying to [comment:3 leamas]:
Your patch will exclude things like COPYING.txt etc. so it cannot be applied.
Yes, it is true.
Nevertheless in my opinion better is check some predefined potential files than checking whole paths in a haphazard way, like in this ticket description (e.g: '/usr/share/php-composer-spdx-licenses/spdx-exceptions.json').
The current licenses checking can generate more tickets and questions in future. If there is lack some potential license files on the list, then all the time there is possible to extend the list.
Thank you for your look on the patch and on this ticket as well.
Best regards. Marcin Haba
Login to comment on this ticket.