8000 BUG: Handle `--f77flags` and `--f90flags` for `meson` by HaoZeke · Pull Request #26703 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

BUG: Handle --f77flags and --f90flags for meson #26703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jun 16, 2024
Merged

BUG: Handle --f77flags and --f90flags for meson #26703

merged 4 commits into from
Jun 16, 2024

Conversation

HaoZeke
Copy link
Member
@HaoZeke HaoZeke commented Jun 16, 2024 •
edited
Loading

Closes #25784. Background and notes:

  • Although the CLI was passing fc_flags to the Meson backend they weren't being used
  • The behavior currently is to (redundantly) collect the unique set of flags passed via either --f77flags or --f90flags
    • They all map to fortran_args for the meson.build (distutils used to have separate handling for the two cases)
    • Eventually these should be merged into a single --fflags argument (basically since they're now redundant post-distutils)
      • This is likely only when we have 3.12 as the lowest supported version, since distutils will need to have both CLI options
  • The test is specially crafted to get an error without -ffixed-form but is otherwise junk
    • Adding a continuation character at the end of the line as well will both get f2py and gfortran to compile without -ffixed-form
    • Or putting the same code in a .f file, thereby signalling to F2PY that this is a Fortran 77 file

It does make sense to handle fortran_args this way though, since it is more robust than setting FFLAGS (which anyway won't work due to a meson bug, mesonbuild/meson#13327).

@HaoZeke HaoZeke marked this pull request as draft June 16, 2024 04:30
@HaoZeke
Copy link
Member Author
HaoZeke commented Jun 16, 2024 •
edited
Loading

EDIT: See #26704 for a related followup issue.

@HaoZeke HaoZeke marked this pull request as ready for review June 16, 2024 04:39
@HaoZeke HaoZeke mentioned this pull request Jun 16, 2024
Open
@HaoZeke
Copy link
Member Author
HaoZeke commented Jun 16, 2024

Actually to prevent scope creep this should go in as is, it fixes the flag passing mechanism, and some more thought needs to go into the other issue, now tracked in #26704

@mattip mattip merged commit fae3738 into numpy:main Jun 16, 2024
66 of 68 checks passed
@mattip
Copy link
Member
mattip commented Jun 16, 2024

Thanks @HaoZeke

@mattip
Copy link
Member
mattip commented Jun 16, 2024

Should this be backported to 2.0?

@mattip
Copy link
Member
mattip commented Jun 16, 2024

Merge-to-main failed azure pipeline tests. @HaoZeke could you take a look?

@HaoZeke
Copy link
Member Author
HaoZeke commented Jun 16, 2024 •
edited
Loading

Merge-to-main failed azure pipeline tests. @HaoZeke could you take a look?

Ah sorry, the util.F2PyTest helper skips Windows tests, so the test added should also skip on Windows (#25134 again).

Looking at the logs it seems like the Windows CI is using the wrong stack though #25000 was why quadmath was added for Windows testers (like @jmrohwer ), and also meson detects it fine, though it fails to find it while linking. Leaving an extract from the logs here for posterity:

RuntimeError: Running f2py failed: ['-m', 'Blah', "--f77flags='-ffixed-form -O2'", '--f90flags="-ffixed-form -Og"', 'C:\\Users\\VSSADM~1\\AppData\\Local\\Temp\\tmpk5lopyhl\\f77fixedform.f95', '--backend', 'meson']
The Meson build system
Version: 1.4.1
Source dir: C:\Users\VssAdministrator\AppData\Local\Temp\tmp26x34xwn
Build dir: C:\Users\VssAdministrator\AppData\Local\Temp\tmp26x34xwn\bbdir
Build type: native build
Project name: Blah
Project version: 0.1
Fortran compiler for the host machine: gfortran (gcc 8.1.0 "GNU Fortran (x86_64-posix-seh-rev0, Built by MinGW-W64 project) 8.1.0")
Fortran linker for the host machine: gfortran ld.bfd 2.30
C compiler for the host machine: cl (msvc 19.00.24247.2 "Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24247.2 for x64")
C linker for the host machine: link link 14.00.24247.2
Host machine cpu family: x86_64
Host machine cpu: x86_64
Program C:\hostedtoolcache\windows\Python\3.11.9\x64\python.exe found: YES (C:\hostedtoolcache\windows\Python\3.11.9\x64\python.exe)
Run-time dependency python found: YES 3.11
Library quadmath found: YES
Build targets in project: 1

Found ninja-1.11.1.git.kitware.jobserver-1 at C:\hostedtoolcache\windows\Python\3.11.9\x64\Scripts\ninja.EXE
ninja: Entering directory `C:/Users/VssAdministrator/AppData/Local/Temp/tmp26x34xwn/bbdir'
[1/6] Compiling C object Blah.cp311-win_amd64.pyd.p/Blahmodule.c.obj
[2/6] Module scanner.
[3/6] Compiling C object Blah.cp311-win_amd64.pyd.p/439c03e4f3b998dc3aa6363e6ec5d442570006cf_.._.._f2py_src_fortranobject.c.obj
[4/6] Compiling Fortran object Blah.cp311-win_amd64.pyd.p/f77fixedform.f95.obj
../f77fixedform.f95:3:8:

      & x)
        1
Warning: Unused dummy argument 'x' at (1) [-Wunused-dummy-argument]
[5/6] Compiling Fortran object Blah.cp311-win_amd64.pyd.p/Blah-f2pywrappers.f.obj
[6/6] Linking target Blah.cp311-win_amd64.pyd
FAILED: Blah.cp311-win_amd64.pyd 
"link"  /MACHINE:x64 /OUT:Blah.cp311-win_amd64.pyd Blah.cp311-win_amd64.pyd.p/f77fixedform.f95.obj Blah.cp311-win_amd64.pyd.p/Blahmodule.c.obj Blah.cp311-win_amd64.pyd.p/Blah-f2pywrappers.f.obj Blah.cp311-win_amd64.pyd.p/439c03e4f3b998dc3aa6363e6ec5d442570006cf_.._.._f2py_src_fortranobject.c.obj "/LIBPATH:C:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.1.0" "/LIBPATH:C:/mingw64/lib/gcc/x86_64-w64-mingw32/8.1.0" "/LIBPATH:C:/mingw64/bin/../lib/gcc" "/LIBPATH:C:/mingw64/lib/gcc" "/LIBPATH:C:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.1.0/../../../../x86_64-w64-mingw32/lib/../lib" "/LIBPATH:C:/mingw64/x86_64-w64-mingw32/lib" "/LIBPATH:C:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.1.0/../../../../lib" "/LIBPATH:C:/mingw64/lib" "/LIBPATH:C:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.1.0/../../../../x86_64-w64-mingw32/lib" "/LIBPATH:C:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.1.0/../../.." "/release" "/nologo" "/OPT:REF" "/DLL" "/IMPLIB:Blah.cp311-win_amd64.lib" "C:\hostedtoolcache\windows\Python\3.11.9\x64\libs\python311.lib" "quadmath.lib" "kernel32.lib" "user32.lib" "gdi32.lib" "winspool.lib" "shell32.lib" "ole32.lib" "oleaut32.lib" "uuid.lib" "comdlg32.lib" "advapi32.lib" "gfortran.lib"
LINK : fatal error LNK1181: cannot open input file 'quadmath.lib'

ninja: build stopped: subcommand failed.
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: C:\hostedtoolcache\windows\Python\3.11.9\x64\Scripts\ninja.EXE -C C:/Users/VssAdministrator/AppData/Local/Temp/tmp26x34xwn/bbdir
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "C:\hostedtoolcache\windows\Python\3.11.9\x64\Lib\site-packages\numpy\f2py\f2py2e.py", line 769, in main
    run_compile()
  File "C:\hostedtoolcache\windows\Python\3.11.9\x64\Lib\site-packages\numpy\f2py\f2py2e.py", line 741, in run_compile
    builder.compile()
  File "C:\hostedtoolcache\windows\Python\3.11.9\x64\Lib\site-packages\numpy\f2py\_backends\_meson.py", line 193, in compile
    self.run_meson(self.build_dir)
  File "C:\hostedtoolcache

The reason why it didn't show up in #26634 which uses a similar test pattern is because that raised an error in meson, so we handle a RuntimeError before trying to link the final module, while here we're trying to catch an ImportError but the Windows CI doesn't import any F2PY modules correctly (since it can't build them).

@HaoZeke HaoZeke mentioned this pull request Jun 16, 2024
Merged
@HaoZeke
Copy link
Member Author
HaoZeke commented Jun 16, 2024

Should this be backported to 2.0?

Yup, that would be great, it's likely to break a lot of -c usage otherwise

@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Jun 16, 2024
@HaoZeke HaoZeke mentioned this pull request Jun 16, 2024
Open
@HaoZeke HaoZeke deleted the fixF2PYflags branch June 16, 2024 23:58
@rgommers
Copy link
Member

The added test here is failing on macOS, see gh-26810. Could you please have a look @HaoZeke? If you have no bandwidth or it's not an easy fix, let's skip the test for now.

@charris
Copy link
Member
charris commented Jun 29, 2024 •
edited
Loading

Note to self, if this is backported, it needs fixes. #26812.

charris added a commit to charris/numpy that referenced this pull request Jul 5, 2024
@charris
BUG: Handle --f77flags and --f90flags for meson [wheel build]
526a2f4 
Backport of numpy#26703, numpy#26706, and numpy#26812.

Closes numpy#25784. Background and notes:
- Although the CLI was passing `fc_flags` to the Meson backend they
  weren't being used
- The behavior currently is to (redundantly) collect the unique set of
  flags passed via either `--f77flags` or `--f90flags`
  - They all map to `fortran_args` for the `meson.build` (`distutils`
    used to have separate handling for the two cases)
  - Eventually these should be merged into a single `--fflags` argument
    (basically since they're now redundant post-distutils)
    - This is likely only when we have 3.12 as the lowest supported
      version, since `distutils` will need to have both CLI options
- The test is specially crafted to get an error without `-ffixed-form`
  but is otherwise junk
  - Adding a continuation character at the end of the line as well will
    both get `f2py` and `gfortran` to compile without `-ffixed-form`
  - Or putting the same code in a `.f` file, thereby signalling to F2PY
    that this is a Fortran 77 file

It does make sense to handle `fortran_args` this way though, since it is
more robust than setting `FFLAGS` (which anyway won't work due to a
`meson` bug, mesonbuild/meson#13327).

Squashed commit of the following:

commit bbc198b48b79417f210bd38de6c60ab98d18857d
Author: Rohit Goswami <rog32@hi.is>
Date:   Fri Jun 28 19:43:41 2024 +0200

    CI,MAINT: Bump gfortran version [wheel build]

    Co-authored-by: ngoldbaum <ngoldbaum@users.noreply.github.com>

commit fffa296b7a611de1ab25f85619e92e6d6c5185a4
Author: Rohit Goswami <rog32@hi.is>
Date:   Fri Jun 28 14:54:50 2024 +0200

    CI,TST: Fix meson tests needing gfortran [wheel build]

commit 807e898e80e544e720512f9feccc9c897cd55f08
Author: Rohit Goswami <rog32@hi.is>
Date:   Sun Jun 16 09:40:06 2024 +0000

    TST: Skip an f2py module test on Windows

commit cbcb6c2c4340f5bbca46e2c6d8474bbed7d90664
Author: Rohit Goswami <rog32@hi.is>
Date:   Sun Jun 16 04:06:16 2024 +0000

    MAINT: Be more robust wrt f2py flags

commit 09e3eb004b0e076271817384a92d489b3b0cc0eb
Author: Rohit Goswami <rog32@hi.is>
Date:   Sun Jun 16 03:44:17 2024 +0000

    TST: Add one for passing arguments to f2py

    Co-authored-by: warrickball <warrickball@users.noreply.github.com>

commit 04d53b44b75537fc5b411cf9e54c2f169198419f
Author: Rohit Goswami <rog32@hi.is>
Date:   Sun Jun 16 00:14:54 2024 +0000

    BUG: Use fortran args from f2py in meson

commit 6f736b0cbe977ba9270c45f7cc22b7ae8ce06ed2
Author: Rohit Goswami <rog32@hi.is>
Date:   Sat Jun 15 23:36:55 2024 +0000

    MAINT,BUG: Correctly skip distutils options
@charris charris mentioned this pull request Jul 5, 2024
Merged
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jul 5, 2024
@HaoZeke HaoZeke mentioned this pull request Aug 18, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
00 - Bug component: numpy.f2py
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BUG: f2py appears to ignore Fortran compiler flags
4 participants
@HaoZeke @mattip @rgommers @charris
0