-
Notifications
You must be signed in to change notification settings - Fork 216
Update meson.build #1192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update meson.build #1192
Conversation
fixup: 5a949ca ("CMake: Set _WIN32_WINNT=0x0601", quickjs-ng#900)
the original is -Ddisable_parser, invert that to make it less confusing
fixup: 8dda2a0 ("fixed hash_map_resize() - added Map/WeakMap in microbench")
fixup: 0a00011 ("Optimize {__proto__:null} object literal creation", quickjs-ng#1118)
fixup: 08db51a ("Add ref-counted string slices", quickjs-ng#1175)
skips building: qjs/qjsc cli, examples, tests, codegen
also removed -M during the test_fib.c codegen: 902cc2c ("Add native module support on Windows") switched examples to dynamic import() instead of static import statement and the lookup was deferred to runtime, thus we don't need to hint qjsc this is a declared module and exclude it from bundling.
Fix unable to `import {fib} from './fib.so';` using qjs cli
|
last 2 commits require #1194 edit: dropped |
libunicode.c
Outdated
| #if __has_include(<codegen_libunicode-table.h>) | ||
| #include <codegen_libunicode-table.h> | ||
| #else | ||
| #include "libunicode-table.h" | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems unnecessary? The point of unicode_gen is to produce libunicode-table.h that then gets checked into git.
Us maintainers don't use meson so it's not necessary to have meson build any of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the point of this is explained in #1187, some distributions may like to avoid pregenerated sources
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could drop this change and keep it downstream, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say let's drop it please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dropped
meson.build
Outdated
| examples = get_option('examples').disable_auto_if(meson.is_subproject()) | ||
|
|
||
| if tests.allowed() | ||
| broken_test = disabler() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my curiosity, what does that do / how does that work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if any target depends on a disabler, it will also become a disabler, which is a no-op
3c9f47a to
f2aead8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a meson expert by any stretch but LGTM insofar I can judge that.
@saghul you want to take a look as well or should I go ahead and merge it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a question, does this PR depend on the qjsc changes?
| $(QJSC) -e -o gen/hello.c examples/hello.js | ||
| $(QJSC) -e -o gen/hello_module.c -m examples/hello_module.js | ||
| $(QJSC) -e -o gen/test_fib.c -M examples/fib.so,fib -m examples/test_fib.js | ||
| $(QJSC) -e -o gen/test_fib.c -m examples/test_fib.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this change depend on the other PR changing qjsc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
See the referenced commits in messages.
Split from #1187