Add cabal flag and CPP option to use Loc from monad-logger. by dmjio · Pull Request #1076 · yesodweb/persistent

dmjio · 2020-04-19T10:39:16Z

If template haskell is disabled in monad-logger, this needs to
be propagated to persistent. Otherwise, type errors ensue.

Database/Persist/Sql/Raw.hs:43:18: error:
    • Couldn't match type ‘Control.Monad.Logger.Loc’
                     with ‘Language.Haskell.TH.Syntax.Loc’
      NB: ‘Language.Haskell.TH.Syntax.Loc’
            is defined in ‘Language.Haskell.TH.Syntax’
                in package ‘template-haskell-2.14.0.0’
          ‘Control.Monad.Logger.Loc’
            is defined in ‘Control.Monad.Logger’
                in package ‘monad-logger-0.3.30’
      Expected type: Control.Monad.Logger.Loc
                     -> Control.Monad.Logger.LogSource
                     -> Control.Monad.Logger.LogLevel
                     -> fast-logger-2.4.15:System.Log.FastLogger.LogStr.LogStr
                     -> IO ()
        Actual type: LogFunc
    • In the second argument of ‘runLoggingT’, namely
        ‘(connLogFunc conn)’
      In a stmt of a 'do' block:
        runLoggingT
          (logDebugNS (pack "SQL") $ T.append sql $ pack $ "; " ++ show vals)
          (connLogFunc conn)
      In the expression:
        do runLoggingT
             (logDebugNS (pack "SQL") $ T.append sql $ pack $ "; " ++ show vals)
             (connLogFunc conn)
           getStmtConn conn sql
   |
43 |                 (connLogFunc conn)
   |                  ^^^^^^^^^^^^^^^^

Database/Persist/Sql/Raw.hs:65:10: error:
    • Couldn't match type ‘Control.Monad.Logger.Loc’
                     with ‘Language.Haskell.TH.Syntax.Loc’
      NB: ‘Language.Haskell.TH.Syntax.Loc’
            is defined in ‘Language.Haskell.TH.Syntax’
                in package ‘template-haskell-2.14.0.0’
          ‘Control.Monad.Logger.Loc’
            is defined in ‘Control.Monad.Logger’
                in package ‘monad-logger-0.3.30’
      Expected type: Control.Monad.Logger.Loc
                     -> Control.Monad.Logger.LogSource
                     -> Control.Monad.Logger.LogLevel
                     -> fast-logger-2.4.15:System.Log.FastLogger.LogStr.LogStr
                     -> IO ()
        Actual type: LogFunc
    • In the second argument of ‘runLoggingT’, namely
        ‘(connLogFunc conn)’
      In a stmt of a 'do' block:
        runLoggingT
          (logDebugNS (pack "SQL") $ T.append sql $ pack $ "; " ++ show vals)
          (connLogFunc conn)
      In the expression:
        do conn <- projectBackend `liftM` ask
           runLoggingT
             (logDebugNS (pack "SQL") $ T.append sql $ pack $ "; " ++ show vals)
             (connLogFunc conn)
           stmt <- getStmt sql
           res <- liftIO $ stmtExecute stmt vals
           ....
   |
65 |         (connLogFunc conn)
   |          ^^^^^^^^^^^^^^^^

Note: this change will not affect any code by default, only when the flag is disabled.

Before submitting your PR, check that you've:

Bumped the version number
Documented new APIs with Haddock markup
Added @since declarations to the Haddock

After submitting your PR:

Update the Changelog.md file with a link to your PR
Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

MaxGabriel · 2020-08-09T00:32:53Z

I'm not sure if I'm reading this right, but it kind of looks like Loc is always exported from monad-logger (the fake one if you have template haskell turned off, the TH one if you have it turned one). Could we instead always import Loc from monad-logger, and then we'd just get the correct version depending on what flag you have on monad-logger?

MaxGabriel · 2020-08-09T17:23:01Z

If I'm correct about that, it should probably be documented in monad-logger that that's the intended approach

dmjio · 2020-08-10T01:14:01Z

I'm trying to understand exactly why I did this. It does seem that monad-logger exports Loc from either template-haskell or its own defined Loc here. So it seems as if the changes I proposed would be redundant. If that were the case I would not expect the error message above to occur.

I honestly think Loc has the correct type, but for some reason GHC is not allowing the type being exported to be used since it comes from a different package. It would seem that nobody has tried to disable template_haskell in monad-logger and then use persistent with it, but I could be wrong here.

Here is a minimal reproduction (nix-build default.nix) of what I'm experiencing using the latest persistent-2.10.5.2 and monad-logger-0.3.34

# default.nix
{ pkgs ? import <nixpkgs> {} }: # nix rev 96069f7d890b90cbf4e8b4b53e15b036210ac146
with pkgs.haskell.lib;
let
  monad-logger = disableCabalFlag pkgs.haskellPackages.monad-logger "template_haskell";
  persistent = pkgs.haskellPackages.persistent.override { inherit monad-logger; };
in
  persistent

(shows above error)

MaxGabriel · 2020-08-11T14:26:02Z

Oh, what I think the issue is is that Persistent does not import Loc from monad-logger, it always imports it from template haskell:

persistent/persistent/Database/Persist/Sql/Types/Internal.hs

Line 39 in be8901e

import Language.Haskell.TH.Syntax (Loc)

Would you be able to try Persistent always importing like this:

import Control.Monad.Logger (LogSource, LogLevel, Loc)

and seeing if that compiles for you?

If template haskell is disabled in monad-logger, this needs to be propagated to persistent. Otherwise, type errors ensue.

Revert monad-logger-th changes.

dmjio · 2020-08-11T17:40:42Z

Nice, worked for both template_haskell enabled and disabled on monad-logger. I reverted the flag stuff, and we can squash the other commits to clean up the history.

MaxGabriel · 2020-08-12T01:51:34Z

Nice, do you mind adding a note to the changelog file about this?

dmjio · 2020-08-12T11:05:18Z

Done.

MaxGabriel · 2020-08-12T13:11:11Z

Thanks, merged! I also added documentation about this to monad-logger

dmjio · 2020-08-12T13:19:36Z

Awesome, thanks a ton 🍻

dmjio added 4 commits August 11, 2020 13:26

Add cabal flag and CPP option to use Loc from monad-logger.

9eaebd7

If template haskell is disabled in monad-logger, this needs to be propagated to persistent. Otherwise, type errors ensue.

Bump version.

b5ed8f5

Export 'Loc' from Control.Monad.Logger.

f49dc49

Revert monad-logger-th changes.

Remove CPP.

685f4d5

MaxGabriel mentioned this pull request Aug 12, 2020

Document Loc snoyberg/monad-logger#26

Merged

Update ChangeLog.md

5d90a53

MaxGabriel approved these changes Aug 12, 2020

View reviewed changes

MaxGabriel merged commit 31e25c4 into yesodweb:master Aug 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cabal flag and CPP option to use Loc from monad-logger.#1076

Add cabal flag and CPP option to use Loc from monad-logger.#1076
MaxGabriel merged 5 commits intoyesodweb:masterfrom
dmjio:monad-logger-th

dmjio commented Apr 19, 2020 •

edited

Loading

Uh oh!

MaxGabriel commented Aug 9, 2020

Uh oh!

MaxGabriel commented Aug 9, 2020

Uh oh!

dmjio commented Aug 10, 2020 •

edited

Loading

Uh oh!

MaxGabriel commented Aug 11, 2020

Uh oh!

dmjio commented Aug 11, 2020

Uh oh!

MaxGabriel commented Aug 12, 2020

Uh oh!

dmjio commented Aug 12, 2020 •

edited

Loading

Uh oh!

MaxGabriel commented Aug 12, 2020

Uh oh!

dmjio commented Aug 12, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dmjio commented Apr 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaxGabriel commented Aug 9, 2020

Uh oh!

MaxGabriel commented Aug 9, 2020

Uh oh!

dmjio commented Aug 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaxGabriel commented Aug 11, 2020

Uh oh!

dmjio commented Aug 11, 2020

Uh oh!

MaxGabriel commented Aug 12, 2020

Uh oh!

dmjio commented Aug 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaxGabriel commented Aug 12, 2020

Uh oh!

dmjio commented Aug 12, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dmjio commented Apr 19, 2020 •

edited

Loading

dmjio commented Aug 10, 2020 •

edited

Loading

dmjio commented Aug 12, 2020 •

edited

Loading