CMB / edbrowse

A command-line editor and web browser.
Other
340 stars 31 forks source link

edbrowse doesn't compile on Termux #57

Closed OParczyk closed 4 years ago

OParczyk commented 4 years ago

Hi! I tried to compile edbrowse from source for Termux, as there is no package available as of now. When I do, I get the following errors: $ make cd src ; make make[1]: Entering directory '/data/data/com.termux/files/home/edbrowse/edbrowse/src' cc -DEDBROWSE_ON_LINUX -Wall -D_FILE_OFFSET_BITS=64 -c -o main.o main.c In file included from main.c:6: ./eb.h:518:2: error: unknown type name 'ushort' ushort bits; /* a bun... ^ main.c:151:2: warning: implicit declaration of function 'pthread_setcancelstate' is invalid in C99 [-Wimplicit-function-declaration] pthread_setcancelstate(PTHREAD_C... ^ main.c:151:25: error: use of undeclared identifier 'PTHREAD_CANCEL_ENABLE' pthread_setcancelstate(PTHREAD_C... ^ main.c:153:2: warning: implicit declaration of function 'pthread_setcanceltype' is invalid in C99 [-Wimplicit-function-declaration] pthread_setcanceltype(PTHREAD_CA... ^ main.c:153:24: error: use of undeclared identifier 'PTHREAD_CANCEL_ASYNCHRONOUS' pthread_setcanceltype(PTHREAD_CA... ^ main.c:156:2: warning: implicit declaration of function 'pthread_cancel' is invalid in C99 [-Wimplicit-function-declaration] pthread_cancel(t1); ^ 3 warnings and 3 errors generated. make[1]: * [<builtin>: main.o] Error 1 make[1]: Leaving directory '/data/data/com.termux/files/home/edbrowse/edbrowse/src' make: * [makefile:5: all] Error 2

When I #define ushort unsigned short in eb.h it becomes $ make cd src ; make make[1]: Entering directory '/data/data/com.termux/files/home/edbrowse/edbrowse/src' cc -DEDBROWSE_ON_LINUX -Wall -D_FILE_OFFSET_BITS=64 -c -o main.o main.c main.c:152:2: warning: implicit declaration of function 'pthread_setcancelstate' is invalid in C99 [-Wimplicit-function-declaration] pthread_setcancelstate(PTHREAD_C... ^ main.c:152:25: error: use of undeclared identifier 'PTHREAD_CANCEL_ENABLE' pthread_setcancelstate(PTHREAD_C... ^ main.c:154:2: warning: implicit declaration of function 'pthread_setcanceltype' is invalid in C99 [-Wimplicit-function-declaration] pthread_setcanceltype(PTHREAD_CA... ^ main.c:154:24: error: use of undeclared identifier 'PTHREAD_CANCEL_ASYNCHRONOUS' pthread_setcanceltype(PTHREAD_CA... ^ main.c:157:2: warning: implicit declaration of function 'pthread_cancel' is invalid in C99 [-Wimplicit-function-declaration] pthread_cancel(t1); ^ 3 warnings and 2 errors generated. make[1]: * [<builtin>: main.o] Error 1 make[1]: Leaving directory '/data/data/com.termux/files/home/edbrowse/edbrowse/src' make: * [makefile:5: all] Error 2

How do I fix this?

eklhad commented 4 years ago

It would not surprise me if some systems, besides windows, require typedef unsigned short ushort;

More surprising is that you have the header pthread.h, but it doesn't have the same symbols or function names, or is missing some important functions? Perhaps look through your pthread.h and advise me what is going on here.

Karl Dahlke

martinetd commented 4 years ago

eklhad wrote on Fri, Dec 06, 2019:

More surprising is that you have the header pthread.h, but it doesn't have the same symbols or function names, or is missing some important functions? Perhaps look through your pthread.h and advise me what is going on here.

bionic (the android libc) does not implement pthread_cancel, and there is no alternative there as far as I know.

So given the whole point here was to kill whatever thread is stuck in duktape without coming back to any edbrowse function for over 45s now I guess there is not much do do without pthread_cancel, we cannot emulate the feature, I would just #ifdef it out for android and hope they don't get stuck in js world...

-- Dominique

eklhad commented 4 years ago

I would just #ifdef it out for android and hope they don't

Hmm - yes we could take out the whole feature, or, we could spin up the new interactive thread and at least let the user save files etc, and the other thread marches on and chews up cpu cycles, since we can't cancel it - but the risk there is if the first thread finishes its js, then 2 threads are reading from standard input and you don't know who you're talking to. Or, if you turn js back on in the second thread and try to use it while the first thread still runs js it will totally blow!

I don't know I'll go with whatever you think on this one.

I had sort of envisioned this, to some degree, that I wouldn't let you turn js back on if the first thread was still running and couldn't be canceled, thus lang/msg-en line 568 and src/messages.h line 668

Karl Dahlke

martinetd commented 4 years ago

eklhad wrote on Fri, Dec 06, 2019:

that I wouldn't let you turn js back on if the first thread was still running and couldn't be canceled

That might be enough to just save data but I do not like having two threads possibly reading input if the js finally exits at some point.

I had a look and people suggest using pthread_exit() from a signal handler with a pthread_kill() if required as an alternative to pthread_cancel; most of the times since we do not use other threads much we will already be on foreground thread and we can just signal another thread if required like I did in my pthread_cancel branch just now.

I just tried and it seems to work, but I was on main thread to it did not test the pthread_kill. Do you have an idea to try?

(on a side note, be it old or new code we really exit the main thread and linux does not like that much -- edbrowse is listed as zombie after that even if it still reads input and can continue working for a while. Ideally we should spawn the inputForever function in a thread and have the main thread just keep joining input threads to be correct. But I guess we do not really care as long as it works)

-- Dominique

eklhad commented 4 years ago

I had a look and people suggest using pthread_exit() from a signal

Seems like I would have tried that first off. Did I try it and some reason it wouldn't work? Like maybe it just set an exit bit and when the thread got to a safe place it would exit, which it never got to, which is how cancel works unless you set those flags to cancel it any time. I don't remember now but it seems odd that I wouldn't have tried that first.

It's easy to test, write a silly web page with

If it interrupts and you're back to input and it looks good, still check the load average to make sure the other thread isn't still churning.

Karl Dahlke

martinetd commented 4 years ago

eklhad wrote on Fri, Dec 06, 2019:

If it interrupts and you're back to input and it looks good, still check the load average to make sure the other thread isn't still churning.

Yes, a simple while(true) in js gets interrupted fine from what I can tell, the behaviour seems indentical to pthread_cancel when the thread we are trying to cancel catches the signal.

My concern was that I did not test the pthread_kill() part because when I tried, pthread_self() always was equal to foreground_thread so it never went to the other if branch with the kill.

Please also give it a try, I think it should always work but this can always differently on other environments... It is in my 'pthread_cancel' branch, on git://github.com/martinetd/edbrowse.git

-- Dominique

OParczyk commented 4 years ago

Thanks to all of you for your interest!

When trying to compile with this branch I still have to #define ushort unsigned short in eb.h manually and get this error when I do:

html-tidy.c:14:10: fatal error: 'tidy.h' file not found

include

    ^~~~~~~~

1 error generated. make[1]: [: html-tidy.o] Error 1 make: [makefile:5: all] Error 2

and several complaints like

html.c:2884:9: warning: implicit declaration of function 'pthread_tryjoin_np' is invalid in C99 [-Wimplicit-function-declaration]

martinetd commented 4 years ago

Oliver Parczyk wrote on Fri, Dec 06, 2019:

When trying to compile with this branch I still have to #define ushort unsigned short in eb.h manually

Yes, I only patched the pthread_cancel stuff.

and get this error when I do:

html-tidy.c:14:10: fatal error: 'tidy.h' file not found

This is because you do not have tidy-html5 installed.. it looks like termux has a 'tidy' package that should work for you.

and several complaints like

html.c:2884:9: warning: implicit declaration of function 'pthread_tryjoin_np' is invalid in C99 [-Wimplicit-function-declaration]

That must be another android incompatibility; there already is a define to fix that for OSX and some BSD, we would need to include android there somehow. I just checked, it looks like you can use ANDROID as a macro to tell it appart, so at start of html.c you can check if that is defined for the pthread_tryjoin_np define

-- Dominique

OParczyk commented 4 years ago

Thanks! installed tidy and duktape now and it dies with

/data/data/com.termux/files/usr/bin/aarch64-linux-android-ld: html.o: in function runTimer': html.c:(.text+0x755c): undefined reference to pthread_tryjoin_np' /data/data/com.termux/files/usr/bin/aarch64-linux-android-ld: html.c:(.text+0x7560): undefined reference to pthread_tryjoin_np' /data/data/com.termux/files/usr/bin/aarch64-linux-android-ld: stringfile.o: in function envFile': stringfile.c:(.text+0x3944): undefined reference to glob' /data/data/com.termux/files/usr/bin/aarch64-linux-android-ld: stringfile.c:(.text+0x3a28): undefined reference to globfree' /data/data/com.termux/files/usr/bin/aarch64-linux-android-ld: stringfile.c:(.text+0x3a50): undefined reference to globfree' /data/data/com.termux/files/usr/bin/aarch64-linux-android-ld: stringfile.c:(.text+0x3a8c): undefined reference to globfree' /data/data/com.termux/files/usr/bin/aarch64-linux-android-ld: stringfile.c:(.text+0x3ab0): undefined reference to `globfree' clang-9: error: linker command failed with exit code 1 (use -v to see invocation) make[1]: [GNUmakefile:75: edbrowse] Error 1 make: [makefile:5: all] Error 2

I'd guess that's related to the last thing you mentioned.

martinetd commented 4 years ago

Oliver Parczyk wrote on Fri, Dec 06, 2019:

and several complaints like

I fixed the ushort and pthread_tryjoin_np as well in a new 'android' branch on my tree, this compiles on termux for me now.

Also had to change an hard-coded /tmp path to use the TMPDIR environment variable after all that.

There seem to be a bug in the termux libduktape package that creates a libduktape.so but when linking the resulting binary expects a libduktape.so.1 to exist and it does not, I had to create a link manually... This should be reported to termux; I wonder who packaged duktape there, did not expect to find a package.

-- Dominique

martinetd commented 4 years ago

Oliver Parczyk wrote on Fri, Dec 06, 2019:

/data/data/com.termux/files/usr/bin/aarch64-linux-android-ld: stringfile.o: in function envFile': stringfile.c:(.text+0x3944): undefined reference to glob' /data/data/com.termux/files/usr/bin/aarch64-linux-android-ld: stringfile.c:(.text+0x3a28): undefined reference to globfree'

This is weird, it is defined in libandroid-glob.so which is in the same package as the glob.h file you must have to get this far (libandroid-glob package)

You can try linking manually (adding -landroid-glob to your LDFLAGS), but that really should not be required...

-- Dominique

OParczyk commented 4 years ago

So after linking libduktape.so to libduktape.so.1 and adding the flags I get greeted by the obligatory

.ebrc: unrecognized keyword linelength at line 49 edbrowse ready

so I guess that works now. Leaving that here for future reference: LDFLAGS=-landroid-glob make

Thanks for your help!

eklhad commented 4 years ago

I pushed your pthread_exit code cause it works for me too; hope it works on all the other systems.

Karl Dahlke

eklhad commented 4 years ago

Ok I pushed some commits from here and there; follow master and I think bsd and android should compile, if I didn't screw something up, although still not sure about the glob issue. If you need to specify libraries then we might need an android specific makefile, or adjustment to the cmake system.

As for linelength, unrecognized keywords in the config file don't cause any trouble. That keyword went away because you can specify line length dynamic at any time with the ll command, and if you want it at startup you can put it in the init function

ll 500

And whenever you can do something like that I don't see the need for an extra config command. So that's the story of that, just remove linelength and you're fine.

Karl Dahlke

OParczyk commented 4 years ago

Hi! Sorry for my very late comment, but I think you've missed a hardcoded /tmp (cannot create temporary directory /tmp/.edbrowse)

@martinetd seems to have fixed this on his branch (also he mentioned it in a previous comment)

~ Oliver Parczyk

eklhad commented 4 years ago

@martinetd seems to have fixed this on his branch

Yes, now in master.

Karl Dahlke

OParczyk commented 4 years ago

Works fine now, thank you for dealing w/ this issue!