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

[ci] remove unnecessary package installations in CI #6488

Merged
merged 8 commits into from
Jun 17, 2024

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Jun 17, 2024

Reduces the set of packages installed in CI jobs.

.ci/setup.sh is responsible for installing some system packages that are common across most CI jobs. Some of the things installed there have been carried around in LightGBM's CI scripts for years, since back when the project used Travis (#3672, #1669).

Some of them are no longer necessary. For example, tools like netcat and iputils-ping provide some useful tools for debugging networking issues, but those tools aren't invoked in this project's CI scripts.

Benefits of these changes:

  • faster builds
  • reduced risk of build failures due to installation problems

@jameslamb jameslamb changed the title WIP: [ci] simplify CI setup WIP: [ci] remove unnecessary package installations in CI Jun 17, 2024
@jameslamb jameslamb changed the title WIP: [ci] remove unnecessary package installations in CI [ci] remove unnecessary package installations in CI Jun 17, 2024
sudo apt-get install --no-install-recommends -y libomp-17-dev
sudo apt-get install -y \
clang-17 \
libomp-17-dev
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With a --no-install-recommends added here, the C++ tests fail:

[ 70%] Building CXX object CMakeFiles/lightgbm_objs.dir/src/treelearner/serial_tree_learner.cpp.o
In file included from /__w/1/s/src/treelearner/linear_tree_learner.cpp:7:
In file included from /__w/1/s/external_libs/eigen/Eigen/Dense:1:
In file included from /__w/1/s/external_libs/eigen/Eigen/Core:22:
In file included from /__w/1/s/external_libs/eigen/Eigen/src/Core/util/ConfigureVectorization.h:346:
In file included from /usr/lib/llvm-17/lib/clang/17/include/emmintrin.h:17:
In file included from /usr/lib/llvm-17/lib/clang/17/include/xmmintrin.h:31:
/usr/lib/llvm-17/lib/clang/17/include/mm_malloc.h:32:40: error: 'alloc_align' attribute parameter 1 is out of bounds
   32 |                                        __alloc_align__(2)))

https://dev.azure.com/lightgbm-ci/lightgbm-ci/_build/results?buildId=16401&view=results

I'm unsure why. For now, just letting apt-get install the other recommended packages seems to resolve that, so I think we should continue doing that.

@jameslamb jameslamb marked this pull request as ready for review June 17, 2024 18:12
if [[ $TASK != "mpi" ]]; then
brew install gcc
fi
brew install gcc
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe that != "mpi" was just a shorter way to represent some other subset of jobs and that this has been carried forward unnecessarily from older versions of these build scripts.

Proposing removing this condition to simplify the scripts a bit... if the job has COMPILER=gcc set, we need to install gcc.

@jameslamb
Copy link
Collaborator Author

Thanks @borchero ! I'm going to keep trying to simplify these builds. There is a lot of stuff that's built up over the years (much of it written by me 😅 ) that's worth re-examining.

@jameslamb jameslamb merged commit 7d33879 into master Jun 17, 2024
41 checks passed
@jameslamb jameslamb deleted the ci/simplify-setup branch June 17, 2024 20:04
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.

None yet

2 participants