backend: add metadata field to Commit for arbitrary extra data - #9891
backend: add metadata field to Commit for arbitrary extra data#9891martinvonz wants to merge 1 commit into
Conversation
c1784ee to
1bbea32
Compare
The new field allows servers and custom clients to attach arbitrary metadata to commits. Since it is intended mainly for shipping data from servers to clients, it is not preserved on rewrite. It is intended to impact the commit ID, however; two commits with different metadata but otherwise identical should have different IDs. We have also discussed metadata that is preserved on rewrite (could be useful e.g. for recording a bug number), as well as metadata/annotations that are attached to commits but don't influence their identity (could be useful e.g. for linking to CI results). This patch only introduces the non-sticky kind because that's the simplest kind to support. The simple backend and the test backend support reading and writing it. The Git backend errors out for now when asked to write a commit with metadata; we may later support it by writing custom commit headers (e.g. "jj:metadata:<key>: <value>"). Note that this changes commit ids in the simple backend and the test backend because the new field is included in the content hash.
1bbea32 to
c8c08f4
Compare
|
LGTM
👍 on supporting this as long as we figure out a way to exclude it from the commit hash. |
|
For the record, this is not something my employer (East River Source Control) will need in the near future as far as I know. I think it will be useful for most forges eventually, however. I'm not sure if it would be useful for Google. The a "sticky" version that gets preserved on rewrite might be more useful there (could be used for CL tags and/or the temporary copy metadata). |
Do you mean that you think the kind of metadata introduced by this patch is useful and that you want the other kinds later? Or are you saying that you want only want the "annotation" kind that is attached to commits after the fact without affecting the identity? |
Yes I'm in favor of starting with the barebones and then later supporting the other scheme which your previous employer has interest in. |
|
LGTM, there may be a point in making a |
|
if indexing or revset function is introduced there may be a point in changing the design. |
|
@OlshaMB: I think it's reasonable to have revset functions based on metadata. What's your concern if we do? |
|
The more i think about the problems that could arise the more the concerns dissipate. I think it's ok. My concern is how would indexing impacted by arbitrary user keys, but if smth arises i probably should just make a pr tweaking key properties. |
| /// rewrite. It is part of the commit's identity; two commits with | ||
| /// different metadata but otherwise identical should have different IDs. | ||
| #[serde(skip)] // raw data wouldn't be useful | ||
| pub metadata: HashMap<String, Vec<u8>>, |
There was a problem hiding this comment.
maybe there is a point in using BTreeMap, since it's already should be sorted. Also a maybe a unnecessary memory question, is there a point in maybe doing Arc<str>, since we never mutate string.
| pub metadata: HashMap<String, Vec<u8>>, | |
| pub metadata: BTreeMap</* Arc<str> */, Vec<u8>>, |
The new field allows servers and custom clients to attach arbitrary metadata to commits. Since it is intended mainly for shipping data from servers to clients, it is not preserved on rewrite. It is intended to impact the commit ID, however; two commits with different metadata but otherwise identical should have different IDs.
We have also discussed metadata that is preserved on rewrite (could be useful e.g. for recording a bug number), as well as metadata/annotations that are attached to commits but don't influence their identity (could be useful e.g. for linking to CI results). This patch only introduces the non-sticky kind because that's the simplest kind to support.
The simple backend and the test backend support reading and writing it. The Git backend errors out for now when asked to write a commit with metadata; we may later support it by writing custom commit headers (e.g. "jj:metadata:: ").
Note that this changes commit ids in the simple backend and the test backend because the new field is included in the content hash.
Checklist
If applicable:
CHANGELOG.mdREADME.md,docs/,demos/)cli/src/config-schema.json)how it works, how it's organized), including any code drafted by an LLM.
an eye towards deleting anything that is irrelevant, clarifying anything
that is confusing, and adding details that are relevant. This includes,
for example, commit descriptions, PR descriptions, and code comments.