Remove distutils #5232
Remove distutils #5232
Conversation
|
Code Climate has analyzed commit 9871639 and detected 7 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 88.8% (50% is the threshold). This pull request will bring the total coverage in the repository to 69.1%. View more on Code Climate. |
| @staticmethod | ||
| def clean_optional_plugins(): | ||
| walk_plugins_setup_py(["clean", "--all"]) | ||
|
|
There was a problem hiding this comment.
Maybe I'm missing some bit. But this is changing the current behavior we have: If we have a build/lib folder inside optional_plugins/html for instance, the current "main clean" is removing those folders as well. But if do the same with this PR changes, optional_plugins/*/build will be not removed.
Did I missed something? Or are you proposing a change on the current behavior?
There was a problem hiding this comment.
@beraldoleal Unless I'm mistaken, none of the installation scenarios we use in Makefile will create a build/lib under optional_plugins/*/build. If it does, this could be handled better when we switch from setup.py to setup.cfg.
There was a problem hiding this comment.
"build" it was just an example, we generate other folders/files there.
Also should we limit us only by the Makefile targets? For instance, some might need to run python3 setup.py develop|build against a specific plugin or even using our global python3 setup.py plugin -i html, so yes, we don't have Makefile targets, but we still make available some targets that might generate those folders.
I find useful to have a general "clean" that will clean everything recursively, but I'm open to change our current behavior, if it makes sense. We just need to stress the impact that this will cause to the contributors.
There was a problem hiding this comment.
I think you're mixing the clean command from setuptools with the one from distutils.
If you clone the current avocado branch master and cd to optional_plugins/robot, then run python3 setup.py develop or/and python3 setup.py build some files will be generated, indeed. If then you run python3 setup.py clean nothing will be removed. The removed calls doesn't remove anything at this moment and
we'll have to address this, specially if we split out the plugins. As I mentioned, this can be solved later.
The reason why python setup.py clean --all shows a lot of output now removing build/* directories and is probably misleading here, is because it's run from the main setup.py that have already imported distutils. However, it still does nothing because the individual targets are already emptied (eggs removed) when the local python3 setup.py clean is run.
You mentioned that an user might use directly the main python3 setup.py build to install avocado, agreed, but in this case the user won't install the plugins so the python setup.py clean --all that wouldo the local calls in every plugin would still be useless.
Hope this make things more clear.
There was a problem hiding this comment.
I think you're mixing the clean command from setuptools with the one from distutils.
I'm aware that setuptools clean doesn't have the --all option. But in terms of functionality we are changing the behavior here (see example bellow).
If you clone the current avocado branch master and cd to optional_plugins/robot, then run python3 setup.py develop or/and python3 setup.py build some files will be generated, indeed. If then you run python3 setup.py clean nothing will be removed.
Indeed, but appending --all it will remove things. And with this PR we are leaving things behind.
...
You mentioned that an user might use directly the main python3 setup.py build to install avocado, agreed, but in this case the user won't install the plugins so the python setup.py clean --all that wouldo the local calls in every plugin would still be useless.
Before this PR:
cd optional_plugins/robot/
python3 setup.py build
cd ../../
python3 setup.py clean --all
ls -lad optional_plulgins/robot/build
ls: cannot access 'optional_plugins/robot/build/': No such file or directory
After this PR:
cd optional_plugins/robot/
python3 setup.py build
cd ../../
python3 setup.py clean # since we removed --all here, I don't have another choice.
ls -lad optional_plulgins/robot/build
drwxrwxr-x. 3 local local 4096 Jan 19 13:20 optional_plugins/robot/build/
As I said, I'm just thinking in terms of functionality, this Patch is changing the current behavior. If this really makes sense, then we need at least to make it clear for contributors. By changing the tool we could avoid changing the behavior. Maybe --all it is not the proper option for that, and a new must be introduced.
Sorry if it is too much noise, but I'm still missing some gaps here.
There was a problem hiding this comment.
I went through the options (and the references) and I don't understand why we can't extend the Clean command on the primary setup.py to also cover our non-standard plugin layout. We already clean files that are non-standard such as the auto-generated .rst files.
I understand this is not perfect and the plugin's setup.py themselves should have the capabilities of cleanup, but I think letting Avocado's main setup.py handle it is a compromise solution we can live with because it doesn't regress in functionality.
There was a problem hiding this comment.
@ana can you help me to understand if this last comment of mine was addressed or not? It has to do with the before and after behavior that @beraldoleal described here.
There was a problem hiding this comment.
@clebergnu if I understood your comment properly (and remember well...), that's the current behavior before this PR and this PR doesn't change it. The main setup.py cleans the full tree.
There was a problem hiding this comment.
@clebergnu if I understood your comment properly (and remember well...), that's the current behavior before this PR and this PR doesn't change it. The main setup.py cleans the full tree.
Hi @ana,
I'm still confused, because based on my experimentation, I can see a different behavior. Before this:
Script started on 2022-03-09 08:26:04-05:00 [TERM="xterm-256color" TTY="/dev/pts/2" COLUMNS="145" LINES="39"]
�]0;cleber@fedora:~/src/avocado/avocado��[?2004h[cleber@p50 avocado]$ git status
�[?2004l
On branch master
Your branch is up to date with 'origin/master'.
nothing to commit, working tree clean
�]0;cleber@fedora:~/src/avocado/avocado��[?2004h[cleber@p50 avocado]$ cd optional_plugins/html/
�[?2004l
�]0;cleber@fedora:~/src/avocado/avocado/optional_plugins/html��[?2004h[cleber@p50 html]$ pyth�on3 setup.py build
�[?2004l
running build
running build_py
creating build
creating build/lib
creating build/lib/avocado_result_html
copying avocado_result_html/__init__.py -> build/lib/avocado_result_html
creating build/lib/tests
copying tests/__init__.py -> build/lib/tests
copying tests/test_html_result.py -> build/lib/tests
running egg_info
creating avocado_framework_plugin_result_html.egg-info
writing avocado_framework_plugin_result_html.egg-info/PKG-INFO
writing dependency_links to avocado_framework_plugin_result_html.egg-info/dependency_links.txt
writing entry points to avocado_framework_plugin_result_html.egg-info/entry_points.txt
writing requirements to avocado_framework_plugin_result_html.egg-info/requires.txt
writing top-level names to avocado_framework_plugin_result_html.egg-info/top_level.txt
writing manifest file 'avocado_framework_plugin_result_html.egg-info/SOURCES.txt'
reading manifest file 'avocado_framework_plugin_result_html.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'avocado_framework_plugin_result_html.egg-info/SOURCES.txt'
creating build/lib/avocado_result_html/templates
copying avocado_result_html/templates/avocado_html.js -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/bootstrap.min.css -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/bootstrap.min.js -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/datatables.bootstrap.js -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/datatables.css -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/datatables.min.js -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/jquery.min.js -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/results.html -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/style.css -> build/lib/avocado_result_html/templates
creating build/lib/avocado_result_html/templates/images
copying avocado_result_html/templates/images/logs_icon.svg -> build/lib/avocado_result_html/templates/images
copying avocado_result_html/templates/images/whiteboard_icon.svg -> build/lib/avocado_result_html/templates/images
�]0;cleber@fedora:~/src/avocado/avocado/optional_plugins/html��[?2004h[cleber@p50 html]$ cd ../../
�[?2004l
�]0;cleber@fedora:~/src/avocado/avocado��[?2004h[cleber@p50 avocado]$ make clean
�[?2004l
/usr/bin/python3 setup.py clean --all
/home/cleber/src/avocado/avocado/setup.py:21: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives
from distutils.command.clean import clean
running clean
'build/lib' does not exist -- can't clean it
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-3.10' does not exist -- can't clean it
>> CLEAN /home/cleber/src/avocado/avocado/optional_plugins/html
running clean
removing 'build/lib' (and everything under it)
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-3.10' does not exist -- can't clean it
removing 'build'
>> CLEAN /home/cleber/src/avocado/avocado/optional_plugins/varianter_yaml_to_mux
running clean
'build/lib' does not exist -- can't clean it
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-3.10' does not exist -- can't clean it
>> CLEAN /home/cleber/src/avocado/avocado/optional_plugins/varianter_cit
running clean
'build/lib' does not exist -- can't clean it
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-3.10' does not exist -- can't clean it
>> CLEAN /home/cleber/src/avocado/avocado/optional_plugins/resultsdb
running clean
'build/lib' does not exist -- can't clean it
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-3.10' does not exist -- can't clean it
>> CLEAN /home/cleber/src/avocado/avocado/optional_plugins/golang
running clean
'build/lib' does not exist -- can't clean it
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-3.10' does not exist -- can't clean it
>> CLEAN /home/cleber/src/avocado/avocado/optional_plugins/robot
running clean
'build/lib' does not exist -- can't clean it
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-3.10' does not exist -- can't clean it
>> CLEAN /home/cleber/src/avocado/avocado/optional_plugins/varianter_pict
running clean
'build/lib' does not exist -- can't clean it
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-3.10' does not exist -- can't clean it
>> CLEAN /home/cleber/src/avocado/avocado/optional_plugins/result_upload
running clean
'build/lib' does not exist -- can't clean it
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-3.10' does not exist -- can't clean it
�]0;cleber@fedora:~/src/avocado/avocado��[?2004h[cleber@p50 avocado]$ ls bui�����[Koptional_plugins/html/bui�����[Kbuild
�[?2004l
ls: cannot access 'optional_plugins/html/build': No such file or directory
�]0;cleber@fedora:~/src/avocado/avocado��[?2004h[cleber@p50 avocado]$ �[?2004l
exit
Script done on 2022-03-09 08:26:38-05:00 [COMMAND_EXIT_CODE="2"]
After this:
Script started on 2022-03-09 08:27:02-05:00 [TERM="xterm-256color" TTY="/dev/pts/2" COLUMNS="145" LINES="39"]
�]0;cleber@fedora:~/src/avocado/avocado��[?2004h[cleber@p50 avocado]$ git status
�[?2004l
On branch remove_distutils
Your branch and 'ana/remove_distutils' have diverged,
and have 6 and 2 different commits each, respectively.
(use "git pull" to merge the remote branch into yours)
nothing to commit, working tree clean
�]0;cleber@fedora:~/src/avocado/avocado��[?2004h[cleber@p50 avocado]$ cd optional_plugins/html/
�[?2004l
�]0;cleber@fedora:~/src/avocado/avocado/optional_plugins/html��[?2004h[cleber@p50 html]$
(reverse-i-search)`': ���p': cd optional_�[7mp�[27mlugins/html/�����������������������������[2Py': python3 setup.�[7mpy�[27m build��������������������������[1@t': �[7mpyt�[27mhon3 setup.py��������������������[1@h': �[7mpyth�[27m����
�[7P[cleber@p50 html]$ pyth�����[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C
�[?2004l
running build
running build_py
creating build
creating build/lib
creating build/lib/avocado_result_html
copying avocado_result_html/__init__.py -> build/lib/avocado_result_html
creating build/lib/tests
copying tests/__init__.py -> build/lib/tests
copying tests/test_html_result.py -> build/lib/tests
running egg_info
creating avocado_framework_plugin_result_html.egg-info
writing avocado_framework_plugin_result_html.egg-info/PKG-INFO
writing dependency_links to avocado_framework_plugin_result_html.egg-info/dependency_links.txt
writing entry points to avocado_framework_plugin_result_html.egg-info/entry_points.txt
writing requirements to avocado_framework_plugin_result_html.egg-info/requires.txt
writing top-level names to avocado_framework_plugin_result_html.egg-info/top_level.txt
writing manifest file 'avocado_framework_plugin_result_html.egg-info/SOURCES.txt'
reading manifest file 'avocado_framework_plugin_result_html.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'avocado_framework_plugin_result_html.egg-info/SOURCES.txt'
creating build/lib/avocado_result_html/templates
copying avocado_result_html/templates/avocado_html.js -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/bootstrap.min.css -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/bootstrap.min.js -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/datatables.bootstrap.js -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/datatables.css -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/datatables.min.js -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/jquery.min.js -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/results.html -> build/lib/avocado_result_html/templates
copying avocado_result_html/templates/style.css -> build/lib/avocado_result_html/templates
creating build/lib/avocado_result_html/templates/images
copying avocado_result_html/templates/images/logs_icon.svg -> build/lib/avocado_result_html/templates/images
copying avocado_result_html/templates/images/whiteboard_icon.svg -> build/lib/avocado_result_html/templates/images
�]0;cleber@fedora:~/src/avocado/avocado/optional_plugins/html��[?2004h[cleber@p50 html]$ cd ../../
�[?2004l
�]0;cleber@fedora:~/src/avocado/avocado��[?2004h[cleber@p50 avocado]$ make clean
�[?2004l
/usr/bin/python3 setup.py clean
running clean
�]0;cleber@fedora:~/src/avocado/avocado��[?2004h[cleber@p50 avocado]$
(reverse-i-search)`': ���l': make c�[7ml�[27mean�������������s': �[7mls�[27m optional_plugins/html/build
�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C
�[2P[cleber@p50 avocado]$ ls���[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C�[C
�[?2004l
�[0m�[01;34mlib�[0m
�]0;cleber@fedora:~/src/avocado/avocado��[?2004h[cleber@p50 avocado]$ ls optional_plugins/html/build/lib/
�[?2004l
�[0m�[01;34mavocado_result_html�[0m �[01;34mtests�[0m
�]0;cleber@fedora:~/src/avocado/avocado��[?2004h[cleber@p50 avocado]$ �[?2004l
exit
Script done on 2022-03-09 08:27:39-05:00 [COMMAND_EXIT_CODE="0"]
If you look at the last few lines, you can see build, build/lib, build/lib/avocado_result_html, etc.
Let me know if I'm missing something.
There was a problem hiding this comment.
@clebergnu yes, if you're using commands from one setup.py (in the plugin) and then from the setup.py in root directory, the behavior is like that. I thought you were talking about the main setup.py all the time...
I'm personally using git clean all the time so I'm closing this issue without plans to revisit it.
ana
force-pushed
the
remove_distutils
branch
3 times, most recently
from
3ae2776 to
859d878
Compare
Make custom command 'clean' to inherit from SimpleCommand instead of the clean command from distutils. Everything is already cleaned by the custom command anyway. Fixes: avocado-framework#5159 Signed-off-by: Ana Guerrero Lopez <anguerre@redhat.com>
This call doesn't clean anything, everything is cleaned up by the custom call command in setup.py. And once distutils is removed from setuptools these calls in the plugins will fail. Signed-off-by: Ana Guerrero Lopez <anguerre@redhat.com>
ana
force-pushed
the
remove_distutils
branch
from
bee4f6c to
6c23def
Compare
Make custom command 'clean' to inherit from SimpleCommand instead of the clean command from distutils.
Everything is already cleaned by the custom command. And because this, remove the clean call for plugins as well.
Once distutils is removed from setuptools these calls in the plugins will fail.