Skip to content

Conversation

@gibranrosa
Copy link
Contributor

Description of the change

This pull request is just to make more visible the workaround to #533 that make Spago work with Windows 10, as commented by @kakkun61 (#533 (comment)) .
I have just removed the fixation of UTF-8 encoding in the main.

Additionally, I have made a Release with that change to make it easy to others: https://github.com/gibranrosa/spago/releases/tag/0.14-windows10

Checklist:

  • Added the change to the "Unreleased" section of the changelog
  • Added some example of the new feature to the README
  • Added a test for the contribution (if applicable)

P.S.: the above checks are not compulsory to get a change merged, so you may skip them. However, taking care of them will result in less work for the maintainers and will be much appreciated 😊

@klntsky
Copy link
Collaborator

klntsky commented Feb 22, 2020

I guess the next comment proposes a better solution for this problem than just using system encoding everywhere[0]. However, PS compiler uses UTF-8 for stdout and stderr as well, so why shouldn't spago do the same?

[0] Note that PS files are always UTF-8 and unicode synonyms can be used for ->, =>, etc.

@gibranrosa
Copy link
Contributor Author

"This pull request is just to make more visible the workaround to #533 that make Spago work with Windows 10, as commented by @kakkun61 (#533 (comment)) ."

I want to make that part emphasized. JUST A WORKAROUND. Since this is just not working with Windows 10.

And my Build Release could be used for now.

@hdgarrood
Copy link
Contributor

Note that the relevant issue is tagged “defect” so this is considered a bug by @f-f, and correctly so too, in my opinion. If a similar error can be found in purs too then I would consider that a bug in purs. We can’t just deny the existence of alternative system encodings because they’re rare amongst English speaking users and because we find them inconvenient.

@f-f
Copy link
Member

f-f commented Feb 22, 2020

This is a complicated topic, so I'll first point out that we've been very careful to make changes on this, as it might easily break lots of workflows. However, I'm also aware that people from non-English-speaking countries usually have trouble getting Spago to run (e.g. I got lots of reports from Japan), so I'd really like to see this fixed.

I have a few notes so far:

  • in the interest of avoiding hostility I'd like to ask everyone to abstain from using GitHub reactions in this thread (both positive and negative), since they can easily be misinterpreted by readers and then polarize the discussion, which I'd like to keep on the technical side instead
  • @gibranrosa thanks for opening this PR - this is clearly an outstanding bug (as Harry noted above), and I understand the will to release a workaround, but as a historical policy we would not like to keep around PRs "for visibility" as we like to keep the backlog quite curated.
    I'd also not like to merge this workaround as it is, because it will break other things (i.e. there's a reason why that line was added at the time, look at CI), so I think your first post would be more useful as a comment to spago init and then spago build throw hGetContents error #533 instead of a PR - people that need this fix will easily find it there
  • as for the "proper" fix for this, I think @kakkun61 and @klntsky are on point here:
    we should fix UTF-8 for file input/output, and use the local encoding for Spago's stdin/stdout/stderr. So it might be enough to replace the line in question with code that fixes UTF-8 only for files.
    However! It turns out that sometimes we do have to fix the environment for executables that we're calling, see When trying to publish, get git: recoverEncode: invalid argument (invalid character) #507.
    It might be enough to implement the fix above (because the problem might come from Spago demanding UTF-8 and getting whatever encoding), but that's an additional problem we have to watch out for.
    I have two questions about this:

@bbarker
Copy link
Contributor

bbarker commented Feb 24, 2020

I have this on a slack reminder, and I will keep reminding myself until I get around to trying it (hopefully later this week!)

@bbarker
Copy link
Contributor

bbarker commented Mar 5, 2020

There appears to be a new issue for me, when running in nix (using a nix-shell if that matters):

]$ spago bump-version --no-dry-run 4.0.0
[info] Generating a new Bower config using the package set versions..
spago: ./packages.dhall: hGetContents: invalid argument (invalid byte sequence)

]$ head packages.dhall 
{-
Welcome to your new Dhall package-set!

Below are instructions for how to edit this file for most use
cases, so that you don't need to know Dhall to use it.

## Warning: Don't Move This Top-Level Comment!

Due to how `dhall format` currently works, this comment's
instructions cannot appear near corresponding sections below

See: koalaman/shellcheck#324 (comment) for possibly related fix

@f-f
Copy link
Member

f-f commented Mar 5, 2020

@bbarker what happens there is exactly the reason why we started fixing the locale to UTF8 (and it's the same failure that happens in the Windows CI on this PR): Dhall reads files with the locale encoding, while we'd like it to use UTF8 encoding.
So I guess we have a couple of options here:

  • patch Dhall upstream to always read files with UTF8 (but it should already be doing this, I recall several fixes going in for that..)
  • or always read the files on the Spago side, and only then pass them to Dhall

@bbarker do you have a shell.nix file that reproduces the issue?

@bbarker
Copy link
Contributor

bbarker commented Mar 6, 2020

@f-f I just did nix-shell -p stack on Ubuntu. System info (Nix on Ubuntu 18):

  • system: "x86_64-linux"
  • host os: Linux 4.15.0-76-generic, Ubuntu, 18.04.3 LTS (Bionic Beaver)
  • multi-user?: no
  • sandbox: no
  • version: nix-env (Nix) 2.3.1
  • channels(brandon): "nixpkgs-19.09.1774.a3070689aef, nixos-19.09.1774.a3070689aef"
  • nixpkgs: /nix/var/nix/profiles/per-user/brandon/channels/nixpkgs

@bbarker
Copy link
Contributor

bbarker commented Mar 6, 2020

Also, output of env:

HOST_PATH=/nix/store/9rmkkh1ylmdqn3cqbssnndw5fg51zyl1-stack-2.1.3.1/bin:/nix/store/9v78r3afqy9xn9zwdj9wfys6sk3vc01d-coreutils-8.31/bin:/nix/store/0zdsw4qdrwi41mfdwqpxknsvk9fz3gkb-findutils-4.7.0/bin:/nix/store/dys98skw6ifw5qsqrhjxjjiydyfbg3nq-diffutils-3.7/bin:/nix/store/g2h4491kab7l06v9rf1lnyjvzdwy5ak0-gnused-4.7/bin:/nix/store/71y5ddyz8vmsw9wgi3gzifcls53r60i9-gnugrep-3.3/bin:/nix/store/84yafwjc6sga23vwsbahhymikjhfbnw4-gawk-5.0.1/bin:/nix/store/aawf0q16ql39w2gwv52qyjfzgbg5f22r-gnutar-1.32/bin:/nix/store/afyqk8219zfv5and3pqahzvn6bpmx3dq-gzip-1.10/bin:/nix/store/iws7dm1jij4lv09mbpwf9i8lc1d4b798-bzip2-1.0.6.0.1-bin/bin:/nix/store/j8fq1ksp37w88rx80blzazldi17f3x7s-gnumake-4.2.1/bin:/nix/store/rm1hz1lybxangc8sdl7xvzs5dcvigvf7-bash-4.4-p23/bin:/nix/store/lmi2nqb1cfzr3ps2zkq97q88zzkhp4mj-patch-2.7.6/bin:/nix/store/y04xj2mbc66zs53fackn1wwlrv31bv3y-xz-5.2.4-bin/bin
LS_COLORS=rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=00:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arc=01;31:*.arj=01;31:*.taz=01;31:*.lha=01;31:*.lz4=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.tzo=01;31:*.t7z=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.dz=01;31:*.gz=01;31:*.lrz=01;31:*.lz=01;31:*.lzo=01;31:*.xz=01;31:*.zst=01;31:*.tzst=01;31:*.bz2=01;31:*.bz=01;31:*.tbz=01;31:*.tbz2=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.war=01;31:*.ear=01;31:*.sar=01;31:*.rar=01;31:*.alz=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.cab=01;31:*.wim=01;31:*.swm=01;31:*.dwm=01;31:*.esd=01;31:*.jpg=01;35:*.jpeg=01;35:*.mjpg=01;35:*.mjpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.webm=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=00;36:*.au=00;36:*.flac=00;36:*.m4a=00;36:*.mid=00;36:*.midi=00;36:*.mka=00;36:*.mp3=00;36:*.mpc=00;36:*.ogg=00;36:*.ra=00;36:*.wav=00;36:*.oga=00;36:*.opus=00;36:*.spx=00;36:*.xspf=00;36:
propagatedBuildInputs=
stdenv=/nix/store/qghrkvk86f9llfkcr1bxsypqbw1a4qmw-stdenv-linux
DISPLAY=:1
HOSTNAME=beb82dell0
OLDPWD=/home/brandon
out=/nix/store/7jdgl5c9klj7a9in8lbz2l78ynyk5qvp-shell
GIT_SSL_CAINFO=/etc/ssl/certs/ca-certificates.crt
JAVA_HOME=/nix/store/8im5gwg0yvrx84hfdr2v133qrl7zyil9-openjdk-8u222-ga/lib/openjdk
CONFIG_SHELL=/nix/store/rm1hz1lybxangc8sdl7xvzs5dcvigvf7-bash-4.4-p23/bin/bash
buildInputs=/nix/store/9rmkkh1ylmdqn3cqbssnndw5fg51zyl1-stack-2.1.3.1
builder=/nix/store/rm1hz1lybxangc8sdl7xvzs5dcvigvf7-bash-4.4-p23/bin/bash
SSH_AUTH_SOCK=/tmp/ssh-NN4RBjgSst0m/agent.1310
CC=gcc
READELF=readelf
buildCommandPath=/tmp/nix-shell-20788-0/.attr-0
CURL_CA_BUNDLE=/etc/ssl/certs/ca-certificates.crt
STRIP=strip
depsBuildTarget=
OBJCOPY=objcopy
propagatedNativeBuildInputs=
depsTargetTarget=
system=x86_64-linux
PWD=/home/brandon/workspace/spago
NIX_PATH=/nix/var/nix/profiles/per-user/brandon/channels/
HOME=/home/brandon
TMP=/run/user/1000
strictDeps=
SSH_AGENT_PID=1312
NIX_ENFORCE_NO_NATIVE=1
RANLIB=ranlib
OBJDUMP=objdump
AS=as
AR=ar
NIX_CC=/nix/store/291ldi6fqsbmkbvbs8is4mcg3jb64ld4-gcc-wrapper-8.3.0
depsBuildBuild=
NIX_STORE=/nix/store
outputs=out
NIX_CC_WRAPPER_x86_64_unknown_linux_gnu_TARGET_HOST=1
configureFlags=
TMPDIR=/run/user/1000
name=shell
NIX_BINTOOLS_WRAPPER_x86_64_unknown_linux_gnu_TARGET_HOST=1
doInstallCheck=
doCheck=
IN_NIX_SHELL=impure
IDEA_JDK=/usr/lib/jvm/zulu-8-amd64
NIX_BINTOOLS=/nix/store/gwwycf3w6cbj0gd2mpgblrdjc24f3cys-binutils-wrapper-2.31.1
NM=nm
HOME_TEMPLATE=/template/brandon
depsHostHostPropagated=
CXX=g++
depsBuildBuildPropagated=
SHELL=/nix/store/lb3hli8d9536g45mndwfwyi6fpny0blh-bash-interactive-4.4-p23/bin/bash
TERM=xterm
NIX_LDFLAGS=-rpath /nix/store/7jdgl5c9klj7a9in8lbz2l78ynyk5qvp-shell/lib64 -rpath /nix/store/7jdgl5c9klj7a9in8lbz2l78ynyk5qvp-shell/lib 
TEMPDIR=/run/user/1000
shell=/nix/store/rm1hz1lybxangc8sdl7xvzs5dcvigvf7-bash-4.4-p23/bin/bash
NIX_HARDENING_ENABLE=fortify stackprotector pic strictoverflow format relro bindnow
NIX_INDENT_MAKE=1
passAsFile=buildCommand
NIX_SSL_CERT_FILE=
SHLVL=2
nixenv=. /nixenv/brandon/.nix-profile/etc/profile.d/nix.sh
NIX_BUILD_CORES=8
SOURCE_DATE_EPOCH=1
TEMP=/run/user/1000
STRINGS=strings
XDG_RUNTIME_DIR=/run/user/1000
depsHostHost=
PATH=/nix/store/lb3hli8d9536g45mndwfwyi6fpny0blh-bash-interactive-4.4-p23/bin:/nix/store/2xwxj5qrrc71asdk1wyq19nz9k845pzs-patchelf-0.9/bin:/nix/store/291ldi6fqsbmkbvbs8is4mcg3jb64ld4-gcc-wrapper-8.3.0/bin:/nix/store/62x7m20m7lm8y8s17cbgha0sf3cmma19-gcc-8.3.0/bin:/nix/store/hblpx8x5w88igmwa4ydnsnb65s363lji-glibc-2.27-bin/bin:/nix/store/9v78r3afqy9xn9zwdj9wfys6sk3vc01d-coreutils-8.31/bin:/nix/store/gwwycf3w6cbj0gd2mpgblrdjc24f3cys-binutils-wrapper-2.31.1/bin:/nix/store/ajrrkivdfvp8dp4vdg5hp1h5hblmanc9-binutils-2.31.1/bin:/nix/store/hblpx8x5w88igmwa4ydnsnb65s363lji-glibc-2.27-bin/bin:/nix/store/9v78r3afqy9xn9zwdj9wfys6sk3vc01d-coreutils-8.31/bin:/nix/store/9rmkkh1ylmdqn3cqbssnndw5fg51zyl1-stack-2.1.3.1/bin:/nix/store/9v78r3afqy9xn9zwdj9wfys6sk3vc01d-coreutils-8.31/bin:/nix/store/0zdsw4qdrwi41mfdwqpxknsvk9fz3gkb-findutils-4.7.0/bin:/nix/store/dys98skw6ifw5qsqrhjxjjiydyfbg3nq-diffutils-3.7/bin:/nix/store/g2h4491kab7l06v9rf1lnyjvzdwy5ak0-gnused-4.7/bin:/nix/store/71y5ddyz8vmsw9wgi3gzifcls53r60i9-gnugrep-3.3/bin:/nix/store/84yafwjc6sga23vwsbahhymikjhfbnw4-gawk-5.0.1/bin:/nix/store/aawf0q16ql39w2gwv52qyjfzgbg5f22r-gnutar-1.32/bin:/nix/store/afyqk8219zfv5and3pqahzvn6bpmx3dq-gzip-1.10/bin:/nix/store/iws7dm1jij4lv09mbpwf9i8lc1d4b798-bzip2-1.0.6.0.1-bin/bin:/nix/store/j8fq1ksp37w88rx80blzazldi17f3x7s-gnumake-4.2.1/bin:/nix/store/rm1hz1lybxangc8sdl7xvzs5dcvigvf7-bash-4.4-p23/bin:/nix/store/lmi2nqb1cfzr3ps2zkq97q88zzkhp4mj-patch-2.7.6/bin:/nix/store/y04xj2mbc66zs53fackn1wwlrv31bv3y-xz-5.2.4-bin/bin:/nixenv/brandon/.nix-profile/bin:/nixenv/brandon/.nix-profile/sbin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/home/brandon/go/bin:/home/brandon/go/bin:/home/brandon/.local/bin
NIX_BUILD_TOP=/run/user/1000
PS1=\n\[\033[1;32m\][nix-shell:\w]\$\[\033[0m\] 
depsBuildTargetPropagated=
SIZE=size
nativeBuildInputs=
LD=ld
ENVSDIR=/nixenv/brandon
patches=
depsTargetTargetPropagated=
_=/nix/store/9v78r3afqy9xn9zwdj9wfys6sk3vc01d-coreutils-8.31/bin/env

@gibranrosa
Copy link
Contributor Author

Real solution is in #595

@gibranrosa gibranrosa closed this Mar 19, 2020
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