Incorrect error message shown when cross-compile dependencies not installed #704

jose-fully-ported · 2023-10-26T16:16:52Z

I was trying to run tests for the nodejs buildpack (to ensure the behavior around alternative package manager selection is tested, since it appears to have changed in the past two days) but am encountering an error:

# install rust
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh

# clone the buildpack repo
git clone https://github.com/heroku/buildpacks-nodejs.git heroku/buildpacks-nodejs

# setup rust for the buildpack
cd heroku/buildpacks-nodejs
rustup update
rustup target add x86_64-unknown-linux-musl

# change to the buildpack in question
cd buildpacks/nodejs-npm-install

# attempt to run tests
INTEGRATION_TEST_CNB_BUILDER=heroku/builder:22 cargo test --locked -- --ignored --test-threads 16

Output

warning: some crates are on edition 2021 which defaults to `resolver = "2"`, but virtual workspaces default to `resolver = "1"`
note: to keep the current resolver, specify `workspace.resolver = "1"` in the workspace root's manifest
note: to use the edition 2021 resolver, specify `workspace.resolver = "2"` in the workspace root's manifest
    Finished test [unoptimized + debuginfo] target(s) in 0.36s
     Running unittests src/main.rs (/Users/josediazgonzalez/go/src/github.com/heroku/buildpacks-nodejs/target/debug/deps/heroku_npm_install_buildpack-887ff546bb0ac9e4)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/integration_test.rs (/Users/josediazgonzalez/go/src/github.com/heroku/buildpacks-nodejs/target/debug/deps/integration_test-ecd4a784e3c3adea)

running 6 tests
test test_npm_install_with_lockfile ... FAILED
test test_npm_install_caching ... FAILED
test test_npm_install_new_package ... FAILED
test test_npm_start_script_creates_a_web_process_launcher ... FAILED
test test_npm_build_scripts ... FAILED
test test_npm_build_scripts_prefers_heroku_build_over_build ... FAILED

failures:

---- test_npm_install_with_lockfile stdout ----
thread 'test_npm_install_with_lockfile' panicked at /Users/josediazgonzalez/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libcnb-test-0.15.0/src/test_runner.rs:121:42:
Test references buildpack `heroku/nodejs`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences

---- test_npm_install_caching stdout ----
thread 'test_npm_install_caching' panicked at /Users/josediazgonzalez/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libcnb-test-0.15.0/src/test_runner.rs:121:42:
Test references buildpack `heroku/nodejs`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences

---- test_npm_install_new_package stdout ----
thread 'test_npm_install_new_package' panicked at /Users/josediazgonzalez/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libcnb-test-0.15.0/src/test_runner.rs:121:42:
Test references buildpack `heroku/nodejs`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- test_npm_start_script_creates_a_web_process_launcher stdout ----
thread 'test_npm_start_script_creates_a_web_process_launcher' panicked at /Users/josediazgonzalez/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libcnb-test-0.15.0/src/test_runner.rs:121:42:
Test references buildpack `heroku/nodejs`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences

---- test_npm_build_scripts stdout ----
thread 'test_npm_build_scripts' panicked at /Users/josediazgonzalez/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libcnb-test-0.15.0/src/test_runner.rs:121:42:
Test references buildpack `heroku/nodejs`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences

---- test_npm_build_scripts_prefers_heroku_build_over_build stdout ----
thread 'test_npm_build_scripts_prefers_heroku_build_over_build' panicked at /Users/josediazgonzalez/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libcnb-test-0.15.0/src/test_runner.rs:121:42:
Test references buildpack `heroku/nodejs`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences


failures:
    test_npm_build_scripts
    test_npm_build_scripts_prefers_heroku_build_over_build
    test_npm_install_caching
    test_npm_install_new_package
    test_npm_install_with_lockfile
    test_npm_start_script_creates_a_web_process_launcher

test result: FAILED. 0 passed; 6 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

error: test failed, to rerun pass `--test integration_test`

Of note, I'm on MacOS Venture 13.6 on a Macbook Air M2, so maybe its an architecture thing?

The text was updated successfully, but these errors were encountered:

edmorley · 2023-10-26T16:29:52Z

@jose-fully-ported Thank you for filing an issue.

When I run the tests locally I don't get that error message (macOS 14.0; M1).

I notice in the STR, there's no mention of brew installing the cross-compilation requirements. If they are missing, then the framework should tell you to install them - or at least it used to. Perhaps this is an error message UX regression after the refactoring in #666 or one of the other similar PRs?

If you run cargo install libcnb-cargo (to install the libcnb cargo plugin) and then run cargo libcnb package from the buildpacks/nodejs-npm-install directory, do you get the instructions about installing the needed requirements? (The test framework performs the same packaging when running the tests, but packaging manually using the CLI outside the tests will make it easier for us to debug.)

If not, then for now could you follow these steps and see if that allows the tests to succeed?

libcnb.rs/libcnb-package/src/cross_compile.rs

Lines 21 to 28 in cdd2eb4

    
                           r"For cross-compilation from macOS to x86_64-unknown-linux-musl, a C compiler and 
        
           linker for the target platform must be installed on your computer. 
        
           The easiest way to install the required cross-compilation toolchain is to run: 
        
           brew install messense/macos-cross-toolchains/x86_64-unknown-linux-musl 
        
           For more information, see: 
        
           https://github.com/messense/homebrew-macos-cross-toolchains",

jose-fully-ported · 2023-10-26T19:12:36Z

So I ran the two commands and got that error from cross_compile.r.

% cargo install libcnb-cargo
    Updating crates.io index
  Downloaded libcnb-cargo v0.15.0
  Downloaded 1 crate (11.0 KB) in 0.80s
  Installing libcnb-cargo v0.15.0
    Updating crates.io index
  Downloaded toml_edit v0.20.5
  Downloaded toml v0.8.5
  Downloaded heck v0.4.1
  Downloaded clap_derive v4.4.7
  Downloaded serde_derive v1.0.190
  Downloaded anstyle v1.0.4
  Downloaded serde v1.0.190
  Downloaded clap_lex v0.6.0
  Downloaded clap_builder v4.4.7
  Downloaded clap v4.4.7
  Downloaded 10 crates (566.3 KB) in 3.72s
   Compiling proc-macro2 v1.0.69
   Compiling unicode-ident v1.0.12
   Compiling serde v1.0.190
   Compiling thiserror v1.0.50
   Compiling camino v1.1.6
   Compiling serde_json v1.0.107
   Compiling semver v1.0.20
   Compiling memchr v2.6.4
   Compiling hashbrown v0.14.2
   Compiling aho-corasick v1.1.2
   Compiling equivalent v1.0.1
   Compiling regex-syntax v0.8.2
   Compiling indexmap v2.0.2
   Compiling libc v0.2.149
   Compiling quote v1.0.33
   Compiling itoa v1.0.9
   Compiling bit-vec v0.6.3
   Compiling syn v2.0.38
   Compiling ryu v1.0.15
   Compiling winnow v0.5.17
   Compiling fnv v1.0.7
   Compiling bit-set v0.5.3
   Compiling lazy_static v1.4.0
   Compiling regex-automata v0.4.3
   Compiling rustix v0.38.20
   Compiling errno v0.3.5
   Compiling regex v1.10.2
   Compiling bstr v1.7.0
   Compiling fancy-regex v0.11.0
   Compiling once_cell v1.18.0
   Compiling cfg-if v1.0.0
   Compiling log v0.4.20
   Compiling bitflags v2.4.1
   Compiling same-file v1.0.6
   Compiling thread_local v1.1.7
   Compiling walkdir v2.4.0
   Compiling uriparse v0.6.4
   Compiling serde_derive v1.0.190
   Compiling thiserror-impl v1.0.50
   Compiling globset v0.4.13
   Compiling heck v0.4.1
   Compiling home v0.5.5
   Compiling fixedbitset v0.4.2
   Compiling clap_lex v0.6.0
   Compiling anstyle v1.0.4
   Compiling either v1.9.0
   Compiling clap_builder v4.4.7
   Compiling which v4.4.2
   Compiling petgraph v0.6.4
   Compiling clap_derive v4.4.7
   Compiling ignore v0.4.20
   Compiling pathdiff v0.2.1
   Compiling clap v4.4.7
   Compiling serde_spanned v0.6.4
   Compiling toml_datetime v0.6.5
   Compiling toml_edit v0.20.5
   Compiling cargo-platform v0.1.4
   Compiling cargo_metadata v0.18.1
   Compiling toml v0.8.5
   Compiling libcnb-proc-macros v0.15.0
   Compiling libcnb-data v0.15.0
   Compiling libcnb-common v0.15.0
   Compiling libcnb-package v0.15.0
   Compiling libcnb-cargo v0.15.0
    Finished release [optimized] target(s) in 24.01s
  Installing /Users/josediazgonzalez/.cargo/bin/cargo-libcnb
   Installed package `libcnb-cargo v0.15.0` (executable `cargo-libcnb`)
josediazgonzalez@Joses-MacBook-Air nodejs-npm-install % cargo libcnb package
🚚 Preparing package directory...
🖥️ Gathering Cargo configuration (for x86_64-unknown-linux-musl)
For cross-compilation from macOS to x86_64-unknown-linux-musl, a C compiler and
linker for the target platform must be installed on your computer.

The easiest way to install the required cross-compilation toolchain is to run:
brew install messense/macos-cross-toolchains/x86_64-unknown-linux-musl

For more information, see:
https://github.com/messense/homebrew-macos-cross-toolchains
❌ Failed to configure Cargo for cross-compilation

I then did the brew install messense/macos-cross-toolchains/x86_64-unknown-linux-musl comamnd and re-ran cargo libcnb package. After this, I could run integration tests (though it took a bit to precompile the test dependencies).

edmorley · 2023-10-26T19:28:10Z

Thank you.

Tomorrow I'll look into why libcnb-test isn't surfacing that more helpful error message - since it should have shown it too.

edmorley · 2023-10-31T09:29:30Z

Ok so this is a regression from #666 in libcnb-test 0.15.0:
b59a9ce#diff-6889e352ea9b53638ed05fa3483ecc6259b1d24abff88b0180be53cafc25d000L108

The previous implementation had to unwrap a temp dir path at one point, and used the ~"but crate wasn't packaged as a buildpack" message for that.

In #666 the code was refactored and the same error message used when unwrapping a completely different return value (the result from packaging).

Before when packaging failed a "Could not package current crate as buildpack" error was returned instead:
b59a9ce#diff-6889e352ea9b53638ed05fa3483ecc6259b1d24abff88b0180be53cafc25d000L97

edmorley · 2023-10-31T09:35:30Z

Ah so there are two parts to the regression:

The error message shown is misleading (the text refers to an unrelated issue) for both packaging the current create, and packaging other crates.
When packaging other crates specifically, the error message is also now missing the debug representation of the underlying error, since that codepath uses panic! rather than .expect() - and when using panic! one has to manually include the underlying error in the message to fully replace the same behaviour as .expect() (because expect is equivalent to panic!("{msg}: {error:?}") not just panic!("{msg}")).

edmorley · 2023-10-31T09:57:32Z

In fact, it seems the regression was actually caught be the tests, which is why they ended up being changed in:
b59a9ce#diff-6a86c81613708c6f5b78432f6f55d3a8de0a6c98ca1babdcb4ae8e683401f8f3L135

The problem is that assert!s and Error return types are being mixed and matched - and the test tested a case which is now handled by an assert lower down the call stack than the now-broken error message.

IMO we should only be using asserts/panics/expects at the runner top level, and not in lower down helper functions.

After #666, incorrect error messages were shown for failures that occurred during buildpack compilation packaging, since the handling in `TestRunner::build_internal` used the wrong message text, plus didn't include the error struct in the `panic!` string. These have been fixed, plus display representations added to all of the new error variants. In addition, the usages of `assert!` and `.unwrap()` in functions that return `Result` have been replaced with new error enum variants. The change in output can be seen in the new tests added by #717. There is one last scenario that doesn't yet have a test - the testing of the cross-compilation output itself. However, I'll add that separately, since it's going to be more complex to test / there are a few different options that we'll need to choose between. Fixes #704. Fixes #709. Fixes #710. GUS-W-14395170.

edmorley · 2023-11-17T19:53:56Z

I've just released libcnb 0.16.0 (which includes the fix for this), and updated the Node.js CNB to use that version.

Now, if I brew uninstall x86_64-unknown-linux-musl (to emulate not having that requirement installed, as was the case for you), I correctly get the help text:

$ cargo test -- --ignored
    Finished test [unoptimized + debuginfo] target(s) in 0.06s
     Running unittests src/main.rs (/Users/emorley/src/buildpacks-nodejs/target/debug/deps/heroku_npm_install_buildpack-c080b2bb78061680)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/integration_test.rs (/Users/emorley/src/buildpacks-nodejs/target/debug/deps/integration_test-0ff1ce4662c8d51e)

running 7 tests
test test_npm_install_with_lockfile ... FAILED
test test_npm_install_caching ... FAILED
test test_npm_start_script_creates_a_web_process_launcher ... FAILED
test test_dependencies_and_missing_lockfile_errors ... FAILED
test test_npm_build_scripts_prefers_heroku_build_over_build ... FAILED
test test_npm_build_scripts ... FAILED
test test_npm_install_new_package ... FAILED

failures:

---- test_npm_install_with_lockfile stdout ----
thread 'test_npm_install_with_lockfile' panicked at /Users/emorley/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libcnb-test-0.16.0/src/test_runner.rs:141:25:
Error packaging buildpack 'heroku/nodejs': Couldn't find cross-compilation toolchain.

For cross-compilation from macOS to x86_64-unknown-linux-musl, a C compiler and
linker for the target platform must be installed on your computer.

The easiest way to install the required cross-compilation toolchain is to run:
brew install messense/macos-cross-toolchains/x86_64-unknown-linux-musl

For more information, see:
https://github.com/messense/homebrew-macos-cross-toolchains

You will also need to install the Rust target, using:
rustup target add x86_64-unknown-linux-musl

...

jose-fully-ported · 2023-11-28T03:24:28Z

<3

edmorley changed the title ~~Failing to run buildpack tests on MacOS Arm~~ Incorrect error message shown when cross-compile dependencies not installed Oct 31, 2023

edmorley added bug Something isn't working libcnb-test labels Oct 31, 2023

edmorley mentioned this issue Oct 31, 2023

Support local buildpacks and meta-buildpacks in libcnb-test #666

Merged

edmorley self-assigned this Oct 31, 2023

edmorley closed this as completed in #720 Nov 7, 2023

edmorley mentioned this issue Nov 9, 2023

Add libcnb-cargo and libcnb-test tests for cross-compile assistence help text #730

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Incorrect error message shown when cross-compile dependencies not installed #704

Incorrect error message shown when cross-compile dependencies not installed #704

jose-fully-ported commented Oct 26, 2023

edmorley commented Oct 26, 2023 •

edited

Loading

jose-fully-ported commented Oct 26, 2023

edmorley commented Oct 26, 2023

edmorley commented Oct 31, 2023

edmorley commented Oct 31, 2023

edmorley commented Oct 31, 2023

edmorley commented Nov 17, 2023

jose-fully-ported commented Nov 28, 2023

Incorrect error message shown when cross-compile dependencies not installed #704

Incorrect error message shown when cross-compile dependencies not installed #704

Comments

jose-fully-ported commented Oct 26, 2023

edmorley commented Oct 26, 2023 • edited Loading

jose-fully-ported commented Oct 26, 2023

edmorley commented Oct 26, 2023

edmorley commented Oct 31, 2023

edmorley commented Oct 31, 2023

edmorley commented Oct 31, 2023

edmorley commented Nov 17, 2023

jose-fully-ported commented Nov 28, 2023

edmorley commented Oct 26, 2023 •

edited

Loading