sdl2_image: fix downloading#3135
Merged
Merged
Conversation
Member
|
Tried a build (without this PR) a few hours ago and worked flawlessly, is this patch still required on your side? |
Member
Author
|
This happens on slow internet connections only. Also it prevents download of commit history for all submodules. |
|
Confirming that this bug is happening to me sometimes, when I use
buildozer/p4a in my home office which is using a slow
ethernet-via-powerline connection.
Am Mi., 9. Apr. 2025 um 06:00 Uhr schrieb Ansh Dadwal <
***@***.***>:
… This happens on slow internet connections only. Also it prevents download
of commit history for all submodules.
—
Reply to this email directly, view it on GitHub
<#3135 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4U63Q3MWCQ4TLO2UB4V2D2YSSP3AVCNFSM6AAAAABZZIVD4CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOOBYGI4DIMZVGY>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
*T-Dynamos* left a comment (kivy/python-for-android#3135)
<#3135 (comment)>
This happens on slow internet connections only. Also it prevents download
of commit history for all submodules.
—
Reply to this email directly, view it on GitHub
<#3135 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4U63Q3MWCQ4TLO2UB4V2D2YSSP3AVCNFSM6AAAAABZZIVD4CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOOBYGI4DIMZVGY>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
The end of a `with` block already does that.
Member
Author
|
@clayote Thanks for taking over this. Please do confirm if same fix is needed for |
Contributor
|
I don't know a lot about sdl3 or even 2, really. The PR looks good, and it only affects the downloading stage, so I figured I'd tidy it a bit. |
Contributor
|
@AndiEcker I don't have a slow connection to test on, could you confirm that this PR fixes your issue? |
|
@clayote: sorry I am not sure how to test this - I tried to put the
following lines into my buildozer.spec:
p4a.branch = T-Dynamos:sdl2_image_fix
or
p4a.branch = sdl2_image_fix
but in both cases I get only an branch-not-exists-error (because your pull
request branch is only available in your fork and not in the P4A repo)
Am Mo., 12. Mai 2025 um 18:04 Uhr schrieb clayote ***@***.***
…:
*clayote* left a comment (kivy/python-for-android#3135)
<#3135 (comment)>
@AndiEcker <https://github.com/AndiEcker> I don't have a slow connection
to test on, could you confirm that this PR fixes your issue?
—
Reply to this email directly, view it on GitHub
<#3135 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4U63QHVZV5FHT3MDL52VD26DIARAVCNFSM6AAAAABZZIVD4CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNZTGMZDIOJQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Member
Author
|
|
Thanks for your help!! silly me - totally forgot the p4a.fork option
I just started the first test build run (needs around 40 minutes).
I will have to do maybe 4 to 6 test build runs before I come back here,
because the fixed bug did not occur on each build run.
Am Di., 13. Mai 2025 um 11:53 Uhr schrieb Ansh Dadwal <
***@***.***>:
… *T-Dynamos* left a comment (kivy/python-for-android#3135)
<#3135 (comment)>
@AndiEcker <https://github.com/AndiEcker>
p4a.fork = T-Dynamos
—
Reply to this email directly, view it on GitHub
<#3135 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4U63RK6IG5OZI4JB5CD6326HFI5AVCNFSM6AAAAABZZIVD4CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNZWGAYDQMBVGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Yesterday and today I did 7 test build runs and tried hard (adding parallel
downloads to overcharge my slow power line ethernet connection, so that the
buildozer build time got doubled), but could not reproduce any (early EOF)
download errors!
Before your fix I got at least 1 build fail out of 3..4 runs.
So it looks like your PR fixed this bug.!!!
¡¡¡ Thank you very much for your great work !!!
Am Di., 13. Mai 2025 um 12:10 Uhr schrieb Andreas Ecker ***@***.***>:
… Thanks for your help!! silly me - totally forgot the p4a.fork option
I just started the first test build run (needs around 40 minutes).
I will have to do maybe 4 to 6 test build runs before I come back here,
because the fixed bug did not occur on each build run.
Am Di., 13. Mai 2025 um 11:53 Uhr schrieb Ansh Dadwal <
***@***.***>:
> *T-Dynamos* left a comment (kivy/python-for-android#3135)
> <#3135 (comment)>
>
> @AndiEcker <https://github.com/AndiEcker>
>
> p4a.fork = T-Dynamos
>
> —
> Reply to this email directly, view it on GitHub
> <#3135 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AA4U63RK6IG5OZI4JB5CD6326HFI5AVCNFSM6AAAAABZZIVD4CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNZWGAYDQMBVGE>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
clayote
approved these changes
May 14, 2025
Contributor
|
@T-Dynamos I think same should be done for |
AndreMiras
reviewed
May 15, 2025
Comment on lines
+23
to
+40
| build_dir = self.get_build_dir(arch.arch) | ||
|
|
||
| with open(os.path.join(build_dir, ".gitmodules"), "r") as file: | ||
| for section in file.read().split('[submodule "')[1:]: | ||
| line_split = section.split(" = ") | ||
| # Parse .gitmodule section | ||
| clone_path, url, branch = ( | ||
| os.path.join(build_dir, line_split[1].split("\n")[0].strip()), | ||
| line_split[2].split("\n")[0].strip(), | ||
| line_split[-1].strip() | ||
| ) | ||
| # Clone if needed | ||
| if not os.path.exists(clone_path) or not os.listdir(clone_path): | ||
| shprint( | ||
| sh.git, "clone", url, | ||
| "--depth", "1", "-b", | ||
| branch, clone_path, "--recursive" | ||
| ) |
Member
There was a problem hiding this comment.
I should have commented that before, but when it's not that obvious what the code is doing, a comment is always helpful.
Maybe we could have added a block comment or method docstring saying:
# Prevents downloading commit history of all submodules, to improve cloning reliability on slow internet connection
Member
|
Thanks @AndiEcker for testing this too and thank you @T-Dynamos for making the fix 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ref: #3115 (comment)