Skip to content

Retrie is broken on Windows #2051

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

Closed
bradrn opened this issue Jul 29, 2021 · 37 comments
Closed

Retrie is broken on Windows #2051

bradrn opened this issue Jul 29, 2021 · 37 comments
Labels
component: hls-retrie-plugin os: windows type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@bradrn
Copy link
Contributor

bradrn commented Jul 29, 2021

Cross-posted from emacs-lsp/lsp-haskell#128 on the recommendation of @michaelpj:

When I try to apply a code action such as ‘Unfold’ or ‘Fold’, I get the following message:

LSP :: Please open an issue in lsp-mode for implementing `11160:retrie:retrieCommand'.

(error "Not enough arguments for format string")

And the relevant part of the log from LSP:

[Trace - 06:55:01 PM] Received response 'workspace/executeCommand - (95)' in 0ms.
Result: {
  "code": -32602,
  "message": "error while parsing args for retrieCommand in plugin retrie: expected Bool, but encountered Null\narg = Object (fromList [(\"description\",String \"Unfold wSheet\"),(\"originatingFile\",String \"file:///c%3A/Users/bradn/Documents/Spreadsheet/supercell/src/Main.hs\"),(\"restrictToOriginatingFile\",Null),(\"rewrites\",Array [Object (fromList [(\"contents\",String \"Main.wSheet\"),(\"tag\",String \"Unfold\")])])])"
}
@jneira jneira added component: hls-retrie-plugin type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. labels Jul 29, 2021
@jneira
Copy link
Member

jneira commented Jul 29, 2021

Hi, thanks for the bug report, could you post the module where you are trying to trigger retry (or the relevant pragmas for retrie)?

@pepeiborra
Copy link
Collaborator

Could you also share a repro using VSCode?

@bradrn
Copy link
Contributor Author

bradrn commented Jul 29, 2021

@jneira It fails for every module I’ve tried.

(Also, it’s not just retrie either: all Wingman operations fail as well.)

@pepeiborra I don’t use VSCode regularly, but I’ll try to reproduce it.

@bradrn
Copy link
Contributor Author

bradrn commented Jul 29, 2021

@pepeiborra VSCode also gives me an error when I attempt to use Retrie or Wingman:

image

@pepeiborra
Copy link
Collaborator

Thanks @bradrn but we need more details to reproduce. Project, steps, versions of hls, ghc, etc.

@bradrn
Copy link
Contributor Author

bradrn commented Jul 29, 2021

Project: this seems to happen for any project, even just plain Stack simple template.

GHC version: again, this seems to be independent of GHC version from what I can tell.

HLS version:

> .\haskell-language-server-wrapper.exe --version
haskell-language-server version: 1.2.0.0 (GHC: 8.10.4) (PATH: C:\Users\bradn\Documents\Haskell\hls\haskell-language-server-wrapper.exe) (GIT hash: be2071e985cb417f984ab00a1aad76dee02d6d0b)

@pepeiborra
Copy link
Collaborator

Can you attach a screen recording too please?

@bradrn
Copy link
Contributor Author

bradrn commented Jul 29, 2021

(EDIT: The uploaded video seems to have cut off all the relevant parts of my original recording. I’ll try to upload a better video.)

@bradrn
Copy link
Contributor Author

bradrn commented Jul 29, 2021

@pepeiborra Here’s a better recording:

hls-error.mp4

As shown towards the end of the video most parts of HLS are working correctly; it is only retrie and parts of Wingman which are buggy.

@jneira
Copy link
Member

jneira commented Jul 29, 2021

hmm from the path of the call to hls in the cli it seems you are on windows, no?

@bradrn
Copy link
Contributor Author

bradrn commented Jul 29, 2021

Correct, I am on Windows 10.

@jneira
Copy link
Member

jneira commented Jul 29, 2021

ok, there is a existing issue about retrie and windows, although the symptoms are not the same afaics #533

@bradrn
Copy link
Contributor Author

bradrn commented Jul 29, 2021

Interesting… not sure if this is the same issue, but they certainly could be related. Do you know of any further tests I could try to diagnose this? (Though it’s getting late here and I don’t have time to do anything just right now.)

@jneira
Copy link
Member

jneira commented Jul 29, 2021

I just tried a retrie fold and i am continue getting the same error as #533. Maybe your way to start emacs makes xargs to be in scope? Your error would be triggered after avoiding that

@bradrn
Copy link
Contributor Author

bradrn commented Jul 29, 2021

Sorry, I’m not sure what you mean… are you saying that I should try removing xargs from PATH, or I should add it to PATH?

@jneira
Copy link
Member

jneira commented Jul 29, 2021

no, no, I think maybe you already have xargs in scope and the error you are getting is the next one
will try to reproduce yours

@jneira
Copy link
Member

jneira commented Jul 29, 2021

However wingman works correctly in my end

@isovector
Copy link
Collaborator

Logs for the wingman actions would be helpful as well. Thanks!

@bradrn
Copy link
Contributor Author

bradrn commented Aug 1, 2021

@isovector Sorry, which logs do you want? Because as far as I can tell the logs are the same for Retrie and Wingman — both give similar parse errors.

@isovector
Copy link
Collaborator

@bradrn maybe just the whole log!

@jneira jneira changed the title Retrie doesn’t work Retrie/Wingman doesn’t work Aug 2, 2021
@jneira
Copy link
Member

jneira commented Aug 2, 2021

Mmm, one thing that share retrie and wingman is ghc-exactprint usage iirc. No evidence to back the intuition though

@bradrn
Copy link
Contributor Author

bradrn commented Aug 2, 2021

Curiouser and curiouser. I tried to test the various code actions available to confirm what does and doesn’t work, and to my surprise I could not find a single Wingman command which failed (EDIT: found one, see next comment). I’m not sure why I reported that Wingman was failing, but whatever it was, it seems to be working perfectly now.

On the other hand, Retrie is still not working, but it seems to give different errors depending on which command I use:

  • ‘Unfold’ and ‘Fold’ both give the parse error mentioned above
  • ‘Unfold in current file’ gives no error, but has no effect either
  • ‘Fold in current file’ fails with ExitFailure 1.

@isovector I tried collecting logs while testing, but I doubt you’ll be interested in all 70000+ lines! I’m not even sure where I would share a file of that size.

@bradrn
Copy link
Contributor Author

bradrn commented Aug 2, 2021

OK, now I’m thoroughly stumped. I decided to try Wingman in another project, and this time round it failed, with an absolutely bizarre error message:

LSP :: `workspace/executeCommand' with `1836:tactics:tacticsDestructLambdaCaseCommand' failed.

(error "C:\\sr\\ghc-8.10.4\\hyphenation-0.8.1-f8b6bb68cfad4b10e18172bc9e3eb5622f323a16\\share\\hyph-en-us.hyp.txt.gz: openBinaryFile: does not exist (No such file or directory)")

Any ideas?

@jneira
Copy link
Member

jneira commented Aug 2, 2021

Ugh, do you have the package installed in that location? D:\sr seems the dir stack sets as %STACK_ROOT% by default when installed in windows, but ghc-8.10.4\\hyphenation-0.8.1-f8b6bb68cfad4b10e18172bc9e3eb5622f323a16 is the typical path cabal uses to install packages with the v2-command. Maybe did you delete it?

@jneira
Copy link
Member

jneira commented Aug 2, 2021

@isovector I tried collecting logs while testing, but I doubt you’ll be interested in all 70000+ lines! I’m not even sure where I would share a file of that size.

I think all lines emitted by wingman would have the plugin name, maybe extracting them from the log while reproducing the error (they should not be so much) would help

@bradrn
Copy link
Contributor Author

bradrn commented Aug 2, 2021

Ugh, do you have the package installed in that location?

The path C:\sr\ghc-8.10.4 doesn’t even exist on my machine. I’m not sure where it’s coming from, unless it’s hardcoded in the source (which it surely wouldn’t be‽).

I think all lines emitted by wingman would have the plugin name, maybe extracting them from the log while reproducing the error (they should not be so much) would help

Good idea, I’ll try this.

@jneira
Copy link
Member

jneira commented Aug 2, 2021

The path C:\sr\ghc-8.10.4 doesn’t even exist on my machine. I’m not sure where it’s coming from, unless it’s hardcoded in the source (which it surely wouldn’t be‽).

very interesting, those paths could be hardcoded while assembling the executable (although they are not directly in the source code as you suspect), how did you got the hls executable? did the vscode extension download for you?

@bradrn
Copy link
Contributor Author

bradrn commented Aug 2, 2021

As mentioned already I’m using Emacs; the executables are simply downloaded from the GitHub releases page.

@jneira
Copy link
Member

jneira commented Aug 2, 2021

As mentioned already I’m using Emacs; the executables are simply downloaded from the GitHub releases page.

oh, right, vscode downloads the executable from the same location. To investigate. I will try to dust off my emacs installation in my windows 10 laptop if i have time.

@isovector
Copy link
Collaborator

isovector commented Aug 2, 2021

You're running into #2015 on the Wingman side

@bradrn
Copy link
Contributor Author

bradrn commented Aug 3, 2021

Thanks @isovector!

@Tarmean
Copy link

Tarmean commented Aug 3, 2021

I've been meaning to look into retrie on windows. My current thinking is that this is an escaping issues when passing backslash-based paths as stdin to xargs

Prelude System.Process> readCreateProcessWithExitCode (shell "xargs grep  -l 'Var'") "C:/Users/name/Projects/natLog/library/Types.hs"
(ExitSuccess,"C:/Users/name/Projects/natLog/library/Types.hs\n","")
Prelude System.Process> readCreateProcessWithExitCode (shell "xargs grep  -l 'Var'") "C:\\Users\\name\\Projects\\natLog\\library\\Types.hs"
(ExitFailure 123,"","grep: C:UsersnameProjectsnatLoglibraryTypes.hs: No such file or directory\n")

Something similar happens when running this in powershell, but weirdly not in cmd.exe .

This command is build when retrie tries to filter down a list of paths here.

This command eventually runs this function in System.Process so maybe the comment is related.

The quickest fix probably would be to substitute \ with / in the paths in the plugin.

@jneira
Copy link
Member

jneira commented Aug 3, 2021

@Tarmean wow, thanks for the further investigation, the fix could be convert the windows paths to unix style ones and does not seem to be too complex, would you be interested in make a pr?

@jneira jneira added this to the 1.4.0 milestone Aug 3, 2021
@Tarmean
Copy link

Tarmean commented Aug 3, 2021

The path conversion was really easy. Unfortunately the current System.Process implementation (well, hPutStr) also seems to mangle newlines in the stdin channel of the created process on windows by turning '\n' into '\r\n'. This also breaks xargs.

The stdin handle is created as 'intPtrToPtr (-1)' and I don't know enough about how this magic works to say how badly things would break if one were to call hSetNewlineMode.

An more palatable hack alternative might be to inline the first xargs invocation, i.e. turning

readCreateProcessWithExitCode (shell "xargs grep -l 'Fizz' | xargs grep -l 'Buzz'") paths

into

readCreateProcessWithExitCode (shell "grep -l 'Fizz' $paths | xargs grep -l 'Buzz'") ""

Either way I'm currently out of time so I will have to experiment more later.

@jneira
Copy link
Member

jneira commented Aug 3, 2021

Unfortunately the current System.Process implementation (well, hPutStr) also seems to mangle newlines in the stdin channel of the created process on windows by turning '\n' into '\r\n'.

I had to add a function to try to avoid that:

let writeFileUTF8NoNewLineTranslation file txt =
withFile file WriteMode $ \h -> do
hSetEncoding h utf8
hSetNewlineMode h noNewlineTranslation
hPutStr h (T.unpack txt)

@Tarmean
Copy link

Tarmean commented Aug 3, 2021

Replacing the usage of getTargetFiles with this version works on windows, the additions are filterChain0 and toUnixFilepath.

This is a gist and not a pullrequest to retrie because I noticed a couple of oddities and want to double check they aren't my fault. I think the weirdness are existing bugs in retrie, though

  • Identifiers with ' aren't found because it generates searches like grep -L 'P''
  • Operators are treated really weirdly. Might be because fixity isn't loaded? (Edit: Maybe only for builtins?)
  • Retrie always demands multiple spaces so a rule foo [] doesn't fire if the occurrence is foo[], similar with superfluous brackets

Here is an example of weird operator behavior

{-# RULES "stepSum" forall x y. sum  (x : y) = x + (sum y) #-}
foo :: Int
foo = sum (1 : 2 : 3 : [])

foo :: Int
foo = 1 : 2 : 3 + (sum [])

@isovector isovector changed the title Retrie/Wingman doesn’t work Retrie is broken on Windows Aug 3, 2021
@jneira jneira removed this from the 1.4.0 milestone Sep 16, 2021
@jneira
Copy link
Member

jneira commented Jan 22, 2022

I think this was closed by #2513, @Tarmean thanks!

@jneira jneira closed this as completed Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: hls-retrie-plugin os: windows type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

No branches or pull requests

5 participants