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

mage:import doesn't use vendored beats on custom beats #13998

Closed
jsoriano opened this issue Oct 10, 2019 · 11 comments · Fixed by #14162
Closed

mage:import doesn't use vendored beats on custom beats #13998

jsoriano opened this issue Oct 10, 2019 · 11 comments · Fixed by #14162
Assignees
Labels
bug :Generator Related to code generators for building custom Beats or modules. Team:Integrations Label for the Integrations team

Comments

@jsoriano
Copy link
Member

jsoriano commented Oct 10, 2019

Since 7.4.0, custom beats reuse common mage targets from beats, for that they make use of mage:import. This is not using vendored beats in the vendor directory, though other go tools do use it. This causes problems if a different version of beats is in the GOPATH, like this one:

# command-line-arguments
./mage_output_file.go:498:5: undefined: common.AddLicenseHeaders
./mage_output_file.go:518:5: undefined: common.CheckLicenseHeaders
Error: error compiling magefiles

It was reported in the mage repository (magefile/mage#244). Migration of Beats to go modules is proposed there as solution.

We will need to look for another solution in the meantime. Or look for an alternative way of reusing targets.

For confirmed bugs, please report:

  • Version: >= 7.4.0
  • Steps to Reproduce: Create a new beat from a version of beats that makes use of mage:import in magefiles, and checkout a different version in the GOPATH.

// cc: @radoondas

@jsoriano jsoriano added bug :Generator Related to code generators for building custom Beats or modules. labels Oct 10, 2019
@fearful-symmetry
Copy link
Contributor

This is not using vendored beats in the vendor directory

Wait, so when mage is building, it will search $GOPATH but not vendor? ...What?

@jsoriano
Copy link
Member Author

Wait, so when mage is building, it will search $GOPATH but not vendor? ...What?

Yes, this seems to be what it is happening, at some step mage seems to ignore the vendor directory for imports marked with mage:import (or something like that).

@fearful-symmetry fearful-symmetry self-assigned this Oct 10, 2019
@fearful-symmetry fearful-symmetry added the Team:Integrations Label for the Integrations team label Oct 10, 2019
@jsoriano
Copy link
Member Author

At some point mage runs go list -f "{{.Dir}}||{{.Name}}" github.com/elastic/beats/dev-tools/mage/target/common, what makes it use the files in GOPATH.

@fearful-symmetry
Copy link
Contributor

When I try to reproduce this, I'm getting a different error: Error parsing magefiles: exit status 1

@fearful-symmetry
Copy link
Contributor

fearful-symmetry commented Oct 10, 2019

Yep:

≻ go list -f '{{.Dir}}||{{.Name}}' github.com/elastic/beats/dev-tools/mage/target/common
can't load package: package github.com/elastic/beats/dev-tools/mage/target/common: cannot find package "github.com/elastic/beats/dev-tools/mage/target/common" in any of:
        /usr/local/Cellar/go/1.12/libexec/src/github.com/elastic/beats/dev-tools/mage/target/common (from $GOROOT)
        /Users/alexk/go/src/github.com/elastic/beats/dev-tools/mage/target/common (from $GOPATH)
≻ go list -f '{{.Dir}}||{{.Name}}' github.com/elastic/beats/dev-tools/mage
/Users/alexk/go/src/github.com/elastic/beats/dev-tools/mage||mage

@fearful-symmetry
Copy link
Contributor

Small update. As @jsoriano and I discovered, the issue is with mage exec'ing go list. The list command doesn't care about the cwd, so it ignores vendor unless you somehow force it to look in the vendor/ directory.

@andresrc
Copy link
Contributor

In the short-term, try to avoid mage:import, so let's document what we should be doing instead. Long-term solution is to wait for the migration to modules.

@andresrc
Copy link
Contributor

@fearful-symmetry can we close this issue by adding some content to the dev guide explaining that we should avoid mage:import for the time being?

@jsoriano
Copy link
Member Author

I would prefer to remove mage:import from the custom beats magefiles, explicitly importing and aliasing targets if needed, and add a note about why we are doing this to the developer's changelog. I could also make these changes.

And we could backport these changes to 7.5 and 7.4, if we do it only the custom beats created from 7.4.0 to 7.4.2 would be affected.

@fearful-symmetry
Copy link
Contributor

Yah, I'm with @jsoriano here. I think removing mage:import from things for the time being is probably the way to go.

@andresrc
Copy link
Contributor

Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug :Generator Related to code generators for building custom Beats or modules. Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants