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

Wrong exit status #2117

Closed
RuvendeGroote opened this issue Aug 11, 2022 · 11 comments · Fixed by #2370
Closed

Wrong exit status #2117

RuvendeGroote opened this issue Aug 11, 2022 · 11 comments · Fixed by #2370

Comments

@RuvendeGroote
Copy link
Contributor

RuvendeGroote commented Aug 11, 2022

Description

Anchor commands (that wrap around other commands like cargo build-bpf) that result in something failing still return exit status 0.

Steps to reproduce

anchor init some-name
cd some-name
echo mistake >> programs/some-name/src/lib.rs
anchor build
echo $?

Actual result

exit status is 0

Expected result

exit status is equal to the exit status of the underlying command. (something non zero as compilation fails)

System

anchor-cli: 0.24.2
avm: 0.24.2
Apple M1 Pro
rustc 1.63.0
solana-cargo-build-bpf 1.10.32
bpf-tools v1.27

@RuvendeGroote
Copy link
Contributor Author

RuvendeGroote commented Aug 11, 2022

I've had the same result when using anchor-cli 0.25.0
I've investigated commit 6baed77eb57d41455a5f3c1e026d86da756d1c8e and seen that the correct exit status reaches this point

std::process::exit(exit.status.code().unwrap());
but I'm then stuck and not able to explain why it goes wrong.

@RuvendeGroote
Copy link
Contributor Author

RuvendeGroote commented Aug 11, 2022

if you run cargo build-bpf; echo $? instead of anchor build you will see an exit status of 1

@RuvendeGroote
Copy link
Contributor Author

Same happens with anchor test. It also happens in our pipelines (they were green but should have been red because of failing tests) which run linux. This has the risk of buggy code being merged and maybe released.

@dnut
Copy link
Contributor

dnut commented Aug 17, 2022

Same happens with anchor test. It also happens in our pipelines (they were green but should have been red because of failing tests) which run linux. This has the risk of buggy code being merged and maybe released.

Yeah this is causing us the same issue in github workflows. I'd like to see this issue get fixed. It's probably an easy fix, but I have too much on my plate at the moment.

@richardwu
Copy link

richardwu commented Aug 30, 2022

Looks like this is fixed upstream (tested when building from source).

The reason cargo install --git https://github.com/project-serum/anchor avm --locked and avm install latest doesn't pick up latest master is because avm install uses git tags (most recent one was v0.25.0 on July 5th w/o fixes).

@armaniferrante any way to cut a new release, or at the very least create a v0.2X.X tag so avm can install the latest?

EDIT: You can use the latest upstream cli built from source in your CI/CD for now:

# Optional --rev <SHA> to freeze source
cargo install --git https://github.com/project-serum/anchor anchor-cli --locked --force

@RuvendeGroote
Copy link
Contributor Author

Looks like this is fixed upstream (tested when building from source).

The reason cargo install --git https://github.com/project-serum/anchor avm --locked and avm install latest doesn't pick up latest master is because avm install uses git tags (most recent one was v0.25.0 on July 5th w/o fixes).

@armaniferrante any way to cut a new release, or at the very least create a v0.2X.X tag so avm can install the latest?

EDIT: You can use the latest upstream cli built from source in your CI/CD for now:

# Optional --rev <SHA> to freeze source
cargo install --git https://github.com/project-serum/anchor anchor-cli --locked --force

Great to hear 👍! I'll look for the fix because I'm curious to see what the problem was. Do we know the commit/pr? Because it should link to this issue for people to see.

@RuvendeGroote
Copy link
Contributor Author

It seems like this is still an issue in 0.26.0 🤔

@Henry-E
Copy link

Henry-E commented Jan 20, 2023

Looks fine to me? Maybe this is a mac thing. Either way, open to checking out any PRs that purport to solve the issue.

root:~/tests/some-name# anchor build
Warning: cargo-build-bpf is deprecated. Please, use cargo-build-sbf
cargo-build-bpf child: /root/.local/share/solana/install/active_release/bin/cargo-build-sbf --arch bpf
error: expected one of `!` or `::`, found `<eof>`
  --> programs/some-name/src/lib.rs:16:1
   |
16 | mistake
   | ^^^^^^^ expected one of `!` or `::`

error: could not compile `some-name` due to previous error

root:~/tests/some-name# echo $?
1

@RuvendeGroote
Copy link
Contributor Author

image
Maybe it's a mac thing indeed. Will try to find time if I can (or someone else can dive in of course)

@RuvendeGroote
Copy link
Contributor Author

I have some further information:

If I check out the commit of the 0.26.0 tag and build the cli, I'm able to use the executable in target to build the broken anchor program and have it return the correct status.

If I run anchor 0.26.0 installed through avm I have the wrong exit status.

I reinstalled 0.26.0 through avm and noticed:
Compiling anchor-cli v0.26.0 (/Users/ruvensalamon/.cargo/git/checkouts/anchor-bf03d42499b9267c/347c225/cli)
which seems to be correct so not sure where the difference lies right now.

@RuvendeGroote
Copy link
Contributor Author

Looks fine to me? Maybe this is a mac thing. Either way, open to checking out any PRs that purport to solve the issue.

root:~/tests/some-name# anchor build
Warning: cargo-build-bpf is deprecated. Please, use cargo-build-sbf
cargo-build-bpf child: /root/.local/share/solana/install/active_release/bin/cargo-build-sbf --arch bpf
error: expected one of `!` or `::`, found `<eof>`
  --> programs/some-name/src/lib.rs:16:1
   |
16 | mistake
   | ^^^^^^^ expected one of `!` or `::`

error: could not compile `some-name` due to previous error

root:~/tests/some-name# echo $?
1

PR opened!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants