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?

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,
If I put "| d2u" there I'll have to remember it and do it every time
automysqlbackup is updated - or create and maintain a Cygwin package
for this script. 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.

On 30 August 2016 at 23:57, cyg Simple <> 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:
> FAQ:         
> Documentation:
> Unsubscribe info:

Problem reports:
Unsubscribe info:

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