Re: [PATCH 01/15] builtin/add.c: rearrange xcalloc arguments

2014-05-27 Thread Brian Gesiak 
Oomph, how embarrassing. Thanks for pointing that out!

Would it be better if I rerolled the patches?

- Brian Gesiak

On Tue, May 27, 2014 at 12:25 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Mon, May 26, 2014 at 11:33 AM, Brian Gesiak modoca...@gmail.com wrote:
 xcalloc takes two arguments: the number of elements and their size.
 run_add_interactive passes the arguments in reverse order, passing the
 size of a char*, followed by the number of char* to be allocated.
 Rearrgange them so they are in the correct order.

 s/Rearrgange/Rearrange/

 Same misspelling afflicts the entire patch series.

 Signed-off-by: Brian Gesiak modoca...@gmail.com
 ---
  builtin/add.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/builtin/add.c b/builtin/add.c
 index 672adc0..488acf4 100644
 --- a/builtin/add.c
 +++ b/builtin/add.c
 @@ -248,7 +248,7 @@ int run_add_interactive(const char *revision, const char 
 *patch_mode,
 int status, ac, i;
 const char **args;

 -   args = xcalloc(sizeof(const char *), (pathspec-nr + 6));
 +   args = xcalloc((pathspec-nr + 6), sizeof(const char *));
 ac = 0;
 args[ac++] = add--interactive;
 if (patch_mode)
 --
 2.0.0.rc1.543.gc8042da
--
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 01/15] builtin/add.c: rearrange xcalloc arguments

2014-05-27 Thread Brian Gesiak 
On Wed, May 28, 2014 at 7:41 AM, Junio C Hamano gits...@pobox.com wrote:
 I do not think it is worth doing this change starting from maint, so
 I've dropped this one and a few others that did not apply to master
 and queued the remainder to 'pu'.

Thank you! I'll keep this in mind when choosing what to branch off of
in the future.

On Wed, May 28, 2014 at 6:35 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
 Etiquette on this list is to avoid top-posting [1].
 ...
 If you do re-roll, perhaps consider simplifying the commit messages.

Thank you for the tips; very much appreciated.

- Brian Gesiak
--
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 00/15] Rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
The vast majority of the Git codebase passes these arguments in the
correct order, but there are some exceptions. This patch series
corrects those exceptions.

Brian Gesiak (15):
  builtin/add.c: rearrange xcalloc arguments
  builtin/ls-remote.c: rearrange xcalloc arguments
  builtin/remote.c: rearrange xcalloc arguments
  commit.c: rearrange xcalloc arguments
  config.c: rearrange xcalloc arguments
  diff.c: rearrange xcalloc arguments
  hash.c: rearrange xcalloc arguments
  hash.h: rearrange xcalloc arguments
  http-push.c: rearrange xcalloc arguments
  imap-send.c: rearrange xcalloc arguments
  notes.c: rearrange xcalloc arguments
  pack-revindex.c: rearrange xcalloc arguments
  reflog-walk.c: rearrange xcalloc arguments
  remote.c: rearrange xcalloc arguments
  transport-helper.c: rearrange xcalloc arguments

builtin/add.c   | 2 +-
builtin/ls-remote.c | 2 +-
builtin/remote.c| 8 
commit.c| 2 +-
config.c| 4 ++--
diff.c  | 2 +-
hash.c  | 2 +-
hash.h  | 2 +-
http-push.c | 2 +-
imap-send.c | 2 +-
notes.c | 6 +++---
pack-revindex.c | 2 +-
reflog-walk.c   | 8 
remote.c| 2 +-
transport-helper.c  | 2 +-
15 files changed, 24 insertions(+), 24 deletions(-)

-- 
2.0.0.rc1.543.gc8042da

--
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 06/15] diff.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
diffstat_add passes the arguments in reverse order, passing the
size of a diffstat_file*, followed by the number of diffstat_file* to
be allocated. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 635dee2..a71dfde 100644
--- a/diff.c
+++ b/diff.c
@@ -1360,7 +1360,7 @@ static struct diffstat_file *diffstat_add(struct 
diffstat_t *diffstat,
  const char *name_b)
 {
struct diffstat_file *x;
-   x = xcalloc(sizeof (*x), 1);
+   x = xcalloc(1, sizeof(*x));
if (diffstat-nr == diffstat-alloc) {
diffstat-alloc = alloc_nr(diffstat-alloc);
diffstat-files = xrealloc(diffstat-files,
-- 
2.0.0.rc1.543.gc8042da

--
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 01/15] builtin/add.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
run_add_interactive passes the arguments in reverse order, passing the
size of a char*, followed by the number of char* to be allocated.
Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 builtin/add.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/add.c b/builtin/add.c
index 672adc0..488acf4 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -248,7 +248,7 @@ int run_add_interactive(const char *revision, const char 
*patch_mode,
int status, ac, i;
const char **args;
 
-   args = xcalloc(sizeof(const char *), (pathspec-nr + 6));
+   args = xcalloc((pathspec-nr + 6), sizeof(const char *));
ac = 0;
args[ac++] = add--interactive;
if (patch_mode)
-- 
2.0.0.rc1.543.gc8042da

--
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 08/15] hash.h: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
prellocate_hash passes the arguments in reverse order, passing the
size of a hash table entry, followed by the number of entries.
Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 hash.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hash.h b/hash.h
index 1d43ac0..3b5d9e7 100644
--- a/hash.h
+++ b/hash.h
@@ -44,7 +44,7 @@ static inline void preallocate_hash(struct hash_table *table, 
unsigned int elts)
 {
assert(table-size == 0  table-nr == 0  table-array == NULL);
table-size = elts * 2;
-   table-array = xcalloc(sizeof(struct hash_table_entry), table-size);
+   table-array = xcalloc(table-size, sizeof(struct hash_table_entry));
 }
 
 #endif
-- 
2.0.0.rc1.543.gc8042da

--
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 12/15] pack-revindex.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
init_pack_revindex passes the arguments in reverse order, passing the
size of a pack_revindex, followed by the number to allocate.
Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 pack-revindex.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pack-revindex.c b/pack-revindex.c
index b4d2b35..f84979b 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -50,7 +50,7 @@ static void init_pack_revindex(void)
if (!num)
return;
pack_revindex_hashsz = num * 11;
-   pack_revindex = xcalloc(sizeof(*pack_revindex), pack_revindex_hashsz);
+   pack_revindex = xcalloc(pack_revindex_hashsz, sizeof(*pack_revindex));
for (p = packed_git; p; p = p-next) {
num = pack_revindex_ix(p);
num = - 1 - num;
-- 
2.0.0.rc1.543.gc8042da

--
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 03/15] builtin/remote.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
builtin/remote.c includes several calls to xcalloc that pass the
arguments in reverse order. Rearrgange them so they are in the
correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 builtin/remote.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index b3ab4cf..9f62021 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -282,7 +282,7 @@ static int config_read_branches(const char *key, const char 
*value, void *cb)
item = string_list_insert(branch_list, name);
 
if (!item-util)
-   item-util = xcalloc(sizeof(struct branch_info), 1);
+   item-util = xcalloc(1, sizeof(struct branch_info));
info = item-util;
if (type == REMOTE) {
if (info-remote_name)
@@ -398,7 +398,7 @@ static int get_push_ref_states(const struct ref 
*remote_refs,
 
item = string_list_append(states-push,
  abbrev_branch(ref-peer_ref-name));
-   item-util = xcalloc(sizeof(struct push_info), 1);
+   item-util = xcalloc(1, sizeof(struct push_info));
info = item-util;
info-forced = ref-force;
info-dest = xstrdup(abbrev_branch(ref-name));
@@ -433,7 +433,7 @@ static int get_push_ref_states_noquery(struct ref_states 
*states)
states-push.strdup_strings = 1;
if (!remote-push_refspec_nr) {
item = string_list_append(states-push, _((matching)));
-   info = item-util = xcalloc(sizeof(struct push_info), 1);
+   info = item-util = xcalloc(1, sizeof(struct push_info));
info-status = PUSH_STATUS_NOTQUERIED;
info-dest = xstrdup(item-string);
}
@@ -446,7 +446,7 @@ static int get_push_ref_states_noquery(struct ref_states 
*states)
else
item = string_list_append(states-push, _((delete)));
 
-   info = item-util = xcalloc(sizeof(struct push_info), 1);
+   info = item-util = xcalloc(1, sizeof(struct push_info));
info-forced = spec-force;
info-status = PUSH_STATUS_NOTQUERIED;
info-dest = xstrdup(spec-dst ? spec-dst : item-string);
-- 
2.0.0.rc1.543.gc8042da

--
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/15] reflog-walk.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
reflog-walk.c includes several calls to xcalloc that pass the arguments
in reverse order. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 reflog-walk.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/reflog-walk.c b/reflog-walk.c
index b2fbdb2..f8d8f13 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -45,7 +45,7 @@ static int read_one_reflog(unsigned char *osha1, unsigned 
char *nsha1,
 static struct complete_reflogs *read_complete_reflog(const char *ref)
 {
struct complete_reflogs *reflogs =
-   xcalloc(sizeof(struct complete_reflogs), 1);
+   xcalloc(1, sizeof(struct complete_reflogs));
reflogs-ref = xstrdup(ref);
for_each_reflog_ent(ref, read_one_reflog, reflogs);
if (reflogs-nr == 0) {
@@ -143,7 +143,7 @@ struct reflog_walk_info {
 
 void init_reflog_walk(struct reflog_walk_info** info)
 {
-   *info = xcalloc(sizeof(struct reflog_walk_info), 1);
+   *info = xcalloc(1, sizeof(struct reflog_walk_info));
 }
 
 int add_reflog_for_walk(struct reflog_walk_info *info,
@@ -207,7 +207,7 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
= reflogs;
}
 
-   commit_reflog = xcalloc(sizeof(struct commit_reflog), 1);
+   commit_reflog = xcalloc(1, sizeof(struct commit_reflog));
if (recno  0) {
commit_reflog-recno = get_reflog_recno_by_time(reflogs, 
timestamp);
if (commit_reflog-recno  0) {
@@ -250,7 +250,7 @@ void fake_reflog_parent(struct reflog_walk_info *info, 
struct commit *commit)
return;
}
 
-   commit-parents = xcalloc(sizeof(struct commit_list), 1);
+   commit-parents = xcalloc(1, sizeof(struct commit_list));
commit-parents-item = commit_info-commit;
 }
 
-- 
2.0.0.rc1.543.gc8042da

--
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/15] commit.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
reduce_heads passes the arguments in reverse order, passing the
size of a commit*, followed by the number of commit* to be allocated.
Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 commit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commit.c b/commit.c
index 6bf4fe0..0fe685f 100644
--- a/commit.c
+++ b/commit.c
@@ -1041,7 +1041,7 @@ struct commit_list *reduce_heads(struct commit_list 
*heads)
p-item-object.flags |= STALE;
num_head++;
}
-   array = xcalloc(sizeof(*array), num_head);
+   array = xcalloc(num_head, sizeof(*array));
for (p = heads, i = 0; p; p = p-next) {
if (p-item-object.flags  STALE) {
array[i++] = p-item;
-- 
2.0.0.rc1.543.gc8042da

--
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 07/15] hash.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
grow_hash_table passes the arguments in reverse order, passing the
size of a hash table entry, followed by the number of entries.
Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hash.c b/hash.c
index 749ecfe..2067be9 100644
--- a/hash.c
+++ b/hash.c
@@ -53,7 +53,7 @@ static void grow_hash_table(struct hash_table *table)
struct hash_table_entry *old_array = table-array, *new_array;
 
new_size = alloc_nr(old_size);
-   new_array = xcalloc(sizeof(struct hash_table_entry), new_size);
+   new_array = xcalloc(new_size, sizeof(struct hash_table_entry));
table-size = new_size;
table-array = new_array;
table-nr = 0;
-- 
2.0.0.rc1.543.gc8042da

--
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/15] remote.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
parse_refspec_internal passes the arguments in reverse order, passing the
size of a refspec, followed by the number to allocate. Rearrgange them
so they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 remote.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index ebed40d..df3267b 100644
--- a/remote.c
+++ b/remote.c
@@ -523,7 +523,7 @@ static void free_refspecs(struct refspec *refspec, int 
nr_refspec)
 static struct refspec *parse_refspec_internal(int nr_refspec, const char 
**refspec, int fetch, int verify)
 {
int i;
-   struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);
+   struct refspec *rs = xcalloc(nr_refspec, sizeof(*rs));
 
for (i = 0; i  nr_refspec; i++) {
size_t llen;
-- 
2.0.0.rc1.543.gc8042da

--
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/15] notes.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
notes.c includes several calls to xcalloc that pass the arguments in
reverse order. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 notes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/notes.c b/notes.c
index 5f07c0b..5fe691d 100644
--- a/notes.c
+++ b/notes.c
@@ -303,7 +303,7 @@ static int note_tree_insert(struct notes_tree *t, struct 
int_node *tree,
free(entry);
return 0;
}
-   new_node = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
+   new_node = (struct int_node *) xcalloc(1, sizeof(struct int_node));
ret = note_tree_insert(t, new_node, n + 1, l, GET_PTR_TYPE(*p),
   combine_notes);
if (ret)
@@ -443,7 +443,7 @@ static void load_subtree(struct notes_tree *t, struct 
leaf_node *subtree,
if (len = 20) {
type = PTR_TYPE_NOTE;
l = (struct leaf_node *)
-   xcalloc(sizeof(struct leaf_node), 1);
+   xcalloc(1, sizeof(struct leaf_node));
hashcpy(l-key_sha1, object_sha1);
hashcpy(l-val_sha1, entry.sha1);
if (len  20) {
@@ -1003,7 +1003,7 @@ void init_notes(struct notes_tree *t, const char 
*notes_ref,
if (!combine_notes)
combine_notes = combine_notes_concatenate;
 
-   t-root = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
+   t-root = (struct int_node *) xcalloc(1, sizeof(struct int_node));
t-first_non_note = NULL;
t-prev_non_note = NULL;
t-ref = notes_ref ? xstrdup(notes_ref) : NULL;
-- 
2.0.0.rc1.543.gc8042da

--
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 09/15] http-push.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
http-push passes the arguments in reverse order, passing the size
of a repo, followed by the number to allocate. Rearrgange them so
they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 http-push.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/http-push.c b/http-push.c
index d4b40c9..1c722e5 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1733,7 +1733,7 @@ int main(int argc, char **argv)
 
git_extract_argv0_path(argv[0]);
 
-   repo = xcalloc(sizeof(*repo), 1);
+   repo = xcalloc(1, sizeof(*repo));
 
argv++;
for (i = 1; i  argc; i++, argv++) {
-- 
2.0.0.rc1.543.gc8042da

--
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 05/15] config.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
config.c includes several calls to xcalloc that pass the arguments
in reverse order: the size of a struct lock_file*, followed by the
number to allocate. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 config.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index 314d8ee..c3612cc 100644
--- a/config.c
+++ b/config.c
@@ -1536,7 +1536,7 @@ int git_config_set_multivar_in_file(const char 
*config_filename,
 * The lock serves a purpose in addition to locking: the new
 * contents of .git/config will be written into it.
 */
-   lock = xcalloc(sizeof(struct lock_file), 1);
+   lock = xcalloc(1, sizeof(struct lock_file));
fd = hold_lock_file_for_update(lock, config_filename, 0);
if (fd  0) {
error(could not lock config file %s: %s, config_filename, 
strerror(errno));
@@ -1791,7 +1791,7 @@ int git_config_rename_section_in_file(const char 
*config_filename,
if (!config_filename)
config_filename = filename_buf = git_pathdup(config);
 
-   lock = xcalloc(sizeof(struct lock_file), 1);
+   lock = xcalloc(1, sizeof(struct lock_file));
out_fd = hold_lock_file_for_update(lock, config_filename, 0);
if (out_fd  0) {
ret = error(could not lock config file %s, config_filename);
-- 
2.0.0.rc1.543.gc8042da

--
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/15] imap-send.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
imap_open_store passes the arguments in reverse order, passing the
size of an imap_store*, followed by the number to allocate.
Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 imap-send.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/imap-send.c b/imap-send.c
index 0bc6f7f..45230e1 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -951,7 +951,7 @@ static struct imap_store *imap_open_store(struct 
imap_server_conf *srvc)
char *arg, *rsp;
int s = -1, preauth;
 
-   ctx = xcalloc(sizeof(*ctx), 1);
+   ctx = xcalloc(1, sizeof(*ctx));
 
ctx-imap = imap = xcalloc(sizeof(*imap), 1);
imap-buf.sock.fd[0] = imap-buf.sock.fd[1] = -1;
-- 
2.0.0.rc1.543.gc8042da

--
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/15] transport-helper.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
xcalloc takes two arguments: the number of elements and their size.
transport_helper_init passes the arguments in reverse order, passing the
size of a helper_data*, followed by the number to allocate. Rearrgange
them so they are in the correct order.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 transport-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport-helper.c b/transport-helper.c
index ad72fbd..cf48913 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1002,7 +1002,7 @@ static struct ref *get_refs_list(struct transport 
*transport, int for_push)
 
 int transport_helper_init(struct transport *transport, const char *name)
 {
-   struct helper_data *data = xcalloc(sizeof(*data), 1);
+   struct helper_data *data = xcalloc(1, sizeof(*data));
data-name = name;
 
if (getenv(GIT_TRANSPORT_HELPER_DEBUG))
-- 
2.0.0.rc1.543.gc8042da

--
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 01/15] builtin/add.c: rearrange xcalloc arguments

2014-05-26 Thread Brian Gesiak 
My apologies! I based my work off of maint, branching off of eea591.

My reasoning was that Documentation/SubmittingPatches states that a
bugfix should be based on 'maint'. [1] Now that I think about it,
this is probably not the kind of bug that statement had in mind.

Should I reroll the patch based on master?

- Brian Gesiak

[1] 
https://github.com/git/git/blob/4a28f169ad29ba452e0e7bea2583914c10c58322/Documentation/SubmittingPatches#L9

On Tue, May 27, 2014 at 8:11 AM, Jeremiah Mahler jmmah...@gmail.com wrote:
 Brian,

 On Tue, May 27, 2014 at 12:33:42AM +0900, Brian Gesiak wrote:
 xcalloc takes two arguments: the number of elements and their size.
 run_add_interactive passes the arguments in reverse order, passing the
 size of a char*, followed by the number of char* to be allocated.
 Rearrgange them so they are in the correct order.

 Signed-off-by: Brian Gesiak modoca...@gmail.com
 ---
  builtin/add.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/builtin/add.c b/builtin/add.c
 index 672adc0..488acf4 100644
 --- a/builtin/add.c
 +++ b/builtin/add.c
 @@ -248,7 +248,7 @@ int run_add_interactive(const char *revision, const char 
 *patch_mode,
   int status, ac, i;
   const char **args;

 - args = xcalloc(sizeof(const char *), (pathspec-nr + 6));
 + args = xcalloc((pathspec-nr + 6), sizeof(const char *));
   ac = 0;
   args[ac++] = add--interactive;
   if (patch_mode)


 This patch doesn't apply to any of the branches I have available
 (master, pu, next).  And there is no line containing pathspec-nr + 6
 anywhere in my builtin/add.c.  Which branch is your work based off?

 --
 Jeremiah Mahler
 jmmah...@gmail.com
 http://github.com/jmahler
--
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 1/2] strbuf: Use _rtrim and _ltrim in strbuf_trim

2014-04-30 Thread Brian Gesiak 
strbuf_trim strips whitespace from the end, then the beginning of a
strbuf. Those operations are duplicated in strbuf_rtrim and
strbuf_ltrim.

Replace strbuf_trim implementation with calls to strbuf_rtrim,
then strbuf_ltrim.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---

This is tangential to my GSoC project; I noticed the duplication
and thought it could be remedied.

 strbuf.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/strbuf.c b/strbuf.c
index 83caf4a..382cf68 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -96,15 +96,8 @@ void strbuf_grow(struct strbuf *sb, size_t extra)
 
 void strbuf_trim(struct strbuf *sb)
 {
-   char *b = sb-buf;
-   while (sb-len  0  isspace((unsigned char)sb-buf[sb-len - 1]))
-   sb-len--;
-   while (sb-len  0  isspace(*b)) {
-   b++;
-   sb-len--;
-   }
-   memmove(sb-buf, b, sb-len);
-   sb-buf[sb-len] = '\0';
+   strbuf_rtrim(sb);
+   strbuf_ltrim(sb);
 }
 void strbuf_rtrim(struct strbuf *sb)
 {
-- 
1.9.2.507.g779792a

--
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 2/2] api-strbuf.txt: Add docs for _trim and _ltrim

2014-04-30 Thread Brian Gesiak 
API documentation for strbuf does not document strbuf_trim or
strbuf_ltrim. Add documentation for these two functions.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 Documentation/technical/api-strbuf.txt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/Documentation/technical/api-strbuf.txt 
b/Documentation/technical/api-strbuf.txt
index 3350d97..4396be9 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -121,10 +121,19 @@ Functions
 
 * Related to the contents of the buffer
 
+`strbuf_trim`::
+
+   Strip whitespace from the beginning and end of a string.
+   Equivalent to performing `strbuf_rtrim()` followed by `strbuf_ltrim()`.
+
 `strbuf_rtrim`::
 
Strip whitespace from the end of a string.
 
+`strbuf_ltrim`::
+
+   Strip whitespace from the beginning of a string.
+
 `strbuf_cmp`::
 
Compare two buffers. Returns an integer less than, equal to, or greater
-- 
1.9.2.507.g779792a

--
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 in GSoC 2014

2014-04-21 Thread Brian Gesiak 
Thank you!

I'm very excited to be participating in this year's GSoC. Google
recommends that students use the next few weeks to get to know their
mentors, read documentation, and get up to speed to begin working on
their projects. Students have also received instructions on submitting
tax forms and other paperwork.

Aside from filing all the requisite paperwork, I plan on reading
through the extensive set of patches on lock files Michael Haggerty
submitted after my initial proposal. I also plan on consulting with my
mentor, Jeff King, on some good first steps.

By the way, my name is Brian Gesiak. I'm a research student at the
University of Tokyo, specializing in parallel and distributed
computing. If you have any questions regarding my project, Unify and
Refactor Temporary File Handling, please feel free to contact me via
this mailing list, or privately via email. I'm also on GitHub[1] and
Twitter[2].

[1] https://github.com/modocache
[2] https://twitter.com/modocache

- Brian Gesiak

On Tue, Apr 22, 2014 at 10:06 AM, Andrew Ardill andrew.ard...@gmail.com wrote:
 Congrats everyone who was successful in being picked for this year's GSoC.

 Fabian with Line options for git rebase --interactive [0]
 Brian Gesiak with Unify and Refactor Temporary File Handling [1]
 Tanay Abhra with Git configuration API improvements [2]

 I look forward to seeing how you go!

 [0] 
 https://www.google-melange.com/gsoc/project/details/google/gsoc2014/bafain/5750085036015616
 [1] 
 https://www.google-melange.com/gsoc/project/details/google/gsoc2014/modocache/5639274879778816
 [2] 
 https://www.google-melange.com/gsoc/project/details/google/gsoc2014/tanayabh/5766466041282560

 Regards,

 Andrew Ardill
 --
 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
--
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] git-rebase: Print name of rev when using shorthand

2014-04-16 Thread Brian Gesiak 
Thank you for the feedback!

 Imagine the case where there are more than one branches
 whose tip points at the commit you came from.
 name-rev will not be able to pick correctly which one to report.

I see. Yes, you're exactly right; the following demonstrates
the problem:

$ git checkout -b xylophone master
$ git checkout -b aardvark master
$ git name-rev --name-only @{-1} # I'd want xylophone, but this
outputs aardvark

So it appears name-rev is not up to the task here.

 I think you would want to use something like:

 upstream_name=$(git rev-parse --symbolic-full-name @{-1})
 if test -n $upstream
 then
 upstream_name=${upstream_name#refs/heads/}
 else
 upstream_name=@{-1}
 fi

 if the change is to be made at that point in the code.

I agree, I will re-roll the patch to use this approach.

 I also wonder if git rebase @{-1} deserve a similar translation
 like you are giving git rebase -.

Personally, I've been using the - shorthand with git checkout
for a year or so, but only learned about @{-1} a few months ago.
I think those who use @{-1} are familiar enough with the concept
that they don't need to have the reference translated to a symbolic
full name. Users familiar with - might not be aware of @{-1},
however, so I'd prefer not to output it as we are currently.

Furthermore, were we to translate @{-1}, does that mean we
should also translate @{-2} or prior? I don't think that's the case,
but then only translating @{-1} would seem inconsistent.
From that point of view I'd prefer to simply translate -,
not @{-1}.

- Brian Gesiak
--
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] git-rebase: Print name of rev when using shorthand

2014-04-16 Thread Brian Gesiak 
The output from a successful invocation of the shorthand command
git rebase - is something like Fast-forwarded HEAD to @{-1},
which includes a relative reference to a revision. Other commands
that use the shorthand -, such as git checkout -, typically
display the symbolic name of the revision.

Change rebase to output the symbolic name of the revision when using
the shorthand. For the example above, the new output is
Fast-forwarded HEAD to master, assuming @{-1} is a reference to
master.

- Use git rev-parse to retreive the name of the rev.
- Update the tests in light of this new behavior.

Requested-by: John Keeping j...@keeping.me.uk
Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 git-rebase.sh | 8 +++-
 t/t3400-rebase.sh | 4 +---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 2c75e9f..42d34a6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -455,7 +455,13 @@ then
*)  upstream_name=$1
if test $upstream_name = -
then
-   upstream_name=@{-1}
+   upstream_name=`git rev-parse --symbolic-full-name @{-1}`
+   if test -n $upstream_name
+   then
+   upstream_name=${upstream_name#refs/heads/}
+   else
+   upstream_name=@{-1}
+   fi
fi
shift
;;
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 80e0a95..2b99940 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -91,7 +91,7 @@ test_expect_success 'rebase from ambiguous branch name' '
 test_expect_success 'rebase off of the previous branch using -' '
git checkout master 
git checkout HEAD^ 
-   git rebase @{-1} expect.messages 
+   git rebase master expect.messages 
git merge-base master HEAD expect.forkpoint 
 
git checkout master 
@@ -100,8 +100,6 @@ test_expect_success 'rebase off of the previous branch 
using -' '
git merge-base master HEAD actual.forkpoint 
 
test_cmp expect.forkpoint actual.forkpoint 
-   # the next one is dubious---we may want to say -,
-   # instead of @{-1}, in the message
test_i18ncmp expect.messages actual.messages
 '
 
-- 
1.9.0.259.gc5d75e8.dirty

--
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

[l10n] date: Note for translators not included in .po files

2014-04-16 Thread Brian Gesiak 
A note for translators in date.c is not included in git.pot.
Namely, the following note from date.c:147 is not included
(https://github.com/git/git/blob/v1.9.2/date.c#L147):

/* TRANSLATORS: %s is n years */

This is a very useful note for translators (in fact, I think
the zh_CN translation for date.c:149 might be a little off
because this note was not included. My Mandarin is rusty,
but I believe n years, m months ago should be expressed
without a comma).

According to po/README, the l10n coordinator is responsible
for updating the git.pot file. Would it be possible to update it based
on v1.9.2 and include the above comment?

By the way, I am trying to organize contributors to produce a Japanese
localization for Core Git. Currently we have plenty of interest but
only two contributors. If you or anyone you know would like to contribute
please visit the repository here: https://github.com/modocache/git-po-ja

Thanks!

- Brian Gesiak
--
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] git-rebase: Print name of rev when using shorthand

2014-04-16 Thread Brian Gesiak 
 The concept of n-th prior checkout (aka @{-n}) and immediately
 previous checkout (aka -) are equivalent, even though the former
 may be more generic.

 You seem to be saying that those who understand the former are with
 superiour mental capacity in general than those who only know the
 latter, and they can always remember where they came from.

 ...have you considered _why_ it is a good thing to show the symbolic
 name in the first place?

I think I failed to express my point here; I don't think people that
use @{-1} have superior mental capacity, but rather simply that
they are aware of the @{-n} method of specifying a previous reference.
So in response to the command git rebase @{-4}, displaying the
result Fast-forwarded HEAD to @{-4} does not contain any unknown
syntax that may confuse them. They may not remember what @{-4}
refers to, but they are aware of the syntax at least.

On the other hand, people who use the - shorthand may or may
not be aware of the @{-n} syntax. In that respect, I think it would
be confusing to display Fast-forwarded HEAD to @{-1} in response
to the command git rebase -; the user may not know what @{-1}
means!

Thus my original point was that I felt displaying a symbolic name in
response to git rebase - was more important than doing so in
response to git rebase @{-1}. The issue isn't about forgetting what
@{-n} refers to, it's whether the user even knows what @{-n} is
supposed to mean.

But in light of your other comments:

 Furthermore, were we to translate @{-1}, does that mean we
 should also translate @{-2} or prior?

 Surely, why not.  If a user is so forgetful to need help remembering
 where s/he was immediately before, wouldn't it be more helpful to
 give here is where you were reminder for older ones to allow them
 to double check they specified the right thing and spot possible
 mistakes?

 ...

 Giving the symbolic name 'master' is good because it is possible
 that the user thought the previous branch was 'frotz', forgetting
 that another branch was checked out tentatively in between, and the
 user ended up rebasing on top of a wrong branch.  Telling what that
 previous branch is is a way to help user spot such a potential
 mistake.  So I am all for making rebase - report what concrete
 branch the branch was replayed on top of, and consider it an incomplete
 improvement if rebase @{-1} (or rebase @{-2}) did not get the
 same help---especially when I know that the underlying mechanism you
 would use to translate @{-1} back to the concrete branch name is the
 same for both cases anyway.

I had not originally thought of this, perhaps because I was preoccupied
with preventing users from seeing syntax they might not be aware of.
But I definitely agree that displaying symbolic names for all @{-n}
is a good way to prevent user error.

 I can buy that would be a lot more work, and I do not want to do it
 (or I do not think I can solve it in a more general way), though.

Perish the thought! :)

I will try to re-roll this patch to include symbolic names for @{-n}.

As usual, thanks for the feedback!

- Brian Gesiak
--
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] git-rebase: Print name of rev when using shorthand

2014-04-13 Thread Brian Gesiak 
The output from a successful invocation of the shorthand command
git rebase - is something like Fast-forwarded HEAD to @{-1},
which includes a relative reference to a revision. Other commands
that use the shorthand -, such as git checkout -, typically
display the symbolic name of the revision.

Change rebase to output the symbolic name of the revision when using
the shorthand. For the example above, the new output is
Fast-forwarded HEAD to master, assuming @{-1} is a reference to
master.

- Use git name-rev to retreive the name of the rev.
- Update the tests in light of this new behavior.

Requested-by: John Keeping j...@keeping.me.uk
Signed-off-by: Brian Gesiak modoca...@gmail.com
---
Previous discussion on this issue:
http://article.gmane.org/gmane.comp.version-control.git/244340

 git-rebase.sh | 2 +-
 t/t3400-rebase.sh | 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 2c75e9f..ab0e081 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -455,7 +455,7 @@ then
*)  upstream_name=$1
if test $upstream_name = -
then
-   upstream_name=@{-1}
+   upstream_name=`git name-rev --name-only @{-1}`
fi
shift
;;
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 80e0a95..2b99940 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -91,7 +91,7 @@ test_expect_success 'rebase from ambiguous branch name' '
 test_expect_success 'rebase off of the previous branch using -' '
git checkout master 
git checkout HEAD^ 
-   git rebase @{-1} expect.messages 
+   git rebase master expect.messages 
git merge-base master HEAD expect.forkpoint 
 
git checkout master 
@@ -100,8 +100,6 @@ test_expect_success 'rebase off of the previous branch 
using -' '
git merge-base master HEAD actual.forkpoint 
 
test_cmp expect.forkpoint actual.forkpoint 
-   # the next one is dubious---we may want to say -,
-   # instead of @{-1}, in the message
test_i18ncmp expect.messages actual.messages
 '
 
-- 
1.9.0.259.gc5d75e8.dirty

--
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] checkout: Fix grammar in inline comment.

2014-04-12 Thread Brian Gesiak 
Inline comment had incorrect grammar. Fix grammatical mistakes and
reflect actual behavior of the function.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 builtin/checkout.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 63151e0..abe1161 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -888,8 +888,8 @@ static int parse_branchname_arg(int argc, const char **argv,
 *
 * case 3: git checkout something [--]
 *
-*   (a) If something is a commit, that is to
-*   switch to the branch or detach HEAD at it.  As a special case,
+*   (a) If something is a commit, switch to that branch or
+*   detach HEAD at that commit.  As a special case,
 *   if something is A...B (missing A or B means HEAD but you can
 *   omit at most one side), and if there is a unique merge base
 *   between A and B, A...B names that merge base.
-- 
1.9.0.259.gc5d75e8.dirty

--
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] git-rebase: Teach rebase - shorthand.

2014-03-19 Thread Brian Gesiak 
Teach rebase the same shorthand as checkout and merge; that is, that -
means the branch we were previously on.

Reported-by: Tim Chase g...@tim.thechases.com
Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 git-rebase.sh |  4 
 t/t3400-rebase.sh | 11 +++
 2 files changed, 15 insertions(+)

diff --git a/git-rebase.sh b/git-rebase.sh
index 5f6732b..2c75e9f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -453,6 +453,10 @@ then
test $fork_point = auto  fork_point=t
;;
*)  upstream_name=$1
+   if test $upstream_name = -
+   then
+   upstream_name=@{-1}
+   fi
shift
;;
esac
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 6d94b1f..6176754 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -88,6 +88,17 @@ test_expect_success 'rebase from ambiguous branch name' '
git rebase master
 '
 
+test_expect_success 'rebase using shorthand' '
+   git checkout master 
+   git checkout -b shorthand HEAD^ 
+   git rebase - 1shorthand.stdout 
+   git checkout master 
+   git branch -D shorthand 
+   git checkout -b shorthand HEAD^ 
+   git rebase @{-1} 1without_shorthand.stdout 
+   test_i18ncmp without_shorthand.stdout shorthand.stdout
+'
+
 test_expect_success 'rebase a single mode change' '
git checkout master 
git branch -D topic 
-- 
1.8.5.2 (Apple Git-48)

--
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] git-rebase: Teach rebase - shorthand.

2014-03-19 Thread Brian Gesiak 
Thank you for the feedback and tweaks!

 Is the eye-candy output to the standard output what is the most
 interesting during the execution of a rebase?  Wouldn't we be
 more interested to make sure that we did transplant the history
 on the same commit between two cases?

I agree. I'll consult the other tests to see how to write such a test.

 Can we use `git name-rev` to put the actual name here, so that people
 who have not done what they intended can hopefully notice sooner?

This sounds like a great idea! Doing so would mirror how `git checkout`
behaves; checkout informs the user of which branch they have switched
to when using the - shorthand: Switched to branch 'master'.

Should I submit a new patch, or reroll this one?

- Brian Gesiak
--
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] git-rebase: Teach rebase - shorthand.

2014-03-18 Thread Brian Gesiak 
Teach rebase the same shorthand as checkout and merge; that is, that -
means the branch we were previously on.

Reported-by: Tim Chase g...@tim.thechases.com
Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 git-rebase.sh | 4 
 t/t3400-rebase.sh | 6 ++
 2 files changed, 10 insertions(+)

diff --git a/git-rebase.sh b/git-rebase.sh
index 5f6732b..2c75e9f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -453,6 +453,10 @@ then
test $fork_point = auto  fork_point=t
;;
*)  upstream_name=$1
+   if test $upstream_name = -
+   then
+   upstream_name=@{-1}
+   fi
shift
;;
esac
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 6d94b1f..00aba9f 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -88,6 +88,12 @@ test_expect_success 'rebase from ambiguous branch name' '
git rebase master
 '
 
+test_expect_success 'rebase using shorthand' '
+   git checkout master
+   git checkout -b shorthand HEAD^
+   GIT_TRACE=1 git rebase -
+'
+
 test_expect_success 'rebase a single mode change' '
git checkout master 
git branch -D topic 
-- 
1.8.5.2 (Apple Git-48)

--
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: [GSoC14][RFC] Proposal Draft: Refactor tempfile handling

2014-03-12 Thread Brian Gesiak 
 Currently the linked list of lockfiles only grows, never shrinks.  Once
 an object has been linked into the list, there is no way to remove it
 again even after the lock has been released.  So if a lock needs to be
 created dynamically at a random place in the code, its memory is
 unavoidably leaked.

Ah yes, I see. I think a good example is
config.git_config_set_multivar_in_file, which even contains a comment
detailing the problem: Since lockfile.c keeps a linked list of all
created lock_file structures, it isn't safe to free(lock).  It's
better to just leave it hanging around.

 But I have a feeling that if we want to use a similar mechanism to
 handle all temporary files (of which there can be more), then it would
 be a good idea to lift this limitation.  It will require some care,
 though, to make sure that record removal is done in a way that is
 threadsafe and safe in the event of all expected kinds of process death.

It sounds like a threadsafe linked-list with an interface to manually
remove elements from the list is the solution here; does that sound
reasonable? Ensuring thread safety without sacrificing readability is
probably more difficult than it sounds, but I don't think it's
impossible.

I'll add some more details on this to my proposal[1]. Thank you!

- Brian Gesiak

[1] 
https://www.google-melange.com/gsoc/proposal/review/student/google/gsoc2014/modocache/5629499534213120
--
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: [GSoC14][RFC] Proposal Draft: Refactor tempfile handling

2014-03-08 Thread Brian Gesiak 
Excellent, thank you very much for the feedback, Jeff! It was very
helpful and encouraging. I've done some more research based on your
comments.

 Once the logic is extracted into a nice API, there are
 several other places that can use it, too: ...

I've found the following four areas so far:

1. lockfile.lock_file
2. git-compat-util.odb_mkstemp
3. git-compat-util.odb_pack_keep
4. diff.prepare_temp_file

Tons of files use (1) and (2). (3) is less common, and (4) is only
used for external diffs.

 the shallow_XX tempfiles

I'm not sure I was able to find this one. Are you referring to the
lock files used when fetching, such as in fetch-pack.c?

 What are the mismatches in how lockfiles and object files are
 handled? E.g., how do we finalize them into place? How should
 the API be designed to minimize race conditions (e.g., if we
 get a signal delivered while we are committing or cleaning up a file)?

I'd say the biggest difference between lockfiles and object files is
that tempfile methods like odb_mkstemp need to know the location of
the object directory. Aside from that, lockfiles and the external diff
files appear to be cleaned up at exit, while temporary object files
tend to have a more finely controlled lifecycle. I'm still
investigating this aspect of the proposal, though.

One question, though: the idea on the ideas page specifies that
temporary pack and object files may optionally be cleaned up in case
of error during program execution. How will users specify their
preference? I think the API for creating temporary files should allow
cleanup options to be specified on a per-file basis. That way each
part of the program that creates tempfiles can specify a different
config value to determine the cleanup policy.

Thanks for all your help so far!

- Brian Gesiak

PS: I'm maintaining a working draft of my proposal here, in case
anyone wants to offer any feedback prior to its submission:
https://gist.github.com/modocache/9434914


On Tue, Mar 4, 2014 at 7:42 AM, Jeff King p...@peff.net wrote:
 On Sun, Mar 02, 2014 at 06:04:39AM +0900, Brian Gesiak wrote:

 My name is Brian Gesiak. I'm a research student at the University of
 Tokyo, and I'm hoping to participate in this year's Google Summer of
 Code by contributing to Git. I'm a longtime user, first-time
 contributor--some of you may have noticed my microproject
 patches.[1][2]

 Yes, we did notice them. Thanks, and welcome. :)

 The ideas page points out that while lock files are closed and
 unlinked[3] when the program exits[4], object pack files implement
 their own brand of temp file creation and deletion. This
 implementation doesn't share the same guarantees as lock files--it is
 possible that the program terminates before the temp file is
 unlinked.[5]

 Lock file references are stored in a linked list. When the program
 exits, this list is traversed and each file is closed and unlinked. It
 seems to me that this mechanism is appropriate for temp files in
 general, not just lock files. Thus, my proposal would be to extract
 this logic into a separate module--tempfile.h, perhaps. Lock and
 object files would share the tempfile implementation.

 That is, both object and lock temp files would be stored in a linked
 list, and all of these would be deleted at program exit.

 Yes, I think this is definitely the right way to go. We should be able
 to unify the tempfile handling for all of git. Once the logic is
 extracted into a nice API, there are several other places that can use
 it, too:

   - the external diff code creates tempfiles and uses its own cleanup
 routines

   - the shallow_XX tempfiles (these are not cleaned right now,
 though I sent a patch recently for them to do their own cleanup)

 Those are just off the top of my head. There may be other spots, too.

 It is worth thinking in your proposal about some of the things that the
 API will want to handle. What are the mismatches in how lockfiles and
 object files are handled? E.g., how do we finalize them into place?
 How should the API be designed to minimize race conditions (e.g., if we
 get a signal delivered while we are committing or cleaning up a file)?

 Please let me know if this seems like it would make for an interesting
 proposal, or if perhaps there is something I am overlooking. Any
 feedback at all would be appreciated. Thank you!

 You definitely have a grasp of what the project is aiming for, and which
 areas need to be touched.

 -Peff
--
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] t3200-branch: test setting branch as own upstream

2014-03-04 Thread Brian Gesiak 
No test asserts that git branch -u refs/heads/my-branch my-branch
emits a warning. Add a test that does so.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 t/t3200-branch.sh | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fcdb867..e6d4015 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -507,6 +507,16 @@ EOF
test_cmp expected actual
 '
 
+test_expect_success '--set-upstream-to shows warning if used to set branch as 
own upstream' '
+   git branch --set-upstream-to refs/heads/my13 my13 2actual 
+   cat expected EOF 
+warning: Not setting branch my13 as its own upstream.
+EOF
+   test_i18ncmp expected actual 
+   test_must_fail git config branch.my13.remote 
+   test_must_fail git config branch.my13.merge
+'
+
 # Keep this test last, as it changes the current branch
 cat expect EOF
 $_z40 $HEAD $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL 1117150200 +
branch: Created from master
-- 
1.8.3.4 (Apple Git-47)

--
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] branch: die when setting branch as own upstream

2014-03-01 Thread Brian Gesiak 
From: modocache modoca...@gmail.com

Branch set as own upstream using one of the following commands returns
immediately with an exit code of 0:

- `git branch --set-upstream-to foo refs/heads/foo`
- `git branch --force --track foo foo`

Since neither of these actions currently set the upstream, an exit code
of 0 is misleading. Instead, exit with a status code indicating failure
by using the die function.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 branch.c  | 9 ++---
 t/t3200-branch.sh | 6 +++---
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/branch.c b/branch.c
index e163f3c..9bac8b5 100644
--- a/branch.c
+++ b/branch.c
@@ -54,13 +54,8 @@ void install_branch_config(int flag, const char *local, 
const char *origin, cons
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
 
-   if (shortname
-!strcmp(local, shortname)
-!origin) {
-   warning(_(Not setting branch %s as its own upstream.),
-   local);
-   return;
-   }
+   if (shortname  !strcmp(local, shortname)  !origin)
+   die(_(Not setting branch %s as its own upstream.), local);
 
strbuf_addf(key, branch.%s.remote, local);
git_config_set(key.buf, origin ? origin : .);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 6164126..3ac493f 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -507,10 +507,10 @@ EOF
test_cmp expected actual
 '
 
-test_expect_success '--set-upstream-to shows warning if used to set branch as 
own upstream' '
-   git branch --set-upstream-to refs/heads/my13 my13 2actual 
+test_expect_success '--set-upstream-to fails if used to set branch as own 
upstream' '
+   test_must_fail git branch --set-upstream-to refs/heads/my13 my13 
2actual 
cat expected EOF 
-warning: Not setting branch my13 as its own upstream.
+fatal: Not setting branch my13 as its own upstream.
 EOF
test_i18ncmp expected actual
 '
-- 
1.8.3.4 (Apple Git-47)

--
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 3/3] branch: die when setting branch as own upstream

2014-03-01 Thread Brian Gesiak 
Branch set as own upstream using one of the following commands returns
immediately with an exit code of 0:

- `git branch --set-upstream-to foo refs/heads/foo`
- `git branch --force --track foo foo`

Since neither of these actions currently set the upstream, an exit code
of 0 is misleading. Instead, exit with a status code indicating failure
by using the die function.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 branch.c  | 9 ++---
 t/t3200-branch.sh | 6 +++---
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/branch.c b/branch.c
index e163f3c..9bac8b5 100644
--- a/branch.c
+++ b/branch.c
@@ -54,13 +54,8 @@ void install_branch_config(int flag, const char *local, 
const char *origin, cons
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
 
-   if (shortname
-!strcmp(local, shortname)
-!origin) {
-   warning(_(Not setting branch %s as its own upstream.),
-   local);
-   return;
-   }
+   if (shortname  !strcmp(local, shortname)  !origin)
+   die(_(Not setting branch %s as its own upstream.), local);
 
strbuf_addf(key, branch.%s.remote, local);
git_config_set(key.buf, origin ? origin : .);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 6164126..3ac493f 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -507,10 +507,10 @@ EOF
test_cmp expected actual
 '
 
-test_expect_success '--set-upstream-to shows warning if used to set branch as 
own upstream' '
-   git branch --set-upstream-to refs/heads/my13 my13 2actual 
+test_expect_success '--set-upstream-to fails if used to set branch as own 
upstream' '
+   test_must_fail git branch --set-upstream-to refs/heads/my13 my13 
2actual 
cat expected EOF 
-warning: Not setting branch my13 as its own upstream.
+fatal: Not setting branch my13 as its own upstream.
 EOF
test_i18ncmp expected actual
 '
-- 
1.8.3.4 (Apple Git-47)

--
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 3/3] branch: die when setting branch as own upstream

2014-03-01 Thread Brian Gesiak 
Sorry for the multiple patches--I noticed the commit author was off in
the first one.

This patch converts the warning to an error, should it be decided that
it's prudent to do so (I'm in favor of doing so). If not, I think the
other two patches I submitted are good to merge.

Thanks for all the feedback so far!

- Brian Gesiak


On Sat, Mar 1, 2014 at 9:23 PM, Brian Gesiak modoca...@gmail.com wrote:
 Branch set as own upstream using one of the following commands returns
 immediately with an exit code of 0:

 - `git branch --set-upstream-to foo refs/heads/foo`
 - `git branch --force --track foo foo`

 Since neither of these actions currently set the upstream, an exit code
 of 0 is misleading. Instead, exit with a status code indicating failure
 by using the die function.

 Signed-off-by: Brian Gesiak modoca...@gmail.com
 ---
  branch.c  | 9 ++---
  t/t3200-branch.sh | 6 +++---
  2 files changed, 5 insertions(+), 10 deletions(-)

 diff --git a/branch.c b/branch.c
 index e163f3c..9bac8b5 100644
 --- a/branch.c
 +++ b/branch.c
 @@ -54,13 +54,8 @@ void install_branch_config(int flag, const char *local, 
 const char *origin, cons
 struct strbuf key = STRBUF_INIT;
 int rebasing = should_setup_rebase(origin);

 -   if (shortname
 -!strcmp(local, shortname)
 -!origin) {
 -   warning(_(Not setting branch %s as its own upstream.),
 -   local);
 -   return;
 -   }
 +   if (shortname  !strcmp(local, shortname)  !origin)
 +   die(_(Not setting branch %s as its own upstream.), local);

 strbuf_addf(key, branch.%s.remote, local);
 git_config_set(key.buf, origin ? origin : .);
 diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
 index 6164126..3ac493f 100755
 --- a/t/t3200-branch.sh
 +++ b/t/t3200-branch.sh
 @@ -507,10 +507,10 @@ EOF
 test_cmp expected actual
  '

 -test_expect_success '--set-upstream-to shows warning if used to set branch 
 as own upstream' '
 -   git branch --set-upstream-to refs/heads/my13 my13 2actual 
 +test_expect_success '--set-upstream-to fails if used to set branch as own 
 upstream' '
 +   test_must_fail git branch --set-upstream-to refs/heads/my13 my13 
 2actual 
 cat expected EOF 
 -warning: Not setting branch my13 as its own upstream.
 +fatal: Not setting branch my13 as its own upstream.
  EOF
 test_i18ncmp expected actual
  '
 --
 1.8.3.4 (Apple Git-47)

--
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

[GSoC14][RFC] Proposal Draft: Refactor tempfile handling

2014-03-01 Thread Brian Gesiak 
Hello all,

My name is Brian Gesiak. I'm a research student at the University of
Tokyo, and I'm hoping to participate in this year's Google Summer of
Code by contributing to Git. I'm a longtime user, first-time
contributor--some of you may have noticed my microproject
patches.[1][2]

I'd like to gather some information on one of the GSoC ideas posted on
the ideas page. Namely, I'm interested in refactoring the way
tempfiles are cleaned up.

The ideas page points out that while lock files are closed and
unlinked[3] when the program exits[4], object pack files implement
their own brand of temp file creation and deletion. This
implementation doesn't share the same guarantees as lock files--it is
possible that the program terminates before the temp file is
unlinked.[5]

Lock file references are stored in a linked list. When the program
exits, this list is traversed and each file is closed and unlinked. It
seems to me that this mechanism is appropriate for temp files in
general, not just lock files. Thus, my proposal would be to extract
this logic into a separate module--tempfile.h, perhaps. Lock and
object files would share the tempfile implementation.

That is, both object and lock temp files would be stored in a linked
list, and all of these would be deleted at program exit.

I'm very enthused about this project--I think it has it all:

- Tangible benefits for the end-user
- Reduced complexity in the codebase
- Ambitious enough to be interesting
- Small enough to realistically be completed in a summer

Please let me know if this seems like it would make for an interesting
proposal, or if perhaps there is something I am overlooking. Any
feedback at all would be appreciated. Thank you!

- Brian Gesiak

[1] http://thread.gmane.org/gmane.comp.version-control.git/242891
[2] http://thread.gmane.org/gmane.comp.version-control.git/242893
[3] https://github.com/git/git/blob/v1.9.0/lockfile.c#L18
[4] https://github.com/git/git/blob/v1.9.0/lockfile.c#L143
[5] https://github.com/git/git/blob/v1.9.0/pack-write.c#L350
--
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 1/2] t3200-branch: test setting branch as own upstream

2014-02-28 Thread Brian Gesiak 
 If you feel like continuing on this series, converting the warning()
 into a die() would be a much more productive use of time (but if you
 don't, I do not see any reason not to take the patches you've posted).

I'd be happy to keep working on this. In fact, I think I have a patch
for this ready. But just to clarify:

 I notice that the warning comes from install_branch_config, which gets
 used both for branch -u, but also in the side effect case I
 mentioned above. Is it possible to trigger this as part of such a case?
 I think maybe git branch -f --track foo foo would do it. If so, we
 should perhaps include a test that it does not break if we upgrade the
 -u case to an error.

Do you mean that install_branch_config should continue to emit a
warning in the side effect case? I'm not sure I agree--how is git
branch -f --track foo foo less erroneous than git branch -u foo
refs/heads/foo? Perhaps I'm missing some insight on how --track is
used.

The tests appear to already cover all instances in which
install_branch_config is called, and bumping the warning to an error
does not cause any test failures.

- Brian Gesiak
--
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 1/2] t3200-branch: test setting branch as own upstream

2014-02-28 Thread Brian Gesiak 
 I just don't want to regress somebody else's workflow due
 to my lack of imagination.

This makes a lot of sense to me, although as-is the function emits a
warning and returns immediately (although with a successful status
code), so I'm also stumped as to what kind of workflow this would be
included in.

In any case, if the jury's out on this one, I suppose the two patches
I submitted are good to merge? Part of me thinks the bump from warning
to error belongs in its own patch anyway.

- Brian Gesiak
--
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 1/2] t3200-branch: test setting branch as own upstream

2014-02-27 Thread Brian Gesiak 
From: modocache modoca...@gmail.com

No test asserts that git branch -u refs/heads/my-branch my-branch
emits a warning. Add a test that does so.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 t/t3200-branch.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fcdb867..f70b96f 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -507,6 +507,14 @@ EOF
test_cmp expected actual
 '
 
+test_expect_success '--set-upstream-to shows warning if used to set branch as 
own upstream' '
+   git branch --set-upstream-to refs/heads/my13 my13 2actual 
+   cat expected EOF 
+warning: Not setting branch my13 as its own upstream.
+EOF
+   test_cmp expected actual
+'
+
 # Keep this test last, as it changes the current branch
 cat expect EOF
 $_z40 $HEAD $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL 1117150200 +
branch: Created from master
-- 
1.8.3.4 (Apple Git-47)

--
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 2/2] branch: use skip_prefix

2014-02-27 Thread Brian Gesiak 
From: modocache modoca...@gmail.com

The install_branch_config function reimplemented the skip_prefix
function inline. Use skip_prefix function instead for brevity.

Signed-off-by: Brian Gesiak modoca...@gmail.com
Reported-by: Michael Haggerty mhag...@alum.mit.edu
---
 branch.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/branch.c b/branch.c
index 723a36b..e163f3c 100644
--- a/branch.c
+++ b/branch.c
@@ -1,3 +1,4 @@
+#include git-compat-util.h
 #include cache.h
 #include branch.h
 #include refs.h
@@ -49,12 +50,11 @@ static int should_setup_rebase(const char *origin)
 
 void install_branch_config(int flag, const char *local, const char *origin, 
const char *remote)
 {
-   const char *shortname = remote + 11;
-   int remote_is_branch = starts_with(remote, refs/heads/);
+   const char *shortname = skip_prefix(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
 
-   if (remote_is_branch
+   if (shortname
 !strcmp(local, shortname)
 !origin) {
warning(_(Not setting branch %s as its own upstream.),
@@ -77,29 +77,29 @@ void install_branch_config(int flag, const char *local, 
const char *origin, cons
strbuf_release(key);
 
if (flag  BRANCH_CONFIG_VERBOSE) {
-   if (remote_is_branch  origin)
+   if (shortname  origin)
printf_ln(rebasing ?
  _(Branch %s set up to track remote branch %s 
from %s by rebasing.) :
  _(Branch %s set up to track remote branch %s 
from %s.),
  local, shortname, origin);
-   else if (remote_is_branch  !origin)
+   else if (shortname  !origin)
printf_ln(rebasing ?
  _(Branch %s set up to track local branch %s 
by rebasing.) :
  _(Branch %s set up to track local branch 
%s.),
  local, shortname);
-   else if (!remote_is_branch  origin)
+   else if (!shortname  origin)
printf_ln(rebasing ?
  _(Branch %s set up to track remote ref %s by 
rebasing.) :
  _(Branch %s set up to track remote ref %s.),
  local, remote);
-   else if (!remote_is_branch  !origin)
+   else if (!shortname  !origin)
printf_ln(rebasing ?
  _(Branch %s set up to track local ref %s by 
rebasing.) :
  _(Branch %s set up to track local ref %s.),
  local, remote);
else
-   die(BUG: impossible combination of %d and %p,
-   remote_is_branch, origin);
+   die(BUG: impossible combination of %p and %p,
+   shortname, origin);
}
 }
 
-- 
1.8.3.4 (Apple Git-47)

--
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 1/2] t3200-branch: test setting branch as own upstream

2014-02-27 Thread Brian Gesiak 
 For an operation like git branch foo origin where setting up the
 tracking is a side effect, a warning makes sense. But the sole purpose
 of the command above is to set the upstream, and we didn't do it; should
 this warning actually be upgraded to an error?

I agree. I originally wrote the test using test_expect_failure--imagine my
surprise when the exit status was 0, despite the fact that the upstream wasn't
set!

 This should use test_i18ncmp, as the string you are matching is
 internationalized.

Patch is on the way, just waiting for the tests to complete. Thanks for pointing
that out! Also, sorry if it's in the Makefile somewhere, but is there
an easy way
to run just a single test file in the t directory?

- Brian Gesiak
--
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/2] t3200-branch: test setting branch as own upstream

2014-02-27 Thread Brian Gesiak 
No test asserts that git branch -u refs/heads/my-branch my-branch
emits a warning. Add a test that does so.

Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 t/t3200-branch.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fcdb867..6164126 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -507,6 +507,14 @@ EOF
test_cmp expected actual
 '
 
+test_expect_success '--set-upstream-to shows warning if used to set branch as 
own upstream' '
+   git branch --set-upstream-to refs/heads/my13 my13 2actual 
+   cat expected EOF 
+warning: Not setting branch my13 as its own upstream.
+EOF
+   test_i18ncmp expected actual
+'
+
 # Keep this test last, as it changes the current branch
 cat expect EOF
 $_z40 $HEAD $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL 1117150200 +
branch: Created from master
-- 
1.8.3.4 (Apple Git-47)

--
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 2/2] branch: use skip_prefix

2014-02-27 Thread Brian Gesiak 
The install_branch_config function reimplemented the skip_prefix
function inline. Use skip_prefix function instead for brevity.

Reported-by: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: Brian Gesiak modoca...@gmail.com
---
 branch.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/branch.c b/branch.c
index 723a36b..e163f3c 100644
--- a/branch.c
+++ b/branch.c
@@ -1,3 +1,4 @@
+#include git-compat-util.h
 #include cache.h
 #include branch.h
 #include refs.h
@@ -49,12 +50,11 @@ static int should_setup_rebase(const char *origin)
 
 void install_branch_config(int flag, const char *local, const char *origin, 
const char *remote)
 {
-   const char *shortname = remote + 11;
-   int remote_is_branch = starts_with(remote, refs/heads/);
+   const char *shortname = skip_prefix(remote, refs/heads/);
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
 
-   if (remote_is_branch
+   if (shortname
 !strcmp(local, shortname)
 !origin) {
warning(_(Not setting branch %s as its own upstream.),
@@ -77,29 +77,29 @@ void install_branch_config(int flag, const char *local, 
const char *origin, cons
strbuf_release(key);
 
if (flag  BRANCH_CONFIG_VERBOSE) {
-   if (remote_is_branch  origin)
+   if (shortname  origin)
printf_ln(rebasing ?
  _(Branch %s set up to track remote branch %s 
from %s by rebasing.) :
  _(Branch %s set up to track remote branch %s 
from %s.),
  local, shortname, origin);
-   else if (remote_is_branch  !origin)
+   else if (shortname  !origin)
printf_ln(rebasing ?
  _(Branch %s set up to track local branch %s 
by rebasing.) :
  _(Branch %s set up to track local branch 
%s.),
  local, shortname);
-   else if (!remote_is_branch  origin)
+   else if (!shortname  origin)
printf_ln(rebasing ?
  _(Branch %s set up to track remote ref %s by 
rebasing.) :
  _(Branch %s set up to track remote ref %s.),
  local, remote);
-   else if (!remote_is_branch  !origin)
+   else if (!shortname  !origin)
printf_ln(rebasing ?
  _(Branch %s set up to track local ref %s by 
rebasing.) :
  _(Branch %s set up to track local ref %s.),
  local, remote);
else
-   die(BUG: impossible combination of %d and %p,
-   remote_is_branch, origin);
+   die(BUG: impossible combination of %p and %p,
+   shortname, origin);
}
 }
 
-- 
1.8.3.4 (Apple Git-47)

--
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 1/2] t3200-branch: test setting branch as own upstream

2014-02-27 Thread Brian Gesiak 
 Hmm. Looks like it is only a problem if you are calling a shell function
 (since it is the shell function's trace output you are seeing). So this
 test would be OK as-is

Indeed, this test passes when run locally, even using sh -x.

I would be in favor of using test_i18ngrep, but it seems like this
test file overwhelmingly uses test_(i18n)cmp, even when inspecting
stderr output. Making double-sure that all tests pass when run with
sh -x seems like a larger endeavor.

Of course, I'd be happy to submit several patches if there's support
for such a change. But as Peff points out it will be a large diff.

- Brian Gesiak

On Fri, Feb 28, 2014 at 4:26 PM, Jeff King p...@peff.net wrote:
 On Fri, Feb 28, 2014 at 02:14:01AM -0500, Jeff King wrote:

 I didn't think we bothered to make sh -x work robustly. I don't mind
 if we do, but git grep -E 'test_(i18n)?cmp .*err shows many potential
 problem spots.

 Just for fun:

   cd t
   make SHELL_PATH=sh -x prove

 causes 326 test failures across 43 scripts. That's slightly misleading,
 because 200 of the failures are all in t0008, and updating one function
 would probably clear up all of them.

 -Peff
--
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