* [RFC/PATCHv2 3/3] gitweb: Silence stderr in parse_commit*() subroutines
From: Jakub Narebski @ 2012-02-15 15:36 UTC (permalink / raw)
To: git; +Cc: rajesh boyapati, Junio C Hamano, Jakub Narebski
In-Reply-To: <1329320203-20272-1-git-send-email-jnareb@gmail.com>
git-rev-list command in parse_commit() and parse_commits() can fail
because $commit_id doesn't point to a valid commit; for example if we
are on unborn branch HEAD doesn't point to a valid commit.
In this case git-rev-list prints
fatal: bad revision 'HEAD'
on its standard error. This commit silences this warning, at the cost
of using shell to redirect it to /dev/null, and relying on
quote_command() to protect against code injection.
Note however that such error message from git (from external command)
usually (but not always) does not appear in web server logs.
Reported-by: rajesh boyapati <boyapatisrajesh@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch first appeared on git mailing list in "Fwd: Git-web error"
thread as
[PATCH] gitweb: Silence stderr in parse_commit*() subroutines
Message-Id: <201202111402.31684.jnareb@gmail.com>
http://article.gmane.org/gmane.comp.version-control.git/190511
More proof of concept rather than real fix for real issue;
it doesn't even fix the issue (for some reason) for original
poster.
gitweb/gitweb.perl | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2eaf585..224ace4 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3338,12 +3338,13 @@ sub parse_commit {
local $/ = "\0";
- open my $fd, "-|", git_cmd(), "rev-list",
+ open my $fd, "-|", quote_command(
+ git_cmd(), "rev-list",
"--parents",
"--header",
"--max-count=1",
$commit_id,
- "--",
+ "--") . ' 2>/dev/null',
or die_error(500, "Open git-rev-list failed");
my $commit_text = <$fd>;
%co = parse_commit_text($commit_text, 1)
@@ -3363,7 +3364,8 @@ sub parse_commits {
local $/ = "\0";
- open my $fd, "-|", git_cmd(), "rev-list",
+ open my $fd, "-|", quote_command(
+ git_cmd(), "rev-list",
"--header",
@args,
("--max-count=" . $maxcount),
@@ -3371,7 +3373,7 @@ sub parse_commits {
@extra_options,
$commit_id,
"--",
- ($filename ? ($filename) : ())
+ ($filename ? ($filename) : ())) . ' 2>/dev/null'
or die_error(500, "Open git-rev-list failed");
while (my $line = <$fd>) {
my %co = parse_commit_text($line);
--
1.7.9
^ permalink raw reply related
* [PATCHv2/RFC 2/3] gitweb: Harden parse_commit and parse_commits
From: Jakub Narebski @ 2012-02-15 15:36 UTC (permalink / raw)
To: git; +Cc: rajesh boyapati, Junio C Hamano, Jakub Narebski
In-Reply-To: <1329320203-20272-1-git-send-email-jnareb@gmail.com>
Gitweb has problems and gives errors when repository it shows is on
unborn branch (HEAD doesn't point to a valid commit), but there exist
other branches.
One of errors that shows in gitweb logs is undefined $commit_id in
parse_commits() subroutine. Therefore we harden both parse_commit()
and parse_commits() against undefined $commit_id, and against no
output from git-rev-list because HEAD doesn't point to a commit.
Reported-by: rajesh boyapati <boyapatisrajesh@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch first appeared on git mailing list in "Fwd: Git-web error"
thread as
[PATCH] gitweb: Harden parse_commit and parse_commits
Message-Id: <201202081604.17187.jnareb@gmail.com>
http://article.gmane.org/gmane.comp.version-control.git/190237
More prevention of generating warnings, rather than real fix.
gitweb/gitweb.perl | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0fdca5b..2eaf585 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3334,6 +3334,8 @@ sub parse_commit {
my ($commit_id) = @_;
my %co;
+ return unless defined $commit_id;
+
local $/ = "\0";
open my $fd, "-|", git_cmd(), "rev-list",
@@ -3343,7 +3345,9 @@ sub parse_commit {
$commit_id,
"--",
or die_error(500, "Open git-rev-list failed");
- %co = parse_commit_text(<$fd>, 1);
+ my $commit_text = <$fd>;
+ %co = parse_commit_text($commit_text, 1)
+ if defined $commit_text;
close $fd;
return %co;
@@ -3353,6 +3357,7 @@ sub parse_commits {
my ($commit_id, $maxcount, $skip, $filename, @args) = @_;
my @cos;
+ return unless defined $commit_id;
$maxcount ||= 1;
$skip ||= 0;
--
1.7.9
^ permalink raw reply related
* [PATCH] git-latexdiff: new command in contrib, to use latexdiff and Git
From: Matthieu Moy @ 2012-02-15 15:49 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
git-latexdiff is a wrapper around latexdiff
(http://www.ctan.org/pkg/latexdiff) that allows using it to diff two
revisions of a LaTeX file.
git-latexdiff is made to work on documents split accross multiple .tex
files (plus possibly figures and other non-diffable files), hence could
not be implemented as a per-file diff driver.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
It seems at least one person is interested in my script, so it
probably deserves to be in contrib/ ;-).
contrib/latex/Makefile | 22 ++++
contrib/latex/README | 12 +++
contrib/latex/git-latexdiff | 222 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 256 insertions(+), 0 deletions(-)
create mode 100644 contrib/latex/Makefile
create mode 100644 contrib/latex/README
create mode 100755 contrib/latex/git-latexdiff
diff --git a/contrib/latex/Makefile b/contrib/latex/Makefile
new file mode 100644
index 0000000..4617906
--- /dev/null
+++ b/contrib/latex/Makefile
@@ -0,0 +1,22 @@
+-include ../../config.mak
+-include ../../config.mak.autogen
+
+ifndef SHELL_PATH
+ SHELL_PATH = /bin/sh
+endif
+
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
+
+SCRIPT=git-latexdiff
+
+.PHONY: install help
+help:
+ @echo 'This is the help target of the Makefile. Current configuration:'
+ @echo ' gitexecdir = $(gitexecdir_SQ)'
+ @echo ' SHELL_PATH = $(SHELL_PATH_SQ)'
+ @echo 'Run "$(MAKE) install" to install $(SCRIPT) in gitexecdir.'
+
+install:
+ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' $(SCRIPT) > '$(gitexecdir_SQ)/$(SCRIPT)'
+ chmod 755 '$(gitexecdir)/$(SCRIPT)'
diff --git a/contrib/latex/README b/contrib/latex/README
new file mode 100644
index 0000000..2d7fdd6
--- /dev/null
+++ b/contrib/latex/README
@@ -0,0 +1,12 @@
+git-latexdiff is a wrapper around latexdiff
+(http://www.ctan.org/pkg/latexdiff) that allows using it to diff two
+revisions of a LaTeX file.
+
+The script internally checks out the full tree for the specified
+revisions, and calls latexdiff with the --flatten option, hence this
+works if the document is split into multiple .tex files.
+
+Try "git latexdiff -h" for more information.
+
+To install, either drop git-latexdiff in your $PATH, or run "make
+install".
diff --git a/contrib/latex/git-latexdiff b/contrib/latex/git-latexdiff
new file mode 100755
index 0000000..13aeb9a
--- /dev/null
+++ b/contrib/latex/git-latexdiff
@@ -0,0 +1,222 @@
+#! /bin/sh
+
+# Author: Matthieu Moy <Matthieu.Moy@imag.fr> (2012)
+
+# Missing features (patches welcome ;-) :
+# - diff the index or the current worktree
+# - checkout only a subdirectory of the repo
+# - hardlink temporary checkouts as much as possible
+
+usage () {
+ cat << EOF
+Usage: $(basename $0) [options] OLD [NEW]
+Call latexdiff on two Git revisions of a file.
+
+OLD and NEW are Git revision identifiers. NEW defaults to HEAD.
+
+Options:
+ --help This help message
+ --main FILE.tex Name of the main LaTeX file
+ --no-view Don't display the resulting PDF file
+ --view View the resulting PDF file
+ (default if -o is not used)
+ --no-cleanup Don't cleanup temp dir after running
+ -o FILE, --output FILE
+ Copy resulting PDF into FILE
+ (usually ending with .pdf)
+EOF
+}
+
+die () {
+ echo "fatal: $@"
+ exit 1
+}
+
+verbose () {
+ if [ "$verbose" = 1 ]; then
+ printf "%s ..." "$@"
+ fi
+}
+
+verbose_progress () {
+ if [ "$verbose" = 1 ]; then
+ printf "." "$@"
+ fi
+}
+
+verbose_done () {
+ if [ "$verbose" = 1 ]; then
+ echo " done."
+ fi
+}
+
+old=
+new=
+main=
+view=maybe
+cleanup=1
+verbose=0
+output=
+initial_dir=$PWD
+
+while test $# -ne 0; do
+ case "$1" in
+ "--help"|"-h")
+ usage
+ exit 0
+ ;;
+ "--main")
+ shift
+ main=$1
+ ;;
+ "--no-view")
+ view=0
+ ;;
+ "--view")
+ view=1
+ ;;
+ "--no-cleanup")
+ cleanup=0
+ ;;
+ "-o"|"--output")
+ shift
+ output=$1
+ ;;
+ "--verbose"|"-v")
+ verbose=1
+ ;;
+ *)
+ if [ "$1" = "" ]; then
+ echo "Empty string not allowed as argument"
+ usage
+ exit 1
+ elif [ "$old" = "" ]; then
+ old=$1
+ elif [ "$new" = "" ]; then
+ new=$1
+ else
+ echo "Bad argument $1"
+ usage
+ exit 1
+ fi
+ ;;
+ esac
+ shift
+done
+
+if [ "$new" = "" ]; then
+ new=HEAD
+fi
+
+if [ "$old" = "" ]; then
+ echo "fatal: Please, provide at least one revision to diff with."
+ usage
+ exit 1
+fi
+
+if [ "$main" = "" ]; then
+ printf "%s" "No --main provided, trying to guess ... "
+ main=$(git grep -l '^[ \t]*\\documentclass')
+ # May return multiple results, but if so the result won't be a file.
+ if [ -r "$main" ]; then
+ echo "Using $main as the main file."
+ else
+ if [ "$main" = "" ]; then
+ echo "No candidate for main file."
+ else
+ echo "Multiple candidates for main file:"
+ printf "%s\n" "$main" | sed 's/^/\t/'
+ fi
+ die "Please, provide a main file with --main FILE.tex."
+ fi
+fi
+
+if [ ! -r "$main" ]; then
+ die "Cannot read $main."
+fi
+
+verbose "Creating temporary directories"
+
+git_prefix=$(git rev-parse --show-prefix)
+cd "$(git rev-parse --show-cdup)" || die "Can't cd back to repository root"
+git_dir="$(git rev-parse --git-dir)" || die "Not a git repository?"
+git_dir=$(cd "$git_dir"; pwd)
+
+main=$git_prefix/$main
+
+tmpdir=$initial_dir/git-latexdiff.$$
+mkdir "$tmpdir" || die "Cannot create temporary directory."
+
+cd "$tmpdir" || die "Cannot cd to $tmpdir"
+
+mkdir old new diff || die "Cannot create old, new and diff directories."
+
+verbose_done
+verbose "Checking out old and new version"
+
+cd old || die "Cannot cd to old/"
+git --git-dir="$git_dir" --work-tree=. checkout "$old" -- . || die "checkout failed for old/"
+verbose_progress
+cd ../new || die "Cannot cd to new/"
+git --git-dir="$git_dir" --work-tree=. checkout "$new" -- . || die "checkout failed for new/"
+verbose_progress
+cd ..
+
+verbose_done
+verbose "Running latexdiff --flatten old/$main new/$main > $main"
+
+latexdiff --flatten old/"$main" new/"$main" > diff.tex || die "latexdiff failed"
+
+mv -f diff.tex new/"$main"
+
+verbose_done
+
+mainbase=$(basename "$main" .tex)
+maindir=$(dirname "$main")
+
+verbose "Compiling result"
+
+compile_error=0
+cd new/"$maindir" || die "Can't cd to new/$maindir"
+if [ -f Makefile ]; then
+ make || compile_error=1
+else
+ pdflatex --interaction errorstopmode "$mainbase" || compile_error=1
+fi
+
+verbose_done
+
+pdffile="$mainbase".pdf
+if [ ! -r "$pdffile" ]; then
+ echo "No PDF file generated."
+ compile_error=1
+fi
+
+if [ ! -s "$pdffile" ]; then
+ echo "PDF file generated is empty."
+ compile_error=1
+fi
+
+if [ "$compile_error" = "1" ]; then
+ echo "Error during compilation. Please examine and cleanup if needed:"
+ echo "Directory: $tmpdir/new/$maindir/"
+ echo " File: $mainbase.tex"
+ # Don't clean up to let the user diagnose.
+ exit 1
+fi
+
+if [ "$output" != "" ]; then
+ abs_pdffile="$PWD/$pdffile"
+ (cd "$initial_dir" && cp "$abs_pdffile" "$output")
+ echo "Output written on $output"
+fi
+
+if [ "$view" = 1 ] || [ "$view" = maybe ] && [ "$output" = "" ]; then
+ xpdf "$pdffile"
+fi
+
+if [ "$cleanup" = 1 ]; then
+ verbose "Cleaning-up result"
+ rm -fr "$tmpdir"
+ verbose_done
+fi
--
1.7.9.111.gf3fb0.dirty
^ permalink raw reply related
* [PATCH (bugfix)] gitweb: Fix 'grep' search for multiple matches in file
From: Jakub Narebski @ 2012-02-15 16:37 UTC (permalink / raw)
To: git
Commit ff7f218 (gitweb: Fix file links in "grep" search, 2012-01-05),
added $file_href variable, to reduce duplication and have the fix
applied in single place.
Unfortunately it made variable defined inside the loop, not taking into
account the fact that $file_href was set only if file changed.
Therefore for files with multiple matches $file_href was undefined for
second and subsequent matches.
Fix this bug by moving $file_href declaration outside loop.
Adds tests for almost all forms of sarch in gitweb, which were missing
from testuite. Note that it only tests if there are no warnings, and
it doesn't check that gitweb finds what it should find.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Commit ff7f218 is in v1.7.9
gitweb/gitweb.perl | 3 ++
t/t9500-gitweb-standalone-no-errors.sh | 39 ++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4932f4b..a2e2023 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6040,9 +6040,10 @@ sub git_search_files {
my $alternate = 1;
my $matches = 0;
my $lastfile = '';
+ my $file_href;
while (my $line = <$fd>) {
chomp $line;
- my ($file, $file_href, $lno, $ltext, $binary);
+ my ($file, $lno, $ltext, $binary);
last if ($matches++ > 1000);
if ($line =~ /^Binary file (.+) matches$/) {
$file = $1;
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 81246a6..90bb605 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -638,6 +638,45 @@ test_expect_success \
'gitweb_run "p=.git;a=tree"'
# ----------------------------------------------------------------------
+# searching
+
+cat >>gitweb_config.perl <<\EOF
+
+# enable search
+$feature{'search'}{'default'} = [1];
+$feature{'grep'}{'default'} = [1];
+$feature{'pickaxe'}{'default'} = [1];
+EOF
+
+test_expect_success \
+ 'search: preparation' \
+ 'echo "1st MATCH" >>file &&
+ echo "2nd MATCH" >>file &&
+ echo "MATCH" >>bar &&
+ git add file bar &&
+ git commit -m "Added MATCH word"'
+
+test_expect_success \
+ 'search: commit author' \
+ 'gitweb_run "p=.git;a=search;h=HEAD;st=author;s=A+U+Thor"'
+
+test_expect_success \
+ 'search: commit message' \
+ 'gitweb_run "p=.git;a=search;h=HEAD;st=commitr;s=MATCH"'
+
+test_expect_success \
+ 'search: grep' \
+ 'gitweb_run "p=.git;a=search;h=HEAD;st=grep;s=MATCH"'
+
+test_expect_success \
+ 'search: pickaxe' \
+ 'gitweb_run "p=.git;a=search;h=HEAD;st=pickaxe;s=MATCH"'
+
+test_expect_success \
+ 'search: projects' \
+ 'gitweb_run "a=project_list;s=.git"'
+
+# ----------------------------------------------------------------------
# non-ASCII in README.html
test_expect_success \
^ permalink raw reply related
* Re: [PATCH 1/3 v5] diff --stat: tests for long filenames and big change counts
From: Junio C Hamano @ 2012-02-15 17:12 UTC (permalink / raw)
To: Zbigniew Jędrzejewski-Szmek; +Cc: git, Michael J Gruber, pclouds
In-Reply-To: <1329303808-16989-1-git-send-email-zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> Eleven tests for various combinations of a long filename and/or big
> change count and ways to specify widths for diff --stat.
> ---
Sign-off?
> Tests added in previous version of 'diff --stat: use full terminal width'
> are extracted into a separate patch. The tests are usefull independently
> of that patch anyway.
Thanks.
> +# 120 character name
> +name=aaaaaaaaaa
> +name=$name$name$name$name$name$name$name$name$name$name$name$name
> +test_expect_success 'preparation' "
> + >\"$name\" &&
> + git add \"$name\" &&
> + git commit -m message &&
> + echo a >\"$name\" &&
> + git commit -m message \"$name\"
> +"
Just for future reference.
The last parameter to test_expect_success shell function is `eval`ed by
the shell and $name is visible inside it; you do not have to use double
quote around it and then use backquote to quote the inner double quote.
> +cat >expect <<'EOF'
> + ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
> +EOF
> +test_expect_success 'format patch graph width defaults to 80 columns' '
> + git format-patch --stat --stdout -1 >output &&
> + grep " | " output >actual &&
> + test_cmp expect actual
> +'
Hrm, this does not seem to pass, making the result of applying [1/3] fail;
I see that the elided name is shown much shorter than the above expects.
Perhaps this test found a bug in a "very long name, small changes" corner
case? If that is the case, we'd mark it as test_expect_failure, and
explain the tests that expect failure demonstrates a buggy behaviour, e.g.
When a pathname is so long that it cannot fit on the column, the
current code truncates it to make sure that the graph part has at
least N columns (enough room to show a meaningful graph). If the
actual change is small (e.g. only one line changed), this results
in the final output that is shorter than the width we aim for. A
couple of new tests marked with test_expect_failure demonstrate
this bug.
in the log message (note that I am not sure if that is the nature of the
bug, or what the actual value of N is).
And then a later patch [2/3] that updates the allocation between name and
graph will turn test_expect_failure to test_expect_success; that will make
it clear that your update fixed the bug.
> +cat >expect <<'EOF'
> + ...aaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
> +EOF
> +test_expect_success 'format patch --stat=width with long name' '
> + git format-patch --stat=40 --stdout -1 >output &&
> + grep " | " output >actual &&
> + test_cmp expect actual
> +'
Likewise.
> +test_expect_success 'format patch --stat-width=width works with long name' '
> + git format-patch --stat-width=40 --stdout -1 >output &&
> + grep " | " output >actual &&
> + test_cmp expect actual
> +'
Likewise.
> +test_expect_success 'format patch --stat=...,name-width with long name' '
> + git format-patch --stat=60,29 --stdout -1 >output &&
> + grep " | " output >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'format patch --stat-name-width with long name' '
> + git format-patch --stat-name-width=29 --stdout -1 >output &&
> + grep " | " output >actual &&
> + test_cmp expect actual
> +'
Likewise.
> +test_expect_success 'preparation' '
There was another "preparation" in this script already, which originally
threw me off while chasing which part of the test is failing. Can you
reword/retitle this one?
> +cat >expect <<'EOF'
> + abcd | 1000 ++++++++++++++++++++++++++++++++++++++++
> +EOF
> +test_expect_success 'format patch graph part width is 40 columns' '
> + git format-patch --stat --stdout -1 >output &&
> + grep " | " output >actual &&
> + test_cmp expect actual
> +'
This test shouldn't be added in [1/3] because "cap the graph to 40-col" is
a new feature that is introduced in the later step.
> +test_expect_success 'format patch ignores COLUMNS' '
> + COLUMNS=200 git format-patch --stat --stdout -1 >output
> + grep " | " output >actual &&
> + test_cmp expect actual
> +'
This is a good test to have in [1/3], but the expectation should not cap the
graph part to 40 columns. The patch that updates diff.c to implement the
cap in [2/3] should have an update to the expectation, whose diff hunk may
look liks this:
@@
cat >expect <<'EOF'
+ abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
- abcd | 1000 ++++++++++++++++++++++++++++++++++++++++
EOF
That would make the effect of the patch clearer.
> +cat >expect <<'EOF'
> + ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++
> +EOF
> +test_expect_success 'format patch --stat=width with big change and long name' '
> + cp abcd aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
> + git add aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
> + git commit -m message &&
> + git format-patch --stat-width=60 --stdout -1 >output &&
> + grep " | " output >actual &&
> + test_cmp expect actual
> +'
> +
> test_done
The same comment as the previous one. Because you change the allocation
to the graph part in your patch to diff.c, which hasn't happened in [1/3]
yet, this should expect the existing behaviour (narrower graph) in this
step, and then be updated to expect the output shown above in the [2/3]
patch that changes the implementation in diff.c.
^ permalink raw reply
* Re: [PATCH/RFC] Document format of basic Git objects
From: Jonathan Nieder @ 2012-02-15 17:31 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Shawn O. Pearce, Scott Chacon
In-Reply-To: <1329312140-24089-1-git-send-email-pclouds@gmail.com>
Hi,
Nguyễn Thái Ngọc Duy wrote:
> Basic objects' format is pretty simple and (I think) well-known.
> However it's good that we document them. At least we can keep track of
> the evolution of an object format. The commit object, for example,
> over the years has learned "encoding" and recently GPG signing.
Yes, I agree.
> This is just a draft text with a bunch of fixmes. But I'd like to hear
> from the community if this is a worthy effort. If so, then whether
> git-cat-file is a proper place for it. Or maybe we put relevant text
> in commit-tree, write-tree and mktag, then refer to them in cat-file
> because cat-file can show raw objects.
About where to place this text, I am of two minds.
1. On one hand, from the user's perspective it would be most intuitive
to place it in a separate git-object(5) manual page. That way,
gitrepository-layout(5), git-fsck(1), git-hash-object(1), the user
manual, and so on would all have one document to link to.
2. On the other hand, from a development perspective I suspect it
would be valuable to put it in the git-fsck(1) page, since that would
have two consequences:
- when changing the documentation, this would provide a reminder to
update fsck.c at the same time
- when changing fsck.c, this would provide a reminder to update the
documentation at the same time
Ok, (2) was tongue in cheek. :) I believe this information belongs in
a dedicated page with a name like gitobject(5), and that you are right
to put it in user-visible documentation instead of hiding it in
Documentation/technical, since it is information needed if one is to
use "git hash-object -w" correctly.
Ok, on to the text itself.
[...]
> --- a/Documentation/git-cat-file.txt
> +++ b/Documentation/git-cat-file.txt
> @@ -100,6 +100,46 @@ for each object specified on stdin that does not exist in the repository:
> <object> SP missing LF
> ------------
>
> +OBJECT FORMAT
> +-------------
> +
> +Tree object consists of a series of tree entries sorted in memcmp()
> +order by entry name.
Missing article ("A tree object", "The tree object", or "Each tree
object"). More importantly, the curious reader might want to know
whether a tree object is supposed to contain entries pointing to other
tree objects for subdirectories or whether the subdirectory's
information is included inline like in the index.
I guess I would expect something like (stealing from the user manual):
TREE OBJECTS
------------
A tree object contains a list of entries, each with a mode,
object type, object name, and filename, sorted by filename. It
represents the contents of a single directory tree.
The object type may be a blob, representing the contents of a
file, another tree, representing the contents of a subdirectory,
or a commit (representing a subproject). Since trees and blobs,
like all other objects, are named by a hash of their contents,
two trees have the same object name if and only if their
contents (including, recursively, the contents of all
subdirectories) are identical. This allows git to quickly
determine the differences between two related tree objects,
since it can ignore any entries with identical object names.
Note that the files all have mode 644 or 755: git actually only
pays attention to the executable bit.
Encoding
~~~~~~~~
Entries are of variable length and self-delimiting. Each entry
consists of
- a POSIX file mode in octal representation
- exactly one space (ASCII SP)
- filename for the entry, as a NUL-terminated string
- 20-byte binary object name
The mode should be 100755 (executable file), 100644 (regular
file), 120000 (symlink), 40000 (subdirectory), or 160000
(subproject), with no leading zeroes. Modes with one leading
zero and the synonym 100664 for 100644 are also accepted for
historical reasons.
The filename may be an arbitrary nonempty string of bytes, as
long as it contains no '/' or NUL character.
The associated object must be a valid blob if the mode indicates
a file or symlink, tree if it indicates a subdirectory, or
commit if it indicates a subproject. The blob associated to a
symlink entry indicates the link target and its content not
have any embedded NULs.
By the way, git fsck seems to tolerate the old "flat tree" format
(i.e., that condition is FSCK_WARN and not FSCK_ERROR), but I don't
see any code supporting it elsewhere in git. Bug?
Sorting
~~~~~~~
... no duplicates, sort order, etc ...
[...]
> +Tag object is ascii plain text in a format similar to email format
> +(RFC 822). It consists of a header and a body, separated by a blank
> +line.
The above description makes me worry that the reader might try some
things that are allowed by RFC 822: rearranging header fields,
continuation lines, and so on.
> The header includes exactly four fields in the following order:
> +
> +1. "object" field, followed by SHA-1 in ascii of the tagged object
> +2. "type" field, followed by the type in ascii of the tagged object
> + (either "commit", "tag", "blob" or "tree" without quotes,
> + case-sensitive)
> +3. "tag" field, followed by the tag name
> +4. "tagger" field, followed by the <XXX, to be named>
> +
> +The tag body contains the tag's message and possibly GPG signature.
This part looks good. Stealing from the user manual again, maybe:
TAG OBJECTS
-----------
A tag object contains an object, object type, tag name, the name
of the person ("tagger") who created the tag, and a message,
which may contain a signature.
------------------------------------------------
$ git cat-file tag v1.5.0
object 437b1b20df4b356c9342dac8d38849f24ef44f27
type commit
tag v1.5.0
tagger Junio C Hamano <junkio@cox.net> 1171411200 +0000
GIT 1.5.0
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
iD8DBQBF0lGqwMbZpPMRm5oRAuRiAJ9ohBLd7s2kqjkKlq1qqC57SbnmzQCdG4ui
nLE/L9aUXdWeTFPron96DLA=
=2E+0
-----END PGP SIGNATURE-----
------------------------------------------------
More precisely, a tag contains at least five lines:
1. "object", followed by a space, followed by the 40-character
textual object name of the tagged object
2. "type" + SP + the type of the tagged object ("commit", "tag",
"blob", or "tree")
3. "tag" + SP + the name of the tag
4. "tagger" + SP + an ident string
5. a blank line
Any remaining text after these lines forms the tag message.
The object field must point to a valid object of type indicated
by the type field. The tag name can be an arbitrary string
without NUL bytes or embedded newlines; in practice it usually
follows the restrictions described in git-check-ref-format(1).
[...]
> +
> +Commit object is in similar format to tag object. The commit body is
> +in plain text of the chosen encoding (by default UTF-8). The commit
> +header has the following fields in listed order
Same considerations apply here --- I'd suggest stealing text from the
commit-object section of the user manual and from commit logs.
Hope that helps,
Jonathan
> +
> +1. One "tree" field, followed by the commit's tree's SHA-1 in ascii
> +2. Zero, one or more "parent" field
> +3. One "author" field, in <XXX to be named> format
> +3. One "committer" field, in <XXX to be named> format
> +4. Optionally one "encoding" field, followed by the encoding used for
> + commit body
> +5. GPG signature (fixme)
> +
> +More headers after these fields are allowed. Unrecognized header
> +fields must be kept untouched if the commit is rewritten. However, a
> +compliant Git implementation produces the above header fields only.
> +
> GIT
> ---
> Part of the linkgit:git[1] suite
^ permalink raw reply
* Re: [PATCH 1/3 v5] diff --stat: tests for long filenames and big change counts
From: Junio C Hamano @ 2012-02-15 17:33 UTC (permalink / raw)
To: Zbigniew Jędrzejewski-Szmek; +Cc: git, Michael J Gruber, pclouds
In-Reply-To: <7vvcn810ml.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
>
>> Eleven tests for various combinations of a long filename and/or big
>> change count and ways to specify widths for diff --stat.
>> ---
>
> Sign-off?
> ...
> The same comment as the previous one. Because you change the allocation
> to the graph part in your patch to diff.c, which hasn't happened in [1/3]
> yet, this should expect the existing behaviour (narrower graph) in this
> step, and then be updated to expect the output shown above in the [2/3]
> patch that changes the implementation in diff.c.
The previous review message in a patch form that can be squashed on top of
this patch.
-- >8 --
Subject: [PATCH] (squash to the previous -- replace the log message with this)
diff --stat: tests for long filenames and big change counts
In preparation for updates to the "diff --stat" that updates the logic
to split the allotted columns into the name part and the graph part to
make the output more readable, add a handful of tests to document the
corner case behaviour in which long filenames and big changes are shown.
When a pathname is so long that it cannot fit on the column, the current
code truncates it to make sure that the graph part has enough room to show
a meaningful graph. If the actual change is small (e.g. only one line
changed), this results in the final output that is shorter than the width
we aim for. A couple of new tests marked with test_expect_failure
demonstrate this bug.
---
t/t4014-format-patch.sh | 35 +++++++++++++++--------------------
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index f6ebb51..0376186 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -897,18 +897,18 @@ test_expect_success 'format patch ignores color.ui' '
# 120 character name
name=aaaaaaaaaa
name=$name$name$name$name$name$name$name$name$name$name$name$name
-test_expect_success 'preparation' "
- >\"$name\" &&
- git add \"$name\" &&
+test_expect_success 'preparation' '
+ >"$name" &&
+ git add "$name" &&
git commit -m message &&
- echo a >\"$name\" &&
- git commit -m message \"$name\"
-"
+ echo a >"$name" &&
+ git commit -m message "$name"
+'
cat >expect <<'EOF'
...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
EOF
-test_expect_success 'format patch graph width defaults to 80 columns' '
+test_expect_failure 'format patch graph width defaults to 80 columns' '
git format-patch --stat --stdout -1 >output &&
grep " | " output >actual &&
test_cmp expect actual
@@ -917,13 +917,13 @@ test_expect_success 'format patch graph width defaults to 80 columns' '
cat >expect <<'EOF'
...aaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
EOF
-test_expect_success 'format patch --stat=width with long name' '
+test_expect_failure 'format patch --stat=width with long name' '
git format-patch --stat=40 --stdout -1 >output &&
grep " | " output >actual &&
test_cmp expect actual
'
-test_expect_success 'format patch --stat-width=width works with long name' '
+test_expect_failure 'format patch --stat-width=width works with long name' '
git format-patch --stat-width=40 --stdout -1 >output &&
grep " | " output >actual &&
test_cmp expect actual
@@ -941,26 +941,21 @@ test_expect_success 'format patch --stat-name-width with long name' '
test_cmp expect actual
'
-test_expect_success 'preparation' '
+test_expect_success 'preparation for big change tests' '
>abcd &&
git add abcd &&
git commit -m message &&
i=0 &&
- while test $i -lt 1000; do
- echo $i &&
- i=$(($i + 1))
+ while test $i -lt 1000
+ do
+ echo $i && i=$(($i + 1))
done >abcd &&
git commit -m message abcd
'
cat >expect <<'EOF'
- abcd | 1000 ++++++++++++++++++++++++++++++++++++++++
+ abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
EOF
-test_expect_success 'format patch graph part width is 40 columns' '
- git format-patch --stat --stdout -1 >output &&
- grep " | " output >actual &&
- test_cmp expect actual
-'
test_expect_success 'format patch ignores COLUMNS' '
COLUMNS=200 git format-patch --stat --stdout -1 >output
@@ -984,7 +979,7 @@ test_expect_success 'format patch --stat-width=width with big change' '
'
cat >expect <<'EOF'
- ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++
+ ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++
EOF
test_expect_success 'format patch --stat=width with big change and long name' '
cp abcd aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
--
1.7.9.1.237.g00b59
^ permalink raw reply related
* Re: [PATCH 1/2] git-svn.perl: perform deletions before anything else
From: Steven Walter @ 2012-02-15 17:47 UTC (permalink / raw)
To: Eric Wong; +Cc: gitster, git
In-Reply-To: <20120212234928.GA4513@dcvr.yhbt.net>
On Sun, Feb 12, 2012 at 6:49 PM, Eric Wong <normalperson@yhbt.net> wrote:
> Steven Walter <stevenrwalter@gmail.com> wrote:
>> On Sun, Feb 12, 2012 at 2:03 AM, Eric Wong <normalperson@yhbt.net> wrote:
>> > Steven Walter <stevenrwalter@gmail.com> wrote:
>> >> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
>> >
>> > Thanks, shall I fixup 2/2 and assume you meant to Sign-off on that, too?
>>
>> Yes, thanks
>
> Ugh, I got a bunch of test failures on t9100-git-svn-basic.sh with your
> updated 1/2 and a trivially merged 2/2:
>
> not ok - 7 detect node change from file to directory #2
I believe that "test_must_fail" is incorrect for this case. "git svn
set-tree" is succeeding, and the git commit is being faithfully
recorded into the svn repository. If svn will allow us to do it, then
I don't think git-svn should artificially fail in the case. This is
using svn 1.6.17
What's the oldest version of svn supported by git-svn? Perhaps if I
retry with that version of svn, I would see a failure. However, if
libsvn-perl reports the failure correctly, isn't that good enough
behavior? No need to fail in git-svn before even trying, IMHO.
> not ok - 12 new symlink is added to a file that was also just made executable
> not ok - 13 modify a symlink to become a file
> not ok - 14 commit with UTF-8 message: locale: en_US.UTF-8
> not ok - 16 check imported tree checksums expected tree checksums
The rest of these problems seem to have been cascading failures
resulting from the unexpected success of "git svn set-tree" in test 7.
This left the git and svn repositories in a different state. To get
these to pass, I changed later references to "bar/zzz" (which is now a
directory) to use "file" instead. I also had to update the expected
checksum values for test 16. Is there a way to validate what the
checksums should be, other than to look at it and say, "yup, the trees
look okay?"
> I would very much appreciate new test cases that can show exactly what's
> fixed by your patches (esp given the only times I run/use git-svn is
> when reviewing patches). Thanks!.
In fact test 7 is exactly what I was trying to make work. The fact
that "git svn set-tree" now succeeds in that case is proof that my
change had the desired effect. I modified test 7 to verify that
set-tree succeeds and that bar/zzz and bar/zzz/yyy get created in
$SVN_TREE.
Assuming you agree with the above analysis, should I squash the test
changes into my 2/2, or would you prefer a separate patch?
--
-Steven Walter <stevenrwalter@gmail.com>
^ permalink raw reply
* Re: [PATCH 2/3 v5] diff --stat: use the full terminal width
From: Junio C Hamano @ 2012-02-15 18:07 UTC (permalink / raw)
To: Zbigniew Jędrzejewski-Szmek; +Cc: git, Michael J Gruber, pclouds
In-Reply-To: <1329303808-16989-2-git-send-email-zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> Use as many columns as necessary for the filenames and up to 40
> columns for the graph.
I started to wonder if it is a good idea to split this step further into
at least two patches:
- using term_columns() to set the default 'stat-width' instead of
hardcoded 80, and do nothing else. As you discovered, this step will
not have to touch any test expectations.
- update the logic to split the stat-width into name part and graph
part. I think as a side-effect this will fix the corner case bugs the
new tests in your [1/3] seem to have discovered, and the fix will be
visible to the part that will update t/t4014 in this step.
> ... I've taken the middle way:
> number_width=4 is hardcoded, but the fprintf is reverted to the previous
> version. I think that this way the code is most readable, independently
> if later changes.
Sensible.
> I haven't done this, because $COLUMNS and the actual terminal width is always
> ignored in tests.
As long as the tests are not affected by ioctl(1) that is OK. I am not
expecting us to be automating the test of that codepath (unless somebody
has clever ideas perhaps using pty, but I suspect that would be a separate
patch anyway).
After writing the attached patch that goes on top of this patch to be
squashed, I am starting to think that "maximum 40 columns for the graph"
may be a mild regression for a project with a shallow hierarchy, namely,
this part.
cat >expect <<'EOF'
- abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ abcd | 1000 ++++++++++++++++++++++++++++++++++++++++
EOF
A bug used to waste space by allocating more than necessary as the minimum
number of columns given for the graph part, even when the change turns out
to be just one line, requiring only one '+' in the graph. The bug was fixed
by this patch not to waste space that way. But now with this "maximum 40"
limit, we can see that it wastes the space even when the stat-width is 80.
Perhaps the maximum for garph_width should be raised to something like
"min(80, stat_width) - name_width"?
-- >8 --
Subject: [PATCH] (squash to the previous -- replace the last line of the
log with the following)
The effect of this change is visible in the patch to t4014 that fixes a
few tests marked with test_expect_failure, and the change to shorten the
maximum graph width to 40 columns.
---
t/t4014-format-patch.sh | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 0376186..88ccc5a 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -908,7 +908,7 @@ test_expect_success 'preparation' '
cat >expect <<'EOF'
...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
EOF
-test_expect_failure 'format patch graph width defaults to 80 columns' '
+test_expect_success 'format patch graph width defaults to 80 columns' '
git format-patch --stat --stdout -1 >output &&
grep " | " output >actual &&
test_cmp expect actual
@@ -917,13 +917,13 @@ test_expect_failure 'format patch graph width defaults to 80 columns' '
cat >expect <<'EOF'
...aaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
EOF
-test_expect_failure 'format patch --stat=width with long name' '
+test_expect_success 'format patch --stat=width with long name' '
git format-patch --stat=40 --stdout -1 >output &&
grep " | " output >actual &&
test_cmp expect actual
'
-test_expect_failure 'format patch --stat-width=width works with long name' '
+test_expect_success 'format patch --stat-width=width works with long name' '
git format-patch --stat-width=40 --stdout -1 >output &&
grep " | " output >actual &&
test_cmp expect actual
@@ -954,8 +954,13 @@ test_expect_success 'preparation for big change tests' '
'
cat >expect <<'EOF'
- abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ abcd | 1000 ++++++++++++++++++++++++++++++++++++++++
EOF
+test_expect_success 'format patch graph part width is 40 columns' '
+ git format-patch --stat --stdout -1 >output &&
+ grep " | " output >actual &&
+ test_cmp expect actual
+'
test_expect_success 'format patch ignores COLUMNS' '
COLUMNS=200 git format-patch --stat --stdout -1 >output
@@ -979,7 +984,7 @@ test_expect_success 'format patch --stat-width=width with big change' '
'
cat >expect <<'EOF'
- ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++
+ ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++
EOF
test_expect_success 'format patch --stat=width with big change and long name' '
cp abcd aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
--
1.7.9.1.237.g00b59
^ permalink raw reply related
* Re: Bulgarian translation of git
From: Junio C Hamano @ 2012-02-15 18:09 UTC (permalink / raw)
To: Jiang Xin; +Cc: Git List
In-Reply-To: <CANYiYbF-M0SLP=XFkD+nEVRth05pf3hohPLyqQ75qjtNGqANMA@mail.gmail.com>
Jiang Xin <worldhello.net@gmail.com> writes:
> I squash the following in the pot initial commit, and update the commit log.
> commit: https://github.com/gotgit/git-po/commit/master%5E
>
> diff --git a/Makefile b/Makefile
> index 87fb3..88268 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2576,7 +2576,6 @@ dist-doc:
>
> distclean: clean
> $(RM) configure
> - $(RM) po/git.pot
Yeah, that change is very sensible. I am surprised that nobody noticed it
so far while this topic was discussed.
^ permalink raw reply
* Re: [StGit PATCH] Parse commit object header correctly
From: Junio C Hamano @ 2012-02-15 18:13 UTC (permalink / raw)
To: Catalin Marinas
Cc: Michael Haggerty, Karl Hasselström,
Andy Green (林安廸), git
In-Reply-To: <CAHkRjk451=_XaQuUXmxAvB3sRRz6-J+c7A2ZrfLwfGz=z05Lag@mail.gmail.com>
Catalin Marinas <catalin.marinas@gmail.com> writes:
> ... I'll
> publish it to the 'master' branch shortly and release a 0.16.1
> hopefully this week.
Thanks.
^ permalink raw reply
* Re: how to determine oldest supported version of git
From: Jonathan Nieder @ 2012-02-15 18:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vaa4k38nj.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> But think again, with the intimate knowledge of how these bugfix topics
> are merged down to older maintenance tracks.
[...]
> But nobody in the development community rebuilds 'maint' every time it is
> updated and runs the result as his or her primary production version. Even
> I do not do that (remember, I run 'next'). I only build and run full test
> suite. Older maintenance tracks are worse. I do not think anybody runs
> them before they are tagged and released.
I can offer one data point. In the context of Debian sid, Gerrit and
I do test each version in daily work before uploading it. I generally
build from and test whatever track is going to be used for the next
upload (usually plus a few extra features I am interested in for
private use) a little while before the release, to be prepared.
Anders sometimes uploads to the Ubuntu PPA which brings more testers.
After the upload, users running "sid" test for about a week before
even more users running "testing" get to take a look at it and test
for the sake of later users who will run "stable".
So little bugs get discovered, with time to fix them.
Even with this, the extra time to migrate from 1.7.6 to 1.7.7, for
example, was very helpful in the context of Debian sid. Like it or
not, new features *do* come with minor regressions, and it helps the
sanity of a package maintainer to not have to suffer people who did
not request a bleeding-edge release complaining about regressions
until there has been time to fix them.
Of course, this has nothing to do with Debian stable, which is an
orthogonal story. I'll discuss that below.
[...]
> * At that point, old 'maint' and 1.7.9.X track cease to receive updates,
> as there is no point maintaining them. It only encourages distros to
> stay behind, adding unnecesary maintenance burden to us.
If you are thinking of distros like Debian stable, then that is just
wishful thinking. Dropping support for old releases does not have any
effect except to cause patches to be missed there. (See iceweasel and
chromium-browser for examples where using the version in Debian stable
is something I would usually not recommend.)
This may seem weird, but keep in mind that people like you and me are
not the target audience for the git package in Debian stable. We use
git heavily. If I am on a machine running stable or RHEL, I will
build a private copy of git in $HOME or ask the sysadmin to install a
more recent git as the first thing I do.
The reason that packages go into Debian stable and then just _don't
change_ is that the target users are not using those packages heavily.
If a new feature (e.g., "signatures from tags get incorporated into
the merge commit from pulling them") causes a regression (e.g., "the
script I used to run every week that pulls my favorite software
package and builds it just stopped working"), then these people get
zero benefit, for a sizable cost.
Though that's a digression. The relevant detail to mention here is
that there is real demand on downstreams to continue to maintain
packages without adding new features. They will help to maintain
old releases if you want. If we want to influence that maintainance,
for example to ensure security bugs are fixed correctly and in the
same way everywhere, a good way is to keep a maintenance branch.
Hoping that clarifies a little.
Jonathan
^ permalink raw reply
* Re: [StGit PATCH] Parse commit object header correctly
From: "Andy Green (林安廸)" @ 2012-02-15 18:40 UTC (permalink / raw)
To: Catalin Marinas
Cc: Michael Haggerty, Junio C Hamano, Karl Hasselström, git
In-Reply-To: <CAHkRjk451=_XaQuUXmxAvB3sRRz6-J+c7A2ZrfLwfGz=z05Lag@mail.gmail.com>
On 02/15/2012 04:24 AM, Somebody in the thread at some point said:
Hi -
> Thank you all for comments and patches. I used a combination of
> Junio's patch with the comments from Michael and a fix from me. I'll
> publish it to the 'master' branch shortly and release a 0.16.1
> hopefully this week.
I cloned the master branch and installed it locally, it's working well.
Thanks a lot to the guys who spent time on this bug and stgit overall,
which I rely heavily on!
-Andy
^ permalink raw reply
* Re: how to determine oldest supported version of git
From: Jonathan Nieder @ 2012-02-15 18:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <20120215183454.GA23016@burratino>
Jonathan Nieder wrote:
> Even with this, the extra time to migrate from 1.7.6 to 1.7.7, for
> example, was very helpful in the context of Debian sid.
Whoops, off by one error. The extra time to move from 1.7.4 to 1.7.5
and 1.7.5 to 1.7.6 was helpful. 1.7.7 was actually pretty painless,
so sid moved to it right away. ;-)
^ permalink raw reply
* Re: git status: small difference between stating whole repository and small subdirectory
From: Jeff King @ 2012-02-15 19:03 UTC (permalink / raw)
To: Piotr Krukowiecki; +Cc: Thomas Rast, Git Mailing List, Nguyen Thai Ngoc Duy
In-Reply-To: <CAA01Cso_8=159UDMFUHiYz1X=gYtpbqRO4h3TMw7N=4YMV8YNg@mail.gmail.com>
On Wed, Feb 15, 2012 at 09:57:29AM +0100, Piotr Krukowiecki wrote:
> All is on local disk and system is idle.
>
> Indeed, after gc the times went down:
> 10s -> 2.3s (subdirectory)
> 17s -> 9.5s (whole repo)
>
> 2 seconds is much better and I'd say acceptable for me. But my questions are:
Obviously these answers didn't come from any deep analysis, but are
educated guesses from me based on previous performance patterns we've
seen on the list:
> - why is it so slow with not packed repo?
Your numbers show that you're I/O-bound:
>>> $ time git status > /dev/null
>>> real 0m41.670s
>>> user 0m0.980s
>>> sys 0m2.908s
>>>
>>> $ time git status -- src/.../somedir > /dev/null
>>> real 0m17.380s
>>> user 0m0.748s
>>> sys 0m0.328s
which is not surprising, since you said you dropped caches before-hand.
Repacking probably reduced your disk footprint by a lot, which meant
less I/O.
I notice that you're still I/O bound even after the repack:
> $ time git status -- .
> real 0m2.503s
> user 0m0.160s
> sys 0m0.096s
>
> $ time git status
> real 0m9.663s
> user 0m0.232s
> sys 0m0.556s
Did you drop caches here, too? Usually that would not be the case on a
warm cache. If it is, then it sounds like you are short on memory to
actually hold the directory tree and object db in cache. If not, what do
the warm cache numbers look like?
> - can it be faster without repacking?
Not really. You're showing an I/O problem, and repacking is git's way of
reducing I/O.
> - even with packed repo, the time on small subdirectory is much higher
> than I'd expect given time on whole repo and subdirectory size - why?
Hard to say without profiling. It may be that we reduced the object db
lookups, saving some time, but still end up stat()ing the whole tree.
The optimization to stat only the directories of interest was in 688cd6d
(status: only touch path we may need to check, 2010-01-14), which went
into v1.7.0. What version of git are you using?
-Peff
^ permalink raw reply
* Re: [PATCH/RFC] Document format of basic Git objects
From: Junio C Hamano @ 2012-02-15 19:48 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1329312140-24089-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> This is just a draft text with a bunch of fixmes. But I'd like to hear
> from the community if this is a worthy effort. If so, then whether
> git-cat-file is a proper place for it. Or maybe we put relevant text
> in commit-tree, write-tree and mktag, then refer to them in cat-file
> because cat-file can show raw objects.
>
> So comments?
This _only_ describes the payload (i.e. without the 'blob <size>\n' header
used in loose object, in other words, what read_object() may return).
There should be a sentence to stress this. As many Git intros (including
my book) begin with the "a short header 'blob <size>\n' concatenated with
the contents is hashed to compute the object name" picture, it would be
confusing unless you explicitly say that you are only describing the
"contents" part.
It makes sense to mention that the cat-file subcommand is used to obtain
this raw data somewhere in the documentation, but I would say the content
of this patch belongs to Documentation/technical/ somewhere.
> PS. This also makes me wonder if tag object supports "encoding".
I do not think so.
> +OBJECT FORMAT
> +-------------
> +
> +Tree object consists of a series of tree entries sorted in memcmp()
> +order by entry name. Each entry consists of:
> +
> +- POSIX file mode encoded in octal ascii
Add ", no 0 padding to the right" at the end, as I heard that every
imitation of Git gets this wrong in its first version.
> +- One space character
> +- Entry name terminated by one character NUL
> +- 20 byte SHA-1 of the entry
> +Tag object is ascii plain text in a format similar to email format
> +(RFC 822). ...
Do not mention "email format (RFC 822)" at all. The differences are
significant enough that it only confuses the readers.
We do not have colon at the end of the header, we do not promise to parse
field names case insensitively, and the way continuation lines are parsed
is totally different (a "similar" construct in RFC 2822 is "folded header
lines", but it is signalled by "folding white space", it discards the
end-of-line from the previous line and makes the result a logical single
line. Our continuation lines are introduced by a single SP and the result
of concatenation keeps the end-of-line from the previous lines, making the
result multiple lines).
Also we do not promise that the lines in the header part are always
<field,value> pairs. So rephrase this while carefully distinguishing
between "a line in header" and "field".
A commit or a tag object begins with the "header" that consists of one
or more lines delimited by LF. The end of the header is signalled by
an empty line.
A "continuation line" in the header begins with a SP. The remainder
of the line, after removing that SP, is concatenated to the previous
line, while retaining the LF at the end of the previous line.
When a line in the header begins with a letter other than SP, and has
at least one SP in it, it is called a "field". A field consists of
the "field name", which is the string before the first SP on the line,
and its "value", which is everything after that SP. When the value
consists of multiple lines, continuation lines are used.
More than one field with the same name can appear in the header of an
object, and the order in which they appear is significant.
In a commit object, the header begins with the following fields that
have such and such meaning.
In a tag object, the header begins with the following fields...
After these defined fields, newer versions of git may add more lines
in the header. Some of them may be fields, others might not be. The
implementations to parse commit and tag objects must ignore lines in
the header that it does not understand without triggering an error.
> ... It consists of a header and a body, separated by a blank
> +line. The header includes exactly four fields in the following order:
> +
If you hand-craft a tag-like object that has unknown field after these
four, how badly the current implementations behave?
> +1. "object" field, followed by SHA-1 in ascii of the tagged object
> +2. "type" field, followed by the type in ascii of the tagged object
> + (either "commit", "tag", "blob" or "tree" without quotes,
> + case-sensitive)
> +3. "tag" field, followed by the tag name
> +4. "tagger" field, followed by the <XXX, to be named>
> +The tag body contains the tag's message and possibly GPG signature.
> +
> +Commit object is in similar format to tag object. The commit body is
It is strange that you introduce tag and then commit. I would think that
readers expect to see them presented in the usual blob/tree/commit/tag
order.
> +in plain text of the chosen encoding (by default UTF-8). The commit
> +header has the following fields in listed order
> +
> +1. One "tree" field, followed by the commit's tree's SHA-1 in ascii
> +2. Zero, one or more "parent" field
> +3. One "author" field, in <XXX to be named> format
> +3. One "committer" field, in <XXX to be named> format
> +4. Optionally one "encoding" field, followed by the encoding used for
> + commit body
> +5. GPG signature (fixme)
> +
> +More headers after these fields are allowed. Unrecognized header
> +fields must be kept untouched if the commit is rewritten.
Replace the first sentence with "New kinds of fields may be added in later
versions of git." and drop the second one entirely. Depending on the
reason and nature of the "rewrite", we may or may not want to keep these
unknown header lines, so it is best to leave the behaviour unspecified.
For example, it makes sense to retain "mergetag" because it is about the
parent, not the resulting commit. It does not make sense to keep "gpgsig"
because it is about the commit you are rewriting to invalidate that old
signature.
^ permalink raw reply
* [PATCH] completion: --list option for git-branch
From: Ralf Thielow @ 2012-02-15 20:36 UTC (permalink / raw)
To: spearce; +Cc: git, gitster, Ralf Thielow
Signed-off-by: Ralf Thielow <ralf.thielow@googlemail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d7367e9..1505cff 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1137,7 +1137,7 @@ _git_branch ()
__gitcomp "
--color --no-color --verbose --abbrev= --no-abbrev
--track --no-track --contains --merged --no-merged
- --set-upstream --edit-description
+ --set-upstream --edit-description --list
"
;;
*)
--
1.7.9.1
^ permalink raw reply related
* [PATCHv2 0/8] gitweb: Faster and improved project search
From: Jakub Narebski @ 2012-02-15 20:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
[Cc-ing Junio because of his involvement in discussion about first
patch in previous version of this series.]
First three patches in this series are mainly about speeding up
project search (and perhaps in the future also project pagination).
Well, first one is unification, refactoring and future-proofing.
The second and third patch could be squashed together; second adds
@fill_only, but third actually uses it.
Next set of patches is about highlighting matched part, making it
easier to recognize why project was selected, what we were searching
for (though better page title would also help second issue).
Well, fourth patch (first in set mentioned above) is here for the
commit message, otherwise it could have been squashed with next one.
Last patch in this series is beginning of using esc_html_match_hl()
for other searches in gitweb -- the easiest part.
Jakub Narebski (8):
gitweb: Refactor checking if part of project info need filling
gitweb: Option for filling only specified info in
fill_project_list_info
gitweb: Faster project search
gitweb: Introduce esc_html_hl_regions
gitweb: Highlight matched part of project name when searching
projects
gitweb: Highlight matched part of project description when searching
projects
gitweb: Highlight matched part of shortened project description
gitweb: Use esc_html_match_hl() in 'grep' search
gitweb/gitweb.perl | 158 ++++++++++++++++++++++++++++++++++++++++++++--------
1 files changed, 135 insertions(+), 23 deletions(-)
--
1.7.9
^ permalink raw reply
* [PATCHv2 1/8] gitweb: Refactor checking if part of project info need filling
From: Jakub Narebski @ 2012-02-15 20:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <1329338332-30358-1-git-send-email-jnareb@gmail.com>
Extract the check if given keys (given parts) of project info needs to
be filled into project_info_needs_filling() subroutine. It is for now
a thin wrapper around "!exists $project_info->{$key}".
Note that "!defined" was replaced by more correct "!exists".
While at it uniquify treating of all project info, adding checks for
'age' field before running git_get_last_activity(), and also checking
for all keys filled in code protected by conditional, and not only
one.
The code now looks like this
foreach my $project (@$project_list) {
if (given keys need to be filled) {
fill given keys
}
...
}
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch is new in this series, and splits off introduction of
project_info_needs_filling() function from partial filling via
@fill_only.
Hopefully this makes change more clear.
Note that this change stands alone, regardless of speeding up project
search.
gitweb/gitweb.perl | 35 +++++++++++++++++++++++++++--------
1 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 057ba5b..e62c2ef 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5195,6 +5195,21 @@ sub git_project_search_form {
print "</div>\n";
}
+# entry for given @keys needs filling if at least one of keys in list
+# is not present in %$project_info
+sub project_info_needs_filling {
+ my ($project_info, @keys) = @_;
+
+ # return List::MoreUtils::any { !exists $project_info->{$_} } @keys;
+ foreach my $key (@keys) {
+ if (!exists $project_info->{$key}) {
+ return 1;
+ }
+ }
+ return;
+}
+
+
# fills project list info (age, description, owner, category, forks)
# for each project in the list, removing invalid projects from
# returned list
@@ -5206,24 +5221,28 @@ sub fill_project_list_info {
my $show_ctags = gitweb_check_feature('ctags');
PROJECT:
foreach my $pr (@$projlist) {
- my (@activity) = git_get_last_activity($pr->{'path'});
- unless (@activity) {
- next PROJECT;
+ if (project_info_needs_filling($pr, 'age', 'age_string')) {
+ my (@activity) = git_get_last_activity($pr->{'path'});
+ unless (@activity) {
+ next PROJECT;
+ }
+ ($pr->{'age'}, $pr->{'age_string'}) = @activity;
}
- ($pr->{'age'}, $pr->{'age_string'}) = @activity;
- if (!defined $pr->{'descr'}) {
+ if (project_info_needs_filling($pr, 'descr', 'descr_long')) {
my $descr = git_get_project_description($pr->{'path'}) || "";
$descr = to_utf8($descr);
$pr->{'descr_long'} = $descr;
$pr->{'descr'} = chop_str($descr, $projects_list_description_width, 5);
}
- if (!defined $pr->{'owner'}) {
+ if (project_info_needs_filling($pr, 'owner')) {
$pr->{'owner'} = git_get_project_owner("$pr->{'path'}") || "";
}
- if ($show_ctags) {
+ if ($show_ctags &&
+ project_info_needs_filling($pr, 'ctags')) {
$pr->{'ctags'} = git_get_project_ctags($pr->{'path'});
}
- if ($projects_list_group_categories && !defined $pr->{'category'}) {
+ if ($projects_list_group_categories &&
+ project_info_needs_filling($pr, 'category')) {
my $cat = git_get_project_category($pr->{'path'}) ||
$project_list_default_category;
$pr->{'category'} = to_utf8($cat);
--
1.7.9
^ permalink raw reply related
* [PATCHv2 2/8] gitweb: Option for filling only specified info in fill_project_list_info
From: Jakub Narebski @ 2012-02-15 20:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <1329338332-30358-1-git-send-email-jnareb@gmail.com>
Introduce project_info_needs_filling($pr, $key[, \%fill_only]), which
is now used in place of simple 'defined $pr->{$key}' to check if
specific slot in project needs to be filled.
This is in preparation of future lazy filling of project info in
project search and pagination of sorted list of projects. The only
functional change is that fill_project_list_info() now checks if 'age'
is already filled before running git_get_last_activity().
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This could have been squashed with the next commit, but this way it is
pure refactoring that shouldn't change gitweb behavior.
Changes from v1:
* Introduction of project_info_needs_filling() was moved to separate
commit; this one just adds @fill_only / %fill_only, without actually
using it.
* It uses \%fill_only (reference to hash) rather than @fill_only,
because project_info_needs_filling() now supports multiple keys,
and because checking key in hash is faster O(1) than checking every
element in array O(n).
Though this shouldn't matter much, as @fill_only has at most two
or three elements, as we would see in the next commit.
gitweb/gitweb.perl | 34 +++++++++++++++++++++++-----------
1 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e62c2ef..cae71f5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5195,54 +5195,66 @@ sub git_project_search_form {
print "</div>\n";
}
-# entry for given @keys needs filling if at least one of keys in list
-# is not present in %$project_info
+
+# entry for given @keys needs filling if at least one of interesting keys
+# in list is not present in %$project_info; key is interesting if $fill_only
+# is not passed, or is empty (all keys are interesting in both of those cases),
+# or if key is in $fill_only hash
+#
+# USAGE:
+# * project_info_needs_filling($project_info, 'key', ...)
+# * project_info_needs_filling($project_info, 'key', ..., \%fill_only)
+# where %fill_only = map { $_ => 1 } @fill_only;
sub project_info_needs_filling {
+ my $fill_only = ref($_[-1]) ? pop : undef;
my ($project_info, @keys) = @_;
# return List::MoreUtils::any { !exists $project_info->{$_} } @keys;
foreach my $key (@keys) {
- if (!exists $project_info->{$key}) {
+ if ((!$fill_only || !%$fill_only || $fill_only->{$key}) &&
+ !exists $project_info->{$key}) {
return 1;
}
}
return;
}
-
# fills project list info (age, description, owner, category, forks)
# for each project in the list, removing invalid projects from
-# returned list
+# returned list, or fill only specified info (removing invalid projects
+# only when filling 'age').
+#
# NOTE: modifies $projlist, but does not remove entries from it
sub fill_project_list_info {
- my $projlist = shift;
+ my ($projlist, @fill_only) = @_;
+ my %fill_only = map { $_ => 1 } @fill_only;
my @projects;
my $show_ctags = gitweb_check_feature('ctags');
PROJECT:
foreach my $pr (@$projlist) {
- if (project_info_needs_filling($pr, 'age', 'age_string')) {
+ if (project_info_needs_filling($pr, 'age', 'age_string', \%fill_only)) {
my (@activity) = git_get_last_activity($pr->{'path'});
unless (@activity) {
next PROJECT;
}
($pr->{'age'}, $pr->{'age_string'}) = @activity;
}
- if (project_info_needs_filling($pr, 'descr', 'descr_long')) {
+ if (project_info_needs_filling($pr, 'descr', 'descr_long', \%fill_only)) {
my $descr = git_get_project_description($pr->{'path'}) || "";
$descr = to_utf8($descr);
$pr->{'descr_long'} = $descr;
$pr->{'descr'} = chop_str($descr, $projects_list_description_width, 5);
}
- if (project_info_needs_filling($pr, 'owner')) {
+ if (project_info_needs_filling($pr, 'owner', \%fill_only)) {
$pr->{'owner'} = git_get_project_owner("$pr->{'path'}") || "";
}
if ($show_ctags &&
- project_info_needs_filling($pr, 'ctags')) {
+ project_info_needs_filling($pr, 'ctags', \%fill_only)) {
$pr->{'ctags'} = git_get_project_ctags($pr->{'path'});
}
if ($projects_list_group_categories &&
- project_info_needs_filling($pr, 'category')) {
+ project_info_needs_filling($pr, 'category', \%fill_only)) {
my $cat = git_get_project_category($pr->{'path'}) ||
$project_list_default_category;
$pr->{'category'} = to_utf8($cat);
--
1.7.9
^ permalink raw reply related
* [PATCHv2 3/8] gitweb: Faster project search
From: Jakub Narebski @ 2012-02-15 20:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <1329338332-30358-1-git-send-email-jnareb@gmail.com>
Before searching by some field the information we search for must be
filled in. For this fill_project_list_info() was enhanced in previous
commit to take additional parameters which part of projects info to
fill. This way we can limit doing expensive calculations (like
running git-for-each-ref to get 'age' / "Last changed" info) only to
projects which we will show as search results.
With this commit the number of git commands used to generate search
results is 2*<matched projects> + 1, and depends on number of matched
projects rather than number of all projects (all repositories).
Note: this is 'git for-each-ref' to find last activity, and 'git config'
for each project, and 'git --version' once.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
search_projects_list() now pre-fills required parts of project info by
itself, so running fill_project_list_info() before calling it is no
longer necessary and actually you should not do it.
No changes from v1
gitweb/gitweb.perl | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index cae71f5..cc2bd6d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2987,6 +2987,10 @@ sub search_projects_list {
return @$projlist
unless ($tagfilter || $searchtext);
+ # searching projects require filling to be run before it;
+ fill_project_list_info($projlist,
+ $tagfilter ? 'ctags' : (),
+ $searchtext ? ('path', 'descr') : ());
my @projects;
PROJECT:
foreach my $pr (@$projlist) {
@@ -5390,12 +5394,13 @@ sub git_project_list_body {
# filtering out forks before filling info allows to do less work
@projects = filter_forks_from_projects_list(\@projects)
if ($check_forks);
- @projects = fill_project_list_info(\@projects);
- # searching projects require filling to be run before it
+ # search_projects_list pre-fills required info
@projects = search_projects_list(\@projects,
'searchtext' => $searchtext,
'tagfilter' => $tagfilter)
if ($tagfilter || $searchtext);
+ # fill the rest
+ @projects = fill_project_list_info(\@projects);
$order ||= $default_projects_order;
$from = 0 unless defined $from;
--
1.7.9
^ permalink raw reply related
* [PATCHv2 5/8] gitweb: Highlight matched part of project name when searching projects
From: Jakub Narebski @ 2012-02-15 20:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <1329338332-30358-1-git-send-email-jnareb@gmail.com>
Use newly introduced esc_html_match_hl() to escape HTML and mark match
with span element with 'match' class. Currently only 'path' part
(i.e. project name) is highlighted; match might be on the project
description.
The code makes use of the fact that defined $search_regexp means that
there was search going on.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Introducing esc_html_match_hl() could have been put together with
introduction of esc_html_hl_regions() in previos commit.
Changes from v1:
* Main part of esc_html_match_hl() got split into esc_html_hl_regions(),
which was introduced in previous commit.
gitweb/gitweb.perl | 18 +++++++++++++++++-
1 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8dcd54b..5596701 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1742,6 +1742,20 @@ sub esc_html_hl_regions {
return $out;
}
+# highlight match (if any), and escape HTML
+sub esc_html_match_hl {
+ my ($str, $regexp) = @_;
+ return esc_html($str) unless defined $regexp;
+
+ my @matches;
+ while ($str =~ /$regexp/g) {
+ push @matches, [$-[0], $+[0]];
+ }
+ return esc_html($str) unless @matches;
+
+ return esc_html_hl_regions($str, 'match', @matches);
+}
+
## ----------------------------------------------------------------------
## functions returning short strings
@@ -5389,7 +5403,9 @@ sub git_project_list_rows {
print "</td>\n";
}
print "<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
- -class => "list"}, esc_html($pr->{'path'})) . "</td>\n" .
+ -class => "list"},
+ esc_html_match_hl($pr->{'path'}, $search_regexp)) .
+ "</td>\n" .
"<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
-class => "list", -title => $pr->{'descr_long'}},
esc_html($pr->{'descr'})) . "</td>\n" .
--
1.7.9
^ permalink raw reply related
* [PATCHv2 4/8] gitweb: Introduce esc_html_hl_regions
From: Jakub Narebski @ 2012-02-15 20:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <1329338332-30358-1-git-send-email-jnareb@gmail.com>
The esc_html_hl_regions() subroutine added in this commit is meant to
highlight in a given string a list of regions (given as a list of
[ beg, end ] pairs of positions in string), using HTML <span> element
with given class.
It is to be used in next commit for highlighting matched part in
project search.
Implementation and enhancement notes:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
* Currently esc_html_hl_regions() subroutine doesn't accept any
parameters, like esc_html() does. We might want for example to
pass nbsp=>1 to it.
It can easily be done with the following code:
my %opts = grep { ref($_) ne "ARRAY" } @sel;
@sel = grep { ref($_) eq "ARRAY" } @sel;
This allow adding parameters after or before regions, e.g.:
esc_html_hl_regions("foo bar", "mark", [ 0, 3 ], -nbsp => 1);
* esc_html_hl_regions() escapes like esc_html(); if we wanted to
highlight with esc_path(), we could pass subroutine reference
to now named esc_gen_hl_regions().
esc_html_hl_regions("foo bar", "mark", \&esc_path, [ 0, 3 ]);
Note that this way we can handle -nbsp=>1 case automatically,
e.g.
esc_html_hl_regions("foo bar", "mark",
sub { esc_html(@_, -nbsp=>1) },
[ 0, 3 ]);
* Alternate solution for highlighting region of a string would be to
use the idea that strings are to be HTML-escaped, and references to
scalars are HTML (like in the idea for generic committags).
This would require modifying gitweb code or esc_html to get list of
fragments, e.g.:
esc_html(\'<span class="mark">', 'foo', \'</span>', ' bar',
{ -nbsp => 1 });
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This commit is here only for the notes in the commit message,
otherwise it could have been squashed with the next commit.
Note that we actually end up using first enhancement in notes section
of the above commit message.
This patch is new; it was not present in v1
gitweb/gitweb.perl | 27 +++++++++++++++++++++++++++
1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index cc2bd6d..8dcd54b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1715,6 +1715,33 @@ sub chop_and_escape_str {
}
}
+# Highlight selected fragments of string, using given CSS class,
+# and escape HTML. It is assumed that fragments do not overlap.
+# Regions are passed as list of pairs (array references).
+#
+# Example: esc_html_hl_regions("foobar", "mark", [ 0, 3 ]) returns
+# '<span class="mark">foo</span>bar'
+sub esc_html_hl_regions {
+ my ($str, $css_class, @sel) = @_;
+ return esc_html($str) unless @sel;
+
+ my $out = '';
+ my $pos = 0;
+
+ for my $s (@sel) {
+ $out .= esc_html(substr($str, $pos, $s->[0] - $pos))
+ if ($s->[0] - $pos > 0);
+ $out .= $cgi->span({-class => $css_class},
+ esc_html(substr($str, $s->[0], $s->[1] - $s->[0])));
+
+ $pos = $s->[1];
+ }
+ $out .= esc_html(substr($str, $pos))
+ if ($pos < length($str));
+
+ return $out;
+}
+
## ----------------------------------------------------------------------
## functions returning short strings
--
1.7.9
^ permalink raw reply related
* [PATCHv3/RFC 7/8] gitweb: Highlight matched part of shortened project description
From: Jakub Narebski @ 2012-02-15 20:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <1329338332-30358-1-git-send-email-jnareb@gmail.com>
Previous commit make gitweb use esc_html_match_hl() to mark match in
the _whole_ description of a project when searching projects.
This commit makes gitweb highlight match in _shortened_ description,
based on match in whole description, using esc_html_match_hl_chopped()
subroutine.
If match is in removed (chopped) part, even partially, then trailing
"... " is highlighted.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
It is marked as RFC, because I am not sure if it is right way to
highlight match in shortened string, or if we better use full string,
or full string if match is in chopped part.
Changes from v2:
* Harden esc_html_match_hl_chopped() against calling with both
$chopped and $regexp undefined (even though it wouldn't happen
with current code).
* esc_html_match_hl_chopped() uses now esc_html_hl_regions(),
like esc_html_match_hl() used to do.
Changes from v1:
* Instead of esc_html_match_hl_chopped() duplicating much od code in
esc_html_match_hl(), make esc_html_match_hl() call the *_chopped()
one with $chopped set to undef.
Now managing highlighting of $chopped part is just a matter of
adjusting and filtering @matches to apply to $chopped rather than
original $str where match was performed.
As a side issue when match span past chop point current code uses
one selection <span class+match">foo... </span> and not two
<span class="match">foo</span><span class="match">... </span>
gitweb/gitweb.perl | 40 ++++++++++++++++++++++++++++++++++++++--
1 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a109ebb..a2e2023 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1747,11 +1747,46 @@ sub esc_html_match_hl {
my ($str, $regexp) = @_;
return esc_html($str) unless defined $regexp;
+ return esc_html_match_hl_chopped($str, undef, $regexp);
+}
+
+
+# highlight match (if any) of shortened string, and escape HTML
+sub esc_html_match_hl_chopped {
+ my ($str, $chopped, $regexp) = @_;
+ return esc_html(defined $chopped ? $chopped : $str) unless defined $regexp;
+
my @matches;
while ($str =~ /$regexp/g) {
push @matches, [$-[0], $+[0]];
}
- return esc_html($str) unless @matches;
+ return esc_html(defined $chopped ? $chopped : $str) unless @matches;
+
+ # filter matches so that we mark chopped string, if it is present
+ if (defined $chopped) {
+ my $tail = "... "; # see chop_str
+ unless ($chopped =~ s/\Q$tail\E$//) {
+ $tail = '';
+ }
+ my $chop_len = length($chopped);
+ my $tail_len = length($tail);
+ my @filtered;
+
+ for my $m (@matches) {
+ if ($m->[0] > $chop_len) {
+ push @filtered, [ $chop_len, $chop_len + $tail_len ] if ($tail_len > 0);
+ last;
+ } elsif ($m->[1] > $chop_len) {
+ push @filtered, [ $m->[0], $chop_len + $tail_len ];
+ last;
+ }
+ push @filtered, $m;
+ }
+
+ # further operations are on chopped string
+ $str = $chopped . $tail;
+ @matches = @filtered;
+ }
return esc_html_hl_regions($str, 'match', @matches);
}
@@ -5409,7 +5444,8 @@ sub git_project_list_rows {
"<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
-class => "list", -title => $pr->{'descr_long'}},
$search_regexp
- ? esc_html_match_hl($pr->{'descr_long'}, $search_regexp)
+ ? esc_html_match_hl_chopped($pr->{'descr_long'},
+ $pr->{'descr'}, $search_regexp)
: esc_html($pr->{'descr'})) .
"</td>\n" .
"<td><i>" . chop_and_escape_str($pr->{'owner'}, 15) . "</i></td>\n";
--
1.7.9
^ permalink raw reply related
* [PATCHv2 8/8] gitweb: Use esc_html_match_hl() in 'grep' search
From: Jakub Narebski @ 2012-02-15 20:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <1329338332-30358-1-git-send-email-jnareb@gmail.com>
Use esc_html_match_hl() in git_search_files() i.e. subroutine that
implements "grep" search, instead of custom code which highlighted
only one, last match.
This required enhancing esc_html_match_hl() to accept -nbsp=>1 and
pass it down to esc_html().
Note that line is untabified (tabs turned into spaces) before
highlighting match, which means that highlighting won't work e.g. for
matching tab character "\t" explicitly; but this issue was present
before this commit, and is not that easy to fix.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch was not present in previous (v1) version of this patch
series.
The 'grep' search was chosen from other searches because of the
following reasons:
* 'pickaxe' search does not show matches in diff, only filenames.
* 'commit' search shortens leading text at beginning, one last match
in the middle, and trailing text at the end; anyway I think this
search should be rewritten to show just "log"-like view with match
highlighting.
gitweb/gitweb.perl | 34 ++++++++++++++--------------------
1 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a2e2023..36a8cca 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1723,20 +1723,22 @@ sub chop_and_escape_str {
# '<span class="mark">foo</span>bar'
sub esc_html_hl_regions {
my ($str, $css_class, @sel) = @_;
- return esc_html($str) unless @sel;
+ my %opts = grep { ref($_) ne 'ARRAY' } @sel;
+ @sel = grep { ref($_) eq 'ARRAY' } @sel;
+ return esc_html($str, %opts) unless @sel;
my $out = '';
my $pos = 0;
for my $s (@sel) {
- $out .= esc_html(substr($str, $pos, $s->[0] - $pos))
+ $out .= esc_html(substr($str, $pos, $s->[0] - $pos), %opts)
if ($s->[0] - $pos > 0);
$out .= $cgi->span({-class => $css_class},
- esc_html(substr($str, $s->[0], $s->[1] - $s->[0])));
+ esc_html(substr($str, $s->[0], $s->[1] - $s->[0]), %opts));
$pos = $s->[1];
}
- $out .= esc_html(substr($str, $pos))
+ $out .= esc_html(substr($str, $pos), %opts)
if ($pos < length($str));
return $out;
@@ -1744,23 +1746,23 @@ sub esc_html_hl_regions {
# highlight match (if any), and escape HTML
sub esc_html_match_hl {
- my ($str, $regexp) = @_;
- return esc_html($str) unless defined $regexp;
+ my ($str, $regexp, %opts) = @_;
+ return esc_html($str, %opts) unless defined $regexp;
- return esc_html_match_hl_chopped($str, undef, $regexp);
+ return esc_html_match_hl_chopped($str, undef, $regexp, %opts);
}
# highlight match (if any) of shortened string, and escape HTML
sub esc_html_match_hl_chopped {
- my ($str, $chopped, $regexp) = @_;
- return esc_html(defined $chopped ? $chopped : $str) unless defined $regexp;
+ my ($str, $chopped, $regexp, %opts) = @_;
+ return esc_html(defined $chopped ? $chopped : $str, %opts) unless defined $regexp;
my @matches;
while ($str =~ /$regexp/g) {
push @matches, [$-[0], $+[0]];
}
- return esc_html(defined $chopped ? $chopped : $str) unless @matches;
+ return esc_html(defined $chopped ? $chopped : $str, %opts) unless @matches;
# filter matches so that we mark chopped string, if it is present
if (defined $chopped) {
@@ -1788,7 +1790,7 @@ sub esc_html_match_hl_chopped {
@matches = @filtered;
}
- return esc_html_hl_regions($str, 'match', @matches);
+ return esc_html_hl_regions($str, 'match', @matches, %opts);
}
## ----------------------------------------------------------------------
@@ -6070,15 +6072,7 @@ sub git_search_files {
print "<div class=\"binary\">Binary file</div>\n";
} else {
$ltext = untabify($ltext);
- if ($ltext =~ m/^(.*)($search_regexp)(.*)$/i) {
- $ltext = esc_html($1, -nbsp=>1);
- $ltext .= '<span class="match">';
- $ltext .= esc_html($2, -nbsp=>1);
- $ltext .= '</span>';
- $ltext .= esc_html($3, -nbsp=>1);
- } else {
- $ltext = esc_html($ltext, -nbsp=>1);
- }
+ $ltext = esc_html_match_hl($ltext, qr/$search_regexp/i, -nbsp=>1);
print "<div class=\"pre\">" .
$cgi->a({-href => $file_href.'#l'.$lno,
-class => "linenr"}, sprintf('%4i', $lno)) .
--
1.7.9
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox