-
Notifications
You must be signed in to change notification settings - Fork 988
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
[bug] conf_info 'tools.microsoft.bash:subsystem' from msys2 is not used by Autotools in test_package #11975
Comments
Fails on test_package see conan-io/conan#11975
test_package will currently fail due to conan-io/conan#11975
Needed to add a bogus generator for some reason. test_package will currently fail due to conan-io/conan#11975
Note: test_package can't use the new Autools generator due to conan-io/conan#11975
No need to actually compile something with this tool we simply need to know if it runs. The win_bash for the run needs to be set explicitly due to the conf not working properly in the test_package see conan-io/conan#11975
Use modern Autotools toolchain and add buildenv_info Simplified test_package No need to actually compile something with this tool we simply need to know if it runs. The win_bash for the run in the test_package needs to be set explicitly due to the conf not working properly in the test_package see conan-io/conan#11975
@memsharded how hard would it to fix or workaround this issue since this is currently blocking me for updating some conan recipes according to CCI standard. If you estimate that it isn't that difficult and relatively contained in the codebase, I might give a crack at this during my weekend. Since I have reached a point in the current sprint in which I need to focus on fixing these recipes "good enough" for our Ultimaker Cura use: See conan-io/conan-center-index#12777 |
This allows us to use the msys2 bash without using the conf_info in the test_package. Which is currently problematic due to: conan-io/conan#11975 Contributes to CURA-9574
Still an issue: conan-io/conan#11975 Contribute to CURA-8831
@memsharded and/or @lasote is it by design that the |
I'm taking a look. |
I detected why it happens, I think this is not on purpose but I need to discuss with the team the issue because it is not easy to fix. |
Note for the team to discuss: It looks like during the test_package process, first it is called try:
install_folder = deps_install(app=app,
create_reference=reference,
ref_or_path=conanfile_abs_path,
install_folder=test_build_folder,
base_folder=test_build_folder,
remotes=remotes,
graph_info=graph_info,
update=update,
build_modes=build_modes,
manifest_folder=manifest_folder,
manifest_verify=manifest_verify,
manifest_interactive=manifest_interactive,
keep_build=keep_build,
recorder=recorder,
require_overrides=require_overrides,
conanfile_path=os.path.dirname(conanfile_abs_path),
test=True # To keep legacy test_package_layout
)
cmd_build(app, conanfile_abs_path, test_build_folder,
source_folder=base_folder, build_folder=test_build_folder,
package_folder=os.path.join(test_build_folder, "package"),
install_folder=install_folder, test=reference) |
I noticed the complexity of this while stepping through the code with the debugger. I think a fix for this will need to follow the regular workflow/planning in your sprints. So it is also save to assume a fix won't be in anytime soon. But is there a policy to be applied for the open PR's in CCI which can't be tested on Windows if they use the bash from the Would the below code be acceptable until the issue is fixed? def build(self):
if self._settings_build.os == "Windows" and not self.conf.get("tools.microsoft.bash:path", default=False, check_type=bool):
return # autoconf needs a bash if there isn't a bash no need to build and def test(self):
if self._settings_build.os == "Windows" and not self.conf.get("tools.microsoft.bash:path", default=False, check_type=bool):
return # autoconf needs a bash if there isn't a bash no need to build |
That is something I cannot answer. I would say skipping the test won't be accepted by the community. |
@nallath and @casperlamboo FYI, see the answer above. I think we need to host the Autotool based recipes, which we use in our dependency chain on our own Artifactory for a limited period. |
Solved in #12095 for 1.53 |
* autoconf v2 ready Note: test_package can't use the new Autools generator due to conan-io/conan#11975 * Revert removal of PATH from env_info From review comment * use unix_path instead of private import functions According review comment * use unix_path instead of private import functions According review comment * pass self to unix_path * applied review comments KB-H013 states that everything needs to be installed under bin for tools. This also means that the `PATH` for the actual bin location `bin/bin` needs to be added to the `build_env` * Add licenses to package * Explicitly use an empty includedirs Should fix KB-H071 * Fix setting unix_path for package_info subsystem isn't known at this time * simplified test_package No need to actually compile something with this tool we simply need to know if it runs. The win_bash for the run needs to be set explicitly due to the conf not working properly in the test_package see conan-io/conan#11975 * Revert "simplified test_package" This reverts commit d9ee4de. * use modern tools in test_package * add conan test_v1_package * m4 is a build requirement * Ducktype Autotools `configure` and `make` This allows us to use the msys2 bash without using the conf_info in the test_package. Which is currently problematic due to: conan-io/conan#11975 Contributes to CURA-9574 * Fix paths on windows in test method Contributes to CURA-8831 * Don't ducktype but inherit The ducktyping seems to affect the usage of the Autotools in the main recipe as well Contributes to CURA-8831 * use unix_path for env_info Contributes to CURA-8831 * remove extra lin Contributes to CURA-8831 * don't use unix_path The subsystem isn't known when consuming projects request the info Contributes to CURA-8831 * fix subsystems on Windows Using some duck typing and inheritance to work around GH issue conan-io/conan#11980 Contributes to CURA-9595 * updated autoconf for conan v2 ready support Contribute to CURA-8831 * autoconf bin contains shell scripts not executable Contribute to CURA-8831 * Removed Autotools workaround from test_package Still an issue: conan-io/conan#11975 Contribute to CURA-8831 * m4 is a run requirement not a tool_req Contribute to CURA-8831 * use mingw32 for gnu tuple version 2.69 doesn't known mingw64, it should hold no difference for the end result. Contribute to CURA-8831 * Run the configure command in the build_folder Contributes to CURA-8831 * Use conanbuild env as scope for testing Contribute to CURA-8831 * Ensure that M4 binary from dependency is used Contributes to CURA-8831 * Set win_bash in configure stage Contributes to CURA-8831 * Move conf keys to user space Applies code-review comments * Use *_folder in favor of undocumented *_path Applies code-review comments * Reuse source files from test_package Applies code-review comments * Apply suggestions from code review Co-authored-by: Jordan Williams <jordan@jwillikers.com> * Use export_conandata_patches Bump up the minimum conan version to 1.52 Applies code-review comments * Removed unused conf_info values As descibed in #13490 Applies code-review comments * Check against empty string for conf value bash path Apply code review comment * Update recipes/autoconf/all/conanfile.py Co-authored-by: Chris Mc <prince.chrismc@gmail.com> * Apply suggestions from code review Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com> Co-authored-by: j.spijker@ultimaker.com <jelle spijker> Co-authored-by: Jordan Williams <jordan@jwillikers.com> Co-authored-by: Chris Mc <prince.chrismc@gmail.com> Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
Environment Details (include every applicable attribute)
Steps to reproduce (Include if Applicable)
Trying to modernize the autoconf recipe of the conan-center-index I get the error
while a log output shows that the conf is set and available:
See Ultimaker/conan-center-index@d33196a for the full change
conanfile.py
test_package conanfile.py
Logs (Executed commands with output) (Include/Attach if Applicable)
Log file
The text was updated successfully, but these errors were encountered: