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

[develop2] Fixing issues with test_package output folder #12992

Conversation

memsharded
Copy link
Member

@memsharded memsharded commented Jan 27, 2023

Changelog: Fix: Fixing issues with test_package output folder.

New proposal to deal with the test_package issues:

  • It is almost impossible to fix it in 1.X, so giving up, will propose another alternatives below
  • in 2.0, remove the artificial test_output folder
  • test_package can remove the build_folder if it is different than the source_folder, and it will remove it before reusing
  • test_package defines a new, more complete folder name like gcc10x86release14static to avoid collisions.

TODOs:

  • Add a more complete hash to the test_package build_folder name to completely avoid collisions
  • Allow defining more parameters for build_folder_vars in cmake_layout() directly

@memsharded memsharded added this to the 2.0.0-beta9 milestone Jan 29, 2023
@memsharded memsharded marked this pull request as ready for review January 29, 2023 22:50
Copy link
Member

@AbrilRBS AbrilRBS left a 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:
Copy link
Member

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!)

Copy link
Member Author

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:
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch :)

@franramirez688 franramirez688 merged commit 7c7e1c3 into conan-io:develop2 Jan 31, 2023
@memsharded memsharded deleted the feature/develop2_test_package_output_folder branch January 31, 2023 09:29
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