From 0cc05f65946d5782e593e069c968e92c4e40a581 Mon Sep 17 00:00:00 2001 From: Rohit Goswami Date: Sat, 18 Nov 2023 21:12:28 +0000 Subject: [PATCH 1/8] TST: Add one for gh-22819 --- numpy/f2py/tests/src/cli/gh_22819.pyf | 6 ++++++ numpy/f2py/tests/test_f2py2e.py | 28 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 numpy/f2py/tests/src/cli/gh_22819.pyf diff --git a/numpy/f2py/tests/src/cli/gh_22819.pyf b/numpy/f2py/tests/src/cli/gh_22819.pyf new file mode 100644 index 000000000000..8eb5bb106a36 --- /dev/null +++ b/numpy/f2py/tests/src/cli/gh_22819.pyf @@ -0,0 +1,6 @@ +python module test_22819 + interface + subroutine hello() + end subroutine hello + end interface +end python module test_22819 diff --git a/numpy/f2py/tests/test_f2py2e.py b/numpy/f2py/tests/test_f2py2e.py index 5f7b56a68a9d..49f0bef8aa96 100644 --- a/numpy/f2py/tests/test_f2py2e.py +++ b/numpy/f2py/tests/test_f2py2e.py @@ -71,6 +71,15 @@ def gh23598_warn(tmpdir_factory): return fn +@pytest.fixture(scope="session") +def gh22819_cli(tmpdir_factory): + """F90 file for testing disallowed CLI arguments in ghff819""" + fdat = util.getpath("tests", "src", "cli", "gh_22819.pyf").read_text() + fn = tmpdir_factory.getbasetemp() / "gh_22819.pyf" + fn.write_text(fdat, encoding="ascii") + return fn + + @pytest.fixture(scope="session") def hello_world_f77(tmpdir_factory): """Generates a single f77 file for testing""" @@ -100,6 +109,25 @@ def f2cmap_f90(tmpdir_factory): return fn +def test_gh22819_cli(capfd, gh22819_cli, monkeypatch): + """Check that module names are handled correctly + gh-22819 + Essentially, the -m name cannot be used to import the module, so the module + named in the .pyf needs to be used instead + + CLI :: -m and a .pyf file + """ + ipath = Path(gh22819_cli) + monkeypatch.setattr(sys, "argv", f"f2py -m blah {ipath}".split()) + with util.switchdir(ipath.parent): + f2pycli() + gen_paths = [item.name for item in ipath.parent.rglob("*") if item.is_file()] + assert "blahmodule.c" not in gen_paths # shouldn't be generated + assert "blah-f2pywrappers.f" not in gen_paths + assert "test_22819-f2pywrappers.f" in gen_paths + assert "test_22819module.c" in gen_paths + + def test_gh23598_warn(capfd, gh23598_warn, monkeypatch): foutl = get_io_paths(gh23598_warn, mname="test") ipath = foutl.f90inp From 9140edde39d133487c3be6011100b2f93e09849a Mon Sep 17 00:00:00 2001 From: Rohit Goswami Date: Sun, 19 Nov 2023 01:05:08 +0000 Subject: [PATCH 2/8] BUG: Correctly disallow shadowed modulenames --- numpy/f2py/f2py2e.py | 60 ++++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/numpy/f2py/f2py2e.py b/numpy/f2py/f2py2e.py index 0b50b98575e3..1e6a4a22f819 100755 --- a/numpy/f2py/f2py2e.py +++ b/numpy/f2py/f2py2e.py @@ -450,6 +450,16 @@ def run_main(comline_list): f2pydir = os.path.dirname(os.path.abspath(cfuncs.__file__)) fobjhsrc = os.path.join(f2pydir, 'src', 'fortranobject.h') fobjcsrc = os.path.join(f2pydir, 'src', 'fortranobject.c') + # gh-22819 -- begin + parser = make_f2py_parser() + args, comline_list = parser.parse_known_args(comline_list) + pyf_files, _ = filter_files("", "[.]pyf([.]src|)", comline_list) + # Checks that no existing modulename is defined in a pyf file + # TODO: Remove all this when scaninputline is replaced + if "-h" not in comline_list: # Can't check what doesn't exist yet, -h creates the pyf + modname = validate_modulename(pyf_files, args.module_name) + comline_list += ['-m', modname] # needed for the rest of scaninputline + # gh-22819 -- end files, options = scaninputline(comline_list) auxfuncs.options = options capi_maps.load_f2cmap_file(options['f2cmap_file']) @@ -516,24 +526,30 @@ def get_prefix(module): p = os.path.dirname(os.path.dirname(module.__file__)) return p -def preparse_sysargv(): - # To keep backwards bug compatibility, newer flags are handled by argparse, - # and `sys.argv` is passed to the rest of `f2py` as is. +def make_f2py_parser(): parser = argparse.ArgumentParser(add_help=False) parser.add_argument("--dep", action="append", dest="dependencies") parser.add_argument("--backend", choices=['meson', 'distutils'], default='distutils') + parser.add_argument("-m", dest="module_name") + return parser + +def preparse_sysargv(): + # To keep backwards bug compatibility, newer flags are handled by argparse, + # and `sys.argv` is passed to the rest of `f2py` as is. + parser = make_f2py_parser() args, remaining_argv = parser.parse_known_args() sys.argv = [sys.argv[0]] + remaining_argv backend_key = args.backend if sys.version_info >= (3, 12) and backend_key == 'distutils': - outmess('Cannot use distutils backend with Python 3.12, using meson backend instead.') + outmess("Cannot use distutils backend with Python 3.12, using meson backend instead.\n") backend_key = 'meson' return { "dependencies": args.dependencies or [], - "backend": backend_key + "backend": backend_key, + "modulename": args.module_name, } def run_compile(): @@ -544,11 +560,11 @@ def run_compile(): # Collect dependency flags, preprocess sys.argv argy = preparse_sysargv() + modulename = argy["modulename"] dependencies = argy["dependencies"] backend_key = argy["backend"] build_backend = f2py_build_generator(backend_key) - i = sys.argv.index('-c') del sys.argv[i] @@ -628,9 +644,7 @@ def run_compile(): if '--quiet' in f2py_flags: setup_flags.append('--quiet') - modulename = 'untitled' sources = sys.argv[1:] - for optname in ['--include_paths', '--include-paths', '--f2cmap']: if optname in sys.argv: i = sys.argv.index(optname) @@ -638,20 +652,9 @@ def run_compile(): del sys.argv[i + 1], sys.argv[i] sources = sys.argv[1:] - pyf_files = [] - if '-m' in sys.argv: - i = sys.argv.index('-m') - modulename = sys.argv[i + 1] - del sys.argv[i + 1], sys.argv[i] - sources = sys.argv[1:] - else: - pyf_files, _sources = filter_files('', '[.]pyf([.]src|)', sources) - sources = pyf_files + _sources - for f in pyf_files: - modulename = auxfuncs.get_f2py_modulename(f) - if modulename: - break - + pyf_files, _sources = filter_files("", "[.]pyf([.]src|)", sources) + sources = pyf_files + _sources + modulename = validate_modulename(pyf_files, modulename) extra_objects, sources = filter_files('', '[.](o|a|so|dylib)', sources) include_dirs, sources = filter_files('-I', '', sources, remove_prefix=1) library_dirs, sources = filter_files('-L', '', sources, remove_prefix=1) @@ -698,6 +701,19 @@ def run_compile(): builder.compile() + +def validate_modulename(pyf_files, modulename='untitled'): + for f in pyf_files: + pyf_modname = auxfuncs.get_f2py_modulename(f) + if modulename != pyf_modname: + outmess( + f"Ignoring -m {modulename}.\n" + f"{f} defines {pyf_modname} to be the modulename.\n" + ) + modulename = pyf_modname + break + return modulename + def main(): if '--help-link' in sys.argv[1:]: sys.argv.remove('--help-link') From 3961474e4053ff2215dd6ee4502165190609cdde Mon Sep 17 00:00:00 2001 From: Rohit Goswami Date: Sun, 19 Nov 2023 01:14:12 +0000 Subject: [PATCH 3/8] MAINT: Strengthen the validation of pyf files --- numpy/f2py/f2py2e.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/numpy/f2py/f2py2e.py b/numpy/f2py/f2py2e.py index 1e6a4a22f819..63105578f70d 100755 --- a/numpy/f2py/f2py2e.py +++ b/numpy/f2py/f2py2e.py @@ -703,15 +703,16 @@ def run_compile(): def validate_modulename(pyf_files, modulename='untitled'): - for f in pyf_files: - pyf_modname = auxfuncs.get_f2py_modulename(f) - if modulename != pyf_modname: - outmess( - f"Ignoring -m {modulename}.\n" - f"{f} defines {pyf_modname} to be the modulename.\n" - ) - modulename = pyf_modname - break + if len(pyf_files) > 1: + raise ValueError("Only one .pyf file per call") + pyff = pyf_files[0] + pyf_modname = auxfuncs.get_f2py_modulename(pyff) + if modulename != pyf_modname: + outmess( + f"Ignoring -m {modulename}.\n" + f"{pyff} defines {pyf_modname} to be the modulename.\n" + ) + modulename = pyf_modname return modulename def main(): From 0f6e3574c00d9e3c05391bb6748ad92cab62376d Mon Sep 17 00:00:00 2001 From: Rohit Goswami Date: Sun, 19 Nov 2023 01:14:27 +0000 Subject: [PATCH 4/8] TST: Add tests for CLI .pyf file work --- numpy/f2py/tests/test_f2py2e.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/numpy/f2py/tests/test_f2py2e.py b/numpy/f2py/tests/test_f2py2e.py index 49f0bef8aa96..315b4fca4504 100644 --- a/numpy/f2py/tests/test_f2py2e.py +++ b/numpy/f2py/tests/test_f2py2e.py @@ -126,6 +126,19 @@ def test_gh22819_cli(capfd, gh22819_cli, monkeypatch): assert "blah-f2pywrappers.f" not in gen_paths assert "test_22819-f2pywrappers.f" in gen_paths assert "test_22819module.c" in gen_paths + assert "Ignoring blah" + + +def test_gh22819_many_pyf(capfd, gh22819_cli, monkeypatch): + """Only one .pyf file allowed + gh-22819 + CLI :: .pyf files + """ + ipath = Path(gh22819_cli) + monkeypatch.setattr(sys, "argv", f"f2py -m blah {ipath} hello.pyf".split()) + with util.switchdir(ipath.parent): + with pytest.raises(ValueError, match="Only one .pyf file per call"): + f2pycli() def test_gh23598_warn(capfd, gh23598_warn, monkeypatch): From fbf0ea0512b19f552a0b643384ae8e6076145a19 Mon Sep 17 00:00:00 2001 From: Rohit Goswami Date: Sun, 19 Nov 2023 01:40:24 +0000 Subject: [PATCH 5/8] MAINT: Fix implementation for gh22819 [f2py] --- numpy/f2py/f2py2e.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/numpy/f2py/f2py2e.py b/numpy/f2py/f2py2e.py index 63105578f70d..54ea2ce4bd2d 100755 --- a/numpy/f2py/f2py2e.py +++ b/numpy/f2py/f2py2e.py @@ -705,14 +705,15 @@ def run_compile(): def validate_modulename(pyf_files, modulename='untitled'): if len(pyf_files) > 1: raise ValueError("Only one .pyf file per call") - pyff = pyf_files[0] - pyf_modname = auxfuncs.get_f2py_modulename(pyff) - if modulename != pyf_modname: - outmess( - f"Ignoring -m {modulename}.\n" - f"{pyff} defines {pyf_modname} to be the modulename.\n" - ) - modulename = pyf_modname + if pyf_files: + pyff = pyf_files[0] + pyf_modname = auxfuncs.get_f2py_modulename(pyff) + if modulename != pyf_modname: + outmess( + f"Ignoring -m {modulename}.\n" + f"{pyff} defines {pyf_modname} to be the modulename.\n" + ) + modulename = pyf_modname return modulename def main(): From 04a280e251da9932040cac8f173a6c7f3c797ab6 Mon Sep 17 00:00:00 2001 From: Rohit Goswami Date: Sun, 19 Nov 2023 01:45:00 +0000 Subject: [PATCH 6/8] DOC: Add an entry to the changelog [f2py] --- doc/release/upcoming_changes/25181.compatibility.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 doc/release/upcoming_changes/25181.compatibility.rst diff --git a/doc/release/upcoming_changes/25181.compatibility.rst b/doc/release/upcoming_changes/25181.compatibility.rst new file mode 100644 index 000000000000..aefe7cd7a437 --- /dev/null +++ b/doc/release/upcoming_changes/25181.compatibility.rst @@ -0,0 +1,4 @@ +``f2py`` will no longer accept ambiguous ``-m`` and ``.pyf`` CLI combinations. +When more than one ``.pyf`` file is passed, an error is raised. When both ``-m`` +and a ``.pyf`` is passed, a warning is emitted and the ``-m`` provided name is +ignored. From 1c4767f2292b65cee7650d17a78b0b0e9494abdb Mon Sep 17 00:00:00 2001 From: Rohit Goswami Date: Sun, 19 Nov 2023 01:47:16 +0000 Subject: [PATCH 7/8] DOC: Rework documentation on -m [f2py] --- doc/source/f2py/usage.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/source/f2py/usage.rst b/doc/source/f2py/usage.rst index 2bcfb3070bcd..ffb4859c1df3 100644 --- a/doc/source/f2py/usage.rst +++ b/doc/source/f2py/usage.rst @@ -239,10 +239,11 @@ Other options ``-m `` Name of an extension module. Default is ``untitled``. -.. warning:: Don't use this option if a signature file (``*.pyf``) is used. +.. warning:: + Don't use this option if a signature file (``*.pyf``) is used. -.. versionchanged:: 2.0.0 - Will ignore ``-m`` if a ``pyf`` file is provided. + .. versionchanged:: 1.26.3 + Will ignore ``-m`` if a ``pyf`` file is provided. ``--[no-]lower`` Do [not] lower the cases in ````. By default, ``--lower`` is From 50b0e81cfbeb4b138aa61a20fbbd5ec30e22b858 Mon Sep 17 00:00:00 2001 From: Rohit Goswami Date: Sun, 19 Nov 2023 02:49:17 +0000 Subject: [PATCH 8/8] MAINT: Handle case where -m is None --- numpy/f2py/f2py2e.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/numpy/f2py/f2py2e.py b/numpy/f2py/f2py2e.py index 54ea2ce4bd2d..47d2fcb2fa21 100755 --- a/numpy/f2py/f2py2e.py +++ b/numpy/f2py/f2py2e.py @@ -456,7 +456,7 @@ def run_main(comline_list): pyf_files, _ = filter_files("", "[.]pyf([.]src|)", comline_list) # Checks that no existing modulename is defined in a pyf file # TODO: Remove all this when scaninputline is replaced - if "-h" not in comline_list: # Can't check what doesn't exist yet, -h creates the pyf + if "-h" not in comline_list and args.module_name: # Can't check what doesn't exist yet, -h creates the pyf modname = validate_modulename(pyf_files, args.module_name) comline_list += ['-m', modname] # needed for the rest of scaninputline # gh-22819 -- end