Please don't use bash when there are syscalls available

Andrew Wilkins andrew.wilkins at canonical.com
Tue Sep 9 22:56:17 UTC 2014


On Wed, Sep 10, 2014 at 4:45 AM, Nate Finch <nate.finch at canonical.com>
wrote:

> A user just complained that he can't bootstrap because Juju is parsing
> stderr text from flock, and his server isn't in English, so the error
> message isn't matching.
>
>
> https://github.com/juju/juju/blob/master/environs/sshstorage/storage.go#L254
>
> Now, I think we all know that parsing error text is a bad idea, but I
> think I understand why it was done - it looks like flock the application
> only returns 1 on this failure, so it's not exactly a unique error code.
>  However, flock the system call returns several different error codes,
> which are quite unique and easy to handle in a way that is not dependent on
> the language of the machine.
>
> It also happens to be already implemented in the syscalls package:
>
> http://golang.org/pkg/syscall/#Flock
>
> So.... let's fix this, and try not to call out to bash unless there's
> absolutely no other way.
>

This is running on a remote system before there is any code deployed. I
won't say there's *no other way*, but you can't invoke syscalls from out of
thin air. Finally, that error message check has nothing to do with flock.
It's checking the result of I/O redirection to base64.

Agreed that parsing the message is dumb. We can fix this with an explicit
file existence check in the command executed ((test -e $path || echo
blah>&2)||base64<$path) or so. In the not too distant future, we will have
no need for sshstorage at all (except in past, supported releases).

-Nate
>
> --
> Juju-dev mailing list
> Juju-dev at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20140910/75abb2d4/attachment.html>


More information about the Juju-dev mailing list