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.

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

chore: fix winterfell deprecations and no-std build setup #533

Merged
merged 4 commits into from
Mar 21, 2024

Conversation

bitwalker
Copy link
Contributor

This change addresses upstream deprecations/changes from facebook/winterfell#262 and 0xPolygonMiden/crypto#290.

As a necessary side effect of those deprecations, our no-std build setup needed to be adjusted. See the above-mentioned PRs for more info, but the gist of the change is to always build with #![no_std], and enable std features conditionally, rather than the reverse. For the most part this has little effect on how code is actually written, aside from needing to be more explicit about imports in some cases (anything that would normally be provided by the libstd prelude).

@bitwalker bitwalker added the refactoring Code clean-ups, improvements, and refactoring label Mar 21, 2024
@bitwalker bitwalker self-assigned this Mar 21, 2024
@hackaugusto
Copy link
Contributor

hackaugusto commented Mar 21, 2024

There seems to be one standing warning and some rustfmt changes:

2024-03-21T18:06:15.0030965Z error: the item `Box` is imported redundantly
2024-03-21T18:06:15.0089521Z ##[error] --> mock/src/main.rs:1:11
2024-03-21T18:06:15.0161020Z   |
2024-03-21T18:06:15.0162741Z 1 | use std::{boxed::Box, fs::File, io::Write, path::PathBuf, time::Instant};
2024-03-21T18:06:15.0164185Z   |           ^^^^^^^^^^
2024-03-21T18:06:15.0165886Z  --> /rustc/1388d7a069d872bcfe5e5dd97ef61fa0a586fac0/library/std/src/prelude/mod.rs:125:13
2024-03-21T18:06:15.0167515Z   |
2024-03-21T18:06:15.0169363Z   = note: the item `Box` is already defined here
2024-03-21T18:06:15.0170161Z   |
2024-03-21T18:06:15.0171029Z   = note: `-D unused-imports` implied by `-D warnings`

Copy link
Contributor

@hackaugusto hackaugusto left a comment

Choose a reason for hiding this comment

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

Thanks!!

@bitwalker bitwalker force-pushed the bitwalker/winterfell-deprecations branch from a245d66 to b173163 Compare March 21, 2024 19:29
@bitwalker bitwalker merged commit 2922568 into next Mar 21, 2024
8 checks passed
@bitwalker bitwalker deleted the bitwalker/winterfell-deprecations branch March 21, 2024 19:37
@bitwalker
Copy link
Contributor Author

@hackaugusto I've gotta remember to re-run all my checks with both stable and nightly toolchains, that's why I missed those items, sorry about that!

Everything is green now, so I've gone ahead and merged, but let me know if anything else comes up related to those changes.

@Dominik1999
Copy link
Collaborator

Can you do the same for main @bitwalker ?

@bobbinth
Copy link
Contributor

@Dominik1999 Once we merge next into main these will go away. Is there any reason to do it sooner?

@Dominik1999
Copy link
Collaborator

@bobbinth yes, the question is if we need to fix this before we merge docs PRs. PRs that touch the docs would go to main. And we can merge with the clippy warning, but that might be bad practice.

@bobbinth
Copy link
Contributor

I think if it affecting only docs, clippy warnings are ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code clean-ups, improvements, and refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants