diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index d1370aa457e..bfcbe391cb0 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -25,7 +25,8 @@ jobs: python-version: - "3.10" - "3.13" - fail-fast: true + - "3.14" + fail-fast: false runs-on: ${{ matrix.os }} env: diff --git a/python/grass/pygrass/modules/grid/grid.py b/python/grass/pygrass/modules/grid/grid.py index a177fd76c83..6aeec060ada 100644 --- a/python/grass/pygrass/modules/grid/grid.py +++ b/python/grass/pygrass/modules/grid/grid.py @@ -352,6 +352,15 @@ def get_cmd(cmdd): ] +def _grass_worker_init(): + """Initialize GRASS environment in spawned worker processes (Windows/spawn).""" + gisbase = os.environ.get("GISBASE", "") + if gisbase and gisbase not in sys.path: + etc_python = os.path.join(gisbase, "etc", "python") + if etc_python not in sys.path: + sys.path.insert(0, etc_python) + + def cmd_exe(args): """Create a mapset, and execute a cmd inside. @@ -383,7 +392,7 @@ def cmd_exe(args): # reset the inputs to for key in mapnames: inputs[key] = mapnames[key] - cmd["inputs"] = inputs.items() + cmd["inputs"] = list(inputs.items()) # set the region to the tile sub.Popen(["g.region", "raster=%s" % key], shell=shell, env=env).wait() else: @@ -615,6 +624,8 @@ def get_works(self): else: ldst, gdst = self.mset.location, self.mset.gisdbase cmd = self.module.get_dict() + cmd["inputs"] = list(cmd["inputs"]) + cmd["outputs"] = list(cmd["outputs"]) groups = list(select(self.module.inputs, "group")) for row, box_row in enumerate(self.bboxes): for col, box in enumerate(box_row): @@ -628,14 +639,12 @@ def get_works(self): inms[key] = "%s@%s" % (self.inlist[key][indx], self.mset.name) # set the computational region, prepare the region parameters bbox = { - **{k[0]: str(v) for k, v in box.items()[:-2]}, + **{k[0]: str(v) for k, v in list(box.items())[:-2]}, "nsres": "%f" % reg.nsres, "ewres": "%f" % reg.ewres, } - new_mset = ( - self.msetstr % (self.start_row + row, self.start_col + col), - ) + new_mset = self.msetstr % (self.start_row + row, self.start_col + col) works.append( ( bbox, @@ -683,13 +692,19 @@ def _actual_run(self, patch): for wrk in self.get_works(): cmd_exe(wrk) else: - pool = mltp.Pool(processes=self.processes) + ctx = mltp.get_context("spawn") + ctx.set_executable(sys.executable) + pool = ctx.Pool(processes=self.processes, initializer=_grass_worker_init) result = pool.map_async(cmd_exe, self.get_works()) result.wait() pool.close() pool.join() if not result.successful(): - raise RuntimeError(_("Execution of subprocesses was not successful")) + try: + result.get() + except Exception as e: + msg = f"Worker failed with: {e}" + raise RuntimeError(msg) from e if patch: if self.move: diff --git a/python/grass/pygrass/modules/interface/module.py b/python/grass/pygrass/modules/interface/module.py index d2f06d097dc..4fc97f20137 100644 --- a/python/grass/pygrass/modules/interface/module.py +++ b/python/grass/pygrass/modules/interface/module.py @@ -745,6 +745,16 @@ def __str__(self): def __repr__(self): return "Module(%r)" % self.name + def __getstate__(self): + """Make Module picklable by excluding unpicklable popen object.""" + state = self.__dict__.copy() + state["_popen"] = None + return state + + def __setstate__(self, state): + """Restore Module from pickled state.""" + self.__dict__.update(state) + @docstring_property(__doc__) def __doc__(self): """{cmd_name}({cmd_params})""" diff --git a/python/grass/pygrass/modules/tests/grass_pygrass_grid_test.py b/python/grass/pygrass/modules/tests/grass_pygrass_grid_test.py index 5f49689688f..4be0d7fc6d2 100644 --- a/python/grass/pygrass/modules/tests/grass_pygrass_grid_test.py +++ b/python/grass/pygrass/modules/tests/grass_pygrass_grid_test.py @@ -1,18 +1,48 @@ """Test main functions of PyGRASS GridModule""" +import functools import multiprocessing - +import sys import pytest import grass.script as gs -from grass.pygrass.modules.grid import GridModule -xfail_mp_spawn = pytest.mark.xfail( - multiprocessing.get_start_method() == "spawn", - reason="Multiprocessing using 'spawn' start method requires pickable functions", - raises=AttributeError, - strict=True, -) + +def _run_grid_module(module_name, project, run_kwargs=None, **kwargs): + if run_kwargs is None: + run_kwargs = {} + + import sys + import os + + gisbase = os.environ.get("GISBASE", "") + if gisbase: + # Add GRASS's Python lib paths so grass.lib (C extensions) can be found + grass_python_paths = [ + p for p in sys.path if p and p.startswith(os.path.join(gisbase, "Python")) + ] + for p in grass_python_paths: + if p not in sys.path: + sys.path.insert(0, p) + os.environ["PYTHONPATH"] = os.pathsep.join( + grass_python_paths + [p for p in sys.path if p] + ) + + import grass.script as gs + + gs.setup.init(project) + + from grass.pygrass.modules.grid.grid import GridModule + + try: + grid = GridModule(module_name, **kwargs) + grid.run(**run_kwargs) + except Exception as e: + import traceback + + traceback.print_exc() + print("GridModule failed: " + str(e), file=sys.stderr) + sys.exit(1) def max_processes(): @@ -23,14 +53,73 @@ def max_processes(): # GridModule uses C libraries which can easily initialize only once # and thus can't easily change location/mapset, so we use a subprocess # to separate individual GridModule calls. -def run_in_subprocess(function): + + +def _run_with_env(env, sys_path, function): + """Set environment and sys.path then run function (for spawn subprocess)""" + import os + from pathlib import Path + import sys + + os.environ.update(env) + gisbase = env.get("GISBASE", "") + # Put GRASS installed paths FIRST so grass.lib (C extensions) are found + # before the development grass package which has no grass.lib + if gisbase: + # Ensure grass.lib (C extensions) are findable by putting etc/python first + etc_python = os.path.join(gisbase, "etc", "python") + # etc_python MUST be at index 0 so installed grass.lib (C extension) + # wins over source checkout grass package which has no grass.lib + grass_paths = [ + p for p in sys_path if p and p.startswith(gisbase) and p != etc_python + ] + # Exclude ANY path that has a "grass" subdirectory and is not under + # GISBASE -- such paths pollute the grass namespace package and hide + # grass.lib (C extensions) which only exist in the installed GRASS. + other_paths = [ + p + for p in sys_path + if p and not p.startswith(gisbase) and not Path(p, "grass").is_dir() + ] + sys.path[:] = [etc_python] + grass_paths + other_paths + # CRITICAL: grass was already imported during subprocess unpickling + # using the parent's sys.path (spawn sends it automatically). + # grass.__path__ therefore points to the source checkout which has + # no grass.lib. Clear all grass.* from sys.modules so they get + # re-imported cleanly using the corrected sys.path above. + for key in list(sys.modules.keys()): + if key == "grass" or key.startswith("grass."): + del sys.modules[key] + if hasattr(os, "add_dll_directory"): + for dll_dir in [ + os.path.join(gisbase, "bin"), + os.path.join(gisbase, "lib"), + os.path.join(gisbase, "extrabin"), + ]: + if Path(dll_dir).is_dir(): + os.add_dll_directory(dll_dir) + else: + sys.path[:] = sys_path + function() + + +def run_in_subprocess(function, check=True): """Run function in a separate process""" - process = multiprocessing.Process(target=function) + ctx = multiprocessing.get_context("spawn") + ctx.set_executable(sys.executable) # force user's Python, not GRASS's + import os + + process = ctx.Process( + target=_run_with_env, args=(dict(os.environ), list(sys.path), function) + ) process.start() process.join() + if check and process.exitcode != 0: + msg = f"Subprocess failed with exit code {process.exitcode}" + raise RuntimeError(msg) + return process.exitcode -@xfail_mp_spawn @pytest.mark.needs_solo_run @pytest.mark.parametrize("processes", list(range(1, max_processes() + 1)) + [None]) def test_processes(tmp_path, processes): @@ -43,9 +132,11 @@ def test_processes(tmp_path, processes): surface = "surface" gs.run_command("r.surf.fractal", output=surface) - def run_grid_module(): - grid = GridModule( + run_in_subprocess( + functools.partial( + _run_grid_module, "r.slope.aspect", + project=project, width=10, height=5, overlap=2, @@ -54,9 +145,7 @@ def run_grid_module(): slope="slope", aspect="aspect", ) - grid.run() - - run_in_subprocess(run_grid_module) + ) info = gs.raster_info("slope") assert info["min"] > 0 @@ -65,7 +154,6 @@ def run_grid_module(): # @pytest.mark.parametrize("split", [False]) # True does not work. -@xfail_mp_spawn @pytest.mark.parametrize("width", [5, 10, 50]) # None does not work. @pytest.mark.parametrize("height", [5, 10, 50]) def test_tiling_schemes(tmp_path, width, height): @@ -78,9 +166,11 @@ def test_tiling_schemes(tmp_path, width, height): surface = "surface" gs.run_command("r.surf.fractal", output=surface) - def run_grid_module(): - grid = GridModule( + run_in_subprocess( + functools.partial( + _run_grid_module, "r.slope.aspect", + project=project, width=width, height=height, overlap=2, @@ -89,15 +179,12 @@ def run_grid_module(): slope="slope", aspect="aspect", ) - grid.run() - - run_in_subprocess(run_grid_module) + ) info = gs.raster_info("slope") assert info["min"] > 0 -@xfail_mp_spawn @pytest.mark.parametrize("overlap", [0, 1, 2, 5]) def test_overlaps(tmp_path, overlap): """Check that overlap accepts different values""" @@ -108,9 +195,11 @@ def test_overlaps(tmp_path, overlap): surface = "surface" gs.run_command("r.surf.fractal", output=surface) - def run_grid_module(): - grid = GridModule( + run_in_subprocess( + functools.partial( + _run_grid_module, "r.slope.aspect", + project=project, width=10, height=5, overlap=overlap, @@ -119,15 +208,12 @@ def run_grid_module(): slope="slope", aspect="aspect", ) - grid.run() - - run_in_subprocess(run_grid_module) + ) info = gs.raster_info("slope") assert info["min"] > 0 -@xfail_mp_spawn @pytest.mark.parametrize("clean", [True, False]) @pytest.mark.parametrize("surface", ["surface", "non_exist_surface"]) def test_cleans(tmp_path, clean, surface): @@ -140,9 +226,12 @@ def test_cleans(tmp_path, clean, surface): if surface == "surface": gs.run_command("r.surf.fractal", output=surface) - def run_grid_module(): - grid = GridModule( + run_in_subprocess( + functools.partial( + _run_grid_module, "r.slope.aspect", + project=project, + run_kwargs={"clean": clean}, width=10, height=5, overlap=0, @@ -151,10 +240,11 @@ def run_grid_module(): slope="slope", aspect="aspect", mapset_prefix=mapset_prefix, - ) - grid.run(clean=clean) - - run_in_subprocess(run_grid_module) + ), + # Don't check exit code when surface is intentionally missing; + # the subprocess is expected to fail in that case. + check=surface != "non_exist_surface", + ) prefixed = 0 for item in project.iterdir(): @@ -169,7 +259,6 @@ def run_grid_module(): assert prefixed, "Not even one prefixed mapset" -@xfail_mp_spawn @pytest.mark.parametrize("patch_backend", [None, "r.patch", "RasterRow"]) def test_patching_backend(tmp_path, patch_backend): """Check patching backend works""" @@ -185,9 +274,11 @@ def test_patching_backend(tmp_path, patch_backend): "v.to.rast", input=points, output=reference, type="point", use="cat" ) - def run_grid_module(): - grid = GridModule( + run_in_subprocess( + functools.partial( + _run_grid_module, "v.to.rast", + project=project, width=10, height=5, overlap=0, @@ -198,16 +289,13 @@ def run_grid_module(): type="point", use="cat", ) - grid.run() - - run_in_subprocess(run_grid_module) + ) mean_ref = float(gs.parse_command("r.univar", map=reference, flags="g")["mean"]) mean = float(gs.parse_command("r.univar", map="output", flags="g")["mean"]) assert abs(mean - mean_ref) < 0.0001 -@xfail_mp_spawn @pytest.mark.parametrize( ("width", "height", "processes"), [ @@ -226,9 +314,11 @@ def test_tiling(tmp_path, width, height, processes): surface = "surface" gs.run_command("r.surf.fractal", output=surface) - def run_grid_module(): - grid = GridModule( + run_in_subprocess( + functools.partial( + _run_grid_module, "r.slope.aspect", + project=project, width=width, height=height, overlap=2, @@ -237,15 +327,12 @@ def run_grid_module(): slope="slope", aspect="aspect", ) - grid.run() - - run_in_subprocess(run_grid_module) + ) info = gs.raster_info("slope") assert info["min"] > 0 -@xfail_mp_spawn @pytest.mark.needs_solo_run @pytest.mark.parametrize( ("processes", "backend"), @@ -265,18 +352,18 @@ def test_patching_error(tmp_path, processes, backend): gs.run_command("g.region", s=0, n=10, w=0, e=10, res=0.1) surface = "fractal" - def run_grid_module(): - grid = GridModule( + run_in_subprocess( + functools.partial( + _run_grid_module, "r.surf.fractal", + project=project, overlap=0, processes=processes, output=surface, patch_backend=backend, debug=True, ) - grid.run() - - run_in_subprocess(run_grid_module) + ) info = gs.parse_command("r.univar", flags="g", map=surface) assert int(info["null_cells"]) == 0