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 the patching to work on Windows without admin. #11

Merged
merged 2 commits into from
Mar 18, 2021

Conversation

clalancette
Copy link
Contributor

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

Like we've done elsewhere, update google_benchmark_vendor to use git apply for applying patches. This allows it to work on Windows without administrator.

@cottsay Note that I did not change this to use git to fetch the sources as we did in uncrustify_vendor. It seemed to work without problems for me on all of Linux, Windows, and macOS without that. Nevertheless, I'd appreciate your insight here into whether I should change this to use GIT_REPOSITORY instead.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@cottsay
Copy link
Contributor

cottsay commented Feb 9, 2021

It seemed to work without problems for me on all of Linux, Windows, and macOS without that.

It only appears to be working because the patches aren't build-ciritical. The insidious nature of the problem that switching to a git upstream works around is that it is a completely silent failure - the only reason we noticed to begin with was that the patches that failed to apply caused a build break.

To repro, set --verbose on your git-apply operations, run git init . in your ROS workspace, and build the package again. You should see this:

$ git init .
Initialized empty Git repository in /opt/ros_src/rolling/.git/
$ colcon build --symlink --merge --install-base /opt/ros/rolling --cmake-args -DFORCE_BUILD_VENDOR_PKG=ON --packages-select google_benchmark_vendor --event-handler console_start_end- --cmake-clean-first --cmake-force-configure
--- stderr: google_benchmark_vendor                              
Skipped patch 'cmake/thread_safety_attributes.cpp'.
Skipped patch 'CMakeLists.txt'.
---

Summary: 1 package finished [2.18s]
  1 package had stderr output: google_benchmark_vendor

@clalancette
Copy link
Contributor Author

Thanks for the info. I'll give this a try a little later on (and switch to using GIT as appropriate).

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Copy link
Contributor Author

All right, well, I couldn't reproduce the problem locally. But given that we went through the same thing in uncrustify, I'm going to assume that we need to switch over to GIT to make this work properly. So I did that in e76349f.

@cottsay cottsay merged commit 6c51b9f into main Mar 18, 2021
@delete-merged-branch delete-merged-branch bot deleted the clalancette/use-git-apply branch March 18, 2021 18:11
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.

None yet

2 participants