Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
On 14/05/14 09:24, Jens Lehmann wrote:
Since git 2.0.0 starting git gui in a submodule using a gitfile fails with
the following error:
No working directory ../../../path
couldn't change working directory
to ../../../path: no such file or
directory
This is because git rev-parse --show-toplevel is only run when git gui
sees a git version of at least 1.7.0 (which is the version in which the
--show-toplevel option was introduced). But it uses vsatisfies to check
that, which is documented to return false when the major version changes,
which is not what we want here.
Change vsatisfies to vcompare when testing for a git version to avoid the
problem when the major version changes (but still use vsatisfies in those
places where the Tk version is checked). This is done for both the place
that caused the reported bug and another spot where the git version is
tested for another feature.
Reported-by: Chris Packham judge.pack...@gmail.com
Reported-by: Yann Dirson ydir...@free.fr
Signed-off-by: Jens Lehmann jens.lehm...@web.de
---
Looks good to me (refer to previous comment about being rusty with tcl).
More importantly
Tested-by: Chris Packham judge.pack...@gmail.com
There is the alternative I posted which just does away with the version
checks (actually that only addressed one of the vcompare call sites).
There are also these checks that I guess are correct (if not
inconsistent) but again who is still using git 1.5.3?
git-gui.sh:1098:= 1.5.3 {
lib/blame.tcl:800: if {[git-version = 1.5.3]} {
lib/blame.tcl:849: if {[git-version = 1.5.3]} {
lib/checkout_op.tcl:513:= 1.5.3 {
Am 07.05.2014 09:49, schrieb Chris Packham:
On 07/05/14 19:28, Chris Packham wrote:
On 07/05/14 00:10, Pat Thoyts wrote:
Chris Packham judge.pack...@gmail.com writes:
On Tue, Apr 29, 2014 at 2:56 PM, Chris Packham judge.pack...@gmail.com
wrote:
Hi Pat,
I'm running git 2.0.0-rc0 (haven't got round to pulling down rc1 yet)
which includes gitgui-0.19.0 and I'm getting a new error when I run
'git gui' in a repository with a .git file (created by git submodule).
I can send you a screencap of the error message off list if you want
but the text is
No working directory ../../../repo
couldn't change working directory to ../../../repo: no such file or
directory
My tcl is a little rusty but I think the problem might be this snippet.
# v1.7.0 introduced --show-toplevel to return the canonical work-tree
if {[package vsatisfies $_git_version 1.7.0]} {
if { [is_Cygwin] } {
catch {set _gitworktree [exec cygpath --windows [git rev-parse
--show-toplevel]]}
} else {
set _gitworktree [git rev-parse --show-toplevel]
}
} else {
# try to set work tree from environment, core.worktree or use
# cdup to obtain a relative path to the top of the worktree. If
# run from the top, the ./ prefix ensures normalize expands pwd.
if {[catch { set _gitworktree $env(GIT_WORK_TREE) }]} {
set _gitworktree [get_config core.worktree]
if {$_gitworktree eq } {
set _gitworktree [file normalize ./[git rev-parse --show-cdup]]
}
}
}
The vsatisfies call probably doesn't handle '2.0.0.rc0' and the
fallback behaviour probably needs to normalise core.worktree
The _git_version variable has already been trimmed to remove such
suffixes so the version comparison here will be ok.
I don't think that's true 'git rev-parse --show-toplevel' does the right
thing - if it's run.
We'll the trimming works but vstatisfies doesn't
puts $_git_version
puts [package vsatisfies $_git_version 1.7.0]
2.0.0
0
Yup, looks like vsatisfies is doing just what it is supposed to (according
to http://www.astro.princeton.edu/~rhl/Tcl-Tk_docs/tcl/package.n.html):
package vsatisfies version1 version2
Returns 1 if scripts written for version2 will work unchanged
with version1 (i.e. version1 is equal to or greater than version2
and they both have the same major version number), 0 otherwise.
The bump in the major number from 1 to 2 makes vsatisfies assume that the
version is not compatible anymore, I believe we should have used vcompare
here and in another place.
git-gui.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index cf2209b..ed2418b 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1283,7 +1283,7 @@ load_config 0
apply_config
# v1.7.0 introduced --show-toplevel to return the canonical work-tree
-if {[package vsatisfies $_git_version 1.7.0]} {
+if {[package vcompare $_git_version 1.7.0]} {
if { [is_Cygwin] } {
catch {set _gitworktree [exec cygpath --windows [git rev-parse
--show-toplevel]]}
} else {
@@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {
close $fd
}
- if {[package vsatisfies $::_git_version 1.6.3]} {
+ if {[package
Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
Junio, I believe this issue needs to be fixed before 2.0 final. Otherwise
git gui will not work inside submodules anymore due to the major version
number change from 1 to 2. I'd like to hear Pat's opinion on this; even
though I think my patch is less risky (as it doesn't change behavior for
pre-2 versions), he might like Chris' proposal better.
Am 13.05.2014 23:24, schrieb Jens Lehmann:
Since git 2.0.0 starting git gui in a submodule using a gitfile fails with
the following error:
No working directory ../../../path
couldn't change working directory
to ../../../path: no such file or
directory
This is because git rev-parse --show-toplevel is only run when git gui
sees a git version of at least 1.7.0 (which is the version in which the
--show-toplevel option was introduced). But it uses vsatisfies to check
that, which is documented to return false when the major version changes,
which is not what we want here.
Change vsatisfies to vcompare when testing for a git version to avoid the
problem when the major version changes (but still use vsatisfies in those
places where the Tk version is checked). This is done for both the place
that caused the reported bug and another spot where the git version is
tested for another feature.
Reported-by: Chris Packham judge.pack...@gmail.com
Reported-by: Yann Dirson ydir...@free.fr
Signed-off-by: Jens Lehmann jens.lehm...@web.de
---
Am 07.05.2014 09:49, schrieb Chris Packham:
On 07/05/14 19:28, Chris Packham wrote:
On 07/05/14 00:10, Pat Thoyts wrote:
Chris Packham judge.pack...@gmail.com writes:
On Tue, Apr 29, 2014 at 2:56 PM, Chris Packham judge.pack...@gmail.com
wrote:
Hi Pat,
I'm running git 2.0.0-rc0 (haven't got round to pulling down rc1 yet)
which includes gitgui-0.19.0 and I'm getting a new error when I run
'git gui' in a repository with a .git file (created by git submodule).
I can send you a screencap of the error message off list if you want
but the text is
No working directory ../../../repo
couldn't change working directory to ../../../repo: no such file or
directory
My tcl is a little rusty but I think the problem might be this snippet.
# v1.7.0 introduced --show-toplevel to return the canonical work-tree
if {[package vsatisfies $_git_version 1.7.0]} {
if { [is_Cygwin] } {
catch {set _gitworktree [exec cygpath --windows [git rev-parse
--show-toplevel]]}
} else {
set _gitworktree [git rev-parse --show-toplevel]
}
} else {
# try to set work tree from environment, core.worktree or use
# cdup to obtain a relative path to the top of the worktree. If
# run from the top, the ./ prefix ensures normalize expands pwd.
if {[catch { set _gitworktree $env(GIT_WORK_TREE) }]} {
set _gitworktree [get_config core.worktree]
if {$_gitworktree eq } {
set _gitworktree [file normalize ./[git rev-parse --show-cdup]]
}
}
}
The vsatisfies call probably doesn't handle '2.0.0.rc0' and the
fallback behaviour probably needs to normalise core.worktree
The _git_version variable has already been trimmed to remove such
suffixes so the version comparison here will be ok.
I don't think that's true 'git rev-parse --show-toplevel' does the right
thing - if it's run.
We'll the trimming works but vstatisfies doesn't
puts $_git_version
puts [package vsatisfies $_git_version 1.7.0]
2.0.0
0
Yup, looks like vsatisfies is doing just what it is supposed to (according
to http://www.astro.princeton.edu/~rhl/Tcl-Tk_docs/tcl/package.n.html):
package vsatisfies version1 version2
Returns 1 if scripts written for version2 will work unchanged
with version1 (i.e. version1 is equal to or greater than version2
and they both have the same major version number), 0 otherwise.
The bump in the major number from 1 to 2 makes vsatisfies assume that the
version is not compatible anymore, I believe we should have used vcompare
here and in another place.
git-gui.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index cf2209b..ed2418b 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1283,7 +1283,7 @@ load_config 0
apply_config
# v1.7.0 introduced --show-toplevel to return the canonical work-tree
-if {[package vsatisfies $_git_version 1.7.0]} {
+if {[package vcompare $_git_version 1.7.0]} {
if { [is_Cygwin] } {
catch {set _gitworktree [exec cygpath --windows [git rev-parse
--show-toplevel]]}
} else {
@@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {
close $fd
}
- if {[package vsatisfies $::_git_version 1.6.3]} {
+ if {[package vcompare $::_git_version 1.6.3]} {
set ls_others [list --exclude-standard]
} else {
set ls_others [list --exclude-per-directory=.gitignore]
--
To unsubscribe from this list: send the line unsubscribe git in
the
Re: [GUILT v2 06/29] Fix the do_get_patch function.
On Tue, May 13, 2014 at 11:13 PM, Jeff Sipek jef...@josefsipek.net wrote:
On Tue, May 13, 2014 at 10:30:42PM +0200, Per Cederqvist wrote:
A patch file consists of:
(1) the description
(2) optional diffstat
(3) the patches
When extracting the patch, we only want part 3. The do_get_patch used
to give us part 2 and part 3. That made for instance this series of
operations fail if guilt.diffstat is true:
guilt push empty-1
guilt push empty-2
guilt pop
guilt fold empty-2
guilt pop
guilt push
I would probably include the actual error here. I got the following (using
patch names a b):
$ guilt pop
Now at a.
$ guilt fold b
error: No changes
$ guilt pop
All patches popped.
$ guilt pu
Applying patch..a
error: No changes
To force apply this patch, use 'guilt push -f'
A bit strange. I see that I made an error in the commands. It should be
guilt new empty-1 and guilt new empty-2. The updated example in the
commit message looks like this:
$ guilt new empty-1
$ guilt new empty-2
$ guilt pop
Now at empty-1
$ guilt fold empty-2
$ guilt pop
All patches popped.
$ guilt push
Applying patch..empty-1
fatal: unrecognized input
To force apply this patch, use 'guilt push -f'
The diff itself is good.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
Unless you object I'll add this signed-off line even though I'm
improving the commit message in preparation for version 3
of the patch series.
/ceder
Signed-off-by: Per Cederqvist ced...@opera.com
---
guilt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/guilt b/guilt
index 8701481..3fc524e 100755
--- a/guilt
+++ b/guilt
@@ -334,7 +334,7 @@ do_get_patch()
{
awk '
BEGIN{}
-/^(diff |---$|--- )/ {patch = 1}
+/^(diff |--- )/ {patch = 1}
patch == 1 {print $0}
END{}
' $1
--
1.8.3.1
--
I already backed up the [server] once, I can do it again.
- a sysadmin threatening to do more frequent backups
--
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: [GUILT v2 07/29] Added test cases for guilt fold.
On Tue, May 13, 2014 at 11:30 PM, Jeff Sipek jef...@josefsipek.net wrote:
On Tue, May 13, 2014 at 10:30:43PM +0200, Per Cederqvist wrote:
Test that we can combine any combination of patches with empty and
non-empty messages, both with and without guilt.diffstat. (All
patches are empty.)
Signed-off-by: Per Cederqvist ced...@opera.com
---
regression/t-035.out | 467
+++
regression/t-035.sh | 62 +++
2 files changed, 529 insertions(+)
create mode 100644 regression/t-035.out
create mode 100755 regression/t-035.sh
...
diff --git a/regression/t-035.sh b/regression/t-035.sh
new file mode 100755
index 000..e914b32
--- /dev/null
+++ b/regression/t-035.sh
@@ -0,0 +1,62 @@
+#!/bin/bash
+#
+# Test the fold code
+#
+
+source $REG_DIR/scaffold
+
+cmd setup_repo
+
+function fixup_time_info
+{
+ cmd guilt pop
+ touch -a -m -t $TOUCH_DATE .git/patches/master/$1
+ cmd guilt push
+}
+
+function empty_patch
+{
+ cmd guilt new empty$1
+ fixup_time_info empty$1
+}
+
+function nonempty_patch
+{
+ if [ $1 = -2 ]; then
+ msg=Another commit message.
+ else
+ msg=A commit message.
+ fi
+
+ cmd guilt new -f -s -m $msg nonempty$1
+ fixup_time_info nonempty$1
+}
+
+for using_diffstat in true false; do
+ cmd git config guilt.diffstat $using_diffstat
+ for patcha in empty nonempty; do
+ for patchb in empty nonempty; do
+
+ if [ $patcha = $patchb ]
+ then
I know that this is before patch 29, but ... style? ;)
Otherwise, looks good. I like this way better than the unrolled loop in v1
of this patch.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
It is hard to change a habit. :-)
I'll fix this and add your signed-off-by.
/ceder
+ suffixa=-1
+ suffixb=-2
+ else
+ suffixa=
+ suffixb=
+ fi
+
+ echo %% $patcha + $patchb (diffstat=$using_diffstat)
+ ${patcha}_patch $suffixa
+ ${patchb}_patch $suffixb
+ cmd guilt pop
+ cmd guilt fold $patchb$suffixb
+ fixup_time_info $patcha$suffixa
+ cmd list_files
+ cmd guilt pop
+ cmd guilt delete -f $patcha$suffixa
+ cmd list_files
+
+ done
+ done
+done
--
1.8.3.1
--
*NOTE: This message is ROT-13 encrypted twice for extra protection*
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] wincred: avoid overwriting configured variables
On Wed, May 14, 2014 at 10:45 AM, Stepan Kasal ka...@ucw.cz wrote: Hello kusma, On Tue, May 13, 2014 at 08:56:54AM +0200, Erik Faye-Lund wrote: --- a/contrib/credential/wincred/Makefile +++ b/contrib/credential/wincred/Makefile @@ -1,12 +1,12 @@ all: git-credential-wincred.exe -CC = gcc -RM = rm -f -CFLAGS = -O2 -Wall - -include ../../../config.mak.autogen -include ../../../config.mak +CC ?= gcc +RM ?= rm -f +CFLAGS ?= -O2 -Wall + prefix ?= /usr/local libexecdir ?= $(prefix)/libexec/git-core Yeah, looks good to me. thanks, but it looks you replied only to my personal mail. Was it intentional? No, sorry about that. Consider the patches Acked-by: Erik Faye-Lund kusmab...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT v2 16/29] Fix backslash handling when creating names of imported patches.
On Wed, May 14, 2014 at 12:09 AM, Jeff Sipek jef...@josefsipek.net wrote:
On Tue, May 13, 2014 at 10:30:52PM +0200, Per Cederqvist wrote:
The 'echo %s' construct sometimes processes escape sequences. (This
%s? Should this be $s?
Yes. Will fix that typo in v3 of the patch series.
/ceder
Otherwise, looks good.
happens, for instance, under Ubuntu 14.04 when /bin/sh is actually
dash.) We don't want that to happen when we are importing commits, so
use 'printf %s $s' instead.
(The -E option of bash that explicitly disables backslash expansion is
not portable; it is not supported by dash.)
Signed-off-by: Per Cederqvist ced...@opera.com
---
guilt-import-commit | 2 +-
regression/t-034.out | 14 +++---
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/guilt-import-commit b/guilt-import-commit
index 6260c56..45f2404 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -30,7 +30,7 @@ for rev in `git rev-list $rhash`; do
# Try to convert the first line of the commit message to a
# valid patch name.
- fname=`echo $s | sed -e s//and/g -e s/[ :]/_/g -e s,[/\\],-,g \
+ fname=`printf %s $s | sed -e s//and/g -e s/[ :]/_/g -e
s,[/\\],-,g \
-e s/['\\[{}]//g -e 's/]//g' -e 's/\*/-/g' \
-e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
-e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
diff --git a/regression/t-034.out b/regression/t-034.out
index 7bc9459..bda4399 100644
--- a/regression/t-034.out
+++ b/regression/t-034.out
@@ -236,7 +236,7 @@ Date: Mon Jan 1 00:00:00 2007 +
About to begin conversion...
Current head: 2a8b1889aa5066193bac978e6bf5073ffcfa6541
Converting 2a8b1889 as can-have-embedded-single-slashes
-Converting 0a46f8fa as backslash-isorbidden
+Converting 0a46f8fa as backslash-is-forbidden
Converting aedb74fd as x
Converting 30187ed0 as cannot@have@the@sequence@at-brace
Converting 106e8e5a as cannot_end_in_
@@ -300,7 +300,7 @@ Applying patch..cannot@have@the@sequence@at-brace.patch
Patch applied.
Applying patch..x.patch
Patch applied.
-Applying patch..backslash-isorbidden.patch
+Applying patch..backslash-is-forbidden.patch
Patch applied.
Applying patch..can-have-embedded-single-slashes.patch
Patch applied.
@@ -311,7 +311,7 @@ Date: Mon Jan 1 00:00:00 2007 +
Can/have/embedded/single/slashes
-commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141
(refs/patches/master/backslash-isorbidden.patch)
+commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141
(refs/patches/master/backslash-is-forbidden.patch)
Author: Author Name author@email
Date: Mon Jan 1 00:00:00 2007 +
@@ -518,8 +518,6 @@ d .git/patches/master
d .git/refs/patches
d .git/refs/patches/master
f 06beca7069b9e576bd431f65d13862ed5d3e2a0f
.git/patches/master/ctrlisforbidden.patch
-f 08267ec6783ea9d1adae55b275198f7594764ed0 .git/patches/master/series
-f 08267ec6783ea9d1adae55b275198f7594764ed0 .git/patches/master/status
f 09b7e9be44ae5ec3a4bb30f5ee9d4ebc2c042f64
.git/patches/master/two_consecutive_dots_(.)_is_forbidden.patch
f 0b971c9a17aeca2319c93d700ffd98acc2a93451
.git/patches/master/question-mark-is-forbidden.patch
f 2b8392f63d61efc12add554555adae30883993cc
.git/patches/master/cannot-end-in-slash-.patch
@@ -529,7 +527,7 @@ f 34e07c584032df137f19bdb66d93f316f00a5ac8
.git/patches/master/tildeisforbidden
f 49bab499826b63deb2bd704629d60c7268c57aee
.git/patches/master/the_sequence_-._is_forbidden.patch
f 5bcddb8ccb6e6e5e8a61e9e56cb2e0f70cbab2f5
.git/patches/master/cannot@have@the@sequence@at-brace.patch
f 637b982fe14a240de181ae63226b27e0c406b3dc
.git/patches/master/asterisk-is-forbidden.patch
-f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3
.git/patches/master/backslash-isorbidden.patch
+f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3
.git/patches/master/backslash-is-forbidden.patch
f 7b103c3c7ae298cd2334f6f49da48bae1424f77b
.git/patches/master/crisalsoforbidden.patch
f 9b810b8c63779c51d2e7f61ab59cd49835041563 .git/patches/master/x.patch
f a22958d9ae9976fd7b2b5a9d0bcd44bf7ad9b08a
.git/patches/master/caretisforbidden.patch
@@ -537,6 +535,8 @@ f ab325bf5a432937fc6f231d3e8a773a62d53952b
.git/patches/master/multiple-slashes
f cb9cffbd4465bddee266c20ccebd14eb687eaa89
.git/patches/master/delisforbidden.patch
f d0885a1a1fdee0fd1e4fedce3f7acd3100540bc4
.git/patches/master/openbracketisforbidden.patch
f d2903523fb66a346596eabbdd1bda4e52b266440
.git/patches/master/check-multiple-.-dots-.-foo.patch
+f da90de1c84138194524994e0bc3bc4ca8189c999 .git/patches/master/series
+f da90de1c84138194524994e0bc3bc4ca8189c999 .git/patches/master/status
f dfc11f76394059909671af036598c5fbe33001ba
.git/patches/master/space_is_forbidden.patch
f e47474c52d6c893f36d0457f885a6dd1267742bb
.git/patches/master/colon_is_forbidden.patch
f e7a5f8912592d9891e6159f5827c8b4f372cc406
Re: [PATCH 0/4] remote-hg: more improvements
It is *way* beyond the quality of any other tool in 'contrib/' and even some tools in the core, like 'git-request-pull' (which has known bugs), and probably even 'git-pt'. Junio, can you comment on this? I understand this probably doesn't really affect the issue at hand, but it'd help clarify if it's ever possible to move out of contrib/ nowadays. I was originally led to believe that its code quality was good enough, and that was why I merged the bottom three patches of the series even down to 'next' in the first place. But after seeing the Of course response that led to [*1*], which made me recall many patch-review interactions with him, I have started to have doubts. The code quality of Git that many projects have come to trust their code with is much more than just the code at each moment keeps working for the users as long as the original author is around. The maintainer of a port to the platform X may lose access to the platform after switching jobs, the maintainer of a bridge to the foreign system Y may stop needing to talk to the foreign system after completing the switch to Git. Anybody can be hit by a bus, get sick, or simply lose interest in his past creations. By reading git log contrib/remote-helpers and comparing it with the logs for the rest of the system, you would realize that the former would not lead to a quality discussion similar to the one that led to [*2*], which was only possible because the change was adequately described to allow anybody to understand the original issue the change was meant to solve. The commit that made the original change made it easy to ask a critical question: You are improving B but at the same time breaking A. Can we do better to help both A and B? And it allowed us to move forward without having to rob Peter to pay Paul. Granted, these contrib/ patches were applied with the understanding that contrib/ stuff can be substandard, because having anything is often better than having nothing, and you cannot go back in time to update the history to make these commits more useful to others who will come later. But I would be lying if I said that I would expect that things will suddenly improve and that the codebase will be maintained for the long haul in a healthy way the minute we move it out of contrib/ to the core. Especially after seeing [*1*], which is just one of the examples that illustrate that there clearly is no will to explain the changes to help others who come later to help maintaining the code. I'll take good care of the codebase, I've spend the time to make it better, Me, me, me, is not what the open source process is about. Thanks for the explanation. I think it underlines well the A) technical issues (quality commits) and the B) social issues (ability to communicate in a friendly way respond constructively), which we discovered are both *essential* for contributing to git. Neglate one or the other, and sooner or later people will refuse to collaborate with you, because it's just too much work to deal with the downsides. My hope is that for future events of this nature to be detected sooner and dealt with more rigidly, e.g by making it clear this or that behavior is not acceptable. I think part of the problem here is that we let the situation install itself, by accepting B-side issues just because A-side was ok, hopeful that B-side would solve itself. Philippe -- 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: [GUILT v2 25/29] guilt push now fails when there are no more patches to push.
On Tue, May 13, 2014 at 11:41 PM, Jeff Sipek jef...@josefsipek.net wrote: On Tue, May 13, 2014 at 10:31:01PM +0200, Per Cederqvist wrote: This makes it easier to script operations on the entire queue, for example run the test suite on each patch in the queue: guilt pop -a;while guilt push; do make test||break; done This brings guilt push in line with the push operation in Mercurial Queues (hg qpush), which fails when there are no patches to apply. Updated the test suite. guilt push -a still does not fail. (It successfully manages to ensure that all patches are pushed, even if it did not have to do anything to make it so.) Signed-off-by: Per Cederqvist ced...@opera.com --- guilt-push | 19 ++- regression/t-020.out | 89 regression/t-020.sh | 13 +++- 3 files changed, 113 insertions(+), 8 deletions(-) ... diff --git a/regression/t-020.sh b/regression/t-020.sh index 906aec6..0f9f85d 100755 --- a/regression/t-020.sh +++ b/regression/t-020.sh @@ -26,6 +26,17 @@ guilt series | while read n ; do done # +# pushing when there is nothing to push +# + +shouldfail guilt push +cmd guilt push -a + +cmd list_files + +cmd git log -p I don't particularly care for the git-log. Otherwise it looks good. Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net In my defense, I was just continuing the pattern already established in that file. Anyhow, I'll add a new commit to the patch series that removes all git log -p from all tests. (They are present in both t-020.sh and t-021.sh.) /ceder + +# # pop all # cmd guilt pop --all @@ -61,7 +72,7 @@ cmd guilt pop --all npatches=`guilt series | wc -l` for n in `_seq -2 $npatches`; do - if [ $n -ge 0 ]; then + if [ $n -gt 0 ]; then cmd guilt push -n $n else shouldfail guilt push -n $n -- 1.8.3.1 -- Evolution, n.: A hypothetical process whereby infinitely improbable events occur with alarming frequency, order arises from chaos, and no one is given credit. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] remote-hg: more improvements
Philippe Vaucher philippe.vauc...@gmail.com writes: Thanks for the explanation. I think it underlines well the A) technical issues (quality commits) and the B) social issues (ability to communicate in a friendly way respond constructively), which we discovered are both *essential* for contributing to git. I'm not entirely convinced of that: there is something akin to drop-dead gorgeous code: code that is so well done that it would not matter with regard to its maintenance whether or not its author dropped dead because it's both done well as well as documented in a manner where the original author could not offer significant additional help. Of course I am a major proponent of this view because of being myself somewhat differently endowed in the respective classes A and B, and so having at least some sort of exchange rate between the two can, however large the conversion fees, only benefit me... -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] remote-hg: more improvements
Thanks for the explanation. I think it underlines well the A) technical issues (quality commits) and the B) social issues (ability to communicate in a friendly way respond constructively), which we discovered are both *essential* for contributing to git. I'm not entirely convinced of that: there is something akin to drop-dead gorgeous code: code that is so well done that it would not matter with regard to its maintenance whether or not its author dropped dead because it's both done well as well as documented in a manner where the original author could not offer significant additional help. I think this only means that you can get away with B issues if A's quality is very very very high, which doens't happen very often. And I doubt that you will be able to get away with it for long anyway, at some point some mechanism will be put in place so the downsides of B aren't visible to everyone... for example with the patches being sent to one person only and this person relays it to the list while filtering B's issues. Philippe -- 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: [GUILT v2 00/29] Teach guilt import-commit how to create legal patch names, and more
On Tue, May 13, 2014 at 11:29 PM, Per Cederqvist ced...@opera.com wrote: On Tue, May 13, 2014 at 10:54 PM, Jeff Sipek jef...@josefsipek.net wrote: On Tue, May 13, 2014 at 04:45:47PM -0400, Theodore Ts'o wrote: On Tue, May 13, 2014 at 10:30:36PM +0200, Per Cederqvist wrote: ... - Changed behavior: by default, guilt no longer changes branch when you push a patch. You need to do git config guilt.reusebranch false to re-enable that. This patch sets the default value of guilt.reusebranch to true; it should in my opinion change to false a year or two after the next release. We've been living with the origin - guilt/origin branch change for a year already, and in fact, these days I've gotten used to the new behavior. Is it really worth it to change the default? So, at first I was skeptical about the branch name prefix change. I've used it for about a year now, and I love it. When I first read Per's idea to change the default to the old-style, I was a bit sad but I understand the motivation. I'm open to either mode being the default since it's easy enough for me to change it for me (thanks, ~/.gitconfig) but I think more people should benefit from the added safety against accidental git-push. (I also like being able to use guilt/master..master to get only the commits I care about.) Thoughts? I don't have a strong opinion on which the default value should be. The scenario where it matters, when you run multiple versions of guilt against the same directory, is probably very rare in practice. If it is mentioned in the release note that it can be changed if needed, that is probably good enough. /ceder I will change the default value to false in the next version of the patch series, unless there are objections. I plan to send it to the list Friday morning. /ceder -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] remote-hg: more improvements
Philippe Vaucher philippe.vauc...@gmail.com writes: Thanks for the explanation. I think it underlines well the A) technical issues (quality commits) and the B) social issues (ability to communicate in a friendly way respond constructively), which we discovered are both *essential* for contributing to git. I'm not entirely convinced of that: there is something akin to drop-dead gorgeous code: code that is so well done that it would not matter with regard to its maintenance whether or not its author dropped dead because it's both done well as well as documented in a manner where the original author could not offer significant additional help. I think this only means that you can get away with B issues if A's quality is very very very high, which doens't happen very often. I would not exactly say get away with B issues. It's like saying you can get away with looking like a sleazebag if you plan the time for a complete border search whenever traveling abroad. Of course that means that traveling into countries where complete border search might entail depriving you of your civic rights and locking you up for decades in a torture camp without due process is plainly not an option. But if you are honest: everybody has to be prepared for that. It's just less likely to occur in practice. Basically you have to write in a manner if a seedy stranger gave me that code on a street corner, I would have no problem checking it in. In practice, the shortcuts offering themselves through civil behavior and mutual trust get a lot more work done. But they _are_ a vector for social engineering. You have to admit that it seems pretty unlikely by now that Felipe is trying to sneak in some NSA-written code without arousing people's suspicions. -- David Kastrup -- 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: t5539 fails on ubuntu for v2.0.0-rc2
On Sat, May 10, 2014 at 10:02:59AM -0400, Jeff King wrote: On Fri, May 09, 2014 at 02:08:27PM -0700, Junio C Hamano wrote: 3. Just disable the http tests when run as root. I think I'd favor 3. But I'd like to be sure that being root is the problem. I agree with both the conclusion and the precondition. Here's the patch. The problem starts in v1.9.2, not in v2.0.0, so it's not technically a regression in this cycle. And we are awfully late in the -rc period. But it is just a change in the test script, and one that seems rather unlikely to produce unexpected side effects. I'll leave it you whether you want to queue it for v2.0.0, or for the next maint release. Hrm, sorry, I was wrong about this. I had thought the auto-network-testing had gone into v1.9.2, but it didn't. So this _is_ a potential regression in v2.0.0. It's still relatively minor, affecting only the test suite, so it can probably wait for post-v2.0.0 if you don't want to do another -rc. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Watchman support for git
On Wed, May 14, 2014 at 6:44 AM, David Turner dtur...@twopensource.com wrote: I'm not so happy that git now needs to link to libjansson.so and libwatchman.so. I would load libwatchman.so at runtime only when core.usewatchman is on, but this is more of personal taste. I assume you mean something with dlopen? I don't really like that because (a) nothing else in git works that way and (b) you would still need the libwatchman headers to build git (or the structs could be copied, but that is ugly). And because we don't link to libdl.so anyway, using dlopen means trading libjansson.so and libwatchman.so for libdl.so. And it's uglier. So forget what I wrote. With that in mind, I think you don't need to keep a fs cache on disk at all. All you need to store (in the index) is the clock value from watchman. After we parse the index, we perform a since query to get path names (and perhaps exists and mode for later). Then we set CE_VALID bit on entries that are _not_ in the query result. The remaining items will be lstat'd by git (see [1] and read-cache.c changes on the next few patches). Assuming the number of updated files is reasonably small, we won't be punished by lookup time. I considered this, but rejected it for a few reasons: 1. We still need to know about the untracked files. I guess we could do an index extension for just that, like your untracked cache. Yes. But consider that the number of untracked files is usually small (otherwise 'git status' would look very messy). And your fscache would need to store excluded file list too, which could be a lot bigger (one pair of .[ch] - one .o). _But_ yours would make 'git status --ignored' work. I don't consider that a major use case for optimization, but people may have different opinions. 2. That doesn't eliminate opendir/readdir, I think. Those are a major cost. I did not thoroughly review your patches from January/February, but I didn't notice anything in there about this part of dir.c. Also the cost of open(nonexistent .gitignore) to do ignore processing. Assuming untracked cache is in use, opendir/readdir is replaced with lstat. And cheap lstat can be solved with watchman without storing anything extra. I solve open(.gitignore) too by checking its stat data with the one in index. If matches, I reuse the version in tree. This does not necessarily make it cheaper, but it increases locality so it might be. _Modified_ .gitignore files have to go through open(.gitignore), but people don't update .gitignore often. 3. Changing a file in the index (e.g. git add) requires clearing the CE_VALID bit; this means that third-party libraries (e.g. jgit) that change the git repo need to understand this extension for correct operation. Maybe that's just the nature of extensions, but it's not something that my present patch set requires. I don't store CE_VALID on disk. Instead I store a new bit CE_WATCHED. Only after loading and validating against watchman that I turn CE_WATCHED to CE_VALID in memory. So JGit, libgit2 are not confused. I assume you won't change your mind about this. Which is fine to me. I will still try out my approach with your libwatchman though. Just curious about its performance and complexity, compared to your approach. A bit off topic, but msys fork has another fscache in compat/win32. If you could come up with a different name, maybe it'll be less confusing for them after merging. But this is not a big deal, as this fscache won't work on windows anyway. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Watchman support for git
One more thing I think we can solve with watchman is the racy timestamp issue (see 29e4d36 and 407c8eb). Basically if a file is updated within a second (and the system only supports timestamp granuarity down to a second) then git can't know if a file is changed by comparing stat data, so it compares content anyway. With watchman, I think we know that a file is updated or not and can disable racy test. How often this happens and whether it is worth supporting is something to think about. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] remote-hg: more improvements
Basically you have to write in a manner if a seedy stranger gave me that code on a street corner, I would have no problem checking it in. In practice, the shortcuts offering themselves through civil behavior and mutual trust get a lot more work done. My point was more that it's very hard to produce high quality commits without social interaction with others explaining what you missed, stuffs you overlooked, etc. And there B issues quickly isolate you. You have to admit that it seems pretty unlikely by now that Felipe is trying to sneak in some NSA-written code without arousing people's suspicions. Not sure where I implied otherwise... and the earlier metaphore about country borders doesn't make much sense to me. Anyway, I think we are speaking about different things. All I'm saying is that humans are social creatures and that thinking you can contribute to projects ran by humans without according a high importance to social behaviors (like Felipe thinks) is not possible. Threads like this are proof that while technical quality might be important for the short term, social behaviors is impossible to ignore in the long term. Both are very important to be an appreciated contributor, or to contribute at all in the long term. Philippe -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] remote-hg: more improvements
Philippe Vaucher philippe.vauc...@gmail.com writes: Basically you have to write in a manner if a seedy stranger gave me that code on a street corner, I would have no problem checking it in. In practice, the shortcuts offering themselves through civil behavior and mutual trust get a lot more work done. My point was more that it's very hard to produce high quality commits without social interaction with others explaining what you missed, stuffs you overlooked, etc. You are overgeneralizing. You are assuming that it's easier for everybody to interact with humans rather than the Tao of Computing. And there B issues quickly isolate you. Unless you were isolated to start with. Anyway, I think we are speaking about different things. All I'm saying is that humans are social creatures and that thinking you can contribute to projects ran by humans without according a high importance to social behaviors (like Felipe thinks) is not possible. You are assuming that according a high importance to social behaviors is all that it takes for anybody. Do you tell the beggar on the next street corner that according a high importance to earning millions would be all that would be necessary for him to afford anything to drink that he'd like? Threads like this are proof that while technical quality might be important for the short term, social behaviors is impossible to ignore in the long term. Not really. Both are very important to be an appreciated contributor, or to contribute at all in the long term. The one thing where social behavior comes in is noticing who tends to be running free software projects. There is the mythical scratching one's itch theory, but it does not fit the bill. Those people who invest enough time into a project's progress to make a fundamental difference tend to do it at the cost of not having any worthwhile amount of time left actually _using_ the product. People mainly working on music software create very little music themselves, people mainly working on text processing software do not write many texts themselves, people writing high-performance operating systems have very little use for high-performance operating systems themselves. All of this might have started at one time as scratching their own itch, but once their contributions become significant, it's almost always the itches of others they are scratching, and continue to scratch, feeling responsible for them due to the skills they have been not as much blessed or cursed but entrusted with. And programming and social skills tend to be packaged separately. So not everybody is gifted with being able to contribute _gracefully_. -- David Kastrup -- 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 01/17] contrib: remove outdated README
I have had patches and contributions rejected in the past, sometimes rudely. Same has happened to many others, if you contribute long enough, it is pretty much guaranteed that it will happen to you. Maintainer is wrong, or you are wrong, or someone is just having a bad day. This is not about a couple of patches I worked in a weekend being rejected. This is about the work I've been doing since the past two years almost like a full-time job dropped to the floor with no explanation at all. I started with the expectation that they were going to move to the core, because Junio said so, then he changed his mind and didn't want to explain his reasoning. It's not just a bad day. Here are two posts where Junio and Michael Haggerty explain the reasoning to you: - http://article.gmane.org/gmane.comp.version-control.git/248727 - http://article.gmane.org/gmane.comp.version-control.git/248693 Basically, in your case it boils down to your social manners. Despite the (good) work you did, many people think the community and git as a whole as more to loose by having to deal with your theatrics, especially since you try to take everyone hostage of your situations. No amount of arguing (calling it ad hominem etc) will change anything at this point, you have to accept that your social actions have a big part of responsibility in this. IMHO, you should change your behavior into a more respectful one and give people some time to discover you changed, otherwise it is innevitable that you'll just get banned/ignored by mostly everyone. We spent way too much energy dealing with these silly issues, please find a way to deal with it that doesn't annoy everyone and doens't affect the friendlyness of the mailing list. Philippe -- 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: [GUILT v2 16/29] Fix backslash handling when creating names of imported patches.
On Wed, May 14, 2014 at 10:56:18AM +0200, Per Cederqvist wrote:
On Wed, May 14, 2014 at 12:09 AM, Jeff Sipek jef...@josefsipek.net wrote:
On Tue, May 13, 2014 at 10:30:52PM +0200, Per Cederqvist wrote:
The 'echo %s' construct sometimes processes escape sequences. (This
%s? Should this be $s?
Yes. Will fix that typo in v3 of the patch series.
ok, Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
/ceder
Otherwise, looks good.
happens, for instance, under Ubuntu 14.04 when /bin/sh is actually
dash.) We don't want that to happen when we are importing commits, so
use 'printf %s $s' instead.
(The -E option of bash that explicitly disables backslash expansion is
not portable; it is not supported by dash.)
Signed-off-by: Per Cederqvist ced...@opera.com
---
guilt-import-commit | 2 +-
regression/t-034.out | 14 +++---
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/guilt-import-commit b/guilt-import-commit
index 6260c56..45f2404 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -30,7 +30,7 @@ for rev in `git rev-list $rhash`; do
# Try to convert the first line of the commit message to a
# valid patch name.
- fname=`echo $s | sed -e s//and/g -e s/[ :]/_/g -e s,[/\\],-,g
\
+ fname=`printf %s $s | sed -e s//and/g -e s/[ :]/_/g -e
s,[/\\],-,g \
-e s/['\\[{}]//g -e 's/]//g' -e 's/\*/-/g' \
-e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
-e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
diff --git a/regression/t-034.out b/regression/t-034.out
index 7bc9459..bda4399 100644
--- a/regression/t-034.out
+++ b/regression/t-034.out
@@ -236,7 +236,7 @@ Date: Mon Jan 1 00:00:00 2007 +
About to begin conversion...
Current head: 2a8b1889aa5066193bac978e6bf5073ffcfa6541
Converting 2a8b1889 as can-have-embedded-single-slashes
-Converting 0a46f8fa as backslash-isorbidden
+Converting 0a46f8fa as backslash-is-forbidden
Converting aedb74fd as x
Converting 30187ed0 as cannot@have@the@sequence@at-brace
Converting 106e8e5a as cannot_end_in_
@@ -300,7 +300,7 @@ Applying patch..cannot@have@the@sequence@at-brace.patch
Patch applied.
Applying patch..x.patch
Patch applied.
-Applying patch..backslash-isorbidden.patch
+Applying patch..backslash-is-forbidden.patch
Patch applied.
Applying patch..can-have-embedded-single-slashes.patch
Patch applied.
@@ -311,7 +311,7 @@ Date: Mon Jan 1 00:00:00 2007 +
Can/have/embedded/single/slashes
-commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141
(refs/patches/master/backslash-isorbidden.patch)
+commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141
(refs/patches/master/backslash-is-forbidden.patch)
Author: Author Name author@email
Date: Mon Jan 1 00:00:00 2007 +
@@ -518,8 +518,6 @@ d .git/patches/master
d .git/refs/patches
d .git/refs/patches/master
f 06beca7069b9e576bd431f65d13862ed5d3e2a0f
.git/patches/master/ctrlisforbidden.patch
-f 08267ec6783ea9d1adae55b275198f7594764ed0 .git/patches/master/series
-f 08267ec6783ea9d1adae55b275198f7594764ed0 .git/patches/master/status
f 09b7e9be44ae5ec3a4bb30f5ee9d4ebc2c042f64
.git/patches/master/two_consecutive_dots_(.)_is_forbidden.patch
f 0b971c9a17aeca2319c93d700ffd98acc2a93451
.git/patches/master/question-mark-is-forbidden.patch
f 2b8392f63d61efc12add554555adae30883993cc
.git/patches/master/cannot-end-in-slash-.patch
@@ -529,7 +527,7 @@ f 34e07c584032df137f19bdb66d93f316f00a5ac8
.git/patches/master/tildeisforbidden
f 49bab499826b63deb2bd704629d60c7268c57aee
.git/patches/master/the_sequence_-._is_forbidden.patch
f 5bcddb8ccb6e6e5e8a61e9e56cb2e0f70cbab2f5
.git/patches/master/cannot@have@the@sequence@at-brace.patch
f 637b982fe14a240de181ae63226b27e0c406b3dc
.git/patches/master/asterisk-is-forbidden.patch
-f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3
.git/patches/master/backslash-isorbidden.patch
+f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3
.git/patches/master/backslash-is-forbidden.patch
f 7b103c3c7ae298cd2334f6f49da48bae1424f77b
.git/patches/master/crisalsoforbidden.patch
f 9b810b8c63779c51d2e7f61ab59cd49835041563 .git/patches/master/x.patch
f a22958d9ae9976fd7b2b5a9d0bcd44bf7ad9b08a
.git/patches/master/caretisforbidden.patch
@@ -537,6 +535,8 @@ f ab325bf5a432937fc6f231d3e8a773a62d53952b
.git/patches/master/multiple-slashes
f cb9cffbd4465bddee266c20ccebd14eb687eaa89
.git/patches/master/delisforbidden.patch
f d0885a1a1fdee0fd1e4fedce3f7acd3100540bc4
.git/patches/master/openbracketisforbidden.patch
f d2903523fb66a346596eabbdd1bda4e52b266440
.git/patches/master/check-multiple-.-dots-.-foo.patch
+f da90de1c84138194524994e0bc3bc4ca8189c999 .git/patches/master/series
+f da90de1c84138194524994e0bc3bc4ca8189c999 .git/patches/master/status
f
Re: [GUILT v2 25/29] guilt push now fails when there are no more patches to push.
On Wed, May 14, 2014 at 11:27:19AM +0200, Per Cederqvist wrote: On Tue, May 13, 2014 at 11:41 PM, Jeff Sipek jef...@josefsipek.net wrote: On Tue, May 13, 2014 at 10:31:01PM +0200, Per Cederqvist wrote: This makes it easier to script operations on the entire queue, for example run the test suite on each patch in the queue: guilt pop -a;while guilt push; do make test||break; done This brings guilt push in line with the push operation in Mercurial Queues (hg qpush), which fails when there are no patches to apply. Updated the test suite. guilt push -a still does not fail. (It successfully manages to ensure that all patches are pushed, even if it did not have to do anything to make it so.) Signed-off-by: Per Cederqvist ced...@opera.com --- guilt-push | 19 ++- regression/t-020.out | 89 regression/t-020.sh | 13 +++- 3 files changed, 113 insertions(+), 8 deletions(-) ... diff --git a/regression/t-020.sh b/regression/t-020.sh index 906aec6..0f9f85d 100755 --- a/regression/t-020.sh +++ b/regression/t-020.sh @@ -26,6 +26,17 @@ guilt series | while read n ; do done # +# pushing when there is nothing to push +# + +shouldfail guilt push +cmd guilt push -a + +cmd list_files + +cmd git log -p I don't particularly care for the git-log. Otherwise it looks good. Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net In my defense, I was just continuing the pattern already established in that file. Anyhow, I'll add a new commit to the patch series that removes all git log -p from all tests. (They are present in both t-020.sh and t-021.sh.) Sounds good. Thanks, Jeff. /ceder + +# # pop all # cmd guilt pop --all @@ -61,7 +72,7 @@ cmd guilt pop --all npatches=`guilt series | wc -l` for n in `_seq -2 $npatches`; do - if [ $n -ge 0 ]; then + if [ $n -gt 0 ]; then cmd guilt push -n $n else shouldfail guilt push -n $n -- 1.8.3.1 -- Evolution, n.: A hypothetical process whereby infinitely improbable events occur with alarming frequency, order arises from chaos, and no one is given credit. -- UNIX is user-friendly ... it's just selective about who its friends are -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] remote-hg: more improvements
My point was more that it's very hard to produce high quality commits without social interaction with others explaining what you missed, stuffs you overlooked, etc. You are overgeneralizing. You are assuming that it's easier for everybody to interact with humans rather than the Tao of Computing. No, what I'm saying is that most non-trivial patches go through a review process involving human interactions. It is very hard to produce a very good non-trivial patch without going through human interactions, just because someone might ask a question about your patch. To be honest I'm not interested in fighting over this so let's agree to disagree. Anyway, I think we are speaking about different things. All I'm saying is that humans are social creatures and that thinking you can contribute to projects ran by humans without according a high importance to social behaviors (like Felipe thinks) is not possible. You are assuming that according a high importance to social behaviors is all that it takes for anybody. Do you tell the beggar on the next street corner that according a high importance to earning millions would be all that would be necessary for him to afford anything to drink that he'd like? I'm sorry that my words aren't clear enough for you to infer the point I'm trying to make. Let's try something simpler: what I was saying is that bad behavior will get you into trouble when contributing (and thus it's important to behave nicely), where Felipe usualy says bad behavior is irrelevant because only truth/quality is important. And programming and social skills tend to be packaged separately. So not everybody is gifted with being able to contribute _gracefully_. Yes it's unfortunate. The amount of talent in our societies that is wasted because of communication problems is probably quite high. I didn't find a way around being social in any human based community yet, but if you have an idea please share. The only way I can see working is that for someone to act as a mediator between the grumpy contributor and the community, but the role of this person is not very pleasant. That or maybe have merges done by some kind of robot with some AI about patch quality, but I doubt it is technically feasible yet. Philippe -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] remote-hg: more improvements
Philippe Vaucher philippe.vauc...@gmail.com writes: I'm sorry that my words aren't clear enough for you to infer the point I'm trying to make. Let's try something simpler: what I was saying is that bad behavior will get you into trouble when contributing (and thus it's important to behave nicely), where Felipe usualy says bad behavior is irrelevant because only truth/quality is important. Do you feel Felipe is in control of what you label bad behavior? Do you feel you are in control over how you react to his behavior? Yes it's unfortunate. The amount of talent in our societies that is wasted because of communication problems is probably quite high. I didn't find a way around being social in any human based community yet, but if you have an idea please share. being social as an isolated feat is self-contradictory. The question is how to function in a particular social context. Stock answers apply to stock behaviors and are obviously most efficient to apply. Yesterday my girl friend bought back a mare she had sold two years ago because its owner did not manage to get along with it. It's a temperamental animal that learns and performs amazingly well for its comparatively compact build. But it's highest in rank or else, and so in the end it got locked up in its stable box most of the time in order to avoid injuries to other horses. Now it's back here at the riding school, and there is little question that there will be some injuries before things settle down again even though most of the horses here know it already. The only way I can see working is that for someone to act as a mediator between the grumpy contributor and the community, but the role of this person is not very pleasant. That or maybe have merges done by some kind of robot with some AI about patch quality, but I doubt it is technically feasible yet. Well, humans are more complex. There are no sure-fire recipes even for working with horses: some of them here have their separate paddocks because things would not settle down, some of them have standard conflicts, there are occasional injuries. The most important standard recipe is to make sure that the areas accessible to multiple horses do not have dead ends small enough for one horse to be able to corner another. That's not really fabulous but still pretty essential. Also enough room all around, obviously. Now humans are often held in conditions that are not species-appropriate and lead to a buildup of tension. Try finding an undisturbed spot in a typical city suitable for devouring a bread roll you hunted down without getting other predators eyeing your prey. Almost impossible. It may be that distributed version control systems offer more possibilities for organizing cooperation in a manner leaving graceful escape paths when things don't work out. It's not what one want to have to rely on permanently but it may be worth thinking about ways to make consequences from difficulties less inevitable and/or grave. -- David Kastrup -- 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 v6 02/42] refs.c: allow passing NULL to ref_transaction_free
Thanks.
I changed the commit message.
On Tue, May 13, 2014 at 3:44 PM, Jonathan Nieder jrnie...@gmail.com wrote:
Ronnie Sahlberg wrote:
Allow ref_transaction_free to be called with NULL and in extension allow
ref_transaction_rollback to be called for a NULL transaction.
In extension = as a result?
Makes sense. It lets someone do the usual
struct ref_transaction *transaction;
int ret = 0;
if (something_fails()) {
ret = -1;
goto cleanup;
}
...
cleanup:
ref_transaction_free(transaction);
return ret;
just like you can already do with free().
This allows us to write code that will
if ( (!transaction ||
ref_transaction_update(...)) ||
(ref_transaction_commit(...) !(transaction = NULL)) {
ref_transaction_rollback(transaction);
...
}
Somewhere in the whitespace and parentheses I'm lost.
Is the idea that when ref_transaction_commit fails it will have
freed the transaction so we need not to roll back to prevent a
double free? I think it would be simpler for the caller to
unconditionally set transaction to NULL after calling
ref_transaction_commit in such a case to avoid use-after-free.
Even better if it is the caller's responsibility to free
the transaction. At any rate, it doesn't seem related to this
patch.
--- a/refs.c
+++ b/refs.c
@@ -3303,6 +3303,9 @@ static void ref_transaction_free(struct
ref_transaction *transaction)
{
int i;
+ if (!transaction)
+ return;
Except for the unclear commit message,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 03/42] refs.c: add a strbuf argument to ref_transaction_commit for error logging
Thanks.
Comment added and whitespace fixed.
On Tue, May 13, 2014 at 4:10 PM, Jonathan Nieder jrnie...@gmail.com wrote:
Ronnie Sahlberg wrote:
Add a strbuf argument to _commit so that we can pass an error string back to
the caller. So that we can do error logging from the caller instead of from
_commit.
Longer term plan is to first convert all callers to use onerr==QUIET_ON_ERR
and craft any log messages from the callers themselves and finally remove the
onerr argument completely.
Very nice.
[...]
+++ b/refs.c
[...]
@@ -3443,6 +3444,9 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-flags,
update-type, onerr);
if (!update-lock) {
+ if (err)
+ strbuf_addf(err ,Cannot lock the ref '%s'.,
+ update-refname);
Tiny nit: whitespace.
[...]
--- a/refs.h
+++ b/refs.h
@@ -268,9 +268,12 @@ void ref_transaction_delete(struct ref_transaction
*transaction,
* Commit all of the changes that have been queued in transaction, as
* atomically as possible. Return a nonzero value if there is a
* problem. The ref_transaction is freed by this function.
+ * If err is non-NULL we will add an error string to it to explain why
+ * the transaction failed.
Probably worth mentioning the error string doesn't end with a newline
so the caller knows how to use it.
With the whitespace fix and with or without the comment tweak,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
diff --git i/refs.c w/refs.c
index 64e8feb..2ca3169 100644
--- i/refs.c
+++ w/refs.c
@@ -3445,7 +3445,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-type, onerr);
if (!update-lock) {
if (err)
- strbuf_addf(err ,Cannot lock the ref '%s'.,
+ strbuf_addf(err, Cannot lock the ref '%s'.,
update-refname);
ret = 1;
goto cleanup;
diff --git i/refs.h w/refs.h
index ff87e14..d45212f 100644
--- i/refs.h
+++ w/refs.h
@@ -268,8 +268,8 @@ void ref_transaction_delete(struct ref_transaction
*transaction,
* Commit all of the changes that have been queued in transaction, as
* atomically as possible. Return a nonzero value if there is a
* problem. The ref_transaction is freed by this function.
- * If err is non-NULL we will add an error string to it to explain why
- * the transaction failed.
+ * If err is non-NULL we will append an error string (with no trailing
+ * newline) to it to explain why the transaction failed.
*/
int ref_transaction_commit(struct ref_transaction *transaction,
const char *msg, struct strbuf *err,
--
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 v6 04/42] refs.c: make ref_update_reject_duplicates take a strbuf argument for errors
Thanks. I updated the commit message to highlight that this means the error string can now be passed all the way back to the caller. On Tue, May 13, 2014 at 5:04 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: Make ref_update_reject_duplicates return any error that occurs through a new strbuf argument. Sensible. The caller-visible effect would be that now ref_transaction_commit() can pass back a helpful error message through its err argument when asked to make multiple updates for the same ref. Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Please pull the patch series use the $( ... ) construct for command substitution
The following changes since commit 6308767f0bb58116cb405e1f4f77f5dfc1589920: Merge branch 'fc/prompt-zsh-read-from-file' (2014-05-13 11:53:14 -0700) are available in the git repository at: https://github.com/devzero2000/git-core.git ep/shell-command-substitution-v4 for you to fetch changes up to 8c8883150c391fae33c18af937594142600e: test-lib-functions.sh: use the $( ... ) construct for command substitution (2014-05-14 05:34:52 -0700) I have re-created a branch with these patches based on a previous observation made here http://www.spinics.net/lists/git/msg230236.html Thanks very much to the people (Matthieu i think) that will make the reviews, I understand it's boring Elia Pinto (83): t5003-archive-zip.sh: use the $( ... ) construct for command substitution t5517-push-mirror.sh: use the $( ... ) construct for command substitution t6002-rev-list-bisect.sh: use the $( ... ) construct for command substitution t7700-repack.sh: use the $( ... ) construct for command substitution t5100-mailinfo.sh: use the $( ... ) construct for command substitution t5520-pull.sh: use the $( ... ) construct for command substitution t6015-rev-list-show-all-parents.sh: use the $( ... ) construct for command substitution t8003-blame-corner-cases.sh: use the $( ... ) construct for command substitution t5300-pack-object.sh: use the $( ... ) construct for command substitution t5522-pull-symlink.sh: use the $( ... ) construct for command substitution t6032-merge-large-rename.sh: use the $( ... ) construct for command substitution t5301-sliding-window.sh: use the $( ... ) construct for command substitution t5530-upload-pack-error.sh: use the $( ... ) construct for command substitution t6034-merge-rename-nocruft.sh: use the $( ... ) construct for command substitution t9100-git-svn-basic.sh: use the $( ... ) construct for command substitution t5302-pack-index.sh: use the $( ... ) construct for command substitution t5537-fetch-shallow.sh: use the $( ... ) construct for command substitution t6132-pathspec-exclude.sh: use the $( ... ) construct for command substitution t9101-git-svn-props.sh: use the $( ... ) construct for command substitution t5303-pack-corruption-resilience.sh: use the $( ... ) construct for command substitution t5538-push-shallow.sh: use the $( ... ) construct for command substitution t7001-mv.sh: use the $( ... ) construct for command substitution t9104-git-svn-follow-parent.sh: use the $( ... ) construct for command substitution t5304-prune.sh: use the $( ... ) construct for command substitution t5550-http-fetch-dumb.sh: use the $( ... ) construct for command substitution t7003-filter-branch.sh: use the $( ... ) construct for command substitution t9105-git-svn-commit-diff.sh: use the $( ... ) construct for command substitution t5305-include-tag.sh: use the $( ... ) construct for command substitution t5551-http-fetch-smart.sh: use the $( ... ) construct for command substitution t7004-tag.sh: use the $( ... ) construct for command substitution t9107-git-svn-migrate.sh: use the $( ... ) construct for command substitution t5500-fetch-pack.sh: use the $( ... ) construct for command substitution t5570-git-daemon.sh: use the $( ... ) construct for command substitution t7006-pager.sh: use the $( ... ) construct for command substitution t9108-git-svn-glob.sh: use the $( ... ) construct for command substitution t5505-remote.sh: use the $( ... ) construct for command substitution t5601-clone.sh: use the $( ... ) construct for command substitution t7103-reset-bare.sh: use the $( ... ) construct for command substitution t9109-git-svn-multi-glob.sh: use the $( ... ) construct for command substitution t5506-remote-groups.sh: use the $( ... ) construct for command substitution t5700-clone-reference.sh: use the $( ... ) construct for command substitution t7406-submodule-update.sh: use the $( ... ) construct for command substitution t9110-git-svn-use-svm-props.sh: use the $( ... ) construct for command substitution t5510-fetch.sh: use the $( ... ) construct for command substitution t5710-info-alternate.sh: use the $( ... ) construct for command substitution t7408-submodule-reference.sh: use the $( ... ) construct for command substitution t9114-git-svn-dcommit-merge.sh: use the $( ... ) construct for command substitution t5515-fetch-merge-logic.sh: use the $( ... ) construct for command substitution t5900-repo-selection.sh: use the $( ... ) construct for command substitution t7504-commit-msg-hook.sh: use the $( ... ) construct for command
[PATCH] grep -I: do not bother to read known-binary files
From: Johannes Schindelin johannes.schinde...@gmx.de
Date: Mon, 8 Nov 2010 16:10:43 +0100
Incidentally, this makes grep -I respect the binary attribute (actually,
the -text attribute, but binary implies that).
Since the attributes are not thread-safe, we now need to switch off
threading if -I was passed.
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
Hi,
this patch has been in msysgit for 3.5 years.
Stepan
builtin/grep.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/builtin/grep.c b/builtin/grep.c
index 43af5b7..8073fbe 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -18,6 +18,7 @@
#include quote.h
#include dir.h
#include pathspec.h
+#include attr.h
static char const * const grep_usage[] = {
N_(git grep [options] [-e] pattern [rev...] [[--] path...]),
@@ -163,6 +164,22 @@ static void work_done(struct work_item *w)
grep_unlock();
}
+static int skip_binary(struct grep_opt *opt, const char *filename)
+{
+ if ((opt-binary GREP_BINARY_NOMATCH)) {
+ static struct git_attr *attr_text;
+ struct git_attr_check check;
+
+ if (!attr_text)
+ attr_text = git_attr(text);
+ memset(check, 0, sizeof(check));
+ check.attr = attr_text;
+ return !git_check_attr(filename, 1, check)
+ ATTR_FALSE(check.value);
+ }
+ return 0;
+}
+
static void *run(void *arg)
{
int hit = 0;
@@ -173,6 +190,9 @@ static void *run(void *arg)
if (!w)
break;
+ if (skip_binary(opt, (const char *)w-source.identifier))
+ continue;
+
opt-output_priv = w;
hit |= grep_source(opt, w-source);
grep_source_clear_data(w-source);
@@ -379,6 +399,9 @@ static int grep_cache(struct grep_opt *opt, const struct
pathspec *pathspec, int
continue;
if (!ce_path_match(ce, pathspec, NULL))
continue;
+ if (skip_binary(opt, ce-name))
+ continue;
+
/*
* If CE_VALID is on, we assume worktree file and its cache
entry
* are identical, even if worktree file has been modified, so
use
@@ -803,6 +826,8 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
string_list_append(path_list, show_in_pager);
use_threads = 0;
}
+ if ((opt.binary GREP_BINARY_NOMATCH))
+ use_threads = 0;
if (!opt.pattern_list)
die(_(no pattern given.));
--
1.9.2.msysgit.0.335.gd2a461f
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option
From: Johannes Schindelin johannes.schinde...@gmx.de
Date: Tue, 8 Feb 2011 00:17:24 -0600
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
Hi,
this patch has been in msysgit for 3 1/4 years.
Stepan
builtin/grep.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/builtin/grep.c b/builtin/grep.c
index 8073fbe..6c82a29 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -897,6 +897,9 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
if (len 4 is_dir_sep(pager[len - 5]))
pager += len - 4;
+ if (opt.ignore_case !strcmp(less, pager))
+ string_list_append(path_list, -i);
+
if (!strcmp(less, pager) || !strcmp(vi, pager)) {
struct strbuf buf = STRBUF_INIT;
strbuf_addf(buf, +/%s%s,
--
1.9.2.msysgit.0.335.gd2a461f
--
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: [GUILT v2 28/29] Added guilt.reusebranch configuration option.
On Tue, May 13, 2014 at 10:31:04PM +0200, Per Cederqvist wrote:
When the option is true (the default), Guilt does not create a new Git
branch when patches are applied. This way, you can switch between
Guilt 0.35 and the current version of Guilt with no issues.
At a future time, maybe a year after Guilt with guilt.reusebranch
support is released, the default should be changed to false to take
advantage of the ability to use a separate Git branch when patches are
applied.
Signed-off-by: Per Cederqvist ced...@opera.com
---
guilt| 28 +++-
regression/scaffold | 1 +
regression/t-062.out | 441
+++
regression/t-062.sh | 137
4 files changed, 601 insertions(+), 6 deletions(-)
create mode 100644 regression/t-062.out
create mode 100755 regression/t-062.sh
...
diff --git a/guilt b/guilt
index 9947acc..7c830eb 100755
--- a/guilt
+++ b/guilt
...
@@ -928,13 +935,22 @@ else
die Unsupported operating system: $UNAME_S
fi
-if [ $branch = $raw_git_branch ] [ -n `get_top 2/dev/null` ]
-then
-# This is for compat with old repositories that still have a
-# pushed patch without the new-style branch prefix.
-old_style_prefix=true
+if [ -n `get_top 2/dev/null` ]; then
+ # If there is at least one pushed patch, we set
+ # old_style_prefix according to how it was pushed. It is only
+ # possible to change the prefix style while no patches are
+ # applied.
+ if [ $branch = $raw_git_branch ]; then
+ old_style_prefix=true
+ else
+ old_style_prefix=false
+ fi
else
-old_style_prefix=false
+ if $reuse_branch; then
+ old_style_prefix=true
+ else
+ old_style_prefix=false
+ fi
I don't know if this is a good idea or not, but:
old_style_prefix=$reuse_branch
fi
_main $@
diff --git a/regression/scaffold b/regression/scaffold
index e4d7487..e4d2f35 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -93,6 +93,7 @@ function setup_git_repo
git config log.date default
git config log.decorate no
git config guilt.diffstat false
+ git config guilt.reusebranch false
}
function setup_guilt_repo
...
diff --git a/regression/t-062.sh b/regression/t-062.sh
new file mode 100755
index 000..85596ca
--- /dev/null
+++ b/regression/t-062.sh
@@ -0,0 +1,137 @@
...
+function fixup_time_info
+{
+ touch -a -m -t $TOUCH_DATE .git/patches/master/$1
+}
+
+cmd setup_repo
+
+cmd git config guilt.reusebranch true
+
+cmd guilt push -a
+cmd list_files
+cmd git for-each-ref
+
+cmd git for-each-ref
+
+cmd list_files
duplicate list_files for-each-ref
+
+for i in `seq 5`; do
+ if [ $i -ge 5 ]; then
+ shouldfail guilt pop
+ else
+ cmd guilt pop
+ fi
+ cmd git for-each-ref
+ cmd guilt push
+ cmd git for-each-ref
+ cmd guilt pop
+ cmd git for-each-ref
+done
+
+# Check that pop -a does the right thing.
What exactly is the right thing? no-op since the above loop poped
everything? (I'd make the comment say what the right thing is.)
Jeff.
--
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like
that.
- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] remote-hg: more improvements
I'm sorry that my words aren't clear enough for you to infer the point I'm trying to make. Let's try something simpler: what I was saying is that bad behavior will get you into trouble when contributing (and thus it's important to behave nicely), where Felipe usualy says bad behavior is irrelevant because only truth/quality is important. Do you feel Felipe is in control of what you label bad behavior? Do you feel you are in control over how you react to his behavior? I feel that Felipe cannot control this (or decided not to), and I feel that I am pretty much in control of my reactions given I am able to not give in to the burst of emotions that some of his behavior can trigger. For the record I'm not really blaming Felipe for what he does, I'm saying a solution has to be found to prevent his behavior from harming people, and that a solution that involves everyone adapting to Felipe is not reasonable. Yesterday my girl friend bought back a mare she had sold two years ago because its owner did not manage to get along with it. It's a temperamental animal that learns and performs amazingly well for its comparatively compact build. But it's highest in rank or else, and so in the end it got locked up in its stable box most of the time in order to avoid injuries to other horses. Now it's back here at the riding school, and there is little question that there will be some injuries before things settle down again even though most of the horses here know it already. I think that is the point: behave properly or be isolated to avoid harming others. Wether you control your behavior or not has little to do with it, it's your behavior that counts. That's how it works in pretty much any communities I know of. It may be that distributed version control systems offer more possibilities for organizing cooperation in a manner leaving graceful escape paths when things don't work out. It's not what one want to have to rely on permanently but it may be worth thinking about ways to make consequences from difficulties less inevitable and/or grave. Sure, I believe my proposal of acting on bad behavior earlier would prevent incidents like this one, because it would defuse situations before they settle in. But that's just a proposal, I'm just an observer here. Philippe -- 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] contrib: remote-helpers: add move warnings (v2.0)
On Tue, May 13, 2014 at 4:37 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Ronnie Sahlberg wrote: Could you please calm down and adjust your behavior. This constant hostility and rudeness makes the mailing list very unpleasant. I explaind that to him multiple times. In the mail I replied to he is once again assuming I'm a *insert-your-favorite-non-smart-adjective*, and explaining to me what a regression is. How many times must one repeat something before one is entitled so thay something is not getting through the skull of another person? 5? 10? 20? But fine, let's assume I do have to adjust my behavior. Maybe I should have said it doesn't register in your brain, or just it fails to grab your attention. But if I have to adjust for saying that (which was true), what do you say to Junio for saying this? (which was not) I know I shouldn't but I will respond anyway. The problem is about you and your behaviour. Not Junio or anyone else. Please adjust and stop your constant hostile, confrontational and abusive behavior on this mailinglist. Your behavior is not a winning strategy and will only result in alienating you and impair your interactions with the rest of the git community. If anything, Junio have shown an amazing amount of patience and restraint with you. I would not have tolerated your kind of behavior in any project I am maintainer for. Stop this idiocy. I presume nothing, because Junio is a riskier target. Please follow Junio's advice. It will benefit both you as well as all other participants in the community. -- 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: [GUILT v2 08/29] Added more test cases for guilt new: empty patches.
On Tue, May 13, 2014 at 10:30:44PM +0200, Per Cederqvist wrote:
Test that empty patches are handled correctly, both with and without
the guilt.diffstat configuration option.
Signed-off-by: Per Cederqvist ced...@opera.com
---
regression/t-020.out | 250
+++
regression/t-020.sh | 60 +
2 files changed, 310 insertions(+)
diff --git a/regression/t-020.out b/regression/t-020.out
index af45734..7e07efa 100644
--- a/regression/t-020.out
+++ b/regression/t-020.out
@@ -1128,3 +1128,253 @@ f 9c18cc7abe6b87f18503714a80a677b4094eb457
.git/patches/master/add
...
+% git log -p
+commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1
+Author: Author Name author@email
+Date: Mon Jan 1 00:00:00 2007 +
+
+patch empty.patch
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date: Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
b/def
+@@ -0,0 +1 @@
++abc
+% git config guilt.diffstat false
+---
+
I'm a bit confused about the above. It looks like contents of an empty
patch with an empty diffstat. But the only time I see a cat in the .sh file
is when you rewrite... oh I got it. I'll comment about it by the 'cat'.
...
diff --git a/regression/t-020.sh b/regression/t-020.sh
index cdd08ba..906aec6 100755
--- a/regression/t-020.sh
+++ b/regression/t-020.sh
@@ -5,6 +5,13 @@
source $REG_DIR/scaffold
+function fixup_time_info
+{
+ cmd guilt pop
+ touch -a -m -t $TOUCH_DATE .git/patches/master/$1
+ cmd guilt push
+}
+
cmd setup_repo
#
@@ -69,6 +76,59 @@ done
cmd list_files
+# push an empty patch with no commit message
+cmd guilt new empty.patch
+fixup_time_info empty.patch
+cmd list_files
+cmd git log -p
+
+# Ensure we can push the empty patch even when guilt.diffstat is true.
+cmd git config guilt.diffstat true
+cmd guilt refresh
+fixup_time_info empty.patch
+cmd list_files
+cmd git log -p
+cmd git config guilt.diffstat false
+
+# Let the patch have a commit message, but no data.
+cat .git/patches/master/empty.patch EOF
cat .git/.../empty.patch EOF
...
EOF
Otherwise, you'll just cat the existing patch and that's it.
+Fix a bug.
+
+From: Per Cederqvist ce...@lysator.liu.se
+
+This commit fixes a serious bug.
+
+FIXME:
+- add a test case
+- track down the bug
+- actually fix it
+EOF
+
+fixup_time_info empty.patch
+cmd list_files
+cmd git log -p
+
+# And once more, with an empty diffstat.
+
+cmd git config guilt.diffstat true
+cmd guilt refresh
+fixup_time_info empty.patch
+cmd list_files
+cmd git log -p
+
+# Restore the diffstat setting and remove the empty patch.
+cmd git config guilt.diffstat false
+cmd guilt refresh
+fixup_time_info empty.patch
+cmd list_files
+cmd git log -p
+# (Cannot delete an applied patch)
+shouldfail guilt delete empty.patch
+cmd guilt pop -a
+cmd guilt delete -f empty.patch
+cmd list_files
+cmd git log -p
+
# FIXME:
# --all
# -a
--
1.8.3.1
--
Fact: 23.6% of all statistics are generated randomly.
--
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: Please pull the patch series use the $( ... ) construct for command substitution
Elia Pinto gitter.spi...@gmail.com writes: The following changes since commit 6308767f0bb58116cb405e1f4f77f5dfc1589920: Merge branch 'fc/prompt-zsh-read-from-file' (2014-05-13 11:53:14 -0700) are available in the git repository at: https://github.com/devzero2000/git-core.git ep/shell-command-substitution-v4 There's a mis-replacement of multiple `..` `..` on the same line in t9300-fast-import.sh. I've sent you a pull request with a fixup. I'm not sure about this one: commit e69c77e580d56d587381066f56027c8a596c237a Author: Elia Pinto gitter.spi...@gmail.com Date: Wed May 14 03:28:11 2014 -0700 t9137-git-svn-dcommit-clobber-series.sh: use the $( ... ) construct for command substitution [...] @@ -38,20 +38,20 @@ test_expect_success 'some unrelated changes to git' test_expect_success 'change file but in unrelated area' - test x\\`sed -n -e 4p file\`\ = x4 - test x\\`sed -n -e 7p file\`\ = x7 + test x$(sed -n -e 4p file) = x4 + test x$(sed -n -e 7p file) = x7 ^ here We're inside from the test_expect_success line. We used to have a literal (because of the backslash), we now have a closing because you removed the \. So, the sed command used to be protected by double-quotes, and it is now outside them. Compare: $ sh -c echo \\`date\`\ Wed May 14 18:47:54 MEST 2014 $ sh -c echo $(date) Wed In your case, it doesn't break because the expected output of sed contains no space, but that seems dangerous to me. I do not understand the use of the \ in front of the ` in the original code. The correct code should be test x\$(sed -n -e 4p file)\ = x4 I guess. perl -i.bak -p -e 's/^4\$//' file perl -i.bak -p -e 's/^7\$//' file - test x\\`sed -n -e 4p file\`\ = x - test x\\`sed -n -e 7p file\`\ = x + test x$(sed -n -e 4p file) = x + test x$(sed -n -e 7p file) = x Likewise. - test x\\`sed -n -e 4p file\`\ = x - test x\\`sed -n -e 7p file\`\ = x - test x\\`sed -n -e 58p file\`\ = x5588 - test x\\`sed -n -e 61p file\`\ = x6611 + test x$(sed -n -e 4p file) = x + test x$(sed -n -e 7p file) = x + test x$(sed -n -e 58p file) = x5588 + test x$(sed -n -e 61p file) = x6611 Likewise. More or less the same issue with commit 020568b9c36c023810a3482b7b73bcadd6406a85 Author: Elia Pinto gitter.spi...@gmail.com Date: Mon Apr 28 05:49:50 2014 -0700 t9114-git-svn-dcommit-merge.sh: use the $( ... ) construct for command substitution [...] diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh index fb41876..cf2e25f 100755 --- a/t/t9114-git-svn-dcommit-merge.sh +++ b/t/t9114-git-svn-dcommit-merge.sh @@ -68,8 +68,8 @@ test_expect_success 'setup git mirror and merge' ' test_debug 'gitk --all sleep 1' test_expect_success 'verify pre-merge ancestry' - test x\`git rev-parse --verify refs/heads/svn^2\` = \ -x\`git rev-parse --verify refs/heads/merge\` + test x\$(git rev-parse --verify refs/heads/svn^2\) = \ +x\$(git rev-parse --verify refs/heads/merge\) git cat-file commit refs/heads/svn^ | grep '^friend$' I'm not sure what's the intent of the \ in front of ` in the original code, but changing it to $(...) changes the meaning: $ sh -c echo \`date\` Wed May 14 18:58:19 MEST 2014 $ sh -c echo \$(date\) sh: 1: Syntax error: end of file unexpected (expecting )) I didn't investigate closely, but I'm getting test failures without your patch, and the script stops in the middle with it so it does break something. @@ -80,10 +80,10 @@ test_expect_success 'git svn dcommit merges' test_debug 'gitk --all sleep 1' test_expect_success 'verify post-merge ancestry' - test x\`git rev-parse --verify refs/heads/svn\` = \ -x\`git rev-parse --verify refs/remotes/origin/trunk \` - test x\`git rev-parse --verify refs/heads/svn^2\` = \ -x\`git rev-parse --verify refs/heads/merge\` + test x\$(git rev-parse --verify refs/heads/svn\) = \ +x\$(git rev-parse --verify refs/remotes/origin/trunk \) + test x\$(git rev-parse --verify refs/heads/svn^2\) = \ +x\$(git rev-parse --verify refs/heads/merge\) Likewise. commit 7e29ac501ce24aa5af3a50f839cd3ad176481a96 Author: Elia Pinto gitter.spi...@gmail.com Date: Wed Mar 26 04:48:40 2014 -0700 t9100-git-svn-basic.sh: use the $( ... ) construct for command substitution -test_expect_success 'able to dcommit to a subdirectory' +test_expect_success 'able to dcommit to a subdirectory' ' There is an actual change other than sed + review and trivial fix here. That makes the review harder. Such change should IMHO not be part of the same series. - git commit -m '/bar/d should
Re: t5539 fails on ubuntu for v2.0.0-rc2
Jeff King p...@peff.net writes: On Sat, May 10, 2014 at 10:02:59AM -0400, Jeff King wrote: On Fri, May 09, 2014 at 02:08:27PM -0700, Junio C Hamano wrote: 3. Just disable the http tests when run as root. I think I'd favor 3. But I'd like to be sure that being root is the problem. I agree with both the conclusion and the precondition. Here's the patch. The problem starts in v1.9.2, not in v2.0.0, so it's not technically a regression in this cycle. And we are awfully late in the -rc period. But it is just a change in the test script, and one that seems rather unlikely to produce unexpected side effects. I'll leave it you whether you want to queue it for v2.0.0, or for the next maint release. Hrm, sorry, I was wrong about this. I had thought the auto-network-testing had gone into v1.9.2, but it didn't. So this _is_ a potential regression in v2.0.0. It's still relatively minor, affecting only the test suite, so it can probably wait for post-v2.0.0 if you don't want to do another -rc. A bit more disturbing is that I did not get the impression that we know the exact reason why these http tests, especially the getwpuid call, fail for Fabio when run as root. And if my impression is correct, then do not run tests as root applied as a solution to the failure report, would merely be sweeping the problem under the rug, even though it is a very good advice in general. Is it a bug in the server itself that it fails to do getpwuid, or is it because the system Fabio's on is somehow screwed up? Until the latter can be ruled out, we might be better off not doing anything, which may give interested parties an easier way to dig deeper into the real cause of getpwuid failing, no? Such a digging may even result in a better solution (e.g. finding a specific pattern of misconfigured systems and stop tests only on them). Personally, I do not think running our tests as root is an interesting enough problem to warrant the effort from us to dig only to come up with such a better solution, and I would be perfectly happy to apply the do not run this test as root patch, or even a broader we do not let any test run as root, unless individual tests explicitly ask us to allow it somewhere in test-lib.sh included by everybody. That may sweep the issue under the rug as a side effect, but it is OK because it is not the primary mission of our tests to find issues in either httpd binaries or the system configuration that may cause the httpd server misbehave in the first place. So... -- 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] grep -I: do not bother to read known-binary files
Stepan Kasal ka...@ucw.cz writes:
From: Johannes Schindelin johannes.schinde...@gmx.de
Date: Mon, 8 Nov 2010 16:10:43 +0100
Incidentally, this makes grep -I respect the binary attribute (actually,
the -text attribute, but binary implies that).
Since the attributes are not thread-safe, we now need to switch off
threading if -I was passed.
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
Hi,
this patch has been in msysgit for 3.5 years.
Stepan
I do not think checking 'text' is the right way to do this. The
attribute controls the eof normalization, and people sometimes want
to keep CRLF terminated files in the repository no matter what the
platform is (an example I heard is a sample protocol exchange over
textual network protocol such as HTTP and SMTP), and the way to do
so is to unset it. That would still let them look for patterns in
and compare the changes to these paths.
Looking for Marking files as binary in gitattributes(5) gives us a
more authoritative alternative, I think. In short:
- If 'diff' is Unset, or
- If 'diff' is Set to X and diff.X.binary is true
then the contents are not suitable for human consumption. I haven't
thought things through to declare that grep -I would want to treat
a PostScript file (which is an example in that section) as a binary
file and refrain from trying to find substrings in it, but my gut
feeling is that we probably should let it look inside such a file,
so replacing 'text' with 'diff' in this patch and doing nothing
about diff.*.binary would be the way to go. Given that the posted
patch was older than 3.5 years, perhaps it needs updating to adhere
the advice given in ab435611 (docs: explain diff.*.binary option,
2011-01-09).
By the way, I wonder if the patch is about performance, implied by
do not bother (to waste time looking inside), though. Is it an
overall win to avoid looking inside -diff files, if that requires
you to use only 1 core and leaving the other 7 idle?
I think the user tells us it is binary with attribute, and the user
tells us not to look into binary with -I is a lot better rationale
to do this change , and that would characterise it as a correctness
patch, not a performance patch.
Thanks.
builtin/grep.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/builtin/grep.c b/builtin/grep.c
index 43af5b7..8073fbe 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -18,6 +18,7 @@
#include quote.h
#include dir.h
#include pathspec.h
+#include attr.h
static char const * const grep_usage[] = {
N_(git grep [options] [-e] pattern [rev...] [[--] path...]),
@@ -163,6 +164,22 @@ static void work_done(struct work_item *w)
grep_unlock();
}
+static int skip_binary(struct grep_opt *opt, const char *filename)
+{
+ if ((opt-binary GREP_BINARY_NOMATCH)) {
+ static struct git_attr *attr_text;
+ struct git_attr_check check;
+
+ if (!attr_text)
+ attr_text = git_attr(text);
+ memset(check, 0, sizeof(check));
+ check.attr = attr_text;
+ return !git_check_attr(filename, 1, check)
+ ATTR_FALSE(check.value);
+ }
+ return 0;
+}
+
static void *run(void *arg)
{
int hit = 0;
@@ -173,6 +190,9 @@ static void *run(void *arg)
if (!w)
break;
+ if (skip_binary(opt, (const char *)w-source.identifier))
+ continue;
+
opt-output_priv = w;
hit |= grep_source(opt, w-source);
grep_source_clear_data(w-source);
@@ -379,6 +399,9 @@ static int grep_cache(struct grep_opt *opt, const struct
pathspec *pathspec, int
continue;
if (!ce_path_match(ce, pathspec, NULL))
continue;
+ if (skip_binary(opt, ce-name))
+ continue;
+
/*
* If CE_VALID is on, we assume worktree file and its cache
entry
* are identical, even if worktree file has been modified, so
use
@@ -803,6 +826,8 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
string_list_append(path_list, show_in_pager);
use_threads = 0;
}
+ if ((opt.binary GREP_BINARY_NOMATCH))
+ use_threads = 0;
if (!opt.pattern_list)
die(_(no pattern given.));
--
1.9.2.msysgit.0.335.gd2a461f
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option
Stepan Kasal ka...@ucw.cz writes:
From: Johannes Schindelin johannes.schinde...@gmx.de
Date: Tue, 8 Feb 2011 00:17:24 -0600
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
Hi,
this patch has been in msysgit for 3 1/4 years.
Stepan
builtin/grep.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/builtin/grep.c b/builtin/grep.c
index 8073fbe..6c82a29 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -897,6 +897,9 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
if (len 4 is_dir_sep(pager[len - 5]))
pager += len - 4;
+ if (opt.ignore_case !strcmp(less, pager))
+ string_list_append(path_list, -i);
I have a feeling that this goes against the recent trend of not
mucking with the expectation of the users on their pagers, if I
recall correctly the arguments for dropping S from the default given
to an unconfigured LESS environment variable.
if (!strcmp(less, pager) || !strcmp(vi, pager)) {
struct strbuf buf = STRBUF_INIT;
strbuf_addf(buf, +/%s%s,
--
1.9.2.msysgit.0.335.gd2a461f
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option
Junio C Hamano gits...@pobox.com writes: Stepan Kasal ka...@ucw.cz writes: From: Johannes Schindelin johannes.schinde...@gmx.de Date: Tue, 8 Feb 2011 00:17:24 -0600 Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de Signed-off-by: Stepan Kasal ka...@ucw.cz --- Hi, this patch has been in msysgit for 3 1/4 years. Stepan builtin/grep.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/grep.c b/builtin/grep.c index 8073fbe..6c82a29 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -897,6 +897,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix) if (len 4 is_dir_sep(pager[len - 5])) pager += len - 4; +if (opt.ignore_case !strcmp(less, pager)) +string_list_append(path_list, -i); I have a feeling that this goes against the recent trend of not mucking with the expectation of the users on their pagers, if I recall correctly the arguments for dropping S from the default given to an unconfigured LESS environment variable. I had the same feeling about we're doing magic in corner-cases, makes it hard for the user to understand what's going on. But the patch actually makes a lot of sense. Without the patch, I get $ git grep -O -i no_openssl Pattern not found (press RETURN) with the patch, I do get a file opened and NO_OPENSSL selected. The lines right below the ones of the patch are aleady (documented) less+vi magic, which actually require the patch to behave well in the presence of -i. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option
Hi, Junio C Hamano wrote: Stepan Kasal ka...@ucw.cz writes: --- a/builtin/grep.c +++ b/builtin/grep.c @@ -897,6 +897,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix) if (len 4 is_dir_sep(pager[len - 5])) pager += len - 4; +if (opt.ignore_case !strcmp(less, pager)) +string_list_append(path_list, -i); I have a feeling that this goes against the recent trend of not mucking with the expectation of the users on their pagers, if I recall correctly the arguments for dropping S from the default given to an unconfigured LESS environment variable. It's just missing an explanation. When command happens to be the magic string less, today git grep -Ocommand -epattern helpfully passes +/pattern to less so you can navigate through the results within a file using the n and shift+n keystrokes. Alas, that doesn't do the right thing for a case-insensitive match. For that case we should pass --IGNORE-CASE to less so that n and shift+n can move between results ignoring case in the pattern. (That's -I, not -i, because it ought to work even when the pattern contains capital letters.) git grep has other options that affect interpretation of the pattern which this patch does not help with: * -v / --ignore-match: probably should disable this feature of -O. * -E / --extended-regexp * -P / --perl-regexp * -F / --fixed-strings: ideally would auto-escape regex specials. * -epattern1 --or -epattern2 And git grep -Ovi has a similar bug, for which the fix is to add \c to the pattern instead of passing an -I option. But as is, it's an improvement, so (except that -i should be replaced by -I) it seems like a good change. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option
Hello, On Wed, May 14, 2014 at 11:26:54AM -0700, Jonathan Nieder wrote: But as is, it's an improvement, so (except that -i should be replaced by -I) it seems like a good change. indeed, -I would match the semantics of git-grep -i . Stepan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option
Jonathan Nieder jrnie...@gmail.com writes: + if (opt.ignore_case !strcmp(less, pager)) + string_list_append(path_list, -i); I have a feeling that this goes against the recent trend of not mucking with the expectation of the users on their pagers, if I recall correctly the arguments for dropping S from the default given to an unconfigured LESS environment variable. It's just missing an explanation. ... (That's -I, not -i, because it ought to work even when the pattern contains capital letters.) Spot on. The change, especially with -I, makes sense. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] Gitweb: Convert UTF-8 encoded file names
Perl has an internal encoding used to store text strings. Currently, trying to
view files with UTF-8 encoded names results in an error (either 404 - Cannot
find file [blob_plain] or XML Parsing Error [blob]). Converting these UTF-8
encoded file names into Perl's internal format resolves these errors.
Signed-off-by: Michael Wagner accou...@mwagner.org
---
gitweb/gitweb.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a9f57d6..6046977 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1056,7 +1056,7 @@ sub evaluate_and_validate_params {
}
}
- our $file_name = $input_params{'file_name'};
+ our $file_name = decode(utf-8, $input_params{'file_name'});
if (defined $file_name) {
if (!is_valid_pathname($file_name)) {
die_error(400, Invalid file parameter);
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git grep -O -i: if the pager is 'less', pass the '-i' option
On Wed, May 14, 2014 at 11:26:54AM -0700, Jonathan Nieder wrote: git grep has other options that affect interpretation of the pattern which this patch does not help with: * -v / --ignore-match: probably should disable this feature of -O. * -E / --extended-regexp * -P / --perl-regexp * -F / --fixed-strings: ideally would auto-escape regex specials. * -epattern1 --or -epattern2 And git grep -Ovi has a similar bug, for which the fix is to add \c to the pattern instead of passing an -I option. We've already found the lines of interest to the user. It would be nice if we could somehow point the pager at them by number, rather than repeating the (slightly incompatible) search. We can do less +25, but that only points it to the first instance (and doesn't highlight it), whereas the current code lets the repeat-search find other instances. I don't think there's a way to queue up a set of interesting lines to visit in less. At least I could not think of one. This is more or less how I ended up at the design of contrib/git-jump, which uses quickfix lists to make such a queue (only editors understand those, not pagers, but I consider being dumped into the editor a feature :) ). But as is, it's an improvement, so (except that -i should be replaced by -I) it seems like a good change. Agreed. Thanks for the list of problematic options. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib: remote-helpers: add move warnings (v2.0)
Marius Storm-Olsen msto...@gmail.com writes:
On 5/13/2014 5:47 PM, Felipe Contreras wrote:
I was going to do more than pointing to commits, I was going to
provide the fixes with test cases and a detailed explanation. But
then you made your decision.
I believe the regression in question, mentioned at the bottom of this post
http://thread.gmane.org/gmane.comp.version-control.git/248263/focus=248269
Since you are not going to do so, I do not feel compelled to fix
the synchronization crash regression that is present in v2.0.0-rc2
and I already warned you about.
is referring to this patch
http://thread.gmane.org/gmane.comp.version-control.git/247546/focus=247549
but I admit, I'm getting a bit fuzzy around these discussions.
Thanks for trying to help.
The patch you pointed out however names 2594a79 as the culprit, but
it has been in 1.8.3 and upwards, so I am not sure what to say. As
I do not recall seeing anything about I already warned you about,
I fished the archive again, but the closest I found was this:
http://thread.gmane.org/gmane.comp.version-control.git/248063/focus=248601
in which we heard You won't be able to find the breakage. when I
hinted bisecting. The only thing we saw was I already said this
multiple times, but let me be clear once more: MASTER HAS A
REGRESSION (for all versions of Mercurial). and I can believe if he
said that exact phrase multiple times, but I do not think he said
anything useful than I broke 2.0 prereleases anywhere---at least I
didn't find any ... with this commit in what way.
So at this point, I would have to say that the users of remote-hg is
taken hostage by its author. One safe way forward at this point in
order to avoid regression would be to revert everything done by him
as suspicious, but that is a route that is too overcautious even for
me, and I am not willing to travel that road.
The synch crash regression points me more towards 3994e64d
(transport-helper: fix sync issue on crashes, 2014-04-12), though.
I would happily revert the merge d508e4a that pulled the topic into
v2.0.0-rc1.
-- 8 --
Subject: [PATCH] Revert Merge branch 'fc/transport-helper-sync-error-fix'
This reverts commit d508e4a8e2391ae2596403b6478d01cf3d5f928f,
reversing changes made to e42552135a2a396f37053a89f44952ea907870b2.
The author of the original topic says he broke the upcoming 2.0
release with something that relates to synchronization crash
regression while refusing to give further specifics, so this would
unfortunately be the safest option for the upcoming release.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/RelNotes/2.0.0.txt | 4 ---
t/t5801-remote-helpers.sh| 31 +
transport-helper.c | 73
3 files changed, 37 insertions(+), 71 deletions(-)
diff --git a/Documentation/RelNotes/2.0.0.txt b/Documentation/RelNotes/2.0.0.txt
index 6e628d4..97f7df0 100644
--- a/Documentation/RelNotes/2.0.0.txt
+++ b/Documentation/RelNotes/2.0.0.txt
@@ -88,10 +88,6 @@ UI, Workflows Features
* git grep learned to behave in a way similar to native grep when
-h (no header) and -c (count) options are given.
- * git push via transport-helper interface (e.g. remote-hg) has
- been updated to allow forced ref updates in a way similar to the
- natively supported transports.
-
* The simple mode is the default for git push.
* git add -u and git add -A, when run without any pathspec, is a
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index a00a660..25fd2e7 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -212,30 +212,19 @@ test_expect_success 'push update refs failure' '
echo update fail file
git commit -a -m update fail
git rev-parse --verify testgit/origin/heads/update expect
- test_expect_code 1 env GIT_REMOTE_TESTGIT_FAILURE=non-fast forward \
- git push origin update
+ GIT_REMOTE_TESTGIT_PUSH_ERROR=non-fast forward
+ export GIT_REMOTE_TESTGIT_PUSH_ERROR
+ test_expect_code 1 git push origin update
git rev-parse --verify testgit/origin/heads/update actual
test_cmp expect actual
)
'
-clean_mark () {
- cut -f 2 -d ' ' $1 |
- git cat-file --batch-check |
- grep commit |
- sort $(basename $1)
-}
-
-cmp_marks () {
- test_when_finished rm -rf git.marks testgit.marks
- clean_mark .git/testgit/$1/git.marks
- clean_mark .git/testgit/$1/testgit.marks
- test_cmp git.marks testgit.marks
-}
-
test_expect_success 'proper failure checks for fetching' '
- (cd local
- test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git fetch 2error
+ (GIT_REMOTE_TESTGIT_FAILURE=1
+ export GIT_REMOTE_TESTGIT_FAILURE
+ cd local
+ test_must_fail git fetch 2 error
cat error
grep -q Error while running fast-import error
)
@@ -243,11
Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
Jens Lehmann jens.lehm...@web.de writes:
Junio, I believe this issue needs to be fixed before 2.0 final. Otherwise
git gui will not work inside submodules anymore due to the major version
number change from 1 to 2. I'd like to hear Pat's opinion on this; even
though I think my patch is less risky (as it doesn't change behavior for
pre-2 versions), he might like Chris' proposal better.
Thanks; I share the same feeling.
Am 13.05.2014 23:24, schrieb Jens Lehmann:
Since git 2.0.0 starting git gui in a submodule using a gitfile fails with
the following error:
No working directory ../../../path
couldn't change working directory
to ../../../path: no such file or
directory
This is because git rev-parse --show-toplevel is only run when git gui
sees a git version of at least 1.7.0 (which is the version in which the
--show-toplevel option was introduced). But it uses vsatisfies to check
that, which is documented to return false when the major version changes,
which is not what we want here.
Change vsatisfies to vcompare when testing for a git version to avoid the
problem when the major version changes (but still use vsatisfies in those
places where the Tk version is checked). This is done for both the place
that caused the reported bug and another spot where the git version is
tested for another feature.
Reported-by: Chris Packham judge.pack...@gmail.com
Reported-by: Yann Dirson ydir...@free.fr
Signed-off-by: Jens Lehmann jens.lehm...@web.de
---
Am 07.05.2014 09:49, schrieb Chris Packham:
On 07/05/14 19:28, Chris Packham wrote:
On 07/05/14 00:10, Pat Thoyts wrote:
Chris Packham judge.pack...@gmail.com writes:
On Tue, Apr 29, 2014 at 2:56 PM, Chris Packham judge.pack...@gmail.com
wrote:
Hi Pat,
I'm running git 2.0.0-rc0 (haven't got round to pulling down rc1 yet)
which includes gitgui-0.19.0 and I'm getting a new error when I run
'git gui' in a repository with a .git file (created by git submodule).
I can send you a screencap of the error message off list if you want
but the text is
No working directory ../../../repo
couldn't change working directory to ../../../repo: no such file or
directory
My tcl is a little rusty but I think the problem might be this snippet.
# v1.7.0 introduced --show-toplevel to return the canonical work-tree
if {[package vsatisfies $_git_version 1.7.0]} {
if { [is_Cygwin] } {
catch {set _gitworktree [exec cygpath --windows [git rev-parse
--show-toplevel]]}
} else {
set _gitworktree [git rev-parse --show-toplevel]
}
} else {
# try to set work tree from environment, core.worktree or use
# cdup to obtain a relative path to the top of the worktree. If
# run from the top, the ./ prefix ensures normalize expands pwd.
if {[catch { set _gitworktree $env(GIT_WORK_TREE) }]} {
set _gitworktree [get_config core.worktree]
if {$_gitworktree eq } {
set _gitworktree [file normalize ./[git rev-parse
--show-cdup]]
}
}
}
The vsatisfies call probably doesn't handle '2.0.0.rc0' and the
fallback behaviour probably needs to normalise core.worktree
The _git_version variable has already been trimmed to remove such
suffixes so the version comparison here will be ok.
I don't think that's true 'git rev-parse --show-toplevel' does the right
thing - if it's run.
We'll the trimming works but vstatisfies doesn't
puts $_git_version
puts [package vsatisfies $_git_version 1.7.0]
2.0.0
0
Yup, looks like vsatisfies is doing just what it is supposed to (according
to http://www.astro.princeton.edu/~rhl/Tcl-Tk_docs/tcl/package.n.html):
package vsatisfies version1 version2
Returns 1 if scripts written for version2 will work unchanged
with version1 (i.e. version1 is equal to or greater than version2
and they both have the same major version number), 0 otherwise.
The bump in the major number from 1 to 2 makes vsatisfies assume that the
version is not compatible anymore, I believe we should have used vcompare
here and in another place.
git-gui.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index cf2209b..ed2418b 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1283,7 +1283,7 @@ load_config 0
apply_config
# v1.7.0 introduced --show-toplevel to return the canonical work-tree
-if {[package vsatisfies $_git_version 1.7.0]} {
+if {[package vcompare $_git_version 1.7.0]} {
if { [is_Cygwin] } {
catch {set _gitworktree [exec cygpath --windows [git rev-parse
--show-toplevel]]}
} else {
@@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {
close $fd
}
-if {[package vsatisfies $::_git_version 1.6.3]} {
+if {[package vcompare $::_git_version 1.6.3]} {
set ls_others [list --exclude-standard]
} else {
set ls_others [list --exclude-per-directory=.gitignore]
Re: t5539 fails on ubuntu for v2.0.0-rc2
On Wed, May 14, 2014 at 10:11:11AM -0700, Junio C Hamano wrote: A bit more disturbing is that I did not get the impression that we know the exact reason why these http tests, especially the getwpuid call, fail for Fabio when run as root. And if my impression is correct, then do not run tests as root applied as a solution to the failure report, would merely be sweeping the problem under the rug, even though it is a very good advice in general. Is it a bug in the server itself that it fails to do getpwuid, or is it because the system Fabio's on is somehow screwed up? I think it's a config problem possibly coupled with an Apache bug. We don't define the User directive (and I think it would be kind of crazy to do so[1]). The default for User is -1, and we see in his log that apache was trying to getpwuid to 4294967295. However, if you don't set User at all, and you are running as root, I would expect Apache to continue running as root. The most I could come up with on this front was that it's a bug in apache 1.3.22: http://www.gossamer-threads.com/lists/apache/users/179688#179688 but that seems like an awfully old version to have on a Natty desktop (I checked, and it seems only to have shipped apache2). So either the bug resurfaced, or something else is going on. Until the latter can be ruled out, we might be better off not doing anything, which may give interested parties an easier way to dig deeper into the real cause of getpwuid failing, no? Such a digging may even result in a better solution (e.g. finding a specific pattern of misconfigured systems and stop tests only on them). Sure, I have no problem with anybody digging deeper. I was mainly worried that it would become a pain for people on such systems, as it causes a false positive in the test suite. But we don't even know how common such systems are, so I'm OK with waiting to see if we have more reports. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib: remote-helpers: add move warnings (v2.0)
Ronnie Sahlberg wrote: On Tue, May 13, 2014 at 4:37 PM, Felipe Contreras felipe.contre...@gmail.com wrote: But if I have to adjust for saying that (which was true), what do you say to Junio for saying this? (which was not) I know I shouldn't but I will respond anyway. The problem is about you and your behaviour. Not Junio or anyone else. Right. So when I say something moderately aggressive (but true), I'm the problem, when Junio says something worst (and false), he is not. Got it. -- Felipe Contreras -- 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] grep -I: do not bother to read known-binary files
On Wed, May 14, 2014 at 05:44:19PM +0200, Stepan Kasal wrote: From: Johannes Schindelin johannes.schinde...@gmx.de Date: Mon, 8 Nov 2010 16:10:43 +0100 Incidentally, this makes grep -I respect the binary attribute (actually, the -text attribute, but binary implies that). Since the attributes are not thread-safe, we now need to switch off threading if -I was passed. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de Signed-off-by: Stepan Kasal ka...@ucw.cz --- Hi, this patch has been in msysgit for 3.5 years. Stepan Hrm. Is this patch still necessary? In the time since this patch was written, we did 0826579 (grep: load file data after checking binary-ness, 2012-02-02), which should do the same thing. It deals with the threading via a lock, but we later learned in 9dd5245 (grep: pre-load userdiff drivers when threaded, 2012-02-02) to hoist that bit out. So I suspect this patch at best is doing nothing, and at worst is wasting extra time doing redundant attribute checks. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] grep -I: do not bother to read known-binary files
On Wed, May 14, 2014 at 10:52:28AM -0700, Junio C Hamano wrote: I do not think checking 'text' is the right way to do this. The attribute controls the eof normalization, and people sometimes want to keep CRLF terminated files in the repository no matter what the platform is (an example I heard is a sample protocol exchange over textual network protocol such as HTTP and SMTP), and the way to do so is to unset it. That would still let them look for patterns in and compare the changes to these paths. Looking for Marking files as binary in gitattributes(5) gives us a more authoritative alternative, I think. In short: - If 'diff' is Unset, or - If 'diff' is Set to X and diff.X.binary is true then the contents are not suitable for human consumption. I responded elsewhere in the thread that I think the patch under discussion is redundant at this point, but just to clarify: grep currently uses the rules you give above, as it builds on the userdiff driver code. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/17] contrib: remove outdated README
Philippe Vaucher wrote: I have had patches and contributions rejected in the past, sometimes rudely. Same has happened to many others, if you contribute long enough, it is pretty much guaranteed that it will happen to you. Maintainer is wrong, or you are wrong, or someone is just having a bad day. This is not about a couple of patches I worked in a weekend being rejected. This is about the work I've been doing since the past two years almost like a full-time job dropped to the floor with no explanation at all. I started with the expectation that they were going to move to the core, because Junio said so, then he changed his mind and didn't want to explain his reasoning. It's not just a bad day. Here are two posts where Junio and Michael Haggerty explain the reasoning to you: - http://article.gmane.org/gmane.comp.version-control.git/248727 - http://article.gmane.org/gmane.comp.version-control.git/248693 Basically, in your case it boils down to your social manners. You are not paying attention at all. Junio did *not* use my social manners as a reason to block the graduation, nor the quality of the code, he used a *TECHNICAL* reason. Prior to his decision there were no complaints about my manners since I returned. It was his *TECHNICAL* decision that triggered this. Junio never explained his *TECHNICAL* reason, and Michael Haggerty simply said there are good technical arguments for and against moving git-remote-hg out of contrib, that was all his explanation for the *TECHNICAL* reason. You, and other people, are using the behavior I displayed *AFTER* Junio made his *TECHNICAL* decision as the cause for his decision not to graduate. That's a wrong direction fallacy. -- Felipe Contreras -- 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] contrib: remote-helpers: add move warnings (v2.0)
Junio C Hamano wrote: So at this point, I would have to say that the users of remote-hg is taken hostage by its author. The users of remote-hg are being affected negatively *because* of your decisions. You have the power to help them by answering a simple question. Yet you refuse to do that. It's all on you. -- Felipe Contreras -- 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 01/17] contrib: remove outdated README
On Wed, May 14, 2014 at 3:35 PM, Felipe Contreras felipe.contre...@gmail.com wrote: You are not paying attention at all. Junio may have been trying to be polite and not tell you directly that attitude was a factor. Whatever. He is the maintainer. Of all the folks in this list, he gets to call the shots when the criteria aren't 100% clear. Quite a few voices here have been heard, all telling you that you are wrong. Even if you might be right about some aspects, you are wrong. Time to let go. good bye, m -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- 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] contrib: remote-helpers: add move warnings (v2.0)
Felipe Contreras felipe.contre...@gmail.com writes: Junio C Hamano wrote: So at this point, I would have to say that the users of remote-hg is taken hostage by its author. The users of remote-hg are being affected negatively *because* of your decisions. You have the power to help them by answering a simple question. Yet you refuse to do that. It's all on you. That is exactly what I would call taking users hostage. I think I already answered that one question: I think after this insane amount of work I'm entitled to an answer for this *one* question. Instead you passive aggressively label me as a troll? This is really disquieting. Junio, do you honestly think I am a troll? Have at least the decency of telling it to me. in $gmane/248853 with: You certainly are acting like one, aren't you? Do you need more? -- 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 01/17] contrib: remove outdated README
On Wed, May 14, 2014 at 02:35:08PM -0500, Felipe Contreras wrote: Prior to his decision there were no complaints about my manners since I returned. It was his *TECHNICAL* decision that triggered this. There have been several complaints about your behavior since you returned[1,2,3,4], in addition to the times I and others have simply left threads because they did not feel that arguing with you was productive (a thing that several people, myself included, told you in the past that they would do). I have not paid all that close attention to this remote-hg contrib argument. Maybe you are 100% right and Junio is wrong and lying and cannot back up his decision, but somehow needs to cover it up through rhetoric. But that does not at all match my past experiences with Junio's behavior. And given my past experiences with you, and your behavior in response to the discussion, I find it likely that either mis-communication or your stubbornness is a likely factor. And also given those experiences, I've avoided wading into the thread myself or spending too much time thinking about it. I can understand that it hurts to be called a troll. I really do believe you are trying your best to improve git. But I hope you also understand that in terms of the time and emotional drains on other participants, from our perspective interacting with you is quite similar to interacting with a troll. Your continued arguing on this topic does not seem like it is going anywhere, and I believe is wasting time and hurting the atmosphere of the list. Please stop. You have had many chances to interact with people on the list[5], and now you are seeing the consequences of your behaviors: people have little tolerance for you and will stop paying attention. -Peff [1] http://article.gmane.org/gmane.comp.version-control.git/247108 [2] http://article.gmane.org/gmane.comp.version-control.git/247121 [4] http://article.gmane.org/gmane.comp.version-control.git/247168 [3] http://article.gmane.org/gmane.comp.version-control.git/248000 [5] Even though you were previously asked to leave, I believe people on the list gave interacting with you a fair shot in the past month or two. And we somehow still ended up at the same place. That's just my perception, of course. I'd invite anybody watching to form their own opinions. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] remote-hg: more improvements
Philippe Vaucher wrote: I'm sorry that my words aren't clear enough for you to infer the point I'm trying to make. Let's try something simpler: what I was saying is that bad behavior will get you into trouble when contributing (and thus it's important to behave nicely), where Felipe usualy says bad behavior is irrelevant because only truth/quality is important. Do you feel Felipe is in control of what you label bad behavior? Do you feel you are in control over how you react to his behavior? I feel that Felipe cannot control this (or decided not to), I am pretty much in control of my behavior. Those who know me personally know that I *never* get angry. The problem is that what you label as bad behavior I do not. For example the atheist billboard that said You KNOW it's a Myth. This Season, Celebrate REASON! caused a lot of furor, and people all over U.S.A. said you cannot say that!, that's offensive!, it's a war against Christmass, that shouldn't be tolerated in modern society, and so on. Of course, it should be tolerated in a modern society, and it should not be offensive, those are facts. Of course people can disagree with those facts (but those people are wrong). The atheists are not engaging in bad behavior, it's just that the majority constituted by religious and bend-over-backwards-liberals have a different definition of bad behavior (a wrong definition). So if somebody convinced me that my behavior is indeed bad, I would certainly change it, but I do not think it's bad. Just the same way nobody has convinced many atheists that their behavior in those billboards is bad. And they can't convince them, because they would need logic, reason, and evidence, and the atheists have them. Yesterday my girl friend bought back a mare she had sold two years ago because its owner did not manage to get along with it. It's a temperamental animal that learns and performs amazingly well for its comparatively compact build. But it's highest in rank or else, and so in the end it got locked up in its stable box most of the time in order to avoid injuries to other horses. Now it's back here at the riding school, and there is little question that there will be some injuries before things settle down again even though most of the horses here know it already. I think that is the point: behave properly or be isolated to avoid harming others. Wether you control your behavior or not has little to do with it, it's your behavior that counts. That's how it works in pretty much any communities I know of. Except the largest and most important communities; countries. In the vast majority of countries what is labeled as bad (a.k.a. illegal) has had much more consideration (millenia) than in most online communities (barely decades). Saying something is stupid might be considered bad to some people, but not all, so you can't get thrown in jail for doing that, even though some peple thing it's harming others. For example, saying religion is stupid. In order to be thrown in jail you have to do something _truly_ bad, something we all agree is harming others. In online communities facts don't matter, if the majority thinks you are doing something bad (even if you are not), you get thrown to jail (or virtual jail). It may be that distributed version control systems offer more possibilities for organizing cooperation in a manner leaving graceful escape paths when things don't work out. It's not what one want to have to rely on permanently but it may be worth thinking about ways to make consequences from difficulties less inevitable and/or grave. Sure, I believe my proposal of acting on bad behavior earlier would prevent incidents like this one, because it would defuse situations before they settle in. But that's just a proposal, I'm just an observer here. What you label bad behavior started *because* of Junio's decision. Junio made an important (to me) decision based on a *TECHNICAL* reason he never explained. My bad behavior basically consisted in pointing out Junio's bad behavior. In this case I'm not very dissimilar to Edward Snowden. Snowden did somethig bad in the eyes of many people, but he did it *because* of something deeper and more important. It was the government that caused Snowden's actions, and it is the government's bad behavior that is important. But if you look at the mass media, they don't concentrate on the government's actions (that's risky), they concentrate on Snowden. This might have changed after Snowden gained support, but at least initially it was this way. You are doing exactly the same as the mass media. Instead of looking at the actions of Junio, namely his *TECHNICAL* decision which *triggered* my bad behavior, you look at me, the easy target, and ignore the real problem. Wait, wait! Before focusing on deciding whether leaking documents is bad, and how exactly it was carried out, why don't you concentrate on the real issue; the fact
Re: [GUILT v2 08/29] Added more test cases for guilt new: empty patches.
On Wed, May 14, 2014 at 7:10 PM, Jeff Sipek jef...@josefsipek.net wrote:
On Tue, May 13, 2014 at 10:30:44PM +0200, Per Cederqvist wrote:
Test that empty patches are handled correctly, both with and without
the guilt.diffstat configuration option.
Signed-off-by: Per Cederqvist ced...@opera.com
---
regression/t-020.out | 250
+++
regression/t-020.sh | 60 +
2 files changed, 310 insertions(+)
diff --git a/regression/t-020.out b/regression/t-020.out
index af45734..7e07efa 100644
--- a/regression/t-020.out
+++ b/regression/t-020.out
@@ -1128,3 +1128,253 @@ f 9c18cc7abe6b87f18503714a80a677b4094eb457
.git/patches/master/add
...
+% git log -p
+commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1
+Author: Author Name author@email
+Date: Mon Jan 1 00:00:00 2007 +
+
+patch empty.patch
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date: Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
b/def
+@@ -0,0 +1 @@
++abc
+% git config guilt.diffstat false
+---
+
I'm a bit confused about the above. It looks like contents of an empty
patch with an empty diffstat. But the only time I see a cat in the .sh file
is when you rewrite... oh I got it. I'll comment about it by the 'cat'.
...
diff --git a/regression/t-020.sh b/regression/t-020.sh
index cdd08ba..906aec6 100755
--- a/regression/t-020.sh
+++ b/regression/t-020.sh
@@ -5,6 +5,13 @@
source $REG_DIR/scaffold
+function fixup_time_info
+{
+ cmd guilt pop
+ touch -a -m -t $TOUCH_DATE .git/patches/master/$1
+ cmd guilt push
+}
+
cmd setup_repo
#
@@ -69,6 +76,59 @@ done
cmd list_files
+# push an empty patch with no commit message
+cmd guilt new empty.patch
+fixup_time_info empty.patch
+cmd list_files
+cmd git log -p
+
+# Ensure we can push the empty patch even when guilt.diffstat is true.
+cmd git config guilt.diffstat true
+cmd guilt refresh
+fixup_time_info empty.patch
+cmd list_files
+cmd git log -p
+cmd git config guilt.diffstat false
+
+# Let the patch have a commit message, but no data.
+cat .git/patches/master/empty.patch EOF
cat .git/.../empty.patch EOF
...
EOF
Otherwise, you'll just cat the existing patch and that's it.
Yes. The intent was to modify empty.patch. This is a bit
embarrassing, since it should have been obvious from
the output of git log -p that the modification didn't work
as I intended. :-)
I'll fix in the v3 series.
/ceder
+Fix a bug.
+
+From: Per Cederqvist ce...@lysator.liu.se
+
+This commit fixes a serious bug.
+
+FIXME:
+- add a test case
+- track down the bug
+- actually fix it
+EOF
+
+fixup_time_info empty.patch
+cmd list_files
+cmd git log -p
+
+# And once more, with an empty diffstat.
+
+cmd git config guilt.diffstat true
+cmd guilt refresh
+fixup_time_info empty.patch
+cmd list_files
+cmd git log -p
+
+# Restore the diffstat setting and remove the empty patch.
+cmd git config guilt.diffstat false
+cmd guilt refresh
+fixup_time_info empty.patch
+cmd list_files
+cmd git log -p
+# (Cannot delete an applied patch)
+shouldfail guilt delete empty.patch
+cmd guilt pop -a
+cmd guilt delete -f empty.patch
+cmd list_files
+cmd git log -p
+
# FIXME:
# --all
# -a
--
1.8.3.1
--
Fact: 23.6% of all statistics are generated randomly.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] remote-hg: more improvements
Felipe Contreras felipe.contre...@gmail.com writes: Philippe Vaucher wrote: [...] Do you feel Felipe is in control of what you label bad behavior? Do you feel you are in control over how you react to his behavior? I feel that Felipe cannot control this (or decided not to), I am pretty much in control of my behavior. Those who know me personally know that I *never* get angry. You are missing the point. The point is not what effect your behavior has on you but what it has on others. As you are unable to attribute feelings to others (like you appear unable to attribute reason to them), you are unable to control it. It's like repairing clockwork when your hands are numb and have never been otherwise. When something does not work, apply extra force does not work with clockwork. It does not work with people, either. The problem is that what you label as bad behavior I do not. It does not matter what you or others want to label it. The effects are there anyway. So if somebody convinced me that my behavior is indeed bad, I would certainly change it, but I do not think it's bad. What effect your behavior has on others is not dependent on what you think about it. If you convinced yourself to be standing perfectly balanced and find yourself falling, there is no point in not catching yourself before smashing your head open just because you _know_ you have been standing perfectly upright. -- David Kastrup -- 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 01/17] contrib: remove outdated README
Felipe Contreras felipe.contre...@gmail.com writes: Philippe Vaucher wrote: I have had patches and contributions rejected in the past, sometimes rudely. Same has happened to many others, if you contribute long enough, it is pretty much guaranteed that it will happen to you. Maintainer is wrong, or you are wrong, or someone is just having a bad day. This is not about a couple of patches I worked in a weekend being rejected. This is about the work I've been doing since the past two years almost like a full-time job dropped to the floor with no explanation at all. I started with the expectation that they were going to move to the core, because Junio said so, then he changed his mind and didn't want to explain his reasoning. It's not just a bad day. Here are two posts where Junio and Michael Haggerty explain the reasoning to you: - http://article.gmane.org/gmane.comp.version-control.git/248727 - http://article.gmane.org/gmane.comp.version-control.git/248693 Basically, in your case it boils down to your social manners. You are not paying attention at all. Junio did *not* use my social manners as a reason to block the graduation, nor the quality of the code, he used a *TECHNICAL* reason. Prior to his decision there were no complaints about my manners since I returned. It was his *TECHNICAL* decision that triggered this. Junio never explained his *TECHNICAL* reason, and Michael Haggerty simply said there are good technical arguments for and against moving git-remote-hg out of contrib, that was all his explanation for the *TECHNICAL* reason. You, and other people, are using the behavior I displayed *AFTER* Junio made his *TECHNICAL* decision as the cause for his decision not to graduate. That's a wrong direction fallacy. I am not interested in distinction between technical and social that much. The points that were raised in the thread started by John Keeping, and some of the points that came to my mind while on that thread, even though I may not have mentioned in *that* thread, that affected the way *I* thought about the issue are these (not exhaustive): - We may be painted in a hard place if remote-hg or remote-bzr take us to a position where the Git as a whole is blocked while it is incompatible with the other side. Maintaining it as an independent project (aka Unbundling) would eliminate that risk, instead of having to handwave and say that risk does not exist. - A remote-helper has to depend on both sides. Keeping it in either contrib/ or in core, as opposed to unbundling, may make things easier for the remote-helper maintainer, because at least it would allow the helper to advance with Git in lock-step (but I never heard that you do not prefer unbundling for this reason). - In a longer term, a properly maintained remote-helpers should work with wide varieties of Git and the remote system versions anyway, so unbundling would be logically the more correct thing to do. - Unbundling would make it less easier to use the remote-helpers for people who are used to keep up with my tree and pick them up from contrib/, but that is a tiny minority these days. Most people use what distros package, and the distros that already package contrib/ remote-helpers will switch their upstream to unbundled repositories in order not to regress their packages for their users. - On the other hand, unbundling will make it easier for the the end-users (both the ones who are fed distro packaged versions and the ones who build from the source _and_ who overcome the less easier because now they have to pull from not just me but from the unbundled places inconvenience) to keep up with the leading/bleeding edge, because the remote-helpers do not have to freeze at the same time other parts of Git are frozen before the release, and users and distros can pick improved remote-helpers up and even mix and match, when they have some other reason that prevents them from updating Git itself. - Unbundling would also involve the risk of making them obscure, and the original reason why we added contrib/ area to host having something is often better than having nothing tools, even if some of them were of lessor quality, was exactly that. While building the momentum and the Git community, it was necessary to have a nursery. That reasoning no longer applies these days, and we have seen examples of third-party independent products that can improve the users' Git life flourishing. We have less need for a nursery is not a reason to kick bundled things out that want to be bundled, but it tells us that we no longer have to be afraid of unbundled things dying in obscurity, if there are other reasons that tell us unbundling is better. I may be missing some others, and I would be lying if I did not at all think of the net liability issue Michael brought up, but the above
Re: [PATCH] contrib: remote-helpers: add move warnings (v2.0)
Junio C Hamano wrote: Felipe Contreras felipe.contre...@gmail.com writes: Junio C Hamano wrote: So at this point, I would have to say that the users of remote-hg is taken hostage by its author. The users of remote-hg are being affected negatively *because* of your decisions. You have the power to help them by answering a simple question. Yet you refuse to do that. It's all on you. That is exactly what I would call taking users hostage. I think I already answered that one question: in $gmane/248853 with: You certainly are acting like one, aren't you? Do you need more? You know full well this is not the question I asked you repeatedly. Stop trying to spin the readers. I asked you here: http://article.gmane.org/gmane.comp.version-control.git/248683 And here: http://article.gmane.org/gmane.comp.version-control.git/248348 And here: http://article.gmane.org/gmane.comp.version-control.git/248368 I made it clear you were not answering the qustion here: http://article.gmane.org/gmane.comp.version-control.git/248685 And here: http://article.gmane.org/gmane.comp.version-control.git/248701 And here (apparently deleted mail): 1400013572-30232-1-git-send-email-felipe.contre...@gmail.com And probably many other times. And I will ask you once again. Please answer this *one* question: 1) Please clarify the reason why you blocked the graduation of remote helpers. Please give the full rationale and do not point to other mails, or other peoples' explanations. If necessary attach such explanations to your full reasoning. This is the *one* question you have refused to answer over and over. -- Felipe Contreras -- 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] grep -I: do not bother to read known-binary files
Jeff King p...@peff.net writes: On Wed, May 14, 2014 at 05:44:19PM +0200, Stepan Kasal wrote: From: Johannes Schindelin johannes.schinde...@gmx.de Date: Mon, 8 Nov 2010 16:10:43 +0100 Incidentally, this makes grep -I respect the binary attribute (actually, the -text attribute, but binary implies that). Since the attributes are not thread-safe, we now need to switch off threading if -I was passed. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de Signed-off-by: Stepan Kasal ka...@ucw.cz --- Hi, this patch has been in msysgit for 3.5 years. Stepan Hrm. Is this patch still necessary? In the time since this patch was written, we did 0826579 (grep: load file data after checking binary-ness, 2012-02-02), which should do the same thing. It deals with the threading via a lock, but we later learned in 9dd5245 (grep: pre-load userdiff drivers when threaded, 2012-02-02) to hoist that bit out. Wow, power of Git history ;-) So I suspect this patch at best is doing nothing, and at worst is wasting extra time doing redundant attribute checks. Sounds like a sensible conclusion. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 10/42] refs.c: ref_transaction_delete to check for error and return status
Change ref_transaction_delete() to do basic error checking and return
non-zero of error. Update all callers to check the return for
ref_transaction_delete(). There are currently no conditions in _delete that
will return error but there will be in the future.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/update-ref.c | 5 +++--
refs.c | 15 ++-
refs.h | 9 +
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 3fab810..fc3512f 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -257,8 +257,9 @@ static const char *parse_cmd_delete(struct strbuf *input,
const char *next)
if (*next != line_termination)
die(delete %s: extra input: %s, refname, next);
- ref_transaction_delete(transaction, refname, old_sha1,
- update_flags, have_old);
+ if (ref_transaction_delete(transaction, refname, old_sha1,
+ update_flags, have_old))
+ die(failed transaction delete for %s, refname);
update_flags = 0;
free(refname);
diff --git a/refs.c b/refs.c
index df376fa..64e3d53 100644
--- a/refs.c
+++ b/refs.c
@@ -3381,19 +3381,24 @@ int ref_transaction_create(struct ref_transaction
*transaction,
return 0;
}
-void ref_transaction_delete(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *old_sha1,
- int flags, int have_old)
+int ref_transaction_delete(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *old_sha1,
+ int flags, int have_old)
{
- struct ref_update *update = add_update(transaction, refname);
+ struct ref_update *update;
+ if (have_old !old_sha1)
+ die(have_old is true but old_sha1 is NULL);
+
+ update = add_update(transaction, refname);
update-flags = flags;
update-have_old = have_old;
if (have_old) {
assert(!is_null_sha1(old_sha1));
hashcpy(update-old_sha1, old_sha1);
}
+ return 0;
}
int update_ref(const char *action, const char *refname,
diff --git a/refs.h b/refs.h
index d4c068d..6026edf 100644
--- a/refs.h
+++ b/refs.h
@@ -266,11 +266,12 @@ int ref_transaction_create(struct ref_transaction
*transaction,
* Add a reference deletion to transaction. If have_old is true, then
* old_sha1 holds the value that the reference should have had before
* the update (which must not be the null SHA-1).
+ * Function returns 0 on success and non-zero on failure.
*/
-void ref_transaction_delete(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *old_sha1,
- int flags, int have_old);
+int ref_transaction_delete(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *old_sha1,
+ int flags, int have_old);
/*
* Commit all of the changes that have been queued in transaction, as
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 15/42] fast-import.c: change update_branch to use ref transactions
Change update_branch() to use ref transactions for updates.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
fast-import.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 6707a66..79d219b 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1679,39 +1679,45 @@ found_entry:
static int update_branch(struct branch *b)
{
static const char *msg = fast-import;
- struct ref_lock *lock;
+ struct ref_transaction *transaction;
unsigned char old_sha1[20];
+ struct strbuf err = STRBUF_INIT;
if (read_ref(b-name, old_sha1))
hashclr(old_sha1);
+
if (is_null_sha1(b-sha1)) {
if (b-delete)
delete_ref(b-name, old_sha1, 0);
return 0;
}
- lock = lock_any_ref_for_update(b-name, old_sha1, 0, NULL);
- if (!lock)
- return error(Unable to lock %s, b-name);
if (!force_update !is_null_sha1(old_sha1)) {
struct commit *old_cmit, *new_cmit;
old_cmit = lookup_commit_reference_gently(old_sha1, 0);
new_cmit = lookup_commit_reference_gently(b-sha1, 0);
if (!old_cmit || !new_cmit) {
- unlock_ref(lock);
return error(Branch %s is missing commits., b-name);
}
if (!in_merge_bases(old_cmit, new_cmit)) {
- unlock_ref(lock);
warning(Not updating %s
(new tip %s does not contain %s),
b-name, sha1_to_hex(b-sha1),
sha1_to_hex(old_sha1));
return -1;
}
}
- if (write_ref_sha1(lock, b-sha1, msg) 0)
- return error(Unable to update %s, b-name);
+ transaction = ref_transaction_begin();
+ if ((!transaction ||
+ ref_transaction_update(transaction, b-name, b-sha1, old_sha1,
+ 0, 1)) ||
+ (ref_transaction_commit(transaction, msg, err)
+!(transaction = NULL))) {
+ ref_transaction_rollback(transaction);
+ error(Unable to update branch %s: %s, b-name, err.buf);
+ strbuf_release(err);
+ return -1;
+ }
return 0;
}
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 17/42] refs.c: change update_ref to use a transaction
Change the update_ref helper function to use a ref transaction internally.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 25 +
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 64e3d53..55aa5a9 100644
--- a/refs.c
+++ b/refs.c
@@ -3405,11 +3405,28 @@ int update_ref(const char *action, const char *refname,
const unsigned char *sha1, const unsigned char *oldval,
int flags, enum action_on_err onerr)
{
- struct ref_lock *lock;
- lock = update_ref_lock(refname, oldval, flags, NULL, onerr);
- if (!lock)
+ struct ref_transaction *t;
+ struct strbuf err = STRBUF_INIT;
+
+ t = ref_transaction_begin();
+ if ((!t ||
+ ref_transaction_update(t, refname, sha1, oldval, flags,
+ !!oldval)) ||
+ (ref_transaction_commit(t, action, err) !(t = NULL))) {
+ const char *str = update_ref failed for ref '%s': %s;
+
+ ref_transaction_rollback(t);
+ switch (onerr) {
+ case UPDATE_REFS_MSG_ON_ERR:
+ error(str, refname, err.buf); break;
+ case UPDATE_REFS_DIE_ON_ERR:
+ die(str, refname, err.buf); break;
+ case UPDATE_REFS_QUIET_ON_ERR: break;
+ }
+ strbuf_release(err);
return 1;
- return update_ref_write(action, refname, sha1, lock, NULL, onerr);
+ }
+ return 0;
}
static int ref_update_compare(const void *r1, const void *r2)
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 19/42] refs.c: ref_transaction_commit should not free the transaction
Change ref_transaction_commit so that it does not free the transaction.
Instead require that a caller will end a transaction by either calling
ref_transaction_rollback or ref_transaction_free.
By having the transaction object remaining valid after _commit returns allows
us to write much nicer code and still be able to call ref_transaction_rollback
safely. Instead of this horribleness
t = ref_transaction_begin();
if ((!t ||
ref_transaction_update(t, refname, sha1, oldval, flags,
!!oldval)) ||
(ref_transaction_commit(t, action, err) !(t = NULL))) {
ref_transaction_rollback(t);
we can now just do the much nicer
t = ref_transaction_begin();
if (!t ||
ref_transaction_update(t, refname, sha1, oldval, flags,
!!oldval) ||
ref_transaction_commit(t, action, err)) {
ref_transaction_rollback(t);
... die/return ...
ref_transaction_free(transaction);
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
branch.c | 1 +
builtin/commit.c | 1 +
builtin/replace.c| 1 +
builtin/tag.c| 1 +
builtin/update-ref.c | 1 +
fast-import.c| 8
refs.c | 14 ++
refs.h | 14 +-
sequencer.c | 8
9 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/branch.c b/branch.c
index 2aa5c82..8e58908 100644
--- a/branch.c
+++ b/branch.c
@@ -305,6 +305,7 @@ void create_branch(const char *head,
ref_transaction_commit(transaction, msg, err))
die_errno(_(%s: failed to write ref: %s),
ref.buf, err.buf);
+ ref_transaction_free(transaction);
}
if (real_ref track)
diff --git a/builtin/commit.c b/builtin/commit.c
index ae1b081..4c577d3 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1726,6 +1726,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
rollback_index_files();
die(_(HEAD: cannot update ref: %s), err.buf);
}
+ ref_transaction_free(transaction);
unlink(git_path(CHERRY_PICK_HEAD));
unlink(git_path(REVERT_HEAD));
diff --git a/builtin/replace.c b/builtin/replace.c
index 11dc2e1..14e8f2f 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -164,6 +164,7 @@ static int replace_object_sha1(const char *object_ref,
ref_transaction_commit(transaction, NULL, err))
die(_(%s: failed to replace ref: %s), ref, err.buf);
+ ref_transaction_free(transaction);
return 0;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index bf2a5c3..fbd2989 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -708,6 +708,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
0, !is_null_sha1(prev)) ||
ref_transaction_commit(transaction, NULL, err))
die(_(%s: cannot update the ref: %s), ref.buf, err.buf);
+ ref_transaction_free(transaction);
if (force !is_null_sha1(prev) hashcmp(prev, object))
printf(_(Updated tag '%s' (was %s)\n), tag,
find_unique_abbrev(prev, DEFAULT_ABBREV));
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index fc3512f..7fe9c11 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -373,6 +373,7 @@ int cmd_update_ref(int argc, const char **argv, const char
*prefix)
update_refs_stdin();
if (ref_transaction_commit(transaction, msg, err))
die(%s, err.buf);
+ ref_transaction_free(transaction);
return 0;
}
diff --git a/fast-import.c b/fast-import.c
index 79d219b..3e356da 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1708,16 +1708,16 @@ static int update_branch(struct branch *b)
}
}
transaction = ref_transaction_begin();
- if ((!transaction ||
+ if (!transaction ||
ref_transaction_update(transaction, b-name, b-sha1, old_sha1,
- 0, 1)) ||
- (ref_transaction_commit(transaction, msg, err)
-!(transaction = NULL))) {
+ 0, 1) ||
+ ref_transaction_commit(transaction, msg, err)) {
ref_transaction_rollback(transaction);
error(Unable to update branch %s: %s, b-name, err.buf);
strbuf_release(err);
return -1;
}
+ ref_transaction_free(transaction);
return 0;
}
diff --git a/refs.c b/refs.c
index a48873c..4673cf4 100644
--- a/refs.c
+++ b/refs.c
@@ -3311,7 +3311,7 @@ struct ref_transaction *ref_transaction_begin(void)
return xcalloc(1, sizeof(struct ref_transaction));
}
-static void
[PATCH v7 16/42] branch.c: use ref transaction for all ref updates
Change create_branch to use a ref transaction when creating the new branch.
ref_transaction_create will check that the ref does not already exist and fail
otherwise meaning that we no longer need to keep a lock on the ref during the
setup_tracking. This simplifies the code since we can now do the transaction
in one single step.
If the forcing flag is false then use ref_transaction_create since this will
fail if the ref already exist. Otherwise use ref_transaction_update.
This also fixes a race condition in the old code where two concurrent
create_branch could race since the lock_any_ref_for_update/write_ref_sha1
did not protect against the ref already existsing. I.e. one thread could end up
overwriting a branch even if the forcing flag is false.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
branch.c | 30 --
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/branch.c b/branch.c
index 660097b..2aa5c82 100644
--- a/branch.c
+++ b/branch.c
@@ -226,7 +226,6 @@ void create_branch(const char *head,
int force, int reflog, int clobber_head,
int quiet, enum branch_track track)
{
- struct ref_lock *lock = NULL;
struct commit *commit;
unsigned char sha1[20];
char *real_ref, msg[PATH_MAX + 20];
@@ -285,15 +284,6 @@ void create_branch(const char *head,
die(_(Not a valid branch point: '%s'.), start_name);
hashcpy(sha1, commit-object.sha1);
- if (!dont_change_ref) {
- lock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL);
- if (!lock)
- die_errno(_(Failed to lock ref for update));
- }
-
- if (reflog)
- log_all_ref_updates = 1;
-
if (forcing)
snprintf(msg, sizeof msg, branch: Reset to %s,
start_name);
@@ -301,13 +291,25 @@ void create_branch(const char *head,
snprintf(msg, sizeof msg, branch: Created from %s,
start_name);
+ if (reflog)
+ log_all_ref_updates = 1;
+
+ if (!dont_change_ref) {
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
+
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_update(transaction, ref.buf, sha1,
+ null_sha1, 0, !forcing) ||
+ ref_transaction_commit(transaction, msg, err))
+ die_errno(_(%s: failed to write ref: %s),
+ ref.buf, err.buf);
+ }
+
if (real_ref track)
setup_tracking(ref.buf + 11, real_ref, track, quiet);
- if (!dont_change_ref)
- if (write_ref_sha1(lock, sha1, msg) 0)
- die_errno(_(Failed to write ref));
-
strbuf_release(ref);
free(real_ref);
}
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 36/42] refs.c: pack all refs before we start to rename a ref
This means that most loose refs will no longer be present after the rename
which triggered a test failure since it assumes the file for an unrelated
ref would still be present after the rename.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c| 3 +++
t/t3200-branch.sh | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index cb14420..157a094 100644
--- a/refs.c
+++ b/refs.c
@@ -2596,6 +2596,9 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
return error(unable to move logfile logs/%s to
TMP_RENAMED_LOG: %s,
oldrefname, strerror(errno));
+ if (pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE))
+ return error(unable to pack refs);
+
if (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) {
error(unable to delete old %s, oldrefname);
goto rollback;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index ac31b71..fafd38a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -289,7 +289,7 @@ test_expect_success 'renaming a symref is not allowed' '
git symbolic-ref refs/heads/master2 refs/heads/master
test_must_fail git branch -m master2 master3
git symbolic-ref refs/heads/master2
- test_path_is_file .git/refs/heads/master
+ test_path_is_missing .git/refs/heads/master
test_path_is_missing .git/refs/heads/master3
'
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 38/42] refs.c: call lock_ref_sha1_basic directly from commit
Skip using the lock_any_ref_for_update wrapper and call lock_ref_sha1_basic
directly from the commit function.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 6556417..a376dd6 100644
--- a/refs.c
+++ b/refs.c
@@ -3476,12 +3476,12 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
for (i = 0; i n; i++) {
struct ref_update *update = updates[i];
- update-lock = lock_any_ref_for_update(update-refname,
- (update-have_old ?
- update-old_sha1 :
- NULL),
- update-flags,
- update-type);
+ update-lock = lock_ref_sha1_basic(update-refname,
+ (update-have_old ?
+ update-old_sha1 :
+ NULL),
+ update-flags,
+ update-type);
if (!update-lock) {
if (err)
strbuf_addf(err, Cannot lock the ref '%s'.,
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 37/42] refs.c: move the check for valid refname to lock_ref_sha1_basic
Move the check for check_refname_format from lock_any_ref_for_update
to lock_ref_sha1_basic. At some later stage we will get rid of
lock_any_ref_for_update completely.
This leaves lock_any_ref_for_updates as a no-op wrapper which could be removed.
But this wrapper is also called from an external caller and we will soon
make changes to the signature to lock_ref_sha1_basic that we do not want to
expose to that caller.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 157a094..6556417 100644
--- a/refs.c
+++ b/refs.c
@@ -2044,6 +2044,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
int missing = 0;
int attempts_remaining = 3;
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
+ return NULL;
+
lock = xcalloc(1, sizeof(struct ref_lock));
lock-lock_fd = -1;
@@ -2135,8 +2138,6 @@ struct ref_lock *lock_any_ref_for_update(const char
*refname,
const unsigned char *old_sha1,
int flags, int *type_p)
{
- if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
- return NULL;
return lock_ref_sha1_basic(refname, old_sha1, flags, type_p);
}
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 31/42] refs.c: remove lock_ref_sha1
lock_ref_sha1 was only called from one place in refc.c and only provided
a check that the refname was sane before adding back the initial refs/
part of the ref path name, the initial refs/ that this caller had already
stripped off before calling lock_ref_sha1.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index aa598ef..cc7556f 100644
--- a/refs.c
+++ b/refs.c
@@ -2126,15 +2126,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
return NULL;
}
-static struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char
*old_sha1)
-{
- char refpath[PATH_MAX];
- if (check_refname_format(refname, 0))
- return NULL;
- strcpy(refpath, mkpath(refs/%s, refname));
- return lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);
-}
-
struct ref_lock *lock_any_ref_for_update(const char *refname,
const unsigned char *old_sha1,
int flags, int *type_p)
@@ -2335,8 +2326,12 @@ static void try_remove_empty_parents(char *name)
/* make sure nobody touched the ref, and unlink */
static void prune_ref(struct ref_to_prune *r)
{
- struct ref_lock *lock = lock_ref_sha1(r-name + 5, r-sha1);
+ struct ref_lock *lock;
+
+ if (check_refname_format(r-name + 5, 0))
+ return;
+ lock = lock_ref_sha1_basic(r-name, r-sha1, 0, NULL);
if (lock) {
unlink_or_warn(git_path(%s, r-name));
unlock_ref(lock);
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 34/42] refs.c: pass the ref log message to _create/delete/update instead of _commit
Change the reference transactions so that we pass the reflog message
through to the create/delete/update function instead of the commit message.
This allows for individual messages for each change in a multi ref
transaction.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
branch.c | 6 +++---
builtin/commit.c | 4 ++--
builtin/fetch.c| 10 --
builtin/receive-pack.c | 4 ++--
builtin/replace.c | 4 ++--
builtin/tag.c | 4 ++--
builtin/update-ref.c | 13 +++--
fast-import.c | 8
refs.c | 34 +-
refs.h | 8
sequencer.c| 4 ++--
walker.c | 6 +++---
12 files changed, 60 insertions(+), 45 deletions(-)
diff --git a/branch.c b/branch.c
index 8e58908..74d55e7 100644
--- a/branch.c
+++ b/branch.c
@@ -300,9 +300,9 @@ void create_branch(const char *head,
transaction = ref_transaction_begin();
if (!transaction ||
- ref_transaction_update(transaction, ref.buf, sha1,
- null_sha1, 0, !forcing) ||
- ref_transaction_commit(transaction, msg, err))
+ ref_transaction_update(transaction, ref.buf, sha1,
+ null_sha1, 0, !forcing, msg) ||
+ ref_transaction_commit(transaction, err))
die_errno(_(%s: failed to write ref: %s),
ref.buf, err.buf);
ref_transaction_free(transaction);
diff --git a/builtin/commit.c b/builtin/commit.c
index 4c577d3..e6bb7b3 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1721,8 +1721,8 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
ref_transaction_update(transaction, HEAD, sha1,
current_head ?
current_head-object.sha1 : NULL,
- 0, !!current_head) ||
- ref_transaction_commit(transaction, sb.buf, err)) {
+ 0, !!current_head, sb.buf) ||
+ ref_transaction_commit(transaction, err)) {
rollback_index_files();
die(_(HEAD: cannot update ref: %s), err.buf);
}
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 5dbd0f0..3a849b0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -374,11 +374,17 @@ static int s_update_ref(const char *action,
struct ref *ref,
int check_old)
{
+ char msg[1024];
+ char *rla = getenv(GIT_REFLOG_ACTION);
+
if (dry_run)
return 0;
+ if (!rla)
+ rla = default_rla.buf;
+ snprintf(msg, sizeof(msg), %s: %s, rla, action);
if (ref_transaction_update(transaction, ref-name, ref-new_sha1,
- ref-old_sha1, 0, check_old))
+ ref-old_sha1, 0, check_old, msg))
return STORE_REF_ERROR_OTHER;
return 0;
@@ -670,7 +676,7 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
}
}
}
- if (ref_transaction_commit(transaction, fetch_ref transaction, NULL))
+ if (ref_transaction_commit(transaction, NULL))
rc |= errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
STORE_REF_ERROR_OTHER;
ref_transaction_free(transaction);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d580176..991c659 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -582,7 +582,7 @@ static const char *update(struct command *cmd, struct
shallow_info *si)
return shallow error;
if (ref_transaction_update(transaction, namespaced_name,
- new_sha1, old_sha1, 0, 1))
+ new_sha1, old_sha1, 0, 1, push))
return failed to update;
return NULL; /* good */
}
@@ -823,7 +823,7 @@ static void execute_commands(struct command *commands,
checked_connectivity = 0;
}
}
- if (ref_transaction_commit(transaction, push, err))
+ if (ref_transaction_commit(transaction, err))
error(%s, err.buf);
ref_transaction_free(transaction);
strbuf_release(err);
diff --git a/builtin/replace.c b/builtin/replace.c
index 14e8f2f..820d703 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -160,8 +160,8 @@ static int replace_object_sha1(const char *object_ref,
transaction = ref_transaction_begin();
if (!transaction ||
ref_transaction_update(transaction, ref, repl, prev,
- 0, !is_null_sha1(prev))
[PATCH v7 35/42] refs.c: pass NULL as *flags to read_ref_full
We call read_ref_full with a pointer to flags from rename_ref but since
we never actually use the returned flags we can just pass NULL here instead.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 9ac9f38..cb14420 100644
--- a/refs.c
+++ b/refs.c
@@ -2601,7 +2601,7 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
goto rollback;
}
- if (!read_ref_full(newrefname, sha1, 1, flag)
+ if (!read_ref_full(newrefname, sha1, 1, NULL)
delete_ref(newrefname, sha1, REF_NODEREF)) {
if (errno==EISDIR) {
if (remove_empty_directories(git_path(%s,
newrefname))) {
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 32/42] refs.c: make prune_ref use a transaction to delete the ref
Change prune_ref to delete the ref using a ref transaction. To do this we also
need to add a new flag REF_ISPRUNING that will tell the transaction that we
do not want to delete this ref from the packed refs. This flag is private to
refs.c and not exposed to external callers.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 27 ---
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index cc7556f..fa5befe 100644
--- a/refs.c
+++ b/refs.c
@@ -29,6 +29,11 @@ static inline int bad_ref_char(int ch)
return 0;
}
+/** Used as a flag to ref_transaction_delete when a loose ref is beeing
+ * pruned.
+ */
+#define REF_ISPRUNING 0x0100
+
/*
* Try to read one refname component from the front of refname. Return
* the length of the component found, or -1 if the component is not
@@ -2326,17 +2331,24 @@ static void try_remove_empty_parents(char *name)
/* make sure nobody touched the ref, and unlink */
static void prune_ref(struct ref_to_prune *r)
{
- struct ref_lock *lock;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
if (check_refname_format(r-name + 5, 0))
return;
- lock = lock_ref_sha1_basic(r-name, r-sha1, 0, NULL);
- if (lock) {
- unlink_or_warn(git_path(%s, r-name));
- unlock_ref(lock);
- try_remove_empty_parents(r-name);
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_delete(transaction, r-name, r-sha1,
+ REF_ISPRUNING, 1) ||
+ ref_transaction_commit(transaction, NULL, err)) {
+ ref_transaction_rollback(transaction);
+ warning(prune_ref: %s, err.buf);
+ strbuf_release(err);
+ return;
}
+ ref_transaction_free(transaction);
+ try_remove_empty_parents(r-name);
}
static void prune_refs(struct ref_to_prune *r)
@@ -3501,8 +3513,9 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (update-lock) {
- delnames[delnum++] = update-lock-ref_name;
ret |= delete_ref_loose(update-lock, update-type);
+ if (!(update-flags REF_ISPRUNING))
+ delnames[delnum++] = update-lock-ref_name;
}
}
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 42/42] refs.c: remove forward declaration of write_ref_sha1
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/refs.c b/refs.c
index 8e12386..aed700b 100644
--- a/refs.c
+++ b/refs.c
@@ -260,8 +260,6 @@ struct ref_entry {
};
static void read_loose_refs(const char *dirname, struct ref_dir *dir);
-static int write_ref_sha1(struct ref_lock *lock,
- const unsigned char *sha1, const char *logmsg);
static struct ref_dir *get_ref_dir(struct ref_entry *entry)
{
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 23/42] receive-pack.c: use a reference transaction for updating the refs
Wrap all the ref updates inside a transaction to make the update atomic.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/receive-pack.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c323081..d580176 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -46,6 +46,8 @@ static void *head_name_to_free;
static int sent_capabilities;
static int shallow_update;
static const char *alt_shallow_file;
+struct strbuf err = STRBUF_INIT;
+struct ref_transaction *transaction;
static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -475,7 +477,6 @@ static const char *update(struct command *cmd, struct
shallow_info *si)
const char *namespaced_name;
unsigned char *old_sha1 = cmd-old_sha1;
unsigned char *new_sha1 = cmd-new_sha1;
- struct ref_lock *lock;
/* only refs/... are allowed */
if (!starts_with(name, refs/) || check_refname_format(name + 5, 0)) {
@@ -580,15 +581,9 @@ static const char *update(struct command *cmd, struct
shallow_info *si)
update_shallow_ref(cmd, si))
return shallow error;
- lock = lock_any_ref_for_update(namespaced_name, old_sha1,
- 0, NULL);
- if (!lock) {
- rp_error(failed to lock %s, name);
- return failed to lock;
- }
- if (write_ref_sha1(lock, new_sha1, push)) {
- return failed to write; /* error() already called */
- }
+ if (ref_transaction_update(transaction, namespaced_name,
+ new_sha1, old_sha1, 0, 1))
+ return failed to update;
return NULL; /* good */
}
}
@@ -812,6 +807,7 @@ static void execute_commands(struct command *commands,
head_name = head_name_to_free = resolve_refdup(HEAD, sha1, 0, NULL);
checked_connectivity = 1;
+ transaction = ref_transaction_begin();
for (cmd = commands; cmd; cmd = cmd-next) {
if (cmd-error_string)
continue;
@@ -827,6 +823,10 @@ static void execute_commands(struct command *commands,
checked_connectivity = 0;
}
}
+ if (ref_transaction_commit(transaction, push, err))
+ error(%s, err.buf);
+ ref_transaction_free(transaction);
+ strbuf_release(err);
if (shallow_update !checked_connectivity)
error(BUG: run 'git fsck' for safety.\n
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 33/42] refs.c: make delete_ref use a transaction
Change delete_ref to use a ref transaction for the deletion. At the same time
since we no longer have any callers of repack_without_ref we can now delete
this function.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 31 ++-
1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/refs.c b/refs.c
index fa5befe..681ac92 100644
--- a/refs.c
+++ b/refs.c
@@ -2482,11 +2482,6 @@ static int repack_without_refs(const char **refnames,
int n)
return commit_packed_refs();
}
-static int repack_without_ref(const char *refname)
-{
- return repack_without_refs(refname, 1);
-}
-
static int delete_ref_loose(struct ref_lock *lock, int flag)
{
if (!(flag REF_ISPACKED) || flag REF_ISSYMREF) {
@@ -2504,24 +2499,18 @@ static int delete_ref_loose(struct ref_lock *lock, int
flag)
int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
{
- struct ref_lock *lock;
- int ret = 0, flag = 0;
+ struct ref_transaction *transaction;
- lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
- if (!lock)
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_delete(transaction, refname, sha1, delopt,
+ sha1 !is_null_sha1(sha1)) ||
+ ref_transaction_commit(transaction, NULL, NULL)) {
+ ref_transaction_rollback(transaction);
return 1;
- ret |= delete_ref_loose(lock, flag);
-
- /* removing the loose one could have resurrected an earlier
-* packed one. Also, if it was not loose we need to repack
-* without it.
-*/
- ret |= repack_without_ref(lock-ref_name);
-
- unlink_or_warn(git_path(logs/%s, lock-ref_name));
- clear_loose_ref_cache(ref_cache);
- unlock_ref(lock);
- return ret;
+ }
+ ref_transaction_free(transaction);
+ return 0;
}
/*
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 22/42] fetch.c: use a single ref transaction for all ref updates
Change store_updated_refs to use a single ref transaction for all refs that
are updated during the fetch. This makes the fetch more atomic when update
failures occur.
Since ref update failures will now no longer occur in the code path for
updating a single ref in s_update_ref, we no longer have as detailed error
message logging the exact reference and the ref log action as in the old cod
Instead since we fail the entire transaction we log a much more generic
message. But since we commit the transaction using MSG_ON_ERR we will log
an error containing the ref name if either locking of writing the ref would
so the regression in the log message is minor.
This will also change the order in which errors are checked for and logged
which may alter which error will be logged if there are multiple errors
occuring during a fetch.
For example, assume we have a fetch for two refs that both would fail.
Where the first ref would fail with ENOTDIR due to a directory in the ref
path not existing, and the second ref in the fetch would fail due to
the check in update_logical_ref():
if (current_branch
!strcmp(ref-name, current_branch-name)
!(update_head_ok || is_bare_repository())
!is_null_sha1(ref-old_sha1)) {
/*
* If this is the head, and it's not okay to update
* the head, and the old value of the head isn't empty...
*/
In the old code since we would update the refs one ref at a time we would
first fail the ENOTDIR and then fail the second update of HEAD as well.
But since the first ref failed with ENOTDIR we would eventually fail the who
fetch with STORE_REF_ERROR_DF_CONFLICT
In the new code, since we defer committing the transaction until all refs
have been processed, we would now detect that the second ref was bad and
rollback the transaction before we would even try start writing the update t
disk and thus we would not return STORE_REF_ERROR_DF_CONFLICT for this case.
I think this new behaviour is more correct, since if there was a problem
we would not even try to commit the transaction but need to highlight this
change in how/what errors are reported.
This change in what error is returned only occurs if there are multiple
refs that fail to update and only some, but not all, of them fail due to
ENOTDIR.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/fetch.c | 34 --
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b41d7b7..5dbd0f0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -45,6 +45,7 @@ static struct transport *gsecondary;
static const char *submodule_prefix = ;
static const char *recurse_submodules_default;
static int shown_url;
+struct ref_transaction *transaction;
static int option_parse_recurse_submodules(const struct option *opt,
const char *arg, int unset)
@@ -373,27 +374,13 @@ static int s_update_ref(const char *action,
struct ref *ref,
int check_old)
{
- char msg[1024];
- char *rla = getenv(GIT_REFLOG_ACTION);
- struct ref_transaction *transaction;
-
if (dry_run)
return 0;
- if (!rla)
- rla = default_rla.buf;
- snprintf(msg, sizeof(msg), %s: %s, rla, action);
- errno = 0;
- transaction = ref_transaction_begin();
- if (!transaction ||
- ref_transaction_update(transaction, ref-name, ref-new_sha1,
- ref-old_sha1, 0, check_old) ||
- ref_transaction_commit(transaction, msg, NULL)) {
- ref_transaction_rollback(transaction);
- return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
- }
- ref_transaction_free(transaction);
+ if (ref_transaction_update(transaction, ref-name, ref-new_sha1,
+ ref-old_sha1, 0, check_old))
+ return STORE_REF_ERROR_OTHER;
+
return 0;
}
@@ -565,6 +552,13 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
goto abort;
}
+ errno = 0;
+ transaction = ref_transaction_begin();
+ if (!transaction) {
+ rc = error(_(cannot start ref transaction\n));
+ goto abort;
+ }
+
/*
* We do a pass for each fetch_head_status type in their enum order, so
* merged entries are written before not-for-merge. That lets readers
@@ -676,6 +670,10 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
}
}
}
+ if (ref_transaction_commit(transaction, fetch_ref transaction, NULL))
+ rc |= errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
+ STORE_REF_ERROR_OTHER;
+
[PATCH v7 21/42] fetch.c: change s_update_ref to use a ref transaction
Change s_update_ref to use a ref transaction for the ref update.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/fetch.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a93c893..b41d7b7 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -375,7 +375,7 @@ static int s_update_ref(const char *action,
{
char msg[1024];
char *rla = getenv(GIT_REFLOG_ACTION);
- static struct ref_lock *lock;
+ struct ref_transaction *transaction;
if (dry_run)
return 0;
@@ -384,15 +384,16 @@ static int s_update_ref(const char *action,
snprintf(msg, sizeof(msg), %s: %s, rla, action);
errno = 0;
- lock = lock_any_ref_for_update(ref-name,
- check_old ? ref-old_sha1 : NULL,
- 0, NULL);
- if (!lock)
- return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
- if (write_ref_sha1(lock, ref-new_sha1, msg) 0)
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_update(transaction, ref-name, ref-new_sha1,
+ ref-old_sha1, 0, check_old) ||
+ ref_transaction_commit(transaction, msg, NULL)) {
+ ref_transaction_rollback(transaction);
return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
STORE_REF_ERROR_OTHER;
+ }
+ ref_transaction_free(transaction);
return 0;
}
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 28/42] refs.c: add transaction.status and track OPEN/CLOSED/ERROR
Track the status of a transaction in a new status field. Check the field for
sanity, i.e. that status must be OPEN when _commit/_create/_delete or
_update is called or else die(BUG:...)
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 37 +
1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index b7fa79b..38193f1 100644
--- a/refs.c
+++ b/refs.c
@@ -3297,6 +3297,12 @@ struct ref_update {
const char refname[FLEX_ARRAY];
};
+enum ref_transaction_status {
+ REF_TRANSACTION_OPEN = 0,
+ REF_TRANSACTION_CLOSED = 1,
+ REF_TRANSACTION_ERROR = 2,
+};
+
/*
* Data structure for holding a reference transaction, which can
* consist of checks and updates to multiple references, carried out
@@ -3306,6 +3312,7 @@ struct ref_transaction {
struct ref_update **updates;
size_t alloc;
size_t nr;
+ enum ref_transaction_status status;
};
struct ref_transaction *ref_transaction_begin(void)
@@ -3329,6 +3336,11 @@ void ref_transaction_free(struct ref_transaction
*transaction)
void ref_transaction_rollback(struct ref_transaction *transaction)
{
+ if (!transaction)
+ return;
+
+ transaction-status = REF_TRANSACTION_ERROR;
+
ref_transaction_free(transaction);
}
@@ -3353,7 +3365,10 @@ int ref_transaction_update(struct ref_transaction
*transaction,
struct ref_update *update;
if (have_old !old_sha1)
- die(have_old is true but old_sha1 is NULL);
+ die(BUG: have_old is true but old_sha1 is NULL);
+
+ if (transaction-status != REF_TRANSACTION_OPEN)
+ die(BUG: update on transaction that is not open);
update = add_update(transaction, refname);
hashcpy(update-new_sha1, new_sha1);
@@ -3372,7 +3387,10 @@ int ref_transaction_create(struct ref_transaction
*transaction,
struct ref_update *update;
if (!new_sha1 || is_null_sha1(new_sha1))
- die(create ref with null new_sha1);
+ die(BUG: create ref with null new_sha1);
+
+ if (transaction-status != REF_TRANSACTION_OPEN)
+ die(BUG: create on transaction that is not open);
update = add_update(transaction, refname);
@@ -3391,7 +3409,10 @@ int ref_transaction_delete(struct ref_transaction
*transaction,
struct ref_update *update;
if (have_old !old_sha1)
- die(have_old is true but old_sha1 is NULL);
+ die(BUG: have_old is true but old_sha1 is NULL);
+
+ if (transaction-status != REF_TRANSACTION_OPEN)
+ die(BUG: delete on transaction that is not open);
update = add_update(transaction, refname);
update-flags = flags;
@@ -3463,8 +3484,13 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
int n = transaction-nr;
struct ref_update **updates = transaction-updates;
- if (!n)
+ if (transaction-status != REF_TRANSACTION_OPEN)
+ die(BUG: commit on transaction that is not open);
+
+ if (!n) {
+ transaction-status = REF_TRANSACTION_CLOSED;
return 0;
+ }
/* Allocate work space */
delnames = xmalloc(sizeof(*delnames) * n);
@@ -3526,6 +3552,9 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
clear_loose_ref_cache(ref_cache);
cleanup:
+ transaction-status = ret ? REF_TRANSACTION_ERROR
+ : REF_TRANSACTION_CLOSED;
+
for (i = 0; i n; i++)
if (updates[i]-lock)
unlock_ref(updates[i]-lock);
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 14/42] sequencer.c: use ref transactions for all ref updates
Change to use ref transactions for all updates to refs.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
sequencer.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 0a80c58..9282a12 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -272,23 +272,31 @@ static int error_dirty_index(struct replay_opts *opts)
static int fast_forward_to(const unsigned char *to, const unsigned char *from,
int unborn, struct replay_opts *opts)
{
- struct ref_lock *ref_lock;
+ struct ref_transaction *transaction;
struct strbuf sb = STRBUF_INIT;
- int ret;
+ struct strbuf err = STRBUF_INIT;
read_cache();
if (checkout_fast_forward(from, to, 1))
exit(1); /* the callee should have complained already */
- ref_lock = lock_any_ref_for_update(HEAD, unborn ? null_sha1 : from,
- 0, NULL);
- if (!ref_lock)
- return error(_(Failed to lock HEAD during fast_forward_to));
strbuf_addf(sb, %s: fast-forward, action_name(opts));
- ret = write_ref_sha1(ref_lock, to, sb.buf);
+
+ transaction = ref_transaction_begin();
+ if ((!transaction ||
+ ref_transaction_update(transaction, HEAD, to, from,
+ 0, !unborn)) ||
+ (ref_transaction_commit(transaction, sb.buf, err)
+!(transaction = NULL))) {
+ ref_transaction_rollback(transaction);
+ error(_(HEAD: Could not fast-forward: %s\n), err.buf);
+ strbuf_release(sb);
+ strbuf_release(err);
+ return -1;
+ }
strbuf_release(sb);
- return ret;
+ return 0;
}
static int do_recursive_merge(struct commit *base, struct commit *next,
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 30/42] refs.c: remove the update_ref_write function
Since we only call update_ref_write from a single place and we only call it
with onerr==QUIET_ON_ERR we can just as well get rid of it and just call
write_ref_sha1 directly.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 35 +--
1 file changed, 9 insertions(+), 26 deletions(-)
diff --git a/refs.c b/refs.c
index 81ba33c..aa598ef 100644
--- a/refs.c
+++ b/refs.c
@@ -3244,25 +3244,6 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
return retval;
}
-static int update_ref_write(const char *action, const char *refname,
- const unsigned char *sha1, struct ref_lock *lock,
- struct strbuf *err, enum action_on_err onerr)
-{
- if (write_ref_sha1(lock, sha1, action) 0) {
- const char *str = Cannot update the ref '%s'.;
- if (err)
- strbuf_addf(err, str, refname);
-
- switch (onerr) {
- case UPDATE_REFS_MSG_ON_ERR: error(str, refname); break;
- case UPDATE_REFS_DIE_ON_ERR: die(str, refname); break;
- case UPDATE_REFS_QUIET_ON_ERR: break;
- }
- return 1;
- }
- return 0;
-}
-
/**
* Information needed for a single ref update. Set new_sha1 to the
* new value or to zero to delete the ref. To check the old value
@@ -3507,14 +3488,16 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (!is_null_sha1(update-new_sha1)) {
- ret = update_ref_write(msg,
- update-refname,
- update-new_sha1,
- update-lock, err,
- UPDATE_REFS_QUIET_ON_ERR);
- update-lock = NULL; /* freed by update_ref_write */
- if (ret)
+ ret = write_ref_sha1(update-lock, update-new_sha1,
+msg);
+ update-lock = NULL; /* freed by write_ref_sha1 */
+ if (ret) {
+ const char *str = Cannot update the ref '%s'.;
+
+ if (err)
+ strbuf_addf(err, str, update-refname);
goto cleanup;
+ }
}
}
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 29/42] refs.c: remove the update_ref_lock function
Since we now only call update_ref_lock with onerr==QUIET_ON_ERR we no longer
need this function and can replace it with just calling lock_any_ref_for_update
directly.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 30 ++
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/refs.c b/refs.c
index 38193f1..81ba33c 100644
--- a/refs.c
+++ b/refs.c
@@ -3244,24 +3244,6 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
return retval;
}
-static struct ref_lock *update_ref_lock(const char *refname,
- const unsigned char *oldval,
- int flags, int *type_p,
- enum action_on_err onerr)
-{
- struct ref_lock *lock;
- lock = lock_any_ref_for_update(refname, oldval, flags, type_p);
- if (!lock) {
- const char *str = Cannot lock the ref '%s'.;
- switch (onerr) {
- case UPDATE_REFS_MSG_ON_ERR: error(str, refname); break;
- case UPDATE_REFS_DIE_ON_ERR: die(str, refname); break;
- case UPDATE_REFS_QUIET_ON_ERR: break;
- }
- }
- return lock;
-}
-
static int update_ref_write(const char *action, const char *refname,
const unsigned char *sha1, struct ref_lock *lock,
struct strbuf *err, enum action_on_err onerr)
@@ -3505,12 +3487,12 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
for (i = 0; i n; i++) {
struct ref_update *update = updates[i];
- update-lock = update_ref_lock(update-refname,
- (update-have_old ?
- update-old_sha1 : NULL),
- update-flags,
- update-type,
- UPDATE_REFS_QUIET_ON_ERR);
+ update-lock = lock_any_ref_for_update(update-refname,
+ (update-have_old ?
+ update-old_sha1 :
+ NULL),
+ update-flags,
+ update-type);
if (!update-lock) {
if (err)
strbuf_addf(err, Cannot lock the ref '%s'.,
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 41/42] refs.c: make rename_ref use a transaction
Change rename_ref to use a single transaction to perform the ref rename.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 73 ++
1 file changed, 20 insertions(+), 53 deletions(-)
diff --git a/refs.c b/refs.c
index 7569f4e..8e12386 100644
--- a/refs.c
+++ b/refs.c
@@ -2591,9 +2591,10 @@ static int rename_tmp_log(const char *newrefname)
int rename_ref(const char *oldrefname, const char *newrefname, const char
*logmsg)
{
- unsigned char sha1[20], orig_sha1[20];
- int flag = 0, logmoved = 0;
- struct ref_lock *lock;
+ unsigned char sha1[20];
+ int flag = 0;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
struct stat loginfo;
int log = !lstat(git_path(logs/%s, oldrefname), loginfo);
const char *symref = NULL;
@@ -2604,7 +2605,7 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
if (log S_ISLNK(loginfo.st_mode))
return error(reflog for %s is a symlink, oldrefname);
- symref = resolve_ref_unsafe(oldrefname, orig_sha1, 1, flag);
+ symref = resolve_ref_unsafe(oldrefname, sha1, 1, flag);
if (flag REF_ISSYMREF)
return error(refname %s is a symbolic ref, renaming it is not
supported,
oldrefname);
@@ -2626,62 +2627,28 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
if (pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE))
return error(unable to pack refs);
- if (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) {
- error(unable to delete old %s, oldrefname);
- goto rollback;
- }
-
- if (!read_ref_full(newrefname, sha1, 1, NULL)
- delete_ref(newrefname, sha1, REF_NODEREF)) {
- if (errno==EISDIR) {
- if (remove_empty_directories(git_path(%s,
newrefname))) {
- error(Directory not empty: %s, newrefname);
- goto rollback;
- }
- } else {
- error(unable to delete existing %s, newrefname);
- goto rollback;
- }
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_delete(transaction, oldrefname, sha1,
+ REF_NODEREF | REF_ISPACKONLY,
+ 1, NULL) ||
+ ref_transaction_update(transaction, newrefname, sha1,
+ NULL, 0, 0, logmsg) ||
+ ref_transaction_commit(transaction, err)) {
+ ref_transaction_rollback(transaction);
+ error(rename_ref failed: %s, err.buf);
+ strbuf_release(err);
+ goto rollbacklog;
}
+ ref_transaction_free(transaction);
if (log rename_tmp_log(newrefname))
- goto rollback;
-
- logmoved = log;
-
- lock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL, NULL, 0);
- if (!lock) {
- error(unable to lock %s for update, newrefname);
- goto rollback;
- }
- lock-force_write = 1;
- hashcpy(lock-old_sha1, orig_sha1);
- if (write_ref_sha1(lock, orig_sha1, logmsg)) {
- error(unable to write current sha1 into %s, newrefname);
- goto rollback;
- }
-
- return 0;
-
- rollback:
- lock = lock_ref_sha1_basic(oldrefname, NULL, 0, NULL, NULL, 0);
- if (!lock) {
- error(unable to lock %s for rollback, oldrefname);
goto rollbacklog;
- }
- lock-force_write = 1;
- flag = log_all_ref_updates;
- log_all_ref_updates = 0;
- if (write_ref_sha1(lock, orig_sha1, NULL))
- error(unable to write current sha1 into %s, oldrefname);
- log_all_ref_updates = flag;
+ return 0;
rollbacklog:
- if (logmoved rename(git_path(logs/%s, newrefname),
git_path(logs/%s, oldrefname)))
- error(unable to restore logfile %s from %s: %s,
- oldrefname, newrefname, strerror(errno));
- if (!logmoved log
+ if (log
rename(git_path(TMP_RENAMED_LOG), git_path(logs/%s, oldrefname)))
error(unable to restore logfile %s from TMP_RENAMED_LOG: %s,
oldrefname, strerror(errno));
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 25/42] walker.c: use ref transaction for ref updates
Switch to using ref transactions in walker_fetch(). As part of the refactoring
to use ref transactions we also fix a potential memory leak where in the
original code if write_ref_sha1() would fail we would end up returning from
the function without free()ing the msg string.
This changes the locking slightly for walker_fetch. Previously the code would
lock all refs before writing them but now we do not lock the refs until the
commit stage. There is thus a very short window where changes could be done
locally during the fetch which would be overwritten when the fetch completes
and commits its transaction. But this window should be reasonably short.
Even if this race does trigger, since both the old code and the new code
just overwrites the refs to the new values without checking or comparing
them with the previous value, this is not too dissimilar to a similar scenario
where you first do a ref change locally and then later do a fetch that
overwrites the local change. With this in mind I do not see the change in
locking semantics to be critical.
Note that this function is only called when fetching from a remote HTTP
repository onto the local (most of the time single-user) repository which
likely means that the type of collissions that the previous locking would
protect against and cause the fetch to fail for to be even more rare.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
walker.c | 51 ++-
1 file changed, 26 insertions(+), 25 deletions(-)
diff --git a/walker.c b/walker.c
index 1dd86b8..6044ccf 100644
--- a/walker.c
+++ b/walker.c
@@ -251,24 +251,18 @@ void walker_targets_free(int targets, char **target,
const char **write_ref)
int walker_fetch(struct walker *walker, int targets, char **target,
const char **write_ref, const char *write_ref_log_details)
{
- struct ref_lock **lock = xcalloc(targets, sizeof(struct ref_lock *));
+ char ref_name[PATH_MAX];
+ struct strbuf err = STRBUF_INIT;
+ struct ref_transaction *transaction;
unsigned char *sha1 = xmalloc(targets * 20);
- char *msg;
- int ret;
+ char *msg = NULL;
int i;
save_commit_buffer = 0;
- for (i = 0; i targets; i++) {
- if (!write_ref || !write_ref[i])
- continue;
-
- lock[i] = lock_ref_sha1(write_ref[i], NULL);
- if (!lock[i]) {
- error(Can't lock ref %s, write_ref[i]);
- goto unlock_and_fail;
- }
- }
+ transaction = ref_transaction_begin();
+ if (!transaction)
+ return -1;
if (!walker-get_recover)
for_each_ref(mark_complete, NULL);
@@ -276,14 +270,14 @@ int walker_fetch(struct walker *walker, int targets, char
**target,
for (i = 0; i targets; i++) {
if (interpret_target(walker, target[i], sha1[20 * i])) {
error(Could not interpret response from server '%s' as
something to pull, target[i]);
- goto unlock_and_fail;
+ goto rollback_and_fail;
}
if (process(walker, lookup_unknown_object(sha1[20 * i])))
- goto unlock_and_fail;
+ goto rollback_and_fail;
}
if (loop(walker))
- goto unlock_and_fail;
+ goto rollback_and_fail;
if (write_ref_log_details) {
msg = xmalloc(strlen(write_ref_log_details) + 12);
@@ -294,19 +288,26 @@ int walker_fetch(struct walker *walker, int targets, char
**target,
for (i = 0; i targets; i++) {
if (!write_ref || !write_ref[i])
continue;
- ret = write_ref_sha1(lock[i], sha1[20 * i], msg ? msg : fetch
(unknown));
- lock[i] = NULL;
- if (ret)
- goto unlock_and_fail;
+ sprintf(ref_name, refs/%s, write_ref[i]);
+ if (ref_transaction_update(transaction, ref_name,
+ sha1[20 * i], NULL,
+ 0, 0))
+ goto rollback_and_fail;
+ }
+
+ if (ref_transaction_commit(transaction, msg ? msg : fetch (unknown),
+ err)) {
+ error(%s, err.buf);
+ goto rollback_and_fail;
}
- free(msg);
+ free(msg);
return 0;
-unlock_and_fail:
- for (i = 0; i targets; i++)
- if (lock[i])
- unlock_ref(lock[i]);
+rollback_and_fail:
+ free(msg);
+ strbuf_release(err);
+ ref_transaction_free(transaction);
return -1;
}
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at
[PATCH v7 27/42] refs.c: make lock_ref_sha1 static
No external callers reference lock_ref_sha1 any more so lets declare it static.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 2 +-
refs.h | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 0e88197..b7fa79b 100644
--- a/refs.c
+++ b/refs.c
@@ -2126,7 +2126,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
return NULL;
}
-struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char
*old_sha1)
+static struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char
*old_sha1)
{
char refpath[PATH_MAX];
if (check_refname_format(refname, 0))
diff --git a/refs.h b/refs.h
index 9e7a1ca..cdea911 100644
--- a/refs.h
+++ b/refs.h
@@ -132,9 +132,6 @@ extern int ref_exists(const char *);
*/
extern int peel_ref(const char *refname, unsigned char *sha1);
-/** Locks a refs/ ref returning the lock on success and NULL on failure. **/
-extern struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char
*old_sha1);
-
/** Locks any ref (for 'HEAD' type refs). */
#define REF_NODEREF0x01
extern struct ref_lock *lock_any_ref_for_update(const char *refname,
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 40/42] refs.c: pass a skip list to name_conflict_fn
Allow passing a list of refs to skip checking to name_conflict_fn.
There are some conditions where we want to allow a temporary conflict and skip
checking those refs. For example if we have a transaction that
1, guarantees that m is a packed refs and there is no loose ref for m
2, the transaction will delete m from the packed ref
3, the transaction will create conflicting m/m
For this case we want to be able to lock and create m/m since we know that the
conflict is only transient. I.e. the conflict will be automatically resolved
by the transaction when it deletes m.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 43 +--
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index 6b8b7fe..7569f4e 100644
--- a/refs.c
+++ b/refs.c
@@ -798,11 +798,19 @@ struct name_conflict_cb {
const char *refname;
const char *oldrefname;
const char *conflicting_refname;
+ const char **skip;
+ int skipnum;
};
static int name_conflict_fn(struct ref_entry *entry, void *cb_data)
{
struct name_conflict_cb *data = (struct name_conflict_cb *)cb_data;
+ int i;
+ for(i = 0; i data-skipnum; i++) {
+ if (!strcmp(entry-name, data-skip[i])) {
+ return 0;
+ }
+ }
if (data-oldrefname !strcmp(data-oldrefname, entry-name))
return 0;
if (names_conflict(data-refname, entry-name)) {
@@ -817,15 +825,21 @@ static int name_conflict_fn(struct ref_entry *entry, void
*cb_data)
* conflicting with the name of an existing reference in dir. If
* oldrefname is non-NULL, ignore potential conflicts with oldrefname
* (e.g., because oldrefname is scheduled for deletion in the same
- * operation).
+ * operation). skip contains a list of refs we want to skip checking for
+ * conflicts with. Refs may be skipped due to us knowing that it will
+ * be deleted later during a transaction that deletes one reference and then
+ * creates a new conflicting reference. For example a rename from m to m/m.
*/
static int is_refname_available(const char *refname, const char *oldrefname,
- struct ref_dir *dir)
+ struct ref_dir *dir,
+ const char **skip, int skipnum)
{
struct name_conflict_cb data;
data.refname = refname;
data.oldrefname = oldrefname;
data.conflicting_refname = NULL;
+ data.skip = skip;
+ data.skipnum = skipnum;
sort_ref_dir(dir);
if (do_for_each_entry_in_dir(dir, 0, name_conflict_fn, data)) {
@@ -2037,7 +2051,8 @@ int dwim_log(const char *str, int len, unsigned char
*sha1, char **log)
static struct ref_lock *lock_ref_sha1_basic(const char *refname,
const unsigned char *old_sha1,
- int flags, int *type_p)
+ int flags, int *type_p,
+ const char **skip, int skipnum)
{
char *ref_file;
const char *orig_refname = refname;
@@ -2084,7 +2099,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
* name is a proper prefix of our refname.
*/
if (missing
-!is_refname_available(refname, NULL, get_packed_refs(ref_cache)))
{
+!is_refname_available(refname, NULL,
+ get_packed_refs(ref_cache),
+ skip, skipnum)) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -2142,7 +2159,7 @@ struct ref_lock *lock_any_ref_for_update(const char
*refname,
const unsigned char *old_sha1,
int flags, int *type_p)
{
- return lock_ref_sha1_basic(refname, old_sha1, flags, type_p);
+ return lock_ref_sha1_basic(refname, old_sha1, flags, type_p, NULL, 0);
}
/*
@@ -2581,6 +2598,9 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
int log = !lstat(git_path(logs/%s, oldrefname), loginfo);
const char *symref = NULL;
+ if (!strcmp(oldrefname, newrefname))
+ return 0;
+
if (log S_ISLNK(loginfo.st_mode))
return error(reflog for %s is a symlink, oldrefname);
@@ -2591,10 +2611,12 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
if (!symref)
return error(refname %s not found, oldrefname);
- if (!is_refname_available(newrefname, oldrefname,
get_packed_refs(ref_cache)))
+ if (!is_refname_available(newrefname, oldrefname,
+ get_packed_refs(ref_cache), NULL, 0))
return 1;
- if (!is_refname_available(newrefname, oldrefname,
[PATCH v7 39/42] refs.c: add a new flag for transaction delete for refs we know are packed only
Add a new flag REF_ISPACKONLY that we can use in ref_transaction_delete.
This flag indicates that the ref does not exist as a loose ref andf only as
a packed ref. If this is the case we then change the commit code so that
we skip taking out a lock file and we skip calling delete_ref_loose.
Check for this flag and die(BUG:...) if used with _update or _create.
At the start of the transaction, before we even start locking any refs,
we add all such REF_ISPACKONLY refs to delnames so that we have a list of
all pack only refs that we will be deleting during this transaction.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/refs.c b/refs.c
index a376dd6..6b8b7fe 100644
--- a/refs.c
+++ b/refs.c
@@ -33,6 +33,10 @@ static inline int bad_ref_char(int ch)
* pruned.
*/
#define REF_ISPRUNING 0x0100
+/** Deletion of a ref that only exists as a packed ref in which case we do not
+ * need to lock the loose ref during the transaction.
+ */
+#define REF_ISPACKONLY 0x0200
/*
* Try to read one refname component from the front of refname. Return
@@ -3335,6 +3339,9 @@ int ref_transaction_update(struct ref_transaction
*transaction,
if (transaction-status != REF_TRANSACTION_OPEN)
die(BUG: update on transaction that is not open);
+ if (flags REF_ISPACKONLY)
+ die(BUG: REF_ISPACKONLY can not be used with updates);
+
update = add_update(transaction, refname);
hashcpy(update-new_sha1, new_sha1);
update-flags = flags;
@@ -3359,6 +3366,9 @@ int ref_transaction_create(struct ref_transaction
*transaction,
if (transaction-status != REF_TRANSACTION_OPEN)
die(BUG: create on transaction that is not open);
+ if (flags REF_ISPACKONLY)
+ die(BUG: REF_ISPACKONLY can not be used with creates);
+
update = add_update(transaction, refname);
hashcpy(update-new_sha1, new_sha1);
@@ -3472,10 +3482,20 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
if (ret)
goto cleanup;
+ for (i = 0; i n; i++) {
+ struct ref_update *update = updates[i];
+
+ if (update-flags REF_ISPACKONLY)
+ delnames[delnum++] = update-refname;
+ }
+
/* Acquire all locks while verifying old values */
for (i = 0; i n; i++) {
struct ref_update *update = updates[i];
+ if (update-flags REF_ISPACKONLY)
+ continue;
+
update-lock = lock_ref_sha1_basic(update-refname,
(update-have_old ?
update-old_sha1 :
@@ -3513,6 +3533,9 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
for (i = 0; i n; i++) {
struct ref_update *update = updates[i];
+ if (update-flags REF_ISPACKONLY)
+ continue;
+
if (update-lock) {
ret |= delete_ref_loose(update-lock, update-type);
if (!(update-flags REF_ISPRUNING))
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 26/42] refs.c: make write_ref_sha1 static
No external users call write_ref_sha1 any more so lets declare it static.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 4 +++-
refs.h | 3 ---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 4673cf4..0e88197 100644
--- a/refs.c
+++ b/refs.c
@@ -251,6 +251,8 @@ struct ref_entry {
};
static void read_loose_refs(const char *dirname, struct ref_dir *dir);
+static int write_ref_sha1(struct ref_lock *lock,
+ const unsigned char *sha1, const char *logmsg);
static struct ref_dir *get_ref_dir(struct ref_entry *entry)
{
@@ -2794,7 +2796,7 @@ static int is_branch(const char *refname)
return !strcmp(refname, HEAD) || starts_with(refname, refs/heads/);
}
-int write_ref_sha1(struct ref_lock *lock,
+static int write_ref_sha1(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg)
{
static char term = '\n';
diff --git a/refs.h b/refs.h
index b7535b5..9e7a1ca 100644
--- a/refs.h
+++ b/refs.h
@@ -150,9 +150,6 @@ extern int commit_ref(struct ref_lock *lock);
/** Release any lock taken but not written. **/
extern void unlock_ref(struct ref_lock *lock);
-/** Writes sha1 into the ref specified by the lock. **/
-extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
const char *msg);
-
/** Setup reflog before using. **/
int log_ref_setup(const char *refname, char *logfile, int bufsize);
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 24/42] fast-import.c: use a ref transaction when dumping tags
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
fast-import.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 3e356da..5587cf6 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1736,15 +1736,22 @@ static void dump_tags(void)
{
static const char *msg = fast-import;
struct tag *t;
- struct ref_lock *lock;
char ref_name[PATH_MAX];
+ struct strbuf err = STRBUF_INIT;
+ struct ref_transaction *transaction;
+ transaction = ref_transaction_begin();
for (t = first_tag; t; t = t-next_tag) {
- sprintf(ref_name, tags/%s, t-name);
- lock = lock_ref_sha1(ref_name, NULL);
- if (!lock || write_ref_sha1(lock, t-sha1, msg) 0)
- failure |= error(Unable to update %s, ref_name);
+ sprintf(ref_name, refs/tags/%s, t-name);
+
+ if (ref_transaction_update(transaction, ref_name, t-sha1,
+ NULL, 0, 0))
+ failure |= error(Unable to update %s, err.buf);
}
+ if (failure || ref_transaction_commit(transaction, msg, err))
+ failure |= error(Unable to update %s, err.buf);
+ ref_transaction_free(transaction);
+ strbuf_release(err);
}
static void dump_marks_helper(FILE *f,
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 08/42] refs.c: change ref_transaction_update() to do error checking and return status
Update ref_transaction_update() do some basic error checking and return
non-zero on error. Update all callers to check ref_transaction_update() for
error. There are currently no conditions in _update that will return error but
there will be in the future.
Also check for BUGs during update and die(BUG:...) if we are calling
_update with have_old but the old_sha1 pointer is NULL.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/update-ref.c | 10 ++
refs.c | 9 +++--
refs.h | 11 ++-
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 2bef2a0..59c4d6b 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -197,8 +197,9 @@ static const char *parse_cmd_update(struct strbuf *input,
const char *next)
if (*next != line_termination)
die(update %s: extra input: %s, refname, next);
- ref_transaction_update(transaction, refname, new_sha1, old_sha1,
- update_flags, have_old);
+ if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
+ update_flags, have_old))
+ die(update %s: failed, refname);
update_flags = 0;
free(refname);
@@ -286,8 +287,9 @@ static const char *parse_cmd_verify(struct strbuf *input,
const char *next)
if (*next != line_termination)
die(verify %s: extra input: %s, refname, next);
- ref_transaction_update(transaction, refname, new_sha1, old_sha1,
- update_flags, have_old);
+ if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
+ update_flags, have_old))
+ die(failed transaction update for %s, refname);
update_flags = 0;
free(refname);
diff --git a/refs.c b/refs.c
index bfba348..77b27ce 100644
--- a/refs.c
+++ b/refs.c
@@ -3342,19 +3342,24 @@ static struct ref_update *add_update(struct
ref_transaction *transaction,
return update;
}
-void ref_transaction_update(struct ref_transaction *transaction,
+int ref_transaction_update(struct ref_transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
const unsigned char *old_sha1,
int flags, int have_old)
{
- struct ref_update *update = add_update(transaction, refname);
+ struct ref_update *update;
+
+ if (have_old !old_sha1)
+ die(have_old is true but old_sha1 is NULL);
+ update = add_update(transaction, refname);
hashcpy(update-new_sha1, new_sha1);
update-flags = flags;
update-have_old = have_old;
if (have_old)
hashcpy(update-old_sha1, old_sha1);
+ return 0;
}
void ref_transaction_create(struct ref_transaction *transaction,
diff --git a/refs.h b/refs.h
index 555ee59..57103aa 100644
--- a/refs.h
+++ b/refs.h
@@ -242,12 +242,13 @@ void ref_transaction_rollback(struct ref_transaction
*transaction);
* be deleted. If have_old is true, then old_sha1 holds the value
* that the reference should have had before the update, or zeros if
* it must not have existed beforehand.
+ * Function returns 0 on success and non-zero on failure.
*/
-void ref_transaction_update(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *new_sha1,
- const unsigned char *old_sha1,
- int flags, int have_old);
+int ref_transaction_update(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ const unsigned char *old_sha1,
+ int flags, int have_old);
/*
* Add a reference creation to transaction. new_sha1 is the value
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 07/42] refs.c: remove the onerr argument to ref_transaction_commit
Since all callers now use QUIET_ON_ERR we no longer need to provide an onerr
argument any more. Remove the onerr argument from the ref_transaction_commit
signature.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/update-ref.c | 3 +--
refs.c | 22 +++---
refs.h | 3 +--
3 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 207e24d..2bef2a0 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -366,8 +366,7 @@ int cmd_update_ref(int argc, const char **argv, const char
*prefix)
if (end_null)
line_termination = '\0';
update_refs_stdin();
- if (ref_transaction_commit(transaction, msg, err,
- UPDATE_REFS_QUIET_ON_ERR))
+ if (ref_transaction_commit(transaction, msg, err))
die(%s, err.buf);
return 0;
}
diff --git a/refs.c b/refs.c
index 8051aac..bfba348 100644
--- a/refs.c
+++ b/refs.c
@@ -3405,8 +3405,7 @@ static int ref_update_compare(const void *r1, const void
*r2)
}
static int ref_update_reject_duplicates(struct ref_update **updates, int n,
- struct strbuf *err,
- enum action_on_err onerr)
+ struct strbuf *err)
{
int i;
for (i = 1; i n; i++)
@@ -3416,22 +3415,13 @@ static int ref_update_reject_duplicates(struct
ref_update **updates, int n,
if (err)
strbuf_addf(err, str, updates[i]-refname);
- switch (onerr) {
- case UPDATE_REFS_MSG_ON_ERR:
- error(str, updates[i]-refname); break;
- case UPDATE_REFS_DIE_ON_ERR:
- die(str, updates[i]-refname); break;
- case UPDATE_REFS_QUIET_ON_ERR:
- break;
- }
return 1;
}
return 0;
}
int ref_transaction_commit(struct ref_transaction *transaction,
- const char *msg, struct strbuf *err,
- enum action_on_err onerr)
+ const char *msg, struct strbuf *err)
{
int ret = 0, delnum = 0, i;
const char **delnames;
@@ -3446,7 +3436,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
/* Copy, sort, and reject duplicate refs */
qsort(updates, n, sizeof(*updates), ref_update_compare);
- ret = ref_update_reject_duplicates(updates, n, err, onerr);
+ ret = ref_update_reject_duplicates(updates, n, err);
if (ret)
goto cleanup;
@@ -3458,7 +3448,8 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
(update-have_old ?
update-old_sha1 : NULL),
update-flags,
- update-type, onerr);
+ update-type,
+ UPDATE_REFS_QUIET_ON_ERR);
if (!update-lock) {
if (err)
strbuf_addf(err, Cannot lock the ref '%s'.,
@@ -3476,7 +3467,8 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
ret = update_ref_write(msg,
update-refname,
update-new_sha1,
- update-lock, err, onerr);
+ update-lock, err,
+ UPDATE_REFS_QUIET_ON_ERR);
update-lock = NULL; /* freed by update_ref_write */
if (ret)
goto cleanup;
diff --git a/refs.h b/refs.h
index 25ae110..555ee59 100644
--- a/refs.h
+++ b/refs.h
@@ -278,8 +278,7 @@ void ref_transaction_delete(struct ref_transaction
*transaction,
* the transaction failed. The string does not end in newline.
*/
int ref_transaction_commit(struct ref_transaction *transaction,
- const char *msg, struct strbuf *err,
- enum action_on_err onerr);
+ const char *msg, struct strbuf *err);
/** Lock a ref and then write its file */
int update_ref(const char *action, const char *refname,
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Please pull the patch series use the $( ... ) construct for command substitution
On Wed, May 14, 2014 at 1:14 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Elia Pinto gitter.spi...@gmail.com writes: The following changes since commit 6308767f0bb58116cb405e1f4f77f5dfc1589920: Merge branch 'fc/prompt-zsh-read-from-file' (2014-05-13 11:53:14 -0700) are available in the git repository at: https://github.com/devzero2000/git-core.git ep/shell-command-substitution-v4 There's a mis-replacement of multiple `..` `..` on the same line in t9300-fast-import.sh. I've sent you a pull request with a fixup. I'm not sure about this one: commit e69c77e580d56d587381066f56027c8a596c237a Author: Elia Pinto gitter.spi...@gmail.com Date: Wed May 14 03:28:11 2014 -0700 t9137-git-svn-dcommit-clobber-series.sh: use the $( ... ) construct for command substitution [...] @@ -38,20 +38,20 @@ test_expect_success 'some unrelated changes to git' test_expect_success 'change file but in unrelated area' - test x\\`sed -n -e 4p file\`\ = x4 - test x\\`sed -n -e 7p file\`\ = x7 + test x$(sed -n -e 4p file) = x4 + test x$(sed -n -e 7p file) = x7 ^ here We're inside from the test_expect_success line. We used to have a literal (because of the backslash), we now have a closing because you removed the \. So, the sed command used to be protected by double-quotes, and it is now outside them. Compare: $ sh -c echo \\`date\`\ Wed May 14 18:47:54 MEST 2014 $ sh -c echo $(date) Wed In your case, it doesn't break because the expected output of sed contains no space, but that seems dangerous to me. I do not understand the use of the \ in front of the ` in the original code. The second argument of test_expect_success is double-quoted, so a bare `...` would be evaluated before test_expect_success is even invoked. Quoting it as \'...\' correctly suppresses the automatic evaluation, giving test_expect_success the opportunity to evaluate it on-demand. In this case, it doesn't matter since file is populated outside the invocation of test_expect_success, but it would matter if file was populated or modified within the test itself. In that sense, the original code with delayed \`...\` evaluation is more robust and future-proof. The correct code should be test x\$(sed -n -e 4p file)\ = x4 I guess. Same issue. The $(...) is being evaluated even before test_expect_success is invoked. Better would be to suspend evaluation via \$(...) to allow test_expect_success to evaluate it on-demand. perl -i.bak -p -e 's/^4\$//' file perl -i.bak -p -e 's/^7\$//' file - test x\\`sed -n -e 4p file\`\ = x - test x\\`sed -n -e 7p file\`\ = x + test x$(sed -n -e 4p file) = x + test x$(sed -n -e 7p file) = x Likewise. - test x\\`sed -n -e 4p file\`\ = x - test x\\`sed -n -e 7p file\`\ = x - test x\\`sed -n -e 58p file\`\ = x5588 - test x\\`sed -n -e 61p file\`\ = x6611 + test x$(sed -n -e 4p file) = x + test x$(sed -n -e 7p file) = x + test x$(sed -n -e 58p file) = x5588 + test x$(sed -n -e 61p file) = x6611 Likewise. More or less the same issue with commit 020568b9c36c023810a3482b7b73bcadd6406a85 Author: Elia Pinto gitter.spi...@gmail.com Date: Mon Apr 28 05:49:50 2014 -0700 t9114-git-svn-dcommit-merge.sh: use the $( ... ) construct for command substitution [...] diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh index fb41876..cf2e25f 100755 --- a/t/t9114-git-svn-dcommit-merge.sh +++ b/t/t9114-git-svn-dcommit-merge.sh @@ -68,8 +68,8 @@ test_expect_success 'setup git mirror and merge' ' test_debug 'gitk --all sleep 1' test_expect_success 'verify pre-merge ancestry' - test x\`git rev-parse --verify refs/heads/svn^2\` = \ -x\`git rev-parse --verify refs/heads/merge\` + test x\$(git rev-parse --verify refs/heads/svn^2\) = \ +x\$(git rev-parse --verify refs/heads/merge\) git cat-file commit refs/heads/svn^ | grep '^friend$' I'm not sure what's the intent of the \ in front of ` in the original code, but changing it to $(...) changes the meaning: $ sh -c echo \`date\` Wed May 14 18:58:19 MEST 2014 $ sh -c echo \$(date\) sh: 1: Syntax error: end of file unexpected (expecting )) I didn't investigate closely, but I'm getting test failures without your patch, and the script stops in the middle with it so it does break something. @@ -80,10 +80,10 @@ test_expect_success 'git svn dcommit merges' test_debug 'gitk --all sleep 1' test_expect_success 'verify post-merge ancestry' - test x\`git rev-parse --verify refs/heads/svn\` = \ -x\`git rev-parse --verify refs/remotes/origin/trunk \` - test x\`git rev-parse --verify
[PATCH v7 11/42] tag.c: use ref transactions when doing updates
Change tag.c to use ref transactions for all ref updates.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/tag.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index c6e8a71..bf2a5c3 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -548,7 +548,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct strbuf ref = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
- struct ref_lock *lock;
struct create_tag_options opt;
char *cleanup_arg = NULL;
int annotate = 0, force = 0, lines = -1;
@@ -556,6 +555,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
struct option options[] = {
OPT_CMDMODE('l', list, cmdmode, N_(list tag names), 'l'),
{ OPTION_INTEGER, 'n', NULL, lines, N_(n),
@@ -701,11 +702,12 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
if (annotate)
create_tag(object, tag, buf, opt, prev, object);
- lock = lock_any_ref_for_update(ref.buf, prev, 0, NULL);
- if (!lock)
- die(_(%s: cannot lock the ref), ref.buf);
- if (write_ref_sha1(lock, object, NULL) 0)
- die(_(%s: cannot update the ref), ref.buf);
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_update(transaction, ref.buf, object, prev,
+ 0, !is_null_sha1(prev)) ||
+ ref_transaction_commit(transaction, NULL, err))
+ die(_(%s: cannot update the ref: %s), ref.buf, err.buf);
if (force !is_null_sha1(prev) hashcmp(prev, object))
printf(_(Updated tag '%s' (was %s)\n), tag,
find_unique_abbrev(prev, DEFAULT_ABBREV));
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 09/42] refs.c: change ref_transaction_create to do error checking and return status
Do basic error checking in ref_transaction_create() and make it return
non-zero on error. Update all callers to check the result of
ref_transaction_create(). There are currently no conditions in _create that
will return error but there will be in the future.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/update-ref.c | 4 +++-
refs.c | 17 +++--
refs.h | 9 +
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 59c4d6b..3fab810 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -225,7 +225,9 @@ static const char *parse_cmd_create(struct strbuf *input,
const char *next)
if (*next != line_termination)
die(create %s: extra input: %s, refname, next);
- ref_transaction_create(transaction, refname, new_sha1, update_flags);
+ if (ref_transaction_create(transaction, refname, new_sha1,
+ update_flags))
+ die(failed transaction create for %s, refname);
update_flags = 0;
free(refname);
diff --git a/refs.c b/refs.c
index 77b27ce..df376fa 100644
--- a/refs.c
+++ b/refs.c
@@ -3362,18 +3362,23 @@ int ref_transaction_update(struct ref_transaction
*transaction,
return 0;
}
-void ref_transaction_create(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *new_sha1,
- int flags)
+int ref_transaction_create(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ int flags)
{
- struct ref_update *update = add_update(transaction, refname);
+ struct ref_update *update;
+
+ if (!new_sha1 || is_null_sha1(new_sha1))
+ die(create ref with null new_sha1);
+
+ update = add_update(transaction, refname);
- assert(!is_null_sha1(new_sha1));
hashcpy(update-new_sha1, new_sha1);
hashclr(update-old_sha1);
update-flags = flags;
update-have_old = 1;
+ return 0;
}
void ref_transaction_delete(struct ref_transaction *transaction,
diff --git a/refs.h b/refs.h
index 57103aa..d4c068d 100644
--- a/refs.h
+++ b/refs.h
@@ -255,11 +255,12 @@ int ref_transaction_update(struct ref_transaction
*transaction,
* that the reference should have after the update; it must not be the
* null SHA-1. It is verified that the reference does not exist
* already.
+ * Function returns 0 on success and non-zero on failure.
*/
-void ref_transaction_create(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *new_sha1,
- int flags);
+int ref_transaction_create(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ int flags);
/*
* Add a reference deletion to transaction. If have_old is true, then
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 00/42] Use ref transactions for all ref updates
This patch series can also be found at https://github.com/rsahlberg/git/tree/ref-transactions This patch series is based on next and expands on the transaction API. It converts all ref updates, inside refs.c as well as external, to use the transaction API for updates. This makes most of the ref updates to become atomic when there are failures locking or writing to a ref. This version completes the work to convert all ref updates to use transactions. Now that all updates are through transactions I will start working on cleaning up the reading of refs and to create an api for managing reflogs but all that will go in a different patch series. Version 7: - Updated commit messages per JNs review comments. - Changed REF_ISPRUNING and REF_ISPACKONLY to be private flags and not exposed through refs.h Version 6: - Convert all updates in refs.c to use transactions too. Version 5: - Reword commit messages for having _create/_delete/_update returning success/failure. There are no conditions yet that return an error from these failures but there will be in the future. So we still check the return from these functions in the callers in preparation for this. - Don't leak memory by just passing a strbuf_detach() pointer to functions. Use obj.buf and explicitely strbuf_release the data afterwards. - Remove the function update_ref_lock. - Remove the function update_ref_write. - Track transaction status and die(BUG:) if we call _create/_delete/_update/ _commit for a transaction that is not OPEN. Version 4: - Rename patch series from Use ref transactions from most callers to Use ref transactions for all ref updates. - Convert all external ref writes to use transactions and make write_ref_sha1 and lock_ref_sha1 static functions. - Change the ref commit and free handling so we no longer pass pointer to pointer to _commit. _commit no longer frees the transaction. The caller MUST call _free itself. - Change _commit to take a strbuf pointer instead of a char* for error reporting back to the caller. - Re-add the walker patch after fixing it. Version 3: - Remove the walker patch for now. Walker needs more complex solution so defer it until the basics are done. - Remove the onerr argument to ref_transaction_commit(). All callers that need to die() on error now have to do this explicitely. - Pass an error string from ref_transaction_commit() back to the callers so that they can craft a nice error message upon failures. - Make ref_transaction_rollback() accept NULL as argument. - Change ref_transaction_commit() to take a pointer to pointer argument for the transaction and have it clear the callers pointer to NULL when invoked. This allows for much nicer handling of transaction rollback on failure. Version 2: - Add a patch to ref_transaction_commit to make it honor onerr even if the error triggered in ref_Transaction_commit itself rather than in a call to other functions (that already honor onerr). - Add a patch to make the update_ref() helper function use transactions internally. - Change ref_transaction_update to die() instead of error() if we pass if a NULL old_sha1 but have have_old == true. - Change ref_transaction_create to die() instead of error() if new_sha1 is false but we pass it a null_sha1. - Change ref_transaction_delete die() instead of error() if we pass if a NULL old_sha1 but have have_old == true. - Change several places to do if(!transaction || ref_transaction_update() || ref_Transaction_commit()) die(generic-message) instead of checking each step separately and having a different message for each failure. Most users are likely not interested in what step of the transaction failed and only whether it failed or not. - Change commit.c to only pass a pointer to ref_transaction_update iff current_head is non-NULL. The previous patch used to compute a garbage pointer for current_head-object.sha1 and relied on the fact that ref_transaction_update would not try to dereference this pointer if !!current_head was 0. - Updated commit message for the walker_fetch change to try to justify why the change in locking semantics should not be harmful. Ronnie Sahlberg (42): refs.c: constify the sha arguments for ref_transaction_create|delete|update refs.c: allow passing NULL to ref_transaction_free refs.c: add a strbuf argument to ref_transaction_commit for error logging refs.c: make ref_update_reject_duplicates take a strbuf argument for errors update-ref.c: log transaction error from the update_ref refs.c: make update_ref_write update a strbuf on failure refs.c: remove the onerr argument to ref_transaction_commit refs.c: change ref_transaction_update() to do error checking and return status refs.c: change ref_transaction_create to do error checking and return status refs.c: ref_transaction_delete to check for error and return status tag.c: use ref transactions
[PATCH v7 06/42] refs.c: make update_ref_write update a strbuf on failure
Change update_ref_write to also update an error strbuf on failure.
This makes the error available to ref_transaction_commit callers if the
transaction failed due to update_ref_sha1/write_ref_sha1 failures.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index 364daf0..8051aac 100644
--- a/refs.c
+++ b/refs.c
@@ -3262,10 +3262,13 @@ static struct ref_lock *update_ref_lock(const char
*refname,
static int update_ref_write(const char *action, const char *refname,
const unsigned char *sha1, struct ref_lock *lock,
- enum action_on_err onerr)
+ struct strbuf *err, enum action_on_err onerr)
{
if (write_ref_sha1(lock, sha1, action) 0) {
const char *str = Cannot update the ref '%s'.;
+ if (err)
+ strbuf_addf(err, str, refname);
+
switch (onerr) {
case UPDATE_REFS_MSG_ON_ERR: error(str, refname); break;
case UPDATE_REFS_DIE_ON_ERR: die(str, refname); break;
@@ -3391,7 +3394,7 @@ int update_ref(const char *action, const char *refname,
lock = update_ref_lock(refname, oldval, flags, NULL, onerr);
if (!lock)
return 1;
- return update_ref_write(action, refname, sha1, lock, onerr);
+ return update_ref_write(action, refname, sha1, lock, NULL, onerr);
}
static int ref_update_compare(const void *r1, const void *r2)
@@ -3473,7 +3476,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
ret = update_ref_write(msg,
update-refname,
update-new_sha1,
- update-lock, onerr);
+ update-lock, err, onerr);
update-lock = NULL; /* freed by update_ref_write */
if (ret)
goto cleanup;
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 01/42] refs.c: constify the sha arguments for ref_transaction_create|delete|update
ref_transaction_create|delete|update has no need to modify the sha1
arguments passed to it so it should use const unsigned char* instead
of unsigned char*.
Some functions, such as fast_forward_to(), already have its old/new
sha1 arguments as consts. This function will at some point need to
use ref_transaction_update() in which case this change is required.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 7 ---
refs.h | 7 ---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 6898263..2114748 100644
--- a/refs.c
+++ b/refs.c
@@ -3338,7 +3338,8 @@ static struct ref_update *add_update(struct
ref_transaction *transaction,
void ref_transaction_update(struct ref_transaction *transaction,
const char *refname,
- unsigned char *new_sha1, unsigned char *old_sha1,
+ const unsigned char *new_sha1,
+ const unsigned char *old_sha1,
int flags, int have_old)
{
struct ref_update *update = add_update(transaction, refname);
@@ -3352,7 +3353,7 @@ void ref_transaction_update(struct ref_transaction
*transaction,
void ref_transaction_create(struct ref_transaction *transaction,
const char *refname,
- unsigned char *new_sha1,
+ const unsigned char *new_sha1,
int flags)
{
struct ref_update *update = add_update(transaction, refname);
@@ -3366,7 +3367,7 @@ void ref_transaction_create(struct ref_transaction
*transaction,
void ref_transaction_delete(struct ref_transaction *transaction,
const char *refname,
- unsigned char *old_sha1,
+ const unsigned char *old_sha1,
int flags, int have_old)
{
struct ref_update *update = add_update(transaction, refname);
diff --git a/refs.h b/refs.h
index 09ff483..6d97700 100644
--- a/refs.h
+++ b/refs.h
@@ -245,7 +245,8 @@ void ref_transaction_rollback(struct ref_transaction
*transaction);
*/
void ref_transaction_update(struct ref_transaction *transaction,
const char *refname,
- unsigned char *new_sha1, unsigned char *old_sha1,
+ const unsigned char *new_sha1,
+ const unsigned char *old_sha1,
int flags, int have_old);
/*
@@ -256,7 +257,7 @@ void ref_transaction_update(struct ref_transaction
*transaction,
*/
void ref_transaction_create(struct ref_transaction *transaction,
const char *refname,
- unsigned char *new_sha1,
+ const unsigned char *new_sha1,
int flags);
/*
@@ -266,7 +267,7 @@ void ref_transaction_create(struct ref_transaction
*transaction,
*/
void ref_transaction_delete(struct ref_transaction *transaction,
const char *refname,
- unsigned char *old_sha1,
+ const unsigned char *old_sha1,
int flags, int have_old);
/*
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 12/42] replace.c: use the ref transaction functions for updates
Update replace.c to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/replace.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 3da1bae..11dc2e1 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -133,7 +133,8 @@ static int replace_object_sha1(const char *object_ref, unsigned char prev[20]; enum object_type obj_type, repl_type; char ref[PATH_MAX]; - struct ref_lock *lock; + struct ref_transaction *transaction; + struct strbuf err = STRBUF_INIT; if (snprintf(ref, sizeof(ref), refs/replace/%s, @@ -156,11 +157,12 @@ static int replace_object_sha1(const char *object_ref, else if (!force) die(replace ref '%s' already exists, ref); - lock = lock_any_ref_for_update(ref, prev, 0, NULL); - if (!lock) - die(%s: cannot lock the ref, ref); - if (write_ref_sha1(lock, repl, NULL) 0) - die(%s: cannot update the ref, ref); + transaction = ref_transaction_begin(); + if (!transaction || + ref_transaction_update(transaction, ref, repl, prev, + 0, !is_null_sha1(prev)) || + ref_transaction_commit(transaction, NULL, err)) + die(_(%s: failed to replace ref: %s), ref, err.buf); return 0; } -- 2.0.0.rc3.471.g2055d11.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 13/42] commit.c: use ref transactions for updates
Change commit.c to use ref transactions for all ref updates.
Make sure we pass a NULL pointer to ref_transaction_update if have_old
is false.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/commit.c | 23 ++-
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index d28505a..ae1b081 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1581,11 +1581,12 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
const char *index_file, *reflog_msg;
char *nl;
unsigned char sha1[20];
- struct ref_lock *ref_lock;
struct commit_list *parents = NULL, **pptr = parents;
struct stat statbuf;
struct commit *current_head = NULL;
struct commit_extra_header *extra = NULL;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
if (argc == 2 !strcmp(argv[1], -h))
usage_with_options(builtin_commit_usage,
builtin_commit_options);
@@ -1707,16 +1708,6 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
strbuf_release(author_ident);
free_commit_extra_headers(extra);
- ref_lock = lock_any_ref_for_update(HEAD,
- !current_head
- ? NULL
- : current_head-object.sha1,
- 0, NULL);
- if (!ref_lock) {
- rollback_index_files();
- die(_(cannot lock HEAD ref));
- }
-
nl = strchr(sb.buf, '\n');
if (nl)
strbuf_setlen(sb, nl + 1 - sb.buf);
@@ -1725,9 +1716,15 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
strbuf_insert(sb, 0, reflog_msg, strlen(reflog_msg));
strbuf_insert(sb, strlen(reflog_msg), : , 2);
- if (write_ref_sha1(ref_lock, sha1, sb.buf) 0) {
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_update(transaction, HEAD, sha1,
+ current_head ?
+ current_head-object.sha1 : NULL,
+ 0, !!current_head) ||
+ ref_transaction_commit(transaction, sb.buf, err)) {
rollback_index_files();
- die(_(cannot update HEAD ref));
+ die(_(HEAD: cannot update ref: %s), err.buf);
}
unlink(git_path(CHERRY_PICK_HEAD));
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 03/42] refs.c: add a strbuf argument to ref_transaction_commit for error logging
Add a strbuf argument to _commit so that we can pass an error string back to
the caller. So that we can do error logging from the caller instead of from
_commit.
Longer term plan is to first convert all callers to use onerr==QUIET_ON_ERR
and craft any log messages from the callers themselves and finally remove the
onerr argument completely.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/update-ref.c | 2 +-
refs.c | 6 +-
refs.h | 5 -
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 405267f..aaa06aa 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -367,7 +367,7 @@ int cmd_update_ref(int argc, const char **argv, const char
*prefix)
if (end_null)
line_termination = '\0';
update_refs_stdin();
- ret = ref_transaction_commit(transaction, msg,
+ ret = ref_transaction_commit(transaction, msg, NULL,
UPDATE_REFS_DIE_ON_ERR);
return ret;
}
diff --git a/refs.c b/refs.c
index 88d73c8..1a7f9d9 100644
--- a/refs.c
+++ b/refs.c
@@ -3423,7 +3423,8 @@ static int ref_update_reject_duplicates(struct ref_update
**updates, int n,
}
int ref_transaction_commit(struct ref_transaction *transaction,
- const char *msg, enum action_on_err onerr)
+ const char *msg, struct strbuf *err,
+ enum action_on_err onerr)
{
int ret = 0, delnum = 0, i;
const char **delnames;
@@ -3452,6 +3453,9 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-flags,
update-type, onerr);
if (!update-lock) {
+ if (err)
+ strbuf_addf(err, Cannot lock the ref '%s'.,
+ update-refname);
ret = 1;
goto cleanup;
}
diff --git a/refs.h b/refs.h
index 6d97700..25ae110 100644
--- a/refs.h
+++ b/refs.h
@@ -274,9 +274,12 @@ void ref_transaction_delete(struct ref_transaction
*transaction,
* Commit all of the changes that have been queued in transaction, as
* atomically as possible. Return a nonzero value if there is a
* problem. The ref_transaction is freed by this function.
+ * If err is non-NULL we will add an error string to it to explain why
+ * the transaction failed. The string does not end in newline.
*/
int ref_transaction_commit(struct ref_transaction *transaction,
- const char *msg, enum action_on_err onerr);
+ const char *msg, struct strbuf *err,
+ enum action_on_err onerr);
/** Lock a ref and then write its file */
int update_ref(const char *action, const char *refname,
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 02/42] refs.c: allow passing NULL to ref_transaction_free
Allow ref_transaction_free to be called with NULL and as a result allow
ref_transaction_rollback to be called for a NULL transaction.
This allows us to write code that will
if ( (!transaction ||
ref_transaction_update(...)) ||
(ref_transaction_commit(...) !(transaction = NULL)) {
ref_transaction_rollback(transaction);
...
}
In this case transaction is reset to NULL IFF ref_transaction_commit() was
invoked and thus the rollback becomes ref_transaction_rollback(NULL) which
is safe. IF the conditional triggered prior to ref_transaction_commit()
then transaction is untouched and then ref_transaction_rollback(transaction)
will rollback the failed transaction.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/refs.c b/refs.c
index 2114748..88d73c8 100644
--- a/refs.c
+++ b/refs.c
@@ -3312,6 +3312,9 @@ static void ref_transaction_free(struct ref_transaction
*transaction)
{
int i;
+ if (!transaction)
+ return;
+
for (i = 0; i transaction-nr; i++)
free(transaction-updates[i]);
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 18/42] refs.c: free the transaction before returning when number of updates is 0
We have to free the transaction before returning in the early check for
'return early if number of updates == 0' or else the following code would
create a memory leak with the transaction never being freed :
t = ref_transaction_begin()
ref_transaction_commit(t)
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 55aa5a9..a48873c 100644
--- a/refs.c
+++ b/refs.c
@@ -3460,8 +3460,10 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
int n = transaction-nr;
struct ref_update **updates = transaction-updates;
- if (!n)
+ if (!n) {
+ ref_transaction_free(transaction);
return 0;
+ }
/* Allocate work space */
delnames = xmalloc(sizeof(*delnames) * n);
--
2.0.0.rc3.471.g2055d11.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
