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

KDE fixes #32487

Closed
wants to merge 5 commits into from
Closed

KDE fixes #32487

wants to merge 5 commits into from

Conversation

HanabishiRecca
Copy link
Contributor

The rest fixes from #32474.

Signed-off-by: Hanabishi <13597663+HanabishiRecca@users.noreply.github.com>
Signed-off-by: Hanabishi <13597663+HanabishiRecca@users.noreply.github.com>
…conditional

Signed-off-by: Hanabishi <13597663+HanabishiRecca@users.noreply.github.com>
Signed-off-by: Hanabishi <13597663+HanabishiRecca@users.noreply.github.com>
Signed-off-by: Hanabishi <13597663+HanabishiRecca@users.noreply.github.com>
@gentoo-bot
Copy link

Pull Request assignment

Submitter: @HanabishiRecca
Areas affected: ebuilds
Packages affected: kde-frameworks/kdelibs4support, kde-frameworks/kio, kde-plasma/plasma-desktop, kde-plasma/plasma-workspace

kde-frameworks/kdelibs4support: @gentoo/kde
kde-frameworks/kio: @gentoo/kde
kde-plasma/plasma-desktop: @gentoo/kde
kde-plasma/plasma-workspace: @gentoo/kde

Linked bugs

No bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment.

If you do not receive any reply to this pull request, please open or link a bug to attract the attention of maintainers.


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. labels Aug 28, 2023
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2023-08-28 15:30 UTC
Newest commit scanned: 93a32a5
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/5b3c635d88/output.html

Copy link
Member

@a17r a17r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR. I will likely pick your changes to kde overlay after all, to avoid revbumps for small fixes, since KDE Frameworks 5.110 and Plasma 5.27.8 are imminent/very near.. There is no need for you to move it over there yourself, unless you would like to. Original authorship and link to review discussion here will be retained of course.

cmake_run_in src cmake_comment_add_subdirectory l10n
}

src_configure() {
local mycmakeargs=(
-DWITH_X11=$(usex X)
-DCMAKE_DISABLE_FIND_PACKAGE_NetworkManager=ON
Copy link
Member

@a17r a17r Sep 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Between #32474 (comment) and this change, did you come to a conclusion that this feature does not do anything (for us?) after all?

I'm just curious if there's more to it, otherwise will need to inquire myself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a standalone kded service generated if NM support is enabled. But I do not actually figured where it is being used.
Anyway, automagic dependencies are bad, so I think disabling it explicitly is fine for now. Introducing new use flags is more complicated question.

Copy link
Member

@a17r a17r Sep 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a standalone kded service generated if NM support is enabled

kded_networkstatus is always being generated, just with or without NM backend support.

@HanabishiRecca
Copy link
Contributor Author

You are welcome.
I don't care about authorship. The only thing that matters is to get the job done.

Comment on lines -22 to -23
dev-libs/libxml2
dev-libs/libxslt
Copy link
Member

@a17r a17r Sep 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also turns out not to be that easy. I traced it back to gentoo/kde@cd9341e and gentoo/kde@0e7d507, both of which still hold true today:

$ lddtree /usr/lib64/qt5/plugins/kf5/kio/kio_help.so
[...]
    libKF5DocTools.so.5 => /usr/lib64/libKF5DocTools.so.5
        libxslt.so.1 => /usr/lib64/libxslt.so.1
    libxml2.so.2 => /usr/lib64/libxml2.so.2
        liblzma.so.5 => /lib64/liblzma.so.5
        libicuuc.so.73 => /usr/lib64/libicuuc.so.73
            libicudata.so.73 => /usr/lib64/libicudata.so.73
    libexslt.so.0 => /usr/lib64/libexslt.so.0
        libgcrypt.so.20 => /usr/lib64/libgcrypt.so.20
            libgpg-error.so.0 => /usr/lib64/libgpg-error.so.0
$ lddtree /usr/lib64/qt5/plugins/kf5/kio/kio_ghelp.so
[...]
    libKF5DocTools.so.5 => /usr/lib64/libKF5DocTools.so.5
        libxslt.so.1 => /usr/lib64/libxslt.so.1
    libxml2.so.2 => /usr/lib64/libxml2.so.2
        liblzma.so.5 => /lib64/liblzma.so.5
        libicuuc.so.73 => /usr/lib64/libicuuc.so.73
            libicudata.so.73 => /usr/lib64/libicudata.so.73
    libexslt.so.0 => /usr/lib64/libexslt.so.0
        libgcrypt.so.20 => /usr/lib64/libgcrypt.so.20
            libgpg-error.so.0 => /usr/lib64/libgpg-error.so.0

However, we could rightfully assume that dev-libs/libxml2 and dev-libs/libxslt are being wrongfully unconditionally depended on here - and that's right after looking at https://invent.kde.org/frameworks/kio/-/blob/kf5/src/kioworkers/help/CMakeLists.txt. These deps had been there since the very beginning of that ebuild (gentoo/kde@7b5c719). It was correct at the time, but the deps became optional after https://invent.kde.org/frameworks/kio/-/commit/9861d5de30c0da09de8bd50407f154f1a6c1f171.

Looking at all that build system code at least spawned a minor upstream cleanup MR: https://invent.kde.org/frameworks/kio/-/merge_requests/1403

@a17r
Copy link
Member

a17r commented Sep 10, 2023

You are welcome. I don't care about authorship.

Unfortunately I can't retain git authorship anyway due to your use of a noreply-github-address, so it will be name-only attribution unless you change that.

gentoo-bot pushed a commit to gentoo/kde that referenced this pull request Sep 10, 2023
Thanks-to: Hanabishi
Closes: gentoo/gentoo#32487
Signed-off-by: Andreas Sturmlechner <asturm@gentoo.org>
gentoo-bot pushed a commit to gentoo/kde that referenced this pull request Sep 10, 2023
Even if there is no linking with libnm.so, the service will be
required at runtime if we build with this feature enabled.

See also: gentoo/gentoo#32487

Signed-off-by: Andreas Sturmlechner <asturm@gentoo.org>
gentoo-bot pushed a commit to gentoo/kde that referenced this pull request Sep 10, 2023
Upstream translations organisation has changed since commit
cd9341e.
Moves handbook handling back into ecm.eclass and automatically fixes
missing BDEPEND=kde-frameworks/kdoctools as well.

See also: gentoo/gentoo#32487

Thanks-to: Hanabishi
Signed-off-by: Andreas Sturmlechner <asturm@gentoo.org>
gentoo-bot pushed a commit to gentoo/kde that referenced this pull request Sep 10, 2023
Follow-up to 3ba8e13, good to revisit
these deps. Making features optional has consequences :)

See also: gentoo/gentoo#32487

Signed-off-by: Andreas Sturmlechner <asturm@gentoo.org>
@HanabishiRecca HanabishiRecca deleted the kde-fixes branch October 4, 2023 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits.
Projects
None yet
4 participants