Skip to content

Make shake a library, make the executable/test-suite use it #360

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ezyang opened this issue Dec 19, 2015 · 13 comments
Closed

Make shake a library, make the executable/test-suite use it #360

ezyang opened this issue Dec 19, 2015 · 13 comments

Comments

@ezyang
Copy link
Contributor

ezyang commented Dec 19, 2015

Otherwise Cabal has to recompile all the files each time you build.

@ndmitchell
Copy link
Owner

I'd love to but I want to test internal functions that Shake API doesn't expose. I also don't want to shove the entire test suite into the lib since it has additional dependencies. I note that Cabal could share the objects of it wanted even with my current setup, it just doesn't. Is there any better way given those constraints?

@ezyang
Copy link
Contributor Author

ezyang commented Dec 19, 2015

Hmm. My inclination would be to export a Shake Internal module, and tell clients not to use it. But if you really want to insist on strong encapsulation, I guess there's not really anything you can do here.

@ndmitchell
Copy link
Owner

I will see how big the internal module ends up being, and how ugly it looks.

@ezyang
Copy link
Contributor Author

ezyang commented Jan 3, 2016

If you're willing to pin to a recent Cabal, this should help: haskell/cabal#269 (I have a patch for this coming down the pipe)

@ndmitchell
Copy link
Owner

I'm not willing to pin to a recent cabal yet - that will cause lots of issues on older versions of GHC.

@ndmitchell
Copy link
Owner

Ah, you have s patch for cabal, not shake - very useful but I imagine it will be a good few years before I would want to make that switch, so I'll still investigate alternatives in the meantime.

@ndmitchell
Copy link
Owner

So I cleaned things up a bit. And also took a closer look, which was quite illuminating:

There are three compiled lumps of code: The library, the shake.exe and the test suite.

There are several pieces of distinct code:

  • General.*, in particular General.Timing and General.String crop up a lot. These pieces are used in all the things, and in particular for the library and exe it must be the same General.Timing since it shares a global MVar.
  • Development.Shake, the exposed pieces, which are used in both executables.
  • Internals of Development.Shake are used in the Run code for a few ByteString versions of Shake methods for performance, which I'm not averse to putting in an internal module. It uses a couple of combinators for producing consistent error messages, which can be duplicated without much concern. There are Pool/Monad/Resources/Progress modules which are tested in isolation by poking at their internal API.
  • Development.Shake.Ninja, which is used in Shake.exe, but the parser is reused in Shake.Config, which ends up in the library.
  • Development.Shake.Make ends up in shake.exe, and nowhere else.

So there don't seem any good options, even if Cabal had the ability to define internal libraries (or at least, I'd have to define lots of internal libs, and it would be super ugly). I don't see a good way to change this for Shake, since the dependencies are quite cross-cutting and fairly sensible. I still think Cabal could avoid compiling the same code with the same flags and inputs more than once, but that might require cooperation from GHC.

@ezyang
Copy link
Contributor Author

ezyang commented Jan 7, 2016

I lost the line of argument. If Cabal had internal libraries, why isn't there a very natural divison: a shake-internal library (which is all of the modules internal modules), the public-facing shake library, and then the executable and test suite which both depend on shake-internal and shake.

Cabal cannot avoid compiling the code multiple times, because it assigns a distinct set of symbol names to each component in the package. So foo in shake gets compiled as shake_foo, but the same foo in the executable gets compiled as main_foo.

@ndmitchell
Copy link
Owner

shake-library and shake.exe currently don't depend on QuickCheck, but the test suite does. If shake-test got merged inside shake-internal it drags in dependencies that then end up being shake-library dependencies, for no good reason, which complicates things like GHC bootstrapping.

Thanks for the info about Cabal - although I note it doesn't strictly have to (although I can see why it would be a pain not to).

@ezyang
Copy link
Contributor Author

ezyang commented Jan 7, 2016

So... wouldn't those bits of code live in the test suite?

Cabal must do it this way, because otherwise how are you going to disambiguate (some internal) Foo in the library versus a Foo in the test-suite.

@ndmitchell
Copy link
Owner

I guess they could - so once Cabal has the feature, and it works with the default Cabal on all the supported GHC's, it's possible to go that way.

In my case all the modules have distinct names, which is deliberate, and means Cabal could skip the prefix. I appreciate it's not the Haskell way, but I've found large corporations often go that way (both my current and previous work did), as it gives much better isolation and free reuse.

Given the current state, I suspect the only reasonable thing to do is close this for now, and wait for Cabal to catch up.

@ezyang
Copy link
Contributor Author

ezyang commented Jan 7, 2016

Well, there's still the issues where the flags could differ. In fact, with different dependencies, the header file you depend on will be different (because the set of VERSION test macros wll be different.)

Anyway, if you are dead set against exporting a "Development.Shake.Internal" module, then I guess there is nothing we can do.

@ezyang ezyang closed this as completed Jan 7, 2016
@ndmitchell
Copy link
Owner

I'd really need a set of Development.Shake.Internal modules, since shoving progress/timing etc. to all go through a single internal module will make the exes much harder to understand - and that's too much for me. So yes, lets leave this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants