[ghc-cabal branch] Run the 'configure' command first

[angerman]

Some other fallout from #18

Function shake  0.037s   41%  ====================     
Database read   0.005s    6%  ==                       
With database   0.000s    0%                           
Running rules   0.045s   50%  =========================
Total           0.090s  100%                           
Error when running Shake build system:
* utils/hsc2hs/stage1/build/tmp/hsc2hs
Run the 'configure' command first.

[angerman]

After some further inspection, it looks like we need to completely drop the GhcCabal builder, or do we want to keep ghc-cabal around? So we'll end up integrating most of ghc-cabal into shaking-up-ghc.

[snowleopard]

Yes, we will drop GhcCabal and the ghc-cabal executable. Let's keep this code while we are working on #18 though, it may be handy as a reference (same as Data.hs).

[angerman]

I got this far. Now I'm feeing stupid :-)

Configuring ghctags-0.1...
shakeArgsWith   0.002s    0%                           
Function shake  0.049s    5%  =                        
Database read   0.001s    0%                           
With database   0.000s    0%                           
Running rules   0.787s   93%  =========================
Total           0.839s  100%                           
Error when running Shake build system:
* inplace/bin/ghctags
Run the 'configure' command first.

[snowleopard]

@angerman OK, I can take over for today -- my turn to feel stupid :-)

[angerman]

Pushing my changes, right now. I think the issue lies with the fact that distdir is relative and not absolute, which makes us build in utils/ghctags/stage2 instead of in .build.

[snowleopard]

I'm getting a different error now, but most likely still relevant to this issue:

Error when running Shake build system:
* inplace/bin/hp2ps.exe
Lint checking error - current directory has changed:
  When:    before building inplace/bin/hp2ps.exe
  Wanted:  D:\msys\home\nam83\ghc
  Got:     D:\msys\home\nam83\ghc\utils\hsc2hs

@angerman Agree, it looks like the issue is with directories.

[ndmitchell]

That error means someone called setCurrentDirectory somewhere. In a multithreaded build system that's a terrible idea.

[ndmitchell]

For info, if anything is changing directory then lint will only catch it some of the time and it will cause really weird failures of not.

[ndmitchell]

Looking at the ghc-cabal branch, I see a definition of Oracles.PackageData.Internals.withCurrentDirectory which precisely matches the one in System.Directory.Extra, although in newer GHC's it's even in System.Directory directly (in which case extra reexports it). Looking for uses of that I see:

liftIO $ withCurrentDirectory (pkgPath pkg)
    (withArgs (["configure", "--distdir", distdir, "--ipid", "$pkg-$version"]) -- ++ config_args)
        runDefaultMain)

That won't work. It works fine if in a different process (where ghc-cabal previously lived), but fails miserably here - the current working directory is a global shared variable and is meaning everyone else has incorrect file paths.

[snowleopard]

@ndmitchell Thanks! Note that Oracles.PackageData.Internals is currently just a crude copy of ghc-cabal's Main.hs and everything is badly broken. @angerman and I are slowly making our way through the mess hoping to adapt ghc-cabal internals to our purposes.

[snowleopard]

The current goal is to figure out how to configure a package without changing directories and put the resulting artefacts in .build.

[ndmitchell]

Good luck! While you are there, I wonder if the repeatedly keep running configure approach is necessary, or if a single global configure could be done which was just copied for every package which needed it. Might give a nice speed benefit, and (if changing directory is important for the configure) might also fix that.

Taking a brief look at the code, it seems to be running things that are in the Cabal library, so I couldn't see any obvious way you could avoid changing directory without also avoiding parts of the Cabal library. Note that Stack etc just call runhaskell Setup for each package, and doing that way you could change directory, although not sure if that gives you enough freedom.

[snowleopard]

While you are there, I wonder if the repeatedly keep running configure approach is necessary, or if a single global configure could be done which was just copied for every package which needed it.

Agreed, that would be preferable.

Note that Stack etc just call runhaskell Setup for each package

But isn't this exactly what we are trying to escape from -- calling ghc-cabal for each package and then parsing the results?

[snowleopard]

@angerman No progress from me again. Was fighting various linker & GMP issues all day :(

[angerman]

@ndmitchell yes the withCurrentDirectory is in there as a crude hack, the cabal library seems to expect to be run (runDefaultMain) from within the directory that contains the .cabal file. I couldn't be bothered yesterday to actually dive into the cabal library and try to figure out how to actually runDefaultMain without being in that directory. And I found withCurrentDirectory through hoogle :-) but it probably should have looked at extra as well, as i couldn't find it locally, I just dumped a copy of the definition into Internals.

The other issue I ran into yesterday, #170 (comment) likely stems from running the default main in the libraries folder, and not providing an absolute path for the distdir.

Trying to absorb ghc-cabal into shaking-up-cabal looks more and more like opening pandoras box. @ndmitchell, pointers towards stack are actually a good idea. Maybe @snoyberg can share some lessons learned from working with the cabal library?

[snoyberg]

We try to avoid using cabal the library a lot actually, and instead just
call out to Setup.hs as a separate process. It's inefficient, but avoids
these kinds of process global things cabal seems to do.

On Fri, Jan 15, 2016, 4:49 AM Moritz Angermann [email protected]
wrote:

@ndmitchell https://github.com/ndmitchell yes the withCurrentDirectory
is in there as a crude hack, the cabal library seems to expect to be run (
runDefaultMain) from within the directory that contains the .cabal file.
I couldn't be bothered yesterday to actually dive into the cabal library
and try to figure out how to actually runDefaultMain without being in
that directory. And I found withCurrentDirectory through hoogle :-) but
it probably should have looked at extra as well, as i couldn't find it
locally, I just dumped a copy of the definition into Internals.

The other issue I ran into yesterday, #170 (comment)
#170 (comment)
likely stems from running the default main in the libraries folder, and
not providing an absolute path for the distdir.

Trying to absorb ghc-cabal into shaking-up-cabal looks more and more like
opening pandoras box. @ndmitchell https://github.com/ndmitchell,
pointers towards stack are actually a good idea. Maybe @snoyberg
https://github.com/snoyberg can share some lessons learned from working
with the cabal library?

—
Reply to this email directly or view it on GitHub
#170 (comment)
.

[angerman]

Thanks @snoyberg; that sounds a bit depressing.
@snowleopard how do we want to proceed with this?

[ndmitchell]

What's the motivation for avoiding calling out and parsing the result? What's the payoff? Parsing the result shouldn't be too hard, but it seems that avoiding the call out is.

[angerman]

@ndmitchell for me--though I'm not sure about @snowleopard--it's the fundamental question, why do I need to shell out to invoke something, which I should in principle be able to call directly. E.g. why do I need that extra glue that effectively does nothing.

Shelling out is the pragmatic solution, while directly using cabal is the more idealistic approach, I guess. Then this started as issue #18 which was about getting rid of ghc-cabal for good. We might have had a too ambition goal here to completely absorb ghc-cabal.

[snowleopard]

@ndmitchell @angerman @snoyberg Thanks all for your thoughts and suggestions.

I have now clarified the rationale behind getting rid of ghc-cabal in #18. Shall we continue this discussion there? In principle we can of course give up on this and come back to using ghc-cabal and package-data.mk files. An alternative solution seems to call Setup.hs; I never considered it, so it is worth discussing the pros and cons.

[bgamari]

Why not simply try to attack the problem at its root? Arguably Cabal shouldn't be relying on the global current directory and it sounds like others will benefit from this being fixed.

[snowleopard]

Why not simply try to attack the problem at its root?

@bgamari That would be my current preference. If this diverts us too much from reaching the first-shake milestone (which people are waiting for) we can postpone this until a later milestone.

[angerman]

@bgamari 👍 That would be the preferable approach! I'm willing to dive into cabal, and try to figure things out. But I'm afraid it would postpone this PR quite a bit.

And maybe we should try to address those issues in an orchestrated approach? I would rather want to address the cabal (potential) cabal issues in a way that would help everyone and not just one project, but make it better for everyone.

As the use of the in place ghc-cabal works sufficiently well in the master branch, I'd suggest to postpone this change until after the first milestone.

[bgamari]

@angerman that sounds reasonable to me. Indeed I'd want to hear from the Cabal folks to see whether ripping out this state is even feasible before beginning an attempt.

[snowleopard]

@angerman @bgamari 👍 to an orchestrated approach.

[snowleopard]

This issue and #18 are now postponed until tree-tremble.

[snowleopard]

This is no longer relevant after #531.