primesieve: new package #22577

Ahajha · 2024-01-29T04:06:40Z

Specify library name and version: primesieve/11.2

This is a continuation of #14530, which was essentially already ready. I've just updated the package to 11.2.

Resolves #14529

I've read the contributing guidelines.
I've used a recent Conan client version close to the currently deployed.
I've tried at least one configuration locally with the conan-center hook activated.

CLAassistant · 2024-01-29T04:06:45Z

All committers have signed the CLA.

AbrilRBS

Quick review

recipes/primesieve/all/conanfile.py

AbrilRBS · 2024-01-31T16:49:16Z

recipes/primesieve/all/test_package/test_package.cpp

+int main()
+{
+  std::vector<uint64_t> primes;
+  generate_n_primes(25, &primes);
+  std::cout << "primes.size() = " << primes.size();
+  check(primes.size() == 25);
+
+  for (uint64_t i = 0; i < primes.size(); i++)
+  {
+    std::cout << "primes[" << i << "] = " << primes[i];
+    check(primes[i] == small_primes[i]);
+  }
+
+  primes.clear();
+
+  generate_n_primes(19, 18446744073709550672ull, &primes);
+  std::cout << "primes.size() = " << primes.size();
+  check(primes.size() == 19);
+
+  for (uint64_t i = 0; i < primes.size(); i++)
+  {
+    std::cout << "primes[" << i << "] = " << primes[i];
+    check(primes[i] == large_primes[i]);
+  }
+
+  std::string errorMsg;
+
+  try
+  {
+    std::vector<uint16_t> primes16;
+    generate_n_primes(10, (1 << 16) - 10, &primes16);
+  }
+  catch (const primesieve_error& e)
+  {
+    errorMsg = e.what();
+  }
+
+  std::cout << "Detect 16-bit overflow: " << errorMsg;
+  check(!errorMsg.empty());
+
+  std::cout << std::endl;
+  std::cout << "All tests passed successfully!" << std::endl;
+
+  return 0;
+}


Can we simplify this by a lot? test_packages should limit themselves to small tests checking that the library was correctly packaged - no need to be generating primes here :)

The library is intended to generate prime numbers, that said this is definitely complicated. I can probably have it generate just one.

Done, program just generates and prints 5 primes.

Ahajha · 2024-02-04T02:50:20Z

@kimwalisch FYI, the PR from a few years ago was never merged, I'm continuing it now in hopes that we can actually add it.

Side question for you, you require C++11, which GCC 5 does support, but the package seems to fail when using it. Any idea why? Do you want to support this version?

@Alex-PLACET I'm continuing your work, which requires you to have the CLA signed (I think it has 4 month expiration or so), would you mind doing that when you get the chance? No rush, still working on getting things fixed.

github-actions · 2024-02-04T04:27:10Z

Hooks produced the following warnings for commit bae1235

primesieve/11.2@#114610158235b0df0e1d4db1a94271e9

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libprimesieve.so' links to system library 'm' but it is not in cpp_info.system_libs.

kimwalisch · 2024-02-04T08:30:49Z

Side question for you, you require C++11, which GCC 5 does support, but the package seems to fail when using it. Any idea why? Do you want to support this version?

Yes, primesieve requires full C++11 (or later) support. Some old GCC compilers only partially support C++11 (e.g. std::align from C++11 is not supported by these compilers) and therefore they fail to build primesieve. E.g. I know that GCC 4.9 cannot compile primesieve.

But if the C++ compiler is able to build primesieve, then it must work correctly at runtime (otherwise this is a bug). If you are experiencing primesieve runtime errors then I am interested in seeing those logs.

Ahajha · 2024-02-04T09:02:20Z

But if the C++ compiler is able to build primesieve, then it must work correctly at runtime (otherwise this is a bug). If you are experiencing primesieve runtime errors then I am interested in seeing those logs.

Conan test packages are only intended to test if the package itself works. Usually this is verifying a simple example compiles, links, and executes without errors. The test package here just generates a handful of prime numbers, which is a simple enough test that it probably won't show any errors unless the packaging is wrong on our end.

Occasionally there is a bug report which involves how it is packaged as the cause of the bug, but those tend to be uncommon.

Alex-PLACET · 2024-02-04T13:00:43Z

@Ahajha, a big thank you for picking up where I left off on my PR. I completely forgot to keep it going. Appreciate your help!

AbrilRBS · 2024-02-07T12:34:11Z

recipes/primesieve/all/conanfile.py

+        rm(self, "*.la", os.path.join(self.package_folder, "lib"))
+        rm(self, "*.pdb", os.path.join(self.package_folder, "lib"))
+        rm(self, "*.pdb", os.path.join(self.package_folder, "bin"))
+        if is_msvc(self) and self.options.shared:


Any insight into this rename? It looks funny that it gets generated with this name in the first place :)

It seems this is set to avoid issues when packaging both static and shared libraries in the same package. This doesn't apply to us, so I think it's fine to do this? Unless we have a way of telling Conan that this is how it's done.

https://github.com/kimwalisch/primesieve/blob/master/CMakeLists.txt#L138

Please, keep the original name, it should work as usual, right? I remember having other projects having this same prefix when exporting dll libs

It would fail without this change, I presume it tries to look for "primesieve".lib but it's "primesieve.dll".lib. I'm fine with changing it back, I just don't know how I would get it to work.

@uilianries @RubenRBS Many more recipes are doing this rename. You can run a grep and see this ad-hoc helper function fix_msvc_libname. The problem is coming from the hook error:

primesieve/11.2: WARN: Using the new toolchains and generators without specifying a build profile (e.g: -pr:b=default) is discouraged and might cause failures and unexpected behavior [HOOK - conan-center.py] post_package_info(): ERROR: [LIBRARY DOES NOT EXIST (KB-H054)] Component primesieve::primesieve library 'primesieve' is listed in the recipe, but not found installed at self.cpp_info.libdirs. Make sure you compiled the library correctly. If so, then the library name should probably be fixed. Otherwise, then the component should be removed. (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H054-LIBRARY-DOES-NOT-EXIST) ERROR: ConanException: [HOOK - conan-center.py] post_package_info(): Some checks failed running the hook, check the output

@Ahajha We just created an issue to review this process conan-io/hooks#528 Let's figure out if there is a bug in the hooks so we can use the original library name instead of the renamed one.

uilianries

Thank you for your PR, it's looking good, just few comments. This PR can be reduced a bit.

uilianries · 2024-02-13T10:29:15Z

recipes/primesieve/all/conanfile.py

+
+        if self.settings.compiler == "gcc" and Version(self.settings.compiler.version) <= "5":
+            raise ConanInvalidConfiguration("GCC<=5 is currently not supported. Contributions with fixes are welcome.")


Suggested change

if self.settings.compiler == "gcc" and Version(self.settings.compiler.version) <= "5":

raise ConanInvalidConfiguration("GCC<=5 is currently not supported. Contributions with fixes are welcome.")

I used to work with GCC 4.7 with C++11 minimal support. GCC 4.8 has a really good coverage of C++11. Usually, we don't add restriction to compiler version when the project only requires C++11

GCC 5 fails to build, not sure why though. @kimwalisch mentioned above that 4.9 is known not to work, for example.

uilianries · 2024-02-13T10:31:47Z

recipes/primesieve/all/conanfile.py

+        rm(self, "*.la", os.path.join(self.package_folder, "lib"))
+        rm(self, "*.pdb", os.path.join(self.package_folder, "lib"))
+        rm(self, "*.pdb", os.path.join(self.package_folder, "bin"))
+        if is_msvc(self) and self.options.shared:


Please, keep the original name, it should work as usual, right? I remember having other projects having this same prefix when exporting dll libs

recipes/primesieve/all/conanfile.py

recipes/primesieve/all/test_v1_package/CMakeLists.txt

conan-center-bot · 2024-02-13T16:35:30Z

Conan v1 pipeline ✔️

All green in build 6 (c85ed30d89ea7fb721c28c54b370680340785e0f):

primesieve/11.2:
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 6 (c85ed30d89ea7fb721c28c54b370680340785e0f):

primesieve/11.2:
All packages built successfully! (All logs)

franramirez688

As this is not an isolated case, I think that we can move it forward and wait for the hooks issue which could solve many other PRs.

Alexis Placet and others added 3 commits December 12, 2022 13:43

primesieve: add version 11.0

d25637e

primesieve: Fix version numer

616f2e1

Update to 11.2

b4160c6

prince-chrismc mentioned this pull request Jan 29, 2024

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

conan-center-bot added the Failed label Jan 29, 2024