Fix: various errors during installation#28
Open
digglife wants to merge 1 commit intostorj-thirdparty:masterfrom
Open
Fix: various errors during installation#28digglife wants to merge 1 commit intostorj-thirdparty:masterfrom
digglife wants to merge 1 commit intostorj-thirdparty:masterfrom
Conversation
Author
|
Hi @kmozurkewich @ayush-srivastava-03 Could you please review this fix? If you are no longer maintaining this project, do you know who should we contact for issue report or PR? Thanks! |
Member
|
Hello! Thank you for the PR. Will get it in the queue for the end of next week. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add Missing space before the git url. This problem causes git clone failure, so the whole share object building process doesn't work.
Remove
sudostatements. If the package path is not writable to current user, then it should not be. We should let user to decide whether to usesudoinstead of prompt password in the middle of installing (Actually install to system site-packages with admin privilege is a bad practice). Also, if user is using virtualenv or install with--user, it's perfectly fine withoutsudo.Avoid echoing the output because system output is unpredictable. It could be multiple lines, so
os.system("echo ", output.decode("utf-8"))could end up executing the following shell commandecho the first line the second line the third linethe second lineandthe third linewill be executed as normal shell command.We don't need to use echo to print out the result.
os.exit(1)should besys.exit(1)