This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Script broken after updating bash to 4.3.46-7?


On 9/4/16, Gene Pavlovsky wrote:
> This issue never affected me personally, but it sounds like a serious
> bug and I'm as glad as anybody that it's finally fixed.
> However, having existing scripts suddenly breaking is not great. I'd
> like to remind that I'm not the author of the script in question,
> [AutoMySQLBackup](http://sourceforge.net/projects/automysqlbackup/).
> If I put "| d2u" there I'll have to remember it and do it every time
> automysqlbackup is updated ...

Right - updating something like automysqlbackup to add "| d2u" after
every dos program call is a non-starter.  But would putting the dos
program inside a script that converted dos -> unix line endings work?
eg - have a mysql script that comes before the windows version of
mysql in your path that does something like
/path/to/windows/version/of/mysql $@ | dos2unix

> - or create and maintain a Cygwin package for this script.

It seems to me that it isn't the script that's broken - it's the whole
idea of having a dos program transparently integrate into an
environment that expects unix line endings that's broken.  So until
there's a bash option that automatically translates '\r\n' line
endings into '\n' line endings you're stuck doing some kind of
work-around or using a cygwin version of mysql.

Lee


> And who knows how many other scripts might be broken,
> I just didn't find it yet?
>
> Having a `read`-specific shell option telling read to treat `\r\n`
> (and only `\r\n`, not \r followed by something else) same as `\n`
> would be bad things to have? For me, this kind of option would be more
> useful than the `igncr` crutch
> Let me say it another way - in OOP programming, one of good practices
> is Single Responsibility Principle - a class should be responsible for
> only one feature/function, and that feature/function should be totally
> encapsulated in that class. Similar to that, an option should be
> responsible for one behavior. With this change to `read`, the `igncr`
> shell option is starting to look like a kitchen sink... split it into
> separate options, please!
> I think making UNIX scripts work on Cygwin with no or minimum
> modifications (or bug-hunting) should be one of high priorities, no?
> If some scripts erroneously have CRLF line endings, it's easy to find
> and `d2u` them, rather than using the `igncr` crutch, but with the
> recent change to `read`, countless scripts might be broken in a
> non-obvious way. Fixing them would require finding out they're broken,
> in the first place. Imagine if I didn't set up my cron to e-mail me
> the cron jobs output? My backup script would just stop working
> silently, and some time later when I needed a recent backup, I would
> find out there aren't any. There might be something else lurking that
> I haven't found yet. Once a script, broken by this change to `read`,
> is found, it must be checked thoroughly to find out where exactly is
> the problem, where to put '| d2u', or maybe 'set -o igncr'. These
> fixes must also be applied anytime a 3rd party script is updated.
> Quite a lot of work!
> Hope you will consider my point.
> Regards,
> Gene.
>
> On 30 August 2016 at 23:57, cyg Simple <cygsimple@gmail.com> wrote:
>> On 8/30/2016 1:38 PM, Eric Blake wrote:
>>> On 08/30/2016 12:04 PM, cyg Simple wrote:
>>>> On 8/29/2016 2:30 PM, Eric Blake wrote:
>>>>>
>>>>> Simplest fix:
>>>>>
>>>>> read ... < <(mysql ... | dos2unix)
>>>>>
>>>>
>>>> This will break when the data returned by mysql is supposed to contain
>>>> \r.
>>>>
>>>>> There. Now you aren't feeding \r to read in the first place.
>>>>>
>>>>
>>>> But you might want to feed \r to read.  It isn't a fix, it is a
>>>> potential work around dependent on the data set results.  If a read
>>>> that
>>>> is supposed to be reading binary data doesn't pass all of the data to
>>>> the routine then it is broken.
>>>
>>> Now we're talking past each other.
>>>
>>> That's what the recent bash fixed.  'read' in bash 3.2.42-4 was broken -
>>> it corrupted binary data, with no recourse, by eating \r (and worse, by
>>> sometimes eating the byte after \r).  'read' in bash 3.2.46-7 is fixed -
>>> by default it is strictly binary (all bytes are read as-is, including
>>> \r), but can also be switched to text mode (using 'igncr', all \r are
>>> ignored).  If you want to preserve mid-line \r but treat line endings of
>>> \r\n as a single byte, then leave binary mode on and strip the line
>>> endings via a separate tool like d2u (note, however, that it is very
>>> rare to have data where mid-line \r is important but line-ending \r\n
>>> should be treated as plain \n).
>>>
>>> I strongly think that using igncr is a crutch, and you normally
>>> shouldn't use it; particularly not if you want to be portable to other
>>> platforms.  Instead, massaging your data through d2u is a great way to
>>> be portable.  But sometimes the ease of ignoring ALL \r is easier than
>>> worrying about portability, so I keep the 'igncr' code in Cygwin.
>>>
>>> And it is only because the OP tried using 'igncr' in the first place
>>> (whether or not it was actually needed) that we have now flushed out the
>>> existence of a latent bug in the 'igncr' implementation that interacts
>>> weirdly with $()\n in PS1.  On that front, I'm still hoping to find time
>>> to debug and/or for someone to post a patch.  But whether PS1 behaves
>>> weirdly under 'igncr' is orthogonal to my suggestion above - using
>>> 'mysql|d2u' is a great way to avoid the need to worry about 'igncr'.
>>>
>>
>> Thank you for the retort Eric.  Happy to know that it is fixed which in
>> the back of my mind I knew already.  I can imagine data such as full
>> message email or a small document data containing \r\n as valid data in
>> the database field and if you use a line ending conversion utility you
>> might loose that data.
>>
>> --
>> cyg Simple
>>
>> --
>> Problem reports:       http://cygwin.com/problems.html
>> FAQ:                   http://cygwin.com/faq/
>> Documentation:         http://cygwin.com/docs.html
>> Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
>>
>
> --
> Problem reports:       http://cygwin.com/problems.html
> FAQ:                   http://cygwin.com/faq/
> Documentation:         http://cygwin.com/docs.html
> Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
>
>

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]