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

Copy-paste things needed for nearcore#8323 #8792

Merged
merged 5 commits into from
Mar 24, 2023

Conversation

Ekleog-NEAR
Copy link
Collaborator

@Ekleog-NEAR Ekleog-NEAR commented Mar 23, 2023

This is only copy-paste, and should be purely non-functional changes only.

Some files were deleted from the 127ff1164ff commit of wasmer, in order to not use space in the git history forever.

Further changes needed to actually implement the protocol update will be done in separate PRs, that will also be extracted from nearcore#8323.

@Ekleog-NEAR Ekleog-NEAR requested a review from a team as a code owner March 23, 2023 17:12
runtime/near-vm/.github/CODEOWNERS Outdated Show resolved Hide resolved
runtime/near-vm/.gitattributes Outdated Show resolved Hide resolved
runtime/near-vm/.cargo/config.toml Outdated Show resolved Hide resolved
runtime/near-vm/.github/ISSUE_TEMPLATE/---bug-report.md Outdated Show resolved Hide resolved
runtime/near-vm/Cargo.lock Outdated Show resolved Hide resolved
runtime/near-vm/PACKAGING.md Outdated Show resolved Hide resolved
runtime/near-vm/SECURITY.md Outdated Show resolved Hide resolved
@@ -0,0 +1,43 @@
targets = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Integrate with the top-level deny.toml

@Ekleog-NEAR
Copy link
Collaborator Author

I’ve dropped these files as part of #8796, so that this PR remains a simple copy-paste even when it’ll have been squashed by near-buildozer :)

@near-bulldozer near-bulldozer bot merged commit 54c5d4f into near:master Mar 24, 2023
near-bulldozer bot pushed a commit that referenced this pull request Mar 27, 2023
This PR builds on top of #8792. It continues the work mainlining #8323.

Ignore the first three commits to review only the changes actually brought by this PR
nikurt pushed a commit that referenced this pull request Mar 28, 2023
This PR builds on top of #8792. It continues the work mainlining #8323.

Ignore the first three commits to review only the changes actually brought by this PR
nikurt pushed a commit that referenced this pull request Mar 29, 2023
This PR builds on top of #8792. It continues the work mainlining #8323.

Ignore the first three commits to review only the changes actually brought by this PR
near-bulldozer bot pushed a commit that referenced this pull request Mar 29, 2023
Promised follow-up of #8792. I migrated one crate per commit at the beginning, and when most crates did not lead to changes in the root-level Cargo.toml started migrating a few at once. Would recommend commit-by-commit review as I checked that cargo resolution didn’t change after each commit.

Given the bitrot potential I’m marking as automerge right now.
@aborg-dev
Copy link
Contributor

Hey @Ekleog-NEAR , for some reason this broke my local test build with the following error message:

akashin@hal ~/r/nearcore (compute_costs_feature)> cargo test
   Compiling near-o11y v0.0.0 (/Users/akashin/repos/nearcore/core/o11y)
   Compiling awc v3.0.0
   Compiling near-vm-vm v0.0.0 (/Users/akashin/repos/nearcore/runtime/near-vm/lib/vm)
   Compiling neard v0.0.0 (/Users/akashin/repos/nearcore/neard)
   Compiling near-vm-compiler-test-derive v0.0.0 (/Users/akashin/repos/nearcore/runtime/near-vm/tests/lib/compiler-test-derive)
   Compiling near-vm-types v0.0.0 (/Users/akashin/repos/nearcore/runtime/near-vm/lib/types)
error: mismatched attribute proc macro signature
  --> runtime/near-vm/tests/lib/compiler-test-derive/src/lib.rs:40:65
   |
40 | pub fn compiler_test(attrs: TokenStream, input: TokenStream) -> TokenStream {
   |                                                                 ^^^^^^^^^^^ found TokenStream2, expected type `proc_macro::TokenStream`
   |
   = note: attribute proc macros must have a signature of `fn(TokenStream, TokenStream) -> TokenStream`

error: mismatched attribute proc macro signature
  --> runtime/near-vm/tests/lib/compiler-test-derive/src/lib.rs:40:29
   |
40 | pub fn compiler_test(attrs: TokenStream, input: TokenStream) -> TokenStream {
   |                             ^^^^^^^^^^^ found TokenStream2, expected type `proc_macro::TokenStream`
   |
   = note: attribute proc macros must have a signature of `fn(TokenStream, TokenStream) -> TokenStream`

error: mismatched attribute proc macro signature
  --> runtime/near-vm/tests/lib/compiler-test-derive/src/lib.rs:40:49
   |
40 | pub fn compiler_test(attrs: TokenStream, input: TokenStream) -> TokenStream {
   |                                                 ^^^^^^^^^^^ found TokenStream2, expected type `proc_macro::TokenStream`
   |
   = note: attribute proc macros must have a signature of `fn(TokenStream, TokenStream) -> TokenStream`

error: could not compile `near-vm-compiler-test-derive` due to 3 previous errors
warning: build failed, waiting for other jobs to finish...

The rustc version: rustc 1.69.0-nightly (13471d3b2 2023-03-02)

Do you have any suggestions on how to fix this?

@nagisa
Copy link
Collaborator

nagisa commented Apr 3, 2023

@Akashin try cargo clean first and foremost.

near-bulldozer bot pushed a commit that referenced this pull request Apr 3, 2023
Github displays nearcore language as WebAssembly which is confusing:
<img width="464" alt="Screenshot 2023-04-03 at 11 49 02" src="https://user-images.githubusercontent.com/3171838/229474854-3597d9ec-6f7c-4efe-815c-11a3e4bee23d.png">
This is caused by #8792.
Fixed by ignoring test wast files.
@aborg-dev
Copy link
Contributor

Thanks for the suggestion, @nagisa. I ran cargo clean before the cargo test, but it still fails with the same error. Another data-point is that cargo build works fine.

Digging more into this and reading https://petanode.com/posts/rust-proc-macro/, is seems like the solution is to use

proc_macro::TokenStream instead of proc_macro2::TokenStream in the function compiler_test(attrs: TokenStream, input: TokenStream) -> TokenStream. But then apparently there was some reason why this was done differently?

@nagisa
Copy link
Collaborator

nagisa commented Apr 3, 2023

Probably not, this code is just quite old and IIRC wasn't part of CI for quite some time at this point, I believe.

@Ekleog-NEAR
Copy link
Collaborator Author

@Akashin Can you try with cargo nextest run? cargo test is unsupported by quite a few of our tests already, and on my GCP VM, running cargo nextest run from the root of aab4fa5 passes. (Using rustc 1.68.2, which is the one specified by the rust-toolchain.toml file on this commit.)

Anyway, if you have a fix to the issue you’re seeing feel free to fix it; as @nagisa said it’s literally just old code copy-pasted from the wasmer that I’m not at all sure whether/how it’s even used in practice :)

@aborg-dev
Copy link
Contributor

@Ekleog-NEAR , ah, I didn't know about nextest, looks fancy!
After a bit of experimentation, I found that using Rust 1.68.2 fixes the problem on my end, so that must have been the root cause.
I think this is good enough for me, thanks for the help!

nikurt pushed a commit to nikurt/nearcore that referenced this pull request Apr 5, 2023
This is only copy-paste, and should be purely non-functional changes only.

Some files were deleted from the 127ff1164ff commit of wasmer, in order to not use space in the git history forever.

Further changes needed to actually implement the protocol update will be done in separate PRs, that will also be extracted from nearcore#8323.
nikurt pushed a commit to nikurt/nearcore that referenced this pull request Apr 5, 2023
This PR builds on top of near#8792. It continues the work mainlining near#8323.

Ignore the first three commits to review only the changes actually brought by this PR
nikurt pushed a commit to nikurt/nearcore that referenced this pull request Apr 5, 2023
Promised follow-up of near#8792. I migrated one crate per commit at the beginning, and when most crates did not lead to changes in the root-level Cargo.toml started migrating a few at once. Would recommend commit-by-commit review as I checked that cargo resolution didn’t change after each commit.

Given the bitrot potential I’m marking as automerge right now.
nikurt pushed a commit to nikurt/nearcore that referenced this pull request Apr 5, 2023
Github displays nearcore language as WebAssembly which is confusing:
<img width="464" alt="Screenshot 2023-04-03 at 11 49 02" src="https://user-images.githubusercontent.com/3171838/229474854-3597d9ec-6f7c-4efe-815c-11a3e4bee23d.png">
This is caused by near#8792.
Fixed by ignoring test wast files.
nikurt pushed a commit that referenced this pull request Apr 28, 2023
Github displays nearcore language as WebAssembly which is confusing:
<img width="464" alt="Screenshot 2023-04-03 at 11 49 02" src="https://user-images.githubusercontent.com/3171838/229474854-3597d9ec-6f7c-4efe-815c-11a3e4bee23d.png">
This is caused by #8792.
Fixed by ignoring test wast files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants