Support base-4.8

[pharpend]

This should not be merged into master until https://github.com/jwiegley/github/pull/100 is merged, and github-0.13.2 is published. The new cabal.config file, as generated by cabal freeze, points to github-0.13.2, which is the version on my branch. However, my change to github has yet to be published on jwiegley's branch.

I unfortunately had to break backward compatibility with ghc <= 7.0.4, but I don't think anyone still uses that.

[pharpend]

Is there some barrier to this being merged?

[wolftune]

I'm testing and finding some issues.

[chreekat]

This patch will be accepted, and thanks for working on it. :) My concerns right now are:

1. No Haskeller (@singpolyma, @nkaretnikov nor myself) has reviewed it yet.
2. There are no instructions attached for how a person can ease through this transition. Yes, I know it's very simple, but still. Do we suggest cleaning out the sandbox and rebuilding? Or just running cabal install? Or maybe just cabal install --only-dependencies?

[pharpend]

Ah ah okay

cabal clean && cabal install -j -fdev --enable tests

That should do it.

I haven't tested this with the old base, so that might not work.

[wolftune]

Those are not the only issues.

First, we should remove base itself from any cabal.config because otherwise that strictly pegs the GHC version when it otherwise might not be needed to be pegged.

Second, even after removing the base constraint itself, I couldn't build with this version of the cabal.config under GHC 7.8.4

We don't need to clean out the sandbox, but we do need to run some cabal install (I always use -fdev) in order to test that things build with changed dependencies.

Now, I could not cabal install at all until I removed cabal.config, and then:

Model/Currency.hs:92:31:
    parse error (possibly incorrect indentation or mismatched brackets)
cabal: Error: some packages failed to install:
Snowdrift-0.1.3 failed during the building phase. The exception was:
ExitFailure 1

So, there are definitely serious issues with this patch. The if statements aren't recognized correctly. I don't know what's wrong or right here. Perhaps this is entirely about GHC 7.10 stuff. Yes, we want it to build with GHC 7.10, but we don't yet want to force everyone to update to 7.10… although with Vagrant option, if we got that to 7.10, I wouldn't actually mind much if we pushed everything to update, but we'd need the actual deploy to update and it certainly isn't the simplest situation here. Maybe this is just a problem with the #if syntax…

But even if that is fixed, we'll need a new cabal.config that actually works under GHC 7.8.x at this point.

[pharpend]

@wolftune It's probably that I wrote separate code for ghc >= 7.10 and ghc < 7.10. But, since I only have 7.10, GHC ignored the < 7.10 code, where I likely made a mistake.

[pharpend]

@singpolyma I made some of the changes you suggested.

[pharpend]

@wolftune I don't see how Model/Currency.hs could be giving you any errors. Here's what the code would look like to GHC post-CPP:

    readsPrec _ ('$':s) = let (ipart, more) = 
                                span (`elem` (",0123456789") s
                              (fpart, rest) = case more of 
                                                  '.' : more' -> 
                                                        span (`elem` "0123456789") more'
                                                  _ -> ("", more)

                              result = Milray $ read $ filter (/= ',') ipart ++ take 4 (fpart ++ repeat '0')
                           in [ (result, rest) ]

Nothing wrong with that code, indentation-wise.

AH! I found it! Stupid GHC and its crappy error messages.

[wolftune]

Note: once this is merged, we should also add to (the latest version otherwise!) of the GUIDE.md that GHC 7.10 is supported, as it currently reads as though we require specifically 7.8.x.

[wolftune]

Current version failed for me under 7.8.4:

Import.hs:127:53:
    Not in scope: ‘mempty’
    Perhaps you meant one of these:
      ‘T.empty’ (imported from Data.Text),
      ‘M.empty’ (imported from Data.Map)

[pharpend]

@wolftune 3873df5 fixes your note.

[pharpend]

It apparently adds merge conflicts too.

I'll get to merging once cabal finishes.

In conclusion, I'll get to merging next week.

[pharpend]

@wolftune Ahh, okay, mempty is in Prelude now.

That should be fixed in 6852354

[pharpend]

Okay, I did the merge on my end.

[wolftune]

Ok, pulled the latest to test. I didn't get the mempty error anymore, but now I get Not in scope: ‘mconcat’ (for many instances of that)

[pharpend]

@wolftune 9bc89d1 should fix that

[wolftune]

Tried building again. Now:

Handler/Project.hs:327:11:
    Not in scope: ‘defaultTimeLocale’
    In the splice: $(widgetFile "applications")

[pharpend]

I'll look at that in the morning

[chreekat]

I spent a little time trying this out and thinking about what it would mean for the project, and I have come to form some opinions. Let me know what you think of them.

1. If NixOS users can't work with ghc-7.8 directly, it might be best to have them work on Snowdrift by using ghc-7.8 via Vagrant.
2. Since testing new code requires manual builds, supporting two versions of ghc/base would double the amount of manual building required for each merge.
3. Some of us (me) don't even currently have access to 7.10, so we can't do the second half of that manual testing anyway.
4. The Snowdrift package must support 7.8, since that's what's available in production.
5. It is definitely in our best interest to use 'cabal freeze' and/or Stackage, and I'm not sure it's possible to support both 7.8 and 7.10 and have frozen dependency versions. Few dependencies support both 7.8 and 7.10 in a single version.

On the basis of these opinions, I think it's best to keep 7.10 support out of master.

That said, we should consider keeping this branch around. There will come a day that we switch from supporting 7.8 to supporting 7.10 (or possibly a day when we hammer out all of the issues with supporting both), and it may prove useful in that eventuality.

[pharpend]

I agree with @chreekat . Why don't you push base-4.8 into its own branch (i.e. make this branch snowdriftcoop/snowdrift -b base-4.8

[chreekat]

I had a brainstorm.

There are two difficulties in supporting both 7.8 and 7.10. One is relatively easy: changes to code files. That difficulty is overcome with CPP. The other is changes to build configuration. That one we don't have a solution to, yet. Here's an idea, though: what if we used a meta build tool like shake? It could be used to pick a particular cabal.config/Snowdrift.cabal based on which ghc is available.

[pharpend]

I don't know how to use shake. I'm not opposed to its usage, I just don't know how to do it. I don't have the time at the moment to learn a new toolchain, so someone else would have to maintain the shake stuff.

[chreekat]

Heh, no worries. If we eventually support both, I do think that's the way we'll go (or maybe just make!), but there's all so much to do right now...

I'm going to close this for now, since we're sort of at an impasse.

[singpolyma]

Since testing new code requires manual builds, supporting two versions of ghc/base would double the amount of manual building required for each merge.

Having support for two versions doesn't mean we have to officially support those versions.

Does git.gnu.io have gitlab CI in it? Would be nice to have an official CI of some kind...

[chreekat]

I don't think many — or any — of the "services" listed in project settings are actually functional. There's a hook for Gitlab CI, but Gitlab CI only supports Gitlab proper I think.

CI would definitely be cool! I was implicitly stating that by talking about "manual builds". :)

On Tue, May 05, 2015 at 12:16:33PM -0700, Stephen Paul Weber wrote:

Since testing new code requires manual builds, supporting two versions of
ghc/base would double the amount of manual building required for each
merge.

Having support for two versions doesn't mean we have to officially support those versions.

Does git.gnu.io have gitlab CI in it? Would be nice to have an official CI of some kind...

— Reply to this email directly or view it on GitHub.*

[sajith]

Bryan Richter notifications@github.com wrote:

CI would definitely be cool! I was implicitly stating that by talking about "manual builds". :)

I've played with Travis CI a bit. I could perhaps add a .travis.yml to snowdrift? Some CI should be better than no CI.

[chreekat]

Hey, thanks for the suggestion! I think we may want to talk about it a bit first. I've started a wiki at https://snowdrift.coop/p/snowdrift/w/en/continuous-integration — please feel free to chime in.

[singpolyma]

I've had a travis config in my snowdrift github for awhile now, but since travis is github-only, it's not a really good fit for snowdrift.