From 45c06ac5bc853292f4e2dc633920a366c7467c5f Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Wed, 30 Aug 2023 13:43:22 -0500 Subject: [PATCH 1/5] Implement a custom distutils command to symlink data_files The default implementation of install_data will always copy files into the destination directory. When we use the 'develop' command, we actually need to specifically tell setuptools to do something with the data_files or they will be ignored. Instead of telling setuptools to use install_data as-is, we can implement a custom version of install_data that will try to symlink the files instead. --- colcon_core/task/python/build.py | 2 +- colcon_core/task/python/symlink_data.py | 26 +++++++++++++++++++++++++ setup.cfg | 2 ++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 colcon_core/task/python/symlink_data.py diff --git a/colcon_core/task/python/build.py b/colcon_core/task/python/build.py index c177b809..c042dbc5 100644 --- a/colcon_core/task/python/build.py +++ b/colcon_core/task/python/build.py @@ -138,7 +138,7 @@ async def build(self, *, additional_hooks=None): # noqa: D102 '--no-deps', ] if setup_py_data.get('data_files'): - cmd += ['install_data'] + cmd += ['symlink_data'] completed = await run( self.context, cmd, cwd=args.build_base, env=env) finally: diff --git a/colcon_core/task/python/symlink_data.py b/colcon_core/task/python/symlink_data.py new file mode 100644 index 00000000..e73886d4 --- /dev/null +++ b/colcon_core/task/python/symlink_data.py @@ -0,0 +1,26 @@ +# Copyright 2023 Open Source Robotics Foundation, Inc. +# Licensed under the Apache License, Version 2.0 + +from distutils.command.install_data import install_data +import os.path +import warnings + + +class symlink_data(install_data): # noqa: N801 + """Like install_data, but symlink files instead of copying.""" + + def copy_file(self, src, dst, **kwargs): # noqa: D102 + if kwargs.get('link'): + return super().copy_file(src, dst, **kwargs) + + kwargs['link'] = 'sym' + src = os.path.abspath(src) + + try: + return super().copy_file(src, dst, **kwargs) + except OSError: + warnings.warning( + 'Failed to symlink data_file(s), falling back to copy') + del kwargs['link'] + + return super().copy_file(src, dst, **kwargs) diff --git a/setup.cfg b/setup.cfg index 855b91d2..4f41a5f8 100644 --- a/setup.cfg +++ b/setup.cfg @@ -143,6 +143,8 @@ colcon_core.verb = test = colcon_core.verb.test:TestVerb console_scripts = colcon = colcon_core.command:main +distutils.commands = + symlink_data = colcon_core.task.python.symlink_data:symlink_data pytest11 = colcon_core_warnings_stderr = colcon_core.pytest.hooks From 9c0655f2811e75d273f2ff08bcbbaa3527ed307e Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Thu, 31 Aug 2023 15:16:33 -0500 Subject: [PATCH 2/5] Move the entry point to avoid all the __init__ --- colcon_core/distutils/__init__.py | 0 colcon_core/distutils/commands/__init__.py | 0 colcon_core/{task/python => distutils/commands}/symlink_data.py | 0 setup.cfg | 2 +- 4 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 colcon_core/distutils/__init__.py create mode 100644 colcon_core/distutils/commands/__init__.py rename colcon_core/{task/python => distutils/commands}/symlink_data.py (100%) diff --git a/colcon_core/distutils/__init__.py b/colcon_core/distutils/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/colcon_core/distutils/commands/__init__.py b/colcon_core/distutils/commands/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/colcon_core/task/python/symlink_data.py b/colcon_core/distutils/commands/symlink_data.py similarity index 100% rename from colcon_core/task/python/symlink_data.py rename to colcon_core/distutils/commands/symlink_data.py diff --git a/setup.cfg b/setup.cfg index 4f41a5f8..c0ec6844 100644 --- a/setup.cfg +++ b/setup.cfg @@ -144,7 +144,7 @@ colcon_core.verb = console_scripts = colcon = colcon_core.command:main distutils.commands = - symlink_data = colcon_core.task.python.symlink_data:symlink_data + symlink_data = colcon_core.distutils.commands.symlink_data:symlink_data pytest11 = colcon_core_warnings_stderr = colcon_core.pytest.hooks From 4b90b14ccf96cd4d5219a0c4b413429069ede800 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Thu, 31 Aug 2023 17:05:00 -0500 Subject: [PATCH 3/5] Drop fallback, symlink or die trying This matches the existing behavior in colcon --- colcon_core/distutils/commands/symlink_data.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/colcon_core/distutils/commands/symlink_data.py b/colcon_core/distutils/commands/symlink_data.py index e73886d4..15663909 100644 --- a/colcon_core/distutils/commands/symlink_data.py +++ b/colcon_core/distutils/commands/symlink_data.py @@ -3,7 +3,6 @@ from distutils.command.install_data import install_data import os.path -import warnings class symlink_data(install_data): # noqa: N801 @@ -15,12 +14,4 @@ def copy_file(self, src, dst, **kwargs): # noqa: D102 kwargs['link'] = 'sym' src = os.path.abspath(src) - - try: - return super().copy_file(src, dst, **kwargs) - except OSError: - warnings.warning( - 'Failed to symlink data_file(s), falling back to copy') - del kwargs['link'] - return super().copy_file(src, dst, **kwargs) From e4799ea1fff9b0786d543ec30c89d77a4dc38716 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 1 Sep 2023 13:21:16 -0500 Subject: [PATCH 4/5] Handle switching between symlink and normal --- colcon_core/distutils/commands/symlink_data.py | 11 ++++++++++- colcon_core/task/python/build.py | 13 ++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/colcon_core/distutils/commands/symlink_data.py b/colcon_core/distutils/commands/symlink_data.py index 15663909..ae1c7e74 100644 --- a/colcon_core/distutils/commands/symlink_data.py +++ b/colcon_core/distutils/commands/symlink_data.py @@ -2,7 +2,7 @@ # Licensed under the Apache License, Version 2.0 from distutils.command.install_data import install_data -import os.path +import os class symlink_data(install_data): # noqa: N801 @@ -12,6 +12,15 @@ def copy_file(self, src, dst, **kwargs): # noqa: D102 if kwargs.get('link'): return super().copy_file(src, dst, **kwargs) + if self.force: + # os.symlink fails if the destination exists as a regular file + if os.path.isdir(dst): + target = os.path.join(dst, os.path.basename(src)) + else: + target = dst + if os.path.exists(dst) and not os.path.islink(dst): + os.remove(target) + kwargs['link'] = 'sym' src = os.path.abspath(src) return super().copy_file(src, dst, **kwargs) diff --git a/colcon_core/task/python/build.py b/colcon_core/task/python/build.py index c042dbc5..0f40978b 100644 --- a/colcon_core/task/python/build.py +++ b/colcon_core/task/python/build.py @@ -114,13 +114,19 @@ async def build(self, *, additional_hooks=None): # noqa: D102 # prevent installation of dependencies specified in setup.py cmd.append('--single-version-externally-managed') self._append_install_layout(args, cmd) + if setup_py_data.get('data_files'): + cmd += ['install_data'] + if rc is not None: + cmd += ['--force'] completed = await run( self.context, cmd, cwd=args.path, env=env) if completed.returncode: return completed.returncode else: - self._undo_install(pkg, args, setup_py_data, python_lib) + rc = self._undo_install(pkg, args, setup_py_data, python_lib) + if rc: + return rc temp_symlinks = self._symlinks_in_build(args, setup_py_data) # invoke `setup.py develop` step in build space @@ -139,6 +145,8 @@ async def build(self, *, additional_hooks=None): # noqa: D102 ] if setup_py_data.get('data_files'): cmd += ['symlink_data'] + if rc is not None: + cmd += ['--force'] completed = await run( self.context, cmd, cwd=args.build_base, env=env) finally: @@ -198,6 +206,8 @@ async def _undo_develop(self, pkg, args, env): ] completed = await run( self.context, cmd, cwd=args.build_base, env=env) + if not completed.returncode: + os.remove(setup_py_build_space) return completed.returncode def _undo_install(self, pkg, args, setup_py_data, python_lib): @@ -240,6 +250,7 @@ def _undo_install(self, pkg, args, setup_py_data, python_lib): with suppress(OSError): os.rmdir(d) os.remove(install_log) + return 0 def _symlinks_in_build(self, args, setup_py_data): items = ['setup.py'] From 367200d017d28842e06626df3aef854b1abda82a Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 2 Feb 2024 14:29:40 -0600 Subject: [PATCH 5/5] Add some function descriptions to clarify new behavior --- colcon_core/task/python/build.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/colcon_core/task/python/build.py b/colcon_core/task/python/build.py index 12246323..5dc89e44 100644 --- a/colcon_core/task/python/build.py +++ b/colcon_core/task/python/build.py @@ -197,6 +197,12 @@ async def _get_available_commands(self, path, env): return commands async def _undo_develop(self, pkg, args, env): + """ + Undo a previously run 'develop' command. + + :returns: None if develop was not previously detected, otherwise + an integer return code where zero indicates success. + """ # undo previous develop if .egg-info is found and develop symlinks egg_info = os.path.join( args.build_base, '%s.egg-info' % pkg.name.replace('-', '_')) @@ -215,6 +221,12 @@ async def _undo_develop(self, pkg, args, env): return completed.returncode def _undo_install(self, pkg, args, setup_py_data, python_lib): + """ + Undo a previously run 'install' command. + + :returns: None if install was not previously detected, otherwise + an integer return code where zero indicates success. + """ # undo previous install if install.log is found install_log = os.path.join(args.build_base, 'install.log') if not os.path.exists(install_log):