[Bug 1269539] Review Request: mozjs38 - JavaScript interpreter and libraries

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Paulo Andrade <paulo.cesar.pereira.de.andrade@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED



--- Comment #2 from Paulo Andrade <paulo.cesar.pereira.de.andrade@xxxxxxxxx> ---
  Marek,

  The package looks quite good. I made some considerations
below. Most important is the License issue, due tonew files
under ASL 2.0.

  Looking at build log, most warnings appear ok, but this
one might have issues:

In file included from
/builddir/build/BUILD/mozjs-38.0.0/js/src/js/src/Unified_cpp_js_src2.cpp:65:0:
/builddir/build/BUILD/mozjs-38.0.0/js/src/irregexp/RegExpEngine.cpp: In member
function 'virtual bool js::irregexp::TextNode::FillInBMInfo(int, int,
js::irregexp::BoyerMooreLookahead*, bool)':
/builddir/build/BUILD/mozjs-38.0.0/js/src/irregexp/RegExpEngine.cpp:4605:48:
warning: array subscript is above array bounds [-Warray-bounds]
                         bm->Set(offset, chars[j]);

looking at the code it appears to be a false positive, likely
because g++ did not understand that it should always return
a value that is in bounds. But I may be mistaken, please
comment.

---8<---
mozjs38.src:18: W: mixed-use-of-spaces-and-tabs (spaces: line 18, tab: line 1)

Looks like you added a missing build requires from mozjs31
spec, but did not match spaces vs tabs :)

---8<---
In js.pc I see:
Version: 38.3.0esrpre

Looks like there is some confusion. But this is minor,
just a comment. Should be expected from a .rc0.

---8<---
I believe is is not strictly an update do mozjs31, so you may
modify the changelog to not say so. It should only have conflicts
with mozjs31-devel and mozjs38-devel, otherwise, runtime will
not conflict.

---8<---
Please remove the bundled mozjs-38.0.0/modules/zlib/
directory before starting build (mozjs31 does not have
a modules subdir). It should not use it due to --with-system-zlib,
but just to be sure.

---8<---
There are some new files under apache license, so, it should
be required to add "ASL 2.0" to the License tag.
In mozjs31 asmjs was simpler, in the js/src/jit
directory, and under MPL 2.0 license. Now it is
under js/src/asmjs/AsmJS and with apache license.

-- 
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




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]