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

Update to LTS-11 #187

Merged
merged 2 commits into from May 18, 2018
Merged

Update to LTS-11 #187

merged 2 commits into from May 18, 2018

Conversation

ehamberg
Copy link
Contributor

Updates stack.yaml to use LTS-11 (GHC 8.2.2). Not sure if the TODO should be kept.

Copy link
Contributor

@fkm3 fkm3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating it!

I'm kicking off the tests.

stack.yaml Outdated
@@ -16,12 +16,9 @@ packages:
extra-deps:
- snappy-framing-0.1.1
- snappy-0.2.0.2
# TODO: Remove these once the new versions are in lts-9.
- haskell-src-exts-1.19.1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment explaining why we need an older version? e.g. name the module that needs to be updated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fkm3 Done!

@fkm3
Copy link
Contributor

fkm3 commented May 14, 2018

I'm getting this error when I try it:

$ stack setup
Downloaded lts-11.8 build plan.    
AesonException "Error in $.packages.cassava.constraints.flags['bytestring--lt-0_10_4']: Invalid flag name: \"bytestring--lt-0_10_4\""

@ehamberg
Copy link
Contributor Author

@fkm3 Looks like this is an issue with some older versions of stack. Can you try to upgrade stack through your package manager or run stack upgrade? From what I can see this was fixed in stack 1.6.1.

@fkm3
Copy link
Contributor

fkm3 commented May 14, 2018

That does fix it locally for me.

The tests use the Dockerfile which gets stack from download.fpcomplete.com/ubuntu. Maybe that repo has an old version of stack. I'll look more closely later today if I get a chance.

fkm3 added a commit to fkm3/tensorflow-haskell that referenced this pull request May 15, 2018
Required by tensorflow#187.

The version we were using is old enough that it doesn't work with the
latest stackage LTS. haskellstack.org says

    There is also a Ubuntu package for Ubuntu 16.10 and up, but the
    distribution's Stack version lags behind, ...

So, instead of asking them to update it, it's probably better to
download the tar of the version we want.

Somehow updating stack surfaced a new pedantic warning in GradientTest,
so I've fixed that as well (and cleanup some code using snake_case).
fkm3 added a commit to fkm3/tensorflow-haskell that referenced this pull request May 15, 2018
Required by tensorflow#187.

The version we were using is old enough that it doesn't work with the
latest stackage LTS. haskellstack.org says

    There is also a Ubuntu package for Ubuntu 16.10 and up, but the
    distribution's Stack version lags behind, ...

So, instead of asking them to update it, it's probably better to
download the tar of the version we want.

Somehow updating stack surfaced a new pedantic warning in GradientTest,
so I've fixed that as well (and cleaned up some code using snake_case).
@fkm3
Copy link
Contributor

fkm3 commented May 15, 2018

Sent #189 to fix the stack version.

@blackgnezdo
Copy link
Contributor

How bad is it to jump to ghc 8.4 and the nightly stackage snapshot? If we are to experience the pain of the upgrade, might as well reduce the incidence (even at the increased intensity :)

fkm3 added a commit that referenced this pull request May 16, 2018
Required by #187.

The version we were using is old enough that it doesn't work with the
latest stackage LTS. haskellstack.org says

    There is also a Ubuntu package for Ubuntu 16.10 and up, but the
    distribution's Stack version lags behind, ...

So, instead of asking them to update it, it's probably better to
download the tar of the version we want.

Somehow updating stack surfaced a new pedantic warning in GradientTest,
so I've fixed that as well.
@fkm3
Copy link
Contributor

fkm3 commented May 16, 2018

If you sync this to HEAD, then the CI tests should pass and we can merge it.

@blackgnezdo, won't switching to nightly increase the incident? No opinion from me though :)

@blackgnezdo
Copy link
Contributor

I meant pick a dated snapshot of nightly. But I'm also somewhat ambivalent. Just throwing it out there. @ehamberg could you rebase to make sure your change is compatible with HEAD?

@ehamberg
Copy link
Contributor Author

@blackgnezdo Sure. Done.

@fkm3
Copy link
Contributor

fkm3 commented May 16, 2018

You still need to merge baa501b into this PR, otherwise the tests will fail. Thanks!

ehamberg pushed a commit to ehamberg/haskell that referenced this pull request May 16, 2018
Required by tensorflow#187.

The version we were using is old enough that it doesn't work with the
latest stackage LTS. haskellstack.org says

    There is also a Ubuntu package for Ubuntu 16.10 and up, but the
    distribution's Stack version lags behind, ...

So, instead of asking them to update it, it's probably better to
download the tar of the version we want.

Somehow updating stack surfaced a new pedantic warning in GradientTest,
so I've fixed that as well.
@ehamberg
Copy link
Contributor Author

@fkm3 Ah. Missed that. Rebased on top of the latest master now.

@blackgnezdo
Copy link
Contributor

I see some build errors

tensorflow-mnist-0.1.0.0: Test suite ParseTest passed

-- Dumping log file due to warnings: /tfhs/.stack-work/logs/tensorflow-records-conduit-0.1.0.0.log

Configuring tensorflow-records-conduit-0.1.0.0...
Preprocessing library for tensorflow-records-conduit-0.1.0.0..
Building library for tensorflow-records-conduit-0.1.0.0..
[1 of 1] Compiling TensorFlow.Records.Conduit ( src/TensorFlow/Records/Conduit.hs, .stack-work/dist/x86_64-linux/Cabal-2.0.1.0/build/TensorFlow/Records/Conduit.o )

/tfhs/tensorflow-records-conduit/src/TensorFlow/Records/Conduit.hs:40:36: warning: [-Wdeprecations]
In the use of type constructor or class `Conduit'
(imported from Data.Conduit, but defined in conduit-1.3.0.2:Data.Conduit.Internal.Conduit):
Deprecated: "Use ConduitT directly"
|
40 | decodeTFRecords :: MonadThrow m => Conduit B.ByteString m BL.ByteString
| ^^^^^^^

/tfhs/tensorflow-records-conduit/src/TensorFlow/Records/Conduit.hs:44:67: warning: [-Wdeprecations]
In the use of type constructor or class `Producer'
(imported from Data.Conduit, but defined in conduit-1.3.0.2:Data.Conduit.Internal.Conduit):
Deprecated: "Use ConduitT directly"
|
44 | sourceTFRecords :: (MonadResource m, MonadThrow m) => FilePath -> Producer m BL.ByteString
| ^^^^^^^^

/tfhs/tensorflow-records-conduit/src/TensorFlow/Records/Conduit.hs:45:40: warning: [-Wdeprecations]
In the use of `=$='
(imported from Data.Conduit, but defined in conduit-1.3.0.2:Data.Conduit.Internal.Conduit):
Deprecated: "Use .|"
|
45 | sourceTFRecords path = sourceFile path =$= decodeTFRecords
| ^^^

/tfhs/tensorflow-records-conduit/src/TensorFlow/Records/Conduit.hs:48:31: warning: [-Wdeprecations]
In the use of type constructor or class `Conduit'
(imported from Data.Conduit, but defined in conduit-1.3.0.2:Data.Conduit.Internal.Conduit):
Deprecated: "Use ConduitT directly"
|
48 | encodeTFRecords :: Monad m => Conduit BL.ByteString m B.ByteString
| ^^^^^^^

/tfhs/tensorflow-records-conduit/src/TensorFlow/Records/Conduit.hs:52:51: warning: [-Wdeprecations]
In the use of type constructor or class `Consumer'
(imported from Data.Conduit, but defined in conduit-1.3.0.2:Data.Conduit.Internal.Conduit):
Deprecated: "Use ConduitT directly"
|
52 | sinkTFRecords :: (MonadResource m) => FilePath -> Consumer BL.ByteString m ()
| ^^^^^^^^

/tfhs/tensorflow-records-conduit/src/TensorFlow/Records/Conduit.hs:53:38: warning: [-Wdeprecations]
In the use of `=$='
(imported from Data.Conduit, but defined in conduit-1.3.0.2:Data.Conduit.Internal.Conduit):
Deprecated: "Use .|"
|
53 | sinkTFRecords path = encodeTFRecords =$= sinkFile path
| ^^^

: error:
Failing due to -Werror.

-- End of log file: /tfhs/.stack-work/logs/tensorflow-records-conduit-0.1.0.0.log

Log files have been written to: /tfhs/.stack-work/logs/

-- While building custom Setup.hs for package tensorflow-records-conduit-0.1.0.0 using:
/root/.stack/setup-exe-cache/x86_64-linux/Cabal-simple_mPHDZzAJ_2.0.1.0_ghc-8.2.2 --builddir=.stack-work/dist/x86_64-linux/Cabal-2.0.1.0 build lib:tensorflow-records-conduit --ghc-options " -ddump-hi -ddump-to-file"
Process exited with code: ExitFailure 1
Logs have been written to: /tfhs/.stack-work/logs/tensorflow-records-conduit-0.1.0.0.log

Configuring tensorflow-records-conduit-0.1.0.0...
Preprocessing library for tensorflow-records-conduit-0.1.0.0..
Building library for tensorflow-records-conduit-0.1.0.0..
[1 of 1] Compiling TensorFlow.Records.Conduit ( src/TensorFlow/Records/Conduit.hs, .stack-work/dist/x86_64-linux/Cabal-2.0.1.0/build/TensorFlow/Records/Conduit.o )

/tfhs/tensorflow-records-conduit/src/TensorFlow/Records/Conduit.hs:40:36: warning: [-Wdeprecations]
    In the use of type constructor or class `Conduit'
    (imported from Data.Conduit, but defined in conduit-1.3.0.2:Data.Conduit.Internal.Conduit):
    Deprecated: "Use ConduitT directly"
   |
40 | decodeTFRecords :: MonadThrow m => Conduit B.ByteString m BL.ByteString
   |                                    ^^^^^^^

/tfhs/tensorflow-records-conduit/src/TensorFlow/Records/Conduit.hs:44:67: warning: [-Wdeprecations]
    In the use of type constructor or class `Producer'
    (imported from Data.Conduit, but defined in conduit-1.3.0.2:Data.Conduit.Internal.Conduit):
    Deprecated: "Use ConduitT directly"
   |
44 | sourceTFRecords :: (MonadResource m, MonadThrow m) => FilePath -> Producer m BL.ByteString
   |                                                                   ^^^^^^^^

/tfhs/tensorflow-records-conduit/src/TensorFlow/Records/Conduit.hs:45:40: warning: [-Wdeprecations]
    In the use of `=$='
    (imported from Data.Conduit, but defined in conduit-1.3.0.2:Data.Conduit.Internal.Conduit):
    Deprecated: "Use .|"
   |
45 | sourceTFRecords path = sourceFile path =$= decodeTFRecords
   |                                        ^^^

/tfhs/tensorflow-records-conduit/src/TensorFlow/Records/Conduit.hs:48:31: warning: [-Wdeprecations]
    In the use of type constructor or class `Conduit'
    (imported from Data.Conduit, but defined in conduit-1.3.0.2:Data.Conduit.Internal.Conduit):
    Deprecated: "Use ConduitT directly"
   |
48 | encodeTFRecords :: Monad m => Conduit BL.ByteString m B.ByteString
   |                               ^^^^^^^

/tfhs/tensorflow-records-conduit/src/TensorFlow/Records/Conduit.hs:52:51: warning: [-Wdeprecations]
    In the use of type constructor or class `Consumer'
    (imported from Data.Conduit, but defined in conduit-1.3.0.2:Data.Conduit.Internal.Conduit):
    Deprecated: "Use ConduitT directly"
   |
52 | sinkTFRecords :: (MonadResource m) => FilePath -> Consumer BL.ByteString m ()
   |                                                   ^^^^^^^^

/tfhs/tensorflow-records-conduit/src/TensorFlow/Records/Conduit.hs:53:38: warning: [-Wdeprecations]
    In the use of `=$='
    (imported from Data.Conduit, but defined in conduit-1.3.0.2:Data.Conduit.Internal.Conduit):
    Deprecated: "Use .|"
   |
53 | sinkTFRecords path = encodeTFRecords =$= sinkFile path
   |                                      ^^^

<no location info>: error: 
Failing due to -Werror.

[ID: 4581505] Build finished after 975 secs, exit value: 1

Warning: Permanently added 'localhost' (ECDSA) to the list of known hosts.
Build script failed with exit code: 1[11:12:27] /tmp/workspace/workspace/tensorflow_haskell/github/presubmit/artifacts/bazel_invocation_ids does not exist in the workspace, skipping bazel invocation collection

@ehamberg
Copy link
Contributor Author

@blackgnezdo Looks like it's failing due to deprecation warnings + -Werror. I can have a look at fixing the Conduit warnings tomorrow, probably.

- The `Conduit`, `Producer` and `Consumer` aliases are deprecated and
  `ConduitT` is used directly instead
- `=$=` is deprecated and replaced by `.|`
@ehamberg
Copy link
Contributor Author

@blackgnezdo I believe this is fixed now.

@blackgnezdo
Copy link
Contributor

@fkm3, I think we are good to merge this now?

@fkm3 fkm3 merged commit 88dafe8 into tensorflow:master May 18, 2018
@fkm3
Copy link
Contributor

fkm3 commented May 18, 2018

Thanks!

@ehamberg ehamberg deleted the lts-11 branch May 19, 2018 12:50
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.

None yet

4 participants