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.

Sign up for GitHub

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

Jump to bottom

Fix text locale encoding issues on windows #914

Merged
merged 1 commit into from
Oct 28, 2019
Merged

Fix text locale encoding issues on windows #914

merged 1 commit into from
Oct 28, 2019

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Oct 25, 2019 •
edited
Loading

Relates to #703.

Overview

  • Forces the current windows console into UTF-8 mode.
  • UTF-8 is far from standard on windows. But it shouldn't be assumed on any platform.
  • This fixes strange characters appearing on windows and test failures with exceptions related to encoding.

@rvl rvl self-assigned this Oct 25, 2019
@rvl rvl mentioned this pull request Oct 25, 2019
Merged
10 tasks
jonathanknowles
jonathanknowles reviewed Oct 28, 2019
View reviewed changes
lib/cli/src/Cardano/CLI.hs Show resolved Hide resolved
lib/cli/src/Cardano/CLI.hs Outdated
setLocaleEncoding utf8
hSetEncoding stdout utf8
hSetEncoding stderr utf8
let utf8CP = 65001 -- code page number for UTF-8
Copy link
Member

Choose a reason for hiding this comment

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

The comment can probably be made redundant if we rename the identifier to something like:

utf8CodePage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

@rvl
Copy link
Contributor Author

rvl commented Oct 28, 2019

I need to move setUtf8Encoding to the cardano-wallet-core package to avoid a package dependency cycle.

@rvl rvl force-pushed the rvl/703/update-haskell-nix branch from 7ccc983 to 717f1b5 Compare October 28, 2019 05:05
@rvl rvl force-pushed the rvl/703/fix-windows-encoding branch from a2dece7 to e504c72 Compare October 28, 2019 07:12
@rvl rvl changed the base branch from rvl/703/update-haskell-nix to master October 28, 2019 07:12
@rvl
Copy link
Contributor Author

rvl commented Oct 28, 2019

Rebased onto master.

@KtorZ KtorZ force-pushed the rvl/703/fix-windows-encoding branch from e504c72 to 75b7b56 Compare October 28, 2019 17:41
@KtorZ
Copy link
Member

KtorZ commented Oct 28, 2019

bors r+

iohk-bors bot added a commit that referenced this pull request Oct 28, 2019
@iohk-bors @rvl
Merge #914
8132207 
914: Fix text locale encoding issues on windows r=KtorZ a=rvl

Relates to #703.

# Overview

- Forces the current windows console into UTF-8 mode.
- UTF-8 is far from standard on windows. But it shouldn't be assumed on any platform.
- This fixes strange characters appearing on windows and test failures with exceptions related to encoding.


Co-authored-by: Rodney Lorrimar <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 28, 2019

Build succeeded

@iohk-bors iohk-bors bot merged commit 75b7b56 into master Oct 28, 2019
iohk-bors bot added a commit that referenced this pull request Oct 28, 2019
@iohk-bors @rvl
Merge #888
3f4449d 
888: Update Haskell.nix r=KtorZ a=rvl

Relates to #703.

# Overview

- [x] Updates the Haskell.nix version so that we can build for windows again.
- [x] Keeps more or less the same structure as before - i.e. no IFD, no niv. I don't want too much change at once.
- [x] Does however update nixpkgs from iohk fork of 18.09 → upstream 19.03.
- [x] Provides a big bundle of scripts and tests that can be run on Windows.

[Hydra jobset](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-888)
[Hercules jobset](https://hercules-ci.com/github/input-output-hk/cardano-wallet)
[Link to download Windows testing bundle for PR #888](https://hydra.iohk.io/job/Cardano/cardano-wallet-pr-888/cardano-wallet-jormungandr-win64/latest)
[Link to download Windows testing bundle (master branch)](https://hydra.iohk.io/job/Cardano/cardano-wallet/cardano-wallet-jormungandr-win64/latest)

# Comments

@angerman Please help

- [x] Fix failure for missing `hsc2hs`
- [x] Fix a previous mistake that affects cross builds (failing to cross-compile bash for some reason)
- [x] Worked around an [evaluation error](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-888#tabs-errors) because weeder and ghci are no longer available from `pkgs.haskellPackages`.
- [ ] Evaluation is taking a rather long time (5+ hours) in Hydra.
- [ ] Some of the Buildkite pipeline steps don't work with the updated Haskell.nix.

# Next PRs

- [ ] Fix failures in sqlite tests on Windows.
- [ ] Fix failure in launch command on Windows.
- [ ] Fix the UTF-8 text issues on Windows ⇒ #914.
- [ ] Add code which tells the windows command prompt to interpret ANSI color codes.
- [ ] Identify other test failures and fix.


Co-authored-by: Rodney Lorrimar <[email protected]>
@KtorZ KtorZ deleted the rvl/703/fix-windows-encoding branch October 28, 2019 18:56
iohk-bors bot added a commit that referenced this pull request Oct 29, 2019
@iohk-bors @rvl
Merge #888
e6c155d 
888: Update Haskell.nix r=KtorZ a=rvl

Relates to #703.

# Overview

- [x] Updates the Haskell.nix version so that we can build for windows again.
- [x] Keeps more or less the same structure as before - i.e. no IFD, no niv. I don't want too much change at once.
- [x] Does however update nixpkgs from iohk fork of 18.09 → upstream 19.03.
- [x] Provides a big bundle of scripts and tests that can be run on Windows.

[Hydra jobset](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-888)
[Hercules jobset](https://hercules-ci.com/github/input-output-hk/cardano-wallet)
[Link to download Windows testing bundle for PR #888](https://hydra.iohk.io/job/Cardano/cardano-wallet-pr-888/cardano-wallet-jormungandr-win64/latest)
[Link to download Windows testing bundle (master branch)](https://hydra.iohk.io/job/Cardano/cardano-wallet/cardano-wallet-jormungandr-win64/latest)

# Comments

@angerman Please help

- [x] Fix failure for missing `hsc2hs`
- [x] Fix a previous mistake that affects cross builds (failing to cross-compile bash for some reason)
- [x] Worked around an [evaluation error](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-888#tabs-errors) because weeder and ghci are no longer available from `pkgs.haskellPackages`.
- [ ] Evaluation is taking a rather long time (5+ hours) in Hydra.
- [ ] Some of the Buildkite pipeline steps don't work with the updated Haskell.nix.

# Next PRs

- [ ] Fix failures in sqlite tests on Windows.
- [ ] Fix failure in launch command on Windows.
- [ ] Fix the UTF-8 text issues on Windows ⇒ #914.
- [ ] Add code which tells the windows command prompt to interpret ANSI color codes.
- [ ] Identify other test failures and fix.


Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this pull request Nov 4, 2019
@iohk-bors @rvl
@cleverca22 @KtorZ
Merge #888 #952
aaead36 
888: Update Haskell.nix r=KtorZ a=rvl

Relates to #703.

# Overview

- [x] Updates the Haskell.nix version so that we can build for windows again.
- [x] Keeps more or less the same structure as before - i.e. no IFD, no niv. I don't want too much change at once.
- [x] Does however update nixpkgs from iohk fork of 18.09 → upstream 19.03.
- [x] Provides a big bundle of scripts and tests that can be run on Windows.

[Hydra jobset](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-888)
[Hercules jobset](https://hercules-ci.com/github/input-output-hk/cardano-wallet)
[Link to download Windows testing bundle for PR #888](https://hydra.iohk.io/job/Cardano/cardano-wallet-pr-888/cardano-wallet-jormungandr-win64/latest)
[Link to download Windows testing bundle (master branch)](https://hydra.iohk.io/job/Cardano/cardano-wallet/cardano-wallet-jormungandr-win64/latest)

# Comments

@angerman Please help

- [x] Fix failure for missing `hsc2hs`
- [x] Fix a previous mistake that affects cross builds (failing to cross-compile bash for some reason)
- [x] Worked around an [evaluation error](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-888#tabs-errors) because weeder and ghci are no longer available from `pkgs.haskellPackages`.
- [ ] Evaluation is taking a rather long time (5+ hours) in Hydra.
- [ ] Some of the Buildkite pipeline steps don't work with the updated Haskell.nix.
- [ ] Tests for windows are built not not executed. The tests should be run under wine.

# Next PRs

- [ ] Fix failures in sqlite tests on Windows ⇒ #918.
- [ ] Fix failure in launch command on Windows.
- [ ] Fix the UTF-8 text issues on Windows ⇒ #914.
- [ ] Add code which tells the windows command prompt to interpret ANSI color codes.
- [ ] Identify other test failures and fix.


952: review golden tests for legacy transaction and generate witnesses using jcli r=KtorZ a=KtorZ

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

N/A

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] I have reviewed the legacy transaction golden tests, now fully using jcli to also generate the witnesses (this feature was missing when the golden tests were initially implemented). 

- [x] I have updated the comment on private key to reflect the real format expected by jcli 

# Comments

<!-- Additional comments or screenshots to attach if any -->

A bit disappointing to some extend since nothing really has changed.. It seems that we were already providing the right output :( 

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: Michael Bishop <[email protected]>
Co-authored-by: KtorZ <[email protected]>
iohk-bors bot added a commit that referenced this pull request Nov 4, 2019
@iohk-bors @rvl @cleverca22
Merge #888
f75f38d 
888: Update Haskell.nix r=KtorZ a=rvl

Relates to #703.

# Overview

- [x] Updates the Haskell.nix version so that we can build for windows again.
- [x] Keeps more or less the same structure as before - i.e. no IFD, no niv. I don't want too much change at once.
- [x] Does however update nixpkgs from iohk fork of 18.09 → upstream 19.03.
- [x] Provides a big bundle of scripts and tests that can be run on Windows.

[Hydra jobset](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-888)
[Hercules jobset](https://hercules-ci.com/github/input-output-hk/cardano-wallet)
[Link to download Windows testing bundle for PR #888](https://hydra.iohk.io/job/Cardano/cardano-wallet-pr-888/cardano-wallet-jormungandr-win64/latest)
[Link to download Windows testing bundle (master branch)](https://hydra.iohk.io/job/Cardano/cardano-wallet/cardano-wallet-jormungandr-win64/latest)

# Comments

@angerman Please help

- [x] Fix failure for missing `hsc2hs`
- [x] Fix a previous mistake that affects cross builds (failing to cross-compile bash for some reason)
- [x] Worked around an [evaluation error](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-888#tabs-errors) because weeder and ghci are no longer available from `pkgs.haskellPackages`.
- [ ] Evaluation is taking a rather long time (5+ hours) in Hydra.
- [ ] Some of the Buildkite pipeline steps don't work with the updated Haskell.nix.
- [ ] Tests for windows are built not not executed. The tests should be run under wine.

# Next PRs

- [ ] Fix failures in sqlite tests on Windows ⇒ #918.
- [ ] Fix failure in launch command on Windows.
- [ ] Fix the UTF-8 text issues on Windows ⇒ #914.
- [ ] Add code which tells the windows command prompt to interpret ANSI color codes.
- [ ] Identify other test failures and fix.


Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: Michael Bishop <[email protected]>
@KtorZ KtorZ mentioned this pull request Nov 5, 2019
Closed
27 tasks
@KtorZ KtorZ added this to the Usability & Compatibility milestone Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathanknowles jonathanknowles jonathanknowles left review comments

@KtorZ KtorZ KtorZ approved these changes

Assignees

@rvl rvl

Labels
None yet
Projects
None yet
Milestone
Usability & Compatibility
Development

Successfully merging this pull request may close these issues.
3 participants
@rvl @KtorZ @jonathanknowles