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

Multi component ghcide setup #374

Merged
merged 21 commits into from
Nov 7, 2019
Merged

Multi component ghcide setup #374

merged 21 commits into from
Nov 7, 2019

Conversation

robrix
Copy link
Contributor

@robrix robrix commented Nov 1, 2019

This PR:

  • Depends on Multi component repl #373.
  • Adds a couple of scripts for computing the correct flags for ghcide to use to load all of our components.
  • Adds a hie.yaml file configuring ghcide thus.

@robrix robrix changed the base branch from multi-component-repl to master November 1, 2019 15:19
Comment on lines +8 to +9
root="$(pwd)"
ghc_version="$(ghc --numeric-version)"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve attempted to compute what I can about the project, but I don’t know how to efficiently and robustly parse cabal.project for the .cabal files or the .cabal files for the package version numbers, or how otherwise to ask cabal for this information since it seems reluctant to tell me its secrets.

Comment on lines +45 to +46
# Emit package flags from the environment file, removing comments & prefixing with -
cabal exec --builddir=$repl_builddir -v0 bash -- -c 'cat $GHC_ENVIRONMENT' | grep -v '^--' | sed -e 's/^/-/'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than using the existing .ghc.environment* file in the project root, I’ve opted to use cabal exec to get the package flags out. I can’t just use echo "-package-conf=$(cabal exec -v0 bash -- -c 'echo $GHC_ENVIRONMENT')" because cabal makes a new file in dist-newstyle/tmp and deletes it as soon as the exec’d program exits, meaning that it would be removed before ghcide could find it.

Comment on lines +30 to +40
echo "-i$root/semantic-analysis/src"
echo "-i$root/semantic-ast/src"
echo "-i$root/semantic-core/src"
echo "-i$root/semantic-java/src"
echo "-i$root/semantic-json/src"
echo "-i$root/semantic-python/src"
echo "-i$root/semantic-tags/src"
echo "-i$root/app"
echo "-i$root/src"
echo "-i$root/bench"
echo "-i$root/test"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty similar to what we were doing in .ghci.semantic (added in #373), but distinct in that VS Code runs ghcide with the PWD env var set to / and apparently the -i flags are resolved relative to that environment variable rather than the actual, you know, working directory, and thus we have to use absolute paths.

Comment on lines +21 to +24
echo "-outputdir $repl_builddir/build/x86_64-osx/ghc-$ghc_version/semantic-0.8.0.0/build"
echo "-odir $repl_builddir/build/x86_64-osx/ghc-$ghc_version/semantic-0.8.0.0/build"
echo "-hidir $repl_builddir/build/x86_64-osx/ghc-$ghc_version/semantic-0.8.0.0/build"
echo "-stubdir $repl_builddir/build/x86_64-osx/ghc-$ghc_version/semantic-0.8.0.0/build"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don’t instruct ghcide to use -fwrite-interface or -fobject-code, but putting these here allows us to share more of the logic for determining the compiler version &c.

Comment on lines +11 to +16
if [ "$1" == "--builddir" ]; then
repl_builddir="$2"
shift 2
else
repl_builddir=dist-newstyle
fi
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can’t pass flags into this program from hie.yaml, but I wanted to reuse it from script/repl without duplicating much of it or adding redundant wrapper scripts, so I’ve made this default to dist-newstyle if it’s unspecified.

Comment on lines +1 to +2
#!/bin/bash
# Computes the paths to files causing changes to the ghci flags. You probably won’t be running this yourself, but rather ghcide will via configuration in hie.yaml.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to the listed files might result in different package flags for ghcide to learn of, so we tell it to watch them.

@@ -11,5 +11,5 @@ repl_builddir=dist-repl
if [[ ! -d $repl_builddir ]]; then
echo "$repl_builddir does not exist, first run 'cabal repl --builddir=$repl_builddir', exit, and then re-run $0"
else
cabal exec --builddir=$repl_builddir ghci -- -ghci-script=.ghci.semantic $@
cabal exec --builddir=$repl_builddir ghci -- -ghci-script=.ghci.semantic $(script/ghci-flags --builddir "$repl_builddir") -no-ignore-dot-ghci $@
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We reuse script/ghci-flags from script/repl in order to share a bunch of the path computation logic.

  2. script/ghci-flags tells ghcide to ignore .ghci files so as to avoid getting confused, so we also say -no-ignore-dot-ghci afterwards because script/repl should load them.

@@ -1,61 +1,17 @@
-- GHCI settings for semantic, collected by running cabal repl -v and checking out the flags cabal passes to ghc.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the deletions in this file are now being handled via script/ghci-flags instead.

Comment on lines +1 to +4
cradle:
bios:
program: script/ghci-flags
dependency-program: script/ghci-flags-dependencies
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. hie-bios recently added support for multiple cabal component cradles, but I couldn’t get it working with ghcide yet.

  2. My direct configuration wasn’t portable due to VS Code using a PWD env var of / and -i flags being resolved relative to that env var rather than the actual working directory. So we had to compute it.

  3. Computing it is probably better for our purposes anyway, since it allows us to share work between this and script/repl.

Comment on lines +1 to +2
#!/bin/bash
# Computes the flags for ghcide to pass to ghci. You probably won’t be running this yourself, but rather ghcide will via configuration in hie.yaml.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach here is similar to the approach of #373: basically just listing all the options we can think of, collected by clever use of cabal repl -v.

A more robust approach would probably be to do something like cabal repl for each component and union together the options, but that actually seems pretty challenging:

  • stuff like package flags come in as two flags
  • some of the flags, like -outputdir, probably shouldn’t be duplicated
  • ordering matters for a lot of the flags
  • some of them conflict with one another, like -i erasing any previously configured load paths
  • we probably don’t want to pass the module names in as that would force ghcide to load everything eagerly rather than when only when you actually look at the file

Maybe someday all of this will be made unnecessary by cabal itself, but until that happy day, this is a pretty good solution for loading & typechecking all of our sources in ghcide, at the cost of requiring us to do a bit more manual work around the project’s packages & versions.

@robrix robrix requested a review from a team November 1, 2019 16:40
@robrix robrix mentioned this pull request Nov 7, 2019
2 tasks
Copy link
Member

@tclem tclem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got this up and running on the repl and with ghcide in VS Code, nice work @robrix!

@robrix robrix merged commit cef6893 into master Nov 7, 2019
@robrix robrix deleted the multi-component-ghcide-setup branch November 7, 2019 18:32
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.

2 participants