Re: [PATCH 3/5] commit: replace the raw buffer with strbuf in read_graft_line

[Jeff King]

On Wed, Aug 16, 2017 at 10:59:02PM +, brian m. carlson wrote:

> On Wed, Aug 16, 2017 at 02:24:27PM +0200, Patryk Obara wrote:
> > On Tue, Aug 15, 2017 at 7:02 PM, Stefan Beller  wrote:
> > >> const int entry_size = GIT_SHA1_HEXSZ + 1;
> > >
> > > outside the scope of this patch:
> > > Is GIT_SHA1_HEXSZ or GIT_MAX_HEXSZ the right call here?
> > 
> > I think neither one. In my opinion, this code should not be so closely
> > coupled to hash parsing code - it should be tasked with parsing
> > whitespace separated list of commit ids without relying on specific
> > commit id length or format.
> 
> What I had intended, although maybe I have not explained this well, was
> that we would have one binary that set up hash functionality as part of
> early setup.  GIT_SHA1_RAWSZ and GIT_SHA1_HEXSZ would turn into
> something like current_hash->rawsz and current_hash->hexsz at that
> point.  The reason I introduced the GIT_MAX constants was to allocate
> memory suitable for whatever hash we picked.
> 
> However, this is only what I had considered for design, and others might
> have different views going forward.  I have, however, based my patches
> on that assumption, and responded to others' comments with those
> statements.

What you wrote here matches my understanding of the general plan. IOW,
we'd expect to "waste" 12 bytes when dealing with a 160-bit sha1 in a
Git binary that's aware of 256-bit hashes. But that seems like a small
price to pay to be able to continue using automatic allocations, versus
rewriting each site to call xmalloc(current_hash->rawsz).

I'd expect most of the GIT_MAX constants to eventually go away in favor
of "struct object_id", but that will still be using the same "big enough
to hold any hash" size under the hood.

> I agree that ideally we should make as much of the code as possible
> ignorant of the hash size, because that will generally result in more
> robust, less brittle code.  I've noticed in this series the use of
> parse_oid_hex, and I agree that's one tool we can use to accomplish that
> goal.

Agreed. Most code should be dealing with the abstract concept of a hash
and shouldn't have to care about the size. I really like parse_oid_hex()
for that reason (and I think parsing is the main place we've found that
needs to care).

-Peff

Re: [PATCH] sub-process: print the cmd when a capability is unsupported

[Christian Couder]

On Wed, Aug 16, 2017 at 5:58 PM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
 I am still wondering if protocol errors should be fatal,
>>>
>>> Yes, please.
>>
>> Unfortunately I think it would prevent new filters or new
>> sub-processes to work with older versions of Git.
>>
>> For example if filters are upgraded company wide to support the new
>> "delay" capability, that would force everyone using the filters to
>> upgrade Git.
>
> I must say that your filter is broken in that case,

Perhaps it is just sloppily written.

> and it is much
> more prudent to die than continuing.  Why is that upgraded filter
> asking for "delay" to an older Git that does not yet know it in the
> first place?

Maybe because in our tests (like in t/t0021/rot13-filter.pl) the
filter just outputs all its capabilities, so the filter writer thought
it should be ok to do the same.

> I just re-read the subprocess_handshake() codepath, and here is my
> understand.  The handshake_capabilities() function first advertises
> the set of capabilities it supports, so that the other side can pick
> and choose which ones to use and ask us to enable in its response.

Yeah, that sounds like the right thing the filter should do. Though I
think that if we really want the filters/subprocesses to always do
this, we have some work on our plate...

> The code under discussion in this thread comes after that, where we
> read the response that tells us what choice the other side made.  If
> we saw something that we never advertised, that indicates one of two
> things.  The other side, i.e. the "upgraded" filter, is not paying
> attention of the capabilities advertisement, and asking something
> its correct operation relies on, but we are not capable of giving
> that unknown feature and operate without it, so after that point the
> exchange of data is a garbage-in-garbage-out.

Maybe it is not paying attention and just following the bad example of
giving all the capabilities it supports even if it can work if some of
them are not supported.

In this case if we error out, we prevent everything to work even if it
could work if we just also "ignored" (though printing a warning is not
exactly ignoring and is the right and the least thing to do) what the
filter told us.

Anyway I don't really mind being very strict and just erroring out in
this case, but I think we should then emphasize more in our test
scripts (maybe by giving a good example) and perhaps also in the doc
that the filters/sub-processes should really pay attention and not
output any capability that are not supported by Git.

[PATCH v4 2/3] builtin/branch: stop supporting the use of --set-upstream option

[Kaartic Sivaraam]

The '--set-upstream' option of branch was deprecated in,

b347d06bf branch: deprecate --set-upstream and show help if we
detect possible mistaken use (Thu, 30 Aug 2012 19:23:13 +0200)

In order to prevent "--set-upstream" on a command line from being taken as
an abbreviated form of "--set-upstream-to", explicitly catch "--set-upstream"
option and die, instead of just removing it from the list of options.

The option is planned to be removed after this change has been around for a few
years.

The before/after behaviour for a simple case follows,

$ git remote
origin

Before,

$ git branch
* master

$ git branch --set-upstream origin/master
The --set-upstream flag is deprecated and will be removed. Consider using 
--track or --set-upstream-to
Branch origin/master set up to track local branch master.

$ echo $?
0

$ git branch
* master
  origin/master

After,

$ git branch
* master

$ git branch --set-upstream origin/master
fatal: the '--set-upstream' option is no longer supported. Please use 
'--track' or '--set-upstream-to' instead.

$ echo $?
128

$ git branch
* master

Helped-by: Martin Ågren ,  Junio C Hamano 

Signed-off-by: Kaartic Sivaraam 
---
 Changes in v4:

- made a few changes suggested by Martin
- hid the '--set-upstream' option from 'git branch -h' as suggested by 
Junio 
- updated commit message as suggested by Junio
- updated error message (this should have been in v3 but somehow got 
skipped)

 Note: I'm not using the word 'removed' in the error messages or in the 
documentation as
 I feel it counter-intuitive from the end user's perspective because 

* 'git branch' still accepts the option in the command line

*  The option has not yet been removed from the synopsis of the 
documentation and I think
   we can't remove it from the 'Synopsis' porion of the documentation as it 
doesn't make
   sense (at least to me) to give a description of an option not listed in 
the synopsis.
   Moreover, we have to state the reason for not supporting it in some 
place.

 I guess the phrase 'no longer supported' is equally communicative. Let me know 
if that was not
 a right decision.

 Documentation/git-branch.txt |  8 
 builtin/branch.c | 25 +++
 t/t3200-branch.sh| 47 ++--
 t/t6040-tracking-info.sh | 20 +++
 4 files changed, 16 insertions(+), 84 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 81bd0a7b7..948d9c9ef 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -195,10 +195,10 @@ start-point is either a local or remote-tracking branch.
branch.autoSetupMerge configuration variable is true.
 
 --set-upstream::
-   If specified branch does not exist yet or if `--force` has been
-   given, acts exactly like `--track`. Otherwise sets up configuration
-   like `--track` would when creating the branch, except that where
-   branch points to is not changed.
+   As this option had confusing syntax it's no longer supported. Please use
+   --track or --set-upstream-to instead.
++
+Note: This could possibly become an alias of --set-upstream-to in the future.
 
 -u ::
 --set-upstream-to=::
diff --git a/builtin/branch.c b/builtin/branch.c
index a3bd2262b..6e3ea5787 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -557,8 +557,8 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
OPT__QUIET(, N_("suppress informational messages")),
OPT_SET_INT('t', "track",  , N_("set up tracking mode 
(see git-pull(1))"),
BRANCH_TRACK_EXPLICIT),
-   OPT_SET_INT( 0, "set-upstream",  , N_("change upstream 
info"),
-   BRANCH_TRACK_OVERRIDE),
+   { OPTION_SET_INT, 0, "set-upstream", , NULL, N_("do not 
use"),
+   PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 
BRANCH_TRACK_OVERRIDE },
OPT_STRING('u', "set-upstream-to", _upstream, 
N_("upstream"), N_("change the upstream info")),
OPT_BOOL(0, "unset-upstream", _upstream, N_("Unset the 
upstream info")),
OPT__COLOR(_use_color, N_("use colored output")),
@@ -755,8 +755,6 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
strbuf_release();
} else if (argc > 0 && argc <= 2) {
struct branch *branch = branch_get(argv[0]);
-   int branch_existed = 0, remote_tracking = 0;
-   struct strbuf buf = STRBUF_INIT;
 
if (!strcmp(argv[0], "HEAD"))
die(_("it does not make sense to create 'HEAD' 
manually"));
@@ -768,28 +766,11 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
  

[PATCH v4 3/3] branch: quote branch/ref names to improve readability

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam 
---
 No changes in this one. Sending this just because of the change in the total 
number
 of commits.

 branch.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/branch.c b/branch.c
index ad5a2299b..a40721f3c 100644
--- a/branch.c
+++ b/branch.c
@@ -90,24 +90,24 @@ int install_branch_config(int flag, const char *local, 
const char *origin, const
if (shortname) {
if (origin)
printf_ln(rebasing ?
- _("Branch %s set up to track remote 
branch %s from %s by rebasing.") :
- _("Branch %s set up to track remote 
branch %s from %s."),
+ _("Branch '%s' set up to track remote 
branch '%s' from '%s' by rebasing.") :
+ _("Branch '%s' set up to track remote 
branch '%s' from '%s'."),
  local, shortname, origin);
else
printf_ln(rebasing ?
- _("Branch %s set up to track local 
branch %s by rebasing.") :
- _("Branch %s set up to track local 
branch %s."),
+ _("Branch '%s' set up to track local 
branch '%s' by rebasing.") :
+ _("Branch '%s' set up to track local 
branch '%s'."),
  local, shortname);
} else {
if (origin)
printf_ln(rebasing ?
- _("Branch %s set up to track remote 
ref %s by rebasing.") :
- _("Branch %s set up to track remote 
ref %s."),
+ _("Branch '%s' set up to track remote 
ref '%s' by rebasing.") :
+ _("Branch '%s' set up to track remote 
ref '%s'."),
  local, remote);
else
printf_ln(rebasing ?
- _("Branch %s set up to track local 
ref %s by rebasing.") :
- _("Branch %s set up to track local 
ref %s."),
+ _("Branch '%s' set up to track local 
ref '%s' by rebasing.") :
+ _("Branch '%s' set up to track local 
ref '%s'."),
  local, remote);
}
}
-- 
2.14.1.534.g641031ecb

[PATCH v4 1/3] test: cleanup cruft of a test

[Kaartic Sivaraam]

Avoiding the clean up step of tests may help in some cases but in other
cases they cause the other unrelated tests to fail for unobvious reasons.
It's better to cleanup a few things to keep other tests from failing
as a result of it.

So, cleanup a cruft left behind by an old test in order for the changes that
are to be introduced to be independent of it.

Signed-off-by: Kaartic Sivaraam 
---
 This was part of [PATCH v3 1/2] and has been separated as it seemed to be
 a "logically separate" change.

 t/t3200-branch.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index dd37ac47c..b54b3ebf3 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -560,6 +560,7 @@ test_expect_success 'use --set-upstream-to modify HEAD' '
 test_expect_success 'use --set-upstream-to modify a particular branch' '
git branch my13 &&
git branch --set-upstream-to master my13 &&
+   test_when_finished "git branch --unset-upstream my13" &&
test "$(git config branch.my13.remote)" = "." &&
test "$(git config branch.my13.merge)" = "refs/heads/master"
 '
-- 
2.14.1.534.g641031ecb

[PATCH v2/RFC] hook: use correct logical variable

[Kaartic Sivaraam]

In general, a 'Sign-off' added should be that of the *committer* and not
that of the *commit's author*. As the 3rd part of the 'prepare-commit-msg'
hook appended the sign-off of the *commit's author* it worked weirdly in
some cases. For example 'git commit --amend -s' when coupled with that part
of the script woked weirdly as illustrated by the following scenario in which
the last commit's log message has the following trailer,

...

Signed-off-by: Random Developer 

and the commit's author is "Random Developer ". Assume
that the commit is trying to be amended by another developer who's identity is
"Another Developer ". When he tries to do

$ git commit --amend -s

with the 3rd part of the hook enabled then the trailer he would see in his 
editor
would be,

...

Signed-off-by: Random Developer 
Signed-off-by: Another Developer 
Signed-off-by: Random Developer 

This is because,

  * the hook is invoked only after the sign-off is appended by the '-s' option

  * the script tries to add the sign-off of the *commit's author* using 
interpret-trailers
and 'interpret-trailers' in it's default configuration tries to adds the 
trailer
when the *neighbouring* trailer isn't the same as the one trying to be 
added.

This is just an example and this kind of issue could repeat if similar 
conditions are
satisified for other cases.

Moreover the rest of Git adds the sign-off of the *committer* using 
sequencer.c::append_signoff().
So, use the correct logical variable that identifies the committer to append 
the sign-off
in the sample hook script.

Bottom line: Being consistent prevents all sorts of weird issues.

Signed-off-by: Kaartic Sivaraam 
---
 Changes in v2:

- updated the commit message
 
 Suggestions regarding ways to improve the message are most welcome.

 templates/hooks--prepare-commit-msg.sample | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/templates/hooks--prepare-commit-msg.sample 
b/templates/hooks--prepare-commit-msg.sample
index a84c3e5a8..12dd8fd88 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -34,7 +34,7 @@ SHA1=$3
 #  *) ;;
 # esac
 
-# SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
+# SOB=$(git var GIT_COMMITTER_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: 
\1/p')
 # git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
 # if test -z "$COMMIT_SOURCE"
 # then
-- 
2.14.0.rc1.434.g6eded367a

Re: [PATCH] hook: use correct logical variable

[Kaartic Sivaraam]

On Tue, 2017-08-15 at 10:28 -0700, Junio C Hamano wrote:
> I did shoot for conciseness, but what is a lot more important is to
> record what is at the core of the issue.  "I found it by doing A"
> can hint to careful readers why doing A leads to an undesirable
> behaviour, but when there are other ways to trigger problems that
> come from the same cause, "I found it by doing A" is less useful
> unless we also record "Doing A reveals the underlying problem X"
> that can be shared by other ways B, C, ... to trigger it.  The
> careful readers need to guess what the X is.
> 
> And once you identify the underlying problem X and record _that_ in
> the log message, I and A in "I found it by doing A" becomes much
> less interesting and the readers do not have to guess.
> 
> Your "A" is 'git commit --amend -s' with the disabled part of hook
> enabled.  But I think 'git commit' without "--amend" and "-s" would
> also show an issue that come from the same root cause.  The hook
> will add SoB that is based on the author, not the committer.  That
> resulting commit would be different from 'git commit -s' without the
> hook enabled, which would add SoB based on the commiter name (that
> would be a "B", that causes a related but different problem that
> comes from the same underlying issue "X" which is "we should
> consistently use the committer info like other parts of the
> system").
> 
> In any case, thanks for a fix-up.  Let's move this forward quickly,
> as it is an update to a topic that is already in 'master'.
> 
> Thanks.

Seeing the reply, I changed my opinion that "it isn't worth the effort
to write a better commit message for the change". I've given a try but
it got a little off-hand. Let me know if there's ways in which it could
be simplified and/or improved.

-- 
Kaartic

Re: [PATCH v3 1/2 / RFC] builtin/branch: stop supporting the use of --set-upstream option

[Kaartic Sivaraam]

On Wed, 2017-08-16 at 12:09 -0700, Junio C Hamano wrote:
> You said that "checkout" does not do a necessary check that is done
> in "branch", so presumably "branch" already has a code to do so that
> is not called by the current "checkout", right?  Then you would add
> a new caller in "checkout" to trigger the same check that is already
> done in "branch", but the code "branch" uses _might_ be too specific
> to the kind of data the current implementation of "branch" uses and
> it _may_ not be easy to call it directly from "checkout" (I didn't
> check if that is the case).  If so, then the check implemented in
> the current "branch" may need to be refactored before it can easily
> be called from the new caller you would be adding to "checkout".
> 
> 
Thanks. Now I get it. What about doing that check in
branch.c::create_branch or branch.c::validate_new_branchname? I guess
creating a branch named HEAD isn't that good an idea in any case. Doing
the check there might prevent a similar situation in future, I guess.
Further "branch" and "checkout" do call branch.c::create_branch which
in turn calls branch.c::validate_new_branchname.

-- 
Kaartic

Re: git clean -fdx deletes tracked files

[Andrew Ardill]

Hi Kim,

I have cc'd the git for windows mailing list, but doing a quick search
on the bug tracker shows this issue which looks related:
https://github.com/git-for-windows/git/issues/607

Hope that helps, seems like it may be an issue with how junctions on
windows are handled by git.

Regards,

Andrew Ardill


On 16 August 2017 at 16:47, Kim Birkelund  wrote:
> Apologies. I should obviously have mentioned which OSes the machines I
> tested on ran.
>
> One Windows 10 (fully updated) and one Windows Server 2016 (also
> updated). I've also seen it in a real repository on our build server
> which is Windows Server 2012 R2.
>
> After my first mail I updated git to latest and could still reproduce.
>
>
>
> On Aug 15, 2017 21:25, "Kevin Daudt"  wrote:
>
> On Tue, Aug 15, 2017 at 08:45:20PM +0200, Kim Birkelund wrote:
>> Hi
>>
>> I hope this is gonna sound as weird to you as it does to me.
>>
>> The link below is a zip of a small git repository that I can reproduce
>> the bug in on 2 machines.
>>
>> Repo: https://www.dropbox.com/s/fz4d0i5ko7s7ktr/test.zip?dl=0
>>
>> It contains 2 folders: helpers and b, each of which is an empty npm
>> module. b\package.json refers to the helpers module.
>>
>> The following reproduces the bug:
>>
>> 1) in terminal cd to the b folder
>> 2) run npm install
>> 3) run git reset HEAD --hard
>> 4) run git clean -fdx
>>
>> At this point both files in the helpers folder has been deleted and
>> running git status confirms this.
>>
>> Tool version:
>>
>> git --version => git version 2.10.2.windows.1
>> node -v => v6.11.2
>> npm -v => 5.3.0
>>
>>
>> I have no idea what is going. Very much hope you can explain :-)
>
> I cannot reproduce it on linux.
>
> git clean -fdx output:
>
>   Removing node_modules/
>   Removing package-lock.json
>
> These are all untracked, and nothing in the helpers dir is being
> removed.

loan

[FINANCE CAPITAL IN]

UNSECURED BUSINESS/PERSONAL LOAN BY LOAN CAPITAL FINANCE
  - NO COLLATERAL
  - MINIMUM DOCUMENTATION
  - BUSINESS LOAN UP TO FIVE(5) MILLION US DOLLARS

  CONTACT US TODAY VIA EMAIL: financecapital...@mail.com

Re: [PATCH] submodule.sh: remove unused variable

[Jonathan Nieder]

Stefan Beller wrote:

> This could have been part of 48308681b0 (git submodule update: have a
> dedicated helper for cloning, 2016-02-29).
>
> Signed-off-by: Stefan Beller 
> ---
>  git-submodule.sh | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Jonathan Nieder 

Thanks for cleaning up.

> diff --git a/git-submodule.sh b/git-submodule.sh
> index e131760eec..9dcec7b356 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -611,7 +611,6 @@ cmd_update()
>   die_if_unmatched "$mode" "$sha1"
>  
>   name=$(git submodule--helper name "$sm_path") || exit
> - url=$(git config submodule."$name".url)
>   if ! test -z "$update"
>   then
>   update_module=$update
> -- 
> 2.14.0.rc0.3.g6c2e499285
> 

Re: [PATCH 3/5] commit: replace the raw buffer with strbuf in read_graft_line

[brian m. carlson]

On Wed, Aug 16, 2017 at 02:24:27PM +0200, Patryk Obara wrote:
> On Tue, Aug 15, 2017 at 7:02 PM, Stefan Beller  wrote:
> >> const int entry_size = GIT_SHA1_HEXSZ + 1;
> >
> > outside the scope of this patch:
> > Is GIT_SHA1_HEXSZ or GIT_MAX_HEXSZ the right call here?
> 
> I think neither one. In my opinion, this code should not be so closely
> coupled to hash parsing code - it should be tasked with parsing
> whitespace separated list of commit ids without relying on specific
> commit id length or format.

What I had intended, although maybe I have not explained this well, was
that we would have one binary that set up hash functionality as part of
early setup.  GIT_SHA1_RAWSZ and GIT_SHA1_HEXSZ would turn into
something like current_hash->rawsz and current_hash->hexsz at that
point.  The reason I introduced the GIT_MAX constants was to allocate
memory suitable for whatever hash we picked.

However, this is only what I had considered for design, and others might
have different views going forward.  I have, however, based my patches
on that assumption, and responded to others' comments with those
statements.

I agree that ideally we should make as much of the code as possible
ignorant of the hash size, because that will generally result in more
robust, less brittle code.  I've noticed in this series the use of
parse_oid_hex, and I agree that's one tool we can use to accomplish that
goal.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [RFC PATCH 01/10] pack: move pack name-related functions

[Jonathan Tan]

On Fri, 11 Aug 2017 14:34:27 -0700
Junio C Hamano  wrote:

> Ben Peart  writes:
> 
> > On 8/9/2017 1:16 PM, Jonathan Tan wrote:
> >
> >> Ah, I forgot to mention this in the cover letter. I thought that one
> >> header was sufficient to cover all pack-related things, so if we wanted
> >> to know which files used pack-related things, we would only need to
> >> search for one string instead of two. Also, the division between
> >> "pack.h" and the hypothetical "packfile.h" was not so clear to me.
> >
> > I prefer having source and the header files that export the functions
> > have matching names to make it easy to find them.  I would prefer
> > packfile.h vs pack.h myself.
> 
> Meaning "If we have packfile.c, packfile.h is preferrable over pack.h"?
> I tend to agree with that.

Fair enough - I've changed it so that the functions now go into
packfile.h. I'll send it out once I know what to base it on (at least
jt/sha1-file-cleanup, and a few more branches that also modify
sha1_file.c).

[PATCH] submodule.sh: remove unused variable

[Stefan Beller]

This could have been part of 48308681b0 (git submodule update: have a
dedicated helper for cloning, 2016-02-29).

Signed-off-by: Stefan Beller 
---
 git-submodule.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index e131760eec..9dcec7b356 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -611,7 +611,6 @@ cmd_update()
die_if_unmatched "$mode" "$sha1"
 
name=$(git submodule--helper name "$sm_path") || exit
-   url=$(git config submodule."$name".url)
if ! test -z "$update"
then
update_module=$update
-- 
2.14.0.rc0.3.g6c2e499285

Re: [PATCH] fix revisions doc about quoting for ':/' notation

[Junio C Hamano]

Andreas Heiduk  writes:

> Am 16.08.2017 um 05:21 schrieb ryenus:
>> To make sure the `` in `:/` is seen as one search string,
>> one should quote/escape `` properly.
>> 
>> Especially, the example given in the manual `:/fix nasty bug` does not
>> work because of missing quotes. The examples are now corrected, and a
>> note about quoting/escaping is added as well.
>
> Right now the documentation describes the syntax as git sees the
> parameters. This is agnostic of the shell or other UI with their
> different quoting rules.  For example users of fish must quote
> `rev@{2}`. A GUI might require no quoting at all. In that case `:/"fix
> nasty bugs"` would be given to git verbatim and hence not find the revision.

These are all good points that I didn't consider when responding.

> Also: Other examples like `HEAD@{5 minutes ago}` need the same quoting.
>
> So my suggestion is to not use quoting in the examples and provide only
> a hint in the text. Example:
>
>  {caret}{/}', e.g. 'HEAD^{/fix nasty bug}'::
> A suffix '{caret}' to a revision parameter, followed by a brace
> pair that contains a text led by a slash,
> is the same as the ':/fix nasty bug' syntax below except that
> it returns the youngest matching commit which is reachable from
> the '' before '{caret}'.
> +   Depending on the given text the shell's word splitting rules
> +   might require additional quoting.

That sounds like a very safe change to adopt, regardless of what we
decide to do to the other part of the proposed change.

Thanks.

Re: [PATCH v2] sub-process: print the cmd when a capability is unsupported

[Junio C Hamano]

Junio C Hamano  writes:

> Ben Peart  writes:
>
>>> -   warning("external filter requested unsupported filter 
>>> capability '%s'",
>>> -   p);
>>> +   warning("subprocess '%s' requested unsupported 
>>> capability '%s'",
>>> +   process->argv[0], p);
>>> }
>>> }
>>>   
>>>
>>
>> This one is even cleaner.  Thanks Lars for pointing out the fact we
>> already had the cmd name.  Looks good.
>
> Thanks, all.  Will queue.

I still think we would want to turn warning() to die(), but it
probably is better to do so in a separate follow-up patch.  That
will give us a good place to record the reason why the current "just
call a warning() and pretend as if nothing bad happend" is wrong.

Re: [PATCH v2 0/2] Allow building with the external sha1dc library

[Junio C Hamano]

Takashi Iwai  writes:

> this is the second attempt to allow linking with the external sha1dc
> shlib.  Now I split to two patches: one for cleaning up of sha1dc
> plumbing codes, and another for adding the option to link with the
> external sha1dc lib.
>
> Other changes from v1:
> - Plumbing codes for external lib are also merged commonly in
>   sha1dc_git.[ch]
> - Check the conflict of extlib vs submodule
> - Drop DC_SHA1_LINK, hoping that everyone is well-mannered
> - Minor rephrasing / corrections of texts
>
>
> thanks,

Thank you for an update.  

I think this round addresses the concerns Ævar had with the previous
round.  Let's wait to hear from him just to be sure.

Re: [Patch size_t V3 13/19] Convert index-pack to size_t

[Ramsay Jones]

On 16/08/17 21:16, Martin Koegler wrote:
> From: Martin Koegler 
> 
> Signed-off-by: Martin Koegler 
> ---
>  builtin/index-pack.c | 26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 7f3ccd0..bf2d728 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -435,7 +435,7 @@ static int is_delta_type(enum object_type type)
>   return (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA);
>  }
>  
> -static void *unpack_entry_data(off_t offset, unsigned long size,
> +static void *unpack_entry_data(off_t offset, size_t size,
>  enum object_type type, unsigned char *sha1)
>  {
>   static char fixed_buf[8192];
> @@ -444,10 +444,10 @@ static void *unpack_entry_data(off_t offset, unsigned 
> long size,
>   void *buf;
>   git_SHA_CTX c;
>   char hdr[32];
> - int hdrlen;
> + size_t hdrlen;
>  
>   if (!is_delta_type(type)) {
> - hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), 
> size) + 1;
> + hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %" PRIuMAX, 
> typename(type), (uintmax_t)size) + 1;
>   git_SHA1_Init();
>   git_SHA1_Update(, hdr, hdrlen);
>   } else
> @@ -489,7 +489,7 @@ static void *unpack_raw_entry(struct object_entry *obj,
> unsigned char *sha1)
>  {
>   unsigned char *p;
> - unsigned long size, c;
> + size_t size, c;
>   off_t base_offset;
>   unsigned shift;
>   void *data;
> @@ -551,11 +551,11 @@ static void *unpack_raw_entry(struct object_entry *obj,
>  }
>  
>  static void *unpack_data(struct object_entry *obj,
> -  int (*consume)(const unsigned char *, unsigned long, 
> void *),
> +  int (*consume)(const unsigned char *, size_t, void *),
>void *cb_data)
>  {
>   off_t from = obj[0].idx.offset + obj[0].hdr_size;
> - off_t len = obj[1].idx.offset - from;
> + size_t len = obj[1].idx.offset - from;

off_t -> size_t.

ATB,
Ramsay Jones

Re: [RFC PATCH] Updated "imported object" design

[Jonathan Tan]

On Wed, 16 Aug 2017 13:32:23 -0700
Junio C Hamano  wrote:

> Jonathan Tan  writes:
> 
> > Also, let me know if there's a better way to send out these patches for
> > review. Some of the code here has been reviewed before, for example.
> >
> > [1] 
> > https://public-inbox.org/git/cover.1502241234.git.jonathanta...@google.com/
> >
> > [2] 
> > https://public-inbox.org/git/ffb734d277132802bcc25baa13e8ede3490af62a.1501532294.git.jonathanta...@google.com/
> >
> > [3] 
> > https://public-inbox.org/git/20170807161031.7c4ea...@twelve2.svl.corp.google.com/
> 
> ... and some of the code exists only in the list archive, so we
> don't know which other topic if any we may want to eject tentatively
> if we wanted to give precedence to move this topic forward over
> others.  I'll worry about it later but help from others is also
> appreciated.

Thanks - I can help take a look when it is time to move the code in.

I think the issue here is whether we want to move this topic forward or
not, that is, if this (special ".imported" objects) is the best way to
solve (at least partially) the connectivity check part of tolerating
missing objects. I hope that we can continue to talk about it.

> This collects names of the objects that are _directly_ referred to
> by imported objects.  An imported pack may have a commit, whose
> top-level tree may or may not appear in the same pack, or the tree
> may exist locally but not in the same pack.  Or the tree may not be
> locally available at all.  In any of these four cases, the top-level
> tree is listed in the "promises" set.  Same for trees and tags.
> 
> I wonder if all of the calls to oidset_insert() in this function
> want to be guarded by "mark it as promised only when the referrent
> is *not* locally available" to keep the promises set minimally
> populated.  The only change needed to fsck in order to make it
> refrain from treating a missing but promised object as an error
> would be:
> 
> -   if (object is missing)
> +   if (object is missing && object is not promised)
> error("that object must be there but missing");
> 
> so there is no point in throwing something that we know we locally
> have in this oidset, right?
> 
> On the other hand, cost of such additional checks in this function
> may outweigh the savings of both memory pressure and look-up cost,
> so I do not know how the tradeoff would turn out.

I also don't know how the tradeoff would turn out, so I leaned towards
the slightly simpler solution of not doing the check. In the future,
maybe a t/perf test can be done to decide between the two.

> > +static int is_promise(const struct object_id *oid)
> > +{
> > +   if (!promises_prepared) {
> > +   if (repository_format_lazy_object)
> > +   for_each_packed_object(add_promise, NULL,
> > +  FOR_EACH_OBJECT_IMPORTED_ONLY);
> > +   promises_prepared = 1;
> > +   }
> > +   return oidset_contains(, oid);
> > +}
> 
> Somehow I'm tempted to call this function "is_promised()" but that
> is a minor naming issue.

I was trying to be consistent in using the name "promise" instead of
"promised object/tag/commit/tree/blob" everywhere, but we can switch if
need be (for example, if we don't want to limit the generic name
"promise" to merely objects).

> >  static const char *describe_object(struct object *obj)
> >  {
> > static struct strbuf buf = STRBUF_INIT;
> > @@ -410,7 +472,7 @@ static void fsck_handle_reflog_oid(const char *refname, 
> > struct object_id *oid,
> > xstrfmt("%s@{%"PRItime"}", refname, 
> > timestamp));
> > obj->used = 1;
> > mark_object_reachable(obj);
> > -   } else {
> > +   } else if (!is_promise(oid)) {
> > error("%s: invalid reflog entry %s", refname, 
> > oid_to_hex(oid));
> > errors_found |= ERROR_REACHABLE;
> > }
> 
> This is about certainly is one place we want to check if the missing
> object is OK, but I'm a bit surprised if this were the only place.
> 
> Don't we need "while trying to follow all the outgoing links from
> this tree object, and we found this object is not available locally;
> normally we would mark it as an error but it turns out that the
> missing one is in the promised set of objects, so it is OK" for the
> normal connectivity traversal codepaths, for example?

That's right. The places to make this change are the same as those in
some earlier patches I sent (patches 2-4 in [1]).

[1] https://public-inbox.org/git/cover.1501532294.git.jonathanta...@google.com/

Re: [Patch size_t V3 00/19] use size_t

[Junio C Hamano]

Martin Koegler  writes:

> From: Martin Koegler 
>
> This patchset is for next [24db08a6e8fed761d3bace7f2d5997806e20b9f7].
> Its a complete collection of all patches. Some errors were fixed and
> it sticks with off_t for length in (pack) files. Object sizes are handled
> as size_t.

Thanks for an update.

I am in the middle of today's integration cycle already, so I won't
be able to apply them to my tree, see how it interacts with various
other topics, and if I can rebase it off of 'next' as of yesterday
(I am hoping that I can update 'next' with a few new topics today)
to give it a more appropriate base, at least until late this evening.  

Making it "a complete collection" is very much appreciated, as I can
forget about random pieces that were picked up so far.  I haven't
looked at the patch text, but I agree that the use of off_t for
something that could become a location in a file and use of size_t
for objects that we handle in-core would be a good place to start
(and probably a good place to stop, at least for now).

> Martin Koegler (19):
>   delta: fix enconding size larger than an "uint" can hold
>   Convert size datatype to size_t
>   Convert zlib.c to size_t
>   delta: Fix offset overflows
>   Convert sha1_file.c to size_t
>   Use size_t for sha1
>   Convert parse_X_buffer to size_t
>   Convert fsck.c & commit.c to size_t
>   Convert cache functions to size_t
>   Add overflow check to get_delta_hdr_size
>   Use size_t for config parsing
>   Convert pack-objects to size_t
>   Convert index-pack to size_t
>   Convert unpack-objects to size_t
>   Convert archive functions to size_t
>   Convert various things to size_t
>   Convert ref-filter to size_t
>   Convert tree-walk to size_t
>   Convert xdiff-interface to size_t

Usually we try to make these more like

delta: fix enconding size larger than an "uint" can hold
pack-objects: use size_t for sizes, not ulong
tree-walk: use size_t for sizes, not ulong

or (perhaps even better for this series)

size_t: convert pack-objects away from ulong
size_t: convert tree-walk away from ulong
...

so that readers of "git shortlog --no-merges" can visually spot
a group of patches that are around the same theme.

>  95 files changed, 525 insertions(+), 489 deletions(-)

That's a lot of changes.  Let's see how well it goes.

Thanks.

Re: [PATCH] fix revisions doc about quoting for ':/' notation

[Andreas Heiduk]

Am 16.08.2017 um 05:21 schrieb ryenus:
> To make sure the `` in `:/` is seen as one search string,
> one should quote/escape `` properly.
> 
> Especially, the example given in the manual `:/fix nasty bug` does not
> work because of missing quotes. The examples are now corrected, and a
> note about quoting/escaping is added as well.

Right now the documentation describes the syntax as git sees the
parameters. This is agnostic of the shell or other UI with their
different quoting rules.  For example users of fish must quote
`rev@{2}`. A GUI might require no quoting at all. In that case `:/"fix
nasty bugs"` would be given to git verbatim and hence not find the revision.

Also: Other examples like `HEAD@{5 minutes ago}` need the same quoting.

So my suggestion is to not use quoting in the examples and provide only
a hint in the text. Example:

 {caret}{/}', e.g. 'HEAD^{/fix nasty bug}'::
A suffix '{caret}' to a revision parameter, followed by a brace
pair that contains a text led by a slash,
is the same as the ':/fix nasty bug' syntax below except that
it returns the youngest matching commit which is reachable from
the '' before '{caret}'.
+   Depending on the given text the shell's word splitting rules
+   might require additional quoting.

Re: reftable [v7]: new ref storage format

[Junio C Hamano]

Shawn Pearce  writes:

> 7th iteration of the reftable storage format.
>
> You can read a rendered version of this here:
> https://googlers.googlesource.com/sop/jgit/+/reftable/Documentation/technical/reftable.md
>
> Changes from v6:
> - Blocks are variable sized, and alignment is optional.
> - ref index is required on variable sized multi-block files.
>
> - restart_count/offsets are again at the end of the block.
> - value_type = 0x3 is only for symbolic references.
> - "other" files cannot be stored in reftable.
>
> - object blocks are explicitly optional.
> - object blocks use position (offset in bytes), not block id.
> - removed complex log_chained format for log blocks
>
> - Layout uses log, ref file extensions
> - Described reader algorithm to obtain a snapshot

I read this version through pretending that I never read the
previous iterations, and it still made sense to me, which is a good
indication that the document is well self contained.  

I think I agree with all the simplication from v6.

I found it a slightly odd that we do not insist that update_indices
that appear in a single reftable file are consecutive, yet we
require that min_update_index of a reftable file must be one greater
than the max_update_index of a previous one.  That is not a new
issue in v7, though.

Thanks.

Re: [RFC PATCH] Updated "imported object" design

[Junio C Hamano]

Jonathan Tan  writes:

> Also, let me know if there's a better way to send out these patches for
> review. Some of the code here has been reviewed before, for example.
>
> [1] 
> https://public-inbox.org/git/cover.1502241234.git.jonathanta...@google.com/
>
> [2] 
> https://public-inbox.org/git/ffb734d277132802bcc25baa13e8ede3490af62a.1501532294.git.jonathanta...@google.com/
>
> [3] 
> https://public-inbox.org/git/20170807161031.7c4ea...@twelve2.svl.corp.google.com/

... and some of the code exists only in the list archive, so we
don't know which other topic if any we may want to eject tentatively
if we wanted to give precedence to move this topic forward over
others.  I'll worry about it later but help from others is also
appreciated.

As to the contents of this patch, overall, everything makes sense,
except for one thing that makes me wonder.  It's not that I see
something specifically incorrect--it is just I do not yet quiet
fathom the implications of.

> +/*
> + * Objects that are believed to be loadable by the lazy loader, because
> + * they are referred to by an imported object. If an object that we have
> + * refers to such an object even though we don't have that object, it is
> + * not an error.
> + */
> +static struct oidset promises;
> +static int promises_prepared;
> +
> +static int add_promise(const struct object_id *oid, struct packed_git *pack,
> +uint32_t pos, void *data)
> +{
> + struct object *obj = parse_object(oid);
> + if (!obj)
> + /*
> +  * Error messages are given when packs are verified, so
> +  * do not print any here.
> +  */
> + return 0;
> + 
> + /*
> +  * If this is a tree, commit, or tag, the objects it refers
> +  * to are promises. (Blobs refer to no objects.)
> +  */
> + if (obj->type == OBJ_TREE) {
> + struct tree *tree = (struct tree *) obj;
> + struct tree_desc desc;
> + struct name_entry entry;
> + if (init_tree_desc_gently(, tree->buffer, tree->size))
> + /*
> +  * Error messages are given when packs are
> +  * verified, so do not print any here.
> +  */
> + return 0;
> + while (tree_entry_gently(, ))
> + oidset_insert(, entry.oid);
> + } else if (obj->type == OBJ_COMMIT) {
> + struct commit *commit = (struct commit *) obj;
> + struct commit_list *parents = commit->parents;
> +
> + oidset_insert(, >tree->object.oid);
> + for (; parents; parents = parents->next)
> + oidset_insert(, >item->object.oid);
> + } else if (obj->type == OBJ_TAG) {
> + struct tag *tag = (struct tag *) obj;
> + oidset_insert(, >tagged->oid);
> + }
> + return 0;
> +}

This collects names of the objects that are _directly_ referred to
by imported objects.  An imported pack may have a commit, whose
top-level tree may or may not appear in the same pack, or the tree
may exist locally but not in the same pack.  Or the tree may not be
locally available at all.  In any of these four cases, the top-level
tree is listed in the "promises" set.  Same for trees and tags.

I wonder if all of the calls to oidset_insert() in this function
want to be guarded by "mark it as promised only when the referrent
is *not* locally available" to keep the promises set minimally
populated.  The only change needed to fsck in order to make it
refrain from treating a missing but promised object as an error
would be:

-   if (object is missing)
+   if (object is missing && object is not promised)
error("that object must be there but missing");

so there is no point in throwing something that we know we locally
have in this oidset, right?

On the other hand, cost of such additional checks in this function
may outweigh the savings of both memory pressure and look-up cost,
so I do not know how the tradeoff would turn out.

> +static int is_promise(const struct object_id *oid)
> +{
> + if (!promises_prepared) {
> + if (repository_format_lazy_object)
> + for_each_packed_object(add_promise, NULL,
> +FOR_EACH_OBJECT_IMPORTED_ONLY);
> + promises_prepared = 1;
> + }
> + return oidset_contains(, oid);
> +}

Somehow I'm tempted to call this function "is_promised()" but that
is a minor naming issue.

>  static const char *describe_object(struct object *obj)
>  {
>   static struct strbuf buf = STRBUF_INIT;
> @@ -410,7 +472,7 @@ static void fsck_handle_reflog_oid(const char *refname, 
> struct object_id *oid,
>   xstrfmt("%s@{%"PRItime"}", refname, 
> timestamp));
>   obj->used = 1;
>   

git add -p breaks after split on change at the top of the file

[Simon Ruderich]

Hello,

The following session reproduces the issue with Git 2.14.1:

$ git init test
$ cd test
$ echo x >file
$ git add file
$ git commit -m commit
$ printf 'a\nb\nx\nc\n' >file

$ git add -p
diff --git a/file b/file
index 587be6b..74a69a0 100644
--- a/file
+++ b/file
@@ -1 +1,4 @@
+a
+b
 x
+c
Stage this hunk [y,n,q,a,d,/,s,e,?]? <-- press s
Split into 2 hunks.
@@ -1 +1,3 @@
+a
+b
 x
Stage this hunk [y,n,q,a,d,/,j,J,g,e,?]? <-- press e

Now delete the line "+a" in your editor and save.

error: patch failed: file:1
error: file: patch does not apply

I expected git add -p to stage this change without error. It
works fine without splitting the hunk (by deleting the first and
last + line in the diff).

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

Re: [PATCH 1/9] Convert pack-objects to size_t

[Martin Koegler]

On Mon, Aug 14, 2017 at 10:08:05AM -0700, Junio C Hamano wrote:
>It may help reducing the maintenance if we introduced obj_size_t
>that is defined to be size_t for now, so that we can later swap
>it to ofs_t or some larger type when we know we do need to
>support objects whose size cannot be expressed in size_t, but I
>do not offhand know what the pros-and-cons with such an approach
>would look like.

Where should the use of obj_size_t end and the use of size_t start? 

We often determine a object size and then pass it to malloc. 
We would start with a larger datatyp and then truncate for memory allocation, 
which use size_t.

Regards,
Martin

[Patch size_t V3 06/19] Use size_t for sha1

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 block-sha1/sha1.c | 2 +-
 block-sha1/sha1.h | 2 +-
 ppc/sha1.c| 2 +-
 ppc/sha1.h| 2 +-
 sha1dc_git.c  | 2 +-
 sha1dc_git.h  | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index 22b125c..8681031 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -203,7 +203,7 @@ void blk_SHA1_Init(blk_SHA_CTX *ctx)
ctx->H[4] = 0xc3d2e1f0;
 }
 
-void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, unsigned long len)
+void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, size_t len)
 {
unsigned int lenW = ctx->size & 63;
 
diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
index 4df6747..9fb0441 100644
--- a/block-sha1/sha1.h
+++ b/block-sha1/sha1.h
@@ -13,7 +13,7 @@ typedef struct {
 } blk_SHA_CTX;
 
 void blk_SHA1_Init(blk_SHA_CTX *ctx);
-void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *dataIn, unsigned long len);
+void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *dataIn, size_t len);
 void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx);
 
 #define platform_SHA_CTX   blk_SHA_CTX
diff --git a/ppc/sha1.c b/ppc/sha1.c
index ec6a192..f0dfcfb 100644
--- a/ppc/sha1.c
+++ b/ppc/sha1.c
@@ -25,7 +25,7 @@ int ppc_SHA1_Init(ppc_SHA_CTX *c)
return 0;
 }
 
-int ppc_SHA1_Update(ppc_SHA_CTX *c, const void *ptr, unsigned long n)
+int ppc_SHA1_Update(ppc_SHA_CTX *c, const void *ptr, size_t n)
 {
unsigned long nb;
const unsigned char *p = ptr;
diff --git a/ppc/sha1.h b/ppc/sha1.h
index 9b24b32..52cac23 100644
--- a/ppc/sha1.h
+++ b/ppc/sha1.h
@@ -16,7 +16,7 @@ typedef struct {
 } ppc_SHA_CTX;
 
 int ppc_SHA1_Init(ppc_SHA_CTX *c);
-int ppc_SHA1_Update(ppc_SHA_CTX *c, const void *p, unsigned long n);
+int ppc_SHA1_Update(ppc_SHA_CTX *c, const void *p, size_t n);
 int ppc_SHA1_Final(unsigned char *hash, ppc_SHA_CTX *c);
 
 #define platform_SHA_CTX   ppc_SHA_CTX
diff --git a/sha1dc_git.c b/sha1dc_git.c
index 4d32b4f..a9076bc 100644
--- a/sha1dc_git.c
+++ b/sha1dc_git.c
@@ -11,7 +11,7 @@ void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)
sha1_to_hex(hash));
 }
 
-void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *vdata, unsigned long len)
+void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *vdata, size_t len)
 {
const char *data = vdata;
/* We expect an unsigned long, but sha1dc only takes an int */
diff --git a/sha1dc_git.h b/sha1dc_git.h
index a8a5c1d..f6051aa 100644
--- a/sha1dc_git.h
+++ b/sha1dc_git.h
@@ -11,7 +11,7 @@ void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);
 /*
  * Same as SHA1DCUpdate, but adjust types to match git's usual interface.
  */
-void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *data, unsigned long len);
+void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *data, size_t len);
 
 #define platform_SHA_CTX SHA1_CTX
 #define platform_SHA1_Init SHA1DCInit
-- 
2.1.4

[Patch size_t V3 05/19] Convert sha1_file.c to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 cache.h | 16 +++
 sha1_file.c | 68 ++---
 streaming.c |  2 +-
 3 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/cache.h b/cache.h
index 5c85844..681dcb6 100644
--- a/cache.h
+++ b/cache.h
@@ -1197,15 +1197,15 @@ static inline const unsigned char 
*lookup_replace_object(const unsigned char *sh
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
 extern int sha1_object_info(const unsigned char *, size_t *);
-extern int hash_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *sha1);
-extern int write_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *return_sha1);
-extern int hash_sha1_file_literally(const void *buf, unsigned long len, const 
char *type, unsigned char *sha1, unsigned flags);
-extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned 
char *);
+extern int hash_sha1_file(const void *buf, size_t len, const char *type, 
unsigned char *sha1);
+extern int write_sha1_file(const void *buf, size_t len, const char *type, 
unsigned char *return_sha1);
+extern int hash_sha1_file_literally(const void *buf, size_t len, const char 
*type, unsigned char *sha1, unsigned flags);
+extern int pretend_sha1_file(void *, size_t, enum object_type, unsigned char 
*);
 extern int force_object_loose(const unsigned char *sha1, time_t mtime);
 extern int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
-extern void *map_sha1_file(const unsigned char *sha1, unsigned long *size);
-extern int unpack_sha1_header(git_zstream *stream, unsigned char *map, 
unsigned long mapsize, void *buffer, unsigned long bufsiz);
+extern void *map_sha1_file(const unsigned char *sha1, size_t *size);
+extern int unpack_sha1_header(git_zstream *stream, unsigned char *map, size_t 
mapsize, void *buffer, size_t bufsiz);
 extern int parse_sha1_header(const char *hdr, size_t *sizep);
 
 /* global flag to enable extra checks when accessing packed objects */
@@ -1731,8 +1731,8 @@ extern off_t find_pack_entry_one(const unsigned char 
*sha1, struct packed_git *)
 
 extern int is_pack_valid(struct packed_git *);
 extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, 
size_t *);
-extern unsigned long unpack_object_header_buffer(const unsigned char *buf, 
unsigned long len, enum object_type *type, size_t *sizep);
-extern unsigned long get_size_from_delta(struct packed_git *, struct 
pack_window **, off_t);
+extern size_t unpack_object_header_buffer(const unsigned char *buf, size_t 
len, enum object_type *type, size_t *sizep);
+extern size_t get_size_from_delta(struct packed_git *, struct pack_window **, 
off_t);
 extern int unpack_object_header(struct packed_git *, struct pack_window **, 
off_t *, size_t *);
 
 /*
diff --git a/sha1_file.c b/sha1_file.c
index 3428172..1b3efea 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -51,7 +51,7 @@ static struct cached_object {
unsigned char sha1[20];
enum object_type type;
void *buf;
-   unsigned long size;
+   size_t size;
 } *cached_objects;
 static int cached_object_nr, cached_object_alloc;
 
@@ -818,8 +818,8 @@ static int check_packed_git_idx(const char *path, struct 
packed_git *p)
 * variable sized table containing 8-byte entries
 * for offsets larger than 2^31.
 */
-   unsigned long min_size = 8 + 4*256 + nr*(20 + 4 + 4) + 20 + 20;
-   unsigned long max_size = min_size;
+   size_t min_size = 8 + 4*256 + nr*(20 + 4 + 4) + 20 + 20;
+   size_t max_size = min_size;
if (nr)
max_size += (nr - 1)*8;
if (idx_size < min_size || idx_size > max_size) {
@@ -1763,7 +1763,7 @@ static int open_sha1_file(const unsigned char *sha1, 
const char **path)
  */
 static void *map_sha1_file_1(const char *path,
 const unsigned char *sha1,
-unsigned long *size)
+size_t *size)
 {
void *map;
int fd;
@@ -1790,13 +1790,13 @@ static void *map_sha1_file_1(const char *path,
return map;
 }
 
-void *map_sha1_file(const unsigned char *sha1, unsigned long *size)
+void *map_sha1_file(const unsigned char *sha1, size_t *size)
 {
return map_sha1_file_1(NULL, sha1, size);
 }
 
-unsigned long unpack_object_header_buffer(const unsigned char *buf,
-   unsigned long len, enum object_type *type, size_t *sizep)
+size_t unpack_object_header_buffer(const unsigned char *buf,
+  size_t len, enum object_type *type, size_t 
*sizep)
 {
unsigned shift;
size_t size, c;
@@ -1821,8 +1821,8 @@ unsigned long unpack_object_header_buffer(const 

[Patch size_t V3 09/19] Convert cache functions to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 cache-tree.c  |  6 +++---
 cache-tree.h  |  2 +-
 cache.h   |  6 +++---
 convert.c | 18 +-
 environment.c |  4 ++--
 read-cache.c  | 18 +-
 6 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 2440d1d..77b3253 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -485,10 +485,10 @@ void cache_tree_write(struct strbuf *sb, struct 
cache_tree *root)
write_one(sb, root, "", 0);
 }
 
-static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
+static struct cache_tree *read_one(const char **buffer, size_t *size_p)
 {
const char *buf = *buffer;
-   unsigned long size = *size_p;
+   size_t size = *size_p;
const char *cp;
char *ep;
struct cache_tree *it;
@@ -568,7 +568,7 @@ static struct cache_tree *read_one(const char **buffer, 
unsigned long *size_p)
return NULL;
 }
 
-struct cache_tree *cache_tree_read(const char *buffer, unsigned long size)
+struct cache_tree *cache_tree_read(const char *buffer, size_t size)
 {
if (buffer[0])
return NULL; /* not the whole tree */
diff --git a/cache-tree.h b/cache-tree.h
index f7b9cab..df59e6e 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -28,7 +28,7 @@ void cache_tree_invalidate_path(struct index_state *, const 
char *);
 struct cache_tree_sub *cache_tree_sub(struct cache_tree *, const char *);
 
 void cache_tree_write(struct strbuf *, struct cache_tree *root);
-struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
+struct cache_tree *cache_tree_read(const char *buffer, size_t size);
 
 int cache_tree_fully_valid(struct cache_tree *);
 int cache_tree_update(struct index_state *, int);
diff --git a/cache.h b/cache.h
index 681dcb6..1ec53e0 100644
--- a/cache.h
+++ b/cache.h
@@ -667,7 +667,7 @@ extern int chmod_index_entry(struct index_state *, struct 
cache_entry *ce, char
 extern int ce_same_name(const struct cache_entry *a, const struct cache_entry 
*b);
 extern void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);
 extern int index_name_is_other(const struct index_state *, const char *, int);
-extern void *read_blob_data_from_index(const struct index_state *, const char 
*, unsigned long *);
+extern void *read_blob_data_from_index(const struct index_state *, const char 
*, size_t *);
 
 /* do stat comparison even if CE_VALID is true */
 #define CE_MATCH_IGNORE_VALID  01
@@ -743,8 +743,8 @@ extern int pack_compression_level;
 extern size_t packed_git_window_size;
 extern size_t packed_git_limit;
 extern size_t delta_base_cache_limit;
-extern unsigned long big_file_threshold;
-extern unsigned long pack_size_limit_cfg;
+extern size_t big_file_threshold;
+extern size_t pack_size_limit_cfg;
 
 /*
  * Accessors for the core.sharedrepository config which lazy-load the value
diff --git a/convert.c b/convert.c
index 1012462..445599b 100644
--- a/convert.c
+++ b/convert.c
@@ -41,9 +41,9 @@ struct text_stat {
unsigned printable, nonprintable;
 };
 
-static void gather_stats(const char *buf, unsigned long size, struct text_stat 
*stats)
+static void gather_stats(const char *buf, size_t size, struct text_stat *stats)
 {
-   unsigned long i;
+   size_t i;
 
memset(stats, 0, sizeof(*stats));
 
@@ -90,7 +90,7 @@ static void gather_stats(const char *buf, unsigned long size, 
struct text_stat *
  * The same heuristics as diff.c::mmfile_is_binary()
  * We treat files with bare CR as binary
  */
-static int convert_is_binary(unsigned long size, const struct text_stat *stats)
+static int convert_is_binary(size_t size, const struct text_stat *stats)
 {
if (stats->lonecr)
return 1;
@@ -101,7 +101,7 @@ static int convert_is_binary(unsigned long size, const 
struct text_stat *stats)
return 0;
 }
 
-static unsigned int gather_convert_stats(const char *data, unsigned long size)
+static unsigned int gather_convert_stats(const char *data, size_t size)
 {
struct text_stat stats;
int ret = 0;
@@ -118,7 +118,7 @@ static unsigned int gather_convert_stats(const char *data, 
unsigned long size)
return ret;
 }
 
-static const char *gather_convert_stats_ascii(const char *data, unsigned long 
size)
+static const char *gather_convert_stats_ascii(const char *data, size_t size)
 {
unsigned int convert_stats = gather_convert_stats(data, size);
 
@@ -140,7 +140,7 @@ const char *get_cached_convert_stats_ascii(const struct 
index_state *istate,
   const char *path)
 {
const char *ret;
-   unsigned long sz;
+   size_t sz;
void *data = read_blob_data_from_index(istate, path, );
ret = gather_convert_stats_ascii(data, sz);
free(data);
@@ -222,7 +222,7 @@ static void check_safe_crlf(const char *path, enum 
crlf_action 

[Patch size_t V3 13/19] Convert index-pack to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 builtin/index-pack.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 7f3ccd0..bf2d728 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -435,7 +435,7 @@ static int is_delta_type(enum object_type type)
return (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA);
 }
 
-static void *unpack_entry_data(off_t offset, unsigned long size,
+static void *unpack_entry_data(off_t offset, size_t size,
   enum object_type type, unsigned char *sha1)
 {
static char fixed_buf[8192];
@@ -444,10 +444,10 @@ static void *unpack_entry_data(off_t offset, unsigned 
long size,
void *buf;
git_SHA_CTX c;
char hdr[32];
-   int hdrlen;
+   size_t hdrlen;
 
if (!is_delta_type(type)) {
-   hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), 
size) + 1;
+   hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %" PRIuMAX, 
typename(type), (uintmax_t)size) + 1;
git_SHA1_Init();
git_SHA1_Update(, hdr, hdrlen);
} else
@@ -489,7 +489,7 @@ static void *unpack_raw_entry(struct object_entry *obj,
  unsigned char *sha1)
 {
unsigned char *p;
-   unsigned long size, c;
+   size_t size, c;
off_t base_offset;
unsigned shift;
void *data;
@@ -551,11 +551,11 @@ static void *unpack_raw_entry(struct object_entry *obj,
 }
 
 static void *unpack_data(struct object_entry *obj,
-int (*consume)(const unsigned char *, unsigned long, 
void *),
+int (*consume)(const unsigned char *, size_t, void *),
 void *cb_data)
 {
off_t from = obj[0].idx.offset + obj[0].hdr_size;
-   off_t len = obj[1].idx.offset - from;
+   size_t len = obj[1].idx.offset - from;
unsigned char *data, *inbuf;
git_zstream stream;
int status;
@@ -728,10 +728,10 @@ struct compare_data {
struct object_entry *entry;
struct git_istream *st;
unsigned char *buf;
-   unsigned long buf_size;
+   size_t buf_size;
 };
 
-static int compare_objects(const unsigned char *buf, unsigned long size,
+static int compare_objects(const unsigned char *buf, size_t size,
   void *cb_data)
 {
struct compare_data *data = cb_data;
@@ -783,7 +783,7 @@ static int check_collison(struct object_entry *entry)
 }
 
 static void sha1_object(const void *data, struct object_entry *obj_entry,
-   unsigned long size, enum object_type type,
+   size_t size, enum object_type type,
const struct object_id *oid)
 {
void *new_data = NULL;
@@ -1311,11 +1311,11 @@ static int write_compressed(struct sha1file *f, void 
*in, unsigned int size)
 
 static struct object_entry *append_obj_to_pack(struct sha1file *f,
   const unsigned char *sha1, void *buf,
-  unsigned long size, enum object_type type)
+  size_t size, enum object_type type)
 {
struct object_entry *obj = [nr_objects++];
unsigned char header[10];
-   unsigned long s = size;
+   size_t s = size;
int n = 0;
unsigned char c = (type << 4) | (s & 15);
s >>= 4;
@@ -1585,10 +1585,10 @@ static void show_pack_info(int stat_only)
chain_histogram[obj_stat[i].delta_depth - 1]++;
if (stat_only)
continue;
-   printf("%s %-6s %" PRIuMAX " %lu %" PRIuMAX,
+   printf("%s %-6s %" PRIuMAX " %" PRIuMAX " %" PRIuMAX,
   oid_to_hex(>idx.oid),
   typename(obj->real_type), (uintmax_t)obj->size,
-  (unsigned long)(obj[1].idx.offset - obj->idx.offset),
+  (uintmax_t)(obj[1].idx.offset - obj->idx.offset),
   (uintmax_t)obj->idx.offset);
if (is_delta_type(obj->type)) {
struct object_entry *bobj = 
[obj_stat[i].base_object_no];
-- 
2.1.4

[Patch size_t V3 02/19] Convert size datatype to size_t

[Martin Koegler]

From: Martin Koegler 

It changes the signature of the core object access function
including any other functions to assure a clean compile if
sizeof(size_t) != sizeof(unsigned long).

Signed-off-by: Martin Koegler 
---
 apply.c  |  6 +++---
 archive-tar.c|  4 ++--
 archive-zip.c|  2 +-
 archive.c|  2 +-
 archive.h|  2 +-
 blame.c  |  4 ++--
 blame.h  |  2 +-
 builtin/cat-file.c   | 14 +++---
 builtin/difftool.c   |  2 +-
 builtin/fast-export.c|  8 
 builtin/fmt-merge-msg.c  |  2 +-
 builtin/fsck.c   |  2 +-
 builtin/grep.c   |  8 
 builtin/index-pack.c | 16 
 builtin/log.c|  4 ++--
 builtin/ls-tree.c|  4 ++--
 builtin/merge-tree.c |  6 +++---
 builtin/mktag.c  |  2 +-
 builtin/notes.c  |  6 +++---
 builtin/pack-objects.c   | 24 ---
 builtin/reflog.c |  2 +-
 builtin/tag.c|  4 ++--
 builtin/unpack-file.c|  2 +-
 builtin/unpack-objects.c | 14 +++---
 builtin/verify-commit.c  |  2 +-
 bundle.c |  2 +-
 cache.h  | 22 ++---
 combine-diff.c   |  9 +
 commit.c |  6 +++---
 config.c |  2 +-
 delta.h  | 26 -
 diff-delta.c | 16 
 diff.c   | 18 -
 diff.h   |  2 +-
 diffcore.h   |  2 +-
 dir.c|  2 +-
 entry.c  |  4 ++--
 fast-import.c| 24 +++
 fsck.c   |  2 +-
 grep.h   |  2 +-
 http-push.c  |  5 +++--
 mailmap.c|  2 +-
 match-trees.c|  4 ++--
 merge-blobs.c|  6 +++---
 merge-blobs.h|  2 +-
 merge-recursive.c|  4 ++--
 notes-cache.c|  2 +-
 notes-merge.c|  2 +-
 notes.c  |  6 +++---
 object.c |  2 +-
 pack-check.c |  2 +-
 pack-objects.h   |  6 +++---
 patch-delta.c| 11 ++-
 read-cache.c |  4 ++--
 ref-filter.c |  4 ++--
 remote-testsvn.c |  4 ++--
 rerere.c |  2 +-
 sha1_file.c  | 50 +---
 streaming.c  |  8 
 streaming.h  |  2 +-
 submodule-config.c   |  2 +-
 t/helper/test-delta.c|  2 +-
 tag.c|  4 ++--
 tree-walk.c  |  8 
 tree.c   |  2 +-
 xdiff-interface.c|  2 +-
 66 files changed, 219 insertions(+), 212 deletions(-)

diff --git a/apply.c b/apply.c
index 41ee63e..af9ffee 100644
--- a/apply.c
+++ b/apply.c
@@ -3082,7 +3082,7 @@ static int apply_binary_fragment(struct apply_state 
*state,
 struct patch *patch)
 {
struct fragment *fragment = patch->fragments;
-   unsigned long len;
+   size_t len;
void *dst;
 
if (!fragment)
@@ -3171,7 +3171,7 @@ static int apply_binary(struct apply_state *state,
if (has_sha1_file(oid.hash)) {
/* We already have the postimage */
enum object_type type;
-   unsigned long size;
+   size_t size;
char *result;
 
result = read_sha1_file(oid.hash, , );
@@ -3233,7 +3233,7 @@ static int read_blob_object(struct strbuf *buf, const 
struct object_id *oid, uns
strbuf_addf(buf, "Subproject commit %s\n", oid_to_hex(oid));
} else {
enum object_type type;
-   unsigned long sz;
+   size_t sz;
char *result;
 
result = read_sha1_file(oid->hash, , );
diff --git a/archive-tar.c b/archive-tar.c
index c6ed96e..719673d 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -115,7 +115,7 @@ static int stream_blocked(const unsigned char *sha1)
 {
struct git_istream *st;
enum object_type type;
-   unsigned long sz;
+   size_t sz;
char buf[BLOCKSIZE];
ssize_t readlen;
 
@@ -240,7 +240,7 @@ static int write_tar_entry(struct archiver_args *args,
struct ustar_header header;
struct strbuf ext_header = STRBUF_INIT;
unsigned int old_mode = mode;
-   unsigned long size, size_in_header;
+   size_t size, size_in_header;
void *buffer;
int err = 0;
 
diff --git a/archive-zip.c b/archive-zip.c
index e8913e5..4492d64 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -295,7 +295,7 @@ static int write_zip_entry(struct archiver_args *args,
void *buffer;
struct git_istream *stream = NULL;
unsigned long flags = 0;
-   unsigned long size;
+   size_t size;

[Patch size_t V3 00/19] use size_t

[Martin Koegler]

From: Martin Koegler 

This patchset is for next [24db08a6e8fed761d3bace7f2d5997806e20b9f7].
Its a complete collection of all patches. Some errors were fixed and
it sticks with off_t for length in (pack) files. Object sizes are handled
as size_t.

Martin Koegler (19):
  delta: fix enconding size larger than an "uint" can hold
  Convert size datatype to size_t
  Convert zlib.c to size_t
  delta: Fix offset overflows
  Convert sha1_file.c to size_t
  Use size_t for sha1
  Convert parse_X_buffer to size_t
  Convert fsck.c & commit.c to size_t
  Convert cache functions to size_t
  Add overflow check to get_delta_hdr_size
  Use size_t for config parsing
  Convert pack-objects to size_t
  Convert index-pack to size_t
  Convert unpack-objects to size_t
  Convert archive functions to size_t
  Convert various things to size_t
  Convert ref-filter to size_t
  Convert tree-walk to size_t
  Convert xdiff-interface to size_t

 Documentation/technical/api-parse-options.txt |   2 +-
 apply.c   |   6 +-
 archive-tar.c |  20 ++---
 archive-zip.c |  24 ++---
 archive.c |   2 +-
 archive.h |   2 +-
 bisect.c  |   2 +-
 blame.c   |   6 +-
 blame.h   |   2 +-
 blob.c|   2 +-
 blob.h|   2 +-
 block-sha1/sha1.c |   2 +-
 block-sha1/sha1.h |   2 +-
 builtin/cat-file.c|  14 +--
 builtin/difftool.c|   2 +-
 builtin/fast-export.c |   8 +-
 builtin/fmt-merge-msg.c   |   4 +-
 builtin/fsck.c|   4 +-
 builtin/grep.c|   8 +-
 builtin/index-pack.c  |  40 -
 builtin/log.c |   4 +-
 builtin/ls-tree.c |   4 +-
 builtin/merge-tree.c  |   6 +-
 builtin/mktag.c   |   4 +-
 builtin/notes.c   |   6 +-
 builtin/pack-objects.c|  84 +-
 builtin/reflog.c  |   2 +-
 builtin/replace.c |   2 +-
 builtin/tag.c |   4 +-
 builtin/unpack-file.c |   2 +-
 builtin/unpack-objects.c  |  34 +++
 builtin/verify-commit.c   |   2 +-
 bundle.c  |   2 +-
 cache-tree.c  |   6 +-
 cache-tree.h  |   2 +-
 cache.h   |  54 ++--
 combine-diff.c|  11 +--
 commit.c  |  22 ++---
 commit.h  |  10 +--
 config.c  |  29 --
 config.h  |   2 +
 convert.c |  18 ++--
 delta.h   |  31 ---
 diff-delta.c  |  42 +
 diff.c|  46 +-
 diff.h|   2 +-
 diffcore-pickaxe.c|   4 +-
 diffcore.h|   2 +-
 dir.c |   6 +-
 dir.h |   2 +-
 entry.c   |   4 +-
 environment.c |   4 +-
 fast-import.c |  24 ++---
 fsck.c|  16 ++--
 fsck.h|   2 +-
 grep.h|   2 +-
 http-push.c   |   5 +-
 mailmap.c |   2 +-
 match-trees.c |   4 +-
 merge-blobs.c |   6 +-
 merge-blobs.h |   2 +-
 merge-recursive.c |   4 +-
 notes-cache.c |   2 +-
 notes-merge.c |   2 +-
 notes.c   |   6 +-
 object.c  |   4 +-
 object.h  |   2 +-
 pack-check.c  |   8 +-
 pack-objects.h|   6 +-
 pack.h|   2 +-
 parse-options.c  

[Patch size_t V3 14/19] Convert unpack-objects to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 builtin/unpack-objects.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 001dd4b..0d8b6b3 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -31,7 +31,7 @@ static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
  */
 struct obj_buffer {
char *buffer;
-   unsigned long size;
+   size_t size;
 };
 
 static struct decoration obj_decorate;
@@ -41,7 +41,7 @@ static struct obj_buffer *lookup_object_buffer(struct object 
*base)
return lookup_decoration(_decorate, base);
 }
 
-static void add_object_buffer(struct object *object, char *buffer, unsigned 
long size)
+static void add_object_buffer(struct object *object, char *buffer, size_t size)
 {
struct obj_buffer *obj;
obj = xcalloc(1, sizeof(struct obj_buffer));
@@ -93,7 +93,7 @@ static void use(int bytes)
die(_("pack exceeds maximum allowed size"));
 }
 
-static void *get_data(unsigned long size)
+static void *get_data(size_t size)
 {
git_zstream stream;
void *buf = xmallocz(size);
@@ -130,7 +130,7 @@ struct delta_info {
struct object_id base_oid;
unsigned nr;
off_t base_offset;
-   unsigned long size;
+   size_t size;
void *delta;
struct delta_info *next;
 };
@@ -139,7 +139,7 @@ static struct delta_info *delta_list;
 
 static void add_delta_to_list(unsigned nr, const struct object_id *base_oid,
  off_t base_offset,
- void *delta, unsigned long size)
+ void *delta, size_t size)
 {
struct delta_info *info = xmalloc(sizeof(*info));
 
@@ -226,7 +226,7 @@ static void write_rest(void)
 }
 
 static void added_object(unsigned nr, enum object_type type,
-void *data, unsigned long size);
+void *data, size_t size);
 
 /*
  * Write out nr-th object from the list, now we know the contents
@@ -234,7 +234,7 @@ static void added_object(unsigned nr, enum object_type type,
  * to be checked at the end.
  */
 static void write_object(unsigned nr, enum object_type type,
-void *buf, unsigned long size)
+void *buf, size_t size)
 {
if (!strict) {
if (write_sha1_file(buf, size, typename(type), 
obj_list[nr].oid.hash) < 0)
@@ -291,7 +291,7 @@ static void resolve_delta(unsigned nr, enum object_type 
type,
  * resolve all the deltified objects that are based on it.
  */
 static void added_object(unsigned nr, enum object_type type,
-void *data, unsigned long size)
+void *data, size_t size)
 {
struct delta_info **p = _list;
struct delta_info *info;
@@ -310,7 +310,7 @@ static void added_object(unsigned nr, enum object_type type,
}
 }
 
-static void unpack_non_delta_entry(enum object_type type, unsigned long size,
+static void unpack_non_delta_entry(enum object_type type, size_t size,
   unsigned nr)
 {
void *buf = get_data(size);
@@ -436,7 +436,7 @@ static void unpack_one(unsigned nr)
 {
unsigned shift;
unsigned char *pack;
-   unsigned long size, c;
+   size_t size, c;
enum object_type type;
 
obj_list[nr].offset = consumed_bytes;
-- 
2.1.4

[Patch size_t V3 07/19] Convert parse_X_buffer to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 blob.c | 2 +-
 blob.h | 2 +-
 builtin/fsck.c | 2 +-
 commit.c   | 2 +-
 commit.h   | 2 +-
 object.c   | 2 +-
 object.h   | 2 +-
 pack-check.c   | 2 +-
 pack.h | 2 +-
 tag.c  | 4 ++--
 tag.h  | 2 +-
 tree.c | 2 +-
 tree.h | 2 +-
 13 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/blob.c b/blob.c
index fa2ab4f..7530624 100644
--- a/blob.c
+++ b/blob.c
@@ -11,7 +11,7 @@ struct blob *lookup_blob(const struct object_id *oid)
return object_as_type(obj, OBJ_BLOB, 0);
 }
 
-int parse_blob_buffer(struct blob *item, void *buffer, unsigned long size)
+int parse_blob_buffer(struct blob *item, void *buffer, size_t size)
 {
item->object.parsed = 1;
return 0;
diff --git a/blob.h b/blob.h
index 4460616..31e9aa3 100644
--- a/blob.h
+++ b/blob.h
@@ -11,7 +11,7 @@ struct blob {
 
 struct blob *lookup_blob(const struct object_id *oid);
 
-int parse_blob_buffer(struct blob *item, void *buffer, unsigned long size);
+int parse_blob_buffer(struct blob *item, void *buffer, size_t size);
 
 /**
  * Blobs do not contain references to other objects and do not have
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 635902c..e31a52a 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -369,7 +369,7 @@ static int fsck_obj(struct object *obj)
 }
 
 static int fsck_obj_buffer(const struct object_id *oid, enum object_type type,
-  unsigned long size, void *buffer, int *eaten)
+  size_t size, void *buffer, int *eaten)
 {
/*
 * Note, buffer may be NULL if type is OBJ_BLOB. See
diff --git a/commit.c b/commit.c
index 22e66b8..79decc2 100644
--- a/commit.c
+++ b/commit.c
@@ -310,7 +310,7 @@ const void *detach_commit_buffer(struct commit *commit, 
unsigned long *sizep)
return ret;
 }
 
-int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long 
size)
+int parse_commit_buffer(struct commit *item, const void *buffer, size_t size)
 {
const char *tail = buffer;
const char *bufptr = buffer;
diff --git a/commit.h b/commit.h
index 6d857f0..82e966e 100644
--- a/commit.h
+++ b/commit.h
@@ -58,7 +58,7 @@ struct commit *lookup_commit_reference_by_name(const char 
*name);
  */
 struct commit *lookup_commit_or_die(const struct object_id *oid, const char 
*ref_name);
 
-int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long 
size);
+int parse_commit_buffer(struct commit *item, const void *buffer, size_t size);
 int parse_commit_gently(struct commit *item, int quiet_on_missing);
 static inline int parse_commit(struct commit *item)
 {
diff --git a/object.c b/object.c
index dff9f52..1a18545 100644
--- a/object.c
+++ b/object.c
@@ -179,7 +179,7 @@ struct object *lookup_unknown_object(const unsigned char 
*sha1)
return obj;
 }
 
-struct object *parse_object_buffer(const struct object_id *oid, enum 
object_type type, unsigned long size, void *buffer, int *eaten_p)
+struct object *parse_object_buffer(const struct object_id *oid, enum 
object_type type, size_t size, void *buffer, int *eaten_p)
 {
struct object *obj;
*eaten_p = 0;
diff --git a/object.h b/object.h
index 0a419ba..8b1bbfd 100644
--- a/object.h
+++ b/object.h
@@ -102,7 +102,7 @@ struct object *parse_object_or_die(const struct object_id 
*oid, const char *name
  * parsing it.  eaten_p indicates if the object has a borrowed copy
  * of buffer and the caller should not free() it.
  */
-struct object *parse_object_buffer(const struct object_id *oid, enum 
object_type type, unsigned long size, void *buffer, int *eaten_p);
+struct object *parse_object_buffer(const struct object_id *oid, enum 
object_type type, size_t size, void *buffer, int *eaten_p);
 
 /** Returns the object, with potentially excess memory allocated. **/
 struct object *lookup_unknown_object(const unsigned  char *sha1);
diff --git a/pack-check.c b/pack-check.c
index a78da5a..da64a91 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -175,7 +175,7 @@ int verify_pack_index(struct packed_git *p)
 
/* Verify SHA1 sum of the index file */
git_SHA1_Init();
-   git_SHA1_Update(, index_base, (unsigned int)(index_size - 20));
+   git_SHA1_Update(, index_base, (size_t)(index_size - 20));
git_SHA1_Final(sha1, );
if (hashcmp(sha1, index_base + index_size - 20))
err = error("Packfile index for %s SHA1 mismatch",
diff --git a/pack.h b/pack.h
index 8294341..0070927 100644
--- a/pack.h
+++ b/pack.h
@@ -75,7 +75,7 @@ struct pack_idx_entry {
 
 struct progress;
 /* Note, the data argument could be NULL if object type is blob */
-typedef int (*verify_fn)(const struct object_id *, enum object_type, unsigned 
long, void*, int*);
+typedef int (*verify_fn)(const struct object_id *, enum object_type, size_t, 
void*, int*);
 
 extern const 

[Patch size_t V3 15/19] Convert archive functions to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 archive-tar.c | 16 
 archive-zip.c | 22 +++---
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index 719673d..ee56b2b 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -12,7 +12,7 @@
 #define BLOCKSIZE  (RECORDSIZE * 20)
 
 static char block[BLOCKSIZE];
-static unsigned long offset;
+static size_t offset;
 
 static int tar_umask = 002;
 
@@ -50,12 +50,12 @@ static void write_if_needed(void)
  * queues up writes, so that all our write(2) calls write exactly one
  * full block; pads writes to RECORDSIZE
  */
-static void do_write_blocked(const void *data, unsigned long size)
+static void do_write_blocked(const void *data, size_t size)
 {
const char *buf = data;
 
if (offset) {
-   unsigned long chunk = BLOCKSIZE - offset;
+   size_t chunk = BLOCKSIZE - offset;
if (size < chunk)
chunk = size;
memcpy(block + offset, buf, chunk);
@@ -77,7 +77,7 @@ static void do_write_blocked(const void *data, unsigned long 
size)
 
 static void finish_record(void)
 {
-   unsigned long tail;
+   size_t tail;
tail = offset % RECORDSIZE;
if (tail)  {
memset(block + offset, 0, RECORDSIZE - tail);
@@ -86,7 +86,7 @@ static void finish_record(void)
write_if_needed();
 }
 
-static void write_blocked(const void *data, unsigned long size)
+static void write_blocked(const void *data, size_t size)
 {
do_write_blocked(data, size);
finish_record();
@@ -198,10 +198,10 @@ static size_t get_path_prefix(const char *path, size_t 
pathlen, size_t maxlen)
 
 static void prepare_header(struct archiver_args *args,
   struct ustar_header *header,
-  unsigned int mode, unsigned long size)
+  unsigned int mode, size_t size)
 {
xsnprintf(header->mode, sizeof(header->mode), "%07o", mode & 0);
-   xsnprintf(header->size, sizeof(header->size), "%011lo", S_ISREG(mode) ? 
size : 0);
+   xsnprintf(header->size, sizeof(header->size), "%011lo", S_ISREG(mode) ? 
(unsigned long)size : 0);
xsnprintf(header->mtime, sizeof(header->mtime), "%011lo", (unsigned 
long) args->time);
 
xsnprintf(header->uid, sizeof(header->uid), "%07o", 0);
@@ -219,7 +219,7 @@ static void prepare_header(struct archiver_args *args,
 
 static void write_extended_header(struct archiver_args *args,
  const unsigned char *sha1,
- const void *buffer, unsigned long size)
+ const void *buffer, size_t size)
 {
struct ustar_header header;
unsigned int mode;
diff --git a/archive-zip.c b/archive-zip.c
index 4492d64..3a54d80 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -186,12 +186,12 @@ static uint32_t clamp32(uintmax_t n)
return (n < max) ? n : max;
 }
 
-static void *zlib_deflate_raw(void *data, unsigned long size,
+static void *zlib_deflate_raw(void *data, size_t size,
  int compression_level,
- unsigned long *compressed_size)
+ size_t *compressed_size)
 {
git_zstream stream;
-   unsigned long maxsize;
+   size_t maxsize;
void *buffer;
int result;
 
@@ -219,9 +219,9 @@ static void *zlib_deflate_raw(void *data, unsigned long 
size,
return buffer;
 }
 
-static void write_zip_data_desc(unsigned long size,
-   unsigned long compressed_size,
-   unsigned long crc)
+static void write_zip_data_desc(size_t size,
+   size_t compressed_size,
+   uint32_t crc)
 {
if (size >= 0x || compressed_size >= 0x) {
struct zip64_data_desc trailer;
@@ -243,9 +243,9 @@ static void write_zip_data_desc(unsigned long size,
 }
 
 static void set_zip_header_data_desc(struct zip_local_header *header,
-unsigned long size,
-unsigned long compressed_size,
-unsigned long crc)
+size_t size,
+size_t compressed_size,
+uint32_t crc)
 {
copy_le32(header->crc32, crc);
copy_le32(header->compressed_size, compressed_size);
@@ -287,8 +287,8 @@ static int write_zip_entry(struct archiver_args *args,
size_t header_extra_size = ZIP_EXTRA_MTIME_SIZE;
int need_zip64_extra = 0;
unsigned long attr2;
-   unsigned long compressed_size;
-   unsigned long crc;
+   size_t compressed_size;
+   uint32_t crc;
int method;

[Patch size_t V3 16/19] Convert various things to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 bisect.c| 2 +-
 blame.c | 2 +-
 builtin/fmt-merge-msg.c | 2 +-
 builtin/mktag.c | 2 +-
 dir.c   | 4 ++--
 dir.h   | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/bisect.c b/bisect.c
index 2549eaf..0580c82 100644
--- a/bisect.c
+++ b/bisect.c
@@ -131,7 +131,7 @@ static void show_list(const char *debug, int counted, int 
nr,
struct commit *commit = p->item;
unsigned flags = commit->object.flags;
enum object_type type;
-   unsigned long size;
+   size_t size;
char *buf = read_sha1_file(commit->object.sha1, , );
const char *subject_start;
int subject_len;
diff --git a/blame.c b/blame.c
index 739a280..f628b42 100644
--- a/blame.c
+++ b/blame.c
@@ -1621,7 +1621,7 @@ static const char *get_next_line(const char *start, const 
char *end)
 static int prepare_lines(struct blame_scoreboard *sb)
 {
const char *buf = sb->final_buf;
-   unsigned long len = sb->final_buf_size;
+   size_t len = sb->final_buf_size;
const char *end = buf + len;
const char *p;
int *lineno;
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 61ab796..3faf100 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -464,7 +464,7 @@ static void fmt_merge_msg_title(struct strbuf *out,
 static void fmt_tag_signature(struct strbuf *tagbuf,
  struct strbuf *sig,
  const char *buf,
- unsigned long len)
+ size_t len)
 {
const char *tag_body = strstr(buf, "\n\n");
if (tag_body) {
diff --git a/builtin/mktag.c b/builtin/mktag.c
index 0663106..ff919a7 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -34,7 +34,7 @@ static int verify_object(const unsigned char *sha1, const 
char *expected_type)
return ret;
 }
 
-static int verify_tag(char *buffer, unsigned long size)
+static int verify_tag(char *buffer, size_t size)
 {
int typelen;
char type[20];
diff --git a/dir.c b/dir.c
index f161c26..0c7c767 100644
--- a/dir.c
+++ b/dir.c
@@ -180,7 +180,7 @@ static size_t common_prefix_len(const struct pathspec 
*pathspec)
  */
 char *common_prefix(const struct pathspec *pathspec)
 {
-   unsigned long len = common_prefix_len(pathspec);
+   size_t len = common_prefix_len(pathspec);
 
return len ? xmemdupz(pathspec->items[0].match, len) : NULL;
 }
@@ -2673,7 +2673,7 @@ static void load_sha1_stat(struct sha1_stat *sha1_stat,
sha1_stat->valid = 1;
 }
 
-struct untracked_cache *read_untracked_extension(const void *data, unsigned 
long sz)
+struct untracked_cache *read_untracked_extension(const void *data, size_t sz)
 {
struct untracked_cache *uc;
struct read_data rd;
diff --git a/dir.h b/dir.h
index e371705..709c72c 100644
--- a/dir.h
+++ b/dir.h
@@ -349,7 +349,7 @@ void untracked_cache_remove_from_index(struct index_state 
*, const char *);
 void untracked_cache_add_to_index(struct index_state *, const char *);
 
 void free_untracked_cache(struct untracked_cache *);
-struct untracked_cache *read_untracked_extension(const void *data, unsigned 
long sz);
+struct untracked_cache *read_untracked_extension(const void *data, size_t sz);
 void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked);
 void add_untracked_cache(struct index_state *istate);
 void remove_untracked_cache(struct index_state *istate);
-- 
2.1.4

[Patch size_t V3 18/19] Convert tree-walk to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 tree-walk.c | 17 +
 tree-walk.h |  4 ++--
 tree.h  |  2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/tree-walk.c b/tree-walk.c
index 7c9f9e3..a7d8b2a 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -22,10 +22,11 @@ static const char *get_mode(const char *str, unsigned int 
*modep)
return str;
 }
 
-static int decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned 
long size, struct strbuf *err)
+static int decode_tree_entry(struct tree_desc *desc, const char *buf, size_t 
size, struct strbuf *err)
 {
const char *path;
-   unsigned int mode, len;
+   unsigned int mode;
+   size_t len;
 
if (size < 23 || buf[size - 21]) {
strbuf_addstr(err, _("too-short tree object"));
@@ -51,7 +52,7 @@ static int decode_tree_entry(struct tree_desc *desc, const 
char *buf, unsigned l
return 0;
 }
 
-static int init_tree_desc_internal(struct tree_desc *desc, const void *buffer, 
unsigned long size, struct strbuf *err)
+static int init_tree_desc_internal(struct tree_desc *desc, const void *buffer, 
size_t size, struct strbuf *err)
 {
desc->buffer = buffer;
desc->size = size;
@@ -60,7 +61,7 @@ static int init_tree_desc_internal(struct tree_desc *desc, 
const void *buffer, u
return 0;
 }
 
-void init_tree_desc(struct tree_desc *desc, const void *buffer, unsigned long 
size)
+void init_tree_desc(struct tree_desc *desc, const void *buffer, size_t size)
 {
struct strbuf err = STRBUF_INIT;
if (init_tree_desc_internal(desc, buffer, size, ))
@@ -68,7 +69,7 @@ void init_tree_desc(struct tree_desc *desc, const void 
*buffer, unsigned long si
strbuf_release();
 }
 
-int init_tree_desc_gently(struct tree_desc *desc, const void *buffer, unsigned 
long size)
+int init_tree_desc_gently(struct tree_desc *desc, const void *buffer, size_t 
size)
 {
struct strbuf err = STRBUF_INIT;
int result = init_tree_desc_internal(desc, buffer, size, );
@@ -106,8 +107,8 @@ static int update_tree_entry_internal(struct tree_desc 
*desc, struct strbuf *err
 {
const void *buf = desc->buffer;
const unsigned char *end = desc->entry.oid->hash + 20;
-   unsigned long size = desc->size;
-   unsigned long len = end - (const unsigned char *)buf;
+   size_t size = desc->size;
+   size_t len = end - (const unsigned char *)buf;
 
if (size < len)
die(_("too-short tree file"));
@@ -487,7 +488,7 @@ int traverse_trees(int n, struct tree_desc *t, struct 
traverse_info *info)
 
 struct dir_state {
void *tree;
-   unsigned long size;
+   size_t size;
unsigned char sha1[20];
 };
 
diff --git a/tree-walk.h b/tree-walk.h
index 68bb78b..9160cc2 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -32,8 +32,8 @@ static inline int tree_entry_len(const struct name_entry *ne)
 
 void update_tree_entry(struct tree_desc *);
 int update_tree_entry_gently(struct tree_desc *);
-void init_tree_desc(struct tree_desc *desc, const void *buf, unsigned long 
size);
-int init_tree_desc_gently(struct tree_desc *desc, const void *buf, unsigned 
long size);
+void init_tree_desc(struct tree_desc *desc, const void *buf, size_t size);
+int init_tree_desc_gently(struct tree_desc *desc, const void *buf, size_t 
size);
 
 /*
  * Helper function that does both tree_entry_extract() and update_tree_entry()
diff --git a/tree.h b/tree.h
index 1dac7fc..69b9233 100644
--- a/tree.h
+++ b/tree.h
@@ -9,7 +9,7 @@ struct strbuf;
 struct tree {
struct object object;
void *buffer;
-   unsigned long size;
+   size_t size;
 };
 
 struct tree *lookup_tree(const struct object_id *oid);
-- 
2.1.4

[Patch size_t V3 11/19] Use size_t for config parsing

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 builtin/pack-objects.c |  6 +++---
 config.c   | 27 ++-
 config.h   |  2 ++
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 9067803..fbb07a8 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2450,7 +2450,7 @@ static int git_pack_config(const char *k, const char *v, 
void *cb)
return 0;
}
if (!strcmp(k, "pack.windowmemory")) {
-   window_memory_limit = git_config_ulong(k, v);
+   window_memory_limit = git_config_size_t(k, v);
return 0;
}
if (!strcmp(k, "pack.depth")) {
@@ -2458,11 +2458,11 @@ static int git_pack_config(const char *k, const char 
*v, void *cb)
return 0;
}
if (!strcmp(k, "pack.deltacachesize")) {
-   max_delta_cache_size = git_config_int(k, v);
+   max_delta_cache_size = git_config_size_t(k, v);
return 0;
}
if (!strcmp(k, "pack.deltacachelimit")) {
-   cache_max_small_delta_size = git_config_int(k, v);
+   cache_max_small_delta_size = git_config_size_t(k, v);
return 0;
}
if (!strcmp(k, "pack.writebitmaphashcache")) {
diff --git a/config.c b/config.c
index bf4e2e4..50f0077 100644
--- a/config.c
+++ b/config.c
@@ -863,6 +863,15 @@ static int git_parse_ssize_t(const char *value, ssize_t 
*ret)
return 1;
 }
 
+int git_parse_size_t(const char *value, size_t *ret)
+{
+   uintmax_t tmp;
+   if (!git_parse_unsigned(value, , 
maximum_signed_value_of_type(size_t)))
+   return 0;
+   *ret = tmp;
+   return 1;
+}
+
 NORETURN
 static void die_bad_number(const char *name, const char *value)
 {
@@ -929,6 +938,14 @@ ssize_t git_config_ssize_t(const char *name, const char 
*value)
return ret;
 }
 
+size_t git_config_size_t(const char *name, const char *value)
+{
+   size_t ret;
+   if (!git_parse_size_t(value, ))
+   die_bad_number(name, value);
+   return ret;
+}
+
 int git_parse_maybe_bool(const char *value)
 {
if (!value)
@@ -1105,7 +1122,7 @@ static int git_default_core_config(const char *var, const 
char *value)
 
if (!strcmp(var, "core.packedgitwindowsize")) {
int pgsz_x2 = getpagesize() * 2;
-   packed_git_window_size = git_config_ulong(var, value);
+   packed_git_window_size = git_config_size_t(var, value);
 
/* This value must be multiple of (pagesize * 2) */
packed_git_window_size /= pgsz_x2;
@@ -1116,17 +1133,17 @@ static int git_default_core_config(const char *var, 
const char *value)
}
 
if (!strcmp(var, "core.bigfilethreshold")) {
-   big_file_threshold = git_config_ulong(var, value);
+   big_file_threshold = git_config_size_t(var, value);
return 0;
}
 
if (!strcmp(var, "core.packedgitlimit")) {
-   packed_git_limit = git_config_ulong(var, value);
+   packed_git_limit = git_config_size_t(var, value);
return 0;
}
 
if (!strcmp(var, "core.deltabasecachelimit")) {
-   delta_base_cache_limit = git_config_ulong(var, value);
+   delta_base_cache_limit = git_config_size_t(var, value);
return 0;
}
 
@@ -1360,7 +1377,7 @@ int git_default_config(const char *var, const char 
*value, void *dummy)
}
 
if (!strcmp(var, "pack.packsizelimit")) {
-   pack_size_limit_cfg = git_config_ulong(var, value);
+   pack_size_limit_cfg = git_config_size_t(var, value);
return 0;
}
 
diff --git a/config.h b/config.h
index 18b6f3f..cbaa3e3 100644
--- a/config.h
+++ b/config.h
@@ -49,11 +49,13 @@ extern int config_with_options(config_fn_t fn, void *,
   struct git_config_source *config_source,
   const struct config_options *opts);
 extern int git_parse_ulong(const char *, unsigned long *);
+extern int git_parse_size_t(const char *, size_t *);
 extern int git_parse_maybe_bool(const char *);
 extern int git_config_int(const char *, const char *);
 extern int64_t git_config_int64(const char *, const char *);
 extern unsigned long git_config_ulong(const char *, const char *);
 extern ssize_t git_config_ssize_t(const char *, const char *);
+extern size_t git_config_size_t(const char *, const char *);
 extern int git_config_bool_or_int(const char *, const char *, int *);
 extern int git_config_bool(const char *, const char *);
 extern int git_config_maybe_bool(const char *, const char *);
-- 
2.1.4

[Patch size_t V3 12/19] Convert pack-objects to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 Documentation/technical/api-parse-options.txt |  2 +-
 builtin/pack-objects.c| 46 +--
 parse-options.c   |  6 ++--
 3 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/Documentation/technical/api-parse-options.txt 
b/Documentation/technical/api-parse-options.txt
index 829b558..22b788e 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -181,7 +181,7 @@ There are some macros to easily define options:
Introduce an option with a size argument. The argument must be a
non-negative integer and may include a suffix of 'k', 'm' or 'g' to
scale the provided value by 1024, 1024^2 or 1024^3 respectively.
-   The scaled value is put into `unsigned_long_var`.
+   The scaled value is put into `size_t`.
 
 `OPT_DATE(short, long, _t_var, description)`::
Introduce an option with date argument, see `approxidate()`.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index fbb07a8..12457ae 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -56,7 +56,7 @@ static struct pack_idx_option pack_idx_opts;
 static const char *base_name;
 static int progress = 1;
 static int window = 10;
-static unsigned long pack_size_limit;
+static size_t pack_size_limit;
 static int depth = 50;
 static int delta_search_threads;
 static int pack_to_stdout;
@@ -72,11 +72,11 @@ static int use_bitmap_index = -1;
 static int write_bitmap_index;
 static uint16_t write_bitmap_options;
 
-static unsigned long delta_cache_size = 0;
-static unsigned long max_delta_cache_size = 256 * 1024 * 1024;
-static unsigned long cache_max_small_delta_size = 1000;
+static size_t delta_cache_size = 0;
+static size_t max_delta_cache_size = 256 * 1024 * 1024;
+static size_t cache_max_small_delta_size = 1000;
 
-static unsigned long window_memory_limit = 0;
+static size_t window_memory_limit = 0;
 
 /*
  * stats
@@ -124,11 +124,11 @@ static void *get_delta(struct object_entry *entry)
return delta_buf;
 }
 
-static unsigned long do_compress(void **pptr, unsigned long size)
+static size_t do_compress(void **pptr, size_t size)
 {
git_zstream stream;
void *in, *out;
-   unsigned long maxsize;
+   size_t maxsize;
 
git_deflate_init(, pack_compression_level);
maxsize = git_deflate_bound(, size);
@@ -149,13 +149,13 @@ static unsigned long do_compress(void **pptr, unsigned 
long size)
return stream.total_out;
 }
 
-static unsigned long write_large_blob_data(struct git_istream *st, struct 
sha1file *f,
+static size_t write_large_blob_data(struct git_istream *st, struct sha1file *f,
   const unsigned char *sha1)
 {
git_zstream stream;
unsigned char ibuf[1024 * 16];
unsigned char obuf[1024 * 16];
-   unsigned long olen = 0;
+   size_t olen = 0;
 
git_deflate_init(, pack_compression_level);
 
@@ -196,7 +196,7 @@ static int check_pack_inflate(struct packed_git *p,
struct pack_window **w_curs,
off_t offset,
off_t len,
-   unsigned long expect)
+   size_t expect)
 {
git_zstream stream;
unsigned char fakebuf[4096], *in;
@@ -238,13 +238,13 @@ static void copy_pack_data(struct sha1file *f,
 }
 
 /* Return 0 if we will bust the pack-size limit */
-static unsigned long write_no_reuse_object(struct sha1file *f, struct 
object_entry *entry,
-  unsigned long limit, int 
usable_delta)
+static size_t write_no_reuse_object(struct sha1file *f, struct object_entry 
*entry,
+   size_t limit, int usable_delta)
 {
size_t size, datalen;
unsigned char header[MAX_PACK_OBJECT_HEADER],
  dheader[MAX_PACK_OBJECT_HEADER];
-   unsigned hdrlen;
+   size_t hdrlen;
enum object_type type;
void *buf;
struct git_istream *st = NULL;
@@ -350,7 +350,7 @@ static unsigned long write_no_reuse_object(struct sha1file 
*f, struct object_ent
 
 /* Return 0 if we will bust the pack-size limit */
 static off_t write_reuse_object(struct sha1file *f, struct object_entry *entry,
-   unsigned long limit, int usable_delta)
+   size_t limit, int usable_delta)
 {
struct packed_git *p = entry->in_pack;
struct pack_window *w_curs = NULL;
@@ -360,7 +360,7 @@ static off_t write_reuse_object(struct sha1file *f, struct 
object_entry *entry,
off_t datalen;
unsigned char header[MAX_PACK_OBJECT_HEADER],
  dheader[MAX_PACK_OBJECT_HEADER];
-   unsigned hdrlen;
+   size_t hdrlen;
 
if (entry->delta)
type = (allow_ofs_delta && 

[Patch size_t V3 10/19] Add overflow check to get_delta_hdr_size

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 delta.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/delta.h b/delta.h
index 2df0f55..dab7352 100644
--- a/delta.h
+++ b/delta.h
@@ -96,6 +96,11 @@ static inline size_t get_delta_hdr_size(const unsigned char 
**datap,
cmd = *data++;
size |= (cmd & 0x7f) << i;
i += 7;
+   if (bitsizeof(size_t) <= i) {
+   die("too large object size");
+   size = 0;
+   break;
+   }
} while (cmd & 0x80 && data < top);
*datap = data;
return size;
-- 
2.1.4

[Patch size_t V3 17/19] Convert ref-filter to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 ref-filter.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 5c903a5..30f249c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -724,7 +724,7 @@ static int grab_objectname(const char *name, const unsigned 
char *sha1,
 }
 
 /* See grab_values */
-static void grab_common_values(struct atom_value *val, int deref, struct 
object *obj, void *buf, unsigned long sz)
+static void grab_common_values(struct atom_value *val, int deref, struct 
object *obj, void *buf, size_t sz)
 {
int i;
 
@@ -739,7 +739,7 @@ static void grab_common_values(struct atom_value *val, int 
deref, struct object
v->s = typename(obj->type);
else if (!strcmp(name, "objectsize")) {
v->value = sz;
-   v->s = xstrfmt("%lu", sz);
+   v->s = xstrfmt("%" PRIuMAX, (uintmax_t)sz);
}
else if (deref)
grab_objectname(name, obj->oid.hash, v, _atom[i]);
@@ -747,7 +747,7 @@ static void grab_common_values(struct atom_value *val, int 
deref, struct object
 }
 
 /* See grab_values */
-static void grab_tag_values(struct atom_value *val, int deref, struct object 
*obj, void *buf, unsigned long sz)
+static void grab_tag_values(struct atom_value *val, int deref, struct object 
*obj, void *buf, size_t sz)
 {
int i;
struct tag *tag = (struct tag *) obj;
@@ -769,7 +769,7 @@ static void grab_tag_values(struct atom_value *val, int 
deref, struct object *ob
 }
 
 /* See grab_values */
-static void grab_commit_values(struct atom_value *val, int deref, struct 
object *obj, void *buf, unsigned long sz)
+static void grab_commit_values(struct atom_value *val, int deref, struct 
object *obj, void *buf, size_t sz)
 {
int i;
struct commit *commit = (struct commit *) obj;
@@ -786,7 +786,7 @@ static void grab_commit_values(struct atom_value *val, int 
deref, struct object
}
else if (!strcmp(name, "numparent")) {
v->value = commit_list_count(commit->parents);
-   v->s = xstrfmt("%lu", (unsigned long)v->value);
+   v->s = xstrfmt("%" PRIuMAX, (uintmax_t)v->value);
}
else if (!strcmp(name, "parent")) {
struct commit_list *parents;
@@ -802,7 +802,7 @@ static void grab_commit_values(struct atom_value *val, int 
deref, struct object
}
 }
 
-static const char *find_wholine(const char *who, int wholen, const char *buf, 
unsigned long sz)
+static const char *find_wholine(const char *who, int wholen, const char *buf, 
size_t sz)
 {
const char *eol;
while (*buf) {
@@ -848,7 +848,7 @@ static const char *copy_email(const char *buf)
return xmemdupz(email, eoemail + 1 - email);
 }
 
-static char *copy_subject(const char *buf, unsigned long len)
+static char *copy_subject(const char *buf, size_t len)
 {
char *r = xmemdupz(buf, len);
int i;
@@ -898,7 +898,7 @@ static void grab_date(const char *buf, struct atom_value 
*v, const char *atomnam
 }
 
 /* See grab_values */
-static void grab_person(const char *who, struct atom_value *val, int deref, 
struct object *obj, void *buf, unsigned long sz)
+static void grab_person(const char *who, struct atom_value *val, int deref, 
struct object *obj, void *buf, size_t sz)
 {
int i;
int wholen = strlen(who);
@@ -957,11 +957,11 @@ static void grab_person(const char *who, struct 
atom_value *val, int deref, stru
}
 }
 
-static void find_subpos(const char *buf, unsigned long sz,
-   const char **sub, unsigned long *sublen,
-   const char **body, unsigned long *bodylen,
-   unsigned long *nonsiglen,
-   const char **sig, unsigned long *siglen)
+static void find_subpos(const char *buf, size_t sz,
+   const char **sub, size_t *sublen,
+   const char **body, size_t *bodylen,
+   size_t *nonsiglen,
+   const char **sig, size_t *siglen)
 {
const char *eol;
/* skip past header until we hit empty line */
@@ -1005,7 +1005,7 @@ static void find_subpos(const char *buf, unsigned long sz,
  * If 'lines' is greater than 0, append that many lines from the given
  * 'buf' of length 'size' to the given strbuf.
  */
-static void append_lines(struct strbuf *out, const char *buf, unsigned long 
size, int lines)
+static void append_lines(struct strbuf *out, const char *buf, size_t size, int 
lines)
 {
int i;
const char *sp, *eol;
@@ -1026,11 +1026,11 @@ static void append_lines(struct strbuf *out, const char 
*buf, unsigned long size
 }
 
 /* See grab_values */
-static void 

[Patch size_t V3 08/19] Convert fsck.c & commit.c to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 builtin/replace.c |  2 +-
 commit.c  | 14 +++---
 commit.h  |  8 
 fsck.c| 14 +++---
 fsck.h|  2 +-
 5 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index f4a85a1..dcd0d1e 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -391,7 +391,7 @@ static int create_graft(int argc, const char **argv, int 
force)
struct commit *commit;
struct strbuf buf = STRBUF_INIT;
const char *buffer;
-   unsigned long size;
+   size_t size;
 
if (get_oid(old_ref, ) < 0)
die(_("Not a valid object name: '%s'"), old_ref);
diff --git a/commit.c b/commit.c
index 79decc2..5ebac6a 100644
--- a/commit.c
+++ b/commit.c
@@ -231,19 +231,19 @@ int unregister_shallow(const struct object_id *oid)
 
 struct commit_buffer {
void *buffer;
-   unsigned long size;
+   size_t size;
 };
 define_commit_slab(buffer_slab, struct commit_buffer);
 static struct buffer_slab buffer_slab = COMMIT_SLAB_INIT(1, buffer_slab);
 
-void set_commit_buffer(struct commit *commit, void *buffer, unsigned long size)
+void set_commit_buffer(struct commit *commit, void *buffer, size_t size)
 {
struct commit_buffer *v = buffer_slab_at(_slab, commit);
v->buffer = buffer;
v->size = size;
 }
 
-const void *get_cached_commit_buffer(const struct commit *commit, unsigned 
long *sizep)
+const void *get_cached_commit_buffer(const struct commit *commit, size_t 
*sizep)
 {
struct commit_buffer *v = buffer_slab_peek(_slab, commit);
if (!v) {
@@ -256,7 +256,7 @@ const void *get_cached_commit_buffer(const struct commit 
*commit, unsigned long
return v->buffer;
 }
 
-const void *get_commit_buffer(const struct commit *commit, unsigned long 
*sizep)
+const void *get_commit_buffer(const struct commit *commit, size_t *sizep)
 {
const void *ret = get_cached_commit_buffer(commit, sizep);
if (!ret) {
@@ -291,7 +291,7 @@ void free_commit_buffer(struct commit *commit)
}
 }
 
-const void *detach_commit_buffer(struct commit *commit, unsigned long *sizep)
+const void *detach_commit_buffer(struct commit *commit, size_t *sizep)
 {
struct commit_buffer *v = buffer_slab_peek(_slab, commit);
void *ret;
@@ -1128,7 +1128,7 @@ int parse_signed_commit(const struct commit *commit,
struct strbuf *payload, struct strbuf *signature)
 {
 
-   unsigned long size;
+   size_t size;
const char *buffer = get_commit_buffer(commit, );
int in_signature, saw_signature = -1;
const char *line, *tail;
@@ -1284,7 +1284,7 @@ struct commit_extra_header 
*read_commit_extra_headers(struct commit *commit,
  const char **exclude)
 {
struct commit_extra_header *extra = NULL;
-   unsigned long size;
+   size_t size;
const char *buffer = get_commit_buffer(commit, );
extra = read_commit_extra_header_lines(buffer, size, exclude);
unuse_commit_buffer(commit, buffer);
diff --git a/commit.h b/commit.h
index 82e966e..fd44de3 100644
--- a/commit.h
+++ b/commit.h
@@ -70,20 +70,20 @@ void parse_commit_or_die(struct commit *item);
  * Associate an object buffer with the commit. The ownership of the
  * memory is handed over to the commit, and must be free()-able.
  */
-void set_commit_buffer(struct commit *, void *buffer, unsigned long size);
+void set_commit_buffer(struct commit *, void *buffer, size_t size);
 
 /*
  * Get any cached object buffer associated with the commit. Returns NULL
  * if none. The resulting memory should not be freed.
  */
-const void *get_cached_commit_buffer(const struct commit *, unsigned long 
*size);
+const void *get_cached_commit_buffer(const struct commit *, size_t *size);
 
 /*
  * Get the commit's object contents, either from cache or by reading the object
  * from disk. The resulting memory should not be modified, and must be given
  * to unuse_commit_buffer when the caller is done.
  */
-const void *get_commit_buffer(const struct commit *, unsigned long *size);
+const void *get_commit_buffer(const struct commit *, size_t *size);
 
 /*
  * Tell the commit subsytem that we are done with a particular commit buffer.
@@ -102,7 +102,7 @@ void free_commit_buffer(struct commit *);
  * Disassociate any cached object buffer from the commit, but do not free it.
  * The buffer (or NULL, if none) is returned.
  */
-const void *detach_commit_buffer(struct commit *, unsigned long *sizep);
+const void *detach_commit_buffer(struct commit *, size_t *sizep);
 
 /* Find beginning and length of commit subject. */
 int find_commit_subject(const char *commit_buffer, const char **subject);
diff --git a/fsck.c b/fsck.c
index feca3a8..9039373 100644
--- a/fsck.c
+++ b/fsck.c
@@ -632,18 +632,18 @@ 

[Patch size_t V3 19/19] Convert xdiff-interface to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 combine-diff.c |  2 +-
 diff.c | 28 ++--
 diffcore-pickaxe.c |  4 ++--
 xdiff-interface.c  |  8 
 xdiff-interface.h  |  6 +++---
 5 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index acf39ec..ad5d177 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -343,7 +343,7 @@ struct combine_diff_state {
struct sline *lost_bucket;
 };
 
-static void consume_line(void *state_, char *line, unsigned long len)
+static void consume_line(void *state_, char *line, size_t len)
 {
struct combine_diff_state *state = state_;
if (5 < len && !memcmp("@@ -", line, 4)) {
diff --git a/diff.c b/diff.c
index c12d062..f665f8d 100644
--- a/diff.c
+++ b/diff.c
@@ -406,7 +406,7 @@ static struct diff_tempfile {
struct tempfile tempfile;
 } diff_temp[2];
 
-typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
+typedef size_t (*sane_truncate_fn)(char *line, size_t len);
 
 struct emit_callback {
int color_diff;
@@ -461,7 +461,7 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec 
*one)
 }
 
 /* like fill_mmfile, but only for size, so we can avoid retrieving blob */
-static unsigned long diff_filespec_size(struct diff_filespec *one)
+static size_t diff_filespec_size(struct diff_filespec *one)
 {
if (!DIFF_FILE_VALID(one))
return 0;
@@ -832,7 +832,7 @@ struct diff_words_buffer {
int orig_nr, orig_alloc;
 };
 
-static void diff_words_append(char *line, unsigned long len,
+static void diff_words_append(char *line, size_t len,
struct diff_words_buffer *buffer)
 {
ALLOC_GROW(buffer->text.ptr, buffer->text.size + len, buffer->alloc);
@@ -949,7 +949,7 @@ static int color_words_output_graph_prefix(struct 
diff_words_data *diff_words)
}
 }
 
-static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
+static void fn_out_diff_words_aux(void *priv, char *line, size_t len)
 {
struct diff_words_data *diff_words = priv;
struct diff_words_style *style = diff_words->style;
@@ -1237,10 +1237,10 @@ const char *diff_line_prefix(struct diff_options *opt)
return msgbuf->buf;
 }
 
-static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, 
unsigned long len)
+static size_t sane_truncate_line(struct emit_callback *ecb, char *line, size_t 
len)
 {
const char *cp;
-   unsigned long allot;
+   size_t allot;
size_t l = len;
 
if (ecb->truncate)
@@ -1270,7 +1270,7 @@ static void find_lno(const char *line, struct 
emit_callback *ecbdata)
ecbdata->lno_in_postimage = strtol(p + 1, NULL, 10);
 }
 
-static void fn_out_consume(void *priv, char *line, unsigned long len)
+static void fn_out_consume(void *priv, char *line, size_t len)
 {
struct emit_callback *ecbdata = priv;
const char *meta = diff_get_color(ecbdata->color_diff, DIFF_METAINFO);
@@ -1492,7 +1492,7 @@ static struct diffstat_file *diffstat_add(struct 
diffstat_t *diffstat,
return x;
 }
 
-static void diffstat_consume(void *priv, char *line, unsigned long len)
+static void diffstat_consume(void *priv, char *line, size_t len)
 {
struct diffstat_t *diffstat = priv;
struct diffstat_file *x = diffstat->files[diffstat->nr - 1];
@@ -2132,7 +2132,7 @@ struct checkdiff_t {
unsigned status;
 };
 
-static int is_conflict_marker(const char *line, int marker_size, unsigned long 
len)
+static int is_conflict_marker(const char *line, int marker_size, size_t len)
 {
char firstchar;
int cnt;
@@ -2155,7 +2155,7 @@ static int is_conflict_marker(const char *line, int 
marker_size, unsigned long l
return 1;
 }
 
-static void checkdiff_consume(void *priv, char *line, unsigned long len)
+static void checkdiff_consume(void *priv, char *line, size_t len)
 {
struct checkdiff_t *data = priv;
int marker_size = data->conflict_marker_size;
@@ -2953,7 +2953,7 @@ void diff_free_filespec_data(struct diff_filespec *s)
 
 static void prep_temp_blob(const char *path, struct diff_tempfile *temp,
   void *blob,
-  unsigned long size,
+  size_t size,
   const struct object_id *oid,
   int mode)
 {
@@ -4536,7 +4536,7 @@ struct patch_id_t {
int patchlen;
 };
 
-static int remove_space(char *line, int len)
+static size_t remove_space(char *line, size_t len)
 {
int i;
char *dst = line;
@@ -4549,10 +4549,10 @@ static int remove_space(char *line, int len)
return dst - line;
 }
 
-static void patch_id_consume(void *priv, char *line, unsigned long len)
+static void patch_id_consume(void *priv, char *line, size_t len)
 {
struct patch_id_t *data = priv;
-   int new_len;
+ 

[Patch size_t V3 04/19] delta: Fix offset overflows

[Martin Koegler]

From: Martin Koegler 

Prevent generating delta offsets beyond 4G.

Signed-off-by: Martin Koegler 
---
 diff-delta.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/diff-delta.c b/diff-delta.c
index dffbab1..4489b79 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -453,6 +453,9 @@ create_delta(const struct delta_index *index,
moff += msize;
msize = left;
 
+   if (moff > 0x)
+   msize = 0;
+
if (msize < 4096) {
int j;
val = 0;
-- 
2.1.4

[Patch size_t V3 01/19] delta: fix enconding size larger than an "uint" can hold

[Martin Koegler]

From: Martin Koegler 

The current delta code produces incorrect pack objects for files > 4GB.

Signed-off-by: Martin Koegler 
---
 diff-delta.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/diff-delta.c b/diff-delta.c
index 3797ce6..db27c1a 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -319,7 +319,8 @@ create_delta(const struct delta_index *index,
 const void *trg_buf, unsigned long trg_size,
 unsigned long *delta_size, unsigned long max_size)
 {
-   unsigned int i, outpos, outsize, moff, msize, val;
+   unsigned int i, val;
+   size_t l, outpos, moff, outsize, msize;
int inscnt;
const unsigned char *ref_data, *ref_top, *data, *top;
unsigned char *out;
@@ -336,20 +337,20 @@ create_delta(const struct delta_index *index,
return NULL;
 
/* store reference buffer size */
-   i = index->src_size;
-   while (i >= 0x80) {
-   out[outpos++] = i | 0x80;
-   i >>= 7;
+   l = index->src_size;
+   while (l >= 0x80) {
+   out[outpos++] = l | 0x80;
+   l >>= 7;
}
-   out[outpos++] = i;
+   out[outpos++] = l;
 
/* store target buffer size */
-   i = trg_size;
-   while (i >= 0x80) {
-   out[outpos++] = i | 0x80;
-   i >>= 7;
+   l = trg_size;
+   while (l >= 0x80) {
+   out[outpos++] = l | 0x80;
+   l >>= 7;
}
-   out[outpos++] = i;
+   out[outpos++] = l;
 
ref_data = index->src_buf;
ref_top = ref_data + index->src_size;
-- 
2.1.4

[Patch size_t V3 03/19] Convert zlib.c to size_t

[Martin Koegler]

From: Martin Koegler 

Signed-off-by: Martin Koegler 
---
 builtin/pack-objects.c |  8 
 cache.h| 12 ++--
 pack-check.c   |  4 ++--
 sha1_file.c|  6 +++---
 wrapper.c  |  8 
 zlib.c |  8 
 6 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d94fd17..9067803 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -225,12 +225,12 @@ static void copy_pack_data(struct sha1file *f,
off_t len)
 {
unsigned char *in;
-   unsigned long avail;
+   size_t avail;
 
while (len) {
in = use_pack(p, w_curs, offset, );
if (avail > len)
-   avail = (unsigned long)len;
+   avail = len;
sha1write(f, in, avail);
offset += avail;
len -= avail;
@@ -1388,8 +1388,8 @@ static void check_object(struct object_entry *entry)
struct pack_window *w_curs = NULL;
const unsigned char *base_ref = NULL;
struct object_entry *base_entry;
-   unsigned long used, used_0;
-   unsigned long avail;
+   size_t used, used_0;
+   size_t avail;
off_t ofs;
unsigned char *buf, c;
 
diff --git a/cache.h b/cache.h
index ee96192..5c85844 100644
--- a/cache.h
+++ b/cache.h
@@ -42,10 +42,10 @@
 #include 
 typedef struct git_zstream {
z_stream z;
-   unsigned long avail_in;
-   unsigned long avail_out;
-   unsigned long total_in;
-   unsigned long total_out;
+   size_t avail_in;
+   size_t avail_out;
+   size_t total_in;
+   size_t total_out;
unsigned char *next_in;
unsigned char *next_out;
 } git_zstream;
@@ -62,7 +62,7 @@ void git_deflate_end(git_zstream *);
 int git_deflate_abort(git_zstream *);
 int git_deflate_end_gently(git_zstream *);
 int git_deflate(git_zstream *, int flush);
-unsigned long git_deflate_bound(git_zstream *, unsigned long);
+size_t git_deflate_bound(git_zstream *, size_t);
 
 /* The length in bytes and in hex digits of an object name (SHA-1 value). */
 #define GIT_SHA1_RAWSZ 20
@@ -1686,7 +1686,7 @@ extern int open_pack_index(struct packed_git *);
  */
 extern void close_pack_index(struct packed_git *);
 
-extern unsigned char *use_pack(struct packed_git *, struct pack_window **, 
off_t, unsigned long *);
+extern unsigned char *use_pack(struct packed_git *, struct pack_window **, 
off_t, size_t *);
 extern void close_pack_windows(struct packed_git *);
 extern void close_all_packs(void);
 extern void unuse_pack(struct pack_window **);
diff --git a/pack-check.c b/pack-check.c
index 6f7714f..a78da5a 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -30,7 +30,7 @@ int check_pack_crc(struct packed_git *p, struct pack_window 
**w_curs,
uint32_t data_crc = crc32(0, NULL, 0);
 
do {
-   unsigned long avail;
+   size_t avail;
void *data = use_pack(p, w_curs, offset, );
if (avail > len)
avail = len;
@@ -65,7 +65,7 @@ static int verify_packfile(struct packed_git *p,
 
git_SHA1_Init();
do {
-   unsigned long remaining;
+   size_t remaining;
unsigned char *in = use_pack(p, w_curs, offset, );
offset += remaining;
if (!pack_sig_ofs)
diff --git a/sha1_file.c b/sha1_file.c
index 97b39b0..3428172 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1228,7 +1228,7 @@ static int in_window(struct pack_window *win, off_t 
offset)
 unsigned char *use_pack(struct packed_git *p,
struct pack_window **w_cursor,
off_t offset,
-   unsigned long *left)
+   size_t *left)
 {
struct pack_window *win = *w_cursor;
 
@@ -2122,8 +2122,8 @@ int unpack_object_header(struct packed_git *p,
 size_t *sizep)
 {
unsigned char *base;
-   unsigned long left;
-   unsigned long used;
+   size_t left;
+   size_t used;
enum object_type type;
 
/* use_pack() assures us we have [base, base + 20) available
diff --git a/wrapper.c b/wrapper.c
index 36630e5..c4253f7 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -67,11 +67,11 @@ static void *do_xmalloc(size_t size, int gentle)
ret = malloc(1);
if (!ret) {
if (!gentle)
-   die("Out of memory, malloc failed (tried to 
allocate %lu bytes)",
-   (unsigned long)size);
+   die("Out of memory, malloc failed (tried to 
allocate %" PRIuMAX " bytes)",
+   (uintmax_t)size);
else {
-   

Re: [PATCH v2 1/4] sha1_file: fix hardcoded size in null_sha1

[Junio C Hamano]

Patryk Obara  writes:

> This prevents compilation error if GIT_MAX_RAWSZ is different than 20.

The above made me scratch my head wondering why because it does not
say what the root cause of the issue is.  It would have avoided a
few strand of lost hair if it were more like this, perhaps:

The array is declared in cache.h as

extern const unsigned char null_sha1[GIT_MAX_RAWSZ];

The definition of it we have in sha1_file.c must match.


>
> Signed-off-by: Patryk Obara 
> ---
>  sha1_file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index b60ae15..f5b5bec 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -32,7 +32,7 @@
>  #define SZ_FMT PRIuMAX
>  static inline uintmax_t sz_fmt(size_t s) { return s; }
>  
> -const unsigned char null_sha1[20];
> +const unsigned char null_sha1[GIT_MAX_RAWSZ];
>  const struct object_id null_oid;
>  const struct object_id empty_tree_oid = {
>   EMPTY_TREE_SHA1_BIN_LITERAL

Re: [PATCH 5/5] commit: rewrite read_graft_line

[Junio C Hamano]

Patryk Obara  writes:

> Junio C Hamano  wrote:
>> I am not sure if this is a good approach.  Just like in 2/5 you can
>> use the MAX thing instead of 20, instead of having each graft entry
>> allocate a separate oid_array.oid[].
>
> Once MAX values were increased memory corruption was caused exactly by
> this line:
>
>> - graft = xmalloc(st_add(sizeof(*graft), st_mult(GIT_SHA1_RAWSZ, i)));
>
> I could've replaced it by:
>
> graft = xmalloc(st_add(sizeof(*graft), st_mult(sizeof(struct
> object_id), i)));
>
> But it seemed to me like short-sighted solution (code might be broken if
> object_id will be modified again in future).

Why?  

Assuming that "struct object_id" would have to be prepared to handle
more than one types of hash functions at the same time, it would be
sufficiently large to hold any type of hash, plus possibly another
member that tells which kind.

The decision to choose between a flex array and a separately
allocatable structure primarily should come from how the enclosing
struct (i.e. the commti_graft structure) is meant to be used.  It is
not meant to be tweaked by adding more parents or removing parents
once it is constructed, which argues for having a flex array to hold
the known and fixed number of parents once it is constructed.

Re: [PATCH 2/5] sha1_file: fix hardcoded size in null_sha1

[Junio C Hamano]

Patryk Obara  writes:

> Junio C Hamano  wrote:
>
>> I said this is OK for "null" because we assume we will use ^\0{len}$
>> for any hash function we choose as the "impossible" value, and for
>> that particular use pattern, we do not need such a union.  Just
>> letting the caller peek at an appropriate number of bytes at the
>> beginning of that NUL buffer for hash the caller wants to use is
>> sufficient.
>
> Do you think I should record this explanation as either commit message
> or comment in sha1_file.c?
>
>> MAX is inevitable only if we envision that we have to handle objects
>> named using two or more hashing schemes at the same time, with the
>> same binary and during the same run inside a single process.
>
> I think this will be the case if "transition one local repository at
> a time" from Jonathan Nieder's transition plan will be followed.
> This plan assumes object_id translation happening e.g. during fetch
> operation.

It would be good if that assumption is made explicit.

Thanks.

Re: What's cooking in git.git (Aug 2017, #03; Mon, 14)

[Junio C Hamano]

Kaartic Sivaraam  writes:

> It's been around 4 or 5 issues of 'What's cooking in git.git' and I haven't
> heard about the patches found at,
>
> http://public-inbox.org/git/<20170730111705.12444-1-kaarticsivaraam91...@gmail.com>
>
> and
>
> https://public-inbox.org/git/<20170730115908.13841-1-kaarticsivaraam91...@gmail.com>
>
> What has happened to them?

Nothing has.  Neither thread seems to have any comment from anybody
but you, and I took it as an indication that people do not think it
is a good change.

I do not find the s/branch/parameter/ too bad (although I would have
said "arguments" instead).  

For the other one, I personally think split sentences and lines make
output look worse, but this is obviously subjective (just like the
patch itself).

Re: [PATCH v3 1/2 / RFC] builtin/branch: stop supporting the use of --set-upstream option

[Junio C Hamano]

Kaartic Sivaraam  writes:

> On Wednesday 16 August 2017 12:28 AM, Junio C Hamano wrote:
>> Some refactoring to make it easier to reuse it from the new caller
>> would be necessary. 
> Sorry but I think I don't get that correctly. What's the "new caller"
> being referred to here?
> What should be refactored?

You said that "checkout" does not do a necessary check that is done
in "branch", so presumably "branch" already has a code to do so that
is not called by the current "checkout", right?  Then you would add
a new caller in "checkout" to trigger the same check that is already
done in "branch", but the code "branch" uses _might_ be too specific
to the kind of data the current implementation of "branch" uses and
it _may_ not be easy to call it directly from "checkout" (I didn't
check if that is the case).  If so, then the check implemented in
the current "branch" may need to be refactored before it can easily
be called from the new caller you would be adding to "checkout".

Re: Submodule regression in 2.14?

[Stefan Beller]

On Wed, Aug 16, 2017 at 11:51 AM, Stefan Beller  wrote:

> Any chance the "did not happen with 2.13" was not
> freshly cloned but tested on an existing repo? If so a hot
> candidate for suspicion is a93dcb0a56 (Merge branch
> 'bw/submodule-is-active', 2017-03-30), IMHO, just
> gut feeling, though.

which makes it a feature, I should add.

Re: Submodule regression in 2.14?

[Stefan Beller]

On Wed, Aug 16, 2017 at 11:35 AM, Lars Schneider
 wrote:
> Hi,
>
> I think we discovered a regression in Git 2.14.1 today.
> It looks like as if "git submodule update --init --recursive" removes
> the "skip submodules" config.
>
> Consider the following steps:
>
> git clone https://server/repo.git
> cd repo
> git config --local submodule.some/other/repo.update none
> git submodule update --init --recursive
> git pull --recurse-submodules
>
> With Git 2.14 the last "git pull" will clone the "some/other/repo"
> submodule. This did not happen with Git 2.13.
>
> Bug or feature? I don't have anymore time for Git today. I am happy to
> provide a proper test case tomorrow, though.

$ git log --oneline v2.13.0..v2.14.1 -- git-submodule.sh
532139940c add: warn when adding an embedded repository
(I am confident this is not the suspect, let's keep searching.
Not a lot happened in submodule land apparently)

Looking through all commits v2.13..v2.14 doesn't have me
suspect any of them.

Any chance the "did not happen with 2.13" was not
freshly cloned but tested on an existing repo? If so a hot
candidate for suspicion is a93dcb0a56 (Merge branch
'bw/submodule-is-active', 2017-03-30), IMHO, just
gut feeling, though.

Oh, wait.
$ git log --oneline v2.13.0..v2.14.1 -- builtin/pull.c
c9c63ee558 Merge branch 'sb/pull-rebase-submodule'
a6d7eb2c7a pull: optionally rebase submodules (remote submodule changes only)
could also be a culprit. Do you have pull.rebase set?

>
> Cheers,
> Lars

Re: [PATCH] fix revisions doc about quoting for ':/' notation

[Junio C Hamano]

ryenus  writes:

> To make sure the `` in `:/` is seen as one search string,
> one should quote/escape `` properly.
>
> Especially, the example given in the manual `:/fix nasty bug` does not
> work because of missing quotes. The examples are now corrected, and a
> note about quoting/escaping is added as well.
> ---
>  Documentation/revisions.txt | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
> index 61277469c..fdfdde0ad 100644
> --- a/Documentation/revisions.txt
> +++ b/Documentation/revisions.txt
> @@ -169,14 +169,14 @@ existing tag object.
>and dereference the tag recursively until a non-tag object is
>found.
>
> -'{caret}{/}', e.g. 'HEAD^{/fix nasty bug}'::
> +'{caret}{/}', e.g. 'HEAD^{/"fix nasty bug"}'::

This made me scratch my head, as I rarely read the formatted result
but look at the documentation in the source form.  The original
meant to quote the whole thing inside a single quote, but AsciiDoc
of course will strip that and instead makes the whole thing typeset
in monospace.

What you did is not wrong per-se, but I think quoting the whole
thing, instead of quoting just what is inside the braces, i.e.
'"HEAD^{/fix nasty bug}"' (or if you can manage it, using single
quote instead of double quote) would read better.

>A suffix '{caret}' to a revision parameter, followed by a brace
>pair that contains a text led by a slash,
>is the same as the ':/fix nasty bug' syntax below except that
>it returns the youngest matching commit which is reachable from
>the '' before '{caret}'.
>
> -':/', e.g. ':/fix nasty bug'::
> +':/', e.g. ':/"fix nasty bug"'::

Likewise.

> @@ -185,7 +185,8 @@ existing tag object.
>e.g. ':/^foo'. The special sequence ':/!' is reserved for modifiers to what
>is matched. ':/!-foo' performs a negative match, while ':/!!foo' matches a
>literal '!' character, followed by 'foo'. Any other sequence beginning with
> -  ':/!' is reserved for now.
> +  ':/!' is reserved for now. And make sure to quote/escape for the text to be
> +  seen as one search string.

Good.

Please sign-off your work (cf. Documentation/SubmittingPatches).

Thanks.

Submodule regression in 2.14?

[Lars Schneider]

Hi,

I think we discovered a regression in Git 2.14.1 today.
It looks like as if "git submodule update --init --recursive" removes
the "skip submodules" config.

Consider the following steps:

git clone https://server/repo.git
cd repo
git config --local submodule.some/other/repo.update none
git submodule update --init --recursive
git pull --recurse-submodules

With Git 2.14 the last "git pull" will clone the "some/other/repo"
submodule. This did not happen with Git 2.13.

Bug or feature? I don't have anymore time for Git today. I am happy to
provide a proper test case tomorrow, though.

Cheers,
Lars

[PATCH/FIXUP 6/2] apply: clarify read_old_data() is about no-index case

[Junio C Hamano]

With the previous fixes to CRLF handling in place, read_old_data()
knows what it wants convert_to_git() to do with respect to CRLF.  In
fact, this codepath is about applying a patch to a file in the
filesystem, which may not exist in the index, or may exist but may
not match what is recorded in the index, or in the extreme case, we
may not even be in a Git repository.  If convert_to_git() peeked at
the index while doing its work, it *would* be a bug.

Pass NULL instead of _index to the function to make sure we
catch future bugs to clarify this.

Signed-off-by: Junio C Hamano 
---

 * NOTE NOTE NOTE: This is not a part of the "squashed diff" I sent
   earlier, and with this applied, you will see failure in t0020.

   The breakage is because convert_to_git(), with your [PATCH 1/2],
   is not yet prepared to be told "there is no need for safe-crlf
   processing, so do not even look at the index".  You either need
   to invent yet another flag similar to SAFE_CRLF_KEEP_CRLF that is
   different from SAFE_CRLF_FALSE to tell the machinery never to
   look at the index, or fix SAFE_CRLF_FALSE itself so that the
   index is not checked when the caller knows safe-crlf processing
   is not needed.

 apply.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/apply.c b/apply.c
index c06f7014a2..ad58cd1c77 100644
--- a/apply.c
+++ b/apply.c
@@ -2301,7 +2301,15 @@ static int read_old_data(struct stat *st, const char 
*path, struct strbuf *buf,
case S_IFREG:
if (strbuf_read_file(buf, path, st->st_size) != st->st_size)
return error(_("unable to open or read %s"), path);
-   convert_to_git(_index, path, buf->buf, buf->len, buf, 
safe_crlf);
+   /*
+* "git apply" without "--index/--cached" should never look
+* at the index; the target file may not have been added to
+* the index yet, and we may not even be in any Git repository.
+* Pass NULL to convert_to_git() to stress this; the function
+* should never look at the index when explicit crlf option
+* is given.
+*/
+   convert_to_git(NULL, path, buf->buf, buf->len, buf, safe_crlf);
return 0;
default:
return -1;
-- 
2.14.1-331-g7631d96230

[PATCH/FIXUP 5/2] apply: localize the CRLF business to read_old_data()

[Junio C Hamano]

Previous changes passed a new APPLY_FLAGS_CR_AT_EOL option down from
load_preimage() to read_old_data(), because the last function in
that callchain needs to decide how its call to convert_to_git()
function is made on the data read from the working tree.

The load_preimage() function and its direct callees, however, are
not limited to the case where the patch is applied to the data in
the working tree (i.e. "git apply" that is working as a better
"patch -p1"), unlike read_old_data(), which deals only with the
patch target in the working tree.  They are also responsible for
driving "git apply --cached" and "git apply --index", both of which
take the current index contents into account and do not need the new
special-casing of CRLF.  Exposing APPLY_FLAGS_CR_AT_EOL bit to them
is misleading.

Instead, just pass the "struct patch" down the same callchain, and
have read_old_data() look at its crlf_in_old member to make the
necessary decision.

Signed-off-by: Junio C Hamano 
---

 This is what I care about the most in these fix-ups.

 apply.c | 30 ++
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/apply.c b/apply.c
index 995973da3d..c06f7014a2 100644
--- a/apply.c
+++ b/apply.c
@@ -2287,12 +2287,12 @@ static void show_stats(struct apply_state *state, 
struct patch *patch)
add, pluses, del, minuses);
 }
 
-#define APPLY_FLAGS_CR_AT_EOL   (1<<0)
-
-static int read_old_data(struct stat *st, const char *path, struct strbuf 
*buf, int flags)
+static int read_old_data(struct stat *st, const char *path, struct strbuf *buf,
+struct patch *patch)
 {
-   enum safe_crlf safe_crlf = flags & APPLY_FLAGS_CR_AT_EOL ?
-   SAFE_CRLF_KEEP_CRLF : SAFE_CRLF_FALSE;
+   enum safe_crlf safe_crlf = (patch->crlf_in_old
+   ? SAFE_CRLF_KEEP_CRLF : SAFE_CRLF_FALSE);
+
switch (st->st_mode & S_IFMT) {
case S_IFLNK:
if (strbuf_readlink(buf, path, st->st_size) < 0)
@@ -3407,9 +3407,9 @@ static int load_patch_target(struct apply_state *state,
 struct strbuf *buf,
 const struct cache_entry *ce,
 struct stat *st,
+struct patch *patch,
 const char *name,
-unsigned expected_mode,
-int flags)
+unsigned expected_mode)
 {
if (state->cached || state->check_index) {
if (read_file_or_gitlink(ce, buf))
@@ -3423,7 +3423,7 @@ static int load_patch_target(struct apply_state *state,
} else if (has_symlink_leading_path(name, strlen(name))) {
return error(_("reading from '%s' beyond a symbolic 
link"), name);
} else {
-   if (read_old_data(st, name, buf, flags))
+   if (read_old_data(st, name, buf, patch))
return error(_("failed to read %s"), name);
}
}
@@ -3447,7 +3447,6 @@ static int load_preimage(struct apply_state *state,
char *img;
struct patch *previous;
int status;
-   int flags = patch->crlf_in_old ? APPLY_FLAGS_CR_AT_EOL : 0;
 
previous = previous_patch(state, patch, );
if (status)
@@ -3457,8 +3456,8 @@ static int load_preimage(struct apply_state *state,
/* We have a patched copy in memory; use that. */
strbuf_add(, previous->result, previous->resultsize);
} else {
-   status = load_patch_target(state, , ce, st,
-  patch->old_name, patch->old_mode, 
flags);
+   status = load_patch_target(state, , ce, st, patch,
+  patch->old_name, patch->old_mode);
if (status < 0)
return status;
else if (status == SUBMODULE_PATCH_WITHOUT_INDEX) {
@@ -3518,8 +3517,7 @@ static int three_way_merge(struct image *image,
  */
 static int load_current(struct apply_state *state,
struct image *image,
-   struct patch *patch,
-   int flags)
+   struct patch *patch)
 {
struct strbuf buf = STRBUF_INIT;
int status, pos;
@@ -3546,7 +3544,7 @@ static int load_current(struct apply_state *state,
if (verify_index_match(ce, ))
return error(_("%s: does not match index"), name);
 
-   status = load_patch_target(state, , ce, , name, mode, flags);
+   status = load_patch_target(state, , ce, , patch, name, mode);
if (status < 0)
return status;
else if (status)
@@ -3597,8 +3595,7 @@ static int try_threeway(struct apply_state *state,
 
/* our_oid is ours */
if (patch->is_new) {
-   int 

[PATCH/FIXUP 4/2] apply: only pay attention to CRLF in the preimage

[Junio C Hamano]

The newly added "patch.has_crlf" member wants to indicate if the
incoming patch expects any CRLF line in the patch target, and
parse_fragment() implements that logic for "git apply".

Rename the member to "patch.crlf_in_old" to clarify what it means,
and fix the logic in parse_fragment() so that it also works correctly
when running "git apply -R", where '+' lines correspond to the patch
target.

Signed-off-by: Junio C Hamano 
---

 There also is an obvious style fix for comment, but I didn't bother
 splitting it out to a separate step.

 apply.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/apply.c b/apply.c
index 7663e63df7..995973da3d 100644
--- a/apply.c
+++ b/apply.c
@@ -220,7 +220,7 @@ struct patch {
unsigned int recount:1;
unsigned int conflicted_threeway:1;
unsigned int direct_to_threeway:1;
-   unsigned int has_crlf:1;
+   unsigned int crlf_in_old:1;
struct fragment *fragments;
char *result;
size_t resultsize;
@@ -1663,13 +1663,15 @@ static void check_whitespace(struct apply_state *state,
record_ws_error(state, result, line + 1, len - 2, state->linenr);
 }
 
-/* Check if the patch has context lines with CRLF or
-   the patch wants to remove lines with CRLF */
+/*
+ * Check if the patch has context lines with CRLF or
+ * the patch wants to remove lines with CRLF.
+ */
 static void check_old_for_crlf(struct patch *patch, const char *line, int len)
 {
if (len >= 2 && line[len-1] == '\n' && line[len-2] == '\r') {
patch->ws_rule |= WS_CR_AT_EOL;
-   patch->has_crlf = 1;
+   patch->crlf_in_old = 1;
}
 }
 
@@ -1730,7 +1732,8 @@ static int parse_fragment(struct apply_state *state,
check_whitespace(state, line, len, 
patch->ws_rule);
break;
case '-':
-   check_old_for_crlf(patch, line, len);
+   if (!state->apply_in_reverse)
+   check_old_for_crlf(patch, line, len);
if (state->apply_in_reverse &&
state->ws_error_action != nowarn_ws_error)
check_whitespace(state, line, len, 
patch->ws_rule);
@@ -1739,6 +1742,8 @@ static int parse_fragment(struct apply_state *state,
trailing = 0;
break;
case '+':
+   if (state->apply_in_reverse)
+   check_old_for_crlf(patch, line, len);
if (!state->apply_in_reverse &&
state->ws_error_action != nowarn_ws_error)
check_whitespace(state, line, len, 
patch->ws_rule);
@@ -3442,7 +3447,7 @@ static int load_preimage(struct apply_state *state,
char *img;
struct patch *previous;
int status;
-   int flags = patch->has_crlf ? APPLY_FLAGS_CR_AT_EOL : 0;
+   int flags = patch->crlf_in_old ? APPLY_FLAGS_CR_AT_EOL : 0;
 
previous = previous_patch(state, patch, );
if (status)

Re: [PATCH/RFC 2/2] File commited with CRLF should roundtrip diff and apply

[Junio C Hamano]

I'll be sending a few patches that apply on top of applying these
two patches to show what I meant in my previous review comments.
The net change to apply.c, when you combine your 2/2 with these,
would become like the attached, which I think makes more sense.

Instead of queuing a squashed result, I thought it may help to send
them as incremental fixes with their own justification.

diff --git a/apply.c b/apply.c
index f2d599141d..c06f7014a2 100644
--- a/apply.c
+++ b/apply.c
@@ -220,6 +220,7 @@ struct patch {
unsigned int recount:1;
unsigned int conflicted_threeway:1;
unsigned int direct_to_threeway:1;
+   unsigned int crlf_in_old:1;
struct fragment *fragments;
char *result;
size_t resultsize;
@@ -1662,6 +1663,19 @@ static void check_whitespace(struct apply_state *state,
record_ws_error(state, result, line + 1, len - 2, state->linenr);
 }
 
+/*
+ * Check if the patch has context lines with CRLF or
+ * the patch wants to remove lines with CRLF.
+ */
+static void check_old_for_crlf(struct patch *patch, const char *line, int len)
+{
+   if (len >= 2 && line[len-1] == '\n' && line[len-2] == '\r') {
+   patch->ws_rule |= WS_CR_AT_EOL;
+   patch->crlf_in_old = 1;
+   }
+}
+
+
 /*
  * Parse a unified diff. Note that this really needs to parse each
  * fragment separately, since the only way to know the difference
@@ -1712,11 +1726,14 @@ static int parse_fragment(struct apply_state *state,
if (!deleted && !added)
leading++;
trailing++;
+   check_old_for_crlf(patch, line, len);
if (!state->apply_in_reverse &&
state->ws_error_action == correct_ws_error)
check_whitespace(state, line, len, 
patch->ws_rule);
break;
case '-':
+   if (!state->apply_in_reverse)
+   check_old_for_crlf(patch, line, len);
if (state->apply_in_reverse &&
state->ws_error_action != nowarn_ws_error)
check_whitespace(state, line, len, 
patch->ws_rule);
@@ -1725,6 +1742,8 @@ static int parse_fragment(struct apply_state *state,
trailing = 0;
break;
case '+':
+   if (state->apply_in_reverse)
+   check_old_for_crlf(patch, line, len);
if (!state->apply_in_reverse &&
state->ws_error_action != nowarn_ws_error)
check_whitespace(state, line, len, 
patch->ws_rule);
@@ -2268,8 +2287,12 @@ static void show_stats(struct apply_state *state, struct 
patch *patch)
add, pluses, del, minuses);
 }
 
-static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)
+static int read_old_data(struct stat *st, const char *path, struct strbuf *buf,
+struct patch *patch)
 {
+   enum safe_crlf safe_crlf = (patch->crlf_in_old
+   ? SAFE_CRLF_KEEP_CRLF : SAFE_CRLF_FALSE);
+
switch (st->st_mode & S_IFMT) {
case S_IFLNK:
if (strbuf_readlink(buf, path, st->st_size) < 0)
@@ -2278,7 +2301,7 @@ static int read_old_data(struct stat *st, const char 
*path, struct strbuf *buf)
case S_IFREG:
if (strbuf_read_file(buf, path, st->st_size) != st->st_size)
return error(_("unable to open or read %s"), path);
-   convert_to_git(_index, path, buf->buf, buf->len, buf, 0);
+   convert_to_git(_index, path, buf->buf, buf->len, buf, 
safe_crlf);
return 0;
default:
return -1;
@@ -3384,6 +3407,7 @@ static int load_patch_target(struct apply_state *state,
 struct strbuf *buf,
 const struct cache_entry *ce,
 struct stat *st,
+struct patch *patch,
 const char *name,
 unsigned expected_mode)
 {
@@ -3399,7 +3423,7 @@ static int load_patch_target(struct apply_state *state,
} else if (has_symlink_leading_path(name, strlen(name))) {
return error(_("reading from '%s' beyond a symbolic 
link"), name);
} else {
-   if (read_old_data(st, name, buf))
+   if (read_old_data(st, name, buf, patch))
return error(_("failed to read %s"), name);
}
}
@@ -3432,7 +3456,7 @@ static int load_preimage(struct apply_state *state,
/* We have a patched copy in memory; use that. */
strbuf_add(, previous->result, 

[PATCH/FIXUP 3/2] apply: remove unused member apply_state.flags

[Junio C Hamano]

The previous step added the "flags" member to apply_state, but it is
never used.  Remove it and move the bit assignment macro to apply.c
as that is just a private implementation detail.

Signed-off-by: Junio C Hamano 
---
 apply.c | 2 ++
 apply.h | 4 
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/apply.c b/apply.c
index 63455cd65f..7663e63df7 100644
--- a/apply.c
+++ b/apply.c
@@ -2282,6 +2282,8 @@ static void show_stats(struct apply_state *state, struct 
patch *patch)
add, pluses, del, minuses);
 }
 
+#define APPLY_FLAGS_CR_AT_EOL   (1<<0)
+
 static int read_old_data(struct stat *st, const char *path, struct strbuf 
*buf, int flags)
 {
enum safe_crlf safe_crlf = flags & APPLY_FLAGS_CR_AT_EOL ?
diff --git a/apply.h b/apply.h
index 192140280f..b3d6783d55 100644
--- a/apply.h
+++ b/apply.h
@@ -33,13 +33,9 @@ enum apply_verbosity {
 #define APPLY_SYMLINK_GOES_AWAY 01
 #define APPLY_SYMLINK_IN_RESULT 02
 
-
-#define APPLY_FLAGS_CR_AT_EOL   (1<<0)
-
 struct apply_state {
const char *prefix;
int prefix_length;
-   int flags;
 
/* These are lock_file related */
struct lock_file *lock_file;
-- 
2.14.1-331-g7631d96230

Re: What's cooking in git.git (Aug 2017, #03; Mon, 14)

[Kaartic Sivaraam]

It's been around 4 or 5 issues of 'What's cooking in git.git' and I haven't
heard about the patches found at,

http://public-inbox.org/git/<20170730111705.12444-1-kaarticsivaraam91...@gmail.com>

and

https://public-inbox.org/git/<20170730115908.13841-1-kaarticsivaraam91...@gmail.com>

What has happened to them?

---
Kaartic

Re: [PATCH v3 1/2 / RFC] builtin/branch: stop supporting the use of --set-upstream option

[Kaartic Sivaraam]

On Wednesday 16 August 2017 12:28 AM, Junio C Hamano wrote:
Some refactoring to make it easier to reuse it from the new caller 
would be necessary. 
Sorry but I think I don't get that correctly. What's the "new caller" 
being referred to here?

What should be refactored?

---
Kaartic

[PATCH v2 2/4] commit: replace the raw buffer with strbuf in read_graft_line

[Patryk Obara]

This simplifies function declaration and allows for use of strbuf_rtrim
instead of modifying buffer directly.

Signed-off-by: Patryk Obara 
---
 builtin/blame.c |  2 +-
 commit.c| 11 ++-
 commit.h|  2 +-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index bda1a78..d4472e9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -488,7 +488,7 @@ static int read_ancestry(const char *graft_file)
return -1;
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
+   struct commit_graft *graft = read_graft_line();
if (graft)
register_commit_graft(graft, 0);
}
diff --git a/commit.c b/commit.c
index 8b28415..499fb14 100644
--- a/commit.c
+++ b/commit.c
@@ -134,15 +134,16 @@ int register_commit_graft(struct commit_graft *graft, int 
ignore_dups)
return 0;
 }
 
-struct commit_graft *read_graft_line(char *buf, int len)
+struct commit_graft *read_graft_line(struct strbuf *line)
 {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   int i;
+   int i, len;
+   char *buf = line->buf;
struct commit_graft *graft = NULL;
const int entry_size = GIT_SHA1_HEXSZ + 1;
 
-   while (len && isspace(buf[len-1]))
-   buf[--len] = '\0';
+   strbuf_rtrim(line);
+   len = line->len;
if (buf[0] == '#' || buf[0] == '\0')
return NULL;
if ((len + 1) % entry_size)
@@ -174,7 +175,7 @@ static int read_graft_file(const char *graft_file)
return -1;
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
+   struct commit_graft *graft = read_graft_line();
if (!graft)
continue;
if (register_commit_graft(graft, 1))
diff --git a/commit.h b/commit.h
index 6d857f0..baecc0a 100644
--- a/commit.h
+++ b/commit.h
@@ -247,7 +247,7 @@ struct commit_graft {
 };
 typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
 
-struct commit_graft *read_graft_line(char *buf, int len);
+struct commit_graft *read_graft_line(struct strbuf *line);
 int register_commit_graft(struct commit_graft *, int);
 struct commit_graft *lookup_commit_graft(const struct object_id *oid);
 
-- 
2.9.5

[PATCH v2 4/4] commit: rewrite read_graft_line

[Patryk Obara]

The previous implementation of read_graft_line used calculations based
on GIT_SHA1_RAWSZ and GIT_SHA1_HEXSZ to determine the number of commit
ids in a single graft line.  New implementation does not depend on these
constants, so it adapts to any object_id buffer size.

To make this possible, FLEX_ARRAY of object_id in struct was replaced
by an oid_array.

Code allocating graft now needs to use memset to zero the memory before
use to start with oid_array in a consistent state.

Updates free_graft function implemented in the previous patch to
properly cleanup an oid_array storing parents.

Signed-off-by: Patryk Obara 
---
 commit.c  | 39 +--
 commit.h  |  2 +-
 shallow.c |  1 +
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/commit.c b/commit.c
index 4d23e72..8bdce36 100644
--- a/commit.c
+++ b/commit.c
@@ -111,6 +111,7 @@ static int commit_graft_pos(const unsigned char *sha1)
 
 static void free_commit_graft(struct commit_graft *graft)
 {
+   oid_array_clear(>parents);
free(graft);
 }
 
@@ -139,35 +140,37 @@ int register_commit_graft(struct commit_graft *graft, int 
ignore_dups)
return 0;
 }
 
+static int parse_next_oid_hex(const char *buf, struct object_id *oid, const 
char **end)
+{
+   while (isspace(buf[0]))
+   buf++;
+   return parse_oid_hex(buf, oid, end);
+}
+
 struct commit_graft *read_graft_line(struct strbuf *line)
 {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   int i, len;
-   char *buf = line->buf;
struct commit_graft *graft = NULL;
-   const int entry_size = GIT_SHA1_HEXSZ + 1;
+   struct object_id oid;
+   const char *tail = NULL;
 
strbuf_rtrim(line);
-   len = line->len;
-   if (buf[0] == '#' || buf[0] == '\0')
+   if (line->buf[0] == '#' || line->len == 0)
return NULL;
-   if ((len + 1) % entry_size)
+   graft = xmalloc(sizeof(*graft));
+   memset(graft, 0, sizeof(*graft));
+   if (parse_oid_hex(line->buf, >oid, ))
goto bad_graft_data;
-   i = (len + 1) / entry_size - 1;
-   graft = xmalloc(st_add(sizeof(*graft), st_mult(GIT_SHA1_RAWSZ, i)));
-   graft->nr_parent = i;
-   if (get_oid_hex(buf, >oid))
+   while (!parse_next_oid_hex(tail, , ))
+   oid_array_append(>parents, );
+   if (tail[0] != '\0')
goto bad_graft_data;
-   for (i = GIT_SHA1_HEXSZ; i < len; i += entry_size) {
-   if (buf[i] != ' ')
-   goto bad_graft_data;
-   if (get_sha1_hex(buf + i + 1, graft->parent[i/entry_size].hash))
-   goto bad_graft_data;
-   }
+   graft->nr_parent = graft->parents.nr;
+
return graft;
 
 bad_graft_data:
-   error("bad graft data: %s", buf);
+   error("bad graft data: %s", line->buf);
free_commit_graft(graft);
return NULL;
 }
@@ -363,7 +366,7 @@ int parse_commit_buffer(struct commit *item, const void 
*buffer, unsigned long s
int i;
struct commit *new_parent;
for (i = 0; i < graft->nr_parent; i++) {
-   new_parent = lookup_commit(>parent[i]);
+   new_parent = lookup_commit(>parents.oid[i]);
if (!new_parent)
continue;
pptr = _list_insert(new_parent, pptr)->next;
diff --git a/commit.h b/commit.h
index baecc0a..96ff375 100644
--- a/commit.h
+++ b/commit.h
@@ -243,7 +243,7 @@ void sort_in_topological_order(struct commit_list **, enum 
rev_sort_order);
 struct commit_graft {
struct object_id oid;
int nr_parent; /* < 0 if shallow commit */
-   struct object_id parent[FLEX_ARRAY]; /* more */
+   struct oid_array parents;
 };
 typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
 
diff --git a/shallow.c b/shallow.c
index f5591e5..892cd90 100644
--- a/shallow.c
+++ b/shallow.c
@@ -33,6 +33,7 @@ int register_shallow(const struct object_id *oid)
xmalloc(sizeof(struct commit_graft));
struct commit *commit = lookup_commit(oid);
 
+   memset(graft, 0, sizeof(*graft));
oidcpy(>oid, oid);
graft->nr_parent = -1;
if (commit && commit->object.parsed)
-- 
2.9.5

[PATCH v2 3/4] commit: implement free_commit_graft

[Patryk Obara]

In preparation for new graft struct version introduced in next commit.

Signed-off-by: Patryk Obara 
---
 commit.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/commit.c b/commit.c
index 499fb14..4d23e72 100644
--- a/commit.c
+++ b/commit.c
@@ -109,15 +109,20 @@ static int commit_graft_pos(const unsigned char *sha1)
commit_graft_sha1_access);
 }
 
+static void free_commit_graft(struct commit_graft *graft)
+{
+   free(graft);
+}
+
 int register_commit_graft(struct commit_graft *graft, int ignore_dups)
 {
int pos = commit_graft_pos(graft->oid.hash);
 
if (0 <= pos) {
if (ignore_dups)
-   free(graft);
+   free_commit_graft(graft);
else {
-   free(commit_graft[pos]);
+   free_commit_graft(commit_graft[pos]);
commit_graft[pos] = graft;
}
return 1;
@@ -163,7 +168,7 @@ struct commit_graft *read_graft_line(struct strbuf *line)
 
 bad_graft_data:
error("bad graft data: %s", buf);
-   free(graft);
+   free_commit_graft(graft);
return NULL;
 }
 
-- 
2.9.5

[PATCH v2 1/4] sha1_file: fix hardcoded size in null_sha1

[Patryk Obara]

This prevents compilation error if GIT_MAX_RAWSZ is different than 20.

Signed-off-by: Patryk Obara 
---
 sha1_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sha1_file.c b/sha1_file.c
index b60ae15..f5b5bec 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -32,7 +32,7 @@
 #define SZ_FMT PRIuMAX
 static inline uintmax_t sz_fmt(size_t s) { return s; }
 
-const unsigned char null_sha1[20];
+const unsigned char null_sha1[GIT_MAX_RAWSZ];
 const struct object_id null_oid;
 const struct object_id empty_tree_oid = {
EMPTY_TREE_SHA1_BIN_LITERAL
-- 
2.9.5

[PATCH v2 0/4] Modernize read_graft_line implementation

[Patryk Obara]

Compared to v1:
- the first patch is dropped to make it easier to merge
- free_graft is now static function in commit.c

I don't know, what are exact rules about adding Reviewed-by footer, so
I didn't add any.


Patryk Obara (4):
  sha1_file: fix hardcoded size in null_sha1
  commit: replace the raw buffer with strbuf in read_graft_line
  commit: implement free_commit_graft
  commit: rewrite read_graft_line

 builtin/blame.c |  2 +-
 commit.c| 55 ---
 commit.h|  4 ++--
 sha1_file.c |  2 +-
 shallow.c   |  1 +
 5 files changed, 37 insertions(+), 27 deletions(-)

-- 
2.9.5

Re: git reset

[Junio C Hamano]

Efim Goncharuk  writes:

> Starting from git v.2.13.0 onwards (v 2.12.2 works fine)
>
> git reset --hard with --work-tree and --git-dir options does not move HEAD to 
> hash/tag specified. HEAD remains on same position.
>
> Example:
>
>> git --work-tree=lib/core --git-dir=lib/core/.git/ reset --hard 0.1.0
>
>
> Note:
> on another hand
>> git -C lib/core reset --hard 0.1.0

... "works", I presume.

Thanks for a report.

A quick bisection points at f57f37e2 ("files-backend: remove the use
of git_path()", 2017-03-26), which is not unexpected.  I didn't dig
further and do not expect have time to do so myself for some time,
though.

Re: [PATCH v2] sub-process: print the cmd when a capability is unsupported

[Junio C Hamano]

Ben Peart  writes:

>> -warning("external filter requested unsupported filter 
>> capability '%s'",
>> -p);
>> +warning("subprocess '%s' requested unsupported 
>> capability '%s'",
>> +process->argv[0], p);
>>  }
>>  }
>>   
>>
>
> This one is even cleaner.  Thanks Lars for pointing out the fact we
> already had the cmd name.  Looks good.

Thanks, all.  Will queue.

Re: [PATCH] push: do not add submodule odb as an alternate when recursing on demand

[Heiko Voigt]

Hi,

was about to write that we are maybe overly cautious here. Because the
current way a submodule ends up in the list to be pushed is through:

find_unpushed_submodules()

that itself collects all changed submodules when submodule_needs_pushing() is
true. In there we have this:

if (!submodule_has_commits(path, commits))
/*
 * NOTE: We do consider it safe to return "no" here. The
 * correct answer would be "We do not know" instead of
 * "No push needed", but it is quite hard to change
 * the submodule pointer without having the submodule
 * around. If a user did however change the submodules
 * without having the submodule around, this indicates
 * an expert who knows what they are doing or a
 * maintainer integrating work from other people. In
 * both cases it should be safe to skip this check.
 */
return 0;

So if the check, whether a submodule has commits, fails for any reason it will
not end up in the list to be pushed.

As a side note: inside submodule_has_commits() there is an add_submodule_odb()
followed by a process to really make sure that the commits are in the
submodule.

So IMO at this point we can be sure that the *database* exists and this extra
check could be dropped if we said that a caller to push_submodule() should make
sure that the submodule exists. The current ones are doing it already (if I did
not miss anything).

On Tue, Aug 15, 2017 at 06:05:25PM -0700, Stefan Beller wrote:
> On Tue, Aug 15, 2017 at 5:11 PM, Junio C Hamano  wrote:
> > Stefan Beller  writes:
> >
> >>> Is "is it populated" a good thing to check here, though?  IIRC,
> >>> add-submodule-odb allows you to add the object database of an
> >>> inactivated submodule, so this seems to change the behaviour.  I do
> >>> not know if the behaviour change is a good thing (i.e. bugfix) or
> >>> not (i.e. regression) offhand, though.
> >>
> >> Good point, we should be able to push non-populated, even inactive(?)
> >> submodules. For that we strictly need add_submodule_odb here
> >> (or the repo object of the submodule, eventually).
> >>
> >> So let's retract this patch for now.
> >
> > Not so fast.
> 
> Ok, I took another look at the code.
> 
> While we may desire that un-populated submodules can be pushed
> (due to checking out another revision where the submodule
> doesn't exist, before pushing), this is not supported currently, because
> the call to run the push in the submodule assumes there is a
> "/.git" on which the child process can operate.
> So for now we HAVE to have the submodule populated.

That is a good point though. In the current form of push_submodule() we need to
have a populated submodule. So IMO to check whether the submodule is actually
*populated* instead of adding the odb is correct and a possible bug fix.

Cheers Heiko

Re: [PATCH] sub-process: print the cmd when a capability is unsupported

[Junio C Hamano]

Christian Couder  writes:

>>> I am still wondering if protocol errors should be fatal,
>>
>> Yes, please.
>
> Unfortunately I think it would prevent new filters or new
> sub-processes to work with older versions of Git.
>
> For example if filters are upgraded company wide to support the new
> "delay" capability, that would force everyone using the filters to
> upgrade Git.

I must say that your filter is broken in that case, and it is much
more prudent to die than continuing.  Why is that upgraded filter
asking for "delay" to an older Git that does not yet know it in the
first place?

I just re-read the subprocess_handshake() codepath, and here is my
understand.  The handshake_capabilities() function first advertises
the set of capabilities it supports, so that the other side can pick
and choose which ones to use and ask us to enable in its response.

The code under discussion in this thread comes after that, where we
read the response that tells us what choice the other side made.  If
we saw something that we never advertised, that indicates one of two
things.  The other side, i.e. the "upgraded" filter, is not paying
attention of the capabilities advertisement, and asking something
its correct operation relies on, but we are not capable of giving
that unknown feature and operate without it, so after that point the
exchange of data is a garbage-in-garbage-out.  Or the other side
wanted to ask for one of the capabilities we advertised, but the
code has typo and their wish to enable a capability that its correct
operation relies on is not understood on this end.  The result is
the same garbage-in-garbage-out.

So "program X asked capability C, which we cannot handle" is a good
diagnosis to give and it is a good change to spell out the name of
the offending program that needs to be fixed, but at the same time,
I do not think it is safe to continue without the other side knowing
that one of the capabilities they need to correctly operate cannot
be turned on.

Re: reftable [v7]: new ref storage format

[Shawn Pearce]

On Tue, Aug 15, 2017 at 11:15 PM, Stefan Beller  wrote:
> On Tue, Aug 15, 2017 at 7:48 PM, Shawn Pearce  wrote:
>> 7th iteration of the reftable storage format.
>>
>> You can read a rendered version of this here:
>> https://googlers.googlesource.com/sop/jgit/+/reftable/Documentation/technical/reftable.md
>>
>> Changes from v6:
>> - Blocks are variable sized, and alignment is optional.
>> - ref index is required on variable sized multi-block files.
>>
>> - restart_count/offsets are again at the end of the block.
>> - value_type = 0x3 is only for symbolic references.
>> - "other" files cannot be stored in reftable.
>>
>> - object blocks are explicitly optional.
>> - object blocks use position (offset in bytes), not block id.
>> - removed complex log_chained format for log blocks
>>
>> - Layout uses log, ref file extensions
>> - Described reader algorithm to obtain a snapshot
>
> - back to the old "intra-block index is last"
>   for all block types. ok.

Yes, it simplifies "streaming writers" who don't want to buffer a lot.

> - changed (only ref?) indexes to start char + 3 byte size:
>   Which starting char do object/log indexes have?

All index blocks use 'i'.

> "Unaligned files must include the ref index to support fast lookup."
>
> Why this? I would imagine the client (which has ~5 branches),
> would not need this, but only a ref block, that's it.

The quoted part is I think incomplete. Unaligned files need the ref
index if there is more than one ref block, as there is no way to
divide the space for binary search. A single ref block with 5 branches
does not need the ref index.

> Ctrl-F for 'block_size' reveals nothing is computed
> relative to the block_size in this format, yet we can
> set it to an arbitrary number. If following the spec,
> the reader at $DAY_JOB needs to be able to read
> both aligned and unaligned reftables, despite our plan
> to ever write aligned ref tables, what would the reader
> use the block_size for? (I think we can omit that field
> from the header/footer now, no?)

Its really helpful to be present for the reader to know how to locate
and read blocks. If the ref index is missing and there are multiple
ref blocks in an aligned file, a reader can use block_size to divide
the space and perform binary search. Even when the ref index is
present, the reader can use block_size to issue a disk IO read of
block_size bytes without reading the block_len of the target block
first.

At $DAY_JOB the block_size is tunable by the writer and could change
at any time, so its useful to have it embedded in the output.

Re: [PATCH v2] sub-process: print the cmd when a capability is unsupported

[Lars Schneider]

> On 16 Aug 2017, at 14:40, Christian Couder  wrote:
> 
> In handshake_capabilities() we use warning() when a capability
> is not supported, so the exit code of the function is 0 and no
> further error is shown. This is a problem because the warning
> message doesn't tell us which subprocess cmd failed.
> 
> On the contrary if we cannot write a packet from this function,
> we use error() and then subprocess_start() outputs:
> 
>initialization for subprocess '' failed
> 
> so we can know which subprocess cmd failed.
> 
> Let's improve the warning() message, so that we can know which
> subprocess cmd failed.
> 
> Helped-by: Lars Schneider 
> Signed-off-by: Christian Couder 
> ---
> Change since previous version:
> 
>  - Use process->argv[0] instead of adding a new parameter to
>handshake_capabilities(), thanks to Lars.
> 
> sub-process.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sub-process.c b/sub-process.c
> index 6edb97c1c6..6ccfaaba99 100644
> --- a/sub-process.c
> +++ b/sub-process.c
> @@ -184,8 +184,8 @@ static int handshake_capabilities(struct child_process 
> *process,
>   if (supported_capabilities)
>   *supported_capabilities |= capabilities[i].flag;
>   } else {
> - warning("external filter requested unsupported filter 
> capability '%s'",
> - p);
> + warning("subprocess '%s' requested unsupported 
> capability '%s'",
> + process->argv[0], p);
>   }
>   }
> 
> -- 
> 2.14.1.146.g7de11f915a
> 

Looks good to me.

Thanks,
Lars

Re: [PATCH v2] sub-process: print the cmd when a capability is unsupported

[Ben Peart]

On 8/16/2017 8:40 AM, Christian Couder wrote:

In handshake_capabilities() we use warning() when a capability
is not supported, so the exit code of the function is 0 and no
further error is shown. This is a problem because the warning
message doesn't tell us which subprocess cmd failed.

On the contrary if we cannot write a packet from this function,
we use error() and then subprocess_start() outputs:

 initialization for subprocess '' failed

so we can know which subprocess cmd failed.

Let's improve the warning() message, so that we can know which
subprocess cmd failed.

Helped-by: Lars Schneider 
Signed-off-by: Christian Couder 
---
Change since previous version:

   - Use process->argv[0] instead of adding a new parameter to
 handshake_capabilities(), thanks to Lars.

  sub-process.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sub-process.c b/sub-process.c
index 6edb97c1c6..6ccfaaba99 100644
--- a/sub-process.c
+++ b/sub-process.c
@@ -184,8 +184,8 @@ static int handshake_capabilities(struct child_process 
*process,
if (supported_capabilities)
*supported_capabilities |= capabilities[i].flag;
} else {
-   warning("external filter requested unsupported filter 
capability '%s'",
-   p);
+   warning("subprocess '%s' requested unsupported capability 
'%s'",
+   process->argv[0], p);
}
}
  



This one is even cleaner.  Thanks Lars for pointing out the fact we 
already had the cmd name.  Looks good.

[no subject]

[donellvavas]

Contact me urgently

Re: [PATCH 5/5] commit: rewrite read_graft_line

[Patryk Obara]

Junio C Hamano  wrote:
> I am not sure if this is a good approach.  Just like in 2/5 you can
> use the MAX thing instead of 20, instead of having each graft entry
> allocate a separate oid_array.oid[].

Once MAX values were increased memory corruption was caused exactly by
this line:

> - graft = xmalloc(st_add(sizeof(*graft), st_mult(GIT_SHA1_RAWSZ, i)));

I could've replaced it by:

graft = xmalloc(st_add(sizeof(*graft), st_mult(sizeof(struct
object_id), i)));

But it seemed to me like short-sighted solution (code might be broken if
object_id will be modified again in future).

> Is this because you expect more than one _kind_ of hashes are used
> at the same time?

Yes - I think more than one kind of hashes will be used at the same time
(meaning in single git binary, not necessarily in a single process), at
the very least to allow read access to repositories in "old" format(s), with
sha1-based grafts. Using MAX for parsing here would prevent this,
requiring either modifying grafts code again in future or forcing right now
some unwelcome design decisions down the line.

My goal here was to make this code compatible with whatever hash
function transition plan will be chosen.

-- 
| ← Ceci n'est pas une pipe
Patryk Obara

Re: Bug with ignorecase on Git and Cygwin

[Torsten Bögershausen]

On Wed, Aug 16, 2017 at 11:50:47AM +, CHEVALLIER Yves wrote:
> Hi, 
> 
> On Cygwin, the config value `ignorecase` is set to `true` during `git init` 
> and it is not possible to change the default value using templates. 
> 
> The issue was discovered while I was tracking a bunch of source files of a 
> SDK. To track the changes I simply rm all the working directory, unzip the 
> new SDK, then git add . and git commit -am "new sdk". In this process an 
> issue with an assembly file with preprocessing was incorrectly named .s 
> instead of .S. In the next version the correction was made, but Git was 
> unable to detect it. 
> 
> That said I've tried to manually change ignorecase from true to false and I 
> still have the issue. Git on Cygwin cannot detect files renamed with a case 
> change. 
> 
> Is it a bug?
> 

The thing is that the underlying file system (under cygwin, Windows in general, 
or MacOs)  treats a.s the same as a.S
So Git can not do better than the file system.
What you can do:

git mv a.S a.s
git commit

(This can be done by a script)

> # It works on Ubuntu
[]

[PATCH v2] sub-process: print the cmd when a capability is unsupported

[Christian Couder]

In handshake_capabilities() we use warning() when a capability
is not supported, so the exit code of the function is 0 and no
further error is shown. This is a problem because the warning
message doesn't tell us which subprocess cmd failed.

On the contrary if we cannot write a packet from this function,
we use error() and then subprocess_start() outputs:

initialization for subprocess '' failed

so we can know which subprocess cmd failed.

Let's improve the warning() message, so that we can know which
subprocess cmd failed.

Helped-by: Lars Schneider 
Signed-off-by: Christian Couder 
---
Change since previous version:

  - Use process->argv[0] instead of adding a new parameter to
handshake_capabilities(), thanks to Lars.

 sub-process.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sub-process.c b/sub-process.c
index 6edb97c1c6..6ccfaaba99 100644
--- a/sub-process.c
+++ b/sub-process.c
@@ -184,8 +184,8 @@ static int handshake_capabilities(struct child_process 
*process,
if (supported_capabilities)
*supported_capabilities |= capabilities[i].flag;
} else {
-   warning("external filter requested unsupported filter 
capability '%s'",
-   p);
+   warning("subprocess '%s' requested unsupported 
capability '%s'",
+   process->argv[0], p);
}
}
 
-- 
2.14.1.146.g7de11f915a

Re: [PATCH] sub-process: print the cmd when a capability is unsupported

[Christian Couder]

On Wed, Aug 16, 2017 at 2:22 AM, Jonathan Nieder  wrote:
> Jonathan Tan wrote:
>> Christian Couder  wrote:
>
>>> In handshake_capabilities() we use warning() when a capability
>>> is not supported, so the exit code of the function is 0 and no
>>> further error is shown. This is a problem because the warning
>>> message doesn't tell us which subprocess cmd failed.
> [...]
>>> Let's improve the warning() message, so that we can know which
>>> subprocess cmd failed.
>>>
>>> Signed-off-by: Christian Couder 
>>
>> This looks reasonable to me.
>>
>> I am still wondering if protocol errors should be fatal,
>
> Yes, please.

Unfortunately I think it would prevent new filters or new
sub-processes to work with older versions of Git.

For example if filters are upgraded company wide to support the new
"delay" capability, that would force everyone using the filters to
upgrade Git.

Re: [RFC 0/3] imap-send curl tunnelling support

[Johannes Schindelin]

Hi,

On Tue, 15 Aug 2017, Stefan Beller wrote:

> On Tue, Aug 15, 2017 at 10:49 AM, Nicolas Morey-Chaisemartin
>  wrote:
> > Ping.
> >
> > I'd like to get feedback from Windows developer on patch #2
> > Patch#3 will probably need some updates as I expected Jeff old curl drop 
> > patches to make it in.
> > As it seems to be going another way a few more ifdefs will be required
> 
> +cc Windows devs

I can has easy-to-pull branch, please?

Thanks,
Dscho

Re: [PATCH 4/5] commit: implement free_commit_graft

[Patryk Obara]

Junio C Hamano  wrote:
> I do not see a need to make this new function extern.  Shouldn't it
> be made "static" and revert the change to commit.h?

Ah, of course :) I was anticipating to find free(graft); in more places
throughout code but forgot to get back to it once there was none
left.

I will change it in v2.

-- 
| ← Ceci n'est pas une pipe
Patryk Obara

Re: [PATCH 3/5] commit: replace the raw buffer with strbuf in read_graft_line

[Patryk Obara]

On Tue, Aug 15, 2017 at 7:02 PM, Stefan Beller  wrote:
>> const int entry_size = GIT_SHA1_HEXSZ + 1;
>
> outside the scope of this patch:
> Is GIT_SHA1_HEXSZ or GIT_MAX_HEXSZ the right call here?

I think neither one. In my opinion, this code should not be so closely
coupled to hash parsing code - it should be tasked with parsing
whitespace separated list of commit ids without relying on specific
commit id length or format.

-- 
| ← Ceci n'est pas une pipe
Patryk Obara

git reset

[Efim Goncharuk]

Hello,

Starting from git v.2.13.0 onwards (v 2.12.2 works fine)

git reset --hard with --work-tree and --git-dir options does not move HEAD to 
hash/tag specified. HEAD remains on same position.

Example:

> git --work-tree=lib/core --git-dir=lib/core/.git/ reset --hard 0.1.0


Note:
on another hand
> git -C lib/core reset --hard 0.1.0


Regards,
Efim Goncharuk

Re: [PATCH v2 1/2] clang-format: outline the git project's coding style

[Johannes Schindelin]

Hi Peff,

On Mon, 14 Aug 2017, Jeff King wrote:

> On Mon, Aug 14, 2017 at 03:54:30PM -0700, Brandon Williams wrote:
> 
> > > And removing that gives me a clean output. I have no idea why my clang
> > > doesn't like these (but presumably yours does). It's clang-format-5.0 in
> > > Debian unstable (and clang-format-3.8, etc).
> > 
> > Those must be features in version 6 (which is what I seem to have
> > installed on my machine).
> 
> OK, that makes sense. The most recent one package for Debian unstable is
> 5.0. AFAICT 5.0 is actually in release freeze for another week or two,
> and 6 is just bleeding-edge that moved on after the release freeze a few
> weeks ago.
> 
> I'm not sure which version it makes sense to target as a minimum, but
> probably not 6 yet. :)

I agree.

There is most likely a middle path between too old and too new, and with
the current pace of the review 5.0 will probably be good enough by the
time this patch series can possibly hit `master`. So I'd guess 5.0 would
be a good version to aim for.

Besides, it may not matter *all* that much which version we target: As I
mentioned elsewhere, the contributor experience would most likely be
vastly improved if this was a bot, say, monitoring GitHub Pull Requests.
It could use filter-branch to apply clang-format to each commit's diff and
report back to the Pull Request either by saying that the style is okay,
or by linking to another repository where the fixed commits live (combined
with instructions how to update the local branch).

Ciao,
Dscho

Re: [PATCH 2/5] sha1_file: fix hardcoded size in null_sha1

[Patryk Obara]

Junio C Hamano  wrote:

> I said this is OK for "null" because we assume we will use ^\0{len}$
> for any hash function we choose as the "impossible" value, and for
> that particular use pattern, we do not need such a union.  Just
> letting the caller peek at an appropriate number of bytes at the
> beginning of that NUL buffer for hash the caller wants to use is
> sufficient.

Do you think I should record this explanation as either commit message
or comment in sha1_file.c?

> MAX is inevitable only if we envision that we have to handle objects
> named using two or more hashing schemes at the same time, with the
> same binary and during the same run inside a single process.

I think this will be the case if "transition one local repository at
a time" from Jonathan Nieder's transition plan will be followed.
This plan assumes object_id translation happening e.g. during fetch
operation.

-- 
| ← Ceci n'est pas une pipe
Patryk Obara

Bug with ignorecase on Git and Cygwin

[CHEVALLIER Yves]

Hi, 

On Cygwin, the config value `ignorecase` is set to `true` during `git init` and 
it is not possible to change the default value using templates. 

The issue was discovered while I was tracking a bunch of source files of a SDK. 
To track the changes I simply rm all the working directory, unzip the new SDK, 
then git add . and git commit -am "new sdk". In this process an issue with an 
assembly file with preprocessing was incorrectly named .s instead of .S. In the 
next version the correction was made, but Git was unable to detect it. 

That said I've tried to manually change ignorecase from true to false and I 
still have the issue. Git on Cygwin cannot detect files renamed with a case 
change. 

Is it a bug?

# It works on Ubuntu

$ git --version
git version 1.8.3.1
$ # To be sure 
$ git config --global init.templateDir /usr/share/git-core/templates
$ cat /usr/share/git-core/templates
# Patched config 
[core] 
dummyvar = true 
ignorecase = false

$ git init dummy 
$ cat dummy/.git/config
# Patched config
[core]
ignorecase = false
dummyvar = true
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true

# It doesn't work on Cygwin:

$ git --version
git version 2.13.2
$ which git
/usr/bin/git
$ # To be sure 
$ git config --global init.templateDir /usr/share/git-core/templates
$ cat /usr/share/git-core/templates
# Patched config 
[core] 
dummyvar = true 
ignorecase = false  
$ git init dummy 
$ cat dummy/.git/config
# Patched config
[core]
ignorecase = true
dummyvar = false
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true

[ANNOUNCE] Git Rev News edition 30

[Christian Couder]

Hi everyone,

The 30th edition of Git Rev News is now published:

  https://git.github.io/rev_news/2017/08/16/edition-30/

Thanks a lot to all the contributors and helpers!

Enjoy,
Christian, Thomas, Jakub and Markus.

MR.SALLM SA.LIF

[sallsm salif]

Hi friend I am a banker, I want to transfer an abandoned sum of
USD15.5 Million to your Bank account. 40/percent will be your share.
No risk involved but keep it as secret. Contact me for more details.

Thanks.
MR.SALLM SALIF

Assalamu`Alaikum.

[Mohammad Ouattara]

Greetings from Dr. mohammad ouattara.

Assalamu`Alaikum.

My Name is Dr. mohammad ouattara, I am a banker by profession. I'm
from Ouagadougou, Burkina Faso, West Africa. My reason for contacting
you is to transfer an abandoned $14.6M to your account.

The owner of this fund died since 2004 with his Next Of Kin. I want to
present you to the bank as the Next of Kin/beneficiary of this fund.

Further details of the transaction shall be forwarded to you as soon
as I receive your return mail indicating your interest.

1) YOUR FULL NAME...
(2) YOUR AGE AND SEX
(3) YOUR CONTACT ADDRESS..
(4) YOUR PRIVATE PHONE N0..
(5) FAX NUMBER..
(6) YOUR COUNTRY OF ORIGIN..
(7) YOUR OCCUPATION.

Have a great day,
Dr. mohammad ouattara.

Re: [RFC 0/3] imap-send curl tunnelling support

[Jeff King]

On Tue, Aug 15, 2017 at 07:46:11PM +0200, Nicolas Morey-Chaisemartin wrote:

> Patch#3 will probably need some updates as I expected Jeff old curl drop 
> patches to make it in.
> As it seems to be going another way a few more ifdefs will be required

I'm not sure where we're going with the old-curl versions thing, but I
don't think it matters much either way for imap-send. If we drop support
for anything, it will be versions of curl less than 7.19.4. But curl
didn't get imap support until 7.34.0 (or at least that's what our
Makefile checks for), so I think that's effectively the oldest version
you'd be dealing with here.

(Which I think means your 7.21.5 #ifdef in patch 3 could never trigger).

-Peff

Re: [RFC 0/3] imap-send curl tunnelling support

[Jeff King]

On Wed, Aug 09, 2017 at 04:43:26PM +0200, Nicolas Morey-Chaisemartin wrote:

> I have a few doubt on patch #2:
> - is socketpair working on all git supported system (windows ?)

I'm pretty sure the answer is no, after searching a bit for mingw and
socketpair. The big question is whether we could come up with a suitable
replacement. And that would depend on how libcurl works on Windows, I
think (because it's going to feed whatever we give it to other syscall
wrappers).

> - should socketpair always be used or limited to the curl over tunnel case ?
>   I don't think there is too much different between an unname pipe and a 
> socketpair but I'm not sure either :)

There's not much difference in practice. The obvious one is that
half-duplex shutdowns require shutdown() on a socket and just close() on
the write half of a pipe. I don't know if we do that or not.

I'd be inclined to leave the existing code alone, though, just because
of the risk of regression (and because I don't think the curl and
non-curl versions actually share that much code). But I haven't looked
deeply, so I may be wrong.

> It appears curl do not support the PREAUTH tag.

Too bad. IMHO preauth is the main reason to use a tunnel in the first
place.

-Peff

Re: [PATCH v4 5/8] interpret-trailers: add --parse convenience option

[Jeff King]

On Tue, Aug 15, 2017 at 01:26:53PM +0200, Martin Ågren wrote:

> >  This command reads some patches or commit messages from either the
> > - arguments or the standard input if no  is specified. Then
> > -this command applies the arguments passed using the `--trailer`
> > -option, if any, to the commit message part of each input file. The
> > -result is emitted on the standard output.
> > + arguments or the standard input if no  is specified. If
> > +`--parse` is specified, the output consists of the parsed trailers.
> > +
> > +Otherwise, the this command applies the arguments passed using the
> > +`--trailer` option, if any, to the commit message part of each input
> > +file. The result is emitted on the standard output.
> 
> "the this"

Thanks.

> I think I get why you use --parse above (and in the synopsis), although
> it kind of feels like it should be --only-input or perhaps "--only-input
> (or --parse)".

I really wanted to point people to --parse as the go-to option for the
parsing mode for the sake of simplicity (in fact I initially considered
not even exposing them at all). And I hoped that if they jumped to the
definition of --parse, that would lead them to the other options.

I dunno. I agree it is does not read particularly well. Probably the
whole description section could be rewritten to cover the newly dual
nature of the command a bit better. But I didn't want to disrupt the
existing flow too much.

-Peff

Re: [BUG] git am sometimes unable to apply git format-patch output file

[Soul Trace]

12.08.2017 20:01, Torsten Bögershausen пишет:

On Sat, Aug 12, 2017 at 06:20:23PM +0200, Torsten Bögershausen wrote:

On Sat, Aug 12, 2017 at 07:02:59PM +0300, Soul Trace wrote:

Hello.

Using git i have found that git am command may sometimes fail to apply patch
file which was created by the git am command.


Steps to reproduce:

Excellent, thanks so much for the detailed bug report.
This kind of information is really appreciated.

Why did I say excellent ?
Because I am working on a patch, which -should- fix exactly this kind of issues.
I send out a patch earlier this day, but it doesn't fix your issue, even if it 
should.
I hope to have a fix soonish.

I need to correct mysef, a litte bit, this doesn't seem to be a bug, but a 
feature.
There are 2 things to be noticed:
- The xml file has been commited with CRLF
- git am strips the CR (because they -may- have been added by a mail program)

There are 2 different solutions:
a) Use git am --keep-cr
b) "Normalize" the xml file(s), and commit them to have LF in the repo,
 they can still have CRLF in the work tree, if needed.
 This is done by
 echo "*.xml text" >>.gitattributes
 touch *.xml
 git add *.xml .gitattributes
 git commit -m "Normalize xml files"


If you really need the xml files with CRLF, use this line instead:
 echo "*.xml eol=CRLF" >>.gitattributes

HTH
/Torsten


Thank you.

I will use a) the solution, because I use git am to apply custom patches 
to the Android tree from vendorsetup.sh in my device tree at ". 
build/envsetup.sh" execution and just want git to apply commits what I 
did earlier, when the upstream repositories changes (instead of forking 
the upstream repositories, and then merge the upstream changes to the 
end of the world).


Thanks for your answer, it's really useful. Good luck in patching your 
issue!

Re: git clean -fdx deletes tracked files

[Kim Birkelund]

Apologies. I should obviously have mentioned which OSes the machines I
tested on ran.

One Windows 10 (fully updated) and one Windows Server 2016 (also
updated). I've also seen it in a real repository on our build server
which is Windows Server 2012 R2.

After my first mail I updated git to latest and could still reproduce.



On Aug 15, 2017 21:25, "Kevin Daudt"  wrote:

On Tue, Aug 15, 2017 at 08:45:20PM +0200, Kim Birkelund wrote:
> Hi
>
> I hope this is gonna sound as weird to you as it does to me.
>
> The link below is a zip of a small git repository that I can reproduce
> the bug in on 2 machines.
>
> Repo: https://www.dropbox.com/s/fz4d0i5ko7s7ktr/test.zip?dl=0
>
> It contains 2 folders: helpers and b, each of which is an empty npm
> module. b\package.json refers to the helpers module.
>
> The following reproduces the bug:
>
> 1) in terminal cd to the b folder
> 2) run npm install
> 3) run git reset HEAD --hard
> 4) run git clean -fdx
>
> At this point both files in the helpers folder has been deleted and
> running git status confirms this.
>
> Tool version:
>
> git --version => git version 2.10.2.windows.1
> node -v => v6.11.2
> npm -v => 5.3.0
>
>
> I have no idea what is going. Very much hope you can explain :-)

I cannot reproduce it on linux.

git clean -fdx output:

  Removing node_modules/
  Removing package-lock.json

These are all untracked, and nothing in the helpers dir is being
removed.

Re: reftable [v7]: new ref storage format

[Stefan Beller]

On Tue, Aug 15, 2017 at 7:48 PM, Shawn Pearce  wrote:
> 7th iteration of the reftable storage format.
>
> You can read a rendered version of this here:
> https://googlers.googlesource.com/sop/jgit/+/reftable/Documentation/technical/reftable.md
>
> Changes from v6:
> - Blocks are variable sized, and alignment is optional.
> - ref index is required on variable sized multi-block files.
>
> - restart_count/offsets are again at the end of the block.
> - value_type = 0x3 is only for symbolic references.
> - "other" files cannot be stored in reftable.
>
> - object blocks are explicitly optional.
> - object blocks use position (offset in bytes), not block id.
> - removed complex log_chained format for log blocks
>
> - Layout uses log, ref file extensions
> - Described reader algorithm to obtain a snapshot

- back to the old "intra-block index is last"
  for all block types. ok.
- changed (only ref?) indexes to start char + 3 byte size:
  Which starting char do object/log indexes have?

"Unaligned files must include the ref index to support fast lookup."

Why this? I would imagine the client (which has ~5 branches),
would not need this, but only a ref block, that's it.

Ctrl-F for 'block_size' reveals nothing is computed
relative to the block_size in this format, yet we can
set it to an arbitrary number. If following the spec,
the reader at $DAY_JOB needs to be able to read
both aligned and unaligned reftables, despite our plan
to ever write aligned ref tables, what would the reader
use the block_size for? (I think we can omit that field
from the header/footer now, no?)