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

rpm packaging #729

Closed
shenlebantongying opened this issue May 21, 2023 · 23 comments
Closed

rpm packaging #729

shenlebantongying opened this issue May 21, 2023 · 23 comments
Assignees

Comments

@shenlebantongying
Copy link
Collaborator

shenlebantongying commented May 21, 2023

Sample RPM to build with qmake/qt5 + qmake/qt6: https://build.opensuse.org/package/view_file/Education/goldendict-ng/goldendict-ng.spec

Help wanted.

@shenlebantongying shenlebantongying closed this as not planned Won't fix, can't repro, duplicate, stale May 22, 2023
@topazus
Copy link

topazus commented Jun 1, 2023

@shenlebantongying Hello, I am interested in packaging this into Fedora. Actually, I have made the tomlplusplus dependency into fedora not long ago. There are some third-party libraries that should be avoided, using the corresponding system dependencies is suggested. Maybe, we can add options (OFF by default), likeUSE_SYSTEM_FMT and USE_SYSTEM_TOMLPLUSPLUS or others, to enable or disable the possibility of building against these dependecies.

For toml++, the include directive of toml++ header file needs to be modified.

#include "metadata.hh"
#include "toml.hpp"
#include <QDebug>
#include <QFile>

❯ rpm -ql tomlplusplus-devel 
/usr/include/toml++
/usr/include/toml++/impl
/usr/include/toml++/impl/array.h
/usr/include/toml++/impl/array.inl
/usr/include/toml++/impl/at_path.h
/usr/include/toml++/impl/at_path.inl
/usr/include/toml++/impl/date_time.h
/usr/include/toml++/impl/formatter.h
/usr/include/toml++/impl/formatter.inl
/usr/include/toml++/impl/forward_declarations.h
/usr/include/toml++/impl/header_end.h
/usr/include/toml++/impl/header_start.h
/usr/include/toml++/impl/json_formatter.h
/usr/include/toml++/impl/json_formatter.inl
/usr/include/toml++/impl/key.h
/usr/include/toml++/impl/make_node.h
/usr/include/toml++/impl/node.h
/usr/include/toml++/impl/node.inl
/usr/include/toml++/impl/node_view.h
/usr/include/toml++/impl/parse_error.h
/usr/include/toml++/impl/parse_result.h
/usr/include/toml++/impl/parser.h
/usr/include/toml++/impl/parser.inl
/usr/include/toml++/impl/path.h
/usr/include/toml++/impl/path.inl
/usr/include/toml++/impl/preprocessor.h
/usr/include/toml++/impl/print_to_stream.h
/usr/include/toml++/impl/print_to_stream.inl
/usr/include/toml++/impl/simd.h
/usr/include/toml++/impl/source_region.h
/usr/include/toml++/impl/std_except.h
/usr/include/toml++/impl/std_initializer_list.h
/usr/include/toml++/impl/std_map.h
/usr/include/toml++/impl/std_new.h
/usr/include/toml++/impl/std_optional.h
/usr/include/toml++/impl/std_string.h
/usr/include/toml++/impl/std_string.inl
/usr/include/toml++/impl/std_utility.h
/usr/include/toml++/impl/std_variant.h
/usr/include/toml++/impl/std_vector.h
/usr/include/toml++/impl/table.h
/usr/include/toml++/impl/table.inl
/usr/include/toml++/impl/toml_formatter.h
/usr/include/toml++/impl/toml_formatter.inl
/usr/include/toml++/impl/unicode.h
/usr/include/toml++/impl/unicode.inl
/usr/include/toml++/impl/unicode_autogenerated.h
/usr/include/toml++/impl/value.h
/usr/include/toml++/impl/version.h
/usr/include/toml++/impl/yaml_formatter.h
/usr/include/toml++/impl/yaml_formatter.inl
/usr/include/toml++/toml.h
/usr/lib64/cmake/tomlplusplus
/usr/lib64/cmake/tomlplusplus/tomlplusplusConfig.cmake
/usr/lib64/cmake/tomlplusplus/tomlplusplusConfigVersion.cmake
/usr/lib64/libtomlplusplus.so
/usr/lib64/pkgconfig/tomlplusplus.pc

It seemed that qtsingleapplication is also one of the dependencies.The similar dependencies also have been packaged for fedora, but they are built against qt4 or qt5.

ref: https://packages.fedoraproject.org/pkgs/qtsingleapplication/
https://packages.fedoraproject.org/pkgs/tomlplusplus/
https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling

@shenlebantongying
Copy link
Collaborator Author

I can add an option to use system toml.

The qtsingleapplication cannot.

@shenlebantongying shenlebantongying self-assigned this Jun 1, 2023
@topazus
Copy link

topazus commented Jun 1, 2023

Okay, I could mark the qtsingleapplication lib as bundled dependency. So, what do you think of the fmt library, I see the fmt lib has been updated to 10.0.0, in this #717 . But I have tried goldendict-ng with system fmt lib of 9.1.0 version, it could build and work. And, what about simple-eb?

@shenlebantongying
Copy link
Collaborator Author

And, what about simple-eb?

@xiaoyifang knows more about eb.

Right now, the simple-eb is used only for windows build.

On linux, https://packages.fedoraproject.org/pkgs/eb/ is used.

@xiaoyifang
Copy link
Owner

when packaging rpm , will qmake be used or cmake?

@shenlebantongying
Copy link
Collaborator Author

shenlebantongying commented Jun 1, 2023

qmake?

Before the next qt release at the end of this year, we can use qmake at the moment?

@topazus
Copy link

topazus commented Jun 1, 2023

when packaging rpm , will qmake be used or cmake?

I would prefer cmake.

@shenlebantongying
Copy link
Collaborator Author

shenlebantongying commented Jun 1, 2023

Then I will double-check the CMake build. We can ship it to Fedora as the first official CMake build :)

@topazus
Copy link

topazus commented Jun 1, 2023

I have a question about the github tags. The curent tag names, like https://github.com/xiaoyifang/goldendict-ng/releases/tag/v23.06.01-ChildrenDay.230531.e771a65f , is too long and not standard. Could you set it to normal versioning, like v1.0.0, or just v23.06.01 when the new release is published later?

@xiaoyifang
Copy link
Owner

xiaoyifang commented Jun 1, 2023

just v23.06.01 when the new release is published later?

Next time I'll take this form as an extra release tag alongside the current release tag.

handled here #786 ,will take effect next time.

@shenlebantongying
Copy link
Collaborator Author

shenlebantongying commented Jun 1, 2023

@xiaoyifang I think we should just use simple-eb as a static library.

Separately packing a library and installing a .so file that only we use is a major pain.

The name libeb.so is conflicting with the existing libeb.

@xiaoyifang
Copy link
Owner

on Linux/Macos , what about use libeb directly and not use simple-eb.

@shenlebantongying
Copy link
Collaborator Author

shenlebantongying commented Jun 1, 2023

@topazus We can use CMake. Defaults options will enable all features.

Add those options -DUSE_SYSTEM_FMT=ON -DUSE_SYSTEM_TOML=ON to use system fmt & toml.

the theirdparty/eb is not used at all -> https://packages.fedoraproject.org/pkgs/eb/ is needed.

@xiaoyifang
Copy link
Owner

xiaoyifang commented Jun 1, 2023

a new release package and tag have been made.
https://github.com/xiaoyifang/goldendict-ng/releases/tag/v23.06.01

The cmake changes have also been pushed to master

@aaly11
Copy link

aaly11 commented Jun 3, 2023

Привет. Интерфейс программы при сборке её с помощью cmake остаётся не локализованным, не смотря на то, что файлы локализации установлены. Вопрос, почему?
Ссылка на проект из Open Build Service.
Для 15.5 программа собрана с помощью qmake (есть ограничение на версию cmake >= 3.25).
Для openSUSE Tumbleweed с помощью cmake.
Также были собраны библиотеки libzim и tomlplusplus.

@topazus
Copy link

topazus commented Jun 27, 2023

Sorry for the a bit late response. There are several issues about goldendict-ng Fedora packaging.

If the conflict could be avoided (i.e. by renaming the binary and data directories) that would be preferred. This would support a scenario in which a user may decide to install both variants of the package.
That would fulfill the "As a general rule, Fedora packages must NOT contain any usage of the Conflicts: field." part of the packaging guidelines.

goldendict-ng.x86_64: W: unused-direct-shlib-dependency /usr/bin/goldendict /lib64/libfmt.so.9

Ref: https://bugzilla.redhat.com/show_bug.cgi?id=2213078#c7

@shenlebantongying
Copy link
Collaborator Author

shenlebantongying commented Jun 27, 2023

  1. Not changing binary name & resource name has a few benefits :)

#690

  • rpmlint warning: unused-direct-shlib-dependency could be fixed by patching the CMake files to pass the as-needed flag for the fmt library, see https://stackoverflow.com/a/65819681 (this patch should be suggested upstream)

Adding the solution suggested will stop the binary linked to libfmt.so in Release build while not in Debug.

I deleted the libfmt.so in Release, and related code paths still work. It is indeed not needed (still needed in link time).

We only use it in a few places at the moment. Turning on Release appears to optimize them all away?? Amazing 😄

The --as-needed flag already has been added to LDFLAGS by default. I do not
have any idea about how to fix this warning.

Ref: https://fedoraproject.org/wiki/Changes/RemoveExcessiveLinking
This one is certainly a bit weird. The source references fmt but the symbol doesn't show up in the binary. I'll need to investigate this a bit further.

I am not sure why cmake is not picking up the flag, but we can add -as-needed manually.


The check just read ldd -u,

https://github.com/rpm-software-management/rpmlint/blob/c40ff528996e11c5502f8dbcd62d361547b88e1d/rpmlint/lddparser.py#L51-L67

@shenlebantongying
Copy link
Collaborator Author

The problem now is that the upstream reviewer appears to be missing for some reason.

I don't think further actions need to be done on our side.

@topazus
Copy link

topazus commented Dec 4, 2023

@shenlebantongying With build on the latest commit of the repository and install the package, the rpmlint -i goldendict-ng reported the following issue, which the issue can be cleared by the chrpath tool though.

goldendict-ng.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/goldendict-ng (RUNPATH: $ORIGIN:$ORIGIN/../lib64)

I do not know where the RPATH introduced. Can you clarify this?

Ref: https://gitlab.kitware.com/cmake/community/-/wikis/doc/cmake/RPATH-handling

@shenlebantongying
Copy link
Collaborator Author

I can reproduce this on my machine. cmake install will add this RPATH to the binary.

Weirdly, according to this note, stripping RPATH is the default behaviour of INSTALL(TARGETS. https://docs.fedoraproject.org/en-US/packaging-guidelines/CMake/#_notes

install(TARGETS ${GOLDENDICT})

@topazus
Copy link

topazus commented Dec 4, 2023

cmake install will add this RPATH to the binary.

Do you have evidence links that says this behavior as I did not encounter before?

@shenlebantongying
Copy link
Collaborator Author

My wording was bad. The document says RPATH will be cleared when doing cmake install, but this is not what we observed, which looks like "cmake install will add this RPATH to the binary."

https://gitlab.kitware.com/cmake/community/-/wikis/doc/cmake/RPATH-handling#default-rpath-settings

By default if you don't change any RPATH related settings, CMake will
link the executables and shared libraries with full RPATH to all used
libraries in the build tree. When installing, it will clear the RPATH of
these targets so they are installed with an empty RPATH.


My current suspicion is qt_standard_project_setup, which modifies a few rpath-related things. It overrides the default behaviour.

qt_standard_project_setup() # availiable after find_package(Qt6 .... Core

Not sure why would Qt do that. Linux distros clearly against it https://fedoraproject.org/wiki/Changes/Broken_RPATH_will_fail_rpmbuild

https://doc.qt.io/qt-6/qt-standard-project-setup.html

On platforms that support RPATH (other than Apple platforms), two values are appended to the CMAKE_INSTALL_RPATH variable by this command. $ORIGIN is appended so that libraries will find other libraries they depend on in the same directory as themselves. $ORIGIN/ is also appended, where is the relative path from CMAKE_INSTALL_BINDIR to CMAKE_INSTALL_LIBDIR.


I think we can avoid qt_standard_project_setup for now.

@topazus
Copy link

topazus commented Dec 5, 2023

@shenlebantongying Thanks for the explanation on this, I will set -DCMAKE_SKIP_RPATH=ON to deal with the rpath issue at the moment. Btw, The goldendict-ng package will be available in Fedora. https://src.fedoraproject.org/rpms/goldendict-ng

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

No branches or pull requests

4 participants