Remove validation errors

[chitrak7]

I have copied some files as required by testsuite.

Also, I have unsuccessfully attempted to resolve #626.

@sighingnow @snowleopard @alpmestan

[snowleopard]

@chitrak7 Thank you! Please see my comments above.

[chitrak7]

@snowleopard does ghc always make vanilla?

[snowleopard]

does ghc always make vanilla?

@chitrak7 Yes, I think it does.

[chitrak7]

@snowleopard I find the logic testsuite uses to find which ways libraries are built quite weak. It simply checks for the presence/absence of PrimoWrappers.hi/dyn_hi/p_hi. Should I use the same logic also?? Is there any other way to do this?

[snowleopard]

@chitrak7 Why don't you look at the contents of ways?

[alpmestan]

Ideally I think we really want to examine the libraryWays and rtsWays of the Flavour we are running with, because it is supposed to contain precisely those pieces of information as far as my understanding goes. If you find that for some reason this is not reliable enough (or falls short in some cases), well first please let me know because I'm curious about this, but we can then simply go with the slightly more "hacky" strategy of checking the existence of build products with all those way-specific extension to figure out what we have actually built. Sounds good to everyone?

[snowleopard]

@alpmestan As I mentioned elsewhere, I think we might want to add a new build flavour, specifically for the testsuite, listing precisely all the ways that we'd like to test (and therefore build).

In other words, the build for testing is different from a usual (default) build: not only we build a different collection of packages, we might also want to specify precisely which ways we want.

[chitrak7]

@snowleopard I have added fore features to this PR.

Why don't you look at the contents of ways?

Can you please explain what you mean when you say this. Exactly whose ways are we supposed to look?

[alpmestan]

@alpmestan As I mentioned elsewhere, I think we might want to add a new build flavour, specifically for the testsuite, listing precisely all the ways that we'd like to test (and therefore build).

I'm not sure I agree with this. The behaviour of the Make build system here, when you e.g say make slowtest, is to run all the tests that are not skipped given the configuration/build artifacts available. So if you don't have profiling libs, all tests that require them will be skipped (which is different from failing, they're just not considered for execution, like Windows tests are not considered for execution on Linux). I would therefore have thought that the --flavour could dictate what we produce and that the testsuite could just go with that, skipping all the tests that require other things.

This by the way does not exclude having a dedicated flavour that just has everything the testsuite needs to run all the tests on a given system.

[chitrak7]

@snowleopard I think I agree with Alp on this one. We should instead modify Hadrian to read input test ways and build libraries based on them. This is what I was trying to fix through when I encountered #626. Aside from that, I favor using input flavors and build flavors to build libraries instead of checking the libraries we have already installed.

[chitrak7]

@snowleopard I think I am finished with major part of this issue. I will handle library ways in a later PR. Can you review this one at present.

[snowleopard]

@alpmestan @chitrak7 OK, you convinced me :)

Then, what about producing a file, e.g. _build/flavour, after each successful build that essentially provides all necessary information for the testsuite (or other possible consumers in the toolchain)? This will make it possible for the user to run Hadrian using their custom build flavour and then run the testsuite, which will pick up the correct build configuration without relying on hacks like checking for p_dyn objects? This will also not tie the testsuite only to the default build flavour.

[snowleopard]

An alternative is then, which you seem to suggest, is to require the user to run the testsuite with exactly the same flavour as during the build.

For example, build --flavour=user must be followed by build test --flavour=user. This might work too.

[snowleopard]

@chitrak7 I've added a couple of more comments. Please have a look.

[alpmestan]

An alternative is then, which you seem to suggest, is to require the user to run the testsuite with exactly the same flavour as during the build.

For example, build --flavour=user must be followed by build test --flavour=user. This might work too.

Right. Or really you'd just do build test --flavour=something (where something is an existing or user defined flavour). This is a quite common need. Say you investigate some ticket, write a patch and want to see if that fixes the failure of test foo. You can then just go ahead and do build test --flavour=suitable_flavour --build-root=_test-foo --only=foo and have a cup of tea/coffee, where suitable_flavour should be whatever is appropriate: quickest if the test doesn't need any fancy way, devel2/perf/prof/etc depending on what's needed.

One reason why I think this works rather nicely is that this is the most flexible approach. I see two alternatives. The first one, at the opposite end of the spectrum, is to just require all the ways of everything to be built before we can run any test. This is annoying as it will significantly increase the duration of the build. The other one, sitting in the middle, is only conceptual but is the best one. It'd consist in building just what we need for just those tests the user is asking to run (as determined by command line options etc). However the testsuite doesn't really give us a way to discover this ahead of time, as far as I know. I mean, we could scan the .T files and try to inspect the source code to figure some things out but... I don't think we want to do this :-) So I think it's a good choice to let users specify a flavour that's compatible with the tests they want to run.

Now... in this scheme that I'm suggesting, what if we call the test rule again on the same build root but with a different flavour than the one we called it on right before? I don't think this would rebuild things with the way our library/executable rules are implemented at the moment. It feels like we should rebuild everything so as to actually produce all the build artefacts variants that the new flavour implies. A bit like how cabal-install does it. Thoughts?

[snowleopard]

@alpmestan Yes, I agree, this looks like the right approach indeed.

Now... in this scheme that I'm suggesting, what if we call the test rule again on the same build root but with a different flavour than the one we called it on right before? I don't think this would rebuild things with the way our library/executable rules are implemented at the moment.

Hmm, are you sure the rebuilds won't happen? We track all command line arguments, so if changing the flavour means a command line changes (e.g. you add -prof) then all affected targets will be rebuilt.

[sighingnow]

what if we call the test rule again on the same build root but with a different flavour than the one we called it on right before?

I agree with this idea. make test doesn't trigger rebuild even if there are changes in sources. I think we should decouple the dependency between the stage1/2 ghc and the hadrain test command. We only need to make sure the corresponding ghc compiler exists. If not, we can report error to user and suggest the hadrian build --flavour=xxx command.

[alpmestan]

Hmm, are you sure the rebuilds won't happen? We track all command line arguments, so if changing the flavour means a command line changes (e.g. you add -prof) then all affected targets will be rebuilt.

@snowleopard I'm not entirely sure, and I'm in the middle of something right now so I can't really give it a shot.

I agree with this idea. make test doesn't trigger rebuild even if there are changes in sources. I think we should decouple the dependency between the stage1/2 ghc and the hadrain test command. We only need to make sure the corresponding ghc compiler exists. If not, we can report error to user and suggest the hadrian build --flavour=xxx command.

@sighingnow Right. I'll be happy as long as I can start from a fresh source tree and just call build -c test and have this command build a GHC. Adding --flavour should then refine what gets built and possibly rebuild things, and the test rules should just react accordingly, picking up or dropping tests depending on what build products are available.

Note that this is the default behaviour of make test indeed but not of the validate script, which by default uses a pre-determined .mk file but lets you supply your own. It might be interesting to map this pre-determined build specification to a Flavour in hadrian land and use it for the validate rule (and just that one, not the test rule) when no --flavour is specified? We would therefore have build validate ~ ./validate and build test ~ make [slow|fast|full]test. And supply a --flavour explicitly would amount to writing a custom validate.mk to be picked up by the ./validate script.

[snowleopard]

OK, looks like all comments have been addressed. Shall I merge this?

[chitrak7]

@alpmestan @snowleopard Is this pull ready to land.

[snowleopard]

@chitrak7 Merged, thanks!