(MODULES-1231) Fix apt::force locale issues

[juniorsysadmin]

The current $install_check variable greps for 'Installed' or 'Candidate', which means that it will give the wrong results when a non-English locale is used. This patch ensures that the check will work properly for non-English locales.

I have done basic testing with locales fr_FR.UTF-8 and en_US.utf8.
I'm not sure how to create an acceptance test for this since it might require a re-login for locale changes to take effect.

[juniorsysadmin]

@greg0ire More code comments added.

[greg0ire]

👍 great work

[daenney]

As this changes a few things this also needs documentation updates about the fact that we now rely on GNU sed and GNU awk.

[daenney]

Perhaps the better way to solve this though, instead of trying to support all different locales, is to simply set LC_ALL to C when calling out. The user will never see this anyway and it keeps our code simpler.

[greg0ire]

@daenney : I disagree, that would be really not clean. I think dpkg / apt must provide a clean way to do this in their API… maybe ask stack overflow ?

[daenney]

Actually, this is exactly what LC_ALL can be used for.

The C locale is a special locale that is meant to be the simplest locale. You could also say that while the other locales are for humans, the C locale is for computers. In the C locale, characters are single bytes, the charset is ASCII, the sorting order is based on the byte values, the language is usually US English.

Apt/dpkg respects the LC_* settings making the output conform to what you've done. Since apt/dpkg were designed as human tools and don't offer a --computer 'sane output for computers' flag the best you can do is switch to LC_ALL=C so you don't have to deal with different locales in your scripts.

[greg0ire]

Thank you for the explanation, it makes a lot of sense, and the solution seems quite acceptable with that in mind.

[daenney]

Considering this it might be worth making sure that Puppet Types and Providers actually do this by default... Scratch that, types and providers do but this isn't a provider.

[daenney]

Since this is an exec what I would do is leave the $install_check command etc. for what they were and add the following:

exec { ...:
  environment => ['LC_ALL'='C', 'LANG'='C'],
}

That's exactly what the Puppet::Uitl::Execution#execute method does which is what providers normally use.

[greg0ire]

@juniorsysadmin : what do you think ? I think the former solution involves less unix pipes, and has been used for a long time, so it must be better to tweak than to change it radically…

[daenney]

Uhm, why did this get added to $install_check instead of setting it through the environment key on the exec?

[juniorsysadmin]

Fixing it now. I thought I left a comment, but obviously not. Will put a comment when it's ready to be reviewed again.

[daenney]

Perfect!

[juniorsysadmin]

Rebased and retested with locales fr_FR.UTF-8 and en_US.utf8. The Debian-osfamily acceptance tests seem to have run successfully as well. Ready for a review again.

[daenney]

👍.

@mhaskel Could you be so kind to posit an opinion before I hit 'merge'?

[underscorgan]

@daenney 👍

[daenney]

Thanks @juniorsysadmin for your contribution!