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

[fftw3] Fix the build error and usage of feature openmp #35483

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

[fftw3] Fix the build error and usage of feature openmp #35483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
317e2dd
[fftw3] Fix the build error and usage of feature openmp
LilyWangLL Dec 4, 2023
2818e9c
update version
LilyWangLL Dec 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions ports/fftw3/fix-openmp.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
diff --git a/CMakeLists.txt b/CMakeLists.txt
index ce438a3..43c74be 100644
index 18f8460..065838c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -356,7 +356,7 @@ if (OPENMP_FOUND)
@@ -351,12 +351,12 @@ if (Threads_FOUND)
endif ()

if (OPENMP_FOUND)
- add_library (${fftw3_lib}_omp ${fftw_omp_SOURCE})
+ add_library (${fftw3_lib}_omp ${SOURCEFILES} ${fftw_omp_SOURCE})
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure, but this seems incorrect to me because it's going to duplicate builds of all those other files, and:

add_library (${fftw3_lib} ${SOURCEFILES})
# ...
target_link_libraries (${fftw3_lib}_omp ${fftw3_lib})

means symbols declared by those files should already be available to ${fftw3_lib}_omp.

I observe that fftw3 spams these functions out with 'X macros' ; is it possible the actual problem is that something else disagrees between the build of fftw3_lib and fftw3_lib_omp?

Copy link
Member

Choose a reason for hiding this comment

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

Oh wait I think I see the problem now. ${fftw3_lib} is being built as a DLL that doesn't correctly export these. I'm still not sure repeating their definitions in the DLL is the correct outcome...

Copy link
Member

Choose a reason for hiding this comment

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

Has this been reported upstream? Worst case the feature could be marked with "supports": "!windows | static".

Block Windows and dynamic is: !(windows & !static).
Demorgan to get rid of parens: !windows | static.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I submitted an upstream issue: FFTW/fftw3#342, waiting for the upstream's response.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is always difficult to report vcpkg issues to upstream unless fully understanding the modifcations which are already in vcpkg.

Copy link
Contributor

Choose a reason for hiding this comment

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

And upstream has very little activity. Still waiting for FFTW/fftw3#331.

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 is true that the upstream response is slow. If there is no response, I will add supports for this feature next week.

target_include_directories (${fftw3_lib}_omp INTERFACE $<INSTALL_INTERFACE:include>)
target_link_libraries (${fftw3_lib}_omp ${fftw3_lib})
target_link_libraries (${fftw3_lib}_omp ${CMAKE_THREAD_LIBS_INIT})
list (APPEND subtargets ${fftw3_lib}_omp)
Expand Down
2 changes: 1 addition & 1 deletion ports/fftw3/install-subtargets.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
diff --git a/CMakeLists.txt b/CMakeLists.txt
index d1e4dff..ea5d579 100644
index 065838c..1e4c2b3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -361,12 +361,8 @@ endif ()
Expand Down
18 changes: 18 additions & 0 deletions ports/fftw3/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,22 @@ file(WRITE "${SOURCE_PATH}/include/fftw3.h" "${_contents}")
file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/include")
file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/share")

if("openmp" IN_LIST FEATURES)
vcpkg_replace_string("${CURRENT_PACKAGES_DIR}/share/${PORT}/FFTW3Config.cmake"
"# defined since 2.8.3"
[[# defined since 2.8.3
include(CMakeFindDependencyMacro)
find_dependency(OpenMP)]])
vcpkg_replace_string("${CURRENT_PACKAGES_DIR}/share/${PORT}f/FFTW3fConfig.cmake"
"# defined since 2.8.3"
[[# defined since 2.8.3
include(CMakeFindDependencyMacro)
find_dependency(OpenMP)]])
vcpkg_replace_string("${CURRENT_PACKAGES_DIR}/share/${PORT}l/FFTW3lConfig.cmake"
"# defined since 2.8.3"
[[# defined since 2.8.3
include(CMakeFindDependencyMacro)
find_dependency(OpenMP)]])
endif()

vcpkg_install_copyright(FILE_LIST "${SOURCE_PATH}/COPYING")
2 changes: 1 addition & 1 deletion ports/fftw3/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "fftw3",
"version": "3.3.10",
"port-version": 8,
"port-version": 9,
"description": "FFTW is a C subroutine library for computing the discrete Fourier transform (DFT) in one or more dimensions, of arbitrary input size, and of both real and complex data (as well as of even/odd data, i.e. the discrete cosine/sine transforms or DCT/DST).",
"homepage": "https://www.fftw.org/",
"license": "GPL-2.0-or-later",
Expand Down
2 changes: 1 addition & 1 deletion versions/baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2610,7 +2610,7 @@
},
"fftw3": {
"baseline": "3.3.10",
"port-version": 8
"port-version": 9
},
"fftwpp": {
"baseline": "2019-12-19",
Expand Down
5 changes: 5 additions & 0 deletions versions/f-/fftw3.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{
"versions": [
{
"git-tree": "fa91b2e346c356e2e420e8ce98d4dd5bde652b2c",
"version": "3.3.10",
"port-version": 9
},
{
"git-tree": "824a4cda47df1a63c0b13f2a603e7d0fb0dac900",
"version": "3.3.10",
Expand Down