Delete the pygame bootstrap #1670

inclement · 2019-02-03T12:23:56Z

The time has come to get rid of the pygame bootstrap. We sometimes find weird things in the code aimed at working with it, but I'm not sure if it even works any more and we definitely don't support it.

I'll leave this issue here for a little while in case anyone wants to raise anything, but in the absence of any complaints about it we'll merge this quite soon.

WIP because I didn't yet check throroughly if I caught everything.

opacam · 2019-02-03T15:50:43Z

¡¡¡Good work @inclement!!! 😄 so far looks good to me, still there will be some pygame entries, as you suspect, in the following recipes:

android
generickndkbuild
sdl (can we remove this recipe? I think that we only use that for pygame...)
sdl2

Also we have a pygame's testapp that we should remove

And we have pygame entries in:

testapps/testlauncher_setup/pygame.py
tests/test_graph.py
ci/constants.py

That is what I found...hope that this helps you with this pr ;)

Update: if we remove sdl...we also should take care to remove sdl references in recipes:

audiostream
pyjnius

inclement · 2019-02-03T17:52:55Z

Thanks @opacam, I hadn't seen all of those references yet.

This seems fairly complete now, but let me know if there's anything else to do.

opacam · 2019-02-03T18:02:12Z

pythonforandroid/recipes/genericndkbuild/__init__.py

@@ -8,7 +8,7 @@ class GenericNDKBuildRecipe(BootstrapNDKRecipe):
    url = None

    depends = [('python2', 'python3', 'python3crystax')]
-    conflicts = ['sdl2', 'pygame', 'sdl']
+    conflicts = ['sdl2', 'sdl']


I think that we can remove sdl of here because you removed sdl recipe 😉

AndreMiras · 2019-02-03T19:48:03Z

pythonforandroid/recipes/android/__init__.py

@@ -88,8 +82,5 @@ def prebuild_arch(self, arch):
                fh.write(
                    '#define SDL_ANDROID_GetJNIEnv SDL_AndroidGetJNIEnv\n'
                )
-            elif is_pygame:
-                fh.write('JNIEnv *SDL_ANDROID_GetJNIEnv(void);\n')
-


ghost · 2019-02-09T07:13:56Z

@inclement I introduced some conflicts with this with my graph stuff, sorry 😬 I propose we get this in before we bother with #1625 because I don't mind rebasing, and we should really get the pygame bootstrap ripped out before we get more and more conflicts, and everyone continues to carry along old & new special case code to deal with it...

AndreMiras · 2019-02-09T10:55:27Z

No worries @Jonast this is a WIP/reminder PR anyway 👍

ghost · 2019-02-09T13:53:20Z

@AndreMiras let's make it not-so-WIP/reminder then!

AndreMiras · 2019-03-06T23:48:41Z

Should we continue with this @inclement ? I can take over if you don't have time but give me write access to your branch, I like deleting code 😈

ghost · 2019-04-01T21:48:32Z

I was told here to write that I want this and I really do

AndreMiras · 2019-04-01T22:08:20Z

I would definitely go for the delete. I'm not entirely sure, but I think I recall during the FOSDEM @tito had some objection here. Or did you @tito ? I think you were saying some recent upstream changes in pygame made it interesting again.
Few of my arguments for killing it are:

it's legacy
sdl2 is awesome
nobody in p4a team is currently {using, test, maintaining} it
it slows us up when bug fixing and feature pushing
less is more

If we're worrying about breaking things for the people who are using it still, we could tag a p4a 0.8.0 release before deleting it. And people who can't move forward with sdl2 can stay on 0.8.0.

ghost · 2019-04-01T22:11:10Z

I agree with every single one of @AndreMiras ' points

inclement · 2019-05-26T13:43:05Z

Rebased and squashed.

@AndreMiras @Jonast Please go ahead and merge if the tests pass and the changes look good to you.

inclement · 2019-05-26T16:27:50Z

Looks like tests are failing only because the recipe update test is confused.

opacam · 2019-05-26T18:02:45Z

¡¡¡Great job!!!

Can you remove pygame entry from this line?

https://github.com/kivy/python-for-android/blob/master/tests/test_graph.py#L107

I think that without that entry we will have all the code free of pygame 😄

Removed pygame references from doc Removed some more pygame references Removed some more pygame references Removed more references to pygame Fixed unicode strings in android module recipe, broken in rebase Deleted pygame recipe Removed references to sdl from audiostream and genericndkbuild Deleted argparse file in pygame bootstrap Removed sdl reference from pyjnius recipe Added blank line to make pep8 happy I think this is an irrelevant aspect of style Removed pygame mention from test

inclement · 2019-05-26T18:04:30Z

Nice catch @opacam, done.

opacam

👍

Looks good to me, and I'm guessing that @AndreMiras and @Jonast has no problem to merge this soon, so I aproved the changes, but I cannot merge it with the master branch so you must do that job yourself...but before do that, consider the point mentioned by @AndreMiras:

If we're worrying about breaking things for the people who are using it still, we could tag a p4a 0.8.0 release before deleting it. And people who can't move forward with sdl2 can stay on 0.8.0.

inclement · 2019-05-26T18:57:39Z

I'm going to go ahead and merge because it'll only break again, and because we're currently directing all buildozer users at p4a master so making a new release is less important now. I'm not clear if the pygame bootstrap was working properly any more anyway.

ghost · 2019-05-26T19:07:25Z

🎉 nice! code cleanup, yaas ❤️

inclement mentioned this pull request Feb 3, 2019

Remove pygame bootstrap #1668

Closed

inclement force-pushed the delete_pygame_bootstrap branch from 0648d5d to 2304f20 Compare February 3, 2019 17:35

opacam reviewed Feb 3, 2019

View reviewed changes

AndreMiras reviewed Feb 3, 2019

View reviewed changes

opacam added the pygame backend label Feb 3, 2019

AndreMiras mentioned this pull request Feb 4, 2019

tox update and linter fixes #1674

Merged

opacam mentioned this pull request Feb 20, 2019

Initial migration to NDK r19 (Part I - The core) #1722

Merged

7 tasks

inclement force-pushed the delete_pygame_bootstrap branch 2 times, most recently from 4f3fcb8 to 8540afb Compare May 26, 2019 13:40

inclement force-pushed the delete_pygame_bootstrap branch 3 times, most recently from 47a9159 to 0691176 Compare May 26, 2019 14:28

inclement force-pushed the delete_pygame_bootstrap branch from 0691176 to 79b2be1 Compare May 26, 2019 18:04

opacam approved these changes May 26, 2019

View reviewed changes

inclement merged commit 00bd9d5 into kivy:master May 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete the pygame bootstrap #1670

Delete the pygame bootstrap #1670

inclement commented Feb 3, 2019

opacam commented Feb 3, 2019 •

edited

Loading

inclement commented Feb 3, 2019

opacam Feb 3, 2019

AndreMiras Feb 3, 2019

ghost commented Feb 9, 2019

AndreMiras commented Feb 9, 2019

ghost commented Feb 9, 2019

AndreMiras commented Mar 6, 2019

ghost commented Apr 1, 2019

AndreMiras commented Apr 1, 2019

ghost commented Apr 1, 2019

inclement commented May 26, 2019

inclement commented May 26, 2019

opacam commented May 26, 2019

inclement commented May 26, 2019

opacam left a comment

inclement commented May 26, 2019

ghost commented May 26, 2019

Delete the pygame bootstrap #1670

Delete the pygame bootstrap #1670

Conversation

inclement commented Feb 3, 2019

opacam commented Feb 3, 2019 • edited Loading

inclement commented Feb 3, 2019

opacam Feb 3, 2019

Choose a reason for hiding this comment

AndreMiras Feb 3, 2019

Choose a reason for hiding this comment

ghost commented Feb 9, 2019

AndreMiras commented Feb 9, 2019

ghost commented Feb 9, 2019

AndreMiras commented Mar 6, 2019

ghost commented Apr 1, 2019

AndreMiras commented Apr 1, 2019

ghost commented Apr 1, 2019

inclement commented May 26, 2019

inclement commented May 26, 2019

opacam commented May 26, 2019

inclement commented May 26, 2019

opacam left a comment

Choose a reason for hiding this comment

inclement commented May 26, 2019

ghost commented May 26, 2019

opacam commented Feb 3, 2019 •

edited

Loading