build 0.10.0 - Githubissues

h-vetinari commented 3 years ago

Try building https://github.com/serge-sans-paille/pythran/pull/1894

conda-forge-linter commented 3 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

h-vetinari commented 3 years ago

Seems that the xsimd-unvendoring for windows hasn't worked for a while... (not least because the line - python -c "import os, shutil; shutil.rmtree(os.path.join('pythran', 'xsimd'))" erroring did not cause the build to stop).

The xsimd package is in the env, the headers are there, the -I%PREFIX%\include is there, so I really don't understand why #include <xsimd/xsimd.hpp> fails.

serge-sans-paille commented 3 years ago

@h-vetinari can you apply the following patch so that we get log info:

diff --git a/omp/__init__.py b/omp/__init__.py
index 3496891..6561f6a 100644
--- a/omp/__init__.py
+++ b/omp/__init__.py
@@ -66,27 +66,36 @@ class OpenMP(object):
             if env_paths:
                 paths.extend(env_paths.split(os.pathsep))

+        print("paths:", paths)
+
         libomp_names = self.get_libomp_names()
+
+        print("libomp_names:", libomp_names)
         for libomp_name in libomp_names:

             cmd = [cxx, '-print-file-name=' + libomp_name]
             # The subprocess can fail in various ways, including because it
             # doesn't support '-print-file-name'. In that case just give up.
             try:
+                print("running:", cmd)
                 path = os.path.dirname(check_output(cmd).decode().strip())
                 if path:
                     paths.append(path)
             except (OSError, CalledProcessError):
                 pass

+        print("paths:", paths)
+
         for libomp_name in libomp_names:
             # Try to load find libomp shared library using loader search dirs
             libomp_path = find_library(libomp_name)
+            print(libomp_name, "->", libomp_path)

             # Try to use custom paths if lookup failed
             if not libomp_path:
                 for path in paths:
                     candidate_path = os.path.join(path, libomp_name)
+                    print("trying:", candidate_path)
                     if os.path.isfile(candidate_path):
                         libomp_path = candidate_path
                         break

serge-sans-paille commented 3 years ago

mmh x86_64-apple-darwin13.4.0-clang++ -print-file-name=libiomp5.dylib seems to fail. Do you have a way to check that?

h-vetinari commented 3 years ago

Shouldn't we be expecting to find libomp.dylib (since llvm-openmp is in the environment)?

isuruf commented 3 years ago

That's expected as the clang doesn't really have a standard place for libomp.dylib.

What can be fixed in pythran is to fix the call to find_library. find_library is called with libomp.dylib, but from docs,

ctypes.util.find_library(name)

Try to find a library and return a pathname. name is the library name without any prefix like lib, suffix like .so, .dylib or version number (this is the form used for the posix linker option -l). If no library can be found, returns None.

h-vetinari commented 3 years ago

Thanks a lot @isuruf! 😊

Any idea why the xsimd-unvendoring on windows is not working despite the having the lib, the headers, the -I include...?

Summary of windows build status changes...	commit	status
https://github.com/conda-forge/pythran-feedstock/pull/56/commits/fa920dae340c2003867ce3636d2801ac5a2b6237 (and before)	✔️
https://github.com/conda-forge/pythran-feedstock/pull/56/commits/ed65adcd57f2b0b1ebf0562ee1d9fc9d29e7f397 (and after)	❌
https://github.com/conda-forge/pythran-feedstock/pull/56/commits/90703abe7f332241ef3f470f53412b7010cafce1 (and after)	✔️
https://github.com/conda-forge/pythran-feedstock/pull/56/commits/99a4b80281d592af5654e2eac0d7daf3b75d8866 (and after)	❌

isuruf commented 3 years ago

You probably need to set compiler.include_dirs = %LIBRARY_PREFIX%\include in setup.cfg

isuruf commented 3 years ago

see https://github.com/serge-sans-paille/pythran/blob/f5832d4eb05ebcda7598b507fee6a1bf1b503716/pythran/pythran-win32.cfg#L4

h-vetinari commented 3 years ago

You probably need to set compiler.include_dirs = %LIBRARY_PREFIX%\include in setup.cfg

Ah, now I think I get it. I had seen the -I%PREFIX%\include and forgotten that this is actually the wrong path on windows, cf. https://docs.conda.io/projects/conda-build/en/latest/user-guide/environment-variables.html?highlight=cmake#environment-variables-set-during-the-build-process

serge-sans-paille commented 3 years ago

I'll apply the omp patch upstream.

serge-sans-paille commented 3 years ago

upstream branch updated!

h-vetinari commented 3 years ago

You probably need to set compiler.include_dirs = %LIBRARY_PREFIX%\include in setup.cfg

I mean, I can patch this into windows builds, but it sounds to me like the include_dirs is wrongly populated (with %PREFIX%\include instead of %PREFIX%\Library\include) somewhere on the way from conda to the pythran-setup (and I couldn't quickly unravel it on the pythran side). Wouldn't this be better to fix without a patch, if possible?

serge-sans-paille commented 3 years ago

Let's patch it at the conda level, then open a PR on pythran that will land in next release?

serge-sans-paille commented 3 years ago

sorry, you picked a trnasinet version, I updated the upstream branch once more :-/

h-vetinari commented 3 years ago

sorry, you picked a trnasinet version, I updated the upstream branch once more :-/

No, I got the most recent one, but made a mistake with the backslash-handling...

conda-forge-linter commented 3 years ago

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

Failed to even lint the recipe, probably because of a conda-smithy bug :cry:. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

conda-forge-linter commented 3 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

h-vetinari commented 3 years ago

Let's patch it at the conda level, then open a PR on pythran that will land in next release?

Well, I think I found a way to do this without patching, but whether I add

[compiler]
include_dirs=%%LIBRARY_PREFIX%%\include

to setup.cfg or to pythran\pythran.cfg, it does not get picked up in the final binary.

Need your help on that please, @serge-sans-paille.

Also, as noted in the upstream issue, the new openmp setup on windows is very noisy.

serge-sans-paille commented 3 years ago

@h-vetinari : I think you should update the source pythran/pythran-win32.cfg before install

h-vetinari commented 3 years ago

@h-vetinari : I think you should update the source pythran/pythran-win32.cfg before install

Right now I'm changing the source pythran\pythran.cfg (but only on windows) - is that functionally any different?

h-vetinari commented 3 years ago

@serge-sans-paille, I'm now showing the pythran.cfg from the site-packages directory (during the test phase), i.e. type %PREFIX%\Lib\site-packages\pythran\pythran.cfg: which now contains

[...snip...]

[compiler]
include_dirs=D:/bld/pythran_1631305376390/_test_env\Library\include

However, this does not seem to get inserted anywhere in the actual calls...?

isuruf commented 3 years ago

As we both said, you need to patch pythran-win32.cfg.

h-vetinari commented 3 years ago

Finally, this passes without vendored xsimd also on windows 🥳

From my POV, this is now ready (well, except for the upstream release of 0.10.0 and rebasing).

BTW, I'd be willing to join as maintainer @conda-forge/pythran

h-vetinari commented 3 years ago

Finally, this passes without vendored xsimd also on windows 🥳

Thanks for the help & patience @isuruf & @serge-sans-paille!

h-vetinari commented 3 years ago

Interestingly, there's one error I hadn't seen yet (due to the quicker iteration cycle) - pypy on aarch complains:

Successfully built pythran
Installing collected packages: pythran
ERROR: Exception:
Traceback (most recent call last):
  File "/drone/src/build_artifacts/[...]/site-packages/pip/_internal/cli/base_command.py", line 173, in _main
    status = self.run(options, args)
  File "/drone/src/build_artifacts/[...]/site-packages/pip/_internal/cli/req_command.py", line 203, in wrapper
    return func(self, options, args)
  File "/drone/src/build_artifacts/[...]/site-packages/pip/_internal/commands/install.py", line 399, in run
    pycompile=options.compile,
  File "/drone/src/build_artifacts/[...]/site-packages/pip/_internal/req/__init__.py", line 81, in install_given_reqs
    pycompile=pycompile,
  File "/drone/src/build_artifacts/[...]/site-packages/pip/_internal/req/req_install.py", line 766, in install
    requested=self.user_supplied,
  File "/drone/src/build_artifacts/[...]/site-packages/pip/_internal/operations/install/wheel.py", line 802, in install_wheel
    requested=requested,
  File "/drone/src/build_artifacts/[...]/site-packages/pip/_internal/operations/install/wheel.py", line 637, in _install_wheel
    file.save()
  File "/drone/src/build_artifacts/[...]/site-packages/pip/_internal/operations/install/wheel.py", line 405, in save
    with open(self.dest_path, "wb") as dest:
ValueError: embedded null byte

serge-sans-paille commented 3 years ago

It's difficult for me to make a connection with pythran there :-)

h-vetinari commented 3 years ago

@mattip Have you encountered ValueError: embedded null byte with pypy on aarch somewhere recently...?

mattip commented 3 years ago

I have not seen that. It seems something is off with the file name:

...
site-packages/pip/_internal/operations/install/wheel.py", line 405, in save
    with open(self.dest_path, "wb") as dest:
ValueError: embedded null byte

h-vetinari commented 3 years ago

I have not seen that. It seems something is off with the file name:

Yeah, that's my suspicion as well, but it only happens for pypy on aarch, which is weird.

h-vetinari commented 3 years ago

Looking at the previous runs, it could also just be a flake... Perhaps not worth to invest much time here for the moment.

h-vetinari commented 3 years ago

BTW, pypy failure on arch didn't show up again. :)

h-vetinari commented 3 years ago

Rebased.

BTW, I'd be willing to join as maintainer @conda-forge/pythran

As suggested further up, I've now added myself as a maintainer. Let me know if that's not desirable and I'll remove the last commit.

h-vetinari commented 3 years ago

I have to say I don't understand why you merged #57 instead of this PR - all the work on the xsimd unvendoring etc. and fixes on windows were missed now. Do you want me to rebase this or abandon it?

serge-sans-paille commented 3 years ago

@h-vetinari Indeed. I merged #57 because it was passing validation, thinking it doesn't prevent us from rebasing this PR on it. But it does involve extra work, so I'm pretty sorry for that :-/

h-vetinari commented 3 years ago

It's less about a bit of rebasing and more about avoiding to publish subpar builds (because, despite the green CI in #57, the problems found & fixed in this PR obviously remained) pointlessly.

h-vetinari commented 3 years ago

Ping 🙃

h-vetinari commented 2 years ago

Ping @serge-sans-paille

serge-sans-paille commented 2 years ago

Thanks for the hard work, and... sorry for the delay. wow, 9 days since last ping :-/

conda-forge / pythran-feedstock

build 0.10.0 #56