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

Work around a locale issue in Ormolu #1520

Merged
merged 11 commits into from
May 26, 2021
Merged

Work around a locale issue in Ormolu #1520

merged 11 commits into from
May 26, 2021

Conversation

arianvp
Copy link
Contributor

@arianvp arianvp commented May 21, 2021

Otherwise ormolu doesn't work

Otherwise ormolu doesn't work
@arianvp arianvp requested a review from mdimjasevic May 21, 2021 08:02
mdimjasevic and others added 5 commits May 25, 2021 15:24
* Removes the Data.Qualified.OptionallyQualified data type from types-common
* Refactor: Move Brig.User.Event* to brig-types

* Make notification checkers give complete call stack with error in galley legalhold integration tests.

* mkApp also returns finalizer for logs

* Use real brig event types in galley tests.

Co-authored-by: Matthias Fischmann <mf@zerobuzz.net>
* Add legalhold status to user profile

* Work around bug in test utils.

see https://wearezeta.atlassian.net/browse/SQSERVICES-471

Co-authored-by: Matthias Fischmann <mf@zerobuzz.net>
* Encode golden jsons using prettyEncode for easier diffs

* Re-encode golden test jsons so they are pretty

* Reformat generated golden tests for easier reviews

Reformatting done like this:
- Install hindent

- Write .hindent.yaml as (note that couple of extensions are missing as
  hindent doesn't support them yet):

  ```
  indent-size: 2
  line-length: 120
  force-trailing-newline: true
  extensions:
  - BangPatterns
  - ConstraintKinds
  - DataKinds
  - DefaultSignatures
  - DerivingStrategies
  - DerivingVia
  - DeriveFunctor
  - DeriveGeneric
  - DeriveTraversable
  - EmptyCase
  - FlexibleContexts
  - FlexibleInstances
  - FunctionalDependencies
  - GADTs
  - InstanceSigs
  - KindSignatures
  - LambdaCase
  - MultiParamTypeClasses
  - MultiWayIf
  - NamedFieldPuns
  - NoImplicitPrelude
  - OverloadedStrings
  - PackageImports
  - PatternSynonyms
  - PolyKinds
  - QuasiQuotes
  - RankNTypes
  - ScopedTypeVariables
  - StandaloneDeriving
  - TemplateHaskell
  - TupleSections
  - TypeApplications
  - TypeFamilies
  - TypeFamilyDependencies
  - TypeOperators
  - UndecidableInstances
  - ViewPatterns
  ```

- Run ormolu on the whole project
@mdimjasevic mdimjasevic requested a review from pcapriotti May 25, 2021 13:26
@mdimjasevic mdimjasevic changed the title Force LC_ALL to UTF-8 Work around a locale issue in Ormolu May 25, 2021
@mdimjasevic
Copy link
Contributor

@arianvp and @pcapriotti , I just updated the PR to use the LOCALE_ARCHIVE variable instead, i.e., this should be a proper workaround for the locale issue.

shell.nix Outdated
@@ -58,6 +58,7 @@ let
};
in pkgs.mkShell {
name = "shell";
LOCAL_ARCHIVE=${glibcLocales}/lib/locale/locale-archive; # works around https://github.com/tweag/ormolu/issues/38
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be pkgs.glibcLocales?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am an absolute beginner at Nix. Should I replace "glibcLocales" with "pkgs.glibcLocales"?

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to wrap it in quotes and qualify glibcLocales with pkgs.

Copy link
Contributor

@pcapriotti pcapriotti left a comment

Choose a reason for hiding this comment

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

This doesn't look right. LOCALE_ARCHIVE should be an environment variable, not an attribute.

@arianvp
Copy link
Contributor Author

arianvp commented May 25, 2021

@pcapriotti attributes of a derivation are exposed as environment variables.

Every attribute is passed as an environment variable to the builder. Attribute values are translated to environment variables as follows:

https://nixos.org/manual/nix/unstable/expressions/derivations.html

@pcapriotti
Copy link
Contributor

@pcapriotti attributes of a derivation are exposed as environment variables.

Ah, I didn't know that, thanks.

@pcapriotti pcapriotti dismissed their stale review May 25, 2021 14:57

Setting the variable there is fine

@mdimjasevic mdimjasevic requested a review from pcapriotti May 26, 2021 05:51
Copy link
Contributor

@pcapriotti pcapriotti left a comment

Choose a reason for hiding this comment

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

I think it should be LOCALE_ARCHIVE, not LOCAL_ARCHIVE, right? Have you tried to see if it fixes your issue?

shell.nix Outdated
@@ -58,6 +58,7 @@ let
};
in pkgs.mkShell {
name = "shell";
LOCAL_ARCHIVE="${pkgs.glibcLocales}/lib/locale/locale-archive"; # works around https://github.com/tweag/ormolu/issues/38
Copy link
Contributor

@pcapriotti pcapriotti May 26, 2021

Choose a reason for hiding this comment

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

Suggested change
LOCAL_ARCHIVE="${pkgs.glibcLocales}/lib/locale/locale-archive"; # works around https://github.com/tweag/ormolu/issues/38
LOCALE_ARCHIVE = "${pkgs.glibcLocales}/lib/locale/locale-archive"; # works around https://github.com/tweag/ormolu/issues/38

@mdimjasevic mdimjasevic merged commit fa96fae into develop May 26, 2021
@mdimjasevic mdimjasevic deleted the arianvp-patch-3 branch May 26, 2021 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants