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

Upgrade ammonite to 1.1.2-30-53edc31 #411

Closed
wants to merge 1 commit into from

Conversation

smarter
Copy link
Contributor

@smarter smarter commented Aug 22, 2018

This is mainly to get com-lihaoyi/Ammonite#851 which
should reduce the amount of unnecessary work done by incremental
compilation in the Mill build. This requires some code changes since
this means we now depend on a more recent version of coursier, as
a side-effect this means that we do not depend on scalaz anymore.

Also use the same ammonite version in the Mill build and in
ScalaModule#ammoniteReplClasspath.

@smarter smarter requested a review from lihaoyi August 22, 2018 07:54
@smarter
Copy link
Contributor Author

smarter commented Aug 22, 2018

Looks like the tests are failing because the snapshot is not published for 2.11.8:

not found: https://repo1.maven.org/maven2/com/lihaoyi/ammonite_2.11.8/1.1.2-28-e8b4996/ammonite_2.11.8-1.1.2-28-e8b4996.pom

@lihaoyi Can the snapshot be published for 2.10/2.11 too? Otherwise I'll revert the change to ScalaModule and keep 1.1.2 there.

@smarter smarter changed the title Upgrade ammonite to 1.1.2-28-e8b4996 Upgrade ammonite to 1.1.2-29-2e6570e Aug 22, 2018
@smarter smarter force-pushed the upgrade-ammonite branch 2 times, most recently from 23f6fe4 to 1bb9d6e Compare August 22, 2018 16:22
@smarter smarter changed the title Upgrade ammonite to 1.1.2-29-2e6570e Upgrade ammonite to 1.1.2-30-53edc31 Aug 22, 2018
This is mainly to get com-lihaoyi/Ammonite#851 which
should reduce the amount of unnecessary work done by incremental
compilation in the Mill build. This requires some code changes since
this means we now depend on a more recent version of coursier, as a
side-effect this means that we do not depend on scalaz anymore.

Also use the same ammonite version in the Mill build and in
ScalaModule#ammoniteReplClasspath.

Also remove an incorrect dependency in the caffeine integration test.
This was always wrong but did not start failing until this commit,
probably due to dependencies appearing in a different order on the
classpath.
@smarter
Copy link
Contributor Author

smarter commented Aug 23, 2018

There's one test failure that doesn't seem spurious, I have no idea what it means:

[406/408] dev.upstreamAssembly 
1 targets failed
publishVersion ammonite.ops.ShelloutException: CommandResult 128
fatal: bad object f424a857d8cd4435feb878b7cf9253f84814a93e
    ammonite.ops.Shellout$.executeStream(Shellout.scala:101)
    ammonite.ops.Shellout$.$anonfun$$percent$percent$1(Shellout.scala:15)
    ammonite.ops.Command.applyDynamic(Shellout.scala:118)
    ammonite.$file.build.$anonfun$publishVersion$4(build.sc:504)
    mill.define.ApplyerGenerated.$anonfun$zipMap$2(ApplicativeGenerated.scala:7)
    mill.define.Task$MappedDest.evaluate(Task.scala:348)

any clue @lihaoyi ?

@lihaoyi
Copy link
Member

lihaoyi commented Aug 23, 2018 via email

@smarter
Copy link
Contributor Author

smarter commented Aug 23, 2018

It didn't trigger in #414 which contains the current PR, so I guess not. Should we merge anyway?

@lihaoyi
Copy link
Member

lihaoyi commented Aug 24, 2018

re-triggering the job seems to make it fail reliably, at least the last 2-3 times i tried; probably worth investigating before merge

@lihaoyi
Copy link
Member

lihaoyi commented Aug 24, 2018

or maybe you can just make sure #414 really-truly works and just merge that

@lihaoyi
Copy link
Member

lihaoyi commented Aug 25, 2018

Googling seems to indicate the error message means there's git repository corruption:

As long as #414 doesn't reproduce the same error, I'd be happy to consider this failure some kind of transient and merging 414

@smarter
Copy link
Contributor Author

smarter commented Aug 25, 2018

I've restarted #414 a couple of times and it always came back green, so let's go with that

@smarter smarter closed this Aug 25, 2018
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.

2 participants