[pacman-dev] [PATCH] Have configure set pactest options that it knows.
This patch is not ready for accepting into the repository. It does not work for VPATH builds (because the paths to all the .py tests are seen as relative to $(builddir)). Dave is working on a fix for this VPATH issue, so a real version of this patch must wait to incorporate his commit. This patch works fine for in-$(srcdir) builds. The purpose of posting it is to follow up Saturday's conversation. I think it implements all of Allan's suggestions (if I understood correctly). The real commit message would be something like: SCRIPTLET_SHELL and LDCONFIG can be set via args to configure, and up 'till now those options were passed to pactest by Makefile.am as command-line args. That works fine for 'make check', but required repeated specification when running pactest manually. This patch makes pactest a configured file so it has direct access to those options and they no longer need to be specified (by Makefile.am nor by hand). Also the default for the pactest '-p' arg is changed to be the pacman that the make builds. The '-p' arg is still available, but it should now be very rare to have to use it when calling pactest manually. --- Makefile.am | 4 -- configure.ac | 1 + test/pacman/.gitignore | 1 + test/pacman/pactest.py | 125 -------------------------------------------- test/pacman/pactest.py.in | 128 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 130 insertions(+), 129 deletions(-) delete mode 100755 test/pacman/pactest.py create mode 100644 test/pacman/pactest.py.in diff --git a/Makefile.am b/Makefile.am index 4d5adae..9689992 100644 --- a/Makefile.am +++ b/Makefile.am @@ -41,10 +41,6 @@ LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ PY_LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ $(top_srcdir)/build-aux/tap-driver.sh PY_LOG_COMPILER = $(top_srcdir)/test/pacman/pactest.py -AM_PY_LOG_FLAGS = \ - --scriptlet-shell $(SCRIPTLET_SHELL) \ - --ldconfig $(LDCONFIG) \ - -p $(top_builddir)/src/pacman/pacman # create the pacman DB and cache directories upon install install-data-local: diff --git a/configure.ac b/configure.ac index cfcc8d1..292260a 100644 --- a/configure.ac +++ b/configure.ac @@ -507,6 +507,7 @@ test/util/Makefile contrib/Makefile Makefile ]) +AC_CONFIG_FILES([test/pacman/pactest.py],[chmod +x test/pacman/pactest.py]) AC_OUTPUT echo " diff --git a/test/pacman/.gitignore b/test/pacman/.gitignore index 0d20b64..39fa72b 100644 --- a/test/pacman/.gitignore +++ b/test/pacman/.gitignore @@ -1 +1,2 @@ *.pyc +pactest.py diff --git a/test/pacman/pactest.py b/test/pacman/pactest.py deleted file mode 100755 index e21cde7..0000000 --- a/test/pacman/pactest.py +++ /dev/null @@ -1,125 +0,0 @@ -#! /usr/bin/python2 -# -# pactest : run automated testing on the pacman binary -# -# Copyright (c) 2006 by Aurelien Foret <orelien@chez.com> -# Copyright (c) 2006-2013 Pacman Development Team <pacman-dev@archlinux.org> -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see <http://www.gnu.org/licenses/>. - -import glob -from optparse import OptionParser -import os -import shutil -import sys -import tempfile - -import pmenv -import tap -import util - -__author__ = "Aurelien FORET" -__version__ = "0.4" - -def resolve_binary_path(option, opt_str, value, parser): - setattr(parser.values, option.dest, os.path.abspath(value)) - -def create_parser(): - usage = "usage: %prog [options] <path/to/testfile.py>..." - description = "Runs automated tests on the pacman binary. Tests are " \ - "described using an easy python syntax, and several can be " \ - "ran at once." - parser = OptionParser(usage = usage, description = description) - - parser.add_option("-v", "--verbose", action = "count", - dest = "verbose", default = 0, - help = "print verbose output") - parser.add_option("-d", "--debug", type = "int", - dest = "debug", default = 0, - help = "set debug level for pacman") - parser.add_option("-p", "--pacman", action = "callback", - callback = resolve_binary_path, type = "string", - dest = "bin", default = "pacman", - help = "specify location of the pacman binary") - parser.add_option("--keep-root", action = "store_true", - dest = "keeproot", default = False, - help = "don't remove the generated pacman root filesystem") - parser.add_option("--nolog", action = "store_true", - dest = "nolog", default = False, - help = "do not log pacman messages") - parser.add_option("--gdb", action = "store_true", - dest = "gdb", default = False, - help = "use gdb while calling pacman") - parser.add_option("--valgrind", action = "store_true", - dest = "valgrind", default = False, - help = "use valgrind while calling pacman") - parser.add_option("--manual-confirm", action = "store_true", - dest = "manualconfirm", default = False, - help = "do not use --noconfirm for pacman calls") - parser.add_option("--scriptlet-shell", type = "string", - dest = "scriptletshell", default = "/bin/sh", - help = "specify path to shell used for install scriptlets") - parser.add_option("--ldconfig", type = "string", - dest = "ldconfig", default = "/sbin/ldconfig", - help = "specify path to ldconfig") - return parser - - -if __name__ == "__main__": - - if sys.hexversion < 0x02070000: - # bailing now with clear message better than mid-run with unhelpful one - tap.bail("Python versions before 2.7 are not supported.") - sys.exit(1) - - # instantiate env and parser objects - root_path = tempfile.mkdtemp() - env = pmenv.pmenv(root=root_path) - opt_parser = create_parser() - (opts, args) = opt_parser.parse_args() - - # add parsed options to env object - util.verbose = opts.verbose - env.pacman["debug"] = opts.debug - env.pacman["bin"] = opts.bin - env.pacman["nolog"] = opts.nolog - env.pacman["gdb"] = opts.gdb - env.pacman["valgrind"] = opts.valgrind - env.pacman["manual-confirm"] = opts.manualconfirm - env.pacman["scriptlet-shell"] = opts.scriptletshell - env.pacman["ldconfig"] = opts.ldconfig - - opts.testcases = [] - for path in args: - opts.testcases += glob.glob(path) - if opts.testcases is None or len(opts.testcases) == 0: - tap.bail("no tests defined, nothing to do") - os.rmdir(root_path) - sys.exit(2) - - for i in opts.testcases: - env.addtest(i) - - # run tests - env.run() - - if not opts.keeproot: - shutil.rmtree(root_path) - else: - tap.diag("pacman testing root saved: %s" % root_path) - - if env.failed > 0: - sys.exit(1) - -# vim: set ts=4 sw=4 et: diff --git a/test/pacman/pactest.py.in b/test/pacman/pactest.py.in new file mode 100644 index 0000000..0d9e1f7 --- /dev/null +++ b/test/pacman/pactest.py.in @@ -0,0 +1,128 @@ +#! /usr/bin/python2 +# +# @configure_input@ +# +# pactest : run automated testing on the pacman binary +# +# Copyright (c) 2006 by Aurelien Foret <orelien@chez.com> +# Copyright (c) 2006-2013 Pacman Development Team <pacman-dev@archlinux.org> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +import glob +from optparse import OptionParser +import os +import shutil +import sys +import tempfile + +BUILDDIR = os.path.dirname(os.path.realpath(__file__)) +SRCDIR = os.path.realpath(os.path.join(BUILDDIR, "@srcdir@")) +if BUILDDIR != SRCDIR: sys.path.insert(0, SRCDIR) +TOP_BUILDDIR = os.path.realpath(os.path.join(BUILDDIR, "@top_builddir@")) +BUILT_EXE = os.path.realpath(os.path.join(TOP_BUILDDIR, "src/pacman/pacman")) + +import pmenv +import tap +import util + +__author__ = "Aurelien FORET" +__version__ = "0.4" + +def resolve_binary_path(option, opt_str, value, parser): + setattr(parser.values, option.dest, os.path.abspath(value)) + +def create_parser(): + usage = "usage: %prog [options] <path/to/testfile.py>..." + description = "Runs automated tests on the pacman binary. Tests are " \ + "described using an easy python syntax, and several can be " \ + "ran at once." + parser = OptionParser(usage = usage, description = description) + + parser.add_option("-v", "--verbose", action = "count", + dest = "verbose", default = 0, + help = "print verbose output") + parser.add_option("-d", "--debug", type = "int", + dest = "debug", default = 0, + help = "set debug level for pacman") + parser.add_option("-p", "--pacman", action = "callback", + callback = resolve_binary_path, type = "string", + dest = "bin", default = BUILT_EXE, + help = "specify location of the pacman binary") + parser.add_option("--keep-root", action = "store_true", + dest = "keeproot", default = False, + help = "don't remove the generated pacman root filesystem") + parser.add_option("--nolog", action = "store_true", + dest = "nolog", default = False, + help = "do not log pacman messages") + parser.add_option("--gdb", action = "store_true", + dest = "gdb", default = False, + help = "use gdb while calling pacman") + parser.add_option("--valgrind", action = "store_true", + dest = "valgrind", default = False, + help = "use valgrind while calling pacman") + parser.add_option("--manual-confirm", action = "store_true", + dest = "manualconfirm", default = False, + help = "do not use --noconfirm for pacman calls") + return parser + + +if __name__ == "__main__": + + if sys.hexversion < 0x02070000: + # bailing now with clear message better than mid-run with unhelpful one + tap.bail("Python versions before 2.7 are not supported.") + sys.exit(1) + + # instantiate env and parser objects + root_path = tempfile.mkdtemp() + env = pmenv.pmenv(root=root_path) + opt_parser = create_parser() + (opts, args) = opt_parser.parse_args() + + # add parsed options to env object + util.verbose = opts.verbose + env.pacman["debug"] = opts.debug + env.pacman["bin"] = opts.bin + env.pacman["nolog"] = opts.nolog + env.pacman["gdb"] = opts.gdb + env.pacman["valgrind"] = opts.valgrind + env.pacman["manual-confirm"] = opts.manualconfirm + # and add configured options + env.pacman["scriptlet-shell"] = "@SCRIPTLET_SHELL@" + env.pacman["ldconfig"] = "@LDCONFIG@" + + opts.testcases = [] + for path in args: + opts.testcases += glob.glob(path) + if opts.testcases is None or len(opts.testcases) == 0: + tap.bail("no tests defined, nothing to do") + os.rmdir(root_path) + sys.exit(2) + + for i in opts.testcases: + env.addtest(i) + + # run tests + env.run() + + if not opts.keeproot: + shutil.rmtree(root_path) + else: + tap.diag("pacman testing root saved: %s" % root_path) + + if env.failed > 0: + sys.exit(1) + +# vim: set ts=4 sw=4 et: -- 1.8.5.2
On Dec 23, 2013, Jeremy Heiner <scalaprotractor@gmail.com> wrote:
a real version of this patch must wait to incorporate [Dave's VPATH fix]
I suspect that will involve: pactest being modified to examine each test filename it receives, looking for the test .py files in both srcdir and builddir, and even doing its own globbing on each name so './pactest sync*' would run both the regular files and the configured file.
On 24/12/13 01:05, Jeremy Heiner wrote:
This patch is not ready for accepting into the repository.
It does not work for VPATH builds (because the paths to all the .py tests are seen as relative to $(builddir)). Dave is working on a fix for this VPATH issue, so a real version of this patch must wait to incorporate his commit. This patch works fine for in-$(srcdir) builds.
The purpose of posting it is to follow up Saturday's conversation. I think it implements all of Allan's suggestions (if I understood correctly). The real commit message would be something like:
SCRIPTLET_SHELL and LDCONFIG can be set via args to configure, and up 'till now those options were passed to pactest by Makefile.am as command-line args. That works fine for 'make check', but required repeated specification when running pactest manually. This patch makes pactest a configured file so it has direct access to those options and they no longer need to be specified (by Makefile.am nor by hand).
Also the default for the pactest '-p' arg is changed to be the pacman that the make builds. The '-p' arg is still available, but it should now be very rare to have to use it when calling pactest manually. --- Makefile.am | 4 -- configure.ac | 1 + test/pacman/.gitignore | 1 + test/pacman/pactest.py | 125 -------------------------------------------- test/pacman/pactest.py.in | 128 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 130 insertions(+), 129 deletions(-) delete mode 100755 test/pacman/pactest.py create mode 100644 test/pacman/pactest.py.in
Anyone want to point out a git command to allow us to see the differences between the old pactest.py and the new pactest.py.in. Otherwise, I'd need to see this as two patches - one to move the file and the second to make the adjustments. Allan
2013/12/24 Allan McRae <allan@archlinux.org>:
On 24/12/13 01:05, Jeremy Heiner wrote:
This patch is not ready for accepting into the repository.
It does not work for VPATH builds (because the paths to all the .py tests are seen as relative to $(builddir)). Dave is working on a fix for this VPATH issue, so a real version of this patch must wait to incorporate his commit. This patch works fine for in-$(srcdir) builds.
The purpose of posting it is to follow up Saturday's conversation. I think it implements all of Allan's suggestions (if I understood correctly). The real commit message would be something like:
SCRIPTLET_SHELL and LDCONFIG can be set via args to configure, and up 'till now those options were passed to pactest by Makefile.am as command-line args. That works fine for 'make check', but required repeated specification when running pactest manually. This patch makes pactest a configured file so it has direct access to those options and they no longer need to be specified (by Makefile.am nor by hand).
Also the default for the pactest '-p' arg is changed to be the pacman that the make builds. The '-p' arg is still available, but it should now be very rare to have to use it when calling pactest manually. --- Makefile.am | 4 -- configure.ac | 1 + test/pacman/.gitignore | 1 + test/pacman/pactest.py | 125 -------------------------------------------- test/pacman/pactest.py.in | 128 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 130 insertions(+), 129 deletions(-) delete mode 100755 test/pacman/pactest.py create mode 100644 test/pacman/pactest.py.in
Anyone want to point out a git command to allow us to see the differences between the old pactest.py and the new pactest.py.in.
Otherwise, I'd need to see this as two patches - one to move the file and the second to make the adjustments.
Allan
After you applied the patch, you can try `git diff -M HEAD~ HEAD`.
Cool: 'git format-patch' also accepts '-M' (I'll resend). Let's hope the recipie Allan follows to grab patches from the list works with it. Anyone know if there's a .gitconfig that makes '-M' the default?
This patch is not ready for accepting into the repository. It does not work for VPATH builds (because the paths to all the .py tests are seen as relative to $(builddir)). Dave is working on a fix for this VPATH issue, so a real version of this patch must wait to incorporate his commit. This patch works fine for in-$(srcdir) builds. The purpose of posting it is to follow up Saturday's conversation. I think it implements all of Allan's suggestions (if I understood correctly). The real commit message would be something like: SCRIPTLET_SHELL and LDCONFIG can be set via args to configure, and up 'till now those options were passed to pactest by Makefile.am as command-line args. That works fine for 'make check', but required repeated specification when running pactest manually. This patch makes pactest a configured file so it has direct access to those options and they no longer need to be specified (by Makefile.am nor by hand). Also the default for the pactest '-p' arg is changed to be the pacman that the make builds. The '-p' arg is still available, but it should now be very rare to have to use it when calling pactest manually. --- Makefile.am | 4 ---- configure.ac | 1 + test/pacman/.gitignore | 1 + test/pacman/{pactest.py => pactest.py.in} | 21 ++++++++++++--------- 4 files changed, 14 insertions(+), 13 deletions(-) rename test/pacman/{pactest.py => pactest.py.in} (88%) mode change 100755 => 100644 diff --git a/Makefile.am b/Makefile.am index 4d5adae..9689992 100644 --- a/Makefile.am +++ b/Makefile.am @@ -41,10 +41,6 @@ LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ PY_LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ $(top_srcdir)/build-aux/tap-driver.sh PY_LOG_COMPILER = $(top_srcdir)/test/pacman/pactest.py -AM_PY_LOG_FLAGS = \ - --scriptlet-shell $(SCRIPTLET_SHELL) \ - --ldconfig $(LDCONFIG) \ - -p $(top_builddir)/src/pacman/pacman # create the pacman DB and cache directories upon install install-data-local: diff --git a/configure.ac b/configure.ac index cfcc8d1..292260a 100644 --- a/configure.ac +++ b/configure.ac @@ -507,6 +507,7 @@ test/util/Makefile contrib/Makefile Makefile ]) +AC_CONFIG_FILES([test/pacman/pactest.py],[chmod +x test/pacman/pactest.py]) AC_OUTPUT echo " diff --git a/test/pacman/.gitignore b/test/pacman/.gitignore index 0d20b64..39fa72b 100644 --- a/test/pacman/.gitignore +++ b/test/pacman/.gitignore @@ -1 +1,2 @@ *.pyc +pactest.py diff --git a/test/pacman/pactest.py b/test/pacman/pactest.py.in old mode 100755 new mode 100644 similarity index 88% rename from test/pacman/pactest.py rename to test/pacman/pactest.py.in index e21cde7..0d9e1f7 --- a/test/pacman/pactest.py +++ b/test/pacman/pactest.py.in @@ -1,5 +1,7 @@ #! /usr/bin/python2 # +# @configure_input@ +# # pactest : run automated testing on the pacman binary # # Copyright (c) 2006 by Aurelien Foret <orelien@chez.com> @@ -25,6 +27,12 @@ import sys import tempfile +BUILDDIR = os.path.dirname(os.path.realpath(__file__)) +SRCDIR = os.path.realpath(os.path.join(BUILDDIR, "@srcdir@")) +if BUILDDIR != SRCDIR: sys.path.insert(0, SRCDIR) +TOP_BUILDDIR = os.path.realpath(os.path.join(BUILDDIR, "@top_builddir@")) +BUILT_EXE = os.path.realpath(os.path.join(TOP_BUILDDIR, "src/pacman/pacman")) + import pmenv import tap import util @@ -50,7 +58,7 @@ def create_parser(): help = "set debug level for pacman") parser.add_option("-p", "--pacman", action = "callback", callback = resolve_binary_path, type = "string", - dest = "bin", default = "pacman", + dest = "bin", default = BUILT_EXE, help = "specify location of the pacman binary") parser.add_option("--keep-root", action = "store_true", dest = "keeproot", default = False, @@ -67,12 +75,6 @@ def create_parser(): parser.add_option("--manual-confirm", action = "store_true", dest = "manualconfirm", default = False, help = "do not use --noconfirm for pacman calls") - parser.add_option("--scriptlet-shell", type = "string", - dest = "scriptletshell", default = "/bin/sh", - help = "specify path to shell used for install scriptlets") - parser.add_option("--ldconfig", type = "string", - dest = "ldconfig", default = "/sbin/ldconfig", - help = "specify path to ldconfig") return parser @@ -97,8 +99,9 @@ def create_parser(): env.pacman["gdb"] = opts.gdb env.pacman["valgrind"] = opts.valgrind env.pacman["manual-confirm"] = opts.manualconfirm - env.pacman["scriptlet-shell"] = opts.scriptletshell - env.pacman["ldconfig"] = opts.ldconfig + # and add configured options + env.pacman["scriptlet-shell"] = "@SCRIPTLET_SHELL@" + env.pacman["ldconfig"] = "@LDCONFIG@" opts.testcases = [] for path in args: -- 1.8.5.2
On 24/12/13 23:11, Jeremy Heiner wrote:
This patch is not ready for accepting into the repository.
It does not work for VPATH builds (because the paths to all the .py tests are seen as relative to $(builddir)). Dave is working on a fix for this VPATH issue, so a real version of this patch must wait to incorporate his commit. This patch works fine for in-$(srcdir) builds.
The purpose of posting it is to follow up Saturday's conversation. I think it implements all of Allan's suggestions (if I understood correctly). The real commit message would be something like:
SCRIPTLET_SHELL and LDCONFIG can be set via args to configure, and up 'till now those options were passed to pactest by Makefile.am as command-line args. That works fine for 'make check', but required repeated specification when running pactest manually. This patch makes pactest a configured file so it has direct access to those options and they no longer need to be specified (by Makefile.am nor by hand).
I would really like SCRIPTLET_SHELL to still be at test run time. My system pacman may have a different shell that the one built in the source tree. Keep it defaulting to the configured value, but changeable. Removing the flag to set LDCONFIG is fine for me. But I like other opinions on this to make sure I'm not missing something.
Also the default for the pactest '-p' arg is changed to be the pacman that the make builds. The '-p' arg is still available, but it should now be very rare to have to use it when calling pactest manually. --- Makefile.am | 4 ---- configure.ac | 1 + test/pacman/.gitignore | 1 + test/pacman/{pactest.py => pactest.py.in} | 21 ++++++++++++--------- 4 files changed, 14 insertions(+), 13 deletions(-) rename test/pacman/{pactest.py => pactest.py.in} (88%) mode change 100755 => 100644
diff --git a/Makefile.am b/Makefile.am index 4d5adae..9689992 100644 --- a/Makefile.am +++ b/Makefile.am @@ -41,10 +41,6 @@ LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ PY_LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ $(top_srcdir)/build-aux/tap-driver.sh PY_LOG_COMPILER = $(top_srcdir)/test/pacman/pactest.py -AM_PY_LOG_FLAGS = \ - --scriptlet-shell $(SCRIPTLET_SHELL) \ - --ldconfig $(LDCONFIG) \ - -p $(top_builddir)/src/pacman/pacman
# create the pacman DB and cache directories upon install install-data-local: diff --git a/configure.ac b/configure.ac index cfcc8d1..292260a 100644 --- a/configure.ac +++ b/configure.ac @@ -507,6 +507,7 @@ test/util/Makefile contrib/Makefile Makefile ]) +AC_CONFIG_FILES([test/pacman/pactest.py],[chmod +x test/pacman/pactest.py]) AC_OUTPUT
echo " diff --git a/test/pacman/.gitignore b/test/pacman/.gitignore index 0d20b64..39fa72b 100644 --- a/test/pacman/.gitignore +++ b/test/pacman/.gitignore @@ -1 +1,2 @@ *.pyc +pactest.py diff --git a/test/pacman/pactest.py b/test/pacman/pactest.py.in old mode 100755 new mode 100644 similarity index 88% rename from test/pacman/pactest.py rename to test/pacman/pactest.py.in index e21cde7..0d9e1f7 --- a/test/pacman/pactest.py +++ b/test/pacman/pactest.py.in @@ -1,5 +1,7 @@ #! /usr/bin/python2 # +# @configure_input@ +# # pactest : run automated testing on the pacman binary # # Copyright (c) 2006 by Aurelien Foret <orelien@chez.com> @@ -25,6 +27,12 @@ import sys import tempfile
+BUILDDIR = os.path.dirname(os.path.realpath(__file__)) +SRCDIR = os.path.realpath(os.path.join(BUILDDIR, "@srcdir@")) +if BUILDDIR != SRCDIR: sys.path.insert(0, SRCDIR) +TOP_BUILDDIR = os.path.realpath(os.path.join(BUILDDIR, "@top_builddir@")) +BUILT_EXE = os.path.realpath(os.path.join(TOP_BUILDDIR, "src/pacman/pacman")) + import pmenv import tap import util @@ -50,7 +58,7 @@ def create_parser(): help = "set debug level for pacman") parser.add_option("-p", "--pacman", action = "callback", callback = resolve_binary_path, type = "string", - dest = "bin", default = "pacman", + dest = "bin", default = BUILT_EXE, help = "specify location of the pacman binary") parser.add_option("--keep-root", action = "store_true", dest = "keeproot", default = False, @@ -67,12 +75,6 @@ def create_parser(): parser.add_option("--manual-confirm", action = "store_true", dest = "manualconfirm", default = False, help = "do not use --noconfirm for pacman calls") - parser.add_option("--scriptlet-shell", type = "string", - dest = "scriptletshell", default = "/bin/sh", - help = "specify path to shell used for install scriptlets") - parser.add_option("--ldconfig", type = "string", - dest = "ldconfig", default = "/sbin/ldconfig", - help = "specify path to ldconfig") return parser
@@ -97,8 +99,9 @@ def create_parser(): env.pacman["gdb"] = opts.gdb env.pacman["valgrind"] = opts.valgrind env.pacman["manual-confirm"] = opts.manualconfirm - env.pacman["scriptlet-shell"] = opts.scriptletshell - env.pacman["ldconfig"] = opts.ldconfig + # and add configured options + env.pacman["scriptlet-shell"] = "@SCRIPTLET_SHELL@" + env.pacman["ldconfig"] = "@LDCONFIG@"
opts.testcases = [] for path in args:
Allan McRae <allan@archlinux.org> wrote:
I would really like SCRIPTLET_SHELL to still be at test run time. My system pacman may have a different shell that the one built in the source tree. Keep it defaulting to the configured value, but changeable.
Hiya, Allan: the autoconf tools (assuming VPATH builds work) already provide everything needed to achieve your goal: just configure a builddir --with-scriptlet-shell matching your system pacman. There's really no need to ever have to type the scriptlet shell more than once (as an arg to configure). Or are you scheming for a RSI worker's comp settlement? Jeremy
On 28/12/13 04:02, Jeremy Heiner wrote:
Allan McRae <allan@archlinux.org> wrote:
I would really like SCRIPTLET_SHELL to still be at test run time. My system pacman may have a different shell that the one built in the source tree. Keep it defaulting to the configured value, but changeable.
Hiya, Allan: the autoconf tools (assuming VPATH builds work) already provide everything needed to achieve your goal: just configure a builddir --with-scriptlet-shell matching your system pacman. There's really no need to ever have to type the scriptlet shell more than once (as an arg to configure). Or are you scheming for a RSI worker's comp settlement?
So I need to reconfigure in a different directory to run with different pacman? No thanks. Allan
Allan McRae <allan@archlinux.org> wrote:
So I need to reconfigure in a different directory to run with different pacman? No thanks.
No, not reconfigure. Just configure. One time, to create the builddir.
From then on, until you 'rm -r' it, that buildir remembers the --with-scriptlet-shell (and other configured args) you set it up with. It is connected to your srcdir (== git workdir) so it sees whatever branch / proposed patch you load there, making it very easy to recompile with the alternate scriptlet-shell, run new tests against your system pacman, or any other thing you need to do under those configured args.
If you are using in-srcdir builds then you are doing reconfigures that are wasting a lot of your time. Maybe it doesn't happen every day, but when someone submits a patch to you that pertains to a configuration other than your current in-srcdir build what do you do? Take the Doxygen patches I posted recently, for example. Don't you need to try those out with at least both --enable- and --disable-doxygen? Please don't tell me you tried it out in-srcdir with your standard configure args, then, after that was done, reconfigured and retested, and then reconfigured back to restore your working dir config! With multiple VPATH builddirs you could avoid all that reconfiguring and even run the makes in parallel. I am completely flummoxed. Your time is infinitely more valuable than any disk space spent on VPATH builddirs. You're very smart, so it will take you little time to adjust your workflow to use these tools. And the payoff will be much less of your time wasted. Good grief! I sound like a shill for autoconf. Sorry about that. I would have to be really desperate before I took that job, even if it actually existed. Dave: I ceded to your desire to author the patch that fixes VPATH builds. So you must understand what I'm saying. Back me up here. And somebody please reassure me there's a CI running with a good cross-product of the configurables. Jeremy
On 28/12/13 12:15, Jeremy Heiner wrote:
Allan McRae <allan@archlinux.org> wrote:
So I need to reconfigure in a different directory to run with different pacman? No thanks.
No, not reconfigure. Just configure. One time, to create the builddir.
From then on, until you 'rm -r' it, that buildir remembers the --with-scriptlet-shell (and other configured args) you set it up with. It is connected to your srcdir (== git workdir) so it sees whatever branch / proposed patch you load there, making it very easy to recompile with the alternate scriptlet-shell, run new tests against your system pacman, or any other thing you need to do under those configured args.
If you are using in-srcdir builds then you are doing reconfigures that are wasting a lot of your time. Maybe it doesn't happen every day, but when someone submits a patch to you that pertains to a configuration other than your current in-srcdir build what do you do? Take the Doxygen patches I posted recently, for example. Don't you need to try those out with at least both --enable- and --disable-doxygen? Please don't tell me you tried it out in-srcdir with your standard configure args, then, after that was done, reconfigured and retested, and then reconfigured back to restore your working dir config!
Yes... lets pretend I have looked at those patches... :)
With multiple VPATH builddirs you could avoid all that reconfiguring and even run the makes in parallel.
I am completely flummoxed. Your time is infinitely more valuable than any disk space spent on VPATH builddirs. You're very smart, so it will take you little time to adjust your workflow to use these tools. And the payoff will be much less of your time wasted.
Or I could stick to using the aliases I have for years "pactest-local" and "pactest-system". Your patch allows me to get rid of pactest-local, which is nice. What I do not like is that your patch is taking away options. It would have been less work to leave in support for those flags than to remove them. VPATH builddir support is gained, so we have two methods. Given we are championing the autotools way, good autoconf macros automatically set a value but provide a flag to override it. Allan
Allan McRae <allan@archlinux.org> wrote:
What I do not like is that your patch is taking away options. It would have been less work to leave in support for those flags than to remove them.
Taking away options is what software engineering is all about. Module. Class. Encapsulation. API. Any script kiddy can barf up a mess that can (sorta) get it done. Is that really the direction you want to steer this project in? I don't care how much less work it would have been to leave those unmentionables unmentioned. Burying dirty laundry never results in clean linen.
Yes... lets pretend I have looked at those patches... :)
That is my whole point! You spent too much time looking at the patches I submitted. I thought: "obviously he's using the autoconf tools so this patch won't take up much of his time". But I was apparently wrong. I apologize. Yet when I suggest using decades-old tools to alleviate the problem the response is "no thanks." Odd.
Given we are championing the autotools way, good autoconf macros automatically set a value but provide a flag to override it.
Who's this "we"?
On 28/12/13 15:21, Jeremy Heiner wrote:
Allan McRae <allan@archlinux.org> wrote:
What I do not like is that your patch is taking away options. It would have been less work to leave in support for those flags than to remove them.
Taking away options is what software engineering is all about. Module. Class. Encapsulation. API. Any script kiddy can barf up a mess that can (sorta) get it done. Is that really the direction you want to steer this project in?
...
I don't care how much less work it would have been to leave those unmentionables unmentioned. Burying dirty laundry never results in clean linen.
Your patch keeps the flag to allow us to use the system pacman but not the flag to allow us to set its scriptlet shell if it is different from the pacman in the build directory. So to actually use a system pacman with a different scriptlet shell will now require a separate build directory configured with the appropriate shell. That is completely non-obvious. Taking away a the --scriptlet-shell option and in the process making the -p potentially require a non-obvious process to provide an appropriate shell seems a poor decision to me.
Yes... lets pretend I have looked at those patches... :)
That is my whole point! You spent too much time looking at the patches I submitted. I thought: "obviously he's using the autoconf tools so this patch won't take up much of his time".
I have not even applied the patches yet, so time is not spent (re)building. Actually building with a patch applied should be the most minor part of reviewing a patch.
But I was apparently wrong. I apologize. Yet when I suggest using decades-old tools to alleviate the problem the response is "no thanks." Odd.
There was no problem. This is still no problem if I do not push a patch removing an option from pactest. I will not spend more time replying to this thread. Patch rejected by me unless the --scriptlet-shell options is kept in pactest.py. Alternatively, there needs to be a community consensus that this is the right thing to do, in which case my decision gets overridden. Allan
On Dec 24, Jeremy Heiner <scalaprotractor@gmail.com> wrote:
Anyone know if there's a .gitconfig that makes '-M' the default?
[diff] renames = true
participants (3)
-
Allan McRae
-
Jeremy Heiner
-
郑文辉(Techlive Zheng)