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

Conversation

@Mic92
Copy link
Contributor

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

This comment has been minimized.

@Mic92

Mic92 Jan 22, 2019

Author Contributor

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

This comment has been minimized.

@vcunat

vcunat Jan 22, 2019

Member

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

This comment has been minimized.

@Mic92

Mic92 Jan 22, 2019

Author Contributor

Yeah. I just switched over.

@Mic92 Mic92 force-pushed the Mic92:c.utf-8 branch from 2855051 to 288422f Jan 22, 2019

@nixos-discourse

This comment has been minimized.

Copy link

nixos-discourse commented Jan 22, 2019

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

Mic92 added some commits Jan 22, 2019

glibc: remove installLocales argument
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

This comment has been minimized.

Copy link
Member

matthewbauer commented Jan 22, 2019

Does this mean we can also get rid of glibcLocales?

@vcunat

This comment has been minimized.

Copy link
Member

vcunat commented Jan 22, 2019

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

@Mic92

This comment has been minimized.

Copy link
Contributor 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

This comment has been minimized.

Copy link
Contributor 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

@ivegotasthma

This comment has been minimized.

Copy link
Contributor

ivegotasthma commented Jan 23, 2019

LGTM 👍

@Mic92 Mic92 merged commit b7ad5e9 into NixOS:staging Jan 23, 2019

10 checks passed

grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details

@Mic92 Mic92 deleted the Mic92:c.utf-8 branch Jan 23, 2019

@Mic92

This comment has been minimized.

Copy link
Contributor Author

Mic92 commented Jan 23, 2019

Next step is to activate this in our stdenv.

@vcunat

This comment has been minimized.

Copy link
Member

vcunat commented Jan 23, 2019

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

@ivegotasthma

This comment has been minimized.

Copy link
Contributor

ivegotasthma 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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link

cw789 commented Jan 31, 2019

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

@Mic92

This comment has been minimized.

Copy link
Contributor 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

python doc: update virtualenv section
- 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

This comment has been minimized.

Copy link
Contributor 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.

@FRidh FRidh referenced this pull request Feb 15, 2019

Open

document locale issues #6878

@Mic92 Mic92 referenced this pull request Feb 15, 2019

Closed

buildPythonPackage: always export `LANG=C.UTF-8` #55826

0 of 10 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment