snowleopard / hadrian

Hadrian: a new build system for the Glasgow Haskell Compiler. Now merged into the GHC tree!
https://gitlab.haskell.org/ghc/ghc/tree/master/hadrian
MIT License
208 stars 39 forks source link

Validation : Stderr mismatch #592

Closed chitrak7 closed 5 years ago

chitrak7 commented 6 years ago

On running validate rule on my terminal today, there were many tests failing due to mismatching stderrs (489). An overall summary is: 6363 total tests, which gave rise to 24929 test cases, of which 18636 were skipped

  58 had missing libraries
5231 expected passes
  87 expected failures

   2 caused framework failures
   0 caused framework warnings
   1 unexpected passes
 888 unexpected failures
  27 unexpected stat failures
chitrak7 commented 6 years ago

I am currently finding out the difference in the make based implementation and current implementation to look for the sources of these errors

angerman commented 6 years ago

Just a heads-up, the the build systems differ more and more these days. In general you might want to look at the actual test that fails and maybe at the testsuite and how the test is invoked. Anything that relies on inplace/xxx will likely fail, as those tools are now in _build/stage<N>/bin/.

chitrak7 commented 6 years ago

@angerman How can this issue be fixed? Currently, I am installing the packages that are in inplace to stage1 also, using a new validate rule.

angerman commented 6 years ago

@chitrak7 I'm not sure I follow. In general you may want to ensure that the location of the various tools can be parameterized throughout the testsuite, so that you can provide the location of the binaries to the testsuite.

chitrak7 commented 6 years ago

@snowleopard @angerman There are 4 main problems with current validation module: 1) 3 missing libraries: check-ppr, check-api-annotations, hp2hs 2) Incorrect path to compiler. 3) ghc-iserv should be in lib/bin and not bin. 4) An error with rts library.

I have solved the problems #1,2&3 as follows: 1) added the libraries and wrote the need rule. 2) additional argument to make command 3) added ghc-iserv to lib/bin path same as with unlit/touchy.

SUMMARY for test run started at Sat May 12 22:51:44 2018 IST 0:45:14 spent to go through 6357 total tests, which gave rise to 23650 test cases, of which 16920 were skipped

  56 had missing libraries
6514 expected passes
  95 expected failures

   0 caused framework failures
   0 caused framework warnings
   1 unexpected passes
  62 unexpected failures
   2 unexpected stat failures

I still couldn't figure out the source of error in rts problem. I believe by solving the issue with rts, number of failures will fall down below 20.

snowleopard commented 6 years ago

@chitrak7 Start sending small PRs to fix validation errors. No need to fix everything in one go.

angerman commented 6 years ago

As I mentioned elsewhere. Please don’t move binaries around. Adapt the testsuite to find binaries in it’s new places.

snowleopard commented 6 years ago

@angerman Could you create an issue where you explain:

Without this, the most natural path to fixing validation errors and other issues (e.g. see #591) is to bring everything back to bin/lib. In fact, I'm inclined to just move everything back to bin/lib to have a fully working Hadrian build as soon as possible. Migrating binaries without a clear rationale and a working technical solution seems to have been rather disruptive for Hadrian development.

ndmitchell commented 6 years ago

I'm inclined to agree with @snowleopard - http://neilmitchell.blogspot.co.uk/2014/07/converting-make-to-shake.html:

The existing Make system will generate object files with particular names in particular places. Often these locations aren't what you would pick if you wrote the build system afresh. However, resist the temptation to "clean up" these pieces during the conversion. Treat the file locations as a specification, which lets you focus on the conversion to Shake without simultaneously redesigning a large and complex build system.

I think you guys have made way too many changes in the conversion, and would recommend resisting the temptation to do more.

angerman commented 6 years ago

I'm just afraid this will break the bindist rule. Which are currently a simple compaction of bin and lib: https://github.com/snowleopard/hadrian/blob/44368b61d78b4ccf4e5aa6312cd64f4b2466efc4/src/Rules/BinaryDist.hs#L25-L29

moving binaries from bin to lib/bin. @alpmestan and I have spent a lot of time killing off ghc-cabal, keeping the source tree as pristine as possible and by that making the binary distribution rule trivial. I'd just be very sad to see all this work (partially) reversed by not making the test-suite parameterized over the location of the tools.

snowleopard commented 6 years ago

@angerman It looks to me that copying one more directory bin/lib is not such a big deal, whereas the snowfall of issues due to the removal of bin/lib is quite substantial. Anyway, this definitely deserves a separate issue, please create one.

alpmestan commented 6 years ago
  • Why you moved some binaries from their usual (for GHC's Make) location bin/lib to bin in Hadrian.

Because they were there for a technical limitation that @angerman lifted. Hadrian's GHC builds are fully relocatable, we can now just move {bin, lib}/ around together and GHC will keep working just fine. Hadrian is currently (and likely will stay forever) the only build system to take advantage of this.

  • Why we shouldn't move these binaries back where they are expected both by the GHC infrastructure and GHC developers.
  • What should be the general approach for dealing with binaries that live in the new location.

The existing GHC infrastructure has been growing for over 25 years and has been built around the "technical limitations" that we were mentionning: everything expected under inplace/..., wrappers in bin/ and actual binaries in lib/bin/, etc. The testsuite has not had to accomodate anything else and people are used to that. I think the proper solution in all aspects is to make all these things configurable. I think it's okay to compromise (like with the ghc-iserv case) for a bit but I'm definitely inclined to do the work needed to make all these things configurable myself, and offer Hadrian a chance to make our lives simpler whenever it can. This doesn't have to all happen now, but I think this is overall the right direction. This will require a few patches to GHC (especially the SysTools module I suppose), the testsuite and what not, but I personally see no good reason not to go down that path. Thoughts?

snowleopard commented 6 years ago

@alpmestan Thanks for the detailed answer! This is an important issue and should be documented separately -- so, could you please open a new ticket to track its progress?

I'm definitely inclined to do the work needed to make all these things configurable myself, and offer Hadrian a chance to make our lives simpler whenever it can. This doesn't have to all happen now, but I think this is overall the right direction.

Sounds great, thank you for helping with this! Even more reasons to document and track this separately.

alpmestan commented 6 years ago

Done: #603.

snowleopard commented 5 years ago

I believe this has been fixed.