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

[ML] Add Boost Test to build environment #263

Merged
merged 2 commits into from Oct 18, 2018

Conversation

@droberts195
Copy link
Contributor

commented Oct 18, 2018

Relates #156

This change includes changes similar to those made in #157,
but for the 6.x branch, and also incorporates rebuilds of
VM setup bundles and Docker image updates.

[ML] Add Boost Test to build environment
Relates #156

This change includes changes similar to those made in #157,
but for the 6.x branch, and also incorporates rebuilds of
VM setup bundles and Docker image updates.

set -e

cd `dirname $0`

docker build -t $ACCOUNT/$REPOSITORY:$VERSION linux_image
docker build --no-cache -t $ACCOUNT/$REPOSITORY:$VERSION linux_image

This comment has been minimized.

Copy link
@hendrikmuhs

hendrikmuhs Oct 18, 2018

Contributor

nit: just out of curiosity, would be nice to add a comment why --no-cache

@@ -16,7 +16,7 @@

ACCOUNT=droberts195
REPOSITORY=ml-linux-test
VERSION=1
VERSION=3

This comment has been minimized.

Copy link
@hendrikmuhs

hendrikmuhs Oct 18, 2018

Contributor

If I get this right, with -e you can pass environment variables down:

docker build .... -e VERSION

and use it in dev-tools/docker/linux_tester/Dockerfile instead of requiring changing it there in sync with this.

This comment has been minimized.

Copy link
@droberts195

droberts195 Oct 18, 2018

Author Contributor

I didn't realise you could use a variable in the FROM clause in a Dockerfile, but you're right that for about a year it's been possible: moby/moby#31352

I don't think I should change how everything fits together in this PR though. Any changes to how the different images are linked should be separate.

@hendrikmuhs
Copy link
Contributor

left a comment

LGTM

@droberts195 droberts195 merged commit 58fecc7 into elastic:6.x Oct 18, 2018

3 checks passed

CLA Commit author has signed the CLA
Details
changelog-status changelog does not require modification
elasticsearch-ci Build finished.
Details

@droberts195 droberts195 deleted the droberts195:boost_test_lib_6x branch Oct 18, 2018

droberts195 added a commit to droberts195/ml-cpp that referenced this pull request Oct 18, 2018
[ML] Add Boost Test to build environment
Relates elastic#156

This is basically the other half of elastic#157.  It does the things that
were done for the 6.x branch in elastic#263 that weren't done in elastic#157.

One extra point of confusion is that the macOS Docker image does
not need updating in this PR because it was already rebuilt by elastic#225
using the updated instructions of elastic#157.  So elastic#225 added Boost Test
for macOS for the master branch.
droberts195 added a commit that referenced this pull request Oct 19, 2018
[ML] Add Boost Test to build environment (#264)
Relates #156

This is basically the other half of #157.  It does the things that
were done for the 6.x branch in #263 that weren't done in #157.

One extra point of confusion is that the macOS Docker image does
not need updating in this PR because it was already rebuilt by #225
using the updated instructions of #157.  So #225 added Boost Test
for macOS for the master branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.