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

LXQt: bump to 1.0.0 #22845

Closed
wants to merge 31 commits into from
Closed

LXQt: bump to 1.0.0 #22845

wants to merge 31 commits into from

Conversation

AdelKS
Copy link
Contributor

@AdelKS AdelKS commented Nov 6, 2021

Hello,

This PR bumps the LXQt packages to the newly released 1.0.0 version. I confirm it works on my amd64 machine, I made sure to keyword everything as unstable. I also used qa-vdb to improve the dependency lists for each package.

One QA notice I get is about .desktop files in various packages, an example:

/usr/share/applications/lxqt-config-powermanagement.desktop: error: value "Settings;DesktopSettings;Qt;LXQt;" 
for key "Categories" in group "Desktop Entry" contains an unregistered value "LXQt"; 
values extending the format should start with "X-"

Which has already been reported in https://bugs.gentoo.org/512074, and I informed upstream about it here lxqt/lxqt#2113

That's about it!

Hope I am helping with this PR and not the opposite.

Adel

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @AdelKS
Areas affected: ebuilds
Packages affected: app-arch/lxqt-archiver, dev-libs/libqtxdg, dev-util/lxqt-build-tools, lxqt-base/liblxqt, lxqt-base/libsysstat...

app-arch/lxqt-archiver: @gentoo/lxqt
dev-libs/libqtxdg: @gentoo/lxqt, @gentoo/qt
dev-util/lxqt-build-tools: @gentoo/lxqt
lxqt-base/liblxqt: @gentoo/lxqt
lxqt-base/libsysstat: @gentoo/lxqt
lxqt-base/lxqt-about: @gentoo/lxqt
lxqt-base/lxqt-admin: @gentoo/lxqt
lxqt-base/lxqt-config: @gentoo/lxqt
lxqt-base/lxqt-globalkeys: @gentoo/lxqt
lxqt-base/lxqt-meta: @gentoo/lxqt
lxqt-base/lxqt-notificationd: @gentoo/lxqt
lxqt-base/lxqt-openssh-askpass: @gentoo/lxqt
lxqt-base/lxqt-panel: @gentoo/lxqt
lxqt-base/lxqt-policykit: @gentoo/lxqt
lxqt-base/lxqt-powermanagement: @gentoo/lxqt
lxqt-base/lxqt-qtplugin: @gentoo/lxqt
lxqt-base/lxqt-runner: @gentoo/lxqt
lxqt-base/lxqt-session: @gentoo/lxqt
lxqt-base/lxqt-sudo: @gentoo/lxqt
media-gfx/lximage-qt: @gentoo/lxqt
media-sound/pavucontrol-qt: @gentoo/lxqt
x11-libs/libfm-qt: @gentoo/lxqt
x11-libs/qtermwidget: @gentoo/qt
x11-misc/pcmanfm-qt: @gentoo/lxqt
x11-misc/qps: @gentoo/qt
x11-misc/screengrab: @gentoo/qt
x11-terms/qterminal: @gentoo/qt
x11-themes/lxqt-themes: @gentoo/lxqt

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 Nov 6, 2021
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-11-06 16:10 UTC
Newest commit scanned: 4d7b840
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/fd146bd267/output.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-11-06 16:25 UTC
Newest commit scanned: 5a3829f
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/7f562cb284/output.html

@thesamesam
Copy link
Member

Just two notes:

  1. re qa-vdb, it's usually correct in terms of adding a dep, but in terms of removing, be careful. Not all deps are linked against (=> it may appear they should be removed when they shouldn't be).
  2. squash the "put keywords to unstable" commit into the original bump one for that package

@AdelKS
Copy link
Contributor Author

AdelKS commented Nov 7, 2021

Hello!

  1. For qa-vdb, I will go through the source code and check for dlopens and system calls. I think in this case the removed deps are okay.
  2. Yep I can do that, I thought about it then I didn't do it thinking this PR will be squashed anyway. But now that I think of it, it's maybe too big to be squashed.

Thanks!

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-11-07 14:20 UTC
Newest commit scanned: 75f3ce6
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/c3b463c27a/output.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-11-07 15:00 UTC
Newest commit scanned: 91ffad0
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/2a1ac7bc8f/output.html

@AdelKS
Copy link
Contributor Author

AdelKS commented Nov 7, 2021

Okay I revisited the dependency changes by having a look at the corresponding CMakeLists.txt for each package and reverted some changes: I have a little bit of knowledge in Qt and I am sure that extra used modules are linked against, but someCMakeLists.txt contain unneeded Qt deps, cmake would probably fail if the dependencies are missing even though they are not needed in the final executable.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-11-07 15:30 UTC
Newest commit scanned: 6d5f8a9
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/e8b16c5d24/output.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-11-07 15:45 UTC
Newest commit scanned: 2518ad5
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/ecaeeaf23e/output.html

@Chiitoo
Copy link
Contributor

Chiitoo commented Nov 16, 2021

Haven't managed to go over this yet, but it will hopefully happen sooner rather than later.

Thanks!

@Chiitoo
Copy link
Contributor

Chiitoo commented Dec 1, 2021

I finally got some testing done, but still got a bunch to do.

For now though, I'm wondering about 'xdg_icon_cache_update' a little. I've noticed it was removed from 'pkg_postrm()' in at least 'lxqt-config', and where it was added, it's only included in 'pkg_postinst()'. Is it really not needed in postrm?

@Chiitoo
Copy link
Contributor

Chiitoo commented Dec 2, 2021

Also, indeed, CMake will complain if some of the dependencies are missing, even if there's no obvious need for them (if they really are unnecessary, an upstream PR for removing them would be nice).

One such example is 'lxqt-config', which seems to still be missing 'kwindowsystem' now. It is pulled in by several of the other packages, but we should probably keep it here as well.

@AdelKS
Copy link
Contributor Author

AdelKS commented Dec 5, 2021

I finally got some testing done, but still got a bunch to do.

Awesome!

For now though, I'm wondering about 'xdg_icon_cache_update' a little. I've noticed it was removed from 'pkg_postrm()' in at least 'lxqt-config', and where it was added, it's only included in 'pkg_postinst()'. Is it really not needed in postrm?

I must admit that I did not read further about it and just followed what the QA Notice suggests. I will have a deeper look at the difference.

One such example is 'lxqt-config', which seems to still be missing 'kwindowsystem' now. It is pulled in by several of the other packages, but we should probably keep it here as well.

I just had a look at their CMakeLists.txt and kwindowsystem is not looked for ? I think every ebuild abides by what's looked for in their respective CMakeLists.txt. If you wish I can go through those again and see if I am not missing anything. qa-vdb in the other hand did not report any missing deps.

@Chiitoo
Copy link
Contributor

Chiitoo commented Dec 11, 2021

@AdelKS
Copy link
Contributor Author

AdelKS commented Dec 11, 2021

Thanks @Chiitoo for your input. I will PR a change to remove those deps upstream. Anything else ?

Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
Signed-off-by: Adel KARA SLIMANE <adel.ks@zegrapher.com>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2022-01-19 12:13 UTC
Newest commit scanned: 2bd6e45
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/e3f3add00c/output.html

@Chiitoo
Copy link
Contributor

Chiitoo commented Jan 19, 2022

I knew that the Gentoo devs are mainly shorthanded and I wanted to help: I understand that this may have been counterproductive. My initial thoughts is that reviewing takes less time than actually doing the changes yourself, but things don't go twice as fast.

I think others are much better at it than I am. I don't have a good workflow in place yet, especially for reviewing changes others did (this is the first time for something this big). For 9999s and such where we have diffs to old files instead of completely new files, changes are easy to see right away, but for new files I would manually diff to the previous versions while going through the commits using 'git rebase', which probably isn't too optimal.

Still though, the worst part is me just being very sleepy all the time.

On the other hand, I plan to stick around, so this is training for me and I'll make better and better changes to the point where I won't need reviewing and then I can help better. If this is what you want, I will not version bump this project again.

That is good to read, and who knows, perhaps you'll have git access to ::gentoo before I do, and will like to join the Gentoo LXQt project and then you can do the whole thing yourself. :]

At this time I don't think I would say no to another pull request, especially if it seems to be taking a while for me to get to it (I do sometimes forget things), but it would be best to ask before putting time into it in case I'm already working on it as well.

Thanks for your time!

I thank you!

@AdelKS
Copy link
Contributor Author

AdelKS commented Jan 19, 2022

Still though, the worst part is me just being very sleepy all the time.

I understand, wish you all the best x)

It would be best to ask before putting time into it in case I'm already working on it as well.

Absolutely, I was sure I am working on it before anyone else as I opened this PR on release (or super close) haha. Next bumps I will ask first now that I know.

For new files I would manually diff to the previous versions while going through the commits using 'git rebase', which probably isn't too optimal.

If I were you, I'd checkout my branch at HEAD and diff the 1.0.0 ebuilds with the untouched 0.17 ones 🤔 vscode has an extension to diff simply between two opened files and display that in the IDE (not command line).

PS: we got feedback on qps lxqt/qps#349

@thesamesam
Copy link
Member

Thank you @Chiitoo and @AdelKS!

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
5 participants