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

Install tools (mage, goimports, etc.) from vendor folder #15998

Merged
merged 12 commits into from
Feb 6, 2020

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Jan 31, 2020

What does this PR do?

This PR adds a new file tools/tools.go to list the tools we need during checking, building and packaging Beats. This is the suggested approach to list tool dependencies: https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module

The following tools are added:

  • github.com/magefile/mage
  • github.com/pierrre/gotestcover
  • github.com/tsg/go-daemon
  • golang.org/x/tools/cmd/goimports
  • github.com/elastic/go-licenser

Why is it important?

The tools are required to run make check on the CI.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made the corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works

@kvch kvch added [zube]: In Review Team:Services (Deprecated) Label for the former Integrations-Services team labels Jan 31, 2020
tools/tools.go Show resolved Hide resolved
tools/tools.go Show resolved Hide resolved
dev-tools/mage/gotool/go.go Outdated Show resolved Hide resolved
Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Looks good to me, like the including of the tools.

Probably should let someone with a little more experience then me on the project give the +1. I am excited to see this completed, along with the python3 work as well.

@@ -37,6 +37,19 @@ type Args struct {
// ArgOpt is a functional option adding info to Args once executed.
type ArgOpt func(args *Args)

type goInstall func(opts ...ArgOpt) error
Copy link

Choose a reason for hiding this comment

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

I see, we copied gotool from go-txfile to the Beats repo. Btw. go-txfile magefile uses the gotools package (minor improvements + generalization of this approach) from https://github.com/urso/magetools.

@@ -140,7 +141,6 @@ require (
google.golang.org/appengine v1.6.5 // indirect
google.golang.org/genproto v0.0.0-20191115221424-83cc0476cb11
google.golang.org/grpc v1.25.1 // indirect
gopkg.in/goracle.v2 v2.16.3 // indirect
Copy link

Choose a reason for hiding this comment

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

why has this been removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has been removed from master in the meantime: #15683

@urso
Copy link

urso commented Feb 3, 2020

I'm missing at least 'stringer' as developer dependency.

We use go:generate in a few places. Grepping code for go:generate I get this:

licenses/license.go
20://go:generate go run ../dev-tools/cmd/license/license_generate.go

metricbeat/module/windows/service/doc.go
21://go:generate go run ../../../helper/windows/run.go -cmd "go tool cgo -godefs defs_service_windows.go" -goarch amd64 -output defs_service_windows_amd64.go
22://go:generate go run ../../../helper/windows/run.go -cmd "go tool cgo -godefs defs_service_windows.go" -goarch 386 -output defs_service_windows_386.go
23://go:generate go run $GOROOT/src/syscall/mksyscall_windows.go -output zservice_windows.go service_windows.go
24://go:generate gofmt -w defs_service_windows_amd64.go defs_service_windows_386.go

metricbeat/helper/windows/pdh/doc.go
20://go:generate go run mkpdh_defs.go
21://go:generate go run ../run.go -cmd "go tool cgo -godefs defs_pdh_windows.go" -goarch amd64 -output defs_pdh_windows_amd64.go
22://go:generate go run ../run.go -cmd "go tool cgo -godefs defs_pdh_windows.go" -goarch 386 -output defs_pdh_windows_386.go
23://go:generate go run $GOROOT/src/syscall/mksyscall_windows.go -output zpdh_windows.go pdh_windows.go
24://go:generate gofmt -w defs_pdh_windows_amd64.go defs_pdh_windows_386.go zpdh_windows.go

filebeat/input/syslog/input.go
38://go:generate ragel -Z -G2 parser.rl -o parser.go
39://go:generate go fmt parser.go

auditbeat/module/file_integrity/flatbuffers.go
32://go:generate flatc --go schema.fbs

auditbeat/module/file_integrity/security_windows.go
70://go:generate go run $GOROOT/src/syscall/mksyscall_windows.go -output zsecurity_windows.go security_windows.go

winlogbeat/sys/eventlogging/syscall_windows.go
139://go:generate go run $GOROOT/src/syscall/mksyscall_windows.go -output zsyscall_windows.go syscall_windows.go

winlogbeat/sys/wineventlog/syscall_windows.go
303://go:generate go run $GOROOT/src/syscall/mksyscall_windows.go -output zsyscall_windows.go syscall_windows.go

packetbeat/procs/syscall_windows.go
72://go:generate go run $GOROOT/src/syscall/mksyscall_windows.go -output zsyscall_windows.go syscall_windows.go

libbeat/idxmgmt/idxmgmt.go
74://go:generate stringer -linecomment -type LoadMode

libbeat/idxmgmt/std.go
71://go:generate stringer -linecomment -type componentType

libbeat/feature/stability.go
20://go:generate stringer -type=Stability

libbeat/monitoring/monitoring.go
36://go:generate stringer -type=Mode

x-pack/filebeat/input/netflow/decoder/fields/doc.go
7://go:generate go run gen.go -output zfields_ipfix.go -export IpfixFields --column-id=1 --column-name=2 --column-type=3 ipfix-information-elements.csv
8://go:generate go run gen.go -output zfields_cert.go -export CertFields --column-pen=1 --column-id=2 --column-name=3 --column-type=4 cert_pen6871.csv
9://go:generate go run gen.go -output zfields_cisco.go -export CiscoFields --column-pen=2 --column-id=3 --column-name=1 --column-type=4 cisco.csv
10://go:generate go run gen.go -output zfields_assorted.go -export AssortedFields --column-pen=1 --column-id=2 --column-name=3 --column-type=4 assorted.csv
11://go:generate go fmt

x-pack/filebeat/input/netflow/doc.go
7://go:generate go run fields_gen.go -output _meta/fields.yml --column-name=2 --column-type=3 --header _meta/fields.header.yml decoder/fields/ipfix-information-elements.csv

x-pack/filebeat/module/cisco/shared/gen.go
16://go:generate go run gen-ftd-ecs-mapping.go stringset.go -output ecs-mapping-processor.yml security-mappings.csv
17://go:generate go run gen-ecs-mapping-docs.go stringset.go -output ecs-mapping-docs.asciidoc security-mappings.csv

x-pack/filebeat/processors/decode_cef/cef/cef.go
15://go:generate ragel -Z -G1 cef.rl -o parser.go
16://go:generate go fmt parser.go
20://go:generate ragel -V -p cef.rl -o cef.dot
21://go:generate dot -T svg cef.dot -o cef.svg

x-pack/filebeat/build/package/module/cisco/shared/gen.go
16://go:generate go run gen-ftd-ecs-mapping.go stringset.go -output ecs-mapping-processor.yml security-mappings.csv
17://go:generate go run gen-ecs-mapping-docs.go stringset.go -output ecs-mapping-docs.asciidoc security-mappings.csv

x-pack/libbeat/licenser/types.go
10://go:generate stringer -type=LicenseType -linecomment=true
24://go:generate stringer -type=State

x-pack/libbeat/management/api/response.go
13://go:generate stringer -type=LicenseType -linecomment=true

Other dependencies: stringer, ragel, dot, flatc, gofmt.

Looks like we inconsistently use gofmt and go fmt. How about replacing them with goimports?

Not all developer dependencies are go tools. We should document them somewhere. e.g. in the readme?

@kvch kvch requested a review from a team as a code owner February 4, 2020 18:17
@kvch
Copy link
Contributor Author

kvch commented Feb 4, 2020

@urso I have replaced gofmt and go fmt with goimports.

I am wondering if we need to add dot, ragel and flatc needs to be versioned at all. Both dot and ragel are supposed to be installed using apt/homebrew/yum/whatever. Are we currently tracking the version of such non-Go tools? If not, do we want to?
flatc is supposed to be compiled if my research was correct. The questions above apply to this case as well.

@urso
Copy link

urso commented Feb 4, 2020

We should at least mention the versions that have been used somehwere. If generated files contain version info in the file header this might be good enough. These generated files are not updated often and newer version of developer tools might introduce breaking changes, that you only recognize after a long time. In this case it is nice to know which version was working last in case you need to fix something ASAP.

@kvch
Copy link
Contributor Author

kvch commented Feb 5, 2020

I am adding the required tools and versions to our contribution guide: https://www.elastic.co/guide/en/beats/devguide/current/beats-contributing.html

@kvch
Copy link
Contributor Author

kvch commented Feb 5, 2020

I have opened a PR against master with the documentation changes. Now I am movint the PR to "In review" again.

kvch added a commit that referenced this pull request Feb 6, 2020
## What does this PR do?

This PR adds information about tool dependencies to the contribution guide.

## Why is it important?

Previously we did not specify what tools were required to run `go generate` in the repo.

## Related issues

#15998
@kvch kvch mentioned this pull request Feb 6, 2020
2 tasks
@kvch kvch merged commit a8cf29f into elastic:go-modules Feb 6, 2020
kvch added a commit that referenced this pull request Feb 26, 2020
## What does this PR do?

This PR adds a new file `tools/tools.go` to list the tools we need during checking, building and packaging Beats. This is the suggested approach to list tool dependencies: https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module

The following tools are added:
- `github.com/magefile/mage`
- `github.com/pierrre/gotestcover`
- `github.com/tsg/go-daemon`
- `golang.org/x/tools/cmd/goimports`
- `github.com/elastic/go-licenser`

## Why is it important?

The tools are required to run `make check` on the CI.
kvch added a commit that referenced this pull request Feb 27, 2020
## What does this PR do?

This PR adds a new file `tools/tools.go` to list the tools we need during checking, building and packaging Beats. This is the suggested approach to list tool dependencies: https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module

The following tools are added:
- `github.com/magefile/mage`
- `github.com/pierrre/gotestcover`
- `github.com/tsg/go-daemon`
- `golang.org/x/tools/cmd/goimports`
- `github.com/elastic/go-licenser`

## Why is it important?

The tools are required to run `make check` on the CI.
kvch added a commit that referenced this pull request Feb 28, 2020
## What does this PR do?

This PR adds a new file `tools/tools.go` to list the tools we need during checking, building and packaging Beats. This is the suggested approach to list tool dependencies: https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module

The following tools are added:
- `github.com/magefile/mage`
- `github.com/pierrre/gotestcover`
- `github.com/tsg/go-daemon`
- `golang.org/x/tools/cmd/goimports`
- `github.com/elastic/go-licenser`

## Why is it important?

The tools are required to run `make check` on the CI.
kvch added a commit that referenced this pull request Mar 2, 2020
## What does this PR do?

This PR adds a new file `tools/tools.go` to list the tools we need during checking, building and packaging Beats. This is the suggested approach to list tool dependencies: https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module

The following tools are added:
- `github.com/magefile/mage`
- `github.com/pierrre/gotestcover`
- `github.com/tsg/go-daemon`
- `golang.org/x/tools/cmd/goimports`
- `github.com/elastic/go-licenser`

## Why is it important?

The tools are required to run `make check` on the CI.
kvch added a commit that referenced this pull request Mar 3, 2020
## What does this PR do?

This PR adds a new file `tools/tools.go` to list the tools we need during checking, building and packaging Beats. This is the suggested approach to list tool dependencies: https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module

The following tools are added:
- `github.com/magefile/mage`
- `github.com/pierrre/gotestcover`
- `github.com/tsg/go-daemon`
- `golang.org/x/tools/cmd/goimports`
- `github.com/elastic/go-licenser`

## Why is it important?

The tools are required to run `make check` on the CI.
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
## What does this PR do?

This PR adds a new file `tools/tools.go` to list the tools we need during checking, building and packaging Beats. This is the suggested approach to list tool dependencies: https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module

The following tools are added:
- `github.com/magefile/mage`
- `github.com/pierrre/gotestcover`
- `github.com/tsg/go-daemon`
- `golang.org/x/tools/cmd/goimports`
- `github.com/elastic/go-licenser`

## Why is it important?

The tools are required to run `make check` on the CI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Services (Deprecated) Label for the former Integrations-Services team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants