-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to using Pixi/Conda for Windows. #802
base: master
Are you sure you want to change the base?
Changes from 1 commit
eaed019
b7b5b52
381c584
dd9ffb1
b553762
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,10 +14,3 @@ | |
path = windows_docker_resources/rticonnextdds-license | ||
url = [email protected]:osrf/rticonnextdds-license.git | ||
branch = license | ||
[submodule "windows_docker_resources/qtaccount"] | ||
path = windows_docker_resources/qtaccount | ||
url = [email protected]:osrf/qtaccount | ||
clalancette marked this conversation as resolved.
Show resolved
Hide resolved
|
||
[submodule "windows_docker_resources/ros2-cookbooks"] | ||
path = windows_docker_resources/ros2-cookbooks | ||
url = [email protected]:ros-infrastructure/ros2-cookbooks | ||
branch = latest |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -50,42 +50,6 @@ | |||||
|
||||||
PipPackage = collections.namedtuple('PipPackage', ['pkgname' ,'importname', 'version']) | ||||||
|
||||||
pip_dependencies = [ | ||||||
PipPackage('EmPy', 'em', '<4'), | ||||||
PipPackage('catkin_pkg', 'catkin_pkg', ''), | ||||||
PipPackage('cryptography', 'cryptography', '<=3.0'), | ||||||
PipPackage('coverage', 'coverage', ''), | ||||||
PipPackage('flake8', 'flake8', '<5.0.0'), | ||||||
PipPackage('flake8-blind-except', 'flake8_blind_except', '==0.1.1'), | ||||||
PipPackage('flake8-builtins', 'flake8_builtins', ''), | ||||||
PipPackage('flake8-class-newline', 'flake8_class_newline', ''), | ||||||
PipPackage('flake8-comprehensions', 'flake8_comprehensions', ''), | ||||||
PipPackage('flake8-deprecated', 'flake8_deprecated', ''), | ||||||
PipPackage('flake8-docstrings', 'flake8_docstrings', ''), | ||||||
PipPackage('flake8-import-order', 'flake8_import_order', ''), | ||||||
PipPackage('flake8-quotes', 'flake8_quotes', ''), | ||||||
PipPackage('importlib-metadata', 'importlib_metadata', ''), | ||||||
PipPackage('lark', 'lark', '==1.1.1'), | ||||||
PipPackage('mypy', 'mypy', '==0.931'), | ||||||
PipPackage('nose', 'nose', ''), | ||||||
PipPackage('osrf-pycommon', 'osrf_pycommon', ''), | ||||||
PipPackage('pathspec', 'pathspec', ''), | ||||||
PipPackage('pydocstyle', 'pydocstyle', ''), | ||||||
PipPackage('pyflakes', 'pyflakes', ''), | ||||||
PipPackage('pyparsing', 'pyparsing', '==2.4.7'), | ||||||
PipPackage('pytest', 'pytest', '==6.2.5'), | ||||||
PipPackage('pytest-cov', 'pytest_cov', ''), | ||||||
PipPackage('pytest-mock', 'pytest_mock', ''), | ||||||
PipPackage('pytest-repeat', 'pytest_repeat', ''), | ||||||
PipPackage('pytest-rerunfailures', 'pytest_rerunfailures', ''), | ||||||
PipPackage('pytest-runner', 'ptr', ''), | ||||||
PipPackage('pytest-timeout', 'pytest_timeout', '==2.1.0'), | ||||||
PipPackage('pyyaml', 'yaml', ''), | ||||||
PipPackage('setuptools', 'setuptools', '==59.6.0'), | ||||||
PipPackage('vcstool', 'vcstool', ''), | ||||||
PipPackage('yamllint', 'yamllint', ''), | ||||||
] | ||||||
|
||||||
colcon_packages = [ | ||||||
PipPackage('colcon-core', 'colcon_core', ''), | ||||||
PipPackage('colcon-defaults', 'colcon_defaults', ''), | ||||||
|
@@ -197,9 +161,6 @@ def __call__(self, parser, namespace, values, option_string=None): | |||||
parser.add_argument( | ||||||
'--force-ansi-color', default=False, action='store_true', | ||||||
help="forces this program to output ansi color") | ||||||
parser.add_argument( | ||||||
'--ros-distro', required=True, | ||||||
help="The ROS distribution being built") | ||||||
parser.add_argument( | ||||||
'--colcon-mixin-url', default=None, | ||||||
help='A mixin index url to be included by colcon') | ||||||
|
@@ -453,78 +414,10 @@ def run(args, build_function, blacklisted_package_names=None): | |||||
|
||||||
# Now inside of the workspace... | ||||||
with change_directory(args.workspace): | ||||||
def need_package_from_pipy(pkg_name): | ||||||
try: | ||||||
importlib.import_module(pkg_name) | ||||||
except ModuleNotFoundError: | ||||||
return True | ||||||
|
||||||
return False | ||||||
|
||||||
print('# BEGIN SUBSECTION: install Python packages') | ||||||
# Print setuptools version | ||||||
job.run(['"%s"' % job.python, '-c', '"import setuptools; print(setuptools.__version__)"'], | ||||||
shell=True) | ||||||
|
||||||
# Print the pip version | ||||||
job.run(['"%s"' % job.python, '-m', 'pip', '--version'], shell=True) | ||||||
|
||||||
# Install pip dependencies | ||||||
pip_packages = [] | ||||||
constraints = [] | ||||||
|
||||||
potential_pip_packages = pip_dependencies | ||||||
if not args.colcon_branch: | ||||||
potential_pip_packages += colcon_packages | ||||||
|
||||||
# We prefer to get packages from the distribution if they are already installed. | ||||||
# If not, we add to the list to install from pip. | ||||||
for pkgname, importname, version in pip_dependencies: | ||||||
if need_package_from_pipy(importname): | ||||||
pip_packages.append(pkgname) | ||||||
# Even if we don't need to install the package, we still add the constraints | ||||||
# (if they exist) so that other package installations will respect these. | ||||||
if version: | ||||||
constraints.append(f'{pkgname}{version}') | ||||||
|
||||||
if sys.platform == 'win32': | ||||||
# Install fork of pyreadline containing fix for deprecation warnings | ||||||
# TODO(jacobperron): Until upstream issue is resolved https://github.com/pyreadline/pyreadline/issues/65 | ||||||
pip_packages += ['git+https://github.com/osrf/pyreadline'] | ||||||
|
||||||
# Setuptools > 61 somehow have broken Windows Debug. Pin it to 59.6.0 here which | ||||||
# matches Ubuntu Jammy, and wait until upstream setuptools settles down. | ||||||
pip_packages += ["setuptools==59.6.0"] | ||||||
|
||||||
pip_packages += ['cryptography', 'lxml', 'numpy'] | ||||||
if args.ros_distro in ('humble', 'iron'): | ||||||
pip_packages.append('netifaces') | ||||||
|
||||||
# to ensure that the build type specific package is installed | ||||||
job.run( | ||||||
['"%s"' % job.python, '-m', 'pip', 'uninstall', '-y'] + | ||||||
['cryptography', 'lxml', 'numpy'], shell=True) | ||||||
|
||||||
if pip_packages: | ||||||
print('Using constraints:') | ||||||
print('\n'.join(constraints)) | ||||||
with open('constraints.txt', 'w') as outfp: | ||||||
outfp.write('\n'.join(constraints) + '\n') | ||||||
|
||||||
pip_cmd = ['"%s"' % job.python, '-m', 'pip', 'install', '-c', 'constraints.txt', '-U'] | ||||||
if sys.platform == 'win32': | ||||||
# Force reinstall so all dependencies are in virtual environment | ||||||
# On Windows since we switch between the debug and non-debug | ||||||
# interpreter all packages need to be reinstalled too | ||||||
pip_cmd.append('--force-reinstall') | ||||||
|
||||||
job.run( | ||||||
pip_cmd + pip_packages, | ||||||
shell=True) | ||||||
|
||||||
vcs_cmd = ['vcs'] | ||||||
|
||||||
if args.colcon_branch: | ||||||
print('# BEGIN SUBSECTION: install custom colcon') | ||||||
# create .repos file for colcon repositories | ||||||
os.makedirs('colcon', exist_ok=True) | ||||||
with open('colcon/colcon.repos', 'w') as h: | ||||||
|
@@ -553,19 +446,30 @@ def need_package_from_pipy(pkg_name): | |||||
['"%s"' % job.python, '-m', 'pip', 'install', '-U'] + | ||||||
['colcon/%s' % pkgname for pkgname, importname, version in colcon_packages], | ||||||
shell=True) | ||||||
print('# END SUBSECTION') | ||||||
|
||||||
colcon_script = which('colcon') | ||||||
|
||||||
# Show what pip has | ||||||
job.run(['"%s"' % job.python, '-m', 'pip', 'freeze', '--all'], shell=True) | ||||||
print('# END SUBSECTION') | ||||||
|
||||||
# Fetch colcon mixins | ||||||
if args.colcon_mixin_url: | ||||||
print('# BEGIN SUBSECTION: Fetch colcon mixins') | ||||||
true_cmd = 'VER>NUL' if sys.platform == 'win32' else 'true' | ||||||
job.run([colcon_script, 'mixin', 'remove', 'default', '||', true_cmd], shell=True) | ||||||
job.run([colcon_script, 'mixin', 'add', 'default', args.colcon_mixin_url], shell=True) | ||||||
job.run([colcon_script, 'mixin', 'update', 'default'], shell=True) | ||||||
print('# END SUBSECTION') | ||||||
|
||||||
print('# BEGIN SUBSECTION: Print python versions') | ||||||
# Print setuptools version | ||||||
job.run(['"%s"' % job.python, '-c', '"import setuptools; print(setuptools.__version__)"'], | ||||||
shell=True) | ||||||
clalancette marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
# Print the pip version | ||||||
job.run(['"%s"' % job.python, '-m', 'pip', '--version'], shell=True) | ||||||
|
||||||
# Show what pip has | ||||||
job.run(['"%s"' % job.python, '-m', 'pip', 'list'], shell=True) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we wouldn't prefer This suggestion is non-blocking.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, this is a good question. Early on in development here, I found that But it is long enough ago that I now don't remember exactly what the problem was. Let me go back and do some testing here and see if that was either a misconfiguration, or something more serious here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, right. The issue popped up immediately when I tried it locally. For reasons I don't understand, when installing python packages via pixi/conda, the output from
(truncated for brevity). As you can see, the output there doesn't give much useful information about which version of the package is installed, and thus in the CI job output it isn't very helpful. That's why I switched to |
||||||
print('# END SUBSECTION') | ||||||
|
||||||
print('# BEGIN SUBSECTION: import repositories') | ||||||
repos_file_urls = [args.repo_file_url] | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,7 +53,7 @@ def show_env(self): | |
# Show the env | ||
self.run(['export'], shell=True) | ||
# Show what pip has | ||
self.run(['"%s"' % self.python, '-m', 'pip', 'freeze', '--all'], shell=True) | ||
self.run(['"%s"' % self.python, '-m', 'pip', 'list'], shell=True) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, I'd actually advocate for |
||
|
||
def setup_env(self): | ||
current_run = self.run | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,7 @@ def show_env(self): | |
# Show the env | ||
self.run(['set'], shell=True) | ||
# Show what pip has | ||
self.run([self.python, '-m', 'pip', 'freeze', '--all']) | ||
self.run([self.python, '-m', 'pip', 'list']) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, I'd actually advocate for |
||
|
||
def setup_env(self): | ||
# Generate the env file | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These repositories can not be archived/removed are still in use in other places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they can probably be archived, but I'd say let's wait to do this for a while until this is all settled.