lambdanative icon indicating copy to clipboard operation
lambdanative copied to clipboard

New module lnjscheme

Open 0-8-15 opened this issue 4 years ago • 25 comments

This depends on #386

Any not-so-time-critical Android API/Java Method - except those restricted Android itself may be called without adding JNI code.

0-8-15 avatar Nov 02 '20 11:11 0-8-15

HOLD ON

I found a dependency issue in this PR, which leads to an idea of generalization. Stay tuned for updates.

0-8-15 avatar Nov 02 '20 16:11 0-8-15

I am a bit concerned about my ability (or lack thereof) to review this as it makes changes to 52 files (1400 lines of code), most of which have nothing to do with the lnJScheme module and its ties into the event system. There are things that now go merged slightly differently, moving code around, line end changes etc - this is in part the fault of how Github presents it as both this branch and the master have moved forward in different aspects. In principle there are four component: a) a lnJScheme new module (which I don't really understand and am hesitant to merge as I don't want future complaints as I got when I accepted #268 without considering future implications), b) a webview module, c) new jScheme related events, and d) a demo app for this module?

mgorges avatar Nov 20 '20 17:11 mgorges

I am a bit concerned about my ability (or lack thereof) to review this as it makes changes to 52 files (1400 lines of code), most of which have nothing to do with the lnJScheme module

If I'm not mistaken all too much, than these many changes are simply a consequence of the git rebase of the PR I did in order to incorporate the upstream changes.

and its ties into the event system.

Yes it does. That allocates a single event number (126) and dispatches to a possibly registered receiver - or is ignored as before.

There are things that now go merged slightly differently, moving code around, line end changes etc - this is in part the fault of how Github presents it as both this branch and the master have moved forward in different aspects.

Sorry for whitespace. I'm slightly visually impaired (a cataract surgery which did not take the 99% turn). I can't see these things.

In principle there are four component:

a) a lnJScheme new module (which I don't really understand and am hesitant to merge as I don't want future complaints as I got when I accepted #268 without considering future implications),

Yes. Though I can't follow your implications.

Personally I'm using this module now for a 14 month, according to git.

In principle I feel that this module has the capacity to put an end to (almost) all the endless JNI battles wrt. Android API changes, as it enables dynamic calls to Java API accessible by JNI and reflections under Android.

That's the main idea of the whole thing.

Time critical things might still need to have their own JNI.

But more or less "simple" calls to Android would be trivial up to the point that one could ship a script file for an alternative to the webview module.

b) a webview module

that's part of part II: the demo case. (Originally those where two separate branches, but after git rebase it did not make much sense to try to dissect them onto different branches anymore.)

c) new jScheme related events

Belongs to part I: a very simple Scheme interpreter (~32Kb) running on the Java side, possibly within the GUI thread.

There is one event number allocated. The implementation is supposed to not fail.

d) a demo app for this module?

For both the modules. The demo has two buttons:

  • evaluate a script file in the Java-side Scheme interpreter. Great for testing API calls without the compile/test cycle of Java.

  • the lambdanative.org website viewed by the webview as part of Android.

(((This will hopefully inspire ideas how t get over the need of the extra handling of hybrid apps in the future. Just change the Android layout on the fly.)))

This webview module is essentially a (stub) application of the lnjscheme module. The idea is to evolve this in to a more capable browser. Right now it's still easier/faster to load than going through the consequences of launch_url -- which for people like me always mean to select on of the browsers choice to use just this one time.

0-8-15 avatar Nov 20 '20 20:11 0-8-15

GO AHEAD -- just managed to compile upstream and found it working.

Notes on the actual changes:

  • moved the jscheme implementation from demo app to module

  • rewritten in the wake of Android disabling some Java features (parts of JNI and reflection)

  • allocated on event number and hook into it. (Should not do damage if the event number is not used elsewhere).

  • moved the 'webview' script into a module of it's own; this module is a bit of a stub. Might receive updates in order to become a nicer looking browser. Meawhile still intended to showcase how to call Java via jScheme

  • updated the demo app to used the new modules

0-8-15 avatar Nov 21 '20 14:11 0-8-15

According to my new gained understanding we better free JNI references. See:

https://www.ibm.com/support/knowledgecenter/en/SSYKE2_8.0.0/com.ibm.java.vm.80.doc/docs/jni_refs.html

Note that this MAY have more consequences elsewhere (if true) and MAY explain why Apps don't resume or die after a while.

0-8-15 avatar Nov 22 '20 16:11 0-8-15

I have tried to merge this in three commits - I can't really test it but it shouldn't have broken existing things in the base LambdaNative implementation. A few todo items, please:

  1. Please add licences to the top of modules/webview/webview.scm and modules/ln_jscheme/ln_jscheme.scm; note that I renamed lnjscheme to ln_jscheme for consistency in the module path only, I didn't change any of the internals
  2. ANDROID_java_public_ProxySettings is a new hook that isn't available in our build system, so that seems missing.
  3. modules/ln_jscheme/Makefile exists - shouldn't this be integrated into the regular build process, or am I missing something?
  4. Please add documentation for all public-facing functions in modules/ln_jscheme and modules/webview to the Wiki

mgorges avatar Nov 27 '20 06:11 mgorges

Actually also 5. Shouldn't modules/webview be called modules/android-webview as unlike the other one we have this one we have for HybridApps, it doesn't support iOS (or any other platform)?

mgorges avatar Nov 27 '20 06:11 mgorges

2. ANDROID_java_public_ProxySettings is a new hook that isn't available in our build system, so that seems missing.

Yeah, that's a horrible thing to have, which just happens to work. However the Androidoverlords are likely unhappy no matter what we do. At one of my phones it does NOT work at all.

That's bad, but what can we do? Fix Android to enable sane use? Hardly we can.

3. modules/ln_jscheme/Makefile exists - shouldn't this be integrated into the regular build process, or am I missing something?

Maybe it should. At least for documentation how to build the .jar from the source code it should IMHO be there.

0-8-15 avatar Nov 29 '20 17:11 0-8-15

1. Please add licences to the top of modules/webview/webview.scm and modules/ln_jscheme/ln_jscheme.scm;

TBD

note that I renamed lnjscheme to ln_jscheme for consistency in the module path only, I didn't change any of the internals

While I can't really follow the motivation behind the rename, I'm not held back with the consequences.

As you changed a lot of whitespace in between (which, for visually impaired persons like me, is a really annoying thing to follow as I can not see it) , removed the language and comments controlling syntax highlighting and whitespace policy (similarly) and did this rename all in one change I ran into a huge conflict battle. (If you only had this done step-by-step!!)

Now I managed to resolve those conflicts. Code wise that is.

The result in practice: none of my apps are really working anymore after git pull upstream master.

It will take time to get a working environment back.

0-8-15 avatar Nov 29 '20 17:11 0-8-15

Actually also 5. Shouldn't modules/webview be called modules/android-webview as unlike the other one we have this one we have for HybridApps, it doesn't support iOS (or any other platform)?

In principle I don't see why this module should - eventually - be Android specific.

Right now it is. But it should be "just a matter of work to be done" to make webkit render on openGL .

0-8-15 avatar Nov 29 '20 17:11 0-8-15

maybe it had been a good idea to include the android_jars subdirectory from the proposed patch into master.

:-(

Edited: or at least the Makefile to create it. Even though that would not have helped much as it would still have required the user to run it once manually (until there is a way to run module specific code upon compile - while I have an idea how that should be possible without changing lambdanative at all - it's not yet there).

0-8-15 avatar Nov 29 '20 18:11 0-8-15

While I can't really follow the motivation behind the rename, I'm not held back with the consequences.

It was internally consistent to name it as all ln-functions were of the ln_xxx way.

As you changed a lot of whitespace in between [...] (If you only had this done step-by-step!!)

This was already a ton of work and you also didn't break them into single commit. As we discussed there was no way to merge 27 commits which touched a bunch of unrelated files due to the strange rebase.

removed the language and comments controlling syntax highlighting and whitespace policy (similarly) [...]

Polluting every file with this Emacs thing (at inconsistent places) is super annoying; either find a global setting or some other way to do this. We use Eclipse or vim. I am also worried that this will break things in the Android substitution.

It will take time to get a working environment back.

This is surprising as I merged all the files you changed. So not sure why it is now broken on your side?

mgorges avatar Nov 29 '20 18:11 mgorges

maybe it had been a good idea to include the android_jars subdirectory from the proposed patch into master.

I only see a deleted file with this name in the pull request?

mgorges avatar Nov 29 '20 18:11 mgorges

Right now it is. But it should be "just a matter of work to be done" to make webkit render on openGL .

Don't we already have a webview that works for iOS?

mgorges avatar Nov 29 '20 18:11 mgorges

Yeah, that's a horrible thing to have, which just happens to work. However the Androidoverlords are likely unhappy no matter what we do. At one of my phones it does NOT work at all.

Sorry, what I am pointing out is that we don't have the substitution for it in our build system so this does nothing.

mgorges avatar Nov 29 '20 18:11 mgorges

maybe it had been a good idea to include the android_jars subdirectory from the proposed patch into master. I only see a deleted file in the pull request?

I believe this is as the .gitignore you merged includes *.jar, which is why it was not in there - am I moving the one from the previous app?

mgorges avatar Nov 29 '20 19:11 mgorges

Am Sun, 29 Nov 2020 11:05:53 -0800 schrieb Matthias Görges [email protected]:

maybe it had been a good idea to include the android_jars subdirectory from the proposed patch into master.
I only see a deleted file in the pull request?

I believe this is as the .gitignore includes *.jar, which is why it was not in there - am I moving the one from the previous app?

Yes. It's unchanged.

0-8-15 avatar Nov 30 '20 09:11 0-8-15

Am Sun, 29 Nov 2020 10:58:20 -0800 schrieb Matthias Görges [email protected]:

Yeah, that's a horrible thing to have, which just happens to work. However the Androidoverlords are likely unhappy no matter what we do. At one of my phones it does NOT work at all.
Sorry, what I am pointing out is that we don't have the substitution for it in our build system so this does nothing.

Maybe I don't understand your remark.

A far as I can see, this File ends up as a ProxySetting.java with a "package @PACKAGENAMEDOT@;" prepended. If missing, the webview part should not even compile.

What am I missing?

0-8-15 avatar Nov 30 '20 09:11 0-8-15

Am Sun, 29 Nov 2020 10:56:38 -0800 schrieb Matthias Görges [email protected]:

Right now it is. But it should be "just a matter of work to be done" to make webkit render on openGL .
Don't we already have a webview that works for iOS?

I don't know about iOS.

For Linux&Windows at least an external Browser is started.

0-8-15 avatar Nov 30 '20 09:11 0-8-15

Am Sun, 29 Nov 2020 10:52:45 -0800 schrieb Matthias Görges [email protected]:

While I can't really follow the motivation behind the rename, I'm not held back with the consequences.
It was internally consistent to name it as all ln-functions were of the ln_xxx way.

As you changed a lot of whitespace in between [...] (If you only had this done step-by-step!!)
This was already a ton of work and you also didn't break them into single commit. As we discussed there was no way to merge 27 commits which touched a bunch of unrelated files due to the strange rebase.

Due to the recent "discovery" that onCreate() may run several times during the lifetime of a process, I wanted to go back and double check what the effect on this code would be.

I knew that the whole pull request became a bit stale already due to the rebase on top of modified merges. The idea was to try a squash commit. But those things take time.

removed the language and comments controlling syntax highlighting and whitespace policy (similarly) [...]
Polluting every file with this Emacs thing (at inconsistent places) is super annoying; either find a global setting or some other way to do this.

Hm, how do you guys do this? How do you specify the syntax mode etc. for files like ANDROID_java_whateverDotNothing?

We use Eclipse or vim. I am also worried that this will break things in the Android substitution.

I'm no fan of editor wars. That's for the young. After a few years an editor is sticky and the fingers. Automated. After 30 years - hardly any other editor can ever compete.

I don't mind you using other editors. I would not mind to have such tags for other editors in the source too. Just don't try to sell it to me. I tried Eclipse a decade ago. Too slow, too much mouse grabbing.

Since my eyesight weakened, I'm really depending on syntax highlighting to work, automatic code reformatting to stick enforce traditional rules, git tagging trailing whitespace with a big red flag etc.

What always surprises me is to meet people who code LISP (Scheme) and have not yet figured out how much emacs make thing easier and automatic.

It will take time to get a working environment back.
This is surprising as I merged all the files you changed. So not sure why it is now broken on your side?

I have absolutely no idea what's broken.

I guess it's two or three single small things.

Some apps (among them, the main "test" app) don't start at all. One starts and seems to freeze -- until I press the back button, at which point it starts to render and shows it's been working all time.

Some functionality just does not work - webview at least no longer starts within my main app.

And the foreground process indicator does not come up, or at least is no longer up once the app does not cover the status line anymore.

:-/

Edited to add: some of the comments, which are gone now, where comments indicating begin, end and source of files included during the substitution process. Those where added to ease navigation and linking back from the merged file to the sources - I bet those would have been helpful regardless of the editor in use.

0-8-15 avatar Nov 30 '20 09:11 0-8-15

Maybe I don't understand your remark. A far as I can see, this File ends up as a ProxySetting.java with a "package @PACKAGENAMEDOT@;" prepended. If missing, the webview part should not even compile. What am I missing?

I guess this is simply a confusion of my part, as all other things like ANDROID_java_oncreate are things to be substituted in the build process using the subtool in targets/android/build-binary. However, this one, which is named ANDROID_java_public_ProxySettings is not explicitly mentioned there. Rather, it is handled intrinsically using code introduced in 2016 but which I had not noticed before.

mgorges avatar Nov 30 '20 10:11 mgorges

Right now it is. But it should be "just a matter of work to be > done" to make webkit render on openGL . Don't we already have a webview that works for iOS? I don't know about iOS. For Linux&Windows at least an external Browser is started.

modules/hybridapp/hybridapp.scm does this via defining USE_HYBRID in modules/hybridapp/IOS_CFLAG_ADDITIONS, with the actual code being in loaders/ios/WKWebViewController.m and loaders/ios/WKWebViewAppDelegate.m?

mgorges avatar Nov 30 '20 10:11 mgorges

Sorry, but I feel really compelled to add one more remark wrt. readabilty of the source. One, which I do well understand that it might not be obvious to people still having full vision:

People like me can not distinguish between one or two white spaces, on or two brackets. Hardly be sure what kind of brackets they are looking at (and hence will never attempt to count them). The worst one can do to them is something like

l(){

This could be anything for me. I could not tell it apart from I({) , (I){ , I({) or similar variation.

If you want to be able to read your code when aged, just *never have adjacent braces of different associativity (left/right) without intervening space.

0-8-15 avatar Nov 30 '20 10:11 0-8-15

Edited to add: some of the comments, which are gone now, where comments indicating begin, end and source of files included during the substitution process. Those where added to ease navigation and linking back from the merged file to the sources - I bet those would have been helpful regardless of the editor in use.

Fine, I added those back in commit 7817106, but then to be consistent they all need end tags too?

mgorges avatar Nov 30 '20 10:11 mgorges

Am Mon, 30 Nov 2020 02:30:46 -0800 schrieb Matthias Görges [email protected]:

Edited to add: some of the comments, which are gone now, where comments indicating begin, end and source of files included during the substitution process. Those where added to ease navigation and linking back from the merged file to the sources - I bet those would have been helpful regardless of the editor in use.

Fine, I added those back in commit 7817106, but then to be consistent they all need end tags too?

For consistency: yes.

0-8-15 avatar Nov 30 '20 11:11 0-8-15