Filter plugin dylib locations
ClosedPublic

Authored by christiaanb on Aug 6 2018, 9:50 AM.

Details

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 Trac #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.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
christiaanb created this revision.Aug 6 2018, 9:50 AM
mpickering accepted this revision.Aug 6 2018, 9:58 AM

I have tested that this patch fixes the panic.

This revision is now accepted and ready to land.Aug 6 2018, 9:58 AM

Can you add a test?

I believe you can just say findDynLibs = filterM doesFileExist.

@monoidal I'll switch to just using filterM. w.r.t a test I'll see what I can do: I'd need to figure out how to either get A: two library search paths for a package, or B: get two libraries for one package.

bgamari requested changes to this revision.Aug 6 2018, 5:42 PM

Requesting filterM change and a test.

This revision now requires changes to proceed.Aug 6 2018, 5:42 PM
christiaanb updated this revision to Diff 17602.Aug 7 2018, 6:44 AM

Use filterM and add tests

Seems there’s another bug: https://ghc.haskell.org/trac/ghc/ticket/15492

I’ll fix that in this patch as well, so don’t merge yet.

christiaanb updated this revision to Diff 17613.Aug 9 2018, 8:16 AM
  • Correctly handle WayDyn in mkPluginUsage

Fixes trac Trac #15492

  • Correctly handle WayDyn in mkPluginUsage

Only run test for Trac Trac #15492 when we have prof libs

  • Correctly handle WayDyn in mkPluginUsage
mpickering accepted this revision.Aug 11 2018, 3:05 AM

This patch works for me.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 11 2018, 1:18 PM
Closed by commit rGHCb324c5624432: Filter plugin dylib locations (authored by christiaanb, committed by monoidal). · Explain Why
This revision was automatically updated to reflect the committed changes.