Re: [PATCH] Makefile: do not compile git with debugging symbols by default
On Thu, Jan 22, 2015 at 06:50:37PM +0600, Alexander Kuleshov wrote: Standard user has no need in debugging information. This patch adds DEBUG=1 option to compile git with debugging symbols and compile without it by default. This explanation is missing why it is beneficial _not_ to have the debugging information. I expect the answer is it makes the executable smaller. And that is true, but it gets smaller still if you run strip on the result: $ make CFLAGS= /dev/null 21 wc -c git 2424248 $ make CFLAGS=-g /dev/null 21 wc -c git 4500816 $ strip git wc -c git 2109200 So I am not sure who this is helping. If you are size-conscious, you should use strip, in which case the -g flag does not matter (and we even have make strip to help you). Is there some other reason to avoid the debugging information? -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
error closing pipe: Bad file descriptor at /usr/share/perl5/Git/SVN/Fetcher.pm line 335.
Hi all, When cloning ``` $ git svn clone https://geuz.org/svn/gmsh/trunk ``` I'm getting the error ``` [...] r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn) error closing pipe: Bad file descriptor at /usr/share/perl5/Git/SVN/Fetcher.pm line 335. error closing pipe: Bad file descriptor at /usr/share/perl5/Git/SVN/Fetcher.pm line 335. out pipe went bad at /usr/share/perl5/Git.pm line 955. ``` after 99 commits were successfully checked out. Apparently, this problem has popped up before in a different context [1]. This is with ``` $ git --version git version 2.2.2 ``` What I can do to help debugging this? Cheers, Nico [1] http://git.661346.n2.nabble.com/git-svn-exits-with-error-error-closing-pipe-Bad-file-descriptor-tt7592213.html#none -- 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 v4 15/33] Produce legal patch names in guilt-import-commit.
I just tried to run the regression suite on my OpenIndiana laptop and I got
this failure.
034: --- t-034.out 2015-01-22 14:02:23.634515474 +
+++ /tmp/guilt.log.148782015-01-22 14:03:54.258790788 +
@@ -83,7 +83,7 @@
[master aedb74f] @
Author: Author Name author@email
1 file changed, 1 insertion(+)
-% create_commit a Backslash\is\forbidden.
+% create_commit a Backslash\is
orbidden.
[master 0a46f8f] Backslash\is\forbidden.
Author: Author Name author@email
1 file changed, 1 insertion(+)
Test failed!
Test: 034
Log file: /tmp/guilt.log.14878
Repo dir: /tmp/guilt reg.12106
make[1]: *** [all] Error 1
It's obviously the cmd command printing that's busted. The following change
makes the test suite pass. Does it work for you? (If so, I'll commit it after
pulling your whole series.)
Thanks,
Jeff.
diff --git a/regression/scaffold b/regression/scaffold
index 97cff4e..593e9da 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -50,7 +50,7 @@ function filter_dd
# usage: cmd cmd..
function cmd
{
- echo % $@
+ printf %% %s\n $*
if ! (
exec 31
rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
On Sun, May 18, 2014 at 11:59:51PM +0200, Per Cederqvist wrote:
Try harder to create patch names that adhere to the rules in
git-check-ref-format(1) when deriving a patch name from the commit
message. Verify that the derived name using git check-ref-format,
and as a final fallback simply use the patch name x (to ensure that
the code is future-proof in case new rules are added in the future).
Always append a .patch suffix to the patch name.
Added test cases.
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt-import-commit | 20 +-
regression/t-034.out | 567
+++
regression/t-034.sh | 71 +++
3 files changed, 656 insertions(+), 2 deletions(-)
create mode 100644 regression/t-034.out
create mode 100755 regression/t-034.sh
diff --git a/guilt-import-commit b/guilt-import-commit
index f14647c..6260c56 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -28,19 +28,35 @@ disp Current head: `git rev-parse \`git_branch\`` 2
for rev in `git rev-list $rhash`; do
s=`git log --pretty=oneline -1 $rev | cut -c 42-`
+ # 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 \
-e s/['\\[{}]//g -e 's/]//g' -e 's/\*/-/g' \
- -e 's/\?/-/g' | tr A-Z a-z`
+ -e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
+ -e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
+
+ if ! valid_patchname $fname; then
+ # Try harder to make it a legal commit name by
+ # removing all but a few safe characters.
+ fname=`echo $fname|tr -d -c _a-zA-Z0-9---/\\n`
+ fi
+ if ! valid_patchname $fname; then
+ # If we failed to derive a legal patch name, use the
+ # name x. (If this happens, we likely have to
+ # append a suffix to make the name unique.)
+ fname=x
+ fi
disp Converting `echo $rev | cut -c 1-8` as $fname
mangle_prefix=1
fname_base=$fname
- while [ -f $GUILT_DIR/$branch/$fname ]; do
+ while [ -f $GUILT_DIR/$branch/$fname.patch ]; do
fname=$fname_base-$mangle_prefix
mangle_prefix=`expr $mangle_prefix + 1`
disp Patch under that name exists...trying '$fname'
done
+ fname=$fname.patch
(
do_make_header $rev
diff --git a/regression/t-034.out b/regression/t-034.out
new file mode 100644
index 000..7bc9459
--- /dev/null
+++ b/regression/t-034.out
@@ -0,0 +1,567 @@
+% setup_git_repo
+% git tag base
+% create_commit a The sequence /. is forbidden.
+[master eebb76e] The sequence /. is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+ create mode 100644 a
+% create_commit a The sequence .lock/ is forbidden.
+[master 45e81b5] The sequence .lock/ is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a A/component/may/not/end/in/foo.lock
+[master bbf3f59] A/component/may/not/end/in/foo.lock
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Two consecutive dots (..) is forbidden.
+[master 1535e67] Two consecutive dots (..) is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Check/multiple/../dots/./foo..patch
+[master 48eb60c] Check/multiple/../dots/./foo..patch
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Space is
[PATCH] Makefile: do not compile git with debugging symbols by default
Standard user has no need in debugging information. This patch adds DEBUG=1 option to compile git with debugging symbols and compile without it by default. Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- Makefile | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b5b4cee..83ff691 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,8 @@ all:: # Define V=1 to have a more verbose compile. # +# Define DEBUG=1 to compile git with debugging symbols. +# # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken. # # Define SANE_TOOL_PATH to a colon-separated list of paths to prepend @@ -363,8 +365,13 @@ GIT-VERSION-FILE: FORCE -include GIT-VERSION-FILE # CFLAGS and LDFLAGS are for the users to override from the command line. +DEBUG_CFLAGS= + +ifdef DEBUG + DEBUG_CFLAGS = -g +endif -CFLAGS = -g -O2 -Wall +CFLAGS = $(DEBUG_CFLAGS) -O2 -Wall LDFLAGS = ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) ALL_LDFLAGS = $(LDFLAGS) -- 2.3.0.rc1 -- 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 v4 14/33] Use git check-ref-format to validate patch names.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
On Sun, May 18, 2014 at 11:59:50PM +0200, Per Cederqvist wrote:
The valid_patchname now lets git check-ref-format do its job instead
of trying (and failing) to implement the same rules. See
git-check-ref-format(1) for a list of the rules. Re-implement rules
added to git check-ref-format after Git 1.5.0, so that guilt rejects
the same names no matter what version of Git we are using (but
versions prior to 1.5.0 are not supported).
Refer to the git-check-ref-format(1) man page in the error messages
produced when valid_patchname indicates that the name is bad.
Added testcases that breaks most of the rules in that man-page.
Signed-off-by: Per Cederqvist ced...@opera.com
---
guilt| 46 +-
guilt-fork | 2 +-
guilt-import | 2 +-
guilt-new| 2 +-
regression/t-025.out | 426
+--
regression/t-025.sh | 12 +-
regression/t-032.out | 4 +-
7 files changed, 474 insertions(+), 20 deletions(-)
diff --git a/guilt b/guilt
index 3fc524e..9567a78 100755
--- a/guilt
+++ b/guilt
@@ -132,14 +132,50 @@ fi
# usage: valid_patchname patchname
valid_patchname()
{
+ # Once we only support Git 1.7.8 and newer, the command below
+ # could be replaced with:
+ #
+ # git check-ref-format --allow-onelevel $1
+ #
+ # Instead, we arbitrarily prepend one level. The result
+ # should be the same, and this is portable to all existing
+ # versions of Git.
+ git check-ref-format a/$1
+ if [ $? -ne 0 ]; then
+ return 1
+ fi
+
+ # We want to reject all names that Git 2.0.0 rejects. In case
+ # we are running an older version, we explicitly check some
+ # cases that were added to Git after version 1.5.0. This code
+ # aims to support Git version 1.5.0 and newer.
+
+ # Git 1.7.6.4 and newer rejects the DEL character.
+ if [ `echo $1|tr -d '\177'` != $1 ]; then
+ return 1
+ fi
+
+ # Git 1.7.8 and newer rejects refs that start or end with
+ # slash or contain multiple adjacent slashes.
case $1 in
- /*|./*|../*|*/./*|*/../*|*/.|*/..|*/|*\ *|*\*)
+ /*|*/|*//*)
return 1;;
- *:*)
- return 1;;
- *)
- return 0;;
esac
+
+ # Git 1.7.8 and newer rejects refname components that end in
+ # .lock.
+ case $1 in
+ *.lock/*|*.lock)
+ return 1;;
+ esac
+
+ # Git 1.8.5 and newer rejects refnames that are made up of the
+ # single character @.
+ if [ $1 = @ ]; then
+ return 1
+ fi
+
+ return 0
}
get_branch()
diff --git a/guilt-fork b/guilt-fork
index a85d391..6447e55 100755
--- a/guilt-fork
+++ b/guilt-fork
@@ -37,7 +37,7 @@ else
fi
if ! valid_patchname $newpatch; then
- die The specified patch name contains invalid characters (:).
+ die The specified patch name is invalid according to
git-check-ref-format(1).
fi
if [ -e $GUILT_DIR/$branch/$newpatch ]; then
diff --git a/guilt-import b/guilt-import
index 3e9b3bb..928e325 100755
--- a/guilt-import
+++ b/guilt-import
@@ -40,7 +40,7 @@ if [ -e $GUILT_DIR/$branch/$newname ]; then
fi
if ! valid_patchname $newname; then
- die The specified patch name contains invalid characters (:).
+ die The specified patch name is invalid according to
git-check-ref-format(1).
fi
# create any directories as needed
diff --git a/guilt-new b/guilt-new
index 9528438..9f7fa44 100755
--- a/guilt-new
+++ b/guilt-new
@@ -64,7 +64,7 @@ fi
if ! valid_patchname $patch; then
disp Patchname is invalid. 2
- die it cannot begin with '/', './' or '../', or contain /./, /../, or
whitespace
+ die It must follow the rules in git-check-ref-format(1).
fi
# create any directories as needed
diff --git a/regression/t-025.out b/regression/t-025.out
index 7811ab1..01bc406 100644
--- a/regression/t-025.out
+++ b/regression/t-025.out
@@ -141,7 +141,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new white space
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
d .git/patches
d .git/patches/master
@@ -211,7 +211,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new /abc
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
Re: [PATCHv2 5/5] refs.c: enable large transactions
On 01/22/2015 03:32 AM, Stefan Beller wrote:
By closing the file descriptors after creating the lock file we are not
limiting the size of the transaction by the number of available file
descriptors.
Signed-off-by: Stefan Beller sbel...@google.com
---
refs.c| 17 +
t/t1400-update-ref.sh | 4 ++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 2013d37..9d01102 100644
--- a/refs.c
+++ b/refs.c
@@ -3055,11 +3055,18 @@ int is_branch(const char *refname)
static int write_sha1_to_lock_file(struct ref_lock *lock,
const unsigned char *sha1)
{
- if (fdopen_lock_file(lock-lk, w) 0
- || fprintf(lock-lk-fp, %s\n, sha1_to_hex(sha1)) != 41)
+ if (lock-lk-fd == -1) {
+ if (reopen_lock_file(lock-lk) 0
+ || fdopen_lock_file(lock-lk, w) 0
+ || fprintf(lock-lk-fp, %s\n, sha1_to_hex(sha1)) != 41
+ || close_lock_file(lock-lk) 0)
+ return -1;
+ } else {
+ if (fdopen_lock_file(lock-lk, w) 0
+ || fprintf(lock-lk-fp, %s\n, sha1_to_hex(sha1)) != 41)
return -1;
- else
- return 0;
+ }
+ return 0;
}
I can't figure out where to apply this series or where to fetch it from,
so I can't see these changes in context, so maybe I'm misunderstanding
something. It looks like this code is doing
open(), close(), open(), fdopen(), write(), fclose(), rename()
on each lockfile. But don't we have enough information to write the
SHA-1 into the lockfile the first time we touch it? I.e., couldn't we
reduce this to
open(), fdopen(), write(), fclose(), rename()
, where the first four calls all happen in the initial loop? If a
problem is discovered when writing a later reference, we would roll back
the transaction anyway.
I understand that this would require a bigger rewrite, so maybe it is
not worth it.
/*
@@ -3761,6 +3768,8 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-refname);
goto cleanup;
}
+ /* Do not keep all lock files open at the same time. */
+ close_lock_file(update-lock-lk);
}
/* Perform updates first so live commits remain referenced */
[...]
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG] Mojibake in git gui and gitk for certain unicode chars
Hello, I’ve noticed git gui and gitk seem to have problems decoding certain unicode characters. E.g., when a commit contains the character «» (thumbs up sign; U+1F44D) in UTF-8 encoding, this character will show as «ð» in gitk. git gui also displays it using the same sequence. When trying to stage lines within the context of such characters, the program will error out (corrupt patch). The character sequence appears to be mojibake introduced by decoding UTF-8 as ISO-8859-1. However, my locale is set to «en_US.utf8». git gui is also set to assume UTF-8 encoding for files, and in the list menu where this encoding is selected, it lists the UTF-8 option under «system encoding», which suggests that my locale is correctly picked up. Is there perchance any heuristics in place which tries decoding files as unicode, with a fall-back to latin1? If so, then potentially the bug could be due to U+1F44D tripping up the decoder, triggering a fall-back, and rendering the characters as mojibake. I’ve noticed a perhaps related glitch when the options in git gui is shown. My committer name contains the character «ß» (latin small letter sharp s; U+00DF). The text field in the options dialog displays this as «Ã», which also seems to be UTF-8 to latin1 mojibake. Curiously, the same character displays just fine when staging parts of files via git gui, so the issue is not quite the same as the one described above. Best regards, Tobias -- 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] Fix unclosed here document in t3301.sh
On Thu, Jan 22, 2015 at 12:59:36PM +0100, Kacper Kornet wrote: Commit 908a3203632a02568df230c0fccf9a2cd8da24e6 introduced indentation to here documents in t3301.sh. However in one place -EOF was missing -, which broke this test when run with mksh-50d. This commit fixes it. This is definitely the right direction, but I was a little surprised it worked at all on other shells! Both bash and dash end the here-doc at the end of the input (in this case the end of the eval string). They end up sucking the EOF and the follow-on commands into the here-doc, and the test literally does nothing except the call to cat. Bash does print a warning in this case. It would be nice to upgrade it to an error (so at least bash users could easily detect the buggy script), but I don't see any way to do so. I guess running with mksh is a good substitute. :) However, in most such instances of this problem, the shell will notice and barf, because it syntactically expects more on the next line: $ sh -c ' cat foo EOF whatever EOF do_something ' sh: 6: Syntax error: end of file unexpected So the problem in this instance is that the here-doc marker is wrong _and_ the test accidentally broke the -chaining: test_expect_success '--no-standard-notes' ' - cat expect-commits EOF + cat expect-commits -EOF The ideal line here would be: cat expect-commits -EOF -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: Pretty format specifier for commit count?
On Thu, Jan 22, 2015 at 11:10:42AM +0100, Michael J Gruber wrote: We do have a linear history when we walk with --first-parent :) Yes, but I do not think it is robust to adding new commits on top. E.g., given: A--B--C---F \/ D--E a --first-parent walk from F will show F-C-B-A. Now imagine the branch advances to I: G--H---I / / A--B--C---F--J \/ D--E A walk from I will show I-H-G-C-B-A. F is no longer mentioned at all, and A, B, and C are now at different positions. This might be OK in Josh's case. I have an intuition that commits can only be _removed_ in this case. Which means position from the _top_ might change, but the position from the root will always be the same (and that is what he wants to be stable). But I did not think hard enough to convince myself that this is always the case. So, for the changelog for commits on a branch, where on a branch is not the git concept but defined by git rev-list --first-parent (more like hg branches), the count from root would be deterministic and the right concept given the appropriate branch workflow. Certainly the distance to root is deterministic. But I think we are really counting number of commits to be output between the root and this commit. I guess if: 1. You only ever start from one traversal point. 2. You are picking only one parent for each merge. then we know that our queue of commits to examine only ever has 0 or 1 items in it. And therefore a commit is either shown in the same position from the end, or not shown at all. Because once we get there, it is deterministic which commits we will show. Generation numbers are monotonous but may increase by steps greater than 1 on that branch if I remember them correctly. I.e., merge commits are weighted by the number of commits that get merged in. Sort of. It is the longest distance to (any) root from the commit. So it is the max() of the generations of the parents, plus one. So for a simple branch/merge between two lines of development, the increase is the number of commits that are merged. But a branch that has its own branches will not increase the generation count by the total number of commits, but rather by the longest individual sub-branch. -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: [GUILT v4 33/33] Document the exit status of guilt push and guilt pop.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net Ok, I think this is all of the patches. I'll wait for you to tell me if the cmd() echo to printf change breaks anything for you. If not, I'll pull your series (via git), tag a 0.36-rc1, and push the whole thing to repo.or.cz. :) Sorry for making this take so long :( Jeff. On Mon, May 19, 2014 at 12:00:09AM +0200, Per Cederqvist wrote: --- Documentation/guilt-pop.txt | 3 +++ Documentation/guilt-push.txt | 3 +++ 2 files changed, 6 insertions(+) diff --git a/Documentation/guilt-pop.txt b/Documentation/guilt-pop.txt index 36fea9e..b0b89cc 100644 --- a/Documentation/guilt-pop.txt +++ b/Documentation/guilt-pop.txt @@ -26,6 +26,9 @@ OPTIONS If no patchname is given, pop the top-most patch. +Exit with a non-zero exit status if requested to pop more patches +than there are on the stack. + Author -- Written by Josef Jeff Sipek jef...@josefsipek.net diff --git a/Documentation/guilt-push.txt b/Documentation/guilt-push.txt index 6ee86b9..6439f21 100644 --- a/Documentation/guilt-push.txt +++ b/Documentation/guilt-push.txt @@ -26,6 +26,9 @@ OPTIONS If no patchname is given, push the next patch in the series onto the tree. +Exit with a non-zero exit status if requested to push more patches +than there are in the series. + Author -- Written by Josef Jeff Sipek jef...@josefsipek.net -- 1.8.3.1 -- Computer Science is no more about computers than astronomy is about telescopes. - Edsger Dijkstra -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Fix unclosed here document in t3301.sh
Commit 908a3203632a02568df230c0fccf9a2cd8da24e6 introduced indentation to here documents in t3301.sh. However in one place -EOF was missing -, which broke this test when run with mksh-50d. This commit fixes it. Signed-off-by: Kacper Kornet drae...@pld-linux.org --- t/t3301-notes.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh index 245406a..433f925 100755 --- a/t/t3301-notes.sh +++ b/t/t3301-notes.sh @@ -658,7 +658,7 @@ test_expect_success '--show-notes=* adds to GIT_NOTES_DISPLAY_REF' ' ' test_expect_success '--no-standard-notes' ' - cat expect-commits EOF + cat expect-commits -EOF commit 2c125331118caba0ff8238b7f4958ac6e93fe39c Author: A U Thor aut...@example.com Date: Thu Apr 7 15:18:13 2005 -0700 -- 2.2.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Mojibake in git gui and gitk for certain unicode chars
On Do, 2015-01-22 at 12:43 +0100, Tobias Getzner wrote: Hello, I’ve noticed git gui and gitk seem to have problems decoding certain unicode characters. E.g., when a commit contains the character «» (thumbs up sign; U+1F44D) in UTF-8 encoding, this character will show as «ð» in gitk. I’ve noticed a perhaps related glitch when the options in git gui is shown. My committer name contains the character «ß» (latin small letter sharp s; U+00DF). The text field in the options dialog displays this as «Ã», I suppose that some of the mojibake characters in the message might have been stripped out of the message because they are control chars. So, «» was rendered as «ð\x9f\x91\x8d». «ß» was rendered as «Ã\x9f». -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 0/5] Fix bug in large transactions
On 01/22/2015 03:32 AM, Stefan Beller wrote: version2: Summary: patches 1-4 look good to me. I sent a separate comment about patch 5, which seems to do more system calls than necessary. Michael -- Michael Haggerty mhag...@alum.mit.edu -- 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 v4 09/33] Test suite: properly check the exit status of commands.
On Sun, May 18, 2014 at 11:59:45PM +0200, Per Cederqvist wrote:
The cmd and shouldfail functions checked the exit status of the
replace_path function instead of the actual command that was running.
(The $? construct checks the exit status of the last command in a
pipeline, not the first command.)
Print an explicit error message if a command that should fail actually
succeeds.
This is a god idea.
The patch still looks good.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
Updated t-032.sh, which used shouldfail instead of cmd in one
place. (The comment in the script makes it clear that the command is
expected to succeed.)
Signed-off-by: Per Cederqvist ced...@opera.com
---
regression/scaffold | 23 ---
regression/t-032.sh | 2 +-
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/regression/scaffold b/regression/scaffold
index 5c8b73e..2e04c83 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -51,19 +51,28 @@ function filter_dd
function cmd
{
echo % $@
- $@ 21 | replace_path return 0
- return 1
+ if ! (
+ exec 31
+ rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
+ exit $rv
+ ) ; then
+ echo % FAIL: The above command should succeed but failed.
+ exit 1
+ fi
}
# usage: shouldfail cmd..
function shouldfail
{
echo % $@
- (
- $@ 21 || return 0
- return 1
- ) | replace_path
- return $?
+ if (
+ exec 31
+ rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
+ exit $rv
+ ) ; then
+ echo % FAIL: The above command should fail but succeeded.
+ exit 1
+ fi
}
# usage: list_files
diff --git a/regression/t-032.sh b/regression/t-032.sh
index b1d5f19..bba401e 100755
--- a/regression/t-032.sh
+++ b/regression/t-032.sh
@@ -28,7 +28,7 @@ shouldfail guilt import -P foo3 foo
cmd guilt import -P foo2 foo
# ok
-shouldfail guilt import foo
+cmd guilt import foo
# duplicate patch name (implicit)
shouldfail guilt import foo
--
1.8.3.1
--
Intellectuals solve problems; geniuses prevent them
- Albert Einstein
--
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
[PATCHv3] rebase -i: respect core.abbrev for real
I have tried to fix this before: see 568950388be2, but it doesn't
really work.
I don't know how it happend, but that commit makes interactive rebase to
respect core.abbrev only during --edit-todo, but not the initial todo
list edit.
For this time I've included a test-case to avoid this frustration again.
The patch change code to use full 40-hex revision ids for todo actions
everywhere and collapse them only to show to user.
Signed-off-by: Kirill A. Shutemov kirill.shute...@linux.intel.com
---
v3:
- use full 40-hex revision ids for todo actions everywhere and collapse them
only to show to user;
v2:
- fix -chain in the test-case
---
git-rebase--interactive.sh| 17 -
t/t3404-rebase-interactive.sh | 7 +++
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index c6a4629cbc2b..c96b9847e9fc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -961,14 +961,13 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
fi
-git rev-list $merges_option --pretty=oneline --abbrev-commit \
- --abbrev=7 --reverse --left-right --topo-order \
+git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n s/^//p |
-while read -r shortsha1 rest
+while read -r sha1 rest
do
- if test -z $keep_empty is_empty_commit $shortsha1 !
is_merge_commit $shortsha1
+ if test -z $keep_empty is_empty_commit $sha1 ! is_merge_commit
$sha1
then
comment_out=$comment_char
else
@@ -977,9 +976,8 @@ do
if test t != $preserve_merges
then
- printf '%s\n' ${comment_out}pick $shortsha1 $rest $todo
+ printf '%s\n' ${comment_out}pick $sha1 $rest $todo
else
- sha1=$(git rev-parse $shortsha1)
if test -z $rebase_root
then
preserve=t
@@ -996,7 +994,7 @@ do
if test f = $preserve
then
touch $rewritten/$sha1
- printf '%s\n' ${comment_out}pick $shortsha1 $rest
$todo
+ printf '%s\n' ${comment_out}pick $sha1 $rest $todo
fi
fi
done
@@ -1020,8 +1018,8 @@ then
# just the history of its first-parent for others that
will
# be rebasing on top of it
git rev-list --parents -1 $rev | cut -d' ' -s -f2
$dropped/$rev
- short=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev)
- sane_grep -v ^[a-z][a-z]* $short $todo
${todo}2 ; mv ${todo}2 $todo
+ sha1=$(git rev-list -1 $rev)
+ sane_grep -v ^[a-z][a-z]* $sha1 $todo ${todo}2
; mv ${todo}2 $todo
rm $rewritten/$rev
fi
done
@@ -1054,6 +1052,7 @@ has_action $todo ||
return 2
cp $todo $todo.backup
+collapse_todo_ids
git_sequence_editor $todo ||
die_abort Could not execute editor
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8197ed29a9ec..a31f7e0430e1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1039,4 +1039,11 @@ test_expect_success 'short SHA-1 collide' '
)
'
+test_expect_success 'respect core.abbrev' '
+ git config core.abbrev 12
+ set_cat_todo_editor
+ test_must_fail git rebase -i HEAD~4 todo-list
+ test 4 = $(grep -c pick [0-9a-f]\{12,\} todo-list)
+'
+
test_done
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] parse-opts: add OPT_SUBCMD()
OPT_SUBCMD can be used for parsing git commands which has not only
short/long options, but subcomands. For example: git bundle (create,
verify and etc...) or git remote (add, rename and etc...)
Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
parse-options.c | 25 +
parse-options.h | 8
t/t0040-parse-options.sh | 13 +
test-parse-options.c | 3 +++
4 files changed, 49 insertions(+)
diff --git a/parse-options.c b/parse-options.c
index 80106c0..79f971e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -121,6 +121,10 @@ static int get_value(struct parse_opt_ctx_t *p,
*(int *)opt-value = unset ? 0 : opt-defval;
return 0;
+ case OPTION_SUBCMD:
+ *(int *)opt-value = 1;
+ return 0;
+
case OPTION_CMDMODE:
/*
* Giving the same mode option twice, although is unnecessary,
@@ -314,6 +318,17 @@ is_abbreviated:
return -2;
}
+static int parse_subcmd_opt(struct parse_opt_ctx_t *p, const char *arg,
+const struct option *options) {
+ const struct option *all_opts = options;
+ for (; options-type != OPTION_END; options++) {
+ if (options-long_name !strcmp(options-long_name, arg))
+ return get_value(p, options, all_opts, 0);
+ }
+
+ return -2;
+}
+
static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,
const struct option *options)
{
@@ -417,6 +432,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
const struct option *options,
const char * const usagestr[])
{
+ int argc = ctx-argc;
int internal_help = !(ctx-flags PARSE_OPT_NO_INTERNAL_HELP);
/* we must reset -opt, unknown short option leave it dangling */
@@ -426,6 +442,11 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
const char *arg = ctx-argv[0];
if (*arg != '-' || !arg[1]) {
+ if (ctx-argc == argc
+ options-type == OPTION_SUBCMD
+ parse_subcmd_opt(ctx, arg, options) == 0){
+ continue;
+ }
if (parse_nodash_opt(ctx, arg, options) == 0)
continue;
if (ctx-flags PARSE_OPT_STOP_AT_NON_OPTION)
@@ -581,6 +602,10 @@ static int usage_with_options_internal(struct
parse_opt_ctx_t *ctx,
size_t pos;
int pad;
+ if (opts-type == OPTION_SUBCMD) {
+ continue;
+ }
+
if (opts-type == OPTION_GROUP) {
fputc('\n', outfile);
if (*opts-help)
diff --git a/parse-options.h b/parse-options.h
index 7940bc7..0e75a85 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -13,6 +13,8 @@ enum parse_opt_type {
OPTION_COUNTUP,
OPTION_SET_INT,
OPTION_CMDMODE,
+ OPTION_SUBCMD,
+
/* options with arguments (usually) */
OPTION_STRING,
OPTION_INTEGER,
@@ -115,6 +117,10 @@ struct option {
#define OPT_END() { OPTION_END }
#define OPT_ARGUMENT(l, h) { OPTION_ARGUMENT, 0, (l), NULL, NULL, \
(h), PARSE_OPT_NOARG}
+
+#define OPT_SUBCMD(l, v, h) { OPTION_SUBCMD, 0, (l), (v), NULL, \
+ (h), PARSE_OPT_NOARG}
+
#define OPT_GROUP(h){ OPTION_GROUP, 0, NULL, NULL, NULL, (h) }
#define OPT_BIT(s, l, v, h, b) { OPTION_BIT, (s), (l), (v), NULL, (h), \
PARSE_OPT_NOARG, NULL, (b) }
@@ -125,8 +131,10 @@ struct option {
#define OPT_SET_INT(s, l, v, h, i) { OPTION_SET_INT, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG, NULL, (i) }
#define OPT_BOOL(s, l, v, h)OPT_SET_INT(s, l, v, h, 1)
+
#define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
NULL, 1}
+
#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG|PARSE_OPT_NONEG,
NULL, (i) }
#define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), N_(n),
(h) }
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index a90c86b..7898a5a 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -65,6 +65,7 @@ verbose: 0
quiet: no
dry run: no
file: (not set)
+subcommand: 1
EOF
check() {
@@ -142,6 +143,7 @@ verbose: 2
quiet: no
dry run: yes
file: prefix/my.file
+subcommand: 1
EOF
test_expect_success 'short options' '
@@ -161,6 +163,7 @@ verbose: 2
quiet: no
dry run: no
file: prefix/fi.le
Re: [PATCH] Fix unclosed here document in t3301.sh
On Thu, Jan 22, 2015 at 12:59 PM, Kacper Kornet drae...@pld-linux.org wrote: Commit 908a3203632a02568df230c0fccf9a2cd8da24e6 introduced indentation to here documents in t3301.sh. However in one place -EOF was missing -, which broke this test when run with mksh-50d. This commit fixes it. Signed-off-by: Kacper Kornet drae...@pld-linux.org Acked-by: Johan Herland jo...@herland.net --- t/t3301-notes.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh index 245406a..433f925 100755 --- a/t/t3301-notes.sh +++ b/t/t3301-notes.sh @@ -658,7 +658,7 @@ test_expect_success '--show-notes=* adds to GIT_NOTES_DISPLAY_REF' ' ' test_expect_success '--no-standard-notes' ' - cat expect-commits EOF + cat expect-commits -EOF commit 2c125331118caba0ff8238b7f4958ac6e93fe39c Author: A U Thor aut...@example.com Date: Thu Apr 7 15:18:13 2005 -0700 -- 2.2.2 -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 5/5] refs.c: enable large transactions
On 22/01/15 02:32, Stefan Beller wrote:
By closing the file descriptors after creating the lock file we are not
limiting the size of the transaction by the number of available file
descriptors.
Signed-off-by: Stefan Beller sbel...@google.com
---
refs.c| 17 +
t/t1400-update-ref.sh | 4 ++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 2013d37..9d01102 100644
--- a/refs.c
+++ b/refs.c
@@ -3055,11 +3055,18 @@ int is_branch(const char *refname)
static int write_sha1_to_lock_file(struct ref_lock *lock,
const unsigned char *sha1)
{
- if (fdopen_lock_file(lock-lk, w) 0
- || fprintf(lock-lk-fp, %s\n, sha1_to_hex(sha1)) != 41)
+ if (lock-lk-fd == -1) {
+ if (reopen_lock_file(lock-lk) 0
+ || fdopen_lock_file(lock-lk, w) 0
fdopen_lock_file() returns a 'FILE *', so this causes sparse to bark:
refs.c:3105:56: error: incompatible types for operation ()
refs.c:3105:56:left side has type struct _IO_FILE [usertype] *
refs.c:3105:56:right side has type int
+ || fprintf(lock-lk-fp, %s\n, sha1_to_hex(sha1)) != 41
+ || close_lock_file(lock-lk) 0)
+ return -1;
+ } else {
+ if (fdopen_lock_file(lock-lk, w) 0
Similarly, sparse barks:
refs.c:3110:53: error: incompatible types for operation ()
refs.c:3110:53:left side has type struct _IO_FILE [usertype] *
refs.c:3110:53:right side has type int
+ || fprintf(lock-lk-fp, %s\n, sha1_to_hex(sha1)) != 41)
return -1;
- else
- return 0;
+ }
+ return 0;
}
/*
@@ -3761,6 +3768,8 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-refname);
goto cleanup;
}
+ /* Do not keep all lock files open at the same time. */
+ close_lock_file(update-lock-lk);
}
/* Perform updates first so live commits remain referenced */
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 47d2fe9..c593a1d 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -979,7 +979,7 @@ run_with_limited_open_files () {
test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating
branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating
branches does not burst open file limit' '
(
for i in $(test_seq 33)
do
@@ -990,7 +990,7 @@ test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large
transaction creating branches
)
'
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction deleting
branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction deleting
branches does not burst open file limit' '
(
for i in $(test_seq 33)
do
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/5] update-ref: test handling large transactions properly
On Thu, Jan 22, 2015 at 11:54:49AM +0100, Michael Haggerty wrote:
+run_with_limited_open_files () {
+ (ulimit -n 32 $@)
+}
Regarding the choice of 32, I wonder what is the worst-case number of
open file descriptors that are needed *before* counting the ones that
are currently wasted on open loose-reference locks. On Linux it seems to
be only 4 with my setup:
$ (ulimit -n 3 git update-ref --stdin /dev/null)
bash: /dev/null: Too many open files
$ (ulimit -n 4 git update-ref --stdin /dev/null)
$
This number might depend a little bit on details of the repository, like
whether config files import other config files. But as long as the
background number of fds required is at least a few less than 32, then
your number should be safe.
Does anybody know of a platform where file descriptors are eaten up
gluttonously by, for example, each shared library that is in use or
something? That's the only think I can think of that could potentially
make your choice of 32 problematic.
It's not just choice of platform. There could be inherited descriptors
in the environment (e.g., the test suite is being run by a shell that
keeps a pipe to CI server open, or something). And the test suite itself
uses several extra descriptors for hiding and showing output.
I think this is the sort of thing that we have to determine with a mix
of guessing and empiricism. 4 is almost certainly too low. 32 looks
pretty big in practice but not so big that it will make the test slow.
I think our best bet is probably to ship it and see if anybody reports
problems while the patch cooks. Then we can bump the number (or find a
new approach) as necessary.
-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: [GUILT v4 32/33] Improved doc and tests for guilt header.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net On Mon, May 19, 2014 at 12:00:08AM +0200, Per Cederqvist wrote: --- Documentation/guilt-header.txt | 5 - regression/t-028.out | 9 + regression/t-028.sh| 3 +++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Documentation/guilt-header.txt b/Documentation/guilt-header.txt index 870bfaf..71b2e66 100644 --- a/Documentation/guilt-header.txt +++ b/Documentation/guilt-header.txt @@ -18,7 +18,10 @@ Prints either the topmost patch's header or the header of a specified patch. -E:: Open the raw patch in an editor, instead of printing it. patchname:: - Name of the patch. + Name of the patch. If a patch with exactly this name exists, + use it. Otherwise, treat the name as a regexp; if the regexp + matches a single patch, use it. Otherwise, list all matching + patch names to stderr and fail. Author -- diff --git a/regression/t-028.out b/regression/t-028.out index ea72a3a..39ac900 100644 --- a/regression/t-028.out +++ b/regression/t-028.out @@ -56,3 +56,12 @@ Patch non-existant is not in the series remove mode patch-with-some-desc +% guilt header de +de does not uniquely identify a patch. Did you mean any of these? + mode + patch-with-some-desc +% guilt header des +blah blah blah + +Signed-off-by: Commiter Name commiter@email + diff --git a/regression/t-028.sh b/regression/t-028.sh index 2ce0378..cd3088c 100755 --- a/regression/t-028.sh +++ b/regression/t-028.sh @@ -35,4 +35,7 @@ shouldfail guilt header non-existant # patch name is a regexp that just happens to match an existing patch. shouldfail guilt header '.*' +shouldfail guilt header de +cmd guilt header des + # FIXME: how do we check that -e works? -- 1.8.3.1 -- You measure democracy by the freedom it gives its dissidents, not the freedom it gives its assimilated conformists. - Abbie Hoffman -- 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: [PATCHv1 0/6] Fix bug in large transactions
Jeff King p...@peff.net writes:
On Wed, Jan 21, 2015 at 03:23:39PM -0800, Stefan Beller wrote:
(reported as: git update-ref --stdin : too many open files, 2014-12-20)
First a test case is introduced to demonstrate the failure,
the patches 2-6 are little refactoring and the last patch
fixes the bug and also marks the bugs as resolved in the
test suite.
Unfortunately this applies on top of origin/next.
Saying applies on next is not very useful to Junio. He is not going to
branch a topic straight from next, as merging it to master would pull
in all of the topics cooking in next (not to mention a bunch of merge
commits which are generally never part of master).
Instead, figure out which topic in next you actually _need_ to build on,
and then it can be branched from there. And if there is no such topic,
then you should not be building on next, of course. :) But I think you
know that part already.
All very true.
I consider anything new that appears late in the cycle, especially
during deep in the pre-release freeze, less for me to apply but more
for others to eyeball the preview of a series the submitter plans to
work on once the next cycle starts, so basing on 'next' does not
hurt too much. For interested others,
git checkout origin/next^0
would be shorter to type than
git checkout origin/next^{/^Merge branch 'sb/atomic-push'}^2
so... ;-)
But what is more troublesome is that neither this or updated v2
applies to 'next'.
Let me try to wiggle it in first.
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] gitk: Remove tcl-format flag from a message that shouldn't have it
xgettext sees % o and interprets it as a placeholder for an octal
number preceded by a space. However, in this case it's not actually a
placeholder, and most translations will replace the % o sequence with
something else. Removing the tcl-format flag from this string prevents
tools like Poedit from freaking out when % o doesn't appear in the
translated string.
The corrected flag will appear in each translation's po file the next time
the translation is updated with `make update-po`.
Signed-off-by: Alex Henrie alexhenri...@gmail.com
---
gitk-git/gitk | 1 +
1 file changed, 1 insertion(+)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 78358a7..dfd458d 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -11237,6 +11237,7 @@ proc prefspage_general {notebook} {
${NS}::label $page.maxwidthl -text [mc Maximum graph width (lines)]
spinbox $page.maxwidth -from 0 -to 100 -width 4 -textvariable maxwidth
grid $page.spacer $page.maxwidthl $page.maxwidth -sticky w
+ #xgettext:no-tcl-format
${NS}::label $page.maxpctl -text [mc Maximum graph width (% of pane)]
spinbox $page.maxpct -from 1 -to 100 -width 4 -textvariable maxgraphpct
grid x $page.maxpctl $page.maxpct -sticky w
--
2.2.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] transport-helper: do not request symbolic refs to remote helpers
On Wed, Jan 21, 2015 at 11:41:51PM -0800, Junio C Hamano wrote: Mike Hommey m...@glandium.org writes: Note the most important part is actually between the parens: that only happens when the remote helper returns '?' to the `list` command, which non-git remotes helpers (like git-remote-hg or git-remote-bzr) do. git-remote-testgit also does, so if you only apply the test parts of the patch, you'll see that the test fails. remote-curl probably doesn't hit the problem because it's not returning '?' to `list`. Hmm, that suggests that the new codepath should be taken only when the remote helper says '?' (does it mean I dunno? where are these documented, by the way?) in Documentation/gitremote-helpers.txt , yes? It wasn't immediately obvious from the patch text that it was the case. The patch changes the behavior in all cases, because it didn't feel necessary to have a different behavior between the normal case and the '?' case: it makes sense to request the ref being pointed at than the symbolic ref in every case. Moreover, this makes existing non-git remote-helpers work without having to modify them to provide a refspec for HEAD (none of the 5 mercurial remote-helpers I checked do). The paragraph before last in the commit message mentioned this in maybe not very clear terms. Mike -- 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-add--interactive: return from list_and_choose if there is zero candidates
This patch introduce the check in list_and_choose() routine for the list. If
list is empty just return.
It can be useful for example if user selects 'add untracked' and there are no
untracked files, Add untracked opens. But it does not make sense in this
case, because there are no untracked files.
Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
git-add--interactive.perl | 5 +
1 file changed, 5 insertions(+)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 94b988c..85b2fe7 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -519,6 +519,9 @@ sub error_msg {
sub list_and_choose {
my ($opts, @stuff) = @_;
my (@chosen, @return);
+ if (!@stuff) {
+ return @return;
+ }
my $i;
my @prefixes = find_unique_prefixes(@stuff) unless $opts-{LIST_ONLY};
@@ -729,6 +732,8 @@ sub add_untracked_cmd {
if (@add) {
system(qw(git update-index --add --), @add);
say_n_paths('added', @add);
+ } else {
+ print No untracked files.\n;
}
print \n;
}
--
2.3.0.rc1.247.gb53aa6f.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: Git messes up 'ø' character
Noralf Trønnes schrieb am 20.01.2015 um 23:26: Den 20.01.2015 23:18, skrev Nico Williams: On Tue, Jan 20, 2015 at 10:38:40PM +0100, Noralf Trønnes wrote: Yes: $ echo Noralf Trønnes | xxd 000: 4e6f 7261 6c66 2054 72f8 6e6e 6573 0aNoralf Tr.nnes. Is there a command I can run that shows that I'm using ISO-8859-1 ? I need something to google with, my previous search only gave locale stuff, which seems fine. The locale(1) command tells you what your locale is set to, but it doesn't say anything about your input method -- it only tells you what your shell and commands started from it expect for input and what they should produce for output. The input method will generally be part of your windowing environment, for which you'll have to search how to check/configure your OS (sometimes it can be set on a per-window basis, sometimes it's a global setting). Even if the windowing environment is set to UTF-8, your terminal emulator might be set to ISO-8859-something, so check the terminal emulator (e.g., rxvt, Terminator, GNOME Terminal, PuTTY, ...). I use putty which was set to ISO-8859-1. Changing this to UTF-8 gave me the correct result: $ echo Noralf Trønnes | xxd 000: 4e6f 7261 6c66 2054 72c3 b86e 6e65 730a Noralf Tr..nnes. Thank you all for helping me! You can also check the encoding of your config file with file .git/config or :set fileencoding in vim. :set fileencoding=utf8 would allow you to convert it easily. (This assumes that the file does not mix encodings.) Michael -- 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: Pretty format specifier for commit count?
j...@joshtriplett.org schrieb am 21.01.2015 um 00:11: On Tue, Jan 20, 2015 at 04:49:53PM -0500, Jeff King wrote: On Mon, Jan 19, 2015 at 05:17:25PM -0800, Josh Triplett wrote: Can you be a bit more specific about the type count that you are after? git describe counts commits since the most recent tag (possibly within a specific subset of all tags). Is that your desired format? That might work, since the repository in question has no tags; I'd actually like commits since root commit. That's basically a generation number. But I'm not sure if that's really what you want; in a non-linear history it's not unique (two children of commit X are both X+1). That would actually be perfectly fine. If I need to distinguish branches, I can either use branch/tag names, or append a commit hash. I don't mind the following: /-B-\ A D \-C-/ A=1 B=C=2 D=3 I could (and probably should) append +hash to the version number for uniqueness, and if I care what order B and C sort in, I can use tags, branches, or some other more clever mechanism. It sounds like you really just want commits counting up from the root, and with side branches to have their own unique numbers. So something like: C / A--B--D A=1 B=2 C=3 D=4 except the last two are assigned arbitrarily. You need some rules for linearizing the commits. I don't care about the numbers assigned to anything not reachable from the committish I start from. But that's not deterministic as you add more starting points (either new ref tips, or just new merges we have to cross). For example, imagine this: G--H /\ C--E \ /\ \ A--B--D---F---I If we start at I, then we might visit H and G first, meaning we learn about C much earlier than we otherwise would. Then we hit F, and get to C from there. But now it it may be in a different position with respect to D! Right, the numbers need to always stay the same as you add more commits over time. If walking a given graph assigns a given set of generation numbers, walking any subgraph should assign all the same generation numbers to the common nodes. I suspect your problem statement may simply assume a linear history, which makes this all much simpler. But we are not likely to add a feature to git that will break badly once you have a non-linear history. :) Not assuming a linear history, but assuming a linear changelog file. :) I think in the linear case that a generation number _would_ be correct, and it is a useful concept by itself. So that may be the best thing to add. Sounds good to me. - Josh Triplett We do have a linear history when we walk with --first-parent :) So, for the changelog for commits on a branch, where on a branch is not the git concept but defined by git rev-list --first-parent (more like hg branches), the count from root would be deterministic and the right concept given the appropriate branch workflow. Generation numbers are monotonous but may increase by steps greater than 1 on that branch if I remember them correctly. I.e., merge commits are weighted by the number of commits that get merged in. Michael -- 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] branch: add support for --dry-run option
Scott Schmit schrieb am 22.01.2015 um 02:37: On Mon, Jan 19, 2015 at 03:20:51PM +0100, Michael J Gruber wrote: Alexander Kuleshov schrieb am 17.01.2015 um 08:35: This patch adds support -d/--dry-run option for branch(es) deletion. If -d/--dry-run option passed to git branch -d branch..., branch(es) will not be removed, instead just print list of branches that are to be removed. For example: $ git branch a b c * master $ git branch -d -n a b c delete branch 'a' (261c0d1) delete branch 'b' (261c0d1) delete branch 'c' (261c0d1) Is there a case where deleting a b c would not delete a b c? Sure: $ cd /tmp/ $ git init foo Initialized empty Git repository in /tmp/foo/.git/ $ cd foo/ $ touch .gitignore $ git add .gitignore $ git commit -m init [master (root-commit) fde5138] init 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 .gitignore $ git checkout -b a Switched to a new branch 'a' $ git branch -d a error: Cannot delete the branch 'a' which you are currently on. $ touch file $ git add file $ git commit -m 'add file' [a e2c2ece] add file 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 file $ git checkout -b b master Switched to a new branch 'b' $ git branch -d a error: The branch 'a' is not fully merged. If you are sure you want to delete it, run 'git branch -D a'. Yes, and that is something that should go into the commit message. Why do you want to add --dry-run? Because -d deletes only fully merged branches. It should have been there in the 1st place, rather than forcing us to ask the question that always needs to answered for a patch: What is the intention? What is it good for? In this case, we have other means to accomplish the same (--list -v), and they are more natural if you want get information about the state of the branches (list verbose) than doing delete dry-run. Michael -- 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 20/24] update-index: test the system before enabling untracked cache
On Wed, Jan 21, 2015 at 10:51:02AM -0800, Junio C Hamano wrote:
It appears that this hijacks a fixed name dir-mtime-test at the root
level of every project managed by Git. Is that intended?
I did think about filename clash, but I chose a fixed name anyway for
simplicity, otherwise we would need to reconstruct paths
dir-mtime-test/... in many places.
If you stuff the name of test directory (default dir-mtime-test)
in a strbuf and formulate test paths by chomping to the original and
then appending /... at the end, like your remove_test_directory()
already does, wouldn't that be sufficient?
It looks actually good. How about this on top?
-- 8 --
diff --git a/builtin/update-index.c b/builtin/update-index.c
index f23ec83..f5f6689 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -32,6 +32,7 @@ static int mark_valid_only;
static int mark_skip_worktree_only;
#define MARK_FLAG 1
#define UNMARK_FLAG 2
+static struct strbuf mtime_dir = STRBUF_INIT;
__attribute__((format (printf, 1, 2)))
static void report(const char *fmt, ...)
@@ -49,28 +50,37 @@ static void report(const char *fmt, ...)
static void remove_test_directory(void)
{
- struct strbuf sb = STRBUF_INIT;
- strbuf_addstr(sb, dir-mtime-test);
- remove_dir_recursively(sb, 0);
- strbuf_release(sb);
+ if (mtime_dir.len)
+ remove_dir_recursively(mtime_dir, 0);
+}
+
+static const char *get_mtime_path(const char *path)
+{
+ static struct strbuf sb = STRBUF_INIT;
+ strbuf_reset(sb);
+ strbuf_addf(sb, %s/%s, mtime_dir.buf, path);
+ return sb.buf;
}
static void xmkdir(const char *path)
{
+ path = get_mtime_path(path);
if (mkdir(path, 0700))
die_errno(_(failed to create directory %s), path);
}
-static int xstat(const char *path, struct stat *st)
+static int xstat_mtime_dir(struct stat *st)
{
- if (stat(path, st))
- die_errno(_(failed to stat %s), path);
+ if (stat(mtime_dir.buf, st))
+ die_errno(_(failed to stat %s), mtime_dir.buf);
return 0;
}
static int create_file(const char *path)
{
- int fd = open(path, O_CREAT | O_RDWR, 0644);
+ int fd;
+ path = get_mtime_path(path);
+ fd = open(path, O_CREAT | O_RDWR, 0644);
if (fd 0)
die_errno(_(failed to create file %s), path);
return fd;
@@ -78,12 +88,14 @@ static int create_file(const char *path)
static void xunlink(const char *path)
{
+ path = get_mtime_path(path);
if (unlink(path))
die_errno(_(failed to delete file %s), path);
}
static void xrmdir(const char *path)
{
+ path = get_mtime_path(path);
if (rmdir(path))
die_errno(_(failed to delete directory %s), path);
}
@@ -102,37 +114,40 @@ static int test_if_untracked_cache_is_supported(void)
{
struct stat st;
struct stat_data base;
- int fd;
+ int fd, ret = 0;
+
+ strbuf_addstr(mtime_dir, mtime-test-XX);
+ if (!mkdtemp(mtime_dir.buf))
+ die_errno(Could not make temporary directory);
fprintf(stderr, _(Testing ));
- xmkdir(dir-mtime-test);
atexit(remove_test_directory);
- xstat(dir-mtime-test, st);
+ xstat_mtime_dir(st);
fill_stat_data(base, st);
fputc('.', stderr);
avoid_racy();
- fd = create_file(dir-mtime-test/newfile);
- xstat(dir-mtime-test, st);
+ fd = create_file(newfile);
+ xstat_mtime_dir(st);
if (!match_stat_data(base, st)) {
close(fd);
fputc('\n', stderr);
fprintf_ln(stderr,_(directory stat info does not
change after adding a new file));
- return 0;
+ goto done;
}
fill_stat_data(base, st);
fputc('.', stderr);
avoid_racy();
- xmkdir(dir-mtime-test/new-dir);
- xstat(dir-mtime-test, st);
+ xmkdir(new-dir);
+ xstat_mtime_dir(st);
if (!match_stat_data(base, st)) {
close(fd);
fputc('\n', stderr);
fprintf_ln(stderr, _(directory stat info does not change
after adding a new directory));
- return 0;
+ goto done;
}
fill_stat_data(base, st);
fputc('.', stderr);
@@ -140,52 +155,57 @@ static int test_if_untracked_cache_is_supported(void)
avoid_racy();
write_or_die(fd, data, 4);
close(fd);
- xstat(dir-mtime-test, st);
+ xstat_mtime_dir(st);
if (match_stat_data(base, st)) {
fputc('\n', stderr);
fprintf_ln(stderr, _(directory stat info changes
after updating a file));
- return 0;
+ goto done;
}
fputc('.', stderr);
avoid_racy();
Re: [PATCH v2 05/18] fsck: Allow demoting errors to warnings via receive.fsck.warn = key
Hi Junio,
On 2015-01-21 22:47, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
@@ -1488,8 +1501,13 @@ static const char *unpack(int err_fd, struct
shallow_info *si)
argv_array_pushl(child.args, index-pack,
--stdin, hdr_arg, keep_arg, NULL);
- if (fsck_objects)
- argv_array_push(child.args, --strict);
+ if (fsck_objects) {
+ if (fsck_severity.len)
+ argv_array_pushf(child.args, --strict=%s,
+ fsck_severity.buf);
+ else
+ argv_array_push(child.args, --strict);
+ }
Hmm. The above two hunks look suspiciously similar. Would it be
worth to give them a single helper function?
Hmm. Not sure. I see what you mean, but for now I found
+ argv_array_pushf(child.args, --strict%s%s,
+ fsck_severity.len ? = : ,
+ fsck_severity.buf);
to be more elegant than to add a fully-fledged new function. But if
you feel strongly, I will gladly implement a separate function; I
would appreciate suggestions as to the function name...
Peff first introduced that trick elsewhere in our codebase, I think,
but I find it a bit too ugly.
As you accumulate fsck_severity strbuf like this anyway:
strbuf_addf(fsck_severity, %s%s=%s,
fsck_severity.len ? , : , var, value);
to flip what to prefix each element on the list with, I wonder if it
is simpler to change that empty string to =, which will allow you
to say this:
argv_array_pushf(child.args, --strict%s, fsck_severity.buf);
But of course! This is what I did now:
-- snip --
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 8e6d1a1..08e3716 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -126,8 +126,8 @@ static int receive_pack_config(const char *var, const char
*value, void *cb)
}
if (skip_prefix(var, receive.fsck., var)) {
- strbuf_addf(fsck_severity, %s%s=%s,
- fsck_severity.len ? , : , var, value);
+ strbuf_addf(fsck_severity, %c%s=%s,
+ fsck_severity.len ? ',' : '=', var, value);
return 0;
}
@@ -1487,8 +1487,7 @@ static const char *unpack(int err_fd, struct shallow_info
*si)
if (quiet)
argv_array_push(child.args, -q);
if (fsck_objects)
- argv_array_pushf(child.args, --strict%s%s,
- fsck_severity.len ? = : ,
+ argv_array_pushf(child.args, --strict%s,
fsck_severity.buf);
child.no_stdout = 1;
child.err = err_fd;
@@ -1507,8 +1506,7 @@ static const char *unpack(int err_fd, struct shallow_info
*si)
argv_array_pushl(child.args, index-pack,
--stdin, hdr_arg, keep_arg, NULL);
if (fsck_objects)
- argv_array_pushf(child.args, --strict%s%s,
- fsck_severity.len ? = : ,
+ argv_array_pushf(child.args, --strict%s,
fsck_severity.buf);
if (fix_thin)
argv_array_push(child.args, --fix-thin);
-- snap --
Or even this:
strbuf_addf(fsck_strict_arg, %s%s=%s,
fsck_strict_arg.len ? , : --strict=, var, value);
Unfortunately not, because just `--strict` needs to be passed in case no
severity levels were overridden.
In any case, I tend to agree with you that it is overkill to add a
helper function for just to add a single element to the argument
list.
I am glad we agree!
Ciao,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails
On Jan 20, 2015, at 7:22 PM, Junio C Hamano gits...@pobox.com wrote: Dan Langille (dalangil) dalan...@cisco.com writes: I did not test this patch. Is that holding up a commit? I am hoping that you rebuilt the Git you use with this patch by the time you wrote the message I am responding to and have been using it for your daily Git needs ;-) Patch v2 has been used in our test environment with success. I got diverted to other projects before I could test Patch v3. I believe it is queued on the 'next' branch so that others like you who need the change can verify the improvements, and others unlike you who do not need the change can make sure the change does not cause unintended consequences. Thank you. — Dan Langille Infrastructure Operations Talos Group Sourcefire, Inc.
Re: [PATCHv2 1/5] update-ref: test handling large transactions properly
On 01/22/2015 03:32 AM, Stefan Beller wrote:
Signed-off-by: Stefan Beller sbel...@google.com
---
t/t1400-update-ref.sh | 28
1 file changed, 28 insertions(+)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 7b4707b..47d2fe9 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -973,4 +973,32 @@ test_expect_success 'stdin -z delete refs works with
packed and loose refs' '
test_must_fail git rev-parse --verify -q $c
'
+run_with_limited_open_files () {
+ (ulimit -n 32 $@)
+}
Regarding the choice of 32, I wonder what is the worst-case number of
open file descriptors that are needed *before* counting the ones that
are currently wasted on open loose-reference locks. On Linux it seems to
be only 4 with my setup:
$ (ulimit -n 3 git update-ref --stdin /dev/null)
bash: /dev/null: Too many open files
$ (ulimit -n 4 git update-ref --stdin /dev/null)
$
This number might depend a little bit on details of the repository, like
whether config files import other config files. But as long as the
background number of fds required is at least a few less than 32, then
your number should be safe.
Does anybody know of a platform where file descriptors are eaten up
gluttonously by, for example, each shared library that is in use or
something? That's the only think I can think of that could potentially
make your choice of 32 problematic.
[...]
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
--
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] Makefile: do not compile git with debugging symbols by default
On Thu, Jan 22, 2015 at 06:36:41PM +0100, Matthieu Moy wrote: Yes, main point is size of executable. The Git executable is a few megabytes, i.e. 0.001% the size of a really small hard disk. The benefit seems really negligible to me. I don't know the layout of the symbols with respect to the code, or whether the stripped version might reduce memory pressure. So in theory it could have a performance impact. But... OTOH, debug information allow users to do better bug reports in case of crash (gdb, valgrind), which outweights by far the benefit of saving a handfull of megabytes IMHO. Me too. Especially for people who are building git themselves, I feel like leaving the symbols is a sane default. Package builders are already using make strip, or some feature of their package-build system (e.g., dh_strip) to take care of this for the normal users. But fundamentally this is a packaging issue, not a build issue. -Peff PS We could still add a DEBUG knob to the Makefile and default it to off. But I do not see much point. If you want to change the CFLAGS, then change the CFLAGS knob. It's much more flexible. -- 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 20/24] update-index: test the system before enabling untracked cache
Duy Nguyen pclo...@gmail.com writes: On Wed, Jan 21, 2015 at 10:51:02AM -0800, Junio C Hamano wrote: It appears that this hijacks a fixed name dir-mtime-test at the root level of every project managed by Git. Is that intended? I did think about filename clash, but I chose a fixed name anyway for simplicity, otherwise we would need to reconstruct paths dir-mtime-test/... in many places. If you stuff the name of test directory (default dir-mtime-test) in a strbuf and formulate test paths by chomping to the original and then appending /... at the end, like your remove_test_directory() already does, wouldn't that be sufficient? It looks actually good. How about this on top? Yeah, looks cleaner. I am not (yet) enthused by the intrusiveness of the overall series, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 5/5] refs.c: enable large transactions
On Thu, Jan 22, 2015 at 12:24:23PM +0100, Michael Haggerty wrote: I can't figure out where to apply this series or where to fetch it from, so I can't see these changes in context, so maybe I'm misunderstanding something. It looks like this code is doing open(), close(), open(), fdopen(), write(), fclose(), rename() on each lockfile. But don't we have enough information to write the SHA-1 into the lockfile the first time we touch it? I.e., couldn't we reduce this to open(), fdopen(), write(), fclose(), rename() , where the first four calls all happen in the initial loop? If a problem is discovered when writing a later reference, we would roll back the transaction anyway. I understand that this would require a bigger rewrite, so maybe it is not worth it. I had a nagging feeling on the multiple-open thing, too, and would much prefer to just write out the contents early (since we know what they are). It looks like we would just need to split write_ref_sha1() into its two halves: 1. Write out the lockfile 2. Commit the change And then call them at the appropriate spots from ref_transaction_commit(). I guess that is maybe a step backwards for abstracted ref backends, though. -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: [GUILT v4 15/33] Produce legal patch names in guilt-import-commit.
Replacing echo with printf as in your patch works fine for me.
I've applied Signed-off-by lines from you for the latest commits,
rebased it on top of your current master commit, and pushed
the series to the oslo-2014-v5 branch of git://repo.or.cz/guilt/ceder.git
/ceder
On Thu, Jan 22, 2015 at 3:15 PM, Jeff Sipek jef...@josefsipek.net wrote:
I just tried to run the regression suite on my OpenIndiana laptop and I got
this failure.
034: --- t-034.out 2015-01-22 14:02:23.634515474 +
+++ /tmp/guilt.log.148782015-01-22 14:03:54.258790788 +
@@ -83,7 +83,7 @@
[master aedb74f] @
Author: Author Name author@email
1 file changed, 1 insertion(+)
-% create_commit a Backslash\is\forbidden.
+% create_commit a Backslash\is
orbidden.
[master 0a46f8f] Backslash\is\forbidden.
Author: Author Name author@email
1 file changed, 1 insertion(+)
Test failed!
Test: 034
Log file: /tmp/guilt.log.14878
Repo dir: /tmp/guilt reg.12106
make[1]: *** [all] Error 1
It's obviously the cmd command printing that's busted. The following change
makes the test suite pass. Does it work for you? (If so, I'll commit it
after
pulling your whole series.)
Thanks,
Jeff.
diff --git a/regression/scaffold b/regression/scaffold
index 97cff4e..593e9da 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -50,7 +50,7 @@ function filter_dd
# usage: cmd cmd..
function cmd
{
- echo % $@
+ printf %% %s\n $*
if ! (
exec 31
rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
On Sun, May 18, 2014 at 11:59:51PM +0200, Per Cederqvist wrote:
Try harder to create patch names that adhere to the rules in
git-check-ref-format(1) when deriving a patch name from the commit
message. Verify that the derived name using git check-ref-format,
and as a final fallback simply use the patch name x (to ensure that
the code is future-proof in case new rules are added in the future).
Always append a .patch suffix to the patch name.
Added test cases.
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt-import-commit | 20 +-
regression/t-034.out | 567
+++
regression/t-034.sh | 71 +++
3 files changed, 656 insertions(+), 2 deletions(-)
create mode 100644 regression/t-034.out
create mode 100755 regression/t-034.sh
diff --git a/guilt-import-commit b/guilt-import-commit
index f14647c..6260c56 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -28,19 +28,35 @@ disp Current head: `git rev-parse \`git_branch\`` 2
for rev in `git rev-list $rhash`; do
s=`git log --pretty=oneline -1 $rev | cut -c 42-`
+ # 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 \
-e s/['\\[{}]//g -e 's/]//g' -e 's/\*/-/g' \
- -e 's/\?/-/g' | tr A-Z a-z`
+ -e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
+ -e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
+
+ if ! valid_patchname $fname; then
+ # Try harder to make it a legal commit name by
+ # removing all but a few safe characters.
+ fname=`echo $fname|tr -d -c _a-zA-Z0-9---/\\n`
+ fi
+ if ! valid_patchname $fname; then
+ # If we failed to derive a legal patch name, use the
+ # name x. (If this happens, we likely have to
+ # append a suffix to make the name unique.)
+ fname=x
+ fi
disp Converting `echo $rev | cut -c 1-8` as $fname
mangle_prefix=1
fname_base=$fname
- while [ -f $GUILT_DIR/$branch/$fname ]; do
+ while [ -f $GUILT_DIR/$branch/$fname.patch ]; do
fname=$fname_base-$mangle_prefix
mangle_prefix=`expr $mangle_prefix + 1`
disp Patch under that name exists...trying '$fname'
done
+ fname=$fname.patch
(
do_make_header $rev
diff --git a/regression/t-034.out b/regression/t-034.out
new file mode 100644
index 000..7bc9459
--- /dev/null
+++ b/regression/t-034.out
@@ -0,0 +1,567 @@
+% setup_git_repo
+% git tag base
+% create_commit a The sequence /. is forbidden.
+[master eebb76e] The sequence /. is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+ create mode 100644 a
+% create_commit a The sequence .lock/ is forbidden.
+[master 45e81b5] The sequence .lock/ is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a A/component/may/not/end/in/foo.lock
+[master bbf3f59] A/component/may/not/end/in/foo.lock
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Two consecutive
Re: [PATCHv2 5/5] refs.c: enable large transactions
On 01/22/2015 02:10 PM, Jeff King wrote: On Thu, Jan 22, 2015 at 12:24:23PM +0100, Michael Haggerty wrote: I can't figure out where to apply this series or where to fetch it from, so I can't see these changes in context, so maybe I'm misunderstanding something. It looks like this code is doing open(), close(), open(), fdopen(), write(), fclose(), rename() on each lockfile. But don't we have enough information to write the SHA-1 into the lockfile the first time we touch it? I.e., couldn't we reduce this to open(), fdopen(), write(), fclose(), rename() , where the first four calls all happen in the initial loop? If a problem is discovered when writing a later reference, we would roll back the transaction anyway. I understand that this would require a bigger rewrite, so maybe it is not worth it. I had a nagging feeling on the multiple-open thing, too, and would much prefer to just write out the contents early (since we know what they are). It looks like we would just need to split write_ref_sha1() into its two halves: 1. Write out the lockfile 2. Commit the change And then call them at the appropriate spots from ref_transaction_commit(). I guess that is maybe a step backwards for abstracted ref backends, though. Nah, the implementation of ref_transaction_commit() will have to differ between backends anyway. I don't think this would be a step backwards. Michael -- Michael Haggerty mhag...@alum.mit.edu -- 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 16/18] fsck: support demoting errors to warnings
On 12/23/2014 06:14 PM, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: On Tue, 23 Dec 2014, Junio C Hamano wrote: I suspect that it would be much better if the configuration variables were organized the other way around, e.g. $ git config fsck.warn missingTagger,someOtherKindOfError I had something similar in an earlier version of my patch series, but it was shot down rightfully: if you want to allow inheriting defaults from $HOME/.gitconfig, you have to configure the severity levels individually. Hmmm. What's wrong with fsck.warn -missingTagger that overrides the earlier one, or even fsck.info missingTagger after having fsck.warn other,missingTagger,yetanother, with the usual last one wins rule? Whoever shot it down rightfully is wrong here, I would think. Sorry I didn't notice this earlier; Johannes, please CC me on these series, especially the ones that I commented on earlier. I might have been the one who shot down the severity=name style of configuration [1]. I don't feel strongly enough to make a big deal about this, especially considering that the other alternative has already been implemented. But for the record, let me explain why I prefer the name=severity style of configuration. First, it is a truer representation of the data structure within the software, which is basically one severity value for each error type. This is not a decisive argument, but it often means that there is less impedance mismatch between the style of configuration and the concepts that it is configuring. For example, $ git config receive.fsck.warn A,B,C $ git config receive.fsck.error C,D,E seems to be configuring two sets, but it is not. It is mysteriously setting C to be an error, in seeming contradiction of the first line [2]. Second, it is not correct to say that this is just an application of the last setting wins rule. The last setting wins rule has heretofore, as far as I know, only covered *single* settings that take a single value. If we applied that rule to the following: $ git config receive.fsck.warn A,B,C $ git config receive.fsck.warn B,F then the net result would be B,F. But that is not your proposal at all; your proposal is for these two settings to be interpreted the same as $ git config receive.fsck.warn A,B,C,F Similarly, the traditional last setting rule, applied to the first example above, wouldn't cause the value of fsck.warn to be reduced to A,B, as you propose. This is not the last setting rule that we are familiar with--it operates *across and within* values and across *multiple* names rather than just across the values for a single name. Third, the severity=name style is hard to inquire via the command line, and probably also incompatible with the simplified internal config API in git (and probably libgit2, JGit, etc). The problem is that determining a *single* setting requires *three* configuration variables be inquired, and that the settings for those three variables need to be processed in the correct order, including the correct order of interleavings. For example, how would you inquire about the configured severity level of missingTaggerEntry using the shell? It would be a mess that would necessarily have to involve git config --get-regexp and error-prone parsing of comma-separated values. It would be so much easier to type $ git config receive.fsck.missingtaggerentry Fourth, the severity=name style would cause config files to get cluttered up with unused values. Suppose you have earlier run $ git config receive.fsck.warn A,B,C $ git config receive.fsck.ignore D,E and now you want to demote B to ignore. You can do $ git config --add receive.fsck.ignore B (don't forget --add or you've silently erased other, unrelated settings!) This gives the behavior that you want. But now your config file looks like [receive fsck] warn = A,B,C ignore = D,E ignore = B The B on the first line is now just being carried along for no reason, but it would be quite awkward to clean it up programmatically. Effectively, these settings can only be added to but never removed because of the way multiple properties are mashed into a single setting. I believe that one of the main arguments for the severity=name style of configuration is that it carries over more easily into convenient command-line options. But I think it will be unusual to want to configure these options by hand on the command line, let alone adjust many settings at the same time. The idea isn't to make it easy to work with repositories that have a level of breakage that fluctuates over time. It is to make it possible to work with *specific* repositories that have known breakage in their history. For such a repo you would configure one or two ignore options one time and then never adjust them again. (And it will also allow us to make our checks stricter in the future without breaking existing
Re: Git compile warnings (under mac/clang)
On Thu, Jan 22, 2015 at 10:20:01PM +0100, Johannes Schindelin wrote: On 2015-01-22 20:59, Stefan Beller wrote: cc Johannes Schindelin johannes.schinde...@gmx.de who is working in the fsck at the moment On Thu, Jan 22, 2015 at 11:43 AM, Michael Blume blume.m...@gmail.com wrote: CC fsck.o fsck.c:110:38: warning: comparison of unsigned enum expression = 0 is always true [-Wtautological-compare] if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX) ~~ ^ ~ According to A2.5.4 of The C Programming Language 2nd edition: Identifiers declared as enumerators (see Par.A.8.4) are constants of type int. Therefore, the warning is incorrect: any assumption about enum fsck_msg_id to be unsigned is false. I'm not sure that made it to ANSI. C99 says (setion 6.7.2.2, paragraph 4): Each enumerated type shall be compatible with char, a signed integer type, or an unsigned integer type. The choice of type is implementation-defined, but shall be capable of representing the values of all the members of the enumeration. I don't have a copy of C89, but this isn't mentioned in the (very cursory) list of changes found in C99. Anyway, that's academic. I think we dealt with a similar situation before, in 3ce3ffb840a1dfa7fcbafa9309fab37478605d08. -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] t/lib-httpd: switch SANITY check for NOT_ROOT
Torsten Bögershausen tbo...@web.de writes: If I run that sequence manually: chmod 755 . touch x chmod a-w . rm x touch y x is gone, (but shoudn't according to POSIX) y is not created, access denied Good (or is that Sad?). diff --git a/t/test-lib.sh b/t/test-lib.sh --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1039,7 +1039,17 @@ test_lazy_prereq NOT_ROOT ' # When the tests are run as root, permission tests will report that # things are writable when they shouldn't be. test_lazy_prereq SANITY ' - test_have_prereq POSIXPERM,NOT_ROOT + mkdir ds + touch ds/x + chmod -w ds + if rm ds/x + then + chmod +w ds + false + else + chmod +w ds + true + fi ' It looks like a better approach overall. Because we cannot know where $(pwd) is when lazy prereq is evaluated (it typically is at the root of the trash hierarchy, but not always) and we would not want to add, leave or remove random files in the working tree that are not expected by the tests proper (e.g. a test that counts untracked paths are not expecting ds/ to be there), your actual fix may need to be a bit more careful, though. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] l10n updates for 2.3.0
Thanks, all. -- 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] transport-helper: do not request symbolic refs to remote helpers
On Thu, Jan 22, 2015 at 09:52:55AM -0800, Junio C Hamano wrote: Mike Hommey m...@glandium.org writes: The patch changes the behavior in all cases, because it didn't feel necessary to have a different behavior between the normal case and the '?' case: it makes sense to request the ref being pointed at than the symbolic ref in every case. Moreover, this makes existing non-git remote-helpers work without having to modify them to provide a refspec for HEAD (none of the 5 mercurial remote-helpers I checked do). I do not question the latter. It is not surprising if all of them share the same limitation that shares the same root in the same impedance mismatch. The trouble I had in supporting makes sense ... in every case was that you said that the code as patched would not work for a symref pointing at another symref. The original code did not have that problem with remote helpers that support the 'list' command. Does the new code avoid regressions for them and if so how? That is what was needed in the justification. For remote helpers that support the 'list' command, asking for a symref and asking for a ref that the symref points at both work OK and behave the same, and hopefully that would be true even when the latter is a symref that points yet another ref, so dereferencing only one level on our end when making a request, instead of letting the remote side dereference, is not likely to cause regression. If I'm not mistaken, in that case with more than one level of symref, nothing would break more than it already is, the bug would only not be fixed for that case. That said, does this theoretical double indirection actually happen in the wild? For one, afaict, it's not even possible to create such a double indirection with git update-ref. You have to edit a .git/refs/ file manually. Mike -- 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: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
On 2015-01-21 23.33, Junio C Hamano wrote: Are you reporting differences between the state before these patches and after, or just the fact that with these patches the named tests break (which may or may not be broken before the patches)? The intention was to report what is now breaking. One example is this one: - git.git/master: ok 15 # skip Test that git rm -f fails if its rm fails (missing SANITY) git.git/pu: not ok 15 - Test that git rm -f fails if its rm fails # #chmod a-w . #test_must_fail git rm -f baz #chmod 775 . # The next step could be to dig further: If I run that sequence manually: chmod 755 . touch x chmod a-w . rm x touch y x is gone, (but shoudn't according to POSIX) y is not created, access denied -- I can see that there are 3 groups of OS/FS combinations: Group 1: File access bits are not maintained, and not obeyed. Typical: VFAT, Git for Windows, (and some network protocols like SAMBA, depending on the OS/FS involved and/or the mount options) Typically core.filemode is false after git init Group 2: File access bits are maintained and obeyed: POSIX/Unix/Linux/Mac OS and CYGWIN Typically core.filemode is true after git init Group 3 : File access bits are maintained, but not (fully) obeyed running as root under Linux/Unix... Or Windows, when a file is allowed to be deleted from a directory without write permissions. - In short, the following seems to be an improvement: diff --git a/t/test-lib.sh b/t/test-lib.sh --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1039,7 +1039,17 @@ test_lazy_prereq NOT_ROOT ' # When the tests are run as root, permission tests will report that # things are writable when they shouldn't be. test_lazy_prereq SANITY ' - test_have_prereq POSIXPERM,NOT_ROOT + mkdir ds + touch ds/x + chmod -w ds + if rm ds/x + then + chmod +w ds + false + else + chmod +w ds + true + fi ' -- 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] transport-helper: do not request symbolic refs to remote helpers
Mike Hommey m...@glandium.org writes: On Thu, Jan 22, 2015 at 09:52:55AM -0800, Junio C Hamano wrote: Does the new code avoid regressions for them and if so how? That is what was needed in the justification. For remote helpers that support the 'list' command, asking for a symref and asking for a ref that the symref points at both work OK and behave the same, and hopefully that would be true even when the latter is a symref that points yet another ref, so dereferencing only one level on our end when making a request, instead of letting the remote side dereference, is not likely to cause regression. If I'm not mistaken, in that case with more than one level of symref, nothing would break more than it already is, the bug would only not be fixed for that case. Yes, I think we are in agreement. All is well. That said, does this theoretical double indirection actually happen in the wild? With the proliferation of Git-using people and third-party tools that work with Git, I think the value of asking that question has diminished. People do strange things. And I do not think the patch under discussion does not introduce any new theoretical funnies; it fixes one known case and leaves the rest unfixed, without introducing any new breakage, which is perfectly fine and exactly how we want to make progress. If the unfixed one has a real-world need to be fixed, somebody will raise hand, and if they do not bother to even raise their hands, that is an indication that it is not worth our time worrying about it. The only thing we need to avoid, while making one step at a time progress, is to paint ourselves to a corner we cannot get out of by promising too much --- and I do not think the patch under discussion does that, either. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses
On 2015-01-22 21.07, brian m. carlson wrote: On Mon, Jan 19, 2015 at 06:21:24PM +0100, Torsten Bögershausen wrote: When parsing an URL, older Git versions did handle URLs like ssh://2001:db8::1/repo.git the same way as ssh://[2001:db8::1]/repo.git Commit 83b058 broke the parsing of IPV6 adresses without []: It was written in mind that the fist ':' in a URL was the beginning of a port number, while the old code was more clever: Literal IPV6 addresses have always at least two ':'. When the hostandport had a ':' and the rest of the hostandport string could be parsed into an integer between 0 and 65536, then it was used as a port number, like host:22. Otherwise the first ':' was assumed to be part of a literal IPV6 address, like ssh://[2001:db8::1]/repo.git or ssh://[::1]/repo.git. Re-introduce the old parsing in get_host_and_port(). Improve the parsing to handle URLs which have a user name and a literal IPV6 like ssh://user@[2001:db8::1]/repo.git. (Thanks to Christian Taube li...@hcf.yourweb.de for reporting this long standing issue) Another regression was introduced in 83b058: A non-RFC conform URL like [localhost:222] could be used in older versions of Git to connect to run ssh -p 222 localhost. The new stricter parsing did not allow this any more. See even 8d3d28f5dba why [localhost:222] should be declared a feature. I'm not sure this is a very good idea. While this may have worked in the past, it's also completely inconsistent with the way all non-SSH URLs work in Git: vauxhall ok % git push https://bmc@2001:470:1f05:79::1/git/bmc/homedir.git master fatal: unable to access 'https://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/': IPv6 numerical address used in URL without brackets vauxhall no % git push https://bmc@[castro.crustytoothpaste.net]/git/bmc/homedir.git master fatal: unable to access 'https://bmc@[castro.crustytoothpaste.net]/git/bmc/homedir.git/': Could not resolve host: [castro.crustytoothpaste.net] vauxhall no % git push https://bmc@[castro.crustytoothpaste.net:443]/git/bmc/homedir.git master fatal: unable to access 'https://bmc@[castro.crustytoothpaste.net:443]/git/bmc/homedir.git/': Could not resolve host: [castro.crustytoothpaste.net I would argue that people using IPv6 literals in URLs should already know how to do it correctly, and the consistency between SSH and HTTPS URLs is a feature, not a bug. In the non-URL SSH form, you still have to use the bracketed form to deal with the case in which somebody creates a directory called 1 in their home directory and writes: We want to support ssh://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/ because e.g. the Git shipped with Debian (1.7.10.4) (and a lot of other installations) supports it. We want to support ssh://bmc@[2001:470:1f05:79::1]/git/bmc/homedir.git/ because that is what other people may expect to work as well: ssh://bmc@[2001:470:1f05:79::1]:/git/bmc/homedir.git/ git push 2001:470:1f05:79::1:1 master when they mean git push [2001:470:1f05:79::1]:1 master That I don't understand this, where is the path name in your example ? Everything after the first ':' is the path in the short form: bmc@hostxx:/git/bmc/homedir.git/ If you really want to use a literal IPV6 with the short form, you must use the brackets: bmc@[2001:470:1f05:79::1]:/git/bmc/homedir.git/ (And you can not have a port number here) Nobody forces somebody to use any specific form. -- 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] Makefile: do not compile git with debugging symbols by default
On Thu, Jan 22, 2015 at 08:00:36AM -0500, Jeff King wrote: On Thu, Jan 22, 2015 at 06:50:37PM +0600, Alexander Kuleshov wrote: Standard user has no need in debugging information. This patch adds DEBUG=1 option to compile git with debugging symbols and compile without it by default. This explanation is missing why it is beneficial _not_ to have the debugging information. I expect the answer is it makes the executable smaller. And that is true, but it gets smaller still if you run strip on the result: $ make CFLAGS= /dev/null 21 wc -c git 2424248 $ make CFLAGS=-g /dev/null 21 wc -c git 4500816 $ strip git wc -c git 2109200 So I am not sure who this is helping. If you are size-conscious, you should use strip, in which case the -g flag does not matter (and we even have make strip to help you). Is there some other reason to avoid the debugging information? Maybe this comes from the misconception that debugging information changes the generated code, which, in fact, it doesn't. $ make CFLAGS=-g LDFLAGS=-Wl,--build-id=none /dev/null 21 wc -c git 4432768 $ strip --strip-debug git wc -c git 2391120 $ cp git git_ $ make -j4 CFLAGS= LDFLAGS=-Wl,--build-id=none /dev/null 21 wc -c git 2400192 $ strip --strip-debug git wc -c git 2391120 $ diff -s git git_ Files git and git_ are identical LDFLAGS=-Wl,--build-id=none just avoids creating a .note.gnu.build-id section containing a uuid that varies between builds. The 9k difference between unstripped vs stripped for the no-debug-info case comes from the removal of the few symbols for source file names (all the symbols from readelf -s git | grep ABS). Mike -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL] l10n updates for 2.3.0
Hi Junio, The following changes since commit 627736ca799edacf13881da7e671964a0afb94b8: Git 2.3.0-rc1 (2015-01-20 17:35:41 -0800) are available in the git repository at: git://github.com/git-l10n/git-po master for you to fetch changes up to 1e607449135792dd117bd528432fc1fbc1115667: l10n: correct indentation of show-branch usage (2015-01-21 15:35:37 +0800) (from the branch description for master local branch) Git l10n for git.git master branch Jean-Noel Avila (2): l10n: fr.po v2.3.0 round 1 l10n: fr.po v2.3.0 round 2 Jiang Xin (10): Merge branch 'master' of git://github.com/alexhenrie/git-po l10n: git.pot: v2.3.0 round 1 (13 new, 11 removed) l10n: zh_CN: translations for git v2.3.0-rc0 Merge branch 'master' of git://github.com/git-l10n/git-po l10n: git.pot: v2.3.0 round 2 (3 updated) Merge branch 'master' of git://github.com/nafmo/git-l10n-sv Merge branch 'v2.3.0' of git://github.com/jnavila/git l10n: zh_CN: various fixes on command arguments Merge branch 'master' of git://github.com/git-l10n/git-po l10n: correct indentation of show-branch usage Joan Perals (1): l10n: ca.po: various fixes Peter Krefting (2): l10n: sv.po: Update Swedish translation (2298t0f0u) l10n: sv.po: Update Swedish translation (2298t0f0u) Ralf Thielow (4): l10n: de.po: translate track as versionieren l10n: de.po: fix typo l10n: de.po: translate 13 new messages l10n: de.po: translate 3 messages Trần Ngọc Quân (2): l10n: vi.po(2298t): Updated and change Plural-Forms l10n: vi.po(2298t): Updated 3 new strings po/ca.po| 1617 ++-- po/de.po| 1872 +-- po/fr.po| 1813 +- po/git.pot | 1771 + po/sv.po| 1815 +- po/vi.po| 2139 +-- po/zh_CN.po | 1902 ++-- 7 files changed, 6506 insertions(+), 6423 deletions(-) -- Jiang Xin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] refs.c: remove unlock_ref from write_ref_sha1
Instead of calling unlock_ref before each return in write_ref_sha1
we can call this after the call. This is a first step to split up
write_ref_sha1 into the write and commit phase which is done in a
later patch.
There is a call in each code path after write_ref_sha1 now. Even in
the last hunk in the error case, the 'goto cleanup;' will make sure
there is a call to unlock_ref.
Signed-off-by: Stefan Beller sbel...@google.com
---
refs.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/refs.c b/refs.c
index aae3b66..4580919 100644
--- a/refs.c
+++ b/refs.c
@@ -2866,9 +2866,11 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
lock-force_write = 1;
hashcpy(lock-old_sha1, orig_sha1);
if (write_ref_sha1(lock, orig_sha1, logmsg)) {
+ unlock_ref(lock);
error(unable to write current sha1 into %s, newrefname);
goto rollback;
}
+ unlock_ref(lock);
return 0;
@@ -2884,6 +2886,7 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
log_all_ref_updates = 0;
if (write_ref_sha1(lock, orig_sha1, NULL))
error(unable to write current sha1 into %s, oldrefname);
+ unlock_ref(lock);
log_all_ref_updates = flag;
rollbacklog:
@@ -3083,22 +3086,19 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
- if (!lock-force_write !hashcmp(lock-old_sha1, sha1)) {
- unlock_ref(lock);
+ if (!lock-force_write !hashcmp(lock-old_sha1, sha1))
return 0;
- }
+
o = parse_object(sha1);
if (!o) {
error(Trying to write ref %s with nonexistent object %s,
lock-ref_name, sha1_to_hex(sha1));
- unlock_ref(lock);
errno = EINVAL;
return -1;
}
if (o-type != OBJ_COMMIT is_branch(lock-ref_name)) {
error(Trying to write non-commit object %s to branch %s,
sha1_to_hex(sha1), lock-ref_name);
- unlock_ref(lock);
errno = EINVAL;
return -1;
}
@@ -3106,7 +3106,6 @@ static int write_ref_sha1(struct ref_lock *lock,
close_ref(lock) 0) {
int save_errno = errno;
error(Couldn't write %s, lock-lk-filename.buf);
- unlock_ref(lock);
errno = save_errno;
return -1;
}
@@ -3114,7 +3113,6 @@ static int write_ref_sha1(struct ref_lock *lock,
if (log_ref_write(lock-ref_name, lock-old_sha1, sha1, logmsg) 0 ||
(strcmp(lock-ref_name, lock-orig_ref_name)
log_ref_write(lock-orig_ref_name, lock-old_sha1, sha1, logmsg)
0)) {
- unlock_ref(lock);
return -1;
}
if (strcmp(lock-orig_ref_name, HEAD) != 0) {
@@ -3141,10 +3139,8 @@ static int write_ref_sha1(struct ref_lock *lock,
}
if (commit_ref(lock)) {
error(Couldn't set %s, lock-ref_name);
- unlock_ref(lock);
return -1;
}
- unlock_ref(lock);
return 0;
}
@@ -3770,7 +3766,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
goto cleanup;
}
/* Do not keep all lock files open at the same time. */
- close_lock_file(update-lock-lk);
+ close_ref(update-lock);
}
/* Perform updates first so live commits remain referenced */
@@ -3780,13 +3776,13 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
if (!is_null_sha1(update-new_sha1)) {
if (write_ref_sha1(update-lock, update-new_sha1,
update-msg)) {
- update-lock = NULL; /* freed by write_ref_sha1
*/
strbuf_addf(err, Cannot update the ref '%s'.,
update-refname);
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
}
- update-lock = NULL; /* freed by write_ref_sha1 */
+ unlock_ref(update-lock);
+ update-lock = NULL;
}
}
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 0/5] So you dislike the sequence of system calls?
This series goes on top of origin/sb/atomic-push-fix for now. I am inclined to squash the first patch into the last commit of origin/sb/atomic-push-fix when rerolling that series as it just fixes the warning Ramsay pointed out. I'd also like to combine this series with the origin/sb/atomic-push-fix in a reroll of either series such that it becomes one larger series. The patches 2,3,4 are just preparations (no change intended) for the patch 5 which then corrects the sequence of system calls such that we don't close and reopen the lock file. (Background: Why am I spending time to fix that bug the way I do?) I think writing out the sha1 early to the .lock file helps in further patch series for cross repository atomic pushes, because if we can split the transaction_commit into two parts, where the latter part only has lock file pathes in memory, dealing with cross repository ref updates becomes easy in such a way: (compare discussion [RFC] Introducing different handling for small/large transactions) cat EOF stdin_pipe delete topic2 2345 update master 4567 2378 repository ../API-consumer # this switches to another repository delete topic2 3459 update master 6787 9878 EOF git update-ref --stdin stdin_pipe Internally update-ref would be easy to implement when all you have left are lock files you'd need to commit. Any feedback welcome! Thanks, Stefan Stefan Beller (5): fixup for refs.c: enable large transactions refs.c: remove unlock_ref from write_ref_sha1 refs.c: move static functions to close and commit refs refs.c: remove committing the ref from write_ref_sha1 refs.c: write values to lock files early for committing refs.c | 81 -- 1 file changed, 44 insertions(+), 37 deletions(-) -- 2.2.1.62.g3f15098 -- 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 4/5] refs.c: remove committing the ref from write_ref_sha1
This makes write_ref_sha1 only write the the lock file, committing
needs to be done outside of that function. This will help us change
the ref_transaction_commit in a later patch.
Signed-off-by: Stefan Beller sbel...@google.com
---
refs.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 6f3cd7b..c108c95 100644
--- a/refs.c
+++ b/refs.c
@@ -2815,8 +2815,11 @@ static int close_ref(struct ref_lock *lock)
return 0;
}
-static int commit_ref(struct ref_lock *lock)
+static int commit_ref(struct ref_lock *lock, const unsigned char *sha1)
{
+ if (!lock-force_write !hashcmp(lock-old_sha1, sha1))
+ return 0;
+
if (commit_lock_file(lock-lk))
return -1;
return 0;
@@ -2879,7 +2882,8 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
}
lock-force_write = 1;
hashcpy(lock-old_sha1, orig_sha1);
- if (write_ref_sha1(lock, orig_sha1, logmsg)) {
+ if (write_ref_sha1(lock, orig_sha1, logmsg)
+ || commit_ref(lock, orig_sha1)) {
unlock_ref(lock);
error(unable to write current sha1 into %s, newrefname);
goto rollback;
@@ -2898,8 +2902,10 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
lock-force_write = 1;
flag = log_all_ref_updates;
log_all_ref_updates = 0;
- if (write_ref_sha1(lock, orig_sha1, NULL))
+ if (write_ref_sha1(lock, orig_sha1, NULL)
+ || commit_ref(lock, orig_sha1))
error(unable to write current sha1 into %s, oldrefname);
+
unlock_ref(lock);
log_all_ref_updates = flag;
@@ -3137,10 +3143,6 @@ static int write_ref_sha1(struct ref_lock *lock,
!strcmp(head_ref, lock-ref_name))
log_ref_write(HEAD, lock-old_sha1, sha1, logmsg);
}
- if (commit_ref(lock)) {
- error(Couldn't set %s, lock-ref_name);
- return -1;
- }
return 0;
}
@@ -3775,7 +3777,8 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
if (!is_null_sha1(update-new_sha1)) {
if (write_ref_sha1(update-lock, update-new_sha1,
- update-msg)) {
+ update-msg)
+ || commit_ref(update-lock, update-new_sha1)) {
strbuf_addf(err, Cannot update the ref '%s'.,
update-refname);
ret = TRANSACTION_GENERIC_ERROR;
@@ -4064,7 +4067,8 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
} else if (commit_lock_file(reflog_lock)) {
status |= error(unable to commit reflog '%s' (%s),
log_file, strerror(errno));
- } else if ((flags EXPIRE_REFLOGS_UPDATE_REF)
commit_ref(lock)) {
+ } else if ((flags EXPIRE_REFLOGS_UPDATE_REF)
+commit_ref(lock, cb.last_kept_sha1)) {
status |= error(couldn't set %s, lock-ref_name);
}
}
--
2.2.1.62.g3f15098
--
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 5/5] refs.c: write values to lock files early for committing
By writing the values to the lock file early we have a better sequence
of system calls. Before this commit we had
open(), close(), open(), fdopen(), write(), fclose(), rename()
Now there is:
open(), fdopen(), write(), fclose(), rename()
The first four operations are performed in the first loop of
ref_transaction_commit (/* Acquire all locks while verifying
old values */), while the renameing is left to the next loop.
Signed-off-by: Stefan Beller sbel...@google.com
---
refs.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index c108c95..3d1890f 100644
--- a/refs.c
+++ b/refs.c
@@ -3767,6 +3767,15 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-refname);
goto cleanup;
}
+ if (!is_null_sha1(update-new_sha1)) {
+ if (write_ref_sha1(update-lock, update-new_sha1,
+ update-msg)) {
+ strbuf_addf(err, Cannot update the ref '%s'.,
+ update-refname);
+ ret = TRANSACTION_GENERIC_ERROR;
+ goto cleanup;
+ }
+ }
/* Do not keep all lock files open at the same time. */
close_ref(update-lock);
}
@@ -3776,9 +3785,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (!is_null_sha1(update-new_sha1)) {
- if (write_ref_sha1(update-lock, update-new_sha1,
- update-msg)
- || commit_ref(update-lock, update-new_sha1)) {
+ if (commit_ref(update-lock, update-new_sha1)) {
strbuf_addf(err, Cannot update the ref '%s'.,
update-refname);
ret = TRANSACTION_GENERIC_ERROR;
--
2.2.1.62.g3f15098
--
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] diff: make -M -C mean the same as -C -M
While -C implies -M, it is quite common to see both on example command lines
here and there. The unintuitive thing is that if -M appears after -C, then
copy detection is turned off because of how the command line arguments are
handled.
Change this so that when both -C and -M appear, whatever their order, copy
detection is on.
Signed-off-by: Mike Hommey m...@glandium.org
---
Interestingly, I even found mentions of -C -M in this order for benchmarks,
on this very list (see 6555655.XSJ9EnW4BY@mako).
diff.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index d1bd534..9081fd8 100644
--- a/diff.c
+++ b/diff.c
@@ -3670,7 +3670,8 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
!strcmp(arg, --find-renames)) {
if ((options-rename_score = diff_scoreopt_parse(arg)) == -1)
return error(invalid argument to -M: %s, arg+2);
- options-detect_rename = DIFF_DETECT_RENAME;
+ if (options-detect_rename != DIFF_DETECT_COPY)
+ options-detect_rename = DIFF_DETECT_RENAME;
}
else if (!strcmp(arg, -D) || !strcmp(arg, --irreversible-delete)) {
options-irreversible_delete = 1;
--
2.2.2.2.g806f5e2.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
Git submodule first time update with proxy
I have a submodule using HTTP URL. I do this: $ git submodule init MySubmodule $ git submodule update MySubmodule The 2nd command fails because the HTTP URL cannot be resolved, this is because it requires a proxy. I have http.proxy setup properly in the .git/config of my parent git repository, so I was hoping the submodule update function would have a way to specify it to inherit the proxy value from the parent config. How can I set up my submodule? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses
On Thu, Jan 22, 2015 at 11:05:29PM +0100, Torsten Bögershausen wrote: We want to support ssh://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/ because e.g. the Git shipped with Debian (1.7.10.4) (and a lot of other installations) supports it. I understand that this used to work, but it probably shouldn't have ever been accepted. It's nonstandard, and if we accept it for ssh, people will want it to work for https, and due to libcurl, it simply won't. I prefer to see our past acceptance of this format as a bug. This is the first that I've heard of anyone noticing this (since 2013), so it can't be in common usage. If we accept it, we should explicitly document it as being deprecated and note that it's inconsistent with the way everything else works. We want to support ssh://bmc@[2001:470:1f05:79::1]/git/bmc/homedir.git/ because that is what other people may expect to work as well: ssh://bmc@[2001:470:1f05:79::1]:/git/bmc/homedir.git/ Everyone expects this to work properly, because it's the standard URL form (RFC 2732). I agree we should support it. git push 2001:470:1f05:79::1:1 master when they mean git push [2001:470:1f05:79::1]:1 master That I don't understand this, where is the path name in your example ? The path in question is $HOME/1. That's why the bracket notation is obligatory in the short form. I agree it's a bit bizarre. Everything after the first ':' is the path in the short form: bmc@hostxx:/git/bmc/homedir.git/ If you really want to use a literal IPV6 with the short form, you must use the brackets: bmc@[2001:470:1f05:79::1]:/git/bmc/homedir.git/ (And you can not have a port number here) Right. In my experience, nobody uses the ssh:// form unless they have to (i.e. they need to use a port number); it's extremely uncommon. So they've already become used to using the bracketed notation, because it's already required for the usual form and it's required in the IPv6 URL standard. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH v2 1/4] apply.c: typofix
On Thu, Jan 22, 2015 at 3:42 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: -* it might with whitespace fuzz. We haven't been asked to +* it might with whitespace fuzz. We weren't asked to (not a native speaker): A quick websearch reveals We haven't been asked to ... is quite commonly used in the web. So it's more of a grammar fix or a rewording of a comment instead of a typofix(which I assume are miss spellings) Correct; it is not grammo or typo fix, but is a rephrasing to match the tense with what follows (i.e. we weren't asked to ignore; we were asked to fix). Ok, I realized I missed my conclusion in the first mail: The commit message and what the patch actually does, do not match. -- 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] Makefile: do not compile git with debugging symbols by default
On Thu, Jan 22, 2015 at 01:35:38PM -0500, Jeff King wrote: On Thu, Jan 22, 2015 at 06:36:41PM +0100, Matthieu Moy wrote: Yes, main point is size of executable. The Git executable is a few megabytes, i.e. 0.001% the size of a really small hard disk. The benefit seems really negligible to me. I don't know the layout of the symbols with respect to the code, or whether the stripped version might reduce memory pressure. So in theory it could have a performance impact. It doesn't. Debugging info is in a part of the file that is not mapped in memory, and in a part that can be removed without affecting the rest of the file, so it's more or less at the end. Mike -- 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 1/4] apply.c: typofix
On Thu, Jan 22, 2015 at 2:58 PM, Junio C Hamano gits...@pobox.com wrote: Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/apply.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/apply.c b/builtin/apply.c index 622ee16..31f8733 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -2326,7 +2326,7 @@ static int match_fragment(struct image *img, /* * The hunk does not apply byte-by-byte, but the hash says -* it might with whitespace fuzz. We haven't been asked to +* it might with whitespace fuzz. We weren't asked to (not a native speaker): A quick websearch reveals We haven't been asked to ... is quite commonly used in the web. So it's more of a grammar fix or a rewording of a comment instead of a typofix(which I assume are miss spellings) * ignore whitespace, we were asked to correct whitespace * errors, so let's try matching after whitespace correction. * -- 2.3.0-rc1-116-g84c5016 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] apply.c: typofix
Stefan Beller sbel...@google.com writes: -* it might with whitespace fuzz. We haven't been asked to +* it might with whitespace fuzz. We weren't asked to (not a native speaker): A quick websearch reveals We haven't been asked to ... is quite commonly used in the web. So it's more of a grammar fix or a rewording of a comment instead of a typofix(which I assume are miss spellings) Correct; it is not grammo or typo fix, but is a rephrasing to match the tense with what follows (i.e. we weren't asked to ignore; we were asked to fix). * ignore whitespace, we were asked to correct whitespace * errors, so let's try matching after whitespace correction. * -- 2.3.0-rc1-116-g84c5016 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: add git apply whitespace expansion tests
On Jan 22, 2015, at 11:23, Junio C Hamano wrote: Kyle J. McKay mack...@gmail.com writes: On Jan 21, 2015, at 14:33, Junio C Hamano wrote: Kyle J. McKay mack...@gmail.com writes: So since I've not been able to get test 2 or 3 to core dump (even before 250b3c6c) I tend to believe you are correct in that the code thinks (incorrectly) that the result should fit within the buffer. Thanks; let me steal your tests when I reroll. Awesome. :) But please squash in this tiny change if using the tests verbatim: Thanks. I actually have a question wrt the need for $MAKE_PATCHES. It would have been more natural to do something like: test_expect_success 'setup' ' printf \t%s\n 1 2 3 4 5 6 before printf \t%s\n 1 2 3 after printf %64s\n a b c after printf \t%s\n 4 5 6 after git diff --no-index before after | sed -e s/before/test-1/ -e s/after/test-1/ patch1.patch printf %64s\n 1 2 3 4 5 6 test-1 printf %64s\n 1 2 3 a b c 4 5 6 expect-1 printf \t%s\n a b c d e f before printf \t%s\n a b c after ... cat test-4 expect-4 printf %64s\n a b c expect-4 while test $x -lt 100 do printf %63s%02d\n $x test-4 printf %63s%02d\n $x expect-4 x=$(( $x + 1 )) done git config core.whitespace tab-in-indent,tabwidth=63 git config apply.whitespace fix ' test_expect_success 'apply with ws expansion (1)' ' git apply patch1.patch test_cmp test-1 expect-1 ' and if you want test files you can just skip tests #2 and later, without introducing an ad-hoc mechanism like you did. Was there something more than that that you wanted from $MAKE_PATCHES? Well, see I found t/t4135/make-patches that makes patches for use by t4135-apply-weird-filenames.sh and thought perhaps that was the approved way to do things. But then it seemed overkill since making the patches takes so little time it didn't seem to warrant a separate directory. But the ability to just make the patch files without requiring any external scripts or test framework seemed nice so I added those two extra lines to make it possible. I don't have any strong feelings about it. The setup test plus 4 explicit tests looks fine. In any case, here is an update to that sanity check patch to catch the two cases the BUG did not trigger. Sometimes the caller under-counted the size of the result but thought that it would still fit within the original (hence allowing us to update in-place by passing postlen==0) but the actual result was larger than the space we have allocated in the postimage, clobbering the piece of memory after the postimage-buf. diff --git a/builtin/apply.c b/builtin/apply.c index 31f8733..3b7ba63 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -2171,6 +2171,12 @@ static void update_pre_post_images(struct image *preimage, ctx++; } + if (postlen + ? postlen new - postimage-buf + : postimage-len new - postimage-buf) + die(BUG: caller miscounted postlen: asked %d, orig = %d, used = %d, + (int)postlen, (int) postimage-len, (int)(new - postimage- buf)); + /* Fix the length of the whole thing */ postimage-len = new - postimage-buf; postimage-nr -= reduced; Nice. No more of those bogus results can slip through that somehow evade evoking a core dump. -Kyle -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/4] apply --whitespace=fix buffer corruption fix
git apply --whitespace=fix used to be able to assume that fixing errors will always reduce the size by e.g. stripping whitespaces at the end of lines or collapsing runs of spaces into tabs at the beginning of lines. An update to accomodate fixes that lengthens the result by e.g. expanding leading tabs into spaces were made long time ago but the logic miscounted the necessary space after such whitespace fixes, leading to either under-allocation or over-usage of already allocated space. The second patch in this series is to illustrate this with a runtime sanity-check to protect us from future breakage (this is a reroll of a how about this weatherbaloon patch $gmane/262579, with Kyle's test script). The third patch corrects the under-counting and makes the new test pass. The fourth patch makes us report the fact that we corrected whitespace errors in the common-context part. When asked to correct whitespace errors, and given a patch that has whitespace errors in the common context (i.e. the lines prefixed with ) either in the patch itself or the corresponding part in the file we are patching, we have been correcting the whitespace errors while at it since around c1beba5b (git-apply --whitespace=fix: fix whitespace fuzz introduced by previous run, 2008-01-30), but we were not reporting that we saw a need to fix them. This is not about the buffer corruption fix, which is the primary focus of this series, but it is here because it is related to whitespace fix. Junio C Hamano (4): typofix apply: make update_pre_post_images() sanity check the given postlen apply: count the size of postimage correctly apply: detect and mark whitespace errors in context lines when fixing builtin/apply.c | 34 ++-- t/t4138-apply-ws-expansion.sh | 121 ++ 2 files changed, 152 insertions(+), 3 deletions(-) create mode 100755 t/t4138-apply-ws-expansion.sh -- 2.3.0-rc1-116-g84c5016 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] apply: count the size of postimage correctly
Under --whitespace=fix option, match_fragment() function examines
the preimage (the common context and the removed lines in the patch)
and the file being patched and checks if they match after correcting
all whitespace errors. When they are found to match, the common
context lines in the preimage is replaced with the fixed copy,
because these lines will then be copied to the corresponding place
in the postimage by a later call to update_pre_post_images(). Lines
that are added in the postimage, under --whitespace=fix, have their
whitespace errors already fixed when apply_one_fragment() prepares
the preimage and the postimage, so in the end, application of the
patch can be done by replacing the block of text in the file being
patched that matched the preimage with what is in the postimage that
was updated by update_pre_post_images().
In the earlier days, fixing whitespace errors always resulted in
reduction of size, either collapsing runs of spaces in the indent to
a tab or removing the trailing whitespaces. These days, however,
some whitespace error fix results in extending the size.
250b3c6c (apply --whitespace=fix: avoid running over the postimage
buffer, 2013-03-22) tried to compute the final postimage size but
its math was flawed. It counted the size of the block of text in
the original being patched after fixing the whitespace errors on its
lines that correspond to the preimage. That number does not have
much to do with how big the final postimage would be.
Instead count (1) the added lines in the postimage, whose size is
the same as in the final patch result because their whitespace
errors have already been corrected, and (2) the fixed size of the
lines that are common.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index da6fb35..e895340 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2336,6 +2336,23 @@ static int match_fragment(struct image *img,
* ignore whitespace, we were asked to correct whitespace
* errors, so let's try matching after whitespace correction.
*
+* While checking the preimage against the target, whitespace
+* errors in both fixed, we count how large the corresponding
+* postimage needs to be. The postimage prepared by
+* apply_one_fragment() has whitespace errors fixed on added
+* lines already, but the common lines were propagated as-is,
+* which may become longer when their whitespace errors are
+* fixed.
+*/
+
+ /* First count added lines in postimage */
+ postlen = 0;
+ for (i = 0; i postimage-nr; i++) {
+ if (!(postimage-line[i].flag LINE_COMMON))
+ postlen += postimage-line[i].len;
+ }
+
+ /*
* The preimage may extend beyond the end of the file,
* but in this loop we will only handle the part of the
* preimage that falls within the file.
@@ -2343,7 +2360,6 @@ static int match_fragment(struct image *img,
strbuf_init(fixed, preimage-len + 1);
orig = preimage-buf;
target = img-buf + try;
- postlen = 0;
for (i = 0; i preimage_limit; i++) {
size_t oldlen = preimage-line[i].len;
size_t tgtlen = img-line[try_lno + i].len;
@@ -2371,7 +2387,10 @@ static int match_fragment(struct image *img,
match = (tgtfix.len == fixed.len - fixstart
!memcmp(tgtfix.buf, fixed.buf + fixstart,
fixed.len - fixstart));
- postlen += tgtfix.len;
+
+ /* Add the length if this is common with the postimage */
+ if (preimage-line[i].flag LINE_COMMON)
+ postlen += tgtfix.len;
strbuf_release(tgtfix);
if (!match)
--
2.3.0-rc1-116-g84c5016
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] apply: detect and mark whitespace errors in context lines when fixing
When the incoming patch has whitespace errors in a common context line (i.e. a line that is expected to be found and is not modified by the patch), apply --whitespace=fix corrects the whitespace errors the line has, in addition to the whitespace error on a line that is updated by the patch. However, we did not count and report that we fixed whitespace errors on such lines. [jc: This is iffy. What if the whitespace error has been fixed in the target since the patch was written? A common context line we see in the patch has errors, and it matches a line in the target that has the errors already corrected, resulting in no change, which we may not want to count after all. On the other hand, we are reporting whitespace errors _in_ the incoming patch, so...] Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/apply.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/apply.c b/builtin/apply.c index e895340..a51a1b0 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -1606,6 +1606,9 @@ static int parse_fragment(const char *line, unsigned long size, if (!deleted !added) leading++; trailing++; + if (!apply_in_reverse + ws_error_action == correct_ws_error) + check_whitespace(line, len, patch-ws_rule); break; case '-': if (apply_in_reverse -- 2.3.0-rc1-116-g84c5016 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] apply: make update_pre_post_images() sanity check the given postlen
git apply --whitespace=fix used to be able to assume that fixing errors will always reduce the size by e.g. stripping whitespaces at the end of lines or collapsing runs of spaces into tabs at the beginning of lines. An update to accomodate fixes that lengthens the result by e.g. expanding leading tabs into spaces were made long time ago but the logic miscounted the necessary space after such whitespace fixes, leading to either under-allocation or over-usage of already allocated space. Illustrate this with a runtime sanity-check to protect us from future breakage. The test was stolen from Kyle McKay who helped to identify the problem. Helped-by: Kyle J. McKay mack...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/apply.c | 6 +++ t/t4138-apply-ws-expansion.sh | 121 ++ 2 files changed, 127 insertions(+) create mode 100755 t/t4138-apply-ws-expansion.sh diff --git a/builtin/apply.c b/builtin/apply.c index 31f8733..da6fb35 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -2171,6 +2171,12 @@ static void update_pre_post_images(struct image *preimage, ctx++; } + if (postlen + ? postlen new - postimage-buf + : postimage-len new - postimage-buf) + die(BUG: caller miscounted postlen: asked %d, orig = %d, used = %d, + (int)postlen, (int) postimage-len, (int)(new - postimage-buf)); + /* Fix the length of the whole thing */ postimage-len = new - postimage-buf; postimage-nr -= reduced; diff --git a/t/t4138-apply-ws-expansion.sh b/t/t4138-apply-ws-expansion.sh new file mode 100755 index 000..0ffe33f --- /dev/null +++ b/t/t4138-apply-ws-expansion.sh @@ -0,0 +1,121 @@ +#!/bin/sh +# +# Copyright (C) 2015 Kyle J. McKay +# + +test_description='git apply test patches with whitespace expansion.' + +. ./test-lib.sh + +test_expect_success setup ' + # + ## create test-N, patchN.patch, expect-N files + # + + # test 1 + printf \t%s\n 1 2 3 4 5 6 before + printf \t%s\n 1 2 3 after + printf %64s\n a b c after + printf \t%s\n 4 5 6 after + git diff --no-index before after | + sed -e s/before/test-1/ -e s/after/test-1/ patch1.patch + printf %64s\n 1 2 3 4 5 6 test-1 + printf %64s\n 1 2 3 a b c 4 5 6 expect-1 + + # test 2 + printf \t%s\n a b c d e f before + printf \t%s\n a b c after + n=10 + x=1 + while test $x -lt $n + do + printf %63s%d\n $x after + x=$(( $x + 1 )) + done + printf \t%s\n d e f after + git diff --no-index before after | + sed -e s/before/test-2/ -e s/after/test-2/ patch2.patch + printf %64s\n a b c d e f test-2 + printf %64s\n a b c expect-2 + x=1 + while test $x -lt $n + do + printf %63s%d\n $x expect-2 + x=$(( $x + 1 )) + done + printf %64s\n d e f expect-2 + + # test 3 + printf \t%s\n a b c d e f before + printf \t%s\n a b c after + n=100 + x=0 + while test $x -lt $n + do + printf %63s%02d\n $x after + x=$(( $x + 1 )) + done + printf \t%s\n d e f after + git diff --no-index before after | + sed -e s/before/test-3/ -e s/after/test-3/ patch3.patch + printf %64s\n a b c d e f test-3 + printf %64s\n a b c expect-3 + x=0 + while test $x -lt $n + do + printf %63s%02d\n $x expect-3 + x=$(( $x + 1 )) + done + printf %64s\n d e f expect-3 + + # test 4 + before + x=0 + while test $x -lt 50 + do + printf \t%02d\n $x before + x=$(( $x + 1 )) + done + cat before after + printf %64s\n a b c after + while test $x -lt 100 + do + printf \t%02d\n $x before + printf \t%02d\n $x after + x=$(( $x + 1 )) + done + git diff --no-index before after | + sed -e s/before/test-4/ -e s/after/test-4/ patch4.patch + test-4 + x=0 + while test $x -lt 50 + do + printf %63s%02d\n $x test-4 + x=$(( $x + 1 )) + done + cat test-4 expect-4 + printf %64s\n a b c expect-4 + while test $x -lt 100 + do + printf %63s%02d\n $x test-4 + printf %63s%02d\n $x expect-4 + x=$(( $x + 1 )) + done + + git config core.whitespace tab-in-indent,tabwidth=63 + git config apply.whitespace fix + +' + +# Note that `patch` can successfully apply all patches when run +# with the --ignore-whitespace option. + +for t in 1 2 3 4 +do + test_expect_success 'apply with ws expansion (t=$t)' ' + git apply patch$t.patch
[PATCH v2 1/4] apply.c: typofix
Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/apply.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/apply.c b/builtin/apply.c index 622ee16..31f8733 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -2326,7 +2326,7 @@ static int match_fragment(struct image *img, /* * The hunk does not apply byte-by-byte, but the hash says -* it might with whitespace fuzz. We haven't been asked to +* it might with whitespace fuzz. We weren't asked to * ignore whitespace, we were asked to correct whitespace * errors, so let's try matching after whitespace correction. * -- 2.3.0-rc1-116-g84c5016 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] fixup for refs.c: enable large transactions
This will fix the warnings Ramsay Jones pointed out.
Signed-off-by: Stefan Beller sbel...@google.com
---
refs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 9fc0e60..aae3b66 100644
--- a/refs.c
+++ b/refs.c
@@ -3058,12 +3058,12 @@ static int write_sha1_to_lock_file(struct ref_lock
*lock,
{
if (lock-lk-fd == -1) {
if (reopen_lock_file(lock-lk) 0
- || fdopen_lock_file(lock-lk, w) 0
+ || fdopen_lock_file(lock-lk, w) == NULL
|| fprintf(lock-lk-fp, %s\n, sha1_to_hex(sha1)) != 41
|| close_lock_file(lock-lk) 0)
return -1;
} else {
- if (fdopen_lock_file(lock-lk, w) 0
+ if (fdopen_lock_file(lock-lk, w) == NULL
|| fprintf(lock-lk-fp, %s\n, sha1_to_hex(sha1)) != 41)
return -1;
}
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] refs.c: move static functions to close and commit refs
By moving the functions up we don't need to have to declare them first
when using them in a later patch.
Signed-off-by: Stefan Beller sbel...@google.com
---
refs.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/refs.c b/refs.c
index 4580919..6f3cd7b 100644
--- a/refs.c
+++ b/refs.c
@@ -2808,6 +2808,20 @@ static int rename_ref_available(const char *oldname,
const char *newname)
static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
const char *logmsg);
+static int close_ref(struct ref_lock *lock)
+{
+ if (close_lock_file(lock-lk))
+ return -1;
+ return 0;
+}
+
+static int commit_ref(struct ref_lock *lock)
+{
+ if (commit_lock_file(lock-lk))
+ return -1;
+ return 0;
+}
+
int rename_ref(const char *oldrefname, const char *newrefname, const char
*logmsg)
{
unsigned char sha1[20], orig_sha1[20];
@@ -2901,20 +2915,6 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
return 1;
}
-static int close_ref(struct ref_lock *lock)
-{
- if (close_lock_file(lock-lk))
- return -1;
- return 0;
-}
-
-static int commit_ref(struct ref_lock *lock)
-{
- if (commit_lock_file(lock-lk))
- return -1;
- return 0;
-}
-
/*
* copy the reflog message msg to buf, which has been allocated sufficiently
* large, while cleaning up the whitespaces. Especially, convert LF to space,
--
2.2.1.62.g3f15098
--
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
Should copy/rename detection consider file overwrites?
Hi, While fooling around with copy/rename detection, I noticed that it doesn't detect the case where you copy or rename a file on top of another: $ git init $ (echo foo; echo bar) foo $ git add foo $ git commit -m foo $ echo 0 bar $ git add bar $ git commit -m bar $ git mv -f foo bar $ git commit -m foobar $ git log --oneline --reverse 7dc2765 foo b0c837d bar 88caeba foobar $ git blame -s -C -C bar 88caebab 1) foo 88caebab 2) bar I can see how this is not trivially representable in e.g. git diff-tree, but shouldn't at least blame try to tell that those lines actually come from 7dc2765? Mike -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC grep '...\{m,n\}?
On Fri, Jan 23, 2015 at 3:19 AM, Junio C Hamano gits...@pobox.com wrote:
Kirill A. Shutemov kirill.shute...@linux.intel.com writes:
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8197ed29a9ec..a31f7e0430e1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1039,4 +1039,11 @@ test_expect_success 'short SHA-1 collide' '
)
'
+test_expect_success 'respect core.abbrev' '
+ git config core.abbrev 12
+ set_cat_todo_editor
+ test_must_fail git rebase -i HEAD~4 todo-list
+ test 4 = $(grep -c pick [0-9a-f]\{12,\} todo-list)
+'
Documentation/CodingGuidelines says
- As to use of grep, stick to a subset of BRE (namely, no \{m,n\},
[::], [==], or [..]) for portability.
- We do not use \{m,n\};
- We do not use -E;
- We do not use ? or + (which are \{0,1\} and \{1,\}
respectively in BRE) but that goes without saying as these
are ERE elements not BRE (note that \? and \+ are not even part
of BRE -- making them accessible from BRE is a GNU extension).
but I see we have multiple hits from git grep 'grep .*\\{' (all in
the t/ directory). I wonder
- if everybody's system is now OK with \{m,n\} these days, or
- there are people who are grateful that we stayed away from using
\{m,n\} but they are not running the tests because their system
is too exotic to pass other parts of the test suite.
Can we switch to using git-grep in the test suite and avoid these
platform issues? Or maybe switch to test-grep.c that is a wrapper of
compat regex to make sure the same feature set is used across
platforms.
--
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: [msysGit] Re: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
On 2015-01-22 23.07, Junio C Hamano wrote: Torsten Bögershausen tbo...@web.de writes: If I run that sequence manually: chmod 755 . touch x chmod a-w . rm x touch y x is gone, (but shoudn't according to POSIX) y is not created, access denied Good (or is that Sad?). It feels that this is by design: In old days under MS/DOS the only way to hinder people from deleting a file was to make it read only with help of the ATTRIB command. https://en.wikipedia.org/wiki/ATTRIB Later NTFS introduced the (ACL like) secutity information, and (unless I am completely wrong) the delete file is part of the modify permission, not write. diff --git a/t/test-lib.sh b/t/test-lib.sh --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1039,7 +1039,17 @@ test_lazy_prereq NOT_ROOT ' # When the tests are run as root, permission tests will report that # things are writable when they shouldn't be. test_lazy_prereq SANITY ' - test_have_prereq POSIXPERM,NOT_ROOT + mkdir ds + touch ds/x + chmod -w ds + if rm ds/x + then + chmod +w ds + false + else + chmod +w ds + true + fi ' It looks like a better approach overall. Because we cannot know where $(pwd) is when lazy prereq is evaluated (it typically is at the root of the trash hierarchy, but not always) and we would not want to add, leave or remove random files in the working tree that are not expected by the tests proper (e.g. a test that counts untracked paths are not expecting ds/ to be there), your actual fix may need to be a bit more careful, though. Thanks. So true, what is a better place or way to run the test ? Can we use /tmp (Which may be a different file system)? Or can we use $HOME/$$ds (Which is an artificial HOME) We already pollute the $PWD here test_lazy_prereq CASE_INSENSITIVE_FS ' echo good CamelCase echo bad camelcase test $(cat CamelCase) != good ' and here: test_lazy_prereq UTF8_NFD_TO_NFC ' Would mkdir $HOME/ds be a better approach then ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git index containing tree extension for unknown path
Thanks for the explanation. In my case it seems we have an invalid index. I modified the gin script [1] to inspect the index and found valid cached trees in the index for pathes for which the index has no entries. Will try to find out who/how the index was corrupted. [1] https://raw.githubusercontent.com/chalstrick/gin/master/gin Output of the script gin.py [header] signature = DIRC version = 2 entries = 1 [entry] entry = 1 ctime = 1421834321.9923096 mtime = 1420815554.0 dev = 2049 ino = 658461 mode = 100644 uid = 1000 gid = 1000 size = 42 sha1 = 6c6909e2b92e763e890a9a42695680762802a50a flags = 14 assume-valid = False extended = False stage = (False, False) name = .gitattributes [extension] extension = 1 signature = TREE size = 103 [cachedTree] name = entryCnt = 2 subtreeCnt = 1 id = f59374e16868ce8385b4ee602c737f917da4af1b [cachedTree] name = a entryCnt = 1 subtreeCnt = 1 id = 4b5e60c30485a91ee60bd869ecc3b4d2b892fc3f [cachedTree] name = b entryCnt = 1 subtreeCnt = 1 id = 50bd519cc418878d4b0ffc27348afd710c188d6d [cachedTree] name = c entryCnt = 1 subtreeCnt = 0 id = 2b230fa0f3d19b497d3dd24e835691e3a921657f [checksum] checksum = True sha1 = 124680d4dec4758ee1ae28f546659d282952ebff Ciao Chris Ciao Chris On Wed, Jan 21, 2015 at 9:39 PM, Junio C Hamano gits...@pobox.com wrote: Christian Halstrick christian.halstr...@gmail.com writes: Is it allowed that the git index contains a tree extension mentioning patch 'x/y/z' while the only entry in the index is a '.gitattributes' files in the root? Depends on the definition of mention, but it is not unexpected that you see x, y, and z in the cache-tree extension as invalidated nodes after you do something like this: rm -fr test git init test cd test mkdir -p x/y/z x/y/z/1 git add x git write-tree # cache-tree is fully valid mv x/y/z x/y/a git add x # cache-tree invalidated z, if appears, should still know that y is its parent and y, if appears, should still know that x is its parent. All of the three should say they have been invalidated by showing a negative entry-count and show the correct subtree count that appear in the extension (i.e. if z is there as an invalidated leaf, it should say -1 0 to indicate an invalidated entry by a negative entry count, with zero subtrees, and y would show -1 1 to indicate an invalidated entry with one subtree, namely z, etc.). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 5/5] refs.c: enable large transactions
How do you run sparse on git?
I noticed there is 'make sparse' though I cannot get it working
here in the corporate world as I have problems with openssl
headers not being found.
Also the line numbers seem to bit off compared to what I have
here, did you need to modify/preprocess files to get sparse running?
As for the fix, would it be sufficient to check != NULL instead of 0?
Thanks,
Stefan
On Thu, Jan 22, 2015 at 4:59 AM, Ramsay Jones
ram...@ramsay1.demon.co.uk wrote:
On 22/01/15 02:32, Stefan Beller wrote:
By closing the file descriptors after creating the lock file we are not
limiting the size of the transaction by the number of available file
descriptors.
Signed-off-by: Stefan Beller sbel...@google.com
---
refs.c| 17 +
t/t1400-update-ref.sh | 4 ++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 2013d37..9d01102 100644
--- a/refs.c
+++ b/refs.c
@@ -3055,11 +3055,18 @@ int is_branch(const char *refname)
static int write_sha1_to_lock_file(struct ref_lock *lock,
const unsigned char *sha1)
{
- if (fdopen_lock_file(lock-lk, w) 0
- || fprintf(lock-lk-fp, %s\n, sha1_to_hex(sha1)) != 41)
+ if (lock-lk-fd == -1) {
+ if (reopen_lock_file(lock-lk) 0
+ || fdopen_lock_file(lock-lk, w) 0
fdopen_lock_file() returns a 'FILE *', so this causes sparse to bark:
refs.c:3105:56: error: incompatible types for operation ()
refs.c:3105:56:left side has type struct _IO_FILE [usertype] *
refs.c:3105:56:right side has type int
+ || fprintf(lock-lk-fp, %s\n, sha1_to_hex(sha1)) != 41
+ || close_lock_file(lock-lk) 0)
+ return -1;
+ } else {
+ if (fdopen_lock_file(lock-lk, w) 0
Similarly, sparse barks:
refs.c:3110:53: error: incompatible types for operation ()
refs.c:3110:53:left side has type struct _IO_FILE [usertype] *
refs.c:3110:53:right side has type int
+ || fprintf(lock-lk-fp, %s\n, sha1_to_hex(sha1)) != 41)
return -1;
- else
- return 0;
+ }
+ return 0;
}
/*
@@ -3761,6 +3768,8 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-refname);
goto cleanup;
}
+ /* Do not keep all lock files open at the same time. */
+ close_lock_file(update-lock-lk);
}
/* Perform updates first so live commits remain referenced */
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 47d2fe9..c593a1d 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -979,7 +979,7 @@ run_with_limited_open_files () {
test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating
branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating
branches does not burst open file limit' '
(
for i in $(test_seq 33)
do
@@ -990,7 +990,7 @@ test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large
transaction creating branches
)
'
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction deleting
branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction deleting
branches does not burst open file limit' '
(
for i in $(test_seq 33)
do
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: add git apply whitespace expansion tests
Kyle J. McKay mack...@gmail.com writes: On Jan 21, 2015, at 14:33, Junio C Hamano wrote: Kyle J. McKay mack...@gmail.com writes: So since I've not been able to get test 2 or 3 to core dump (even before 250b3c6c) I tend to believe you are correct in that the code thinks (incorrectly) that the result should fit within the buffer. Thanks; let me steal your tests when I reroll. Awesome. :) But please squash in this tiny change if using the tests verbatim: Thanks. I actually have a question wrt the need for $MAKE_PATCHES. It would have been more natural to do something like: test_expect_success 'setup' ' printf \t%s\n 1 2 3 4 5 6 before printf \t%s\n 1 2 3 after printf %64s\n a b c after printf \t%s\n 4 5 6 after git diff --no-index before after | sed -e s/before/test-1/ -e s/after/test-1/ patch1.patch printf %64s\n 1 2 3 4 5 6 test-1 printf %64s\n 1 2 3 a b c 4 5 6 expect-1 printf \t%s\n a b c d e f before printf \t%s\n a b c after ... cat test-4 expect-4 printf %64s\n a b c expect-4 while test $x -lt 100 do printf %63s%02d\n $x test-4 printf %63s%02d\n $x expect-4 x=$(( $x + 1 )) done git config core.whitespace tab-in-indent,tabwidth=63 git config apply.whitespace fix ' test_expect_success 'apply with ws expansion (1)' ' git apply patch1.patch test_cmp test-1 expect-1 ' and if you want test files you can just skip tests #2 and later, without introducing an ad-hoc mechanism like you did. Was there something more than that that you wanted from $MAKE_PATCHES? In any case, here is an update to that sanity check patch to catch the two cases the BUG did not trigger. Sometimes the caller under-counted the size of the result but thought that it would still fit within the original (hence allowing us to update in-place by passing postlen==0) but the actual result was larger than the space we have allocated in the postimage, clobbering the piece of memory after the postimage-buf. diff --git a/builtin/apply.c b/builtin/apply.c index 31f8733..3b7ba63 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -2171,6 +2171,12 @@ static void update_pre_post_images(struct image *preimage, ctx++; } + if (postlen + ? postlen new - postimage-buf + : postimage-len new - postimage-buf) + die(BUG: caller miscounted postlen: asked %d, orig = %d, used = %d, + (int)postlen, (int) postimage-len, (int)(new - postimage-buf)); + /* Fix the length of the whole thing */ postimage-len = new - postimage-buf; postimage-nr -= reduced; -- 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] Documentation: fix version numbering
Version numbers in asciidoc-generated content (such as man pages) went missing as of da8a366 (Documentation: refactor common operations into variables). Fix by putting the underscore back in the variable name. Signed-off-by: Sven van Haastregt sve...@gmail.com --- Documentation/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 2f6b6aa..3e39e28 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -103,7 +103,7 @@ ASCIIDOC_HTML = xhtml11 ASCIIDOC_DOCBOOK = docbook ASCIIDOC_CONF = -f asciidoc.conf ASCIIDOC_COMMON = $(ASCIIDOC) $(ASCIIDOC_EXTRA) $(ASCIIDOC_CONF) \ - -agit-version=$(GIT_VERSION) + -agit_version=$(GIT_VERSION) TXT_TO_HTML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_HTML) TXT_TO_XML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_DOCBOOK) MANPAGE_XSL = manpage-normal.xsl -- 2.2.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses
On Mon, Jan 19, 2015 at 06:21:24PM +0100, Torsten Bögershausen wrote: When parsing an URL, older Git versions did handle URLs like ssh://2001:db8::1/repo.git the same way as ssh://[2001:db8::1]/repo.git Commit 83b058 broke the parsing of IPV6 adresses without []: It was written in mind that the fist ':' in a URL was the beginning of a port number, while the old code was more clever: Literal IPV6 addresses have always at least two ':'. When the hostandport had a ':' and the rest of the hostandport string could be parsed into an integer between 0 and 65536, then it was used as a port number, like host:22. Otherwise the first ':' was assumed to be part of a literal IPV6 address, like ssh://[2001:db8::1]/repo.git or ssh://[::1]/repo.git. Re-introduce the old parsing in get_host_and_port(). Improve the parsing to handle URLs which have a user name and a literal IPV6 like ssh://user@[2001:db8::1]/repo.git. (Thanks to Christian Taube li...@hcf.yourweb.de for reporting this long standing issue) Another regression was introduced in 83b058: A non-RFC conform URL like [localhost:222] could be used in older versions of Git to connect to run ssh -p 222 localhost. The new stricter parsing did not allow this any more. See even 8d3d28f5dba why [localhost:222] should be declared a feature. I'm not sure this is a very good idea. While this may have worked in the past, it's also completely inconsistent with the way all non-SSH URLs work in Git: vauxhall ok % git push https://bmc@2001:470:1f05:79::1/git/bmc/homedir.git master fatal: unable to access 'https://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/': IPv6 numerical address used in URL without brackets vauxhall no % git push https://bmc@[castro.crustytoothpaste.net]/git/bmc/homedir.git master fatal: unable to access 'https://bmc@[castro.crustytoothpaste.net]/git/bmc/homedir.git/': Could not resolve host: [castro.crustytoothpaste.net] vauxhall no % git push https://bmc@[castro.crustytoothpaste.net:443]/git/bmc/homedir.git master fatal: unable to access 'https://bmc@[castro.crustytoothpaste.net:443]/git/bmc/homedir.git/': Could not resolve host: [castro.crustytoothpaste.net I would argue that people using IPv6 literals in URLs should already know how to do it correctly, and the consistency between SSH and HTTPS URLs is a feature, not a bug. In the non-URL SSH form, you still have to use the bracketed form to deal with the case in which somebody creates a directory called 1 in their home directory and writes: git push 2001:470:1f05:79::1:1 master when they mean git push [2001:470:1f05:79::1]:1 master -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Git compile warnings (under mac/clang)
These are probably minor, I only bring them up because Git's build is generally so quiet that it might be worth squashing these too. CC fsck.o fsck.c:110:38: warning: comparison of unsigned enum expression = 0 is always true [-Wtautological-compare] if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX) ~~ ^ ~ 1 warning generated. AR libgit.a /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: libgit.a(gettext.o) has no symbols CC builtin/remote.o builtin/remote.c:1572:5: warning: add explicit braces to avoid dangling else [-Wdangling-else] else ^ builtin/remote.c:1580:5: warning: add explicit braces to avoid dangling else [-Wdangling-else] else ^ 2 warnings generated. (the warning about libgit.a(gettext.o) is probably because I'm building with NO_GETTEXT -- I've never been able to get gettext to work on my mac) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 5/5] refs.c: enable large transactions
On 22/01/15 19:16, Stefan Beller wrote: How do you run sparse on git? $ make sparse sp-out 21 I noticed there is 'make sparse' though I cannot get it working here in the corporate world as I have problems with openssl headers not being found. If you can build git with gcc, you should be able to run 'make sparse' (modulo bugs, of course!). Having said that, I build sparse from source and (on Linux) I'm running: $ sparse --version v0.5.0-30-gca3309e $ The most up-to-date (from git://git.kernel.org/pub/scm/devel/sparse/sparse.git) is actually: $ sparse --version v0.5.0-41-g6c2d743 $ which should work just fine. (I also run sparse on cygwin and MinGW). Also the line numbers seem to bit off compared to what I have here, did you need to modify/preprocess files to get sparse running? No, I am simply building the 'pu' branch (currently @ 028c360). As for the fix, would it be sufficient to check != NULL instead of 0? Hmm, I didn't give it any thought, but don't you want that to be '== NULL'? (you don't want to use a NULL lock-lk-fp in the following fprintf()). Or simply '!fdopen_lock_file(lock-lk, w)' I suppose. ATB, Ramsay Jones -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 5/5] refs.c: enable large transactions
On Thu, Jan 22, 2015 at 8:33 AM, Michael Haggerty mhag...@alum.mit.edu wrote: On 01/22/2015 02:10 PM, Jeff King wrote: On Thu, Jan 22, 2015 at 12:24:23PM +0100, Michael Haggerty wrote: I can't figure out where to apply this series or where to fetch it from, so I can't see these changes in context, so maybe I'm misunderstanding something. It looks like this code is doing open(), close(), open(), fdopen(), write(), fclose(), rename() on each lockfile. But don't we have enough information to write the SHA-1 into the lockfile the first time we touch it? I.e., couldn't we reduce this to open(), fdopen(), write(), fclose(), rename() , where the first four calls all happen in the initial loop? If a problem is discovered when writing a later reference, we would roll back the transaction anyway. I understand that this would require a bigger rewrite, so maybe it is not worth it. I had a nagging feeling on the multiple-open thing, too, and would much prefer to just write out the contents early (since we know what they are). It looks like we would just need to split write_ref_sha1() into its two halves: 1. Write out the lockfile 2. Commit the change And then call them at the appropriate spots from ref_transaction_commit(). I guess that is maybe a step backwards for abstracted ref backends, though. Nah, the implementation of ref_transaction_commit() will have to differ between backends anyway. I don't think this would be a step backwards. Michael I also dislike the double open/close thing, but I just wanted to come up with a quick and unobtrusive fix which doesn't rewrite the whole refs backend as we have some code churn in the refs lately. Michael, I forgot your short term intentions on the refs backend, so I tried to be shy with that bug fix. What huge changes are you planning in the next few weeks w.r.t. the refs handling? I would look more into that if there are no code conflicts likely to arise. Thanks, Stefan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git compile warnings (under mac/clang)
cc Johannes Schindelin johannes.schinde...@gmx.de who is working in the fsck at the moment cc Peter Wu pe...@lekensteyn.nl who worked on builtin/remote.c a few weeks ago I just compiled origin/pu to test and also found a problem (doesn't happen in origin/master): http.c: In function 'get_preferred_languages': http.c:1020:2: warning: implicit declaration of function 'setlocale' [-Wimplicit-function-declaration] retval = setlocale(LC_MESSAGES, NULL); ^ http.c:1020:21: error: 'LC_MESSAGES' undeclared (first use in this function) retval = setlocale(LC_MESSAGES, NULL); ^ http.c:1020:21: note: each undeclared identifier is reported only once for each function it appears in so I cc Yi EungJun eungjun...@navercorp.com as well. On Thu, Jan 22, 2015 at 11:43 AM, Michael Blume blume.m...@gmail.com wrote: These are probably minor, I only bring them up because Git's build is generally so quiet that it might be worth squashing these too. CC fsck.o fsck.c:110:38: warning: comparison of unsigned enum expression = 0 is always true [-Wtautological-compare] if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX) ~~ ^ ~ 1 warning generated. AR libgit.a /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: libgit.a(gettext.o) has no symbols CC builtin/remote.o builtin/remote.c:1572:5: warning: add explicit braces to avoid dangling else [-Wdangling-else] else ^ builtin/remote.c:1580:5: warning: add explicit braces to avoid dangling else [-Wdangling-else] else ^ 2 warnings generated. (the warning about libgit.a(gettext.o) is probably because I'm building with NO_GETTEXT -- I've never been able to get gettext to work on my mac) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3] rebase -i: respect core.abbrev for real
Kirill A. Shutemov kirill.shute...@linux.intel.com writes:
I have tried to fix this before: see 568950388be2, but it doesn't
really work.
I don't know how it happend, but that commit makes interactive rebase to
respect core.abbrev only during --edit-todo, but not the initial todo
list edit.
For this time I've included a test-case to avoid this frustration again.
The patch change code to use full 40-hex revision ids for todo actions
everywhere and collapse them only to show to user.
Signed-off-by: Kirill A. Shutemov kirill.shute...@linux.intel.com
---
v3:
- use full 40-hex revision ids for todo actions everywhere and collapse
them
only to show to user;
@@ -1054,6 +1052,7 @@ has_action $todo ||
return 2
cp $todo $todo.backup
+collapse_todo_ids
git_sequence_editor $todo ||
die_abort Could not execute editor
OK, the matching expand_todo_ids is just beyond the horizon of this
patch context.
I was hoping that with this change we internally operate with the
full object names throughout the program, only to show shortened
ones in the editor, but I still see a handful of rev-parse --short
outside collapse_todo_ids. Upon closer inspection, it turns out
that they are only about formatting # Rebase a..b onto c, which is
never rewritten in transform/collapse/expand_todo_ids, so I think
all is well.
Thanks.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8197ed29a9ec..a31f7e0430e1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1039,4 +1039,11 @@ test_expect_success 'short SHA-1 collide' '
)
'
+test_expect_success 'respect core.abbrev' '
+ git config core.abbrev 12
+ set_cat_todo_editor
+ test_must_fail git rebase -i HEAD~4 todo-list
+ test 4 = $(grep -c pick [0-9a-f]\{12,\} todo-list)
+'
+
test_done
--
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 16/18] fsck: support demoting errors to warnings
Hi Michael, On 2015-01-22 16:49, Michael Haggerty wrote: On 12/23/2014 06:14 PM, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: On Tue, 23 Dec 2014, Junio C Hamano wrote: I suspect that it would be much better if the configuration variables were organized the other way around, e.g. $ git config fsck.warn missingTagger,someOtherKindOfError I had something similar in an earlier version of my patch series, but it was shot down rightfully: if you want to allow inheriting defaults from $HOME/.gitconfig, you have to configure the severity levels individually. Hmmm. What's wrong with fsck.warn -missingTagger that overrides the earlier one, or even fsck.info missingTagger after having fsck.warn other,missingTagger,yetanother, with the usual last one wins rule? Whoever shot it down rightfully is wrong here, I would think. Sorry I didn't notice this earlier; Johannes, please CC me on these series, especially the ones that I commented on earlier. Very sorry, this is my fault. It can only be explained by my switching around some tools for other tools to work with email-based patch submission (which I had not done in a long time). But still, my mistake. [1] I prefer to think that I just offered a little gentle discussion that informed Johannes's independent decision :-) You did convince me back then. I just did not want to put up a fight against Junio because I was more interested in getting this feature merged before the holidays (it does feel awkward for me to leave work unwrapped-up before leaving for an extended amount of time, but I guess I am getting more used to that). So now I cannot avoid discussing this issue properly... In essence, I agreed with Junio from the point of view of an elegant implementation. But then, Michael is correct that it does not really matter as much how complicated the code is, but that it is much more important that the feature is elegant to use. Now let's step back a bit and think about the users which is supposed to be supported by this patch series: Git repository hosters -- such as GitHub -- need to ensure a certain cleanliness of the repositories they host (for a range of reasons, including the prevention of malicious attacks, or helping users publish their code in a correct form). And the scenario in which the feature needs to be used is most likely started by some Git user pushing some commits, and `git receive-pack` triggering an error. Then the user files a trouble ticket and GitHubber needs to inspect the error and the respective object. Now, in the vast number of cases I imagine that the objects *are* faulty. However, on occasion the problem should not prevent the push, e.g. when somebody crafted a commit object with two authors, forgetting that the tools usually cannot handle such commits. Then the GitHubber has to decide on a case by case basis whether to demote that error to a warning and allow the object to be pushed *into that specific repository*. I do see the need for this feature to be simple and robust, from the users' point of view. In other words, I agree with Michael that we need to avoid confusing settings such as ``` [receive.fsck] warn = missing-tagger-entry error = missing-tagger-entry ``` This feature will be used rarely enough that the poor soul stuck with interpreting the above config section won't remember that a very specific version of last setting wins is in effect. If I remember correctly, Peff suggested that there needs to be a way to handle these settings in the /etc/gitconfig $HOME/.gitconfig $XDG../gitconfig .git/config cascade, but now I am puzzled whether it is even desirable to demote fsck errors globally, i.e. whether we really need to pay attention to that config cascade. And finally, in the course of preparing this patch series, we came up with an alternative solution to the problem: the receive.fsck.skiplist (i.e. a file that contains a sorted list of SHA-1s of objects that should be skipped from fsck'ing). I am more and more convinced that this is the most convenient tool for the scenario described above: manual inspection of individual objects will tell whether it is safe to allow them onto the server or not. However, others might disagree and prefer the explicit approach, e.g. when some source generates a consistent stream of objects triggering fsck errors. Summary: I have no preference how to specify the severity levels of fsck messages, but I will gladly change my code to whatever you (meaning Junio and Michael in particular) want to see implemented. Thanks for helping me with this feature, Dscho -- 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] Makefile: do not compile git with debugging symbols by default
Hello Jeff, Yes, main point is size of executable. I'm sorry, didn't see 'strip' target. What if we will strip git and other executable files by default and add -g option and don't strip it if DEBUG=1 passed to make. Something like this: git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) git.o \ $(BUILTIN_OBJS) $(LIBS) ifneq ($(DEBUG),1) $(MAKE) strip endif Thank you. 2015-01-22 19:00 GMT+06:00 Jeff King p...@peff.net: On Thu, Jan 22, 2015 at 06:50:37PM +0600, Alexander Kuleshov wrote: Standard user has no need in debugging information. This patch adds DEBUG=1 option to compile git with debugging symbols and compile without it by default. This explanation is missing why it is beneficial _not_ to have the debugging information. I expect the answer is it makes the executable smaller. And that is true, but it gets smaller still if you run strip on the result: $ make CFLAGS= /dev/null 21 wc -c git 2424248 $ make CFLAGS=-g /dev/null 21 wc -c git 4500816 $ strip git wc -c git 2109200 So I am not sure who this is helping. If you are size-conscious, you should use strip, in which case the -g flag does not matter (and we even have make strip to help you). Is there some other reason to avoid the debugging information? -Peff -- _ 0xAX -- 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] Makefile: do not compile git with debugging symbols by default
Or even still -g as it now, because anyway debugging info will be removed with stripping 2015-01-22 22:51 GMT+06:00 Alexander Kuleshov kuleshovm...@gmail.com: Hello Jeff, Yes, main point is size of executable. I'm sorry, didn't see 'strip' target. What if we will strip git and other executable files by default and add -g option and don't strip it if DEBUG=1 passed to make. Something like this: git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) git.o \ $(BUILTIN_OBJS) $(LIBS) ifneq ($(DEBUG),1) $(MAKE) strip endif Thank you. 2015-01-22 19:00 GMT+06:00 Jeff King p...@peff.net: On Thu, Jan 22, 2015 at 06:50:37PM +0600, Alexander Kuleshov wrote: Standard user has no need in debugging information. This patch adds DEBUG=1 option to compile git with debugging symbols and compile without it by default. This explanation is missing why it is beneficial _not_ to have the debugging information. I expect the answer is it makes the executable smaller. And that is true, but it gets smaller still if you run strip on the result: $ make CFLAGS= /dev/null 21 wc -c git 2424248 $ make CFLAGS=-g /dev/null 21 wc -c git 4500816 $ strip git wc -c git 2109200 So I am not sure who this is helping. If you are size-conscious, you should use strip, in which case the -g flag does not matter (and we even have make strip to help you). Is there some other reason to avoid the debugging information? -Peff -- _ 0xAX -- _ 0xAX -- 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: [PATCHv1 0/6] Fix bug in large transactions
On Thu, Jan 22, 2015 at 9:58 AM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
v2 applies to sb/atomic-push instead of next and will result in a one
line merge conflict with next.
I acctually tried to apply on 'next' and also on 'sb/atomic-push'
and both failed.
That's strange indeed. I just checked out origin/sb/atomic-push (which
points at 04b39f195baf, 2015-01-12 16:24:02, Document receive.advertiseatomic)
and applied the patches v2 from my outgoing mailbox without problems.
I do not remember to have changed my development process a lot there.
(I was not using notes, but that should not matter at all)
git format-patch --subject-prefix=PATCHv2 --cover-letter
origin/sb/atomic-push..HEAD
$EDITOR -cover-letter.patch
git send-email 00* --to=...
I had to wiggle the patches to make them apply on
the latter, and that is what is queued on 'pu' now, but I would not
be surprised if I made silly mistakes while doing so, so please
double check the result and catch them if I did.
I just fetched origin/sb/atomic-push-fix and the only difference I can spot
compared to the local branch is in d4ad3f1cdcb (refs.c: remove lock_fd
from struct ref_lock) in refs.c in the hunk:
@@ -2335,8 +2333,8 @@ static struct ref_lock
*lock_ref_sha1_basic(const char *refname,
goto error_return;
}
- lock-lock_fd = hold_lock_file_for_update(lock-lk, ref_file, lflags);
- if (lock-lock_fd 0) {
+ if (hold_lock_file_for_update(lock-lk, ref_file, lflags) 0) {
+ last_errno = errno;
if (errno == ENOENT --attempts_remaining 0)
/*
* Maybe somebody just deleted one of the
(whitespaces broken here)
That hunk has the additional line + last_errno = errno;, which I do
not have locally, but that's the exact problem I spotted when trying to
merge my local branch to origin/next which then results in a merge
conflict with the patch from origin/jk/lock-ref-sha1-basic-return-errors
as that introduces the line + last_errno = errno;.
Sorry for the confusion,
Stefan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 5/5] refs.c: enable large transactions
On 22/01/15 19:51, Ramsay Jones wrote:
On 22/01/15 19:16, Stefan Beller wrote:
How do you run sparse on git?
$ make sparse sp-out 21
BTW, you can get gcc to warn about this also:
$ rm refs.o
$ make CFLAGS='-Wall -Wextra' refs.o
* new build flags
CC refs.o
In file included from cache.h:4:0,
from refs.c:1:
git-compat-util.h: In function ‘git_has_dos_drive_prefix’:
git-compat-util.h:277:56: warning: unused parameter ‘path’ [-Wunused-parameter]
static inline int git_has_dos_drive_prefix(const char *path)
^
In file included from cache.h:4:0,
from refs.c:1:
git-compat-util.h: In function ‘xsize_t’:
git-compat-util.h:689:10: warning: comparison between signed and unsigned
integer expressions [-Wsign-compare]
if (len (size_t) len)
^
refs.c: In function ‘check_refname_component’:
refs.c:54:61: warning: unused parameter ‘flags’ [-Wunused-parameter]
static int check_refname_component(const char *refname, int flags)
^
refs.c: In function ‘warn_if_dangling_symref’:
refs.c:1814:78: warning: unused parameter ‘sha1’ [-Wunused-parameter]
static int warn_if_dangling_symref(const char *refname, const unsigned char
*sha1,
^
refs.c: In function ‘log_ref_write_fd’:
refs.c:3042:14: warning: comparison between signed and unsigned integer
expressions [-Wsign-compare]
if (written != len)
^
refs.c: In function ‘write_sha1_to_lock_file’:
refs.c:3105:42: warning: ordered comparison of pointer with integer zero
[-Wextra]
|| fdopen_lock_file(lock-lk, w) 0
^
refs.c:3110:39: warning: ordered comparison of pointer with integer zero
[-Wextra]
if (fdopen_lock_file(lock-lk, w) 0
^
refs.c: In function ‘create_symref’:
refs.c:3220:18: warning: comparison between signed and unsigned integer
expressions [-Wsign-compare]
if (sizeof(ref) = len) {
^
refs.c: In function ‘read_ref_at_ent’:
refs.c:3277:15: warning: unused parameter ‘email’ [-Wunused-parameter]
const char *email, unsigned long timestamp, int tz,
^
refs.c: In function ‘read_ref_at_ent_oldest’:
refs.c:3324:19: warning: unused parameter ‘email’ [-Wunused-parameter]
const char *email, unsigned long timestamp,
^
refs.c: In function ‘for_each_reflog_ent_reverse’:
refs.c:3451:22: warning: comparison between signed and unsigned integer
expressions [-Wsign-compare]
cnt = (sizeof(buf) pos) ? sizeof(buf) : pos;
^
refs.c:3451:43: warning: signed and unsigned type in conditional expression
[-Wsign-compare]
cnt = (sizeof(buf) pos) ? sizeof(buf) : pos;
^
refs.c: In function ‘ref_transaction_free’:
refs.c:3659:16: warning: comparison between signed and unsigned integer
expressions [-Wsign-compare]
for (i = 0; i transaction-nr; i++) {
^
$
Notice the [-Wextra] warnings above. ;-)
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 5/5] refs.c: enable large transactions
On Thu, Jan 22, 2015 at 12:13 PM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: Notice the [-Wextra] warnings above. ;-) ATB, Ramsay Jones Thanks, I put that into my config.mak Though recompiling the whole project yields 4 [-Wempty-body] 477 [-Wmissing-field-initializers] 966 [-Wsign-compare] 899 [-Wunused-parameter] so maybe I'll disable it again when I think it's too much output. -- 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 24/24] git-status.txt: advertisement for untracked cache
On Tue, Jan 20, 2015 at 08:03:33PM +0700, Nguyễn Thái Ngọc Duy wrote: When a good user sees the too long, consider -uno advice when running `git status`, they should check out the man page to find out more. This change suggests they try untracked cache before -uno. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git-status.txt | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt index def635f..7850f53 100644 --- a/Documentation/git-status.txt +++ b/Documentation/git-status.txt @@ -58,7 +58,10 @@ When `-u` option is not used, untracked files and directories are shown (i.e. the same as specifying `normal`), to help you avoid forgetting to add newly created files. Because it takes extra work to find untracked files in the filesystem, this mode may take some -time in a large working tree. You can use `no` to have `git status` +time in a large working tree. +Consider to enable untracked cache and split index if supported (see This might sound more natural (at least to my ears) if you wrote Consider enabling untracked cache…. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
RFC grep '...\{m,n\}?
Kirill A. Shutemov kirill.shute...@linux.intel.com writes:
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8197ed29a9ec..a31f7e0430e1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1039,4 +1039,11 @@ test_expect_success 'short SHA-1 collide' '
)
'
+test_expect_success 'respect core.abbrev' '
+ git config core.abbrev 12
+ set_cat_todo_editor
+ test_must_fail git rebase -i HEAD~4 todo-list
+ test 4 = $(grep -c pick [0-9a-f]\{12,\} todo-list)
+'
Documentation/CodingGuidelines says
- As to use of grep, stick to a subset of BRE (namely, no \{m,n\},
[::], [==], or [..]) for portability.
- We do not use \{m,n\};
- We do not use -E;
- We do not use ? or + (which are \{0,1\} and \{1,\}
respectively in BRE) but that goes without saying as these
are ERE elements not BRE (note that \? and \+ are not even part
of BRE -- making them accessible from BRE is a GNU extension).
but I see we have multiple hits from git grep 'grep .*\\{' (all in
the t/ directory). I wonder
- if everybody's system is now OK with \{m,n\} these days, or
- there are people who are grateful that we stayed away from using
\{m,n\} but they are not running the tests because their system
is too exotic to pass other parts of the test suite.
If the former, we may want to drop the \{m,n\} from the forbidden
list.
--
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] Documentation: fix version numbering
On Thu, Jan 22, 2015 at 07:32:33PM +, Sven van Haastregt wrote: Version numbers in asciidoc-generated content (such as man pages) went missing as of da8a366 (Documentation: refactor common operations into variables). Fix by putting the underscore back in the variable name. Signed-off-by: Sven van Haastregt sve...@gmail.com --- Documentation/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 2f6b6aa..3e39e28 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -103,7 +103,7 @@ ASCIIDOC_HTML = xhtml11 ASCIIDOC_DOCBOOK = docbook ASCIIDOC_CONF = -f asciidoc.conf ASCIIDOC_COMMON = $(ASCIIDOC) $(ASCIIDOC_EXTRA) $(ASCIIDOC_CONF) \ - -agit-version=$(GIT_VERSION) + -agit_version=$(GIT_VERSION) Yes, this does seem to fix it. My apologies for the bug, and thanks for noticing. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH] Makefile: do not compile git with debugging symbols by default
Alexander Kuleshov kuleshovm...@gmail.com writes: Hello Jeff, Yes, main point is size of executable. The Git executable is a few megabytes, i.e. 0.001% the size of a really small hard disk. The benefit seems really negligible to me. OTOH, debug information allow users to do better bug reports in case of crash (gdb, valgrind), which outweights by far the benefit of saving a handfull of megabytes IMHO. On a side note, I find it very frustrating when a program I use crashes, opens a bug report wizard, and end up telling me sorry, your distro removed the debug symbols, recompile everything if you want to report a bug. I understand that for a few users, the size of executable matters. But this category of users should be able to find the strip target or something equivalent. -- 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] transport-helper: do not request symbolic refs to remote helpers
Mike Hommey m...@glandium.org writes: The patch changes the behavior in all cases, because it didn't feel necessary to have a different behavior between the normal case and the '?' case: it makes sense to request the ref being pointed at than the symbolic ref in every case. Moreover, this makes existing non-git remote-helpers work without having to modify them to provide a refspec for HEAD (none of the 5 mercurial remote-helpers I checked do). I do not question the latter. It is not surprising if all of them share the same limitation that shares the same root in the same impedance mismatch. The trouble I had in supporting makes sense ... in every case was that you said that the code as patched would not work for a symref pointing at another symref. The original code did not have that problem with remote helpers that support the 'list' command. Does the new code avoid regressions for them and if so how? That is what was needed in the justification. For remote helpers that support the 'list' command, asking for a symref and asking for a ref that the symref points at both work OK and behave the same, and hopefully that would be true even when the latter is a symref that points yet another ref, so dereferencing only one level on our end when making a request, instead of letting the remote side dereference, is not likely to cause regression. -- 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: [PATCHv1 0/6] Fix bug in large transactions
Stefan Beller sbel...@google.com writes: v2 applies to sb/atomic-push instead of next and will result in a one line merge conflict with next. I acctually tried to apply on 'next' and also on 'sb/atomic-push' and both failed. I had to wiggle the patches to make them apply on the latter, and that is what is queued on 'pu' now, but I would not be surprised if I made silly mistakes while doing so, so please double check the result and catch them if I did. 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
[ANNOUNCE] Guilt v0.36-rc1
Guilt v0.36-rc1 is available for download. Guilt (Git Quilt) is a series of shell scripts which add a Mercurial queues-like functionality and interface to git. Tarballs: http://guilt.31bits.net/src/ Git repo: git://repo.or.cz/guilt.git This is the first release candidate for the long awaited v0.36. Yes, it's been *way* too long since the last release, but hopefully that will change going forward. By far, the most changes come from Per Cederqvist who cleaned up some rather dark corners in the code. Thanks! As always, patches, and other feedback is welcome. Josef Jeff Sipek. Changes since v0.35: Alan Jenkins (5): [GUILT 1/6] Refuse to push corrupt patches [GUILT 2/6] guilt-header: fix patch corruption [GUILT 3/6] Handle paths that contain spaces [GUILT 4/6] Run regression tests in a directory which contains spaces [GUILT 5/6] Allow guilt scripts to be run from a directory which contains spaces Dave Chinner (1): guilt: fix from: xxx filtering in the patch description Jari Aalto (1): Documentation/Makefile: Run xmlto with --skip-validation Jonathan Nieder (1): Drop unneeded git version check. Josef 'Jeff' Sipek (12): [GUILT 6/6] Allow the regression tests to be run from a directory with spaces in guilt: remove a useless cat commit: don't lose commits fix direct invocation messages guilt: fix -h invocation Documentation: update HOWTO to use 'guilt foo' Documentation: clean up the makefile Documentation: guilt git don't use hyphenated commands regress: setup_git_repo can assert that the repo is setup as intended FreeBSD is much like Darwin regress: cmd and shouldfail shouldn't print escapes Guilt v0.36-rc1 Per Cederqvist (43): get rid of cat: write error: Broken pipe error message [GUILT] handle branches with slashes in guilt-graph Testsuite: get rid of Broken pipe errors from yes. The tests should not fail if log.date or log.decorate are set. get rid of cat: write error: Broken pipe error message The tests should not fail if log.date or log.decorate are set. Testsuite: get rid of Broken pipe errors from yes. Handle empty patches and patches with only a header. Fix fatal guilt graph error in sha1sum invocation. Change git branch when patches are applied. The tests should not fail if guilt.diffstat is set. Allow guilt delete -f to run from a dir which contains spaces. Added test case for guilt delete -f. Allow guilt import-commit to run from a dir which contains spaces. guilt new: Accept more than 4 arguments. Fix the do_get_patch function. Added test cases for guilt fold. Added more test cases for guilt new: empty patches. Test suite: properly check the exit status of commands. Run test_failed if the exit status of a test script is bad. test suite: remove pointless redirection. guilt header: more robust header selection. Check that guilt header '.*' fails. Use git check-ref-format to validate patch names. Produce legal patch names in guilt-import-commit. Fix backslash handling when creating names of imported patches. guilt graph no longer loops when no patches are applied. guilt-graph: Handle commas in branch names. Check that guilt graph works when working on a branch with a comma. guilt graph: Handle patch names containing quotes. The log.decorate setting should not influence import-commit. The log.decorate setting should not influence patchbomb. The log.decorate setting should not influence guilt rebase. disp no longer processes backslashes. guilt push now fails when there are no more patches to push. guilt pop now fails when there are no more patches to pop. Minor testsuite fix. Fix coding style errors in t-061.sh. Added guilt.reusebranch configuration option. Added a short style guide, and Emacs settings. Don't use git log -p in the test suite. Improved doc and tests for guilt header. Document the exit status of guilt push and guilt pop. Theodore Ts'o (2): guilt: skip empty line after from: line in patch descriptoin guilt: fix date parsing -- The obvious mathematical breakthrough would be development of an easy way to factor large prime numbers. - Bill Gates, The Road Ahead, pg. 265 -- 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: [PATCHv1 0/6] Fix bug in large transactions
On Thu, Jan 22, 2015 at 12:00 AM, Junio C Hamano gits...@pobox.com wrote:
Jeff King p...@peff.net writes:
On Wed, Jan 21, 2015 at 03:23:39PM -0800, Stefan Beller wrote:
(reported as: git update-ref --stdin : too many open files, 2014-12-20)
First a test case is introduced to demonstrate the failure,
the patches 2-6 are little refactoring and the last patch
fixes the bug and also marks the bugs as resolved in the
test suite.
Unfortunately this applies on top of origin/next.
Saying applies on next is not very useful to Junio. He is not going to
branch a topic straight from next, as merging it to master would pull
in all of the topics cooking in next (not to mention a bunch of merge
commits which are generally never part of master).
Instead, figure out which topic in next you actually _need_ to build on,
and then it can be branched from there. And if there is no such topic,
then you should not be building on next, of course. :) But I think you
know that part already.
All very true.
I consider anything new that appears late in the cycle, especially
during deep in the pre-release freeze, less for me to apply but more
for others to eyeball the preview of a series the submitter plans to
work on once the next cycle starts, so basing on 'next' does not
hurt too much. For interested others,
git checkout origin/next^0
would be shorter to type than
git checkout origin/next^{/^Merge branch 'sb/atomic-push'}^2
so... ;-)
But what is more troublesome is that neither this or updated v2
applies to 'next'.
v2 applies to sb/atomic-push instead of next and will result in a one
line merge conflict with next.
I know we're late in the cycle, hence I was just sending out for reviews.
Though as it's not a feature, but a bug fix it may be worth picking it
up now if it's easy to apply.
Thanks,
Stefan
Let me try to wiggle it in first.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 5/5] refs.c: enable large transactions
On 22/01/15 20:20, Stefan Beller wrote: On Thu, Jan 22, 2015 at 12:13 PM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: Notice the [-Wextra] warnings above. ;-) ATB, Ramsay Jones Thanks, I put that into my config.mak Though recompiling the whole project yields 4 [-Wempty-body] 477 [-Wmissing-field-initializers] 966 [-Wsign-compare] 899 [-Wunused-parameter] so maybe I'll disable it again when I think it's too much output. Yes, you don't want to use -Wextra for everyday usage! :-D [BTW, I don't know if sparse works on OS X, so ...] ATB, Ramsay Jones -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git compile warnings (under mac/clang)
On Thursday 22 January 2015 11:59:54 Stefan Beller wrote: cc Johannes Schindelin johannes.schinde...@gmx.de who is working in the fsck at the moment cc Peter Wu pe...@lekensteyn.nl who worked on builtin/remote.c a few weeks ago I just compiled origin/pu to test and also found a problem (doesn't happen in origin/master): http.c: In function 'get_preferred_languages': http.c:1020:2: warning: implicit declaration of function 'setlocale' [-Wimplicit-function-declaration] retval = setlocale(LC_MESSAGES, NULL); ^ http.c:1020:21: error: 'LC_MESSAGES' undeclared (first use in this function) retval = setlocale(LC_MESSAGES, NULL); ^ http.c:1020:21: note: each undeclared identifier is reported only once for each function it appears in so I cc Yi EungJun eungjun...@navercorp.com as well. On Thu, Jan 22, 2015 at 11:43 AM, Michael Blume blume.m...@gmail.com wrote: These are probably minor, I only bring them up because Git's build is generally so quiet that it might be worth squashing these too. CC fsck.o fsck.c:110:38: warning: comparison of unsigned enum expression = 0 is always true [-Wtautological-compare] if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX) ~~ ^ ~ 1 warning generated. AR libgit.a /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: libgit.a(gettext.o) has no symbols CC builtin/remote.o builtin/remote.c:1572:5: warning: add explicit braces to avoid dangling else [-Wdangling-else] else ^ builtin/remote.c:1580:5: warning: add explicit braces to avoid dangling else [-Wdangling-else] else ^ 2 warnings generated. Hi, these warnings were present in v3 of the git-remote patch. v4 was proposed to overcome these issues, but I have yet to respond to Junio's feedback at http://www.spinics.net/lists/git/msg243652.html (Message-ID: xmqqr3vx9ad5@gitster.dls.corp.google.com) (cc'ing Junio to let him know I am still alive :p) I'll get back to this next week, had some other tasks to prepare for. Kind regards, Peter (the warning about libgit.a(gettext.o) is probably because I'm building with NO_GETTEXT -- I've never been able to get gettext to work on my mac) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git compile warnings (under mac/clang)
Hi Stefan, On 2015-01-22 20:59, Stefan Beller wrote: cc Johannes Schindelin johannes.schinde...@gmx.de who is working in the fsck at the moment On Thu, Jan 22, 2015 at 11:43 AM, Michael Blume blume.m...@gmail.com wrote: CC fsck.o fsck.c:110:38: warning: comparison of unsigned enum expression = 0 is always true [-Wtautological-compare] if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX) ~~ ^ ~ According to A2.5.4 of The C Programming Language 2nd edition: Identifiers declared as enumerators (see Par.A.8.4) are constants of type int. Therefore, the warning is incorrect: any assumption about enum fsck_msg_id to be unsigned is false. Ciao, Johannes -- 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
