Skip to content

Fix: various errors during installation#28

Open
digglife wants to merge 1 commit intostorj-thirdparty:masterfrom
digglife:setup
Open

Fix: various errors during installation#28
digglife wants to merge 1 commit intostorj-thirdparty:masterfrom
digglife:setup

Conversation

@digglife
Copy link

@digglife digglife commented May 13, 2023

  1. Add Missing space before the git url. This problem causes git clone failure, so the whole share object building process doesn't work.

  2. Remove sudo statements. If the package path is not writable to current user, then it should not be. We should let user to decide whether to use sudo instead 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 without sudo.

  3. 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 command

    echo the first line
    the second line
    the third line

    the second line and the third line will be executed as normal shell command.

    We don't need to use echo to print out the result.

  4. os.exit(1) should be sys.exit(1)

@digglife
Copy link
Author

digglife commented Jun 4, 2023

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!

@kmozurkewich
Copy link
Member

Hello!

Thank you for the PR. Will get it in the queue for the end of next week.

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.

2 participants