* Re: [PATCH 1/2] require pathspec for "git add -u/-A"
From: Jeff King @ 2013-03-13 4:08 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Junio C Hamano, git
In-Reply-To: <vpqhakgiusr.fsf@grenoble-inp.fr>
On Tue, Mar 12, 2013 at 02:58:44PM +0100, Matthieu Moy wrote:
> > I guess we already rejected the idea of being able to shut off the
> > warning and just get the new behavior, in favor of having people
> > specify it manually each time?
>
> Somehow, but we may find a way to do so, as long as it temporary (i.e.
> something that will have no effect after the transition period), and
> that is is crystal clear that it's temporary.
Yeah, I think this is easy as long as it is "enable the new behavior
now" and not "toggle between new and old behavior". That is, a boolean
rather than a selector, with a note that it will go away at the behavior
switch.
The only downside I see is that a user may switch it on now, saying
"Yes, I understand and prefer the new behavior", but some script they
run might not expect it. We can warn against that in the documentation,
but that may or may not be enough.
Here's a series which does that; if it's the direction we want to go, I
think we'd want to rebase Junio's "now add -u is full-tree" patch on
top.
[1/2]: t2200: check that "add -u" limits itself to subdirectory
[2/2]: add: respect add.updateroot config option
-Peff
^ permalink raw reply
* Re: [PATCH] tag: --force is quiet about new tags
From: Junio C Hamano @ 2013-03-13 3:33 UTC (permalink / raw)
To: Phil Hord; +Cc: git, phil.hord, Michael J Gruber
In-Reply-To: <1363130021-28624-1-git-send-email-hordp@cisco.com>
Phil Hord <hordp@cisco.com> writes:
> git tag --force is used to replace an existing tag with
> a new reference. Git helpfully tells the user the old
> ref when this happens. But if the tag name is new and does
> not exist, git tells the user the old ref anyway (000000).
>
> Teach git to ignore --force if the tag is new. Add a test
> for this and also to ensure --force can replace tags at all.
>
> Signed-off-by: Phil Hord <hordp@cisco.com>
> ---
I think we would still want to allow the operation to go through,
even when the --force option is given, to create a new tag. I agree
that the message should not say "Updated". So teaching Git not to
issue the "Updated" message makes perfect sense. It is somewhat
misleading to say we are teaching Git to ignore the option, though.
Thanks.
^ permalink raw reply
* Re: linux-next: unneeded merge in the security tree
From: Junio C Hamano @ 2013-03-13 3:17 UTC (permalink / raw)
To: Theodore Ts'o
Cc: Linus Torvalds, James Morris, Stephen Rothwell, linux-next,
Linux Kernel Mailing List, Git Mailing List
In-Reply-To: <20130313023026.GD16919@thunk.org>
Theodore Ts'o <tytso@mit.edu> writes:
> On Tue, Mar 12, 2013 at 02:30:04PM -0700, Junio C Hamano wrote:
>> Theodore Ts'o <tytso@mit.edu> writes:
>>
>> > [remote "origin"]
>> > url = git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>> > fetch = +refs/heads/master:refs/heads/master
>> > mergeoptions = --ff-only
>> >
>>
>> Is there an escape hatch for that rare case? IOW, how does a
>> submaintainer who configured the above to override --ff-only?
>
> Hmm, maybe we would need to add a --no-ff-only? Or they could just
> do:
>
> git fetch origin
> git merge FETCH_HEAD
Hmm, neither feel quite nice.
I haven't heard any comments on my alternative proposal, by the
way. Did the message get lost?
^ permalink raw reply
* Re: linux-next: unneeded merge in the security tree
From: Theodore Ts'o @ 2013-03-13 2:30 UTC (permalink / raw)
To: Linus Torvalds, Junio C Hamano
Cc: James Morris, Stephen Rothwell, linux-next,
Linux Kernel Mailing List, Git Mailing List
In-Reply-To: <CA+55aFwHJtOU4Qzt3XZsER165kTc5P0ATQP2wPHvuUiVic8bnA@mail.gmail.com>
On Tue, Mar 12, 2013 at 02:30:04PM -0700, Junio C Hamano wrote:
> Theodore Ts'o <tytso@mit.edu> writes:
>
> > [remote "origin"]
> > url = git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> > fetch = +refs/heads/master:refs/heads/master
> > mergeoptions = --ff-only
> >
>
> Is there an escape hatch for that rare case? IOW, how does a
> submaintainer who configured the above to override --ff-only?
Hmm, maybe we would need to add a --no-ff-only? Or they could just
do:
git fetch origin
git merge FETCH_HEAD
On Tue, Mar 12, 2013 at 02:28:39PM -0700, Linus Torvalds wrote:
>
> Of course, I'm not really sure if we want to list the flags. Maybe
> it's better to just introduce the notion of "upstream" directly, and
> make that a flag, and make "origin" default to that when you clone.
> And then have git use different heurstics for pulling upstream (like
> warning by default when doing a back-merge, perhaps?)
What if git automaticallly set up the origin branch to have a certain
set of mergeoptions by default? That would probably be right for most
users, but it makes it obvious what's going on when they take a look
at the .git/config file, and doesn't make the remote that happens to
have the name "origin" as having certain magic properties. Using a
set of mergeoptions would also be bit more general, and might have
applications in the future.
- Ted
^ permalink raw reply
* Re: [PATCH v3 1/3] mergetools/p4merge: swap LOCAL and REMOTE
From: David Aguilar @ 2013-03-13 2:05 UTC (permalink / raw)
To: Kevin Bracey; +Cc: git, Ciaran Jessup, Scott Chacon, Alex Riesen
In-Reply-To: <1363137142-18606-1-git-send-email-kevin@bracey.fi>
On Tue, Mar 12, 2013 at 6:12 PM, Kevin Bracey <kevin@bracey.fi> wrote:
> Reverse LOCAL and REMOTE when invoking P4Merge as a mergetool, so that
> the incoming branch is now in the left-hand, blue triangle pane, and the
> current branch is in the right-hand, green circle pane.
>
> This change makes use of P4Merge consistent with its built-in help, its
> reference documentation, and Perforce itself. But most importantly, it
> makes merge results clearer. P4Merge is not totally symmetrical between
> left and right; despite changing a few text labels from "theirs/ours" to
> "left/right" when invoked manually, it still retains its original
> Perforce "theirs/ours" viewpoint.
>
> Most obviously, in the result pane P4Merge shows changes that are common
> to both branches in green. This is on the basis of the current branch
> being green, as it is when invoked from Perforce; it means that lines in
> the result are blue if and only if they are being changed by the merge,
> making the resulting diff clearer.
>
> Note that P4Merge now shows "ours" on the right for both diff and merge,
> unlike other diff/mergetools, which always have REMOTE on the right.
> But observe that REMOTE is the working tree (ie "ours") for a diff,
> while it's another branch (ie "theirs") for a merge.
>
> Ours and theirs are reversed for a rebase - see "git help rebase".
> However, this does produce the desired "show the results of this commit"
> effect in P4Merge - changes that remain in the rebased commit (in your
> branch, but not in the new base) appear in blue; changes that do not
> appear in the rebased commit (from the new base, or common to both) are
> in green. If Perforce had rebase, they'd probably not swap ours/theirs,
> but make P4Merge show common changes in blue, picking out our changes in
> green. We can't do that, so this is next best.
>
> Signed-off-by: Kevin Bracey <kevin@bracey.fi>
> ---
This seems sensible to apply. The commit message is a bit long,
but I think it's justified since this is exactly the kind of thing
I would tend to forget after enough time has passed.
Ditto on the create_virtual_base patch. Your latest patch
addressed Junio's note about making it take 2 args.
FWIW, please feel free to add:
Reviewed-by: David Aguilar <davvid@gmail.com>
Thanks.
> mergetools/p4merge | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mergetools/p4merge b/mergetools/p4merge
> index 8a36916..46b3a5a 100644
> --- a/mergetools/p4merge
> +++ b/mergetools/p4merge
> @@ -22,7 +22,7 @@ diff_cmd () {
> merge_cmd () {
> touch "$BACKUP"
> $base_present || >"$BASE"
> - "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED"
> + "$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED"
> check_unchanged
> }
>
> --
> 1.8.2.rc3.7.g1100d09.dirty
>
--
David
^ permalink raw reply
* [PATCH v3 1/3] mergetools/p4merge: swap LOCAL and REMOTE
From: Kevin Bracey @ 2013-03-13 1:12 UTC (permalink / raw)
To: git; +Cc: David Aguilar, Ciaran Jessup, Scott Chacon, Alex Riesen,
Kevin Bracey
In-Reply-To: <1362601978-16911-1-git-send-email-kevin@bracey.fi>
Reverse LOCAL and REMOTE when invoking P4Merge as a mergetool, so that
the incoming branch is now in the left-hand, blue triangle pane, and the
current branch is in the right-hand, green circle pane.
This change makes use of P4Merge consistent with its built-in help, its
reference documentation, and Perforce itself. But most importantly, it
makes merge results clearer. P4Merge is not totally symmetrical between
left and right; despite changing a few text labels from "theirs/ours" to
"left/right" when invoked manually, it still retains its original
Perforce "theirs/ours" viewpoint.
Most obviously, in the result pane P4Merge shows changes that are common
to both branches in green. This is on the basis of the current branch
being green, as it is when invoked from Perforce; it means that lines in
the result are blue if and only if they are being changed by the merge,
making the resulting diff clearer.
Note that P4Merge now shows "ours" on the right for both diff and merge,
unlike other diff/mergetools, which always have REMOTE on the right.
But observe that REMOTE is the working tree (ie "ours") for a diff,
while it's another branch (ie "theirs") for a merge.
Ours and theirs are reversed for a rebase - see "git help rebase".
However, this does produce the desired "show the results of this commit"
effect in P4Merge - changes that remain in the rebased commit (in your
branch, but not in the new base) appear in blue; changes that do not
appear in the rebased commit (from the new base, or common to both) are
in green. If Perforce had rebase, they'd probably not swap ours/theirs,
but make P4Merge show common changes in blue, picking out our changes in
green. We can't do that, so this is next best.
Signed-off-by: Kevin Bracey <kevin@bracey.fi>
---
mergetools/p4merge | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 8a36916..46b3a5a 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -22,7 +22,7 @@ diff_cmd () {
merge_cmd () {
touch "$BACKUP"
$base_present || >"$BASE"
- "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED"
+ "$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED"
check_unchanged
}
--
1.8.2.rc3.7.g1100d09.dirty
^ permalink raw reply related
* Re: [PATCH v3 05/13] match_{base,path}name: replace strncmp_icase with memequal_icase
From: Duy Nguyen @ 2013-03-13 1:14 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1363093500-16796-6-git-send-email-pclouds@gmail.com>
On Tue, Mar 12, 2013 at 8:04 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> +static inline int memequal_icase(const char *a, const char *b, int n)
> +{
> + if (!memcmp(a, b, n))
> + return 1;
> +
> + if (!ignore_case)
> + return 0;
> +
> + /*
> + * This assumes that ASCII is used in most repositories. We
> + * try the ascii-only version first (i.e. Git's
> + * toupper). Failing that, fall back to normal strncasecmp.
> + */
> + while (n && toupper(*a) == toupper(*b)) {
> + a++;
> + b++;
> + n--;
> + }
> + if (!n)
> + return 1;
> + return !strncasecmp(a, b, n);
> +}
Note, the ignore_case == 1 codepath was not tested and measured. I
suspect that strncasecmp may be more complex to support locales and
the "LANG=C" version should suffice in most case. But it's just
guesses at this moment.
--
Duy
^ permalink raw reply
* [PATCH v3 3/3] git-merge-one-file: revise merge error reporting
From: Kevin Bracey @ 2013-03-13 1:12 UTC (permalink / raw)
To: git; +Cc: David Aguilar, Ciaran Jessup, Scott Chacon, Alex Riesen,
Kevin Bracey
In-Reply-To: <1363137142-18606-1-git-send-email-kevin@bracey.fi>
Commit 718135e improved the merge error reporting for the resolve
strategy's merge conflict and permission conflict cases, but led to a
malformed "ERROR: in myfile.c" message in the case of a file added
differently.
This commit reverts that change, and uses an alternative approach without
this flaw.
Signed-off-by: Kevin Bracey <kevin@bracey.fi>
---
git-merge-one-file.sh | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 0f164e5..78b07a8 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -104,11 +104,13 @@ case "${1:-.}${2:-.}${3:-.}" in
;;
esac
+ ret=0
src1=$(git-unpack-file $2)
src2=$(git-unpack-file $3)
case "$1" in
'')
- echo "Added $4 in both, but differently."
+ echo "ERROR: Added $4 in both, but differently."
+ ret=1
orig=$(git-unpack-file $2)
create_virtual_base "$orig" "$src2"
;;
@@ -121,10 +123,9 @@ case "${1:-.}${2:-.}${3:-.}" in
# Be careful for funny filename such as "-L" in "$4", which
# would confuse "merge" greatly.
git merge-file "$src1" "$orig" "$src2"
- ret=$?
- msg=
- if [ $ret -ne 0 ]; then
- msg='content conflict'
+ if [ $? -ne 0 ]; then
+ echo "ERROR: Content conflict in $4"
+ ret=1
fi
# Create the working tree file, using "our tree" version from the
@@ -133,18 +134,11 @@ case "${1:-.}${2:-.}${3:-.}" in
rm -f -- "$orig" "$src1" "$src2"
if [ "$6" != "$7" ]; then
- if [ -n "$msg" ]; then
- msg="$msg, "
- fi
- msg="${msg}permissions conflict: $5->$6,$7"
- ret=1
- fi
- if [ "$1" = '' ]; then
+ echo "ERROR: Permissions conflict: $5->$6,$7"
ret=1
fi
if [ $ret -ne 0 ]; then
- echo "ERROR: $msg in $4"
exit 1
fi
exec git update-index -- "$4"
--
1.8.2.rc3.7.g1100d09.dirty
^ permalink raw reply related
* [PATCH v3 2/3] mergetools/p4merge: create a base if none available
From: Kevin Bracey @ 2013-03-13 1:12 UTC (permalink / raw)
To: git; +Cc: David Aguilar, Ciaran Jessup, Scott Chacon, Alex Riesen,
Kevin Bracey
In-Reply-To: <1363137142-18606-1-git-send-email-kevin@bracey.fi>
Originally, with no base, Git gave P4Merge $LOCAL as a dummy base:
p4merge "$LOCAL" "$LOCAL" "$REMOTE" "$MERGED"
Commit 0a0ec7bd changed this to:
p4merge "empty file" "$LOCAL" "$REMOTE" "$MERGED"
to avoid the problem of being unable to save in some circumstances with
similar inputs.
Unfortunately this approach produces much worse results on differing
inputs. P4Merge really regards the blank file as the base, and once you
have just a couple of differences between the two branches you end up
with one a massive full-file conflict. The 3-way diff is not readable,
and you have to invoke "difftool MERGE_HEAD HEAD" manually to get a
useful view.
The original approach appears to have invoked special 2-way merge
behaviour in P4Merge that occurs only if the base filename is "" or
equal to the left input. You get a good visual comparison, and it does
not auto-resolve differences. (Normally if one branch matched the base,
it would autoresolve to the other branch).
But there appears to be no way of getting this 2-way behaviour and being
able to reliably save. Having base==left appears to be triggering other
assumptions. There are tricks the user can use to force the save icon
on, but it's not intuitive.
So we now follow a suggestion given in the original patch's discussion:
generate a virtual base, consisting of the lines common to the two
branches. This is the same as the technique used in resolve and octopus
merges, so we relocate that code to a shared function.
Note that if there are no differences at the same location, this
technique can lead to automatic resolution without conflict, combining
everything from the 2 files. As with the other merges using this
technique, we assume the user will inspect the result before saving.
Signed-off-by: Kevin Bracey <kevin@bracey.fi>
---
Documentation/git-sh-setup.txt | 6 ++++++
git-merge-one-file.sh | 18 +++++-------------
git-sh-setup.sh | 12 ++++++++++++
mergetools/p4merge | 6 +++++-
4 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 6a9f66d..5d709d0 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -82,6 +82,12 @@ get_author_ident_from_commit::
outputs code for use with eval to set the GIT_AUTHOR_NAME,
GIT_AUTHOR_EMAIL and GIT_AUTHOR_DATE variables for a given commit.
+create_virtual_base::
+ modifies the first file so only lines in common with the
+ second file remain. If there is insufficient common material,
+ then the first file is left empty. The result is suitable
+ as a virtual base input for a 3-way merge.
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index f612cb8..0f164e5 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -104,30 +104,22 @@ case "${1:-.}${2:-.}${3:-.}" in
;;
esac
- src2=`git-unpack-file $3`
+ src1=$(git-unpack-file $2)
+ src2=$(git-unpack-file $3)
case "$1" in
'')
echo "Added $4 in both, but differently."
- # This extracts OUR file in $orig, and uses git apply to
- # remove lines that are unique to ours.
- orig=`git-unpack-file $2`
- sz0=`wc -c <"$orig"`
- @@DIFF@@ -u -La/$orig -Lb/$orig $orig $src2 | git apply --no-add
- sz1=`wc -c <"$orig"`
-
- # If we do not have enough common material, it is not
- # worth trying two-file merge using common subsections.
- expr $sz0 \< $sz1 \* 2 >/dev/null || : >$orig
+ orig=$(git-unpack-file $2)
+ create_virtual_base "$orig" "$src2"
;;
*)
echo "Auto-merging $4"
- orig=`git-unpack-file $1`
+ orig=$(git-unpack-file $1)
;;
esac
# Be careful for funny filename such as "-L" in "$4", which
# would confuse "merge" greatly.
- src1=`git-unpack-file $2`
git merge-file "$src1" "$orig" "$src2"
ret=$?
msg=
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 795edd2..349a5d4 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -249,6 +249,18 @@ clear_local_git_env() {
unset $(git rev-parse --local-env-vars)
}
+# Generate a virtual base file for a two-file merge. Uses git apply to
+# remove lines from $1 that are not in $2, leaving only common lines.
+create_virtual_base() {
+ sz0=$(wc -c <"$1")
+ @@DIFF@@ -u -La/"$1" -Lb/"$1" "$1" "$2" | git apply --no-add
+ sz1=$(wc -c <"$1")
+
+ # If we do not have enough common material, it is not
+ # worth trying two-file merge using common subsections.
+ expr $sz0 \< $sz1 \* 2 >/dev/null || : >"$1"
+}
+
# Platform specific tweaks to work around some commands
case $(uname -s) in
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 46b3a5a..5a608ab 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -21,7 +21,11 @@ diff_cmd () {
merge_cmd () {
touch "$BACKUP"
- $base_present || >"$BASE"
+ if ! $base_present
+ then
+ cp -- "$LOCAL" "$BASE"
+ create_virtual_base "$BASE" "$REMOTE"
+ fi
"$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED"
check_unchanged
}
--
1.8.2.rc3.7.g1100d09.dirty
^ permalink raw reply related
* Re: [PATCH v2 3/6] match_basename: use strncmp instead of strcmp
From: Duy Nguyen @ 2013-03-13 1:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7glc72rn.fsf@alter.siamese.dyndns.org>
On Wed, Mar 13, 2013 at 3:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> strncmp is provided length information which could be taken advantage
>>> by the underlying implementation.
>
> After all, strcmp() could also be optimized to fetch word-at-a-time
> while being careful about not overstepping the page boundary.
It boils down to giving more information to the underlying
implementation with hope that it can be useful for something. Although
at this point I think strncmp vs strcmp vs memcmp may be not worth
doing (we keep explicit length comparison to reduce *cmp calls
though). The gain is relatively small and will become even smaller
after we avoid running exclude on tracked files (which eliminates like
2/3 of the processed entries).
--
Duy
^ permalink raw reply
* Re: [PATCH v3 04/13] match_basename: use strncmp instead of strcmp
From: Duy Nguyen @ 2013-03-13 1:05 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git
In-Reply-To: <CALWbr2w71DsORxsFBBj4N2wzYQomjvCqxGfovbTDjGAT9eASyQ@mail.gmail.com>
On Wed, Mar 13, 2013 at 12:40 AM, Antoine Pelisse <apelisse@gmail.com> wrote:
>> --- a/dir.c
>> +++ b/dir.c
>> @@ -636,12 +636,14 @@ int match_basename(const char *basename, int basenamelen,
>> int flags)
>> {
>> if (prefix == patternlen) {
>> - if (!strcmp_icase(pattern, basename))
>> + if (patternlen == basenamelen &&
>> + !strncmp_icase(pattern, basename, patternlen))
>> return 1;
>> } else if (flags & EXC_FLAG_ENDSWITH) {
>> if (patternlen - 1 <= basenamelen &&
>> - !strcmp_icase(pattern + 1,
>> - basename + basenamelen - patternlen + 1))
>> + !strncmp_icase(pattern + 1,
>> + basename + basenamelen - patternlen + 1,
>> + patternlen - 1))
>> return 1;
>
>
> I can see that you kept strncmp(), was it worse with memcmp() ?
One step at a time. 05/13 replace strncmp_icase with memcmp (for when
ignore_case == 0).
--
Duy
^ permalink raw reply
* Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline
From: Duy Nguyen @ 2013-03-13 1:03 UTC (permalink / raw)
To: Karsten Blees
Cc: Ramkumar Ramachandra, Git List, Junio C Hamano,
Torsten Bögershausen, Robert Zeh, Jeff King, Erik Faye-Lund,
Drew Northup
In-Reply-To: <513FB85C.5010106@gmail.com>
On Wed, Mar 13, 2013 at 6:21 AM, Karsten Blees <karsten.blees@gmail.com> wrote:
> Hmmm...I don't see how filesystem changes since last invocation can solve the problem, or am I missing something? I think what you mean to say is that the daemon should keep track of the filesystem *state* of the working copy, or alternatively the deltas/changes to some known state (such as .git/index)?
I think git process can keep track of filesystem state (and save it
down if necessary). But when git process is not running, system state
changes and it cannot know about. The daemon helps filling this gap
(and basically keeps git "running" (in a light form) throughout a
development session). For example if we know only 5 files have changed
since the last refresh, we only need to re-stat those 5. The same for
untracked/ignored file checking,
> I'm also still skeptical whether a daemon will improve overall performance. In my understanding its essentially a filesystem cache in user-mode. The difference to using the OS filesystem cache directly (via lstat/readdir) is that we replace ~50k sys-calls with a single IPC call (i.e. the git <--> fswatch daemon communication is less 'chatty'). However, the 'chattyness' is still there between the fswatch daemon and the OS / inotify.
I think it attempts to reduce unnecessary system calls, not eliminate
them all. In the "5 changed files" above, a few IPC calls are done to
retrieve the file list, then 5 lstat will be issued (by git, not the
daemon) instead of thousands of them.
>Consider 'git status; make; make clean; git status'...that's a *lot* of changes to process for nothing (potentially slowing down make).
Yeah. In my opinion, the daemon should realize that at some point
accumulated changes are too much that it's not worth collecting
anymore, and drop them all. Git will do it the normal/slow way. After
that the daemon picks up again. We only optimize for the case when
little changes are made in filesystem.
> Then there's the issue of stale data in the cache. Modifying porcelain commands that use 'git status --porcelain' to compile their changesets will want 100% exact data. I'm not saying its not doable, but adding another platform specific, caching daemon to the tool chain doesn't exactly simplify things...
>
> But perhaps I'm too pessimistic (or just stigmatized by inherently slow and out-of-date TGitCache/TSvnCache on Windows :-)
Thanks. I didn't know about TGitCache. Will dig it up. Maybe we can
learn something from it (or realize the daemon approach is futile
after all).
--
Duy
^ permalink raw reply
* Re: difftool -d symlinks, under what conditions
From: Matt McClure @ 2013-03-13 0:56 UTC (permalink / raw)
To: John Keeping
Cc: Junio C Hamano, David Aguilar, git@vger.kernel.org, Tim Henigan
In-Reply-To: <20130313001758.GH2317@serenity.lan>
On Tue, Mar 12, 2013 at 8:17 PM, John Keeping <john@keeping.me.uk> wrote:
> Does this work for your original scenario?
Yes. Thanks!
--
Matt McClure
http://www.matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure
^ permalink raw reply
* Re: [PATCH] difftool: Make directory diff symlink work tree
From: Matt McClure @ 2013-03-13 0:26 UTC (permalink / raw)
To: John Keeping; +Cc: David Aguilar, git@vger.kernel.org, Tim Henigan
In-Reply-To: <20130312192459.GD2317@serenity.lan>
On Tue, Mar 12, 2013 at 3:24 PM, John Keeping <john@keeping.me.uk> wrote:
> When I tried this I got the expected behaviour even without this patch.
>
> It turns out that an uncommitted, but *staged* change emits the SHA1 of
> the blob rather than the null SHA1. Do you get the desired behaviour if
> you "git reset" before using difftool?
I tried this:
$ git diff --raw HEAD^
:100644 100644 f35de0f... ead9399... M diff-lib.c
$ git reset HEAD^
Unstaged changes after reset:
M diff-lib.c
$ git diff --raw
:100644 100644 f35de0f... 0000000... M diff-lib.c
$ git difftool -d
and the last command did indeed create symlinks into my working tree
rather than file copies.
So... it seems that using git-reset is at least a workaround to get
the symlink behavior I want as a user, though the dance I have to do
is a little more awkward than `git difftool -d HEAD^` would be.
> If so I think you want some new mode of operation for difftool instead
> of this patch which will also affect unrelated commands.
Are you suggesting that difftool do the reset work above given a new
option or by default?
--
Matt McClure
http://www.matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure
^ permalink raw reply
* Re: difftool -d symlinks, under what conditions
From: John Keeping @ 2013-03-13 0:17 UTC (permalink / raw)
To: Matt McClure
Cc: Junio C Hamano, David Aguilar, git@vger.kernel.org, Tim Henigan
In-Reply-To: <3222724986386016520@unknownmsgid>
On Tue, Mar 12, 2013 at 04:48:16PM -0600, Matt McClure wrote:
> On Mar 12, 2013, at 4:16 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> > Matt McClure <matthewlmcclure@gmail.com> writes:
> >
> > * If you are comparing two trees, and especially if your RHS is not
> > HEAD, you will send everything to a temporary without
> > symlinks. Any edit made by the user will be lost.
>
> I think you're suggesting to use a symlink any time the content of any
> given RHS revision is the same as the working tree.
>
> I imagine that might confuse me as a user. It would create
> circumstances where some files are symlinked and others aren't for
> reasons that won't be straightforward.
>
> I imagine solving that case, I might instead implement a copy back to
> the working tree with conflict detection/resolution. Some earlier
> iterations of the directory diff feature used copy back without
> conflict detection and created situations where I clobbered my own
> changes by finishing a directory diff after making edits concurrently.
The code to copy back working tree files is already there, it just
triggers using the same logic as the creation of symlinks in the first
place and doesn't attempt any conflict detection. I suspect that any
more comprehensive solution will need to restrict the use of "git
difftool -d" whenever the index contains unmerged entries or when there
are both staged and unstaged changes, since the merge resolution will
cause these states to be lost.
The implementation of Junio's suggestion is relatively straightforward
(this is untested, although t7800 passes, and can probably be improved
by someone better versed in Perl). Does this work for your original
scenario?
-- >8 --
diff --git a/git-difftool.perl b/git-difftool.perl
index 0a90de4..5f093ae 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -83,6 +83,21 @@ sub exit_cleanup
exit($status | ($status >> 8));
}
+sub use_wt_file
+{
+ my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
+ my $null_sha1 = '0' x 40;
+
+ if ($sha1 eq $null_sha1) {
+ return 1;
+ } elsif (not $symlinks) {
+ return 0;
+ }
+
+ my $wt_sha1 = $repo->command_oneline('hash-object', "$workdir/$file");
+ return $sha1 eq $wt_sha1;
+}
+
sub setup_dir_diff
{
my ($repo, $workdir, $symlinks) = @_;
@@ -159,10 +174,10 @@ EOF
}
if ($rmode ne $null_mode) {
- if ($rsha1 ne $null_sha1) {
- $rindex .= "$rmode $rsha1\t$dst_path\0";
- } else {
+ if (use_wt_file($repo, $workdir, $dst_path, $rsha1, $symlinks)) {
push(@working_tree, $dst_path);
+ } else {
+ $rindex .= "$rmode $rsha1\t$dst_path\0";
}
}
}
--
1.8.2.rc2.4.g7799588
^ permalink raw reply related
* Re: [PATCH 14/19] Document pull-all and push-all
From: Paul Campbell @ 2013-03-13 0:13 UTC (permalink / raw)
To: Holger Hellmuth (IKS)
Cc: git, David Michael Barr, Kindjal, bibendi, Herman van Rink,
mhoffman, Nate Jones
In-Reply-To: <513F45DE.6010304@ira.uka.de>
On Tue, Mar 12, 2013 at 3:12 PM, Holger Hellmuth (IKS)
<hellmuth@ira.uka.de> wrote:
> Am 09.03.2013 20:28, schrieb Paul Campbell:
>
>> From 7dcd40ab8687a588b7b0c6ff914a7cfb601b6774 Mon Sep 17 00:00:00 2001
>> From: Herman van Rink <rink@initfour.nl>
>> Date: Tue, 27 Mar 2012 13:59:16 +0200
>> Subject: [PATCH 14/19] Document pull-all and push-all
>>
>> ---
>> contrib/subtree/git-subtree.txt | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/contrib/subtree/git-subtree.txt
>> b/contrib/subtree/git-subtree.txt
>> index e0957ee..c8fc103 100644
>> --- a/contrib/subtree/git-subtree.txt
>> +++ b/contrib/subtree/git-subtree.txt
>> @@ -92,13 +92,19 @@ pull::
>> Exactly like 'merge', but parallels 'git pull' in that
>> it fetches the given commit from the specified remote
>> repository.
>> -
>> +
>> +pull-all::
>> + Perform a pull operation on all in .gittrees registered subtrees.
>> +
>> push::
>> Does a 'split' (see below) using the <prefix> supplied
>> and then does a 'git push' to push the result to the
>> repository and refspec. This can be used to push your
>> subtree to different branches of the remote repository.
>>
>> +push-all::
>> + Perform a pull operation on all in .gittrees registered subtrees.
>
> -----
> pull->push
>
Thanks.
>> +
>> split::
>> Extract a new, synthetic project history from the
>> history of the <prefix> subtree. The new history
>>
>
--
Paul [W] Campbell
^ permalink raw reply
* Re: Questions/investigations on git-subtree and tags
From: Paul Campbell @ 2013-03-12 23:57 UTC (permalink / raw)
To: Jeremy Rosen; +Cc: Junio C Hamano, git
In-Reply-To: <2131964232.420820.1363082545306.JavaMail.root@openwide.fr>
On Tue, Mar 12, 2013 at 10:02 AM, Jeremy Rosen <jeremy.rosen@openwide.fr> wrote:
> Paul, I'm not quite sure where I should go from here...
>
> should I send you a patch so you make it a V3 of your patch ? should I
> send a patch superseeding yours ?
>
> I have also found a similar problem in git-subtree pull, which needs
> the same fix.
>
> in the mean time, attached is the current version of my changes
>
> Cordialement
>
> Jérémy Rosen
>
> fight key loggers : write some perl using vim
>
Thanks Jérémy.
Nothing in your patches are dependant on anything I'm working on. I'll
gladly take them for my own tree but feel free to submit them (inline,
rather than as attachments) to the list and cc: David Greene
<greened@obbligato.org> the subtree area maintainer.
--
Paul [W] Campbell
^ permalink raw reply
* Re: [PATCH] difftool: Make directory diff symlink work tree
From: John Keeping @ 2013-03-12 23:40 UTC (permalink / raw)
To: Matt McClure; +Cc: David Aguilar, git@vger.kernel.org, Tim Henigan
In-Reply-To: <7906294865355046191@unknownmsgid>
On Tue, Mar 12, 2013 at 05:12:28PM -0600, Matt McClure wrote:
> On Mar 12, 2013, at 1:25 PM, John Keeping <john@keeping.me.uk> wrote:
>
> > When I tried this I got the expected behaviour even without this patch.
>
> git diff --raw commit
>
> emits the null SHA1 if the working tree file's stat differs from the
> blob corresponding to commit. Is that the case you observed?
Yes, although it's slightly more subtle than that - the null SHA1 only
occurs if the working tree file has unstaged changes; if you add the
changes to the index then the null SHA1 is no longer used since the blob
is now available in Git's object store.
John
^ permalink raw reply
* [PATCH] tag: --force is quiet about new tags
From: Phil Hord @ 2013-03-12 23:13 UTC (permalink / raw)
To: git; +Cc: phil.hord, Michael J Gruber, Junio C Hamano, Phil Hord
git tag --force is used to replace an existing tag with
a new reference. Git helpfully tells the user the old
ref when this happens. But if the tag name is new and does
not exist, git tells the user the old ref anyway (000000).
Teach git to ignore --force if the tag is new. Add a test
for this and also to ensure --force can replace tags at all.
Signed-off-by: Phil Hord <hordp@cisco.com>
---
builtin/tag.c | 2 +-
t/t7004-tag.sh | 12 ++++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index f826688..af3af3f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -582,7 +582,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die(_("%s: cannot lock the ref"), ref.buf);
if (write_ref_sha1(lock, object, NULL) < 0)
die(_("%s: cannot update the ref"), ref.buf);
- if (force && hashcmp(prev, object))
+ if (force && !is_null_sha1(prev) && hashcmp(prev, object))
printf(_("Updated tag '%s' (was %s)\n"), tag, find_unique_abbrev(prev, DEFAULT_ABBREV));
strbuf_release(&buf);
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index f5a79b1..c8d6e9f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -104,6 +104,18 @@ test_expect_success 'creating a tag using HEAD directly should succeed' '
tag_exists myhead
'
+test_expect_success '--force can create a tag with the name of one existing' '
+ tag_exists mytag &&
+ git tag --force mytag &&
+ tag_exists mytag'
+
+test_expect_success '--force is moot with a non-existing tag name' '
+ git tag newtag >expect &&
+ git tag --force forcetag >actual &&
+ test_cmp expect actual
+'
+git tag -d newtag forcetag
+
# deleting tags:
test_expect_success 'trying to delete an unknown tag should fail' '
--
1.8.2.rc3.394.g2617418.dirty
^ permalink raw reply related
* Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline
From: Karsten Blees @ 2013-03-12 23:21 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Duy Nguyen, Junio C Hamano, Torsten Bögershausen,
Robert Zeh, Jeff King, Erik Faye-Lund, Drew Northup
In-Reply-To: <1362946623-23649-1-git-send-email-artagnon@gmail.com>
Am 10.03.2013 21:17, schrieb Ramkumar Ramachandra:
> git operations are slow on repositories with lots of files, and lots
> of tiny filesystem calls like lstat(), getdents(), open() are
> reposible for this. On the linux-2.6 repository, for instance, the
> numbers for "git status" look like this:
>
> top syscalls sorted top syscalls sorted
> by acc. time by number
> ----------------------------------------------
> 0.401906 40950 lstat 0.401906 40950 lstat
> 0.190484 5343 getdents 0.150055 5374 open
> 0.150055 5374 open 0.190484 5343 getdents
> 0.074843 2806 close 0.074843 2806 close
> 0.003216 157 read 0.003216 157 read
>
> To solve this problem, we propose to build a daemon which will watch
> the filesystem using inotify and report batched up events over a UNIX
> socket.
[...]
> +
> +The credential C API is meant to be called by Git code which needs
> +information aboutx filesystem changes. It is centered around an
> +object representing the changes the filesystem since the last
> +invocation.
> +
Hmmm...I don't see how filesystem changes since last invocation can solve the problem, or am I missing something? I think what you mean to say is that the daemon should keep track of the filesystem *state* of the working copy, or alternatively the deltas/changes to some known state (such as .git/index)?
I'm also still skeptical whether a daemon will improve overall performance. In my understanding its essentially a filesystem cache in user-mode. The difference to using the OS filesystem cache directly (via lstat/readdir) is that we replace ~50k sys-calls with a single IPC call (i.e. the git <--> fswatch daemon communication is less 'chatty'). However, the 'chattyness' is still there between the fswatch daemon and the OS / inotify. Consider 'git status; make; make clean; git status'...that's a *lot* of changes to process for nothing (potentially slowing down make).
Then there's the issue of stale data in the cache. Modifying porcelain commands that use 'git status --porcelain' to compile their changesets will want 100% exact data. I'm not saying its not doable, but adding another platform specific, caching daemon to the tool chain doesn't exactly simplify things...
But perhaps I'm too pessimistic (or just stigmatized by inherently slow and out-of-date TGitCache/TSvnCache on Windows :-)
^ permalink raw reply
* Re: [PATCH v3 09/13] exclude: filter out patterns not applicable to the current directory
From: Eric Sunshine @ 2013-03-12 23:13 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1363093500-16796-10-git-send-email-pclouds@gmail.com>
On Tue, Mar 12, 2013 at 9:04 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> The algorithm implemented here is a naive one. Patterns can be either
> active or passive:
>
> - When we enter a new directory (e.g. from root to foo), currently
> active patterns may no longer be applicable and can be turned to
> passive.
>
> - On the opposite, when we leave a directory (foo back to roo),
s/roo/root/
Also, perhaps you meant s/On/Or/ ?
> +/*
> + * If pushed is non-zero, we have entered a new directory. Some
> + * pathname patterns may no longer applicable. Go over all active
s/applicable/be applicable/
-- ES
^ permalink raw reply
* Re: [PATCH] difftool: Make directory diff symlink work tree
From: Matt McClure @ 2013-03-12 23:12 UTC (permalink / raw)
To: John Keeping; +Cc: David Aguilar, git@vger.kernel.org, Tim Henigan
In-Reply-To: <20130312192459.GD2317@serenity.lan>
On Mar 12, 2013, at 1:25 PM, John Keeping <john@keeping.me.uk> wrote:
> When I tried this I got the expected behaviour even without this patch.
git diff --raw commit
emits the null SHA1 if the working tree file's stat differs from the
blob corresponding to commit. Is that the case you observed?
^ permalink raw reply
* Re: difftool -d symlinks, under what conditions
From: Matt McClure @ 2013-03-12 22:48 UTC (permalink / raw)
To: Junio C Hamano
Cc: John Keeping, David Aguilar, git@vger.kernel.org, Tim Henigan
In-Reply-To: <7vehfk5kn2.fsf@alter.siamese.dyndns.org>
On Mar 12, 2013, at 4:16 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Matt McClure <matthewlmcclure@gmail.com> writes:
>
>> An alternative approach would be to reuse git-diff's option parsing
>
> I do not think you want to go there. That wouldn't solve the third
> case in my previous message, no?
I think I don't fully understand your third bullet.
> * If you are comparing two trees, and especially if your RHS is not
> HEAD, you will send everything to a temporary without
> symlinks. Any edit made by the user will be lost.
I think you're suggesting to use a symlink any time the content of any
given RHS revision is the same as the working tree.
I imagine that might confuse me as a user. It would create
circumstances where some files are symlinked and others aren't for
reasons that won't be straightforward.
I imagine solving that case, I might instead implement a copy back to
the working tree with conflict detection/resolution. Some earlier
iterations of the directory diff feature used copy back without
conflict detection and created situations where I clobbered my own
changes by finishing a directory diff after making edits concurrently.
^ permalink raw reply
* Re: [PATCH v8 2/5] blame: introduce $ as "end of file" in -L syntax
From: Junio C Hamano @ 2013-03-12 22:34 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Bo Yang, Zbigniew Jędrzejewski-Szmek, Will Palmer
In-Reply-To: <7vk3psicgf.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Thomas Rast <trast@student.ethz.ch> writes:
>
>> To save the user a lookup of the last line number, introduce $ as a
>> shorthand for the last line. This is mostly useful to spell "until
>> the end of the file" as '-L<begin>,$'.
>
> Doesn't "-L <begin>" or "-L <begin>," do that already? If it were
> to introduce "-L $-4," or "-L$-4,+2", I would understand why the
> addition may be useful, but otherwise I do not think it adds much
> value.
It is a quiet-period so there is no need to rush, but did anything
happened further on this series?
^ permalink raw reply
* Re: difftool -d symlinks, under what conditions
From: Junio C Hamano @ 2013-03-12 22:16 UTC (permalink / raw)
To: Matt McClure
Cc: John Keeping, David Aguilar, git@vger.kernel.org, Tim Henigan
In-Reply-To: <CAJELnLGBr1wOX4-3rCNjPpPLezc_6FgyeuPqty268JR0==qtvQ@mail.gmail.com>
Matt McClure <matthewlmcclure@gmail.com> writes:
> An alternative approach would be to reuse git-diff's option parsing
> and make it tell git-difftool when git-diff sees the working tree
> case. At this point, I haven't seen an obvious place in the source
> where git-diff makes that choice, but if someone could point me in the
> right direction, I think I'd actually prefer that approach. What do
> you think?
I do not think you want to go there. That wouldn't solve the third
case in my previous message, no?
^ permalink raw reply
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