Opened 10 years ago
Last modified 9 years ago
#2462 closed enhancement
[PATCH] SpiderMonkey ESR31 upgrade — at Version 37
Reported by: | Yves | Owned by: | Yves |
---|---|---|---|
Priority: | Should Have | Milestone: | Alpha 18 |
Component: | Core engine | Keywords: | spidermonkey javascript |
Cc: | Patch: |
Description (last modified by )
Change History (37)
comment:1 by , 10 years ago
comment:2 by , 10 years ago
The better to start on the upgrade now rather than wait until after it's been released ;)
comment:3 by , 10 years ago
But in that case you take the risk that you need to change drastically if the guys @Mozilla do so.
comment:4 by , 10 years ago
There are two big advantages when starting with the upgrade now.
- If we find problems that need to be fixed in SpiderMonkey, we can still get these changes in before the next release. The Firefox (and SpiderMonkey) development starts in the mozilla central repository where big features get integrated. After that it moves to the "Aurora" stage where usually only important bugfixes get applied. This transition will be around the 28th of April, so it's good to start now.
- When updating around once a week, it's easier to figure out the reason if a change breaks something. You can check last week's commit logs or ask people what happened last week. That's harder if you have to search in a period of multiple months.
Sometimes there are bugs and it can take a while to figure out it's not a problem in our code, but the advantages outweight this problem.
comment:5 by , 10 years ago
Description: | modified (diff) |
---|
comment:6 by , 10 years ago
C++11 support is required to use SpiderMonkey 31. I have checked today if patching the headers would be an alternative to avoid that, but it would result in too deep changes. I've replaced all the static_asserts, but then didn't investigate any further when I figured out that the headers also use move constructors. I've created ticket #2669 for the C++11 migration.
comment:13 by , 10 years ago
Description: | modified (diff) |
---|
comment:22 by , 10 years ago
Milestone: | Alpha 17 → Alpha 18 |
---|
comment:27 by , 9 years ago
Keywords: | review added |
---|---|
Summary: | SpiderMonkey ESR31 upgrade → [PATCH] SpiderMonkey ESR31 upgrade |
The ESR31 branch is ready for review.
Open tests:
- Test on the Mac
Notes:
- I'm not using a new directory for the SpiderMonkey headers, this means they are part of the diff. You can make the diff more readable this way:
git diff master --name-status | grep -v 'libraries/source/spidermonkey/include-win32' | xargs git diff master --
TODOs (before committing):
- Add new Windows binaries compiled with the for .. of bugfix
- Revert VS2010 compatibility patches (if agreed to drop support for VS versions <2013)
TODOs (C++11 related, before committing)
- Agree definitely on dropping support for GCC < 4.6
- Agree definitely on dropping support for Visual Studio < 2013
comment:28 by , 9 years ago
Just a note that has_broken_pch can be removed from premake4.lua and the GCC check updated to fail with GCC < 4.6.
Also should with-c++11 GCC option forced enabled?
follow-ups: 31 33 comment:30 by , 9 years ago
Some initial comments on the ESR31 branch:
- When building (Ubuntu 14.04, gcc 4.8.2):
In file included from ../../../source/third_party/tinygettext/src/dictionary_manager.cpp:20:0: ../../../source/third_party/tinygettext/include/tinygettext/dictionary_manager.hpp:88:59: warning: ‘auto_ptr’ is deprecated (declared at /usr/include/c++/4.8/backward/auto_ptr.h:87) [-Wdeprecated-declarations] void set_filesystem(std::auto_ptr<FileSystem> filesystem); ^ ../../../source/third_party/tinygettext/src/dictionary_manager.cpp:237:72: warning: ‘auto_ptr’ is deprecated (declared at /usr/include/c++/4.8/backward/auto_ptr.h:87) [-Wdeprecated-declarations] DictionaryManager::set_filesystem(std::auto_ptr<FileSystem> filesystem_) ^
I guess we either want to fix our copy of tinygettext, or disable the warnings.
source/scriptinterface/third_party/ObjectToIDMap.h
: The class isObjectIdCache
, so shouldn't this beObjectIdCache.h
?
LICENSE.txt
: Added trailing whitespace. Also, quite a few other changes to source files are adding trailing whitespace. (Not hugely important, but it makes "git diff" unhappy.)
README.txt
: Need to be careful not to apply that change to SVN.
build/premake/premake4.lua
: Since the use of--with-c++11
was removed, the correspondingnewoption
line should be removed too.
build/workspaces/clean-workspaces.sh
: Should continue to deletemozjs24
(at least for a few months), else people who update a dirty workspace will havemozjs24
hanging around forever.
libraries/osx/build-osx-libs.sh
: Why change to$CPPFLAGS
? That doesn't seem to be defined, and NSPR seems to include some C++ code so I'd guess it ought to still haveCXXFLAGS
set.
libraries/source/spidermonkey/
: Might be nice if something likeREADME.txt
explained where all the.diff
files come from and why they're needed (particularly so that people can tell when they're not needed any more and can be deleted).
comment:31 by , 9 years ago
Replying to Philip:
Some initial comments on the ESR31 branch:
- When building (Ubuntu 14.04, gcc 4.8.2):
...
I guess we either want to fix our copy of tinygettext, or disable the warnings.
I've seen those and I agree that we have to fix/disable them, but they are related to C++11 and not SpiderMonkey.
source/scriptinterface/third_party/ObjectToIDMap.h
: The class isObjectIdCache
, so shouldn't this beObjectIdCache.h
?
They renamed the class to ObjectToIDMap in later versions of SpiderMonkey. I've first used the newer version and then copied the code from an older version. I think I'll leave it this way and it will match after the next upgrade.
LICENSE.txt
: Added trailing whitespace. Also, quite a few other changes to source files are adding trailing whitespace. (Not hugely important, but it makes "git diff" unhappy.)
I've fixed most of these now.
README.txt
: Need to be careful not to apply that change to SVN.
I've reverted the changes in README.txt just to be sure.
build/premake/premake4.lua
: Since the use of--with-c++11
was removed, the correspondingnewoption
line should be removed too.
That was only a quick change to enable c++11 for this branch. I'm going to make a separate commit that fixes some clang issues, removes generation of unsupported workspaces and removes some workarounds for GCC < 4.2.
build/workspaces/clean-workspaces.sh
: Should continue to deletemozjs24
(at least for a few months), else people who update a dirty workspace will havemozjs24
hanging around forever.
Done
libraries/osx/build-osx-libs.sh
: Why change to$CPPFLAGS
? That doesn't seem to be defined, and NSPR seems to include some C++ code so I'd guess it ought to still haveCXXFLAGS
set.
That's an issue, indeed. I've reverted an older version and haven't noticed the code has been changed in the meantime (r15410). Fixed.
libraries/source/spidermonkey/
: Might be nice if something likeREADME.txt
explained where all the.diff
files come from and why they're needed (particularly so that people can tell when they're not needed any more and can be deleted).
There's some description in build.sh and some patches contain some information in the header comment. All of them shouldn't be needed anymore in the next version. Two are backports and the third is a mix of backports and one new fix (tracelogger flushing). I've added some more information to these comments.
comment:33 by , 9 years ago
Replying to Philip:
Some initial comments on the ESR31 branch:
- When building (Ubuntu 14.04, gcc 4.8.2):
In file included from ../../../source/third_party/tinygettext/src/dictionary_manager.cpp:20:0: ../../../source/third_party/tinygettext/include/tinygettext/dictionary_manager.hpp:88:59: warning: ‘auto_ptr’ is deprecated (declared at /usr/include/c++/4.8/backward/auto_ptr.h:87) [-Wdeprecated-declarations] void set_filesystem(std::auto_ptr<FileSystem> filesystem); ^ ../../../source/third_party/tinygettext/src/dictionary_manager.cpp:237:72: warning: ‘auto_ptr’ is deprecated (declared at /usr/include/c++/4.8/backward/auto_ptr.h:87) [-Wdeprecated-declarations] DictionaryManager::set_filesystem(std::auto_ptr<FileSystem> filesystem_) ^I guess we either want to fix our copy of tinygettext, or disable the warnings.
#2522 is relevant and should fix those warnings.
comment:36 by , 9 years ago
Replying to stanislas69:
Do I need VS2013 ?
Yes, from now on you need VS2013 to build on Windows. We have decided that it's not worth the additional efforts and trouble to support older versions with C++11. It's already a challenge to support three different platforms and compilers (not only with C++11, but it gets a bit harder now).
comment:37 by , 9 years ago
Description: | modified (diff) |
---|
I thought SpiderMonkey 3.1 will be released in July 2014?
"The next release will be SpiderMonkey 31, around July 2014." https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey