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

Create a dumb librt recipe and refactor the affected recipes to make use of this #1618

Merged
merged 1 commit into from
Jan 28, 2019

Conversation

opacam
Copy link
Member

@opacam opacam commented Jan 28, 2019

Because some recipes inserts flags without our control (-lrt) and this is a problem because librt does not exist in android, but has been integrated into libc, so, this recipe create a temporary symbolic link from libc to librt so the compilation can find the necessary symbols/functions to finish the compilation

Note: Once we end the build we will remove the created symbolic link

¡¡¡Thanks @AndreMiras, for this idea of "dumb" librt recipe!!!

…e use of this

Because some recipes inserts flags without our control (`-lrt`) and this is a problem because librt does not exist in android, but has been integrated into libc, so, this recipe create a temporary symbolic link from libc to librt so the compilation can find the necessary `symbols/functions` to finish the compilation

Note: Once we end the build we will remove the created symbolic link
@opacam
Copy link
Member Author

opacam commented Jan 28, 2019

I see that fails on create the link (permission error)...

This was not happening with non docker tests...

@opacam
Copy link
Member Author

opacam commented Jan 28, 2019

Aaaa...I see:

#1600 (comment)

It's the same error:

The problem in here is that if a user has installed the ndk in some directory as a root user, he will get this error if he runs the p4a command as a regular user...
I think that should not be a problem (we usually will manually download the ndk or we will do that via buildozer as a regular users)...a shame that we cannot test it properly via CI :<

Maybe we should reproduce in the docker what a user would do?
download all sdk/ndk files as a regular user and install it into a writable user location...that way the errors in CI should be gone...I think...

Copy link
Member

@AndreMiras AndreMiras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome ❤️
As for the Docker environment, I agree, we should discuss/tackle in a dedicated task

@ghost
Copy link

ghost commented Jan 28, 2019

@opacam @AndreMiras this appears to break build for me: #1621

@ghost
Copy link

ghost commented Jan 28, 2019

Oops, seems like this was broken before. I just never tried lxml in that environment since it was broken for other reasons for a while and previously I used another way to build things 🤷‍♀️ but still, something that probably should be fixed 😬

@opacam
Copy link
Member Author

opacam commented Jan 28, 2019

@Jonast, I have a solution for docker, still not created a pr, but I will, don't worry ;)

@opacam
Copy link
Member Author

opacam commented Jan 28, 2019

@Jonast, If you are in hurry with this I will do it now...do you need that?

@ghost
Copy link

ghost commented Jan 28, 2019

@opacam take your time it's not urgent. lxml is an optional component for my application so I cna just disable it for now. but yeah, would be pretty nice to see a solution - I was already pondering some LD_LIBRARY_PATH hacks to get this to work just out of curiosity, but I don't mind if you are going to care of it instead 😄 👍

@opacam
Copy link
Member Author

opacam commented Jan 28, 2019

Here it is the commit that will solve the docker issue with lxml and greenlet gevent:

opacam@ad9c35a

I will create the pr as soon as I have the explanations (I only move code in there, from one place to another, except for the variable ANDROID_HOME)

@ghost
Copy link

ghost commented Jan 28, 2019

@opacam did you just make the NDK folder writable? Because I would not consider that a fix to this, I think the problem is that it is assumed to be writable in the first place. The fault is IMHO definitely with the librt recipe and not the environment

@opacam
Copy link
Member Author

opacam commented Jan 28, 2019

No, I change the download directory into a user directory

@ghost
Copy link

ghost commented Jan 28, 2019

Which has the same outcome really. You are changing it from a system-wide to a user install to make it writable - this shouldn't be required, and isn't required for any other recipe. In my eyes that's a workaround hack, not a fix 🤷‍♂️ not that I don't think that's not possibly useful for other reasons, but I don't think this should be required to be able to build librt

@ghost
Copy link

ghost commented Jan 28, 2019

(as a side note I don't use that docker container anyway, so this wouldn't fix anything for me)

@AndreMiras
Copy link
Member

yes I do would prefer that we make it work just like other recipes. So adding build artifact to a dedicated recipe build folder and then link it via -Ldir, see #1621 (comment)

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.

2 participants