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

support webp gnome system-wide when is requested #14131

Closed
wants to merge 2 commits into from
Closed

support webp gnome system-wide when is requested #14131

wants to merge 2 commits into from

Conversation

okias
Copy link
Contributor

@okias okias commented Dec 26, 2019

Links to open bugs in Closes: inside commits

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @okias
Areas affected: ebuilds
Packages affected: x11-libs/gdk-pixbuf, x11-libs/gdk-pixbuf-loader-webp

x11-libs/gdk-pixbuf: @gentoo/gnome
x11-libs/gdk-pixbuf-loader-webp: @andkit, @gentoo/graphics, @gentoo/proxy-maint

Linked bugs

Bugs linked: 703864, 703866, 693062


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 Dec 26, 2019
@okias
Copy link
Contributor Author

okias commented Mar 11, 2020

recreate merge request with x11-libs/gdk-pixbuf-loader-webp 0.0.1 (freshly released)

# Copyright 1999-2019 Gentoo Authors
# Distributed under the terms of the GNU General Public License v2

EAPI=6
Copy link
Member

@a17r a17r Mar 19, 2020

Choose a reason for hiding this comment

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

no EAPI-7 love? this is in conflict with your git summary.


LICENSE="LGPL-2+"
SLOT="0"
KEYWORDS="~alpha ~amd64 ~arm ~arm64 ~hppa ~ia64 ~mips ~ppc ~ppc64 ~s390 ~sh ~sparc ~x86 ~amd64-linux ~x86-linux ~ppc-macos ~x64-macos ~x86-macos ~sparc-solaris ~sparc64-solaris ~x64-solaris ~x86-solaris"
Copy link
Member

Choose a reason for hiding this comment

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

where are all these keywords coming from? who tested those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

from original package

Copy link
Member

Choose a reason for hiding this comment

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

I don't see that:

Keywords for x11-libs/gdk-pixbuf-loader-webp:
               |                               |   u   |  
               | a   a     p           s a r   |   n   |  
               | m   r i   p   h m s   p l i m | e u s | r
               | d a m a p c x p 6 3   a p s i | a s l | e
               | 6 r 6 6 p 6 8 p 8 9 s r h c p | p e o | p
               | 4 m 4 4 c 4 6 a k 0 h c a v s | i d t | o
---------------+-------------------------------+-------+-------
20160328234507 | o o o o o o o o o o o o o o o | 6 o 0 | gentoo

PATCHES=( "${FILESDIR}/web-pixbuf-loader_gentoo.patch" )

DEPEND="
>=media-libs/libwebp-0.4.3[${MULTILIB_USEDEP}]
Copy link
Member

Choose a reason for hiding this comment

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

looks like a slot operator candidate.

KEYWORDS="~alpha ~amd64 ~arm ~arm64 ~hppa ~ia64 ~mips ~ppc ~ppc64 ~s390 ~sh ~sparc ~x86 ~amd64-linux ~x86-linux ~ppc-macos ~x64-macos ~x86-macos ~sparc-solaris ~sparc64-solaris ~x64-solaris ~x86-solaris"

S="${WORKDIR}/${MY_PN}-${COMMIT}"
PATCHES=( "${FILESDIR}/web-pixbuf-loader_gentoo.patch" )
Copy link
Member

@a17r a17r Mar 19, 2020

Choose a reason for hiding this comment

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

weird to find PATCHES above dependencies.

@@ -0,0 +1,54 @@
# Copyright 1999-2019 Gentoo Authors
Copy link
Member

Choose a reason for hiding this comment

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

Invalid copyright, don't you use repoman?

Copy link
Contributor Author

@okias okias Mar 19, 2020

Choose a reason for hiding this comment

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

I made it (and also pushed and created MR) in 2019...

Copy link
Member

Choose a reason for hiding this comment

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

How should I understand this then?

recreate merge request with x11-libs/gdk-pixbuf-loader-webp 0.0.1 (freshly released)

should this be closed in favor of a different PR?

@a17r a17r requested a review from leio March 19, 2020 13:39
@okias
Copy link
Contributor Author

okias commented Mar 19, 2020

if you find time to merge it, I'll rebase & fix it.

@a17r
Copy link
Member

a17r commented Mar 19, 2020

don't you have that backwards?

@okias
Copy link
Contributor Author

okias commented Mar 19, 2020

@a17r Not really, since it was rotting here (and in two bugzilla entries) for 3 months until you looked at it... I willing to make it in acceptable state, but only if there is chance to get it merged (to have benefits for users wanting use webp inside gnome or other GTK apps).

@a17r
Copy link
Member

a17r commented Mar 19, 2020

I'm spending my free time here as well for a package I will likely never use, please understand that @gentoo/gnome must ack before I am able to merge.

!<gnome-base/librsvg-2.31.0
!<x11-libs/gtk+-2.21.3:2
!<x11-libs/gtk+-2.90.4:3
webp? ( x11-libs/gdk-pixbuf-loader-webp:0=[${MULTILIB_USEDEP}] )
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC QA team was fighting against optional runtime dependencies. Have things changed? Also shouldn't it be PDEPEND to prevent circular dependency?

Copy link
Contributor

Choose a reason for hiding this comment

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

And slot-operator is not needed here.

@mattst88
Copy link
Contributor

mattst88 commented Mar 19, 2020

@okias, you are right to be annoyed that your PRs sit unreviewed by @gentoo/gnome, but that isn't @a17r's fault and he's /helping/ you here.

I'm on @gentoo/gnome and I'm willing to commit your PRs (in fact, I have recently; and I would love to see more from you!).

@okias
Copy link
Contributor Author

okias commented Mar 19, 2020

@mattst88 of course, I just expressed that code was laying for long time, nothing against @a17r ! :) If this PR is generally ok, I'll polish it. Just give me few days.

@mattst88
Copy link
Contributor

Thanks :)

@okias
Copy link
Contributor Author

okias commented Mar 30, 2020

 * Updating gdk-pixbuf loader cache ...
/var/tmp/portage/gui-libs/webp-pixbuf-loader-0.0.1/temp/environment: line 1564: emktemp: command not found
/var/tmp/portage/gui-libs/webp-pixbuf-loader-0.0.1/temp/environment: line 1565: : No such file or directory

it's local tmp_file=$(emktemp);
any idea what can be wrong?

@juippis
Copy link
Member

juippis commented Apr 1, 2020

 * Updating gdk-pixbuf loader cache ...
/var/tmp/portage/gui-libs/webp-pixbuf-loader-0.0.1/temp/environment: line 1564: emktemp: command not found
/var/tmp/portage/gui-libs/webp-pixbuf-loader-0.0.1/temp/environment: line 1565: : No such file or directory

it's local tmp_file=$(emktemp);
any idea what can be wrong?

Without looking into this PR much, this error is because eutils.eclass isn't inherited.

@okias
Copy link
Contributor Author

okias commented Apr 7, 2020

ready for merge :) EAPI must be 6, since it does use gnome2_gdk_pixbuf_update which is broken in EAPI 7.
Otherwise in great shape.

@okias okias requested a review from a17r April 7, 2020 19:14
@okias
Copy link
Contributor Author

okias commented Apr 10, 2020

Ready to review.


LICENSE="LGPL-2+"
SLOT="0"
KEYWORDS="~alpha ~amd64 ~arm ~arm64 ~hppa ~ia64 ~mips ~ppc ~ppc64 ~s390 ~sparc ~x86 ~amd64-linux ~x86-linux"
Copy link
Member

Choose a reason for hiding this comment

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

did you test all those arches? again, the current ebuild has none at all.

Keywords for x11-libs/gdk-pixbuf-loader-webp:
               |                             |   u   |  
               | a   a     p         s a r   |   n   |  
               | m   r i   p   h m s p l i m | e u s | r
               | d a m a p c x p 6 3 a p s i | a s l | e
               | 6 r 6 6 p 6 8 p 8 9 r h c p | p e o | p
               | 4 m 4 4 c 4 6 a k 0 c a v s | i d t | o
---------------+-----------------------------+-------+-------
20160328234507 | o o o o o o o o o o o o o o | 6 o 0 | gentoo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in v3. Added intersection between libwebp & gdk-pixbuf . Since it's just glue code.
I cannot test all arches, but this could be valid point to assume it will work on these arches, right?

Is it ok with you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@a17r ?

x11-libs/gdk-pixbuf/gdk-pixbuf-2.40.0-r1.ebuild Outdated Show resolved Hide resolved
- use correct upstream name
- move from x11-libs to gui-libs
- migrate to meson

Closes: https://bugs.gentoo.org/693062
Closes: https://bugs.gentoo.org/703864

v2:
 - EAPI=6 must be preserved due to gnome2_gdk_pixbuf_update
   incompatible with EAPI=7
v3:
 - matched keyword with libwebp U gdk-pixbuf-loader

Signed-off-by: David Heidelberg <david@ixit.cz>
Since webp is widely used and we have available gdk-pixbuf wrapper,
let's use it.

v2:
 - drop sh architecture
 - update webp package name
v3:
 - fix DEPEND (thanks @a17r )
 - update homepage

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

Signed-off-by: David Heidelberg <david@ixit.cz>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-04-17 16:09 UTC
Newest commit scanned: 25c86f4
Status: ❌ broken

New issues caused by PR:
https://qa-reports.gentoo.org/output/gentoo-ci/d2d58c5/output.html#www-apps/postfixadmin

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/d2d58c5/output.html

@okias okias closed this Apr 29, 2020
@okias
Copy link
Contributor Author

okias commented Apr 29, 2020

Pushed into ::guru overlay for this moment.

@leio
Copy link
Member

leio commented Apr 29, 2020

@gentoo/gnome does not maintain x11-libs/gdk-pixbuf-loader-webp, gdk-pixbuf shouldn't be changed like that, at least not so fast.

@Arfrever
Copy link
Contributor

 * Updating gdk-pixbuf loader cache ...
/var/tmp/portage/gui-libs/webp-pixbuf-loader-0.0.1/temp/environment: line 1564: emktemp: command not found
/var/tmp/portage/gui-libs/webp-pixbuf-loader-0.0.1/temp/environment: line 1565: : No such file or directory

it's local tmp_file=$(emktemp);
any idea what can be wrong?

This problem and other problems in gnome2-utils.eclass were reported in https://bugs.gentoo.org/694012.
gnome2-utils.eclass was fixed in 410a967.
So EAPI="7" can now be used in ebuilds using gnome2-utils.eclass.

@okias
Copy link
Contributor Author

okias commented Apr 29, 2020

@Arfrever thank you, updated in ::guru

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