* Re: [PATCH] Ignore SIGPIPE when running a filter driver
From: Junio C Hamano @ 2012-02-20 22:11 UTC (permalink / raw)
To: Jehan Bing; +Cc: git, j.sixt, peff, jrnieder
In-Reply-To: <1329771217-9088-1-git-send-email-jehan@orb.com>
Jehan Bing <jehan@orb.com> writes:
> diff --git a/convert.c b/convert.c
> index c06309f..5d312cb 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -2,6 +2,7 @@
> #include "attr.h"
> #include "run-command.h"
> #include "quote.h"
> +#include "sigchain.h"
>
> /*
> * convert.c - convert a file when checking it out and checking it in.
> @@ -360,12 +361,16 @@ static int filter_buffer(int in, int out, void *data)
> if (start_command(&child_process))
> return error("cannot fork to run external filter %s", params->cmd);
>
> + sigchain_push(SIGPIPE, SIG_IGN);
> +
> write_err = (write_in_full(child_process.in, params->src, params->size) < 0);
> if (close(child_process.in))
> write_err = 1;
> if (write_err)
> error("cannot feed the input to external filter %s", params->cmd);
>
> + sigchain_pop(SIGPIPE);
> +
Thanks.
I think this is OK on a POSIX system where this function is run by
start_async() which is implemented with a forked child process.
I do not now if it poses a issue on Windows, though. Johannes, any
comments?
^ permalink raw reply
* Re: git status: small difference between stating whole repository and small subdirectory
From: Jeff King @ 2012-02-20 22:41 UTC (permalink / raw)
To: Junio C Hamano
Cc: Thomas Rast, Nguyen Thai Ngoc Duy, Piotr Krukowiecki,
Git Mailing List
In-Reply-To: <7v1uppkvpx.fsf@alter.siamese.dyndns.org>
On Mon, Feb 20, 2012 at 02:04:26PM -0800, Junio C Hamano wrote:
> > ... Things may have changed since then, of course, but I at
> > least know that they were sufficient in 34110cd^.
>
> Looking at where cache_tree_free() is called, I think back then the
> two-way merge was deemed OK, but we did not trust three-way merge or
> merge-recursive at all.
Thanks, I'll take a look more closely at those cases.
> It is OK to check that we do not over-invalidate for performance, but it
> is a lot more important to make sure we do not under-invalidate for
> correctness. I am a bit worried that you seem to be putting more stress
> on the former.
I think it is just selection bias of the specific parts of his tests
that I was responding to. I completely agree that correctness is way
more important, and I'm also trying to come up with tests to validate
correctness. I just wasn't talking about them there.
I still think replaying real-world test cases is going to be more likely
to find issues in invalidation. I can come up with lots of simple
test-cases, but they're not likely to find anything we wouldn't find in
the code with trivial inspection. I think a combination of careful
analysis and real-world validation is going to be more helpful in the
long run than the kind of simplistic tests that are in t0090.
-Peff
^ permalink raw reply
* Re: [RFC] pre-rebase: Refuse to rewrite commits that are reachable from upstream
From: Junio C Hamano @ 2012-02-20 22:43 UTC (permalink / raw)
To: Johan Herland; +Cc: git, jnareb, philipoakley
In-Reply-To: <1329772071-11301-1-git-send-email-johan@herland.net>
Johan Herland <johan@herland.net> writes:
> Teach the pre-rebase sample hook to refuse rewriting commits on a branch
> that are present in that branch's @{upstream}. This is to prevent users
> from rewriting commits that have already been published.
If the user has configured an option to always create @{u} when creating a
branch from somewhere else, transplanting $n good commits from his master
that is forked from the shared master onto his maint would be done like
this:
$ git checkout -b copy master
$ git rebase -i --onto maint HEAD~$n
If these good commits have been published to 'master', because the
upstream of 'copy' is set to the local 'master', would the new mechanism
hinder this attempt to backport good fixes? Perhaps it is safer to
trigger only when @{u} exists and it is not local?
But because you wanted to discuss more about the issues than the
implementation, let me think aloud a bit, reviewing what I usually do.
I keep things simpler by sticking to a very simple rule. I allow myself to
rebase only what is not yet in 'next', so the logic becomes a simple "am I
creating a new commit based on what is already in 'next'?"
During the course of integration testing with 'next', however, I often
find a topic or two that I have merged to it is less than ideal, and of
course, the whole point of doing integration testing with 'next' is to
find such problematic topics before pushing 'next' out. I rewind 'next',
rebuild the problematic topics, and then rebuild 'next', and all of these
happen before 'next' is pushed out. The step that rewinds 'next' that
acquired problematic versions of the topics makes the topics eligible for
rebase.
That would mean that a configuration variable "rebase.bottomLimit = next"
is sufficient to implement such a check for me. No per-branch bottom is
needed, because everything is merged to 'next' and tested to see if they
do not need further rebases for fixing them up before they are published.
Perhaps "I mistakenly rebased something that I have already published" is
a mere symptom a bigger problem. The issue may not be that we do not give
them a good tool to help them to be more careful with less effort on their
part before they rebase. It may instead be that it is too easy to publish
branches that are not ready to be pushed out, and that is the real cause
of the "I realized I need to fix the topic and I fixed it, but I did not
realize that it was too late and I shouldn't have rebased" problem.
I wonder if it would be a more direct solution to the issue you are
raising to give them a good tool to help them to be more careful with less
effort on their part before they publish (not before they rebase).
^ permalink raw reply
* Re: git-subtree Ready #2
From: Junio C Hamano @ 2012-02-20 23:14 UTC (permalink / raw)
To: Jeff King; +Cc: David A. Greene, git, Avery Pennarun
In-Reply-To: <20120220205346.GA6335@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Feb 15, 2012 at 10:07:16PM -0600, David A. Greene wrote:
> ...
> Of course there's no real reason we can't take it slow by putting it in
> contrib, and then graduating from there. It just seems like an
> unnecessary and complicated interim step. Either way, I do think it's
> worth saving the commit history by doing a real merge.
>
> I dunno. It is really up to Junio, I guess.
It sounds like the simplest and cleanest would be to treat it as if its
current version came as a patch submission, cook it just like any other
topic in 'pu' down to 'next' down to eventually 'master', with the usual
review cycle of pointing out what is wrong and needs fixing followed by a
series of re-rolls.
The total amount of change does not look too bad, either:
$ git diff --stat master...origin/subtree
contrib/subtree/.gitignore | 5 +
contrib/subtree/COPYING | 339 +++++++++++++++++
contrib/subtree/Makefile | 45 +++
contrib/subtree/README | 8 +
contrib/subtree/git-subtree.sh | 712 ++++++++++++++++++++++++++++++++++++
contrib/subtree/git-subtree.txt | 366 ++++++++++++++++++
contrib/subtree/t/Makefile | 71 ++++
contrib/subtree/t/t7900-subtree.sh | 508 +++++++++++++++++++++++++
contrib/subtree/todo | 50 +++
t/test-lib.sh | 11 +-
10 files changed, 2114 insertions(+), 1 deletion(-)
It does look like it needs to start its life in contrib/ if we were to put
this in git.git. I haven't looked at the script fully, but it has an issue
from its first line, which is marked with "#!/bin/bash". It is unclear if
it is infested by bash-isms beyond repair (in which case "#!/bin/bash" is
fine), or it was written portably but was marked with "#!/bin/bash" just
by inertia. A patch that corresponds to the above diffstat immediately
shows many style issues including trailing eye-sore whitespaces.
It seems that it is even capable of installing from contrib/subtree, so
keeping it in contrib/ while many issues it may have gets fixed would not
hurt the original goal of giving the script more visibility.
The change to t/test-lib.sh should be made independent of this topic, I
would think.
----------------------------------------------------------------
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e28d5fd..c877a91 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -55,6 +55,7 @@ unset $(perl -e '
.*_TEST
PROVE
VALGRIND
+ BUILD_DIR
));
my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
print join("\n", @vars);
@@ -924,7 +925,15 @@ then
# itself.
TEST_DIRECTORY=$(pwd)
fi
-GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+
+if test -z "$GIT_BUILD_DIR"
+then
+ echo Here
+ # We allow tests to override this, in case they want to run tests
+ # outside of t/, e.g. for running tests on the test library
+ # itself.
+ GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+fi
if test -n "$valgrind"
then
----------------------------------------------------------------
This change deserves its own justification.
After looking at the history of subtree branch there, however, I agree
that it would not help anybody to have its history in my tree with log
messages like these (excerpt from shortlog output):
update todo
Some todo items reported by pmccurdy
todo
Docs: when pushing to github, the repo path needs to end in .git
todo
todo^
todo
todo: idea for a 'git subtree grafts' command
^ permalink raw reply related
* Re: git status: small difference between stating whole repository and small subdirectory
From: Junio C Hamano @ 2012-02-20 23:31 UTC (permalink / raw)
To: Jeff King
Cc: Thomas Rast, Nguyen Thai Ngoc Duy, Piotr Krukowiecki,
Git Mailing List
In-Reply-To: <20120220224140.GA7116@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I still think replaying real-world test cases is going to be more likely
> to find issues in invalidation.
Yes, but it depends on what kind of replaying you have in mind. It is
very hard to come up with "replaying real-world test case".
For example, randomly picking commit pair <$A,$B> from the kernel
repository and running
git reset --hard $A
git checkout $B
T0=$(git write-tree)
drop-cache-tree
T1=$(git write-tree)
test "$T0" = "$T1" && test "$T0" = $(git rev-parse $B^{tree})
is necessary but I do not think that is sufficient. We also want to do
something like:
git reset --hard $A
... modify paths that do not change between $A and $B
... git add these paths
git write-tree
git checkout $B
with and without the "git write-tree" to see the part of the cache-tree
smudged by the modification behaves sanely. The codepath that is used
to deal with the case where the index does not match $A but matches $B is
also different, so the "modify path and git add" step would have to be
crafted carefully to touch all bases.
^ permalink raw reply
* Re: [PATCH 0/8 v6] diff --stat: use the full terminal width
From: Junio C Hamano @ 2012-02-20 23:41 UTC (permalink / raw)
To: Zbigniew Jędrzejewski-Szmek; +Cc: git, Michael J Gruber, pclouds, j.sixt
In-Reply-To: <1329775034-21551-1-git-send-email-zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> JC:
>> Perhaps the maximum for garph_width should be raised to something like
>> "min(80, stat_width) - name_width"?
> I think that a graph like
> a | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> b | 1 -
> is not very readable. I like the consistency forced by the 40-column limit.
> But I guess that this is very subjective.
The above makes it very obvious that there is a huge amount of change made
to 'a' and a bit of deletion to 'b', compared to a mini-graph that is
truncated to half the screen width.
Besides, the above is what you would get without your patch on 80-column
terminal, no?
^ permalink raw reply
* [ANNOUNCE] git-link -- builds repository browser links
From: Georgi Valkov @ 2012-02-21 0:06 UTC (permalink / raw)
To: git
Hi all,
If you find yourselves sending out a ton of gitweb, cgit or github links
every day, git-link is a sub-command that can speed things up for you by
eliminating pointy-clicky time that you would spend in a browser.
Example:
> $ git config --add link.url http://git.kernel.org/?p=git/git.git
> $ git config --add link.browser gitweb
> $ git link v1.7.9~^{tree}
>
http://git.kernel.org/?p=git/git.git;a=tree;h=903db0d86a809b1f84415654369a3cf6dff5f4d5
> $ git link --clipboard fsck.c
>
http://git.kernel.org/?p=git/git.git;a=blob;h=6c855f84f01c19678399d85181da1094bd61b371;f=fsck.c
Links:
- https://github.com/gvalkov/git-link
- http://pypi.python.org/pypi/gitlink
Best,
G.
^ permalink raw reply
* Re: [PATCH 0/8 v6] diff --stat: use the full terminal width
From: Zbigniew Jędrzejewski-Szmek @ 2012-02-21 0:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael J Gruber, pclouds, j.sixt
In-Reply-To: <7v4nuljcnk.fsf@alter.siamese.dyndns.org>
On 02/21/2012 12:41 AM, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl> writes:
>
>> JC:
>>> Perhaps the maximum for garph_width should be raised to something like
>>> "min(80, stat_width) - name_width"?
>> I think that a graph like
>> a | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> b | 1 -
>> is not very readable. I like the consistency forced by the 40-column limit.
>> But I guess that this is very subjective.
>
> The above makes it very obvious that there is a huge amount of change made
> to 'a' and a bit of deletion to 'b', compared to a mini-graph that is
> truncated to half the screen width.
Yes. But the same graph with 40 columns tells me exactly the same thing.
OTOH, if the filenames+graph fill the whole 80 columns, everything is
nicely aligned wrt. to text above and below. Maybe it should be
configurable after all?
> Besides, the above is what you would get without your patch on 80-column
> terminal, no?
Yes.
Zbyszek
^ permalink raw reply
* Re: [RFC] pre-rebase: Refuse to rewrite commits that are reachable from upstream
From: Johan Herland @ 2012-02-21 0:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, jnareb, philipoakley
In-Reply-To: <7vobstjfcs.fsf@alter.siamese.dyndns.org>
On Mon, Feb 20, 2012 at 23:43, Junio C Hamano <gitster@pobox.com> wrote:
> Johan Herland <johan@herland.net> writes:
>> Teach the pre-rebase sample hook to refuse rewriting commits on a branch
>> that are present in that branch's @{upstream}. This is to prevent users
>> from rewriting commits that have already been published.
>
> If the user has configured an option to always create @{u} when creating a
> branch from somewhere else, transplanting $n good commits from his master
> that is forked from the shared master onto his maint would be done like
> this:
>
> $ git checkout -b copy master
> $ git rebase -i --onto maint HEAD~$n
>
> If these good commits have been published to 'master', because the
> upstream of 'copy' is set to the local 'master', would the new mechanism
> hinder this attempt to backport good fixes? Perhaps it is safer to
> trigger only when @{u} exists and it is not local?
Ah, you're talking about setting branch.autosetupmerge = always. I
didn't know about that until I looked it up, just now. My first
impression is "who would want to use that?", but since it's there, we
should not break it. So, yes, we should probably apply the check when
@{u} exists, and refers to a remote-tracking branch. (One could argue
that this is also unsafe, since the current repo is a perfectly valid
remote, but if you do that, I'd be inclined to let you shoot yourself
in the foot.)
Also, if this check becomes part of core git, we obviously want some
config option to en/disable it. Preferably at multiple levels, as
discussed earlier in this thread:
core.rewriteUpstream = refuse/false, warn, allow/true
(en/disables the check for the entire repo, but may be overridden by:)
remote.<name>.rewriteUpstream = refuse/false, warn, allow/true
(en/disables the check for branches whose upstream is the given
remote, but may be overridden by:)
branch.<name>.rewriteUpstream = refuse/false, warn, allow/true
(en/disables the check for the given local branch)
This allows fine-grained control of which branches/remotes you
consider 'public', and which are ok to rewrite.
> But because you wanted to discuss more about the issues than the
> implementation, let me think aloud a bit, reviewing what I usually do.
>
> I keep things simpler by sticking to a very simple rule. I allow myself to
> rebase only what is not yet in 'next', so the logic becomes a simple "am I
> creating a new commit based on what is already in 'next'?"
>
> During the course of integration testing with 'next', however, I often
> find a topic or two that I have merged to it is less than ideal, and of
> course, the whole point of doing integration testing with 'next' is to
> find such problematic topics before pushing 'next' out. I rewind 'next',
> rebuild the problematic topics, and then rebuild 'next', and all of these
> happen before 'next' is pushed out. The step that rewinds 'next' that
> acquired problematic versions of the topics makes the topics eligible for
> rebase.
>
> That would mean that a configuration variable "rebase.bottomLimit = next"
> is sufficient to implement such a check for me. No per-branch bottom is
> needed, because everything is merged to 'next' and tested to see if they
> do not need further rebases for fixing them up before they are published.
What you are describing here may be a common workflow, but
"rebase.bottomLimit" is still very specific to that kind of workflow.
What I'm after is a much more workflow-agnostic concept of:
"If I have pushed something, I should probably not rebase it"
I believe this rule is sufficiently common in most workflows to
warrant the addition of this safety-check to Git. (And making the
safety-check configurable caters to those that don't need/want it.)
> Perhaps "I mistakenly rebased something that I have already published" is
> a mere symptom a bigger problem. The issue may not be that we do not give
> them a good tool to help them to be more careful with less effort on their
> part before they rebase. It may instead be that it is too easy to publish
> branches that are not ready to be pushed out, and that is the real cause
> of the "I realized I need to fix the topic and I fixed it, but I did not
> realize that it was too late and I shouldn't have rebased" problem.
>
> I wonder if it would be a more direct solution to the issue you are
> raising to give them a good tool to help them to be more careful with less
> effort on their part before they publish (not before they rebase).
Yes, in many cases, the user will probably agree that the rewrite
should ideally have happened _before_ the branch was first published.
However, I'm not sure how we can help the user _not_ publish the
branch until it's ready. We could throw in a warning (or even a stupid
"Are you really sure you want to publish?") before pushing a branch,
but I don't think it would help at all.
I think the following decribes what often happens for many users:
1. User A pushes the branch to a public repo.
2. User B points out a simple mistake in the branch.
3. User A makes a fix
4. User A squashes the fix into the (already-published) history.
5. User A attempts to push the "fixed" history (but is rejected by
the public repo because of non-fast-forward).
At this point, the damage is already done, since often neither of the
following alternatives are desirable:
6a. (safe) User A realizes the error and undoes the history-rewrite
(this is not easy for novice users), leaving the fix on top of the
already-published history, and re-pushes a fast-forwarding history.
OR
6b. (dangerous) User A reattempts the push with --force. User B (and
user C, D, ...) is left to clean up the mess.
You could say that User A should be more careful and push to a "less
public" repo in step #1 (thus allowing the fix to be squashed before
pushing to a "more public" repo in step #5), but how "public" is
"public" enough to have someone point out the bug, but still
"unpublic" enough to allow rebase?
In general, I really cannot see any opportunity before step #4 where
we can stop (or warn) the user from what is about to happen. And I
think that refusing rewrites of commits that are already present in
the @{upstream} remote-tracking branch is good enough to help most
users avoid steps #4 through #6 (in a push-based workflow[1]). In
fact, from a pedagogical POV, I think step #4 is probably the best
spot for novice users to learn exactly the distinction between
acceptable and unacceptable history rewrites (instead of having it
explained to them as a consequence of the step #5).
Thanks for your insight,
...Johan
[1]: For non-push-based workflows (such as send-email, sharing
bundles, pulls from local repo, etc.) we will need other methods for
determining which commits have been published. This have been
discussed earlier in the thread, but is outside the scope of what I
want to accomplish for now.
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* [PATCH] contrib: added git-diffall
From: Tim Henigan @ 2012-02-21 0:59 UTC (permalink / raw)
To: git, gitster; +Cc: tim.henigan
This script adds directory diff support to git. It launches a single
instance of the user-configured external diff tool and performs a
directory diff between the specified revisions. The before/after files
are copied to a tmp directory to do this. Either 'diff.tool' or
'merge.tool' must be set before running the script.
The existing 'git difftool' command already allows the user to view diffs
using an external tool. However if multiple files have changed, a
separate instance of the tool is launched for each one. This can be
tedious when many files are involved.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
This script has been hosted on GitHub [1] since April 2010. Enough people
have found it useful that I hope it will be considered for inclusion in
the standard git install, either in contrib or as a new core command.
[1]: https://github.com/thenigan/git-diffall
contrib/diffall/README | 18 +++
contrib/diffall/git-diffall | 275 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 293 insertions(+)
create mode 100644 contrib/diffall/README
create mode 100755 contrib/diffall/git-diffall
diff --git a/contrib/diffall/README b/contrib/diffall/README
new file mode 100644
index 0000000..12881d2
--- /dev/null
+++ b/contrib/diffall/README
@@ -0,0 +1,18 @@
+The git-diffall script provides a directory based diff mechanism
+for git. The script relies on the diff.tool configuration option
+to determine what diff viewer is used.
+
+This script is compatible with all the forms used to specify a
+range of revisions to diff:
+
+ 1. git diffall: shows diff between working tree and staged changes
+ 2. git diffall --cached [<commit>]: shows diff between staged changes and HEAD (or other named commit)
+ 3. git diffall <commit>: shows diff between working tree and named commit
+ 4. git diffall <commit> <commit>: show diff between two named commits
+ 5. git diffall <commit>..<commit>: same as above
+ 6. git diffall <commit>...<commit>: show the changes on the branch containing and up to the second , starting at a common ancestor of both <commit>
+
+Note: all forms take an optional path limiter [--] [<path>*]
+
+This script is based on an example provided by Thomas Rast on the Git list [1]:
+[1] http://thread.gmane.org/gmane.comp.version-control.git/124807
diff --git a/contrib/diffall/git-diffall b/contrib/diffall/git-diffall
new file mode 100755
index 0000000..ef01eda
--- /dev/null
+++ b/contrib/diffall/git-diffall
@@ -0,0 +1,275 @@
+#!/bin/bash -e
+# Copyright 2010 - 2012, Tim Henigan <tim.henigan@gmail.com>
+#
+# Perform a directory diff between commits in the repository using
+# the external diff or merge tool specified in the user's config.
+
+USAGE='[--cached] [--copy-back] [-x|--extcmd=<command>] <commit>{0,2} -- <path>*
+
+ --cached Compare to the index rather than the working tree.
+
+ --copy-back Copy files back to the working tree when the diff
+ tool exits (in case they were modified by the
+ user). This option is only valid if the diff
+ compared with the working tree.
+
+ -x=<command>
+ --extcmd=<command> Specify a custom command for viewing diffs.
+ git-diffall ignores the configured defaults and
+ runs $command $LOCAL $REMOTE when this option is
+ specified. Additionally, $BASE is set in the
+ environment.
+'
+
+SUBDIRECTORY_OK=1
+. "$(git --exec-path)/git-sh-setup"
+
+TOOL_MODE=diff
+. "$(git --exec-path)/git-mergetool--lib"
+
+merge_tool="$(get_merge_tool)"
+if test -z "$merge_tool"
+then
+ echo "Error: Either the 'diff.tool' or 'merge.tool' option must be set."
+ usage
+fi
+
+start_dir=$(pwd)
+
+# needed to access tar utility
+cdup=$(git rev-parse --show-cdup) &&
+cd "$cdup" || {
+ echo >&2 "Cannot chdir to $cdup, the toplevel of the working tree"
+ exit 1
+}
+
+# mktemp is not available on all platforms (missing from msysgit)
+# Use a hard-coded tmp dir if it is not available
+if test -z $(which mktemp 2>/dev/null)
+then
+ tmp=/tmp/git-diffall-tmp
+else
+ tmp="$(mktemp -d -t tmp.XXXXXX)"
+fi
+mkdir -p "$tmp"
+
+left=
+right=
+paths=
+path_sep=
+compare_staged=
+common_ancestor=
+left_dir=
+right_dir=
+diff_tool=
+copy_back=
+
+while test $# != 0
+do
+ case "$1" in
+ -h|--h|--he|--hel|--help)
+ usage
+ ;;
+ --cached)
+ compare_staged=1
+ ;;
+ --copy-back)
+ copy_back=1
+ ;;
+ -x|--e|--ex|--ext|--extc|--extcm|--extcmd)
+ diff_tool=$2
+ shift
+ ;;
+ --)
+ path_sep=1
+ ;;
+ -*)
+ echo Invalid option: "$1"
+ usage
+ ;;
+ *)
+ # could be commit, commit range or path limiter
+ case "$1" in
+ *...*)
+ left=${1%...*}
+ right=${1#*...}
+ common_ancestor=1
+ ;;
+ *..*)
+ left=${1%..*}
+ right=${1#*..}
+ ;;
+ *)
+ if test -n "$path_sep"
+ then
+ if test -z "$paths"
+ then
+ paths=$1
+ else
+ paths="$paths $1"
+ fi
+ elif test -z "$left"
+ then
+ left=$1
+ elif test -z "$right"
+ then
+ right=$1
+ else
+ if test -z "$paths"
+ then
+ paths=$1
+ else
+ paths="$paths $1"
+ fi
+ fi
+ ;;
+ esac
+ ;;
+ esac
+ shift
+done
+
+# Determine the set of files which changed
+if test -n "$left" && test -n "$right"
+then
+ left_dir="cmt-$(git rev-parse --short $left)"
+ right_dir="cmt-$(git rev-parse --short $right)"
+
+ if test -n "$compare_staged"
+ then
+ usage
+ elif test -n "$common_ancestor"
+ then
+ git diff --name-only "$left"..."$right" -- $paths > "$tmp"/filelist
+ else
+ git diff --name-only "$left" "$right" -- $paths > "$tmp"/filelist
+ fi
+elif test -n "$left"
+then
+ left_dir="cmt-$(git rev-parse --short $left)"
+
+ if test -n "$compare_staged"
+ then
+ right_dir="staged"
+ git diff --name-only --cached "$left" -- $paths > "$tmp"/filelist
+ else
+ right_dir="working_tree"
+ git diff --name-only "$left" -- $paths > "$tmp"/filelist
+ fi
+else
+ left_dir="HEAD"
+
+ if test -n "$compare_staged"
+ then
+ right_dir="staged"
+ git diff --name-only --cached -- $paths > "$tmp"/filelist
+ else
+ right_dir="working_tree"
+ git diff --name-only -- $paths > "$tmp"/filelist
+ fi
+fi
+
+# Exit immediately if there are no diffs
+if test ! -s "$tmp"/filelist
+then
+ exit 0
+fi
+
+if test -n "$copy_back" && test "$right_dir" != "working_tree"
+then
+ echo "--copy-back is only valid when diff includes the working tree."
+ exit 1
+fi
+
+# Create the named tmp directories that will hold the files to be compared
+mkdir -p "$tmp"/"$left_dir" "$tmp"/"$right_dir"
+
+# Populate the tmp/right_dir directory with the files to be compared
+if test -n "$right"
+then
+ while read name
+ do
+ ls_list=$(git ls-tree $right $name)
+ if test -n "$ls_list"
+ then
+ mkdir -p "$tmp"/"$right_dir"/"$(dirname "$name")"
+ git show "$right":"$name" > "$tmp"/"$right_dir"/"$name" || true
+ fi
+ done < "$tmp"/filelist
+elif test -n "$compare_staged"
+then
+ while read name
+ do
+ ls_list=$(git ls-files -- $name)
+ if test -n "$ls_list"
+ then
+ mkdir -p "$tmp"/"$right_dir"/"$(dirname "$name")"
+ git show :"$name" > "$tmp"/"$right_dir"/"$name"
+ fi
+ done < "$tmp"/filelist
+else
+ if test -n "$(which gnutar 2>/dev/null)"
+ then
+ gnutar --ignore-failed-read -c -T "$tmp"/filelist | (cd "$tmp"/"$right_dir" && gnutar -x)
+ else
+ tar --ignore-failed-read -c -T "$tmp"/filelist | (cd "$tmp"/"$right_dir" && tar -x)
+ fi
+fi
+
+# Populate the tmp/left_dir directory with the files to be compared
+while read name
+do
+ if test -n "$left"
+ then
+ ls_list=$(git ls-tree $left $name)
+ if test -n "$ls_list"
+ then
+ mkdir -p "$tmp"/"$left_dir"/"$(dirname "$name")"
+ git show "$left":"$name" > "$tmp"/"$left_dir"/"$name" || true
+ fi
+ else
+ if test -n "$compare_staged"
+ then
+ ls_list=$(git ls-tree HEAD $name)
+ if test -n "$ls_list"
+ then
+ mkdir -p "$tmp"/"$left_dir"/"$(dirname "$name")"
+ git show HEAD:"$name" > "$tmp"/"$left_dir"/"$name"
+ fi
+ else
+ mkdir -p "$tmp"/"$left_dir"/"$(dirname "$name")"
+ git show :"$name" > "$tmp"/"$left_dir"/"$name"
+ fi
+ fi
+done < "$tmp"/filelist
+
+cd "$tmp"
+LOCAL="$left_dir"
+REMOTE="$right_dir"
+
+if test -n "$diff_tool"
+then
+ export BASE
+ eval $diff_tool '"$LOCAL"' '"$REMOTE"'
+else
+ run_merge_tool "$merge_tool" false
+fi
+
+# This function is called on exit
+cleanup () {
+ cd "$start_dir"
+
+ # Copy files back to the working dir, if requested
+ if test -n "$copy_back" && test "$right_dir" = "working_tree"
+ then
+ git_top_dir=$(git rev-parse --show-toplevel)
+ find "$tmp/$right_dir" -type f|while read file; do
+ cp "$file" "$git_top_dir/${file#$tmp/$right_dir/}"
+ done
+ fi
+
+ # Remove the tmp directory
+ rm -rf "$tmp"
+}
+
+trap cleanup EXIT
--
1.7.9.GIT
^ permalink raw reply related
* Re: [PATCH] Ignore SIGPIPE when running a filter driver
From: Jonathan Nieder @ 2012-02-21 3:01 UTC (permalink / raw)
To: Jehan Bing; +Cc: git, gitster, j.sixt, peff
In-Reply-To: <1329771217-9088-1-git-send-email-jehan@orb.com>
Hi,
Jehan Bing wrote:
> If a filter is not defined or if it fails, git behaves as if the filter
> is a no-op passthru. However, if the filter exits before reading all
> the content, and depending on the timing git, could be kill with
> SIGPIPE instead.
>
> Ignore SIGPIPE while processing the filter to detect when it exits
> early and fallback to using the unfiltered content.
>
> Signed-off-by: Jehan Bing <jehan@orb.com>
For the benefit of the uninitiated ("how would ignoring an error help
me detect an error?"): setting the SIGPIPE handler to SIG_IGN does not
actually ignore the broken pipe condition but causes it to be reported
as an I/O error, errno == EPIPE. That means instead of being killed
by SIGPIPE, git gets to fall back to passthrough and report the
filter's mistake:
error: cannot feed the input to external filter <foo>
error: external filter <foo> failed
[...]
> +++ b/convert.c
[...]
> @@ -360,12 +361,16 @@ static int filter_buffer(int in, int out, void *data)
> if (start_command(&child_process))
> return error("cannot fork to run external filter %s", params->cmd);
>
> + sigchain_push(SIGPIPE, SIG_IGN);
Setting the signal disposition after launching the external filter
which would otherwise inherit it, so the filter does not have to cope
with unfamiliar SIGPIPE handling[*]. Phew.
> +
> write_err = (write_in_full(child_process.in, params->src, params->size) < 0);
> if (close(child_process.in))
> write_err = 1;
> if (write_err)
> error("cannot feed the input to external filter %s", params->cmd);
>
> + sigchain_pop(SIGPIPE);
> +
This happens in an async procedure. SIGPIPE is ignored in the
following block in the other thread:
if (strbuf_read(&nbuf, async.out, len) < 0) {
error("read from external filter %s failed", cmd);
ret = 0;
}
if (close(async.out)) {
error("read from external filter %s failed", cmd);
ret = 0;
}
if (finish_async(&async)) {
That implies a tiny behavior change: if there is an I/O error reading
from async.out at the right moment and stderr is going to a closed
pipe, inability to report the error can result in the error flag being
set on stderr instead of the process being killed. I don't think
anyone will notice.
So at least on POSIX-y platforms, this patch looks good to me. Thanks
for writing it.
Sincerely,
Jonathan
[*] See http://bugs.python.org/issue1652 for some stories about what
we are narrowly escaping here. :)
^ permalink raw reply
* Re: git-subtree Ready #2
From: David A. Greene @ 2012-02-21 5:31 UTC (permalink / raw)
To: Jeff King; +Cc: git, Avery Pennarun
In-Reply-To: <20120220205346.GA6335@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Of course there's no real reason we can't take it slow by putting it in
> contrib, and then graduating from there. It just seems like an
> unnecessary and complicated interim step. Either way, I do think it's
> worth saving the commit history by doing a real merge.
I have no problem starting off in contrib. I was asking more about the
logistics of getting there.
> As a non-user, I am totally fine with it. I think the burden is not that
> high, and you have already promised to deal with ongoing maintenance
> issues.
Indeed, I'm more than happy to help with maintenance. I have one or two
fixes/enhancements on my list, in fact.
-Dave
^ permalink raw reply
* Re: git-subtree Ready #2
From: David A. Greene @ 2012-02-21 5:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, Avery Pennarun
In-Reply-To: <7vd399jdwc.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
> It sounds like the simplest and cleanest would be to treat it as if its
> current version came as a patch submission, cook it just like any other
> topic in 'pu' down to 'next' down to eventually 'master', with the usual
> review cycle of pointing out what is wrong and needs fixing followed by a
> series of re-rolls.
Ok, but we will preserve the history via the subtree merge, yes?
> The total amount of change does not look too bad, either:
Yes, it's a fairly small tool.
> It does look like it needs to start its life in contrib/ if we were to put
> this in git.git.
That sounds good to me. It should get a good shakedown before graduating.
> I haven't looked at the script fully, but it has an issue from its
> first line, which is marked with "#!/bin/bash". It is unclear if it
> is infested by bash-isms beyond repair (in which case "#!/bin/bash" is
> fine), or it was written portably but was marked with "#!/bin/bash"
> just by inertia. A patch that corresponds to the above diffstat
> immediately shows many style issues including trailing eye-sore
> whitespaces.
Ok.
> It seems that it is even capable of installing from contrib/subtree, so
> keeping it in contrib/ while many issues it may have gets fixed would not
> hurt the original goal of giving the script more visibility.
Right, I intentially designed it that way.
> The change to t/test-lib.sh should be made independent of this topic, I
> would think.
Ok, I'll propose those changes separately. They are a prerequisite for
a git-subtree that is easily testable while in contrib.
> ----------------------------------------------------------------
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index e28d5fd..c877a91 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -55,6 +55,7 @@ unset $(perl -e '
> .*_TEST
> PROVE
> VALGRIND
> + BUILD_DIR
> ));
> my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
> print join("\n", @vars);
> @@ -924,7 +925,15 @@ then
> # itself.
> TEST_DIRECTORY=$(pwd)
> fi
> -GIT_BUILD_DIR="$TEST_DIRECTORY"/..
> +
> +if test -z "$GIT_BUILD_DIR"
> +then
> + echo Here
> + # We allow tests to override this, in case they want to run tests
> + # outside of t/, e.g. for running tests on the test library
> + # itself.
> + GIT_BUILD_DIR="$TEST_DIRECTORY"/..
> +fi
>
> if test -n "$valgrind"
> then
> ----------------------------------------------------------------
> This change deserves its own justification.
I'll put a patch together with a more extensive explanation. Basically,
tests run outside of the top-level t/ directory don't work because there
are all sort of assumptions in test-lib.sh about where they live. There
are comments in test-lib.sh indicating that it should support tests in
other directories but I could not make it work out of the box.
> After looking at the history of subtree branch there, however, I agree
> that it would not help anybody to have its history in my tree with log
> messages like these (excerpt from shortlog output):
>
> update todo
> Some todo items reported by pmccurdy
> todo
> Docs: when pushing to github, the repo path needs to end in .git
> todo
> todo^
> todo
> todo: idea for a 'git subtree grafts' command
Ok, these are Avery's commits. I don't know that I have enough context
to improve the logs but I will look throught revisions and try to figure
things out. Avery, could you be of any help here? It sounds like we
need more descriptive log messages.
-Dave
^ permalink raw reply
* Re: git-subtree Ready #2
From: Junio C Hamano @ 2012-02-21 6:34 UTC (permalink / raw)
To: David A. Greene; +Cc: Jeff King, git, Avery Pennarun
In-Reply-To: <87ehtowxu7.fsf@smith.obbligato.org>
greened@obbligato.org (David A. Greene) writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Jeff King <peff@peff.net> writes:
>>
>> It sounds like the simplest and cleanest would be to treat it as if its
>> current version came as a patch submission, cook it just like any other
>> topic in 'pu' down to 'next' down to eventually 'master', with the usual
>> review cycle of pointing out what is wrong and needs fixing followed by a
>> series of re-rolls.
>
> Ok, but we will preserve the history via the subtree merge, yes?
I'll comment on just this part, but a short answer is "no, I do not think
so".
Even though you left "Jeff King writes", you removed everything he said
that I was quoting, and in order to understand why the answer is 'no', it
would have been better if you kept this part from what he said in your
reply:
>> ... Either way, I do think it's
>> worth saving the commit history by doing a real merge.
as that was what I was agreeing to with my "as if ... a patch submission".
>> After looking at the history of subtree branch there, however, I agree
>> that it would not help anybody to have its history in my tree with log
>> messages like these (excerpt from shortlog output):
>>
>> update todo
>> Some todo items reported by pmccurdy
>> todo
>> Docs: when pushing to github, the repo path needs to end in .git
>> todo
>> todo^
>> todo
>> todo: idea for a 'git subtree grafts' command
>
> Ok, these are Avery's commits. I don't know that I have enough context
> to improve the logs but I will look throught revisions and try to figure
> things out. Avery, could you be of any help here? It sounds like we
> need more descriptive log messages.
That was not what I was suggesting.
I was saying that the history up to the current state, littered with these
commits that are not "logical progression" but merely "a snapshot of
then-current state" may not be worth preserving, with or without better
messages.
Rewriting the entire history to make it a logical progression just for the
sake of history is obviously not worth the effort.
Which suggests that taking the end result that exists at the tip of your
subtree branch as a single code-drop, without pulling its history, lets us
start from a reasonably well tested state and would not lose us anything
of value. And that was what I was suggesting. For our history to explain
why/how the code got there better, another approach might be to instead
treat your bd7b2cf (Add 'contrib/subtree/' from commit '2793ee6ba...',
2012-01-29), which is where you took Avery's then-current state, as the
code-drop event that adds everything in contrib/subtree/ with a single
patch submission. I.e. in a git.git repository:
git checkout -b subtree master
git fetch git://sources.obbligato.org/git/git.git subtree
git merge --squash bd7b2cf
git commit -m "contrib: add git-subtree from Avery's tree"
to take the tip of your subtree branch. The history up to that point is
in Avery's repository where he stopped, which such an approach will not
pull in to git.git. And then we can replay bd7b2cf..FETCH_HEAD like so:
git checkout FETCH_HEAD
git rebase --onto subtree bd7b2cf
git push . HEAD:subtree
git checkout pu
git merge subtree
to preserve the history since that single code-drop event that records
your effort to adjust the code-dump into a better shape to live in our
contrib/ area. That will make it clear the division of blame on the code
added to git.git between Avery (everything before the squashed merge) and
you (everything after that).
^ permalink raw reply
* Re: [PATCH 0/8 v6] diff --stat: use the full terminal width
From: Junio C Hamano @ 2012-02-21 7:05 UTC (permalink / raw)
To: Zbigniew Jędrzejewski-Szmek; +Cc: git, Michael J Gruber, pclouds, j.sixt
In-Reply-To: <4F42E4C2.7070801@in.waw.pl>
Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> On 02/21/2012 12:41 AM, Junio C Hamano wrote:
>> Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl> writes:
>>
>>> JC:
>>>> Perhaps the maximum for garph_width should be raised to something like
>>>> "min(80, stat_width) - name_width"?
>>> I think that a graph like
>>> a | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> b | 1 -
>>> is not very readable. I like the consistency forced by the 40-column limit.
>>> But I guess that this is very subjective.
>>
>> The above makes it very obvious that there is a huge amount of change made
>> to 'a' and a bit of deletion to 'b', compared to a mini-graph that is
>> truncated to half the screen width.
> Yes. But the same graph with 40 columns tells me exactly the same thing.
That is a bogus argument, isn't it? You can say the same thing if you
limited the length of the graph bars to 10-columns if you only compare
between 1000 and 1. You can even do with just 5-columns. For that matter,
without any graph bar at all, it tells us exactly the same thing because
we have numbers. Does that mean we do not need any bar? Of course not.
We use bars as visual aid.
Imagine what happens to the graph if you had paths with medium amount of
changes like 980, 800, 40, in addition to 1000 and 1. By limiting the
length of the bars more than necessary, you are losing the resolution
without a good reason, and that is why I find 40-column limit a poor
design choice.
>> Besides, the above is what you would get without your patch on 80-column
>> terminal, no?
>
> Yes.
I think this "use at most 40-places for the graph bar" was your response
to somebody's observation that "on 200-column terminal, we will still see
the commit log messages (and for many projects, also patch text) that are
designed to be comfortably viewable within the 80-column on the left, and
overlong graph bar stands out like an ugly sore thumb".
While that "ugliness" observation might be a valid one to make, I do not
think limiting the length of the graph bar without taking the length of
the name part into account at all is the right solution to it.
After all, that is exactly the same thinking that led to the bug in the
current code that you fixed with your series, isn't it? Our safety code
truncated the graph bar width too early without taking the width needed to
show the names into account, and then when the names turn out to be all
short, we ended up wasting space on the right hand side, because we made
the bars too short and the decision was made too early in the code.
If the problem you are addressing is to make sure that the diffstat part
in the series of lines that are structured like this:
log message part ~80 column
diff stat part that can extend very very very very very very very long
patch text part ~80 column
does not become overly long, wouldn't it be a more natural solution to
make sure that when the total (i.e. name and graph) length can fit to
align with the message and patch (i.e. traditional ~80 col regardless of
the terminal width), not to give it too much width? If the names are
short, like "a" and "b", that may result in graph bar part to use ~70
columns or so, and if the names are long, like in a Java project, you may
allocate 50 columns to the name and leave only 50 columns or so for the
graph part.
A simple heuristic might be to see if name part (without truncation) and
the graph part (without scaling) fits under 100-columns if the terminal is
wider than that, and if so limit the whole thing to 100-columns before
deciding the allocation of the total into two parts. If the name part
alone is very wide, showing the name and the graph using the whole
terminal width would give you a better result than using the bars that are
artificially capped to a short limit, I would imagine.
^ permalink raw reply
* Re: git-subtree Ready #2
From: Junio C Hamano @ 2012-02-21 7:10 UTC (permalink / raw)
To: David A. Greene; +Cc: Jeff King, git, Avery Pennarun
In-Reply-To: <7vwr7gitjl.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> greened@obbligato.org (David A. Greene) writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Jeff King <peff@peff.net> writes:
>>>
>>> It sounds like the simplest and cleanest would be to treat it as if its
>>> current version came as a patch submission, cook it just like any other
>>> topic in 'pu' down to 'next' down to eventually 'master', with the usual
>>> review cycle of pointing out what is wrong and needs fixing followed by a
>>> series of re-rolls.
>>
>> Ok, but we will preserve the history via the subtree merge, yes?
>
> I'll comment on just this part, but a short answer is "no, I do not think
> so".
>
> Even though you left "Jeff King writes", you removed everything he said
> that I was quoting, and in order to understand why the answer is 'no', it
> would have been better if you kept this part from what he said in your
> reply:
>
>>> ... Either way, I do think it's
>>> worth saving the commit history by doing a real merge.
>
> as that was what I was agreeing to with my "as if ... a patch submission".
Ehh, s/agree/disagree/;
^ permalink raw reply
* Re: git status: small difference between stating whole repository and small subdirectory
From: Piotr Krukowiecki @ 2012-02-21 7:21 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Thomas Rast, Nguyen Thai Ngoc Duy, Git Mailing List
In-Reply-To: <7v8vjxjd47.fsf@alter.siamese.dyndns.org>
On Tue, Feb 21, 2012 at 12:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
[...]
Hi,
I hope the fixes will also help git-svn users? I.e. there's a lot of
rebasing (and cherry-picking - at least in my case) and probably some
other stuff going on under hood. I.e. I hope that if I git-svn rebase
or cherry-pick, it won't invalidate the cache...
Thanks,
--
Piotr Krukowiecki
^ permalink raw reply
* Re: [RFC] pre-rebase: Refuse to rewrite commits that are reachable from upstream
From: Junio C Hamano @ 2012-02-21 7:44 UTC (permalink / raw)
To: Johan Herland; +Cc: git, jnareb, philipoakley
In-Reply-To: <CALKQrgcDDVH8rxn80ZTMyR3y3n3Vs1LSCtH=ZiwG7it_LE2wgQ@mail.gmail.com>
Johan Herland <johan@herland.net> writes:
> What you are describing here may be a common workflow, but
> "rebase.bottomLimit" is still very specific to that kind of workflow.
> What I'm after is a much more workflow-agnostic concept of:
>
> "If I have pushed something, I should probably not rebase it"
Your "this branch pushes directly to that remote branch, so I can check if
it will result in rewrite of published commit" is even *less* generic than
having a single bottomLimit in my illustration.
I may not push out my topic branches directly, only the aggregate of them
in 'next', but once 'next' is pushed out, they are not eligible for
rebasing. A per-branch bottom, e.g. rebase.$branch.bottomLimit, might
make it more flexible to cover such a case, though.
On the other hand, without any such safety, a merge to 'next' would give
many conflicts and "shortlog master..next" will show many duplicates after
any topic that are already merged to it are accidentally rewritten, and it
is just the matter of using reflog on topic branches to recover from such
a mistake.
>> I wonder if it would be a more direct solution to the issue you are
>> raising to give them a good tool to help them to be more careful with less
>> effort on their part before they publish (not before they rebase).
>
> ..., I'm not sure how we can help the user _not_ publish the
> branch until it's ready.
I think we are in agreement that we do not think of a good solution
offhand to the real cause of the issue, except by encouraging the use of
throw-away review branches, perhaps.
> I think the following decribes what often happens for many users:
>
> 1. User A pushes the branch to a public repo.
> 2. User B points out a simple mistake in the branch.
That's the CVS workflow, and it is not "a" public repo but "the" public
repo shared between A and B (and also with all the project participants).
> 3. User A makes a fix
> 4. User A squashes the fix into the (already-published) history.
> 5. User A attempts to push the "fixed" history (but is rejected by
> the public repo because of non-fast-forward).
> At this point, the damage is already done,...
Which is probably a sufficient safety which the user can learn from. If
this happens too often, that probably means we are not helping them enough
to learn not to "commit --amend" or "rebase" if they are using Git as a
better CVS.
> You could say that User A should be more careful and push to a "less
> public" repo in step #1 (thus allowing the fix to be squashed before
> pushing to a "more public" repo in step #5),...
That is essentially a workflow that uses throw-away review branches in a
distributed environment, and at that point, we are not constrained by the
limitation of the CVS workflow. While still in early review cycles (which
corresponds to being in our 'pu'), "commit --amend" and "rebase" are fine
tool to be used. And...
> but how "public" is
> "public" enough to have someone point out the bug, but still
> "unpublic" enough to allow rebase?
... I can imagine that currently that is determined purely by project
convention. Perhaps there needs a way to mark throw-away review branches
like 'pu' (or saying the same thing from the different perspective, to
mark cast-in-stone integration branches like 'next') so that tools can
mechanically decide what should and should not be rewritten.
To extend the idea of promoting throw-away review branches further,
perhaps it might help if there is an easy way to let the users publish
their "master" to a branch that is not the "master" of the central shared
repository even in the CVS workflow (e.g. by default a push from user A
always goes to refs/review/A/master), and to have an option to "git push"
that makes it go to the "master" when the user really means the branch is
ready (and it would move refs/review/A/master to attic to be later gc'ed).
> ... And I
> think that refusing rewrites of commits that are already present in
> the @{upstream} remote-tracking branch is good enough to help most
> users avoid steps #4 through #6 (in a push-based workflow[1]).
See above regarding branches that should not be rebased even if they are
not directly pushed out.
> In
> fact, from a pedagogical POV, I think step #4 is probably the best
> spot for novice users to learn exactly the distinction between
> acceptable and unacceptable history rewrites (instead of having it
> explained to them as a consequence of the step #5).
I doubt you have enough information at point #4, unless you restrict the
workflow you allow your novice users to use fairly severely, to give
appropriate advice. While I agree with you that it would be the best if
we could do so at step #4 without stopping the user from doing what s/he
needs to do with false positive, I think it is not pedagogical POV but
dreaming if the world were ideal without knowing what it would take to
make it ideal.
At least I don't know offhand what kind of changes are needed to restrict
the user actions to an "approved" workflow so that step #4 can make a
useful decision (that is, no false positives and small enough false
negatives).
^ permalink raw reply
* Patchset NTLM-Authentication
From: Schmidt, Marco @ 2012-02-21 8:07 UTC (permalink / raw)
To: git; +Cc: gitster, avarab
[-- Attachment #1: Type: text/plain, Size: 401 bytes --]
Support NTLM-Authentication against proxy.
I have include a minimal patch set to support NTLM authentification
against a proxy server.
The patch adds a boolean configuration variable "http.proxyauthany" to
enable CURLOPT_PROXYAUTH = CURLAUTH_ANY.
I'm not conform with your patch submitting policy. I had no chance to
track the git repository from my current location.
Marco Schmidt
[-- Attachment #2: 0002-Einf-hren-der-HTTP-Proxy-Authentifizierung.patch --]
[-- Type: application/octet-stream, Size: 1553 bytes --]
From 8a3892eca69a0c603a53b3d3aa052f45c4a5b648 Mon Sep 17 00:00:00 2001
From: Marco Schmidt <Marco.Schmidt@cassidian.com>
Date: Fri, 27 Jan 2012 13:08:44 +0100
Subject: [PATCH 2/4] =?UTF-8?q?Einf=FChren=20der=20HTTP=20Proxy=20Authentifi?=
=?UTF-8?q?zierung?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
---
http.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/http.c b/http.c
index f3b2c90..dc51d41 100644
--- a/http.c
+++ b/http.c
@@ -41,6 +41,7 @@ static long curl_low_speed_limit = -1;
static long curl_low_speed_time = -1;
static int curl_ftp_no_epsv;
static const char *curl_http_proxy;
+static int curl_http_proxyauthany = 0;
static const char *curl_cookie_file;
static char *user_name, *user_pass, *description;
static const char *user_agent;
@@ -222,6 +223,10 @@ static int http_options(const char *var, const char *value, void *cb)
}
if (!strcmp("http.proxy", var))
return git_config_string(&curl_http_proxy, var, value);
+
+ if (!strcmp("http.proxyauthany", var)) {
+ curl_http_proxyauthany = git_config_bool(var, value);
+ }
if (!strcmp("http.cookiefile", var))
return git_config_string(&curl_cookie_file, var, value);
@@ -330,6 +335,12 @@ static CURL *get_curl_handle(void)
if (curl_http_proxy)
curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+
+ if (curl_http_proxyauthany) {
+ curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
+ printf("CURLOPT_PROXYAUTH!!!!");
+ }
+
return result;
}
--
1.7.8
[-- Attachment #3: 0003-First-working-step.patch --]
[-- Type: application/octet-stream, Size: 1519 bytes --]
From 2b7336c4959fc4df30da9e68f652f2c5941f0c91 Mon Sep 17 00:00:00 2001
From: Marco Schmidt <Marco.Schmidt@cassidian.com>
Date: Fri, 27 Jan 2012 14:35:15 +0100
Subject: [PATCH 3/4] First working step
---
http.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/http.c b/http.c
index dc51d41..3dbd390 100644
--- a/http.c
+++ b/http.c
@@ -173,6 +173,8 @@ static int git_config_path(const char **result,
static int http_options(const char *var, const char *value, void *cb)
{
+ fprintf(stderr, "http_options: %s = %s\n", var, value);
+
if (!strcmp("http.sslverify", var)) {
curl_ssl_verify = git_config_bool(var, value);
return 0;
@@ -226,6 +228,8 @@ static int http_options(const char *var, const char *value, void *cb)
if (!strcmp("http.proxyauthany", var)) {
curl_http_proxyauthany = git_config_bool(var, value);
+ fprintf(stderr, "http_options: curl_http_proxyauthany = %i", curl_http_proxyauthany);
+ return 0;
}
if (!strcmp("http.cookiefile", var))
@@ -336,9 +340,12 @@ static CURL *get_curl_handle(void)
if (curl_http_proxy)
curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+ fprintf(stderr, "DEBUG: curl_http_proxyauthany\n");
+
+ /* http proxy could be set from enviroment - set CURLOPT_PROXYAUTH */
if (curl_http_proxyauthany) {
curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
- printf("CURLOPT_PROXYAUTH!!!!");
+ fprintf(stderr, "curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);\n");
}
--
1.7.8
[-- Attachment #4: 0004-remove-debug-information.patch --]
[-- Type: application/octet-stream, Size: 1512 bytes --]
From 55b76b5f658904d240db29bb0d59f8c1dcedd4b5 Mon Sep 17 00:00:00 2001
From: Marco Schmidt <Marco.Schmidt@cassidian.com>
Date: Fri, 27 Jan 2012 14:40:57 +0100
Subject: [PATCH 4/4] remove debug information
---
http.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/http.c b/http.c
index 3dbd390..fbc52f0 100644
--- a/http.c
+++ b/http.c
@@ -173,8 +173,6 @@ static int git_config_path(const char **result,
static int http_options(const char *var, const char *value, void *cb)
{
- fprintf(stderr, "http_options: %s = %s\n", var, value);
-
if (!strcmp("http.sslverify", var)) {
curl_ssl_verify = git_config_bool(var, value);
return 0;
@@ -228,7 +226,6 @@ static int http_options(const char *var, const char *value, void *cb)
if (!strcmp("http.proxyauthany", var)) {
curl_http_proxyauthany = git_config_bool(var, value);
- fprintf(stderr, "http_options: curl_http_proxyauthany = %i", curl_http_proxyauthany);
return 0;
}
@@ -340,13 +337,12 @@ static CURL *get_curl_handle(void)
if (curl_http_proxy)
curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
- fprintf(stderr, "DEBUG: curl_http_proxyauthany\n");
-
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
/* http proxy could be set from enviroment - set CURLOPT_PROXYAUTH */
if (curl_http_proxyauthany) {
curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
- fprintf(stderr, "curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);\n");
}
+#endif
return result;
--
1.7.8
^ permalink raw reply related
* Re: git-subtree Ready #2
From: Junio C Hamano @ 2012-02-21 8:44 UTC (permalink / raw)
To: David A. Greene; +Cc: Jeff King, git, Avery Pennarun
In-Reply-To: <7vwr7gitjl.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> greened@obbligato.org (David A. Greene) writes:
>
>> Ok, but we will preserve the history via the subtree merge, yes?
>
> I'll comment on just this part, but a short answer is "no, I do not think
> so".
> ...
> I was saying that the history up to the current state, littered with these
> commits that are not "logical progression" but merely "a snapshot of
> then-current state" may not be worth preserving, with or without better
> messages.
>
> Rewriting the entire history to make it a logical progression just for the
> sake of history is obviously not worth the effort.
Having said all that, my preference is not so strong to out-right veto
doing a true merge; I wouldn't lose sleep if we end up merging the tip of
your subtree branch with all the history behind it as-is.
BUT.
Even though I freely admit that I was the guilty one who came up with
"merge -s subtree", and Linus's "gitk merge" was the original sin, having
a subtree merge like gitk, git-gui and gitweb in the history is not
without downsides.
The most problematic one that we regularly suffer from is that the
commands in the log family cannot work well across a subtree merge with
pathspec limiting, e.g. "git log git-gui/po", and we have to resort to
something like:
$ cd git-gui/po &&
git rev-list --parents HEAD . |
while read commit parent
do
git log --pretty=short $parent..$commit^2 -- :/po
done | git shortlog -n -e
to achieve what should be a simple "git shortlog -n -e git-gui/po". I
suspect that a subtree merge may also lead bisection into uninteresting
tangents as it joins otherwise disjoint history.
If we still have an active upstream that grows its history in a separate
repository, like gitk and git-gui do, we cannot avoid a subtree merge in
order to continue merging from them. Because you seem to be taking over
and are going to maintain it as part of git.git proper, eventually aiming
to move it out of contrib/, it's just that I do not think it is worth the
trouble.
^ permalink raw reply
* HTTP error 511 [Was: Secure (https) proxy authentification]
From: Nicolas Mailhot @ 2012-02-21 8:58 UTC (permalink / raw)
To: Nicolas Mailhot; +Cc: squid3, ietf-http-wg, Jeff King, git, Daniel Stenberg
In-Reply-To: <4ec05cf797322715a960743aeec0a48b.squirrel@arekh.dyndns.org>
Le Dim 19 février 2012 11:22, Nicolas Mailhot a écrit :
> 511 is exactly what I need. I was not aware of it. Is it simplemented in any
> browser yet? Where should I point the browser writers to get it implemented?
>
> http://tools.ietf.org/id/draft-nottingham-http-new-status-04.txt ?
I take that back. 511 is almost exactly what we need. However, when I pointed
the authors of some of the tools that pass through our proxy to it (curl, git)
they told me they could not parse html code in their tools, so they really
need a location (or similar) field containing the address of the
authentication portal to communicate it to the user. Without this field, they
can only stop with 'Network authentication is needed' instead of 'Please open
<url> in your browser to proceed'.
http://article.gmane.org/gmane.comp.version-control.git/191085
http://article.gmane.org/gmane.comp.version-control.git/191087
http://article.gmane.org/gmane.comp.version-control.git/191086
(the nearest thing there is in the spec is the url in meta, but it's only in
the example, not mandatory, and no one will write code for something they can
not be sure will exist)
We'd like to support those tools properly as their users' previous clumsy
attempts to navigate our current non-standard redirection method resulted in
internal security investigations.
It is a problem in our setup as we only block some URLs (others are allowed
transparently without auth), and we use several proxy farms in different
physical sites (to avoid spofs). So just opening any url in a browser won't
trigger an authentication request (the url may not be blocked, or the browser
may pass through a gateway where the user IP is already authorized, while
git/etc tried to access through another one).
Could you please revise the error 511 definition to add such a field ?
Regards,
--
Nicolas Mailhot
^ permalink raw reply
* Re* [PATCH 0/5] diff --ignore-case
From: Junio C Hamano @ 2012-02-21 9:02 UTC (permalink / raw)
To: Thomas Rast; +Cc: Johannes Sixt, git, Chris Leong
In-Reply-To: <7vvcn1l21d.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Thomas Rast <trast@inf.ethz.ch> writes:
>
>> I wonder which one of us misunderstood the original request ;-)
>
> Heh, I did ;-)
>
>> It was
>>
>> } Is there any way to run diff -G with a case insensitivity flag?
>>
>> and I took that to mean "I want to find addition/removal of a string
>> like -G does, but I don't know how it was capitalized".
>
> I think it is just the matter of checking REG_ICASE that may be set in
> revs->grep_filter.regflags, and propagating it down to the regcomp at the
> beginning of diffcore_pickaxe_grep().
>
> Want to try and see how well it works?
The gist of the patch should look like this.
Even though I haven't done anything more than just to compile and run "git
log -p -i -G search 233054d", it looks obviously correct ;-)
Note that this does not depend on any of the "diff --ignore-case" topic,
and it may conflict with [PATCH 6/5]. Among the two "else if" that flips
the REG_ICASE in revision.c, it needs to be decided if only one of them,
or both of them should set the new option. I haven't thought things through
to decide which.
diff.h | 1 +
diffcore-pickaxe.c | 6 +++++-
revision.c | 1 +
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/diff.h b/diff.h
index 7af5f1e..182cb0e 100644
--- a/diff.h
+++ b/diff.h
@@ -82,6 +82,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
#define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 27)
#define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28)
#define DIFF_OPT_FUNCCONTEXT (1 << 29)
+#define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)
#define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)
#define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 380a837..8ffb741 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -138,8 +138,12 @@ static void diffcore_pickaxe_grep(struct diff_options *o)
{
int err;
regex_t regex;
+ int cflags = REG_EXTENDED | REG_NEWLINE;
- err = regcomp(®ex, o->pickaxe, REG_EXTENDED | REG_NEWLINE);
+ if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE))
+ cflags |= REG_ICASE;
+
+ err = regcomp(®ex, o->pickaxe, cflags);
if (err) {
char errbuf[1024];
regerror(err, ®ex, errbuf, 1024);
diff --git a/revision.c b/revision.c
index 819ff01..b3554ed 100644
--- a/revision.c
+++ b/revision.c
@@ -1582,6 +1582,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->grep_filter.regflags |= REG_EXTENDED;
} else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
revs->grep_filter.regflags |= REG_ICASE;
+ DIFF_OPT_SET(&revs->diffopt, PICKAXE_IGNORE_CASE);
} else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
revs->grep_filter.fixed = 1;
} else if (!strcmp(arg, "--all-match")) {
^ permalink raw reply related
* Re: git-subtree Ready #2
From: Thomas Rast @ 2012-02-21 9:07 UTC (permalink / raw)
To: David A. Greene; +Cc: Junio C Hamano, Jeff King, git, Avery Pennarun
In-Reply-To: <87ehtowxu7.fsf@smith.obbligato.org>
greened@obbligato.org (David A. Greene) writes:
>> -GIT_BUILD_DIR="$TEST_DIRECTORY"/..
>> +
>> +if test -z "$GIT_BUILD_DIR"
>> +then
>> + echo Here
>> + # We allow tests to override this, in case they want to run tests
>> + # outside of t/, e.g. for running tests on the test library
>> + # itself.
>> + GIT_BUILD_DIR="$TEST_DIRECTORY"/..
>> +fi
>
> I'll put a patch together with a more extensive explanation. Basically,
> tests run outside of the top-level t/ directory don't work because there
> are all sort of assumptions in test-lib.sh about where they live. There
> are comments in test-lib.sh indicating that it should support tests in
> other directories but I could not make it work out of the box.
Note that this will conflict with tr/perftest, which is already in next.
It had a similar override, but then made do with the existing
GIT_TEST_INSTALLED facility. I think you do need the above here,
because you are not sourcing test-lib from the directory where the test
lives. It may then be cleaner to again use GIT_BUILD_DIR in
t/perf/perf-lib.sh since it does not require knowledge (outside of
test-lib) that $GIT_BUILD_DIR/bin-wrappers/ holds the binaries.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: Patchset NTLM-Authentication
From: Thomas Rast @ 2012-02-21 9:22 UTC (permalink / raw)
To: Schmidt, Marco; +Cc: git, gitster, avarab
In-Reply-To: <4CDEC141B5583D408E79F2931DB7708301802B70@GSX300A.mxchg.m.corp>
"Schmidt, Marco" <Marco.Schmidt@cassidian.com> writes:
> I'm not conform with your patch submitting policy. I had no chance to
> track the git repository from my current location.
You should read Documentation/SubmittingPatches. If your location does
not let you use any git:// or git+http:// ("smart http") transports, you
can use the wereHamster's bundler service https://bundler.caurea.org/ to
get a bundle that you can download over http. (If you cannot download
*anything* over http, what is your internet access good for?)
Looking at the attachments:
[0002-Einf-hren-der-HTTP-Proxy-Authentifizierung.patch]
[0003-First-working-step.patch]
[0004-remove-debug-information.patch]
* You seem to have forgotten the first patch. As it stands, the 0002
one does not apply to my tree.
* The commit messages are useless. The second one at least says
"introduce HTTP proxy authentication" which tells me something about
*what it does*, but in German. It should also tell me why and how and
possible other considerations. See SubmittingPatches.
* The commits should "spring into perfect existence". That is, in the
above you should squash 3 and 4 into 2 so that 2 starts out working
(presumably -- I'm only looking at the subjects) . You can use rebase
-i to clean everything up.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Git tree object storing policy
From: Ivan Tolstosheyev @ 2012-02-21 9:22 UTC (permalink / raw)
To: git
Hello,
now tree object is a simple list of <attributes, hash, name>sorted by name
(tricky sorted, cause we assuming that directory name "$X" is actually "$X/"
in comparison function). The problem is, that if I want to insert 10k files
in empty git repository on / folder there will be 10k new
trees with sizes from (1 to 10k)*(hash+name+attribute)+eps .
itroot@localhost ~/tmp> cat git-test.sh
#!/usr/bin/env bash
git init test
cd test
for i in `seq 1 10000`
do
touch ${i} ; git add ${i} ; git commit -m "Add ${i}" ;
done
cd ..
du -hs test
itroot@localhost ~/tmp>
itroot@localhost ~/tmp> ./git-test.sh
...
180M test
itroot@localhost ~/tmp>
180 MB!!!?? and 7.4M after `git gc` - thanks to delta compression!
Ok, you can say that this example is artificial, and I can add 10k files
with 1 commit. Thats true. But manipulating files in big tree objects
(in a big directories) is storage-expensive, and if I need to store a
lot of files in one directory and frequently change them - git just
don't scales now properly at this use-case.
What do I propose?
We can add another git object, named for example "btree" ,
that contains another "btree" objects or files. This will be a simple
btree structure (tree entries sorted practically by name, BTW,
maybe it's time to fix sorting =] ), that allows us to do insertion,
removal, search in ln(n) time. But - we do not have troubles
with big direcories now. BTW, if all directories are small, btree
will be tree-like - just btree pointing to files.
So, one big tree with 10k files transforms to (hmm, for example...)
101 btrees - one, pointing to 100 btrees, and thay points to files.
(100 entries per btree is a wild guess =) )
Suggestions?
^ 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