-
Notifications
You must be signed in to change notification settings - Fork 280
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
gmock issue building realtime_tools on arch linux #1030
Comments
Same issue on debian 10 buster This is when building Manually installing I will try applying #1022 later |
Applying #1022 solved the issue for me |
@GladOSkar which package from EDIT: Another thought is that the #1022 patch fixes the issue on debian, but not arch? If the magic is adding |
@jwhendy it was Yeah I saw that #1022 didn't fix it for you, weird. An unusual thing I did is that I applied each patch line manually (didn't use his file or the patch command) because I feared conflict since their patch is for kinetic. From how I understood their PR I don't think the issue is with /usr/include/gmock, but rather catkin's gmock detection in the cmake file is faulty for newer versions of gmock. Not entirely sure though. |
Any pointers on how best to do this? This repo stops at If I just clone @sloretz 's fork using the
|
Also, do you actually have these directories, or are they really variables that get resolved?
I don't have any |
From looking at those directories, here is arch's layout:
|
Hmmm, that's super interesting.
Can you post the same, as well as something like (just a few, you don't need all of these)
I want to understand why those files are missing for me, but not you. |
Oh, whoops. Bah. I'm collaborating with an Oskar on arch packages and thought you were him. Okay, I think this is a Debian vs. Arch thing, then. I definitely don't have a full src folder like that for either of these packages. |
Haha no worries :) Good luck finding a solution! |
Thanks for the post of the directories. I think that's a great start and enabled me to create an arch bug report. I'll post back when we resolve. I'm not sure where to find a de facto directory structure of what should/should not exist and where... |
@jwhendy Any update on this? Btw the referenced PRs have been merged and released in the meantime so you might want to give it another try on Arch. |
@dirk-thomas I haven't done anything since I didn't hear any news here or on the Arch bug.
Are there others besides #1022? In my first post, I note that I built directly from sloretz's fork to try #1022 directly. I can certainly try again later today. From my last comment:
This would be super helpful. I couldn't figure out from the code what, specifically, the cmake check is not finding. Any coaching here? The posts of directory structure from @GladOSkar were potentially promising, as they show Arch doesn't have the same If I can nail down what As a final aside, I remembered that I did this without success.
Is there a reason this is only proposed for |
I was think of #1021 which was also an issue on Buster. Though it might not be relevant here.
You can either look at the directory structure on Ubuntu or use the paths which are being checked in the CMake code.
This corner case has never been necessary. It is extremely unlikely now that gtest and gmock are bundled together that you only have gtest but not gmock and want to provide gmock from source within the workspace. |
I'll do a clean build from master tonight. Thanks!
Indeed, above I tried to replicate the Ubuntu filesystem screenshot posted by @GladOSkar without success. I've also looked at the code and it looks like the find script is looking for Is anyone able to explain what's going wrong in plain english? So far, we haven't pinpointed what's actually going wrong. Based on the information I've provided, what looks awry? Or, what information would you need to suggest what's going wrong? |
I just re-built and installed
I get the same build error with
And here is my attempt to understand why this fails. Here's where it looks:
That calls this:
Thus, we look for two files from what I can tell:
At face value, do we have those files?
Ok, edit
What do we get?
So, it looks like it's looking or
Ah! Interesting. No error. I didn't expect that, and was pretty sure I did this up above. That said, printing out the includes from that function did leave me puzzled. Note lines 258-259:
In my
|
These are the exact paths used by the Debian package
Maybe that was necessary for versions of googletest which ship only the sources? If you want to remove something like this it would need to be thoroughly tested that it doesn't break any use case (on Ubuntu, on Debian, different version of googletest, from-source, etc.)
Please use references to exact lines of code otherwise I am unable to follow your argument.
Please feel free to propose any kind of PR. Only when seeing the actual proposed diff I can give you concrete feedback. [As mentioned above please make sure to do thorough testing of any changes - often it is hard to justify the necessary testing effort for "just cleaning up something".] |
Well, something is awry, as even after fixing the
Maybe, but I think it could be an accident based on findings below.
I thought I did? The bullet you're commenting on was a closing summary based on the code and output I'd already laid out, including both linked lines and copy/pasted chunks. To summarize again: my observation was that printing out the I did make a potential breakthrough while writing this. Here was an experiment on this function just to make sure the passed path was really what I thought:
I then added output to the function called:
Output:
How is I tracked it down to this:
Removing the I now get this as the only relevant build output:
Now that I had a culprit, it explains the behavior I commented on above as well. By default, these are the paths passed to
As observed in a previous comment,
Thoughts based on this? For the
For the I'd propose either removing it or using Aside: is there a list of distros you ROS requires testing on before merging a PR? You stressed testing a couple times above, but in this case #1022 looks to have complicated things. In the end, it was probably that arch doesn't have I plan to hunt down the full story of what happened since 0.7.18 and now, then can post back with PR proposals. |
Since |
It looks like he did, and then you merged it. I don't see any discussion of testing at all, nor does it look to refer to any issues so I'm not sure what specific issues prompted this. I'm digging through internals more and am a bit out of my league to know what, exactly, needs fixing. I undid all my system changes, so I am back to the default file locations for gmock and gtest on arch linux. My intuition was to change
... to this:
I'm not sure what the desired outcome was, but Doing some output testing by modifying the code here:
Now, when I fix that to point at
Starting with the first, it's this line. Printing out the directories:
I would not expect
Adding some messages yields:
So, we're back to the filesystem woes:
So, I think one could make the case that arch should align these two packages to have the same I still think something is off, though. If I change this line to this:
That error still exists, even though that's where the files are. I then copied
That's where I'll leave it for now. I feel extremely inefficient in tracking this down and there appears to be at least a couple independent sets of paths maintained depending on what goes hunting for |
Both code snippets are producing the very same result in CMake. Your code change is a no op. The variable was before and is afterwards a list with two paths. |
@jwhendy I'm unable to reproduce using this Dockerfile, but I'm not familiar with arch so I probably I made a mistake. Mind showing me what the correct steps are? FROM archlinux/base
RUN pacman -Sy --noconfirm base-devel
# Add a user to build stuff
RUN useradd ros --create-home
# give user passwordless sudo access
RUN echo "ros ALL=(ALL) NOPASSWD: ALL" >> /etc/sudoers
# Get yay
RUN pacman -Sy --noconfirm git go \
&& cd /tmp \
&& sudo -u ros git clone https://aur.archlinux.org/yay.git \
&& cd yay \
&& sudo -u ros makepkg -si --noconfirm
# Install ros-melodic-catkin with modifications to use 0.7.19
RUN sudo -u ros yay --useask -S --noconfirm gmock gtest python python-catkin_pkg python-empy python-nose ros-build-tools-py3 cmake \
&& cd /tmp \
&& sudo -u ros git clone https://github.com/ros-melodic-arch/ros-melodic-catkin.git \
&& cd ros-melodic-catkin \
&& sed -i.bak 's/0.7.18/0.7.19/g' .SRCINFO \
&& sed -i.bak 's/0.7.18/0.7.19/g' PKGBUILD \
&& sed -i.bak 's/b6a7428944911a011b0c3b0e465f2db04d219ce5f72cf3e2f76194dd055f1f49/b83d66640df99f72bc37160e8b60a76df6c87ff8dcbb9ab096911c44f08d13e1/' .SRCINFO \
&& sed -i.bak 's/b6a7428944911a011b0c3b0e465f2db04d219ce5f72cf3e2f76194dd055f1f49/b83d66640df99f72bc37160e8b60a76df6c87ff8dcbb9ab096911c44f08d13e1/' PKGBUILD \
&& sudo -u ros makepkg -si --noconfirm
# Try to build realtime tools
RUN sudo -u ros yay --useask -S --noconfirm ros-build-tools-py3 ros-melodic-realtime-tools I guess this being at the end of the output means it succeeded?
The package seems to exist
|
@dirk-thomas follow-up on the src warnings. As mentioned above, this line in the default gmock
With this default, I get:
I didn't know what to make this, so I guessed
I'm guessing the proposal would be to take this up with googletest and arch? Edit: for clarification, arch doesn't ship with a |
I don't understand all of the gtest/gmock logic, but I had a look through it. Hopefully this is useful. IIUC catkin first tries to decide whether to #1022 is supposed to just refactor the logic that makes the decision (catkin was choosing to use GTest from the find module when it should have built gtest/gmock from source), but as reported by @mikepurvis it seems to have broken the code that builds gtest/gmock on Xenial. #1040 doesn't touch the decision logic, but instead fixes the finding of gtest/gmock sources on Jessie/Xenial. Neither #1022 or #1040 address Arch (this ticket). I don't know if it's the decision logic or the code that finds gtest/gmock sources that needs to be fixed for Arch, but I suspect it's the latter based on @jwhendy's note:
If it got that far then there weren't find modules for both GTest and GMock, so it tried to build from source. The logic to build gtest/gmock assumes there is a IIUC IIUC the locations gtest CMakeList.txt:
gmock CMakeList.txt: |
Actually, it looks even more complicated. It looks like |
Reopening since I don't think #1040 is expected to resolve the issue on Arch. |
Thanks for the analysis @sloretz ! I think at this point, the Thus far:
For arch's files,
I tried this file from Ubuntu Trusty and no longer get the errors. It points to a |
Hi, here is a PKGBUILD for the googletest that I have used to install both gtest and gmock version 1.8.1, which hopefully provides all the necessary files in order to be found by catkin. The realtime-tools then compile without problems. There is just small problem that there is no VERSION in the CMakeLists and catkin is complaining. |
Should be resolved with the merge of this PR: anthraxx/arch-pkgbuilds#33. Since no change needs to be made in this codebase, feel free to close this issue. |
Sounds good to me. Please feel free to comment on the closed ticket and it can be reopened if necessary. |
Did some more thorough testing and even though catkin now builds on Arch with the above linked PR, realtime-tools still does not. Sorry for the haste, request to reopen this issue. |
@acxz any notable error messages you could post? |
The error message is the exact same as in the OP. When I install catkin it identifies gmock and gtest but when I want to use those catkin cmake functions it chokes. Sadly I don’t have time to spend on this further. One thing I want to try once I do get some time or if someone wants to PR, is change how gtest/gmock is installed on Arch to exactly match the ubuntu package hierarchy. If that doesn’t fix it, I’ll admit defeat. |
Ok, just trying to understand and I can look into this later this week/weekend. For reference, can you?
|
|
I recently encountered the same problem on Manjaro. I manually compiled ros-melodic-realtime-tools and found that the problem was GMOCK_MAIN_LIBRARIES . CATKIN_ENABLE_TESTING is "ON" while GMOCK_MAIN_LIBRARIES was empty. Then I checked the CMake Module and found that FindGTest is already shipped with CMake but not FindGMock. Here is my solution:
Hope it would help. |
@liborw you mentioned that your PKGBUILD solves building realtime-tools, but I tested it out and it did not work for me. |
@jiaaseon mentioned something important that the GMOCK_MAIN_LIBRARIES variable is empty. Based on that advice I am patching the AUR package for realtime-tools to not test if that variable is empty. ros-melodic-arch/ros-melodic-realtime-tools#5 Also submitted a PR to realtime-tools with the patch: ros-controls/realtime_tools#55 |
Finally got around to resolving this. Submitted a patch to the Arch Linux repos to get gtest updated to 1.10.0 and restructure the installed files based on upstream recommendations. Just need to add one more path for the googletest sources see above linked PR in catkin's gtest.cmake and we good to go. |
I'm seeing this on Fedora 32 with ROS Noetic as well. Which fix works?
|
I'm trying to build ros-melodic-realtime-tools from the arch linux AUR. The PKGBUILD shows how the make commands are called.
I get the following error:
I made a slight edit to a function to see where it was looking.
This prints out as:
I do have gmock/gtest installed, and the dirs are in
/usr/include
. For the heck of it, I tried symlinking all*.h
files into/usr/include
directly, but that didn't work out.I think this is related to #933 and #970. I also saw #1022 and adapted ros-melodic-catkin to build from @sloretz 's fork, but this did not solve my issue. I get the same original error about gmock not being found.
Any other details that would be helpful, or debug output I could add to assist with this?
The text was updated successfully, but these errors were encountered: