search

magefile / mage

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

Feature: sh.Exec with optional stdin #318

Open flowchartsman opened 3 years ago

flowchartsman commented 3 years ago

Part of one of my build steps is a docker login using AWS creds. For security the docker executable prefers you to pass the password on stdin as opposed to with arguments and throws a warn every time you do so, which I have to eat and then attempt to discern from a real failure. This also means the full command (with password) is output to the console on failure. I think an optional io.Writer for stdin would solve this problem.

natefinch commented 3 years ago

Yeah.... it gets hard to manage so many optional parameters. Maybe what we need is a struct you can fill out and it'll make sane defaults for anything you leave out (which mostly gets us back to basically using os/exec.... but I think we can still make a better UX)

DavidGamba commented 3 years ago

Hi guys, I have been experimenting with an alternative, let me know if you guys like it and @natefinch if you see value in it being part of mage let me know and we can see how to integrate it.

https://github.com/DavidGamba/dgtools/tree/master/run

carolynvs commented 3 years ago

@DavidGamba I would love to see mage go in this direction: fluent with a struct to contain all the various parameters currently passed to run. We can still have the same functions as before, but if they used this fluent struct under the covers, it would give a lot more flexibility without having to keep adding more function variants (Run, RunV, etc) and make it easier to extend without forking. 👍

carolynvs commented 3 years ago

If this is something you would consider doing, I would help implement/review/test. 😀

flowchartsman commented 3 years ago

I'm not a fan of fluent interfaces, since options can be repeated and context for them can get kind of confusing. I'm not sure how much a fluent interface buys over a well-considered config struct with sensible defaults, but, like @natefinch said, that gets us pretty close to os/exec. I could be convinced, though! I got about halfway through a proposal before I got pulled away on something else, so I'll try and drop something here in the next several days.

carolynvs commented 3 years ago

I ended up writing a new struct PreparedCommand that lets you build up a command incrementally, and set things like environment variables, the current directory, make new commands using a set of defaults, etc. It's just a general purpose wrapper around os/exec that I build into a new package that works like mage/sh along with helpers for other things that I needed.

I think this shx package could be useful to have in upstream mage, because then people don't need to ask for special commands like "RunWith" or "RunFrom", etc. If anyone is interested

https://pkg.go.dev/github.com/carolynvs/magex

ghostsquad commented 2 years ago

May want to look at the functional options pattern over a fluent interface.

https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis