Fix #3391: build all packages if no target is specified.

[ezyang]

Fixes #3391. CC @hesselink , @23Skidoo , @hvr, @dcoutts, @acfoltzer. Please test and report.

Signed-off-by: Edward Z. Yang [email protected]

[mention-bot]

@ezyang, thanks for your PR! By analyzing the annotation information on this pull request, we identified @dcoutts to be a potential reviewer

[ezyang]

This behavior is probably wrong for REPL, because when you pass multiple targets to repl you get the repl brought up multiple times. Which is a bit silly. #3659

[hvr]

I notice this triggers building pretty-show-1.6.12 (exe:ppsh) when called in cabal's top-level folder... why?

[ezyang]

That's because pretty-show depends on happy which has a custom setup, which HEAD currently decides to pick the inplace Cabal to build with. See #3706

[23Skidoo]

Nice!

[dcoutts]

That's because pretty-show depends on happy which has a custom setup, which HEAD currently decides to pick the inplace Cabal to build with. See #3706

@ezyang then doesn't that mean we've got the wrong meaning of "all packages" here? Surely we want all packages that are local to the project, not all packages that are being built inplace. Pulling in non-local packages to get built inplace is an implementation detail and not something that should determine the default set of things to build. Note that determining local packages as opposed to inplace is not hard.

[ezyang]

@dcoutts, no, this behavior is unrelated to this patch; you can repro it if you just say cabal new-build cabal with a sufficiently recent HEAD. pretty-show is built because it's a dep of cabal-install, and cabal-install is a local package.

[dcoutts]

Ok true, if it doesn't pick pretty-show as a target then that's fine.

[dcoutts]

Do we really think this is what people want? That if they're in a dir with a package it'll build just that package and if they're anywhere else within the project (e.g. some random subdir, or in a top level dir that contains no .cabal file) that it'll build everything? It seems a little confusing to me.

As an alternative idea: when there's nothing obvious to build, report a message saying what packages are available in the project, and mention an "all" target to build them all.

I'm not strongly advocating one or another, just want to make sure we're sure this is a good idea.

[hesselink]

Another option which might be good for consistency is to look at what stack does. IIUC it builds all packages when you call build without arguments, regardless of your cwd. You can also give it package names (does the obvious thing of building those packages), or directories, in which case it'll build all packages under that directory. So calling build . would build the current package if you're in that dir.

[ezyang]

I'm OK with @hesselink's plan. Do file paths work already? Relevant bug: #3318

[ezyang]

I pushed a commit with @hesselink's proposed semantics. Please give it a try.

[dcoutts]

I'm OK with @hesselink's plan. Do file paths work already? Relevant bug: #3318

Yes, they do. That ticket is about paths to packages that are not part of the project.

[dcoutts]

So that behaviour is indeed more consistent, then the worry is that it's rather different behaviour from what cabal has done in the past, and that might be confusing for users. That said, we can give it a go and see what people think.

[hvr]

Honestly, I like @hesselink's proposal the least (sorry!), I liked both what @ezyang implemented originally, as well as what @dcoutts proposed much better. It's quite non-intuitive to me, being used to other build-systems, which take into account the current $CWD (and are thus more DWIM-compliant). Especially since make does this, this assumption is also built into editors/ide which are easier to use with cabal new-build if it's close to make behaviour.

Why can't we do one of the other two suggestions?

[ezyang]

If cabal new-build . preserves the old semantics, does that alleviate some of the concerns? (Running code and rough consensus!)

[hvr]

@ezyang Only if there was some global configuration setting I could set so that cabal new-build behaves as if it was cabal new-build .. I.e. I don't want to have to remind myself to type that . argument. Defaults matter, and cabal new-build always meaning cabal new-build all would be a bad default IMHO.

[ezyang]

I reset the PR to the first proposed fix.

[hesselink]

@hvr No need to be sorry, I was just presenting an option :) Anything is better than having to specify all packages on the command line. IIUC right now it acts like current cabal if you're in a package dir (build that package) and builds all packages otherwise? That sounds good to me. I think the only use cases I've had are 1) build one package, either from the current dir or by name, 2) build a set of packages by names and 3) build all packages in the project. All of those are possible with this PR 👍 The only other scenario I can imagine is building a subset of packages contained in a directory tree. What happens now if you pass a path to build?

[ezyang]

The only other scenario I can imagine is building a subset of packages contained in a directory tree.

Nothing like that is implemented. I'm inclined to not implement unless someone asks for it?

[acfoltzer]

I like the new behavior a lot, but that might just be because of the grooves worn into my brain by Stack.

I did notice that we need a friendlier error message when running without a target in a directory with no .cabal file and no cabal.project in an ancestor directory:

% cd /tmp
% cabal new-build
BadPackageLocations [BadLocGlobEmptyMatch "./*.cabal"]

[ezyang]

See #3636

[BardurArantsson]

I get the sense that what most people care most about is that "cabal new-build" in the 'top' directory does The Right Thing. Therefore I'm going to (somewhat arbitrarily) merge and let's see what people think after there's a little bit of practical experience with this.

EDIT: Let's not forget that "new-build" is still beta in 2.0 (AFAIUI), so it's not like we need to get this perfect from the start.

Btw, huge thanks to @ezyang for the ridiculous amount of code he's been pushing out! 😸

EDIT#2: Generally, I think we're spending hugely too much time bikeshedding instead of experimenting. Obviously, Cabal (the library) is special, but it seems a lot of us (including me!) are also walking on eggshells when it comes to cabal-install, but I must say that I feel that this is unwarranted given the guarantees that cabal-install gives wrt. backward-compat (i.e. basically none, officially at least. Other than being installable from X previous versions of itself + various GHC versions.).