magefile / mage

a Make/rake-like dev tool using Go
https://magefile.org
Apache License 2.0
4.17k stars 256 forks source link

Returning dynamic functions #101

Closed pauldotknopf closed 3 years ago

pauldotknopf commented 6 years ago
func ensureGoTool(tool, pkg string) func() error {
    return func() error {
        _, err := exec.LookPath(tool)
        if err != nil {
            fmt.Printf("couldn't find tool %s, attempting to download\n", tool)
            if _, err := sh.Exec(nil, os.Stdout, os.Stderr, "go", "get", "-u", pkg); err != nil {
                return err
            }
            _, err := exec.LookPath(tool)
            if err != nil {
                return fmt.Errorf("couldn't download tool %s from %s", tool, pkg)
            }
        }
        return nil
    }
}
func Vendor() error {
    mg.Deps(ensureGoTool("vndr", "github.com/LK4D4/vndr"))
    return sh.Run("vndr")
}
func Lint() error {
    mg.Deps(ensureGoTool("gometalinter", "github.com/alecthomas/gometalinter"))
    return sh.Run("gometalinter", "--config", ".gometalinter.json", "./pkg/...")
}
func Do() error {
    mg.Deps(Vendor, Lint)
    return nil
}

The problem is that with mage do, ensureGoTool only get's run once. There is probably some optimization to prevent similar tasks from being called multiple times. How do I disable it in this case?

natefinch commented 6 years ago

Interesting... I'm not sure if it's possible to get this to work, with mg.Deps. I'll look into it more, but for now, the fix would be to make separate real functions for each, like ensureVndr and ensureLint, that then call ensureGoTool with the right args.

pauldotknopf commented 6 years ago

make separate real functions for each, like ensureVndr and ensureLint

...I don't wanna...

;)

Awesome project by the way!

swdunlop commented 6 years ago

I have the same issue in our complex magefile and have used workaround from @natefinch. The mg.Deps function and its cousins maintain a table with the address of targets that have been started by Mage.

I believe this could be improved by defining a target interface that identifies a target that can handle the responsibility of preventing restart, such as:

// Target describes a target that prevents restarts using something like
// sync.Once.
type Target interface {
    // RunTarget will run a target on the first call and return nil on later
    // calls.  It should block until the first call completes.
    RunTarget(ctx context.Context) error
}

With this, we can add helpful types that can implement this interface for closures, such as:

// NewTarget produces a target that will run the provided function only once.
func NewTarget(impl func(ctx context.Context) error) Target {
    return &onceTarget{impl: impl}
}

type onceTarget struct {
    once sync.Once
    impl func(ctx context.Context) error
}

// RunTarget implements Target using a sync.Once
func (t *onceTarget) RunTarget(ctx context.Context) error {
    var err error
    t.once.Do(func() {
        err = impl(ctx)
    })
    return err
}

The Target interface provides an opportunity for targets that manage maps of targets to sync.Once, such as:

func updateHost(hostname string) Target {
    hostUpdateCtl.Lock()
    defer hostUpdateCtl.Unlock()
    h := hostUpdate(hostname)
    _, dup := updateHostOnce[h]
    if !dup {
        updateHostOnce[h] = new(sync.Once)
    }
    return h
}

type hostUpdate string

// RunTarget will only update a host if it has not been updated.
func (h hostUpdate) RunTarget(ctx context.Context) error {
    hostUpdateCtl.RLock()
    once := updateHostOnce[h]
    hostUpdateCtl.RUnlock()

    var err error
    once.Do(func() {
        err = h.Update(ctx)
    })
    return err
}

var (
    hostUpdateCtl sync.RWMutex 
    updateHostOnce = map[hostUpdate]*sync.Once{}
)

And yes, I do use Mage to update my hosts, build my applications, and fetch my beer. Great tool. :)

natefinch commented 6 years ago

Interesting, so are you saying that mg.Deps would look for things passed to it that implement that interface, and instead of doing its own sync.Once dance, let you do the dance for it? That sounds very promising.

so then the call site for this would be something like

func UpdateProd() {
    mg.Deps(foo, bar, updateHost("prod.me.com"))
    // ....
}

Basically that interface would mean "I know what I'm doing, just run me and I'll handle my own idempotence, thankyouverymuch".

swdunlop commented 6 years ago

Just that, or "I think I know what I am doing, and I'll let mg.NewTarget handle my idempotence." :)

swdunlop commented 5 years ago

I added an experimental interface, mg.Dependency to https://github.com/swdunlop/mage/tree/issue-101 along with mg.OnceMap and revised the various mg.Deps to use dependencies.

Applying mg.Dependency and mg.OnceMap to the example from @pauldotknopf, I have:

// a once map ensures we only look for and possibly get a tool once.
var ensureGoToolOnce mg.OnceMap

type ensureGoTool string

func (dep ensureGoTool) RunDependency(ctx context.Context) error {
    pkg := string(dep)
    ensureGoToolOnce.Do(pkg, func() error {
        tool := path.Base(pkg)
        _, err := exec.LookPath(tool)
        if err == nil {
            return nil
        }

        fmt.Printf("attempting to get %s from %s\n", tool, pkg)
        if _, err := sh.Exec(nil, os.Stdout, os.Stderr, "go", "get", "-u", pkg); err != nil {
            return err
        }

        _, err = exec.LookPath(tool)
        if err != nil {
            return fmt.Errorf("couldn't find tool %s in path", tool, pkg)
        }
        return nil
    })
}

func Vendor() error {
    mg.Deps(ensureGoTool("github.com/LK4D4/vndr"))
    return sh.Run("vndr")
}

func Lint() error {
    mg.Deps(ensureGoTool("github.com/alecthomas/gometalinter"))
    return sh.Run("gometalinter", "--config", ".gometalinter.json", "./pkg/...")
}

func Do() error {
    mg.Deps(Vendor, Lint)
    return nil
}

A more complex example, based loosely on my own private use of mage:

// Deploys a local configuration for development.
func DeployLocal() {
    mg.Deps(
        identity{`nats@localhost`, serverUsage},
        identity{`redis@localhost`, serverUsage},
        identity{`app@localhost`, clientUsage},
        // ...
    )
}

var (
    clientUsage = x509.ExtKeyUsageClientAuth
    serverUsage = x509.ExtKeyUsageServerAuth
)

// authority describes a Certificate Authority in a deployment.
type authority string

func (dep authority) RunDependency(ctx context.Context) error {
    return once.authority.Do(dep.generate)
}

// generates a certificate and key if needed for this certificate authority.
func (dep authority) generate() error { /* ... */ }

// identity describes an X.509 Certificate in a deployment.
type identity struct {
    name  string
    usage x509.ExtKeyUsage
}

func (dep identity) RunDependency(ctx context.Context) error {
    return once.identity.Do(dep.generate)
}

// generates a certificate and key if needed for this identity.
func (dep identity) generate() error {
    mg.Deps(dep.Authority())
    /* ... */
}

// authority returns the certificate authority that authenticates this identity.
func (dep identity) authority() authority { /* ... */ }

var once struct {
    service, identity, authority mg.OnceMap
}

Let me know if you want me to squash the changes and submit a pull request.

natefinch commented 3 years ago

You can now pass functions with arguments to mg.Deps and it'll do the right thing, thusly:

mg.Deps(mg.F(ensureGoTool, "vndr", "github.com/LK4D4/vndr"))

(note, this assumes ensureGoTool is the function that actually does the work, not something that returns a function)

There's also an mg.Fn interface that you can fulfill if you want to construct your own type of dependency and pass that to mg.Deps... but probably mg.F will do what you want 99% of the time.

swdunlop commented 3 years ago

Thank you for mg.Fn, a recent internal project caused me to loop back and extract mage-svc for managing local services using this feature.