Re: git log --no-walk --tags produces strange result for certain user
On 12/16/2013 12:52 PM, Kirill Likhodedov wrote: I received one more complaint for this issue, and now it appears in a public repository https://github.com/spray/spray To reproduce: # git clone https://github.com/spray/spray # cd spray # git log --no-walk --tags --pretty=%H %d --decorate=full | tail -3 3273edafcd9f9701d62e061c5257c0a09e2e1fb7 (tag: refs/tags/v0.8.0-RC1) ff3a2946bc54da76ddb47e82c81419cc7ae3db6b (tag: refs/tags/v0.7.0) 8b4043428b90b7f45b7241b3c2c032cf785479ce So here the last hash doesn't have a decoration. The problem is that reference refs/tags/v0.5.0 points at a tag object 8f6ca98087 which itself points at another tag object 2eddbcbff4 which finally points at commit 8b4043428b. Probably we should handle recursive tag objects like this, but OTOH I can't think of a reason why one would want to create them in the first place. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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
git quietly fails on https:// URL, https errors are never reported to user
On one of my FreeBSD systems I can't clone github through https URL. It only says Cloning into 'MyProject'..., writes some files, but then deletes everything, without printing anything else at all. Exit code is 128. Replacing https:// with git:// makes it work fine. While debugging, I find that remove_junk() deletes all directories from under __cxa_finalize. Before this, exit(128) is called from recvline_fh (Debug: Remote helper quit.) And this function in turn is called from under refs = transport_get_remote_refs(transport); I think you need to make sure that any https errors (in this and other locations) are reported to the user, and git never quits on error without saying what the error is. git-1.8.5.2 Yuri -- To unsubscribe from this list: send the line 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-p4: exception when cloning a perforce repository
dam...@iwi.me wrote on Wed, 15 Jan 2014 09:56 +0100: p4 fstat //depot/openssl/0.9.8j/openssl/include/openssl/bn.h@59702 ... depotFile //depot/openssl/0.9.8j/openssl/include/openssl/bn.h ... headAction edit ... headType symlink ... headTime 1237906419 ... headRev 2 ... headChange 59702 ... headModTime 1231329423 p4 print -q //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 | od -c 000 p4 print //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#1 //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#1 - add change 59574 (text) p4 print //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 - edit change 59702 (symlink) That's interesting. When I do the equivalent p4 print commands it shows something like this. arf-git-test$ p4 fstat //depot/bn.h ... depotFile //depot/bn.h ... clientFile /dev/shm/trash directory.t9802-git-p4-filetype/cli/bn.h ... isMapped ... headAction edit ... headType symlink ... headTime 1389876870 ... headRev 2 ... headChange 8 ... headModTime 1389876870 ... haveRev 2 arf-git-test$ p4 print //depot/bn.h#1 //depot/bn.h#1 - add change 7 (text) file-text arf-git-test$ p4 print //depot/bn.h#2 //depot/bn.h#2 - edit change 8 (symlink) /elsewhere/bn.h I don't know how you manage to get a symlink with an empty destination like that. I'll work on a way to hack around this failure. In the mean time, if you're game, it might be fun to see what p4 does with such a repository. You could make a client for just that little subdir, check out at 59702 and see what is there: mkdir testmess cd testmess cat EOF | p4 client -i Client: testmess Description: testmess Root: $(pwd) View: //depot/openssl/0.9.8j/openssl/include/openssl/... //testmess/... EOF then take a look at how p4 represents the empty symlink in the filesystem: p4 sync @59702 ls -la bn.h -- Pete -- To unsubscribe from this list: send the line 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-p4: exception when cloning a perforce repository
On 16 Jan 2014, at 14:08, Pete Wyckoff p...@padd.com wrote:
dam...@iwi.me wrote on Wed, 15 Jan 2014 09:56 +0100:
p4 fstat //depot/openssl/0.9.8j/openssl/include/openssl/bn.h@59702
... depotFile //depot/openssl/0.9.8j/openssl/include/openssl/bn.h
... headAction edit
... headType symlink
... headTime 1237906419
... headRev 2
... headChange 59702
... headModTime 1231329423
p4 print -q //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 | od -c
000
p4 print //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#1
//depot/openssl/0.9.8j/openssl/include/openssl/bn.h#1 - add change 59574
(text)
p4 print //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2
//depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 - edit change 59702
(symlink)
That's interesting. When I do the equivalent p4 print commands
it shows something like this.
arf-git-test$ p4 fstat //depot/bn.h
... depotFile //depot/bn.h
... clientFile /dev/shm/trash directory.t9802-git-p4-filetype/cli/bn.h
... isMapped
... headAction edit
... headType symlink
... headTime 1389876870
... headRev 2
... headChange 8
... headModTime 1389876870
... haveRev 2
arf-git-test$ p4 print //depot/bn.h#1
//depot/bn.h#1 - add change 7 (text)
file-text
arf-git-test$ p4 print //depot/bn.h#2
//depot/bn.h#2 - edit change 8 (symlink)
/elsewhere/bn.h
I don't know how you manage to get a symlink with an empty
destination like that.
I'll work on a way to hack around this failure. In the mean time,
if you're game, it might be fun to see what p4 does with such a
repository. You could make a client for just that little subdir,
check out at 59702 and see what is there:
mkdir testmess
cd testmess
cat EOF | p4 client -i
Client: testmess
Description: testmess
Root: $(pwd)
View: //depot/openssl/0.9.8j/openssl/include/openssl/... //testmess/...
EOF
then take a look at how p4 represents the empty symlink
in the filesystem:
p4 sync @59702
ls -la bn.h
I’ve tried exactly your commands, and I’ve got an empty folder..
{14:38}~/p4/testmess ➭ p4 sync @59702
//depot/openssl/0.9.8j/openssl/include/openssl/aes.h#2 - refreshing
/home/dgerard/p4/testmess/aes.h
//depot/openssl/0.9.8j/openssl/include/openssl/asn1.h#2 - refreshing
/home/dgerard/p4/testmess/asn1.h
//depot/openssl/0.9.8j/openssl/include/openssl/asn1_mac.h#2 - refreshing
/home/dgerard/p4/testmess/asn1_mac.h
//depot/openssl/0.9.8j/openssl/include/openssl/asn1t.h#2 - refreshing
/home/dgerard/p4/testmess/asn1t.h
//depot/openssl/0.9.8j/openssl/include/openssl/bio.h#2 - refreshing
/home/dgerard/p4/testmess/bio.h
//depot/openssl/0.9.8j/openssl/include/openssl/blowfish.h#2 - refreshing
/home/dgerard/p4/testmess/blowfish.h
//depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 - refreshing
/home/dgerard/p4/testmess/bn.h
//depot/openssl/0.9.8j/openssl/include/openssl/buffer.h#2 - refreshing
/home/dgerard/p4/testmess/buffer.h
[…]
{14:39}~/p4/testmess ➭ ls -la
total 12
drwxr-xr-x 2 dgerard dgerard 4096 janv. 16 14:37 .
drwxr-xr-x 4 dgerard dgerard 4096 janv. 16 14:34 ..
-rw-r--r-- 1 dgerard dgerard 93 janv. 16 14:37 .perforce
Then I tried to sync the previous changeset, which is ok :
{14:44}~/p4/testmess ➭ p4 sync -f @59701
//depot/openssl/0.9.8j/openssl/include/openssl/aes.h#1 - updating
/home/dgerard/p4/testmess/aes.h
[…]
{14:44}~/p4/testmess ➭ l
total 0
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 aes.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 asn1.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 asn1_mac.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 asn1t.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 bio.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 blowfish.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 bn.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 buffer.h
[…]
However, when trying to sync to the appropriate changeset :
{14:44}~/p4/testmess ➭ p4 sync -f @59702
//depot/openssl/0.9.8j/openssl/include/openssl/aes.h#2 - updating
/home/dgerard/p4/testmess/aes.h
rename: /home/dgerard/p4/testmess/aes.h: No such file or directory
//depot/openssl/0.9.8j/openssl/include/openssl/asn1.h#2 - updating
/home/dgerard/p4/testmess/asn1.h
rename: /home/dgerard/p4/testmess/asn1.h: No such file or directory
//depot/openssl/0.9.8j/openssl/include/openssl/asn1_mac.h#2 - updating
/home/dgerard/p4/testmess/asn1_mac.h
rename: /home/dgerard/p4/testmess/asn1_mac.h: No such file or directory
//depot/openssl/0.9.8j/openssl/include/openssl/asn1t.h#2 - updating
/home/dgerard/p4/testmess/asn1t.h
rename: /home/dgerard/p4/testmess/asn1t.h: No such file or directory
//depot/openssl/0.9.8j/openssl/include/openssl/bio.h#2 - updating
/home/dgerard/p4/testmess/bio.h
rename: /home/dgerard/p4/testmess/bio.h: No such file or directory
//depot/openssl/0.9.8j/openssl/include/openssl/blowfish.h#2 - updating
/home/dgerard/p4/testmess/blowfish.h
rename: /home/dgerard/p4/testmess/blowfish.h: No such file or directory
Re: git-p4: exception when cloning a perforce repository
dam...@iwi.me wrote on Thu, 16 Jan 2014 14:46 +0100:
On 16 Jan 2014, at 14:08, Pete Wyckoff p...@padd.com wrote:
dam...@iwi.me wrote on Wed, 15 Jan 2014 09:56 +0100:
p4 fstat //depot/openssl/0.9.8j/openssl/include/openssl/bn.h@59702
... depotFile //depot/openssl/0.9.8j/openssl/include/openssl/bn.h
... headAction edit
... headType symlink
... headTime 1237906419
... headRev 2
... headChange 59702
... headModTime 1231329423
p4 print -q //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 | od -c
000
p4 print //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#1
//depot/openssl/0.9.8j/openssl/include/openssl/bn.h#1 - add change 59574
(text)
p4 print //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2
//depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 - edit change 59702
(symlink)
That's interesting. When I do the equivalent p4 print commands
it shows something like this.
arf-git-test$ p4 fstat //depot/bn.h
... depotFile //depot/bn.h
... clientFile /dev/shm/trash directory.t9802-git-p4-filetype/cli/bn.h
... isMapped
... headAction edit
... headType symlink
... headTime 1389876870
... headRev 2
... headChange 8
... headModTime 1389876870
... haveRev 2
arf-git-test$ p4 print //depot/bn.h#1
//depot/bn.h#1 - add change 7 (text)
file-text
arf-git-test$ p4 print //depot/bn.h#2
//depot/bn.h#2 - edit change 8 (symlink)
/elsewhere/bn.h
I don't know how you manage to get a symlink with an empty
destination like that.
I'll work on a way to hack around this failure. In the mean time,
if you're game, it might be fun to see what p4 does with such a
repository. You could make a client for just that little subdir,
check out at 59702 and see what is there:
mkdir testmess
cd testmess
cat EOF | p4 client -i
Client: testmess
Description: testmess
Root: $(pwd)
View: //depot/openssl/0.9.8j/openssl/include/openssl/... //testmess/...
EOF
then take a look at how p4 represents the empty symlink
in the filesystem:
p4 sync @59702
ls -la bn.h
I’ve tried exactly your commands, and I’ve got an empty folder..
{14:38}~/p4/testmess ➭ p4 sync @59702
//depot/openssl/0.9.8j/openssl/include/openssl/aes.h#2 - refreshing
/home/dgerard/p4/testmess/aes.h
//depot/openssl/0.9.8j/openssl/include/openssl/asn1.h#2 - refreshing
/home/dgerard/p4/testmess/asn1.h
//depot/openssl/0.9.8j/openssl/include/openssl/asn1_mac.h#2 - refreshing
/home/dgerard/p4/testmess/asn1_mac.h
//depot/openssl/0.9.8j/openssl/include/openssl/asn1t.h#2 - refreshing
/home/dgerard/p4/testmess/asn1t.h
//depot/openssl/0.9.8j/openssl/include/openssl/bio.h#2 - refreshing
/home/dgerard/p4/testmess/bio.h
//depot/openssl/0.9.8j/openssl/include/openssl/blowfish.h#2 - refreshing
/home/dgerard/p4/testmess/blowfish.h
//depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 - refreshing
/home/dgerard/p4/testmess/bn.h
//depot/openssl/0.9.8j/openssl/include/openssl/buffer.h#2 - refreshing
/home/dgerard/p4/testmess/buffer.h
[…]
{14:39}~/p4/testmess ➭ ls -la
total 12
drwxr-xr-x 2 dgerard dgerard 4096 janv. 16 14:37 .
drwxr-xr-x 4 dgerard dgerard 4096 janv. 16 14:34 ..
-rw-r--r-- 1 dgerard dgerard 93 janv. 16 14:37 .perforce
Then I tried to sync the previous changeset, which is ok :
{14:44}~/p4/testmess ➭ p4 sync -f @59701
//depot/openssl/0.9.8j/openssl/include/openssl/aes.h#1 - updating
/home/dgerard/p4/testmess/aes.h
[…]
{14:44}~/p4/testmess ➭ l
total 0
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 aes.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 asn1.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 asn1_mac.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 asn1t.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 bio.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 blowfish.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 bn.h
-r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 buffer.h
[…]
However, when trying to sync to the appropriate changeset :
{14:44}~/p4/testmess ➭ p4 sync -f @59702
//depot/openssl/0.9.8j/openssl/include/openssl/aes.h#2 - updating
/home/dgerard/p4/testmess/aes.h
rename: /home/dgerard/p4/testmess/aes.h: No such file or directory
//depot/openssl/0.9.8j/openssl/include/openssl/asn1.h#2 - updating
/home/dgerard/p4/testmess/asn1.h
rename: /home/dgerard/p4/testmess/asn1.h: No such file or directory
//depot/openssl/0.9.8j/openssl/include/openssl/asn1_mac.h#2 - updating
/home/dgerard/p4/testmess/asn1_mac.h
rename: /home/dgerard/p4/testmess/asn1_mac.h: No such file or directory
//depot/openssl/0.9.8j/openssl/include/openssl/asn1t.h#2 - updating
/home/dgerard/p4/testmess/asn1t.h
rename: /home/dgerard/p4/testmess/asn1t.h: No such file or directory
//depot/openssl/0.9.8j/openssl/include/openssl/bio.h#2 - updating
/home/dgerard/p4/testmess/bio.h
rename: /home/dgerard/p4/testmess/bio.h: No such file or directory
Re: git-p4: exception when cloning a perforce repository
On 16 Jan 2014, at 15:45, Pete Wyckoff p...@padd.com wrote: Oh cool, that helps a lot. P4 is just broken here, so we can get away with being a bit sloppy in git. I'll try just pretending empty symlinks are not in the repo. Hopefully you'll have a future commit in your p4 repo that brings back bn.h properly. Thanks ! I would love to use git instead of perforce if possible :) Still not sure about how I'll test this. I can test for you, no probleme with that. Thanks, -- Pete -- To unsubscribe from this list: send the line 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-gui: fallback right pane to packed widgets with Tk 8.4
Hi All,
Max, you are right, my TK version is 8.4.
I applied the patch and it now works perfectly.
Thanks!!
Benoît
On Tue, Jan 14, 2014 at 5:58 PM, Max Kirillov m...@max630.net wrote:
Since 918dbf58, git-gui crashes if started with Tk 8.4. The reason is that
tk 8.5 does not support -stretch option for panedwindow.
Without the option it's not possible to properly expand the right half -
the commit area is expanded, while desired behavior is to expand the diff
area. So the whole feature should be disabled with Tk
version less than 8.5.
Signed-off-by: Max Kirillov m...@max630.net
---
git-gui/git-gui.sh | 32 +---
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index e2e710e..147be8c 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -3196,18 +3196,28 @@ unset i
# -- Diff and Commit Area
#
-${NS}::panedwindow .vpane.lower -orient vertical
-${NS}::frame .vpane.lower.commarea
-${NS}::frame .vpane.lower.diff -relief sunken -borderwidth 1 -height 500
-.vpane.lower add .vpane.lower.diff
-.vpane.lower add .vpane.lower.commarea
-.vpane add .vpane.lower
-if {$use_ttk} {
- .vpane.lower pane .vpane.lower.diff -weight 1
- .vpane.lower pane .vpane.lower.commarea -weight 0
+if {$have_tk85} {
+ ${NS}::panedwindow .vpane.lower -orient vertical
+ ${NS}::frame .vpane.lower.commarea
+ ${NS}::frame .vpane.lower.diff -relief sunken -borderwidth 1 -height
500
+ .vpane.lower add .vpane.lower.diff
+ .vpane.lower add .vpane.lower.commarea
+ .vpane add .vpane.lower
+ if {$use_ttk} {
+ .vpane.lower pane .vpane.lower.diff -weight 1
+ .vpane.lower pane .vpane.lower.commarea -weight 0
+ } else {
+ .vpane.lower paneconfigure .vpane.lower.diff -stretch always
+ .vpane.lower paneconfigure .vpane.lower.commarea -stretch
never
+ }
} else {
- .vpane.lower paneconfigure .vpane.lower.diff -stretch always
- .vpane.lower paneconfigure .vpane.lower.commarea -stretch never
+ frame .vpane.lower -height 300 -width 400
+ frame .vpane.lower.commarea
+ frame .vpane.lower.diff -relief sunken -borderwidth 1
+ pack .vpane.lower.diff -fill both -expand 1
+ pack .vpane.lower.commarea -side bottom -fill x
+ .vpane add .vpane.lower
+ .vpane paneconfigure .vpane.lower -sticky nsew
}
# -- Commit Area Buttons
--
1.8.4.2.1566.g3c1a064
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Rebase with remote branch?
I have two branches, 'master' and 'work'. 'master' I never work out of, and I merge changes into it from another SVN repository (I do not use git-svn, I do this all by hand as real git commits). My 'work' branch is technically my 'master' in the git-trunk sense, as that is my personal main line of development and I merge directly to 'master' from 'work'. I like to rebase 'work' onto 'master' but by doing so, I have to 'git push --force origin work' due to the fact that the rebase puts my current branch behind origin/work. Should I stick to merging between 'master' and 'work' only ? Rebase is only for local-only branches, right? -- To unsubscribe from this list: send the line 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 quietly fails on https:// URL, https errors are never reported to user
On Thu, Jan 16, 2014 at 04:27:03AM -0800, Yuri wrote: While debugging, I find that remove_junk() deletes all directories from under __cxa_finalize. Before this, exit(128) is called from recvline_fh (Debug: Remote helper quit.) And this function in turn is called from under refs = transport_get_remote_refs(transport); I think you need to make sure that any https errors (in this and other locations) are reported to the user, and git never quits on error without saying what the error is. We used to print Reading from helper 'git-remote-https' failed in this instance. But in the majority of cases, remote-https has printed a useful message already to stderr, and the extra line just confused people. The downside, as you noticed, is that when the helper dies without printing an error, the user is left with no message. Unfortunately, detecting whether the helper printed a useful message is non-trivial. It's possible we could do more detection based on the helper's death (e.g., if it died by signal, print a message) and at least say _something_. But even if we do so, the message isn't going to tell you what went wrong, only that something unexpected happened. It is up to the helper to print something useful, and if it didn't, it should be fixed. So the most important bug to fix here, IMHO, is figuring out why git-remote-https died without printing an error message. Is it possible to strace (or truss) the helper process? What it was doing when it died may be helpful. Rather than picking through strace -f output, you can use this hack to trace just the helper process: cat /in/your/$PATH/git-remote-strace \EOF #!/bin/sh protocol=$(echo $2 | cut -d: -f1) exec strace git-remote-$protocol $@ EOF git clone strace::https://github.com/your/repo.git -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 v4 1/6] submodule: Make 'checkout' update_module explicit
W. Trevor King wk...@tremily.us writes: This avoids the current awkwardness of having either '' or 'checkout' for checkout-mode updates, which makes testing for checkout-mode updates (or non-checkout-mode updates) easier. Signed-off-by: W. Trevor King wk...@tremily.us --- git-submodule.sh | 27 +++ 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 5247f78..5e8776c 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -803,17 +803,10 @@ cmd_update() update_module=$update else update_module=$(git config submodule.$name.update) - case $update_module in - '') - ;; # Unset update mode - checkout | rebase | merge | none) - ;; # Known update modes - !*) - ;; # Custom update command - *) - die $(eval_gettext Invalid update mode '$update_module' for submodule '$name') - ;; - esac + if test -z $update_module + then + update_module=checkout + fi fi Is this a good change? It removes the code to prevent a broken configuration value from slipping through. The code used to stop early to give the user a chance to fix it before actually letting submodule update to go into the time consuming part, e.g. a call to module_clone, but that code is now lost. displaypath=$(relative_path $prefix$sm_path) @@ -882,11 +875,16 @@ Maybe you want to use 'update --init'?) case ;$cloned_modules; in *;$name;*) # then there is no local change to integrate - update_module= ;; + update_module=checkout ;; esac must_die_on_failure= case $update_module in + checkout) + command=git checkout $subforce -q + die_msg=$(eval_gettext Unable to checkout '\$sha1' in submodule path '\$displaypath') + say_msg=$(eval_gettext Submodule path '\$displaypath': checked out '\$sha1') + ;; ... *) - command=git checkout $subforce -q - die_msg=$(eval_gettext Unable to checkout '\$sha1' in submodule path '\$displaypath') - say_msg=$(eval_gettext Submodule path '\$displaypath': checked out '\$sha1') - ;; + die $(eval_gettext Invalid update mode '$update_module' for submodule '$name') esac These two hunks make sense. It is clear in the updated code that checkout is what is implemented with that git checkout $subforce -q, and not any other random update mode. -- To unsubscribe from this list: send the line 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 3/6] submodule: Explicit local branch creation in module_clone
W. Trevor King wk...@tremily.us writes:
The previous code only checked out branches in cmd_add. This commit
moves the branch-checkout logic into module_clone, where it can be
shared by cmd_add and cmd_update. I also update the initial checkout
command to use 'reset' to preserve branches setup during module_clone.
With this change, folks cloning submodules for the first time via:
$ git submodule update ...
will get a local branch instead of a detached HEAD, unless they are
using the default checkout-mode updates. This is a change from the
previous situation where cmd_update always used checkout-mode logic
(regardless of the requested update mode) for updates that triggered
an initial clone, which always resulted in a detached HEAD.
This commit does not change the logic for updates after the initial
clone, which will continue to create detached HEADs for checkout-mode
updates, and integrate remote work with the local HEAD (detached or
not) in other modes.
The motivation for the change is that developers doing local work
inside the submodule are likely to select a non-checkout-mode for
updates so their local work is integrated with upstream work.
Developers who are not doing local submodule work stick with
checkout-mode updates so any apparently local work is blown away
during updates. For example, if upstream rolls back the remote branch
or gitlinked commit to an earlier version, the checkout-mode developer
wants their old submodule checkout to be rolled back as well, instead
of getting a no-op merge/rebase with the rolled-back reference.
By using the update mode to distinguish submodule developers from
black-box submodule consumers, we can setup local branches for the
developers who will want local branches, and stick with detached HEADs
for the developers that don't care.
Signed-off-by: W. Trevor King wk...@tremily.us
---
git-submodule.sh | 53 -
1 file changed, 36 insertions(+), 17 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 68dcbe1..4a09951 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -246,6 +246,9 @@ module_name()
# $3 = URL to clone
# $4 = reference repository to reuse (empty for independent)
# $5 = depth argument for shallow clones (empty for deep)
+# $6 = (remote-tracking) starting point for the local branch (empty for HEAD)
+# $7 = local branch to create (empty for a detached HEAD, unless $6 is
+# also empty, in which case the local branch is left unchanged)
#
# Prior to calling, cmd_update checks that a possibly existing
# path is not a git repository.
@@ -259,6 +262,8 @@ module_clone()
url=$3
reference=$4
depth=$5
+ start_point=$6
+ local_branch=$7
quiet=
if test -n $GIT_QUIET
then
@@ -312,7 +317,16 @@ module_clone()
echo gitdir: $rel/$a $sm_path/.git
rel=$(echo $a | sed -e 's|[^/][^/]*|..|g')
- (clear_local_git_env; cd $sm_path GIT_WORK_TREE=. git config
core.worktree $rel/$b)
+ (
+ clear_local_git_env
+ cd $sm_path
+ GIT_WORK_TREE=. git config core.worktree $rel/$b
+ # ash fails to wordsplit ${local_branch:+-B $local_branch...}
Interesting...
+ case $local_branch in
+ '') git checkout -f -q ${start_point:+$start_point} ;;
+ ?*) git checkout -f -q -B $local_branch
${start_point:+$start_point} ;;
+ esac
I am wondering if it makes more sense if you did this instead:
git checkout -f -q ${start_point:+$start_point}
if test -n $local_branch
then
git checkout -B $local_branch HEAD
fi
The optional re-attaching to the local_branch done with the second
checkout would be a non-destructive no-op to the working tree and
to the index, but it does distinguish between creating the branch
anew and resetting the existing branch in its output (note that
there is no -q to squelch it). By doing it this way, when we
later teach git branch -f and git checkout -B to report more
about what commits have been lost by such a resetting, you will get
the safety for free if you made the switching with -B run without
-q here.
+ ) || die $(eval_gettext Unable to setup cloned submodule
'\$sm_path')
}
isnumber()
@@ -475,16 +489,14 @@ Use -f if you really want to add it. 2
echo $(eval_gettext Reactivating local git
directory for submodule '\$sm_name'.)
fi
fi
- module_clone $sm_path $sm_name $realrepo $reference
$depth || exit
- (
- clear_local_git_env
- cd $sm_path
- # ash fails to wordsplit ${branch:+-b $branch...}
- case $branch in
- '') git checkout -f -q ;;
- ?*) git checkout -f -q -B
Re: [PATCH v4 1/6] submodule: Make 'checkout' update_module explicit
On Thu, Jan 16, 2014 at 10:46:36AM -0800, Junio C Hamano wrote: W. Trevor King wk...@tremily.us writes: @@ -803,17 +803,10 @@ cmd_update() update_module=$update else update_module=$(git config submodule.$name.update) - case $update_module in - '') - ;; # Unset update mode - checkout | rebase | merge | none) - ;; # Known update modes - !*) - ;; # Custom update command - *) - die $(eval_gettext Invalid update mode '$update_module' for submodule '$name') - ;; - esac + if test -z $update_module + then + update_module=checkout + fi fi Is this a good change? It removes the code to prevent a broken configuration value from slipping through. The code used to stop early to give the user a chance to fix it before actually letting submodule update to go into the time consuming part, e.g. a call to module_clone, but that code is now lost. Avoiding useless clones is probably more important than avoiding duplicate Invalid update mode messages. I'll reinstate the case statement in v5, but I think it should live outside of the “load from config” block, in case someone adds the ability to set arbitrary update modes from the command line (`--update merge`, `--update '!command'`, etc.). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 4/6] t7406: Just-cloned checkouts update to the gitlinked hash with 'reset'
W. Trevor King wk...@tremily.us writes: To preserve the local branch, for situations where we're not on a detached HEAD. Signed-off-by: W. Trevor King wk...@tremily.us --- This should be a part of some other change that actually changes how this git submodule update checks out the submodule, no? t/t7406-submodule-update.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 0825a92..5aa9591 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -703,7 +703,7 @@ test_expect_success 'submodule update places git-dir in superprojects git-dir re git clone super_update_r super_update_r2 (cd super_update_r2 git submodule update --init --recursive actual - test_i18ngrep Submodule path .submodule/subsubmodule.: checked out actual + test_i18ngrep Submodule path .submodule/subsubmodule.: .git reset --hard -q actual (cd submodule/subsubmodule git log ../../expected ) -- To unsubscribe from this list: send the line 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 quietly fails on https:// URL, https errors are never reported to user
On 01/16/2014 10:03, Jeff King wrote: We used to print Reading from helper 'git-remote-https' failed in this instance. But in the majority of cases, remote-https has printed a useful message already to stderr, and the extra line just confused people. The downside, as you noticed, is that when the helper dies without printing an error, the user is left with no message. In my case it was some random misconfiguration of the libraries handling https that caused them to fail in some way silently. Full update of the supporting libraries fixed this. But previous forced git update didn't fix it. I think your should bring back this printout. Errors only happen in a very low percentage of cases, and printing some more would be very helpful for troubleshooting. I am not sure what happened in git-remote-https, stream may have looked legit to it, or maybe it got some error from the supporting libraries and didn't report it. I can't tell now because I updated and the problem is gone. Yuri -- To unsubscribe from this list: send the line 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 3/6] submodule: Explicit local branch creation in module_clone
On Thu, Jan 16, 2014 at 11:18:00AM -0800, Junio C Hamano wrote:
W. Trevor King wk...@tremily.us writes:
@@ -312,7 +317,16 @@ module_clone()
echo gitdir: $rel/$a $sm_path/.git
rel=$(echo $a | sed -e 's|[^/][^/]*|..|g')
- (clear_local_git_env; cd $sm_path GIT_WORK_TREE=. git config
core.worktree $rel/$b)
+ (
+ clear_local_git_env
+ cd $sm_path
+ GIT_WORK_TREE=. git config core.worktree $rel/$b
+ # ash fails to wordsplit ${local_branch:+-B $local_branch...}
Interesting...
That's copied out of the old cmd_add code ;). I don't have ash
intalled to actually confirm the comment.
+ case $local_branch in
+ '') git checkout -f -q ${start_point:+$start_point} ;;
+ ?*) git checkout -f -q -B $local_branch
${start_point:+$start_point} ;;
+ esac
I am wondering if it makes more sense if you did this instead:
git checkout -f -q ${start_point:+$start_point}
if test -n $local_branch
then
git checkout -B $local_branch HEAD
fi
The optional re-attaching to the local_branch done with the second
checkout would be a non-destructive no-op to the working tree and
to the index, but it does distinguish between creating the branch
anew and resetting the existing branch in its output (note that
there is no -q to squelch it). By doing it this way, when we
later teach git branch -f and git checkout -B to report more
about what commits have been lost by such a resetting, you will get
the safety for free if you made the switching with -B run without
-q here.
This is immediately post-non-checkout-clone. There are no local
branches to clobber yet.
@@ -475,16 +489,14 @@ Use -f if you really want to add it. 2
echo $(eval_gettext Reactivating local git
directory for submodule '\$sm_name'.)
fi
fi
- module_clone $sm_path $sm_name $realrepo $reference
$depth || exit
- (
- clear_local_git_env
- cd $sm_path
- # ash fails to wordsplit ${branch:+-b $branch...}
- case $branch in
- '') git checkout -f -q ;;
- ?*) git checkout -f -q -B $branch origin/$branch ;;
- esac
- ) || die $(eval_gettext Unable to checkout submodule
'\$sm_path')
+ if test -n $branch
+ then
+ start_point=origin/$branch
+ local_branch=$branch
+ else
+ start_point=
+ fi
I'd feel safer if the else clause explicitly cleared $local_branch
by assigning an empty string to it; it would make it a lot clearer
that when $branch is an empty string here, we do not want to
trigger the new codepath to run checkout with -B $local_branch in
module_clone is what you mean.
Ok. Will add in v5.
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v4 4/6] t7406: Just-cloned checkouts update to the gitlinked hash with 'reset'
On Thu, Jan 16, 2014 at 11:22:52AM -0800, Junio C Hamano wrote: W. Trevor King wk...@tremily.us writes: To preserve the local branch, for situations where we're not on a detached HEAD. Signed-off-by: W. Trevor King wk...@tremily.us --- This should be a part of some other change that actually changes how this git submodule update checks out the submodule, no? Sure, we can squash both this test fix and the subsequent new test patch into patch #3 in v5. I was just splitting them out because backwards compatibility was a concern, and separate patches makes it easy for me to explain why the results changed here without getting lost in patch #3's implementation details. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 3/6] submodule: Explicit local branch creation in module_clone
W. Trevor King wk...@tremily.us writes: @@ -817,11 +831,15 @@ cmd_update() displaypath=$(relative_path $prefix$sm_path) - if test $update_module = none - then + case $update_module in + none) echo Skipping submodule '$displaypath' continue - fi + ;; + checkout) + local_branch= + ;; + esac I wonder if there is a way to avoid detaching (and you may need to update the coddpath that resets the submodule to the commit specified by the superproject tree) when it is safe to do so. For an end user, running submodule update is similar to running git pull in a project that does not use submodules, expressing I want to catch up with the work done by others. In a working tree with local changes, we do allow you to run git pull as long as your local changes do not overlap with the work done by others, and the result of the pull would look as if you did not have any of the local changes when you ran git pull and then you did the local changes on top of the state that is up-to-date with their work. Can't we design submodule update --checkout to work in a similar fashion? The updated superproject may say it wants $oSHA-1 at a submodule path P, and also its .gitmodules may say that commit is supposed to be at the tip of branch B=submodule.P.branch in the submodule repository. You may locally have advanced that branch in your submodule repository in the meantime to point at $ySHA-1 while others worked in the superproject and the submodule, and the difference $oSHA-1...$ySHA-1 can be considered as the local change made by you from the perspective of the superproject. Without thinking things through, if $ySHA-1 matches or is a descendant of $oSHA-1 (assuming that remote-tracking branch origin/$B in the submodule does point at $oSHA-1 in either case), it should be safe to do this update. And in a situation where you cannot do the checkout safely, it is perfectly fine to say the submodules X and Y have local changes; you cannot do 'submodule update' until you upstream them and fail the update, just like we fail a 'git pull' saying you cannot do pull until you commit them, no? Perhaps that kind of 'git submodule update' is parallel to 'git pull' in the project without submodules is better done with other update modes like --rebase or --merge. If so, how should we explain what 'submodule update --checkout' is to the end users? Is it supposed to be like git fetch git checkout origin/master? -- To unsubscribe from this list: send the line 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 1/6] submodule: Make 'checkout' update_module explicit
2014/1/16 W. Trevor King wk...@tremily.us: Avoiding useless clones is probably more important than avoiding duplicate Invalid update mode messages. No, it's not duplicate code. I'll explain, please follow me: @@ -803,17 +803,10 @@ cmd_update() update_module=$update else update_module=$(git config submodule.$name.update) - case $update_module in - '') - ;; # Unset update mode - checkout | rebase | merge | none) - ;; # Known update modes - !*) - ;; # Custom update command - *) - die $(eval_gettext Invalid update mode '$update_module' for submodule '$name') - ;; - esac This is a *validation*. It's done before going more through the code and die early. *) + die $(eval_gettext Invalid update mode '$update_module' for submodule '$name') This should be an *assert* - it means if you reach this case statement you (programmer) have messed the code something in the code before. In fact in my original patch I wrote something like invalid update_module at this flow. Please keep both as Junio said. Thanks, Francesco -- To unsubscribe from this list: send the line 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 1/6] submodule: Make 'checkout' update_module explicit
On Thu, Jan 16, 2014 at 09:07:22PM +0100, Francesco Pretto wrote: 2014/1/16 W. Trevor King wk...@tremily.us: Avoiding useless clones is probably more important than avoiding duplicate Invalid update mode messages. No, it's not duplicate code. I meant “duplicating the Invalid update mode error message”. I missed the die-early distinction, but I understand now. I think its non-DRY to have an early case statement to validate the update_module, and a later case statement to use it. Still, keeping those separate statements in sync shouldn't be too hard ;). Please keep both as Junio said. That's what I said I'd do in the email you're quoting ;). Are you ok with the die-early validation checking all update_module settings instead of just checking the loaded-from config branch? Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 6/6] Documentation: Describe 'submodule update' modes in detail
W. Trevor King wk...@tremily.us writes: The old documentation did not distinguish between cloning and non-cloning updates and lacked clarity on which operations would lead to detached HEADs, and which would not. The new documentation addresses these issues while updating the docs to reflect the changes introduced by this branch's explicit local branch creation in module_clone. I also add '--checkout' to the usage summary and group the update-mode options into a single set. Signed-off-by: W. Trevor King wk...@tremily.us --- Documentation/git-submodule.txt | 36 +++- Documentation/gitmodules.txt| 4 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index bfef8a0..02500b4 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -15,8 +15,8 @@ SYNOPSIS 'git submodule' [--quiet] init [--] [path...] 'git submodule' [--quiet] deinit [-f|--force] [--] path... 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] - [-f|--force] [--rebase] [--reference repository] [--depth depth] - [--merge] [--recursive] [--] [path...] + [-f|--force] [--rebase|--merge|--checkout] [--reference repository] + [--depth depth] [--recursive] [--] [path...] 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n] [commit] [--] [path...] 'git submodule' [--quiet] foreach [--recursive] command @@ -155,13 +155,31 @@ it contains local modifications. update:: Update the registered submodules, i.e. clone missing submodules and - checkout the commit specified in the index of the containing repository. - This will make the submodules HEAD be detached unless `--rebase` or - `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying - `--checkout`. Setting the key `submodule.$name.update` to `!command` - will cause `command` to be run. `command` can be any arbitrary shell - command that takes a single argument, namely the sha1 to update to. + checkout the commit specified in the index of the containing + repository. The update mode defaults to 'checkout', but be + configured with the 'submodule.name.update' setting or the + '--rebase', '--merge', or 'checkout' options. Not '--checkout'? Other than that, the updated text above is far easier to understand. Good job. ++ +For updates that clone missing submodules, checkout-mode updates will +create submodules with detached HEADs; all other modes will create +submodules with a local branch named after 'submodule.path.branch'. ++ +For updates that do not clone missing submodules, the submodule's HEAD That is, updates that update submodules that are already checked out? +is only touched when the remote reference does not match the +submodule's HEAD (for none-mode updates, the submodule is never +touched). The remote reference is usually the gitlinked commit from +the superproject's tree, but with '--remote' it is the upstream +subproject's 'submodule.name.branch'. This remote reference is +integrated with the submodule's HEAD using the specified update mode. I think copying some motivation from the log message of 06b1abb5 (submodule update: add --remote for submodule's upstream changes, 2012-12-19) would help the readers here. A naïve expectation from a casual reader of the above would be The superproject's tree ought to point at the same commit as the tip of the branch used in the submodule (modulo mirroring delays and somesuch), if the repository of the superproject and submodules are maintained properly, which would lead to when would any sane person need to use --remote in the first place???. If I am reading 06b1abb5 correctly, the primary motivation behind --remote seems to be that it is exactly to help the person who wants to update superproject to satisify the ... are maintained properly part by fetching the latest in each of the submodules in his superproject in preparation to 'git add .' them. I still do not think --remote was a better way than the foreach, but that is a separate topic. If the person who works in the superproject does not control the progress of, and/or does not care what development is happening in, the submodules, he can push the superproject tree out without even bothering to update the commits in the submodules bound to his superproject tree, and the consumers of such a superproject could catch up with the advancement of submodule by using --remote individually to bring themselves up to date. But I do not think that is what you envisioned as the best recommended practice when you wrote 06b1abb5. +For checkout-mode updates, that will result in a detached HEAD. For +rebase- and merge-mode updates, the commit referenced by
Re: [PATCH v4 4/6] t7406: Just-cloned checkouts update to the gitlinked hash with 'reset'
W. Trevor King wk...@tremily.us writes: On Thu, Jan 16, 2014 at 11:22:52AM -0800, Junio C Hamano wrote: W. Trevor King wk...@tremily.us writes: To preserve the local branch, for situations where we're not on a detached HEAD. Signed-off-by: W. Trevor King wk...@tremily.us --- This should be a part of some other change that actually changes how this git submodule update checks out the submodule, no? Sure, we can squash both this test fix and the subsequent new test patch into patch #3 in v5. I was just splitting them out because backwards compatibility was a concern, and separate patches makes it easy for me to explain why the results changed here without getting lost in patch #3's implementation details. On the contrary, if we had this as part of patch #3, it would have helped reviewing the patch itself, as that would have served as one more clue to illustrate the effect that is externally visible to end users. Besides, having them separate will break bisection. -- To unsubscribe from this list: send the line 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 6/6] Documentation: Describe 'submodule update' modes in detail
On Thu, Jan 16, 2014 at 12:21:04PM -0800, Junio C Hamano wrote: W. Trevor King wk...@tremily.us writes: @@ -155,13 +155,31 @@ it contains local modifications. update:: Update the registered submodules, i.e. clone missing submodules and - checkout the commit specified in the index of the containing repository. - This will make the submodules HEAD be detached unless `--rebase` or - `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying - `--checkout`. Setting the key `submodule.$name.update` to `!command` - will cause `command` to be run. `command` can be any arbitrary shell - command that takes a single argument, namely the sha1 to update to. + checkout the commit specified in the index of the containing + repository. The update mode defaults to 'checkout', but be + configured with the 'submodule.name.update' setting or the + '--rebase', '--merge', or 'checkout' options. Not '--checkout'? Oops, will fix in v5. ++ +For updates that clone missing submodules, checkout-mode updates will +create submodules with detached HEADs; all other modes will create +submodules with a local branch named after 'submodule.path.branch'. ++ +For updates that do not clone missing submodules, the submodule's HEAD That is, updates that update submodules that are already checked out? It's submodules for which $sm_path/.git does not exist. +is only touched when the remote reference does not match the +submodule's HEAD (for none-mode updates, the submodule is never +touched). The remote reference is usually the gitlinked commit from +the superproject's tree, but with '--remote' it is the upstream +subproject's 'submodule.name.branch'. This remote reference is +integrated with the submodule's HEAD using the specified update mode. I think copying some motivation from the log message of 06b1abb5 (submodule update: add --remote for submodule's upstream changes, 2012-12-19) would help the readers here. I think a brief reference to --remote is best here, mentioning that it has something to do with the upstream subproject. More detail on when you should use --remote should probably go under the docs for --remote ;). A naïve expectation from a casual reader of the above would be The superproject's tree ought to point at the same commit as the tip of the branch used in the submodule (modulo mirroring delays and somesuch), What is the branch used in the submodule? The remote subproject's current submodule.name.branch? The local submodule's submodule.name.branch (or localBranch) branch? The submodule's current HEAD? if the repository of the superproject and submodules are maintained properly, which would lead to when would any sane person need to use --remote in the first place???. --remote is not for sane people (who will probably be pulling from withing the submodule itself). --remote is for folks who track too many submodules to care about them as individuals, who just want to blindly update to whatever the upstream subproject maintainer has in submodule.name.branch. For example, if you are a distribution with a hundred submodules tracking all the projects you package, and want to bump them all to a their current trunk tip in one fell swoop. If I am reading 06b1abb5 correctly, the primary motivation behind --remote seems to be that it is exactly to help the person who wants to update superproject to satisify the ... are maintained properly part by fetching the latest in each of the submodules in his superproject in preparation to 'git add .' them. I still do not think --remote was a better way than the foreach, but that is a separate topic. I agree now ;), the problems with: $ git submodule foreach 'git pull' are: 1. You may not be on the “right” local branch to begin with, and 2. You may not have out-of-tree submodule configs setting up the “right” upstream for that branch. 06b1abb did not address the former, and added a new .gitmodules-level submodule.name.branch to help with the latter. I now think all of this would be easier if we had automatic submodule local-branch checkouts (fixing problem 1) and synchronized out-of-tree submodule configs (fixing problem 2). Fixing problem 1 is all you need if you aren't interested in sharing your out-of-tree configs. I think 06b1abb was inspired by “we already have a way to integrate changes in the gitlinked commit, we should reuse those to integrate other changes”. In hindsight, changes in the gitlinked commit are really a checkout-time issue, while changes in other places (like the remote subproject) are pull-time issues. Mixing them together was more confusing than it was worth. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description:
Re: [PATCH v4 6/6] Documentation: Describe 'submodule update' modes in detail
On Thu, Jan 16, 2014 at 12:55:21PM -0800, W. Trevor King wrote: On Thu, Jan 16, 2014 at 12:21:04PM -0800, Junio C Hamano wrote: W. Trevor King wk...@tremily.us writes: @@ -155,13 +155,31 @@ it contains local modifications. update:: Update the registered submodules, i.e. clone missing submodules and - checkout the commit specified in the index of the containing repository. - This will make the submodules HEAD be detached unless `--rebase` or - `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying - `--checkout`. Setting the key `submodule.$name.update` to `!command` - will cause `command` to be run. `command` can be any arbitrary shell - command that takes a single argument, namely the sha1 to update to. + checkout the commit specified in the index of the containing + repository. The update mode defaults to 'checkout', but be + configured with the 'submodule.name.update' setting or the + '--rebase', '--merge', or 'checkout' options. Not '--checkout'? Oops, will fix in v5. Shouldn't this also be `--checkout` (backticks), according to CodingGuidelines. This applies to all this options in this patch I think. ++ +For updates that clone missing submodules, checkout-mode updates will +create submodules with detached HEADs; all other modes will create +submodules with a local branch named after 'submodule.path.branch'. ++ +For updates that do not clone missing submodules, the submodule's HEAD That is, updates that update submodules that are already checked out? It's submodules for which $sm_path/.git does not exist. +is only touched when the remote reference does not match the +submodule's HEAD (for none-mode updates, the submodule is never +touched). The remote reference is usually the gitlinked commit from +the superproject's tree, but with '--remote' it is the upstream +subproject's 'submodule.name.branch'. This remote reference is +integrated with the submodule's HEAD using the specified update mode. I think copying some motivation from the log message of 06b1abb5 (submodule update: add --remote for submodule's upstream changes, 2012-12-19) would help the readers here. I think a brief reference to --remote is best here, mentioning that it has something to do with the upstream subproject. More detail on when you should use --remote should probably go under the docs for --remote ;). A naïve expectation from a casual reader of the above would be The superproject's tree ought to point at the same commit as the tip of the branch used in the submodule (modulo mirroring delays and somesuch), What is the branch used in the submodule? The remote subproject's current submodule.name.branch? The local submodule's submodule.name.branch (or localBranch) branch? The submodule's current HEAD? if the repository of the superproject and submodules are maintained properly, which would lead to when would any sane person need to use --remote in the first place???. --remote is not for sane people (who will probably be pulling from withing the submodule itself). --remote is for folks who track too many submodules to care about them as individuals, who just want to blindly update to whatever the upstream subproject maintainer has in submodule.name.branch. For example, if you are a distribution with a hundred submodules tracking all the projects you package, and want to bump them all to a their current trunk tip in one fell swoop. If I am reading 06b1abb5 correctly, the primary motivation behind --remote seems to be that it is exactly to help the person who wants to update superproject to satisify the ... are maintained properly part by fetching the latest in each of the submodules in his superproject in preparation to 'git add .' them. I still do not think --remote was a better way than the foreach, but that is a separate topic. I agree now ;), the problems with: $ git submodule foreach 'git pull' are: 1. You may not be on the “right” local branch to begin with, and 2. You may not have out-of-tree submodule configs setting up the “right” upstream for that branch. 06b1abb did not address the former, and added a new .gitmodules-level submodule.name.branch to help with the latter. I now think all of this would be easier if we had automatic submodule local-branch checkouts (fixing problem 1) and synchronized out-of-tree submodule configs (fixing problem 2). Fixing problem 1 is all you need if you aren't interested in sharing your out-of-tree configs. I think 06b1abb was inspired by “we already have a way to integrate changes in the gitlinked commit, we should reuse those to integrate other changes”. In hindsight, changes in the gitlinked commit are really a checkout-time issue, while changes in other places (like the remote subproject)
What's cooking in git.git (Jan 2014, #03; Thu, 16)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
I am planning to tag 1.9-rc0 preview release from the tip of
'master' soonish. Hopefully a few fix-up topics still in flight and
also updates to git-gui, gitk, git-svn, i18n, etc. from respective
area maintainers can be merged by the time 1.9-rc1 will be tagged.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[Graduated to master]
* br/sha1-name-40-hex-no-disambiguation (2014-01-07) 1 commit
(merged to 'next' on 2014-01-10 at 2a0973b)
+ sha1_name: don't resolve refs when core.warnambiguousrefs is false
(this branch is used by jk/warn-on-object-refname-ambiguity.)
When parsing a 40-hex string into the object name, the string is
checked to see if it can be interpreted as a ref so that a warning
can be given for ambiguity. The code kicked in even when the
core.warnambiguousrefs is set to false to squelch this warning, in
which case the cycles spent to look at the ref namespace were an
expensive no-op, as the result was discarded without being used.
* jk/pull-rebase-using-fork-point (2014-01-09) 1 commit
(merged to 'next' on 2014-01-10 at e86e59d)
+ rebase: fix fork-point with zero arguments
* jl/submodule-mv-checkout-caveat (2014-01-07) 2 commits
(merged to 'next' on 2014-01-10 at 8d3953c)
+ rm: better document side effects when removing a submodule
+ mv: better document side effects when moving a submodule
With a submodule that was initialized in an old fashioned way
without gitlinks, switching branches in the superproject between
the one with and without the submodule may leave the submodule
working tree with its embedded repository behind, as there may be
unexpendable state there. Document and warn users about this.
* jn/pager-lv-default-env (2014-01-07) 1 commit
(merged to 'next' on 2014-01-10 at 22d4755)
+ pager: set LV=-c alongside LESS=FRSX
Just like we give a reasonable default for less via the LESS
environment variable, specify a reasonable default for lv via the
LV environment variable when spawning the pager.
* mh/shorten-unambigous-ref (2014-01-09) 3 commits
+ shorten_unambiguous_ref(): tighten up pointer arithmetic
+ gen_scanf_fmt(): delete function and use snprintf() instead
+ shorten_unambiguous_ref(): introduce a new local variable
* mm/mv-file-to-no-such-dir-with-slash (2014-01-10) 1 commit
+ mv: let 'git mv file no-such-dir/' error out on Windows, too
Finishing touches to a topic that has already been merged to 'master'.
* ow/stash-with-ifs (2014-01-07) 1 commit
(merged to 'next' on 2014-01-10 at 4fc9bdb)
+ stash: handle specifying stashes with $IFS
The implementation of 'git stash $cmd stash@{...}' did not quote
the stash argument properly and left it split at IFS whitespace.
* rr/completion-format-coverletter (2014-01-07) 1 commit
(merged to 'next' on 2014-01-10 at d2dbc9d)
+ completion: complete format.coverLetter
The bash/zsh completion code did not know about format.coverLetter
among many format.* configuration variables.
--
[New Topics]
* ab/subtree-doc (2014-01-13) 1 commit
- subtree: fix argument validation in add/pull/push
Will merge to 'next'.
* jk/complete-merge-base (2014-01-13) 2 commits
- completion: handle --[no-]fork-point options to git-rebase
- completion: complete merge-base options
Will merge to 'next'.
* po/everyday-doc (2014-01-13) 1 commit
- Make 'git help everyday' work
* rb/merge-prepare-commit-msg-hook (2014-01-10) 4 commits
- merge: drop unused arg from abort_commit method signature
- merge: make prepare_to_commit responsible for write_merge_state
- t7505: ensure cleanup after hook blocks merge
- t7505: add missing
Expose more merge states (e.g. $GIT_DIR/MERGE_MODE) to hooks that
run during git merge. The log stressed too much on one hook,
prepare-commit-msg, but it would equally apply to other hooks like
post-merge, I think.
Waiting to give a chance to reroll.
* bl/blame-full-history (2014-01-14) 1 commit
- blame: new option --prefer-first to better handle merged cherry-picks
* da/pull-ff-configuration (2014-01-15) 2 commits
- pull: add --ff-only to the help text
- pull: add pull.ff configuration
Will merge to 'next'.
* jc/maint-pull-docfix (2014-01-14) 2 commits
- Documentation: git pull does not have the -m option
- Merge branch 'jc/maint-pull-docfix-for-409b8d82' into jc/maint-pull-docfix
(this branch uses jc/maint-pull-docfix-for-409b8d82.)
Will merge to 'next'.
* jc/maint-pull-docfix-for-409b8d82 (2014-01-14) 1 commit
- Documentation: exclude irrelevant options from git pull
(this branch is used by jc/maint-pull-docfix.)
Will merge to 'next'.
* jc/revision-range-unpeel
Re: [PATCH v4 3/6] submodule: Explicit local branch creation in module_clone
On Thu, Jan 16, 2014 at 11:43:44AM -0800, Junio C Hamano wrote: W. Trevor King wk...@tremily.us writes: @@ -817,11 +831,15 @@ cmd_update() displaypath=$(relative_path $prefix$sm_path) - if test $update_module = none - then + case $update_module in + none) echo Skipping submodule '$displaypath' continue - fi + ;; + checkout) + local_branch= + ;; + esac I wonder if there is a way to avoid detaching (and you may need to update the coddpath that resets the submodule to the commit specified by the superproject tree) when it is safe to do so. … Perhaps that kind of 'git submodule update' is parallel to 'git pull' in the project without submodules is better done with other update modes like --rebase or --merge. If so, how should we explain what 'submodule update --checkout' is to the end users? Is it supposed to be like git fetch git checkout origin/master? It sounds like you're looking for: submodule.name.update = !git merge --ff-only That's fine for folks who want that sort of advancement, but I think there will also be blind updaters who just want the gitlinked commit, and don't care if that blows away local work, because they never work locally in the submodule. They'll still prefer the current checkout-mode with it's clobbering. I think the best way to explain this to users is to have 'git checkout' (with an optional '--recurse-submdules' trial period) checkout the gitlinked commit automatically. Then there is never local submodule work that is not committed or stashed in the superproject (or stashed on some out-of-the-way branch in the submodule). Currently we have: 1. Checkout a superproject branch and currently gitlinked submodule. 2. Do local work on the submodule. 3. Alter the superproject and its gitlinks. 4. 'git submodule update' to integrate your work from 2 with the changes from 3 and checkout the appropriate submodule commit. I think it would make more sense to: 1. Checkout a superproject branch and currently gitlinked submodule. 2. Do local work on the submodule. 3. Commit your new gitlink to the superproject (or stash it, or put it on a temporary submodule branch and reset the submodule HEAD to the old value). 4. Alter the superproject and its gitlinks, using the existing logic to integrate conflicts. Automatically checkout the appropriate submodule commit (as the appropriate submodule branch). That shifts “dealing with local submodule changes” from an integration-time issue (I just called submodule update, what changes are local?) to a pre-checkout-time issue (I've got a dirty submodule (it's HEAD is not the gitlinked commit, all of these changes are local). I think that's a lot easier to wrap your head around. This series is a stop-gap to avoid detached HEADs after cloning, non-checkout updates, while we hash out the real solution ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH 0/6] Make 'git help everyday' work - relnotes
From: Stefan Näwe stefan.na...@atlas-elektronik.com [...] I'd really like to see 'git help relnotes' working as well... Stefan Stefan, Were you thinking that all the release notes would be quoted verbatim in the one long man page? Or that it would be a set of links to each of the individual text files (see the ifdef::stalenotes[] in git/Documentation/git.txt)? The latter allows individual release notes to be checked, but still leaves folks with a difficult search problem if they want to find when some command was 'tweaked'. Obviously, any method would need to be easy to maintain. And the RelNotes symlink would need handling. Philip -- -- To unsubscribe from this list: send the line 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 6/6] Documentation: Describe 'submodule update' modes in detail
On Thu, Jan 16, 2014 at 09:02:22PM +, John Keeping wrote: On Thu, Jan 16, 2014 at 12:55:21PM -0800, W. Trevor King wrote: On Thu, Jan 16, 2014 at 12:21:04PM -0800, Junio C Hamano wrote: Not '--checkout'? Oops, will fix in v5. Shouldn't this also be `--checkout` (backticks), according to CodingGuidelines. This applies to all this options in this patch I think. I can change that too. The existing content is inconsistent between backticks and single quotes, but I see no mention of single quotes in CodingGuidelines. Thanks for the reference. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
[PATCH] .gitignore: Ignore editor backup and swap files
Signed-off-by: Alexander Berntsen alexan...@plaimi.net --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index b5f9def..2905c21 100644 --- a/.gitignore +++ b/.gitignore @@ -240,3 +240,5 @@ *.pdb /Debug/ /Release/ +*~ +.*.swp -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 6/6] Documentation: Describe 'submodule update' modes in detail
W. Trevor King wk...@tremily.us writes: A naïve expectation from a casual reader of the above would be The superproject's tree ought to point at the same commit as the tip of the branch used in the submodule (modulo mirroring delays and somesuch), What is the branch used in the submodule? The remote subproject's current submodule.name.branch? The local submodule's submodule.name.branch (or localBranch) branch? The submodule's current HEAD? They are good questions that such casual readers would have, and giving answers to them in this part of the documentation would be a good way to give them a clear picture of how the command is designed to be used. -- To unsubscribe from this list: send the line 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] .gitignore: Ignore editor backup and swap files
Alexander Berntsen alexan...@plaimi.net writes: Signed-off-by: Alexander Berntsen alexan...@plaimi.net --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index b5f9def..2905c21 100644 --- a/.gitignore +++ b/.gitignore @@ -240,3 +240,5 @@ *.pdb /Debug/ /Release/ +*~ +.*.swp I personally do not mind listing these common ones too much, but if I am not mistaken, our policy on this file so far has been that it lists build artifacts, and not personal preference (the *.swp entry is useless for those who never use vim, for example). These paths that depend on your choice of the editor and other tools can still be managed in your personal .git/info/exclude in the meantime. -- To unsubscribe from this list: send the line 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: problematic git status output with some translations (such as fr_FR)
Hi, On Fri, 20 Dec 2013, Duy Nguyen wrote: On Fri, Dec 20, 2013 at 3:50 AM, Jonathan Nieder jrnie...@gmail.com wrote: Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: This includes the colon in the translated string, to make it easier to remember to keep the non-breaking space before it. Hmph, recent 3651e45c (wt-status: take the alignment burden off translators, 2013-11-05) seems to have gone in the different direction when it updated similar code for the non-unmerged paths. Yes, if this seems to go in the right direction, I'd add a follow-up for that when rerolling. i'm fine either way. In both cases, the alignment burden is taken off the translators. But I agree that it's best to keep the colon in the translatable string so that translators can add the space required by proper typography. So I'd favor Jonathan's approach. I just tested his patch and it solves the immediate problem for me. Without the patch I have this (French translation): | Chemins non fusionnés : | (utilisez git add fichier... pour marquer comme résolu) | | modifié des deux côtés :fichier And with it: | Chemins non fusionnés : | (utilisez git add fichier... pour marquer comme résolu) | | modifié des deux côtés : fichier However it looks like the padding is calculated on bytes and not on (wide) characters, so we have 3 extra spaces before the filename (one for ô, é, and the non-breaking space). This can be problematic for translations where all characters require multiple bytes. :-) The patch is also incomplete since it currently breaks the test suite in multiple places (the unnecessary padding before the filename goes away), for example: --- expected2014-01-16 21:37:11.270535950 + +++ actual 2014-01-16 21:37:11.270535950 + @@ -5,7 +5,7 @@ Unmerged paths: (use git add/rm file... as appropriate to mark resolution) - both added: conflict.txt - deleted by them:main.txt + both added: conflict.txt + deleted by them: main.txt no changes added to commit (use git add and/or git commit -a) not ok 11 - status when conflicts with add and rm advice (deleted by them) That was my two euros to help get this fixed because it annoys me quite often. Cheers, -- Raphaël Hertzog ◈ Debian Developer Discover the Debian Administrator's Handbook: → http://debian-handbook.info/get/ -- To unsubscribe from this list: send the line 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] .gitignore: Ignore editor backup and swap files
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 16/01/14 23:06, Junio C Hamano wrote: I personally do not mind listing these common ones too much, but if I am not mistaken, our policy on this file so far has been that it lists build artifacts, and not personal preference (the *.swp entry is useless for those who never use vim, for example). I don't see any downside to having this in .gitignore. However, not including it doesn't significantly impede my workflow either. I will not argue ferociously for this patch's inclusion. :-) - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLYWh8ACgkQRtClrXBQc7X7WAD9GTUId4ipGdL334Oo6Yn9duSA qixEG95nJ2FsGz5/KNsA/icUkn2BYZRCHevsnYyUFkinOiApckkqpMOTAk5Wsd4D =7Fvb -END PGP SIGNATURE- -- To unsubscribe from this list: send the line 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 6/6] Documentation: Describe 'submodule update' modes in detail
From: Junio C Hamano gits...@pobox.com W. Trevor King wk...@tremily.us writes: [...] @@ -155,13 +155,31 @@ it contains local modifications. update:: Update the registered submodules, i.e. clone missing submodules and - checkout the commit specified in the index of the containing repository. - This will make the submodules HEAD be detached unless `--rebase` or - `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying - `--checkout`. Setting the key `submodule.$name.update` to `!command` - will cause `command` to be run. `command` can be any arbitrary shell - command that takes a single argument, namely the sha1 to update to. + checkout the commit specified in the index of the containing + repository. The update mode defaults to 'checkout', but be nit: s/but be/but can be/ ? + configured with the 'submodule.name.update' setting or the + '--rebase', '--merge', or 'checkout' options. [...] -- To unsubscribe from this list: send the line 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 6/6] Documentation: Describe 'submodule update' modes in detail
On Thu, Jan 16, 2014 at 10:18:06PM -, Philip Oakley wrote: From: Junio C Hamano gits...@pobox.com W. Trevor King wk...@tremily.us writes: + repository. The update mode defaults to 'checkout', but be nit: s/but be/but can be/ ? Thanks. Queuing for v5. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH] .gitignore: Ignore editor backup and swap files
Junio C Hamano wrote: These paths that depend on your choice of the editor and other tools can still be managed in your personal .git/info/exclude in the meantime. Or $HOME/.config/git/ignore to not have to make the same setting in every repository. :) Maybe it would make sense to add a hint about that somewhere to user-manual.txt. Even better would be to automatically include some common exclude patterns globally without requiring any manual configuration, but that would take some care to make sure the patterns and how to disable them are documented clearly. -- 8 -- Subject: gitignore doc: add global gitignore to synopsis The gitignore(5) manpage already documents $XDG_CONFIG_HOME/git/ignore but it is easy to forget that it exists. Add a reminder to the synopsis. Noticed while looking for a place to put a list of scratch filenames in the cwd used by one's editor of choice. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Documentation/gitignore.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt index f971960..37c9470 100644 --- a/Documentation/gitignore.txt +++ b/Documentation/gitignore.txt @@ -7,7 +7,7 @@ gitignore - Specifies intentionally untracked files to ignore SYNOPSIS -$GIT_DIR/info/exclude, .gitignore +$HOME/.config/git/ignore, $GIT_DIR/info/exclude, .gitignore DESCRIPTION --- -- 1.8.5.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] .gitignore: Ignore editor backup and swap files
On 16/01/14 22:06, Junio C Hamano wrote: Alexander Berntsen alexan...@plaimi.net writes: Signed-off-by: Alexander Berntsen alexan...@plaimi.net --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index b5f9def..2905c21 100644 --- a/.gitignore +++ b/.gitignore @@ -240,3 +240,5 @@ *.pdb /Debug/ /Release/ +*~ +.*.swp I personally do not mind listing these common ones too much, but if I am not mistaken, our policy on this file so far has been that it lists build artifacts, and not personal preference (the *.swp entry is useless for those who never use vim, for example). These paths that depend on your choice of the editor and other tools can still be managed in your personal .git/info/exclude in the meantime. As a vim user, I have these set in my ~/.gitignore file, which I refer to from ~/.gitconfig using core.excludesfile. ;-) ATB, Ramsay Jones -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: If the ca path is not specified, use the defaults
Jonathan Nieder jrnie...@gmail.com writes:
FWIW this should help on Mac OS X, too. Folks using git on mac
at $DAYJOB have been using the workaround described at
http://mercurial.selenic.com/wiki/CACertificates#Mac_OS_X_10.6_and_higher
so I forgot to report it. :/
Hmph, is that the same issue, though? That page seems to suggest
using an empty ca file that does not have any useful information as
a workaround. The issue Fedora folks saw is that we see a directory
/etc/ssl/certs exist on the system, and blindly attempt to use it as
SSL_ca_path when the directory is not suitable to be used as such.
In any case, I tried to summarize the discussion in the updated log
message. I wanted to say does not but stopped at should not in
the last paragraph for now. Maybe Ram can say something before we
merge it to 'next'.
The patch in the meantime will be queued on 'pu'.
-- 8 --
From: Ruben Kerkhof ru...@rubenkerkhof.com
Date: Wed, 15 Jan 2014 21:31:11 +0400
Subject: [PATCH] send-email: /etc/ssl/certs/ directory may not be usable as
ca_path
When sending patches on Fedora rawhide with
git-1.8.5.2-1.fc21.x86_64 and perl-IO-Socket-SSL-1.962-1.fc21.noarch,
with the following
[sendemail]
smtpencryption = tls
smtpserver = smtp.gmail.com
smtpuser = ru...@rubenkerkhof.com
smtpserverport = 587
git-send-email fails with:
STARTTLS failed! SSL connect attempt failed with unknown error
error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate
verify failed at /usr/libexec/git-core/git-send-email line 1236.
The current code detects the presence of /etc/ssl/certs directory
(it actually is a symlink to another directory, but that does not
matter) and uses SSL_ca_path to point at it when initializing the
connection with IO::Socket::SSL or Net::SMTP::SSL. However, on the
said platform, it seems that this directory is not designed to be
used as SSL_ca_path. Using a single file inside that directory
(cert.pem, which is a Mozilla CA bundle) with SSL_ca_file does work,
and also not specifying any SSL_ca_file/SSL_ca_path (and letting the
library use its own default) and asking for peer verification does
work.
By removing the code that blindly defaults $smtp_ssl_cert_path to
/etc/ssl/certs, we can prevent the codepath that treats any
directory specified with that variable as usable for SSL_ca_path
from incorrectly triggering.
This change could introduce a regression for people on a platform
whose certificate directory is /etc/ssl/certs but its IO::Socket:SSL
somehow fails to use it as SSL_ca_path without being told. Using
/etc/ssl/certs directory as SSL_ca_path by default like the current
code does would have been hiding such a broken installation without
its user needing to do anything. These users can still work around
such a platform bug by setting the configuration variable explicitly
to point at /etc/ssl/certs.
This change should not negate what 35035bbf (send-email: be explicit
with SSL certificate verification, 2013-07-18), which was the
original change that introduced the defaulting to /etc/ssl/certs/,
attempted to do, which is to make sure we do not communicate over
insecure connection by default, triggering warning from the library.
Cf. https://bugzilla.redhat.com/show_bug.cgi?id=1043194
Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
Signed-off-by: Ruben Kerkhof ru...@rubenkerkhof.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
git-send-email.perl | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 3782c3b..689944f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1095,7 +1095,8 @@ sub ssl_verify_params {
}
if (!defined $smtp_ssl_cert_path) {
- $smtp_ssl_cert_path = /etc/ssl/certs;
+ # use the OpenSSL defaults
+ return (SSL_verify_mode = SSL_VERIFY_PEER());
}
if ($smtp_ssl_cert_path eq ) {
--
1.8.5.3-493-gb139ac2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
file permissions in Git repo
Hi We have a repository which holds lots of shell and perl scripts. We add the files to repository (from windows client) with executable permissions (using cygwin) but when we pull that repository on another machine (windows or linux), files dont have executable permission. Can you please provide a solutions for this? We also tried post-commit and post-receive hooks to set the permission but it didnt seem to work either. 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
'git log' escape symbols shown as ESC[33 and ESC[m
When I run 'git log' on FreeBSD-9.2, I get output like this: ESC[33mcommit 398e78c62fd507a317de7c2abb8e25c9fac7ac9eESC[m Merge: 5fb8f6e d2138ba ... ESC is white on black background. Why ESC[33m aren't expanded by the terminal? Is this because git prints an unsupported sequence? Hex of what git writes to terminal is here: 0x 1b5b 6d63 6f6d 6d69 7420 6636 6432 6136 3032 3965 6661 6439 6635 6334 3161 6261 |.[33mcommit f6d2a6029efad9f5c41aba| 0x0022 3961 3830 6131 3032 3138 6332 6333 3465 6662 1b5b 6d0a 4d65 7267 653a 2033 3938 6537 |9a80a10218c2c34efb.[m.Merge: 398e7| I think it tries to print the line in yellow (color code 33), and prints the wrong sequence. The correct sequence would be: \033[1;33mString Goes Here\033[0m It misses 1; in the beginning, and 0 in the end, this is why the sequence is not interpreted. Why does it print a wrong sequence? Is this because this is some kind of linuxism that doesn't work on FreeBSD maybe? Also, there are the termcap functions that allow to determine what does the actual terminal supports. You should first check for cap bits corresponding to the features you expect, if you expect something uncommon. Yuri -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream
With git-next, where git pull --rebase can print out fatal: No such ref: '' if git pull --rebase is run on branches without an upstream. With git at b139ac2589b15d55cd9fa5c6957da44b150d0737, the following commands demonstrate the problem: git init repo1 cd repo1 touch a; git add a; git commit -m a cd .. git clone repo1 repo2 cd repo2 git config remote.origin.fetch refs/heads/master:refs/remotes/origin/master git checkout -b test git pull --rebase This results in the following output: fatal: No such ref: '' Current branch test is up to date. So the pull --rebase looks like it works, but it prints out a spurious fatal error. I've managed to bisect this down to https://github.com/gitster/git/commit/48059e405028ebf8a09c5a9aede89dfb460cce98. Looks like get_remote_merge_branch is called without arguments, and it returns an empty string. This string is passed as-is to git merge-base, which causes the error. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] diff_filespec cleanups and optimizations
I recently came across a repository with a commit containing 100 million paths in its tree. Cleverly, the whole repo fits into a 1.5K packfile (can you guess how it was done?). Not so cleverly, running diff-tree --root on that commit uses a large amount of memory. :) I do not think it is worth optimizing for such a pathological repository. But I was curious how much it would want (it OOM'd on my 64-bit 16G machine). The answer is roughly: 100,000,000 * ( 8 bytes per pointer to diff_filepair in the diff_queue + 32 bytes per diff_filepair struct + 2 * ( 96 bytes per diff_filespec struct + 12 bytes per filename (in this case) ) ) which is about 25G. Plus malloc overhead. So obviously this example is unreasonable. A more reasonable large case is something like WebKit at ~150K files, doing a diff against the empty tree. That's only 37M. But while looking at it, I noticed a bunch of cleanups for diff_filespec. With the patches below, sizeof(struct diff_filespec) on my 64-bit machine goes from 96 bytes down to 80. Compiling with -m32 goes from 64 bytes down to 52. The first few patches have cleanup value aside from the struct size improvement. The last two are pure optimization. I doubt the optimization is noticeable for any real-life cases, so I don't mind if they get dropped. But they're quite trivial and obvious. [1/5]: diff_filespec: reorder dirty_submodule macro definitions [2/5]: diff_filespec: drop funcname_pattern_ident field [3/5]: diff_filespec: drop xfrm_flags field [4/5]: diff_filespec: reorder is_binary field [5/5]: diff_filespec: use only 2 bits for is_binary flag -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
[PATCH 1/5] diff_filespec: reorder dirty_submodule macro definitions
diff_filespec has a 2-bit dirty_submodule field and
defines two flags as macros. Originally these were right
next to each other, but a new field was accidentally added
in between in commit 4682d85. This patch puts the field and
its flags back together.
Using an enum like:
enum {
DIRTY_SUBMODULE_UNTRACKED = 1,
DIRTY_SUBMODULE_MODIFIED = 2
} dirty_submodule;
would be more obvious, but it bloats the structure. Limiting
the enum size like:
} dirty_submodule : 2;
might work, but it is not portable.
Signed-off-by: Jeff King p...@peff.net
---
diffcore.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/diffcore.h b/diffcore.h
index 1c16c85..f822f9e 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -43,9 +43,9 @@ struct diff_filespec {
unsigned should_free : 1; /* data should be free()'ed */
unsigned should_munmap : 1; /* data should be munmap()'ed */
unsigned dirty_submodule : 2; /* For submodules: its work tree is
dirty */
- unsigned is_stdin : 1;
#define DIRTY_SUBMODULE_UNTRACKED 1
#define DIRTY_SUBMODULE_MODIFIED 2
+ unsigned is_stdin : 1;
unsigned has_more_entries : 1; /* only appear in combined diff */
struct userdiff_driver *driver;
/* data should be considered binary; -1 means don't know yet */
--
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] diff_filespec: drop xfrm_flags field
The only mention of this field in the code is by some
debugging code which prints it out (and it will always be
zero, since we never touch it otherwise). It was obsoleted
very early on by 25d5ea4 ([PATCH] Redo rename/copy detection
logic., 2005-05-24).
Signed-off-by: Jeff King p...@peff.net
---
No savings here on 64-bit, since this ended up going to padding, but it
is what makes the next patch work.
diff.c | 4 ++--
diffcore.h | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c
index 6b4cd0e..8e4a6a9 100644
--- a/diff.c
+++ b/diff.c
@@ -4139,9 +4139,9 @@ void diff_debug_filespec(struct diff_filespec *s, int x,
const char *one)
DIFF_FILE_VALID(s) ? valid : invalid,
s-mode,
s-sha1_valid ? sha1_to_hex(s-sha1) : );
- fprintf(stderr, queue[%d] %s size %lu flags %d\n,
+ fprintf(stderr, queue[%d] %s size %lu\n,
x, one ? one : ,
- s-size, s-xfrm_flags);
+ s-size);
}
void diff_debug_filepair(const struct diff_filepair *p, int i)
diff --git a/diffcore.h b/diffcore.h
index 92e4d48..22993e1 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -31,7 +31,6 @@ struct diff_filespec {
void *cnt_data;
unsigned long size;
int count; /* Reference count */
- int xfrm_flags; /* for use by the xfrm */
int rename_used; /* Count of rename users */
unsigned short mode; /* file mode */
unsigned sha1_valid : 1; /* if true, use sha1 and trust mode;
--
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] diff_filespec: reorder is_binary field
The middle of the diff_filespec struct contains a mixture of
ints, shorts, and bit-fields, followed by a pointer. On an
x86-64 system with an LP64 or LLP64 data model (i.e., most
of them), the integers and flags end up being padded out by
41 bits to put the pointer at an 8-byte boundary.
After the pointer, we have the int is_binary field, which
is only 32 bits. We end up wasting another 32 bits to pad
the struct size up to a multiple of 64 bits.
We can move the is_binary field before the pointer, which
lets the compiler store it where we used to have padding.
This shrinks the top padding to only 9 bits (from the
bit-fields), and eliminates the bottom padding entirely,
dropping the struct size from 88 to 80 bytes.
On a 32-bit system, there is no benefit, but nor should
there be any harm (we only need 4-byte alignment there, so
we were already using only 9 bits of padding).
Signed-off-by: Jeff King p...@peff.net
---
diffcore.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/diffcore.h b/diffcore.h
index 22993e1..d911bf0 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -45,9 +45,9 @@ struct diff_filespec {
#define DIRTY_SUBMODULE_MODIFIED 2
unsigned is_stdin : 1;
unsigned has_more_entries : 1; /* only appear in combined diff */
- struct userdiff_driver *driver;
/* data should be considered binary; -1 means don't know yet */
int is_binary;
+ struct userdiff_driver *driver;
};
extern struct diff_filespec *alloc_filespec(const char *);
--
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] diff_filespec: use only 2 bits for is_binary flag
The is_binary flag needs only three values: -1, 0, and 1.
However, we use a whole 32-bit int for it on most systems
(both 32- and 64- bit).
Instead, we can mark it to use only 2 bits. On 32-bit
systems, this lets it end up as part of the bitfield above
(saving 4 bytes). On 64-bit systems, we don't see any change
(because the savings end up as padding), but it does leave
room for another free 32-bit value to be added later.
Signed-off-by: Jeff King p...@peff.net
---
I don't typically use bit-sized integers like this for anything but
unsigned integers to be used as flags. My understanding is that using
signed integers is explicitly permitted by the standard. I don't know if
we're guaranteed a 2's-complement representation, but I can't imagine an
implementation providing any range besides -2..1, which is what we need.
diffcore.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/diffcore.h b/diffcore.h
index d911bf0..79de8cf 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -46,7 +46,7 @@ struct diff_filespec {
unsigned is_stdin : 1;
unsigned has_more_entries : 1; /* only appear in combined diff */
/* data should be considered binary; -1 means don't know yet */
- int is_binary;
+ int is_binary : 2;
struct userdiff_driver *driver;
};
--
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line 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 log' escape symbols shown as ESC[33 and ESC[m
On Thu, Jan 16, 2014 at 04:34:01PM -0800, Yuri wrote: When I run 'git log' on FreeBSD-9.2, I get output like this: ESC[33mcommit 398e78c62fd507a317de7c2abb8e25c9fac7ac9eESC[m Merge: 5fb8f6e d2138ba ... ESC is white on black background. Why ESC[33m aren't expanded by the terminal? Is this because git prints an unsupported sequence? Are you using less as your pager (it is the default in git unless you have set your PAGER environment variable)? If so, do you have the R option set to pass through ANSI codes? Git will set this automatically in your LESS variable if you do not already have such a variable (but it will not touch it if you already have it set, and are missing R). Hex of what git writes to terminal is here: 0x 1b5b 6d63 6f6d 6d69 7420 6636 6432 6136 3032 3965 6661 6439 6635 6334 3161 6261 |.[33mcommit f6d2a6029efad9f5c41aba| 0x0022 3961 3830 6131 3032 3138 6332 6333 3465 6662 1b5b 6d0a 4d65 7267 653a 2033 3938 6537 |9a80a10218c2c34efb.[m.Merge: 398e7| I think it tries to print the line in yellow (color code 33), and prints the wrong sequence. The correct sequence would be: \033[1;33mString Goes Here\033[0m It misses 1; in the beginning, and 0 in the end, this is why the sequence is not interpreted. No, the \033[33m is correct. The 1; in your string is turning on the bold attribute, and we are not trying to do that. The 0 in the reset is optional (at least according to [1]; I do not have an actual standard to reference). But I do not think that is your problem anyway; the ESC you are seeing is almost certainly generated by less escaping the \033. Why does it print a wrong sequence? Is this because this is some kind of linuxism that doesn't work on FreeBSD maybe? No. See above. Also, there are the termcap functions that allow to determine what does the actual terminal supports. You should first check for cap bits corresponding to the features you expect, if you expect something uncommon. Almost every terminal supports ANSI colors. The notable exceptions is the Windows console, but we handle the translation there. If you have a terminal that actually doesn't support ANSI colors, please let us know and we can see what is going on. But I'm reasonably sure that you are just running up against the escaping in less here. -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: 'git log' escape symbols shown as ESC[33 and ESC[m
On 01/16/2014 17:47, Jeff King wrote: Are you using less as your pager (it is the default in git unless you have set your PAGER environment variable)? If so, do you have the R option set to pass through ANSI codes? Git will set this automatically in your LESS variable if you do not already have such a variable (but it will not touch it if you already have it set, and are missing R). My PAGER variable was set to more. PAGER=more is also a default for a newly created user in FreeBSD. So what would be the correct fix here in general, so that git will work fine for a new unchanged user? Yuri -- To unsubscribe from this list: send the line 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 log' escape symbols shown as ESC[33 and ESC[m
On Thu, Jan 16, 2014 at 06:02:24PM -0800, Yuri wrote:
On 01/16/2014 17:47, Jeff King wrote:
Are you using less as your pager (it is the default in git unless you
have set your PAGER environment variable)? If so, do you have the R
option set to pass through ANSI codes? Git will set this automatically
in your LESS variable if you do not already have such a variable (but
it will not touch it if you already have it set, and are missing R).
My PAGER variable was set to more. PAGER=more is also a default for
a newly created user in FreeBSD.
Interesting. I take it that more does not pass through ANSI codes at
all, then.
So what would be the correct fix here in general, so that git will
work fine for a new unchanged user?
This was a non-issue until 4c7f181 (make color.ui default to 'auto',
2013-06-10), which was released in git v1.8.4, as people had to
explicitly turn color on. I'm cc-ing Matthieu, who authored that commit.
You can:
git config color.ui false
to turn off color completely. But in this case, I think it is more
exact to tell git simply that your pager does not handle color:
git config color.pager false
Obviously that does not help the new unchanged user. I think we can
be smarter about guessing whether the pager can actually handle color,
based on the name. Is it possible for you to compile git with the patch
below? It should make your problem go away without having to configure
anything. It can't cover every possible pager, but I think it should
catch the common ones.
---
diff --git a/cache.h b/cache.h
index 83a2726..7fd1977 100644
--- a/cache.h
+++ b/cache.h
@@ -1215,7 +1215,8 @@ static inline ssize_t write_str_in_full(int fd, const
char *str)
extern void setup_pager(void);
extern const char *pager_program;
extern int pager_in_use(void);
-extern int pager_use_color;
+extern int pager_use_color_config;
+extern int pager_use_color(void);
extern int term_columns(void);
extern int decimal_width(int);
extern int check_pager_config(const char *cmd);
diff --git a/color.c b/color.c
index f672885..ffbff40 100644
--- a/color.c
+++ b/color.c
@@ -184,7 +184,7 @@ static int check_auto_color(void)
{
if (color_stdout_is_tty 0)
color_stdout_is_tty = isatty(1);
- if (color_stdout_is_tty || (pager_in_use() pager_use_color)) {
+ if (color_stdout_is_tty || (pager_in_use() pager_use_color())) {
char *term = getenv(TERM);
if (term strcmp(term, dumb))
return 1;
diff --git a/config.c b/config.c
index d969a5a..2a8236b 100644
--- a/config.c
+++ b/config.c
@@ -991,7 +991,7 @@ int git_default_config(const char *var, const char *value,
void *dummy)
return git_default_advice_config(var, value);
if (!strcmp(var, pager.color) || !strcmp(var, color.pager)) {
- pager_use_color = git_config_bool(var,value);
+ pager_use_color_config = git_config_bool(var,value);
return 0;
}
diff --git a/environment.c b/environment.c
index 3c76905..5cd642f 100644
--- a/environment.c
+++ b/environment.c
@@ -39,7 +39,7 @@ size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
size_t delta_base_cache_limit = 16 * 1024 * 1024;
unsigned long big_file_threshold = 512 * 1024 * 1024;
const char *pager_program;
-int pager_use_color = 1;
+int pager_use_color_config = -1;
const char *editor_program;
const char *askpass_program;
const char *excludes_file;
diff --git a/pager.c b/pager.c
index 0cc75a8..a816af3 100644
--- a/pager.c
+++ b/pager.c
@@ -182,3 +182,38 @@ int check_pager_config(const char *cmd)
pager_program = c.value;
return c.want;
}
+
+static int pager_can_handle_color(void)
+{
+ const char *pager = git_pager(1);
+
+ /*
+* If it's less, we automatically set R and can handle color,
+* unless the user already has a LESS variable that does not
+* include R.
+*/
+ if (!strcmp(pager, less)) {
+ const char *x = getenv(LESS);
+ return !x || !!strchr(x, 'R');
+ }
+
+ /*
+* We know that more does not pass through colors at all.
+*/
+ if (!strcmp(pager, more))
+ return 0;
+
+ /*
+* Otherwise, we don't recognize it. Guess that it can probably handle
+* color. This matches historical behavior, though it is probably not
+* the safest default.
+*/
+ return 1;
+}
+
+int pager_use_color(void)
+{
+ if (pager_use_color_config 0)
+ pager_use_color_config = pager_can_handle_color();
+ return pager_use_color_config;
+}
--
To unsubscribe from this list: send the line 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: with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream
On Thu, Jan 16, 2014 at 05:08:14PM -0800, Siddharth Agarwal wrote: With git-next, where git pull --rebase can print out fatal: No such ref: '' if git pull --rebase is run on branches without an upstream. This is already fixed in bb3f458 (rebase: fix fork-point with zero arguments, 2014-01-09), I think. -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: 'git log' escape symbols shown as ESC[33 and ESC[m
On Thu, Jan 16, 2014 at 06:28:10PM -0800, Yuri wrote: On 01/16/2014 18:13, Jeff King wrote: Interesting. I take it that more does not pass through ANSI codes at all, then. Actually, 'more -R' also passes colors on FreeBSD. So maybe you can always add -R if PAGER=more on *BSD (any of them) ? This will fix this issue. Ah, if more can handle the colors, then that is preferable. I do think it would have to be system-specific, though. Unlike less, there are many implementations of more, and quite a lot of them will probably not support colors. We can make it a build-time option, and set it to on for FreeBSD. I know, it is unpleasant when you add some new minor feature (like term colors), and people begin to complain about some related issues. But turning colors off isn't a good approach also. App just needs to be smarter about when and how to use them. Agreed. It is simply that most people seem to either use less, or not set their PAGER at all. I think FreeBSD is the exception here. I would go even further, and convey even more information with colors. For example, make all dates which are less than 24 hours red. That's an orthogonal issue. I'm not sure I agree, but if you are interested, feel free to prepare a patch, which will get some discussion going. -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: 'git log' escape symbols shown as ESC[33 and ESC[m
On Thu, Jan 16, 2014 at 06:29:21PM -0800, Jonathan Nieder wrote: + /* +* We know that more does not pass through colors at all. +*/ + if (!strcmp(pager, more)) + return 0; I seem to remember that on some systems more is the name of the full-featured pager that knows how to scroll forward and backward and handle color. (My memory could be faulty. A search in the makefile for DEFAULT_PAGER=more only finds AIX, which is not the platform I was thinking of.) Yeah, my we know here was still guessing. According to Yuri, FreeBSD is the system you are thinking of. :) On a stock Debian system more is especially primitive, which means that it passes colors through, too. It being so primitive also means it is not a particularly good choice for the PAGER setting, though, so probably that's not too important. Yeah, colors do get passed through on Debian. It's possible that other primitive more implementations are OK, too (and certainly we have defaulted to on for them until now). I think we should make an effort to set MORE=R on FreeBSD. We can perhaps just set it unconditionally, and assume that primitive more will ignore it. And then assume that more will handle colors (either because of the R setting, or because it is too dumb to escape it). I can prepare a patch series, but I happily no longer have any antique systems to test this kind of stuff on. -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 v4 6/6] Documentation: Describe 'submodule update' modes in detail
On Thu, Jan 16, 2014 at 01:55:37PM -0800, Junio C Hamano wrote: W. Trevor King wk...@tremily.us writes: On Thu, Jan 16, 2014 at 12:21:04PM -0800, Junio C Hamano wrote: W. Trevor King wk...@tremily.us writes: +is only touched when the remote reference does not match the +submodule's HEAD (for none-mode updates, the submodule is never +touched). The remote reference is usually the gitlinked commit from +the superproject's tree, but with '--remote' it is the upstream +subproject's 'submodule.name.branch'. This remote reference is +integrated with the submodule's HEAD using the specified update mode. … A naïve expectation from a casual reader of the above would be The superproject's tree ought to point at the same commit as the tip of the branch used in the submodule (modulo mirroring delays and somesuch), What is the branch used in the submodule? The remote subproject's current submodule.name.branch? The local submodule's submodule.name.branch (or localBranch) branch? The submodule's current HEAD? They are good questions that such casual readers would have, and giving answers to them in this part of the documentation would be a good way to give them a clear picture of how the command is designed to be used. How about: Note that the update command only interacts with the submodule's HEAD. It doesn't care what this head points to. If the submodule has a branch checked out, HEAD will reference that branch. If the submodule's HEAD is detached, it will reference a commit. After following any references, the commit referenced by the submodule's HEAD may resolve to the commit gitlinked by the superproject, or it may not (if you have made local submodule changes, or checked out a different superproject branch). The update command does not adjust your submodule's HEAD to point at the gitlinked commit before performing any integration. It just takes your submodle's HEAD, whatever it points to, and integrates the remote reference. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream
On 01/16/2014 06:21 PM, Jeff King wrote: On Thu, Jan 16, 2014 at 05:08:14PM -0800, Siddharth Agarwal wrote: With git-next, where git pull --rebase can print out fatal: No such ref: '' if git pull --rebase is run on branches without an upstream. This is already fixed in bb3f458 (rebase: fix fork-point with zero arguments, 2014-01-09), I think. If I'm reading the patch correctly, that only fixes it for git rebase, not for git pull --rebase. git-pull.sh contains a separate invocation of git merge-base --fork-point. -- To unsubscribe from this list: send the line 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 log' escape symbols shown as ESC[33 and ESC[m
On Thu, Jan 16, 2014 at 09:35:48PM -0500, Jeff King wrote: I think we should make an effort to set MORE=R on FreeBSD. We can perhaps just set it unconditionally, and assume that primitive more will ignore it. And then assume that more will handle colors (either because of the R setting, or because it is too dumb to escape it). I can prepare a patch series, but I happily no longer have any antique systems to test this kind of stuff on. Meh. I figured I would have to go to an antique system to find breakage, but it is easy to do it on Debian: $ MORE=R more more: unknown option -R So we do need to make it conditional. -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
[PATCH 0/3] improved out-of-the-box color settings
On Thu, Jan 16, 2014 at 09:35:48PM -0500, Jeff King wrote: I can prepare a patch series, but I happily no longer have any antique systems to test this kind of stuff on. Here it is. For those just joining us, the problem is that on some systems, git log out of the box produces ANSI codes which are munged by the pager and look terrible. I'm trying to improve that by auto-configuring more (to get color in more cases that can handle it), and by dynamically adjusting color.pager's default based on which pager is in use (to turn off color in cases we know won't work). I was able to test my assumptions about MORE on Debian, and adjusted the patches accordingly from the previous discussion. I am still guessing that versions of more without R will pass through the ANSI codes as-is. That works on Debian. If anybody has an older system to test and report on, I'd love to hear it. The second patch turns on MORE=R only for FreeBSD. Yuri, if you can confirm that my patch works for you, that would be excellent. And if you (or anyone) has access to NetBSD or OpenBSD to test the second patch on, I'd welcome updates to config.mak.uname for those systems. We also set LV=-c along with LESS. I punted on detecting that in the third patch, as I do not know the exact format of LV. It looks like just checking for c might not be enough, and I need to actually understand the option format (i.e., -c or -dc, but not -Ac). I'll leave that to somebody who cares more about LV to improve (with these patches, the behavior for them should remain the same, so it's at least not making anything worse). [1/3]: setup_pager: refactor LESS/LV environment setting [2/3]: setup_pager: set MORE=R [3/3]: pager: disable colors for some known-bad configurations -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
[PATCH 2/3] setup_pager: set MORE=R
When we run the pager, we always set LESS=R to tell the pager to pass through ANSI colors. On modern versions of FreeBSD, the system more can do the same trick. Unlike less, we may be dealing with various versions of more that do not understand the R option, but do know how to read options from MORE (Debian's more is an example). For this reason, we make the setting a compile-time option (but turn it on by default on FreeBSD). Signed-off-by: Jeff King p...@peff.net --- Makefile | 7 +++ config.mak.uname | 1 + pager.c | 4 3 files changed, 12 insertions(+) diff --git a/Makefile b/Makefile index b4af1e2..6cd7a2a 100644 --- a/Makefile +++ b/Makefile @@ -312,6 +312,9 @@ all:: # you want to use something different. The value will be interpreted by the # shell at runtime when it is used. # +# Define PAGER_MORE_UNDERSTANDS_R if your system's more pager +# can pass-through ANSI colors using the R option. +# # Define DEFAULT_EDITOR to a sensible editor command (defaults to vi) if you # want to use something different. The value will be interpreted by the shell # if necessary when it is used. Examples: @@ -1608,6 +1611,10 @@ DEFAULT_PAGER_CQ_SQ = $(subst ','\'',$(DEFAULT_PAGER_CQ)) BASIC_CFLAGS += -DDEFAULT_PAGER='$(DEFAULT_PAGER_CQ_SQ)' endif +ifdef PAGER_MORE_UNDERSTANDS_R +BASIC_CFLAGS += -DPAGER_MORE_UNDERSTANDS_R +endif + ifdef SHELL_PATH SHELL_PATH_CQ = $(subst ,\,$(subst \,\\,$(SHELL_PATH))) SHELL_PATH_CQ_SQ = $(subst ','\'',$(SHELL_PATH_CQ)) diff --git a/config.mak.uname b/config.mak.uname index 7d31fad..d63babc 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -188,6 +188,7 @@ ifeq ($(uname_S),FreeBSD) endif PYTHON_PATH = /usr/local/bin/python HAVE_PATHS_H = YesPlease + PAGER_MORE_UNDERSTANDS_R = YesPlease endif ifeq ($(uname_S),OpenBSD) NO_STRCASESTR = YesPlease diff --git a/pager.c b/pager.c index 90d237e..2303164 100644 --- a/pager.c +++ b/pager.c @@ -87,6 +87,10 @@ void setup_pager(void) argv_array_push(env, LESS=FRSX); if (!getenv(LV)) argv_array_push(env, LV=-c); +#ifdef PAGER_MORE_UNDERSTANDS_R + if (!getenv(MORE)) + argv_array_push(env, MORE=R); +#endif pager_process.env = argv_array_detach(env, NULL); if (start_command(pager_process)) -- 1.8.5.2.500.g8060133 -- To unsubscribe from this list: send the line 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] send-email: If the ca path is not specified, use the defaults
On Jan 16, 2014, at 15:19, Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: FWIW this should help on Mac OS X, too. Folks using git on mac at $DAYJOB have been using the workaround described at http://mercurial.selenic.com/wiki/CACertificates#Mac_OS_X_10.6_and_higher so I forgot to report it. :/ Hmph, is that the same issue, though? That page seems to suggest using an empty ca file that does not have any useful information as a workaround. I had written up this draft email that describes the situation on OS X and decided it might not be exactly on topic and wasn't going to send it, but, since you asked... ;) OpenSSL's default location can be found with: openssl version -d On my Ubuntu system I get this: $ openssl version -d OPENSSLDIR: /usr/lib/ssl Then if I look in there like so: $ ls -l /usr/lib/ssl total 8 lrwxrwxrwx 1 root root 14 certs - /etc/ssl/certs drwxr-xr-x 2 root root 4096 engines drwxr-xr-x 2 root root 4096 misc lrwxrwxrwx 1 root root 20 openssl.cnf - /etc/ssl/openssl.cnf lrwxrwxrwx 1 root root 16 private - /etc/ssl/private And that's how it ends up using /etc/ssl/certs. From the SSL_CTX_load_verify_locations man page: When looking up CA certificates, the OpenSSL library will first search the certificates in CAfile, then those in CApath. The default CAfile is cert.pem and the default CApath is certs both located in the openssl version -d directory. See the crypto/ cryptlib.h header [1]. On my OS X platform depending on which version of OpenSSL I'm using, the OPENSSLDIR path would be one of these: /System/Library/OpenSSL /opt/local/etc/openssl And neither of those uses a certs directory, they both use a cert.pem bundle instead: $ ls -l /System/Library/OpenSSL total 32 lrwxrwxrwx 1 root wheel42 cert.pem - ../../../usr/share/curl/ curl-ca-bundle.crt drwxr-xr-x 2 root wheel68 certs drwxr-xr-x 8 root wheel 272 misc -rw-r--r-- 1 root wheel 9381 openssl.cnf drwxr-xr-x 2 root wheel68 private # the certs directory is empty $ ls -l /opt/local/etc/openssl total 32 lrwxrwxrwx 1 root admin35 cert.pem@ - ../../share/curl/curl- ca-bundle.crt drwxr-xr-x 9 root admin 306 misc/ -rw-r--r-- 1 root admin 10835 openssl.cnf Notice neither of those refers to /etc/ssl/certs at all. So the short answer is, yes, hard-coding /etc/ssl/certs as the path on OS X is incorrect and if setting /etc/ssl/certs as the path has the effect of replacing the default locations the verification will fail. Of course Apple patches the included version of OpenSSL starting with OS X 10.6 to look in Apple's keychain, but if you're using a MacPorts-built version that won't happen and still /etc/ssl/ certs will be wrong in both cases. As for the hint in that wiki/CACertificates link above, that does seem odd to me as well. A much better solution would be (if python simply MUST have a file) to export the system's list of trusted root certificates like so: security export -k \ /System/Library/Keychains/SystemRootCertificates.keychain \ -t certs -f pemseq rootcerts.pem # the generated rootcerts.pem file is suitable for use with the # openssl verify -CAfile option (root certs) # Substituting SystemCACertificates for SystemRootCertificates # produces a file suitable for use with the # openssl verify -untrusted option (intermediate certs) and then point python to that rootcerts.pem file. This file [2] may be helpful in understanding what's in SystemRootCertificates.keychain and SystemCACertificates.keychain. The intermediate certs may also need to be exported to a file and pointed to as well (a quick glance at the hgrc docs did not turn up an option for this). --Kyle [1] http://git.openssl.org/gitweb/?p=openssl.git;a=blob;f=crypto/cryptlib.h#l84 [2] http://www.opensource.apple.com/source/security_certificates/security_certificates-32641/CertificateInstructions.rtf -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] setup_pager: refactor LESS/LV environment setting
The way the code is structured now, we have to repeat
ourselves in fetching the environment variables (which will
get annoying as we add more). Instead, let's use an
argv_array. That removes a lot of the extra conditionals
and makes it easier to add new variables.
It does means we'll leak the memory for the array, but:
1. This function is only called once per program.
2. We're now leaking heap memory instead of wasting BSS on
the static array.
Signed-off-by: Jeff King p...@peff.net
---
I actually think we can free pager_process.env after
start_command is run. You _cannot_ do so with argv, though,
and I'd rather leak this tiny bit than have a hard-to-track
assumption on memory lifetime buried here.
pager.c | 20 +---
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/pager.c b/pager.c
index 0cc75a8..90d237e 100644
--- a/pager.c
+++ b/pager.c
@@ -1,6 +1,7 @@
#include cache.h
#include run-command.h
#include sigchain.h
+#include argv-array.h
#ifndef DEFAULT_PAGER
#define DEFAULT_PAGER less
@@ -63,6 +64,7 @@ const char *git_pager(int stdout_is_tty)
void setup_pager(void)
{
const char *pager = git_pager(isatty(1));
+ struct argv_array env = ARGV_ARRAY_INIT;
if (!pager || pager_in_use())
return;
@@ -80,17 +82,13 @@ void setup_pager(void)
pager_process.use_shell = 1;
pager_process.argv = pager_argv;
pager_process.in = -1;
- if (!getenv(LESS) || !getenv(LV)) {
- static const char *env[3];
- int i = 0;
-
- if (!getenv(LESS))
- env[i++] = LESS=FRSX;
- if (!getenv(LV))
- env[i++] = LV=-c;
- env[i] = NULL;
- pager_process.env = env;
- }
+
+ if (!getenv(LESS))
+ argv_array_push(env, LESS=FRSX);
+ if (!getenv(LV))
+ argv_array_push(env, LV=-c);
+ pager_process.env = argv_array_detach(env, NULL);
+
if (start_command(pager_process))
return;
--
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] pager: disable colors for some known-bad configurations
Some pagers do not like the ANSI colors we print. It used to
be that one had to opt into the colors, and you would
usually check your pager config at the same time. These
days, we turn on color.ui by default, meaning that some
users may see broken git log from the start, before they
have configured anything. They can fix it by turning off
color.pager, but we should try to make things work
out-of-the-box as much as possible.
The common cause of this is that the user is using less or
more, has setup its config variable in the environment (so
that we do not override it), but has not included R in
their config.
For other pagers, we simply continue to guess that the pager
can handle it. This is compatible with the current behavior
(and will keep exotic things like diff-highlight | less
working without further config). The downside is that other
random pagers may break.
Signed-off-by: Jeff King p...@peff.net
---
cache.h | 3 ++-
color.c | 2 +-
config.c | 2 +-
environment.c | 2 +-
pager.c | 45 +
5 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index 83a2726..7fd1977 100644
--- a/cache.h
+++ b/cache.h
@@ -1215,7 +1215,8 @@ static inline ssize_t write_str_in_full(int fd, const
char *str)
extern void setup_pager(void);
extern const char *pager_program;
extern int pager_in_use(void);
-extern int pager_use_color;
+extern int pager_use_color_config;
+extern int pager_use_color(void);
extern int term_columns(void);
extern int decimal_width(int);
extern int check_pager_config(const char *cmd);
diff --git a/color.c b/color.c
index f672885..ffbff40 100644
--- a/color.c
+++ b/color.c
@@ -184,7 +184,7 @@ static int check_auto_color(void)
{
if (color_stdout_is_tty 0)
color_stdout_is_tty = isatty(1);
- if (color_stdout_is_tty || (pager_in_use() pager_use_color)) {
+ if (color_stdout_is_tty || (pager_in_use() pager_use_color())) {
char *term = getenv(TERM);
if (term strcmp(term, dumb))
return 1;
diff --git a/config.c b/config.c
index d969a5a..2a8236b 100644
--- a/config.c
+++ b/config.c
@@ -991,7 +991,7 @@ int git_default_config(const char *var, const char *value,
void *dummy)
return git_default_advice_config(var, value);
if (!strcmp(var, pager.color) || !strcmp(var, color.pager)) {
- pager_use_color = git_config_bool(var,value);
+ pager_use_color_config = git_config_bool(var,value);
return 0;
}
diff --git a/environment.c b/environment.c
index 3c76905..5cd642f 100644
--- a/environment.c
+++ b/environment.c
@@ -39,7 +39,7 @@ size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
size_t delta_base_cache_limit = 16 * 1024 * 1024;
unsigned long big_file_threshold = 512 * 1024 * 1024;
const char *pager_program;
-int pager_use_color = 1;
+int pager_use_color_config = -1;
const char *editor_program;
const char *askpass_program;
const char *excludes_file;
diff --git a/pager.c b/pager.c
index 2303164..63e9252 100644
--- a/pager.c
+++ b/pager.c
@@ -184,3 +184,48 @@ int check_pager_config(const char *cmd)
pager_program = c.value;
return c.want;
}
+
+static int pager_can_handle_color(void)
+{
+ const char *pager = git_pager(1);
+
+ /*
+* If it's less, we automatically set R and can handle color,
+* unless the user already has a LESS variable that does not
+* include R.
+*/
+ if (!strcmp(pager, less)) {
+ const char *x = getenv(LESS);
+ return !x || !!strchr(x, 'R');
+ }
+
+ if (!strcmp(pager, more)) {
+#ifdef PAGER_MORE_UNDERSTANDS_R
+ /*
+* An advanced more that knows R is in the same boat as
+* less.
+*/
+ const char *x = getenv(MORE);
+ return !x || !!strchr(x, 'R');
+#else
+ /*
+* For a more primitive more, just assume that it will pass
+* through the control codes verbatim.
+*/
+ return 1;
+#endif
+ }
+
+ /*
+* Otherwise, we don't recognize it. Guess that it can probably handle
+* color. This matches what we have done historically.
+*/
+ return 1;
+}
+
+int pager_use_color(void)
+{
+ if (pager_use_color_config 0)
+ pager_use_color_config = pager_can_handle_color();
+ return pager_use_color_config;
+}
--
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line 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] .gitignore: Ignore editor backup and swap files
Junio C Hamano gits...@pobox.com writes: Alexander Berntsen alexan...@plaimi.net writes: Signed-off-by: Alexander Berntsen alexan...@plaimi.net --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index b5f9def..2905c21 100644 --- a/.gitignore +++ b/.gitignore @@ -240,3 +240,5 @@ *.pdb /Debug/ /Release/ +*~ +.*.swp I personally do not mind listing these common ones too much, but if I am not mistaken, our policy on this file so far has been that it lists build artifacts, and not personal preference (the *.swp entry is useless for those who never use vim, for example). These paths that depend on your choice of the editor and other tools can still be managed in your personal .git/info/exclude in the meantime. Here is a somewhat related question: if one places a file .dir-locals.el in the top directory of the checkout with the contents: ;;; Directory Local Variables ;;; See Info node `(emacs) Directory Variables' for more information. ((c-mode (c-default-style . linux) (indent-tabs-mode . t))) Then all edits in the whole checkout done with Emacs in C files use the right indentation style. Obviously, that's a personal preference setting in that it is useless for those who never use Emacs (the file can be written/edited using M-x add-dir-local-variable RET). It's still providing significant convenience for a number of users while touching only a single file. -- 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] .gitignore: Ignore editor backup and swap files
On Fri, Jan 17, 2014 at 5:43 AM, Jonathan Nieder jrnie...@gmail.com wrote: Subject: gitignore doc: add global gitignore to synopsis The gitignore(5) manpage already documents $XDG_CONFIG_HOME/git/ignore but it is easy to forget that it exists. Add a reminder to the synopsis. Yes! I knew about the xdg thing but was not aware about $xdg/git/ignore. No more updating .git/info/exclude on every newly cloned repo.. -- 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 2/3] setup_pager: set MORE=R
On Jan 16, 2014, at 20:21, Jeff King wrote: When we run the pager, we always set LESS=R to tell the pager to pass through ANSI colors. On modern versions of FreeBSD, the system more can do the same trick. [snip] diff --git a/pager.c b/pager.c index 90d237e..2303164 100644 --- a/pager.c +++ b/pager.c @@ -87,6 +87,10 @@ void setup_pager(void) argv_array_push(env, LESS=FRSX); if (!getenv(LV)) argv_array_push(env, LV=-c); +#ifdef PAGER_MORE_UNDERSTANDS_R + if (!getenv(MORE)) + argv_array_push(env, MORE=R); +#endif How about adding a leading - to both the LESS and MORE settings? Since you're in there patching... :) The man page for more states: Options are also taken from the environment variable MORE (make sure to precede them with a dash (``-'')) but command line options will override them. And while the less man page does not have that wording, it does show this: LESS=-options; export LESS and this: LESS=-Dn9.1$-Ds4.1 So it looks like both LESS and MORE prefer to have their options start with a '-' more or less. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
