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

Initial windows support changes #48

Merged
merged 3 commits into from
Dec 20, 2018
Merged

Initial windows support changes #48

merged 3 commits into from
Dec 20, 2018

Conversation

btrepp
Copy link
Collaborator

@btrepp btrepp commented Dec 19, 2018

Solves getting compilation working, and making bundle work on windows as per #47

PR'd for code review purposes, I'm not sure if these are the best solutions and won't cause issues for non windows machines.

Unfortunately the encoding issue seems trickier, I'm not sure whether embedding the template or writing it to disk is causing the bad characters to be created, and my research into this has failed (so I might need a haskell guru to help fix this part). I could just change packages.dhall to be ascii, but I feel like this is weak solution as dhall supports unicode, so we should be able to work with that template file.

Copy link
Member

@f-f f-f left a comment

Choose a reason for hiding this comment

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

@btrepp thanks!

I left some comments, but this should be pretty good, it looks like Travis is happy with this, so other OSes didn't break :)

Re the encoding problem: the issue might be in the line after the one you linked, in which we format the file inplace with Unicode. It's worth trying to remove that line (since it doesn't actually affect the result of the operation) and see if that fixes it. If not, then the problem is in how Turtle writes the file (which I find unlikely, since it's a basic hPutStr, source here)

app/Spago.hs Outdated Show resolved Hide resolved
stack.yaml Show resolved Hide resolved
@btrepp
Copy link
Collaborator Author

btrepp commented Dec 19, 2018

So on the encoding issue, commenting out that line causes spago init to successfully complete (I think before the error I get was the format line you mentioned crashing) but on disk (opened with vim) the packages.dhall is still corrupted. This means spago install dies with the error init used to have.

My other thoughts might be how the file is being embedded in Templates.hs (this appears to be literate haskell? well outside of my realm now :)).

@f-f
Copy link
Member

f-f commented Dec 19, 2018

@btrepp oh right: we use file-embed to read the files (and yes, that uses Template Haskell); it looks like there's an issue open already about Unicode encoding

Stack can't resolve dependencies correctly,
recommended action was to add this line
to stack.yaml.

This may be better fixed upstream in turtle,
as it seems stack comes with a newer version
of win32 than turtle supports
The quotes caused issues on windows. Purs would
complain about having no files.

Changing the quote style fixes the command
on windows
templates/packages.dhall Outdated Show resolved Hide resolved
@f-f f-f merged commit cf3b59b into purescript:master Dec 20, 2018
@f-f
Copy link
Member

f-f commented Dec 20, 2018

Thanks @btrepp! 👏

elliotdavies pushed a commit to elliotdavies/spago that referenced this pull request Jul 1, 2019
* Fixes stack build on windows

Stack can't resolve dependencies correctly,
recommended action was to add this line
to stack.yaml.

This may be better fixed upstream in turtle,
as it seems stack comes with a newer version
of win32 than turtle supports

* Fixes bundle on windows

The quotes caused issues on windows. Purs would
complain about having no files.

Changing the quote style fixes the command
on windows
f-f added a commit that referenced this pull request Sep 27, 2023
Co-authored-by: Fabrizio Ferrai <fabrizio.ferrai@gmail.com>
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.

None yet

4 participants