Skip to content

Conversation

alsora
Copy link
Contributor

@alsora alsora commented Sep 27, 2021

The code in getPackageFromPluginXMLFilePath currently never exits from the while loop if the package.xml file is not found.
The reason is that the code assumes that parent_path() will eventually return an empty path, but this is not the case.
See source code for the function: https://github.com/ros2/rcpputils/blob/master/src/filesystem_helper.cpp#L172-L182

In the situation where the file is not found, the while loop will eventually reach the root of the filesystem and parent_path will continuously return the root itself.

This PR adds a check to make sure that we exit from the function if the path returned from parent_path is equal to the previous one.

Signed-off-by: Alberto Soragna alberto.soragna@gmail.com

… not found

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
@alsora
Copy link
Contributor Author

alsora commented Nov 11, 2021

@jacobperron @hidmic friendly ping

Copy link
Contributor

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

I think this was a bug introduced when switching to rcpputils post-Foxy.
In Foxy, pluginlib used it's own filesystem helper implementation:

while (true) {
if (pluginlib::impl::fs::exists(parent / "package.xml")) {
std::string package_file_path = (parent / "package.xml").string();
return extractPackageNameFromPackageXML(package_file_path);
}
// Recursive case - hop one folder up
parent = parent.parent_path();
// Base case - reached root and cannot find what we're looking for
if (parent.string().empty()) {
return "";
}

which does return empty:

path parent("");
for (auto it = this->cbegin(); it != --this->cend(); ++it) {
parent /= *it;
}
return parent;

So we don't have to backport this to Foxy, but we should for Galactic.

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@jacobperron jacobperron merged commit 840db44 into ros:ros2 Nov 13, 2021
@cottsay
Copy link
Member

cottsay commented Dec 3, 2022

@Mergifyio backport galactic

@mergify
Copy link

mergify bot commented Dec 3, 2022

backport galactic

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Dec 3, 2022
… not found (#220)

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
(cherry picked from commit 840db44)
cottsay pushed a commit that referenced this pull request Dec 3, 2022
… not found (#220) (#243)

(cherry picked from commit 840db44)

Co-authored-by: Alberto Soragna <alberto.soragna@gmail.com>
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
@alsora alsora deleted the asoragna/fix-infinite-loop branch March 29, 2023 10:39
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.

3 participants