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

Bugfix: buildFlags="-C directory" is invalid #3380

Closed
wants to merge 1 commit into from

Conversation

saffraan
Copy link

When set buildFlags="-C dir", go build failure. Because the "-C direcotry" option is filled after "go build -o xxx", for example:

go build -o main -C /root/main

but the "-C dir" option should be filled after "go build":

go build -C /root/main -o main

in go version 1.20.

When set buildFlags="-C dir", go build failure. Because the "-C direcotry" option is filled after "go build -o xxx", for example:

go build -o main -C /root/main

but the "-C dir" option should be filled after "go build":

go build -C /root/main -o main

in go version 1.20.
@aarzilli
Copy link
Member

I've tried it both ways (go build -o something -C somethingelse and go build -C somethingelse -o something) and they seem to both do the exact same thing. What is the build failure exactly?

@saffraan
Copy link
Author

I've tried it both ways (go build -o something -C somethingelse and go build -C somethingelse -o something) and they seem to both do the exact same thing. What is the build failure exactly?

(base) ➜ /tmp cd go-example
(base) ➜ go-example ls
go.mod main.go
(base) ➜ go-example cat main.go
package main

import "fmt"

func main(){
fmt.Println("Hello word!")
}
(base) ➜ go-example cat go.mod
module go-example

go 1.20
(base) ➜ go-example cd ..
(base) ➜ /tmp go build -gcflags all="-N -l" -C /tmp/go-example -o main
go: go.mod file not found in current directory or any parent directory; see 'go help modules'
(base) ➜ /tmp go build -C /tmp/go-example -gcflags all="-N -l" -o main
(base) ➜ /tmp ls /tmp/go-example/
go.mod main main.go
(base) ➜ /tmp

@saffraan
Copy link
Author

I've tried it both ways (go build -o something -C somethingelse and go build -C somethingelse -o something) and they seem to both do the exact same thing. What is the build failure exactly?

sorry, I found the bug in debug mode, I haven't try to run go build just with -C optional.

@aarzilli
Copy link
Member

Looks like a bug in cmd/go, let's see.

@saffraan
Copy link
Author

Looks like a bug in cmd/go, let's see.

Yes, this is a bug of golang, if fixed, the pr can be closed.

@aarzilli
Copy link
Member

According to the release notes for 1.21 it looks like -C needs to be the first flag passed:

 The -C dir flag must now be the first flag on the command-line when used. 

so we should do something about this. However I don't like the approach in this PR, I think it's too complicated and we should just change goBuildArgs to start the args slice with the buildflags string:

func goBuildArgs(debugname string, pkgs []string, buildflags string, isTest bool) []string {
	args := []string{}
	if buildflags != "" {
		args = append(args, config.SplitQuotedFields(buildflags, '\'')...)
	}
	args = append(args, "-o", debugname)
	...

aarzilli added a commit to aarzilli/delve that referenced this pull request Aug 8, 2023
The -C argument must come first on the command line of 'go build' so
pass the flags specified by build-flags first.

Replaces go-delve#3380
aarzilli added a commit to aarzilli/delve that referenced this pull request Aug 8, 2023
The -C argument must come first on the command line of 'go build' so
pass the flags specified by build-flags first.

Replaces go-delve#3380
aarzilli added a commit to aarzilli/delve that referenced this pull request Aug 10, 2023
The -C argument must come first on the command line of 'go build' if
the flags specified by the user via  build-flags start with -C pass it
first.

Replaces go-delve#3380
derekparker pushed a commit that referenced this pull request Aug 14, 2023
The -C argument must come first on the command line of 'go build' if
the flags specified by the user via  build-flags start with -C pass it
first.

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

Successfully merging this pull request may close these issues.

4 participants