Regression: local repository file+noindex ignores noindex part

[alt-romes]

Describe the bug
Having a cabal project with a local repository using url: file+noindex://... works with cabal 3.10.2.1 but fails with HEAD.

To Reproduce

cabal init -m -n --simple --lib
mkdir repo
echo "packages: ." > cabal.project
echo "repository local" >> cabal.project
echo "  url: file+noindex://$(pwd)/repo" >> cabal.project

Then, building with cabal from HEAD will produce an error message like the following:

Error during construction of local+noindex local repository index:
/var/folders/tv/35hlch6s3y15hfvndc71l6d40000gn/T/tmp.umaedU6Xpe/repo/noindex.cache:
openBinaryFile: does not exist (No such file or directory)

If your package depends on a package from the local repository the build will simply fail.

Then, building with cabal-3.10.2.1 will succeed. In fact, it looks like cabal-3.10.2.1 will create a noindex.cache file in the repository.

[Mikolaj]

It'd be great to see where the bug has been introduced and whether the fix would be contained to cabal-install (together with cabal-install-solver) or whether it might involve any fixes to the Cabal API or file format (the latter probably unlikely). For now, I'm going to assume this does not block the release of Cabal 3.12.0.0, but is a likely blocker for cabal-install 3.12.1.0.

[cloudyluna]

Hi! I believe this bug location lives within this readRepoIndex function:

cabal/cabal-install/src/Distribution/Client/IndexUtils.hs

Line 424 in 94277d1

readRepoIndex

Note that I tested this using git tag of cabal-install-v3.12.0.0-prerelease (I tested for this bug in cabal-3.10.3.0 beforehand and found it works without this bug, the same as in cabal-3.10.2.1)

This was introduced in this commit to be specific: d608d02.

For relevant strace info it it helps (left picture is the compiled, unchanged version from mentioned git tag above. The right one is the same but patched with previously committed code. Please scroll down below for the patch.)

relevant ad-hoc patch to restore this file+noindex behavior for testing, though I'm pretty sure it breaks the recently introduced behavior.

diff --git a/cabal-install/src/Distribution/Client/IndexUtils.hs b/cabal-install/src/Distribution/Client/IndexUtils.hs
index 2dc7d37e2..945afcc0a 100644
--- a/cabal-install/src/Distribution/Client/IndexUtils.hs
+++ b/cabal-install/src/Distribution/Client/IndexUtils.hs
@@ -430,16 +430,15 @@ readRepoIndex
   -> IO (PackageIndex UnresolvedSourcePackage, [Dependency], IndexStateInfo)
 readRepoIndex verbosity repoCtxt repo idxState =
   handleNotFound $ do
-    ret@(_, _, isi) <-
-      readPackageIndexCacheFile
-        verbosity
-        mkAvailablePackage
-        (RepoIndex repoCtxt repo)
-        idxState
-    when (isRepoRemote repo) $ do
-      warnIfIndexIsOld =<< getIndexFileAge repo
-      dieIfRequestedIdxIsNewer isi
-    pure ret
+    when (isRepoRemote repo) $ warnIfIndexIsOld =<< getIndexFileAge repo
+    -- note that if this step fails due to a bad repo cache, the the procedure can still succeed by reading from the existing cache, which is updated regardless.
+    updateRepoIndexCache verbosity (RepoIndex repoCtxt repo)
+      `catchIO` (\e -> warn verbosity $ "unable to update the repo index cache -- " ++ displayException e)
+    readPackageIndexCacheFile
+      verbosity
+      mkAvailablePackage
+      (RepoIndex repoCtxt repo)
+      idxState
   where
     mkAvailablePackage pkgEntry =
       SourcePackage
@@ -460,8 +459,8 @@ readRepoIndex verbosity repoCtxt repo idxState =
       if isDoesNotExistError e
         then do
           case repo of
-            RepoRemote{..} -> dieWithException verbosity $ MissingPackageList repoRemote
-            RepoSecure{..} -> dieWithException verbosity $ MissingPackageList repoRemote
+            RepoRemote{..} -> warn verbosity $ errMissingPackageList repoRemote
+            RepoSecure{..} -> warn verbosity $ errMissingPackageList repoRemote
             RepoLocalNoIndex local _ ->
               warn verbosity $
                 "Error during construction of local+noindex "
@@ -479,15 +478,10 @@ readRepoIndex verbosity repoCtxt repo idxState =
         RepoSecure{..} -> warn verbosity $ warnOutdatedPackageList repoRemote dt
         RepoLocalNoIndex{} -> return ()
 
-    dieIfRequestedIdxIsNewer isi =
-      let latestTime = isiHeadTime isi
-       in case idxState of
-            IndexStateTime t -> when (t > latestTime) $ case repo of
-              RepoSecure{..} ->
-                dieWithException verbosity $ UnusableIndexState repoRemote latestTime t
-              RepoRemote{} -> pure ()
-              RepoLocalNoIndex{} -> return ()
-            IndexStateHead -> pure ()
+    errMissingPackageList repoRemote =
+      "The package list for '"
+        ++ unRepoName (remoteRepoName repoRemote)
+        ++ "' does not exist. Run 'cabal update' to download it."
 
     warnOutdatedPackageList repoRemote dt =
       "The package list for '"

Hope this helps a bit in narrowing down the actual problem.

[alt-romes]

@jasagredo @andreabedini tagging you as the authors of the commit which introduced the regression.

[alt-romes]

Thanks for investigating @cloudyluna !

[andreabedini]

Thank you @alt-romes, I'll have a look.

[andreabedini]

I think I understand where the problem is. I am working on a patch.

[robbert-vdh]

This change also affects active-repositories: :none, as builds now fail if there is no up to date Hackage index even if Hackage isn't being used (e.g. all packages are provided through Nix). Is this the same issue as this, or should I create a separate ticket?