Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change imports to work with functored AST #8084

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Ericson2314
Copy link

Unfortunately due to https://gitlab.haskell.org/ghc/ghc/-/issues/19507
this cannot be perfectly seamless

@jgm
Copy link
Owner

jgm commented Jun 17, 2022

Is it possible to get this building properly with the CI?

@Ericson2314
Copy link
Author

Ericson2314 commented Jun 17, 2022

I'll rebase it. I would think it should work with pandoc-types pre- and post- my PR.

@Ericson2314
Copy link
Author

Oh good, this does work with the old deps too as confirmed by CI.

@jgm
Copy link
Owner

jgm commented Jul 4, 2022

Can you add to this PR stanzas in cabal.project that force it to use the modified versions of pandoc-types and pandoc-lua-marshall (from the repositories)?
Otherwise we're not really testing how everything fits together.

@Ericson2314
Copy link
Author

Fix conflicts and then re-added that commit. From my local building, We seems to be building with and without the other PRs, as intended.

@Ericson2314
Copy link
Author

@jgm Tests failed because of a cabal install bug. I worked around the bug by checking out the repos manually. I would be happy to do a call any answer any questions you have, walk you through things live, if they helped you review this.

@jgm
Copy link
Owner

jgm commented Sep 26, 2022

@Ericson2314 Can you point me to documentation of the cabal install bug, and instructions for working around it?
What is triggering the bug in this case?

@Ericson2314
Copy link
Author

@jgm Sorry I keep on missing your replies! haskell/cabal#6888 is the issue. I think the workaround was just to go into those repos and check out the right branch/commit by hand.

@jgm
Copy link
Owner

jgm commented Nov 6, 2022

Yes, I think you need to use tag: instead of branch:.

@Ericson2314
Copy link
Author

@jgm Should I push some tags then? I would have to push more if you want changes to any of the constituent PRs.

@jgm
Copy link
Owner

jgm commented Nov 6, 2022

Yes, without the tags, we don't have CI testing how it all fits together.
Obviously the tags would have to be changed at some point, and eventually removed in favor of released versions, but the first step is to be able to evaluate this.

(actually revs not tags) to work around haskell/cabal#6888
@Ericson2314
Copy link
Author

Ericson2314 commented Nov 6, 2022

Oh! One can just put in a commit hash as a "tag". That's not so many hops to jump through then :). Done.

(I did it blind in github, knock on wood there's not a silly typo!)

@Ericson2314
Copy link
Author

cabal: Cannot select only the dependencies (as requested by the
'--only-dependencies' flag), the package pandoc-types-1.22.2.1 is required by
a dependency of one of the other targets.

Ugh ugh ugh...

I understand as a maintainer it's much less stressful to let CI build things instead of trying them out locally, but perhaps building locally is the path of least resistance at this point? I would be happy to take some time to go on a call and answer any questions in real time if that is of any use.

@jgm
Copy link
Owner

jgm commented Nov 7, 2022

Just set the version of your version of pandoc-types to 1.22.2.1 for now.
If your changes are accepted, of course we'll have to create a new release with a bumped version, but this isn't required for testing.

@Ericson2314
Copy link
Author

@jgm erm... I think it is 1.22.2.1 already? I just added modules to pandoc-types.cabal.

@jgm
Copy link
Owner

jgm commented Nov 7, 2022

OK, I jumped to conclusions. I see what is going on. If I could eliminate the separate "dependencies only" step in the CI, that would fix it -- that step was needed because of a cabal-install bug that has been fixed a while back, I think, so let me look into this.

Meanwhile I built this locally. Results:

Compiler warnings:

src/Text/Pandoc/Citeproc/Name.hs:102:11: warning: [-Wincomplete-patterns]
    Pattern match(es) are non-exhaustive
    In a case alternative:
        Patterns of type ‘[[[Inline]]]’ not matched:
            (_:_)
            (_:_)
            (_:_)
            (_:_)
            ...
    |
102 |           case commaParts of
    |           ^^^^^^^^^^^^^^^^^^...

src/Text/Pandoc/Citeproc/Name.hs:102:11: warning:
    Pattern match checker ran into -fmax-pmcheck-models=30 limit, so
      • Redundant clauses might not be reported at all
      • Redundant clauses might be reported as inaccessible
      • Patterns reported as unmatched might actually be matched
    Suggested fix:
      Increase the limit or resolve the warnings to suppress this message.
    |
102 |           case commaParts of
    |           ^^^^^^^^^^^^^^^^^^...
Running 1 test suites...
Test suite test-pandoc-lua-engine: RUNNING...
Running 1 test suites...
Test suite test-pandoc: RUNNING...
pandoc Lua engine
  Lua filters
    macro expansion via filter:                 FAIL
      test/Tests/Lua.hs:237:
      a '{{helloworld}}' string is expanded
      expected: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Emph [Inline {unInline = Str "Hello, World"}]}]}]
       but got: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Str "{{helloworld}}"}]}]
      Use -p '/macro expansion via filter/' to rerun this test only.
    convert all plains to paras:                FAIL
      test/Tests/Lua.hs:237:
      plains become para
      expected: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = BulletList [[Block {unBlock = Para [Inline {unInline = Str "alfa"}]}],[Block {unBlock = Para [Inline {unInline = Str "bravo"}]}]]}]
       but got: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = BulletList [[Block {unBlock = Plain [Inline {unInline = Str "alfa"}]}],[Block {unBlock = Plain [Inline {unInline = Str "bravo"}]}]]}]
      Use -p '/convert all plains to paras/' to rerun this test only.
    convert display math to inline math:        FAIL
      test/Tests/Lua.hs:237:
      display math becomes inline math
      expected: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Math InlineMath "5+5"}]}]
       but got: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Math DisplayMath "5+5"}]}]
      Use -p '/convert display math to inline math/' to rerun this test only.
    parse raw markdown blocks:                  FAIL
      test/Tests/Lua.hs:237:
      raw markdown block is converted
      expected: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Emph [Inline {unInline = Str "charly"}]},Inline {unInline = Space},Inline {unInline = Strong [Inline {unInline = Str "delta"}]}]}]
       but got: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = RawBlock (Format "markdown") "*charly* **delta**"}]
      Use -p '/parse raw markdown blocks/' to rerun this test only.
    allow shorthand functions for quote types:  FAIL
      test/Tests/Lua.hs:237:
      single quoted becomes double quoted string
      expected: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Quoted DoubleQuote [Inline {unInline = Str "simple"}]}]}]
       but got: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Quoted SingleQuote [Inline {unInline = Str "simple"}]}]}]
      Use -p '/allow shorthand functions for quote types/' to rerun this test only.
    Count inlines via metatable catch-all:      FAIL
      test/Tests/Lua.hs:237:
      filtering with metatable catch-all failed
      expected: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Str "7"}]}]
       but got: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Str "0"}]}]
      Use -p '/Count inlines via metatable catch-all/' to rerun this test only.
    Smart constructors:                         FAIL
      test/Tests/Lua.hs:237:
      smart constructors returned a wrong result
      expected: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = BulletList [[Block {unBlock = Para [Inline {unInline = Str "Hello"}]}],[Block {unBlock = Para [Inline {unInline = Str "World"}]}]]},Block {unBlock = DefinitionList [([Inline {unInline = Str "foo"}],[[Block {unBlock = Para [Inline {unInline = Str "placeholder"}]}]])]},Block {unBlock = LineBlock [[Inline {unInline = Str "Moin"}],[Inline {unInline = Str "Welt"}]]},Block {unBlock = OrderedList (1,DefaultStyle,DefaultDelim) [[Block {unBlock = Plain [Inline {unInline = Str "one"}]}],[Block {unBlock = Plain [Inline {unInline = Str "two"}]}]]}]
       but got: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para []}]
      Use -p '/Smart constructors/' to rerun this test only.
    Convert header upper case:                  FAIL
      test/Tests/Lua.hs:237:
      converting header to upper case failed
      expected: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Header 1 ("",[],[]) [Inline {unInline = Str "LES"},Inline {unInline = Space},Inline {unInline = Str "\201TATS-UNIS"}]},Block {unBlock = Para [Inline {unInline = Str "text"}]}]
       but got: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Header 1 ("",[],[]) [Inline {unInline = Str "les"},Inline {unInline = Space},Inline {unInline = Str "\233tats-unis"}]},Block {unBlock = Para [Inline {unInline = Str "text"}]}]
      Use -p '/Convert header upper case/' to rerun this test only.
    Attribute lists are convenient to use:      FAIL
      test/Tests/Lua.hs:237:
      Attr doesn't behave as expected
      expected: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Div ("",[],[("one","eins"),("three","3"),("five","5")]) [Block {unBlock = Para [Inline {unInline = Str "nil"}]}]}]
       but got: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Div ("",[],[("one","1"),("two","2"),("three","3")]) [Block {unBlock = Para [Inline {unInline = Str "nil"}]}]}]
      Use -p '/Attribute lists are convenient to use/' to rerun this test only.
    Filter list of inlines:                     FAIL
      test/Tests/Lua.hs:237:
      List of inlines
      expected: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Str "Hello,"},Inline {unInline = Space},Inline {unInline = Str "World!"},Inline {unInline = Space},Inline {unInline = Str "Wassup?"}]}]
       but got: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Str "Hello,"},Inline {unInline = LineBreak},Inline {unInline = Str "World!"},Inline {unInline = Space},Inline {unInline = Str "Wassup?"}]}]
      Use -p '/Filter list of inlines/' to rerun this test only.
    Script filename is set:                     FAIL
      test/Tests/Lua.hs:237:
      unexpected script name
      expected: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Str "lua/script-name.lua"}]}]
       but got: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Str "ignored"}]}]
      Use -p '/Script filename is set/' to rerun this test only.
    require file:                               FAIL
      test/Tests/Lua.hs:237:
      requiring file failed
      expected: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Str "lua/require-file.lua"}]}]
       but got: Pandoc (Meta {unMeta = fromList []}) [Block {unBlock = Para [Inline {unInline = Str "ignored"}]}]
      Use -p '/require file/' to rerun this test only.
  Lua modules
    pandoc:                                     FAIL (0.01s)
      walk_block // block walking order:
      lua/module/pandoc.lua:313: 
      expected values to be equal, got '1234' and ''
      
      walk_inline // inline walking order:
      lua/module/pandoc.lua:332: 
      expected values to be equal, got '1234' and ''
      
      
      Use -p '$0=="pandoc Lua engine.Lua modules.pandoc"' to rerun this test only.
    pandoc.utils:                               FAIL
      make_sections // sanity check:
      lua/module/pandoc-utils.lua:90: 
      expected values to be equal, got 'Div' and 'Block'
      
      from_simple_table // converts SimpleTable to Table:
      lua/module/pandoc-utils.lua:297: 
      expected values to be equal, got 'Table' and 'Block'
      
      
      Use -p '/pandoc.utils/' to rerun this test only.
couldn't read native
  Custom writers
    default testsuite:                          FAIL
      ExitFailure 64
      Use -p '/default testsuite/' to rerun this test only.
couldn't read native
    tables testsuite:                           FAIL
      ExitFailure 64
      Use -p '/tables testsuite/' to rerun this test only.

16 out of 43 tests failed (0.15s)

@jgm
Copy link
Owner

jgm commented Nov 7, 2022

The cabal fix I need to avoid a separate --dependencies-only is only in the latest version, 3.8.1.0:

Apply local options only to local packages haskell/cabal#7998 haskell/cabal#7973

Command-line ghc-options only applies to local packages
program-options stanza only applies to local packages

jgm added a commit that referenced this pull request Nov 7, 2022
Previously we had to work around a cabal-install bug that caused
`-Werror` to react to dependencies as well as local packages,
but this is fixed in cabal 3.8.1.0.  There are some cases in which
building dependencies only doesn't work (see #8084).
@Ericson2314
Copy link
Author

Thanks @jgm! And thanks for reminding me about those tests.

@jgm
Copy link
Owner

jgm commented Nov 7, 2022

I just pushed a CI change that should help, so if you rebase against master, hopefully the CI will build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants