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

fix abuse of Conan attributes in fftw #23792

Merged
merged 1 commit into from Apr 29, 2024

Conversation

memsharded
Copy link
Member

Specify library name and version: fftw

This recipe is abusing the build_folder, overwriting it. According to the docs: https://docs.conan.io/2/reference/conanfile.html

Public attributes and methods, like build(), self.package_folder, are reserved for Conan. Don’t use public members for custom fields or methods in the recipes.

The changes proposed seems to both simplify the recipe and avoid this issue.

This was originated in conan-io/conan#16166, as recent changes in develop2 makes the -DCMAKE_TOOLCHAIN_PATH defined in Presets at generation time relative between build-folder and generators-folder, but if build_folder is changed under the hood, things do not longer match.

Close conan-io/conan#16166

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 1 (6caffb8db77c7769061eab22cf45c436005fe61a):

  • fftw/3.3.8:
    All packages built successfully! (All logs)

  • fftw/3.3.10:
    All packages built successfully! (All logs)

  • fftw/3.3.9:
    All packages built successfully! (All logs)


Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 1 (6caffb8db77c7769061eab22cf45c436005fe61a):

  • fftw/3.3.10:
    All packages built successfully! (All logs)

  • fftw/3.3.8:
    All packages built successfully! (All logs)

  • fftw/3.3.9:
    All packages built successfully! (All logs)

Copy link
Contributor

@jcar87 jcar87 left a comment

Choose a reason for hiding this comment

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

LGTM

The resulting contents are the same before and after:

├── conaninfo.txt
├── conanmanifest.txt
├── include
│   ├── fftw3.f
│   ├── fftw3.f03
│   ├── fftw3.h
│   ├── fftw3l.f03
│   └── fftw3q.f03
├── lib
│   ├── libfftw3.a
│   ├── libfftw3f.a
│   └── libfftw3l.a
└── licenses
    └── COPYRIGHT


# Potentially avoid side effects due to build_folder property tweak.
self._current_precision = None
cmake.install()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the proposed approach makes sense: files are added to the install folder (the libraries), and every other file is just overwritten with exactly the same contents. This is equivalent to the way it's done in CMake to support multi-configuration (Release/Debug) in the same install tree: configure twice, build twice, install twice to same location.

Was wondering if it would be worth passing --fresh to CMake configure, or clearing the build folder ahead of starting, but from what I can see, we wouldn't benefit much, as the result is the same.

@conan-center-bot conan-center-bot merged commit 1d39b11 into conan-io:master Apr 29, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] latest conan 2.3.0-git does not install fftw
4 participants