Skip to content

Updated CMake PRB#1463

Merged
AlvinMooreSr merged 8 commits into
apple:release-6.1from
AlvinMooreSr:better-ci
Apr 17, 2019
Merged

Updated CMake PRB#1463
AlvinMooreSr merged 8 commits into
apple:release-6.1from
AlvinMooreSr:better-ci

Conversation

@AlvinMooreSr

Copy link
Copy Markdown
Contributor

Updated build images
Added support for running cpack and mounted source directory using very long directory name because of bug with cpack

@mpilman

mpilman commented Apr 15, 2019

Copy link
Copy Markdown
Contributor

There is a Dockerfile and a docker-compose.yml in build/cmake that I wrote. These are based off CentOS 6 which brings several benefits over Ubuntu (mostly the devtoolset-7 packages which properly port new compilers to old systems which solves many of our problems).

I would suggest we simply switch to this new docker image. I would also be willing to maintain this.

@AlvinMooreSr

Copy link
Copy Markdown
Contributor Author

test this please

@AlvinMooreSr

Copy link
Copy Markdown
Contributor Author

@mpilman We are currently considering using those files for 6.2 (master) but will continue to use our existing docker for our current release.

@mpilman

mpilman commented Apr 16, 2019

Copy link
Copy Markdown
Contributor

gotcha - makes sense

@AlvinMooreSr

Copy link
Copy Markdown
Contributor Author

test this please

@AlvinMooreSr

Copy link
Copy Markdown
Contributor Author

test this please

1 similar comment
@AlvinMooreSr

Copy link
Copy Markdown
Contributor Author

test this please

Switched docker image to version including file so that cpack will work
@AlvinMooreSr AlvinMooreSr requested a review from mpilman April 16, 2019 20:02

@mpilman mpilman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM - added two comments but nothing that I think is too important. So you should decide whether you want to change anything.

Comment thread build/docker-compose.yaml Outdated
volumes:
- ..:/foundationdb
working_dir: /foundationdb
- ..:/__this_is_some_very_long_name_dir_that_is_needed_to_fix_a_bug_with_cpack__/foundationdb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

small correction: this is not a bug in cpack but an issue with RPM debug packages. But I don't think it matters too much

Comment thread build/docker-compose.yaml Outdated
snapshot-cmake: &snapshot-cmake
<<: *build-setup
command: bash -c 'if [ -f CMakeLists.txt ]; then mkdir -p "$${BUILD_DIR}" && cd "$${BUILD_DIR}" && cmake .. && make -j "$${MAKEJOBS}"; fi'
command: bash -c 'if [ -f CMakeLists.txt ]; then mkdir -p "$${BUILD_DIR}" && cd "$${BUILD_DIR}" && cmake -DINSTALL_TARGET=RPM -DFDB_RELEASE=0 -DUSE_VALGRIND=0 /__this_is_some_very_long_name_dir_that_is_needed_to_fix_a_bug_with_cpack__/foundationdb && make -j "$${MAKEJOBS}" packages preinstall && cpack; fi'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think these cmake arguments are necessary. I don't know what INSTALL_TARGET does (this is not in the source code). FDB_RELEASE and USE_VALGRING are set to OFF by default - so passing them here doesn't do anything.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Use only to allow others to easily see some of the major build options

@AlvinMooreSr AlvinMooreSr merged commit 799a9d2 into apple:release-6.1 Apr 17, 2019
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