https://bugzilla.redhat.com/show_bug.cgi?id=2231252 --- Comment #16 from Keyue Hu <hukeyue@xxxxxxxxxxx> --- (In reply to Benson Muite from comment #15) > Thanks for adding this to Fedora. Initial comments: Thanks for your work. > a) Can you use SPDX identifier > GPL-2.0-only > or > GPL-2.0-or-later > See > https://docs.fedoraproject.org/en-US/packaging-guidelines/ > LicensingGuidelines/#_valid_license_short_names It’s GPL-2.0-only. There is a COPYING file under the top directory of source documenting it. > > b) Perhaps add > BuildRequires: aspio-devel I don’t know what’s aspio doing but if you mean asio, it is almost header-only library. (Does it get packaged in fedora as well?) > > c) Some third party code is distributed with the tarball and built: > https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora- > review-2231252-yass/fedora-rawhide-x86_64/06265743-yass/builder-live.log.gz > Perhaps indicate what is bundled and explain why a version packaged in > Fedora cannot be used, see: > https://packages.fedoraproject.org/pkgs/gtest/gtest > https://packages.fedoraproject.org/pkgs/http-parser/http-parser > https://packages.fedoraproject.org/pkgs/abseil-cpp/abseil-cpp > https://packages.fedoraproject.org/pkgs/google-benchmark/google-benchmark > https://packages.fedoraproject.org/pkgs/xxhash/xxhash Thanks for pointing it out. There are two major obstacles from adopting system libraries, one is that it is a cross-platform project supporting msvc(visual c++), mingw32, Linux, musl-based Linux, FreeBSD and macOS targets. There are some reasons to keep things in one place and make it work across different platforms. The other is that yass largely depend on c++ libraries where some of them has no stable ABI which means things break on upgrading or downgrading the shared library. googletest and google-benchmark are on test purposes and c++ based. The api might vary between even two snapshots, so it is built from source if possible. For abseil-cpp, it is also c++ based. We are using the latest lts branch and from previous experience, we really don’t want to move from it as it might break sanitizers tests such as memory sanitizer. See https://clang.llvm.org/docs/MemorySanitizer.html. As to xxhash, we just bumped to lastest release 0.8.2. If fedora switches to it already, we can move to the system one. Otherwise, something might break as int64x2 or some definition is not found in some platforms. For http-parser, I really can’t catch up whether it is the same thing in yass project. The http-parser used comes from node project and not maintained for some years (get replaced by another parser in typed script). We can catch it up if it won’t get rid from fedora. > > BoringSSL is not packaged, but maybe OpenSSL can be used instead: > https://packages.fedoraproject.org/pkgs/openssl/openssl Quiche library is bound to boringssl and we don’t have alternatives. > > Quiche is not available: > https://github.com/google/quiche > but probably worth packaging separately. Though it currently uses Bazel > which is only available as a copr: > https://copr.fedorainfracloud.org/coprs/vbatts/bazel/ > The build seems to use CMake, so maybe something that can be contributed > upstream. Quiche is out from google internal library and used for two open source projects namely, chromium and envoy. You can start from it if want to make it a distributable package. I don’t believe there is a stable ABI because they breaks things from time to time. Officially, they support gn and bazel build system but only part of it is open source. If you are looking at bazel, you can’t miss envoy’s code. They separate every single library there. If I miss something here, please point it out in reply. Thanks in advance. > > Other implementations listed at: > https://github.com/quicwg/base-drafts/wiki/Implementations > > Possibly easier to package are: > https://github.com/h2o/quicly > https://github.com/p-quic/pquic > https://github.com/private-octopus/picoquic > https://github.com/alibaba/xquic (Depends on BabaSSL/Tongsou or BoringSSL) I must admit I don’t be familiar with most of them. But there are many quiche libraries such the one from cloudflare is in golang and not related. https://github.com/cloudflare/quiche > > Maybe also useful might be: > https://packages.fedoraproject.org/search?query=quic > > d) Can Fedora build macros be used: > %cmake > %cmake_build > %cmake_install > > See > https://docs.fedoraproject.org/en-US/packaging-guidelines/CMake/ We are passing many custom options to cmake. If the rpm macros can work instead, we should replace them. > > e) Is golang needed? It seems Go code is used only in tools/ and this has > dependencies which are not included. yes, it is mimic required but besides the tool directory, it is used in boringssl to generate error-handling c code. > > f) The line > Source0: > https://github.com/Chilledheart/yass/archive/refs/tags/%{version}.tar.gz > should be > Source0: %{url}/archive/%{version}/%{name}-%{version}.tar.gz > > see > https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ > #_git_tags > Though the srpm seems to use full git source rather than a commit as third > party code is included. No worry, the upstream will upload the git whole archive without history in every release such as 1.3.14 release, there is https://github.com/Chilledheart/yass/releases/download/1.3.14/yass-1.3.14.tar.gz I will correct the url in next update. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2231252 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202231252%23c16 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
