Use meson build backend for scipy#4276
Conversation
|
Thanks so much for your work on meson @ryanking13! |
|
Okay, I think now CI passes. |
35a7eba to
944e06a
Compare
|
cc: @lesteve I hope not, but this may break something. Please let me know if something goes wrong after merging this PR. |
|
Sounds good, currently Scipy is a bit broken on |
|
Side-comment, it would probably be worth doing a PR on conda-forge/pkg-config-feedstock to bump the build number and have a package available with the fix from conda-forge/pkg-config-feedstock#31 |
FYI I ran the scipy tests locally on this PR and they pass, i.e. this PR doesn't break anything else than what is already broken in All in all, I think this can be merged! |
|
Oh, thanks so much for the check @lesteve! |
| for lib in ["lapack", "blas"]: | ||
| if path.endswith(f"cython_{lib}.c"): | ||
| header_path = Path(path).with_name(f"_{lib}_subroutines.h") | ||
| header_name = f"_{lib}_subroutines.h" |
There was a problem hiding this comment.
Maybe put this in a separate find_header function?
There was a problem hiding this comment.
Or maybe this is fine as is...
There was a problem hiding this comment.
I moved it to a separate function. Thanks for the suggestion!
| if tmp is not None: | ||
| line = tmp |
There was a problem hiding this comment.
Can you explain this change? In particular we used to quit out here if replay_f2c returns None but now we don't? Why? When does this happen? What is the subprocess call below going to do in this case?
Also, we should switch handle_command to return an int since in compile_main it says sys.exit(handle_command(args, build_args)) we can just return 0 or return returncode in here.
There was a problem hiding this comment.
As mentioned in another comment, meson runs gfortran multiple times during the compiler check phase, so sometimes replay_f2c doesn't do anything. So I changed it to run gfortran normally in that case. That is why I removed the check for -dumpversion, as it will be handled as well.
There was a problem hiding this comment.
Also, we should switch handle_command to return an int since in compile_main it says sys.exit(handle_command(args, build_args)) we can just return 0 or return returncode in here.
Right, thanks for the pointer.
There was a problem hiding this comment.
Would the following work:
tmp = replay_f2c(line)
if tmp is None:
# No source file, it's a query for information about the compiler. Pretend we're
# gfortran by letting gfortran handle it
return subprocess.run(line).returncode
line = tmp
Or is it important that we are calling handle_command_generate_args on it? I think it would help to have some comments here since this is a really confusing execution path.
There was a problem hiding this comment.
I think your code will work. Thanks for the suggestion.
since this is a really confusing execution path.
I agree. Actually pywasmcross is becoming more complicated as we add more symlinks... I'll try do to some refactorings when I have bandwidth.
| extra_vars = set( | ||
| ["PYODIDE", "PKG_CONFIG_PATH", "CMAKE_TOOLCHAIN_FILE", "PYO3_CONFIG_FILE"] | ||
| ) | ||
| extra_vars = set(["PYODIDE", "CMAKE_TOOLCHAIN_FILE", "PYO3_CONFIG_FILE"]) |
There was a problem hiding this comment.
Is there a reason PKG_CONFIG_LIBDIR doesn't belong here?
There was a problem hiding this comment.
These variables are for checking extra variables that does not come from Makefile.envs but are added from the code . So PKG_CONFIG_PATH or PKG_CONFIG_LIBDIR does not need to belong there.
There was a problem hiding this comment.
Could you add a comment like:
# extra variables that does not come from Makefile.envs but are added by build_env.py3f99856 to
cecb420
Compare
Description
Trying to use meson build backend when building scipy.
Close #3649