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

conda-build: Add 'windows-latest' to conda build workflow #449

Closed
wants to merge 45 commits into from

Conversation

jjerphan
Copy link
Collaborator

@jjerphan jjerphan commented Jun 1, 2023

Reference Issues/PRs

Follow-up of #318.
Towards #190.

What does this implement/fix? Explain your changes.

Add Windows to the conda build workflow.

Any other comments?

It is willingly breaking other configurations on the CI to find one that works for Windows on conda-forge.

@jjerphan jjerphan changed the title github: Add 'windows-latest' to conda build workflow conda-build: Add 'windows-latest' to conda build workflow Jun 2, 2023
Signed-off-by: Julien Jerphanion <[email protected]>
Signed-off-by: Julien Jerphanion <[email protected]>
Signed-off-by: Julien Jerphanion <[email protected]>
Signed-off-by: Julien Jerphanion <[email protected]>
Signed-off-by: Julien Jerphanion <[email protected]>
Signed-off-by: Julien Jerphanion <[email protected]>
Signed-off-by: Julien Jerphanion <[email protected]>
@jjerphan jjerphan force-pushed the conda-build/add-windows-latest branch from c17d9b8 to 9fdbc4a Compare June 5, 2023 09:21
Signed-off-by: Julien Jerphanion <[email protected]>
Signed-off-by: Julien Jerphanion <[email protected]>
@jjerphan jjerphan force-pushed the conda-build/add-windows-latest branch from 10b76c3 to 2509a50 Compare June 5, 2023 09:58
Signed-off-by: Julien Jerphanion <[email protected]>
lz4 is currently not resolved on Windows when building for conda-forge.

This improves the resolution of LZ4, making it available for this
configuration.

Signed-off-by: Julien Jerphanion <[email protected]>
@jjerphan jjerphan force-pushed the conda-build/add-windows-latest branch from 747f99f to 89234b2 Compare June 5, 2023 12:23
Yes.

Signed-off-by: Julien Jerphanion <[email protected]>
@jjerphan jjerphan force-pushed the conda-build/add-windows-latest branch from 89234b2 to fdc41ec Compare June 5, 2023 13:07
This should remove the following error met with MSVC 14.29.30133:

```
  D:\a\ArcticDB\ArcticDB\cpp\arcticdb/storage/storages.hpp(75): error
  C7510: 'KeyNotFoundException': use of dependent type name must be
  prefixed with 'typename'
```

See:
https://github.com/man-group/ArcticDB/actions/runs/5177504090/jobs/9327681067#step:6:374

Signed-off-by: Julien Jerphanion <[email protected]>
@jjerphan jjerphan force-pushed the conda-build/add-windows-latest branch from 2374b5f to 83a7943 Compare June 5, 2023 13:53
Signed-off-by: Julien Jerphanion <[email protected]>
Signed-off-by: Julien Jerphanion <[email protected]>
@jjerphan
Copy link
Collaborator Author

jjerphan commented Jun 6, 2023

pcre cannot be linked on Windows for now, eventhough it apparently is resolved.

There's no CMake configuration for the version of pcre distributed on conda-forge for Windows.

conda-forge/pcre-feedstock#24 is improving the build for Windows regarding CMake.

@jjerphan jjerphan force-pushed the conda-build/add-windows-latest branch from 146d2b7 to f67d0af Compare July 4, 2023 12:27
@jjerphan jjerphan force-pushed the conda-build/add-windows-latest branch 2 times, most recently from 64acc86 to 9ad10cb Compare July 4, 2023 14:15
Reason: needed for python 3.6 which we do not build packages for on
conda-forge.

Also try not to install dependencies via install_requires.

Signed-off-by: Julien Jerphanion <[email protected]>
@jjerphan jjerphan force-pushed the conda-build/add-windows-latest branch from f8833b5 to 9ad57b0 Compare July 5, 2023 16:43
@jjerphan
Copy link
Collaborator Author

jjerphan commented Jul 5, 2023

I tried many things but I still obtain linker errors for the generated sources of the .proto files at the end of the build .

Those errors are due to symbols that are present in the object files of the generated source but that are missing in the shared library of protobuf.

There are 5 missing symbols:

"private: static char const * const * const google::protobuf::FieldDescriptor::kCppTypeToName" (?kCppTypeToName@FieldDescriptor@protobuf@google@@0QBQEBDB)

"class google::protobuf::internal::ExplicitlyConstructed<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,8> google::protobuf::internal::fixed_address_empty_string" (?fixed_address_empty_string@internal@protobuf@google@@3V?$ExplicitlyConstructed@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@$07@123@A)

"void * const * const google::protobuf::internal::kGlobalEmptyTable" (?kGlobalEmptyTable@internal@protobuf@google@@3QBQEAXB)

"struct google::protobuf::internal::DescriptorTable const descriptor_table_google_2fprotobuf_2fany_2eproto" (?descriptor_table_google_2fprotobuf_2fany_2eproto@@3UDescriptorTable@internal@protobuf@google@@B)

"struct google::protobuf::AnyDefaultTypeInternal google::protobuf::_Any_default_instance_" (?_Any_default_instance_@protobuf@google@@3UAnyDefaultTypeInternal@12@A)

The following is used:

  • libprotobuf==3.20.3
  • protobuf==3.20.3
  • MSVC 19.36.32535.0
  • the dynamic runtime (by specifying /MD)

Missing symbols for similar configurations also have been reported in some currently open issues of protobuf. such as:

All linker errors
6>arcticdb_proto.lib(config.pb.obj) : error LNK2001: unresolved external symbol "private: static char const * const * const google::protobuf::FieldDescriptor::kCppTypeToName" (?kCppTypeToName@FieldDescriptor@protobuf@google@@0QBQEBDB)
6>arcticdb_proto.lib(logger.pb.obj) : error LNK2001: unresolved external symbol "private: static char const * const * const google::protobuf::FieldDescriptor::kCppTypeToName" (?kCppTypeToName@FieldDescriptor@protobuf@google@@0QBQEBDB)
6>arcticdb_proto.lib(descriptors.pb.obj) : error LNK2001: unresolved external symbol "private: static char const * const * const google::protobuf::FieldDescriptor::kCppTypeToName" (?kCppTypeToName@FieldDescriptor@protobuf@google@@0QBQEBDB)
6>arcticdb_proto.lib(storage.pb.obj) : error LNK2001: unresolved external symbol "private: static char const * const * const google::protobuf::FieldDescriptor::kCppTypeToName" (?kCppTypeToName@FieldDescriptor@protobuf@google@@0QBQEBDB)
6>arcticdb_proto.lib(nfs_backed_storage.pb.obj) : error LNK2001: unresolved external symbol "class google::protobuf::internal::ExplicitlyConstructed<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,8> google::protobuf::internal::fixed_address_empty_string" (?fixed_address_empty_string@internal@protobuf@google@@3V?$ExplicitlyConstructed@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@$07@123@A)
6>arcticdb_proto.lib(s3_storage.pb.obj) : error LNK2001: unresolved external symbol "class google::protobuf::internal::ExplicitlyConstructed<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,8> google::protobuf::internal::fixed_address_empty_string" (?fixed_address_empty_string@internal@protobuf@google@@3V?$ExplicitlyConstructed@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@$07@123@A)
6>arcticdb_proto.lib(storage.pb.obj) : error LNK2001: unresolved external symbol "class google::protobuf::internal::ExplicitlyConstructed<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,8> google::protobuf::internal::fixed_address_empty_string" (?fixed_address_empty_string@internal@protobuf@google@@3V?$ExplicitlyConstructed@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@$07@123@A)
6>arcticdb_proto.lib(lmdb_storage.pb.obj) : error LNK2001: unresolved external symbol "class google::protobuf::internal::ExplicitlyConstructed<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,8> google::protobuf::internal::fixed_address_empty_string" (?fixed_address_empty_string@internal@protobuf@google@@3V?$ExplicitlyConstructed@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@$07@123@A)
6>arcticdb_proto.lib(mongo_storage.pb.obj) : error LNK2001: unresolved external symbol "class google::protobuf::internal::ExplicitlyConstructed<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,8> google::protobuf::internal::fixed_address_empty_string" (?fixed_address_empty_string@internal@protobuf@google@@3V?$ExplicitlyConstructed@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@$07@123@A)
6>arcticdb_proto.lib(config.pb.obj) : error LNK2001: unresolved external symbol "class google::protobuf::internal::ExplicitlyConstructed<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,8> google::protobuf::internal::fixed_address_empty_string" (?fixed_address_empty_string@internal@protobuf@google@@3V?$ExplicitlyConstructed@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@$07@123@A)
6>arcticdb_proto.lib(utils.pb.obj) : error LNK2001: unresolved external symbol "class google::protobuf::internal::ExplicitlyConstructed<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,8> google::protobuf::internal::fixed_address_empty_string" (?fixed_address_empty_string@internal@protobuf@google@@3V?$ExplicitlyConstructed@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@$07@123@A)
6>arcticdb_proto.lib(logger.pb.obj) : error LNK2001: unresolved external symbol "class google::protobuf::internal::ExplicitlyConstructed<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,8> google::protobuf::internal::fixed_address_empty_string" (?fixed_address_empty_string@internal@protobuf@google@@3V?$ExplicitlyConstructed@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@$07@123@A)
6>arcticdb_proto.lib(descriptors.pb.obj) : error LNK2001: unresolved external symbol "class google::protobuf::internal::ExplicitlyConstructed<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,8> google::protobuf::internal::fixed_address_empty_string" (?fixed_address_empty_string@internal@protobuf@google@@3V?$ExplicitlyConstructed@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@$07@123@A)
6>arcticdb_proto.lib(config.pb.obj) : error LNK2001: unresolved external symbol "void * const * const google::protobuf::internal::kGlobalEmptyTable" (?kGlobalEmptyTable@internal@protobuf@google@@3QBQEAXB)
6>arcticdb_proto.lib(logger.pb.obj) : error LNK2001: unresolved external symbol "void * const * const google::protobuf::internal::kGlobalEmptyTable" (?kGlobalEmptyTable@internal@protobuf@google@@3QBQEAXB)
6>arcticdb_proto.lib(descriptors.pb.obj) : error LNK2001: unresolved external symbol "void * const * const google::protobuf::internal::kGlobalEmptyTable" (?kGlobalEmptyTable@internal@protobuf@google@@3QBQEAXB)
6>arcticdb_proto.lib(storage.pb.obj) : error LNK2001: unresolved external symbol "void * const * const google::protobuf::internal::kGlobalEmptyTable" (?kGlobalEmptyTable@internal@protobuf@google@@3QBQEAXB)
6>arcticdb_proto.lib(storage.pb.obj) : error LNK2001: unresolved external symbol "struct google::protobuf::internal::DescriptorTable const descriptor_table_google_2fprotobuf_2fany_2eproto" (?descriptor_table_google_2fprotobuf_2fany_2eproto@@3UDescriptorTable@internal@protobuf@google@@B)
6>arcticdb_proto.lib(storage.pb.obj) : error LNK2001: unresolved external symbol "struct google::protobuf::AnyDefaultTypeInternal google::protobuf::_Any_default_instance_" (?_Any_default_instance_@protobuf@google@@3UAnyDefaultTypeInternal@12@A)

versions for packages on conda-forge for protobuf 3

The protobuf 4 compilation will also be perform, but we ignore this
part for the moment just to be able to compile for 3 and see if this
works.
@jjerphan jjerphan force-pushed the conda-build/add-windows-latest branch from 9ad57b0 to 39cdb1f Compare July 6, 2023 17:23
The linker fails to find symbols eventhough they are present
in both `libprotobuf.dll` and the `libprotobuf.lib`.

This has been checked:

 - `protoc` and `libprotobuf` have the same version (3.20.3)
 - the linker call for the ultimate target, `arcticdb_ext`,
 correctly contains the path to `libprotobuf.lib`
 - `arcticdb_core_static` links successfully while containing
 the objects of the C++ source files generated by `protoc`
 - the presence of the 5 symbols reported as "missing" has been
 checked with `dumpbin` and Dependencies [1] and they are present
 as exported in both `libprotobuf.dll` and `libprotobuf.lib`.

This has been tried:
 - using `OBJECT` for building intermediary targets.

This has not been tried:
 - checking toolchains and ABI version compatibility with
 conda-forge's
 - creating an executable which depends on `arcticdb_core_static`
 observing if any other specific problems occurs

This commit removes the dependence on `libprotobuf-lite` which is
redundant with `libprotobuf`.

1: https://github.com/lucasg/Dependencies
2: https://protobuf.dev/support/version-support/#python
@jjerphan jjerphan force-pushed the conda-build/add-windows-latest branch from 39cdb1f to d00fbd5 Compare July 6, 2023 17:25
@jjerphan
Copy link
Collaborator Author

Potential cause of failure to have a look at: the packaging of abseil and libprotobuf on conda-forge.

@h-vetinari
Copy link

Potential cause of failure to have a look at: the packaging of abseil and libprotobuf on conda-forge.

I haven't looked at the issues here, but the only hard requirement you need w.r.t. this ATM is to compile with C++17 (or perhaps C++20).

@jjerphan
Copy link
Collaborator Author

I haven't looked at the issues here, but the only hard requirement you need w.r.t. this ATM is to compile with C++17 (or perhaps C++20).

We already compile with C++17, the problem lies in the code-base with some code for headers being inlined unusually via macros (I did have time to have a look at that yet). @Klaim might be able to give more details in this regard.

@Klaim
Copy link
Collaborator

Klaim commented Oct 11, 2023

From memory (didnt get back to this yet): The issue is that the symbols that should be generated for the objects coming from compiling the protobuf-generated code are correctly generated but for some reason they are not found on linking.
The object files being brought into another lib then another lib might be making this complicated but I remember when we investigated we checked that the symbols were correctly available but the linking step was still failing.
Once I fix the issues with the inline headers #943 I will investigate more.
That issue might be related because it's bascially making the whole program Ill-Formed, No Diagnostic Required (IFNDR), which basically means the whole program is broken and we cannot assume anything correctly without fixing that first.

@jjerphan jjerphan mentioned this pull request Oct 11, 2023
@jjerphan
Copy link
Collaborator Author

Failures might be related to conda-forge/libprotobuf-feedstock#201.

@jjerphan jjerphan force-pushed the conda-build/add-windows-latest branch from 99b14c5 to d8c1193 Compare November 29, 2023 14:39
Signed-off-by: Julien Jerphanion <[email protected]>
Signed-off-by: Julien Jerphanion <[email protected]>
@jjerphan
Copy link
Collaborator Author

Closing for now, see #1412.

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.

4 participants