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

Fix/clean-up LDSHARED #1360

Closed
AndreMiras opened this issue Sep 6, 2018 · 3 comments
Closed

Fix/clean-up LDSHARED #1360

AndreMiras opened this issue Sep 6, 2018 · 3 comments

Comments

@AndreMiras
Copy link
Member

LDSHARED doesn't seem to be set in PythonRecipe leading to failing compilation in some scenarios.
The compilation may be failing because p4a would then pick up the system linker rather than the cross-compiler one.
For this reasons some recipes added that change in their get_recipe_env() method, like below:

env['LDSHARED'] = env['CC'] + ' -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions'

As in last master commit 0dc7664 we have at least 17 of them:

$ grep -r "env\['LDSHARED'\] = env\['CC'\]" pythonforandroid/recipes/ | wc -l
17

In his PR JonasT tries to address it via pythonforandroid/archs.py, see https://github.com/kivy/python-for-android/pull/1357/files#diff-569e13021e33ced8b54385f55b49cbe6R115
This probably requires investigation to know where is the proper place to put it and then the pull request fixing it should also clean recipes overriding it in their get_recipe_env() method.

@ghost
Copy link

ghost commented Sep 6, 2018

By the way, similar issue: env['CFLAGS'] += ' -I{}/sources/python/{}/include/python/'.format(self.ctx.ndk_dir, self.ctx.python_recipe.version[0:3])

Your greenlet recipe pull request does that, mine does, I've seen yet another do it - maybe this should be handled centrally as well? I am pretty sure it could go in the same place in archs.py.

@ghost
Copy link

ghost commented Sep 6, 2018

I added a pull request here: #1361 (still WIP, I'm building my app with it right now to see if things blow up)

@AndreMiras
Copy link
Member Author

Even though #1361 is now merged, we still need to clean existing recipes to remove redundant env['LDSHARED'] override before closing the issue.
I think it would be great to also verify everything is going OK at unit test level e.g. checking all recipes get_recipe_env() behaviour.

AndreMiras added a commit to AndreMiras/python-for-android that referenced this issue Jan 29, 2019
Since kivy#1361 was merged, we can drop LDSHARED override.
inclement added a commit that referenced this issue Jan 30, 2019
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

No branches or pull requests

1 participant