Skip to content

Commit

Permalink
Add a missing migration warning for android.arch -> android.archs
Browse files Browse the repository at this point in the history
  • Loading branch information
@misl6
misl6 committed Oct 8, 2021
1 parent 05ff81d commit d832733
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 0 deletions.
1 change: 1 addition & 0 deletions buildozer/default.spec
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ fullscreen = 0
#android.copy_libs = 1

# (list) The Android archs to build for, choices: armeabi-v7a, arm64-v8a, x86, x86_64
# In past, was `android.arch` as we weren't supporting builds for multiple archs at the same time.
android.archs = arm64-v8a, armeabi-v7a

# (int) overrides automatic versionCode computation (used in build.gradle)
Expand Down
8 changes: 8 additions & 0 deletions buildozer/targets/android.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ class TargetAndroid(Target):

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if self.buildozer.config.has_option(
"app", "android.arch"
) and not self.buildozer.config.has_option("app", "android.archs"):
raise BuildozerException(
"`android.archs` not detected, instead `android.arch` "
"is present. Are you using an old and not yet "
"migrated `buildozer.spec` file? "
"See `default.spec` template for reference.")
self._archs = self.buildozer.config.getlist(
'app', 'android.archs', DEFAULT_ARCHS)
self._build_dir = join(
Expand Down

5 comments on commit d832733

@RobertFlatt
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this solution is flawed as there is no demonstrated need to force users migrate.
It just creates problems, all downside and no upside.

I'm not the only one kivy/python-for-android#2467 (comment)

@misl6
Copy link
Member Author

@misl6 misl6 commented on d832733 Oct 9, 2021

Choose a reason for hiding this comment

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

What do you suggest?
Also, I don't get the link between a change made into a utility in p4a, and that one.
Can you explain?

@RobertFlatt
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest: make .arch a list, and remove .archs

Because

  • There is no reason to force users to change.
  • The .arch list is backwards compatible, this is why I care.
  • The .arch list is easy to implement, each list element is an instance of the existing p4a option
  • The .arch list is intuitive for users; need another architecture, just add it.

And the link because I had responded to this comment there: "Making the arch parameter no longer optional breaks existing code. Was this intentional?"

@misl6
Copy link
Member Author

@misl6 misl6 commented on d832733 Oct 9, 2021

Choose a reason for hiding this comment

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

That could work, but I feel is not fine for long term. So maybe we can support it as an alternative and add a deprecation warning? I will keep that in mind.
Consider that is not the only one migration error that is gonna be present, cause this version of buildozer is only compatible with an actually unreleased (develop) version of python-for-android and it will not be back-compatible (at least for the aab part), unless some changes are made.

@RobertFlatt
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for listening.

but I feel is not fine for long term

If you have some time could you help me understand what is on your mind?

I get the master/develop thing, I think it is OK, users are familiar with switching to 'develop' (and back) to try new features.

Please sign in to comment.