Re: [PATCH 0/3] git name-rev --weight

2012-08-30 Thread Philip Oakley

From: Junio C Hamano gits...@pobox.com
Sent: Wednesday, August 29, 2012 10:17 PM

So here is an attempt to teach name-rev a mode that tries to base
its name on oldest tag that can reach the commit.  It needs the
reset_revision_walk() call recently added to the revision traversal
API, and applies to bcc0a3e (v1.7.11-rc0~111^2~2) or newer.

Note that this can benefit from caching, as the weight of the tag
(rather, the commit that is tagged) will never change once a history
is made, but that part is left as an exercise to the reader.


Is --weight the right term to use for the user (cli) interface? 
Wouldn't '--oldest' (or similar) be a better statement of what is 
desired (absent clock skew).


While 'weight' may be a good internal technical description it didn't 
convey to me what was being sought (maybe -- deepest'?).




It correctly names 0136db586c in the kernel history as based on
v3.5-rc1 as tags/v3.5-rc1~83^2~81^2~76, not on v3.6-rc1, as we saw
on the list recently.

Once it is verified to operate correctly and updated to perform
properly, we can start passing --weight when describe --contains
runs the command.

Junio C Hamano (3):
 name-rev: lose unnecessary typedef
 name_rev: clarify when a new tip-name is assigned to a commit
 name-rev: --weight option (WIP)

builtin/name-rev.c | 142 
-

1 file changed, 120 insertions(+), 22 deletions(-)

--
1.7.12.285.ga3d5fc0



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


git blame shows wrong Not commited yet entries

2012-08-30 Thread Thomas Ackermann
 
Hi,

I am using MsysGit 1.7.11 on WinXP 32 bit and experience the folllowing strange 
behaviour:
 
For a file like File.txt in the repo, git blame file.txt (note the lower 
case)
shows Not commited yet for every single line in the file. 
git blame File.txt (correct upper case spelling) gives the correct output.
core.ignorecase is true so this behaviour is not what I expected.

Is this a bug in MsysGit or some kind of intended behaviour (or bug) in Git?


---
Thomas
--
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] Thunderbird: fix appp.sh format problems

2012-08-30 Thread Marco Stornelli
The current script has got the following problems:

1) It doesn't work if the language used by Thunderbird is not English;
2) The field To: filled by format-patch is not evaluated;
3) The field Cc: is loaded from Cc used in the commit message
instead of using the Cc field filled by format-patch in the email
header. In addition, in the commit message we can find several tags
(acked-by, tested-by, reported-by...), so it'd better to use only the
information provided by format-patch.

Added comments for point 1). Fixed point 2) and 3).

Signed-off-by: Marco Stornelli marco.storne...@gmail.com
---
 contrib/thunderbird-patch-inline/appp.sh |   18 ++
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/contrib/thunderbird-patch-inline/appp.sh 
b/contrib/thunderbird-patch-inline/appp.sh
index 5eb4a51..e6e1b85 100755
--- a/contrib/thunderbird-patch-inline/appp.sh
+++ b/contrib/thunderbird-patch-inline/appp.sh
@@ -6,6 +6,9 @@
 
 # ExternalEditor can be downloaded at http://globs.org/articles.php?lng=enpg=2
 
+# NOTE: You must change some words in this script according to the language
+# used by Mozilla Thunderbird, as Subject, To, Don't remove this line.
+
 CONFFILE=~/.appprc
 
 SEP=-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
@@ -26,17 +29,24 @@ fi
 cd -  /dev/null
 
 SUBJECT=`sed -n -e '/^Subject: /p' ${PATCH}`
-HEADERS=`sed -e '/^'${SEP}'$/,$d' $1`
 BODY=`sed -e 1,/${SEP}/d $1`
 CMT_MSG=`sed -e '1,/^$/d' -e '/^---$/,$d' ${PATCH}`
 DIFF=`sed -e '1,/^---$/d' ${PATCH}`
 
-CCS=`echo -e $CMT_MSG\n$HEADERS | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
-   -e 's/^Signed-off-by: \(.*\)/\1,/gp'`
+export PATCH
+CCS=`perl -e 'local $/=undef; open FILE, $ENV{'PATCH'}; $text=FILE;
+close FILE; $cc = $1 if $text =~ /Cc: (.*?(,\n .*?)*)\n/s; $cc =~ s/\n//g;
+print $cc;'`
+
+TO=`perl -e 'local $/=undef; open FILE, $ENV{'PATCH'}; $text=FILE;
+close FILE; $to = $1 if $text =~ /To: (.*?(,\n .*?)*)\n/s; $to =~ s/\n//g;
+print $to;'`
 
+# Change Subject before next line according to Thunderbird language
 echo $SUBJECT  $1
+# Change To according to Thunderbird language
+echo To: $TO  $1
 echo Cc: $CCS  $1
-echo $HEADERS | sed -e '/^Subject: /d' -e '/^Cc: /d'  $1
 echo $SEP  $1
 
 echo $CMT_MSG  $1
-- 
1.7.3.4
--
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] Thunderbird: fix appp.sh format problems

2012-08-30 Thread Marco Stornelli
The current script has got the following problems:

1) It doesn't work if the language used by Thunderbird is not English;
2) The field To: filled by format-patch is not evaluated;
3) The field Cc: is loaded only from Cc used in the commit message
instead of using even the Cc field filled by format-patch in the email
header.

Added comments for point 1). Fixed point 2) and 3).

Signed-off-by: Marco Stornelli marco.storne...@gmail.com
---
v2: changed the commit message to reflect better the script implementation

 contrib/thunderbird-patch-inline/appp.sh |   18 ++
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/contrib/thunderbird-patch-inline/appp.sh 
b/contrib/thunderbird-patch-inline/appp.sh
index 5eb4a51..e6e1b85 100755
--- a/contrib/thunderbird-patch-inline/appp.sh
+++ b/contrib/thunderbird-patch-inline/appp.sh
@@ -6,6 +6,9 @@
 
 # ExternalEditor can be downloaded at http://globs.org/articles.php?lng=enpg=2
 
+# NOTE: You must change some words in this script according to the language
+# used by Mozilla Thunderbird, as Subject, To, Don't remove this line.
+
 CONFFILE=~/.appprc
 
 SEP=-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
@@ -26,17 +29,24 @@ fi
 cd -  /dev/null
 
 SUBJECT=`sed -n -e '/^Subject: /p' ${PATCH}`
-HEADERS=`sed -e '/^'${SEP}'$/,$d' $1`
 BODY=`sed -e 1,/${SEP}/d $1`
 CMT_MSG=`sed -e '1,/^$/d' -e '/^---$/,$d' ${PATCH}`
 DIFF=`sed -e '1,/^---$/d' ${PATCH}`
 
-CCS=`echo -e $CMT_MSG\n$HEADERS | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
-   -e 's/^Signed-off-by: \(.*\)/\1,/gp'`
+export PATCH
+CCS=`perl -e 'local $/=undef; open FILE, $ENV{'PATCH'}; $text=FILE;
+close FILE; $cc = $1 if $text =~ /Cc: (.*?(,\n .*?)*)\n/s; $cc =~ s/\n//g;
+print $cc;'`
+
+TO=`perl -e 'local $/=undef; open FILE, $ENV{'PATCH'}; $text=FILE;
+close FILE; $to = $1 if $text =~ /To: (.*?(,\n .*?)*)\n/s; $to =~ s/\n//g;
+print $to;'`
 
+# Change Subject before next line according to Thunderbird language
 echo $SUBJECT  $1
+# Change To according to Thunderbird language
+echo To: $TO  $1
 echo Cc: $CCS  $1
-echo $HEADERS | sed -e '/^Subject: /d' -e '/^Cc: /d'  $1
 echo $SEP  $1
 
 echo $CMT_MSG  $1
-- 
1.7.3.4
--
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: GC of alternate object store

2012-08-30 Thread Oswald Buddenhagen
On Wed, Aug 29, 2012 at 08:52:27AM -0700, Junio C Hamano wrote:
 (I won't comment on the other parts in this discussion).
 
which is kinda unfortunate. ;)

 Oswald Buddenhagen o...@kde.org writes:
  i did exacty that. the tags are *still* not populated - git just tries
  very hard to treat them specially.
 
 Doesn't
 
   git push $over_there 'refs/*:refs/remotes/mine/*'
 
 push your tag v1.0 to refs/remotes/mine/v1.0 over there?  The
 version of git I ship seems to do this just fine.
 
as i wrote before, i'm pulling, not pushing, so any differences could be
blamed on that (or git version 1.7.12.23.g948900e).
anyway, it seems this new version does fetch the tags under the remotes'
namespaces, after all - but it still imports them into the global tag
namespace as well, which of course makes a mess with the duplicated
tags.

and for many repos i'm getting something like this (this is kinda new):

Fetching qtbase
remote: Counting objects: 62375, done.
remote: Compressing objects: 100% (28049/28049), done.
remote: Total 55704 (delta 45280), reused 36646 (delta 27368)
Receiving objects: 100% (55704/55704), 16.76 MiB | 4.94 MiB/s, done.
Resolving deltas: 100% (45280/45280), completed with 3017 local objects.
fatal: bad object 90f0f499ec5953d60d616a2ff541ecaf8b0c31a2
error: ../qt5/qtbase did not send all necessary objects

both the aggregator and the fetched repos run cleanly through fsck.

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


dangling submodule references after rebase

2012-08-30 Thread Stijn Souffriau

Hi all,

I am using a repository that has a sub module which is being committed 
to frequently by myself as well as others. Because of the heavy 
concurrent development I need to do a lot of rebasing. Since the sub 
module commit hashes referenced by the parent repository can become 
dangling as a result of rebasing the sub module I am required to do lots 
of manual fixing of the references in the parent repository using an 
interactive rebase. This is a tedious, error-prone procedure which I 
would like to automate.


I was wondering if anyone has thought about solving this problem yet in 
the past and what might be a good solution?


I was thinking something along the lines of extending the add and commit 
commands so that a parent repository would signal to the sub modules 
that it's index or some if it's commits reference certain sub module 
commits; and also the rebase command so that it would update the parent 
repository commits with new hashes using the information stored by the 
add or commit commands. The procedure would have to be made recursive 
because changing commits in the parent repository might also require 
changing commits in it's parent repository as well.


I'm still no quite sure for which sub module rebase operations the 
referencing parent repository commits would actually have to be 
updated. The reason being that the rebased commits might still be 
referenced by another branch and so they might continue to exist after 
the rebase which raises the question if the parent repository commits 
need to be udated or not. I think this question would have to be 
answered by the add and commit commands which would also have to specify 
a referenced branch in addition to referenced commits so that the parent 
repo commits would only have to be updated if the commits on this branch 
are rebased. By default this could be the branch checked out in the sub 
module at the time the referencing commit was made.


For obvious reasons this should only be done for newly made, unpushed 
and unpulled commits in the repository. However, it might be interesting 
to also enable people to manually bind a parent repo commits to a 
submodule branch so that the commits in this parent repo branch are 
updated when the sub module branch is rebased.


I would like see this feature end up in the mainline and so I'm very 
interested in your opinions.


Thanks,

Stijn

--
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: relative objects/info/alternates doesn't work on remote SMB repo

2012-08-30 Thread Nguyen Thai Ngoc Duy
On Wed, Aug 29, 2012 at 1:43 PM, Orgad and Raizel Shaneh
org...@gmail.com wrote:
 Hi,

 I have a repo accessed through //server/share/foo/repo (Using msysgit).

 .git/objects/info/alternates contains '../../../bare/objects'

 Running 'git status' (or almost any other action) gives the following output:
 error: object directory /server/share/foo/bare/objects does not exist;
 check .git/objects/info/alternates.

 Note that it tries to access /server instead of //server.

Could be path normalization. What does git rev-parse --git-dir say?
Try to run it at top working directory and a subdirectory as well.

If you set GIT_OBJECT_DIRECTORY environment variable to
//server/share/foo/repo/.git/objects, does it work?
-- 
Duy
--
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 3/3] checkout: reorder option handling

2012-08-30 Thread Nguyễn Thái Ngọc Duy
checkout operates in three different modes. On top of that it tries to
be smart by guessing the branch name for switching. This results in
messy option handling code. This patch reorders it so that

 - cmd_checkout() is responsible for parsing, preparing input and
   determining mode

 - Code of each mode is in checkout_paths() and checkout_branch(),
   where sanity checks are performed

Another slight improvement is always print branch name (or commit
name) when printing errors related ot them. This helps catch the case
where an option is mistaken as branch/commit.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Changes since v2 (the first two patches are not resent):

  - leave track mode unspecified if --detach/--orphan is specified
  - merge cmd_checkout_entry() into checkout_paths()
  - rename cmd_switch() to checkout_branch()

 builtin/checkout.c | 177 ++---
 1 tập tin đã bị thay đổi, 102 được thêm vào(+), 75 bị xóa(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 78abaeb..b0c5133 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -217,7 +217,8 @@ static int checkout_merged(int pos, struct checkout *state)
return status;
 }
 
-static int checkout_paths(const struct checkout_opts *opts)
+static int checkout_paths(const struct checkout_opts *opts,
+ const char *revision)
 {
int pos;
struct checkout state;
@@ -229,7 +230,35 @@ static int checkout_paths(const struct checkout_opts *opts)
int stage = opts-writeout_stage;
int merge = opts-merge;
int newfd;
-   struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
+   struct lock_file *lock_file;
+
+   if (opts-track != BRANCH_TRACK_UNSPECIFIED)
+   die(_(%s cannot be used with updating paths), --track);
+
+   if (opts-new_branch_log)
+   die(_(%s cannot be used with updating paths), -l);
+
+   if (opts-force  opts-patch_mode)
+   die(_(%s cannot be used with updating paths), -f);
+
+   if (opts-force_detach)
+   die(_(%s cannot be used with updating paths), --detach);
+
+   if (opts-merge  opts-patch_mode)
+   die(_(%s cannot be used with %s), --merge, --patch);
+
+   if (opts-force  opts-merge)
+   die(_(%s cannot be used with %s), -f, -m);
+
+   if (opts-new_branch)
+   die(_(Cannot update paths and switch to branch '%s' at the 
same time.),
+   opts-new_branch);
+
+   if (opts-patch_mode)
+   return run_add_interactive(revision, --patch=checkout,
+  opts-pathspec);
+
+   lock_file = xcalloc(1, sizeof(struct lock_file));
 
newfd = hold_locked_index(lock_file, 1);
if (read_cache_preload(opts-pathspec)  0)
@@ -763,11 +792,6 @@ static int git_checkout_config(const char *var, const char 
*value, void *cb)
return git_xmerge_config(var, value, NULL);
 }
 
-static int interactive_checkout(const char *revision, const char **pathspec)
-{
-   return run_add_interactive(revision, --patch=checkout, pathspec);
-}
-
 struct tracking_name_data {
const char *name;
char *remote;
@@ -930,6 +954,51 @@ static int switch_unborn_to_new_branch(const struct 
checkout_opts *opts)
return status;
 }
 
+static int checkout_branch(struct checkout_opts *opts,
+  struct branch_info *new)
+{
+   if (opts-pathspec)
+   die(_(paths cannot be used with switching branches));
+
+   if (opts-patch_mode)
+   die(_(%s cannot be used with switching branches),
+   --patch);
+
+   if (opts-writeout_stage)
+   die(_(%s cannot be used with switching branches),
+   --ours/--theirs);
+
+   if (opts-force  opts-merge)
+   die(_(%s cannot be used with %s), -f, -m);
+
+   if (opts-force_detach  opts-new_branch)
+   die(_(%s cannot be used with %s),
+   --detach, -b/-B/--orphan);
+
+   if (opts-new_orphan_branch) {
+   if (opts-track != BRANCH_TRACK_UNSPECIFIED)
+   die(_(%s cannot be used with %s), --orphan, -t);
+   } else if (opts-force_detach) {
+   if (opts-track != BRANCH_TRACK_UNSPECIFIED)
+   die(_(%s cannot be used with %s), --detach, -t);
+   } else if (opts-track == BRANCH_TRACK_UNSPECIFIED)
+   opts-track = git_branch_track;
+
+   if (new-name  !new-commit)
+   die(_(Cannot switch branch to a non-commit '%s'.),
+   new-name);
+
+   if (!new-commit  opts-new_branch) {
+   unsigned char rev[20];
+   int flag;
+
+   if (!read_ref_full(HEAD, rev, 0, flag) 
+   (flag  REF_ISSYMREF)  is_null_sha1(rev))
+   return switch_unborn_to_new_branch(opts);

Re: [PATCH 2/2] commit: use a priority queue in merge base functions

2012-08-30 Thread Jeff King
On Wed, Aug 29, 2012 at 05:05:40PM -0400, Jeff King wrote:

 You would want this on top:
 [...]
 but t6024 still fails (it clearly is finding a different merge base than
 the test expects).  I'll trace through it, but it will have to be later
 tonight.

The problem in t6024 is caused by the fact that the commit timestamps
for every commit are identical. The linear commit_list has the property
that we always insert a new commit at the end of a chain of commits with
the same timestamp. So it works as a stable priority queue in the sense
that items with the same priority are returned in insertion order.

But the heap-based priority queue does not. Nor can it do so without
extra storage requirements, as heaps are inherently unstable. The
simplest way to make it stable is to add an insertion counter to the
comparison function. The patch below does this, and it resolves the
issue. It does waste one int per queue element.

I think you could also make a priority queue based on a binary search
tree that would be stable. It's slightly less efficient to create an
initial queue (you can heapify in O(n), but building the tree takes O(n
lg n)).  But we do not care about that, as we always build the queue by
inserting elements, anyway.  The other downside of using a tree is that
you would want a self-balancing tree for good performance (especially
since we tend to insert commits in sorted order), which increases the
code complexity.

Anyway, since this isn't yielding any performance benefit, I'm not going
to go down that route. But stability of the queue is something that we
need to consider if we ever do replace commit_list with a different data
structure.

Here's the patch to make the existing priority queue stable (by wasting
space) in case we ever want to use it.

-Peff

diff --git a/commit.c b/commit.c
index 8259871..a99d909 100644
--- a/commit.c
+++ b/commit.c
@@ -593,7 +593,7 @@ static int interesting(struct queue *q)
 {
int i;
for (i = 0; i  q-nr; i++) {
-   struct commit *commit = q-items[i];
+   struct commit *commit = q-items[i].item;
if (commit-object.flags  STALE)
continue;
return 1;
diff --git a/queue.c b/queue.c
index 7be6b86..1bdd948 100644
--- a/queue.c
+++ b/queue.c
@@ -3,18 +3,28 @@ static void queue_heapify_up(struct queue *pq)
 
 static inline void queue_swap(struct queue *pq, unsigned i, unsigned j)
 {
-   void *tmp = pq-items[i];
+   struct queue_item tmp = pq-items[i];
pq-items[i] = pq-items[j];
pq-items[j] = tmp;
 }
 
+static inline int queue_cmp(struct queue *pq, unsigned i, unsigned j)
+{
+   int cmp = pq-cmp(pq-items[i].item, pq-items[j].item);
+   if (cmp)
+   return cmp;
+   if (pq-items[i].counter  pq-items[j].counter)
+   return 1;
+   return -1;
+}
+
 static void queue_heapify_up(struct queue *pq)
 {
unsigned i = pq-nr - 1;
while (i  0) {
int parent = (i-1)/2;
 
-   if (pq-cmp(pq-items[i], pq-items[parent]) = 0)
+   if (queue_cmp(pq, i, parent) = 0)
return;
 
queue_swap(pq, i, parent);
@@ -25,7 +35,9 @@ void queue_insert(struct queue *pq, void *item)
 void queue_insert(struct queue *pq, void *item)
 {
ALLOC_GROW(pq-items, pq-nr + 1, pq-alloc);
-   pq-items[pq-nr++] = item;
+   pq-items[pq-nr].item = item;
+   pq-items[pq-nr].counter = pq-counter++;
+   pq-nr++;
queue_heapify_up(pq);
 }
 
@@ -35,11 +47,9 @@ static void queue_heapify_down(struct queue *pq)
while (1) {
int largest = i, left = 2*i + 1, right = 2*i + 2;
 
-   if (left  pq-nr 
-   pq-cmp(pq-items[left], pq-items[largest])  0)
+   if (left  pq-nr  queue_cmp(pq, left, largest)  0)
largest = left;
-   if (right  pq-nr 
-   pq-cmp(pq-items[right], pq-items[largest])  0)
+   if (right  pq-nr  queue_cmp(pq, right, largest)  0)
largest = right;
 
if (largest == i)
@@ -52,7 +62,7 @@ void *queue_peek(struct queue *pq)
 
 void *queue_peek(struct queue *pq)
 {
-   return pq-nr ? pq-items[0] : NULL;
+   return pq-nr ? pq-items[0].item : NULL;
 }
 
 void *queue_pop(struct queue *pq)
@@ -61,7 +71,7 @@ void *queue_pop(struct queue *pq)
 
if (!pq-nr)
return NULL;
-   ret = pq-items[0];
+   ret = pq-items[0].item;
 
pq-items[0] = pq-items[--pq-nr];
queue_heapify_down(pq);
diff --git a/queue.h b/queue.h
index cc471b5..a70f7d7 100644
--- a/queue.h
+++ b/queue.h
@@ -3,11 +3,17 @@ struct queue {
 
 typedef int (*queue_comparison_func_t)(const void *, const void *);
 
+struct queue_item {
+   void *item;
+   unsigned counter;
+};
+
 struct queue {
queue_comparison_func_t cmp;
-   void **items;
+   struct queue_item *items;

Re: [PATCH 2/2] commit: use a priority queue in merge base functions

2012-08-30 Thread Jeff King
On Thu, Aug 30, 2012 at 08:54:21AM -0400, Jeff King wrote:

 On Wed, Aug 29, 2012 at 05:05:40PM -0400, Jeff King wrote:
 
  You would want this on top:
  [...]
  but t6024 still fails (it clearly is finding a different merge base than
  the test expects).  I'll trace through it, but it will have to be later
  tonight.
 
 The problem in t6024 is caused by the fact that the commit timestamps
 for every commit are identical.

So I was able to have my queue behave just like commit_list by fixing
the stability issue. But I still have no clue what is going on in t6024.
It does this for each commit it makes:

  [...]
  GIT_AUTHOR_DATE=2006-12-12 23:00:00 git commit -m 1 a1 
  [...]
  GIT_AUTHOR_DATE=2006-12-12 23:00:01 git commit -m A a1 
  [...]

which is just bizarre. At first I thought it was buggy, and that it
really wanted to be setting COMMITTER_DATE (in which case it should
really just be using test_tick, anyway). But if you do that, the test
fails (even using a regular commit_list)!

So is the test buggy? Or are the identical commit timestamps part of the
intended effect? I can't see how that would be, since:

  1. You would need to set COMMITTER_DATE for that anyway, as you are
 otherwise creating a race condition.

  2. Why would you set AUTHOR_DATE? It's not used by the merge code at
 all.

The script originally comes from here:

  http://thread.gmane.org/gmane.comp.version-control.git/33566/focus=33852

and the discussion implies that the AUTHOR_DATEs were added to avoid a
race condition with the timestamps. But why would that ever have worked?

Confused...

-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


Re: relative objects/info/alternates doesn't work on remote SMB repo

2012-08-30 Thread Orgad and Raizel Shaneh
On Thu, Aug 30, 2012 at 3:51 PM, Nguyen Thai Ngoc Duy pclo...@gmail.com wrote:

 On Wed, Aug 29, 2012 at 1:43 PM, Orgad and Raizel Shaneh
 org...@gmail.com wrote:
  Hi,
 
  I have a repo accessed through //server/share/foo/repo (Using msysgit).
 
  .git/objects/info/alternates contains '../../../bare/objects'
 
  Running 'git status' (or almost any other action) gives the following
  output:
  error: object directory /server/share/foo/bare/objects does not exist;
  check .git/objects/info/alternates.
 
  Note that it tries to access /server instead of //server.

 Could be path normalization. What does git rev-parse --git-dir say?
 Try to run it at top working directory and a subdirectory as well.

 If you set GIT_OBJECT_DIRECTORY environment variable to
 //server/share/foo/repo/.git/objects, does it work?
 --
 Duy

git rev-parse --git-dir in a subdirectory has //server

setting GIT_OBJECT_DIRECTORY prints fatal: bad object HEAD on git status.

- Orgad
--
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: relative objects/info/alternates doesn't work on remote SMB repo

2012-08-30 Thread Nguyen Thai Ngoc Duy
On Thu, Aug 30, 2012 at 8:12 PM, Orgad and Raizel Shaneh
org...@gmail.com wrote:
 Could be path normalization. What does git rev-parse --git-dir say?
 Try to run it at top working directory and a subdirectory as well.

 If you set GIT_OBJECT_DIRECTORY environment variable to
 //server/share/foo/repo/.git/objects, does it work?

 git rev-parse --git-dir in a subdirectory has //server

Hmm where is your git repository? That does not look like a git
repository's path.

 setting GIT_OBJECT_DIRECTORY prints fatal: bad object HEAD on git status.

I guessed you put your repo in .../repo/.git, but I was probably
wrong. Try setting again, pointing GIT_OBJECT_DIRECTORY to the
objects directory inside your repository. I just want to make see if
it's because git miscalculates this path. If setting the env variable
works, then it probably does.
-- 
Duy
--
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/2] commit: use a priority queue in merge base functions

2012-08-30 Thread Jeff King
On Thu, Aug 30, 2012 at 09:03:27AM -0400, Jeff King wrote:

 So I was able to have my queue behave just like commit_list by fixing
 the stability issue. But I still have no clue what is going on in t6024.
 It does this for each commit it makes:
 
   [...]
   GIT_AUTHOR_DATE=2006-12-12 23:00:00 git commit -m 1 a1 
   [...]
   GIT_AUTHOR_DATE=2006-12-12 23:00:01 git commit -m A a1 
   [...]
 
 which is just bizarre. At first I thought it was buggy, and that it
 really wanted to be setting COMMITTER_DATE (in which case it should
 really just be using test_tick, anyway). But if you do that, the test
 fails (even using a regular commit_list)!
 
 So is the test buggy? Or are the identical commit timestamps part of the
 intended effect? I can't see how that would be, since:
 
   1. You would need to set COMMITTER_DATE for that anyway, as you are
  otherwise creating a race condition.
 
   2. Why would you set AUTHOR_DATE? It's not used by the merge code at
  all.
 
 The script originally comes from here:
 
   http://thread.gmane.org/gmane.comp.version-control.git/33566/focus=33852
 
 and the discussion implies that the AUTHOR_DATEs were added to avoid a
 race condition with the timestamps. But why would that ever have worked?

That thread mentions that this is to fix Shawn's bug, which I think is
this:

  http://article.gmane.org/gmane.comp.version-control.git/33559

IOW, the real thing that t6024 is trying to test is that we handle the
fake empty tree properly. And the AUTHOR_DATEs probably were just there
to try to fix the race condition, and they should really just be
test_ticks (and I can't see how they ever would have helped anything; I
suspect they were a placebo inserted at the same time as another change,
and got credited with fixing the race).

That still leaves the question of why the test fails when the commits
get distinct timestamps.

-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


Re: relative objects/info/alternates doesn't work on remote SMB repo

2012-08-30 Thread Orgad and Raizel Shaneh
On Thu, Aug 30, 2012 at 4:22 PM, Nguyen Thai Ngoc Duy pclo...@gmail.com wrote:
 On Thu, Aug 30, 2012 at 8:12 PM, Orgad and Raizel Shaneh
 org...@gmail.com wrote:
 Could be path normalization. What does git rev-parse --git-dir say?
 Try to run it at top working directory and a subdirectory as well.

 If you set GIT_OBJECT_DIRECTORY environment variable to
 //server/share/foo/repo/.git/objects, does it work?

 git rev-parse --git-dir in a subdirectory has //server

 Hmm where is your git repository? That does not look like a git
 repository's path.


Let me try to explain again.
I have /d/share/bare, which is a bare repository, and /d/share/repo
which is a clone with a relative path to bare/.git/objects in its
.git/objects/info/alternates

D:\share is configured as a SMB shared folder. It is accessed using
//server/share.
I do not clone from this directory, but work directly in it using 'cd
//server/share', then performing git operations.

 setting GIT_OBJECT_DIRECTORY prints fatal: bad object HEAD on git status.

 I guessed you put your repo in .../repo/.git, but I was probably
 wrong. Try setting again, pointing GIT_OBJECT_DIRECTORY to the
 objects directory inside your repository. I just want to make see if
 it's because git miscalculates this path. If setting the env variable
 works, then it probably does.
 --
 Duy

Same result. fatal: bad object HEAD. Tried even using a full (local)
path to the objects dir.

- Orgad
--
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: [RFH] .mailmap late summer cleaning

2012-08-30 Thread Erik Faye-Lund
On Wed, Aug 29, 2012 at 7:16 PM, Junio C Hamano gits...@pobox.com wrote:
 We have the mailmap mechanism to unify entries for a person with
 multiple e-mail addresses into one Name address, but it seems
 that the .mailmap file hasn't been keeping up to the changes in the
 real world.

 Does any of you contributors who has commits in my tree want to
 update how you appear in shortlog -e -s -n output?  It may show
 your ancient e-mail address, for example.  If so, here is what I
 want you to do.

snip

 I'll collect responses to this message, and update the file sometime
 late next week.


Erik Faye-Lund kusmab...@gmail.com kusmab...@googlemail.com
--
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: [RFH] .mailmap late summer cleaning

2012-08-30 Thread Ralf Thielow
On Wed, Aug 29, 2012 at 7:16 PM, Junio C Hamano gits...@pobox.com wrote:
...
 Martin von Zweigbergk martinv...@gmail.com y...@google.com

 I'll collect responses to this message, and update the file sometime
 late next week.


Ralf Thielow ralf.thie...@gmail.com ralf.thie...@googlemail.com
--
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 0/3] git name-rev --weight

2012-08-30 Thread Junio C Hamano
Philip Oakley philipoak...@iee.org writes:

 Is --weight the right term to use for the user (cli) interface?
 Wouldn't '--oldest' (or similar) be a better statement of what is
 desired (absent clock skew).

 While 'weight' may be a good internal technical description it didn't
 convey to me what was being sought (maybe -- deepest'?).

I agree with you that weight represents what it internally does.  I
however think that oldest is not quite good, as it still leaves
the source of possible confusion.  It has at least 3 (or 4,
depending on how you count) possible meanings.

 - Is it the one with the oldest timestamp (and if so, do we use the
   committer date, or do we use the tagger date that may be much
   newer than the committer date)?

 - Is it the one with its longest path down to the root is the
   shortest (i.e. with smallest generation number)?

 - Is it the one with the smallest number of ancestor commits?

For the purpose of oldest tag that contains this commit, I think
the last one would give the most intuitive answer, but depending on
your use case, you may want to enhance the command to support other
definition of oldest; it does not feel quite right to have this
particular definition (the last one) squat on the generic --oldest
name.

We could punt to tautology and call it --contains, meaning that is
the logic used to implement describe --contains ;-) but that is
not satisfactory, either.

I dunno.
--
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: GC of alternate object store

2012-08-30 Thread Junio C Hamano
Oswald Buddenhagen o...@kde.org writes:

 Doesn't
 
  git push $over_there 'refs/*:refs/remotes/mine/*'
 
 push your tag v1.0 to refs/remotes/mine/v1.0 over there?  The
 version of git I ship seems to do this just fine.
 
 as i wrote before, i'm pulling, not pushing,...

You would need to decline the automatic tag following with --no-tags
(which in hindsight is misnamed; it really means do not auto-follow
tags), like so:

cd $over_there 
git fetch --no-tags $my_repository 'refs/*:refs/remotes/mine/*'

Otherwise, you will also get tags in refs/tags/.
--
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 3/3] checkout: reorder option handling

2012-08-30 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:

  Changes since v2 (the first two patches are not resent):
 ...
   - merge cmd_checkout_entry() into checkout_paths()

I didn't think of this when I reviewed the previous one; I think it
makes sense.  Thanks.
--
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/2] commit: use a priority queue in merge base functions

2012-08-30 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Anyway, since this isn't yielding any performance benefit, I'm not going
 to go down that route. But stability of the queue is something that we
 need to consider if we ever do replace commit_list with a different data
 structure.

 Here's the patch to make the existing priority queue stable (by wasting
 space) in case we ever want to use it.

Thanks for being thorough and showing a good analysis.

If we want stability, the space to hold insertion counter is not
wasted, by the way.

I actually think the generic queue implementation may want to
handle elements that are not just single pointers.  Just like
qsort() is not about sorting an array of pointers but it is about
sorting an array of elements of caller specified size, perhaps
queue can hold elements of size the caller specified (set in stone
when the queue is initialized).

Then, a caller that wants a stable priority queue of commits can
tell the queue to manage struct { struct commit *c; int gen; },
use the gen field for stability in its comparison callback, etc.,
while a caller that does not need stability can tell the queue to
manage struct commit *.  That way, the generic queue layer does
not have to worry about wasting the insertion counter space, no?


--
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] Support for setitimer() on platforms lacking it

2012-08-30 Thread Joachim Schmitz
 From: Junio C Hamano [mailto:gits...@pobox.com]
 Sent: Tuesday, August 28, 2012 10:16 PM
 To: Joachim Schmitz
 Cc: git@vger.kernel.org
 Subject: Re: [PATCH 1/2] Support for setitimer() on platforms lacking it
 
 Joachim Schmitz j...@schmitz-digital.de writes:
 
  Implementation includes getitimer(), but for now it is static.
  Supports ITIMER_REAL only.
 
  Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
  ---
  May need a header file for ITIMER_*, struct itimerval and the prototypes,
  But for now, and the HP NonStop platform this isn't needed, here
  sys/time has ITIMER_* and struct timeval, and the prototypes can
  vo into git-compat-util.h for now (Patch 2/2)
 
   compat/itimer.c | 50 ++
   1 file changed, 50 insertions(+)
   create mode 100644 compat/itimer.c
 
  diff --git a/compat/itimer.c b/compat/itimer.c
  new file mode 100644
  index 000..713f1ff
  --- /dev/null
  +++ b/compat/itimer.c
  @@ -0,0 +1,50 @@
  +#include ../git-compat-util.h
  +
  +static int git_getitimer(int which, struct itimerval *value)
  +{
  +   int ret = 0;
  +
  +   switch (which) {
  +   case ITIMER_REAL:
  +   value-it_value.tv_usec = 0;
  +   value-it_value.tv_sec = alarm(0);
  +   ret = 0; /* if alarm() fails, we get a SIGLIMIT */
  +   break;
  +   case ITIMER_VIRTUAL: /* FALLTHRU */
  +   case ITIMER_PROF: errno = ENOTSUP; ret = -1; break;
  +   default: errno = EINVAL; ret = -1;
  +   }
 
 Just a style thing, but we align case arms and switch statements,
 like this:
 
   switch (which) {
 case ...:
   stmt;
 break;
   default:
   stmt;
 break;
   }

OK, I'll fix the syle

 Because alarm() runs in integral seconds granularity, this could
 return 0.0 sec (i.e. do not re-trigger this alarm any more) in
 ovalue after setting alarm(1) (via git_setitimer()) and calling this
 function (via git_setitimer() again) before the timer expires, no?
 Is it a desired behaviour?

Unintentional, never really thought about this.
 
 What I am most worried about is that callers _might_ take this
 emulation too seriously, grab the remainder from getitimer(), and
 drives a future call to getitimer() with the returned value, and
 accidentally cause the recurring nature of the request to be
 disabled.
 
 I see no existing code calls setitimer() with non-NULL ovalue, and I
 do not think we would add a new caller that would do so in any time
 soon, so it may not be a bad idea to drop support of returning the
 remaining timer altogether from this emulation layer (just like
 giving anything other than ITIMER_REAL gives us ENOTSUP).  That
 would sidestep the whole we cannot answer how many milliseconds are
 still remaining on the timer when using emulation based on alarm().

Should we leave tv_usec untouched then? That was we round up on the next (and 
subsequent?) round(s). Or just set to ENOTSUP in
setitimer if ovalue is !NULL?

  +int git_setitimer(int which, const struct itimerval *value,
  +   struct itimerval *ovalue)
  +{
  +   int ret = 0;
  +
  +   if (!value
  +   || value-it_value.tv_usec  0
  +   || value-it_value.tv_usec  100
  +   || value-it_value.tv_sec  0) {
  +   errno = EINVAL;
  +   return -1;
  +   }
  +
  +   else if (ovalue)
  +   if (!git_getitimer(which, ovalue))
  +   return -1; /* errno set in git_getitimer() */
  +
  +   else
  +   switch (which) {
  +   case ITIMER_REAL:
  +   alarm(value-it_value.tv_sec +
  +   (value-it_value.tv_usec  0) ? 1 : 0);
 
 Why is this capped to 1 second?  Is this because no existing code
 uses the timer for anything other than 1 second or shorter?  If that
 is the case, that needs at least some documenting (or a possibly
 support for longer expiration, if it is not too cumbersome to add).

As you mention alarm() has only seconds resolution. It is tv_sec plus 1 if 
there are tv_usecs  0, it is rounding up, so we don't
cancel the alarm() if tv_sec is 0 but tv_usec is not. Looks OK to me?
 
  +   ret = 0; /* if alarm() fails, we get a SIGLIMIT */
  +   break;
  +   case ITIMER_VIRTUAL: /* FALLTHRU */
  +   case ITIMER_PROF: errno = ENOTSUP; ret = -1; break;
 
 Please don't add a misleading fallthru label here.  We do not say
 fallthru when two case arms do _exactly_ the same thing.  Only
 when the one arm does some pre-action before the common action, i.e.
 
   switch (which) {
 case one:
   do some thing specific to one;
 /* fallthru */
   case two:
   do some thing common between one and two;
   break;
   }
 
 we label it fallthru to make it clear to the readers that it is
 not missing a break but is 

Re: [PATCH v2] Thunderbird: fix appp.sh format problems

2012-08-30 Thread Junio C Hamano
Marco Stornelli marco.storne...@gmail.com writes:

 The current script has got the following problems:

 1) It doesn't work if the language used by Thunderbird is not English;
 2) The field To: filled by format-patch is not evaluated;
 3) The field Cc: is loaded only from Cc used in the commit message
 instead of using even the Cc field filled by format-patch in the email
 header.

 Added comments for point 1). Fixed point 2) and 3).

 Signed-off-by: Marco Stornelli marco.storne...@gmail.com
 ---
 v2: changed the commit message to reflect better the script implementation

I actually thought what the log message of the previous version
claimed to do was much more sensible.

The language used in the above 3 items describe what you perceive as
a problem, but it is unclear what the desired behaviour that is
different from the current one is.  (2) ... is not evaluated
implies , which is a problem, so fix it by evaluating it, but
there is no single obvious fix to (3) loaded only from Cc in commit
and not Cc from format-patch.  Should it ignore Cc from commit log
message, or should it take Cc from both log message and e-mail
header?  I personally think it should the former, but it is not
clear which you think is the right thing to do (or perhaps you have
a third answer) if you only say Fixed point 3.

  contrib/thunderbird-patch-inline/appp.sh |   18 ++
  1 files changed, 14 insertions(+), 4 deletions(-)

 diff --git a/contrib/thunderbird-patch-inline/appp.sh 
 b/contrib/thunderbird-patch-inline/appp.sh
 index 5eb4a51..e6e1b85 100755
 --- a/contrib/thunderbird-patch-inline/appp.sh
 +++ b/contrib/thunderbird-patch-inline/appp.sh
 @@ -6,6 +6,9 @@
  
  # ExternalEditor can be downloaded at 
 http://globs.org/articles.php?lng=enpg=2
  
 +# NOTE: You must change some words in this script according to the language
 +# used by Mozilla Thunderbird, as Subject, To, Don't remove this line.
 +
  CONFFILE=~/.appprc
  
  SEP=-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
 @@ -26,17 +29,24 @@ fi
  cd -  /dev/null
  
  SUBJECT=`sed -n -e '/^Subject: /p' ${PATCH}`
 -HEADERS=`sed -e '/^'${SEP}'$/,$d' $1`
  BODY=`sed -e 1,/${SEP}/d $1`
  CMT_MSG=`sed -e '1,/^$/d' -e '/^---$/,$d' ${PATCH}`
  DIFF=`sed -e '1,/^---$/d' ${PATCH}`
  
 -CCS=`echo -e $CMT_MSG\n$HEADERS | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
 - -e 's/^Signed-off-by: \(.*\)/\1,/gp'`
 +export PATCH
 +CCS=`perl -e 'local $/=undef; open FILE, $ENV{'PATCH'}; $text=FILE;
 +close FILE; $cc = $1 if $text =~ /Cc: (.*?(,\n .*?)*)\n/s; $cc =~ s/\n//g;
 +print $cc;'`
 +
 +TO=`perl -e 'local $/=undef; open FILE, $ENV{'PATCH'}; $text=FILE;
 +close FILE; $to = $1 if $text =~ /To: (.*?(,\n .*?)*)\n/s; $to =~ s/\n//g;
 +print $to;'`
  
 +# Change Subject before next line according to Thunderbird language
  echo $SUBJECT  $1

before next line???  I do not think you meant to rename the
variable $SUBJECT to $localized string for subject.

 +# Change To according to Thunderbird language

# Change To:  according to...

would be less confusing, as the line has to to on it when viewed
case insensitively.

 +echo To: $TO  $1

  echo Cc: $CCS  $1
 -echo $HEADERS | sed -e '/^Subject: /d' -e '/^Cc: /d'  $1
  echo $SEP  $1
  
  echo $CMT_MSG  $1
--
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] Support for setitimer() on platforms lacking it

2012-08-30 Thread Junio C Hamano
Joachim Schmitz j...@schmitz-digital.de writes:

 I see no existing code calls setitimer() with non-NULL ovalue, and I
 do not think we would add a new caller that would do so in any time
 soon, so it may not be a bad idea to drop support of returning the
 remaining timer altogether from this emulation layer (just like
 giving anything other than ITIMER_REAL gives us ENOTSUP).  That
 would sidestep the whole we cannot answer how many milliseconds are
 still remaining on the timer when using emulation based on alarm().

 Should we leave tv_usec untouched then? That was we round up on
 the next (and subsequent?) round(s). Or just set to ENOTSUP in
 setitimer if ovalue is !NULL?

I was alluding to the latter.

  +  switch (which) {
  +  case ITIMER_REAL:
  +  alarm(value-it_value.tv_sec +
  +  (value-it_value.tv_usec  0) ? 1 : 0);
 
 Why is this capped to 1 second?  Is this because no existing code
 uses the timer for anything other than 1 second or shorter?  If that
 is the case, that needs at least some documenting (or a possibly
 support for longer expiration, if it is not too cumbersome to add).

 As you mention alarm() has only seconds resolution. It is tv_sec
 plus 1 if there are tv_usecs  0, it is rounding up, so we don't
 cancel the alarm() if tv_sec is 0 but tv_usec is not. Looks OK to
 me?

Can a caller use setitimer to be notified in 5 seconds?
--
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


[PATCHv2 0/3] Improve branch UI for setting upstream information

2012-08-30 Thread Carlos Martín Nieto
Hi all,

As a result of making --unset-upstream fail if the given branch
doesn't exist, I discovered a copy-paste error in on the the tests in
the patch after it, so I'm resending the whole thing.

The changes from the last reroll are the tightening of the situations
where git will show an error message (not it's just if the branch is
new and exists as remote-tracking) which I already sent as a reply in
the other thread; and making --unset-upstream error out on bad input,
which I already mentioned above.

   cmn

Carlos Martín Nieto (3):
  branch: introduce --set-upstream-to
  branch: add --unset-upstream option
  branch: deprecate --set-upstream and show help if we detect possible
mistaken use

 Documentation/git-branch.txt | 14 -
 builtin/branch.c | 60 +--
 t/t3200-branch.sh| 67 
 3 files changed, 137 insertions(+), 4 deletions(-)

-- 
1.7.12.3.g0dd8ef6

--
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: deprecate --set-upstream and show help if we detect possible mistaken use

2012-08-30 Thread Carlos Martín Nieto
This interface is error prone, and a better one (--set-upstream-to)
exists. Add a message listing the alternatives and suggest how to fix
a --set-upstream invocation in case the user only gives one argument
which causes a local branch with the same name as a remote-tracking
one to be created. The typical case is

git branch --set-upstream origin/master

when the user meant

git branch --set-upstream master origin/master

assuming that the current branch is master. Show a message telling the
user how to undo their action and get what they wanted. For the
command above, the message would be

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

If you wanted to make 'master' track 'origin/master', do this:

git branch -d origin/master
git branch --set-upstream-to origin/master

Signed-off-by: Carlos Martín Nieto c...@elego.de
---
 builtin/branch.c  | 26 ++
 t/t3200-branch.sh | 34 ++
 2 files changed, 60 insertions(+)

diff --git a/builtin/branch.c b/builtin/branch.c
index 557995d..5e95e35 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -881,10 +881,36 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
git_config_set_multivar(buf.buf, NULL, NULL, 1);
strbuf_release(buf);
} else if (argc  0  argc = 2) {
+   struct branch *branch = branch_get(argv[0]);
+   int branch_existed = 0, remote_tracking = 0;
+   struct strbuf buf = STRBUF_INIT;
+
if (kinds != REF_LOCAL_BRANCH)
die(_(-a and -r options to 'git branch' do not make 
sense with a branch name));
+
+   if (track == BRANCH_TRACK_OVERRIDE)
+   fprintf(stderr, _(The --set-upstream flag is 
deprecated and will be removed. Consider using --track or 
--set-upstream-to\n));
+
+   strbuf_addf(buf, refs/remotes/%s, branch-name);
+   remote_tracking = ref_exists(buf.buf);
+   strbuf_release(buf);
+
+   branch_existed = ref_exists(branch-refname);
create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
  force_create, reflog, 0, quiet, track);
+
+   /*
+* We only show the instructions if the user gave us
+* one branch which doesn't exist locally, but is the
+* name of a remote-tracking branch.
+*/
+   if (argc == 1  track == BRANCH_TRACK_OVERRIDE 
+   !branch_existed  remote_tracking) {
+   fprintf(stderr, _(\nIf you wanted to make '%s' track 
'%s', do this:\n\n), head, branch-name);
+   fprintf(stderr, _(git branch -d %s\n), 
branch-name);
+   fprintf(stderr, _(git branch --set-upstream-to 
%s\n), branch-name);
+   }
+
} else
usage_with_options(builtin_branch_usage, options);
 
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 1018e8b..f2a076c 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -402,6 +402,40 @@ test_expect_success 'test --unset-upstream on a particular 
branch' \
  test_must_fail git config branch.my14.remote 
  test_must_fail git config branch.my14.merge'
 
+test_expect_success '--set-upstream shows message when creating a new branch 
that exists as remote-tracking' \
+'git update-ref refs/remotes/origin/master HEAD 
+ git branch --set-upstream origin/master 2actual 
+ test_when_finished git update-ref -d refs/remotes/origin/master 
+ test_when_finished git branch -d origin/master 
+ cat expected EOF 
+The --set-upstream flag is deprecated and will be removed. Consider using 
--track or --set-upstream-to
+
+If you wanted to make ''master'' track ''origin/master'', do this:
+
+git branch -d origin/master
+git branch --set-upstream-to origin/master
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success '--set-upstream with two args only shows the deprecation 
message' \
+'git branch --set-upstream master my13 2actual 
+ test_when_finished git branch --unset-upstream master 
+ cat expected EOF 
+The --set-upstream flag is deprecated and will be removed. Consider using 
--track or --set-upstream-to
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success '--set-upstream with one arg only shows the deprecation 
message if the branch existed' \
+'git branch --set-upstream my13 2actual 
+ test_when_finished git branch --unset-upstream my13 
+ cat expected EOF 
+The --set-upstream flag is deprecated and will be removed. Consider using 
--track or --set-upstream-to
+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 

[PATCH 2/3] branch: add --unset-upstream option

2012-08-30 Thread Carlos Martín Nieto
We have ways of setting the upstream information, but if we want to
unset it, we need to resort to modifying the configuration manually.

Teach branch an --unset-upstream option that unsets this information.

Signed-off-by: Carlos Martín Nieto c...@elego.de
---
 Documentation/git-branch.txt |  5 +
 builtin/branch.c | 21 ++---
 t/t3200-branch.sh| 19 +++
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index e41c4b5..9c1d2f1 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -14,6 +14,7 @@ SYNOPSIS
[(--merged | --no-merged | --contains) [commit]] [pattern...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] branchname 
[start-point]
 'git branch' (--set-upstream-to=upstream | -u upstream) [branchname]
+'git branch' --unset-upstream [branchname]
 'git branch' (-m | -M) [oldbranch] newbranch
 'git branch' (-d | -D) [-r] branchname...
 'git branch' --edit-description [branchname]
@@ -180,6 +181,10 @@ start-point is either a local or remote-tracking branch.
considered branchname's upstream branch. If no branchname
is specified, then it defaults to the current branch.
 
+--unset-upstream::
+   Remove the upstream information for branchname. If no branch
+   is specified it defaults to the current branch.
+
 --edit-description::
Open an editor and edit the text to explain what the branch is
for, to be used by various other commands (e.g. `request-pull`).
diff --git a/builtin/branch.c b/builtin/branch.c
index 3c978eb..557995d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -712,7 +712,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
int delete = 0, rename = 0, force_create = 0, list = 0;
int verbose = 0, abbrev = -1, detached = 0;
int reflog = 0, edit_description = 0;
-   int quiet = 0;
+   int quiet = 0, unset_upstream = 0;
const char *new_upstream = NULL;
enum branch_track track;
int kinds = REF_LOCAL_BRANCH;
@@ -728,6 +728,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
OPT_SET_INT( 0, set-upstream,  track, change upstream info,
BRANCH_TRACK_OVERRIDE),
OPT_STRING('u', set-upstream-to, new_upstream, upstream, 
change the upstream info),
+   OPT_BOOLEAN(0, unset-upstream, unset_upstream, Unset the 
upstream info),
OPT__COLOR(branch_use_color, use colored output),
OPT_SET_INT('r', remotes, kinds, act on remote-tracking 
branches,
REF_REMOTE_BRANCH),
@@ -796,10 +797,10 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 0);
 
-   if (!delete  !rename  !edit_description  !new_upstream  argc == 
0)
+   if (!delete  !rename  !edit_description  !new_upstream  
!unset_upstream  argc == 0)
list = 1;
 
-   if (!!delete + !!rename + !!force_create + !!list + !!new_upstream  1)
+   if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + 
!!unset_upstream  1)
usage_with_options(builtin_branch_usage, options);
 
if (abbrev == -1)
@@ -865,6 +866,20 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
 * info and making sure new_upstream is correct
 */
create_branch(head, branch-name, new_upstream, 0, 0, 0, quiet, 
BRANCH_TRACK_OVERRIDE);
+   } else if (unset_upstream) {
+   struct branch *branch = branch_get(argv[0]);
+   struct strbuf buf = STRBUF_INIT;
+
+   if (!branch_has_merge_config(branch)) {
+   die(_(Branch '%s' has no upstream information), 
branch-name);
+   }
+
+   strbuf_addf(buf, branch.%s.remote, branch-name);
+   git_config_set_multivar(buf.buf, NULL, NULL, 1);
+   strbuf_reset(buf);
+   strbuf_addf(buf, branch.%s.merge, branch-name);
+   git_config_set_multivar(buf.buf, NULL, NULL, 1);
+   strbuf_release(buf);
} else if (argc  0  argc = 2) {
if (kinds != REF_LOCAL_BRANCH)
die(_(-a and -r options to 'git branch' do not make 
sense with a branch name));
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index e9019ac..1018e8b 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -383,6 +383,25 @@ test_expect_success 'use --set-upstream-to modify a 
particular branch' \
  test $(git config branch.my13.remote) = . 
  test $(git config branch.my13.merge) = refs/heads/master'
 
+test_expect_success '--unset-upstream should fail if given a non-existent 
branch' \
+'test_must_fail git branch --unset-upstream 

Re: [PATCH 1/3] remote-testsvn.c: Avoid the getline() GNU extension function

2012-08-30 Thread Ramsay Jones
Erik Faye-Lund wrote:
 On Sat, Aug 25, 2012 at 7:13 PM, Ramsay Jones
 ram...@ramsay1.demon.co.uk wrote:

 The getline() function is a GNU extension (you need to define
 _GNU_SOURCE before including stdio.h) and is, therefore, not
 portable. In particular, getline() is not available on MinGW.
 
 Actually, getline is a POSIX-2008 feature, so it's not (simply) a GNU 
 extension:
 
 http://pubs.opengroup.org/onlinepubs/9699919799/functions/getline.html

Thanks for the reference. I hadn't noticed this being included into
POSIX. Well, it's only been four years ... :-D

Thanks again.

ATB,
Ramsay Jones


--
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/3] remote-testsvn.c: Avoid the getline() GNU extension function

2012-08-30 Thread Ramsay Jones
Junio C Hamano wrote:
 Erik Faye-Lund kusmab...@gmail.com writes:
 
 On Sat, Aug 25, 2012 at 7:13 PM, Ramsay Jones
 ram...@ramsay1.demon.co.uk wrote:

 The getline() function is a GNU extension (you need to define
 _GNU_SOURCE before including stdio.h) and is, therefore, not
 portable. In particular, getline() is not available on MinGW.

 Actually, getline is a POSIX-2008 feature, so it's not (simply) a GNU 
 extension:

 http://pubs.opengroup.org/onlinepubs/9699919799/functions/getline.html
 
 True, thanks for pointing it out.  Justify the change with something
 like this instead, perhaps?
 
   The getline() function, even though is in POSIX.1-2008, is
   not available on some platforms.  Use strbuf_getline() for
   portability.
 
 By the way, the remainder of the proposed log message talks about
 the difference between getline() that keeps the terminating LF vs
 strbuf_getline() that drops it.  Would strbuf_getwholeline() be a
 better alternative?

I don't think it matters; I was just making sure Florian was aware
of the difference (it will not appear in the commit message anyway),
including the memory leak in the original code.

ATB,
Ramsay Jones


--
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] vcs-svn: Fix 'fa/remote-svn' and 'fa/vcs-svn' in pu

2012-08-30 Thread Ramsay Jones
Florian Achleitner wrote:
 Hi!
 
 Thanks for your fixups. I'm currently integrating them in a new series.
 On what platform did you find that problems? 
 Tried to reproduce them on 64bit Linux. Anyways the fixes look very 
 reasonable.

I found the problem on Linux, cygwin and MinGW. That would be *32-bit* Linux,
cygwin and MinGW, of course. ;-)

ATB,
Ramsay Jones


--
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/3] branch: introduce --set-upstream-to

2012-08-30 Thread Ralf Thielow
On Thu, Aug 30, 2012 at 7:23 PM, Carlos Martín Nieto c...@elego.de wrote:
 behaviour. To work around this, introduce --set-upstream-to which
 accepts a compulsory argument indicating what the new upstream branch
 should be and one optinal argument indicating which branch to change,
 defaulting to HEAD.


Could you please also add this new option to the
contrib/completion/git-completion.bash
script?

Thanks!
--
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: [PATCHv2 0/3] Improve branch UI for setting upstream information

2012-08-30 Thread Carlos Martín Nieto
Junio C Hamano gits...@pobox.com writes:

 Carlos Martín Nieto c...@elego.de writes:

 As a result of making --unset-upstream fail if the given branch
 doesn't exist, I discovered a copy-paste error in on the the tests in
 the patch after it, so I'm resending the whole thing.

 The changes from the last reroll are the tightening of the situations
 where git will show an error message (not it's just if the branch is
 new and exists as remote-tracking) which I already sent as a reply in
 the other thread; and making --unset-upstream error out on bad input,
 which I already mentioned above.

 Thanks.

 In addition to --unset-upstream must fail on i-dont-exist branch
 in [2/3], I am wondering if we would want to also make sure the
 command fails when the upstream information is not set for the
 branch, i.e. something like the following on top.

 What do you think?

  t/t3200-branch.sh | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git i/t/t3200-branch.sh w/t/t3200-branch.sh
 index 1018e8b..a0aaedd 100755
 --- i/t/t3200-branch.sh
 +++ w/t/t3200-branch.sh
 @@ -393,7 +393,9 @@ test_expect_success 'test --unset-upstream on HEAD' \
   git branch --set-upstream-to my14 
   git branch --unset-upstream 
   test_must_fail git config branch.master.remote 
 - test_must_fail git config branch.master.merge'
 + test_must_fail git config branch.master.merge 
 + test_must_fail git branch --unset-upstream
 +'

Yeah, this looks good, makes sure that it will still behave correctly
even if the code path for these two situations diverges.

   cmn
--
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 5/4] wincred: port to generic credential helper (UNTESTED)

2012-08-30 Thread Junio C Hamano
Erik Faye-Lund kusmab...@gmail.com writes:

 On Mon, Aug 27, 2012 at 12:04 AM, Philipp A. Hartmann p...@qo.cx wrote:
 From: Philipp A. Hartmann p...@qo.cx

 This patch is an experiment to port the wincred helper
 to the generic implementation.  As of know, it is
 completely untested.

 In addition to porting the helper to the generic API,
 this patch clears up all passwords from memory, which
 reduces the total amount to saved lines.

 Signed-off-by: Philipp A. Hartmann p...@qo.cx
 ---

 The porting was fairly easy, but due to the lack of a testing
 platform, it is completely untested.

 Erik: Can you try to have look?

 Sorry for the late reply, I'm currently in bed with pneumonia.

 But I gave it a quick go, but as-is it's a NACK; a wall of warnings and 
 errors.

Thanks, and get well soon.

For now, let's not worry about merging the let's share code across
helpers bit that comes later in Philipp's series.
--
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: [PATCHv2 0/3] Improve branch UI for setting upstream information

2012-08-30 Thread Junio C Hamano
car...@cmartin.tk (Carlos Martín Nieto) writes:

 Junio C Hamano gits...@pobox.com writes:

 Carlos Martín Nieto c...@elego.de writes:

 As a result of making --unset-upstream fail if the given branch
 doesn't exist, I discovered a copy-paste error in on the the tests in
 the patch after it, so I'm resending the whole thing.

 The changes from the last reroll are the tightening of the situations
 where git will show an error message (not it's just if the branch is
 new and exists as remote-tracking) which I already sent as a reply in
 the other thread; and making --unset-upstream error out on bad input,
 which I already mentioned above.

 Thanks.

 In addition to --unset-upstream must fail on i-dont-exist branch
 in [2/3], I am wondering if we would want to also make sure the
 command fails when the upstream information is not set for the
 branch, i.e. something like the following on top.

 What do you think?

  t/t3200-branch.sh | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git i/t/t3200-branch.sh w/t/t3200-branch.sh
 index 1018e8b..a0aaedd 100755
 --- i/t/t3200-branch.sh
 +++ w/t/t3200-branch.sh
 @@ -393,7 +393,9 @@ test_expect_success 'test --unset-upstream on HEAD' \
   git branch --set-upstream-to my14 
   git branch --unset-upstream 
   test_must_fail git config branch.master.remote 
 - test_must_fail git config branch.master.merge'
 + test_must_fail git config branch.master.merge 
 + test_must_fail git branch --unset-upstream
 +'

 Yeah, this looks good, makes sure that it will still behave correctly
 even if the code path for these two situations diverges.

Alright; will squash.

Thanks.
--
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 2/3] demonstrate broken 'git cherry-pick three one two'

2012-08-30 Thread Junio C Hamano
Martin von Zweigbergk martinv...@gmail.com writes:

 Cherry-picking commits out of order (w.r.t. commit time stamp) doesn't
 currently work. Add a test case to demonstrate it.

 Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
 ---
  t/t3508-cherry-pick-many-commits.sh | 15 +++
  1 file changed, 15 insertions(+)

 diff --git a/t/t3508-cherry-pick-many-commits.sh 
 b/t/t3508-cherry-pick-many-commits.sh
 index 75f7ff4..fff20c3 100755
 --- a/t/t3508-cherry-pick-many-commits.sh
 +++ b/t/t3508-cherry-pick-many-commits.sh
 @@ -44,6 +44,21 @@ test_expect_success 'cherry-pick first..fourth works' '
   check_head_differs_from fourth
  '
  
 +test_expect_failure 'cherry-pick three one two works' '
 + git checkout -f first 
 + test_commit one 
 + test_commit two 
 + test_commit three 
 + git checkout -f master 
 + git reset --hard first 
 + git cherry-pick three one two 
 + git diff --quiet three 
 + git diff --quiet HEAD three 
 + test $(git log --reverse --format=%s first..) == three
 +one
 +two
 +'

test $A == $B is not POSIX.  I'll drop '=' when queuing, so no
need to resend.

Thanks.

 +
  test_expect_success 'output to keep user entertained during multi-pick' '
   cat -\EOF expected 
   [master OBJID] second
--
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/2] commit: use a priority queue in merge base functions

2012-08-30 Thread Jeff King
On Thu, Aug 30, 2012 at 09:23:25AM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  Anyway, since this isn't yielding any performance benefit, I'm not going
  to go down that route. But stability of the queue is something that we
  need to consider if we ever do replace commit_list with a different data
  structure.
 
  Here's the patch to make the existing priority queue stable (by wasting
  space) in case we ever want to use it.
 
 Thanks for being thorough and showing a good analysis.
 
 If we want stability, the space to hold insertion counter is not
 wasted, by the way.

It is if there is another data structure that will do the same job with
the same performance characteristics and without using that space.

But I'm not even sure it is an issue in practice. There are ~320K
objects in linux-2.6. Even if we inserted _all_ of them at once into a
queue (which we don't; it's usually more like a couple dozen, with a few
pathological cases being equal to the number of refs we have, which is
usually in the hundreds), then we are talking about an extra 2.5M on a
64-bit system. Compared to dropping an O(n^2) operation to O(lg n),
that's probably not even going to be noticeable.

 I actually think the generic queue implementation may want to
 handle elements that are not just single pointers.  Just like
 qsort() is not about sorting an array of pointers but it is about
 sorting an array of elements of caller specified size, perhaps
 queue can hold elements of size the caller specified (set in stone
 when the queue is initialized).
 
 Then, a caller that wants a stable priority queue of commits can
 tell the queue to manage struct { struct commit *c; int gen; },
 use the gen field for stability in its comparison callback, etc.,
 while a caller that does not need stability can tell the queue to
 manage struct commit *.  That way, the generic queue layer does
 not have to worry about wasting the insertion counter space, no?

Yeah, that would be more generic. One wonky thing I didn't point out in
my implementation is this:

 +static inline int queue_cmp(struct queue *pq, unsigned i, unsigned j)
 +{
 +   int cmp = pq-cmp(pq-items[i].item, pq-items[j].item);
 +   if (cmp)
 +   return cmp;
 +   if (pq-items[i].counter  pq-items[j].counter)
 +   return 1;
 +   return -1;
 +}

Notice how the counter comparison is backwards to the regular
comparison. That's because the queue is actually a max-queue (I did it
that way since we are indexing on timestamp, and want to go in reverse
chronological order). But the counter part wants to prioritize minimums,
so you have to reverse it. You could also implement a min-queue and ask
the caller to reverse their comparison function.

But really, a caller could theoretically want to prioritize in either
direction (e.g., giving a LIFO behavior to elements with the same
priority). Pulling the counter out of the queue would allow that.

The reason I didn't, though, is that it would make the interface a huge
pain if the caller had to do this:

  struct stable_commit_queue_element qe;
  qe.commit = commit;
  qe.counter = counter++; /* who is holding the global counter? */
  queue_push(q, qe);

instead of:

  queue_push(q, commit);

I suspect you could build a queue object, and then implement a
stable queue on top of it with some clever use of function pointers
for the comparison function.

-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


Re: git blame shows wrong Not commited yet entries

2012-08-30 Thread Junio C Hamano
Thomas Ackermann th.acke...@arcor.de writes:

 I am using MsysGit 1.7.11 on WinXP 32 bit and experience the folllowing 
 strange behaviour:
  
 For a file like File.txt in the repo, git blame file.txt (note the lower 
 case)
 shows Not commited yet for every single line in the file. 
 git blame File.txt (correct upper case spelling) gives the correct output.
 core.ignorecase is true so this behaviour is not what I expected.

What happens when you do this?

$ echo garbage no-such-file-tracked.txt
$ git blame no-such-file-tracked.txt

If you see everything attributed to the working tree file, I _think_
you are seeing exactly the same thing.  As far as your repository
history is concerned, file.txt is never tracked (File.txt is),
and when Git asks the contents for file.txt in the working tree,
Windows case insensitive filesystem gives it the contents of
File.txt instead, all lines in which is initially attributed to
the working tree.  When blame tries to pass the blame around to the
commit at HEAD and down the history, it never finds file.txt, so
all the blame is given to this phamtom contents from File.txt
Windows gave you as if it were stored in file.txt, which does not
really exist.

An obvious workaround is to say git blame File.txt.  You might be
able to patch MsysGit so that git blame file.txt errors out when
it does not find such a path in the HEAD commit, though.
--
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/2] commit: use a priority queue in merge base functions

2012-08-30 Thread Jeff King
On Thu, Aug 30, 2012 at 09:33:48AM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  The script originally comes from here:
 
http://thread.gmane.org/gmane.comp.version-control.git/33566/focus=33852
 
  and the discussion implies that the AUTHOR_DATEs were added to avoid a
  race condition with the timestamps. But why would that ever have worked?
 
 I do not see how AUTHOR_DATE would affect anything there, either,
 other than to give reprodusible object names.  The test only sets
 committer-date upfront, so without setting author-date, you would
 still get different object names on commits.  Which suggests me that
 there may be something that tiebreaks based on object names?

Hmm. I wouldn't think so. The order should come from timestamps, with
ties broken by order of insertion, which in turn comes from traversal,
which depends only on parent order. We do check some raw sha1s in the
expected output, but it is only for blobs, not commits.  I guess it
could be some weirdness inside merge-recursive, though, and
not part of the merge-base computation.

So I really don't know how AUTHOR_DATE would change anything. And
indeed, after removing them it still passes on my machine.

However, I think I may understand why it fails if you tweak the
committer dates.

When my unstable-queue was used, I noticed that the merge bases returned
were the same (as you would expect), but that they came in a different
order. Which makes sense, as the order of multiple bases would not
necessarily be deterministic (they do not have an ancestry relationship,
or they would not be merge bases).

So the issue is that when you do a recursive merge with multiple bases,
the order in which you visit the recursive bases is going to impact the
exact conflicts you see. In theory, after the merge is done, you're
going to be at the same state, but the conflicts you see along the way
will be different. And it is this conflicted state that the test is
looking at.

The current test expects a particular order of merge bases based on the
same-second commit timestamps. There is no race condition because of the
setting of GIT_COMMITTER_DATE at the beginning (and _that_ is the real
thing that fixed the race conditions Dscho saw in the thread above; the
AUTHOR_DATE was just a red herring).

So the test is not broken or racy, which is good. It is just testing
something that is somewhat of an implementation detail. We could switch
it to use test_tick, and then adjust the expected output to look for the
expected conflict that git happens to generate in that case. But that is
no better than the current behavior.

But I'm not sure there is a way to test what it wants to test (that we
hit a conflict that involves one of the recursive merge bases) without
relying on the implementation detail. So I'm inclined to just leave it
in place.

-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


Re: [PATCH 2/2] commit: use a priority queue in merge base functions

2012-08-30 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Compared to dropping an O(n^2) operation to O(lg n), that's
 probably not even going to be noticeable.

Yeah, that is something we would want to use when we eventually
update the main revision traverser, not this codepath localized to
the merge-base.

Thanks.  I like (and agree with) everything you wrote in this
message.
--
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/2] commit: use a priority queue in merge base functions

2012-08-30 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 So the issue is that when you do a recursive merge with multiple bases,
 the order in which you visit the recursive bases is going to impact the
 exact conflicts you see.

Yeah, that explains it.

 So the test is not broken or racy, which is good. It is just testing
 something that is somewhat of an implementation detail. We could switch
 it to use test_tick, and then adjust the expected output to look for the
 expected conflict that git happens to generate in that case. But that is
 no better than the current behavior.

True.

 But I'm not sure there is a way to test what it wants to test (that we
 hit a conflict that involves one of the recursive merge bases) without
 relying on the implementation detail. So I'm inclined to just leave it
 in place.

OK.
--
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 archive --format zip utf-8 issues

2012-08-30 Thread Jeff King
On Sat, Aug 11, 2012 at 11:37:05PM +0200, Sven Strickroth wrote:

 Am 11.08.2012 22:53 schrieb René Scharfe:
  The standard says we need to convert to CP437, or to UTF-8, or provide 
  both versions. A more interesting question is: What's supported by which 
  programs?
  
  The ZIP functionality built into Windows 7 doesn't seem to work with 
  UTF-8 encoded filenames (except for those that only use the ASCII 
  subset), and to ignore the UTF-8 part if both are given.
 
 I played a bit with the git source code and found out, that
 
 diff --git a/archive-zip.c b/archive-zip.c
 index f5af81f..e0ccb4f 100644
 --- a/archive-zip.c
 +++ b/archive-zip.c
 @@ -257,7 +257,7 @@ static int write_zip_entry(struct archiver_args *args,
   copy_le16(dirent.creator_version,
   S_ISLNK(mode) || (S_ISREG(mode)  (mode  0111)) ? 0x0317 : 0);
   copy_le16(dirent.version, 10);
 - copy_le16(dirent.flags, flags);
 + copy_le16(dirent.flags, flags+2048);
   copy_le16(dirent.compression_method, method);
   copy_le16(dirent.mtime, zip_time);
   copy_le16(dirent.mdate, zip_date);
 --
 works with 7-zip, however, not with Windows 7 build-in zip.
 
 If I create a zip file with 7-zip which contains umlauts and other
 unicode chars like (國立1-.txt) the Windows 7 build-in zip displays
 them correctly, too.

Ping on this stalled discussion.

It seems like there are two separate issues here:

  1. Knowing the encoding of pathnames in the repository.

  2. Setting the right flags in zip output.

A full solution would handle both parts, but let's ignore (1) for a
moment, and assume we have utf-8 (or can massage into utf-8 from an
encoding specified by the user).

It seems like just setting the magic utf-8 flag would be the only thing
we need to do, according to the standard. But according to discussions
referenced elsewhere in this thread, that flag was invented only in
2007, so we may be dealing with older implementations (I have no idea
how common they would be; that may be the problem with Windows 7's zip
you are seeing). We could re-encode to cp437, which the standard
specifies, but apparently some implementations do not respect that
(and use a local code page instead). And it cannot represent all utf-8
characters, anyway.

It sounds like 7-zip has figured out a more portable solution. Can you
show us a sample of 7-zip's output with utf-8 characters to compare to
what git generates? I wonder if it is using a combination of methods.

-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] Make git-svn branch patterns match complete URL

2012-08-30 Thread Ammon Riley
When using the {word,[...]} style of configuration for tags and branches,
it appears the intent is to only match whole path parts, since the words
in the {} pattern are meta-character quoted.

When the pattern word appears in the beginning or middle of the url,
it's matched completely, since the left side, pattern, and (non-empty)
right side are joined together with path separators.

However, when the pattern word appears at the end of the URL, the
right side is an empty pattern, and the resulting regex matches
more than just the specified pattern.

For example, if you specify something along the lines of

branches = branches/project/{release_1,release_2}

and your repository also contains branches/project/release_1_2, you
will also get the release_1_2 branch.  By restricting the match regex
with anchors, this is avoided.

Signed-off-by: Ammon Riley ammon.ri...@gmail.com
---
Tested with Subversion 1.6; applies against maint, and master.

 perl/Git/SVN/GlobSpec.pm  | 4 +++-
 t/t9154-git-svn-fancy-glob.sh | 9 +
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/perl/Git/SVN/GlobSpec.pm b/perl/Git/SVN/GlobSpec.pm
index 96cfd98..c95f5d7 100644
--- a/perl/Git/SVN/GlobSpec.pm
+++ b/perl/Git/SVN/GlobSpec.pm
@@ -44,7 +44,9 @@ sub new {
my $right = join('/', @right);
$re = join('/', @patterns);
$re = join('\/',
-  grep(length, quotemeta($left), ($re), quotemeta($right)));
+  grep(length, quotemeta($left),
+($re)(?=/|\$),
+quotemeta($right)));
my $left_re = qr/^\/\Q$left\E(\/|$)/;
bless { left = $left, right = $right, left_regex = $left_re,
regex = qr/$re/, glob = $glob, depth = $depth }, $class;
diff --git a/t/t9154-git-svn-fancy-glob.sh b/t/t9154-git-svn-fancy-glob.sh
index a6a56a6..b780e0e 100755
--- a/t/t9154-git-svn-fancy-glob.sh
+++ b/t/t9154-git-svn-fancy-glob.sh
@@ -21,6 +21,15 @@ test_expect_success 'add red branch' 
test_must_fail git rev-parse refs/remotes/blue

 
+test_expect_success 'add gre branch' 
+   GIT_CONFIG=.git/svn/.metadata git config --unset 
svn-remote.svn.branches-maxRev 
+   git config svn-remote.svn.branches 'branches/{red,gre}:refs/remotes/*' 

+   git svn fetch 
+   git rev-parse refs/remotes/red 
+   test_must_fail git rev-parse refs/remotes/green 
+   test_must_fail git rev-parse refs/remotes/blue
+   
+
 test_expect_success 'add green branch' 
GIT_CONFIG=.git/svn/.metadata git config --unset 
svn-remote.svn.branches-maxRev 
git config svn-remote.svn.branches 
'branches/{red,green}:refs/remotes/*' 
-- 
1.7.11.3

--
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 0/4] merge-base updates

2012-08-30 Thread Junio C Hamano
This replaces the topmost commit from the earlier series that tried
to optimize N*(N-1) loop with N traversals.  With this, running the
following script in the kernel repository:

-- 8 --
#!/bin/sh

git rev-list --committer=torva...@linux-foundation.org \
--max-parents=2 --min-parents=2 --parents v3.5..v3.6-rc2 RL

cmd='
while read result parent1 parent2
do
$GIT merge-base $parent1 $parent2
done RL
'

GIT=rungit master time sh -c $cmd :stock
GIT=../git.git/git time sh -c $cmd :optim
cmp :stock :optim
-- 8 --

shows a slight but measurable boost in the performance of
merge-base itself.

42.45user 3.79system 0:46.62elapsed 99%CPU (0avgtext+0avgdata 
408176maxresident)k
0inputs+24outputs (0major+2708085minor)pagefaults 0swaps
39.99user 3.42system 0:43.27elapsed 100%CPU (0avgtext+0avgdata 
408192maxresident)k
0inputs+24outputs (0major+2179604minor)pagefaults 0swaps

Junio C Hamano (4):
  merge_bases_many(): split out the logic to paint history
  merge-base: --is-ancestor A B
  in_merge_bases(): use paint_down_to_common()
  get_merge_bases_many(): walk from many tips in parallel

 builtin/merge-base.c |  21 
 commit.c | 143 +--
 2 files changed, 114 insertions(+), 50 deletions(-)

-- 
1.7.12.293.g6aeebca

--
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/4] merge_bases_many(): split out the logic to paint history

2012-08-30 Thread Junio C Hamano
Introduce a new helper function paint_down_to_common() that takes
the same parameters as merge_bases_many(), but without the first
optimization of not painting anything when one is one of the
twos (or vice versa), and the last clean-up of removing the common
ancestor that is known to be an ancestor of another common one.

This way, the caller of the new function could tell if one is
reachable from any of the twos by simply looking at the flag bits
of one.  If (and only if) it is painted in PARENT2, it is
reachable from one of the twos.
---
 commit.c | 47 ---
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/commit.c b/commit.c
index 12e5396..0058fa5 100644
--- a/commit.c
+++ b/commit.c
@@ -581,28 +581,12 @@ static struct commit *interesting(struct commit_list 
*list)
return NULL;
 }
 
-static struct commit_list *merge_bases_many(struct commit *one, int n, struct 
commit **twos)
+static struct commit_list *paint_down_to_common(struct commit *one, int n, 
struct commit **twos)
 {
struct commit_list *list = NULL;
struct commit_list *result = NULL;
int i;
 
-   for (i = 0; i  n; i++) {
-   if (one == twos[i])
-   /*
-* We do not mark this even with RESULT so we do not
-* have to clean it up.
-*/
-   return commit_list_insert(one, result);
-   }
-
-   if (parse_commit(one))
-   return NULL;
-   for (i = 0; i  n; i++) {
-   if (parse_commit(twos[i]))
-   return NULL;
-   }
-
one-object.flags |= PARENT1;
commit_list_insert_by_date(one, list);
for (i = 0; i  n; i++) {
@@ -643,9 +627,34 @@ static struct commit_list *merge_bases_many(struct commit 
*one, int n, struct co
}
}
 
-   /* Clean up the result to remove stale ones */
free_commit_list(list);
-   list = result; result = NULL;
+   return result;
+}
+
+static struct commit_list *merge_bases_many(struct commit *one, int n, struct 
commit **twos)
+{
+   struct commit_list *list = NULL;
+   struct commit_list *result = NULL;
+   int i;
+
+   for (i = 0; i  n; i++) {
+   if (one == twos[i])
+   /*
+* We do not mark this even with RESULT so we do not
+* have to clean it up.
+*/
+   return commit_list_insert(one, result);
+   }
+
+   if (parse_commit(one))
+   return NULL;
+   for (i = 0; i  n; i++) {
+   if (parse_commit(twos[i]))
+   return NULL;
+   }
+
+   list = paint_down_to_common(one, n, twos);
+
while (list) {
struct commit_list *next = list-next;
if (!(list-item-object.flags  STALE))
-- 
1.7.12.293.g6aeebca

--
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/4] merge-base: --is-ancestor A B

2012-08-30 Thread Junio C Hamano
In many scripted Porcelain commands, we find this idiom:

if test $(git rev-parse --verify A) = $(git merge-base A B)
then
... A is an ancestor of B ...
fi

But you do not have to compute exact merge-base only to see if A is
an ancestor of B.  Give them a more direct way to use the underlying
machinery.
---
 builtin/merge-base.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index 4f30f1b..615aa04 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -70,6 +70,20 @@ static int handle_octopus(int count, const char **args, int 
reduce, int show_all
return 0;
 }
 
+static int show_is_ancestor(int argc, const char **argv)
+{
+   struct commit *one, *two;
+
+   if (argc != 2)
+   die(--is-ancestor takes exactly two commits);
+   one = get_commit_reference(argv[0]);
+   two = get_commit_reference(argv[1]);
+   if (in_merge_bases(one, two))
+   return 0;
+   else
+   return 1;
+}
+
 int cmd_merge_base(int argc, const char **argv, const char *prefix)
 {
struct commit **rev;
@@ -77,11 +91,14 @@ int cmd_merge_base(int argc, const char **argv, const char 
*prefix)
int show_all = 0;
int octopus = 0;
int reduce = 0;
+   int is_ancestor = 0;
 
struct option options[] = {
OPT_BOOLEAN('a', all, show_all, output all common 
ancestors),
OPT_BOOLEAN(0, octopus, octopus, find ancestors for a 
single n-way merge),
OPT_BOOLEAN(0, independent, reduce, list revs not reachable 
from others),
+   OPT_BOOLEAN(0, is-ancestor, is_ancestor,
+   is the first one ancestor of the other?),
OPT_END()
};
 
@@ -89,6 +106,10 @@ int cmd_merge_base(int argc, const char **argv, const char 
*prefix)
argc = parse_options(argc, argv, prefix, options, merge_base_usage, 0);
if (!octopus  !reduce  argc  2)
usage_with_options(merge_base_usage, options);
+   if (is_ancestor  (show_all | octopus | reduce))
+   die(--is-reachable cannot be used with other options);
+   if (is_ancestor)
+   return show_is_ancestor(argc, argv);
if (reduce  (show_all || octopus))
die(--independent cannot be used with other options);
 
-- 
1.7.12.293.g6aeebca

--
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 tip/core/rcu 0/5] Documentation and rcutorture changes

2012-08-30 Thread Josh Triplett
On Thu, Aug 30, 2012 at 02:46:03PM -0700, Paul E. McKenney wrote:
 On Thu, Aug 30, 2012 at 11:56:09AM -0700, Josh Triplett wrote:
  On Thu, Aug 30, 2012 at 11:44:48AM -0700, Paul E. McKenney wrote:
   Hello!
   
   This series covers changes to rcutorture and documentation updates.
   The individual patches in this series are as follows:
   
   1.Update rcutorture default values so that casual rcutorture
 users will do more aggressive testing.
   2.Make rcutorture track CPU-hotplug latency statistics.
   3.Document SRCU's new-found ability to be used by offline and
 idle CPUs, and also emphasize SRCU's limitations.
   4.Use the new pr_*() interfaces in rcutorture.
   5.Prevent kthread-initialization races in rcutorture.
   
 Thanx, Paul
   
   
   
b/Documentation/RCU/checklist.txt |6 +
b/Documentation/RCU/whatisRCU.txt |9 +-
b/kernel/rcutorture.c |4 -
kernel/rcutorture.c   |  152 
   +++---
4 files changed, 108 insertions(+), 63 deletions(-)
  
  Something seems wrong with this diffstat; how'd the b/ prefixes get
  there, and why does it list kernel/rcutorture.c twice, once with and
  once without?
 
 Hmmm...  It seems quite reproducible.  I did the usual git-format-patch
 and ran the resulting set of patches through diffstat.  I seem to have a
 broken diffstat...
 
 However, git diff --stat v3.6-rc1..hotplug.2012.08.28a generates the
 following:
 
  kernel/rcutree.c   |   93 
 +++-
  kernel/rcutree.h   |3 --
  kernel/rcutree_trace.c |4 +-
  kernel/sched/core.c|   41 ++---
  4 files changed, 43 insertions(+), 98 deletions(-)
 
 Which does look much better.

You might try generating your cover letter template via git format-patch
--cover-letter, which will automatically give you a list of patches and
a git-produced diffstat; much easier than trying to format a cover
letter by hand.  Meanwhile, you might consider sending your patches as a
bug report to diffstat upstream: Thomas E. Dickey
dic...@invisible-island.net.

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