Bug 1336728

Since I maintain one mozjsXY and am co-maintainer of another,
I will review this mozjs45. I also may need it in the future.

  Most of it should be trivial, as the spec is matching the pattern
of mozjs{17,24,31,38}.

1)  I found it a bit weird this configure option
	--disable-optimize \
but noticed that CFLAGS and CXXFLAGS were still being correctly used,
but from reading the configure macros, if enabled it would just add
a redundant -O2. Is it related to patch2 (#1219542 shows an error
about nested functions too deep in optimization..)? Probably need some
notes about it.

2)  I noticed it is now built with a bundled/static jemalloc. Is it
patched? Should either have a "Provides: jemalloc-static", use the
system one or build with --disable-jemalloc.

3) Please fix the LICENSE file:
---8<---
$ cat 1336728-mozjs45/rpms-unpacked/mozjs45-45.1.1-1.fc24.x86_64.rpm/usr/share/licenses/mozjs45/LICENSE
Please see the file toolkit/content/license.html for the copyright licensing
conditions attached to this codebase, including copies of the licenses
concerned.

You are not granted rights or licenses to the trademarks of the
Mozilla Foundation or any party, including without limitation the
Firefox name or logo.

For more information, see: http://www.mozilla.org/foundation/licensing.html
---8<---
Also please check the partial fedora-review output below, and
comment about any missing entry in the License tag:
---8<---
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "BSD (4 clause) ISC", "LGPL (v2.1 or later) (with incorrect FSF
     address)", "*No copyright* BSL", "*No copyright* BSD", "LGPL (v2 or
     later) (with incorrect FSF address)", "*No copyright* MPL (v2.0)",
     "Apache (v2.0) BSD (2 clause)", "*No copyright* BSD (3 clause)",
     "Apache (v2.0) MPL (v2.0)", "AFL-3.0", "LGPL (v2.1)", "ISC", "GPL (v2
     or later)", "libpng MPL (v2.0)", "*No copyright* MIT/X11 (BSD like)",
     "*No copyright* LGPL (v2.1 or later)", "GPL (v3 or later)", "AFL-2.1",
     "Apache (v2.0) BSD (3 clause) MIT/X11 (BSD like)", "zlib/libpng",
     "BSL", "GPL (v2 or later) (with incorrect FSF address)", "BSD (2
     clause)", "*No copyright* Apache (v2.0)", "Apache (v2.0)", "FreeType",
     "BSD (3 clause) ISC", "*No copyright* MPL (v2.0) BSD (3 clause)", "*No
     copyright* Public domain MPL (v2.0)", "MIT/X11 (BSD like)", "MPL
     (v1.1)", "*No copyright* Public domain", "libpng", "LGPL", "BSD (3
     clause)", "*No copyright* MPL (v1.1) GPL (unversioned/unknown
     version)", "*No copyright* Beerware MPL (v2.0)", "MPL (v2.0)", "MPL
     (v2.0) BSD (3 clause)", "MPL (v2.0) MIT/X11 (BSD like) LGPL (v2.1 or
     later)", "GPL (v3)", "*No copyright* MPL (v2.0) BSD", "BSD (3 clause)
     GPL (v2)", "Unknown or generated", "BSD (4 clause)", "BSL (v1.0)",
     "MPL (v1.1) GPL (unversioned/unknown version)", "MPL (v2.0) MIT/X11
     (BSD like)", "Apache (v2.0) MIT/X11 (BSD like)", "LGPL (v2.1 or
     later)", "BSD (3 clause) MIT/X11 (BSD like)", "GPL (v2)", "MPL (v1.0)
     LGPL (v2 or later) (with incorrect FSF address)", "MPL (v2.0) BSD (2
     clause)". 30918 files have unknown license. Detailed output of
     licensecheck in /home/pcpa/1336728-mozjs45/licensecheck.txt
---8<---

An extra note, if bundling jemalloc, it should not be
Provides: jemalloc-static but something like, looks quite long :):
Provides: bundled(jemalloc) = 4.0.0-12-ged4883285e111b426e5769b24dad164ebacaa5b9
based on firefox-45.1.1esr/memory/jemalloc/src/VERSION

Thanks Paulo for taking this review.

(In reply to Paulo Andrade from comment #2)
>   Most of it should be trivial, as the spec is matching the pattern
> of mozjs{17,24,31,38}.
> 
> 1)  I found it a bit weird this configure option
> 	--disable-optimize \
> but noticed that CFLAGS and CXXFLAGS were still being correctly used,
> but from reading the configure macros, if enabled it would just add
> a redundant -O2. Is it related to patch2 (#1219542 shows an error
> about nested functions too deep in optimization..)? Probably need some
> notes about it.

I think that --enable-optimize adds -O3 optimization. Maybe it is related to #1219542, however this optimization also caused tests regression in mozjs38 with newer gcc. With this option mozjs45 is also failing on ARM in koji.

> 
> 2)  I noticed it is now built with a bundled/static jemalloc. Is it
> patched? Should either have a "Provides: jemalloc-static", use the
> system one or build with --disable-jemalloc.

Since upstream does not allow to system jemalloc and Fedora firefox also uses bundled jemalloc, I decided to use bundled one.
So added Provides.

> 
> 3) Please fix the LICENSE file:
> ---8<---
> $ cat
> 1336728-mozjs45/rpms-unpacked/mozjs45-45.1.1-1.fc24.x86_64.rpm/usr/share/
> licenses/mozjs45/LICENSE
> Please see the file toolkit/content/license.html for the copyright licensing
> conditions attached to this codebase, including copies of the licenses
> concerned.
> 
> You are not granted rights or licenses to the trademarks of the
> Mozilla Foundation or any party, including without limitation the
> Firefox name or logo.
> 
> For more information, see: http://www.mozilla.org/foundation/licensing.html
> ---8<---
> Also please check the partial fedora-review output below, and
> comment about any missing entry in the License tag:
> ---8<---
>      Note: Checking patched sources after %prep for licenses. Licenses
>      found: "BSD (4 clause) ISC", "LGPL (v2.1 or later) (with incorrect FSF
>      address)", "*No copyright* BSL", "*No copyright* BSD", "LGPL (v2 or
>      later) (with incorrect FSF address)", "*No copyright* MPL (v2.0)",
>      "Apache (v2.0) BSD (2 clause)", "*No copyright* BSD (3 clause)",
>      "Apache (v2.0) MPL (v2.0)", "AFL-3.0", "LGPL (v2.1)", "ISC", "GPL (v2
>      or later)", "libpng MPL (v2.0)", "*No copyright* MIT/X11 (BSD like)",
>      "*No copyright* LGPL (v2.1 or later)", "GPL (v3 or later)", "AFL-2.1",
>      "Apache (v2.0) BSD (3 clause) MIT/X11 (BSD like)", "zlib/libpng",
>      "BSL", "GPL (v2 or later) (with incorrect FSF address)", "BSD (2
>      clause)", "*No copyright* Apache (v2.0)", "Apache (v2.0)", "FreeType",
>      "BSD (3 clause) ISC", "*No copyright* MPL (v2.0) BSD (3 clause)", "*No
>      copyright* Public domain MPL (v2.0)", "MIT/X11 (BSD like)", "MPL
>      (v1.1)", "*No copyright* Public domain", "libpng", "LGPL", "BSD (3
>      clause)", "*No copyright* MPL (v1.1) GPL (unversioned/unknown
>      version)", "*No copyright* Beerware MPL (v2.0)", "MPL (v2.0)", "MPL
>      (v2.0) BSD (3 clause)", "MPL (v2.0) MIT/X11 (BSD like) LGPL (v2.1 or
>      later)", "GPL (v3)", "*No copyright* MPL (v2.0) BSD", "BSD (3 clause)
>      GPL (v2)", "Unknown or generated", "BSD (4 clause)", "BSL (v1.0)",
>      "MPL (v1.1) GPL (unversioned/unknown version)", "MPL (v2.0) MIT/X11
>      (BSD like)", "Apache (v2.0) MIT/X11 (BSD like)", "LGPL (v2.1 or
>      later)", "BSD (3 clause) MIT/X11 (BSD like)", "GPL (v2)", "MPL (v1.0)
>      LGPL (v2 or later) (with incorrect FSF address)", "MPL (v2.0) BSD (2
>      clause)". 30918 files have unknown license. Detailed output of
>      licensecheck in /home/pcpa/1336728-mozjs45/licensecheck.txt
> ---8<---

Added MPLv1.1 in License:
And %license now installs copy of MPLv2.0 license.

Is this what you thought about?



Spec URL: https://mskalick.fedorapeople.org/mozjs45/mozjs45.spec
SRPM URL: https://mskalick.fedorapeople.org/mozjs45/mozjs45-45.1.1-2.fc23.src.rpm

  Please correct package build before submitting, as it does
not work with:

%license %{SOURCE1}

as it will expand the full path:

error: File not found: /builddir/build/BUILDROOT/mozjs45-45.1.1-2.fc25.x86_64/builddir/build/SOURCES/LICENSE.txt

  Otherwise, the package is approved!

  Depending on if 0ad updates to use mozjs45, I would like to
also be a co-maintainer. So, I would like if you add me as
such.

Thank you very much.

Spec URL: https://mskalick.fedorapeople.org/mozjs45/mozjs45.spec
SRPM URL: https://mskalick.fedorapeople.org/mozjs45/mozjs45-45.1.1-3.fc23.src.rpm

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/mozjs45

mozjs45-45.1.1-3.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-1515138bb9

mozjs45-45.1.1-3.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-1515138bb9

mozjs45-45.1.1-4.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-7c6aef3b6c

mozjs45-45.1.1-4.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-7c6aef3b6c

mozjs45-45.1.1-6.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-d2ff587d77

mozjs45-45.1.1-6.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-d2ff587d77

mozjs45-45.1.1-6.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.