svaarala / duktape

Duktape - embeddable Javascript engine with a focus on portability and compact footprint
MIT License
5.97k stars 515 forks source link

Duktape fails to compile with DJGPP (DOS port of GCC) #2472

Open shdon opened 2 years ago

shdon commented 2 years ago

There is a very simple fix, though... I added in

#ifdef __DJGPP
#undef DUK_USE_DATE_PRS_STRPTIME
#endif

in duk_config.h after the OS/compiler detection logic and my test program compiled and ran flawlessly.

svaarala commented 2 years ago

Thanks @shdon, I'll try to figure out a suitable fix. It'd be great if you could try the fix :)

svaarala commented 2 years ago

@shdon Could you provide me output from the following:

djgpp -E -x c -dM /dev/null

I'm trying to figure out which existing platform djgpp currently matches (I'm guessing it's generic Unix but just want to be sure). It can maybe be handled there as a spot fix, or maybe djgpp should have its own platform file.

shdon commented 2 years ago

I figure the most relevant ones are (the variants of) __MSDOS__, __unix__, __GNUC__ and __DJGPP.

defines.txt

svaarala commented 2 years ago

Just to be sure, could you show the printout from:

print(Duktape.env);
shdon commented 2 years ago

Using Duktape's built-in print function it terminates with a SIGABRT, but substituting in my own implementation, I get the output: ll p nl p2 a8 x86 unknown gcc

svaarala commented 2 years ago

Ok, so it's most likely detected as "generic UNIX".

svaarala commented 2 years ago

Any idea why the built-in print() would SIGABRT?

svaarala commented 2 years ago

@shdon Could you check if https://github.com/svaarala/duktape/pull/2473 works for you, and what Duktape.env you get with it?

shdon commented 2 years ago

Had some trouble getting configure.py to run, as I didn't have PyYAML that was compatible with Python 2 (configure.py doesn't like Python 3) and compiling duktape.c takes a few minutes on my old DOS machine, but eventually got it done. The new Duktape.env is ll p nl p2 a8 x86 msdos djgpp

Next I'll try to see if I can find out where duk__print is failing.

shdon commented 2 years ago

Ah, you can ignore that bit. I got the Duktape.env from my test program, which simply didn't include print() and alert(), so the SIGABRT was effectively just a ReferenceError: identifier 'print' undefined.

svaarala commented 2 years ago

:+1: So looks good in this pull now?

svaarala commented 2 years ago

Re: SIGABRT, it's best practice for the first call into Duktape to be a protected one (e.g. duk_safe_call()) so that you should not easily be able to invoke an abort.

shdon commented 2 years ago

This was just a very simple 3-line test program:

duk_context *ctx = duk_create_heap_default ();
duk_eval_string_nosult (ctx, "print(Duktape.env);");
duk_destroy_heap (ctx);

Mostly as the build environment has been a nightmare to set up and I didn't want to jump through hoops to get the cmdline sample to build, but ended up doing that anyway.

And yeah, it looks good! Glad to see it is set to show up in 2.8.0 already, without having to wait for 3.0.0 😊

svaarala commented 2 years ago

Ok, merging -- thanks for the testing effort!

phcoder commented 2 years ago

Can we have a .1 release with this? I'm porting tic80 to djgpp and this is one of pieces it needs