mage icon indicating copy to clipboard operation
mage copied to clipboard

Allow composition/extension of default context

Open arcticicestudio opened this issue 4 years ago • 5 comments

First, thanks for this fantastic project 💪 In my opinion, a similar build system like Mage should be supported as official Go tool that is used by the go command by default so projects can be build with customizations like variable initialization during build time with -ldflags, e.g. to inject version string parsed from Git repository or the build date/time (which is currently not possible when simply running go get -u IMPORT_PATH 😟)

Anyway, I've worked on a small (personal/opinionated) Go library/package that provides helper and Mage targets/tasks I often re-wrote for each of my projects. This will allow me to simply import them into my magefile.go (using the mage:import directive) to reduce the boilerplate code in each project and maintaining overhead in multiple places.

The library/package is mainly designed for Go Monorepos, but can be used for a single Go app repository as well. It provides a struct (lets call it ModuleConfig) that stores general information for Mage targets/tasks that are implemented as methods on this struct. The provides Mage targets/tasks are for the whole project like e.g. Lint() or Format(). Another struct (lets call it AppConfig) stores information especially for a one specific application within the Go Monorepo. This struct is used for Mage targets/tasks that are scoped for this specific application and will be used in the magefile.go by importing them via and alias (e.g. server.Build). This structure allows to have "global" tasks for the whole project as well as Mage targets/tasks for all the different applications within the Monorepo.

EachAppConfig struct is created in the magefile.go and stored in the ModuleConfigstruct using a simple map (map[string]*AppConfig) where the key name is the name of the application. In order to pass the AppConfig to the imported Mage targets/tasks, a wrapper function (func(appName string) *AppConfig) is stored in a context.Context and then passed to the imported Mage target/task functions. The function calls the wrapper function (passing it's own application as parameter) to receive the AppConfig. This approach works great when the imported Mage targets/tasks are called through a kind of "collection" function(e.g. Build()) within the magefile.go that now calls all the imported functions (e.g. mg.SerialCtxDeps(passAppCtx(), server.Build, cli.Build)), but not when directly calling an imported alias function from the terminal (e.g. mage cli:build or mage server:build). The reason is that the helper function passAppCtx() is not called that adds the wrapper function via context.WithValue. Therefore the imported alias function only receives the default empty context passed by Mage and is not able to call the wrapper function to receive the AppConfig.

I hope this wall of text is reasonably understandable, but adding example code would go beyond the scope of this issue 😆

Long story short

It would be great to have a function (e.g. mage.SetDefaultContext(func() context.Context) or variable with a specific name (e.g. MageDefaultContext context.Context) to override the default context passed to every Mage target/task function instead of an empty one via context.Background().

Use Case

Pass wrapper/helper function(s) through context to imported (alias) Mage targets/tasks like build metadata. This is especially useful when working in a Go Monorepo where each included application provides specific Mage targets/tasks through imported alias functions.

I tried to implement the function approach by implementing it, parsing it and editing the template, but still without success 😟 Would be great to get some feedback on the idea and can try to help implementing and reviewing the feature if it gets accepted.

I guess it shouldn't be too hard to implement and might also help to solve many of the requests in this repository where users like to pass parameters to Mage target/task functions since this would allow to simply add all the parameters to the context.

arcticicestudio avatar Nov 05 '19 08:11 arcticicestudio

This is an interesting idea. I think that instead of just a static replacement, we probably would want a way to wrap the default context. There's already an optional timeout you can give to the context, and we want to eventually capture ctrl-C and cancel the context when we get that signal. So to avoid losing these features, it's probably best to make this a function that could use Context.WithValue() to add data to it.

So something like this:

var UpdateContext = func(ctx context.Context) context.Context {
 return ctx.WithValue("key", "some value from somewhere")
}

natefinch avatar Nov 05 '19 18:11 natefinch

That's exactly what I imagined :smile: Maybe prefixing this special variable with Mage (MageUpdateContext) can also help to avoid possible name collisions.

This feature could be the most simple solution for all the requests to allow additional parameter since users can add everything to the context, like a function that can be extracted and called within the Mage target function to get data outside from the functions scope. It can be seen as the container for all the additional parameters a function should have.

Also implementing this feature can be done without breaking the API (in contrast to changing the allowed target function signatures) and won't require a major version bump but a simple minor version (1.10.0).

Let me know if there's anything I can help with to get this into Mage. I could try to implement it, but I guess I'll need to find some time to get familiar with the current code base and logic.

arcticicestudio avatar Nov 05 '19 20:11 arcticicestudio

@arcticicestudio Is your personal/opinionated Go library/package available somewhere? I've got a few piles of similar things lying around for working in Monorepos, and I'd love to compare notes. I don't want to hijack this issue though.

StevenACoffman avatar Nov 29 '19 18:11 StevenACoffman

@StevenACoffman It's currently stored on my own server, but it'll be open sourced when I've cleaned up the API, resolved pending tasks and wrote the documentation. Your repository README contains some interesting information and links to other projects, but I guess my lib is special since it it tailored especially for mage. I'll let you know when the repository is up :smile:

arcticicestudio avatar Dec 08 '19 19:12 arcticicestudio

@arcticicestudio Yeah, that's where I prototyped some stuff that I used for some closed source mage helpers. I put together an awesome-mage that I would love to add your library to once it's released.

StevenACoffman avatar Dec 09 '19 15:12 StevenACoffman