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

🐛Fixes CLI hanging in commands calling 'make.exe' #180

Merged
merged 4 commits into from
Nov 19, 2021

Conversation

BennyBot
Copy link
Member

@BennyBot BennyBot commented Nov 2, 2021

Summary:

Previously: If 'make.exe' wasn't found, the 'EchoPipes' used to communicate the build status to the user would hang.
Changes: Used try/except on the 'pros make' command and 'pros build-compile-commands' commands. If an exception is thrown when running 'make.exe', the program will close the 'EchoPipe' class(es), print an error message to the screen, print a warning IF the pros toolchain is not found, and then exit.

Motivation:

The hanging terminal got on my nerves while trying to get the toolchain installed correctly on my windows pc.

References (optional):

This may be related to #83, as calling 'make.exe' through 'build-compile-commands' would hang the CLI if any exception was thrown during that subprocess.

Test Plan:

Ran both 'pros make' and 'pros build-compile-commands' with and without the toolchain environmental variable. Acts normally if 'make.exe' is there, and exits without hanging if an error is thrown during the calling of 'make.exe'
Screenshot of non-hanging terminal after both commands fail shown below.
cli hanging fix

pros/conductor/project/__init__.py Outdated Show resolved Hide resolved
pros/conductor/project/__init__.py Outdated Show resolved Hide resolved
pros/conductor/project/__init__.py Outdated Show resolved Hide resolved
pros/conductor/project/__init__.py Outdated Show resolved Hide resolved
@BennyBot BennyBot requested a review from WillXuCodes November 2, 2021 19:50
@BennyBot
Copy link
Member Author

BennyBot commented Nov 4, 2021

'pros make' and 'pros build-compile-commands' commands WITH 'PROS_TOOLCHAIN' in environmental variables.
image

'pros make' and 'pros build-compile-commands' commands WITHOUT 'PROS_TOOLCHAIN' in environmental variables.
image

@HotelCalifornia
Copy link
Contributor

@BennyBot one further test: try to build a project with no PROS_TOOLCHAIN variable but the toolchain folder is on PATH

@BennyBot
Copy link
Member Author

BennyBot commented Nov 4, 2021

'pros make' and 'pros build-compile-commands' commands without 'PROS_TOOLCHAIN' in environmental variables but the toolchain in PATH.
image
I made it only give the warning that pros toolchain was not detected if the subprocess threw an exception, because if someone went through the lengths to not have the PROS_TOOLCHAIN env. variable but still have it work then they probably wouldn't want a warning whenever they built. I could move this warning outside of the try/except block if you think it should warn every time.

@kunwarsahni01 kunwarsahni01 merged commit fcb3094 into develop Nov 19, 2021
@kunwarsahni01 kunwarsahni01 deleted the bugfix/cli-hanging branch November 19, 2021 17:00
@kunwarsahni01 kunwarsahni01 mentioned this pull request Nov 19, 2021
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.

4 participants