Updates to installation guide #772

danforbes · 2020-11-07T23:02:04Z

Depends on https://github.com/substrate-developer-hub/substrate-node-template/pull/105

bkchr · 2020-11-08T09:26:23Z

docs/tutorials/add-a-pallet/use-a-pallet.md

@@ -6,7 +6,7 @@ Now you are ready to compile and run your node that has been enhanced with nickn
 from the Nicks pallet. Compile the node in release mode with:

 ```bash
-cargo build --release
+WASM_BUILD_TOOLCHAIN=nightly-2020-10-05 cargo build --release


All these tutorials use 2.0.0?

You should use a nightly that is around the release of 2.0.0.

And we should also mention which version should be installed for the tutorials.

Yes, all our tutorials are on v2.0.0 🎉 Furthermore, each tutorial (including this one) mentions the specific version of Substrate/the Node Template on which it is built ✔️

Can you recommend a specific nightly? I was under the impression that 2020-10-05 was "the right" one to use w/ v2.0.0. Is this incorrect? Is there a compelling reason to move to another nightly? I have been specifying 2020-10-05 for a while now and I would prefer not to change things if it's not necessary.

If that works it is okay, but there was some other nightly that broke the compilation in between as well and I think it was before october.

I believe we have constantly been referencing 2020-10-05 with success and no one reporting issues afaik @danforbes @bkchr

docs/knowledgebase/getting-started/index.md

athei

I did not go through the whole file but it just overwrites all changes I made. Not sure why it should be on me to resolve merge conflicts when my PR merged way earlier.

athei · 2020-11-09T09:45:41Z

docs/knowledgebase/getting-started/index.md

-# Add the rust compiler and other tools to your PATH.
-# Make sure to add this to your shell startup script, too.
+# Configure


I do not think it's necessary to dive into the details of what this does. Furthermore, I don't believe it's necessary to add this to the startup script.

You source that file manually in every terminal you want to use rust?

Sorry I just realized I never responded to this comment. I'm not sure if the installation scripts have been updated or something but it's no longer necessary to manually add the command to the shell startup script. I have not added such a command, nor do I need to source the file manually in ever terminal. I reviewed all the installation instructions for Rustup that I could find and I was not able to see this listed anywhere as a requirement https://rust-lang.github.io/rustup/installation/index.html

athei · 2020-11-09T09:49:01Z

docs/knowledgebase/getting-started/index.md

-[`nightly` builds](https://doc.rust-lang.org/book/appendix-07-nightly-rust.html) to allow
-compiled substrate compatible runtimes to Wasm.
+[`nightly` builds](https://doc.rust-lang.org/book/appendix-07-nightly-rust.html) to allow you to
+compile Rust code to the Wasm target.


You can compile Rust code without nightly to Wasm. It is the substrate runtime that requires nightly because of design decisions we made: paritytech/substrate#1252

athei · 2020-11-09T09:54:31Z

docs/knowledgebase/getting-started/index.md

-Because the nightly toolchain is a moving target and receives daily changes the chance
-that some of them break the substrate build from time to time is non-negligible.
-
-Therefore it is advised to use a fixed nightly version rather than the latest one to
-build the runtime. You can install a specific version using this command:
+Developers that are building _with_ Substrate (as opposed to the developers building Substrate
+_itself_) should use a specific Rust nightly version that is known to be compatible with the version


This distinction of with and itself is confusing to me. There is no technical difference. For that reason I specifically removed it.

Substrate master moves with the nightly versions, this means if a future version breaks something we fix it on master. This often makes Substrate master fail to compile with an older nightly. So, if you are using Substrate 2.0.0 they should use a specific nightly version. However, if you develop on Substrate master you will probably need to update your nightly regularly.

So then we should write that instead. Working with master vs. working with a release. You need to tell people the why or they will ignore your advice.

You can write whatever you want, people will still ignore it xD :D

yes I agree with @athei the distinction is confusing and just creates more questions than answers.

athei · 2020-11-09T09:55:47Z

docs/knowledgebase/getting-started/index.md

-If you want to build the runtime with the latest nightly compiler which should **generally** be
-possible you can install the unspecific `nightly` toolchain:
+Developers that are building Substrate _itself_ should always uses the latest bug-free versions of
+Rust stable and nightly. To ensure your Rust compiler is always up to date, you should run:


Same as above

wheresaddie · 2020-11-10T12:32:02Z

happy to look this over once all the modifications are made and do a final edit @danforbes

danforbes · 2020-11-13T20:25:23Z

I did not go through the whole file but it just overwrites all changes I made. Not sure why it should be on me to resolve merge conflicts when my PR merged way earlier.

Not on you whatsoever, @athei. As I mentioned here #739 (comment) my apologies for missing the original PR. I appreciate your contributions but it's important that the people who are maintaining this document and the resources that it supports feel comfortable with the approach that is taken.

athei · 2020-11-16T07:19:14Z

I did not go through the whole file but it just overwrites all changes I made. Not sure why it should be on me to resolve merge conflicts when my PR merged way earlier.

Not on you whatsoever, @athei. As I mentioned here #739 (comment) my apologies for missing the original PR. I appreciate your contributions but it's important that the people who are maintaining this document and the resources that it supports feel comfortable with the approach that is taken.

Can you incorporate what Basti said to explain why this distinction between build with and on substrate?

* Update index.md minor modification to clarify objects of install

wheresaddie · 2020-11-17T19:51:08Z

docs/knowledgebase/getting-started/index.md

+[`nightly` builds](https://doc.rust-lang.org/book/appendix-07-nightly-rust.html) to allow you to
+compile Substrate runtime code to the Wasm target. There is
+[a GitHub Issue](https://github.com/paritytech/substrate/issues/1252) that describes this
+requirement in detail.


I might suggest removing the citation for #1252 here unless you feel its such an FAQ this optimizes the set up time

Completely agree

wheresaddie · 2020-11-17T19:52:00Z

docs/knowledgebase/getting-started/index.md

+and
+[Makefile](https://github.com/substrate-developer-hub/substrate-node-template/blob/master/Makefile)
+to specify the Rust nightly version and encapsulate the following steps. Use Rustup to install the
+correct nightly:

 ```bash
 rustup install nightly-<yyyy-MM-dd>


at the time of writing 2020-11-17, the current default nightly which compiles is 2020-10-05

I don't want to add this because it creates the question: "the current default nightly that compiles with what?"

It may be good idea to mention how to find the correct nightly for any case. You mention how to do it in Pokadot but why not mention that a way to find out is to take a look to the date of the release and use that date in the nightly-<date of release>

From my POV that gets too deeply into supporting other projects. We need to be very careful about what we are documenting here. We are trying to teach people the best practices for working with Substrate code. I think that at this time it is very important to use a well-known nightly version that corresponds with whatever project you are using. I do not want to encourage people to release Substrate-based code without specifying an exact nightly to use, nor do I want to encourage our users to try to "guess" which nightly they should use. This is definitely an area where I am deciding not to document every possible technical detail in order to encourage people to follow what I believe we should set up as best practice.

I agree with almost everything except.

encourage our users to try to "guess" which nightly they should use

I think that this a good to have as a way to troubleshoot. In the back it happens all the time, your machine doesn't has the same nightly as mine because we update at different time. My point is that it is a rule of thumb to get the date of the release and use it for nightly. I'm not talking about other projects I referring to every time we have a Substrate release.

Nope, it could go there but it isn't covered. It would be suggesting them to use a very specific date which will work in 99.9999% of the cases. Substrate 2.0 was release in September 22 so using nightly-2020-09-22 will work for them.

No I'm sorry that is just not something I am comfortable supporting at this time.

I don't get why not. There is no reason for not doing it. It is correct to do it and will prevent us explaining the same thing every time or adding a lot of patches as you have been doing. Please give a good justification for not doing it.

Just saying that you are not comfortable with something that doesn't requires support isn't a valid justification.

I would say I provided that justification in my previous comment #772 (comment) I don't feel comfortable setting a precedent that the Knowledge Base provides general support for any or all Substrate-based projects. I don't even really feel comfortable with the Knowledge Base having these instructions at all. I really believe this information belongs with the Node Template. I do not think it is good practice for a Substrate-based project to make a release without specifying the nightly it builds with, so therefore I do not feel comfortable providing any type of support or advice for these cases. It is "undefined behavior". That is my justification for not feeling comfortable supporting this.

wheresaddie · 2020-11-17T19:53:29Z

docs/knowledgebase/getting-started/index.md

 rustup target add wasm32-unknown-unknown --toolchain nightly
 ```

-This toolchain is not tied to a specific version and will be updated just as the
-`stable` toolchain:
+**It may be necessary to occasionally rerun `rustup update`** if a change in the upstream Substrate


Id remove this comment as again it creates more questions than answers and by the time people need to rerun rustup update I hope they are far enough into the community that this is a known req at times

I think this is something that the core developers have asked that we keep in the docs in order to help with some common questions they receive. I hear what you're saying for sure and I do think it's important we don't get too into the weeds in this doc, but given the above and the fact that this statement seems like it logically dovetails with the preceding statement I think I'm going to leave it at this time.

apopiak

Seems fine to me, left a question.

apopiak · 2020-11-18T15:18:53Z

docs/knowledgebase/getting-started/index.md

-Note that this builds only the runtime with the specified toolchain. The rest of project will
-be compiled with your default toolchain, which is usually the latest installed stable toolchain.


Why remove this, it seems like helpful info for this context?

riusricardo

All the SKIP_WASM_BUILD should have a value either 1 or true

riusricardo · 2020-11-18T15:16:27Z

docs/knowledgebase/getting-started/index.md

+and
+[Makefile](https://github.com/substrate-developer-hub/substrate-node-template/blob/master/Makefile)
+to specify the Rust nightly version and encapsulate the following steps. Use Rustup to install the
+correct nightly:

 ```bash
 rustup install nightly-<yyyy-MM-dd>


It may be good idea to mention how to find the correct nightly for any case. You mention how to do it in Pokadot but why not mention that a way to find out is to take a look to the date of the release and use that date in the nightly-<date of release>

docs/tutorials/add-contracts-pallet.md

docs/tutorials/add-a-pallet/import-a-pallet.md

docs/tutorials/create-a-pallet.md

Co-authored-by: Ricardo Rius <[email protected]>

Updates to installation guide

9dec59b

danforbes requested review from folsen, jimmychu0807, wheresaddie, shawntabrizi, bkchr, riusricardo, kaichaosun and joepetrowski November 7, 2020 23:02

Update tutorial commands

5057661

bkchr reviewed Nov 8, 2020

View reviewed changes

This was referenced Nov 8, 2020

Update index.md #768

Merged

Rework how the toolchains are explained #739

Merged

apopiak reviewed Nov 9, 2020

View reviewed changes

docs/knowledgebase/getting-started/index.md Outdated Show resolved Hide resolved

athei suggested changes Nov 9, 2020

View reviewed changes

Suggestions from @athei and @apopiak review

3aa491d

wheresaddie and others added 2 commits November 16, 2020 09:05

Update index.md (#778)

bf5263e

* Update index.md minor modification to clarify objects of install

A few minor rewordings

ff7371d

wheresaddie reviewed Nov 17, 2020

View reviewed changes

athei approved these changes Nov 18, 2020

View reviewed changes

Trim content per @wheresaddie

a4f4fb2

apopiak approved these changes Nov 18, 2020

View reviewed changes

riusricardo reviewed Nov 18, 2020

View reviewed changes

Dan Forbes and others added 3 commits November 18, 2020 08:03

Do not use env vars with empty values

910e56b

Co-authored-by: Ricardo Rius <[email protected]>

Suggestion per @apopiak

28fd949

Rework heading title

5866031

danforbes merged commit 69573cf into source Nov 18, 2020

danforbes deleted the df/fix/installation branch November 18, 2020 17:50

		Note that this builds only the runtime with the specified toolchain. The rest of project will
		be compiled with your default toolchain, which is usually the latest installed stable toolchain.

Updates to installation guide #772

Updates to installation guide #772

Conversation

danforbes commented Nov 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

athei left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wheresaddie commented Nov 10, 2020

danforbes commented Nov 13, 2020

athei commented Nov 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wheresaddie Nov 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riusricardo Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riusricardo Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riusricardo Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apopiak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riusricardo left a comment

Choose a reason for hiding this comment

riusricardo Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

athei left a comment •

edited

Loading

wheresaddie Nov 17, 2020 •

edited

Loading

riusricardo Nov 18, 2020 •

edited

Loading

riusricardo Nov 18, 2020 •

edited

Loading

riusricardo Nov 18, 2020 •

edited

Loading

riusricardo Nov 18, 2020 •

edited

Loading