diff --git a/.github/ci-bionic/after_make.sh b/.github/ci-bionic/after_make.sh deleted file mode 100644 index 5ea56eb8..00000000 --- a/.github/ci-bionic/after_make.sh +++ /dev/null @@ -1,13 +0,0 @@ -#!/bin/sh -l - -set -x - -# Install -make install - -# Compile examples -cd ../examples -mkdir build -cd build -cmake .. -make diff --git a/.github/ci-bionic/dependencies.yaml b/.github/ci-bionic/dependencies.yaml deleted file mode 100644 index 0471a9ff..00000000 --- a/.github/ci-bionic/dependencies.yaml +++ /dev/null @@ -1,5 +0,0 @@ -repositories: - ign-msgs: - type: git - url: https://github.com/ignitionrobotics/ign-msgs - version: master diff --git a/.github/ci/after_make.sh b/.github/ci/after_make.sh new file mode 100644 index 00000000..b740068d --- /dev/null +++ b/.github/ci/after_make.sh @@ -0,0 +1,18 @@ +#!/bin/sh -l + +set -x + +# Install +make install + +# So ign-tools finds fuel-tools +export IGN_CONFIG_PATH=/usr/local/share/ignition +export LD_LIBRARY_PATH=/usr/local/lib/:$LD_LIBRARY_PATH + +# Compile example +cd ../example +mkdir build +cd build +cmake .. +make +cd ../../build diff --git a/.github/ci/packages.apt b/.github/ci/packages.apt new file mode 100644 index 00000000..241e94ec --- /dev/null +++ b/.github/ci/packages.apt @@ -0,0 +1,12 @@ +curl +libcurl4-openssl-dev +libgflags-dev +libignition-cmake2-dev +libignition-common3-dev +libignition-math6-dev +libignition-msgs6-dev +libignition-tools-dev +libjsoncpp-dev +libtinyxml2-dev +libyaml-dev +libzip-dev diff --git a/.github/workflows/ci-bionic.yml b/.github/workflows/ci-bionic.yml deleted file mode 100644 index 1e188cf4..00000000 --- a/.github/workflows/ci-bionic.yml +++ /dev/null @@ -1,36 +0,0 @@ -name: Ubuntu Bionic CI - -on: [push, pull_request] - -jobs: - bionic-ci: - runs-on: ubuntu-latest - name: Ubuntu Bionic CI - steps: - - name: Checkout - uses: actions/checkout@v2 - - name: Bionic CI - id: ci - uses: ignition-tooling/ubuntu-bionic-ci-action@master - with: - apt-dependencies: | - pkg-config - libcurl4-openssl-dev - libjsoncpp-dev - libzip-dev - libgflags-dev - libtinyxml2-dev - lcov - curl - libyaml-dev - libignition-cmake2-dev - libignition-common3-dev - libignition-math6-dev - libprotobuf-dev - protobuf-compiler - libprotoc-dev - ruby - libtinyxml2-dev - libignition-tools-dev - codecov-token: ${{ secrets.CODECOV_TOKEN }} - script-after-make: ../.github/ci-bionic/after_make.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..5d9be4dd --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,27 @@ +name: Ubuntu CI + +on: [push] + +jobs: + bionic-ci: + runs-on: ubuntu-latest + name: Ubuntu Bionic CI + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Compile and test + id: ci + uses: ignition-tooling/action-ignition-ci@master + with: + codecov-token: ${{ secrets.CODECOV_TOKEN }} + focal-ci: + runs-on: ubuntu-latest + name: Ubuntu Focal CI + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Compile and test + id: ci + uses: ignition-tooling/action-ignition-ci@focal + with: + codecov-token: ${{ secrets.CODECOV_TOKEN }} diff --git a/Changelog.md b/Changelog.md index abe0d448..62b6eb79 100644 --- a/Changelog.md +++ b/Changelog.md @@ -39,6 +39,23 @@ ### Ignition Fuel Tools 3.x.x (20xx-xx-xx) +### Ignition Fuel Tools 3.3.0 (2020-07-29) + +1. openrobotics to OpenRobotics + * [Pull request 75](https://github.com/ignitionrobotics/ign-fuel-tools/pull/75) + +1. Fix world tests + * [Pull request 76](https://github.com/ignitionrobotics/ign-fuel-tools/pull/76) + +1. Add missing dependency in Actions CI + * [Pull request 86](https://github.com/ignitionrobotics/ign-fuel-tools/pull/86) + +1. Backport file fetching + * [Pull request 84](https://github.com/ignitionrobotics/ign-fuel-tools/pull/84) + +1. Convert model:// to Fuel URLs instead of absolute paths + * [Pull request 85](https://github.com/ignitionrobotics/ign-fuel-tools/pull/85) + ### Ignition Fuel Tools 3.2.2 (2020-05-18) 1. Fix URL encodings in RestClient. diff --git a/src/FuelClient_TEST.cc b/src/FuelClient_TEST.cc index 783013ce..3bb52ebd 100644 --- a/src/FuelClient_TEST.cc +++ b/src/FuelClient_TEST.cc @@ -473,10 +473,10 @@ TEST_F(FuelClientTest, DownloadModel) "/test_cache/fuel.ignitionrobotics.org/iche033/models/Rescue Randy/2"); EXPECT_TRUE(common::exists( "test_cache/fuel.ignitionrobotics.org/iche033/models/Rescue Randy/2")); - const std::string model_sdf_path = + const std::string modelSdfPath = "test_cache/fuel.ignitionrobotics.org/iche033/models/Rescue Randy/2/" "model.sdf"; - EXPECT_TRUE(common::exists(model_sdf_path)); + EXPECT_TRUE(common::exists(modelSdfPath)); EXPECT_TRUE(common::exists( "test_cache/fuel.ignitionrobotics.org/iche033/models/Rescue Randy/2/" "model.config")); @@ -494,14 +494,21 @@ TEST_F(FuelClientTest, DownloadModel) "/test_cache/fuel.ignitionrobotics.org/iche033/models/Rescue Randy/2", cachedPath); - // Check that pbr paths have been updated. - std::ifstream ifs(model_sdf_path); - std::string model_sdf((std::istreambuf_iterator(ifs)), + // Check that URIs have been updated. + std::ifstream ifs(modelSdfPath); + std::string modelSdf((std::istreambuf_iterator(ifs)), std::istreambuf_iterator()); - EXPECT_EQ(std::string::npos, model_sdf.find("model://")); - EXPECT_EQ(std::string::npos, model_sdf.find("model://")); - EXPECT_EQ(std::string::npos, model_sdf.find("model://")); - EXPECT_EQ(std::string::npos, model_sdf.find("model://")); + EXPECT_EQ(std::string::npos, modelSdf.find("model://")); + EXPECT_EQ(std::string::npos, modelSdf.find("model://")); + EXPECT_EQ(std::string::npos, modelSdf.find("model://")); + EXPECT_EQ(std::string::npos, modelSdf.find("model://")); + EXPECT_EQ(std::string::npos, modelSdf.find("model://")); + + EXPECT_NE(std::string::npos, modelSdf.find("https://")); + EXPECT_NE(std::string::npos, modelSdf.find("https://")); + EXPECT_NE(std::string::npos, modelSdf.find("https://")); + EXPECT_NE(std::string::npos, modelSdf.find("https://")); + EXPECT_NE(std::string::npos, modelSdf.find("https://")); } // Try using nonexistent URL diff --git a/src/Interface_TEST.cc b/src/Interface_TEST.cc index 62bef2bc..c0623119 100644 --- a/src/Interface_TEST.cc +++ b/src/Interface_TEST.cc @@ -166,7 +166,7 @@ TEST(Interface, FetchResources) { // Check it's not cached common::URI worldUrl{ - "https://fuel.ignitionrobotics.org/1.0/nate/worlds/Empty"}; + "https://fuel.ignitionrobotics.org/1.0/openrobotics/worlds/Test world"}; { Result res = client.CachedWorld(worldUrl, cachedPath); EXPECT_FALSE(res) << "Cached Path: " << cachedPath; @@ -177,13 +177,12 @@ TEST(Interface, FetchResources) std::string path = fetchResourceWithClient(worldUrl.Str(), client); // Check it was downloaded to `1` - EXPECT_EQ(path, common::cwd() + - "/test_cache/fuel.ignitionrobotics.org/nate/worlds/Empty/1"); - EXPECT_TRUE(common::exists( - "test_cache/fuel.ignitionrobotics.org/nate/worlds/Empty/1")); - EXPECT_TRUE(common::exists( - "test_cache/fuel.ignitionrobotics.org/nate/worlds/Empty/1/" - "empty.world")); + EXPECT_EQ(path, common::cwd() + "/test_cache/fuel.ignitionrobotics.org/" + "openrobotics/worlds/Test world/1"); + EXPECT_TRUE(common::exists("test_cache/fuel.ignitionrobotics.org/" + "openrobotics/worlds/Test world/1")); + EXPECT_TRUE(common::exists("test_cache/fuel.ignitionrobotics.org/" + "openrobotics/worlds/Test world/1/test.world")); // Check it is cached { @@ -191,8 +190,8 @@ TEST(Interface, FetchResources) EXPECT_TRUE(res); EXPECT_EQ(Result(ResultType::FETCH_ALREADY_EXISTS), res); EXPECT_EQ(common::cwd() + - "/test_cache/fuel.ignitionrobotics.org/nate/worlds/Empty/1", - cachedPath); + "/test_cache/fuel.ignitionrobotics.org/openrobotics/worlds/" + "Test world/1", cachedPath); } } diff --git a/src/LocalCache.cc b/src/LocalCache.cc index 61c18bcc..0b5f54d2 100644 --- a/src/LocalCache.cc +++ b/src/LocalCache.cc @@ -57,34 +57,36 @@ class ignition::fuel_tools::LocalCachePrivate /// \brief return all models in a given Owner/models directory public: std::vector ModelsInPath(const std::string &_path); - /// \brief Associate model:// URI paths with paths on disk + /// \brief Associate model:// URI paths with Fuel URLs. /// \param[in] _modelVersionedDir Directory containing the model. + /// \param[in] _id Model's Fuel URL. /// \return True if the paths were fixed. False could occur if the /// `model.config` file is not present or contains XML errors. - public: bool FixPaths(const std::string &_modelVersionedDir); + public: bool FixPaths(const std::string &_modelVersionedDir, + const ModelIdentifier &_id); /// \brief Helper function to fix model:// URI paths in geometry elements. /// \param[in] _geomElem Pointer to the geometry element. - /// \param[in] _modelVersionedDir Directory containing the model. + /// \param[in] _id Model identifier /// \sa FixPaths public: void FixPathsInGeomElement(tinyxml2::XMLElement *_geomElem, - const std::string &_modelVersionedDir); + const ModelIdentifier &_id); /// \brief Helper function to fix model:// URI paths in material elements. /// \param[in] _matElem Pointer to the material element. - /// \param[in] _modelVersionedDir Directory containing the model. + /// \param[in] _id Model identifier /// \sa FixPaths public: void FixPathsInMaterialElement( tinyxml2::XMLElement *_matElem, - const std::string &_modelVersionedDir); + const ModelIdentifier &_id); /// \brief Helper function to fix a single model:// URI that is contained /// in an element. /// \param[in] _elem Pointer to an element tha contains a URI. - /// \param[in] _modelVersionedDir Directory containing the model. + /// \param[in] _id Model identifier /// \sa FixPaths public: void FixPathsInUri(tinyxml2::XMLElement *_elem, - const std::string &_modelVersionedDir); + const ModelIdentifier &_id); /// \brief client configuration public: const ClientConfig *config = nullptr; @@ -413,8 +415,8 @@ bool LocalCache::SaveModel( return false; } - // Convert model:// URIs to locations on disk. - this->dataPtr->FixPaths(modelVersionedDir); + // Convert model:// URIs to Fuel URLs + this->dataPtr->FixPaths(modelVersionedDir, _id); // Cleanup the zip file. if (!common::removeDirectoryOrFile(zipFile)) @@ -426,7 +428,8 @@ bool LocalCache::SaveModel( } ////////////////////////////////////////////////// -bool LocalCachePrivate::FixPaths(const std::string &_modelVersionedDir) +bool LocalCachePrivate::FixPaths(const std::string &_modelVersionedDir, + const ModelIdentifier &_id) { // Get model.config std::string modelConfigPath = common::joinPaths( @@ -498,7 +501,7 @@ bool LocalCachePrivate::FixPaths(const std::string &_modelVersionedDir) while (collisionElem) { this->FixPathsInGeomElement( - collisionElem->FirstChildElement("geometry"), _modelVersionedDir); + collisionElem->FirstChildElement("geometry"), _id); // Next collision element. collisionElem = collisionElem->NextSiblingElement("collision"); } @@ -509,9 +512,9 @@ bool LocalCachePrivate::FixPaths(const std::string &_modelVersionedDir) while (visualElem) { this->FixPathsInGeomElement( - visualElem->FirstChildElement("geometry"), _modelVersionedDir); + visualElem->FirstChildElement("geometry"), _id); this->FixPathsInMaterialElement( - visualElem->FirstChildElement("material"), _modelVersionedDir); + visualElem->FirstChildElement("material"), _id); visualElem = visualElem->NextSiblingElement("visual"); } linkElem = linkElem->NextSiblingElement("link"); @@ -531,7 +534,7 @@ bool LocalCachePrivate::FixPaths(const std::string &_modelVersionedDir) auto filenameElem = skinElem->FirstChildElement("filename"); if (filenameElem) { - this->FixPathsInUri(filenameElem, _modelVersionedDir); + this->FixPathsInUri(filenameElem, _id); } skinElem = skinElem->NextSiblingElement("skin"); } @@ -543,7 +546,7 @@ bool LocalCachePrivate::FixPaths(const std::string &_modelVersionedDir) auto filenameElem = animationElem->FirstChildElement("filename"); if (filenameElem) { - this->FixPathsInUri(filenameElem, _modelVersionedDir); + this->FixPathsInUri(filenameElem, _id); } animationElem = animationElem->NextSiblingElement("animation"); } @@ -556,34 +559,57 @@ bool LocalCachePrivate::FixPaths(const std::string &_modelVersionedDir) ////////////////////////////////////////////////// void LocalCachePrivate::FixPathsInUri(tinyxml2::XMLElement *_elem, - const std::string &_modelVersionedDir) + const ModelIdentifier &_id) { if (!_elem) return; - std::string uri = _elem->GetText(); + std::string oldUri = _elem->GetText(); std::string prefix = "model://"; // Make sure the URI is of the form model:// - if (uri.find(prefix) != std::string::npos) - { - int firstSlash = uri.find('/', prefix.size()+1); - std::string suffix = uri.substr(firstSlash); + if (oldUri.find(prefix) == std::string::npos) + return; - // Convert the model:// to point to an actual file. - // \todo(nkoenig) Handle URIs to other models. For - // example, ModelA may use something from ModelB. - std::string diskPath = common::joinPaths("file:/", - _modelVersionedDir, suffix); + int firstSlash = oldUri.find('/', prefix.size()+1); - _elem->SetText(diskPath.c_str()); + auto resourceName = oldUri.substr(prefix.size(), firstSlash - prefix.size()); + + if (resourceName != _id.Name()) + { + // On Blueprint and Citadel, just warn the user + // From Dome, use the name on the URI (resourceName) and assume the same + // owner + igndbg << "Model [" << _id.Name() + << "] loading resource from another model, named [" << resourceName + << "]. On Blueprint (ign-fuel-tools 3) and Citadel " + << "(ign-fuel-tools 4), [" << resourceName << "] is ignored. " + << "From Dome (ign-fuel-tools 5), [" << _id.Name() + << "] will be used. If [" << resourceName + << "] is not a model belonging to owner [" << _id.Owner() + << "], fix your SDF file!" << std::endl; } + + auto filePath = oldUri.substr(firstSlash); + + // Construct a model file URL used to download from the server + auto fuelUrl = + _id.Server().Url().Str() + '/' + + _id.Server().Version() + '/' + + _id.Owner() + + "/models/" + + _id.Name() + '/' + + _id.VersionStr() + + "/files" + + filePath; + + _elem->SetText(fuelUrl.c_str()); } ////////////////////////////////////////////////// void LocalCachePrivate::FixPathsInMaterialElement( tinyxml2::XMLElement *_matElem, - const std::string &_modelVersionedDir) + const ModelIdentifier &_id) { if (!_matElem) return; @@ -597,7 +623,7 @@ void LocalCachePrivate::FixPathsInMaterialElement( // Convert the "model://" URI pattern to file:// while (uriElem) { - this->FixPathsInUri(uriElem, _modelVersionedDir); + this->FixPathsInUri(uriElem, _id); uriElem = uriElem->NextSiblingElement("uri"); } } @@ -616,30 +642,30 @@ void LocalCachePrivate::FixPathsInMaterialElement( tinyxml2::XMLElement *albedoElem = workflowElem->FirstChildElement("albedo_map"); if (albedoElem) - this->FixPathsInUri(albedoElem, _modelVersionedDir); + this->FixPathsInUri(albedoElem, _id); tinyxml2::XMLElement *normalElem = workflowElem->FirstChildElement("normal_map"); if (normalElem) - this->FixPathsInUri(normalElem, _modelVersionedDir); + this->FixPathsInUri(normalElem, _id); tinyxml2::XMLElement *envElem = workflowElem->FirstChildElement("environment_map"); if (envElem) - this->FixPathsInUri(envElem, _modelVersionedDir); + this->FixPathsInUri(envElem, _id); tinyxml2::XMLElement *emissiveElem = workflowElem->FirstChildElement("emissive_map"); if (emissiveElem) - this->FixPathsInUri(emissiveElem, _modelVersionedDir); + this->FixPathsInUri(emissiveElem, _id); // metal workflow specific elements if (workflow == "metal") { tinyxml2::XMLElement *metalnessElem = workflowElem->FirstChildElement("metalness_map"); if (metalnessElem) - this->FixPathsInUri(metalnessElem, _modelVersionedDir); + this->FixPathsInUri(metalnessElem, _id); tinyxml2::XMLElement *roughnessElem = workflowElem->FirstChildElement("roughness_map"); if (roughnessElem) - this->FixPathsInUri(roughnessElem, _modelVersionedDir); + this->FixPathsInUri(roughnessElem, _id); } // specular workflow specific elements else if (workflow == "specular") @@ -647,11 +673,11 @@ void LocalCachePrivate::FixPathsInMaterialElement( tinyxml2::XMLElement *specularElem = workflowElem->FirstChildElement("specular_map"); if (specularElem) - this->FixPathsInUri(specularElem, _modelVersionedDir); + this->FixPathsInUri(specularElem, _id); tinyxml2::XMLElement *glossinessElem = workflowElem->FirstChildElement("glossiness_map"); if (glossinessElem) - this->FixPathsInUri(glossinessElem, _modelVersionedDir); + this->FixPathsInUri(glossinessElem, _id); } } } @@ -661,7 +687,7 @@ void LocalCachePrivate::FixPathsInMaterialElement( ////////////////////////////////////////////////// void LocalCachePrivate::FixPathsInGeomElement(tinyxml2::XMLElement *_geomElem, - const std::string &_modelVersionedDir) + const ModelIdentifier &_id) { if (!_geomElem) return; @@ -673,7 +699,7 @@ void LocalCachePrivate::FixPathsInGeomElement(tinyxml2::XMLElement *_geomElem, { tinyxml2::XMLElement *uriElem = meshElem->FirstChildElement("uri"); // Convert the "model://" URI pattern to file:// - this->FixPathsInUri(uriElem, _modelVersionedDir); + this->FixPathsInUri(uriElem, _id); } }