https://bugzilla.redhat.com/show_bug.cgi?id=1289760 --- Comment #5 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> --- Some comments on Ranjan's review (going into a bit more detail than I usually would): (In reply to Ranjan Maitra from comment #2) > Also fedora-review provides the following: > > - BuildRequires not needed: gcc This changed recently. https://fedorahosted.org/fpc/ticket/490 and https://fedorahosted.org/fpc/ticket/497 have the full history, but the relevant part is the following change to guidelines [https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires_2]: "It is important that your package list all necessary build dependencies using the BuildRequires?: tag. You may assume that enough of an environment exists for RPM to function and execute basic shell scripts, but you should not assume any other packages are present as RPM dependencies and anything brought into the buildroot by the build system may change over time." fedora-review is wrong here, and one SHOULD have BuildRequires:gcc, though things work just fine without, and will do so for the forseeable future. The spec file is correct. > 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. > [!]: License field in the package spec file matches the actual license. > Note: Checking patched sources after %prep for licenses. Licenses > found: "GPL (v2 or later)", "GPL (v3 or later)", "Unknown or > generated", "MIT/X11 (BSD like)", "LGPL (v3 or later)", "LGPL (v2.1 or > later)". 86 files have unknown license. The spec file specifies License:LGPLv3+. According to the COPYING file and to https://github.com/nbourdau/drawtk/commit/46604561b6d3396e067ab this is correct. Note: the License tag specifies the *effective* license of the *binary* package [1, 2]. So anything that is not part of the binary rpm (like install/install-sh) doesn't matter at all. If you combine GPLv2+ with BSD or LGPL or MIT, the effective license is GPLv2+. So, if a file in the binary rpm has at least one GPL licensed file, that file is GPL. If a file only had sources with more permissive licenses, than that file would have some other license. So you need to determine the effective license of all files in the binary rpm and put the result in License. What licensecheck returns in the review is just the starting point. [1] https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License:_field [2] https://fedoraproject.org/wiki/Licensing:FAQ?rd=Licensing/FAQ#What_is_.22effective_license.22_and_do_I_need_to_know_that_for_the_License:_tag.3F Licensecheck says: drawtk-2.0/AUTHORS: UNKNOWN drawtk-2.0/configure.ac: *No copyright* UNKNOWN drawtk-2.0/INSTALL: UNKNOWN drawtk-2.0/Makefile.am: *No copyright* UNKNOWN drawtk-2.0/README: *No copyright* UNKNOWN drawtk-2.0/configure: GENERATED FILE drawtk-2.0/aclocal.m4: GENERATED FILE drawtk-2.0/tests/Makefile.am: *No copyright* UNKNOWN drawtk-2.0/tests/navy.png: *No copyright* UNKNOWN drawtk-2.0/tests/test-video-custom.c: GPL (v3 or later) drawtk-2.0/tests/navy.png.license: LGPL (v2.1 or later) drawtk-2.0/tests/test.ogv: UNKNOWN drawtk-2.0/tests/test1.c: GPL (v3 or later) drawtk-2.0/tests/Makefile.in: GENERATED FILE drawtk-2.0/tests/test-video.c: GPL (v3 or later) drawtk-2.0/tests/test-events.c: GPL (v3 or later) drawtk-2.0/doc/dtk_addtime.3: UNKNOWN drawtk-2.0/doc/dtk_nanosleep.3: UNKNOWN drawtk-2.0/doc/dtk_load_video_tcp.3: UNKNOWN drawtk-2.0/doc/dtk_bgcolor.3: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_create_image.3: UNKNOWN drawtk-2.0/doc/dtk_update_screen.3: UNKNOWN drawtk-2.0/doc/dtk_move_shape.3: UNKNOWN drawtk-2.0/doc/dtk_window_getsize.3: UNKNOWN drawtk-2.0/doc/dtk_create_string.3: UNKNOWN drawtk-2.0/doc/dtk_set_event_handler.3: *No copyright* UNKNOWN drawtk-2.0/doc/Makefile.am: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_create_rectangle_2p.3: UNKNOWN drawtk-2.0/doc/dtk_difftime_us.3: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_create_composite_shape.3: UNKNOWN drawtk-2.0/doc/dtk_gettime.3: UNKNOWN drawtk-2.0/doc/dtk_relmove_shape.3: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_load_video_gst.3: UNKNOWN drawtk-2.0/doc/dtk_load_image.3: UNKNOWN drawtk-2.0/doc/dtk_create_rectangle_hw.3: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_destroy_font.3: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_relrotate_shape.3: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_video_getstate.3: UNKNOWN drawtk-2.0/doc/dtk_video_exec.3: UNKNOWN drawtk-2.0/doc/examples/bars.c: GPL (v3 or later) drawtk-2.0/doc/examples/bounce.c: GPL (v3 or later) drawtk-2.0/doc/examples/errormon.c: GPL (v3 or later) drawtk-2.0/doc/examples/Makefile: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_process_events.3: UNKNOWN drawtk-2.0/doc/dtk_load_video_test.3: UNKNOWN drawtk-2.0/doc/dtk_setcolor_shape.3: UNKNOWN drawtk-2.0/doc/dtk_create_cross.3: UNKNOWN drawtk-2.0/doc/dtk_create_complex_shape.3: UNKNOWN drawtk-2.0/doc/dtk_get_color.3: UNKNOWN drawtk-2.0/doc/dtk_draw_shape.3: UNKNOWN drawtk-2.0/doc/dtk_create_circle.3: UNKNOWN drawtk-2.0/doc/dtk_close.3: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_create_circle_str.3: UNKNOWN drawtk-2.0/doc/dtk_create_arrow.3: UNKNOWN drawtk-2.0/doc/dtk_texture_getsize.3: UNKNOWN drawtk-2.0/doc/dtk_difftime_ns.3: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_create_shape.3: UNKNOWN drawtk-2.0/doc/Makefile.in: GENERATED FILE drawtk-2.0/doc/dtk_destroy_texture.3: UNKNOWN drawtk-2.0/doc/dtk_load_video_udp.3: UNKNOWN drawtk-2.0/doc/dtk_create_window.3: UNKNOWN drawtk-2.0/doc/dtk_clear_screen.3: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_destroy_shape.3: UNKNOWN drawtk-2.0/doc/dtk_load_video_file.3: UNKNOWN drawtk-2.0/doc/dtk_difftime_s.3: UNKNOWN drawtk-2.0/doc/dtk_rotate_shape.3: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_difftime_ms.3: *No copyright* UNKNOWN drawtk-2.0/doc/dtk_make_current_window.3: UNKNOWN drawtk-2.0/doc/dtk_load_font.3: UNKNOWN drawtk-2.0/doc/dtk_create_line.3: UNKNOWN drawtk-2.0/doc/dtk_create_triangle.3: UNKNOWN drawtk-2.0/lib/Makefile.am: *No copyright* UNKNOWN drawtk-2.0/lib/clock_gettime.h: LGPL (v3 or later) drawtk-2.0/lib/clock_nanosleep.h: LGPL (v3 or later) drawtk-2.0/lib/clock_gettime.c: LGPL (v3 or later) drawtk-2.0/lib/convfiletime.h: LGPL (v3 or later) drawtk-2.0/lib/timespec.h: LGPL (v3 or later) drawtk-2.0/lib/Makefile.in: GENERATED FILE drawtk-2.0/lib/clock_nanosleep.c: LGPL (v3 or later) drawtk-2.0/config/config.h.in: *No copyright* GENERATED FILE drawtk-2.0/NEWS: *No copyright* UNKNOWN drawtk-2.0/COPYING: UNKNOWN drawtk-2.0/Makefile.in: GENERATED FILE drawtk-2.0/ChangeLog: *No copyright* UNKNOWN drawtk-2.0/m4/pkg-custom.m4: GPL (v2 or later) (with incorrect FSF address) GENERATED FILE drawtk-2.0/m4/api-exports.m4: *No copyright* UNKNOWN drawtk-2.0/m4/ltsugar.m4: UNKNOWN drawtk-2.0/m4/lt~obsolete.m4: UNKNOWN drawtk-2.0/m4/libtool.m4: GPL (v2 or later) drawtk-2.0/m4/ltversion.m4: GENERATED FILE drawtk-2.0/m4/ltoptions.m4: UNKNOWN drawtk-2.0/m4/funcarg.m4: *No copyright* UNKNOWN drawtk-2.0/src/drawtk.pc.in: *No copyright* UNKNOWN drawtk-2.0/src/video.c: LGPL (v3 or later) drawtk-2.0/src/colors.c: LGPL (v3 or later) drawtk-2.0/src/dtk_time.h: LGPL (v3 or later) drawtk-2.0/src/shapes.h: LGPL (v3 or later) drawtk-2.0/src/shapes.c: LGPL (v3 or later) drawtk-2.0/src/imagetex.c: LGPL (v3 or later) drawtk-2.0/src/texmanager.c: LGPL (v3 or later) drawtk-2.0/src/Makefile.am: *No copyright* UNKNOWN drawtk-2.0/src/fonttex.h: LGPL (v3 or later) drawtk-2.0/src/window.c: LGPL (v3 or later) drawtk-2.0/src/vidpipe_creation.c: LGPL (v3 or later) drawtk-2.0/src/time.c: LGPL (v3 or later) drawtk-2.0/src/dtk_event.h: LGPL (v3 or later) drawtk-2.0/src/fonttex.c: LGPL (v3 or later) drawtk-2.0/src/dtk_colors.h: LGPL (v3 or later) drawtk-2.0/src/drawtk.h: LGPL (v3 or later) drawtk-2.0/src/create_shape.c: LGPL (v3 or later) drawtk-2.0/src/vidpipe_creation.h: LGPL (v3 or later) drawtk-2.0/src/Makefile.in: GENERATED FILE drawtk-2.0/src/events.c: LGPL (v3 or later) drawtk-2.0/src/dtk_video.h: LGPL (v3 or later) drawtk-2.0/src/texmanager.h: LGPL (v3 or later) drawtk-2.0/src/window.h: LGPL (v3 or later) drawtk-2.0/build-aux/install-sh: MIT/X11 (BSD like) drawtk-2.0/build-aux/compile: GPL (v2 or later) GENERATED FILE drawtk-2.0/build-aux/config.sub: GPL (v2 or later) GENERATED FILE drawtk-2.0/build-aux/ltmain.sh: GPL (v2 or later) GENERATED FILE drawtk-2.0/build-aux/depcomp: GPL (v2 or later) GENERATED FILE drawtk-2.0/build-aux/config.guess: GPL (v2 or later) GENERATED FILE drawtk-2.0/build-aux/missing: GPL (v2 or later) GENERATED FILE As mentioned above, we can disregard anything in build-aux/, m4/, the Makefiles. In the end it seems we have the main sources, which are LGPLv3+, and some tests, which are GPLv3+. If the tests end up in the binary rpm, then "and GPLv3" should be in License. > [!]: License file installed when any subpackage combination is installed. I believe this comment is wrong. The main package has the COPYING file, and the -devel subpackage Requires:%{name}%{?_isa} = %{version}-%{release}. That is enough. > [!]: %build honors applicable compiler flags or justifies otherwise. %configure and %make_build macros are designed to DTRT. When they are used, the result is almost always correct. Of course is is always good to look at the build logs: /bin/sh ../libtool --silent --tag=CC --mode=compile gcc -DHAVE_CONFIG_H -I. -I../config -I/usr/include/freetype2 -I/usr/include/libpng16 -pthread -I/usr/include/gstreamer-0.10 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/libxml2 -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables -c -o colors.lo colors.c So yes, the build flags include the hardening macros (-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong) and seem fine. > [?]: Package uses nothing in %doc for runtime. The best way to verify that is to check the list of files in the binary rpm for anything suspicious. In this case, /usr/share/doc/drawtk has some text files and source files, i.e. nothing which looks like it could be used by the library. > Generic: > [!]: Final provides and requires are sane (see attachments). > [!]: Fully versioned dependency in subpackages if applicable. > Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in drawtk- > debuginfo This dependency is not used in case of -debuginfo packages. (The reason is that those packages tend to be huge, and forcing them to be kept in sync automatically using dependencies would make updates much more heavyweight. -debuginfo packages are allowed to get out of sync, and the user has to either update them manually or use a yum/dnf plugin to do that automatically.) > [!]: Package functions as described. In case of library packages, if they compile, they usually work fine. Do you have any reason to believe it is broken? The review should always describe what is broken, "functions as described" is too generic to be useful. > [!]: %check is present and all tests pass. > [!]: Rpmlint is run on all installed packages. > Note: Mock build failed This is a bug in fedora-review (or maybe mock?). In fact the package builds and install fine in mock. When doing reviews I simply cut out stuff like that (when fedora-review or rpmlint screws up), because it serves no purpose and only confuses the author of the package. (In reply to Dmitry Mikhirev from comment #4) > There is a public repository on github: https://github.com/nbourdau/drawtk > I archived sources from alioth repo because it contains exactly the same > files that were in the latest released tarball that seems lost. Please put this comment in the spec file. It will be useful to other people who rebuild your package or look for sources. > This library is needed only for possible future development. OK. > >[!]: License field in the package spec file matches the actual license. > > Thank you, I will fix this. Please see my comment above; I don't think it is incorrect. -- 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://admin.fedoraproject.org/mailman/listinfo/package-review