Autobuilder: fall back when os.Executable fails #383
Conversation
This can happen under tracing, perhaps because of github/codeql-tracer#29
| case "windows": | ||
| return "win64", nil | ||
| } | ||
| return "", errors.New("Unknown OS: " + runtime.GOOS) |
There was a problem hiding this comment.
Personally I like the fmt.Errorf more than errors.New, but that's entirely a style thing. Also, I think "unsupported" is more accurate here.
| return "", errors.New("Unknown OS: " + runtime.GOOS) | |
| return "", fmt.Errorf("Unsupported OS: %s", runtime.GOOS) |
| // Fall back to rebuilding our own path from the extractor root: | ||
| extractorRoot := os.Getenv("CODEQL_EXTRACTOR_GO_ROOT") | ||
| if extractorRoot == "" { | ||
| return "", errors.New("CODEQL_EXTRACTOR_GO_ROOT not set") |
There was a problem hiding this comment.
We should probably have a similar message here as when CODEQL_EXTRACTOR_GO_SOURCE_ARCHIVE_DIR is unset below.
|
I'll improve the message in a followup since time it pressing |
| @@ -206,6 +207,51 @@ func checkVendor() bool { | |||
| return true | |||
| } | |||
|
|
|||
| func getOsToolsSubdir() (string, error) { | |||
There was a problem hiding this comment.
This is known asos.Getenv("CODEQL_PLATFORM")
There was a problem hiding this comment.
Yeah, but that was being filtered out, presumably also by the tracer. It was known to autobuild.sh but not its child go-autobuild.
There was a problem hiding this comment.
@matt-gretton-dann The tracer shouldn't filter out CODEQL_ variables, right?
There was a problem hiding this comment.
The tracer has a concept of critical variables which it tries to preserve in the environment. But also when the tracer is turned off (as is the case when we trace go-autouild) we remove those critical variables. I believe this was historically deemed appropriate to try and preserve the environment the user would expect
CODEQL_PLATFORM is one of those critical env. vars.
This can happen under tracing, perhaps because of https://github.com/github/codeql-tracer/issues/29