search

conan-io / conan-package-tools

Conan Package Tools. Helps with massive package creation and CI integration (Travis CI, Appveyor...)
MIT License
165 stars 70 forks source link

CONAN_USE_DOCKER is read as an str #573

Closed dvirtz closed 2 years ago

dvirtz commented 3 years ago

Description of Problem, Request, or Question

Code should read CONAN_USE_DOCKER using get_bool_from_env

https://github.com/conan-io/conan-package-tools/blob/7eff1ff01baacbe4ec1056340eea5a49ed3e82a2/cpt/packager.py#L234

Environment Details

Steps to reproduce (Include if Applicable)

run CPT with CONAN_USE_DOCKER being set to 'False'

Build logs (Include if Available)

uilianries commented 2 years ago

Good catch! Thanks for reporting.

uilianries commented 2 years ago

Actually, the current state is correct, it's not a bug. In the past we had 2 env vars, CONAN_USE_DOCKER and CONAN_DOCKER_IMAGE. We needed to pass CONAN_USE_DOCKER=1 and CONAN_DOCKER_IMAGE=conanio/gcc10 (for instance), but it doesn't make sense, because if you pass a docker image name, is because you want Docker. So we kept CONAN_USE_DOCKER, but automatically evaluated by CONAN_DOCKER_IMAGE. Besides that, if you pass a string to CONAN_USE_DOCKER, it will be considered "True" when validating, but by default, it's False. So, the main point is, you don't need CONAN_USE_DOCKER, use only CONAN_DOCKER_IMAGE when you want to run Docker.

dvirtz commented 2 years ago

I don't see how that makes the current behavior correct? The fact is that setting CONAN_USE_DOCKER to any string value has the same effect as setting it to True. I can indeed work around that by unsetting CONAN_DOCKER_IMAGE to disable docker so it's not a blocker me.

uilianries commented 2 years ago

I don't see how that makes the current behavior correct? The fact is that setting CONAN_USE_DOCKER to any string value has the same effect as setting it to True.

Correct, because CONAN_USE_DOCKER is boolean, and by default is False.

I can indeed work around that by unsetting CONAN_DOCKER_IMAGE to disable docker so it's not a blocker me.

You should use CONAN_DOCKER_IMAGE only when you want to use Docker. You don't need to care about CONAN_USE_DOCKER, only CONAN_DOCKER_IMAGE is needed. If don't need Docker, don't set them, just like you are doing now.