Installation of Node and Testnet #2
Merged
Conversation
This file contains 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
A new mock response file is provided that represents the new release format, which renames the assets and also provides the additional testnet assets. The tests were updated for new asset names and version numbers.
If not running in elevated mode, the shell profile will be modified to add an extra statement to source an `env` file, in the same manner as `rustup` operates. Doing it this way makes it easier to modify the profile only once. Right now this only works for a Bash shell. Cross platform considerations will follow shortly.
In the case where the user supplies a version, we don't need to use the Github API and instead just go straight to S3 to download the archive.
The only real modification required for this was to the `env` file to also put `~/.safe/node` on the PATH. Otherwise it goes through identical code paths to the client installation, which has already been tested quite thoroughly. Strictly speaking, the `install_bin` function should have test coverage for the node, but as I already explained previously, the setup code for these tests is quite involved, so there would be a lot of duplication with minimal benefit.
jacderida
force-pushed
the
install-node
branch
16 times, most recently
from
c77dbdb
to
5f3c384
Compare
A few integration tests were defined in CI. They were done here because it's just a bit easier to use new shell sessions than if they were defined as Rust tests. The tests install a specific version of the binaries, which will avoid rate limiting because it doesn't use the Github API. One of the tests runs non-elevated, which then uses a new shell session to make sure safe is available, i.e., that the change to the environment variable has been applied. The other test runs in non-elevated mode and checks that the binaries are immediately available. On Github Actions, in macOS you can only run elevated, so there is no non-elevated test. The usage of `reqwest` was changed to switch off default features, which prevents the `openssl` crate being referenced in the dependency graph. We have this crate denied in our shared `cargo deny` configuration, although I don't know why. The `rustls-tls` feature then needs to be explicitly enabled for using HTTPS. Since this was the first time Clippy ran in CI, some flagged issues were fixed.
This will be used in the `update` command to apply the updates to the locations the binaries were installed. We keep track because the user can provide custom locations.
For Linux, we are using ~/.bashrc as the default, whereas for macOS we will use ~/.zshrc, since ZSH is the default shell.
jacderida
force-pushed
the
install-node
branch
7 times, most recently
from
e3e4552
to
e8a69ba
Compare
Since this goes through all the same code paths, not much was required other than adding a new entry to the `AssetType` enum.
jacderida
changed the title
| version, | ||
| no_modify_shell_profile, | ||
| ) | ||
| .await | ||
| } | ||
| None => { | ||
| println!("interactive gui"); | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Just a small suggestion, but can we maybe print the help screen if no args were passed? https://docs.rs/clap/latest/clap/struct.Command.html#method.print_help
There was a problem hiding this comment.
Hey Roland, thanks for the feedback. My plan was that if no arguments were passed, we would run an interactive GUI, which is what rustup does too. Well, sorry, just to clarify, it's not a GUI, I guess, but a TUI.
Do you think that's not worth doing?
There was a problem hiding this comment.
Ah I see! That would be pretty useful to the non-tech people I assume. But I'm not so sure, they will again be exposed to the techy CLIs of safenode/safe/testnet after using this tool.
There was a problem hiding this comment.
Yeah, to be honest, the value of it may be quite low. I might ask this question on the forum.
RolandSherwin
approved these changes
Mar 28, 2023
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.
5ea39e2 chore: updates for dealing with new release format
A new mock response file is provided that represents the new release format, which renames the
assets and also provides the additional testnet assets.
The tests were updated for new asset names and version numbers.
361a490 feat: modify shell profile for safe on path
If not running in elevated mode, the shell profile will be modified to add an extra statement to
source an
envfile, in the same manner asrustupoperates. Doing it this way makes it easier tomodify the profile only once.
Right now this only works for a Bash shell. Cross platform considerations will follow shortly.
c46b3b2 feat: support installing specific versions
In the case where the user supplies a version, we don't need to use the Github API and instead just
go straight to S3 to download the archive.
e56e889 feat: provide node command
The only real modification required for this was to the
envfile to also put~/.safe/nodeon thePATH. Otherwise it goes through identical code paths to the client installation, which has already
been tested quite thoroughly.
Strictly speaking, the
install_binfunction should have test coverage for the node, but as Ialready explained previously, the setup code for these tests is quite involved, so there would be a
lot of duplication with minimal benefit.
720d981 ci: run some integration tests
A few integration tests were defined in CI. They were done here because it's just a bit easier to
use new shell sessions than if they were defined as Rust tests. The tests install a specific version
of the binaries, which will avoid rate limiting because it doesn't use the Github API. One of the
tests runs non-elevated, which then uses a new shell session to make sure safe is available, i.e.,
that the change to the environment variable has been applied. The other test runs in non-elevated
mode and checks that the binaries are immediately available. On Github Actions, in macOS you can
only run elevated, so there is no non-elevated test.
The usage of
reqwestwas changed to switch off default features, which prevents theopensslcrate being referenced in the dependency graph. We have this crate denied in our shared
cargo denyconfiguration, although I don't know why. The
rustls-tlsfeature then needs to be explicitlyenabled for using HTTPS.
Since this was the first time Clippy ran in CI, some flagged issues were fixed.
5158a4c chore: add settings file to keep track of installs
This will be used in the
updatecommand to apply the updates to the locations the binaries wereinstalled. We keep track because the user can provide custom locations.
83507c9 chore: vary location of shell profile
For Linux, we are using ~/.bashrc as the default, whereas for macOS we will use ~/.zshrc, since
ZSH is the default shell.
1c709fb feat: provide
testnetsubcommandSince this goes through all the same code paths, not much was required other than adding a new entry
to the
AssetTypeenum.