[PATCH] Fix unclosed here document in t3301.sh

2015-01-22 Thread Kacper Kornet
Commit 908a3203632a02568df230c0fccf9a2cd8da24e6 introduced  indentation
to here documents in t3301.sh. However in one place -EOF was missing
-, which broke this test when run with mksh-50d. This commit fixes it.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 t/t3301-notes.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 245406a..433f925 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -658,7 +658,7 @@ test_expect_success '--show-notes=* adds to 
GIT_NOTES_DISPLAY_REF' '
 '
 
 test_expect_success '--no-standard-notes' '
-   cat expect-commits EOF
+   cat expect-commits -EOF
commit 2c125331118caba0ff8238b7f4958ac6e93fe39c
Author: A U Thor aut...@example.com
Date:   Thu Apr 7 15:18:13 2005 -0700
-- 
2.2.2
--
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: [ANNOUNCE] Git v1.9-rc0

2014-01-28 Thread Kacper Kornet
On Mon, Jan 27, 2014 at 10:58:29AM -0800, Jonathan Nieder wrote:
 Hi,

 Kacper Kornet wrote:

  The change in release numbering also breaks down gitolite v2 setups. One
  of the gitolite commands, gl-compile-conf, expects the output of git 
  --version 
  to match /git version (\d+)\.(\d+)\.(\d+)/. 

  I have no idea how big problem it is, as I don't know how many people
  hasn't migrate to gitolite v3 yet. 

 http://qa.debian.org/popcon.php?package=gitolite says there are some.
 I guess soon we'll see if there are complaints.

 http://gitolite.com/gitolite/migr.html says gitolite v2 is still
 maintained.  Hopefully the patch to gitolite v2 to fix this would not
 be too invasive --- e.g., how about this patch (untested)?

 Thanks,
 Jonathan

 diff --git i/src/gl-compile-conf w/src/gl-compile-conf
 index f497ae5..8508313 100755
 --- i/src/gl-compile-conf
 +++ w/src/gl-compile-conf
 @@ -394,8 +394,9 @@ die 
  the server.  If it is not, please edit ~/.gitolite.rc on the server and
  set the \$GIT_PATH variable to the correct value\n
   unless $git_version;
 -my ($gv_maj, $gv_min, $gv_patchrel) = ($git_version =~ m/git version 
 (\d+)\.(\d+)\.(\d+)/);
 +my ($gv_maj, $gv_min, $gv_patchrel) = ($git_version =~ m/git version 
 (\d+)\.(\d+)\.([^.-]*)/);
  die $ABRT I can't understand $git_version\n unless ($gv_maj = 1);
 +$gv_patchrel = 0 unless ($gv_patchrel =~ m/^\d+$/);
  $git_version = $gv_maj*1 + $gv_min*100 + $gv_patchrel;  # now it's 
 normalised

  die \n\t\t* AAARGH! *\n .

It works for 1.9.rc1 but I think it will fail with final 1.9. The
following version should be ok:

diff --git a/src/gl-compile-conf b/src/gl-compile-conf
index f497ae5..c391468 100755
--- a/src/gl-compile-conf
+++ b/src/gl-compile-conf
@@ -394,7 +394,7 @@ die 
 the server.  If it is not, please edit ~/.gitolite.rc on the server and
 set the \$GIT_PATH variable to the correct value\n
  unless $git_version;
-my ($gv_maj, $gv_min, $gv_patchrel) = ($git_version =~ m/git version 
(\d+)\.(\d+)\.(\d+)/);
+my ($gv_maj, $gv_min, undef, $gv_patchrel) = ($git_version =~ m/git version 
(\d+)\.(\d+)(\.(\d+))?/);
 die $ABRT I can't understand $git_version\n unless ($gv_maj = 1);
 $git_version = $gv_maj*1 + $gv_min*100 + $gv_patchrel;  # now it's 
normalised
 
-- 
  Kacper
--
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: [ANNOUNCE] Git v1.9-rc0

2014-01-27 Thread Kacper Kornet
On Tue, Jan 21, 2014 at 02:14:22PM -0800, Junio C Hamano wrote:
 It has been reported that turning git.rc into git.res does not like
 the new 2-dewey-decimal release numbering scheme; packagers of
 various distro might find similar issues in their build procedures,
 in which case they have about 3 weeks to update them until the final
 release.

The change in release numbering also breaks down gitolite v2 setups. One
of the gitolite commands, gl-compile-conf, expects the output of git --version 
to match /git version (\d+)\.(\d+)\.(\d+)/. 

I have no idea how big problem it is, as I don't know how many people
hasn't migrate to gitolite v3 yet. 

-- 
  Kacper
--
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] Fix '\%o' for printf from coreutils

2013-10-31 Thread Kacper Kornet
The printf utility provided by coreutils when interpreting '\%o' format
does not recognize %o as formatting directive. For example
printf '\%o 0 returns \%o and warning: ignoring excess arguments,
starting with ‘0’, which results in failed tests in
t5309-pack-delta-cycles.sh. In most shells the test ends with success as
the printf is a builtin utility.

Fix it by using '\\%o' which is interpreted consistently in all versions
of printf.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---

I've found it while testing v1.8.5-rc0 with mksh which does not
provide a builtin printf.

Kacper

 t/lib-pack.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/lib-pack.sh b/t/lib-pack.sh
index 7e8685b..b96e125 100644
--- a/t/lib-pack.sh
+++ b/t/lib-pack.sh
@@ -12,10 +12,10 @@
 # Print the big-endian 4-byte octal representation of $1
 uint32_octal () {
n=$1
-   printf '\%o' $(($n / 16777216)); n=$((n % 16777216))
-   printf '\%o' $(($n /65536)); n=$((n %65536))
-   printf '\%o' $(($n /  256)); n=$((n %  256))
-   printf '\%o' $(($n   ));
+   printf '\\%o' $(($n / 16777216)); n=$((n % 16777216))
+   printf '\\%o' $(($n /65536)); n=$((n %65536))
+   printf '\\%o' $(($n /  256)); n=$((n %  256))
+   printf '\\%o' $(($n   ));
 }
 
 # Print the big-endian 4-byte binary representation of $1
-- 
1.8.4.2

-- 
  Kacper Kornet
--
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: What's cooking in git.git (Aug 2013, #06; Tue, 27)

2013-08-27 Thread Kacper Kornet
On Tue, Aug 27, 2013 at 12:22:30PM -0700, Junio C Hamano wrote:
 * kk/tests-with-no-perl (2013-08-24) 4 commits
  - reset test: modernize style
  - t/t7106-reset-unborn-branch.sh: Add PERL prerequisite
  - add -i test: use skip_all instead of repeated PERL prerequisite
  - Make test using invalid commit with -C more strict

  Am I waiting for another reroll?

From these four commits only the second and last one are from me and I'm
happy with their form as in pu (I see that you have already introduced
your improvement of  commit --allow-empty to the last one). Unless
there are some remarks to them.

But I don't know what about Jonathan's commits.

As a matter of fact I have no idea how I even should reroll the topic
that includes commits with mixed authorship.

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


[BUG] Shallow fetch can result in broken git repo

2013-08-25 Thread Kacper Kornet
Starting from 6035d6a fetch-pack: prepare updated shallow file before
fetching the pack the shallow fetches of commits with tags can result
in broken git repo.  The following script illustrates the problem:

#!/bin/sh

mkdir repo1 repo2
cd repo1
git init
for i in `seq 1 3`; do
echo $i  foo
git add foo
git commit -m Commit $i
done
git tag tag HEAD~1

cd ../repo2
git init
git fetch --depth=2 ../repo1 master:branch
git fsck

The function fetch_pack in this case is called twice. During the
second called alternate_shallow_file contains \0 (it is set to this
value by commit_lock_file(shallow_lock) during first call of
fetch_pack). In the result the file .git/shallow, created during first
call to fetch_pack, is removed and the git repo ends with broken link.

The two possible fixes which I see are:

1) Replace back if (alternate_shallow_file) condition in fetch pack with 
   if (args-depth  0) 

2) alternate_shallow_file should be copy of shallow_lock.filename not a
   reference to it

But I'm not able to determine by myself which one (if any) is a correct fix
to the problem.

-- 
  Kacper
--
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/3] Make test using invalid commit with -C more strict

2013-08-23 Thread Kacper Kornet
In the test 'using invalid commit with -C' git-commit would have failed
even if the -C option  had been given the correct commit, as there was
nothing to commit. Fix it by making sure there is always something to
commit and git-commit fails because of the invalid commit provided to
it.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 t/t7501-commit.sh | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 99ce36f..699a603 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -53,7 +53,10 @@ test_expect_success PERL 'can use paths with --interactive' '
 '
 
 test_expect_success 'using invalid commit with -C' '
-   test_must_fail git commit -C bogus
+   echo bong file 
+   git add file 
+   test_must_fail git commit -C bogus 
+   git reset
 '
 
 test_expect_success 'nothing to commit' '
-- 
1.8.3.4


From 8ad7ef374e1b05cb73d6cf445c44f10e5ec36be3 Mon Sep 17 00:00:00 2001
From: Kacper Kornet drae...@pld-linux.org
Date: Sat, 24 Aug 2013 03:58:05 +0100
Subject: [PATCH 2/3] t/t3701-add-interactive.sh: Add PERL prerequisite

The test 'patch mode ignores unmerged entries' uses git-add -p, so it
depends on the perl code.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 t/t3701-add-interactive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 9fab25c..8514220 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -330,7 +330,7 @@ test_expect_success PERL 'split hunk add -p (edit)' '
! grep ^+15 actual
 '
 
-test_expect_success 'patch mode ignores unmerged entries' '
+test_expect_success PERL 'patch mode ignores unmerged entries' '
git reset --hard 
test_commit conflict 
test_commit non-conflict 
-- 
1.8.3.4


From 14f135b178bfaba67653bc3bedf65ec45e38bc76 Mon Sep 17 00:00:00 2001
From: Kacper Kornet drae...@pld-linux.org
Date: Sat, 24 Aug 2013 04:01:02 +0100
Subject: [PATCH 3/3] t/t7106-reset-unborn-branch.sh: Add PERL prerequisite

The test 'reset -p' uses git-reset -p, so it depends on the perl code.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 t/t7106-reset-unborn-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh
index 8062cf5..499cd88 100755
--- a/t/t7106-reset-unborn-branch.sh
+++ b/t/t7106-reset-unborn-branch.sh
@@ -27,7 +27,7 @@ test_expect_success 'reset $file' '
test $(git ls-files) = b
 '
 
-test_expect_success 'reset -p' '
+test_expect_success PERL 'reset -p' '
rm .git/index 
git add a 
echo y | git reset -p 
-- 
1.8.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 0/3] Fixes for tests run without perl

2013-08-23 Thread Kacper Kornet
This is a set of fixes for problems found while running
test suite without perl installed.

Kacper
--
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/3] t/t3701-add-interactive.sh: Add PERL prerequisite

2013-08-23 Thread Kacper Kornet
The test 'patch mode ignores unmerged entries' uses git-add -p, so it
depends on the perl code.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 t/t3701-add-interactive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 9fab25c..8514220 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -330,7 +330,7 @@ test_expect_success PERL 'split hunk add -p (edit)' '
! grep ^+15 actual
 '
 
-test_expect_success 'patch mode ignores unmerged entries' '
+test_expect_success PERL 'patch mode ignores unmerged entries' '
git reset --hard 
test_commit conflict 
test_commit non-conflict 
-- 
1.8.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 1/3] Make test using invalid commit with -C more strict

2013-08-23 Thread Kacper Kornet
In the test 'using invalid commit with -C' git-commit would have failed
even if the -C option  had been given the correct commit, as there was
nothing to commit. Fix it by making sure there is always something to
commit and git-commit fails because of the invalid commit provided to
it.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 t/t7501-commit.sh | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 99ce36f..699a603 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -53,7 +53,10 @@ test_expect_success PERL 'can use paths with --interactive' '
 '
 
 test_expect_success 'using invalid commit with -C' '
-   test_must_fail git commit -C bogus
+   echo bong file 
+   git add file 
+   test_must_fail git commit -C bogus 
+   git reset
 '
 
 test_expect_success 'nothing to commit' '
-- 
1.8.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


git svn exits with error error closing pipe: Bad file descriptor

2013-07-18 Thread Kacper Kornet
After upgrade to subversion 1.8.0 on some repositories
git svn clone shows two warnings:

error closing pipe: Bad file descriptor at
/home/users/kornet/giti/libexec/git-core/git-svn line 0.

at exit. They appear because process git cat-file --batch exits before
command_close_bidi_pipe is called by destructor during perl exit.  The
solution is to call the destructor explicitly, e.q.:

index ff1ce3d..6811738 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2068,6 +2068,10 @@ sub gc_directory {
}
 }
 
+END {
+undef $_repository;
+}
+
 __END__
 
 Data structures:

However I'm not sure whether it is not a symptom of some other error.
What's confuses me more is that the warnings don't appear for all
repositories. For example they appear for  http://svn.pld-linux.org/svn/cdpl/
while cloning http://svn.pld-linux.org/svn/atob ends normally.
On the other hand it is consistent between two different machines with
different Linux distributions and perl versions.

-- 
  Kacper Kornet
--
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] Fix revision walk for commits with the same dates

2013-03-22 Thread Kacper Kornet
Logic in still_interesting function allows to stop the commits
traversing if the oldest processed commit is not older then the
youngest commit on the list to process and the list contains only
commits marked as not interesting ones. It can be premature when dealing
with a set of coequal commits. For example git rev-list A^! --not B
provides wrong answer if all commits in the range A..B had the same
commit time and there are more then 7 of them.

To fix this problem the relevant part of the logic in still_interesting
is changed to: the walk can be stopped if the oldest processed commit is
younger then the youngest commit on the list to processed.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---

I don't know whether the first version was overlooked or deemed as not
worthy. So just in case I resend it. Changes since the first version:

1. The test has been added
2. The commit log has been rewritten


 revision.c |  2 +-
 t/t6009-rev-list-parent.sh | 13 +
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index ef60205..cf620c6 100644
--- a/revision.c
+++ b/revision.c
@@ -709,7 +709,7 @@ static int still_interesting(struct commit_list *src, 
unsigned long date, int sl
 * Does the destination list contain entries with a date
 * before the source list? Definitely _not_ done.
 */
-   if (date  src-item-date)
+   if (date = src-item-date)
return SLOP;
 
/*
diff --git a/t/t6009-rev-list-parent.sh b/t/t6009-rev-list-parent.sh
index 3050740..66cda17 100755
--- a/t/t6009-rev-list-parent.sh
+++ b/t/t6009-rev-list-parent.sh
@@ -133,4 +133,17 @@ test_expect_success 'dodecapus' '
check_revlist --min-parents=13 
check_revlist --min-parents=4 --max-parents=11 tetrapus
 '
+
+test_expect_success 'ancestors with the same commit time' '
+
+   test_tick_keep=$test_tick 
+   for i in 1 2 3 4 5 6 7 8; do
+   test_tick=$test_tick_keep
+   test_commit t$i
+   done 
+   git rev-list t1^! --not t$i result 
+   expect 
+   test_cmp expect result
+'
+
 test_done
-- 
1.8.2

-- 
  Kacper Kornet
--
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] Fix revision walk for commits with the same dates

2013-03-22 Thread Kacper Kornet
On Fri, Mar 22, 2013 at 01:45:47PM -0700, Junio C Hamano wrote:
 Kacper Kornet drae...@pld-linux.org writes:

  Logic in still_interesting function allows to stop the commits
  traversing if the oldest processed commit is not older then the
  youngest commit on the list to process and the list contains only
  commits marked as not interesting ones. It can be premature when dealing
  with a set of coequal commits. For example git rev-list A^! --not B
  provides wrong answer if all commits in the range A..B had the same
  commit time and there are more then 7 of them.

  To fix this problem the relevant part of the logic in still_interesting
  is changed to: the walk can be stopped if the oldest processed commit is
  younger then the youngest commit on the list to processed.

 Is the made-up test case to freeze the clock even interesting?  The
 slop logic is merely a heuristic to compensate for effects caused by
 skewed or non-monototic clocks, so in a different repository you may
 even need to fuzz the timestamp comparison further

   if (date - 10  src-item-date)

 or something silly like that.

I don't think it is a made-up test case. For example it is easy to get a
number of coequal commits by using git rebase -i. So I argue that git
should treat correctly ranges of such commits.

-- 
  Kacper Kornet
--
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] t1507: Test that branchname@{upstream} is interpreted as branch

2013-03-17 Thread Kacper Kornet
Syntax branchname@{upstream} should interpret its argument as a name of
a branch. Add the test to check that it doesn't try to interpret it as a
refname if the branch in question does not exist.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---

Maybe I'm too cautious adding this test. But just in case here it is.

 t/t1507-rev-parse-upstream.sh | 4 
 1 file changed, 4 insertions(+)

diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index d6e5761..b27a720 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -54,6 +54,10 @@ test_expect_success 'my-side@{upstream} resolves to correct 
full name' '
test refs/remotes/origin/side = $(full_name my-side@{u})
 '
 
+test_expect_success 'refs/heads/my-side@{upstream} does not resolve to 
my-side{upstream}' '
+   test_must_fail full_name refs/heads/my-side@{upstream}
+'
+
 test_expect_success 'my-side@{u} resolves to correct commit' '
git checkout side 
test_commit 5 
-- 
1.8.2

--
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] rev-parse: Clarify documentation of @{upstream} syntax

2013-03-16 Thread Kacper Kornet
git-rev-parse interprets string in string@{upstream} as a name of
a branch not a ref. For example refs/heads/master@{upstream} looks
for an upstream branch that is merged by git-pull to ref
refs/heads/refs/heads/master not to refs/heads/master. However the
documentation could misled a user to believe that the string is
interpreted as ref.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 Documentation/revisions.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 678d175..314e25d 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -88,10 +88,10 @@ some output processing may assume ref names in UTF-8.
   The construct '@\{-n\}' means the nth branch checked out
   before the current one.
 
-'refname@\{upstream\}', e.g. 'master@\{upstream\}', '@\{u\}'::
-  The suffix '@\{upstream\}' to a ref (short form 'refname@\{u\}') refers to
-  the branch the ref is set to build on top of.  A missing ref defaults
-  to the current branch.
+'branchname@\{upstream\}', e.g. 'master@\{upstream\}', '@\{u\}'::
+  The suffix '@\{upstream\}' to a branchname (short form 'branchname@\{u\}')
+  refers to the branch that the branch specified by branchname is set to build 
on
+  top of.  A missing branchname defaults to the current one.
 
 'rev{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0'::
   A suffix '{caret}' to a revision parameter means the first parent of
-- 
1.8.2

--
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] Fix revision walk for commits with the same dates

2013-03-07 Thread Kacper Kornet
git rev-list A^! --not B provides wrong answer if all commits in the
range A..B had the same commit times and there are more then 8 of them.
This commits fixes the logic in still_interesting function to prevent
this error.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 revision.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index ef60205..cf620c6 100644
--- a/revision.c
+++ b/revision.c
@@ -709,7 +709,7 @@ static int still_interesting(struct commit_list *src, 
unsigned long date, int sl
 * Does the destination list contain entries with a date
 * before the source list? Definitely _not_ done.
 */
-   if (date  src-item-date)
+   if (date = src-item-date)
return SLOP;
 
/*
-- 
1.8.2.rc2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 3/3] Add option to transpose parents of merge commit

2012-11-27 Thread Kacper Kornet
When the changes are pushed upstream, and in the meantime someone else
updated upstream branch git advises to use git pull. This results in
history:

 ---A---B---C--
 \ /
  D---E

where B is the local commit. D, E are commits pushed by someone else
when the developer was working on B. However sometimes the following
history is preferable:

---A---D---C'--
\ /
 '-B-'

The difference between C and C' is the order of parents. This change
allow for easier way to obtain this effect by introducing the option
--transpose-parents to git-merge and git-pull, which changes the order
of parents in resulting commit moving the original first parent to
the very end of list of parents.

The transposition is done just before the commit is finalized, so the
meaning of our and their for conflict resolution is not changed, so
ours denotes local version and theirs upstream.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 Documentation/merge-options.txt |  7 +++
 builtin/commit.c|  8 +++-
 builtin/merge.c | 16 
 commit.c| 11 +++
 commit.h|  2 ++
 git-pull.sh |  4 +++-
 6 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 0bcbe0a..b4fbfdc 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -88,6 +88,13 @@ option can be used to override --squash.
Synonyms to --stat and --no-stat; these are deprecated and will be
removed in the future.
 
+--transpose-parents::
+   Transpose the parents in the final commit. The change is made
+   just before the commit so the meaning of 'our' and 'their'
+   concepts remains the same (i.e. 'our' means current branch before
+   the merge).
+
+
 ifndef::git-pull[]
 -q::
 --quiet::
diff --git a/builtin/commit.c b/builtin/commit.c
index ee0e884..ab2b844 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1477,6 +1477,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
} else if (whence == FROM_MERGE) {
struct strbuf m = STRBUF_INIT;
FILE *fp;
+   int reversed_order=0;
 
if (!reflog_msg)
reflog_msg = commit (merge);
@@ -1484,10 +1485,13 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
while (strbuf_getline(m, fp, '\n') != EOF) {
if (!strcmp(m.buf, no-ff))
allow_fast_forward = 0;
+   if (!strcmp(m.buf, reversed-order))
+   reversed_order = 1;
}
fclose(fp);
}
-   pptr = commit_list_insert(current_head, pptr)-next;
+   if (!reversed_order)
+   pptr = commit_list_insert(current_head, pptr)-next;
fp = fopen(git_path(MERGE_HEAD), r);
if (fp == NULL)
die_errno(_(could not open '%s' for reading),
@@ -1502,6 +1506,8 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
}
fclose(fp);
strbuf_release(m);
+   if (reversed_order)
+   pptr = commit_list_insert(current_head, pptr)-next;
if (allow_fast_forward)
parents = reduce_heads(parents);
} else {
diff --git a/builtin/merge.c b/builtin/merge.c
index a96e8ea..41738a5 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -65,6 +65,7 @@ static int abort_current_merge;
 static int show_progress = -1;
 static int default_to_upstream;
 static const char *sign_commit;
+static int reversed_order=0;
 
 static struct strategy all_strategy[] = {
{ recursive,  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -213,6 +214,7 @@ static struct option builtin_merge_options[] = {
{ OPTION_STRING, 'S', gpg-sign, sign_commit, N_(key id),
  N_(GPG sign commit), PARSE_OPT_OPTARG, NULL, (intptr_t)  },
OPT_BOOLEAN(0, overwrite-ignore, overwrite_ignore, N_(update 
ignored files (default))),
+   OPT_BOOLEAN(0, transpose-parents, reversed_order, N_(reverse order 
of parents)),
OPT_END()
 };
 
@@ -822,9 +824,9 @@ static int merge_trivial(struct commit *head, struct 
commit_list *remoteheads)
 
write_tree_trivial(result_tree);
printf(_(Wonderful.\n));
-   parent-item = head;
+   parent-item = reversed_order ? remoteheads-item : head;
parent-next = xmalloc(sizeof(*parent-next));
-   parent-next-item = remoteheads-item;
+   parent-next-item = reversed_order ? head : remoteheads-item;
parent-next-next = NULL;
prepare_to_commit(remoteheads);
if (commit_tree(merge_msg, result_tree

[PATCH v2 1/3] Process MERGE_MODE before MERGE_HEAD

2012-11-27 Thread Kacper Kornet
It is in preparation to introduce --transpose-parents option to
git-merge, when the content of MERGE_MODE will dictate how the
MERGE_HEAD is interpreted.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 builtin/commit.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 1dd2ec5..273332f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1481,6 +1481,12 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
 
if (!reflog_msg)
reflog_msg = commit (merge);
+   if (!stat(git_path(MERGE_MODE), statbuf)) {
+   if (strbuf_read_file(sb, git_path(MERGE_MODE), 0)  
0)
+   die_errno(_(could not read MERGE_MODE));
+   if (!strcmp(sb.buf, no-ff))
+   allow_fast_forward = 0;
+   }
pptr = commit_list_insert(current_head, pptr)-next;
fp = fopen(git_path(MERGE_HEAD), r);
if (fp == NULL)
@@ -1496,12 +1502,6 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
}
fclose(fp);
strbuf_release(m);
-   if (!stat(git_path(MERGE_MODE), statbuf)) {
-   if (strbuf_read_file(sb, git_path(MERGE_MODE), 0)  
0)
-   die_errno(_(could not read MERGE_MODE));
-   if (!strcmp(sb.buf, no-ff))
-   allow_fast_forward = 0;
-   }
if (allow_fast_forward)
parents = reduce_heads(parents);
} else {
-- 
1.8.0.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


[PATCH v2 2/3] Allow for MERGE_MODE to specify more then one mode

2012-11-27 Thread Kacper Kornet
Presently only one merge mode exists: non-fast-forward. But in future
the second one (transpose-parents) will be added, so the need to read
all lines of MERGE_MODE.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 builtin/commit.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 273332f..ee0e884 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1427,7 +1427,6 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
unsigned char sha1[20];
struct ref_lock *ref_lock;
struct commit_list *parents = NULL, **pptr = parents;
-   struct stat statbuf;
int allow_fast_forward = 1;
struct commit *current_head = NULL;
struct commit_extra_header *extra = NULL;
@@ -1481,11 +1480,12 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
 
if (!reflog_msg)
reflog_msg = commit (merge);
-   if (!stat(git_path(MERGE_MODE), statbuf)) {
-   if (strbuf_read_file(sb, git_path(MERGE_MODE), 0)  
0)
-   die_errno(_(could not read MERGE_MODE));
-   if (!strcmp(sb.buf, no-ff))
-   allow_fast_forward = 0;
+   if((fp = fopen(git_path(MERGE_MODE), r))) {
+   while (strbuf_getline(m, fp, '\n') != EOF) {
+   if (!strcmp(m.buf, no-ff))
+   allow_fast_forward = 0;
+   }
+   fclose(fp);
}
pptr = commit_list_insert(current_head, pptr)-next;
fp = fopen(git_path(MERGE_HEAD), r);
-- 
1.8.0.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


[PATCH v2 0/3] Add option to change order of parents in merge commit

2012-11-27 Thread Kacper Kornet
The second version of patches introducing option to change order
of parents in merge commits. The changes in respect to the previous
version:

- I have divided the changes to the preparatory ones, which
  only refactore the code without introducing new functionality, and
  the commit which introduces the new option

- The documentation for the new options has been added

This is not yet a final version, as the tests are missing. But maybe while I'm 
working
on them there will be some comments.

Kacper Kornet (3):
  Process MERGE_MODE before MERGE_HEAD
  Allow for MERGE_MODE to specify more then one mode
  Add option to transpose parents of merge commit

 Documentation/merge-options.txt |  7 +++
 builtin/commit.c| 22 ++
 builtin/merge.c | 16 
 commit.c| 11 +++
 commit.h|  2 ++
 git-pull.sh |  4 +++-
 6 files changed, 49 insertions(+), 13 deletions(-)

-- 
1.8.0.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 v2 2/3] Allow for MERGE_MODE to specify more then one mode

2012-11-27 Thread Kacper Kornet
On Tue, Nov 27, 2012 at 06:17:28PM -0800, Junio C Hamano wrote:
 Kacper Kornet drae...@pld-linux.org writes:

  Presently only one merge mode exists: non-fast-forward. But in future
  the second one (transpose-parents) will be added, so the need to read
  all lines of MERGE_MODE.

  Signed-off-by: Kacper Kornet drae...@pld-linux.org
  ---
   builtin/commit.c | 12 ++--
   1 file changed, 6 insertions(+), 6 deletions(-)

  diff --git a/builtin/commit.c b/builtin/commit.c
  index 273332f..ee0e884 100644
  --- a/builtin/commit.c
  +++ b/builtin/commit.c
  @@ -1427,7 +1427,6 @@ int cmd_commit(int argc, const char **argv, const 
  char *prefix)
  unsigned char sha1[20];
  struct ref_lock *ref_lock;
  struct commit_list *parents = NULL, **pptr = parents;
  -   struct stat statbuf;
  int allow_fast_forward = 1;
  struct commit *current_head = NULL;
  struct commit_extra_header *extra = NULL;
  @@ -1481,11 +1480,12 @@ int cmd_commit(int argc, const char **argv, const 
  char *prefix)

  if (!reflog_msg)
  reflog_msg = commit (merge);
  -   if (!stat(git_path(MERGE_MODE), statbuf)) {
  -   if (strbuf_read_file(sb, git_path(MERGE_MODE), 0)  
  0)
  -   die_errno(_(could not read MERGE_MODE));
  -   if (!strcmp(sb.buf, no-ff))
  -   allow_fast_forward = 0;
  +   if((fp = fopen(git_path(MERGE_MODE), r))) {

 Style: s/if((fp/if ((fp/;

  +   while (strbuf_getline(m, fp, '\n') != EOF) {
  +   if (!strcmp(m.buf, no-ff))
  +   allow_fast_forward = 0;
  +   }
  +   fclose(fp);

 This needs a bit more careful planning for interacting with other
 people's programs, I suspect.

 Your updated builtin/merge.c may write an extra LF after no-ff to
 make this parser to grok it, but it is entirely plausible that
 people have their own Porcelain that writes no-ff without LF
 (because that is what we read from this file, and I suspect the
 current code would ignore no-ff\n).

 At least strbuf_getline() would give us no-ff when either no-ff
 or no-ff\n terminates the file, so updated code would be able to
 grok what other people would write, but if other people want to read
 MERGE_MODE we write, at least we shouldn't break them when we only
 write no-ff in it (once you start writing reverse-parents in the
 file, they will be broken anyway, as they do not currently expect
 such token in this file).

At this stage in the patch series the format of MERGE_MODE is not
changed - no-ff is printed without \n. What should be changed is the
next patch. Relevant part should read:

diff --git a/builtin/merge.c b/builtin/merge.c
index a96e8ea..5ceb291 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -993,6 +999,8 @@ static void write_merge_state(struct commit_list 
*remoteheads)
if (fd  0)
die_errno(_(Could not open '%s' for writing), filename);
strbuf_reset(buf);
+   if (reversed_order)
+   strbuf_addf(buf, reversed-order\n);
if (!allow_fast_forward)
strbuf_addf(buf, no-ff);
if (write_in_full(fd, buf.buf, buf.len) != buf.len)

This way when only no-ff is specified all parsers should be happy. If
reversed-order is specified together no-ff the external parser
probably would fail. Which in my opinion is a good think at this point,
as it can't correctly interpret MERGE_MODE anyway.

-- 
  Kacper
--
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 3/3] Add option to transpose parents of merge commit

2012-11-27 Thread Kacper Kornet
On Tue, Nov 27, 2012 at 06:18:00PM -0800, Junio C Hamano wrote:
 Kacper Kornet drae...@pld-linux.org writes:

  +--transpose-parents::
  +   Transpose the parents in the final commit. The change is made
  +   just before the commit so the meaning of 'our' and 'their'
  +   concepts remains the same (i.e. 'our' means current branch before
  +   the merge).
  +

 How does this interact with Octopus merges?

It moves the original first parent to the last position. And nothing
more. I have forgotten to mention it in the documentation.

  diff --git a/builtin/commit.c b/builtin/commit.c
  index ee0e884..ab2b844 100644
  --- a/builtin/commit.c
  +++ b/builtin/commit.c
  @@ -1477,6 +1477,7 @@ int cmd_commit(int argc, const char **argv, const 
  char *prefix)
  } else if (whence == FROM_MERGE) {
  struct strbuf m = STRBUF_INIT;
  FILE *fp;
  +   int reversed_order=0;

 Style. s/=/ = /;

  +   OPT_BOOLEAN(0, transpose-parents, reversed_order, N_(reverse order 
  of parents)

 It smells more like --reverse-parents (if you deal only with
 two-head merges), no?

I have changes to --transpose-parents because of the octopus merges.
Although it is not a mathematical transposition in this case, but a cycle
permutation. 

-- 
  Kacper
--
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: [RFC/PATCH] Option to revert order of parents in merge commit

2012-11-26 Thread Kacper Kornet
On Fri, Nov 23, 2012 at 06:58:49PM -0800, Junio C Hamano wrote:
 Kacper Kornet drae...@pld-linux.org writes:

  The following patch is an attempt to implement this idea.

 I think revert is a wrong word (implying you have already done
 something and you are trying to defeat the effect of that old
 something), and you meant to say reverse (i.e. the opposite of
 normal) or something.

You are right. Probably transpose is the best description what the patch
really does.

 I am unsure about the usefulness of this, though.

 After completing a topic on branch A, you would merge it to your own
 copy of the integration branch (e.g. 'master') and try to push,
 which may be rejected due to non-fast-forwardness:

 $ git checkout master
 $ git merge A
 $ git push

 At that point, if you _care_ about the merge parent order, you could
 do this (still on 'master'):

 $ git fetch origin
 $ git reset --hard origin/master
 $ git merge A
 $ test test test
 $ git push

 With --reverse-parents, it would become:

 $ git pull --reverse-parents
 $ test test test
 $ git push

 which certainly is shorter and looks simpler.  The workflow however
 would encourage people to work directly on the master branch, which
 is a bit of downside.

Our developers work mainly on master branches. The project consists of
many thousands independent git repositories, and at the given time a
developer usually wants to make only one commit in the given repository
and push his changes upstream. So he usually doesn't care to make a
branch.  Then after failed pushed, one needs to add creation and removal
of temporary branch (see the commit message of the suggested patch).
The possibility to do git pull --reverse-parent would make the life
easier in this case.

 Is there any interaction between this pull --reverse-parents
 change and possible conflict resolution when the command stops and
 asks the user for help?  For example, whom should --ours and -X
 ours refer to?  Us, or the upstream?

The change of order of parents happens at the very last moment, so
ours in merge options is local version and theirs upstream.

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


[RFC/PATCH] Option to revert order of parents in merge commit

2012-11-23 Thread Kacper Kornet
When the changes are pushed upstream, and in the meantime someone else
updated upstream branch git advises to use git pull. This results in
history:

 ---A---B---C--
 \ /
  D---E

where B is my commit. D, E are commits pushed by someone else when I was
working on B. However sometimes the following history is preferable:

---A---D---C'--
\ /
  -B-

The difference between C and C' is the order of parents. Presently to
obtain it, instead of git pull, one needs to do (assuming that I am on
the master branch):

git fetch origin
git branch tmp_branch
git reset --hard origin/master
git merge tmp_branch

Reverting from wrong pull is more cumbersome. It would be simpler if git
merge learn an option to reverse order of parents in the produced
commits, so one could do:

git fetch origin
git merge --revert-order origin/master

The following patch is an attempt to implement this idea.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---

I'm not 100% percent sure that it is a good idea. But it would make life
of our developers easier and produced nicer history then using git pull.
Git pull seems to written for a case of single maintainer who gathers
contributions from other developers and incorporates them in master
branch. However in my opinion it doesn't produce best history when many
developers modify the canonical repository. 

 builtin/commit.c | 22 ++
 builtin/merge.c  | 16 
 commit.c | 11 +++
 commit.h |  2 ++
 4 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 1dd2ec5..ab2b844 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1427,7 +1427,6 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
unsigned char sha1[20];
struct ref_lock *ref_lock;
struct commit_list *parents = NULL, **pptr = parents;
-   struct stat statbuf;
int allow_fast_forward = 1;
struct commit *current_head = NULL;
struct commit_extra_header *extra = NULL;
@@ -1478,10 +1477,21 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
} else if (whence == FROM_MERGE) {
struct strbuf m = STRBUF_INIT;
FILE *fp;
+   int reversed_order=0;
 
if (!reflog_msg)
reflog_msg = commit (merge);
-   pptr = commit_list_insert(current_head, pptr)-next;
+   if((fp = fopen(git_path(MERGE_MODE), r))) {
+   while (strbuf_getline(m, fp, '\n') != EOF) {
+   if (!strcmp(m.buf, no-ff))
+   allow_fast_forward = 0;
+   if (!strcmp(m.buf, reversed-order))
+   reversed_order = 1;
+   }
+   fclose(fp);
+   }
+   if (!reversed_order)
+   pptr = commit_list_insert(current_head, pptr)-next;
fp = fopen(git_path(MERGE_HEAD), r);
if (fp == NULL)
die_errno(_(could not open '%s' for reading),
@@ -1496,12 +1506,8 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
}
fclose(fp);
strbuf_release(m);
-   if (!stat(git_path(MERGE_MODE), statbuf)) {
-   if (strbuf_read_file(sb, git_path(MERGE_MODE), 0)  
0)
-   die_errno(_(could not read MERGE_MODE));
-   if (!strcmp(sb.buf, no-ff))
-   allow_fast_forward = 0;
-   }
+   if (reversed_order)
+   pptr = commit_list_insert(current_head, pptr)-next;
if (allow_fast_forward)
parents = reduce_heads(parents);
} else {
diff --git a/builtin/merge.c b/builtin/merge.c
index a96e8ea..8d0ed18 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -65,6 +65,7 @@ static int abort_current_merge;
 static int show_progress = -1;
 static int default_to_upstream;
 static const char *sign_commit;
+static int reversed_order=0;
 
 static struct strategy all_strategy[] = {
{ recursive,  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -213,6 +214,7 @@ static struct option builtin_merge_options[] = {
{ OPTION_STRING, 'S', gpg-sign, sign_commit, N_(key id),
  N_(GPG sign commit), PARSE_OPT_OPTARG, NULL, (intptr_t)  },
OPT_BOOLEAN(0, overwrite-ignore, overwrite_ignore, N_(update 
ignored files (default))),
+   OPT_BOOLEAN(0, revert-order, reversed_order, N_(reverse order of 
parents)),
OPT_END()
 };
 
@@ -822,9 +824,9 @@ static int merge_trivial(struct commit *head, struct 
commit_list *remoteheads)
 
write_tree_trivial(result_tree);
printf(_(Wonderful.\n));
-   parent-item = head;
+   parent-item

Re: [PATCH v3 0/5] push: update remote tags only with force

2012-11-14 Thread Kacper Kornet
On Wed, Nov 14, 2012 at 12:29:14AM -0600, Chris Rorvick wrote:

2. Require force when updating tag references, even on a fast-forward.

   push: flag updates
   push: flag updates that require force
   push: update remote tags only with force

   An email thread initiated by Angelo Borsotti did not come to a
   consensus on how push should behave with regard to tag references.

  I think the original motivation of allowing fast-forward updates to
  tags was for people who wanted to have today's recommended version
  tag that can float from day to day. I tend to think that was a
  misguided notion and it is better implemented with a tip of a
  branch (iow, I personally am OK with the change to forbid tag
  updates altogether, without --force).

   I think a key point is that you currently cannot be sure your push
   will not clobber a tag (lightweight or not) in the remote.

  Do not update, only add new may be a good feature, but at the same
  time I have this suspicion that its usefulness may not necessarily
  be limited to refs/tags/* hierarchy.

  I dunno.

 Are you suggesting allowing forwards for just refs/heads/*?  I
 initially went this route based on some feedback in the original
 thread, but being that specific broke a couple tests in t5516 (i.e.,
 pushing to refs/remotes/origin/master and another into refs/tmp/*.)
 My initial thought was that I'd broken something and I need to modify
 the patch, but now I think I should just modify those tests.  Branches
 are restricted to refs/heads/* (if I understand correctly), so
 allowing fast-forwards when pushing should be limited to this
 hierarchy, too.

What about notes? I think they should be treated in the same way as
branches. My impression is that tags are exceptional in this respect.

-- 
  Kacper
--
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 push tags

2012-10-29 Thread Kacper Kornet
On Mon, Oct 29, 2012 at 09:12:52AM +0100, Angelo Borsotti wrote:
 Hi,

 to let the owner of a remote repository (one on which git-push
 deposits objects) disallow
 others to change tags, a key on its config file could be used.
 An option on git-push, or environment variable, or key in config file
 of the repo from which git-push takes objects do not help in enforcing
 the policy not to update tags in the remote repo.

It think these are two separate issues:

1. Enforcing policy that the server should not accept changes in
existing tags. This can be easily done with hooks.

2. Perform operation: push my tag if and only if it doesn't overwrite
tag which already exists on the server. I think currently it is not possible
with git.

-- 
  Kacper
--
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 push tags

2012-10-29 Thread Kacper Kornet
On Mon, Oct 29, 2012 at 07:35:00AM -0400, Jeff King wrote:
 On Mon, Oct 29, 2012 at 07:21:52AM -0400, Drew Northup wrote:

   I would have expected git to at least complain about updating an
   annotated tag with another annotated tag. But it actually uses the same
   fast-forward rule, just on the pointed-to commits. So a fast-forward
   annotated re-tag will throw away the old tag object completely. Which
   seems a bit crazy to me.

   It seems like a no-brainer to me that annotated tags should not replace
   each other without a force, no matter where in the refs hierarchy they
   go.

   For lightweight tags, I think it's more gray. They are just pointers
   into history. Some projects may use them to tag immutable official
   versions, but I also see them used as shared bookmarks. Requiring -f
   may make the latter use more annoying. On the other hand, bookmark tags
   tend not to be pushed, or if they are, it is part of a mirror-like
   backup which should be forcing all updates anyway.

  Would that be an endorsement of continuing to build a patch set
  including the snippet that Kacper posted earlier (1) in response to my
  comment about not being sure how complicated all of this would be or
  not?

 That patch just blocks non-forced updates to refs/tags/. I think a saner
 start would be to disallow updating non-commit objects without a force.
 We already do so for blobs and trees because they are not (and cannot
 be) fast forwards. The fact that annotated tags are checked for
 fast-forward seems to me to be a case of it happens to work that way
 and not anything planned. Since such a push drops the reference to the
 old version of the tag, it should probably require a force.

I'm not sure. Looking at 37fde87 (Fix send-pack for non-commitish
tags.) I have an impression that Junio allowed for fast-forward pushes
of annotated tags on purpose. 

 Then on top of that we can talk about what lightweight tags should do.
 I'm not sure. Following the regular fast-forward rules makes some sense
 to me, because you are never losing objects. But there may be
 complications with updating tags in general because of fetch's rules,
 and we would be better off preventing people from accidentally doing so.
 I think a careful review of fetch's tag rules would be in order before
 making any decision there.

The problem with the current behaviour is, that one can never be 100% sure
that his push will not overwrite someone else tag.

-- 
  Kacper
--
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 push tags

2012-10-26 Thread Kacper Kornet
On Thu, Oct 25, 2012 at 05:16:00PM -0400, Drew Northup wrote:
 On Thu, Oct 25, 2012 at 3:05 PM, Angelo Borsotti
 angelo.borso...@gmail.com wrote:
  Are remote repositories less protected than the local ones? I
  think that to be consistent, the same strategy should be used on all
  repositories, i.e. rejecting changes on tags by default, unless they
  are forced.

 So here we come to the core argument. Is sounds to me like you want
 changes to remote tags to work differently from push updates to ALL
 other references. The required change, if I'm not mistaken, would be
 for tags to not permit fast-forward updates while all other references
 would be pushed normally. From my brief and un-enlightened look at the
 push code I can't see that being as easy as it sounds.

I think the patch below obtains the requested behaviour:

diff --git a/remote.c b/remote.c
index 04fd9ea..7fcb51e 100644
--- a/remote.c
+++ b/remote.c
@@ -1320,7 +1320,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int 
send_mirror,
!ref-deletion 
!is_null_sha1(ref-old_sha1) 
(!has_sha1_file(ref-old_sha1)
- || !ref_newer(ref-new_sha1, ref-old_sha1));
+ || !prefixcmp(ref-name, refs/tags) || 
!ref_newer(ref-new_sha1, ref-old_sha1));
 
if (ref-nonfastforward  !ref-force  !force_update) {
ref-status = REF_STATUS_REJECT_NONFASTFORWARD;

-- 
  Kacper Kornet
--
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 push tags

2012-10-26 Thread Kacper Kornet
On Fri, Oct 26, 2012 at 02:07:09PM -0400, Drew Northup wrote:
 On Fri, Oct 26, 2012 at 1:42 PM, Kacper Kornet drae...@pld-linux.org wrote:
  On Thu, Oct 25, 2012 at 05:16:00PM -0400, Drew Northup wrote:
  On Thu, Oct 25, 2012 at 3:05 PM, Angelo Borsotti
  angelo.borso...@gmail.com wrote:
   Are remote repositories less protected than the local ones? I
   think that to be consistent, the same strategy should be used on all
   repositories, i.e. rejecting changes on tags by default, unless they
   are forced.

  So here we come to the core argument. Is sounds to me like you want
  changes to remote tags to work differently from push updates to ALL
  other references. The required change, if I'm not mistaken, would be
  for tags to not permit fast-forward updates while all other references
  would be pushed normally. From my brief and un-enlightened look at the
  push code I can't see that being as easy as it sounds.

  I think the patch below obtains the requested behaviour:

  diff --git a/remote.c b/remote.c
  index 04fd9ea..7fcb51e 100644
  --- a/remote.c
  +++ b/remote.c
  @@ -1320,7 +1320,7 @@ void set_ref_status_for_push(struct ref *remote_refs, 
  int send_mirror,
  !ref-deletion 
  !is_null_sha1(ref-old_sha1) 
  (!has_sha1_file(ref-old_sha1)
  - || !ref_newer(ref-new_sha1, ref-old_sha1));
  + || !prefixcmp(ref-name, refs/tags) || 
  !ref_newer(ref-new_sha1, ref-old_sha1));

  if (ref-nonfastforward  !ref-force  !force_update) {
  ref-status = REF_STATUS_REJECT_NONFASTFORWARD;

  --
Kacper Kornet

 Kacper,
 I obviously didn't dig deep enough. In any case, I presume that's what
 he's asking for. I can't remember if git is still forcing tags to
 always be located in refs/tags however. I didn't think it was.

I have based my assumption about location of tags on gitglossary:

tag
 A ref under refs/tags/ namespace that points to an object of an
 arbitrary type (typically a tag points to either a tag or a commit
 object).

-- 
  Kacper
--
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 push tags

2012-10-26 Thread Kacper Kornet
On Fri, Oct 26, 2012 at 08:35:50PM +0200, Angelo Borsotti wrote:
 Hello

 Drew,

 I made some further tests on git-push to see if it handled branches
 and tags in the same way, and have discovered the following
 differences:

 - git push origin --delete master
   remote: error: By default, deleting the current branch is denied

 - git push origin --delete vx(where vx is a tag)
   ... accepted

 This is consistent with what is done on the local repo: deleting the
 current branch is disallowed, but deleting a tag is allowed (even when
 HEAD points to it).
 That means that git-push does not handle branches and tags exactly the same.

 Kacper

 thank you for the patch. To keep downward compatibility, the denial to
 update tags should perhaps be enabled with some option.

You are probably right. The proper submission should also contain a
test. I have sent a crude patch to show that the behaviour asked by you
is possible to obtain.

I will try to prepare a formal submission patch, but I can't say how
long it will take me. So if you want to do it by yourself feel free.

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


rev-parse -q --verify with not existent upstream branch

2012-08-31 Thread Kacper Kornet

Documentation to git-rev-parse claims that

-q, --quiet
   Only meaningful in --verify mode. Do not output an error
   message if the first argument is not a valid object name; instead exit
   with non-zero status silently.


However when I try to check if the current branch has a configured
upstream one with:

git rev-parse -q --verify  '@{u}'

I still receives the error message:

error: No upstream configured for branch 'threading'

Additinally withour -q --verify options the error message is printed
twice:

error: No upstream configured for branch 'threading'
@{u}
error: No upstream configured for branch 'threading'
fatal: ambiguous argument '@{u}': unknown revision or path not in the
working tree.
Use '--' to separate paths from revision

-- 
  Kacper
--
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 1/2] t6300: test sort with multiple keys

2012-08-21 Thread Kacper Kornet
Documentation of git-for-each-ref says that --sort=key option can be
used multiple times, in which case the last key becomes the primary key.
However this functionality was never checked in test suite and is
currently broken. This commit adds appropriate test in preparation for fix.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 t/t6300-for-each-ref.sh | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 1721784..a0d82d4 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -456,4 +456,14 @@ test_atom refs/tags/signed-long contents subject line
 body contents
 $sig
 
+cat expected \EOF
+408fe76d02a785a006c2e9c669b7be5589ede96d commit...@example.com 
refs/tags/master
+90b5ebede4899eda64893bc2a4c8f1d6fb6dfc40 commit...@example.com refs/tags/bogo
+EOF
+
+test_expect_failure 'Verify sort with multiple keys' '
+   git for-each-ref --format=%(objectname) %(taggeremail) %(refname) 
--sort=objectname --sort=taggeremail \
+   refs/tags/bogo refs/tags/master  actual 
+   test_cmp expected actual
+'
 test_done
-- 
1.7.12.rc3


-- 
  Kacper Kornet
--
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 2/2] for-each-ref: Fix sort with multiple keys

2012-08-21 Thread Kacper Kornet
The linked list describing sort options was not correctly set up in
opt_parse_sort. In the result, contrary to the documentation, only the
last of multiple --sort options to git-for-each-ref was taken into
account. This commit fixes it.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 builtin/for-each-ref.c  | 4 +++-
 t/t6300-for-each-ref.sh | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index b01d76a..0c5294e 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -962,7 +962,9 @@ static int opt_parse_sort(const struct option *opt, const 
char *arg, int unset)
if (!arg) /* should --no-sort void the list ? */
return -1;
 
-   *sort_tail = s = xcalloc(1, sizeof(*s));
+   s = xcalloc(1, sizeof(*s));
+   s-next = *sort_tail;
+   *sort_tail = s;
 
if (*arg == '-') {
s-reverse = 1;
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index a0d82d4..752f5cb 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -461,7 +461,7 @@ cat expected \EOF
 90b5ebede4899eda64893bc2a4c8f1d6fb6dfc40 commit...@example.com refs/tags/bogo
 EOF
 
-test_expect_failure 'Verify sort with multiple keys' '
+test_expect_success 'Verify sort with multiple keys' '
git for-each-ref --format=%(objectname) %(taggeremail) %(refname) 
--sort=objectname --sort=taggeremail \
refs/tags/bogo refs/tags/master  actual 
test_cmp expected actual
-- 
1.7.12.rc3
--
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] t6300: test sort with multiple keys

2012-08-19 Thread Kacper Kornet
Documentation of git-for-each-ref says that --sort=key option can be
used multiple times, in which case the last key becomes the primary key.
However this functionality was never checked in test suite and is
currently broken. This commit adds appropriate test in preparation for fix.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 t/t6300-for-each-ref.sh | 33 -
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 1721784..3d59bfc 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -242,7 +242,32 @@ test_expect_success 'Verify descending sort' '
test_cmp expected actual
 '
 
+test_expect_success 'Create branches to test sort with multiple keys' '
+   git checkout -b Branch1 
+   echo foo  one 
+   git commit -a -m Branch1 commit 
+   git checkout -b Branch2 
+   echo foo  one 
+   git commit -a -m Branch2 commit
+'
+
+test_atom refs/heads/Branch1 objectname 
32fca05e9f638021a123a84226acf17756acc18b
+test_atom refs/heads/Branch2 objectname 
194a5b89ac661a114566ba4374bc06c2797539f3
+
 cat expected \EOF
+67a36f10722846e891fbada1ba48ed035de75581 commitrefs/heads/master
+194a5b89ac661a114566ba4374bc06c2797539f3 commitrefs/heads/Branch2
+32fca05e9f638021a123a84226acf17756acc18b commitrefs/heads/Branch1
+EOF
+
+test_expect_failure 'Verify sort with multiple keys' '
+   git for-each-ref --sort=objectname --sort=committerdate refs/heads  
actual 
+   test_cmp expected actual
+'
+
+cat expected \EOF
+'refs/heads/Branch1'
+'refs/heads/Branch2'
 'refs/heads/master'
 'refs/remotes/origin/master'
 'refs/tags/testtag'
@@ -264,6 +289,8 @@ test_expect_success 'Quoting style: python' '
 '
 
 cat expected \EOF
+refs/heads/Branch1
+refs/heads/Branch2
 refs/heads/master
 refs/remotes/origin/master
 refs/tags/testtag
@@ -285,6 +312,8 @@ for i in --perl --shell -s --python --python --tcl 
--tcl --perl; do
 done
 
 cat expected \EOF
+Branch1
+Branch2
 master
 testtag
 EOF
@@ -296,6 +325,8 @@ test_expect_success 'Check short refname format' '
 '
 
 cat expected EOF
+
+
 origin/master
 EOF
 
@@ -309,7 +340,7 @@ cat expected EOF
 EOF
 
 test_expect_success 'Check short objectname format' '
-   git for-each-ref --format=%(objectname:short) refs/heads actual 
+   git for-each-ref --format=%(objectname:short) refs/heads/master 
actual 
test_cmp expected actual
 '
 
-- 
1.7.12.rc3
--
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] for-each-ref: Fix sort with multiple keys

2012-08-19 Thread Kacper Kornet
The linked list describing sort options was not correctly set up in
opt_parse_sort. In the result, contrary to the documentation. only the
last of multiple --sort options to git-for-each-ref was taken into
account. This commit fixes it.

Signed-off-by: Kacper Kornet drae...@pld-linux.org
---
 builtin/for-each-ref.c  | 4 +++-
 t/t6300-for-each-ref.sh | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index b01d76a..0c5294e 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -962,7 +962,9 @@ static int opt_parse_sort(const struct option *opt, const 
char *arg, int unset)
if (!arg) /* should --no-sort void the list ? */
return -1;
 
-   *sort_tail = s = xcalloc(1, sizeof(*s));
+   s = xcalloc(1, sizeof(*s));
+   s-next = *sort_tail;
+   *sort_tail = s;
 
if (*arg == '-') {
s-reverse = 1;
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 3d59bfc..4c5d8ba 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -260,7 +260,7 @@ cat expected \EOF
 32fca05e9f638021a123a84226acf17756acc18b commitrefs/heads/Branch1
 EOF
 
-test_expect_failure 'Verify sort with multiple keys' '
+test_expect_success 'Verify sort with multiple keys' '
git for-each-ref --sort=objectname --sort=committerdate refs/heads  
actual 
test_cmp expected actual
 '
-- 
1.7.12.rc3
--
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] t6300: test sort with multiple keys

2012-08-19 Thread Kacper Kornet
On Sun, Aug 19, 2012 at 05:38:29PM -0700, Junio C Hamano wrote:
 Kacper Kornet drae...@pld-linux.org writes:

  Documentation of git-for-each-ref says that --sort=key option can be
  used multiple times, in which case the last key becomes the primary key.
  However this functionality was never checked in test suite and is
  currently broken. This commit adds appropriate test in preparation for fix.

  Signed-off-by: Kacper Kornet drae...@pld-linux.org
  ---

 Thanks.

  +test_expect_success 'Create branches to test sort with multiple keys' '
  +   git checkout -b Branch1 
  +   echo foo  one 
  +   git commit -a -m Branch1 commit 
  +   git checkout -b Branch2 
  +   echo foo  one 
  +   git commit -a -m Branch2 commit
  +'
  +
  +test_atom refs/heads/Branch1 objectname 
  32fca05e9f638021a123a84226acf17756acc18b
  +test_atom refs/heads/Branch2 objectname 
  194a5b89ac661a114566ba4374bc06c2797539f3

 Do these need to be Branch[12], not branch[12] for the code to
 exhibit the bug?  If not, please don't be creative in names like
 these.  On case corrupting filesystems you may write Branch1 and
 they may come back as branch1, but that is not what we are testing
 here.

Branches names can be lowercased. Only the commit messages should be
preserved as they produce the test depends on the lexicographical order
of created SHA1s.

  @@ -296,6 +325,8 @@ test_expect_success 'Check short refname format' '
   '

   cat expected EOF
  +
  +
   origin/master

 What are these blank line outputs?

The upstreams of Branch1 and Branch2.

   EOF

  @@ -309,7 +340,7 @@ cat expected EOF
   EOF

   test_expect_success 'Check short objectname format' '
  -   git for-each-ref --format=%(objectname:short) refs/heads actual 
  +   git for-each-ref --format=%(objectname:short) refs/heads/master 
  actual 
  test_cmp expected actual
   '

 All in all, I have to wonder if you can limit the updates to other
 unrelated tests if you added a new test near the end.  Also doesn't
 the existing test already create enough refs to let you sort with
 multiple keys and demonstrate the breakage already, without adding new
 refs and objects?

My intention was to group all tests to sort in one place. But if the
preferred place for a new one is at the end, then it is possible to find
the adequate refs among existing ones.

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