-
Notifications
You must be signed in to change notification settings - Fork 95
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
Explicitly set locale to C.UTF-8 inside buildAndWatch script #236
Conversation
This should fix the problem reported in #29
Yup, fixed! |
#30 only sets the environment variable during the Nix build and in a Nix shell. That used to be fine since A better solution would probably be rewriting the Haskell code so that it did not depend on the system's locale settings, but getting that right seems like a bit of a pain—the way locale stuff works in the standard library seems pretty fiddly, and it might even depend on Hakyll internals. |
@@ -12,6 +12,7 @@ function buildAndWatchWithNix() { | |||
exit 1 | |||
fi | |||
|
|||
export LC_ALL=C.UTF-8 # fix locale error with Hakyll (see #29) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than exporting this for the entire script we could run the individual commands with this set. For example:
nix-build -A builder && LC_ALL=C.UTF8 ./result/bin/…
that would prevent the change from leaking outside of the command that needs it.
Personally I'd prefer to understand exactly which part of the pipeline needs the envvar, and set it only there, but approving, since I'm sure you know what you're doing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @tomjaguarpaw that we can probably just merge this for now and maybe revisit later see if we can remove the need for it.
Probably best to set the env var for any invocation of |
This should fix the problem reported in #29