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.

Sign up for GitHub

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

Jump to bottom

Update Rust documentation with workflow speed improvements #1024

Closed
wants to merge 2 commits into from
Closed

Update Rust documentation with workflow speed improvements #1024

wants to merge 2 commits into from

Conversation

tgross35
Copy link
Collaborator

@tgross35 tgross35 commented Jul 15, 2023 •
edited
Loading

This updates instructions for downloading the Rust source tree to use the tarball rather than cloning the repository.

I'll submit this to the list but a quick review here is nice :)

@tgross35
Copy link
Collaborator Author

Talked on Zulip and the team recommended using the source tarball rather than cloning via git. This is significantly faster compared to the time to clone, and it only keeps the library/ directory so space usage is much better.

Technically rustup componnet add rust-src adds the entire source tree. I can adjust it to do this if needed, but I don't think anything outside of library is used.

I have tested this by using the standalone installer in a docker container then runninng the command, default build with Rust succeeds.

@tgross35 tgross35 force-pushed the doc-speed-improvements branch 2 times, most recently from 83982cb to 96d7b75 Compare July 16, 2023 01:14
@ojeda
Copy link
Member

ojeda commented Jul 16, 2023 •
edited
Loading

Update sample 'make' commands to use '-j$(nproc)' to run parallel builds

Originally, I intentionally did not write extra options, because the idea was to show what was required, not anything extra.

In other words, the document is not intended to be a general guide to kernel development, or to replace the kernel documentation elsewhere nor the GNU Make documentation; in order to keep it minimal and focused on the Rust bits.

@tgross35
Copy link
Collaborator Author

Understood, I can remove that.

Are you ok with switching to the tarball?

@ojeda
Copy link
Member

ojeda commented Jul 16, 2023

Unsure -- can you think of some reason not to do it? For instance, is it easier/harder when updating? I guess in both cases the user can simply delete the folder and reapply the instructions, right?

By the way, the sentence "...upgrading the Rust compiler version later on requires manually updating this clone." would need to be slightly tweaked too if we do it.

@tgross35
Copy link
Collaborator Author

Unsure -- can you think of some reason not to do it?

Nope, but I thought maybe you could :) It seems straightforward enough and I think that is how rustup does it

By the way, the sentence "...upgrading the Rust compiler version later on requires manually updating this clone." would need to be slightly tweaked too if we do it.

Thanks, I will update this when I fix the other thing

@bjorn3
Copy link
Member

bjorn3 commented Jul 16, 2023

There are two separate tarballs. One with the source code of the entirety of rustc which you can use to build rustc from scratch (rustc-1.71.0-src) and one which contains just enough to build the standard library (rust-1.71.0-src). It is the later which ends up providing the files in lib/rustlib/src/rust. It has a different layout from rustc-src though.

@tgross35
Copy link
Collaborator Author

There are two separate tarballs. One with the source code of the entirety of rustc which you can use to build rustc from scratch (rustc-1.71.0-src) and one which contains just enough to build the standard library (rust-1.71.0-src). It is the later which ends up providing the files in lib/rustlib/src/rust. It has a different layout from rustc-src though.

That one is much smaller, I'll update this to use that one. Thanks for the link

@tgross35 tgross35 force-pushed the doc-speed-improvements branch 2 times, most recently from a5c3e43 to 03028b0 Compare July 17, 2023 03:49
@tgross35
Copy link
Collaborator Author

I removed mentions of -j$(nproc) and updated to use the smaller source tarball. Great change, it takes about 5 seconds to download instead of about 8 minutes for the original git clone.

I can submit this to the list if there aren't any further changes needed.

@ojeda
Copy link
Member

ojeda commented Jul 17, 2023 •
edited
Loading

Sounds good. (To be clear, reviews typically happen in the list, i.e. you may get more feedback there.)

I would recommend splitting the changes into two different patches, since they are independent. Also, I would explain the improvement in time in the commit message.

With an standalone installer, is the sysroot the same when you install a different version? I am asking because if the sysroot stays the same in some cases (unlike e.g. rustup ones), the clone would have the advantage of making it a bit easier to switch versions (i.e. just checking out the other tag).

@tgross35 tgross35 force-pushed the doc-speed-improvements branch 3 times, most recently from 40f7554 to 93a4cfd Compare July 18, 2023 04:18
@tgross35
Copy link
Collaborator Author

tgross35 commented Jul 18, 2023 •
edited
Loading

With an standalone installer, is the sysroot the same when you install a different version? I am asking because if the sysroot stays the same in some cases (unlike e.g. rustup ones), the clone would have the advantage of making it a bit easier to switch versions (i.e. just checking out the other tag).

It seems like they do not change by version, testing on debian the system package manager uses /usr/lib/rustlib/src and the standalone installer uses /usr/local/lib/rustlib/src. I updated the documentation to just suggest removing /lib/rustlib/src/rust within the rust sysroot, which is sufficient (verified the command works again after removing just that directory).

I will post this to the list shortly.

edit: all done https://lore.kernel.org/rust-for-linux/20230718054416.861412-1-tmgross@umich.edu/T/#t

@tgross35
docs: rust: update instructions for obtaining 'core' source
1a61d68 
The source for Rust's 'core' library is needed to build the kernel with
Rust support. This must be obtained manually when using a non-rustup
install, such as when using 'rustc' from a package manager or from a
standalone installer. Currently, the documentation suggests cloning the
'rust' repository to obtain these sources, but this is quite slow (on
the order of a few minutes).

This patch changes this documentation to suggest using the source
tarball instead, which includes only needed information (<5M) and is
significantly faster to download. This is more in line with what
'rustup' does.

Signed-off-by: Trevor Gross <tmgross@umich.edu>
@tgross35
docs: rust: clarify what 'rustup override' does
002cac1 
The behavior of 'rustup override' is not very well known. This patch is
a small edit that adds details about what it does, so users have a better
understanding of how it affects their system toolchain (i.e., it does
not affect system toolchain and only sets a directory-specific
override).

Signed-off-by: Trevor Gross <tmgross@umich.edu>
@tgross35 tgross35 closed this Jul 18, 2023
@ojeda
Copy link
Member

ojeda commented Jul 18, 2023

Thanks! I see it in Lore now (it took a while to arrive, it seems).

Looks quite good for a first submission!

tgross35 added a commit to tgross35/linux that referenced this pull request Aug 3, 2023
@tgross35
docs: rust: update instructions for obtaining 'core' source
bb665b7 
The source for Rust's 'core' library is needed to build the kernel with
Rust support. This sometimes needs to be obtained by hand when using a
standalone version of 'rustc' not managed by 'rustup'. Currently, the
documentation suggests cloning the 'rust' repository to obtain these
sources, but this is quite slow (on the order of a multiple minutes).

Change this documentation to suggest using the source tarball instead.
The tarball includes only needed files (<5M) and is significantly faster
to download; this is more in line with what 'rustup' does.

Also simplify wording of the relevant section.

Link: Rust-for-Linux#1024
Signed-off-by: Trevor Gross <tmgross@umich.edu>
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this pull request Aug 3, 2023
@tgross35 @intel-lab-lkp
docs: rust: update instructions for obtaining 'core' source
ab1831d 
The source for Rust's 'core' library is needed to build the kernel with
Rust support. This sometimes needs to be obtained by hand when using a
standalone version of 'rustc' not managed by 'rustup'. Currently, the
documentation suggests cloning the 'rust' repository to obtain these
sources, but this is quite slow (on the order of a multiple minutes).

Change this documentation to suggest using the source tarball instead.
The tarball includes only needed files (<5M) and is significantly faster
to download; this is more in line with what 'rustup' does.

Also simplify wording of the relevant section.

Link: Rust-for-Linux#1024
Signed-off-by: Trevor Gross <tmgross@umich.edu>
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
ojeda pushed a commit that referenced this pull request Aug 20, 2023
@tgross35 @ojeda
docs: rust: update instructions for obtaining 'core' source
8cb4012 
The source for Rust's 'core' library is needed to build the kernel with
Rust support. This sometimes needs to be obtained by hand when using a
standalone version of 'rustc' not managed by 'rustup'. Currently, the
documentation suggests cloning the 'rust' repository to obtain these
sources, but this is quite slow (on the order of a multiple minutes).

Change this documentation to suggest using the source tarball instead.
The tarball includes only needed files (<5M) and is significantly faster
to download; this is more in line with what 'rustup' does.

Also simplify wording of the relevant section.

Link: #1024
Signed-off-by: Trevor Gross <tmgross@umich.edu>
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20230803060437.12157-2-tmgross@umich.edu
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ojeda ojeda ojeda left review comments

@bjorn3 bjorn3 bjorn3 left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tgross35 @ojeda @bjorn3