* Re: [PATCH 17/18] sha1_file: always allow relative paths to alternates
From: Jacob Keller @ 2016-10-04 6:50 UTC (permalink / raw)
To: Jeff King; +Cc: Git mailing list, René Scharfe
In-Reply-To: <20161003203622.7uz76ay5f7bqqpfm@sigill.intra.peff.net>
On Mon, Oct 3, 2016 at 1:36 PM, Jeff King <peff@peff.net> wrote:
> We recursively expand alternates repositories, so that if A
> borrows from B which borrows from C, A can see all objects.
>
> For the root object database, we allow relative paths, so A
> can point to B as "../B/objects". However, we currently do
> not allow relative paths when recursing, so B must use an
> absolute path to reach C.
>
> That is an ancient protection from c2f493a (Transitively
> read alternatives, 2006-05-07) that tries to avoid adding
> the same alternate through two different paths. Since
> 5bdf0a8 (sha1_file: normalize alt_odb path before comparing
> and storing, 2011-09-07), we use a normalized absolute path
> for each alt_odb entry.
>
> This means that in most cases the protection is no longer
> necessary; we will detect the duplicate no matter how we got
> there (but see below). And it's a good idea to get rid of
> it, as it creates an unnecessary complication when setting
> up recursive alternates (B has to know that A is going to
> borrow from it and make sure to use an absolute path).
>
I think this makes sense. We already normalize a path, and if the
normalization is too complicated, then we (now) fail nicely so we
should always have an absolute path to the store.
> Note that our normalization doesn't actually look at the
> filesystem, so it can still be fooled by crossing symbolic
> links. But that's also true of absolute paths, so it's not a
> good reason to disallow only relative paths (it's
> potentially a reason to switch to real_path(), but that's a
> separate and non-trivial change).
Hmm, ya using real_path would fix that but I definitely agree that's
not trivial and can be done in the future if we think it is or becomes
necessary.
^ permalink raw reply
* Re: [PATCH 18/18] alternates: use fspathcmp to detect duplicates
From: Jacob Keller @ 2016-10-04 6:51 UTC (permalink / raw)
To: Jeff King; +Cc: Git mailing list, René Scharfe
In-Reply-To: <20161003203626.styj2vwcmgwnpx4v@sigill.intra.peff.net>
On Mon, Oct 3, 2016 at 1:36 PM, Jeff King <peff@peff.net> wrote:
> On a case-insensitive filesystem, we should realize that
> "a/objects" and "A/objects" are the same path. We already
> use fspathcmp() to check against the main object directory,
> but until recently we couldn't use it for comparing against
> other alternates (because their paths were not
> NUL-terminated strings). But now we can, so let's do so.
>
Yep, makes sense.
> Note that we also need to adjust count-objects to load the
> config, so that it can see the setting of core.ignorecase
> (this is required by the test, but is also a general bugfix
> for users of count-objects).
Also makes sense.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> builtin/count-objects.c | 2 ++
> sha1_file.c | 2 +-
> t/t5613-info-alternate.sh | 17 +++++++++++++++++
> 3 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/count-objects.c b/builtin/count-objects.c
> index a700409..a04b4f2 100644
> --- a/builtin/count-objects.c
> +++ b/builtin/count-objects.c
> @@ -97,6 +97,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
> OPT_END(),
> };
>
> + git_config(git_default_config, NULL);
> +
> argc = parse_options(argc, argv, prefix, opts, count_objects_usage, 0);
> /* we do not take arguments other than flags for now */
> if (argc)
> diff --git a/sha1_file.c b/sha1_file.c
> index b514167..b05ec9c 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -260,7 +260,7 @@ static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)
> * thing twice, or object directory itself.
> */
> for (alt = alt_odb_list; alt; alt = alt->next) {
> - if (!strcmp(path->buf, alt->path))
> + if (!fspathcmp(path->buf, alt->path))
> return 0;
> }
> if (!fspathcmp(path->buf, normalized_objdir))
> diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
> index 76525a0..926fe14 100755
> --- a/t/t5613-info-alternate.sh
> +++ b/t/t5613-info-alternate.sh
> @@ -116,4 +116,21 @@ test_expect_success 'relative duplicates are eliminated' '
> test_cmp expect actual.alternates
> '
>
> +test_expect_success CASE_INSENSITIVE_FS 'dup finding can be case-insensitive' '
> + git init --bare insensitive.git &&
> + # the previous entry for "A" will have used uppercase
> + cat >insensitive.git/objects/info/alternates <<-\EOF &&
> + ../../C/.git/objects
> + ../../a/.git/objects
> + EOF
> + cat >expect <<-EOF &&
> + alternate: $(pwd)/C/.git/objects
> + alternate: $(pwd)/B/.git/objects
> + alternate: $(pwd)/A/.git/objects
> + EOF
> + git -C insensitive.git count-objects -v >actual &&
> + grep ^alternate: actual >actual.alternates &&
> + test_cmp expect actual.alternates
> +'
> +
> test_done
> --
> 2.10.0.618.g82cc264
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2016, #07; Fri, 23)
From: Johannes Schindelin @ 2016-10-04 8:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqd1jpkkea.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 27 Sep 2016, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > In your previous kitchen status ("What's cooking") you hinted at a
> > possible v2.10.1 soon. I have a couple of bugfixes lined up for Git
> > for Windows and would like to avoid unnecessarily frequent release
> > engineering... Any more concrete ideas on a date for this version?
>
> I scanned RelNotes for 2.11 and identified these topics that we'd want
> to have in 'maint'.
>
> bw/pathspec-remove-unused-extern-decl # 1 (6 days ago)
> rs/checkout-some-states-are-const # 1 (6 days ago)
> rs/strbuf-remove-fix # 1 (6 days ago)
> rs/unpack-trees-reduce-file-scope-global # 1 (6 days ago)
> mr/vcs-svn-printf-ulong # 1 (6 days ago)
> sy/git-gui-i18n-ja # 7 (12 days ago)
> jk/fix-remote-curl-url-wo-proto # 1 (12 days ago)
> js/git-gui-commit-gpgsign # 2 (12 days ago)
> jk/patch-ids-no-merges # 2 (6 days ago)
> ew/http-do-not-forget-to-call-curl-multi-remove-handle # 3 (6 days ago)
> rs/xdiff-merge-overlapping-hunks-for-W-context # 1 (6 days ago)
> ks/perf-build-with-autoconf # 1 (6 days ago)
> jt/format-patch-base-info-above-sig # 1 (6 days ago)
> jk/rebase-i-drop-ident-check # 1 (6 days ago)
> jk/reduce-gc-aggressive-depth # 1 (6 days ago)
> et/add-chmod-x # 1 (6 days ago)
> tg/add-chmod+x-fix # 7 (24 hours ago)
>
> Most are internal clean-ups that I do not mind leaving out, but I
> think we want to have that "add --chmod=+x" fix in. As it hasn't
> been enough time passed since the topic was merged to 'master', I'd
> say either
>
> (1) 2.10.1 with everything other than the last two in a few days
> and 2.10.2 late next week with "add --chmod=+x" fix, or
>
> (2) just a single 2.10.1 with everything late next week.
>
> I can go either way and welcome suggestions. I'd start merging
> older topics in the above list to 'maint' soonish, but not today.
Sorry for the delay in answering. By now, it was probably obvious to you
that (2) was my preference ;-)
Thanks,
Dscho
^ permalink raw reply
* broken post-via-gmane link from https://git-scm.com/community
From: Sandro Santilli @ 2016-10-04 8:11 UTC (permalink / raw)
To: git
The "post via gmane" link on https://git-scm.com/community points
to an unexistent server 'post.gmane.org':
http://post.gmane.org/post.php?group=gmane.comp.version-control.git
--strk;
() Free GIS & Flash consultant/developer
/\ https://strk.kbt.io/services.html
^ permalink raw reply
* color.diff.whitespace unused on removed lines
From: Sandro Santilli @ 2016-10-04 8:14 UTC (permalink / raw)
To: git
The color.diff.whitespace configuration is not used on
removed lines, but only on added lines.
As I'm removing trailing whitespaces all my diffs fail to
give me any information about the real action being taken,
due to this lack of support.
As a workaround, I've found the -R switch for "git show" [1]
but I thought about asking if this is a bug or intended behavior,
and in case it is intended I'd like to know why.
Thanks in advance.
[1]
http://stackoverflow.com/questions/5257553/coloring-white-space-in-git-diffs-output/11509388#11509388
--strk;
() Free GIS & Flash consultant/developer
/\ https://strk.kbt.io/services.html
^ permalink raw reply
* Re: broken post-via-gmane link from https://git-scm.com/community
From: Andrey Rybak @ 2016-10-04 8:52 UTC (permalink / raw)
To: Sandro Santilli; +Cc: git
In-Reply-To: <20161004081157.GB17002@localhost>
Hi,
On 04.10.2016 11:11, Sandro Santilli wrote:
> The "post via gmane" link on https://git-scm.com/community points
> to an unexistent server 'post.gmane.org':
> http://post.gmane.org/post.php?group=gmane.comp.version-control.git
It would probably be better to address this on git-scm.com
github page, where the source for the site is hosted:
https://github.com/git/git-scm.com/issues
^ permalink raw reply
* Re: broken post-via-gmane link from https://git-scm.com/community
From: Sandro Santilli @ 2016-10-04 8:54 UTC (permalink / raw)
To: Andrey Rybak; +Cc: git
In-Reply-To: <36c73304-aceb-7c51-2788-5ba4cdbc862f@gmail.com>
On Tue, Oct 04, 2016 at 11:52:18AM +0300, Andrey Rybak wrote:
> Hi,
>
> On 04.10.2016 11:11, Sandro Santilli wrote:
> > The "post via gmane" link on https://git-scm.com/community points
> > to an unexistent server 'post.gmane.org':
> > http://post.gmane.org/post.php?group=gmane.comp.version-control.git
> It would probably be better to address this on git-scm.com
> github page, where the source for the site is hosted:
> https://github.com/git/git-scm.com/issues
Done:
https://github.com/git/git-scm.com/issues/859
--strk;
() Free GIS & Flash consultant/developer
/\ https://strk.kbt.io/services.html
^ permalink raw reply
* Merge conflicts in .gitattributes can cause trouble
From: Lars Schneider @ 2016-10-04 10:19 UTC (permalink / raw)
To: git; +Cc: Jeff King, Johannes.Schindelin, me
Hi,
If there is a conflict in the .gitattributes during a merge then it looks
like as if the attributes are not applied (which kind of makes sense as Git
would not know what to do). As a result Git can treat e.g. binary files
as text and they can end up with changed line endings in the working tree.
After resolving the conflict in .gitattributes all files would be marked
as binary, again, and the user can easily commit the wrongly changed line
endings.
Consider this script on Windows:
$ git init .
$ touch first.commit
$ git add .
$ git commit -m "first commit"
$ git checkout -b branch
$ printf "*.bin binary\n" >> .gitattributes
$ git add .
$ git commit -m "tracking *.bin files"
$ git checkout master
$ printf "binary\ndata\n" > file.dat # <-- Unix line ending!
$ printf "*.dat binary\n" >> .gitattributes # <-- Tell Git to keep Unix line ending!
$ git add .
$ git commit -m "tracking *.dat files"
$ git cat-file -p :file.dat | od -c
0000000 b i n a r y \n d a t a \n
^^^^ ^^^^ <-- Correct!
$ git checkout branch
$ git merge master # <-- Causes merge conflict!
$ printf "*.bin binary\n*.dat binary\n" > .gitattributes # <-- Fix merge conflict!
$ git add .
$ git commit -m "merged"
$ git cat-file -p :file.dat | od -c
0000000 b i n a r y \r \n d a t a \r \n
^^^^^^^^ ^^^^^^^^ <-- Wrong!
Possible solutions:
1. We could print an appropriate warning if we detect a merge conflict
in .gitattributes
2. We could disable all line ending conversions in case of a merge conflict
(I am not exactly sure about all the implications, though)
3. We could salvage what we could of the .gitattributes file,
perhaps by using the version from HEAD (or more likely, the ours stage of
the index) -- suggested by Peff on the related GitHub issue mentioned below
Thoughts?
Thanks,
Lars
PS: I noticed that behavior while working with Git LFS and started a discussion
about it here: https://github.com/github/git-lfs/issues/1544
^ permalink raw reply
* Re: Repeatable Extraction
From: Johannes Schindelin @ 2016-10-04 10:25 UTC (permalink / raw)
To: chris king; +Cc: git
In-Reply-To: <CAJQwtsidixAAJKp7-b2PmXgs=mS+PbT5ebOmKLJU1nEn7UJ2og@mail.gmail.com>
Hi Chris,
On Tue, 27 Sep 2016, chris king wrote:
> Is there a way automate extraction that will repeatably generate the
> same files? Currently, each time I extract git portable many of the
> binaries change slightly. For example, if I extract twice using
>
> PortableGit-2.10.0-32-bit.7z.exe -y -gm2
>
> then Beyond Compare tells me that many of the files in usr\bin have
> changed at offset 0x88 and 0x89. Why is that?
The reason is that you look at 32-bit, where technical limitations force
us to hard-code a certain base address for all of the includede MSYS2 .dll
files (i.e. all libraries that require, or implement, the POSIX emulation
layer called MSYS2).
To avoid clashes with other .dll files, that base address is adjusted via
the post-install.bat script for your particular environment.
If you want to avoid that, you will have to extract the installer via
7-Zip: it is a self-extracting .7z archive (and the self-extractor
automatically executes post-install.bat, which subsequently deletes
itself).
Ciao,
Johannes
^ permalink raw reply
* GL bug: can not commit, reports error on changed submodule directory
From: ern0 @ 2016-10-04 10:40 UTC (permalink / raw)
To: git
When I say:
$ gl commit -m "blah blah"
It reports:
✘ Failed to read file into stream: Is a directory
Reason: I have a submodule which has changes.
$ git status
On branch develop
Your branch is up-to-date with 'origin/develop'.
Changes not staged for commit:
(use "git add/rm <file>..." to update what will be committed)
(use "git checkout -- <file>..." to discard changes in working directory)
modified: remoting (new commits)
no changes added to commit (use "git add" and/or "git commit -a")
Workaround: I should sync the directory...
$ cd remoting
$ git commit -am "yada"
$ cd ..
$ git commit -am "yada yada"
$ git push
$ echo I feel clean now
$ echo "# wow" >> test.py
$ gl commit -m "added wow"
...and it works again.
--
ern0
dataflow evangelist
^ permalink raw reply
* Re: Slow pushes on 'pu' - even when up-to-date..
From: Heiko Voigt @ 2016-10-04 11:18 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Stefan Beller, Git Mailing List
In-Reply-To: <CA+55aFyos78qODyw57V=w13Ux5-8SvBqObJFAq22K+XKPWVbAA@mail.gmail.com>
Hi,
On Mon, Oct 03, 2016 at 02:11:36PM -0700, Linus Torvalds wrote:
> This seems to be because I'm now on 'pu' as of a day or two ago in
> order to test the abbrev logic, but lookie here:
>
> time git ls-remote ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
> .. shows all the branches and tags ..
> real 0m0.655s
> user 0m0.011s
> sys 0m0.004s
>
> so the remote is fast to connect to, and with network connection
> overhead and everything, it's just over half a second. But then:
>
> time git push ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
The reason behind this is when pushing to an address we do not easily
have the remote refs to compare available. When pushing an existing ref
it would be easy and could get a shortcut but it gets more complicated
for new refs. Currently we fall back to walking the whole history since
that is "the most correct way" we have. But obviously it is not a
practical solution in any way.
I mentioned this fact when discussing the current state and my patches
to make this check less painful. So we still need to think about a
solution for this check when passing an address.
IMO: It's definitely not ready to be switched on as default, unless we
find something a lot cheaper for the above case.
My idea of a solution goes like this:
* collect all SHA1's of the remotes refs
* check if we have them locally
* if not we abort and tell the user to fetch them somehow into local
refs or disable the check
* when we have them locally we proceed passing those SHA1's as bases
instead of --remotes=<name>
Cheers Heiko
^ permalink raw reply
* Re: Merge conflicts in .gitattributes can cause trouble
From: Duy Nguyen @ 2016-10-04 11:26 UTC (permalink / raw)
To: Lars Schneider; +Cc: git, Jeff King, Johannes Schindelin, me
In-Reply-To: <248A6E81-8D5C-4183-9756-51A0D5193E3E@gmail.com>
On Tue, Oct 4, 2016 at 5:19 PM, Lars Schneider <larsxschneider@gmail.com> wrote:
> Hi,
>
>
> If there is a conflict in the .gitattributes during a merge then it looks
> like as if the attributes are not applied (which kind of makes sense as Git
> would not know what to do). As a result Git can treat e.g. binary files
> as text and they can end up with changed line endings in the working tree.
> After resolving the conflict in .gitattributes all files would be marked
> as binary, again, and the user can easily commit the wrongly changed line
> endings.
>
> Consider this script on Windows:
>
> $ git init .
> $ touch first.commit
> $ git add .
> $ git commit -m "first commit"
>
> $ git checkout -b branch
> $ printf "*.bin binary\n" >> .gitattributes
> $ git add .
> $ git commit -m "tracking *.bin files"
>
> $ git checkout master
> $ printf "binary\ndata\n" > file.dat # <-- Unix line ending!
> $ printf "*.dat binary\n" >> .gitattributes # <-- Tell Git to keep Unix line ending!
> $ git add .
> $ git commit -m "tracking *.dat files"
> $ git cat-file -p :file.dat | od -c
> 0000000 b i n a r y \n d a t a \n
> ^^^^ ^^^^ <-- Correct!
> $ git checkout branch
> $ git merge master # <-- Causes merge conflict!
> $ printf "*.bin binary\n*.dat binary\n" > .gitattributes # <-- Fix merge conflict!
> $ git add .
> $ git commit -m "merged"
> $ git cat-file -p :file.dat | od -c
> 0000000 b i n a r y \r \n d a t a \r \n
> ^^^^^^^^ ^^^^^^^^ <-- Wrong!
>
> Possible solutions:
>
> 1. We could print an appropriate warning if we detect a merge conflict
> in .gitattributes
This is good regardless, to encourage people to resolve conflicts in
.gitattributes first. A good place for this warning may be "git
status"?
> 2. We could disable all line ending conversions in case of a merge conflict
> (I am not exactly sure about all the implications, though)
>
> 3. We could salvage what we could of the .gitattributes file,
> perhaps by using the version from HEAD (or more likely, the ours stage of
> the index) -- suggested by Peff on the related GitHub issue mentioned below
We already have code to fall back to index version in some cases,
adding "fall back on merge conflicts" (and probably updating the index
lookup code too because it looks for stage 0 now) sounds reasonable
(especially with the warning in #1).
BTW whoever fixes this probably should do the same for .gitignore files.
--
Duy
^ permalink raw reply
* Re: Reference a submodule branch instead of a commit
From: Heiko Voigt @ 2016-10-04 11:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeremy Morton, git
In-Reply-To: <xmqqfuod6yw2.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 03, 2016 at 12:00:45PM -0700, Junio C Hamano wrote:
> Jeremy Morton <admin@game-point.net> writes:
>
> > At the moment, supermodules must reference a given commit in each of
> > its submodules. If one is in control of a submodule and it changes on
> > a regular basis, this can cause a lot of overhead with "submodule
> > updated" commits in the supermodule. It would be useful of git allows
> > the option of referencing a submodule's branch instead of a given
> > submodule commit. How about adding this functionality?
>
> When somebody downstream fetches from your superproject and grabs
> the set of submodules, how would s/he know what _exact_ state you
> meant to record? When s/he says "I have your superproject commit X,
> which binds submodule's branch Y at path sub/, and it simply does
> not work. Your project is broken", how do you go about reproducing
> the exact state s/he had trouble with to help her/him?
>
> The only thing s/he knows is that the commit used from the submodule
> must be one of the commits that was on branch Y at some point in
> time, hopefully close to the timestamp recorded in the commit in the
> superproject. And your record in the history of the superproject
> does not tell you more than that, so you wouldn't have any idea
> better than what s/he already has to help.
>
> Hence, such a "functionality" will never happen, at least in the
> exact form you are describing.
>
> It is conceivable to add some feature that allows you to squelch the
> report that the submodule recorded in your superproject is not up to
> date from "git status" etc. to help those who thinks it is OK to not
> bind the latest submodule commit to the superproject all the time,
> though.
We already have options to support these kinds of workflows. Look at the
option '--remote' for 'git submodule update'.
You then only have to commit the submodule if you do not want to see it
as dirty locally, but you will always get the tip of a remote tracking
branch when updating.
Cheers Heiko
^ permalink raw reply
* Re: [RFC PATCH] clone: add clone.recursesubmodules config option
From: Heiko Voigt @ 2016-10-04 11:41 UTC (permalink / raw)
To: Stefan Beller
Cc: Jeremy Morton, Chris Packham, git@vger.kernel.org, mara.kim,
Junio C Hamano
In-Reply-To: <CAGZ79kbNVy7VFj31m7VKZYP6xphkV_d9Y1x9Q0_=5PZ+_068HA@mail.gmail.com>
On Mon, Oct 03, 2016 at 10:18:32AM -0700, Stefan Beller wrote:
> On Mon, Oct 3, 2016 at 8:36 AM, Jeremy Morton <admin@game-point.net> wrote:
> > Did this ever get anywhere? Can we recursively update submodules with "git
> > pull" in the supermodule now?
>
> I think the idea is sound.
I am confused there is nothing handling *pull* here? This patch was
about clone. Handling 'pull' is a much bigger topic[1].
Cheers Heiko
[1] https://github.com/jlehmann/git-submod-enhancements/wiki/Recursive-submodule-checkout
^ permalink raw reply
* Re: Slow pushes on 'pu' - even when up-to-date..
From: Jeff King @ 2016-10-04 11:44 UTC (permalink / raw)
To: Heiko Voigt
Cc: Linus Torvalds, Junio C Hamano, Stefan Beller, Git Mailing List
In-Reply-To: <20161004111845.GA20309@book.hvoigt.net>
On Tue, Oct 04, 2016 at 01:18:45PM +0200, Heiko Voigt wrote:
> On Mon, Oct 03, 2016 at 02:11:36PM -0700, Linus Torvalds wrote:
> > This seems to be because I'm now on 'pu' as of a day or two ago in
> > order to test the abbrev logic, but lookie here:
> >
> > time git ls-remote ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
> > .. shows all the branches and tags ..
> > real 0m0.655s
> > user 0m0.011s
> > sys 0m0.004s
> >
> > so the remote is fast to connect to, and with network connection
> > overhead and everything, it's just over half a second. But then:
> >
> > time git push ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
>
> The reason behind this is when pushing to an address we do not easily
> have the remote refs to compare available. When pushing an existing ref
> it would be easy and could get a shortcut but it gets more complicated
> for new refs. Currently we fall back to walking the whole history since
> that is "the most correct way" we have. But obviously it is not a
> practical solution in any way.
>
> I mentioned this fact when discussing the current state and my patches
> to make this check less painful. So we still need to think about a
> solution for this check when passing an address.
>
> IMO: It's definitely not ready to be switched on as default, unless we
> find something a lot cheaper for the above case.
>
> My idea of a solution goes like this:
> * collect all SHA1's of the remotes refs
> * check if we have them locally
> * if not we abort and tell the user to fetch them somehow into local
> refs or disable the check
> * when we have them locally we proceed passing those SHA1's as bases
> instead of --remotes=<name>
As I argued in [1], I think it's not just "this must be cheaper" but
"this must not be enabled if submodules are not in use at all". Most
repositories don't have submodules enabled at all, so anything that
cause any extra traversal, even of a portion of the history, is going to
be a net negative for a lot of people.
I think the only sane default is going to be some kind of heuristic that
says "submodules are probably in use". Something like "is there a
.gitmodules file" is not perfect (you can have gitlink entries without
it), but it's a really cheap constant-time check.
-Peff
[1] Quoted in
http://public-inbox.org/git/xmqqh9aaot49.fsf@gitster.mtv.corp.google.com/
^ permalink raw reply
* Re: Re: Slow pushes on 'pu' - even when up-to-date..
From: Heiko Voigt @ 2016-10-04 12:04 UTC (permalink / raw)
To: Jeff King; +Cc: Linus Torvalds, Junio C Hamano, Stefan Beller, Git Mailing List
In-Reply-To: <20161004114428.4wyq54afd4td3epp@sigill.intra.peff.net>
On Tue, Oct 04, 2016 at 07:44:28AM -0400, Jeff King wrote:
> > My idea of a solution goes like this:
> > * collect all SHA1's of the remotes refs
> > * check if we have them locally
> > * if not we abort and tell the user to fetch them somehow into local
> > refs or disable the check
> > * when we have them locally we proceed passing those SHA1's as bases
> > instead of --remotes=<name>
>
> As I argued in [1], I think it's not just "this must be cheaper" but
> "this must not be enabled if submodules are not in use at all". Most
> repositories don't have submodules enabled at all, so anything that
> cause any extra traversal, even of a portion of the history, is going to
> be a net negative for a lot of people.
>
> I think the only sane default is going to be some kind of heuristic that
> says "submodules are probably in use". Something like "is there a
> .gitmodules file" is not perfect (you can have gitlink entries without
> it), but it's a really cheap constant-time check.
I agree. We are adding convenience for submodules, so we can also say a
checked out ".gitmodules" file is a must to have convenience.
I am not sure if I agree on another layer of options for this as
suggested in your post. More options mean more implementation
complexity and more confusion on the users side.
How about we choose our defaults based on the existence of a checked out
.gitmodules file? So the default would only be --recurse-submodules=check
if there is a .gitmodules file in the worktree. All other users need to
either pass or explicitly configure it.
Cheers Heiko
> [1] Quoted in
> http://public-inbox.org/git/xmqqh9aaot49.fsf@gitster.mtv.corp.google.com/
^ permalink raw reply
* Re: Re: Slow pushes on 'pu' - even when up-to-date..
From: Jeff King @ 2016-10-04 12:07 UTC (permalink / raw)
To: Heiko Voigt
Cc: Linus Torvalds, Junio C Hamano, Stefan Beller, Git Mailing List
In-Reply-To: <20161004120421.GA20701@book.hvoigt.net>
On Tue, Oct 04, 2016 at 02:04:21PM +0200, Heiko Voigt wrote:
> > I think the only sane default is going to be some kind of heuristic that
> > says "submodules are probably in use". Something like "is there a
> > .gitmodules file" is not perfect (you can have gitlink entries without
> > it), but it's a really cheap constant-time check.
>
> I agree. We are adding convenience for submodules, so we can also say a
> checked out ".gitmodules" file is a must to have convenience.
>
> I am not sure if I agree on another layer of options for this as
> suggested in your post. More options mean more implementation
> complexity and more confusion on the users side.
>
> How about we choose our defaults based on the existence of a checked out
> .gitmodules file? So the default would only be --recurse-submodules=check
> if there is a .gitmodules file in the worktree. All other users need to
> either pass or explicitly configure it.
That's OK with me. Though you may end up in the long run wanting some
name for the default behavior (e.g., if people configure something else
and then want to override back to "auto" in some instances), but that
can probably come later.
-Peff
^ permalink raw reply
* Re: [PATCH v8 00/11] Git filter protocol
From: Jeff King @ 2016-10-04 12:11 UTC (permalink / raw)
To: Junio C Hamano
Cc: Lars Schneider, Torsten Bögershausen, git, Stefan Beller,
Jakub Narębski, Martin-Louis Bright, ramsay
In-Reply-To: <xmqqvax974dl.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 03, 2016 at 10:02:14AM -0700, Junio C Hamano wrote:
> The timeout would be good for you to give a message "filter process
> running the script '%s' is not exiting; I am waiting for it". The
> user is still left with a hung Git, and can then see if that process
> is hanging around. If it is, then we found a buggy filter. Or we
> found a buggy Git. Either needs to be fixed. I do not think it
> would help anybody by doing a kill(2) to sweep possible bugs under
> the rug.
I would argue that we should not even bother with such a timeout. This
is an exceptional, buggy condition, and hanging is not at all restricted
to this particular case. If git is hanging, then the right tools are
"ps" or "strace" to figure out what is going on. I know that not all
users are comfortable with those tools, but enough are in practice that
the bugs get ironed out, without git having to carry a bunch of extra
timing code that is essentially never exercised.
-Peff
^ permalink raw reply
* Re: [PATCH 3/3] abbrev: auto size the default abbreviation
From: Jeff King @ 2016-10-04 12:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <xmqqbmz051yp.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 03, 2016 at 06:37:18PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> >> OK, as Linus's "count at the point of use" is already in 'next',
> >> could you make it incremental with a log message?
> >
> > Sure. I wasn't sure if you actually liked my direction or not, so I was
> > mostly just showing off what the completed one would look like.
>
> To be quite honest, I am not just unsure if I liked your direction;
> rather I am not sure if I actually understood what you perceived as
> a difference that matters between the two approaches. I wanted to
> hear you explain the difference in terms of "Linus's does this, but
> it is bad in X and Y way, so let's avoid it and do it like Z
> instead". One effective way to extract that out of you was to force
> you to justify the "incremental" update.
>
> And it seems that I succeeded ;-).
>
> I am still not sure if I 100% agree with your first paragraph, but
> at least now I think I see where you are coming from.
For the record, I am OK with Linus's patch as-is. It's mostly "that's
not how I would have done it, and the flow seems confusing to me". But
that's subjective; I don't think there are any functional flaws in it.
> You probably will hear from Ramsay about extern-ness of msb().
Heh. I seem to have a real problem with that lately.
-Peff
^ permalink raw reply
* [PATCH v9 02/14] convert: modernize tests
From: larsxschneider @ 2016-10-04 12:59 UTC (permalink / raw)
To: git; +Cc: ramsay, jnareb, gitster, j6t, tboegi, peff, mlbright,
Lars Schneider
In-Reply-To: <20161004125947.67104-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
Use `test_config` to set the config, check that files are empty with
`test_must_be_empty`, compare files with `test_cmp`, and remove spaces
after ">" and "<".
Please note that the "rot13" filter configured in "setup" keeps using
`git config` instead of `test_config` because subsequent tests might
depend on it.
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t0021-conversion.sh | 58 +++++++++++++++++++++++++--------------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index e799e59..dc50938 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -38,8 +38,8 @@ script='s/^\$Id: \([0-9a-f]*\) \$/\1/p'
test_expect_success check '
- cmp test.o test &&
- cmp test.o test.t &&
+ test_cmp test.o test &&
+ test_cmp test.o test.t &&
# ident should be stripped in the repository
git diff --raw --exit-code :test :test.i &&
@@ -47,10 +47,10 @@ test_expect_success check '
embedded=$(sed -ne "$script" test.i) &&
test "z$id" = "z$embedded" &&
- git cat-file blob :test.t > test.r &&
+ git cat-file blob :test.t >test.r &&
- ./rot13.sh < test.o > test.t &&
- cmp test.r test.t
+ ./rot13.sh <test.o >test.t &&
+ test_cmp test.r test.t
'
# If an expanded ident ever gets into the repository, we want to make sure that
@@ -130,7 +130,7 @@ test_expect_success 'filter shell-escaped filenames' '
# delete the files and check them out again, using a smudge filter
# that will count the args and echo the command-line back to us
- git config filter.argc.smudge "sh ./argc.sh %f" &&
+ test_config filter.argc.smudge "sh ./argc.sh %f" &&
rm "$normal" "$special" &&
git checkout -- "$normal" "$special" &&
@@ -141,7 +141,7 @@ test_expect_success 'filter shell-escaped filenames' '
test_cmp expect "$special" &&
# do the same thing, but with more args in the filter expression
- git config filter.argc.smudge "sh ./argc.sh %f --my-extra-arg" &&
+ test_config filter.argc.smudge "sh ./argc.sh %f --my-extra-arg" &&
rm "$normal" "$special" &&
git checkout -- "$normal" "$special" &&
@@ -154,9 +154,9 @@ test_expect_success 'filter shell-escaped filenames' '
'
test_expect_success 'required filter should filter data' '
- git config filter.required.smudge ./rot13.sh &&
- git config filter.required.clean ./rot13.sh &&
- git config filter.required.required true &&
+ test_config filter.required.smudge ./rot13.sh &&
+ test_config filter.required.clean ./rot13.sh &&
+ test_config filter.required.required true &&
echo "*.r filter=required" >.gitattributes &&
@@ -165,17 +165,17 @@ test_expect_success 'required filter should filter data' '
rm -f test.r &&
git checkout -- test.r &&
- cmp test.o test.r &&
+ test_cmp test.o test.r &&
./rot13.sh <test.o >expected &&
git cat-file blob :test.r >actual &&
- cmp expected actual
+ test_cmp expected actual
'
test_expect_success 'required filter smudge failure' '
- git config filter.failsmudge.smudge false &&
- git config filter.failsmudge.clean cat &&
- git config filter.failsmudge.required true &&
+ test_config filter.failsmudge.smudge false &&
+ test_config filter.failsmudge.clean cat &&
+ test_config filter.failsmudge.required true &&
echo "*.fs filter=failsmudge" >.gitattributes &&
@@ -186,9 +186,9 @@ test_expect_success 'required filter smudge failure' '
'
test_expect_success 'required filter clean failure' '
- git config filter.failclean.smudge cat &&
- git config filter.failclean.clean false &&
- git config filter.failclean.required true &&
+ test_config filter.failclean.smudge cat &&
+ test_config filter.failclean.clean false &&
+ test_config filter.failclean.required true &&
echo "*.fc filter=failclean" >.gitattributes &&
@@ -197,8 +197,8 @@ test_expect_success 'required filter clean failure' '
'
test_expect_success 'filtering large input to small output should use little memory' '
- git config filter.devnull.clean "cat >/dev/null" &&
- git config filter.devnull.required true &&
+ test_config filter.devnull.clean "cat >/dev/null" &&
+ test_config filter.devnull.required true &&
for i in $(test_seq 1 30); do printf "%1048576d" 1; done >30MB &&
echo "30MB filter=devnull" >.gitattributes &&
GIT_MMAP_LIMIT=1m GIT_ALLOC_LIMIT=1m git add 30MB
@@ -207,7 +207,7 @@ test_expect_success 'filtering large input to small output should use little mem
test_expect_success 'filter that does not read is fine' '
test-genrandom foo $((128 * 1024 + 1)) >big &&
echo "big filter=epipe" >.gitattributes &&
- git config filter.epipe.clean "echo xyzzy" &&
+ test_config filter.epipe.clean "echo xyzzy" &&
git add big &&
git cat-file blob :big >actual &&
echo xyzzy >expect &&
@@ -215,20 +215,20 @@ test_expect_success 'filter that does not read is fine' '
'
test_expect_success EXPENSIVE 'filter large file' '
- git config filter.largefile.smudge cat &&
- git config filter.largefile.clean cat &&
+ test_config filter.largefile.smudge cat &&
+ test_config filter.largefile.clean cat &&
for i in $(test_seq 1 2048); do printf "%1048576d" 1; done >2GB &&
echo "2GB filter=largefile" >.gitattributes &&
git add 2GB 2>err &&
- ! test -s err &&
+ test_must_be_empty err &&
rm -f 2GB &&
git checkout -- 2GB 2>err &&
- ! test -s err
+ test_must_be_empty err
'
test_expect_success "filter: clean empty file" '
- git config filter.in-repo-header.clean "echo cleaned && cat" &&
- git config filter.in-repo-header.smudge "sed 1d" &&
+ test_config filter.in-repo-header.clean "echo cleaned && cat" &&
+ test_config filter.in-repo-header.smudge "sed 1d" &&
echo "empty-in-worktree filter=in-repo-header" >>.gitattributes &&
>empty-in-worktree &&
@@ -240,8 +240,8 @@ test_expect_success "filter: clean empty file" '
'
test_expect_success "filter: smudge empty file" '
- git config filter.empty-in-repo.clean "cat >/dev/null" &&
- git config filter.empty-in-repo.smudge "echo smudged && cat" &&
+ test_config filter.empty-in-repo.clean "cat >/dev/null" &&
+ test_config filter.empty-in-repo.smudge "echo smudged && cat" &&
echo "empty-in-repo filter=empty-in-repo" >>.gitattributes &&
echo dead data walking >empty-in-repo &&
--
2.10.0
^ permalink raw reply related
* [PATCH v9 06/14] pkt-line: extract set_packet_header()
From: larsxschneider @ 2016-10-04 12:59 UTC (permalink / raw)
To: git; +Cc: ramsay, jnareb, gitster, j6t, tboegi, peff, mlbright,
Lars Schneider
In-Reply-To: <20161004125947.67104-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
Extracted set_packet_header() function converts an integer to a 4 byte
hex string. Make this function locally available so that other pkt-line
functions could use it.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
pkt-line.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/pkt-line.c b/pkt-line.c
index 0a9b61c..e8adc0f 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -97,10 +97,20 @@ void packet_buf_flush(struct strbuf *buf)
strbuf_add(buf, "0000", 4);
}
-#define hex(a) (hexchar[(a) & 15])
-static void format_packet(struct strbuf *out, const char *fmt, va_list args)
+static void set_packet_header(char *buf, const int size)
{
static char hexchar[] = "0123456789abcdef";
+
+ #define hex(a) (hexchar[(a) & 15])
+ buf[0] = hex(size >> 12);
+ buf[1] = hex(size >> 8);
+ buf[2] = hex(size >> 4);
+ buf[3] = hex(size);
+ #undef hex
+}
+
+static void format_packet(struct strbuf *out, const char *fmt, va_list args)
+{
size_t orig_len, n;
orig_len = out->len;
@@ -111,10 +121,7 @@ static void format_packet(struct strbuf *out, const char *fmt, va_list args)
if (n > LARGE_PACKET_MAX)
die("protocol error: impossibly long line");
- out->buf[orig_len + 0] = hex(n >> 12);
- out->buf[orig_len + 1] = hex(n >> 8);
- out->buf[orig_len + 2] = hex(n >> 4);
- out->buf[orig_len + 3] = hex(n);
+ set_packet_header(&out->buf[orig_len], n);
packet_trace(out->buf + orig_len + 4, n - 4, 1);
}
--
2.10.0
^ permalink raw reply related
* [PATCH v9 07/14] pkt-line: add packet_write_fmt_gently()
From: larsxschneider @ 2016-10-04 12:59 UTC (permalink / raw)
To: git; +Cc: ramsay, jnareb, gitster, j6t, tboegi, peff, mlbright,
Lars Schneider
In-Reply-To: <20161004125947.67104-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
packet_write_fmt() would die in case of a write error even though for
some callers an error would be acceptable. Add packet_write_fmt_gently()
which writes a formatted pkt-line like packet_write_fmt() but does not
die in case of an error. The function is used in a subsequent patch.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
pkt-line.c | 34 ++++++++++++++++++++++++++++++----
pkt-line.h | 1 +
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/pkt-line.c b/pkt-line.c
index e8adc0f..56915f0 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -125,16 +125,42 @@ static void format_packet(struct strbuf *out, const char *fmt, va_list args)
packet_trace(out->buf + orig_len + 4, n - 4, 1);
}
+static int packet_write_fmt_1(int fd, int gently,
+ const char *fmt, va_list args)
+{
+ struct strbuf buf = STRBUF_INIT;
+ ssize_t count;
+
+ format_packet(&buf, fmt, args);
+ count = write_in_full(fd, buf.buf, buf.len);
+ if (count == buf.len)
+ return 0;
+
+ if (!gently) {
+ check_pipe(errno);
+ die_errno("packet write with format failed");
+ }
+ return error("packet write with format failed");
+}
+
void packet_write_fmt(int fd, const char *fmt, ...)
{
- static struct strbuf buf = STRBUF_INIT;
va_list args;
- strbuf_reset(&buf);
va_start(args, fmt);
- format_packet(&buf, fmt, args);
+ packet_write_fmt_1(fd, 0, fmt, args);
+ va_end(args);
+}
+
+int packet_write_fmt_gently(int fd, const char *fmt, ...)
+{
+ int status;
+ va_list args;
+
+ va_start(args, fmt);
+ status = packet_write_fmt_1(fd, 1, fmt, args);
va_end(args);
- write_or_die(fd, buf.buf, buf.len);
+ return status;
}
void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
diff --git a/pkt-line.h b/pkt-line.h
index 1902fb3..3caea77 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -23,6 +23,7 @@ void packet_flush(int fd);
void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
void packet_buf_flush(struct strbuf *buf);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
/*
* Read a packetized line into the buffer, which must be at least size bytes
--
2.10.0
^ permalink raw reply related
* [PATCH v9 09/14] pkt-line: add packet_write_gently()
From: larsxschneider @ 2016-10-04 12:59 UTC (permalink / raw)
To: git; +Cc: ramsay, jnareb, gitster, j6t, tboegi, peff, mlbright,
Lars Schneider
In-Reply-To: <20161004125947.67104-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
packet_write_fmt_gently() uses format_packet() which lets the caller
only send string data via "%s". That means it cannot be used for
arbitrary data that may contain NULs.
Add packet_write_gently() which writes arbitrary data and does not die
in case of an error. The function is used by other pkt-line functions in
a subsequent patch.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
pkt-line.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/pkt-line.c b/pkt-line.c
index 286eb09..3fd4dc0 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -171,6 +171,22 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)
return status;
}
+static int packet_write_gently(const int fd_out, const char *buf, size_t size)
+{
+ static char packet_write_buffer[LARGE_PACKET_MAX];
+ const size_t packet_size = size + 4;
+
+ if (packet_size > sizeof(packet_write_buffer))
+ return error("packet write failed - data exceeds max packet size");
+
+ packet_trace(buf, size, 1);
+ set_packet_header(packet_write_buffer, packet_size);
+ memcpy(packet_write_buffer + 4, buf, size);
+ if (write_in_full(fd_out, packet_write_buffer, packet_size) == packet_size)
+ return 0;
+ return error("packet write failed");
+}
+
void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
{
va_list args;
--
2.10.0
^ permalink raw reply related
* [PATCH v9 11/14] convert: make apply_filter() adhere to standard Git error handling
From: larsxschneider @ 2016-10-04 12:59 UTC (permalink / raw)
To: git; +Cc: ramsay, jnareb, gitster, j6t, tboegi, peff, mlbright,
Lars Schneider
In-Reply-To: <20161004125947.67104-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
apply_filter() returns a boolean that tells the caller if it
"did convert or did not convert". The variable `ret` was used throughout
the function to track errors whereas `1` denoted success and `0`
failure. This is unusual for the Git source where `0` denotes success.
Rename the variable and flip its value to make the function easier
readable for Git developers.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
convert.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/convert.c b/convert.c
index 986c239..597f561 100644
--- a/convert.c
+++ b/convert.c
@@ -451,7 +451,7 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,
*
* (child --> cmd) --> us
*/
- int ret = 1;
+ int err = 0;
struct strbuf nbuf = STRBUF_INIT;
struct async async;
struct filter_params params;
@@ -477,23 +477,20 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,
return 0; /* error was already reported */
if (strbuf_read(&nbuf, async.out, len) < 0) {
- error("read from external filter '%s' failed", cmd);
- ret = 0;
+ err = error("read from external filter '%s' failed", cmd);
}
if (close(async.out)) {
- error("read from external filter '%s' failed", cmd);
- ret = 0;
+ err = error("read from external filter '%s' failed", cmd);
}
if (finish_async(&async)) {
- error("external filter '%s' failed", cmd);
- ret = 0;
+ err = error("external filter '%s' failed", cmd);
}
- if (ret) {
+ if (!err) {
strbuf_swap(dst, &nbuf);
}
strbuf_release(&nbuf);
- return ret;
+ return !err;
}
static struct convert_driver {
--
2.10.0
^ permalink raw reply related
* [PATCH v9 10/14] pkt-line: add functions to read/write flush terminated packet streams
From: larsxschneider @ 2016-10-04 12:59 UTC (permalink / raw)
To: git; +Cc: ramsay, jnareb, gitster, j6t, tboegi, peff, mlbright,
Lars Schneider
In-Reply-To: <20161004125947.67104-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
write_packetized_from_fd() and write_packetized_from_buf() write a
stream of packets. All content packets use the maximal packet size
except for the last one. After the last content packet a `flush` control
packet is written.
read_packetized_to_strbuf() reads arbitrary sized packets until it
detects a `flush` packet.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
pkt-line.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
pkt-line.h | 8 ++++++++
2 files changed, 77 insertions(+)
diff --git a/pkt-line.c b/pkt-line.c
index 3fd4dc0..8ffde22 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -196,6 +196,47 @@ void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
va_end(args);
}
+int write_packetized_from_fd(int fd_in, int fd_out)
+{
+ static char buf[LARGE_PACKET_DATA_MAX];
+ int err = 0;
+ ssize_t bytes_to_write;
+
+ while (!err) {
+ bytes_to_write = xread(fd_in, buf, sizeof(buf));
+ if (bytes_to_write < 0)
+ return COPY_READ_ERROR;
+ if (bytes_to_write == 0)
+ break;
+ err = packet_write_gently(fd_out, buf, bytes_to_write);
+ }
+ if (!err)
+ err = packet_flush_gently(fd_out);
+ return err;
+}
+
+int write_packetized_from_buf(const char *src_in, size_t len, int fd_out)
+{
+ static char buf[LARGE_PACKET_DATA_MAX];
+ int err = 0;
+ size_t bytes_written = 0;
+ size_t bytes_to_write;
+
+ while (!err) {
+ if ((len - bytes_written) > sizeof(buf))
+ bytes_to_write = sizeof(buf);
+ else
+ bytes_to_write = len - bytes_written;
+ if (bytes_to_write == 0)
+ break;
+ err = packet_write_gently(fd_out, src_in + bytes_written, bytes_to_write);
+ bytes_written += bytes_to_write;
+ }
+ if (!err)
+ err = packet_flush_gently(fd_out);
+ return err;
+}
+
static int get_packet_data(int fd, char **src_buf, size_t *src_size,
void *dst, unsigned size, int options)
{
@@ -305,3 +346,31 @@ char *packet_read_line_buf(char **src, size_t *src_len, int *dst_len)
{
return packet_read_line_generic(-1, src, src_len, dst_len);
}
+
+ssize_t read_packetized_to_strbuf(int fd_in, struct strbuf *sb_out)
+{
+ int packet_len;
+
+ size_t orig_len = sb_out->len;
+ size_t orig_alloc = sb_out->alloc;
+
+ for (;;) {
+ strbuf_grow(sb_out, LARGE_PACKET_DATA_MAX);
+ packet_len = packet_read(fd_in, NULL, NULL,
+ // TODO: explain + 1
+ sb_out->buf + sb_out->len, LARGE_PACKET_DATA_MAX+1,
+ PACKET_READ_GENTLE_ON_EOF);
+ if (packet_len <= 0)
+ break;
+ sb_out->len += packet_len;
+ }
+
+ if (packet_len < 0) {
+ if (orig_alloc == 0)
+ strbuf_release(sb_out);
+ else
+ strbuf_setlen(sb_out, orig_len);
+ return packet_len;
+ }
+ return sb_out->len - orig_len;
+}
diff --git a/pkt-line.h b/pkt-line.h
index 3fa0899..18eac64 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -25,6 +25,8 @@ void packet_buf_flush(struct strbuf *buf);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
int packet_flush_gently(int fd);
int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+int write_packetized_from_fd(int fd_in, int fd_out);
+int write_packetized_from_buf(const char *src_in, size_t len, int fd_out);
/*
* Read a packetized line into the buffer, which must be at least size bytes
@@ -77,8 +79,14 @@ char *packet_read_line(int fd, int *size);
*/
char *packet_read_line_buf(char **src_buf, size_t *src_len, int *size);
+/*
+ * Reads a stream of variable sized packets until a flush packet is detected.
+ */
+ssize_t read_packetized_to_strbuf(int fd_in, struct strbuf *sb_out);
+
#define DEFAULT_PACKET_MAX 1000
#define LARGE_PACKET_MAX 65520
+#define LARGE_PACKET_DATA_MAX (LARGE_PACKET_MAX - 4)
extern char packet_buffer[LARGE_PACKET_MAX];
#endif
--
2.10.0
^ permalink raw reply related
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