Re: [PATCH] push: add remote.pushThin to control thin pack generation

2014-12-10 Thread brian m. carlson
On Wed, Dec 10, 2014 at 11:49:49PM +, brian m. carlson wrote:
 From: brian m. carlson brian.carl...@cpanel.net
 
 Thin packs are enabled when pushing by default, but with a large number
 of refs and a fast network, git may spend more time trying to find a
 good delta than it would spend simply sending data over the network.
 Add a per-remote option, pushThin, that controls the default for pushes
 on that remote.

I just realized this patch doesn't apply outside of the particular repo
I was using.  Consider this more of an RFC, since I'd like to avoid
going this route if possible, in favor of a more robust approach.

 Signed-off-by: brian m. carlson brian.carl...@cpanel.net
 ---
  SOURCES/git/Documentation/config.txt | 6 ++
  SOURCES/git/builtin/push.c   | 6 --
  SOURCES/git/remote.c | 3 +++
  SOURCES/git/remote.h | 1 +
  SOURCES/git/transport.c  | 2 ++
  5 files changed, 16 insertions(+), 2 deletions(-)
 
 diff --git a/SOURCES/git/Documentation/config.txt 
 b/SOURCES/git/Documentation/config.txt
 index 9220725..7fededd 100644
 --- a/SOURCES/git/Documentation/config.txt
 +++ b/SOURCES/git/Documentation/config.txt
 @@ -2178,6 +2178,12 @@ remote.name.push::
   The default set of refspec for linkgit:git-push[1]. See
   linkgit:git-push[1].
  
 +remote.name.pushThin::
 + If true (the default), pass the \--thin option to
 + linkgit:git-pack-objects[1] during push.  This results in a smaller
 + pack being sent and can improve push time over slow networks.  Over
 + fast networks, setting this value to false may improve performance.
 +
  remote.name.mirror::
   If true, pushing to this remote will automatically behave
   as if the `--mirror` option was given on the command line.
 diff --git a/SOURCES/git/builtin/push.c b/SOURCES/git/builtin/push.c
 index a076b19..ae39677 100644
 --- a/SOURCES/git/builtin/push.c
 +++ b/SOURCES/git/builtin/push.c
 @@ -15,7 +15,7 @@ static const char * const push_usage[] = {
   NULL,
  };
  
 -static int thin = 1;
 +static int thin = -1;
  static int deleterefs;
  static const char *receivepack;
  static int verbosity;
 @@ -347,7 +347,9 @@ static int push_with_options(struct transport *transport, 
 int flags)
   if (receivepack)
   transport_set_option(transport,
TRANS_OPT_RECEIVEPACK, receivepack);
 - transport_set_option(transport, TRANS_OPT_THIN, thin ? yes : NULL);
 +
 + if (thin != -1)
 + transport_set_option(transport, TRANS_OPT_THIN, thin ? yes : 
 NULL);
  
   if (!is_empty_cas(cas)) {
   if (!transport-smart_options)
 diff --git a/SOURCES/git/remote.c b/SOURCES/git/remote.c
 index f624217..54777cb 100644
 --- a/SOURCES/git/remote.c
 +++ b/SOURCES/git/remote.c
 @@ -175,6 +175,7 @@ static struct remote *make_remote(const char *name, int 
 len)
  
   ret = xcalloc(1, sizeof(struct remote));
   ret-prune = -1;  /* unspecified */
 + ret-push_thin = 1; /* default on */
   ALLOC_GROW(remotes, remotes_nr + 1, remotes_alloc);
   remotes[remotes_nr++] = ret;
   ret-name = xstrndup(name, len);
 @@ -445,6 +446,8 @@ static int handle_config(const char *key, const char 
 *value, void *cb)
   if (git_config_string(v, key, value))
   return -1;
   add_push_refspec(remote, v);
 + } else if (!strcmp(subkey, .pushthin)) {
 + remote-push_thin = git_config_bool(key, value);
   } else if (!strcmp(subkey, .fetch)) {
   const char *v;
   if (git_config_string(v, key, value))
 diff --git a/SOURCES/git/remote.h b/SOURCES/git/remote.h
 index 8b62efd..badf266 100644
 --- a/SOURCES/git/remote.h
 +++ b/SOURCES/git/remote.h
 @@ -46,6 +46,7 @@ struct remote {
   int skip_default_update;
   int mirror;
   int prune;
 + int push_thin;
  
   const char *receivepack;
   const char *uploadpack;
 diff --git a/SOURCES/git/transport.c b/SOURCES/git/transport.c
 index 70d38e4..2f495fa 100644
 --- a/SOURCES/git/transport.c
 +++ b/SOURCES/git/transport.c
 @@ -987,6 +987,8 @@ struct transport *transport_get(struct remote *remote, 
 const char *url)
   ret-smart_options-receivepack = remote-receivepack;
   }
  
 + transport_set_option(ret, TRANS_OPT_THIN, remote-push_thin ? yes : 
 NULL);
 +
   return ret;
  }
  
 -- 
 2.2.0
 
 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: Poor push performance with large number of refs

2014-12-10 Thread brian m. carlson
On Thu, Dec 11, 2014 at 08:41:07AM +0700, Duy Nguyen wrote:
 It could be a regression by fbd4a70 (list-objects: mark more commits
 as edges in mark_edges_uninteresting - 2013-08-16). That commit makes
 --thin a lot more agressive (reading lots of trees). You can try to
 revert that commit (or use a git version without that commit) and see
 if it improves performance. If so, we probably want to enable that
 code for shallow repos only.

That's exactly it.  With Git 2.2.0, --no-thin was 2.295s, --thin was
8.769s, and --thin with the patch reverted was 3.645s.

I'll come up with a patch.  Thanks for the suggestion.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


[PATCH] list-objects: mark fewer commits as edges for non-shallow clones

2014-12-10 Thread brian m. carlson
In commit fbd4a70 (list-objects: mark more commits as edges in
mark_edges_uninteresting - 2013-08-16), we made --thin much more
aggressive by reading lots more trees.  This produces much smaller packs
for shallow clones; however, it causes a significant performance
regression for non-shallow clones with lots of refs (23.322 seconds vs.
4.785 seconds with 22400 refs).  Limit this extra edge-marking to
shallow clones to avoid poor performance.

Suggested-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 list-objects.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/list-objects.c b/list-objects.c
index 2910bec..274ebb4 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -151,13 +151,14 @@ void mark_edges_uninteresting(struct rev_info *revs, 
show_edge_fn show_edge)
 {
struct commit_list *list;
int i;
+   int shallow = is_repository_shallow();
 
for (list = revs-commits; list; list = list-next) {
struct commit *commit = list-item;
 
if (commit-object.flags  UNINTERESTING) {
mark_tree_uninteresting(commit-tree);
-   if (revs-edge_hint  !(commit-object.flags  SHOWN)) 
{
+   if (shallow  revs-edge_hint  
!(commit-object.flags  SHOWN)) {
commit-object.flags |= SHOWN;
show_edge(commit);
}
@@ -165,6 +166,8 @@ void mark_edges_uninteresting(struct rev_info *revs, 
show_edge_fn show_edge)
}
mark_edge_parents_uninteresting(commit, revs, show_edge);
}
+   if (!shallow)
+   return;
if (revs-edge_hint) {
for (i = 0; i  revs-cmdline.nr; i++) {
struct object *obj = revs-cmdline.rev[i].item;
-- 
2.2.0.rc0.207.ga3a616c

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git-http-backend auth via Kerberos

2014-12-18 Thread brian m. carlson
On Thu, Dec 18, 2014 at 10:19:19PM +, Dan Langille (dalangil) wrote:
 This is what happens without a valid ticket:
 
 $ git clone https://us.example.com/git/clamav-bytecode-compiler
 Cloning into 'clamav-bytecode-compiler'...
 Username for 'https://us.example.com': dan
 Password for 'https://d...@us.example.com': 
 fatal: Authentication failed for 
 'https://us.example.com/git/clamav-bytecode-compiler/'

So there are two ways to do this.  One is allowing users to clone
without any credentials, which I take it you are trying to avoid.  If
that *is* what you're going for, I can provide you with my Apache
configuration, which does allow that.

What I would recommend is going to
https://us.example.com/git/clamav-bytecode-compiler/info/refs in a web
browser without a ticket, and see if it prompts you for a username and
password.  When you get that working, it will probably also work for you
with git.

You can also run git with GIT_CURL_VERBOSE=1 and see the protocol
exchange printed out, so you can see what's happening.  You'll obviously
want to see if the server offers Basic auth as well as Negotiate.

You might also try specifying KrbMethodK5Passwd on explicitly.  I don't
happen to use that option (I use Kerberos to avoid passwords altogether)
so that might work for you.

I don't know what version of git you're using, but some older versions
will still prompt for a password whenever authentication fails.
Therefore, just because you're getting a password prompt doesn't mean
that providing a password will necessarily work.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: git-http-backend auth via Kerberos

2014-12-19 Thread brian m. carlson
On Fri, Dec 19, 2014 at 03:07:20PM +, Dan Langille (dalangil) wrote:
 Correct, we are trying to avoid that.  In addition, there is only https, no 
 http.

I don't think HTTPS versus HTTP matters.  I use Kerberos over HTTPS only
and it works fine.

 To be clear, for the following tests, there is no Kerberos ticket.
 
 I tried that URL using three different browsers.  Each time I am prompted for
 a username  password.  After entering valid credentials, I get:
 
 Chrome: No data received. Unable to load the webpage because the server
 sent no data. Error code: ERR_EMPTY_RESPONSE
 
 With Firefox: The connection was reset
 
 Safari: Safari Can’t Open The Page
 
 However, I did stumble across something which seems promising.
 
 After the above failures, if I then browse to /gitweb/
 (where I see expected results) and then go to the info/refs URL you supplied,
 I see data such as this:
 
fe068a8ae55cea4bb90e2cc714107f4c5389d516   refs/heads/0.96.x
d384a963980e9b40e34dea80eac280cf2d4b7c73   refs/heads/0.97.x
 
 Conclusion: there is something in the /gitweb auth which is succeeding and 
 then
 allowing /git to work

That could possibly be due to KrbSaveCredentials.

 For the record, this is the gitweb auth:
 
 Location /gitweb
   SSLOptions +StdenvVars
   Options +ExecCGI +FollowSymLinks +SymLinksIfOwnerMatch
 
   AuthType   Kerberos
   AuthName   “us.example.com
   KrbAuthRealms  us.example.com
   KrbServiceName HTTP/us.example.com
   Krb5Keytab /usr/local/etc/apache22/repo-test.keytab
   KrbMethodNegotiate on
   KrbMethodk5Passwd  on

Does it work if you set this value (KrbMethodK5Passwd on) explicitly in
the other configuration?  That might be sufficient.

 
 That attempt is shown here: http://dpaste.com/04RG37E.txt
 
  You'll obviously
  want to see if the server offers Basic auth as well as Negotiate.
 
 I’m not up on my knowledge here.  You mean the Kerberos server?

No, I meant the HTTP server, which it looks like from your attempt it
does.  I'm not really sure what the issue is after looking at that,
although it looks like Git isn't sending the username and password.
I'll try to look at this a little more this weekend.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] list-objects: mark fewer commits as edges for non-shallow clones

2014-12-20 Thread brian m. carlson
On Thu, Dec 11, 2014 at 11:26:47AM -0800, Junio C Hamano wrote:
 The right approach would be more like allocating one more bit in
 struct rev_info (call that edge_hint_aggressive), give a new option
 --objects-edge-aggressive, and do something like
 
   if (thin) {
   use_internal_rev_list = 1;
   argv_array_push(rp, is_repository_shallow()
   ? --objects-edge-aggressive
 : --objects-edge);
   }
 
 in this codepath?  I'd actually suggest is_repository_shallow()
 detection to happen one level even higher (i.e. make decision at the
 caller of pack-objects) and decide to pass either --thin or
 --thin-aggressive, so that we can make sure that the damage caused
 by fbd4a70 to be limited only to fetches into shallow repository
 with stronger confidence.

Sorry it's taken me so long to get back to this.  Real life keeps
getting in the way.

I think adding --objects-edge-aggressive is the best way forward here
and then applying the patch above.  If we add --thin-aggressive, we push
the problem to a higher level, which will require more code changes and
make the performance regression continue to affect external remote
helpers, which we don't want.  We know what the right decision is here,
so let's just do it.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] list-objects: mark fewer commits as edges for non-shallow clones

2014-12-20 Thread brian m. carlson
On Thu, Dec 11, 2014 at 05:51:54PM +0700, Duy Nguyen wrote:
 I'm glad it's now working better for you. Out of curiosity, have you
 enabled pack bitmap on this repo? I would expect it to reduce time
 some (or a lot) more, or we would find something to optimize further.

The client performs much, much worse with pack bitmaps.  The server has
bitmaps enabled, but it doesn't have any patches for this issue applied
to it.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


[PATCH v2 2/3] rev-list: add an option to mark fewer edges as uninteresting

2014-12-20 Thread brian m. carlson
In commit fbd4a70 (list-objects: mark more commits as edges in
mark_edges_uninteresting - 2013-08-16), we marked an increasing number
of edges uninteresting.  This change, and the subsequent change to make
this conditional on --objects-edge, are used by --thin to make much
smaller packs for shallow clones.

Unfortunately, they cause a significant performance regression when
pushing non-shallow clones with lots of refs (23.322 seconds vs.
4.785 seconds with 22400 refs).  Add an option to git rev-list,
--objects-edge-aggressive, that preserves this more aggressive behavior,
while leaving --objects-edge to provide more performant behavior.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/git-rev-list.txt | 3 ++-
 Documentation/rev-list-options.txt | 4 
 list-objects.c | 4 ++--
 revision.c | 6 ++
 revision.h | 1 +
 5 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index fd7f8b5..5b11922 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -46,7 +46,8 @@ SYNOPSIS
 [ \--extended-regexp | -E ]
 [ \--fixed-strings | -F ]
 [ \--date=(local|relative|default|iso|iso-strict|rfc|short) ]
-[ [\--objects | \--objects-edge] [ \--unpacked ] ]
+[ [ \--objects | \--objects-edge | \--objects-edge-aggressive ]
+  [ \--unpacked ] ]
 [ \--pretty | \--header ]
 [ \--bisect ]
 [ \--bisect-vars ]
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 2277fcb..8cb6f92 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -657,6 +657,10 @@ These options are mostly targeted for packing of Git 
repositories.
objects in deltified form based on objects contained in these
excluded commits to reduce network traffic.
 
+--objects-edge-aggressive::
+   Similar to `--objects-edge`, but it tries harder to find excluded
+   commits at the cost of increased time.
+
 --unpacked::
Only useful with `--objects`; print the object IDs that are not
in packs.
diff --git a/list-objects.c b/list-objects.c
index 2910bec..2a139b6 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -157,7 +157,7 @@ void mark_edges_uninteresting(struct rev_info *revs, 
show_edge_fn show_edge)
 
if (commit-object.flags  UNINTERESTING) {
mark_tree_uninteresting(commit-tree);
-   if (revs-edge_hint  !(commit-object.flags  SHOWN)) 
{
+   if (revs-edge_hint_aggressive  
!(commit-object.flags  SHOWN)) {
commit-object.flags |= SHOWN;
show_edge(commit);
}
@@ -165,7 +165,7 @@ void mark_edges_uninteresting(struct rev_info *revs, 
show_edge_fn show_edge)
}
mark_edge_parents_uninteresting(commit, revs, show_edge);
}
-   if (revs-edge_hint) {
+   if (revs-edge_hint_aggressive) {
for (i = 0; i  revs-cmdline.nr; i++) {
struct object *obj = revs-cmdline.rev[i].item;
struct commit *commit = (struct commit *)obj;
diff --git a/revision.c b/revision.c
index 75dda92..753dd2f 100644
--- a/revision.c
+++ b/revision.c
@@ -1853,6 +1853,12 @@ static int handle_revision_opt(struct rev_info *revs, 
int argc, const char **arg
revs-tree_objects = 1;
revs-blob_objects = 1;
revs-edge_hint = 1;
+   } else if (!strcmp(arg, --objects-edge-aggressive)) {
+   revs-tag_objects = 1;
+   revs-tree_objects = 1;
+   revs-blob_objects = 1;
+   revs-edge_hint = 1;
+   revs-edge_hint_aggressive = 1;
} else if (!strcmp(arg, --verify-objects)) {
revs-tag_objects = 1;
revs-tree_objects = 1;
diff --git a/revision.h b/revision.h
index 9cb5adc..033a244 100644
--- a/revision.h
+++ b/revision.h
@@ -93,6 +93,7 @@ struct rev_info {
blob_objects:1,
verify_objects:1,
edge_hint:1,
+   edge_hint_aggressive:1,
limited:1,
unpacked:1,
boundary:2,
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/3] Improve push performance with lots of refs

2014-12-20 Thread brian m. carlson
This series contains patches to address a significant push performance
regression in repositories with large amounts of refs.  It avoids
performing expensive edge marking unless the repository is shallow.

The first patch in the series is a fix for a minor typo I discovered
when editing the documentation.  The second patch implements git
rev-list --objects-edge-aggressive, and the final patch ensures it's
used for shallow repos only.  As the final patch was suggested by Junio,
it will need his sign-off.

I considered Junio's suggestion for a --thin-aggressive, but felt that
it was better to have the fix localized to a single location to improve
maintainability and that --thin-aggressive would be uncommonly used
outside of automatic invocations.

Also, as I understand it, the goal of thin packs is to improve
performance by sending fewer objects.  In most cases, the benefit of the
decreased time spent marking edges push times will dwarf the increase in
time it takes for a slightly larger pack to go over the wire, so it
doesn't make sense to make this a tunable.

The original fix was suggested by Duy Nguyen.

brian m. carlson (3):
  Documentation: add missing article in rev-list-options.txt
  rev-list: add an option to mark fewer edges as uninteresting
  pack-objects: use --objects-edge-aggressive only for shallow repos

 Documentation/git-rev-list.txt | 3 ++-
 Documentation/rev-list-options.txt | 7 ++-
 builtin/pack-objects.c | 4 +++-
 list-objects.c | 4 ++--
 revision.c | 6 ++
 revision.h | 1 +
 6 files changed, 20 insertions(+), 5 deletions(-)

-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 3/3] pack-objects: use --objects-edge-aggressive only for shallow repos

2014-12-20 Thread brian m. carlson
Using --objects-edge-aggressive is important for shallow repos, as it
can result in a much smaller pack (in some cases, 10% of the size).
However, it performs poorly on large non-shallow repositories with many
refs.  Since shallow repositories are less likely to have many refs (due
to having less history), the smaller pack size is advantageous there.
Adjust pack-objects to use --objects-edge-aggressive only for shallow
repositories.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/rev-list-options.txt | 3 ++-
 builtin/pack-objects.c | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 8cb6f92..2984f40 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -659,7 +659,8 @@ These options are mostly targeted for packing of Git 
repositories.
 
 --objects-edge-aggressive::
Similar to `--objects-edge`, but it tries harder to find excluded
-   commits at the cost of increased time.
+   commits at the cost of increased time.  This is used instead of
+   `--objects-edge` to build ``thin'' packs for shallow repositories.
 
 --unpacked::
Only useful with `--objects`; print the object IDs that are not
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3f9f5c7..f3ba861 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2711,7 +2711,9 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
argv_array_push(rp, pack-objects);
if (thin) {
use_internal_rev_list = 1;
-   argv_array_push(rp, --objects-edge);
+   argv_array_push(rp, is_repository_shallow()
+   ? --objects-edge-aggressive
+   : --objects-edge);
} else
argv_array_push(rp, --objects);
 
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/3] Documentation: add missing article in rev-list-options.txt

2014-12-20 Thread brian m. carlson
Add the missing article a.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/rev-list-options.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index afccfdc..2277fcb 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -653,7 +653,7 @@ These options are mostly targeted for packing of Git 
repositories.
 --objects-edge::
Similar to `--objects`, but also print the IDs of excluded
commits prefixed with a ``-'' character.  This is used by
-   linkgit:git-pack-objects[1] to build ``thin'' pack, which records
+   linkgit:git-pack-objects[1] to build a ``thin'' pack, which records
objects in deltified form based on objects contained in these
excluded commits to reduce network traffic.
 
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] configure.ac,trace.c: check for CLOCK_MONOTONIC

2014-12-21 Thread brian m. carlson
On Sun, Dec 21, 2014 at 10:53:35AM -0800, Reuben Hawkins wrote:
 CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3
 systems being used in production.  This change makes compiling git
 less tedious on older platforms.

While I'm not opposed to this change, I expect that you'll find there's
lots of subtle breakage when trying to compile (and run the testsuite
for) git on RHEL 3.  I've spoken up before to prevent breakage on
RHEL/CentOS 5 (since $DAYJOB still supports it), but I'm not sure
anyone's looking out for something as old as RHEL 3.  I expect you'll
probably have some pain points with perl and curl, among others.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


[PATCH v3 3/4] pack-objects: use --objects-edge-aggressive only for shallow repos

2014-12-23 Thread brian m. carlson
Using --objects-edge-aggressive is important for shallow repos, as it
can result in a much smaller pack (in some cases, 10% of the size).
However, it performs poorly on large non-shallow repositories with many
refs.  Since shallow repositories are less likely to have many refs (due
to having less history), the smaller pack size is advantageous there.
Adjust pack-objects to use --objects-edge-aggressive only for shallow
repositories.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/rev-list-options.txt | 3 ++-
 builtin/pack-objects.c | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 8cb6f92..2984f40 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -659,7 +659,8 @@ These options are mostly targeted for packing of Git 
repositories.
 
 --objects-edge-aggressive::
Similar to `--objects-edge`, but it tries harder to find excluded
-   commits at the cost of increased time.
+   commits at the cost of increased time.  This is used instead of
+   `--objects-edge` to build ``thin'' packs for shallow repositories.
 
 --unpacked::
Only useful with `--objects`; print the object IDs that are not
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3f9f5c7..f3ba861 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2711,7 +2711,9 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
argv_array_push(rp, pack-objects);
if (thin) {
use_internal_rev_list = 1;
-   argv_array_push(rp, --objects-edge);
+   argv_array_push(rp, is_repository_shallow()
+   ? --objects-edge-aggressive
+   : --objects-edge);
} else
argv_array_push(rp, --objects);
 
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 1/4] Documentation: add missing article in rev-list-options.txt

2014-12-23 Thread brian m. carlson
Add the missing article a.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/rev-list-options.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index afccfdc..2277fcb 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -653,7 +653,7 @@ These options are mostly targeted for packing of Git 
repositories.
 --objects-edge::
Similar to `--objects`, but also print the IDs of excluded
commits prefixed with a ``-'' character.  This is used by
-   linkgit:git-pack-objects[1] to build ``thin'' pack, which records
+   linkgit:git-pack-objects[1] to build a ``thin'' pack, which records
objects in deltified form based on objects contained in these
excluded commits to reduce network traffic.
 
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 0/4] Improve push performance with lots of refs

2014-12-23 Thread brian m. carlson
This series contains patches to address a significant push performance
regression in repositories with large amounts of refs.  It avoids
performing expensive edge marking unless the repository is shallow.

The first patch in the series is a fix for a minor typo I discovered
when editing the documentation.  The second patch implements git
rev-list --objects-edge-aggressive, and the third patch ensures it's
used for shallow repos only.  The final patch ensures that sending packs
to shallow clients use --object-edge-aggressive as well.

The only change from v2 is the addition of a fourth patch, which fixes
t5500.  It's necessary because the test wants packs for fetches to
shallow clones to be minimal.

I'm not especially thrilled with having to provide a --shallow command
line argument, but the alternative is to buffer a potentially large
amount of data in order to determine whether the remote side is shallow.

The original fix was suggested by Duy Nguyen.

brian m. carlson (4):
  Documentation: add missing article in rev-list-options.txt
  rev-list: add an option to mark fewer edges as uninteresting
  pack-objects: use --objects-edge-aggressive only for shallow repos
  upload-pack: use --objects-edge-aggressive for shallow fetches

 Documentation/git-pack-objects.txt | 7 ++-
 Documentation/git-rev-list.txt | 3 ++-
 Documentation/rev-list-options.txt | 7 ++-
 builtin/pack-objects.c | 7 ++-
 list-objects.c | 4 ++--
 revision.c | 6 ++
 revision.h | 1 +
 upload-pack.c  | 4 +++-
 8 files changed, 32 insertions(+), 7 deletions(-)

-- 
2.2.1.209.g41e5f3a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 2/4] rev-list: add an option to mark fewer edges as uninteresting

2014-12-23 Thread brian m. carlson
In commit fbd4a70 (list-objects: mark more commits as edges in
mark_edges_uninteresting - 2013-08-16), we marked an increasing number
of edges uninteresting.  This change, and the subsequent change to make
this conditional on --objects-edge, are used by --thin to make much
smaller packs for shallow clones.

Unfortunately, they cause a significant performance regression when
pushing non-shallow clones with lots of refs (23.322 seconds vs.
4.785 seconds with 22400 refs).  Add an option to git rev-list,
--objects-edge-aggressive, that preserves this more aggressive behavior,
while leaving --objects-edge to provide more performant behavior.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/git-rev-list.txt | 3 ++-
 Documentation/rev-list-options.txt | 4 
 list-objects.c | 4 ++--
 revision.c | 6 ++
 revision.h | 1 +
 5 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index fd7f8b5..5b11922 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -46,7 +46,8 @@ SYNOPSIS
 [ \--extended-regexp | -E ]
 [ \--fixed-strings | -F ]
 [ \--date=(local|relative|default|iso|iso-strict|rfc|short) ]
-[ [\--objects | \--objects-edge] [ \--unpacked ] ]
+[ [ \--objects | \--objects-edge | \--objects-edge-aggressive ]
+  [ \--unpacked ] ]
 [ \--pretty | \--header ]
 [ \--bisect ]
 [ \--bisect-vars ]
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 2277fcb..8cb6f92 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -657,6 +657,10 @@ These options are mostly targeted for packing of Git 
repositories.
objects in deltified form based on objects contained in these
excluded commits to reduce network traffic.
 
+--objects-edge-aggressive::
+   Similar to `--objects-edge`, but it tries harder to find excluded
+   commits at the cost of increased time.
+
 --unpacked::
Only useful with `--objects`; print the object IDs that are not
in packs.
diff --git a/list-objects.c b/list-objects.c
index 2910bec..2a139b6 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -157,7 +157,7 @@ void mark_edges_uninteresting(struct rev_info *revs, 
show_edge_fn show_edge)
 
if (commit-object.flags  UNINTERESTING) {
mark_tree_uninteresting(commit-tree);
-   if (revs-edge_hint  !(commit-object.flags  SHOWN)) 
{
+   if (revs-edge_hint_aggressive  
!(commit-object.flags  SHOWN)) {
commit-object.flags |= SHOWN;
show_edge(commit);
}
@@ -165,7 +165,7 @@ void mark_edges_uninteresting(struct rev_info *revs, 
show_edge_fn show_edge)
}
mark_edge_parents_uninteresting(commit, revs, show_edge);
}
-   if (revs-edge_hint) {
+   if (revs-edge_hint_aggressive) {
for (i = 0; i  revs-cmdline.nr; i++) {
struct object *obj = revs-cmdline.rev[i].item;
struct commit *commit = (struct commit *)obj;
diff --git a/revision.c b/revision.c
index 75dda92..753dd2f 100644
--- a/revision.c
+++ b/revision.c
@@ -1853,6 +1853,12 @@ static int handle_revision_opt(struct rev_info *revs, 
int argc, const char **arg
revs-tree_objects = 1;
revs-blob_objects = 1;
revs-edge_hint = 1;
+   } else if (!strcmp(arg, --objects-edge-aggressive)) {
+   revs-tag_objects = 1;
+   revs-tree_objects = 1;
+   revs-blob_objects = 1;
+   revs-edge_hint = 1;
+   revs-edge_hint_aggressive = 1;
} else if (!strcmp(arg, --verify-objects)) {
revs-tag_objects = 1;
revs-tree_objects = 1;
diff --git a/revision.h b/revision.h
index 9cb5adc..033a244 100644
--- a/revision.h
+++ b/revision.h
@@ -93,6 +93,7 @@ struct rev_info {
blob_objects:1,
verify_objects:1,
edge_hint:1,
+   edge_hint_aggressive:1,
limited:1,
unpacked:1,
boundary:2,
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 4/4] upload-pack: use --objects-edge-aggressive for shallow fetches

2014-12-23 Thread brian m. carlson
When fetching into a shallow repository, we want to aggressively mark
edges as uninteresting, since this decreases the pack size.  However,
is_shallow_repository() returns false on the server side, since the
server is not shallow.

As the server, we get an indication of whether the client is shallow
based on stdin; however, by the time we get to parsing stdin, we already
will have called setup_revisions and won't be able to change our
decision anymore.  Teach pack-objects a --shallow option to indicate
that the remote side is shallow and use it in upload-pack.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/git-pack-objects.txt | 7 ++-
 builtin/pack-objects.c | 5 -
 upload-pack.c  | 4 +++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-pack-objects.txt 
b/Documentation/git-pack-objects.txt
index d2d8f47..c2f76fb 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -13,7 +13,7 @@ SYNOPSIS
[--no-reuse-delta] [--delta-base-offset] [--non-empty]
[--local] [--incremental] [--window=n] [--depth=n]
[--revs [--unpacked | --all]] [--stdout | base-name]
-   [--keep-true-parents]  object-list
+   [--shallow] [--keep-true-parents]  object-list
 
 
 DESCRIPTION
@@ -190,6 +190,11 @@ required objects and is thus unusable by Git without 
making it
 self-contained. Use `git index-pack --fix-thin`
 (see linkgit:git-index-pack[1]) to restore the self-contained property.
 
+--shallow::
+   Optimize a pack that will be provided to a client with a shallow
+   repository.  This option, combined with \--thin, can result in a
+   smaller pack at the cost of speed.
+
 --delta-base-offset::
A packed archive can express the base object of a delta as
either a 20-byte object name or as an offset in the
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f3ba861..6bfbd3d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2613,6 +2613,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
 {
int use_internal_rev_list = 0;
int thin = 0;
+   int shallow = 0;
int all_progress_implied = 0;
struct argv_array rp = ARGV_ARRAY_INIT;
int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;
@@ -2677,6 +2678,8 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
  PARSE_OPT_OPTARG, option_parse_unpack_unreachable },
OPT_BOOL(0, thin, thin,
 N_(create thin packs)),
+   OPT_BOOL(0, shallow, shallow,
+N_(create packs suitable for shallow fetches)),
OPT_BOOL(0, honor-pack-keep, ignore_packed_keep,
 N_(ignore packs that have companion .keep file)),
OPT_INTEGER(0, compression, pack_compression_level,
@@ -2711,7 +2714,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
argv_array_push(rp, pack-objects);
if (thin) {
use_internal_rev_list = 1;
-   argv_array_push(rp, is_repository_shallow()
+   argv_array_push(rp, is_repository_shallow() || shallow
? --objects-edge-aggressive
: --objects-edge);
} else
diff --git a/upload-pack.c b/upload-pack.c
index ac9ac15..b531a32 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -86,7 +86,7 @@ static void create_pack_file(void)
corruption on the remote side.;
int buffered = -1;
ssize_t sz;
-   const char *argv[12];
+   const char *argv[13];
int i, arg = 0;
FILE *pipe_fd;
 
@@ -100,6 +100,8 @@ static void create_pack_file(void)
argv[arg++] = --thin;
 
argv[arg++] = --stdout;
+   if (shallow_nr)
+   argv[arg++] = --shallow;
if (!no_progress)
argv[arg++] = --progress;
if (use_ofs_delta)
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/4] rev-list: add an option to mark fewer edges as uninteresting

2014-12-24 Thread brian m. carlson
On Tue, Dec 23, 2014 at 01:51:42PM -0500, Jeff King wrote:
 On Tue, Dec 23, 2014 at 12:55:48PM -0500, Michael Blume wrote:
 
  This patch causes an error on my mac, test 5500 fetch-pack errors on
  part 44 - fetch creating new shallow root. It looks for remote: Total
  1 in the fetch output and gets 3 instead.
 
 It fails for me on Linux, too. Interestingly the tip of the series
 passes. I didn't investigate further.

Yes, the tip of the series fixes that issue.  I had trouble coming up
with a good technique to split the patches in a way such that they were
logically distinct yet addressed the failure.  I had an epiphany on how
to do that last night, so I'll reroll sometime today.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v3 0/4] Improve push performance with lots of refs

2014-12-24 Thread brian m. carlson
On Tue, Dec 23, 2014 at 10:40:54AM -0800, Junio C Hamano wrote:
 You spell --thin-aggressive as two words, --thin --shallow, in
 this series, essentially, no?

Essentially, yes.  It became obligatory after I noticed the test
failure, since that test actually checks whether the remote side sends
a shallow-optimized pack.

 I think this is going in the right direction.  The shallow
 propagated on the wire from the fetcher is the right thing to use
 to make this decision.
 
 I wonder if the call to is_repository_shallow() is still necessary
 (read: I would prefer to see it go away) where we decide between
 --objects-edge and --objects-edge-aggressive.

Okay.  I'll try to push it up the stack a little bit.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


[PATCH v4 0/3] Improve push performance with lots of refs

2014-12-24 Thread brian m. carlson
This series contains patches to address a significant push performance
regression in repositories with large amounts of refs.  It avoids
performing expensive edge marking unless the repository is shallow.

The first patch in the series is a fix for a minor typo I discovered
when editing the documentation.  The second patch implements git
rev-list --objects-edge-aggressive, and the third patch ensures it's
used for pushing to and fetching from shallow repos only.

The changes from v3 are to use --objects-edge-aggressive from the point
it's introduced (this preserves bisectability) and to make higher-level
commands pass --shallow for any shallow pushing and fetching instead of
trying to have pack-objects determine it.

The original fix was suggested by Duy Nguyen.

brian m. carlson (3):
  Documentation: add missing article in rev-list-options.txt
  rev-list: add an option to mark fewer edges as uninteresting
  pack-objects: use --objects-edge-aggressive for shallow repos

 Documentation/git-pack-objects.txt | 7 ++-
 Documentation/git-rev-list.txt | 3 ++-
 Documentation/rev-list-options.txt | 7 ++-
 builtin/pack-objects.c | 7 ++-
 list-objects.c | 4 ++--
 revision.c | 6 ++
 revision.h | 1 +
 send-pack.c| 3 +++
 upload-pack.c  | 4 +++-
 9 files changed, 35 insertions(+), 7 deletions(-)

-- 
2.2.1.209.g41e5f3a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 2/3] rev-list: add an option to mark fewer edges as uninteresting

2014-12-24 Thread brian m. carlson
In commit fbd4a70 (list-objects: mark more commits as edges in
mark_edges_uninteresting - 2013-08-16), we marked an increasing number
of edges uninteresting.  This change, and the subsequent change to make
this conditional on --objects-edge, are used by --thin to make much
smaller packs for shallow clones.

Unfortunately, they cause a significant performance regression when
pushing non-shallow clones with lots of refs (23.322 seconds vs.
4.785 seconds with 22400 refs).  Add an option to git rev-list,
--objects-edge-aggressive, that preserves this more aggressive behavior,
while leaving --objects-edge to provide more performant behavior.
Preserve the current behavior for the moment by using the aggressive
option.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/git-rev-list.txt | 3 ++-
 Documentation/rev-list-options.txt | 4 
 builtin/pack-objects.c | 2 +-
 list-objects.c | 4 ++--
 revision.c | 6 ++
 revision.h | 1 +
 6 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index fd7f8b5..5b11922 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -46,7 +46,8 @@ SYNOPSIS
 [ \--extended-regexp | -E ]
 [ \--fixed-strings | -F ]
 [ \--date=(local|relative|default|iso|iso-strict|rfc|short) ]
-[ [\--objects | \--objects-edge] [ \--unpacked ] ]
+[ [ \--objects | \--objects-edge | \--objects-edge-aggressive ]
+  [ \--unpacked ] ]
 [ \--pretty | \--header ]
 [ \--bisect ]
 [ \--bisect-vars ]
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 2277fcb..8cb6f92 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -657,6 +657,10 @@ These options are mostly targeted for packing of Git 
repositories.
objects in deltified form based on objects contained in these
excluded commits to reduce network traffic.
 
+--objects-edge-aggressive::
+   Similar to `--objects-edge`, but it tries harder to find excluded
+   commits at the cost of increased time.
+
 --unpacked::
Only useful with `--objects`; print the object IDs that are not
in packs.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3f9f5c7..f93a17c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2711,7 +2711,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
argv_array_push(rp, pack-objects);
if (thin) {
use_internal_rev_list = 1;
-   argv_array_push(rp, --objects-edge);
+   argv_array_push(rp, --objects-edge-aggressive);
} else
argv_array_push(rp, --objects);
 
diff --git a/list-objects.c b/list-objects.c
index 2910bec..2a139b6 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -157,7 +157,7 @@ void mark_edges_uninteresting(struct rev_info *revs, 
show_edge_fn show_edge)
 
if (commit-object.flags  UNINTERESTING) {
mark_tree_uninteresting(commit-tree);
-   if (revs-edge_hint  !(commit-object.flags  SHOWN)) 
{
+   if (revs-edge_hint_aggressive  
!(commit-object.flags  SHOWN)) {
commit-object.flags |= SHOWN;
show_edge(commit);
}
@@ -165,7 +165,7 @@ void mark_edges_uninteresting(struct rev_info *revs, 
show_edge_fn show_edge)
}
mark_edge_parents_uninteresting(commit, revs, show_edge);
}
-   if (revs-edge_hint) {
+   if (revs-edge_hint_aggressive) {
for (i = 0; i  revs-cmdline.nr; i++) {
struct object *obj = revs-cmdline.rev[i].item;
struct commit *commit = (struct commit *)obj;
diff --git a/revision.c b/revision.c
index 75dda92..753dd2f 100644
--- a/revision.c
+++ b/revision.c
@@ -1853,6 +1853,12 @@ static int handle_revision_opt(struct rev_info *revs, 
int argc, const char **arg
revs-tree_objects = 1;
revs-blob_objects = 1;
revs-edge_hint = 1;
+   } else if (!strcmp(arg, --objects-edge-aggressive)) {
+   revs-tag_objects = 1;
+   revs-tree_objects = 1;
+   revs-blob_objects = 1;
+   revs-edge_hint = 1;
+   revs-edge_hint_aggressive = 1;
} else if (!strcmp(arg, --verify-objects)) {
revs-tag_objects = 1;
revs-tree_objects = 1;
diff --git a/revision.h b/revision.h
index 9cb5adc..033a244 100644
--- a/revision.h
+++ b/revision.h
@@ -93,6 +93,7 @@ struct rev_info {
blob_objects:1,
verify_objects:1

[PATCH v4 3/3] pack-objects: use --objects-edge-aggressive for shallow repos

2014-12-24 Thread brian m. carlson
When fetching into or pushing from a shallow repository, we want to
aggressively mark edges as uninteresting, since this decreases the pack
size.  However, aggressively marking edges can negatively affect
performance on large non-shallow repositories with lots of refs.

Teach pack-objects a --shallow option to indicate that we're pushing
from or fetching into a shallow repository.  Use
--objects-edge-aggressive only for shallow repositories and otherwise
use --objects-edge, which performs better in the general case.  Update
the callers to pass the --shallow option when they are dealing with a
shallow repository.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/git-pack-objects.txt | 7 ++-
 Documentation/rev-list-options.txt | 3 ++-
 builtin/pack-objects.c | 7 ++-
 send-pack.c| 3 +++
 upload-pack.c  | 4 +++-
 5 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-pack-objects.txt 
b/Documentation/git-pack-objects.txt
index d2d8f47..c2f76fb 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -13,7 +13,7 @@ SYNOPSIS
[--no-reuse-delta] [--delta-base-offset] [--non-empty]
[--local] [--incremental] [--window=n] [--depth=n]
[--revs [--unpacked | --all]] [--stdout | base-name]
-   [--keep-true-parents]  object-list
+   [--shallow] [--keep-true-parents]  object-list
 
 
 DESCRIPTION
@@ -190,6 +190,11 @@ required objects and is thus unusable by Git without 
making it
 self-contained. Use `git index-pack --fix-thin`
 (see linkgit:git-index-pack[1]) to restore the self-contained property.
 
+--shallow::
+   Optimize a pack that will be provided to a client with a shallow
+   repository.  This option, combined with \--thin, can result in a
+   smaller pack at the cost of speed.
+
 --delta-base-offset::
A packed archive can express the base object of a delta as
either a 20-byte object name or as an offset in the
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 8cb6f92..2984f40 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -659,7 +659,8 @@ These options are mostly targeted for packing of Git 
repositories.
 
 --objects-edge-aggressive::
Similar to `--objects-edge`, but it tries harder to find excluded
-   commits at the cost of increased time.
+   commits at the cost of increased time.  This is used instead of
+   `--objects-edge` to build ``thin'' packs for shallow repositories.
 
 --unpacked::
Only useful with `--objects`; print the object IDs that are not
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f93a17c..d816587 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2613,6 +2613,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
 {
int use_internal_rev_list = 0;
int thin = 0;
+   int shallow = 0;
int all_progress_implied = 0;
struct argv_array rp = ARGV_ARRAY_INIT;
int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;
@@ -2677,6 +2678,8 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
  PARSE_OPT_OPTARG, option_parse_unpack_unreachable },
OPT_BOOL(0, thin, thin,
 N_(create thin packs)),
+   OPT_BOOL(0, shallow, shallow,
+N_(create packs suitable for shallow fetches)),
OPT_BOOL(0, honor-pack-keep, ignore_packed_keep,
 N_(ignore packs that have companion .keep file)),
OPT_INTEGER(0, compression, pack_compression_level,
@@ -2711,7 +2714,9 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
argv_array_push(rp, pack-objects);
if (thin) {
use_internal_rev_list = 1;
-   argv_array_push(rp, --objects-edge-aggressive);
+   argv_array_push(rp, shallow
+   ? --objects-edge-aggressive
+   : --objects-edge);
} else
argv_array_push(rp, --objects);
 
diff --git a/send-pack.c b/send-pack.c
index 949cb61..25947d7 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -47,6 +47,7 @@ static int pack_objects(int fd, struct ref *refs, struct 
sha1_array *extra, stru
NULL,
NULL,
NULL,
+   NULL,
};
struct child_process po = CHILD_PROCESS_INIT;
int i;
@@ -60,6 +61,8 @@ static int pack_objects(int fd, struct ref *refs, struct 
sha1_array *extra, stru
argv[i++] = -q;
if (args-progress)
argv[i++] = --progress;
+   if (is_repository_shallow())
+   argv[i++] = --shallow;
po.argv = argv;
po.in = -1;
po.out = args

[PATCH v4 1/3] Documentation: add missing article in rev-list-options.txt

2014-12-24 Thread brian m. carlson
Add the missing article a.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/rev-list-options.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index afccfdc..2277fcb 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -653,7 +653,7 @@ These options are mostly targeted for packing of Git 
repositories.
 --objects-edge::
Similar to `--objects`, but also print the IDs of excluded
commits prefixed with a ``-'' character.  This is used by
-   linkgit:git-pack-objects[1] to build ``thin'' pack, which records
+   linkgit:git-pack-objects[1] to build a ``thin'' pack, which records
objects in deltified form based on objects contained in these
excluded commits to reduce network traffic.
 
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] remote-curl: fall back to Basic auth if Negotiate fails.

2014-12-26 Thread brian m. carlson
Apache servers using mod_auth_kerb can be configured to allow the user
to authenticate either using Negotiate (using the Kerberos ticket) or
Basic authentication (using the Kerberos password).  Often, one will
want to use Negotiate authentication if it is available, but fall back
to Basic authentication if the ticket is missing or expired.

Teach the HTTP client code to stop trying authentication mechanisms that
don't use a password (currently Negotiate) after the first failure,
since if they failed the first time, they will never succeed.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
I was able to reproduce the problem on my server.  This fixes the
problem for me both when info/refs requires authentication and when it
does not.  Dan, please try and see if this fixes the problem for you.

I'm not clear on whether NTLM is a passwordless authentication method.
Since I don't use Windows or NTLM, I can't test it, but if it is, just
adding it to HTTP_AUTH_PASSWORDLESS should be sufficient.

 http.c| 14 ++
 http.h|  5 -
 remote-curl.c | 13 -
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/http.c b/http.c
index 040f362..e3e4c65 100644
--- a/http.c
+++ b/http.c
@@ -986,6 +986,16 @@ static void extract_content_type(struct strbuf *raw, 
struct strbuf *type,
strbuf_addstr(charset, ISO-8859-1);
 }
 
+void disable_passwordless_auth(struct active_request_slot *slot)
+{
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+#define HTTP_AUTH_PASSWORDLESS (CURLAUTH_GSSNEGOTIATE)
+   curl_easy_setopt(slot-curl, CURLOPT_HTTPAUTH,
+CURLAUTH_ANY  ~HTTP_AUTH_PASSWORDLESS);
+#endif
+}
+
+
 /* http_request() targets */
 #define HTTP_REQUEST_STRBUF0
 #define HTTP_REQUEST_FILE  1
@@ -1035,6 +1045,9 @@ static int http_request(const char *url,
curl_easy_setopt(slot-curl, CURLOPT_HTTPHEADER, headers);
curl_easy_setopt(slot-curl, CURLOPT_ENCODING, gzip);
 
+   if (options-no_passwordless_auth)
+   disable_passwordless_auth(slot);
+
ret = run_one_slot(slot, results);
 
if (options  options-content_type) {
@@ -1139,6 +1152,7 @@ static int http_request_reauth(const char *url,
}
 
credential_fill(http_auth);
+   options-no_passwordless_auth = 1;
 
return http_request(url, result, target, options);
 }
diff --git a/http.h b/http.h
index 473179b..fc42bf5 100644
--- a/http.h
+++ b/http.h
@@ -98,6 +98,8 @@ extern int handle_curl_result(struct slot_results *results);
 int run_one_slot(struct active_request_slot *slot,
 struct slot_results *results);
 
+void disable_passwordless_auth(struct active_request_slot *slot);
+
 #ifdef USE_CURL_MULTI
 extern void fill_active_slots(void);
 extern void add_fill_function(void *data, int (*fill)(void *));
@@ -138,7 +140,8 @@ extern char *get_remote_object_url(const char *url, const 
char *hex,
 /* Options for http_get_*() */
 struct http_get_options {
unsigned no_cache:1,
-keep_error:1;
+keep_error:1,
+no_passwordless_auth:1;
 
/* If non-NULL, returns the content-type of the response. */
struct strbuf *content_type;
diff --git a/remote-curl.c b/remote-curl.c
index dd63bc2..89bf4ea 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -369,6 +369,8 @@ struct rpc_state {
struct strbuf result;
unsigned gzip_request : 1;
unsigned initial_buffer : 1;
+   /* Automatic authentication didn't work, so don't try it again. */
+   unsigned no_passwordless_auth : 1;
 };
 
 static size_t rpc_out(void *ptr, size_t eltsize,
@@ -467,6 +469,9 @@ static int probe_rpc(struct rpc_state *rpc, struct 
slot_results *results)
curl_easy_setopt(slot-curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
curl_easy_setopt(slot-curl, CURLOPT_FILE, buf);
 
+   if (rpc-no_passwordless_auth)
+   disable_passwordless_auth(slot);
+
err = run_slot(slot, results);
 
curl_slist_free_all(headers);
@@ -510,8 +515,10 @@ static int post_rpc(struct rpc_state *rpc)
 
do {
err = probe_rpc(rpc, results);
-   if (err == HTTP_REAUTH)
+   if (err == HTTP_REAUTH) {
credential_fill(http_auth);
+   rpc-no_passwordless_auth = 1;
+   }
} while (err == HTTP_REAUTH);
if (err != HTTP_OK)
return -1;
@@ -533,6 +540,9 @@ retry:
curl_easy_setopt(slot-curl, CURLOPT_URL, rpc-service_url);
curl_easy_setopt(slot-curl, CURLOPT_ENCODING, gzip);
 
+   if (rpc-no_passwordless_auth)
+   disable_passwordless_auth(slot);
+
if (large_request) {
/* The request body is large and the size cannot be predicted.
 * We must use chunked encoding to send

Re: [PATCH] remote-curl: fall back to Basic auth if Negotiate fails.

2014-12-27 Thread brian m. carlson
On Sat, Dec 27, 2014 at 12:56:04PM -0500, Jeff King wrote:
 On Sat, Dec 27, 2014 at 04:01:33AM +, brian m. carlson wrote:
 
  Apache servers using mod_auth_kerb can be configured to allow the user
  to authenticate either using Negotiate (using the Kerberos ticket) or
  Basic authentication (using the Kerberos password).  Often, one will
  want to use Negotiate authentication if it is available, but fall back
  to Basic authentication if the ticket is missing or expired.
  
  Teach the HTTP client code to stop trying authentication mechanisms that
  don't use a password (currently Negotiate) after the first failure,
  since if they failed the first time, they will never succeed.
  
  Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
  ---
  I was able to reproduce the problem on my server.  This fixes the
  problem for me both when info/refs requires authentication and when it
  does not.  Dan, please try and see if this fixes the problem for you.
  
  I'm not clear on whether NTLM is a passwordless authentication method.
  Since I don't use Windows or NTLM, I can't test it, but if it is, just
  adding it to HTTP_AUTH_PASSWORDLESS should be sufficient.
 
 I don't think this should make things any worse for NTLM if it is. It
 would just not get the benefit of the feature you are adding, and
 somebody with a working setup can test and add it at that time, right?

Correct.

 I'm not familiar enough with Negotiate auth to do give a thorough review
 on the logic above. But FWIW, it makes sense to me, and the code looks
 correct.

libcurl will try very hard to use something other than Basic auth, even
over HTTPS.  If Basic and something else are offered, libcurl will never
use Basic.  I should probably make a note of that in the commit message.

 The credential struct is already a global for all requests. If you made
 the no_passwordless flag similarly global, it would be enough to set
 it in handle_curl_result and respect it in get_curl_handle.

I'll reroll with that change.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] remote-curl: fall back to Basic auth if Negotiate fails.

2014-12-27 Thread brian m. carlson
On Sat, Dec 27, 2014 at 04:29:49PM -0500, Jeff King wrote:
  since if they failed the first time, they will never succeed
 
 Are there other GSSAPI methods where this is not the case? I don't know
 of any, and AFAICT git's support is used only for Kerberos, so this is
 probably safe for now. If somebody can produce a concrete case that
 behaves differently, we can untangle it then.

GSSAPI defines a token-based interface.  While it's possible that a user
could have multiple tokens, I don't think libcurl knows how to select
different ones, so TTBOMK it will always present the same token and
hence always get the same response.

libcurl could theoretically be linked against any shared library
implementing the GSSAPI version 2.  I only know if it being used in
Kerberos and NTLM in practice.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: bearer token authorization with HTTPS transport

2014-12-30 Thread brian m. carlson
On Tue, Dec 30, 2014 at 11:24:09AM -0800, David Renshaw wrote:
 Hi,
 I would like to be able to serve a git repository over HTTPS from a
 web server that requires OAuth2-style bearer tokens for authorization.
 For more context, see this thread:
 https://groups.google.com/forum/#!topic/sandstorm-dev/4oigfb4-9E4
 
 Does anyone here have any advice about how to convince a git client to
 add an Authorization: Bearer token header?
 
 I can think of a few approaches:
 
 (1) I could modify the curl remote helper to insert the header if it
 sees a bearertoken config option. I have in fact written a
 proof-of-concept patch that does this (see
 https://github.com/dwrensha/git/commit/4da7b64b85b3b6652abe7), but I
 don't know how much of chance something like this has of getting
 merged into the mainline git client.

An idea that might be interesting is to add a framework to select a set
of authentication types (defaulting, of course, to any).  As part of
that, you could add a type, bearer, that uses the password we've
collected via the credential helper as the bearer token.

I think that has the best chance of getting merged designwise.  I've
CC'd Peff, as he has touched the area a lot and might have other
suggestions.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


[PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails

2015-01-01 Thread brian m. carlson
Apache servers using mod_auth_kerb can be configured to allow the user
to authenticate either using Negotiate (using the Kerberos ticket) or
Basic authentication (using the Kerberos password).  Often, one will
want to use Negotiate authentication if it is available, but fall back
to Basic authentication if the ticket is missing or expired.

However, libcurl will try very hard to use something other than Basic
auth, even over HTTPS.  If Basic and something else are offered, libcurl
will never attempt to use Basic, even if the other option fails.
Teach the HTTP client code to stop trying authentication mechanisms that
don't use a password (currently Negotiate) after the first failure,
since if they failed the first time, they will never succeed.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 http.c| 16 
 http.h|  3 +++
 remote-curl.c | 11 ++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/http.c b/http.c
index 040f362..815194d 100644
--- a/http.c
+++ b/http.c
@@ -62,6 +62,8 @@ static const char *user_agent;
 
 static struct credential cert_auth = CREDENTIAL_INIT;
 static int ssl_cert_password_required;
+/* Should we allow non-password-based authentication (e.g. GSSAPI)? */
+int http_passwordless_auth = 1;
 
 static struct curl_slist *pragma_header;
 static struct curl_slist *no_pragma_header;
@@ -986,6 +988,16 @@ static void extract_content_type(struct strbuf *raw, 
struct strbuf *type,
strbuf_addstr(charset, ISO-8859-1);
 }
 
+void disable_passwordless_auth(struct active_request_slot *slot)
+{
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+#define HTTP_AUTH_PASSWORDLESS (CURLAUTH_GSSNEGOTIATE)
+   curl_easy_setopt(slot-curl, CURLOPT_HTTPAUTH,
+CURLAUTH_ANY  ~HTTP_AUTH_PASSWORDLESS);
+#endif
+}
+
+
 /* http_request() targets */
 #define HTTP_REQUEST_STRBUF0
 #define HTTP_REQUEST_FILE  1
@@ -1035,6 +1047,9 @@ static int http_request(const char *url,
curl_easy_setopt(slot-curl, CURLOPT_HTTPHEADER, headers);
curl_easy_setopt(slot-curl, CURLOPT_ENCODING, gzip);
 
+   if (!http_passwordless_auth)
+   disable_passwordless_auth(slot);
+
ret = run_one_slot(slot, results);
 
if (options  options-content_type) {
@@ -1139,6 +1154,7 @@ static int http_request_reauth(const char *url,
}
 
credential_fill(http_auth);
+   http_passwordless_auth = 0;
 
return http_request(url, result, target, options);
 }
diff --git a/http.h b/http.h
index 473179b..71943d3 100644
--- a/http.h
+++ b/http.h
@@ -98,6 +98,8 @@ extern int handle_curl_result(struct slot_results *results);
 int run_one_slot(struct active_request_slot *slot,
 struct slot_results *results);
 
+void disable_passwordless_auth(struct active_request_slot *slot);
+
 #ifdef USE_CURL_MULTI
 extern void fill_active_slots(void);
 extern void add_fill_function(void *data, int (*fill)(void *));
@@ -112,6 +114,7 @@ extern int active_requests;
 extern int http_is_verbose;
 extern size_t http_post_buffer;
 extern struct credential http_auth;
+extern int http_passwordless_auth;
 
 extern char curl_errorstr[CURL_ERROR_SIZE];
 
diff --git a/remote-curl.c b/remote-curl.c
index dd63bc2..4ca5447 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -467,6 +467,9 @@ static int probe_rpc(struct rpc_state *rpc, struct 
slot_results *results)
curl_easy_setopt(slot-curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
curl_easy_setopt(slot-curl, CURLOPT_FILE, buf);
 
+   if (!http_passwordless_auth)
+   disable_passwordless_auth(slot);
+
err = run_slot(slot, results);
 
curl_slist_free_all(headers);
@@ -510,8 +513,10 @@ static int post_rpc(struct rpc_state *rpc)
 
do {
err = probe_rpc(rpc, results);
-   if (err == HTTP_REAUTH)
+   if (err == HTTP_REAUTH) {
credential_fill(http_auth);
+   http_passwordless_auth = 0;
+   }
} while (err == HTTP_REAUTH);
if (err != HTTP_OK)
return -1;
@@ -533,6 +538,9 @@ retry:
curl_easy_setopt(slot-curl, CURLOPT_URL, rpc-service_url);
curl_easy_setopt(slot-curl, CURLOPT_ENCODING, gzip);
 
+   if (!http_passwordless_auth)
+   disable_passwordless_auth(slot);
+
if (large_request) {
/* The request body is large and the size cannot be predicted.
 * We must use chunked encoding to send it.
@@ -617,6 +625,7 @@ retry:
err = run_slot(slot, NULL);
if (err == HTTP_REAUTH  !large_request) {
credential_fill(http_auth);
+   http_passwordless_auth = 0;
goto retry;
}
if (err != HTTP_OK)
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git

Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails

2015-01-03 Thread brian m. carlson

On Sat, Jan 03, 2015 at 06:19:23AM -0500, Jeff King wrote:

This pattern gets repeated in several places. Now that
http_passwordless_auth is a global, can we handle it automatically for
the callers, as below (which, aside from compiling, is completely
untested by me)?


This looks good (although I haven't tested it).


Note that this is in a slightly different boat than credential_fill.
Ideally we would also handle picking up credentials on behalf of the
callers of get_curl_handle/handle_curl_result. But that may involve
significant work and/or prompting the user, which we _must_ avoid if we
do not know if we are going to retry the request (and only the caller
knows that for sure). However, in the case of http_passwordless_auth, we
are just setting a flag, so it's OK to do it preemptively.


Right.  We already prompt the user for a username and password in that 
case, so we already have the credentials that we need.



diff --git a/http.c b/http.c
index 040f362..2bbcdf1 100644
--- a/http.c
+++ b/http.c
@@ -62,6 +62,8 @@ static const char *user_agent;

static struct credential cert_auth = CREDENTIAL_INIT;
static int ssl_cert_password_required;
+/* Should we allow non-password-based authentication (e.g. GSSAPI)? */
+static int http_passwordless_auth = 1;

static struct curl_slist *pragma_header;
static struct curl_slist *no_pragma_header;
@@ -318,7 +320,12 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
#endif
#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-   curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
+   {
+   int flags = CURLAUTH_ANY;


I think this needs to be unsigned long or it can cause undefined 
behavior, since libcurl uses unsigned long in the flags.  I'll fix that 
up when I reroll.  I'll need your sign-off since it will essentially be 
your work.



+   if (!http_passwordless_auth)
+   flags = ~CURLAUTH_GSSNEGOTIATE;
+   curl_easy_setopt(result, CURLOPT_HTTPAUTH, flags);
+   }
#endif

if (http_proactive_auth)
@@ -870,6 +877,7 @@ int handle_curl_result(struct slot_results *results)
credential_reject(http_auth);
return HTTP_NOAUTH;
} else {
+   http_passwordless_auth = 0;
return HTTP_REAUTH;
}
} else {


Note that you could probably drop http_passwordless_auth completely, and
just keep a:

 static int http_auth_methods = CURLAUTH_ANY;

and then drop CURLAUTH_GSSNEGOTIATE from it instead of setting the
passwordless_auth flag to 0 (again, it happens in one place, so I don't
know that it needs an extra layer of abstraction).


This does seem to handle both cases well.  It also has the pleasant 
side effect of being static.

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] bash completion: allow git stash store options completion

2015-01-14 Thread brian m. carlson

On Tue, Jan 13, 2015 at 10:40:40AM -0800, Junio C Hamano wrote:

Alexander Kuleshov kuleshovm...@gmail.com writes:


This patch adds bash completion for git stash 'store' subcommand
which apperead at bd514cad (stash: introduce 'git stash store', 18 Jun 2013)

Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---


Hmph.  The create and store subcommands are not end-user facing;
they are meant to be used in scripts.  I am not sure if we want to
complete them in the first place.  I know create already is in the
list of completion candidates, but I wonder if adding store is
making things worse.


For what it's worth, I'll often sketch out a script at the command line 
before putting it in a file (either because I realize I'll need to do 
the same thing again or I want to share it with others), so I think this 
might be a useful addition.

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses

2015-01-22 Thread brian m. carlson

On Thu, Jan 22, 2015 at 11:05:29PM +0100, Torsten Bögershausen wrote:

We want to support ssh://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/
  because e.g. the Git shipped with Debian (1.7.10.4) (and a lot of other 
installations) supports it.


I understand that this used to work, but it probably shouldn't have ever 
been accepted.  It's nonstandard, and if we accept it for ssh, people 
will want it to work for https, and due to libcurl, it simply won't.


I prefer to see our past acceptance of this format as a bug.  This is 
the first that I've heard of anyone noticing this (since 2013), so it 
can't be in common usage.


If we accept it, we should explicitly document it as being deprecated and 
note that it's inconsistent with the way everything else works.



We want to support ssh://bmc@[2001:470:1f05:79::1]/git/bmc/homedir.git/
   because that is what other people may expect to work as well:
ssh://bmc@[2001:470:1f05:79::1]:/git/bmc/homedir.git/


Everyone expects this to work properly, because it's the standard URL 
form (RFC 2732).  I agree we should support it.



 git push 2001:470:1f05:79::1:1 master
when they mean

 git push [2001:470:1f05:79::1]:1 master

That I don't understand this, where is the path name in your example ?


The path in question is $HOME/1.  That's why the bracket notation is 
obligatory in the short form.  I agree it's a bit bizarre.



Everything after the first ':' is the path in the short form:
bmc@hostxx:/git/bmc/homedir.git/

If you really want to use a literal IPV6 with the short form, you must use the 
brackets:
bmc@[2001:470:1f05:79::1]:/git/bmc/homedir.git/
(And you can not have a port number here)


Right.  In my experience, nobody uses the ssh:// form unless they have 
to (i.e. they need to use a port number); it's extremely uncommon.  So 
they've already become used to using the bracketed notation, because 
it's already required for the usual form and it's required in the IPv6 
URL standard.

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses

2015-01-22 Thread brian m. carlson

On Mon, Jan 19, 2015 at 06:21:24PM +0100, Torsten Bögershausen wrote:

When parsing an URL, older Git versions did handle
URLs like ssh://2001:db8::1/repo.git the same way as
ssh://[2001:db8::1]/repo.git

Commit 83b058 broke the parsing of IPV6 adresses without []:
It was written in mind that the fist ':' in a URL was the beginning of a
port number, while the old code was more clever:
Literal IPV6 addresses have always at least two ':'.
When the hostandport had a ':' and the rest of the hostandport string
could be parsed into an integer between 0 and 65536, then it was used
as a port number, like host:22.
Otherwise the first ':' was assumed to be part of a literal IPV6 address,
like ssh://[2001:db8::1]/repo.git or ssh://[::1]/repo.git.

Re-introduce the old parsing in get_host_and_port().

Improve the parsing to handle URLs which have a user name and a literal
IPV6 like ssh://user@[2001:db8::1]/repo.git.
(Thanks to Christian Taube li...@hcf.yourweb.de for reporting this long
standing issue)

Another regression was introduced in 83b058:
A non-RFC conform URL like [localhost:222] could be used in older versions
of Git to connect to run ssh -p 222 localhost.
The new stricter parsing did not allow this any more.
See even 8d3d28f5dba why [localhost:222] should be declared a feature.


I'm not sure this is a very good idea.  While this may have worked in 
the past, it's also completely inconsistent with the way all non-SSH 
URLs work in Git:


 vauxhall ok % git push https://bmc@2001:470:1f05:79::1/git/bmc/homedir.git 
master
 fatal: unable to access 
'https://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/': IPv6 numerical address 
used in URL without brackets

 vauxhall no % git push 
https://bmc@[castro.crustytoothpaste.net]/git/bmc/homedir.git master
 fatal: unable to access 
'https://bmc@[castro.crustytoothpaste.net]/git/bmc/homedir.git/': Could not 
resolve host: [castro.crustytoothpaste.net]

 vauxhall no % git push 
https://bmc@[castro.crustytoothpaste.net:443]/git/bmc/homedir.git master
 fatal: unable to access 
'https://bmc@[castro.crustytoothpaste.net:443]/git/bmc/homedir.git/': Could not 
resolve host: [castro.crustytoothpaste.net

I would argue that people using IPv6 literals in URLs should already 
know how to do it correctly, and the consistency between SSH and HTTPS 
URLs is a feature, not a bug.  In the non-URL SSH form, you still have 
to use the bracketed form to deal with the case in which somebody 
creates a directory called 1 in their home directory and writes:


 git push 2001:470:1f05:79::1:1 master 


when they mean

 git push [2001:470:1f05:79::1]:1 master
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH 24/24] git-status.txt: advertisement for untracked cache

2015-01-22 Thread brian m. carlson

On Tue, Jan 20, 2015 at 08:03:33PM +0700, Nguyễn Thái Ngọc Duy wrote:

When a good user sees the too long, consider -uno advice when
running `git status`, they should check out the man page to find out
more. This change suggests they try untracked cache before -uno.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/git-status.txt | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index def635f..7850f53 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -58,7 +58,10 @@ When `-u` option is not used, untracked files and 
directories are
shown (i.e. the same as specifying `normal`), to help you avoid
forgetting to add newly created files.  Because it takes extra work
to find untracked files in the filesystem, this mode may take some
-time in a large working tree.  You can use `no` to have `git status`
+time in a large working tree.
+Consider to enable untracked cache and split index if supported (see


This might sound more natural (at least to my ears) if you wrote 
Consider enabling untracked cache….

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] Documentation: fix version numbering

2015-01-22 Thread brian m. carlson

On Thu, Jan 22, 2015 at 07:32:33PM +, Sven van Haastregt wrote:

Version numbers in asciidoc-generated content (such as man pages)
went missing as of da8a366 (Documentation: refactor common operations
into variables).  Fix by putting the underscore back in the variable
name.

Signed-off-by: Sven van Haastregt sve...@gmail.com
---
Documentation/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2f6b6aa..3e39e28 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -103,7 +103,7 @@ ASCIIDOC_HTML = xhtml11
ASCIIDOC_DOCBOOK = docbook
ASCIIDOC_CONF = -f asciidoc.conf
ASCIIDOC_COMMON = $(ASCIIDOC) $(ASCIIDOC_EXTRA) $(ASCIIDOC_CONF) \
-   -agit-version=$(GIT_VERSION)
+   -agit_version=$(GIT_VERSION)


Yes, this does seem to fix it.  My apologies for the bug, and thanks for 
noticing.

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails

2015-02-19 Thread brian m. carlson
On Wed, Feb 18, 2015 at 04:17:46PM +, Dan Langille (dalangil) wrote:
 I just built from ‘master’, on FreeBSD 9.3:
 
 cd ~/src
 git clone https://github.com/git/git.git
 cd git
 gmake
 
 Then tried ~/src/git/git clone https://OUR_REPO
 
  It cores too, and I see: git-remote-https.core

Can you compile with debugging symbols and provide a backtrace?  I'm not 
seeing any such behavior on my end, and I'm not sure whether it's my 
patch or something else that might be present in master.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses

2015-02-19 Thread brian m. carlson

On Thu, Feb 19, 2015 at 09:54:52AM -0800, Junio C Hamano wrote:

I can see that you do not agree with the If we accept it part
(where it refers to allowing [...] was a bug.)---past acceptance
was not a bug for you.

Brian is for that If we accept it, and sees it as a bug.

So let's see what he comes up with as a follow-up to the we should
explicitly document it part.


Here's what I propose:

-- 8 --
Subject: [PATCH] Documentation: note deprecated syntax for IPv6 SSH URLs

We have historically accepted some invalid syntax for SSH URLs
containing IPv6 literals.  Older versions of Git accepted URLs missing
the brackets required by RFC 2732.  Note that this behavior is
deprecated and that other protocol handlers will not accept this syntax.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
Documentation/urls.txt | 4 
1 file changed, 4 insertions(+)

diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index 9ccb246..2c1a84f 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -38,6 +38,10 @@ The ssh and git protocols additionally support ~username 
expansion:
- git://host.xz{startsb}:port{endsb}/~{startsb}user{endsb}/path/to/repo.git/
- {startsb}user@{endsb}host.xz:/~{startsb}user{endsb}/path/to/repo.git/

+For backwards compatibility reasons, Git, when using ssh URLs, accepts
+some URLs containing IPv6 literals that are missing the brackets. This
+syntax is deprecated, and other protocol handlers do not permit this.
+
For local repositories, also supported by Git natively, the following
syntaxes may be used:

--
2.2.1.209.g41e5f3a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: odb_mkstemp's 0444 permission broke write/delete access on AFP

2015-02-19 Thread brian m. carlson
On Tue, Feb 17, 2015 at 09:51:38AM +0100, Matthieu Moy wrote:
 This should be fixable from Git itself, by replacing the calls to
 unlink with something like
 
 int unlink_or_chmod(...) {
   if (unlink(...)) {
   chmod(...); // give user write permission
   return unlink(...);
   }
 }
 
 This does not add extra cost in the normal case, and would fix this
 particular issue for afp shares. So, I think that would fix the biggest
 problem for afp-share users without disturbing others. It seems
 reasonable to me to do that unconditionnally.

This can have security issues if you're trying to unlink a symlink, as 
chmod will dereference the symlink but unlink will not.  Giving the file 
owner write permission may not be sufficient, as the user may be a 
member of a group with write access to the repo.  A malicious user who 
also has access to the repo could force the current user to chmod an 
arbitrary file such that it had looser permissions.

I've seen a case where Perl's ExtUtils::MakeMaker chmoded 
/etc/mime.types 0666 as a result of this.

I don't think there's a secure way to implement this unless you're on an 
OS with lchmod or fchmodat that supports AT_SYMLINK_NOFOLLOW.  Linux is 
not one of those systems.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'

2015-01-30 Thread brian m. carlson

On Fri, Jan 30, 2015 at 07:24:45AM +0100, Tom G. Christensen wrote:

The '--no-xmailer' option is a Getopt::Long boolean option. The
'--no-' prefix (as in --no-xmailer) for boolean options is not
supported in Getopt::Long version 2.32 which was released with Perl 5.8.0.
This version only supports '--no' as in '--noxmailer'.  More recent
versions of Getopt::Long, such as version 2.34, support either prefix. So
use the older form in the tests.

See also:

d2559f734bba7fe5257720356a92f3b7a5b0d37c
907a0b1e04ea31cb368e9422df93d8ebb0187914
84eeb687de7a6c7c42af3fb51b176e0f412a979e
3fee1fe87144360a1913eab86af9ad136c810076

Signed-off-by: Tom G. Christensen t...@statsbiblioteket.dk
---
t/t9001-send-email.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index af6a3e8..30df6ae 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1580,20 +1580,20 @@ do_xmailer_test () {

test_expect_success $PREREQ '--[no-]xmailer without any configuration' '
do_xmailer_test 1 --xmailer 
-   do_xmailer_test 0 --no-xmailer
+   do_xmailer_test 0 --noxmailer


I don't think this is an adequate fix.  The documented option is 
--no-xmailer.  If your version of Getopt::Long is not capable of that, 
then the program doesn't work as documented, and the test is correctly 
failing.  --noxmailer is not documented at all, so it's not something we 
should be testing.


We should probably require a certain version of Getopt::Long or 
explicitly handle this in the parsing code itself.  I think the former 
is a better choice, since no security-supported OS still ships with such 
a positively ancient version.

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails

2015-01-05 Thread brian m. carlson

On Mon, Jan 05, 2015 at 09:23:32PM +, Dan Langille (dalangil) wrote:
I have tried both patches.  Neither succeeds here.  I patched git 
version 2.2.1 but I don’t think that affects this.


You are patching the client side, correct?  That's the side that needs 
patching here.


Just so the list knows, I will be sending a reroll to the existing 
patch, but the patches I've posted do indeed work in my testing.


Before I flood the list with debug runs, I wanted to make sure I was 
testing with an appropriate configuration:


Location /git
SSLOptions +StdenvVars
Options +ExecCGI +FollowSymLinks +SymLinksIfOwnerMatch

  # By default, allow access to anyone.
  Order allow,deny
  Allow from All

  # Enable Kerberos authentication using mod_auth_kerb.
 AuthType   Kerberos
 AuthName   “us.example.org
 KrbAuthRealms  us.example.org
 # I have tried both with and without the following line:
 KrbServiceName HTTP/us.example.org
 Krb5Keytab /usr/local/etc/apache22/repo-test.keytab
  KrbMethodNegotiate on
  KrbSaveCredentials on
  KrbVerifyKDC on
  KrbServiceName Any
 # I have tried with and without this line:
 KrbMethodk5Passwd  on
  Require valid-user
/Location


I'm not sure why it's not working for you.  Here's a snippet from my 
config:


 SetEnv GIT_HTTP_EXPORT_ALL 1
 SetEnv REMOTE_USER $REDIRECT_REMOTE_USER
 AuthType Kerberos
 AuthName Kerberos Login
 KrbMethodNegotiate on
 KrbMethodK5Passwd off
 KrbAuthRealms CRUSTYTOOTHPASTE.NET
 Krb5Keytab /etc/krb5.apache.keytab

When I was testing, I set KrbMethodK5Passwd to on and it did in fact 
work.  I'm using Debian's Apache 2.4.10-9 with mod_auth_kerb 5.4-2.2.

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: bearer token authorization with HTTPS transport

2015-01-05 Thread brian m. carlson

On Tue, Dec 30, 2014 at 08:42:10PM -0500, Jeff King wrote:

Another option would be to just teach the credential code to accept a
bearer field from a credential helper. We would need to:

 1. Teach the credential code that getting a bearer token is
sufficient (it does not need to prompt for username/password).

 2. Teach the http code to pull the bearer field out and stick it in an
Authorization header.


For the benefit of someone trying to implement this, libcurl has the 
CURLOPT_XOAUTH2_BEARER option.  It does requires libcurl 7.33, though.

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH 0/2] Getopt::Long workaround in send-email

2015-02-13 Thread brian m. carlson

On Fri, Feb 13, 2015 at 12:19:27PM -0800, Junio C Hamano wrote:

The first one is a replay of Kyle's workaround for older versions of
Getopt::Long that did not take --no-option to negate a boolean
option --option.  The second one revert the workarounds made to
the test script over time, and should break if the first one does
not work well for older Getopt::Long (I have no reason to suspect it
would break, though).

I am inclined to squash these into one commit before starting to
merge them down to 'next' and then to 'master', after getting
Tested-by: from those with older Getopt::Long (prior to 2.32).

Obviously, tc/t9001-noxmailer topic will become unnecessary and be
dropped when that happens.


I think this is a good fix.  It preserves the documented behavior even 
on less capable systems.

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails

2015-01-07 Thread brian m. carlson

On Tue, Jan 06, 2015 at 04:07:01PM +, Dan Langille (dalangil) wrote:

 HTTP/1.1 401 Authorization Required
 Date: Tue, 06 Jan 2015 16:02:48 GMT
 Server: Apache
 WWW-Authenticate: Negotiate


Your server is set up incorrectly.  You should see a Negotiate line and 
a Basic line as well.  Right now, you only have Negotiate set up, so if 
you don't have a ticket, it's going to fail.


I'd recommend making sure that you can access it using a web browser 
after running kdestroy.  That will ensure that it's working properly.

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


[PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails

2015-01-07 Thread brian m. carlson
Apache servers using mod_auth_kerb can be configured to allow the user
to authenticate either using Negotiate (using the Kerberos ticket) or
Basic authentication (using the Kerberos password).  Often, one will
want to use Negotiate authentication if it is available, but fall back
to Basic authentication if the ticket is missing or expired.

However, libcurl will try very hard to use something other than Basic
auth, even over HTTPS.  If Basic and something else are offered, libcurl
will never attempt to use Basic, even if the other option fails.
Teach the HTTP client code to stop trying authentication mechanisms that
don't use a password (currently Negotiate) after the first failure,
since if they failed the first time, they will never succeed.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
Signed-off-by: Jeff King p...@peff.net
---
Peff's original change was to get_curl_handle; however, we retry the
second time with the same slot and we may not call get_curl_handle
again, so I had to move that change to get_active_slot.  This has been
tested pushing with both Negotiate and Basic against an HTTPS server
both when info/refs was protected and when it was not.

 http.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/http.c b/http.c
index 040f362..44b130c 100644
--- a/http.c
+++ b/http.c
@@ -62,6 +62,9 @@ static const char *user_agent;
 
 static struct credential cert_auth = CREDENTIAL_INIT;
 static int ssl_cert_password_required;
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+static unsigned long http_auth_methods = CURLAUTH_ANY;
+#endif
 
 static struct curl_slist *pragma_header;
 static struct curl_slist *no_pragma_header;
@@ -580,6 +583,9 @@ struct active_request_slot *get_active_slot(void)
curl_easy_setopt(slot-curl, CURLOPT_UPLOAD, 0);
curl_easy_setopt(slot-curl, CURLOPT_HTTPGET, 1);
curl_easy_setopt(slot-curl, CURLOPT_FAILONERROR, 1);
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+   curl_easy_setopt(slot-curl, CURLOPT_HTTPAUTH, http_auth_methods);
+#endif
if (http_auth.password)
init_curl_http_auth(slot-curl);
 
@@ -870,6 +876,9 @@ int handle_curl_result(struct slot_results *results)
credential_reject(http_auth);
return HTTP_NOAUTH;
} else {
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+   http_auth_methods = ~CURLAUTH_GSSNEGOTIATE;
+#endif
return HTTP_REAUTH;
}
} else {
@@ -986,6 +995,7 @@ static void extract_content_type(struct strbuf *raw, struct 
strbuf *type,
strbuf_addstr(charset, ISO-8859-1);
 }
 
+
 /* http_request() targets */
 #define HTTP_REQUEST_STRBUF0
 #define HTTP_REQUEST_FILE  1
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git Scaling: What factors most affect Git performance for a large repo?

2015-02-20 Thread brian m. carlson

On Fri, Feb 20, 2015 at 11:08:55PM +0100, Sebastian Schuberth wrote:

On 20.02.2015 01:03, brian m. carlson wrote:


If you want good performance, I'd recommend the latest version of Git
both client- and server-side.  Newer versions of Git provide pack
bitmaps, which can dramatically speed up clones and fetches, and Git


Do you happen now which version, if at all, of JGit and Gerrit support 
pack bitmaps?


They were originally implemented in JGit, but I don't know what version, 
sorry.  Some googling tells me that it's probably version 3.0.



2.3.0 fixes a performance regression with large numbers of refs in
non-shallow repositories.


Do you also know in what Git version the regression was introduced?


v1.8.4-rc3-8-gfbd4a70.  It was fixed in v2.2.1-65-g2dacf26.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: Git Scaling: What factors most affect Git performance for a large repo?

2015-02-19 Thread brian m. carlson
.

If you end up having pain points, we're certainly interested in 
working through those.  I've brought up performance problems and people 
are generally responsive.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [GSoC][PATCH v2] log: forbid log --graph --no-walk

2015-03-14 Thread brian m. carlson
On Sun, Mar 15, 2015 at 01:33:07AM +0200, epilys wrote:
 diff --git a/builtin/log.c b/builtin/log.c
 index dd8f3fc..0194133 100644
 --- a/builtin/log.c
 +++ b/builtin/log.c
 @@ -155,6 +155,8 @@ static void cmd_log_init_finish(int argc, const char 
 **argv, const char *prefix,
   memset(w, 0, sizeof(w));
   userformat_find_requirements(NULL, w);
  
 +if (rev-graph  rev-no_walk)
 +die(--graph and --no-walk are incompatible);

It looks like you indented here with four spaces instead of a tab.  We
prefer tabs in Git.

   if (!rev-show_notes_given  (!rev-pretty_given || w.notes))
   rev-show_notes = 1;
   if (rev-show_notes)
 diff --git a/t/t4202-log.sh b/t/t4202-log.sh
 index 5f2b290..4dd939b 100755
 --- a/t/t4202-log.sh
 +++ b/t/t4202-log.sh
 @@ -887,4 +887,8 @@ test_expect_success GPG 'log --graph --show-signature for 
 merged tag' '
   grep ^| | gpg: Good signature actual
  '
  
 +test_expect_success 'forbid log --graph --no-walk' '
 +test_must_fail git log --graph --no-walk

And here as well.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [GSoC][PATCH v2] log: forbid log --graph --no-walk

2015-03-14 Thread brian m. carlson
On Sun, Mar 15, 2015 at 01:55:28AM +0200, epilys wrote:
 On 03/15/2015 01:47 AM, brian m. carlson wrote:
  It looks like you indented here with four spaces instead of a tab. 
  We prefer tabs in Git.
 
 Messed that up. Do you think I should resubmit a v3 or am I hogging
 the mailing list too much?

You're going to want to submit a v3.  While you're at it, you probably
want to drop the [GSoC] from the patch header, as it will be part of the
commit message when applied, and we don't want that.  Finally, you
probably want to use your full name in the From: line, as that will be
used to create the commit, and we prefer full names over aliases as well.

It can be helpful to try formatting the patch with git format-patch and
then checking over it with less and applying it with git am to see how
it will look to other Git developers.

This is a small patch, so it's not as big a deal, but for larger series
I generally try to wait two or three days (at least one of which is a
weekday) before posting a new version so that people have time to read,
test, and comment on it.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: What's cooking in git.git (Mar 2015, #07; Fri, 20)

2015-03-21 Thread brian m. carlson
On Fri, Mar 20, 2015 at 08:20:58PM -0700, Junio C Hamano wrote:
 I had an impression that the series may see at least one reroll to
 polish it further before it gets ready for 'next', as I only saw
 discussions on the tangent (e.g. possible futures) and didn't see
 serious reviews of the code that we will actually be using.

If people have suggestions on how to improve it, I'm eager to hear them
and submit a reroll or follow-up patches, as appropriate.  Making
changes now would be much better than having to do it down the line.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] git=p4.py rebase now honor's client spec

2015-03-19 Thread brian m. carlson
On Thu, Mar 19, 2015 at 12:28:09PM +, Sam Bishop wrote:
 When using the git-p4.py script, I found that if I used a client spec when
 cloning out a perforce repository, and then using a git-p4.py rebase, that
 the rebase command would be using the current perforce client spec,
 instead of the one used when doing the initial clone. My proposed patch
 causes the rebase command to switch to the perforce client spec used when
 doing the initial git-p4.py clone.
 
 This email and any attachments may contain confidential and
 proprietary information of Blackboard that is for the sole use of the
 intended recipient. If you are not the intended recipient, disclosure,
 copying, re-distribution or other use of any of this information is
 strictly prohibited. Please immediately notify the sender and delete
 this transmission if you received this email in error.

You might want to read Documentation/SubmittingPatches for information
on patch submission procedures.  Most notably, you'll need to sign-off
your work to indicate that you can submit it under the appropriate
license.

The confidentiality notice above is not only inappropriate for a public
mailing list, it discourages people from even looking at your patch, as
you just claimed it was confidential and proprietary and nobody wants to
be sued.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


[PATCH 06/16] refs: convert for_each_ref_submodule to struct object_id

2015-03-20 Thread brian m. carlson
Convert the callers as well.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 refs.c | 4 ++--
 refs.h | 2 +-
 revision.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/refs.c b/refs.c
index 75d8970..758bdd9 100644
--- a/refs.c
+++ b/refs.c
@@ -1964,9 +1964,9 @@ int for_each_ref(each_ref_fn fn, void *cb_data)
return do_for_each_ref(ref_cache, , fn, 0, 0, cb_data);
 }
 
-int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void 
*cb_data)
 {
-   return do_for_each_ref(get_ref_cache(submodule), , fn, 0, 0, cb_data);
+   return do_for_each_ref_oid(get_ref_cache(submodule), , fn, 0, 0, 
cb_data);
 }
 
 int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
diff --git a/refs.h b/refs.h
index 6c4a8c0..d3ff0b1 100644
--- a/refs.h
+++ b/refs.h
@@ -106,7 +106,7 @@ extern int for_each_glob_ref(each_ref_fn, const char 
*pattern, void *);
 extern int for_each_glob_ref_in(each_ref_fn, const char *pattern, const char* 
prefix, void *);
 
 extern int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void 
*cb_data);
-extern int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data);
+extern int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, 
void *cb_data);
 extern int for_each_ref_in_submodule(const char *submodule, const char *prefix,
each_ref_fn_oid fn, void *cb_data);
 extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid 
fn, void *cb_data);
diff --git a/revision.c b/revision.c
index 7b05c89..c2d8b1c 100644
--- a/revision.c
+++ b/revision.c
@@ -2099,7 +2099,7 @@ static int handle_revision_pseudo_opt(const char 
*submodule,
 * register it in the list at the top of handle_revision_opt.
 */
if (!strcmp(arg, --all)) {
-   handle_refs(submodule, revs, *flags, for_each_ref_submodule);
+   handle_refs_oid(submodule, revs, *flags, 
for_each_ref_submodule);
handle_refs_oid(submodule, revs, *flags, head_ref_submodule);
clear_ref_exclusion(revs-ref_excludes);
} else if (!strcmp(arg, --branches)) {
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 15/16] Remove unneeded *_oid functions.

2015-03-20 Thread brian m. carlson
While these functions were needed during the intermediate steps of
converting for_each_ref and friends to struct object_id, there is no
longer any need to have these wrapper functions.  Update each of the
functions that the wrapper functions call and remove the _oid wrapper
functions themselves.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 builtin/rev-parse.c | 27 +++
 builtin/show-ref.c  | 23 +--
 log-tree.c  | 19 +++
 reachable.c | 13 -
 shallow.c   | 33 ++---
 5 files changed, 41 insertions(+), 74 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 74eae6a..d6a6599 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -190,19 +190,14 @@ static int show_default(void)
return 0;
 }
 
-static int show_reference(const char *refname, const unsigned char *sha1, int 
flag, void *cb_data)
+static int show_reference(const char *refname, const struct object_id *oid, 
int flag, void *cb_data)
 {
if (ref_excluded(ref_excludes, refname))
return 0;
-   show_rev(NORMAL, sha1, refname);
+   show_rev(NORMAL, oid-hash, refname);
return 0;
 }
 
-static int show_reference_oid(const char *refname, const struct object_id 
*oid, int flag, void *cb_data)
-{
-   return show_reference(refname, oid-hash, flag, cb_data);
-}
-
 static int anti_reference(const char *refname, const struct object_id *oid, 
int flag, void *cb_data)
 {
show_rev(REVERSED, oid-hash, refname);
@@ -650,7 +645,7 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
continue;
}
if (!strcmp(arg, --all)) {
-   for_each_ref(show_reference_oid, NULL);
+   for_each_ref(show_reference, NULL);
continue;
}
if (starts_with(arg, --disambiguate=)) {
@@ -658,45 +653,45 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
continue;
}
if (!strcmp(arg, --bisect)) {
-   for_each_ref_in(refs/bisect/bad, 
show_reference_oid, NULL);
+   for_each_ref_in(refs/bisect/bad, 
show_reference, NULL);
for_each_ref_in(refs/bisect/good, 
anti_reference, NULL);
continue;
}
if (starts_with(arg, --branches=)) {
-   for_each_glob_ref_in(show_reference_oid, arg + 
11,
+   for_each_glob_ref_in(show_reference, arg + 11,
refs/heads/, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
if (!strcmp(arg, --branches)) {
-   for_each_branch_ref(show_reference_oid, NULL);
+   for_each_branch_ref(show_reference, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
if (starts_with(arg, --tags=)) {
-   for_each_glob_ref_in(show_reference_oid, arg + 
7,
+   for_each_glob_ref_in(show_reference, arg + 7,
refs/tags/, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
if (!strcmp(arg, --tags)) {
-   for_each_tag_ref(show_reference_oid, NULL);
+   for_each_tag_ref(show_reference, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
if (starts_with(arg, --glob=)) {
-   for_each_glob_ref(show_reference_oid, arg + 7, 
NULL);
+   for_each_glob_ref(show_reference, arg + 7, 
NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
if (starts_with(arg, --remotes=)) {
-   for_each_glob_ref_in(show_reference_oid, arg + 
10,
+   for_each_glob_ref_in(show_reference, arg + 10,
refs/remotes/, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
if (!strcmp(arg, --remotes

[PATCH 05/16] refs: convert head_ref to struct object_id

2015-03-20 Thread brian m. carlson
Convert head_ref and head_ref_submodule to use struct object_id.
Introduce some wrappers in some of the callers to handle
incompatibilities between each_ref_fn and each_ref_fn_oid.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 builtin/show-ref.c |  7 ++-
 log-tree.c |  7 ++-
 reachable.c|  7 ++-
 refs.c | 16 
 refs.h |  4 ++--
 revision.c |  2 +-
 shallow.c  | 19 ---
 7 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 5ba1f30..d499f93 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -88,6 +88,11 @@ match:
return 0;
 }
 
+static int show_ref_oid(const char *refname, const struct object_id *oid, int 
flag, void *cbdata)
+{
+   return show_ref(refname, oid-hash, flag, cbdata);
+}
+
 static int add_existing(const char *refname, const unsigned char *sha1, int 
flag, void *cbdata)
 {
struct string_list *list = (struct string_list *)cbdata;
@@ -225,7 +230,7 @@ int cmd_show_ref(int argc, const char **argv, const char 
*prefix)
}
 
if (show_head)
-   head_ref(show_ref, NULL);
+   head_ref(show_ref_oid, NULL);
for_each_ref(show_ref, NULL);
if (!found_match) {
if (verify  !quiet)
diff --git a/log-tree.c b/log-tree.c
index 51cc695..9288b37 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -135,6 +135,11 @@ static int add_ref_decoration(const char *refname, const 
unsigned char *sha1, in
return 0;
 }
 
+static int add_ref_decoration_oid(const char *refname, const struct object_id 
*oid, int flags, void *cb_data)
+{
+   return add_ref_decoration(refname, oid-hash, flags, cb_data);
+}
+
 static int add_graft_decoration(const struct commit_graft *graft, void 
*cb_data)
 {
struct commit *commit = lookup_commit(graft-oid.hash);
@@ -150,7 +155,7 @@ void load_ref_decorations(int flags)
if (!loaded) {
loaded = 1;
for_each_ref(add_ref_decoration, flags);
-   head_ref(add_ref_decoration, flags);
+   head_ref(add_ref_decoration_oid, flags);
for_each_commit_graft(add_graft_decoration, NULL);
}
 }
diff --git a/reachable.c b/reachable.c
index a647267..d49385a 100644
--- a/reachable.c
+++ b/reachable.c
@@ -32,6 +32,11 @@ static int add_one_ref(const char *path, const unsigned char 
*sha1, int flag, vo
return 0;
 }
 
+static int add_one_ref_oid(const char *path, const struct object_id *oid, int 
flag, void *cb_data)
+{
+   return add_one_ref(path, oid-hash, flag, cb_data);
+}
+
 /*
  * The traversal will have already marked us as SEEN, so we
  * only need to handle any progress reporting here.
@@ -169,7 +174,7 @@ void mark_reachable_objects(struct rev_info *revs, int 
mark_reflog,
for_each_ref(add_one_ref, revs);
 
/* detached HEAD is not included in the list above */
-   head_ref(add_one_ref, revs);
+   head_ref(add_one_ref_oid, revs);
 
/* Add all reflog info */
if (mark_reflog)
diff --git a/refs.c b/refs.c
index 710bd6a..75d8970 100644
--- a/refs.c
+++ b/refs.c
@@ -1931,30 +1931,30 @@ static int do_for_each_ref_oid(struct ref_cache *refs, 
const char *base,
return do_for_each_entry(refs, base, do_one_ref, data);
 }
 
-static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
+static int do_head_ref(const char *submodule, each_ref_fn_oid fn, void 
*cb_data)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
int flag;
 
if (submodule) {
-   if (resolve_gitlink_ref(submodule, HEAD, sha1) == 0)
-   return fn(HEAD, sha1, 0, cb_data);
+   if (resolve_gitlink_ref(submodule, HEAD, oid.hash) == 0)
+   return fn(HEAD, oid, 0, cb_data);
 
return 0;
}
 
-   if (!read_ref_full(HEAD, RESOLVE_REF_READING, sha1, flag))
-   return fn(HEAD, sha1, flag, cb_data);
+   if (!read_ref_full(HEAD, RESOLVE_REF_READING, oid.hash, flag))
+   return fn(HEAD, oid, flag, cb_data);
 
return 0;
 }
 
-int head_ref(each_ref_fn fn, void *cb_data)
+int head_ref(each_ref_fn_oid fn, void *cb_data)
 {
return do_head_ref(NULL, fn, cb_data);
 }
 
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void 
*cb_data)
 {
return do_head_ref(submodule, fn, cb_data);
 }
diff --git a/refs.h b/refs.h
index 7fe7a39..6c4a8c0 100644
--- a/refs.h
+++ b/refs.h
@@ -95,7 +95,7 @@ typedef int each_ref_fn_oid(const char *refname,
  * modifies the reference also returns a nonzero value to immediately
  * stop the iteration.
  */
-extern int head_ref(each_ref_fn, void *);
+extern int head_ref(each_ref_fn_oid, void *);
 extern int for_each_ref(each_ref_fn, void *);
 extern

[PATCH 01/16] refs: convert struct ref_entry to use struct object_id

2015-03-20 Thread brian m. carlson
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 refs.c | 46 +++---
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/refs.c b/refs.c
index 9edf18b..689a46d 100644
--- a/refs.c
+++ b/refs.c
@@ -129,7 +129,7 @@ struct ref_value {
 * null.  If REF_ISSYMREF, then this is the name of the object
 * referred to by the last reference in the symlink chain.
 */
-   unsigned char sha1[20];
+   struct object_id oid;
 
/*
 * If REF_KNOWS_PEELED, then this field holds the peeled value
@@ -137,7 +137,7 @@ struct ref_value {
 * be peelable.  See the documentation for peel_ref() for an
 * exact definition of peelable.
 */
-   unsigned char peeled[20];
+   struct object_id peeled;
 };
 
 struct ref_cache;
@@ -321,8 +321,8 @@ static struct ref_entry *create_ref_entry(const char 
*refname,
die(Reference has invalid name: '%s', refname);
len = strlen(refname) + 1;
ref = xmalloc(sizeof(struct ref_entry) + len);
-   hashcpy(ref-u.value.sha1, sha1);
-   hashclr(ref-u.value.peeled);
+   hashcpy(ref-u.value.oid.hash, sha1);
+   oidclr(ref-u.value.peeled);
memcpy(ref-name, refname, len);
ref-flag = flag;
return ref;
@@ -596,7 +596,7 @@ static int is_dup_ref(const struct ref_entry *ref1, const 
struct ref_entry *ref2
/* This is impossible by construction */
die(Reference directory conflict: %s, ref1-name);
 
-   if (hashcmp(ref1-u.value.sha1, ref2-u.value.sha1))
+   if (oidcmp(ref1-u.value.oid, ref2-u.value.oid))
die(Duplicated ref, and SHA1s don't match: %s, ref1-name);
 
warning(Duplicated ref: %s, ref1-name);
@@ -644,7 +644,7 @@ static int ref_resolves_to_object(struct ref_entry *entry)
 {
if (entry-flag  REF_ISBROKEN)
return 0;
-   if (!has_sha1_file(entry-u.value.sha1)) {
+   if (!has_sha1_file(entry-u.value.oid.hash)) {
error(%s does not point to a valid object!, entry-name);
return 0;
}
@@ -692,7 +692,7 @@ static int do_one_ref(struct ref_entry *entry, void 
*cb_data)
/* Store the old value, in case this is a recursive call: */
old_current_ref = current_ref;
current_ref = entry;
-   retval = data-fn(entry-name + data-trim, entry-u.value.sha1,
+   retval = data-fn(entry-name + data-trim, entry-u.value.oid.hash,
  entry-flag, data-cb_data);
current_ref = old_current_ref;
return retval;
@@ -1166,7 +1166,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
line.len == PEELED_LINE_LENGTH 
line.buf[PEELED_LINE_LENGTH - 1] == '\n' 
!get_sha1_hex(line.buf + 1, sha1)) {
-   hashcpy(last-u.value.peeled, sha1);
+   hashcpy(last-u.value.peeled.hash, sha1);
/*
 * Regardless of what the file header said,
 * we definitely know the value of *this*
@@ -1345,7 +1345,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache 
*refs,
if (ref == NULL)
return -1;
 
-   hashcpy(sha1, ref-u.value.sha1);
+   hashcpy(sha1, ref-u.value.oid.hash);
return 0;
 }
 
@@ -1432,7 +1432,7 @@ static int resolve_missing_loose_ref(const char *refname,
 */
entry = get_packed_ref(refname);
if (entry) {
-   hashcpy(sha1, entry-u.value.sha1);
+   hashcpy(sha1, entry-u.value.oid.hash);
if (flags)
*flags |= REF_ISPACKED;
return 0;
@@ -1726,9 +1726,9 @@ static enum peel_status peel_entry(struct ref_entry 
*entry, int repeel)
if (entry-flag  REF_KNOWS_PEELED) {
if (repeel) {
entry-flag = ~REF_KNOWS_PEELED;
-   hashclr(entry-u.value.peeled);
+   oidclr(entry-u.value.peeled);
} else {
-   return is_null_sha1(entry-u.value.peeled) ?
+   return is_null_oid(entry-u.value.peeled) ?
PEEL_NON_TAG : PEEL_PEELED;
}
}
@@ -1737,7 +1737,7 @@ static enum peel_status peel_entry(struct ref_entry 
*entry, int repeel)
if (entry-flag  REF_ISSYMREF)
return PEEL_IS_SYMREF;
 
-   status = peel_object(entry-u.value.sha1, entry-u.value.peeled);
+   status = peel_object(entry-u.value.oid.hash, 
entry-u.value.peeled.hash);
if (status == PEEL_PEELED || status == PEEL_NON_TAG)
entry-flag |= REF_KNOWS_PEELED;
return status;
@@ -1752,7 +1752,7 @@ int peel_ref(const char *refname, unsigned char *sha1)
|| !strcmp(current_ref-name, refname

[PATCH 16/16] refs: convert struct ref_lock to struct object_id

2015-03-20 Thread brian m. carlson
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 refs.c | 22 +++---
 refs.h |  2 +-
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/refs.c b/refs.c
index 941e466..b54525a 100644
--- a/refs.c
+++ b/refs.c
@@ -2099,16 +2099,16 @@ static struct ref_lock *verify_lock(struct ref_lock 
*lock,
 {
if (read_ref_full(lock-ref_name,
  mustexist ? RESOLVE_REF_READING : 0,
- lock-old_sha1, NULL)) {
+ lock-old_oid.hash, NULL)) {
int save_errno = errno;
error(Can't verify ref %s, lock-ref_name);
unlock_ref(lock);
errno = save_errno;
return NULL;
}
-   if (hashcmp(lock-old_sha1, old_sha1)) {
+   if (hashcmp(lock-old_oid.hash, old_sha1)) {
error(Ref %s is at %s but expected %s, lock-ref_name,
-   sha1_to_hex(lock-old_sha1), sha1_to_hex(old_sha1));
+   oid_to_hex(lock-old_oid), sha1_to_hex(old_sha1));
unlock_ref(lock);
errno = EBUSY;
return NULL;
@@ -2254,7 +2254,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
}
 
refname = resolve_ref_unsafe(refname, resolve_flags,
-lock-old_sha1, type);
+lock-old_oid.hash, type);
if (!refname  errno == EISDIR) {
/* we are trying to lock foo but we used to
 * have foo/bar which now does not exist;
@@ -2268,7 +2268,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
goto error_return;
}
refname = resolve_ref_unsafe(orig_refname, resolve_flags,
-lock-old_sha1, type);
+lock-old_oid.hash, type);
}
if (type_p)
*type_p = type;
@@ -2278,7 +2278,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
orig_refname, strerror(errno));
goto error_return;
}
-   missing = is_null_sha1(lock-old_sha1);
+   missing = is_null_oid(lock-old_oid);
/* When the ref did not exist and we are creating it,
 * make sure there is no existing ref that is packed
 * whose name begins with our refname, nor a ref whose
@@ -2866,7 +2866,7 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
goto rollback;
}
lock-force_write = 1;
-   hashcpy(lock-old_sha1, orig_sha1);
+   hashcpy(lock-old_oid.hash, orig_sha1);
if (write_ref_sha1(lock, orig_sha1, logmsg)) {
error(unable to write current sha1 into %s, newrefname);
goto rollback;
@@ -3069,7 +3069,7 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
-   if (!lock-force_write  !hashcmp(lock-old_sha1, sha1)) {
+   if (!lock-force_write  !hashcmp(lock-old_oid.hash, sha1)) {
unlock_ref(lock);
return 0;
}
@@ -3098,9 +3098,9 @@ static int write_ref_sha1(struct ref_lock *lock,
return -1;
}
clear_loose_ref_cache(ref_cache);
-   if (log_ref_write(lock-ref_name, lock-old_sha1, sha1, logmsg)  0 ||
+   if (log_ref_write(lock-ref_name, lock-old_oid.hash, sha1, logmsg)  0 
||
(strcmp(lock-ref_name, lock-orig_ref_name) 
-log_ref_write(lock-orig_ref_name, lock-old_sha1, sha1, logmsg)  
0)) {
+log_ref_write(lock-orig_ref_name, lock-old_oid.hash, sha1, 
logmsg)  0)) {
unlock_ref(lock);
return -1;
}
@@ -3124,7 +3124,7 @@ static int write_ref_sha1(struct ref_lock *lock,
  head_sha1, head_flag);
if (head_ref  (head_flag  REF_ISSYMREF) 
!strcmp(head_ref, lock-ref_name))
-   log_ref_write(HEAD, lock-old_sha1, sha1, logmsg);
+   log_ref_write(HEAD, lock-old_oid.hash, sha1, logmsg);
}
if (commit_ref(lock)) {
error(Couldn't set %s, lock-ref_name);
diff --git a/refs.h b/refs.h
index b893f4a..538391c 100644
--- a/refs.h
+++ b/refs.h
@@ -7,7 +7,7 @@ struct ref_lock {
char *ref_name;
char *orig_ref_name;
struct lock_file *lk;
-   unsigned char old_sha1[20];
+   struct object_id old_oid;
int lock_fd;
int force_write;
 };
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 13/16] refs: convert for_each_reflog to struct object_id

2015-03-20 Thread brian m. carlson
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 builtin/fsck.c   |  2 +-
 builtin/reflog.c |  4 ++--
 refs.c   | 10 +-
 refs.h   |  2 +-
 revision.c   |  2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 05616a0..91eb4f6 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -476,7 +476,7 @@ static int fsck_handle_reflog_ent(unsigned char *osha1, 
unsigned char *nsha1,
return 0;
 }
 
-static int fsck_handle_reflog(const char *logname, const unsigned char *sha1, 
int flag, void *cb_data)
+static int fsck_handle_reflog(const char *logname, const struct object_id 
*oid, int flag, void *cb_data)
 {
for_each_reflog_ent(logname, fsck_handle_reflog_ent, NULL);
return 0;
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 66ee402..bf03fd7 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -449,14 +449,14 @@ static int expire_reflog(const char *ref, const unsigned 
char *sha1, int unused,
return status;
 }
 
-static int collect_reflog(const char *ref, const unsigned char *sha1, int 
unused, void *cb_data)
+static int collect_reflog(const char *ref, const struct object_id *oid, int 
unused, void *cb_data)
 {
struct collected_reflog *e;
struct collect_reflog_cb *cb = cb_data;
size_t namelen = strlen(ref);
 
e = xmalloc(sizeof(*e) + namelen + 1);
-   hashcpy(e-sha1, sha1);
+   hashcpy(e-sha1, oid-hash);
memcpy(e-reflog, ref, namelen + 1);
ALLOC_GROW(cb-e, cb-nr + 1, cb-alloc);
cb-e[cb-nr++] = e;
diff --git a/refs.c b/refs.c
index de7ac1c..bbcb044 100644
--- a/refs.c
+++ b/refs.c
@@ -3491,7 +3491,7 @@ int for_each_reflog_ent(const char *refname, 
each_reflog_ent_fn fn, void *cb_dat
  * must be empty or end with '/'.  Name will be used as a scratch
  * space, but its contents will be restored before return.
  */
-static int do_for_each_reflog(struct strbuf *name, each_ref_fn fn, void 
*cb_data)
+static int do_for_each_reflog(struct strbuf *name, each_ref_fn_oid fn, void 
*cb_data)
 {
DIR *d = opendir(git_path(logs/%s, name-buf));
int retval = 0;
@@ -3516,11 +3516,11 @@ static int do_for_each_reflog(struct strbuf *name, 
each_ref_fn fn, void *cb_data
strbuf_addch(name, '/');
retval = do_for_each_reflog(name, fn, cb_data);
} else {
-   unsigned char sha1[20];
-   if (read_ref_full(name-buf, 0, sha1, NULL))
+   struct object_id oid;
+   if (read_ref_full(name-buf, 0, oid.hash, NULL))
retval = error(bad ref for %s, 
name-buf);
else
-   retval = fn(name-buf, sha1, 0, 
cb_data);
+   retval = fn(name-buf, oid, 0, 
cb_data);
}
if (retval)
break;
@@ -3531,7 +3531,7 @@ static int do_for_each_reflog(struct strbuf *name, 
each_ref_fn fn, void *cb_data
return retval;
 }
 
-int for_each_reflog(each_ref_fn fn, void *cb_data)
+int for_each_reflog(each_ref_fn_oid fn, void *cb_data)
 {
int retval;
struct strbuf name;
diff --git a/refs.h b/refs.h
index 89217a7..c491c44 100644
--- a/refs.h
+++ b/refs.h
@@ -249,7 +249,7 @@ int for_each_reflog_ent_reverse(const char *refname, 
each_reflog_ent_fn fn, void
  * Calls the specified function for each reflog file until it returns nonzero,
  * and returns the value
  */
-extern int for_each_reflog(each_ref_fn, void *);
+extern int for_each_reflog(each_ref_fn_oid, void *);
 
 #define REFNAME_ALLOW_ONELEVEL 1
 #define REFNAME_REFSPEC_PATTERN 2
diff --git a/revision.c b/revision.c
index ffd3528..b491d22 100644
--- a/revision.c
+++ b/revision.c
@@ -1283,7 +1283,7 @@ static int handle_one_reflog_ent(unsigned char *osha1, 
unsigned char *nsha1,
return 0;
 }
 
-static int handle_one_reflog(const char *path, const unsigned char *sha1, int 
flag, void *cb_data)
+static int handle_one_reflog(const char *path, const struct object_id *oid, 
int flag, void *cb_data)
 {
struct all_refs_cb *cb = cb_data;
cb-warned_bad_reflog = 0;
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 10/16] refs: convert namespaced ref iteration functions to object_id

2015-03-20 Thread brian m. carlson
Convert head_ref_namespaced and for_each_namespaced_ref to use struct
object_id.  Update the various callbacks to use struct object_id
internally as well.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 http-backend.c | 14 +++---
 refs.c | 12 ++--
 refs.h |  4 ++--
 upload-pack.c  | 28 ++--
 4 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/http-backend.c b/http-backend.c
index b6c0484..e0d6627 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -350,16 +350,16 @@ static void run_service(const char **argv)
exit(1);
 }
 
-static int show_text_ref(const char *name, const unsigned char *sha1,
+static int show_text_ref(const char *name, const struct object_id *oid,
int flag, void *cb_data)
 {
const char *name_nons = strip_namespace(name);
struct strbuf *buf = cb_data;
-   struct object *o = parse_object(sha1);
+   struct object *o = parse_object(oid-hash);
if (!o)
return 0;
 
-   strbuf_addf(buf, %s\t%s\n, sha1_to_hex(sha1), name_nons);
+   strbuf_addf(buf, %s\t%s\n, oid_to_hex(oid), name_nons);
if (o-type == OBJ_TAG) {
o = deref_tag(o, name, 0);
if (!o)
@@ -402,21 +402,21 @@ static void get_info_refs(char *arg)
strbuf_release(buf);
 }
 
-static int show_head_ref(const char *refname, const unsigned char *sha1,
+static int show_head_ref(const char *refname, const struct object_id *oid,
int flag, void *cb_data)
 {
struct strbuf *buf = cb_data;
 
if (flag  REF_ISSYMREF) {
-   unsigned char unused[20];
+   struct object_id unused;
const char *target = resolve_ref_unsafe(refname,
RESOLVE_REF_READING,
-   unused, NULL);
+   unused.hash, NULL);
const char *target_nons = strip_namespace(target);
 
strbuf_addf(buf, ref: %s\n, target_nons);
} else {
-   strbuf_addf(buf, %s\n, sha1_to_hex(sha1));
+   strbuf_addf(buf, %s\n, oid_to_hex(oid));
}
 
return 0;
diff --git a/refs.c b/refs.c
index 0d9b340..1fa2ec0 100644
--- a/refs.c
+++ b/refs.c
@@ -2015,27 +2015,27 @@ int for_each_replace_ref(each_ref_fn_oid fn, void 
*cb_data)
return do_for_each_ref_oid(ref_cache, refs/replace/, fn, 13, 0, 
cb_data);
 }
 
-int head_ref_namespaced(each_ref_fn fn, void *cb_data)
+int head_ref_namespaced(each_ref_fn_oid fn, void *cb_data)
 {
struct strbuf buf = STRBUF_INIT;
int ret = 0;
-   unsigned char sha1[20];
+   struct object_id oid;
int flag;
 
strbuf_addf(buf, %sHEAD, get_git_namespace());
-   if (!read_ref_full(buf.buf, RESOLVE_REF_READING, sha1, flag))
-   ret = fn(buf.buf, sha1, flag, cb_data);
+   if (!read_ref_full(buf.buf, RESOLVE_REF_READING, oid.hash, flag))
+   ret = fn(buf.buf, oid, flag, cb_data);
strbuf_release(buf);
 
return ret;
 }
 
-int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
+int for_each_namespaced_ref(each_ref_fn_oid fn, void *cb_data)
 {
struct strbuf buf = STRBUF_INIT;
int ret;
strbuf_addf(buf, %srefs/, get_git_namespace());
-   ret = do_for_each_ref(ref_cache, buf.buf, fn, 0, 0, cb_data);
+   ret = do_for_each_ref_oid(ref_cache, buf.buf, fn, 0, 0, cb_data);
strbuf_release(buf);
return ret;
 }
diff --git a/refs.h b/refs.h
index 951e465..6d2d66d 100644
--- a/refs.h
+++ b/refs.h
@@ -113,8 +113,8 @@ extern int for_each_tag_ref_submodule(const char 
*submodule, each_ref_fn_oid fn,
 extern int for_each_branch_ref_submodule(const char *submodule, 
each_ref_fn_oid fn, void *cb_data);
 extern int for_each_remote_ref_submodule(const char *submodule, 
each_ref_fn_oid fn, void *cb_data);
 
-extern int head_ref_namespaced(each_ref_fn fn, void *cb_data);
-extern int for_each_namespaced_ref(each_ref_fn fn, void *cb_data);
+extern int head_ref_namespaced(each_ref_fn_oid fn, void *cb_data);
+extern int for_each_namespaced_ref(each_ref_fn_oid fn, void *cb_data);
 
 static inline const char *has_glob_specials(const char *pattern)
 {
diff --git a/upload-pack.c b/upload-pack.c
index 0566ce0..2105bc2 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -681,16 +681,16 @@ static void receive_needs(void)
 }
 
 /* return non-zero if the ref is hidden, otherwise 0 */
-static int mark_our_ref(const char *refname, const unsigned char *sha1, int 
flag, void *cb_data)
+static int mark_our_ref(const char *refname, const struct object_id *oid, int 
flag, void *cb_data)
 {
-   struct object *o = lookup_unknown_object(sha1);
+   struct object *o = lookup_unknown_object(oid-hash);
 
if (ref_is_hidden(refname)) {
o-flags |= HIDDEN_REF

[PATCH 12/16] refs: rename do_for_each_ref_oid to do_for_each_ref

2015-03-20 Thread brian m. carlson
do_for_each_ref was unused due to previous patches, so rename
do_for_each_ref_oid to do_for_each_ref.  Similarly, remove the unused fn
member from struct ref_entry in favor of renaming the fn_oid member.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 refs.c | 43 +++
 1 file changed, 11 insertions(+), 32 deletions(-)

diff --git a/refs.c b/refs.c
index 025b3c0..de7ac1c 100644
--- a/refs.c
+++ b/refs.c
@@ -668,8 +668,7 @@ struct ref_entry_cb {
const char *base;
int trim;
int flags;
-   each_ref_fn *fn;
-   each_ref_fn_oid *fn_oid;
+   each_ref_fn_oid *fn;
void *cb_data;
 };
 
@@ -693,13 +692,8 @@ static int do_one_ref(struct ref_entry *entry, void 
*cb_data)
/* Store the old value, in case this is a recursive call: */
old_current_ref = current_ref;
current_ref = entry;
-   if (data-fn_oid) {
-   retval = data-fn_oid(entry-name + data-trim, 
entry-u.value.oid,
-entry-flag, data-cb_data);
-   } else {
-   retval = data-fn(entry-name + data-trim, 
entry-u.value.oid.hash,
-entry-flag, data-cb_data);
-   }
+   retval = data-fn(entry-name + data-trim, entry-u.value.oid,
+entry-flag, data-cb_data);
current_ref = old_current_ref;
return retval;
 }
@@ -1904,28 +1898,13 @@ static int do_for_each_entry(struct ref_cache *refs, 
const char *base,
  * 0.
  */
 static int do_for_each_ref(struct ref_cache *refs, const char *base,
-  each_ref_fn fn, int trim, int flags, void *cb_data)
-{
-   struct ref_entry_cb data;
-   data.base = base;
-   data.trim = trim;
-   data.flags = flags;
-   data.fn = fn;
-   data.fn_oid = NULL;
-   data.cb_data = cb_data;
-
-   return do_for_each_entry(refs, base, do_one_ref, data);
-}
-
-static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
   each_ref_fn_oid fn, int trim, int flags, void 
*cb_data)
 {
struct ref_entry_cb data;
data.base = base;
data.trim = trim;
data.flags = flags;
-   data.fn = NULL;
-   data.fn_oid = fn;
+   data.fn = fn;
data.cb_data = cb_data;
 
return do_for_each_entry(refs, base, do_one_ref, data);
@@ -1961,23 +1940,23 @@ int head_ref_submodule(const char *submodule, 
each_ref_fn_oid fn, void *cb_data)
 
 int for_each_ref(each_ref_fn_oid fn, void *cb_data)
 {
-   return do_for_each_ref_oid(ref_cache, , fn, 0, 0, cb_data);
+   return do_for_each_ref(ref_cache, , fn, 0, 0, cb_data);
 }
 
 int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void 
*cb_data)
 {
-   return do_for_each_ref_oid(get_ref_cache(submodule), , fn, 0, 0, 
cb_data);
+   return do_for_each_ref(get_ref_cache(submodule), , fn, 0, 0, cb_data);
 }
 
 int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
 {
-   return do_for_each_ref_oid(ref_cache, prefix, fn, strlen(prefix), 0, 
cb_data);
+   return do_for_each_ref(ref_cache, prefix, fn, strlen(prefix), 0, 
cb_data);
 }
 
 int for_each_ref_in_submodule(const char *submodule, const char *prefix,
each_ref_fn_oid fn, void *cb_data)
 {
-   return do_for_each_ref_oid(get_ref_cache(submodule), prefix, fn, 
strlen(prefix), 0, cb_data);
+   return do_for_each_ref(get_ref_cache(submodule), prefix, fn, 
strlen(prefix), 0, cb_data);
 }
 
 int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
@@ -2012,7 +1991,7 @@ int for_each_remote_ref_submodule(const char *submodule, 
each_ref_fn_oid fn, voi
 
 int for_each_replace_ref(each_ref_fn_oid fn, void *cb_data)
 {
-   return do_for_each_ref_oid(ref_cache, refs/replace/, fn, 13, 0, 
cb_data);
+   return do_for_each_ref(ref_cache, refs/replace/, fn, 13, 0, cb_data);
 }
 
 int head_ref_namespaced(each_ref_fn_oid fn, void *cb_data)
@@ -2035,7 +2014,7 @@ int for_each_namespaced_ref(each_ref_fn_oid fn, void 
*cb_data)
struct strbuf buf = STRBUF_INIT;
int ret;
strbuf_addf(buf, %srefs/, get_git_namespace());
-   ret = do_for_each_ref_oid(ref_cache, buf.buf, fn, 0, 0, cb_data);
+   ret = do_for_each_ref(ref_cache, buf.buf, fn, 0, 0, cb_data);
strbuf_release(buf);
return ret;
 }
@@ -2077,7 +2056,7 @@ int for_each_glob_ref(each_ref_fn_oid fn, const char 
*pattern, void *cb_data)
 
 int for_each_rawref(each_ref_fn_oid fn, void *cb_data)
 {
-   return do_for_each_ref_oid(ref_cache, , fn, 0,
+   return do_for_each_ref(ref_cache, , fn, 0,
   DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
 }
 
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 14/16] refs: rename each_ref_fn_oid to each_ref_fn

2015-03-20 Thread brian m. carlson
each_ref_fn is no longer used, so rename each_ref_fn_oid to each_ref_fn.
Update the documentation to note the change in function signature.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/technical/api-ref-iteration.txt |  2 +-
 refs.c| 48 +--
 refs.h| 44 +++-
 revision.c|  6 ++--
 4 files changed, 47 insertions(+), 53 deletions(-)

diff --git a/Documentation/technical/api-ref-iteration.txt 
b/Documentation/technical/api-ref-iteration.txt
index 02adfd4..37379d8 100644
--- a/Documentation/technical/api-ref-iteration.txt
+++ b/Documentation/technical/api-ref-iteration.txt
@@ -6,7 +6,7 @@ Iteration of refs is done by using an iterate function which 
will call a
 callback function for every ref. The callback function has this
 signature:
 
-   int handle_one_ref(const char *refname, const unsigned char *sha1,
+   int handle_one_ref(const char *refname, const struct object_id *oid,
   int flags, void *cb_data);
 
 There are different kinds of iterate functions which all take a
diff --git a/refs.c b/refs.c
index bbcb044..941e466 100644
--- a/refs.c
+++ b/refs.c
@@ -668,7 +668,7 @@ struct ref_entry_cb {
const char *base;
int trim;
int flags;
-   each_ref_fn_oid *fn;
+   each_ref_fn *fn;
void *cb_data;
 };
 
@@ -1624,7 +1624,7 @@ char *resolve_refdup(const char *ref, int resolve_flags, 
unsigned char *sha1, in
 /* The argument to filter_refs */
 struct ref_filter {
const char *pattern;
-   each_ref_fn_oid *fn;
+   each_ref_fn *fn;
void *cb_data;
 };
 
@@ -1898,7 +1898,7 @@ static int do_for_each_entry(struct ref_cache *refs, 
const char *base,
  * 0.
  */
 static int do_for_each_ref(struct ref_cache *refs, const char *base,
-  each_ref_fn_oid fn, int trim, int flags, void 
*cb_data)
+  each_ref_fn fn, int trim, int flags, void *cb_data)
 {
struct ref_entry_cb data;
data.base = base;
@@ -1910,7 +1910,7 @@ static int do_for_each_ref(struct ref_cache *refs, const 
char *base,
return do_for_each_entry(refs, base, do_one_ref, data);
 }
 
-static int do_head_ref(const char *submodule, each_ref_fn_oid fn, void 
*cb_data)
+static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
 {
struct object_id oid;
int flag;
@@ -1928,73 +1928,73 @@ static int do_head_ref(const char *submodule, 
each_ref_fn_oid fn, void *cb_data)
return 0;
 }
 
-int head_ref(each_ref_fn_oid fn, void *cb_data)
+int head_ref(each_ref_fn fn, void *cb_data)
 {
return do_head_ref(NULL, fn, cb_data);
 }
 
-int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void 
*cb_data)
+int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 {
return do_head_ref(submodule, fn, cb_data);
 }
 
-int for_each_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_ref(each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(ref_cache, , fn, 0, 0, cb_data);
 }
 
-int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void 
*cb_data)
+int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
 {
return do_for_each_ref(get_ref_cache(submodule), , fn, 0, 0, cb_data);
 }
 
-int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
+int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(ref_cache, prefix, fn, strlen(prefix), 0, 
cb_data);
 }
 
 int for_each_ref_in_submodule(const char *submodule, const char *prefix,
-   each_ref_fn_oid fn, void *cb_data)
+   each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(get_ref_cache(submodule), prefix, fn, 
strlen(prefix), 0, cb_data);
 }
 
-int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_tag_ref(each_ref_fn fn, void *cb_data)
 {
return for_each_ref_in(refs/tags/, fn, cb_data);
 }
 
-int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid fn, void 
*cb_data)
+int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
 {
return for_each_ref_in_submodule(submodule, refs/tags/, fn, cb_data);
 }
 
-int for_each_branch_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_branch_ref(each_ref_fn fn, void *cb_data)
 {
return for_each_ref_in(refs/heads/, fn, cb_data);
 }
 
-int for_each_branch_ref_submodule(const char *submodule, each_ref_fn_oid fn, 
void *cb_data)
+int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
 {
return for_each_ref_in_submodule(submodule, refs/heads/, fn, cb_data);
 }
 
-int for_each_remote_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_remote_ref(each_ref_fn fn, void *cb_data

[PATCH 00/16] Convert parts of refs.c to struct object_id

2015-03-20 Thread brian m. carlson
This is part 1 of n in converting code to use struct object_id.

refs.c, and the for_each_ref series of functions particularly, is the
source for many instances of object IDs in the codebase.  Therefore, it
makes sense to convert this series of functions to provide a basis for
further conversions.

This series is essentially just for_each_ref and friends, the callbacks,
and callers.  Other parts of refs.c will be converted in a later series,
so as to keep the number of patches to a reasonable size.

There should be no functional change from this patch series.

This series is based on top of the bc/object-id series in pu.  The
series will be rebased on next once bc/object-id makes it into next.

Available from the following repositories in either object-id-part1 or
object-id-part1-pu (will be rebased):

  https://github.com/bk2204/git.git
  https://git.crustytoothpaste.net/git/bmc/git.git

brian m. carlson (16):
  refs: convert struct ref_entry to use struct object_id
  refs: convert for_each_tag_ref to struct object_id
  refs: convert remaining users of for_each_ref_in to object_id
  refs: convert for_each_ref_in_submodule to object_id
  refs: convert head_ref to struct object_id
  refs: convert for_each_ref_submodule to struct object_id
  revision: remove unused _oid helper.
  refs: convert for_each_ref to struct object_id
  refs: convert for_each_replace_ref to struct object_id
  refs: convert namespaced ref iteration functions to object_id
  refs: convert for_each_rawref to struct object_id.
  refs: rename do_for_each_ref_oid to do_for_each_ref
  refs: convert for_each_reflog to struct object_id
  refs: rename each_ref_fn_oid to each_ref_fn
  Remove unneeded *_oid functions.
  refs: convert struct ref_lock to struct object_id

 Documentation/technical/api-ref-iteration.txt |   2 +-
 bisect.c  |   8 +-
 builtin/branch.c  |   4 +-
 builtin/checkout.c|   4 +-
 builtin/describe.c|  12 +--
 builtin/fetch.c   |   6 +-
 builtin/for-each-ref.c|   4 +-
 builtin/fsck.c|  18 ++---
 builtin/name-rev.c|   6 +-
 builtin/pack-objects.c|  14 ++--
 builtin/receive-pack.c|   4 +-
 builtin/reflog.c  |   8 +-
 builtin/remote.c  |  14 ++--
 builtin/replace.c |  14 ++--
 builtin/rev-parse.c   |   8 +-
 builtin/show-branch.c |  24 +++---
 builtin/show-ref.c|  16 ++--
 builtin/tag.c |   8 +-
 fetch-pack.c  |  18 -
 help.c|   2 +-
 http-backend.c|  14 ++--
 log-tree.c|  10 +--
 notes.c   |   2 +-
 reachable.c   |   4 +-
 refs.c| 104 +-
 refs.h|   6 +-
 remote.c  |   8 +-
 replace_object.c  |   4 +-
 revision.c|  18 +++--
 server-info.c |   6 +-
 sha1_name.c   |   4 +-
 shallow.c |   8 +-
 submodule.c   |   6 +-
 transport.c   |  10 +--
 upload-pack.c |  28 +++
 walker.c  |   4 +-
 36 files changed, 224 insertions(+), 206 deletions(-)

-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 04/16] refs: convert for_each_ref_in_submodule to object_id

2015-03-20 Thread brian m. carlson
Convert for_each_ref_in_submodule and all of its caller.  Introduce two
temporary wrappers in revision.c to handle the incompatibilities between
each_ref_fn and each_ref_fn_oid.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 refs.c  | 10 +-
 refs.h  |  8 
 revision.c  | 28 +---
 submodule.c |  2 +-
 4 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/refs.c b/refs.c
index 2c7bbd4..710bd6a 100644
--- a/refs.c
+++ b/refs.c
@@ -1975,9 +1975,9 @@ int for_each_ref_in(const char *prefix, each_ref_fn_oid 
fn, void *cb_data)
 }
 
 int for_each_ref_in_submodule(const char *submodule, const char *prefix,
-   each_ref_fn fn, void *cb_data)
+   each_ref_fn_oid fn, void *cb_data)
 {
-   return do_for_each_ref(get_ref_cache(submodule), prefix, fn, 
strlen(prefix), 0, cb_data);
+   return do_for_each_ref_oid(get_ref_cache(submodule), prefix, fn, 
strlen(prefix), 0, cb_data);
 }
 
 int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
@@ -1985,7 +1985,7 @@ int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
return for_each_ref_in(refs/tags/, fn, cb_data);
 }
 
-int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid fn, void 
*cb_data)
 {
return for_each_ref_in_submodule(submodule, refs/tags/, fn, cb_data);
 }
@@ -1995,7 +1995,7 @@ int for_each_branch_ref(each_ref_fn_oid fn, void *cb_data)
return for_each_ref_in(refs/heads/, fn, cb_data);
 }
 
-int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+int for_each_branch_ref_submodule(const char *submodule, each_ref_fn_oid fn, 
void *cb_data)
 {
return for_each_ref_in_submodule(submodule, refs/heads/, fn, cb_data);
 }
@@ -2005,7 +2005,7 @@ int for_each_remote_ref(each_ref_fn_oid fn, void *cb_data)
return for_each_ref_in(refs/remotes/, fn, cb_data);
 }
 
-int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+int for_each_remote_ref_submodule(const char *submodule, each_ref_fn_oid fn, 
void *cb_data)
 {
return for_each_ref_in_submodule(submodule, refs/remotes/, fn, 
cb_data);
 }
diff --git a/refs.h b/refs.h
index ff1a41a..7fe7a39 100644
--- a/refs.h
+++ b/refs.h
@@ -108,10 +108,10 @@ extern int for_each_glob_ref_in(each_ref_fn, const char 
*pattern, const char* pr
 extern int head_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data);
 extern int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data);
 extern int for_each_ref_in_submodule(const char *submodule, const char *prefix,
-   each_ref_fn fn, void *cb_data);
-extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, 
void *cb_data);
-extern int for_each_branch_ref_submodule(const char *submodule, each_ref_fn 
fn, void *cb_data);
-extern int for_each_remote_ref_submodule(const char *submodule, each_ref_fn 
fn, void *cb_data);
+   each_ref_fn_oid fn, void *cb_data);
+extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid 
fn, void *cb_data);
+extern int for_each_branch_ref_submodule(const char *submodule, 
each_ref_fn_oid fn, void *cb_data);
+extern int for_each_remote_ref_submodule(const char *submodule, 
each_ref_fn_oid fn, void *cb_data);
 
 extern int head_ref_namespaced(each_ref_fn fn, void *cb_data);
 extern int for_each_namespaced_ref(each_ref_fn fn, void *cb_data);
diff --git a/revision.c b/revision.c
index 86406a2..6b9cf3a 100644
--- a/revision.c
+++ b/revision.c
@@ -1217,6 +1217,12 @@ static int handle_one_ref(const char *path, const 
unsigned char *sha1, int flag,
return 0;
 }
 
+static int handle_one_ref_oid(const char *path, const struct object_id *oid,
+   int flag, void *cb_data)
+{
+   return handle_one_ref(path, oid-hash, flag, cb_data);
+}
+
 static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
unsigned flags)
 {
@@ -1250,6 +1256,14 @@ static void handle_refs(const char *submodule, struct 
rev_info *revs, unsigned f
for_each(submodule, handle_one_ref, cb);
 }
 
+static void handle_refs_oid(const char *submodule, struct rev_info *revs, 
unsigned flags,
+   int (*for_each)(const char *, each_ref_fn_oid, void *))
+{
+   struct all_refs_cb cb;
+   init_all_refs_cb(cb, revs, flags);
+   for_each(submodule, handle_one_ref_oid, cb);
+}
+
 static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
 {
struct all_refs_cb *cb = cb_data;
@@ -2056,12 +2070,12 @@ void parse_revision_opt(struct rev_info *revs, struct 
parse_opt_ctx_t *ctx,
ctx-argc -= n;
 }
 
-static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void 
*cb_data)
+static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn_oid fn, 
void *cb_data)
 {
return

Re: [PATCH 00/16] Convert parts of refs.c to struct object_id

2015-03-20 Thread brian m. carlson
On Fri, Mar 20, 2015 at 12:52:20PM -0700, Junio C Hamano wrote:
 brian m. carlson sand...@crustytoothpaste.net writes:
 
  This is part 1 of n in converting code to use struct object_id.
 
 OK --- 'n' up there looked funny ;-)

It is, unfortunately, a large task.  I wish I could quantify it better
than N, but I expect it will be a progressive task stretching out over
several versions.

 I'd like to see the series reviewed by those who recently worked
 heavily on the subsystem involved (namely, refs API); please do not
 forget to include area experts to Cc the next time around when you
 update the other areas with the same theme.  On the other hand, once
 the struct object_id design is settled and everybody agrees that
 it is good enough, those who were involved in the review of that
 framework do not necessarily have to be on the Cc list (of course
 they are welcome to participate).  But reviews by area experts are
 more important to avoid breaking the users in the individual areas
 that are now going to use the new object-id infrastructure.

Okay, I'll do that next time around.  I expect to do at least one
reroll, so I'll CC them with the next iteration if they don't pipe up at
first.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


[PATCH 08/16] refs: convert for_each_ref to struct object_id

2015-03-20 Thread brian m. carlson
Convert for_each_ref, for_each_glob_ref, and for_each_glob_ref_in to use
struct object_id, as the latter two call the former with the function
pointer they are provided.

Convert callers to refer to properly-typed functions.  Convert uses of
the constant 20 to GIT_SHA1_RAWSZ.  Where possible, convert modified
functions to use struct object_id instead of unsigned char [20].

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 builtin/checkout.c |  4 ++--
 builtin/fetch.c|  6 +++---
 builtin/name-rev.c |  6 +++---
 builtin/pack-objects.c | 10 +-
 builtin/receive-pack.c |  4 ++--
 builtin/reflog.c   |  4 ++--
 builtin/remote.c   | 14 +++---
 builtin/rev-parse.c| 10 +-
 builtin/show-branch.c  | 24 
 builtin/show-ref.c |  4 ++--
 fetch-pack.c   | 18 ++
 help.c |  2 +-
 log-tree.c |  2 +-
 notes.c|  2 +-
 reachable.c|  2 +-
 refs.c | 16 
 refs.h |  6 +++---
 remote.c   |  8 
 revision.c |  8 
 server-info.c  |  6 +++---
 sha1_name.c|  4 ++--
 shallow.c  |  6 +++---
 submodule.c|  4 ++--
 transport.c| 10 +-
 walker.c   |  4 ++--
 25 files changed, 97 insertions(+), 87 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 52d6cbb..f59616f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -685,10 +685,10 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
 }
 
 static int add_pending_uninteresting_ref(const char *refname,
-const unsigned char *sha1,
+const struct object_id *oid,
 int flags, void *cb_data)
 {
-   add_pending_sha1(cb_data, refname, sha1, UNINTERESTING);
+   add_pending_sha1(cb_data, refname, oid-hash, UNINTERESTING);
return 0;
 }
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 75a55e5..37223b3 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -179,13 +179,13 @@ static void add_merge_config(struct ref **head,
}
 }
 
-static int add_existing(const char *refname, const unsigned char *sha1,
+static int add_existing(const char *refname, const struct object_id *oid,
int flag, void *cbdata)
 {
struct string_list *list = (struct string_list *)cbdata;
struct string_list_item *item = string_list_insert(list, refname);
-   item-util = xmalloc(20);
-   hashcpy(item-util, sha1);
+   item-util = xmalloc(GIT_SHA1_RAWSZ);
+   hashcpy(item-util, oid-hash);
return 0;
 }
 
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 3c8f319..31f576d 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -138,9 +138,9 @@ static int tipcmp(const void *a_, const void *b_)
return hashcmp(a-sha1, b-sha1);
 }
 
-static int name_ref(const char *path, const unsigned char *sha1, int flags, 
void *cb_data)
+static int name_ref(const char *path, const struct object_id *oid, int flags, 
void *cb_data)
 {
-   struct object *o = parse_object(sha1);
+   struct object *o = parse_object(oid-hash);
struct name_ref_data *data = cb_data;
int can_abbreviate_output = data-tags_only  data-name_only;
int deref = 0;
@@ -160,7 +160,7 @@ static int name_ref(const char *path, const unsigned char 
*sha1, int flags, void
}
}
 
-   add_to_tip_table(sha1, path, can_abbreviate_output);
+   add_to_tip_table(oid-hash, path, can_abbreviate_output);
 
while (o  o-type == OBJ_TAG) {
struct tag *t = (struct tag *) o;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 65eb4fe..586bb3b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2101,14 +2101,14 @@ static void ll_find_deltas(struct object_entry **list, 
unsigned list_size,
 #define ll_find_deltas(l, s, w, d, p)  find_deltas(l, s, w, d, p)
 #endif
 
-static int add_ref_tag(const char *path, const unsigned char *sha1, int flag, 
void *cb_data)
+static int add_ref_tag(const char *path, const struct object_id *oid, int 
flag, void *cb_data)
 {
-   unsigned char peeled[20];
+   struct object_id peeled;
 
if (starts_with(path, refs/tags/)  /* is a tag? */
-   !peel_ref(path, peeled) /* peelable? */
-   packlist_find(to_pack, peeled, NULL))  /* object packed? */
-   add_object_entry(sha1, OBJ_TAG, NULL, 0);
+   !peel_ref(path, peeled.hash) /* peelable? */
+   packlist_find(to_pack, peeled.hash, NULL))  /* object packed? 
*/
+   add_object_entry(oid-hash, OBJ_TAG, NULL, 0);
return 0;
 }
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 8266c1f..40ad5c5 100644

[PATCH 09/16] refs: convert for_each_replace_ref to struct object_id

2015-03-20 Thread brian m. carlson
Update callbacks to take the proper parameters and use struct object_id
elsewhere in the callbacks.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 builtin/replace.c | 14 +++---
 refs.c|  4 ++--
 refs.h|  2 +-
 replace_object.c  |  4 ++--
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 85d39b5..c873180 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -35,7 +35,7 @@ struct show_data {
enum replace_format format;
 };
 
-static int show_reference(const char *refname, const unsigned char *sha1,
+static int show_reference(const char *refname, const struct object_id *oid,
  int flag, void *cb_data)
 {
struct show_data *data = cb_data;
@@ -44,19 +44,19 @@ static int show_reference(const char *refname, const 
unsigned char *sha1,
if (data-format == REPLACE_FORMAT_SHORT)
printf(%s\n, refname);
else if (data-format == REPLACE_FORMAT_MEDIUM)
-   printf(%s - %s\n, refname, sha1_to_hex(sha1));
+   printf(%s - %s\n, refname, oid_to_hex(oid));
else { /* data-format == REPLACE_FORMAT_LONG */
-   unsigned char object[20];
+   struct object_id object;
enum object_type obj_type, repl_type;
 
-   if (get_sha1(refname, object))
+   if (get_sha1(refname, object.hash))
return error(Failed to resolve '%s' as a valid 
ref., refname);
 
-   obj_type = sha1_object_info(object, NULL);
-   repl_type = sha1_object_info(sha1, NULL);
+   obj_type = sha1_object_info(object.hash, NULL);
+   repl_type = sha1_object_info(oid-hash, NULL);
 
printf(%s (%s) - %s (%s)\n, refname, 
typename(obj_type),
-  sha1_to_hex(sha1), typename(repl_type));
+  oid_to_hex(oid), typename(repl_type));
}
}
 
diff --git a/refs.c b/refs.c
index 4d07dec..0d9b340 100644
--- a/refs.c
+++ b/refs.c
@@ -2010,9 +2010,9 @@ int for_each_remote_ref_submodule(const char *submodule, 
each_ref_fn_oid fn, voi
return for_each_ref_in_submodule(submodule, refs/remotes/, fn, 
cb_data);
 }
 
-int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+int for_each_replace_ref(each_ref_fn_oid fn, void *cb_data)
 {
-   return do_for_each_ref(ref_cache, refs/replace/, fn, 13, 0, cb_data);
+   return do_for_each_ref_oid(ref_cache, refs/replace/, fn, 13, 0, 
cb_data);
 }
 
 int head_ref_namespaced(each_ref_fn fn, void *cb_data)
diff --git a/refs.h b/refs.h
index 2c78450..951e465 100644
--- a/refs.h
+++ b/refs.h
@@ -101,7 +101,7 @@ extern int for_each_ref_in(const char *, each_ref_fn_oid, 
void *);
 extern int for_each_tag_ref(each_ref_fn_oid, void *);
 extern int for_each_branch_ref(each_ref_fn_oid, void *);
 extern int for_each_remote_ref(each_ref_fn_oid, void *);
-extern int for_each_replace_ref(each_ref_fn, void *);
+extern int for_each_replace_ref(each_ref_fn_oid, void *);
 extern int for_each_glob_ref(each_ref_fn_oid, const char *pattern, void *);
 extern int for_each_glob_ref_in(each_ref_fn_oid, const char *pattern, const 
char* prefix, void *);
 
diff --git a/replace_object.c b/replace_object.c
index 0ab2dc1..f0b39f0 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -53,7 +53,7 @@ static int register_replace_object(struct replace_object 
*replace,
 }
 
 static int register_replace_ref(const char *refname,
-   const unsigned char *sha1,
+   const struct object_id *oid,
int flag, void *cb_data)
 {
/* Get sha1 from refname */
@@ -68,7 +68,7 @@ static int register_replace_ref(const char *refname,
}
 
/* Copy sha1 from the read ref */
-   hashcpy(repl_obj-replacement, sha1);
+   hashcpy(repl_obj-replacement, oid-hash);
 
/* Register new object */
if (register_replace_object(repl_obj, 1))
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 11/16] refs: convert for_each_rawref to struct object_id.

2015-03-20 Thread brian m. carlson
Convert callbacks to use struct object_id internally as well.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 builtin/branch.c   |  4 ++--
 builtin/describe.c | 12 ++--
 builtin/for-each-ref.c |  4 ++--
 builtin/fsck.c | 16 
 refs.c | 10 +-
 refs.h |  2 +-
 6 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index dc6f0b2..e591651 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -328,7 +328,7 @@ static int match_patterns(const char **pattern, const char 
*refname)
return 0;
 }
 
-static int append_ref(const char *refname, const unsigned char *sha1, int 
flags, void *cb_data)
+static int append_ref(const char *refname, const struct object_id *oid, int 
flags, void *cb_data)
 {
struct append_ref_cb *cb = (struct append_ref_cb *)(cb_data);
struct ref_list *ref_list = cb-ref_list;
@@ -365,7 +365,7 @@ static int append_ref(const char *refname, const unsigned 
char *sha1, int flags,
 
commit = NULL;
if (ref_list-verbose || ref_list-with_commit || merge_filter != 
NO_FILTER) {
-   commit = lookup_commit_reference_gently(sha1, 1);
+   commit = lookup_commit_reference_gently(oid-hash, 1);
if (!commit) {
cb-ret = error(_(branch '%s' does not point at a 
commit), refname);
return 0;
diff --git a/builtin/describe.c b/builtin/describe.c
index 9103193..f88ad16 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -119,10 +119,10 @@ static void add_to_known_names(const char *path,
}
 }
 
-static int get_name(const char *path, const unsigned char *sha1, int flag, 
void *cb_data)
+static int get_name(const char *path, const struct object_id *oid, int flag, 
void *cb_data)
 {
int is_tag = starts_with(path, refs/tags/);
-   unsigned char peeled[20];
+   struct object_id peeled;
int is_annotated, prio;
 
/* Reject anything outside refs/tags/ unless --all */
@@ -134,10 +134,10 @@ static int get_name(const char *path, const unsigned char 
*sha1, int flag, void
return 0;
 
/* Is it annotated? */
-   if (!peel_ref(path, peeled)) {
-   is_annotated = !!hashcmp(sha1, peeled);
+   if (!peel_ref(path, peeled.hash)) {
+   is_annotated = !!oidcmp(oid, peeled);
} else {
-   hashcpy(peeled, sha1);
+   oidcpy(peeled, oid);
is_annotated = 0;
}
 
@@ -154,7 +154,7 @@ static int get_name(const char *path, const unsigned char 
*sha1, int flag, void
else
prio = 0;
 
-   add_to_known_names(all ? path + 5 : path + 10, peeled, prio, sha1);
+   add_to_known_names(all ? path + 5 : path + 10, peeled.hash, prio, 
oid-hash);
return 0;
 }
 
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 008513c..36dc48d 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -840,7 +840,7 @@ struct grab_ref_cbdata {
  * A call-back given to for_each_ref().  Filter refs and keep them for
  * later object processing.
  */
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int 
flag, void *cb_data)
+static int grab_single_ref(const char *refname, const struct object_id *oid, 
int flag, void *cb_data)
 {
struct grab_ref_cbdata *cb = cb_data;
struct refinfo *ref;
@@ -878,7 +878,7 @@ static int grab_single_ref(const char *refname, const 
unsigned char *sha1, int f
 */
ref = xcalloc(1, sizeof(*ref));
ref-refname = xstrdup(refname);
-   hashcpy(ref-objectname, sha1);
+   hashcpy(ref-objectname, oid-hash);
ref-flag = flag;
 
cnt = cb-grab_cnt;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index a27515a..05616a0 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -25,7 +25,7 @@ static int include_reflogs = 1;
 static int check_full = 1;
 static int check_strict;
 static int keep_cache_objects;
-static unsigned char head_sha1[20];
+static struct object_id head_oid;
 static const char *head_points_at;
 static int errors_found;
 static int write_lost_and_found;
@@ -482,13 +482,13 @@ static int fsck_handle_reflog(const char *logname, const 
unsigned char *sha1, in
return 0;
 }
 
-static int fsck_handle_ref(const char *refname, const unsigned char *sha1, int 
flag, void *cb_data)
+static int fsck_handle_ref(const char *refname, const struct object_id *oid, 
int flag, void *cb_data)
 {
struct object *obj;
 
-   obj = parse_object(sha1);
+   obj = parse_object(oid-hash);
if (!obj) {
-   error(%s: invalid sha1 pointer %s, refname, 
sha1_to_hex(sha1));
+   error(%s: invalid sha1 pointer %s, refname, oid_to_hex(oid));
errors_found |= ERROR_REACHABLE;
/* We'll continue with the rest despite the error

[PATCH 07/16] revision: remove unused _oid helper.

2015-03-20 Thread brian m. carlson
Now that all the callers of handle_refs are gone, rename handle_refs_oid
to handle_refs and update the callers accordingly.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 revision.c | 22 +++---
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/revision.c b/revision.c
index c2d8b1c..1fea8c5 100644
--- a/revision.c
+++ b/revision.c
@@ -1249,14 +1249,6 @@ void add_ref_exclusion(struct string_list 
**ref_excludes_p, const char *exclude)
 }
 
 static void handle_refs(const char *submodule, struct rev_info *revs, unsigned 
flags,
-   int (*for_each)(const char *, each_ref_fn, void *))
-{
-   struct all_refs_cb cb;
-   init_all_refs_cb(cb, revs, flags);
-   for_each(submodule, handle_one_ref, cb);
-}
-
-static void handle_refs_oid(const char *submodule, struct rev_info *revs, 
unsigned flags,
int (*for_each)(const char *, each_ref_fn_oid, void *))
 {
struct all_refs_cb cb;
@@ -2099,21 +2091,21 @@ static int handle_revision_pseudo_opt(const char 
*submodule,
 * register it in the list at the top of handle_revision_opt.
 */
if (!strcmp(arg, --all)) {
-   handle_refs_oid(submodule, revs, *flags, 
for_each_ref_submodule);
-   handle_refs_oid(submodule, revs, *flags, head_ref_submodule);
+   handle_refs(submodule, revs, *flags, for_each_ref_submodule);
+   handle_refs(submodule, revs, *flags, head_ref_submodule);
clear_ref_exclusion(revs-ref_excludes);
} else if (!strcmp(arg, --branches)) {
-   handle_refs_oid(submodule, revs, *flags, 
for_each_branch_ref_submodule);
+   handle_refs(submodule, revs, *flags, 
for_each_branch_ref_submodule);
clear_ref_exclusion(revs-ref_excludes);
} else if (!strcmp(arg, --bisect)) {
-   handle_refs_oid(submodule, revs, *flags, 
for_each_bad_bisect_ref);
-   handle_refs_oid(submodule, revs, *flags ^ (UNINTERESTING | 
BOTTOM), for_each_good_bisect_ref);
+   handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);
+   handle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), 
for_each_good_bisect_ref);
revs-bisect = 1;
} else if (!strcmp(arg, --tags)) {
-   handle_refs_oid(submodule, revs, *flags, 
for_each_tag_ref_submodule);
+   handle_refs(submodule, revs, *flags, 
for_each_tag_ref_submodule);
clear_ref_exclusion(revs-ref_excludes);
} else if (!strcmp(arg, --remotes)) {
-   handle_refs_oid(submodule, revs, *flags, 
for_each_remote_ref_submodule);
+   handle_refs(submodule, revs, *flags, 
for_each_remote_ref_submodule);
clear_ref_exclusion(revs-ref_excludes);
} else if ((argcount = parse_long_opt(glob, argv, optarg))) {
struct all_refs_cb cb;
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 02/16] refs: convert for_each_tag_ref to struct object_id

2015-03-20 Thread brian m. carlson
To allow piecemeal conversion of the for_each_*_ref functions, introduce
an additional typedef for a callback function that takes struct
object_id * instead of unsigned char *.  Provide an extra field in
struct ref_entry_cb for this callback and ensure at most one is set at a
time.  Temporarily suffix these new entries with _oid to distinguish
them.  Convert for_each_tag_ref and its callers to use the new _oid
functions, introducing temporary wrapper functions to avoid type
mismatches.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 builtin/pack-objects.c |  4 ++--
 builtin/rev-parse.c|  7 ++-
 builtin/tag.c  |  8 
 refs.c | 34 ++
 refs.h | 10 +-
 5 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d816587..65eb4fe 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -542,11 +542,11 @@ static enum write_one_status write_one(struct sha1file *f,
return WRITE_ONE_WRITTEN;
 }
 
-static int mark_tagged(const char *path, const unsigned char *sha1, int flag,
+static int mark_tagged(const char *path, const struct object_id *oid, int flag,
   void *cb_data)
 {
unsigned char peeled[20];
-   struct object_entry *entry = packlist_find(to_pack, sha1, NULL);
+   struct object_entry *entry = packlist_find(to_pack, oid-hash, NULL);
 
if (entry)
entry-tagged = 1;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 95328b8..ba5f3a0 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -198,6 +198,11 @@ static int show_reference(const char *refname, const 
unsigned char *sha1, int fl
return 0;
 }
 
+static int show_reference_oid(const char *refname, const struct object_id 
*oid, int flag, void *cb_data)
+{
+   return show_reference(refname, oid-hash, flag, cb_data);
+}
+
 static int anti_reference(const char *refname, const unsigned char *sha1, int 
flag, void *cb_data)
 {
show_rev(REVERSED, sha1, refname);
@@ -675,7 +680,7 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
continue;
}
if (!strcmp(arg, --tags)) {
-   for_each_tag_ref(show_reference, NULL);
+   for_each_tag_ref(show_reference_oid, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index e633f4e..b765da1 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -215,7 +215,7 @@ free_return:
free(buf);
 }
 
-static int show_reference(const char *refname, const unsigned char *sha1,
+static int show_reference(const char *refname, const struct object_id *oid,
  int flag, void *cb_data)
 {
struct tag_filter *filter = cb_data;
@@ -224,14 +224,14 @@ static int show_reference(const char *refname, const 
unsigned char *sha1,
if (filter-with_commit) {
struct commit *commit;
 
-   commit = lookup_commit_reference_gently(sha1, 1);
+   commit = lookup_commit_reference_gently(oid-hash, 1);
if (!commit)
return 0;
if (!contains(commit, filter-with_commit))
return 0;
}
 
-   if (points_at.nr  !match_points_at(refname, sha1))
+   if (points_at.nr  !match_points_at(refname, oid-hash))
return 0;
 
if (!filter-lines) {
@@ -242,7 +242,7 @@ static int show_reference(const char *refname, const 
unsigned char *sha1,
return 0;
}
printf(%-15s , refname);
-   show_tag_lines(sha1, filter-lines);
+   show_tag_lines(oid-hash, filter-lines);
putchar('\n');
}
 
diff --git a/refs.c b/refs.c
index 689a46d..2fe934f 100644
--- a/refs.c
+++ b/refs.c
@@ -669,6 +669,7 @@ struct ref_entry_cb {
int trim;
int flags;
each_ref_fn *fn;
+   each_ref_fn_oid *fn_oid;
void *cb_data;
 };
 
@@ -692,8 +693,13 @@ static int do_one_ref(struct ref_entry *entry, void 
*cb_data)
/* Store the old value, in case this is a recursive call: */
old_current_ref = current_ref;
current_ref = entry;
-   retval = data-fn(entry-name + data-trim, entry-u.value.oid.hash,
- entry-flag, data-cb_data);
+   if (data-fn_oid) {
+   retval = data-fn_oid(entry-name + data-trim, 
entry-u.value.oid,
+entry-flag, data-cb_data);
+   } else {
+   retval = data-fn(entry-name + data-trim, 
entry-u.value.oid.hash

[PATCH 03/16] refs: convert remaining users of for_each_ref_in to object_id

2015-03-20 Thread brian m. carlson
Remove the temporary for_each_ref_in_oid function and update the users
of it.  Convert the users of for_each_branch_ref and
for_each_remote_ref (which use for_each_ref_in under the hood) as well.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 bisect.c|  8 
 builtin/rev-parse.c | 10 +-
 refs.c  | 13 -
 refs.h  |  6 +++---
 4 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/bisect.c b/bisect.c
index 10f5e57..03d5cd9 100644
--- a/bisect.c
+++ b/bisect.c
@@ -400,16 +400,16 @@ struct commit_list *find_bisection(struct commit_list 
*list,
return best;
 }
 
-static int register_ref(const char *refname, const unsigned char *sha1,
+static int register_ref(const char *refname, const struct object_id *oid,
int flags, void *cb_data)
 {
if (!strcmp(refname, bad)) {
current_bad_oid = xmalloc(sizeof(*current_bad_oid));
-   hashcpy(current_bad_oid-hash, sha1);
+   oidcpy(current_bad_oid, oid);
} else if (starts_with(refname, good-)) {
-   sha1_array_append(good_revs, sha1);
+   sha1_array_append(good_revs, oid-hash);
} else if (starts_with(refname, skip-)) {
-   sha1_array_append(skipped_revs, sha1);
+   sha1_array_append(skipped_revs, oid-hash);
}
 
return 0;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index ba5f3a0..ec0ca86 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -203,9 +203,9 @@ static int show_reference_oid(const char *refname, const 
struct object_id *oid,
return show_reference(refname, oid-hash, flag, cb_data);
 }
 
-static int anti_reference(const char *refname, const unsigned char *sha1, int 
flag, void *cb_data)
+static int anti_reference(const char *refname, const struct object_id *oid, 
int flag, void *cb_data)
 {
-   show_rev(REVERSED, sha1, refname);
+   show_rev(REVERSED, oid-hash, refname);
return 0;
 }
 
@@ -658,7 +658,7 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
continue;
}
if (!strcmp(arg, --bisect)) {
-   for_each_ref_in(refs/bisect/bad, 
show_reference, NULL);
+   for_each_ref_in(refs/bisect/bad, 
show_reference_oid, NULL);
for_each_ref_in(refs/bisect/good, 
anti_reference, NULL);
continue;
}
@@ -669,7 +669,7 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
continue;
}
if (!strcmp(arg, --branches)) {
-   for_each_branch_ref(show_reference, NULL);
+   for_each_branch_ref(show_reference_oid, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
@@ -696,7 +696,7 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
continue;
}
if (!strcmp(arg, --remotes)) {
-   for_each_remote_ref(show_reference, NULL);
+   for_each_remote_ref(show_reference_oid, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
diff --git a/refs.c b/refs.c
index 2fe934f..2c7bbd4 100644
--- a/refs.c
+++ b/refs.c
@@ -1969,16 +1969,11 @@ int for_each_ref_submodule(const char *submodule, 
each_ref_fn fn, void *cb_data)
return do_for_each_ref(get_ref_cache(submodule), , fn, 0, 0, cb_data);
 }
 
-static int for_each_ref_in_oid(const char *prefix, each_ref_fn_oid fn, void 
*cb_data)
+int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
 {
return do_for_each_ref_oid(ref_cache, prefix, fn, strlen(prefix), 0, 
cb_data);
 }
 
-int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
-{
-   return do_for_each_ref(ref_cache, prefix, fn, strlen(prefix), 0, 
cb_data);
-}
-
 int for_each_ref_in_submodule(const char *submodule, const char *prefix,
each_ref_fn fn, void *cb_data)
 {
@@ -1987,7 +1982,7 @@ int for_each_ref_in_submodule(const char *submodule, 
const char *prefix,
 
 int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
 {
-   return for_each_ref_in_oid(refs/tags/, fn, cb_data);
+   return for_each_ref_in(refs/tags/, fn, cb_data);
 }
 
 int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
@@ -1995,7 +1990,7 @@ int for_each_tag_ref_submodule(const char *submodule, 
each_ref_fn fn, void *cb_d
return for_each_ref_in_submodule(submodule, refs/tags/, fn, cb_data);
 }
 
-int

Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails

2015-03-11 Thread brian m. carlson

On Wed, Mar 11, 2015 at 07:33:05PM +, Dan Langille (dalangil) wrote:

On Mar 10, 2015, at 6:29 PM, brian m. carlson sand...@crustytoothpaste.net 
wrote:
Does it work with a ticket if you specify a username, as in the
following URL?
https://b...@git.crustytoothpaste.net/git/bmc/homedir.git


Yes, that does work.  Our project is 98% of the way there now.

I looked at both libcurl and git environment variables to see if there
was a way to specify the user without putting it in the URL.  I didn’t see one.

My next step is the git configuration, either server or client.  Do you know
if I should stop looking now because it’s not there?


You might try looking at git config --help.  It looks like there's a
credential.username option that might do what you want.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v2 01/10] Define a structure for object IDs.

2015-03-12 Thread brian m. carlson

On Wed, Mar 11, 2015 at 05:26:56PM -0700, Junio C Hamano wrote:

brian m. carlson sand...@crustytoothpaste.net writes:


Michael Haggerty recommended that I call the structure element sha1
instead of oid in case we want to turn this into a union if we decide to
go the additional hash route.


I'd advise against it.


Yeah, after re-reading this, I think hash is the best choice for the
underlying element name.  That way, it's different from oid, which
would be the instances of the struct itself, and it's sufficiently
different from unconverted places in the code, which don't typically use
that term.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v2 01/10] Define a structure for object IDs.

2015-03-11 Thread brian m. carlson

On Tue, Mar 10, 2015 at 07:38:42PM -0700, Kyle J. McKay wrote:

GIT_SHA1_HEXSZ will always be exactly 2 * GIT_SHA1_RAWSZ, right?  In
fact, if it's not things will almost certainly break, yes?

Does it make more sense then to reflect this requirement by using:

 #define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)

instead?


Yes.  I'll make that change in the next version.


I don't see anything wrong with this.  However, in part 02/10 the
utility functions all use oid in their names, so I'm thinking that
it may make more sense to just go with:

struct object_id {
unsigned char oid[GIT_SHA1_RAWSZ];
};

to match?


Michael Haggerty recommended that I call the structure element sha1
instead of oid in case we want to turn this into a union if we decide to
go the additional hash route.

I think it can also improve readability if we use oid only for the
instances of the struct itself, especially since it makes it more
obvious what code has been converted already.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v2 04/10] archive.c: convert to use struct object_id

2015-03-11 Thread brian m. carlson

On Wed, Mar 11, 2015 at 03:20:08PM +0100, Michael Haggerty wrote:

On 03/08/2015 12:23 AM, brian m. carlson wrote:

 struct directory {
struct directory *up;
-   unsigned char sha1[20];
+   unsigned char sha1[GIT_SHA1_RAWSZ];


This is a local struct, and I think it is only a three-line change to
change the sha1 member to

   struct object_id oid;

Though perhaps you are delaying that change for some concrete reason.


No, I just overlooked it.  I'll fix that in the re-roll.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v2 01/10] Define a structure for object IDs.

2015-03-12 Thread brian m. carlson

On Thu, Mar 12, 2015 at 11:28:10AM +0100, Michael Haggerty wrote:

On 03/12/2015 01:26 AM, Junio C Hamano wrote:

And that would break the abstraction effort if you start calling the
field with a name that is specific to the underlying hash function.
The caller has to change o-sha1 to o-sha256 instead of keeping
that as o-oid and letting the callee handle the implementation
details when calling

if (!hashcmp(o1-oid, o2-oid))
; /* they are the same */
else
; /* they are different */
[...]


Hmm, I guess you imagine that we might sometimes pack SHA-1s, sometimes
SHA-256s (or whatever) in the oid field, which would be dimensioned
large enough for either one (with, say, SHA-1s padded with zeros).

I was imagining that this would evolve into a union (or maybe struct) of
different hash types, like

   struct object_id {
   unsigned char hash_type;
   union {
   unsigned char sha1[GIT_SHA1_RAWSZ];
   unsigned char sha256[GIT_SHA256_RAWSZ];
   } hash;
   };

BTW in either case, any hopes of mapping object_id objects directly on
top of buffer memory would disappear.


What I think might be more beneficial is to make the hash function
specific to a particular repository, and therefore you could maintain
something like this:

 struct object_id {
 unsigned char hash[GIT_MAX_RAWSZ];
 };

and make hash_type (or hash_length) a global[0].  I don't think it's
very worthwhile to try to mix two different hash functions in the same
repository, so we could still map directly onto buffer memory if we
decide that's portable enough.  I expect the cases where we need to do
that will be relatively limited.

Regardless, it seems that this solution has the most support (including
Junio's) and it's more self-documenting than my current set of patches,
so I'm going to go with it for now.  It should be easy to change if the
consensus goes back the other way.

[0] I personally think globals are a bit gross, but they don't seem to
have the problems that they would if git were a shared library.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] t7510: do not fail when gpg warns about insecure memory

2015-03-08 Thread brian m. carlson

On Sun, Mar 08, 2015 at 06:15:55PM -0400, Eric Sunshine wrote:

On Sun, Mar 8, 2015 at 6:04 PM, brian m. carlson
sand...@crustytoothpaste.net wrote:

Perhaps this is better?

 Unfortunately when running the test, that message is found in the  standard
output of git show -s --show-signature, but in the standard  error of git
verify-commit -v causing the comparisons of both standard  output and
standard error to fail.


That doesn't help me parse it any better.  It's the but without a
corresponding not which seems to be throwing me off. Typically, one
would write this but not that or not this but that. I can't tell
if there is a not missing or if the but is supposed to be an and
or if something else was intended.


The intended meaning is and with the additional sense of contrast. 
The sentence, if read with verbal emphasis, is, …is found in the 
standard *output* of git show -s --signature, but in the standard 
*error* of git verify-commit -v, thus demonstrating why the test fails: 
the pairs of output files don't match up.


Maybe you can suggest a less confusing wording.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: Please consider adding a -f switch to git-clone (or something similar)

2015-03-07 Thread brian m. carlson

On Sat, Mar 07, 2015 at 11:26:28PM +0100, Andreas Schwab wrote:

Diego Viola diego.vi...@gmail.com writes:


I know I could git-init in a empty directory


You can also git init a non-empty directory.


I have a script to set up a new throwaway VM with my dotfiles using git.
It looks a bit like the following ($BRANCH != master):

 SSH=ssh $DEST
 
 $SSH cd; $GIT init

 git push --receive-pack=$GIT-receive-pack $DEST:~/.git $BRANCH
 $SSH 
 $GIT pull . $BRANCH
 $GIT submodule update --init
 

It relies on the ability to git init a non-empty directory.  $BRANCH can 
be master if you use the new updateInstead functionality in git 2.3.0, 
and you can use git pull from a remote location instead of the push/pull 
pair if that suits you better.

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


[PATCH v2 06/10] bulk-checkin.c: convert to use struct object_id

2015-03-07 Thread brian m. carlson
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 bulk-checkin.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/bulk-checkin.c b/bulk-checkin.c
index 0c4b8a7..e50f60e 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -24,7 +24,7 @@ static struct bulk_checkin_state {
 
 static void finish_bulk_checkin(struct bulk_checkin_state *state)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
struct strbuf packname = STRBUF_INIT;
int i;
 
@@ -36,11 +36,11 @@ static void finish_bulk_checkin(struct bulk_checkin_state 
*state)
unlink(state-pack_tmp_name);
goto clear_exit;
} else if (state-nr_written == 1) {
-   sha1close(state-f, sha1, CSUM_FSYNC);
+   sha1close(state-f, oid.sha1, CSUM_FSYNC);
} else {
-   int fd = sha1close(state-f, sha1, 0);
-   fixup_pack_header_footer(fd, sha1, state-pack_tmp_name,
-state-nr_written, sha1,
+   int fd = sha1close(state-f, oid.sha1, 0);
+   fixup_pack_header_footer(fd, oid.sha1, state-pack_tmp_name,
+state-nr_written, oid.sha1,
 state-offset);
close(fd);
}
@@ -48,7 +48,7 @@ static void finish_bulk_checkin(struct bulk_checkin_state 
*state)
strbuf_addf(packname, %s/pack/pack-, get_object_directory());
finish_tmp_packfile(packname, state-pack_tmp_name,
state-written, state-nr_written,
-   state-pack_idx_opts, sha1);
+   state-pack_idx_opts, oid.sha1);
for (i = 0; i  state-nr_written; i++)
free(state-written[i]);
 
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 03/10] bisect.c: convert leaf functions to use struct object_id

2015-03-07 Thread brian m. carlson
Convert some constants to GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 bisect.c | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/bisect.c b/bisect.c
index 8c6d843..2692d54 100644
--- a/bisect.c
+++ b/bisect.c
@@ -15,7 +15,7 @@
 static struct sha1_array good_revs;
 static struct sha1_array skipped_revs;
 
-static unsigned char *current_bad_sha1;
+static struct object_id *current_bad_oid;
 
 static const char *argv_checkout[] = {checkout, -q, NULL, --, NULL};
 static const char *argv_show_branch[] = {show-branch, NULL, NULL};
@@ -404,8 +404,8 @@ static int register_ref(const char *refname, const unsigned 
char *sha1,
int flags, void *cb_data)
 {
if (!strcmp(refname, bad)) {
-   current_bad_sha1 = xmalloc(20);
-   hashcpy(current_bad_sha1, sha1);
+   current_bad_oid = xmalloc(sizeof(*current_bad_oid));
+   hashcpy(current_bad_oid-sha1, sha1);
} else if (starts_with(refname, good-)) {
sha1_array_append(good_revs, sha1);
} else if (starts_with(refname, skip-)) {
@@ -564,7 +564,7 @@ static struct commit_list *skip_away(struct commit_list 
*list, int count)
 
for (i = 0; cur; cur = cur-next, i++) {
if (i == index) {
-   if (hashcmp(cur-item-object.sha1, current_bad_sha1))
+   if (hashcmp(cur-item-object.sha1, 
current_bad_oid-sha1))
return cur;
if (previous)
return previous;
@@ -607,7 +607,7 @@ static void bisect_rev_setup(struct rev_info *revs, const 
char *prefix,
 
/* rev_argv.argv[0] will be ignored by setup_revisions */
argv_array_push(rev_argv, bisect_rev_setup);
-   argv_array_pushf(rev_argv, bad_format, sha1_to_hex(current_bad_sha1));
+   argv_array_pushf(rev_argv, bad_format, 
sha1_to_hex(current_bad_oid-sha1));
for (i = 0; i  good_revs.nr; i++)
argv_array_pushf(rev_argv, good_format,
 sha1_to_hex(good_revs.sha1[i]));
@@ -628,7 +628,7 @@ static void bisect_common(struct rev_info *revs)
 }
 
 static void exit_if_skipped_commits(struct commit_list *tried,
-   const unsigned char *bad)
+   const struct object_id *bad)
 {
if (!tried)
return;
@@ -637,12 +637,12 @@ static void exit_if_skipped_commits(struct commit_list 
*tried,
   The first bad commit could be any of:\n);
print_commit_list(tried, %s\n, %s\n);
if (bad)
-   printf(%s\n, sha1_to_hex(bad));
+   printf(%s\n, oid_to_hex(bad));
printf(We cannot bisect more!\n);
exit(2);
 }
 
-static int is_expected_rev(const unsigned char *sha1)
+static int is_expected_rev(const struct object_id *oid)
 {
const char *filename = git_path(BISECT_EXPECTED_REV);
struct stat st;
@@ -658,7 +658,7 @@ static int is_expected_rev(const unsigned char *sha1)
return 0;
 
if (strbuf_getline(str, fp, '\n') != EOF)
-   res = !strcmp(str.buf, sha1_to_hex(sha1));
+   res = !strcmp(str.buf, oid_to_hex(oid));
 
strbuf_release(str);
fclose(fp);
@@ -719,7 +719,7 @@ static struct commit **get_bad_and_good_commits(int *rev_nr)
struct commit **rev = xmalloc(len * sizeof(*rev));
int i, n = 0;
 
-   rev[n++] = get_commit_reference(current_bad_sha1);
+   rev[n++] = get_commit_reference(current_bad_oid-sha1);
for (i = 0; i  good_revs.nr; i++)
rev[n++] = get_commit_reference(good_revs.sha1[i]);
*rev_nr = n;
@@ -729,8 +729,8 @@ static struct commit **get_bad_and_good_commits(int *rev_nr)
 
 static void handle_bad_merge_base(void)
 {
-   if (is_expected_rev(current_bad_sha1)) {
-   char *bad_hex = sha1_to_hex(current_bad_sha1);
+   if (is_expected_rev(current_bad_oid)) {
+   char *bad_hex = oid_to_hex(current_bad_oid);
char *good_hex = join_sha1_array_hex(good_revs, ' ');
 
fprintf(stderr, The merge base %s is bad.\n
@@ -750,7 +750,7 @@ static void handle_bad_merge_base(void)
 static void handle_skipped_merge_base(const unsigned char *mb)
 {
char *mb_hex = sha1_to_hex(mb);
-   char *bad_hex = sha1_to_hex(current_bad_sha1);
+   char *bad_hex = sha1_to_hex(current_bad_oid-sha1);
char *good_hex = join_sha1_array_hex(good_revs, ' ');
 
warning(the merge base between %s and [%s] 
@@ -781,7 +781,7 @@ static void check_merge_bases(int no_checkout)
 
for (; result; result = result-next) {
const unsigned char *mb = result-item-object.sha1;
-   if (!hashcmp(mb, current_bad_sha1)) {
+   if (!hashcmp(mb, current_bad_oid-sha1

[PATCH v2 07/10] diff: convert struct combine_diff_path to object_id

2015-03-07 Thread brian m. carlson
Also, convert a constant to GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 combine-diff.c | 56 
 diff-lib.c | 10 +-
 diff.h |  5 +++--
 tree-diff.c| 10 +-
 4 files changed, 41 insertions(+), 40 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 91edce5..ec25202 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -44,9 +44,9 @@ static struct combine_diff_path *intersect_paths(struct 
combine_diff_path *curr,
memset(p-parent, 0,
   sizeof(p-parent[0]) * num_parent);
 
-   hashcpy(p-sha1, q-queue[i]-two-sha1);
+   hashcpy(p-oid.sha1, q-queue[i]-two-sha1);
p-mode = q-queue[i]-two-mode;
-   hashcpy(p-parent[n].sha1, q-queue[i]-one-sha1);
+   hashcpy(p-parent[n].oid.sha1, q-queue[i]-one-sha1);
p-parent[n].mode = q-queue[i]-one-mode;
p-parent[n].status = q-queue[i]-status;
*tail = p;
@@ -77,7 +77,7 @@ static struct combine_diff_path *intersect_paths(struct 
combine_diff_path *curr,
continue;
}
 
-   hashcpy(p-parent[n].sha1, q-queue[i]-one-sha1);
+   hashcpy(p-parent[n].oid.sha1, q-queue[i]-one-sha1);
p-parent[n].mode = q-queue[i]-one-mode;
p-parent[n].status = q-queue[i]-status;
 
@@ -284,7 +284,7 @@ static struct lline *coalesce_lines(struct lline *base, int 
*lenbase,
return base;
 }
 
-static char *grab_blob(const unsigned char *sha1, unsigned int mode,
+static char *grab_blob(const struct object_id *oid, unsigned int mode,
   unsigned long *size, struct userdiff_driver *textconv,
   const char *path)
 {
@@ -294,20 +294,20 @@ static char *grab_blob(const unsigned char *sha1, 
unsigned int mode,
if (S_ISGITLINK(mode)) {
blob = xmalloc(100);
*size = snprintf(blob, 100,
-Subproject commit %s\n, sha1_to_hex(sha1));
-   } else if (is_null_sha1(sha1)) {
+Subproject commit %s\n, oid_to_hex(oid));
+   } else if (is_null_oid(oid)) {
/* deleted blob */
*size = 0;
return xcalloc(1, 1);
} else if (textconv) {
struct diff_filespec *df = alloc_filespec(path);
-   fill_filespec(df, sha1, 1, mode);
+   fill_filespec(df, oid-sha1, 1, mode);
*size = fill_textconv(textconv, df, blob);
free_filespec(df);
} else {
-   blob = read_sha1_file(sha1, type, size);
+   blob = read_sha1_file(oid-sha1, type, size);
if (type != OBJ_BLOB)
-   die(object '%s' is not a blob!, sha1_to_hex(sha1));
+   die(object '%s' is not a blob!, oid_to_hex(oid));
}
return blob;
 }
@@ -389,7 +389,7 @@ static void consume_line(void *state_, char *line, unsigned 
long len)
}
 }
 
-static void combine_diff(const unsigned char *parent, unsigned int mode,
+static void combine_diff(const struct object_id *parent, unsigned int mode,
 mmfile_t *result_file,
 struct sline *sline, unsigned int cnt, int n,
 int num_parent, int result_deleted,
@@ -897,7 +897,7 @@ static void show_combined_header(struct combine_diff_path 
*elem,
 int show_file_header)
 {
struct diff_options *opt = rev-diffopt;
-   int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
+   int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? GIT_SHA1_HEXSZ : 
DEFAULT_ABBREV;
const char *a_prefix = opt-a_prefix ? opt-a_prefix : a/;
const char *b_prefix = opt-b_prefix ? opt-b_prefix : b/;
const char *c_meta = diff_get_color_opt(opt, DIFF_METAINFO);
@@ -914,11 +914,11 @@ static void show_combined_header(struct combine_diff_path 
*elem,
 , elem-path, line_prefix, c_meta, c_reset);
printf(%s%sindex , line_prefix, c_meta);
for (i = 0; i  num_parent; i++) {
-   abb = find_unique_abbrev(elem-parent[i].sha1,
+   abb = find_unique_abbrev(elem-parent[i].oid.sha1,
 abbrev);
printf(%s%s, i ? , : , abb);
}
-   abb = find_unique_abbrev(elem-sha1, abbrev);
+   abb = find_unique_abbrev(elem-oid.sha1, abbrev);
printf(..%s%s\n, abb, c_reset);
 
if (mode_differs) {
@@ -991,7 +991,7 @@ static void show_patch_diff(struct combine_diff_path *elem, 
int num_parent,
 
/* Read the result of merge first */
if (!working_tree_file)
-   result = grab_blob(elem-sha1

[PATCH v2 09/10] patch-id: convert to use struct object_id

2015-03-07 Thread brian m. carlson
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 builtin/patch-id.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 77db873..c208e7e 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -1,14 +1,14 @@
 #include builtin.h
 
-static void flush_current_id(int patchlen, unsigned char *id, unsigned char 
*result)
+static void flush_current_id(int patchlen, struct object_id *id, struct 
object_id *result)
 {
char name[50];
 
if (!patchlen)
return;
 
-   memcpy(name, sha1_to_hex(id), 41);
-   printf(%s %s\n, sha1_to_hex(result), name);
+   memcpy(name, oid_to_hex(id), GIT_SHA1_HEXSZ + 1);
+   printf(%s %s\n, oid_to_hex(result), name);
 }
 
 static int remove_space(char *line)
@@ -53,23 +53,23 @@ static int scan_hunk_header(const char *p, int *p_before, 
int *p_after)
return 1;
 }
 
-static void flush_one_hunk(unsigned char *result, git_SHA_CTX *ctx)
+static void flush_one_hunk(struct object_id *result, git_SHA_CTX *ctx)
 {
-   unsigned char hash[20];
+   unsigned char hash[GIT_SHA1_RAWSZ];
unsigned short carry = 0;
int i;
 
git_SHA1_Final(hash, ctx);
git_SHA1_Init(ctx);
/* 20-byte sum, with carry */
-   for (i = 0; i  20; ++i) {
-   carry += result[i] + hash[i];
-   result[i] = carry;
+   for (i = 0; i  GIT_SHA1_RAWSZ; ++i) {
+   carry += result-sha1[i] + hash[i];
+   result-sha1[i] = carry;
carry = 8;
}
 }
 
-static int get_one_patchid(unsigned char *next_sha1, unsigned char *result,
+static int get_one_patchid(struct object_id *next_oid, struct object_id 
*result,
   struct strbuf *line_buf, int stable)
 {
int patchlen = 0, found_next = 0;
@@ -77,7 +77,7 @@ static int get_one_patchid(unsigned char *next_sha1, unsigned 
char *result,
git_SHA_CTX ctx;
 
git_SHA1_Init(ctx);
-   hashclr(result);
+   oidclr(result);
 
while (strbuf_getwholeline(line_buf, stdin, '\n') != EOF) {
char *line = line_buf-buf;
@@ -93,7 +93,7 @@ static int get_one_patchid(unsigned char *next_sha1, unsigned 
char *result,
else if (!memcmp(line, \\ , 2)  12  strlen(line))
continue;
 
-   if (!get_sha1_hex(p, next_sha1)) {
+   if (!get_oid_hex(p, next_oid)) {
found_next = 1;
break;
}
@@ -143,7 +143,7 @@ static int get_one_patchid(unsigned char *next_sha1, 
unsigned char *result,
}
 
if (!found_next)
-   hashclr(next_sha1);
+   oidclr(next_oid);
 
flush_one_hunk(result, ctx);
 
@@ -152,15 +152,15 @@ static int get_one_patchid(unsigned char *next_sha1, 
unsigned char *result,
 
 static void generate_id_list(int stable)
 {
-   unsigned char sha1[20], n[20], result[20];
+   struct object_id oid, n, result;
int patchlen;
struct strbuf line_buf = STRBUF_INIT;
 
-   hashclr(sha1);
+   oidclr(oid);
while (!feof(stdin)) {
-   patchlen = get_one_patchid(n, result, line_buf, stable);
-   flush_current_id(patchlen, sha1, result);
-   hashcpy(sha1, n);
+   patchlen = get_one_patchid(n, result, line_buf, stable);
+   flush_current_id(patchlen, oid, result);
+   oidcpy(oid, n);
}
strbuf_release(line_buf);
 }
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 05/10] zip: use GIT_SHA1_HEXSZ for trailers

2015-03-07 Thread brian m. carlson
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 archive-zip.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/archive-zip.c b/archive-zip.c
index 4bde019..b669e50 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -427,12 +427,12 @@ static void write_zip_trailer(const unsigned char *sha1)
copy_le16(trailer.entries, zip_dir_entries);
copy_le32(trailer.size, zip_dir_offset);
copy_le32(trailer.offset, zip_offset);
-   copy_le16(trailer.comment_length, sha1 ? 40 : 0);
+   copy_le16(trailer.comment_length, sha1 ? GIT_SHA1_HEXSZ : 0);
 
write_or_die(1, zip_dir, zip_dir_offset);
write_or_die(1, trailer, ZIP_DIR_TRAILER_SIZE);
if (sha1)
-   write_or_die(1, sha1_to_hex(sha1), 40);
+   write_or_die(1, sha1_to_hex(sha1), GIT_SHA1_HEXSZ);
 }
 
 static void dos_time(time_t *time, int *dos_date, int *dos_time)
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 10/10] apply: convert threeway_stage to object_id

2015-03-07 Thread brian m. carlson
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 builtin/apply.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 65b97ee..75c5342 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -208,7 +208,7 @@ struct patch {
struct patch *next;
 
/* three-way fallback result */
-   unsigned char threeway_stage[3][20];
+   struct object_id threeway_stage[3];
 };
 
 static void free_fragment_list(struct fragment *list)
@@ -3424,11 +3424,11 @@ static int try_threeway(struct image *image, struct 
patch *patch,
if (status) {
patch-conflicted_threeway = 1;
if (patch-is_new)
-   hashclr(patch-threeway_stage[0]);
+   hashclr(patch-threeway_stage[0].sha1);
else
-   hashcpy(patch-threeway_stage[0], pre_sha1);
-   hashcpy(patch-threeway_stage[1], our_sha1);
-   hashcpy(patch-threeway_stage[2], post_sha1);
+   hashcpy(patch-threeway_stage[0].sha1, pre_sha1);
+   hashcpy(patch-threeway_stage[1].sha1, our_sha1);
+   hashcpy(patch-threeway_stage[2].sha1, post_sha1);
fprintf(stderr, Applied patch to '%s' with conflicts.\n, 
patch-new_name);
} else {
fprintf(stderr, Applied patch to '%s' cleanly.\n, 
patch-new_name);
@@ -4184,14 +4184,14 @@ static void add_conflicted_stages_file(struct patch 
*patch)
 
remove_file_from_cache(patch-new_name);
for (stage = 1; stage  4; stage++) {
-   if (is_null_sha1(patch-threeway_stage[stage - 1]))
+   if (is_null_oid(patch-threeway_stage[stage - 1]))
continue;
ce = xcalloc(1, ce_size);
memcpy(ce-name, patch-new_name, namelen);
ce-ce_mode = create_ce_mode(mode);
ce-ce_flags = create_ce_flags(stage);
ce-ce_namelen = namelen;
-   hashcpy(ce-sha1, patch-threeway_stage[stage - 1]);
+   hashcpy(ce-sha1, patch-threeway_stage[stage - 1].sha1);
if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD)  0)
die(_(unable to add cache entry for %s), 
patch-new_name);
}
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 08/10] commit: convert parts to struct object_id

2015-03-07 Thread brian m. carlson
Convert struct commit_graft and necessary local parts of commit.c.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 commit.c  | 32 
 commit.h  |  4 ++--
 log-tree.c|  2 +-
 send-pack.c   |  2 +-
 shallow.c |  8 
 upload-pack.c |  2 +-
 6 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/commit.c b/commit.c
index a8c7577..89c207e 100644
--- a/commit.c
+++ b/commit.c
@@ -55,12 +55,12 @@ struct commit *lookup_commit(const unsigned char *sha1)
 
 struct commit *lookup_commit_reference_by_name(const char *name)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
struct commit *commit;
 
-   if (get_sha1_committish(name, sha1))
+   if (get_sha1_committish(name, oid.sha1))
return NULL;
-   commit = lookup_commit_reference(sha1);
+   commit = lookup_commit_reference(oid.sha1);
if (parse_commit(commit))
return NULL;
return commit;
@@ -99,7 +99,7 @@ static int commit_graft_alloc, commit_graft_nr;
 static const unsigned char *commit_graft_sha1_access(size_t index, void *table)
 {
struct commit_graft **commit_graft_table = table;
-   return commit_graft_table[index]-sha1;
+   return commit_graft_table[index]-oid.sha1;
 }
 
 static int commit_graft_pos(const unsigned char *sha1)
@@ -110,7 +110,7 @@ static int commit_graft_pos(const unsigned char *sha1)
 
 int register_commit_graft(struct commit_graft *graft, int ignore_dups)
 {
-   int pos = commit_graft_pos(graft-sha1);
+   int pos = commit_graft_pos(graft-oid.sha1);
 
if (0 = pos) {
if (ignore_dups)
@@ -148,12 +148,12 @@ struct commit_graft *read_graft_line(char *buf, int len)
i = (len + 1) / 41 - 1;
graft = xmalloc(sizeof(*graft) + 20 * i);
graft-nr_parent = i;
-   if (get_sha1_hex(buf, graft-sha1))
+   if (get_oid_hex(buf, graft-oid))
goto bad_graft_data;
for (i = 40; i  len; i += 41) {
if (buf[i] != ' ')
goto bad_graft_data;
-   if (get_sha1_hex(buf + i + 1, graft-parent[i/41]))
+   if (get_sha1_hex(buf + i + 1, graft-parent[i/41].sha1))
goto bad_graft_data;
}
return graft;
@@ -302,7 +302,7 @@ int parse_commit_buffer(struct commit *item, const void 
*buffer, unsigned long s
 {
const char *tail = buffer;
const char *bufptr = buffer;
-   unsigned char parent[20];
+   struct object_id parent;
struct commit_list **pptr;
struct commit_graft *graft;
 
@@ -312,10 +312,10 @@ int parse_commit_buffer(struct commit *item, const void 
*buffer, unsigned long s
tail += size;
if (tail = bufptr + 46 || memcmp(bufptr, tree , 5) || bufptr[45] != 
'\n')
return error(bogus commit object %s, 
sha1_to_hex(item-object.sha1));
-   if (get_sha1_hex(bufptr + 5, parent)  0)
+   if (get_sha1_hex(bufptr + 5, parent.sha1)  0)
return error(bad tree pointer in commit %s,
 sha1_to_hex(item-object.sha1));
-   item-tree = lookup_tree(parent);
+   item-tree = lookup_tree(parent.sha1);
bufptr += 46; /* tree  + hex sha1 + \n */
pptr = item-parents;
 
@@ -324,7 +324,7 @@ int parse_commit_buffer(struct commit *item, const void 
*buffer, unsigned long s
struct commit *new_parent;
 
if (tail = bufptr + 48 ||
-   get_sha1_hex(bufptr + 7, parent) ||
+   get_sha1_hex(bufptr + 7, parent.sha1) ||
bufptr[47] != '\n')
return error(bad parents in commit %s, 
sha1_to_hex(item-object.sha1));
bufptr += 48;
@@ -334,7 +334,7 @@ int parse_commit_buffer(struct commit *item, const void 
*buffer, unsigned long s
 */
if (graft  (graft-nr_parent  0 || grafts_replace_parents))
continue;
-   new_parent = lookup_commit(parent);
+   new_parent = lookup_commit(parent.sha1);
if (new_parent)
pptr = commit_list_insert(new_parent, pptr)-next;
}
@@ -342,7 +342,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(graft-parent[i]);
+   new_parent = lookup_commit(graft-parent[i].sha1);
if (!new_parent)
continue;
pptr = commit_list_insert(new_parent, pptr)-next;
@@ -1580,10 +1580,10 @@ struct commit *get_merge_parent(const char *name)
 {
struct object *obj;
struct commit *commit;
-   unsigned char sha1[20];
-   if (get_sha1(name, sha1

[PATCH v2 04/10] archive.c: convert to use struct object_id

2015-03-07 Thread brian m. carlson
Convert a hard-coded 20 as well.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 archive.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/archive.c b/archive.c
index 96057ed..46d9025 100644
--- a/archive.c
+++ b/archive.c
@@ -101,7 +101,7 @@ static void setup_archive_check(struct git_attr_check 
*check)
 
 struct directory {
struct directory *up;
-   unsigned char sha1[20];
+   unsigned char sha1[GIT_SHA1_RAWSZ];
int baselen, len;
unsigned mode;
int stage;
@@ -354,7 +354,7 @@ static void parse_treeish_arg(const char **argv,
time_t archive_time;
struct tree *tree;
const struct commit *commit;
-   unsigned char sha1[20];
+   struct object_id oid;
 
/* Remotes are only allowed to fetch actual refs */
if (remote  !remote_allow_unreachable) {
@@ -362,15 +362,15 @@ static void parse_treeish_arg(const char **argv,
const char *colon = strchrnul(name, ':');
int refnamelen = colon - name;
 
-   if (!dwim_ref(name, refnamelen, sha1, ref))
+   if (!dwim_ref(name, refnamelen, oid.sha1, ref))
die(no such ref: %.*s, refnamelen, name);
free(ref);
}
 
-   if (get_sha1(name, sha1))
+   if (get_sha1(name, oid.sha1))
die(Not a valid object name);
 
-   commit = lookup_commit_reference_gently(sha1, 1);
+   commit = lookup_commit_reference_gently(oid.sha1, 1);
if (commit) {
commit_sha1 = commit-object.sha1;
archive_time = commit-date;
@@ -379,21 +379,21 @@ static void parse_treeish_arg(const char **argv,
archive_time = time(NULL);
}
 
-   tree = parse_tree_indirect(sha1);
+   tree = parse_tree_indirect(oid.sha1);
if (tree == NULL)
die(not a tree object);
 
if (prefix) {
-   unsigned char tree_sha1[20];
+   struct object_id tree_oid;
unsigned int mode;
int err;
 
err = get_tree_entry(tree-object.sha1, prefix,
-tree_sha1, mode);
+tree_oid.sha1, mode);
if (err || !S_ISDIR(mode))
die(current working directory is untracked);
 
-   tree = parse_tree_indirect(tree_sha1);
+   tree = parse_tree_indirect(tree_oid.sha1);
}
ar_args-tree = tree;
ar_args-commit_sha1 = commit_sha1;
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 02/10] Define utility functions for object IDs.

2015-03-07 Thread brian m. carlson
There are several utility functions (hashcmp and friends) that are used
for comparing object IDs (SHA-1 values).  Using these functions, which
take pointers to unsigned char, with struct object_id requires tiresome
access to the sha1 member, which bloats code and violates the desired
encapsulation.  Provide wrappers around these functions for struct
object_id for neater, more maintainable code.  Use the new constants to
avoid the hard-coded 20s and 40s throughout the original functions.

These functions simply call the underlying pointer-to-unsigned-char
versions to ensure that any performance improvements will be passed
through to the new functions.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
I'm not very excited about having to put the #include in the middle of
cache.h.  The alternative, of course, is to move enum object_type up,
which I can do if necessary.  Another alternative is to simply move the
struct object_id definitions to cache.h instead of object.h, which might
be cleaner.

I'm interested in hearing opinions about the best way to go forward.

 cache.h | 37 +
 hex.c   | 16 +---
 2 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/cache.h b/cache.h
index 761c570..f9addcc 100644
--- a/cache.h
+++ b/cache.h
@@ -368,6 +368,11 @@ static inline enum object_type object_type(unsigned int 
mode)
OBJ_BLOB;
 }
 
+/* This must go before the oid* functions, but after the declaration of enum
+ * object_type.
+ */
+#include object.h
+
 /* Double-check local_repo_env below if you add to this list. */
 #define GIT_DIR_ENVIRONMENT GIT_DIR
 #define GIT_NAMESPACE_ENVIRONMENT GIT_NAMESPACE
@@ -710,13 +715,13 @@ extern char *sha1_pack_name(const unsigned char *sha1);
 extern char *sha1_pack_index_name(const unsigned char *sha1);
 
 extern const char *find_unique_abbrev(const unsigned char *sha1, int);
-extern const unsigned char null_sha1[20];
+extern const unsigned char null_sha1[GIT_SHA1_RAWSZ];
 
 static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)
 {
int i;
 
-   for (i = 0; i  20; i++, sha1++, sha2++) {
+   for (i = 0; i  GIT_SHA1_RAWSZ; i++, sha1++, sha2++) {
if (*sha1 != *sha2)
return *sha1 - *sha2;
}
@@ -724,20 +729,42 @@ static inline int hashcmp(const unsigned char *sha1, 
const unsigned char *sha2)
return 0;
 }
 
+static inline int oidcmp(const struct object_id *o1, const struct object_id 
*o2)
+{
+   return hashcmp(o1-sha1, o2-sha1);
+}
+
 static inline int is_null_sha1(const unsigned char *sha1)
 {
return !hashcmp(sha1, null_sha1);
 }
 
+static inline int is_null_oid(const struct object_id *oid)
+{
+   return !hashcmp(oid-sha1, null_sha1);
+}
+
 static inline void hashcpy(unsigned char *sha_dst, const unsigned char 
*sha_src)
 {
-   memcpy(sha_dst, sha_src, 20);
+   memcpy(sha_dst, sha_src, GIT_SHA1_RAWSZ);
 }
+
+static inline void oidcpy(struct object_id *dst, const struct object_id *src)
+{
+   hashcpy(dst-sha1, src-sha1);
+}
+
 static inline void hashclr(unsigned char *hash)
 {
-   memset(hash, 0, 20);
+   memset(hash, 0, GIT_SHA1_RAWSZ);
 }
 
+static inline void oidclr(struct object_id *oid)
+{
+   hashclr(oid-sha1);
+}
+
+
 #define EMPTY_TREE_SHA1_HEX \
4b825dc642cb6eb9a060e54bf8d69288fbee4904
 #define EMPTY_TREE_SHA1_BIN_LITERAL \
@@ -944,8 +971,10 @@ extern int for_each_abbrev(const char *prefix, 
each_abbrev_fn, void *);
  * null-terminated string.
  */
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
+extern int get_oid_hex(const char *hex, struct object_id *sha1);
 
 extern char *sha1_to_hex(const unsigned char *sha1);   /* static buffer 
result! */
+extern char *oid_to_hex(const struct object_id *oid);  /* same static buffer 
as sha1_to_hex */
 extern int read_ref_full(const char *refname, int resolve_flags,
 unsigned char *sha1, int *flags);
 extern int read_ref(const char *refname, unsigned char *sha1);
diff --git a/hex.c b/hex.c
index cfd9d72..7e25a88 100644
--- a/hex.c
+++ b/hex.c
@@ -38,7 +38,7 @@ const signed char hexval_table[256] = {
 int get_sha1_hex(const char *hex, unsigned char *sha1)
 {
int i;
-   for (i = 0; i  20; i++) {
+   for (i = 0; i  GIT_SHA1_RAWSZ; i++) {
unsigned int val;
/*
 * hex[1]=='\0' is caught when val is checked below,
@@ -56,15 +56,20 @@ int get_sha1_hex(const char *hex, unsigned char *sha1)
return 0;
 }
 
+int get_oid_hex(const char *hex, struct object_id *oid)
+{
+   return get_sha1_hex(hex, oid-sha1);
+}
+
 char *sha1_to_hex(const unsigned char *sha1)
 {
static int bufno;
-   static char hexbuffer[4][41];
+   static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];
static const char hex[] = 0123456789abcdef;
char *buffer = hexbuffer[3  ++bufno], *buf = buffer;
int i

[PATCH v2 01/10] Define a structure for object IDs.

2015-03-07 Thread brian m. carlson
Many places throughout the code use unsigned char [20] to store object IDs
(SHA-1 values).  This leads to lots of hardcoded numbers throughout the
codebase.  It also leads to confusion about the purposes of a buffer.

Introduce a structure for object IDs.  This allows us to obtain the benefits
of compile-time checking for misuse.  The structure is expected to remain
the same size and have the same alignment requirements on all known
platforms, compared to the array of unsigned char, although this is not
required for correctness.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 object.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/object.h b/object.h
index 6416247..f99b502 100644
--- a/object.h
+++ b/object.h
@@ -1,6 +1,14 @@
 #ifndef OBJECT_H
 #define OBJECT_H
 
+/* The length in bytes and in hex digits of an object name (SHA-1 value). */
+#define GIT_SHA1_RAWSZ 20
+#define GIT_SHA1_HEXSZ 40
+
+struct object_id {
+   unsigned char sha1[GIT_SHA1_RAWSZ];
+};
+
 struct object_list {
struct object *item;
struct object_list *next;
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 00/10] Use a structure for object IDs.

2015-03-07 Thread brian m. carlson
This is a patch series to convert some of the relevant uses of unsigned
char [20] to struct object_id.

The goal of this series to improve type-checking in the codebase and to
make it easier to move to a different hash function if the project
decides to do that.  This series does not convert all of the codebase,
but only parts.  I've dropped some of the patches from earlier (which no
longer apply) and added others.

Certain parts of the code have to be converted before others to keep the
patch sizes small, maintainable, and bisectable, so functions and
structures that are used across the codebase (e.g. struct object) will
be converted later.  Conversion has been done in a somewhat haphazard
manner by converting modules with leaf functions and less commonly used
structs first.

Since part of the goal is to ease a move to a different hash function if
the project decides to do that, I've adopted Michael Haggerty's
suggestion of using variables named oid, naming the structure member
sha1, and using GIT_SHA1_RAWSZ and GIT_SHA1_HEXSZ as the constants.

I've been holding on to this series for a long time in hopes of
converting more of the code before submitting, but realized that this
deprived others of the ability to use the new structure and required me
to rebase extremely frequently.

brian m. carlson (10):
  Define a structure for object IDs.
  Define utility functions for object IDs.
  bisect.c: convert leaf functions to use struct object_id
  archive.c: convert to use struct object_id
  zip: use GIT_SHA1_HEXSZ for trailers
  bulk-checkin.c: convert to use struct object_id
  diff: convert struct combine_diff_path to object_id
  commit: convert parts to struct object_id
  patch-id: convert to use struct object_id
  apply: convert threeway_stage to object_id

 archive-zip.c  |  4 ++--
 archive.c  | 18 +-
 bisect.c   | 40 +++---
 builtin/apply.c| 14 +++---
 builtin/patch-id.c | 34 -
 bulk-checkin.c | 12 ++--
 cache.h| 37 
 combine-diff.c | 56 +++---
 commit.c   | 32 +++
 commit.h   |  4 ++--
 diff-lib.c | 10 +-
 diff.h |  5 +++--
 hex.c  | 16 +---
 log-tree.c |  2 +-
 object.h   |  8 
 send-pack.c|  2 +-
 shallow.c  |  8 
 tree-diff.c| 10 +-
 upload-pack.c  |  2 +-
 19 files changed, 181 insertions(+), 133 deletions(-)

-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 07/10] diff: convert struct combine_diff_path to object_id

2015-03-13 Thread brian m. carlson
Also, convert a constant to GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 combine-diff.c | 56 
 diff-lib.c | 10 +-
 diff.h |  5 +++--
 tree-diff.c| 10 +-
 4 files changed, 41 insertions(+), 40 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 91edce5..8eb7278 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -44,9 +44,9 @@ static struct combine_diff_path *intersect_paths(struct 
combine_diff_path *curr,
memset(p-parent, 0,
   sizeof(p-parent[0]) * num_parent);
 
-   hashcpy(p-sha1, q-queue[i]-two-sha1);
+   hashcpy(p-oid.hash, q-queue[i]-two-sha1);
p-mode = q-queue[i]-two-mode;
-   hashcpy(p-parent[n].sha1, q-queue[i]-one-sha1);
+   hashcpy(p-parent[n].oid.hash, q-queue[i]-one-sha1);
p-parent[n].mode = q-queue[i]-one-mode;
p-parent[n].status = q-queue[i]-status;
*tail = p;
@@ -77,7 +77,7 @@ static struct combine_diff_path *intersect_paths(struct 
combine_diff_path *curr,
continue;
}
 
-   hashcpy(p-parent[n].sha1, q-queue[i]-one-sha1);
+   hashcpy(p-parent[n].oid.hash, q-queue[i]-one-sha1);
p-parent[n].mode = q-queue[i]-one-mode;
p-parent[n].status = q-queue[i]-status;
 
@@ -284,7 +284,7 @@ static struct lline *coalesce_lines(struct lline *base, int 
*lenbase,
return base;
 }
 
-static char *grab_blob(const unsigned char *sha1, unsigned int mode,
+static char *grab_blob(const struct object_id *oid, unsigned int mode,
   unsigned long *size, struct userdiff_driver *textconv,
   const char *path)
 {
@@ -294,20 +294,20 @@ static char *grab_blob(const unsigned char *sha1, 
unsigned int mode,
if (S_ISGITLINK(mode)) {
blob = xmalloc(100);
*size = snprintf(blob, 100,
-Subproject commit %s\n, sha1_to_hex(sha1));
-   } else if (is_null_sha1(sha1)) {
+Subproject commit %s\n, oid_to_hex(oid));
+   } else if (is_null_oid(oid)) {
/* deleted blob */
*size = 0;
return xcalloc(1, 1);
} else if (textconv) {
struct diff_filespec *df = alloc_filespec(path);
-   fill_filespec(df, sha1, 1, mode);
+   fill_filespec(df, oid-hash, 1, mode);
*size = fill_textconv(textconv, df, blob);
free_filespec(df);
} else {
-   blob = read_sha1_file(sha1, type, size);
+   blob = read_sha1_file(oid-hash, type, size);
if (type != OBJ_BLOB)
-   die(object '%s' is not a blob!, sha1_to_hex(sha1));
+   die(object '%s' is not a blob!, oid_to_hex(oid));
}
return blob;
 }
@@ -389,7 +389,7 @@ static void consume_line(void *state_, char *line, unsigned 
long len)
}
 }
 
-static void combine_diff(const unsigned char *parent, unsigned int mode,
+static void combine_diff(const struct object_id *parent, unsigned int mode,
 mmfile_t *result_file,
 struct sline *sline, unsigned int cnt, int n,
 int num_parent, int result_deleted,
@@ -897,7 +897,7 @@ static void show_combined_header(struct combine_diff_path 
*elem,
 int show_file_header)
 {
struct diff_options *opt = rev-diffopt;
-   int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
+   int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? GIT_SHA1_HEXSZ : 
DEFAULT_ABBREV;
const char *a_prefix = opt-a_prefix ? opt-a_prefix : a/;
const char *b_prefix = opt-b_prefix ? opt-b_prefix : b/;
const char *c_meta = diff_get_color_opt(opt, DIFF_METAINFO);
@@ -914,11 +914,11 @@ static void show_combined_header(struct combine_diff_path 
*elem,
 , elem-path, line_prefix, c_meta, c_reset);
printf(%s%sindex , line_prefix, c_meta);
for (i = 0; i  num_parent; i++) {
-   abb = find_unique_abbrev(elem-parent[i].sha1,
+   abb = find_unique_abbrev(elem-parent[i].oid.hash,
 abbrev);
printf(%s%s, i ? , : , abb);
}
-   abb = find_unique_abbrev(elem-sha1, abbrev);
+   abb = find_unique_abbrev(elem-oid.hash, abbrev);
printf(..%s%s\n, abb, c_reset);
 
if (mode_differs) {
@@ -991,7 +991,7 @@ static void show_patch_diff(struct combine_diff_path *elem, 
int num_parent,
 
/* Read the result of merge first */
if (!working_tree_file)
-   result = grab_blob(elem-sha1

[PATCH v2 04/10] archive.c: convert to use struct object_id

2015-03-13 Thread brian m. carlson
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 archive.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/archive.c b/archive.c
index 96057ed..d37c41d 100644
--- a/archive.c
+++ b/archive.c
@@ -101,7 +101,7 @@ static void setup_archive_check(struct git_attr_check 
*check)
 
 struct directory {
struct directory *up;
-   unsigned char sha1[20];
+   struct object_id oid;
int baselen, len;
unsigned mode;
int stage;
@@ -177,7 +177,7 @@ static void queue_directory(const unsigned char *sha1,
d-stage   = stage;
c-bottom  = d;
d-len = sprintf(d-path, %.*s%s/, (int)base-len, base-buf, 
filename);
-   hashcpy(d-sha1, sha1);
+   hashcpy(d-oid.hash, sha1);
 }
 
 static int write_directory(struct archiver_context *c)
@@ -191,7 +191,7 @@ static int write_directory(struct archiver_context *c)
d-path[d-len - 1] = '\0'; /* no trailing slash */
ret =
write_directory(c) ||
-   write_archive_entry(d-sha1, d-path, d-baselen,
+   write_archive_entry(d-oid.hash, d-path, d-baselen,
d-path + d-baselen, d-mode,
d-stage, c) != READ_TREE_RECURSIVE;
free(d);
@@ -354,7 +354,7 @@ static void parse_treeish_arg(const char **argv,
time_t archive_time;
struct tree *tree;
const struct commit *commit;
-   unsigned char sha1[20];
+   struct object_id oid;
 
/* Remotes are only allowed to fetch actual refs */
if (remote  !remote_allow_unreachable) {
@@ -362,15 +362,15 @@ static void parse_treeish_arg(const char **argv,
const char *colon = strchrnul(name, ':');
int refnamelen = colon - name;
 
-   if (!dwim_ref(name, refnamelen, sha1, ref))
+   if (!dwim_ref(name, refnamelen, oid.hash, ref))
die(no such ref: %.*s, refnamelen, name);
free(ref);
}
 
-   if (get_sha1(name, sha1))
+   if (get_sha1(name, oid.hash))
die(Not a valid object name);
 
-   commit = lookup_commit_reference_gently(sha1, 1);
+   commit = lookup_commit_reference_gently(oid.hash, 1);
if (commit) {
commit_sha1 = commit-object.sha1;
archive_time = commit-date;
@@ -379,21 +379,21 @@ static void parse_treeish_arg(const char **argv,
archive_time = time(NULL);
}
 
-   tree = parse_tree_indirect(sha1);
+   tree = parse_tree_indirect(oid.hash);
if (tree == NULL)
die(not a tree object);
 
if (prefix) {
-   unsigned char tree_sha1[20];
+   struct object_id tree_oid;
unsigned int mode;
int err;
 
err = get_tree_entry(tree-object.sha1, prefix,
-tree_sha1, mode);
+tree_oid.hash, mode);
if (err || !S_ISDIR(mode))
die(current working directory is untracked);
 
-   tree = parse_tree_indirect(tree_sha1);
+   tree = parse_tree_indirect(tree_oid.hash);
}
ar_args-tree = tree;
ar_args-commit_sha1 = commit_sha1;
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 03/10] bisect.c: convert leaf functions to use struct object_id

2015-03-13 Thread brian m. carlson
Convert some constants to GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 bisect.c | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/bisect.c b/bisect.c
index 8c6d843..10f5e57 100644
--- a/bisect.c
+++ b/bisect.c
@@ -15,7 +15,7 @@
 static struct sha1_array good_revs;
 static struct sha1_array skipped_revs;
 
-static unsigned char *current_bad_sha1;
+static struct object_id *current_bad_oid;
 
 static const char *argv_checkout[] = {checkout, -q, NULL, --, NULL};
 static const char *argv_show_branch[] = {show-branch, NULL, NULL};
@@ -404,8 +404,8 @@ static int register_ref(const char *refname, const unsigned 
char *sha1,
int flags, void *cb_data)
 {
if (!strcmp(refname, bad)) {
-   current_bad_sha1 = xmalloc(20);
-   hashcpy(current_bad_sha1, sha1);
+   current_bad_oid = xmalloc(sizeof(*current_bad_oid));
+   hashcpy(current_bad_oid-hash, sha1);
} else if (starts_with(refname, good-)) {
sha1_array_append(good_revs, sha1);
} else if (starts_with(refname, skip-)) {
@@ -564,7 +564,7 @@ static struct commit_list *skip_away(struct commit_list 
*list, int count)
 
for (i = 0; cur; cur = cur-next, i++) {
if (i == index) {
-   if (hashcmp(cur-item-object.sha1, current_bad_sha1))
+   if (hashcmp(cur-item-object.sha1, 
current_bad_oid-hash))
return cur;
if (previous)
return previous;
@@ -607,7 +607,7 @@ static void bisect_rev_setup(struct rev_info *revs, const 
char *prefix,
 
/* rev_argv.argv[0] will be ignored by setup_revisions */
argv_array_push(rev_argv, bisect_rev_setup);
-   argv_array_pushf(rev_argv, bad_format, sha1_to_hex(current_bad_sha1));
+   argv_array_pushf(rev_argv, bad_format, oid_to_hex(current_bad_oid));
for (i = 0; i  good_revs.nr; i++)
argv_array_pushf(rev_argv, good_format,
 sha1_to_hex(good_revs.sha1[i]));
@@ -628,7 +628,7 @@ static void bisect_common(struct rev_info *revs)
 }
 
 static void exit_if_skipped_commits(struct commit_list *tried,
-   const unsigned char *bad)
+   const struct object_id *bad)
 {
if (!tried)
return;
@@ -637,12 +637,12 @@ static void exit_if_skipped_commits(struct commit_list 
*tried,
   The first bad commit could be any of:\n);
print_commit_list(tried, %s\n, %s\n);
if (bad)
-   printf(%s\n, sha1_to_hex(bad));
+   printf(%s\n, oid_to_hex(bad));
printf(We cannot bisect more!\n);
exit(2);
 }
 
-static int is_expected_rev(const unsigned char *sha1)
+static int is_expected_rev(const struct object_id *oid)
 {
const char *filename = git_path(BISECT_EXPECTED_REV);
struct stat st;
@@ -658,7 +658,7 @@ static int is_expected_rev(const unsigned char *sha1)
return 0;
 
if (strbuf_getline(str, fp, '\n') != EOF)
-   res = !strcmp(str.buf, sha1_to_hex(sha1));
+   res = !strcmp(str.buf, oid_to_hex(oid));
 
strbuf_release(str);
fclose(fp);
@@ -719,7 +719,7 @@ static struct commit **get_bad_and_good_commits(int *rev_nr)
struct commit **rev = xmalloc(len * sizeof(*rev));
int i, n = 0;
 
-   rev[n++] = get_commit_reference(current_bad_sha1);
+   rev[n++] = get_commit_reference(current_bad_oid-hash);
for (i = 0; i  good_revs.nr; i++)
rev[n++] = get_commit_reference(good_revs.sha1[i]);
*rev_nr = n;
@@ -729,8 +729,8 @@ static struct commit **get_bad_and_good_commits(int *rev_nr)
 
 static void handle_bad_merge_base(void)
 {
-   if (is_expected_rev(current_bad_sha1)) {
-   char *bad_hex = sha1_to_hex(current_bad_sha1);
+   if (is_expected_rev(current_bad_oid)) {
+   char *bad_hex = oid_to_hex(current_bad_oid);
char *good_hex = join_sha1_array_hex(good_revs, ' ');
 
fprintf(stderr, The merge base %s is bad.\n
@@ -750,7 +750,7 @@ static void handle_bad_merge_base(void)
 static void handle_skipped_merge_base(const unsigned char *mb)
 {
char *mb_hex = sha1_to_hex(mb);
-   char *bad_hex = sha1_to_hex(current_bad_sha1);
+   char *bad_hex = sha1_to_hex(current_bad_oid-hash);
char *good_hex = join_sha1_array_hex(good_revs, ' ');
 
warning(the merge base between %s and [%s] 
@@ -781,7 +781,7 @@ static void check_merge_bases(int no_checkout)
 
for (; result; result = result-next) {
const unsigned char *mb = result-item-object.sha1;
-   if (!hashcmp(mb, current_bad_sha1)) {
+   if (!hashcmp(mb, current_bad_oid-hash

[PATCH v2 10/10] apply: convert threeway_stage to object_id

2015-03-13 Thread brian m. carlson
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 builtin/apply.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 65b97ee..c2c8f39 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -208,7 +208,7 @@ struct patch {
struct patch *next;
 
/* three-way fallback result */
-   unsigned char threeway_stage[3][20];
+   struct object_id threeway_stage[3];
 };
 
 static void free_fragment_list(struct fragment *list)
@@ -3424,11 +3424,11 @@ static int try_threeway(struct image *image, struct 
patch *patch,
if (status) {
patch-conflicted_threeway = 1;
if (patch-is_new)
-   hashclr(patch-threeway_stage[0]);
+   oidclr(patch-threeway_stage[0]);
else
-   hashcpy(patch-threeway_stage[0], pre_sha1);
-   hashcpy(patch-threeway_stage[1], our_sha1);
-   hashcpy(patch-threeway_stage[2], post_sha1);
+   hashcpy(patch-threeway_stage[0].hash, pre_sha1);
+   hashcpy(patch-threeway_stage[1].hash, our_sha1);
+   hashcpy(patch-threeway_stage[2].hash, post_sha1);
fprintf(stderr, Applied patch to '%s' with conflicts.\n, 
patch-new_name);
} else {
fprintf(stderr, Applied patch to '%s' cleanly.\n, 
patch-new_name);
@@ -4184,14 +4184,14 @@ static void add_conflicted_stages_file(struct patch 
*patch)
 
remove_file_from_cache(patch-new_name);
for (stage = 1; stage  4; stage++) {
-   if (is_null_sha1(patch-threeway_stage[stage - 1]))
+   if (is_null_oid(patch-threeway_stage[stage - 1]))
continue;
ce = xcalloc(1, ce_size);
memcpy(ce-name, patch-new_name, namelen);
ce-ce_mode = create_ce_mode(mode);
ce-ce_flags = create_ce_flags(stage);
ce-ce_namelen = namelen;
-   hashcpy(ce-sha1, patch-threeway_stage[stage - 1]);
+   hashcpy(ce-sha1, patch-threeway_stage[stage - 1].hash);
if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD)  0)
die(_(unable to add cache entry for %s), 
patch-new_name);
}
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 02/10] Define utility functions for object IDs.

2015-03-13 Thread brian m. carlson
There are several utility functions (hashcmp and friends) that are used
for comparing object IDs (SHA-1 values).  Using these functions, which
take pointers to unsigned char, with struct object_id requires tiresome
access to the sha1 member, which bloats code and violates the desired
encapsulation.  Provide wrappers around these functions for struct
object_id for neater, more maintainable code.  Use the new constants to
avoid the hard-coded 20s and 40s throughout the original functions.

These functions simply call the underlying pointer-to-unsigned-char
versions to ensure that any performance improvements will be passed
through to the new functions.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 cache.h | 32 
 hex.c   | 16 +---
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/cache.h b/cache.h
index 6582c35..95206a3 100644
--- a/cache.h
+++ b/cache.h
@@ -718,13 +718,13 @@ extern char *sha1_pack_name(const unsigned char *sha1);
 extern char *sha1_pack_index_name(const unsigned char *sha1);
 
 extern const char *find_unique_abbrev(const unsigned char *sha1, int);
-extern const unsigned char null_sha1[20];
+extern const unsigned char null_sha1[GIT_SHA1_RAWSZ];
 
 static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)
 {
int i;
 
-   for (i = 0; i  20; i++, sha1++, sha2++) {
+   for (i = 0; i  GIT_SHA1_RAWSZ; i++, sha1++, sha2++) {
if (*sha1 != *sha2)
return *sha1 - *sha2;
}
@@ -732,20 +732,42 @@ static inline int hashcmp(const unsigned char *sha1, 
const unsigned char *sha2)
return 0;
 }
 
+static inline int oidcmp(const struct object_id *oid1, const struct object_id 
*oid2)
+{
+   return hashcmp(oid1-hash, oid2-hash);
+}
+
 static inline int is_null_sha1(const unsigned char *sha1)
 {
return !hashcmp(sha1, null_sha1);
 }
 
+static inline int is_null_oid(const struct object_id *oid)
+{
+   return !hashcmp(oid-hash, null_sha1);
+}
+
 static inline void hashcpy(unsigned char *sha_dst, const unsigned char 
*sha_src)
 {
-   memcpy(sha_dst, sha_src, 20);
+   memcpy(sha_dst, sha_src, GIT_SHA1_RAWSZ);
 }
+
+static inline void oidcpy(struct object_id *dst, const struct object_id *src)
+{
+   hashcpy(dst-hash, src-hash);
+}
+
 static inline void hashclr(unsigned char *hash)
 {
-   memset(hash, 0, 20);
+   memset(hash, 0, GIT_SHA1_RAWSZ);
 }
 
+static inline void oidclr(struct object_id *oid)
+{
+   hashclr(oid-hash);
+}
+
+
 #define EMPTY_TREE_SHA1_HEX \
4b825dc642cb6eb9a060e54bf8d69288fbee4904
 #define EMPTY_TREE_SHA1_BIN_LITERAL \
@@ -952,8 +974,10 @@ extern int for_each_abbrev(const char *prefix, 
each_abbrev_fn, void *);
  * null-terminated string.
  */
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
+extern int get_oid_hex(const char *hex, struct object_id *sha1);
 
 extern char *sha1_to_hex(const unsigned char *sha1);   /* static buffer 
result! */
+extern char *oid_to_hex(const struct object_id *oid);  /* same static buffer 
as sha1_to_hex */
 extern int read_ref_full(const char *refname, int resolve_flags,
 unsigned char *sha1, int *flags);
 extern int read_ref(const char *refname, unsigned char *sha1);
diff --git a/hex.c b/hex.c
index cfd9d72..899b74a 100644
--- a/hex.c
+++ b/hex.c
@@ -38,7 +38,7 @@ const signed char hexval_table[256] = {
 int get_sha1_hex(const char *hex, unsigned char *sha1)
 {
int i;
-   for (i = 0; i  20; i++) {
+   for (i = 0; i  GIT_SHA1_RAWSZ; i++) {
unsigned int val;
/*
 * hex[1]=='\0' is caught when val is checked below,
@@ -56,15 +56,20 @@ int get_sha1_hex(const char *hex, unsigned char *sha1)
return 0;
 }
 
+int get_oid_hex(const char *hex, struct object_id *oid)
+{
+   return get_sha1_hex(hex, oid-hash);
+}
+
 char *sha1_to_hex(const unsigned char *sha1)
 {
static int bufno;
-   static char hexbuffer[4][41];
+   static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];
static const char hex[] = 0123456789abcdef;
char *buffer = hexbuffer[3  ++bufno], *buf = buffer;
int i;
 
-   for (i = 0; i  20; i++) {
+   for (i = 0; i  GIT_SHA1_RAWSZ; i++) {
unsigned int val = *sha1++;
*buf++ = hex[val  4];
*buf++ = hex[val  0xf];
@@ -73,3 +78,8 @@ char *sha1_to_hex(const unsigned char *sha1)
 
return buffer;
 }
+
+char *oid_to_hex(const struct object_id *oid)
+{
+   return sha1_to_hex(oid-hash);
+}
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 09/10] patch-id: convert to use struct object_id

2015-03-13 Thread brian m. carlson
Convert some magic numbers to the new GIT_SHA1 constants.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 builtin/patch-id.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 77db873..ba34dac 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -1,14 +1,14 @@
 #include builtin.h
 
-static void flush_current_id(int patchlen, unsigned char *id, unsigned char 
*result)
+static void flush_current_id(int patchlen, struct object_id *id, struct 
object_id *result)
 {
char name[50];
 
if (!patchlen)
return;
 
-   memcpy(name, sha1_to_hex(id), 41);
-   printf(%s %s\n, sha1_to_hex(result), name);
+   memcpy(name, oid_to_hex(id), GIT_SHA1_HEXSZ + 1);
+   printf(%s %s\n, oid_to_hex(result), name);
 }
 
 static int remove_space(char *line)
@@ -53,23 +53,23 @@ static int scan_hunk_header(const char *p, int *p_before, 
int *p_after)
return 1;
 }
 
-static void flush_one_hunk(unsigned char *result, git_SHA_CTX *ctx)
+static void flush_one_hunk(struct object_id *result, git_SHA_CTX *ctx)
 {
-   unsigned char hash[20];
+   unsigned char hash[GIT_SHA1_RAWSZ];
unsigned short carry = 0;
int i;
 
git_SHA1_Final(hash, ctx);
git_SHA1_Init(ctx);
/* 20-byte sum, with carry */
-   for (i = 0; i  20; ++i) {
-   carry += result[i] + hash[i];
-   result[i] = carry;
+   for (i = 0; i  GIT_SHA1_RAWSZ; ++i) {
+   carry += result-hash[i] + hash[i];
+   result-hash[i] = carry;
carry = 8;
}
 }
 
-static int get_one_patchid(unsigned char *next_sha1, unsigned char *result,
+static int get_one_patchid(struct object_id *next_oid, struct object_id 
*result,
   struct strbuf *line_buf, int stable)
 {
int patchlen = 0, found_next = 0;
@@ -77,7 +77,7 @@ static int get_one_patchid(unsigned char *next_sha1, unsigned 
char *result,
git_SHA_CTX ctx;
 
git_SHA1_Init(ctx);
-   hashclr(result);
+   oidclr(result);
 
while (strbuf_getwholeline(line_buf, stdin, '\n') != EOF) {
char *line = line_buf-buf;
@@ -93,7 +93,7 @@ static int get_one_patchid(unsigned char *next_sha1, unsigned 
char *result,
else if (!memcmp(line, \\ , 2)  12  strlen(line))
continue;
 
-   if (!get_sha1_hex(p, next_sha1)) {
+   if (!get_oid_hex(p, next_oid)) {
found_next = 1;
break;
}
@@ -143,7 +143,7 @@ static int get_one_patchid(unsigned char *next_sha1, 
unsigned char *result,
}
 
if (!found_next)
-   hashclr(next_sha1);
+   oidclr(next_oid);
 
flush_one_hunk(result, ctx);
 
@@ -152,15 +152,15 @@ static int get_one_patchid(unsigned char *next_sha1, 
unsigned char *result,
 
 static void generate_id_list(int stable)
 {
-   unsigned char sha1[20], n[20], result[20];
+   struct object_id oid, n, result;
int patchlen;
struct strbuf line_buf = STRBUF_INIT;
 
-   hashclr(sha1);
+   oidclr(oid);
while (!feof(stdin)) {
-   patchlen = get_one_patchid(n, result, line_buf, stable);
-   flush_current_id(patchlen, sha1, result);
-   hashcpy(sha1, n);
+   patchlen = get_one_patchid(n, result, line_buf, stable);
+   flush_current_id(patchlen, oid, result);
+   oidcpy(oid, n);
}
strbuf_release(line_buf);
 }
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 00/10] Use a structure for object IDs.

2015-03-13 Thread brian m. carlson

On Wed, Mar 11, 2015 at 01:35:06PM -0700, Junio C Hamano wrote:

Michael Haggerty mhag...@alum.mit.edu writes:

4. We continue to support working with SHA-1s declared to be (unsigned
char *) in some performance-critical code, even as we migrate most other
code to using SHA-1s embedded within a (struct object_id). This will
cost some duplication of code. To accept this approach, we would need an
idea of *how much* code duplication would be needed. E.g., how many
functions will need both (unsigned char *) versions and (struct
object_id *) versions?


Ideally, only the ones that knows the underlying hash function is
SHA-1 (i.e. anybody who mentions git_SHA_CTX), moves bytes from/to
in-core object name field and raw range of bytes (e.g. sha1hash());
everybody else like hashcpy() and hashcmp() should be able to do its
thing only on structs and a generic-looking constant that denotes
how many bytes there are in the hash (or even sizeof(struct oid)).

I do not know what kind of code duplication you are worried about,
though.  If a callee needs unsigned char *, the caller that has a
struct object_id *o should pass o-hash to the callee.


That's the plan.  My goal (which may or may not be achievable) is to
make hashcpy, hashcmp, and similar functions an implementation detail of
struct object_id functions.  If we have to use o-hash somewhere, okay.
I'm much more interested in practicality than theoretical purity.  I
want these changes to provide easier-to-change, more maintainable code,
not more complex and difficult code.


And please do not suggest switching to C++; all it would do to
overload these into a single name is to make the callers harder to
read.


I'm not considering that.  I've attempted to compile git with g++
before as an idle curiosity, and I gave up almost immediately because it
looked like a bunch of work.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


[PATCH v2 00/10] Use a structure for object IDs.

2015-03-13 Thread brian m. carlson
This is a patch series to convert some of the relevant uses of unsigned
char [20] to struct object_id.

The goal of this series to improve type-checking in the codebase and to
make it easier to move to a different hash function if the project
decides to do that.

There should be no functional change from this patch series.

I've tried to adopt most of the suggestions where possible without major
reworking.

Changes since v1:
* Call the struct member hash.
* Convert some additional magic numbers to be based off the constants
  GIT_SHA1_RAWSZ and GIT_SHA1_HEXSZ.  Introduce variables where this
  makes sense.
* Define GIT_SHA1_HEXSZ in terms of GIT_SHA1_RAWSZ.
* Move the definition of struct object_id to cache.h.  No new #includes
  are required as a result.
* Use better names for arguments to oidcmp.
* Convert one or two structs that were missed last time.

You can see this series on the object-id branch (may be rebased) at

  https://github.com/bk2204/git.git
  https://git.crustytoothpaste.net/git/bmc/git.git

Both repositories are pushed at the same time.

brian m. carlson (10):
  Define a structure for object IDs.
  Define utility functions for object IDs.
  bisect.c: convert leaf functions to use struct object_id
  archive.c: convert to use struct object_id
  zip: use GIT_SHA1_HEXSZ for trailers
  bulk-checkin.c: convert to use struct object_id
  diff: convert struct combine_diff_path to object_id
  commit: convert parts to struct object_id
  patch-id: convert to use struct object_id
  apply: convert threeway_stage to object_id

 archive-zip.c  |  4 ++--
 archive.c  | 22 ++---
 bisect.c   | 40 +++---
 builtin/apply.c| 14 +++---
 builtin/patch-id.c | 34 -
 bulk-checkin.c | 12 ++--
 cache.h| 40 ++
 combine-diff.c | 56 +++---
 commit.c   | 56 +-
 commit.h   |  4 ++--
 diff-lib.c | 10 +-
 diff.h |  5 +++--
 hex.c  | 16 +---
 log-tree.c |  2 +-
 send-pack.c|  2 +-
 shallow.c  |  8 
 tree-diff.c| 10 +-
 upload-pack.c  |  2 +-
 18 files changed, 192 insertions(+), 145 deletions(-)

-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 01/10] Define a structure for object IDs.

2015-03-13 Thread brian m. carlson
Many places throughout the code use unsigned char [20] to store object IDs
(SHA-1 values).  This leads to lots of hardcoded numbers throughout the
codebase.  It also leads to confusion about the purposes of a buffer.

Introduce a structure for object IDs.  This allows us to obtain the benefits
of compile-time checking for misuse.  The structure is expected to remain
the same size and have the same alignment requirements on all known
platforms, compared to the array of unsigned char, although this is not
required for correctness.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 cache.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/cache.h b/cache.h
index 761c570..6582c35 100644
--- a/cache.h
+++ b/cache.h
@@ -43,6 +43,14 @@ int git_deflate_end_gently(git_zstream *);
 int git_deflate(git_zstream *, int flush);
 unsigned long git_deflate_bound(git_zstream *, unsigned long);
 
+/* The length in bytes and in hex digits of an object name (SHA-1 value). */
+#define GIT_SHA1_RAWSZ 20
+#define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)
+
+struct object_id {
+   unsigned char hash[GIT_SHA1_RAWSZ];
+};
+
 #if defined(DT_UNKNOWN)  !defined(NO_D_TYPE_IN_DIRENT)
 #define DTYPE(de)  ((de)-d_type)
 #else
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 08/10] commit: convert parts to struct object_id

2015-03-13 Thread brian m. carlson
Convert struct commit_graft and necessary local parts of commit.c.
Also, convert several constants based on the hex length of an SHA-1 to
use GIT_SHA1_HEXSZ, and move several magic constants into variables for
readability.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 commit.c  | 56 ++--
 commit.h  |  4 ++--
 log-tree.c|  2 +-
 send-pack.c   |  2 +-
 shallow.c |  8 
 upload-pack.c |  2 +-
 6 files changed, 39 insertions(+), 35 deletions(-)

diff --git a/commit.c b/commit.c
index a8c7577..2d9de80 100644
--- a/commit.c
+++ b/commit.c
@@ -55,12 +55,12 @@ struct commit *lookup_commit(const unsigned char *sha1)
 
 struct commit *lookup_commit_reference_by_name(const char *name)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
struct commit *commit;
 
-   if (get_sha1_committish(name, sha1))
+   if (get_sha1_committish(name, oid.hash))
return NULL;
-   commit = lookup_commit_reference(sha1);
+   commit = lookup_commit_reference(oid.hash);
if (parse_commit(commit))
return NULL;
return commit;
@@ -99,7 +99,7 @@ static int commit_graft_alloc, commit_graft_nr;
 static const unsigned char *commit_graft_sha1_access(size_t index, void *table)
 {
struct commit_graft **commit_graft_table = table;
-   return commit_graft_table[index]-sha1;
+   return commit_graft_table[index]-oid.hash;
 }
 
 static int commit_graft_pos(const unsigned char *sha1)
@@ -110,7 +110,7 @@ static int commit_graft_pos(const unsigned char *sha1)
 
 int register_commit_graft(struct commit_graft *graft, int ignore_dups)
 {
-   int pos = commit_graft_pos(graft-sha1);
+   int pos = commit_graft_pos(graft-oid.hash);
 
if (0 = pos) {
if (ignore_dups)
@@ -138,22 +138,23 @@ struct commit_graft *read_graft_line(char *buf, int len)
/* The format is just Commit Parent1 Parent2 ...\n */
int i;
struct commit_graft *graft = NULL;
+   const int entry_size = GIT_SHA1_HEXSZ + 1;
 
while (len  isspace(buf[len-1]))
buf[--len] = '\0';
if (buf[0] == '#' || buf[0] == '\0')
return NULL;
-   if ((len + 1) % 41)
+   if ((len + 1) % entry_size)
goto bad_graft_data;
-   i = (len + 1) / 41 - 1;
-   graft = xmalloc(sizeof(*graft) + 20 * i);
+   i = (len + 1) / entry_size - 1;
+   graft = xmalloc(sizeof(*graft) + GIT_SHA1_RAWSZ * i);
graft-nr_parent = i;
-   if (get_sha1_hex(buf, graft-sha1))
+   if (get_oid_hex(buf, graft-oid))
goto bad_graft_data;
-   for (i = 40; i  len; i += 41) {
+   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/41]))
+   if (get_sha1_hex(buf + i + 1, graft-parent[i/entry_size].hash))
goto bad_graft_data;
}
return graft;
@@ -302,39 +303,42 @@ int parse_commit_buffer(struct commit *item, const void 
*buffer, unsigned long s
 {
const char *tail = buffer;
const char *bufptr = buffer;
-   unsigned char parent[20];
+   struct object_id parent;
struct commit_list **pptr;
struct commit_graft *graft;
+   const int tree_entry_len = GIT_SHA1_HEXSZ + 5;
+   const int parent_entry_len = GIT_SHA1_HEXSZ + 7;
 
if (item-object.parsed)
return 0;
item-object.parsed = 1;
tail += size;
-   if (tail = bufptr + 46 || memcmp(bufptr, tree , 5) || bufptr[45] != 
'\n')
+   if (tail = bufptr + tree_entry_len + 1 || memcmp(bufptr, tree , 5) ||
+   bufptr[tree_entry_len] != '\n')
return error(bogus commit object %s, 
sha1_to_hex(item-object.sha1));
-   if (get_sha1_hex(bufptr + 5, parent)  0)
+   if (get_sha1_hex(bufptr + 5, parent.hash)  0)
return error(bad tree pointer in commit %s,
 sha1_to_hex(item-object.sha1));
-   item-tree = lookup_tree(parent);
-   bufptr += 46; /* tree  + hex sha1 + \n */
+   item-tree = lookup_tree(parent.hash);
+   bufptr += tree_entry_len + 1; /* tree  + hex sha1 + \n */
pptr = item-parents;
 
graft = lookup_commit_graft(item-object.sha1);
-   while (bufptr + 48  tail  !memcmp(bufptr, parent , 7)) {
+   while (bufptr + parent_entry_len  tail  !memcmp(bufptr, parent , 
7)) {
struct commit *new_parent;
 
-   if (tail = bufptr + 48 ||
-   get_sha1_hex(bufptr + 7, parent) ||
-   bufptr[47] != '\n')
+   if (tail = bufptr + parent_entry_len + 1 ||
+   get_sha1_hex(bufptr + 7, parent.hash) ||
+   bufptr[parent_entry_len] != '\n')
return

[PATCH v2 06/10] bulk-checkin.c: convert to use struct object_id

2015-03-13 Thread brian m. carlson
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 bulk-checkin.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/bulk-checkin.c b/bulk-checkin.c
index 0c4b8a7..c80e503 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -24,7 +24,7 @@ static struct bulk_checkin_state {
 
 static void finish_bulk_checkin(struct bulk_checkin_state *state)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
struct strbuf packname = STRBUF_INIT;
int i;
 
@@ -36,11 +36,11 @@ static void finish_bulk_checkin(struct bulk_checkin_state 
*state)
unlink(state-pack_tmp_name);
goto clear_exit;
} else if (state-nr_written == 1) {
-   sha1close(state-f, sha1, CSUM_FSYNC);
+   sha1close(state-f, oid.hash, CSUM_FSYNC);
} else {
-   int fd = sha1close(state-f, sha1, 0);
-   fixup_pack_header_footer(fd, sha1, state-pack_tmp_name,
-state-nr_written, sha1,
+   int fd = sha1close(state-f, oid.hash, 0);
+   fixup_pack_header_footer(fd, oid.hash, state-pack_tmp_name,
+state-nr_written, oid.hash,
 state-offset);
close(fd);
}
@@ -48,7 +48,7 @@ static void finish_bulk_checkin(struct bulk_checkin_state 
*state)
strbuf_addf(packname, %s/pack/pack-, get_object_directory());
finish_tmp_packfile(packname, state-pack_tmp_name,
state-written, state-nr_written,
-   state-pack_idx_opts, sha1);
+   state-pack_idx_opts, oid.hash);
for (i = 0; i  state-nr_written; i++)
free(state-written[i]);
 
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 05/10] zip: use GIT_SHA1_HEXSZ for trailers

2015-03-13 Thread brian m. carlson
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 archive-zip.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/archive-zip.c b/archive-zip.c
index 4bde019..b669e50 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -427,12 +427,12 @@ static void write_zip_trailer(const unsigned char *sha1)
copy_le16(trailer.entries, zip_dir_entries);
copy_le32(trailer.size, zip_dir_offset);
copy_le32(trailer.offset, zip_offset);
-   copy_le16(trailer.comment_length, sha1 ? 40 : 0);
+   copy_le16(trailer.comment_length, sha1 ? GIT_SHA1_HEXSZ : 0);
 
write_or_die(1, zip_dir, zip_dir_offset);
write_or_die(1, trailer, ZIP_DIR_TRAILER_SIZE);
if (sha1)
-   write_or_die(1, sha1_to_hex(sha1), 40);
+   write_or_die(1, sha1_to_hex(sha1), GIT_SHA1_HEXSZ);
 }
 
 static void dos_time(time_t *time, int *dos_date, int *dos_time)
-- 
2.2.1.209.g41e5f3a

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 00/10] Use a structure for object IDs.

2015-03-14 Thread brian m. carlson

On Fri, Mar 13, 2015 at 11:39:26PM +, brian m. carlson wrote:


Changes since v1:


I just realized that I sent this out as v2 instead of v3.  It really is
v3.  My apologies for the error.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails

2015-03-10 Thread brian m. carlson

On Tue, Mar 10, 2015 at 06:05:46PM +, Dan Langille (dalangil) wrote:

We have made progress I think.

With stock git:

tl;dr: 1 - with a ticket, you get prompted, but hitting ENTER succeeds.
  2 - without a ticket, nothing works


With patched git:

tl;dr: 1 - with a ticket,entering credentials, SUCCEEDS; just hit enter, 
failure


If I have a valid ticket, why am I being prompted for credentials?


libcurl won't even attempt authentication if you don't have a username
specified.  I know that the web server should be able to figure it out
from your credentials, so it shouldn't matter what username you provide.
This is an unfortuate quirk of libcurl.

Also, are you using 2.3.0, or one of the earlier patched versions?  That
might affect how it works.


It appears patched git always wants credentials entered and ignores the
valid ticket.


So what I think is happening is that you didn't specify a username, but
git got a 401, so it prompted.  Now it actually attempts to use the
password you provided, whereas before it did not.

Does it work with a ticket if you specify a username, as in the
following URL?
https://b...@git.crustytoothpaste.net/git/bmc/homedir.git
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] t7510: do not fail when gpg warns about insecure memory

2015-03-08 Thread brian m. carlson

On Sun, Mar 08, 2015 at 05:43:41PM -0400, Eric Sunshine wrote:

On Sun, Mar 8, 2015 at 11:40 AM, Kyle J. McKay mack...@gmail.com wrote:

Depending on how gpg was built, it may issue the following
message to stderr when run:

  Warning: using insecure memory!

Unfortunately when running the test, that message gets
collected in the stdout result of git show -s --show-signature
but is collected in the stderr result of git verify-commit -v


I'm having trouble parsing this. Is there a word missing?


causing both the stdout and stderr result comparisions to fail.


Perhaps this is better?

 Unfortunately when running the test, that message is found in the 
 standard output of git show -s --show-signature, but in the standard 
 error of git verify-commit -v causing the comparisons of both standard 
 output and standard error to fail.

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


<    1   2   3   4   5   6   7   8   9   10   >