Fix cabal warning about use of a deprecated profiling flag (Fixes #2350)

[hesiod]

This fixes issue #2350. See the commit for details.
This PR also includes a minor refactoring commit which improves consistency.

[sjakobi]

Thanks for working on this @hesiod!

The code looks good to me!

How did you test this with a Cabal version < 1.21.1? Do you know some tricks that I don't know?

I've only tested this with Cabal-1.24.0.0 and Cabal-1.22.7.0, which were already in my package db, the latter by executing stack exec -- ghc-pkg unregister Cabal-1.24.0.0.

[sjakobi]

And could you add a changelog entry, please!

[hesiod]

After some tinkering, I finally got a way to test this with Cabal-1.20.0.4. Along the way, I think I may have discovered one or two incorrect version bounds, but see for yourself:

• In stack.yaml:
  • You'll need to change the resolver to lts-2.22 (earlier lts-2 should work, too)
  • Add Cabal-1.20.0.4 to the extra-deps
• In stack.cabal:
  • Change the lower bound on process to 1.2.1.0
  • Change the lower bound on deepseq to 1.3
  • Add old-locale to the build-deps
• Run stack solver --update-config && stack build

The three changes to the cabal file seem to be "real bugs", and while the third case (old-locale) is partly handled in source code, it lacks the required handling in the cabal file (i.e. a flag). I created PR #2381 for these fixes.

Otherwise, stack builds fine with the changes and stack build --profile works as expected, even with Cabal-1.20.0.4.

[sjakobi]

Seems like there's a misunderstanding here. Sorry for not communicating more clearly!

When I asked about testing with different Cabal versions above, I was referring to envConfigCabalVersion which is the version of Cabal that stack uses to build the various Setup.hs files during the build process. You can determine that version with stack exec -- ghc-pkg --no-user-package-db field --simple-output Cabal version, it's the first version listed. It's the same version as the one used in stack path --dist-dir.

This version of Cabal is entirely unrelated to the version of Cabal that the stack package depends on and that you can find when executing stack list-dependencies in the stack project.

---

So when I asked

How did you test this with a Cabal version < 1.21.1? Do you know some tricks that I don't know?

I wondered if you managed to make stack use such a version for the build process.

I guess stack should just have something like stack setup --install-cabal VERSION in addition to stack setup --upgrade-cabal.

[sjakobi]

It seems like the distinction of the two Cabal versions should also be documented somewhere. Do you possibly have a suggestion how to do this, so people might actually find it, @hesiod?

What about the haddocks for EnvConfig? Would you have seen them?

[hesiod]

Oops - I probably just read "test with older Cabal versions" and went ahead and tried to build stack with Cabal-1.20.0.4! Furthermore, I didn't realize there was a stack-7.8.yaml and a time-locale-compat package that handle all the locale compatibility logic that I then tried to solve again - lesson learned!

Regarding documenting the distinction between different Cabal versions: I'm not sure whether I understood you right, but if you mean documenting the difference in the profiling flag used by stack, I don't think that's really required or useful: First, the change is solely a syntactic change, AFAIK, there is no actual difference between --enable-executable-profiling and --enable-profiling (however, the former could be removed in later cabal versions). Also, there already is Cabal version dependent behaviour in that very function, so it does not seem all that uncommon to leave such behaviour undocumented. (Looking over the code, I realized that newerCabal and newerProfFlagCabal might as well be merged into one, since 1.21 was an unreleased development series of cabal.)

[sjakobi]

Regarding documenting the distinction between different Cabal versions: I'm not sure whether I understood you right

No, I was under the impression that you didn't realize the distinction between the Cabal version that stack depends on as a library, and the Cabal version that stack uses to compile other packages.

But you did?

[sjakobi]

Thanks, that's a nice improvement.

[hesiod]

It certainly looks like an improvement from this POV, but actually, this just reverts the changes I made earlier to check for Cabal >= 1.21.1, because checking for a Cabal development version just isn't really useful. But thanks for the compliment anyway!

[hesiod]

Well, @sjakobi, I think we managed to misunderstand each other on multiple levels 😄

1. Difference between "the Cabal version that stack depends on as a library, and the Cabal version that stack uses to compile other packages" - didn't realize that at first, but after your comments, it became clear to me
2. You meant that "the distinction of the two Cabal versions should also be documented somewhere" - I thought you were referring to the difference of Cabal < 1.21.1 and Cabal >= 1.21.1 regarding the --enable-executable-profiling flag, i.e. the subject of this patch, and were asking me to document this behaviour, but I wasn't sure about that, so I asked you to clarify. Since I indeed misunderstood your question regarding documentation, don't mind my comments above.

[sjakobi]

I think this is the wrong way around now – we want to use --enable-profiling for the more recent versions of Cabal.

[hesiod]

Good catch - fixed in c39acbd

[sjakobi]

Well, @sjakobi, I think we managed to misunderstand each other on multiple levels

Ha! :)

Ok, I think this will be good to merge once my comment on profFlag has been addressed.

I would really like to have this tested at least manually with Cabal-1.20 though, so in case you'd also like to pick up #2386 be sure to let me know! :)

[sjakobi]

LGTM!