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

Reset the cache-dir when a component is added to HscEnv #29

Merged
merged 1 commit into from
May 12, 2020

Conversation

fendor
Copy link
Collaborator

@fendor fendor commented May 8, 2020

Also, adds a prefix for the cache-dir: InstalledUnitId.

closes #15
closes #13

@mpickering
Copy link
Owner

This patch looks as I imagined it. We really need to refactor that 6-tuple into a record now!

exe/Main.hs Outdated Show resolved Hide resolved
exe/Main.hs Outdated Show resolved Hide resolved
@mpickering
Copy link
Owner

We discussed in IRC about what needs to happen for this patch to be finished.

  • A comment about the invariants which need to be preserved about the cache directories and about why multiple mains are not an issue.
  • Try to use just the filtered out inplace packages from the options in the salt rather than all components currently loaded.

@fendor
Copy link
Collaborator Author

fendor commented May 9, 2020

Indeed and I am on it.

@mpickering
Copy link
Owner

pretty sure it does.
In my logs:

"Making new HscEnv[\"main\"]"

Which is from:

            print ("Making new HscEnv" ++ (show inplace))

Right it does, but it might as well not as removeInplacePackage will never match on main as no components will ever depend on main. My idea instead was to look at which components removeInplacePackage actually removed and use those as the salt.

@fendor fendor force-pushed the bad-interface-files branch 2 times, most recently from 680c536 to dd57aae Compare May 11, 2020 14:03
@fendor fendor requested a review from mpickering May 11, 2020 14:04
Additionally a prefix for the cache-dir: InstalledUnitId.
Sort components for cache-dir so that order doesnt matter

Also remove "executable" component dependencies as they
are not important for interface files.
@fendor
Copy link
Collaborator Author

fendor commented May 12, 2020

Ready to merge, in my opinion.
cc @mpickering

@mpickering
Copy link
Owner

So the conclusion is it fixes some issues but there are still problems?

@fendor
Copy link
Collaborator Author

fendor commented May 12, 2020

for this pr, yes, but I will continue to venture into the problem on Cabal.
For future reference this error https://gist.github.com/fendor/93d8d7685a1c09371e6e2f1ed09bd26b
can be produced by opening cabal-install, followed by Cabal and then reopening only cabal-install component.

@mpickering
Copy link
Owner

Would be good if we could reproduce it on some smaller projects other than GHC and Cabal..

@fendor
Copy link
Collaborator Author

fendor commented May 12, 2020

I will provide a minimal reproducible project. I have a good idea how to reproduce it.

@fendor
Copy link
Collaborator Author

fendor commented May 12, 2020

A pretty small example: https://github.com/fendor/ghcide-bad-interface-files

@mpickering mpickering merged commit ca41320 into mpickering:hls May 12, 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.

Multi-component: Prepend package-id to cache directory name Multi component: Interface files get our of sync
2 participants