Spec URL: http://rpms.jdulaney.com/review/micropython.spec SRPM URL: http://rpms.jdulaney.com/review/micropython-1.6-1.fc24.src.rpm Description: Implementation of Python 3 with very low memory footprint Fedora Account System Username: jdulaney
I note that I disable debuginfo excraction as the build process itself does not generate it. I will work with upstream on this.
Unfortunately, I found a potentially "fun" licensing problem with the USB code in stmhal/usbhost: https://github.com/micropython/micropython/issues/26 Specifically, the code in that directory is under http://www.st.com/web/en/resource/legal/legal_agreement/license_agreement/ultimate-liberty-v2.txt?sc=software_license_agreement_liberty_v2 which isn't an open source license The notice of that license in the source tarball is in stmhal/usbhost/Release_Notes.html The other slightly dubious piece in the embedded system parts of the tarball is what looks to be a pre-built binary at cc3200/bootmanager/relocator/relocator.bin The MicroPython-for-Unix build doesn't *use* any of those pieces, but I'm not sure of the potential implications of having them in the SRPM. Should this BZ be set to block FE-Legal to ask them the question? A couple of other items worth taking a second look at: - fedora-review complained about the LICENSE file being marked as %doc instead of %license - there's a micropython-upip tarball embedded in the tools directory which I haven't looked inside yet (I'm still working through the rest of the review checklist, but figured it made sense to raise these items immediately rather than waiting)
fedora-review & rpmlint both complained about not being able to retrieve the upstream source tarball, but I believe that's a false alarm caused by GitHub's use of 302 redirects to locate the actual tarball: https://fedorahosted.org/FedoraReview/ticket/270
*** Bug 1113915 has been marked as a duplicate of this bug. ***
> The MicroPython-for-Unix build doesn't *use* any of those pieces, but I'm not > sure of the potential implications of having them in the SRPM. Should this BZ > be set to block FE-Legal to ask them the question? My reading of the license says that it is fine to have the file in SRPM: the license allows free copying, but limits *use* ("only on our crappy hardware"). As long as that file or anything built from it is not included in the binary rpms, it is not possible to violate the license. To make sure that it is not used, remove the file in %prep. > relocator.bin https://github.com/micropython/micropython/commit/f8146d021ba933034e7a6244e4267a6626357cf7 I think we can treat this as "firmware" [https://fedoraproject.org/wiki/Licensing:Main#Binary_Firmware], and distribute it. It must have the right license though.
(In reply to Nick Coghlan from comment #2) > Unfortunately, I found a potentially "fun" licensing problem with the USB > code in stmhal/usbhost: https://github.com/micropython/micropython/issues/26 > > Specifically, the code in that directory is under > http://www.st.com/web/en/resource/legal/legal_agreement/license_agreement/ > ultimate-liberty-v2.txt?sc=software_license_agreement_liberty_v2 which isn't > an open source license > > The notice of that license in the source tarball is in > stmhal/usbhost/Release_Notes.html > > The other slightly dubious piece in the embedded system parts of the tarball > is what looks to be a pre-built binary at > cc3200/bootmanager/relocator/relocator.bin > > The MicroPython-for-Unix build doesn't *use* any of those pieces, but I'm > not sure of the potential implications of having them in the SRPM. Should > this BZ be set to block FE-Legal to ask them the question? I went ahead and did so, better to be certain. > > A couple of other items worth taking a second look at: > > - fedora-review complained about the LICENSE file being marked as %doc > instead of %license > Oops, I will fix that. I had grabbed the spec from the previous review. > - there's a micropython-upip tarball embedded in the tools directory which I > haven't looked inside yet > > > (I'm still working through the rest of the review checklist, but figured it > made sense to raise these items immediately rather than waiting) (In reply to Zbigniew Jędrzejewski-Szmek from comment #5) > > The MicroPython-for-Unix build doesn't *use* any of those pieces, but I'm not > > sure of the potential implications of having them in the SRPM. Should this BZ > > be set to block FE-Legal to ask them the question? > > My reading of the license says that it is fine to have the file in SRPM: the > license allows free copying, but limits *use* ("only on our crappy > hardware"). As long as that file or anything built from it is not included > in the binary rpms, it is not possible to violate the license. > To make sure that it is not used, remove the file in %prep. > > > relocator.bin > > https://github.com/micropython/micropython/commit/ > f8146d021ba933034e7a6244e4267a6626357cf7 > > I think we can treat this as "firmware" > [https://fedoraproject.org/wiki/Licensing:Main#Binary_Firmware], and > distribute it. It must have the right license though. I will ask upstream about the nature of this, though it doesn't actually get included in the 'Unix' build. If need be, I can remove it via patch.
I asked upstream about the binary: https://github.com/micropython/micropython/issues/1968 I received the following response: > Hi, > > It's part of the CC3200 SDK supplied by TI. It is licensed the same way > as the other files from Texas Instruments. The source code is also > available in the SDK, I just used the .bin for convenience... > > Cheers, Daniel Looking around, it looks like an MIT-ish license. However, it appears to not be included in the build on Fedora, and to be safe, I added a line to the spec file to rm it. I also went ahead and fixed the %license.
Spec URL: http://rpms.jdulaney.com/review/micropython.spec SRPM URL: http://rpms.jdulaney.com/review/micropython-1.6-2.fc25.src.rpm
I concur with Zbigniew in comment 5. Just delete those files in %prep. Lifting FE-Legal.
Okay, went ahead and removed that entire directory just to be sure (rather than pick and choose the non-freely licensed files) Did a scratchbuild that may be found here: http://koji.fedoraproject.org/koji/taskinfo?taskID=13567775
Spec URL: http://rpms.jdulaney.com/review/micropython.spec SRPM URL: http://rpms.jdulaney.com/review/micropython-1.6-3.fc25.src.rpm
Thanks folks! I'll finish up the rest of the review today and post the full checklist.
Package looks good to me - there are a couple of items in the SHOULD section which I haven't checked (e.g. multi-arch support), or which are definitely missing (parallel build support, non-English translations) I also wasn't sure if/how the "-debuginfo" item applied to a binary like MicroPython, so I flagged it as not evaluated, rather than not applicable. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. 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. [x]: License field in the package spec file matches the actual license. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [?]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 bytes in 1 files. [x: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: 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 %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [!]: Uses parallel make %{?_smp_mflags} macro. [-]: 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]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [!]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [?]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [?]: Packages should try to preserve timestamps of original installed files. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. Rpmlint ------- Checking: micropython-1.6-3.fc25.x86_64.rpm micropython-1.6-3.fc25.src.rpm micropython.x86_64: W: no-manual-page-for-binary micropython 2 packages and 0 specfiles checked; 0 errors, 1 warnings. Rpmlint (installed packages) ---------------------------- sh: /usr/bin/python: No such file or directory micropython.x86_64: W: no-manual-page-for-binary micropython 1 packages and 0 specfiles checked; 0 errors, 1 warnings. Requires -------- micropython (rpmlib, GLIBC filtered): libc.so.6()(64bit) libdl.so.2()(64bit) libffi.so.6()(64bit) libm.so.6()(64bit) rtld(GNU_HASH) Provides -------- micropython: micropython micropython(x86-64) Source checksums ---------------- https://github.com/micropython/micropython/archive/v1.6.tar.gz : CHECKSUM(SHA256) this package : f8ea3faffa797de1a06c16c8f57c784c665b318ca08ac17f74dcbc95322d47de CHECKSUM(SHA256) upstream package : f8ea3faffa797de1a06c16c8f57c784c665b318ca08ac17f74dcbc95322d47de Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20 Command line :/usr/bin/fedora-review --rpm-spec -b 1323966 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, C/C++ Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/micropython
Builds for F24 and Rawhide are done, thanks for the review!