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

StylishHaskell shifting the working dir, this could lead to race condition #4234

Open
soulomoon opened this issue May 16, 2024 · 7 comments
Open
Labels
component: formatters priority: high High priority item status: blocked Not actionable, because blocked by upstream/GHC etc. type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@soulomoon
Copy link
Collaborator

soulomoon commented May 16, 2024

StylishHaskell shifting the working dir, it could make other part of hls depending on cwd result in error in some race condition.

Such as Hlint, Stan, Template haskel, session loader etc. We should not shifting the working dir after initilization.

@soulomoon soulomoon added type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. status: needs triage labels May 16, 2024
@soulomoon
Copy link
Collaborator Author

#4231 Could alleviate the some potential race condition. But there are still a lot of place we could not clean up

@soulomoon
Copy link
Collaborator Author

soulomoon commented May 16, 2024

I am wondering if there is an alternaive function than loadConfig that does not depend on cwd to load the stylish config.

@michaelpj
Copy link
Collaborator

We could ask upstream to expose a version that takes the directory as an argument.

@soulomoon
Copy link
Collaborator Author

soulomoon commented May 17, 2024

Yep, I've craete an issue there

@fendor fendor added component: formatters priority: high High priority item status: blocked Not actionable, because blocked by upstream/GHC etc. and removed status: needs triage labels Jun 26, 2024
@fendor
Copy link
Collaborator

fendor commented Jun 26, 2024

This is quite bad 😅 Perhaps we can copy-paste the loadConfig code from stylish-haskell and fix it ourselves until upstream fixed it?

@jhrcek
Copy link
Collaborator

jhrcek commented Jun 26, 2024

Seems like stylish's loadConfig already supports specifying directory as argument (Maybe FilePath). We just need to pass Just instead of Nothing, or am I missing something?

@jhrcek
Copy link
Collaborator

jhrcek commented Jun 26, 2024

Ok, the Maybe FilePath is the path to the stylish config file itselt, not to a "starting directory" as I imagined.
So this will require some upstream changes after all 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: formatters priority: high High priority item status: blocked Not actionable, because blocked by upstream/GHC etc. type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

4 participants