Skip to content
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

Proper fix for osproc.nim on Android #5646

Merged
merged 4 commits into from
Apr 2, 2017
Merged

Conversation

cheatfate
Copy link
Member

Fix sigtimedwait to use direct syscall to __NR_rt_sigtimedwait (available since Android SDK L3).

Testing needed...

@ghost
Copy link

ghost commented Apr 1, 2017

yay, I can test this. What do I need to install Nim in Termux fully from scratch (I have Termux installed)? I have Android 6.0

@stisa
Copy link
Contributor

stisa commented Apr 1, 2017

@cheatfate just tested this, thanks a lot!
I get a type mismatch when doing koch boot:
Error: type mismatch: got (clong) but expected 'cint = int32'

Changing the return type of syscall from clong to cint seems to "fix" it, and I can do koch boot fine.

@TiberiumN : my steps (android 7, 64bit) :

apt install git
apt install clang
git clone https://github.com/cheatfate/Nim -b android_stwait
cd Nim && git clone https://github.com/nim-lang/csources
apt install libandroid-glob && apt install libandroid-glob-dev

In csources/build.sh line 32: LINK_FLAGS="${LDFLAGS:-} -landroid-glob "

In whatever c_source/#_# ( mine was 2_2 , I think it's 2_2 for 64bit and 2_1 for 32bit ) folder it's building, in stdlib_osproc.c change "bin/sh" to "sh" (line 400)
now in csources, sh ./build.sh should work and bootstraps nim.
To build devel then:
In config/nim.cfg add passL=" -landroid-glob " and define:android
In lib/posix/posix.nim : line 283: when defined(macosx) or defined(android): and on line 1623: when not defined(macosx) and not defined(android) ( you just need to add the android part )
In lib/pure/osproc.nim "bin/sh" -> "sh" (line 744 )
Now ./bin/nim c koch should work, and we can build nim

./koch boot

Nimble may have some problems, I remember libman saying he had to patch getTempDir to use the termux specific folder or something.

I'll look into adding support in platform.nim so that we can have csources for android too.

@ghost
Copy link

ghost commented Apr 1, 2017

@stisa
it fails for me on ./bin/nim c koch:
"lib/posix/posix.nim(2430, 21) Error: type mismatch: got (clong) but expected 'cint = int32' "

My phone is 64bit, Termux reports aarch64

@stisa
Copy link
Contributor

stisa commented Apr 1, 2017

@TiberiumN I did write about it:

I get a type mismatch when doing koch boot:
Error: type mismatch: got (clong) but expected 'cint = int32'

Changing the return type of syscall from clong to cint seems to "fix" it, and I can do koch boot fine.

@ghost
Copy link

ghost commented Apr 1, 2017

@stisa ah, sorry :)

@ghost
Copy link

ghost commented Apr 1, 2017

@stisa so I can confirm:
Xiaomi Redmi 4 Pro, Android 6.0.1, MIUI 8.2 Global
Works with your steps

@ghost
Copy link

ghost commented Apr 1, 2017

@stisa
and same error for nimble- Read-only file system

@cheatfate
Copy link
Member Author

cheatfate commented Apr 1, 2017

@stisa i know that this fix will allow you to bootstrap Nim, but please test this function:

import osproc

proc waitForExit(p: Process, timeout: int = -1): int

with timeout > 0.

This function was affected by this fix, and i want to be sure it still working...

@stisa
Copy link
Contributor

stisa commented Apr 1, 2017

@cheatfate You are right, it doesn't work (btw the compiler still complains that you return a clong from syscall and it expects a cint as result in sigtimedwait ).
If I change the return type of syscall to cint, it compiles but sigtimedwait returns -1, which I guess is to be expected when randomly changing return types?

@cheatfate
Copy link
Member Author

@stisa please check this one.

@stisa
Copy link
Contributor

stisa commented Apr 1, 2017

Nope, still -1 and oserror. I tried printing NR_rt_sigtimedwait.cint , it outputs 137 , which looking here is correct. errno is 22, which should translate to EINVAL .

@cheatfate
Copy link
Member Author

EINVAL is a problem...

@cheatfate
Copy link
Member Author

cheatfate commented Apr 1, 2017

Ok i think final fix must work, i have tested it on Linux with -d:android and it works, so i think it will work on Android too.

@stisa
Copy link
Contributor

stisa commented Apr 1, 2017

This one works for me, great work @cheatfate! Thanks a lot.

code used as a test:

import osproc
var p = startProcess("/data/data/com.termux/files/usr/bin/ping", 
                      args=["-c","2","google.com"], options={poParentStreams})

echo p.waitForExit(3333)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants