cpp-best-practices / gui_starter_template

A template CMake project to get you started with C++ and tooling

The Unlicense

2.5k stars 447 forks source link

Test using setup-python directly for common packages #190

Closed lefticus closed 2 years ago

lefticus commented 2 years ago

gcovr, conan, ninja, cmake are all fully supported with pip install on all 3 platforms
using setup-python and pip3 directly removes one variable from the build process
using pip3 directly to install all 3 provides consistency across platforms

codecov-commenter commented 2 years ago

Codecov Report

Merging #190 (b1f45dc) into main (26a39b1) will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##              main      #190   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines           20        20           
=========================================
  Hits            20        20

Flag	Coverage Δ
Linux	`∅ <ø> (∅)`
Windows	`100.00% <ø> (ø)`
macOS	`∅ <ø> (∅)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 26a39b1...b1f45dc. Read the comment docs.

lefticus commented 2 years ago

This partially a workaround for https://github.com/aminya/setup-cpp/issues/27 but also I think it is not a bad idea, as mentioned in the commit message, it gives consistency across all platforms for the versions and mechanism that common tools are installed.

ddalcino commented 2 years ago

Just for clarification: Is this meant to be permanent, or just a temporary fix until https://github.com/aminya/setup-cpp/issues/27 is fixed?

lefticus commented 2 years ago

Just for clarification: Is this meant to be permanent, or just a temporary fix until aminya/setup-cpp#27 is fixed?

In my opinion, it would be permanent. I see it as one less potential issue while maintaining this project, and for our users in the future. I don't see any reason to use a mix of chocolatey, package managers, etc, when python gives us the same versions of the same packages across all platforms.

lefticus commented 2 years ago

It is not setup-cpp's fault that the pre-installed Python broke on Windows images. In fact setup-cpp does not call "setup-python" until it actually needed. I do not approve of this, as this is like discarding all the things it was considered in setup-cpp and trying to re-implement them in bash.

It is not discarding everything setup-cpp is used for. Setup-cpp still does all of the hard work of finding and installing compilers across compilers.

aminya commented 2 years ago

It is not setup-cpp's fault that the pre-installed Python broke on Windows images. In fact setup-cpp does not call "setup-python" until it actually needed. I do not approve of this, as this is like discarding all the things it was considered in setup-cpp and trying to re-implement them in bash.

It is not discarding everything setup-cpp is used for. Setup-cpp still does all of the hard work of finding and installing compilers across compilers.

Here I specifically meant the pip installations. As I mentioned in the above comments, it is needed to check if:

python paths are added to PATH on all platforms
if pip is installed and is up to date

lefticus commented 2 years ago

I'm just going to close and remove this PR, since setup-cpp's python integration is working again.

aminya commented 2 years ago

Worth mentioning that the pip issue is a serious problem in their launchers. It is affecting many people on Windows. See the progress here: https://github.com/pypa/pip/issues/10875