From 25f0907d05607f6a4030f871c0f1f0f0cc757b64 Mon Sep 17 00:00:00 2001 From: Nicolas F Date: Fri, 8 Mar 2019 16:48:31 +0100 Subject: [PATCH 1/2] overviewer: replace optparse with argparse optparse is deprecated, and switching to argparse is mostly but not entirely trivial. Nicely telling the user about shell quoting had to be removed because I didn't see an easy way to do that with argparse. --- overviewer.py | 198 +++++++++++++++++++++----------------------------- 1 file changed, 84 insertions(+), 114 deletions(-) diff --git a/overviewer.py b/overviewer.py index 838bff8..b5f8182 100755 --- a/overviewer.py +++ b/overviewer.py @@ -32,7 +32,7 @@ import subprocess import multiprocessing import time import logging -from optparse import OptionParser, OptionGroup +from argparse import ArgumentParser from overviewer_core import util from overviewer_core import logger @@ -44,8 +44,8 @@ from overviewer_core import observer from overviewer_core.nbt import CorruptNBTError helptext = """ -%prog [--rendermodes=...] [options] -%prog --config= [options]""" +%(prog)s [--rendermodes=...] [options] +%(prog)s --config= [options]""" def main(): @@ -70,103 +70,100 @@ def main(): avail_north_dirs = ['lower-left', 'upper-left', 'upper-right', 'lower-right', 'auto'] # Parse for basic options - parser = OptionParser(usage=helptext, add_help_option=False) - parser.add_option("-h", "--help", dest="help", action="store_true", - help="Show this help message and exit.") - parser.add_option("-c", "--config", dest="config", action="store", - help="Specify the config file to use.") - parser.add_option("-p", "--processes", dest="procs", action="store", type="int", - help="The number of local worker processes to spawn. Defaults to the number " - "of CPU cores your computer has.") + parser = ArgumentParser(usage=helptext) + parser.add_argument("-c", "--config", dest="config", action="store", + help="Specify the config file to use.") + parser.add_argument("-p", "--processes", dest="procs", action="store", type=int, + help="The number of local worker processes to spawn. Defaults to the " + "number of CPU cores your computer has.") - parser.add_option("--pid", dest="pid", action="store", help="Specify the pid file to use.") + parser.add_argument("--pid", dest="pid", action="store", help="Specify the pid file to use.") # Options that only apply to the config-less render usage - parser.add_option("--rendermodes", dest="rendermodes", action="store", - help="If you're not using a config file, specify which rendermodes to " - "render with this option. This is a comma-separated list.") + parser.add_argument("--rendermodes", dest="rendermodes", action="store", + help="If you're not using a config file, specify which rendermodes to " + "render with this option. This is a comma-separated list.") + parser.add_argument("world", nargs='?', + help="Path or name of the world you want to render.") + parser.add_argument("output", nargs='?', + help="Output directory for the rendered map.") # Useful one-time render modifiers: - parser.add_option("--forcerender", dest="forcerender", action="store_true", - help="Force re-render the entire map.") - parser.add_option("--check-tiles", dest="checktiles", action="store_true", - help="Check each tile on disk and re-render old tiles.") - parser.add_option("--no-tile-checks", dest="notilechecks", action="store_true", - help="Only render tiles that come from chunks that have changed since the " - "last render (the default).") + render_modifiers = parser.add_mutually_exclusive_group() + render_modifiers.add_argument("--forcerender", dest="forcerender", action="store_true", + help="Force re-render the entire map.") + render_modifiers.add_argument("--check-tiles", dest="checktiles", action="store_true", + help="Check each tile on disk and re-render old tiles.") + render_modifiers.add_argument("--no-tile-checks", dest="notilechecks", action="store_true", + help="Only render tiles that come from chunks that have changed " + "since the last render (the default).") # Useful one-time debugging options: - parser.add_option("--check-terrain", dest="check_terrain", action="store_true", - help="Try to locate the texture files. Useful for debugging texture" - " problems.") - parser.add_option("-V", "--version", dest="version", - help="Display version information and then exits.", action="store_true") - parser.add_option("--check-version", dest="checkversion", - help="Fetch information about the latest version of Overviewer.", - action="store_true") - parser.add_option("--update-web-assets", dest='update_web_assets', action="store_true", - help="Update web assets. Will *not* render tiles or update " - "overviewerConfig.js.") + parser.add_argument("--check-terrain", dest="check_terrain", action="store_true", + help="Try to locate the texture files. Useful for debugging texture" + " problems.") + parser.add_argument("-V", "--version", dest="version", + help="Display version information and then exits.", action="store_true") + parser.add_argument("--check-version", dest="checkversion", + help="Fetch information about the latest version of Overviewer.", + action="store_true") + parser.add_argument("--update-web-assets", dest='update_web_assets', action="store_true", + help="Update web assets. Will *not* render tiles or update " + "overviewerConfig.js.") # Log level options: - parser.add_option("-q", "--quiet", dest="quiet", action="count", default=0, - help="Print less output. You can specify this option multiple times.") - parser.add_option("-v", "--verbose", dest="verbose", action="count", default=0, - help="Print more output. You can specify this option multiple times.") - parser.add_option("--simple-output", dest="simple", action="store_true", default=False, - help="Use a simple output format, with no colors or progress bars.") + parser.add_argument("-q", "--quiet", dest="quiet", action="count", default=0, + help="Print less output. You can specify this option multiple times.") + parser.add_argument("-v", "--verbose", dest="verbose", action="count", default=0, + help="Print more output. You can specify this option multiple times.") + parser.add_argument("--simple-output", dest="simple", action="store_true", default=False, + help="Use a simple output format, with no colors or progress bars.") # create a group for "plugin exes" # (the concept of a plugin exe is only loosely defined at this point) - exegroup = OptionGroup(parser, "Other Scripts", - "These scripts may accept different arguments than the ones " - "listed above") - exegroup.add_option("--genpoi", dest="genpoi", action="store_true", - help="Run the genPOI script.") - exegroup.add_option("--skip-scan", dest="skipscan", action="store_true", - help="When running GenPOI, don't scan for entities.") - exegroup.add_option("--skip-players", dest="skipplayers", action="store_true", - help="When running GenPOI, don't scan player data.") + exegroup = parser.add_argument_group("Other Scripts", "These scripts may accept different " + "arguments than the ones listed above.") + exegroup.add_argument("--genpoi", dest="genpoi", action="store_true", + help="Run the genPOI script.") + exegroup.add_argument("--skip-scan", dest="skipscan", action="store_true", + help="When running GenPOI, don't scan for entities.") + exegroup.add_argument("--skip-players", dest="skipplayers", action="store_true", + help="When running GenPOI, don't scan player data.") - parser.add_option_group(exegroup) - - options, args = parser.parse_args() + args = parser.parse_args() # first thing to do is check for stuff in the exegroup: - if options.genpoi: + if args.genpoi: # remove the "--genpoi" option from sys.argv before running genPI sys.argv.remove("--genpoi") g = __import__("overviewer_core.aux_files", {}, {}, ["genPOI"]) g.genPOI.main() return 0 - if options.help: - parser.print_help() - return 0 # re-configure the logger now that we've processed the command line options - logger.configure(logging.INFO + 10 * options.quiet - 10 * options.verbose, - verbose=options.verbose > 0, simple=options.simple) + logger.configure(logging.INFO + 10 * args.quiet - 10 * args.verbose, + verbose=args.verbose > 0, simple=args.simple) ########################################################################## # This section of main() runs in response to any one-time options we have, # such as -V for version reporting - if options.version: + if args.version: print("Minecraft Overviewer %s" % util.findGitVersion() + " (%s)" % util.findGitHash()[:7]) try: import overviewer_core.overviewer_version as overviewer_version print("built on %s" % overviewer_version.BUILD_DATE) - if options.verbose > 0: + if args.verbose > 0: print("Build machine: %s %s" % (overviewer_version.BUILD_PLATFORM, overviewer_version.BUILD_OS)) print("Read version information from %r" % overviewer_version.__file__) except ImportError: print("(build info not found)") - if options.verbose > 0: + if args.verbose > 0: print("Python executable: %r" % sys.executable) print(sys.version) - if not options.checkversion: + if not args.checkversion: return 0 - if options.checkversion: + if args.checkversion: print("Currently running Minecraft Overviewer %s" % util.findGitVersion() + " (%s)" % util.findGitHash()[:7]) try: @@ -179,7 +176,7 @@ def main(): print("See https://overviewer.org/downloads for more information.") except Exception: print("Failed to fetch latest version info.") - if options.verbose > 0: + if args.verbose > 0: import traceback traceback.print_exc() else: @@ -187,22 +184,22 @@ def main(): return 1 return 0 - if options.pid: - if os.path.exists(options.pid): + if args.pid: + if os.path.exists(args.pid): try: - with open(options.pid, 'r') as fpid: + with open(args.pid, 'r') as fpid: pid = int(fpid.read()) if util.pid_exists(pid): print("Overviewer is already running (pid exists) - exiting.") return 0 except (IOError, ValueError): pass - with open(options.pid, "w") as f: + with open(args.pid, "w") as f: f.write(str(os.getpid())) # if --check-terrain was specified, but we have NO config file, then we cannot # operate on a custom texture path. we do terrain checking with a custom texture # pack later on, after we've parsed the config file - if options.check_terrain and not options.config: + if args.check_terrain and not args.config: import hashlib from overviewer_core.textures import Textures tex = Textures() @@ -220,7 +217,7 @@ def main(): return 0 # if no arguments are provided, print out a helpful message - if len(args) == 0 and not options.config: + if not (args.world and args.output) and not args.config: # first provide an appropriate error for bare-console users # that don't provide any options if util.is_bare_console(): @@ -244,7 +241,7 @@ def main(): # This section does some sanity checking on the command line options passed # in. It checks to see if --config was given that no worldname/destdir were # given, and vice versa - if options.config and args: + if args.config and (args.world and args.output): print() print("If you specify --config, you need to specify the world to render as well as " "the destination in the config file, not on the command line.") @@ -257,34 +254,19 @@ def main(): parser.print_help() return 1 - if not options.config and len(args) < 2: + if not args.config and (args.world or args.output) and not (args.world and args.output): logging.error("You must specify both the world directory and an output directory") parser.print_help() return 1 - if not options.config and len(args) > 2: - # it's possible the user has a space in one of their paths but didn't - # properly escape it attempt to detect this case - for start in range(len(args)): - if not os.path.exists(args[start]): - for end in range(start + 1, len(args) + 1): - if os.path.exists(" ".join(args[start:end])): - logging.warning( - "It looks like you meant to specify \"%s\" as your world dir or your " - "output dir but you forgot to put quotes around the directory, since " - "it contains spaces." % " ".join(args[start:end])) - return 1 - logging.error("Too many command line arguments") - parser.print_help() - return 1 ######################################################################### # These two halfs of this if statement unify config-file mode and # command-line mode. mw_parser = configParser.MultiWorldParser() - if not options.config: + if not args.config: # No config file mode. - worldpath, destdir = map(os.path.expanduser, args) + worldpath, destdir = map(os.path.expanduser, [args.world, args.output]) logging.debug("Using %r as the world directory", worldpath) logging.debug("Using %r as the output directory", destdir) @@ -292,8 +274,8 @@ def main(): mw_parser.set_config_item("outputdir", destdir) rendermodes = ['lighting'] - if options.rendermodes: - rendermodes = options.rendermodes.replace("-", "_").split(",") + if args.rendermodes: + rendermodes = args.rendermodes.replace("-", "_").split(",") # Now for some good defaults renders = util.OrderedDict() @@ -306,7 +288,7 @@ def main(): mw_parser.set_config_item("renders", renders) else: - if options.rendermodes: + if args.rendermodes: logging.error("You cannot specify --rendermodes if you give a config file. " "Configure your rendermodes in the config file instead.") parser.print_help() @@ -314,7 +296,7 @@ def main(): # Parse the config file try: - mw_parser.parse(os.path.expanduser(options.config)) + mw_parser.parse(os.path.expanduser(args.config)) except configParser.MissingConfigException as e: # this isn't a "bug", so don't print scary traceback logging.error(str(e)) @@ -322,14 +304,14 @@ def main(): # Add in the command options here, perhaps overriding values specified in # the config - if options.procs: - mw_parser.set_config_item("processes", options.procs) + if args.procs: + mw_parser.set_config_item("processes", args.procs) # Now parse and return the validated config try: config = mw_parser.get_validated_config() except Exception as ex: - if options.verbose: + if args.verbose: logging.exception("An error was encountered with your configuration. " "See the information below.") else: # no need to print scary traceback! @@ -337,7 +319,7 @@ def main(): logging.error(str(ex)) return 1 - if options.check_terrain: # we are already in the "if configfile" branch + if args.check_terrain: # we are already in the "if configfile" branch logging.info("Looking for a few common texture files...") for render_name, render in config['renders'].iteritems(): logging.info("Looking at render %r.", render_name) @@ -357,18 +339,6 @@ def main(): logging.info("Welcome to Minecraft Overviewer!") logging.debug("Current log level: {0}.".format(logging.getLogger().level)) - # Override some render configdict options depending on one-time command line - # modifiers - if ( - bool(options.forcerender) + - bool(options.checktiles) + - bool(options.notilechecks) - ) > 1: - logging.error("You cannot specify more than one of --forcerender, " - "--check-tiles, and --no-tile-checks. These options conflict.") - parser.print_help() - return 1 - def set_renderchecks(checkname, num): for name, render in config['renders'].iteritems(): if render.get('renderchecks', 0) == 3: @@ -377,13 +347,13 @@ def main(): else: render['renderchecks'] = num - if options.forcerender: + if args.forcerender: logging.info("Forcerender mode activated. ALL tiles will be rendered.") set_renderchecks("forcerender", 2) - elif options.checktiles: + elif args.checktiles: logging.info("Checking all tiles for updates manually.") set_renderchecks("checktiles", 1) - elif options.notilechecks: + elif args.notilechecks: logging.info("Disabling all tile mtime checks. Only rendering tiles " "that need updating since last render.") set_renderchecks("notilechecks", 0) @@ -444,7 +414,7 @@ def main(): assetMrg = assetmanager.AssetManager(destdir, config.get('customwebassets', None)) # If we've been asked to update web assets, do that and then exit - if options.update_web_assets: + if args.update_web_assets: assetMrg.output_noconfig() logging.info("Web assets have been updated.") return 0 @@ -594,8 +564,8 @@ def main(): logging.debug("Final cache stats:") for c in caches: logging.debug("\t%s: %s hits, %s misses", c.__class__.__name__, c.hits, c.misses) - if options.pid: - os.remove(options.pid) + if args.pid: + os.remove(args.pid) logging.info("Your render has been written to '%s', open index.html to view it." % destdir) From 3a923c29327690545db16e9f0d37dbc7b8a4a12b Mon Sep 17 00:00:00 2001 From: Nicolas F Date: Sat, 9 Mar 2019 18:26:43 +0100 Subject: [PATCH 2/2] overviewer: warn about shell quoting issues This reimplements the warning about missing quotes on the command line that was removed with the previous commit. --- overviewer.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/overviewer.py b/overviewer.py index b5f8182..a2b5593 100755 --- a/overviewer.py +++ b/overviewer.py @@ -129,7 +129,21 @@ def main(): exegroup.add_argument("--skip-players", dest="skipplayers", action="store_true", help="When running GenPOI, don't scan player data.") - args = parser.parse_args() + args, unknowns = parser.parse_known_args() + + # Check for possible shell quoting issues + if len(unknowns) > 0: + possible_mistakes = [] + for i in xrange(len(unknowns) + 1): + possible_mistakes.append(" ".join([args.world, args.output] + unknowns[:i])) + possible_mistakes.append(" ".join([args.output] + unknowns[:i])) + for mistake in possible_mistakes: + if os.path.exists(mistake): + logging.warning("Looks like you tried to make me use {0} as an argument, but " + "forgot to quote the argument correctly. Try using \"{0}\" " + "instead if the spaces are part of the path.".format(mistake)) + parser.error("Too many arguments.") + parser.error("Too many arguments.") # first thing to do is check for stuff in the exegroup: if args.genpoi: