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

Add openfhe-julia #7879

Merged
merged 6 commits into from
Dec 31, 2023
Merged

Add openfhe-julia #7879

merged 6 commits into from
Dec 31, 2023

Conversation

sloede
Copy link
Contributor

@sloede sloede commented Dec 31, 2023

No description provided.

@sloede
Copy link
Contributor Author

sloede commented Dec 31, 2023

@giordano Have you seen this error from the Windows build before:

[04:06:28]   The imported target "OPENFHEcore" references the file
[04:06:28] 
[04:06:28]      "/workspace/destdir/lib/libOPENFHEcore.dll"
[04:06:28] 
[04:06:28]   but this file does not exist.

When checking out the contents of autogenerated JLL tarballs for OpenFHE_jll (e.g., OpenFHE.v1.1.2.x86_64-w64-mingw32-cxx11.tar.gz from https://github.com/JuliaBinaryWrappers/OpenFHE_jll.jl/releases/tag/OpenFHE-v1.1.2%2B0), it seems like that indeed, all files in the lib directory have a .dll.a extension, while all the actual .dll files are in the bin directory.

Do you know what I can to to make the correct OpenFHE libraries available to downstream packages - do I need to modify the OpenFHE build_tarballs.jl or is it something I can/need to fix here, e.g., by modifying my CMake setup?

@sloede
Copy link
Contributor Author

sloede commented Dec 31, 2023

Hm, looking at the file OpenFHETargets-release.cmake in the JLL product, I find the following lines:

# Import target "OPENFHEcore" for configuration "Release"
set_property(TARGET OPENFHEcore APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
set_target_properties(OPENFHEcore PROPERTIES
  IMPORTED_IMPLIB_RELEASE "${_IMPORT_PREFIX}/lib/libOPENFHEcore.dll.a"
  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/lib/libOPENFHEcore.dll"
  )

list(APPEND _IMPORT_CHECK_TARGETS OPENFHEcore )
list(APPEND _IMPORT_CHECK_FILES_FOR_OPENFHEcore "${_IMPORT_PREFIX}/lib/libOPENFHEcore.dll.a" "${_IMPORT_PREFIX}/lib/libOPENFHEcore.dll" )

Thus it seems clear to me that the IMPORTED_LOCATION_RELEASE path should refer to the bin directory and so should the path to the dll in _IMPORT_CHECK_FILES_FOR_OPENFHEcore.

To me, this strongly hints at a problem in the CMake files provided by OpenFHE_jll.jl. And indeed, fixing this manually in an interactive session of BB, the openfhe-julia build works. Do you, by any chance, know what could be wrong on the OpenFHE CMake setup side and what to do about it?

@sloede
Copy link
Contributor Author

sloede commented Dec 31, 2023

Ah, the BB auditor is at "fault" here:

<snip>
┌ Warning: lib/libOPENFHEbinfhe.dll should be in `bin`!
└ @ BinaryBuilder.Auditor ~/.julia/packages/BinaryBuilder/0CUml/src/Auditor.jl:229
┌ Warning: lib/libOPENFHEcore.dll should be in `bin`!
└ @ BinaryBuilder.Auditor ~/.julia/packages/BinaryBuilder/0CUml/src/Auditor.jl:229
┌ Warning: lib/libOPENFHEpke.dll should be in `bin`!
└ @ BinaryBuilder.Auditor ~/.julia/packages/BinaryBuilder/0CUml/src/Auditor.jl:229
┌ Warning: Simple buildsystem detected; Moving all `.dll` files to `bin`!
└ @ BinaryBuilder.Auditor ~/.julia/packages/BinaryBuilder/0CUml/src/Auditor.jl:240

is there a way I can prevent this? Or should I even prevent this and instead manually modify the OpenFHETargets-release.cmake to reflect the changed paths?

@sloede
Copy link
Contributor Author

sloede commented Dec 31, 2023

#7880 attempts to hotfix the underlying reason for the error, let's hold off merging this PR until then.

@sloede sloede marked this pull request as draft December 31, 2023 09:04
@sloede sloede closed this Dec 31, 2023
@sloede sloede reopened this Dec 31, 2023
@sloede sloede closed this Dec 31, 2023
@sloede sloede reopened this Dec 31, 2023
@sloede sloede closed this Dec 31, 2023
@sloede sloede reopened this Dec 31, 2023
@sloede sloede marked this pull request as ready for review December 31, 2023 14:52
@sloede
Copy link
Contributor Author

sloede commented Dec 31, 2023

@giordano ready to merge now 😊

@giordano
Copy link
Member

I think you want to follow

# These are the platforms we will build for by default, unless further
# platforms are passed in on the command line
include("../../L/libjulia/common.jl")
platforms = vcat(libjulia_platforms.(julia_versions)...)
filter!(!Sys.iswindows, platforms) # Singular does not support Windows
platforms = expand_cxxstring_abis(platforms)
BuildDependency(PackageSpec(;name="libjulia_jll", version=v"1.10.8")),
so that you target all julia versions (libjulia breaks API/ABI across minor versions)

@sloede
Copy link
Contributor Author

sloede commented Dec 31, 2023

I think you want to follow
[...]
so that you target all julia versions (libjulia breaks API/ABI across minor versions)

Thanks a lot for the hint (and the suggestion), that is incredibly helpful!

@giordano giordano merged commit ab0c062 into JuliaPackaging:master Dec 31, 2023
@sloede sloede deleted the msl/openfhe-julia branch December 31, 2023 16:16
@sloede
Copy link
Contributor Author

sloede commented Jan 2, 2024

@giordano Do you have an idea why the binaries generated for, e.g., Julia v1.8 seem to work well (GHA log), while for v1.9 and v1.10 they fail with

ERROR: LoadError: InitError: could not load library "C:\Users\runneradmin\.julia\artifacts\dc58129f8d04ad71389779de34cc61fcb0618967\bin\libopenfhe_julia.dll"
%1 is not a valid Win32 application.

(GHA log)? I assume it has to do something with the libjulia_jll dependency, but I don't even know how to start debugging this 😢

MichelJuillard pushed a commit to MichelJuillard/Yggdrasil that referenced this pull request Jan 8, 2024
* Add openfhe-julia

* Rename dir to match product name (?)

* Build for all Julia versions

* Remove Julia v1.6 build

* Update O/openfhe_julia/build_tarballs.jl

Co-authored-by: Mosè Giordano <[email protected]>

* Remove manual removal of Julia v1.6

---------

Co-authored-by: Mosè Giordano <[email protected]>
grasph pushed a commit to Moelf/Yggdrasil that referenced this pull request Jul 1, 2024
* Add openfhe-julia

* Rename dir to match product name (?)

* Build for all Julia versions

* Remove Julia v1.6 build

* Update O/openfhe_julia/build_tarballs.jl

Co-authored-by: Mosè Giordano <[email protected]>

* Remove manual removal of Julia v1.6

---------

Co-authored-by: Mosè Giordano <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants