Page MenuHomePhabricator

[utils] Convert update_{llc_,}test_checks.py to Python 3
AbandonedPublic

Authored by MaskRay on Jan 23 2018, 3:21 PM.

Details

Reviewers
spatel
Summary

Python 2.7 will retire in 2 years. We may extract common parts of the two files to deduplicate. By converting to Python 3, we'll have less trouble dealing with different import mechanism.

Diff Detail

Event Timeline

MaskRay created this revision.Jan 23 2018, 3:21 PM
MatzeB added a subscriber: MatzeB.Jan 23 2018, 3:27 PM

LLVM's GettingStarted.html says "python >= 2.7" is fine for LLVM development.

The old recurring theme here is that:

  • macOS only ships with python 2.7 to this day. So people need to install extra packages there for python 3.
  • Modern linux distributions only install python 3 by default o people need to install extra packages there for python 2.7.

Admittedly macOS doesn't even ship with a python2.7 symlink so this script may have been broken for an out-of-the box macOS configuration anyway...

MaskRay added a comment.EditedJan 23 2018, 4:16 PM

LLVM's GettingStarted.html says "python >= 2.7" is fine for LLVM development.

The old recurring theme here is that:

  • macOS only ships with python 2.7 to this day. So people need to install extra packages there for python 3.
  • Modern linux distributions only install python 3 by default o people need to install extra packages there for python 2.7.

Admittedly macOS doesn't even ship with a python2.7 symlink so this script may have been broken for an out-of-the box macOS configuration anyway...

Thanks for your attention... I'd like to see llvm moving away from legacy python2.7

I intent to add something called update_clang_test_checks.py (.cc -> .ll or .cc -> .s) and find these scripts really should be combined..

I intent to add something called update_clang_test_checks.py (.cc -> .ll or .cc -> .s) and find these scripts really should be combined..

I fully support automating clang test checks - see D17999 for some existing work in this direction.

I also support combining the opt and llc scripts, but I don't have the python skills to do it - rL305208 . My only experience with python is a couple of days on these scripts, so you can assume anything I've done is amateur at best.

For this reason, I'm also not qualified to review major improvements to the scripts, but I appreciate being cc'd here to know about the direction (and of course I use the scripts all the time).

re: macOS - I develop on macOS almost exclusively (I may even be the last one using Xcode to hack on LLVM!)...so I must've done something to be able to run these scripts on my machine. :)

bogner added a subscriber: bogner.Jan 24 2018, 1:08 PM

Thanks for your attention... I'd like to see llvm moving away from legacy python2.7

Changes to make these scripts work with both 2.7 and 3 are probably a more practical approach in the short term. I suspect there will still be a fair amount of resistance to dropping python2.7 support in LLVM for a bit yet.

I intent to add something called update_clang_test_checks.py (.cc -> .ll or .cc -> .s) and find these scripts really should be combined..

As Sanjay mentioned, this has been attempted in the past. It may make more sense to break out some of the common parts into a library and use it in both, as the scripts do have slightly different needs. I've been thinking about how to approach this, and suspect some of the ideas implemented in update_mir_test_checks will help.

I'd like to see llvm moving away from legacy python2.7

That needs discussion on llvm-dev to make sure everyone is on the same page, GettingStarted.rst is updated etc. Forcing just a single script to python3 is not a good idea.

FWIW I don't understand why we waste time making our script compatible to python2 and python3 at the same time, it just doubles testing efforts. I'd rather see us go all-in on python3 (or stay with python2 and don't bother). And I personally think we should go all-in for python3...

so I must've done something to be able to run these scripts on my machine. :)

Well installing any homebrew or macports python (which is often a dependency for other packages) will give you a python2 with the proper symlinks in place. It's usually people maintaining CI infrastructured that are bothered by macOS (or linux) default installations.

so I must've done something to be able to run these scripts on my machine. :)

Well installing any homebrew or macports python (which is often a dependency for other packages) will give you a python2 with the proper symlinks in place. It's usually people maintaining CI infrastructured that are bothered by macOS (or linux) default installations.

Mac's system python comes with a python2.7 symlink. Today you don't need to do anything for this script to work.

so I must've done something to be able to run these scripts on my machine. :)

Well installing any homebrew or macports python (which is often a dependency for other packages) will give you a python2 with the proper symlinks in place. It's usually people maintaining CI infrastructured that are bothered by macOS (or linux) default installations.

Mac's system python comes with a python2.7 symlink. Today you don't need to do anything for this script to work.

I remembered that some scripts didn't work on macOS out of the box, but indeed python2.7 exists. It looks like just a python2 symlink is missing.

Thanks for your preference of python3 over python2.7. Is the only objection

  1. not out-of-box on Mac OS X
  2. inconsistency with other scripts with the shebang (#!/usr/bin/python or python2)

Is the decision "we should not make it Python 3" ?

Is the decision "we should not make it Python 3" ?

I think it is: "We should not make it python 3 without discussing this on the llvm-dev mailing list".