Git development
 help / color / mirror / Atom feed
* [PATCH 2/3] pass struct commit to diff_tree_combined_merge()
From: René Scharfe @ 2011-12-17 10:20 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Jens Lehmann
In-Reply-To: <4EEC6BD4.4040302@lsrfire.ath.cx>

Instead of passing the hash of a commit and then searching that
same commit in the single caller, simply pass the commit directly.

---
 combine-diff.c |    7 +++----
 diff.h         |    3 ++-
 log-tree.c     |    4 +---
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index cfe6230..a2e8dcf 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1196,10 +1196,9 @@ void diff_tree_combined(const unsigned char *sha1,
 	}
 }
 
-void diff_tree_combined_merge(const unsigned char *sha1,
-			     int dense, struct rev_info *rev)
+void diff_tree_combined_merge(const struct commit *commit, int dense,
+			      struct rev_info *rev)
 {
-	struct commit *commit = lookup_commit(sha1);
 	struct commit_list *parent = commit->parents;
 	struct sha1_array parents = SHA1_ARRAY_INIT;
 
@@ -1207,6 +1206,6 @@ void diff_tree_combined_merge(const unsigned char *sha1,
 		sha1_array_append(&parents, parent->item->object.sha1);
 		parent = parent->next;
 	}
-	diff_tree_combined(sha1, &parents, dense, rev);
+	diff_tree_combined(commit->object.sha1, &parents, dense, rev);
 	sha1_array_clear(&parents);
 }
diff --git a/diff.h b/diff.h
index 96085cb..ae71f4c 100644
--- a/diff.h
+++ b/diff.h
@@ -13,6 +13,7 @@ struct strbuf;
 struct diff_filespec;
 struct userdiff_driver;
 struct sha1_array;
+struct commit;
 
 typedef void (*change_fn_t)(struct diff_options *options,
 		 unsigned old_mode, unsigned new_mode,
@@ -198,7 +199,7 @@ extern void show_combined_diff(struct combine_diff_path *elem, int num_parent,
 
 extern void diff_tree_combined(const unsigned char *sha1, const struct sha1_array *parents, int dense, struct rev_info *rev);
 
-extern void diff_tree_combined_merge(const unsigned char *sha1, int, struct rev_info *);
+extern void diff_tree_combined_merge(const struct commit *commit, int dense, struct rev_info *rev);
 
 void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b);
 
diff --git a/log-tree.c b/log-tree.c
index e7694a3..319bd31 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -599,9 +599,7 @@ int log_tree_diff_flush(struct rev_info *opt)
 
 static int do_diff_combined(struct rev_info *opt, struct commit *commit)
 {
-	unsigned const char *sha1 = commit->object.sha1;
-
-	diff_tree_combined_merge(sha1, opt->dense_combined_merges, opt);
+	diff_tree_combined_merge(commit, opt->dense_combined_merges, opt);
 	return !opt->loginfo;
 }
 
-- 
1.7.8

^ permalink raw reply related

* [PATCH 1/3] use struct sha1_array in diff_tree_combined()
From: René Scharfe @ 2011-12-17 10:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Jens Lehmann

Maintaining an array of hashes is easier using sha1_array than
open-coding it.  This patch also fixes a leak of the SHA1 array
in  diff_tree_combined_merge().

---
 builtin/diff.c |   12 ++++++------
 combine-diff.c |   34 +++++++++++++---------------------
 diff.h         |    3 ++-
 submodule.c    |   14 +++++---------
 4 files changed, 26 insertions(+), 37 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index 0fe638f..387afa7 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -14,6 +14,7 @@
 #include "log-tree.h"
 #include "builtin.h"
 #include "submodule.h"
+#include "sha1-array.h"
 
 struct blobinfo {
 	unsigned char sha1[20];
@@ -169,7 +170,7 @@ static int builtin_diff_combined(struct rev_info *revs,
 				 struct object_array_entry *ent,
 				 int ents)
 {
-	const unsigned char (*parent)[20];
+	struct sha1_array parents = SHA1_ARRAY_INIT;
 	int i;
 
 	if (argc > 1)
@@ -177,12 +178,11 @@ static int builtin_diff_combined(struct rev_info *revs,
 
 	if (!revs->dense_combined_merges && !revs->combine_merges)
 		revs->dense_combined_merges = revs->combine_merges = 1;
-	parent = xmalloc(ents * sizeof(*parent));
-	for (i = 0; i < ents; i++)
-		hashcpy((unsigned char *)(parent + i), ent[i].item->sha1);
-	diff_tree_combined(parent[0], parent + 1, ents - 1,
+	for (i = 1; i < ents; i++)
+		sha1_array_append(&parents, ent[i].item->sha1);
+	diff_tree_combined(ent[0].item->sha1, &parents,
 			   revs->dense_combined_merges, revs);
-	free((void *)parent);
+	sha1_array_clear(&parents);
 	return 0;
 }
 
diff --git a/combine-diff.c b/combine-diff.c
index 214014d..cfe6230 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -8,6 +8,7 @@
 #include "log-tree.h"
 #include "refs.h"
 #include "userdiff.h"
+#include "sha1-array.h"
 
 static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)
 {
@@ -1116,15 +1117,14 @@ static void handle_combined_callback(struct diff_options *opt,
 }
 
 void diff_tree_combined(const unsigned char *sha1,
-			const unsigned char parent[][20],
-			int num_parent,
+			const struct sha1_array *parents,
 			int dense,
 			struct rev_info *rev)
 {
 	struct diff_options *opt = &rev->diffopt;
 	struct diff_options diffopts;
 	struct combine_diff_path *p, *paths = NULL;
-	int i, num_paths, needsep, show_log_first;
+	int i, num_paths, needsep, show_log_first, num_parent = parents->nr;
 
 	diffopts = *opt;
 	diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
@@ -1144,7 +1144,7 @@ void diff_tree_combined(const unsigned char *sha1,
 			diffopts.output_format = stat_opt;
 		else
 			diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
-		diff_tree_sha1(parent[i], sha1, "", &diffopts);
+		diff_tree_sha1(parents->sha1[i], sha1, "", &diffopts);
 		diffcore_std(&diffopts);
 		paths = intersect_paths(paths, i, num_parent);
 
@@ -1199,22 +1199,14 @@ void diff_tree_combined(const unsigned char *sha1,
 void diff_tree_combined_merge(const unsigned char *sha1,
 			     int dense, struct rev_info *rev)
 {
-	int num_parent;
-	const unsigned char (*parent)[20];
 	struct commit *commit = lookup_commit(sha1);
-	struct commit_list *parents;
-
-	/* count parents */
-	for (parents = commit->parents, num_parent = 0;
-	     parents;
-	     parents = parents->next, num_parent++)
-		; /* nothing */
-
-	parent = xmalloc(num_parent * sizeof(*parent));
-	for (parents = commit->parents, num_parent = 0;
-	     parents;
-	     parents = parents->next, num_parent++)
-		hashcpy((unsigned char *)(parent + num_parent),
-			parents->item->object.sha1);
-	diff_tree_combined(sha1, parent, num_parent, dense, rev);
+	struct commit_list *parent = commit->parents;
+	struct sha1_array parents = SHA1_ARRAY_INIT;
+
+	while (parent) {
+		sha1_array_append(&parents, parent->item->object.sha1);
+		parent = parent->next;
+	}
+	diff_tree_combined(sha1, &parents, dense, rev);
+	sha1_array_clear(&parents);
 }
diff --git a/diff.h b/diff.h
index 0c51724..96085cb 100644
--- a/diff.h
+++ b/diff.h
@@ -12,6 +12,7 @@ struct diff_queue_struct;
 struct strbuf;
 struct diff_filespec;
 struct userdiff_driver;
+struct sha1_array;
 
 typedef void (*change_fn_t)(struct diff_options *options,
 		 unsigned old_mode, unsigned new_mode,
@@ -195,7 +196,7 @@ struct combine_diff_path {
 extern void show_combined_diff(struct combine_diff_path *elem, int num_parent,
 			      int dense, struct rev_info *);
 
-extern void diff_tree_combined(const unsigned char *sha1, const unsigned char parent[][20], int num_parent, int dense, struct rev_info *rev);
+extern void diff_tree_combined(const unsigned char *sha1, const struct sha1_array *parents, int dense, struct rev_info *rev);
 
 extern void diff_tree_combined_merge(const unsigned char *sha1, int, struct rev_info *);
 
diff --git a/submodule.c b/submodule.c
index 68c1ba9..788d532 100644
--- a/submodule.c
+++ b/submodule.c
@@ -373,15 +373,11 @@ static void collect_submodules_from_diff(struct diff_queue_struct *q,
 
 static void commit_need_pushing(struct commit *commit, struct commit_list *parent, int *needs_pushing)
 {
-	const unsigned char (*parents)[20];
-	unsigned int i, n;
+	struct sha1_array parents = SHA1_ARRAY_INIT;
 	struct rev_info rev;
 
-	n = commit_list_count(parent);
-	parents = xmalloc(n * sizeof(*parents));
-
-	for (i = 0; i < n; i++) {
-		hashcpy((unsigned char *)(parents + i), parent->item->object.sha1);
+	while (parent) {
+		sha1_array_append(&parents, parent->item->object.sha1);
 		parent = parent->next;
 	}
 
@@ -389,9 +385,9 @@ static void commit_need_pushing(struct commit *commit, struct commit_list *paren
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = collect_submodules_from_diff;
 	rev.diffopt.format_callback_data = needs_pushing;
-	diff_tree_combined(commit->object.sha1, parents, n, 1, &rev);
+	diff_tree_combined(commit->object.sha1, &parents, 1, &rev);
 
-	free((void *)parents);
+	sha1_array_clear(&parents);
 }
 
 int check_submodule_needs_pushing(unsigned char new_sha1[20], const char *remotes_name)
-- 
1.7.8

^ permalink raw reply related

* Re: best way to fastforward all tracking branches after a fetch
From: Sitaram Chamarty @ 2011-12-17 10:11 UTC (permalink / raw)
  To: Gelonida N; +Cc: git
In-Reply-To: <20111217101009.GA19248@sita-lt.atc.tcs.com>


[-- Attachment #1.1: Type: text/plain, Size: 2703 bytes --]

oops; forgot the program...

On Sat, Dec 17, 2011 at 03:40:09PM +0530, Sitaram Chamarty wrote:
> On Sat, Dec 10, 2011 at 01:26:32PM +0100, Gelonida N wrote:
> > Hi,
> > 
> > What is the best way to fastforward all fastforwardable tracking
> > branches after a git fetch?
> 
> I know this is a somewhat closed topic, but I took some time to
> clean up a program I have been using for a while, including some
> changes based upon ideas elsewhere in this thread.  The program
> "git-branch-check" is attached, and requires perl > 5.10.0.
> 
> Note that this does a lot more than just fast-forward all
> branches, although it can do that as well.
> 
> I alias it (in ~/.gitconfig) to 'bc', so I just run "git bc".
> Running with "-h" shows usage:
> 
>     Usage: /home/sitaram/bin/git-branch-check [options] [branches]
> 
>     Check or fast forward branches.  Default: act upon all local branches if no
>     arguments supplied, or just the current branch if '-c' is passed.
>             -c      act upon current branch only
>             -ff     don't just check, try to fast forward also
>             -md     max diff (default 100; see below for details)
>             -h      help
>     'max diff':
>         hide output for two branches different by more than so many commits
> 
> My usual usage is just "git bc -c", which may give me:
> 
>        1        pu...origin/pu
>        1        pu...github/pu
>       13        pu...master
>            5    pu...q
>            7    pu...vrs
> 
> This quickly tells me my 'pu' is one ahead of both my own
> gitolite server as well as github's copy, and that it is 13
> commits ahead of master.  The (unreleased and frequently
> rebased) feature branches 'q' and 'vrs' are ahead of pu, which
> means a rebase is not pending.  Without the "-c" I may see the
> status of master versus its own upstream and other remotes,
> etc., also.
> 
> The purpose of the max diff limit (default 100) is to hide, for
> example, the pair 'master' and 'man' from the git.git repo.
> Otherwise you'd see something like:
> 
>     27249 973    master...man
> 
> which is pretty meaningless.  The sum of those two numbers
> should be less than the max.
> 
> "git bc -ff" will attempt to fast forward all selected branches
> that are ancestors of their respective upstreams.  The current
> branch will not be ff-ed if the tree is dirty, since you can't
> do this by 'git branch -f'; it has to be an actual merge
> command.
> 
> The output is not (currently) pipable to other programs because
> I use colors (obtained from 'git config --get-color') and
> currently it is not conditional on STDOUT being a tty.



[-- Attachment #1.2: git-branch-check --]
[-- Type: text/plain, Size: 5677 bytes --]

#!/usr/bin/perl -s
use 5.10.0;
use strict;
use warnings;

# ----------------------------------------------------------------------

# bare-minimum subset of 'Tsh' (see github.com/sitaramc/tsh)
{
    my($rc, $text);
    sub rc { return $rc || 0; }
    sub text { return $text || ''; }
    sub lines { return split /\n/, $text; }
    sub try {
        my $cmd = shift; die "try: expects only one argument" if @_;
        $text = `( $cmd ) 2>&1; echo -n RC=\$?`;
        if ($text =~ s/RC=(\d+)$//) {
            $rc = $1;
            return (not $rc);
        }
        die "couldnt find RC= in result; this should not happen:\n$text\n\n...\n";
    }
}

# ----------------------------------------------------------------------

# options; the "-s" above sets one or more of these
# (the format of the lines below is special; it is used by usage() to generate
# help text for the options)
# BEGIN OPTIONS
    our $c;     # act upon current branch only
    our $ff;    # don't just check, try to fast forward also
    our $md;    # max diff (default 100; see below for details)
    our $h;     # help
# END OPTIONS
    $md ||= 100;

# get this over with; usage() exits so don't worry
    usage() if $h;

# get current branch
    my $current = '';
    try "git symbolic-ref HEAD" or die "DETACHED HEAD or no repo";
    ($current = (lines)[0]) =~ s(refs/heads/)();

# get branch names
    # first, all local branches as keys of a hash with upstream name if any, as the value
    my %upstream;
    try "git for-each-ref --perl '--format=\$upstream{%(refname:short)} = %(upstream:short);' refs/heads"
        or die "for-each-ref 1 failed";
    eval text;

    # local branches as a list; keep $current at the top, and the rest sorted
    my @local = ($current, grep { $_ ne $current } sort keys %upstream);

    # remote branches as a list
    try "git for-each-ref '--format=%(refname:short)' refs/remotes"
        or die "for-each-ref 2 failed";
    my @remote = lines;

# decide what branches to act upon.  Default: all local branches.  If any
# arguments are given, then those.  If '-c' is passed, only current branch.
    my @branches = @local;
    @branches = @ARGV if @ARGV;
    @branches = ($current) if $c;

# ----------------------------------------------------------------------

# show the tree state if it's dirty
    print "dirty:\n", text if dirty();

# process selected branches
    for my $b (@branches) {
        # attempt a fast-forward if -ff is passed
        ff($b, $upstream{$b}, $current) if ($ff);
        # check against its own upstream
        check($b, $upstream{$b});
        # then against all remote branches of the same name (I typically have
        # my own gitolite server as 'upstream' but also have github and google
        # code as additional remotes that I push my branches to)
        check($b, grep(m(^[^/]+/$b$), @remote));
    }
    # ...then against all local branches.  We do this in a separate loop
    # so their output is kept separate from the remote compares above.
    for my $b (@branches) {
        check($b, @local);
    }

# DONE...

# ----------------------------------------------------------------------
# subroutines
# ----------------------------------------------------------------------

sub ff {
    # b=branch, u=upstream, c=current
    my ($b, $u, $c) = @_;

    unless ($u) {
        say "$b does not have an upstream";
        return;
    }

    if ($b eq $c and dirty()) {
        say "working tree is dirty; skipping ff for (current branch) $b";
        return;
    }

    # $l = number of commits "l"eft side has over the "r"ight (similarly $r...)
    my($l, $r) = compare($b, $u);
    if ($r and not $l) {
        # there is something to update, and ff is possible
        if ($b eq $c) {
            # current branch; needs an actual merge
            try("git merge --ff-only $u") or die "$b: 'git merge --ff-only $u' failed:\n" .  text;
        } else {
            # other branches can be forced
            try("git branch -f $b $u") or die "$b: 'git branch -f $b $u' failed:\n" .  text;
        }
    }
}

sub check {
    my ($b, @list) = @_;
    state %seen;

    for my $u (@list) {
        next unless $u;
        next if $b eq $u or $seen{$b}{$u};
        # seeing a...b is as good as seeing b...a also
        $seen{$b}{$u} = 1;
        $seen{$u}{$b} = 1;

        my ($l, $r) = compare($b, $u);

        my $abs = $l + $r; next unless $abs;    # if they're equal, don't show it
        next if $abs >= $md;                    # if they're too far apart, don't show it

        print spacepad(4, $l) . color('green') . ($l || ' ');
        print spacepad(4, $r) . color('red')   . ($r || ' ');
        say color('reset') . "    $b...$u";
    }
}

sub compare {
    my ($b, $u) = @_;

    try("git rev-list $u..$b") or die "'git rev-list $u..$b' failed:\n" .  text;
    my $l = lines;

    try("git rev-list $b..$u") or die "'git rev-list $b..$u' failed:\n" .  text;
    my $r = lines;

    return($l, $r);
}

sub dirty {
    try "git status -s -uno | cut -c1-2 | sort | uniq -c; /./";
}

sub color {
    my $color = shift;
    return `git config --get-color "" $color`;
}

sub spacepad {
    return " " x ($_[0] - length($_[1]));
}

sub usage {
    print "
Usage: $0 [options] [branches]

Check or fast forward branches.  Default: act upon all local branches if no
arguments supplied, or just the current branch if '-c' is passed.
";
    @ARGV=($0);
    for ( grep { /BEGIN OPTIONS/../END OPTIONS/ and not /OPTION/ } <> ) {
        s/our \$/\t-/;
        s/; *#/\t/;
        print;
    }
    say "\'max diff':\n    hide output for two branches different by more than so many commits";
    exit 1;
}

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: best way to fastforward all tracking branches after a fetch
From: Sitaram Chamarty @ 2011-12-17 10:10 UTC (permalink / raw)
  To: Gelonida N; +Cc: git
In-Reply-To: <jbvj5o$skt$1@dough.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2477 bytes --]

On Sat, Dec 10, 2011 at 01:26:32PM +0100, Gelonida N wrote:
> Hi,
> 
> What is the best way to fastforward all fastforwardable tracking
> branches after a git fetch?

I know this is a somewhat closed topic, but I took some time to
clean up a program I have been using for a while, including some
changes based upon ideas elsewhere in this thread.  The program
"git-branch-check" is attached, and requires perl > 5.10.0.

Note that this does a lot more than just fast-forward all
branches, although it can do that as well.

I alias it (in ~/.gitconfig) to 'bc', so I just run "git bc".
Running with "-h" shows usage:

    Usage: /home/sitaram/bin/git-branch-check [options] [branches]

    Check or fast forward branches.  Default: act upon all local branches if no
    arguments supplied, or just the current branch if '-c' is passed.
            -c      act upon current branch only
            -ff     don't just check, try to fast forward also
            -md     max diff (default 100; see below for details)
            -h      help
    'max diff':
        hide output for two branches different by more than so many commits

My usual usage is just "git bc -c", which may give me:

       1        pu...origin/pu
       1        pu...github/pu
      13        pu...master
           5    pu...q
           7    pu...vrs

This quickly tells me my 'pu' is one ahead of both my own
gitolite server as well as github's copy, and that it is 13
commits ahead of master.  The (unreleased and frequently
rebased) feature branches 'q' and 'vrs' are ahead of pu, which
means a rebase is not pending.  Without the "-c" I may see the
status of master versus its own upstream and other remotes,
etc., also.

The purpose of the max diff limit (default 100) is to hide, for
example, the pair 'master' and 'man' from the git.git repo.
Otherwise you'd see something like:

    27249 973    master...man

which is pretty meaningless.  The sum of those two numbers
should be less than the max.

"git bc -ff" will attempt to fast forward all selected branches
that are ancestors of their respective upstreams.  The current
branch will not be ff-ed if the tree is dirty, since you can't
do this by 'git branch -f'; it has to be an actual merge
command.

The output is not (currently) pipable to other programs because
I use colors (obtained from 'git config --get-color') and
currently it is not conditional on STDOUT being a tty.

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: git-svn: multiple fetch lines
From: Jonathan Nieder @ 2011-12-17 10:05 UTC (permalink / raw)
  To: Nathan Gray; +Cc: git, Eric Wong
In-Reply-To: <CA+7g9Jxd3mhbra34f+MiJRt36Lb6gVHi1nOCP8Zo5y-G9jB3qA@mail.gmail.com>

Hi Nathan,

Nathan Gray wrote:

> I'm in a conversation with the support fellow of the very nice Tower
> git interface for OS X and we need clarification on a point.  Does
> git-svn explicitly support multiple "fetch =" lines in an svn-remote
> section or is it just an accident that it works?  My belief is that
> such support is intended and his is that it is accidental.

It's true that the documentation is not as clear about this as one
might like.  Documentation/git-svn.txt leaves it to the reader to
infer that this is supported by analogy with "fetch =" lines in native
git [remote] sections:

	'git svn' stores [svn-remote] configuration information in the
	repository .git/config file.  It is similar the core git
	[remote] sections except 'fetch' keys do not accept glob
	arguments; but they are instead handled by the 'branches'
	and 'tags' keys.

The change description for the patch that introduced that functionality
is more helpful.

	$ git log -S'Could not find a \"svn-remote.*.fetch\" key' git-svn.perl
	commit 706587fc
	Author: Eric Wong <normalperson@yhbt.net>
	Date:   Thu Jan 18 17:50:01 2007 -0800

	    git-svn: add support for metadata in .git/config
	    
	    Of course, we handle metadata migrations from previous versions
	    and we have added unit tests.
	    
	    The new .git/config remotes resemble non-SVN remotes.  Below
	    is an example with comments:
[example using multiple 'fetch' keys snipped]

Perhaps such an example would be useful for the git-svn(1) manpage,
too.  Any ideas for where to add such a note, and how it should be
worded?  (Of course, if you can phrase such an idea in patch form,
that would be the most convenient.)

Hope that helps,
Jonathan

^ permalink raw reply

* Re: [PATCH] Escape file:// URL's to meet subversion SVN::Ra requirements
From: Jonathan Nieder @ 2011-12-17  9:50 UTC (permalink / raw)
  To: Ben Walton; +Cc: normalperson, git
In-Reply-To: <1320251895-6348-2-git-send-email-bwalton@artsci.utoronto.ca>

Hi again,

Ben Walton wrote:

> Previously only http/https URL's were uri escaped.  When building
> against subversion 1.7, this was causing a segfault in perl after
> an assertion failure in the SVN::Ra bindings during in t9134.

(Not a segfault, just a core dump.)  Thanks.

[....]
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -5366,6 +5366,9 @@ sub escape_url {
>  	if ($url =~ m#^(https?)://([^/]+)(.*)$#) {
>  		my ($scheme, $domain, $uri) = ($1, $2, escape_uri_only($3));
>  		$url = "$scheme://$domain$uri";
> +	} elsif ($url =~ m#^(file)://(.*)$#) {
> +		my ($scheme, $uri) = ($1, escape_uri_only($2));
> +		$url = "$scheme://$uri";

This has two obvious effects, one good and one bad.

The good effect is that it converts spaces to %20.  Both old and new
versions of Subversion seem to be happy to treat %20 as a space, and
new versions of Subversion are not happy to treat a space as a space,
so this conversion can only be a good thing.

The bad effect is that it converts percent signs to %25.  So commands
like "git svn clone file:///path/to/test%20repository" that previously
worked might not work any more, if v1.6.5-rc0~61 (svn: assume URLs
from the command-line are URI-encoded, 2009-08-16) did not do its job
completely.

In other words, it seems to me like you are on the right track. ;-)

Another possible approach: to imitate the svn command line tools, we
could use SVN::Client::url_from_path in some appropriate place. 

Next steps:

 - track down the trouble on svn 1.6.x that Eric mentioned
 - fix any remaining tests that still don't pass

Thanks for getting this started.  Will sleep and play with it a little
more.

Jonathan

^ permalink raw reply

* Re: [PATCH] make "git push -v" actually verbose
From: Jeff King @ 2011-12-17  9:41 UTC (permalink / raw)
  To: git; +Cc: Tay Ray Chuan, Junio C Hamano
In-Reply-To: <20111217093713.GA2073@sigill.intra.peff.net>

On Sat, Dec 17, 2011 at 04:37:15AM -0500, Jeff King wrote:

> Providing a single "-v" to "git push" currently does
> nothing. Giving two flags ("git push -v -v") turns on the
> first level of verbosity.

One minor clarification: it is not technically true that "git push -v"
does nothing. It just does not do the interesting "show a verbose status
table" operation, which is almost certainly what the user wants (and
what happened before the commits I mentioned). It does print "Pushing to
$url", since that happens above the transport layer. But I'm pretty sure
that is not what users of "-v" are interested in. :)

-Peff

^ permalink raw reply

* [PATCH] make "git push -v" actually verbose
From: Jeff King @ 2011-12-17  9:37 UTC (permalink / raw)
  To: git; +Cc: Tay Ray Chuan, Junio C Hamano

Providing a single "-v" to "git push" currently does
nothing. Giving two flags ("git push -v -v") turns on the
first level of verbosity.

This is caused by a regression introduced in 8afd8dc (push:
support multiple levels of verbosity, 2010-02-24). Before
the series containing 8afd8dc, the verbosity handling for
fetching and pushing was completely separate. Commit bde873c
refactored the verbosity handling out of the fetch side, and
then 8afd8dc converted push to use the refactored code.

However, the fetch and push sides numbered and passed along
their verbosity levels differently. For both, a verbosity
level of "-1" meant "quiet", and "0" meant "default output".
But from there they differed.

For fetch, a verbosity level of "1" indicated to the "fetch"
program that it should make the status table slightly more
verbose, showing up-to-date entries. A verbosity level of
"2" meant that we should pass a verbose flag to the
transport; in the case of fetch-pack, this displays protocol
debugging information.

As a result, the refactored code in bde873c checks for
"verbosity >= 2", and only then passes it on to the
transport. From the transport code's perspective, a
verbosity of 0 or 1 both meant "0".

Push, on the other hand, does not show its own status table;
that is always handled by the transport layer or below
(originally send-pack itself, but these days it is done by
the transport code). So a verbosity level of 1 meant that we
should pass the verbose flag to send-pack, so that it knows
we want a verbose status table. However, once 8afd8dc
switched it to the refactored fetch code, a verbosity level
of 1 was now being ignored.  Thus, you needed to
artificially bump the verbosity to 2 (via "-v -v") to have
any effect.

We can fix this by letting the transport code know about the
true verbosity level (i.e., let it distinguish level 0 or
1).

We then have to also make an adjustment to any transport
methods that assumed "verbose > 0" meant they could spew
lots of debugging information. Before, they could only get
"0" or "2", but now they will also receive "1". They need to
adjust their condition for turning on such spew from
"verbose > 0" to "verbose > 1".

Signed-off-by: Jeff King <peff@peff.net>
---
This is an old bug, obviously, but it has been bugging me
off and on for the past year, so I finally decided to track
it down.

Sorry for the epic-length commit message. It was a subtle
bug, and the actual patch makes it very difficult to
understand why it works and doesn't regress the fetch case.
Hopefully it made sense. :)

 transport.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/transport.c b/transport.c
index 51814b5..48002b9 100644
--- a/transport.c
+++ b/transport.c
@@ -215,7 +215,7 @@ static struct ref *get_refs_via_rsync(struct transport *transport, int for_push)
 	rsync.argv = args;
 	rsync.stdout_to_stderr = 1;
 	args[0] = "rsync";
-	args[1] = (transport->verbose > 0) ? "-rv" : "-r";
+	args[1] = (transport->verbose > 1) ? "-rv" : "-r";
 	args[2] = buf.buf;
 	args[3] = temp_dir.buf;
 	args[4] = NULL;
@@ -268,7 +268,7 @@ static int fetch_objs_via_rsync(struct transport *transport,
 	rsync.argv = args;
 	rsync.stdout_to_stderr = 1;
 	args[0] = "rsync";
-	args[1] = (transport->verbose > 0) ? "-rv" : "-r";
+	args[1] = (transport->verbose > 1) ? "-rv" : "-r";
 	args[2] = "--ignore-existing";
 	args[3] = "--exclude";
 	args[4] = "info";
@@ -351,7 +351,7 @@ static int rsync_transport_push(struct transport *transport,
 	args[i++] = "-a";
 	if (flags & TRANSPORT_PUSH_DRY_RUN)
 		args[i++] = "--dry-run";
-	if (transport->verbose > 0)
+	if (transport->verbose > 1)
 		args[i++] = "-v";
 	args[i++] = "--ignore-existing";
 	args[i++] = "--exclude";
@@ -527,7 +527,7 @@ static int fetch_refs_via_pack(struct transport *transport,
 	args.lock_pack = 1;
 	args.use_thin_pack = data->options.thin;
 	args.include_tag = data->options.followtags;
-	args.verbose = (transport->verbose > 0);
+	args.verbose = (transport->verbose > 1);
 	args.quiet = (transport->verbose < 0);
 	args.no_progress = !transport->progress;
 	args.depth = data->options.depth;
@@ -981,7 +981,7 @@ int transport_set_option(struct transport *transport,
 void transport_set_verbosity(struct transport *transport, int verbosity,
 	int force_progress)
 {
-	if (verbosity >= 2)
+	if (verbosity >= 1)
 		transport->verbose = verbosity <= 3 ? verbosity : 3;
 	if (verbosity < 0)
 		transport->verbose = -1;
-- 
1.7.7.4.13.g57bf4

^ permalink raw reply related

* [PATCH 2/3] gitweb: esc_html() site name for title in OPML
From: Jakub Narebski @ 2011-12-17  9:22 UTC (permalink / raw)
  To: git; +Cc: Juergen Kreileder, John Hawley, admin
In-Reply-To: <1324113743-21498-1-git-send-email-jnareb@gmail.com>

From: Jürgen Kreileder <jk@blackdown.de>

This escapes the site name in OPML (XML uses the same escaping rules
as HTML).  Also fixes encoding issues because esc_html() uses
to_utf8().

Signed-off-by: Jürgen Kreileder <jk@blackdown.de>
Acked-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 35126cd..dcf4658 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7863,11 +7863,12 @@ sub git_opml {
 		-charset => 'utf-8',
 		-content_disposition => 'inline; filename="opml.xml"');
 
+	my $title = esc_html($site_name);
 	print <<XML;
 <?xml version="1.0" encoding="utf-8"?>
 <opml version="1.0">
 <head>
-  <title>$site_name OPML Export</title>
+  <title>$title OPML Export</title>
 </head>
 <body>
 <outline text="git RSS feeds">
-- 
1.7.6

^ permalink raw reply related

* [PATCH 3/3] gitweb: Output valid utf8 in git_blame_common('data')
From: Jakub Narebski @ 2011-12-17  9:22 UTC (permalink / raw)
  To: git; +Cc: Juergen Kreileder, John Hawley, admin
In-Reply-To: <1324113743-21498-1-git-send-email-jnareb@gmail.com>

From: Jürgen Kreileder <jk@blackdown.de>

Otherwise when javascript-actions are enabled gitweb shown broken
author names in the tooltips on blame pages ('blame_incremental'
view).

Signed-off-by: Jürgen Kreileder <jk@blackdown.de>
Acked-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index dcf4658..d24763b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6244,7 +6244,9 @@ sub git_blame_common {
 			-type=>"text/plain", -charset => "utf-8",
 			-status=> "200 OK");
 		local $| = 1; # output autoflush
-		print while <$fd>;
+		while (my $line = <$fd>) {
+			print to_utf8($line);
+		}
 		close $fd
 			or print "ERROR $!\n";
 
-- 
1.7.6

^ permalink raw reply related

* [PATCH 1/3] gitweb: Call to_utf8() on input string in chop_and_escape_str()
From: Jakub Narebski @ 2011-12-17  9:22 UTC (permalink / raw)
  To: git; +Cc: Juergen Kreileder, John Hawley, admin
In-Reply-To: <1324113743-21498-1-git-send-email-jnareb@gmail.com>

From: Jürgen Kreileder <jk@blackdown.de>

a) To fix the comparison with the chopped string,
   otherwise we compare bytes with characters, as
   chop_str() must run to_utf8() for correct operation
b) To give the title attribute correct encoding;
   we need to mark strings as UTF-8 before outpur

Signed-off-by: Jürgen Kreileder <jk@blackdown.de>
Acked-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f80f259..35126cd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1696,6 +1696,7 @@ sub chop_and_escape_str {
 	my ($str) = @_;
 
 	my $chopped = chop_str(@_);
+	$str = to_utf8($str);
 	if ($chopped eq $str) {
 		return esc_html($chopped);
 	} else {
-- 
1.7.6

^ permalink raw reply related

* [PATCH 0/3 (resend)] gitweb: Various to_utf8 / esc_html fixes
From: Jakub Narebski @ 2011-12-17  9:22 UTC (permalink / raw)
  To: git; +Cc: Juergen Kreileder, John Hawley, admin, Jakub Narebski

Sorry for resend of this series, but I forgot to generate patches in
UTF-8 instead of i18n.logoutputencoding=iso-8859-2


This is post-release resend of Jürgen patches (which were sent
during feature-freeze).

I have slightly extended commit messages, and added my ACK.

Jürgen Kreileder (3):
  gitweb: Call to_utf8() on input string in chop_and_escape_str()
  gitweb: esc_html() site name for title in OPML
  gitweb: Output valid utf8 in git_blame_common('data')

 gitweb/gitweb.perl |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

-- 
1.7.6

^ permalink raw reply

* [PATCH 3/3] gitweb: Output valid utf8 in git_blame_common('data')
From: Jakub Narebski @ 2011-12-17  9:15 UTC (permalink / raw)
  To: git; +Cc: Juergen Kreileder, John Hawley, admin
In-Reply-To: <1324113324-21328-1-git-send-email-jnareb@gmail.com>

From: Jürgen Kreileder <jk@blackdown.de>

Otherwise when javascript-actions are enabled gitweb shown broken
author names in the tooltips on blame pages ('blame_incremental'
view).

Signed-off-by: Jürgen Kreileder <jk@blackdown.de>
Acked-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index dcf4658..d24763b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6244,7 +6244,9 @@ sub git_blame_common {
 			-type=>"text/plain", -charset => "utf-8",
 			-status=> "200 OK");
 		local $| = 1; # output autoflush
-		print while <$fd>;
+		while (my $line = <$fd>) {
+			print to_utf8($line);
+		}
 		close $fd
 			or print "ERROR $!\n";
 
-- 
1.7.6

^ permalink raw reply related

* [PATCH 2/3] gitweb: esc_html() site name for title in OPML
From: Jakub Narebski @ 2011-12-17  9:15 UTC (permalink / raw)
  To: git; +Cc: Juergen Kreileder, John Hawley, admin
In-Reply-To: <1324113324-21328-1-git-send-email-jnareb@gmail.com>

From: Jürgen Kreileder <jk@blackdown.de>

This escapes the site name in OPML (XML uses the same escaping rules
as HTML).  Also fixes encoding issues because esc_html() uses
to_utf8().

Signed-off-by: Jürgen Kreileder <jk@blackdown.de>
Acked-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 35126cd..dcf4658 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7863,11 +7863,12 @@ sub git_opml {
 		-charset => 'utf-8',
 		-content_disposition => 'inline; filename="opml.xml"');
 
+	my $title = esc_html($site_name);
 	print <<XML;
 <?xml version="1.0" encoding="utf-8"?>
 <opml version="1.0">
 <head>
-  <title>$site_name OPML Export</title>
+  <title>$title OPML Export</title>
 </head>
 <body>
 <outline text="git RSS feeds">
-- 
1.7.6

^ permalink raw reply related

* [PATCH 1/3] gitweb: Call to_utf8() on input string in chop_and_escape_str()
From: Jakub Narebski @ 2011-12-17  9:15 UTC (permalink / raw)
  To: git; +Cc: Juergen Kreileder, John Hawley, admin
In-Reply-To: <1324113324-21328-1-git-send-email-jnareb@gmail.com>

From: Jürgen Kreileder <jk@blackdown.de>

a) To fix the comparison with the chopped string,
   otherwise we compare bytes with characters, as
   chop_str() must run to_utf8() for correct operation
b) To give the title attribute correct encoding;
   we need to mark strings as UTF-8 before outpur

Signed-off-by: Jürgen Kreileder <jk@blackdown.de>
Acked-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f80f259..35126cd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1696,6 +1696,7 @@ sub chop_and_escape_str {
 	my ($str) = @_;
 
 	my $chopped = chop_str(@_);
+	$str = to_utf8($str);
 	if ($chopped eq $str) {
 		return esc_html($chopped);
 	} else {
-- 
1.7.6

^ permalink raw reply related

* [PATCH 0/3] gitweb: Various to_utf8 / esc_html fixes
From: Jakub Narebski @ 2011-12-17  9:15 UTC (permalink / raw)
  To: git; +Cc: Juergen Kreileder, John Hawley, admin, Jakub Narebski

This is post-release resend of Jürgen patches (which were sent
during feature-freeze).

I have slightly extended commit messages, and added my ACK.

Jürgen Kreileder (3):
  gitweb: Call to_utf8() on input string in chop_and_escape_str()
  gitweb: esc_html() site name for title in OPML
  gitweb: Output valid utf8 in git_blame_common('data')

 gitweb/gitweb.perl |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

-- 
1.7.6

^ permalink raw reply

* Re: [PATCH] Escape file:// URL's to meet subversion SVN::Ra requirements
From: Jonathan Nieder @ 2011-12-17  8:45 UTC (permalink / raw)
  To: Eric Wong; +Cc: Ben Walton, git
In-Reply-To: <20111102220941.GA3925@dcvr.yhbt.net>

Hi Eric,

Eric Wong wrote:

> I don't have much time to help you fix it, but I got numerous errors on
> SVN 1.6.x (svn 1.6.12).  Can you make sure things continue to work on
> 1.6 and earlier, also?
[...]
> Here are the tests that failed for me:
>
> make[1]: *** [t9100-git-svn-basic.sh] Error 1
[...]

I tried the patch with subversion 1.6.17dfsg-3 from Debian and all
tests passed.  Odd.  Could you send output from running the test with
-v -i with Ben's patch applied?

Ah, the subversion CHANGES file mentions a bugfix that might explain
the difference:

| Version 1.6.13
[...]
|    * properly canonicalize a URL (r984928, -31)

Thanks,
Jonathan

^ permalink raw reply

* Re: [msysGit] [PATCH] gitk: fix the display of files when filtered by path
From: Junio C Hamano @ 2011-12-17  6:31 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: Martin von Zweigbergk, Pat Thoyts, Johannes Schindelin,
	David Aguilar, Git, msysGit
In-Reply-To: <20111215225945.GG20629@bloggs.ozlabs.ibm.com>

Paul Mackerras <paulus@samba.org> writes:

> On Thu, Dec 15, 2011 at 11:42:38AM -0800, Martin von Zweigbergk wrote:
>
>> git://git.kernel.org/pub/scm/gitk/gitk.git is still down.
>
> I have just created a repository on ozlabs.org for gitk, since I don't
> have kernel.org access at this point.  The repository is:
>
> git://ozlabs.org/~paulus/gitk.git
> ...
> Your patches are in the master branch.  I applied them back in July
> but then kernel.org went down.

Thanks.

All pulled and resulted in one liner update to the draft Release Notes for
the next release.

diff --git a/Documentation/RelNotes/1.7.9.txt b/Documentation/RelNotes/1.7.9.txt
index cd3c256..f476667 100644
--- a/Documentation/RelNotes/1.7.9.txt
+++ b/Documentation/RelNotes/1.7.9.txt
@@ -4,6 +4,8 @@ Git v1.7.9 Release Notes (draft)
 Updates since v1.7.8
 --------------------
 
+ * Accumulated gitk updates since early this year.
+
  * git-gui updated to 0.16.0.
 
  * git-p4 (in contrib/) updates.

^ permalink raw reply related

* Re: [PATCH] docs: brush up obsolete bits of git-fsck manpage
From: Junio C Hamano @ 2011-12-17  6:10 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111217012811.GC20225@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Using "--patience", on the other hand, does find it as a common line,

Ahh, that must be it.  Thanks, and sorry for the noise.

^ permalink raw reply

* Re: git-p4 issue
From: Michael Horowitz @ 2011-12-17  5:52 UTC (permalink / raw)
  To: Vitor Antunes; +Cc: git
In-Reply-To: <loom.20110513T162233-874@post.gmane.org>

Vitor,

I know it has been a long time, but I finally tried the below, and it
works for me.  I was doing things wrong at first, but I think I
finally understand what is going on...

First, the branchList is great, that is what I needed.  Now I can
completely ignore the p4 branches.  It would be great if there was an
option to not have it query p4 branches at all, because we have so
many branches here it takes forever, and I am just using branchList.
What I did for now was to use your branchUser option, and pass it a
bogus username, like "XXXXXX".  At least this way it returns fast, but
would be nice to not have it try to query at all.

I think I understand now why all this is needed if you want it to
detect branches on its own.  Since in Perforce branches are just
directories, there is no way to tell if a directory is a branch or
not, and people could organize branches in an arbitrary directory
structure.  I was thinking you could look at sibling directories under
the same parent, which may be common, but there is no way to know for
sure, so better to be explicit.

The other odd issue is that if you do not add the new branch to
branchList (or p4 branches if you are using them) before you do the
git-p4 sync, then since it goes through change numbers in order, it
will never go back again and catch the branch in a change it already
synced.  This is why I could never get it to work, because I never had
it in place before I synced the relevant change.  What I realized
though is that I can always do a "git-p4 sync --detect-branches
//depot/foo/bar/@all" again, even though I already have the most
recent change, and it will re-detect any branches I missed (given I
have since added them to branchList).

What I had been thinking though is that if it had already detected
branches once, it could at least update the remote p4 branches it
already knew about to point at the latest revision, just not try to
detect new ones, and not rely on branchList.  Of course you would
still need branchList to detect any new branches.

Anyway, works a lot better than it did before.

Thanks,

Mike


On Fri, May 13, 2011 at 10:31 AM, Vitor Antunes <vitor.hda@gmail.com> wrote:
> Hi Michael,
>
> Michael Horowitz <michael.horowitz <at> ieee.org> writes:
>
>>
>> Vitor,
>>
>> > Could you please search for the following set of patches in this
>> > mailing list?
>> >
>> > [PATCH v2 0/3] git-p4: Improve branch support
>> >
>> > I think I sent v2 twice somehow, so please make sure you pick the
>> > latest ;)
>> > In these patches I add the possibility to use a "git-p4.branchList"
>> > configuration to define the branches. The patch is still to be
>> > approved because most people in the mailing list do not use branch
>> > detection, but I use it daily and it is working in my side. Could
>> > you please test it?
>>
>> Can you send this patch again?  It looks like you had previously
>> responded to the list only, so I never got this message, as I wasn't
>> on the list at the time (I am now).  I only saw this because I was
>> searching the archive for something else.  I searched for the patch,
>> but the actual patch body doesn't seem to be in the archive.
>
> I am also not in the mailing list, I just follow its RSS and try to
> follow up on the git-p4 related topics ;) That is the reason why you
> were not included in the reply.
>
> But I have been a bit busy and did not see this email passing by. Sorry.
> Luckily I had a tab opened in this thread, which I looked at today
> wondering what it was about! :P
>
> Please follow the link to the thread [1] and you can open each of the
> entries. Take care not to apply patch 1/3 as it may require you to clone
> everything again.
>
>> Thanks,
>>
>> Mike
>>
>
> Regards,
> Vitor
>
> [1] http://thread.gmane.org/gmane.comp.version-control.git/167998/focus=168000
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] attr: map builtin userdiff drivers to well-known extensions
From: Jonathan Nieder @ 2011-12-17  3:38 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, Junio C Hamano, git, Brandon Casey
In-Reply-To: <20111217012118.GB20225@sigill.intra.peff.net>

Jeff King wrote:

>                          Or maybe just drop the C ones (and probably the
> objc one based on other parts of the thread) and do the rest?

Yes, that.

This way, someone wondering why the C ones are not used by default can
easily look at your patch, see that they are utterly broken, and help
us fix it.  That's how progress is made.

^ permalink raw reply

* Re:  Adding hooks.directory config option; wiring into run_hook
From: Aaron Schrab @ 2011-12-17  3:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Christopher Dale, git
In-Reply-To: <7vmxasie6k.fsf@alter.siamese.dyndns.org>

At 10:06 -0800 16 Dec 2011, Junio C Hamano <gitster@pobox.com> wrote:
>Christopher Dale <chrelad@gmail.com> writes:
>
>> ...
>> trusted path execution policies. These systems require that any file
>> that can be executed exhibit at least the following characteristics:
>>
>>   * The executable, it's directory, and each directory above it are
>>     not writable.
>>
>> Since the hooks directory is within a directory that by it's very nature
>> requires write permissions,...
>
>Sorry, but I am not interested at all. If you can write into $GIT_DIR/config
>then you can point at any directory with hooks.directory and that would mean
>it would defeat your "trusted path execution policies".

How does that defeat the policy?  It would certainly allow somebody who 
can write to $GIT_DIR to disable hooks, use hooks that were meant for a 
different repository, or perhaps even use executables that weren't 
intended to be hooks.  If the proposed configuration option were 
modified by an attacker to point to some directory that doesn't meet the 
requirements, then the underlying system would still prevent execution; 
the same result that would happen if they'd try to install hooks in the 
traditional location.

I see other problems with that policy, at least with the short 
description that was provided.  Unless there are also restrictions on 
the allowed owners of the executable and its containing directories, an 
attacker would still be able to install new executables and then remove 
write permissions before trying to use them.  But, that flaw (if it 
exists) wouldn't be affected by the proposed change to git.

Requiring that all executables on a secured system be installed by an 
admin doesn't seem to be a completely unreasonable requirement.  The 
supplied patch looks to be fairly small and easy to understand, so I 
wouldn't think that including it would present much of a problem for 
maintaining git.

The option might also be useful to allow the same hooks directory to be 
used for multiple repositories, although symlinks would likely be a 
better way to do that.

^ permalink raw reply

* Re:  Adding hooks.directory config option; wiring into run_hook
From: Aaron Schrab @ 2011-12-17  1:58 UTC (permalink / raw)
  To: Christopher Dale; +Cc: git
In-Reply-To: <CADQnX_e76LzuRUDOKFOsRHU=e8Cw+qh5x1BdW5HMEdMmP5PaHg@mail.gmail.com>

At 12:00 -0600 16 Dec 2011, Christopher Dale <chrelad@gmail.com> wrote:
>Since the hooks directory is within a directory that by it's very 
>nature requires write permissions,

Are you sure about this?  I'd think that this would mainly be used for 
bare repositories.  For a test I created a bare repository, removed 
write permission, changed the owner to root, then pushed into it from 
another repository.  So it seems that, at least for basic usage, a bare 
repository can be modified even if with write permission removed since 
no entries are being created or removed at the top level.

For this to be a workaround, new repositories would need to be created 
by an admin.  The requirement that the containing directory not be 
writeable wouldn't necessarily be an issue; at least on Linux I'm able 
to create files/directories as root even in a non-writeable directory.

^ permalink raw reply

* Re: [PATCH] docs: brush up obsolete bits of git-fsck manpage
From: Jeff King @ 2011-12-17  1:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy5ucgsic.fsf@alter.siamese.dyndns.org>

On Fri, Dec 16, 2011 at 12:40:11PM -0800, Junio C Hamano wrote:

> By the way did you hand-tweak your patch in any way?
> 
> I am not complaining that it does not apply (it does), but I am curious
> how you got the line that begins with "corruption it finds ..." neatly in
> preimage and postimage; it could become a common line but doing so makes
> the patch unreadable and that is what I am getting from "git show" after
> applying the patch.

No, I don't think I tweaked it at all. You can fetch the original commit
(888b4eb) at:

  git://github.com/peff/git.git jk/fsck-docs

Running "git show" produces the same output as the patch I sent. I
double-checked with older versions of git, and they all produce the same
output for me. Ditto for applying what I sent and running "git show" on
the result.

Using "--patience", on the other hand, does find it as a common line,
Weird.

-Peff

^ permalink raw reply

* Re: [PATCH] attr: map builtin userdiff drivers to well-known extensions
From: Jeff King @ 2011-12-17  1:21 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git, Brandon Casey
In-Reply-To: <4EEBC0A7.3030303@kdbg.org>

On Fri, Dec 16, 2011 at 11:05:27PM +0100, Johannes Sixt wrote:

> Am 16.12.2011 20:21, schrieb Jeff King:
> > I'm not clear from what you wrote on whether you were saying it is
> > simply sub-optimal, or whether on balance it is way worse than the
> > default funcname matching.
> 
> I'm saying the latter. Okay, we're talking "only" about hunk headers.
> But when you are reviewing patches, they are *extremely* useful and a
> time-saver; when they are wrong or not present, they are exactly the
> opposite.

Right. I don't think it is worth arguing "well, it's only funcname
headers". Because that same argument applies to both the advantages
(i.e., hopefully with the patch we are generating better funcname
headers) and disadvantage (i.e., it seems that we might be generating
worse funcname headers).

So it is really a question of "how good" or "how bad" for each style.

> Sure I have. What I didn't say (sorry for that!), but wanted to hint at
> is that this is to experiment with a pattern in order to ultimately
> improve the built-in pattern. The topic came up just the other day, and
> I took Thomas Rast's suggestion to experiment with a simplified pattern:
> 
> http://thread.gmane.org/gmane.comp.version-control.git/186355/focus=186439
> 
> But as is, the built-in pattern misses way too many anchor points in C++
> code.

Yeah, I can certainly agree that the patterns could be better.

Maybe we should just table the extension mapping for now, then, and see
if the patterns improve? Or maybe just drop the C ones (and probably the
objc one based on other parts of the thread) and do the rest?

-Peff

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox