Migrate to OsPath

[kokobd]

As suggested by @pepeiborra, we have a better alternative to FilePath now. See: https://hasufell.github.io/posts/2022-06-29-fixing-haskell-filepaths.html

And changing the implementation of NormalizedFilePath might greatly impact HLS performance, according to the existing comment.

lsp/lsp-types/src/Language/LSP/Types/Uri.hs

Lines 158 to 160 in a41954e

	-- This is one of the most performance critical parts of ghcide, do not
	-- modify it without profiling.
	data NormalizedFilePath = NormalizedFilePath NormalizedUri !FilePath

I'm planning to:

1. Modify lsp-types to use OsPath in a fork
2. Create a hls branch using that fork
3. Profiling, to verify the performance (especially memory) impact

[kokobd]

Converting between OsPath and FilePath requires the ability to express failure. So we can not preserve the signature of functions like this, if NormalizedFilePath contains OsPath but not FilePath.

fromNormalizedFilePath :: NormalizedFilePath -> FilePath

To maintain backward compatibility, I will create a new type NormalizedOsPath, and change HLS to use it.

[michaelpj]

To maintain backward compatibility, I will create a new type NormalizedOsPath, and change HLS to use it.

I do not think we should worry too much about this. We don't have that many clients, and I'm pretty close to releasing a change that will break essentially the entire package 😅 I'm happy to keep releasing major versions as we break things

[kokobd]

But keep in mind that using the new filepath requires projects that depends on ghc to upgrade to GHC 9.6, which is not possible for HLS in the near future. NormalizedFilePath is just a standalone type that resides in lsp-types, and is not referenced by other types or functions in lsp. So maybe we should just leave it there to avoid trouble.

Pepe Iborra suggested creating a package filepath-compat which is not tied to newer version of GHC. I think we should restrict usages of filepath-compat to HLS only (it's an application, after all), and don't depend on it here in lsp.

So, I believe we can create NormalizedOsPath in HLS, and move it here when needed.

[michaelpj]

Hmm, could we do a CPP dance to have a NormalizedFilePath that uses OsPath on newer versions of GHC? Or is that just going to be a nightmare?

[kokobd]

could we do a CPP dance

We can, and I think it won't be too hard. But clients will see different API when using different versions of GHC, that might be confusing in my mind. After all, filepath itself just add a new type OsPath, leaving FilePath alone. NormalizedFilePath isn't too much more than a normalized FilePath, so maybe it's reasonable to follow filepath, to conditionally add a NormalizedOsPath on newer GHC versions?

[michaelpj]

Ugh, good point. We could have a single NormalizedFilePath and have filePathToNormalizedFilePath and osPathToNormalizedFilePath on 9.6+ only?

[hasufell]

I'm not sure what NormalizedFilePath is but you can easily maintain a uniform API, e.g. by having only one extraction function NormalizedFilePath -> FilePath. The OsPath codepath would then invoke decodeFS, while the other would probably just unwrap the newtype.

However... do mind that the point of OsPath is to avoid encoding/decoding at the FFI boundary... so you shoudn't convert to or from String yourself in the internal API. That would signal a problem with the approach.