Git development
 help / color / mirror / Atom feed
* Re: core.autocrlf & Cygwin - files incorrectly flagged as modified
From: Robin Rosenberg @ 2009-12-17  7:00 UTC (permalink / raw)
  To: David Antliff; +Cc: git
In-Reply-To: <loom.20091215T225528-115@post.gmane.org>

tisdag 15 december 2009 23:24:15 skrev  David Antliff:
> I approached the Cygwin mailing list about this problem, and the package
> maintainer suggested I ask here as well, in case someone can shed some
>  light on this.
> 
> I am using git-1.6.4.2 (as part of Cygwin-1.7 beta) and I've observed some
> behaviour that seems wrong to me.
> 
> When we deployed git on Cygwin, we decided to set core.autocrlf=true, as
>  many of our users are working with DOS/Windows files (CRLF endings). Some
>  users are using Linux and working on the same code. I'm not actually sure
>  why we chose to use this setting - there's a lot of conflicting advice
>  around. I think the final decision was made so that kdiff3 would operate
>  correctly. However we did go to lengths to ensure that everyone is using
>  the same configuration setting.
> 
> The problem is that sometimes, after a git-clone, the output of git-status
>  and git-diff shows entire files as being different. However these files
>  have not been modified by the user - only git has had a chance to change
>  them (due to autocrlf=true). But surely if git has converted the file
>  automatically, it should know that it has to compensate for this when
>  comparing with the local repository?

AFAIK, this happens if you have CRLF line endings in the blobs in the repo.

-- robin

^ permalink raw reply

* first-parent equivalent for show-branch
From: Jeenu V @ 2009-12-17  7:31 UTC (permalink / raw)
  To: git

Hi,

I wanted to know if there's a --first-parent equivalent for git
show-branch, where in it shows only the first -parents in a vertical
column. I feel it'd make the display less cluttered.

As of now, I think, a column shows commits in a merged branch, and
even parents of that branch, which makes reading bit uneasy. What
would be preferable instead to show only the first parent in a column
- something like the output of

git log --graph --pretty=oneline --abbrev-commit --first-parent --all
| sed 's#|\|/##g'

along with the header and naming strings of show-branch.

Thanks
-- 
:J

^ permalink raw reply

* [RFC/PATCH] gitweb: link to toggle 'no merges' option
From: Giuseppe Bilotta @ 2009-12-17  9:05 UTC (permalink / raw)
  To: git; +Cc: jnareb, Giuseppe Bilotta

In views that support --no-merges, introduce a link that toggles the
option.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>

---
 gitweb/gitweb.css  |   11 +++++++++++
 gitweb/gitweb.perl |   14 ++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

This is something I've been wanting for a while. There are a number of
things that don't 'click' with this proof of concept, and I'm coming to
the list to hear the opinion of users and developers on how to improve
the thing.

The patch is live at http://git.oblomov.eu/, an example affected page is
http://git.oblomov.eu/git/shortlog

Things that are sure to change:

 * the aesthetics and location of the toggle link (it shows on mousehover
   in the title). Other options I've considered are:
    + next to pagination (first | prev | next), either before or after
      the existing entries
    + on mouseover for the table section that refers to the (short)log;
      this would make it possible to put it summary view too, for example

 * if you toggle merge view when not on the first page, the reference
   (first) commit in the view is likely to change drastically, which
   causes confusion. I have not found a satisfactory solution for this,
   since the obvious way to 'lock' the view (start paginating from the
   current top commit) prevents prev/next navigation

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 50067f2..0da6ef0 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -572,3 +572,14 @@ span.match {
 div.binary {
 	font-style: italic;
 }
+
+span.merge_toggle a {
+	font-size: 66%;
+	color: white !important;
+	font-weight: normal;
+	vertical-align: top;
+	text-decoration:none;
+	visibility: hidden;
+}
+
+*:hover > span.merge_toggle a { visibility:visible }
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7e477af..a63f419 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3118,11 +3118,15 @@ sub git_header_html {
 	my $status = shift || "200 OK";
 	my $expires = shift;
 
+	my $can_have_merges = grep(/^$action$/, @{$allowed_options{'--no-merges'}});
+	my $has_merges = !grep(/^--no-merges$/, @extra_options);
+
 	my $title = "$site_name";
 	if (defined $project) {
 		$title .= " - " . to_utf8($project);
 		if (defined $action) {
 			$title .= "/$action";
+			$title .= " (no merges)" unless $has_merges;
 			if (defined $file_name) {
 				$title .= " - " . esc_path($file_name);
 				if ($action eq "tree" && $file_name !~ m|/$|) {
@@ -3235,6 +3239,16 @@ EOF
 		print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
 		if (defined $action) {
 			print " / $action";
+			if ($can_have_merges) {
+				print " <span class='merge_toggle'>";
+				if ($has_merges) {
+					printf('<a href="%s">hide merges</a>', href(-replay=>1, 'extra_options'=>('--no-merges', @extra_options)));
+				} else {
+					my @href_extra = grep(!/^--no-merges$/, @extra_options);
+					printf('<a href="%s">show merges</a>', href(-replay=>1, 'extra_options'=>@href_extra));
+				}
+				print "</span>";
+			}
 		}
 		print "\n";
 	}
-- 
1.6.3.rc1.192.gdbfcb

^ permalink raw reply related

* Re: [PATCH] Let format-patch and rebase ignore trivial merges.
From: Bernhard R. Link @ 2009-12-17  9:35 UTC (permalink / raw)
  To: git; +Cc: Johannes Sixt
In-Reply-To: <4B29106C.1040501@viscovery.net>

* Johannes Sixt <j.sixt@viscovery.net> [091216 17:53]:
> Bernhard R. Link schrieb:
> > --prune-tree makes rev-list without paths equivalent to
> > "git rev-list $options -- ." (or .. or ../.. and so on,
> > if you are in some subdirectory).
> > This is the new default for format-patch and rebase
>
> Why do you need a new option when you can just add "-- ." to the rev-list
> invocation?

I want the default for format-patch changed.

For this I think it is easiest to add a new rev_info flag, as otherwise
format-patch would need to duplicate parsing the rev_list options
and either duplicate applying revs->prune_data or changing the argv for
setup_revisions with some special casing of bare repository and non-bare
repository cases.

And if there is that rev_info flag I think it is most logical to make
it accessible from the outside.

That also allows to revert to the old format-patch behaviour,
in case someone uses format-patch not to get some appliable patches but
some differently formated log or for something else I cannot imagine.

And when there is that option, I think it is more robust to use that
in merge -m and merge -i, as "-- ." only does the right thing by chance
because both only work with a non-bare repository and have
cd_to_toplevel.

Hochachtungsvoll,
	Bernhard R. Link
-- 
Please do not CC me if git@vger.kernel.org also gets a copy.

^ permalink raw reply

* Re: How do I show only log messages for commits on a specific branch?
From: David Roden @ 2009-12-17  9:20 UTC (permalink / raw)
  To: git
In-Reply-To: <20091216101647.GB27373@bc-bd.org>

bd@bc-bd.org wrote:

> imagine this:
> 
> --A--*--B (new)
> /
> *--X--*--Y (master)
> 
> Now I'd like to list only log messages for A..B (X..B would be okay too).
> 
> I know of
> 
> git log master..new
> 
> however then I need to remember that I branched new of master, and to be
> honest, sometimes I forget.
> 
> So how do I:
> 
> git please-tell-me-the-branch-I-started-this-branch-from new
> 
> Thanks in advance,

I have had wonderful results using

    git log new --not master

or

    git log new ^master

This is using git 1.6.4.4 and this way of specifying commits is not 
mentioned in the man page—it seems to work, though.

	David

^ permalink raw reply

* how can I push a sub-tree
From: Zhi Li @ 2009-12-17  9:50 UTC (permalink / raw)
  To: git

Hi,

I tried to find a way to push a sub-tree. In section 6.7 of "Pro Git",
there's a way for pulling a git sub-tree. But I have not found the
opposite: push a git sub-tree. Can someone help me?

BTW, I appologize for not reading archives of this list.

Zhi

^ permalink raw reply

* Re: how can I push a sub-tree
From: Tay Ray Chuan @ 2009-12-17 10:14 UTC (permalink / raw)
  To: Zhi Li; +Cc: git
In-Reply-To: <2986b3940912170150o17118a07i8f91e785c08e2ac9@mail.gmail.com>

Hi,

On Thu, Dec 17, 2009 at 5:50 PM, Zhi Li <lizhi1215@gmail.com> wrote:
> I tried to find a way to push a sub-tree. In section 6.7 of "Pro Git",
> there's a way for pulling a git sub-tree. But I have not found the
> opposite: push a git sub-tree. Can someone help me?

I've not read the book, but perhaps you mean submodules?

If so, refer to the part in the git submodule tutorial that describes
how to make changes within a submodule on the GitWiki at
http://git.or.cz/gitwiki/GitSubmoduleTutorial.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: Transplant branch from another repository
From: Jeenu V @ 2009-12-17 11:06 UTC (permalink / raw)
  To: git
In-Reply-To: <E1NFVyv-0000EN-Qx@xyzzy.farnsworth.org>

On Tue, Dec 1, 2009 at 10:23 PM, Dale Farnsworth <dale@farnsworth.org> wrote:
> [...]
> Try "cd B; git fetch A a:b"

Thanks.
-- 
:J

^ permalink raw reply

* Re: [PATCH] Let format-patch and rebase ignore trivial merges.
From: Johannes Sixt @ 2009-12-17 11:40 UTC (permalink / raw)
  To: git
In-Reply-To: <20091217093547.GA25451@pcpool00.mathematik.uni-freiburg.de>

Bernhard R. Link schrieb:
> * Johannes Sixt <j.sixt@viscovery.net> [091216 17:53]:
>> Bernhard R. Link schrieb:
>>> --prune-tree makes rev-list without paths equivalent to
>>> "git rev-list $options -- ." (or .. or ../.. and so on,
>>> if you are in some subdirectory).
>>> This is the new default for format-patch and rebase
>> Why do you need a new option when you can just add "-- ." to the rev-list
>> invocation?
> 
> I want the default for format-patch changed.

I do not see why format-patch would have to be changed. The case that you
outline (a merge -s ours happened and you want to follow only one parent)
is rare enough and even more rarly will somebody want to apply
format-patch to such a history.

But I guess that you are actually not interested in format-patch per se,
but rather in rebase (which uses format-patch).

> For this I think it is easiest to add a new rev_info flag, as otherwise
> format-patch would need to duplicate parsing the rev_list options
> and either duplicate applying revs->prune_data or changing the argv for
> setup_revisions with some special casing of bare repository and non-bare
> repository cases.

I haven't looked at the code, but wouldn't it be matter of "if we do not
have any pathspec, add '.'" *after* all options are parsed?

> And when there is that option, I think it is more robust to use that
> in merge -m and merge -i, as "-- ." only does the right thing by chance
> because both only work with a non-bare repository and have
> cd_to_toplevel.

git rev-list -- . works in a bare repository, too. If you hard-code "-- ."
in the rev-list invocations in git-rebase[--interactive], then it cannot
be said that this works "by chance" due to cd_to_toplevel.

-- Hannes

^ permalink raw reply

* Re: potential null dereference
From: René Scharfe @ 2009-12-17 12:30 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: git
In-Reply-To: <4B2783DD.5060301@gmail.com>

Am 15.12.2009 13:41, schrieb Jiri Slaby:
> Hi,
> 
> Stanse found the following error in unpack-trees.c:
> dereferencing NULL pointer here.[. * o src_index]
> 
> int unpack_trees(unsigned len, struct tree_desc *t, struct
> unpack_trees_options *o)
> {
>  int ret;
>  static struct cache_entry *dfc;
> ...
>  if (o->src_index) {                   <-- loc0
>   o->result.timestamp.sec = o->src_index->timestamp.sec;
>   o->result.timestamp.nsec = o->src_index->timestamp.nsec;
>  }
>  o->merge_size = len;
> 
>  if (!dfc)
>   dfc = xcalloc(1, ((1 + (0) + 8) & ~7));
>  o->df_conflict_entry = dfc;
> 
>  if (len) {
> ...
>  }
> 
>  if (o->merge) {
>   while (o->pos < o->src_index->cache_nr) { <-- here
> 
> It triggers, because there is a test for o->src_index being NULL at
> loc0, but here, it is dereferenced without a check. Can this happen
> (e.g. does o->merge != NULL imply o->src_index != NULL)?

Running "git grep -w -B70 unpack_trees" and looking for "src_index"
using less' search command showed me that src_index is never NULL when
unpack_trees() is called.

> Further, there is a warning in log-tree.c:
> pointer always points to valid memory here, but checking for not
> NULL.[parents]
> 
> static int log_tree_diff(struct rev_info *opt, struct commit *commit,
> struct log_info *log)
> {
>  int showed_log;
>  struct commit_list *parents;
>  unsigned const char *sha1 = commit->object.sha1;
> 
>  if (!opt->diff && !((&opt->diffopt)->flags & (1 << 14)))
>   return 0;
> 
> 
>  parents = commit->parents;
>  if (!parents) {            <-- loc0
>   if (opt->show_root_diff) {
>    diff_root_tree_sha1(sha1, "", &opt->diffopt);
>    log_tree_diff_flush(opt);
>   }
>   return !opt->loginfo;     <-- loc1
>  }
> 
>  if (parents && parents->next) { <-- here
> 
> I.e. if parents was NULL at loc0, we escaped at loc1. But we check
> parents against NULL here again.

The check may be duplicate, but I suspect removing it won't change the
resulting object code -- the compiler should be smart enough to come to
the same conclusion.

Thanks,
René

^ permalink raw reply

* Re: b5227d8 changes meaning of "ls-files -x 'pattern'"
From: Sitaram Chamarty @ 2009-12-17 12:54 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20091214182420.GA31594@coredump.intra.peff.net>

On Mon, Dec 14, 2009 at 11:55 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Dec 14, 2009 at 09:21:06PM +0530, Sitaram Chamarty wrote:

>> Before b5227d8, the following two commands would produce different
>> outputs (say on git.git):
>>
>>       git ls-files
>>       git ls-files -x '*.c'
>>
>> From b5227d8 onward, they produce the same output.   The second command
>> no longer excludes *.c files.
>>
>> I was unable to understand the commit message completely but it sounds
>> like this was intentionally changed to do this.

> Yes, it was intentional. Excludes are about untracked files, not about
> restricting parts of the index. The point of the change was to bring
> "ls-files" in harmony with other parts of git. For example, prior to

OK; makes sense -- thanks for the explanation.

> However, for your use case, I can see the utility of an option to limit
> the output of ls-files for a particular invocation. It's just that "-x"
> is tied into the excludes mechanism, which doesn't do that.
>
> I would not be opposed to a patch to add an option that means "exclude
> these index entries from the output list." And for the sake of backwards

I rather doubt if my C skills extend that far these days; I'll figure out
other ways of doing this if I need it :-)

Thanks again,

Sitaram

^ permalink raw reply

* Re: [RFC/PATCH] gitweb: link to toggle 'no merges' option
From: Etienne Vallette d'Osia @ 2009-12-17 13:03 UTC (permalink / raw)
  To: git
In-Reply-To: <1261040753-4859-1-git-send-email-giuseppe.bilotta@gmail.com>

Oh, this is exactly what I wanted for a long long time...
I hope this feature will be merged soon !

Le 12/17/2009 10:05 AM, Giuseppe Bilotta a écrit :
> In views that support --no-merges, introduce a link that toggles the
> option.
> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> 
> ---
>  gitweb/gitweb.css  |   11 +++++++++++
>  gitweb/gitweb.perl |   14 ++++++++++++++
>  2 files changed, 25 insertions(+), 0 deletions(-)
> 
> This is something I've been wanting for a while. There are a number of
> things that don't 'click' with this proof of concept, and I'm coming to
> the list to hear the opinion of users and developers on how to improve
> the thing.
> 
> The patch is live at http://git.oblomov.eu/, an example affected page is
> http://git.oblomov.eu/git/shortlog
> 
> Things that are sure to change:
> 
>  * the aesthetics and location of the toggle link (it shows on mousehover
>    in the title). Other options I've considered are:
>     + next to pagination (first | prev | next), either before or after
>       the existing entries
>     + on mouseover for the table section that refers to the (short)log;
>       this would make it possible to put it summary view too, for example
> 
>  * if you toggle merge view when not on the first page, the reference
>    (first) commit in the view is likely to change drastically, which
>    causes confusion. I have not found a satisfactory solution for this,
>    since the obvious way to 'lock' the view (start paginating from the
>    current top commit) prevents prev/next navigation
> 
> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
> index 50067f2..0da6ef0 100644
> --- a/gitweb/gitweb.css
> +++ b/gitweb/gitweb.css
> @@ -572,3 +572,14 @@ span.match {
>  div.binary {
>  	font-style: italic;
>  }
> +
> +span.merge_toggle a {
> +	font-size: 66%;
> +	color: white !important;
> +	font-weight: normal;
> +	vertical-align: top;
> +	text-decoration:none;
> +	visibility: hidden;
> +}
> +
> +*:hover > span.merge_toggle a { visibility:visible }
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 7e477af..a63f419 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3118,11 +3118,15 @@ sub git_header_html {
>  	my $status = shift || "200 OK";
>  	my $expires = shift;
>  
> +	my $can_have_merges = grep(/^$action$/, @{$allowed_options{'--no-merges'}});
> +	my $has_merges = !grep(/^--no-merges$/, @extra_options);
> +
>  	my $title = "$site_name";
>  	if (defined $project) {
>  		$title .= " - " . to_utf8($project);
>  		if (defined $action) {
>  			$title .= "/$action";
> +			$title .= " (no merges)" unless $has_merges;
>  			if (defined $file_name) {
>  				$title .= " - " . esc_path($file_name);
>  				if ($action eq "tree" && $file_name !~ m|/$|) {
> @@ -3235,6 +3239,16 @@ EOF
>  		print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
>  		if (defined $action) {
>  			print " / $action";
> +			if ($can_have_merges) {
> +				print " <span class='merge_toggle'>";
> +				if ($has_merges) {
> +					printf('<a href="%s">hide merges</a>', href(-replay=>1, 'extra_options'=>('--no-merges', @extra_options)));
> +				} else {
> +					my @href_extra = grep(!/^--no-merges$/, @extra_options);
> +					printf('<a href="%s">show merges</a>', href(-replay=>1, 'extra_options'=>@href_extra));
> +				}
> +				print "</span>";
> +			}
>  		}
>  		print "\n";
>  	}

--
Etienne Vallette d'Osia

^ permalink raw reply

* Re: diff attribute ignored by show and log -p
From: Nanako Shiraishi @ 2009-12-17 13:17 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <76718490912162123r49f9bd90n8e032c144d0cdbac@mail.gmail.com>

Quoting Jay Soffian <jaysoffian@gmail.com>

>> But is apparently ignored by "git log -p" and "git show" which just
>> use internal diff. Is this behavior intentional?
>
> Ah, --ext-diff, and the reasoning behind requiring it for log/show is
> explained in 72909be.

The need to give --ext-diff is mentioned in 72909be (Add diff-option
--ext-diff, 2007-06-30) but its log message doesn't 'explain' why external
diff isn't used by default and you need to pass that extra option.

Probably --ext-diff should be the default?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: How do I show only log messages for commits on a specific branch?
From: Stefan Völkel @ 2009-12-17 14:20 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Elijah Newren, git
In-Reply-To: <20091217002024.GM25474@genesis.frugalware.org>

Hi,

> git log $(git reflog show new|sed -n 's/ .*//;$ p')..new

I had trouble getting the above to work as a git alias (someething wrt
escaping I guess), however this:

        gl = log --graph --pretty=oneline --abbrev-commit
	bl = !sh -c 'git gl $0 --not $(git for-each-ref --format=%\\(refname\\)
refs/heads/* | grep -v refs/heads/$0)'

does exactly what I want:

	# git reflog show new
	76cacd3 new@{0}: commit: B
	9638379 new@{1}: commit: A
	7944f55 new@{2}: branch: Created from HEAD

	# git gl
	* 76cacd3 B
	* 9638379 A
	* 7944f55 Y
	* b8e4a96 X

	# git bl new
	* 76cacd3 B
	* 9638379 A

Thanks for the help,

	Stefan

^ permalink raw reply

* Re: How do I show only log messages for commits on a specific branch?
From: Pat Notz @ 2009-12-17 15:04 UTC (permalink / raw)
  To: bd; +Cc: git
In-Reply-To: <20091216101647.GB27373@bc-bd.org>

> So how do I:
>
>        git please-tell-me-the-branch-I-started-this-branch-from new
>

You may also be interested in the git show-branch command.  The output
may be unintuitive at first but once you grok it it's pretty useful...
especially if you have several branches.

Aside from the man page, here's a blog post the describes the output:
https://wincent.com/wiki/Understanding_the_output_of_%22git_show-branch%22

~ Pat

^ permalink raw reply

* Re: [RFC/PATCH] gitweb: link to toggle 'no merges' option
From: Jakub Narebski @ 2009-12-17 15:37 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git
In-Reply-To: <1261040753-4859-1-git-send-email-giuseppe.bilotta@gmail.com>

On Thu, 17 Dec 2009 10:05 +0100, Giuseppe Bilotta wrote:

> In views that support --no-merges, introduce a link that toggles the
> option.

I like this idea of introducing interface for so far hidden feature
(well, except hidden in one of feed <link ...> in page header).
 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> 
> ---
>  gitweb/gitweb.css  |   11 +++++++++++
>  gitweb/gitweb.perl |   14 ++++++++++++++
>  2 files changed, 25 insertions(+), 0 deletions(-)
> 
> This is something I've been wanting for a while. There are a number of
> things that don't 'click' with this proof of concept, and I'm coming to
> the list to hear the opinion of users and developers on how to improve
> the thing.
> 
> The patch is live at http://git.oblomov.eu/, an example affected page is
> http://git.oblomov.eu/git/shortlog

You should tell here that one must put mouse over main header (the one
with 'projects / project / action' breadcrumbs) for the new link to be
visible... because I was wondering where is this new link.

> 
> Things that are sure to change:
> 
>  * the aesthetics and location of the toggle link (it shows on mousehover
>    in the title). 

It is not only the question where to put link, but also where to put
the *code* (where the code should belong to).

At first I thought: WTF? Why the feature that deals with log-like views
is put in very generic and common for all actions/views git_header_html
subroutine?  Especially after change that made all loglike views use
common infrastructure of git_log_generic.

But then I realized that it is specific example of *generic* feature
that deals with extra_options... which admittedly is currently limited
to '--no-merges' only.  So if it is put in git_header_html, then all
views with HTML output (which does not include 'atom' and 'rss' actions,
but which actions IIRC have their own handling of '--no-merges')
which have support for extra_options would have ability to turn them
on and off.

What you need to add (if this link is to be in git_header_html) is
to create links only when $status == 200, because otherwise the link
would be present also for error pages, which I think is wrong.

>    Other options I've considered are: 
>     + next to pagination (first | prev | next), either before or after
>       the existing entries

This would fit with the fact that sometimes present "patches" link is on
the line with pagination links, after pagination links.

But this secondary navigation bar is about other views, and extra_options
is about modifying current view, and functions more like toggles.  OTOH
we have such toggle for 'blame' <-> 'blame_incremental' switch in 
secondary navigation bar.

Also this would mean that each view type would have to handle extra_options
itself, as secondary navigation bar is very much action-specific.  Not
that it matters now, with only single '--no-merges' option supported.

>     + on mouseover for the table section that refers to the (short)log;
>       this would make it possible to put it summary view too, for example

This would mean having link inside link, as those headers in summary view
functions as link to 'shortlog' view (quite useful I think), and to the
project summary in the 'shortlog' view itself (I'm not sure how useful 
that is).  We already have problems with ref markers being links inside
links and having broken layout in some strict XHTML conformance browsers.


In short: I am not sure what would be the best solution.  Nevertheless
I think that link should be more visible, and perhaps more toggle-like.

>  * if you toggle merge view when not on the first page, the reference
>    (first) commit in the view is likely to change drastically, which
>    causes confusion. I have not found a satisfactory solution for this,
>    since the obvious way to 'lock' the view (start paginating from the
>    current top commit) prevents prev/next navigation

Alternate solution would be to clean page (start from 0th page) when
changing to '--no-merges'.

You would probably need something like 'skip' option, with number of
commits, not number of pages to skip, and/or 'skip_to' which takes
commit-id.  But i have not thought about this much... 

> 
> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
> index 50067f2..0da6ef0 100644
> --- a/gitweb/gitweb.css
> +++ b/gitweb/gitweb.css
> @@ -572,3 +572,14 @@ span.match {
>  div.binary {
>  	font-style: italic;
>  }
> +
> +span.merge_toggle a {
> +	font-size: 66%;
> +	color: white !important;
> +	font-weight: normal;
> +	vertical-align: top;
> +	text-decoration:none;
> +	visibility: hidden;
> +}

I think it should be more visible.

Otherwise only people "in the know" would be able to use this.

> +
> +*:hover > span.merge_toggle a { visibility:visible }

I'd rather not have this rule to have different style, i.e. not all
in single line.  Unless it is for RFC only...

> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 7e477af..a63f419 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3118,11 +3118,15 @@ sub git_header_html {
>  	my $status = shift || "200 OK";
>  	my $expires = shift;
>  
> +	my $can_have_merges = grep(/^$action$/, @{$allowed_options{'--no-merges'}});
> +	my $has_merges = !grep(/^--no-merges$/, @extra_options);
> +

Wouldn't it be better to use straight

 +	my $no_merges = grep(/^--no-merges$/, @extra_options);

Because $has_merges is true also for example for 'tree' view... which
absolutely doesn't make any sense whatsoever.


In more generic solution (which could be perhaps put in a separate commit)
it could be:

  +	my %extra_options = map { $_ => 0 } keys %$allowed_options;
  +	$extra_options{$_} = 1 foreach @extra_options;

which means that %extra_options is hash which keys are allowed options,
and which has true value for allowed option which is actually used.

Or something like that, if above is to cryptic.

>  	my $title = "$site_name";
>  	if (defined $project) {
>  		$title .= " - " . to_utf8($project);
>  		if (defined $action) {
>  			$title .= "/$action";
> +			$title .= " (no merges)" unless $has_merges;

Wouldn't it be better to use

  +			$title .= " (no merges)" if $no_merges;

More straightforward, and without misleading $has_merges.

>  			if (defined $file_name) {
>  				$title .= " - " . esc_path($file_name);
>  				if ($action eq "tree" && $file_name !~ m|/$|) {
> @@ -3235,6 +3239,16 @@ EOF
>  		print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
>  		if (defined $action) {
>  			print " / $action";
> +			if ($can_have_merges) {
> +				print " <span class='merge_toggle'>";
> +				if ($has_merges) {
> +					printf('<a href="%s">hide merges</a>', href(-replay=>1, 'extra_options'=>('--no-merges', @extra_options)));

It would be more symmetric to use:

  +					my @href_extra = ('--no-merges', @extra_options);
  +					printf('<a href="%s">hide merges</a>', href(-replay=>1, 'extra_options'=>@href_extra));


> +				} else {
> +					my @href_extra = grep(!/^--no-merges$/, @extra_options);
> +					printf('<a href="%s">show merges</a>', href(-replay=>1, 'extra_options'=>@href_extra));
> +				}

and then this conditional could be simplified a bit.

> +				print "</span>";
> +			}
>  		}
>  		print "\n";
>  	}
> -- 
> 1.6.3.rc1.192.gdbfcb
> 
> 

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: git-reflog 70 minutes at 100% cpu and counting
From: Eric Paris @ 2009-12-17 16:29 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Jeff King, git
In-Reply-To: <alpine.LFD.2.00.0912161841210.23173@xanadu.home>

On Thu, 2009-12-17 at 00:38 -0500, Nicolas Pitre wrote:

> Moving the reflog data aside (i.e. mv .git/logs .git/logs.bak) it seems 
> that d936ff8 is not referenced anymore.
> 
> I found the other one as follows:
> 
> First I tried
> 
> $ git rev-list --all --objects
> 
> This resulted in:
> 
> [...]
> 4f7911b0b0dbd187131a109cf00161a0c6a9d727 arch/x86
> ea868257c1eabc31e0ea7941efa42b543978b3fa arch/x86/kvm
> a0c11ead723956c667172a9f3fb6787684fe7ff5 arch/x86/kvm/paging_tmpl.h
> b556b6aad8b1aacfecb1dd4a56dbd389674687b5 arch/x86/kvm/x86.c
> 68a9733ae3315d7e2bfec2037dfeee4db8a6f6a1 drivers
> error: Could not read 29b6c2fb1390b4fd350a5ecc78f1156fc5d91e9f
> fatal: bad tree object 29b6c2fb1390b4fd350a5ecc78f1156fc5d91e9f
> 
> Because of the way objects are enumerated, we can be pretty sure that 
> the bad tree object is referenced by the tree object 68a9733a 
> corresponding to drivers/.  Let's verify that:
> 
> $ git ls-tree 68a9733a
> 100644 blob 00cf9553f74065291612b0971337f79995933a06    Kconfig
> 100644 blob c1bf41737936ab00be4a87563a0bb0638074785d    Makefile
> 040000 tree d4e847de9bf2450842936582ea7cc6778413825b    accessibility
> 040000 tree 29b6c2fb1390b4fd350a5ecc78f1156fc5d91e9f    acpi

This alone almost certainly tells me how I broke it.

For quite some time (a period of months) linux-next was broken and I had
to carry a patch to ACPI to make it boot.  I dropped that patch at the
head of my stgit trees in all of my repositories.  So I wouldn't be at
all surprised to learn that eventually kernel-2 found that object in
kernel-1.  Sometime when I dropped that patch from kernel-1 (because it
finally got fixed upstream) I can see how it broke.

But now that patch shouldn't be needed by any tree since I have long
since dropped it from the stgit stack.  So if we cleaned up all of the
useless objects in this tree I bet this object wouldn't be needed.  Not
exactly a situation that I'd expect git to be able to dig out of itself
thought.

I'm creating clean repos and going to do no work in my -alt    :)

Thanks everyone!

-Eric

^ permalink raw reply

* Re: diff attribute ignored by show and log -p
From: Jay Soffian @ 2009-12-17 16:44 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: git
In-Reply-To: <20091217221740.6117@nanako3.lavabit.com>

On Thu, Dec 17, 2009 at 8:17 AM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
> Quoting Jay Soffian <jaysoffian@gmail.com>
>
>>> But is apparently ignored by "git log -p" and "git show" which just
>>> use internal diff. Is this behavior intentional?
>>
>> Ah, --ext-diff, and the reasoning behind requiring it for log/show is
>> explained in 72909be.
>
> The need to give --ext-diff is mentioned in 72909be (Add diff-option
> --ext-diff, 2007-06-30) but its log message doesn't 'explain' why external
> diff isn't used by default and you need to pass that extra option.

"To prevent funky games with external diff engines, git-log and
friends prevent external diff engines from being called."

Seemed reason enough to me.

> Probably --ext-diff should be the default?

Or available via a config option anyway.

j.

^ permalink raw reply

* git svn mkdirs ignores compressed unhandled.log files
From: Robert Zeh @ 2009-12-17 17:10 UTC (permalink / raw)
  To: git; +Cc: Eric Wong

It looks like there is a conflict between git svn gc and git svn mkdirs.  The git svn mkdirs command only looks at unhandled.log files.   Shouldn't it also look at any compressed unhandled.log files too?

Robert

^ permalink raw reply

* adding additional remote refs to a remote repo
From: Peter Petrakis @ 2009-12-17 17:43 UTC (permalink / raw)
  To: git

I'm setting up a remote repo to aggregate some development which spans
multiple git trees. So I have a bare repo setup using gitosis that I can happily
commit to. I've added the entries directly to the config on the server...

-sh-3.2$ ls
branches  config  description  git-daemon-export-ok  HEAD  hooks  info
 objects  refs
-sh-3.2$ cat config
[core]
        repositoryformatversion = 0
        filemode = true
        bare = true
[remote "drbd-8.2"]
        url = git://git.drbd.org/drbd-8.2.git
        fetch = +refs/heads/*:refs/remotes/drbd-8.2/*


but when I clone the repo, the remote doesn't show up.  What am I doing wrong?
I can easily add the remotes in my local copy and work with the
product. The problem
is that we're tracking this upstream project and need to manage
multiple versions of
it to serve past releases. So in my case the branches will directly
track the upstream
development + any changes we need and then be merged into master.

For example in my local clone.

[remote "drbd-8.2"]
        url = git://git.drbd.org/drbd-8.2.git
        fetch = +refs/heads/*:refs/remotes/drbd-8.2/*
[branch "drbd-8.2.7-merge-branch"]
        remote = drbd-8.2
        merge = refs/heads/master

Now if I push this  drbd-8.2.7-merge-branch to the central repo, the
next guy won't
know for sure what this was tracking. I also don't want to have to
'add remote ...'
every time I clone a new copy. Thanks.

Peter

^ permalink raw reply

* Re: adding additional remote refs to a remote repo
From: Junio C Hamano @ 2009-12-17 18:19 UTC (permalink / raw)
  To: Peter Petrakis; +Cc: git
In-Reply-To: <a422b4da0912170943q2613faeao208eaa674d1a0afa@mail.gmail.com>

Peter Petrakis <peter.petrakis@gmail.com> writes:

> Now if I push this drbd-8.2.7-merge-branch to the central repo, the next
> guy won't know for sure what this was tracking. I also don't want to
> have to 'add remote ...'  every time I clone a new copy. Thanks.

The kind of information you listed above are kept in $GIT_DIR/config of
each repository.  This file is kept private to each repository, i.e. not
cloned, fetched or pushed across repositories [*1*].

This is a deliberate design decision.

 - The file records mostly personal preferences, e.g. how user works with
   his branches in the repository, if the integration is done via rebase
   instead of merge, how the output is colored, etc.  Your colleagues can
   and will use branches other than the ones that may be publicly shared
   (like drbd-8.2.7-merge-branch), and how they use their private branches
   is not something you want to dictate nor interfere with.

 - The [remote "foo"] sections and friends (e.g. [branch "bar"]) in the
   "$GIT_DIR/config" in that bare repository you created specifies how
   that particular repository interacts with the remote named "foo" and
   how branch "bar" is integrated with other things.  These may not
   necessarily match, and if you are using it as a middleman repository
   between your developers and the project upstream, they will in general
   not match, the setting the developer repositories want to use.  This
   should be obvious if you think about where remote.origin.url should
   point at.  In the middleman repository, you would want "git fetch"
   without "origin" to slurp from the upstream and in the developer clones
   of this repository, you would want to "git fetch" to update from this
   middleman repository.

 - The file can has pointers to external programs to be used as filters;
   blindly copying it upon clone has security implications as well.


[Footnote]

*1* Often people use README-like document in-tree to tell developers about
the specific settings the project wants them to use (i.e. "how to work
with this project"), together with coding styles, submission guidelines,
etc., that are not tied to the use of git.

^ permalink raw reply

* Re: git svn mkdirs ignores compressed unhandled.log files
From: Eric Wong @ 2009-12-17 20:08 UTC (permalink / raw)
  To: Robert Zeh; +Cc: git
In-Reply-To: <8BB233FB-4269-4B14-8703-A4FF1E25FB0D@gmail.com>

Robert Zeh <robert.a.zeh@gmail.com> wrote:
> It looks like there is a conflict between git svn gc and git svn
> mkdirs.  The git svn mkdirs command only looks at unhandled.log files.
> Shouldn't it also look at any compressed unhandled.log files too?

Hi Robert,

Yes, an oversight. Does this patch work for you? (Highly untested)

Would you mind writing a test case, been a bit busy with other stuff.
Thanks.

diff --git a/git-svn.perl b/git-svn.perl
index a4b052c..d362de7 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2740,21 +2740,44 @@ sub do_fetch {
 
 sub mkemptydirs {
 	my ($self, $r) = @_;
+
+	sub scan {
+		my ($r, $empty_dirs, $line) = @_;
+		if (defined $r && $line =~ /^r(\d+)$/) {
+			return 0 if $1 > $r;
+		} elsif ($line =~ /^  \+empty_dir: (.+)$/) {
+			$empty_dirs->{$1} = 1;
+		} elsif ($line =~ /^  \-empty_dir: (.+)$/) {
+			my @d = grep {m[^\Q$1\E(/|$)]} (keys %$empty_dirs);
+			delete @$empty_dirs{@d};
+		}
+		1; # continue
+	};
+
 	my %empty_dirs = ();
+	my $gz_file = "$self->{dir}/unhandled.log.gz";
+	if (-f $gz_file) {
+		if (!$can_compress) {
+			warn "Compress::Zlib could not be found; ",
+			     "empty directories in $gz_file will not be read\n";
+		} else {
+			my $gz = Compress::Zlib::gzopen($gz_file, "rb") or
+				die "Unable to open $gz_file: $!\n";
+			my $line;
+			while ($gz->gzreadline($line) > 0) {
+				scan($r, \%empty_dirs, $line) or last;
+			}
+			$gz->gzclose;
+		}
+	}
 
-	open my $fh, '<', "$self->{dir}/unhandled.log" or return;
-	binmode $fh or croak "binmode: $!";
-	while (<$fh>) {
-		if (defined $r && /^r(\d+)$/) {
-			last if $1 > $r;
-		} elsif (/^  \+empty_dir: (.+)$/) {
-			$empty_dirs{$1} = 1;
-		} elsif (/^  \-empty_dir: (.+)$/) {
-			my @d = grep {m[^\Q$1\E(/|$)]} (keys %empty_dirs);
-			delete @empty_dirs{@d};
+	if (open my $fh, '<', "$self->{dir}/unhandled.log") {
+		binmode $fh or croak "binmode: $!";
+		while (<$fh>) {
+			scan($r, \%empty_dirs, $_) or last;
 		}
+		close $fh;
 	}
-	close $fh;
 
 	my $strip = qr/\A\Q$self->{path}\E(?:\/|$)/;
 	foreach my $d (sort keys %empty_dirs) {
-- 
Eric Wong

^ permalink raw reply related

* Re: diff attribute ignored by show and log -p
From: Jeff King @ 2009-12-17 20:24 UTC (permalink / raw)
  To: Jay Soffian; +Cc: Nanako Shiraishi, git
In-Reply-To: <76718490912170844i7d5f25d1hc32590b877dbf295@mail.gmail.com>

On Thu, Dec 17, 2009 at 11:44:16AM -0500, Jay Soffian wrote:

> > The need to give --ext-diff is mentioned in 72909be (Add diff-option
> > --ext-diff, 2007-06-30) but its log message doesn't 'explain' why external
> > diff isn't used by default and you need to pass that extra option.
> 
> "To prevent funky games with external diff engines, git-log and
> friends prevent external diff engines from being called."
> 
> Seemed reason enough to me.

I don't remember discussing it at that time, but much later when
touching the external diff code and adding textconv, Junio and I came to
the conclusion that textconv was reasonable to do as part of "git log
-p", as the result is just an internal conversion that still results in
a text diff. Whereas an external diff might be terribly confusing, as it
could be spawning graphical viewers or producing output which does not
match well with the log output.

> > Probably --ext-diff should be the default?
> 
> Or available via a config option anyway.

If you were going to do such a config option, it might make sense to
make it an attribute of the diff driver rather than a global "use
external diff". Then each diff driver could say "Yes, I am reasonable to
be run as part of log -p".

-Peff

^ permalink raw reply

* Re: [RFC/PATCH] gitweb: link to toggle 'no merges' option
From: Giuseppe Bilotta @ 2009-12-17 20:41 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200912171637.45810.jnareb@gmail.com>

2009/12/17 Jakub Narebski <jnareb@gmail.com>:
> On Thu, 17 Dec 2009 10:05 +0100, Giuseppe Bilotta wrote:
>> This is something I've been wanting for a while. There are a number of
>> things that don't 'click' with this proof of concept, and I'm coming to
>> the list to hear the opinion of users and developers on how to improve
>> the thing.
>>
>> The patch is live at http://git.oblomov.eu/, an example affected page is
>> http://git.oblomov.eu/git/shortlog
>
> You should tell here that one must put mouse over main header (the one
> with 'projects / project / action' breadcrumbs) for the new link to be
> visible... because I was wondering where is this new link.

I mentioned it right below, but I do realize that people might have
gone looking for it before reading further down ;-)

>> Things that are sure to change:
>>
>>  * the aesthetics and location of the toggle link (it shows on mousehover
>>    in the title).
>
> It is not only the question where to put link, but also where to put
> the *code* (where the code should belong to).
>
> At first I thought: WTF? Why the feature that deals with log-like views
> is put in very generic and common for all actions/views git_header_html
> subroutine?  Especially after change that made all loglike views use
> common infrastructure of git_log_generic.
>
> But then I realized that it is specific example of *generic* feature
> that deals with extra_options... which admittedly is currently limited
> to '--no-merges' only.  So if it is put in git_header_html, then all
> views with HTML output (which does not include 'atom' and 'rss' actions,
> but which actions IIRC have their own handling of '--no-merges')
> which have support for extra_options would have ability to turn them
> on and off.

Exactly. Although the code as it is now is not really flexible enough
to handle other cases. I see further down a couple of your ideas would
make this kind of thing more extensible.

> What you need to add (if this link is to be in git_header_html) is
> to create links only when $status == 200, because otherwise the link
> would be present also for error pages, which I think is wrong.

Good point. That's an easy fix too 8-)

>>    Other options I've considered are:
>>     + next to pagination (first | prev | next), either before or after
>>       the existing entries
>
> This would fit with the fact that sometimes present "patches" link is on
> the line with pagination links, after pagination links.
>
> But this secondary navigation bar is about other views, and extra_options
> is about modifying current view, and functions more like toggles.  OTOH
> we have such toggle for 'blame' <-> 'blame_incremental' switch in
> secondary navigation bar.

Well, at least for this particular option the toggle is closer in
concept to pagination (which also modifies the 'current' view, in a
way).

> Also this would mean that each view type would have to handle extra_options
> itself, as secondary navigation bar is very much action-specific.  Not
> that it matters now, with only single '--no-merges' option supported.

Even with more options, I have half an idea on how to refactor this
handling in a way that should make it easy for views to handle the
extra options.

>>     + on mouseover for the table section that refers to the (short)log;
>>       this would make it possible to put it summary view too, for example
>
> This would mean having link inside link, as those headers in summary view
> functions as link to 'shortlog' view (quite useful I think), and to the
> project summary in the 'shortlog' view itself (I'm not sure how useful
> that is).  We already have problems with ref markers being links inside
> links and having broken layout in some strict XHTML conformance browsers.

(I still think that prohibiting link-in-link is idiotic, but this is
not the place where this should be discussed)

> In short: I am not sure what would be the best solution.  Nevertheless
> I think that link should be more visible, and perhaps more toggle-like.

I'll see if I can cook something up.

>>  * if you toggle merge view when not on the first page, the reference
>>    (first) commit in the view is likely to change drastically, which
>>    causes confusion. I have not found a satisfactory solution for this,
>>    since the obvious way to 'lock' the view (start paginating from the
>>    current top commit) prevents prev/next navigation
>
> Alternate solution would be to clean page (start from 0th page) when
> changing to '--no-merges'.
>
> You would probably need something like 'skip' option, with number of
> commits, not number of pages to skip, and/or 'skip_to' which takes
> commit-id.  But i have not thought about this much...

IIRC the log code uses pagination the way it is now because that
reflects the git log works. This would make a different approach more
time consuming on the script part.

>> +
>> +*:hover > span.merge_toggle a { visibility:visible }
>
> I'd rather not have this rule to have different style, i.e. not all
> in single line.  Unless it is for RFC only...

I can make it more like the others.

>> +     my $can_have_merges = grep(/^$action$/, @{$allowed_options{'--no-merges'}});
>> +     my $has_merges = !grep(/^--no-merges$/, @extra_options);
>> +
>
> Wouldn't it be better to use straight
>
>  +      my $no_merges = grep(/^--no-merges$/, @extra_options);
>
> Because $has_merges is true also for example for 'tree' view... which
> absolutely doesn't make any sense whatsoever.

The reason why I have two vars is that one checks if we care about the
option, and the other is to see if it's enabled or not. We don't want
the 'show merges' toggle to appear in view which don't handle the
--no-merge option.

> In more generic solution (which could be perhaps put in a separate commit)
> it could be:
>
>  +     my %extra_options = map { $_ => 0 } keys %$allowed_options;
>  +     $extra_options{$_} = 1 foreach @extra_options;
>
> which means that %extra_options is hash which keys are allowed options,
> and which has true value for allowed option which is actually used.

I like this approach. It's also easier to extend to other options, if
and when we'll have them.

I'll try to roll out a new patch with a cleaner design and taking your
suggestions into considerations

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: [RFC/PATCH] gitweb: link to toggle 'no merges' option
From: Jakub Narebski @ 2009-12-17 21:14 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git
In-Reply-To: <cb7bb73a0912171241j56ecd2f1y3dc66cf3b86bd784@mail.gmail.com>

Giuseppe Bilotta wrote:
> 2009/12/17 Jakub Narebski <jnareb@gmail.com>:
>> On Thu, 17 Dec 2009 10:05 +0100, Giuseppe Bilotta wrote:

[...]
>>> +     my $can_have_merges = grep(/^$action$/, @{$allowed_options{'--no-merges'}});
>>> +     my $has_merges = !grep(/^--no-merges$/, @extra_options);
>>> +
>>
>> Wouldn't it be better to use straight
>>
>>  +      my $no_merges = grep(/^--no-merges$/, @extra_options);
>>
>> Because $has_merges is true also for example for 'tree' view... which
>> absolutely doesn't make any sense whatsoever.
> 
> The reason why I have two vars is that one checks if we care about the
> option, and the other is to see if it's enabled or not. We don't want
> the 'show merges' toggle to appear in view which don't handle the
> --no-merge option.

Perhaps I didn't made myself clear.

What I wanted to ask is to switch $has_merges to $no_merges, not to remove
$can_have_merges.  Its a question of semantics of $has_merges, which do not
mean that action has (handles) merges.

This is of course the question of style, but I think this would make code
more maintainable.


Of course if you go %extra_options hash route this issue wouldn't matter.

-- 
Jakub Narebski
Poland

^ 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