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

qutebrowser: update to 2.0.1 #28286

Closed
wants to merge 1 commit into from
Closed

qutebrowser: update to 2.0.1 #28286

wants to merge 1 commit into from

Conversation

Byl3x
Copy link
Contributor

@Byl3x Byl3x commented Jan 28, 2021

General

Have the results of the proposed changes been tested?

  • I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
  • I generally don't use the affected packages but briefly tested this PR

@ericonr
Copy link
Member

ericonr commented Jan 28, 2021

Please add checkdepends="$depends" to the template.

@Byl3x
Copy link
Contributor Author

Byl3x commented Jan 28, 2021

I don't understand, my native arch is x86_64 and building it in a vm(that didn't have the previous version or anything related) with xbps-src works too

@ericonr
Copy link
Member

ericonr commented Jan 28, 2021

This is running the tests, which you can do by running ./xbps-src -Q pkg qutebrowser.

Also, please squash the commits.

revision=1
build_style=python3-module
hostmakedepends="python3-setuptools asciidoc"
depends="python3-PyQt5-quick python3-Jinja2 python3-Pygments python3-pyPEG2
python3-yaml python3-attrs python3-PyQt5-opengl python3-PyQt5-sql
qt5-plugin-sqlite python3-setuptools"
checkdepends="$depends"
Copy link
Member

Choose a reason for hiding this comment

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

try adding python3-pytest

@Byl3x
Copy link
Contributor Author

Byl3x commented Jan 28, 2021

Well the tests are causing the issue and when I look at the build logs for the older versions they skipped the tests, so I'm wondering if the previous versions passed the tests, I'll try it
edit: Checks also fail with 1.14.1_1 on x86_64

@folliehiyuki
Copy link
Contributor

Should we do something to #27490 too? qutebrowser 2.0.0 comes with the adblocking feature

@pbui
Copy link
Contributor

pbui commented Jan 28, 2021

2.0.1 has been released: https://github.com/qutebrowser/qutebrowser/releases/tag/v2.0.1

@Byl3x
Copy link
Contributor Author

Byl3x commented Jan 28, 2021

So I tried running the test and found out that the test needs a running Xorg server to function(it displays a window) and it is also deprecated, I'm not sure how to change the build so that it doesn't run the tests

@ericonr
Copy link
Member

ericonr commented Jan 29, 2021

Please apply this patch:

diff --git a/srcpkgs/qutebrowser/template b/srcpkgs/qutebrowser/template
index c80d32792f..4c160a1adf 100644
--- a/srcpkgs/qutebrowser/template
+++ b/srcpkgs/qutebrowser/template
@@ -7,7 +7,7 @@ hostmakedepends="python3-setuptools asciidoc"
 depends="python3-PyQt5-quick python3-Jinja2 python3-Pygments python3-pyPEG2
  python3-yaml python3-attrs python3-PyQt5-opengl python3-PyQt5-sql
  qt5-plugin-sqlite python3-setuptools"
- checkdepends="$depends python3-pytest"
+checkdepends="$depends python3-pytest xvfb-run"
 short_desc="Keyboard-focused browser with a minimal GUI"
 maintainer="Érico Nogueira <ericonr@disroot.org>"
 license="GPL-3.0-or-later"
@@ -37,6 +37,12 @@ pre_build() {
 	a2x -f manpage doc/qutebrowser.1.asciidoc
 }
 
+do_check() {
+	# testing with pytest would be nice, but requires unpackaged plugins:
+	# pytest-bdd, pytest-benchmark, pytest-instafail, pytest-mock, pytest-qt, pytest-rerunfailures
+	:
+}
+
 post_install() {
 	vman doc/qutebrowser.1
 	vinstall misc/org.qutebrowser.qutebrowser.desktop 644 usr/share/applications

Copy link
Member

@ahesford ahesford left a comment

Choose a reason for hiding this comment

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

In addition to bumping to the fresh 2.0.1, you need to go through the Quick checklist for packages provided by the project. The dependencies need to be brought in line with upstream recommendation.

Regarding python3-Pygments becoming optional, it would be nice to know where in qutebrowser it is used so we can decide if it should be retained as a dependency of this package.

@ahesford
Copy link
Member

ahesford commented Jan 29, 2021

+do_check() {
+	# testing with pytest would be nice, but requires unpackaged plugins:
+	# pytest-bdd, pytest-benchmark, pytest-instafail, pytest-mock, pytest-qt, pytest-rerunfailures
+	:
+}

@ericonr We do provide python3-pytest-mock. As for the others, are the tests that depend on them so numerous that the entire check should be skipped, or would it be feasible to --ignore or --ignore-glob the affected tests?

@ericonr
Copy link
Member

ericonr commented Jan 29, 2021

@ahesford we also provide pytest-qt, fwiw.

The issue is that the tests seem to check for these plugins at startup, and it complains immediatelly about them not being there, instead of being runtime failures.

@Byl3x Byl3x changed the title qutebrowser: update to 2.0.0 qutebrowser: update to 2.0.1 Jan 29, 2021
@Byl3x
Copy link
Contributor Author

Byl3x commented Jan 29, 2021

@ahesford python3-Pygments is used for syntax highlighting when viewing page source(only with :view-source --pygments or on WebKit) and also when formatting JSON files

@ahesford
Copy link
Member

Syntax highlighting of source and JSON seems worth keeping the Pygments dependency. The other obsolete dependencies still need to be dropped.

@Byl3x
Copy link
Contributor Author

Byl3x commented Jan 29, 2021

I checked the dependencies, only python3-pyPEG2 shouldn't be required. Also the python3-Pygments are only used if you choose to use :view-source with the --pygments flag or when using a userscript that is in the official repo or when you use QtWebKit instead of QtWebEngine

@ahesford
Copy link
Member

From the list:

Remove the attrs (attr) dependency.

Move the setuptools dependency from runtime (for pkg_resources) to build-time.

If the Pygments support really requires a --pygments flag (it doesn't in the current version---:view-source is definitely colored for me), then I would drop its requirement here, since having to explicitly opt in would mean that syntax coloring isn't the expected default behavior.

@Byl3x
Copy link
Contributor Author

Byl3x commented Jan 29, 2021

I didn't notice the attrs dependency removal, sorry. What I meant with the Pygments also was that to use it you have to do :view-source --pygments so I think that it's safe to drop the requirement

@paper42
Copy link
Member

paper42 commented Jan 29, 2021

python3-pyPEG2 isn't used anywhere else, so it should be safe to drop the package (upstream dead)

qt5-plugin-sqlite python3-setuptools"
depends="python3-PyQt5-quick python3-Jinja2 python3-yaml python3-PyQt5-opengl python3-PyQt5-sql
qt5-plugin-sqlite"
checkdepends="$depends python3-pytest xvfb-run"
Copy link
Member

Choose a reason for hiding this comment

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

If you're skipping tests, don't add checkdepends.

add checkdepends line

add python3-pytest

update checksum, bump version to 2.0.1

add do_check() patch

remove excess line

remove unneeded dependencies

remove checkdepends
Copy link
Member

@ahesford ahesford left a comment

Choose a reason for hiding this comment

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

This update is working for me.

@paper42
Copy link
Member

paper42 commented Jan 31, 2021

What do you think about adding pdf.js as a dependency. It's an optional dependency which allows showing PDF files inside the browser (Ctrl+p in the download dialog).

@ahesford
Copy link
Member

I wouldn't add that dependency. When the browser presents the option to display PDFs, I think it is pretty clear that it wants pdf.js, so users should know where to look if they want to enable support.

@ahesford ahesford closed this in ce5ab1f Feb 1, 2021
uw2021 pushed a commit to uw2021/void-packages that referenced this pull request Feb 4, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants