diff --git a/Containerfile b/Containerfile index 087ff369..654187af 100644 --- a/Containerfile +++ b/Containerfile @@ -10,7 +10,10 @@ RUN dnf update -y && \ python-flake8 \ python-virtualenv \ python-pip \ - git + gawk \ + socat \ + git \ + git-email RUN sed -i 's/log_driver = "journald"/log_driver = "json-file"/' /usr/share/containers/containers.conf diff --git a/container-compose.yml b/container-compose.yml index bbc50556..8b92f26e 100644 --- a/container-compose.yml +++ b/container-compose.yml @@ -185,6 +185,7 @@ services: - orbit_source=./orbit environment: TZ: ${SINGULARITY_TIMEZONE} + HOSTNAME: ${SINGULARITY_HOSTNAME} volumes: - type: volume source: denis-db diff --git a/create_rubric.py b/create_rubric.py new file mode 100755 index 00000000..c218b17c --- /dev/null +++ b/create_rubric.py @@ -0,0 +1,24 @@ +#!/bin/env python + +from argparse import ArgumentParser as ap +import git +import os + + +def main(): + parser = ap(prog='create_rubric', description='create rubric') + parser.add_argument('-n', type=int, help='number of patches to examine', + required=True) + + args = parser.parse_args() + + repo = git.Repo(os.getcwd()) + for i in range(args.n): + patch = repo.git.execute(['git', 'show', f'HEAD~{args.n-i-1}']) + changelines = list(filter(lambda line: line.startswith('--- ') or line.startswith('+++ '), patch.split('\n'))) + changeline_pairs = {(fromfile, tofile): 0 for fromfile, tofile in zip(changelines[::2], changelines[1::2])} + print(f'{"[" if i == 0 else ""}{changeline_pairs}{"," if i < args.n - 1 else "]"}') + + +if __name__ == '__main__': + main() diff --git a/denis/configure.py b/denis/configure.py index c18060d9..bc3059f7 100755 --- a/denis/configure.py +++ b/denis/configure.py @@ -2,6 +2,8 @@ from datetime import datetime from argparse import ArgumentParser as ap +from pathlib import Path +import os import db @@ -32,6 +34,11 @@ def add_final(parser, required=True): help='Final submission due date timetamp', required=required) + def add_rubric(parser): + parser.add_argument('-r', '--rubric', + type=str, + help='Rubric to enforce on incoming patchsets') + command_parsers = parser.add_subparsers(dest='command', required=True) create_parser = command_parsers.add_parser('create') @@ -39,12 +46,14 @@ def add_final(parser, required=True): add_initial(create_parser) add_peer_review(create_parser) add_final(create_parser) + add_rubric(create_parser) alter_parser = command_parsers.add_parser('alter') add_assignment(alter_parser) add_initial(alter_parser, required=False) add_peer_review(alter_parser, required=False) add_final(alter_parser, required=False) + add_rubric(alter_parser) remove_parser = command_parsers.add_parser('remove') add_assignment(remove_parser) @@ -67,17 +76,35 @@ def add_final(parser, required=True): subparser_func(**kwargs) -def create(assignment, initial, peer_review, final): +def dirty(): + Path('/tmp/dirty').touch() + + +# since the rubric is copied into the container by the wrapper script, +# we always place it in /tmp/rubric to simplify things +def load_rubric(): + try: + with open('/tmp/rubric', 'r') as rubric_file: + os.unlink('/tmp/rubric') + return rubric_file.read() + except FileNotFoundError: + return None + + +def create(assignment, initial, peer_review, final, rubric): + rubric = load_rubric() try: db.Assignment.create(name=assignment, initial_due_date=initial, peer_review_due_date=peer_review, - final_due_date=final) + final_due_date=final, + rubric=rubric) + dirty() except db.peewee.IntegrityError: print('cannot create assignment with duplicate name') -def alter(assignment, initial, peer_review, final): +def alter(assignment, initial, peer_review, final, rubric): alterations = {} if initial is not None: alterations[db.Assignment.initial_due_date] = initial @@ -85,6 +112,8 @@ def alter(assignment, initial, peer_review, final): alterations[db.Assignment.peer_review_due_date] = peer_review if final is not None: alterations[db.Assignment.final_due_date] = final + if (rubric := load_rubric()) is not None: + alterations[db.Assignment.rubric] = rubric if not alterations: return print('At least one new date must be specified') query = (db.Assignment @@ -92,6 +121,8 @@ def alter(assignment, initial, peer_review, final): .where(db.Assignment.name == assignment)) if query.execute() < 1: print(f'no such assignment {assignment}') + else: + dirty() def remove(assignment): @@ -100,6 +131,8 @@ def remove(assignment): .where(db.Assignment.name == assignment)) if query.execute() < 1: print(f'no such assignment {assignment}') + else: + dirty() def dump(fmt_iso): @@ -107,18 +140,24 @@ def timestamp_to_formatted(timestamp): dt = datetime.fromtimestamp(timestamp).astimezone() return dt.isoformat() if fmt_iso else dt.strftime('%a %b %d %Y %T %Z (%z)') + if os.path.exists('/tmp/dirty'): + print('WARNING: Denis database is dirty, reload to update waiters') + print(' --- Assignments ---') for asn in db.Assignment.select(): print(f'''{asn.name}: \tInitial:\t{timestamp_to_formatted(asn.initial_due_date)} \tPeer Review:\t{timestamp_to_formatted(asn.peer_review_due_date)} -\tFinal:\t\t{timestamp_to_formatted(asn.final_due_date)}''') +\tFinal:\t\t{timestamp_to_formatted(asn.final_due_date)} +{"\tRubric:\n" + str(asn.rubric) if asn.rubric else ""}''') def reload(): import os import signal os.kill(1, signal.SIGUSR1) + if os.path.exists('/tmp/dirty'): + os.remove('/tmp/dirty') if __name__ == '__main__': diff --git a/denis/configure.sh b/denis/configure.sh index f22f4950..b629a08d 100755 --- a/denis/configure.sh +++ b/denis/configure.sh @@ -1,7 +1,31 @@ #!/bin/sh set -e +PODMAN=${PODMAN:-podman} COMPOSE=${COMPOSE:-podman-compose} +# a rubric is passed as a path to a file, +# but the main script is executed inside the container +# so copy any rubric into the container, +# if one is specified. +get_next= +rubric_file= +for arg in "$@" +do + if [ "$arg" = '-r' ] + then + get_next=yes + elif [ -n "$get_next" ] + then + rubric_file=$arg + fi +done + +if [ -n "$rubric_file" ] +then + ${PODMAN} cp "$rubric_file" singularity_denis_1:/tmp/rubric +fi + + ${COMPOSE} exec denis ./configure.py "$@" diff --git a/denis/db.py b/denis/db.py index 7dcf00ac..131a6cdd 100755 --- a/denis/db.py +++ b/denis/db.py @@ -15,6 +15,7 @@ class Assignment(BaseModel): initial_due_date = peewee.IntegerField() peer_review_due_date = peewee.IntegerField() final_due_date = peewee.IntegerField() + rubric = peewee.TextField(null=True) class PeerReviewAssignment(BaseModel): diff --git a/denis/final.py b/denis/final.py index cbee80df..3728d311 100755 --- a/denis/final.py +++ b/denis/final.py @@ -11,4 +11,6 @@ print(f'final subs for {assignment} released') -utilities.update_tags(assignment, 'final') +tags = utilities.update_tags(assignment, 'final') + +utilities.run_automated_checks(tags) diff --git a/denis/initial.py b/denis/initial.py index 5faa098c..71cf2d05 100755 --- a/denis/initial.py +++ b/denis/initial.py @@ -58,4 +58,6 @@ print(f'initial subs for {assignment} released') -utilities.update_tags(assignment, 'initial') +tags = utilities.update_tags(assignment, 'initial') + +utilities.run_automated_checks(tags) diff --git a/denis/peer_review.py b/denis/peer_review.py index 72bc418a..32095a27 100755 --- a/denis/peer_review.py +++ b/denis/peer_review.py @@ -14,5 +14,7 @@ print(f'peer review subs for {assignment} released') -utilities.update_tags(assignment, 'review1') -utilities.update_tags(assignment, 'review2') +tags1 = utilities.update_tags(assignment, 'review1') +tags2 = utilities.update_tags(assignment, 'review2') + +utilities.run_automated_checks(tags1 + tags2, peer=True) diff --git a/denis/utilities.py b/denis/utilities.py index 0120a7e0..d6bd453d 100644 --- a/denis/utilities.py +++ b/denis/utilities.py @@ -1,6 +1,10 @@ +import re +import os import git +import copy import subprocess import tempfile +import difflib import orbit.db import mailman.db @@ -38,35 +42,232 @@ def release_subs(sub_ids): input=journal_data, check=True) +def configure_repo(repo): + with repo.config_writer() as config: + config.set_value('user', 'name', 'denis') + config.set_value('user', 'email', 'denis@denis') + + def update_tags(assignment, component): grd_tbl = mailman.db.Gradeable - subs = (grd_tbl.select() + gbls = (grd_tbl.select() .order_by(-grd_tbl.timestamp) .where(grd_tbl.assignment == assignment) .where(grd_tbl.component == component)) with tempfile.TemporaryDirectory() as repo_path: repo = git.Repo.clone_from(PULL_URL, repo_path) repo.create_remote(REMOTE_NAME, PUSH_URL) - repo.config_writer().set_value('user', 'name', 'denis').release() - (repo.config_writer().set_value('user', 'email', 'denis@denis') - .release()) + configure_repo(repo) if 'EMPTY' not in repo.tags: repo.git.commit('--allow-empty', '-m', 'No gradeable submission.') repo.create_tag('EMPTY') + updated_tags = [] for user in orbit.db.User.select(): new_tag_name = f'{assignment}_{component}_{user.username}' + updated_tags.append(new_tag_name) if new_tag_name in repo.tags: print('Potential issue? Attempted to create duplicate tag ' f'{new_tag_name}') continue - user_sub = subs.where(grd_tbl.user == user.username).first() - if not user_sub or (id := user_sub.submission_id) not in repo.tags: + user_gbl = gbls.where(grd_tbl.user == user.username).first() + if not user_gbl or (id := user_gbl.submission_id) not in repo.tags: msg = 'No gradeable submission' to_promote = repo.tags['EMPTY'] else: - msg = user_sub.status + msg = user_gbl.auto_feedback to_promote = repo.tags[id] repo.create_tag(new_tag_name, ref=to_promote.commit, message=msg) repo.git.push(REMOTE_NAME, tags=True) + + return updated_tags + + +def check_corrupt_or_missing(repo, tag): + [assignment, component, user] = tag.split('_') + + grd_tbl = mailman.db.Gradeable + gradable = (grd_tbl.select() + .order_by(-grd_tbl.timestamp) + .where(grd_tbl.assignment == assignment) + .where(grd_tbl.component == component) + .where(grd_tbl.user == user)).first() + + msg = 'corruption and existence check' + msg += '\n' + msg += '------------------------------' + msg += '\n\n' + if not gradable or gradable.auto_feedback[-1] == '!': + repo.git.execute(['git', 'notes', '--ref=grade', 'add', tag, '-m', '0']) + if not gradable: + msg += 'automatic 0 due to missing submission!' + else: + msg += 'automatic 0 due to corrupt submission!' + else: + msg += 'PATCHSET APPLIES' + + msg += '\n\n' + return msg + + +def check_signed_off_by(repo, tag): + [_, _, user] = tag.split('_') + hostname = os.getenv("HOSTNAME") + + msg = 'signed off by check' + msg += '\n' + msg += '-------------------' + msg += '\n\n' + + commits = reversed(repo.git.execute(['git', 'rev-list', tag]).split('\n')) + + expected_dco = f'Signed-off-by: {user} <{user}@{hostname}>' + + missing = [] + malformed = [] + for i, commit in enumerate(commits): + patch = repo.git.execute(['git', 'show', commit]) + + match = re.search(r'^\s+(Signed-off-by:\s+.+\s+<.+>)$', patch, re.MULTILINE) + if match: + found_dco = match.group(1) + if expected_dco == found_dco: + continue + malformed.append(f'malformed line {found_dco} in patch {i}\n') + else: + missing.append(f'{i}') + + if (n := len(missing)) > 0: + msg += f'Signed-off-by: missing from patch{"es" if n > 1 else ""} {",".join(missing)}\n' + for mal in malformed: + msg += mal + + if len(missing) == len(malformed) == 0: + msg += 'ALL PATCHES SIGNED OFF CORRECTLY\n' + + msg += '\n' + return msg + + +def check_diffstat(repo, tag): + msg = 'diffstat check' + msg += '\n' + msg += '--------------' + msg += '\n\n' + + root = repo.git.execute(['git', 'rev-list', '--max-parents=0', tag]) + calculated_diffstat = repo.git.execute(['git', 'diff', '--stat', '--summary', f'{root}..{tag}']) + cover = repo.git.execute(['git', 'show', root]) + cover_lines = [line.strip() for line in cover.split('\n')] + + rev_diffstat = [] + last = None + collect = False + for i in reversed(cover_lines): + if collect: + if len(i) == 0: + break + rev_diffstat.append(f' {i}') + if len(i) == 0 and last == '--': + collect = True + last = i + + cover_diffstat = '\n'.join(reversed(rev_diffstat)) + diff_diffstat = '\n'.join(difflib.unified_diff(calculated_diffstat.split(), cover_diffstat.split())) + + msg += 'diffstat diff' + msg += '\n' + msg += diff_diffstat if len(diff_diffstat) > 0 else 'NO DIFFERENCE: DIFFSTAT VERIFIED' + msg += '\n\n' + + return msg + + +def check_subject_tag(repo, tag): + [_, component, _] = tag.split('_') + + msg = 'subject tag check' + msg += '\n' + msg += '-----------------' + msg += '\n\n' + + commits = reversed(repo.git.execute(['git', 'rev-list', tag]).split('\n')) + # from 0/n .. n/n + expected_max_index = str(len(list(copy.copy(commits))) - 1) + + rfc_offset = 0 + found_version = None + bad = {} + for i, commit in enumerate(commits): + patch = repo.git.execute(['git', 'show', commit]) + + match = re.search(r'^\s*\[(.*)\].*', patch, re.MULTILINE) + if not match: + msg += f'patch {i} no tag found!\n' + bad[i] = True + continue + subj_tag = match.group(1) + msg += f'patch {i} tag {subj_tag}\n' + subj_tag_parts = subj_tag.split(' ') + match component: + case 'initial' if subj_tag_parts[0] == 'RFC': + msg += 'initial submission correctly labeled RFC\n' + rfc_offset = 1 + case 'initial' if subj_tag_parts[0] == 'PATCH': + msg += 'initial submission missing RFC!\n' + bad[i] = True + case 'final' if subj_tag_parts[0] == 'RFC': + msg += 'final submission incorrectly labeled RFC!\n' + rfc_offset = 1 + bad[i] = True + case 'final' if subj_tag_parts[0] == 'PATCH': + msg += 'final submission correctly non RFC\n' + + if subj_tag_parts[rfc_offset] != 'PATCH': + msg += 'tag missing "PATCH" in expected place\n' + bad[i] = True + + if found_version is None: + found_version = subj_tag_parts[rfc_offset+1] + elif (this_version := subj_tag_parts[rfc_offset+1]) != found_version: + msg += f'tag version mismatch: expected {found_version} found {this_version}!\n' + bad[i] = True + + [this_index, max_index] = subj_tag_parts[rfc_offset+2].split('/') + if this_index != str(i): + msg += f'patch index mismatch: expected {i} found {this_index}' + bad[i] = True + + if max_index != expected_max_index: + msg += f'patch max index mismatch: expected {expected_max_index} found {max_index}' + bad[i] = True + + if not any(bad): + msg += 'ALL SUBJECT TAGS IN CORRECT FORMAT\n' + + return msg + + +def run_automated_checks(tags, peer=False): + with tempfile.TemporaryDirectory() as repo_path: + repo = git.Repo.clone_from(PULL_URL, repo_path) + + remote = repo.create_remote(REMOTE_NAME, PUSH_URL) + remote.fetch('refs/notes/*:refs/notes/*') + configure_repo(repo) + + for tag in tags: + msg = 'Automated tests by denis' + msg += '\n\n' + msg += check_corrupt_or_missing(repo, tag) + + if msg[-3] != '!' and not peer: + msg += '\n\n' + msg += check_diffstat(repo, tag) + msg += check_signed_off_by(repo, tag) + msg += check_subject_tag(repo, tag) + + repo.git.execute(['git', 'notes', '--ref=denis', 'add', tag, '-m', msg]) + + remote.push('refs/notes/*:refs/notes/*') diff --git a/git/admin.sh b/git/admin.sh index f3a9e46e..bbd5189a 100755 --- a/git/admin.sh +++ b/git/admin.sh @@ -1,6 +1,6 @@ #!/bin/sh set -e -DOCKER_COMPOSE=${DOCKER_COMPOSE:-podman-compose} +PODMAN_COMPOSE=${PODMAN_COMPOSE:-podman-compose} -$DOCKER_COMPOSE exec git ./create-repo.sh "$@" +$PODMAN_COMPOSE exec git ./create-repo.sh "$@" diff --git a/mailman/db.py b/mailman/db.py index 4b48ace5..978b351e 100755 --- a/mailman/db.py +++ b/mailman/db.py @@ -27,7 +27,7 @@ class Gradeable(BaseModel): user = peewee.TextField() assignment = peewee.TextField() component = peewee.TextField() - status = peewee.TextField(null=True) + auto_feedback = peewee.TextField(null=True) if __name__ == '__main__': diff --git a/mailman/inspector.py b/mailman/inspector.py index 95dce1a1..69da6fe2 100755 --- a/mailman/inspector.py +++ b/mailman/inspector.py @@ -78,7 +78,7 @@ def gradables(assignment, username, component): for gbl in query: print(gbl.submission_id, datetime.fromtimestamp(gbl.timestamp).astimezone().isoformat(), - gbl.assignment, gbl.user, gbl.component, gbl.status) + gbl.assignment, gbl.user, gbl.component, gbl.auto_feedback) def missing(assignment): diff --git a/mailman/patchset.py b/mailman/patchset.py index 7738f05d..ae9ebf45 100644 --- a/mailman/patchset.py +++ b/mailman/patchset.py @@ -1,6 +1,8 @@ +import re import git import pathlib import sys +import ast import tempfile REMOTE_PUSH_URL = 'http://git:8000/cgi-bin/git-receive-pack/grading.git' @@ -19,10 +21,9 @@ def try_or_false(do, exc): return False -def tag_and_push(repo_path, tag_name): +def tag_and_push(repo, tag_name, msg=None): try: - repo = git.Repo(repo_path) - repo.create_tag(tag_name) + repo.create_tag(tag_name, message=msg) repo.create_remote('grading', REMOTE_PUSH_URL) repo.git.push('grading', tags=True) return True @@ -31,14 +32,11 @@ def tag_and_push(repo_path, tag_name): return False -author_args = ['-c', 'user.name=mailman', '-c', - 'user.email=mailman@mailman'] git_am_args = ['git', '-c', 'advice.mergeConflict=false', - *author_args, 'am', '--keep'] + 'am', '--keep'] -def do_check(repo_path, cover_letter, patches): - repo = git.Repo.init(repo_path) +def do_check(repo, cover_letter, patches, asn): whitespace_errors = [] def am_cover_letter(keep_empty=True): @@ -51,15 +49,68 @@ def am_cover_letter(keep_empty=True): git.GitCommandError): return "missing cover letter!" - repo.git.execute(["git", *author_args, "am", "--abort"]) + repo.git.execute(["git", "am", "--abort"]) if not try_or_false(lambda: am_cover_letter(keep_empty=True), git.GitCommandError): return ("missing cover letter and " "first patch failed to apply!") + rubric = None + if rubric_text := asn.rubric: + try: + rubric = ast.literal_eval(rubric_text) + except SyntaxError: + pass + + # check for correct # of patches in patchset + if rubric and (rubric_count := len(rubric)) != (sub_count := len(patches)): + return f'patch count {sub_count} violates expected rubric patch count of {rubric_count}!' + for i, patch in enumerate(patches): patch_abspath = str(maildir / patch.msg_id) + with open(patch_abspath, 'r') as patch_file: + patch_content = patch_file.read() + match = re.search(r'^\s+Signed-off-by:\s+.+\s+<(.+)@.+>$', patch_content, re.MULTILINE) + if match: + found_author = match.group(1) + else: + return f'illegal patch {i+1}: missing Signed-off-by line!' + + changelines = list(filter(lambda line: line.startswith('--- ') or line.startswith('+++ '), patch_content.split('\n'))) + for change in changelines: + file = change.split(' ')[1].strip() + if file == '/dev/null': + continue + first_dir = file.split('/')[1] + if first_dir != found_author: + file_fixed = file[2:] + return f'illegal patch {i+1}: permission denied for path {file_fixed}!' + + # we assume first level directory is correct by this point + # so we can replace it with some random template value + # requirments: file is either being created of modified + # therefore we assume some second of a pair contains the template author + # rubric is a list of dictionaries mapping changepair tuples + # to an integer counting the number of times that changepair is found + if rubric: + other = list(rubric[0].keys())[0][1].split('/')[1] + for j in range(int(len(changelines)/2)): + changepair = [changelines[2*j], changelines[2*j+1]] + if changepair[0] != '--- /dev/null/': + changepair[0] = changepair[0].replace(found_author, other) + if changepair[1] != '+++ /dev/null/': + changepair[1] = changepair[1].replace(found_author, other) + try: + rubric[i][tuple(changepair)] += 1 + except KeyError: + pass + # a changepair (e.g. ('--- fromfile', '+++ tofile') may appear + # more than in the rubric o if modifications to a file + # are more sparse than in the example used to generate the rubric + if any(count < 1 for count in rubric[i].values()): + return f'patch {i+1} violates the assignment rubric!' + # Try and apply and fail if there are whitespace errors def do_git_am(extra_args=[]): repo.git.execute([*git_am_args, *extra_args, patch_abspath]), @@ -69,7 +120,7 @@ def do_git_am(extra_args=[]): git.GitCommandError): continue - repo.git.execute(["git", *author_args, "am", "--abort"]) + repo.git.execute(["git", "am", "--abort"]) # Try again, if we succeed, count this patch as a whitespace error if try_or_false(lambda: do_git_am(), git.GitCommandError): @@ -80,29 +131,32 @@ def do_git_am(extra_args=[]): return f'patch {i+1} failed to apply!' if whitespace_errors: - return ('whitespace error patch(es) ' + return (f'whitespace error patch{"es" if len(whitespace_errors) > 1 else ""} ' f'{",".join(whitespace_errors)}?') else: return 'patchset applies.' -def check(cover_letter, patches, submission_id): +def check(cover_letter, patches, submission_id, asn): with tempfile.TemporaryDirectory() as repo_path: - status = do_check(repo_path, cover_letter, patches) - if status[-1] == '!': - repo = git.Repo(repo_path) + repo = git.Repo.init(repo_path) + with repo.config_writer() as config: + config.set_value('user', 'name', 'mailman') + config.set_value('user', 'email', 'mailman@mailman') + auto_feedback = do_check(repo, cover_letter, patches, asn) + if auto_feedback[-1] == '!': for patch in patches: patch_abspath = str(maildir / patch.msg_id) - repo.git.execute(['git', *author_args, 'commit', '--allow-empty', '-F', patch_abspath]) - tag_and_push(repo_path, submission_id) - return status + repo.git.execute(['git', 'commit', '--allow-empty', '-F', patch_abspath]) + tag_and_push(repo, submission_id, msg=auto_feedback) + return auto_feedback def apply_peer_review(email, submission_id, review_id): args = [*git_am_args, '--empty=keep'] patch_abspath = str(maildir / email.msg_id) - status = 'sucessfully stored peer review' + auto_feedback = 'sucessfully stored peer review' with tempfile.TemporaryDirectory() as repo_path: try: @@ -110,10 +164,13 @@ def apply_peer_review(email, submission_id, review_id): multi_options=[f'--branch={review_id}', '--single-branch', '--no-tags']) + with repo.config_writer() as config: + config.set_value('user', 'name', 'mailman') + config.set_value('user', 'email', 'mailman@mailman') repo.git.execute([*args, patch_abspath]) - tag_and_push(repo_path, submission_id) + tag_and_push(repo, submission_id) except git.GitCommandError as e: print(e, file=sys.stderr) - status = 'failed to apply peer review' + auto_feedback = 'failed to apply peer review' - return status + return auto_feedback diff --git a/mailman/submit.py b/mailman/submit.py index c7f69f3b..9f52bd12 100755 --- a/mailman/submit.py +++ b/mailman/submit.py @@ -65,7 +65,7 @@ def set_status(status): gr_db = db.Gradeable if asn := asn_db.get_or_none(asn_db.name == emails[0].rcpt): cover_letter, *patches = emails - status = patchset.check(cover_letter, patches, logfile) + auto_feedback = patchset.check(cover_letter, patches, logfile, asn) if len(emails) < 2: return set_status('missing patches') typ = ('initial' if timestamp < asn.initial_due_date @@ -73,11 +73,11 @@ def set_status(status): if not typ: return set_status(f'{asn.name} past due') gr_db.create(submission_id=logfile, timestamp=timestamp, user=user, - assignment=asn.name, component=typ, status=status) + assignment=asn.name, component=typ, auto_feedback=auto_feedback) return set_status(f'{asn.name}: {typ}') if reply_id: - status = patchset.apply_peer_review(emails[0], logfile, reply_id) + auto_feedback = patchset.apply_peer_review(emails[0], logfile, reply_id) if not (orig := gr_db.get_or_none(gr_db.submission_id == reply_id)): return set_status('not a reply to a submission') asn_name = orig.assignment @@ -100,7 +100,7 @@ def set_status(status): return set_status('reviewed wrong submission') gr_db.create(submission_id=logfile, timestamp=timestamp, user=user, assignment=asn_name, component=typ, - status=status) + auto_feedback=auto_feedback) return set_status(f'{asn_name}: {typ}') return set_status('Not a recognized recipient') diff --git a/orbit/Containerfile b/orbit/Containerfile index f083a0d7..9927359d 100644 --- a/orbit/Containerfile +++ b/orbit/Containerfile @@ -18,6 +18,7 @@ FROM alpine:3.20 AS orbit RUN apk update && apk upgrade && apk add \ py3-bcrypt \ + py3-gitpython \ py3-peewee \ py3-markdown \ uwsgi-python3 \ diff --git a/orbit/radius.py b/orbit/radius.py index ab26f8f0..e9e7a6de 100644 --- a/orbit/radius.py +++ b/orbit/radius.py @@ -4,6 +4,7 @@ import base64 import bcrypt +import git import html import markdown import os @@ -425,7 +426,7 @@ class OopsStatus: class AsmtTable: def __init__(self, assignment, oopsieness, peer1, peer2, init, - review1, review2, final): + review1, review2, final, user): self.assignment = assignment self.name = assignment.name self.oopsieness = oopsieness @@ -435,6 +436,10 @@ def __init__(self, assignment, oopsieness, peer1, peer2, init, self.review1 = review1 self.review2 = review2 self.final = final + self.user = user + self.review1_grade = None + self.review2_grade = None + self.final_grade = None def oops_button_hover(self): match self.oopsieness: @@ -448,6 +453,72 @@ def oops_button_hover(self): case OopsStatus.UNAVAILABLE: return "You have already used your oopsie" + def get_automated_feedback(self, attr): + if attr not in ['init', 'final'] or (gbl := getattr(self, attr)) is None: + return 'No submission' + + match attr: + case 'init': + due_date = int(self.assignment.initial_due_date) + case 'final': + due_date = int(self.assignment.final_due_date) + + if due_date < int(datetime.now().timestamp()): + return gbl.auto_feedback + + match gbl.auto_feedback[-1]: + case '.': + return 'Submission accepted' + case '?': + return 'Issues detected in patchset' + case '!': + return 'Submission rejected' + case _: + return '---' + + def get_human_feedback(self): + tag = f'{self.name}_final_{self.user}' + repo = git.Repo('/var/lib/git/grading.git') + try: + return repo.git.execute(['git', 'notes', '--ref=feedback', 'show', tag]) + except git.GitCommandError: + return '-' + + def get_grade(self, attr): + tag = f'{self.name}_{attr}_{self.user}' + repo = git.Repo('/var/lib/git/grading.git') + if attr not in ['final', 'review1', 'review2'] or getattr(self, attr) is None: + try: + return repo.git.execute(['git', 'notes', '--ref=grade', 'show', tag]) + except git.GitCommandError: + return '-' + if (cached_grade := getattr(self, f'{attr}_grade')) is not None: + return cached_grade + try: + grade = repo.git.execute(['git', 'notes', '--ref=grade', 'show', tag]) + setattr(self, f'{attr}_grade', grade) + return grade + except git.GitCommandError: + return '-' + + def get_total_score(self): + oopsie_used_here = self.oopsieness == OopsStatus.USED_HERE + final_grade = self.get_grade('final') + + if oopsie_used_here: + return final_grade + + review1_grade = self.get_grade('review1') + review2_grade = self.get_grade('review2') + + if '-' in [final_grade, review1_grade, review2_grade]: + return '-' + + try: + return format(int(final_grade) * .8 + int(review1_grade) * .1 + int(review2_grade) * .1, '.1f') + except ValueError: + return "???" + def gradeable_row(self, item_name, gradeable, rightmost_col): return f"""
| Total Score: - | +Total Score: {self.get_total_score()} | Timestamp | Submission ID | Request an 'Oopsie' | @@ -594,7 +673,7 @@ def handle_dashboard(rocket): rev2 = asn_gradeables.where(grd_tbl.component == 'review2').first() final = asn_gradeables.where(grd_tbl.component == 'final').first() ret += str(AsmtTable(assignment, oopsieness, peer1, peer2, init, rev1, - rev2, final)) + rev2, final, rocket.session.username)) return rocket.respond(ret + '', 'Dashboard') diff --git a/orbit/warpdrive.sh b/orbit/warpdrive.sh index 8d92572d..c78e8158 100755 --- a/orbit/warpdrive.sh +++ b/orbit/warpdrive.sh @@ -4,6 +4,6 @@ set -e -DOCKER_COMPOSE=${DOCKER_COMPOSE:-podman-compose} +PODMAN_COMPOSE=${PODMAN_COMPOSE:-podman-compose} -$DOCKER_COMPOSE exec orbit ./hyperspace.py "$@" +$PODMAN_COMPOSE exec orbit ./hyperspace.py "$@" diff --git a/script-lint.sh b/script-lint.sh index 10cf94fa..2731cad9 100755 --- a/script-lint.sh +++ b/script-lint.sh @@ -5,8 +5,12 @@ require shellcheck set -ex +# -x needed to make shellcheck follow `source` command shellcheck script-lint.sh -shellcheck test.sh +shellcheck -x test.sh +shellcheck -x test-sub.sh +shellcheck -x test-sub-check.sh +shellcheck -x test-sub2.sh shellcheck orbit/warpdrive.sh shellcheck denis/configure.sh shellcheck mailman/inspector.sh @@ -16,6 +20,5 @@ shellcheck git/setup-repo.sh shellcheck git/cgi-bin/git-receive-pack shellcheck git/hooks/post-update -# -x needed to make shellcheck follow `source` command shellcheck -x backup/backup.sh shellcheck -x backup/restore.sh diff --git a/start.sh b/start.sh index 574df7f4..504c0028 100755 --- a/start.sh +++ b/start.sh @@ -11,6 +11,27 @@ podman-compose logs -f submatrix 2>&1 | sed '/Synapse now listening on TCP port if [ -f test.sh ] then ./test.sh + if [ -f test-sub.sh ] + then + podman-compose down + yes | podman volume prune + podman-compose up -d + podman-compose logs -f submatrix 2>&1 | sed '/Synapse now listening on TCP port 8008/ q' + ./dev_sockets.sh & + git config --global user.name PINP + git config --global user.email podman@podman + ./test-sub.sh + ./test-sub-check.sh + podman-compose down + yes | podman volume prune + podman-compose up -d + ./test-sub2.sh + podman-compose down + yes | podman volume prune + podman-compose up -d + ./test-sub3.sh + + fi else virtualenv . pip install -r requirements.txt diff --git a/test-lib b/test-lib new file mode 100644 index 00000000..1b9607fb --- /dev/null +++ b/test-lib @@ -0,0 +1,195 @@ +# This line: +# - aborts the script after any pipeline returns nonzero (e) +# - shows all commands as they are run (x) +# - sets any dereference of an unset variable to trigger an error (u) +# - causes the return value of a pipeline to be the nonzero return value +# of the furthest right failing command or zero if no command failed (o pipefail) +set -exuo pipefail + +PODMAN=${PODMAN:-podman} +PODMAN_COMPOSE=${PODMAN_COMPOSE:-podman-compose} +SCRIPT_DIR=$(dirname "$(readlink -f "$0")") +WORKDIR=$(mktemp -d) + +HOSTNAME_FROM_DOTENV="$(sh -c ' +set -o allexport +. ./.env +exec jq -r -n "env.SINGULARITY_HOSTNAME" +')" + +SINGULARITY_HOSTNAME=${SINGULARITY_HOSTNAME:-"${HOSTNAME_FROM_DOTENV}"} + +setup_testdir() { + # Create test dir if it does not exist yet + mkdir -p test + + # Reset the test directory + rm -f test/* + + # put the cert in there + ${PODMAN} cp singularity_nginx_1:/etc/ssl/nginx/fullchain.pem test/ca_cert.pem +} + +CURL_OPTS=( \ +--verbose \ +--cacert test/ca_cert.pem \ +--fail \ +--no-progress-meter \ +) + + +get_git_port() { ${PODMAN} port singularity_git_1 | awk -F':' '{ print $2 }' ; } ; + +# preconditions: +# - SCRIPT_DIR defined (singularity repo root) +# - WORKDIR defined (arbitrary) temp directory +setup_submissions_and_grading_repo() { + pushd "$SCRIPT_DIR" + # nuke grading repo inside container + # shellcheck disable=SC2016 + ${PODMAN_COMPOSE} exec git ash -c 'cd /var/lib/git/grading.git && for t in $(git tag); do git tag -d $t; done' + # crete and push fresh submissions repo + rm -rf repos/submissions + git/admin.sh submissions "course submissions repository" + pushd repos + git init --bare submissions + echo "course submissions repository" > submissions/description + # create a temporary workdir to push an initial commit to the submission repo + git init submissions_init + pushd submissions_init + echo "# submissions" > README.md + git add README.md + git status + git -c user.name=singularity -c user.email=singularity@singularity commit -sm 'init submissions repo' + git push ../submissions master + popd + rm -rf submissions_init + pushd submissions + git push --mirror http://localhost:"$(get_git_port)"/cgi-bin/git-receive-pack/submissions + popd + popd + popd + + pushd "$WORKDIR" + mkdir certs + pushd certs + ${PODMAN} volume export singularity_ssl-certs > certs.tar + tar xf certs.tar + popd + git clone http://localhost:"$(get_git_port)"/submissions + popd +} + +create_lighting_assignment() { + test -n "$1" + test -n "$2" + test -n "$3" + test -n "$4" + + local asn="$1" + local i_s="$2" + local p_s="$3" + local f_s="$4" + set +u + local rub="$5" + set -u + + RUBRIC= + if [ -n "$rub" ] + then + RUBRIC="-r $rub" + fi + + pushd "$SCRIPT_DIR" + # create or recreate setup assignment + if denis/configure.sh dump | grep -q "^$asn:"; then + denis/configure.sh remove -a "$asn" + fi + denis/configure.sh create -a "$asn" -i "$(date -d "$i_s secs" +%s)" -p "$(date -d "$p_s secs" +%s)" -f "$(date -d "$f_s secs" +%s)" ${RUBRIC} + denis/configure.sh reload + popd +} + +# preconditions: +# - called after setup_submissions_and_grading_repo +setup_submissions_for() { + test -n "$1" + local user="$1" + + pushd "$SCRIPT_DIR" + orbit/warpdrive.sh -u "$user" -p builder -n || orbit/warpdrive.sh -u "$user" -p builder -m + popd + + pushd "$WORKDIR"/submissions + git config user.name "$user" + git config user.email "$user"@localhost.localdomain + git config sendemail.smtpUser "$user" + git config sendemail.smtpPass builder + git config sendemail.smtpserver localhost.localdomain + git config sendemail.smtpserverport 465 + git config sendemail.smtpencryption ssl + popd +} + +# preconditions: +# - no non-tracked files in submissions repo +# - called after setup_submissions_for +enter_and_checkout() { + test -n "$1" + local branch="$1" + + pushd "$WORKDIR"/submissions + git checkout --orphan "$1" + if [ -n "$(ls)" ] + then + git rm -rf ./* + fi +} + +# preconditions: +# - called after a call to enter_and_checkout +exit_after_sending() { + test -n "$1" + local asn="$1" + + git send-email \ + --confirm=never \ + --smtp-ssl-cert-path="$WORKDIR"/certs/fullchain.pem \ + --to "$asn"@localhost.localdomain \ + ./*.patch + rm ./*.patch + popd + sleep 1 +} + +# preconditions: +# - called after a call to enter_and_checkout +write_commit_to() { + test -n "$1" && test -n "$2" + local content="$1" + local file="$2" + set +u # necessary since $3 is unbound in 2 arg case + local opt="$3" + set -x + + mkdir -p $(dirname "$file") + + if [ "$opt" == "append" ]; then + echo "$content" >> "$file" + else + echo "$content" > "$file" + fi + + git add "$file" + git commit -sm "add $content to $file" +} + +# preconditions: +# - called after a call to write_commit_to +fixup_cover() { + test -n "$1" + sed -i "s/\*\*\* SUBJECT HERE \*\*\*/$1/g" *0000-cover-letter.patch + sed -i "s/\*\*\* BLURB HERE \*\*\*/$1/g" *0000-cover-letter.patch + sed -i "\$a\\Signed-off-by: $(git config user.name) <$(git config user.email)>" *0000-cover-letter.patch +} + diff --git a/test-sub-check.sh b/test-sub-check.sh new file mode 100755 index 00000000..4f49ae3d --- /dev/null +++ b/test-sub-check.sh @@ -0,0 +1,95 @@ +#!/usr/bin/env bash + +source test-lib + +setup_testdir + +# login as bob +curl --url "https://$SINGULARITY_HOSTNAME/login" \ + --unix-socket ./socks/https.sock \ + "${CURL_OPTS[@]}" \ + -c test/cookies \ + --data "username=bob&password=builder" \ + | tee test/login_success \ + | grep "msg = bob authenticated by password" + + +# check for setup assignment and save dashboard +curl --url "https://$SINGULARITY_HOSTNAME/dashboard" \ + --unix-socket ./socks/https.sock \ + -b test/cookies \ + "${CURL_OPTS[@]}" \ + | tee test/dashboard \ + | grep "setup" + +grep "Total Score: 64.9" test/dashboard + +grep "missing cover letter and first patch failed to apply!" test/dashboard + +grep "bib Peer Review" test/dashboard + +grep "bab Peer Review" test/dashboard + +grep "patchset applies." test/dashboard + +grep "Good effort but try harder next time." test/dashboard + +# login as bab +curl --url "https://$SINGULARITY_HOSTNAME/login" \ + --unix-socket ./socks/https.sock \ + "${CURL_OPTS[@]}" \ + -c test/cookies_bab \ + --data "username=bab&password=builder" \ + | tee test/login_success_bab \ + | grep "msg = bab authenticated by password" + + +# check for setup assignment and save dashboard +curl --url "https://$SINGULARITY_HOSTNAME/dashboard" \ + --unix-socket ./socks/https.sock \ + -b test/cookies_bab \ + "${CURL_OPTS[@]}" \ + | tee test/dashboard_bab \ + | grep "setup" + +grep "Total Score: 0.0" test/dashboard_bab + +grep "bib Peer Review" test/dashboard_bab + +grep "bob Peer Review" test/dashboard_bab + +grep "patchset applies." test/dashboard_bab + +grep "illegal patch 1: permission denied for path work!" test/dashboard_bab + +grep 'Please review git' test/dashboard_bab + +# login as bib +curl --url "https://$SINGULARITY_HOSTNAME/login" \ + --unix-socket ./socks/https.sock \ + "${CURL_OPTS[@]}" \ + -c test/cookies_bib \ + --data "username=bib&password=builder" \ + | tee test/login_success_bib \ + | grep "msg = bib authenticated by password" + + +# check for setup assignment and save dashboard +curl --url "https://$SINGULARITY_HOSTNAME/dashboard" \ + --unix-socket ./socks/https.sock \ + -b test/cookies_bib \ + "${CURL_OPTS[@]}" \ + | tee test/dashboard_bib \ + | grep "setup" + +grep "Total Score: 80.0" test/dashboard_bib + +grep "bob Peer Review" test/dashboard_bib + +grep "bab Peer Review" test/dashboard_bib + +grep "patchset applies." test/dashboard_bib + +grep 'Perfect. But no peer review!' test/dashboard_bib + +echo "ALL SUBMISSION TESTS PASS" diff --git a/test-sub.sh b/test-sub.sh new file mode 100755 index 00000000..d02470ff --- /dev/null +++ b/test-sub.sh @@ -0,0 +1,198 @@ +#!/bin/bash + +# assumes singularity is running when invoked +# script is written to be as idempotent as posible + +source test-lib + +setup_submissions_and_grading_repo + +cat << EOF > "$WORKDIR"/setup_rubric +[{('--- /dev/null', '+++ b/bob/setup/work'): 0}, +{('--- a/bob/setup/work', '+++ b/bob/setup/work'): 0}] +EOF + +create_lighting_assignment setup 15 25 30 "$WORKDIR"/setup_rubric + +# setup_intial_bob patchsets +setup_submissions_for bob + +# good patchset +enter_and_checkout setup_initial_bob_good +write_commit_to "abc" bob/setup/work +write_commit_to "def" bob/setup/work "append" +git format-patch --rfc --cover-letter -v1 -2 +fixup_cover "Good patchset" +exit_after_sending setup + +# corrupt patchset +enter_and_checkout setup_initial_bob_corrupt +write_commit_to "abc" bob/setup/work +write_commit_to "def" bob/setup/work "append" +write_commit_to "ghi" bob/setup/work "append" +git format-patch --rfc --cover-letter -v1 -3 +rm v1-0002-*.patch +fixup_cover "Corrupt patchset" +exit_after_sending setup + +# patchset with whitespace errors +enter_and_checkout setup_initial_bob_whitespace +write_commit_to "abc" bob/setup/work +write_commit_to "def " bob/setup/work "append" +git format-patch --rfc --cover-letter -v1 -2 +fixup_cover "Patchset with whitespace errors" +exit_after_sending setup + +# patchset with no cover letter +enter_and_checkout setup_initial_bob_nocover +write_commit_to "abc" bob/setup/work +write_commit_to "def" bob/setup/work "append" +git format-patch --rfc -v1 -2 +exit_after_sending setup + +# patchset with no cover letter and corrupt first patch +enter_and_checkout setup_initial_bob_nocover-corrupt +write_commit_to "abc" bob/setup/work +write_commit_to "def" bob/setup/work "append" +write_commit_to "ghi" bob/setup/work "append" +git format-patch --rfc -v1 -2 +exit_after_sending setup + +# setup_initial_bab submissions +setup_submissions_for bab + +# bab good patchset +enter_and_checkout setup_initial_bab_good +write_commit_to "abc" bab/setup/work +write_commit_to "def" bab/setup/work "append" +git format-patch --rfc --cover-letter -v1 -2 +fixup_cover "Good patchset" +exit_after_sending setup + +# setup_initial_bib submissions +setup_submissions_for bib + +# bib good patchset +enter_and_checkout setup_initial_bib_good +write_commit_to "abc" bib/setup/work +write_commit_to "def" bib/setup/work "append" +git format-patch --rfc --cover-letter -v1 -2 +fixup_cover "Good patchset" +exit_after_sending setup + +# investigate grading repo +pushd "$WORKDIR" +git clone http://localhost:"$(get_git_port)"/grading.git +pushd grading +git fetch --tag + +STATUSES=( +'patchset applies.' +'patch 2 failed to apply!' +'whitespace error patch 2?' +'missing cover letter!' +'missing cover letter and first patch failed to apply!' +'patchset applies.' +'patchset applies.') + +# submitted patchses should have statuses in this order +# assumption: first ID tag is first patch submitted by this script and IDs increase monotonically +i=0 +for t in $(git tag); do + git show -s --oneline "$t" | grep -q "${STATUSES[$i]}" + if [[ $i == 5 ]]; then + PEER1_ID=$t + elif [[ $i == 6 ]]; then + PEER2_ID=$t + fi + i=$((i+1)) +done +popd + +echo "wait for initial submission deadline (5 secs)" +sleep 5 + +setup_submissions_for bob + +# setup bob peer review +enter_and_checkout setup_reviews_bob + +# submit peer review 1 +cat <
|---|