git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [PATCH v3 02/36] builtin/write-tree: convert to struct object_id
  2018-03-12  2:27 [PATCH v3 00/36] object_id part 12 brian m. carlson
@ 2018-03-12  2:27 ` brian m. carlson
  2018-03-14 16:48 ` [PATCH v3 00/36] object_id part 12 Junio C Hamano
  2018-03-14 17:31 ` Junio C Hamano
  2 siblings, 0 replies; 6+ messages in thread
From: brian m. carlson @ 2018-03-12  2:27 UTC (permalink / raw)
  To: git
  Cc: Nguyễn Thái Ngọc Duy, Patryk Obara, Jeff King,
	Eric Sunshine

This is needed to convert parts of the cache-tree code.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/write-tree.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/write-tree.c b/builtin/write-tree.c
index bd0a78aa3c..299a121531 100644
--- a/builtin/write-tree.c
+++ b/builtin/write-tree.c
@@ -19,7 +19,7 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
 {
 	int flags = 0, ret;
 	const char *prefix = NULL;
-	unsigned char sha1[20];
+	struct object_id oid;
 	const char *me = "git-write-tree";
 	struct option write_tree_options[] = {
 		OPT_BIT(0, "missing-ok", &flags, N_("allow missing objects"),
@@ -38,10 +38,10 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
 	argc = parse_options(argc, argv, unused_prefix, write_tree_options,
 			     write_tree_usage, 0);
 
-	ret = write_cache_as_tree(sha1, flags, prefix);
+	ret = write_cache_as_tree(oid.hash, flags, prefix);
 	switch (ret) {
 	case 0:
-		printf("%s\n", sha1_to_hex(sha1));
+		printf("%s\n", oid_to_hex(&oid));
 		break;
 	case WRITE_TREE_UNREADABLE_INDEX:
 		die("%s: error reading the index", me);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 00/36] object_id part 12
  2018-03-12  2:27 [PATCH v3 00/36] object_id part 12 brian m. carlson
  2018-03-12  2:27 ` [PATCH v3 02/36] builtin/write-tree: convert to struct object_id brian m. carlson
@ 2018-03-14 16:48 ` Junio C Hamano
  2018-03-14 19:46   ` Junio C Hamano
  2018-03-15  1:05   ` brian m. carlson
  2018-03-14 17:31 ` Junio C Hamano
  2 siblings, 2 replies; 6+ messages in thread
From: Junio C Hamano @ 2018-03-14 16:48 UTC (permalink / raw)
  To: brian m. carlson
  Cc: git, Nguyễn Thái Ngọc Duy, Patryk Obara,
	Jeff King, Eric Sunshine

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> This is the twelfth in a series of patches to convert various parts of
> the code to struct object_id.
>
> Changes from v2:
> * Rebase onto master (to fix "typename" → "type_name" changes).
> * Replace some uses of hashcpy with memcpy.
> * Replace some instances of "20" with references to the_hash_algo.
>
> Changes from v1:
> * Rebase onto master.
>
> tbdiff output below.
>
> brian m. carlson (36):
>   bulk-checkin: convert index_bulk_checkin to struct object_id
>   builtin/write-tree: convert to struct object_id
>   cache-tree: convert write_*_as_tree to object_id
>   cache-tree: convert remnants to struct object_id
>   resolve-undo: convert struct resolve_undo_info to object_id
>   tree: convert read_tree_recursive to struct object_id
>   ref-filter: convert grab_objectname to struct object_id
>   strbuf: convert strbuf_add_unique_abbrev to use struct object_id
>   wt-status: convert struct wt_status_state to object_id
>   Convert find_unique_abbrev* to struct object_id
>   http-walker: convert struct object_request to use struct object_id
>   send-pack: convert remaining functions to struct object_id
>   replace_object: convert struct replace_object to object_id
>   builtin/mktag: convert to struct object_id
>   archive: convert write_archive_entry_fn_t to object_id
>   archive: convert sha1_file_to_archive to struct object_id
>   builtin/index-pack: convert struct ref_delta_entry to object_id
>   sha1_file: convert read_loose_object to use struct object_id
>   sha1_file: convert check_sha1_signature to struct object_id
>   streaming: convert open_istream to use struct object_id
>   builtin/mktree: convert to struct object_id
>   sha1_file: convert assert_sha1_type to object_id
>   sha1_file: convert retry_bad_packed_offset to struct object_id
>   packfile: convert unpack_entry to struct object_id
>   Convert remaining callers of sha1_object_info_extended to object_id
>   sha1_file: convert sha1_object_info* to object_id
>   builtin/fmt-merge-msg: convert remaining code to object_id
>   builtin/notes: convert static functions to object_id
>   tree-walk: convert get_tree_entry_follow_symlinks internals to
>     object_id
>   streaming: convert istream internals to struct object_id
>   tree-walk: convert tree entry functions to object_id
>   sha1_file: convert read_object_with_reference to object_id
>   sha1_file: convert read_sha1_file to struct object_id
>   Convert lookup_replace_object to struct object_id
>   sha1_file: introduce a constant for max header length
>   convert: convert to struct object_id

As always, thanks for working on this.  

After this series, what jumps at me out of output from

    git grep -e '[^0-9A-Za-z_][24]0[^0-9A-Za-z_]' -- '*.[ch]' \
		':!*sha1*' ':!contrib/' ':!compat/'

are code that parses the incoming patch in apply.c (where the full
blob object names used for binary patches are assumed to be in
SHA-1), builtin/pack-objects.c (where it has to know the current
file format of a packfile intimately) and diff.c (where it clips the
length to which the blob object names on the "index" lines are
abbreviated to).  Changing 40 in the last one to "the hex length of
the currently deployed hash" should be relatively uncontroversial.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 00/36] object_id part 12
  2018-03-12  2:27 [PATCH v3 00/36] object_id part 12 brian m. carlson
  2018-03-12  2:27 ` [PATCH v3 02/36] builtin/write-tree: convert to struct object_id brian m. carlson
  2018-03-14 16:48 ` [PATCH v3 00/36] object_id part 12 Junio C Hamano
@ 2018-03-14 17:31 ` Junio C Hamano
  2018-03-15  0:53   ` brian m. carlson
  2 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2018-03-14 17:31 UTC (permalink / raw)
  To: brian m. carlson
  Cc: git, Nguyễn Thái Ngọc Duy, Patryk Obara,
	Jeff King, Eric Sunshine

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

>     -+		buf += the_hash_algo->rawsz;
>     -+		size -= the_hash_algo->rawsz;
>     ++		memcpy(it->oid.hash, (const unsigned char*)buf, rawsz);
>     ++		buf += rawsz;
>     ++		size -= rawsz;
>       	}

Using memcpy() to stuff the hash[] field of oid structure with a
bare byte array of rawsz bytes appears twice as a pattern in these
patches.  I wonder if this is something we want to abstract behind
the API, e.g.

	size_t oidstuff_(struct object_id *oid, const unsigned char *buf)
	{
		size_t rawsz = the_hash_algo->rawsz;
		memcpy(oid->hash, buf, rawsz);
                return rawsz;
	}

It just felt a bit uneven to be using a bare-metal memcpy() when
oidcpy() abstraction releaves the callers from having to be aware of
the rawsz all the time.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 00/36] object_id part 12
  2018-03-14 16:48 ` [PATCH v3 00/36] object_id part 12 Junio C Hamano
@ 2018-03-14 19:46   ` Junio C Hamano
  2018-03-15  1:05   ` brian m. carlson
  1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2018-03-14 19:46 UTC (permalink / raw)
  To: brian m. carlson
  Cc: git, Nguyễn Thái Ngọc Duy, Patryk Obara,
	Jeff King, Eric Sunshine

Junio C Hamano <gitster@pobox.com> writes:

> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
>> This is the twelfth in a series of patches to convert various parts of
>> the code to struct object_id.
>>
>> brian m. carlson (36):
>>   ...
> As always, thanks for working on this.  

There are a few topics that add new callsites to functions that are
updated (e.g. find_unique_abbrev()), so I'll need to do a bit of the
usual evil merging to coax this topic in.  Please holler if you spot
my screw-up in what I'll push out in a few hours.

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 00/36] object_id part 12
  2018-03-14 17:31 ` Junio C Hamano
@ 2018-03-15  0:53   ` brian m. carlson
  0 siblings, 0 replies; 6+ messages in thread
From: brian m. carlson @ 2018-03-15  0:53 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Nguyễn Thái Ngọc Duy, Patryk Obara,
	Jeff King, Eric Sunshine

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

On Wed, Mar 14, 2018 at 10:31:37AM -0700, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> >     -+		buf += the_hash_algo->rawsz;
> >     -+		size -= the_hash_algo->rawsz;
> >     ++		memcpy(it->oid.hash, (const unsigned char*)buf, rawsz);
> >     ++		buf += rawsz;
> >     ++		size -= rawsz;
> >       	}
> 
> Using memcpy() to stuff the hash[] field of oid structure with a
> bare byte array of rawsz bytes appears twice as a pattern in these
> patches.  I wonder if this is something we want to abstract behind
> the API, e.g.
> 
> 	size_t oidstuff_(struct object_id *oid, const unsigned char *buf)
> 	{
> 		size_t rawsz = the_hash_algo->rawsz;
> 		memcpy(oid->hash, buf, rawsz);
>                 return rawsz;
> 	}
> 
> It just felt a bit uneven to be using a bare-metal memcpy() when
> oidcpy() abstraction releaves the callers from having to be aware of
> the rawsz all the time.

Duy suggested oidread and oidwrite, which I can certainly implement.
I'm also comfortable with just keeping hashcpy around for these cases if
we want.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 00/36] object_id part 12
  2018-03-14 16:48 ` [PATCH v3 00/36] object_id part 12 Junio C Hamano
  2018-03-14 19:46   ` Junio C Hamano
@ 2018-03-15  1:05   ` brian m. carlson
  1 sibling, 0 replies; 6+ messages in thread
From: brian m. carlson @ 2018-03-15  1:05 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Nguyễn Thái Ngọc Duy, Patryk Obara,
	Jeff King, Eric Sunshine

[-- Attachment #1: Type: text/plain, Size: 1165 bytes --]

On Wed, Mar 14, 2018 at 09:48:30AM -0700, Junio C Hamano wrote:
> As always, thanks for working on this.  
> 
> After this series, what jumps at me out of output from
> 
>     git grep -e '[^0-9A-Za-z_][24]0[^0-9A-Za-z_]' -- '*.[ch]' \
> 		':!*sha1*' ':!contrib/' ':!compat/'
> 
> are code that parses the incoming patch in apply.c (where the full
> blob object names used for binary patches are assumed to be in
> SHA-1), builtin/pack-objects.c (where it has to know the current
> file format of a packfile intimately) and diff.c (where it clips the
> length to which the blob object names on the "index" lines are
> abbreviated to).  Changing 40 in the last one to "the hex length of
> the currently deployed hash" should be relatively uncontroversial.

I have patches that hit several of these places, but not diff.c.  I'll
probably pick that piece up next.

By the way, thanks for a useful regex.  It's a little bit different
than what I've been using, but provides a nice overview.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-12  2:27 [PATCH v3 00/36] object_id part 12 brian m. carlson
2018-03-12  2:27 ` [PATCH v3 02/36] builtin/write-tree: convert to struct object_id brian m. carlson
2018-03-14 16:48 ` [PATCH v3 00/36] object_id part 12 Junio C Hamano
2018-03-14 19:46   ` Junio C Hamano
2018-03-15  1:05   ` brian m. carlson
2018-03-14 17:31 ` Junio C Hamano
2018-03-15  0:53   ` brian m. carlson

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox