Git development
 help / color / mirror / Atom feed
* Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff
From: Jakub Narebski @ 2012-02-10 21:56 UTC (permalink / raw)
  To: Jeff King; +Cc: Michał Kiedrowicz, git
In-Reply-To: <20120210202008.GA5874@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> (I was hoping not to need to get a running gitweb installation in
> order to see the output).

Well, there is always git-instaweb ;-)

-- 
Jakub Narębski

^ permalink raw reply

* [PATCH] diff-highlight: Work for multiline changes too
From: Michał Kiedrowicz @ 2012-02-10 21:47 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michał Kiedrowicz
In-Reply-To: <20120210213209.GA7582@sigill.intra.peff.net>

Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
---

After looking at outputs I noticed that it can also ignore lines with
prefixes/suffixes that consist only of punctuation (asterisk, semicolon, dot,
etc), because otherwise whole line is highlighted except for terminating
punctuation.

 contrib/diff-highlight/diff-highlight |   96 ++++++++++++++++++++++-----------
 1 files changed, 65 insertions(+), 31 deletions(-)

diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight
index d893898..4811550 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -1,28 +1,40 @@
 #!/usr/bin/perl
 
+use warnings;
+use strict;
+
 # Highlight by reversing foreground and background. You could do
 # other things like bold or underline if you prefer.
 my $HIGHLIGHT   = "\x1b[7m";
 my $UNHIGHLIGHT = "\x1b[27m";
 my $COLOR = qr/\x1b\[[0-9;]*m/;
 
-my @window;
+my $context;
+my @removed = ();
+my @added = ();
+my $started = 0;
 
 while (<>) {
-	# We highlight only single-line changes, so we need
-	# a 4-line window to make a decision on whether
-	# to highlight.
-	push @window, $_;
-	next if @window < 4;
-	if ($window[0] =~ /^$COLOR*(\@| )/ &&
-	    $window[1] =~ /^$COLOR*-/ &&
-	    $window[2] =~ /^$COLOR*\+/ &&
-	    $window[3] !~ /^$COLOR*\+/) {
-		print shift @window;
-		show_pair(shift @window, shift @window);
-	}
-	else {
-		print shift @window;
+	if (/^$COLOR*-/) {
+		push @removed, $_;
+	} elsif (/^$COLOR*\+/) {
+		push @added, $_;
+	} else {
+		if ($started == 1 ) {
+			show_pairs(\@removed, \@added);
+		} else {
+			print @removed;
+			print @added;
+		}
+		@removed = ();
+		@added = ();
+		print $_;
+
+		if (/^$COLOR*(\@| )/) {
+			$started = 1;
+		} else {
+			$started = 0;
+		}
 	}
 
 	# Most of the time there is enough output to keep things streaming,
@@ -38,23 +50,33 @@ while (<>) {
 	}
 }
 
-# Special case a single-line hunk at the end of file.
-if (@window == 3 &&
-    $window[0] =~ /^$COLOR*(\@| )/ &&
-    $window[1] =~ /^$COLOR*-/ &&
-    $window[2] =~ /^$COLOR*\+/) {
-	print shift @window;
-	show_pair(shift @window, shift @window);
-}
-
-# And then flush any remaining lines.
-while (@window) {
-	print shift @window;
-}
+show_pairs(\@removed, \@added);
 
 exit 0;
 
-sub show_pair {
+sub show_pairs {
+	my $a = shift;
+	my $b = shift;
+
+	if (scalar(@{$a}) == scalar(@{$b}) && scalar(@${a}) > 0) {
+		my @removed;
+		my @added;
+
+		for(my $i = 0; $i < scalar(@{$a}); $i++) {
+			my ($rm, $add) = highlight_pair($a->[$i], $b->[$i]);
+			push @removed, $rm;
+			push @added, $add;
+		}
+
+		print @removed;
+		print @added;
+	} else {
+		print @{$a};
+		print @{$b};
+	}
+}
+
+sub highlight_pair {
 	my @a = split_line(shift);
 	my @b = split_line(shift);
 
@@ -101,8 +123,20 @@ sub show_pair {
 		}
 	}
 
-	print highlight(\@a, $pa, $sa);
-	print highlight(\@b, $pb, $sb);
+	my $prefa = join('', @a[0..($pa-1)]);
+	my $prefb = join('', @b[0..($pb-1)]);
+	my $sufa = join('', @a[($sa+1)..$#a]);
+	my $sufb = join('', @b[($sb+1)..$#b]);
+
+	# Highlight only if prefix or suffix is interesting (i.e. not consisting
+	# of color and (for prefix) +/-). Otherwise we would highlight whole
+	# lines.
+	if ($prefa =~ /^($COLOR)*-(\s|$COLOR)*$/ && $sufa =~ /^(\s|$COLOR)*$/
+		&& $prefb =~ /^($COLOR)*\+(\s|$COLOR)*$/ && $sufb =~ /^(\s|$COLOR)*$/) {
+		return join('', @a), join('', @b);
+	} else {
+		return highlight(\@a, $pa, $sa), highlight(\@b, $pb, $sb);
+	}
 }
 
 sub split_line {
-- 
1.7.3.4

^ permalink raw reply related

* Re: [PATCH 1/5] gitweb: Option for filling only specified info in fill_project_list_info
From: Junio C Hamano @ 2012-02-10 21:44 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio C Hamano, git
In-Reply-To: <201202102230.13193.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

>> Exactly.  Why do you need @fill_only at all?  If you are interested in
>> ctags and you want to make sure ctags is available, the question you want
>> to ask the helper function is "Does the project structure already have
>> ctags field?".  Why does the helper function needs to know anything else?
>
> It is to support incremental filling of project info.  The code is to
> go like this:
>
>   create
>   filter
>   fill part
>   filter
>   fill rest
>
> We need @fill_only for the "fill part".

Again, why?

> As filling project info is
> potentially expensive (especially the 'age' field),

So you wouldn't say "I am interested in 'age' field" but show interest in,
and fill, cheaper fields in the earlier "fill" calls, and then...

> doing it on narrowed
> (filtered) list of project is a performance win.

... you drop uninteresting projects by using the partially filled
information, and show interest in more expensive 'age' in the later round
for surviving projects.

It still does not explain why you need @fill_only.
Hrm...

^ permalink raw reply

* Re: Git documentation at kernel.org
From: Junio C Hamano @ 2012-02-10 21:38 UTC (permalink / raw)
  To: Ted Ts'o
  Cc: Konstantin Ryabitsev, Matthieu Moy, Clemens Buchacher, ftpadmin,
	Petr Onderka, git
In-Reply-To: <20120210212030.GD5381@thunk.org>

Ted Ts'o <tytso@mit.edu> writes:

> Hmm... good point.  That does make it hard.  I could imagine making it
> work by having separate hierarchies, and then using apache rewrite
> rules so that anything that doesn't begin with vX.Y.Z in the top level
> of software/scm/git/docs/* gets redirected to LATEST/*, where LATEST is
> a symlink that is managed via kup.

We could move vX.Y.Zs out of scm/git/docs/ hierarchy.  The existing links
from external sites rarely point at a documentation page of a specific
version, I suspect.

People can be trained to look at scm/git/old-docs/vX.Y.Z when they want to
see how older command set looked like, even those who know that in olden
days they would have consulted scm/git/docs/vX.Y.Z for that information;
they are much less of a problem than existing pages whose links want to
stay working.

^ permalink raw reply

* Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff
From: Michał Kiedrowicz @ 2012-02-10 21:36 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20120210213209.GA7582@sigill.intra.peff.net>

Jeff King <peff@peff.net> wrote:

> On Fri, Feb 10, 2012 at 10:29:16PM +0100, Michał Kiedrowicz wrote:
> 
> > > Have you considered contributing back the enhancements to
> > > contrib/diff-highlight? 
> > 
> > Yeah, I did. In fact, at work I have a hacked version of your
> > diff-highlight that supports multiline changes and I use it every day.
> > But I just couldn't make myself fix your long README and send a
> > patch :).
> 
> Heh. Why don't you show your hack in the meantime. Even if the code is
> ugly or the documentation is missing, I'd like to see the output, and
> maybe we can fix those other things together.
> 
> -Peff

Then just give must few minutes.

^ permalink raw reply

* Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff
From: Jeff King @ 2012-02-10 21:32 UTC (permalink / raw)
  To: Michał Kiedrowicz; +Cc: git
In-Reply-To: <20120210222916.2721e9e6@gmail.com>

On Fri, Feb 10, 2012 at 10:29:16PM +0100, Michał Kiedrowicz wrote:

> > Have you considered contributing back the enhancements to
> > contrib/diff-highlight? 
> 
> Yeah, I did. In fact, at work I have a hacked version of your
> diff-highlight that supports multiline changes and I use it every day.
> But I just couldn't make myself fix your long README and send a
> patch :).

Heh. Why don't you show your hack in the meantime. Even if the code is
ugly or the documentation is missing, I'd like to see the output, and
maybe we can fix those other things together.

-Peff

^ permalink raw reply

* Re: [PATCH 1/5] gitweb: Option for filling only specified info in fill_project_list_info
From: Jakub Narebski @ 2012-02-10 21:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsjiipiyu.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > If @fill_only is empty, it means for fill_project_list_info to fill
> > all the data, if it is not empty it means that those fields needs to
> > be filled.
> 
> That is exactly what I am questioning.  Do you need "I need all these
> fields to be present" and "I will fill these other fields" (which is what
> @fill_only is about, no?) that is different from the former?

"I need all these fields" is a property of callsite.  fill_project_list_info()
will be called more than one time in 2/5 to incrementally complete project
info, see below.
 
"I will fill those fields" is a property of piece of code protected by
a conditional inside fill_project_list_info().

> > The code of fill_project_list_info goes like this
> >
> >   if (do we need to fill 'age' or 'age_string'?) {
> >     fill 'age' and 'age_string'
> >   }
> >   if (do we need to fill 'desc_long' or 'descr'?) {
> >     fill 'descr_long' and 'descr'
> >   }
> >   if (we are interested in 'ctags' &&
> >       do we need to fill 'ctags'?) {
> >     fill 'ctags'
> >   }
> >   ...
> 
> Exactly.  Why do you need @fill_only at all?  If you are interested in
> ctags and you want to make sure ctags is available, the question you want
> to ask the helper function is "Does the project structure already have
> ctags field?".  Why does the helper function needs to know anything else?

It is to support incremental filling of project info.  The code is to
go like this:

  create
  filter
  fill part
  filter
  fill rest

We need @fill_only for the "fill part".  As filling project info is
potentially expensive (especially the 'age' field), doing it on narrowed
(filtered) list of project is a performance win.  That is what 2/5 is
about.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff
From: Michał Kiedrowicz @ 2012-02-10 21:29 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20120210202008.GA5874@sigill.intra.peff.net>

Jeff King <peff@peff.net> wrote:

> On Fri, Feb 10, 2012 at 10:18:12AM +0100, Michał Kiedrowicz wrote:
> 
> > The code that comares lines is based on
> > contrib/diff-highlight/diff-highlight, except that it works with
> > multiline changes too.  It also won't highlight lines that are
> > completely different because that would only make the output unreadable.
> > Combined diffs are not supported but a following commit will change it.
> 
> Have you considered contributing back the enhancements to
> contrib/diff-highlight? 

Yeah, I did. In fact, at work I have a hacked version of your
diff-highlight that supports multiline changes and I use it every day.
But I just couldn't make myself fix your long README and send a
patch :).

Maybe I'll cook something in my spare time.

> I took a look at handling multi-line changes
> when I originally wrote it, but I was worried too much about failing to
> match up lines properly, and ending up with too much noise in the diff.
> Maybe your "don't highlight lines that are completely different" rule
> helps that, though.

I must say that it works great for me. Most often it's very helping.
Like every heuristics it sometimes goes the wrong way, but it's so rare
that I don't find it disturbing.

> 
> Do you have any examples handy? (I was hoping not to need to get a
> running gitweb installation in order to see the output).
> 
> -Peff

Nope. Except for comparing diffs in various commits in gitweb-1.7.9 and
from my branch, I just created a dummy commit with different kinds of
changes to check if they are properly colorized. 

^ permalink raw reply

* Re: A note on modern git plus ancient meld ("wrong number of arguments")
From: Junio C Hamano @ 2012-02-10 21:28 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: David Aguilar, Jeff Epler, git, Sebastian Schuberth,
	Charles Bailey
In-Reply-To: <20120210082106.GA7871@burratino>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Just parse version numbers instead.  We can detect the version number
> by running "meld --version" and postprocessing it.

Hmm. I am debating myself if it may be more efficient, less error prone
and simpler for the users if we gave them "mergetool.meld.useOutput"
configuration option to tweak.

When an older meld fails when given --output for real (not with the dry
run current code tries with --help), can we sanely detect that particular
failure?  If we can do so, another possibility may be to do something like
this:

merge_cmd () {
	meld_has_output_option=$(git config --bool mergetool.meld.useOutput)
	case "$meld_has_output_option" in
        false)
		... do the non-output thing ...
		;;
	true)
		"$merge_tool_path" --output "$MERGED" "$LOCAL" "$BASE" "$REMOTE"
		;;
	*)
		"$merge_tool_path" --output "$MERGED" "$LOCAL" "$BASE" "$REMOTE"
		if it failed due to missing --output support?
		then
			meld_has_output_option=no
                        git config mergetool.meld.useOutput false
			merge_cmd
		fi
                ;;
	esac
}

^ permalink raw reply

* Re: git svn problem
From: Sam Vilain @ 2012-02-10 21:21 UTC (permalink / raw)
  To: Serhat Sevki Dincer; +Cc: git
In-Reply-To: <CAPqC6xRtZXwv+U6AKRUXDz=m-G4AjgWksbwqeMD_qzS8YC=DoQ@mail.gmail.com>

On 2/10/12 10:15 AM, Serhat Sevki Dincer wrote:
> Hi,
>
> I am using git-svn (1.7.4.1-3 on ubuntu) to get a project. It has two
> svn repositories, apparently disjoint. First half is at
> http://svn.plone.org/svn/plone/plone.app.locales, and the continuation
> at http://svn.plone.org/svn/collective/plone.app.locales
> How can i get a nice linear git-svn repository? the second one is the
> new location of the project. I am only interested in the trunks btw.

Import them separately to different git-svn remotes, and once they are 
in the same repository you can graft them together using 
.git/info/grafts (see man gitrepository-layout).  Once it looks right 
(check using 'gitk' etc), make it permanent using git filter-branch. 
You'll also want to remove the .git/svn directory, and re–run 'git svn 
fetch' so that git svn's revision database is recomputed.  Don't forget 
the -A option to 'git svn fetch'!

Good luck,
Sam

^ permalink raw reply

* Re: Git documentation at kernel.org
From: Ted Ts'o @ 2012-02-10 21:20 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Konstantin Ryabitsev, Matthieu Moy, Clemens Buchacher, ftpadmin,
	Petr Onderka, git
In-Reply-To: <7vhayyphlw.fsf@alter.siamese.dyndns.org>

On Fri, Feb 10, 2012 at 12:18:35PM -0800, Junio C Hamano wrote:
> That would not work very well without changing the historical directory
> structure (which I think was the point of this discussion "please keep
> these stale links alive").
> 
> The toplevel index.html in the pub/software/scm/git/docs/ directory and
> its pointees were the set of docs for the latest version, and older
> versions were rooted at pub/software/scm/git/docs/vX.Y.Z/.  Links that
> point at software/scm/git/docs/git-cat-file.html still need to work, and
> the path needs to be updatable without having to include the preformatted
> documentation for all the historical versions in the same tarball.

Hmm... good point.  That does make it hard.  I could imagine making it
work by having separate hierarchies, and then using apache rewrite
rules so that anything that doesn't begin with vX.Y.Z in the top level
of software/scm/git/docs/* gets redirected to LATEST/*, where LATEST is
a symlink that is managed via kup.

I don't know if the k.org folks would consider that acceptable, though.

  	     	    	  	      	   - Ted

^ permalink raw reply

* Re: [PATCH v2 28/51] refs.c: rename ref_array -> ref_dir
From: Junio C Hamano @ 2012-02-10 21:17 UTC (permalink / raw)
  To: Jeff King
  Cc: Michael Haggerty, Junio C Hamano, git, Drew Northup,
	Jakub Narebski, Heiko Voigt, Johan Herland, Julian Phillips
In-Reply-To: <20120210204457.GD5504@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> If everything_local() is trying to check that the references are in the
>> local repository plus alternates, then it is incorrect that
>> everything_local() doesn't consider alternate references in its
>> determination.  My guess is that this is the case, and that something
>> like the following might be the fix:
>
> Junio could answer more authoritatively than I, but I am pretty sure it
> is the latter. The point is to skip the expensive find_common
> negotiation if we know that there are no objects to fetch. Thus the
> "local" here is "do we have them on this side of the git-protocol
> connection", not "do we have them in our non-alternates repository".

Correct.  The function is about "do we need to get any object from the
other side?" optimization.

I originally thought to go through the rest of your message, but I
realized I can just say "everything you said is correct and I have nothing
more to add."

Thanks.

^ permalink raw reply

* Re: [PATCH] Remove empty ref directories while reading loose refs
From: Jeff King @ 2012-02-10 20:53 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Junio C Hamano, Michael Haggerty
In-Reply-To: <1328891127-17150-1-git-send-email-pclouds@gmail.com>

On Fri, Feb 10, 2012 at 11:25:27PM +0700, Nguyen Thai Ngoc Duy wrote:

> Empty directories in $GIT_DIR/refs increases overhead at startup.
> Removing a ref does not remove its parent directories even if it's the
> only file left so empty directories will be hanging around.
> [...]
> This patch removes empty directories as we see while traversing
> $GIT_DIR/refs and reverts be7c6d4 because it's no longer needed.

It feels wrong to me to be writing to the repository during what would
otherwise be a read-only operation. Especially without locking. Doesn't
this create a race condition with:

  git update-ref refs/foo/bar $sha1 &      (a)
  git for-each-ref                         (b)

if you have this sequence of events:

  1. (a) wants to create the ref, so it must first mkdir
     ".git/refs/foo".

  2. (b) is reading refs and notices the empty "foo" directory. It
     rmdirs it.

  3. (a) now attempts to create "bar" inside the newly created "foo"
     directory. This fails, because the directory does not exist.

A similar race already can happen with:

  git update-ref refs/foo/bar $sha1 &
  git update-ref refs/foo $sha1

since the latter will remove a stale "foo" directory before it can
create the new ref file.  But that race is OK, I think. Those are both
write operations, and one of them _must_ fail, because they are in
conflict (and I think even with the race they fail gracefully, with the
latter one "winning").

> pack-refs was taught of cleaning up empty directories in be7c6d4
> (pack-refs: remove newly empty directories - 2010-07-06), but it only
> checks parent directories of packed refs only. Already empty dirs are
> left untouched.

I'd much rather have pack-refs simply learn to remove all stale
directories. We at least know that "gc" is a slightly riskier operation.

-Peff

^ permalink raw reply

* Re: [PATCH v2 28/51] refs.c: rename ref_array -> ref_dir
From: Jeff King @ 2012-02-10 20:44 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: Junio C Hamano, git, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips
In-Reply-To: <4F352F03.2030104@alum.mit.edu>

On Fri, Feb 10, 2012 at 03:51:47PM +0100, Michael Haggerty wrote:

> First problem: everything_local() seems to be either broken or used
> incorrectly.  I can't decide which because I don't know what its
> semantics are *supposed* to be.
> 
> If everything_local() is trying to check that the references are all in
> the local repository itself, then it is incorrect for clone to enter
> alternates into extra_refs because everything_local() then mistakes them
> for local.
> 
> If everything_local() is trying to check that the references are in the
> local repository plus alternates, then it is incorrect that
> everything_local() doesn't consider alternate references in its
> determination.  My guess is that this is the case, and that something
> like the following might be the fix:

Junio could answer more authoritatively than I, but I am pretty sure it
is the latter. The point is to skip the expensive find_common
negotiation if we know that there are no objects to fetch. Thus the
"local" here is "do we have them on this side of the git-protocol
connection", not "do we have them in our non-alternates repository".

> > ----------------------------- builtin/fetch-pack.c -----------------------------
> > index 9500f35..4257a8d 100644
> > @@ -581,6 +581,11 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
> >  	*refs = newlist;
> >  }
> >  
> > +static void mark_alternate_complete(const struct ref *ref, void *unused)
> > +{
> > +	mark_complete(NULL, ref->old_sha1, 0, NULL);
> > +}
> > +
> >  static int everything_local(struct ref **refs, int nr_match, char **match)
> >  {
> >  	struct ref *ref;
> > @@ -609,6 +614,7 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
> >  
> >  	if (!args.depth) {
> >  		for_each_ref(mark_complete, NULL);
> > +		for_each_alternate_ref(mark_alternate_complete, NULL);
> >  		if (cutoff)
> >  			mark_recent_complete_commits(cutoff);
> >  	}
> 
> With this patch, then the full test suite passes even if I take out the
> code that adds the alternate refs to extra_refs.

That looks sane to me.

> Specifically: (without the above patch) I commented out the call to
> add_extra_ref() in clone.c:add_one_reference(), then ran t5700 through
> step 8 then aborted.  insert_one_alternate_ref() was called four times:
> 
> insert_one_alternate_ref(ccc25a1f9655742174c93f48f616bea8ad0bc6ff)
> insert_one_alternate_ref(ccc25a1f9655742174c93f48f616bea8ad0bc6ff)
> insert_one_alternate_ref(5355551c5a927a2b6349505ada2da4bb702c0a49)
> insert_one_alternate_ref(5355551c5a927a2b6349505ada2da4bb702c0a49)
> 
> (The duplication here seems strange.)

I think the duplication can be explained. The alternate-refs mechanism
gets the list of refs by running "git ls-remote" on each alternate.
The symbolic refs appear as refs in that list. So in t5700, for example,
I get:

  $ git ls-remote B
  56a2e291e54b1a92180fe2072152e6aa0919fc5f        HEAD
  56a2e291e54b1a92180fe2072152e6aa0919fc5f        refs/heads/master
  3ee812e8486c2474e2b03be5c0b42e33092da069        refs/remotes/origin/HEAD
  3ee812e8486c2474e2b03be5c0b42e33092da069        refs/remotes/origin/master

Because you don't actually care about the alternate refs themselves, but
only about their sha1 values, you could eliminate duplicates early. The
code in receive-pack already does this; fetch-pack should probably do
the same (it's not _wrong_ not to, but in some pathological cases, the
duplicates can make performance worse).

> want 5355551c5a927a2b6349505ada2da4bb702c0a49 multi_ack_detailed
> side-band-64k thin-pack ofs-delta
> want 5355551c5a927a2b6349505ada2da4bb702c0a49
> #E
> 
> The "5355551c" object corresponds to refs/remotes/origin/master in the
> alternate object store:
> 
> $ (cd B; git for-each-ref)
> ccc25a1f9655742174c93f48f616bea8ad0bc6ff commit	refs/heads/master
> 5355551c5a927a2b6349505ada2da4bb702c0a49 commit	refs/remotes/origin/HEAD
> 5355551c5a927a2b6349505ada2da4bb702c0a49 commit	refs/remotes/origin/master
> 
> It seems to me that even in the absence of short-circuiting due to
> everything_local() returning true, the presence of the alternate refs
> should be suppressing the "want" lines for those references.

Yeah, that definitely seems like a bug in find_common.

-Peff

^ permalink raw reply

* Re: 1.7.9, libcharset missing from EXTLIBS
From: Junio C Hamano @ 2012-02-10 20:25 UTC (permalink / raw)
  To: git; +Cc: dilyan.palauzov, Ævar Arnfjörð Bjarmason
In-Reply-To: <7vipjer0yn.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Дилян Палаузов <dilyan.palauzov@aegee.org> writes:
>
>>>> What I am wondering is there are systems that need to include the header,
>>>> but locale_charset() does not live in /lib/libcharset.a, in which case we
>>>> cannot make HAVE_LIBCHARSET_H imply use of -lcharset.
>>
>> I do not understand this.  If you want to use a function from
>> libcharset, you have to use both #include <libcharset.h> and
>> -lcharset.
>
> You are mistaken.
>
> The only constraint is that you have to "#include <libcharset.h>" and need
> to link with the library that has locale_charset() defined.

I think the follow-ups in this thread already demonstrated why it is an
insufficient solution to make HAVE_LIBCHARSET_H imply -lcharset.

We would instead need:

	ifeq ($(uname_S),MyHomeBrewLinux)
        	HAVE_LIBCHARSET_H = YesPlease
                EXTLIBS += -lcharset
	endif

or

	# Define NEEDS_CHARSETLIB if you use HAVE_LIBCHARSET_H and
        # need to link with -lcharset
	NEEDS_CHARSETLIB =

	ifeq ($(uname_S),MyHomeBrewLinux)
        	HAVE_LIBCHARSET_H = YesPlease
		NEEDS_CHARSETLIB = YesPlease
	endif

	ifdef NEEDS_CHARSETLIB
        	EXTLIBS += -lcharset
	endif        

or something like that, I guess.

^ permalink raw reply

* Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff
From: Jeff King @ 2012-02-10 20:24 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Michał Kiedrowicz, git
In-Reply-To: <201202101555.20163.jnareb@gmail.com>

On Fri, Feb 10, 2012 at 03:55:19PM +0100, Jakub Narebski wrote:

> > I think highlighting inline and side-by-side diff outputs is
> > something different from "git diff --word-diff". I find it useful for
> > people who are used to these diff formats (i.e. me :).
> 
> I was thinking about *using* "git diff --word-diff" for diff refinement
> highlighting of inline (unified) and side-by-side diff... 

Yeah, you might be able to get better results out of a real diff engine
(i.e., having "git diff" do the color by doing a real LCS-match) than
out of diff-highlight (which is really just a couple of simple
heuristics). OTOH, the heuristics sometimes make the result "less noisy"
because they don't find little bits of commonality, and instead
highlight only a single chunk per line.

So if somebody wanted to work on that, I'd be really curious to see if
they could get better results. It's not high enough priority for me
personally, as I find diff-highlight is "good enough".

-Peff

^ permalink raw reply

* Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff
From: Jeff King @ 2012-02-10 20:20 UTC (permalink / raw)
  To: Michał Kiedrowicz; +Cc: git
In-Reply-To: <1328865494-24415-7-git-send-email-michal.kiedrowicz@gmail.com>

On Fri, Feb 10, 2012 at 10:18:12AM +0100, Michał Kiedrowicz wrote:

> The code that comares lines is based on
> contrib/diff-highlight/diff-highlight, except that it works with
> multiline changes too.  It also won't highlight lines that are
> completely different because that would only make the output unreadable.
> Combined diffs are not supported but a following commit will change it.

Have you considered contributing back the enhancements to
contrib/diff-highlight? I took a look at handling multi-line changes
when I originally wrote it, but I was worried too much about failing to
match up lines properly, and ending up with too much noise in the diff.
Maybe your "don't highlight lines that are completely different" rule
helps that, though.

Do you have any examples handy? (I was hoping not to need to get a
running gitweb installation in order to see the output).

-Peff

^ permalink raw reply

* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Philip Oakley @ 2012-02-10 20:19 UTC (permalink / raw)
  To: Jakub Narebski, Johan Herland; +Cc: git
In-Reply-To: <201202102038.55710.jnareb@gmail.com>

From: "Jakub Narebski" <jnareb@gmail.com> Sent: Friday, February 10, 2012 
7:38 PM
> On Tue, 7 Feb 2012, Johan Herland wrote:
>
>> (we are pretty much in violent agreement, so I will only comment where
>> I find it necessary)
>
> So now comes the hard part: actually implementing (well, designing and
> implementing) prototypes for 'secret' trait and 'public' trait...

It all sounds very sensible.

The one additional implemenation idea I'd like is to have, which is somewhat 
at a tangent to the secrecy issue, is having some record of what was pushed 
(made Public). For the sub-module case this is so that its super project can 
know if its submodules are public or not. A public super project that has 
'secret' sub-modules is awkward to say the least.

That's my thought anyway...

>
>> On Tue, Feb 7, 2012 at 15:31, Jakub Narebski <jnareb@gmail.com> wrote:
>
>> > Also, when thinking about different scenarios of why one would like to
>> > mark commit as 'secret', we might want to be able to mark commit as
>> > secret / unpublishable with respect to _subset_ of remotes, so e.g.
>> > I am prevented from accidentally publishing commits marked as 'secret'
>> > to public repository, or to CI/QA repository, but I can push (perhaps
>> > with warning) to group repository, together with 'secret'-ness state
>> > of said commit...
>> >
>> > ... though it wouldn't be as much 'secret' as 'confidential' ;-)
>>
>> Another way to achieve this would be to have a config flag to control
>> whether Git checks for the 'secret' flag before pushing. This config
>> flag could be set at the system/user level (to enable/disable the
>> feature as a whole), at the repo level (to enable/disable it in a
>> given repo), at the remote level (to enable/disable it on a given
>> repo), and finally at the branch level (to enable-disable it for a
>> given branch (and its upstream)). Thus you could have a .git/config
>> that looked like this:
>>
>>   [core]
>>   refusePushSecret = true
>>
>>   [remote "foo"]
>>   refusePushSecret = false
>>   url = ...
>>   fetch = ...
>>
>>   [branch "baz"]
>>   remote = foo
>>   merge = refs/heads/baz
>>   refusePushSecret = true
>>
>> This config would:
>>
>>  - refuse to push 'secret' commits from branch 'baz'
>> (branch.baz.refusePushSecret == true)
>>
>>  - but allow to push other branches with 'secret' commits to remote
>> 'foo' (remote.foo.refusePushSecret == false)
>>
>>  - but refuse to push 'secret' commits to other remotes
>> (core.refusePushSecret == true)
>>
>> (The order of precedence would be: branch config > remote config >
>> repo config > user config > system config > default when unset)
>
> You read my mind.
>
>> I am unsure whether the 'secret'-ness of a commit should follow across
>> the push, but if you do (assuming we store the 'secret' flag using
>> git-notes) this is simply a matter of synchronizing the
>> refs/notes/secret to the same remote.
>
> I think it should, so that 'secret' commit would not escape by accident
> via a group secret repository.
>
> What makes it hard (I think) is that we would prefer to transfer
> 'secret'-ness only for pushed commits.  That might be problem for notes
> based implementation of 'secret' annotation and 'secret'-ness transfer...
> though I guess knowing that there exist 'secret' commit with given SHA1
> which we do not have and should not have is not much breach of
> confidentiality.  Still...
>
> -- 
> Jakub Narebski
> Poland
> --
> 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
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2012.0.1913 / Virus Database: 2112/4801 - Release Date: 02/10/12
> 

^ permalink raw reply

* Re: Git documentation at kernel.org
From: Junio C Hamano @ 2012-02-10 20:18 UTC (permalink / raw)
  To: Ted Ts'o
  Cc: Konstantin Ryabitsev, Matthieu Moy, Junio C Hamano,
	Clemens Buchacher, ftpadmin, Petr Onderka, git
In-Reply-To: <20120210195736.GA5381@thunk.org>

Ted Ts'o <tytso@mit.edu> writes:

> How about this as something *way* simpler?  Define a way of marking
> the top of a particular directory hierarchy as a tree.  Then the
> *only* way of updating that tree is all or nothing.  That is, someone
> submits a signed tarball; then after the signed tarball has its
> signature checked, it gets unpacked into a dir.new, and then we rename
> dir to dir.old, rename dir.new to dir, and then dir.old gets removed.
>
> That way there's no conflicts between directories that are managed via
> the kup-servers PUT and DELETE commands, and those where they get
> uploaded as a single tarball to create or replace a specific directory
> hierarcy, or which can be deleted only as a entire directory hierarcy.
>
> What do you think?

That would not work very well without changing the historical directory
structure (which I think was the point of this discussion "please keep
these stale links alive").

The toplevel index.html in the pub/software/scm/git/docs/ directory and
its pointees were the set of docs for the latest version, and older
versions were rooted at pub/software/scm/git/docs/vX.Y.Z/.  Links that
point at software/scm/git/docs/git-cat-file.html still need to work, and
the path needs to be updatable without having to include the preformatted
documentation for all the historical versions in the same tarball.

^ permalink raw reply

* Re: 1.7.9, libcharset missing from EXTLIBS
From: Erik Faye-Lund @ 2012-02-10 20:10 UTC (permalink / raw)
  To: Dilyan Palauzov; +Cc: git
In-Reply-To: <20120210205242.Horde.Ip6kLu3yGeFPNXWKXsdW3wA@webmail.aegee.org>

2012/2/10 Dilyan Palauzov <Dilyan.Palauzov@aegee.org>:
> Hello,
>
> on my system locale_charset is included in libiconv as local symbol and thus
> linking with -liconv is not sufficient.
>
>  nm /usr/lib64/libiconv.so.2.5.1 | grep locale
>  0000000000012694 t locale_charset
>
> whereas in libcharset the symbol is global:
>
>  nm /usr/lib64/libcharset.so.1 |grep locale
>  0000000000000c50 T locale_charset
>

This is from the import library for libiconv we use for Git for Windows:

$ nm /mingw/lib/libiconv.dll.a | grep locale_charset
00000000 I __imp__locale_charset
00000000 T _locale_charset

Looks pretty defined to me.

^ permalink raw reply

* Re: Git documentation at kernel.org
From: Jeff King @ 2012-02-10 20:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Clemens Buchacher, ftpadmin, Petr Onderka, git
In-Reply-To: <7vmx8rtu3e.fsf@alter.siamese.dyndns.org>

On Thu, Feb 09, 2012 at 04:23:01PM -0800, Junio C Hamano wrote:

> It might be a workable short term workaround to redirect
> 
>     http://www.kernel.org/pub/software/scm/git/docs/$anything
> 
> to
> 
>     http://schacon.github.com/git/$anything
> 
> although that would not give you an access to the list of documentations
> for older releases, e.g.
> 
>     http://www.kernel.org/pub/software/scm/git/docs/v1.6.0/git.html

If there is interest in this, we would be happy to host the
documentation. Let me know if that is the case, and we can give it a
much better URL than schacon.github.com. However, I tend to think that
since the project is hosted[1] at kernel.org, the official documentation
site should be there as well.

-Peff

[1] Of course, git being git, it is not really hosted _anywhere_ in
    particular. But convention thus far has said that the kernel.org
    repository is the official one.

^ permalink raw reply

* Re: Git documentation at kernel.org
From: Jeff King @ 2012-02-10 20:01 UTC (permalink / raw)
  To: Konstantin Ryabitsev
  Cc: Theodore Tso, Matthieu Moy, Junio C Hamano, Clemens Buchacher,
	ftpadmin, Petr Onderka, git
In-Reply-To: <1328900154.3171.27.camel@i5.mricon.com>

On Fri, Feb 10, 2012 at 01:55:54PM -0500, Konstantin Ryabitsev wrote:

> I have a few comments off the top of my head:
> 
>      1. "kup rm" will need to be modified, as it currently only allows
>         deleting things that have a matching signature. The alternative
>         is for UNPACK to create a foo.tar.manifest file that will be
>         consulted upon "kup rm" to clean up any unpacked contents upon
>         the deletion of the source archive. Note, that there are many,
>         many gotchas with this solution -- e.g. .manifest should
>         probably contain checksums, too, as there are bound to be
>         conditions when two tarballs reference the same files, and you
>         want to make sure that you delete files matching the contents of
>         the old tarball, not the newer one, etc.

For this particular use case, I don't know if that would be necessary.
According to Junio, previously:

  The k.org site kept these files under /pub/software/scm/git/docs/. The
  in-development "master" version of pages were placed directly
  underneath that directory, and the documentation pages for older
  versions were kept in vX.Y.Z subdirectory of that directory.

If we tweak that slightly to "all versions are kept in vX.Y.Z
subdirectory, and the root version is simply a symlink or redirect to
the latest vX.Y.Z", then there is no deletion required. The pusher is
always adding new versions, and updating a link[1].

But even if it would be sufficient for this use case, kup developers
may not want such a half-implemented scheme in their protocol.

-Peff

[1] There is a slight complication that the subdirectories live _inside_
    of the root directory, so it is not implementable with a single
    symlink.  You could get around that with a few clever http redirects.

^ permalink raw reply

* Re: Git documentation at kernel.org
From: Ted Ts'o @ 2012-02-10 19:57 UTC (permalink / raw)
  To: Konstantin Ryabitsev
  Cc: Matthieu Moy, Junio C Hamano, Clemens Buchacher, ftpadmin,
	Petr Onderka, git
In-Reply-To: <1328900154.3171.27.camel@i5.mricon.com>

How about this as something *way* simpler?  Define a way of marking
the top of a particular directory hierarchy as a tree.  Then the
*only* way of updating that tree is all or nothing.  That is, someone
submits a signed tarball; then after the signed tarball has its
signature checked, it gets unpacked into a dir.new, and then we rename
dir to dir.old, rename dir.new to dir, and then dir.old gets removed.

That way there's no conflicts between directories that are managed via
the kup-servers PUT and DELETE commands, and those where they get
uploaded as a single tarball to create or replace a specific directory
hierarcy, or which can be deleted only as a entire directory hierarcy.

What do you think?

						- Ted

^ permalink raw reply

* Re: 1.7.9, libcharset missing from EXTLIBS
From: Dilyan Palauzov @ 2012-02-10 19:52 UTC (permalink / raw)
  To: git
In-Reply-To: <7vipjer0yn.fsf@alter.siamese.dyndns.org>

Hello,

on my system locale_charset is included in libiconv as local symbol  
and thus linking with -liconv is not sufficient.

   nm /usr/lib64/libiconv.so.2.5.1 | grep locale
   0000000000012694 t locale_charset

whereas in libcharset the symbol is global:

   nm /usr/lib64/libcharset.so.1 |grep locale
   0000000000000c50 T locale_charset

looking at libiconv-1.4/lib/Makefile.in there is written

SOURCES = $(srcdir)/iconv.c $(srcdir)/../libcharset/lib/localcharset.c  
$(srcdir)/relocatable.c

OBJECTS = iconv.lo localcharset.lo relocatable.lo  
$(OBJECTS_EXP_@WOE32DLL@) $(OBJECTS_RES_@WOE32@)

libiconv.la : $(OBJECTS)
         $(LIBTOOL_LINK) $(CC) $(LDFLAGS) $(CFLAGS) -o libiconv.la  
-rpath $(libdir) -version-info $(LIBICONV_VERSION_INFO) -no-undefined  
$(OBJECTS)

this means, that libiconv.la includes $(OBJECTS) and thus  
libiconv-1.4/libcharset/lib/localecharset.c .

In libiconv-1.4/libcharset/lib/localecharset.c is written

#ifdef STATIC
STATIC
#endif
const char *
locale_charset (void) { ...}

and the preliminary localcharset.o still has locale_charset as global symbol:

libiconv-1.14/lib/.libs # nm localcharset.o |grep locale
0000000000000000 T locale_charset

Digging further, in libiconv-1.4/include/iconv.h some functions are  
prefixed with LIBICONV_DLL_EXPORTED , which is the same as
#define LIBICONV_DLL_EXPORTED  
__attribute__((__visibility__("default"))) , all the rest including  
locale_charset is compiled with  
__attribute__((__visibility__("hidden"))), and hence locale_charset is  
not exported from libiconv.

Greetings
   Dilian


----- Message from Дилян Палаузов <dilyan.palauzov@aegee.org> ---------
    Date: Fri, 10 Feb 2012 02:29:24 +0100
    From: Дилян Палаузов <dilyan.palauzov@aegee.org>
Subject: 1.7.9, libcharset missing from EXTLIBS
      To: git@vger.kernel.org


> Hello,
>
> git 1.7.9 makes use of libcharset and /Makefile contains:
>
> ifdef HAVE_LIBCHARSET_H
>         BASIC_CFLAGS += -DHAVE_LIBCHARSET_H
> endif
>
> when building git-daemon., the compiler reports
> make V=1
> cc  -I. -DUSE_LIBPCRE -pthread -DHAVE_PATHS_H -DHAVE_LIBCHARSET_H  
> -DHAVE_DEV_TTY -DSHA1_HEADER='<openssl/sha.h>'  -DNO_STRLCPY -o  
> git-daemon -L/usr/lib64 -L/lib64  daemon.o libgit.a xdiff/lib.a   
> -lpcre -lz  -liconv  -lcrypto -pthread
> /tmp/ccvPEthi.ltrans0.ltrans.o: In function `main':
> ccvPEthi.ltrans0.o:(.text.startup+0x59): undefined reference to  
> `locale_charset'
> collect2: ld returned 1 exit status
> make: *** [git-daemon] Error 1
>
>
> and the problem is, that libcharset is not used when linking.  To  
> solve this, please replace the above extract from /Makefile with
>
> ifdef HAVE_LIBCHARSET_H
>         BASIC_CFLAGS += -DHAVE_LIBCHARSET_H
> 	EXTLIBS += -lcharset
> endif
>
> Със здраве
>   Дилян


----- End message from Дилян Палаузов <dilyan.palauzov@aegee.org> -----

^ permalink raw reply

* Re: Git documentation at kernel.org
From: Junio C Hamano @ 2012-02-10 19:51 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Clemens Buchacher, ftpadmin, Petr Onderka, git
In-Reply-To: <vpqbop6tyj6.fsf@bauges.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Clemens Buchacher <drizzd@aon.at> writes:
>>
>>> Please restore access to the following files when possible. Some sites
>>> are referencing those, including kernel.org itself:
>>>
>>>  http://www.kernel.org/pub/software/scm/git/docs/git.html
>>
>> The pages reachable from this used to be living documents in that every
>> time the 'master' branch was updated at k.org, automatically a server side
>> hook script generated a new set of HTML pages and updated them.
>
> Is it possible to have the static HTML uploaded from another machine,
> not necessarily for each push, but e.g. for every release?

It would probably be possible, but I do not have that much time and
patience to sign 600+ files in the preformatted HTML tree one-by-one and
upload them using kup.

^ 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