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

CI failure: can't find include file rts/fs.h #554

Closed izgzhen closed 6 years ago

izgzhen commented 6 years ago
| Copy file (untracked): _build/stage1/libffi/build/inst/lib/libffi.a => _build/stage1/rts/build/libCffi_thr_l.a
| Successfully built custom library 'libffi'
Installing library in /home/travis/build/snowleopard/hadrian/ghc/_build/stage1/lib/../lib/x86_64-linux-ghc-8.5.20180402/rts-1.0
hadrian: can't find include file rts/fs.h
shakeArgsWith     0.000s    0%                           
Function shake   14.669s    1%                           
Database read     0.000s    0%                           
With database     0.001s    0%                           
Running rules  1314.416s   98%  ======================== 
Total          1329.086s  100%                           
Error when running Shake build system:
* _build/stage1/lib/package.conf.d/rts-1.0.conf
ExitFailure 1

https://travis-ci.org/snowleopard/hadrian/jobs/361477751

https://travis-ci.org/snowleopard/hadrian/jobs/361477747

please leave a word if you are looking into this bug, thanks!

snowleopard commented 6 years ago

I haven't yet looked at this but I have a feeling that this is caused by a missing dependency on fs.h.

These files are copied since #533.

Also, this code may be relevant:

https://github.com/snowleopard/hadrian/blob/3a68f11c9d6ceebdb09f85804143dd2f1d49a502/src/Rules/Compile.hs#L38-L58

Or not :)

izgzhen commented 6 years ago

rts/fs.h is found in ["rts/.","rts/build","rts/../includes","rts/includes","rts/includes/dist-derivedconstants/header","/home/zhen/repos/ghc/_build/stage1/rts/build/.","/home/zhen/repos/ghc/_build/stage1/rts/build/build","/home/zhen/repos/ghc/_build/stage1/rts/build/../includes","/home/zhen/repos/ghc/_build/stage1/rts/build/includes","/home/zhen/repos/ghc/_build/stage1/rts/build/includes/dist-derivedconstants/header"].

The bug is the rts/ prefix.

izgzhen commented 6 years ago

HACK: mkdir rts/rts; cp rts/fs* rts/rts

izgzhen commented 6 years ago

failed with another (probably unrelated) bug: https://github.com/snowleopard/hadrian/issues/559

izgzhen commented 6 years ago

@Mistuke @snowleopard any idea on how to understand the HACK and fit it in our code?

Mistuke commented 6 years ago

I don't quite understand why those includes are treated differently than the others. They're copied in tree way before any dependency analysis is done. But we probably don't need to install them anyway. Try in the GHC checkout removing https://github.com/ghc/ghc/blob/master/rts/rts.cabal.in#L132 and the next line.

Note that it's not the building that fails, it's the package installing phase.

I don't have a Linux machine handy atm. but if you can't modify the tree easily I'll go set up a vm.

snowleopard commented 6 years ago

I am with @Mistuke here: I don't quite understand why fs headers are causing issues.

But I'm getting more convinced that I'm right in suspecting that needDependencies is key here. These headers are different from others because they are generated and needDependencies might be unable to discover them.

I'll try to have a look at this issue tonight.

izgzhen commented 6 years ago

@Mistuke

Try in the GHC checkout removing https://github.com/ghc/ghc/blob/master/rts/rts.cabal.in#L132 and the next line.

I tried and it worked.

snowleopard commented 6 years ago

I've added a workaround to fix the fs*.h issue: #566:

    -- TODO: Get rid of this workaround.
    -- See https://github.com/snowleopard/hadrian/issues/554
    root -/- buildDir rtsContext -/- "rts/fs.h"     <~ return "rts"
    root -/- buildDir rtsContext -/- "rts/fs_rts.h" <~ return "rts"

The right way to fix this could be to add fs*.h files to includes/rts which is scanned by Cabal copy. This is how other header files are found.

@izgzhen @Mistuke What do you think?

In the meanwhile, the current workaround works, so I'm lowering the priority of this issue.

izgzhen commented 6 years ago

looks good!

Mistuke commented 6 years ago

Yes that's good, though looking at it again. I probably don't care about having these installed. As the user can't use them easily anyway since they need to know the right name space. I will commit a change removing these from install once I get home.

Sorry for the extra work.

On Thu, Apr 12, 2018, 03:02 Zhen Zhang notifications@github.com wrote:

looks good!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/snowleopard/hadrian/issues/554#issuecomment-380650846, or mute the thread https://github.com/notifications/unsubscribe-auth/ABH3Kc_4WBicbhLMrGbBaccFGafuR9akks5tnrWhgaJpZM4TExys .

izgzhen commented 6 years ago
shakeArgsWith    0.000s    0%                           
Function shake   8.356s    1%                           
Database read    0.000s    0%                           
With database    0.000s    0%                           
Running rules  566.332s   98%  =========================
Total          574.688s  100%                           
Error when running Shake build system:
* _build/stage1/lib/package.conf.d/rts-1.0.conf
* _build/stage1/rts/build/rts/fs_rts.h
* rts/fs_rts.h
Error, file does not exist and no rule available:
  rts/fs_rts.h
CallStack (from HasCallStack):
  error, called at src/Development/Shake/Internal/Rules/File.hs:180:58 in shake-0.16.3-D18CK3931i8GsNl2vi6IKs:Development.Shake.Internal.Rules.File

I still saw them in error msg on my machine building the latest commit though :( no yet investigating into it

Mistuke commented 6 years ago

Commit https://github.com/ghc/ghc/commit/111556f9e809962a91666c99d96cf80db361ee32 removes these files from install-includes

Mistuke commented 6 years ago

Actually I've had to revert this, now I remember why they were there to begin with. The reach-ability analysis forces them to be. Because they're reachable from an exported header. This is all very annoying.

The exported headers are not useful. In fact you can't even call them so moving them to include would be a mistake I think. I think I know a fix that would work both for the testsuite and make/hadrian. I'll whip it up after work today.

Mistuke commented 6 years ago

sigh well that's annoying. the CI failure was unrelated. I put up https://phabricator.haskell.org/D4591 for review to run it through the mac osx builder. if it's green I'll push the removal again.

Sorry for the delay.

Mistuke commented 6 years ago

https://github.com/ghc/ghc/commit/5417c68977db2f2c2c1ce3b8b19ac1f540df471c done. the CI was clean so the failure was unrelated. You should be able to remove the work-around @snowleopard

snowleopard commented 6 years ago

@Mistuke Great! I'll remove my workaround and will close this today.

snowleopard commented 6 years ago

Done in the above commit. I think we can close this now.