Numpy build broken on Cygwin since #20450

[embray]

Since the upgrade to Numpy 1.11, the patch cygwin-lapack_lite-setup.py.patch no longer applies. This is hardly the first time (#17014, etc. :)

It's not entirely clear whether this patch is even needed anymore. The first version of it was added 8 years ago: ​http://git.sagemath.org/sage.git/commit/?h=develop&id=f0ef7690e4f601e289608a09fe30df9d242eb3e8

It's not exactly clear what issue this is fixing, and I can't find any obvious reference to what it might have been--not in any of the more recent Cygwin-related documentation or anywhere. Numpy compiles fine without it.

[embray]

I think there may also be an issue with Numpy and BLAS configuration. After rebasing my Cython branch I'm running into some build issues with SciPy? that I never encountered before, which seem to do with how Numpy is reporting what libraries it was built with. I need to spend more time investigating though.

[embray]

From my testing I've concluded that the patch in question can be dropped. It seems that with some ancient version of Numpy the -shared flag was being excluded when linking this module on Cygwin for some reason or other. That is no longer the case though.

---

New commits:

​1dc8611

Remove Cygwin-only patch to numpy.linalg.setup

[gouezel]

Works for me as well.

[vbraun]

Ok this prevents all patches from being applied. Note the difference:

$ for patch in "patches/*.patch"; do echo $patch; done
patches/numpy-1.10.1-asarray_conversion.patch patches/numpy-1.10.2-no-hardcode-blas.patch
$ for patch in patches/*.patch; do echo $patch; done
patches/numpy-1.10.1-asarray_conversion.patch
patches/numpy-1.10.2-no-hardcode-blas.patch

[embray]

Yup, that's a typo.

[embray]

This raises a question: If the patchbot passed for the original version of this change, what is being patched in Numpy that there isn't a regression test for?

[embray]

I'm marking this as "needs_info" out of concern that this was previously marked "fixed" and it's not clear how the issue was caught.

---

New commits:

​0293325	Remove Cygwin-only patch to numpy.linalg.setup
​76b3369	Don't put spurious quotes around wildcard expansion.

[vbraun]

Numpy was not recompiled because you didn't change package-version.txt

[embray]

Why would I change package-version.txt?

[vbraun]

to trigger recompilation if necessary

[jdemeyer]

Replying to embray:

Why would I change package-version.txt?

To elaborate a bit on this: the build system of Sage tracks versions of installed packages. If you change the spkg-install script or change patches of a package, Sage will not automatically rebuild that package.

If you do want the package to be rebuilt, you need to increase the patchlevel in package-version.txt. This means either appending .p0 or changing .pN to .p(N+1).

You should always increase the patchlevel when changing the spkg-install script or patches of a package unless there is a good reason not to. Typical reasons "not to":

• A completely trivial change, like a typo.

• A change which affects only the build system of the package but does not change the installed package (not even in a minor way).

• The change affects only a particular platform and the package did not build on that platform before.

Regardless of the above reasons, a reviewer should always check that the changed package builds and works fine. This means doing a forced rebuild of the package using sage -f PACKAGE. Ideally, a build from scratch (make distclean; make) should also be tested.

[embray]

Okay, that sounds reasonable-ish. I'll touch the patch number then. Should I also just go ahead and squash my commits or do we not care about that?

[jdemeyer]

Replying to embray:

Should I also just go ahead and squash my commits or do we not care about that?

I would say that Sage has a policy to not squash commits, at least not while a branch is in review.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​702cb85

Bump patch level since a patch was removed.

[jdemeyer]

Builds fine on Linux too.