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

Fix workflow for building 'OSConfig for MC' on Ubuntu 14.04 #777

Closed
wants to merge 2 commits into from

Conversation

jepio
Copy link
Member

@jepio jepio commented Oct 24, 2024

Description

The workflow for building 'OSConfig for MC' was changed to no longer build CMake v3.21.7 from sources, which led to using the default cmake from the ubuntu 14.04 repository, which is v2.8. OSConfig doesn't allow building with cmake version less than v3.2. Switch to installing the cmake3 package, which provides cmake v3.5, to meet the minimum cmake version requirement, and adjust the 'Build Azure OSConfig' step to account for that cmake version not supporting the --parallel argument.

Checklist

  • I have read the contribution guidelines.
  • I added unit-tests to validate my changes. All unit tests are passing.
  • I have merged the latest main branch prior to this PR submission.
  • I submitted this PR against the main branch.

Our src/CMakeLists.txt specifies this:

  cmake_minimum_required(VERSION 3.2)

The Ubuntu 14.04 package "cmake" install cmake 2.8, so switch to cmake3 which
installs cmake 3.5.

Signed-off-by: Jeremi Piotrowski <jpiotrowski@microsoft.com>
'OSConfig for MC' is built on Ubuntu 14.04 so that the resulting artifact is
portable across supported distros. Ubuntu 14.04 comes with CMake 3.5, which
doesn't support `--parallel`. 'OSConfig for MC' is small so we can afford to
not build in parallel. An alternative is to directly invoke `make -jX`.

Signed-off-by: Jeremi Piotrowski <jpiotrowski@microsoft.com>
@jepio jepio requested a review from a team as a code owner October 24, 2024 13:37
@jepio
Copy link
Member Author

jepio commented Oct 24, 2024

@jepio
Copy link
Member Author

jepio commented Oct 24, 2024

Validated that these changes work in my own fork here: https://github.com/jepio/azure-osconfig/actions/runs/11501723726/job/32015144189

@@ -5,7 +5,7 @@ ARG DEBIAN_FRONTEND=noninteractive
RUN apt-get -y update && apt-get -y install \
gcc \
git \
cmake \
cmake3 \
Copy link
Contributor

@MariusNi MariusNi Oct 24, 2024

Choose a reason for hiding this comment

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

cmake3 \

All understood now. Rather than this, adding cmake 3 by package, we'll try to bring back the build from source.

Copy link
Member Author

Choose a reason for hiding this comment

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

https://github.com/Azure/azure-osconfig/actions/runs/11490120376/job/31980227024 disagrees with what you're saying:

-- The C compiler identification is GNU 4.8.4
-- The CXX compiler identification is GNU 4.8.4
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
CMake Error at CMakeLists.txt:9 (cmake_minimum_required):
  CMake 3.2 or higher is required.  You are running version 2.8.12.2

Copy link
Contributor

Choose a reason for hiding this comment

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

@MariusNi
Copy link
Contributor

MariusNi commented Oct 24, 2024

Validated that these changes work in my own fork here: https://github.com/jepio/azure-osconfig/actions/runs/11501723726/job/32015144189

Thanks but we do not believe this is the right way to address this.

@AhmedBM
Copy link
Contributor

AhmedBM commented Oct 24, 2024

@jepio thanks for the PR. @MariusNi will be bringing back the CMake 3.x for ubuntu 14 from source so these changes should not be needed.

@jepio
Copy link
Member Author

jepio commented Oct 24, 2024

@jepio thanks for the PR. @MariusNi will be bringing back the CMake 3.x for ubuntu 14 from source so these changes should not be needed.

That's fine too 👍
Will close this once a fix is present in main.

@MariusNi
Copy link
Contributor

Thanks for jumping on this while it was night time here, investigating and proposing a fix. Much appreciated! The problem was fixed by re-adding back CMake 3 built from source as we do for the other distros. Luckily that was possible without having to readd other dependencies. This PR can be closed now.

@MariusNi MariusNi closed this Oct 25, 2024
@MariusNi
Copy link
Contributor

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.

3 participants