Restore TemplateHaskell pragma in hls-graph

[pepeiborra]

The pragma is required for the embed-files flag and was removed recently in #2523.

@jneira @Anton-Latukha the PR is open for any ideas that extend CI to check the build with this flag.

[Anton-Latukha]

I obviously have limited knowledge of the project.

Since I'm already sitting - would contribute to the patch.

The patch may uncover a bit of work.

[Anton-Latukha]

Oh. I thought we in CI do not test ghcide sub-project dependencies & so though to add a test-suites into the process.

But:

🅸 ~/s/h/c/haskell-language-server:fix-hls-graph-build⨯>cabal test shake-bench
No tests to run for the package shake-bench-0.1.0.2
🅸 ~/s/h/c/haskell-language-server:fix-hls-graph-build⨯>cabal test hls-graph
No tests to run for the package hls-graph-1.5.1.1
🅸 ~/s/h/c/haskell-language-server:fix-hls-graph-build⨯>cabal test hie-compat
No tests to run for the package hie-compat-0.2.1.0
🅸 ~/s/h/c/haskell-language-server:fix-hls-graph-build⨯>cabal test hls-plugin-api
No tests to run for the package hls-plugin-api-1.2.0.2
🅸 ~/s/h/c/haskell-language-server:fix-hls-graph-build⨯>cabal test hls-test-utils
No tests to run for the package hls-test-utils-1.1.0.2
🅸 ~/s/h/c/haskell-language-server:fix-hls-graph-build⨯>

There are no test-suites, no benchmarks for them. So CI there is not of any help.

[pepeiborra]

Sorry I wasn't clear - we want to test that hls-graph builds with the embed-files flag

[jneira]

well the unique thing I can think off without too much changes is add a step in the testing workflow building the package with the non default value for the flag
the package is not big and will not take too much time I hope

[michaelpj]

Possibly dumb question: how bad would it be to just use file-embed always?

[Anton-Latukha]

[skip circleci]

[Anton-Latukha]

michaelpj raised a sound question.

[Anton-Latukha]

The request/PR shows that there is a broad structurally important question. There is a number of subprojects & they have a number of flags. & CI does not tests any of them.

1. We can not test all flag combinations.
2. We can not test flag combinations * platform * GHC matrix.
3. That is why decided to create a workflow:
   1) tests flags in "default" case. under default currently took Linux & recommended GHC version;
   2) Builds one maximal combination where all important flags are maximally enabled & so has some cross-testing;
   3) Considered that enough attention and computational power to flags testing;
   4) There are things to tune in the workflow.

[pepeiborra]

Possibly dumb question: how bad would it be to just use file-embed always?

It introduces a dependency on TH, which makes HLS-graph less portable. Which probably doesn't matter much.

[Anton-Latukha]

The details (early termination rules) we can tweak in other PRs.

I consider this ready to be reviewed & merged.

[Anton-Latukha]

I would start CI configuration consolidation effort at #2422 (comment).

[Anton-Latukha]

This state confuses me.

Seems like all requested changes were addressed & the PR is still in the change requested mode.

I rechecked a couple of times & can not tell what is the cause.

[jneira]

if a revision requests changes the revision author (in this case me) has to approve manually the pr to make it mergeable

[jneira]

To have a new workflow is nice for correctness and completeness but we have too much workflow filed so I would try to deduplicate workflows before adding more

[Anton-Latukha]

The difficulty with computation would start where there would no be new build workers for PRs, so far it looks like CI have more then enough build workers, so far I haven't seen waits on resources.

We had caching workflow added, which runs async and has short-circuiting. caching saves computational time overall.

HLint workflow runs one worker & takes ~1m to run.

Flags runs one worker ~5m.

Testing takes 15 workers, takes 3-4 of them ~90m & Benchmark takes 3 workers, 2 workers for ~95m.

So I arrived and added several CI workflows, but in reality, they do not use CI resources really. In this case for me it seems easier to have 1 workflow that takes 1 worker for 5 minutes to make a special build, than bundling that functionality inside testing. But if it is Ok to use 1 build of matrix inside testing to check flags - that seems a good option. I wanted to discuss the CI matrix use overall in the separate report.

So we only have a code complexity, because of shotgun surgery, because of the CI design (but most CIs have alike yaml-based design). And looking at DHall results (but I would still work on it a bit before reporting) - the current shotgun surgery situation in CI starts to look quite good.