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

Please relax upper bounds #469

Closed
ezzieyguywuf opened this issue Jan 8, 2021 · 12 comments
Closed

Please relax upper bounds #469

ezzieyguywuf opened this issue Jan 8, 2021 · 12 comments

Comments

@ezzieyguywuf
Copy link

ezzieyguywuf commented Jan 8, 2021

Specifically, I have ran the test suite with the following with no issues:

  • aeson-1.5.4.1
  • base-compat-0.11.2
  • tasty-1.2.3

By relaxing these upper bounds, and releasing a new version, we will be better able to maintain the gentoo package.

ezzieyguywuf added a commit to ezzieyguywuf/gentoo-haskell that referenced this issue Jan 8, 2021
Upstream issue[1] has been opened to relax the same bounds.

Also, the test package has been added, though it has to be restricted
due to a circular depenedency. Check the comments in the ebuild for a
one-off workaround if you want to run the tests.

[1]: faylang/fay#469

Signed-off-by: Wolfgang E. Sanyer <WolfgangESanyer@gmail.com>
@swamp-agr
Copy link
Contributor

Hi @ezzieyguywuf, could you please provide some details, which versions of GHC and cabal-install are currently used in Gentoo packages?

@ezzieyguywuf
Copy link
Author

ezzieyguywuf commented Jan 9, 2021 via email

ezzieyguywuf added a commit to ezzieyguywuf/gentoo-haskell that referenced this issue Jan 9, 2021
Upstream issue[1] has been opened to relax the same bounds.

Also, the test package has been added, though it has to be restricted
due to a circular depenedency. Check the comments in the ebuild for a
one-off workaround if you want to run the tests.

[1]: faylang/fay#469

Signed-off-by: Wolfgang E. Sanyer <WolfgangESanyer@gmail.com>
ezzieyguywuf added a commit to ezzieyguywuf/gentoo-haskell that referenced this issue Jan 9, 2021
Upstream issue[1] has been opened to relax the same bounds.

Also, the test package has been added, though it has to be restricted
due to a circular depenedency. Check the comments in the ebuild for a
one-off workaround if you want to run the tests.

[1]: faylang/fay#469

Signed-off-by: Wolfgang E. Sanyer <WolfgangESanyer@gmail.com>
@swamp-agr
Copy link
Contributor

Thanks @ezzieyguywuf for your reply! I tried to bump changes and to pass test-cases on GHC 8.8 with cabal-install 3.2.0.0 and it brings some challenges among the way:

  • ghc-pkg should be pointed out explicitly.
  • fay-base path should be specified explicitly.
  • For some reason (I think, it comes with aeson major version bump) actual JSON results contain extra newline characters, so I have to decide how to align tests preserving compatibility with earlier aeson version.
    tests/serialization.hs:                            FAIL (0.94s)
      src/tests/Tests.hs:120:
      tests/serialization.hs
      expected: "{ instance: 'Parametric',\n  slot1: { instance: 'ConcreteRecord', concreteField: 123 } }\n{ instance: 'Parametric',\n  slot1: { instance: 'ConcreteRecord', concreteField: 123 } }\n{ instance: 'Parametric',\n  slot1: { instance: 'ConcreteRecord', concreteField: 123 } }\n{ instance: 'Just',\n  slot1: { instance: 'ConcreteRecord', concreteField: 42 } }\n{ instance: 'Just',\n  slot1: { instance: 'ConcreteRecord', concreteField: 42 } }\n{ instance: 'Just',\n  slot1: { instance: 'ConcreteRecord', concreteField: 42 } }\n42\n{ instance: 'Just', slot1: 42 }\n{ instance: 'Just',\n  slot1: { instance: 'ConcreteRecord', concreteField: 42 } }\n"
       but got: "{\n  instance: 'Parametric',\n  slot1: { instance: 'ConcreteRecord', concreteField: 123 }\n}\n{\n  instance: 'Parametric',\n  slot1: { instance: 'ConcreteRecord', concreteField: 123 }\n}\n{\n  instance: 'Parametric',\n  slot1: { instance: 'ConcreteRecord', concreteField: 123 }\n}\n{\n  instance: 'Just',\n  slot1: { instance: 'ConcreteRecord', concreteField: 42 }\n}\n{\n  instance: 'Just',\n  slot1: { instance: 'ConcreteRecord', concreteField: 42 }\n}\n{\n  instance: 'Just',\n  slot1: { instance: 'ConcreteRecord', concreteField: 42 }\n}\n42\n{ instance: 'Just', slot1: 42 }\n{\n  instance: 'Just',\n  slot1: { instance: 'ConcreteRecord', concreteField: 42 }\n}\n"

I will continue investigation and share progress with you.

@ezzieyguywuf
Copy link
Author

ezzieyguywuf commented Jan 9, 2021 via email

@swamp-agr
Copy link
Contributor

swamp-agr commented Jan 9, 2021

Topics

  1. extra newlines.
  2. circular dependencies.

extra newlines

Thanks for collaboration. I will also cross-check base-compat for this particular reason.

circular dependencies

  • While fay package contains the compiler, fay-base contains an alternative base (Prelude included) that could be compiled with both GHC and fay.
  • Thus, from my point of view, they are separate entities and should exist in separate packages (I am open to other considerations and hopefully could change my mind in future).
  • I do agree that test-cases execution should be delegated to cabal via test-suite stanza.
  • In order to mitigate issue with circular dependencies, I would propose instead to split compiler test-cases and base test-cases and move fay-base cases into corresponding test-suite.

Let me make an assessment for these activities. As of now, my plan is roughly following:

  1. Set up CI pipeline for tests (Allow upload on upgraded hackage #472) with cabal-install 3.2.0.0.
  2. Resolve missing fay-base test errors.
  3. Investigate and resolve extra newlines appeared. (cannot reproduce on CI server)
  4. Make release on hackage (with circular dependencies) in order to unblock packaging activities.
  5. Move fay-tests to test-suite.
  6. Split tests onto fay and fay-base test suites.
  7. Make release on hackage (w/o circular dependencies) to fully resolve the situation.

What do you think about proposed plan?

@ezzieyguywuf
Copy link
Author

ezzieyguywuf commented Jan 9, 2021 via email

@swamp-agr
Copy link
Contributor

update

  • v2-style build was causing missing fay-base package errors. Fay executable was not able to determine fay-base package location since v2-build stores package into Cabal Store (yet another package database).

  • With cabal v2-exec -- ghc-pkg --package-db ./dist-newstyle/packagedb/ghc-$(ghc --numeric-version) list I was able to get broken package database with fay and fay-base that could not be repaired.

  • The solution for CI pipeline was to force v1-install (I do realise that it is a subject of deprecation in next releases). For now it fixes following points:

    • Set up CI pipeline for tests with cabal-install 3.2.0.0.
    • Resolve missing fay-base test errors.
      3 out of 268 tests failed (103.37s)
      

@ezzieyguywuf
Copy link
Author

@swamp-agr thanks for the updates.

Regarding your v2-style issues, I think this is similar to the issues I was seeing with packaging in gentoo as well.

I didn't dig too much into the code, but this is sort of where my suggestion to wrap up the lib:fay and lib:fay-base into a single module came from.

Also, I noticed that fay-base was moved into this repository from outside - can you elaborate on why this change was made?

I have a feel that simply restructuring the project a bit will help with the v2-style issues, and probably solve my gentoo problems as well.

@swamp-agr
Copy link
Contributor

Hi @ezzieyguywuf, thank you for comments.

  • Regarding v2 issues. They needed to investigate further. From what I can observe right now, some mechanics of --base-path argument violated, i.e. I cannot find package sources, while with v1 installation sources stored in $HOME/.cabal/share/$ARCH-$OS-ghc-$GHCVERSION/fay-base-X.X.X.X/src and could be located via ghc-pkg.
  • Regarding reasons to join repositories. It was long time ago. According to CHANGELOG, there were reasons to resolve issues with dependant packages, like fay-text, but to be honest, I could be wrong. I joined as maintainer a bit later, maybe @bergmark could bring more light on the reasons.

@ezzieyguywuf
Copy link
Author

Sure, let's see if @bergmark can provide more background. I have a feeling that whatever the dependency issues were, there's a better fix.

Again, I'm not expert, but just looking at the project structure and comparing to other projects I've seen, my gut tells me that this is where the opportunity for improvement lies.

@swamp-agr
Copy link
Contributor

update

  • fay-0.24.2.0 was successfully uploaded on Hackage: https://hackage.haskell.org/package/fay
  • extra newlines: cannot reproduce on CI issues with JSON (will resolve it separately).
  • Make release on hackage (with circular dependencies) in order to unblock packaging activities.

@swamp-agr
Copy link
Contributor

#473 created to track circular dependencies.

@ezzieyguywuf if you don't mind, feel free to check Hackage and close this issue.

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

No branches or pull requests

2 participants