Open mattly opened 6 years ago
I should note that this only solves the problem with bake, it doesn't solve the problem with ok
. 😿
I fixed ok
, but now it comes down to making sure everything is escaped properly in each of the types. I did that for git and github and updated the tests.
This is the sort of thing that makes me question the whole project to begin with, because Bash does all this magic with string quoting and argument positions with rules that are easy to trip over. What's been happening is:
ok github "my dotfiles" mattly/dotfiles
and ok was calling github status my dotfiles mattly/dotfiles --ssh
. github status 'my dotfiles' 'mattly/dotfiles' '--ssh'
, then github was calling ./git status my dotfiles git@github.com:mattly/dotfiles.git
../git status 'my dotfiles' 'git@github.com:mattly/dotfiles.git
but then git was calling [ ! -d my dotfiles ]
so I had to fix everywhere in git where $target_dir
was used so it was quoted properly." 'git' 'status' '-uno' '-b' '--porcelain'"
so you know things were quoted directly. And you have to do the same thing with the mocks and respond_to
. It's ugly, but workable.It's the sort of thing that makes me think I should try to rework how bork works to remove a layer or two of indirection, but then if I did that bork manifests wouldn't have the same sort of repurposability they have with status/satisfy/etc. Or maybe push the "data" of the manifests off into an actual data file, or its own format, but then you couldn't have that inline logic you so often need. May as well just use ansible, then. If I had the inclination I'd eliminate bake
and setup some sort of super-magic multi-environment testing harness with some sort of virtualization/container tech, but I don't. If someone else wants to tackle that, let me know.
Then problem then becomes, how do we ensure people adhere to the stuff necessary to maintain good quoting all the way through? And then I get tempted to port quickcheck to Bash, so that for example, you don't test "directory works with 'mydir' and 'my dir'", but rather "directory works with anything that resembles a valid directory name", which I'm not even really sure how to begin going about. Then I remember I have lots of other things I could be doing with my spare time that are much higher on the "want to" list. I mean maybe you could use lists and for
as well. That might be worth a shot.
Anyway, the problem is large but not insurmountable, but it's a lot of work and could cause some breakage with existing scripts or behaviors.
DO NOT MERGE. Actually I disabled merges that don't pass CI, and this sure as hell won't.
So this is a stab at #87 by @frdmn whereby bake isn't handling quoted arguments well. And, I knew I was doing something hacky with bake, but I really had no idea just how hacky. Short of reworking all the types to turn
bake ls -la $dir
intobake "ls -la $dir"
, this seems to be the next best thing: escaping quotes on every item in the argument list to bake.The problem is
bake
is so central to everything, all for the sake of having nice, stubbable tests, whereby I could test the logic for f.e.apt
on my mac, orbrew
on a linux CI server, without resorting to VMs and whatnot. So now pretty much all of the tests are going to fail and need to have theirrespond_to
andbaking_output
stuff changed.Since I'm not even sure this will have the desired effect without breaking everything (it seems to work ok locally) I'd like to get some eyes on it before going to the effort of updating all the tests.