Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion pkg/aflow/action/kernel/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ func BuildKernel(buildDir, srcDir, cfg string, cleanup bool) error {
const compileCommands = "compile_commands.json"
makeArgs = append(makeArgs, "-s", path.Base(image), compileCommands)
if _, err := osutil.RunCmd(time.Hour, srcDir, "make", makeArgs...); err != nil {
return aflow.FlowError(err)
buildErr := build.ExtractRootCause(err, targets.Linux, srcDir)
if buildErr == err {
return aflow.FlowError(err)
}
return aflow.FlowError(fmt.Errorf("%w\n\nRoot cause:\n%w", err, buildErr))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this won't work. The problem is that we truncate errors, see #6896.
At the very least we should print buildErr first, and maybe take the last part of err.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

But the current err should actually be quite small, right?
It doesn't include the full output:

func (err *VerboseError) Error() string {
return err.Err.Error()
}

ExtractRootCause's output should also be reasonably small:

func (err *KernelError) Error() string {
return string(err.Report)
}

I think it should fit even without truncation.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We may prepend the full output later b/c what you on the dashboard is not the short output, it's full output.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In the context of #6896, we'd probably want to keep what we keep in Error reasonably short (= do not keep full logs there) for display in the trajectory table and separately store the full log separately (and make it available via some link).

}
if !cleanup {
return nil
Expand Down
4 changes: 2 additions & 2 deletions pkg/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func Image(params Params) (details ImageDetails, err error) {
} // Try to preserve the build error otherwise.
}
if err != nil {
err = extractRootCause(err, params.TargetOS, params.KernelDir)
err = ExtractRootCause(err, params.TargetOS, params.KernelDir)
return
}
if key := filepath.Join(params.OutputDir, "key"); osutil.IsExist(key) {
Expand Down Expand Up @@ -220,7 +220,7 @@ func compilerIdentity(compiler string) (string, error) {
return "", fmt.Errorf("no output from compiler --version")
}

func extractRootCause(err error, OS, kernelSrc string) error {
func ExtractRootCause(err error, OS, kernelSrc string) error {
if err == nil {
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/build/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func TestExtractRootCause(t *testing.T) {
}
}
t.Run(fmt.Sprint(i), func(t *testing.T) {
got := extractRootCause(err, targetOs, test.src)
got := ExtractRootCause(err, targetOs, test.src)
if got != nil {
assert.Equal(t, expected, got)
}
Expand Down
Loading