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

Allow setting custom p4a URL instead of fork #1305

Merged
merged 3 commits into from
Mar 14, 2021
Merged

Allow setting custom p4a URL instead of fork #1305

merged 3 commits into from
Mar 14, 2021

Conversation

syrykh
Copy link
Contributor

@syrykh syrykh commented Mar 12, 2021

Currently it's not possible to set own p4a mirror (not a fork on GitHub) in buildozer.spec.
This possibility already exists for kivy_ios, but not for p4a.

This PR allows to setting custom URL for p4a in the spec, instead of simply specifying fork.

The URL is used at first place. If no URL is specified, then fallback to github fork happens:

Backward compatibility with old specs is preserved with no behavioral changes.

@akshayaurora akshayaurora requested review from AndreMiras and opacam and removed request for AndreMiras March 12, 2021 12:39
opacam
opacam previously approved these changes Mar 13, 2021
Copy link
Member

@opacam opacam left a comment

Choose a reason for hiding this comment

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

LGTM, nice work and thanks!! 😄

Note: I leave a couple of suggestions, in case that you want to apply them, but is already fine if we merge as it is, since we almost didn't use Python3's f-strings in buildozer 😉

buildozer/targets/android.py Outdated Show resolved Hide resolved
buildozer/targets/android.py Outdated Show resolved Hide resolved
AndreMiras
AndreMiras previously approved these changes Mar 13, 2021
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.

Well done! I wish you wrote some tests.
Dare to give it a try?

@syrykh
Copy link
Contributor Author

syrykh commented Mar 13, 2021

Well done! I wish you wrote some tests.
Dare to give it a try?

Thank you! I'll give it a try.

@AndreMiras
Copy link
Member

Great to hear, you could look into tests/targets/test_android.py then.
You could use the init_target() helper method to create a setting with p4a.url and mock the cmd() method using patch_buildozer_cmd() to show it got called with the custom git clone url.

syrykh and others added 2 commits March 14, 2021 09:02
Co-authored-by: Pol Canelles <canellestudi@gmail.com>
@@ -53,7 +53,7 @@ def init_buildozer(temp_dir, target, options=None):
spec = []
for line in default_spec:
if line.strip():
match = re.search(r"[#\s]?([a-z_\.]+)", line)
match = re.search(r"[#\s]?([0-9a-z_.]+)", line)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is needed to match "p4a", as it contains digit.

@syrykh
Copy link
Contributor Author

syrykh commented Mar 14, 2021

@AndreMiras, tests were added. Could you please check them?

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.

Excellent work with the tests, that's exactly what I was expecting 👏
Thank you for taking time on this as well 🙏

@AndreMiras AndreMiras merged commit 1a3fb61 into kivy:master Mar 14, 2021
@syrykh
Copy link
Contributor Author

syrykh commented Mar 14, 2021

@AndreMiras , @opacam , thank you very much!

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.

3 participants