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

travis-script.sh: test bootstrapping #3349

Closed
wants to merge 1 commit into from
Closed

travis-script.sh: test bootstrapping #3349

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 16, 2016

No description provided.

@ghost
Copy link
Author

ghost commented Apr 16, 2016

Works as expected, meaning fails right now (see #3344).

@23Skidoo
Copy link
Member

I'm a bit worried about how this will affect compile times on Travis.

@23Skidoo
Copy link
Member

@Tuncer Can you rebase this against master and do push -f? I want to evaluate the impact of this PR on compile times before merging.

@ezyang
Copy link
Contributor

ezyang commented Apr 17, 2016

@23Skidoo If we replace the code which uses cabal-install to build cabal-install, with code that uses the bootstrap script, I think that the time to build should be comparable.

@ghost
Copy link
Author

ghost commented Apr 17, 2016

Rebased, which also incorporates the fixed bootstrap.sh, so it should work now.

@ghost
Copy link
Author

ghost commented Apr 17, 2016

@ezyang, I've thought about replacing bits in there as well, but I wasn't sure which parts are okay to remove. Is it a better idea for you to make those changes in a follow-up commit?

@ezyang
Copy link
Contributor

ezyang commented Apr 17, 2016

Yes, assuming @23Skidoo is OK with the Travis time we can defer it.

@ghost
Copy link
Author

ghost commented Apr 17, 2016

Extra commit that moves cabal update and optional cabal install parsec to a later stage, so that we don't run update twice and also use the freshly bootstrapped cabal-install.

@23Skidoo
Copy link
Member

This is now blocked by #3345.

@23Skidoo
Copy link
Member

It looks like #3356 needs to be merged to make the build bot error go away.

@23Skidoo
Copy link
Member

23Skidoo commented May 8, 2016

So the bootstrap script also needs to check that a recent version of bytestring is installed (and install it if it's not).

@ghost
Copy link
Author

ghost commented May 14, 2016

Added bytestring, and while at it, fixed a tiny whitespace issue separately.

@23Skidoo
Copy link
Member

Still fails, bytestring has to be installed before binary.

@ghost
Copy link
Author

ghost commented May 15, 2016

Fixed order.

@ghost
Copy link
Author

ghost commented May 15, 2016

Adjusted bytestring regex to require >=0.10.2.

@ghost
Copy link
Author

ghost commented May 15, 2016

Changed bytestring version that gets fetched to 0.10.6.0, in order to relax its base vsn bound. The previous error was that bytestring >=0.10.2.0 is needed, and 0.10.6.0 is the last bytestring release before the lower bound is bumped.

@ghost
Copy link
Author

ghost commented May 15, 2016

First, binary-0.8.3.0 complained that it needs bytestring >=0.10.2:

Configuring binary-0.8.3.0...
Setup: Encountered missing dependencies:
bytestring >=0.10.2

To fix that, I've tried giving it 0.10.8.0, but that failed because GHC 7.6.3's base is too old. Therefore, I've changed it to 0.10.6.0, since that's the last version before the base lower bound was bumped, and we cannot go older than 0.10.2.0 anyway.

Now we have a new error:

Configuring cabal-install-1.25.0.0...
Warning: This package indirectly depends on multiple versions of the same
package. This is highly likely to cause a compile failure.
package Cabal-1.25.0.0 requires binary-0.5.1.1
package text-1.2.2.1 requires binary-0.8.3.0
package cabal-install-1.25.0.0 requires binary-0.8.3.0
package unix-2.6.0.1 requires bytestring-0.10.0.2
package binary-0.5.1.1 requires bytestring-0.10.0.2
package Cabal-1.25.0.0 requires bytestring-0.10.0.2
package zlib-0.6.1.1 requires bytestring-0.10.6.0
package text-1.2.2.1 requires bytestring-0.10.6.0
package tar-0.5.0.3 requires bytestring-0.10.6.0
package parsec-3.1.9 requires bytestring-0.10.6.0
package network-2.6.2.1 requires bytestring-0.10.6.0
package hashable-1.2.4.0 requires bytestring-0.10.6.0
package hackage-security-0.5.1.0 requires bytestring-0.10.6.0
package ed25519-0.0.5.0 requires bytestring-0.10.6.0
package cryptohash-sha256-0.11.7.1 requires bytestring-0.10.6.0
package cabal-install-1.25.0.0 requires bytestring-0.10.6.0
package binary-0.8.3.0 requires bytestring-0.10.6.0
package base64-bytestring-1.0.0.1 requires bytestring-0.10.6.0
package base16-bytestring-0.1.1.6 requires bytestring-0.10.6.0
package HTTP-4000.3.3 requires bytestring-0.10.6.0
Building cabal-install-1.25.0.0...
[...]
Distribution/Client/World.hs:103:33:
    Couldn't match type `B.ByteString'
                  with `bytestring-0.10.0.2:Data.ByteString.Lazy.Internal.ByteString'
    Expected type: [Char]
                   -> bytestring-0.10.0.2:Data.ByteString.Lazy.Internal.ByteString
      Actual type: [Char] -> B.ByteString
    In the second argument of `(.)', namely `B.pack'
    In the expression: writeFileAtomic world . B.pack
    In a stmt of a 'do' block:
      writeFileAtomic world . B.pack
      $ unlines [(display pkg) | pkg <- pkgsNewWorld]

What should we do?

@hvr
Copy link
Member

hvr commented May 15, 2016

@Tuncer fwiw, bytestring-0.10.8.1 will support GHC 7.6 again

@ghost
Copy link
Author

ghost commented May 15, 2016

@hvr, thanks.

@23Skidoo, what happened to the new hackage-security release? Is it ready, and would it help us avoid adding bytestring and also allow us to remove the other temporary packages?

@23Skidoo
Copy link
Member

I think what we can try doing to fix the error is check the version of bytestring installed and then install an older version of binary if bytestring is < 0.10.2 (binary-0.8.2.1 should work).

@ghost
Copy link
Author

ghost commented May 15, 2016

@hvr, now fetching bytestring 0.10.8.1.

@ghost
Copy link
Author

ghost commented May 15, 2016

@23Skidoo, can you file a ticket to remove the extra packages once the new hackage-security release can be integrated in bootstrap.sh?

@23Skidoo
Copy link
Member

23Skidoo commented May 17, 2016

I'm probably missing something. The failure on 7.6 and 7.4 is due to us having installed two versions of bytestring, so I'm not sure how using a newer hackage-security will help here. I believe that the way to fix this is to use the bytestring that comes with GHC instead (which means using binary-0.8.2.1 on 7.4 and 7.6).

@ghost
Copy link
Author

ghost commented May 17, 2016

Some of the deps were meant to be intermittent until the new hackage-security package is out, so assuming we have a new hackage-security, I'd prefer to first trim deps and then fix any issues left.

@23Skidoo
Copy link
Member

Which ones? If you're referring to the dependency on cryptohash, it has already been removed in favour of cryptohash-sha256. That happened in hackage-security 0.5.1.0

@ghost
Copy link
Author

ghost commented May 17, 2016

That explains why I haven't been able to find it.

@ghost
Copy link
Author

ghost commented May 17, 2016

I'll modify bootstrap.sh accordingly.

BYTESTRING_VER="0.10.8.1"; BYTESTRING_VER_REGEXP="0\.10\.[2-9]"
# >=0.10.2

case "$GHC_VER" in
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove all mentions of bytestring from the bootstrap script if we add a similar conditional for binary instead. So GHC <= 7.6 will use binary-0.8.2.1 (which depends on bytestring >= 0.9 and therefore should work with the versions of bytestring that come with GHC 7.6 and 7.4) and GHC > 7.6 will use binary-0.8.3.0.

Copy link
Author

Choose a reason for hiding this comment

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

Added a commit, please let me know if that's what you had in mind.

Copy link
Member

@23Skidoo 23Skidoo May 17, 2016

Choose a reason for hiding this comment

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

If you remove this conditional and all mentions of bytestring from the script, I think that just conditionally defining BINARY_VER and BINARY_VER_REGEXP (like you do here for bytestring) will work.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, right, now I got what you're saying.

@23Skidoo
Copy link
Member

And now it fails because tar requires bytestring-builder when compiled against bytesting < 0.10. So we'll also need to conditionally install bytestring-builder depending on the version bytestring.

@ghost
Copy link
Author

ghost commented May 17, 2016

Okay, added another commit.

@23Skidoo
Copy link
Member

Merged: 2645970.

@23Skidoo 23Skidoo closed this May 17, 2016
@23Skidoo
Copy link
Member

@Tuncer Thanks for your patience!

@23Skidoo
Copy link
Member

One other thing we can do is to patch binary to support bytestring-0.9 via bytestring-builder (like tar does, see haskell/tar#14). That'd allow us to always install the latest version of binary on all supported versions of GHC.

@ghost ghost deleted the bootstrap-ci branch May 20, 2016 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants