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.

Sign up for GitHub

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

Jump to bottom

autoconf: fix package info on windows #14118

Merged
merged 2 commits into from
Nov 10, 2022
Merged

autoconf: fix package info on windows #14118

merged 2 commits into from
Nov 10, 2022

Conversation

SpaceIm
Copy link
Contributor

@SpaceIm SpaceIm commented Nov 9, 2022

Fix regression on windows introduced in #12896

We must still use legacy unix_path in package_info().
see: conan-io/conan#12499 & conan-io/conan#12192 (comment)

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the conan-center hook activated.

@github-actions github-actions bot mentioned this pull request Nov 9, 2022
Open
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline

All green in build 3 (ae9007de3a618f3eb9cec11f7de94e3d47806be2):

  • autoconf/2.71@:
    All packages built successfully! (All logs)

    🔸 Informative: This recipe is not ready for Conan v2

    We have started the migration process to Conan v2 and exporting recipes successfully will be required in the future.
    This is just an informative note to gain awareness about the process, no need to take any action. The plan is to enforce smaller steps that are easier to fix and, eventually, this conan export step will work.
    See the recipe migration guide to know more about the changes required.

    ERROR: Error loading conanfile at '/home/conan/w/prod_cci_PR-14118/recipes/autoconf/all/conanfile.py': Unable to load conanfile in /home/conan/w/prod_cci_PR-14118/recipes/autoconf/all/conanfile.py
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/conan/w/prod_cci_PR-14118/recipes/autoconf/all/conanfile.py", line 7, in <module>
    from conans import tools as tools_legacy
ImportError: cannot import name 'tools' from 'conans' (/opt/pyenv/versions/3.7.13/lib/python3.7/site-packages/conans/__init__.py)

  • autoconf/2.69@:
    All packages built successfully! (All logs)

    🔸 Informative: This recipe is not ready for Conan v2

    We have started the migration process to Conan v2 and exporting recipes successfully will be required in the future.
    This is just an informative note to gain awareness about the process, no need to take any action. The plan is to enforce smaller steps that are easier to fix and, eventually, this conan export step will work.
    See the recipe migration guide to know more about the changes required.

    ERROR: Error loading conanfile at '/home/conan/w/prod_cci_PR-14118/recipes/autoconf/all/conanfile.py': Unable to load conanfile in /home/conan/w/prod_cci_PR-14118/recipes/autoconf/all/conanfile.py
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/conan/w/prod_cci_PR-14118/recipes/autoconf/all/conanfile.py", line 7, in <module>
    from conans import tools as tools_legacy
ImportError: cannot import name 'tools' from 'conans' (/opt/pyenv/versions/3.7.13/lib/python3.7/site-packages/conans/__init__.py)

jellespijker
jellespijker approved these changes Nov 9, 2022
View reviewed changes
Copy link
Contributor

@jellespijker jellespijker left a comment

Choose a reason for hiding this comment

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

Couple of minor remarks questions, but overall it looks good

recipes/autoconf/all/conanfile.py
autotools = Autotools(self)
autotools.configure()
autotools.make()

def package(self):
autotools = Autotools(self)
autotools.install(args=[f"DESTDIR={unix_path(self, self.package_folder)}"]) # Need to specify the `DESTDIR` as a Unix path, aware of the subsystem
# TODO: can be replaced by autotools.install() if required_conan_version = ">=1.54.0"
autotools.install(args=[f"DESTDIR={unix_path(self, self.package_folder)}"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a FIXME comment once CCI uses the Conan version with your pr conan-io/conan#12193

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's fixed in 1.54.0 so for me the TODO comment has already everything

recipes/autoconf/all/test_package/conanfile.py
test_type = "explicit"
win_bash = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe move this to the build_requirements method for consistency

Copy link
Contributor Author

@SpaceIm SpaceIm Nov 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

no, it doesn't work in test package (conan bug I guess)

recipes/autoconf/all/test_v1_package/conanfile.py


class TestPackageConan(ConanFile):
settings = "os", "compiler", "build_type", "arch"
exports_sources = "../test_package/configure.ac", "../test_package/config.h.in", "../test_package/Makefile.in", "../test_package/test_package_c.c", "../test_package/test_package_cpp.cpp",
settings = "os", "arch", "compiler", "build_type"
test_type = "explicit"

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the win_bash be set as well for test_v1_package ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, in conan v1 it's a different mechanism with win_bash in self.run directly.

@SpaceIm SpaceIm mentioned this pull request Nov 10, 2022
Merged
4 tasks
@conan-center-bot conan-center-bot merged commit fd35b24 into conan-io:master Nov 10, 2022
@SpaceIm SpaceIm deleted the fix/autoconf branch November 10, 2022 20:08
@SpaceIm SpaceIm mentioned this pull request Nov 11, 2022
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jwillikers jwillikers jwillikers approved these changes

@prince-chrismc prince-chrismc prince-chrismc approved these changes

@jellespijker jellespijker jellespijker approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@SpaceIm @conan-center-bot @jellespijker @prince-chrismc @jwillikers