Please don't use bash when there are syscalls available

[Andrew Wilkins]

On Wed, Sep 10, 2014 at 6:57 PM, Nate Finch <nate.finch at canonical.com>
wrote:

> Thanks for the clarification, I misunderstood what the code was doing.
>  I'm glad to hear this code won't be needed for much longer, but I think we
> should backport your explicit check so that non-english users can use Juju.
>

I have filed https://bugs.launchpad.net/juju-core/+bug/1367695, assigned to
1.20.8.


> On Tue, Sep 9, 2014 at 6:56 PM, Andrew Wilkins <
> andrew.wilkins at canonical.com> wrote:
>
>> 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/ce334bb5/attachment-0001.html>