[Suggestion] Refactor mx.py, source file too large

[neomatrix369]

mx.py is growing rapidly and with this can be unreadable over time.

IDEs like IntelliJ/PyCharm are slowing down parsing the contents. Besides that its best to separate the different aspects of mx.py into separate source files/modules of their own, just like the other mx_[xxx].py files out there.

I glanced through the code and here are some of the high-level parts we could split mx.py into:

• high-level tasks i.e. build, test, benchmark, etc...
• git related functionality
• hg related functionality
• code blocks that split tasks into processes for multi-processing
• suite related functionality (Creation and other management tasks)
• dependency resolution related functionality
• maven & dependency resolution related functionality
• gradle & dependency resolution related functionality
• distribution related functionality
• IDE related functionality
• others...

Please make your own suggestions on how you would best like it to be split, but from the current layout of the classes and methods in it, the above was apparent but maybe a better design is inherent and I'm not seeing it immediately.

A good rule to keep such important files lean: only have high-level implementations that outline what the program does, and the how it does it (i.e. implementation parts) would be best to encapsulate elsewhere via Packages, Modules, Classes, etc... This would make it easier to read and change, especially to debug when things break.

I'm happy to slowly refactor it and create PRs for individual concerns - let me know if this is okay and I will create a simple PR to start with?

[dougxc]

Thanks for the offer @neomatrix369 Refactoring mx.py into smaller modules has been a long time, low priority task and it would great to get some help with that.

[neomatrix369]

Thanks for the offer @neomatrix369 Refactoring mx.py into smaller modules has been a long time, low priority task and it would great to get some help with that.

Cool I'm onto it, just raised my first PR #195, although will raise the refactoring ones asap

[neomatrix369]

I have gone through the mx.py file - first pass, a quick glance and so far it appears that most of the classes and methods/functions in it are nearly state-machines (independent), although I'm likely to be wrong. Although there are groups of them that call or dependent on the others, it's more or less clear but when I start working on them we can confirm.

Do you want to caution me about any areas where there might be tangles that I should avoid or keep for the last?

I'll try to group them together as much as possible and then split them. Some of them cannot just be grouped - might fall under misc category (let's see).

[dougxc]

One problem you will run into is that there's no clear indication of what parts of mx.py are used downstream and will break if those parts are moved elsewhere. In the Graal repo, this search provides some clues: https://github.com/oracle/graal/search?l=Python&q=mx.
I wonder if there's some tool to extract the API of mx.py by analyzing a bunch of repos that use it.

Disclaimer: This poor "labelling" of public API reflects the organic growth of mx from a small helper script into a full blown build system (and more) by a group whose expertise in Python best practices was/is somewhat rudimentary ;-) We should aim to rectify this with the refactoring you are doing (e.g. use _ and __ prefixes to denote "protected" and "private" members).

[neomatrix369]

One problem you will run into is that there's no clear indication of what parts of mx.py are used downstream and will break if those parts are moved elsewhere. In the Graal repo, this search provides some clues: https://github.com/oracle/graal/search?l=Python&q=mx.
I wonder if there's some tool to extract the API of mx.py by analyzing a bunch of repos that use it.

Disclaimer: This poor "labelling" of public API reflects the organic growth of mx from a small helper script into a full blown build system (and more) by a group whose expertise in Python best practices was/is somewhat rudimentary ;-) We should aim to rectify this with the refactoring you are doing (e.g. use _ and __ prefixes to denote "protected" and "private" members).

Thanks for all that info, I'll look into these things in the meanwhile before I raise a PR for refactoring.

I was curious to know about the answers to the below :

• is the travis build against each PR good enough to catch any issue (if not, most of the issues - so we know if a change during refactoring has broken mx?)
• are there plans to add more tests to the mx test suite and then run these during the travis / PR builds? We can have important tests that always tell us if some change has broken the original working
• I'm intending to run a full graal build each time I raise a PR, via the circleci job I have running via awesome-graal - this for me a litmus test for the validity and viability of the changes made to mx each time.

Do you have similar views on the above?

About the dependency graphs and call graphs I'm thinking of some solution, will try share via a separate PR.

[neomatrix369]

See mx.py (internal) dependency graph from pycharm:

(orthogonal)
(std hierarchical)

Apologies, you will have to zoom several levels deep to be able to see the graphs clearly.

Otherwise please try to load the UML diagrams (remove .txt extension from filename):
mx.py.uml.txt

[dougxc]

Unfortunately the Travis gate for mx is very incomplete and so we mainly rely on testing mx on a number of downstream repos such as Graal internally. Testing your changes against Graal is therefore a good idea and will very roughly approximate the internal testing we do.

[neomatrix369]

Okay while we have this in place, I will try to add to the test suites, and see how I can best bring the flow from these other efforts into tests - so we have one central place to rely on.

[neomatrix369]

I'm trying to run the tests in the tests folder and so far not able to do it, I get this error:

platform darwin -- Python 2.7.15, pytest-2.9.1, py-1.6.0, pluggy-0.3.1
rootdir: /Users/swami/git-repos/OpenJDK/graal-stuff/mx/tests/mx.mxtests, inifile:
plugins: cov-2.1.0
collecting 0 itemssuite named mxtests not found
collected 0 items / 1 errors

===================================================== ERRORS ======================================================
_________________________________________ ERROR collecting mx_mxtests.py __________________________________________
tests/mx.mxtests/mx_mxtests.py:11: in <module>
    _suite = mx.suite('mxtests')
mx.py:10085: in suite
    abort('suite named ' + name + ' not found', context=context)
mx.py:12536: in abort
    raise SystemExit(error_code)
E   SystemExit: 1

Here's my scripts: https://github.com/neomatrix369/mx/tree/add-code-coverage-deps (see https://github.com/neomatrix369/mx/blob/add-code-coverage-deps/run-mx-tests.sh)

[dougxc]

Sorry, the person who wrote tests/mx.mxtests is no longer with the team and I'm not sure if anyone else remaining understands it (maybe @gilles-duboscq ?).

[neomatrix369]

What is the target python version expected in order to run mx.py?
Python 2.7?

A lot of the code in there represents Python 3.7, for eg. all the print() calls use () like in Python 3?

How as this been running in python 2.7? I think the downstream repos might have a mixed language approach as well - is that the case?

[dougxc]

The target version is currently both Python 2.7 and 3.6 and we gate on both. This was done to enable the transition to Python3 with the pending EOL of Python 2. While currently mx itself works on both python version, Graal itself and all other GraalVM related repos are still in the process of becoming compatible with Python3.

[neomatrix369]

The target version is currently both Python 2.7 and 3.6 and we gate on both. This was done to enable the transition to Python3 with the pending EOL of Python 2. While currently mx itself works on both python version, Graal itself and all other GraalVM related repos are still in the process of becoming compatible with Python3.

Thanks, that explains why I ran into version issues when I extracted a block of code from mx.py into its own source file. I also ran into version issue in one of the downstream source files when I switched to Python 3.7.

[neomatrix369]

Sorry, the person who wrote tests/mx.mxtests is no longer with the team and I'm not sure if anyone else remaining understands it (maybe @gilles-duboscq ?).

@gilles-duboscq Any suggestions on what to do with the tests?

[dougxc]

I spoke with @gilles-duboscq offline and we both agree you should just ignore tests/mx.mxtests. It doesn't even appear to contain any tests, but just infrastructure to write python based tests. We will remove this code to avoid further confusion.
If you want to add some testing, then the tests should be probably be written using the standard Python unittest module.

[neomatrix369]

I spoke with @gilles-duboscq offline and we both agree you should just ignore tests/mx.mxtests. It doesn't even appear to contain any tests, but just infrastructure to write python based tests. We will remove this code to avoid further confusion.
If you want to add some testing, then the tests should be probably be written using the standard Python unittest module.

Thats a good point and I agree, I was going to do the same, remove the folder and create a new one and yes using standard Python testing libraries unittest, coverage, etc...