[commit: ghc] master: Filter plugin dylib locations (b324c56)
git at git.haskell.org
git at git.haskell.org
Sat Aug 11 18:18:48 UTC 2018
Repository : ssh://git@git.haskell.org/ghc
On branch : master
Link : http://ghc.haskell.org/trac/ghc/changeset/b324c5624432f2c3d5b0a689fdff75a1ccc563f5/ghc
>---------------------------------------------------------------
commit b324c5624432f2c3d5b0a689fdff75a1ccc563f5
Author: Christiaan Baaij <christiaan.baaij at gmail.com>
Date: Sat Aug 11 18:56:34 2018 +0200
Filter plugin dylib locations
Summary:
Previously we just created a cartesian product of the library
paths of the plugin package and the libraries of the package.
Of course, some of these combinations result in a filepath of
a file doesn't exists, leading to #15475.
Instead of making `haskFile` return Nothing in case a file
doesn't exist (which would hide errors), we look at all the
possible dylib locations and ensure that at least one of those
locations is an existing file. If the list turns out to be
empty however, we panic.
Reviewers: mpickering, bgamari
Reviewed By: mpickering
Subscribers: monoidal, rwbarton, carter
GHC Trac Issues: #15475
Differential Revision: https://phabricator.haskell.org/D5048
>---------------------------------------------------------------
b324c5624432f2c3d5b0a689fdff75a1ccc563f5
compiler/deSugar/DsUsage.hs | 35 +++++++++++++++++-----
testsuite/tests/plugins/Makefile | 9 ++++++
testsuite/tests/plugins/all.T | 7 +++++
...nge.stderr => plugin-recomp-change-prof.stderr} | 6 ++++
testsuite/tests/plugins/plugin-recomp/Makefile | 4 ++-
5 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/compiler/deSugar/DsUsage.hs b/compiler/deSugar/DsUsage.hs
index 45d4dcf..58c31ee 100644
--- a/compiler/deSugar/DsUsage.hs
+++ b/compiler/deSugar/DsUsage.hs
@@ -25,6 +25,7 @@ import Maybes
import Packages
import Finder
+import Control.Monad (filterM)
import Data.List
import Data.IORef
import Data.Map (Map)
@@ -166,14 +167,19 @@ mkPluginUsage hsc_env pluginModule
= case lookupPluginModuleWithSuggestions dflags pNm Nothing of
-- The plug is from an external package, we just look up the dylib that
-- contains the plugin
- LookupFound _ pkg ->
+ LookupFound _ pkg -> do
let searchPaths = collectLibraryPaths dflags [pkg]
libs = packageHsLibs dflags pkg
- dynlibs = [ searchPath </> mkHsSOName platform lib
+ dynlibLocs = [ searchPath </> mkHsSOName platform lib
| searchPath <- searchPaths
, lib <- libs
]
- in mapM hashFile (nub dynlibs)
+ dynlibs <- filterM doesFileExist dynlibLocs
+ case dynlibs of
+ [] -> pprPanic
+ ("mkPluginUsage: no dylibs, tried:\n" ++ unlines dynlibLocs)
+ (ppr pNm)
+ _ -> mapM hashFile (nub dynlibs)
_ -> do
foundM <- findPluginModule hsc_env pNm
case foundM of
@@ -186,10 +192,25 @@ mkPluginUsage hsc_env pluginModule
return (nub (pluginObject : depObjects))
_ -> pprPanic "mkPluginUsage: no object or dylib" (ppr pNm)
where
- -- plugins are shared libraries, so add WayDyn to the dflags in order to get
- -- the correct filenames and library paths; just in case the object that is
- -- currently being build is not going to be linked dynamically
- dflags = addWay' WayDyn (hsc_dflags hsc_env)
+ -- plugins are shared libraries, so WayDyn should be part of the dflags in
+ -- order to get the correct filenames and library paths.
+ --
+ -- We can distinguish two scenarios:
+ --
+ -- 1. The dflags do not contain WayDyn, in this case we need to remove
+ -- all other ways and only add WayDyn. Why? Because other ways change
+ -- the library tags, i.e. WayProf adds `_p`, and we would end up looking
+ -- for a profiled plugin which might not be installed. See #15492
+ --
+ -- 2. The dflags do contain WayDyn, in this case we can leave the ways as
+ -- is, because the plugin must be compiled with the same ways as the
+ -- module that is currently being build, e.g., if the module is
+ -- build with WayDyn and WayProf, then the plugin that was used
+ -- would've also had to been build with WayProf (and WayDyn).
+ dflags1 = hsc_dflags hsc_env
+ dflags = if WayDyn `elem` ways dflags1
+ then dflags1
+ else updateWays (addWay' WayDyn (dflags1 {ways = []}))
platform = targetPlatform dflags
pNm = moduleName (mi_module pluginModule)
pPkg = moduleUnitId (mi_module pluginModule)
diff --git a/testsuite/tests/plugins/Makefile b/testsuite/tests/plugins/Makefile
index 688ac04..8a6af5b 100644
--- a/testsuite/tests/plugins/Makefile
+++ b/testsuite/tests/plugins/Makefile
@@ -112,3 +112,12 @@ plugin-recomp-change:
"$(TEST_HC)" $(TEST_HC_OPTS) $(ghcPluginWayFlags) -v0 plugin-recomp-test.hs -package-db plugin-recomp/pkg.plugins01/local.package.conf -fplugin PurePlugin
"$(MAKE)" -s --no-print-directory -C plugin-recomp package.plugins01 TOP=$(TOP) RUN=-DRUN2
"$(TEST_HC)" $(TEST_HC_OPTS) $(ghcPluginWayFlags) -v0 plugin-recomp-test.hs -package-db plugin-recomp/pkg.plugins01/local.package.conf -fplugin PurePlugin
+
+# Should recompile the module because the plugin changed, test for 15492
+.PHONY: plugin-recomp-change-prof
+plugin-recomp-change-prof:
+ "$(TEST_HC)" $(TEST_HC_OPTS) $(ghcPluginWayFlags) -v0 plugin-recomp-test.hs -package-db plugin-recomp/pkg.plugins01/local.package.conf -fplugin PurePlugin
+ "$(TEST_HC)" $(TEST_HC_OPTS) -prof -osuf p_o -hisuf p_hi -v0 plugin-recomp-test.hs -package-db plugin-recomp/pkg.plugins01/local.package.conf -fplugin PurePlugin
+ "$(MAKE)" -s --no-print-directory -C plugin-recomp package.plugins01 TOP=$(TOP) RUN=-DRUN2
+ "$(TEST_HC)" $(TEST_HC_OPTS) $(ghcPluginWayFlags) -v0 plugin-recomp-test.hs -package-db plugin-recomp/pkg.plugins01/local.package.conf -fplugin PurePlugin
+ "$(TEST_HC)" $(TEST_HC_OPTS) -prof -osuf p_o -hisuf p_hi -v0 plugin-recomp-test.hs -package-db plugin-recomp/pkg.plugins01/local.package.conf -fplugin PurePlugin
diff --git a/testsuite/tests/plugins/all.T b/testsuite/tests/plugins/all.T
index 48efb05..339b9ba 100644
--- a/testsuite/tests/plugins/all.T
+++ b/testsuite/tests/plugins/all.T
@@ -161,3 +161,10 @@ test('plugin-recomp-change',
pre_cmd('$MAKE -s --no-print-directory -C plugin-recomp package.plugins01 TOP={top}')
],
run_command, ['$MAKE -s --no-print-directory plugin-recomp-change'])
+
+test('plugin-recomp-change-prof',
+ [extra_files(['plugin-recomp/', 'plugin-recomp-test.hs']),
+ pre_cmd('$MAKE -s --no-print-directory -C plugin-recomp package.plugins01 TOP={top}'),
+ when(not config.have_profiling,skip)
+ ],
+ run_command, ['$MAKE -s --no-print-directory plugin-recomp-change-prof'])
diff --git a/testsuite/tests/plugins/plugin-recomp-change.stderr b/testsuite/tests/plugins/plugin-recomp-change-prof.stderr
similarity index 50%
copy from testsuite/tests/plugins/plugin-recomp-change.stderr
copy to testsuite/tests/plugins/plugin-recomp-change-prof.stderr
index 91747c8..b801805 100644
--- a/testsuite/tests/plugins/plugin-recomp-change.stderr
+++ b/testsuite/tests/plugins/plugin-recomp-change-prof.stderr
@@ -3,4 +3,10 @@ Got options:
Simple Plugin Pass Run
Simple Plugin Passes Queried
Got options:
+Simple Plugin Pass Run
+Simple Plugin Passes Queried
+Got options:
+Simple Plugin Pass Run 2
+Simple Plugin Passes Queried
+Got options:
Simple Plugin Pass Run 2
diff --git a/testsuite/tests/plugins/plugin-recomp/Makefile b/testsuite/tests/plugins/plugin-recomp/Makefile
index db2df8d..9ee7737 100644
--- a/testsuite/tests/plugins/plugin-recomp/Makefile
+++ b/testsuite/tests/plugins/plugin-recomp/Makefile
@@ -16,6 +16,8 @@ package.%:
"$(GHC_PKG)" init pkg.$*/local.package.conf
- pkg.$*/setup configure --distdir pkg.$*/dist -v0 $(CABAL_PLUGIN_BUILD) --ghc-option="$(RUN)" --prefix="$(HERE)/pkg.$*/install" --with-compiler="$(TEST_HC)" --with-hc-pkg="$(GHC_PKG)" --package-db=pkg.$*/local.package.conf $(if $(findstring YES,$(HAVE_PROFILING)), --enable-library-profiling)
+ # The bogus extra-lib-dirs ensures the package is registered with multiple
+ # dynamic-library-directories which tests that the fix for #15475 works
+ pkg.$*/setup configure --distdir pkg.$*/dist -v0 $(CABAL_PLUGIN_BUILD) --ghc-option="$(RUN)" --prefix="$(HERE)/pkg.$*/install" --with-compiler="$(TEST_HC)" --with-hc-pkg="$(GHC_PKG)" --extra-lib-dirs="$(HERE)" --package-db=pkg.$*/local.package.conf $(if $(findstring YES,$(HAVE_PROFILING)), --enable-library-profiling)
pkg.$*/setup build --distdir pkg.$*/dist -v0
pkg.$*/setup install --distdir pkg.$*/dist -v0
More information about the ghc-commits
mailing list