Re: What is missing from Git v2.0
Felipe Contreras wrote: This is a false dichotomy; there aren't just two kinds of Git users. There is such a category of Git users who are not fresh-out-of-the-boat, yet not power users either. Oh, I didn't mean to suggest a dichotomy of any kind. However these are the two groups (I suggest) are the most immediately relevant - one calls for change, and the other would be negatively impacted. Unless the aliases are already there by default. Others, with knowledge far beyond mine, have pointed out the problems with this. I'd suggest the argument most relevant to my own statements is how it impacts the learning proccess, and makes it more likely that users will learn aliases _as_ commands, which of course is incorrect and potentially harmful. And if default aliases were such a bad idea, why do most (all?) version control systems out there have them? I'm so tempted just to sass and say that it's because they aren't git... But on a more serious note, a feature (any feature) being in one vcs doesn't mean, by default, that it's right for git. The status quo may be a mistake on the part of it's followers. (And, historically, has been many times - for an transculturally-acceptable example, consider the rejection of Galileo's astronomical research by the Vatican of the time.) Just because Mercurial et. all does something doesn't mean git needs to, or even should. It needs objective consideration, not to just be ushered through on the basis of tradition. -- James Denholm -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-rebase: fix probable reflog typo
Felipe Contreras felipe.contre...@gmail.com writes: Matthieu Moy wrote: Felipe Contreras felipe.contre...@gmail.com writes: Commit 26cd160 (rebase -i: use a better reflog message) tried to produce a better reflog message, however, it seems a statement was introduced by mistake. 'comment_for_reflog start' basically overides the GIT_REFLOG_ACTION we just set. So, one needs to reset $GIT_REFLOG_ACTION to what it used to be if is it to be used later. However, it seems to me that the comment_for_reflog start is used only for this checkout command. If so, there's no need for the comment_for_reflog start before the if statement either. Exactly. But if this variable is only meant for this command, it should be `VAR=VAL command`, that would make it clear without the need of a comment. I don't understand. Are you suggesting to replace the shell function output with an external command? If not, which command would you want to call here? -- 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 v4 4/6] patch-id: make it stable against hunk reordering
On Wed, Apr 23, 2014 at 03:05:42PM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: On Wed, Apr 23, 2014 at 10:39:23AM -0700, Junio C Hamano wrote: Are these three patches the same as what has been queued on mt/patch-id-stable topic and cooking in 'next' for a few weeks? Not exactly - at your request I implemented git config options to control patch id behaviour. Documentation and tests updated to match. After comparing the patches 4-6 and the one that has been in 'next' for a few weeks, I tried to like the new one, but I couldn't. I'm fine with the one in next too. I was under the impression that you wanted me to change the behaviour so I worked on this, but previous version was sufficient for my purposes (which is really all about putting diff.orderfile upstream). The new one, if I am reading the patch correctly, makes the stable variant the default if - the configuration explicitly asks to use it; or - diff.orderfile, which is a new configuration that did not exist, is used. At the first glance, the latter makes it look as if that will not hurt any existing users/repositories, but the thing is, the producer of the patches is different from the consumer of the patches. There needs to be a way for a consumer to say I need stable variant on the patches when computing patch-id on a patch that arrived. As long as two different producers use two different orders, the consumer of the patches from these two sources is forced to use the stable variant if some sort of cache is kept keyed with the patch-ids. But diff.orderfile configuration is all and only about the producer, and does not help the case at all. OK, we can just drop that, that's easy. Compared to it, the previous version forced people who do not have a particular reason to choose between the variants to use the new stable variant, which was a lot simpler solution. The reason why I merged the previous one to 'next' was because I wanted to see if the behaviour change is as grave as I thought, or I was just being unnecessary cautious. If there is no third-party script that precomputes something and stores them under these hashes that breaks with this change, I do not see any reason not to make the new stable one the default. Ah okay, so we just wait a bit and see and if all is quiet the existing patch will graduate to master with time? Totally cool. I thought you wanted me to add the config option, but if everyone is happy as is, I don't need it personally at all. I however suspect that the ideal stable implementation may be different from what you implemented. What if we compute a patch-id on a reordered patch as if the files came in the usual order? ATM patch id does not have any concept of the usual order, so that's one problem - how does one figure out what the order would be? I have no idea - is this documented anywhere? Also I'm guessing this would depend on the state of the git tree which would be another behaviour change: previously patch-id worked fine outside any git tree. That would be another way to achieve the stable-ness for sane cases (i.e. no funny you could split one patch with two hunks into two patches with one hunk each twice mentioning the same path which is totally an uninteresting use case---diff.orderfile would not even produce such a patch) Yes I'm thinking we should drop this hunk in the patch: let's support reordering files but not splitting them. This makes the change even smaller, so I now think we should just go for it. and a huge difference would be that it would produce the same patch-id as existing versions of Git, if the patch is not reordered. Is that asking for a moon (I admit that I haven't looked at the code involved too deeply)? Yes this would be a bunch of code to write - certainly much more complex than the existing small patch which just tweaks the checksum slightly to make it stable. -- MST -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
James Denholm wrote: Felipe Contreras wrote: This is a false dichotomy; there aren't just two kinds of Git users. There is such a category of Git users who are not fresh-out-of-the-boat, yet not power users either. Oh, I didn't mean to suggest a dichotomy of any kind. However these are the two groups (I suggest) are the most immediately relevant - one calls for change, and the other would be negatively impacted. Nobody would be negatively impacted. Who would be impacted negatively by having default aliases? Unless the aliases are already there by default. Others, with knowledge far beyond mine, have pointed out the problems with this. And I have showed they are not problems. I'd suggest the argument most relevant to my own statements is how it impacts the learning proccess, and makes it more likely that users will learn aliases _as_ commands, which of course is incorrect and potentially harmful. That is an assumption. Why would a user think 'co' is a command? And if default aliases were such a bad idea, why do most (all?) version control systems out there have them? I'm so tempted just to sass and say that it's because they aren't git... But on a more serious note, a feature (any feature) being in one vcs doesn't mean, by default, that it's right for git. How is Git different from any other version control systems? Commands are commands. The status quo may be a mistake on the part of it's followers. Yes, it might, but it's not. (And, historically, has been many times - for an transculturally-acceptable example, consider the rejection of Galileo's astronomical research by the Vatican of the time.) Yes, I'm perfecly aware that everybody _can_ be wrong, that doesn't mean they _are_. Just because Mercurial et. all does something doesn't mean git needs to, or even should. It needs objective consideration, not to just be ushered through on the basis of tradition. Again, this is a red herring. Nobody argued that Git should do this because others are doing it. You failed to answer the question, so I'm asking it again: If default aliases were such a bad idea, why do most (all?) version control systems out there have them? Your answer seems to be along the lines of: they made a mistake and they are all wrong. Is it? But, surely if it's a mistake on their part you should be able to find people affected by this horrible error. This would validate the arguments that others have put forward; if we do X we will have problem Y. Well, other projects have done X, do they have problem Y? -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-rebase: fix probable reflog typo
Matthieu Moy wrote: Felipe Contreras felipe.contre...@gmail.com writes: Matthieu Moy wrote: Felipe Contreras felipe.contre...@gmail.com writes: Commit 26cd160 (rebase -i: use a better reflog message) tried to produce a better reflog message, however, it seems a statement was introduced by mistake. 'comment_for_reflog start' basically overides the GIT_REFLOG_ACTION we just set. So, one needs to reset $GIT_REFLOG_ACTION to what it used to be if is it to be used later. However, it seems to me that the comment_for_reflog start is used only for this checkout command. If so, there's no need for the comment_for_reflog start before the if statement either. Exactly. But if this variable is only meant for this command, it should be `VAR=VAL command`, that would make it clear without the need of a comment. I don't understand. Are you suggesting to replace the shell function output with an external command? If not, which command would you want to call here? Recently some code was changed to do 'test_must_fail env VAR=VAL command', why can't we do the same? -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
Felipe Contreras felipe.contre...@gmail.com writes: James Denholm wrote: Felipe Contreras wrote: This is a false dichotomy; there aren't just two kinds of Git users. There is such a category of Git users who are not fresh-out-of-the-boat, yet not power users either. Oh, I didn't mean to suggest a dichotomy of any kind. However these are the two groups (I suggest) are the most immediately relevant - one calls for change, and the other would be negatively impacted. Nobody would be negatively impacted. Who would be impacted negatively by having default aliases? The people having to read and understand scripts written in the expectation of default aliases. And I have showed they are not problems. You managed to convince yourself, so feel free to put aliases in every Git you use and distribute. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 05/12] cherry-pick: remember rerere-autoupdate
On Wed, Apr 23, 2014 at 10:44 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- diff --git a/t/t3504-cherry-pick-rerere.sh b/t/t3504-cherry-pick-rerere.sh index e6a6481..274b2cc 100755 --- a/t/t3504-cherry-pick-rerere.sh +++ b/t/t3504-cherry-pick-rerere.sh @@ -42,4 +42,43 @@ test_expect_success 'cherry-pick conflict without rerere' ' test_must_fail test_cmp expect foo ' +test_expect_success 'cherry-pick --rerere-autoupdate' ' + test_when_finished rm -rf rerere + ( + git init rerere + cd rerere + test_config rerere.enabled true + echo one content-a git add content-a + echo one content-b git add content-b + git commit -m one + git checkout -b conflict master + echo conflict-a content-a + git commit -a -m conflict-a + echo conflict-b content-b + git commit -a -m conflict-b + git checkout master + echo two content-a + echo two content-b + git commit -a -m two + git checkout -b test + test_must_fail git cherry-pick master..conflict + echo resolved-a content-a + git add content-a + test_must_fail git cherry-pick --continue + echo resolved-b content-b + git add content-b + git cherry-pick --continue + git reset --hard master Broken -chain. + git log --oneline --all --decorate --graph + test_must_fail git cherry-pick --rerere-autoupdate master..conflict + git log --oneline --all --decorate --graph + echo resolved-a expected + test_cmp expected content-a Ditto. + test_must_fail git cherry-pick --continue + echo resolved-b expected + test_cmp expected content-b + git cherry-pick --continue + ) +' + test_done -- 1.9.2+fc1.2.gfbaae8c -- 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 clone gives zero file permissions
Hello, I installed git to my Windows 7 workstation and cloned http://git.ipxe.org/ipxe.git; by using the Git GUI. ipxe-23042014/src tree looks like this in Cygwin bash: d-+ 1 peltoju Domain Users 0 Apr 23 09:00 arch d-+ 1 peltoju Domain Users 0 Apr 23 09:00 bin d-+ 1 peltoju Domain Users 0 Apr 23 09:00 config d-+ 1 peltoju Domain Users 0 Apr 23 09:00 core d-+ 1 peltoju Domain Users 0 Apr 23 09:00 crypto d-+ 1 peltoju Domain Users 0 Apr 23 09:00 doc --+ 1 peltoju Domain Users 62631 Apr 23 09:00 doxygen.cfg d-+ 1 peltoju Domain Users 0 Apr 23 09:00 drivers d-+ 1 peltoju Domain Users 0 Apr 23 09:00 hci d-+ 1 peltoju Domain Users 0 Apr 23 09:00 image d-+ 1 peltoju Domain Users 0 Apr 23 09:00 include d-+ 1 peltoju Domain Users 0 Apr 23 09:00 interface d-+ 1 peltoju Domain Users 0 Apr 23 09:00 libgcc --+ 1 peltoju Domain Users 6146 Apr 23 09:00 Makefile --+ 1 peltoju Domain Users 38982 Apr 23 09:00 Makefile.housekeeping d-+ 1 peltoju Domain Users 0 Apr 23 09:00 net d-+ 1 peltoju Domain Users 0 Apr 23 09:00 tests d-+ 1 peltoju Domain Users 0 Apr 23 09:00 usr d-+ 1 peltoju Domain Users 0 Apr 23 09:00 util Files have no permissions, same goes with subfolders, e.g. $ ls -l config/ total 47 --+ 1 peltoju Domain Users 699 Apr 23 09:00 colour.h --+ 1 peltoju Domain Users 6961 Apr 23 09:00 config.c --+ 1 peltoju Domain Users 518 Apr 23 09:00 config_ethernet.c --+ 1 peltoju Domain Users 584 Apr 23 09:00 config_fc.c --+ 1 peltoju Domain Users 474 Apr 23 09:00 config_infiniband.c --+ 1 peltoju Domain Users 875 Apr 23 09:00 config_net80211.c --+ 1 peltoju Domain Users 478 Apr 23 09:00 config_romprefix.c --+ 1 peltoju Domain Users 555 Apr 23 09:00 config_route.c Any explanation available? git-gui version 0.19.GITGUI git version 1.9.0.msysgit.0 - Jussi Peltonen -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
David Kastrup wrote: Felipe Contreras felipe.contre...@gmail.com writes: James Denholm wrote: Felipe Contreras wrote: This is a false dichotomy; there aren't just two kinds of Git users. There is such a category of Git users who are not fresh-out-of-the-boat, yet not power users either. Oh, I didn't mean to suggest a dichotomy of any kind. However these are the two groups (I suggest) are the most immediately relevant - one calls for change, and the other would be negatively impacted. Nobody would be negatively impacted. Who would be impacted negatively by having default aliases? The people having to read and understand scripts written in the expectation of default aliases. Which are imaginary. And I have showed they are not problems. You managed to convince yourself, so feel free to put aliases in every Git you use and distribute. There is evidence for the claim that there won't be those problems. You have absolutely no evidence there there will. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-rebase: fix probable reflog typo
Felipe Contreras felipe.contre...@gmail.com writes: Recently some code was changed to do 'test_must_fail env VAR=VAL command', why can't we do the same? I guess we can. -- 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: What is missing from Git v2.0
Felipe Contreras felipe.contre...@gmail.com writes: David Kastrup wrote: Felipe Contreras felipe.contre...@gmail.com writes: James Denholm wrote: Felipe Contreras wrote: This is a false dichotomy; there aren't just two kinds of Git users. There is such a category of Git users who are not fresh-out-of-the-boat, yet not power users either. Oh, I didn't mean to suggest a dichotomy of any kind. However these are the two groups (I suggest) are the most immediately relevant - one calls for change, and the other would be negatively impacted. Nobody would be negatively impacted. Who would be impacted negatively by having default aliases? The people having to read and understand scripts written in the expectation of default aliases. Which are imaginary. And I prefer them to stay that way since then one does not need to worry about them. And I have showed they are not problems. You managed to convince yourself, so feel free to put aliases in every Git you use and distribute. There is evidence for the claim that there won't be those problems. You have absolutely no evidence there there will. As I said: you managed to convince yourself and may milk that for all that it's worth. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Unicode: update of combining code points
Torsten Bögershausen: Some of the code points which have 0 length on the display are called combining, others are called vowels or accents. E.g. 5BF is not marked any of them, but if you look at the glyph, it should be combining (please correct me if that is wrong). All combining characters has a non-zero combining class in http://www.unicode.org/Public/UNIDATA/UnicodeData.txt (fourth field, called Canonical_Combining_Class in http://www.unicode.org/reports/tr44/ ). For instance, the aforementioned U+05BF is defined as follows: 05BF;HEBREW POINT RAFE;Mn;23;NSM;N; The combining class is 23, so this is a combining character. There is a difference between non-spacing combining marks (Mn in the third column (General_Category)) and others (Mc for spacing marks and Me for enclosing marks), so they might need specifial handling. Additionally, you have the zero-width characters, such as U+200B Zero Width Space. These have the Cf class, although it also contains visible characters IIRC. -- \\// Peter - http://www.softwolves.pp.se/ -- 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-request-pull: add --stat option
Which is passed on to git diff. I very need this option instead of changing the terminal size. Signed-off-by: Jiri Slaby jsl...@suse.cz --- git-request-pull.sh | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/git-request-pull.sh b/git-request-pull.sh index 5c1599752314..a23f03fddec0 100755 --- a/git-request-pull.sh +++ b/git-request-pull.sh @@ -13,6 +13,7 @@ OPTIONS_STUCKLONG= OPTIONS_SPEC='git request-pull [options] start url [end] -- pshow patch text as well +stat= specify stat output (see man git-diff for details) ' . git-sh-setup @@ -21,11 +22,16 @@ GIT_PAGER= export GIT_PAGER patch= +stat=--stat while case $# in 0) break ;; esac do case $1 in -p) patch=-p ;; + --stat) + stat=$1=$2 + shift + ;; --) shift; break ;; -*) @@ -152,6 +158,6 @@ then fi git shortlog ^$baserev $headrev -git diff -M --stat --summary $patch $merge_base..$headrev || status=1 +git diff -M $stat --summary $patch $merge_base..$headrev || status=1 exit $status -- 1.9.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 8/9] t4204-patch-id.sh: default is now stable
update test to match behaviour change Signed-off-by: Michael S. Tsirkin m...@redhat.com --- t/t4204-patch-id.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh index cd13e8e..03f91ce 100755 --- a/t/t4204-patch-id.sh +++ b/t/t4204-patch-id.sh @@ -138,8 +138,8 @@ test_expect_success 'splitting patch affects id with --unstable' ' ' #Now test various option combinations. -test_expect_success 'default is unstable' ' - test_patch_id relevant default +test_expect_success 'default is stable' ' + test_patch_id irrelevant default ' test_expect_success 'patchid.stable = true is stable' ' -- MST -- 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 v5 9/9] Documentation/git-patch-id.txt: default is stable
Update documentation to match behaviour change. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Documentation/git-patch-id.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt index e21b79b..9299b90 100644 --- a/Documentation/git-patch-id.txt +++ b/Documentation/git-patch-id.txt @@ -33,12 +33,13 @@ OPTIONS With this option, reordering file diffs that make up a patch or splitting a diff up to multiple diffs that touch the same path does not affect the ID. - This is the default if patchid.stable is set to true. + This is the default. --unstable:: Use a non-symmetrical sum of hashes, such that reordering or splitting the patch does affect the ID. - This is the default. + This is the default if patchid.stable is set to false. + This was the default value for git 1.9 and older. patch:: The diff to create the ID of. -- MST -- 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 v5 5/9] patch-id: document new behaviour
Clarify that patch ID can now be a sum of hashes, not a hash. Document how command line and config options affect the behaviour. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Documentation/git-patch-id.txt | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt index 312c3b1..e21b79b 100644 --- a/Documentation/git-patch-id.txt +++ b/Documentation/git-patch-id.txt @@ -8,14 +8,14 @@ git-patch-id - Compute unique ID for a patch SYNOPSIS [verse] -'git patch-id' patch +'git patch-id' [--stable | --unstable] patch DESCRIPTION --- -A patch ID is nothing but a SHA-1 of the diff associated with a patch, with -whitespace and line numbers ignored. As such, it's reasonably stable, but at -the same time also reasonably unique, i.e., two patches that have the same patch -ID are almost guaranteed to be the same thing. +A patch ID is nothing but a sum of SHA-1 of the diff hunks associated with a +patch, with whitespace and line numbers ignored. As such, it's reasonably +stable, but at the same time also reasonably unique, i.e., two patches that +have the same patch ID are almost guaranteed to be the same thing. IOW, you can use this thing to look for likely duplicate commits. @@ -27,6 +27,19 @@ This can be used to make a mapping from patch ID to commit ID. OPTIONS --- + +--stable:: + Use a symmetrical sum of hashes as the patch ID. + With this option, reordering file diffs that make up a patch or + splitting a diff up to multiple diffs that touch the same path + does not affect the ID. + This is the default if patchid.stable is set to true. + +--unstable:: + Use a non-symmetrical sum of hashes, such that reordering + or splitting the patch does affect the ID. + This is the default. + patch:: The diff to create the ID of. -- MST -- 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 v5 6/9] patch-id-test: test stable and unstable behaviour
Verify that patch ID supports an algorithm
that is stable against diff split and reordering.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
t/t4204-patch-id.sh | 128 +++-
1 file changed, 117 insertions(+), 11 deletions(-)
diff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh
index d2c930d..cd13e8e 100755
--- a/t/t4204-patch-id.sh
+++ b/t/t4204-patch-id.sh
@@ -5,27 +5,51 @@ test_description='git patch-id'
. ./test-lib.sh
test_expect_success 'setup' '
- test_commit initial foo a
- test_commit first foo b
- git checkout -b same HEAD^
- test_commit same-msg foo b
- git checkout -b notsame HEAD^
- test_commit notsame-msg foo c
+ as=a a a a a a a a # eight a
+ test_write_lines $as foo
+ test_write_lines $as bar
+ git add foo bar
+ git commit -a -m initial
+ test_write_lines $as b foo
+ test_write_lines $as b bar
+ git commit -a -m first
+ git checkout -b same master
+ git commit --amend -m same-msg
+ git checkout -b notsame master
+ echo c foo
+ echo c bar
+ git commit --amend -a -m notsame-msg
+ git checkout -b split master
+ test_write_lines d $as b foo
+ test_write_lines d $as b bar
+ git commit -a -m split
+ git checkout -b merged master
+ git checkout split -- foo bar
+ git commit --amend -a -m merged
+ test_write_lines bar foo bar-then-foo
+ test_write_lines foo bar foo-then-bar
'
test_expect_success 'patch-id output is well-formed' '
- git log -p -1 | git patch-id output
+ git log -p -1 | git patch-id output
grep ^[a-f0-9]\{40\} $(git rev-parse HEAD)$ output
'
+#calculate patch id. Make sure output is not empty.
calc_patch_id () {
- git patch-id |
- sed s# .*## patch-id_$1
+ name=$1
+ shift
+ git patch-id $@ |
+ sed s/ .*// patch-id_$name
+ test_line_count -gt 0 patch-id_$name
+}
+
+get_top_diff () {
+ git log -p -1 $@ -O bar-then-foo --
}
get_patch_id () {
- git log -p -1 $1 | git patch-id |
- sed s# .*## patch-id_$1
+ get_top_diff $1 | calc_patch_id $@
}
test_expect_success 'patch-id detects equality' '
@@ -56,6 +80,88 @@ test_expect_success 'whitespace is irrelevant in footer' '
test_cmp patch-id_master patch-id_same
'
+cmp_patch_id () {
+ if
+ test $1 = relevant
+ then
+ ! test_cmp patch-id_$2 patch-id_$3
+ else
+ test_cmp patch-id_$2 patch-id_$3
+ fi
+}
+
+test_patch_id_file_order () {
+ relevant=$1
+ shift
+ name=order-${1}-$relevant
+ shift
+ get_top_diff master | calc_patch_id $name $@
+ git checkout same
+ git format-patch -1 --stdout -O foo-then-bar |
+ calc_patch_id ordered-$name $@
+ cmp_patch_id $relevant $name ordered-$name
+
+}
+
+test_patch_id_split () {
+ relevant=$1
+ shift
+ name=split-${1}-$relevant
+ shift
+ get_top_diff merged | calc_patch_id $name $@
+ (git log -p -1 -O foo-then-bar split~1; git diff split~1..split) |
+ calc_patch_id split-$name $@
+ cmp_patch_id $relevant $name split-$name
+}
+
+# combined test for options
+test_patch_id () {
+ test_patch_id_file_order $@
+ test_patch_id_split $@
+}
+
+# small tests with detailed diagnostic for basic options.
+test_expect_success 'file order is irrelevant with --stable' '
+ test_patch_id_file_order irrelevant --stable --stable
+'
+
+test_expect_success 'file order is relevant with --unstable' '
+ test_patch_id_file_order relevant --unstable --unstable
+'
+
+test_expect_success 'splitting patch is irrelevant with --stable' '
+ test_patch_id_split irrelevant --stable --stable
+'
+
+test_expect_success 'splitting patch affects id with --unstable' '
+ test_patch_id_split relevant --unstable --unstable
+'
+
+#Now test various option combinations.
+test_expect_success 'default is unstable' '
+ test_patch_id relevant default
+'
+
+test_expect_success 'patchid.stable = true is stable' '
+ test_config patchid.stable true
+ test_patch_id irrelevant patchid.stable=true
+'
+
+test_expect_success 'patchid.stable = false is unstable' '
+ test_config patchid.stable false
+ test_patch_id relevant patchid.stable=false
+'
+
+test_expect_success '--unstable overrides patchid.stable = true' '
+ test_config patchid.stable true
+ test_patch_id relevant patchid.stable=true--unstable --unstable
+'
+
+test_expect_success '--stable overrides patchid.stable = false' '
+ test_config patchid.stable false
+ test_patch_id irrelevant patchid.stable=false--stable --stable
+'
+
test_expect_success 'patch-id supports git-format-patch MIME output' '
get_patch_id master
[PATCH v5 4/9] patch-id: make it stable against hunk reordering
Patch id changes if users
1. reorder file diffs that make up a patch
or
2. split a patch up to multiple diffs that touch the same path
(keeping hunks within a single diff ordered to make patch valid).
As the result is functionally equivalent, a different patch id is
surprising to many users.
In particular, reordering files using diff -O is helpful to make patches
more readable (e.g. API header diff before implementation diff).
Add an option to change patch-id behaviour making it stable against
these two kinds of patch change:
1. calculate SHA1 hash for each hunk separately and sum all hashes
(using a symmetrical sum) to get patch id
2. hash the file-level headers together with each hunk (not just the
first hunk)
We use a 20byte sum and not xor - since xor would give 0 output
for patches that have two identical diffs, which isn't all that
unlikely (e.g. append the same line in two places).
The new behaviour is enabled
- when patchid.stable is true
- when --stable flag is present
Using a new flag --unstable or setting patchid.stable to false force
the historical behaviour.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
builtin/patch-id.c | 89 --
1 file changed, 73 insertions(+), 16 deletions(-)
diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 3cfe02d..037cf2f 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -1,17 +1,14 @@
#include builtin.h
-static void flush_current_id(int patchlen, unsigned char *id, git_SHA_CTX *c)
+static void flush_current_id(int patchlen, unsigned char *id, unsigned char
*result)
{
- unsigned char result[20];
char name[50];
if (!patchlen)
return;
- git_SHA1_Final(result, c);
memcpy(name, sha1_to_hex(id), 41);
printf(%s %s\n, sha1_to_hex(result), name);
- git_SHA1_Init(c);
}
static int remove_space(char *line)
@@ -56,10 +53,31 @@ static int scan_hunk_header(const char *p, int *p_before,
int *p_after)
return 1;
}
-static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct
strbuf *line_buf)
+static void flush_one_hunk(unsigned char *result, git_SHA_CTX *ctx)
{
- int patchlen = 0, found_next = 0;
+ unsigned char hash[20];
+ unsigned short carry = 0;
+ int i;
+
+ git_SHA1_Final(hash, ctx);
+ git_SHA1_Init(ctx);
+ /* 20-byte sum, with carry */
+ for (i = 0; i 20; ++i) {
+ carry += result[i] + hash[i];
+ result[i] = carry;
+ carry = 8;
+ }
+}
+
+static int get_one_patchid(unsigned char *next_sha1, unsigned char *result,
+ struct strbuf *line_buf, int stable)
+{
+ int patchlen = 0, found_next = 0, hunks = 0;
int before = -1, after = -1;
+ git_SHA_CTX ctx, header_ctx;
+
+ git_SHA1_Init(ctx);
+ hashclr(result);
while (strbuf_getwholeline(line_buf, stdin, '\n') != EOF) {
char *line = line_buf-buf;
@@ -98,7 +116,19 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
if (before == 0 after == 0) {
if (!memcmp(line, @@ -, 4)) {
/* Parse next hunk, but ignore line numbers. */
+ if (stable) {
+ /* Hash the file-level headers together
with each hunk. */
+ if (hunks) {
+ flush_one_hunk(result, ctx);
+ /* Prepend saved header ctx for
next hunk. */
+ memcpy(ctx, header_ctx,
sizeof(ctx));
+ } else {
+ /* Save header ctx for next
hunk. */
+ memcpy(header_ctx, ctx,
sizeof(ctx));
+ }
+ }
scan_hunk_header(line, before, after);
+ hunks++;
continue;
}
@@ -107,7 +137,10 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
break;
/* Else we're parsing another header. */
+ if (stable hunks)
+ flush_one_hunk(result, ctx);
before = after = -1;
+ hunks = 0;
}
/* If we get here, we're inside a hunk. */
@@ -119,39 +152,63 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
/* Compute the sha without whitespace */
len = remove_space(line);
patchlen += len;
-
[PATCH v5 2/9] test: add test_write_lines helper
API and implementation as suggested by Junio.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
t/test-lib-functions.sh | 5 +
1 file changed, 5 insertions(+)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index aeae3ca..0e21275 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -712,6 +712,11 @@ test_ln_s_add () {
fi
}
+# This function writes out its parameters, one per line
+test_write_lines () {
+ printf %s\n $@;
+}
+
perl () {
command $PERL_PATH $@
}
--
MST
--
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 v5 7/9] patch-id: change default to stable
--stable has been the default in 'next' for a few weeks with no ill effects. Change the default to that so that users don't have to remember to enable it. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- builtin/patch-id.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/patch-id.c b/builtin/patch-id.c index 037cf2f..0b267af 100644 --- a/builtin/patch-id.c +++ b/builtin/patch-id.c @@ -198,9 +198,9 @@ int cmd_patch_id(int argc, const char **argv, const char *prefix) git_config(git_patch_id_config, stable); - /* If nothing is set, default to unstable. */ + /* If nothing is set, default to stable. */ if (stable 0) - stable = 0; + stable = 1; if (argc == 2 !strcmp(argv[1], --stable)) stable = 1; -- MST -- 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 v5 1/9] diff: add a config option to control orderfile
I always want my diffs to show header files first, then .c files, then the rest. Make it possible to set orderfile though a config option to achieve this. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- diff.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/diff.c b/diff.c index b79432b..6bcb26b 100644 --- a/diff.c +++ b/diff.c @@ -264,6 +264,9 @@ int git_diff_basic_config(const char *var, const char *value, void *cb) return 0; } + if (!strcmp(var, diff.orderfile)) + return git_config_string(default_diff_options.orderfile, var, value); + if (starts_with(var, submodule.)) return parse_submodule_config_option(var, value); -- MST -- 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 v5 3/9] tests: new test for orderfile options
The test is very basic and can be extended.
Couldn't find a good existing place to put it,
so created a new file.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
t/t4056-diff-order.sh | 63 +++
1 file changed, 63 insertions(+)
create mode 100755 t/t4056-diff-order.sh
diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
new file mode 100755
index 000..0404f50
--- /dev/null
+++ b/t/t4056-diff-order.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+test_description='diff orderfile'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ as=a a a a a a a a # eight a
+ test_write_lines $as foo
+ test_write_lines $as bar
+ git add foo bar
+ git commit -a -m initial
+ test_write_lines $as b foo
+ test_write_lines $as b bar
+ git commit -a -m first
+ test_write_lines bar foo bar-then-foo
+ test_write_lines foo bar foo-then-bar
+ git diff -Ofoo-then-bar HEAD~1..HEAD diff-foo-then-bar
+ git diff -Obar-then-foo HEAD~1..HEAD diff-bar-then-foo
+'
+
+test_diff_well_formed () {
+ grep ^+b $1 added
+ grep ^-b $1 removed
+ grep ^+++ $1 oldfiles
+ grep ^--- $1 newfiles
+ test_line_count = 2 added
+ test_line_count = 0 removed
+ test_line_count = 2 oldfiles
+ test_line_count = 2 newfiles
+}
+
+test_expect_success 'diff output with -O is well-formed' '
+ test_diff_well_formed diff-foo-then-bar
+ test_diff_well_formed diff-bar-then-foo
+'
+
+test_expect_success 'flag -O affects diff output' '
+ ! test_cmp diff-foo-then-bar diff-bar-then-foo
+'
+
+test_expect_success 'orderfile is same as -O' '
+ test_config diff.orderfile foo-then-bar
+ git diff HEAD~1..HEAD diff-foo-then-bar-config
+ test_config diff.orderfile bar-then-foo
+ git diff HEAD~1..HEAD diff-bar-then-foo-config
+ test_cmp diff-foo-then-bar diff-foo-then-bar-config
+ test_cmp diff-bar-then-foo diff-bar-then-foo-config
+'
+
+test_expect_success '-O overrides orderfile' '
+ test_config diff.orderfile foo-then-bar
+ git diff -Obar-then-foo HEAD~1..HEAD diff-bar-then-foo-flag
+ test_cmp diff-bar-then-foo diff-bar-then-foo-flag
+'
+
+test_expect_success '/dev/null is same as no orderfile' '
+ git diff -O/dev/null HEAD~1..HEADdiff-null-orderfile
+ git diff HEAD~1..HEAD diff-default
+ test_cmp diff-null-orderfile diff-default
+'
+
+test_done
--
MST
--
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 clone gives zero file permissions
Am 4/24/2014 10:24, schrieb Jussi Peltonen: I installed git to my Windows 7 workstation and cloned http://git.ipxe.org/ipxe.git; by using the Git GUI. ipxe-23042014/src tree looks like this in Cygwin bash: Files have no permissions, same goes with subfolders, e.g. $ ls -l config/ total 47 --+ 1 peltoju Domain Users 699 Apr 23 09:00 colour.h ... If that's the only problem, then I'd say: don't use Cygwin's ls to look at these files ;-) So, really, what is the problem? You are using: git-gui version 0.19.GITGUI git version 1.9.0.msysgit.0 This doesn't look like Cygwin's git. That there are some discrepancies in the result produced by Git for Windows is not surprising (but I'm not saying is expected). -- Hannes -- 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: error: git-remote-https died of signal 13
On Thu, Apr 24, 2014 at 12:15 AM, Jeff King p...@peff.net wrote:
I suspect the curl patch below may fix it:
diff --git a/lib/multi.c b/lib/multi.c
index bc93264..72e4825 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -1804,10 +1804,13 @@ static void close_all_connections(struct Curl_multi
*multi)
conn = Curl_conncache_find_first_connection(multi-conn_cache);
while(conn) {
+SIGPIPE_VARIABLE(pipe_st);
conn-data = multi-closure_handle;
+sigpipe_ignore(conn-data, pipe_st);
/* This will remove the connection from the cache */
(void)Curl_disconnect(conn, FALSE);
+sigpipe_restore(pipe_st);
conn = Curl_conncache_find_first_connection(multi-conn_cache);
}
The patch fixes the problem,
Greg
--
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] git tag --contains : avoid stack overflow
Thanks for all suggestions and explanations.
The diff against PATCH v2 is below, PATCH v3 follows.
Have a nice day,
Stepan
Subject: [PATCH] fixup! git tag --contains : avoid stack overflow
---
t/t7004-tag.sh | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index db82f6d..a911df0 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1423,12 +1423,15 @@ EOF
test_cmp expect actual
'
-ulimit_stack=ulimit -s 64
-test_lazy_prereq ULIMIT 'bash -c '$ulimit_stack''
+run_with_limited_stack () {
+ (ulimit -s 64 $@)
+}
+
+test_lazy_prereq ULIMIT 'run_with_limited_stack true'
-expect
# we require bash and ulimit, this excludes Windows
test_expect_success ULIMIT '--contains works in a deep repo' '
+ expect
i=1
while test $i -lt 4000
do
@@ -1442,7 +1445,7 @@ EOF
done | git fast-import
git checkout master
git tag far-far-away HEAD^
- bash -c '$ulimit_stack' git tag --contains HEAD actual
+ run_with_limited_stack git tag --contains HEAD actual
test_cmp expect actual
'
--
1.9.0.msysgit.0.119.g722efef
--
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] git tag --contains: avoid stack overflow
From: Jean-Jacques Lafay jeanjacques.la...@gmail.com
In large repos, the recursion implementation of contains(commit,
commit_list) may result in a stack overflow. Replace the recursion with
a loop to fix it.
This problem is more apparent on Windows than on Linux, where the stack
is more limited by default.
See also this thread on the msysGit list:
https://groups.google.com/d/topic/msysgit/FqT6boJrb2g/discussion
[jes: re-written to imitate the original recursion more closely]
Thomas Braun pointed out several documentation shortcomings.
Tests are run only if ulimit -s is available. This means they cannot
be run on Windows.
Signed-off-by: Jean-Jacques Lafay jeanjacques.la...@gmail.com
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
Tested-by: Stepan Kasal ka...@ucw.cz
---
Oops, actually there is one more omission, the comments needs to be
fixed:
-# we require bash and ulimit, this excludes Windows
+# we require ulimit, this excludes Windows
I forgot to add this to the preceding diff, but the final version
here has this fixed.
builtin/tag.c | 90 --
t/t7004-tag.sh | 26 +
2 files changed, 101 insertions(+), 15 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 6c7c6bd..f344002 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -80,11 +80,19 @@ static int in_commit_list(const struct commit_list *want,
struct commit *c)
return 0;
}
-static int contains_recurse(struct commit *candidate,
+enum contains_result {
+ CONTAINS_UNKNOWN = -1,
+ CONTAINS_NO = 0,
+ CONTAINS_YES = 1,
+};
+
+/*
+ * Test whether the candidate or one of its parents is contained in the list.
+ * Do not recurse to find out, though, but return -1 if inconclusive.
+ */
+static enum contains_result contains_test(struct commit *candidate,
const struct commit_list *want)
{
- struct commit_list *p;
-
/* was it previously marked as containing a want commit? */
if (candidate-object.flags TMP_MARK)
return 1;
@@ -92,26 +100,78 @@ static int contains_recurse(struct commit *candidate,
if (candidate-object.flags UNINTERESTING)
return 0;
/* or are we it? */
- if (in_commit_list(want, candidate))
+ if (in_commit_list(want, candidate)) {
+ candidate-object.flags |= TMP_MARK;
return 1;
+ }
if (parse_commit(candidate) 0)
return 0;
- /* Otherwise recurse and mark ourselves for future traversals. */
- for (p = candidate-parents; p; p = p-next) {
- if (contains_recurse(p-item, want)) {
- candidate-object.flags |= TMP_MARK;
- return 1;
- }
- }
- candidate-object.flags |= UNINTERESTING;
- return 0;
+ return -1;
}
-static int contains(struct commit *candidate, const struct commit_list *want)
+/*
+ * Mimicking the real stack, this stack lives on the heap, avoiding stack
+ * overflows.
+ *
+ * At each recursion step, the stack items points to the commits whose
+ * ancestors are to be inspected.
+ */
+struct stack {
+ int nr, alloc;
+ struct stack_entry {
+ struct commit *commit;
+ struct commit_list *parents;
+ } *stack;
+};
+
+static void push_to_stack(struct commit *candidate, struct stack *stack)
+{
+ int index = stack-nr++;
+ ALLOC_GROW(stack-stack, stack-nr, stack-alloc);
+ stack-stack[index].commit = candidate;
+ stack-stack[index].parents = candidate-parents;
+}
+
+static enum contains_result contains(struct commit *candidate,
+ const struct commit_list *want)
{
- return contains_recurse(candidate, want);
+ struct stack stack = { 0, 0, NULL };
+ int result = contains_test(candidate, want);
+
+ if (result != CONTAINS_UNKNOWN)
+ return result;
+
+ push_to_stack(candidate, stack);
+ while (stack.nr) {
+ struct stack_entry *entry = stack.stack[stack.nr - 1];
+ struct commit *commit = entry-commit;
+ struct commit_list *parents = entry-parents;
+
+ if (!parents) {
+ commit-object.flags |= UNINTERESTING;
+ stack.nr--;
+ }
+ /*
+* If we just popped the stack, parents-item has been marked,
+* therefore contains_test will return a meaningful 0 or 1.
+*/
+ else switch (contains_test(parents-item, want)) {
+ case CONTAINS_YES:
+ commit-object.flags |= TMP_MARK;
+ stack.nr--;
+ break;
+ case CONTAINS_NO:
+ entry-parents = parents-next;
+ break;
+ case CONTAINS_UNKNOWN:
+
Re: error: git-remote-https died of signal 13
On Thu, 24 Apr 2014, Jeff King wrote: Thanks, that's very helpful. I wasn't able to reproduce your problem locally, but I suspect the curl patch below may fix it: ... Daniel, I think the similar fix to curl_multi_cleanup in commit a900d45 missed this code path, and we need something like the above patch. I know you were trying to keep the SIGPIPE mess at the entrance-points to the library, and this works against that. But we need a SessionHandle to pass to sigpipe_ignore to look at its no_signal flag, and of course in the case of multi we may have several such handles. If there's a similar flag we can check on the multi handle, we could just cover all of curl_multi_cleanup with a single ignore/reset pair. Thanks a lot for this! I've taken this issue over to the libcurl mailing list and we'll work out the best way to address it over there... At least we know the patch works as intended. -- / daniel.haxx.se -- 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] setup: Fix windows path buffer over-stepping
Fix a buffer over-stepping issue triggered by providing an absolute path
that is similar to the work tree path.
abspath_part_inside_repo() may currently increment the path pointer by
offset_1st_component() + wtlen, which is too much, since
offset_1st_component() is a subset of wtlen.
For the *nix-style prefix '/', this does (by luck) not cause any issues,
since offset_1st_component() is 1 and there will always be a '/' or '\0'
that can absorb this.
In the case of DOS-style prefixes though, the offset_1st_component() is
3 and this can potentially over-step the string buffer. For example if
work_tree = c:/r
path = c:/rl
Then wtlen is 4, and incrementing the path pointer by (3 + 4) would
end up 2 bytes outside a string buffer of length 6.
Similarly if
work_tree = c:/r
path = c:/rl/d/a
Then (since the loop starts by also incrementing the pointer one step),
this would mean that the function would miss checking if c:/rl/d could
be the work_tree, arguably this is unlikely though, since it would only
be possible with symlinks on windows.
Fix this by simply avoiding to increment by offset_1st_component() and
wtlen at the same time.
Signed-off-by: Martin Erik Werner martinerikwer...@gmail.com
---
This is a follow-up on 655ee9e mw/symlinks which is currently merged into
master, prospective for git v2.0.0, the issue only affects v2.0.0-rc0.
setup.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index 613e3b3..0a22f8b 100644
--- a/setup.c
+++ b/setup.c
@@ -29,7 +29,7 @@ static int abspath_part_inside_repo(char *path)
return -1;
wtlen = strlen(work_tree);
len = strlen(path);
- off = 0;
+ off = offset_1st_component(path);
/* check if work tree is already the prefix */
if (wtlen = len !strncmp(path, work_tree, wtlen)) {
@@ -45,7 +45,7 @@ static int abspath_part_inside_repo(char *path)
off = wtlen;
}
path0 = path;
- path += offset_1st_component(path) + off;
+ path += off;
/* check each '/'-terminated level */
while (*path) {
--
1.9.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: What is missing from Git v2.0
On Wed, 23 Apr 2014 22:35:55 +, Felipe Contreras wrote: ... Anyway, if you disagree change one of your frequently used passwords to a chapter of The Lord of the Rings for a day. Let's see if you still think that. Proving that one extreme isn't the optimum doesn't prove the other is. Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
On Thu, Apr 24, 2014 at 03:23:54AM -0500, Felipe Contreras wrote: There is evidence for the claim that there won't be those problems. You have absolutely no evidence there there will. Felipe, It's clear that you've not been able to produce evidence that can convince most of the people on this thread. Simply repeating the same assertions over and over again, in a shrill fashion, is not likely to convince those of us who that this would not be a good idea for git v2.0. Creating a ~/.gitconfig file if one doesn't already is one I agree with, and at least on Unix systems, telling them that the config file lives in ~/.gitconfig, or where ever it might happen to be on other platforms, is a good one. If it's in some really weird place on Windows, then sure, we can tell them about git config -e. But the point is to let the user look at the default .gitconfig file, where we can put in comments to help explain what is going on, and perhaps have links to web pages for more information. I don't even think we need to query the user to fill out all of the fields. We can prepopulate a lot of the fields (name, e-mail address, etc.) from OS specific defaults that are available on most systems --- specifically, the default values we would use the name and e-mail address are not specified in a config file. We can just tell the user that we have created a default .gitconfig file, and tell them how they can take a look at it. In the long term, if the worry is how to bridge the gap between complete newbies, one way of dealing with this is to have a tutorial mode (off by default, on in the default .gitconfig) which despenses some helpful hints at certain strategic points (i.e., after five commits, give a message that introduces git log --oneline, after the third merge commit is created by the user, give a message which introduces git log --merge, and so on). The challenge is not strawing over the line to the point where the hints become as annoying as clippy, but that is what UX labs are for, to tune the experience for completely new users to git. Without doing a formal UX experiment, all of us are going to making assertions without formal evidence --- at best some of us who have tutored a few newbies might have some anecdates, but remember the old saying about the plural of anecdote not being data. Cheers, - Ted -- 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: [RTC/PATCH] Add 'update-branch' hook
Felipe Contreras felipe.contre...@gmail.com writes: I have a branch which should always be recompiled on update; post-update-branch would be a good place for that. And why would pre-update-branch not serve that purpose? Because the code that needs to be compiled is not yet in the workspace And it won't be in 'post-update-branch' either. Then you are using a very odd definition of post update % git checkout master % git branch feature-a stable - update-branch hook will be called here The hook will get 'feature-a' as the first argument, but the code in the workspace would correspond to 'master'; the checked out branch (pre or post). Then the hooks should be called 'pre-branch', 'post-branch'; there is no update involved. The hook I need is actually post-merge, since merge is the command that updates the workspace. Sorry for the noise. -- -- Stephe -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
I don't even think we need to query the user to fill out all of the fields. We can prepopulate a lot of the fields (name, e-mail address, etc.) from OS specific defaults that are available on most systems --- specifically, the default values we would use the name and e-mail address are not specified in a config file. Please don't. Or you end up again with Commiters like sb@localhost, sbeller@(None) or alike. I mean it's just one question once you setup a new computer, so I'd really like to see that question and then answer myself (at university/employer I might put in another email address than at home anyway, and I'm sure my boxes have no sane defaults) 2014-04-24 15:41 GMT+02:00 Theodore Ts'o ty...@mit.edu: On Thu, Apr 24, 2014 at 03:23:54AM -0500, Felipe Contreras wrote: There is evidence for the claim that there won't be those problems. You have absolutely no evidence there there will. Felipe, It's clear that you've not been able to produce evidence that can convince most of the people on this thread. Simply repeating the same assertions over and over again, in a shrill fashion, is not likely to convince those of us who that this would not be a good idea for git v2.0. Creating a ~/.gitconfig file if one doesn't already is one I agree with, and at least on Unix systems, telling them that the config file lives in ~/.gitconfig, or where ever it might happen to be on other platforms, is a good one. If it's in some really weird place on Windows, then sure, we can tell them about git config -e. But the point is to let the user look at the default .gitconfig file, where we can put in comments to help explain what is going on, and perhaps have links to web pages for more information. I don't even think we need to query the user to fill out all of the fields. We can prepopulate a lot of the fields (name, e-mail address, etc.) from OS specific defaults that are available on most systems --- specifically, the default values we would use the name and e-mail address are not specified in a config file. We can just tell the user that we have created a default .gitconfig file, and tell them how they can take a look at it. In the long term, if the worry is how to bridge the gap between complete newbies, one way of dealing with this is to have a tutorial mode (off by default, on in the default .gitconfig) which despenses some helpful hints at certain strategic points (i.e., after five commits, give a message that introduces git log --oneline, after the third merge commit is created by the user, give a message which introduces git log --merge, and so on). The challenge is not strawing over the line to the point where the hints become as annoying as clippy, but that is what UX labs are for, to tune the experience for completely new users to git. Without doing a formal UX experiment, all of us are going to making assertions without formal evidence --- at best some of us who have tutored a few newbies might have some anecdates, but remember the old saying about the plural of anecdote not being data. Cheers, - Ted -- 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 2/3] fetch.c: change s_update_ref to use a ref transaction
Fixed. Thanks.
On Wed, Apr 23, 2014 at 1:12 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Tue, Apr 22, 2014 at 2:45 PM, Ronnie Sahlberg sahlb...@google.com wrote:
Change s_update_ref to use a ref transaction for the ref update.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Doubled sign-off.
---
builtin/fetch.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a93c893..5c15584 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -375,7 +375,7 @@ static int s_update_ref(const char *action,
{
char msg[1024];
char *rla = getenv(GIT_REFLOG_ACTION);
- static struct ref_lock *lock;
+ struct ref_transaction *transaction;
if (dry_run)
return 0;
@@ -384,15 +384,14 @@ static int s_update_ref(const char *action,
snprintf(msg, sizeof(msg), %s: %s, rla, action);
errno = 0;
- lock = lock_any_ref_for_update(ref-name,
- check_old ? ref-old_sha1 : NULL,
- 0, NULL);
- if (!lock)
- return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
- if (write_ref_sha1(lock, ref-new_sha1, msg) 0)
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_update(transaction, ref-name, ref-new_sha1,
+ ref-old_sha1, 0, check_old) ||
+ ref_transaction_commit(transaction, msg,
UPDATE_REFS_QUIET_ON_ERR))
return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
STORE_REF_ERROR_OTHER;
+
return 0;
}
--
1.9.1.518.g16976cb.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: [PATCH v5 2/9] test: add test_write_lines helper
Michael S. Tsirkin wrote:
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -712,6 +712,11 @@ test_ln_s_add () {
fi
}
+# This function writes out its parameters, one per line
+test_write_lines () {
+ printf %s\n $@;
+}
+
Thanks for fixing this.
Nits:
* no need for the trailing semicolon
* it's probably worth documenting this in t/README as well so people
writing new test scripts know what it's about.
Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
GIT, libcurl and GSS-Negotiate
Hello, I'm having problems while trying to authenticate against a TFS hosted repository. I experience the same problem in git for windows and in git for linux (both versions are 1.9.2). The problem is described on a [github msysgit/git issue](https://github.com/msysgit/git/issues/171) To shortly resume it, the problem is that: * when the authentication method (WWW-Authenticate) is Negotiate AND * when the server proposes a NTLMSSP_CHALLENGE in response of the client's NTLMSSP_NEGOTIATE, = libcurl yields an Authentication problem. Ignoring this.\n And the communication is closed. At this point, in a normal communication, the client should send a NTLMSSP_AUTH containing a Kerberos ticket. Having seen the libcurl source code, I think we're passing through the lines from 776 to 780 of [http.c](https://github.com/bagder/curl/blob/2e57c7e0fcfb9214b2a9dfa8b3da258ded013b8a/lib/http.c). Some guy, on the github issue page, has suggested that this could be related to an update of libcurl, when git was at its 1.8.2 version. I'm not debugging libcurl, and I can't reproduce this problem @home. So, has somebody already experienced the same problem? Is there a solution? Many thanks in advance, Ivo -- http://www.nilleb.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 5/9] patch-id: document new behaviour
Michael S. Tsirkin wrote: Documentation/git-patch-id.txt | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) Ah, there's the documentation. Please squash this with the patch that introduces the new behavior so they can be reviewed together more easily (both now and later when people do archeology). [...] +--stable:: + Use a symmetrical sum of hashes as the patch ID. + With this option, reordering file diffs that make up a patch or + splitting a diff up to multiple diffs that touch the same path + does not affect the ID. + This is the default if patchid.stable is set to true. This doesn't explain to me why I would want to use --stable versus --unstable. Maybe an EXAMPLES section would help? The only reason I can think of to use --unstable is for compatibility with historical patch-ids. Is there any other reason? At this point in the series there is no patchid.stable configuration. +--unstable:: + Use a non-symmetrical sum of hashes, such that reordering What is a non-symmetrical sum? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 4/9] patch-id: make it stable against hunk reordering
Hi, Michael S. Tsirkin wrote: Patch id changes if users 1. reorder file diffs that make up a patch or 2. split a patch up to multiple diffs that touch the same path (keeping hunks within a single diff ordered to make patch valid). As the result is functionally equivalent, a different patch id is surprising to many users. Hm. If the goal is that functionally equivalent patches are guaranteed to produce the same patch-id, I wonder if we should be doing something like the following: 1. apply the patch in memory 2. generate a new diff 3. use that new diff to produce a patch-id Otherwise issues like --diff-algorithm=patience versus =myers will create trouble too. I don't think that avoiding false negatives for patch comparison without doing something like that is really possible. On the other hand if someone reorders file diffs within a patch, that is a potentially very common thing to do and something worth fixing. In other words, while your (1) makes perfect sense to me, case (2) seems less convincing. The downside of allowing reordering hunks is that it can potentially make different patches to be treated the same (for example if they were making similar changes to different functions) when the ordering previously caused them to be distinguished. But that wasn't something people could count on anyway, so I don't mind. Should the internal patch-id computation used by commands like 'git cherry' (see diff.c::diff_get_patch_id) get the same change? (Not a rhetorical question --- I don't know what the right choice would be there.) [...] The new behaviour is enabled - when patchid.stable is true - when --stable flag is present Using a new flag --unstable or setting patchid.stable to false force the historical behaviour. Which is the default? [...] builtin/patch-id.c | 89 -- 1 file changed, 73 insertions(+), 16 deletions(-) Documentation? Tests? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-request-pull: add --stat option
Jiri Slaby jsl...@suse.cz writes:
Which is passed on to git diff. I very need this option instead of
changing the terminal size.
Signed-off-by: Jiri Slaby jsl...@suse.cz
---
Interesting. I wonder if that suggests perhaps the default may be
better if it were --stat=80 regardless of your terminal width.
Oh, wait. That is the default we use when the output is not
connected to the terminal.
Initially, I thought that the motivation behind this is that you got
complaint from the recipient of your request that was generated to
fit the width of _your_ taste (which is a lot wider than the
standard 80) because you run the command in a wide terminal. But
that does not sound like it, as sending out a request would go more
like:
$ git request-pull ... request.txt
$ edit request.txt
$ mua send request.txt
and you would be getting 80-column output in that workflow.
What are you using the output of the script for, and why do you
very need this instead of changing the terminal size?
I am puzzled.
Perhaps is it the case that --stat output with full width of the
terminal does *not* suit _your_ taste (not just the recipient's),
and that is not limited to the request-pull output, but shared
across log -p --stat, diff --stat, and friends? I wonder if it
would be a better solution for you and those in the same situation
to set diff.statgraphwidth or something so that all these output are
limited to a reasonable width, if that is the case?
Perhaps that diff.statgraphwidth that only specifies the graph part
is too unwieldy and having a diff.statwidth or something that allows
you to customize that 80 or terminal width in a more direct way is
needed?
Regardless, having a way to pass thru an option, like this patch
does, is independently a good thing, I would tend to think. But I
need it instead of changing the terminal size does not look like a
sufficient and readable justification that describes why.
git-request-pull.sh | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/git-request-pull.sh b/git-request-pull.sh
index 5c1599752314..a23f03fddec0 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -13,6 +13,7 @@ OPTIONS_STUCKLONG=
OPTIONS_SPEC='git request-pull [options] start url [end]
--
pshow patch text as well
+stat= specify stat output (see man git-diff for details)
'
. git-sh-setup
@@ -21,11 +22,16 @@ GIT_PAGER=
export GIT_PAGER
patch=
+stat=--stat
whilecase $# in 0) break ;; esac
do
case $1 in
-p)
patch=-p ;;
+ --stat)
+ stat=$1=$2
+ shift
+ ;;
If somebody did not want to give diffstat output for whatever
reason, wouldn't it be natural to want to say
request-pull --stat= ...other options...
rather than having to say it with an explicit empty string, i.e.
request-pull --stat ...other options...
In other words, I think the patch should also add
--stat=*)
stat=$1
;;
--)
shift; break ;;
-*)
@@ -152,6 +158,6 @@ then
fi
git shortlog ^$baserev $headrev
-git diff -M --stat --summary $patch $merge_base..$headrev || status=1
+git diff -M $stat --summary $patch $merge_base..$headrev || status=1
This would not let the command notice a user error on the command
line of request-pull, e.g.
request-pull --stat='30 bar baz' ...other options...
because it will end up passing --stat=30, bar and baz as
separate options to it, no?
diff -M ${stat+=$stat} ...
perhaps?
exit $status
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
I may have missunderstood. So today you cannot commit if you don't provide an email address (usually the first time you try to commit, git asks to git config --global author.email=y...@mail.here), if I remember correctly, so there is definitely a valid (i.e. user approved) email address. 2014-04-24 17:47 GMT+02:00 ty...@mit.edu: On Thu, Apr 24, 2014 at 05:00:13PM +0200, Stefan Beller wrote: I don't even think we need to query the user to fill out all of the fields. We can prepopulate a lot of the fields (name, e-mail address, etc.) from OS specific defaults that are available on most systems --- specifically, the default values we would use the name and e-mail address are not specified in a config file. Please don't. Or you end up again with Commiters like sb@localhost, sbeller@(None) or alike. I mean it's just one question once you setup a new computer, so I'd really like to see that question and then answer myself (at university/employer I might put in another email address than at home anyway, and I'm sure my boxes have no sane defaults) But that's no worse than what we have today. What if we print what the defaults were, which might help encourage the user to actually run the git config -e command? - Ted -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
Stefan Beller wrote: I may have missunderstood. So today you cannot commit if you don't provide an email address (usually the first time you try to commit, git asks to git config --global author.email=y...@mail.here), if I remember correctly, so there is definitely a valid (i.e. user approved) email address. Not true. But you do get a big wall of text when you make your first commit without an EMAIL envvar or configured [user] section, including | You can suppress this message by setting them explicitly: | | git config --global user.name Your Name | git config --global user.email y...@example.com | | After doing this, you may fix the identity used for this commit with: | | git commit --amend --reset-author Ciao, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/6] patch-id: make it stable against hunk reordering
Michael S. Tsirkin m...@redhat.com writes: On Wed, Apr 23, 2014 at 03:05:42PM -0700, Junio C Hamano wrote: After comparing the patches 4-6 and the one that has been in 'next' for a few weeks, I tried to like the new one, but I couldn't. I'm fine with the one in next too. I was under the impression that you wanted me to change the behaviour so I worked on this,... What I wanted to see was to make sure this would not kick in unless the user explicitly asks. patchid.stable configuration variable is very much welcome, and if it defaulted to false with or without diff.orderfile, that would have been very much welcome. Then nobody will be surprised. But diff.orderfile configuration is all and only about the producer, and does not help the case at all. OK, we can just drop that, that's easy. Compared to it, the previous version forced people who do not have a particular reason to choose between the variants to use the new stable variant, which was a lot simpler solution. The reason why I merged the previous one to 'next' was because I wanted to see if the behaviour change is as grave as I thought, or I was just being unnecessary cautious. If there is no third-party script that precomputes something and stores them under these hashes that breaks with this change, I do not see any reason not to make the new stable one the default. Ah okay, so we just wait a bit and see and if all is quiet the existing patch will graduate to master with time? Totally cool. I thought you wanted me to add the config option, but if everyone is happy as is, I don't need it personally at all. ... or if we see problems, then build a fix on top to introduce patchid.stable that defaults to false and not linking the feature with diff.ordefile. Adding a new feature turned-off by default is the safer thing to do. When nothing changes, by defintion there won't be a new breakage. That is the default stance this project has taken for a long time, and what made us trusted by projects that manage their source files using our product. It however is to favour stability and no surprise over progress, and it may not be an optimal thing to do if the new feature is compellingly good. In some cases like this one, we may need to experiment to find out the need of the users, and introducing the feature with a configuration that is explicitly opt-in is one way to do so. We may or may not see many people using that feature without disrupting existing users that way. Cooking in 'next' with the feature turned-on by default is another way that is riskier, but in this particular case, the population that is likely to be affected are power users, which would match the audience of 'next'---those who still want stability but want to be closer to the cutting edge. -- 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: [RTC/PATCH] Add 'update-branch' hook
Stephen Leake wrote: Felipe Contreras felipe.contre...@gmail.com writes: I have a branch which should always be recompiled on update; post-update-branch would be a good place for that. And why would pre-update-branch not serve that purpose? Because the code that needs to be compiled is not yet in the workspace And it won't be in 'post-update-branch' either. Then you are using a very odd definition of post update It's not. The branch was updated, not the workspace. % git checkout master % git branch feature-a stable - update-branch hook will be called here The hook will get 'feature-a' as the first argument, but the code in the workspace would correspond to 'master'; the checked out branch (pre or post). Then the hooks should be called 'pre-branch', 'post-branch'; there is no update involved. Of course there is. A 'branch' hook would be triggered when you create a new branch (e.g. `git branch`), however, it should not be triggered when you update a branch (e.g. `git rebase`). The hook I need is actually post-merge, since merge is the command that updates the workspace. I'd say it's probably 'post-checkout'. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
tytso@ wrote: On Thu, Apr 24, 2014 at 05:00:13PM +0200, Stefan Beller wrote: I don't even think we need to query the user to fill out all of the fields. We can prepopulate a lot of the fields (name, e-mail address, etc.) from OS specific defaults that are available on most systems --- specifically, the default values we would use the name and e-mail address are not specified in a config file. Please don't. Or you end up again with Commiters like sb@localhost, sbeller@(None) or alike. I mean it's just one question once you setup a new computer, so I'd really like to see that question and then answer myself (at university/employer I might put in another email address than at home anyway, and I'm sure my boxes have no sane defaults) But that's no worse than what we have today. What if we print what the defaults were, which might help encourage the user to actually run the git config -e command? In addition we shouldn't consider user@host a valid e-mail. In the vast majority of cases it's not. http://article.gmane.org/gmane.comp.version-control.git/232027 -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mergetool: run prompt only if guessed tool
Junio C Hamano gits...@pobox.com writes: Perhaps like this? I take that your original motivation was to confirm to run a tool on this particular (as opposed to another) path, but the user can also take the prompt as to confirm to run this (as opposed to some other) tool. The latter of which of course is irritating for those who told which exact tool to use. I think the large part of the reason why you and Felipe had to have a long back-and-forth was because it was unclear that the prompt served these two purposes from the documentation, so I attempted to clarify the first motivation by adding a brief half-sentence. I'll queue the following as a separate documentation patch. We may want to polish the wording, so I'll keep it out of 'next' for now. I think the main patch is good for 'next' with or without doc update to be cooked during the remainder of this cycle, and I merged it there already. Thanks. -- 8 -- Subject: [PATCH] mergetool: document the default for --[no-]prompt The original motivation of using the prompt was to confirm to run a tool on this particular (as opposed to another) path, but the user can also take the prompt as to confirm to run this (as opposed to some other) tool. The latter of which of course is irritating for those who told which exact tool to use, which is the reason why we are flipping the default. During the review discussion of the patch, many people (including the maintainer) missed that a user can find the prompt useful way to skip running the tool on particular paths. Clarify it by adding a brief half-sentence to the description. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-mergetool.txt | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index 07137f2..e846c2e 100644 --- a/Documentation/git-mergetool.txt +++ b/Documentation/git-mergetool.txt @@ -71,11 +71,13 @@ success of the resolution after the custom tool has exited. --no-prompt:: Don't prompt before each invocation of the merge resolution program. + This is the default if the merge resolution program is + explicitly specified with the `--tool` option or with the + `merge.tool` configuration variable. --prompt:: - Prompt before each invocation of the merge resolution program. - This is the default behaviour; the option is provided to - override any configuration settings. + Prompt before each invocation of the merge resolution program + to give the user a chance to skip the path. TEMPORARY FILES --- -- 2.0.0-rc0-224-g3c1c0b8 -- 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 v5 2/9] test: add test_write_lines helper
Jonathan Nieder jrnie...@gmail.com writes:
Michael S. Tsirkin wrote:
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -712,6 +712,11 @@ test_ln_s_add () {
fi
}
+# This function writes out its parameters, one per line
+test_write_lines () {
+printf %s\n $@;
+}
+
Thanks for fixing this.
Nits:
* no need for the trailing semicolon
Good eyes. I was wondering if I wrote that (which I found very
unlikely).
* it's probably worth documenting this in t/README as well so people
writing new test scripts know what it's about.
Sounds like a good idea.
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: What is missing from Git v2.0
Stefan Beller wrote: I may have missunderstood. So today you cannot commit if you don't provide an email address (usually the first time you try to commit, git asks to git config --global author.email=y...@mail.here), if I remember correctly, so there is definitely a valid (i.e. user approved) email address. That's not true, that's only the case if you don't have a fully qualified hostname, like 'localhost', but if you do, like localhost.redhat, then Git assumes your email is user@localhost.redhat, and it's valid. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
Jonathan Nieder wrote: Stefan Beller wrote: I may have missunderstood. So today you cannot commit if you don't provide an email address (usually the first time you try to commit, git asks to git config --global author.email=y...@mail.here), if I remember correctly, so there is definitely a valid (i.e. user approved) email address. Not true. But you do get a big wall of text when you make your first commit without an EMAIL envvar or configured [user] section, including Only if you don't have a fully qualified hostname. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] setup: Fix windows path buffer over-stepping
Martin Erik Werner martinerikwer...@gmail.com writes:
Fix a buffer over-stepping issue triggered by providing an absolute path
that is similar to the work tree path.
abspath_part_inside_repo() may currently increment the path pointer by
offset_1st_component() + wtlen, which is too much, since
offset_1st_component() is a subset of wtlen.
For the *nix-style prefix '/', this does (by luck) not cause any issues,
since offset_1st_component() is 1 and there will always be a '/' or '\0'
that can absorb this.
In the case of DOS-style prefixes though, the offset_1st_component() is
3 and this can potentially over-step the string buffer. For example if
work_tree = c:/r
path = c:/rl
Then wtlen is 4, and incrementing the path pointer by (3 + 4) would
end up 2 bytes outside a string buffer of length 6.
Similarly if
work_tree = c:/r
path = c:/rl/d/a
Then (since the loop starts by also incrementing the pointer one step),
this would mean that the function would miss checking if c:/rl/d could
be the work_tree, arguably this is unlikely though, since it would only
be possible with symlinks on windows.
Fix this by simply avoiding to increment by offset_1st_component() and
wtlen at the same time.
Signed-off-by: Martin Erik Werner martinerikwer...@gmail.com
---
This is a follow-up on 655ee9e mw/symlinks which is currently merged into
master, prospective for git v2.0.0, the issue only affects v2.0.0-rc0.
Thanks for a fix and from a cursory read of the surrounding code, I
think the patch makes sense.
I appreciate your doing so before the breakage hits a released
version very much.
setup.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index 613e3b3..0a22f8b 100644
--- a/setup.c
+++ b/setup.c
@@ -29,7 +29,7 @@ static int abspath_part_inside_repo(char *path)
return -1;
wtlen = strlen(work_tree);
len = strlen(path);
- off = 0;
+ off = offset_1st_component(path);
/* check if work tree is already the prefix */
if (wtlen = len !strncmp(path, work_tree, wtlen)) {
@@ -45,7 +45,7 @@ static int abspath_part_inside_repo(char *path)
off = wtlen;
}
path0 = path;
- path += offset_1st_component(path) + off;
+ path += off;
/* check each '/'-terminated level */
while (*path) {
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
David Kastrup wrote: Felipe Contreras felipe.contre...@gmail.com writes: David Kastrup wrote: The people having to read and understand scripts written in the expectation of default aliases. Which are imaginary. And I prefer them to stay that way since then one does not need to worry about them. If everybody was afraid of moving because of imaginary fears like you, nothing would get done in this world. Rational people distinguish the imaginary from the real. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1
Hi,
On Apr 22, 2014 2:53 AM, Junio C Hamano gits...@pobox.com wrote:
Richard Hansen rhan...@bbn.com writes:
Both bash and zsh subject the value of PS1 to parameter expansion,
command substitution, and arithmetic expansion. Rather than include
the raw, unescaped branch name in PS1 when running in two- or
three-argument mode, construct PS1 to reference a variable that holds
the branch name. Because the shells do not recursively expand, this
avoids arbitrary code execution by specially-crafted branch names such
as '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)'.
Signed-off-by: Richard Hansen rhan...@bbn.com
I'd like to see this patch eyeballed by those who have been involved
in the script (shortlog and blame tells me they are SZEDER and
Simon, CC'ed), so that we can hopefully merge it by the time -rc1 is
tagged.
I think this is a sensible thing to do. However, for now I can only check the
patch on my phone, hence I can't say any more (e.g. acked or reviewed by) than
that, unfortunately.
+ # not needed anymore; keep user's
+ # environment clean
+ unset __git_ps1_upstream_name
+ fi
We already have a lot of stuff in the user's environment beginning with __git,
so I don't think the unset is necessary.
Best,
Gábor
Nďż˝r��ybďż˝X��ǧvďż˝^ďż˝)Ţş{.nďż˝+ا���ܨ}ďż˝ďż˝ďż˝Ć zďż˝j:+v���zZ+��+zf���h���~i���zďż˝ ďż˝w���?��)ߢf
Re: What is missing from Git v2.0
Andreas Krey wrote: On Wed, 23 Apr 2014 22:35:55 +, Felipe Contreras wrote: ... Anyway, if you disagree change one of your frequently used passwords to a chapter of The Lord of the Rings for a day. Let's see if you still think that. Proving that one extreme isn't the optimum doesn't prove the other is. It's called hyperbole, and it's a very common and very effective rhetorical device. Nobody argued that the extreme opposite was best. My point was that the amount of characters you type _does_ matter, and I think I proved my point. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/9] tests: new test for orderfile options
Michael S. Tsirkin m...@redhat.com writes:
The test is very basic and can be extended.
Couldn't find a good existing place to put it,
so created a new file.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
t/t4056-diff-order.sh | 63
+++
1 file changed, 63 insertions(+)
create mode 100755 t/t4056-diff-order.sh
diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
new file mode 100755
index 000..0404f50
Huh? What codebase is this based on?
I think we had t4056 since b5277730 (t4056: add new tests for git
diff -O, 2013-12-18).
Puzzled...
--- /dev/null
+++ b/t/t4056-diff-order.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+test_description='diff orderfile'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ as=a a a a a a a a # eight a
+ test_write_lines $as foo
+ test_write_lines $as bar
+ git add foo bar
+ git commit -a -m initial
+ test_write_lines $as b foo
+ test_write_lines $as b bar
+ git commit -a -m first
+ test_write_lines bar foo bar-then-foo
+ test_write_lines foo bar foo-then-bar
+ git diff -Ofoo-then-bar HEAD~1..HEAD diff-foo-then-bar
+ git diff -Obar-then-foo HEAD~1..HEAD diff-bar-then-foo
+'
+
+test_diff_well_formed () {
+ grep ^+b $1 added
+ grep ^-b $1 removed
+ grep ^+++ $1 oldfiles
+ grep ^--- $1 newfiles
+ test_line_count = 2 added
+ test_line_count = 0 removed
+ test_line_count = 2 oldfiles
+ test_line_count = 2 newfiles
+}
+
+test_expect_success 'diff output with -O is well-formed' '
+ test_diff_well_formed diff-foo-then-bar
+ test_diff_well_formed diff-bar-then-foo
+'
+
+test_expect_success 'flag -O affects diff output' '
+ ! test_cmp diff-foo-then-bar diff-bar-then-foo
+'
+
+test_expect_success 'orderfile is same as -O' '
+ test_config diff.orderfile foo-then-bar
+ git diff HEAD~1..HEAD diff-foo-then-bar-config
+ test_config diff.orderfile bar-then-foo
+ git diff HEAD~1..HEAD diff-bar-then-foo-config
+ test_cmp diff-foo-then-bar diff-foo-then-bar-config
+ test_cmp diff-bar-then-foo diff-bar-then-foo-config
+'
+
+test_expect_success '-O overrides orderfile' '
+ test_config diff.orderfile foo-then-bar
+ git diff -Obar-then-foo HEAD~1..HEAD diff-bar-then-foo-flag
+ test_cmp diff-bar-then-foo diff-bar-then-foo-flag
+'
+
+test_expect_success '/dev/null is same as no orderfile' '
+ git diff -O/dev/null HEAD~1..HEADdiff-null-orderfile
+ git diff HEAD~1..HEAD diff-default
+ test_cmp diff-null-orderfile diff-default
+'
+
+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: What is missing from Git v2.0
Theodore Ts'o wrote: On Thu, Apr 24, 2014 at 03:23:54AM -0500, Felipe Contreras wrote: There is evidence for the claim that there won't be those problems. You have absolutely no evidence there there will. It's clear that you've not been able to produce evidence that can convince most of the people on this thread. Which only proves you don't want to be convinced, no evidence could convince you. I don't even think we need to query the user to fill out all of the fields. We can prepopulate a lot of the fields (name, e-mail address, etc.) from OS specific defaults that are available on most systems --- specifically, the default values we would use the name and e-mail address are not specified in a config file. Most systems don't have a configured $EMAIL, so those defautls would be wrong. It's so evident that no evide could convince you that you don't even bothere to answer my question: Why does Mercurial, Bazaar, Subversion, CVS, and pretty much everything uses aliases? Since you are conveniently not answering, I'll answer for you: Because such hypothetical problems wouldn't actually occur in reality with Git, just like they don't occur in other tools. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
David Kastrup d...@gnu.org writes: d...@mailtor.net writes: It would be nice if we could change the flags to either a) avoid cutting off b) indicate something has been cut off (- I prefer this) I assume there are more people with a similar workflow who're still unaware of this feature. I would joke about how 3 letter agencies introduced this flag to backdoor open source projects, but, well.. Most terminals are wider than three letters. I am having a hard time to decide if you genuinely misread what you are responding to, or if you are joking. If the latter, I find the joke mildly funny in a twisted way ;-) But the tangent aside... Still, it is a total nuisance. I am constantly doing -S RET on my git output. This should be left alone as an entirely personal preference quite unrelated to Git. There is no point in having Git configure a default different from what is used elsewhere. I almost agree with the general principle of the last sentence, but with a bit of reservation. The default value for LESS (i.e. when the user does not have any) we pass is FRSX, and the Porcelain output these days is colored by default. If we don't set a default at all, the end-user experience for a newbie will be bad, especially without R. Among the other three, F and X are to avoid a short output (e.g git show on a one-liner with a short explanation) from asking for confirmation to leave the pager and from clearing the screen upon leaving the pager, and are generally accepted as good things (or at least, we haven't seen much issue raised after we started passing the default LESS for those who do not have their own in their environment). Use of S is very subjective. While I personally do appreciate that we have it by default, I can perfectly well understand why some people do not want to see it in the default. The best we can do is to arrange so that people from one of the camps have their favorite out of the box and those from the other camp need to tell Git that they want to (or do not want to) fold long lines. Traditionally, because the tool grew in a context of being used in a project whose participants are at least not malicious, always having to be on the lookout for fear of middle-of-line tabs hiding bad contents near the right edges of lines has never been an issue. If somebody brought up a potential issue of such mode of attack back then, Linus may have chosen the default differently. I may have myself chosen not to have S, if I were the maintainer when the LESS default was originally introduced, and had I been made aware of this issue. I am not opposed to changing the default in the longer term, as long as we have a solid transition plan to ensure that it won't disrupt and/or upset existing users too much. -- 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 v5 4/9] patch-id: make it stable against hunk reordering
Jonathan Nieder jrnie...@gmail.com writes: Should the internal patch-id computation used by commands like 'git cherry' (see diff.c::diff_get_patch_id) get the same change? (Not a rhetorical question --- I don't know what the right choice would be there.) I thought about it but I did not think of a reason why. If we do not store the patch-id (it would be a misnomer especially after this series, it is mor like patch signature), and we generate the patch to be hashed internally without getting affected by any user input given per-invocation, then nothing is externally observable even if we used two completely different definition of patch id computation, and I think these preconditions do hold. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
On Thu, Apr 24, 2014 at 01:26:33PM -0500, Felipe Contreras wrote: Jonathan Nieder wrote: Stefan Beller wrote: I may have missunderstood. So today you cannot commit if you don't provide an email address (usually the first time you try to commit, git asks to git config --global author.email=y...@mail.here), if I remember correctly, so there is definitely a valid (i.e. user approved) email address. Not true. But you do get a big wall of text when you make your first commit without an EMAIL envvar or configured [user] section, including Only if you don't have a fully qualified hostname. No, we alway warn if the values weren't explicitly provided: $ git config --global --unset user.email $ git commit --allow-empty -m foo [master 1e987ba] foo Committer: Jeff King p...@sigill.intra.peff.net Your name and email address were configured automatically based on your username and hostname. Please check that they are accurate. You can suppress this message by setting them explicitly: git config --global user.name Your Name git config --global user.email y...@example.com After doing this, you may fix the identity used for this commit with: git commit --amend --reset-author but we will consider several sources explicit, like $GIT_COMMITTER_EMAIL, $EMAIL, and of course user.email: $ EMAIL=whate...@example.com git commit --allow-empty -m foo [master e75f17a] foo We die when the values are implicitly derived from the system _and_ they look bogus: $ sudo rm /etc/mailname $ sudo hostname bogus $ git commit --allow-empty -m foo *** Please tell me who you are. Run git config --global user.email y...@example.com git config --global user.name Your Name to set your account's default identity. Omit --global to set the identity only in this repository. fatal: unable to auto-detect email address (got 'peff@bogus.(none)') -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: What is missing from Git v2.0
On Thu, 24 Apr 2014, Felipe Contreras wrote: David Kastrup wrote: Felipe Contreras felipe.contre...@gmail.com writes: David Kastrup wrote: The people having to read and understand scripts written in the expectation of default aliases. Which are imaginary. And I prefer them to stay that way since then one does not need to worry about them. If everybody was afraid of moving because of imaginary fears like you, nothing would get done in this world. Rational people distinguish the imaginary from the real. The exact same thing could be said for your position that typing rebase instead of rb causes a major problem Calm down and stop accusing everyone of sticking their heads in the ground David Lang -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
Felipe Contreras felipe.contre...@gmail.com writes: Andreas Krey wrote: On Wed, 23 Apr 2014 22:35:55 +, Felipe Contreras wrote: ... Anyway, if you disagree change one of your frequently used passwords to a chapter of The Lord of the Rings for a day. Let's see if you still think that. Proving that one extreme isn't the optimum doesn't prove the other is. It's called hyperbole, and it's a very common and very effective rhetorical device. Let us conclude this discussion by declaring you the best rhetor then. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
Junio C Hamano gits...@pobox.com writes: Traditionally, because the tool grew in a context of being used in a project whose participants are at least not malicious, always having to be on the lookout for fear of middle-of-line tabs hiding bad contents near the right edges of lines has never been an issue. My beef is not with hiding bad contents but with hiding contents. It makes the output useless for seeing what is actually happening as soon as the option starts having an effect. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
David Kastrup d...@gnu.org writes: Junio C Hamano gits...@pobox.com writes: Traditionally, because the tool grew in a context of being used in a project whose participants are at least not malicious, always having to be on the lookout for fear of middle-of-line tabs hiding bad contents near the right edges of lines has never been an issue. My beef is not with hiding bad contents but with hiding contents. It makes the output useless for seeing what is actually happening as soon as the option starts having an effect. My suspicion is that one of the reasons why S was chosen to be in the default was to mildly discourage people from busting the usual line-length limit, but I am not Linus ;-) -- 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 v6 1/2] add: add --ignore-submodules[=when] parameter
Allow ignoring submodules (or not) by command line switch, like diff
and status do.
This commit is also a prerequisite for the next one in series, which
adds the --ignore-submodules switch to git commit. That's why a new
argument is added to public function add_files_to_cache(), and its
call sites are updated to pass NULL.
Signed-off-by: Ronald Weiss weiss.ron...@gmail.com
---
Git add currently doesn't honor ignore setting from .gitmodules or
.git/config, which is related functionality, however I'd like to
change that in another patch, coming soon.
Patch changelog:
v6
* corrected wording and formatting errors (as pointed out by Eric Sunshine)
v5
* fixed file mode of added test script (644 - 755)
* cleaned up commit message
Documentation/git-add.txt| 7 +-
builtin/add.c| 16 --
builtin/checkout.c | 2 +-
builtin/commit.c | 2 +-
cache.h | 2 +-
t/t3704-add-ignore-submodules.sh | 47
6 files changed, 70 insertions(+), 6 deletions(-)
create mode 100755 t/t3704-add-ignore-submodules.sh
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 9631526..ef69f8b 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
[--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
[--intent-to-add | -N] [--refresh] [--ignore-errors]
[--ignore-missing]
- [--] [pathspec...]
+ [--ignore-submodules[=when]] [--] [pathspec...]
DESCRIPTION
---
@@ -164,6 +164,11 @@ for git add --no-all pathspec..., i.e. ignored removed
files.
be ignored, no matter if they are already present in the work
tree or not.
+--ignore-submodules[=when]::
+ Can be used to override any settings of the 'submodule.*.ignore'
+ option in linkgit:git-config[1] or linkgit:gitmodules[5].
+ when can be either none or all, defaulting to all.
+
\--::
This option can be used to separate command-line options from
the list of files, (useful when filenames might be mistaken
diff --git a/builtin/add.c b/builtin/add.c
index 459208a..85f2110 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -83,7 +83,8 @@ static void update_callback(struct diff_queue_struct *q,
}
int add_files_to_cache(const char *prefix,
- const struct pathspec *pathspec, int flags)
+ const struct pathspec *pathspec, int flags,
+ const char *ignore_submodules_arg)
{
struct update_callback_data data;
struct rev_info rev;
@@ -99,6 +100,12 @@ int add_files_to_cache(const char *prefix,
rev.diffopt.format_callback = update_callback;
rev.diffopt.format_callback_data = data;
rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
+
+ if (ignore_submodules_arg) {
+ DIFF_OPT_SET(rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+ handle_ignore_submodules_arg(rev.diffopt,
ignore_submodules_arg);
+ }
+
run_diff_files(rev, DIFF_RACY_IS_MODIFIED);
return !!data.add_errors;
}
@@ -237,6 +244,8 @@ static int ignore_add_errors, intent_to_add, ignore_missing;
static int addremove = ADDREMOVE_DEFAULT;
static int addremove_explicit = -1; /* unspecified */
+static char *ignore_submodule_arg;
+
static int ignore_removal_cb(const struct option *opt, const char *arg, int
unset)
{
/* if we are told to ignore, we are not adding removals */
@@ -262,6 +271,9 @@ static struct option builtin_add_options[] = {
OPT_BOOL( 0 , refresh, refresh_only, N_(don't add, only refresh the
index)),
OPT_BOOL( 0 , ignore-errors, ignore_add_errors, N_(just skip files
which cannot be added because of errors)),
OPT_BOOL( 0 , ignore-missing, ignore_missing, N_(check if - even
missing - files are ignored in dry run)),
+ { OPTION_STRING, 0, ignore-submodules, ignore_submodule_arg,
N_(when),
+ N_(ignore changes to submodules, optional when: all, none. (Default:
all)),
+ PARSE_OPT_OPTARG, NULL, (intptr_t)all },
OPT_END(),
};
@@ -434,7 +446,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
plug_bulk_checkin();
- exit_status |= add_files_to_cache(prefix, pathspec, flags);
+ exit_status |= add_files_to_cache(prefix, pathspec, flags,
ignore_submodule_arg);
if (add_new_files)
exit_status |= add_files(dir, flags);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 07cf555..607af47 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -525,7 +525,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
* entries in the index.
*/
- add_files_to_cache(NULL, NULL, 0);
+
[PATCH v6 2/2] commit: add --ignore-submodules[=when] parameter
Allow ignoring submodules (or not) by command line switch, like diff
and status do.
Git commit honors the 'ignore' setting from .gitmodules or .git/config,
but didn't allow to override it from command line.
This patch depends on Jens Lehmann's patch commit -m: commit staged
submodules regardless of ignore config. Without it,
commit -m --ignore-submodules would not work and tests introduced
here would fail.
Signed-off-by: Ronald Weiss weiss.ron...@gmail.com
---
Patch changelog:
v6
* corrected wording and formatting errors (as pointed out by Eric Sunshine)
v5
* fixed file mode of added test script (644 - 755)
* replaced test_might_fail with test_must_fail in test script
Documentation/git-commit.txt| 7
builtin/commit.c| 8 +++-
t/t7513-commit-ignore-submodules.sh | 80 +
3 files changed, 94 insertions(+), 1 deletion(-)
create mode 100755 t/t7513-commit-ignore-submodules.sh
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0bbc8f5..55995be 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,6 +13,7 @@ SYNOPSIS
[-F file | -m msg] [--reset-author] [--allow-empty]
[--allow-empty-message] [--no-verify] [-e] [--author=author]
[--date=date] [--cleanup=mode] [--[no-]status]
+ [--ignore-submodules[=when]]
[-i | -o] [-S[key-id]] [--] [file...]
DESCRIPTION
@@ -277,6 +278,12 @@ The possible options are:
The default can be changed using the status.showUntrackedFiles
configuration variable documented in linkgit:git-config[1].
+--ignore-submodules[=when]::
+ Can be used to override any settings of the 'submodule.*.ignore'
+ option in linkgit:git-config[1] or linkgit:gitmodules[5].
+ when can be either none, dirty, untracked or all,
+ defaulting to all.
+
-v::
--verbose::
Show unified diff between the HEAD commit and what
diff --git a/builtin/commit.c b/builtin/commit.c
index 5444111..dc1d8d0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -361,7 +361,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
*/
if (all || (also pathspec.nr)) {
fd = hold_locked_index(index_lock, 1);
- add_files_to_cache(also ? prefix : NULL, pathspec, 0, NULL);
+ add_files_to_cache(also ? prefix : NULL, pathspec, 0,
ignore_submodule_arg);
refresh_cache_or_die(refresh_flags);
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
@@ -1540,6 +1540,9 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
OPT_BOOL(0, amend, amend, N_(amend previous commit)),
OPT_BOOL(0, no-post-rewrite, no_post_rewrite, N_(bypass
post-rewrite hook)),
{ OPTION_STRING, 'u', untracked-files, untracked_files_arg,
N_(mode), N_(show untracked files, optional modes: all, normal, no.
(Default: all)), PARSE_OPT_OPTARG, NULL, (intptr_t)all },
+ { OPTION_STRING, 0, ignore-submodules, ignore_submodule_arg,
N_(when),
+ N_(ignore changes to submodules, optional when: all, none.
(Default: all)),
+ PARSE_OPT_OPTARG, NULL, (intptr_t)all },
/* end commit contents options */
OPT_HIDDEN_BOOL(0, allow-empty, allow_empty,
@@ -1578,6 +1581,9 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
argc = parse_and_validate_options(argc, argv, builtin_commit_options,
builtin_commit_usage,
prefix, current_head, s);
+
+ s.ignore_submodule_arg = ignore_submodule_arg;
+
if (dry_run)
return dry_run_commit(argc, argv, prefix, current_head, s);
index_file = prepare_index(argc, argv, prefix, current_head, 0);
diff --git a/t/t7513-commit-ignore-submodules.sh
b/t/t7513-commit-ignore-submodules.sh
new file mode 100755
index 000..10ae178
--- /dev/null
+++ b/t/t7513-commit-ignore-submodules.sh
@@ -0,0 +1,80 @@
+#!/bin/sh
+#
+# Copyright (c) 2014 Ronald Weiss
+#
+
+test_description='Test of git commit --ignore-submodules'
+
+. ./test-lib.sh
+
+test_expect_success 'create submodule' '
+ test_create_repo sm
+ (
+ cd sm
+ foo
+ git add foo
+ git commit -m Add foo
+ )
+ git submodule add ./sm
+ git commit -m Add sm
+'
+
+update_sm () {
+ (
+ cd sm
+ echo bar foo
+ git add foo
+ git commit -m Updated foo
+ )
+}
+
+test_expect_success 'commit -a --ignore-submodules=all ignores dirty
submodule' '
+ update_sm
+ test_must_fail git commit -a --ignore-submodules=all -m Update sm
+'
+
+test_expect_success 'commit -a --ignore-submodules=none
Re: What is missing from Git v2.0
On Thu, Apr 24, 2014 at 09:41:06AM -0400, Theodore Ts'o wrote: On Thu, Apr 24, 2014 at 03:23:54AM -0500, Felipe Contreras wrote: Creating a ~/.gitconfig file if one doesn't already is one I agree with, and at least on Unix systems, telling them that the config file lives in ~/.gitconfig, or where ever it might happen to be on other platforms, is a good one. If it's in some really weird place on Windows, then sure, we can tell them about git config -e. But the point is to let the user look at the default .gitconfig file, where we can put in comments to help explain what is going on, and perhaps have links to web pages for more information. I think the idea of a commented gitconfig is a good solution. We could include default aliases commented, so that a new user would just have to uncomment them. That way, he will understand they are aliases and not commands, learn how to tune them to its own needs and it won't annoy anyone because they will be commented by default, ideally with explicit commentaries around them. Furthermore, this could be a good way to show a new user all the possibilities of git, or at least its configuration. Documentation is a good thing when you know what you are looking for, but when you are beginning, you don't know what can be done, and reading a complete and commented example configuration could be a good way to discover it. pgpbPbeQEGNjc.pgp Description: PGP signature
Re: [PATCH v5 5/9] patch-id: document new behaviour
On Thu, Apr 24, 2014 at 10:33:25AM -0700, Jonathan Nieder wrote: Michael S. Tsirkin wrote: Documentation/git-patch-id.txt | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) Ah, there's the documentation. Please squash this with the patch that introduces the new behavior so they can be reviewed together more easily (both now and later when people do archeology). [...] +--stable:: + Use a symmetrical sum of hashes as the patch ID. + With this option, reordering file diffs that make up a patch or + splitting a diff up to multiple diffs that touch the same path + does not affect the ID. + This is the default if patchid.stable is set to true. This doesn't explain to me why I would want to use --stable versus --unstable. Maybe an EXAMPLES section would help? The only reason I can think of to use --unstable is for compatibility with historical patch-ids. Is there any other reason? At this point in the series there is no patchid.stable configuration. +--unstable:: + Use a non-symmetrical sum of hashes, such that reordering What is a non-symmetrical sum? Non-symmetrical combination function is better? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 4/9] patch-id: make it stable against hunk reordering
On Thu, Apr 24, 2014 at 10:30:44AM -0700, Jonathan Nieder wrote: Hi, Michael S. Tsirkin wrote: Patch id changes if users 1. reorder file diffs that make up a patch or 2. split a patch up to multiple diffs that touch the same path (keeping hunks within a single diff ordered to make patch valid). As the result is functionally equivalent, a different patch id is surprising to many users. Hm. If the goal is that functionally equivalent patches are guaranteed to produce the same patch-id, I wonder if we should be doing something like the following: 1. apply the patch in memory 2. generate a new diff 3. use that new diff to produce a patch-id Otherwise issues like --diff-algorithm=patience versus =myers will create trouble too. I don't think that avoiding false negatives for patch comparison without doing something like that is really possible. On the other hand if someone reorders file diffs within a patch, that is a potentially very common thing to do and something worth fixing. In other words, while your (1) makes perfect sense to me, case (2) seems less convincing. I agree it's less convincing: one would have to edit patch by hand (which I used to sometimes do to make important parts more prominent, but stopped doing in favor of splitting a patch). I'm not 100% sure whether it's worth supporting or not. The downside of allowing reordering hunks is that it can potentially make different patches to be treated the same (for example if they were making similar changes to different functions) when the ordering previously caused them to be distinguished. But that wasn't something people could count on anyway, so I don't mind. I think this example convinces me. I'll drop this support in the next version. Should the internal patch-id computation used by commands like 'git cherry' (see diff.c::diff_get_patch_id) get the same change? (Not a rhetorical question --- I don't know what the right choice would be there.) [...] The new behaviour is enabled - when patchid.stable is true - when --stable flag is present Using a new flag --unstable or setting patchid.stable to false force the historical behaviour. Which is the default? [...] builtin/patch-id.c | 89 -- 1 file changed, 73 insertions(+), 16 deletions(-) Documentation? Tests? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
On Thu, Apr 24, 2014 at 12:29:21PM -0700, Junio C Hamano wrote:
David Kastrup d...@gnu.org writes:
Junio C Hamano gits...@pobox.com writes:
Traditionally, because the tool grew in a context of being used in a
project whose participants are at least not malicious, always having
to be on the lookout for fear of middle-of-line tabs hiding bad
contents near the right edges of lines has never been an issue.
My beef is not with hiding bad contents but with hiding contents.
It makes the output useless for seeing what is actually happening as
soon as the option starts having an effect.
My suspicion is that one of the reasons why S was chosen to be in
the default was to mildly discourage people from busting the usual
line-length limit, but I am not Linus ;-)
I would think it's the opposite. Long lines look _horrible_ without
-S, as they get wrapped at awkward points. Using -S means that long
lines don't bug you, unless you really want to scroll over and see the
content.
I really think the right solution here is to teach less to make it more
obvious that there is something worth scrolling over to. Here's a very
rough patch for less, if you want to see what I'm thinking of.
diff --git a/input.c b/input.c
index b211323..01aa411 100755
--- a/input.c
+++ b/input.c
@@ -178,6 +178,7 @@ get_forw_line:
*/
if (chopline || hshift 0)
{
+ set_chopped_marker(ch_tell()-1);
do
{
if (ABORT_SIGS())
diff --git a/line.c b/line.c
index 1eb3914..b3358a0 100755
--- a/line.c
+++ b/line.c
@@ -1080,6 +1080,20 @@ set_status_col(c)
attr[0] = AT_NORMAL|AT_HILITE;
}
+ public void
+set_chopped_marker(pos)
+ POSITION pos;
+{
+ /*
+* Roll back output by one character; probably
+* we need to actually walk curr back further
+* for multibyte characters?
+*/
+ column--;
+ curr--;
+ store_char('', AT_NORMAL|AT_HILITE, NULL, pos);
+}
+
/*
* Get a character from the current line.
* Return the character as the function return value,
-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 v5 3/9] tests: new test for orderfile options
On Thu, Apr 24, 2014 at 11:45:35AM -0700, Junio C Hamano wrote:
Michael S. Tsirkin m...@redhat.com writes:
The test is very basic and can be extended.
Couldn't find a good existing place to put it,
so created a new file.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
t/t4056-diff-order.sh | 63
+++
1 file changed, 63 insertions(+)
create mode 100755 t/t4056-diff-order.sh
diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
new file mode 100755
index 000..0404f50
Huh? What codebase is this based on?
I think we had t4056 since b5277730 (t4056: add new tests for git
diff -O, 2013-12-18).
Puzzled...
Yes I didn't rebase in a while: 7794a680e63a2a11b73cb1194653662f2769a792
was the tip.
I'll rebase, sorry.
--- /dev/null
+++ b/t/t4056-diff-order.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+test_description='diff orderfile'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ as=a a a a a a a a # eight a
+ test_write_lines $as foo
+ test_write_lines $as bar
+ git add foo bar
+ git commit -a -m initial
+ test_write_lines $as b foo
+ test_write_lines $as b bar
+ git commit -a -m first
+ test_write_lines bar foo bar-then-foo
+ test_write_lines foo bar foo-then-bar
+ git diff -Ofoo-then-bar HEAD~1..HEAD diff-foo-then-bar
+ git diff -Obar-then-foo HEAD~1..HEAD diff-bar-then-foo
+'
+
+test_diff_well_formed () {
+ grep ^+b $1 added
+ grep ^-b $1 removed
+ grep ^+++ $1 oldfiles
+ grep ^--- $1 newfiles
+ test_line_count = 2 added
+ test_line_count = 0 removed
+ test_line_count = 2 oldfiles
+ test_line_count = 2 newfiles
+}
+
+test_expect_success 'diff output with -O is well-formed' '
+ test_diff_well_formed diff-foo-then-bar
+ test_diff_well_formed diff-bar-then-foo
+'
+
+test_expect_success 'flag -O affects diff output' '
+ ! test_cmp diff-foo-then-bar diff-bar-then-foo
+'
+
+test_expect_success 'orderfile is same as -O' '
+ test_config diff.orderfile foo-then-bar
+ git diff HEAD~1..HEAD diff-foo-then-bar-config
+ test_config diff.orderfile bar-then-foo
+ git diff HEAD~1..HEAD diff-bar-then-foo-config
+ test_cmp diff-foo-then-bar diff-foo-then-bar-config
+ test_cmp diff-bar-then-foo diff-bar-then-foo-config
+'
+
+test_expect_success '-O overrides orderfile' '
+ test_config diff.orderfile foo-then-bar
+ git diff -Obar-then-foo HEAD~1..HEAD diff-bar-then-foo-flag
+ test_cmp diff-bar-then-foo diff-bar-then-foo-flag
+'
+
+test_expect_success '/dev/null is same as no orderfile' '
+ git diff -O/dev/null HEAD~1..HEADdiff-null-orderfile
+ git diff HEAD~1..HEAD diff-default
+ test_cmp diff-null-orderfile diff-default
+'
+
+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: Harmful LESS flags
Jeff King p...@peff.net writes: I would think it's the opposite. Long lines look _horrible_ without -S, as they get wrapped at awkward points. Using -S means that long lines don't bug you, unless you really want to scroll over and see the content. I really think the right solution here is to teach less to make it more obvious that there is something worth scrolling over to. Here's a very rough patch for less, if you want to see what I'm thinking of. Yes, I think that was suggested as an issue worth bringing up with less maintainers earlier in the thread already (and that was why I didn't repeat it). If we were in the business of updating less to suit many users' needs (the needs of our users included), we may even want to advocate turning R on by default. And I do agree that the chopped marker would be a very sensible thing to show in the -S output; I would have chosen $ myself for that to match an existing practice in (setq truncate-lines t) in Emacs, 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: Harmful LESS flags
On Thu, Apr 24, 2014 at 02:47:01PM -0700, Junio C Hamano wrote: And I do agree that the chopped marker would be a very sensible thing to show in the -S output; I would have chosen $ myself for that to match an existing practice in (setq truncate-lines t) in Emacs, though. Hmm. I do not use Emacs, but I explicitly avoided $ because of its end-of-line connotations. E.g., in cat -A it means the opposite: this is the real \n end-of-line. But if there's existing precedent for $, that would be fine with me. -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: Harmful LESS flags
Jeff King p...@peff.net writes: On Thu, Apr 24, 2014 at 12:29:21PM -0700, Junio C Hamano wrote: David Kastrup d...@gnu.org writes: Junio C Hamano gits...@pobox.com writes: Traditionally, because the tool grew in a context of being used in a project whose participants are at least not malicious, always having to be on the lookout for fear of middle-of-line tabs hiding bad contents near the right edges of lines has never been an issue. My beef is not with hiding bad contents but with hiding contents. It makes the output useless for seeing what is actually happening as soon as the option starts having an effect. My suspicion is that one of the reasons why S was chosen to be in the default was to mildly discourage people from busting the usual line-length limit, but I am not Linus ;-) I would think it's the opposite. Long lines look _horrible_ without -S, as they get wrapped at awkward points. Using -S means that long lines don't bug you, unless you really want to scroll over and see the content. I prefer horrible over useless. I really think the right solution here is to teach less to make it more obvious that there is something worth scrolling over to. Here's a very rough patch for less, if you want to see what I'm thinking of. Still useless. I'm not actually interested in a more prominent I could be useful indicator. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Harmful LESS flags
On Thu, Apr 24, 2014 at 11:48:30PM +0200, David Kastrup wrote: I really think the right solution here is to teach less to make it more obvious that there is something worth scrolling over to. Here's a very rough patch for less, if you want to see what I'm thinking of. Still useless. I'm not actually interested in a more prominent I could be useful indicator. So don't set -S, then. There are two questions here: 1. Can less do a better job of indicating what's in the input when -S is in effect? 2. What should get put into $LESS by default? I was specifically addressing (1). Your comment does not help at all there. It could have an impact on (2), but you didn't say anything besides I don't like it. That doesn't add anything to the conversation. -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 v5 5/9] patch-id: document new behaviour
Michael S. Tsirkin m...@redhat.com writes: +--unstable:: + Use a non-symmetrical sum of hashes, such that reordering What is a non-symmetrical sum? Non-symmetrical combination function is better? I do not think either is very good X-. The primary points to convey for --stable are: - Two patches produced by comparing the same two trees with two different settings for -Oorderfile will result in the same patchc signature, thereby allowing the computed result to be used as a key to index some metainformation about the change between the two trees; - It will produce a result different from the plain vanilla patch-id has always produced even when used on a diff output taken without any use of -Oorderfile, thereby making existing databases keyed by patch-ids unusable. The fact that we happened to use a patch-id that catches that somebody reordered the same patch into different file order and declares that they are two different changes is a more historical accident than a designed goal. I would even say that we would have used the stable version from the beginning if we thought that -Oorderfile would be widely used when these two features both appeared. Even though I was the guilty one who introduced it, I'd admit that -Oorderfile has merely been a curiosity from its inception and has been a failed experiment, not in the sense that the feature does not work as adverertised (it does), but in the sense that it is not widely used (evidenced by the lack of complaints on missing diff.orderfile for a long time) at all. With -Oorderfile being a failed experiment, the unstability did not matter, so it has stuck. The only two things worth mentioning about --unstable, if our future direction is to see diff.orderfile and --stable a lot more widely used, are: (1) it keeps producing the same patch-id as existing versions of Git, so users with existing databases (who do not deal with reordered patches) may want to use it; and perhaps (2) it will not consider a patch taken with -Oorderfile and another without it from the same source the same patches. Mathmatically speaking, mentioning non-symmetrial might be one way of expressing the latter point (2), but stressing on that alone without mentioning (1) misses the point. (2) is _not_ a designed feature, so it is not very interesting. Unless you have an existing database, there is no reason to use --unstable. On the other hand (1) is a very relevant thing to mention, as we are talking about a feature that, if unused, may break existing users' data. -- 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: Harmful LESS flags
Jeff King p...@peff.net writes: On Thu, Apr 24, 2014 at 11:48:30PM +0200, David Kastrup wrote: I really think the right solution here is to teach less to make it more obvious that there is something worth scrolling over to. Here's a very rough patch for less, if you want to see what I'm thinking of. Still useless. I'm not actually interested in a more prominent I could be useful indicator. So don't set -S, then. I don't. Git does it unasked for. There are two questions here: 1. Can less do a better job of indicating what's in the input when -S is in effect? 2. What should get put into $LESS by default? I was specifically addressing (1). Your comment does not help at all there. It could have an impact on (2), but you didn't say anything besides I don't like it. That doesn't add anything to the conversation. No, I said it is useless, which is different from I don't like it. The information is not copypastable from a terminal window since it is cut off. It is also useless for review since one does not actually know what's in there. The only thing it has going for it is that it's prettier than the actually usable information. Which might sometimes be nice if one is not interested overly in the payload, like when using --graph. But then even a graph display wants to get copypasted into windows with a different size from the terminal window, like in URL:http://code.google.com/p/lilypond/issues/detail?id=3723#c7. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 03/12] revert/cherry-pick: add --quiet option
Felipe Contreras wrote:
@@ -635,9 +637,10 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
}
if (opts-skip_empty is_index_unchanged() == 1) {
- warning(_(skipping %s... %s),
- find_unique_abbrev(commit-object.sha1,
DEFAULT_ABBREV),
- msg.subject);
+ if (!opts-quiet)
+ warning(_(skipping %s... %s),
+ find_unique_abbrev(commit-object.sha1,
DEFAULT_ABBREV),
+ msg.subject);
Personally, I don't see much value in inventing a new option for
suppressing one message.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 04/12] revert/cherry-pick: add --skip option
Felipe Contreras wrote: Akin to 'am --skip' and 'rebase --skip'. I don't recall why my original sequencer series didn't include this option. Perhaps Jonathan remembers? -- 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 v7 06/12] builtin: add rewrite helper
Felipe Contreras wrote: So that we can load and store rewrites, as well as other operations on a list of rewritten commits. Please elaborate. Explain why this code shouldn't go in sequencer.c. -- 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 v7 12/12] cherry-pick: copy notes and run hooks
Felipe Contreras wrote: If no action-name is specified, nothing is done. Why? Is it because git-rebase implements its own notes-copy-on-rewrite logic? Otherwise, I agree with the goal of making notes.rewrite.command work for cherry-pick. -- 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 v7 12/12] cherry-pick: copy notes and run hooks
Felipe Contreras wrote:
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 43631b4..fd085e1 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -248,7 +248,7 @@ pick_one () {
test -d $rewritten
pick_one_preserving_merges $@ return
- output eval git cherry-pick $strategy_args $empty_args $ff $@
+ output eval git cherry-pick --action-name '' $strategy_args
$empty_args $ff $@
Passing an empty action-name looks quite ugly. Is there a better way
to achieve this?
--
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: Harmful LESS flags
Hi, David Kastrup wrote: Jeff King p...@peff.net writes: There are two questions here: 1. Can less do a better job of indicating what's in the input when -S is in effect? 2. What should get put into $LESS by default? I was specifically addressing (1). Your comment does not help at all there. It could have an impact on (2), but you didn't say anything besides I don't like it. That doesn't add anything to the conversation. No, I said it is useless, which is different from I don't like it. The information is not copypastable from a terminal window since it is cut off. It is also useless for review since one does not actually know what's in there. The only thing it has going for it is that it's prettier than the actually usable information. I disagree with your characterization of what's useful here, but it really doesn't matter. Why are you still arguing? I think it would be fine to change git's default for LESS to FRX and document that change wherever the documentation currently mentions FRSX, if someone wants to write a patch for it. (Such a change would sit in pu or next until after 2.0.0 is released, of course.) In the meantime, when you're on machines using the current default, you have two choices: a) set the LESS envvar in your .profile explicitly b) hit the two keys '-', shift+S when git opens a pager The argument about safety is a red herring here, since it's always possible that a patch will wrap to make new lines with '+' or '-' or '@@' at the beginning that are equally confusing. Hoping that clarifies, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 03/12] revert/cherry-pick: add --quiet option
Ramkumar Ramachandra wrote:
Felipe Contreras wrote:
@@ -635,9 +637,10 @@ static int do_pick_commit(struct commit *commit,
struct replay_opts *opts)
}
if (opts-skip_empty is_index_unchanged() == 1) {
- warning(_(skipping %s... %s),
- find_unique_abbrev(commit-object.sha1,
DEFAULT_ABBREV),
- msg.subject);
+ if (!opts-quiet)
+ warning(_(skipping %s... %s),
+ find_unique_abbrev(commit-object.sha1,
DEFAULT_ABBREV),
+ msg.subject);
Personally, I don't see much value in inventing a new option for
suppressing one message.
It's not one message; it's two messages from cherry-pick itself, and all the
messages from `git commit`. Didn't we alread discuss this?
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 06/12] builtin: add rewrite helper
Ramkumar Ramachandra wrote: Felipe Contreras wrote: So that we can load and store rewrites, as well as other operations on a list of rewritten commits. Please elaborate. Explain why this code shouldn't go in sequencer.c. Isn't it obvious? Because sequencer.c wouldn't be the only user. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 12/12] cherry-pick: copy notes and run hooks
Ramkumar Ramachandra wrote: Felipe Contreras wrote: If no action-name is specified, nothing is done. Why? Is it because git-rebase implements its own notes-copy-on-rewrite logic? Yes, and `git rebase` uses `git cherry-pick`. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 12/12] cherry-pick: copy notes and run hooks
Ramkumar Ramachandra wrote:
Felipe Contreras wrote:
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 43631b4..fd085e1 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -248,7 +248,7 @@ pick_one () {
test -d $rewritten
pick_one_preserving_merges $@ return
- output eval git cherry-pick $strategy_args $empty_args $ff $@
+ output eval git cherry-pick --action-name '' $strategy_args
$empty_args $ff $@
Passing an empty action-name looks quite ugly. Is there a better way
to achieve this?
I want `git cherry-pick` to be able to do two things:
1) Omit the whole notes and hooks code
2) Specify a name other than cherry-pick to use for those
The --action-name argument achieves both.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is missing from Git v2.0
Felipe's === = The publish tracking branch = I still have problems getting upstream branches correctly configured as to have this introduced, anyway, I suppose it's optional, so nothing to add on that. By the way, remote branch managing has improved a lot, one of the best things I see for branching and remotes is the git remote show command, but I think further work should be done. Help messages FTW! = Reject non-fast-forward pulls by default = Not having this introduced yet allows newbie people to use git with just 4 commands, without bothering around with fetch and merge and so. = Use stage instead of index = Totally agree with this. = Default aliases = I hate aliases, make scripts more difficult to read and understand. I would instead try to improve knowledge on this feature. I have to agree with David Lang's first message, and The cherry-pick = pick thing would be the only thing I would see with good eyes, just because it's too long and has a dash. Juno's == The idea about ~/.gitconfig seems incredible simple and effective to me. I would however try to keep it simple, and minimize the form. Mine I have taught (or tried to) a lot of people Git. And this is some of the stuff I have seen they have difficulties with: - Remembering the commands, for example, remembering add, commit push and pull, which I think we can all agree is the most core and simple combination of Git commands. - What command comes for what they need. If I want to share everything, what should I do? - Most of them, have real difficulties on remembering the flows. There are too many commands for the start. I wouldn't nevertheless suppress any of them, I would rather do a tuto on-the-go. Here are some ideas I have thought of: == Command Output== At the moment, there are several commands that don't output any help text, and many others, that although they have become more verbose with the years, they aren't too verbose yet. One of the things I most recommend to anyone is to run git status just before any command (push, commit, add, etc.) to get sure they are doing what they thing they will. For example, running git add won't tell you what you just added, nor what you could do now. I would put some output there, maybe the git status output or something similar that helps the user to know what just happened. Git status doesn't say much about remotes, and suggesting pushing if a remote is outdated, would be fantastic. Checkout command has decreased verbosity from a previous version, where it stated which branch it came from and to which branch it was switching to. As an extreme thing, I would consider adding a configuration parameter default, core.helptext=True that could switch off all this stuff. ==Running git== This is a very basic idea, and I suppose it isn't too helpful or realistic, but might give someone an idea. I would first make that running git, just git, tell the user the possibilities he has. I don't know of any power user that uses git to remember the commands. At the moment, git[1] just tells many of the commands available, without any classification, maybe classifying them as commiting branching and remote could help a little. Regards, Javier Domingo Cansino [1] git output: javier@frodo:~$ git usage: git [--version] [--help] [-C path] [-c name=value] [--exec-path[=path]] [--html-path] [--man-path] [--info-path] [-p|--paginate|--no-pager] [--no-replace-objects] [--bare] [--git-dir=path] [--work-tree=path] [--namespace=name] command [args] The most commonly used git commands are: addAdd file contents to the index bisect Find by binary search the change that introduced a bug branch List, create, or delete branches checkout Checkout a branch or paths to the working tree clone Clone a repository into a new directory commit Record changes to the repository diff Show changes between commits, commit and working tree, etc fetch Download objects and refs from another repository grep Print lines matching a pattern init Create an empty Git repository or reinitialize an existing one logShow commit logs merge Join two or more development histories together mv Move or rename a file, a directory, or a symlink pull Fetch from and integrate with another repository or a local branch push Update remote refs along with associated objects rebase Forward-port local commits to the updated upstream head reset Reset current HEAD to the specified state rm Remove files from the working tree and from the index show Show various types of objects status Show the working tree status tagCreate, list, delete or verify a tag object signed with GPG 'git help -a' and 'git help -g' lists available subcommands and some concept guides. See 'git help
Re: What is missing from Git v2.0
Javier Domingo Cansino wrote: Felipe's === = The publish tracking branch = I still have problems getting upstream branches correctly configured as to have this introduced, anyway, I suppose it's optional, so nothing to add on that. I did so too, until I patch `git branch -v` to be useful. = Reject non-fast-forward pulls by default = Not having this introduced yet allows newbie people to use git with just 4 commands, without bothering around with fetch and merge and so. I don't understand what you are trying to say. There is no change for those people, when the pull fails they would be told to use `git pull --merge` if not sure. = Use stage instead of index = Totally agree with this. = Default aliases = I hate aliases, make scripts more difficult to read and understand. You are assuming that everyone will start to use aliases in scripts, which is not going to happen enough to be a problem. Try to find svn or hg scripts with aliases, let's see how many you find. Mine I have taught (or tried to) a lot of people Git. And this is some of the stuff I have seen they have difficulties with: - Remembering the commands, for example, remembering add, commit push and pull, which I think we can all agree is the most core and simple combination of Git commands. - What command comes for what they need. If I want to share everything, what should I do? - Most of them, have real difficulties on remembering the flows. There are too many commands for the start. I wouldn't nevertheless suppress any of them, I would rather do a tuto on-the-go. I think you are on the right track but the solution is not to shrug shoulders. We should acknowledge there are serious problems with the interface, list them, and try to fix them. One example is `git add $tracked_file` being wrong, it should be `git stage $tracked_file`. The real problem is that the core developers of Git don't acknowledge these user-interface issues, according the them the interface doesn't require any major changes. Which goes contrary to what most of the world believes. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-p4: format-patch to diff-tree change breaks binary patches
When applying binary patches a full index is required. format-patch already handles this, but diff-tree needs '--full-index' argument to always output full index. Signed-off-by: Tolga Ceylan tolga.cey...@gmail.com --- git-p4.py |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index cdfa2df..4ee6739 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1311,7 +1311,7 @@ class P4Submit(Command, P4UserMap): else: die(unknown modifier %s for %s % (modifier, path)) -diffcmd = git diff-tree -p \%s\ % (id) +diffcmd = git diff-tree --full-index -p \%s\ % (id) patchcmd = diffcmd + | git apply tryPatchCmd = patchcmd + --check - applyPatchCmd = patchcmd + --check --apply - -- 1.7.9.5 -- 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: Store refreshed stat info in a separate file?
On Sat, Apr 19, 2014 at 12:43 AM, Junio C Hamano gits...@pobox.com wrote: Having said that, I do not think there is a fundamental reason why the stat data has to live inside the same index file. A separate file is just fine, as long as you can reliably detect that they went out of sync for whatever reason (e.g. the index proper updated, a stale stat file left beind), and storing the trailer checksum from the corresponding index in this new file is an obvious and good solution. I've gone further and store index updates (including entry removals and additions) to the second index file so that index I/O cost is now proportional to the number of changed entries, not the work tree size (sort of). Which makes it scale much better when the work tree is huge. There is one flaw though. I'm expecting many yuck responses from people. So let's try to settle it now, or drop the idea. The idea is we can support another mode, where index content is stored in two files, the small $GIT_DIR/index and large $GIT_DIR/index.base. index contains changes that should be applied to index.base. Whenever you do something to the index, index records those actions. Git reads both index.base and index, then replay the action to have the final index in memory. index.base contains full worktree data and remains unchanged until index becomes too big/slow that changes should be merged back to index.base. This works great (my prototype passed the test suite), and even greater than index v5 because v5 still rewrites the whole index file when an entry is added or removed. But there is a problem with atomic update. The good old rename() does not work well with 2 files. This is not a problem with the C part, I can still make atomic update work. Scripts, on the other hand, may rely on mv or similar commands/functions to prepare a temp index and move it to $GIT_DIR/index. The workaround is merge back two files into a single index file so that scripts can mv $temp_index as before and pay the whole-index I/O penalty. An alternative is store two files in one, the one index file actually consists two subfiles. We avoid the atomic update problem, but we pay I/O cost for writing 10MB every time an index is updated (but not hashing 10MB file) and introduce a new index format. This is even yuckier in my opinion. Should I continue, or drop it? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] git tag --contains: avoid stack overflow
On Thu, Apr 24, 2014 at 02:24:39PM +0200, Stepan Kasal wrote: From: Jean-Jacques Lafay jeanjacques.la...@gmail.com In large repos, the recursion implementation of contains(commit, commit_list) may result in a stack overflow. Replace the recursion with a loop to fix it. This problem is more apparent on Windows than on Linux, where the stack is more limited by default. See also this thread on the msysGit list: https://groups.google.com/d/topic/msysgit/FqT6boJrb2g/discussion [jes: re-written to imitate the original recursion more closely] Thomas Braun pointed out several documentation shortcomings. Tests are run only if ulimit -s is available. This means they cannot be run on Windows. Signed-off-by: Jean-Jacques Lafay jeanjacques.la...@gmail.com Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de Tested-by: Stepan Kasal ka...@ucw.cz Thanks, this version looks good to me. -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
