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

Improve compute build rust compilation error messaging #60

Merged
merged 2 commits into from May 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Expand Up @@ -16,6 +16,8 @@ RELEASE_CHANGELOG.md
**/src
!pkg/compute/testdata/build/src
**/target
rust-toolchain
.cargo

# Binaries for programs and plugins
*.exe
Expand Down
4 changes: 2 additions & 2 deletions pkg/compute/build.go
Expand Up @@ -17,7 +17,7 @@ import (
// Toolchain abstracts a Compute@Edge source language toolchain.
type Toolchain interface {
Verify(out io.Writer) error
Build(out io.Writer) error
Build(out io.Writer, verbose bool) error
}

// GetToolchain returns a Toolchain for the provided language.
Expand Down Expand Up @@ -122,7 +122,7 @@ func (c *BuildCommand) Exec(in io.Reader, out io.Writer) (err error) {

progress.Step(fmt.Sprintf("Building package using %s toolchain...", lang))

if err := toolchain.Build(progress); err != nil {
if err := toolchain.Build(progress, c.Globals.Flag.Verbose); err != nil {
return err
}

Expand Down
37 changes: 29 additions & 8 deletions pkg/compute/rust.go
Expand Up @@ -153,7 +153,7 @@ func (r Rust) Verify(out io.Writer) error {

// Build implments the Toolchain interface and attempts to compile the package
// Rust source to a Wasm binary.
func (r Rust) Build(out io.Writer) error {
func (r Rust) Build(out io.Writer, verbose bool) error {
// Get binary name from Cargo.toml.
var m CargoManifest
if err := m.Read("Cargo.toml"); err != nil {
Expand All @@ -164,12 +164,27 @@ func (r Rust) Build(out io.Writer) error {
// Specify the toolchain using the `cargo +<version>` syntax.
toolchain := fmt.Sprintf("+%s", RustToolchainVersion)

args := []string{
toolchain,
"build",
"--bin",
binName,
"--release",
"--target",
WasmWasiTarget,
"--color",
"always",
Comment on lines +175 to +176
Copy link
Member

Choose a reason for hiding this comment

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

Not necessary for this PR, but if people start using this tool in CI, they may not want colors.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup I thought about this too. I think we need to do a sweep on the commands in another PR to ensure we don't assume TTY, ANSI etc support.

}
if verbose {
args = append(args, "--verbose")
}

// Call cargo build with Wasm Wasi target and release flags.
// gosec flagged this:
// G204 (CWE-78): Subprocess launched with variable
// Disabling as the variables come from trusted sources.
/* #nosec */
cmd := exec.Command("cargo", toolchain, "build", "--bin", binName, "--release", "--target", WasmWasiTarget)
cmd := exec.Command("cargo", args...)

// Add debuginfo RUSTFLAGS to command environment to ensure DWARF debug
// infomation (such as, source mappings) are compiled into the binary.
Expand Down Expand Up @@ -201,15 +216,21 @@ func (r Rust) Build(out io.Writer) error {
_, errStderr = io.Copy(stderr, stderrIn)
wg.Wait()

// Wait for the command to exit.
if err := cmd.Wait(); err != nil {
return fmt.Errorf("error during compilation process: %w", err)
}
if errStdout != nil {
return fmt.Errorf("error during compilation process: %w", errStdout)
return fmt.Errorf("error streaming stdout output from child process: %w", errStdout)
}
if errStderr != nil {
return fmt.Errorf("error during compilation process: %w", errStderr)
return fmt.Errorf("error streaming stderr output from child process: %w", errStderr)
}

// Wait for the command to exit.
if err := cmd.Wait(); err != nil {
// If we're not in verbose mode return the bufferred stderr output
// from cargo as the error.
if !verbose && stderrBuf.Len() > 0 {
return fmt.Errorf("error during compilation process:\n%s", strings.TrimSpace(stderrBuf.String()))
}
return fmt.Errorf("error during compilation process")
phamann marked this conversation as resolved.
Show resolved Hide resolved
}

// Get working directory.
Expand Down