search

faylang / fay

A proper subset of Haskell that compiles to JavaScript
https://github.com/faylang/fay/wiki
BSD 3-Clause "New" or "Revised" License
1.29k stars 86 forks source link

Please relax upper bounds #469

Closed ezzieyguywuf closed 3 years ago

ezzieyguywuf commented 3 years ago

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

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

swamp-agr commented 3 years ago

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

ezzieyguywuf commented 3 years ago

Sure.

We're up to ghc 8.10.3 and cabal-install 3.2.0.0

swamp-agr commented 3 years ago

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:

    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 commented 3 years ago

I saw the same thing with the newlines, my working theory is that it has to do with newer base-compat, but I haven't not really dug into it too much

Regarding ghc-pkg and fay-base, i didn't run into issues here, but maybe I got lucky because of newer GHC?

Finally, since we're talking about fay-base...

I did run into a circular dependency issue with fay-tests and fay-base.

This should probably be a separate issue, but is it possible to:

  1. Use a proper test-suite stanza
  2. Merge fay-base and library fay into a single module (i think this will fix my circular dependency issue)

I'm happy to help however I can, please let me know

swamp-agr commented 3 years ago

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

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

  1. Set up CI pipeline for tests (#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 commented 3 years ago

What do you think about proposed plan?

😍

swamp-agr commented 3 years ago

update

ezzieyguywuf commented 3 years ago

@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 commented 3 years ago

Hi @ezzieyguywuf, thank you for comments.

ezzieyguywuf commented 3 years ago

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 commented 3 years ago

update

swamp-agr commented 3 years ago

473 created to track circular dependencies.

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