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

Fix usage of relative paths with environment variables #2890

Merged
merged 1 commit into from
Nov 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions examples/plugins/actor_collisions/actor_collisions.world
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@
"/>
</plugin>
<skin>
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.0</scale>
</skin>
<animation name="walking">
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.000000</scale>
<interpolate_x>true</interpolate_x>
</animation>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,11 @@
"/>
</plugin>
<skin>
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.0</scale>
</skin>
<animation name="walking">
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.000000</scale>
<interpolate_x>true</interpolate_x>
</animation>
Expand Down
8 changes: 7 additions & 1 deletion gazebo/common/CommonIface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,13 @@ std::string common::asFullPath(const std::string &_uri,
#endif

// Use platform-specific separator
return ignition::common::joinPaths(path, uri);
auto result = ignition::common::joinPaths(path, uri);

// If result doesn't exist, return it unchanged
if (!exists(result))
return _uri;

return result;
}

//////////////////////////////////////////////////
Expand Down
1 change: 1 addition & 0 deletions gazebo/common/CommonIface.hh
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ namespace gazebo
/// If the URI is already a full path or contains a scheme, it won't be
/// modified.
/// If the URI is a relative path, the file path will be prepended.
/// If the URI and path combination doesn't exist, the URI won't be changed.
/// \param[in] _uri URI, which can have a scheme, or be full or relative
/// paths.
/// \param[in] _filePath The path to a file in disk.
Expand Down
99 changes: 34 additions & 65 deletions gazebo/common/CommonIface_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <ignition/common/Filesystem.hh>
#include <sdf/parser.hh>

#include "test_config.h"
#include "gazebo/common/CommonIface.hh"
#include "gazebo/common/SystemPaths.hh"
#include "test/util.hh"
Expand Down Expand Up @@ -259,93 +260,58 @@ TEST_F(CommonIface_TEST, directoryOps)
/////////////////////////////////////////////////
TEST_F(CommonIface_TEST, asFullPath)
{
const std::string relativeUriUnix{"meshes/collision.dae"};
const std::string relativeUriWindows{"meshes\\collision.dae"};
const std::string absoluteUriUnix{"/path/to/collision.dae"};
const std::string absoluteUriWindows{"C:\\path\\to\\collision.dae"};
const std::string schemeUri{"https://website.com/collision.dae"};
auto src = std::string(PROJECT_SOURCE_PATH);

const auto relativeUri = ignition::common::joinPaths("data", "box.dae");
const auto absoluteUri = ignition::common::joinPaths(src, "test", "data",
"box.dae");
const auto schemeUri{"https://website.com/collision.dae"};

// Empty path
{
const std::string path{""};

EXPECT_EQ(relativeUriUnix, common::asFullPath(relativeUriUnix, path));
EXPECT_EQ(relativeUriWindows, common::asFullPath(relativeUriWindows, path));
EXPECT_EQ(absoluteUriUnix, common::asFullPath(absoluteUriUnix, path));
EXPECT_EQ(absoluteUriWindows, common::asFullPath(absoluteUriWindows, path));
EXPECT_EQ(relativeUri, common::asFullPath(relativeUri, path));
EXPECT_EQ(absoluteUri, common::asFullPath(absoluteUri, path));
EXPECT_EQ(schemeUri, common::asFullPath(schemeUri, path));
}

// Data string
{
const std::string path{"data-string"};

EXPECT_EQ(relativeUriUnix, common::asFullPath(relativeUriUnix, path));
EXPECT_EQ(relativeUriWindows, common::asFullPath(relativeUriWindows, path));
EXPECT_EQ(absoluteUriUnix, common::asFullPath(absoluteUriUnix, path));
EXPECT_EQ(absoluteUriWindows, common::asFullPath(absoluteUriWindows, path));
EXPECT_EQ(relativeUri, common::asFullPath(relativeUri, path));
EXPECT_EQ(absoluteUri, common::asFullPath(absoluteUri, path));
EXPECT_EQ(schemeUri, common::asFullPath(schemeUri, path));
}

#ifdef _WIN32
{
// Absolute Windows path
const std::string path{"C:\\abs\\path\\file"};
// Absolute path
const auto path = ignition::common::joinPaths(src, "test", "path");

// Directory
EXPECT_EQ("C:\\abs\\path\\meshes\\collision.dae",
common::asFullPath(relativeUriUnix, path));
EXPECT_EQ("C:\\abs\\path\\meshes\\collision.dae",
common::asFullPath(relativeUriWindows, path));
// TODO(anyone) Support absolute Unix-style URIs on Windows
EXPECT_EQ(absoluteUriWindows, common::asFullPath(absoluteUriWindows, path));
EXPECT_EQ(ignition::common::joinPaths(src, "test", "data", "box.dae"),
common::asFullPath(relativeUri, path));
EXPECT_EQ(absoluteUri, common::asFullPath(absoluteUri, path));
EXPECT_EQ(schemeUri, common::asFullPath(schemeUri, path));

// File
auto filePath = ignition::common::joinPaths(path, "file.sdf");

EXPECT_EQ("C:\\abs\\path\\file\\meshes\\collision.dae",
common::asFullPath(relativeUriUnix, filePath));
EXPECT_EQ("C:\\abs\\path\\file\\meshes\\collision.dae",
common::asFullPath(relativeUriWindows, filePath));
// TODO(anyone) Support absolute Unix-style URIs on Windows
EXPECT_EQ(absoluteUriWindows, common::asFullPath(absoluteUriWindows,
filePath));
const auto filePath = ignition::common::joinPaths(src, "test", "file.sdf");
EXPECT_EQ(ignition::common::joinPaths(src, "test", "data", "box.dae"),
common::asFullPath(relativeUri, filePath));
EXPECT_EQ(absoluteUri, common::asFullPath(absoluteUri, filePath));
EXPECT_EQ(schemeUri, common::asFullPath(schemeUri, filePath));
}
#else
{
// Absolute Unix path
const std::string path{"/abs/path/file"};

// Directory
EXPECT_EQ("/abs/path/meshes/collision.dae",
common::asFullPath(relativeUriUnix, path));
EXPECT_EQ("/abs/path/meshes/collision.dae",
common::asFullPath(relativeUriWindows, path));
EXPECT_EQ(absoluteUriUnix, common::asFullPath(absoluteUriUnix, path));
// TODO(anyone) Support absolute Windows paths on Unix
EXPECT_EQ(schemeUri, common::asFullPath(schemeUri, path));

// File
auto filePath = ignition::common::joinPaths(path, "file.sdf");

EXPECT_EQ("/abs/path/file/meshes/collision.dae",
common::asFullPath(relativeUriUnix, filePath));
EXPECT_EQ("/abs/path/file/meshes/collision.dae",
common::asFullPath(relativeUriWindows, filePath));
EXPECT_EQ(absoluteUriUnix, common::asFullPath(absoluteUriUnix, filePath));
// TODO(anyone) Support absolute Windows paths on Unix
EXPECT_EQ(schemeUri, common::asFullPath(schemeUri, filePath));
}
#endif
}

/////////////////////////////////////////////////
TEST_F(CommonIface_TEST, fullPathsMesh)
{
std::string filePath{"/tmp/model.sdf"};
std::string relativePath{"meshes/collision.dae"};
auto path = ignition::common::joinPaths(std::string(PROJECT_SOURCE_PATH),
"test");

std::string filePath = ignition::common::joinPaths(path, "model.sdf");
std::string relativePath = ignition::common::joinPaths("data", "box.dae");

std::stringstream ss;
ss << "<sdf version='" << SDF_VERSION << "'>"
Expand Down Expand Up @@ -419,7 +385,7 @@ TEST_F(CommonIface_TEST, fullPathsMesh)
auto uriElem = meshElem->GetElement("uri");
ASSERT_NE(nullptr, uriElem);

EXPECT_EQ(ignition::common::joinPaths("/tmp", relativePath),
EXPECT_EQ(ignition::common::joinPaths(path, relativePath),
uriElem->Get<std::string>());
}

Expand Down Expand Up @@ -447,16 +413,19 @@ TEST_F(CommonIface_TEST, fullPathsMesh)
auto uriElem = meshElem->GetElement("uri");
ASSERT_NE(nullptr, uriElem);

EXPECT_EQ(ignition::common::joinPaths("/tmp", relativePath),
EXPECT_EQ(ignition::common::joinPaths(path, relativePath),
uriElem->Get<std::string>());
}
}

/////////////////////////////////////////////////
TEST_F(CommonIface_TEST, fullPathsActor)
{
std::string filePath{"/tmp/actor.sdf"};
std::string relativePath{"meshes/walk.dae"};
auto path = ignition::common::joinPaths(std::string(PROJECT_SOURCE_PATH),
"test");

std::string filePath = ignition::common::joinPaths(path, "model.sdf");
std::string relativePath = ignition::common::joinPaths("data", "box.dae");

std::stringstream ss;
ss << "<sdf version='" << SDF_VERSION << "'>"
Expand Down Expand Up @@ -499,14 +468,14 @@ TEST_F(CommonIface_TEST, fullPathsActor)
ASSERT_NE(nullptr, skinElem);
skinFilenameElem = skinElem->GetElement("filename");
ASSERT_NE(nullptr, skinFilenameElem);
EXPECT_EQ(ignition::common::joinPaths("/tmp", relativePath),
EXPECT_EQ(ignition::common::joinPaths(path, relativePath),
skinFilenameElem->Get<std::string>());

animationElem = actorElem->GetElement("animation");
ASSERT_NE(nullptr, animationElem);
animationFilenameElem = animationElem->GetElement("filename");
ASSERT_NE(nullptr, animationFilenameElem);
EXPECT_EQ(ignition::common::joinPaths("/tmp", relativePath),
EXPECT_EQ(ignition::common::joinPaths(path, relativePath),
animationFilenameElem->Get<std::string>());
}

Expand Down
4 changes: 2 additions & 2 deletions test/worlds/actor_collisions.world
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@

<actor name="actor">
<skin>
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.0</scale>
</skin>
<animation name="walking">
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.000000</scale>
<interpolate_x>true</interpolate_x>
</animation>
Expand Down
4 changes: 2 additions & 2 deletions worlds/actor.world
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@
</include>
<actor name="actor">
<skin>
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.0</scale>
</skin>
<pose>0 0 0 0 0 0</pose>
<animation name="walking">
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.000000</scale>
<interpolate_x>true</interpolate_x>
</animation>
Expand Down
2 changes: 1 addition & 1 deletion worlds/actor_bvh.world
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<actor name="actor">
<pose>2 2 2 0 0 0</pose>
<skin>
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
</skin>
<animation name="anim">
<filename>cmu-13_26.bvh</filename>
Expand Down
8 changes: 4 additions & 4 deletions worlds/cafe.world
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@
<actor name="actor1">
<pose>0 1 1.25 0 0 0</pose>
<skin>
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.0</scale>
</skin>
<animation name="walking">
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.000000</scale>
<interpolate_x>true</interpolate_x>
</animation>
Expand All @@ -106,11 +106,11 @@
<actor name="actor2">
<pose>-2 -2 1.25 0 0 0</pose>
<skin>
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.0</scale>
</skin>
<animation name="walking">
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.000000</scale>
<interpolate_x>true</interpolate_x>
</animation>
Expand Down
4 changes: 2 additions & 2 deletions worlds/profiler.world
Original file line number Diff line number Diff line change
Expand Up @@ -341,11 +341,11 @@
<actor name="actor1">
<pose>0 5 1.25 0 0 0</pose>
<skin>
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.0</scale>
</skin>
<animation name="walking">
<filename>walk.dae</filename>
<filename>file://media/models/walk.dae</filename>
<scale>1.000000</scale>
<interpolate_x>true</interpolate_x>
</animation>
Expand Down