-
Notifications
You must be signed in to change notification settings - Fork 991
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
[develop2] Fixing issues with test_package output folder #12992
[develop2] Fixing issues with test_package output folder #12992
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 your explanation answered most of my questions for this
@@ -51,8 +51,12 @@ def cmake_layout(conanfile, generator=None, src_folder=".", build_folder="build" | |||
|
|||
def get_build_folder_custom_vars(conanfile): | |||
|
|||
build_vars = conanfile.conf.get("tools.cmake.cmake_layout:build_folder_vars", | |||
default=[], check_type=list) | |||
if conanfile.tested_reference_str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After your explanation I quite like this. My only gripe would be that as far as I can see, we have now lost the test_output
bit in the path, so it'd be harder to gitignore these folders now.
May be worth re-adding them before merging (Or if it's still there, please point me to the code that does that as I must have missed it!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a cmake_layout
, the base folder name is already an argument to the function (default "build"
), so that will still be present, is the same default name that must be ignored for local builds (conan install .
) too.
@@ -41,7 +41,7 @@ def cmake_layout(conanfile, generator=None, src_folder=".", build_folder="build" | |||
|
|||
conanfile.cpp.source.includedirs = ["include"] | |||
|
|||
if multi and not user_defined_build: | |||
if multi: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch :)
Changelog: Fix: Fixing issues with test_package output folder.
New proposal to deal with the
test_package
issues:test_output
foldertest_package
can remove thebuild_folder
if it is different than thesource_folder
, and it will remove it before reusingtest_package
defines a new, more complete folder name likegcc10x86release14static
to avoid collisions.TODOs:
build_folder_vars
incmake_layout()
directly