* Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)
From: Junio C Hamano @ 2012-01-16 22:47 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqhazv3m17.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> but is that the right thing to do if suffix came from "$4"?
>>
>> As far as I can see, "$4" is used to append "." in very limited cases, and
>> nobody explicitly passes SP as "$4" when calling this, so it may be easier
>> to read if you moved this before that "if we have 3 or more args, use the
>> fourth one as the suffix" block, i.e. something like this?
>
> Why not, but in case someone explicitely passes " " as $4 in the future,
> it's likely to be better to strip it for the same reason we strip it here.
I doubt that would be sufficent or appropriate. If some caller _WANTS_ to
add a SP, shouldn't we be devising a way to tell zsh to add it without
quoting, instead of silently stripping?
> I don't care much either way in this case.
>
>> + # Because we use '-o nospace' under bash, we need to compensate
>> + # for it by appending SP after completed word ourselves.
>> + local suffix="${BASH_VERSION+ }"
>
> Not sure why you reworded the comment, but I don't think it's a good
> idea to remove the "ZSH would quote the trailing space added with -S"
> that I had added, because this is really the reason we do a special case
> here. Your version is misleading, because we use -o nospace for ZSH too.
Ok, use of "-o nospace" in Zsh is what I missed. I thought the issue was
about the nospace emulation.
So does that mean we would be forcing zsh users to add SP themselves? I
wonder if we can do better than that.
^ permalink raw reply
* Re: Signed tags in octopus merge..
From: Junio C Hamano @ 2012-01-16 22:49 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <CA+55aFzRN2F5PZDZPRmbj9occZwA6E6Pi+S+M_Qq2EfS6sctyA@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Just a heads-up and congrats: octopus merges of signed tags work well,
> and did exactly the RightThing(tm), both at merge time and with
> "--show-signature".
>
> I knew it was supposed to work, but I have to admit that I was a bit
> apprehensive about it when I tried.
>
> Current top-of-head (commit 81d48f0aee54) in the kernel, in case you care.
I looked at it again, and it makes me wonder if we should further reword
it to say "side branch #1, tagged 'devicetree-for-linus'" instead of the
current "parent #2, tagged 'devicetree-for-linus'". It looks very weird to
start counting from #2, when we know we will never show #1 there.
^ permalink raw reply
* Re: [PATCH] test-lib: add the test_bash convenience function
From: Junio C Hamano @ 2012-01-16 22:51 UTC (permalink / raw)
To: Jeff King; +Cc: Jens Lehmann, Git Mailing List
In-Reply-To: <20120115232413.GA14724@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Nice. Many times I have added such a "bash" or "gdb" invocation then
> forgotten "-v", only to scratch my head at why the test seemed to be
> hanging.
>
> Two minor nits on the patch itself:
> ...
> 1. It may be worth putting a warning in the comment that this is never
> to be used in a real test, but only temporarily inserted.
>
> 2. I do this not just with bash, but with "gdb". I wonder if it is worth
> making this "test_foo bash", for some value of "foo" (the ones that
> occur to me are "debug" and "run", but of course they are taken).
>
> Actually, I wonder if the existing test_debug could handle this
> already (though you do have to remember to add "--debug" to your
> command line, then).
I wondered the same thing from a different angle. My first reaction was
"Why is this called 'bash' not 'sh'?" which naturally led to the same
direction as yours "why not an arbitrary command 'test_debug xxx'?"
test_pause perhaps?
^ permalink raw reply
* Re: Signed tags in octopus merge..
From: Jacob Helwig @ 2012-01-16 22:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <7vobu3uusw.fsf@alter.siamese.dyndns.org>
On Mon, Jan 16, 2012 at 2:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> Just a heads-up and congrats: octopus merges of signed tags work well,
>> and did exactly the RightThing(tm), both at merge time and with
>> "--show-signature".
>>
>> I knew it was supposed to work, but I have to admit that I was a bit
>> apprehensive about it when I tried.
>>
>> Current top-of-head (commit 81d48f0aee54) in the kernel, in case you care.
>
> I looked at it again, and it makes me wonder if we should further reword
> it to say "side branch #1, tagged 'devicetree-for-linus'" instead of the
> current "parent #2, tagged 'devicetree-for-linus'". It looks very weird to
> start counting from #2, when we know we will never show #1 there.
>
My immediate thought regarding the "side branch #1" version is not
wanting to have to do the math (even though it's a simple n+1), if I
decide to convert that text into ^ parent selection notation.
--
Jacob Helwig
http://technosorcery.net/about/me
^ permalink raw reply
* Re: Signed tags in octopus merge..
From: Linus Torvalds @ 2012-01-16 23:06 UTC (permalink / raw)
To: Jacob Helwig; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <CAJ8aY1Hi47uyYSjAmtXfDEqgyc8T21WqXdEA0kGS7SQKxQ5b5g@mail.gmail.com>
On Mon, Jan 16, 2012 at 2:58 PM, Jacob Helwig <jacob@technosorcery.net> wrote:
>
> My immediate thought regarding the "side branch #1" version is not
> wanting to have to do the math (even though it's a simple n+1)
"Math is hard, let's go shopping".
But I think even barbie could do the "add one" thing. That said, I
think the current thing is already more than good enough, and I don't
think it's at all confusing to talk about "parent #2". In fact, I
think it's more obvious than "side branch #1".
Linus
^ permalink raw reply
* Re: Simulating an empty git repository without having said repository on disk
From: Richard Hartmann @ 2012-01-16 23:09 UTC (permalink / raw)
To: Jeff King; +Cc: Git List
In-Reply-To: <20120116204131.GC18699@sigill.intra.peff.net>
On Mon, Jan 16, 2012 at 21:41, Jeff King <peff@peff.net> wrote:
> I don't use vcsh, but I seem to recall that it works by overlaying the
> working trees of different repositories on each other, right?
In parallel, but yes.
> So you
> can't just say "oh, files in foo/ belong to repository 'bar'". You must
> take the union of the set of tracked files from all repos, then find the
> difference of that from the set of all files.
Correct.
> Can individual repos mark things as excluded, too? Or do you have a
> master exclusion list? I.e., if I decide that I don't want "foo" tracked
> at all, how do I tell vcsh?
That's something I am still contemplating as there are several ways:
* excludes
* pre-/appends to the gitignore of every repo
* runtime magic
Feedback welcome :)
> I'm not sure why you care about the empty tree if you are only looking
> at untracked files. Or perhaps the problem is that you are using "git
> status", which fundamentally cares about looking at differences between
> HEAD and the index, even though you don't care in this case. In that case,
> maybe "git ls-files -o" would be more appropriate?
--others does not work as I need to look at several repos. I tried to
get the union of --others, but that creates 'argument too large' very
quickly.
Initially, I tried with find, but as that is depth-first, it takes
ages when compared to git's early stopping at directories.
> The most straightforward way in git would be to generate a temporary
> index that mentions all of the tracked files, like this:
>
> tmp=/some/tmp/index
> rm -f $tmp
> for i in repo; do
> git --git-dir=$repo ls-files -z |
> GIT_INDEX_FILE=$tmp xargs -0 git update-index --add
> done
> GIT_INDEX_FILE=$tmp git ls-files -o
>
> but that is very close to your "create an empty git repo" (in fact, you
> might even need to in order for update-index to be happy). But it would
> give you a place to put a master exclusion list (you would use it as
> --exclude=... in the final ls-files).
>
> If you have per-repo exclusion lists, then you could take a different
> approach, and simply run "git ls-files -o" for each repository. Any
> files which appeared in _every_ output would be untracked (since tracked
> files or individually-excluded files would be omitted from at least one
> repo). Like:
See above, but I will try yours as well.
> perl -lne "/^\s*$count (.*)/ and print \$1"
I know I sound picky, but I would also like to avoid any third-party
dependencies if possible. Perl is common, but not installed
everywhere.
> The downside is that you are doing $count traversals of the untracked
> directories. On an OS with a reasonable lstat and a directory structure
> that fits into cache, that is probably not too big a deal, though.
With cold cache, it can take ages. Especially once you have a few
git-annex repos in $HOME.
> I took a lot of guesses at exactly what you want. It might be more clear
> if you gave us an example situation along with the output you expect.
repo foo tracks .foo and .foo.d, bar .bar, etc
% vcsh list #lists repos
foo
bar
baz
% ls -aR
.foo
.foo.d/
.bar
.baz.d/
.quux.d/
.quux.d/foo
.quux.d/quux.d/quux
.quux.d/quux.d/quuux
.quux.d/quux.d/quuuux
.quux.d/quuuux.d/quuuux
pants
shirts
% vcsh run foo git ls-files # run command in context of repo foo
.foo
.foo.d
.quux.d/foo
% vcsh list-untracked # with the code I want in it
.quux.d/quux.d/
.quux.d/quuuux.d/
pants
shirts
%
I hope that makes sense.
The only part that does not already work today is list-untracked.
For failed attempts look at
https://github.com/RichiH/vcsh/tree/list-untracked
https://github.com/RichiH/vcsh/tree/list-untracked-2
Thanks,
Richard
^ permalink raw reply
* [PATCHv3 0/4] git-p4: small fixes to branches and labels
From: Luke Diamand @ 2012-01-16 23:14 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Luke Diamand
This is the third version of some small fixes to git-p4 branch and
label handling.
It was suggested for the earlier version that the author handling
could be simplified to use the 'author' variable. The code can
be simplified, but the author is the wrong value to use - it is
just the author of the commit, not the tag. Use the creator of the
label, or, if that does not exist ("p4 tag ..."), the p4 user.
This change does not fix the other problems with git-p4 labels:
- two p4 labels on the same changelist will fall over
- labels must match exactly the list of files imported
- you can't import a label without a p4 commit
Luke Diamand (4):
git-p4: handle p4 branches and labels containing shell chars
git-p4: cope with labels with empty descriptions
git-p4: importing labels should cope with missing owner
git-p4: add test for p4 labels
contrib/fast-import/git-p4 | 79 ++++++++++++++++++++----------------
t/t9803-git-p4-shell-metachars.sh | 48 ++++++++++++++++++++++
t/t9804-git-p4-label.sh | 73 ++++++++++++++++++++++++++++++++++
3 files changed, 165 insertions(+), 35 deletions(-)
create mode 100755 t/t9804-git-p4-label.sh
--
1.7.8.rc1.209.geac91.dirty
^ permalink raw reply
* [PATCH 1/4] git-p4: handle p4 branches and labels containing shell chars
From: Luke Diamand @ 2012-01-16 23:14 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Luke Diamand
In-Reply-To: <1326755689-3344-1-git-send-email-luke@diamand.org>
Don't use shell expansion when detecting branches, as it will
fail if the branch name contains a shell metachar. Similarly
for labels.
Add additional test for branches with shell metachars.
Signed-off-by: Luke Diamand <luke@diamand.org>
---
contrib/fast-import/git-p4 | 8 +++---
t/t9803-git-p4-shell-metachars.sh | 48 +++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 3e1aa27..822e6f1 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -799,7 +799,7 @@ class P4Submit(Command, P4UserMap):
def canChangeChangelists(self):
# check to see if we have p4 admin or super-user permissions, either of
# which are required to modify changelists.
- results = p4CmdList("protects %s" % self.depotPath)
+ results = p4CmdList(["protects", self.depotPath])
for r in results:
if r.has_key('perm'):
if r['perm'] == 'admin':
@@ -1758,7 +1758,7 @@ class P4Sync(Command, P4UserMap):
def getLabels(self):
self.labels = {}
- l = p4CmdList("labels %s..." % ' '.join (self.depotPaths))
+ l = p4CmdList(["labels", "%s..." % ' '.join (self.depotPaths)])
if len(l) > 0 and not self.silent:
print "Finding files belonging to labels in %s" % `self.depotPaths`
@@ -1800,7 +1800,7 @@ class P4Sync(Command, P4UserMap):
command = "branches"
for info in p4CmdList(command):
- details = p4Cmd("branch -o %s" % info["branch"])
+ details = p4Cmd(["branch", "-o", info["branch"]])
viewIdx = 0
while details.has_key("View%s" % viewIdx):
paths = details["View%s" % viewIdx].split(" ")
@@ -1938,7 +1938,7 @@ class P4Sync(Command, P4UserMap):
sourceRef = self.gitRefForBranch(sourceBranch)
#print "source " + sourceBranch
- branchParentChange = int(p4Cmd("changes -m 1 %s...@1,%s" % (sourceDepotPath, firstChange))["change"])
+ branchParentChange = int(p4Cmd(["changes", "-m", "1", "%s...@1,%s" % (sourceDepotPath, firstChange)])["change"])
#print "branch parent: %s" % branchParentChange
gitParent = self.gitCommitByP4Change(sourceRef, branchParentChange)
if len(gitParent) > 0:
diff --git a/t/t9803-git-p4-shell-metachars.sh b/t/t9803-git-p4-shell-metachars.sh
index db04375..db67020 100755
--- a/t/t9803-git-p4-shell-metachars.sh
+++ b/t/t9803-git-p4-shell-metachars.sh
@@ -57,6 +57,54 @@ test_expect_success 'deleting with shell metachars' '
)
'
+# Create a branch with a shell metachar in its name
+#
+# 1. //depot/main
+# 2. //depot/branch$3
+
+test_expect_success 'branch with shell char' '
+ test_when_finished cleanup_git &&
+ test_create_repo "$git" &&
+ (
+ cd "$cli" &&
+
+ mkdir -p main &&
+
+ echo f1 >main/f1 &&
+ p4 add main/f1 &&
+ p4 submit -d "main/f1" &&
+
+ p4 integrate //depot/main/... //depot/branch\$3/... &&
+ p4 submit -d "integrate main to branch\$3" &&
+
+ echo f1 >branch\$3/shell_char_branch_file &&
+ p4 add branch\$3/shell_char_branch_file &&
+ p4 submit -d "branch\$3/shell_char_branch_file" &&
+
+ p4 branch -i <<-EOF &&
+ Branch: branch\$3
+ View: //depot/main/... //depot/branch\$3/...
+ EOF
+
+ p4 edit main/f1 &&
+ echo "a change" >> main/f1 &&
+ p4 submit -d "a change" main/f1 &&
+
+ p4 integrate -b branch\$3 &&
+ p4 resolve -am branch\$3/... &&
+ p4 submit -d "integrate main to branch\$3" &&
+
+ cd "$git" &&
+
+ git config git-p4.branchList main:branch\$3 &&
+ "$GITP4" clone --dest=. --detect-branches //depot@all &&
+ git log --all --graph --decorate --stat &&
+ git reset --hard p4/depot/branch\$3 &&
+ test -f shell_char_branch_file &&
+ test -f f1
+ )
+'
+
test_expect_success 'kill p4d' '
kill_p4d
'
--
1.7.8.rc1.209.geac91.dirty
^ permalink raw reply related
* [PATCH 3/4] git-p4: importing labels should cope with missing owner
From: Luke Diamand @ 2012-01-16 23:14 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Luke Diamand
In-Reply-To: <1326755689-3344-1-git-send-email-luke@diamand.org>
In p4, the Owner field is optional. If it is missing,
construct something sensible rather than crashing.
Signed-off-by: Luke Diamand <luke@diamand.org>
---
contrib/fast-import/git-p4 | 63 ++++++++++++++++++++++++-------------------
1 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index f7707f2..efb2dad 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -563,6 +563,26 @@ class Command:
class P4UserMap:
def __init__(self):
self.userMapFromPerforceServer = False
+ self.myP4UserId = None
+
+ def p4UserId(self):
+ if self.myP4UserId:
+ return self.myP4UserId
+
+ results = p4CmdList("user -o")
+ for r in results:
+ if r.has_key('User'):
+ self.myP4UserId = r['User']
+ return r['User']
+ die("Could not find your p4 user id")
+
+ def p4UserIsMe(self, p4User):
+ # return True if the given p4 user is actually me
+ me = self.p4UserId()
+ if not p4User or p4User != me:
+ return False
+ else:
+ return True
def getUserCacheFilename(self):
home = os.environ.get("HOME", os.environ.get("USERPROFILE"))
@@ -700,7 +720,6 @@ class P4Submit(Command, P4UserMap):
self.verbose = False
self.preserveUser = gitConfig("git-p4.preserveUser").lower() == "true"
self.isWindows = (platform.system() == "Windows")
- self.myP4UserId = None
def check(self):
if len(p4CmdList("opened ...")) > 0:
@@ -808,25 +827,6 @@ class P4Submit(Command, P4UserMap):
return 1
return 0
- def p4UserId(self):
- if self.myP4UserId:
- return self.myP4UserId
-
- results = p4CmdList("user -o")
- for r in results:
- if r.has_key('User'):
- self.myP4UserId = r['User']
- return r['User']
- die("Could not find your p4 user id")
-
- def p4UserIsMe(self, p4User):
- # return True if the given p4 user is actually me
- me = self.p4UserId()
- if not p4User or p4User != me:
- return False
- else:
- return True
-
def prepareSubmitTemplate(self):
# remove lines in the Files section that show changes to files outside the depot path we're committing into
template = ""
@@ -1664,6 +1664,12 @@ class P4Sync(Command, P4UserMap):
if self.stream_file.has_key('depotFile'):
self.streamOneP4File(self.stream_file, self.stream_contents)
+ def make_email(self, userid):
+ if userid in self.users:
+ return self.users[userid]
+ else:
+ return "%s <a@b>" % userid
+
def commit(self, details, files, branch, branchPrefixes, parent = ""):
epoch = details["time"]
author = details["user"]
@@ -1687,10 +1693,7 @@ class P4Sync(Command, P4UserMap):
committer = ""
if author not in self.users:
self.getUserMapFromPerforceServer()
- if author in self.users:
- committer = "%s %s %s" % (self.users[author], epoch, self.tz)
- else:
- committer = "%s <a@b> %s %s" % (author, epoch, self.tz)
+ committer = "%s %s %s" % (self.make_email(author), epoch, self.tz)
self.gitStream.write("committer %s\n" % committer)
@@ -1735,11 +1738,15 @@ class P4Sync(Command, P4UserMap):
self.gitStream.write("from %s\n" % branch)
owner = labelDetails["Owner"]
- tagger = ""
- if author in self.users:
- tagger = "%s %s %s" % (self.users[owner], epoch, self.tz)
+
+ # Try to use the owner of the p4 label, or failing that,
+ # the current p4 user id.
+ if owner:
+ email = self.make_email(owner)
else:
- tagger = "%s <a@b> %s %s" % (owner, epoch, self.tz)
+ email = self.make_email(self.p4UserId())
+ tagger = "%s %s %s" % (email, epoch, self.tz)
+
self.gitStream.write("tagger %s\n" % tagger)
description = labelDetails["Description"]
--
1.7.8.rc1.209.geac91.dirty
^ permalink raw reply related
* [PATCH 4/4] git-p4: add test for p4 labels
From: Luke Diamand @ 2012-01-16 23:14 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Luke Diamand
In-Reply-To: <1326755689-3344-1-git-send-email-luke@diamand.org>
Add basic test of p4 label import. Checks label import and
import with shell metachars; labels with different length
descriptions.
Signed-off-by: Luke Diamand <luke@diamand.org>
---
t/t9804-git-p4-label.sh | 73 +++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 73 insertions(+), 0 deletions(-)
create mode 100755 t/t9804-git-p4-label.sh
diff --git a/t/t9804-git-p4-label.sh b/t/t9804-git-p4-label.sh
new file mode 100755
index 0000000..eba3521
--- /dev/null
+++ b/t/t9804-git-p4-label.sh
@@ -0,0 +1,73 @@
+test_description='git-p4 p4 label tests'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+# Basic p4 label tests.
+#
+# Note: can't have more than one label per commit - others
+# are silently discarded.
+#
+test_expect_success 'basic p4 labels' '
+ test_when_finished cleanup_git &&
+ (
+ cd "$cli" &&
+ mkdir -p main &&
+
+ echo f1 >main/f1 &&
+ p4 add main/f1 &&
+ p4 submit -d "main/f1" &&
+
+ echo f2 >main/f2 &&
+ p4 add main/f2 &&
+ p4 submit -d "main/f2" &&
+
+ echo f3 >main/file_with_\$metachar &&
+ p4 add main/file_with_\$metachar &&
+ p4 submit -d "file with metachar" &&
+
+ p4 tag -l tag_f1_only main/f1 &&
+ p4 tag -l tag_with\$_shell_char main/... &&
+
+ echo f4 >main/f4 &&
+ p4 add main/f4 &&
+ p4 submit -d "main/f4" &&
+
+ p4 label -i <<-EOF &&
+ Label: long_label
+ Description:
+ A Label first line
+ A Label second line
+ View: //depot/...
+ EOF
+
+ p4 tag -l long_label ... &&
+
+ p4 labels ... &&
+
+ cd "$git" &&
+ pwd &&
+ "$GITP4" clone --dest=. --detect-labels //depot@all &&
+
+ git tag &&
+ git tag >taglist &&
+ test_line_count = 3 taglist &&
+
+ cd main &&
+ git checkout tag_tag_f1_only &&
+ ! test -f f2 &&
+ git checkout tag_tag_with\$_shell_char &&
+ test -f f1 && test -f f2 && test -f file_with_\$metachar &&
+
+ git show tag_long_label | grep -q "A Label second line"
+ )
+'
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done
--
1.7.8.rc1.209.geac91.dirty
^ permalink raw reply related
* [PATCH 2/4] git-p4: cope with labels with empty descriptions
From: Luke Diamand @ 2012-01-16 23:14 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Luke Diamand
In-Reply-To: <1326755689-3344-1-git-send-email-luke@diamand.org>
Use an explicit length for the data in a label, rather
than EOT, so that labels with empty descriptions are
passed through correctly.
Signed-off-by: Luke Diamand <luke@diamand.org>
---
contrib/fast-import/git-p4 | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 822e6f1..f7707f2 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1741,9 +1741,11 @@ class P4Sync(Command, P4UserMap):
else:
tagger = "%s <a@b> %s %s" % (owner, epoch, self.tz)
self.gitStream.write("tagger %s\n" % tagger)
- self.gitStream.write("data <<EOT\n")
- self.gitStream.write(labelDetails["Description"])
- self.gitStream.write("EOT\n\n")
+
+ description = labelDetails["Description"]
+ self.gitStream.write("data %d\n" % len(description))
+ self.gitStream.write(description)
+ self.gitStream.write("\n")
else:
if not self.silent:
--
1.7.8.rc1.209.geac91.dirty
^ permalink raw reply related
* Re: [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees
From: Junio C Hamano @ 2012-01-16 23:21 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Jonathan Nieder
In-Reply-To: <1326681407-6344-3-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Normally cache-tree will not produce trees from an index that has
> CE_INTENT_TO_ADD entries. This is a safe measure to avoid
> mis-interpreting user's intention regarding this flag.
s/safe/safety/;
> There are situations however where users want to create trees/commits
> regardless i-t-a entries.
A new command line option "--no-check-intent-to-add" to "commit" without
any configuration bit may be a useful addition as a first cut, and in
order to help users to which "there are situations" is more than 50% of
the time, a configuration that can be overriden by "--check-intent-to-add"
may be a usability improvement over that first cut, but if this is really
about "there are situations", then a configuration that cannot be
overriden by command line option feels a wrong way to go about it.
Is this really about "there are situations" to begin with? I am suspecting
that this patch is either:
(1) making it easier to use a wrong workflow, by promoting a way to
bypass a useful safety measure;
(2) fixing an earlier UI mistake (iow, the interpretation #2 in the old
discussion is always the right one and the existing safety measure is
misguided) in such a way that allows you to work around an objection
from a bonehead maintainer who refuses to admit that earlier mistake;
and/or
(3) splitting the Git userbase into two and making the resulting system
harder to teach.
If it is (2), and I suspect it may be the case, we might want to rather
honestly describe that the future direction is to fix it, and describe the
configuration option as "an early opt-in" switch, together with the usual
three-step deprecation and migration schedule to make the new behaviour
the default in a future version. From the timeline point of view, it
probably can coincide with the change to always start an editor when
recording a merge commit.
In any case, for this change to help people who add more than one paths
with "add -N" and want to include only a subset of them in the commit, we
may want to explicitly teach them to add what they want to before
committing with the new command line option in the documentation.
^ permalink raw reply
* Re: Re* Regulator updates for 3.3
From: Junio C Hamano @ 2012-01-16 23:33 UTC (permalink / raw)
To: Pete Harlan
Cc: Linus Torvalds, Mark Brown, Liam Girdwood, linux-kernel,
Git Mailing List
In-Reply-To: <4F136BE4.4040502@pcharlan.com>
Pete Harlan <pgit@pcharlan.com> writes:
> On 01/10/2012 10:59 PM, Junio C Hamano wrote:
>> There may be existing scripts that leave the standard input and the
>> standard output of the "git merge" connected to whatever environment the
>> scripts were started, and such invocation might trigger the above
>> "interactive session" heuristics. Such scripts can export GIT_MERGE_LEGACY
>> environment variable set to "yes" to force the traditional behaviour.
>
> The name GIT_MERGE_LEGACY gives no clue about what flavor of legacy
> merge behavior is being enabled. Something like GIT_MERGE_LEGACY_EDIT
> might be clearer, or perhaps just have GIT_MERGE_EDIT=0 to get the old
> behavior without reference to whether or not that behavior is
> considered legacy.
Hrm.
The only case your suggestion may make a difference would be when we find
another earlier UI mistake we would want to correct in a backward
incompatible way that affects _existing_ scripts.
With your suggestion, they need to export "GIT_MERGE_EDIT=0" today, and
they will need to update again to export "GIT_MERGE_SOMETHINGELSE=0" when
such an incompatible change comes.
With a single "GIT_MERGE_LEGACY=YesPlease", they can be future-proofed today
and will not be affected when we make another incompatible change.
So I am not sure why separating the big-red-switch into smaller pieces
would be an improvement, especially wnen the scripts that want to specify
finer-grained control of features can use "--[no-]edit" options to
explicitly ask for it.
^ permalink raw reply
* Re: [PATCH 2/3] git-p4: Search for parent commit on branch creation
From: Vitor Antunes @ 2012-01-16 23:41 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: git
In-Reply-To: <20120116185738.GA21996@padd.com>
On Mon, Jan 16, 2012 at 6:57 PM, Pete Wyckoff <pw@padd.com> wrote:
> This looks much better without the need for "--force". It'll be
> great to fix this major branch detection problem. Can you make a
> couple of further minor changes?
Of course I can :)
>> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
>> @@ -2012,7 +2014,28 @@ class P4Sync(Command, P4UserMap):
>> - self.commit(description, filesForCommit, branch, [branchPrefix], parent)
>> + parentFound = 0
>> + if len(parent) > 0:
>> + self.checkpoint()
>> + for blob in read_pipe_lines("git rev-list --reverse --no-merges %s" % parent):
>> + blob = blob.strip()
>> + tempBranch = self.tempBranchLocation + os.sep + "%d-%s" % (change, blob)
>> + if self.verbose:
>> + print "Creating temporary branch: " + tempBranch
>> + self.commit(description, filesForCommit, tempBranch, [branchPrefix], blob)
>> + self.tempBranches.append(tempBranch)
>> + self.checkpoint()
>> + if len( read_pipe("git diff-tree %s %s" % (blob, tempBranch)) ) == 0:
>> + parentFound = 1
>> + if self.verbose:
>> + print "Found parent of %s in commit %s" % (branch, blob)
>> + break
>> + if parentFound:
>> + self.commit(description, filesForCommit, branch, [branchPrefix], blob)
>> + else:
>> + if self.verbose:
>> + print "Parent of %s not found. Committing into head of %s" % (branch, parent)
>> + self.commit(description, filesForCommit, branch, [branchPrefix], parent)
>
> 1. Move the tempBranch commit outside of the "for blob" loop.
> It can have no parent, and the diff-tree will still tell you
> if you found the same contents. Instead of a ref for
> each blob inspected for each change, you'll just have one ref
> per change. Only one checkpoint() after the tempBranch
> commit should be needed.
You're right. Completely oversaw that. Will improve the code
accordingly.
> 2. Nit. parentFound is boolean, use True/False, not 1/0.
That was not a nice thing to do... thanks for noticing :)
>> @@ -2347,6 +2370,12 @@ class P4Sync(Command, P4UserMap):
>> + # Cleanup temporary branches created during import
>> + if self.tempBranches != []:
>> + for branch in self.tempBranches:
>> + os.remove(".git" + os.sep + branch)
>> + os.rmdir(".git" + os.sep + self.tempBranchLocation)
>> +
>
> 3. Deleting refs should probably use "git update-ref -d"
> just in case GIT_DIR is not ".git". I think you could just
> leave the "git-p4-tmp" directory around, but use
> os.environ["GIT_DIR"] instead of ".git" if you want to
> delete it.
Will use os.environ.get, which can be configured to return ".git" if
$GIT_DIR is not defined. Is this ok?
> 4. Paths are best manipulated with os.path.join(dir, file), to handle
> weirdnesses like drive letters.
Perfect. I was completely unaware of that method. Thanks for the tip.
> Eventually if the fast-import protocol learns to delete the refs
> it creates, we can clean up a bit more nicely. I think there was
> agreement this was a good idea, just needs someone to do it
> sometime.
One can always hope ;)
Thanks,
Vitor
^ permalink raw reply
* Re: Re* Regulator updates for 3.3
From: Martin Fick @ 2012-01-16 23:43 UTC (permalink / raw)
To: Junio C Hamano
Cc: Pete Harlan, Linus Torvalds, Mark Brown, Liam Girdwood,
linux-kernel, Git Mailing List
In-Reply-To: <7v62gbussz.fsf@alter.siamese.dyndns.org>
On Monday, January 16, 2012 04:33:00 pm Junio C Hamano
wrote:
> With your suggestion, they need to export
> "GIT_MERGE_EDIT=0" today, and they will need to update
> again to export "GIT_MERGE_SOMETHINGELSE=0" when such an
> incompatible change comes.
>
> With a single "GIT_MERGE_LEGACY=YesPlease", they can be
> future-proofed today and will not be affected when we
> make another incompatible change.
>
> So I am not sure why separating the big-red-switch into
> smaller pieces would be an improvement, especially wnen
> the scripts that want to specify finer-grained control
> of features can use "--[no-]edit" options to explicitly
> ask for it.
Then, what would I do if I write a script which uses the new
edit functionality (without even being aware that there was
an old way) and you introduce a new incompatibility? I
can't turn on GIT_MERGE_LEGACY then since it would revert to
behavior which my script would not expect (since it was
written after the current incompatibility, but before the
new one)!
-Martin
^ permalink raw reply
* Re: [PATCH 2/3] git-p4: Search for parent commit on branch creation
From: Vitor Antunes @ 2012-01-17 0:10 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: git
In-Reply-To: <CAOpHH-UkyK-c_AHUOPbQQmW9cQQypDvirMR0Jb7vTGSQF7RZpw@mail.gmail.com>
On Mon, Jan 16, 2012 at 11:41 PM, Vitor Antunes <vitor.hda@gmail.com> wrote:
> On Mon, Jan 16, 2012 at 6:57 PM, Pete Wyckoff <pw@padd.com> wrote:
>> 1. Move the tempBranch commit outside of the "for blob" loop.
>> It can have no parent, and the diff-tree will still tell you
>> if you found the same contents. Instead of a ref for
>> each blob inspected for each change, you'll just have one ref
>> per change. Only one checkpoint() after the tempBranch
>> commit should be needed.
>
> You're right. Completely oversaw that. Will improve the code
> accordingly.
Apparently I did not oversee it. Assume you have added a new file to
HEAD of parent branch, but you branched from a previous commit. When the
new branch is committed over HEAD the new file will, incorrectly, be
part of it and diff-tree will not work as expected.
I should avoid taking 6 months to submit a patch to avoid forgetting why
I did what I did :)
Vitor
^ permalink raw reply
* Re: Commit changes to remote repository
From: Junio C Hamano @ 2012-01-17 0:12 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Carlos Martín Nieto, ruperty, git
In-Reply-To: <vpqboq6fjfy.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Carlos Martín Nieto <cmn@elego.de> writes:
>
>> You're trying to push to a non-bare repository and change the
>> currently active branch, which can cause problems, so git isn't
>> letting you. There's an explanation of bare and non-bare at
>> http://bare-vs-nonbare.gitrecipes.de/ but the short and sweet is that
>> you should init the repo you want to use as the central point with
>> --bare and do modifications locally and then push there.
>
> An alternative is to push to a temporary, non-checked-out branch.
Or more generally, treat such a push as if you are pulling in the opposite
direction. So in this example,
> I sometimes do
>
> laptop$ git push desktop HEAD:incomming
>
> and then
>
> desktop$ git merge incomming
you pretend as if you are running "git pull" on your desktop in order to
integrate the work done on your laptop. If you did
desktop$ git pull laptop
you would store where the branches on the laptop are in the remote
tracking branches for "laptop" remote in your desktop's repository.
So a good way to simulate that would be:
laptop$ git push desktop master:refs/remotes/laptop/master
and then run:
desktop$ git merge laptop/master
> The push does not disturb the worktree on the desktop, and the merge is
> done manually on the receiving machine.
^ permalink raw reply
* Re: [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees
From: Nguyen Thai Ngoc Duy @ 2012-01-17 1:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
In-Reply-To: <7vaa5nutbp.fsf@alter.siamese.dyndns.org>
2012/1/17 Junio C Hamano <gitster@pobox.com>:
> A new command line option "--no-check-intent-to-add" to "commit" without
> any configuration bit may be a useful addition as a first cut, and in
> order to help users to which "there are situations" is more than 50% of
> the time, a configuration that can be overriden by "--check-intent-to-add"
> may be a usability improvement over that first cut, but if this is really
> about "there are situations", then a configuration that cannot be
> overriden by command line option feels a wrong way to go about it.
"git -c key=value commit" may help.
> Is this really about "there are situations" to begin with? I am suspecting
> that this patch is either:
>
> (1) making it easier to use a wrong workflow, by promoting a way to
> bypass a useful safety measure;
>
> (2) fixing an earlier UI mistake (iow, the interpretation #2 in the old
> discussion is always the right one and the existing safety measure is
> misguided) in such a way that allows you to work around an objection
> from a bonehead maintainer who refuses to admit that earlier mistake;
> and/or
>
> (3) splitting the Git userbase into two and making the resulting system
> harder to teach.
This patch is towards (3). I agree that "add -N" can be confusing to
new users because it does not actually add anything. Those who do not
read manual carefully and commit without checking the result may fall
into that trap. So a user is expected to "level up" a bit and
hopefully by the time he/she feels the need to get around the safety
check and discover this flag, they should be ready to go without the
safety check.
Although I would not oppose deprecating the old behavior if you go with (2) ;)
> In any case, for this change to help people who add more than one paths
> with "add -N" and want to include only a subset of them in the commit, we
> may want to explicitly teach them to add what they want to before
> committing with the new command line option in the documentation.
yeah, keep telling people "this does not add any thing, you need to
git-add again without -N" after running "add -N" using advice
framework when this config is on?
--
Duy
^ permalink raw reply
* Re: git grep doesn't follow symbolic link
From: Nguyen Thai Ngoc Duy @ 2012-01-17 1:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: Pang Yan Han, Thomas Rast, Ramkumar Ramachandra, Bertrand BENOIT,
git
In-Reply-To: <7vwr8ruv1j.fsf@alter.siamese.dyndns.org>
On Tue, Jan 17, 2012 at 5:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> It's not wrong per se. It's an implication that users have to take
>> when they choose to use it. We may help make it clear that the
>> symlinks point to untracked files by putting some indication in the
>> diff.
>>
>> When I do "git log -Sfoo -- '*.cxx'" I don't really care if bar.cxx is
>> a symlink. Neither does my compiler. It may be a symlink's target
>> change that makes "foo" appear. Git could help me detect that quickly
>> instead of sticking with tracked contents only.
>
> As there is nothing in Git that tells that whatever is pointed at by
> bar.cxx that happens to be in your filesystem today had "foo" in it when
> that historical version of the commit whose bar.cxx symlink was updated to
> point to that file. It is *WRONG* to show the commit as something that
> changes bar.cxx to contain "foo" (or more precisely, changes the count of
> "foo" in it).
>
> Why is it so hard to understand?
OK resolving links to untracked contents is bad and should only be
supported in --no-index case, resolving links to tracked contents
should be ok in principal? (I'm not sure how messed up diff code could
be with these changes)
--
Duy
^ permalink raw reply
* Re: [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees
From: Jonathan Nieder @ 2012-01-17 2:47 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git
In-Reply-To: <CACsJy8B_qGphVz8PFhPNLsOe-Ve7xb+biNP8Ok7dXiygu3KoSg@mail.gmail.com>
Nguyen Thai Ngoc Duy wrote:
> Although I would not oppose deprecating the old behavior if you go with (2) ;)
My reaction on reading the patch and clarifying followup was similar
to Junio's. I don't think a configuration item makes sense unless we
are planning to flip the default in the future, and it would probably
be clearer to document it that way.
By the way, thanks very much for working on this. I wish I had more
time to offer more than comments.
Sincerely,
Jonathan
^ permalink raw reply
* What's cooking in git.git (Jan 2012, #04; Mon, 16)
From: Junio C Hamano @ 2012-01-17 3:27 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with '-' are
only in 'pu' (proposed updates) while commits prefixed with '+' are in
'next'.
Here are the repositories that have my integration branches:
With maint, master, next, pu, todo:
git://git.kernel.org/pub/scm/git/git.git
git://repo.or.cz/alt-git.git
https://code.google.com/p/git-core/
https://github.com/git/git
With only maint and master:
git://git.sourceforge.jp/gitroot/git-core/git.git
git://git-core.git.sourceforge.net/gitroot/git-core/git-core
With all the topics and integration branches:
https://github.com/gitster/git
The preformatted documentation in HTML and man format are found in:
git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/
git://repo.or.cz/git-{htmldocs,manpages}.git/
https://code.google.com/p/git-{htmldocs,manpages}.git/
https://github.com/gitster/git-{htmldocs,manpages}.git/
--------------------------------------------------
[New Topics]
* mh/maint-show-ref-doc (2012-01-13) 2 commits
(merged to 'next' on 2012-01-16 at 8573f09)
+ git-show-ref doc: typeset regexp in fixed width font
+ git-show-ref: fix escaping in asciidoc source
Will merge to 'master' by v1.7.9-rc2.
* nd/pathspec-recursion-cleanup (2012-01-16) 2 commits
(merged to 'next' on 2012-01-16 at 0189264)
+ diff-index: enable recursive pathspec matching in unpack_trees
+ Document limited recursion pathspec matching with wildcards
Will merge to 'master' by v1.7.9-rc2.
* tr/maint-word-diff-incomplete-line (2012-01-12) 1 commit
(merged to 'next' on 2012-01-16 at 58ddaaf)
+ word-diff: ignore '\ No newline at eof' marker
Will merge to 'master' by v1.7.9-rc2.
--------------------------------------------------
[Graduated to "master"]
* jc/request-pull-show-head-4 (2012-01-10) 1 commit
(merged to 'next' on 2012-01-11 at 8d98a6b)
+ request-pull: use the real fork point when preparing the message
Hopefully the final finishing touch to the request-pull script that was
updated during this cycle.
* jk/maint-upload-archive (2012-01-11) 1 commit
(merged to 'next' on 2012-01-11 at 5c0bfa9)
+ archive: re-allow HEAD:Documentation on a remote invocation
Running "git archive" remotely and asking for a partial tree of a ref,
e.g. HEAD:$path was forbidden by a recent change to tighten security, but
was found to be overly restrictive.
* jn/maint-gitweb-grep-fix (2012-01-05) 2 commits
(merged to 'next' on 2012-01-13 at a15e6ab)
+ gitweb: Harden "grep" search against filenames with ':'
+ gitweb: Fix file links in "grep" search
* ss/maint-msys-cvsexportcommit (2012-01-11) 2 commits
(merged to 'next' on 2012-01-11 at 007aab1)
+ git-cvsexportcommit: Fix calling Perl's rel2abs() on MSYS
+ t9200: On MSYS, do not pass Windows-style paths to CVS
--------------------------------------------------
[Stalled]
* jc/advise-push-default (2011-12-18) 1 commit
- push: hint to use push.default=upstream when appropriate
Peff had a good suggestion outlining an updated code structure so that
somebody new can try to dip his or her toes in the development. Any
takers?
Waiting for a reroll.
* jc/split-blob (2011-12-01) 6 commits
. WIP (streaming chunked)
- chunked-object: fallback checkout codepaths
- bulk-checkin: support chunked-object encoding
- bulk-checkin: allow the same data to be multiply hashed
- new representation types in the packstream
- varint-in-pack: refactor varint encoding/decoding
Not ready.
At least pack-objects and fsck need to learn the new encoding for the
series to be usable locally, and then index-pack/unpack-objects needs to
learn it to be used remotely.
--------------------------------------------------
[Cooking]
* jc/pull-signed-tag (2012-01-11) 1 commit
- merge: use editor by default in interactive sessions
Per Linus's strong suggestion, sugarcoated (aka "taking blame for the
original UI screw-ups") so that it is easier for me to swallow and accept
a potentially huge backward incompatibility issue, "git merge" is made to
launch an editor to explain the merge in the merge commit by default in
interactive sessions.
May need renaming the backward compatibility "GIT_MERGE_LEGACY"
environment variable, and also will need a smoother migration plan.
Will defer till the next cycle.
* nd/commit-ignore-i-t-a (2012-01-16) 2 commits
- commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees
- cache-tree: update API to take abitrary flags
May want to consider this as fixing an earlier UI mistake, and not as a
feature that devides the userbase.
Will defer till the next cycle.
* nd/maint-refname-in-hierarchy-check (2012-01-11) 1 commit
- Fix incorrect ref namespace check
Avoid getting confused by "ref/headxxx" and mistaking it as if it is under
the "refs/heads/" hierarchy.
Not urgent.
* jc/advise-i18n (2011-12-22) 1 commit
- i18n of multi-line advice messages
Allow localization of advice messages that tend to be longer and
multi-line formatted. For now this is deliberately limited to advise()
interface and not vreportf() in general as touching the latter has
interactions with error() that has plumbing callers whose prefix "error: "
should never be translated.
Not urgent.
* rr/sequencer (2012-01-11) 2 commits
- sequencer: factor code out of revert builtin
- revert: prepare to move replay_action to header
Moving large chunk of code out of cherry-pick/revert for later reuse,
primarily to prepare for the next cycle.
Not urgent.
* tr/maint-mailinfo (2012-01-16) 2 commits
- mailinfo: with -b, keep space after [foo]
- am: learn passing -b to mailinfo
Looked reasonable.
Not urgent.
* jk/credentials (2012-01-11) 3 commits
(merged to 'next' on 2012-01-16 at 1c6c94a)
+ unix-socket: do not let close() or chdir() clobber errno during cleanup
+ credential-cache: report more daemon connection errors
+ unix-socket: handle long socket pathnames
Minor fix-ups to the new feature.
Will merge to 'master' by v1.7.9-rc2.
* pw/p4-view-updates (2012-01-11) 5 commits
- git-p4: add tests demonstrating spec overlay ambiguities
- git-p4: adjust test to adhere to stricter useClientSpec
- git-p4: clarify comment
- git-p4: fix verbose comment typo
- git-p4: only a single ... wildcard is supported
* rs/diff-postimage-in-context (2012-01-06) 1 commit
(merged to 'next' on 2012-01-09 at 9635032)
+ xdiff: print post-image for common records instead of pre-image
Looked reasonable.
Not urgent.
* cb/push-quiet (2012-01-08) 3 commits
- t5541: avoid TAP test miscounting
- fix push --quiet: add 'quiet' capability to receive-pack
- server_supports(): parse feature list more carefully
Looked reasonable.
Not urgent.
* nd/clone-detached (2012-01-16) 10 commits
- clone: print advice on checking out detached HEAD
- clone: allow --branch to take a tag
- clone: refuse to clone if --branch points to bogus ref
- clone: --branch=<branch> always means refs/heads/<branch>
- clone: delay cloning until after remote HEAD checking
- clone: factor out remote ref writing
- clone: factor out HEAD update code
- clone: factor out checkout code
- clone: write detached HEAD in bare repositories
- t5601: add missing && cascade
(this branch uses nd/clone-single-branch.)
Looking good.
Not urgent.
* nd/clone-single-branch (2012-01-08) 1 commit
(merged to 'next' on 2012-01-09 at 6c3c759)
+ clone: add --single-branch to fetch only one branch
(this branch is used by nd/clone-detached.)
Looked reasonable.
Not urgent.
* jn/gitweb-unspecified-action (2012-01-09) 1 commit
- gitweb: Fix actionless dispatch for non-existent objects
Looked reasonable.
Not urgent.
* nd/index-pack-no-recurse (2012-01-16) 3 commits
- index-pack: eliminate unlimited recursion in get_base_data()
- index-pack: eliminate recursion in find_unresolved_deltas
- Eliminate recursion in setting/clearing marks in commit list
Much better explained than the previous round.
Will defer till the next cycle.
* mh/ref-api-rest (2011-12-12) 35 commits
- repack_without_ref(): call clear_packed_ref_cache()
- read_packed_refs(): keep track of the directory being worked in
- is_refname_available(): query only possibly-conflicting references
- refs: read loose references lazily
- read_loose_refs(): take a (ref_entry *) as argument
- struct ref_dir: store a reference to the enclosing ref_cache
- sort_ref_dir(): take (ref_entry *) instead of (ref_dir *)
- do_for_each_ref_in_dir*(): take (ref_entry *) instead of (ref_dir *)
- add_entry(): take (ref_entry *) instead of (ref_dir *)
- search_ref_dir(): take (ref_entry *) instead of (ref_dir *)
- find_containing_direntry(): use (ref_entry *) instead of (ref_dir *)
- add_ref(): take (ref_entry *) instead of (ref_dir *)
- read_packed_refs(): take (ref_entry *) instead of (ref_dir *)
- find_ref(): take (ref_entry *) instead of (ref_dir *)
- is_refname_available(): take (ref_entry *) instead of (ref_dir *)
- get_loose_refs(): return (ref_entry *) instead of (ref_dir *)
- get_packed_refs(): return (ref_entry *) instead of (ref_dir *)
- refs: wrap top-level ref_dirs in ref_entries
- get_ref_dir(): keep track of the current ref_dir
- do_for_each_ref(): only iterate over the subtree that was requested
- refs: sort ref_dirs lazily
- sort_ref_dir(): do not sort if already sorted
- refs: store references hierarchically
- refs.c: rename ref_array -> ref_dir
- struct ref_entry: nest the value part in a union
- check_refname_component(): return 0 for zero-length components
- free_ref_entry(): new function
- refs.c: reorder definitions more logically
- is_refname_available(): reimplement using do_for_each_ref_in_array()
- names_conflict(): simplify implementation
- names_conflict(): new function, extracted from is_refname_available()
- repack_without_ref(): reimplement using do_for_each_ref_in_array()
- do_for_each_ref_in_arrays(): new function
- do_for_each_ref_in_array(): new function
- do_for_each_ref(): correctly terminate while processesing extra_refs
The API for extra anchoring points may require rethought first; that would
hopefully make the "ref" part a lot simpler. And that is happening in
another topic (which has graduated to 'master').
Will defer till the next cycle.
* ss/git-svn-prompt-sans-terminal (2012-01-04) 3 commits
- fixup! 15eaaf4
- git-svn, perl/Git.pm: extend Git::prompt helper for querying users
(merged to 'next' on 2012-01-05 at 954f125)
+ perl/Git.pm: "prompt" helper to honor GIT_ASKPASS and SSH_ASKPASS
The bottom one has been replaced with a rewrite based on comments from
Ævar. The second one needs more work, both in perl/Git.pm and prompt.c, to
give precedence to tty over SSH_ASKPASS when terminal is available.
Will defer till the next cycle.
* cb/git-daemon-tests (2012-01-08) 5 commits
(merged to 'next' on 2012-01-08 at 1db8351)
+ git-daemon tests: wait until daemon is ready
+ git-daemon: produce output when ready
+ git-daemon: add tests
+ dashed externals: kill children on exit
+ run-command: optionally kill children on exit
Will defer till the next cycle.
* jk/parse-object-cached (2012-01-06) 3 commits
(merged to 'next' on 2012-01-08 at 8c6fa4a)
+ upload-pack: avoid parsing tag destinations
+ upload-pack: avoid parsing objects during ref advertisement
+ parse_object: try internal cache before reading object db
These are a bit scary changes, but I do think they are worth doing.
Will defer till the next cycle.
^ permalink raw reply
* Re: Signed tags in octopus merge..
From: Junio C Hamano @ 2012-01-17 3:31 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Jacob Helwig, Git Mailing List
In-Reply-To: <CA+55aFzZXSCt1AwOotMZJ+GcNcJKL2OcsPOtaZ3=cvraJ=PD+Q@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Mon, Jan 16, 2012 at 2:58 PM, Jacob Helwig <jacob@technosorcery.net> wrote:
>>
>> My immediate thought regarding the "side branch #1" version is not
>> wanting to have to do the math (even though it's a simple n+1)
>
> "Math is hard, let's go shopping".
>
> But I think even barbie could do the "add one" thing. That said, I
> think the current thing is already more than good enough, and I don't
> think it's at all confusing to talk about "parent #2". In fact, I
> think it's more obvious than "side branch #1".
I think Jacob is correct to point out that mice are even dumber than
Barbie and cannot do the "add one" thing, so let's leave the output as is.
Thanks for an amusing response.
^ permalink raw reply
* Re: Re* Regulator updates for 3.3
From: Pete Harlan @ 2012-01-17 5:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, Mark Brown, Liam Girdwood, linux-kernel,
Git Mailing List
In-Reply-To: <7v62gbussz.fsf@alter.siamese.dyndns.org>
On 01/16/2012 03:33 PM, Junio C Hamano wrote:
> Pete Harlan <pgit@pcharlan.com> writes:
>
>> On 01/10/2012 10:59 PM, Junio C Hamano wrote:
>>> There may be existing scripts that leave the standard input and the
>>> standard output of the "git merge" connected to whatever environment the
>>> scripts were started, and such invocation might trigger the above
>>> "interactive session" heuristics. Such scripts can export GIT_MERGE_LEGACY
>>> environment variable set to "yes" to force the traditional behaviour.
>>
>> The name GIT_MERGE_LEGACY gives no clue about what flavor of legacy
>> merge behavior is being enabled. Something like GIT_MERGE_LEGACY_EDIT
>> might be clearer, or perhaps just have GIT_MERGE_EDIT=0 to get the old
>> behavior without reference to whether or not that behavior is
>> considered legacy.
>
> Hrm.
>
> The only case your suggestion may make a difference would be when we find
> another earlier UI mistake we would want to correct in a backward
> incompatible way that affects _existing_ scripts.
>
> With your suggestion, they need to export "GIT_MERGE_EDIT=0" today, and
> they will need to update again to export "GIT_MERGE_SOMETHINGELSE=0" when
> such an incompatible change comes.
Which is a good thing, because maybe they started using Git after the
current proposed change (which they like), and what you see as new
becomes their "legacy" behavior. If you change something after that,
you can't use GIT_MERGE_LEGACY=yes for that one also because which
legacy is it preserving?
In general, naming configuration variables "DO_IT_<THIS_WAY>" instead
of "DO_IT_THE_OLD_WAY" is better because it's self-documenting. The
only time I think I'd prefer "LEGACY" is if you're planning on
deprecating and removing it eventually and you want to indicate
something to that effect in the name.
--
Pete Harlan
pgit@pcharlan.com
^ permalink raw reply
* [PATCH 0/4] Remove a user of extra_refs in clone
From: mhagger @ 2012-01-17 5:50 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
When cloning, write_remote_refs() creates local packed refs from the
refs read from the remote repository. It does this by creating extra
refs for the references then calling pack_refs() to bake the extra
refs into the packed-refs file, then calling clear_extra_refs().
This is silly and relies on the kludgy extra_refs mechanism, which I
want to get rid of. Instead, add a function call add_packed_ref() to
the refs API, and use it to create packed refs (in the memory cache)
directly. Then call pack_refs() as before to write the packed-refs
file.
Because the new add_packed_ref() function allows references (perhaps
many of them) to be added to an existing ref_array, it would be
inefficient to re-sort the list after every addition. So instead,
append new entries to the end of the ref_array and note that the array
is unsorted. Then, before the ref_array is used, check if it is
unsorted and sort it if necessary.
A side effect of this change is that the new packed references are
left in the in-memory packed reference cache after the return from
write_remote_refs() (whereas previously, the refs were stored as
temporary extra refs that were purged before return from the
function). I can't see any place in the following code where this
would make a difference, but there is quite a bit of code there so it
is hard to audit. Confirmation that this is OK would be welcome.
Michael Haggerty (4):
pack_refs(): remove redundant check
ref_array: keep track of whether references are sorted
add_packed_ref(): new function in the refs API.
write_remote_refs(): create packed (rather than extra) refs
builtin/clone.c | 3 +--
pack-refs.c | 3 +--
refs.c | 39 ++++++++++++++++++++++++++++++++-------
refs.h | 6 ++++++
4 files changed, 40 insertions(+), 11 deletions(-)
--
1.7.8.3
^ permalink raw reply
* [PATCH 2/4] ref_array: keep track of whether references are sorted
From: mhagger @ 2012-01-17 5:50 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Michael Haggerty
In-Reply-To: <1326779434-20106-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Keep track of how many entries in a ref_array are already sorted. In
sort_ref_array(), only call qsort() if the dir contains unsorted
entries (i.e., if references have been appended to the end of the list
since the last call to sort_ref_array()).
Sort ref_arrays only when needed, namely in search_ref_array() and in
do_for_each_ref(). However, never sort the extra_refs, because
extra_refs can contain multiple entries with the same name.
This change is currently not useful, because entries are not added to
ref_arrays after they are created. But in a moment they will be...
Implementation note: we could store a binary "sorted" value instead of
an integer, but storing the number of sorted entries leaves the way
open for a couple of possible future optimizations:
* In sort_ref_array(), sort *only* the unsorted entries, then merge
them with the sorted entries. This should be faster if most of the
entries are already sorted.
* Teach search_ref_array() to do a binary search of any sorted
entries, and if unsuccessful do a linear search of any unsorted
entries. This would avoid the need to sort the list every time that
search_ref_array() is called, and (given some intelligence about how
often to sort) could significantly improve the speed in certain
hypothetical usage patterns.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 33 ++++++++++++++++++++++++++-------
1 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 6f436f1..268816f 100644
--- a/refs.c
+++ b/refs.c
@@ -17,6 +17,15 @@ struct ref_entry {
struct ref_array {
int nr, alloc;
+
+ /*
+ * Entries with index 0 <= i < sorted are sorted by name. New
+ * entries are appended to the list unsorted, and are sorted
+ * only when required; thus we avoid the need to sort the list
+ * after the addition of every reference.
+ */
+ int sorted;
+
struct ref_entry **refs;
};
@@ -105,12 +114,18 @@ static int is_dup_ref(const struct ref_entry *ref1, const struct ref_entry *ref2
}
}
+/*
+ * Sort the entries in array (if they are not already sorted).
+ */
static void sort_ref_array(struct ref_array *array)
{
int i, j;
- /* Nothing to sort unless there are at least two entries */
- if (array->nr < 2)
+ /*
+ * This check also prevents passing a zero-length array to qsort(),
+ * which is a problem on some platforms.
+ */
+ if (array->sorted == array->nr)
return;
qsort(array->refs, array->nr, sizeof(*array->refs), ref_entry_cmp);
@@ -124,7 +139,7 @@ static void sort_ref_array(struct ref_array *array)
}
array->refs[++i] = array->refs[j];
}
- array->nr = i + 1;
+ array->sorted = array->nr = i + 1;
}
static struct ref_entry *search_ref_array(struct ref_array *array, const char *refname)
@@ -137,7 +152,7 @@ static struct ref_entry *search_ref_array(struct ref_array *array, const char *r
if (!array->nr)
return NULL;
-
+ sort_ref_array(array);
len = strlen(refname) + 1;
e = xmalloc(sizeof(struct ref_entry) + len);
memcpy(e->name, refname, len);
@@ -168,6 +183,10 @@ static struct ref_cache {
static struct ref_entry *current_ref;
+/*
+ * The extra_refs is never sorted, because it is allowed to contain entries
+ * with duplicate names.
+ */
static struct ref_array extra_refs;
static void clear_ref_array(struct ref_array *array)
@@ -176,7 +195,7 @@ static void clear_ref_array(struct ref_array *array)
for (i = 0; i < array->nr; i++)
free(array->refs[i]);
free(array->refs);
- array->nr = array->alloc = 0;
+ array->sorted = array->nr = array->alloc = 0;
array->refs = NULL;
}
@@ -268,7 +287,6 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
!get_sha1_hex(refline + 1, sha1))
hashcpy(last->peeled, sha1);
}
- sort_ref_array(array);
}
void add_extra_ref(const char *refname, const unsigned char *sha1, int flag)
@@ -404,7 +422,6 @@ static struct ref_array *get_loose_refs(struct ref_cache *refs)
{
if (!refs->did_loose) {
get_ref_dir(refs, "refs", &refs->loose);
- sort_ref_array(&refs->loose);
refs->did_loose = 1;
}
return &refs->loose;
@@ -720,6 +737,8 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
for (i = 0; i < extra->nr; i++)
retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);
+ sort_ref_array(packed);
+ sort_ref_array(loose);
while (p < packed->nr && l < loose->nr) {
struct ref_entry *entry;
int cmp = strcmp(packed->refs[p]->name, loose->refs[l]->name);
--
1.7.8.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox