Skip to content

[ML] More CMake tidyups#2347

Merged
droberts195 merged 7 commits intoelastic:mainfrom
droberts195:cmake_tidyups
Jul 7, 2022
Merged

[ML] More CMake tidyups#2347
droberts195 merged 7 commits intoelastic:mainfrom
droberts195:cmake_tidyups

Conversation

@droberts195
Copy link
Copy Markdown

@droberts195 droberts195 commented Jul 6, 2022

  1. We no longer need references to make as we always use cmake
  2. Use camelCase for variables in build.gradle
  3. Don't require that BOOST_ROOT be an environment variable when
    building natively on Windows
  4. Remove the last occurrence of $ML_KEEP_GOING
  5. Include all variations of CMake build directory in Git/Docker ignores
  6. Docker builds should not hardcode the "Release" config

1. We no longer need references to make as we always use cmake
2. Use camelCase for variables in build.gradle
3. Don't require that BOOST_ROOT be an environment variable when
   building natively on Windows
4. Remove the last occurrence of $ML_KEEP_GOING
commandLine shell
args '-c', "source ./set_env.sh && 3rd_party/dependency_report.sh --csv \"${outputs.files.singleFile}\""
// TODO: this won't work on Windows outside of Git bash
args shellFlag, "${setEnv} && 3rd_party/dependency_report.sh --csv \"${outputs.files.singleFile}\""
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'm about to raise a PR which replaces this bash script with a portable cmake equivalent

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We need to be careful with this - something in release manager calls 3rd_party/dependency_report.sh. Any new script needs to be added in parallel and then the old one only removed after release manager is changed over.

Copy link
Copy Markdown
Contributor

@edsavage edsavage left a comment

Choose a reason for hiding this comment

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

LGTM

@droberts195 droberts195 merged commit 9b8b900 into elastic:main Jul 7, 2022
@droberts195 droberts195 deleted the cmake_tidyups branch July 7, 2022 13:53
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.

2 participants