Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

glibc: C.UTF-8 locales #54485

Merged
merged 2 commits into from Jan 23, 2019
Merged

glibc: C.UTF-8 locales #54485

merged 2 commits into from Jan 23, 2019

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Jan 22, 2019

Motivation for this change

This makes it possible to switch from C to C.utf-8 locales in stdenv and our glibc package as many applications now expect unicode locales (i.e. python). Debian and Fedora are already on board with this. Also systemd has now support for this.

Current status: works!

LOCALE_ARCHIVE= python -c 'import locale; locale.setlocale(locale.LC_ALL, "C.UTF-8")'
LOCALE_ARCHIVE= python -c 'import locale; locale.setlocale(locale.LC_ALL, "en_US.UTF-8")'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/nix/store/pxz7gzhgdyh5h3r0n09q6kik1dx9s0hx-python3-3.7.2-env/lib/python3.7/locale.py", line 604, in setlocale
    return _setlocale(category, locale)
locale.Error: unsupported locale setting

cc @FRidh

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@lheckemann lheckemann added this to the 19.03 milestone Jan 22, 2019
@@ -0,0 +1,3236 @@
C.utf-8 locales from https://salsa.debian.org/glibc-team/glibc/blob/49767c9f7de4828220b691b29de0baf60d8a54ec/debian/patches/localedata/locale-C.diff
--- /dev/null
+++ b/localedata/locales/C
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I can outsource this rather large file.
Since fetchpatch is not available while stdenv bootstrapping maybe fetchurl works.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetchurl certainly does work. All sources are fetched that way even during bootstrapping.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I just switched over.

@nixos-discourse
Copy link

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/nixos-19-03-feature-freeze/1950/3

Since we now install a sane default this should be no longer necessary.
If it is still needed, it should be easy enough to do this in an overlay.
@matthewbauer
Copy link
Member

Does this mean we can also get rid of glibcLocales?

@vcunat
Copy link
Member

vcunat commented Jan 22, 2019

No, many people want something else instead of the C. part of $LOCALE.

@Mic92
Copy link
Member Author

Mic92 commented Jan 22, 2019

@matthewbauer inside of nixpkgs we can get rid of glibcLocales for the most part. For NixOS we want to support more locales also for some images it would be nice if we can drop it i.e. containers, flatpak images or netboot installation in favor of C.UTF-8. Maybe we can even default our installation image to it?

@Mic92 Mic92 changed the title [WIP] C.UTF-8 locales: C.UTF-8 locales: Jan 22, 2019
@Mic92
Copy link
Member Author

Mic92 commented Jan 22, 2019

@GrahamcOfBorg build glibc

@Mic92 Mic92 changed the title C.UTF-8 locales: glibc: C.UTF-8 locales Jan 22, 2019
@ghost
Copy link

ghost commented Jan 23, 2019

LGTM 👍

@Mic92 Mic92 merged commit b7ad5e9 into NixOS:staging Jan 23, 2019
@Mic92 Mic92 deleted the c.utf-8 branch January 23, 2019 08:24
@Mic92
Copy link
Member Author

Mic92 commented Jan 23, 2019

Next step is to activate this in our stdenv.

@vcunat
Copy link
Member

vcunat commented Jan 23, 2019

Right, that sounds like a relatively safe change, too.

@ghost
Copy link

ghost commented Jan 23, 2019

I was running nix-review to test out the changes. Perhaps its my local environment but here's the output.

[asthma@dev:~/work/nixpkgs]$ nix-review pr 54485
$ git fetch --force https://github.com/NixOS/nixpkgs staging:refs/nix-review/0
pull/54485/head:refs/nix-review/1
remote: Enumerating objects: 94, done.
remote: Counting objects: 100% (94/94), done.
remote: Total 110 (delta 94), reused 94 (delta 94), pack-reused 16
Receiving objects: 100% (110/110), 30.15 KiB | 15.08 MiB/s, done.
Resolving deltas: 100% (94/94), completed with 76 local objects.
From https://github.com/NixOS/nixpkgs
+ c64951515b3...9b055427f5d staging              -> refs/nix-review/0  (forced
update)
+ 8844f09d53a...d966f31f23b refs/pull/54485/head -> refs/nix-review/1
(forced update)
$ git worktree add /home/asthma/.cache/nix-review/pr-54485/nixpkgs
9b055427f5d6070a9cd3d2c7aa65c4989b0a78d7
Preparing worktree (detached HEAD 9b055427f5d)
Checking out files: 100% (18064/18064), done.
HEAD is now at 9b055427f5d alsaLib: 1.1.7 -> 1.1.8
$ git merge --no-commit d966f31f23b5cb37c0e6bd8553f8503c0465f205
Auto-merging pkgs/top-level/all-packages.nix
Automatic merge went well; stopped before committing as requested
$ nix build --no-link --keep-going --max-jobs 4 --option build-use-sandbox
true -f /home/asthma/.cache/nix-review/pr-54485/build.nix
error: syntax error, unexpected IF, expecting ID or OR_KW or DOLLAR_CURLY or
'"', at /home/asthma/.cache/nix-review/pr-54485/build.nix:5852:21
https://github.com/NixOS/nixpkgs/pull/54485
550 package are marked as broken and were skipped:

-- a list of the broken packages --

2 package were blacklisted:
tests.nixos-functions.nixos-test tests.nixos-functions.nixosTest-test

23609 package failed to build:
-- a huge list of packages --

@FRidh
Copy link
Member

FRidh commented Jan 23, 2019

Thanks. I think it's a good change, but please don't merge changes to fundamental parts like glibc this quickly. It's good to offer some time for feedback.

@vcunat
Copy link
Member

vcunat commented Jan 23, 2019

@ivegotasthma: I'm not sure what got wrong, but the resulting commit b7ad5e9 evaluates fine for me.

@cw789
Copy link
Contributor

cw789 commented Jan 31, 2019

I'm of the opinion this will resolve NixOS/nix#318 & #20192, right?

@Mic92
Copy link
Member Author

Mic92 commented Jan 31, 2019

Only the first step. An export LC_ALL=C.utf-8 is missing.

danbst added a commit to danbst/nixpkgs that referenced this pull request Feb 5, 2019
- make it easy to switch python 2/3
- add glibcLocales dep (not sure it is still required after NixOS#54485
  but manual is also used by users of Nixpkgs versions <19.03)
- use `mkShell`
- use `libffi`, as it is required for Python3 `cryptography` (in my setup it worked with Python2 without this dep)
- set PYTHONPATH. Either this or `source venv/bin/activate` was missing.

Closes NixOS#46909
@Mic92
Copy link
Member Author

Mic92 commented Feb 8, 2019

@cw789 yes, but we also need it on macOS. I looked into that.
I hope that all we need to do is to add C to UTF8LINKS:
https://opensource.apple.com/source/adv_cmds/adv_cmds-118/usr-share-locale.tproj/mklocale/BSDmakefile.auto.html

The package is built in pkgs/os-specific/darwin/apple-source-releases/adv_cmds in nixpkgs.

Then we can set export LANG=C.UTF-8 for all targets.

@cdyson37
Copy link
Contributor

I'm using nixpkgs on a non nixos operating system and having various problems due to locales (including crashes using boost::filesystem). My workaround is to undo part of this PR and enable all locales by commenting out the SUPPORTED-LOCALES= line. It's a bit of a shame I have to hack the nix file for glibc to do this. Could we restore an option to have all locales?

I know I could set LOCALE_ARCHIVE=... but that's impure and makes things flaky when e.g. running jobs that don't source my bashrc.

Happy to open a separate issue to track the idea.

@Mic92
Copy link
Member Author

Mic92 commented Jul 26, 2019

@cdyson37 we never had all locales enabled in glibc. There is an extra package called glibcLocales that contains them. I am not quite sure, why boost is crashing in your case. Something like LOCALE_ARCHIVE could be also set as part of a home-manager module in a nicer way.

@cdyson37
Copy link
Contributor

Okay - I don't know how things were before :) But I'd like to be able to run applications that depend on locales without having to set an environment variable. I don't mind having to compile everything as I have to do that anyway. Perhaps we could introduce an option to the glibc derivation? I'm happy to have a go, might need some advice on writing a test for it though.

@matthewbauer
Copy link
Member

Okay - I don't know how things were before :) But I'd like to be able to run applications that depend on locales without having to set an environment variable. I don't mind having to compile everything as I have to do that anyway. Perhaps we could introduce an option to the glibc derivation? I'm happy to have a go, might need some advice on writing a test for it though.

Apps that "Depend on locales" is pretty rare. Usually they just depend on just one locale, or perhaps just UTF-8 support. Where this usually becomes an issue is when the locale-archive on the host system is a different version than the Glibc version we are using. Otherwise detection by Glibc should be automatic.

@vcunat
Copy link
Member

vcunat commented Jul 27, 2019

In your case I'd use a wrapProgram or something similar to set the variable to glibcLocales (you can override the large thing to only contain the locales that you want).

@cdyson37
Copy link
Contributor

In this case (at least) you'd have to wrap every program that used boost::filesystem::unique_path (it may well be a bug in that library). Here's an example:

default.nix:

with import <nixpkgs> {};
buildEnv {
  name = "myenv";
  paths = [gcc boost];
  extraOutputsToInstall = ["dev"];
}

test_unique_path.cpp:

#include <boost/filesystem.hpp>

int main ()
{
  boost::filesystem::unique_path ("/somewhere"); // does not need to exist
}

Then

$ nix-build
$ result/bin/g++ test_unique_path.cpp -o test_unique_path -I result/include -Wl,-rpath,result/lib -Lresult/lib -lboost_filesystem -lboost_system
$ echo $LANG
en_GB.UTF-8
$ ./test_unique_path 
terminate called after throwing an instance of 'std::runtime_error'
  what():  locale::facet::_S_create_c_locale name not valid
Aborted (core dumped)

Note that unsetting LANG leads the program to complete successfully.

Backtrace:

#0  0x00007ffff78f7be0 in raise () from /nix/store/681354n3k44r8z90m35hm8945vsp95h1-glibc-2.27/lib/libc.so.6
#1  0x00007ffff78f8dc1 in abort () from /nix/store/681354n3k44r8z90m35hm8945vsp95h1-glibc-2.27/lib/libc.so.6
#2  0x00007ffff7ebdf75 in __gnu_cxx::__verbose_terminate_handler() () from /nix/store/hlnxw4k6931bachvg5sv0cyaissimswb-gcc-7.4.0-lib/lib/libstdc++.so.6
#3  0x00007ffff7ebbd66 in ?? () from /nix/store/hlnxw4k6931bachvg5sv0cyaissimswb-gcc-7.4.0-lib/lib/libstdc++.so.6
#4  0x00007ffff7ebbdb1 in std::terminate() () from /nix/store/hlnxw4k6931bachvg5sv0cyaissimswb-gcc-7.4.0-lib/lib/libstdc++.so.6
#5  0x00007ffff7ebbff3 in __cxa_throw () from /nix/store/hlnxw4k6931bachvg5sv0cyaissimswb-gcc-7.4.0-lib/lib/libstdc++.so.6
#6  0x00007ffff7ee497f in std::__throw_runtime_error(char const*) () from /nix/store/hlnxw4k6931bachvg5sv0cyaissimswb-gcc-7.4.0-lib/lib/libstdc++.so.6
#7  0x00007ffff7ede0a4 in std::locale::facet::_S_create_c_locale(__locale_struct*&, char const*, __locale_struct*) ()
   from /nix/store/hlnxw4k6931bachvg5sv0cyaissimswb-gcc-7.4.0-lib/lib/libstdc++.so.6
#8  0x00007ffff7ecfa59 in std::locale::_Impl::_Impl(char const*, unsigned long) () from /nix/store/hlnxw4k6931bachvg5sv0cyaissimswb-gcc-7.4.0-lib/lib/libstdc++.so.6
#9  0x00007ffff7ed07a2 in std::locale::locale(char const*) () from /nix/store/hlnxw4k6931bachvg5sv0cyaissimswb-gcc-7.4.0-lib/lib/libstdc++.so.6
#10 0x00007ffff7fca233 in boost::filesystem::path::codecvt() () from result/lib/libboost_filesystem.so.1.67.0
#11 0x00007ffff7fcad35 in boost::filesystem::detail::unique_path(boost::filesystem::path const&, boost::system::error_code*) ()
   from result/lib/libboost_filesystem.so.1.67.0
#12 0x0000000000402c11 in boost::filesystem::unique_path(boost::filesystem::path const&) ()
#13 0x0000000000402294 in main ()

It's a bit worrying that setting LANG can change a function like that so like I say it may be a bug in the library.

@vcunat
Copy link
Member

vcunat commented Jul 29, 2019

Throwing such an exception seems a correct way to handle unusable $LANG. Well, the message printed to user might be a bit more friendly :-)

@cdyson37
Copy link
Contributor

Some background here: https://www.boost.org/doc/libs/1_70_0/libs/filesystem/doc/reference.html Search for "POSIX concerns".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants