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

media-gfx/nomacs: 3.14.2 version bump. Add plugins support. #15649

Closed
wants to merge 1 commit into from

Conversation

band-a-prend
Copy link
Contributor

@band-a-prend band-a-prend commented May 5, 2020

The updated nomacs ebuild contains:

  • replace xdg-utils with xdg eclass;
  • add 'plugins' USE-flag with appropriate SRC_URI variable,
    src_unpack() function and changes within src_configure() function;
  • add src_prepare() section as there is no PATCHES array within ebuild.

Unfortunately the HEIF support is also required heif-plugin [1] that is not in portage tree.

[1] https://github.com/jakar/qt-heif-image-plugin

Closes: https://bugs.gentoo.org/712918

Update:
- Add : new package media-plugins/qt-heif-image-plugin for HEIF file format suppport.
- Update media-gfx/nomacs-3.14.2.ebuild with heif USE-flag
to install RDEPEND media-plugins/qt-heif-image-plugin if enabled.

Decide to drop direct RDEPEND heif support in media-gfx/nomacs via heif USE-flag because the heif plugin is currently compatible only with media-libs/libheif:0/1.6 and this could prevent nomacs stabilisation process and add additional bugs.

Addition of plugins USE-flag is mainly useful for PaintPlugin.

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @band-a-prend
Areas affected: ebuilds
Packages affected: media-gfx/nomacs

media-gfx/nomacs: @gentoo/qt

Linked bugs

Bugs linked: 712918


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). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels May 5, 2020
@band-a-prend
Copy link
Contributor Author

Could I add ebuild media-plugins/qt-heif-image-plugin under gentoo-qt project co-maintainig instead of proxy-maint?

@Chiitoo
Copy link
Contributor

Chiitoo commented May 5, 2020

Could I add ebuild media-plugins/qt-heif-image-plugin under gentoo-qt project co-maintainig instead of proxy-maint?

We are quite understaffed unfortunately, but as long as we're on 'media-gfx/nomacs', I guess it could make sense, and I personally don't have anything against this. That being said, I still can't push things directly to the main Gentoo ebuild repository myself, so someone will in the end proxy for me as well... so I can't really go 100% and say "go for it".

@Pesa
Copy link
Contributor

Pesa commented May 5, 2020

Could I add ebuild media-plugins/qt-heif-image-plugin under gentoo-qt project co-maintainig instead of proxy-maint?

makes sense to me

Copy link
Contributor

@Pesa Pesa left a comment

Choose a reason for hiding this comment

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

This plugin is currently in progress for inclusion in qtimageformats.
See the Qt Gerrit[1] page or bug report[2] for updates.

This effort seems to have stalled?

if use plugins ; then
einfo "Unpacking ${PN}-plugins-$(ver_cut 1-2).tar.gz to /var/tmp/portage/sci-physics/${P}/work/${P}/ImageLounge/plugins"
mkdir "${P}"/ImageLounge/plugins
tar -C "${P}"/ImageLounge/plugins --strip-components=1 -xzf "${DISTDIR}/${PN}-plugins-$(ver_cut 1-2).tar.gz" || die
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't manually invoke tar, see https://devmanual.gentoo.org/ebuild-writing/functions/src_unpack/index.html

Just let portage do the unpacking and then use mv if needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This plugin library is fully runtime dependency. It works without re-compilation of nomacs for nomacs-3.12 too and for lximage-qt.

cmake option ENABLE_HEIF is not used actually for linux build and with sed command is applied in ebuild to avoid misleading in notification during configuration process.

Should I create COMMON_DEPS variable and move to it all stuff without this plugin and then add this new variable to RDEPEND and DEPEND?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced with conditional unpack call.

I also fix installation and search nomacs-plugin directory in src_prepare() section:

if use plugins ; then
	# Fix nomacs-plugins installation and search library directory
	sed -i "s:lib/nomacs-plugins:$(get_libdir)/nomacs-plugins:" "${S}/plugins/cmake/Utils.cmake" || die
	sed -i "s:lib/nomacs-plugins:$(get_libdir)/nomacs-plugins:" "${S}/src/DkCore/DkPluginManager.cpp" || die
fi

to avoid installation of plugins into /usr/lib for amd64 arch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move early RDEPEND to COMMON_DEPEND variable with placement of media-plugins/qt-heif-image-plugin into RDEPEND without placement into DEPEND.

RDEPEND="
	${COMMON_DEPEND}
	heif? ( media-plugins/qt-heif-image-plugin )
"
DEPEND="${COMMON_DEPEND}"


inherit cmake

DESCRIPTION="qt-heif-image-plugin: Qt plugin for HEIF images"
Copy link
Contributor

Choose a reason for hiding this comment

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

delete "qt-heif-image-plugin:" from the description

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

Comment on lines 18 to 20
This has been successfully used with the following:
<pkg>media-gfx/lximage-qt</pkg>
<pkg>media-gfx/nomacs</pkg>
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if I'd include this, it will eventually become outdated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

@band-a-prend
Copy link
Contributor Author

This effort seems to have stalled?

I don't know exactly. This info is from project page. Some recent activity was on march 2020 but the progress is really slow.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-05-06 13:06 UTC
Newest commit scanned: d1c9f8e
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/fb01342f97/output.html

@band-a-prend
Copy link
Contributor Author

This effort seems to have stalled?

@Pesa,
because of last release was in 2018 and it unknown later progress I'm now deep in thought is it worth to place this exclusively runtime plugin in main portage tree or it's better to place in gentoo guru project? Especially because of minor popularity of HEIF format in my view. In the case of guru repo the 2nd and 3rd could be completely be dropped.

@band-a-prend
Copy link
Contributor Author

I finally decide to remove heif USE-flag. Please see the Update part main pull request message for details.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-05-10 21:50 UTC
Newest commit scanned: 94cbe8b
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/03506e1cfe/output.html

@Pesa
Copy link
Contributor

Pesa commented May 10, 2020

Decide to drop direct RDEPEND heif support in media-gfx/nomacs via heif USE-flag because the heif plugin is currently compatible only with media-libs/libheif:0/1.6 and this could prevent nomacs stabilisation process and add additional bugs.

That's not really a valid concern. If we need to stabilize a newer version of libheif, we'll do it, I don't see the problem...

HOMEPAGE="https://nomacs.org/"
SRC_URI="
https://github.com/${PN}/${PN}/archive/${PV}.tar.gz -> ${P}.tar.gz
plugins? ( https://github.com/${PN}/${PN}-plugins/archive/${PV}.tar.gz -> ${PN}-plugins-$(ver_cut 1-2).tar.gz )
Copy link
Contributor

Choose a reason for hiding this comment

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

why not PV instead of ver_cut ...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because of plugins this time has version 3.14 while nomacs has 3.14.2.

Copy link
Contributor

Choose a reason for hiding this comment

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

then you have to adjust the tarball URL, not the local archive name (the part after the arrow). Did you even test this stuff? If the plugins version is 3.14, this code is broken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry. I initially made ebuild for 3.14 and then remember that 3.14.2 was released so during repoman manifest the plugin tarball initially downloaded for nomacs-3.14.ebuild.
And I then forgot to update the URL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Repaired.

raw? ( media-libs/libraw:= )
tiff? (
dev-qt/qtimageformats:5
media-libs/tiff:0
Copy link
Contributor

Choose a reason for hiding this comment

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

why :0 here? seems unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

dev-qt/qtimageformats:5
media-libs/tiff:0
)
zip? ( dev-libs/quazip[qt5(+)] )
Copy link
Contributor

Choose a reason for hiding this comment

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

The qt5 flag is long gone from quazip, it's time to drop the USE dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed use flag.


src_unpack() {
unpack "${P}.tar.gz"
use plugins && unpack "${PN}-plugins-$(ver_cut 1-2).tar.gz" && mv "${PN}-plugins-$(ver_cut 1-2)" "${S}/plugins"
Copy link
Contributor

Choose a reason for hiding this comment

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

don't use such a long list of &&, split the two commands and put them inside an if use plugins;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 81 to 91
pkg_postinst() {
xdg_pkg_postinst
}

pkg_postrm() {
xdg_pkg_postrm
}
Copy link
Contributor

Choose a reason for hiding this comment

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

aren't these redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, as nomacs doesn't install anything to /usr/share/icons then there is no need to update icon cache. I need to revert it to use xdg_desktop_database_update from xdg-utils.eclass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted.

@band-a-prend
Copy link
Contributor Author

I don't see the problem...

I'm afraid as the plugin (v 0.3.3 of Sep 2018) README.md file states it compatible for >=libheif-1.2, already doesn't work with libheif-1.5.1 and accidentally work again with libheif-1.6.1 it could become broken again for next major release of libheif.

So I prefer to place it in guru repository and to avoid affect with issues the media-gfx/nomacs package directly this case.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-05-11 22:41 UTC
Newest commit scanned: 0f5697d
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/0db848e321/output.html

src_unpack() {
unpack "${P}.tar.gz"
if use plugins ; then
unpack "${PN}-plugins-$(ver_cut 1-2).tar.gz"
Copy link
Contributor

Choose a reason for hiding this comment

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

${PN}-plugins-$(ver_cut 1-2) is repeated at least 3 times, use a variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced with PLUGIN_PKG="${PN}-plugins-$(ver_cut 1-2)" variable.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-05-12 00:06 UTC
Newest commit scanned: 094aa0e
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/db3cc5f582/output.html

@band-a-prend
Copy link
Contributor Author

@Pesa

@band-a-prend
Copy link
Contributor Author

@Pesa , ping!
Are any additional changes required before merge request?

Copy link
Contributor

@Pesa Pesa left a comment

Choose a reason for hiding this comment

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

}

src_prepare() {
default
Copy link
Contributor

Choose a reason for hiding this comment

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

do you need to run both default and cmake_src_prepare?

Copy link
Member

Choose a reason for hiding this comment

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

cmake_src_prepare must be run instead of default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I conditionally modify .cmake file so it seems I need run cmake_src_prepare after applied changes, isn't it?

default is removed.

Copy link
Member

Choose a reason for hiding this comment

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

no, cmake is not autotools ;)

I recommend to run cmake_src_prepare before modifying sources so that any patches continue to apply to vanilla source.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, cmake_src_prepare is moved to the top of src_prepare()

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-06-16 11:01 UTC
Newest commit scanned: 0415559
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/d4aea8d4b2/output.html

@Chiitoo
Copy link
Contributor

Chiitoo commented Jun 16, 2020

Can't think of anything to add.

Quick build-tests come out good too.

Thanks!

The updated nomacs ebuild contains:
- add 'plugins' USE-flag with appropriate SRC_URI variable,
  and changes within src_unpack() and src_configure() functions;
- add src_prepare() section as there is no PATCHES array in ebuild
  with fix for nomacs-plugins intallation and search path.

Closes: https://bugs.gentoo.org/712918

Signed-off-by: Sergey Torokhov <torokhov-s-a@yandex.ru>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-06-16 13:02 UTC
Newest commit scanned: c9c19fc
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/2a685005de/output.html

@a17r
Copy link
Member

a17r commented Jun 16, 2020

thanks!

NeddySeagoon pushed a commit to NeddySeagoon/gentoo-arm64 that referenced this pull request Jun 19, 2020
The updated nomacs ebuild contains:
- add 'plugins' USE-flag with appropriate SRC_URI variable,
  and changes within src_unpack() and src_configure() functions;
- add src_prepare() section as there is no PATCHES array in ebuild
  with fix for nomacs-plugins intallation and search path.

Closes: https://bugs.gentoo.org/712918

Signed-off-by: Sergey Torokhov <torokhov-s-a@yandex.ru>
Closes: gentoo#15649
Signed-off-by: Andreas Sturmlechner <asturm@gentoo.org>
@band-a-prend band-a-prend deleted the nomacs branch June 25, 2020 17:26
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). bug linked Bug/Closes found in footer, and cross-linked with the PR.
Projects
None yet
6 participants