cabal icon indicating copy to clipboard operation
cabal copied to clipboard

Fix nix config option

Open cbclemmer opened this issue 3 years ago • 3 comments

Closes #8452 I did not create a test for the config files when I made #8054. This should test config files for leaving out nix, "nix: True", and "nix: False". This also adds a line in the --help output for the nix option, it may not be necessary and can be easily removed if it is not needed.

@mhwombat it would be useful for you to confirm that this fix removes the error you were seeing @Mikolaj I assigned you to review, if this isn't correct, please redirect the review.


Please include the following checklist in your PR:

Please also shortly describe how you tested your change. Bonus points for added tests!

cbclemmer avatar Oct 10 '22 17:10 cbclemmer

But uh... let me get the tests passing firsts

cbclemmer avatar Oct 10 '22 18:10 cbclemmer

It looks like this pr for bytestring is causing the tests to fail. It looks like the test that failed depended upon getting the latest version of bytestring and then building it, it then tried to build the data-array-byte package but couldn't find it. I'm getting the same results locally when running the test command but I can build the bytestring repo locally just fine. Maybe this is an issue with the server caching packages or something? @Mikolaj What do you think?

cbclemmer avatar Oct 10 '22 22:10 cbclemmer

Relevant links: https://github.com/haskell/cabal/blob/master/cabal-testsuite/PackageTests/postCheckoutCommand/cabal.positive.project https://github.com/haskell/bytestring/pull/410 https://github.com/Bodigrim/data-array-byte

cbclemmer avatar Oct 10 '22 22:10 cbclemmer

@Mikolaj Tests are passing 🎉

cbclemmer avatar Oct 15 '22 19:10 cbclemmer

@cbclemmer: thank you for the diagnosis of the spurious CI failure. Indeed that seems to have been the problem and we have since fixed it after a lot of confusion. A pity I haven't looked at this PR earlier. :)

@mhwombat, might we solicit your review? Even just an absent-minded look at the code, a checkout, a run and a report in a comment that it works fine now? I will have a closer look now, but another pair of eyes is very welcome (and also we formally require two reviews for each PR).

Mikolaj avatar Oct 17 '22 10:10 Mikolaj

The CI failure is probably caused by https://bugs.launchpad.net/ubuntu/+source/git/+bug/1993586 (and by us using the latest Ubuntu container).

Mikolaj avatar Oct 21 '22 16:10 Mikolaj

May be fixed in https://github.com/haskell/cabal/pull/8546 (if CI passes there).

Mikolaj avatar Oct 21 '22 16:10 Mikolaj

The CI fix seems to work. Let me rebase on it...

Mikolaj avatar Oct 21 '22 19:10 Mikolaj

@mergify rebase

Mikolaj avatar Oct 21 '22 19:10 Mikolaj

rebase

✅ Branch has been successfully rebased

mergify[bot] avatar Oct 21 '22 19:10 mergify[bot]

@Mikolaj Thank you, I was really confused

cbclemmer avatar Oct 21 '22 22:10 cbclemmer

This time it seems it failed due to some corruption, hanging and cancellation after 360min. I'm restarting.

Mikolaj avatar Oct 22 '22 07:10 Mikolaj

Succes. Quick, @cbclemmer, set one of the merge_me labels to merge the PR while it still passes CI. :)

Mikolaj avatar Oct 22 '22 09:10 Mikolaj

Edit: with mergify waiting 2 days after the label is set, as per the process.

Mikolaj avatar Oct 22 '22 09:10 Mikolaj