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

Simplify _build_prepared #1630

Open
Julian-O opened this issue Jul 8, 2023 · 0 comments
Open

Simplify _build_prepared #1630

Julian-O opened this issue Jul 8, 2023 · 0 comments
Labels

Comments

@Julian-O
Copy link
Contributor

Julian-O commented Jul 8, 2023

Versions

  • Buildozer: 2.2.1

Description

The _build_prepared code is overly complicated, and should be refactored.

The Buildozer instance wants to make sure if doesn't prepare a build twice for the same target.

In the current implementation, it attaches its own attribute, _build_prepared to the Target instance. It sets it to True when it has finished prepare_for_build, but the value is ignored. It checks for its existence at the beginning of prepare_for_build and build.

[Note: This triggers style-checkers that don't like attributes with leading underscores being accessed from outside the class.]

Meanwhile, the osx target has a check_build_prepared(), which has four weird behaviours:

  • It is only called after prepare_for_build(), so we know the build has been prepared. It appears to be a pointless check.
  • It is only called in the case of a debug build, not a release build, which is an unexplained inconsistency.
  • Despite its name, it doesn't check the value of _build_prepared. Instead, it sets the value, thus (re)declaring it built.
  • It sets the value to False. The value is never used, so this is functionally equivalent to setting it to True, but it isn't clear if some difference was intended.

[Note: This triggers style-checkers that don't like attributes being set in methods that aren't declared in the __init__ methods]

My recommendation:

  • In the base class, Target.__init__(), add an instance attribute called is_build_prepared` and set it to False. (No leading underscore - it is a public attribute,)
  • Modify the Buildozer class to assume its existence and use it as a Boolean.
  • Remove check_build_prepare() from osx.

[I am raising this as a bug report rather than a simple PR, because I am aware I haven't figured out why the osx target is behaving this way, and that can make refactoring more dangerous.]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants