{"thread":{"id":"60489","subject":"[PATCH 0/3] t: improve compatibility with NixOS","startedAt":"2023-11-08T07:29:57Z","lastAt":"2023-11-13T23:42:14Z","messageCount":41,"participants":["Patrick Steinhardt","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"484562","messageId":"cover.1699428122.git.ps@pks.im","threadId":"60489","inReplyTo":null,"subject":"[PATCH 0/3] t: improve compatibility with NixOS","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-08T07:29:52Z","receivedAt":"2023-11-08T07:29:57Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series improves compatibility of our test suite with NixOS.\n\nNixOS is somewhat special compared to more conventional Linux distros\nbecause it doesn't follow the Filesystem Hierarchy Specification for\nmost of the part. Instead, packages are installed into the Nix store\nin `/nix` with hashed paths that change frequently when upgrading the\nsystem to a newer generation. Consequentially, paths cannot be hardcoded\nand must instead be computed at runtime.\n\nWe have two such issues in our test harness right now:\n\n - t/lib-httpd searches Apache httpd and its modules directory in a\n list of hardcoded paths.\n\n - t9164 doesn't propagate PATH to a script and thus cannot find the\n basename(1) utility.\n\nBoth of these issues are fixed in this patch series. In addition, this\npatch series fixes an upcoming issue in httpd's passwd files caused by\nthe deprecation of the crypt(3) function.\n\nPatrick\n\nPatrick Steinhardt (3):\n t/lib-httpd: dynamically detect httpd and modules path\n t/lib-httpd: stop using legacy crypt(3) for authentication\n t9164: fix inability to find basename(1) in hooks\n\n t/lib-httpd.sh | 51 ++++++++++++++++++---------\n t/lib-httpd/passwd | 2 +-\n t/lib-httpd/proxy-passwd | 2 +-\n t/t9164-git-svn-dcommit-concurrent.sh | 12 +++++--\n 4 files changed, 45 insertions(+), 22 deletions(-)\n\n-- \n2.42.0\n\n"},{"id":"484563","messageId":"ac059db8fedc6493c64f703814c7db11adb4385e.1699428122.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699428122.git.ps@pks.im","subject":"[PATCH 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-08T07:29:56Z","receivedAt":"2023-11-08T07:30:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In order to set up the Apache httpd server, we need to locate both the\nhttpd binary and its default module path. This is done with a hardcoded\nlist of locations that we scan. While this works okayish with distros\nthat more-or-less follow the Filesystem Hierarchy Standard, it falls\napart on others like NixOS that don't.\n\nWhile it is possible to specify these paths via `LIB_HTTPD_PATH` and\n`LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer\nto figure out how to set those up. And in fact we can do better by\ndynamically detecting both httpd and its module path at runtime:\n\n - The httpd binary can be located via PATH.\n\n - The module directory can (in many cases) be derived via the\n `HTTPD_ROOT` compile-time variable.\n\nRefactor the code to do so. If runtime detection of the paths fails we\ncontinue to fall back to the hardcoded list of paths.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/lib-httpd.sh | 51 +++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 34 insertions(+), 17 deletions(-)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 2fb1b2ae561..06038f72607 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -55,25 +55,42 @@ fi\n \n HTTPD_PARA=\"\"\n \n-for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n-do\n-\tif test -x \"$DEFAULT_HTTPD_PATH\"\n-\tthen\n-\t\tbreak\n-\tfi\n-done\n-\n-for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n-\t\t\t\t '/usr/lib/apache2/modules' \\\n-\t\t\t\t '/usr/lib64/httpd/modules' \\\n-\t\t\t\t '/usr/lib/httpd/modules' \\\n-\t\t\t\t '/usr/libexec/httpd'\n-do\n-\tif test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n+DEFAULT_HTTPD_PATH=\"$(command -v httpd)\"\n+if test -z \"$DEFAULT_HTTPD_PATH\" -o ! -x \"$DEFAULT_HTTPD_PATH\"\n+then\n+\tfor DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n+\tdo\n+\t\tif test -x \"$DEFAULT_HTTPD_PATH\"\n+\t\tthen\n+\t\t\tbreak\n+\t\tfi\n+\tdone\n+fi\n+\n+if test -x \"$DEFAULT_HTTPD_PATH\"\n+then\n+\tHTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n+\tif test -n \"$HTTPD_ROOT\" -a -d \"$HTTPD_ROOT/modules\"\n \tthen\n-\t\tbreak\n+\t\tDEFAULT_HTTPD_MODULE_PATH=\"$HTTPD_ROOT/modules\"\n \tfi\n-done\n+\tunset HTTPD_ROOT\n+fi\n+\n+if test -z \"$DEFAULT_HTTPD_MODULE_PATH\" -o ! -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n+then\n+\tfor DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n+\t\t\t\t\t '/usr/lib/apache2/modules' \\\n+\t\t\t\t\t '/usr/lib64/httpd/modules' \\\n+\t\t\t\t\t '/usr/lib/httpd/modules' \\\n+\t\t\t\t\t '/usr/libexec/httpd'\n+\tdo\n+\t\tif test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n+\t\tthen\n+\t\t\tbreak\n+\t\tfi\n+\tdone\n+fi\n \n case $(uname) in\n \tDarwin)\n-- \n2.42.0\n\n"},{"id":"484564","messageId":"23835763002c5f5cd68db7bdc9e4c083dda3558f.1699428122.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699428122.git.ps@pks.im","subject":"[PATCH 2/3] t/lib-httpd: stop using legacy crypt(3) for authentication","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-08T07:30:01Z","receivedAt":"2023-11-08T07:30:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When setting up httpd for our tests, we also install a passwd and\nproxy-passwd file that contain the test user's credentials. These\ncredentials currently use crypt(3) as the password encryption schema.\n\nThis schema can be considered deprecated nowadays as it is not safe\nanymore. Quoting Apache httpd's documentation [1]:\n\n> Unix only. Uses the traditional Unix crypt(3) function with a\n> randomly-generated 32-bit salt (only 12 bits used) and the first 8\n> characters of the password. Insecure.\n\nThis is starting to cause issues in modern Linux distributions. glibc\nhas deprecated its libcrypt library that used to provide crypt(3) in\nfavor of the libxcrypt library. This newer replacement provides a\ncompile time switch to disable insecure password encryption schemata,\nwhich causes crypt(3) to always return `EINVAL`. The end result is that\nhttpd tests that exercise authentication will fail on distros that use\nlibxcrypt without these insecure encryption schematas.\n\nRegenerate the passwd files to instead use the default password\nencryption schema, which is md5. While it feels kind of funny that an\nMD5-based encryption schema should be more secure than anything else, it\nis the current default and supported by all platforms. Furthermore, it\nreally doesn't matter all that much given that these files are only used\nfor testing purposes anyway.\n\n[1]: https://httpd.apache.org/docs/2.4/misc/password_encryptions.html\n\nSigned-off-by: Patrick Steinhardt \n---\n t/lib-httpd/passwd | 2 +-\n t/lib-httpd/proxy-passwd | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/lib-httpd/passwd b/t/lib-httpd/passwd\nindex 99a34d64874..d9c122f3482 100644\n--- a/t/lib-httpd/passwd\n+++ b/t/lib-httpd/passwd\n@@ -1 +1 @@\n-user@host:xb4E8pqD81KQs\n+user@host:$apr1$LGPmCZWj$9vxEwj5Z5GzQLBMxp3mCx1\ndiff --git a/t/lib-httpd/proxy-passwd b/t/lib-httpd/proxy-passwd\nindex 77c25138e07..2ad7705d9a3 100644\n--- a/t/lib-httpd/proxy-passwd\n+++ b/t/lib-httpd/proxy-passwd\n@@ -1 +1 @@\n-proxuser:2x7tAukjAED5M\n+proxuser:$apr1$RxS6MLkD$DYsqQdflheq4GPNxzJpx5.\n-- \n2.42.0\n\n"},{"id":"484565","messageId":"b50e625f9677469ecb33150343b88d5ac119dadd.1699428122.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699428122.git.ps@pks.im","subject":"[PATCH 3/3] t9164: fix inability to find basename(1) in hooks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-08T07:30:05Z","receivedAt":"2023-11-08T07:30:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The post-commit hook in t9164 is executed with a default environment.\nTo work around this issue we already write the current `PATH` value into\nthe hook script. But this is done at a point where we already tried to\nexecute non-built-in commands, namely basename(1). While this seems to\nwork alright on most platforms, it fails on NixOS.\n\nSet the `PATH` variable earlier to fix this issue. Note that we do this\nvia two separate heredocs. This is done because in the first one we do\nwant to expand variables, whereas in the second one we don't.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t9164-git-svn-dcommit-concurrent.sh | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t9164-git-svn-dcommit-concurrent.sh b/t/t9164-git-svn-dcommit-concurrent.sh\nindex c8e6c0733f4..3b64022dc57 100755\n--- a/t/t9164-git-svn-dcommit-concurrent.sh\n+++ b/t/t9164-git-svn-dcommit-concurrent.sh\n@@ -47,9 +47,15 @@ setup_hook()\n \techo \"cnt=$skip_revs\" > \"$hook_type-counter\"\n \trm -f \"$rawsvnrepo/hooks/\"*-commit # drop previous hooks\n \thook=\"$rawsvnrepo/hooks/$hook_type\"\n-\tcat > \"$hook\" <<- 'EOF1'\n+\tcat >\"$hook\" <<-EOF\n \t\t#!/bin/sh\n \t\tset -e\n+\n+\t\tPATH=\\\"$PATH\\\"\n+\t\texport PATH\n+\tEOF\n+\n+\tcat >>\"$hook\" <<-'EOF'\n \t\tcd \"$1/..\" # \"$1\" is repository location\n \t\texec >> svn-hook.log 2>&1\n \t\thook=\"$(basename \"$0\")\"\n@@ -59,11 +65,11 @@ setup_hook()\n \t\tcnt=\"$(($cnt - 1))\"\n \t\techo \"cnt=$cnt\" > ./$hook-counter\n \t\t[ \"$cnt\" = \"0\" ] || exit 0\n-EOF1\n+\tEOF\n+\n \tif [ \"$hook_type\" = \"pre-commit\" ]; then\n \t\techo \"echo 'commit disallowed' >&2; exit 1\" >>\"$hook\"\n \telse\n-\t\techo \"PATH=\\\"$PATH\\\"; export PATH\" >>\"$hook\"\n \t\techo \"svnconf=\\\"$svnconf\\\"\" >>\"$hook\"\n \t\tcat >>\"$hook\" <<- 'EOF2'\n \t\t\tcd work-auto-commits.svn\n-- \n2.42.0\n\n"},{"id":"484568","messageId":"xmqqcywk7k0d.fsf@gitster.g","threadId":"60489","inReplyTo":"ac059db8fedc6493c64f703814c7db11adb4385e.1699428122.git.ps@pks.im","subject":"Re: [PATCH 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-08T07:59:46Z","receivedAt":"2023-11-08T07:59:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> In order to set up the Apache httpd server, we need to locate both the\n> httpd binary and its default module path. This is done with a hardcoded\n> list of locations that we scan. While this works okayish with distros\n> that more-or-less follow the Filesystem Hierarchy Standard, it falls\n> apart on others like NixOS that don't.\n>\n> While it is possible to specify these paths via `LIB_HTTPD_PATH` and\n> `LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer\n> to figure out how to set those up. And in fact we can do better by\n> dynamically detecting both httpd and its module path at runtime:\n>\n> - The httpd binary can be located via PATH.\n>\n> - The module directory can (in many cases) be derived via the\n> `HTTPD_ROOT` compile-time variable.\n>\n> Refactor the code to do so. If runtime detection of the paths fails we\n> continue to fall back to the hardcoded list of paths.\n\nHmph.\n\nI do not think we would want to punish the distros that follow the\nFHS that was created explicitly to help developers by standardizing\nlocations of various things, with an approach this patch takes that\nthrows everthing with bathwater and rely on $PATH first.\n\nWould it be sufficient to please NixOS if we simply append $(command\n-v apache) or whatever after the well known candidate locations?\n\nI know \"command -v\" is in POSIX, and on both bash and dash (the two\nshells most distros use), it works as this patch expects, but its\nportability is also a bit worrysome, especially because the whole\npoint of this patch is to support a platform that is, eh, on the\nfringe.\n\nSo, I dunno.\n"},{"id":"484569","messageId":"xmqq8r787jy9.fsf@gitster.g","threadId":"60489","inReplyTo":"23835763002c5f5cd68db7bdc9e4c083dda3558f.1699428122.git.ps@pks.im","subject":"Re: [PATCH 2/3] t/lib-httpd: stop using legacy crypt(3) for authentication","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-08T08:01:02Z","receivedAt":"2023-11-08T08:01:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> When setting up httpd for our tests, we also install a passwd and\n> proxy-passwd file that contain the test user's credentials. These\n> credentials currently use crypt(3) as the password encryption schema.\n>\n> This schema can be considered deprecated nowadays as it is not safe\n> anymore. Quoting Apache httpd's documentation [1]:\n>\n>> Unix only. Uses the traditional Unix crypt(3) function with a\n>> randomly-generated 32-bit salt (only 12 bits used) and the first 8\n>> characters of the password. Insecure.\n>\n> This is starting to cause issues in modern Linux distributions. glibc\n> has deprecated its libcrypt library that used to provide crypt(3) in\n> favor of the libxcrypt library. This newer replacement provides a\n> compile time switch to disable insecure password encryption schemata,\n> which causes crypt(3) to always return `EINVAL`. The end result is that\n> httpd tests that exercise authentication will fail on distros that use\n> libxcrypt without these insecure encryption schematas.\n>\n> Regenerate the passwd files to instead use the default password\n> encryption schema, which is md5. While it feels kind of funny that an\n> MD5-based encryption schema should be more secure than anything else, it\n> is the current default and supported by all platforms. Furthermore, it\n> really doesn't matter all that much given that these files are only used\n> for testing purposes anyway.\n\nThis step makes quite a lot of sense, as we are changing this not at\nall for security but for portability ;-)\n\n>\n> [1]: https://httpd.apache.org/docs/2.4/misc/password_encryptions.html\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> t/lib-httpd/passwd | 2 +-\n> t/lib-httpd/proxy-passwd | 2 +-\n> 2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/lib-httpd/passwd b/t/lib-httpd/passwd\n> index 99a34d64874..d9c122f3482 100644\n> --- a/t/lib-httpd/passwd\n> +++ b/t/lib-httpd/passwd\n> @@ -1 +1 @@\n> -user@host:xb4E8pqD81KQs\n> +user@host:$apr1$LGPmCZWj$9vxEwj5Z5GzQLBMxp3mCx1\n> diff --git a/t/lib-httpd/proxy-passwd b/t/lib-httpd/proxy-passwd\n> index 77c25138e07..2ad7705d9a3 100644\n> --- a/t/lib-httpd/proxy-passwd\n> +++ b/t/lib-httpd/proxy-passwd\n> @@ -1 +1 @@\n> -proxuser:2x7tAukjAED5M\n> +proxuser:$apr1$RxS6MLkD$DYsqQdflheq4GPNxzJpx5.\n"},{"id":"484571","messageId":"ZUtmAyVHO4ROhLEq@tanuki","threadId":"60489","inReplyTo":"xmqqcywk7k0d.fsf@gitster.g","subject":"Re: [PATCH 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-08T10:42:11Z","receivedAt":"2023-11-08T10:42:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Nov 08, 2023 at 04:59:46PM +0900, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > In order to set up the Apache httpd server, we need to locate both the\n> > httpd binary and its default module path. This is done with a hardcoded\n> > list of locations that we scan. While this works okayish with distros\n> > that more-or-less follow the Filesystem Hierarchy Standard, it falls\n> > apart on others like NixOS that don't.\n> >\n> > While it is possible to specify these paths via `LIB_HTTPD_PATH` and\n> > `LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer\n> > to figure out how to set those up. And in fact we can do better by\n> > dynamically detecting both httpd and its module path at runtime:\n> >\n> > - The httpd binary can be located via PATH.\n> >\n> > - The module directory can (in many cases) be derived via the\n> > `HTTPD_ROOT` compile-time variable.\n> >\n> > Refactor the code to do so. If runtime detection of the paths fails we\n> > continue to fall back to the hardcoded list of paths.\n> \n> Hmph.\n> \n> I do not think we would want to punish the distros that follow the\n> FHS that was created explicitly to help developers by standardizing\n> locations of various things, with an approach this patch takes that\n> throws everthing with bathwater and rely on $PATH first.\n> \n> Would it be sufficient to please NixOS if we simply append $(command\n> -v apache) or whatever after the well known candidate locations?\n\nI was a bit torn myself when writing this. I can also see a potential\nfuture where we would drop the hardcoded list of locations altogether in\nfavor of always using PATH. After all we already rely on PATH to resolve\nother tools as well, so why should httpd be special there?\n\nBut in the end I opted to use the more conservative approach of using\nboth PATH and the static list as I didn't want to break other distros. I\ndon't mind to make this even more conservative and resolve via PATH as a\nlast resort, only.\n\nPatrick\n\n> I know \"command -v\" is in POSIX, and on both bash and dash (the two\n> shells most distros use), it works as this patch expects, but its\n> portability is also a bit worrysome, especially because the whole\n> point of this patch is to support a platform that is, eh, on the\n> fringe.\n> \n> So, I dunno.\n"},{"id":"484578","messageId":"cover.1699455383.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699428122.git.ps@pks.im","subject":"[PATCH v2 0/3] t: improve compatibility with NixOS","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-08T14:57:15Z","receivedAt":"2023-11-08T14:57:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the second version of my patch series to improve compatibility\nof our tests with NixOS.\n\nThere's only a single change compared to v1. As suggested by Junio, we\nnow use the executable discovered via PATH and the modules detected via\n`HTTPD_ROOT` as fallback values if none of our hardcoded paths were\ndetected. This should further ensure that the refactoring doesn't cause\nany issues on platforms that follow the FHS more closely than NixOS\ndoes. It also has the benefit that the change is now a lot more straight\nforward.\n\nPatrick\n\nPatrick Steinhardt (3):\n t/lib-httpd: dynamically detect httpd and modules path\n t/lib-httpd: stop using legacy crypt(3) for authentication\n t9164: fix inability to find basename(1) in hooks\n\n t/lib-httpd.sh | 10 ++++++++--\n t/lib-httpd/passwd | 2 +-\n t/lib-httpd/proxy-passwd | 2 +-\n t/t9164-git-svn-dcommit-concurrent.sh | 12 +++++++++---\n 4 files changed, 19 insertions(+), 7 deletions(-)\n\nRange-diff against v1:\n1: ac059db8fed ! 1: cee8fbebf84 t/lib-httpd: dynamically detect httpd and modules path\n @@ Commit message\n - The module directory can (in many cases) be derived via the\n `HTTPD_ROOT` compile-time variable.\n \n - Refactor the code to do so. If runtime detection of the paths fails we\n - continue to fall back to the hardcoded list of paths.\n + Amend the code to do so. The new runtime-detected paths will only be\n + used in case none of the hardcoded paths are usable.\n \n Signed-off-by: Patrick Steinhardt \n \n @@ t/lib-httpd.sh: fi\n HTTPD_PARA=\"\"\n \n -for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n --do\n --\tif test -x \"$DEFAULT_HTTPD_PATH\"\n --\tthen\n --\t\tbreak\n --\tfi\n --done\n --\n --for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n --\t\t\t\t '/usr/lib/apache2/modules' \\\n --\t\t\t\t '/usr/lib64/httpd/modules' \\\n --\t\t\t\t '/usr/lib/httpd/modules' \\\n --\t\t\t\t '/usr/libexec/httpd'\n --do\n --\tif test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n -+DEFAULT_HTTPD_PATH=\"$(command -v httpd)\"\n -+if test -z \"$DEFAULT_HTTPD_PATH\" -o ! -x \"$DEFAULT_HTTPD_PATH\"\n -+then\n -+\tfor DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n -+\tdo\n -+\t\tif test -x \"$DEFAULT_HTTPD_PATH\"\n -+\t\tthen\n -+\t\t\tbreak\n -+\t\tfi\n -+\tdone\n -+fi\n -+\n -+if test -x \"$DEFAULT_HTTPD_PATH\"\n -+then\n -+\tHTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n -+\tif test -n \"$HTTPD_ROOT\" -a -d \"$HTTPD_ROOT/modules\"\n ++for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2' \"$(command -v httpd)\"\n + do\n + \tif test -x \"$DEFAULT_HTTPD_PATH\"\n \tthen\n --\t\tbreak\n -+\t\tDEFAULT_HTTPD_MODULE_PATH=\"$HTTPD_ROOT/modules\"\n +@@ t/lib-httpd.sh: do\n \tfi\n --done\n -+\tunset HTTPD_ROOT\n -+fi\n -+\n -+if test -z \"$DEFAULT_HTTPD_MODULE_PATH\" -o ! -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n + done\n + \n ++if test -x \"$DEFAULT_HTTPD_PATH\"\n +then\n -+\tfor DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n -+\t\t\t\t\t '/usr/lib/apache2/modules' \\\n -+\t\t\t\t\t '/usr/lib64/httpd/modules' \\\n -+\t\t\t\t\t '/usr/lib/httpd/modules' \\\n -+\t\t\t\t\t '/usr/libexec/httpd'\n -+\tdo\n -+\t\tif test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n -+\t\tthen\n -+\t\t\tbreak\n -+\t\tfi\n -+\tdone\n ++\tDETECTED_HTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n +fi\n - \n - case $(uname) in\n - \tDarwin)\n ++\n + for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n + \t\t\t\t '/usr/lib/apache2/modules' \\\n + \t\t\t\t '/usr/lib64/httpd/modules' \\\n + \t\t\t\t '/usr/lib/httpd/modules' \\\n +-\t\t\t\t '/usr/libexec/httpd'\n ++\t\t\t\t '/usr/libexec/httpd' \\\n ++\t\t\t\t \"${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}\"\n + do\n + \tif test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n + \tthen\n2: 23835763002 = 2: f4c6c754f1e t/lib-httpd: stop using legacy crypt(3) for authentication\n3: b50e625f967 = 3: 361f1bd9c88 t9164: fix inability to find basename(1) in hooks\n\nbase-commit: 98009afd24e2304bf923a64750340423473809ff\n-- \n2.42.0\n\n"},{"id":"484579","messageId":"cee8fbebf84422f73c38d55b5730053121b74e0f.1699455383.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699455383.git.ps@pks.im","subject":"[PATCH v2 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-08T14:57:19Z","receivedAt":"2023-11-08T14:57:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In order to set up the Apache httpd server, we need to locate both the\nhttpd binary and its default module path. This is done with a hardcoded\nlist of locations that we scan. While this works okayish with distros\nthat more-or-less follow the Filesystem Hierarchy Standard, it falls\napart on others like NixOS that don't.\n\nWhile it is possible to specify these paths via `LIB_HTTPD_PATH` and\n`LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer\nto figure out how to set those up. And in fact we can do better by\ndynamically detecting both httpd and its module path at runtime:\n\n - The httpd binary can be located via PATH.\n\n - The module directory can (in many cases) be derived via the\n `HTTPD_ROOT` compile-time variable.\n\nAmend the code to do so. The new runtime-detected paths will only be\nused in case none of the hardcoded paths are usable.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/lib-httpd.sh | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 5fe3c8ab69d..26c1632ea9a 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -55,7 +55,7 @@ fi\n \n HTTPD_PARA=\"\"\n \n-for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2' \"$(command -v httpd)\"\n do\n \tif test -x \"$DEFAULT_HTTPD_PATH\"\n \tthen\n@@ -63,11 +63,17 @@ do\n \tfi\n done\n \n+if test -x \"$DEFAULT_HTTPD_PATH\"\n+then\n+\tDETECTED_HTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n+fi\n+\n for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n \t\t\t\t '/usr/lib/apache2/modules' \\\n \t\t\t\t '/usr/lib64/httpd/modules' \\\n \t\t\t\t '/usr/lib/httpd/modules' \\\n-\t\t\t\t '/usr/libexec/httpd'\n+\t\t\t\t '/usr/libexec/httpd' \\\n+\t\t\t\t \"${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}\"\n do\n \tif test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n \tthen\n-- \n2.42.0\n\n"},{"id":"484580","messageId":"f4c6c754f1e519b655d45572bd7566183d98138b.1699455383.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699455383.git.ps@pks.im","subject":"[PATCH v2 2/3] t/lib-httpd: stop using legacy crypt(3) for authentication","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-08T14:57:23Z","receivedAt":"2023-11-08T14:57:27Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When setting up httpd for our tests, we also install a passwd and\nproxy-passwd file that contain the test user's credentials. These\ncredentials currently use crypt(3) as the password encryption schema.\n\nThis schema can be considered deprecated nowadays as it is not safe\nanymore. Quoting Apache httpd's documentation [1]:\n\n> Unix only. Uses the traditional Unix crypt(3) function with a\n> randomly-generated 32-bit salt (only 12 bits used) and the first 8\n> characters of the password. Insecure.\n\nThis is starting to cause issues in modern Linux distributions. glibc\nhas deprecated its libcrypt library that used to provide crypt(3) in\nfavor of the libxcrypt library. This newer replacement provides a\ncompile time switch to disable insecure password encryption schemata,\nwhich causes crypt(3) to always return `EINVAL`. The end result is that\nhttpd tests that exercise authentication will fail on distros that use\nlibxcrypt without these insecure encryption schematas.\n\nRegenerate the passwd files to instead use the default password\nencryption schema, which is md5. While it feels kind of funny that an\nMD5-based encryption schema should be more secure than anything else, it\nis the current default and supported by all platforms. Furthermore, it\nreally doesn't matter all that much given that these files are only used\nfor testing purposes anyway.\n\n[1]: https://httpd.apache.org/docs/2.4/misc/password_encryptions.html\n\nSigned-off-by: Patrick Steinhardt \n---\n t/lib-httpd/passwd | 2 +-\n t/lib-httpd/proxy-passwd | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/lib-httpd/passwd b/t/lib-httpd/passwd\nindex 99a34d64874..d9c122f3482 100644\n--- a/t/lib-httpd/passwd\n+++ b/t/lib-httpd/passwd\n@@ -1 +1 @@\n-user@host:xb4E8pqD81KQs\n+user@host:$apr1$LGPmCZWj$9vxEwj5Z5GzQLBMxp3mCx1\ndiff --git a/t/lib-httpd/proxy-passwd b/t/lib-httpd/proxy-passwd\nindex 77c25138e07..2ad7705d9a3 100644\n--- a/t/lib-httpd/proxy-passwd\n+++ b/t/lib-httpd/proxy-passwd\n@@ -1 +1 @@\n-proxuser:2x7tAukjAED5M\n+proxuser:$apr1$RxS6MLkD$DYsqQdflheq4GPNxzJpx5.\n-- \n2.42.0\n\n"},{"id":"484581","messageId":"361f1bd9c88e3e6b7b135ba67b39d3bf4d70e322.1699455383.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699455383.git.ps@pks.im","subject":"[PATCH v2 3/3] t9164: fix inability to find basename(1) in hooks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-08T14:57:27Z","receivedAt":"2023-11-08T14:57:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The post-commit hook in t9164 is executed with a default environment.\nTo work around this issue we already write the current `PATH` value into\nthe hook script. But this is done at a point where we already tried to\nexecute non-built-in commands, namely basename(1). While this seems to\nwork alright on most platforms, it fails on NixOS.\n\nSet the `PATH` variable earlier to fix this issue. Note that we do this\nvia two separate heredocs. This is done because in the first one we do\nwant to expand variables, whereas in the second one we don't.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t9164-git-svn-dcommit-concurrent.sh | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t9164-git-svn-dcommit-concurrent.sh b/t/t9164-git-svn-dcommit-concurrent.sh\nindex c8e6c0733f4..3b64022dc57 100755\n--- a/t/t9164-git-svn-dcommit-concurrent.sh\n+++ b/t/t9164-git-svn-dcommit-concurrent.sh\n@@ -47,9 +47,15 @@ setup_hook()\n \techo \"cnt=$skip_revs\" > \"$hook_type-counter\"\n \trm -f \"$rawsvnrepo/hooks/\"*-commit # drop previous hooks\n \thook=\"$rawsvnrepo/hooks/$hook_type\"\n-\tcat > \"$hook\" <<- 'EOF1'\n+\tcat >\"$hook\" <<-EOF\n \t\t#!/bin/sh\n \t\tset -e\n+\n+\t\tPATH=\\\"$PATH\\\"\n+\t\texport PATH\n+\tEOF\n+\n+\tcat >>\"$hook\" <<-'EOF'\n \t\tcd \"$1/..\" # \"$1\" is repository location\n \t\texec >> svn-hook.log 2>&1\n \t\thook=\"$(basename \"$0\")\"\n@@ -59,11 +65,11 @@ setup_hook()\n \t\tcnt=\"$(($cnt - 1))\"\n \t\techo \"cnt=$cnt\" > ./$hook-counter\n \t\t[ \"$cnt\" = \"0\" ] || exit 0\n-EOF1\n+\tEOF\n+\n \tif [ \"$hook_type\" = \"pre-commit\" ]; then\n \t\techo \"echo 'commit disallowed' >&2; exit 1\" >>\"$hook\"\n \telse\n-\t\techo \"PATH=\\\"$PATH\\\"; export PATH\" >>\"$hook\"\n \t\techo \"svnconf=\\\"$svnconf\\\"\" >>\"$hook\"\n \t\tcat >>\"$hook\" <<- 'EOF2'\n \t\t\tcd work-auto-commits.svn\n-- \n2.42.0\n\n"},{"id":"484583","messageId":"20231108164432.GA1028115@coredump.intra.peff.net","threadId":"60489","inReplyTo":"ZUtmAyVHO4ROhLEq@tanuki","subject":"Re: [PATCH 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-08T16:44:32Z","receivedAt":"2023-11-08T16:44:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 08, 2023 at 11:42:11AM +0100, Patrick Steinhardt wrote:\n\n> I was a bit torn myself when writing this. I can also see a potential\n> future where we would drop the hardcoded list of locations altogether in\n> favor of always using PATH. After all we already rely on PATH to resolve\n> other tools as well, so why should httpd be special there?\n\nhttpd/apache2 is often in sbin, which is not as commonly found in the\nPATH of regular users. E.g., this is the case on Debian (it is\n/sbin/apache2, and a newly created user will not have that in their\nPATH).\n\n-Peff\n"},{"id":"484584","messageId":"20231108165426.GB1028115@coredump.intra.peff.net","threadId":"60489","inReplyTo":"cee8fbebf84422f73c38d55b5730053121b74e0f.1699455383.git.ps@pks.im","subject":"Re: [PATCH v2 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-08T16:54:26Z","receivedAt":"2023-11-08T16:54:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 08, 2023 at 03:57:19PM +0100, Patrick Steinhardt wrote:\n\n> While it is possible to specify these paths via `LIB_HTTPD_PATH` and\n> `LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer\n> to figure out how to set those up. And in fact we can do better by\n> dynamically detecting both httpd and its module path at runtime:\n> \n> - The httpd binary can be located via PATH.\n> \n> - The module directory can (in many cases) be derived via the\n> `HTTPD_ROOT` compile-time variable.\n> \n> Amend the code to do so. The new runtime-detected paths will only be\n> used in case none of the hardcoded paths are usable.\n\nIf these improve detection on your platform, I think that is a good\nthing and they are worth doing. But as a generic mechanism, I have two\ncomments:\n\n> -for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n> +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2' \"$(command -v httpd)\"\n> do\n> \tif test -x \"$DEFAULT_HTTPD_PATH\"\n> \tthen\n\nThe binary goes under the name \"httpd\", but also \"apache2\". But the PATH\nsearch only looks for \"httpd\". Should we check \"command -v apache2\" as\nwell?\n\nThis also means we may run \"test -x\" on an empty string, but that is\nprobably OK in practice (we could guard it with \"test -n\", though).\n\n> +if test -x \"$DEFAULT_HTTPD_PATH\"\n> +then\n> +\tDETECTED_HTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n> +fi\n\nI was really pleased to see this and hoped it could replace the\nkitchen-sink list of paths in the hunk below. But sadly I think it\ndepends on having a configured apache setup. On my Debian system, for\nexample, I have the \"apache2-bin\" package installed but not \"apache\"\n(because only the latter actually sets up a system apache daemon, which\nI don't want). And thus there is no config:\n\n $ /usr/sbin/apache2 -D HTTPD_ROOT\n apache2: Could not open configuration file /etc/apache2/apache2.conf: No such file or directory\n\nSo without a system config file to act as a template for our custom\nconfig, I think we are stuck with guessing where the installer might\nhave put them.\n\n-Peff\n"},{"id":"484585","messageId":"20231108170205.GC1028115@coredump.intra.peff.net","threadId":"60489","inReplyTo":"f4c6c754f1e519b655d45572bd7566183d98138b.1699455383.git.ps@pks.im","subject":"Re: [PATCH v2 2/3] t/lib-httpd: stop using legacy crypt(3) for authentication","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-08T17:02:05Z","receivedAt":"2023-11-08T17:02:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 08, 2023 at 03:57:23PM +0100, Patrick Steinhardt wrote:\n\n> Regenerate the passwd files to instead use the default password\n> encryption schema, which is md5. While it feels kind of funny that an\n> MD5-based encryption schema should be more secure than anything else, it\n> is the current default and supported by all platforms. Furthermore, it\n> really doesn't matter all that much given that these files are only used\n> for testing purposes anyway.\n\nThanks for doing this. I died inside a little while adding the\nproxy-passwd one recently in 29ae2c9e74 (add basic http proxy tests,\n2023-02-16). There I mused about moving to bcrypt in a separate patch,\nwhich I think is probably the least-bad option from a security\nperspective. But I agree that md5 is more likely to be available\neverywhere, and we certainly don't care about security here.\n\n-Peff\n"},{"id":"484586","messageId":"20231108172125.GD1028115@coredump.intra.peff.net","threadId":"60489","inReplyTo":"361f1bd9c88e3e6b7b135ba67b39d3bf4d70e322.1699455383.git.ps@pks.im","subject":"Re: [PATCH v2 3/3] t9164: fix inability to find basename(1) in hooks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-08T17:21:25Z","receivedAt":"2023-11-08T17:21:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 08, 2023 at 03:57:27PM +0100, Patrick Steinhardt wrote:\n\n> diff --git a/t/t9164-git-svn-dcommit-concurrent.sh b/t/t9164-git-svn-dcommit-concurrent.sh\n> index c8e6c0733f4..3b64022dc57 100755\n> --- a/t/t9164-git-svn-dcommit-concurrent.sh\n> +++ b/t/t9164-git-svn-dcommit-concurrent.sh\n> @@ -47,9 +47,15 @@ setup_hook()\n> \techo \"cnt=$skip_revs\" > \"$hook_type-counter\"\n> \trm -f \"$rawsvnrepo/hooks/\"*-commit # drop previous hooks\n> \thook=\"$rawsvnrepo/hooks/$hook_type\"\n> -\tcat > \"$hook\" <<- 'EOF1'\n> +\tcat >\"$hook\" <<-EOF\n> \t\t#!/bin/sh\n> \t\tset -e\n> +\n> +\t\tPATH=\\\"$PATH\\\"\n> +\t\texport PATH\n> +\tEOF\n\nThe quoting here is weird. The original escaped the double-quotes\nbecause it was echo-ing and interpolated string:\n\n> -\t\techo \"PATH=\\\"$PATH\\\"; export PATH\" >>\"$hook\"\n\nBut the here-doc interpolation does not treat double-quotes as special,\nand you end up with actual double-quote characters at the beginning and\nend of your $PATH variable, wrecking those entries (but I guess it works\nsome of the time depending on whether those parts of your $PATH are\nimportant).\n\nIt also interpolates PATH while making the here-doc. That's what you\nwant to convince the hook's inner shell to use the same PATH as the\nouter shell, but it also means that the inner shell will parse and\nevaluate it. I don't think anyone is going to inject '\"; rm -rf /' into\ntheir own test runs, but it does mean we'd do the wrong thing if their\n$PATH contains double-quotes or backslashes.\n\nCertainly the original suffered from the same issue, so it may not be\nworth worrying about. But the more robust way to do it is:\n\n # export ORIG_PATH, which presumably is not cleared the same way PATH\n # is, so that the hook can access it\n ORIG_PATH=$PATH &&\n export ORIG_PATH &&\n\n cat >\"$hook\" <orig-path\n cat >\"$hook\" < writes:\n\n> ... But the more robust way to do it is:\n>\n> # export ORIG_PATH, which presumably is not cleared the same way PATH\n> # is, so that the hook can access it\n> ORIG_PATH=$PATH &&\n> export ORIG_PATH &&\n>\n> cat >\"$hook\" < # pull the original path from the caller\n> PATH=$ORIG_PATH\n> export PATH\n>\n> ...do other stuff...\n> EOF\n>\n> That's assuming that environment variables make it intact to the hook at\n> all (it is not clear to me why the original $PATH doesn't).\n\nYeah, the parenthetical comment points at the crux of the issue. I\ncan tell from the patch what issue the platform throws at us we are\ntrying to work around, but it is frustrating not to know why the\nplatform does such unpleasant things in the first place.\n\nThanks.\n"},{"id":"484597","messageId":"xmqqsf5f4vl8.fsf@gitster.g","threadId":"60489","inReplyTo":"20231108165426.GB1028115@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-09T00:30:11Z","receivedAt":"2023-11-09T00:30:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Nov 08, 2023 at 03:57:19PM +0100, Patrick Steinhardt wrote:\n>\n>> While it is possible to specify these paths via `LIB_HTTPD_PATH` and\n>> `LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer\n>> to figure out how to set those up. And in fact we can do better by\n>> dynamically detecting both httpd and its module path at runtime:\n>> \n>> - The httpd binary can be located via PATH.\n>> \n>> - The module directory can (in many cases) be derived via the\n>> `HTTPD_ROOT` compile-time variable.\n>> \n>> Amend the code to do so. The new runtime-detected paths will only be\n>> used in case none of the hardcoded paths are usable.\n>\n> If these improve detection on your platform, I think that is a good\n> thing and they are worth doing. But as a generic mechanism, I have two\n> comments:\n>\n>> -for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n>> +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2' \"$(command -v httpd)\"\n>> do\n>> \tif test -x \"$DEFAULT_HTTPD_PATH\"\n>> \tthen\n>\n> The binary goes under the name \"httpd\", but also \"apache2\". But the PATH\n> search only looks for \"httpd\". Should we check \"command -v apache2\" as\n> well?\n\nInteresting. If $() were on the right hand side of an assignment,\nthe command that fails may also something to watch for, but I think\n$? from $(command -v no-such-httpd) would be discarded, so it would\nbe fine in this case. A trivia I found out today: dash exits with 127\nand bash exits with 1 when doing \n\n $shell -c 'command -v no-such-httpd'\n\n> This also means we may run \"test -x\" on an empty string, but that is\n> probably OK in practice (we could guard it with \"test -n\", though).\n\nGood thinking. We are not worried about exotic or misbehaving\nshells in this thread, so hopefully shell built-in 'test' would say\n\"no\" when asked if \"\" is executable (in other words, \"\" is not\nmistaken as \".\"). But an explicit \"test -n\" would save the readers\nby removing the worry.\n\n> ... So without a system config file to act as a template for our custom\n> config, I think we are stuck with guessing where the installer might\n> have put them.\n\nRight. Thanks for a careful analysis.\n"},{"id":"484601","messageId":"ZUx8adNt3Ky-U09W@tanuki","threadId":"60489","inReplyTo":"xmqqwmus3zvd.fsf@gitster.g","subject":"Re: [PATCH v2 3/3] t9164: fix inability to find basename(1) in hooks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-09T06:30:01Z","receivedAt":"2023-11-09T06:30:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Nov 09, 2023 at 02:43:02AM +0900, Junio C Hamano wrote:\n> Jeff King writes:\n> \n> > ... But the more robust way to do it is:\n> >\n> > # export ORIG_PATH, which presumably is not cleared the same way PATH\n> > # is, so that the hook can access it\n> > ORIG_PATH=$PATH &&\n> > export ORIG_PATH &&\n> >\n> > cat >\"$hook\" < > # pull the original path from the caller\n> > PATH=$ORIG_PATH\n> > export PATH\n> >\n> > ...do other stuff...\n> > EOF\n> >\n> > That's assuming that environment variables make it intact to the hook at\n> > all (it is not clear to me why the original $PATH doesn't).\n> \n> Yeah, the parenthetical comment points at the crux of the issue. I\n> can tell from the patch what issue the platform throws at us we are\n> trying to work around, but it is frustrating not to know why the\n> platform does such unpleasant things in the first place.\n> \n> Thanks.\n\nIndeed, I should've described this better in the commit message. Quoting\n[1]:\n\n> By default, Subversion executes hook scripts with an empty\n> environment—that is, no environment variables are set at all, not even\n> $PATH (or %PATH%, under Windows). Because of this, many administrators\n> are baffled when their hook program runs fine by hand, but doesn't\n> work when invoked by Subversion. Administrators have historically\n> worked around this problem by manually setting all the environment\n> variables their hook scripts need in the scripts themselves.\n\nSo it's not an issue of the environment, but rather an implementation\ndetail of how Subversion hooks work. It's surprising that this does not\nfail on other platforms -- maybe the shell has a default PATH there that\nallow us to locate basename(1)? I dunno.\n\nWill add to the commit message.\n\nPatrick\n\n[1]: https://svnbook.red-bean.com/en/1.8/svn.reposadmin.create.html#svn.reposadmin.create.hooks\n"},{"id":"484602","messageId":"ZUx8b9LequWRRlz3@tanuki","threadId":"60489","inReplyTo":"20231108165426.GB1028115@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-09T06:30:07Z","receivedAt":"2023-11-09T06:30:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Nov 08, 2023 at 11:54:26AM -0500, Jeff King wrote:\n> On Wed, Nov 08, 2023 at 03:57:19PM +0100, Patrick Steinhardt wrote:\n> \n> > While it is possible to specify these paths via `LIB_HTTPD_PATH` and\n> > `LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer\n> > to figure out how to set those up. And in fact we can do better by\n> > dynamically detecting both httpd and its module path at runtime:\n> > \n> > - The httpd binary can be located via PATH.\n> > \n> > - The module directory can (in many cases) be derived via the\n> > `HTTPD_ROOT` compile-time variable.\n> > \n> > Amend the code to do so. The new runtime-detected paths will only be\n> > used in case none of the hardcoded paths are usable.\n> \n> If these improve detection on your platform, I think that is a good\n> thing and they are worth doing. But as a generic mechanism, I have two\n> comments:\n> \n> > -for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n> > +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2' \"$(command -v httpd)\"\n> > do\n> > \tif test -x \"$DEFAULT_HTTPD_PATH\"\n> > \tthen\n> \n> The binary goes under the name \"httpd\", but also \"apache2\". But the PATH\n> search only looks for \"httpd\". Should we check \"command -v apache2\" as\n> well?\n\nDoesn't hurt to do it.\n\n> This also means we may run \"test -x\" on an empty string, but that is\n> probably OK in practice (we could guard it with \"test -n\", though).\n\nFair, will do.\n\n> > +if test -x \"$DEFAULT_HTTPD_PATH\"\n> > +then\n> > +\tDETECTED_HTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n> > +fi\n> \n> I was really pleased to see this and hoped it could replace the\n> kitchen-sink list of paths in the hunk below. But sadly I think it\n> depends on having a configured apache setup. On my Debian system, for\n> example, I have the \"apache2-bin\" package installed but not \"apache\"\n> (because only the latter actually sets up a system apache daemon, which\n> I don't want). And thus there is no config:\n> \n> $ /usr/sbin/apache2 -D HTTPD_ROOT\n> apache2: Could not open configuration file /etc/apache2/apache2.conf: No such file or directory\n> \n> So without a system config file to act as a template for our custom\n> config, I think we are stuck with guessing where the installer might\n> have put them.\n\nIndeed. Also, paths like `/usr/libexec/apache2` wouldn't be possible to\nbe discovered via `${HTTPD_ROOT}/modules` anyway. It's kind of a shame\nthat there is no way (or at least no way I know of) to portably discover\nthe default modules directory used by httpd.\n\nPatrick\n"},{"id":"484603","messageId":"ZUyD9Z9tsq9O2IkU@tanuki","threadId":"60489","inReplyTo":"ZUx8adNt3Ky-U09W@tanuki","subject":"Re: [PATCH v2 3/3] t9164: fix inability to find basename(1) in hooks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-09T07:02:13Z","receivedAt":"2023-11-09T07:02:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Nov 09, 2023 at 07:30:01AM +0100, Patrick Steinhardt wrote:\n> On Thu, Nov 09, 2023 at 02:43:02AM +0900, Junio C Hamano wrote:\n> > Jeff King writes:\n[snip]\n> > By default, Subversion executes hook scripts with an empty\n> > environment—that is, no environment variables are set at all, not even\n> > $PATH (or %PATH%, under Windows). Because of this, many administrators\n> > are baffled when their hook program runs fine by hand, but doesn't\n> > work when invoked by Subversion. Administrators have historically\n> > worked around this problem by manually setting all the environment\n> > variables their hook scripts need in the scripts themselves.\n> \n> So it's not an issue of the environment, but rather an implementation\n> detail of how Subversion hooks work. It's surprising that this does not\n> fail on other platforms -- maybe the shell has a default PATH there that\n> allow us to locate basename(1)? I dunno.\n\nYup, that's in fact it. Bash falls back to a default PATH that can be\nqueried via getconf(1p), and this works alright on most platforms. Not\non NixOS though.\n\nPatrick\n"},{"id":"484606","messageId":"cover.1699513524.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699428122.git.ps@pks.im","subject":"[PATCH v3 0/3] t: improve compatibility with NixOS","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-09T07:09:47Z","receivedAt":"2023-11-09T07:09:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the third version of my patch series to improve compatibility of\nour tests with NixOS.\n\nChanges compared to v2:\n\n - Patch 1: We now check for both httpd and apache2 binaries via\n PATH.\n\n - Patch 1: We're a bit more defensive and will check whether\n variables are empty before passing them to either `test -d` or\n `test -x`.\n\n - Patch 3: Clarified why PATH is unset.\n\n - Patch 3: Instead of writing PATH into the hook we now write it\n into the `hook-env` configuration file read by Subversion.\n\nPatrick\n\nPatrick Steinhardt (3):\n t/lib-httpd: dynamically detect httpd and modules path\n t/lib-httpd: stop using legacy crypt(3) for authentication\n t9164: fix inability to find basename(1) in Subversion hooks\n\n t/lib-httpd.sh | 17 +++++++++++++----\n t/lib-httpd/passwd | 2 +-\n t/lib-httpd/proxy-passwd | 2 +-\n t/t9164-git-svn-dcommit-concurrent.sh | 9 ++++++++-\n 4 files changed, 23 insertions(+), 7 deletions(-)\n\nRange-diff against v2:\n1: cee8fbebf84 ! 1: e4c75c492dd t/lib-httpd: dynamically detect httpd and modules path\n @@ t/lib-httpd.sh: fi\n HTTPD_PARA=\"\"\n \n -for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n -+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2' \"$(command -v httpd)\"\n ++for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' \\\n ++\t\t\t '/usr/sbin/apache2' \\\n ++\t\t\t \"$(command -v httpd)\" \\\n ++\t\t\t \"$(command -v apache2)\"\n do\n - \tif test -x \"$DEFAULT_HTTPD_PATH\"\n +-\tif test -x \"$DEFAULT_HTTPD_PATH\"\n ++\tif test -n \"$DEFAULT_HTTPD_PATH\" -a -x \"$DEFAULT_HTTPD_PATH\"\n \tthen\n -@@ t/lib-httpd.sh: do\n + \t\tbreak\n \tfi\n done\n \n @@ t/lib-httpd.sh: do\n +\t\t\t\t '/usr/libexec/httpd' \\\n +\t\t\t\t \"${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}\"\n do\n - \tif test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n +-\tif test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n ++\tif test -n \"$DEFAULT_HTTPD_MODULE_PATH\" -a -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n \tthen\n + \t\tbreak\n + \tfi\n2: f4c6c754f1e = 2: 3dce76da477 t/lib-httpd: stop using legacy crypt(3) for authentication\n3: 361f1bd9c88 ! 3: 6891e254155 t9164: fix inability to find basename(1) in hooks\n @@ Metadata\n Author: Patrick Steinhardt \n \n ## Commit message ##\n - t9164: fix inability to find basename(1) in hooks\n + t9164: fix inability to find basename(1) in Subversion hooks\n \n - The post-commit hook in t9164 is executed with a default environment.\n - To work around this issue we already write the current `PATH` value into\n - the hook script. But this is done at a point where we already tried to\n - execute non-built-in commands, namely basename(1). While this seems to\n - work alright on most platforms, it fails on NixOS.\n + Hooks executed by Subversion are spawned with an empty environment. By\n + default, not even variables like PATH will be propagated to them. In\n + order to ensure that we're still able to find required executables, we\n + thus write the current PATH variable into the hook script itself and\n + then re-export it in t9164.\n \n - Set the `PATH` variable earlier to fix this issue. Note that we do this\n - via two separate heredocs. This is done because in the first one we do\n - want to expand variables, whereas in the second one we don't.\n + This happens too late in the script though, as we already tried to\n + execute the basename(1) utility before exporting the PATH variable. This\n + tends to work on most platforms as the fallback value of PATH for Bash\n + (see `getconf PATH`) is likely to contain this binary. But on more\n + exotic platforms like NixOS this is not the case, and thus the test\n + fails.\n +\n + While we could work around this issue by simply setting PATH earlier, it\n + feels fragile to inject a user-controlled value into the script and have\n + the shell interpret it. Instead, we can refactor the hook setup to write\n + a `hooks-env` file that configures PATH for us. Like this, Subversion\n + will know to set up the environment as expected for all hooks.\n \n Signed-off-by: Patrick Steinhardt \n \n ## t/t9164-git-svn-dcommit-concurrent.sh ##\n @@ t/t9164-git-svn-dcommit-concurrent.sh: setup_hook()\n + \t\t\t\"passed to setup_hook\" >&2 ; return 1; }\n \techo \"cnt=$skip_revs\" > \"$hook_type-counter\"\n \trm -f \"$rawsvnrepo/hooks/\"*-commit # drop previous hooks\n - \thook=\"$rawsvnrepo/hooks/$hook_type\"\n --\tcat > \"$hook\" <<- 'EOF1'\n -+\tcat >\"$hook\" <<-EOF\n - \t\t#!/bin/sh\n - \t\tset -e\n -+\n -+\t\tPATH=\\\"$PATH\\\"\n -+\t\texport PATH\n -+\tEOF\n +\n -+\tcat >>\"$hook\" <<-'EOF'\n - \t\tcd \"$1/..\" # \"$1\" is repository location\n - \t\texec >> svn-hook.log 2>&1\n - \t\thook=\"$(basename \"$0\")\"\n -@@ t/t9164-git-svn-dcommit-concurrent.sh: setup_hook()\n - \t\tcnt=\"$(($cnt - 1))\"\n - \t\techo \"cnt=$cnt\" > ./$hook-counter\n - \t\t[ \"$cnt\" = \"0\" ] || exit 0\n --EOF1\n ++\t# Subversion hooks run with an empty environment by default. We thus\n ++\t# need to propagate PATH so that we can find executables.\n ++\tcat >\"$rawsvnrepo/conf/hooks-env\" <<-EOF\n ++\t[default]\n ++\tPATH = ${PATH}\n +\tEOF\n +\n + \thook=\"$rawsvnrepo/hooks/$hook_type\"\n + \tcat > \"$hook\" <<- 'EOF1'\n + \t\t#!/bin/sh\n +@@ t/t9164-git-svn-dcommit-concurrent.sh: EOF1\n \tif [ \"$hook_type\" = \"pre-commit\" ]; then\n \t\techo \"echo 'commit disallowed' >&2; exit 1\" >>\"$hook\"\n \telse\n\nbase-commit: dadef801b365989099a9929e995589e455c51fed\n-- \n2.42.0\n\n"},{"id":"484607","messageId":"e4c75c492dd89fd7464db2b3028b2bb9e6addbf8.1699513524.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699513524.git.ps@pks.im","subject":"[PATCH v3 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-09T07:09:52Z","receivedAt":"2023-11-09T07:09:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In order to set up the Apache httpd server, we need to locate both the\nhttpd binary and its default module path. This is done with a hardcoded\nlist of locations that we scan. While this works okayish with distros\nthat more-or-less follow the Filesystem Hierarchy Standard, it falls\napart on others like NixOS that don't.\n\nWhile it is possible to specify these paths via `LIB_HTTPD_PATH` and\n`LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer\nto figure out how to set those up. And in fact we can do better by\ndynamically detecting both httpd and its module path at runtime:\n\n - The httpd binary can be located via PATH.\n\n - The module directory can (in many cases) be derived via the\n `HTTPD_ROOT` compile-time variable.\n\nAmend the code to do so. The new runtime-detected paths will only be\nused in case none of the hardcoded paths are usable.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/lib-httpd.sh | 17 +++++++++++++----\n 1 file changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 5fe3c8ab69d..6ab8f273a3a 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -55,21 +55,30 @@ fi\n \n HTTPD_PARA=\"\"\n \n-for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' \\\n+\t\t\t '/usr/sbin/apache2' \\\n+\t\t\t \"$(command -v httpd)\" \\\n+\t\t\t \"$(command -v apache2)\"\n do\n-\tif test -x \"$DEFAULT_HTTPD_PATH\"\n+\tif test -n \"$DEFAULT_HTTPD_PATH\" -a -x \"$DEFAULT_HTTPD_PATH\"\n \tthen\n \t\tbreak\n \tfi\n done\n \n+if test -x \"$DEFAULT_HTTPD_PATH\"\n+then\n+\tDETECTED_HTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n+fi\n+\n for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n \t\t\t\t '/usr/lib/apache2/modules' \\\n \t\t\t\t '/usr/lib64/httpd/modules' \\\n \t\t\t\t '/usr/lib/httpd/modules' \\\n-\t\t\t\t '/usr/libexec/httpd'\n+\t\t\t\t '/usr/libexec/httpd' \\\n+\t\t\t\t \"${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}\"\n do\n-\tif test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n+\tif test -n \"$DEFAULT_HTTPD_MODULE_PATH\" -a -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n \tthen\n \t\tbreak\n \tfi\n-- \n2.42.0\n\n"},{"id":"484608","messageId":"3dce76da477852e5e6964ea20cad2bfe2788a96f.1699513524.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699513524.git.ps@pks.im","subject":"[PATCH v3 2/3] t/lib-httpd: stop using legacy crypt(3) for authentication","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-09T07:09:56Z","receivedAt":"2023-11-09T07:10:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When setting up httpd for our tests, we also install a passwd and\nproxy-passwd file that contain the test user's credentials. These\ncredentials currently use crypt(3) as the password encryption schema.\n\nThis schema can be considered deprecated nowadays as it is not safe\nanymore. Quoting Apache httpd's documentation [1]:\n\n> Unix only. Uses the traditional Unix crypt(3) function with a\n> randomly-generated 32-bit salt (only 12 bits used) and the first 8\n> characters of the password. Insecure.\n\nThis is starting to cause issues in modern Linux distributions. glibc\nhas deprecated its libcrypt library that used to provide crypt(3) in\nfavor of the libxcrypt library. This newer replacement provides a\ncompile time switch to disable insecure password encryption schemata,\nwhich causes crypt(3) to always return `EINVAL`. The end result is that\nhttpd tests that exercise authentication will fail on distros that use\nlibxcrypt without these insecure encryption schematas.\n\nRegenerate the passwd files to instead use the default password\nencryption schema, which is md5. While it feels kind of funny that an\nMD5-based encryption schema should be more secure than anything else, it\nis the current default and supported by all platforms. Furthermore, it\nreally doesn't matter all that much given that these files are only used\nfor testing purposes anyway.\n\n[1]: https://httpd.apache.org/docs/2.4/misc/password_encryptions.html\n\nSigned-off-by: Patrick Steinhardt \n---\n t/lib-httpd/passwd | 2 +-\n t/lib-httpd/proxy-passwd | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/lib-httpd/passwd b/t/lib-httpd/passwd\nindex 99a34d64874..d9c122f3482 100644\n--- a/t/lib-httpd/passwd\n+++ b/t/lib-httpd/passwd\n@@ -1 +1 @@\n-user@host:xb4E8pqD81KQs\n+user@host:$apr1$LGPmCZWj$9vxEwj5Z5GzQLBMxp3mCx1\ndiff --git a/t/lib-httpd/proxy-passwd b/t/lib-httpd/proxy-passwd\nindex 77c25138e07..2ad7705d9a3 100644\n--- a/t/lib-httpd/proxy-passwd\n+++ b/t/lib-httpd/proxy-passwd\n@@ -1 +1 @@\n-proxuser:2x7tAukjAED5M\n+proxuser:$apr1$RxS6MLkD$DYsqQdflheq4GPNxzJpx5.\n-- \n2.42.0\n\n"},{"id":"484609","messageId":"6891e2541552073c6827d81d2b761252f253bea2.1699513524.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699513524.git.ps@pks.im","subject":"[PATCH v3 3/3] t9164: fix inability to find basename(1) in Subversion hooks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-09T07:10:01Z","receivedAt":"2023-11-09T07:10:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hooks executed by Subversion are spawned with an empty environment. By\ndefault, not even variables like PATH will be propagated to them. In\norder to ensure that we're still able to find required executables, we\nthus write the current PATH variable into the hook script itself and\nthen re-export it in t9164.\n\nThis happens too late in the script though, as we already tried to\nexecute the basename(1) utility before exporting the PATH variable. This\ntends to work on most platforms as the fallback value of PATH for Bash\n(see `getconf PATH`) is likely to contain this binary. But on more\nexotic platforms like NixOS this is not the case, and thus the test\nfails.\n\nWhile we could work around this issue by simply setting PATH earlier, it\nfeels fragile to inject a user-controlled value into the script and have\nthe shell interpret it. Instead, we can refactor the hook setup to write\na `hooks-env` file that configures PATH for us. Like this, Subversion\nwill know to set up the environment as expected for all hooks.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t9164-git-svn-dcommit-concurrent.sh | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t9164-git-svn-dcommit-concurrent.sh b/t/t9164-git-svn-dcommit-concurrent.sh\nindex c8e6c0733f4..d1dec89c3b7 100755\n--- a/t/t9164-git-svn-dcommit-concurrent.sh\n+++ b/t/t9164-git-svn-dcommit-concurrent.sh\n@@ -46,6 +46,14 @@ setup_hook()\n \t\t\t\"passed to setup_hook\" >&2 ; return 1; }\n \techo \"cnt=$skip_revs\" > \"$hook_type-counter\"\n \trm -f \"$rawsvnrepo/hooks/\"*-commit # drop previous hooks\n+\n+\t# Subversion hooks run with an empty environment by default. We thus\n+\t# need to propagate PATH so that we can find executables.\n+\tcat >\"$rawsvnrepo/conf/hooks-env\" <<-EOF\n+\t[default]\n+\tPATH = ${PATH}\n+\tEOF\n+\n \thook=\"$rawsvnrepo/hooks/$hook_type\"\n \tcat > \"$hook\" <<- 'EOF1'\n \t\t#!/bin/sh\n@@ -63,7 +71,6 @@ EOF1\n \tif [ \"$hook_type\" = \"pre-commit\" ]; then\n \t\techo \"echo 'commit disallowed' >&2; exit 1\" >>\"$hook\"\n \telse\n-\t\techo \"PATH=\\\"$PATH\\\"; export PATH\" >>\"$hook\"\n \t\techo \"svnconf=\\\"$svnconf\\\"\" >>\"$hook\"\n \t\tcat >>\"$hook\" <<- 'EOF2'\n \t\t\tcd work-auto-commits.svn\n-- \n2.42.0\n\n"},{"id":"484618","messageId":"20231109073250.GA2698227@coredump.intra.peff.net","threadId":"60489","inReplyTo":"e4c75c492dd89fd7464db2b3028b2bb9e6addbf8.1699513524.git.ps@pks.im","subject":"Re: [PATCH v3 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-09T07:32:50Z","receivedAt":"2023-11-09T07:32:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 09, 2023 at 08:09:52AM +0100, Patrick Steinhardt wrote:\n\n> -for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n> +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' \\\n> +\t\t\t '/usr/sbin/apache2' \\\n> +\t\t\t \"$(command -v httpd)\" \\\n> +\t\t\t \"$(command -v apache2)\"\n> do\n> -\tif test -x \"$DEFAULT_HTTPD_PATH\"\n> +\tif test -n \"$DEFAULT_HTTPD_PATH\" -a -x \"$DEFAULT_HTTPD_PATH\"\n\nSorry to be a pedant, but I'm not sure if we might have portability\nproblems with \"-a\". It's an XSI extension, and POSIX labels it as\nobsolescent because it can create parsing ambiguities.\n\nWe do have a few instances, but only in corners of the test suite that\nprobably don't get as much exposure (t/perf and valgrind/valgrind.sh).\nSo maybe not worth worrying about, but it's easy to write it as:\n\n if test -n \"$DEFAULT_HTTPD_PATH\" && test -x \"$DEFAULT_HTTPD_PATH\"\n\n-Peff\n"},{"id":"484619","messageId":"20231109073517.GB2698227@coredump.intra.peff.net","threadId":"60489","inReplyTo":"6891e2541552073c6827d81d2b761252f253bea2.1699513524.git.ps@pks.im","subject":"Re: [PATCH v3 3/3] t9164: fix inability to find basename(1) in Subversion hooks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-09T07:35:17Z","receivedAt":"2023-11-09T07:35:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 09, 2023 at 08:10:01AM +0100, Patrick Steinhardt wrote:\n\n> +\t# Subversion hooks run with an empty environment by default. We thus\n> +\t# need to propagate PATH so that we can find executables.\n> +\tcat >\"$rawsvnrepo/conf/hooks-env\" <<-EOF\n> +\t[default]\n> +\tPATH = ${PATH}\n> +\tEOF\n\nThis is so much less ugly than the shell shenanigans discussed in the\nearlier round. Thanks for finding it. :)\n\nThe ${PATH} here is interpolated by the here-doc. I guess it's possible\nsomebody's exotic PATH could break the svn conf syntax, but it's\nprobably not worth worrying about.\n\n-Peff\n"},{"id":"484620","messageId":"20231109073615.GC2698227@coredump.intra.peff.net","threadId":"60489","inReplyTo":"cover.1699513524.git.ps@pks.im","subject":"Re: [PATCH v3 0/3] t: improve compatibility with NixOS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-09T07:36:15Z","receivedAt":"2023-11-09T07:36:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 09, 2023 at 08:09:47AM +0100, Patrick Steinhardt wrote:\n\n> this is the third version of my patch series to improve compatibility of\n> our tests with NixOS.\n> \n> Changes compared to v2:\n> \n> - Patch 1: We now check for both httpd and apache2 binaries via\n> PATH.\n> \n> - Patch 1: We're a bit more defensive and will check whether\n> variables are empty before passing them to either `test -d` or\n> `test -x`.\n> \n> - Patch 3: Clarified why PATH is unset.\n> \n> - Patch 3: Instead of writing PATH into the hook we now write it\n> into the `hook-env` configuration file read by Subversion.\n\nThanks. This looks good to me, modulo a minor shell nit I mentioned in\nthe first patch.\n\n-Peff\n"},{"id":"484621","messageId":"ZUyMFZ7c9_rlu5lk@tanuki","threadId":"60489","inReplyTo":"20231109073250.GA2698227@coredump.intra.peff.net","subject":"Re: [PATCH v3 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-09T07:36:53Z","receivedAt":"2023-11-09T07:37:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Nov 09, 2023 at 02:32:50AM -0500, Jeff King wrote:\n> On Thu, Nov 09, 2023 at 08:09:52AM +0100, Patrick Steinhardt wrote:\n> \n> > -for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n> > +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' \\\n> > +\t\t\t '/usr/sbin/apache2' \\\n> > +\t\t\t \"$(command -v httpd)\" \\\n> > +\t\t\t \"$(command -v apache2)\"\n> > do\n> > -\tif test -x \"$DEFAULT_HTTPD_PATH\"\n> > +\tif test -n \"$DEFAULT_HTTPD_PATH\" -a -x \"$DEFAULT_HTTPD_PATH\"\n> \n> Sorry to be a pedant, but I'm not sure if we might have portability\n> problems with \"-a\". It's an XSI extension, and POSIX labels it as\n> obsolescent because it can create parsing ambiguities.\n> \n> We do have a few instances, but only in corners of the test suite that\n> probably don't get as much exposure (t/perf and valgrind/valgrind.sh).\n> So maybe not worth worrying about, but it's easy to write it as:\n\nYeah, I was grepping for it in our codebase and saw other occurrences,\nso I assumed it was fair game. If we're going to convert it to the\nbelow, how about I send another patch on top that also converts the\npreexisting instances so that the next one grepping for it isn't going\nto repeat the same mistake?\n\nPatrick\n\n> if test -n \"$DEFAULT_HTTPD_PATH\" && test -x \"$DEFAULT_HTTPD_PATH\"\n> \n> -Peff\n"},{"id":"484622","messageId":"xmqqzfzn1i9u.fsf@gitster.g","threadId":"60489","inReplyTo":"ZUyMFZ7c9_rlu5lk@tanuki","subject":"Re: [PATCH v3 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-09T07:46:05Z","receivedAt":"2023-11-09T07:46:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Yeah, I was grepping for it in our codebase and saw other occurrences,\n> so I assumed it was fair game. If we're going to convert it to the\n> below, how about I send another patch on top that also converts the\n> preexisting instances so that the next one grepping for it isn't going\n> to repeat the same mistake?\n\nYup, an independent clean-up would be fine. Now we need to find a\nway to give better visibility to CodingGuidelines, which already\nsays this:\n\n - We do not write our \"test\" command with \"-a\" and \"-o\" and use \"&&\"\n or \"||\" to concatenate multiple \"test\" commands instead, because\n the use of \"-a/-o\" is often error-prone. E.g.\n\n test -n \"$x\" -a \"$a\" = \"$b\"\n\n is buggy and breaks when $x is \"=\", but\n\n test -n \"$x\" && test \"$a\" = \"$b\"\n\n does not have such a problem.\n\n"},{"id":"484623","messageId":"20231109074831.GA2699305@coredump.intra.peff.net","threadId":"60489","inReplyTo":"ZUyMFZ7c9_rlu5lk@tanuki","subject":"Re: [PATCH v3 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-09T07:48:31Z","receivedAt":"2023-11-09T07:48:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 09, 2023 at 08:36:53AM +0100, Patrick Steinhardt wrote:\n\n> > Sorry to be a pedant, but I'm not sure if we might have portability\n> > problems with \"-a\". It's an XSI extension, and POSIX labels it as\n> > obsolescent because it can create parsing ambiguities.\n> > \n> > We do have a few instances, but only in corners of the test suite that\n> > probably don't get as much exposure (t/perf and valgrind/valgrind.sh).\n> > So maybe not worth worrying about, but it's easy to write it as:\n> \n> Yeah, I was grepping for it in our codebase and saw other occurrences,\n> so I assumed it was fair game. If we're going to convert it to the\n> below, how about I send another patch on top that also converts the\n> preexisting instances so that the next one grepping for it isn't going\n> to repeat the same mistake?\n\nI would be very happy with that. :)\n\n-Peff\n"},{"id":"484625","messageId":"ZUyQ9DOLccTeJElf@tanuki","threadId":"60489","inReplyTo":"xmqqzfzn1i9u.fsf@gitster.g","subject":"Re: [PATCH v3 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-09T07:57:40Z","receivedAt":"2023-11-09T07:57:45Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Nov 09, 2023 at 04:46:05PM +0900, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > Yeah, I was grepping for it in our codebase and saw other occurrences,\n> > so I assumed it was fair game. If we're going to convert it to the\n> > below, how about I send another patch on top that also converts the\n> > preexisting instances so that the next one grepping for it isn't going\n> > to repeat the same mistake?\n> \n> Yup, an independent clean-up would be fine. Now we need to find a\n> way to give better visibility to CodingGuidelines, which already\n> says this:\n\nOkay, I'll send one in. Do you want me to send a v4 of this patch series\nor will you squash in below changes into patch 1/3?\n\n> - We do not write our \"test\" command with \"-a\" and \"-o\" and use \"&&\"\n> or \"||\" to concatenate multiple \"test\" commands instead, because\n> the use of \"-a/-o\" is often error-prone. E.g.\n> \n> test -n \"$x\" -a \"$a\" = \"$b\"\n> \n> is buggy and breaks when $x is \"=\", but\n> \n> test -n \"$x\" && test \"$a\" = \"$b\"\n> \n> does not have such a problem.\n\nI did indeed spot this part of our coding style now. I didn't bother to\nlook farther when I found other examples where we used `-a` and `-o`,\nbut that issue will be gone once we've dropped all of these usages.\n\nPatrick\n\n-- >8 --\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 6ab8f273a3..0a74922d7f 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -60,7 +60,7 @@ for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' \\\n \t\t\t \"$(command -v httpd)\" \\\n \t\t\t \"$(command -v apache2)\"\n do\n-\tif test -n \"$DEFAULT_HTTPD_PATH\" -a -x \"$DEFAULT_HTTPD_PATH\"\n+\tif test -n \"$DEFAULT_HTTPD_PATH\" && test -x \"$DEFAULT_HTTPD_PATH\"\n \tthen\n \t\tbreak\n \tfi\n@@ -78,7 +78,7 @@ for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n \t\t\t\t '/usr/libexec/httpd' \\\n \t\t\t\t \"${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}\"\n do\n-\tif test -n \"$DEFAULT_HTTPD_MODULE_PATH\" -a -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n+\tif test -n \"$DEFAULT_HTTPD_MODULE_PATH\" && test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n \tthen\n \t\tbreak\n \tfi\n\n"},{"id":"484694","messageId":"41b9dada2e0b2e713328e6a4d31f713a2d3ffa38.1699596457.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699596457.git.ps@pks.im","subject":"[PATCH v4 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-10T08:17:00Z","receivedAt":"2023-11-10T08:17:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In order to set up the Apache httpd server, we need to locate both the\nhttpd binary and its default module path. This is done with a hardcoded\nlist of locations that we scan. While this works okayish with distros\nthat more-or-less follow the Filesystem Hierarchy Standard, it falls\napart on others like NixOS that don't.\n\nWhile it is possible to specify these paths via `LIB_HTTPD_PATH` and\n`LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer\nto figure out how to set those up. And in fact we can do better by\ndynamically detecting both httpd and its module path at runtime:\n\n - The httpd binary can be located via PATH.\n\n - The module directory can (in many cases) be derived via the\n `HTTPD_ROOT` compile-time variable.\n\nAmend the code to do so.\n\nNote that the new runtime-detected paths will only be used as a fallback\nin case none of the hardcoded paths are usable. For the PATH lookup this\nis because httpd is typically installed into \"/usr/sbin\", which is often\nnot included in the user's PATH variable. And the module path detection\nrelies on a configured httpd installation and may thus not work in all\ncases, either.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/lib-httpd.sh | 17 +++++++++++++----\n 1 file changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 9ea74927c40..f69d0da51d1 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -55,22 +55,31 @@ fi\n \n HTTPD_PARA=\"\"\n \n-for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' \\\n+\t\t\t '/usr/sbin/apache2' \\\n+\t\t\t \"$(command -v httpd)\" \\\n+\t\t\t \"$(command -v apache2)\"\n do\n-\tif test -x \"$DEFAULT_HTTPD_PATH\"\n+\tif test -n \"$DEFAULT_HTTPD_PATH\" && test -x \"$DEFAULT_HTTPD_PATH\"\n \tthen\n \t\tbreak\n \tfi\n done\n \n+if test -x \"$DEFAULT_HTTPD_PATH\"\n+then\n+\tDETECTED_HTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n+fi\n+\n for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n \t\t\t\t '/usr/lib/apache2/modules' \\\n \t\t\t\t '/usr/lib64/httpd/modules' \\\n \t\t\t\t '/usr/lib/httpd/modules' \\\n \t\t\t\t '/usr/libexec/httpd' \\\n-\t\t\t\t '/usr/lib/apache2'\n+\t\t\t\t '/usr/lib/apache2' \\\n+\t\t\t\t \"${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}\"\n do\n-\tif test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n+\tif test -n \"$DEFAULT_HTTPD_MODULE_PATH\" && test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n \tthen\n \t\tbreak\n \tfi\n-- \n2.42.0\n\n"},{"id":"484695","messageId":"cover.1699596457.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699428122.git.ps@pks.im","subject":"[PATCH v4 0/3] t: improve compatibility with NixOS","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-10T08:16:56Z","receivedAt":"2023-11-10T08:17:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the fourth version of my patch series to improve compatibility\nof our test suite with NixOS.\n\nThree changes compared to v3:\n\n - Switched from `test -a ` to `test && test\n `.\n\n - Improved the commit message to explain why the new\n runtime-detected paths are only used as a fallback.\n\n - Rebased on top of 0e3b67e2aa (ci: add support for GitLab CI,\n 2023-11-09), which has been merged to `next` and caused conflicts.\n\nTechnically speaking this series also depends on 0763c3a2c4 (http:\nupdate curl http/2 info matching for curl 8.3.0, 2023-09-15), without\nwhich the tests will fail on NixOS machines with a recent libcurl.\n\nPatrick\n\nPatrick Steinhardt (3):\n t/lib-httpd: dynamically detect httpd and modules path\n t/lib-httpd: stop using legacy crypt(3) for authentication\n t9164: fix inability to find basename(1) in Subversion hooks\n\n t/lib-httpd.sh | 17 +++++++++++++----\n t/lib-httpd/passwd | 2 +-\n t/lib-httpd/proxy-passwd | 2 +-\n t/t9164-git-svn-dcommit-concurrent.sh | 9 ++++++++-\n 4 files changed, 23 insertions(+), 7 deletions(-)\n\nRange-diff against v3:\n1: e4c75c492dd ! 1: 41b9dada2e0 t/lib-httpd: dynamically detect httpd and modules path\n @@ Commit message\n - The module directory can (in many cases) be derived via the\n `HTTPD_ROOT` compile-time variable.\n \n - Amend the code to do so. The new runtime-detected paths will only be\n - used in case none of the hardcoded paths are usable.\n + Amend the code to do so.\n +\n + Note that the new runtime-detected paths will only be used as a fallback\n + in case none of the hardcoded paths are usable. For the PATH lookup this\n + is because httpd is typically installed into \"/usr/sbin\", which is often\n + not included in the user's PATH variable. And the module path detection\n + relies on a configured httpd installation and may thus not work in all\n + cases, either.\n \n Signed-off-by: Patrick Steinhardt \n \n @@ t/lib-httpd.sh: fi\n +\t\t\t \"$(command -v apache2)\"\n do\n -\tif test -x \"$DEFAULT_HTTPD_PATH\"\n -+\tif test -n \"$DEFAULT_HTTPD_PATH\" -a -x \"$DEFAULT_HTTPD_PATH\"\n ++\tif test -n \"$DEFAULT_HTTPD_PATH\" && test -x \"$DEFAULT_HTTPD_PATH\"\n \tthen\n \t\tbreak\n \tfi\n @@ t/lib-httpd.sh: fi\n \t\t\t\t '/usr/lib/apache2/modules' \\\n \t\t\t\t '/usr/lib64/httpd/modules' \\\n \t\t\t\t '/usr/lib/httpd/modules' \\\n --\t\t\t\t '/usr/libexec/httpd'\n -+\t\t\t\t '/usr/libexec/httpd' \\\n + \t\t\t\t '/usr/libexec/httpd' \\\n +-\t\t\t\t '/usr/lib/apache2'\n ++\t\t\t\t '/usr/lib/apache2' \\\n +\t\t\t\t \"${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}\"\n do\n -\tif test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n -+\tif test -n \"$DEFAULT_HTTPD_MODULE_PATH\" -a -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n ++\tif test -n \"$DEFAULT_HTTPD_MODULE_PATH\" && test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n \tthen\n \t\tbreak\n \tfi\n2: 3dce76da477 = 2: 7d28c32feaa t/lib-httpd: stop using legacy crypt(3) for authentication\n3: 6891e254155 = 3: 6a7773aadba t9164: fix inability to find basename(1) in Subversion hooks\n\nbase-commit: 0e3b67e2aa25edb7e1a5c999c87b52a7b3a7649a\n-- \n2.42.0\n\n"},{"id":"484696","messageId":"7d28c32feaaadb411f4d2b0f37b38acdab9e4a58.1699596457.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699596457.git.ps@pks.im","subject":"[PATCH v4 2/3] t/lib-httpd: stop using legacy crypt(3) for authentication","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-10T08:17:04Z","receivedAt":"2023-11-10T08:17:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When setting up httpd for our tests, we also install a passwd and\nproxy-passwd file that contain the test user's credentials. These\ncredentials currently use crypt(3) as the password encryption schema.\n\nThis schema can be considered deprecated nowadays as it is not safe\nanymore. Quoting Apache httpd's documentation [1]:\n\n> Unix only. Uses the traditional Unix crypt(3) function with a\n> randomly-generated 32-bit salt (only 12 bits used) and the first 8\n> characters of the password. Insecure.\n\nThis is starting to cause issues in modern Linux distributions. glibc\nhas deprecated its libcrypt library that used to provide crypt(3) in\nfavor of the libxcrypt library. This newer replacement provides a\ncompile time switch to disable insecure password encryption schemata,\nwhich causes crypt(3) to always return `EINVAL`. The end result is that\nhttpd tests that exercise authentication will fail on distros that use\nlibxcrypt without these insecure encryption schematas.\n\nRegenerate the passwd files to instead use the default password\nencryption schema, which is md5. While it feels kind of funny that an\nMD5-based encryption schema should be more secure than anything else, it\nis the current default and supported by all platforms. Furthermore, it\nreally doesn't matter all that much given that these files are only used\nfor testing purposes anyway.\n\n[1]: https://httpd.apache.org/docs/2.4/misc/password_encryptions.html\n\nSigned-off-by: Patrick Steinhardt \n---\n t/lib-httpd/passwd | 2 +-\n t/lib-httpd/proxy-passwd | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/lib-httpd/passwd b/t/lib-httpd/passwd\nindex 99a34d64874..d9c122f3482 100644\n--- a/t/lib-httpd/passwd\n+++ b/t/lib-httpd/passwd\n@@ -1 +1 @@\n-user@host:xb4E8pqD81KQs\n+user@host:$apr1$LGPmCZWj$9vxEwj5Z5GzQLBMxp3mCx1\ndiff --git a/t/lib-httpd/proxy-passwd b/t/lib-httpd/proxy-passwd\nindex 77c25138e07..2ad7705d9a3 100644\n--- a/t/lib-httpd/proxy-passwd\n+++ b/t/lib-httpd/proxy-passwd\n@@ -1 +1 @@\n-proxuser:2x7tAukjAED5M\n+proxuser:$apr1$RxS6MLkD$DYsqQdflheq4GPNxzJpx5.\n-- \n2.42.0\n\n"},{"id":"484697","messageId":"6a7773aadba4fec5daa1e215a50ab122515ea456.1699596457.git.ps@pks.im","threadId":"60489","inReplyTo":"cover.1699596457.git.ps@pks.im","subject":"[PATCH v4 3/3] t9164: fix inability to find basename(1) in Subversion hooks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-10T08:17:09Z","receivedAt":"2023-11-10T08:17:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hooks executed by Subversion are spawned with an empty environment. By\ndefault, not even variables like PATH will be propagated to them. In\norder to ensure that we're still able to find required executables, we\nthus write the current PATH variable into the hook script itself and\nthen re-export it in t9164.\n\nThis happens too late in the script though, as we already tried to\nexecute the basename(1) utility before exporting the PATH variable. This\ntends to work on most platforms as the fallback value of PATH for Bash\n(see `getconf PATH`) is likely to contain this binary. But on more\nexotic platforms like NixOS this is not the case, and thus the test\nfails.\n\nWhile we could work around this issue by simply setting PATH earlier, it\nfeels fragile to inject a user-controlled value into the script and have\nthe shell interpret it. Instead, we can refactor the hook setup to write\na `hooks-env` file that configures PATH for us. Like this, Subversion\nwill know to set up the environment as expected for all hooks.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t9164-git-svn-dcommit-concurrent.sh | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t9164-git-svn-dcommit-concurrent.sh b/t/t9164-git-svn-dcommit-concurrent.sh\nindex c8e6c0733f4..d1dec89c3b7 100755\n--- a/t/t9164-git-svn-dcommit-concurrent.sh\n+++ b/t/t9164-git-svn-dcommit-concurrent.sh\n@@ -46,6 +46,14 @@ setup_hook()\n \t\t\t\"passed to setup_hook\" >&2 ; return 1; }\n \techo \"cnt=$skip_revs\" > \"$hook_type-counter\"\n \trm -f \"$rawsvnrepo/hooks/\"*-commit # drop previous hooks\n+\n+\t# Subversion hooks run with an empty environment by default. We thus\n+\t# need to propagate PATH so that we can find executables.\n+\tcat >\"$rawsvnrepo/conf/hooks-env\" <<-EOF\n+\t[default]\n+\tPATH = ${PATH}\n+\tEOF\n+\n \thook=\"$rawsvnrepo/hooks/$hook_type\"\n \tcat > \"$hook\" <<- 'EOF1'\n \t\t#!/bin/sh\n@@ -63,7 +71,6 @@ EOF1\n \tif [ \"$hook_type\" = \"pre-commit\" ]; then\n \t\techo \"echo 'commit disallowed' >&2; exit 1\" >>\"$hook\"\n \telse\n-\t\techo \"PATH=\\\"$PATH\\\"; export PATH\" >>\"$hook\"\n \t\techo \"svnconf=\\\"$svnconf\\\"\" >>\"$hook\"\n \t\tcat >>\"$hook\" <<- 'EOF2'\n \t\t\tcd work-auto-commits.svn\n-- \n2.42.0\n\n"},{"id":"484714","messageId":"20231110214126.GB2758295@coredump.intra.peff.net","threadId":"60489","inReplyTo":"cover.1699596457.git.ps@pks.im","subject":"Re: [PATCH v4 0/3] t: improve compatibility with NixOS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-10T21:41:26Z","receivedAt":"2023-11-10T21:41:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 10, 2023 at 09:16:56AM +0100, Patrick Steinhardt wrote:\n\n> this is the fourth version of my patch series to improve compatibility\n> of our test suite with NixOS.\n> \n> Three changes compared to v3:\n> \n> - Switched from `test -a ` to `test && test\n> `.\n> \n> - Improved the commit message to explain why the new\n> runtime-detected paths are only used as a fallback.\n> \n> - Rebased on top of 0e3b67e2aa (ci: add support for GitLab CI,\n> 2023-11-09), which has been merged to `next` and caused conflicts.\n\nThanks, these all look good to me.\n\n-Peff\n"},{"id":"484731","messageId":"xmqqo7g1tb06.fsf@gitster.g","threadId":"60489","inReplyTo":"41b9dada2e0b2e713328e6a4d31f713a2d3ffa38.1699596457.git.ps@pks.im","subject":"Re: [PATCH v4 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-11T00:00:09Z","receivedAt":"2023-11-11T00:00:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> +if test -x \"$DEFAULT_HTTPD_PATH\"\n> +then\n> +\tDETECTED_HTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n> +fi\n\nWith this patch, my test run starts like so:\n\n rm -f -r 'test-results'\n *** prove ***\n apache2: Could not open configuration file /etc/apache2/apache2.conf: No such file or directory\n ...\n\nI find the error message leaking mildly annoying, and would suggest\ndoing something like the following on top.\n\ndiff --git c/t/lib-httpd.sh w/t/lib-httpd.sh\nindex 0a74922d7f..03493ee72b 100644\n--- c/t/lib-httpd.sh\n+++ w/t/lib-httpd.sh\n@@ -68,7 +68,7 @@ done\n \n if test -x \"$DEFAULT_HTTPD_PATH\"\n then\n-\tDETECTED_HTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n+\tDETECTED_HTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V 2>/dev/null | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n fi\n \n for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n"},{"id":"484733","messageId":"xmqqh6lttaj4.fsf@gitster.g","threadId":"60489","inReplyTo":"cover.1699596457.git.ps@pks.im","subject":"Re: [PATCH v4 0/3] t: improve compatibility with NixOS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-11T00:10:23Z","receivedAt":"2023-11-11T00:10:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Three changes compared to v3:\n>\n> - Switched from `test -a ` to `test && test\n> `.\n>\n> - Improved the commit message to explain why the new\n> runtime-detected paths are only used as a fallback.\n>\n> - Rebased on top of 0e3b67e2aa (ci: add support for GitLab CI,\n> 2023-11-09), which has been merged to `next` and caused conflicts.\n\nPlease, no. \"The other topic has been merged to 'next'\" is *not* a\ngood reason to do this. Before that, the other topic was in 'seen'\nand causing conflicts already, so getting into 'next' did not create\nany new reason for rebasing.\n\nI'll manage this time, but please do not do such a rebase unless you\nare asked to do so. The rebase will force me to do (1) detach from\n'next' and apply these, (2) discard the result and detach from the\nbase of where the previous iteration is queued, (3) apply the new\niteration with \"am -3\" to undo the rebase, before I can compare the\nnew iteration with the old iteration.\n\n> Technically speaking this series also depends on 0763c3a2c4 (http:\n> update curl http/2 info matching for curl 8.3.0, 2023-09-15), without\n> which the tests will fail on NixOS machines with a recent libcurl.\n\nThanks for that note. This topic has been queued on top of\nv2.43.0-rc1 which has 0763c3a2c4, so we'd be safe.\n\nWill queue.\n"},{"id":"484784","messageId":"ZVHM-0d7g45P3Uoj@tanuki","threadId":"60489","inReplyTo":"xmqqh6lttaj4.fsf@gitster.g","subject":"Re: [PATCH v4 0/3] t: improve compatibility with NixOS","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-13T07:15:07Z","receivedAt":"2023-11-13T07:15:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Nov 11, 2023 at 09:10:23AM +0900, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > Three changes compared to v3:\n> >\n> > - Switched from `test -a ` to `test && test\n> > `.\n> >\n> > - Improved the commit message to explain why the new\n> > runtime-detected paths are only used as a fallback.\n> >\n> > - Rebased on top of 0e3b67e2aa (ci: add support for GitLab CI,\n> > 2023-11-09), which has been merged to `next` and caused conflicts.\n> \n> Please, no. \"The other topic has been merged to 'next'\" is *not* a\n> good reason to do this. Before that, the other topic was in 'seen'\n> and causing conflicts already, so getting into 'next' did not create\n> any new reason for rebasing.\n> \n> I'll manage this time, but please do not do such a rebase unless you\n> are asked to do so. The rebase will force me to do (1) detach from\n> 'next' and apply these, (2) discard the result and detach from the\n> base of where the previous iteration is queued, (3) apply the new\n> iteration with \"am -3\" to undo the rebase, before I can compare the\n> new iteration with the old iteration.\n\nFair enough. I assumed that it would ease your workload instead of\ncreating more work for you. But I'll keep in mind that it doesn't and\nrefrain from doing this in the future.\n\n> > Technically speaking this series also depends on 0763c3a2c4 (http:\n> > update curl http/2 info matching for curl 8.3.0, 2023-09-15), without\n> > which the tests will fail on NixOS machines with a recent libcurl.\n> \n> Thanks for that note. This topic has been queued on top of\n> v2.43.0-rc1 which has 0763c3a2c4, so we'd be safe.\n> \n> Will queue.\n\nThanks.\n\nPatrick\n"},{"id":"484785","messageId":"ZVHNKk9QUtzFR-wp@tanuki","threadId":"60489","inReplyTo":"xmqqo7g1tb06.fsf@gitster.g","subject":"Re: [PATCH v4 1/3] t/lib-httpd: dynamically detect httpd and modules path","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-13T07:15:54Z","receivedAt":"2023-11-13T07:15:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Nov 11, 2023 at 09:00:09AM +0900, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > +if test -x \"$DEFAULT_HTTPD_PATH\"\n> > +then\n> > +\tDETECTED_HTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n> > +fi\n> \n> With this patch, my test run starts like so:\n> \n> rm -f -r 'test-results'\n> *** prove ***\n> apache2: Could not open configuration file /etc/apache2/apache2.conf: No such file or directory\n> ...\n> \n> I find the error message leaking mildly annoying, and would suggest\n> doing something like the following on top.\n> \n> diff --git c/t/lib-httpd.sh w/t/lib-httpd.sh\n> index 0a74922d7f..03493ee72b 100644\n> --- c/t/lib-httpd.sh\n> +++ w/t/lib-httpd.sh\n> @@ -68,7 +68,7 @@ done\n> \n> if test -x \"$DEFAULT_HTTPD_PATH\"\n> then\n> -\tDETECTED_HTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n> +\tDETECTED_HTTPD_ROOT=\"$(\"$DEFAULT_HTTPD_PATH\" -V 2>/dev/null | sed -n 's/^ -D HTTPD_ROOT=\"\\(.*\\)\"$/\\1/p')\"\n> fi\n> \n> for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n\nYup, makes sense. Thanks for the tweak.\n\nPatrick\n"},{"id":"484827","messageId":"xmqqcywd43vz.fsf@gitster.g","threadId":"60489","inReplyTo":"ZVHM-0d7g45P3Uoj@tanuki","subject":"Re: [PATCH v4 0/3] t: improve compatibility with NixOS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-13T23:42:08Z","receivedAt":"2023-11-13T23:42:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Fair enough. I assumed that it would ease your workload instead of\n> creating more work for you. But I'll keep in mind that it doesn't and\n> refrain from doing this in the future.\n\nThanks. \n\nWith or without conflicts, basing your work on 'next' would not be a\ngood idea to begin with, as we never merge 'next' down to 'master'\n(we only merge individual topics).\n\nThe following applies not specifically to you but to all\ncontributors.\n\nWhat is helpful is in a tricky case is to start your topic branch\ndevelopment at an appropriate base (often the tip of 'master', but\nfor a bugfix the tip of 'maint'), merge in other topics in flight\nyou depend on that are not in the target base (which limits what you\ncan depend on if you are writing a bugfix on 'maint'), and then\nstart building on top of the merge result. Then communicate how you\nbuilt this (artificial) base clearly in your cover letter.\n\nWhether you have \"other topics in flight\" merged to your base or\nnot, creating a trial merge of your topic to seen to see how others'\nwork-in-progress may interact with your work would be a valuable way\nto keep aware of what is cooking and how you will be affected.\n\nThanks.\n\n\n"}]}