Bug 1933419 - Review Request: js-jquery-ui - jQuery user interface
Summary: Review Request: js-jquery-ui - jQuery user interface
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Silvie Chlupova
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-02-27 16:32 UTC by Mattias Ellert
Modified: 2021-03-27 00:42 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-03-19 18:49:02 UTC
Type: ---
Embargoed:
schlupov: fedora-review+


Attachments (Terms of Use)

Description Mattias Ellert 2021-02-27 16:32:45 UTC
Spec URL: http://www.grid.tsl.uu.se/review/js-jquery-ui.spec
SRPM URL: http://www.grid.tsl.uu.se/review/js-jquery-ui-1.12.1-1.fc35.src.rpm

Description:
A curated set of user interface interactions, effects, widgets, and
themes built on top of the jQuery JavaScript Library.

Fedora Account System Username: ellert

Until now, jquery-ui has been provided by the xstatic-jquery-ui-common package built from the python-XStatic-jquery-ui srpm. This package was recently retired due to not being installable:

https://bugzilla.redhat.com/show_bug.cgi?id=1897725

Rather than trying to make that package installable again (with a lot of missing retired python dependencies), I suggest packaging jquery-ui as a javascript package. This also meas it will get a sensible package name.

Note that this package bundles some build dependencies, using the same technique that is used in the already existing js-jquery package.

Comment 1 Miro Hrončok 2021-02-28 21:55:19 UTC
> Rather than trying to make that package installable again (with a lot of missing retired python dependencies), I suggest packaging jquery-ui as a javascript package. This also meas it will get a sensible package name.

+1 for this approach

Comment 2 Silvie Chlupova 2021-03-02 14:05:29 UTC
Just a few comments, first of all, I would like to thank you for your effort to get the package into Fedora.
I don't see any dist directory, but you use install -m 644 -p dist/* %{buildroot}%{_jsdir}/%{jsname}.
See the Rpmlint section, there are 3 warnings.
You also write that jshint has a non-free license, but according to https://github.com/jshint/jshint/blob/master/LICENSE jshint has an MIT license, the same as your package. 

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

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]: 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.
[-]: 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.
[x]: 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.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 30720 bytes in 3 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]: 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 must not depend on deprecated() packages.
[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:
[-]: 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).
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[!]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[-]: 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.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[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]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[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: No rpmlint messages.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: js-jquery-ui-1.12.1-1.fc35.noarch.rpm
          js-jquery-ui-1.12.1-1.fc35.src.rpm
js-jquery-ui.src: W: strange-permission create-source.sh 755
js-jquery-ui.src: W: invalid-url Source1: jquery-ui-1.12.1-node-modules.tar.gz
js-jquery-ui.src: W: invalid-url Source0: jquery-ui-1.12.1.tar.gz
2 packages and 0 specfiles checked; 0 errors, 3 warnings.




Rpmlint (installed packages)
----------------------------
1 packages and 0 specfiles checked; 0 errors, 0 warnings.



Requires
--------
js-jquery-ui (rpmlib, GLIBC filtered):
    js-jquery
    web-assets-filesystem

Comment 3 Mattias Ellert 2021-03-02 18:00:25 UTC
(In reply to Silvie Chlupova from comment #2)
> Just a few comments, first of all, I would like to thank you for your effort
> to get the package into Fedora.
> I don't see any dist directory, but you use install -m 644 -p dist/*
> %{buildroot}%{_jsdir}/%{jsname}.

There is no dist directory in the source tarball.
The dist directory and the files in it are created when the package is compiled (by running the command in the %build section).
These files are then packaged in the %install section.

> See the Rpmlint section, there are 3 warnings.

See below.

> You also write that jshint has a non-free license, but according to
> https://github.com/jshint/jshint/blob/master/LICENSE jshint has an MIT
> license, the same as your package. 

The LICENSE file in external/jshint/LICENSE/LICENSE is incomplete. It reflects license of the sources of jshint itself. However, the jshint.js file contains some bundled source files from other projects with different licenses. The part of jshint that is non-free is the bundled copy of jslint.js, starting at line 52736 in the copy of jshint.js that is included in the upstream sources of jquery-ui. The license for this file (which is included in the bundled copy) contains the condition:

"The Software shall be used for Good, not Evil."

Which means that the software is not free to use for any purpose, and hence non-free, and therefore not distributable in Fedora.

Some might argue that this was not the intention of the author of the code when he wrote the license, but this is the strict legal interpretation of the license text.

For more information see here: https://www.techrepublic.com/article/how-jshint-learned-the-hard-way-not-to-use-ethical-source-licensing/

The project did change the license in 2020, but the copy in the jquery-ui source tarball still has the old license.

The file is not necessary for building the software, so removing it does no break the source rpm.


> Package Review
> ==============
> 
> ===== SHOULD items =====
> 
> Generic:
> [!]: SourceX tarball generation or download is documented.
>      Note: Package contains tarball without URL, check comments
> 
> 
> Rpmlint
> -------
> Checking: js-jquery-ui-1.12.1-1.fc35.noarch.rpm
>           js-jquery-ui-1.12.1-1.fc35.src.rpm
> js-jquery-ui.src: W: strange-permission create-source.sh 755
> js-jquery-ui.src: W: invalid-url Source1:
> jquery-ui-1.12.1-node-modules.tar.gz
> js-jquery-ui.src: W: invalid-url Source0: jquery-ui-1.12.1.tar.gz
> 2 packages and 0 specfiles checked; 0 errors, 3 warnings.

Since the source tarballs in the srpm are either modified or generated, there are no URLs from where they can be downloaded. How the sources are created are instead documented by including the script that generates them in the source rpm. See:

https://fedoraproject.org/wiki/Packaging:SourceURL#When_Upstream_uses_Prohibited_Code

The script is supposed to be executable, so the file permissions are not that strange.


Thank you for the review.
I hope I have addressed your comments.

Comment 4 Silvie Chlupova 2021-03-08 08:58:38 UTC
> I hope I have addressed your comments.

You have, thank you.

Comment 5 Tomas Hrcka 2021-03-10 22:22:04 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/js-jquery-ui

Comment 6 Fedora Update System 2021-03-11 13:06:07 UTC
FEDORA-EPEL-2021-a8ba75a9b4 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-a8ba75a9b4

Comment 7 Fedora Update System 2021-03-11 13:06:08 UTC
FEDORA-2021-079710c327 has been submitted as an update to Fedora 33. https://bodhi.fedoraproject.org/updates/FEDORA-2021-079710c327

Comment 8 Fedora Update System 2021-03-11 13:06:10 UTC
FEDORA-2021-e360f8a8a7 has been submitted as an update to Fedora 32. https://bodhi.fedoraproject.org/updates/FEDORA-2021-e360f8a8a7

Comment 9 Fedora Update System 2021-03-11 13:07:07 UTC
FEDORA-EPEL-2021-6155880f1d has been submitted as an update to Fedora EPEL 8. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-6155880f1d

Comment 10 Fedora Update System 2021-03-11 19:51:53 UTC
FEDORA-2021-63c7040d9d has been pushed to the Fedora 34 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-63c7040d9d \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-63c7040d9d

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 11 Fedora Update System 2021-03-12 00:18:05 UTC
FEDORA-EPEL-2021-6155880f1d has been pushed to the Fedora EPEL 8 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-6155880f1d

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 12 Fedora Update System 2021-03-12 00:19:16 UTC
FEDORA-2021-079710c327 has been pushed to the Fedora 33 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-079710c327 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-079710c327

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 13 Fedora Update System 2021-03-12 00:21:24 UTC
FEDORA-2021-e360f8a8a7 has been pushed to the Fedora 32 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-e360f8a8a7 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-e360f8a8a7

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 14 Fedora Update System 2021-03-12 00:27:35 UTC
FEDORA-EPEL-2021-a8ba75a9b4 has been pushed to the Fedora EPEL 7 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-a8ba75a9b4

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 15 Fedora Update System 2021-03-19 18:49:02 UTC
FEDORA-2021-079710c327 has been pushed to the Fedora 33 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 16 Fedora Update System 2021-03-19 18:52:14 UTC
FEDORA-2021-e360f8a8a7 has been pushed to the Fedora 32 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 17 Fedora Update System 2021-03-19 20:11:41 UTC
FEDORA-2021-63c7040d9d has been pushed to the Fedora 34 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 18 Fedora Update System 2021-03-27 00:29:32 UTC
FEDORA-EPEL-2021-6155880f1d has been pushed to the Fedora EPEL 8 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 19 Fedora Update System 2021-03-27 00:42:15 UTC
FEDORA-EPEL-2021-a8ba75a9b4 has been pushed to the Fedora EPEL 7 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.