4 commits: 3 unchanged, 1 rewritten.
@@ Commit message
comment|approve, as the user, with the message on stdin: what ssh's
git-shell-commands run. So the comment file, the approve line, the
rules (no self-approval, comments on an older revision, the 32 KiB
- cap), the event and the rebuild are the same, byte for byte.
+ cap), the event and the rebuild are the same, byte for byte. With
+ FORGE_VIA=web, which the server sets, the two refusals that tell you to
+ check out or push (a review that moved on since the page was built, a
+ tip it never had) say to reload the page instead; over ssh they read as
+ before.
A private repo you can't read is the missing repo's 404, as for its
pages. Otherwise a post takes a user: a session, from the forge's own
@@ Commit message
Caddy sends these posts to the private server before it looks in www,
so a public repo's review, which is static files, takes them too.
+ The scenarios' HTTP client reads -d @file as curl does, for a body
+ longer than Linux lets one argument be.
+
## bin/forge ##
# their release downloads (sent as attachments by that server, and left
@@ bin/forge: EOF
@public file {
root $FORGE_WWW
try_files {path} {path}/
+@@ bin/forge: review_changed() {
+ #
+ # Stored as <repo>.git/forge/comments/<review>/<n>: header lines, a blank
+ # line, the message. A line comment records the line's text too, so it still
+-# reads right once the review moves on.
++# reads right once the review moves on. With FORGE_VIA=web (a review page,
++# through lib/private-server.ts), comment and approve word a refusal for a
++# page: reload it, rather than push or check out.
+ sc_comment() {
+ [ $# -ge 2 ] || die "usage: comment <owner>/<repo> <id|branch> [--at <sha>] [--line <path>:<line>] <message"
+ open_review "$1" "$2"
+@@ bin/forge: sc_comment() {
+ # You commented on the revision you have checked out; if the review has
+ # moved on since, the comment belongs to that older revision.
+ if [ -n "$at" ] && [ "$at" != "$RTIP" ]; then
+- rev_of "$at" >/dev/null || die "$(printf %.7s "$at") isn't a pushed revision of #$RID; push it, or name the review by number"
++ if ! rev_of "$at" >/dev/null; then
++ [ "${FORGE_VIA:-}" != web ] || die "$(printf %.7s "$at") isn't a revision of #$RID; reload the page and comment again"
++ die "$(printf %.7s "$at") isn't a pushed revision of #$RID; push it, or name the review by number"
++ fi
+ sha=$at
+ fi
+ path= line= text=
+@@ bin/forge: sc_approve() {
+ [ "$RLIVE" = 1 ] || die "#$RID is closed"
+ [ "$user" != "$RAUTHOR" ] || die "you opened #$RID; an approval has to come from someone else"
+ rev=$(rev_of "$RTIP" || true)
+- [ -z "$at" ] || [ "$at" = "$RTIP" ] ||
+- die "#$RID has moved on to ${rev:+r$rev }$(printf %.7s "$RTIP") since $(printf %.7s "$at"); run forge checkout $RID and look again"
++ if [ -n "$at" ] && [ "$at" != "$RTIP" ]; then
++ moved="#$RID has moved on to ${rev:+r$rev }$(printf %.7s "$RTIP") since $(printf %.7s "$at")"
++ [ "${FORGE_VIA:-}" != web ] || die "$moved; reload the page to see what changed, and approve that"
++ die "$moved; run forge checkout $RID and look again"
++ fi
+ tree=$(git --git-dir="$RDIR" rev-parse "$RTIP^{tree}")
+ if awk -v u="$user" -v t="$tree" '$1 == "approve" && $3 == u && $5 == t { f = 1 } END { exit !f }' "$RF"; then
+ echo "you already approved this tree of #$RID"
## lib/login.ts ##
@@ lib/private-server.ts: export class Gate {
+
+export function shellCmd(cfg: PrivateConfig, user: string, args: string[], input: string): Promise<{ code: number | null; out: string; err: string }> {
+ return new Promise((ok, no) => {
-+ const env: NodeJS.ProcessEnv = { ...process.env, FORGE_ROOT: cfg.root, FORGE_USER: user };
++ // FORGE_VIA: refusals worded for a page (reload it), not for a checkout.
++ const env: NodeJS.ProcessEnv = { ...process.env, FORGE_ROOT: cfg.root, FORGE_USER: user, FORGE_VIA: 'web' };
+ if (cfg.baseUrl) env.FORGE_BASE_URL = cfg.baseUrl;
+ const p = spawn(FORGE_BIN, ['shell-cmd', ...args], { cwd: cfg.root, env, stdio: ['pipe', 'pipe', 'pipe'], timeout: 5 * 60 * 1000 });
+ const out: Buffer[] = [];
@@ lib/private-server.ts: export function privateHandler(cfg: PrivateConfig): http.
else if (url.pathname === '/private/' && get) listPage(req, res);
else if (url.pathname === '/login' || url.pathname === '/private') res.writeHead(308, { Location: url.pathname + '/' + url.search }).end();
+ ## test/http.ts ##
+@@
+ // -w <format> then format, with %{http_code} and %{redirect_url}
+ // -H 'Name: value' a request header (more than one: repeat it)
+ // -X <method> the request's method
+-// -d <data> a POST body, form-encoded unless -H says otherwise
++// -d <data> a POST body, form-encoded unless -H says otherwise; @file reads it from file
+ // -u user:password basic auth
+ // --max-time <s> give up after that long (exit 28, as curl)
+ // -s, -N accepted, and what it does anyway: no progress, no buffering
+@@ test/http.ts: for (let i = 0; i < args.length; i++) {
+ else if (a === '-I') head = true;
+ else if (a === '-w') format = next();
+ else if (a === '-X') method = next();
+- else if (a === '-d') data = next();
++ else if (a === '-d') {
++ // @file: the file's contents less carriage returns and newlines, as curl
++ // reads it, for bodies longer than an argument may be.
++ const d = next();
++ data = d.startsWith('@') ? fs.readFileSync(d.slice(1), 'utf8').replace(/[\r\n]/g, '') : d;
++ }
+ else if (a === '--max-time') maxTime = Number(next());
+ else if (a === '-u') headers.push(['Authorization', `Basic ${Buffer.from(next()).toString('base64')}`]);
+ else if (a === '-H') {
+
## test/lib.sh ##
# sync_builds hooks and ssh commands build pages before returning
@@ test/scenarios/web-comment-limit.sh (new)
+has "$T/res" '"error":"That is a lot at once: wait a minute"'
+eq "$(approve "$cat")" 200
+
-+big() { node -e 'process.stdout.write("body=" + "a".repeat(Number(process.argv[1])))' "$1"; }
-+code() { http -s -o "$T/res" -w '%{http_code}' -H "Authorization: Bearer $cat" -d "$(big "$1")" "$PRIV/dami/greet/reviews/1/comment"; }
++# The body from a file: Linux takes no argument longer than 128 KiB.
++code() {
++ node -e 'process.stdout.write("body=" + "a".repeat(Number(process.argv[1])))' "$1" >"$T/body"
++ http -s -o "$T/res" -w '%{http_code}' -H "Authorization: Bearer $cat" -d "@$T/body" "$PRIV/dami/greet/reviews/1/comment"
++}
+eq "$(code 32768)" 400
+has "$T/res" '"error":"comment: longer than 32 KiB"'
+eq "$(code 200000)" 413
@@ test/scenarios/web-comment-outdated.sh (new)
+# A review page posts the tip it shows. When the review has moved on since
+# the page was built, a comment from it belongs to the revision it showed,
+# as one from a checkout of that revision does, and an approval from it is
-+# refused with what forge approve says. A tip the review never had is
-+# refused too.
++# refused. A tip the review never had is refused too. The forge words
++# these refusals for a page (reload it); over ssh they still say to check
++# out or push.
+. "$(dirname "$0")/../lib.sh"
+user dami
+user ana
@@ test/scenarios/web-comment-outdated.sh (new)
+has "$d/2" '^rev 2$'
+
+post approve "at=$r1" >"$T/res"
-+has "$T/res" "\"error\":\"#1 has moved on to r2 $(printf %.7s "$r2") since $(printf %.7s "$r1"); run forge checkout 1 and look again\"\\} 400\$"
++has "$T/res" "\"error\":\"#1 has moved on to r2 $(printf %.7s "$r2") since $(printf %.7s "$r1"); reload the page to see what changed, and approve that\"\\} 400\$"
++fails_with "^forge: #1 has moved on to r2 $(printf %.7s "$r2") since $(printf %.7s "$r1"); run forge checkout 1 and look again\$" ssh_as ana approve dami/greet 1 --at "$r1"
+lacks "$FORGE_ROOT/dami/greet.git/forge/reviews/1" '^approve '
+main=$(git --git-dir="$FORGE_ROOT/dami/greet.git" rev-parse main)
+post comment "body=Nowhere&at=$main" >"$T/res"
-+has "$T/res" "\"error\":\"$(printf %.7s "$main") isn't a pushed revision of #1; push it, or name the review by number\"\\} 400\$"
++has "$T/res" "\"error\":\"$(printf %.7s "$main") isn't a revision of #1; reload the page and comment again\"\\} 400\$"
++echo Nowhere | fails_with "^forge: $(printf %.7s "$main") isn't a pushed revision of #1; push it, or name the review by number\$" ssh_as ana comment dami/greet 1 --at "$main"
+post comment "body=Nowhere&at=nope" >"$T/res"
+has "$T/res" '"error":"comment: at is not a commit id"\} 400$'
+post approve "at=$r2" >"$T/res"