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

doc: Add missing --locked to C2Rust installation instructions #17694

Closed
wants to merge 1 commit into from
Closed

doc: Add missing --locked to C2Rust installation instructions #17694

wants to merge 1 commit into from

Conversation

JKRhb
Copy link
Member

@JKRhb JKRhb commented Feb 23, 2022

Contribution description

This PR adds a second --locked flag to the install instructions of C2Rust. Without it, the installation fails as described in #17492.

Testing procedure

Documentation should be updated.

Issues/PRs references

Fixes #17492 (again).

@github-actions github-actions bot added the Area: doc Area: Documentation label Feb 23, 2022
@JKRhb JKRhb requested a review from chrysn February 23, 2022 23:50
@JKRhb JKRhb added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Feb 23, 2022
kaspar030
kaspar030 reviewed Feb 24, 2022
View reviewed changes
doc/doxygen/src/using-rust.md
@@ -122,7 +122,7 @@ and needs some patches applied:
```shell
$ rustup install nightly-2019-12-05
$ rustup component add --toolchain nightly-2019-12-05 rustfmt rustc-dev
$ cargo +nightly-2019-12-05 install c2rust
$ cargo +nightly-2019-12-05 install --locked c2rust
Copy link
Contributor

Choose a reason for hiding this comment

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

hm, isn't this either-or? either install from crates.io (using this line), or from the checked-out path? and, older cargo versions don't support/honor "--locked" when installing from crates.io (vs. local path), IIRC.

@chrysn knows more.

Copy link
Member

Choose a reason for hiding this comment

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

That's a weird error in the documentation I wrote -- that line shouldn't be there in the first place, the relevant install is 3 lines down where I fixed the --locked in https://github.com/RIOT-OS/RIOT/pull/17504/files

(The c2rust uploaded to crates.io is also not the one working, no clue what went wrong there precisely; that changed line should just go away)

@chrysn chrysn mentioned this pull request Feb 24, 2022
Merged
chrysn
chrysn requested changes Feb 24, 2022
View reviewed changes
Copy link
Member

@chrysn chrysn left a comment

Choose a reason for hiding this comment

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

It's an incomplete fix because there shouldn't be two installs in the first place, see #17700 for a full fix.

doc/doxygen/src/using-rust.md
@@ -122,7 +122,7 @@ and needs some patches applied:
```shell
$ rustup install nightly-2019-12-05
$ rustup component add --toolchain nightly-2019-12-05 rustfmt rustc-dev
$ cargo +nightly-2019-12-05 install c2rust
$ cargo +nightly-2019-12-05 install --locked c2rust
Copy link
Member

Choose a reason for hiding this comment

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

That's a weird error in the documentation I wrote -- that line shouldn't be there in the first place, the relevant install is 3 lines down where I fixed the --locked in https://github.com/RIOT-OS/RIOT/pull/17504/files

(The c2rust uploaded to crates.io is also not the one working, no clue what went wrong there precisely; that changed line should just go away)

@JKRhb
Copy link
Member Author

JKRhb commented Feb 24, 2022

Superseded by #17700

@JKRhb JKRhb closed this Feb 24, 2022
@JKRhb JKRhb deleted the rust-locked-flag branch February 24, 2022 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaspar030 kaspar030 kaspar030 left review comments

@chrysn chrysn chrysn requested changes

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

Assignees
No one assigned
Labels
Area: doc Area: Documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Using Rust doc toolchain info is obsolete
3 participants
@JKRhb @chrysn @kaspar030