Hadrian: simple targets for building libraries and executables
ClosedPublic

Authored by alpmestan on Dec 10 2018, 3:03 PM.

Details

Summary

This patch introduces (phony) build targets of the form

(1) stage<N>:<lib>:<name>   (e.g: stage1:lib:Cabal)
(2) stage<N>:<exe>:<name>   (e.g: stage2:exe:ghc-bin)

where (1) builds the given library with the stage N compiler and (2) builds the
given executable with the stage N-1 compiler. This patch may be generating too
many such targets but it's a first stab that we can refine.

This fixes Trac #15949.

Test Plan

hadrian/build.sh stage1:exe:ghc-bin

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
alpmestan created this revision.Dec 10 2018, 3:03 PM

What a nice little build rule. We could add some more intelligence (see my comments), although what you already have is very useful too.

Also, this should be documented too.

hadrian/src/Rules/SimpleTargets.hs
13

Do we have any conflicts with exe and lib having the same name? If not, perhaps, we could just drop the middle component?

Alternatively, we could allow dropping the middle component in the common case where the simple target is unambiguous. In fact, we could drop the stage component too in some unambiguous cases, like rts.

38

I guess we're after pkgLibraryFile rather than pkgConfFile?

39

Let's report an informative error on stage0:exe:*.

Also, this should be documented too.

Absolutely. See my comment on the hadrian/testsuite patch, I was going to document it all in the quickstart page but I'll instead augment the in-tree docs. First, I'll make both of those diffs document the features they're introducing, and then I'll probably try and find a way to integrate the contents of the quickstart page we have on trac in the in-tree hadrian documentation as well.

hadrian/src/Rules/SimpleTargets.hs
13

I don't think we do have any conflict yet, but if I remember correctly this is precisely due to us dancing around the lack of support for multi-component packages in the make build system.

With this in mind, I just went for the unambiguous, future-proof & safest option that Ben described in #15949, as-is.

I certainly am in favor of introducing simplifications, and more generally anything that will make the life of ghc devs easier. It's just that I was under the impression that we would perhaps some day start making use of multi-component packages, once make is gone. It's certainly not going to happen tomorrow though, so perhaps you could bring up your ideas of simplifications in Trac #15949 and see if there's some agreement around them?

We can then independently merge this patch to make the simple targets available to everyone _right now_, or wait for some agreement to take place in Trac #15949 and refine this patch accordingly. Up to you.

38

No, this was deliberate. pkgLibraryFile points to a single library artifact, while the package database entry for the package (the .conf file) is usually a handy "meta-target" to refer to the whole library being installed, including the library artifacts in all the relevant ways. I just kept this spirit here but perhaps there's a good argument in favor of pkgLibraryFile that I'm missing?

39

Indeed, good point, will do in an upcoming update.

alpmestan added inline comments.Dec 11 2018, 5:24 AM
hadrian/src/Rules/SimpleTargets.hs
39

Would it be better not to generate a rule for those cases in the first place? It's a simple predicate to add to the list comprehension for targets above. If we want hadrian to be really user-friendly I do see a point in generating those rules but explaining why those targets "don't make sense", so to speak.

alpmestan updated this revision to Diff 19088.Dec 11 2018, 5:50 AM
  • document the simple package targets, handle stage0 exes
alpmestan marked 5 inline comments as done.Dec 11 2018, 5:51 AM

Added some documentation for those new targets and the error reporting @snowleopard requested for "stage 0 executables".

bgamari accepted this revision.Dec 11 2018, 12:06 PM

Thank you for doing this, @alpmestan. This looks great!

This revision is now accepted and ready to land.Dec 11 2018, 12:06 PM
snowleopard accepted this revision.Dec 11 2018, 1:02 PM

Many thanks, @alpmestan! All looks great now.

hadrian/src/Rules/SimpleTargets.hs
13

Thanks -- good to know we don't have any name conflicts at the moment.

I'm happy with the current implementation. Let's just keep this in mind as a potential future enhancement.

39

I think having a custom error message for Stage0 executable is friendlier. Let's stick to your solution!

alpmestan marked 4 inline comments as done.Dec 11 2018, 2:26 PM
This revision was automatically updated to reflect the committed changes.