[commit: ghc] master: ghc-pkg: Try opening lockfiles in read-write mode first (f86de44)

[git at git.haskell.org]

Repository : ssh://git@git.haskell.org/ghc

On branch  : master
Link       : http://ghc.haskell.org/trac/ghc/changeset/f86de44dac0a6ca40c5fcd65f3a1944c45fa6011/ghc

>---------------------------------------------------------------

commit f86de44dac0a6ca40c5fcd65f3a1944c45fa6011
Author: Ben Gamari <bgamari.foss at gmail.com>
Date:   Tue Aug 29 14:26:55 2017 -0400

    ghc-pkg: Try opening lockfiles in read-write mode first
    
    As pointed out in #13945, some filesystems only allow allow exclusive
    locks if the fd being locked was opened for write access. This causes
    ghc-pkg to fail as it first attempts to open and exclusively lock its
    lockfile in read-only mode to accomodate package databases for which we
    lack write permissions (e.g.  global package databases).
    
    Instead, we now try read-write mode first, falling back to read-only
    mode if this fails.
    
    Reviewers: austin
    
    Subscribers: rwbarton, thomie
    
    GHC Trac Issues: #13945
    
    Differential Revision: https://phabricator.haskell.org/D3897


>---------------------------------------------------------------

f86de44dac0a6ca40c5fcd65f3a1944c45fa6011
 libraries/ghc-boot/GHC/PackageDb.hs | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/libraries/ghc-boot/GHC/PackageDb.hs b/libraries/ghc-boot/GHC/PackageDb.hs
index bf83d25..9ce07e7 100644
--- a/libraries/ghc-boot/GHC/PackageDb.hs
+++ b/libraries/ghc-boot/GHC/PackageDb.hs
@@ -239,15 +239,21 @@ lockPackageDbWith mode file = do
   -- DB for reading then we will require that the installer/packaging has
   -- included the lock file.
   --
-  -- Thus the logic here is to first try opening in read-only mode (to handle
-  -- global read-only DBs) and if the file does not exist then try opening in
-  -- read/write mode to create the lock file. If either succeed then lock the
-  -- file. IO exceptions (other than the first open attempt failing due to the
-  -- file not existing) simply propagate.
+  -- Thus the logic here is to first try opening in read-write mode
+  -- and if that fails we try read-only (to handle global read-only DBs).
+  -- If either succeed then lock the file. IO exceptions (other than the first
+  -- open attempt failing due to the file not existing) simply propagate.
+  --
+  -- Note that there is a complexity here which was discovered in #13945: some
+  -- filesystems (e.g. NFS) will only allow exclusive locking if the fd was
+  -- opened for write access. We would previously try opening the lockfile for
+  -- read-only access first, however this failed when run on such filesystems.
+  -- Consequently, we now try read-write access first, falling back to read-only
+  -- if are denied permission (e.g. in the case of a global database).
   catchJust
-    (\e -> if isDoesNotExistError e then Just () else Nothing)
-    (lockFileOpenIn ReadMode)
-    (const $ lockFileOpenIn ReadWriteMode)
+    (\e -> if isPermissionError e then Just () else Nothing)
+    (lockFileOpenIn ReadWriteMode)
+    (const $ lockFileOpenIn ReadMode)
   where
     lock = file <.> "lock"