* Re: [PATCH 5/6] builtin/verify-tag: Add --format to verify-tag
From: Santiago Torres @ 2016-09-23 14:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, peff, sunshine, walters
In-Reply-To: <xmqqintn3aai.fsf@gitster.mtv.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 2366 bytes --]
On Thu, Sep 22, 2016 at 02:16:21PM -0700, Junio C Hamano wrote:
> santiago@nyu.edu writes:
>
> > From: Santiago Torres <santiago@nyu.edu>
> >
> > Callers of verify-tag may want to cross-check the tagname from refs/tags
> > with the tagname from the tag object header upon GPG verification. This
> > is to avoid tag refs that point to an incorrect object.
> >
> > Add a --format parameter to git verify-tag to print the formatted tag
> > object header in addition to or instead of the --verbose or --raw GPG
> > verification output.
> >
> > Signed-off-by: Santiago Torres <santiago@nyu.edu>
> > ---
> > builtin/verify-tag.c | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> > index 7a1121b..319d469 100644
> > --- a/builtin/verify-tag.c
> > +++ b/builtin/verify-tag.c
> > @@ -12,12 +12,15 @@
> > #include <signal.h>
> > #include "parse-options.h"
> > #include "gpg-interface.h"
> > +#include "ref-filter.h"
> >
> > static const char * const verify_tag_usage[] = {
> > - N_("git verify-tag [-v | --verbose] <tag>..."),
> > + N_("git verify-tag [-v | --verbose] [--format=<format>] <tag>..."),
> > NULL
> > };
> >
> > +char *fmt_pretty;
>
> Does this have to be extern? I do not think so; prepend "static "
> in front of it.
>
> > while (i < argc) {
> > unsigned char sha1[20];
> > const char *name = argv[i++];
> > if (get_sha1(name, sha1))
> > had_error = !!error("tag '%s' not found.", name);
> > else {
> > - if (verify_and_format_tag(sha1, name, NULL, flags))
> > + if (verify_and_format_tag(sha1, name, fmt_pretty, flags))
>
> OK. The callchain from here is
>
> verify_and_format_tag()
> -> run_gpg_verify()
> -> print_signature_buffer()
>
> so not cramming QUIET into the flags parameter that is already
> passed is cumbersome. As I said in my earlier review, it would make
> more sense to have the conditional NOT in print_signature_buffer()
> but in its caller, but it still is OK to add GPG_VERIFY_QUIET bit
> to the flag, which you would check in run_gpg_verify() to decide not
> to call print_signature_buffer().
>
Yeah, in retrospect, this sounds like a more reasonable approach than
doing it on gpg-nterface. I'll keep the QUIET bit then.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH 6/6] builtin/tag: add --format argument for tag -v
From: Santiago Torres @ 2016-09-23 14:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, peff, sunshine, walters, Lukas P, Lukas Puehringer
In-Reply-To: <xmqqeg4b39ye.fsf@gitster.mtv.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 859 bytes --]
> OK, you said something about for_each_ref() in an earlier commit,
> but what you meant was this one, which takes each_tag_name_fn.
Oh yeah, sorry for the confusion.
>
> The function for_each_tag_name(), the type each_tag_name_fn, and the
> function of that type verify_tag(), are ALL file-scope static in
> this single file, builtin/tag.c. It seems to me that it is not
> necessary to make the format string global at all.
Oh, ok. I was thinking that this was preferred over changing the
signature of those functions. (I drew my conclusion from log.c). I'll
take this other road then.
>
> ...
>
> There are minor implementation and design issues I spotted, but
> overall I think the feature the series attempts to add may be a good
> thing to have.
>
Thanks for the review! I'll re-roll shortly.
-Santiago.
> Thanks.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* [PATCH v2 4/3] init: combine set_git_dir_init() and init_db() into one
From: Nguyễn Thái Ngọc Duy @ 2016-09-23 11:12 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, git, max.nordlund,
Nguyễn Thái Ngọc Duy
In-Reply-To: <xmqqbmzf6e0r.fsf@gitster.mtv.corp.google.com>
Commit "init: do not set core.worktree more often than necessary" adds a
subtle dependency between set_git_dir_init() and init_db(). The former
_must_ be called before init_db() so that original_git_dir can be set
properly. If something else, like enter_repo() or setup_git_directory(),
is used instead, the trick in that commit breaks down.
To eliminate the possibility that init_db() in future may be called
without set_git_dir_init(), init_db() now calls that function internally
(and does not allow anybody else to use it).
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
I think a separate commit for this is better than combining back to
2/3 so we can explain the problem properly (without making 2/3 commit
message even longer)
Not sure if you want to s/contains/contain/ in 2/3 by yourself or I
should resend the whole series. Let me know.
builtin/clone.c | 15 +++++++--------
builtin/init-db.c | 18 +++++++++++-------
cache.h | 5 +++--
3 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 6616392..29b1832 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -928,23 +928,22 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
set_git_work_tree(work_tree);
}
- junk_git_dir = git_dir;
+ junk_git_dir = real_git_dir ? real_git_dir : git_dir;
if (safe_create_leading_directories_const(git_dir) < 0)
die(_("could not create leading directories of '%s'"), git_dir);
- set_git_dir_init(git_dir, real_git_dir, 0);
- if (real_git_dir) {
- git_dir = real_git_dir;
- junk_git_dir = real_git_dir;
- }
-
if (0 <= option_verbosity) {
if (option_bare)
fprintf(stderr, _("Cloning into bare repository '%s'...\n"), dir);
else
fprintf(stderr, _("Cloning into '%s'...\n"), dir);
}
- init_db(option_template, INIT_DB_QUIET);
+
+ init_db(git_dir, real_git_dir, option_template, INIT_DB_QUIET);
+
+ if (real_git_dir)
+ git_dir = real_git_dir;
+
write_config(&option_config);
git_config(git_default_config, NULL);
diff --git a/builtin/init-db.c b/builtin/init-db.c
index d70fc45..ee7942f 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -312,8 +312,9 @@ static void create_object_directory(void)
strbuf_release(&path);
}
-int set_git_dir_init(const char *git_dir, const char *real_git_dir,
- int exist_ok)
+static int set_git_dir_init(const char *git_dir,
+ const char *real_git_dir,
+ int exist_ok)
{
original_git_dir = xstrdup(real_path(git_dir));
@@ -362,10 +363,14 @@ static void separate_git_dir(const char *git_dir)
write_file(git_link, "gitdir: %s", git_dir);
}
-int init_db(const char *template_dir, unsigned int flags)
+int init_db(const char *git_dir, const char *real_git_dir,
+ const char *template_dir, unsigned int flags)
{
int reinit;
- const char *git_dir = get_git_dir();
+
+ set_git_dir_init(git_dir, real_git_dir, flags & INIT_DB_EXIST_OK);
+
+ git_dir = get_git_dir();
if (git_link)
separate_git_dir(git_dir);
@@ -585,7 +590,6 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
set_git_work_tree(work_tree);
}
- set_git_dir_init(git_dir, real_git_dir, 1);
-
- return init_db(template_dir, flags);
+ flags |= INIT_DB_EXIST_OK;
+ return init_db(git_dir, real_git_dir, template_dir, flags);
}
diff --git a/cache.h b/cache.h
index b2d77f3..7fc875f 100644
--- a/cache.h
+++ b/cache.h
@@ -525,9 +525,10 @@ extern void verify_non_filename(const char *prefix, const char *name);
extern int path_inside_repo(const char *prefix, const char *path);
#define INIT_DB_QUIET 0x0001
+#define INIT_DB_EXIST_OK 0x0002
-extern int set_git_dir_init(const char *git_dir, const char *real_git_dir, int);
-extern int init_db(const char *template_dir, unsigned int flags);
+extern int init_db(const char *git_dir, const char *real_git_dir,
+ const char *template_dir, unsigned int flags);
extern void sanitize_stdfds(void);
extern int daemonize(void);
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [RFC PATCH] revision: new rev%n shorthand for rev^n..rev
From: Vegard Nossum @ 2016-09-23 10:52 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Santi Béjar, Kevin Bracey, Philip Oakley,
Vegard Nossum
I use rev^..rev daily, and I'm surely not the only one. To save typing
(or copy-pasting, if the rev is long -- like a full SHA-1 or branch name)
we can make rev% a shorthand for that.
The existing syntax rev^! seems like it should do the same, but it
doesn't really do the right thing for merge commits (it gives only the
merge itself).
As a natural generalisation, we also accept rev%n where n excludes the
nth parent of rev. It _may_ be more useful to define rev%n for an m-way
merge as:
rev
^rev^1
^rev^[... except n]
^rev^m
so that you can see only the commits that arrived via the nth parent,
but this might be questionable/unintuitive in case any of the parents
that share commits (as you would get fewer commits than expected).
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
Documentation/revisions.txt | 14 +++++++++++++
builtin/rev-parse.c | 38 ++++++++++++++++++++++++++++++++++
revision.c | 50 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 102 insertions(+)
diff --git Documentation/revisions.txt Documentation/revisions.txt
index 4bed5b1..ab2dc2c 100644
--- Documentation/revisions.txt
+++ Documentation/revisions.txt
@@ -281,6 +281,14 @@ is a shorthand for 'HEAD..origin' and asks "What did the origin do since
I forked from them?" Note that '..' would mean 'HEAD..HEAD' which is an
empty range that is both reachable and unreachable from HEAD.
+Parent Exclusion Notation
+~~~~~~~~~~~~~~~~~~~~~~~~~
+The '<rev>%{<n>}', Parent Exclusion Notation::
+Shorthand for '<rev>{caret}<n>..<rev>', with '<n>' = 1 if not
+given. This is typically useful for merge commits where you
+can just pass '<commit>%' to get all the commits in the branch
+that was merged in merge commit '<commit>'.
+
Other <rev>{caret} Parent Shorthand Notations
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Two other shorthands exist, particularly useful for merge commits,
@@ -316,6 +324,10 @@ Revision Range Summary
<rev2> but exclude those that are reachable from both. When
either <rev1> or <rev2> is omitted, it defaults to `HEAD`.
+'<rev>%{<n>}', e.g. 'HEAD%, HEAD%2'::
+ Equivalent to '<rev>{caret}<n>..<rev>', with '<n>' = 1 if not
+ given.
+
'<rev>{caret}@', e.g. 'HEAD{caret}@'::
A suffix '{caret}' followed by an at sign is the same as listing
all parents of '<rev>' (meaning, include anything reachable from
@@ -339,6 +351,8 @@ spelt out:
C I J F C
B..C = ^B C C
B...C = B ^F C G H D E B C
+ B% = B^..B
+ = B ^B^1 E I J F B
C^@ = C^1
= F I J F
B^@ = B^1 B^2 B^3
diff --git builtin/rev-parse.c builtin/rev-parse.c
index 76cf05e..f081b81 100644
--- builtin/rev-parse.c
+++ builtin/rev-parse.c
@@ -292,6 +292,42 @@ static int try_difference(const char *arg)
return 0;
}
+static int try_branch(const char *arg)
+{
+ char *percent;
+ unsigned char sha1[20];
+ unsigned char end[20];
+
+ /*
+ * <rev>%{<n>} is shorthand for <rev>^<n>..<rev>, with <n> = 1 if
+ * not given. This is typically used for merge commits where you
+ * can just pass <merge>% and it will show you all the commits in
+ * the branch that was merged (for octopus merges, <n> is the nth
+ * branch).
+ */
+
+ if (!(percent = strstr(arg, "%")))
+ return 0;
+
+ *percent = '^';
+ if (!get_sha1_committish(arg, sha1)) {
+ *percent = '%';
+ return 0;
+ }
+
+ *percent = '\0';
+ if (!get_sha1_committish(arg, end)) {
+ *percent = '%';
+ return 0;
+ }
+
+ show_rev(NORMAL, end, arg);
+ *percent = '^';
+ show_rev(REVERSED, sha1, arg);
+ *percent = '%';
+ return 1;
+}
+
static int try_parent_shorthands(const char *arg)
{
char *dotdot;
@@ -839,6 +875,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
/* Not a flag argument */
if (try_difference(arg))
continue;
+ if (try_branch(arg))
+ continue;
if (try_parent_shorthands(arg))
continue;
name = arg;
diff --git revision.c revision.c
index 969b3d1..e20b618 100644
--- revision.c
+++ revision.c
@@ -1519,6 +1519,56 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
}
*dotdot = '.';
}
+
+ /*
+ * <rev>%{<n>} is shorthand for <rev>^<n>..<rev>, with <n> = 1 if
+ * not given. This is typically used for merge commits where you
+ * can just pass <merge>% and it will show you all the commits in
+ * the branch that was merged (for octopus merges, <n> is the nth
+ * branch).
+ */
+ dotdot = strstr(arg, "%");
+ if (dotdot) {
+ unsigned char sha1[20];
+ unsigned char end[20];
+ struct object *a_obj, *b_obj;
+ unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);
+ unsigned int a_flags;
+
+ *dotdot = '\0';
+ if (get_sha1_committish(arg, end)) {
+ if (revs->ignore_missing)
+ return 0;
+ die("Unknown revision %s", arg);
+ }
+
+ *dotdot = '^';
+ if (get_sha1_committish(arg, sha1)) {
+ if (revs->ignore_missing)
+ return 0;
+ die("Unknown revision %s", arg);
+ }
+
+ a_obj = parse_object(sha1);
+ b_obj = parse_object(end);
+ if (!a_obj || !b_obj) {
+ if (revs->ignore_missing)
+ return 0;
+ die("Invalid revision range %s", arg);
+ }
+
+ a_flags = flags_exclude;
+ a_obj->flags |= a_flags;
+ b_obj->flags |= flags;
+ *dotdot = '^';
+ add_rev_cmdline(revs, a_obj, arg, REV_CMD_LEFT, a_flags);
+ add_pending_object(revs, a_obj, arg);
+ *dotdot = '\0';
+ add_rev_cmdline(revs, b_obj, arg, REV_CMD_RIGHT, flags);
+ add_pending_object(revs, b_obj, arg);
+ *dotdot = '%';
+ return 0;
+ }
dotdot = strstr(arg, "^@");
if (dotdot && !dotdot[2]) {
*dotdot = 0;
--
2.10.0.rc0.1.g07c9292
^ permalink raw reply related
* Re: Limitiations of git rebase --preserve-merges --interactive
From: Stefan Haller @ 2016-09-23 10:40 UTC (permalink / raw)
To: Stefan Beller, Kevin Daudt
Cc: Anatoly Borodin, git@vger.kernel.org, Jonathan Nieder
In-Reply-To: <CAGZ79kYoFH0dbQwvL_2JOtd8aybH8MKCtzy+p_88+QykVTGUtg@mail.gmail.com>
Stefan Beller <sbeller@google.com> wrote:
> On Thu, Sep 22, 2016 at 12:48 PM, Kevin Daudt <me@ikke.info> wrote:
> > On Thu, Sep 22, 2016 at 07:33:11PM +0000, Anatoly Borodin wrote:
> >> Hi Stefan,
> >>
> >> this section was added to the manual in the commit
> >> cddb42d2c58a9de9b2b5ef68817778e7afaace3e by "Jonathan Nieder"
> >> <jrnieder@gmail.com> 6 years ago. Maybe he remembers better?
> >>
> >
> > Just to make it clear, this section explicitly talks about 'bugs' with
> > preserve-merges and interactive rebase. Without the --preserve-merges
> > option, those operations works as expected.
> >
> > The reason, as that section explains, is that it's not possible to store
> > the merge structure in the flat todo list. I assume this means git
> > internally remembers where the merge commit was, and then restores it
> > while rebasing.
> >
> > Changing the order, or dropping commits might then give unexpected
> > results.
> >
>
> The commit message may help as well:
>
> rebase -i -p: document shortcomings
>
> The rebase --preserve-merges facility presents a list of commits
> in its instruction sheet and uses a separate table to keep
> track of their parents. Unfortunately, in practice this means
> that with -p after most attempts to rearrange patches, some
> commits have the "wrong" parent and the resulting history is
> rarely what the caller expected.
>
> Yes, it would be nice to fix that. But first, add a warning to the
> manual to help the uninitiated understand what is going on.
Thanks, but all of this still talks about the issues in very generic
terms ("most attempts to rearrange patches"). I'm interested in more
details as to exactly what kind of attempts do or don't work. In
particular, I'm interested in fixup/squash commands (without reordering
anything else), or dropping (non-merge) commits.
I could of course experiment with these and try to find out myself, but
I was hoping someone would just know the answer off the top of their
head, saving me some time.
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
^ permalink raw reply
* Re: Limitiations of git rebase --preserve-merges --interactive
From: Stefan Haller @ 2016-09-23 10:40 UTC (permalink / raw)
To: Anatoly Borodin, git
In-Reply-To: <ns1gr4$pjs$1@blaine.gmane.org>
Anatoly Borodin <anatoly.borodin@gmail.com> wrote:
> PS There are also some pieces of "what should work" in these tests:
>
> t/t3409-rebase-preserve-merges.sh*
> t/t3410-rebase-preserve-dropped-merges.sh*
> t/t3411-rebase-preserve-around-merges.sh*
> t/t3414-rebase-preserve-onto.sh*
Thanks, this is interesting; I'm having trouble understanding the tests
though. Some of them use rebase -p -i, but I don't understand why they
use -i, or why that even works in a test (i.e. why it doesn't open an
editor).
In one test I saw "GIT_EDITOR=: git rebase -i -p", which I guess means
"use the initially given todo sheet unchanged". I don't see any tests
that do an interactive rebase and actually change the todo list.
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
^ permalink raw reply
* [PATCH v3 2/2] gitweb: use highlight's shebang detection
From: Ian Kelling @ 2016-09-23 9:08 UTC (permalink / raw)
To: git; +Cc: jnareb
In-Reply-To: <20160923090846.3086-1-ian@iankelling.org>
The "highlight" binary can, in some cases, determine the language type
by the means of file contents, for example the shebang in the first line
for some scripting languages. Make use of this autodetection for files
which syntax is not known by gitweb. In that case, pass the blob
contents to "highlight --force"; the parameter is needed to make it
always generate HTML output (which includes HTML-escaping).
Although we now run highlight on files which do not end up highlighted,
performance is virtually unaffected because when we call highlight, we
also call sanitize() instead of esc_html(), which is significantly
slower. After curling blob view of unhighlighted large and small text
files of perl code and license text 100 times each on a local
Apache/2.4.23 (Debian) instance, it's logs indicate +-1% difference in
request time for all file types.
Document the feature and improve syntax highlight documentation, add
test to ensure gitweb doesn't crash when language detection is used.
Signed-off-by: Ian Kelling <ian@iankelling.org>
---
Documentation/gitweb.conf.txt | 21 ++++++++++++++-------
gitweb/gitweb.perl | 10 +++++-----
t/t9500-gitweb-standalone-no-errors.sh | 8 ++++++++
3 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index a79e350..e632089 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -246,13 +246,20 @@ $highlight_bin::
Note that 'highlight' feature must be set for gitweb to actually
use syntax highlighting.
+
-*NOTE*: if you want to add support for new file type (supported by
-"highlight" but not used by gitweb), you need to modify `%highlight_ext`
-or `%highlight_basename`, depending on whether you detect type of file
-based on extension (for example "sh") or on its basename (for example
-"Makefile"). The keys of these hashes are extension and basename,
-respectively, and value for given key is name of syntax to be passed via
-`--syntax <syntax>` to highlighter.
+*NOTE*: for a file to be highlighted, its syntax type must be detected
+and that syntax must be supported by "highlight". The default syntax
+detection is minimal, and there are many supported syntax types with no
+detection by default. There are three options for adding syntax
+detection. The first and second priority are `%highlight_basename` and
+`%highlight_ext`, which detect based on basename (the full filename, for
+example "Makefile") and extension (for example "sh"). The keys of these
+hashes are the basename and extension, respectively, and the value for a
+given key is the name of the syntax to be passed via `--syntax <syntax>`
+to "highlight". The last priority is the "highlight" configuration of
+`Shebang` regular expressions to detect the language based on the first
+line in the file, (for example, matching the line "#!/bin/bash"). See
+the highlight documentation and the default config at
+/etc/highlight/filetypes.conf for more details.
+
For example if repositories you are hosting use "phtml" extension for
PHP files, and you want to have correct syntax-highlighting for those
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 6cb4280..44094f4 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3931,15 +3931,16 @@ sub guess_file_syntax {
# or return original FD if no highlighting
sub run_highlighter {
my ($fd, $highlight, $syntax) = @_;
- return $fd unless ($highlight && defined $syntax);
+ return $fd unless ($highlight);
close $fd;
+ my $syntax_arg = (defined $syntax) ? "--syntax $syntax" : "--force";
open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',
'$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',
'--', "-fe=$fallback_encoding")." | ".
quote_command($highlight_bin).
- " --replace-tabs=8 --fragment --syntax $syntax |"
+ " --replace-tabs=8 --fragment $syntax_arg |"
or die_error(500, "Couldn't open file or run syntax highlighter");
return $fd;
}
@@ -7063,8 +7064,7 @@ sub git_blob {
my $highlight = gitweb_check_feature('highlight');
my $syntax = guess_file_syntax($highlight, $file_name);
- $fd = run_highlighter($fd, $highlight, $syntax)
- if $syntax;
+ $fd = run_highlighter($fd, $highlight, $syntax);
git_header_html(undef, $expires);
my $formats_nav = '';
@@ -7117,7 +7117,7 @@ sub git_blob {
$line = untabify($line);
printf qq!<div class="pre"><a id="l%i" href="%s#l%i" class="linenr">%4i</a> %s</div>\n!,
$nr, esc_attr(href(-replay => 1)), $nr, $nr,
- $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);
+ $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);
}
}
close $fd
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index e94b2f1..6d06ed9 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -709,6 +709,14 @@ test_expect_success HIGHLIGHT \
git commit -m "Add test.sh" &&
gitweb_run "p=.git;a=blob;f=test.sh"'
+test_expect_success HIGHLIGHT \
+ 'syntax highlighting (highlighter language autodetection)' \
+ 'git config gitweb.highlight yes &&
+ echo "#!/usr/bin/perl" > test &&
+ git add test &&
+ git commit -m "Add test" &&
+ gitweb_run "p=.git;a=blob;f=test"'
+
# ----------------------------------------------------------------------
# forks of projects
--
2.9.3
^ permalink raw reply related
* [PATCH v3 1/2] gitweb: remove unused function parameter
From: Ian Kelling @ 2016-09-23 9:08 UTC (permalink / raw)
To: git; +Cc: jnareb
In-Reply-To: <2a5aead0-d521-85eb-f304-7091683d8b40@gmail.com>
Signed-off-by: Ian Kelling <ian@iankelling.org>
---
gitweb/gitweb.perl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 33d701d..6cb4280 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3913,7 +3913,7 @@ sub blob_contenttype {
# guess file syntax for syntax highlighting; return undef if no highlighting
# the name of syntax can (in the future) depend on syntax highlighter used
sub guess_file_syntax {
- my ($highlight, $mimetype, $file_name) = @_;
+ my ($highlight, $file_name) = @_;
return undef unless ($highlight && defined $file_name);
my $basename = basename($file_name, '.in');
return $highlight_basename{$basename}
@@ -7062,7 +7062,7 @@ sub git_blob {
$have_blame &&= ($mimetype =~ m!^text/!);
my $highlight = gitweb_check_feature('highlight');
- my $syntax = guess_file_syntax($highlight, $mimetype, $file_name);
+ my $syntax = guess_file_syntax($highlight, $file_name);
$fd = run_highlighter($fd, $highlight, $syntax)
if $syntax;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2] gitweb: use highlight's shebang detection
From: Ian Kelling @ 2016-09-23 9:08 UTC (permalink / raw)
To: Jakub Narębski, git
In-Reply-To: <2a5aead0-d521-85eb-f304-7091683d8b40@gmail.com>
On Thu, Sep 22, 2016, at 03:50 PM, Jakub Narębski wrote:
> W dniu 22.09.2016 o 00:18, Ian Kelling napisał:
>
> > The highlight binary can detect language by shebang when we can't tell
> > the syntax type by the name of the file. In that case, pass the blob
> > to "highlight --force" and the resulting html will have markup for
> > highlighting if the language was detected.
>
> This description feels a bit convoluted. Perhaps something like this:
>
> The "highlight" binary can, in some cases, determine the language type
> by the means of file contents, for example the shebang in the first
> line
> for some scripting languages. Make use of this autodetection for files
> which syntax is not known by gitweb. In that case, pass the blob
> contents to "highlight --force"; the parameter is needed to make it
> always generate HTML output (which includes HTML-escaping).
Nice. Using it in v3.
>
> Also, we might want to have the information about performance of this
> solution either in the commit message, or in commit comments.
I tested it more rigorously and added to v3 commit message.
>
> >
> > Document the feature and improve syntax highlight documentation, add
> > test to ensure gitweb doesn't crash when language detection is used,
>
> All right.
>
> > and remove an unused parameter from gitweb_check_feature().
>
> First, that is guess_file_syntax(), not gitweb_check_feature().
> Second, this change could be made into independent patch, for example
> preparatory one.
Oops. I split it out in v3.
>
> >
> > Signed-off-by: Ian Kelling <ian@iankelling.org>
> > ---
> > Documentation/gitweb.conf.txt | 21 ++++++++++++++-------
> > gitweb/gitweb.perl | 14 +++++++-------
> > t/t9500-gitweb-standalone-no-errors.sh | 8 ++++++++
> > 3 files changed, 29 insertions(+), 14 deletions(-)
> >
> > diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
> > index a79e350..e632089 100644
> > --- a/Documentation/gitweb.conf.txt
> > +++ b/Documentation/gitweb.conf.txt
> > @@ -246,13 +246,20 @@ $highlight_bin::
> > Note that 'highlight' feature must be set for gitweb to actually
> > use syntax highlighting.
> > +
> > -*NOTE*: if you want to add support for new file type (supported by
> > -"highlight" but not used by gitweb), you need to modify `%highlight_ext`
> > -or `%highlight_basename`, depending on whether you detect type of file
> > -based on extension (for example "sh") or on its basename (for example
> > -"Makefile"). The keys of these hashes are extension and basename,
> > -respectively, and value for given key is name of syntax to be passed via
> > -`--syntax <syntax>` to highlighter.
> > +*NOTE*: for a file to be highlighted, its syntax type must be detected
> > +and that syntax must be supported by "highlight". The default syntax
> > +detection is minimal, and there are many supported syntax types with no
> > +detection by default. There are three options for adding syntax
> > +detection. The first and second priority are `%highlight_basename` and
> > +`%highlight_ext`, which detect based on basename (the full filename, for
> > +example "Makefile") and extension (for example "sh"). The keys of these
> > +hashes are the basename and extension, respectively, and the value for a
> > +given key is the name of the syntax to be passed via `--syntax <syntax>`
> > +to "highlight". The last priority is the "highlight" configuration of
> > +`Shebang` regular expressions to detect the language based on the first
> > +line in the file, (for example, matching the line "#!/bin/bash"). See
> > +the highlight documentation and the default config at
> > +/etc/highlight/filetypes.conf for more details.
> > +
>
> I think the rewrite is a bit more readable.
>
> > For example if repositories you are hosting use "phtml" extension for
> > PHP files, and you want to have correct syntax-highlighting for those
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index 33d701d..44094f4 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -3913,7 +3913,7 @@ sub blob_contenttype {
> > # guess file syntax for syntax highlighting; return undef if no highlighting
> > # the name of syntax can (in the future) depend on syntax highlighter used
> > sub guess_file_syntax {
> > - my ($highlight, $mimetype, $file_name) = @_;
> > + my ($highlight, $file_name) = @_;
>
> Right.
>
> > return undef unless ($highlight && defined $file_name);
> > my $basename = basename($file_name, '.in');
> > return $highlight_basename{$basename}
> > @@ -3931,15 +3931,16 @@ sub guess_file_syntax {
> > # or return original FD if no highlighting
> > sub run_highlighter {
> > my ($fd, $highlight, $syntax) = @_;
> > - return $fd unless ($highlight && defined $syntax);
> > + return $fd unless ($highlight);
>
> Run highlighter if it is defined, even if gitweb doesn't know syntax,
> right.
>
> >
> > close $fd;
> > + my $syntax_arg = (defined $syntax) ? "--syntax $syntax" : "--force";
> > open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
> > quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',
> > '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',
> > '--', "-fe=$fallback_encoding")." | ".
> > quote_command($highlight_bin).
> > - " --replace-tabs=8 --fragment --syntax $syntax |"
> > + " --replace-tabs=8 --fragment $syntax_arg |"
>
> Use '--force' if syntax is unknown, right.
>
> > or die_error(500, "Couldn't open file or run syntax highlighter");
> > return $fd;
> > }
> > @@ -7062,9 +7063,8 @@ sub git_blob {
> > $have_blame &&= ($mimetype =~ m!^text/!);
> >
> > my $highlight = gitweb_check_feature('highlight');
> > - my $syntax = guess_file_syntax($highlight, $mimetype, $file_name);
> > - $fd = run_highlighter($fd, $highlight, $syntax)
> > - if $syntax;
> > + my $syntax = guess_file_syntax($highlight, $file_name);
> > + $fd = run_highlighter($fd, $highlight, $syntax);
>
> Remove unused parameter from callsite, *and* run highlighter even if we
> don't know syntax.
>
> >
> > git_header_html(undef, $expires);
> > my $formats_nav = '';
> > @@ -7117,7 +7117,7 @@ sub git_blob {
> > $line = untabify($line);
> > printf qq!<div class="pre"><a id="l%i" href="%s#l%i" class="linenr">%4i</a> %s</div>\n!,
> > $nr, esc_attr(href(-replay => 1)), $nr, $nr,
> > - $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);
> > + $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);
>
> This is a bit of code duplication / sync from run_highlighter(), but
> it is not your fault; it was there (and I don't know how to improve it).
>
> > }
> > }
> > close $fd
> > diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> > index e94b2f1..576db6d 100755
> > --- a/t/t9500-gitweb-standalone-no-errors.sh
> > +++ b/t/t9500-gitweb-standalone-no-errors.sh
>
> Nice.
>
> > @@ -709,6 +709,14 @@ test_expect_success HIGHLIGHT \
> > git commit -m "Add test.sh" &&
> > gitweb_run "p=.git;a=blob;f=test.sh"'
> >
> > +test_expect_success HIGHLIGHT \
> > + 'syntax highlighting (highlighter language autodetection)' \
> > + 'git config gitweb.highlight yes &&
>
> Modern way would be
>
> + 'test_config gitweb.highlight yes &&
>
> but other tests in this file do not use it.
>
> > + echo "#!/usr/bin/ruby" > test &&
>
> Preferred style would be
>
> + echo "#!/usr/bin/ruby" >test &&
>
> but other tests in this file do not use it.
Agreed, but leaving it as is for consistency.
>
> Sidenote: why Ruby, and not sh / bash, Perl or Python?
Not sh / bash, just to exercise more functionality of highlight by using
a different language than the other test. ruby just was the first thing
to come to mind since I've worked with it recently, but since you made
me think of it, perl is more likely to exist in the builtin config for
longer, and it seems a bit more fitting with gitweb, so its perl in v3.
>
> > + git add test &&
> > + git commit -m "Add test" &&
> > + gitweb_run "p=.git;a=blob;f=test"'
> > +
> > # ----------------------------------------------------------------------
> > # forks of projects
> >
> >
>
> Thank you for your work.
> --
> Jakub Narębski
>
The only changes in v3 are the ones I described here.
Thank you for reviewing this.
--
Ian Kelling
^ permalink raw reply
* Re: [PATCH] run-command: async_exit no longer needs to be public
From: Lars Schneider @ 2016-09-23 8:26 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, Jeff King, GIT Mailing-list
In-Reply-To: <78f2bdd0-f6ad-db5c-f9f2-f90528bc4f77@ramsayjones.plus.com>
> On 22 Sep 2016, at 18:56, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>
>
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
>
> Hi Lars,
>
> If you need to re-roll your 'ls/filter-process' branch, could you please
> squash this into the relevant commit c42a4cbc ("run-command: move check_pipe()
> from write_or_die to run_command", 20-09-2016).
>
> [Note that commit 9658846c ("write_or_die: handle EPIPE in async threads",
> 24-02-2016) introduced async_exit() specifically for use in the implementation
> of check_pipe(). Now that you have moved check_pipe() into run-command.c,
> it no longer needs to be public.]
Hi Ramsay,
thanks for noticing this. I actually hope that I don't need another re-roll :-)
If I don't re-roll. Should I make a patch with this cleanup or do you
take care of it?
Thanks,
Lars
^ permalink raw reply
* Keychain access does not work under macOS Sierra
From: Nicolas Vollmar @ 2016-09-23 7:44 UTC (permalink / raw)
To: git@vger.kernel.org
It seems there were some changes at the keychain in macOS Sierra, after upgrading git seems not to be able to find the client certificate required to connect to our server.
fatal: unable to access 'https://xxx:8443/git/proj.git/': SSL: Can't find the certificate "John Doe" and its private key in the Keychain.
We would appreciate some hints how to address that.
Kind regards,
Nicolas
^ permalink raw reply
* Re: [PATCH 1/2] ls-files: adding support for submodules
From: Jeff King @ 2016-09-23 6:06 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, Brandon Williams, git@vger.kernel.org
In-Reply-To: <CAGZ79kZ+LTd5PuT4+Z9K6d+GQ-33E=tLY1Fokbp22uNoygaEtg@mail.gmail.com>
On Thu, Sep 22, 2016 at 10:47:17PM -0700, Stefan Beller wrote:
> On Thu, Sep 22, 2016 at 8:41 PM, Jeff King <peff@peff.net> wrote:
>
> >> * As Stefan alluded to (much) earlier, it might be a better idea
> >> to have these 'prefix' as the global option to "git" potty, not
> >> to each subcommand that happens to support them;
> >
> > That seems like it would be nice, but there's going to be an interim
> > period where some commands do not respect the global "--prefix" at all
> > (in the worst case, consider a third party command).
>
> My current line of thinking is to have a new flag in command struct in
> git.c to enable the global --prefix, (c.f. RUN_SETUP | NEED_WORK_TREE)
> so we'd have a ALLOW_OUTSIDE_PREFIX flag which can be used to enable
> this feature. In case that flag is not set, but a user tries a
> --prefix=<somewhere>
> we can still
>
> die("nope, we don't do that");
Yeah, a positive "I support this" flag would at least let us correctly
flag errors, which is the best we can do. That won't work for
non-builtins, but perhaps it is good enough in practice.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] ls-files: adding support for submodules
From: Stefan Beller @ 2016-09-23 5:47 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Brandon Williams, git@vger.kernel.org
In-Reply-To: <20160923034113.4rnps3nogvzxkfjx@sigill.intra.peff.net>
On Thu, Sep 22, 2016 at 8:41 PM, Jeff King <peff@peff.net> wrote:
>> * As Stefan alluded to (much) earlier, it might be a better idea
>> to have these 'prefix' as the global option to "git" potty, not
>> to each subcommand that happens to support them;
>
> That seems like it would be nice, but there's going to be an interim
> period where some commands do not respect the global "--prefix" at all
> (in the worst case, consider a third party command).
My current line of thinking is to have a new flag in command struct in
git.c to enable the global --prefix, (c.f. RUN_SETUP | NEED_WORK_TREE)
so we'd have a ALLOW_OUTSIDE_PREFIX flag which can be used to enable
this feature. In case that flag is not set, but a user tries a
--prefix=<somewhere>
we can still
die("nope, we don't do that");
^ permalink raw reply
* Re: Bug? Short command line options
From: Jeff King @ 2016-09-23 4:47 UTC (permalink / raw)
To: Anatoly Borodin; +Cc: git
In-Reply-To: <ns19t4$s0t$1@blaine.gmane.org>
On Thu, Sep 22, 2016 at 07:03:00PM +0000, Anatoly Borodin wrote:
> is there a good reason why
>
> git fetch -vpnf
>
> works like
>
> git fetch -v -p -n -f
>
> and
>
> git commit -avem msg
>
> works like
>
> git commit -a -v -e -m msg
>
> etc etc, but
>
> git log -wWp
>
> says
>
> fatal: unrecognized argument: -wWp
Yes. The reason is that the arguments to git-log are passed to the
revision.c parser, which predates our parse_options() infrastructure,
and does not understand bundled options.
It could be updated to use parse_options(), but nobody has done so yet.
-Peff
^ permalink raw reply
* Re: error
From: Jeff King @ 2016-09-23 4:46 UTC (permalink / raw)
To: Luciano Schillagi; +Cc: git
In-Reply-To: <15EA3A56-BAE8-4FAF-B277-9628307899AF@gmail.com>
On Thu, Sep 22, 2016 at 08:02:35PM -0300, Luciano Schillagi wrote:
> please, what should I do to fix this error? thanks
>
> Luko ~ $ git init
> error: malformed value for push.default: aguas
> error: Must be one of nothing, matching, simple, upstream or current.
> fatal: bad config variable 'push.default' in file '/Users/imac/.gitconfig' at line 16
> -bash: __git_ps1: command not found
Your config file has a bogus value in it. Try:
git config --global --unset push.default
Or you may want to simply edit /Users/imac/.gitconfig by hand. It is not
clear where the bogus value came from, but possibly the file is
corrupted in some way (so you are better off examining it first before
asking git to blindly change it).
-Peff
^ permalink raw reply
* [PATCH] ident: handle NULL ai_canonname
From: Jeff King @ 2016-09-23 4:37 UTC (permalink / raw)
To: Stefan Beller
Cc: Junio C Hamano, Jonas Thiel, John Keeping, Heiko Voigt,
git@vger.kernel.org
In-Reply-To: <20160923040730.76stbefz2ivyfy45@sigill.intra.peff.net>
On Fri, Sep 23, 2016 at 12:07:30AM -0400, Jeff King wrote:
> I have access to an OS X system, but if I understand the bug correctly,
> reproducing it may involve re-setting the system hostname, which is not
> something I'll be able to do. But I'll give it a shot.
Actually, it turned out to be pretty simple to reproduce (after reading
3e8a00a that John found, anyway; hooray for detailed commit messages).
We just have to fake the output of gethostname(), but that is easily
done since we can modify git's source. :)
So I was able to reproduce the bug, and indeed, the patch I posted fixes
it. Here it is with a commit message.
Jonas, I'd be curious to know what the output of "hostname" is on your
system.
-- >8 --
Subject: [PATCH] ident: handle NULL ai_canonname
We call getaddrinfo() to try to convert a short hostname
into a fully-qualified one (to use it as an email domain).
If there isn't a canonical name, getaddrinfo() will
generally return either a NULL addrinfo list, or one in
which ai->ai_canonname is a copy of the original name.
However, if the result of gethostname() looks like an IP
address, then getaddrinfo() behaves differently on some
systems. On OS X, it will return a "struct addrinfo" with a
NULL ai_canonname, and we segfault feeding it to strchr().
This is hard to test reliably because it involves not only a
system where we we have to fallback to gethostname() to come
up with an ident, but also where the hostname is a number
with no dots. But I was able to replicate the bug by faking
a hostname, like:
diff --git a/ident.c b/ident.c
index e20a772..b790d28 100644
--- a/ident.c
+++ b/ident.c
@@ -128,6 +128,7 @@ static void add_domainname(struct strbuf *out, int *is_bogus)
*is_bogus = 1;
return;
}
+ xsnprintf(buf, sizeof(buf), "1");
if (strchr(buf, '.'))
strbuf_addstr(out, buf);
else if (canonical_name(buf, out) < 0) {
and running "git var GIT_AUTHOR_IDENT" on an OS X system.
Before this patch it segfaults, and after we correctly
complain of the bogus "user@1.(none)" address (though this
bogus address would be suitable for non-object uses like
writing reflogs).
Reported-by: Jonas Thiel <jonas.lierschied@gmx.de>
Diagnosed-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Jeff King <peff@peff.net>
---
ident.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ident.c b/ident.c
index e20a772..d17b5bd 100644
--- a/ident.c
+++ b/ident.c
@@ -101,7 +101,7 @@ static int canonical_name(const char *host, struct strbuf *out)
memset (&hints, '\0', sizeof (hints));
hints.ai_flags = AI_CANONNAME;
if (!getaddrinfo(host, NULL, &hints, &ai)) {
- if (ai && strchr(ai->ai_canonname, '.')) {
+ if (ai && ai->ai_canonname && strchr(ai->ai_canonname, '.')) {
strbuf_addstr(out, ai->ai_canonname);
status = 0;
}
--
2.10.0.482.gae5a597
^ permalink raw reply related
* Re: [PATCH v2 2/2] mailinfo: unescape quoted-pair in header fields
From: Jeff King @ 2016-09-23 4:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kevin Daudt, git, Swift Geek
In-Reply-To: <xmqq60pn37gs.fsf@gitster.mtv.corp.google.com>
On Thu, Sep 22, 2016 at 03:17:23PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Mon, Sep 19, 2016 at 08:54:40PM +0200, Kevin Daudt wrote:
> >
> >> + ...
> >> + while ((c = *in++) != 0) {
> >> + if (take_next_literally) {
> >> + take_next_literally = 0;
> >> + } else {
> >> [...]
> >> + }
> >> +
> >> + strbuf_addch(line, c);
> >> + }
> >> +}
> >
> > It needs to `free(in)` at the end of the function.
>
> Ehh, in has been incremented and is pointing at the terminating NUL
> there, so it would be more like
>
> char *to_free, *in;
>
> to_free = strbuf_detach(line, NULL);
> in = to_free;
> ...
> while ((c = *in++)) {
> ...
> }
> free(to_free);
>
> I would think ;-).
Oops, yes. It is beginning to make the "strbuf_swap()" look less
convoluted. :)
-Peff
^ permalink raw reply
* Re: Re: Re: Homebrew and Git
From: Jeff King @ 2016-09-23 4:07 UTC (permalink / raw)
To: Stefan Beller; +Cc: Jonas Thiel, John Keeping, Heiko Voigt, git@vger.kernel.org
In-Reply-To: <CAGZ79kYUcyCrjPfqWtyWE-kaqbQZPG1qosc3qJrDfHixivyFew@mail.gmail.com>
On Thu, Sep 22, 2016 at 08:57:45AM -0700, Stefan Beller wrote:
> On Thu, Sep 22, 2016 at 2:23 AM, Jonas Thiel <jonas.lierschied@gmx.de> wrote:
> > Sorry for my late reply. Thanks for your support -- I really appreciate that.
> >
> > @Jeff: Unfortunately, I do not know how to implement the patch you provided. Can you explain how to do that?
>
> I think this should do:
>
> git clone https://github.com/git/git
> cd git
> # get the email
> wget http://public-inbox.org/git/20160921084841.phq7cfbagi5k7ku4@sigill.intra.peff.net/raw
> # apply patch:
> git am raw
I publish my topic branches via git, too, so it may be easier to replace
these steps with:
git clone -b jk/ident-canonname https://github.com/peff/git
cd git
> make
> make install
After installing, you probably want to do:
PATH=/path/to/git/bin-wrappers:$PATH
to run the new git (and make sure that any git sub-commands that are run
come from the built version, not the rest of the system).
I have access to an OS X system, but if I understand the bug correctly,
reproducing it may involve re-setting the system hostname, which is not
something I'll be able to do. But I'll give it a shot.
-Peff
^ permalink raw reply
* Re: [PATCH 3/3] docs/cvs-migration: mention cvsimport caveats
From: Jeff King @ 2016-09-23 3:56 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20160922131526.GB6320@thyrsus.com>
On Thu, Sep 22, 2016 at 09:15:26AM -0400, Eric S. Raymond wrote:
> Jeff King <peff@peff.net>:
> > Back when this guide was written, cvsimport was the only
> > game in town. These days it is probably not the best option.
>
> It is absolutely not. As I have tried to point out here before, it
> is *severely* broken in its processing of branchy CVS repositories.
>
> Nobody wanted to hear that, but it's still true. Recommending it
> is irresponsible.
I think your points came across, and that is why we have the big warning
in git-cvsimport in the first place. This is really just adding a
pointer to that warning from another relevant location (that frankly, I
didn't even know existed until fixing a nearby problem).
I _do_ think cvsimport, buggy as it may be, may still have some
potential value over other solutions (if you have a simple history, and
it is easier to install or run than the alternatives). But I converted
all of my CVS history to git over ten years ago and have never looked
back. I really don't know if that is the case or not.
So personally I have no objection if somebody wants to rewrite the
gitcvs-migration page to discuss the other options more thoroughly, or
warn more clearly about cvsimport's flaws. These patches were just
"Jeez, we are not even warning people _at all_, so at the minimum we
should do so". I am not qualified to write on the current state of
the art in CVS importing.
-Peff
^ permalink raw reply
* Re: Bug: pager.<cmd> doesn't work well with editors
From: Jeff King @ 2016-09-23 3:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Anatoly Borodin, git
In-Reply-To: <xmqqfuor6ee3.fsf@gitster.mtv.corp.google.com>
On Thu, Sep 22, 2016 at 10:19:32AM -0700, Junio C Hamano wrote:
> The level at which configurability happens might be one issue
> (i.e. you may want different pager for two operating modes for the
> same command, hence your need to use "tag.list" not just "tag"), but
> I think another issue is that it conflates if the output need to be
> paged (on/off) and what pager should be used when the output is
> paged. When we see that a user sets "pager.tag", we should not have
> made it an instruction to Git that _all_ output from "git tag" must
> be paged.
Yes, we could have done it the other way, but I think this was a natural
consequence of implementing it git.c. It _only_ knows about "all output
from git-tag" and nothing else.
At any rate, I do not see much point in moving away from it even if we
change the underlying implementation to be more flexible, if only
because it would be a gratuitous incompatibility.
> So I think we are fundamentally on the same page; it is just you are
> aiming higher than I was, but we both recognize the need for separate
> codepaths in a single command to decide if the output should be paged.
Yeah. In my examples there are really two proposed improvements:
1. The decision over whether and when to start a pager is pushed down
from git.c into individual commands.
2. As a side effect of (1), commands must declare "this is who I am"
to look up the correct config. But "who I am" no longer needs to
be a whole command, so we are free to slice up the namespace more
finely. But we do not have to.
This might also be an opportunity to add more conditions. Like "run
the pager if I am doing log output with -p, but not otherwise" or
something. I dunno. That does not sound useful to me, but maybe
somebody else would find it so.
And I think you are getting at a (3), which is something like:
3. The config namespace can be made richer, so that "whether" and
"how" are split. E.g., "pager.log.command" and "pager.log.enabled"
or something.
I do not mind that, but we would probably want to keep "pager.log"
for compatibility, at which point I wonder if the new system is
worth the bother.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] ls-files: adding support for submodules
From: Jeff King @ 2016-09-23 3:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brandon Williams, git
In-Reply-To: <xmqq7fa36bwm.fsf@gitster.mtv.corp.google.com>
On Thu, Sep 22, 2016 at 11:13:13AM -0700, Junio C Hamano wrote:
> In any case, I would strongly recommend against exposing this (or
> anything for that matter) "--prefix" to the end-user, especially
> because this feature is likely to be applicable to many subcommands,
> and some subcommands would want different sort of prefixing made to
> different things.
Fair enough. I was thinking that this was similar to other options like
"read-tree --prefix" or "archive --prefix". But if nobody really wants
it for anything non-internal, then certainly keeping it as an internal
feature is an easy way to avoid being stuck with a bad public interface
in the long term.
> What we internally call "prefix" and "--submodule-prefix" is closely
> related in that they both interact with pathspecs.
Yeah, I didn't think about pathspecs at all (since they are totally
disabled in patch 1, and I hadn't really read through patch 2 carefully
yet).
> * As Stefan alluded to (much) earlier, it might be a better idea
> to have these 'prefix' as the global option to "git" potty, not
> to each subcommand that happens to support them;
That seems like it would be nice, but there's going to be an interim
period where some commands do not respect the global "--prefix" at all
(in the worst case, consider a third party command).
> * It is unclear how this should interact with commands that are run
> in a subdirectory of the working tree. E.g. what should the
> prefix and the pathspec look like if the command in the above
> example is started in w/git.git/Documentation subdirectory, i.e.
>
> $ cd ~
> $ git -C w/git.git/Documentation ls-files \
> --submodule-prefix=??????? -- '???????' |
> xargs ls -1 -l
>
> Should we error out if we are not at the top of the working tree
> when --submodule-prefix is given?
Without thinking too hard on it, it seems like the submodule prefix
just needs to come after the normal "prefix" that we add when moving to
the top-level of a tree. So:
cd foo
git ls-files --submodule-prefix=bar
should show "foo/bar". Or another way of thinking about it is that the
submodule prefix is always relative to the current directory. Recursion
into submodule would always happen at their top-level, and so would do
the right thing.
But again, that's without thinking hard on it. There may be some corner
cases.
-Peff
^ permalink raw reply
* Re: [PATCH] clone: pass --progress decision to recursive submodules
From: Jeff King @ 2016-09-23 3:31 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kbTi+5oo325v+v8GB7isKRfqys6WRGUARHiMvieDz+png@mail.gmail.com>
On Thu, Sep 22, 2016 at 08:36:01AM -0700, Stefan Beller wrote:
> > Signed-off-by: Jeff King <peff@peff.net>
>
> Acked and thanked by Stefan ;)
Thanks.
> > + /*
> > + * We want to show progress for recursive submodule clones iff
> > + * we did so for the main clone. But only the transport knows
> > + * the final decision for this flag, so we need to rescue the value
> > + * before we free the transport.
> > + */
> > + submodule_progress = transport->progress;
> > +
>
> Good point! I was aware of this bug (but I did not consider it to be
> impactful or as you put it "much worse"), but I anticipated we would
> need some refactoring
To be fair, "much worse" is relative; it's _just_ a disabled progress
bar. :)
I do think this counts as a real regression, though. The "do not pass
down explicit --progress" bug was something probably nobody ever cared
about. But losing progress with the default settings is a thing people
might actually notice.
> of the transport code, e.g. have the decision via isatty(2) as a
> separate outside function that we consult before we even setup the
> transport and then pass it down to the submodules as well. This seems
> to solve this bug elegantly.
Yeah, I considered passing a "no really, stderr is a tty" environment
variable down (which would have avoided all of the boilerplate
propagation of --progress through the various helpers). But when I
realized that we do not handle explicit "--progress" either, the
correct solution became more obvious.
And hopefully all that propagation boilerplate will eventually go away
(or at least be simplified) as the submodule code consolidates in C.
> > static int clone_submodule(const char *path, const char *gitdir, const char *url,
> > - const char *depth, struct string_list *reference, int quiet)
> > + const char *depth, struct string_list *reference,
> > + int quiet, int progress)
>
> I am not sure if having both quiet and progress is maintainable well,
> but it get's the job done here, specifically if we consider this patch a bug
> fix that we'd want to merge down to maint.
Yeah, I had a similar thought that we might need to combine these, or
possibly that we could even drop "quite". But I tried to err on the side
of making the minimal change, as this isn't really a code path I'm very
familiar with.
-Peff
^ permalink raw reply
* our company's open source award for Git
From: Frank Graffagnino @ 2016-09-23 1:01 UTC (permalink / raw)
To: git
Not sure if this is the appropriate mailing list or not. If not, I apologize.
Our company (METECS) decided this year to try and give back to the
open source communities that help us do our work. We had a vote for
our 2016 METECS Open Source Award and Git came in 2nd place! So we
will be making a donation to Git at the SFC for $300 soon. We hope
that you all put it to good use.
Other winners this year were Blender and the Free Software Foundation.
Thanks to all of the people who put in work to make Git such a great
tool. We appreciate you.
Frank Graffagnino
METECS
^ permalink raw reply
* error
From: Luciano Schillagi @ 2016-09-22 23:02 UTC (permalink / raw)
To: git
Hi,
please, what should I do to fix this error? thanks
Luko ~ $ git init
error: malformed value for push.default: aguas
error: Must be one of nothing, matching, simple, upstream or current.
fatal: bad config variable 'push.default' in file '/Users/imac/.gitconfig' at line 16
-bash: __git_ps1: command not found
^ permalink raw reply
* Re: [PATCH v2] gitweb: use highlight's shebang detection
From: Jakub Narębski @ 2016-09-22 22:50 UTC (permalink / raw)
To: Ian Kelling, git
In-Reply-To: <20160921221856.27830-1-ian@iankelling.org>
W dniu 22.09.2016 o 00:18, Ian Kelling napisał:
> The highlight binary can detect language by shebang when we can't tell
> the syntax type by the name of the file. In that case, pass the blob
> to "highlight --force" and the resulting html will have markup for
> highlighting if the language was detected.
This description feels a bit convoluted. Perhaps something like this:
The "highlight" binary can, in some cases, determine the language type
by the means of file contents, for example the shebang in the first line
for some scripting languages. Make use of this autodetection for files
which syntax is not known by gitweb. In that case, pass the blob
contents to "highlight --force"; the parameter is needed to make it
always generate HTML output (which includes HTML-escaping).
Also, we might want to have the information about performance of this
solution either in the commit message, or in commit comments.
>
> Document the feature and improve syntax highlight documentation, add
> test to ensure gitweb doesn't crash when language detection is used,
All right.
> and remove an unused parameter from gitweb_check_feature().
First, that is guess_file_syntax(), not gitweb_check_feature().
Second, this change could be made into independent patch, for example
preparatory one.
>
> Signed-off-by: Ian Kelling <ian@iankelling.org>
> ---
> Documentation/gitweb.conf.txt | 21 ++++++++++++++-------
> gitweb/gitweb.perl | 14 +++++++-------
> t/t9500-gitweb-standalone-no-errors.sh | 8 ++++++++
> 3 files changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
> index a79e350..e632089 100644
> --- a/Documentation/gitweb.conf.txt
> +++ b/Documentation/gitweb.conf.txt
> @@ -246,13 +246,20 @@ $highlight_bin::
> Note that 'highlight' feature must be set for gitweb to actually
> use syntax highlighting.
> +
> -*NOTE*: if you want to add support for new file type (supported by
> -"highlight" but not used by gitweb), you need to modify `%highlight_ext`
> -or `%highlight_basename`, depending on whether you detect type of file
> -based on extension (for example "sh") or on its basename (for example
> -"Makefile"). The keys of these hashes are extension and basename,
> -respectively, and value for given key is name of syntax to be passed via
> -`--syntax <syntax>` to highlighter.
> +*NOTE*: for a file to be highlighted, its syntax type must be detected
> +and that syntax must be supported by "highlight". The default syntax
> +detection is minimal, and there are many supported syntax types with no
> +detection by default. There are three options for adding syntax
> +detection. The first and second priority are `%highlight_basename` and
> +`%highlight_ext`, which detect based on basename (the full filename, for
> +example "Makefile") and extension (for example "sh"). The keys of these
> +hashes are the basename and extension, respectively, and the value for a
> +given key is the name of the syntax to be passed via `--syntax <syntax>`
> +to "highlight". The last priority is the "highlight" configuration of
> +`Shebang` regular expressions to detect the language based on the first
> +line in the file, (for example, matching the line "#!/bin/bash"). See
> +the highlight documentation and the default config at
> +/etc/highlight/filetypes.conf for more details.
> +
I think the rewrite is a bit more readable.
> For example if repositories you are hosting use "phtml" extension for
> PHP files, and you want to have correct syntax-highlighting for those
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 33d701d..44094f4 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3913,7 +3913,7 @@ sub blob_contenttype {
> # guess file syntax for syntax highlighting; return undef if no highlighting
> # the name of syntax can (in the future) depend on syntax highlighter used
> sub guess_file_syntax {
> - my ($highlight, $mimetype, $file_name) = @_;
> + my ($highlight, $file_name) = @_;
Right.
> return undef unless ($highlight && defined $file_name);
> my $basename = basename($file_name, '.in');
> return $highlight_basename{$basename}
> @@ -3931,15 +3931,16 @@ sub guess_file_syntax {
> # or return original FD if no highlighting
> sub run_highlighter {
> my ($fd, $highlight, $syntax) = @_;
> - return $fd unless ($highlight && defined $syntax);
> + return $fd unless ($highlight);
Run highlighter if it is defined, even if gitweb doesn't know syntax, right.
>
> close $fd;
> + my $syntax_arg = (defined $syntax) ? "--syntax $syntax" : "--force";
> open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
> quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',
> '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',
> '--', "-fe=$fallback_encoding")." | ".
> quote_command($highlight_bin).
> - " --replace-tabs=8 --fragment --syntax $syntax |"
> + " --replace-tabs=8 --fragment $syntax_arg |"
Use '--force' if syntax is unknown, right.
> or die_error(500, "Couldn't open file or run syntax highlighter");
> return $fd;
> }
> @@ -7062,9 +7063,8 @@ sub git_blob {
> $have_blame &&= ($mimetype =~ m!^text/!);
>
> my $highlight = gitweb_check_feature('highlight');
> - my $syntax = guess_file_syntax($highlight, $mimetype, $file_name);
> - $fd = run_highlighter($fd, $highlight, $syntax)
> - if $syntax;
> + my $syntax = guess_file_syntax($highlight, $file_name);
> + $fd = run_highlighter($fd, $highlight, $syntax);
Remove unused parameter from callsite, *and* run highlighter even if we
don't know syntax.
>
> git_header_html(undef, $expires);
> my $formats_nav = '';
> @@ -7117,7 +7117,7 @@ sub git_blob {
> $line = untabify($line);
> printf qq!<div class="pre"><a id="l%i" href="%s#l%i" class="linenr">%4i</a> %s</div>\n!,
> $nr, esc_attr(href(-replay => 1)), $nr, $nr,
> - $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);
> + $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);
This is a bit of code duplication / sync from run_highlighter(), but
it is not your fault; it was there (and I don't know how to improve it).
> }
> }
> close $fd
> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> index e94b2f1..576db6d 100755
> --- a/t/t9500-gitweb-standalone-no-errors.sh
> +++ b/t/t9500-gitweb-standalone-no-errors.sh
Nice.
> @@ -709,6 +709,14 @@ test_expect_success HIGHLIGHT \
> git commit -m "Add test.sh" &&
> gitweb_run "p=.git;a=blob;f=test.sh"'
>
> +test_expect_success HIGHLIGHT \
> + 'syntax highlighting (highlighter language autodetection)' \
> + 'git config gitweb.highlight yes &&
Modern way would be
+ 'test_config gitweb.highlight yes &&
but other tests in this file do not use it.
> + echo "#!/usr/bin/ruby" > test &&
Preferred style would be
+ echo "#!/usr/bin/ruby" >test &&
but other tests in this file do not use it.
Sidenote: why Ruby, and not sh / bash, Perl or Python?
> + git add test &&
> + git commit -m "Add test" &&
> + gitweb_run "p=.git;a=blob;f=test"'
> +
> # ----------------------------------------------------------------------
> # forks of projects
>
>
Thank you for your work.
--
Jakub Narębski
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox