Skip to content
This repository has been archived by the owner on Feb 28, 2021. It is now read-only.

Scripts for "cargo install" #392

Open
CodeSandwich opened this issue Apr 23, 2020 · 7 comments
Open

Scripts for "cargo install" #392

CodeSandwich opened this issue Apr 23, 2020 · 7 comments

Comments

@CodeSandwich
Copy link
Contributor

We have scripts/build-release, which generates the node and cli binaries.

It would be convenient to have a script to install the binaries from source with cargo install. It would ease the process of installating from source and remove the $PATH step.

My proposition:
scripts/install_node:

DEFAULT_CHAIN=ffnet cargo install --path node 

scripts/install_cli:

cargo install --path cli

Also we should turn https://github.com/radicle-dev/radicle-registry#build-from-source to "install from source" using these new scripts and update the link in http://registry.radicle.xyz/docs/getting-started/#installation.

@NunoAlexandre
Copy link
Contributor

Good stuff! Some thoughts :

  • I always read "build from source", so "install from source" is not so much of common language. Maybe that's fine though

  • I would prefer to separate the build from the installation. Say you have a local version of the binaries and we want to build a different one, and run both to compare something. With this approach, building implies installing, so doing that would require picking commands or ad hoc some to get there. If we separate the steps we wouldn't need that.

Maybe we pass a flag to the script to signal whether we want to install it or not?

@geigerzaehler
Copy link

I like the overall idea but agree with @NunoAlexandre’s point. We should keep the language and make the installation optional. We should also considering making ffnet the default DEFAULT_CHAIN and specifying devnet as the chain in ./scripts/build-dev-node.

@CodeSandwich
Copy link
Contributor Author

There seems to be a misunderstanding, I want to add the installation scripts, not replace any existing ones, build-release would be kept untouched.

I always read "build from source", so "install from source" is not so much of common language.

I agree, install from source does look cumbersome, lets not rename it.

We should also considering making ffnet the default DEFAULT_CHAIN

That would force us, the developers, to pass DEFAULT_CHAIN=dev on every test compilation. I think that the current state is much more ergonomic for us. Building the binaries for the public is rare and scripted anyway.

@geigerzaehler
Copy link

That would force us, the developers, to pass DEFAULT_CHAIN=dev on every test compilation. I think that the current state is much more ergonomic for us. Building the binaries for the public is rare and scripted anyway.

This depends on how we’re building the node for development. I personally use ./scripts/build-dev-node and ./scripts/run-dev-node as documented. Unless somebody uses cargo build --release alone this we could just adjust these scripts.

Another thing that came to mind: cargo install only works if ~/.cargo/bin is on PATH. So even with that script we need some additional explanation. Considering that I’d prefer to not have the script to keep them number of scripts we have to maintain and the users can possibly run small.

@CodeSandwich
Copy link
Contributor Author

Unless somebody uses cargo build --release alone this we could just adjust these scripts.

Sometimes I do use cargo directly, sometimes I'm using a custom script. I think that overall it's just another thing to remember and creates a trap for the newcomers. And what are we gaining? For every release build there are hundreds of development builds, so we should optimize the compilation UX for them.

cargo install only works if ~/.cargo/bin is on PATH

I think that this is set up automatically during the installation of cargo.

@geigerzaehler
Copy link

cargo install only works if ~/.cargo/bin is on PATH

I think that this is set up automatically during the installation of cargo.

Could you verify this. If so, I’m down for this change.

@CodeSandwich
Copy link
Contributor Author

I've verified it and it's true: https://github.com/rust-lang/rustup#installation

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants