diff mbox

[lttng-ust,v2] Fix: (un)install targets of Python agent

Message ID 1488473765-3345-1-git-send-email-francis.deslauriers@efficios.com
State Accepted, archived
Headers show

Commit Message

Francis Deslauriers March 2, 2017, 4:56 p.m. UTC
This Makefile was using Distutils' setup.py to install the Python agent
but was using the Autoconf's $pkgpythondir variable for the uninstall
process. The two folders can be different on some distributions which
made the uninstall attempting to delete a non-existant folder and
effectively not uninstalling.

We now run a phony installation of the bindings in a temporary directory
and use the tree structure of the install folder to infere the location
of the files on the system to delete them.

Also, we print a warning if the install directory is not included in the
PYTHONPATH variable.

Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
---
 python-lttngust/Makefile.am | 38 ++++++++++++++++++++-------
 python-lttngust/setup.py.in | 64 +++++++++++++++++++++++++++++++++------------
 2 files changed, 77 insertions(+), 25 deletions(-)

Comments

Mathieu Desnoyers March 2, 2017, 9:23 p.m. UTC | #1
merged with modifications.

----- On Mar 2, 2017, at 11:56 AM, Francis Deslauriers francis.deslauriers at efficios.com wrote:

> This Makefile was using Distutils' setup.py to install the Python agent
> but was using the Autoconf's $pkgpythondir variable for the uninstall
> process. The two folders can be different on some distributions which
> made the uninstall attempting to delete a non-existant folder and
> effectively not uninstalling.
> 
> We now run a phony installation of the bindings in a temporary directory
> and use the tree structure of the install folder to infere the location
> of the files on the system to delete them.
> 
> Also, we print a warning if the install directory is not included in the
> PYTHONPATH variable.
> 
> Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
> ---
> python-lttngust/Makefile.am | 38 ++++++++++++++++++++-------
> python-lttngust/setup.py.in | 64 +++++++++++++++++++++++++++++++++------------
> 2 files changed, 77 insertions(+), 25 deletions(-)
> 
> diff --git a/python-lttngust/Makefile.am b/python-lttngust/Makefile.am
> index cc28989..c5bb167 100644
> --- a/python-lttngust/Makefile.am
> +++ b/python-lttngust/Makefile.am
> @@ -1,7 +1,6 @@
> # Use setup.py for the installation instead of Autoconf.
> # This ease the installation process and assure a *pythonic*
> # installation.
> -agent_path=lttngust
> all-local:
> 	$(PYTHON) setup.py build --verbose
> 
> @@ -13,13 +12,34 @@ install-exec-local:
> 	$(PYTHON) setup.py install $$opts;
> 
> clean-local:
> -	rm -rf build
> +	rm -rf $(builddir)/build
> 
> -uninstall-local:
> -	rm -rf $(DESTDIR)$(pkgpythondir)
> -
> -EXTRA_DIST=$(agent_path)
> +# Distutils' setup.py does not include an uninstall target, we thus need to do
> +# it manually. We fake an install in a temporary folder and use the generated
> +# tree structure to infere the actual location within the install prefix.
> +# 1. Create temporary file and folder
> +# 2. Set the root install folder for a temporary folder
> +# 3. Install in that temporary folder and record all the files installed
> +# 4. If DESTDIR is set, prepend it to the paths of the install files
> +# 5. Remove the installed files and the Python package folder
> +# 6. Remove the files created by this target
> 
> -# Remove automake generated file before dist
> -dist-hook:
> -	rm -rf $(distdir)/$(agent_path)/__init__.py
> +uninstall-local:
> +	$(eval TMP_INSTALLED_FILES:=$(shell mktemp
> $(builddir)/tmp-installed-files-XXXXXX))
> +	$(eval TMP_INSTALL_DIR:=$(shell mktemp -d $(builddir)/tmp-install-dir-XXXXXX))
> +	$(eval TMP_BUILD_DIR:=$(shell mktemp -d $(builddir)/tmp-build-dir-XXXXXX))
> +	@opts="--root=$(TMP_INSTALL_DIR) --prefix=$(prefix) --record
> $(TMP_INSTALLED_FILES) --no-compile $(DISTSETUPOPTS)"; \
> +	if [ "$(DESTDIR)" != "" ]; then \
> +		opts="$$opts --root=$(DESTDIR)"; \
> +	else \
> +		opts="$$opts --root=$(TMP_INSTALL_DIR)"; \
> +	fi; \
> +	$(PYTHON) setup.py build --build-base $(TMP_BUILD_DIR) install $$opts ; \
> +	if [ "$(DESTDIR)" != "" ]; then \
> +		$(SED) -i "s|^|$(DESTDIR)/|g" $(TMP_INSTALLED_FILES); \
> +	fi; \
> +	cat $(TMP_INSTALLED_FILES) | xargs rm -rf
> +	$(GREP) "__init__.py" $(TMP_INSTALLED_FILES) | xargs dirname | xargs rm -rf
> +	rm -f $(TMP_INSTALLED_FILES)
> +	rm -rf $(TMP_INSTALL_DIR)
> +	rm -rf $(TMP_BUILD_DIR)
> diff --git a/python-lttngust/setup.py.in b/python-lttngust/setup.py.in
> index 370fd61..303d624 100644
> --- a/python-lttngust/setup.py.in
> +++ b/python-lttngust/setup.py.in
> @@ -15,22 +15,54 @@
> # along with this library; if not, write to the Free Software Foundation, Inc.,
> # 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
> 
> +import os
> +import sys
> +
> from distutils.core import setup, Extension
> 
> +PY_PATH_WARN_MSG = """
> +-------------------------------------WARNING------------------------------------
> +The install directory used:\n ({0})\nis not included in your PYTHONPATH.
> +
> +To add this directory to your Python search path permanently you can add the
> +following command to your .bashrc/.zshrc:
> +    export PYTHONPATH="${{PYTHONPATH}}:{0}"
> +--------------------------------------------------------------------------------
> +"""
> +
> +def main():
> +    dist = setup(name='lttngust',
> +            version='@PACKAGE_VERSION@',
> +            description='LTTng-UST Python agent',
> +            packages=['lttngust'],
> +            package_dir={'lttngust': 'lttngust'},
> +            options={'build': {'build_base': 'build'}},
> +            url='http://lttng.org',
> +            license='LGPL-2.1',
> +            classifiers=[
> +                'Development Status :: 5 - Production/Stable',
> +                'Intended Audience :: Developers',
> +                'License :: OSI Approved :: GNU Lesser General Public License
> v2 (LGPLv2)',
> +                'Programming Language :: Python :: 2.7',
> +                'Programming Language :: Python :: 3'
> +                'Topic :: System :: Logging',
> +                ])
> +
> +# After the installation, we check that the install directory is included in
> +# the Python search path and we print a warning message when it's not. We need
> +# to do this because Python search path differs depending on the distro and
> +# some distros don't include any `/usr/local/` (the default install prefix) in
> +# the search path. This is also useful for out-of-tree installs and tests. It's
> +# only relevant to make this check on the `install` command.
> +
> +    if 'install' in dist.command_obj:
> +        install_dir = dist.command_obj['install'].install_libbase
> +        if install_dir not in sys.path:
> +            # We can't consider this an error because if affects every
> +            # distro differently. We only warn the user that some
> +            # extra configuration is needed to use the agent
> +            abs_install_dir = os.path.abspath(install_dir)
> +            print(PY_PATH_WARN_MSG.format(abs_install_dir))
> 
> -setup(name='lttngust',
> -      version='@PACKAGE_VERSION@',
> -      description='LTTng-UST Python agent',
> -      packages=['lttngust'],
> -      package_dir={'lttngust': 'lttngust'},
> -      options={'build': {'build_base': 'build'}},
> -      url='http://lttng.org',
> -      license='LGPL-2.1',
> -      classifiers=[
> -          'Development Status :: 5 - Production/Stable',
> -          'Intended Audience :: Developers',
> -          'License :: OSI Approved :: GNU Lesser General Public License v2
> (LGPLv2)',
> -          'Programming Language :: Python :: 2.7',
> -          'Programming Language :: Python :: 3'
> -          'Topic :: System :: Logging',
> -      ])
> +if __name__ == '__main__':
> +    main()
> --
> 2.7.4
diff mbox

Patch

diff --git a/python-lttngust/Makefile.am b/python-lttngust/Makefile.am
index cc28989..c5bb167 100644
--- a/python-lttngust/Makefile.am
+++ b/python-lttngust/Makefile.am
@@ -1,7 +1,6 @@ 
 # Use setup.py for the installation instead of Autoconf.
 # This ease the installation process and assure a *pythonic*
 # installation.
-agent_path=lttngust
 all-local:
 	$(PYTHON) setup.py build --verbose
 
@@ -13,13 +12,34 @@  install-exec-local:
 	$(PYTHON) setup.py install $$opts;
 
 clean-local:
-	rm -rf build
+	rm -rf $(builddir)/build
 
-uninstall-local:
-	rm -rf $(DESTDIR)$(pkgpythondir)
-
-EXTRA_DIST=$(agent_path)
+# Distutils' setup.py does not include an uninstall target, we thus need to do
+# it manually. We fake an install in a temporary folder and use the generated
+# tree structure to infere the actual location within the install prefix.
+# 1. Create temporary file and folder
+# 2. Set the root install folder for a temporary folder
+# 3. Install in that temporary folder and record all the files installed
+# 4. If DESTDIR is set, prepend it to the paths of the install files
+# 5. Remove the installed files and the Python package folder
+# 6. Remove the files created by this target
 
-# Remove automake generated file before dist
-dist-hook:
-	rm -rf $(distdir)/$(agent_path)/__init__.py
+uninstall-local:
+	$(eval TMP_INSTALLED_FILES:=$(shell mktemp $(builddir)/tmp-installed-files-XXXXXX))
+	$(eval TMP_INSTALL_DIR:=$(shell mktemp -d $(builddir)/tmp-install-dir-XXXXXX))
+	$(eval TMP_BUILD_DIR:=$(shell mktemp -d $(builddir)/tmp-build-dir-XXXXXX))
+	@opts="--root=$(TMP_INSTALL_DIR) --prefix=$(prefix) --record $(TMP_INSTALLED_FILES) --no-compile $(DISTSETUPOPTS)"; \
+	if [ "$(DESTDIR)" != "" ]; then \
+		opts="$$opts --root=$(DESTDIR)"; \
+	else \
+		opts="$$opts --root=$(TMP_INSTALL_DIR)"; \
+	fi; \
+	$(PYTHON) setup.py build --build-base $(TMP_BUILD_DIR) install $$opts ; \
+	if [ "$(DESTDIR)" != "" ]; then \
+		$(SED) -i "s|^|$(DESTDIR)/|g" $(TMP_INSTALLED_FILES); \
+	fi; \
+	cat $(TMP_INSTALLED_FILES) | xargs rm -rf
+	$(GREP) "__init__.py" $(TMP_INSTALLED_FILES) | xargs dirname | xargs rm -rf
+	rm -f $(TMP_INSTALLED_FILES)
+	rm -rf $(TMP_INSTALL_DIR)
+	rm -rf $(TMP_BUILD_DIR)
diff --git a/python-lttngust/setup.py.in b/python-lttngust/setup.py.in
index 370fd61..303d624 100644
--- a/python-lttngust/setup.py.in
+++ b/python-lttngust/setup.py.in
@@ -15,22 +15,54 @@ 
 # along with this library; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
 
+import os
+import sys
+
 from distutils.core import setup, Extension
 
+PY_PATH_WARN_MSG = """
+-------------------------------------WARNING------------------------------------
+The install directory used:\n ({0})\nis not included in your PYTHONPATH.
+
+To add this directory to your Python search path permanently you can add the
+following command to your .bashrc/.zshrc:
+    export PYTHONPATH="${{PYTHONPATH}}:{0}"
+--------------------------------------------------------------------------------
+"""
+
+def main():
+    dist = setup(name='lttngust',
+            version='@PACKAGE_VERSION@',
+            description='LTTng-UST Python agent',
+            packages=['lttngust'],
+            package_dir={'lttngust': 'lttngust'},
+            options={'build': {'build_base': 'build'}},
+            url='http://lttng.org',
+            license='LGPL-2.1',
+            classifiers=[
+                'Development Status :: 5 - Production/Stable',
+                'Intended Audience :: Developers',
+                'License :: OSI Approved :: GNU Lesser General Public License v2 (LGPLv2)',
+                'Programming Language :: Python :: 2.7',
+                'Programming Language :: Python :: 3'
+                'Topic :: System :: Logging',
+                ])
+
+# After the installation, we check that the install directory is included in
+# the Python search path and we print a warning message when it's not. We need
+# to do this because Python search path differs depending on the distro and
+# some distros don't include any `/usr/local/` (the default install prefix) in
+# the search path. This is also useful for out-of-tree installs and tests. It's
+# only relevant to make this check on the `install` command.
+
+    if 'install' in dist.command_obj:
+        install_dir = dist.command_obj['install'].install_libbase
+        if install_dir not in sys.path:
+            # We can't consider this an error because if affects every
+            # distro differently. We only warn the user that some
+            # extra configuration is needed to use the agent
+            abs_install_dir = os.path.abspath(install_dir)
+            print(PY_PATH_WARN_MSG.format(abs_install_dir))
 
-setup(name='lttngust',
-      version='@PACKAGE_VERSION@',
-      description='LTTng-UST Python agent',
-      packages=['lttngust'],
-      package_dir={'lttngust': 'lttngust'},
-      options={'build': {'build_base': 'build'}},
-      url='http://lttng.org',
-      license='LGPL-2.1',
-      classifiers=[
-          'Development Status :: 5 - Production/Stable',
-          'Intended Audience :: Developers',
-          'License :: OSI Approved :: GNU Lesser General Public License v2 (LGPLv2)',
-          'Programming Language :: Python :: 2.7',
-          'Programming Language :: Python :: 3'
-          'Topic :: System :: Logging',
-      ])
+if __name__ == '__main__':
+    main()