git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref

From
Junio C Hamano <gitster@pobox.com>
Date
May 22, 2025, 20:51 UTC
Message-ID
<xmqqv7ps4bef.fsf@gitster.g>
In-Reply-To
<20250522185524.18398-6-sandals@crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
Show 26 quoted lines
> +static int check_stash_topology(struct repository *r, struct commit *stash)
> +{
> +	struct commit *p1, *p2, *p3 = NULL;
> +
> +	/* stash must have two or three parents */
> +	if (!stash->parents || !stash->parents->next ||
> +			(stash->parents->next->next && stash->parents->next->next->next))
> +		return -1;
> +	p1 = stash->parents->item;
> +	p2 = stash->parents->next->item;
> +	if (stash->parents->next->next)
> +		p3 = stash->parents->next->next->item;
> +	if (repo_parse_commit(r, p1) || repo_parse_commit(r, p2) ||
> +			(p3 && repo_parse_commit(r, p3)))
> +		return -1;
> +	/* p2 must have a single parent, p3 must have no parents */
> +	if (!p2->parents || p2->parents->next || (p3 && p3->parents))
> +		return -1;
> +	if (repo_parse_commit(r, p2->parents->item))
> +		return -1;
> +	/* p2^1 must equal p1 */
> +	if (!oideq(&p1->object.oid, &p2->parents->item->object.oid))
> +		return -1;
> +
> +	return 0;
> +}

This is much more elaborate than the existing "does this commit look like a stash entry" test done elsewhere, isn't it? Very nicely done.

Show 41 quoted lines
> +	this = lookup_commit_reference(r, oid);
> +	buffer = repo_get_commit_buffer(r, this, &bufsize);
> +	orig_author = find_commit_header(buffer, "author", &author_len);
> +	orig_committer = find_commit_header(buffer, "committer", &committer_len);
> +
> +	if (!orig_author || !orig_committer) {
> +		ret = error(_("cannot parse commit %s"), oid_to_hex(oid));
> +		goto out;
> +	}
> +
> +	if (split_ident_line(&id, orig_author, author_len) < 0 ||
> +	    split_ident_line(&id, orig_committer, committer_len) < 0) {
> +		ret = error(_("invalid author or committer for %s"), oid_to_hex(oid));
> +		goto out;
> +	}
> +
> +	p = strstr(buffer, "\n\n");
> +	strbuf_addstr(&msg, "git stash: ");
> +
> +	if (p)
> +		strbuf_add(&msg, p + 2, bufsize - (p + 2 - buffer));
> +	strbuf_complete_line(&msg);
> +
> +	author = xmemdupz(orig_author, author_len);
> +	committer = xmemdupz(orig_committer, committer_len);
> +
> +	if (commit_tree_extended(msg.buf, msg.len,
> +				 r->hash_algo->empty_tree, parents,
> +				 out, author, committer,
> +				 NULL, NULL)) {
> +		ret = error(_("could not write commit"));
> +		goto out;
> +	}
> +out:
> +	strbuf_release(&msg);
> +	repo_unuse_commit_buffer(r, this, buffer);
> +	free_commit_list(parents);
> +	free(author);
> +	free(committer);
> +	return ret;
> +}
Nice.
Show 48 quoted lines
> +	if (argc) {
> +		/*
> +		 * Find each specified stash, and load data into the array.
> +		 */
> +		for (i = 0; i < argc; i++) {
> +			struct object_id oid;
> +			struct commit *stash;
> +
> +			if (parse_stash_revision(&revision, argv[i], 1) ||
> +			    get_oid_with_context(r, revision.buf,
> +						 GET_OID_QUIETLY | GET_OID_GENTLY,
> +						 &oid, &unused)) {
> +				res = error(_("unable to find stash entry %s"), argv[i]);
> +				goto out;
> +			}
> +
> +			stash = lookup_commit_reference(r, &oid);
> +			if (!stash || check_stash_topology(r, stash)) {
> +				res = error(_("%s does not look like a stash commit"),
> +						revision.buf);
> +				goto out;
> +			}
> +			oid_array_append(&items, &oid);
> +		}
> +	} else {
> +		/*
> +		 * Walk the reflog, finding each stash entry, and load data into the
> +		 * array.
> +		 */
> +		struct complete_reflogs *crl = read_complete_reflog("refs/stash");
> +		for (i = 0; i < crl->nr; i++) {
> +			/*
> +			 * Iterate from the latest item in the reflog to the
> +			 * earliest.  crl->items starts from the earliest first.
> +			 */
> +			struct reflog_info *item = crl->items + (crl->nr - i - 1);
> +			struct commit *stash;
> +
> +			stash = lookup_commit_reference(r, &item->noid);
> +			if (!stash || check_stash_topology(r, stash)) {
> +				res = error(_("%s does not look like a stash commit"),
> +						revision.buf);
> +				goto out;
> +			}
> +			oid_array_append(&items, &item->noid);
> +		}
> +		free_complete_reflog(crl);
> +	}

Hmph. As we work with the commit objects in the above part already, would it still make sense to use an oid array? That would force us to look up the same set of commits using the object names again in the later loop. I would have thought we might use commit_list or something to accumulate commits, and then consume in the loop below.

Not a huge deal, though.  But I somehow find it more natural.  

In addition, we would not have to worry about the ssize_t cast and other complications if we used commit_list to hold these incoming stash entries.

Previous: brian m. carlsonNext: brian m. carlson
Message 66 of 72 in “Importing and exporting stashes to refs”
  1. 0/4 Importing and exporting stashes to refsbrian m. carlson, May 8, 2025
  2. 2/4 builtin/stash: factor out revision parsing into a functionbrian m. carlson, May 8, 2025
  3. Junio C HamanoMay 9, 2025
  4. 3/4 builtin/stash: provide a way to export stashes to a refbrian m. carlson, May 8, 2025
  5. Junio C HamanoMay 9, 2025
  6. Junio C HamanoMay 9, 2025
  7. Jeff KingMay 10, 2025
  8. Phillip WoodMay 12, 2025
  9. Junio C HamanoMay 12, 2025
  10. 1/4 object-name: make get_oid quietly return an errorbrian m. carlson, May 8, 2025
  11. Junio C HamanoMay 9, 2025
  12. brian m. carlsonMay 9, 2025
  13. 4/4 builtin/stash: provide a way to import stashes from a refbrian m. carlson, May 8, 2025
  14. Junio C HamanoMay 9, 2025
  15. brian m. carlsonMay 11, 2025
  16. Jeff KingMay 10, 2025
  17. Junio C HamanoMay 12, 2025
  18. Jeff KingMay 12, 2025
  19. Junio C HamanoMay 12, 2025
  20. brian m. carlsonMay 12, 2025
  21. Jeff KingMay 10, 2025
  22. Phillip WoodMay 12, 2025
  23. Junio C HamanoMay 9, 2025
  24. brian m. carlsonMay 9, 2025
  25. D. Ben KnobleMay 9, 2025
  26. brian m. carlsonMay 9, 2025
  27. D. Ben KnobleMay 10, 2025
  28. Makefile: avoid constant rebuilds with compilation databasebrian m. carlson, May 22, 2025
  29. 3/5 builtin/stash: factor out revision parsing into a functionbrian m. carlson, May 22, 2025
  30. Junio C HamanoMay 22, 2025
  31. brian m. carlsonMay 23, 2025
  32. Junio C HamanoMay 24, 2025
  33. brian m. carlsonMay 26, 2025
  34. 2/5 reflog-walk: expose read_complete_reflogbrian m. carlson, May 22, 2025
  35. Ramsay JonesMay 22, 2025
  36. brian m. carlsonMay 23, 2025
  37. Ramsay JonesMay 24, 2025
  38. brian m. carlsonMay 26, 2025
  39. Phillip WoodMay 29, 2025
  40. Junio C HamanoMay 29, 2025
  41. 0/5 Importing and exporting stashes to refsbrian m. carlson, May 22, 2025
  42. 0/4 Importing and exporting stashes to refsbrian m. carlson, Jun 1, 2025
  43. 1/4 object-name: make get_oid quietly return an errorbrian m. carlson, Jun 1, 2025
  44. 2/4 builtin/stash: factor out revision parsing into a functionbrian m. carlson, Jun 1, 2025
  45. 4/4 builtin/stash: provide a way to import stashes from a refbrian m. carlson, Jun 1, 2025
  46. 3/4 builtin/stash: provide a way to export stashes to a refbrian m. carlson, Jun 1, 2025
  47. Phillip WoodJun 5, 2025
  48. Kristoffer HaugsbakkJun 11, 2025
  49. brian m. carlsonJun 11, 2025
  50. Phillip WoodJun 5, 2025
  51. 0/4 Importing and exporting stashes to refsbrian m. carlson, Jun 12, 2025
  52. 2/4 builtin/stash: factor out revision parsing into a functionbrian m. carlson, Jun 12, 2025
  53. 1/4 object-name: make get_oid quietly return an errorbrian m. carlson, Jun 12, 2025
  54. 3/4 builtin/stash: provide a way to export stashes to a refbrian m. carlson, Jun 12, 2025
  55. 4/4 builtin/stash: provide a way to import stashes from a refbrian m. carlson, Jun 12, 2025
  56. Phillip WoodJun 25, 2025
  57. Junio C HamanoJun 25, 2025
  58. 1/5 object-name: make get_oid quietly return an errorbrian m. carlson, May 22, 2025
  59. Junio C HamanoMay 22, 2025
  60. 5/5 builtin/stash: provide a way to import stashes from a refbrian m. carlson, May 22, 2025
  61. Junio C HamanoMay 22, 2025
  62. brian m. carlsonMay 26, 2025
  63. Junio C HamanoMay 22, 2025
  64. Phillip WoodMay 23, 2025
  65. 4/5 builtin/stash: provide a way to export stashes to a refbrian m. carlson, May 22, 2025
  66. Junio C HamanoMay 22, 2025
  67. brian m. carlsonMay 26, 2025
  68. Phillip WoodMay 29, 2025
  69. brian m. carlsonMay 22, 2025
  70. Junio C HamanoMay 22, 2025
  71. Kristoffer HaugsbakkJun 11, 2025
  72. brian m. carlsonJun 12, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.