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

update builder.py to add Windows support #982

Merged
merged 10 commits into from
Jan 28, 2019
Merged

update builder.py to add Windows support #982

merged 10 commits into from
Jan 28, 2019

Conversation

kejxu
Copy link
Contributor

@kejxu kejxu commented Jan 10, 2019

update builder.py to support building on Windows:

  • exclude drive letter so that path[1:] gives the correct result on Windows
  • abstract the action of writing env and setup scripts into script writers
  • define script templates in a separate step (for better readability)
  • add templates for Windows batch scripts
  • expand variables for env.bat template and setup.bat template

this code change does not alter the existing logic on Linux

johnsonshih and others added 9 commits January 9, 2019 15:44
* Skip the drive letter if run on Windows

* Remove unnecessary check for win32
* Update catkin_package.cmake

* Update catkin_package.cmake

* Update builder.py

* Normalize the path from subs['cmake_prefix_path']

* Revert the unrelated changes from my bad merge in the previous commit.

Revert the unrelated changes from my bad merge in the previous commit to keep this PR clean.
* Create env.bat and setup.bat in build_cmake_package for Windows (#9)

* define string templates in a separate step

* change script writer function names

* fix indentation for script template
@kejxu kejxu changed the title update builder.py to add cross-platform (Linux+Windows) support update builder.py to add Windows support Jan 10, 2019
@dirk-thomas
Copy link
Member

Thanks for the patch.

@dirk-thomas dirk-thomas merged commit b2f73e0 into ros:kinetic-devel Jan 28, 2019
@kejxu kejxu deleted the add_batch_script_template_and_writer_in_builder.py branch January 28, 2019 21:08
at-wat added a commit to at-wat/catkin that referenced this pull request Mar 18, 2019
Filenames setup.sh and env.sh go .sh after ros#982.
This seems caused test failures of pure cmake packages on the build farm.
at-wat added a commit to at-wat/catkin that referenced this pull request Mar 18, 2019
Filenames setup.sh and env.sh get .sh after ros#982.
This seems to cause test failures of pure cmake packages on the build farm.
at-wat added a commit to at-wat/catkin that referenced this pull request Mar 18, 2019
Filenames setup.sh and env.sh get .sh after ros#982.
This seems to cause test failures of pure cmake packages on the build farm.

Following builds on the build farm looks affected by this bug.

http://build.ros.org/view/Mdev/job/Mdev__yp-spur__ubuntu_bionic_amd64/18/console
```
02:29:23 Invoking '. /opt/ros/melodic/setup.sh && . /tmp/ws/devel_isolated/setup.sh && PYTHONIOENCODING=utf_8 PYTHONUNBUFFERED=1 catkin_make_isolated --force-cmake --cmake-args -DBUILD_TESTING=1 -DCATKIN_ENABLE_TESTING=1 -DCATKIN_SKIP_TESTING=0 -DCATKIN_TEST_RESULTS_DIR=/tmp/ws/test_results --catkin-make-args run_tests' in '/tmp/ws'
02:29:23 /bin/sh: 9: .: Can't open /tmp/ws/devel_isolated/ypspur/setup.sh
```
http://build.ros.org/view/Mdev/job/Mdev__rc_genicam_api__ubuntu_bionic_amd64/17/console
```
09:42:52 Invoking '. /opt/ros/melodic/setup.sh && . /tmp/ws/devel_isolated/setup.sh && PYTHONIOENCODING=utf_8 PYTHONUNBUFFERED=1 catkin_make_isolated --force-cmake --cmake-args -DBUILD_TESTING=1 -DCATKIN_ENABLE_TESTING=1 -DCATKIN_SKIP_TESTING=0 -DCATKIN_TEST_RESULTS_DIR=/tmp/ws/test_results --catkin-make-args run_tests' in '/tmp/ws'
09:42:52 /bin/sh: 4: .: Can't open /tmp/ws/devel_isolated/rc_genicam_api/setup.sh
```

---
Expression evaluation order of python:
```python
>>> 'a' + 'b' if True else 'c'
'ab'
>>> 'a' + 'b' if False else 'c'
'c'
```
at-wat added a commit to at-wat/catkin that referenced this pull request Mar 18, 2019
Filenames setup.sh and env.sh get .sh after ros#982.
This seems to cause test failures of pure cmake packages on the buildfarm.

Following builds on the buildfarm looks affected by this bug.

http://build.ros.org/view/Mdev/job/Mdev__yp-spur__ubuntu_bionic_amd64/18/console
```
02:29:23 Invoking '. /opt/ros/melodic/setup.sh && . /tmp/ws/devel_isolated/setup.sh && PYTHONIOENCODING=utf_8 PYTHONUNBUFFERED=1 catkin_make_isolated --force-cmake --cmake-args -DBUILD_TESTING=1 -DCATKIN_ENABLE_TESTING=1 -DCATKIN_SKIP_TESTING=0 -DCATKIN_TEST_RESULTS_DIR=/tmp/ws/test_results --catkin-make-args run_tests' in '/tmp/ws'
02:29:23 /bin/sh: 9: .: Can't open /tmp/ws/devel_isolated/ypspur/setup.sh
```
http://build.ros.org/view/Mdev/job/Mdev__rc_genicam_api__ubuntu_bionic_amd64/17/console
```
09:42:52 Invoking '. /opt/ros/melodic/setup.sh && . /tmp/ws/devel_isolated/setup.sh && PYTHONIOENCODING=utf_8 PYTHONUNBUFFERED=1 catkin_make_isolated --force-cmake --cmake-args -DBUILD_TESTING=1 -DCATKIN_ENABLE_TESTING=1 -DCATKIN_SKIP_TESTING=0 -DCATKIN_TEST_RESULTS_DIR=/tmp/ws/test_results --catkin-make-args run_tests' in '/tmp/ws'
09:42:52 /bin/sh: 4: .: Can't open /tmp/ws/devel_isolated/rc_genicam_api/setup.sh
```

---
Expression evaluation order of python:
```python
>>> 'a' + 'b' if True else 'c'
'ab'
>>> 'a' + 'b' if False else 'c'
'c'
```
@@ -554,13 +655,14 @@ def build_cmake_package(
make_install_cmd = [last_env] + make_install_cmd
run_command(make_install_cmd, build_dir, quiet)

# If we are installing, and a env.sh exists, don't overwrite it
if install and os.path.exists(prefix_destdir(os.path.join(install_target, 'env.sh'), destdir)):
env_script = 'env' + '.bat' if sys.platform == 'win32' else '.sh'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change broke non-Windows platforms: see #1000.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my apologies for the regression!

dirk-thomas pushed a commit that referenced this pull request Mar 18, 2019
Filenames setup.sh and env.sh get .sh after #982.
This seems to cause test failures of pure cmake packages on the buildfarm.

Following builds on the buildfarm looks affected by this bug.

http://build.ros.org/view/Mdev/job/Mdev__yp-spur__ubuntu_bionic_amd64/18/console
```
02:29:23 Invoking '. /opt/ros/melodic/setup.sh && . /tmp/ws/devel_isolated/setup.sh && PYTHONIOENCODING=utf_8 PYTHONUNBUFFERED=1 catkin_make_isolated --force-cmake --cmake-args -DBUILD_TESTING=1 -DCATKIN_ENABLE_TESTING=1 -DCATKIN_SKIP_TESTING=0 -DCATKIN_TEST_RESULTS_DIR=/tmp/ws/test_results --catkin-make-args run_tests' in '/tmp/ws'
02:29:23 /bin/sh: 9: .: Can't open /tmp/ws/devel_isolated/ypspur/setup.sh
```
http://build.ros.org/view/Mdev/job/Mdev__rc_genicam_api__ubuntu_bionic_amd64/17/console
```
09:42:52 Invoking '. /opt/ros/melodic/setup.sh && . /tmp/ws/devel_isolated/setup.sh && PYTHONIOENCODING=utf_8 PYTHONUNBUFFERED=1 catkin_make_isolated --force-cmake --cmake-args -DBUILD_TESTING=1 -DCATKIN_ENABLE_TESTING=1 -DCATKIN_SKIP_TESTING=0 -DCATKIN_TEST_RESULTS_DIR=/tmp/ws/test_results --catkin-make-args run_tests' in '/tmp/ws'
09:42:52 /bin/sh: 4: .: Can't open /tmp/ws/devel_isolated/rc_genicam_api/setup.sh
```

---
Expression evaluation order of python:
```python
>>> 'a' + 'b' if True else 'c'
'ab'
>>> 'a' + 'b' if False else 'c'
'c'
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants