Import paths are relative to cradle

[pepeiborra]

I noticed ghcide HEAD was broken on the ghcide submodule of the haskell-language-server repo, after the recent changes to import paths. Hopefully this fixes the issue

Fixes #780

[pepeiborra]

Once again I had to special case the implicit cradle...

[pepeiborra]

@wz1000 check that my change to not filter by doesFileExist when extending the known files doesn't break anything

[wz1000]

Why remove the check that the files exist? The file paths are still coming from the same source as far as I can tell, last I remember they were generated by something like a cartesian product of import paths and module names.

[wz1000]

I think the files eventually go here, so this is not correct: https://github.com/haskell/ghcide/blob/master/src/Development/IDE/Core/Rules.hs#L374

[jneira]

Once again I had to special case the implicit cradle...

would that be needed after #782?

[pepeiborra]

Why remove the check that the files exist? The file paths are still coming from the same source as far as I can tell, last I remember they were generated by something like a cartesian product of import paths and module names.

I'm removing the check that the files exist (when extending the set of known files) because otherwise ghcide doesn't pick up new modules created after updating the cradle.

Is the Cartesian product logic in hie-bios or in ghc?

[pepeiborra]

I think the files eventually go here, so this is not correct: https://github.com/haskell/ghcide/blob/master/src/Development/IDE/Core/Rules.hs#L374

this doesn't seem related

[pepeiborra]

Once again I had to special case the implicit cradle...

would that be needed after #782?

I don't know, it depends on what is changed in #782.

[wz1000]

I think the files eventually go here, so this is not correct: https://github.com/haskell/ghcide/blob/master/src/Development/IDE/Core/Rules.hs#L374

this doesn't seem related

It is. We call use_ GetModSummaryWithoutTimestamps on each file name put into the known files list, which will fail every time if any of those files don't exist.

[wz1000]

Why remove the check that the files exist? The file paths are still coming from the same source as far as I can tell, last I remember they were generated by something like a cartesian product of import paths and module names.

I'm removing the check that the files exist (when extending the set of known files) because otherwise ghcide doesn't pick up new modules created after updating the cradle.

Is the Cartesian product logic in hie-bios or in ghc?

It's in ghcide: https://github.com/haskell/ghcide/blob/master/session-loader/Development/IDE/Session.hs#L359

[pepeiborra]

Is the Cartesian product logic in hie-bios or in ghc?

It's in ghcide: https://github.com/haskell/ghcide/blob/master/session-loader/Development/IDE/Session.hs#L359

I see your point now. I've restored the filtering and implemented an alternative solution for missing modules which preserves the old behaviour.

EDIT: Hmm, I don't think the alternative solution works. ghcide will not locate a missing module when it is created because it is not in the set of known files. Perhaps extending the set of known files to a map of candidates will do it.

[pepeiborra]

The problem is that we are trying to use the KnownFiles to determine what imports are part of the local project, and the KnownFiles do not have enough information for that.

The solution is to enrich the KnownFiles to become KnownTargets, which provides a mapping from every target to a list of normalised file paths that do exist for them. The keys of the mapping are the targets and can be used to determine local imports, whereas the values of the mapping are still used to drive the module graph

[wz1000]

Won't we still end up calling rawDependencyInformation with a bunch of non existent files?

[pepeiborra]

Won't we still end up calling rawDependencyInformation with a bunch of non existent files?

I restored the filtering, so I don't think so

[azure-pipelines]

Command 'ru' is not supported by Azure Pipelines.

Supported commands

• help:
• Get descriptions, examples and documentation about supported commands
• Example: help "command_name"
• list:
• List all pipelines for this repository using a comment.
• Example: "list"
• run:
• Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
• Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
• where:
• Report back the Azure DevOps orgs that are related to this repository and org
• Example: "where"

See additional documentation.

[pepeiborra]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[pepeiborra]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[pepeiborra]

I have disabled a flaky test that was blocking this PR. Have you noticed this test failing before?

[wz1000]

Yes, I saw it for the first time yesterday. Rerunning the job fixed it. Maybe saving ifaces exposed this? Switching the order of iface-error-1 and iface-error-2 could be one way to tell.

…

On Sat, 12 Sep, 2020, 13:25 Pepe Iborra, ***@***.***> wrote: I have disabled a flaky test that was blocking this PR. Have you noticed this test failing before? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#781 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABEW5UXR6RTX6GNKTQPLZOLSFMSL7ANCNFSM4RFN7X4Q> .

[pepeiborra]

Yes, I saw it for the first time yesterday. Rerunning the job fixed it. Maybe saving ifaces exposed this? Switching the order of iface-error-1 and iface-error-2 could be one way to tell.
…

I think it's likely related to saving ifaces, I don't recall seeing it before, but maybe CI history can confirm. Could you investigate? The failure could be indicative of a real bug.

I have not managed to reproduce it locally though, it's very annoying.