Skip to content
This repository was archived by the owner on Nov 30, 2023. It is now read-only.

Add vcpkg context from base cpp image to the cpp-mariadb container #1322

Merged
merged 6 commits into from
Feb 24, 2022

Conversation

dnastrain
Copy link
Contributor

@dnastrain dnastrain commented Feb 23, 2022

#781 updates for cpp-mariadb container

Description


This PR includes:

  • Updated Dockerfile and test.sh with Vcpkg changes taken from the containers/cpp copy
  • Added reinstall-cmake.sh file, taken verbatim from containers/cpp
  • Updated README.md documentation for Vcpkg, including a couple tweaks when compared to the containers/cpp version, such as refer to docker-compose.yml rather than devcontainer.json for REINSTALL_CMAKE_VERSION_FROM_SOURCE sample
  • Also includes 1 test update for the base container/cpp/test-project/test.sh

PR Checklist


Use the check-list below to ensure your branch is ready for PR. If the item is not applicable, leave it blank.

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My code follows the code style of this project.
  • I ran the lint checks which produced no new errors nor warnings for my changes.
  • I have checked to ensure there aren't other open Pull Requests for the same update/change.

Does this introduce a breaking change?


  • Yes
  • No

If this introduces a breaking change, please describe the impact and migration path for existing applications below.

Testing


  • This container's test-project/test.sh updated to ensure Vcpkg is configured same as containers/cpp
  • A dev cpp base-image was used for validate the changes, i.e. FROM mcr.microsoft.com/vscode/devcontainers/cpp:dev-debian-11 used in the Dockerfile, since the other images have not published (yet)

Other information or known dependencies


NOTE: Due to mariadb-cpp-connector versions currently supported, this container does not currently support arm64 -- README.md updated accordingly.

@dnastrain
Copy link
Contributor Author

hi @Chuxel , this PR is ready for review. cc: @bderusha

@dnastrain dnastrain requested a review from Chuxel February 23, 2022 23:40
Copy link
Member

@Chuxel Chuxel left a comment

Choose a reason for hiding this comment

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

The execute bits were not set on the scripts, so they failed to build, but I pushed an update and things worked just fine!

LMK if you want to do any further review before merge!

@dnastrain
Copy link
Contributor Author

dnastrain commented Feb 24, 2022

The execute bits were not set on the scripts, so they failed to build, but I pushed an update and things worked just fine!

LMK if you want to do any further review before merge!

Thx @Chuxel! -- If that 'fixed it', then is ok to merge imho, especially since I think adding arm64 support is a "separate concern"

@Chuxel Chuxel merged commit 45d6be1 into microsoft:main Feb 24, 2022
@dnastrain dnastrain deleted the dnastrain/cpp-mariadb-vcpkg branch March 7, 2022 15:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants