From 1c76f121a2698deb0b7ff9f37940cc7b9f0a10f8 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 6 Sep 2016 18:03:53 +0200 Subject: [PATCH 1/7] userscripts: fix FIFO on Windows The userscript FIFO on Windows suffered the same problem that open-editor once did, because files on Windows can't be opened with write access by two different processes. We kept the oshandle around and only closed it when the process exited, which means that userscripts could not actually write any commands to the FIFO. This patch closes the file earlier, allowing the userscript to actually write commands to it. See also https://lists.schokokeks.org/pipermail/qutebrowser/2016-September/000256.html --- qutebrowser/commands/userscripts.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index 0a8b19524..65af02609 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -280,14 +280,10 @@ class _WindowsUserscriptRunner(_BaseUserscriptRunner): This also means the userscript *has* to use >> (append) rather than > (overwrite) to write to the file! - - Attributes: - _oshandle: The oshandle of the temp file. """ def __init__(self, win_id, parent=None): super().__init__(win_id, parent) - self._oshandle = None def _cleanup(self): """Clean up temporary files after the userscript finished.""" @@ -301,12 +297,7 @@ class _WindowsUserscriptRunner(_BaseUserscriptRunner): except OSError: log.procs.exception("Failed to read command file!") - try: - os.close(self._oshandle) - except OSError: - log.procs.exception("Failed to close file handle!") super()._cleanup() - self._oshandle = None self.finished.emit() @pyqtSlot() @@ -323,7 +314,9 @@ class _WindowsUserscriptRunner(_BaseUserscriptRunner): self._kwargs = kwargs try: - self._oshandle, self._filepath = tempfile.mkstemp(text=True) + handle = tempfile.NamedTemporaryFile(delete=False) + handle.close() + self._filepath = handle.name except OSError as e: message.error(self._win_id, "Error while creating tempfile: " "{}".format(e)) From 0ab1902c98d667f36e1eaa892c620e0a0004df87 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 6 Sep 2016 18:41:24 +0200 Subject: [PATCH 2/7] add an userscript test for Windows --- tests/end2end/data/userscripts/open_current_url.bat | 1 + tests/end2end/features/spawn.feature | 8 ++++++++ 2 files changed, 9 insertions(+) create mode 100644 tests/end2end/data/userscripts/open_current_url.bat diff --git a/tests/end2end/data/userscripts/open_current_url.bat b/tests/end2end/data/userscripts/open_current_url.bat new file mode 100644 index 000000000..60d72d594 --- /dev/null +++ b/tests/end2end/data/userscripts/open_current_url.bat @@ -0,0 +1 @@ +echo open -t %QUTE_URL% >> "%QUTE_FIFO%" \ No newline at end of file diff --git a/tests/end2end/features/spawn.feature b/tests/end2end/features/spawn.feature index 2d43b011b..76ef5620a 100644 --- a/tests/end2end/features/spawn.feature +++ b/tests/end2end/features/spawn.feature @@ -40,3 +40,11 @@ Feature: :spawn Then the following tabs should be open: - about:blank - about:blank (active) + @windows + Scenario: Running :spawn with userscript on Windows + When I open about:blank + And I run :spawn -u (testdata)/userscripts/open_current_url.bat + And I wait until about:blank is loaded + Then the following tabs should be open: + - about:blank + - about:blank (active) \ No newline at end of file From 22ac19b15107969797be52418ca0054248797d4c Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 6 Sep 2016 20:33:48 +0200 Subject: [PATCH 3/7] style fixes --- qutebrowser/commands/userscripts.py | 3 --- tests/end2end/data/userscripts/open_current_url.bat | 2 +- tests/end2end/features/spawn.feature | 1 + 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index 65af02609..a8d741f6c 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -282,9 +282,6 @@ class _WindowsUserscriptRunner(_BaseUserscriptRunner): (overwrite) to write to the file! """ - def __init__(self, win_id, parent=None): - super().__init__(win_id, parent) - def _cleanup(self): """Clean up temporary files after the userscript finished.""" if self._cleaned_up: diff --git a/tests/end2end/data/userscripts/open_current_url.bat b/tests/end2end/data/userscripts/open_current_url.bat index 60d72d594..e57eb2567 100644 --- a/tests/end2end/data/userscripts/open_current_url.bat +++ b/tests/end2end/data/userscripts/open_current_url.bat @@ -1 +1 @@ -echo open -t %QUTE_URL% >> "%QUTE_FIFO%" \ No newline at end of file +echo open -t %QUTE_URL% >> "%QUTE_FIFO%" diff --git a/tests/end2end/features/spawn.feature b/tests/end2end/features/spawn.feature index 76ef5620a..e2d3fe3c2 100644 --- a/tests/end2end/features/spawn.feature +++ b/tests/end2end/features/spawn.feature @@ -40,6 +40,7 @@ Feature: :spawn Then the following tabs should be open: - about:blank - about:blank (active) + @windows Scenario: Running :spawn with userscript on Windows When I open about:blank From f6c6f766cdfe08a1cfdb0cf863b8e3d314116432 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 8 Sep 2016 23:13:00 +0200 Subject: [PATCH 4/7] add newline at the end of the file --- tests/end2end/features/spawn.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/end2end/features/spawn.feature b/tests/end2end/features/spawn.feature index e2d3fe3c2..469120cc2 100644 --- a/tests/end2end/features/spawn.feature +++ b/tests/end2end/features/spawn.feature @@ -48,4 +48,4 @@ Feature: :spawn And I wait until about:blank is loaded Then the following tabs should be open: - about:blank - - about:blank (active) \ No newline at end of file + - about:blank (active) From b0114768c754a8ed598adc90fe725ae08ebfaa3f Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 8 Sep 2016 23:14:10 +0200 Subject: [PATCH 5/7] fix spawn tests on Windows On Windows, no echo.exe exists normally, so calling echo from the tests is no good idea, since it relies on Cygwin to be installed and in %PATH% (so that echo.exe is available). This fixes this by providing a small echo.bat which is callable from the tests, and then using a platform-specific path to the executable instead of the hardcoded "echo". This should ensure that the tests pass even on systems where echo.exe is not installed. Note that we can't simply use a do-nothing exe (like rundll or hh.exe), as we're passing parameters, and those executables may behave differently in the presence of those parameters. --- tests/end2end/data/userscripts/echo.bat | 6 ++++++ tests/end2end/features/conftest.py | 14 ++++++++++++++ tests/end2end/features/spawn.feature | 14 +++++++------- 3 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 tests/end2end/data/userscripts/echo.bat diff --git a/tests/end2end/data/userscripts/echo.bat b/tests/end2end/data/userscripts/echo.bat new file mode 100644 index 000000000..0f37ae2cd --- /dev/null +++ b/tests/end2end/data/userscripts/echo.bat @@ -0,0 +1,6 @@ +@echo off +rem This is needed because echo does not exist as an external program on +rem Windows, so we can't call echo(.exe) from qutebrowser, but it's useful for +rem tests. This little file is callable via :spawn and mimics (in a very naive +rem way) the echo command line utility. +echo %* diff --git a/tests/end2end/features/conftest.py b/tests/end2end/features/conftest.py index 100c15f0c..1dccf6d6e 100644 --- a/tests/end2end/features/conftest.py +++ b/tests/end2end/features/conftest.py @@ -19,6 +19,7 @@ """Steps for bdd-like tests.""" +import os import re import sys import time @@ -34,6 +35,18 @@ from qutebrowser.utils import log from helpers import utils +def get_echo_exe_path(): + """Return the path to an echo-like command, depending on the system. + + Return: + Path to the "echo"-utility. + """ + if sys.platform == "win32": + return os.path.join(utils.abs_datapath(), 'userscripts', 'echo.bat') + else: + return 'echo' + + @pytest.hookimpl(hookwrapper=True) def pytest_runtest_makereport(item, call): """Add a BDD section to the test output.""" @@ -215,6 +228,7 @@ def run_command(quteproc, httpbin, tmpdir, command): command = command.replace('(port)', str(httpbin.port)) command = command.replace('(testdata)', utils.abs_datapath()) command = command.replace('(tmpdir)', str(tmpdir)) + command = command.replace('(echo-exe)', get_echo_exe_path()) quteproc.send_cmd(command, count=count, invalid=invalid) diff --git a/tests/end2end/features/spawn.feature b/tests/end2end/features/spawn.feature index 469120cc2..5cdeac029 100644 --- a/tests/end2end/features/spawn.feature +++ b/tests/end2end/features/spawn.feature @@ -1,7 +1,7 @@ Feature: :spawn Scenario: Running :spawn - When I run :spawn -v echo "Hello" + When I run :spawn -v (echo-exe) "Hello" Then the message "Command exited successfully." should be shown Scenario: Running :spawn with command that does not exist @@ -19,18 +19,18 @@ Feature: :spawn Then the error "Error while splitting command: No closing quotation" should be shown Scenario: Running :spawn with url variable - When I run :spawn echo {url} - Then "Executing echo with args ['about:blank'], userscript=False" should be logged + When I run :spawn (echo-exe) {url} + Then "Executing * with args ['about:blank'], userscript=False" should be logged Scenario: Running :spawn with url variable in fully encoded format When I open data/title with spaces.html - And I run :spawn echo {url} - Then "Executing echo with args ['http://localhost:(port)/data/title%20with%20spaces.html'], userscript=False" should be logged + And I run :spawn (echo-exe) {url} + Then "Executing * with args ['http://localhost:(port)/data/title%20with%20spaces.html'], userscript=False" should be logged Scenario: Running :spawn with url variable in pretty decoded format When I open data/title with spaces.html - And I run :spawn echo {url:pretty} - Then "Executing echo with args ['http://localhost:(port)/data/title with spaces.html'], userscript=False" should be logged + And I run :spawn (echo-exe) {url:pretty} + Then "Executing * with args ['http://localhost:(port)/data/title with spaces.html'], userscript=False" should be logged @posix Scenario: Running :spawn with userscript From 101d30fe1e1204ea8db7f0cfaaa8f6bf94e4150d Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 11 Sep 2016 16:15:27 +0200 Subject: [PATCH 6/7] Make _get_echo_exe_path() private in bdd conftest --- tests/end2end/features/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/end2end/features/conftest.py b/tests/end2end/features/conftest.py index f035ac96c..03f082c22 100644 --- a/tests/end2end/features/conftest.py +++ b/tests/end2end/features/conftest.py @@ -36,7 +36,7 @@ from qutebrowser.browser import pdfjs from helpers import utils -def get_echo_exe_path(): +def _get_echo_exe_path(): """Return the path to an echo-like command, depending on the system. Return: @@ -236,7 +236,7 @@ def run_command(quteproc, httpbin, tmpdir, command): command = command.replace('(testdata)', utils.abs_datapath()) command = command.replace('(tmpdir)', str(tmpdir)) command = command.replace('(dirsep)', os.sep) - command = command.replace('(echo-exe)', get_echo_exe_path()) + command = command.replace('(echo-exe)', _get_echo_exe_path()) quteproc.send_cmd(command, count=count, invalid=invalid) From 0b816181c0f617a2cfc0ebb6e888ac712cb03989 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 11 Sep 2016 16:16:19 +0200 Subject: [PATCH 7/7] Update changelog --- CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 920f20f66..220138205 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -157,6 +157,7 @@ Fixed - The default `sk` keybinding now sets the commandline to `:bind` correctly - Fixed hang when using multiple spaces in a row with the URL completion - Fixed crash when closing a window without focusing it +- Userscripts now can access QUTE_FIFO correctly on Windows v0.8.3 (unreleased) -------------------