Override targets from imported Magefile

[anonfunc]

An example would be importing a package with a known Build and Deploy, and overriding the Deploy to add additional logic.

Right now, I do this by removing the "// mage: import" comment and adding a bunch of new functions like:

// Cleans known output paths.
func Clean() error {
    mg.Deps(imported.Clean)
    return nil
}

Ideally, my Clean would take priority over an imported Clean.

I haven't dug too much into the internals, my apologies if this is a crazy request.

[natefinch]

That's not a crazy request, but I'll need to think about it. The problem with overriding is that then it can get confusing which target you're running.

What might work better is to give the function in the imported package a signature that won't be treated as a target, and then your Clean target can just call it like normal code. ... what might be useful in that case is a way to mark a function as explicitly not something you want treated as a target so you can give it a signature that still looks like a mage target. Like

// mage:noexport
func Clean() error {

}

That assumes that it's ok to make everyone who imports the package write their own Clean target, which could then call this Clean function in the imported library. If you want some people to get it imported and some not, then things get tricky, and overriding would work better.... but like I said, I'd need to think on that.

The other option, of course, is not to call it Clean, call it CleanFiles or CleanBase or something, and then write a Clean target in the importing magefile. That'll give you two targets when you run mage -l but that might not be the end of the world. But if you need to override a lot of imports, that gets annoying, too.

Sorry for the stream of consciousness replies... I don't know exactly your use case, so I'm trying to get as many ideas out there as possible, in case one of them can be useful to you.

[anonfunc]

Here's my use case, more explicitly: I've got a variety of repos which are mostly Go projects with a standard layout and the same deployment command. There are a few exceptions: one repo might have several related deployments, one repo might want to run something after the deployment, etc.

Previously, we were copying a Makefile around and editing it for the exceptional projects, but I decided that Mage looks perfect for this: put the common build logic in a package, import it into each repo via the mage:import comments, everything builds the same way, build maintainers are happy. (Mage also looks great for a bunch of other reasons, but this was a big reason.)

This leads to finding the "right" solution for the exceptions. So far, we've considered (or implemented) the following:

• More granular target packages, so we can say "import build and test, but not deploy"
• Importing the target packages and delegating (snippet above)
• Not importing the targets, but using reusable functions, putting the same target functions everywhere. Lots of copy-pasting here from repo to repo, but it's not terrible. Most of the imported targets are thin wrappers around functions like this, to support this pattern.
• Importing the targets, but having our own Clean() or Deploy() target functions which take priority. This is the missing feature tracked by this issue, and would be my preferred solution. There are definitely big caveats here: the dependency graph of the imported targets won't know about the overridden target, and thus won't call it. However, my target use case is for "leaf" targets which aren't depended on and I'm perfectly okay with ignoring all of the ramifications. 😁

The fun bit is that I tried defining a Clean(), and thought it was going to work because mage silently runs the imported target, not the locally defined one. (If you decide that overriding should be unsupported, making that an error case would be nice.)

[natefinch]

The dependency graph of the imported targets won't know about the overridden target, and thus won't call it.

This is a good point and worth keeping in mind.

mage silently runs the imported target, not the locally defined one

Oh, that's a bug. There's code to check for duplicates and error out, but evidently it's not working correctly with imported targets.

[ihgann]

There is a bit of a hacky way around this, but I've done this in my own projects in the meantime.

In your targets Magefile, you can specify some primary args:

var (
    BuildFn = build
)

func Build(ctx context.Context) error {
    return BuildFn(ctx)
}

func build(ctx context.Context) {
    // Your logic here
}

Then, in your magefile where you are importing:

import (
    // mage:import
    "....../targets.go" // Your targets file
)

func init() {
    targets.BuildFn = overrideBuild
}

func overrideBuild(ctx context.Context) error {
    // Your overridden logic here
}

This is very ugly, I acknowledge, but it is a workaround until this is native. In my opinion, an option to specify targets within // mage:import would be nice.

[anonfunc]

@ihgann, thank you! This is a good workaround.

[bjorndm]

For our use case a //mage:noexport magic comment would be very useful.

[viktorvoltaire]

I created a PR https://github.com/magefile/mage/pull/380 that adds the ability to // mage:skip on functions, it doesnt solve all problems mentioned in this issue, but maybe some

[mirogta]

Is this about mixing mage targets and other non-mage targets but still exported functions in the same package?

I wonder if we could get around the issue and prevent adding more magic comments.

This is what worked for us: We split the package in two (or more), where some packages have mage targets and some have just normal exported functions, not mage targets. Sometimes the mage target is just a single line call to the "normal" exported function (not // _build mage). Then we don't need any overrides or comments to specify which exported functions shouldn't be exported as mage targets.

I'd love to hear why this wouldn't this work for you @anonfunc @ihgann @bjorndm?