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] Fix install destination path. #1332

Conversation

seanyen
Copy link
Contributor

@seanyen seanyen commented Feb 13, 2019

Follow the catkin guide to place output binaries correctly for cross-platform:

install(TARGETS your_library your_other_library
        ARCHIVE DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}
        LIBRARY DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}
        RUNTIME DESTINATION ${CATKIN_GLOBAL_BIN_DESTINATION})
install(TARGETS your_node
        RUNTIME DESTINATION ${CATKIN_PACKAGE_BIN_DESTINATION})

@rhaschke
Copy link
Contributor

Dear @seanyen-msft,
thanks a lot for your effort to bring rviz onto Windows. As you already filed several PRs along this direction (and I assume more will follow), I suggest to collect all those commits on a single development branch and have a single work-in-progress pull-request for it. If the porting is finished, we then can merge all related commits in a single action.
If you agree with this approach, may I ask you to re-organize your existing PRs in this regard?

@rhaschke
Copy link
Contributor

I'm not familiar with ROS on Windows. How do you install ROS on Window? Do you follow the approach desribed here?
In order to allow for continuous integration / validation of your Windows porting effort, I suggest to setup https://github.com/marketplace/appveyor.

@seanyen
Copy link
Contributor Author

seanyen commented Feb 13, 2019

@rhaschke Did you advice that you will create a staging branch that I can submit PRs toward to? Or I just re-organize everything I have into one single PR but still submitting to melodic-devel?

Please excuse me I haven't introduced myself. I am the developer who is working on this project https://aka.ms/ros (here is an earlier announcement). And we do have daily CI build already (https://ms-iot.github.io/ROSOnWindows/Build/buildfarm.html) and all the rviz related changes are under https://github.com/ms-iot/rviz/tree/init_windows. Our CI build is referring to a customized rosdistro, and for rviz, it has been redirected to use ms-iot\rviz as the upstream. Our short-term goal is to bring up ROS1 melodic to run natively on Windows, upstream the effort we made to the original repositories, and point our CI back to the official rosdistro. That's why I and @kejxu created so many PRs in many ROS repositories lately. :)

@rhaschke
Copy link
Contributor

Did you advice that you will create a staging branch that I can submit PRs toward to? Or I just re-organize everything I have into one single PR but still submitting to melodic-devel?

I am open to both options, whatever better fits you. The first option allows to accept intermediate commits and thus is slightly easier for the review process. I added a corresponding branch: wip-windows-port. Please tell, if you prefer the other option. In this case I will delete the branch again.

Thanks for all the links and introducing yourself and your project. I will have a look later into your CI approach.

@seanyen
Copy link
Contributor Author

seanyen commented Feb 13, 2019

@rhaschke Thanks for creating the wip-windows-port branch and please keep it. I will be re-sending the PRs to that branch soon.

@seanyen
Copy link
Contributor Author

seanyen commented Feb 13, 2019

Closed it since it is moved to #1340

@seanyen seanyen closed this Feb 13, 2019
@seanyen
Copy link
Contributor Author

seanyen commented Feb 13, 2019

@rhaschke I resent all the existing PRs to the wip-windows-port branch. There will be more and will be coming soon. I tried to keep each PR independent enough to only address one issue. Hope that is easier for review.

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.

2 participants