[PATCH v3 5/5] init: kill git_link variable

2016-09-24 Thread Nguyễn Thái Ngọc Duy 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/init-db.c | 16 
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 8069cd2..37e318b 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -22,7 +22,6 @@
 static int init_is_bare_repository = 0;
 static int init_shared_repository = -1;
 static const char *init_db_template_dir;
-static const char *git_link;
 
 static void copy_templates_1(struct strbuf *path, struct strbuf *template,
 DIR *dir)
@@ -312,7 +311,7 @@ static void create_object_directory(void)
strbuf_release();
 }
 
-static void separate_git_dir(const char *git_dir)
+static void separate_git_dir(const char *git_dir, const char *git_link)
 {
struct stat st;
 
@@ -349,22 +348,15 @@ int init_db(const char *git_dir, const char *real_git_dir,
if (!exist_ok && !stat(real_git_dir, ))
die(_("%s already exists"), real_git_dir);
 
-   /*
-* make sure symlinks are resolved because we'll be
-* moving the target repo later on in separate_git_dir()
-*/
-   git_link = xstrdup(real_path(git_dir));
set_git_dir(real_path(real_git_dir));
+   git_dir = get_git_dir();
+   separate_git_dir(git_dir, original_git_dir);
}
else {
set_git_dir(real_path(git_dir));
-   git_link = NULL;
+   git_dir = get_git_dir();
}
startup_info->have_repository = 1;
-   git_dir = get_git_dir();
-
-   if (git_link)
-   separate_git_dir(git_dir);
 
safe_create_dir(git_dir, 0);
 
-- 
2.8.2.524.g6ff3d78

[PATCH v3 4/5] init: do not set unnecessary core.worktree

2016-09-24 Thread Nguyễn Thái Ngọc Duy 
The function needs_work_tree_config() that is called from
create_default_files() is supposed to be fed the path to ".git" that
looks as if it is at the top of the working tree, and decide if that
location matches the actual worktree being used.  This comparison allows
"git init" to decide if core.worktree needs to be recorded in the
working tree.

In the current code, however, we feed the return value from
get_git_dir(), which can be totally different from what the function
expects when "gitdir" file is involved.  Instead of giving the path to
the ".git" at the top of the working tree, we end up feeding the actual
path that the file points at.

This original location of ".git" however is only known to init_db().
Make init_db() save it and have it passed to create_default_files() as a
new parameter, which passes the correct location down to
needs_work_tree_config() to fix this.

Noticed-by: Max Nordlund 
Helped-by: Michael J Gruber 
Helped-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/init-db.c | 9 ++---
 t/t0001-init.sh   | 2 ++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 68c1ae3..8069cd2 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -171,7 +171,8 @@ static int needs_work_tree_config(const char *git_dir, 
const char *work_tree)
return 1;
 }
 
-static int create_default_files(const char *template_path)
+static int create_default_files(const char *template_path,
+   const char *original_git_dir)
 {
struct stat st1;
struct strbuf buf = STRBUF_INIT;
@@ -263,7 +264,7 @@ static int create_default_files(const char *template_path)
/* allow template config file to override the default */
if (log_all_ref_updates == -1)
git_config_set("core.logallrefupdates", "true");
-   if (needs_work_tree_config(get_git_dir(), work_tree))
+   if (needs_work_tree_config(original_git_dir, work_tree))
git_config_set("core.worktree", work_tree);
}
 
@@ -337,6 +338,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
 {
int reinit;
int exist_ok = flags & INIT_DB_EXIST_OK;
+   char *original_git_dir = xstrdup(real_path(git_dir));
 
if (real_git_dir) {
struct stat st;
@@ -375,7 +377,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
 */
check_repository_format();
 
-   reinit = create_default_files(template_dir);
+   reinit = create_default_files(template_dir, original_git_dir);
 
create_object_directory();
 
@@ -412,6 +414,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
   git_dir, len && git_dir[len-1] != '/' ? "/" : "");
}
 
+   free(original_git_dir);
return 0;
 }
 
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 488564b..b8fc588 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -400,9 +400,11 @@ test_expect_success 're-init from a linked worktree' '
test_commit first &&
git worktree add ../linked-worktree &&
mv .git/info/exclude expected-exclude &&
+   cp .git/config expected-config &&
find .git/worktrees -print | sort >expected &&
git -C ../linked-worktree init &&
test_cmp expected-exclude .git/info/exclude &&
+   test_cmp expected-config .git/config &&
find .git/worktrees -print | sort >actual &&
test_cmp expected actual
)
-- 
2.8.2.524.g6ff3d78

[PATCH v3 1/5] init: correct re-initialization from a linked worktree

2016-09-24 Thread Nguyễn Thái Ngọc Duy 
When 'git init' is called from a linked worktree, we treat '.git'
dir (which is $GIT_COMMON_DIR/worktrees/something) as the main
'.git' (i.e. $GIT_COMMON_DIR) and populate the whole repository skeleton
in there. It does not harm anything (*) but it is still wrong.

Since 'git init' calls set_git_dir() at preparation time, which
indirectly calls get_common_dir() and correctly detects multiple
worktree setup, all git_path_buf() calls in create_default_files() will
return correct paths in both single and multiple worktree setups. The
only thing left is copy_templates(), which targets $GIT_DIR, not
$GIT_COMMON_DIR.

Fix that with get_git_common_dir(). This function will return $GIT_DIR
in single-worktree setup, so we don't have to make a special case for
multiple-worktree here.

(*) It does in fact, thanks to another bug. More on that later.

Noticed-by: Max Nordlund 
Helped-by: Michael J Gruber 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/init-db.c |  2 +-
 t/t0001-init.sh   | 15 +++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index cc09fca..d5d7558 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -138,7 +138,7 @@ static void copy_templates(const char *template_dir)
goto close_free_return;
}
 
-   strbuf_addstr(, get_git_dir());
+   strbuf_addstr(, get_git_common_dir());
strbuf_complete(, '/');
copy_templates_1(, _path, dir);
 close_free_return:
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 8ffbbea..488564b 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -393,4 +393,19 @@ test_expect_success 'remote init from does not use config 
from cwd' '
test_cmp expect actual
 '
 
+test_expect_success 're-init from a linked worktree' '
+   git init main-worktree &&
+   (
+   cd main-worktree &&
+   test_commit first &&
+   git worktree add ../linked-worktree &&
+   mv .git/info/exclude expected-exclude &&
+   find .git/worktrees -print | sort >expected &&
+   git -C ../linked-worktree init &&
+   test_cmp expected-exclude .git/info/exclude &&
+   find .git/worktrees -print | sort >actual &&
+   test_cmp expected actual
+   )
+'
+
 test_done
-- 
2.8.2.524.g6ff3d78

[PATCH v3 2/5] init: call set_git_dir_init() from within init_db()

2016-09-24 Thread Nguyễn Thái Ngọc Duy 
The next commit requires that set_git_dir_init() must be called before
init_db(). Let's make sure nobody can do otherwise.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 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(_config);
 
git_config(git_default_config, NULL);
diff --git a/builtin/init-db.c b/builtin/init-db.c
index d5d7558..5cb05d9 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -311,8 +311,9 @@ static void create_object_directory(void)
strbuf_release();
 }
 
-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)
 {
if (real_git_dir) {
struct stat st;
@@ -359,10 +360,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);
@@ -582,7 +587,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

[PATCH v3 3/5] init: kill set_git_dir_init()

2016-09-24 Thread Nguyễn Thái Ngọc Duy 
This is a pure code move, necessary to kill the global variable git_link
later (and also helps a bit in the next patch).

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/init-db.c | 50 +-
 1 file changed, 21 insertions(+), 29 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 5cb05d9..68c1ae3 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -311,34 +311,6 @@ static void create_object_directory(void)
strbuf_release();
 }
 
-static int set_git_dir_init(const char *git_dir,
-   const char *real_git_dir,
-   int exist_ok)
-{
-   if (real_git_dir) {
-   struct stat st;
-
-   if (!exist_ok && !stat(git_dir, ))
-   die(_("%s already exists"), git_dir);
-
-   if (!exist_ok && !stat(real_git_dir, ))
-   die(_("%s already exists"), real_git_dir);
-
-   /*
-* make sure symlinks are resolved because we'll be
-* moving the target repo later on in separate_git_dir()
-*/
-   git_link = xstrdup(real_path(git_dir));
-   set_git_dir(real_path(real_git_dir));
-   }
-   else {
-   set_git_dir(real_path(git_dir));
-   git_link = NULL;
-   }
-   startup_info->have_repository = 1;
-   return 0;
-}
-
 static void separate_git_dir(const char *git_dir)
 {
struct stat st;
@@ -364,9 +336,29 @@ int init_db(const char *git_dir, const char *real_git_dir,
const char *template_dir, unsigned int flags)
 {
int reinit;
+   int exist_ok = flags & INIT_DB_EXIST_OK;
 
-   set_git_dir_init(git_dir, real_git_dir, flags & INIT_DB_EXIST_OK);
+   if (real_git_dir) {
+   struct stat st;
 
+   if (!exist_ok && !stat(git_dir, ))
+   die(_("%s already exists"), git_dir);
+
+   if (!exist_ok && !stat(real_git_dir, ))
+   die(_("%s already exists"), real_git_dir);
+
+   /*
+* make sure symlinks are resolved because we'll be
+* moving the target repo later on in separate_git_dir()
+*/
+   git_link = xstrdup(real_path(git_dir));
+   set_git_dir(real_path(real_git_dir));
+   }
+   else {
+   set_git_dir(real_path(git_dir));
+   git_link = NULL;
+   }
+   startup_info->have_repository = 1;
git_dir = get_git_dir();
 
if (git_link)
-- 
2.8.2.524.g6ff3d78

Re: [PATCH v2 4/3] init: combine set_git_dir_init() and init_db() into one

2016-09-24 Thread Duy Nguyen 
On Sat, Sep 24, 2016 at 11:55:33AM -0700, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> > I think this 4/3 is not quite enough to fix the damage to the code
> > caused by 2/3.
> > ...
> > after 4/3 is applied, we should be able to remove the global
> > variable 2/3 introduced, make init_db() receive that information as
> > the return value of set_git_dir_init(), and pass that as a parameter
> > to create_default_files().
> 
> That would look something like this squashed into 4/3, I think.  I
> am not sure if a commit that squashes 2/3, 3/3, 4/3 and this update
> together is harder to understand than keeping 2/3, 3/3 and a fixed
> 4/3 separate, though.  The end result looks much better structured
> than before 2/3 is applied to my quick scan-through of the code.
> ...

How about this?

  [1/5] init: correct re-initialization from a linked worktree
  [2/5] init: call set_git_dir_init() from within init_db()
  [3/5] init: kill set_git_dir_init()
  [4/5] init: do not set unnecessary core.worktree
  [5/5] init: kill git_link variable

I went a bit further, merging set_git_dir_init() back to init_db() so
I can kill "git_link" variable cleanly in 5/5. 3/5 does make init_db()
a bit longer, but not to the alarming level yet. By 4/5,
set_git_dir_init() is gone, so init_db() just needs to save and pass
original_git_dir down to needs_work_tree_config().
--
Duy

Re: [PATCH v8 05/11] pkt-line: add packet_flush_gently()

2016-09-24 Thread Jakub Narębski 
W dniu 20.09.2016 o 21:02, larsxschnei...@gmail.com pisze:
> From: Lars Schneider 
> 
> packet_flush() would die in case of a write error even though for some
> callers an error would be acceptable. Add packet_flush_gently() which
> writes a pkt-line flush packet like packet_flush() but does not die in
> case of an error. The function is used in a subsequent patch.
> 
> Signed-off-by: Lars Schneider 

Looks good.

I guess the difference in treatment from packet_write_fmt_gently() in
the previous patch is that there isn't anything to extract to form
a common code function... I have skipped a few iterations of this series.

> ---
>  pkt-line.c | 8 
>  pkt-line.h | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/pkt-line.c b/pkt-line.c
> index 3b465fd..19f0271 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -91,6 +91,14 @@ void packet_flush(int fd)
>   write_or_die(fd, "", 4);
>  }
>  
> +int packet_flush_gently(int fd)
> +{
> + packet_trace("", 4, 1);
> + if (write_in_full(fd, "", 4) == 4)
> + return 0;
> + return error("flush packet write failed");
> +}
> +
>  void packet_buf_flush(struct strbuf *buf)
>  {
>   packet_trace("", 4, 1);
> diff --git a/pkt-line.h b/pkt-line.h
> index 3caea77..3fa0899 100644
> --- a/pkt-line.h
> +++ b/pkt-line.h
> @@ -23,6 +23,7 @@ void packet_flush(int fd);
>  void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format 
> (printf, 2, 3)));
>  void packet_buf_flush(struct strbuf *buf);
>  void packet_buf_write(struct strbuf *buf, const char *fmt, ...) 
> __attribute__((format (printf, 2, 3)));
> +int packet_flush_gently(int fd);
>  int packet_write_fmt_gently(int fd, const char *fmt, ...) 
> __attribute__((format (printf, 2, 3)));
>  
>  /*
>

Re: [PATCH v3 2/2] gitweb: use highlight's shebang detection

2016-09-24 Thread Ian Kelling 
On Sat, Sep 24, 2016, at 09:21 AM, Jakub Narębski wrote:
> W dniu 24.09.2016 o 00:15, Jakub Narębski pisze:
> 
> Sidenote: this way of benchmarking of gitweb falls between two ways of
> doing a benchmark.
> 
> The first method is to simply run gitweb as a standalone script, passing
> its parameters in CGI environment variables; just like the test suite
> does it.  You would 'time' / 'times' it a few times, drop outliers, and
> take average or a median.  With this method you don't even need to set
> up a web server.
> 
> The second is to use a specialized program to benchmark the server-side
> of a web page, for example 'ab' (ApacheBench), httperf, curl-loader
> or JMeter.  The first one is usually distributed together with Apache
> web server, so you probably have it installed already.  Those tools
> provide timing statistics.

Good to know. Thanks.

Re: [PATCH v3 2/2] gitweb: use highlight's shebang detection

2016-09-24 Thread Ian Kelling 
On Fri, Sep 23, 2016, at 03:15 PM, Jakub Narębski wrote:
> W dniu 23.09.2016 o 11:08, Ian Kelling napisał:
>
> > 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).
>
> Right.
>
> >
> > 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.
>
> This paragraph is a bit unclear, for example it is not obvious what
> "..., which is significantly slower" refers to: sanitize() or esc_html().
>
> I think it would be better to write:
>
>   Although we now run highlight on files which do not end up highlighted,
>   performance is virtually unaffected because when we call highlight, it
>   is used for escaping HTML.  In the case that highlight is used, gitweb
>   calls sanitize() instead of esc_html(), and the latter is significantly
>   slower (it does more, being roughly a superset of sanitize()).

Agree. Done in v4.

>
> >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.
>
> Also, "curling" is not the word I would like to see. I would say:
>
>   Simple benchmark comparing performance of 'blob' view of files without
>   syntax highlighting in gitweb before and after this change indicates
>   ±1% difference in request time for all file types.  Benchmark was
>   performed on local instance on Debian, using Apache/2.4.23 web server
>   and CGI/PSGI/FCGI/mod_perl.
>
>   ^^--- select one
>
> Or something like that; I'm not sure how detailed this should be.
> But it is nice to have such benchmark in the commit message.


Sounds  good. Used it in v4.

>
> Anyway I think that adding yet another configuration toggle for selecting
> whether to use "highlight" syntax autodetection or not would be just an
> unnecessary complication.
>
> Note that the performance loss might be quite higher on MS Windows, with
> its higher cost of fork.  But then they probably do not configure
> server-side highligher anyway.
>
> >
> > Document the feature and improve syntax highlight documentation, add
> > test to ensure gitweb doesn't crash when language detection is used.
>
> Good.
>
> >
> > Signed-off-by: Ian Kelling 
> > ---
> >  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::
>
> We should probably say what does it mean to be "highlight"[1] compatible,
> but it is outside of scope for this patch, and I think also out of scope
> of this series.
>
> > 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 ` 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 `
> > +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

[PATCH v4 2/2] gitweb: use highlight's shebang detection

2016-09-24 Thread Ian Kelling 
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, it
is used for escaping HTML.  In the case that highlight is used, gitweb
calls sanitize() instead of esc_html(), and the latter is significantly
slower (it does more, being roughly a superset of sanitize()).  Simple
benchmark comparing performance of 'blob' view of files without syntax
highlighting in gitweb before and after this change indicates ±1%
difference in request time for all file types.  Benchmark was performed
on local instance on Debian, using Apache/2.4.23 web server and CGI.

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 
---

Notes:
The only change from v3 is the commit message as suggested by Jakub
Narębski

 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 ` 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 `
+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);

[PATCH v4 1/2] gitweb: remove unused guess_file_syntax() parameter

2016-09-24 Thread Ian Kelling 
Signed-off-by: Ian Kelling 
---

Notes:
The only change from v3 is a more descriptive commit message

 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

Re: [PATCH v8 04/11] pkt-line: add packet_write_fmt_gently()

2016-09-24 Thread Jakub Narębski 
W dniu 20.09.2016 o 21:02, larsxschnei...@gmail.com pisze:
> From: Lars Schneider 
> 
> packet_write_fmt() would die in case of a write error even though for
> some callers an error would be acceptable. Add packet_write_fmt_gently()
> which writes a formatted pkt-line like packet_write_fmt() but does not
> die in case of an error. The function is used in a subsequent patch.

Looks good.

> 
> Signed-off-by: Lars Schneider 
> ---
>  pkt-line.c | 34 ++
>  pkt-line.h |  1 +
>  2 files changed, 31 insertions(+), 4 deletions(-)

Re: [PATCH v8 03/11] run-command: move check_pipe() from write_or_die to run_command

2016-09-24 Thread Jakub Narębski 
W dniu 20.09.2016 o 21:02, larsxschnei...@gmail.com pisze:
> From: Lars Schneider 
> 
> Move check_pipe() to run_command and make it public. This is necessary
> to call the function from pkt-line in a subsequent patch.

All right.

> 
> Signed-off-by: Lars Schneider 
> ---
>  run-command.c  | 13 +
>  run-command.h  |  2 ++
>  write_or_die.c | 13 -
>  3 files changed, 15 insertions(+), 13 deletions(-)

Diffstat looks correct.

Not to add to your burden, but perhaps somebody could add to his/her
TODO documenting check_pipe() in Documentation/technical/api-run-command.txt
Or is it not worth it?

Best regards,
-- 
Jakub Narębski

Re: [PATCH v8 02/11] pkt-line: extract set_packet_header()

2016-09-24 Thread Jakub Narębski 
W dniu 20.09.2016 o 21:02, larsxschnei...@gmail.com pisze:

> From: Lars Schneider 
>
> Subject: [PATCH v8 02/11] pkt-line: extract set_packet_header()
> 
> set_packet_header() converts an integer to a 4 byte hex string. Make
> this function locally available so that other pkt-line functions can
> use it.

Ah. I have trouble understanding this commit message, as the
set_packet_header() was not available before this patch, but it
is good if one reads it together with commit summary / title.

Writing

  Extracted set_packet_header() function converts...

or

  New set_packet_header() function converts... 

would make it more clear, but it is all right as it is now.
Perhaps also

  ... could use it.

as currently no other pkt-line function but the one set_packet_header()
was extracted from, namely format_packet(), uses it.

But that is just nitpicking; no need to change on that account.

> 
> Signed-off-by: Lars Schneider

Re: [PATCH v8 01/11] pkt-line: rename packet_write() to packet_write_fmt()

2016-09-24 Thread Jakub Narębski 
Hello Lars,

W dniu 20.09.2016 o 21:02, larsxschnei...@gmail.com pisze:

> From: Lars Schneider 
> 
> packet_write() should be called packet_write_fmt() as the string
> parameter can be formatted.

I would say:

  packet_write() should be called packet_write_fmt() because it
  is printf-like function where first parameter is format string.
  
Or something like that.  But such minor change might be not worth
yet another reroll of this patch series.

Perhaps it would be a good idea to explain the reasoning behind
this change:

  This is important distinction to know from the name if the
  function accepts arbitrary binary data and/or arbitrary
  strings to be written - packet_write[_fmt()] do not.

> 
> Suggested-by: Junio C Hamano 

Just so nobody wonders later why this patch was needed/suggested.

> Signed-off-by: Lars Schneider 
> ---
>  builtin/archive.c|  4 ++--
>  builtin/receive-pack.c   |  4 ++--
>  builtin/remote-ext.c |  4 ++--
>  builtin/upload-archive.c |  4 ++--
>  connect.c|  2 +-
>  daemon.c |  2 +-
>  http-backend.c   |  2 +-
>  pkt-line.c   |  2 +-

The header of the renamed function looks now very nice:

 void packet_write_fmt(int fd, const char *fmt, ...)
   ^^^ ^^^

>  pkt-line.h   |  2 +-
>  shallow.c|  2 +-
>  upload-pack.c| 30 +++---
>  11 files changed, 29 insertions(+), 29 deletions(-)

Diffstat looks correct.  Was the patch generated by doing search
and replace?

Best,
-- 
Jakub Narębski

Re: [PATCH v3] checkout: eliminate unnecessary merge for trivial checkout

2016-09-24 Thread Philip Oakley 

Hi Junio,

From: "Junio C Hamano" 

"Philip Oakley"  writes:


> >"git checkout -b foo" (without -f -m or ) is defined in
> >the manual as being a shortcut for/equivalent to:
> >
> >(1a) "git branch foo"
> >(1b) "git checkout foo"
> >
> >However, it has been our experience in our observed use cases and all
> >the existing git tests, that it can be treated as equivalent to:
> >
> >(2a) "git branch foo"
> >(2b) "git symbolic-ref HEAD refs/heads/foo"
> >...
> >
> I am still not sure if I like the change of what "checkout -b" is this
> late in the game, though.

...
That said, you're much more on the frontline of receiving negative
feedback about doing that than I am. :)  How would you like to
proceed?


I didn't see an initial confirmation as to what the issue really
was. You indicated the symptom ('a long checkout time'), but then we
missed out on hard facts and example repos, so that the issue was
replicable.


I took it as a given, trivial and obvious optimization opportunity,
that it is wasteful having to traverse two trees to consolidate and
reflect their differences into the working tree when we know upfront
that these two trees are identical, no matter what the overhead for
doing so is.


I agree, and I believe Ben agrees.




At the moment there is the simple workaround of an alias that executes
that two step command dance to achieve what you needed, and Junio has
outlined the issues he needed to be covered from his maintainer
perspective (e.g. the detection of sparse checkouts). Confirming the
root causes would help in setting a baseline.

I hope that is of help - I'd seen that the discussion had gone quiet.


Some of the problems I have are:

(1) "git checkout -b NEW", "git checkout", "git checkout HEAD^0"
and "git checkout HEAD" (no other parameters to any of them)
ought to give identical index and working tree.  It is too
confusing to leave subtly different results that will lead to
hard to diagnose bugs for only one of them.

(2) The proposed log message talks only about "performance
optimization",


   while the purpose of the change is more 
about

changing the definition


Here I think is the misunderstanding. His purpose is NOT to change the 
definition (IIUC). As I read the message you reference below (and Ben's 
other messages), I understood that he was trying to achieve what you said 
(i.e. optimise the trivial and obvious opportunity) of selecting for the 
common case (underlying conditions) where the two command sequences are 
identical. If the selected case / conditions is not identical then it is 
defined wrongly...


I suspect that it was Ben's 'soft' explanation that allowed the discussion 
to diverge.



of what "git checkout -b 
NEW" is from

"git branch NEW && git checkout NEW" to "git branch NEW && git
symbolic-ref HEAD refs/heads/NEW".  The explanation in a Ben's
later message <007401d21278$445eba80$cd1c2f80$@gmail.com> does
a much better job contrasting the two.

(3) I identified only one difference as an example sufficient to
point out why the patch provided is not a pure optimization but
behaviour change.  Fixing that example alone to avoid change in
the behaviour is trivial (see if the "info/sparse-checkout"
file is present and refrain from skipping the proper checkout),


This is probably the point Ben needs to take on board to narrow the 
conditions down. There may be others.



but a much larger problem is that I do not know (and Ben does
not, I suspect) know what other behaviour changes the patch is
introducing, and worse, the checks are sufficiently dense too
detailed and intimate to the implementation of unpack_trees()
that it is impossible for anybody to make sure the exceptions
defined in this patch and updates to other parts of the system
will be kept in sync.


I did not believe he was proposing such a change to behaviour, hence his 
difficulty in responding (or at least that is my perception). I.e. he was 
digging a hole in the wrong place.


It is possible that he had accidentally introduced a behavious change, and 
having failed to explictly say "This patch (should) produces no behavious 
change", which then continued to re-inforce the misunderstanding.




So my inclination at this point, unless we see somebody invents a
clever way to solve (3), is that any change that violates (1),
i.e. as long as the patch does "Are we doing '-b NEW'?  Then we do
something subtly different", is not acceptable, and solving (3) in a
maintainable way smells like quite a hard thing to do.  But it would
be ideal if (3) is solved cleanly, as we will then not have to worry
about changing behaviour at all and can apply the optimization for
all of the four cases equally.  As a side effect, that approach
would solve problem (2) above.

If we were to punt

Re: [RFC/PATCH 0/6] Add --format to tag verification

2016-09-24 Thread Jakub Narębski 
W dniu 22.09.2016 o 21:01, Stefan Beller pisze:
> On Thu, Sep 22, 2016 at 11:53 AM,   wrote:
> 
>>
>> P.S. Gmane seems to be broken for git after it was rebooted. Should we ping
>> them about it?
> 
> I think most of the git developers have moved on and reference emails by
> message id. An archive of all messages of the mailing list is found at
> 
> public-inbox.org/git/
> 
> (You can git-clone it to have a distributed copy of the whole archive)
> 
> public-inbox.org/git//
> public-inbox.org/git//raw
> 
> is a good point to link to.
> 
> However, feel free to ping gmane. :)

Two relevant articles are the following:
 [1]: https://lwn.net/Articles/695695/
 [2]: https://lwn.net/Articles/699704/

In short: Gmane creator and maintainer wanted to retire from being
maintainer[1] because of DDoS attack against its web interface, so he
stopped supporting wen interface (NNTP continues working). Thus
public-inbox.org was created (or just advertised more). Later
Gmane got new maintainer[2], but without code for web interface
(I don't know why it is).

So there is hope that Gmane web interface would be up some time in
the future; in meantime one can use public-inbox URLs.

HTH,
-- 
Jakub Narębski

Re: What's cooking in git.git (Sep 2016, #07; Fri, 23)

2016-09-24 Thread Johannes Schindelin 
Hi Junio,

On Fri, 23 Sep 2016, Junio C Hamano wrote:

> A bunch of topics have graduated to 'next', including a few that
> were so far marked as "needs review" or "will hold", as I think
> giving them a greater visibility and guinea pigs would be the most
> efficient way to get feedback from the real world ;-) Some of them
> may be "Meh" topic, which might be why they weren't getting any
> feedback so far, but at least this way we'd know if there are
> breakages in them (in which case we can just revert and discard
> them).

In your previous kitchen status ("What's cooking") you hinted at a
possible v2.10.1 soon. I have a couple of bugfixes lined up for Git for
Windows and would like to avoid unnecessarily frequent release
engineering... Any more concrete ideas on a date for this version?

Also, I found https://tinyurl.com/gitCal very convenient a URL to point
to, do you plan to update that for v2.11.0?

Thanks,
Dscho

Re: [PATCH v2 4/3] init: combine set_git_dir_init() and init_db() into one

2016-09-24 Thread Junio C Hamano 
Junio C Hamano  writes:

> I think this 4/3 is not quite enough to fix the damage to the code
> caused by 2/3.
> ...
> after 4/3 is applied, we should be able to remove the global
> variable 2/3 introduced, make init_db() receive that information as
> the return value of set_git_dir_init(), and pass that as a parameter
> to create_default_files().

That would look something like this squashed into 4/3, I think.  I
am not sure if a commit that squashes 2/3, 3/3, 4/3 and this update
together is harder to understand than keeping 2/3, 3/3 and a fixed
4/3 separate, though.  The end result looks much better structured
than before 2/3 is applied to my quick scan-through of the code.

In any case, the log message of 2/3 needs to be updated to retitle
it, I think.  "do not ... more often than necessary" makes it sound
as if we were doing things that did not make any difference in the
end result, wasting cycles.  But what you actually wanted to achieve
was not to "avoid unnecessary work"--doing so gave a broken
behaviour and that was what you were fixing, "do not record broken
core.worktree", perhaps?

The solution (if we squash 2-4 and the fixup below) is to stop
feeding get_git_dir() to needs_work_tree_config(), because the
parameter to the latter is the path to ".git" that presumably is at
the top of the working tree, and get_git_dir() is not that when
"gitdir" file is involved.  So a rewritten log message may say
something like...

init: do not set unnecessary core.worktree

The function needs_work_tree_config() that is called from
create_default_files() is supposed to be fed the path to ".git"
that looks as if it is at the top of the working tree, and
decide if that location matches the actual worktree being used.
This comparison allows "git init" to decide if core.worktree
needs to be recorded in the working tree.

In the current code, however, we feed the return value from
get_git_dir(), which can be totally different from what the
function expects when "gitdir" file is involved.  Instead of
giving the path to the ".git" at the top of the working tree, we
end up feeding the actual path that the file points at.

This original location of ".git" however is only known to a
helper function set_git_dir_init() that must be called before
init_db() is called (they both have only two callsites, one in
"git init" and the other in "git clone"), and in the current
code, this original location is not visible to its callers.

By doing the following two things:

* Move call to set_git_dir_init() to init_db(), as the two must
  always be called in this order, and adjust its current
  callers.

* Make set_git_dir_init() return the original location of ".git"
  to the caller, which is init_db(), and have it passed to
  create_default_files() as a new parameter.

   pass the correct location down to needs_work_tree_config() to fix
   this.

This suggests that 2/3, 3/3 and fixed 4/3 could be done in two
logical steps.  The first bullet point can be done as a separate
preparatory step, and on top of that, the second bullet point can be
done as a separate "fix".



 builtin/init-db.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index ee7942f..527722c 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -23,7 +23,6 @@ static int init_is_bare_repository = 0;
 static int init_shared_repository = -1;
 static const char *init_db_template_dir;
 static const char *git_link;
-static const char *original_git_dir;
 
 static void copy_templates_1(struct strbuf *path, struct strbuf *template,
 DIR *dir)
@@ -172,7 +171,8 @@ static int needs_work_tree_config(const char *git_dir, 
const char *work_tree)
return 1;
 }
 
-static int create_default_files(const char *template_path)
+static int create_default_files(const char *template_path,
+   const char *original_git_dir)
 {
struct stat st1;
struct strbuf buf = STRBUF_INIT;
@@ -312,11 +312,11 @@ static void create_object_directory(void)
strbuf_release();
 }
 
-static int set_git_dir_init(const char *git_dir,
-   const char *real_git_dir,
-   int exist_ok)
+static char *set_git_dir_init(const char *git_dir,
+ const char *real_git_dir,
+ int exist_ok)
 {
-   original_git_dir = xstrdup(real_path(git_dir));
+   char *original_git_dir = xstrdup(real_path(git_dir));
 
if (real_git_dir) {
struct stat st;
@@ -339,7 +339,7 @@ static int set_git_dir_init(const char *git_dir,
git_link = NULL;
}
startup_info->have_repository = 1;
-   return 0;
+   return original_git_dir;
 }
 
 static void separate_git_dir(const char *git_dir)
@@ -367,9 +367,10 @@

Re: [PATCH v3] checkout: eliminate unnecessary merge for trivial checkout

2016-09-24 Thread Junio C Hamano 
"Philip Oakley"  writes:

>> > >"git checkout -b foo" (without -f -m or ) is defined in
>> > >the manual as being a shortcut for/equivalent to:
>> > >
>> > >(1a) "git branch foo"
>> > >(1b) "git checkout foo"
>> > >
>> > >However, it has been our experience in our observed use cases and all
>> > >the existing git tests, that it can be treated as equivalent to:
>> > >
>> > >(2a) "git branch foo"
>> > >(2b) "git symbolic-ref HEAD refs/heads/foo"
>> > >...
>> > >
>> > I am still not sure if I like the change of what "checkout -b" is this
>> > late in the game, though.
>>
>> ...
>> That said, you're much more on the frontline of receiving negative
>> feedback about doing that than I am. :)  How would you like to
>> proceed?
>
> I didn't see an initial confirmation as to what the issue really
> was. You indicated the symptom ('a long checkout time'), but then we
> missed out on hard facts and example repos, so that the issue was
> replicable.

I took it as a given, trivial and obvious optimization opportunity,
that it is wasteful having to traverse two trees to consolidate and
reflect their differences into the working tree when we know upfront
that these two trees are identical, no matter what the overhead for
doing so is.

> At the moment there is the simple workaround of an alias that executes
> that two step command dance to achieve what you needed, and Junio has
> outlined the issues he needed to be covered from his maintainer
> perspective (e.g. the detection of sparse checkouts). Confirming the
> root causes would help in setting a baseline.
>
> I hope that is of help - I'd seen that the discussion had gone quiet.

Some of the problems I have are:

 (1) "git checkout -b NEW", "git checkout", "git checkout HEAD^0"
 and "git checkout HEAD" (no other parameters to any of them)
 ought to give identical index and working tree.  It is too
 confusing to leave subtly different results that will lead to
 hard to diagnose bugs for only one of them.

 (2) The proposed log message talks only about "performance
 optimization", while the purpose of the change is more about
 changing the definition of what "git checkout -b NEW" is from
 "git branch NEW && git checkout NEW" to "git branch NEW && git
 symbolic-ref HEAD refs/heads/NEW".  The explanation in a Ben's
 later message <007401d21278$445eba80$cd1c2f80$@gmail.com> does
 a much better job contrasting the two.

 (3) I identified only one difference as an example sufficient to
 point out why the patch provided is not a pure optimization but
 behaviour change.  Fixing that example alone to avoid change in
 the behaviour is trivial (see if the "info/sparse-checkout"
 file is present and refrain from skipping the proper checkout),
 but a much larger problem is that I do not know (and Ben does
 not, I suspect) know what other behaviour changes the patch is
 introducing, and worse, the checks are sufficiently dense too
 detailed and intimate to the implementation of unpack_trees()
 that it is impossible for anybody to make sure the exceptions
 defined in this patch and updates to other parts of the system
 will be kept in sync.

So my inclination at this point, unless we see somebody invents a
clever way to solve (3), is that any change that violates (1),
i.e. as long as the patch does "Are we doing '-b NEW'?  Then we do
something subtly different", is not acceptable, and solving (3) in a
maintainable way smells like quite a hard thing to do.  But it would
be ideal if (3) is solved cleanly, as we will then not have to worry
about changing behaviour at all and can apply the optimization for
all of the four cases equally.  As a side effect, that approach
would solve problem (2) above.

If we were to punt on keeping the sanity (1) and introduce a subtly
different "create a new branch and point the HEAD at it", an easier
way out may be be one of

 1. a totally new command, e.g. "git branch-switch NEW" that takes
only a single argument and no other "checkout" options, or

 2. a new option to "git checkout" that takes _ONLY_ a single
argument and incompatible with any other option or command line
argument, or

 3. an alias that does "git branch" followed by "git symbolic-ref".

Neither of the first two sounds palatable, though.

Re: [PATCH] git-gui: stop using deprecated merge syntax

2016-09-24 Thread Johannes Sixt 

Am 24.09.2016 um 13:30 schrieb René Scharfe:

Starting with v2.5.0 git merge can handle FETCH_HEAD internally and
warns when it's called like 'git merge  HEAD ' because
that syntax is deprecated.  Use this feature in git-gui and get rid of
that warning.

Signed-off-by: Rene Scharfe 
---
Tested only _very_ lightly!

 git-gui/lib/merge.tcl | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/git-gui/lib/merge.tcl b/git-gui/lib/merge.tcl
index 460d32f..5ab6f8f 100644
--- a/git-gui/lib/merge.tcl
+++ b/git-gui/lib/merge.tcl
@@ -112,12 +112,7 @@ method _start {} {
close $fh
set _last_merged_branch $branch

-   set cmd [list git]
-   lappend cmd merge
-   lappend cmd --strategy=recursive
-   lappend cmd [git fmt-merge-msg <[gitdir FETCH_HEAD]]
-   lappend cmd HEAD
-   lappend cmd $name
+   set cmd [list git merge --strategy=recursive FETCH_HEAD]

ui_status [mc "Merging %s and %s..." $current_branch $stitle]
set cons [console::new [mc "Merge"] "merge $stitle"]



Much better than my version. I had left fmt-merge-msg and added --no-log 
to treat merge.log config suitably. But this works too, and is much more 
obvious.


Tested-by: Johannes Sixt 

-- Hannes

Re: [PATCH v3 2/2] gitweb: use highlight's shebang detection

2016-09-24 Thread Junio C Hamano 
Jakub Narębski  writes:

>> Also, "curling" is not the word I would like to see. I would say:
>> 
>>   Simple benchmark comparing performance of 'blob' view of files without
>>   syntax highlighting in gitweb before and after this change indicates
>>   ±1% difference in request time for all file types.  Benchmark was
>>   performed on local instance on Debian, using Apache/2.4.23 web server
>>   and CGI/PSGI/FCGI/mod_perl.
>> 
>>   ^^--- select one

or state that all of them produced similar results ;-)

>> Or something like that; I'm not sure how detailed this should be.
>> But it is nice to have such benchmark in the commit message.
>
> Sidenote: this way of benchmarking of gitweb falls between two ways of
> doing a benchmark.

All good comments.  Thanks.

Re: [PATCH v3 2/2] gitweb: use highlight's shebang detection

2016-09-24 Thread Jakub Narębski 
W dniu 24.09.2016 o 00:15, Jakub Narębski pisze:
> W dniu 23.09.2016 o 11:08, Ian Kelling napisał: 

>>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.
> 
> Also, "curling" is not the word I would like to see. I would say:
> 
>   Simple benchmark comparing performance of 'blob' view of files without
>   syntax highlighting in gitweb before and after this change indicates
>   ±1% difference in request time for all file types.  Benchmark was
>   performed on local instance on Debian, using Apache/2.4.23 web server
>   and CGI/PSGI/FCGI/mod_perl.
> 
>   ^^--- select one
> 
> Or something like that; I'm not sure how detailed this should be.
> But it is nice to have such benchmark in the commit message.

Sidenote: this way of benchmarking of gitweb falls between two ways of
doing a benchmark.

The first method is to simply run gitweb as a standalone script, passing
its parameters in CGI environment variables; just like the test suite
does it.  You would 'time' / 'times' it a few times, drop outliers, and
take average or a median.  With this method you don't even need to set
up a web server.

The second is to use a specialized program to benchmark the server-side
of a web page, for example 'ab' (ApacheBench), httperf, curl-loader
or JMeter.  The first one is usually distributed together with Apache
web server, so you probably have it installed already.  Those tools
provide timing statistics.

[...]
> Note that the performance loss might be quite higher on MS Windows, with
> its higher cost of fork.  But then they probably do not configure
> server-side highligher anyway.

Re: [PATCH v3] checkout: eliminate unnecessary merge for trivial checkout

2016-09-24 Thread Philip Oakley 

Ben,
Using a 'bottom / in-line' posting flow is much preferred, which may require 
some manual editing[1], hopefully I have it about right...

Philip
--
[1] this is massaged and mangled Outlook Express, sometimes one has to work 
with the tools at hand...


From: "Ben Peart" 

From: Junio C Hamano [mailto:gits...@pobox.com]
> Junio C Hamano 
> >"git checkout -b foo" (without -f -m or ) is defined in
> >the manual as being a shortcut for/equivalent to:
> >
> >(1a) "git branch foo"
> >(1b) "git checkout foo"
> >
> >However, it has been our experience in our observed use cases and all
> >the existing git tests, that it can be treated as equivalent to:
> >
> >(2a) "git branch foo"
> >(2b) "git symbolic-ref HEAD refs/heads/foo"
> >...
> >
> I am still not sure if I like the change of what "checkout -b" is this
> late in the game, though.
>
> Having said all that.
>
> I do see the merit of having a shorthand way to invoke your 2 above.
> It is just that I am not convinced that it is the best way to achieve 
> that goal to redefine what "git checkout -b " (no other 
> parameters) does.

>
---

I understand the reluctance to change the existing behavior of the "git 
checkout -b " command.


I see this as a tradeoff between taking advantage of the muscle memory for 
the existing command and coming up with a new shortcut command and 
training people to use it instead.


The fact that all the use cases we've observed and all the git test cases 
actually produce the same results but significantly faster with that 
change in behavior made me hope we could redefine the command to take 
advantage of the muscle memory.


That said, you're much more on the frontline of receiving negative 
feedback about doing that than I am. :)  How would you like to proceed?


The discussion can often feel harsh [2], especially if there is accidental 
'talking past each other', which is usually because of differing 
perspectives on the issues.


I didn't see an initial confirmation as to what the issue really was. You 
indicated the symptom ('a long checkout time'), but then we missed out on 
hard facts and example repos, so that the issue was replicable.


Is there an example public repo that you can show the issue on? (or 
anonymise a private one - there is a script for that [3])


Can you give local timings (and indication of the hardware and software 
versions used for the test, and if appropriate, network setup)?


I know at my work that sometime our home drives are multiply mapped to H:, a 
C:/homedrive directory and a $netshare/me network directory via the 
Microsofy roaming profiles, and if there is hard synchronization (or 
whatever term is appropriate) there can be sudden slowdowns as local C: 
writes drop from 'instant' to 'forever'...


Is there anything special about the repos that have the delays? Is it a 
local process issue that causes the repos to develop those symptoms (see 
above about not being sure why you have these issues), in which case it 
could be local self inflicted issues, or it could be that you have a 
regulatory issue for that domain that requires such symptoms, which would 
shift the problem from a 'don't do that' response to a 'hmm, how to cover 
this'.



At the moment there is the simple workaround of an alias that executes that 
two step command dance to achieve what you needed, and Junio has outlined 
the issues he needed to be covered from his maintainer perspective (e.g. the 
detection of sparse checkouts). Confirming the root causes would help in 
setting a baseline.


I hope that is of help - I'd seen that the discussion had gone quiet.

--
Philip


[2] Been there, feel your pain. It's not in any way malicious, just a 
reflection that email can be a poor medium for such discussions.
[3] https://public-inbox.org/git/20140827170127.ga6...@peff.net/ suggest 
that the `git fast-export --anonymize --all` maybe the approach.

[PATCH] git-gui: stop using deprecated merge syntax

2016-09-24 Thread René Scharfe 
Starting with v2.5.0 git merge can handle FETCH_HEAD internally and
warns when it's called like 'git merge  HEAD ' because
that syntax is deprecated.  Use this feature in git-gui and get rid of
that warning.

Signed-off-by: Rene Scharfe 
---
Tested only _very_ lightly!

 git-gui/lib/merge.tcl | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/git-gui/lib/merge.tcl b/git-gui/lib/merge.tcl
index 460d32f..5ab6f8f 100644
--- a/git-gui/lib/merge.tcl
+++ b/git-gui/lib/merge.tcl
@@ -112,12 +112,7 @@ method _start {} {
close $fh
set _last_merged_branch $branch
 
-   set cmd [list git]
-   lappend cmd merge
-   lappend cmd --strategy=recursive
-   lappend cmd [git fmt-merge-msg <[gitdir FETCH_HEAD]]
-   lappend cmd HEAD
-   lappend cmd $name
+   set cmd [list git merge --strategy=recursive FETCH_HEAD]
 
ui_status [mc "Merging %s and %s..." $current_branch $stitle]
set cons [console::new [mc "Merge"] "merge $stitle"]
-- 
2.10.0

Ich werde warten, von Ihnen zu hören.

2016-09-24 Thread Andrew Hau Chan 



--
Lieber Freund,

Wie geht es Ihnen heute? Ich habe eine Investitionsmöglichkeit mit Ihnen 
zu teilen, die die Übertragung einer großen Geldsumme zum gegenseitigen 
Nutzen für beide von uns betreffen.


Mein Name ist Andrew Hau Chung, ich in einem Finanzinstitut arbeiten 
hier in Hong Kong.


Wenn Sie interessiert sind, kontaktieren Sie mich durch meine private
E-Mail-Adresse:(andrewhauf...@gmail.com)

Ihre prompte Antwort wird geschätzt.

Dein,
Andrew Hau Chung