"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: make gotd reject sha2 repos earlier
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Wed, 7 Aug 2024 16:08:02 +0200

Download raw body.

Thread
On Wed, Aug 07, 2024 at 03:01:03PM +0200, Omar Polo wrote:
> > +	/*
> > +	 * SHA2 repositories cannot be used with gotd until Git protocol v2
> > +	 * support is added. Reject them at startup for now.
> > +	 */
> > +	TAILQ_FOREACH(repo, &gotd.repos, entry) {
> > +		struct got_repository *r;
> > +
> > +		error = got_repo_open(&r, repo->path, NULL, NULL);
> > +		if (error)
> > +			fatalx("%s: %s", repo->path, error->msg);
> > +
> > +		if (got_repo_get_object_format(r) != GOT_HASH_SHA1) {
> > +			error = got_error_msg(GOT_ERR_NOT_IMPL,
> > +			    "sha256 object IDs unsupported in network "
> > +			    "protocol");
> > +			fatalx("%s: %s", repo->path, error->msg);
> > +		}
> > +
> > +		got_repo_close(r);
> > +	}
> 
> didn't we also allowed repositories to be listed but not yet created?
> or maybe i'm remembering wrong.

In theory, yes, the intention was to do this. Thanks for reminding me.
However, it turns out I already broke this functionality back in commit
  4b3827cd43394b89d2af822dcd1d9a9179c1ee10
  "make gitwrapper ignore 'permission denied' for repository paths"

Since then parse.y runs yyerror() when a repository does not exist,
causing gotd to exit on startup. Which can be fixed separately.

Here is a new version of the proposed diff which ignores ENOENT.

-----------------------------------------------
 make gotd(8) reject sha256 repositories at startup for now
 
diff a5f79a896f1d163672b9b2e4c9a7064ebc4936f9 8b093277201726214cfc0a89bec6c19d0b56bf7f
commit - a5f79a896f1d163672b9b2e4c9a7064ebc4936f9
commit + 8b093277201726214cfc0a89bec6c19d0b56bf7f
blob - e087575f78236937985aeb124917d2ff9962a9de
blob + 4d75296b687289f7e3ab2a674a5ad53b3b7c8601
--- gotd/gotd.c
+++ gotd/gotd.c
@@ -2093,6 +2093,30 @@ main(int argc, char **argv)
 	if (pw->pw_uid == 0)
 		fatalx("cannot run %s as the superuser", getprogname());
 
+	/*
+	 * SHA2 repositories cannot be used with gotd until Git protocol v2
+	 * support is added. Reject them at startup for now.
+	 */
+	TAILQ_FOREACH(repo, &gotd.repos, entry) {
+		struct got_repository *r;
+
+		error = got_repo_open(&r, repo->path, NULL, NULL);
+		if (error) {
+			if (error->code == GOT_ERR_ERRNO && errno == ENOENT)
+				continue;
+			fatalx("%s: %s", repo->path, error->msg);
+		}
+
+		if (got_repo_get_object_format(r) != GOT_HASH_SHA1) {
+			error = got_error_msg(GOT_ERR_NOT_IMPL,
+			    "sha256 object IDs unsupported in network "
+			    "protocol");
+			fatalx("%s: %s", repo->path, error->msg);
+		}
+
+		got_repo_close(r);
+	}
+
 	if (noaction) {
 		fprintf(stderr, "configuration OK\n");
 		return 0;
blob - 10b2b156100b2de5d6fa3f09483b5ac4658b3074
blob + 9f370db70d268838b93dbe6d96b498def97df472
--- lib/read_gitconfig.c
+++ lib/read_gitconfig.c
@@ -111,13 +111,8 @@ got_repo_read_gitconfig(int *gitconfig_repository_form
 	tags = got_gitconfig_get_tag_list(gitconfig, "extensions");
 	if (extnames && extvals && nextensions && tags) {
 		size_t numext = 0;
-		TAILQ_FOREACH(node, &tags->fields, link) {
-			char *ext = node->field;
-			char *val = got_gitconfig_get_str(gitconfig,
-			    "extensions", ext);
-			if (get_boolean_val(val))
-				numext++;
-		}
+		TAILQ_FOREACH(node, &tags->fields, link)
+			numext++;
 		*extnames = calloc(numext, sizeof(char *));
 		if (*extnames == NULL) {
 			err = got_error_from_errno("calloc");
@@ -132,23 +127,21 @@ got_repo_read_gitconfig(int *gitconfig_repository_form
 			char *ext = node->field;
 			char *val = got_gitconfig_get_str(gitconfig,
 			    "extensions", ext);
-			if (get_boolean_val(val)) {
-				char *extstr = NULL, *valstr = NULL;
+			char *extstr = NULL, *valstr = NULL;
 
-				extstr = strdup(ext);
-				if (extstr == NULL) {
-					err = got_error_from_errno("strdup");
-					goto done;
-				}
-				valstr = strdup(val);
-				if (valstr == NULL) {
-					err = got_error_from_errno("strdup");
-					goto done;
-				}
-				(*extnames)[(*nextensions)] = extstr;
-				(*extvals)[(*nextensions)] = valstr;
-				(*nextensions)++;
+			extstr = strdup(ext);
+			if (extstr == NULL) {
+				err = got_error_from_errno("strdup");
+				goto done;
 			}
+			valstr = strdup(val);
+			if (valstr == NULL) {
+				err = got_error_from_errno("strdup");
+				goto done;
+			}
+			(*extnames)[(*nextensions)] = extstr;
+			(*extvals)[(*nextensions)] = valstr;
+			(*nextensions)++;
 		}
 	}
 
blob - 8e364cdc52ed8662ae2562398c885d5a21c3061b
blob + 688ca609f943549450332e25c6e5531c66a3b239
--- regress/gotd/prepare_test_repo.sh
+++ regress/gotd/prepare_test_repo.sh
@@ -21,7 +21,7 @@ make_repo()
 	local repo_path="$1"
 	local no_tree="$2"
 
-	gotadmin init "${repo_path}"
+	gotadmin init -A "$GOT_TEST_ALGO" "${repo_path}"
 
 	if [ -n "$no_tree" ]; then
 		return