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

Buildozer class too complex; should be split. #1629

Open
Julian-O opened this issue Jul 7, 2023 · 2 comments
Open

Buildozer class too complex; should be split. #1629

Julian-O opened this issue Jul 7, 2023 · 2 comments
Labels

Comments

@Julian-O
Copy link
Contributor

Julian-O commented Jul 7, 2023

Versions

  • Buildozer: 2.2.1

Description

This is a proposal to give the developers a chance to redirect my efforts if they are wrong-headed.

The file tools/__init__.py defines (amongst other thngs) the main Buildozer class.

Unfortunately, it weighs in at over 1,200 lines, and represents a God Object. It has too many duties and should be broken down in a number of separate files with better separation of concern.

Below I have attempted to survey its responsibilities, and divide them into separate areas. This is still an early proposal stage, and I am sure this will will evolve as we dig down. At this stage, I am just trying to show that there is an opportunity to break it down into parts. I hope the result will make it easier to add unit-tests and enable future refactoring of the pieces.

The responsibilities include:

  • The core Buildozer functionality

    • Declare API/Exceptions
    • Find target plugins
    • Know what the core steps are of how to do an install.
      • Including init, clean, prep for build, requirements.
      • Delegate the actual work to targets and other libraries.
  • Buildozer class currently supports many low-level operations that are the very basic building blocks of a build/install:

    • download content from urls
    • pip/virtualenv installations and validations.
    • find binaries in a path.
    • run command-line commands (while tracking output)
    • run command-line commands with expect
    • mkdir, rmdir, file_matches, file_exists, rename, file and tree copies, extract compressed files,
    • run binaries (!)
    • clean a name for use as a package.
    • check if user is an admin
    • track and increment build numbers

    These should initially be moved into a separate support library. (e.g. installops).

    That way, the code that knows the high-level logic of "We need to install the Platform SDK next" doesn't need to know the details of how to unzip a file, and the unit-tests can confirm that files are being unzipped without having to be concerned with targets.

    Perhaps the webserver should be included here, although it might better belong in its own file.

  • The Buildozer class contains a miniature reimplementation of the logging module, complete with platform-dependent colour formatting.

    This should initially be moved into a separate library (and eventually be migrated to use the standard library, so we benefit from the extra functionality and reduce the maintenance obligations.)

  • It supports versioning.

    This can be delegated to an separate class.

  • It supports command-line parsing and displaying usage information.

    This can be be separated from the class that does the actual work.

  • There is some complex code related to getting configuration information from:

    • spec files (including validating)
    • command line options
    • config files
    • environment variables.
    • legacy sources (following migration rules)
    • default commands

    These should be separated out from the code that does the work. The result should be a configuration object that can be interrogated by the lower-level code.

  • Tracking the key locations:

    • user bin dir
    • root_dir
    • user_build_dir
    • bulldozer_dir
    • platform_dir
    • app_dir
    • applibs_dir
    • global_buildozer_dir
    • global_platform_dir
    • global_packages_dir

    This should be handled by a single class, informed by the configuration object. If it were to migrate to use Python 3's pathlib, that would be an improvement too.

@Julian-O
Copy link
Contributor Author

Just a note to celebrate progress (with more coming down the pipeline):

buildozer/__init__.py's line length, by date

2023-01-29: 1243
2023-07-08: 1240
2023-07-21: 1172
2023-07-29: 1037

@Julian-O
Copy link
Contributor Author

More progress has been happening.

buildozer/__init__.py's line length, by date
2023-01-29: 1243
2023-07-08: 1240
2023-07-21: 1172
2023-07-29: 1037
2023-08-23: 1023
2023-08-25: 1011
2023-08-26: 977
2023-08-26: 967
2023-08-30: 921
2023-09-10: 761

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