Skip to content

Commit c58d7ea

Browse files
committed
Simplify student cli execution by inferring lab with files in cwd (#62)
* add get_assignment_by_pathname function to global registrar * add behavior to student mode to infer assignment when not specified * add indirection operator to asmdata for nicer semantics * add tests for a very basic "dumb" assignment
1 parent 56621dc commit c58d7ea

10 files changed

Lines changed: 254 additions & 41 deletions

File tree

include/asmgrader/api/asm_data.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ class AsmData
2626
// NOLINTNEXTLINE(google-runtime-operator) - nicer semantics?
2727
std::uintptr_t operator&() const { return address_; }
2828

29+
Result<T> operator*() const { return get_value(); }
30+
2931
std::uintptr_t get_address() const { return address_; }
3032

3133
/// Get the value currently present in the asm program

include/asmgrader/api/asm_symbol.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class AsmSymbolResult : public Result<T>
5050
};
5151

5252
template <typename T>
53-
class AsmSymbol : AsmData<T>
53+
class AsmSymbol : public AsmData<T>
5454
{
5555
public:
5656
AsmSymbol(Program& prog, std::string name, std::uintptr_t address);

include/asmgrader/common/linux.hpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
#pragma once
22

3+
#include <asmgrader/common/aliases.hpp>
34
#include <asmgrader/common/expected.hpp>
45
#include <asmgrader/common/extra_formatters.hpp>
56
#include <asmgrader/logging.hpp>
67

78
#include <fmt/format.h>
89
#include <fmt/ostream.h>
10+
#include <gsl/zstring>
911
#include <libassert/assert.hpp>
1012
#include <range/v3/algorithm/transform.hpp>
1113

@@ -106,6 +108,23 @@ inline Expected<> kill(pid_t pid, int sig) {
106108
return {};
107109
}
108110

111+
/// see access(2)
112+
/// returns success/failure; logs failure at debug level
113+
inline Expected<> access(gsl::czstring path, int mode) {
114+
ASSERT((static_cast<u64>(mode) & ~static_cast<u64>(R_OK | W_OK | X_OK | F_OK)) == 0, mode);
115+
116+
int res = ::access(path, mode);
117+
118+
if (res == -1) {
119+
auto err = make_error_code(errno);
120+
121+
LOG_DEBUG("access failed: '{}'", err);
122+
return err;
123+
}
124+
125+
return {};
126+
}
127+
109128
/// args and envp do NOT need to have an extra NULL element; this is added for you.
110129
/// see execve(2)
111130
/// returns success/failure; logs failure at debug level

include/asmgrader/registrars/global_registrar.hpp

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,12 @@ class GlobalRegistrar
3636
void add_test(std::unique_ptr<TestBase> test) noexcept;
3737

3838
template <typename Func>
39-
requires(std::is_void_v<std::invoke_result_t<Func, Assignment>>)
39+
requires(std::is_void_v<std::invoke_result_t<Func, Assignment&>>)
4040
void for_each_assignment(Func&& fun);
4141

4242
template <typename Func>
43-
requires(!std::is_void_v<std::invoke_result_t<Func, Assignment>>)
44-
std::vector<std::invoke_result_t<Func, Assignment>> for_each_assignment(Func&& fun);
43+
requires(!std::is_void_v<std::invoke_result_t<Func, Assignment&>>)
44+
std::vector<std::invoke_result_t<Func, Assignment&>> for_each_assignment(Func&& fun);
4545

4646
auto get_assignments() noexcept {
4747
return registered_assignments_ |
@@ -61,6 +61,18 @@ class GlobalRegistrar
6161
return std::nullopt;
6262
}
6363

64+
std::optional<std::reference_wrapper<Assignment>> get_assignment_by_pathname(std::string_view pathname) {
65+
auto name_matcher = [pathname](const std::unique_ptr<Assignment>& assignment) {
66+
return assignment->get_exec_path() == pathname;
67+
};
68+
69+
if (auto iter = ranges::find_if(registered_assignments_, name_matcher); iter != registered_assignments_.end()) {
70+
return **iter;
71+
}
72+
73+
return std::nullopt;
74+
}
75+
6476
/// Find the assignment corresponding to the `Assignment` attribute of metadata,
6577
/// or create a new assignment if one does not exist.
6678
template <typename... MetadataAttrs>
@@ -102,16 +114,16 @@ Assignment& GlobalRegistrar::find_or_create_assignment(metadata::Metadata<Metada
102114
}
103115

104116
template <typename Func>
105-
requires(std::is_void_v<std::invoke_result_t<Func, Assignment>>)
117+
requires(std::is_void_v<std::invoke_result_t<Func, Assignment&>>)
106118
void GlobalRegistrar::for_each_assignment(Func&& fun) {
107-
for (auto& assignement : get_assignments()) {
108-
std::forward<Func>(fun)(assignement);
119+
for (auto& assignment : get_assignments()) {
120+
std::forward<Func>(fun)(assignment);
109121
}
110122
}
111123

112124
template <typename Func>
113-
requires(!std::is_void_v<std::invoke_result_t<Func, Assignment>>)
114-
std::vector<std::invoke_result_t<Func, Assignment>> GlobalRegistrar::for_each_assignment(Func&& fun) {
125+
requires(!std::is_void_v<std::invoke_result_t<Func, Assignment&>>)
126+
std::vector<std::invoke_result_t<Func, Assignment&>> GlobalRegistrar::for_each_assignment(Func&& fun) {
115127
std::vector<std::invoke_result_t<Func, Assignment>> result;
116128
result.reserve(registered_assignments_.size());
117129

src/app/student_app.cpp

Lines changed: 87 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include "app/student_app.hpp"
22

3+
#include "api/assignment.hpp"
4+
#include "common/linux.hpp"
35
#include "grading_session.hpp"
46
#include "logging.hpp"
57
#include "output/plaintext_serializer.hpp"
@@ -9,29 +11,109 @@
911
#include "test_runner.hpp"
1012
#include "user/program_options.hpp"
1113

14+
#include <fmt/base.h>
1215
#include <fmt/format.h>
1316
#include <libassert/assert.hpp>
17+
#include <range/v3/view/transform.hpp>
1418

1519
#include <cstdlib>
20+
#include <filesystem>
21+
#include <functional>
1622
#include <memory>
1723
#include <optional>
24+
#include <string>
25+
#include <string_view>
26+
#include <vector>
27+
28+
#include <unistd.h>
1829

1930
namespace asmgrader {
2031

32+
namespace {
33+
34+
bool can_execute(const std::filesystem::path& path) {
35+
namespace fs = std::filesystem;
36+
37+
if (!fs::is_regular_file(path)) {
38+
LOG_TRACE("{} is not a regular file", path);
39+
return false;
40+
}
41+
42+
if (!linux::access(path.c_str(), X_OK)) {
43+
LOG_TRACE("{} is not executable by this program", path);
44+
return false;
45+
}
46+
47+
return true;
48+
}
49+
50+
std::reference_wrapper<Assignment> infer_assignment_or_exit() {
51+
namespace fs = std::filesystem;
52+
53+
auto& registrar = GlobalRegistrar::get();
54+
55+
std::vector<std::reference_wrapper<Assignment>> infer_res;
56+
57+
for (fs::path cwd = fs::current_path(); const auto& dirent : fs::directory_iterator{cwd}) {
58+
const auto& path = dirent.path();
59+
std::string pathname = path.filename().string();
60+
LOG_TRACE("Checking if {:?} matches any assignments {}", pathname, dirent.path());
61+
62+
if (!can_execute(path)) {
63+
continue;
64+
}
65+
66+
if (auto found = registrar.get_assignment_by_pathname(pathname)) {
67+
LOG_TRACE("{:?} matches an assignment", pathname);
68+
infer_res.push_back(found.value());
69+
}
70+
}
71+
72+
if (infer_res.size() == 0) {
73+
fmt::println(stderr, "Could not infer assignment based on files in the current working directory! Please "
74+
"either change directories or specify the assignment explicitly by name. Exiting.");
75+
std::exit(1);
76+
}
77+
78+
if (infer_res.size() > 1) {
79+
fmt::println(
80+
stderr,
81+
"More than 1 valid assignment found in current working directory when attempting to infer "
82+
"assignment.\nThis is ambiguous! Please specify the assignment explicitly by name.\n(Found: {::?})",
83+
infer_res |
84+
ranges::views::transform([](auto& assignment_ref) { return assignment_ref.get().get_exec_path(); }));
85+
std::exit(1);
86+
}
87+
88+
// infer_res.size() == 1
89+
return infer_res.front();
90+
}
91+
92+
} // namespace
93+
94+
Assignment& StudentApp::get_assignment_or_exit() const {
95+
if (OPTS.assignment_name.empty()) {
96+
LOG_DEBUG("Assignment unspecified; attempting to infer based on files in cwd.");
97+
return infer_assignment_or_exit();
98+
}
99+
100+
auto opt_assignment = GlobalRegistrar::get().get_assignment(OPTS.assignment_name);
101+
ASSERT(opt_assignment.has_value(), "Internal error locating assignment", OPTS.assignment_name);
102+
103+
return opt_assignment.value();
104+
}
105+
21106
int StudentApp::run_impl() {
22107
LOG_TRACE("Registered tests: {::}", GlobalRegistrar::get().for_each_assignment([](const Assignment& assignment) {
23108
return fmt::format("{:?}: {}", assignment.get_name(), assignment.get_test_names());
24109
}));
25110

26-
auto assignment = GlobalRegistrar::get().get_assignment(OPTS.assignment_name);
27-
28-
// should be verified by CLI; double-check here just in case
29-
ASSERT(assignment, "Error locating assignment {}", OPTS.assignment_name);
111+
Assignment& assignment = get_assignment_or_exit();
30112

31113
StdoutSink output_sink;
32114
std::shared_ptr output_serializer =
33115
std::make_shared<PlainTextSerializer>(output_sink, OPTS.colorize_option, OPTS.verbosity);
34-
AssignmentTestRunner runner{*assignment, output_serializer, OPTS.tests_filter};
116+
AssignmentTestRunner runner{assignment, output_serializer, OPTS.tests_filter};
35117

36118
output_serializer->on_run_metadata(RunMetadata{});
37119
AssignmentResult res = runner.run_all(OPTS.file_name);

src/app/student_app.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#pragma once
22

3+
#include "api/assignment.hpp"
34
#include "app/app.hpp" // IWYU pragma: export
45

56
namespace asmgrader {
@@ -11,6 +12,8 @@ class StudentApp final : public App
1112

1213
private:
1314
int run_impl() override;
15+
16+
Assignment& get_assignment_or_exit() const;
1417
};
1518

1619
} // namespace asmgrader

src/user/cl_args.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,22 @@ void CommandLineArgs::setup_parser() {
6161
// maybe want to switch to another lib, or just do it myself. Need arg choices in help.
6262

6363
// clang-format off
64+
std::string assignment_help_str = fmt::format("The assignment to run tests on.\n"
65+
#ifndef PROFESSOR_VERSION
66+
"If left unspecified, will attempt to infer the assignment based on files in the current working directory.\n"
67+
#endif
68+
"One of: {}", assignment_names.empty() ? "<No assignments; this is probably an error>" : fmt::format("{:n}", assignment_names));
6469
auto& assignment_arg = arg_parser_.add_argument("assignment")
6570
.store_into(opts_buffer_.assignment_name)
66-
// Add all assignment names to help msg, since argparse won't add choices by
67-
// default for some reason
68-
.help(fmt::format("The assignment to run tests on\nOne of: {:n}", assignment_names));
71+
#ifdef PROFESSOR_VERSION
72+
.help(assignment_help_str);
73+
#else
74+
// inferring the lab is only supported in student mode for now
75+
.nargs(0, 1) // [optional]
76+
.help(assignment_help_str);
77+
#endif // !PROFESSOR_VERSION
78+
// Add all assignment names to help msg, since argparse won't add choices by
79+
// default for some reason
6980
GlobalRegistrar::get().for_each_assignment([&](const Assignment& assignment) {
7081
assignment_arg.add_choice(assignment.get_name());
7182
});

src/user/program_options.hpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,17 @@ struct ProgramOptions
116116

117117
TRY(ensure_is_directory(search_path, "Search path {:?}"));
118118

119+
// Only check the database path if it's not the default
120+
// non-existance will be handled properly in ProfessorApp
121+
if (database_path != DEFAULT_DATABASE_PATH) {
122+
TRY(ensure_is_regular_file(database_path, "Database file {:?}"));
123+
}
124+
125+
// If the assignment name is empty, we're going to be attempting to infer it elsewhere
126+
if (assignment_name.empty()) {
127+
return {};
128+
}
129+
119130
// The CLI should verify that the specified assignment is valid
120131
// We'll check here just in case and return an error if it's not
121132
auto assignment = TRYE(GlobalRegistrar::get().get_assignment(assignment_name),
@@ -129,12 +140,6 @@ struct ProgramOptions
129140
TRY(Program::check_is_compat_elf(exec_file_name));
130141
}
131142

132-
// Only check the database path if it's not the default
133-
// non-existance will be handled properly in ProfessorApp
134-
if (database_path != DEFAULT_DATABASE_PATH) {
135-
TRY(ensure_is_regular_file(database_path, "Database file {:?}"));
136-
}
137-
138143
return {};
139144
}
140145
};

0 commit comments

Comments
 (0)