* Re: [PATCH v3 1/5] http: support file handles for HTTP_KEEP_ERROR
From: SZEDER Gábor @ 2019-01-09 12:15 UTC (permalink / raw)
To: Masaya Suzuki; +Cc: git, jrnieder, peff, sunshine
In-Reply-To: <20190108024741.62176-2-masayasuzuki@google.com>
On Mon, Jan 07, 2019 at 06:47:37PM -0800, Masaya Suzuki wrote:
> HTTP_KEEP_ERROR makes it easy to debug HTTP transport errors. In order
> to make HTTP_KEEP_ERROR enabled for all requests, file handles need to
> be supported.
>
> Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
> ---
> http.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/http.c b/http.c
> index 0b6807cef9..06450da96e 100644
> --- a/http.c
> +++ b/http.c
> @@ -1991,16 +1991,19 @@ static int http_request_reauth(const char *url,
> /*
> * If we are using KEEP_ERROR, the previous request may have
> * put cruft into our output stream; we should clear it out before
> - * making our next request. We only know how to do this for
> - * the strbuf case, but that is enough to satisfy current callers.
> + * making our next request.
> */
> if (options && options->keep_error) {
> switch (target) {
> case HTTP_REQUEST_STRBUF:
> strbuf_reset(result);
> break;
> + case HTTP_REQUEST_FILE:
> + fflush(result);
> + ftruncate(fileno(result), 0);
Some GCC versions complain about the above line:
http.c: In function ‘http_request_reauth’:
http.c:1961:3: error: ignoring return value of ‘ftruncate’, declared with attribute warn_unused_result [-Werror=unused-result]
ftruncate(fileno(result), 0);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
CC shell.o
CC remote-testsvn.o
CC vcs-svn/line_buffer.o
cc1: all warnings being treated as errors
make: *** [http.o] Error 1
make: *** Waiting for unfinished jobs....
> + break;
> default:
> - BUG("HTTP_KEEP_ERROR is only supported with strbufs");
> + BUG("Unknown http_request target");
> }
> }
>
> --
> 2.20.1.97.g81188d93c3-goog
>
^ permalink raw reply
* Re: Translation of git manpages
From: Jean-Noël Avila @ 2019-01-09 11:24 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Alexander Shopov, Jordi Mas, Ralf Thielow, Christopher Díaz,
Marco Paolone, Gwan-gyeong Mun, Vasco Almeida,
Dimitriy Ryazantcev, Peter Krefting,
Trần Ngọc Quân, Alessandro Menti, Jiang Xin, git,
Jeff King
In-Reply-To: <87lg3v6yqr.fsf@evledraar.gmail.com>
On Wed, Jan 09 2019, Ævar Arnfjörð Bjarmason wrote:
> > I think a way to have early exposure of these to a lot more people >
would be to have these on the git-scm.com site. Jeff knows more about >
the build process there. > > I see the general completion of French &
German is at ~10%, but > maybe there's some pages that are fully
translated already? We could > then have some UI or git-scm.com that
allows you to switch between > languages along with some general
overview page per-language.
Publishing on git-scm is on my TODO list. It' s a bit more difficult
than updating the original manpages, because the translation process
with po4a cannot be done in memory, which does not fit with the way the
publishing is done directly from gitster/git repo on github into the
database, plus the underlying data structure of the website needs to
evolve to account for new languages (and Ruby on Rails is a special beast) .
> > > Are the translations contributed through >
https://hosted.weblate.org/projects/git-manpages/translations/ >
something that license-wise (Signed-off-by etc.) we'd eventually be >
able to upstream in git.git?
I retained the same license as Git for the translation repo, to prevent
future issues with integration of the result in other projects. Right
now, there's no Signed-off-by line in Weblate issued commits. I opened
an issue to have them added. Should this policy have been enforced from
the start or is it too late for later inclusion in git.git?
> > It would be great to eventually have something we can build as part >
of our build process, so e.g. Debian can have git-man-de similar to >
manpages-de now.
I don't understand you remark. The point of the proposed patch is to
allow to build and install the translated manpages from a working tree
of git, by adding a checkout of git-manpages-l10n in a subdir of
Documentation (note to self: propose a patch to document this). From
this setup, generating proper localized manpage packages is perfectly
doable. Am I missing anything?
^ permalink raw reply
* Re: Translation of git manpages
From: Jeff King @ 2019-01-09 10:32 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Jean-Noël AVILA, Alexander Shopov, Jordi Mas, Ralf Thielow,
Christopher Díaz, Marco Paolone, Gwan-gyeong Mun,
Vasco Almeida, Dimitriy Ryazantcev, Peter Krefting,
Trần Ngọc Quân, Alessandro Menti, Jiang Xin, git
In-Reply-To: <87lg3v6yqr.fsf@evledraar.gmail.com>
On Tue, Jan 08, 2019 at 02:54:36PM +0100, Ævar Arnfjörð Bjarmason wrote:
> > There is already a kernel of translation in French, from my experiments and a
> > previous effort of German translation[6] was gettextized. If you have such
> > archives for other languages, I'd be happy to integrate them, even if they are
> > not up to date.
>
> Thanks. This has come up on list many times before and it's great to
> have some movement on it.
>
> I think a way to have early exposure of these to a lot more people would
> be to have these on the git-scm.com site. Jeff knows more about the
> build process there.
Actually, Jean-Noël has done as much or more work as me on the manpage
(and book) build process for the site. :)
Basically, we just pull the pages from git.git, run them through
asciidoctor with a few ugly regex tweaks, and then that goes to the
site. We should be able to do the same for translations, and I agree
that even partial translations on the site may be a good way to get
exposure. The way the book translations are handled is probably a
workable model.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2019, #01; Mon, 7)
From: Jeff King @ 2019-01-09 10:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, git
In-Reply-To: <xmqq7efg6o0d.fsf@gitster-ct.c.googlers.com>
On Mon, Jan 07, 2019 at 03:34:10PM -0800, Junio C Hamano wrote:
> * jk/proto-v2-hidden-refs-fix (2018-12-14) 3 commits
> - upload-pack: support hidden refs with protocol v2
> - parse_hide_refs_config: handle NULL section
> - serve: pass "config context" through to individual commands
>
> The v2 upload-pack protocol implementation failed to honor
> hidden-ref configuration, which has been corrected.
>
> Will merge to 'next'.
Sorry I didn't catch this before it hit 'next', but I think we were
planning to scrap this in favor of:
https://public-inbox.org/git/20181218124750.GC30471@sigill.intra.peff.net/
-Peff
^ permalink raw reply
* Re: Typo in a Windows installer item description.
From: Ævar Arnfjörð Bjarmason @ 2019-01-09 10:05 UTC (permalink / raw)
To: Aleksey Svistunov; +Cc: git
In-Reply-To: <6412c78a-b09f-0652-26d8-a0a4e80da952@dev.zodiac.tv>
On Wed, Jan 09 2019, Aleksey Svistunov wrote:
> Hi there.
>
> I could be wrong but there is a typo on one of the installation step's
> text. A word sport instead of support is used.
>
> Here is a screenshot in attachments which shows where exactly the typo is.
It's not a typo, "sport" can be used as a verb meaning to display or
have a notable feature, e.g.[1]:
Jen's sporting a new pair of shoes; he was sporting a new wound from
the combat
But maybe that copy should be simplified. I think the GFW bug tracker is
the right place to raise that as a suggestion[2].
1. https://en.wiktionary.org/wiki/sport#Verb
2. https://github.com/git-for-windows/git/issues
^ permalink raw reply
* Typo in a Windows installer item description.
From: Aleksey Svistunov @ 2019-01-09 9:39 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 380 bytes --]
Hi there.
I could be wrong but there is a typo on one of the installation step's
text. A word sport instead of support is used.
Here is a screenshot in attachments which shows where exactly the typo is.
Best wishes!
P.s. my personal email (svav@ymail.com) was denied by your post system
with a message "policy analysis reported: Your address is not liked
source for email"
[-- Attachment #2: git_typo.jpg --]
[-- Type: image/jpeg, Size: 84743 bytes --]
^ permalink raw reply
* Re: Regression: submodule worktrees can clobber core.worktree config
From: Duy Nguyen @ 2019-01-09 9:12 UTC (permalink / raw)
To: Tomasz Śniatowski; +Cc: Git Mailing List
In-Reply-To: <CAG0vfyTdAyEeAuNUpjTrMjUpmT0XNx1ffdbQwYS3fs13UFnP6w@mail.gmail.com>
On Wed, Jan 9, 2019 at 1:40 PM Tomasz Śniatowski <tsniatowski@vewd.com> wrote:
> > The good news is, I have something that should make it work reliably.
> > But I don't know if it will make it to 2.21 or not.
>
> That's good to hear, is there something I can try out or track?
You can try this
https://gitlab.com/pclouds/git/commits/submodules-in-worktrees
which should support multiple worktrees in either submodules or
supermodules. Be careful though, that branch is only reviewed by me
and I'm not a heavy submodule user to give it more day-to-day testing.
For tracking, if you want I can CC you when I send these patches here
for review. Otherwise you can check Junio's "what's cooking" mails
from time to time and search "submodule".
--
Duy
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2019, #01; Mon, 7)
From: Martin Ågren @ 2019-01-09 7:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, brian m. carlson
In-Reply-To: <xmqq7efg6o0d.fsf@gitster-ct.c.googlers.com>
On Tue, 8 Jan 2019 at 00:34, Junio C Hamano <gitster@pobox.com> wrote:
> * bc/sha-256 (2018-11-14) 12 commits
> - hash: add an SHA-256 implementation using OpenSSL
> - sha256: add an SHA-256 implementation using libgcrypt
> - Add a base implementation of SHA-256 support
> - commit-graph: convert to using the_hash_algo
> - t/helper: add a test helper to compute hash speed
> - sha1-file: add a constant for hash block size
> - t: make the sha1 test-tool helper generic
> - t: add basic tests for our SHA-1 implementation
> - cache: make hashcmp and hasheq work with larger hashes
> - hex: introduce functions to print arbitrary hashes
> - sha1-file: provide functions to look up hash algorithms
> - sha1-file: rename algorithm to "sha1"
>
> Add sha-256 hash and plug it through the code to allow building Git
> with the "NewHash".
AddressSanitizer barks at current pu (855f98be272f19d16564e) for a
handful of tests.
One example is t5702-protocol-v2.sh. I've tracked this one down to
ce1a82c251 ("Merge branch 'bc/sha-256' into jch", 2019-01-08).
Reverting that merge makes the problem go away, at least in t5702.
Notably bc/sha-256 on its own has no problems with t5702, possibly
because there has been quite some work done on the test script itself
in bc/sha-256..pu.
There are a few failing tests with AddressSanitizer on pu and they might
be caused by different topics. I've got to run, but thought I'd just
mention this one for now. Here's AddressSanitizer's first complaint for
t5702:
==1691823==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x6040000004f2 at pc 0x0000004ea0fd bp 0x7ffc53082590 sp
0x7ffc53081d40
READ of size 32 at 0x6040000004f2 thread T0
#0 0x4ea0fc in __asan_memcpy
llvm/projects/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cc:23
#1 0x8603ec in oidset_insert oidset.c
#2 0x86c977 in add_promisor_object packfile.c:2129:4
#3 0x86c07a in for_each_object_in_pack packfile.c:2070:7
#4 0x86c535 in for_each_packed_object packfile.c:2095:7
#5 0x86c651 in is_promisor_object packfile.c:2151:4
#6 0x5ae77d in mark_object builtin/fsck.c:173:6
#7 0x5b0824 in mark_object_reachable builtin/fsck.c:200:2
#8 0x5b0824 in fsck_handle_ref builtin/fsck.c:509
#9 0x8e6987 in do_for_each_repo_ref_iterator refs/iterator.c:418:12
#10 0x8cb84f in do_for_each_ref refs.c:1466:9
#11 0x8cb84f in refs_for_each_rawref refs.c:1532
#12 0x8cb84f in for_each_rawref refs.c:1538
#13 0x5acd1f in get_default_heads builtin/fsck.c:524:2
#14 0x5acd1f in cmd_fsck builtin/fsck.c:846
#15 0x52f022 in run_builtin git.c:422:11
#16 0x52d583 in handle_builtin git.c:655:8
#17 0x52c00b in run_argv git.c:709:4
#18 0x52c00b in cmd_main git.c:806
#19 0x6c4569 in main common-main.c:45:9
#20 0x7f5fd6c23b96 in __libc_start_main
/build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
#21 0x41c799 in _start (git+0x41c799)
0x6040000004f2 is located 0 bytes to the right of 34-byte region
[0x6040000004d0,0x6040000004f2)
allocated by thread T0 here:
#0 0x4eb4cf in malloc
llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146
#1 0x9fa1db in do_xmalloc wrapper.c:60:8
#2 0x9fa2fd in do_xmallocz wrapper.c:100:8
#3 0x9fa2fd in xmallocz_gently wrapper.c:113
#4 0x86a877 in unpack_compressed_entry packfile.c:1588:11
#5 0x86a02e in unpack_entry packfile.c:1737:11
#6 0x867431 in cache_or_unpack_entry packfile.c:1439:10
#7 0x867431 in packed_object_info packfile.c:1506
#8 0x96b7be in oid_object_info_extended sha1-file.c:1394:10
#9 0x96d7d0 in read_object sha1-file.c:1434:6
#10 0x96d7d0 in read_object_file_extended sha1-file.c:1476
#11 0x85cf40 in repo_read_object_file ./object-store.h:174:9
#12 0x85cf40 in parse_object object.c:273
#13 0x86c752 in add_promisor_object packfile.c:2108:23
#14 0x86c07a in for_each_object_in_pack packfile.c:2070:7
#15 0x86c535 in for_each_packed_object packfile.c:2095:7
#16 0x86c651 in is_promisor_object packfile.c:2151:4
#17 0x5ae77d in mark_object builtin/fsck.c:173:6
#18 0x5b0824 in mark_object_reachable builtin/fsck.c:200:2
#19 0x5b0824 in fsck_handle_ref builtin/fsck.c:509
#20 0x8e6987 in do_for_each_repo_ref_iterator refs/iterator.c:418:12
#21 0x8cb84f in do_for_each_ref refs.c:1466:9
#22 0x8cb84f in refs_for_each_rawref refs.c:1532
#23 0x8cb84f in for_each_rawref refs.c:1538
#24 0x5acd1f in get_default_heads builtin/fsck.c:524:2
#25 0x5acd1f in cmd_fsck builtin/fsck.c:846
#26 0x52f022 in run_builtin git.c:422:11
#27 0x52d583 in handle_builtin git.c:655:8
#28 0x52c00b in run_argv git.c:709:4
#29 0x52c00b in cmd_main git.c:806
#30 0x6c4569 in main common-main.c:45:9
#31 0x7f5fd6c23b96 in __libc_start_main
/build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
^ permalink raw reply
* Re: [PATCH 3/3] t0006-date.sh: add `human` date format tests.
From: Johannes Sixt @ 2019-01-09 6:58 UTC (permalink / raw)
To: Stephen P. Smith
Cc: git, Linus Torvalds, Junio C Hamano,
Ævar Arnfjörð Bjarmason
In-Reply-To: <6530822.TNJIEUz5BA@thunderbird>
Am 09.01.19 um 01:44 schrieb Stephen P. Smith:
> On Tuesday, January 8, 2019 2:27:22 PM MST Johannes Sixt wrote:
>> Am 31.12.18 um 01:31 schrieb Stephen P. Smith:
>>> +
>>> +TODAY_REGEX='[A-Z][a-z][a-z] [012][0-9]:[0-6][0-9] .0200'
> <snip>
>> The $...REGEX expansions must be put in double-quotes to protect them
>> from field splitting. But then the tests do not pass anymore (I tested
>> only t4202). Please revisit this change.
>
> I will later figure out why you are seeing the fields splitting but I am not.
> In the mean time I will change the quoting.
In this line
TODAY_REGEX='[A-Z][a-z][a-z] [012][0-9]:[0-6][0-9] .0200'
no field splitting occurs. The quoting is fine here. But notice that the
value of $TODAY_REGEX contains blanks.
In this line
check_human_date "$(($(date +%s)-18000)) +0200" $TODAY_REGEX
the value of $TODAY_REGEX is substituted and then the value is split
into fields at the blanks because the expansion is not quoted.
As a consequence, function check_human_date considers only the first
part of $TODAY_REGEX, i.e. 'A-Z][a-z][a-z]' (which is parameter $2), but
ignores everything else (because it does not use $3 or $4).
-- Hannes
^ permalink raw reply
* Re: Regression: submodule worktrees can clobber core.worktree config
From: Tomasz Śniatowski @ 2019-01-09 6:40 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8Cvc8v_4OEmpgKPWSO5csV6jRya7mnSQjEs4mMhHRq4AQ@mail.gmail.com>
On Wed, 9 Jan 2019 at 00:23, Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Wed, Jan 9, 2019 at 5:56 AM Tomasz Śniatowski <tsniatowski@vewd.com> wrote:
> >
> > After upgrading to 2.20.1 I noticed in some submodule+worktree scenarios git
> > will break the submodule configuration. Reproducible with:
> > git init a && (cd a; touch a; git add a; git commit -ma)
> > git init b && (cd b; git submodule add ../a; git commit -mb)
> > git -C b worktree add ../b2
> > git -C b/a worktree add ../../b2/a
> > git -C b status
> > git -C b2 submodule update
> > git -C b status
> >
> > The submodule update in the _worktree_ puts an invalid core.worktree value in
> > the _original_ repository submodule config (b/.git/modules/a/config), causing
> > the last git status to error out with:
> > fatal: cannot chdir to '../../../../../../b2/a': No such file or directory
> > fatal: 'git status --porcelain=2' failed in submodule a
> >
> > Looking at the config file itself, the submodule update operation applies the
> > following change (the new path is invalid):
> > - worktree = ../../../a
> > + worktree = ../../../../../../b2/a
> >
> > This worked fine on 2.19.2 (no config change, no error), and was useful to have
> > a worktree with (large) submodules that are also worktrees.
>
> This scenario is not supported (or at least known to be broken in
> theory) so I wouldn't call this a regression even if it happens to
> work on 2.19.2 for some reason.
This scenario worked fine for quite a while now, at least since around 2.15
(as I started using this early 2018). It "just worked", to be honest, just
needed the worktree submodules to be manually set up as worktrees too.
> The good news is, I have something that should make it work reliably.
> But I don't know if it will make it to 2.21 or not.
That's good to hear, is there something I can try out or track?
--
Tomasz Śniatowski
^ permalink raw reply
* Re: git-lfs integration?
From: Harald Dunkel @ 2019-01-09 6:10 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git
In-Reply-To: <87k1jf6tls.fsf@evledraar.gmail.com>
Hi Ævar,
thanx very much for your detailed response. Exactly what I was
looking for.
Regards
Harri
^ permalink raw reply
* [PATCH v3 2/2] tree:<depth>: skip some trees even when collecting omits
From: Matthew DeVore @ 2019-01-09 2:59 UTC (permalink / raw)
To: git
Cc: Matthew DeVore, sbeller, git, jeffhost, peff, stefanbeller,
jonathantanmy, pclouds, gitster
In-Reply-To: <20190109025914.247473-1-matvore@google.com>
If a tree has already been recorded as omitted, we don't need to
traverse it again just to collect its omits. Stop traversing trees a
second time when collecting omits.
Signed-off-by: Matthew DeVore <matvore@google.com>
---
list-objects-filter.c | 18 ++++++++++++------
t/t6112-rev-list-filters-objects.sh | 11 ++++++++++-
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 786e0dd0b1..ee449de3f7 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -107,18 +107,19 @@ struct seen_map_entry {
size_t depth;
};
-static void filter_trees_update_omits(
+/* Returns 1 if the oid was in the omits set before it was invoked. */
+static int filter_trees_update_omits(
struct object *obj,
struct filter_trees_depth_data *filter_data,
int include_it)
{
if (!filter_data->omits)
- return;
+ return 0;
if (include_it)
- oidset_remove(filter_data->omits, &obj->oid);
+ return oidset_remove(filter_data->omits, &obj->oid);
else
- oidset_insert(filter_data->omits, &obj->oid);
+ return oidset_insert(filter_data->omits, &obj->oid);
}
static enum list_objects_filter_result filter_trees_depth(
@@ -171,12 +172,17 @@ static enum list_objects_filter_result filter_trees_depth(
if (already_seen) {
filter_res = LOFR_SKIP_TREE;
} else {
+ int been_omitted = filter_trees_update_omits(
+ obj, filter_data, include_it);
seen_info->depth = filter_data->current_depth;
- filter_trees_update_omits(obj, filter_data, include_it);
if (include_it)
filter_res = LOFR_DO_SHOW;
- else if (filter_data->omits)
+ else if (filter_data->omits && !been_omitted)
+ /*
+ * Must update omit information of children
+ * recursively; they have not been omitted yet.
+ */
filter_res = LOFR_ZERO;
else
filter_res = LOFR_SKIP_TREE;
diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index 706845f1d9..eb9e4119e2 100755
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -283,7 +283,7 @@ test_expect_success 'verify tree:0 includes trees in "filtered" output' '
# Make sure tree:0 does not iterate through any trees.
-test_expect_success 'filter a GIANT tree through tree:0' '
+test_expect_success 'verify skipping tree iteration when not collecting omits' '
GIT_TRACE=1 git -C r3 rev-list \
--objects --filter=tree:0 HEAD 2>filter_trace &&
grep "Skipping contents of tree [.][.][.]" filter_trace >actual &&
@@ -377,6 +377,15 @@ test_expect_success 'test tree:# filter provisional omit for blob and tree' '
expect_has_with_different_name r4 filt/subdir
'
+test_expect_success 'verify skipping tree iteration when collecting omits' '
+ GIT_TRACE=1 git -C r4 rev-list --filter-print-omitted \
+ --objects --filter=tree:0 HEAD 2>filter_trace &&
+ grep "^Skipping contents of tree " filter_trace >actual &&
+
+ echo "Skipping contents of tree subdir/..." >expect &&
+ test_cmp expect actual
+'
+
# Test tree:<depth> where a tree is iterated to twice - once where a subentry is
# too deep to be included, and again where the blob inside it is shallow enough
# to be included. This makes sure we don't use LOFR_MARK_SEEN incorrectly (we
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* [PATCH v3 1/2] list-objects-filter: teach tree:# how to handle >0
From: Matthew DeVore @ 2019-01-09 2:59 UTC (permalink / raw)
To: git
Cc: Matthew DeVore, sbeller, git, jeffhost, peff, stefanbeller,
jonathantanmy, pclouds, gitster
In-Reply-To: <20190109025914.247473-1-matvore@google.com>
Implement positive values for <depth> in the tree:<depth> filter. The
exact semantics are described in Documentation/rev-list-options.txt.
The long-term goal at the end of this is to allow a partial clone to
eagerly fetch an entire directory of files by fetching a tree and
specifying <depth>=1. This, for instance, would make a build operation
fast and convenient. It is fast because the partial clone does not need
to fetch each file individually, and convenient because the user does
not need to supply a sparse-checkout specification.
Another way of considering this feature is as a way to reduce
round-trips, since the client can get any number of levels of
directories in a single request, rather than wait for each level of tree
objects to come back, whose entries are used to construct a new request.
Signed-off-by: Matthew DeVore <matvore@google.com>
---
Documentation/rev-list-options.txt | 9 ++-
list-objects-filter-options.c | 7 +-
list-objects-filter-options.h | 3 +-
list-objects-filter.c | 116 +++++++++++++++++++++++-----
t/t6112-rev-list-filters-objects.sh | 111 ++++++++++++++++++++++++++
5 files changed, 219 insertions(+), 27 deletions(-)
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index bab5f50b17..f8ab00f7c9 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -734,8 +734,13 @@ specification contained in <path>.
+
The form '--filter=tree:<depth>' omits all blobs and trees whose depth
from the root tree is >= <depth> (minimum depth if an object is located
-at multiple depths in the commits traversed). Currently, only <depth>=0
-is supported, which omits all blobs and trees.
+at multiple depths in the commits traversed). <depth>=0 will not include
+any trees or blobs unless included explicitly in the command-line (or
+standard input when --stdin is used). <depth>=1 will include only the
+tree and blobs which are referenced directly by a commit reachable from
+<commit> or an explicitly-given object. <depth>=2 is like <depth>=1
+while also including trees and blobs one more level removed from an
+explicitly-given commit or tree.
--no-filter::
Turn off any previous `--filter=` argument.
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index e8da2e8581..5285e7674d 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -50,16 +50,15 @@ static int gently_parse_list_objects_filter(
}
} else if (skip_prefix(arg, "tree:", &v0)) {
- unsigned long depth;
- if (!git_parse_ulong(v0, &depth) || depth != 0) {
+ if (!git_parse_ulong(v0, &filter_options->tree_exclude_depth)) {
if (errbuf) {
strbuf_addstr(
errbuf,
- _("only 'tree:0' is supported"));
+ _("expected 'tree:<depth>'"));
}
return 1;
}
- filter_options->choice = LOFC_TREE_NONE;
+ filter_options->choice = LOFC_TREE_DEPTH;
return 0;
} else if (skip_prefix(arg, "sparse:oid=", &v0)) {
diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
index af64e5c66f..477cd97029 100644
--- a/list-objects-filter-options.h
+++ b/list-objects-filter-options.h
@@ -10,7 +10,7 @@ enum list_objects_filter_choice {
LOFC_DISABLED = 0,
LOFC_BLOB_NONE,
LOFC_BLOB_LIMIT,
- LOFC_TREE_NONE,
+ LOFC_TREE_DEPTH,
LOFC_SPARSE_OID,
LOFC_SPARSE_PATH,
LOFC__COUNT /* must be last */
@@ -44,6 +44,7 @@ struct list_objects_filter_options {
struct object_id *sparse_oid_value;
char *sparse_path_value;
unsigned long blob_limit_value;
+ unsigned long tree_exclude_depth;
};
/* Normalized command line arguments */
diff --git a/list-objects-filter.c b/list-objects-filter.c
index a62624a1ce..786e0dd0b1 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -10,6 +10,7 @@
#include "list-objects.h"
#include "list-objects-filter.h"
#include "list-objects-filter-options.h"
+#include "oidmap.h"
#include "oidset.h"
#include "object-store.h"
@@ -84,11 +85,43 @@ static void *filter_blobs_none__init(
* A filter for list-objects to omit ALL trees and blobs from the traversal.
* Can OPTIONALLY collect a list of the omitted OIDs.
*/
-struct filter_trees_none_data {
+struct filter_trees_depth_data {
struct oidset *omits;
+
+ /*
+ * Maps trees to the minimum depth at which they were seen. It is not
+ * necessary to re-traverse a tree at deeper or equal depths than it has
+ * already been traversed.
+ *
+ * We can't use LOFR_MARK_SEEN for tree objects since this will prevent
+ * it from being traversed at shallower depths.
+ */
+ struct oidmap seen_at_depth;
+
+ unsigned long exclude_depth;
+ unsigned long current_depth;
};
-static enum list_objects_filter_result filter_trees_none(
+struct seen_map_entry {
+ struct oidmap_entry base;
+ size_t depth;
+};
+
+static void filter_trees_update_omits(
+ struct object *obj,
+ struct filter_trees_depth_data *filter_data,
+ int include_it)
+{
+ if (!filter_data->omits)
+ return;
+
+ if (include_it)
+ oidset_remove(filter_data->omits, &obj->oid);
+ else
+ oidset_insert(filter_data->omits, &obj->oid);
+}
+
+static enum list_objects_filter_result filter_trees_depth(
struct repository *r,
enum list_objects_filter_situation filter_situation,
struct object *obj,
@@ -96,43 +129,86 @@ static enum list_objects_filter_result filter_trees_none(
const char *filename,
void *filter_data_)
{
- struct filter_trees_none_data *filter_data = filter_data_;
+ struct filter_trees_depth_data *filter_data = filter_data_;
+ struct seen_map_entry *seen_info;
+ int include_it = filter_data->current_depth <
+ filter_data->exclude_depth;
+ int filter_res;
+ int already_seen;
+
+ /*
+ * Note that we do not use _MARK_SEEN in order to allow re-traversal in
+ * case we encounter a tree or blob again at a shallower depth.
+ */
switch (filter_situation) {
default:
BUG("unknown filter_situation: %d", filter_situation);
- case LOFS_BEGIN_TREE:
+ case LOFS_END_TREE:
+ assert(obj->type == OBJ_TREE);
+ filter_data->current_depth--;
+ return LOFR_ZERO;
+
case LOFS_BLOB:
- if (filter_data->omits) {
- oidset_insert(filter_data->omits, &obj->oid);
- /* _MARK_SEEN but not _DO_SHOW (hard omit) */
- return LOFR_MARK_SEEN;
+ filter_trees_update_omits(obj, filter_data, include_it);
+ return include_it ? LOFR_MARK_SEEN | LOFR_DO_SHOW : LOFR_ZERO;
+
+ case LOFS_BEGIN_TREE:
+ seen_info = oidmap_get(
+ &filter_data->seen_at_depth, &obj->oid);
+ if (!seen_info) {
+ seen_info = xcalloc(1, sizeof(*seen_info));
+ oidcpy(&seen_info->base.oid, &obj->oid);
+ seen_info->depth = filter_data->current_depth;
+ oidmap_put(&filter_data->seen_at_depth, seen_info);
+ already_seen = 0;
} else {
- /*
- * Not collecting omits so no need to to traverse tree.
- */
- return LOFR_SKIP_TREE | LOFR_MARK_SEEN;
+ already_seen =
+ filter_data->current_depth >= seen_info->depth;
}
- case LOFS_END_TREE:
- assert(obj->type == OBJ_TREE);
- return LOFR_ZERO;
+ if (already_seen) {
+ filter_res = LOFR_SKIP_TREE;
+ } else {
+ seen_info->depth = filter_data->current_depth;
+ filter_trees_update_omits(obj, filter_data, include_it);
+
+ if (include_it)
+ filter_res = LOFR_DO_SHOW;
+ else if (filter_data->omits)
+ filter_res = LOFR_ZERO;
+ else
+ filter_res = LOFR_SKIP_TREE;
+ }
+ filter_data->current_depth++;
+ return filter_res;
}
}
-static void* filter_trees_none__init(
+static void filter_trees_free(void *filter_data) {
+ struct filter_trees_depth_data *d = filter_data;
+ if (!d)
+ return;
+ oidmap_free(&d->seen_at_depth, 1);
+ free(d);
+}
+
+static void *filter_trees_depth__init(
struct oidset *omitted,
struct list_objects_filter_options *filter_options,
filter_object_fn *filter_fn,
filter_free_fn *filter_free_fn)
{
- struct filter_trees_none_data *d = xcalloc(1, sizeof(*d));
+ struct filter_trees_depth_data *d = xcalloc(1, sizeof(*d));
d->omits = omitted;
+ oidmap_init(&d->seen_at_depth, 0);
+ d->exclude_depth = filter_options->tree_exclude_depth;
+ d->current_depth = 0;
- *filter_fn = filter_trees_none;
- *filter_free_fn = free;
+ *filter_fn = filter_trees_depth;
+ *filter_free_fn = filter_trees_free;
return d;
}
@@ -430,7 +506,7 @@ static filter_init_fn s_filters[] = {
NULL,
filter_blobs_none__init,
filter_blobs_limit__init,
- filter_trees_none__init,
+ filter_trees_depth__init,
filter_sparse_oid__init,
filter_sparse_path__init,
};
diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index eb32505a6e..706845f1d9 100755
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -294,6 +294,117 @@ test_expect_success 'filter a GIANT tree through tree:0' '
! grep "Skipping contents of tree [^.]" filter_trace
'
+# Test tree:# filters.
+
+expect_has () {
+ commit=$1 &&
+ name=$2 &&
+
+ hash=$(git -C r3 rev-parse $commit:$name) &&
+ grep "^$hash $name$" actual
+}
+
+test_expect_success 'verify tree:1 includes root trees' '
+ git -C r3 rev-list --objects --filter=tree:1 HEAD >actual &&
+
+ # We should get two root directories and two commits.
+ expect_has HEAD "" &&
+ expect_has HEAD~1 "" &&
+ test_line_count = 4 actual
+'
+
+test_expect_success 'verify tree:2 includes root trees and immediate children' '
+ git -C r3 rev-list --objects --filter=tree:2 HEAD >actual &&
+
+ expect_has HEAD "" &&
+ expect_has HEAD~1 "" &&
+ expect_has HEAD dir1 &&
+ expect_has HEAD pattern &&
+ expect_has HEAD sparse1 &&
+ expect_has HEAD sparse2 &&
+
+ # There are also 2 commit objects
+ test_line_count = 8 actual
+'
+
+test_expect_success 'verify tree:3 includes everything expected' '
+ git -C r3 rev-list --objects --filter=tree:3 HEAD >actual &&
+
+ expect_has HEAD "" &&
+ expect_has HEAD~1 "" &&
+ expect_has HEAD dir1 &&
+ expect_has HEAD dir1/sparse1 &&
+ expect_has HEAD dir1/sparse2 &&
+ expect_has HEAD pattern &&
+ expect_has HEAD sparse1 &&
+ expect_has HEAD sparse2 &&
+
+ # There are also 2 commit objects
+ test_line_count = 10 actual
+'
+
+# Test provisional omit collection logic with a repo that has objects appearing
+# at multiple depths - first deeper than the filter's threshold, then shallow.
+
+test_expect_success 'setup r4' '
+ git init r4 &&
+
+ echo foo > r4/foo &&
+ mkdir r4/subdir &&
+ echo bar > r4/subdir/bar &&
+
+ mkdir r4/filt &&
+ cp -r r4/foo r4/subdir r4/filt &&
+
+ git -C r4 add foo subdir filt &&
+ git -C r4 commit -m "commit msg"
+'
+
+expect_has_with_different_name () {
+ repo=$1 &&
+ name=$2 &&
+
+ hash=$(git -C $repo rev-parse HEAD:$name) &&
+ ! grep "^$hash $name$" actual &&
+ grep "^$hash " actual &&
+ ! grep "~$hash" actual
+}
+
+test_expect_success 'test tree:# filter provisional omit for blob and tree' '
+ git -C r4 rev-list --objects --filter-print-omitted --filter=tree:2 \
+ HEAD >actual &&
+ expect_has_with_different_name r4 filt/foo &&
+ expect_has_with_different_name r4 filt/subdir
+'
+
+# Test tree:<depth> where a tree is iterated to twice - once where a subentry is
+# too deep to be included, and again where the blob inside it is shallow enough
+# to be included. This makes sure we don't use LOFR_MARK_SEEN incorrectly (we
+# can't use it because a tree can be iterated over again at a lower depth).
+
+test_expect_success 'tree:<depth> where we iterate over tree at two levels' '
+ git init r5 &&
+
+ mkdir -p r5/a/subdir/b &&
+ echo foo > r5/a/subdir/b/foo &&
+
+ mkdir -p r5/subdir/b &&
+ echo foo > r5/subdir/b/foo &&
+
+ git -C r5 add a subdir &&
+ git -C r5 commit -m "commit msg" &&
+
+ git -C r5 rev-list --objects --filter=tree:4 HEAD >actual &&
+ expect_has_with_different_name r5 a/subdir/b/foo
+'
+
+test_expect_success 'tree:<depth> which filters out blob but given as arg' '
+ blob_hash=$(git -C r4 rev-parse HEAD:subdir/bar) &&
+
+ git -C r4 rev-list --objects --filter=tree:1 HEAD $blob_hash >actual &&
+ grep ^$blob_hash actual
+'
+
# Delete some loose objects and use rev-list, but WITHOUT any filtering.
# This models previously omitted objects that we did not receive.
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* [PATCH v3 0/2] support for filtering trees and blobs based on depth
From: Matthew DeVore @ 2019-01-09 2:59 UTC (permalink / raw)
To: git
Cc: Matthew DeVore, sbeller, git, jeffhost, peff, stefanbeller,
jonathantanmy, pclouds, gitster
In-Reply-To: <20181210234030.176178-1-matvore@google.com>
This applies suggestions from Jonathan Tan and Junio. These are mostly
stylistic and readability changes, although there is also an added test case
in t/t6112-rev-list-filters-objects.sh which checks for the scenario when
filtering which would exclude a blob, but the blob is given on the command
line.
This has been rebased onto master, while the prior version was based on next.
Thank you,
Matthew DeVore (2):
list-objects-filter: teach tree:# how to handle >0
tree:<depth>: skip some trees even when collecting omits
Documentation/rev-list-options.txt | 9 +-
list-objects-filter-options.c | 7 +-
list-objects-filter-options.h | 3 +-
list-objects-filter.c | 122 +++++++++++++++++++++++-----
t/t6112-rev-list-filters-objects.sh | 122 +++++++++++++++++++++++++++-
5 files changed, 235 insertions(+), 28 deletions(-)
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply
* Re: [PATCH v2 2/2] tree:<depth>: skip some trees even when collecting omits
From: MATTHEW DEVORE @ 2019-01-09 2:47 UTC (permalink / raw)
To: Jonathan Tan
Cc: matvore, git, sbeller, git, jeffhost, peff, stefanbeller, pclouds
In-Reply-To: <20190108232251.37748-1-jonathantanmy@google.com>
> On January 8, 2019 at 3:22 PM Jonathan Tan <jonathantanmy@google.com> wrote:
>
>
> > > -static void filter_trees_update_omits(
> > > +static int filter_trees_update_omits(
> > > struct object *obj,
> > > struct filter_trees_depth_data *filter_data,
> > > int include_it)
> > > {
> > > if (!filter_data->omits)
> > > - return;
> > > + return 1;
> > >
> > > if (include_it)
> > > - oidset_remove(filter_data->omits, &obj->oid);
> > > + return oidset_remove(filter_data->omits, &obj->oid);
> > > else
> > > - oidset_insert(filter_data->omits, &obj->oid);
> > > + return oidset_insert(filter_data->omits, &obj->oid);
> > > }
> >
> > I think this function is getting too magical - if filter_data->omits is
> > not set, we pretend that we have omitted the tree, because we want the
> > same behavior when not needing omits and when the tree is omitted. Could
> > this be done another way?
>
> Giving some more thought to this, since this is a static function, maybe
> documenting it as "Returns 1 if the objects that this object references need to
> be traversed for "omits" updates, and 0 otherwise" (with the appropriate code
> updates) would suffice.
That's not bad. But I sent a correction which is more like "/* Returns 1 if the oid was in the omits set before it was invoked. */" and returns 0 if omits was NULL. I thought it clearer when the function returns a value in terms of its own arguments and logic, rather than what the caller needs to do. The code I save going with your suggestion (vs. the one I just sent) is offset by the necessity of more detailed comments.
^ permalink raw reply
* Re: [PATCH v2 1/2] list-objects-filter: teach tree:# how to handle >0
From: MATTHEW DEVORE @ 2019-01-09 2:43 UTC (permalink / raw)
To: Junio C Hamano, Jonathan Tan
Cc: matvore, git, sbeller, git, jeffhost, peff, stefanbeller, pclouds
In-Reply-To: <xmqqbm4q4t3j.fsf@gitster-ct.c.googlers.com>
> On January 8, 2019 at 3:39 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Also, the asterisk sticks to the variable, not type, i.e.
>
> struct filter_trees_depth_data *d = filter_data;
>
Fixed. Thanks.
^ permalink raw reply
* Re: [PATCH 3/3] t0006-date.sh: add `human` date format tests.
From: Stephen P. Smith @ 2019-01-09 0:44 UTC (permalink / raw)
To: Johannes Sixt
Cc: git, Linus Torvalds, Junio C Hamano,
Ævar Arnfjörð Bjarmason
In-Reply-To: <a8a586d9-dad7-606f-948c-06725ac3e062@kdbg.org>
On Tuesday, January 8, 2019 2:27:22 PM MST Johannes Sixt wrote:
> Am 31.12.18 um 01:31 schrieb Stephen P. Smith:
> > +
> > +TODAY_REGEX='[A-Z][a-z][a-z] [012][0-9]:[0-6][0-9] .0200'
<snip>
> The $...REGEX expansions must be put in double-quotes to protect them
> from field splitting. But then the tests do not pass anymore (I tested
> only t4202). Please revisit this change.
>
> -- Hannes
I will later figure out why you are seeing the fields splitting but I am not.
In the mean time I will change the quoting.
I started working on test updates based on prior comments this past weekend.
sps
^ permalink raw reply
* Re: [PATCH v2 2/2] tree:<depth>: skip some trees even when collecting omits
From: Matthew DeVore @ 2019-01-09 0:29 UTC (permalink / raw)
To: Jonathan Tan, matvore
Cc: git, sbeller, git, jeffhost, peff, stefanbeller, pclouds
In-Reply-To: <20190108020034.23648-1-jonathantanmy@google.com>
Thank you for the review :) See below.
On 2019/01/07 18:00, Jonathan Tan wrote:
>> -static void filter_trees_update_omits(
>> +static int filter_trees_update_omits(
>> struct object *obj,
>> struct filter_trees_depth_data *filter_data,
>> int include_it)
>> {
>> if (!filter_data->omits)
>> - return;
>> + return 1;
>>
>> if (include_it)
>> - oidset_remove(filter_data->omits, &obj->oid);
>> + return oidset_remove(filter_data->omits, &obj->oid);
>> else
>> - oidset_insert(filter_data->omits, &obj->oid);
>> + return oidset_insert(filter_data->omits, &obj->oid);
>> }
> I think this function is getting too magical - if filter_data->omits is
> not set, we pretend that we have omitted the tree, because we want the
> same behavior when not needing omits and when the tree is omitted. Could
> this be done another way?
Yes, returning a manipulative lie when omits is NULL is rather
confusing. So I changed it to this (interdiff):
+/* Returns 1 if the oid was in the omits set before it was invoked. */
static int filter_trees_update_omits(
struct object *obj,
struct filter_trees_depth_data *filter_data,
int include_it)
{
if (!filter_data->omits)
- return 1;
+ return 0;
if (include_it)
return oidset_remove(filter_data->omits, &obj->oid);
@@ -177,7 +178,7 @@ static enum list_objects_filter_result
filter_trees_depth(
if (include_it)
filter_res = LOFR_DO_SHOW;
- else if (!been_omitted)
+ else if (filter_data->omits && !been_omitted)
/*
* Must update omit information of children
* recursively; they have not been omitted yet.
^ permalink raw reply
* Re: [PATCH v4 0/1] pack-redundant: remove unused functions
From: 16657101987 @ 2019-01-09 0:29 UTC (permalink / raw)
To: gitster; +Cc: 16657101987, git, sunchao9, worldhello.net
In-Reply-To: <xmqqzhsb3q1c.fsf@gitster-ct.c.googlers.com>
> 16657101987@163.com writes:
>
>> From: Sun Chao <sunchao9@huawei.com>
>>
>> I'm particularly grateful to Junio and JiangXin for fixing the patches,
>> and I noticed Junio send a new commit to remove more unused codes and
>> suggest to SQUASH it.
>>
>> So I create this new version of patches to do this work, I also have
>> checked the left codes and remove a unused struct based on Junio's
>> last commit of `https://github.com/gitster/git/commits/sc/pack-redundant`.
>>
>> --
>>
>> Sun Chao (1):
>> pack-redundant: remove unused functions
>
> Is this meant to replace [v3 3/3]?
I'm Sun Chao and because my huawei email account can't send
email outside from company, so I used 163 email account to
send new path at home. I'm sorry for not explaining that.
Yes, this is meant to replace [v3 3/3], and I have noticed the
patch is applied, Thanks very much.
^ permalink raw reply
* Re: [PATCH v2 1/2] list-objects-filter: teach tree:# how to handle >0
From: Jonathan Tan @ 2019-01-08 23:41 UTC (permalink / raw)
To: gitster; +Cc: jonathantanmy, matvore, matvore, git
In-Reply-To: <xmqqftu24t80.fsf@gitster-ct.c.googlers.com>
> Jonathan Tan <jonathantanmy@google.com> writes:
>
> >> For your reference, here is an interdiff for this particular patch after
> >> applying your comments:
> >
> > The interdiff looks good, thanks. All my issues are resolved.
>
> Just to make sure. That's not "v2 is good", but "v2 plus that
> proposed update, when materializes, would be good", right? I'll
> mark the topic as "expecting a reroll" then.
Yes, that's right - there should be a reroll of this topic. Also,
besides the proposed update, I have a comment in patch 2 of this set.
^ permalink raw reply
* Re: [PATCH v2 1/2] list-objects-filter: teach tree:# how to handle >0
From: Junio C Hamano @ 2019-01-08 23:39 UTC (permalink / raw)
To: Jonathan Tan
Cc: matvore, git, sbeller, git, jeffhost, peff, stefanbeller, pclouds
In-Reply-To: <20190108015631.22727-1-jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
>> +static void filter_trees_free(void *filter_data) {
>> + struct filter_trees_depth_data* d = filter_data;
>> + oidmap_free(&d->seen_at_depth, 1);
>> + free(d);
>> +}
>
> Check for NULL-ness of filter_data too, to match the usual behavior of
> free functions.
Also, the asterisk sticks to the variable, not type, i.e.
struct filter_trees_depth_data *d = filter_data;
^ permalink raw reply
* Re: [PATCH v2 1/2] list-objects-filter: teach tree:# how to handle >0
From: Junio C Hamano @ 2019-01-08 23:36 UTC (permalink / raw)
To: Jonathan Tan
Cc: matvore, matvore, git, sbeller, git, jeffhost, peff, stefanbeller,
pclouds
In-Reply-To: <20190108231945.36970-1-jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
>> For your reference, here is an interdiff for this particular patch after
>> applying your comments:
>
> The interdiff looks good, thanks. All my issues are resolved.
Just to make sure. That's not "v2 is good", but "v2 plus that
proposed update, when materializes, would be good", right? I'll
mark the topic as "expecting a reroll" then.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 1/2] Use packet_reader instead of packet_read_line
From: Masaya Suzuki @ 2019-01-08 23:27 UTC (permalink / raw)
To: Josh Steadmon, Masaya Suzuki, Git Mailing List, Jeff King
In-Reply-To: <20190107223324.GB54613@google.com>
On Mon, Jan 7, 2019 at 2:33 PM Josh Steadmon <steadmon@google.com> wrote:
>
> On 2018.12.29 13:19, Masaya Suzuki wrote:
> > By using and sharing a packet_reader while handling a Git pack protocol
> > request, the same reader option is used throughout the code. This makes
> > it easy to set a reader option to the request parsing code.
> >
> > Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
> > ---
> > builtin/archive.c | 19 ++++++-------
> > builtin/receive-pack.c | 60 +++++++++++++++++++++--------------------
> > fetch-pack.c | 61 +++++++++++++++++++++++-------------------
> > remote-curl.c | 22 ++++++++++-----
> > send-pack.c | 37 ++++++++++++-------------
> > upload-pack.c | 38 +++++++++++++-------------
> > 6 files changed, 129 insertions(+), 108 deletions(-)
> >
> > diff --git a/builtin/archive.c b/builtin/archive.c
> > index d2455237c..2fe1f05ca 100644
> > --- a/builtin/archive.c
> > +++ b/builtin/archive.c
> > @@ -27,10 +27,10 @@ static int run_remote_archiver(int argc, const char **argv,
> > const char *remote, const char *exec,
> > const char *name_hint)
> > {
> > - char *buf;
> > int fd[2], i, rv;
> > struct transport *transport;
> > struct remote *_remote;
> > + struct packet_reader reader;
> >
> > _remote = remote_get(remote);
> > if (!_remote->url[0])
> > @@ -53,18 +53,19 @@ static int run_remote_archiver(int argc, const char **argv,
> > packet_write_fmt(fd[1], "argument %s\n", argv[i]);
> > packet_flush(fd[1]);
> >
> > - buf = packet_read_line(fd[0], NULL);
> > - if (!buf)
> > + packet_reader_init(&reader, fd[0], NULL, 0, PACKET_READ_CHOMP_NEWLINE);
> > +
> > + if (packet_reader_read(&reader) != PACKET_READ_NORMAL)
>
> packet_read_line() can also return NULL if the packet is zero-length, so
> you may want to add a "|| reader.pktlen <= 0" to the condition here (and
> in other places where we were checking that packet_read_line() != NULL)
> to make sure the behavior doesn't change. See discussion on my previous
> attempt[1] to refactor this in builtin/archive.c.
>
> [1]: https://public-inbox.org/git/20180912053519.31085-1-steadmon@google.com/
That is interesting. In Documentation/technical/protocol-common.txt,
it says "Implementations SHOULD NOT send an empty pkt-line ("0004").".
The existing code won't distinguish "0000" and "0004", while "0004" is
actually not a valid pkt-line. I'll make this patch with no behavior
change, but I think we can make that behavior change to stop accepting
0004 as 0000, and remove the pktlen checks.
^ permalink raw reply
* Re: Regression: submodule worktrees can clobber core.worktree config
From: Duy Nguyen @ 2019-01-08 23:22 UTC (permalink / raw)
To: Tomasz Śniatowski; +Cc: Git Mailing List
In-Reply-To: <CAG0vfyQeA3Hm7AsYgYtP4v-Yg0=rKXW0YYfg_emAwEscZha4VA@mail.gmail.com>
On Wed, Jan 9, 2019 at 5:56 AM Tomasz Śniatowski <tsniatowski@vewd.com> wrote:
>
> After upgrading to 2.20.1 I noticed in some submodule+worktree scenarios git
> will break the submodule configuration. Reproducible with:
> git init a && (cd a; touch a; git add a; git commit -ma)
> git init b && (cd b; git submodule add ../a; git commit -mb)
> git -C b worktree add ../b2
> git -C b/a worktree add ../../b2/a
> git -C b status
> git -C b2 submodule update
> git -C b status
>
> The submodule update in the _worktree_ puts an invalid core.worktree value in
> the _original_ repository submodule config (b/.git/modules/a/config), causing
> the last git status to error out with:
> fatal: cannot chdir to '../../../../../../b2/a': No such file or directory
> fatal: 'git status --porcelain=2' failed in submodule a
>
> Looking at the config file itself, the submodule update operation applies the
> following change (the new path is invalid):
> - worktree = ../../../a
> + worktree = ../../../../../../b2/a
>
> This worked fine on 2.19.2 (no config change, no error), and was useful to have
> a worktree with (large) submodules that are also worktrees.
This scenario is not supported (or at least known to be broken in
theory) so I wouldn't call this a regression even if it happens to
work on 2.19.2 for some reason.
The good news is, I have something that should make it work reliably.
But I don't know if it will make it to 2.21 or not.
> Bisects down to:
> 74d4731da1 submodule--helper: replace connect-gitdir-workingtree by
> ensure-core-worktree
>
> --
> Tomasz Śniatowski
--
Duy
^ permalink raw reply
* Re: [PATCH v2 2/2] tree:<depth>: skip some trees even when collecting omits
From: Jonathan Tan @ 2019-01-08 23:22 UTC (permalink / raw)
To: jonathantanmy
Cc: matvore, git, sbeller, git, jeffhost, peff, stefanbeller, pclouds
In-Reply-To: <20190108020034.23648-1-jonathantanmy@google.com>
> > -static void filter_trees_update_omits(
> > +static int filter_trees_update_omits(
> > struct object *obj,
> > struct filter_trees_depth_data *filter_data,
> > int include_it)
> > {
> > if (!filter_data->omits)
> > - return;
> > + return 1;
> >
> > if (include_it)
> > - oidset_remove(filter_data->omits, &obj->oid);
> > + return oidset_remove(filter_data->omits, &obj->oid);
> > else
> > - oidset_insert(filter_data->omits, &obj->oid);
> > + return oidset_insert(filter_data->omits, &obj->oid);
> > }
>
> I think this function is getting too magical - if filter_data->omits is
> not set, we pretend that we have omitted the tree, because we want the
> same behavior when not needing omits and when the tree is omitted. Could
> this be done another way?
Giving some more thought to this, since this is a static function, maybe
documenting it as "Returns 1 if the objects that this object references need to
be traversed for "omits" updates, and 0 otherwise" (with the appropriate code
updates) would suffice.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox