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

Reject index-states after last known index-state #8944

Merged

Conversation

jasagredo
Copy link
Collaborator

@jasagredo jasagredo commented May 9, 2023

The index-state warning expresses an internal mismatch in the existing index-states and the requested one, but this does not need an action from the user. On the other hand, requesting an index-state after the last known index means that the user should perform cabal update. This PR demotes the first one to verbose mode only and throws an error on the second one.

QA notes

When building a project with a timestamp for which no index-state exists, cabal now warns the user and picks the previous index-state.

The index-state warning that said:

Warning: Requested index-state 2023-04-24T00:00:09Z is newer than
'hackage.haskell.org'! Falling back to older state (2023-04-23T19:32:13Z).
  1. Setting the index-state in-between two existing index-states warns in verbose mode
❯ cabal build all -v
...
Reading available packages of hackage.haskell.org...
Using historical state as of 2023-05-09T13:27:17Z as explicitly requested (via
command line / project configuration)
There is no index-state for 'hackage.haskell.org' exactly at the requested
timestamp (2023-05-09T13:27:17Z). Falling back to the previous index-state
that exists: 2023-05-09T12:58:00Z
index-state(hackage.haskell.org) = 2023-05-09T12:58:00Z (HEAD =
2023-05-09T12:58:00Z)
  1. Setting the index-state after the most-recent known index-state errors
❯ cabal build all
Error: cabal: Latest known index-state for 'hackage.haskell.org'
(2023-05-09T13:27:18Z) is older than the requested index-state
(2024-04-24T00:00:00Z). 
Run 'cabal update' or set the index-state to a value at or before
2023-05-09T13:27:18Z.
  1. Setting the index-state before the oldest known index-state warns with normal verbosity
Warning: There is no index-state for 'hackage.haskell.org' exactly 
at the requested timestamp (1900-01-01T00:00:00Z). Also, there are 
no index-states before the one requested, so the repository 
'hackage.haskell.org' will be empty.

Please include the following checklist in your PR:

Bonus points for added automated tests!

ulysses4ever
ulysses4ever previously approved these changes May 9, 2023
Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thank you!

Do you think you could factor out the message so that we don’t have two copies of it in the source?

I think putting a line break before “Suggestion:” would make it prettier: there’ll be nice parallelism with “Warning:”. But that’s subjective of course.

@Kleidukos
Copy link
Member

Hi @jasagredo and thank you for pushing this.

I'd like to suggest that the message is reworded with perhaps less "reassurance" and more "what next" phrasing.
In all cases I've encountered this message, it meant that I was trying to build someone's project whilst not having run cabal update for a bit on my machine.

What do you think of something like:

Warning: The project requires index-state for hackage.haskell.org to be 2023-04-24T00:00:09Z,
but it is not available.
Please run `cabal update` to get the latest state.
In the meantime, falling back to use 2023-04-23T19:32:13Z.

@ulysses4ever
Copy link
Collaborator

ulysses4ever commented May 9, 2023

Yes, I really like @Kleidukos’ suggestion!

@jasagredo
Copy link
Collaborator Author

jasagredo commented May 9, 2023

That sounds good but there are some problems I think. From what I can see, not having done cabal update will not produce any warnings or errors:

❯ grep hackage cabal.project
  , hackage.haskell.org 2024-04-24T00:00:00Z

❯ cabal build mylib
Build profile: -w ghc-9.2.7 -O1
In order, the following will be built (use -v for more details):

Notice I set the index state to next year.

However this error is printed when there is not an index state at the specified timestamp so that cabal falls back to the previous one.

I think the case you want to express is not exactly this one. However I agree with looking for a rephrasing, what about:

Warning: The project requires index-state for hackage.haskell.org to be 
2023-04-24T00:00:09Z, but it is not available or doesn't exist. 
Suggestion: Run `cabal update` to get the latest existing index-state.
Warning: Falling back to use the latest index-state cabal knows about 
before the requested timestamp: 2023-04-23T19:32:13Z. 
Suggestion: Use 2023-04-23T19:32:13Z as the index-state instead of 
the requested one. The build plan will be the same.

(Once I add more lines in becomes less readable :/)

@angerman
Copy link
Collaborator

angerman commented May 9, 2023

I don't think cabal should report anything here except with -v, unelss the known indices are older than the request index state.

If the known indices are newer than the requested index state, there should be no noise at all (unless explicitly requested with -v).

What's the value of telling users that the index state they choose is newer than the last one in the index, when we know the index has newer states as well?

This just leads users to produce patches to adjust the index states precisely to index state changes. Only to silence cabal. I don't see any value in these kinds of changes.

Thusly,
if latestStateOfIndex(index) > requestedStateOfIndex(index) do nothing.
If requestedStateOfIndex(index) > latestStateOfIndex(index), tell the user they need to run cabal update and abort! This is a fatal error, you will not get a reliable build plan. Either fix the index state, or update your index. The times people have had solver issues because their index wasn't up-to-date, and they missed the minor warning that cabal prints, I can't count on my fingers anymore.

if -v be chatty about it.

@jasagredo
Copy link
Collaborator Author

I agree that acting on this warning is only going to silence cabal, the result of adjusting the index-state will be the same if not done.

@michaelpj
Copy link
Collaborator

michaelpj commented May 9, 2023

If requestedStateOfIndex(index) > latestStateOfIndex(index), tell the user they need to run cabal update and abort! This is a fatal error.

I will say that I have occasionally used an index-state in the future (e.g. tomorrow) as a shorthand for "now or very soon", but I'd probably live. Perhaps this is the case that should be a warning: that's something that the user might want to address (but might be fine).

EDIT: actually, I could just have used HEAD in those circumstances and that would have expressed my desire better, so I withdraw my objection.

@jasagredo
Copy link
Collaborator Author

What about this:

  • Setting the index to a future timestamp
❯ /home/javier/code/cabal/dist-newstyle/build/x86_64-linux/ghc-9.2.7/cabal-install-3.11.0.0/x/cabal/build/cabal/cabal build all
Error: cabal: Latest known index-state for 'hackage.haskell.org'
(2023-05-09T13:27:18Z) is older than the requested index-state
(2024-04-24T00:00:00Z). Run 'cabal update' or set the index-state to a value
at or before 2023-05-09T13:27:18Z.
  • Setting the index to a non existing timestamp but older than the on-disk index-state file (no warning)
❯ /home/javier/code/cabal/dist-newstyle/build/x86_64-linux/ghc-9.2.7/cabal-install-3.11.0.0/x/cabal/build/cabal/cabal build all
Resolving dependencies...
  • Same but verbose
❯ /home/javier/code/cabal/dist-newstyle/build/x86_64-linux/ghc-9.2.7/cabal-install-3.11.0.0/x/cabal/build/cabal/cabal build all -v
...
Reading available packages of hackage.haskell.org...
Using historical state as of 2023-05-09T13:27:17Z as explicitly requested (via
command line / project configuration)
There is no index-state for 'hackage.haskell.org' exactly at the requested
timestamp (2023-05-09T13:27:17Z). Falling back to the previous index-state
that exists: 2023-05-09T12:58:00Z
index-state(hackage.haskell.org) = 2023-05-09T12:58:00Z (HEAD =
2023-05-09T12:58:00Z)

@Kleidukos
Copy link
Member

Last nitpick: We should have the

run cabal update

On a new line.

Otherwise, all good for me! And my thanks again for your interest in improving our UΧ.

@jasagredo jasagredo force-pushed the jasagredo/improve-index-state-warning branch from 7ea4dfd to b7d12f5 Compare May 9, 2023 15:28
@jasagredo
Copy link
Collaborator Author

I fixed up the last commit and the PR description. However this change now would require a changelog entry at least and maybe some documentation, but I cannot do that now, so I'm leaving the checkboxes un-ticked.

@jasagredo jasagredo force-pushed the jasagredo/improve-index-state-warning branch from b7d12f5 to 16cea77 Compare May 10, 2023 08:51
@jasagredo
Copy link
Collaborator Author

I added a changelog entry. This is ready to merge I think

@jasagredo jasagredo changed the title Reword index-state mismatch warning Reject index-states after last known index-state May 10, 2023
@Kleidukos Kleidukos added the attention: needs-manual-qa PR is destined for manual QA label May 10, 2023
@Kleidukos
Copy link
Member

All good to me! :)

@Kleidukos Kleidukos added the squash+merge me Tell Mergify Bot to squash-merge label May 10, 2023
Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch evolved from a simple rewording to two changes in behavior: one with making cabal less verbose and one with a new failure condition. This is a significant change. I think someone else should look into it. @gbaz @fgaz @Mikolaj

@ulysses4ever ulysses4ever dismissed their stale review May 10, 2023 09:44

PR evolved significantly.

@Kleidukos Kleidukos removed the squash+merge me Tell Mergify Bot to squash-merge label May 10, 2023
Copy link
Collaborator

@andreabedini andreabedini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments inline. Also we need tests to codify the behaviour in all situation. See cabal-testsuite/PackageTests/NewUpdate/UpdateIndexState/ for inspiration.

cabal-install/src/Distribution/Client/IndexUtils.hs Outdated Show resolved Hide resolved
cabal-install/src/Distribution/Client/IndexUtils.hs Outdated Show resolved Hide resolved
@Kleidukos
Copy link
Member

@jasagredo Do you want my help to fix the conflicts of your PR?

@jasagredo
Copy link
Collaborator Author

I can try to fix it next week is I get some free time after work, if that is OK. I think from @andreabedini's comments that I need to rethink the approach

@Kleidukos
Copy link
Member

@jasagredo yes of course, just do not hesitate to reach out if you're facing difficulties

@jasagredo jasagredo force-pushed the jasagredo/improve-index-state-warning branch from 16cea77 to ef9bf9e Compare June 6, 2023 13:06
Copy link
Collaborator

@andreabedini andreabedini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jasagredo for taking the time to update this. I think it looks good now.
I am suggesting to leave a little comment to our future selves and to revert the change around warnIfIndexIsOld. Let me know if you agree with this.

cabal-install/src/Distribution/Client/IndexUtils.hs Outdated Show resolved Hide resolved
cabal-install/src/Distribution/Client/IndexUtils.hs Outdated Show resolved Hide resolved
@jasagredo jasagredo force-pushed the jasagredo/improve-index-state-warning branch from ef9bf9e to e005225 Compare June 22, 2023 13:40
alt-romes pushed a commit to alt-romes/cabal that referenced this pull request Dec 4, 2023
…ange only. (haskell#9386)

* add more breadcrumbs for how to use remote packages.

* Hackage should be capitalized

* Update doc/cabal-project.rst

reword.

* Update doc/cabal-project.rst

reword.

* Update doc/cabal-project.rst

clearer.

* Update doc/cabal-project.rst

clearer!

* Update doc/cabal-project.rst

good mechanical description.

* wrap.

* Update doc/cabal-project.rst

add missing double ticks.

* clarify english, and follow a linguistic pattern better.

* Update doc/cabal-project.rst

* doc: render math with HTML to make it selectable (fix haskell#8453) (haskell#9361)

* doc: render math with HTML to make it selectable (fix haskell#8453)

* Update doc/conf.py

Co-authored-by: Bryan Richter <[email protected]>

---------

Co-authored-by: Bryan Richter <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Avoid double space in "Executing install plan ..."

* Add a change log entry for double space avoidance

* Ignore CmmSourcesExe Demo

Ignore because it warns about missing MachDeps.h

* [cabal-7825] Implement external command system

Fix haskell#2349 and haskell#7825

* Bump to latest dependencies for GHC 9.8.1

* cabal.project: clean out obsolete `allow-newer`s

* update GH validate workflow to ghc 9.2.8, 9.4.7, 9.6.3

* Revert haskell#3639 (Don't pass -package-db and -package flags to --abi-hash) (haskell#9384)

* Revert haskell#3639 (Don't pass -package-db and -package flags to --abi-hash)

With ghc>=9.6 `ghc --abi-hash` initialises the plugins so it will fail
if a cabal file specifies `ghc-options: -fplugin=Foo`.

Closes: haskell#9375

* Also revert in GHC.hs

---------

Co-authored-by: Hamish Mackenzie <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Use the newer haskell-actions organisation

* Restructure Cabal documentation top-level parts

The goal is for users to easier find pages for typical problems through search engines and page navigation.
- The top-level layout is based on the popular documentation structure by https://documentation.divio.com/ to give a
   clear structure to users and future documentation contributors:
  * Guides: Present a solution to a single, atomic, typical user problem.
  * Reference: Describe user API (CLI fields, syntax etc) with technical rigour and completeness.
  * Explanation: Discuss background information, scope, design decisions etc.
- Move existing documentation roughly into these categories with minimal editing as the basis for further editing.
- Rename guide titles to mention how-to for improving SEO.
- Rename some files to improve SEO since that name becomes part of the URL (often called slug).
  Important page keywords should appear in the slug as well to make pages rank higher in search engines.

* Rename master_doc to root_doc (changed in version 4.0 of Sphynx)

* Add instance Ord for Field, FieldLine, SectionArg and Name

* Do not run CI for documentation changes

The github workflows are not run if the
changes are completely contained within
the doc/ subdirectory. The only exception
is the users-guide.yml github action.

* Move Backpack section to user guides

* Remove TBW virtual modules section

* Add reinstall test to LinkerOptions/NonignoredConfigs

* Record install options

* Reject index-states after last known index-state (haskell#8944)

Co-authored-by: Javier Sagredo <[email protected]>
Co-authored-by: Andrea Bedini <[email protected]>
Co-authored-by: Andrea Bedini <[email protected]>

* Note how to do "not equal" with constraints

* Use comma with then

Co-authored-by: Artem Pelenitsyn <[email protected]>

* Use narrow rather than upset

* Say something about hackage deprecations

* Fix AutogenModulesToggling test

By converting this to a setupTest we use the in-tree Cabal library
rather than relying on a proxy of the GHC version to provide the right
Cabal library version.

Supersedes haskell#9398

* Require version 3,11 of Cabal to support --semaphore flag

Fixes haskell#9197

* Add dependencies used by `PackageTests` to exe:cabal-tests

The runner allows the tests to use extra dependencies and the custom Prelude
from 'cabal-testsuite'.
However, if the tests use a dependency, say 'directory', and there are two
packages with the same unit id available in the store, the test fails since
it doesn't know which one to pick.
By including an extra dependency to directory, we force the test runner to
use a specific version directory, fixing the test failure.

* Use Paths_cabal_install for cabal-install version number (haskell#9421)

* Use PackageInfo for cabal-install version number

* Use Paths_cabal_install instead

* Adjust documentation

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Document --profiling-detail in setup-commands.

Fixes haskell#9182

* Add test requirement to PR template

Adding test becomes a checkmark instead of “bonus points”.

* A 'cabal path' command. (haskell#8879)

* Add a 'cabal path' command.

* Formatting fix.

* Another formatting fix.

* Categorise "cabal path" as global command.

* Allow individual paths to be printed.

* Less duplication.

* Add config-file to "cabal path".

* Use sum type instead of strings.

* cabal path: support --installdir.

* Add documentation.

* Better text.

* Formatting.

* Add some tests.

* Improve tests.

* Add changelog entry.

* Mention "cabal path" in directory documentation.

---------

Co-authored-by: Artem Pelenitsyn <[email protected]>

* Reimplement `cabal check` (haskell#8427)

* Fix Semigroup target instance

When two target names are the same, `mappend`ing them should not
error but just pick the first name.

* Add `desugarBuildToolSimple`

* Reimplement cabal check

* Reorder test output

* Fix autogen modules tests .cabal files

* Add a number of tests

* Add test for haskell#7423

i.e. Do not warn on -O2 if under off-by-default package configuration
flag conditional.

* Add a regression for:

    * Add another -WErrr test
        This is to make sure we do *not* report it if it is under
        a user, off-by-default flag.
    * Add test for non manual user flags.
    * Add “absolute path in extra-lib-dirs” test
    * Add if/else test
    * Add “dircheck on abspath” check
    * Add Package version internal test
    * Add PackageVersionsStraddle test

* Add changelog for haskell#8427

* Integrate various reviews

* Integrate Artem’s review

(review) Clarify `combineNames` documentation

By explaining the way it operates (working if the two names are equal
or one is empty) and renaming the function from `combineName` to
`combineNames`.

(review) Use guards instead of if/then/else

(review) Match inside argument list

(review) Replace “white” with “allow”

(review) Fix typo in comment

(review) Fix typo in Check module documentation

(review) Harmonise indentation for `data` decls

First field goes in a new line than the data constructor, so we
have more space.

(review) Rename `Prim` module to `Types`

(review) Add checkPackageFilesGPD

`checkPackageFiles` — which works on PD — was used to perform IO. We
introduce a function that does the same thing but works on GPD (which
is more principled).

`checkPackageFiles` cannot just be removed, since it is part of the
interface of Distribution.PackageDescription.Check. Deprecation can
be planned once “new check” is up and running.

* Integrate Andreas’ review

(review) Add named section to missing upper bound check

“miss upper bound” checks will now list target type and name (“On
executable 'myexe', these packages miss upper bounds”) for easier
fixing by the user.

(review) remove `cabal gen-bounds` suggestion

Reasonable as `cabal gen-bounds` is stricter than `cabal check`, see
haskell#8427 (comment)
Once `gen-bounds` behaves in line with `check` we can readd the
suggestion.

(review) Do not warn on shared bounds

When a target which depends on an internal library shares some
dependencies with the latter, do not warn on upper bounds.

An example is clearer

    library
     build-depends: text < 5
    ⁝
     build-depends: myPackage,        ← no warning, internal
                    text,             ← no warning, shared bound
                    monadacme         ← warning!

* Integrate Artem’s review /II

(review) Split Check.hs

Check.hs has been split in multiple file, each une sub 1000 lines:

Check              857 lines
Check.Common       147 lines
Check.Conditional  204 lines
Check.Monad        352 lines
Check.Paths        387 lines
Check.Target       765 lines
Check.Warning      865 lines

Migration guide:
- Check              GPD/PD checks plus work-tree checks.
- Check.Common       common types and functions that are
                     *not* part of monadic checking setup.
- Check.Conditional  checks on CondTree and related matter
                     (variables, duplicate modules).
- Check.Monad        Backbone of the checks, monadic inter-
                     face and related functions.
- Check.Paths        Checks on files, directories, globs.
- Check.Target       Checks on realised targets (libraries,
                     executables, benchmarks, testsuites).
- Check.Warning      Datatypes and strings for warnings
                     and severities.

(review) remove useless section header

(review) Fix typo

(review) Add warnings documentation (list)

For each warning, we document constructor/brief description
in the manual.  This might not be much useful as not but it
will come handy when introducing `--ignore=WARN` and similar
flags.

* (review Andreas) Clarify CheckExplanation comment

Whoever modifies `CheckExplanation` data constructors needs to be
aware that the documentation in  doc/cabal-commands.rst  has to be
updated too.

* Move internal Check modules to `other-modules`

No need to expose Distribution.PackageDescription.Check.*
to the world. API for checking, for cabal-install and other
tools, should be in Distribution.PackageDescription.Check.

* Make fourmolu happy

Cabal codebase has now a formatter/style standard (see haskell#8950).

“Ravioli ravioli, give me the formuoli”

* Do not check for OptO in scripts

See haskell#8963 for reason and clarification requests.

* Remove useless PackageId parameter

It is now in the Reader part of CheckM monad.

* Do not check PVP on internal targets

Internal: testsuite, benchmark.
See haskell#8361.

* Make hlint happy

* Fix haskell#9122

When checking internal version ranges, we need to make sure we
are not mistaking a libraries with the same name but from different
packages. See haskell#9132.

* Fix grammar

neither…nor, completing what done in haskell#9162

* Integrate Brandon’s review: grammar

* Remove unnecessary `-fvia-C` check

Brandon’s review/II.

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* ci: Enable windows tests for 9.6.3

There were two failing tests:

1. CCompilerOverride, was attempting to use gcc.exe rather than
   clang.exe without also overriding the C options which led to
   incorrect options being passed to gcc.exe. The fix is to override to
   clang.exe on ghc-9.4 or newer.
2. ForeignLibs exposes a bug in GHC
   (https://gitlab.haskell.org/ghc/ghc/-/issues/24185) and hence is
   skipped for GHCs newer than 9.4 where it was first introduced.

Towards fixing haskell#8451, we just need to fix the shared library issue now.

* testsuite: Be explicit about runtime test dependencies

Issue haskell#8356 reports occasional errors from running the testsuite about
multiple package versions available. This stems from the invokation of
`runghc` not being explicit about all dependencies of the testsuite.

The solution is provide a component in the cabal file which is explicit
about which packages the tests can depend on. This component has a
build-depends section which lists all the dependencies that the tests
require.

It would be better if this component was a library component but we
can't do this with a Custom setup because of limitations to do with
per-component builds.

Then we also enable `-hide-all-packages`, so the dependency will not be
available if it is not explicitly listed as a dependency.

You could also imagine a future where the Setup.hs script found the test
files and compiled a single executable which would run all the tests,
rather than invoking runghc on each one individually.

Fixes haskell#8356

* hurd: Enable using $ORIGIN in RPATH

GNU/Hurd fully supports RPATH and the $ORIGIN development, and we indeed
want to use it for relocatable installations shipped in Debian GNU/Hurd.

* Fix the platform string for GNU/Hurd

Since version 9.4.7-1, ghc fails to build on the GNU/Hurd port of Debian,
see

https://buildd.debian.org/status/fetch.php?pkg=ghc&arch=hurd-i386&ver=9.4.7-1&stamp=1697717885&raw=0

Error, rule finished running but did not produce file:
  _build/stage0/lib/i386-gnu-ghc-9.4.6/ghc-boot-th-9.4.7/libHSghc-boot-th-9.4.7.a

and indeed, what did get produce was rather
_build/stage0/lib/i386-hurd-ghc-9.4.6/ghc-boot-th-9.4.7/libHSghc-boot-th-9.4.7.a
(i386-hurd instead of i386-gnu).

This is due to confusion between hurd and gnu in various places.  Apparently
previous versions of ghc were using gnu for the GNU/Hurd port, and thus
putting libraries etc. in i386-gnu. So we have to follow the existing
practice.

* Fix configuation of ldProgram

Standard GNU `ld` ues `--relocatable` while `ld.gold` uses a `-relocatable`
flag (with a single `-`). Code will now detect both versions.

* Chain configuration of ldProgram

`ldProgram` gets configured in two places, a seemingly default and
a GHC specific version. The later needs to be updated so that it
first calls the default configuration and then the new GHC version.

* Use linker capability detection to improve linker use

The function `comperSupportsGhciLibs` has been renamed to
`linkerSupportsGhciLibs` because its about the linker not the compiler.

The function `comperSupportsGhciLibs` was using the compiler version
as a proxy for whether the linker supports relocatable objects. Now
support for relocatable objects is detected by running the linker.

* add `merge+no rebase`

and a few typos while reviewing

* Add support for 64-bit SPARC as a separate architecture

Previously, sparc64 was defined as an alias for the 32-bit SPARC
architecture which was true while SPARC mainland was mostly 32
bits. More recently, 64-bit SPARC has become a port of its own,
so it needs to be treated as a separate architecture.

* Remove debug-conflict-sets flag from solver package

Fixes haskell#8937.

The debug-conflict-sets build flag probably hasn't been used for a long time,
and it isn't currently tested. This commit removes the flag, converts the
ConflictSet type back to a newtype, and removes an unnecessary instance.

* Finish improvements to the CI configuration for documentation changes (haskell#9460)

* Add bootstrap postjob to CI config

Add a new job to the bootstrap.yml GitHub action config.
This job succeeds if, and only if, all the other bootstrap
jobs succeed.

* Do not run bootstrap CI jobs for documentation changes

The approach was already introduced in haskell#9355 for the validate jobs.
This commit introduces the same change also for the bootstrap jobs.

* Also ignore CONTRIBUTING.md and README.md in CI

We do not run the entire CI suite for documentation changes.
Previously, only changes which were restricted to the 'docs/'
subdirectory were considered to be documentation changes.
With this commit we also recognize changes to README.md and
CONTRIBUTING.md as documentation changes.

* Document improved CI for documentation in CONTRIBUTING.md

The CONTRIBUTING.md file now mentions that documentation changes
do not waste expensive CI resources.

* Recognize all README.md in subdirs as documentation

Expensive CI jobs should not run on changes which affect only
README.md files.

* formatting: Add style-commit makefile target

This target allows you to format a range of commits, for example:

```
make style-commit COMMIT=HEAD~1
> Last commit is formatted
make style-commit COMMIT=abcde
> Commits between HEAD and abcde are formatted
```

* Fix assertion failure when combining build-tool-depends and --enable-documentation

The `setDocumentation` function was modifying the elaborated package
after the hash was computed. This led to the assertion failing as the
computed hash was different to what was computed in the initial install
plan.

Therefore in order to fix this we either needed to:

1. Set elabBuildHaddocks = False at the point where the hash is
   initially computed.
2. Verify that elabBuildHaddocks = True will not lead to unexpected
   results.

The latter has been implemented.

The elabBuildHaddocks option is only consulted in
`hasValidHaddockTargets`, at which point documentation building the
executable component is disabled because elabHaddockExecutables is
False.

In the added test we ensure this by checking that we didn't build
documentation for the executable which is built because of
build-tool-depends.

Fixes haskell#6006 haskell#8313

* testsuite: Improve error message in findDependencyInStore

* Only move code to Simple/GHC/Build*

* CPP mingw32_HOST_OS for more imports

* cabal-install-solver: fix pkgconf 1.9 --modversion regression

Check that the numbers of *versions* output is equal to the number of pkgconf's

fixes haskell#8923

The pkgconf behavior was reverted upstream in 2.0

(this should cover the case too of checking that equal pkgList lines are output also)

* External commands: Add tests for haskell#9402 haskell#9403 haskell#9404

This adds 4 tests which test the new external commands feature:

* ExternalCommand - Tests the expected usage of external command invoked
  via cabal-install
* ExternalCommandSetup - Tests that the ./Setup interface does not
  support external commands (haskell#9403)
* ExternalCommandEnv - Tests that environment variables are set and
  preserved appropiately  (haskell#9402)
* ExternalCommandHelp - Test that `cabal help <cmd>` is interpreted appropiately (haskell#9404)

* Finish off the external commands feature

* Remove 'CommandDelegate' in favour of abstracting the fallback in
  'commandsRun', there is a new variant 'commdandRunWithFallback' which
  takes a continuation
  - This restores the modularity between the `Cabal` library and
    `cabal-install` as now `Cabal` doesn't need to know anything about
    the external command interface.
  - Fixes haskell#9403
* Set the $CABAL environment variable to the current executable path
  - This allows external commands to be implemented by calling $CABAL,
    which is strongly preferred to linking against the Cabal library as
    there is no easy way to guantee your tool and `cabal-install` link
    against the same `Cabal` library.
  - Fixes haskell#9402
* Pass the name of the argument
  - This allows external commands to be implemented as symlinks to an
    executable, and multiple commands can be interpreted by the same
    executable.
  - Fixes haskell#9405
* `cabal help <cmd>` is interpreted as `cabal-<cmd> --help` for external
  commands.
  - This allows the `help` command to also work for external
  commands and hence they are better integrated into cabal-install.
  - Fixes haskell#9404

The tests are updated to test all these additions.

These features bring the external command interface up to par with the
cargo external command interface.

* Use Base16 hash for script path.

Issue haskell#9334 shows that `%` characters on Windows result in invalid
paths, also `/` characters on Linux create invalid paths.

This changes from using base64 to using base16 with the same length
we use for unit-ids.

* Migrate to haskell-actions/setup

As of 2023-09-09, haskell/action/setup is no longer maintained.

The comment

  # latest is mandatory for cabal-testsuite, see haskell#8133

is removed; as the validate job was already fixing a version of cabal-install.

* testsuite: Introduce Cabal-tests library for common testsuite functions

I noticed that Distribution.Utils.TempTestDir was only used in the
testsuite but defined in the Cabal library. Rather than expose this in
the public interface of the `Cabal` library, it is cleaner to refactor
it into a separate library (`Cabal-tests`) which can be used by any
testsuite component.

Also, in future it gives a clearer place to put utility functions which
need to be shared across the testsuite but not exposed in Cabal.
Cabal-tests can also freely add dependencies (such as exceptions) which
we might want to avoid adding to the Cabal library.

Fixes haskell#9453

* GHC 9.8 compat: pacify -Wx-partial

* GHC 9.8 compat: update hashes of data structures as computed by Structured

It seems, GHC 9.8 changed something in the code generation for data types.
Structured class is supposed to catch such cases.

* GHC 9.8 compat: bump base, update Unknown GHC

And bump Cabal's "supported version" of GHC

* CI: GHC 9.8

* merge master

* Hackage should be capitalized

---------

Co-authored-by: Artem Pelenitsyn <[email protected]>
Co-authored-by: Bryan Richter <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Phil de Joux <[email protected]>
Co-authored-by: Yvan Sraka <[email protected]>
Co-authored-by: Andreas Abel <[email protected]>
Co-authored-by: Jens Petersen <[email protected]>
Co-authored-by: Andrea Bedini <[email protected]>
Co-authored-by: Hamish Mackenzie <[email protected]>
Co-authored-by: Malte Neuss <[email protected]>
Co-authored-by: Bodigrim <[email protected]>
Co-authored-by: David Binder <[email protected]>
Co-authored-by: David Binder <[email protected]>
Co-authored-by: Javier Sagredo <[email protected]>
Co-authored-by: Andrea Bedini <[email protected]>
Co-authored-by: Matthew Pickering <[email protected]>
Co-authored-by: Fendor <[email protected]>
Co-authored-by: Hécate Moonlight <[email protected]>
Co-authored-by: Andreas Klebinger <[email protected]>
Co-authored-by: Francesco Ariis <[email protected]>
Co-authored-by: Troels Henriksen <[email protected]>
Co-authored-by: Samuel Thibault <[email protected]>
Co-authored-by: Erik de Castro Lopo <[email protected]>
Co-authored-by: brandon s allbery kf8nh <[email protected]>
Co-authored-by: John Paul Adrian Glaubitz <[email protected]>
Co-authored-by: Kristen Kozak <[email protected]>
Co-authored-by: Javier Sagredo <[email protected]>
erikd added a commit to erikd/cabal that referenced this pull request Apr 22, 2024
…ange only. (haskell#9386)

* add more breadcrumbs for how to use remote packages.

* Hackage should be capitalized

* Update doc/cabal-project.rst

reword.

* Update doc/cabal-project.rst

reword.

* Update doc/cabal-project.rst

clearer.

* Update doc/cabal-project.rst

clearer!

* Update doc/cabal-project.rst

good mechanical description.

* wrap.

* Update doc/cabal-project.rst

add missing double ticks.

* clarify english, and follow a linguistic pattern better.

* Update doc/cabal-project.rst

* doc: render math with HTML to make it selectable (fix haskell#8453) (haskell#9361)

* doc: render math with HTML to make it selectable (fix haskell#8453)

* Update doc/conf.py

Co-authored-by: Bryan Richter <[email protected]>

---------

Co-authored-by: Bryan Richter <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Avoid double space in "Executing install plan ..."

* Add a change log entry for double space avoidance

* Ignore CmmSourcesExe Demo

Ignore because it warns about missing MachDeps.h

* [cabal-7825] Implement external command system

Fix haskell#2349 and haskell#7825

* Bump to latest dependencies for GHC 9.8.1

* cabal.project: clean out obsolete `allow-newer`s

* update GH validate workflow to ghc 9.2.8, 9.4.7, 9.6.3

* Revert haskell#3639 (Don't pass -package-db and -package flags to --abi-hash) (haskell#9384)

* Revert haskell#3639 (Don't pass -package-db and -package flags to --abi-hash)

With ghc>=9.6 `ghc --abi-hash` initialises the plugins so it will fail
if a cabal file specifies `ghc-options: -fplugin=Foo`.

Closes: haskell#9375

* Also revert in GHC.hs

---------

Co-authored-by: Hamish Mackenzie <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Use the newer haskell-actions organisation

* Restructure Cabal documentation top-level parts

The goal is for users to easier find pages for typical problems through search engines and page navigation.
- The top-level layout is based on the popular documentation structure by https://documentation.divio.com/ to give a
   clear structure to users and future documentation contributors:
  * Guides: Present a solution to a single, atomic, typical user problem.
  * Reference: Describe user API (CLI fields, syntax etc) with technical rigour and completeness.
  * Explanation: Discuss background information, scope, design decisions etc.
- Move existing documentation roughly into these categories with minimal editing as the basis for further editing.
- Rename guide titles to mention how-to for improving SEO.
- Rename some files to improve SEO since that name becomes part of the URL (often called slug).
  Important page keywords should appear in the slug as well to make pages rank higher in search engines.

* Rename master_doc to root_doc (changed in version 4.0 of Sphynx)

* Add instance Ord for Field, FieldLine, SectionArg and Name

* Do not run CI for documentation changes

The github workflows are not run if the
changes are completely contained within
the doc/ subdirectory. The only exception
is the users-guide.yml github action.

* Move Backpack section to user guides

* Remove TBW virtual modules section

* Add reinstall test to LinkerOptions/NonignoredConfigs

* Record install options

* Reject index-states after last known index-state (haskell#8944)

Co-authored-by: Javier Sagredo <[email protected]>
Co-authored-by: Andrea Bedini <[email protected]>
Co-authored-by: Andrea Bedini <[email protected]>

* Note how to do "not equal" with constraints

* Use comma with then

Co-authored-by: Artem Pelenitsyn <[email protected]>

* Use narrow rather than upset

* Say something about hackage deprecations

* Fix AutogenModulesToggling test

By converting this to a setupTest we use the in-tree Cabal library
rather than relying on a proxy of the GHC version to provide the right
Cabal library version.

Supersedes haskell#9398

* Require version 3,11 of Cabal to support --semaphore flag

Fixes haskell#9197

* Add dependencies used by `PackageTests` to exe:cabal-tests

The runner allows the tests to use extra dependencies and the custom Prelude
from 'cabal-testsuite'.
However, if the tests use a dependency, say 'directory', and there are two
packages with the same unit id available in the store, the test fails since
it doesn't know which one to pick.
By including an extra dependency to directory, we force the test runner to
use a specific version directory, fixing the test failure.

* Use Paths_cabal_install for cabal-install version number (haskell#9421)

* Use PackageInfo for cabal-install version number

* Use Paths_cabal_install instead

* Adjust documentation

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Document --profiling-detail in setup-commands.

Fixes haskell#9182

* Add test requirement to PR template

Adding test becomes a checkmark instead of “bonus points”.

* A 'cabal path' command. (haskell#8879)

* Add a 'cabal path' command.

* Formatting fix.

* Another formatting fix.

* Categorise "cabal path" as global command.

* Allow individual paths to be printed.

* Less duplication.

* Add config-file to "cabal path".

* Use sum type instead of strings.

* cabal path: support --installdir.

* Add documentation.

* Better text.

* Formatting.

* Add some tests.

* Improve tests.

* Add changelog entry.

* Mention "cabal path" in directory documentation.

---------

Co-authored-by: Artem Pelenitsyn <[email protected]>

* Reimplement `cabal check` (haskell#8427)

* Fix Semigroup target instance

When two target names are the same, `mappend`ing them should not
error but just pick the first name.

* Add `desugarBuildToolSimple`

* Reimplement cabal check

* Reorder test output

* Fix autogen modules tests .cabal files

* Add a number of tests

* Add test for haskell#7423

i.e. Do not warn on -O2 if under off-by-default package configuration
flag conditional.

* Add a regression for:

    * Add another -WErrr test
        This is to make sure we do *not* report it if it is under
        a user, off-by-default flag.
    * Add test for non manual user flags.
    * Add “absolute path in extra-lib-dirs” test
    * Add if/else test
    * Add “dircheck on abspath” check
    * Add Package version internal test
    * Add PackageVersionsStraddle test

* Add changelog for haskell#8427

* Integrate various reviews

* Integrate Artem’s review

(review) Clarify `combineNames` documentation

By explaining the way it operates (working if the two names are equal
or one is empty) and renaming the function from `combineName` to
`combineNames`.

(review) Use guards instead of if/then/else

(review) Match inside argument list

(review) Replace “white” with “allow”

(review) Fix typo in comment

(review) Fix typo in Check module documentation

(review) Harmonise indentation for `data` decls

First field goes in a new line than the data constructor, so we
have more space.

(review) Rename `Prim` module to `Types`

(review) Add checkPackageFilesGPD

`checkPackageFiles` — which works on PD — was used to perform IO. We
introduce a function that does the same thing but works on GPD (which
is more principled).

`checkPackageFiles` cannot just be removed, since it is part of the
interface of Distribution.PackageDescription.Check. Deprecation can
be planned once “new check” is up and running.

* Integrate Andreas’ review

(review) Add named section to missing upper bound check

“miss upper bound” checks will now list target type and name (“On
executable 'myexe', these packages miss upper bounds”) for easier
fixing by the user.

(review) remove `cabal gen-bounds` suggestion

Reasonable as `cabal gen-bounds` is stricter than `cabal check`, see
haskell#8427 (comment)
Once `gen-bounds` behaves in line with `check` we can readd the
suggestion.

(review) Do not warn on shared bounds

When a target which depends on an internal library shares some
dependencies with the latter, do not warn on upper bounds.

An example is clearer

    library
     build-depends: text < 5
    ⁝
     build-depends: myPackage,        ← no warning, internal
                    text,             ← no warning, shared bound
                    monadacme         ← warning!

* Integrate Artem’s review /II

(review) Split Check.hs

Check.hs has been split in multiple file, each une sub 1000 lines:

Check              857 lines
Check.Common       147 lines
Check.Conditional  204 lines
Check.Monad        352 lines
Check.Paths        387 lines
Check.Target       765 lines
Check.Warning      865 lines

Migration guide:
- Check              GPD/PD checks plus work-tree checks.
- Check.Common       common types and functions that are
                     *not* part of monadic checking setup.
- Check.Conditional  checks on CondTree and related matter
                     (variables, duplicate modules).
- Check.Monad        Backbone of the checks, monadic inter-
                     face and related functions.
- Check.Paths        Checks on files, directories, globs.
- Check.Target       Checks on realised targets (libraries,
                     executables, benchmarks, testsuites).
- Check.Warning      Datatypes and strings for warnings
                     and severities.

(review) remove useless section header

(review) Fix typo

(review) Add warnings documentation (list)

For each warning, we document constructor/brief description
in the manual.  This might not be much useful as not but it
will come handy when introducing `--ignore=WARN` and similar
flags.

* (review Andreas) Clarify CheckExplanation comment

Whoever modifies `CheckExplanation` data constructors needs to be
aware that the documentation in  doc/cabal-commands.rst  has to be
updated too.

* Move internal Check modules to `other-modules`

No need to expose Distribution.PackageDescription.Check.*
to the world. API for checking, for cabal-install and other
tools, should be in Distribution.PackageDescription.Check.

* Make fourmolu happy

Cabal codebase has now a formatter/style standard (see haskell#8950).

“Ravioli ravioli, give me the formuoli”

* Do not check for OptO in scripts

See haskell#8963 for reason and clarification requests.

* Remove useless PackageId parameter

It is now in the Reader part of CheckM monad.

* Do not check PVP on internal targets

Internal: testsuite, benchmark.
See haskell#8361.

* Make hlint happy

* Fix haskell#9122

When checking internal version ranges, we need to make sure we
are not mistaking a libraries with the same name but from different
packages. See haskell#9132.

* Fix grammar

neither…nor, completing what done in haskell#9162

* Integrate Brandon’s review: grammar

* Remove unnecessary `-fvia-C` check

Brandon’s review/II.

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* ci: Enable windows tests for 9.6.3

There were two failing tests:

1. CCompilerOverride, was attempting to use gcc.exe rather than
   clang.exe without also overriding the C options which led to
   incorrect options being passed to gcc.exe. The fix is to override to
   clang.exe on ghc-9.4 or newer.
2. ForeignLibs exposes a bug in GHC
   (https://gitlab.haskell.org/ghc/ghc/-/issues/24185) and hence is
   skipped for GHCs newer than 9.4 where it was first introduced.

Towards fixing haskell#8451, we just need to fix the shared library issue now.

* testsuite: Be explicit about runtime test dependencies

Issue haskell#8356 reports occasional errors from running the testsuite about
multiple package versions available. This stems from the invokation of
`runghc` not being explicit about all dependencies of the testsuite.

The solution is provide a component in the cabal file which is explicit
about which packages the tests can depend on. This component has a
build-depends section which lists all the dependencies that the tests
require.

It would be better if this component was a library component but we
can't do this with a Custom setup because of limitations to do with
per-component builds.

Then we also enable `-hide-all-packages`, so the dependency will not be
available if it is not explicitly listed as a dependency.

You could also imagine a future where the Setup.hs script found the test
files and compiled a single executable which would run all the tests,
rather than invoking runghc on each one individually.

Fixes haskell#8356

* hurd: Enable using $ORIGIN in RPATH

GNU/Hurd fully supports RPATH and the $ORIGIN development, and we indeed
want to use it for relocatable installations shipped in Debian GNU/Hurd.

* Fix the platform string for GNU/Hurd

Since version 9.4.7-1, ghc fails to build on the GNU/Hurd port of Debian,
see

https://buildd.debian.org/status/fetch.php?pkg=ghc&arch=hurd-i386&ver=9.4.7-1&stamp=1697717885&raw=0

Error, rule finished running but did not produce file:
  _build/stage0/lib/i386-gnu-ghc-9.4.6/ghc-boot-th-9.4.7/libHSghc-boot-th-9.4.7.a

and indeed, what did get produce was rather
_build/stage0/lib/i386-hurd-ghc-9.4.6/ghc-boot-th-9.4.7/libHSghc-boot-th-9.4.7.a
(i386-hurd instead of i386-gnu).

This is due to confusion between hurd and gnu in various places.  Apparently
previous versions of ghc were using gnu for the GNU/Hurd port, and thus
putting libraries etc. in i386-gnu. So we have to follow the existing
practice.

* Fix configuation of ldProgram

Standard GNU `ld` ues `--relocatable` while `ld.gold` uses a `-relocatable`
flag (with a single `-`). Code will now detect both versions.

* Chain configuration of ldProgram

`ldProgram` gets configured in two places, a seemingly default and
a GHC specific version. The later needs to be updated so that it
first calls the default configuration and then the new GHC version.

* Use linker capability detection to improve linker use

The function `comperSupportsGhciLibs` has been renamed to
`linkerSupportsGhciLibs` because its about the linker not the compiler.

The function `comperSupportsGhciLibs` was using the compiler version
as a proxy for whether the linker supports relocatable objects. Now
support for relocatable objects is detected by running the linker.

* add `merge+no rebase`

and a few typos while reviewing

* Add support for 64-bit SPARC as a separate architecture

Previously, sparc64 was defined as an alias for the 32-bit SPARC
architecture which was true while SPARC mainland was mostly 32
bits. More recently, 64-bit SPARC has become a port of its own,
so it needs to be treated as a separate architecture.

* Remove debug-conflict-sets flag from solver package

Fixes haskell#8937.

The debug-conflict-sets build flag probably hasn't been used for a long time,
and it isn't currently tested. This commit removes the flag, converts the
ConflictSet type back to a newtype, and removes an unnecessary instance.

* Finish improvements to the CI configuration for documentation changes (haskell#9460)

* Add bootstrap postjob to CI config

Add a new job to the bootstrap.yml GitHub action config.
This job succeeds if, and only if, all the other bootstrap
jobs succeed.

* Do not run bootstrap CI jobs for documentation changes

The approach was already introduced in haskell#9355 for the validate jobs.
This commit introduces the same change also for the bootstrap jobs.

* Also ignore CONTRIBUTING.md and README.md in CI

We do not run the entire CI suite for documentation changes.
Previously, only changes which were restricted to the 'docs/'
subdirectory were considered to be documentation changes.
With this commit we also recognize changes to README.md and
CONTRIBUTING.md as documentation changes.

* Document improved CI for documentation in CONTRIBUTING.md

The CONTRIBUTING.md file now mentions that documentation changes
do not waste expensive CI resources.

* Recognize all README.md in subdirs as documentation

Expensive CI jobs should not run on changes which affect only
README.md files.

* formatting: Add style-commit makefile target

This target allows you to format a range of commits, for example:

```
make style-commit COMMIT=HEAD~1
> Last commit is formatted
make style-commit COMMIT=abcde
> Commits between HEAD and abcde are formatted
```

* Fix assertion failure when combining build-tool-depends and --enable-documentation

The `setDocumentation` function was modifying the elaborated package
after the hash was computed. This led to the assertion failing as the
computed hash was different to what was computed in the initial install
plan.

Therefore in order to fix this we either needed to:

1. Set elabBuildHaddocks = False at the point where the hash is
   initially computed.
2. Verify that elabBuildHaddocks = True will not lead to unexpected
   results.

The latter has been implemented.

The elabBuildHaddocks option is only consulted in
`hasValidHaddockTargets`, at which point documentation building the
executable component is disabled because elabHaddockExecutables is
False.

In the added test we ensure this by checking that we didn't build
documentation for the executable which is built because of
build-tool-depends.

Fixes haskell#6006 haskell#8313

* testsuite: Improve error message in findDependencyInStore

* Only move code to Simple/GHC/Build*

* CPP mingw32_HOST_OS for more imports

* cabal-install-solver: fix pkgconf 1.9 --modversion regression

Check that the numbers of *versions* output is equal to the number of pkgconf's

fixes haskell#8923

The pkgconf behavior was reverted upstream in 2.0

(this should cover the case too of checking that equal pkgList lines are output also)

* External commands: Add tests for haskell#9402 haskell#9403 haskell#9404

This adds 4 tests which test the new external commands feature:

* ExternalCommand - Tests the expected usage of external command invoked
  via cabal-install
* ExternalCommandSetup - Tests that the ./Setup interface does not
  support external commands (haskell#9403)
* ExternalCommandEnv - Tests that environment variables are set and
  preserved appropiately  (haskell#9402)
* ExternalCommandHelp - Test that `cabal help <cmd>` is interpreted appropiately (haskell#9404)

* Finish off the external commands feature

* Remove 'CommandDelegate' in favour of abstracting the fallback in
  'commandsRun', there is a new variant 'commdandRunWithFallback' which
  takes a continuation
  - This restores the modularity between the `Cabal` library and
    `cabal-install` as now `Cabal` doesn't need to know anything about
    the external command interface.
  - Fixes haskell#9403
* Set the $CABAL environment variable to the current executable path
  - This allows external commands to be implemented by calling $CABAL,
    which is strongly preferred to linking against the Cabal library as
    there is no easy way to guantee your tool and `cabal-install` link
    against the same `Cabal` library.
  - Fixes haskell#9402
* Pass the name of the argument
  - This allows external commands to be implemented as symlinks to an
    executable, and multiple commands can be interpreted by the same
    executable.
  - Fixes haskell#9405
* `cabal help <cmd>` is interpreted as `cabal-<cmd> --help` for external
  commands.
  - This allows the `help` command to also work for external
  commands and hence they are better integrated into cabal-install.
  - Fixes haskell#9404

The tests are updated to test all these additions.

These features bring the external command interface up to par with the
cargo external command interface.

* Use Base16 hash for script path.

Issue haskell#9334 shows that `%` characters on Windows result in invalid
paths, also `/` characters on Linux create invalid paths.

This changes from using base64 to using base16 with the same length
we use for unit-ids.

* Migrate to haskell-actions/setup

As of 2023-09-09, haskell/action/setup is no longer maintained.

The comment

  # latest is mandatory for cabal-testsuite, see haskell#8133

is removed; as the validate job was already fixing a version of cabal-install.

* testsuite: Introduce Cabal-tests library for common testsuite functions

I noticed that Distribution.Utils.TempTestDir was only used in the
testsuite but defined in the Cabal library. Rather than expose this in
the public interface of the `Cabal` library, it is cleaner to refactor
it into a separate library (`Cabal-tests`) which can be used by any
testsuite component.

Also, in future it gives a clearer place to put utility functions which
need to be shared across the testsuite but not exposed in Cabal.
Cabal-tests can also freely add dependencies (such as exceptions) which
we might want to avoid adding to the Cabal library.

Fixes haskell#9453

* GHC 9.8 compat: pacify -Wx-partial

* GHC 9.8 compat: update hashes of data structures as computed by Structured

It seems, GHC 9.8 changed something in the code generation for data types.
Structured class is supposed to catch such cases.

* GHC 9.8 compat: bump base, update Unknown GHC

And bump Cabal's "supported version" of GHC

* CI: GHC 9.8

* merge master

* Hackage should be capitalized

---------

Co-authored-by: Artem Pelenitsyn <[email protected]>
Co-authored-by: Bryan Richter <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Phil de Joux <[email protected]>
Co-authored-by: Yvan Sraka <[email protected]>
Co-authored-by: Andreas Abel <[email protected]>
Co-authored-by: Jens Petersen <[email protected]>
Co-authored-by: Andrea Bedini <[email protected]>
Co-authored-by: Hamish Mackenzie <[email protected]>
Co-authored-by: Malte Neuss <[email protected]>
Co-authored-by: Bodigrim <[email protected]>
Co-authored-by: David Binder <[email protected]>
Co-authored-by: David Binder <[email protected]>
Co-authored-by: Javier Sagredo <[email protected]>
Co-authored-by: Andrea Bedini <[email protected]>
Co-authored-by: Matthew Pickering <[email protected]>
Co-authored-by: Fendor <[email protected]>
Co-authored-by: Hécate Moonlight <[email protected]>
Co-authored-by: Andreas Klebinger <[email protected]>
Co-authored-by: Francesco Ariis <[email protected]>
Co-authored-by: Troels Henriksen <[email protected]>
Co-authored-by: Samuel Thibault <[email protected]>
Co-authored-by: Erik de Castro Lopo <[email protected]>
Co-authored-by: brandon s allbery kf8nh <[email protected]>
Co-authored-by: John Paul Adrian Glaubitz <[email protected]>
Co-authored-by: Kristen Kozak <[email protected]>
Co-authored-by: Javier Sagredo <[email protected]>
@michaelpj
Copy link
Collaborator

I forgot that this patch landed and tried to use a future index-state to get something that just got released :( I note that my previous comment was wrong - using HEAD is bad because it's much more permissive. I can pretty safely commit an index-state of "start of tomorrow" and open myself to at worst a little bit of non-determinism between now and then, whereas HEAD is a lot more non-determinism until I fix it

I guess I'll just have to be less lazy and write down the actual current time...

@jasagredo
Copy link
Collaborator Author

Which is actually good for determinism. I was just exactly setting up a PR to ensure this in our repo as we had this exact discussion yesterday IntersectMBO/ouroboros-consensus#1158

@9999years
Copy link
Collaborator

Hello, this patch is making a bunch of cabal-install commands fail which previously did not fail, e.g.:

$ cabal exec -v -- ghc --print-libdir
this build was affected by the following (project) config files:
- /nix/store/vsjh8lkhafvp90qhvari5k0qd4bcrv1l-cabal.project
Running: /nix/store/gmgzg1l65xy7v8liqrvac96n1hzlb8ab-ghc-9.6.3-with-packages/bin/ghc --print-global-package-db
Reading available packages of hackage.haskell.org...
Using most recent state (could not read timestamp file)
Error: [Cabal-7160]
The package list for 'hackage.haskell.org' does not exist. Run 'cabal update' to download it.
CallStack (from HasCallStack):
  dieWithException, called at src/Distribution/Client/IndexUtils.hs:464:31 in cabal-install-3.12.1.0-1HzjdJjLHzi5FPN9QfJl2w:Distribution.Client.IndexUtils

This is frustrating, because I'm using Nix and already have a package database with all the packages I'll need to install available and ready. Cabal shouldn't need to consult the index at all. Passing --offline or --index-state="@$(date +%s)" doesn't seem to help either.

9999years added a commit to 9999years/cabal that referenced this pull request Aug 27, 2024
All of the packages Cabal needs may already be present in the local
package database; we don't know yet!

This is a partial revert of haskell#8944.

See: haskell#8944 (comment)
@jasagredo
Copy link
Collaborator Author

@9999years I think the usage of cabal exec is doing exactly what it is supposed to do:

, commandDescription = Just $ \pname ->
wrapText $
"During development it is often useful to run build tasks and perform"
++ " one-off program executions to experiment with the behavior of build"
++ " tools. It is convenient to run these tools in the same way "
++ pname
++ " itself would. The `"
++ pname
++ " v2-exec` command provides a way to"
++ " do so.\n"
++ "\n"
++ "Compiler tools will be configured to see the same subset of the store"
++ " that builds would see. The PATH is modified to make all executables in"
++ " the dependency tree available (provided they have been built already)."
++ " Commands are also rewritten in the way cabal itself would. For"
++ " example, `"
++ pname
++ " v2-exec ghc` will consult the configuration"
++ " to choose an appropriate version of ghc and to include any"
++ " ghc-specific flags requested."

Namely it has to consult the index-state to resolve any executables, or to resolve the with-compiler option:

➜ cabal exec -- ghc --version
Resolving dependencies...
The Glorious Glasgow Haskell Compilation System, version 9.8.2

~\aa via λ 9.6.6 took 2s
➜ ghc --version
The Glorious Glasgow Haskell Compilation System, version 9.6.6

So I think the issue is that you are calling cabal exec with a wrong environment, not that cabal errors if the package list does not exist.

@9999years
Copy link
Collaborator

@jasagredo Is that true? I'm not sure why cabal exec would need to consult the index, as long as all the packages in correct versions are installed and active in the ghc-pkg database. Am I missing something here?

@jasagredo
Copy link
Collaborator Author

I am unsure but I think loading the configuration of a project is an all-or-nothing action. So there is no such thing as "check my wanted ghc, and also these other options, also check the paths for build-tool-depends that I declare, but do not check the index".

My understanding is that in order to for example provide the build-tool-depends, cabal needs to resolve a build plan, which requires going through the available packages, which requires reading the index.

@ulysses4ever
Copy link
Collaborator

May be much better for visibility to open a new ticket for discussion.

It feels to me like what the Nix-based workflow needs is the low-level Setup.hs interface rather than a holistic tool for dependency resolution and building like cabal-install. Case in point, the Haskell infrastructure in nixpkgs is built around the Setup.hs interface. Trying to prevent cabal-install from executing random parts of its operations sounds like a wrong approach. Maybe there should be another tool that operates on a level somewhere between the low and the high?..

I agree that It's annoying when something that used to work stops working. But changing anything at all breaks someone's workflow (cf. the corresponding xkcd) and we cannot not change a living project. When doing any changes, what should be prioritized: to support the rather exotic case of Nix or general improvements in cabal operation (like the one this patch implements). I'd argue the latter.

9999years added a commit to MercuryTechnologies/cabal that referenced this pull request Sep 9, 2024
All of the packages Cabal needs may already be present in the local
package database; we don't know yet!

This is a partial revert of haskell#8944.

See: haskell#8944 (comment)
@pwm
Copy link

pwm commented Oct 28, 2024

6142386198 also breaks active-repositories: none, which for years was a simple way to tell cabal @9999years 's use-case. I'm now not sure how to tell cabal that I don't need it to manage the package list.

@gbaz
Copy link
Collaborator

gbaz commented Nov 2, 2024

Breaking active-repositories: none is very bad and we should fix this asap!

ulysses4ever pushed a commit that referenced this pull request Nov 3, 2024
All of the packages Cabal needs may already be present in the local
package database; we don't know yet!

This is a partial revert of #8944.

See: #8944 (comment)
@gbaz
Copy link
Collaborator

gbaz commented Nov 3, 2024

@pwm how are you passing the active-repositories flag? not sure if i can reproduce on HEAD.

cabal --active-repositories :none build exe:cabal
Warning: this is a debug build of cabal-install with assertions enabled.
Resolving dependencies...

@ulysses4ever
Copy link
Collaborator

ulysses4ever commented Nov 3, 2024

@gbaz in order to reproduce, you have to remove ~/.cache/cabal/packages/hackage.haskell.org (or it's non-XDG equivalent).

@pwm
Copy link

pwm commented Nov 3, 2024

@gbaz I'm setting active-repositories: none in cabal.project, which I've been doing for years + what @ulysses4ever says re you need to not have the package repo present.

My use case, which I think many others share, is that I use Cabal for building (because it's incremental), but I don't want Cabal to manage my packages (because I use Nix). For now I'm using @9999years 's patch (linked above) to revert this breaking change.

@gbaz
Copy link
Collaborator

gbaz commented Nov 3, 2024

got it -- easy to reproduce now by setting an empty directory as CABAL_DIR in the environment.

That said, I think this issue is somewhat less severe than I feared, because it doesn't affect most builds with active-repositories: none -- rather, they can still be conducted, just following a cabal update.

ulysses4ever pushed a commit that referenced this pull request Nov 4, 2024
All of the packages Cabal needs may already be present in the local
package database; we don't know yet!

This is a partial revert of #8944.

See: #8944 (comment)
ulysses4ever pushed a commit that referenced this pull request Nov 7, 2024
All of the packages Cabal needs may already be present in the local
package database; we don't know yet!

This is a partial revert of #8944.

See: #8944 (comment)
Mikolaj pushed a commit that referenced this pull request Nov 9, 2024
All of the packages Cabal needs may already be present in the local
package database; we don't know yet!

This is a partial revert of #8944.

See: #8944 (comment)
mergify bot pushed a commit that referenced this pull request Nov 10, 2024
All of the packages Cabal needs may already be present in the local
package database; we don't know yet!

This is a partial revert of #8944.

See: #8944 (comment)
(cherry picked from commit d58a75e)
vst added a commit to vst/haskell-template-hebele that referenced this pull request Dec 19, 2024
This is due to haskell/cabal#8944

Previously, we were getting only a warning. It was fine as we are using
Nix to provision the packages.

Now (as of cabal-install v3.12.1.0), we are getting an error. To avoid
this, we need to run `cabal update` prior to running the tests.

Note that there are ongoing discussions on how to handle this going
forward.
vst added a commit to vst/haskell-template-hebele that referenced this pull request Dec 19, 2024
This is due to haskell/cabal#8944

Previously, we were getting only a warning. It was fine as we are using
Nix to provision the packages.

Now (as of cabal-install v3.12.1.0), we are getting an error. To avoid
this, we need to run `cabal update` prior to running the tests.

Note that there are ongoing discussions on how to handle this going
forward.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attention: needs-manual-qa PR is destined for manual QA merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days squash+merge me Tell Mergify Bot to squash-merge tested-on: windows QA has been successful on Windows
Projects
Status: To Test
Development

Successfully merging this pull request may close these issues.