Merge pull request #47 from AntelopeIO/mikelik/github_url

Add support for locations starting with `https://github.com/`

include/antler/project/dependency.hpp

@@ -9,6 +9,7 @@
 #include <iostream>
 #include <utility> // std::pair
 
+#include "location.hpp"
 #include "version.hpp"
 #include "yaml.hpp"
 #include "../system/utils.hpp"
@@ -60,7 +61,7 @@ class dependency {
  [[nodiscard]] inline const std::string& location() const noexcept { return m_loc; }
  /// Set the location field of this dependency.
  /// @param s The new from location of this dependency.
- void location(std::string s) noexcept { m_loc = std::move(s); }
+ void location(std::string s) noexcept { m_loc = location::strip_github_com(std::move(s)); }
 
  /// Report on the status of this dependencies from field: does it look like an archive?
  /// @return true if location ends in an archive format (e.g. ".tar.gz", ".tgz", etc")

include/antler/project/location.hpp

@@ -9,6 +9,11 @@
 
 namespace antler::project::location {
 
+/// If location is github URL then strips out `https://github.com/` from URL. Otherwise returns input argument.
+/// @param loc Github location - either full URL or shorthand
+/// @return string_view location striped from `https://github.com/`
+[[nodiscard]] std::string_view strip_github_com(std::string_view location);
+
 /// @param l Location to evaluate.
 /// @return true if l looks an archive.
 [[nodiscard]] bool is_archive(std::string_view l);

include/antler/project/net_utils.hpp

@@ -154,7 +154,7 @@ namespace antler::project {
 
  static inline bool is_shorthand(std::string_view s) {
  auto sub = s.substr(0, s.find_last_of("/"));
- return sub.size() != s.size() && sub.find_last_of("/") == std::string_view::npos;
+ return s.size() > (sub.size() + 1) && sub.find_last_of("/") == std::string_view::npos;
  }
 
  private:

include/antler/project/object.hpp

@@ -90,12 +90,19 @@ namespace antler::project {
 
  /// Update or insert a dependency.
  /// @param dep The dependency to upsert.
- /// @return true if it is an insert, false if it is an update
+ /// @return false if it is an insert, true if it is an update
  bool upsert_dependency(antler::project::dependency&& dep) noexcept {
  const auto& itr = m_dependencies.find(dep.name());
- bool has_value = itr == m_dependencies.end();
+ bool has_value = itr != m_dependencies.end();
+
+ if(has_value) {
+ itr->second = dep;
+ }
+ else {
+ auto name = dep.name();
+ m_dependencies.emplace(std::move(name), std::move(dep));
+ }
 
- m_dependencies.emplace(dep.name(), std::move(dep));
  return has_value;
  }
 

include/antler/system/utils.hpp

@@ -86,6 +86,8 @@ namespace antler::system {
  for (const auto& arg : args) {
  cmd += " " + arg;
  }
+ //redirect stderr to stdout in case caller wants to parse the error
+ cmd += " 2>&1";
 
  FILE* h = popen(cmd.c_str(), "r");
  if (h == nullptr) {

src/dependency.cpp

@@ -72,12 +72,11 @@ void dependency::patch_remove(const system::fs::path& path) noexcept {
  m_patchfiles.erase(i);
 }
 
-
 void dependency::set(std::string nm, std::string_view loc, std::string_view tag, std::string_view rel, std::string_view hash) {
 
  m_name = std::move(nm);
 
- m_loc = loc;
+ m_loc = location::strip_github_com(loc);
 
  m_tag_or_commit = tag;
  m_rel = rel;

src/location.cpp

@@ -22,7 +22,17 @@ bool is_archive(std::string_view s) {
  ends_with(s, ".tar.zst");
 }
 
-static inline bool is_github(std::string_view s) { return starts_with(s, "https://github.com"); }
+constexpr std::string_view github_com = "https://github.com/";
+
+std::string_view strip_github_com(std::string_view location) {
+ if (is_github_repo(location)) {
+ return location.substr(github_com.size());
+ } 
+
+ return location;
+}
+
+static inline bool is_github(std::string_view s) { return starts_with(s, github_com); }
 
 bool is_github_archive(std::string_view s) { return is_github(s) && is_archive(s); }
 
@@ -34,12 +44,12 @@ bool is_github_repo(std::string_view s) { return is_github(s) && !is_archive(s);
 
 bool is_reachable(std::string_view l) {
  if (!is_github_shorthand(l)) {
- system::error_log("In this version of antler-proj only github shorthands (i.e. org/project) are supported. Generalized git repos and archives will be supported in a future version.");
+ system::error_log("In this version of antler-proj only github shorthands (i.e. org/project) and github URLs (i.e. https://github.com/org/project) are supported. Archives will be supported in a future version.");
  return false;
  }
 
  return github{}.is_reachable(l);
- // TODO add support for general git repos and archives
+ // TODO add support for archives
  if (is_github_repo(l) || is_github_shorthand(l)) {
  return github{}.is_reachable(l);
  } else if (is_archive(l) || is_url(l) || is_github_archive(l)) {

src/object.cpp

@@ -97,7 +97,7 @@ object::type_t object::type() const noexcept {
 
 
 void object::upsert_dependency(antler::project::dependency&& dep) noexcept {
- // If possible, find a dependency with matching name and reurrn it.
+ // If possible, find a dependency with matching name and return it.
  auto i = std::find_if(m_dependencies.begin(), m_dependencies.end(), [dep](const antler::project::dependency& d) { return d.name() == dep.name(); } );
  if( i == m_dependencies.end() )
  m_dependencies.emplace_back(dep);

src/project-parse.cpp

@@ -161,11 +161,12 @@ template <typename NODE_T>
  os << "Duplicate " << tok << " values in dependency list: " << i.val() << ", " << rv.location() << "\n";
  return {};
  }
- if (!dependency::validate_location(sv_from_csubstr(i.val()))) {
+ auto location = strip_github_com(sv_from_csubstr(i.val()));
+ if (!dependency::validate_location(location)) {
  os << "Invalid location: " << i.val() << "\n";
  return {};
  }
- rv.location(sv_from_csubstr(i.val()));
+ rv.location(location);
  } break;
 
  case token::patch: {

tests/CMakeLists.txt

@@ -13,6 +13,7 @@ add_executable(unit_tests
  dependency_tests.cpp
  location_tests.cpp
  object_tests.cpp
+ net_utils_tests.cpp
  project_tests.cpp
  init_tests.cpp
  version_constraint_tests.cpp 

tests/dependency_tests.cpp

@@ -18,13 +18,6 @@ TEST_CASE("Testing dependency validating locations") {
  CHECK(d.location() == "larryk85/does-not-exist");
 
  CHECK(!d.is_valid_location());
-
- // TODO: reinstate these tests when support for general git repos and archives is added
- //d.location("https://github.com/larryk85/cturtle");
- //CHECK(d.is_valid_location());
-
- //d.location("https://github.com/larryk85/does-not-exist");
- //CHECK(!d.is_valid_location());
 }
 
 TEST_CASE("Testing dependency yaml conversions") {

tests/location_tests.cpp

@@ -4,9 +4,10 @@
 
 #include <catch2/catch.hpp>
 
-TEST_CASE("Testing location clone") {
- using namespace antler::project;
+using namespace std::literals;
+using namespace antler::project;
 
+TEST_CASE("Testing location clone") {
  antler::system::fs::remove_all("./clone_test");
 
  CHECK(location::clone_github_repo("antelopeio", "antler-proj", "main", 10, "./clone_test/foo2"));
@@ -17,8 +18,6 @@ TEST_CASE("Testing location clone") {
 }
 
 TEST_CASE("Testing location github REST API requests") {
- using namespace antler::project;
-
  std::string default_branch = location::get_github_default_branch("antelopeio", "antler-proj");
  CHECK(default_branch == "main");
 
@@ -27,3 +26,14 @@ TEST_CASE("Testing location github REST API requests") {
 
  CHECK_THROWS(location::get_github_default_branch("antelopeio", "repo-does-not-exist"));
 }
+
+TEST_CASE("strip_github_com") {
+ CHECK(location::strip_github_com("https://github.com/org/project"sv) == "org/project"sv);
+ CHECK(location::strip_github_com("https://github.com/org/project/z"sv) == "org/project/z"sv);
+ CHECK(location::strip_github_com("https://github.com/org"sv) == "org"sv);
+ CHECK(location::strip_github_com("https://xyz.com"sv) == "https://xyz.com"sv);
+ CHECK(location::strip_github_com(""sv) == ""sv);
+ CHECK(location::strip_github_com("https://github.com"sv) == "https://github.com"sv);
+ CHECK(location::strip_github_com("https://github.saobby.my.eu.orgx"sv) == "https://github.saobby.my.eu.orgx"sv);
+ CHECK(location::strip_github_com("https://github.com/"sv) == ""sv);
+}

tests/net_utils_tests.cpp

@@ -0,0 +1,20 @@
+/// @copyright See `LICENSE` in the root directory of this project.
+
+#include <antler/project/net_utils.hpp>
+
+#include <catch2/catch.hpp>
+
+using namespace std::literals;
+
+TEST_CASE("shorthand") {
+ using namespace antler::project;
+
+ CHECK(github::is_shorthand("org/project"sv));
+ CHECK(github::is_shorthand("org/a"sv));
+
+ CHECK(github::is_shorthand("https://github.com/org/project"sv) == false);
+ CHECK(github::is_shorthand("org/project/"sv) == false);
+ CHECK(github::is_shorthand("org/"sv) == false);
+ CHECK(github::is_shorthand("org"sv) == false);
+ CHECK(github::is_shorthand(""sv) == false);
+}

tests/object_tests.cpp

@@ -28,7 +28,7 @@ TEST_CASE("Testing object") {
  CHECK(!app1.dependency_exists("dep1"));
  CHECK(!app1.find_dependency("dep1"));
 
- CHECK(app1.upsert_dependency(std::move(dep1)));
+ CHECK(app1.upsert_dependency(std::move(dep1)) == false);
 
  CHECK(app1.dependency_exists("dep1"));
  const auto& d = app1.find_dependency("dep1");
@@ -40,7 +40,7 @@ TEST_CASE("Testing object") {
  CHECK(d->hash().empty());
 
  CHECK(app1.dependencies().size() == 1);
- CHECK(app1.upsert_dependency({"dep2", "larryk85/foo2"}));
+ CHECK(app1.upsert_dependency({"dep2", "larryk85/foo2"}) == false);
  CHECK(app1.dependencies().size() == 2);
  CHECK(app1.remove_dependency("dep2"));
 

tests/project_tests.cpp

@@ -14,6 +14,7 @@ TEST_CASE("Testing project") {
  {"appd", "C++", "", ""} };
 
  apps[0].upsert_dependency({"foo", "https://github.com/larryk85/dune", "v13.3"});
+ apps[0].upsert_dependency({"foo", "mikelik/dune", "v13.4"});
  apps[1].upsert_dependency({"bar", "https://github.com/larryk85/fast_math", "blah"});
  apps[0].upsert_dependency({"baz", "https://github.com/antelopeio/leap", "v2.2.2v"});
  apps[1].upsert_dependency({"libc", ""});
@@ -22,7 +23,8 @@ TEST_CASE("Testing project") {
  {"libc", "C", "", ""},
  {"libd", "C++", "", ""} };
 
- libs[0].upsert_dependency({"foo", "https://github.com/larryk85/dune", "v13.3"});
+ libs[0].upsert_dependency({"foo", "https://github.com/larryk85/dune", "main"});
+ libs[0].upsert_dependency({"foo", "mikelik/dune2", "branch"});
  libs[0].upsert_dependency({"bar", "https://github.com/larryk85/fast_math", "blah"});
  libs[1].upsert_dependency({"baz", "https://github.com/antelopeio/leap", "v2.2.2v"});
 
@@ -43,7 +45,11 @@ TEST_CASE("Testing project") {
  CHECK(proj.version().to_string() == "1.3.4");
 
  CHECK(proj.apps().size() == 4);
+ CHECK(proj.apps()["appa"].dependencies()["foo"].location() == "mikelik/dune");
+ CHECK(proj.apps()["appa"].dependencies()["foo"].tag() == "v13.4");
  CHECK(proj.libs().size() == 3);
+ CHECK(proj.libs()["libb"].dependencies()["foo"].location() == "mikelik/dune2");
+ CHECK(proj.libs()["libb"].dependencies()["foo"].tag() == "branch");
 }
 
 

tools/add_to.hpp

@@ -119,7 +119,7 @@ namespace antler {
  obj.upsert_dependency(std::move(dep));
 
  // We have values, so query the user if they want to apply.
- system::info_log("\nObject name (to update): {0}\n"
+ system::info_log("Adding dependency:\nObject name (to update): {0}\n"
  "Dependency name: {1}\n"
  "Dependency location: {2}\n"
  "tag/commit hash: {3}\n"