From: Stefan Beller
Add a repository argument to allow the link_alt_odb_entries caller to
be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commits,
From: Stefan Beller
Add a repository argument to allow the read_info_alternates caller to
be more specific about which repository to read from. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commi
From: Stefan Beller
Add a repository argument to allow the link_alt_odb_entry caller to be
more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commit, us
From: Stefan Beller
Add a repository argument to allow the alt_odb_usable caller to be
more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
Since the implementation does not y
The approximate_object_count() function maintains a rough count of
objects in a repository to estimate how long object name abbreviates
should be. Object names are scoped to a repository and the
appropriate length may differ by repository, so the object count
should not be global.
Signed-off-by:
In a process with multiple repositories open, packfile accessors
should be associated to a single repository and not shared globally.
Move packed_git and packed_git_mru into the_repository and adjust
callers to reflect this.
Patch generated by
1. Moving the struct packed_git declaration to objec
Each repository's object store can be initialized independently, so
they must not share a run_once variable.
Signed-off-by: Stefan Beller
Signed-off-by: Jonathan Nieder
---
object-store.h | 8 +++-
packfile.c | 7 +++
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/o
On 08/30/2017 06:55 AM, Jeff King wrote:
> [...]
> Something like this, which AFAICT is about as safe as the existing code
> in its list manipulation. It retains the "active" flag as an additional
> check which I think isn't strictly necessary, but potentially catches
> some logic errors.
>
> The
From: Stefan Beller
In a process with multiple repositories open, alternates should be
associated to a single repository and not shared globally. Move
alt_odb_list and alt_odb_tail into the_repository and adjust callers
to reflect this.
No functional change intended.
Signed-off-by: Stefan Belle
From: Stefan Beller
The object store field will contain any objects needed for access to
objects in a given repository.
This patch introduces the object store but for now it is empty. C99
forbids empty structs, but common C compilers cope well with them and
this struct will gain members very so
The MRU cache that keeps track of recently used packs is represented
using two global variables:
struct mru packed_git_mru_storage;
struct mru *packed_git_mru = &packed_git_mru_storage;
Callers never assign to the packed_git_mru pointer, though, so we can
simplify by eliminating i
Hi,
Most of the credit for this series should go to Stefan Beller. I just
decided to pull the trigger on sending out what we have so far.
This series is about API. It makes no functional change yet.
Today, when a git command wants to operate on some objects from another
repository (e.g., a sub
On Wed, Aug 30, 2017 at 12:55:55AM -0400, Jeff King wrote:
> > I feel like right now we meet (1) and not (2). But I think if we keep to
> > that lower bar of (1), it might not be that bad. We're assuming now that
> > there's no race on the tempfile->active flag, for instance. We could
> > probably
On Tue, Aug 29, 2017 at 05:48:09PM -0400, Jeff King wrote:
> On Tue, Aug 29, 2017 at 09:22:08PM +0200, Martin Ågren wrote:
>
> > One take-away for me from this thread is that memory-leak-plugging seems
> > to be appreciated, i.e., it's not just an academic problem. I think I'll
> > look into it s
On Wed, Aug 30, 2017 at 12:31:47AM -0400, Jeff King wrote:
> > It was surprisingly hard trying to get that code to do the right thing,
> > non-racily, in every error path. Since `remove_tempfiles()` can be
> > called any time (even from a signal handler), the linked list of
> > `tempfile` objects
On Wed, Aug 30, 2017 at 6:31 AM, Jeff King wrote:
> On Wed, Aug 30, 2017 at 05:25:18AM +0200, Michael Haggerty wrote:
>> It was surprisingly hard trying to get that code to do the right thing,
>> non-racily, in every error path. Since `remove_tempfiles()` can be
>> called any time (even from a sig
On Wed, Aug 30, 2017 at 05:25:18AM +0200, Michael Haggerty wrote:
> >>> In the long run we may want to drop the "tempfiles must remain forever"
> >>> rule. This is certainly not the first time it has caused confusion or
> >>> leaks. And I don't think it's a fundamental issue, just the way the code
On 08/29/2017 09:12 PM, Jeff King wrote:
> On Tue, Aug 29, 2017 at 12:09:28PM -0700, Brandon Williams wrote:
>
>>> -- >8 --
>>> Subject: [PATCH] config: use a static lock_file struct
>>>
>>> When modifying git config, we xcalloc() a struct lock_file
>>> but never free it. This is necessary because
On Tue, Aug 29, 2017 at 04:08:25PM -0400, Jeff Hostetler wrote:
> I just wanted to jump in here and say I've done some initial
> testing of this against VSTS and so far it seems fine. And yes,
> we have a custom git server.
Great, thank you for checking.
> VSTS doesn't support the "git://" prot
On 08/29/2017 08:07 PM, Brandon Williams wrote:
> On 08/29, Michael Haggerty wrote:
>> [...]
>> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
>> index d19b3bfba5..83a088118f 100644
>> --- a/refs/packed-backend.c
>> +++ b/refs/packed-backend.c
>> @@ -1082,7 +1082,50 @@ static int packed
v3 looks good to me. Thanks!
Reviewed-by: Michael Haggerty
Michael
On Tue, Aug 29, 2017 at 09:22:08PM +0200, Martin Ågren wrote:
> One take-away for me from this thread is that memory-leak-plugging seems
> to be appreciated, i.e., it's not just an academic problem. I think I'll
> look into it some more, i.e., slowly pursue option 2. :-) That might help
> give som
On 08/29, Jeff Hostetler wrote:
>
>
> On 8/25/2017 1:35 PM, Jonathan Nieder wrote:
> >Hi,
> >
> >Jeff King wrote:
> >>On Thu, Aug 24, 2017 at 03:53:21PM -0700, Brandon Williams wrote:
> >
> >>>Another version of Git's wire protocol is a topic that has been discussed
> >>>and
> >>>attempted by ma
On 8/25/2017 1:35 PM, Jonathan Nieder wrote:
Hi,
Jeff King wrote:
On Thu, Aug 24, 2017 at 03:53:21PM -0700, Brandon Williams wrote:
Another version of Git's wire protocol is a topic that has been discussed and
attempted by many in the community over the years. The biggest challenge, as
fa
On 29 August 2017 at 20:53, Jeff King wrote:
> On Tue, Aug 29, 2017 at 06:51:52PM +0100, Lars Schneider wrote:
>> What if we run a few selected tests with valgrind and count all files that
>> valgrind mentions (a single leak has multiple file mentions because of
>> the stack trace and other leak i
On Tue, Aug 29, 2017 at 12:09:28PM -0700, Brandon Williams wrote:
> > -- >8 --
> > Subject: [PATCH] config: use a static lock_file struct
> >
> > When modifying git config, we xcalloc() a struct lock_file
> > but never free it. This is necessary because the tempfile
> > code (upon which the locki
On 08/29, Brandon Williams wrote:
> On 08/29, Jeff King wrote:
> > On Tue, Aug 29, 2017 at 02:53:41PM -0400, Jeff King wrote:
> >
> > > It looks like the config code has a minor-ish leak. Patch to follow.
> >
> > Here it is.
> >
> > -- >8 --
> > Subject: [PATCH] config: use a static lock_file st
On 08/29, Jeff King wrote:
> On Tue, Aug 29, 2017 at 02:53:41PM -0400, Jeff King wrote:
>
> > It looks like the config code has a minor-ish leak. Patch to follow.
>
> Here it is.
>
> -- >8 --
> Subject: [PATCH] config: use a static lock_file struct
>
> When modifying git config, we xcalloc() a
On Tue, Aug 29, 2017 at 02:53:41PM -0400, Jeff King wrote:
> It looks like the config code has a minor-ish leak. Patch to follow.
Here it is.
-- >8 --
Subject: [PATCH] config: use a static lock_file struct
When modifying git config, we xcalloc() a struct lock_file
but never free it. This is nec
On Tue, Aug 29, 2017 at 06:51:52PM +0100, Lars Schneider wrote:
> I set $TOOL_OPTIONS in valgrind.sh: to
> '--leak-check=full --errors-for-leak-kinds=definite'
>
> ... but I also had to adjust t/test-lib-functions.sh:test_create_repo
> as I ran into the error "cannot run git init -- have you bui
On 08/29, Michael Haggerty wrote:
> This should be the second-to-last patch series in the quest for
> mmapped packed-refs.
>
> Before this patch series, there is still more coupling than necessary
> between `files_ref_store` and `packed_ref_store`:
>
> * `files_ref_store` adds packed references (
On 08/29, Michael Haggerty wrote:
> Now the outside world interacts with the packed ref store only via the
> generic refs API plus a few lock-related functions. This allows us to
> delete some functions that are no longer used, thereby completing the
> encapsulation of the packed ref store.
Love s
On 08/29, Michael Haggerty wrote:
> Implement `packed_delete_refs()` using a reference transaction. This
> means that `files_delete_refs()` can use `refs_delete_refs()` instead
> of `repack_without_refs()` to delete any packed references, decreasing
> the coupling between the classes.
>
> Signed-o
> On 28 Aug 2017, at 06:11, Martin Ågren wrote:
>
> On 28 August 2017 at 01:23, Jeff King wrote:
>> On Sun, Aug 27, 2017 at 10:04:55PM +0200, Lars Schneider wrote:
>>
>>> I did run all tests under valgrind using "make valgrind" and I found
>>> the following files with potential issues:
>>>
>>
In one code path we return a literal -1 and not a symbolic constant. The
value -1 would be interpreted as TRANSACTION_NAME_CONFLICT, which is
wrong. Use TRANSACTION_GENERIC_ERROR instead (that is the only other
return value we have to choose from).
Noticed-by: Michael Haggerty
Signed-off-by: Mart
After the previous patch, none of the functions we call hold on to
`referent.buf`, so we can safely release the string buffer before
returning.
Signed-off-by: Martin Ågren
---
v3: identical to v2
refs/files-backend.c | 31 ---
1 file changed, 20 insertions(+), 11 del
split_symref_update() receives a string-pointer `referent` and adds it
to the list of `affected_refnames`. The list simply holds on to the
pointers it is given, it does not copy the strings and it does not ever
free them. The `referent` string in split_symref_update() belongs to a
string buffer in
On Tue, Aug 29, 2017 at 12:23:11PM +0100, Daniel Watkins wrote:
> Now that `make` produces a file, we should have a clean target to remove
> it.
> [...]
> +clean:
> + $(RM) diff-highlight
> +
Makes sense. Thanks!
-Peff
On Tue, Aug 29, 2017 at 03:58:00PM +, Kevin Willford wrote:
> > > The return value of the get_files_dirs call is never being used.
> > > Looking at the history of the file and it was originally only
> > > being used for debug output statements. Also when
> > > read_tree_recursive return value
>
> On Mon, Aug 28, 2017 at 02:28:28PM -0600, Kevin Willford wrote:
>
> > The return value of the get_files_dirs call is never being used.
> > Looking at the history of the file and it was originally only
> > being used for debug output statements. Also when
> > read_tree_recursive return value
On Mon, Aug 28, 2017 at 8:59 PM, Ben Peart wrote:
>
> On 8/3/2017 5:19 AM, Christian Couder wrote:
>>
>> +Helpers
>> +===
>> +
>> +ODB helpers are commands that have to be registered using either the
>> +"odb..subprocessCommand" or the "odb..scriptCommand"
>> +config variables.
>> +
>> +Regist
Stefan Beller venit, vidit, dixit 28.08.2017 20:24:
> On Sat, Aug 26, 2017 at 7:47 AM, Kévin Le Gouguec
> wrote:
>> Hi,
>>
>> I've asked this question on the git-users Google Groups list[1], and
>> while the answers there were interesting, I still cannot figure
>> whether my problem comes from an
Now that `make` produces a file, we should have a clean target to remove
it.
Signed-off-by: Daniel Watkins
---
contrib/diff-highlight/Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile
index fbf5c5824..f2be7cc92 10064
On 29 August 2017 at 10:39, Michael Haggerty wrote:
> On 08/28/2017 10:32 PM, Martin Ågren wrote:
>> After the previous patch, none of the functions we call hold on to
>> `referent.buf`, so we can safely release the string buffer before
>> returning.
>
> This patch looks good to me, but I did noti
On Fri, Aug 25, 2017 at 11:23 PM, Jonathan Tan wrote:
> On Fri, 25 Aug 2017 08:14:08 +0200
> Christian Couder wrote:
>
>> As Git is used by more and more by people having different needs, I
>> think it is not realistic to expect that we can optimize its object
>> storage for all these different n
On Mon, Aug 28, 2017 at 02:28:29PM -0600, Kevin Willford wrote:
> The code was using two string_lists, one for the directories and
> one for the files. The code never checks the lists independently
> so we should be able to only use one list. The string_list also
> is a O(log n) for lookup and i
On 08/28/2017 10:32 PM, Martin Ågren wrote:
> After the previous patch, none of the functions we call hold on to
> `referent.buf`, so we can safely release the string buffer before
> returning.
This patch looks good to me, but I did notice a pre-existing problem in
the area...
> ---
> refs/files
On 08/28/2017 10:32 PM, Martin Ågren wrote:
> split_symref_update() receives a string-pointer `referent` and adds it
> to the list of `affected_refnames`. The list simply holds on to the
> pointers it is given, it does not copy the strings and it never frees
> them. The `referent` string in split_s
Implement the methods needed to support reference transactions for
the packed-refs backend. The new methods are not yet used.
Signed-off-by: Michael Haggerty
---
refs/packed-backend.c | 313 +-
refs/packed-backend.h | 9 ++
2 files changed, 319 i
`packed_ref_store` is going to want to store some transaction-wide
data, so make a place for it.
Signed-off-by: Michael Haggerty
---
refs/refs-internal.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index b02dc5a7e3..d7d344de73 100644
--- a/refs
If the deletion steps unexpectedly fail, it is less bad to leave a
reference without its reflog than it is to leave a reflog without its
reference, since the latter is an invalid repository state.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 35 +--
Now the outside world interacts with the packed ref store only via the
generic refs API plus a few lock-related functions. This allows us to
delete some functions that are no longer used, thereby completing the
encapsulation of the packed ref store.
Signed-off-by: Michael Haggerty
---
refs/packe
When processing a `files_ref_store` transaction, it is sometimes
necessary to delete some references from the "packed-refs" file. Do
that using a reference transaction conducted against the
`packed_ref_store`.
This change further decouples `files_ref_store` from
`packed_ref_store`. It also fixes m
The old code incremented the packed ref cache reference count when
acquiring the packed-refs lock, and decremented the count when
releasing the lock. This is unnecessary because a locked packed-refs
file cannot be changed, so there is no reason for the cache to become
stale.
Moreover, the extra re
Implement `packed_delete_refs()` using a reference transaction. This
means that `files_delete_refs()` can use `refs_delete_refs()` instead
of `repack_without_refs()` to delete any packed references, decreasing
the coupling between the classes.
Signed-off-by: Michael Haggerty
---
refs/files-backe
Used a `packed_ref_store` transaction in the implementation of
`files_initial_transaction_commit()` rather than using internal
features of the packed ref store. This further decouples
`files_ref_store` from `packed_ref_store`.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 29 +++
Now that the packed reference store supports transactions, we can use
a transaction to write the packed versions of references that we want
to pack. This decreases the coupling between `files_ref_store` and
`packed_ref_store`.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 24 +++
Currently, a loose reference is deleted even before locking the
`packed-refs` file, let alone deleting any packed version of the
reference. This leads to two problems, demonstrated by two new tests:
* While a reference is being deleted, other processes might see the
old, packed value of the refe
This should be the second-to-last patch series in the quest for
mmapped packed-refs.
Before this patch series, there is still more coupling than necessary
between `files_ref_store` and `packed_ref_store`:
* `files_ref_store` adds packed references (e.g., during `git clone`
and `git pack-refs`)
On Mon, Aug 28, 2017 at 06:45:29PM -0400, Ben Peart wrote:
> > Since the debug output has been removed and the caller isn't
> > checking the return value there is no reason to keep calulating
> > and returning a value.
> >
>
> Did a quick search and you're right, nothing is using the return valu
On Mon, Aug 28, 2017 at 02:28:28PM -0600, Kevin Willford wrote:
> The return value of the get_files_dirs call is never being used.
> Looking at the history of the file and it was originally only
> being used for debug output statements. Also when
> read_tree_recursive return value is non zero it
On Mon, Aug 28, 2017 at 02:28:27PM -0600, Kevin Willford wrote:
> In merge_trees if process_renames or process_entry returns less
> than zero, the method will just return and not free re_merge,
> re_head, or entries.
>
> This change cleans up the allocated variables before returning
> to the call
62 matches
Mail list logo