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

zeroize's cargo test fails immediately #237

Closed
burdges opened this issue Jul 25, 2019 · 12 comments
Closed

zeroize's cargo test fails immediately #237

burdges opened this issue Jul 25, 2019 · 12 comments

Comments

@burdges
Copy link

burdges commented Jul 25, 2019 •
edited
Loading

There is seemingly some bug in the proc macros

error: proc-macro derive panicked
 --> zeroize/tests/zeroize_derive.rs:7:14
  |
7 |     #[derive(Zeroize)]
  |              ^^^^^^^
  |
  = help: message: unknown zeroize attribute: (drop)

I must work around this anyways though until dalek adopts zeroize
@burdges burdges changed the title cargo test fails immediately zeroize's cargo test fails immediately Jul 25, 2019
burdges added a commit to w3f/schnorrkel that referenced this issue Jul 25, 2019
@burdges
Fix NCC-1905_Web3Foundation_Schnorrkel-006
a2e6573 
Zeroize crate's derive is broken right now.
iqlusioninc/crates#237

Also dalek needs to switch
dalek-cryptography/curve25519-dalek#236
@tarcieri
Copy link
Collaborator

These proc macros are used in several projects, although the attribute parser could definitely use some work.

Can you provide a failing code example?

@burdges
Copy link
Author

burdges commented Jul 25, 2019 •
edited
Loading

cargo test suffices.. or pasting doc tests into playground. It's maybe a miss-matched version for the zeroize_derive crate?

@tarcieri
Copy link
Collaborator

I cannot reproduce your issue. The tests also run in CI every time any change is made to this repo:

$ cargo test
    Finished dev [unoptimized + debuginfo] target(s) in 0.15s
     Running /Users/bascule/src/iqlusion/crates/target/debug/deps/zeroize-868d63cfefbad716

running 5 tests
test tests::zeroize_box ... ok
test tests::zeroize_vec ... ok
test tests::zeroize_vec_past_len ... ok
test tests::zeroize_byte_arrays ... ok
test tests::zeroize_string ... ok

test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running /Users/bascule/src/iqlusion/crates/target/debug/deps/zeroize_derive-2cd2a713b1ce329b

running 2 tests
test custom_derive_tests::derive_tuple_struct_test ... ok
test custom_derive_tests::derive_struct_test ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

   Doc-tests zeroize

running 4 tests
test src/lib.rs -  (line 69) ... ok
test src/lib.rs -  (line 95) ... ok
test src/lib.rs -  (line 80) ... ok
test src/lib.rs -  (line 24) ... ok

test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

@burdges
Copy link
Author

burdges commented Jul 25, 2019 •
edited
Loading

I cut & paste your doc test into the playground : https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=bac38f5fb1ac22f94f5094cc2ca47651

@tarcieri
Copy link
Collaborator

The zeroize crate is not available in the playground:

error[E0432]: unresolved import `zeroize`

@burdges
Copy link
Author

burdges commented Jul 25, 2019 •
edited
Loading

I see the same error I copied above. Oops no

Anyways I'll show the bash version:

$ git clone https://github.com/iqlusioninc/crates
Cloning into 'crates'...
remote: Enumerating objects: 36, done.
remote: Counting objects: 100% (36/36), done.
remote: Compressing objects: 100% (30/30), done.
remote: Total 3037 (delta 13), reused 17 (delta 6), pack-reused 3001
Receiving objects: 100% (3037/3037), 842.65 KiB | 1.66 MiB/s, done.
Resolving deltas: 100% (1670/1670), done.
$ cd crates/zeroize
$ cargo test
   Compiling proc-macro2 v0.4.30
   Compiling unicode-xid v0.1.0
   Compiling syn v0.15.39
   Compiling quote v0.6.13
   Compiling synstructure v0.10.2
   Compiling zeroize_derive v0.9.0 (/Users/jeff/t/t/crates/zeroize_derive)
   Compiling zeroize v0.9.2 (/Users/jeff/t/t/crates/zeroize)
error: proc-macro derive panicked
 --> zeroize/tests/zeroize_derive.rs:7:14
  |
7 |     #[derive(Zeroize)]
  |              ^^^^^^^
  |
  = help: message: unknown zeroize attribute: (drop)

error: aborting due to previous error

error: Could not compile `zeroize`.
warning: build failed, waiting for other jobs to finish...
error: build failed

@tarcieri
Copy link
Collaborator

The crate versions I see compiling are all the same (they should be, since Cargo.lock is checked in).

What version of Rust are you using?

@burdges
Copy link
Author

burdges commented Jul 25, 2019

Appears the latest nightly:

$ rustup update
info: syncing channel updates for 'stable-x86_64-apple-darwin'
info: syncing channel updates for 'nightly-x86_64-apple-darwin'
info: checking for self-updates

   stable-x86_64-apple-darwin unchanged - rustc 1.36.0 (a53f9df32 2019-07-03)
  nightly-x86_64-apple-darwin unchanged - rustc 1.38.0-nightly (03f19f7ff 2019-07-24)

@tarcieri
Copy link
Collaborator

That seems to be the main issue, then. We only test on stable.

I'll leave this open for now as I do plan on rewriting the attribute parser before the next release, which may incidentally address this.

@burdges
Copy link
Author

burdges commented Jul 25, 2019

I see. I suppose proc macros have some flux on nightly.

@tarcieri
Copy link
Collaborator

This is breaking the builds for some of my docs, so I'm going to look into it ASAP.

tarcieri pushed a commit that referenced this issue Jul 27, 2019
@tony-iqlusion
zeroize: Improved attribute parser (fixes #237)
c2df57d 
The previous attribute parser used a hack where it would compare to the
s-exp representation of zeroize attributes to determine of they are
`drop` or `no_drop`. This was a bit of a hack that broke with a recent
nightly, which removed the spaces surrounding the ident.

This commit rewrites the attribute parser to properly walk the attribute
`syn::Meta` nodes in the AST until it arrives at a `syn::Ident`
containing either `drop` or `no_drop`. All other AST nodes will result
in a panic.

This relaxes the previous restriction that any `#[derive(Zeroize)]` MUST
be annotated with either `drop` or `no_drop`. This was put in place
because `zeroize` v0.9 switched from default drop to requiring an
explicit attribute. However v0.8 was the only version with implict drop,
and has been yanked for several weeks. The change is fully backwards
compatible and should have no effect on any existing v0.9 users.
tony-iqlusion added a commit that referenced this issue Jul 27, 2019
@tony-iqlusion
Merge pull request #238 from iqlusioninc/zeroize/fix-nightly-support
9af5365 
zeroize: Improved attribute parser (fixes #237)
@tarcieri
Copy link
Collaborator

I rewrote the attribute parser in #238 and confirmed it passes on nightly.

Will release right now as v0.9.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@tarcieri @burdges