Implement 'sdist-ghc' rule #265
Conversation
| -- | Like copyDirectory, but with a predicate to decide which files/directories | ||
| -- to include. | ||
| copyDirectory' :: (FilePath -> IO Bool) -> FilePath -> FilePath -> Action () | ||
| copyDirectory' test source target = do |
There was a problem hiding this comment.
I will rework this function because it has a flaw, it will recreate the full path that was given as source in the destination directory.
Interesting! Does this mean that Hadrian is building too much stuff, which is not needed for GHC itself, but only for the testsuite? If the answer is positive, then we probably need to modify |
Agreed, this feels strange. But as I understand on some systems Alex and Happy are just unavailable. (Please correct me if I'm wrong.)
What we could do is to make Alex and Happy optional builders (see @simonmar Does the above sound reasonable? |
|
I came across this relevant note in Makefiles, worth recording here: # Note [Implicit rule search algorithm]
#
# The order in which implicit rules are defined can influence a build.
#
# Case study: genprimpos/Lexer.hs
#
# We have two implicit rules for creating .o files, which after instantiating
# with a specific directory ($1=utils/genprimops) and distdir ($2=dist) look
# like this:
#
# utils/genprimops/dist/build/%.o : utils/genprimops/dist/build/%.hs
# <recipe1>
# utils/genprimops/dist/build/%.o : utils/genprimops/./%.hs
# <recipe2>
#
# The first rule is defined in hs-suffix-way-rules.mk (this file), the other
# in hs-suffix-way-rules-srcdir.mk.
#
# Assume for rest of this story that %=Lexer.
#
# In a normal repository checkout, neither Lexer.hs exists, but we have a rule
# to generate the one in the build directory by running alex on Lexer.x (the
# rule for that operation is defined in hs-suffix-rules-srcdir.mk). Since make
# needs to make a choice which of the above two implicit rules to follow (it
# never runs 2 recipes for the same target, unless double colon rules are
# used, which we don't), logically it will choose the first rule: Lexer.o will
# depend on Lexer.hs in the build directory, that file will be build, and then
# Lexer.o can be build.
#
# In an sdist however, Lexer.hs is present in the source directory. It was
# copied there during the creation of the sdist by a rule in
# sdist-ghc-file.mk. And this time we *don't* know how to generate the
# Lexer.hs in the build directory, because 1) alex is not installed when
# building from sdist and 2) the sdist creation process renamed Lexer.x to
# Lexer.x.source. So normally make would now choose the second rule: Lexer.o
# will depend on Lexer.hs in the source directory, for which nothing needs to
# be done, and then Lexer.o can be build.
#
# There is however another actor in play, a rule in sdist-ghc-file.mk, which
# after after instantiating with the same directory ($1=utils/genprimops) and
# distdir ($2=dist) looks like this:
#
# sdist_utils/genprimops_dist_Lexer : utils/genprimops/dist/build/Lexer.hs
#
# Note that this is not an implicit rule, there aren't any %'s. This rule
# *explicitly* depends on Lexer.hs in the build directory. What follows is the
# following:
#
# * make thinks Lexer.hs in the build directory "ought to exist" [1],
# because it is an explicit dependency of /some/ target.
#
# * this puts our two implicit rules on equal footing: one depends on a
# file that exists, the other on a file that ought to exist. Make's
# implicit rule search algorithm doesn't distinguish between these two
# cases [1].
#
# * to break the tie, make chooses the rule that is defined first. Lexer.o
# will depend on Lexer.hs in the build directory, which doesn't exist,
# and which make doesn't know how to build, causing a build failure.
#
# To prevent this from happening we define rules for haskell source files in
# the source directory before those in the distdir.
#
# Alternative solutions:
#
# * Don't include the explicit rule above when not creating an sdist, as
# that is the only time when it is needed.
#
# * Merge the two implicit rules, with help from $1_$2_HS_SRCS from
# hs-sources.mk, which is sdist aware.
#
# * Require alex and happy to be installed when building from an sdist,
# simplifying all this drastically.
#
# [1] https://www.gnu.org/software/make/manual/make.html#Implicit-Rule-Search |
A first quick look at Debian shows me that Alex and Happy are available on all architectures where GHC is available. I have send out a request to the debian-haskell mailing list. Lets wait what they say. I can't say anything about the other platforms (MacOS, Windows, ...). |
@KaiHa After reading all discussions I could find on this topic, I'm inclined to propose to bite the bullet and say that the first version of Hadrian adds Alex and Happy as build requirements for building from source tarballs. This greatly simplifies things. If at some point this causes problems for someone we will come back to this issue and see what we can do. But let's not over-engineer the first version.
On Windows, Alex and Happy are easily available via |
|
I suspect in the days when Happy and Alex were optional Cabal was also not yet invented. Nowadays these things are easy with Stack or Cabal. |
@snowleopard after removing |
|
@KaiHa I've pushed a commit (see above) that simplified some of your code related to this PR. I believe my simplifications are correct but I have no way to test them -- could you try on your side? Are you planning on finishing this soon? |
@snowleopard It is a long time since I had the last time a look at it (sorry for that). But I will try to test your changes in the next days.
Hmm, to be honest this issue is more convoluted than I had expected. It will take me some time to wrap my head around it again. If I am hindering you, feel free to take it away from me. |
|
@KaiHa Thanks! I would prefer that we at least partially solve this using the code that you've already written and then if you do not have time to continue, we could leave an open issue detailing what else needs to be done. What do you think? We can take the third option from the above comment (#265 (comment)): I'll leave some more comments in your your code. |
| @@ -64,6 +64,7 @@ executable hadrian | |||
| , Rules.Program | |||
| , Rules.Register | |||
| , Rules.Selftest | |||
| , Rules.Sdist | |||
There was a problem hiding this comment.
Let's call this SourceDist to match the name of the function.
| @@ -29,7 +29,7 @@ defaultKnownPackages = | |||
| , filepath, genapply, genprimopcode, ghc, ghcBoot, ghcBootTh, ghcCabal, ghci | |||
| , ghcPkg, ghcPrim, ghcTags, haddock, haskeline, hsc2hs, hoopl, hp2ps, hpc | |||
| , hpcBin, integerGmp, integerSimple, iservBin, libffi, mkUserGuidePart | |||
| , parallel, pretty, primitive, process, rts, runGhc, stm, templateHaskell | |||
| , pretty, process, rts, runGhc, templateHaskell | |||
There was a problem hiding this comment.
I don't think we should remove these packages from defaultKnownPackages, as it will then make it impossible to build them with Hadrian. I suggest you simply filter them out for now at the use site. The right way would probably be to use getPackages expression for extracting the list of packages that we actually build by default (see https://github.com/snowleopard/hadrian/blob/master/src/Rules.hs#L43 for example).
There was a problem hiding this comment.
OK, I will give it a try. I would like to rebase this PR to your latest master. Do you have any objections?
There was a problem hiding this comment.
I have rebased the PR and did the easy modifications you proposed. I still have to figure out how to exclude parallel, primitive and stm. I will have a look at it in the next days
There was a problem hiding this comment.
@KaiHa Thanks! The simplest way could be just:
sourceDistPackages = defaultKnownPackages \\ [parallel, primitive, stm]:)
There was a problem hiding this comment.
There seems to be a misunderstanding. I am not asking how to exclude parallel, primitive and stm from the tarball. This is already done (because that was also the case for the make generated tarball).
The reason for the removal of them from defaultKnownPackages is that ./build.sh --flavour=quick on the extracted sources is failing, because parallel, primitive and stm are intentional not part of the tarball.
There was a problem hiding this comment.
@KaiHa Ah, sorry for the confusion. I did check recently that Hadrian doesn't build these packages by default, but I didn't realise that Hadrian still parsed their cabal files in order to determine overall package dependencies!
I've just pushed a fix for this, so I think this should no longer be a problem for you -- the build should succeed on the extracted sources.
There was a problem hiding this comment.
@snowleopard I have rebased this PR once again so it is based on your latest master. With the changes you made in 67f433b hadrian can build ghc from the created tarball. I think my work on this PR is done.
|
@snowleopard are the CI builds on OSX and Windows reliable? I have problems to imagine how my changes have introduced the errors I see in the logs. |
|
@KaiHa I've restarted both CI builds. The Windows one failed on download (this happens sometimes, I need to figure out hot to deal with this better). The MacOSX one looks like a broken GHC, not Hadrian. This also happens regularly. |
|
@KaiHa Just to let you know: I've tested the |
See #219
This is a first rough implementation of the 'sdist-ghc' rule that has its flaws. I put some additional libraries into the tar-ball, because otherwise the build from the tar-ball fails (didn't investigate further).
Also I am not happy (no pun intended) with putting generated files into the tar-ball. The ghc
./aclocal.m4script needs a patch if we leave the generated.hsfiles in the_build/directory, otherwise./configurewill complain if alex or happy is missing. Furthermore I believe hadrian itself must learn how to deal with missing alex and happy.