Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups
  2019-01-09 21:37                 ` Stefan Beller
@ 2019-01-09 22:42                   ` Stefan Beller
  2019-01-10  6:17                     ` Jeff King
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Beller @ 2019-01-09 22:42 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, René Scharfe, git,
	Ævar Arnfjörð Bjarmason

On Wed, Jan 9, 2019 at 1:37 PM Stefan Beller <sbeller@google.com> wrote:
>
> > > Yikes, the conflicts with sb/more-repo-in-api is quite irritating.
> > > I think I'll postpone the later parts of this series and ask this to
> > > be sent after sb/more-repo-in-api matures a bit mroe.
> >
> > There were several conflicts, but it was mostly just tedious textual
> > fixups. I pushed the result to:
> >
> >   https://github.com/peff/git.git resolve-oid-more-repo
> >
> > But I'm happy to wait and rebase if sb/more-repo-in-api is close to
> > graduating.
>
> The merge looks good to me, though I just looked quickly.
> The series itself is also a pleasant read.

Compiling this leads to:

sha1-file.c:1424:33: error: incompatible pointer types passing 'const
struct object_id *' to parameter of type 'const unsigned char *'
[-Werror,-Wincompatible-pointer-types]
        if ((p = has_packed_and_bad(r, repl)) != NULL)
                                       ^~~~
./packfile.h:149:95: note: passing argument to parameter 'sha1' here
extern const struct packed_git *has_packed_and_bad(struct repository
*r, const unsigned char *sha1);

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

* Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups
  2019-01-09 22:42                   ` Stefan Beller
@ 2019-01-10  6:17                     ` Jeff King
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2019-01-10  6:17 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Junio C Hamano, René Scharfe, git,
	Ævar Arnfjörð Bjarmason

On Wed, Jan 09, 2019 at 02:42:28PM -0800, Stefan Beller wrote:

> On Wed, Jan 9, 2019 at 1:37 PM Stefan Beller <sbeller@google.com> wrote:
> >
> > > > Yikes, the conflicts with sb/more-repo-in-api is quite irritating.
> > > > I think I'll postpone the later parts of this series and ask this to
> > > > be sent after sb/more-repo-in-api matures a bit mroe.
> > >
> > > There were several conflicts, but it was mostly just tedious textual
> > > fixups. I pushed the result to:
> > >
> > >   https://github.com/peff/git.git resolve-oid-more-repo
> > >
> > > But I'm happy to wait and rebase if sb/more-repo-in-api is close to
> > > graduating.
> >
> > The merge looks good to me, though I just looked quickly.
> > The series itself is also a pleasant read.
> 
> Compiling this leads to:
> 
> sha1-file.c:1424:33: error: incompatible pointer types passing 'const
> struct object_id *' to parameter of type 'const unsigned char *'
> [-Werror,-Wincompatible-pointer-types]
>         if ((p = has_packed_and_bad(r, repl)) != NULL)
>                                        ^~~~
> ./packfile.h:149:95: note: passing argument to parameter 'sha1' here
> extern const struct packed_git *has_packed_and_bad(struct repository
> *r, const unsigned char *sha1);

Eek, sorry about that. I did the merge on a detached HEAD, and my
config.mak relaxes compilation warnings in that case (since I am often
sight-seeing to old versions that have warnings which have since been
fixed). And the result passes the tests since "repl" and "repl->hash"
are effectively the same pointer.

I've pushed up the fix (s/repl/repl->hash/). Thanks for noticing.

-Peff

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

* Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups
  2019-01-08 21:16               ` Jeff King
@ 2019-01-09 21:37                 ` Stefan Beller
  2019-01-09 22:42                   ` Stefan Beller
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Beller @ 2019-01-09 21:37 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, René Scharfe, git,
	Ævar Arnfjörð Bjarmason

> > Yikes, the conflicts with sb/more-repo-in-api is quite irritating.
> > I think I'll postpone the later parts of this series and ask this to
> > be sent after sb/more-repo-in-api matures a bit mroe.
>
> There were several conflicts, but it was mostly just tedious textual
> fixups. I pushed the result to:
>
>   https://github.com/peff/git.git resolve-oid-more-repo
>
> But I'm happy to wait and rebase if sb/more-repo-in-api is close to
> graduating.

The merge looks good to me, though I just looked quickly.
The series itself is also a pleasant read.

Stefan

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

* Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups
  2019-01-08 18:52             ` Junio C Hamano
@ 2019-01-08 21:16               ` Jeff King
  2019-01-09 21:37                 ` Stefan Beller
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff King @ 2019-01-08 21:16 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: René Scharfe, git, Ævar Arnfjörð Bjarmason

On Tue, Jan 08, 2019 at 10:52:19AM -0800, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Jeff King <peff@peff.net> writes:
> >
> >> Yeah, they should. I think one of them will need René's patch, which
> >> changes the body of quick_has_loose(). I can roll it as a separate topic
> >> if that's easier (or just wait a week or so until René's cleanups
> >> graduate).
> >
> > Nah, what I got is already good to work with.  Both series are
> > straight-forward and I do not expect them needing long fermentation.
> 
> Yikes, the conflicts with sb/more-repo-in-api is quite irritating.
> I think I'll postpone the later parts of this series and ask this to
> be sent after sb/more-repo-in-api matures a bit mroe.

There were several conflicts, but it was mostly just tedious textual
fixups. I pushed the result to:

  https://github.com/peff/git.git resolve-oid-more-repo

But I'm happy to wait and rebase if sb/more-repo-in-api is close to
graduating.

-Peff

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

* Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups
  2019-01-08 18:07           ` Junio C Hamano
  2019-01-08 18:27             ` Derrick Stolee
@ 2019-01-08 18:52             ` Junio C Hamano
  2019-01-08 21:16               ` Jeff King
  1 sibling, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2019-01-08 18:52 UTC (permalink / raw)
  To: Jeff King; +Cc: René Scharfe, git, Ævar Arnfjörð Bjarmason

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

> Jeff King <peff@peff.net> writes:
>
>> Yeah, they should. I think one of them will need René's patch, which
>> changes the body of quick_has_loose(). I can roll it as a separate topic
>> if that's easier (or just wait a week or so until René's cleanups
>> graduate).
>
> Nah, what I got is already good to work with.  Both series are
> straight-forward and I do not expect them needing long fermentation.

Yikes, the conflicts with sb/more-repo-in-api is quite irritating.
I think I'll postpone the later parts of this series and ask this to
be sent after sb/more-repo-in-api matures a bit mroe.


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

* Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups
  2019-01-08 18:07           ` Junio C Hamano
@ 2019-01-08 18:27             ` Derrick Stolee
  2019-01-08 18:52             ` Junio C Hamano
  1 sibling, 0 replies; 11+ messages in thread
From: Derrick Stolee @ 2019-01-08 18:27 UTC (permalink / raw)
  To: Junio C Hamano, Jeff King
  Cc: René Scharfe, git, Ævar Arnfjörð Bjarmason

On 1/8/2019 1:07 PM, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
>> Yeah, they should. I think one of them will need René's patch, which
>> changes the body of quick_has_loose(). I can roll it as a separate topic
>> if that's easier (or just wait a week or so until René's cleanups
>> graduate).
> Nah, what I got is already good to work with.  Both series are
> straight-forward and I do not expect them needing long fermentation.

I'm just chiming in to say that this series was a very satisfying read, 
and the changes were clear-cut and mechanical.

Thanks!
-Stolee

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

* Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups
  2019-01-08 18:05         ` Jeff King
@ 2019-01-08 18:07           ` Junio C Hamano
  2019-01-08 18:27             ` Derrick Stolee
  2019-01-08 18:52             ` Junio C Hamano
  0 siblings, 2 replies; 11+ messages in thread
From: Junio C Hamano @ 2019-01-08 18:07 UTC (permalink / raw)
  To: Jeff King; +Cc: René Scharfe, git, Ævar Arnfjörð Bjarmason

Jeff King <peff@peff.net> writes:

> Yeah, they should. I think one of them will need René's patch, which
> changes the body of quick_has_loose(). I can roll it as a separate topic
> if that's easier (or just wait a week or so until René's cleanups
> graduate).

Nah, what I got is already good to work with.  Both series are
straight-forward and I do not expect them needing long fermentation.


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

* Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups
  2019-01-08 17:39       ` Junio C Hamano
@ 2019-01-08 18:05         ` Jeff King
  2019-01-08 18:07           ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff King @ 2019-01-08 18:05 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: René Scharfe, git, Ævar Arnfjörð Bjarmason

On Tue, Jan 08, 2019 at 09:39:48AM -0800, Junio C Hamano wrote:

> > I skimmed them; they look good to me.  6 and 8 are particularly
> > satisfying; getting rid of hash copy operations just feels nice. :)
> >
> > Junio only took 1 to 5 into pu; 6, 7 and its sidekick 8, 10 and 11
> > conflict with sb/more-repo-in-api; 9 could go in unmodified.
> 
> I think these later steps are based on something a lot newer than
> the result of applying your updates to the jk/loose-object-cache
> series they fix.  I think I untangled and backported one of the
> earlier commits but then I stopped after 05/11.

Sorry, I applied René's patches on top of master and then built on that,
treating it like a new topic. But of course your jk/loose-object-cache
is older.

> I do not think it is important to keep jk/loose-object-cache and
> these two follow-up topics mergeable to the maintenance track, so
> I'll see if the patches behave better if queued directly on top of
> 3b2f8a02 ("Merge branch 'jk/loose-object-cache'", 2019-01-04), or
> even a yet newer random point on 'master'.

Yeah, they should. I think one of them will need René's patch, which
changes the body of quick_has_loose(). I can roll it as a separate topic
if that's easier (or just wait a week or so until René's cleanups
graduate).

-Peff

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

* Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups
  2019-01-08 16:40     ` René Scharfe
@ 2019-01-08 17:39       ` Junio C Hamano
  2019-01-08 18:05         ` Jeff King
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2019-01-08 17:39 UTC (permalink / raw)
  To: René Scharfe; +Cc: Jeff King, git, Ævar Arnfjörð Bjarmason

René Scharfe <l.s.r@web.de> writes:

> Am 07.01.2019 um 09:31 schrieb Jeff King:
>> I also cleaned up my sha1/object_id patch and rebased it on top of what
>> you have here. Though as I worked on it, it expanded in scope a bit.
>> Possibly it should be a separate series entirely, but that would create
>> some annoying textual conflicts on merge.
>> 
>>   [01/11]: sha1-file: fix outdated sha1 comment references
>>   [02/11]: update comment references to sha1_object_info()
>>   [03/11]: http: use struct object_id instead of bare sha1
>>   [04/11]: sha1-file: modernize loose object file functions
>>   [05/11]: sha1-file: modernize loose header/stream functions
>>   [06/11]: sha1-file: convert pass-through functions to object_id
>>   [07/11]: convert has_sha1_file() callers to has_object_file()
>>   [08/11]: sha1-file: drop has_sha1_file()
>>   [09/11]: sha1-file: prefer "loose object file" to "sha1 file" in messages
>>   [10/11]: sha1-file: avoid "sha1 file" for generic use in messages
>>   [11/11]: prefer "hash mismatch" to "sha1 mismatch"
>
> I skimmed them; they look good to me.  6 and 8 are particularly
> satisfying; getting rid of hash copy operations just feels nice. :)
>
> Junio only took 1 to 5 into pu; 6, 7 and its sidekick 8, 10 and 11
> conflict with sb/more-repo-in-api; 9 could go in unmodified.

I think these later steps are based on something a lot newer than
the result of applying your updates to the jk/loose-object-cache
series they fix.  I think I untangled and backported one of the
earlier commits but then I stopped after 05/11.

I do not think it is important to keep jk/loose-object-cache and
these two follow-up topics mergeable to the maintenance track, so
I'll see if the patches behave better if queued directly on top of
3b2f8a02 ("Merge branch 'jk/loose-object-cache'", 2019-01-04), or
even a yet newer random point on 'master'.

Thanks.



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

* Re: [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups
  2019-01-07  8:31   ` [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups Jeff King
@ 2019-01-08 16:40     ` René Scharfe
  2019-01-08 17:39       ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: René Scharfe @ 2019-01-08 16:40 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason

Am 07.01.2019 um 09:31 schrieb Jeff King:
> I also cleaned up my sha1/object_id patch and rebased it on top of what
> you have here. Though as I worked on it, it expanded in scope a bit.
> Possibly it should be a separate series entirely, but that would create
> some annoying textual conflicts on merge.
> 
>   [01/11]: sha1-file: fix outdated sha1 comment references
>   [02/11]: update comment references to sha1_object_info()
>   [03/11]: http: use struct object_id instead of bare sha1
>   [04/11]: sha1-file: modernize loose object file functions
>   [05/11]: sha1-file: modernize loose header/stream functions
>   [06/11]: sha1-file: convert pass-through functions to object_id
>   [07/11]: convert has_sha1_file() callers to has_object_file()
>   [08/11]: sha1-file: drop has_sha1_file()
>   [09/11]: sha1-file: prefer "loose object file" to "sha1 file" in messages
>   [10/11]: sha1-file: avoid "sha1 file" for generic use in messages
>   [11/11]: prefer "hash mismatch" to "sha1 mismatch"

I skimmed them; they look good to me.  6 and 8 are particularly
satisfying; getting rid of hash copy operations just feels nice. :)

Junio only took 1 to 5 into pu; 6, 7 and its sidekick 8, 10 and 11
conflict with sb/more-repo-in-api; 9 could go in unmodified.

René

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

* [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups
  2019-01-06 16:39 ` jk/loose-object-cache René Scharfe
@ 2019-01-07  8:31   ` Jeff King
  2019-01-08 16:40     ` René Scharfe
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff King @ 2019-01-07  8:31 UTC (permalink / raw)
  To: René Scharfe
  Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason

On Sun, Jan 06, 2019 at 05:39:14PM +0100, René Scharfe wrote:

> Am 28.12.2018 um 19:04 schrieb Junio C Hamano:
> > * jk/loose-object-cache (2018-11-24) 10 commits
> >   (merged to 'next' on 2018-12-28 at 5a5faf384e)
> >  + odb_load_loose_cache: fix strbuf leak
> >  + fetch-pack: drop custom loose object cache
> >  + sha1-file: use loose object cache for quick existence check
> >  + object-store: provide helpers for loose_objects_cache
> >  + sha1-file: use an object_directory for the main object dir
> >  + handle alternates paths the same as the main object dir
> >  + sha1_file_name(): overwrite buffer instead of appending
> >  + rename "alternate_object_database" to "object_directory"
> >  + submodule--helper: prefer strip_suffix() to ends_with()
> >  + fsck: do not reuse child_process structs
> > 
> >  Originally merged to 'next' on 2018-11-24
> > 
> >  Code clean-up with optimization for the codepath that checks
> >  (non-)existence of loose objects.
> > 
> >  Will merge to 'master'.
> 
> So this has hit master in the meantime.  We discussed a sort performance
> fix in [1]; I'll reply with a short series containing a cleaned-up and
> rebased version as a follow-up.
> 
>   object-store: factor out odb_loose_cache()
>   object-store: factor out odb_clear_loose_cache()
>   object-store: use one oid_array per subdirectory for loose cache

Thanks! With the exception of one tiny nit, this looks good to me.

I also cleaned up my sha1/object_id patch and rebased it on top of what
you have here. Though as I worked on it, it expanded in scope a bit.
Possibly it should be a separate series entirely, but that would create
some annoying textual conflicts on merge.

  [01/11]: sha1-file: fix outdated sha1 comment references
  [02/11]: update comment references to sha1_object_info()
  [03/11]: http: use struct object_id instead of bare sha1
  [04/11]: sha1-file: modernize loose object file functions
  [05/11]: sha1-file: modernize loose header/stream functions
  [06/11]: sha1-file: convert pass-through functions to object_id
  [07/11]: convert has_sha1_file() callers to has_object_file()
  [08/11]: sha1-file: drop has_sha1_file()
  [09/11]: sha1-file: prefer "loose object file" to "sha1 file" in messages
  [10/11]: sha1-file: avoid "sha1 file" for generic use in messages
  [11/11]: prefer "hash mismatch" to "sha1 mismatch"

 Documentation/git-fsck.txt |   6 +-
 apply.c                    |   2 +-
 builtin/cat-file.c         |   6 +-
 builtin/fetch.c            |   7 +-
 builtin/index-pack.c       |   2 +-
 builtin/pack-objects.c     |   4 +-
 builtin/reflog.c           |   2 +-
 builtin/show-ref.c         |   2 +-
 bulk-checkin.c             |   2 +-
 cache-tree.c               |   4 +-
 cache.h                    |   6 +-
 http-push.c                |   2 +-
 http-walker.c              |  10 +-
 http.c                     |  14 +--
 http.h                     |   6 +-
 object-store.h             |  20 ++--
 object.c                   |   4 +-
 refs.c                     |   2 +-
 send-pack.c                |   2 +-
 sha1-file.c                | 219 +++++++++++++++++--------------------
 streaming.c                |  16 +--
 t/t1450-fsck.sh            |   2 +-
 22 files changed, 161 insertions(+), 179 deletions(-)

-Peff

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

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-28 18:04 What's cooking in git.git (Dec 2018, #02; Fri, 28) Junio C Hamano
2019-01-06 16:39 ` jk/loose-object-cache René Scharfe
2019-01-07  8:31   ` [PATCH 0/11] jk/loose-object-cache sha1/object_id fixups Jeff King
2019-01-08 16:40     ` René Scharfe
2019-01-08 17:39       ` Junio C Hamano
2019-01-08 18:05         ` Jeff King
2019-01-08 18:07           ` Junio C Hamano
2019-01-08 18:27             ` Derrick Stolee
2019-01-08 18:52             ` Junio C Hamano
2019-01-08 21:16               ` Jeff King
2019-01-09 21:37                 ` Stefan Beller
2019-01-09 22:42                   ` Stefan Beller
2019-01-10  6:17                     ` Jeff King

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


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