Two gitweb bugs related to javascript-actions

2014-07-22 Thread Robert Luberda
Hi

Some time ago I reported the two following bugs to Debian BTS,
could you please look at them?

  1. http://bugs.debian.org/741883 -- gitweb blame does not work
correctly when $feature{'javascript-actions'} is enabled

This should be one-line change fix, which I really would like to be
applied to the git package itself, not to do the same change over and
over again every time my gitweb package is updated on my system.

  2. https://bugs.debian.org/741884 feature{'javascript-actions'} breaks
external links

This might be more challenging, but the simplest approach would be avoid
adding those strange '[#?]js=1' parts to non-gitweb-generated links.

Thanks a lot,
robert
--
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: git filter-branch does not make tree replacements permanent

2013-04-11 Thread Robert Luberda

Hi,

The git filter-branch man page states:

  NOTE: This command honors .git/info/grafts and .git/refs/replace/. 
If you have any grafts or replacement refs defined, running 
this command will make them permanent.

However the command  does not seem to honor tree (or blob) objects
replacements. The bug can be reproduced (with both git 1.7.10 and 
1.8.2.1.342.gfa7285d) in the following simple steps:


1. Setup:
git init
for i in a b c d; do echo $i >> f; git add f; git commit -m "$i"; done
git diff HEAD~2..HEAD~1  # the output is non-empty
2. Add replacement for some tree object
git replace `git log --format=raw | sed -ne 's/^tree //p' | sed -n 2,3p 
| tac`
git diff HEAD~2..HEAD~1  # the output is now empty
3. Run filter-branch:
git filter-branch
4. Verify that unfortunatelly it did nothing:
git replace | xargs git replace -d
git diff HEAD~2..HEAD~1  # the output is still not empty



The following work-around works for me for tree objects replacements,
but I don't think it is suitable for inclusion in git sources. Most
probably git write-tree should be changed instead to take both the blob
and tree replacements into account.

 
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index ac2a005..68064f2 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -276,6 +276,16 @@ test $commits -eq 0 && die "Found nothing to rewrite"
 
 # Rewrite the commits
 
+write_tree_func()
+{
+ m="$(git write-tree "$@")"
+ if [ -e "$ORIG_GIT_DIR/refs/replace/$m" ] ; then
+   n="$(cat "$ORIG_GIT_DIR/refs/replace/$m")"
+   echo "Replaced tree $m with $n" >&2
+   m="$n"
+ fi
+ echo "$m"
+}
 git_filter_branch__commit_count=0
 while read commit parents; do
git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))
@@ -342,8 +352,8 @@ while read commit parents; do
sed -e '1,/^$/d' <../commit | \
eval "$filter_msg" > ../message ||
die "msg filter failed: $filter_msg"
-   workdir=$workdir @SHELL_PATH@ -c "$filter_commit" "git commit-tree" \
-   $(git write-tree) $parentstr < ../message > ../map/$commit ||
+   workdir=$workdir /bin/sh -c "$filter_commit" "git commit-tree" \
+   $(write_tree_func) $parentstr < ../message > ../map/$commit ||
die "could not write rewritten commit"
 done <../revs

Regards,
robert
 
--
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/RFC] git svn: don't introduce new paragraph for git-svn-id

2012-08-24 Thread Robert Luberda
Junio C Hamano wrote:
> Eric Wong  writes:
>>
>> I think having "svn" in "svn.trimsvnlog" twice is redundant and not ideal.
>> Perhaps just --trim-log and svn.trimlog?
> 
> Do we ever want to trim "our" log when relaying the Git commits back
> to subversion?  Having "svn" in "trimsvnlog" makes it clear that the
> logs from subversion side is getting trimmed.

`git commit' already trims the messages (except for removing the leading
whitespaces from the first non-whitespace-only line) and git svn doesn't
change that.

The new option affects the way the messages are imported from svn to
git, with one exception when the --add-author-from option of dcommit is
used (in which case it may skip adding an extra new line character
before the `From: ' line). For that reason --trim-svn-log might be a
better name.

Regards,
robert
--
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/RFC] git svn: optionally trim imported log messages

2012-08-24 Thread Robert Luberda
Junio C Hamano writes:

Hi,

Junio, for some reason I don't get mails from you, I've just discovered
your e-mails on gmane news list. Anyway many thanks for your comments,
I'll fix them and send updated patch next week.


>> +When committing to svn from git (as part of 'commit-diff', 'set-tree'
>> +or 'dcommit') and '--add-author-from' is in effect, a new line character
>> +is not added before the `From: ` line if the log message ends with
>> +a pseudo-headers section.
> 
> I think it would be saner to call them "trailers" to avoid
> confusion.

Thanks, I haven't got any idea how to call them, especially because
existing git documentation refers to them just by using the word `line',
e.g.:

 git-am.txt: Add a `Signed-off-by:` line to the commit message,
 git-cherry-pick.txt:Add Signed-off-by line at the end of the

(The "trailer" keyword appears once in SubmittingPatches and - in a bit
different meaning in technical/pack-format.txt)

> 
> I've seen S-o-b, Cc and Change-Id there, but does anybody actually
> put "From: " there?

Yes, `git svn dcommit --add-author-from' adds such a trailer to the  svn
log message, and then resets or rebases the git index against svn, so
that the message with the trailer appears in git.

> 
> There needs an explanation to the reader why this is an optional
> feature.

OK, I'll add some explanation. Basically it is optional, per Eric
request, for backward compatibility  to make it possible to work on a
centralized clone of svn repository by people using both old and new
versions of git svn.

> 
> The prerequisite mechanism is to allow some tests in an environment
> where they cannot be run (e.g. no SVN available on the platform);
> any code that uses set_prereq unconditionally looks extremely
> strange.  It is OK while the patch is in RFC stage under active
> debugging, but they would want to go away before the polishing is
> done.

OK, I used it merely for checking that the tests work on older version
of git svn, and I didn't break the backward compatibility by accident.
Will be dropped from next version of the patch.

> 
> What does En-El stand for?  We often see (but not in Git where we
> prefer LF for that purpose)
> 
>   NL='
> ' ;# newline
> 
> and using $NL to mean "empty" may be misleading to the readers.

NL means newline. The new line characters implicitly added after each
commit message line, that's why the value is empty. But, yes, this can
be misleading. I'd prefer to keep it short, so would EL (i.e.
`empty-line') be an acceptable name?

>> +N=$((N + 1)) &&
> 

Sorry, it was a typo, I meant to use $(($N + 1)). Thanks for catching this.

> 
>   next_N () {
>   N=$(($N + 1)) &&
> ...
>   }
> 
> (the above also has two style fixes).

Just to be sure: shall the `...' line start a new level of indentation
or is it a typo? (I guess that the two style fixes are just after the
function name.)


Thanks a lot,
robert
--
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/RFC] git svn: handle errors and concurrent commits in dcommit

2012-08-24 Thread Robert Luberda
Eric Wong wrote:

Hi,

> 
> Oops, I'll push the following out since Junio already merged your
> original:

I can see that you haven't pushed the change yet. Maybe it would be a
good idea to fix other style mistakes  (extra spaces after redirections,
lack of spaces after function names, `[' used instead of `test', etc) as
well? I can prepare a patch if you think it is a good idea.

Regards,
robert



--
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] t9164: More style fixes

2012-08-30 Thread Robert Luberda
Make sure t9164 conforms with the coding guidelines:
 - remove spaces after redirection operators;
 - insert spaces between function names and parentheses;
 - split `if ...; then' lines;
 - use `test' instead of `['.

Signed-off-by: Robert Luberda 
---
 t/t9164-git-svn-dcommit-concurrent.sh |   69 -
 1 file changed, 33 insertions(+), 36 deletions(-)

diff --git a/t/t9164-git-svn-dcommit-concurrent.sh 
b/t/t9164-git-svn-dcommit-concurrent.sh
index d8464d4..d75ebdc 100755
--- a/t/t9164-git-svn-dcommit-concurrent.sh
+++ b/t/t9164-git-svn-dcommit-concurrent.sh
@@ -12,16 +12,15 @@ test_expect_success 'setup svn repository' '
svn_cmd checkout "$svnrepo" work.svn &&
(
cd work.svn &&
-   echo >file && echo > auto_updated_file
+   echo >file && echo >auto_updated_file
svn_cmd add file auto_updated_file &&
svn_cmd commit -m "initial commit"
) &&
svn_cmd checkout "$svnrepo" work-auto-commits.svn
 '
 N=0
-next_N()
-{
-   N=$(( $N + 1 ))
+next_N () {
+   N=$(($N + 1))
 }
 
 # Setup SVN repository hooks to emulate SVN failures or concurrent commits
@@ -35,39 +34,39 @@ next_N()
 # the hook should be applied for (each time the hook is run, the given
 # number is decreased by one until it gets 0, in which case the hook
 # will execute its real action)
-setup_hook()
-{
+setup_hook () {
hook_type="$1"  # "pre-commit" or "post-commit"
skip_revs="$2"
-   [ "$hook_type" = "pre-commit" ] ||
-   [ "$hook_type" = "post-commit" ] ||
+   test "$hook_type" = "pre-commit"  ||
+   test "$hook_type" = "post-commit"  ||
{ echo "ERROR: invalid argument ($hook_type)" \
"passed to setup_hook" >&2 ; return 1; }
-   echo "cnt=$skip_revs" > "$hook_type-counter"
+   echo "cnt=$skip_revs" >"$hook_type-counter"
rm -f "$rawsvnrepo/hooks/"*-commit # drop previous hooks
hook="$rawsvnrepo/hooks/$hook_type"
-   cat > "$hook" <<- 'EOF1'
+   cat >"$hook" <<-'EOF1'
#!/bin/sh
set -e
cd "$1/.."  # "$1" is repository location
-   exec >> svn-hook.log 2>&1
+   exec >>svn-hook.log 2>&1
hook="$(basename "$0")"
echo "*** Executing $hook $@"
set -x
-   . ./$hook-counter
+   . "./$hook-counter"
cnt="$(($cnt - 1))"
-   echo "cnt=$cnt" > ./$hook-counter
-   [ "$cnt" = "0" ] || exit 0
+   echo "cnt=$cnt" >"./$hook-counter"
+   test "$cnt" = "0" || exit 0
 EOF1
-   if [ "$hook_type" = "pre-commit" ]; then
+   if test "$hook_type" = "pre-commit"
+   then
echo "echo 'commit disallowed' >&2; exit 1" >>"$hook"
else
echo "PATH=\"$PATH\"; export PATH" >>"$hook"
echo "svnconf=\"$svnconf\"" >>"$hook"
-   cat >>"$hook" <<- 'EOF2'
+   cat >>"$hook" <<-'EOF2'
cd work-auto-commits.svn
svn up --config-dir "$svnconf"
-   echo "$$" >> auto_updated_file
+   echo "$$" >>auto_updated_file
svn commit --config-dir "$svnconf" \
-m "auto-committing concurrent change"
exit 0
@@ -76,8 +75,7 @@ EOF2
chmod 755 "$hook"
 }
 
-check_contents()
-{
+check_contents () {
gitdir="$1"
(cd ../work.svn && svn_cmd up) &&
test_cmp file ../work.svn/file &&
@@ -89,7 +87,7 @@ test_expect_success 'check if post-commit hook creates a 
concurrent commit' '
(
cd work.svn &&
cp auto_updated_file au_file_saved &&
-   echo 1 >> file &&
+   echo 1 >>file &&
svn_cmd commit -m "changing file" &&
svn_cmd up &&

[PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id

2012-08-01 Thread Robert Luberda
While importing changes from SVN by `git svn fetch' strip any
white spaces from beginnings and endings of SVN commit messages
and skip adding a new line character before `git-svn-id:'
line in case the commit message ends with another pseudo-header
(like From:, Signed-off-by: or Change-Id:, etc.).

This patch allows one to use gerrit code review system on git-svn-managed
repositories. gerrit expects its `Change-Id:' header to appear in the
last paragraph of commit message and `git-svn-id:' following a new
line character was breaking this expectation.
---
 perl/Git/SVN.pm|5 +-
 t/t9122-git-svn-author.sh  |4 +-
 t/t9163-git-svn-import-messages.sh |  174 
 3 files changed, 180 insertions(+), 3 deletions(-)
 create mode 100755 t/t9163-git-svn-import-messages.sh

diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index b8b3474..bf22408 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1015,7 +1015,8 @@ sub do_git_commit {
print $msg_fh $log_entry->{log} or croak $!;
restore_commit_header_env($old_env);
unless ($self->no_metadata) {
-   print $msg_fh "\ngit-svn-id: $log_entry->{metadata}\n"
+   print $msg_fh "\n" unless $log_entry->{log} =~ 
m/\n\n([\w-]+:\s.*\n)+$/;
+   print $msg_fh "git-svn-id: $log_entry->{metadata}\n"
  or croak $!;
}
$msg_fh->flush == 0 or croak $!;
@@ -1803,6 +1804,8 @@ sub make_log_entry {
close $un or croak $!;
 
$log_entry{date} = parse_svn_date($log_entry{date});
+   $log_entry{log} =~ s/^\s*//;
+   $log_entry{log} =~ s/\s*$//;
$log_entry{log} .= "\n";
my $author = $log_entry{author} = check_author($log_entry{author});
my ($name, $email) = defined $::users{$author} ? @{$::users{$author}}
diff --git a/t/t9122-git-svn-author.sh b/t/t9122-git-svn-author.sh
index 30013b7..c1d55eb 100755
--- a/t/t9122-git-svn-author.sh
+++ b/t/t9122-git-svn-author.sh
@@ -68,8 +68,8 @@ test_expect_success 'interact with it via git svn' '
 
# Make sure there are no commit messages with excess blank lines
test $(grep "^ " actual.2 | wc -l) = 3 &&
-   test $(grep "^ " actual.3 | wc -l) = 5 &&
-   test $(grep "^ " actual.4 | wc -l) = 5 &&
+   test $(grep "^ " actual.3 | wc -l) = 4 &&
+   test $(grep "^ " actual.4 | wc -l) = 4 &&
 
# Make sure there are no svn commit messages with excess blank lines
(
diff --git a/t/t9163-git-svn-import-messages.sh 
b/t/t9163-git-svn-import-messages.sh
new file mode 100755
index 000..46b7c5b
--- /dev/null
+++ b/t/t9163-git-svn-import-messages.sh
@@ -0,0 +1,174 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Robert Luberda
+#
+
+test_description='git svn check log messages imported from svn'
+. ./lib-git-svn.sh
+
+get_file_contents()
+{
+   for line in "$@"; do
+   echo "$line"
+   done
+}
+
+svn_commit()
+{
+   N=`expr $N + 1`
+   get_file_contents "$@" > svn-message.$N;
+   (cd work.svn && echo "$N" >> file &&
+   svn_cmd commit -F ../svn-message.$N file)
+}
+
+git_svn_dcommit()
+{
+   N=`expr $N + 1`
+   get_file_contents "$@" > git-svn-message.$N;
+   (cd work.git && echo "$N" >> file &&
+   git commit -a -F ../git-svn-message.$N &&
+   git svn dcommit )
+}
+
+fetch_and_check()
+{
+   get_file_contents "$@" >expected.$N &&
+   echo "GIT-SVN-ID-LINE" >> expected.$N   &&
+   (cd work.git && git svn rebase) &&
+   (cd work.git && git show -s HEAD) | sed -ne '/^/,${
+   s/^//
+   s/^git-svn-id: .*$/GIT-SVN-ID-LINE/
+   p
+   }' > actual.$N  &&
+   test_cmp expected.$N actual.$N
+}
+
+
+test_expect_success 'setup svn & git repository' '
+   svn_cmd checkout "$svnrepo" work.svn &&
+   (
+   cd work.svn &&
+   echo >file
+   svn_cmd add file &&
+   svn_cmd commit -m "initial commit"
+   ) &&
+   git svn clone "$svnrepo" work.git
+'
+
+test_expect_success 'check empty line is added before git-svn-id' '
+   svn_commit "test message 1" &&
+   fetch_and_check "test message 1" \
+   ""
+'
+
+test_expect_success 'no empty line before git-svn-id if ends with 
pseudo-header' '
+   svn_commit "test message 2" \
+  

[PATCH/RFC] git svn: handle errors and concurrent commits in dcommit

2012-08-01 Thread Robert Luberda
base_cmd();
-   print STDERR "W: $d and ", $gs->refname,
-" differ, using @finish:\n",
-join("\n", @diff), "\n";
-   } else {
-   print "No changes between current HEAD and ",
- $gs->refname,
- "\nResetting to the latest ",
- $gs->refname, "\n";
-   @finish = qw/reset --mixed/;
-   }
-   command_noisy(@finish, $gs->refname);
+   my @diff = dcommit_rebase(@$linear_refs == 0, $d, 
$gs->refname, undef);
 
-   $rewritten_parent = command_oneline(qw/rev-parse HEAD/);
+   $rewritten_parent = command_oneline(qw/rev-parse/, 
$gs->refname);
 
if (@diff) {
+   $current_head = command_oneline(qw/rev-parse 
HEAD/);
@refs = ();
my ($url_, $rev_, $uuid_, $gs_) =
  working_head_info('HEAD', \@refs);
@@ -1019,6 +1050,7 @@ sub cmd_dcommit {
}
$parents = \%p;
$linear_refs = \@l;
+   undef $last_rev;
}
}
}
diff --git a/t/t9164-git-svn-dcommit-concrrent.sh 
b/t/t9164-git-svn-dcommit-concrrent.sh
new file mode 100755
index 000..7916a63
--- /dev/null
+++ b/t/t9164-git-svn-dcommit-concrrent.sh
@@ -0,0 +1,173 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Robert Luberda
+#
+
+test_description='concurrent git svn dcommit'
+. ./lib-git-svn.sh
+
+
+
+test_expect_success 'setup svn repository' '
+   svn_cmd checkout "$svnrepo" work.svn &&
+   (
+   cd work.svn &&
+   echo >file && echo > auto_updated_file
+   svn_cmd add file auto_updated_file &&
+   svn_cmd commit -m "initial commit"
+   ) &&
+   svn_cmd checkout "$svnrepo" work-auto-commits.svn
+'
+N=0
+next_N()
+{
+   N=`expr $N + 1`
+}
+
+# Setup SVN repository hooks to emulate SVN failures or concurrent commits
+# The function adds either
+# either pre-commit  hook, which causes SVN commit given in second argument to 
fail
+# or post-commit hook, which creates a new commit (a new line added to
+#auto_updated_file) after given SVN commit
+# The second argument contains a number (not SVN revision) of commit the the 
hook
+# should be applied for.
+setup_hook()
+{
+   hook_type="$1"  # pre-commit to fail commit or post-commit to make 
concurrent commit
+   skip_revs="$2"  # skip number of revisions before applying the hook
+   # the number is decremented by one each time hook is 
called until
+   # it gets 0, in which case the real part of hook is 
executed
+   [ "$hook_type" = "pre-commit" ] || [ "$hook_type" = "post-commit" ] ||
+   { echo "ERROR: invalid argument ($hook_type) passed to 
setup_hook" >&2 ; return 1; }
+   echo "cnt=$skip_revs" > "$hook_type-counter"
+   rm -f "$rawsvnrepo/hooks/"*-commit # drop previous hooks
+   hook="$rawsvnrepo/hooks/$hook_type"
+   cat > "$hook" <<- 'EOF1'
+   #!/bin/sh
+   set -e
+   cd "$1/.."  # "$1" is repository location
+   exec >> svn-hook.log 2>&1
+   hook="`basename "$0"`"
+   echo "*** Executing $hook $@"
+   set -x
+   . ./$hook-counter
+   cnt=`expr "$cnt" - 1` || [ $? = 1 ] # expr returns error code 1 
if expression is 0
+   echo "cnt=$cnt" > ./$hook-counter
+   [ "$cnt" = "0" ] || exit 0
+EOF1
+   if [ "$hook_type" = "pre-commit" ]; then
+   echo "echo 'commit disallowed' >&2; exit 1" >> "$hook"
+   else
+   echo "PATH=\"$PATH\"; export PATH" >> $hook
+   echo "svnconf=\"$svnconf\"" >> $hook
+   cat >> "$hook" <<- 'EOF2'
+   cd work-auto-commits.svn
+   svn up --config-dir "$svnconf"
+   echo "$

Re: [PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id

2012-08-01 Thread Robert Luberda
Eric Wong wrote:

Hi,

> 
> I've long wanted to change this, but it breaks compatibility if folks
> are importing from the same repo, sharing changes and one upgrades
> git-svn.

Yes, I'm aware of this. That's why in our team at work everybody is
forced to use the modified version of git-svn:)
[ A bit of background: we have recently started using git svn at after
our main repository had been migrated to svn. Previously git over
clearcase approach was used; we established some workflow and a part of
it was to use gerrit for code reviews, however it turned out to be
hardly possible with git svn. On the other hand the lack of error
handling was pretty annoying when changes get rejected by svn pre-commit
hook. That's why I wrote the two patches I sent today. ]

> How about making this optional and configurable at init/clone time?

I don't think it will be hard to make it configurable. I can try to make
such a change, do you have any preferences about the option and
configuration key names?

Thanks a lot,
robert
--
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/RFC] git svn: handle errors and concurrent commits in dcommit

2012-08-07 Thread Robert Luberda
Eric Wong wrote:

Hi,

> 
> A few minor comments inline...
> Please ensure all error messages and code are readable in
> 80-column terminals.
> Also, keep opening "{" on the same line as the if/unless.
> Backticks don't nest properly, nowadays, we prefer:
>   N=$(expr $N + 1)
>> +cp auto_updated_file auto_updated_file_saved
> Need "&&" to check for failure on cp
>> +sed -i 1d auto_updated_file && git commit -am "commit change 
>> $N.3" &&
> I don't believe "sed -i" is portable enough for this test.


Many thanks for the comments! I've fixed all of the above and will send
updated patch in next e-mail. Please let me know if you have any further
comments.



>> +echo "PATH=\"$PATH\"; export PATH" >> $hook
>> +echo "svnconf=\"$svnconf\"" >> $hook
>> +cat >> "$hook" <<- 'EOF2'
>> +cd work-auto-commits.svn
>> +svn up --config-dir "$svnconf"
> 
> That doesn't seem to interact well with users who depend on
> svn_cmd pointing to something non-standard.  Not sure
> what to do about it, though

I have no idea how to change it either. I've tried to source the
lib-git-svn.sh file inside the hook, but it sources test-lib.sh, and the
latter script doesn't work well if it is sourced by non-test script.
Anyway I the part of my original patch unchanged.

Regards,
robert
--
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/RFC] git svn: handle errors and concurrent commits in dcommit

2012-08-07 Thread Robert Luberda
27;);
-   my @finish;
-   if (@diff) {
-   @finish = rebase_cmd();
-   print STDERR "W: $d and ", $gs->refname,
-" differ, using @finish:\n",
-join("\n", @diff), "\n";
-   } else {
-   print "No changes between current HEAD and ",
- $gs->refname,
- "\nResetting to the latest ",
- $gs->refname, "\n";
-   @finish = qw/reset --mixed/;
-   }
-   command_noisy(@finish, $gs->refname);
+   my @diff = dcommit_rebase(@$linear_refs == 0, $d,
+   $gs->refname, undef);
 
-   $rewritten_parent = command_oneline(qw/rev-parse HEAD/);
+   $rewritten_parent = command_oneline(qw/rev-parse/,
+   $gs->refname);
 
if (@diff) {
+   $current_head = command_oneline(qw/rev-parse
+   HEAD/);
@refs = ();
my ($url_, $rev_, $uuid_, $gs_) =
  working_head_info('HEAD', \@refs);
@@ -1019,6 +1054,7 @@ sub cmd_dcommit {
}
$parents = \%p;
        $linear_refs = \@l;
+   undef $last_rev;
}
}
}
diff --git a/t/t9164-git-svn-dcommit-concrrent.sh 
b/t/t9164-git-svn-dcommit-concrrent.sh
new file mode 100755
index 000..aac2dda
--- /dev/null
+++ b/t/t9164-git-svn-dcommit-concrrent.sh
@@ -0,0 +1,216 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Robert Luberda
+#
+
+test_description='concurrent git svn dcommit'
+. ./lib-git-svn.sh
+
+
+
+test_expect_success 'setup svn repository' '
+   svn_cmd checkout "$svnrepo" work.svn &&
+   (
+   cd work.svn &&
+   echo >file && echo > auto_updated_file
+   svn_cmd add file auto_updated_file &&
+   svn_cmd commit -m "initial commit"
+   ) &&
+   svn_cmd checkout "$svnrepo" work-auto-commits.svn
+'
+N=0
+next_N()
+{
+   N=$(( $N + 1 ))
+}
+
+# Setup SVN repository hooks to emulate SVN failures or concurrent commits
+# The function adds
+# either pre-commit  hook, which causes SVN commit given in second argument
+#to fail
+# or post-commit hook, which creates a new commit (a new line added to
+#auto_updated_file) after given SVN commit
+# The first argument contains a type of the hook
+# The second argument contains a number (not SVN revision) of commit
+# the hook should be applied for (each time the hook is run, the given
+# number is decreased by one until it gets 0, in which case the hook
+# will execute its real action)
+setup_hook()
+{
+   hook_type="$1"  # "pre-commit" or "post-commit"
+   skip_revs="$2"
+   [ "$hook_type" = "pre-commit" ] ||
+   [ "$hook_type" = "post-commit" ] ||
+   { echo "ERROR: invalid argument ($hook_type)" \
+   "passed to setup_hook" >&2 ; return 1; }
+   echo "cnt=$skip_revs" > "$hook_type-counter"
+   rm -f "$rawsvnrepo/hooks/"*-commit # drop previous hooks
+   hook="$rawsvnrepo/hooks/$hook_type"
+   cat > "$hook" <<- 'EOF1'
+   #!/bin/sh
+   set -e
+   cd "$1/.."  # "$1" is repository location
+   exec >> svn-hook.log 2>&1
+   hook="$(basename "$0")"
+   echo "*** Executing $hook $@"
+   set -x
+   . ./$hook-counter
+   cnt="$(($cnt - 1))"
+   echo "cnt=$cnt" > ./$hook-counter
+   [ "$cnt" = "0" ] || exit 0
+EOF1
+   if [ "$hook_type" = "pre-commit" ]; then
+   echo "echo 'commit disallowed' >&2; exit 1" >> "$hook"
+   else
+   echo "PATH=\"$PATH\"; export PATH" >> $hook
+   echo "svnconf=\"$svnconf\"" >> $hoo

Re: [PATCH/RFC] git svn: don't introduce new paragraph for git-svn-id

2012-08-19 Thread Robert Luberda
Eric Wong wrote:

Hi,
>> I don't think it will be hard to make it configurable. I can try to make
>> such a change, do you have any preferences about the option and
>> configuration key names?
> 
> No preference off the top of my head.  As long as it makes sense to
> enough people here and is consistent in style with existing options in
> git.

I have been quite a busy recently, so it took me longer that I thought.

It was quite hard for me to think some sensible option name, and finally
have chosen --trim-svn-log (svn.trimsvnlog as config key name). Please
let me know if such name is ok for you. If not, I'll try to find a
different one (but as I wrote I'm not really good at giving names to
options/functions/variables, etc. :()

I considered making the option a default one for new git svn clones, so
that existing repositories would use the older approach, but I gave up
the idea, and implemented the simpler solution, in which the option must
be given explicitly if one needs the new behavior. If making it a
default for new clones would make sense for you, I can try to implement
this as well.

For consistency, the `--add-author-from' option was modified not to add
an extra new line before 'From: ' line when the newly introduced option
is in effect.

I'm sending a new patch in next e-mail, could you please look at it and
share any comments you might have? One thing I was not sure about is the
requirement, introduced in the change, of having a whitespace character
after a colon in pseudo-header lines
(e.g. `From:somebody ' won't be considered as a
pseudo-header) - is this consistent with a way git handles
headers/pseudo-headers?

Best regards,
robert
--
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/RFC] git svn: handle errors and concurrent commits in dcommit

2012-08-19 Thread Robert Luberda
Eric Wong wrote:

Hi,

> Junio C Hamano  wrote:
>> I should have asked this yesterday, but do you mean you want to have
>> your "maint" in the upcoming 1.7.12?  This does look like a useful
>> thing to do, but does not seem like a regression fix to me.
> 
> Yeah, I wasn't sure what to name it since my master is still carrying
> Michael's larger SVN 1.7 changes.   Perhaps I should've named my "maint"
> "for-git-master" in this case...


While working on my next patch, I've accidentally discovered that bash gives
the following errors in the test file introduced in my commit :

./t9164-git-svn-dcommit-concrrent.sh: line 65: $hook: ambiguous redirect
./t9164-git-svn-dcommit-concrrent.sh: line 66: $hook: ambiguous redirect

(The test succeeds, even though assignments of the PATH and svnconf
variables is missing; is seems svn treats an empty value of --config-dir
as the current dir.)


Sorry about not noticing this before, dash is used as default /bin/sh on
my system. A simple patch for this issue is included below.

There is also a typo in the test file name: `concrrent' instead of
`concurrent', but it most probably doesn't make any sense to make a
patch for it.


>8--->8--->8--->8--->8--->8--->8--->8--->8--->8--->8--->8--->8
Subject: [PATCH] Add missing quotes in test t9164

This fixes `ambiguous redirect' error given by bash.
---
 t/t9164-git-svn-dcommit-concrrent.sh |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9164-git-svn-dcommit-concrrent.sh
b/t/t9164-git-svn-dcommit-concrrent.sh
index aac2dda..676ef00 100755
--- a/t/t9164-git-svn-dcommit-concrrent.sh
+++ b/t/t9164-git-svn-dcommit-concrrent.sh
@@ -62,8 +62,8 @@ EOF1
if [ "$hook_type" = "pre-commit" ]; then
echo "echo 'commit disallowed' >&2; exit 1" >> "$hook"
else
-   echo "PATH=\"$PATH\"; export PATH" >> $hook
-   echo "svnconf=\"$svnconf\"" >> $hook
+   echo "PATH=\"$PATH\"; export PATH" >> "$hook"
+   echo "svnconf=\"$svnconf\"" >> "$hook"
cat >> "$hook" <<- 'EOF2'
cd work-auto-commits.svn
svn up --config-dir "$svnconf"
--
1.7.10.4


>8--->8--->8--->8--->8--->8--->8--->8--->8--->8--->8--->8--->8




Regards,
robert
--
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/RFC] git svn: optionally trim imported log messages

2012-08-19 Thread Robert Luberda
 enabled, and passed log entry ends with a pseudo-headers section (i.e.
+# section starting with empty line followed by one or more pseudo-headers
+# like 'From: ' or 'Change-Id: ' etc.)
+sub avoid_extra_new_line {
+   return 0 unless $_trim_svn_log;
+
+   my $log_entry = shift;
+   return ($log_entry =~ m/\n\n([\w-]+:\s.*\n)+$/);
+}
+
 sub do_git_commit {
my ($self, $log_entry) = @_;
my $lr = $self->last_rev;
@@ -1015,7 +1026,9 @@ sub do_git_commit {
print $msg_fh $log_entry->{log} or croak $!;
restore_commit_header_env($old_env);
unless ($self->no_metadata) {
-   print $msg_fh "\ngit-svn-id: $log_entry->{metadata}\n"
+   print $msg_fh "\n"
+   unless avoid_extra_new_line($log_entry->{log});
+   print $msg_fh "git-svn-id: $log_entry->{metadata}\n"
  or croak $!;
}
$msg_fh->flush == 0 or croak $!;
@@ -1821,6 +1834,8 @@ sub make_log_entry {
close $un or croak $!;
 
$log_entry{date} = parse_svn_date($log_entry{date});
+   $log_entry{log} =~ s/^\s*// if $_trim_svn_log;
+   $log_entry{log} =~ s/\s*$// if $_trim_svn_log;
$log_entry{log} .= "\n";
    my $author = $log_entry{author} = check_author($log_entry{author});
my ($name, $email) = defined $::users{$author} ? @{$::users{$author}}
diff --git a/t/t9165-git-svn-import-messages.sh 
b/t/t9165-git-svn-import-messages.sh
new file mode 100755
index 000..1ad4485
--- /dev/null
+++ b/t/t9165-git-svn-import-messages.sh
@@ -0,0 +1,387 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Robert Luberda
+#
+
+test_description='checks behaviour of --trim-svn-log option of git svn'
+. ./lib-git-svn.sh
+
+# The test cases in this file check if the --trim-svn-log option
+# of git svn works as expected.
+#
+# Basically the test cases use two git repositories:
+# * work-TRIM.git, created by git svn clone with the option enabled,
+# * work-NOTR.git, created with the option disabled.
+#
+# Each test case (except for the first two) does the following:
+# 1. commit a change to svn with either svn commit, or git svn dcommit
+#(what is important is the commit log message, not the changed file)
+# 2. git svn rebase the  work-NOTR.git repository and check its most recent
+#log message against some expected output
+# 3. git svn rebase the work-TRIM.git repository and similarly check the
+#latest log message
+#
+# The following two prerequisites are defined mostly for debugging purposes
+# to make it possible to skip test cases or parts of the test cases that
+# operate on one of the two git repositories. For example to ignore checking
+# of log messages imported when --trim-svn-log is enabled, one needs to comment
+# out the PRQ_TRIM pre-requisite (this makes it possible to run the test with
+# a version of git svn that does not support the option yet). Similarly
+# commenting out the PRQ_NOTR pre-requisite will check only the effects of the
+# svn log trimming option.
+# Please note that a whole test case must be marked as requiring one of
+# those pre-requisites if and only if it uses `git svn dcommit' for committing
+# changes to svn.
+test_set_prereq PRQ_TRIM
+test_set_prereq PRQ_NOTR
+
+N=0
+NL=""  # for better readability only, e.g.:
+   # "$NL" "   " "$NL" is cleaner than "" "   " ""
+
+next_N()
+{
+   N=$((N + 1)) &&
+   echo "N is $N"
+}
+
+# An utility function used for writing log messages to files
+get_file_contents()
+{
+   for line in "$@"; do
+   echo "$line"
+   done
+}
+
+# Commit to svn using 'svn commit'.
+# Usage:
+#  svn_commit commit_log_line ...
+# For example:
+#  svn_commit "message line 1" "message line 2"
+# will create a svn commit which log message is
+#  "message line 1\nmessage line 2\n\n"
+# Please note the two ending new line characters - the first one
+# is added by us and the second one is added by svn (which seems
+# to always adds a new line character on its own).
+svn_commit()
+{
+   get_file_contents "$@" > t${N}-svn-message &&
+   (
+   cd work.svn && svn_cmd up &&
+   echo "$N" >> file &&
+   svn_cmd commit -F ../t${N}-svn-message file
+   )
+}
+
+# Commit to svn using 'git svn dcommit' in either work-TRIM.git or
+# work-NOTR.git repository.
+# Usage:
+#  git_svn_dcm [ git_svn_dcommit_options ] TRIM | NOTR commit_log_line ...
+# Examples:
+#  git_svn_dcm TRIM "message line 1" "message line 2"
+#  git_svn_dcm NOTR "message line 1" "message line 2"
+#  git_svn_dcm --add-author-from TRIM "message line 1&qu