search

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

Add clean routines for fs files copied by configure #533

Closed Mistuke closed 6 years ago

Mistuke commented 6 years ago

Following https://phabricator.haskell.org/D4416 configure will now be copying or hard-linking certain files to a few packages. the Make based system will remove them on clean.

This patch adds the Hadrian part.

izgzhen commented 6 years ago

why in the Makfile's patch the files to delete are explicitly named, but in this path the outer directories are removed?

Mistuke commented 6 years ago

I don't quite follow, from the source and documentation of removeFiles I gathered it only removes the outer directory is no file pattern is specified https://github.com/ndmitchell/shake/blob/f1163b9d960a1b983f35d71fb5935e59d71da70a/src/Development/Shake/Internal/Rules/Directory.hs#L294

and walk https://github.com/ndmitchell/shake/blob/fa914235116a50bbe5d613d6ead103962c3c1bc1/src/Development/Shake/Internal/FilePattern.hs#L302

So what am I missing here...

izgzhen commented 6 years ago

Oh, sorry, I just noticed that fs.* part.

snowleopard commented 6 years ago

@Mistuke Thank you for making a patch for Hadrian too!

Happy to merge it, although in principle Hadrian tries to avoid contaminating the source tree. So perhaps at some point we could copy the files into the build directory instead.

Mistuke commented 6 years ago

@snowleopard unfortunately we made this decision because there are no good options here. The main problem is that these files are replacement functions for file I/O and so every source that does an fopen or sopen will require them. But we also can't break Cabal's sdist which currently as a matter of principle in the design can't pack files pointing outside the source folder.

This left us with either duplicating all the files, copying them in the build system or create a new package which would only have these components sources. The last approach was discarded since we don't know what the fallout would be of adding dependencies to the rts and base packages.

In the future base will be mitigated, rts can maybe use the base implementation. If that's possible then the rest can just get a new dependency.

snowleopard commented 6 years ago

@Mistuke I see, thank you!