Simplify CI builds #3084

allanrenucci · 2017-09-07T12:57:48Z

No description provided.

felixmulder

How does the publishing relate to groups? Will all tests run and then the publishing? Or will it be "in the group" in some way?

Otherwise LGTM, and happy to see the new drone functionality get in so we can throw away the matrix build

felixmulder · 2017-09-07T13:06:47Z

project/scripts/sbt

-  SHOULD_RUN=true
+CMD="$1"
+
+if [ -z "$CMD" ]; then


Nitpick, this should be:

if [ -z ${CMD+x} ]; then

otherwise you can pass empty string here and it will accept it as valid input.

My understanding is that it is the other way around:

> FOO=""; if test -z ${FOO+x}; then echo "Empty"; else echo "Full"; fi Full > FOO=""; if test -z "$FOO"; then echo "Empty"; else echo "Full"; fi Empty

Maybe I wasn't clear - you're testing if the variable FOO is empty string - instead of testing if the variable is set at all. E.g. setting FOO="" versus not setting it at all

:)

But seriously though, nitpick 😛

I want the script to exit if the variable is not set or if it is an empty string. An empty cmd should be reject IMO. With -z "$CMD", it rejects both cases.

felixmulder · 2017-09-07T13:07:19Z

project/scripts/sbtPublish

-# PGP Credentials:
-PGP_PW=$4
+# Usage:
+# SONATYPE_USER=<sonatype user> SONATYPE_PW=<sonatype pw> PGP_PW=<pgp pw> ./sbtPublish <publish cmd>


allanrenucci · 2017-09-07T13:13:25Z

Unfortunately there seems to be a major issue. The builds in the same group run in parallel but they share the same workspace... So they fail 😭
Maybe there is a way to not share the same workspace.

felixmulder · 2017-09-07T13:38:13Z

Maybe there is a way to not share the same workspace.

Yeah, this was our original fear with the matrix builds as well - but turns out that the matrix build actually separates things 😂

smarter · 2017-09-07T18:29:41Z

The ivy cache does not seem to be used anymore with this PR, in http://dotty-ci.epfl.ch/lampepfl/dotty/103/1/test everything gets downloaded.

allanrenucci · 2017-09-07T18:33:57Z

@smarter Good catch. I was trying to understand the slow down, this might be the cause

allanrenucci · 2017-09-08T07:12:47Z

project/scripts/sbt

 fi

-if [ $SHOULD_RUN = true ]; then
-  # get the ivy2 cache:
-  ln -s /var/cache/drone/ivy2 "$HOME/.ivy2" || true


@felixmulder What was || true for?

I think this was just there to allow ln to fail, having a cache is not the most important thing to this script. It could definitely be removed, but we then loose the ability to test without /var/cache/drone/ivy2 being present - not a big deal I guess.

I had the ln command run as an initial step before the tests but it was not persisted between the steps and the CI started re-downloading artifacts. So I added it in all the CI scripts

We used to have a common script that was sourced from the other scripts - if there's too much duplication, you could consider doing something similar eventually.

Note that ln also fails if "$HOME/.ivy2" already exists, so I think you might get spurious error messages in the log upon running the script the second time on the same container.

Ah, true! That's exactly why, I remember now!

Does ln survive multiple steps? Because I tried to do it once before all the steps, but then the CI would redownload artifacts: http://dotty-ci.epfl.ch/lampepfl/dotty/102

I will experiment

It doesn't seem so 🤔

It does, however, survive within a single command section, IIRC

allanrenucci · 2017-09-08T07:17:10Z

@felixmulder Can you review again. I managed to solve the issue of shared workspace by running each instance of sbt in its own directory

felixmulder · 2017-09-08T07:21:37Z

project/scripts/genDocs

+echo "Working directory: $PWD"
+
+# get the ivy2 cache
+ln -s /var/cache/drone/ivy2 "$HOME/.ivy2"


Just a thought that popped up - does it make sense to do the release builds without the cache?

cc: @smarter

does it make sense to do the release builds without the cache?

I don't see in what situation that could make a difference?

Corrupt cache causing issues with jars - that's why we chose to have it static rather than using the dynamic caching plugin. Other than that - I've got nothing :)

felixmulder · 2017-09-08T07:27:09Z

.drone.yml

+    group: test
+    image: lampepfl/dotty:2017-09-05
+    commands:
+      - cp -R . /tmp/0/ && cd /tmp/0/


I see you went for the shortest possible name here 😄

allanrenucci · 2017-09-08T08:05:18Z

.drone.yml


  publish_nightly:
    image: lampepfl/dotty:2017-09-05
    environment:
      - NIGHTLYBUILD=yes
    commands:
-      - ./project/scripts/sbt ";clean ;publishLocal" "${CI_PUBLISH}"


publish does not publishLocal anymore. What was the reason for publishing locally here?

cc: @smarter @felixmulder

IIRC something to do with testing the SBT integrations, but I'll defer to @smarter

I don't think publishLocal has been necessary for anything in a long time.

smarter · 2017-09-08T12:27:17Z

project/scripts/sbtPublish

 CMD="     ;set credentials in ThisBuild := Seq(Credentials(\"Sonatype Nexus Repository Manager\", \"oss.sonatype.org\", \"$2\", \"$3\"))"
 CMD="$CMD ;set pgpPassphrase := Some(\"\"\"$4\"\"\".toCharArray)"
 CMD="$CMD ;set pgpSecretRing := file(\"/keys/secring.asc\")"
 CMD="$CMD ;set pgpPublicRing := file(\"/keys/pubring.asc\")"
 CMD="$CMD $RELEASE_CMD"

+# run sbt with the supplied arg
 sbt -J-Xmx4096m \


Would be nice if there was less duplication between sbt and sbtPublish

smarter

Stuff is still being downloaded: http://dotty-ci.epfl.ch/lampepfl/dotty/115/1/test

smarter · 2017-09-08T12:30:04Z

Also probably unrelated to this PR but the CI logs used to be in color, what happened to that?

allanrenucci · 2017-09-08T13:03:47Z

Stuff is still being downloaded

@smarter This happened also before this PR: http://dotty-ci.epfl.ch/lampepfl/dotty/116/3/test
It happens during the bootstrapped tests because when we generated the Dotty Docker image, we didn't compile with bootstrapped Dotty. I can address this in a separate PR

allanrenucci · 2017-09-08T13:07:31Z

Also probably unrelated to this PR but the CI logs used to be in color, what happened to that?

I see colored logs. Maybe an issue with Firefox?

allanrenucci · 2017-09-08T13:20:36Z

project/scripts/genDocs

+sbt -J-Xmx4096m \
+    -J-XX:ReservedCodeCacheSize=512m \
+    -J-XX:MaxMetaspaceSize=1024m \
+    -Ddotty.drone.mem=4096m \


@felixmulder What is this option for?

See https://github.com/lampepfl/dotty/blob/master/project/Build.scala#L627-L632

Is it different than setting -J-Xmx4096m?

Got it. One is for the JVM running sbt and the other for the forked JVM for the tests

allanrenucci · 2017-09-08T14:56:42Z

@smarter I removed the duplication in the scripts. If this looks good to you, I am merging it

OlivierBlanvillain · 2017-09-08T15:09:44Z

project/scripts/sbt

 # run sbt with the supplied arg
 sbt -J-Xmx4096m \
    -J-XX:ReservedCodeCacheSize=512m \
    -J-XX:MaxMetaspaceSize=1024m \
-    -Ddotty.drone.mem=4096m "$CMD"
+    -Ddotty.drone.mem=4096m \


So we still use that thing?

From what I understood, -J-Xmx4096m is an option passed to the VM that run sbt and Ddotty.drone.mem=4096m will be passed to the forked VMs

OlivierBlanvillain · 2017-09-08T15:10:36Z

project/scripts/sbt

 # run sbt with the supplied arg
 sbt -J-Xmx4096m \
    -J-XX:ReservedCodeCacheSize=512m \
    -J-XX:MaxMetaspaceSize=1024m \
-    -Ddotty.drone.mem=4096m "$CMD"
+    -Ddotty.drone.mem=4096m \
+    -ivy /var/cache/drone/ivy2 \


I this doesn't do what you think it does. This way you still download all the dependence needed for sbt itself...

No sure about that. If you look at the CI test logs (except for test_bootstrapped), I don't see any download.

smarter · 2017-09-09T00:47:51Z

Would be good to make sure there is no speed loss by comparing master CI time and this branch CI time when nothing else is running.

This reverts commit f04a1b6.

Nice option that prints the command executed in a script but it leaks secrets...

allanrenucci · 2017-09-09T11:02:20Z

No thorough benchmark but this PR took 12 min to build when nothing else was running. Looking at the history of the builds, build time is rarely under 12min.

This reverts commit c53bcbc.

This reverts commit 851c3c2.

allanrenucci changed the title ~~Drone~~ Simplify CI builds Sep 7, 2017

felixmulder approved these changes Sep 7, 2017

View reviewed changes

allanrenucci force-pushed the drone branch 4 times, most recently from 0f93cae to e68fdde Compare September 7, 2017 15:51

allanrenucci commented Sep 8, 2017

View reviewed changes

felixmulder approved these changes Sep 8, 2017

View reviewed changes

allanrenucci commented Sep 8, 2017

View reviewed changes

allanrenucci force-pushed the drone branch from f15ebf6 to 0eb0898 Compare September 8, 2017 08:27

allanrenucci requested a review from smarter September 8, 2017 08:45

smarter approved these changes Sep 8, 2017

View reviewed changes

smarter reviewed Sep 8, 2017

View reviewed changes

smarter requested changes Sep 8, 2017

View reviewed changes

allanrenucci commented Sep 8, 2017

View reviewed changes

allanrenucci force-pushed the drone branch 2 times, most recently from d012eea to 44ea37e Compare September 8, 2017 14:21

allanrenucci force-pushed the drone branch from 44ea37e to 53ef5de Compare September 8, 2017 14:58

OlivierBlanvillain reviewed Sep 8, 2017

View reviewed changes

smarter approved these changes Sep 9, 2017

View reviewed changes

allanrenucci added 15 commits September 9, 2017 11:51

Simplify CI builds

34e2c8c

Update script documentation

604e17a

Fix typo

d4b3061

Workaround shared workspace

d9346ea

Try to fix CI failures

e40c322

Doc

76b9cc1

Fix caching

1eda903

Test CI duration with sbt tests

05d41e3

Revert "Test CI duration with sbt tests"

ddd56bd

This reverts commit f04a1b6.

Fix filter on sbt tests and slack

a64448c

Documentation

74b181e

Remove set -x option

ca96aeb

Nice option that prints the command executed in a script but it leaks secrets...

Fix typo

cf497b7

Remove code duplication in scripts

63d0254

Update Docker image

df034b5

allanrenucci force-pushed the drone branch from fda0a30 to df034b5 Compare September 9, 2017 09:52

allanrenucci merged commit c53bcbc into scala:master Sep 9, 2017

allanrenucci added a commit that referenced this pull request Sep 9, 2017

Revert "Simplify CI builds (#3084)"

31cfea1

This reverts commit c53bcbc.

allanrenucci mentioned this pull request Sep 9, 2017

Revert "Simplify CI builds" #3092

Merged

allanrenucci added a commit that referenced this pull request Sep 9, 2017

Revert "Simplify CI builds (#3084)" (#3092)

851c3c2

This reverts commit c53bcbc.

allanrenucci added a commit to allanrenucci/dotty that referenced this pull request Sep 9, 2017

Revert "Revert "Simplify CI builds (scala#3084)" (scala#3092)"

dfb31b2

This reverts commit 851c3c2.

allanrenucci deleted the drone branch September 9, 2017 16:36

Simplify CI builds #3084

Simplify CI builds #3084

Uh oh!

Conversation

allanrenucci commented Sep 7, 2017

Uh oh!

felixmulder left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allanrenucci commented Sep 7, 2017

Uh oh!

felixmulder commented Sep 7, 2017

Uh oh!

smarter commented Sep 7, 2017

Uh oh!

allanrenucci commented Sep 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixmulder Sep 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixmulder Sep 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allanrenucci commented Sep 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smarter Sep 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smarter left a comment

Choose a reason for hiding this comment

Uh oh!

smarter commented Sep 8, 2017

Uh oh!

allanrenucci commented Sep 8, 2017

Uh oh!

felixmulder Sep 8, 2017 •

edited

Loading

felixmulder Sep 8, 2017 •

edited

Loading

smarter Sep 8, 2017 •

edited

Loading