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

Check dependencies to avoid a useless sudo on Archlinux #17158

Merged
merged 1 commit into from
Jan 12, 2024
Merged

Check dependencies to avoid a useless sudo on Archlinux #17158

merged 1 commit into from
Jan 12, 2024

Conversation

Ealrann
Copy link
Contributor

@Ealrann Ealrann commented Dec 23, 2023

Purpose:

Prevent a useless call to sudo during this installation of Chia on Archlinux, by checking if the dependencies are not already installed.
The point is to make the installation easier:

  • sudo forces the user to type its password (event if there is nothing to do).
  • Some people prefer to use su and don't setup sudo. In this case, the installation of chia is impossible without modifying the instal.sh. (it's what I do for years)

Old Behavior:

Calling sudo every time to install the dependencies.

New Behavior:

Verify if git and openssl are installed, then call sudo pacman -S git openssl only if needed.

Testing Notes:

I wanted to keep the existing code, so I simply added a if statement. But in fact, we can simplify the code for Archlinux:
no dependencies needs to be installed (at least line 213), because:

  • openssl is a base package of archlinux (base => coreutils => openssl)
  • git is already called line 71 (so no point to install it line 213, it's too late).

@Ealrann Ealrann requested a review from a team as a code owner December 23, 2023 00:07
@hoffmang9
Copy link
Member

Thanks for this. Can you sign your commit or open a new PR with a signed commit to start it?

@Ealrann
Copy link
Contributor Author

Ealrann commented Dec 23, 2023

Oh sorry, I've just corrected it

Copy link

coveralls-official bot commented Dec 23, 2023

Pull Request Test Coverage Report for Build 7305042311

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 35 unchanged lines in 9 files lost coverage.
  • Overall coverage decreased (-0.03%) to 90.55%

Files with Coverage Reduction New Missed Lines %
chia/rpc/rpc_server.py 1 88.4%
chia/rpc/wallet_rpc_api.py 1 88.09%
tests/core/util/test_lockfile.py 1 91.09%
chia/timelord/timelord_launcher.py 2 69.77%
chia/wallet/wallet_node.py 2 88.31%
chia/server/node_discovery.py 3 78.35%
chia/full_node/full_node_api.py 4 77.93%
chia/full_node/full_node.py 8 84.38%
chia/timelord/timelord.py 13 79.0%
Totals Coverage Status
Change from base Build 7304634562: -0.03%
Covered Lines: 94019
Relevant Lines: 103797

💛 - Coveralls

@hoffmang9 hoffmang9 added the Cleanup Code cleanup label Dec 23, 2023
Copy link
Member

@hoffmang9 hoffmang9 left a comment

Choose a reason for hiding this comment

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

lgtm with thannks!

@emlowe emlowe added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Jan 12, 2024
@cmmarslender cmmarslender merged commit c05fd88 into Chia-Network:main Jan 12, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog Cleanup Code cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants