Skip to content
This repository has been archived by the owner on Jan 19, 2024. It is now read-only.

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

Merged
merged 4 commits into from
Apr 26, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2712d12
Add support for locations starting with `https://github.com/`
Apr 24, 2023
be46c46
Code review fixes
Apr 25, 2023
063aa8c
Revert comment change
Apr 25, 2023
25f72a1
Code review - add comment
Apr 25, 2023
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
3 changes: 2 additions & 1 deletion include/antler/project/dependency.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <iostream>
#include <utility> // std::pair

#include "location.hpp"
#include "version.hpp"
#include "yaml.hpp"
#include "../system/utils.hpp"
Expand Down Expand Up @@ -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")
Expand Down
5 changes: 5 additions & 0 deletions include/antler/project/location.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion include/antler/project/net_utils.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
13 changes: 10 additions & 3 deletions include/antler/project/object.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 2 additions & 0 deletions include/antler/system/utils.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Copy link
Contributor

Choose a reason for hiding this comment

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

what does it affect? if there will be error you return nullopt and if not you'll unlikely to have error output at all. And even if you do, parsing of output will likely break.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It resolves issue: sh: 1: gh: not found in the original issue.

I believe Bucky's intention was to run command line gh --version quietly (the function name is execute_quiet).
While stdout was silenced, the stderr was still printing error (which was ignored anyway).

Copy link
Contributor

Choose a reason for hiding this comment

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

got you. in that case cmd += "2> /dev/null" sounds to me like more self-descriptive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In current usages of execute_quiet both 2>&1 and 2> /dev/null are fine.
If someone would like to read the stderr if the return code is 0 (there are such cases) with the existing solution he will have such chance. In yours we are losing stderr output. So I will leave the current solution.

Copy link
Contributor

@dimas1185 dimas1185 Apr 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I doubt that could be a case. if we will have non-critical error output on zero return code that may corrupt parsing of a valid data. Ideally that should be similar to subprocess.run in python. i.e. stdout and stderr piping is customizable. but I would leave it to next issue if that will be really needed. If you believe your case is more likely to happen than mine, just leave comment like pipe error output from console to stdout so caller can parse error string and I'll approve

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, done


FILE* h = popen(cmd.c_str(), "r");
if (h == nullptr) {
Expand Down
3 changes: 1 addition & 2 deletions src/dependency.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 13 additions & 3 deletions src/location.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(jackfan.us.kg.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); }

Expand All @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion src/object.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions src/project-parse.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 0 additions & 7 deletions tests/dependency_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
18 changes: 14 additions & 4 deletions tests/location_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand All @@ -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");

Expand All @@ -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.comx"sv) == "https://github.comx"sv);
CHECK(location::strip_github_com("https://github.com/"sv) == ""sv);
}
20 changes: 20 additions & 0 deletions tests/net_utils_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);
}
4 changes: 2 additions & 2 deletions tests/object_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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"));

Expand Down
8 changes: 7 additions & 1 deletion tests/project_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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", ""});
Expand All @@ -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"});

Expand All @@ -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");
}


Expand Down
2 changes: 1 addition & 1 deletion tools/add_to.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down