Skip to content
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

Print what cabal.project files get used (fixes #8519) #8617

Closed

Conversation

pigsinablanket
Copy link

  • Print what cabal.project files get used

  • Update testsuite


Please include the following checklist in your PR:

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

* Print what cabal.project files get used

* Update testsuite
@ulysses4ever
Copy link
Collaborator

ulysses4ever commented Nov 24, 2022

Thank you! We need to look into the test failures one by one, I guess.

UNEXPECTED FAIL: PackageTests/Configure/cabal.test.hs PackageTests/ListBin/Script/cabal.test.hs PackageTests/Freeze/disable-benchmarks.test.hs PackageTests/Get/OnlyDescription/cabal.test.hs PackageTests/Freeze/dry-run.test.hs PackageTests/Freeze/freeze.test.hs PackageTests/Freeze/enable-benchmarks.test.hs PackageTests/Freeze/disable-tests.test.hs PackageTests/Freeze/enable-tests.test.hs PackageTests/NewBuild/CmdClean/Orphan/cabal.test.hs PackageTests/NewBuild/CmdClean/Keep/cabal.test.hs PackageTests/NewBuild/CmdClean/Script/cabal.test.hs PackageTests/NewBuild/CmdRepl/ScriptRerun/cabal.test.hs PackageTests/NewBuild/CmdRepl/Script/cabal.test.hs PackageTests/NewBuild/CmdRun/ScriptRerun/cabal.test.hs PackageTests/NewBuild/CmdRun/ScriptNoExtension/cabal.test.hs PackageTests/NewBuild/CmdRun/ScriptWithProjectBlock/cabal.test.hs PackageTests/NewBuild/CmdBuild/ScriptRerun/cabal.test.hs PackageTests/NewBuild/CmdBuild/Script/cabal.test.hs PackageTests/NewBuild/CmdBuild/ScriptBuildRepl/cabal.test.hs PackageTests/NewBuild/CmdBuild/ScriptBuildRun/cabal.test.hs PackageTests/NewUpdate/UpdateIndexState/update-index-state.test.hs

I'll need to get to desktop to go over those but the first couple I saw are as follows.

  • PackageTests/Configure/cabal.test.hs — doesn't look like the output was updated: it just says that the new message about project files is unexpected. Can you make sure that you --accepted it?

  • PackageTests/ListBin/Script/cabal.test.hs — I wonder how list-bin tests work at all: list-bin is supposed to show the actual location... Can you inspect the PackageTests/ListBin/Script directory and see what was there before, especially the .out file...

@Mikolaj Mikolaj added type: enhancement re: project-file Concerning cabal.project files labels Nov 24, 2022
@ffaf1
Copy link
Collaborator

ffaf1 commented Nov 24, 2022

I see some files have not been updated, e.g. PackageTests/Configure, which leads to +- <ROOT>/cabal.project errors.

Are you sure you --accepted all the tests?

@pigsinablanket
Copy link
Author

  • PackageTests/Configure/cabal.test.hs - I was missing some deps and it got skipped, reran and accepted it.
  • Nothing particularly unexpected about the ListBin/Script/cabal.out
diff --git a/cabal-testsuite/PackageTests/ListBin/Script/cabal.out b/cabal-testsuite/PackageTests/ListBin/Script/cabal.out
index 2a3f04466..dda79e79c 100644
--- a/cabal-testsuite/PackageTests/ListBin/Script/cabal.out
+++ b/cabal-testsuite/PackageTests/ListBin/Script/cabal.out
@@ -1,4 +1,6 @@
 # cabal list-bin
+This build is affected by the following project file(s):
+- /home/pigs/git/cabal/cabal.project
 Resolving dependencies...
 Build profile: -w ghc-<GHCVER> -O1
 In order, the following will be built:

However, as I expressed concern in #8519, --accept is using a hard coded absolute path instead of <ROOT> is causing a failure when ran on a different machine and I suspect that is what is happening with the other failing tests.

  This build is affected by the following project file(s):
-- /home/pigs/git/cabal/cabal.project
+- /home/runner/work/cabal/cabal/cabal.project

@ulysses4ever
Copy link
Collaborator

Indeed, all other failures come from absolute paths (recorded by --acepted, according to pigsinablanket). We need someone with deeper knowledge of the test suite here...

@pigsinablanket
Copy link
Author

Observation: The path doesn't normalize when it uses the root level git directory cabal.project, but does normalize the path when the test has its own cabal.project in it's own directory.

Suggestion: We can extend NormalizerEnv to include a field for the root level git path alongside the existing test source directory. I am suggesting changing <ROOT> to <TEST_ROOT> and add a PROJECT_ROOT.

@ulysses4ever
Copy link
Collaborator

@pigsinablanket good catch! Sounds like a plausible explanation and a right way to fix it. Hope to see it fixed!

@ulysses4ever
Copy link
Collaborator

@pigsinablanket please, feel free to let us know if there's any trouble with your plan! Maybe together we can find a solution.

@pigsinablanket
Copy link
Author

@pigsinablanket please, feel free to let us know if there's any trouble with your plan! Maybe together we can find a solution.

I am having some difficulty discovering the root path of the git directory. I noticed there was a gitProgram definition and had plans to use that to run something like git rev-parse --show-toplevel but I haven't figured out how to plumb the information around. Thoughts?

@ulysses4ever
Copy link
Collaborator

@pigsinablanket looks like we're lacking the knowledge of this aspect of the test suite, and you may have discovered even more than we understand. Could you, please, post a little commentary of what you've tried so far with pointers to code related to this task (adding support for <PROJECT_ROOT>).

@Mikolaj
Copy link
Member

Mikolaj commented Dec 29, 2022

@pigsinablanket: ping? We'd like to help. :)

@ulysses4ever
Copy link
Collaborator

@pigsinablanket another gentle ping. It'd be awesome to have it in 3.10, for which the window is rapidly closing, sadly…

@ulysses4ever
Copy link
Collaborator

Dear all, may I suggest something silly but easier. Can we simply not record this output? I'm not knowledge in details of *withMarkedOutput and alike. @andreasabel I think you are? Do you have an idea how to exclude this from the reference output?

We can always open a ticket requesting a change along the lines described above (<PROJECT_ROOT> etc). But this way we don't make the perfect enemy of the good: our users don't care about intricacies of our test suite, and the demand to have the message about the project file is pressuring.

@andreasabel
Copy link
Member

@pigsinablanket wrote:

Observation: The path doesn't normalize when it uses the root level git directory cabal.project, but does normalize the path when the test has its own cabal.project in it's own directory.

I guess we are talking about mismatches like https://github.com/haskell/cabal/actions/runs/3538370618/jobs/5939148567#step:18:3042

--- /home/runner/work/cabal/cabal/cabal-testsuite/PackageTests/ListBin/Script/cabal.dist/cabal.out.normalized	2022-11-24 07:21:05.997788646 +0000
+++ /home/runner/work/cabal/cabal/cabal-testsuite/PackageTests/ListBin/Script/cabal.dist/cabal.comp.out.normalized	2022-11-24 07:21:05.997788646 +0000
@@ -1,6 +1,6 @@
 # cabal list-bin
 This build is affected by the following project file(s):
-- /home/pigs/git/cabal/cabal.project
+- /home/runner/work/cabal/cabal/cabal.project

@andreasabel
Copy link
Member

I notice that with this PR we are getting informed about the project files even when doing cabal update:

$ cabal update
...
This build is affected by the following project file(s):
- .../cabal.project
- .../cabal.project.local
Downloading the latest package list from hackage.haskell.org

I suppose there could be repositories listed in these project files which get updated by cabal update, so it is fine to inform us.

But maybe the formulation should be more neutral, instead of

This build is affected by the following project file(s)

say something like

Taking into account the following project file(s)

@ulysses4ever ulysses4ever added the attention: needs-help Help wanted with this issue/PR label Mar 19, 2023
@Kleidukos
Copy link
Member

@pigsinablanket We'd like to offer our help with rebasing your PR. Would you accept it? :)

@ulysses4ever
Copy link
Collaborator

No activity for a long time and superseded by #10507. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attention: needs-help Help wanted with this issue/PR re: project-file Concerning cabal.project files type: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display within which cabal.project we build
6 participants