Build SpiderMonkey from `mozilla-central` #367

Xmader · 2024-06-19T04:37:54Z

Switching to mozilla-central from the ESR versions could save us from bugs that were solved several months ago in upstream SpiderMonkey tip but haven't been backported to ESR.

In order to make the target version consistent,
the mozilla-central commit hash to be used is stored in the mozcentral.version file.

Switching to mozilla-central from the ESR versions could save us from bugs that were solved several months ago in upstream SpiderMonkey tip but haven't been backported to ESR. The mozilla-central commit hash to be used is stored in the `mozcentral.version` file.

…K version The latest version of SpiderMonkey on macOS requires at least Xcode SDK 14.4, but none is available on Github Actions runner's macOS 13 image. See https://github.com/actions/runner-images/blob/main/images/macos/macos-13-Readme.md#installed-sdks ERROR: SDK version "13.3" is too old. Please upgrade to at least 14.4. Try updating your system Xcode.

…macOS SDK version" The hacky solution doesn't work. This reverts commit e4c37db.

…DK in 126 alpha Using the commit for mozilla-central tag `FIREFOX_NIGHTLY_125_END`

Using the commit for mozilla-central tag `FIREFOX_NIGHTLY_120_END`

Using the commit for mozilla-central tag `FIREFOX_NIGHTLY_115_END`

This reverts commit 275b0bf.

This reverts commit 3a07532.

…he use of `JS::ColumnNumberOneOrigin` class after upgrading SpiderMonkey to mozilla-central tip

…iningStopped` virtual method after upgrading SpiderMonkey to mozilla-central tip

…rapped in a `mozilla::UniquePtr` instead of passing it directly to the function

… (`mozilla::UniquePtr<char[], ...>`) instead of a pure `char[]` after upgrading SpiderMonkey to mozilla-central tip

…resent on `JS::RealmCreationOptions` after upgrading SpiderMonkey to mozilla-central tip

…rings

Probably due to a bug in the build system, we could not use `JS::Prefs::setAtStartup_weakrefs(true);` or `JS::Prefs::setAtStartup_experimental_iterator_helpers(true);`: ``` undefined symbol: _ZN2JS5Prefs9weakrefs_E ``` Further more, the file `js/PrefsGenerated.h` does not get copied into the installation location of SpiderMonkey, but stays in `/js/src/_build/js/public/PrefsGenerated.h`.

…UTF8CharsZ` instead of a pure `char*` after upgrading SpiderMonkey to mozilla-central tip

Xmader · 2024-06-20T10:04:16Z

~~Windows build is disabled for now~~

Fixed in 3883b62

This reverts commit 04d028b.

…em-zlib` option is not available on Windows

The current SpiderMonkey requires Visual Studio 2022 or newer. The Windows 2019 runner only has Visual Studio Enterprise 2019 installed. See https://github.com/actions/runner-images/blob/main/images/windows/Windows2019-Readme.md#visual-studio-enterprise-2019

…fter reporting it in the firefox Matrix chat, updating SpiderMonkey to mozilla-central tip See https://bugzilla.mozilla.org/show_bug.cgi?id=1904747

Xmader · 2024-07-02T22:04:30Z

TODO

Add CI to automatically update the commit to the tip and make pull requests (done in a7b92e8)
Figure out JobQueue for JS::AutoDebuggerJobQueueInterruption (see https://hg.mozilla.org/releases/mozilla-esr128/file/tip/js/src/builtin/Promise.cpp#l6946) (would be separate PRs)
Takeaways

…when mozilla-central is updated

philippedistributive · 2024-07-09T14:51:40Z

include/JobQueue.hh

@@ -78,6 +78,11 @@ void runJobs(JSContext *cx) override;
 */
 bool empty() const override;

+/**
+ * @return true if the job queue stops draining, which results in `empty()` being false after `runJobs()`.


Suggested change

* @return true if the job queue stops draining, which results in `empty()` being false after `runJobs()`.

* @return true if the job queue stopped draining, which results in `empty()` being false after `runJobs()`.

I copied this from https://hg.mozilla.org/mozilla-central/file/d25c9bd/js/public/Promise.h#l84.

I couldn't understand why you'll need to change this.
Is this grammatically incorrect?

indeed, yes, "stops" refers to the present or future, whereas "stopped" refers to the past

setup.sh

philippedistributive · 2024-07-09T14:56:44Z

setup.sh

+sed -i'' -e 's/shared_lib = self._pretty_path(libdef.output_path, backend_file)/shared_lib = libdef.lib_name/' ./python/mozbuild/mozbuild/backend/recursivemake.py
+sed -i'' -e 's/if version < Version(mac_sdk_min_version())/if False/' ./build/moz.configure/toolchain.configure # do not verify the macOS SDK version as the required version is not available on Github Actions runner
+sed -i'' -e 's/return JS::GetWeakRefsEnabled() == JS::WeakRefSpecifier::Disabled/return false/' ./js/src/vm/GlobalObject.cpp # forcibly enable FinalizationRegistry
+sed -i'' -e 's/return !IsIteratorHelpersEnabled()/return false/' ./js/src/vm/GlobalObject.cpp # forcibly enable iterator helpers


@caleb-distributive is this truly needed? Doesn't the switch in PyInit_pythonmonkey cover this?

See the commit message in 42e3f53

philippedistributive · 2024-07-09T14:59:03Z

src/ExceptionType.cc

@@ -276,7 +276,7 @@ JSObject *ExceptionType::toJsError(JSContext *cx, PyObject *exceptionValue, PyOb
      JS::RootedString filename(cx, JS_NewStringCopyZ(cx, PyUnicode_AsUTF8(fileName)));
      JS::RootedString message(cx, JS_NewStringCopyZ(cx, msgStream.str().c_str()));
      // stack argument cannot be passed in as a string anymore (deprecated), and could not find a proper example using the new argument type
-      if (!JS::CreateError(cx, JSExnType::JSEXN_ERR, nullptr, filename, lineno, 0, nullptr, message, JS::NothingHandleValue, &rval)) {
+      if (!JS::CreateError(cx, JSExnType::JSEXN_ERR, nullptr, filename, lineno, JS::ColumnNumberOneOrigin(1), nullptr, message, JS::NothingHandleValue, &rval)) {


Why no longer 0?

JS::ColumnNumberOneOrigin(1)

One Origin

JS::CreateError()'s function signature changed to CreateError(..., JS::ColumnNumberOneOrigin column,...)

See https://hg.mozilla.org/mozilla-central/file/d25c9bd/js/public/ErrorReport.h#l537

JS::ColumnNumberOneOrigin is defined in https://hg.mozilla.org/mozilla-central/file/d25c9bd/js/public/ColumnNumber.h#l324

philippedistributive · 2024-07-09T15:00:07Z

src/JobQueue.cc

+}
+
+bool JobQueue::isDrainingStopped() const {
+  // TODO (Tom Tang): implement this by detecting if the Python event-loop is still running


what's the specific challenge in implementing this?

It would have to modify the internal implementation of Python's asyncio event-loop.

src/modules/pythonmonkey/pythonmonkey.cc

philippedistributive · 2024-07-09T15:02:52Z

src/modules/pythonmonkey/pythonmonkey.cc

@@ -549,8 +550,6 @@ PyMODINIT_FUNC PyInit_pythonmonkey(void)

  JS::RealmCreationOptions creationOptions = JS::RealmCreationOptions();
  JS::RealmBehaviors behaviours = JS::RealmBehaviors();
-  creationOptions.setWeakRefsEnabled(JS::WeakRefSpecifier::EnabledWithoutCleanupSome); // enable FinalizationRegistry
-  creationOptions.setIteratorHelpersEnabled(true);


@caleb-distributive please double-check with change above: why is this new way better?

#367 (comment)

philippedistributive · 2024-07-09T15:04:58Z

tests/python/test_arrays.py

@@ -1098,7 +1098,8 @@ class Counter {
    assert (False)
  except Exception as e:
    assert str(type(e)) == "<class 'pythonmonkey.SpiderMonkeyError'>"
-    assert str(e).__contains__("TypeError: this is null")
+    assert "TypeError:" in str(e)


why is this better?

in is a basic operator in Python syntax.
https://realpython.com/python-in-operator/#strings

__contains__ is the magic method for user-defined classes to implement the in operator, which shouldn't be used directly.

https://docs.python.org/3/reference/expressions.html#membership-test-details

For the string and bytes types, x in y is True if and only if x is a substring of y. An equivalent test is y.find(x) != -1. Empty strings are always considered to be a substring of any other string, so "" in "abc" will return True.

For user-defined classes which define the __contains__() method, x in y returns True if y.__contains__(x) returns a true value, and False otherwise.

Xmader · 2024-07-10T18:55:51Z

@philippedistributive 53af133

setWeakRefsEnabled and setIteratorHelpersEnabled both no longer exist on JS::RealmCreationOptions - they moved to SpiderMonkey/Firefox prefs
(See https://hg.mozilla.org/mozilla-central/file/d25c9bd/modules/libpref/init/StaticPrefList.yaml#l7803).

Xmader requested a review from wesgarland June 19, 2024 04:37

Xmader added 16 commits June 19, 2024 04:44

chore: mozcentral.version file is required for building from source

cb468af

chore: Rust v1.76 is required to build the latest SpiderMonkey

5e1d162

Revert "chore: force the SpiderMonkey build system not to verify the …

12ed88b

…macOS SDK version" The hacky solution doesn't work. This reverts commit e4c37db.

chore: update setup.sh

647d083

chore: WIP: only macOS 14 runners can use SDK 14.4 (XCode 15.3)

b685d83

chore: WIP: can macOS 13 actually use SDK version 14.4?

4fdc927

chore: SpiderMonkey 125 should be good as it switched to macOS 14.4 S…

2d73bd5

…DK in 126 alpha Using the commit for mozilla-central tag `FIREFOX_NIGHTLY_125_END`

chore: try SpiderMonkey 120

3a07532

Using the commit for mozilla-central tag `FIREFOX_NIGHTLY_120_END`

chore: switch back to SpiderMonkey 115 should be fail-proof

275b0bf

Using the commit for mozilla-central tag `FIREFOX_NIGHTLY_115_END`

chore: rollback

8f70da7

Revert "chore: switch back to SpiderMonkey 115 should be fail-proof"

21e4719

This reverts commit 275b0bf.

Revert "chore: try SpiderMonkey 120"

72565d7

This reverts commit 3a07532.

chore: force the build system not to verify the macOS SDK version

a4ef58c

chore: change back to the tip commit

ff396af

chore: WIP: ignore Windows builds for now

04d028b

Xmader linked an issue Jun 19, 2024 that may be closed by this pull request

Build the spidermonkey bundle from mozilla-central instead of using the ESR #269

Closed

Xmader added this to the v1.0.0 milestone Jun 19, 2024

Xmader added 9 commits June 20, 2024 06:54

fix: passing a column number offset for JS source code now requires t…

492fa80

…he use of `JS::ColumnNumberOneOrigin` class after upgrading SpiderMonkey to mozilla-central tip

fix: extending JS::JobQueue class now needs to implement the `isDra…

f3df728

…iningStopped` virtual method after upgrading SpiderMonkey to mozilla-central tip

fix: for JS::NewExternalArrayBuffer, the custom destructor is now w…

79b6437

…rapped in a `mozilla::UniquePtr` instead of passing it directly to the function

fix: JSErrorReport's filename property is now a JS::UniqueChars…

09bacb5

… (`mozilla::UniquePtr<char[], ...>`) instead of a pure `char[]` after upgrading SpiderMonkey to mozilla-central tip

fix: setWeakRefsEnabled and setIteratorHelpersEnabled no longer p…

53af133

…resent on `JS::RealmCreationOptions` after upgrading SpiderMonkey to mozilla-central tip

feat: SpiderMonkey now supports APIs for one-byte latin1 JSExternalSt…

0415557

…rings

fix: JSErrorReport's filename property should now be a `JS::Const…

ec75f13

…UTF8CharsZ` instead of a pure `char*` after upgrading SpiderMonkey to mozilla-central tip

WIP: bug in defining the debugger global object, disable pmdb for now

39ee6af

Xmader added 8 commits June 28, 2024 19:39

chore: re-enable Windows builds

46c148a

This reverts commit 04d028b.

fix: in SpiderMonkey's js/src/configure script, the `--without-syst…

82bf020

…em-zlib` option is not available on Windows

chore: fix uncrustify build on Windows

6168acb

fix: crash in Debug Build of SpiderMonkey

4556799

docs(CI): explain why we need Windows 2022 runner to build SpiderMonkey

3f78041

chore(CI): use the Windows 2022 runner entirely

48bda0a

The JS_Utf8BufferIsCompilableUnit issue got fixed within 24 hours a…

da4f0da

…fter reporting it in the firefox Matrix chat, updating SpiderMonkey to mozilla-central tip See https://bugzilla.mozilla.org/show_bug.cgi?id=1904747

Xmader marked this pull request as ready for review July 2, 2024 22:04

Xmader and others added 2 commits July 4, 2024 20:40

chore(CI): create a CI action that automatically create pull request …

a7b92e8

…when mozilla-central is updated

Merge branch 'main' into Xmader/chore/moz-central

75c45be

philippedistributive requested review from philippedistributive and removed request for wesgarland July 8, 2024 16:16