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

SciPy 1.14.0 #277

Merged
merged 12 commits into from
Jun 25, 2024
Merged

SciPy 1.14.0 #277

merged 12 commits into from
Jun 25, 2024

Conversation

h-vetinari
Copy link
Member

No description provided.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari
Copy link
Member Author

@rgommers, any idea about this symbol:

lld-link: error: undefined symbol: scipy_sf_error_get_action

This is failing the windows builds; rest looks fine so far.

@rgommers
Copy link
Contributor

That is a shared library, new in 1.14.0: scipy/scipy#20321. It's the first one, so not surprised there is a hiccup with it. There are some link warnings visible in the logs:

[330/1383] Linking target scipy/special/sf_error_state.dll
lld-link: warning: ignoring unknown argument '--target=x86_64-pc-windows-msvc'

lld-link: warning: ignoring unknown argument '-nostdlib'

lld-link: warning: ignoring unknown argument '-Xclang'

lld-link: warning: ignoring unknown argument '--dependent-lib=msvcrt'

lld-link: warning: ignoring unknown argument '-fuse-ld=lld'

lld-link: warning: ignoring unknown argument '-Wl,-defaultlib:%BUILD_PREFIX%/Library/lib/clang/17/lib/windows/clang_rt.builtins-x86_64.lib'

Why it's failing in this setup I'm not exactly sure about though.

@h-vetinari
Copy link
Member Author

h-vetinari commented May 30, 2024

That is a shared library, new in 1.14.0: scipy/scipy#20321.

In that case it's probably a regular symbol-visibility-in-shared-windows-libraries issue, which needs __declspec(dllimport) / __declspec(dllexport) to work (example), which could look like

// SF_ERR_DLL
// inspired by https://github.com/abseil/abseil-cpp/blob/20240116.2/absl/base/config.h#L736-L753
//
// When building sf_error_state as a DLL, this macro expands to `__declspec(dllexport)`
// so we can annotate symbols appropriately as being exported. When used in
// headers consuming a DLL, this macro expands to `__declspec(dllimport)` so
// that consumers know the symbol is defined inside the DLL. In all other cases,
// the macro expands to nothing.
// Note: SF_ERR_DLL_EXPORTS is set in CMakeLists.txt when building shared sf_error_state
//       SF_ERR_DLL_IMPORTS is set by us as part of the interface for consumers of the DLL
#if defined(SF_ERR_DLL_EXPORTS)
#define SF_ERR_DLL __declspec(dllexport)
#elif defined(SF_ERR_DLL_IMPORTS)
#define SF_ERR_DLL __declspec(dllimport)
#else
#define SF_ERR_DLL
#endif // defined(SF_ERR_DLL_EXPORTS)

plus an equivalent of the following CMake code:

# for building sf_error_state target
if(WIN32 AND MSVC)
  set_target_properties(sf_error_state PROPERTIES DEFINE_SYMBOL "SF_ERR_DLL_EXPORTS")
  if(BUILD_SHARED_LIBS)
    target_compile_definitions(sf_error_state INTERFACE SF_ERR_DLL_IMPORTS)
  endif()
endif()

I have more experience with this than I'd like to, e.g. from long efforts around shared library builds in conda-forge.

The takeaway: Shared libraries with MSVC generally need source-code level annotations. There's one limited escape hatch on CMake (CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS), but even that doesn't work for global data symbols. Unless meson has a magic bullet here, that what we'll need to do IMO.

CC @steppi

@rgommers
Copy link
Contributor

The takeaway: Shared libraries with MSVC generally need source-code level annotations. There's one limited escape hatch on CMake (CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS), but even that doesn't work for global data symbols. Unless meson has a magic bullet here, that what we'll need to do IMO.

Oh yes, fair. We keep being caught out by not having CI coverage for this at all - nothing but MinGW in SciPy CI and wheel builds:(

Could you please move this to a new SciPy issue? This is a blocker for 1.14.0.

@rgommers
Copy link
Contributor

It's not happy with implib: true:

..\scipy\special\meson.build:36:21: ERROR: Got unknown keyword arguments "implib"

Catching up with this topic now, will look at the upstream PR.

@rgommers
Copy link
Contributor

Azure is refusing to show me the logs for the second to last commit here. Would you mind pasting the relevant errors in a comment? Useful anyway for posterity.

@h-vetinari
Copy link
Member Author

Azure is refusing to show me the logs for the second to last commit here. Would you mind pasting the relevant errors in a comment? Useful anyway for posterity.

Yeah, azure deletes the logs of any run that is cancelled by another push. I should have many thought to cancel by hand. In any case, I reverted the last commit, so you'll get a fresh run.

In any case, the failure boiled down to:

[1274/1383] Linking target scipy/special/_special_ufuncs.cp311-win_amd64.pyd
FAILED: scipy/special/_special_ufuncs.cp311-win_amd64.pyd 
"lld-link"  /MACHINE:x64 /OUT:scipy/special/_special_ufuncs.cp311-win_amd64.pyd scipy/special/_special_ufuncs.cp311-win_amd64.pyd.p/_special_ufuncs.cpp.obj scipy/special/_special_ufuncs.cp311-win_amd64.pyd.p/_special_ufuncs_docs.cpp.obj scipy/special/_special_ufuncs.cp311-win_amd64.pyd.p/sf_error.cc.obj "/nologo" "/OPT:REF" "/DLL" "/IMPLIB:scipy\special\_special_ufuncs.cp311-win_amd64.lib" "--target=x86_64-pc-windows-msvc" "-nostdlib" "-Xclang" "--dependent-lib=msvcrt" "-fuse-ld=lld" "-Wl,-defaultlib:%BUILD_PREFIX%/Library/lib/clang/17/lib/windows/clang_rt.builtins-x86_64.lib" "scipy/special/sf_error_state.lib" "%PREFIX%\libs\python311.lib" "kernel32.lib" "user32.lib" "gdi32.lib" "winspool.lib" "shell32.lib" "ole32.lib" "oleaut32.lib" "uuid.lib" "comdlg32.lib" "advapi32.lib"
lld-link: error: undefined symbol: scipy_sf_error_get_action
>>> referenced by scipy/special/_special_ufuncs.cp311-win_amd64.pyd.p/sf_error.cc.obj:(void __cdecl sf_error_v(char const *, enum sf_error_t, char const *, char *))

@h-vetinari
Copy link
Member Author

I did some more digging in the upstream PR, but not sure what's happening with the import lib.

@h-vetinari h-vetinari changed the title SciPy 1.14.0rc1 SciPy 1.14.0rc2 Jun 16, 2024
@h-vetinari h-vetinari mentioned this pull request Jun 17, 2024
@h-vetinari h-vetinari changed the title SciPy 1.14.0rc2 SciPy 1.14.0 Jun 24, 2024
@h-vetinari h-vetinari added automerge Merge the PR when CI passes and removed automerge Merge the PR when CI passes labels Jun 24, 2024
@h-vetinari h-vetinari added the automerge Merge the PR when CI passes label Jun 24, 2024
@h-vetinari h-vetinari merged commit 40d7f9e into conda-forge:main Jun 25, 2024
17 of 20 checks passed
@h-vetinari h-vetinari deleted the bump branch June 25, 2024 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge the PR when CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants