I first stumbled over this line of code and was thinking "why is there a list of x-pack options"? I now get that it either has 1 or 2 entries for with and without x-pack. I wonder if a better name for the slice would be buildOptions or similar??

What about seeting this line above b.Xpack check and then only have an if that overwrites it? Would be more similar to what you do with buildCmd. Outcome is the same.

What about .exe for Windows.

In order to omit any XPack knowledge from the CrossBuild function and the crossBuildParams struct, I suggest creating a separate build target like crossBuildXPack in the filebeat/magefile.go that invokes a new function mage.CrossBuildXPack() that is doing CrossBuild(WithTarget("buildXPack"), InDir("x-pack", BeatName)).

And then add crossBuildXPack as a dependency of package.

Thank you for your input @andrewkroh, I pushed a new change, let me know if this is looking better now. Once we are settled with how this should work I will extend it to all beats + tackle testing

If you do add some kind of target/work dir parameter to GolangCrossBuilder then I think you can use filepath.Rel here to get a relative path from the current working directory to the target dir.

comment on exported function CrossBuildXPack should be of the form "CrossBuildXPack ..."

You are the lucky person who adds the first Elastic licensed Go code to the project. This code needs to have the Elastic license header and we need to figure out a way to enforce these checks.

My preferred way would be to make https://github.com/elastic/go-licenser handle more license types.

And I assume that the top-level make check fails because these files do not have Apache headers (I didn't check). That can be addressed by merging elastic/go-licenser#14 and using the new -exclude flag.

I opened elastic/go-licenser#15 to handle this, will update this PR once that's merged

Thanks for adding this @exekias I just created #7837 which leverage your work :)

Personally I use beatsfmt. This one searches for a .go_license_header file relative to the source file to be formatted.

How about:

// CrossBuildXPack executes the 'golangCrossBuild' target in the Beat's
// associated x-pack directory to produce a version of the Beat that contains
// Elastic licensed content.

I think it's important to document that the directory is automatically suffixed with the current sub-directory path relative to the repo root. I wasn't expecting this.

I was expecting to the see it used like InDir(filepath.Join("x-pack", BeatName)). Or make the signature be InDir(path ...string) and automatically invoke filepath.Join(path...) in the implementation.

I can do that, but I don't have the BeatName in CrossBuild, it's just using the current subidr. In filebeat/magefile.go I could pass InDir("filebeat") for oss build and InDir("x-pack", "filebeat") in the xpack. WDYT?

I think there is a variable named BeatName that is defined in settings.go that you can use. Does that work?

Got it, I updated this again, I think I finally understood what you meant :)

LGTM

No need to exclude vendor or .git its in the default from go licenser.

https://github.com/elastic/go-licenser/blob/02324788cd84244b695b57aec2a4c388ea2d8fc8/main.go#L92

This magefile change is missing for the other beats.

s/uses/use/

How come this was changed? Won't this be chowning files unnecessarily?

I've changed the logic of DockerChown to not recursively walk up the path, instead it walk down the path, using only params.outputDir will change the permission bellow that directory instead. The other implementation had issues with using absolute paths.

I will revert it back, it will work with both logic.