search

blitzpp / blitz

Blitz++ Multi-Dimensional Array Library for C++
https://github.com/blitzpp/blitz/wiki
Other
406 stars 84 forks source link

Use python2 command because we are NOT Python3 compatible. #31

Closed citibeth closed 6 years ago

citibeth commented 6 years ago

By default, python is supposed to mean only Python2. Some distros link python -> python3, causing problems for older scripts that assume Python2. If you need Python2, you should use the command python2.

See https://github.com/spack/spack/pull/7960

slayoo commented 6 years ago

Confirming it worked on Travis: https://travis-ci.org/blitzpp/blitz/jobs/379357392#L450

Merging and closing, yet adding a reminder that this is not actually checked on Windows Appveyor build: https://github.com/blitzpp/blitz/issues/32

citibeth commented 6 years ago

Blitz needs a cmake build to work on windows.

On Thu, May 17, 2018 at 4:08 AM Sylwester Arabas notifications@github.com wrote:

@slayoo commented on this pull request.

OK

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/pull/31#pullrequestreview-120933690, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1cd4sLZwa_lcPE8y-trrmhmoOzQZRTks5tzTASgaJpZM4UAEO_ .

slayoo commented 6 years ago

I fully agree, it would be great to move to CMake and have Windows builds supported this way, but still as of now the shipped VS files do work! see e.g.: https://ci.appveyor.com/project/slayoo/blitz-nh158

slayoo commented 6 years ago

Apparently, this change caused "brew install --HEAD blitz" to start failing: https://travis-ci.org/igfuw/libmpdataxx/jobs/388702067

citibeth commented 6 years ago

Apparently, this change caused "brew install --HEAD blitz" to start failing:

HomeBrew needs to fix this. The purpose of standards is that if everyone conforms to them, then things work. I think that PEP-0394 is pretty clear that the python2 command needs to be provided for any Python2 distribution. If we leave it as python, then we will break on other (non-standard) Python distros that have decided to make python mean Python3. If the choice is to break non-standard Python distro A or non-standard Python distro B, then we should go with whatever choice keeps us most standards-compliant.

BTW... if you build and install the "plain vanilla" python-2.7.8 distribution, you will end up with a python2 command and a python command, both linked to Python2. At some later date, python will start to mean Python3. You should use python only if your code is known to work with both versions of Python.

From my experience with Spack (another software installer like HomeBrew)... we have do deal with upstream packages (eg: Blitz++) that do all sorts of crazy, non-standard things. And we only have limited ability to fix or change upstream packages, even when they are in the wrong. But in this case, I think Blitz++ is following the standards. HomeBrew can make it work without changing Blitz++.

slayoo commented 6 years ago

@redgene (CC-ing you as the most recent commiter to Homebrew's blitz formula)

There are two things to be updated in the brew's blitz.rb:

I don't have any mac at hand nowadays, hence asking for help. Thanks!

citibeth commented 6 years ago

On Thu, Jun 7, 2018 at 5:44 AM, Sylwester Arabas notifications@github.com wrote:

@redgene https://github.com/redgene (CC-ing you as the most recent commiter to Homebrew's blitz formula)

There are two things to be updated in the brew's blitz.rb:

The core problem seems to be that Brew's Python recipe, when installing Python2, installs a python command, but not a python2 command. This is not in conformance with PEP 394, which specifies that Python2 installations must install a command called python2. (Another possible reason the python2 command doesn't exist is that Brew is installing Python3; in which case it won't work with Blitz++ because Python2 is required there)

https://www.python.org/dev/peps/pep-0394/

  • using the new github release by default (and not the 0.10 from SF)

Based on my work with Spack http://spack.io I would recommend against this. In the case of Spack, we want to (a) be able to checksum downloads to detect and avoid security tampering, and (b) ensure repeatability of installations, making it possible to debug large software stacks. If a recipe simply installs the latest develop branch of a package, then neither of theses properties hold true.

If we at Blitz++ want others to use a newer version of our software, I recommend that we create a release.

-- Elizabeth

slayoo commented 6 years ago

fully agree re releases, and I meant the latest release (we have already two on GH) not the snapshot (which is already obtainable with brew install --HEAD)

slayoo commented 6 years ago

just created a PR for homebrew suggesting addition of python2 as a build dependency: https://github.com/Homebrew/homebrew-core/pull/28985

slayoo commented 6 years ago

merged! https://github.com/Homebrew/homebrew-core/commit/9154f2c29637f69ed92505aaf2aa1dfcd57f1373