-
Notifications
You must be signed in to change notification settings - Fork 711
With v2 commands being the default, use dist-newstyle. #8833
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Just in case, did you check if defaultDistPref
is used anywhere besides help messages? If so, the change may have unpredictable consequences. I'm approving because I'm reckless when it comes to breaking backwards compatibilit, but other may be more cautious…
I ran the tests locally.
I found 15 hits in 7 files for cabal/cabal-install/src/Distribution/Client/Install.hs Lines 1274 to 1285 in 0f51816
|
I also noticed a hard-coded cabal/Cabal/src/Distribution/Simple.hs Lines 516 to 527 in 0f51816
|
The word or abbreviation "dist" is also used in a few strings that can be found grepping for cabal/Cabal-tests/tests/HackageTests.hs Lines 369 to 371 in 0f51816
cabal/cabal-testsuite/main/cabal-tests.hs Line 105 in 0f51816
|
Thank you for the PR. From the PR description alone I can't guess if this changes the behaviour of cabal or only the help text and whether it affects only v2 or also the Setup and v1 API. If it does change the behaviour, could we list the changes with tickboxes and for each discuss the consequences (to correctness and to backward compatibility)? |
4004c13
to
3c6c24d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please feel free to set the "merge_me" label
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhoh, I've just found #7924. Could we make sure that changes in "multiple files, in separate packages" are not really needed and document here why not?
I am not sure whether this is the right fix or not (I err towards not). As @Mikolaj points out this affects the v1- and ./Setup API so now the default location for those commands will also be |
3c6c24d
to
6cfc38f
Compare
From #8832, bumps the location of the default directory where Cabal puts generated build files to
(default dist-newstyle)
.