v2-sdist now respects --ignore-project

[cbclemmer]

It seems that many commands did not respect the --ignore-project flag at all and always built from the cabal.project file that the program can find either in it's current directory or the parent directories. This PR changes the readProjectConfig function of the Distribution.Client.ProjectConfig to accept a boolean that will determine if the function will return the only the global project config or the settings found in the local cabal.project file. Whatever is returned gets merged with the local foo.cabal file and the package config is returned. This should make it so that if the --ignore-project flag is set then the local cabal.project file will be ignored and the global config and the foo.cabal file are merged together and the sdist command is run with that configuration.

There was some discussion about the other commands that may or may not work with this flag. It was unclear to me whether or not they should be fixed and to what extent. This pr is just to have a proof of concept for the other commands so that I know that this flag is now doing what it is supposed to do and more work can be done later to the other commands making it behave the same way.

I wrote a test for this that tests the readProjectConfig with the flag set to true (all other tests that use this command were set to false for consistency). I'm not sure how to abstract this test any farther up the chain but that may not be needed since the readProjectConfig function is the only one with any significant change.

Below is some output with the new changes showing that the --ignore-project flag ignores the cabal.project file for the "cabal" project when in the "cabal-install" folder.

---

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog.
• The documentation has been updated, if necessary.

[cbclemmer]

I didn't see the errors on validate.sh when I made the PR. I'm working on fixing them. It looks like the --project-file flag and the --ignore-project flag don't work together well. I assume that the --project-file flag should take precedence to the --ignore-project flag. I'll ensure that happens and turn it back to review, unless something with tests needs to change.

[Mikolaj]

There have been some CI (validate) breakage in the last days, just fixed. If you rebase, perhaps some errors are going to vanish?

[jneira]

@Mergifyio rebase master

[mergify]

rebase master

✅ Branch has been successfully rebased

[cbclemmer]

Ok, I think I have it ironed out. The reason the tests were failing is because the tests implicitly use the "--project-file" flag that is not given in the test file but in configuring the cabal function for the test file. This caused "--ignore-project" flag to be set to false and messed up the test. I think there were some tests, especially in the "PackageTests/SDist" directory, that needed the "--ignore-project" flag not because it had anything to do with the thing being tested, but because it would fail otherwise. I removed all references to "--ignore-project" in these tests because they now passed either way when they would not pass on master without the "--ignore-project" flag. I went through the issues that were linked to the tests I changed and it seems like none of them mention the "--ignore-project" flag at all.

Also, just so I know I'm fixing the issue at hand I made the example project given by @andreabedini in #7965 and this was the result.

~/tmp/test » ./cabal v2-sdist 
Cloning into '~/tmp/test/dist-newstyle/src/flat-5d3bff3e9951fc1'...
remote: Enumerating objects: 4069, done.
remote: Counting objects: 100% (895/895), done.
remote: Compressing objects: 100% (344/344), done.
^Cfetch-pack: unexpected disconnect while reading sideband packet

~/tmp/test » rm -rf dist-newstyle                                                                                                   130 ↵
~/tmp/test » ./cabal v2-sdist --ignore-project
Wrote tarball sdist to
/home/colton/tmp/test/dist-newstyle/sdist/test-0.1.0.0.tar.gz

[cbclemmer]

I also added two "cabal.project" files to two of the tests because the "--project-file" flag is always set to "cabal.project". This made it look for the "cabal.project" file and found it in the root cabal directory. I assume that using the "--ignore-project" flag was an attempt to mitigate this behavior but it isn't its intended functionality and whenever I fixed it, it broke what I assume was a less than perfect solution. I hope my changes better reflect the way it's supposed to work. It if doesn't I can change the functionality.

[cbclemmer]

I have no idea why these tests are failing now
Error: Ambiguous module name ‘System.Directory’:
Is happening on multiple tests. Some haven't been changed in years.

[Mikolaj]

Yes, apologies. Probably some problems with GHA cache: #8120 (comment)

[Mikolaj]

Now the cache fixed itself. Let's hope it won't repeat.

[Mikolaj]

@Colton-Clemmer, @jneira: are you fine to merge?

[jneira]

@Colton-Clemmer, @jneira: are you fine to merge?

thanks for the ping, only a minor comment, marked as resolved the previous ones

[cbclemmer]

@Mikolaj Thanks for letting me know that cleared up

[Mikolaj]

Apparently some validates timed out (1h, cancelled). I blame GHA until proven innocent. Will restart once it finishes.

[cbclemmer]

Can someone help this PR along? not sure what's wrong with it.

[andreabedini]

@Colton-Clemmer It looks like the workflow had failed for some network issue, then it got rerun but mergify still thinks it had failed.

Let's try this:

@Mergifyio refresh

[andreabedini]

I don't think it worked :-/

[Mikolaj]

Strange. Let's be insistent.

[Mikolaj]

@Mergifyio rebase

[mergify]

rebase

✅ Branch has been successfully rebased

[Mikolaj]

@Mergifyio refresh

[mergify]

refresh

✅ Pull request refreshed

[Mikolaj]

@Mergifyio requeue

[mergify]

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

[Mikolaj]

I think the problem may be that the docs/readthedocs.org:cabal job failed randomly and we have no way of restarting it (unlike all other tests). So we need to make some fake commits or ammends to restart all tests. I will try something.

[Mikolaj]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated

[Mikolaj]

This seems to have done the trick (that is, restarted all tests). This merge commit I added is probably going to vanish after the next rebase, but if not, please remove it, because it pollutes git history

[Mikolaj]

O dear, I haven't noticed the merge_me label. But, luckily, the git history pollution is minimal. Thank you for this contribution!

[andreabedini]

Thank you @Colton-Clemmer !