Makefile (golang make) throw an error with multi path in go env

[cyb0225]

What happened:

When I tried to follow this documentation, some of commands do not work well in my computer.

When I use make format, I got the following error logs.

make[1]: Entering directory '/home/cyb/project/ospp/layotto'
===========> Running go codes format
gofmt -s -w .
/home/cyb/go:/mnt/e/env/gopath/bin/goimports -w -local mosn.io/layotto .
/bin/bash: /home/cyb/go:/mnt/e/env/gopath/bin/goimports: No such file or directory
make[1]: *** [make/golang.mk:187: go.format] Error 127
make[1]: Leaving directory '/home/cyb/project/ospp/layotto'
make: *** [Makefile:31: _run] Error 2

And I found the source code in Makefile.

GO := go
GO_FMT := gofmt
GO_IMPORTS := goimports

GO_MODULE := mosn.io/layotto
VERSION_PACKAGE := main
BINARY_PREFIX := layotto

GOPATH := $(shell go env GOPATH)
ifeq ($(origin GOBIN), undefined)
	GOBIN := $(GOPATH)/bin
endif

go.format: go.format.verify
	@echo "===========> Running go codes format"
	$(GO_FMT) -s -w .
	$(GOPATH)/bin/$(GO_IMPORTS) -w -local $(GO_MODULE) .
	$(GO) mod tidy
	cd components && $(GO) mod tidy
	cd demo && $(GO) mod tidy
	cd sdk/go-sdk && $(GO) mod tidy
	cd spec && $(GO) mod tidy

I have noticed that the problem is probably caused by the direct use of the full path of go env in the Makefile.
So I manually executed the commands in the Makefile and found that they can run successfully.

cyb@DESKTOP-6AOA5PJ:~/project/ospp/layotto$ go env GOPATH
/home/cyb/go:/mnt/e/env/gopath

cyb@DESKTOP-6AOA5PJ:~/project/ospp/layotto$ /home/cyb/go/bin/goimports -w -local mosn.io/layotto .
cyb@DESKTOP-6AOA5PJ:~/project/ospp/layotto$ goimports -w -local mosn.io/layotto .

What you expected to happen:

Why I have more than one golang env path?
One is the installation of Golang on WSL that I set up in the configuration file, and the other is the path of Goland, which is for using the version control of Goland conveniently on WSL.
Therefore, I think it is necessary to consider multiple path scenarios in the Makefile since go env already supports it by default.

How to reproduce it (as minimally and precisely as possible):

I think we may choose the first one to avoid this problem.
Here is my sulotion.

cyb@DESKTOP-6AOA5PJ:~/project/ospp/layotto$ go env GOPATH | cut -d ':' -f 1
/home/cyb/go

So the source code should be change into the following one.

layotto/make/golang.mk

Line 31 in 7abad2f

GOPATH := $(shell go env GOPATH)

GOPATH := $(shell go env GOPATH | cut -d ':' -f 1)

I modified the Makefile without authorization and tested it with "make format", which passed the test successfully.

cyb@DESKTOP-6AOA5PJ:~/project/ospp/layotto$ make format
make[1]: Entering directory '/home/cyb/project/ospp/layotto'
===========> Running go codes format
gofmt -s -w .
/home/cyb/go/bin/goimports -w -local mosn.io/layotto .
go mod tidy
cd components && go mod tidy
cd demo && go mod tidy
cd sdk/go-sdk && go mod tidy
go: downloading google.golang.org/protobuf v1.26.0-rc.1
go: downloading golang.org/x/net v0.0.0-20190311183353-d8887717615a
go: downloading golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a
cd spec && go mod tidy
make[1]: Leaving directory '/home/cyb/project/ospp/layotto'

Anything else we need to know?:

I think that Is it true that the docs documentation is not consistent with the Layotto Makefile version.
I can not found the command of make check and ``in make help, and I got the following logs when I use it.

cyb@DESKTOP-6AOA5PJ:~/project/ospp/layotto$ make check
make[1]: Entering directory '/home/cyb/project/ospp/layotto'
make[1]: *** No rule to make target 'check'.  Stop.
make[1]: Leaving directory '/home/cyb/project/ospp/layotto'
make: *** [Makefile:31: _run] Error 2

I am not entirely sure if make all is a built-in command in Makefile. When I tried to use it, I also encountered the issue of the target not existing, but I can still use the make command normally.

cyb@DESKTOP-6AOA5PJ:~/project/ospp/layotto$ make all
make[1]: Entering directory '/home/cyb/project/ospp/layotto'
make[1]: *** No rule to make target 'all'.  Stop.
make[1]: Leaving directory '/home/cyb/project/ospp/layotto'
make: *** [Makefile:31: _run] Error 2

If you also approve of this solution or have another ideas, I would be happy to take on the bug fix and update the documentation.
I sincerely wish for the continuous development of Layotto.

[github-actions]

Hi @cyb0225,
Thanks for opening an issue! 🎉

[cyb0225]

/kind easy

[wenxuwan]

/cc @seeflood

[seeflood]

I'm a shell noob and don't quite understand the problem. @Xunzhuo Could u help review it?
@cyb0225 Could u submit a PR to fix it?

[cyb0225]

@seeflood Of coures.

[cyb0225]

However, I am not sure if you have similar issues when running make check and make all.

[seeflood]

There's no rule for make check and make all. I think the doc is not consistent

[cyb0225]

OK，I will update the docs as well.

[Xunzhuo]

I think you just need to add goimport in your PATH.

[cyb0225]

I have already add goimport in my PATH. I can call it directly.

cyb@DESKTOP-6AOA5PJ:~/project/ospp/layotto$ go env GOPATH
/home/cyb/go:/mnt/e/env/gopath

cyb@DESKTOP-6AOA5PJ:~/project/ospp/layotto$ /home/cyb/go/bin/goimports -w -local mosn.io/layotto .
cyb@DESKTOP-6AOA5PJ:~/project/ospp/layotto$ goimports -w -local mosn.io/layotto .

And in the go.mk, it uses $(GOPATH)/bin/$(GO_IMPORTS) -w -local $(GO_MODULE) . to call goimport. So I don't think adding goimport in the env PATH can solve this problem.
But I might be able to modify the Makefile to directly call goimport $(GO_IMPORTS) -w -local $(GO_MODULE) .. What do you think about this proposal?
I just tried it and it works, but I'm not sure which approach is better. (Maybe someone haven't add the bin to PATH)
Could you give me some suggestions? Thanks a lot.

[cyb0225]

@seeflood I'm sorry to bother you again. I found that make all is mentioned in the documentation and some issues #561 for local testing, but I cannot use this command and find the corresponding .PHONY in the Makefile.
I don’t know how to perform local tests before submitting the PR without make all.

[seeflood]

@cyb0225 You can use make help to see the options:

try make test and make format

[cyb0225]

I see, thank you very mach !
I will make a pr soon.

[Xunzhuo]

Hey @cyb0225, thanks for raising a PR, things look good to me! Thanks for your contribution : )

[Xunzhuo]

/area tools

[Xunzhuo]

/close

Thanks! @cyb0225. I wrote the robot to manage our issues and PRs. These are the commands it provided https://github.com/mosn/layotto/blob/main/.github/workflows/kube-bot.yaml#L34.

Different roles have different privileges to use cmds. Hope you can grow in the community and have more priviledges to ask the bot to do more things : )

Current cmds you can use:

I0310 15:40:32.922700    1900 engine.go:222] Available commands for @cyb0225:
[close reopen good-first-issue help-wanted kind uncc retest cc assign unassign remove-kind]

Have fun : )

[cyb0225]

Thanks! Good useful tools !