Script broken after updating bash to 4.3.46-7?

Script broken after updating bash to 4.3.46-7?

[Gene Pavlovsky]

After I updated Cygwin yesterday, a daily database backup bash script
(`automysqlbackup`) broke.
My previous bash was 4.3.42-4 (installed when I updated Cygwin on
2016/07/23), current is 4.3.46-7.
Here's the code snippet:
```bash
  local i;i=0;
  while read -r; do alldbnames[i++]="$REPLY"; done < <(mysql
--user="${CONFIG_mysql_dump_username}"
--password="${CONFIG_mysql_dump_password}"
--host="${CONFIG_mysql_dump_host}" "${mysql_opt[@]}" --batch
--skip-column-names -e "show databases")
```
This is supposed to get the list of all databases. Before it worked.
Now every item on the list ends with the CR character ($'\r'), causing
a bunch of issues with further script opreation. I'm using official
MariaDB Windows x64 binaries.
Frankly if mysql does output CRLF line endings, I don't know why that
script worked before, considering that this command in an interactive
shell produces a similar result:
```bash
# echo $'information_schema\r' | { read -r var; echo "{$var}"; }
}information_schema
```
But it did work somehow... Question is - what made it stop working
now, and what would be best way to fix it?

Regards,
Gene.

--
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

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

[Gene Pavlovsky]

Looks like it's related to a recent change in bash, which is `read`
now honors Cygwin-specific `igncr` shell option (`set -o igncr`),
which I didn't enable.
Adding `set -o igncr` to the top of the script does the job, however
I'd like to know how many more scripts are potentially malfunctioning
now?
It's lucky that one sent e-mails on errors, some others might just
break silently.
Is it advisable to add `set -o igncr` to /etc/profile or SHELLOPTS? I
didn't use that feature before and am worried about some other
negative side effects.
--Gene

On 27 August 2016 at 02:49, Gene Pavlovsky <gene.pavlovsky@gmail.com> wrote:
> After I updated Cygwin yesterday, a daily database backup bash script
> (`automysqlbackup`) broke.
> My previous bash was 4.3.42-4 (installed when I updated Cygwin on
> 2016/07/23), current is 4.3.46-7.
> Here's the code snippet:
> ```bash
>   local i;i=0;
>   while read -r; do alldbnames[i++]="$REPLY"; done < <(mysql
> --user="${CONFIG_mysql_dump_username}"
> --password="${CONFIG_mysql_dump_password}"
> --host="${CONFIG_mysql_dump_host}" "${mysql_opt[@]}" --batch
> --skip-column-names -e "show databases")
> ```
> This is supposed to get the list of all databases. Before it worked.
> Now every item on the list ends with the CR character ($'\r'), causing
> a bunch of issues with further script opreation. I'm using official
> MariaDB Windows x64 binaries.
> Frankly if mysql does output CRLF line endings, I don't know why that
> script worked before, considering that this command in an interactive
> shell produces a similar result:
> ```bash
> # echo $'information_schema\r' | { read -r var; echo "{$var}"; }
> }information_schema
> ```
> But it did work somehow... Question is - what made it stop working
> now, and what would be best way to fix it?
>
> Regards,
> Gene.

--
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

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

[Andrey Repin]

Greetings, Gene Pavlovsky!

> Looks like it's related to a recent change in bash, which is `read`
> now honors Cygwin-specific `igncr` shell option (`set -o igncr`),
> which I didn't enable.
> Adding `set -o igncr` to the top of the script does the job, however
> I'd like to know how many more scripts are potentially malfunctioning
> now?
> It's lucky that one sent e-mails on errors, some others might just
> break silently.
> Is it advisable to add `set -o igncr` to /etc/profile or SHELLOPTS? I
> didn't use that feature before and am worried about some other
> negative side effects.

It is advisable to not have CR's in your scripts to begin with.


-- 
With best regards,
Andrey Repin
Saturday, August 27, 2016 19:14:10

Sorry for my terrible english...


--
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

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

[Gene Pavlovsky]

Andrey,
That's what I personally think, none of the scripts I use have CRs,
and this is why I'd prefer not using the `igncr` option.
However the recent change to how `read` works makes it necessary to
modify existing scripts which interoperate with Windows console
programs (in my case, Windows build of mysql (MariaDB)) that produce
CRLF line endings.
--Gene

On 27 August 2016 at 19:15, Andrey Repin <anrdaemon@yandex.ru> wrote:
> Greetings, Gene Pavlovsky!
>
>> Looks like it's related to a recent change in bash, which is `read`
>> now honors Cygwin-specific `igncr` shell option (`set -o igncr`),
>> which I didn't enable.
>> Adding `set -o igncr` to the top of the script does the job, however
>> I'd like to know how many more scripts are potentially malfunctioning
>> now?
>> It's lucky that one sent e-mails on errors, some others might just
>> break silently.
>> Is it advisable to add `set -o igncr` to /etc/profile or SHELLOPTS? I
>> didn't use that feature before and am worried about some other
>> negative side effects.
>
> It is advisable to not have CR's in your scripts to begin with.
>
>
> --
> With best regards,
> Andrey Repin
> Saturday, August 27, 2016 19:14:10
>
> Sorry for my terrible english...
>

--
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

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

[Nellis, Kenneth]

From: Gene Pavlovsky 
> Andrey,
> That's what I personally think, none of the scripts I use have CRs,
> and this is why I'd prefer not using the `igncr` option.
> However the recent change to how `read` works makes it necessary to
> modify existing scripts which interoperate with Windows console
> programs (in my case, Windows build of mysql (MariaDB)) that produce
> CRLF line endings.
> --Gene

I sympathize, but it could be argued that your script was invalid;
that it properly should have piped the output of mysql through d2u.

--Ken Nellis

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

[cyg Simple]

On 8/27/2016 12:15 PM, Andrey Repin wrote:
> Greetings, Gene Pavlovsky!
> 
>> Looks like it's related to a recent change in bash, which is `read`
>> now honors Cygwin-specific `igncr` shell option (`set -o igncr`),
>> which I didn't enable.
>> Adding `set -o igncr` to the top of the script does the job, however
>> I'd like to know how many more scripts are potentially malfunctioning
>> now?
>> It's lucky that one sent e-mails on errors, some others might just
>> break silently.
>> Is it advisable to add `set -o igncr` to /etc/profile or SHELLOPTS? I
>> didn't use that feature before and am worried about some other
>> negative side effects.
> 
> It is advisable to not have CR's in your scripts to begin with.
> 

I thought this was about the data the script was reading and not the
script itself.  Data can have \r regardless of where in the data it
exists.  That said relying on defaults for a process has been taboo
since the dawn of computing.  Often defaults change on a whim, they are
bicycle shed colors, a maintainer may prefer blue instead of red in any
given release.  If your script requires a specific mode for the shell to
process data properly then be sure to set it before reading the 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

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

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 1397 bytes --]

On 08/26/2016 06:49 PM, Gene Pavlovsky wrote:
> After I updated Cygwin yesterday, a daily database backup bash script
> (`automysqlbackup`) broke.
> My previous bash was 4.3.42-4 (installed when I updated Cygwin on
> 2016/07/23), current is 4.3.46-7.
> Here's the code snippet:
> ```bash
>   local i;i=0;
>   while read -r; do alldbnames[i++]="$REPLY"; done < <(mysql
> --user="${CONFIG_mysql_dump_username}"
> --password="${CONFIG_mysql_dump_password}"
> --host="${CONFIG_mysql_dump_host}" "${mysql_opt[@]}" --batch
> --skip-column-names -e "show databases")
> ```
> This is supposed to get the list of all databases. Before it worked.

By pure luck.  The 'read' builtin was inconsistent on cygwin, in that it
FORCED people to use text mode (and ate \r before \n), even when they
wanted pure binary input, which was different than all other aspects of
bash; and worse had a bug where \r before something else would cause the
something else to be incorrectly eaten.

> Now every item on the list ends with the CR character ($'\r'), causing
> a bunch of issues with further script opreation. I'm using official
> MariaDB Windows x64 binaries.

Simplest fix:

read ... < <(mysql ... | dos2unix)

There. Now you aren't feeding \r to read in the first place.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[cyg Simple]

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.

-- 
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

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

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 2246 bytes --]

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'.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[cyg Simple]

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

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

[Gene Pavlovsky]

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 - 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.
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

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

[Lee]

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

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

[Gene Pavlovsky]

Lee, you have a point.

Adding mysql.sh that runs `mysql.exe | d2u`, or using a cygwin version
of mysql looks like the most viable solution at the moment.
Or staying with the older version of bash, in which `read` still
handled `\r\n` line endings...
My point is OK I'll fix that script, who knows what other scripts
might be broken now? And each one might need an investigation before I
find out which program's windows version needs a wrapper script etc.

--Gene

On 4 September 2016 at 20:37, Lee <ler762@gmail.com> wrote:
> 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
>

--
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

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

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 636 bytes --]

On 09/04/2016 04:38 AM, 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/).

Then submit a bug report to the author of that script to tell them to
fix their buggy script to portably strip carriage returns.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]