On Sun, Oct 01, 2023 at 09:40:05PM -0500, Eric W. Biederman wrote:
> From: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
>
> Two basic functions are provided:
> - convert_object_file Takes an object file it's type and hash algorithm
> and converts it into the equivalent object file that would
> have been generated with hash algorithm "to".
>
> For blob objects there is no conversation to be done and it is an
> error to use this function on them.
>
> For commit, tree, and tag objects embedded oids are replaced by the
> oids of the objects they refer to with those objects and their
> object ids reencoded in with the hash algorithm "to". Signatures
> are rearranged so that they remain valid after the object has
> been reencoded.
>
> - repo_oid_to_algop which takes an oid that refers to an object file
> and returns the oid of the equivalent object file generated
> with the target hash algorithm.
>
> The pair of files object-file-convert.c and object-file-convert.h are
> introduced to hold as much of this logic as possible to keep this
> conversion logic cleanly separated from everything else and in the
> hopes that someday the code will be clean enough git can support
> compiling out support for sha1 and the various conversion functions.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
> ---
> Makefile | 1 +
> object-file-convert.c | 57 +++++++++++++++++++++++++++++++++++++++++++
> object-file-convert.h | 24 ++++++++++++++++++
> 3 files changed, 82 insertions(+)
> create mode 100644 object-file-convert.c
> create mode 100644 object-file-convert.h
>
> diff --git a/Makefile b/Makefile
> index 577630936535..f7e824f25cda 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1073,6 +1073,7 @@ LIB_OBJS += notes-cache.o
> LIB_OBJS += notes-merge.o
> LIB_OBJS += notes-utils.o
> LIB_OBJS += notes.o
> +LIB_OBJS += object-file-convert.o
> LIB_OBJS += object-file.o
> LIB_OBJS += object-name.o
> LIB_OBJS += object.o
> diff --git a/object-file-convert.c b/object-file-convert.c
> new file mode 100644
> index 000000000000..4777aba83636
> --- /dev/null
> +++ b/object-file-convert.c
> @@ -0,0 +1,57 @@
> +#include "git-compat-util.h"
> +#include "gettext.h"
> +#include "strbuf.h"
> +#include "repository.h"
> +#include "hash-ll.h"
> +#include "object.h"
> +#include "object-file-convert.h"
> +
> +int repo_oid_to_algop(struct repository *repo, const struct object_id *src,
> + const struct git_hash_algo *to, struct object_id *dest)
> +{
> + /*
> + * If the source algorithm is not set, then we're using the
> + * default hash algorithm for that object.
> + */
> + const struct git_hash_algo *from =
> + src->algo ? &hash_algos[src->algo] : repo->hash_algo;
> +
> + if (from == to) {
> + if (src != dest)
> + oidcpy(dest, src);
> + return 0;
> + }
> + return -1;
> +}
In it's current form, `repo_oid_to_algop()` basically never does
anything except for copying over the object ID because we do not handle
the case where object hashes are different. I assume this is intended,
as we basically only provide stubs in this commit. But still, it would
help to document this in-code as well with a comment.
> +int convert_object_file(struct strbuf *outbuf,
> + const struct git_hash_algo *from,
> + const struct git_hash_algo *to,
> + const void *buf, size_t len,
> + enum object_type type,
> + int gentle)
> +{
> + int ret;
> +
> + /* Don't call this function when no conversion is necessary */
> + if ((from == to) || (type == OBJ_BLOB))
> + BUG("Refusing noop object file conversion");
The extra braces around comparisons are unneeded and to the best of my
knowledge not customary in our code base. Also, error messages should
start with a lower-case letter.
> + switch (type) {
> + case OBJ_COMMIT:
> + case OBJ_TREE:
> + case OBJ_TAG:
> + default:
> + /* Not implemented yet, so fail. */
> + ret = -1;
> + break;
> + }
It's a bit weird that we handle all object types except for blobs
separately, and then still have a `default` statement. I would've
thought that we should handle the object types specifically and set `ret
= -1` for all of them, and then the `default` case would instead call
`BUG()` due to an unknown object type.
> + if (!ret)
> + return 0;
> + if (gentle) {
> + strbuf_release(outbuf);
> + return ret;
> + }
Do you really intend to call `strbuf_release()` on the caller provided
buffer, or should this rather be `strbuf_reset()`? Memory management of
such an in/out parameter should typically be handled by the caller, not
the callee.
> + die(_("Failed to convert object from %s to %s"),
> + from->name, to->name);
> +}
The error message should start with a lower-case letter.
> diff --git a/object-file-convert.h b/object-file-convert.h
> new file mode 100644
> index 000000000000..a4f802aa8eea
> --- /dev/null
> +++ b/object-file-convert.h
> @@ -0,0 +1,24 @@
> +#ifndef OBJECT_CONVERT_H
> +#define OBJECT_CONVERT_H
> +
> +struct repository;
> +struct object_id;
> +struct git_hash_algo;
> +struct strbuf;
> +#include "object.h"
> +
> +int repo_oid_to_algop(struct repository *repo, const struct object_id *src,
> + const struct git_hash_algo *to, struct object_id *dest);
> +
> +/*
> + * Convert an object file from one hash algorithm to another algorithm.
> + * Return -1 on failure, 0 on success.
> + */
> +int convert_object_file(struct strbuf *outbuf,
> + const struct git_hash_algo *from,
> + const struct git_hash_algo *to,
> + const void *buf, size_t len,
> + enum object_type type,
> + int gentle);
It would be nice to document what `gentle` does.
Patrick
> +#endif /* OBJECT_CONVERT_H */
> --
> 2.41.0
>
Attachment:
signature.asc
Description: PGP signature
