Add `--basedir` flag

[angerman]

When executing cabal (or even the library, which I consider even worse), many commands rely on the current working directory to be the one containing the cabal file.

I propose we add a --basedir flag, that is part of the local build information of type Maybe FilePath, and relative file lookups are done relative to the --basedir. If the --basedir is Nothing, the behavior would be identical to the current one. However it would allow to execute cabal from a directory that is not the one that contains the .cabal file.

It looks like the new-* commands suffer from this limitation as well.

[23Skidoo]

There's a --cabal-file flag for configure, isn't it basically the same thing (--basedir is the basename of --cabal-file)?

[angerman]

You can provide the GenericPackageDescription when using the cabal api, in which case you don't have that anymore.

[mgsloan]

Figured it'd be more useful for me to comment here rather than the PR, since I can't really give a proper review:

Is this feature worth changing the API for Setup.hs hooks? That's going to cause a lot of code to need to change.

In particular, why not use setWorkingDirectory? Do you still want some paths to resolve using the actual CWD?

[angerman]

@mgsloan True. I'm not absolutely happy about changing the hooks. Luckily there is a warning right at the hooks, which put me a bit more at ease:

-- | Hooks allow authors to add specific functionality before and after a
-- command is run, and also to specify additional preprocessors.
--
-- * WARNING: The hooks interface is under rather constant flux as we try to
-- understand users needs. Setup files that depend on this interface may
-- break in future releases.

The idea here is to completely remove any dependence on the CWD (which I consider a defect). The motivation is to make cabal threadable. Right now to you are bound to a single process when dealing with the cabal library, which prevents you from performing multiple cabal lib operations in parallel without careful locking around CWDs.

This came up while working on GHC's new hadrian build system and rolling the custom cabal frontend (ghc-cabal) into hadrian directly.

@ezyang also remarked, that due to the CWD limitation, cabal currently has to shell out to separate processes to do parallel builds.

[23Skidoo]

That comment is outdated - we avoid changing the hook interface nowadays, hence weird stuff like passing ProgramDb in ConfigFlags. It's less of a problem now that we have custom-setup, but still if we're breaking compatibility in this area, I'd prefer to do a single big step along the lines of #3600 instead of multiple small changes spread across releases.

Maybe you can achieve what you want by passing the info you need inside the *Flags structures?

[angerman]

The primary reason for the hooks change is the .buildinfo file.

[mgsloan]

Yeah, I agree that if changes are made the hooks API then it'd be better to do one big change than little ones. #3600 seems like a lot of work, and there is no design there yet, so it doesn't seem likely to make it into the next release.

How about having a record type for each input? So:

data PreConfContext = PreConfContext
    { preConfBuildDir :: FilePath }

So, the idea is that users would be encouraged in the documentation to use the field accessors. Could even try to encourage this even more by only exporting the PreConfContext constructor from an internal module. Though, IIRC if you hide the constructor but not the fields, the haddocks can be a bit confusing.

If you wanted to get real wild you could do

class HasBuildDir a where
  buildDir :: a -> FilePath

instance HasBuildDir PreConfContext where
  buildDir = preConfBuildDir

Then again, if this were applied to all the hooks, it would be even more breakage. Would allow for more graceful future extension, though. Could even fold in the other existing arguments like Args / Flags, but that would be more of a pain for the refactoring.

This is probably less useful, but may also make sense to have PreConfOutput types as well, with functions like mkPreConfOutput :: HookedBuildInfo -> PreConfOutput. This way, there's the option of adding more info to the result, but user code wouldn't need to change if there's a default way of populating that extra info.

[angerman]

While I would like to shave this yak, it's a bit too hairy (#3600) right now. I'll try to jam the needed info into the Flags, to prevent having to break the Hooks API.

@mgsloan most of the hooks do have the LocalBuildInfo, which kind of works like what you describe I believe. That however is not available in the pre hooks for chicken-and-egg reasons.

[mgsloan]

That however is not available in the pre hooks for chicken-and-egg reasons.

I see, well how about at least something like

data PreInfo = PreInfo 
  { preBuildPrefix :: FilePath
  }

Once again, either documented that fields should be preferred, or better yet, only export the constructor from an internal module. This would allow later extension of PreInfo without breaking code

[23Skidoo]

@mgsloan Yep, this is one of the things planned for #3600. Quoting #3065:

If you design a hook interface in the future, I think every hook should be given a type HookNameArgs -> IO (), where HookNameArgs is a per-hook, abstract data structure.

[angerman]

This was mostly fixed in #4874