Skip to content
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

[Windows][melodic] remove path splash separator from 'package_dir' #267

Merged
merged 1 commit into from Jul 10, 2019

Conversation

seanyen
Copy link
Contributor

@seanyen seanyen commented Feb 19, 2019

When running catkin_make (or catkin_make_isolated) on Windows, distutils throws errors for cv_bridge. And it turned out that distutils doesn't like the path separator not matched to the underlying OS convention in setup.py.

Traceback (most recent call last):
  File "C:/dr_ws/src/vision_opencv/cv_bridge\setup.py", line 11, in <module>
    setup(**d)
  File "C:\opt\python27amd64\lib\distutils\core.py", line 151, in setup
    dist.run_commands()
  File "C:\opt\python27amd64\lib\distutils\dist.py", line 953, in run_commands
    self.run_command(cmd)
  File "C:\opt\python27amd64\lib\distutils\dist.py", line 972, in run_command
    cmd_obj.run()
  File "C:\opt\python27amd64\lib\distutils\command\build.py", line 127, in run
    self.run_command(cmd_name)
  File "C:\opt\python27amd64\lib\distutils\cmd.py", line 326, in run_command
    self.distribution.run_command(command)
  File "C:\opt\python27amd64\lib\distutils\dist.py", line 971, in run_command
    cmd_obj.ensure_finalized()
  File "C:\opt\python27amd64\lib\distutils\cmd.py", line 109, in ensure_finalized
    self.finalize_options()
  File "C:\opt\python27amd64\lib\distutils\command\build_py.py", line 56, in finalize_options
    self.package_dir[name] = convert_path(path)
  File "C:\opt\python27amd64\lib\distutils\util.py", line 126, in convert_path
    raise ValueError, "path '%s' cannot end with '/'" % pathname
ValueError: path 'python/' cannot end with '/'

Per the example in catkin doc, it is not mandatory to tail the package_dir with / so it can be easily fixed by removing it and make the code more portable.

@mjcarroll
Copy link
Contributor

@seanyen Can you give this a quick rebase on the latest melodic branch to see that it works with travis?

@seanyen
Copy link
Contributor Author

seanyen commented Jul 10, 2019

@mjcarroll I just did a rebase and it seemed the CI result looking good.

@mjcarroll mjcarroll merged commit 0bbebb7 into ros-perception:melodic Jul 10, 2019
@mjcarroll
Copy link
Contributor

Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants