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

Add a PyInstaller build for unsupported distros #27

Merged
merged 48 commits into from
Aug 3, 2021

Conversation

velovix
Copy link
Member

@velovix velovix commented May 22, 2021

This adds configuration to build the CLI as a PyInstaller executable along with the necessary supporting code. The PyInstaller executable would be an option for users of distributions whose package manager we don't have support for.

Since there's no automatic update mechanism available, this also implements a new self-update command that pulls the latest version of the CLI and replaces the current version with that one.

Unfortunately, this version of the CLI does not ship with its own version of Docker Compose. Packaging Compose into PyInstaller isn't straightforward, and it can't be run as a module because the Python interpreter is embedded in the binary. Instead, the expectation is that the user installs Compose with whatever mechanism they prefer.

See #13, which this PR does not fix on its own.

To-do Before Merge

  • Figure out where the executables should be uploaded to
  • Update the self-update command to pull from a real URL
  • Manual testing
    • Ubuntu 18.04
    • Ubuntu 20.04
    • CentOS 7

@velovix velovix mentioned this pull request May 22, 2021
3 tasks
@velovix velovix marked this pull request as ready for review May 28, 2021 18:08
.circleci/config.yml Show resolved Hide resolved
brainframe/cli/commands/update.py Outdated Show resolved Hide resolved
brainframe/cli/frozen_utils.py Outdated Show resolved Hide resolved
brainframe/cli/frozen_utils.py Outdated Show resolved Hide resolved
brainframe/cli/main.py Outdated Show resolved Hide resolved
brainframe/cli/translations/install.en.yml Outdated Show resolved Hide resolved
brainframe/cli/translations/update.en.yml Outdated Show resolved Hide resolved
brainframe/cli/translations/update.en.yml Outdated Show resolved Hide resolved
package/upload_binary.py Outdated Show resolved Hide resolved
package/upload_binary.py Outdated Show resolved Hide resolved
@velovix velovix requested a review from BryceBeagle May 29, 2021 04:40
@velovix velovix requested a review from apockill June 4, 2021 19:55
brainframe/cli/config.py Outdated Show resolved Hide resolved
brainframe/cli/docker_compose.py Show resolved Hide resolved
brainframe/cli/docker_compose.py Show resolved Hide resolved
brainframe/cli/frozen_utils.py Outdated Show resolved Hide resolved
brainframe/cli/frozen_utils.py Outdated Show resolved Hide resolved
if is_frozen():
path = _pyinstaller_tmp_path() / target
if not path.is_file():
raise FileNotFoundError()
Copy link
Member

Choose a reason for hiding this comment

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

You should give this exceptions more info:

import errno
import os

raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), filename)

Copy link
Member Author

Choose a reason for hiding this comment

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

Usually I'm down to provide a lot of information in exceptions, but for this use-case it would all go unused always. The caller always knows what file it's asking for and would never provide the message to the user directly because we want a translated, user-readable error. If you gotta use errno to use FileNotFoundError properly, maybe I should go back to a custom exception.

brainframe/cli/commands/self_update.py Outdated Show resolved Hide resolved
@velovix velovix requested a review from BryceBeagle July 27, 2021 03:02
Copy link
Member

@BryceBeagle BryceBeagle left a comment

Choose a reason for hiding this comment

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

My vote is to add the errno information to the FileNotFoundError, but it's not a deal-breaker if it doesn't happen

@velovix velovix requested a review from apockill July 28, 2021 01:34
Copy link
Member

@apockill apockill left a comment

Choose a reason for hiding this comment

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

Just make sure to check those boxes before merging!

@velovix
Copy link
Member Author

velovix commented Aug 3, 2021

Manual tests looking good!

@velovix velovix merged commit d7cb483 into master Aug 3, 2021
@velovix velovix deleted the feature/13-pyinstaller branch August 3, 2021 00:11
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