Re: [RFC/PATCH v4 1/3] add high resolution timer function to debug performance issues

[Noel Grandin]

On 2014-05-20 21:11, Karsten Blees wrote:

  * implement Mac OSX version using mach_absolute_time





Note that unlike the Windows and Linux APIs, mach_absolute_time does not do correction for frequency-scaling and 
cross-CPU synchronization with the TSC.


I'm not aware of anything else that you could use on MacOS, so your best bet is probably just to use mach_absolute_time 
and document it's shortcomings.


Regards, Noel.

Disclaimer: http://www.peralex.com/disclaimer.html


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH v4 1/3] add high resolution timer function to debug performance issues

[Karsten Blees]

Am 21.05.2014 09:31, schrieb Noel Grandin:
 On 2014-05-20 21:11, Karsten Blees wrote:
   * implement Mac OSX version using mach_absolute_time


 
 
 Note that unlike the Windows and Linux APIs, mach_absolute_time does not do 
 correction for frequency-scaling

I don't have a MAC so I can't test any of this, but supposedly 
mach_timebase_info() returns the frequency of mach_absolute_time(), so you 
could do similar frequency-scaling as I do for Windows with 
QueryPerformanceFrequency().

 and cross-CPU synchronization with the TSC.
 

The TSC is synchronized across cores and sockets on modern x86 hardware [1] (at 
least since Intel Nehalem, i.e. all Core i[357] processors). On older machines, 
I would expect the OS API to choose a more appropriate time source, e.g. the 
HPET. I'm not proposing to use asm(rdtsc) or anything like that...

[1] 
https://software.intel.com/en-us/articles/best-timing-function-for-measuring-ipp-api-timing

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

As-salam..............

[Majid Al Futtaim]

As-salam Sir, 
 
Regarding to possible investment, I wish to bring to your notice my interest to 
partner with you on any great business investment opportunity. 
 
Please kindly furnish me with any business details and a proposal for a Joint 
venture partnership with you. Looking forward to going into good business 
relationship with you. 

Please provide this detail for me to know you better and proceed. 
My direct email is :   majid_al.futt...@myself.com  or 
majidalfutt...@hotmail.com

 1. Your full names:
 2. Your contact address:
 3. Your profession:
 4. Your telephone mobile and fax number:
 5.Age

6 The Type of business you will like me to invest on and also tell me little 
about yourself, I hope to hear from you soon.
May the peace of Almighty Allah be with you.
Regards,
Majid Al Futtaim..
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] Git v2.0.0-rc4

[Martin Erik Werner]

On Tue, May 20, 2014 at 05:24:50PM -0700, Junio C Hamano wrote:
(...)
  * git reset learned the -N option, which does not reset the index
fully for paths the index knows about but the tree-ish the command
resets to does not (these paths are kept as intend-to-add entries).

I find it quite hard to parse this sentance. Maybe something like:

which keeps paths as intent-to-add entries if they are currently
staged, but not present in the tree-ish being reset to.

would be clearer (I hope I've actually managed to understand it..)?

(...)
  * Commands that take pathspecs on the command line misbehaved when
the pathspec is given as an absolute pathname (which is a
practice not particularly encouraged) that points at a symbolic
link in the working tree.
(merge later 655ee9e mw/symlinks to maint.)

In order to include the latest cleanup to this patchset:
setup: fix windows path buffer over-stepping
this should be 6127ff6 instead. Sorry if it's unneeded to note, but just
wanted to make sure :)

--
Martin Erik Werner martinerikwer...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/9] handle alternate charsets for remote http errors

[Jeff King]

As of commit 426e70d (remote-curl: show server content on http errors,
2013-04-05), we relay any text/plain errors shown by the remote http
server to the user. However, we were lazy back then and left this TODO
in place:

   /*
* We only show text/plain parts, as other types are likely
* to be ugly to look at on the user's terminal.
*
* TODO should handle ; charset=XXX, and re-encode into
* logoutputencoding
*/

This series actually implements that, along with a few other cleanups.

  [1/9]: test-lib: preserve GIT_CURL_VERBOSE from the environment
  [2/9]: strbuf: add strbuf_tolower function
  [3/9]: daemon/config: factor out duplicate xstrdup_tolower
  [4/9]: http: normalize case of returned content-type
  [5/9]: t/lib-httpd: use write_script to copy CGI scripts
  [6/9]: t5550: test display of remote http error messages
  [7/9]: remote-curl: recognize text/plain with a charset parameter
  [8/9]: strbuf: add strbuf_reencode helper
  [9/9]: remote-curl: reencode http error messages

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/9] test-lib: preserve GIT_CURL_VERBOSE from the environment

[Jeff King]

Turning on this variable can be useful when debugging http
tests. It does break a few tests in t5541, but it is not
a variable that the user is likely to have enabled
accidentally.

Signed-off-by: Jeff King p...@peff.net
---
Not necessary for this series, but I found it helpful.

I took a look at making the tests in t5541 work with it, but it's a
little nasty (we have to separate the curl output from the regular
stderr). Unsetting GIT_CURL_VERBOSE for those tests would also be an
option. Or just dropping this patch is OK with me, too; it's really only
for debugging.

 t/test-lib.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index c081668..f7e55b1 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -91,6 +91,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $($PERL_PATH -e '
VALGRIND
UNZIP
PERF_
+   CURL_VERBOSE
));
my @vars = grep(/^GIT_/  !/^GIT_($ok)/o, @env);
print join(\n, @vars);
-- 
2.0.0.rc1.436.g03cb729

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/9] strbuf: add strbuf_tolower function

[Jeff King]

This makes config's lowercase() function public.

Note that we could continue to offer a pure-string
lowercase, but there would be no callers (in most
pure-string cases, we actually duplicate and lowercase the
duplicate).

Signed-off-by: Jeff King p...@peff.net
---
This ones gets used later on...

 Documentation/technical/api-strbuf.txt | 4 
 config.c   | 8 +---
 strbuf.c   | 7 +++
 strbuf.h   | 1 +
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/Documentation/technical/api-strbuf.txt 
b/Documentation/technical/api-strbuf.txt
index 3350d97..8480f89 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -125,6 +125,10 @@ Functions
 
Strip whitespace from the end of a string.
 
+`strbuf_tolower`::
+
+   Lowercase each character in the buffer using `tolower`.
+
 `strbuf_cmp`::
 
Compare two buffers. Returns an integer less than, equal to, or greater
diff --git a/config.c b/config.c
index a30cb5c..03ce5c6 100644
--- a/config.c
+++ b/config.c
@@ -147,12 +147,6 @@ int git_config_include(const char *var, const char *value, 
void *data)
return ret;
 }
 
-static void lowercase(char *p)
-{
-   for (; *p; p++)
-   *p = tolower(*p);
-}
-
 void git_config_push_parameter(const char *text)
 {
struct strbuf env = STRBUF_INIT;
@@ -180,7 +174,7 @@ int git_config_parse_parameter(const char *text,
strbuf_list_free(pair);
return error(bogus config parameter: %s, text);
}
-   lowercase(pair[0]-buf);
+   strbuf_tolower(pair[0]);
if (fn(pair[0]-buf, pair[1] ? pair[1]-buf : NULL, data)  0) {
strbuf_list_free(pair);
return -1;
diff --git a/strbuf.c b/strbuf.c
index ee96dcf..a1b8a47 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -106,6 +106,13 @@ void strbuf_ltrim(struct strbuf *sb)
sb-buf[sb-len] = '\0';
 }
 
+void strbuf_tolower(struct strbuf *sb)
+{
+   size_t i;
+   for (i = 0; i  sb-len; i++)
+   sb-buf[i] = tolower(sb-buf[i]);
+}
+
 struct strbuf **strbuf_split_buf(const char *str, size_t slen,
 int terminator, int max)
 {
diff --git a/strbuf.h b/strbuf.h
index 39c14cf..6b6f745 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -45,6 +45,7 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t 
len)
 extern void strbuf_trim(struct strbuf *);
 extern void strbuf_rtrim(struct strbuf *);
 extern void strbuf_ltrim(struct strbuf *);
+extern void strbuf_tolower(struct strbuf *sb);
 extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);
 
 /*
-- 
2.0.0.rc1.436.g03cb729

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/9] daemon/config: factor out duplicate xstrdup_tolower

[Jeff King]

We have two implementations of the same function; let's drop
that to one. We take the name from daemon.c, but the
implementation (which is just slightly more efficient) from
the config code.

Signed-off-by: Jeff King p...@peff.net
---
Unlike the last patch, this one does _not_ get used elsewhere in this
series. It's just a cleanup I happened to notice while writing the other
one. It could be omitted, or applied separately.

 builtin/config.c | 15 +--
 daemon.c |  8 
 strbuf.c | 13 +
 strbuf.h |  2 ++
 4 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 5677c94..fcd8474 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -395,19 +395,6 @@ static int urlmatch_collect_fn(const char *var, const char 
*value, void *cb)
return 0;
 }
 
-static char *dup_downcase(const char *string)
-{
-   char *result;
-   size_t len, i;
-
-   len = strlen(string);
-   result = xmalloc(len + 1);
-   for (i = 0; i  len; i++)
-   result[i] = tolower(string[i]);
-   result[i] = '\0';
-   return result;
-}
-
 static int get_urlmatch(const char *var, const char *url)
 {
char *section_tail;
@@ -422,7 +409,7 @@ static int get_urlmatch(const char *var, const char *url)
if (!url_normalize(url, config.url))
die(%s, config.url.err);
 
-   config.section = dup_downcase(var);
+   config.section = xstrdup_tolower(var);
section_tail = strchr(config.section, '.');
if (section_tail) {
*section_tail = '\0';
diff --git a/daemon.c b/daemon.c
index eba1255..f9c63e9 100644
--- a/daemon.c
+++ b/daemon.c
@@ -475,14 +475,6 @@ static void make_service_overridable(const char *name, int 
ena)
die(No such service %s, name);
 }
 
-static char *xstrdup_tolower(const char *str)
-{
-   char *p, *dup = xstrdup(str);
-   for (p = dup; *p; p++)
-   *p = tolower(*p);
-   return dup;
-}
-
 static void parse_host_and_port(char *hostport, char **host,
char **port)
 {
diff --git a/strbuf.c b/strbuf.c
index a1b8a47..d289d1a 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -577,3 +577,16 @@ int fprintf_ln(FILE *fp, const char *fmt, ...)
return -1;
return ret + 1;
 }
+
+char *xstrdup_tolower(const char *string)
+{
+   char *result;
+   size_t len, i;
+
+   len = strlen(string);
+   result = xmalloc(len + 1);
+   for (i = 0; i  len; i++)
+   result[i] = tolower(string[i]);
+   result[i] = '\0';
+   return result;
+}
diff --git a/strbuf.h b/strbuf.h
index 6b6f745..25328b9 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -184,4 +184,6 @@ extern int printf_ln(const char *fmt, ...);
 __attribute__((format (printf,2,3)))
 extern int fprintf_ln(FILE *fp, const char *fmt, ...);
 
+char *xstrdup_tolower(const char *);
+
 #endif /* STRBUF_H */
-- 
2.0.0.rc1.436.g03cb729

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/9] http: normalize case of returned content-type

[Jeff King]

The content-type string curl hands us comes straight from
the server, and may have odd capitalization. RFC 2616 states
that content-types are case-insensitive. We already handle
this when checking for text/plain (by using strcasecmp), but
do not when checking for a smart-http content-type.

We could simply convert the latter to use strcasecmp, but as
we add more parsing of the header, having it normalized will
simplify our parsing code.

Note that there is one caveat. RFC 2616 notes that the type
itself is case insensitive, as are parameter names. However,
parameter valuse may be case-sensitive, depending on the
individual parameter. In practice, we are OK, though. We
currently only look at the type itself. In the future we
will start looking at charset parameters, but those are also
case-insensitive. And it doesn't seem likely that we would
look at any other parameters.

Signed-off-by: Jeff King p...@peff.net
---
I think this is fine. If not, we can either:

  1. Use strcasecmp and friends more consistently when
 parsing/comparing (later bits of the series will need to be
 adjusted).

  2. Downcase here in a more context-aware way.

 http.c| 4 +++-
 remote-curl.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/http.c b/http.c
index 94e1afd..cd6c328 100644
--- a/http.c
+++ b/http.c
@@ -957,9 +957,11 @@ static int http_request(const char *url,
 
ret = run_one_slot(slot, results);
 
-   if (options  options-content_type)
+   if (options  options-content_type) {
curlinfo_strbuf(slot-curl, CURLINFO_CONTENT_TYPE,
options-content_type);
+   strbuf_tolower(options-content_type);
+   }
 
if (options  options-effective_url)
curlinfo_strbuf(slot-curl, CURLINFO_EFFECTIVE_URL,
diff --git a/remote-curl.c b/remote-curl.c
index 52c2d96..a5ab977 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -205,7 +205,7 @@ static int show_http_message(struct strbuf *type, struct 
strbuf *msg)
 * TODO should handle ; charset=XXX, and re-encode into
 * logoutputencoding
 */
-   if (strcasecmp(type-buf, text/plain))
+   if (strcmp(type-buf, text/plain))
return -1;
 
strbuf_trim(msg);
-- 
2.0.0.rc1.436.g03cb729

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/9] t5550: test display of remote http error messages

[Jeff King]

Since commit 426e70d (remote-curl: show server content on
http errors, 2013-04-05), we relay any text/plain error
messages from the remote server to the user. However, we
never tested it.

Signed-off-by: Jeff King p...@peff.net
---
 t/lib-httpd.sh |  1 +
 t/lib-httpd/apache.conf|  4 
 t/lib-httpd/error.sh   | 17 +
 t/t5550-http-fetch-dumb.sh | 10 ++
 4 files changed, 32 insertions(+)
 create mode 100755 t/lib-httpd/error.sh

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 8e680ef..f7640be 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -113,6 +113,7 @@ prepare_httpd() {
mkdir -p $HTTPD_DOCUMENT_ROOT_PATH
cp $TEST_PATH/passwd $HTTPD_ROOT_PATH
install_script broken-smart-http.sh
+   install_script error.sh
 
ln -s $LIB_HTTPD_MODULE_PATH $HTTPD_ROOT_PATH/modules
 
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 3a03e82..b384d79 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -97,12 +97,16 @@ Alias /auth/dumb/ www/auth/dumb/
 /LocationMatch
 ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1
 ScriptAlias /broken_smart/ broken-smart-http.sh/
+ScriptAlias /error/ error.sh/
 Directory ${GIT_EXEC_PATH}
Options FollowSymlinks
 /Directory
 Files broken-smart-http.sh
Options ExecCGI
 /Files
+Files error.sh
+  Options ExecCGI
+/Files
 Files ${GIT_EXEC_PATH}/git-http-backend
Options ExecCGI
 /Files
diff --git a/t/lib-httpd/error.sh b/t/lib-httpd/error.sh
new file mode 100755
index 000..786f281
--- /dev/null
+++ b/t/lib-httpd/error.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+printf Status: 500 Intentional Breakage\n
+
+printf Content-Type: 
+case $PATH_INFO in
+*html*)
+   printf text/html
+   ;;
+*text*)
+   printf text/plain
+   ;;
+esac
+printf \n
+
+printf \n
+printf this is the error message\n
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 1a3a2b6..13defd3 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -171,5 +171,15 @@ test_expect_success 'did not use upload-pack service' '
test_cmp exp act
 '
 
+test_expect_success 'git client shows text/plain errors' '
+   test_must_fail git clone $HTTPD_URL/error/text 2stderr 
+   grep this is the error message stderr
+'
+
+test_expect_success 'git client does not show html errors' '
+   test_must_fail git clone $HTTPD_URL/error/html 2stderr 
+   ! grep this is the error message stderr
+'
+
 stop_httpd
 test_done
-- 
2.0.0.rc1.436.g03cb729

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/9] t/lib-httpd: use write_script to copy CGI scripts

[Jeff King]

Using write_script will set our shebang line appropriately
with $SHELL_PATH. The script that is there now is quite
simple and likely to succeed even with a non-POSIX /bin/sh,
but it does not hurt to be defensive.

Signed-off-by: Jeff King p...@peff.net
---
 t/lib-httpd.sh   | 6 +-
 t/lib-httpd/broken-smart-http.sh | 1 -
 2 files changed, 5 insertions(+), 2 deletions(-)
 mode change 100755 = 100644 t/lib-httpd/broken-smart-http.sh

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 252cbf1..8e680ef 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -105,10 +105,14 @@ else
Could not identify web server at '$LIB_HTTPD_PATH'
 fi
 
+install_script () {
+   write_script $HTTPD_ROOT_PATH/$1 $TEST_PATH/$1
+}
+
 prepare_httpd() {
mkdir -p $HTTPD_DOCUMENT_ROOT_PATH
cp $TEST_PATH/passwd $HTTPD_ROOT_PATH
-   cp $TEST_PATH/broken-smart-http.sh $HTTPD_ROOT_PATH
+   install_script broken-smart-http.sh
 
ln -s $LIB_HTTPD_MODULE_PATH $HTTPD_ROOT_PATH/modules
 
diff --git a/t/lib-httpd/broken-smart-http.sh b/t/lib-httpd/broken-smart-http.sh
old mode 100755
new mode 100644
index f7ebfff..82cc610
--- a/t/lib-httpd/broken-smart-http.sh
+++ b/t/lib-httpd/broken-smart-http.sh
@@ -1,4 +1,3 @@
-#!/bin/sh
 printf Content-Type: text/%s\n html
 echo
 printf %s\n 001e# service=git-upload-pack
-- 
2.0.0.rc1.436.g03cb729

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: untracked file deleted from the master branch, when checked out to it from a local branch

[Sergei Organov]

Arup Rakshit arupraks...@rocketmail.com writes:


[...]

 Now you can see, that I have created, a new file called *file.txt*, in the 
 *master branch*.

And here is your basic misunderstanding. You've created file.txt indeed,
but not in the *master branch* (or any branch). You've created it in the
working directory. Only after you add/commit it, will it be created in
the branch that is checked out at this time. Until then, git won't touch
the file when you switch between branches.

-- Sergey.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

[Jeff King]

Commit 426e70d (remote-curl: show server content on http
errors, 2013-04-05) tried to recognize text/plain content
types, but fails to do so if they have any parameters.

This patches makes our parsing more liberal, though we still
do not do anything useful with a charset parameter.

Signed-off-by: Jeff King p...@peff.net
---
 remote-curl.c  | 26 ++
 t/lib-httpd/error.sh   |  8 +++-
 t/t5550-http-fetch-dumb.sh |  5 +
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index a5ab977..6d1b206 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -194,20 +194,30 @@ static void free_discovery(struct discovery *d)
}
 }
 
+/*
+ * We only show text/plain parts, as other types are likely
+ * to be ugly to look at on the user's terminal.
+ */
+static int content_type_is_terminal_friendly(struct strbuf *type)
+{
+   const char *p;
+
+   p = skip_prefix(type-buf, text/plain);
+   if (!p || (*p  *p != ';'))
+   return 0;
+
+   return 1;
+}
+
 static int show_http_message(struct strbuf *type, struct strbuf *msg)
 {
const char *p, *eol;
 
-   /*
-* We only show text/plain parts, as other types are likely
-* to be ugly to look at on the user's terminal.
-*
-* TODO should handle ; charset=XXX, and re-encode into
-* logoutputencoding
-*/
-   if (strcmp(type-buf, text/plain))
+   if (!content_type_is_terminal_friendly(type))
return -1;
 
+   /* TODO should record charset and reencode msg to logOutputEncoding */
+
strbuf_trim(msg);
if (!msg-len)
return -1;
diff --git a/t/lib-httpd/error.sh b/t/lib-httpd/error.sh
index 786f281..02e80b3 100755
--- a/t/lib-httpd/error.sh
+++ b/t/lib-httpd/error.sh
@@ -3,6 +3,7 @@
 printf Status: 500 Intentional Breakage\n
 
 printf Content-Type: 
+charset=iso8859-1
 case $PATH_INFO in
 *html*)
printf text/html
@@ -10,8 +11,13 @@ case $PATH_INFO in
 *text*)
printf text/plain
;;
+*charset*)
+   printf text/plain; charset=utf-8
+   charset=utf-8
+   ;;
 esac
 printf \n
 
 printf \n
-printf this is the error message\n
+printf this is the error message\n |
+iconv -f us-ascii -t $charset
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 13defd3..b35b261 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -181,5 +181,10 @@ test_expect_success 'git client does not show html errors' 
'
! grep this is the error message stderr
 '
 
+test_expect_success 'git client shows text/plain with a charset' '
+   test_must_fail git clone $HTTPD_URL/error/charset 2stderr 
+   grep this is the error message stderr
+'
+
 stop_httpd
 test_done
-- 
2.0.0.rc1.436.g03cb729

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/9] strbuf: add strbuf_reencode helper

[Jeff King]

This is a convenience wrapper around `reencode_string_len`
and `strbuf_attach`.

Signed-off-by: Jeff King p...@peff.net
---
 Documentation/technical/api-strbuf.txt |  5 +
 strbuf.c   | 17 +
 strbuf.h   |  1 +
 3 files changed, 23 insertions(+)

diff --git a/Documentation/technical/api-strbuf.txt 
b/Documentation/technical/api-strbuf.txt
index 8480f89..a468816 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -129,6 +129,11 @@ Functions
 
Lowercase each character in the buffer using `tolower`.
 
+`strbuf_reencode`::
+
+   Replace the contents of the strbuf with a reencoded form.  Returns -1
+   on error, 0 on success.
+
 `strbuf_cmp`::
 
Compare two buffers. Returns an integer less than, equal to, or greater
diff --git a/strbuf.c b/strbuf.c
index d289d1a..a717d9b 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,5 +1,6 @@
 #include cache.h
 #include refs.h
+#include utf8.h
 
 int starts_with(const char *str, const char *prefix)
 {
@@ -113,6 +114,22 @@ void strbuf_tolower(struct strbuf *sb)
sb-buf[i] = tolower(sb-buf[i]);
 }
 
+int strbuf_reencode(struct strbuf *sb, const char *from, const char *to)
+{
+   char *out;
+   int len;
+
+   if (same_encoding(from, to))
+   return 0;
+
+   out = reencode_string_len(sb-buf, sb-len, to, from, len);
+   if (!out)
+   return -1;
+
+   strbuf_attach(sb, out, len, len);
+   return 0;
+}
+
 struct strbuf **strbuf_split_buf(const char *str, size_t slen,
 int terminator, int max)
 {
diff --git a/strbuf.h b/strbuf.h
index 25328b9..a068dd6 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -46,6 +46,7 @@ extern void strbuf_trim(struct strbuf *);
 extern void strbuf_rtrim(struct strbuf *);
 extern void strbuf_ltrim(struct strbuf *);
 extern void strbuf_tolower(struct strbuf *sb);
+extern int strbuf_reencode(struct strbuf *sb, const char *from, const char 
*to);
 extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);
 
 /*
-- 
2.0.0.rc1.436.g03cb729

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 9/9] remote-curl: reencode http error messages

[Jeff King]

As of the last commit, we now recognize an error message
with a content-type text/plain; charset=utf-16 as text,
but we ignore the charset parameter entirely. Let's encode
it to log_output_encoding, which is presumably something the
user's terminal can handle.

Signed-off-by: Jeff King p...@peff.net
---
 remote-curl.c  | 37 +
 t/lib-httpd/error.sh   |  4 
 t/t5550-http-fetch-dumb.sh |  5 +
 3 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 6d1b206..1dc90d7 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -194,11 +194,34 @@ static void free_discovery(struct discovery *d)
}
 }
 
+static char *find_param(const char *str, const char *name)
+{
+   int len = strlen(name);
+
+   for (; *str; str++) {
+   const char *p;
+
+   if (*p++ != ' ')
+   continue;
+
+   if (strncmp(p, name, len))
+   continue;
+   p += len;
+
+   if (*p++ != '=')
+   continue;
+
+   return xstrndup(p, strchrnul(p, ' ') - p);
+   }
+
+   return NULL;
+}
+
 /*
  * We only show text/plain parts, as other types are likely
  * to be ugly to look at on the user's terminal.
  */
-static int content_type_is_terminal_friendly(struct strbuf *type)
+static int content_type_is_terminal_friendly(struct strbuf *type, char 
**charset)
 {
const char *p;
 
@@ -206,17 +229,23 @@ static int content_type_is_terminal_friendly(struct 
strbuf *type)
if (!p || (*p  *p != ';'))
return 0;
 
+   *charset = find_param(p, charset);
+   /* default charset from rfc2616 */
+   if (!*charset)
+   *charset = xstrdup(iso8859-1);
+
return 1;
 }
 
 static int show_http_message(struct strbuf *type, struct strbuf *msg)
 {
const char *p, *eol;
+   char *charset;
 
-   if (!content_type_is_terminal_friendly(type))
+   if (!content_type_is_terminal_friendly(type, charset))
return -1;
-
-   /* TODO should record charset and reencode msg to logOutputEncoding */
+   strbuf_reencode(msg, charset, get_log_output_encoding());
+   free(charset);
 
strbuf_trim(msg);
if (!msg-len)
diff --git a/t/lib-httpd/error.sh b/t/lib-httpd/error.sh
index 02e80b3..4efbce7 100755
--- a/t/lib-httpd/error.sh
+++ b/t/lib-httpd/error.sh
@@ -15,6 +15,10 @@ case $PATH_INFO in
printf text/plain; charset=utf-8
charset=utf-8
;;
+*utf16*)
+   printf text/plain; charset=utf-16
+   charset=utf-16
+   ;;
 esac
 printf \n
 
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index b35b261..01b8aae 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -186,5 +186,10 @@ test_expect_success 'git client shows text/plain with a 
charset' '
grep this is the error message stderr
 '
 
+test_expect_success 'http error messages are reencoded' '
+   test_must_fail git clone $HTTPD_URL/error/utf16 2stderr 
+   grep this is the error message stderr
+'
+
 stop_httpd
 test_done
-- 
2.0.0.rc1.436.g03cb729
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Slight inconsistency between ref delete commands.

[Sergei Organov]

Hello,

Was writing conversion script from CVS to git for my repo and noticed
slight inconsistency in git-tag, git-branch, and git-update-ref behavior:

$ git --version
git version 1.9.3
$ git tag -d  echo success
success
$ git branch -d  echo success
fatal: branch name required
$ git update-ref -d  echo success
usage: git update-ref [options] -d refname [oldval]
   or: git update-ref [options]refname newval [oldval]
   or: git update-ref [options] --stdin [-z]

-m reason   reason of the update
-ddelete the reference
--no-derefupdate refname not the one it points to
-zstdin has NUL-terminated arguments
--stdin   read updates from stdin


Noticed when used xargs without -r switch, like this:

git for-each-ref --format=%(refname) refs/tags/*-merge | xargs -n 1 git 
update-ref -d

-- Sergey.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] rebase: test ack

[Michael S. Tsirkin]

On Tue, May 20, 2014 at 08:13:27AM -0700, Junio C Hamano wrote:
 Michael S. Tsirkin m...@redhat.com writes:
 
  Just to clarify I can post v2 of 4/4 without reposting 1-3 since they
  are queued?
 
 If you need to update anything queued only on 'pu' but not yet in
 'next', it is customary to resend the whole for everybody to see
 (what is already in 'next' should only be built upon incrementally
 and not updated with replacement patches), while noting which ones
 are the same as before.  Christian Couder has been doing it nicely
 in his recent rerolls (if the series were not in 'next', that is).
 
 Thanks.

Actually I don't see anything like it in pu.
What I would like is for 1-3 to be in pu,
4/4 was for illustrative purposes it's not yet
ready for that, and 1-3 are useful by themselves.
I could then iterate on 4/4 without reposting 1-3.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] t6006 (rev-list-format): quote format strings to avoid error on some shells

[Alexey Shumkin]

This patch is redundant then.
It will be squashed into next patch series.

On Tue, May 20, 2014 at 06:48:43PM +0400, Alexey Shumkin wrote:
 Added in 0a144b3 (t4205, t6006: add failing tests for the case when
 i18n.logOutputEncoding is set, 2014-05-19) tests give no error
 (somehow) with Bash as /bin/sh but fail for some other shells.
 
 Quote format strings to avoid errors.
 
 Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
 Suggested-by: Ramsay Jones ram...@ramsay1.demon.co.uk
 ---
  t/t6006-rev-list-format.sh | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)
 
 diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
 index c6e9a73..19434ad 100755
 --- a/t/t6006-rev-list-format.sh
 +++ b/t/t6006-rev-list-format.sh
 @@ -149,7 +149,7 @@ commit $head1
  $added
  EOF
  
 -test_format subject-truncated %($truncate_count,trunc)%s EOF
 +test_format subject-truncated %($truncate_count,trunc)%s EOF
  commit $head2
  changed (ge${changed_utf8_part}ndert)..
  commit $head1
 @@ -259,7 +259,7 @@ commit $head1
  $added_iso88591
  EOF
  
 -test_format complex-subject-trunc %($truncate_count,trunc)%s EOF
 +test_format complex-subject-trunc %($truncate_count,trunc)%s EOF
  commit $head3
  Test printing of c..
  commit $head2
 @@ -268,7 +268,7 @@ commit $head1
  added (hinzugef${added_utf8_part_iso88591}gt..
  EOF
  
 -test_format complex-subject-mtrunc %($truncate_count,mtrunc)%s EOF
 +test_format complex-subject-mtrunc %($truncate_count,mtrunc)%s EOF
  commit $head3
  Test prin..ex bodies
  commit $head2
 @@ -277,7 +277,7 @@ commit $head1
  added (hi..f${added_utf8_part_iso88591}gt) foo
  EOF
  
 -test_format complex-subject-ltrunc %($truncate_count,ltrunc)%s EOF
 +test_format complex-subject-ltrunc %($truncate_count,ltrunc)%s EOF
  commit $head3
  .. of complex bodies
  commit $head2
 @@ -314,7 +314,7 @@ commit $head1
  $added
  EOF
  
 -test_format complex-subject-commitencoding-unset-trunc 
 %($truncate_count,trunc)%s EOF
 +test_format complex-subject-commitencoding-unset-trunc 
 %($truncate_count,trunc)%s EOF
  commit $head3
  Test printing of c..
  commit $head2
 @@ -323,7 +323,7 @@ commit $head1
  added (hinzugef${added_utf8_part}gt..
  EOF
  
 -test_format complex-subject-commitencoding-unset-mtrunc 
 %($truncate_count,mtrunc)%s EOF
 +test_format complex-subject-commitencoding-unset-mtrunc 
 %($truncate_count,mtrunc)%s EOF
  commit $head3
  Test prin..ex bodies
  commit $head2
 @@ -332,7 +332,7 @@ commit $head1
  added (hi..f${added_utf8_part}gt) foo
  EOF
  
 -test_format complex-subject-commitencoding-unset-ltrunc 
 %($truncate_count,ltrunc)%s EOF
 +test_format complex-subject-commitencoding-unset-ltrunc 
 %($truncate_count,ltrunc)%s EOF
  commit $head3
  .. of complex bodies
  commit $head2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 1/5] t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs

[Alexey Shumkin]

The expected SHA-1 digests are always available in variables. Use
them instead of hardcoding.

That was introduced in a742f2a (t4205 (log-pretty-formats): don't
hardcode SHA-1 in expected outputs, 2013-06-26) but unfortunately was
not followed in 5e1361c (log: properly handle decorations with chained
tags, 2013-12-17)

Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
---
 t/t4205-log-pretty-formats.sh | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 2a6278b..f9f33ae 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -296,6 +296,10 @@ EOF
test_cmp expected actual
 '
 
+# save HEAD's SHA-1 digest (with no abbreviations) to use it below
+# as far as the next test amends HEAD
+old_head1=$(git rev-parse --verify HEAD~0)
+
 test_expect_success 'left/right alignment formatting with stealing' '
git commit --amend -m short --author long long long l...@me.com 
git log --pretty=format:%(10,trunc)%s%(10,ltrunc)% an actual 
@@ -310,6 +314,10 @@ EOF
test_cmp expected actual
 '
 
+# get new digests (with no abbreviations)
+head1=$(git rev-parse --verify HEAD~0) 
+head2=$(git rev-parse --verify HEAD~1) 
+
 test_expect_success 'log decoration properly follows tag chain' '
git tag -a tag1 -m tag1 
git tag -a tag2 -m tag2 tag1 
@@ -317,9 +325,9 @@ test_expect_success 'log decoration properly follows tag 
chain' '
git commit --amend -m shorter 
git log --no-walk --tags --pretty=%H %d --decorate=full actual 
cat EOF expected 
-6a908c10688b2503073c39c9ba26322c73902bb5  (tag: refs/tags/tag2)
-9f716384d92283fb915a4eee5073f030638e05f9  (tag: refs/tags/message-one)
-b87e4cccdb77336ea79d89224737be7ea8e95367  (tag: refs/tags/message-two)
+$head1  (tag: refs/tags/tag2)
+$head2  (tag: refs/tags/message-one)
+$old_head1  (tag: refs/tags/message-two)
 EOF
sort actual actual1 
test_cmp expected actual1
-- 
1.9.2-15

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 0/5] Reroll patches. Pretty print truncate does not work

[Alexey Shumkin]

This version (v4) differs from the previuos (v3):
1. Fixed typo ISO8895-1 (vs ISO8859-1)
2. Fixed t4205 test: tested format strings are double-quoted

Alexey Shumkin (5):
  t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs
  t4041, t4205, t6006, t7102: Don't hardcode tested encoding value
  t4205 (log-pretty-format): Use `tformat` rather than `format`
  t4205, t6006: Add failing tests for the case when
i18n.logOutputEncoding is set
  pretty.c: format string with truncate respects logOutputEncoding

 pretty.c |   7 +-
 t/t4041-diff-submodule-option.sh |   7 +-
 t/t4205-log-pretty-formats.sh| 217 ++-
 t/t6006-rev-list-format.sh   | 110 
 t/t7102-reset.sh |  13 ++-
 5 files changed, 282 insertions(+), 72 deletions(-)

-- 
1.9.2-15

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 4/5] t4205, t6006: Add failing tests for the case when i18n.logOutputEncoding is set

[Alexey Shumkin]

Pretty format string %(N,[ml]trunc)%s truncates subject to a given
length with an appropriate padding. This works for non-ASCII texts when
i18n.logOutputEncoding is UTF-8 only (independently of a printed commit
message encoding) but does not work when i18n.logOutputEncoding is NOT
UTF-8.

There were no breakages as far as were no tests for the case
when both a commit message and logOutputEncoding are not UTF-8.

Add failing tests for that which will be fixed in the next patch.

Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
Reviewed-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Reviewed-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
 t/t4205-log-pretty-formats.sh | 140 ++
 t/t6006-rev-list-format.sh|  75 +-
 2 files changed, 213 insertions(+), 2 deletions(-)

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index c03a65e..74babce 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -154,6 +154,17 @@ EOF
test_cmp expected actual
 '
 
+test_expect_success 'left alignment formatting. i18n.logOutputEncoding' '
+   git -c i18n.logOutputEncoding=$test_encoding log 
--pretty=tformat:%(40)%s actual 
+   qz_to_tab_space EOF | iconv -f utf-8 -t $test_encoding expected 
+message twoZ
+message oneZ
+add barZ
+$(commit_msg)Z
+EOF
+   test_cmp expected actual
+'
+
 test_expect_success 'left alignment formatting at the nth column' '
git log --pretty=tformat:%h %|(40)%s actual 
qz_to_tab_space EOF expected 
@@ -165,6 +176,17 @@ EOF
test_cmp expected actual
 '
 
+test_expect_success 'left alignment formatting at the nth column. 
i18n.logOutputEncoding' '
+   git -c i18n.logOutputEncoding=$test_encoding log --pretty=tformat:%h 
%|(40)%s actual 
+   qz_to_tab_space EOF | iconv -f utf-8 -t $test_encoding expected 
+$head1 message twoZ
+$head2 message oneZ
+$head3 add barZ
+$head4 $(commit_msg)Z
+EOF
+   test_cmp expected actual
+'
+
 test_expect_success 'left alignment formatting with no padding' '
git log --pretty=tformat:%(1)%s actual 
cat EOF expected 
@@ -176,6 +198,17 @@ EOF
test_cmp expected actual
 '
 
+test_expect_success 'left alignment formatting with no padding. 
i18n.logOutputEncoding' '
+   git -c i18n.logOutputEncoding=$test_encoding log 
--pretty=tformat:%(1)%s actual 
+   cat EOF | iconv -f utf-8 -t $test_encoding expected 
+message two
+message one
+add bar
+$(commit_msg)
+EOF
+   test_cmp expected actual
+'
+
 test_expect_success 'left alignment formatting with trunc' '
git log --pretty=tformat:%(10,trunc)%s actual 
qz_to_tab_space EOF expected 
@@ -187,6 +220,17 @@ EOF
test_cmp expected actual
 '
 
+test_expect_failure 'left alignment formatting with trunc. 
i18n.logOutputEncoding' '
+   git -c i18n.logOutputEncoding=$test_encoding log 
--pretty=tformat:%(10,trunc)%s actual 
+   qz_to_tab_space EOF | iconv -f utf-8 -t $test_encoding expected 
+message ..
+message ..
+add bar  Z
+initial...
+EOF
+   test_cmp expected actual
+'
+
 test_expect_success 'left alignment formatting with ltrunc' '
git log --pretty=tformat:%(10,ltrunc)%s actual 
qz_to_tab_space EOF expected 
@@ -198,6 +242,17 @@ EOF
test_cmp expected actual
 '
 
+test_expect_failure 'left alignment formatting with ltrunc. 
i18n.logOutputEncoding' '
+   git -c i18n.logOutputEncoding=$test_encoding log 
--pretty=tformat:%(10,ltrunc)%s actual 
+   qz_to_tab_space EOF | iconv -f utf-8 -t $test_encoding expected 
+..sage two
+..sage one
+add bar  Z
+..${sample_utf8_part}lich
+EOF
+   test_cmp expected actual
+'
+
 test_expect_success 'left alignment formatting with mtrunc' '
git log --pretty=tformat:%(10,mtrunc)%s actual 
qz_to_tab_space EOF expected 
@@ -209,6 +264,17 @@ EOF
test_cmp expected actual
 '
 
+test_expect_failure 'left alignment formatting with mtrunc. 
i18n.logOutputEncoding' '
+   git -c i18n.logOutputEncoding=$test_encoding log 
--pretty=tformat:%(10,mtrunc)%s actual 
+   qz_to_tab_space EOF | iconv -f utf-8 -t $test_encoding expected 
+mess.. two
+mess.. one
+add bar  Z
+init..lich
+EOF
+   test_cmp expected actual
+'
+
 test_expect_success 'right alignment formatting' '
git log --pretty=tformat:%(40)%s actual 
qz_to_tab_space EOF expected 
@@ -220,6 +286,17 @@ EOF
test_cmp expected actual
 '
 
+test_expect_success 'right alignment formatting. i18n.logOutputEncoding' '
+   git -c i18n.logOutputEncoding=$test_encoding log 
--pretty=tformat:%(40)%s actual 
+   qz_to_tab_space EOF | iconv -f utf-8 -t $test_encoding expected 
+Zmessage two
+Zmessage one
+Z  

[PATCH v4 5/5] pretty.c: format string with truncate respects logOutputEncoding

[Alexey Shumkin]

Pretty format string %(N,[ml]trunc)%s truncates subject to a given
length with an appropriate padding. This works for non-ASCII texts when
i18n.logOutputEncoding is UTF-8 only (independently of a printed commit
message encoding) but does not work when i18n.logOutputEncoding is NOT
UTF-8.

In 7e77df3 (pretty: two phase conversion for non utf-8 commits, 2013-04-19)
'format_commit_item' function assumes commit message to be in UTF-8.
And that was so until ecaee80 (pretty: --format output should honor
logOutputEncoding, 2013-06-26) where conversion to logOutputEncoding was
added before calling 'format_commit_message'.

Correct this by converting a commit message to UTF-8 first (as it
assumed in 7e77df3 (pretty: two phase conversion for non utf-8 commits,
2013-04-19)). Only after that convert a commit message to an actual
logOutputEncoding.

Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
Reviewed-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 pretty.c  | 7 ++-
 t/t4205-log-pretty-formats.sh | 8 
 t/t6006-rev-list-format.sh| 6 +++---
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/pretty.c b/pretty.c
index 6e266dd..25e8825 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1507,13 +1507,18 @@ void format_commit_message(const struct commit *commit,
context.commit = commit;
context.pretty_ctx = pretty_ctx;
context.wrap_start = sb-len;
+   /*
+* convert a commit message to UTF-8 first
+* as far as 'format_commit_item' assumes it in UTF-8
+*/
context.message = logmsg_reencode(commit,
  context.commit_encoding,
- output_enc);
+ utf8);
 
strbuf_expand(sb, format, format_commit_item, context);
rewrap_message_tail(sb, context, 0, 0, 0);
 
+   /* then convert a commit message to an actual output encoding */
if (output_enc) {
if (same_encoding(utf8, output_enc))
output_enc = NULL;
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 74babce..c84ec9a 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -220,7 +220,7 @@ EOF
test_cmp expected actual
 '
 
-test_expect_failure 'left alignment formatting with trunc. 
i18n.logOutputEncoding' '
+test_expect_success 'left alignment formatting with trunc. 
i18n.logOutputEncoding' '
git -c i18n.logOutputEncoding=$test_encoding log 
--pretty=tformat:%(10,trunc)%s actual 
qz_to_tab_space EOF | iconv -f utf-8 -t $test_encoding expected 
 message ..
@@ -242,7 +242,7 @@ EOF
test_cmp expected actual
 '
 
-test_expect_failure 'left alignment formatting with ltrunc. 
i18n.logOutputEncoding' '
+test_expect_success 'left alignment formatting with ltrunc. 
i18n.logOutputEncoding' '
git -c i18n.logOutputEncoding=$test_encoding log 
--pretty=tformat:%(10,ltrunc)%s actual 
qz_to_tab_space EOF | iconv -f utf-8 -t $test_encoding expected 
 ..sage two
@@ -264,7 +264,7 @@ EOF
test_cmp expected actual
 '
 
-test_expect_failure 'left alignment formatting with mtrunc. 
i18n.logOutputEncoding' '
+test_expect_success 'left alignment formatting with mtrunc. 
i18n.logOutputEncoding' '
git -c i18n.logOutputEncoding=$test_encoding log 
--pretty=tformat:%(10,mtrunc)%s actual 
qz_to_tab_space EOF | iconv -f utf-8 -t $test_encoding expected 
 mess.. two
@@ -420,7 +420,7 @@ initial...   A U Thor
 EOF
test_cmp expected actual
 '
-test_expect_failure 'left/right alignment formatting with stealing. 
i18n.logOutputEncoding' '
+test_expect_success 'left/right alignment formatting with stealing. 
i18n.logOutputEncoding' '
git -c i18n.logOutputEncoding=$test_encoding log 
--pretty=tformat:%(10,trunc)%s%(10,ltrunc)% an actual 
cat EOF | iconv -f utf-8 -t $test_encoding expected 
 short long  long long
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 42bdefe..19434ad 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -259,7 +259,7 @@ commit $head1
 $added_iso88591
 EOF
 
-test_format complex-subject-trunc %($truncate_count,trunc)%s failure EOF
+test_format complex-subject-trunc %($truncate_count,trunc)%s EOF
 commit $head3
 Test printing of c..
 commit $head2
@@ -268,7 +268,7 @@ commit $head1
 added (hinzugef${added_utf8_part_iso88591}gt..
 EOF
 
-test_format complex-subject-mtrunc %($truncate_count,mtrunc)%s failure EOF
+test_format complex-subject-mtrunc %($truncate_count,mtrunc)%s EOF
 commit $head3
 Test prin..ex bodies
 commit $head2
@@ -277,7 +277,7 @@ commit $head1
 added (hi..f${added_utf8_part_iso88591}gt) foo
 EOF
 
-test_format complex-subject-ltrunc %($truncate_count,ltrunc)%s failure EOF
+test_format complex-subject-ltrunc %($truncate_count,ltrunc)%s EOF
 commit $head3
 .. of complex bodies
 commit $head2
-- 

[PATCH v4 2/5] t4041, t4205, t6006, t7102: Don't hardcode tested encoding value

[Alexey Shumkin]

The tested encoding is always available in a variable. Use it instead of
hardcoding. Also, to be in line with other tests use ISO8859-1
(uppercase) rather then iso8859-1.

Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
---
 t/t4041-diff-submodule-option.sh |  7 +--
 t/t4205-log-pretty-formats.sh| 11 +++
 t/t6006-rev-list-format.sh   | 35 +++
 t/t7102-reset.sh | 13 -
 4 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 1751c83..463d63b 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -11,6 +11,9 @@ This test tries to verify the sanity of the --submodule 
option of git diff.
 
 . ./test-lib.sh
 
+# Tested non-UTF-8 encoding
+test_encoding=ISO8859-1
+
 # String added in German (translated with Google Translate), encoded in 
UTF-8,
 # used in sample commit log messages in add_file() function below.
 added=$(printf hinzugef\303\274gt)
@@ -23,8 +26,8 @@ add_file () {
echo $name $name 
git add $name 
test_tick 
-   msg_added_iso88591=$(echo Add $name ($added $name) | 
iconv -f utf-8 -t iso8859-1) 
-   git -c 'i18n.commitEncoding=iso8859-1' commit -m 
$msg_added_iso88591
+   msg_added_iso88591=$(echo Add $name ($added $name) | 
iconv -f utf-8 -t $test_encoding) 
+   git -c i18n.commitEncoding=$test_encoding commit -m 
$msg_added_iso88591
done /dev/null 
git rev-parse --short --verify HEAD
)
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index f9f33ae..f5ea3f8 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -7,6 +7,9 @@
 test_description='Test pretty formats'
 . ./test-lib.sh
 
+# Tested non-UTF-8 encoding
+test_encoding=ISO8859-1
+
 sample_utf8_part=$(printf f\303\244ng)
 
 commit_msg () {
@@ -27,8 +30,8 @@ test_expect_success 'set up basic repos' '
bar 
git add foo 
test_tick 
-   git config i18n.commitEncoding iso8859-1 
-   git commit -m $(commit_msg iso8859-1) 
+   git config i18n.commitEncoding $test_encoding 
+   git commit -m $(commit_msg $test_encoding) 
git add bar 
test_tick 
git commit -m add bar 
@@ -56,8 +59,8 @@ test_expect_success 'alias user-defined format' '
test_cmp expected actual
 '
 
-test_expect_success 'alias user-defined tformat with %s (iso8859-1 encoding)' '
-   git config i18n.logOutputEncoding iso8859-1 
+test_expect_success 'alias user-defined tformat with %s (ISO8859-1 encoding)' '
+   git config i18n.logOutputEncoding $test_encoding 
git log --oneline expected-s 
git log --pretty=tformat:%h %s actual-s 
git config --unset i18n.logOutputEncoding 
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 9874403..9e4ba62 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -9,19 +9,22 @@ test_description='git rev-list --pretty=format test'
 . $TEST_DIRECTORY/lib-terminal.sh
 
 test_tick
+# Tested non-UTF-8 encoding
+test_encoding=ISO8859-1
+
 # String added in German
 # (translated with Google Translate),
 # encoded in UTF-8, used as a commit log message below.
 added=$(printf added (hinzugef\303\274gt) foo)
-added_iso88591=$(echo $added | iconv -f utf-8 -t iso8859-1)
+added_iso88591=$(echo $added | iconv -f utf-8 -t $test_encoding)
 # same but changed
 changed=$(printf changed (ge\303\244ndert) foo)
-changed_iso88591=$(echo $changed | iconv -f utf-8 -t iso8859-1)
+changed_iso88591=$(echo $changed | iconv -f utf-8 -t $test_encoding)
 
 test_expect_success 'setup' '
: foo 
git add foo 
-   git config i18n.commitEncoding iso8859-1 
+   git config i18n.commitEncoding $test_encoding 
git commit -m $added_iso88591 
head1=$(git rev-parse --verify HEAD) 
head1_short=$(git rev-parse --verify --short $head1) 
@@ -124,9 +127,9 @@ EOF
 
 test_format encoding %e EOF
 commit $head2
-iso8859-1
+$test_encoding
 commit $head1
-iso8859-1
+$test_encoding
 EOF
 
 test_format subject %s EOF
@@ -206,16 +209,16 @@ test_expect_success '%C(auto) respects --color=auto 
(stdout not tty)' '
)
 '
 
-iconv -f utf-8 -t iso8859-1  commit-msg EOF
+iconv -f utf-8 -t $test_encoding  commit-msg EOF
 Test printing of complex bodies
 
 This commit message is much longer than the others,
-and it will be encoded in iso8859-1. We should therefore
-include an iso8859 character: ¡bueno!
+and it will be encoded in $test_encoding. We should therefore
+include an ISO8859 character: ¡bueno!
 EOF
 
 test_expect_success 'setup complex body' '
-   git config i18n.commitencoding iso8859-1 
+   git config i18n.commitencoding $test_encoding 
echo change2 foo  git 

[PATCH v4 3/5] t4205 (log-pretty-format): Use `tformat` rather than `format`

[Alexey Shumkin]

Use `tformat` to avoid using of `echo` to complete end of line.

Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
---
 t/t4205-log-pretty-formats.sh | 52 +++
 1 file changed, 13 insertions(+), 39 deletions(-)

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index f5ea3f8..c03a65e 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -144,9 +144,7 @@ test_expect_success 'setup more commits' '
 '
 
 test_expect_success 'left alignment formatting' '
-   git log --pretty=format:%(40)%s actual 
-   # complete the incomplete line at the end
-   echo actual 
+   git log --pretty=tformat:%(40)%s actual 
qz_to_tab_space EOF expected 
 message twoZ
 message oneZ
@@ -157,9 +155,7 @@ EOF
 '
 
 test_expect_success 'left alignment formatting at the nth column' '
-   git log --pretty=format:%h %|(40)%s actual 
-   # complete the incomplete line at the end
-   echo actual 
+   git log --pretty=tformat:%h %|(40)%s actual 
qz_to_tab_space EOF expected 
 $head1 message twoZ
 $head2 message oneZ
@@ -170,9 +166,7 @@ EOF
 '
 
 test_expect_success 'left alignment formatting with no padding' '
-   git log --pretty=format:%(1)%s actual 
-   # complete the incomplete line at the end
-   echo actual 
+   git log --pretty=tformat:%(1)%s actual 
cat EOF expected 
 message two
 message one
@@ -183,9 +177,7 @@ EOF
 '
 
 test_expect_success 'left alignment formatting with trunc' '
-   git log --pretty=format:%(10,trunc)%s actual 
-   # complete the incomplete line at the end
-   echo actual 
+   git log --pretty=tformat:%(10,trunc)%s actual 
qz_to_tab_space EOF expected 
 message ..
 message ..
@@ -196,9 +188,7 @@ EOF
 '
 
 test_expect_success 'left alignment formatting with ltrunc' '
-   git log --pretty=format:%(10,ltrunc)%s actual 
-   # complete the incomplete line at the end
-   echo actual 
+   git log --pretty=tformat:%(10,ltrunc)%s actual 
qz_to_tab_space EOF expected 
 ..sage two
 ..sage one
@@ -209,9 +199,7 @@ EOF
 '
 
 test_expect_success 'left alignment formatting with mtrunc' '
-   git log --pretty=format:%(10,mtrunc)%s actual 
-   # complete the incomplete line at the end
-   echo actual 
+   git log --pretty=tformat:%(10,mtrunc)%s actual 
qz_to_tab_space EOF expected 
 mess.. two
 mess.. one
@@ -222,9 +210,7 @@ EOF
 '
 
 test_expect_success 'right alignment formatting' '
-   git log --pretty=format:%(40)%s actual 
-   # complete the incomplete line at the end
-   echo actual 
+   git log --pretty=tformat:%(40)%s actual 
qz_to_tab_space EOF expected 
 Zmessage two
 Zmessage one
@@ -235,9 +221,7 @@ EOF
 '
 
 test_expect_success 'right alignment formatting at the nth column' '
-   git log --pretty=format:%h %|(40)%s actual 
-   # complete the incomplete line at the end
-   echo actual 
+   git log --pretty=tformat:%h %|(40)%s actual 
qz_to_tab_space EOF expected 
 $head1  message two
 $head2  message one
@@ -248,9 +232,7 @@ EOF
 '
 
 test_expect_success 'right alignment formatting with no padding' '
-   git log --pretty=format:%(1)%s actual 
-   # complete the incomplete line at the end
-   echo actual 
+   git log --pretty=tformat:%(1)%s actual 
cat EOF expected 
 message two
 message one
@@ -261,9 +243,7 @@ EOF
 '
 
 test_expect_success 'center alignment formatting' '
-   git log --pretty=format:%(40)%s actual 
-   # complete the incomplete line at the end
-   echo actual 
+   git log --pretty=tformat:%(40)%s actual 
qz_to_tab_space EOF expected 
 Z message two  Z
 Z message one  Z
@@ -274,9 +254,7 @@ EOF
 '
 
 test_expect_success 'center alignment formatting at the nth column' '
-   git log --pretty=format:%h %|(40)%s actual 
-   # complete the incomplete line at the end
-   echo actual 
+   git log --pretty=tformat:%h %|(40)%s actual 
qz_to_tab_space EOF expected 
 $head1   message two  Z
 $head2   message one  Z
@@ -287,9 +265,7 @@ EOF
 '
 
 test_expect_success 'center alignment formatting with no padding' '
-   git log --pretty=format:%(1)%s actual 
-   # complete the incomplete line at the end
-   echo actual 
+   git log --pretty=tformat:%(1)%s actual 
cat EOF expected 
 message two
 message one
@@ -305,9 +281,7 @@ old_head1=$(git rev-parse --verify HEAD~0)
 
 test_expect_success 'left/right alignment formatting with stealing' '
git commit --amend -m short --author long long long l...@me.com 
-   git log --pretty=format:%(10,trunc)%s%(10,ltrunc)% an actual 
- 

[PATCHv2 10/19] git-submodule.sh: convert test -a/-o to and ||

[Elia Pinto]

The interaction with unary operators and operator precedence
for  and || are better known than -a and -o, and for that
reason we prefer them. Replace all existing instances
of -a and -o to save readers from the burden of thinking
about such things.
---
This is version 2 of the patch to git-submodule of the 
patch series convert  test -a/-o to  and ||. 
It contains the fixes identified by Johannes and
Matthieu. 

 git-submodule.sh |   32 
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index b55d83a..1e3a5a6 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -396,7 +396,7 @@ cmd_add()
sed -e 's|/$||' -e 's|:*/*\.git$||' -e 's|.*[/:]||g')
fi
 
-   if test -z $repo -o -z $sm_path; then
+   if test -z $repo || test -z $sm_path; then
usage
fi
 
@@ -453,7 +453,7 @@ Use -f if you really want to add it. 2
# perhaps the path exists and is already a git repo, else clone it
if test -e $sm_path
then
-   if test -d $sm_path/.git -o -f $sm_path/.git
+   if test -d $sm_path/.git || test -f $sm_path/.git
then
eval_gettextln Adding existing repo at '\$sm_path' to 
the index
else
@@ -835,7 +835,7 @@ Maybe you want to use 'update --init'?)
continue
fi
 
-   if ! test -d $sm_path/.git -o -f $sm_path/.git
+   if ! test -d $sm_path/.git || test -f $sm_path/.git
then
module_clone $sm_path $name $url $reference 
$depth || exit
cloned_modules=$cloned_modules;$name
@@ -860,11 +860,11 @@ Maybe you want to use 'update --init'?)
die $(eval_gettext Unable to find current 
${remote_name}/${branch} revision in submodule path '\$sm_path')
fi
 
-   if test $subsha1 != $sha1 -o -n $force
+   if test $subsha1 != $sha1 || test -n $force
then
subforce=$force
# If we don't already have a -f flag and the submodule 
has never been checked out
-   if test -z $subsha1 -a -z $force
+   if test -z $subsha1  test -z $force
then
subforce=-f
fi
@@ -1034,7 +1034,7 @@ cmd_summary() {
then
head=$rev
test $# = 0 || shift
-   elif test -z $1 -o $1 = HEAD
+   elif test -z $1 || test $1 = HEAD
then
# before the first commit: compare with an empty tree
head=$(git hash-object -w -t tree --stdin /dev/null)
@@ -1059,13 +1059,17 @@ cmd_summary() {
while read mod_src mod_dst sha1_src sha1_dst status sm_path
do
# Always show modules deleted or type-changed 
(blob-module)
-   test $status = D -o $status = T  echo $sm_path  
continue
+   case $status in
+   [DT])
+   printf '%s\n' $sm_path 
+   continue
+   esac
# Respect the ignore setting for --for-status.
if test -n $for_status
then
name=$(module_name $sm_path)
ignore_config=$(get_submodule_config $name 
ignore none)
-   test $status != A -a $ignore_config = all  
continue
+   test $status != A  test $ignore_config = all 
 continue
fi
# Also show added or modified modules which are checked 
out
GIT_DIR=$sm_path/.git git-rev-parse --git-dir 
/dev/null 21 
@@ -1125,7 +1129,7 @@ cmd_summary() {
*)
errmsg=
total_commits=$(
-   if test $mod_src = 16 -a $mod_dst = 16
+   if test $mod_src = 16  test $mod_dst = 16
then
range=$sha1_src...$sha1_dst
elif test $mod_src = 16
@@ -1162,7 +1166,7 @@ cmd_summary() {
# i.e. deleted or changed to blob
test $mod_dst = 16  echo $errmsg
else
-   if test $mod_src = 16 -a $mod_dst = 16
+   if test $mod_src = 16  test $mod_dst = 16
then
limit=
test $summary_limit -gt 0  
limit=-$summary_limit
@@ -1233,7 +1237,11 @@ cmd_status()
say U$sha1 $displaypath
continue
fi

Re: [PATCH v8 28/44] refs.c: make write_ref_sha1 static

[Ronnie Sahlberg]

Thanks!

On Tue, May 20, 2014 at 5:51 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Ronnie Sahlberg wrote:

 No external users call write_ref_sha1 any more so lets declare it static.

 Yay!

 [...]
 +++ b/refs.c
 @@ -251,6 +251,8 @@ struct ref_entry {
 [...]
  static void read_loose_refs(const char *dirname, struct ref_dir *dir);
 +static int write_ref_sha1(struct ref_lock *lock,
 +   const unsigned char *sha1, const char *logmsg);

 Is this forward declaration needed?


No. Removed.
(I could have sworn I had a patch to remove this forward declaration.)

 [...]
 --- a/refs.h
 +++ b/refs.h
 @@ -150,9 +150,6 @@ extern int commit_ref(struct ref_lock *lock);
  /** Release any lock taken but not written. **/
  extern void unlock_ref(struct ref_lock *lock);

 -/** Writes sha1 into the ref specified by the lock. **/
 -extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, 
 const char *msg);

 (nit) Would be nice to keep the documentation comment.

Moved this comment to refs.c

Please see ref-transactions branch
Thanks!


 Thanks,
 Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/5] t4205, t6006: Add failing tests for the case when i18n.logOutputEncoding is set

[Ramsay Jones]

On 21/05/14 14:20, Alexey Shumkin wrote:
 Pretty format string %(N,[ml]trunc)%s truncates subject to a given
 length with an appropriate padding. This works for non-ASCII texts when
 i18n.logOutputEncoding is UTF-8 only (independently of a printed commit
 message encoding) but does not work when i18n.logOutputEncoding is NOT
 UTF-8.
 
 There were no breakages as far as were no tests for the case
 when both a commit message and logOutputEncoding are not UTF-8.
 
 Add failing tests for that which will be fixed in the next patch.
 
 Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
 Reviewed-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 Reviewed-by: Ramsay Jones ram...@ramsay1.demon.co.uk

Hmm, I didn't really review these patches. I simply noted a problem
on my system and provided you with an extended bug-report and
assisted you in fixing it up. So, if it even warrants a mention in
the commit message, then 'Helped-by:' would be nearer the mark.

Thanks!

ATB,
Ramsay Jones



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

git push rejected due being behind after git svn dcommit without any changes local/remote

[Henning Sprang]

Hi,

I have a client that still uses svn, but I decided to version my work
on the project with git locally - using git svn to sync the svn and my
local git repo.

To have a backup , I additionally push my changes to a remote git repository.

Now, many(maybe every) times when doing the git push to the remote git
repository, after having done an svn dcommit to sync to the remote
svn, the push gets rejected:

... Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart

You'd say, can happen, when some developers work against the remote
git repo, others with svn. But I'm the only developer on the project,
and no one ever commits to the remote svn nor the remote git repo. So,
I'm clueless.

How can I find out what happens and how to prevent that?

Thanks in advance,
Henning

-- 
Henning Sprang
http://www.sprang.de
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] format-patch --signature-file file

[Jeff King]

On Tue, May 20, 2014 at 11:46:51AM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  If it were just --signature, I'd agree. After all, nobody is even
  complaining. But this is also in preparation for --signature-file.
  Should the user create a file without a trailing newline?
 
 Ahh, I missed that part.
 
 I am fine with processing it with stripspace().

I wasn't planning on anything as drastic as stripspace. I really just
wanted to suppress the one newline, which is almost certainly the right
thing to include for --signature, but the wrong thing for
--signature-file (i.e., the patch I posted earlier).

Stripspace() would drop all extra whitespace, and I wondered if people
would _want_ it in their sigs (e.g., a blank line after the --  but
before their .sig content).

I dunno. Maybe it is not worth caring too much about. I don't want to
hold up Jeremiah's patch for something that I suspect neither of us
cares _that_ much about (I know I am not planning on using
--signature-file myself). I just don't want to deal with a patch later
that says oh, this spacing is wrong and have to respond yes, but we
have to retain it so as not to break existing users.

 By the way, at some point we may want to move that helper function
 to strbuf.c, but that is a separate issue.

Agreed. I was touching some string functions earlier today and noticed
that strbuf.c actually contains a lot of non-strbuf functions for
dealing with C strings. That's fine, I guess, but I also wondered if we
should have a separate file for C-string functions. I suppose it doesn't
matter that much either way, as long as it's in a libgit.a file (and
stripspace currently is _not_, which I assume is what you were
indicating above).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Slight inconsistency between ref delete commands.

[Jeff King]

On Wed, May 21, 2014 at 02:35:46PM +0400, Sergei Organov wrote:

 Was writing conversion script from CVS to git for my repo and noticed
 slight inconsistency in git-tag, git-branch, and git-update-ref behavior:
 
 $ git --version
 git version 1.9.3
 $ git tag -d  echo success
 success

This makes sense to me. tag -d takes zero or more tags and deletes
them.

 $ git branch -d  echo success
 fatal: branch name required

Here I think branch -d is being overly picky. It should behave the
same as tag. I'd welcome a patch for that.

 $ git update-ref -d  echo success

Here we cannot do the same zero or more behavior, because of:

 usage: git update-ref [options] -d refname [oldval]

...we need to be able to take an optional oldval for each argument.

 Noticed when used xargs without -r switch, like this:
 
 git for-each-ref --format=%(refname) refs/tags/*-merge | xargs -n 1 git 
 update-ref -d

I know this is a side note to the inconsistency you found, but it would
be nice to be able to do that with a single update-ref invocation. Not
only for simplicity, but also because it would be more efficient
(deleting a packed ref has to rewrite the whole packed-refs file; we can
get away with one rewrite if we know we are deleting multiple refs).

Recently-ish, update-ref learned a --stdin mode, which I think you
could use like:

  git for-each-ref --format='delete %(refname)' refs/tags/*-merge |
  git update-ref --stdin

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] rebase: test ack

[Junio C Hamano]

Michael S. Tsirkin m...@redhat.com writes:

 On Tue, May 20, 2014 at 08:13:27AM -0700, Junio C Hamano wrote:
 Michael S. Tsirkin m...@redhat.com writes:
 
  Just to clarify I can post v2 of 4/4 without reposting 1-3 since they
  are queued?
 
 If you need to update anything queued only on 'pu' but not yet in
 'next', it is customary to resend the whole for everybody to see
 (what is already in 'next' should only be built upon incrementally
 and not updated with replacement patches), while noting which ones
 are the same as before.  Christian Couder has been doing it nicely
 in his recent rerolls (if the series were not in 'next', that is).
 
 Thanks.

 Actually I don't see anything like it in pu.

The way I usually work is to apply a non-fix (i.e. enhancement)
series on a topic branch forked from 'master' (or the last tagged
version contained in its tip) and see if it makes sense, and then
try-merge the result to 'next' to see if it is free of potential
funny interactions with other topics that are already in flight.
After that happens, the topic branch is merged to somewhere in 'pu'.

It is possible that I did not have time to go through all the steps
above (after all, I had to make another -rc release and there was an
unexpected last-minute change of plans in the morning that blew a
few hours of work).  Or there may have been some merge conflicts
that I didn't feel like resolving for various reasons (e.g. if I
knew the series would be rerolled anyway, it can wait; if the other
topic that interacts with your series has been cooking sufficiently
long in 'next' and if it is very close to the final release of this
cycle, it may be easier to wait for the other topic to graduate to
'master', which would happen soon after this cycle finishes, and ask
you to rebase your series).

I don't remember which ;-)


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH v4 3/3] add command performance tracing to debug scripted commands

[Jeff King]

On Tue, May 20, 2014 at 09:11:24PM +0200, Karsten Blees wrote:

 Add performance tracing to identify which git commands are called and how
 long they execute. This is particularly useful to debug performance issues
 of scripted commands.
 
 Usage example:  GIT_TRACE_PERFORMANCE=~/git-trace.log git stash list
 
 Creates a log file like this:
 performance: at trace.c:319, time: 0.000303280 s: git command: 'git' 
 'rev-parse' '--git-dir'
 performance: at trace.c:319, time: 0.000334409 s: git command: 'git' 
 'rev-parse' '--is-inside-work-tree'
 performance: at trace.c:319, time: 0.000215243 s: git command: 'git' 
 'rev-parse' '--show-toplevel'
 performance: at trace.c:319, time: 0.000410639 s: git command: 'git' 'config' 
 '--get-colorbool' 'color.interactive'
 performance: at trace.c:319, time: 0.000394077 s: git command: 'git' 'config' 
 '--get-color' 'color.interactive.help' 'red bold'
 performance: at trace.c:319, time: 0.000280701 s: git command: 'git' 'config' 
 '--get-color' '' 'reset'
 performance: at trace.c:319, time: 0.000908185 s: git command: 'git' 
 'rev-parse' '--verify' 'refs/stash'
 performance: at trace.c:319, time: 0.028827774 s: git command: 'git' 'stash' 
 'list'

Neat. I actually wanted something like this just yesterday. It looks
like you are mainly tracing the execution of programs. Would it make
sense to just tie this to regular trace_* calls, and if
GIT_TRACE_PERFORMANCE is set, add a timestamp to each line?

Then we would not need to add separate trace_command_performance calls,
and other parts of the code that are already instrumented with GIT_TRACE
would get the feature for free.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH v4 2/3] add trace_performance facility to debug performance issues

[Jeff King]

On Tue, May 20, 2014 at 09:11:19PM +0200, Karsten Blees wrote:

 Add trace_performance and trace_performance_since macros that print file
 name, line number, time and an optional printf-formatted text to the file
 specified in environment variable GIT_TRACE_PERFORMANCE.
 
 Unless enabled via GIT_TRACE_PERFORMANCE, these macros have no noticeable
 impact on performance, so that test code may be shipped in release builds.
 
 MSVC: variadic macros (__VA_ARGS__) require VC++ 2005 or newer.

I think we still have some Unix compilers that do not do variadic
macros, either. For a while, people were compiling with antique stuff
like SUNWspro and MIPSpro. I don't know if they still do, if they use
gcc on such systems now, or if those systems have finally been
decomissioned.

But either we need to change our stance on variadic macros, or this
feature needs to be able to be compiled conditionally.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] format-patch --signature-file file

[Jeff King]

On Wed, May 21, 2014 at 09:55:43AM -0700, Jeremiah Mahler wrote:

 I am fine with including your previous patch.

I think that would be my preference, but we'll see what Junio says.

 Would like me to test it out and create another patch set?

Yeah, that would be the logical next step. I think the only thing you
should need to adjust in your patch is to strip only one line from the
output, rather than two.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] Git v2.0.0-rc4

[Junio C Hamano]

Martin Erik Werner martinerikwer...@gmail.com writes:

  * Commands that take pathspecs on the command line misbehaved when
the pathspec is given as an absolute pathname (which is a
practice not particularly encouraged) that points at a symbolic
link in the working tree.
(merge later 655ee9e mw/symlinks to maint.)

 In order to include the latest cleanup to this patchset:
 setup: fix windows path buffer over-stepping
 this should be 6127ff6 instead. Sorry if it's unneeded to note, but just
 wanted to make sure :)

Yeah, that commit is more like fix to a not-quite-right fix rather
than cleanup, and is indeed sitting at the tip of mw/symlinks
topic I still hold onto, so that it can be later merged to 'maint'.
And I agree that it is necessary to merge to 6127ff6 when the topic
is merged to 'maint'.

The entries in the release notes are fed to the ML (stands for
Merge Later) script found on my 'todo' branch from its standard
input to remind me of bugfix topics that need to go to 'maint', and
the process should have caught it (i.e. the topic has grown and its
tip no longer points at the named commit since the entry was
written), but somehow I missed it.

Will fix it up.  Thanks for noticing.



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] Git v2.0.0-rc4

[Junio C Hamano]

Felipe Contreras felipe.contre...@gmail.com writes:

 Junio C Hamano wrote:

  * The remote-helper interface to fast-import/fast-export via the
transport-helper has been tightened to avoid leaving the import
marks file from a failed/crashed run, as such a file that is out-of-
sync with reality confuses a later invocation of itself.

 Really? Where are the patches for that?

 I think it's fair to say the way the remote-helpers and transport-helper
 has been handled for v2.0 has been a total disaster.

Thanks for noticing.  The last-minute change of plans in the morning
on the -rc release day did not help.  Will remove.

Anything else I missed?


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 27/44] walker.c: use ref transaction for ref updates

[Ronnie Sahlberg]

On Tue, May 20, 2014 at 5:46 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Ronnie Sahlberg wrote:

 This changes the locking slightly for walker_fetch. Previously the code would
 lock all refs before writing them but now we do not lock the refs until the
 commit stage. There is thus a very short window where changes could be done
 locally during the fetch which would be overwritten when the fetch completes
 and commits its transaction. But this window should be reasonably short.
 Even if this race does trigger, since both the old code and the new code
 just overwrites the refs to the new values without checking or comparing
 them with the previous value, this is not too dissimilar to a similar 
 scenario
 where you first do a ref change locally and then later do a fetch that
 overwrites the local change. With this in mind I do not see the change in
 locking semantics to be critical.

 Sounds scary.  The usual approach is

 old_sha1 = ...
 ... various checks ...

 transaction = transaction_begin(err)
 transaction_update(transaction, refname, new_sha1, old_sha1, ...);
 transaction_commit(transaction, err);

 which is not racy because _update checks against old_sha1.

 If I understand correctly, you are saying 'have_old' is false here so
 we don't have the usual protection.  If the ... various checks ...
 section shown above is empty, that should be fine and there is no
 actual change in semantics.  If the ... various checks ... section
 shown above is nonempty then it could be a problem.

Yeah, we don't have the old sha1 so this function is already imo a bit dodgy.
The transaction system in this series can not really handle this with
the exact same semantics as the old code
but we do gain the ability to do things such as

transaction_begin
... lock all refs...
...do other stuff.
transaction_update for all refs
transaction_commit

in the next patch series when the transactions start supporting
multiple updates for the same ref.
However that support comes reasonably late in the next series and I
would want to avoid growing this series much more.


This is the patch in the next series that restore the lock-everything-first.
It relies on that we will do all locking in _update already and not
wait until _commit as well as that it allows multiple updates to the
same ref as long as all updates, except the first, are of the form
have_old==1 and old_sha1==new_sha1-of-previous-update.


diff --git a/walker.c b/walker.c
index 94d4988..dd8c11e 100644
--- a/walker.c
+++ b/walker.c
@@ -255,18 +255,31 @@ int walker_fetch(struct walker *walker, int targets, char
struct strbuf err = STRBUF_INIT;
struct ref_transaction *transaction;
unsigned char *sha1 = xmalloc(targets * 20);
+   unsigned char transaction_sha1[20];
char *msg;
int i;

save_commit_buffer = 0;

if (write_ref) {
+   memset(transaction_sha1, 0xff, 20);
transaction = transaction_begin(err);
if (!transaction) {
error(%s, err.buf);
strbuf_release(err);
return -1;
}
+   /* Lock all refs by updating to a temporary sha1 */
+   for (i = 0; i  targets; i++) {
+   if (!write_ref[i])
+   continue;
+   strbuf_reset(ref_name);
+   strbuf_addf(ref_name, refs/%s, write_ref[i]);
+   transaction_update_sha1(transaction, ref_name.buf,
+   transaction_sha1, NULL, 0, 0,
+   msg ? msg : fetch (unknown),
+   err);
+   }
}
if (!walker-get_recover)
for_each_ref(mark_complete, NULL);
@@ -295,7 +308,8 @@ int walker_fetch(struct walker *walker, int targets, char **
strbuf_reset(ref_name);
strbuf_addf(ref_name, refs/%s, write_ref[i]);
if (transaction_update_sha1(transaction, ref_name.buf,
-   sha1[20 * i], NULL, 0, 0,
+   sha1[20 * i], transaction_sha1,
+   0, 1,
msg ? msg : fetch (unknown),
err))
break;




 [...]
 --- a/walker.c
 +++ b/walker.c
 @@ -251,24 +251,18 @@ void walker_targets_free(int targets, char **target, 
 const char **write_ref)
  int walker_fetch(struct walker *walker, int targets, char **target,
const char **write_ref, const char *write_ref_log_details)
  {
 - struct ref_lock **lock = xcalloc(targets, sizeof(struct ref_lock *));
 + char ref_name[PATH_MAX];

 We tend to prefer strbuf instead of fixed-size buffers in new code.

Changed.


 

Re: [RFC/PATCH v4 3/3] add command performance tracing to debug scripted commands

[Junio C Hamano]

Jeff King p...@peff.net writes:

 On Tue, May 20, 2014 at 09:11:24PM +0200, Karsten Blees wrote:

 Add performance tracing to identify which git commands are called and how
 long they execute. This is particularly useful to debug performance issues
 of scripted commands.
 
 Usage example:  GIT_TRACE_PERFORMANCE=~/git-trace.log git stash list
 
 Creates a log file like this:
 performance: at trace.c:319, time: 0.000303280 s: git command: 'git' 
 'rev-parse' '--git-dir'
 performance: at trace.c:319, time: 0.000334409 s: git command: 'git' 
 'rev-parse' '--is-inside-work-tree'
 performance: at trace.c:319, time: 0.000215243 s: git command: 'git' 
 'rev-parse' '--show-toplevel'
 performance: at trace.c:319, time: 0.000410639 s: git command: 'git' 
 'config' '--get-colorbool' 'color.interactive'
 performance: at trace.c:319, time: 0.000394077 s: git command: 'git' 
 'config' '--get-color' 'color.interactive.help' 'red bold'
 performance: at trace.c:319, time: 0.000280701 s: git command: 'git' 
 'config' '--get-color' '' 'reset'
 performance: at trace.c:319, time: 0.000908185 s: git command: 'git' 
 'rev-parse' '--verify' 'refs/stash'
 performance: at trace.c:319, time: 0.028827774 s: git command: 'git' 'stash' 
 'list'

 Neat. I actually wanted something like this just yesterday. It looks
 like you are mainly tracing the execution of programs. Would it make
 sense to just tie this to regular trace_* calls, and if
 GIT_TRACE_PERFORMANCE is set, add a timestamp to each line?

Yeah, I very much like both, the output and your suggestion to hook
it into the existing infrastructure.

 Then we would not need to add separate trace_command_performance calls,
 and other parts of the code that are already instrumented with GIT_TRACE
 would get the feature for free.

 -Peff

 -- 
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] format-patch --signature-file file

[Junio C Hamano]

Jeff King p...@peff.net writes:

 On Tue, May 20, 2014 at 11:46:51AM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  If it were just --signature, I'd agree. After all, nobody is even
  complaining. But this is also in preparation for --signature-file.
  Should the user create a file without a trailing newline?
 
 Ahh, I missed that part.
 
 I am fine with processing it with stripspace().

 I wasn't planning on anything as drastic as stripspace. I really just
 wanted to suppress the one newline, which is almost certainly the right
 thing to include for --signature, but the wrong thing for
 --signature-file (i.e., the patch I posted earlier).
 ...
 I dunno. Maybe it is not worth caring too much about.

I suggested stripspace() because I know we do not care too
much, actually.

Cleansing blank lines in one way for many types of user input
(e.g. commit log messages and tag messages) while doing it in a
completely different way just for --signature-file is warranted if
there is a good reason for them to be different, but I did not think
of any, and I still don't.  So...

 By the way, at some point we may want to move that helper function
 to strbuf.c, but that is a separate issue.

 Agreed. I was touching some string functions earlier today and noticed
 that strbuf.c actually contains a lot of non-strbuf functions for
 dealing with C strings. That's fine, I guess, but I also wondered if we
 should have a separate file for C-string functions. I suppose it doesn't
 matter that much either way, as long as it's in a libgit.a file (and
 stripspace currently is _not_, which I assume is what you were
 indicating above).

Yes, I thought you would have used it in your follow-up patch if it
were more prominent.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] rebase: test ack

[Michael S. Tsirkin]

On Wed, May 21, 2014 at 09:54:47AM -0700, Junio C Hamano wrote:
 Michael S. Tsirkin m...@redhat.com writes:
 
  On Tue, May 20, 2014 at 08:13:27AM -0700, Junio C Hamano wrote:
  Michael S. Tsirkin m...@redhat.com writes:
  
   Just to clarify I can post v2 of 4/4 without reposting 1-3 since they
   are queued?
  
  If you need to update anything queued only on 'pu' but not yet in
  'next', it is customary to resend the whole for everybody to see
  (what is already in 'next' should only be built upon incrementally
  and not updated with replacement patches), while noting which ones
  are the same as before.  Christian Couder has been doing it nicely
  in his recent rerolls (if the series were not in 'next', that is).
  
  Thanks.
 
  Actually I don't see anything like it in pu.
 
 The way I usually work is to apply a non-fix (i.e. enhancement)
 series on a topic branch forked from 'master' (or the last tagged
 version contained in its tip) and see if it makes sense, and then
 try-merge the result to 'next' to see if it is free of potential
 funny interactions with other topics that are already in flight.
 After that happens, the topic branch is merged to somewhere in 'pu'.
 
 It is possible that I did not have time to go through all the steps
 above (after all, I had to make another -rc release and there was an
 unexpected last-minute change of plans in the morning that blew a
 few hours of work).  Or there may have been some merge conflicts
 that I didn't feel like resolving for various reasons (e.g. if I
 knew the series would be rerolled anyway, it can wait; if the other
 topic that interacts with your series has been cooking sufficiently
 long in 'next' and if it is very close to the final release of this
 cycle, it may be easier to wait for the other topic to graduate to
 'master', which would happen soon after this cycle finishes, and ask
 you to rebase your series).
 
 I don't remember which ;-)
 

Oh sorry, didn't mean to try to pressure you. I was just surprised
not to see it there. I know this applies cleanly to next so I'll just
wait for 2.0 to be out.

-- 
MST
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] format-patch --signature-file file

[Jeff King]

On Wed, May 21, 2014 at 10:37:05AM -0700, Junio C Hamano wrote:

  I wasn't planning on anything as drastic as stripspace. I really just
  wanted to suppress the one newline, which is almost certainly the right
  thing to include for --signature, but the wrong thing for
  --signature-file (i.e., the patch I posted earlier).
  ...
  I dunno. Maybe it is not worth caring too much about.
 
 I suggested stripspace() because I know we do not care too
 much, actually.
 
 Cleansing blank lines in one way for many types of user input
 (e.g. commit log messages and tag messages) while doing it in a
 completely different way just for --signature-file is warranted if
 there is a good reason for them to be different, but I did not think
 of any, and I still don't.  So...

I didn't think of mine as cleansing. It is more like do not duplicate a
newline ourselves if there is already one there.  But I guess those are
two sides of the same coin.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/5] t4205, t6006: Add failing tests for the case when i18n.logOutputEncoding is set

[Junio C Hamano]

Ramsay Jones ram...@ramsay1.demon.co.uk writes:

 On 21/05/14 14:20, Alexey Shumkin wrote:
 Pretty format string %(N,[ml]trunc)%s truncates subject to a given
 length with an appropriate padding. This works for non-ASCII texts when
 i18n.logOutputEncoding is UTF-8 only (independently of a printed commit
 message encoding) but does not work when i18n.logOutputEncoding is NOT
 UTF-8.
 
 There were no breakages as far as were no tests for the case
 when both a commit message and logOutputEncoding are not UTF-8.
 
 Add failing tests for that which will be fixed in the next patch.
 
 Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
 Reviewed-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 Reviewed-by: Ramsay Jones ram...@ramsay1.demon.co.uk

 Hmm, I didn't really review these patches. I simply noted a problem
 on my system and provided you with an extended bug-report and
 assisted you in fixing it up. So, if it even warrants a mention in
 the commit message, then 'Helped-by:' would be nearer the mark.

I had the same impression.  The same for Duy's.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] format-patch --signature-file file

[Junio C Hamano]

Jeff King p...@peff.net writes:

 On Wed, May 21, 2014 at 10:37:05AM -0700, Junio C Hamano wrote:

  I wasn't planning on anything as drastic as stripspace. I really just
  wanted to suppress the one newline, which is almost certainly the right
  thing to include for --signature, but the wrong thing for
  --signature-file (i.e., the patch I posted earlier).
  ...
  I dunno. Maybe it is not worth caring too much about.
 
 I suggested stripspace() because I know we do not care too
 much, actually.
 
 Cleansing blank lines in one way for many types of user input
 (e.g. commit log messages and tag messages) while doing it in a
 completely different way just for --signature-file is warranted if
 there is a good reason for them to be different, but I did not think
 of any, and I still don't.  So...

 I didn't think of mine as cleansing. It is more like do not duplicate a
 newline ourselves if there is already one there.  But I guess those are
 two sides of the same coin.

Yeah, I agree with the last sentence.  My mention of cleansing
took into account your do we want to omit the leading blank as
well? as well.  In any case, wouldn't reusing stripspace() make the
fix-up shorter?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] rebase: test ack

[Junio C Hamano]

Michael S. Tsirkin m...@redhat.com writes:

 On Wed, May 21, 2014 at 09:54:47AM -0700, Junio C Hamano wrote:
 Michael S. Tsirkin m...@redhat.com writes:
 
  On Tue, May 20, 2014 at 08:13:27AM -0700, Junio C Hamano wrote:
  Michael S. Tsirkin m...@redhat.com writes:
  
   Just to clarify I can post v2 of 4/4 without reposting 1-3 since they
   are queued?
  
  If you need to update anything queued only on 'pu' but not yet in
  'next', it is customary to ...
 
  Actually I don't see anything like it in pu.
 ... 
 Oh sorry, didn't mean to try to pressure you. I was just surprised
 not to see it there. I know this applies cleanly to next so I'll just
 wait for 2.0 to be out.

Oh, no.  No pressure felt and no need to be sorry about anything.

I described a preferred procedure when the topic appeared in 'pu',
and I didn't answer your question for topics that are not even in
'pu' yet.

Being in 'pu' and not in 'next' is not much different from not being
in 'pu', so the preferred procedure is to send out the entire series
(unless it is a large 47-patch series ;-) to give everybody another
chance to comment, and it would be extra nice if you indicated which
ones are unchanged since the previous round to help those who did
already saw them.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH v4 2/3] add trace_performance facility to debug performance issues

[Karsten Blees]

Am 21.05.2014 18:58, schrieb Jeff King:
 On Tue, May 20, 2014 at 09:11:19PM +0200, Karsten Blees wrote:
 
 Add trace_performance and trace_performance_since macros that print file
 name, line number, time and an optional printf-formatted text to the file
 specified in environment variable GIT_TRACE_PERFORMANCE.

 Unless enabled via GIT_TRACE_PERFORMANCE, these macros have no noticeable
 impact on performance, so that test code may be shipped in release builds.

 MSVC: variadic macros (__VA_ARGS__) require VC++ 2005 or newer.
 
 I think we still have some Unix compilers that do not do variadic
 macros, either. For a while, people were compiling with antique stuff
 like SUNWspro and MIPSpro. I don't know if they still do, if they use
 gcc on such systems now, or if those systems have finally been
 decomissioned.
 
 But either we need to change our stance on variadic macros, or this
 feature needs to be able to be compiled conditionally.
 
 -Peff
 

Macros are mainly used to supply __FILE__ and __LINE__, so that lazy people 
don't need to think of a unique message for each use of trace_performance_*. 
Without __FILE__, __LINE__ and message, the output would be pretty useless 
(i.e. just the time without any additional info).

If there's platforms that don't support variadic macros, I'd suggest to drop 
the __FILE__ __LINE__ feature completely and make message mandatory (with the 
added benefit that manually provided messages don't change if the code is 
moved, i.e. trace logs would become somewhat comparable across versions).

(adding cc: Dscho as IIRC the __FILE__ __LINE__ idea was originally his).

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 25/44] receive-pack.c: use a reference transaction for updating the refs

[Ronnie Sahlberg]

On Tue, May 20, 2014 at 1:37 PM, Ronnie Sahlberg sahlb...@google.com wrote:
 On Tue, May 20, 2014 at 12:42 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Ronnie Sahlberg wrote:

 Wrap all the ref updates inside a transaction to make the update atomic.

 Interesting.

 [...]
 --- a/builtin/receive-pack.c
 +++ b/builtin/receive-pack.c
 @@ -46,6 +46,8 @@ static void *head_name_to_free;
  static int sent_capabilities;
  static int shallow_update;
  static const char *alt_shallow_file;
 +static struct strbuf err = STRBUF_INIT;

 I think it would be cleaner for err to be local.  It isn't used for
 communication between functions.

 Done.


 [...]
 @@ -580,15 +581,9 @@ static const char *update(struct command *cmd, struct 
 shallow_info *si)
   update_shallow_ref(cmd, si))
   return shallow error;

 - lock = lock_any_ref_for_update(namespaced_name, old_sha1,
 -0, NULL);
 - if (!lock) {
 - rp_error(failed to lock %s, name);
 - return failed to lock;
 - }
 - if (write_ref_sha1(lock, new_sha1, push)) {
 - return failed to write; /* error() already called */
 - }
 + if (ref_transaction_update(transaction, namespaced_name,
 +new_sha1, old_sha1, 0, 1, err))
 + return failed to update;

 The original used rp_error to send an error message immediately via
 sideband.  This drops that --- intended?

 Oops. I misread it as a normal error()


 The old error string shown on the push status line was was failed to
 lock or failed to write which makes it clear that the cause is
 contention or database problems or filesystem problems, respectively.
 After this change it would say failed to update which is about as
 clear as failed.

 I changed it to return xstrdup(err.buf) which should be as detailed as
 we can get.


 Would it be safe to send err.buf as-is over the wire, or does it
 contain information or formatting that wouldn't be suitable for the
 client?  (I haven't thought this through completely yet.)  Is there
 some easy way to distinguish between failure to lock and failure to
 write?  Or is there some one-size-fits-all error for this case?

 I think err.buf is what we want.


 When the transaction fails, we need to make sure that all ref updates
 emit 'ng' and not 'ok' in receive-pack.c::report (see the example at
 the end of Documentation/technical/pack-protocol.txt for what this
 means).  What error string should they use?  Is there some way to make
 it clear to the user which ref was the culprit?

 What should happen when checks outside the ref transaction system
 cause a ref update to fail?  I'm thinking of

  * per-ref 'update' hook (see githooks(5))
  * fast-forward check
  * ref creation/deletion checks
  * attempt to push to the currently checked out branch

 I think the natural thing to do would be to put each ref update in its
 own transaction to start so the semantics do not change right away.
 If there are obvious answers to all these questions, then a separate
 patch could combine all these into a single transaction; or if there
 are no obvious answers, we could make the single-transaction-per-push
 semantics optional (using a configuration variable or protocol
 capability or something) to make it possible to experiment.

 I changed it to use one transaction per ref for now.

 Please see the update ref-transactions branch.

I have added support for atomic pushes towards the end of the -next series.
https://github.com/rsahlberg/git/tree/ref-transactions-next

It is activated by a new --atomic-push argument to send-pack and is
then negotiated by a new option in the wire protocol.



 Thanks!
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Documentation: use command-line when used as a compound adjective, and fix other minor grammatical issues

[Jason St. John]

Signed-off-by: Jason St. John jstj...@purdue.edu
---
Junio pointed out in the link below that there are a number of instances of e.g.
command line option (without a hyphen) in the documentation. This patch fixes
all of them that I was able to find. The other minor grammatical improvements 
were
noticed while working on the switch to command-line option; they seemed too
trivial to justify a separate patch.

https://marc.info/?l=gitm=140054322310981w=2

 Documentation/config.txt   | 10 +-
 Documentation/diff-config.txt  |  2 +-
 Documentation/git-bisect.txt   |  2 +-
 Documentation/git-config.txt   |  2 +-
 Documentation/git-daemon.txt   |  6 +++---
 Documentation/git-fast-import.txt  | 10 +-
 Documentation/git-help.txt | 12 ++--
 Documentation/git-ls-files.txt |  6 +++---
 Documentation/git-read-tree.txt|  2 +-
 Documentation/git-send-email.txt   |  6 +++---
 Documentation/git-svn.txt  |  4 ++--
 Documentation/git-web--browse.txt  |  4 ++--
 Documentation/git.txt  |  8 
 Documentation/gitcli.txt   |  8 
 Documentation/gitk.txt |  2 +-
 Documentation/gitweb.conf.txt  |  2 +-
 Documentation/howto/setup-git-server-over-http.txt |  2 +-
 Documentation/technical/http-protocol.txt  |  2 +-
 Documentation/user-manual.txt  |  8 
 19 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1932e9b..553b300 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -381,7 +381,7 @@ false), while all other repositories are assumed to be bare 
(bare
 core.worktree::
Set the path to the root of the working tree.
This can be overridden by the GIT_WORK_TREE environment
-   variable and the '--work-tree' command line option.
+   variable and the '--work-tree' command-line option.
The value can be an absolute path or relative to the path to
the .git directory, which is either specified by --git-dir
or GIT_DIR, or automatically discovered.
@@ -523,7 +523,7 @@ core.askpass::
environment variable. If not set, fall back to the value of the
'SSH_ASKPASS' environment variable or, failing that, a simple password
prompt. The external program shall be given a suitable prompt as
-   command line argument and write the password on its STDOUT.
+   command-line argument and write the password on its STDOUT.
 
 core.attributesfile::
In addition to '.gitattributes' (per-directory) and
@@ -1324,7 +1324,7 @@ grep.extendedRegexp::
 gpg.program::
Use this custom program instead of gpg found on $PATH when
making or verifying a PGP signature. The program must support the
-   same command line interface as GPG, namely, to verify a detached
+   same command-line interface as GPG, namely, to verify a detached
signature, gpg --verify $file - $signature is run, and the
program is expected to signal a good signature by exiting with
code 0, and to generate an ascii-armored detached signature, the
@@ -2295,7 +2295,7 @@ status.submodulesummary::
submodules when `diff.ignoreSubmodules` is set to 'all' or only
for those submodules where `submodule.name.ignore=all`. To
also view the summary for ignored submodules you can either use
-   the --ignore-submodules=dirty command line option or the 'git
+   the --ignore-submodules=dirty command-line option or the 'git
submodule summary' command, which shows a similar output but does
not honor these settings.
 
@@ -2317,7 +2317,7 @@ submodule.name.branch::
 submodule.name.fetchRecurseSubmodules::
This option can be used to control recursive fetching of this
submodule. It can be overridden by using the --[no-]recurse-submodules
-   command line option to git fetch and git pull.
+   command-line option to git fetch and git pull.
This setting will override that from in the linkgit:gitmodules[5]
file.
 
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index f07b451..b001779 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -76,7 +76,7 @@ diff.ignoreSubmodules::
this setting when reporting uncommitted changes. Setting it to
'all' disables the submodule summary normally shown by 'git commit'
and 'git status' when 'status.submodulesummary' is set unless it is
-   overridden by using the --ignore-submodules command line option.
+   overridden by using the --ignore-submodules command-line option.
The 'git submodule' 

Re: [PATCH] Documentation: use command-line when used as a compound adjective, and fix other minor grammatical issues

[Jason St. John]

On Wed, May 21, 2014 at 2:52 PM, Jason St. John jstj...@purdue.edu wrote:

 Signed-off-by: Jason St. John jstj...@purdue.edu
 ---
 Junio pointed out in the link below that there are a number of instances of 
 e.g.
 command line option (without a hyphen) in the documentation. This patch 
 fixes
 all of them that I was able to find. The other minor grammatical improvements 
 were
 noticed while working on the switch to command-line option; they seemed too
 trivial to justify a separate patch.

 https://marc.info/?l=gitm=140054322310981w=2

  Documentation/config.txt   | 10 +-
  Documentation/diff-config.txt  |  2 +-
  Documentation/git-bisect.txt   |  2 +-
  Documentation/git-config.txt   |  2 +-
  Documentation/git-daemon.txt   |  6 +++---
  Documentation/git-fast-import.txt  | 10 +-
  Documentation/git-help.txt | 12 ++--
  Documentation/git-ls-files.txt |  6 +++---
  Documentation/git-read-tree.txt|  2 +-
  Documentation/git-send-email.txt   |  6 +++---
  Documentation/git-svn.txt  |  4 ++--
  Documentation/git-web--browse.txt  |  4 ++--
  Documentation/git.txt  |  8 
  Documentation/gitcli.txt   |  8 
  Documentation/gitk.txt |  2 +-
  Documentation/gitweb.conf.txt  |  2 +-
  Documentation/howto/setup-git-server-over-http.txt |  2 +-
  Documentation/technical/http-protocol.txt  |  2 +-
  Documentation/user-manual.txt  |  8 
  19 files changed, 49 insertions(+), 49 deletions(-)

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 1932e9b..553b300 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -381,7 +381,7 @@ false), while all other repositories are assumed to be 
 bare (bare
  core.worktree::
 Set the path to the root of the working tree.
 This can be overridden by the GIT_WORK_TREE environment
 -   variable and the '--work-tree' command line option.
 +   variable and the '--work-tree' command-line option.
 The value can be an absolute path or relative to the path to
 the .git directory, which is either specified by --git-dir
 or GIT_DIR, or automatically discovered.
 @@ -523,7 +523,7 @@ core.askpass::
 environment variable. If not set, fall back to the value of the
 'SSH_ASKPASS' environment variable or, failing that, a simple password
 prompt. The external program shall be given a suitable prompt as
 -   command line argument and write the password on its STDOUT.
 +   command-line argument and write the password on its STDOUT.

  core.attributesfile::
 In addition to '.gitattributes' (per-directory) and
 @@ -1324,7 +1324,7 @@ grep.extendedRegexp::
  gpg.program::
 Use this custom program instead of gpg found on $PATH when
 making or verifying a PGP signature. The program must support the
 -   same command line interface as GPG, namely, to verify a detached
 +   same command-line interface as GPG, namely, to verify a detached
 signature, gpg --verify $file - $signature is run, and the
 program is expected to signal a good signature by exiting with
 code 0, and to generate an ascii-armored detached signature, the
 @@ -2295,7 +2295,7 @@ status.submodulesummary::
 submodules when `diff.ignoreSubmodules` is set to 'all' or only
 for those submodules where `submodule.name.ignore=all`. To
 also view the summary for ignored submodules you can either use
 -   the --ignore-submodules=dirty command line option or the 'git
 +   the --ignore-submodules=dirty command-line option or the 'git
 submodule summary' command, which shows a similar output but does
 not honor these settings.

 @@ -2317,7 +2317,7 @@ submodule.name.branch::
  submodule.name.fetchRecurseSubmodules::
 This option can be used to control recursive fetching of this
 submodule. It can be overridden by using the --[no-]recurse-submodules
 -   command line option to git fetch and git pull.
 +   command-line option to git fetch and git pull.
 This setting will override that from in the linkgit:gitmodules[5]
 file.

 diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
 index f07b451..b001779 100644
 --- a/Documentation/diff-config.txt
 +++ b/Documentation/diff-config.txt
 @@ -76,7 +76,7 @@ diff.ignoreSubmodules::
 this setting when reporting uncommitted changes. Setting it to
 'all' disables the submodule summary normally shown by 'git commit'
 and 'git status' when 'status.submodulesummary' is set unless it is
 -   overridden 

Re: [PATCH] Documentation: use command-line when used as a compound adjective, and fix other minor grammatical issues

[Junio C Hamano]

Jason St. John jstj...@purdue.edu writes:

 diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
 index 022e74e..01200ed 100644
 --- a/Documentation/user-manual.txt
 +++ b/Documentation/user-manual.txt
 @@ -173,13 +173,13 @@ index 65898fa..b002dc6 100644
  --- a/init-db.c
  +++ b/init-db.c
  @@ -7,7 +7,7 @@
 - 
 +
   int main(int argc, char **argv)
   {
  -char *sha1_dir = getenv(DB_ENVIRONMENT), *path;
  +char *sha1_dir, *path;
   int len, i;
 - 
 +
   if (mkdir(.git, 0755)  0) {
  

I know it would not make any difference either way when printed, but
aren't we showing how a typical diff output looks like with this
example, and each of these two otherwise blank lines should actually
be a line with a single SP on it?

Everything else looked fine.  Thanks for cleaning the docs up.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation: use command-line when used as a compound adjective, and fix other minor grammatical issues

[Junio C Hamano]

Jason St. John jstj...@purdue.edu writes:

   int main(int argc, char **argv)
   {
  -  char *sha1_dir = getenv(DB_ENVIRONMENT), *path;
  +  char *sha1_dir, *path;
 int len, i;
 -
 +
 if (mkdir(.git, 0755)  0) {
  


 I just noticed some trailing whitespace got stripped out here by my
 text editor, and I'm not sure if this will break formatting. Should I
 resubmit this patch without the above hunk?

It is easy enough for me to edit it out from the patch just before
applying, so no need to resend.  Unless there are other necessary
changes spotted by people who review the patch, that is.

Please do not quote the entire mail without trimming it down to
relevant parts.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] format-patch --signature-file file

[Jeff King]

On Wed, May 21, 2014 at 11:26:18AM -0700, Junio C Hamano wrote:

 Yeah, I agree with the last sentence.  My mention of cleansing
 took into account your do we want to omit the leading blank as
 well? as well.  In any case, wouldn't reusing stripspace() make the
 fix-up shorter?

Not really. Doing stripspace() on the file we read would be a one-liner,
but it doesn't address the extra line. We _already_ have
stripspace-compatible output in the file, with a trailing newline. It's
the one we add for the cmdline arg or default (which lack a newline
generally) that causes the doubling.

So I think it would be something like:

diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..0312402 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -844,8 +844,13 @@ static void gen_message_id(struct rev_info *info, char 
*base)
 
 static void print_signature(void)
 {
-   if (signature  *signature)
-   printf(-- \n%s\n\n, signature);
+   if (signature  *signature) {
+   struct strbuf buf = STRBUF_INIT;
+   strbuf_addstr(buf, signature);
+   stripspace(buf, 0);
+   printf(-- \n%s\n, buf.buf);
+   strbuf_release(buf);
+   }
 }
 
 static void add_branch_description(struct strbuf *buf, const char *branch_name)

I.e., make sure we have consistent stripspace output, and then stop
adding the extra newline in the printf (notice we still include the
extra blank at the end, though we can obviously easily drop it here).
Compare to the previous patch I sent, or to the shortest possible:

diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..16b8771 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -845,7 +845,8 @@ static void gen_message_id(struct rev_info *info, char 
*base)
 static void print_signature(void)
 {
if (signature  *signature)
-   printf(-- \n%s\n\n, signature);
+   printf(-- \n%s%s\n, signature,
+  ends_with(signature, \n) ?  : \n);
 }
 
 static void add_branch_description(struct strbuf *buf, const char *branch_name)

But I'm not sure length of code is the right metric. They're all
pretty easy to do, if we can decide on which.

I think the ratio of usefulness to number of words in this sub-thread is
getting off. I'd be OK with any of:

  1. Leave it as-is. Files get two blank lines.

  2. Conditional newline

  3. Stripspace.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH v4 2/3] add trace_performance facility to debug performance issues

[Jeff King]

On Wed, May 21, 2014 at 08:34:47PM +0200, Karsten Blees wrote:

 Macros are mainly used to supply __FILE__ and __LINE__, so that lazy
 people don't need to think of a unique message for each use of
 trace_performance_*. Without __FILE__, __LINE__ and message, the
 output would be pretty useless (i.e. just the time without any
 additional info).
 
 If there's platforms that don't support variadic macros, I'd suggest
 to drop the __FILE__ __LINE__ feature completely and make message
 mandatory (with the added benefit that manually provided messages
 don't change if the code is moved, i.e. trace logs would become
 somewhat comparable across versions).

I do think __FILE__ and __LINE__ can be useful, and it would not be the
end of the world to simply omit them on platforms that can't do the
variadic macros (there shouldn't be many these days, and I think they
are used to living with reduced functionality in some cases).

But if this were attached to trace_printf, we would always have a
message anyway, no?

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v7 0/2] format-patch --signature-file file

[Jeremiah Mahler]

v7 of patch to add format-patch --signature-file file option.

This revision includes a patch from Jeff King to fix the odd number
spaces produced by print_signature().

Jeff King (1):
  format-patch: make newline after signature conditional

Jeremiah Mahler (1):
  format-patch --signature-file file

 Documentation/config.txt   |  4 
 Documentation/git-format-patch.txt |  4 
 builtin/log.c  | 25 +++--
 t/t4014-format-patch.sh| 32 
 4 files changed, 63 insertions(+), 2 deletions(-)

-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v7 1/2] format-patch: make newline after signature conditional

[Jeremiah Mahler]

From: Jeff King p...@peff.net

When we print an email signature, we print the divider --
\n, then the signature string, then two newlines.
Traditionally the signature is a one-liner (and the default
is just the git version), so the extra newline makes sense.

But one could easily specify a longer, multi-line signature,
like:

  git format-patch --signature='
  this is my long signature

  it has multiple lines
  ' ...

We should notice that it already has its own trailing
newline, and suppress one of ours.

Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 builtin/log.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..5acc048 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -844,8 +844,13 @@ static void gen_message_id(struct rev_info *info, char 
*base)
 
 static void print_signature(void)
 {
-   if (signature  *signature)
-   printf(-- \n%s\n\n, signature);
+   if (!signature || !*signature)
+   return;
+
+   printf(-- \n%s, signature);
+   if (signature[strlen(signature)-1] != '\n')
+   putchar('\n');
+   putchar('\n');
 }
 
 static void add_branch_description(struct strbuf *buf, const char *branch_name)
-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v7 2/2] format-patch --signature-file file

[Jeremiah Mahler]

Added option that allows a signature file to be used with format-patch
so that signatures with newlines and other special characters can be
easily included.

  $ git format-patch --signature-file ~/.signature -1

The config variable format.signaturefile is also provided so that it
can be added by default.

  $ git config format.signaturefile ~/.signature

  $ git format-patch -1

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 Documentation/config.txt   |  4 
 Documentation/git-format-patch.txt |  4 
 builtin/log.c  | 16 
 t/t4014-format-patch.sh| 32 
 4 files changed, 56 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1932e9b..140ed77 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1114,6 +1114,10 @@ format.signature::
Set this variable to the empty string () to suppress
signature generation.
 
+format.signaturefile::
+   Works just like format.signature except the contents of the
+   file specified by this variable will be used as the signature.
+
 format.suffix::
The default for format-patch is to output files with the suffix
`.patch`. Use this variable to change that suffix (make sure to
diff --git a/Documentation/git-format-patch.txt 
b/Documentation/git-format-patch.txt
index 5c0a4ab..c0fd470 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -14,6 +14,7 @@ SYNOPSIS
   [(--attach|--inline)[=boundary] | --no-attach]
   [-s | --signoff]
   [--signature=signature | --no-signature]
+  [--signature-file=file]
   [-n | --numbered | -N | --no-numbered]
   [--start-number n] [--numbered-files]
   [--in-reply-to=Message-Id] [--suffix=.sfx]
@@ -233,6 +234,9 @@ configuration options in linkgit:git-notes[1] to use this 
workflow).
signature option is omitted the signature defaults to the Git version
number.
 
+--signature-file=file::
+   Works just like --signature except the signature is read from a file.
+
 --suffix=.sfx::
Instead of using `.patch` as the suffix for generated
filenames, use specified suffix.  A common alternative is
diff --git a/builtin/log.c b/builtin/log.c
index 5acc048..28d22fd 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -673,6 +673,7 @@ static void add_header(const char *value)
 static int thread;
 static int do_signoff;
 static const char *signature = git_version_string;
+static const char *signature_file;
 static int config_cover_letter;
 
 enum {
@@ -742,6 +743,8 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
}
if (!strcmp(var, format.signature))
return git_config_string(signature, var, value);
+   if (!strcmp(var, format.signaturefile))
+   return git_config_pathname(signature_file, var, value);
if (!strcmp(var, format.coverletter)) {
if (value  !strcasecmp(value, auto)) {
config_cover_letter = COVER_AUTO;
@@ -1235,6 +1238,8 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
PARSE_OPT_OPTARG, thread_callback },
OPT_STRING(0, signature, signature, N_(signature),
N_(add a signature)),
+   OPT_FILENAME(0, signature-file, signature_file,
+   N_(add a signature from a file)),
OPT__QUIET(quiet, N_(don't print the patch filenames)),
OPT_END()
};
@@ -1452,6 +1457,17 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
cover_letter = (config_cover_letter == COVER_ON);
}
 
+   if (signature_file) {
+   if (signature  signature != git_version_string)
+   die(_(cannot specify both signature and 
signature-file));
+
+   struct strbuf buf = STRBUF_INIT;
+
+   if (strbuf_read_file(buf, signature_file, 128)  0)
+   die_errno(_(unable to read signature file '%s'), 
signature_file);
+   signature = strbuf_detach(buf, NULL);
+   }
+
if (in_reply_to || thread || cover_letter)
rev.ref_message_ids = xcalloc(1, sizeof(struct string_list));
if (in_reply_to) {
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 9c80633..e604f65 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -762,6 +762,38 @@ test_expect_success 'format-patch --signature= 
suppresses signatures' '
! grep ^-- \$ output
 '
 
+cat expect -\EOF
+
+Test User test.em...@kernel.org
+http://git.kernel.org/cgit/git/git.git
+
+git.kernel.org/?p=git/git.git;a=summary
+
+EOF
+
+test_expect_success 'format-patch --signature-file=file' '
+

Re: [PATCH v6] format-patch --signature-file file

[Junio C Hamano]

Jeremiah Mahler jmmah...@gmail.com writes:

 diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
 index 9c80633..049493d 100755
 --- a/t/t4014-format-patch.sh
 +++ b/t/t4014-format-patch.sh
 @@ -762,6 +762,38 @@ test_expect_success 'format-patch --signature= 
 suppresses signatures' '
   ! grep ^-- \$ output
  '
  
 +cat expect -\EOF
 +
 +Test User test.em...@kernel.org
 +http://git.kernel.org/cgit/git/git.git
 +
 +git.kernel.org/?p=git/git.git;a=summary
 +
 +EOF

We have been trying not to do the above in recent test updates.  It
would be nice if this set-up did not have to be outside of the usual
test_expect_success structure.

 +test_expect_success 'format-patch --signature-file=file' '
 + git format-patch --stdout --signature-file=expect -1 output 
 + check_patch output 
 + sed -n -e /^-- $/,\$p output | sed -e 1 d | sed -e \$d | sed -e 
 \$d output2 

Overlong line.  If we can't make this pipeline shorter, at least
fold it to a reasonable length, e.g.

sed -n -e ... output |
sed -e '1d' -e \$d |
sed -e \$d output2 

or something.

The SP between the address 1 and insn d looks somewhat funny and
ugly, especially given that you write the other one $d without
such a SP.  Was there a specific reason why it was needed?

I would further think that renaming a few files and updating the way
the check is done may make the whole thing easier to understand.

 * rename the input for --signature-file to mail-signature.

 * keep the name output to hold the format-patch output, i.e.

 git format-patch -1 --stdout --signature-file=mail-signature output

 * Instead of munging the mail signature part of the output too
   excessively to match the input, formulate the expected output
   using mail-signature as an input, i.e.

 sed -e '1,/^-- $/d' output actual 
 {
cat mail-signature  echo  echo
 } expect 
 test_cmp expect actual

Alternatively, the third-bullet point above may want to be further
future-proofed by using stripspace, e.g.

sed -e '1/^-- $/d' output | git stripspace actual 
git stripspace mail-signature expect 
test_cmp expect actual

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] format-patch --signature-file file

[Jeremiah Mahler]

On Wed, May 21, 2014 at 04:47:39PM -0400, Jeff King wrote:
 On Wed, May 21, 2014 at 11:26:18AM -0700, Junio C Hamano wrote:
 
...
 I think the ratio of usefulness to number of words in this sub-thread is
 getting off. I'd be OK with any of:
...
 
 -Peff

I think this has become a discussion about the airspeed of an unladen swallow.
:-)

I think we're close to a final patch. v7 sent.

-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 1/2] format-patch: make newline after signature conditional

[Junio C Hamano]

Jeremiah Mahler jmmah...@gmail.com writes:

 From: Jeff King p...@peff.net

 When we print an email signature, we print the divider --
 \n, then the signature string, then two newlines.
 Traditionally the signature is a one-liner (and the default
 is just the git version), so the extra newline makes sense.

 But one could easily specify a longer, multi-line signature,
 like:

   git format-patch --signature='
   this is my long signature

   it has multiple lines
   ' ...

 We should notice that it already has its own trailing
 newline, and suppress one of ours.

That is a half-good example; the first line being a blank is
misleading, as we are not doing anything about that, though.

Other than that, the patch looks OK to me.  If anybody complains we
can fix it later to do more cleansing.

 Signed-off-by: Jeff King p...@peff.net
 Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
 ---
  builtin/log.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

 diff --git a/builtin/log.c b/builtin/log.c
 index 39e8836..5acc048 100644
 --- a/builtin/log.c
 +++ b/builtin/log.c
 @@ -844,8 +844,13 @@ static void gen_message_id(struct rev_info *info, char 
 *base)
  
  static void print_signature(void)
  {
 - if (signature  *signature)
 - printf(-- \n%s\n\n, signature);
 + if (!signature || !*signature)
 + return;
 +
 + printf(-- \n%s, signature);
 + if (signature[strlen(signature)-1] != '\n')
 + putchar('\n');
 + putchar('\n');
  }
  
  static void add_branch_description(struct strbuf *buf, const char 
 *branch_name)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] format-patch --signature-file file

[Junio C Hamano]

Junio C Hamano gits...@pobox.com writes:

 Jeremiah Mahler jmmah...@gmail.com writes:

 diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
 index 9c80633..049493d 100755
 --- a/t/t4014-format-patch.sh
 +++ b/t/t4014-format-patch.sh
 @@ -762,6 +762,38 @@ test_expect_success 'format-patch --signature= 
 suppresses signatures' '
  ! grep ^-- \$ output
  '
  
 +cat expect -\EOF
 +
 +Test User test.em...@kernel.org
 +http://git.kernel.org/cgit/git/git.git
 +
 +git.kernel.org/?p=git/git.git;a=summary
 +
 +EOF

 We have been trying not to do the above in recent test updates.  It
 would be nice if this set-up did not have to be outside of the usual
 test_expect_success structure.

It seems you sent v7 before I had a chance to comment on this one.
The above was merely it would be nicer... and I am OK as-is.  The
comments on the rest are a bit more serious, though.

Thanks.


 +test_expect_success 'format-patch --signature-file=file' '
 +git format-patch --stdout --signature-file=expect -1 output 
 +check_patch output 
 +sed -n -e /^-- $/,\$p output | sed -e 1 d | sed -e \$d | sed -e 
 \$d output2 

 Overlong line.  If we can't make this pipeline shorter, at least
 fold it to a reasonable length, e.g.

   sed -n -e ... output |
 sed -e '1d' -e \$d |
 sed -e \$d output2 

 or something.

 The SP between the address 1 and insn d looks somewhat funny and
 ugly, especially given that you write the other one $d without
 such a SP.  Was there a specific reason why it was needed?

 I would further think that renaming a few files and updating the way
 the check is done may make the whole thing easier to understand.

  * rename the input for --signature-file to mail-signature.

  * keep the name output to hold the format-patch output, i.e.

  git format-patch -1 --stdout --signature-file=mail-signature output

  * Instead of munging the mail signature part of the output too
excessively to match the input, formulate the expected output
using mail-signature as an input, i.e.

  sed -e '1,/^-- $/d' output actual 
  {
   cat mail-signature  echo  echo
  } expect 
  test_cmp expect actual

 Alternatively, the third-bullet point above may want to be further
 future-proofed by using stripspace, e.g.

 sed -e '1/^-- $/d' output | git stripspace actual 
 git stripspace mail-signature expect 
 test_cmp expect actual

 Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] Git v2.0.0-rc4

[Richard Hansen]

On 2014-05-20 20:24, Junio C Hamano wrote:
 Fixes since v1.9 series
 ---
 
 Unless otherwise noted, all the fixes since v1.9 in the maintenance
 track are contained in this release (see the maintenance releases'
 notes for details).
[...]

  * The shell prompt script (in contrib/), when using the PROMPT_COMMAND
interface, used an unsafe construct when showing the branch name in
$PS1.
(merge 8976500 rh/prompt-pcmode-avoid-eval-on-refname later to maint).

That commit has been merged to maint and is in v1.9.3.

Also, 1e4119c (git-prompt.sh: don't assume the shell expands the value
of PS1) is a fix that is in v2.0.0-rc4 but not v1.9.x.  Maybe add
something like:

 * The shell prompt script (in contrib/), when using Zsh and the
   precmd() interface, printed ${__git_ps1_branch_name} in the prompt
   instead of the branch name (regression in v1.9.3).
   (merge 1e4119c rh/prompt-pcmode-avoid-eval-on-refname later to
   maint).

-Richard
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] format-patch --signature-file file

[Jeremiah Mahler]

On Wed, May 21, 2014 at 02:24:27PM -0700, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 
...
 
 It seems you sent v7 before I had a chance to comment on this one.
 The above was merely it would be nicer... and I am OK as-is.  The
 comments on the rest are a bit more serious, though.
 
 Thanks.
 

It is not a problem.  Thanks again for all your feedback.

-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git push rejected due being behind after git svn dcommit without any changes local/remote

[Thomas Ferris Nicolaisen]

On Wed, May 21, 2014 at 6:13 PM, Henning Sprang
henning.spr...@gmail.com wrote:

 You'd say, can happen, when some developers work against the remote
 git repo, others with svn. But I'm the only developer on the project,
 and no one ever commits to the remote svn nor the remote git repo. So,
 I'm clueless.

 How can I find out what happens and how to prevent that?


This is normal. Every time you update or sync against the SVN server,
your local history is rewritten if you have local commits: All your
local commits are rebased on top of the latest changes coming from
SVN.

If you want to avoid this, only push to your Git mirror when your
git-svn clone and the SVN repo are in sync. Doing so ruins the whole
point of your Git mirror, I assume.

So, I'm afraid you have to get used to just force-pushing to your Git
mirror. Even though this is not considered good practice, it should be
fine as long as you are the only one using this mirror.

Alternatively, you could consider some other mechanism for backup (rsync, etc).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] Git v2.0.0-rc4

[Junio C Hamano]

Richard Hansen rhan...@bbn.com writes:

  * The shell prompt script (in contrib/), when using the PROMPT_COMMAND
interface, used an unsafe construct when showing the branch name in
$PS1.
(merge 8976500 rh/prompt-pcmode-avoid-eval-on-refname later to maint).

 That commit has been merged to maint and is in v1.9.3.

 Also, 1e4119c (git-prompt.sh: don't assume the shell expands the value
 of PS1) is a fix that is in v2.0.0-rc4 but not v1.9.x.  Maybe add
 something like:

  * The shell prompt script (in contrib/), when using Zsh and the
precmd() interface, printed ${__git_ps1_branch_name} in the prompt
instead of the branch name (regression in v1.9.3).
(merge 1e4119c rh/prompt-pcmode-avoid-eval-on-refname later to
maint).

Yes, already done this morning but not yet ready to be pushed out.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] format-patch --signature-file file

[Jeremiah Mahler]

On Wed, May 21, 2014 at 02:13:06PM -0700, Junio C Hamano wrote:
 Jeremiah Mahler jmmah...@gmail.com writes:
 
...
  ! grep ^-- \$ output
...
 
 We have been trying not to do the above in recent test updates.  It
 would be nice if this set-up did not have to be outside of the usual
 test_expect_success structure.
 

Jeff caught those ! grep instances in my patch.

I can submit a separate patch to fix instances like those in test cases
unrelated to this signature-file patch.

-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] format-patch --signature-file file

[Junio C Hamano]

Jeremiah Mahler jmmah...@gmail.com writes:

 On Wed, May 21, 2014 at 02:13:06PM -0700, Junio C Hamano wrote:
 Jeremiah Mahler jmmah...@gmail.com writes:
 
 ...
 ! grep ^-- \$ output
 ...
 
 We have been trying not to do the above in recent test updates.  It
 would be nice if this set-up did not have to be outside of the usual
 test_expect_success structure.
 

 Jeff caught those ! grep instances in my patch.

Hmm, I didn't mean that one, and I do not offhand what is wrong
about ! grep that says output should not contain this string.

The problem is a cat you added outside test_expect_*; the recent
push is to have as little executable outside them, especially the
set-up code to prepare for the real tests.

i.e. we have been trying to write new tests (and convert old ones)
like this:

test_expect_success 'I test such and such ' '
cat input-for-test -\EOF 
here comes input
EOF
git command-to-be-tested input-for-test actual 
cat expected -\EOF 
here comes expected output
EOF
test_cmp expected actual
'

not like this:

cat input-for-test -\EOF 
here comes input
EOF
test_expect_success 'I test such and such ' '
git command-to-be-tested input-for-test actual 
cat expected -\EOF 
here comes expected output
EOF
test_cmp expected actual
'
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 2/2] format-patch --signature-file file

[Junio C Hamano]

Jeremiah Mahler jmmah...@gmail.com writes:

 + if (signature_file) {
 + if (signature  signature != git_version_string)
 + die(_(cannot specify both signature and 
 signature-file));
 +
 + struct strbuf buf = STRBUF_INIT;

builtin/log.c:1460:3: error: ISO C90 forbids mixed declarations and
code [-Werror=declaration-after-statement]
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 30/44] refs.c: add transaction.status and track OPEN/CLOSED/ERROR

[Jonathan Nieder]

Ronnie Sahlberg wrote:

 --- a/refs.c
 +++ b/refs.c
 @@ -3308,6 +3308,12 @@ struct ref_update {
   const char refname[FLEX_ARRAY];
  };
  
 +enum ref_transaction_status {
 + REF_TRANSACTION_OPEN   = 0,
 + REF_TRANSACTION_CLOSED = 1,
 + REF_TRANSACTION_ERROR  = 2,

What is the difference between _TRANSACTION_CLOSED and
_TRANSACTION_ERROR?

[...]
 @@ -3340,6 +3347,11 @@ void ref_transaction_free(struct ref_transaction 
 *transaction)
  
  void ref_transaction_rollback(struct ref_transaction *transaction)
  {
 + if (!transaction)
 + return;
 +
 + transaction-status = REF_TRANSACTION_ERROR;
 +
   ref_transaction_free(transaction);

Once the transaction is freed, transaction-status is not reachable any
more so no one can tell that you've set it to _ERROR.  What is the
intended effect?

[...]
 @@ -3366,6 +3378,9 @@ int ref_transaction_update(struct ref_transaction 
 *transaction,
   if (have_old  !old_sha1)
   die(BUG: have_old is true but old_sha1 is NULL);
  
 + if (transaction-status != REF_TRANSACTION_OPEN)
 + die(BUG: update on transaction that is not open);

Ok.

[...]
 @@ -3538,6 +3564,9 @@ int ref_transaction_commit(struct ref_transaction 
 *transaction,
   clear_loose_ref_cache(ref_cache);
  
  cleanup:
 + transaction-status = ret ? REF_TRANSACTION_ERROR
 +   : REF_TRANSACTION_CLOSED;

Nit: odd use of whitespace.

Overall thoughts: I like the idea of enforcing the API more strictly
(after an error, the only permitted operations are...).  The details
leave me a little confused because I don't think anything is
distinguishing between _CLOSED and _ERROR.  Maybe the enum only needs
two states.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 31/44] refs.c: remove the update_ref_lock function

[Jonathan Nieder]

Ronnie Sahlberg wrote:

 Signed-off-by: Ronnie Sahlberg sahlb...@google.com
 ---
  refs.c | 30 ++
  1 file changed, 6 insertions(+), 24 deletions(-)

Reviewed-by: Jonathan Nieder jrnie...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] format-patch --signature-file file

[Jeff King]

On Wed, May 21, 2014 at 02:58:42PM -0700, Junio C Hamano wrote:

! grep ^-- \$ output
  ...
  
  We have been trying not to do the above in recent test updates.  It
  would be nice if this set-up did not have to be outside of the usual
  test_expect_success structure.
  
 
  Jeff caught those ! grep instances in my patch.
 
 Hmm, I didn't mean that one, and I do not offhand what is wrong
 about ! grep that says output should not contain this string.

I think he is responding to my earlier request to use test_must_fail
instead of !.  But there is a subtlety there he does not know, which
is that we typically only use the former for git programs, and rely on
! for normal Unix commands.

 The problem is a cat you added outside test_expect_*; the recent
 push is to have as little executable outside them, especially the
 set-up code to prepare for the real tests.
 
 i.e. we have been trying to write new tests (and convert old ones)
 like this:
 
 test_expect_success 'I test such and such ' '
 cat input-for-test -\EOF 
 here comes input
 EOF
 git command-to-be-tested input-for-test actual 
 cat expected -\EOF 
 here comes expected output
 EOF
 test_cmp expected actual
 '
 
 not like this:
 
 cat input-for-test -\EOF 
 here comes input
 EOF
 test_expect_success 'I test such and such ' '
 git command-to-be-tested input-for-test actual 
 cat expected -\EOF 
 here comes expected output
 EOF
 test_cmp expected actual
 '

Yeah, I noticed and gave a pass on this in earlier review, because the
file is used across many tests. So burying it in the first test that
uses it is probably a bad thing. However, it could go in its own setup
test.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 32/44] refs.c: remove the update_ref_write function

[Jonathan Nieder]

Ronnie Sahlberg wrote:

 +++ b/refs.c
[...]
 @@ -3518,14 +3499,16 @@ int ref_transaction_commit(struct ref_transaction 
 *transaction,
   struct ref_update *update = updates[i];
  
   if (!is_null_sha1(update-new_sha1)) {
 - ret = update_ref_write(msg,
 -update-refname,
 -update-new_sha1,
 -update-lock, err,
 -UPDATE_REFS_QUIET_ON_ERR);
 + ret = write_ref_sha1(update-lock, update-new_sha1,
 +  msg);

This changes the return value on error from 1 to -1.  That seems like a
good change.  It's probably worth mentioning in the commit message.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 33/44] refs.c: remove lock_ref_sha1

[Jonathan Nieder]

Ronnie Sahlberg wrote:

 Signed-off-by: Ronnie Sahlberg sahlb...@google.com
 ---
  refs.c | 15 +--
  1 file changed, 5 insertions(+), 10 deletions(-)

Heh. :)

Reviewed-by: Jonathan Nieder jrnie...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 30/44] refs.c: add transaction.status and track OPEN/CLOSED/ERROR

[Ronnie Sahlberg]

Please pull my ref-transactions branch.

On Wed, May 21, 2014 at 3:00 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Ronnie Sahlberg wrote:

 --- a/refs.c
 +++ b/refs.c
 @@ -3308,6 +3308,12 @@ struct ref_update {
   const char refname[FLEX_ARRAY];
  };

 +enum ref_transaction_status {
 + REF_TRANSACTION_OPEN   = 0,
 + REF_TRANSACTION_CLOSED = 1,
 + REF_TRANSACTION_ERROR  = 2,

 What is the difference between _TRANSACTION_CLOSED and
 _TRANSACTION_ERROR?

Closed is a transaction that has been committed successfully, and
which we can not do any more updates onto.
Error is a transaction that has failed, and which we can not do any
more updates onto.

The distinction could be useful if in the future we add support to
reuse a transaction


 [...]
 @@ -3340,6 +3347,11 @@ void ref_transaction_free(struct ref_transaction 
 *transaction)

  void ref_transaction_rollback(struct ref_transaction *transaction)
  {
 + if (!transaction)
 + return;
 +
 + transaction-status = REF_TRANSACTION_ERROR;
 +
   ref_transaction_free(transaction);

 Once the transaction is freed, transaction-status is not reachable any
 more so no one can tell that you've set it to _ERROR.  What is the
 intended effect?

ref_transaction_rollback is no more. It has been removed.


 [...]
 @@ -3366,6 +3378,9 @@ int ref_transaction_update(struct ref_transaction 
 *transaction,
   if (have_old  !old_sha1)
   die(BUG: have_old is true but old_sha1 is NULL);

 + if (transaction-status != REF_TRANSACTION_OPEN)
 + die(BUG: update on transaction that is not open);

 Ok.

 [...]
 @@ -3538,6 +3564,9 @@ int ref_transaction_commit(struct ref_transaction 
 *transaction,
   clear_loose_ref_cache(ref_cache);

  cleanup:
 + transaction-status = ret ? REF_TRANSACTION_ERROR
 +   : REF_TRANSACTION_CLOSED;

 Nit: odd use of whitespace.

fixed in ref-transactions.



 Overall thoughts: I like the idea of enforcing the API more strictly
 (after an error, the only permitted operations are...).  The details
 leave me a little confused because I don't think anything is
 distinguishing between _CLOSED and _ERROR.  Maybe the enum only needs
 two states.

A buggy caller might do :

transaction_begin()
transaction_update()
transaction_commit()  (A)
transaction_update() (B)
transaction_commit() (C)

After A the transaction in no longer open and until we decide we want
to add support for re-usable transactions (which may or may not be a
good idea) we need to make sure that both B and C fails.
Since the transaction in A completed successfully we can't really mark
is as ERROR, instead we flag it as CLOSED.




 Thanks,
 Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] format-patch --signature-file file

[Jeremiah Mahler]

On Wed, May 21, 2014 at 02:58:42PM -0700, Junio C Hamano wrote:
 Jeremiah Mahler jmmah...@gmail.com writes:
 
...
 The problem is a cat you added outside test_expect_*; the recent
 push is to have as little executable outside them, especially the
 set-up code to prepare for the real tests.
 
 i.e. we have been trying to write new tests (and convert old ones)
 like this:
 
 test_expect_success 'I test such and such ' '
 cat input-for-test -\EOF 
 here comes input
 EOF
 git command-to-be-tested input-for-test actual 
 cat expected -\EOF 
 here comes expected output
 EOF
 test_cmp expected actual
 '
 
 not like this:
 
 cat input-for-test -\EOF 
 here comes input
 EOF
 test_expect_success 'I test such and such ' '
 git command-to-be-tested input-for-test actual 
 cat expected -\EOF 
 here comes expected output
 EOF
 test_cmp expected actual
 '

Now I understand.

Below is one of the updated test cases.

test_expect_success 'format-patch --signature-file=mail-signature' '
cat mail-signature -\EOF

Test User test.em...@kernel.org
http://git.kernel.org/cgit/git/git.git

git.kernel.org/?p=git/git.git;a=summary

EOF
git format-patch --stdout --signature-file=mail-signature -1 output 
check_patch output 
sed -n -e /^-- $/,\$p output | sed -e 1d | sed -e \$d output2 
test_cmp mail-signature output2
'

-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH v4 1/3] add high resolution timer function to debug performance issues

[Richard Hansen]

On 2014-05-20 15:11, Karsten Blees wrote:
 Add a getnanotime() function that returns nanoseconds since 01/01/1970 as
 unsigned 64-bit integer (i.e. overflows in july 2554).

Must it be relative to epoch?  If it was relative to system boot (like
the NetBSD kernel's nanouptime() function), then you wouldn't have to
worry about clock adjustments messing with performance stats and you
might have more options for implementing getnanotime() on various platforms.

-Richard
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] format-patch --signature-file file

[Junio C Hamano]

Jeff King p...@peff.net writes:

 I think he is responding to my earlier request to use test_must_fail
 instead of !.  But there is a subtlety there he does not know, which
 is that we typically only use the former for git programs, and rely on
 ! for normal Unix commands.

Ok, that explains it.  Thanks.

 Yeah, I noticed and gave a pass on this in earlier review, because the
 file is used across many tests. So burying it in the first test that
 uses it is probably a bad thing. However, it could go in its own setup
 test.

Yeah, placing it in its own setup may be the best.  There are quite
a many set-ups outside the tests in this script from the olden days,
so I am OK if left it as-is and have a separate clean-up patch after
this topic settles.  I am also OK to add a new one the new right way
so that a later clean-up patch does not have to change what is added
in this step.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH v4 1/3] add high resolution timer function to debug performance issues

[Richard Hansen]

On 2014-05-21 18:14, Richard Hansen wrote:
 On 2014-05-20 15:11, Karsten Blees wrote:
 Add a getnanotime() function that returns nanoseconds since 01/01/1970 as
 unsigned 64-bit integer (i.e. overflows in july 2554).
 
 Must it be relative to epoch?  If it was relative to system boot (like
 the NetBSD kernel's nanouptime() function),

or relative to some other arbitrary reference point

 then you wouldn't have to
 worry about clock adjustments messing with performance stats and you
 might have more options for implementing getnanotime() on various platforms.
 
 -Richard
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 30/44] refs.c: add transaction.status and track OPEN/CLOSED/ERROR

[Jonathan Nieder]

Ronnie Sahlberg wrote:

 Please pull my ref-transactions branch.

I'm at bd5736cb (2014-05-21 13:46) now.

 On Wed, May 21, 2014 at 3:00 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Ronnie Sahlberg wrote:

 --- a/refs.c
 +++ b/refs.c
 @@ -3308,6 +3308,12 @@ struct ref_update {
   const char refname[FLEX_ARRAY];
  };

 +enum ref_transaction_status {
 + REF_TRANSACTION_OPEN   = 0,
 + REF_TRANSACTION_CLOSED = 1,
 + REF_TRANSACTION_ERROR  = 2,

 What is the difference between _TRANSACTION_CLOSED and
 _TRANSACTION_ERROR?

 Closed is a transaction that has been committed successfully, and
 which we can not do any more updates onto.
 Error is a transaction that has failed, and which we can not do any
 more updates onto.

That means that both mean the caller cannot do any more updates,
right?  Why not call them both _CLOSED?

 The distinction could be useful if in the future we add support to
 reuse a transaction

If the distinction becomes useful in the future, wouldn't that
be the moment to add a new state?

I don't mean that there has to be a big useful distinction.  E.g.,
maybe the idea is that when using a debugger it can be useful to see
the difference between _ERROR and _CLOSED or something, and I think
that would be fine as long as the intended meaning is documented
(e.g., using comments).  My only complaint is that it's hard to
understand the code and keep track of which status should be used in a
given place unless there is some distinction between them.

[...]
 ref_transaction_rollback is no more. It has been removed.

Thanks.  Sorry I missed that last time.

Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] format-patch --signature-file file

[Jeremiah Mahler]

On Wed, May 21, 2014 at 03:15:55PM -0700, Junio C Hamano wrote:
 Jeff King p...@peff.net writes:
 
 Yeah, placing it in its own setup may be the best.  There are quite
 a many set-ups outside the tests in this script from the olden days,
 so I am OK if left it as-is and have a separate clean-up patch after
 this topic settles.  I am also OK to add a new one the new right way
 so that a later clean-up patch does not have to change what is added
 in this step.

I like the idea of limiting the scope of this data so it couldn't
inadvertently impact later tests.

But placing the same data inside multiple test cases creates duplication.

Is there a way to define data once for a limited set of tests?

-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] format-patch --signature-file file

[Junio C Hamano]

Jeremiah Mahler jmmah...@gmail.com writes:

 Below is one of the updated test cases.

 test_expect_success 'format-patch --signature-file=mail-signature' '
   cat mail-signature -\EOF

   Test User test.em...@kernel.org
   http://git.kernel.org/cgit/git/git.git

   git.kernel.org/?p=git/git.git;a=summary

   EOF
   git format-patch --stdout --signature-file=mail-signature -1 output 
   check_patch output 
   sed -n -e /^-- $/,\$p output | sed -e 1d | sed -e \$d output2 
   test_cmp mail-signature output2
 '

Hmph, there are still few things I do not understand in the above.

 * Why does mail-signature file have a leading blank line?  Is it
   typical users would want to have one there?

 * Similarly, why does mail-signature file need a trailing blank
   line?  Is it usual users would want to have one there?

 * Why three sed in the pipeline?  Wouldn't

sed -e 1,/^-- \$/d output | ...

   be more direct way to start the pipeline without having to strip
   the -- \n line with the extra one?

 * Given a mail-signature file that does not end with an incomplete
   line (i.e. we did not have to add the newline to make it
   complete), what are the expected lines in the output after the
   -- \n separator?

   Whatever it is, I think it is easier to read the tests if done
   like so:

 sed -e 1,/^-- \$/d output actual 
 {
 do something to turn mail-signature to what we expect
 } expect 
 test_cmp expect actual

   If that do something is to append an extra newline, then it
   would make it a lot clear to do

 {
 cat mail-signature  echo
 } expect

   than a 'sed -e \$d' tucked at the end of the pipeline that only
   tells us we are removing one line that has _something_ without
   explicitly saying what we are removing, no?

Thanks.



   
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] format-patch --signature-file file

[Junio C Hamano]

Jeremiah Mahler jmmah...@gmail.com writes:

 On Wed, May 21, 2014 at 03:15:55PM -0700, Junio C Hamano wrote:
 Jeff King p...@peff.net writes:
 
 Yeah, placing it in its own setup may be the best.  There are quite
 a many set-ups outside the tests in this script from the olden days,
 so I am OK if left it as-is and have a separate clean-up patch after
 this topic settles.  I am also OK to add a new one the new right way
 so that a later clean-up patch does not have to change what is added
 in this step.

 I like the idea of limiting the scope of this data so it couldn't
 inadvertently impact later tests.

 But placing the same data inside multiple test cases creates duplication.

 Is there a way to define data once for a limited set of tests?

That is what Jeff ment by used across many tests. ... it could go
in its own setup.

In other words,

test_expect_success 'prepare mail-signature input' '
cat mail-signature -\EOF
...
EOF
'

test_expect_success 'one test that uses mail-signature' '
use mail-signature 
test the output
'

test_expect_success 'another test that uses mail-signature' '
use mail-signature in a different way 
test the output
'

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 34/44] refs.c: make prune_ref use a transaction to delete the ref

[Jonathan Nieder]

Ronnie Sahlberg wrote:

 Change prune_ref to delete the ref using a ref transaction. To do this we also
 need to add a new flag REF_ISPRUNING that will tell the transaction that we
 do not want to delete this ref from the packed refs.

Interesting.  Since the flag is per ref update, it even would allow
deleting some refs and pruning others in the same transaction.  Makes
sense.

Looks like this doesn't batch up multiple ref-prunings into a single
transaction.  Makes sense (it would just make the period while refs
are locked longer without having any real benefit).

[...]
 +#define REF_ISPRUNING0x0100

Can this conflict with bit values declared elsewhere some day?  It
would be more comfortable if refs.h also had a note about bits =
0x100 being reserved for private use.

The rest of the patch looks good.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] format-patch --signature-file file

[Jeremiah Mahler]

On Wed, May 21, 2014 at 03:37:11PM -0700, Junio C Hamano wrote:
 Jeremiah Mahler jmmah...@gmail.com writes:
 
  Below is one of the updated test cases.
 
  test_expect_success 'format-patch --signature-file=mail-signature' '
  cat mail-signature -\EOF
 
  Test User test.em...@kernel.org
  http://git.kernel.org/cgit/git/git.git
 
  git.kernel.org/?p=git/git.git;a=summary
 
  EOF
  git format-patch --stdout --signature-file=mail-signature -1 output 
  check_patch output 
  sed -n -e /^-- $/,\$p output | sed -e 1d | sed -e \$d output2 
  test_cmp mail-signature output2
  '
 
 Hmph, there are still few things I do not understand in the above.
 
  * Why does mail-signature file have a leading blank line?  Is it
typical users would want to have one there?
 
  * Similarly, why does mail-signature file need a trailing blank
line?  Is it usual users would want to have one there?
 

I think whatever the user puts in their signature should be reproduced.
It is probably isn't common to have leading or trailing blank lines.
But if they are there they should be reproduced.

  * Why three sed in the pipeline?  Wouldn't
 
 sed -e 1,/^-- \$/d output | ...
 
be more direct way to start the pipeline without having to strip
the -- \n line with the extra one?
 

Yes, that is much cleaner.

  * Given a mail-signature file that does not end with an incomplete
line (i.e. we did not have to add the newline to make it
complete), what are the expected lines in the output after the
-- \n separator?
 
Whatever it is, I think it is easier to read the tests if done
like so:
 
  sed -e 1,/^-- \$/d output actual 
  {
  do something to turn mail-signature to what we expect
  } expect 
  test_cmp expect actual
 
If that do something is to append an extra newline, then it
would make it a lot clear to do
 
  {
  cat mail-signature  echo
  } expect
 
than a 'sed -e \$d' tucked at the end of the pipeline that only
tells us we are removing one line that has _something_ without
explicitly saying what we are removing, no?
 

This does make more sense.  Since Git prints a blank line when it prints
the signature, this test also adds a blank line to produce the same
expected result.  This is much better than deleting two lines for no
obvious reason.

-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 35/44] refs.c: make delete_ref use a transaction

[Jonathan Nieder]

Ronnie Sahlberg wrote:

 --- a/refs.c
 +++ b/refs.c
[...]
 @@ -2515,24 +2510,18 @@ static int delete_ref_loose(struct ref_lock *lock, 
 int flag, struct strbuf *err)
  
  int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
  {
 - struct ref_lock *lock;
 - int ret = 0, flag = 0;
 + struct ref_transaction *transaction;
  
[...]
 - lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
 + if (!transaction ||
 + ref_transaction_delete(transaction, refname, sha1, delopt,
 +sha1  !is_null_sha1(sha1), err) ||

What should happen when is_null_sha1(sha1)?  In that case the
caller has asked us to check that the ref doesn't exist before
deleting it, which doesn't make a lot of sense, but the old
code did exactly that if I read it correctly.  The new code
seems to disable verification instead.

Do we know that no callers call delete_ref with such an argument?
Would it make sense to just die with a BUG: message to make
the contract more clear?

[...]
 - unlink_or_warn(git_path(logs/%s, lock-ref_name));

When does the reflog get deleted in the new code?

Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 36/44] refs.c: pass the ref log message to _create/delete/update instead of _commit

[Jonathan Nieder]

Ronnie Sahlberg wrote:

 Change the reference transactions so that we pass the reflog message
 through to the create/delete/update function instead of the commit message.

Nice.

[...]
 --- a/builtin/fetch.c
 +++ b/builtin/fetch.c
 @@ -673,7 +673,6 @@ static int store_updated_refs(const char *raw_url, const 
 char *remote_name,
   }
   }
   }
 -
   if (rc  STORE_REF_ERROR_DF_CONFLICT)
   error(_(some local refs could not be updated; try running\n
  'git remote prune %s' to remove any old, conflicting 

Stray whitespace change?

[...]
 --- a/refs.c
 +++ b/refs.c
[...]
 @@ -3264,6 +3264,7 @@ struct ref_update {
   int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
   struct ref_lock *lock;
   int type;
 + const char *msg;
   const char refname[FLEX_ARRAY];

Should be 'char *msg' since we own the memory (or perhaps a strbuf).

[...]
 @@ -3297,9 +3298,10 @@ void ref_transaction_free(struct ref_transaction 
 *transaction)
   if (!transaction)
   return;
  
 - for (i = 0; i  transaction-nr; i++)
 + for (i = 0; i  transaction-nr; i++) {
 +   free((char *)transaction-updates[i]-msg);
   free(transaction-updates[i]);

Whitespace?

No need to cast.

The rest of the patch looks good.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 37/44] refs.c: pass NULL as *flags to read_ref_full

[Jonathan Nieder]

Ronnie Sahlberg wrote:

 Signed-off-by: Ronnie Sahlberg sahlb...@google.com
 ---
  refs.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Jonathan Nieder jrnie...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 38/44] refs.c: pack all refs before we start to rename a ref

[Jonathan Nieder]

Ronnie Sahlberg wrote:

 This means that most loose refs will no longer be present after the rename

Is this to handle the git branch -m foo/bar foo case or for some other
purpose?

[...]
 --- a/t/t3200-branch.sh
 +++ b/t/t3200-branch.sh
 @@ -289,7 +289,7 @@ test_expect_success 'renaming a symref is not allowed' '
   git symbolic-ref refs/heads/master2 refs/heads/master 
   test_must_fail git branch -m master2 master3 
   git symbolic-ref refs/heads/master2 
 - test_path_is_file .git/refs/heads/master 
 + test_path_is_missing .git/refs/heads/master 
   test_path_is_missing .git/refs/heads/master3

It's kind of silly that this test is mucking about in the .git directory
at all.  Shouldn't the check be something like

git rev-parse --verify refs/heads/master 
test_must_fail git symbolic-ref refs/heads/master3 
test_must_fail git rev-parse refs/heads/master3

?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/9] strbuf: add strbuf_tolower function

[Kyle J. McKay]

On May 21, 2014, at 03:27, Jeff King wrote:


This makes config's lowercase() function public.

Note that we could continue to offer a pure-string
lowercase, but there would be no callers (in most
pure-string cases, we actually duplicate and lowercase the
duplicate).

Signed-off-by: Jeff King p...@peff.net
---
This ones gets used later on...

Documentation/technical/api-strbuf.txt | 4 
config.c   | 8 +---
strbuf.c   | 7 +++
strbuf.h   | 1 +
4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/ 
technical/api-strbuf.txt

index 3350d97..8480f89 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -125,6 +125,10 @@ Functions

Strip whitespace from the end of a string.

+`strbuf_tolower`::
+
+   Lowercase each character in the buffer using `tolower`.
+
`strbuf_cmp`::

	Compare two buffers. Returns an integer less than, equal to, or  
greater

diff --git a/config.c b/config.c
index a30cb5c..03ce5c6 100644
--- a/config.c
+++ b/config.c
@@ -147,12 +147,6 @@ int git_config_include(const char *var, const  
char *value, void *data)

return ret;
}

-static void lowercase(char *p)
-{
-   for (; *p; p++)
-   *p = tolower(*p);
-}
-
void git_config_push_parameter(const char *text)
{
struct strbuf env = STRBUF_INIT;
@@ -180,7 +174,7 @@ int git_config_parse_parameter(const char *text,
strbuf_list_free(pair);
return error(bogus config parameter: %s, text);
}
-   lowercase(pair[0]-buf);
+   strbuf_tolower(pair[0]);
if (fn(pair[0]-buf, pair[1] ? pair[1]-buf : NULL, data)  0) {
strbuf_list_free(pair);
return -1;
diff --git a/strbuf.c b/strbuf.c
index ee96dcf..a1b8a47 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -106,6 +106,13 @@ void strbuf_ltrim(struct strbuf *sb)
sb-buf[sb-len] = '\0';
}

+void strbuf_tolower(struct strbuf *sb)
+{
+   size_t i;
+   for (i = 0; i  sb-len; i++)
+   sb-buf[i] = tolower(sb-buf[i]);
+}
+


Wouldn't a direct transfer of the lowercase function be something more  
like:



void strbuf_tolower(struct strbuf *sb)
{
char *p = sb-buf;
for (; *p; p++)
*p = tolower(*p);
}

That seems to me to be a bit more efficient.  According to the  
comments in strbuf.c, people can always assume buf is non NULL and - 
buf is NUL terminated even for a freshly initialized strbuf.



struct strbuf **strbuf_split_buf(const char *str, size_t slen,
 int terminator, int max)
{
diff --git a/strbuf.h b/strbuf.h
index 39c14cf..6b6f745 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -45,6 +45,7 @@ static inline void strbuf_setlen(struct strbuf  
*sb, size_t len)

extern void strbuf_trim(struct strbuf *);
extern void strbuf_rtrim(struct strbuf *);
extern void strbuf_ltrim(struct strbuf *);
+extern void strbuf_tolower(struct strbuf *sb);
extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);

/*
--
2.0.0.rc1.436.g03cb729



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

[Kyle J. McKay]

On May 21, 2014, at 03:33, Jeff King wrote:


Commit 426e70d (remote-curl: show server content on http
errors, 2013-04-05) tried to recognize text/plain content
types, but fails to do so if they have any parameters.

This patches makes our parsing more liberal, though we still
do not do anything useful with a charset parameter.

Signed-off-by: Jeff King p...@peff.net
---
remote-curl.c  | 26 ++
t/lib-httpd/error.sh   |  8 +++-
t/t5550-http-fetch-dumb.sh |  5 +
3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index a5ab977..6d1b206 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -194,20 +194,30 @@ static void free_discovery(struct discovery *d)
}
}

+/*
+ * We only show text/plain parts, as other types are likely
+ * to be ugly to look at on the user's terminal.
+ */
+static int content_type_is_terminal_friendly(struct strbuf *type)
+{
+   const char *p;
+
+   p = skip_prefix(type-buf, text/plain);
+   if (!p || (*p  *p != ';'))
+   return 0;
+
+   return 1;
+}
+


I think that a strict reading of RFC 2616 allows text/plain ;  
charset=utf-8 as well as text/plain;charset=utf-8 and text/plain;  
charset=utf-8.  So perhaps this if line instead:


+   if (!p || (*p  *p != ';'  *p != ' '  *p != '\t'))

See RFC 2616 section 2.2 for the definition of linear white space  
(LWS) and the end of section 2.1 for the implied *LWS rule that  
allows it.



static int show_http_message(struct strbuf *type, struct strbuf *msg)
{
const char *p, *eol;

-   /*
-* We only show text/plain parts, as other types are likely
-* to be ugly to look at on the user's terminal.
-*
-* TODO should handle ; charset=XXX, and re-encode into
-* logoutputencoding
-*/
-   if (strcmp(type-buf, text/plain))
+   if (!content_type_is_terminal_friendly(type))
return -1;

+	/* TODO should record charset and reencode msg to  
logOutputEncoding */

+
strbuf_trim(msg);
if (!msg-len)
return -1;
diff --git a/t/lib-httpd/error.sh b/t/lib-httpd/error.sh
index 786f281..02e80b3 100755
--- a/t/lib-httpd/error.sh
+++ b/t/lib-httpd/error.sh
@@ -3,6 +3,7 @@
printf Status: 500 Intentional Breakage\n

printf Content-Type: 
+charset=iso8859-1
case $PATH_INFO in
*html*)
printf text/html
@@ -10,8 +11,13 @@ case $PATH_INFO in
*text*)
printf text/plain
;;
+*charset*)
+   printf text/plain; charset=utf-8
+   charset=utf-8
+   ;;
esac
printf \n

printf \n
-printf this is the error message\n
+printf this is the error message\n |
+iconv -f us-ascii -t $charset
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 13defd3..b35b261 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -181,5 +181,10 @@ test_expect_success 'git client does not show  
html errors' '

! grep this is the error message stderr
'

+test_expect_success 'git client shows text/plain with a charset' '
+   test_must_fail git clone $HTTPD_URL/error/charset 2stderr 
+   grep this is the error message stderr
+'
+
stop_httpd
test_done
--
2.0.0.rc1.436.g03cb729



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 9/9] remote-curl: reencode http error messages

[Kyle J. McKay]

On May 21, 2014, at 03:33, Jeff King wrote:


As of the last commit, we now recognize an error message
with a content-type text/plain; charset=utf-16 as text,
but we ignore the charset parameter entirely. Let's encode
it to log_output_encoding, which is presumably something the
user's terminal can handle.

Signed-off-by: Jeff King p...@peff.net
---
remote-curl.c  | 37 +
t/lib-httpd/error.sh   |  4 
t/t5550-http-fetch-dumb.sh |  5 +
3 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 6d1b206..1dc90d7 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -194,11 +194,34 @@ static void free_discovery(struct discovery *d)
}
}

+static char *find_param(const char *str, const char *name)
+{
+   int len = strlen(name);
+
+   for (; *str; str++) {
+   const char *p;
+
+   if (*p++ != ' ')
+   continue;
+
+   if (strncmp(p, name, len))
+   continue;
+   p += len;
+
+   if (*p++ != '=')
+   continue;
+
+   return xstrndup(p, strchrnul(p, ' ') - p);
+   }
+
+   return NULL;
+}
+
/*
 * We only show text/plain parts, as other types are likely
 * to be ugly to look at on the user's terminal.
 */
-static int content_type_is_terminal_friendly(struct strbuf *type)
+static int content_type_is_terminal_friendly(struct strbuf *type,  
char **charset)

{
const char *p;

@@ -206,17 +229,23 @@ static int  
content_type_is_terminal_friendly(struct strbuf *type)

if (!p || (*p  *p != ';'))
return 0;

+   *charset = find_param(p, charset);
+   /* default charset from rfc2616 */
+   if (!*charset)
+   *charset = xstrdup(iso8859-1);


Actually the name should be ISO-8859-1.  See RFC 2616 section  
3.7.1.  Since it's case insensitive iso-8859-1 would be fine too.



+
return 1;
}

static int show_http_message(struct strbuf *type, struct strbuf *msg)
{
const char *p, *eol;
+   char *charset;

-   if (!content_type_is_terminal_friendly(type))
+   if (!content_type_is_terminal_friendly(type, charset))
return -1;
-
-	/* TODO should record charset and reencode msg to  
logOutputEncoding */

+   strbuf_reencode(msg, charset, get_log_output_encoding());
+   free(charset);

strbuf_trim(msg);
if (!msg-len)
diff --git a/t/lib-httpd/error.sh b/t/lib-httpd/error.sh
index 02e80b3..4efbce7 100755
--- a/t/lib-httpd/error.sh
+++ b/t/lib-httpd/error.sh
@@ -15,6 +15,10 @@ case $PATH_INFO in
printf text/plain; charset=utf-8
charset=utf-8
;;
+*utf16*)
+   printf text/plain; charset=utf-16
+   charset=utf-16
+   ;;
esac
printf \n

diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index b35b261..01b8aae 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -186,5 +186,10 @@ test_expect_success 'git client shows text/ 
plain with a charset' '

grep this is the error message stderr
'

+test_expect_success 'http error messages are reencoded' '
+   test_must_fail git clone $HTTPD_URL/error/utf16 2stderr 
+   grep this is the error message stderr
+'
+
stop_httpd
test_done
--
2.0.0.rc1.436.g03cb729


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH v4 3/3] add command performance tracing to debug scripted commands

[Karsten Blees]

Am 21.05.2014 18:55, schrieb Jeff King:
 On Tue, May 20, 2014 at 09:11:24PM +0200, Karsten Blees wrote:
 
 Add performance tracing to identify which git commands are called and how
 long they execute. This is particularly useful to debug performance issues
 of scripted commands.

 Usage example:  GIT_TRACE_PERFORMANCE=~/git-trace.log git stash list

 Creates a log file like this:
 performance: at trace.c:319, time: 0.000303280 s: git command: 'git' 
 'rev-parse' '--git-dir'
 performance: at trace.c:319, time: 0.000334409 s: git command: 'git' 
 'rev-parse' '--is-inside-work-tree'
 performance: at trace.c:319, time: 0.000215243 s: git command: 'git' 
 'rev-parse' '--show-toplevel'
 performance: at trace.c:319, time: 0.000410639 s: git command: 'git' 
 'config' '--get-colorbool' 'color.interactive'
 performance: at trace.c:319, time: 0.000394077 s: git command: 'git' 
 'config' '--get-color' 'color.interactive.help' 'red bold'
 performance: at trace.c:319, time: 0.000280701 s: git command: 'git' 
 'config' '--get-color' '' 'reset'
 performance: at trace.c:319, time: 0.000908185 s: git command: 'git' 
 'rev-parse' '--verify' 'refs/stash'
 performance: at trace.c:319, time: 0.028827774 s: git command: 'git' 'stash' 
 'list'
 
 Neat. I actually wanted something like this just yesterday. It looks
 like you are mainly tracing the execution of programs. Would it make
 sense to just tie this to regular trace_* calls, and if
 GIT_TRACE_PERFORMANCE is set, add a timestamp to each line?
 
 Then we would not need to add separate trace_command_performance calls,
 and other parts of the code that are already instrumented with GIT_TRACE
 would get the feature for free.
 
 -Peff
 

IMO printing timestamps in all trace output would be a useful feature on its 
own, but its not what I'm trying to achieve here. Timestamps only give you a 
broad overview of when things started, not how long they took. And calculating 
the durations from timestamps in the log output is tedious work, esp. if 
multiple processes and threads are involved (would need pid and thread-id as 
well).

The first patch helps calculating durations (without having to mess with struct 
timespec/timeval), and the second helps logging the results. Its basically a 
utility for manual profiling. Printing total command execution time (this 
patch) is just one possible use case.

E.g. if I'm interested in a particular code section, I throw in 2 lines of code 
(before and after the code section). This gives very accurate results, without 
significantly affecting overall performance. I can then push the changes to my 
Linux/Windows box and get comparable results there. No need to disable 
optimization. No worries that the profiling tool isn't available on the other 
platform. No analyzing megabytes of mostly irrelevant profiling data.

Does that make sense?

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH v4 1/3] add high resolution timer function to debug performance issues

[Karsten Blees]

Am 22.05.2014 00:14, schrieb Richard Hansen:
 On 2014-05-20 15:11, Karsten Blees wrote:
 Add a getnanotime() function that returns nanoseconds since 01/01/1970 as
 unsigned 64-bit integer (i.e. overflows in july 2554).
 
 Must it be relative to epoch?  If it was relative to system boot (like
 the NetBSD kernel's nanouptime() function), then you wouldn't have to
 worry about clock adjustments messing with performance stats and you
 might have more options for implementing getnanotime() on various platforms.
 
 -Richard
 

Normalizing to the epoch adds the ability to use the same timestamps (div 10e9) 
in other time-related functions (e.g. gmtime, ctime etc.), with very little 
overhead (one 64-bit integer addition per call).

The getnanotime() implementation is actually platform independent and can be 
backed by any time source that returns nanoseconds relative to anything. 
Getnanotime() is synced to the system clock only once on startup, so if your 
time source is monotonic (which I think NetBSD's nanouptime() is), you don't 
have to worry about clock adjustments.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 39/44] refs.c: move the check for valid refname to lock_ref_sha1_basic

[Jonathan Nieder]

Ronnie Sahlberg wrote:
 --- a/refs.c
 +++ b/refs.c
 @@ -2044,6 +2044,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
 *refname,
   int missing = 0;
   int attempts_remaining = 3;
  
 + if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
 + return NULL;

Are we sure that no callers are locking a poorly named ref
as preparation for repairing it?

To see what works already in that vein, I tried:

$ git rev-parse HEAD .git/refs/heads/foo..bar
$ git branch -m foo..bar something-saner
fatal: Invalid branch name: 'foo..bar'

git branch -m has an explicit codepath (recovery = 1;) to handle
this case, but it looks like it was not well tested and regressed in
v1.7.8-rc0~19^2~7 (resolve_ref(): verify that the input refname has
the right format, 2011-09-15).

Is what the recovery codepath of branch -m does misguided?  One
school of thought would be that people with malformed refs are
responsible for recovering using low-level tools like mv and vi
instead of normal git commands since normal git commands should never
have created such a bad situation.  Another school of thought would
assert that

 * git commands can have bugs
 * the format checked by check_refname_format() keeps getting stricter
   over time

which means it's nice when people can recover with 'update-ref -d'
or 'branch -m'.  It's not obvious to me what the right thing to do
here is (maybe a special flag to be attached to a ref update during
recovery?).

Hope that helps,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v8 0/2] format-patch --signature-file file

[Jeremiah Mahler]

v8 of patch to add format-patch --signature-file file option.

This revision includes more suggestions from Jeff King and Junio C Hamano.

  - Renamed variable which stores the signature to 'mail-signature'
which better describes what is being stored.

  - Fixed a declaration after code (forbidden by ISO C90).

  - Simplified sed usage in test cases from a pipeline with
three stages to just one.

  - Enclosed the creation of test data ('cat mail-signature') inside
a test_expect_* case.

  - Test cases now add one blank line to the actual signature to match
the signature output of Git.  Previously, the last of the output
was deleted.

Thanks again for all the suggestions.

Jeff King (1):
  format-patch: make newline after signature conditional

Jeremiah Mahler (1):
  format-patch --signature-file file

 Documentation/config.txt   |  4 
 Documentation/git-format-patch.txt |  4 
 builtin/log.c  | 25 +--
 t/t4014-format-patch.sh| 41 ++
 4 files changed, 72 insertions(+), 2 deletions(-)

-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v8 1/2] format-patch: make newline after signature conditional

[Jeremiah Mahler]

From: Jeff King p...@peff.net

When we print an email signature, we print the divider --
\n, then the signature string, then two newlines.
Traditionally the signature is a one-liner (and the default
is just the git version), so the extra newline makes sense.

But one could easily specify a longer, multi-line signature,
like:

  git format-patch --signature='
  this is my long signature

  it has multiple lines
  ' ...

We should notice that it already has its own trailing
newline, and suppress one of ours.

Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 builtin/log.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..5acc048 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -844,8 +844,13 @@ static void gen_message_id(struct rev_info *info, char 
*base)
 
 static void print_signature(void)
 {
-   if (signature  *signature)
-   printf(-- \n%s\n\n, signature);
+   if (!signature || !*signature)
+   return;
+
+   printf(-- \n%s, signature);
+   if (signature[strlen(signature)-1] != '\n')
+   putchar('\n');
+   putchar('\n');
 }
 
 static void add_branch_description(struct strbuf *buf, const char *branch_name)
-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v8 2/2] format-patch --signature-file file

[Jeremiah Mahler]

Added option that allows a signature file to be used with format-patch
so that signatures with newlines and other special characters can be
easily included.

  $ git format-patch --signature-file ~/.signature -1

The config variable format.signaturefile is also provided so that it
can be added by default.

  $ git config format.signaturefile ~/.signature

  $ git format-patch -1

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 Documentation/config.txt   |  4 
 Documentation/git-format-patch.txt |  4 
 builtin/log.c  | 16 +++
 t/t4014-format-patch.sh| 41 ++
 4 files changed, 65 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1932e9b..140ed77 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1114,6 +1114,10 @@ format.signature::
Set this variable to the empty string () to suppress
signature generation.
 
+format.signaturefile::
+   Works just like format.signature except the contents of the
+   file specified by this variable will be used as the signature.
+
 format.suffix::
The default for format-patch is to output files with the suffix
`.patch`. Use this variable to change that suffix (make sure to
diff --git a/Documentation/git-format-patch.txt 
b/Documentation/git-format-patch.txt
index 5c0a4ab..c0fd470 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -14,6 +14,7 @@ SYNOPSIS
   [(--attach|--inline)[=boundary] | --no-attach]
   [-s | --signoff]
   [--signature=signature | --no-signature]
+  [--signature-file=file]
   [-n | --numbered | -N | --no-numbered]
   [--start-number n] [--numbered-files]
   [--in-reply-to=Message-Id] [--suffix=.sfx]
@@ -233,6 +234,9 @@ configuration options in linkgit:git-notes[1] to use this 
workflow).
signature option is omitted the signature defaults to the Git version
number.
 
+--signature-file=file::
+   Works just like --signature except the signature is read from a file.
+
 --suffix=.sfx::
Instead of using `.patch` as the suffix for generated
filenames, use specified suffix.  A common alternative is
diff --git a/builtin/log.c b/builtin/log.c
index 5acc048..5e3cc29 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -673,6 +673,7 @@ static void add_header(const char *value)
 static int thread;
 static int do_signoff;
 static const char *signature = git_version_string;
+static const char *signature_file;
 static int config_cover_letter;
 
 enum {
@@ -742,6 +743,8 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
}
if (!strcmp(var, format.signature))
return git_config_string(signature, var, value);
+   if (!strcmp(var, format.signaturefile))
+   return git_config_pathname(signature_file, var, value);
if (!strcmp(var, format.coverletter)) {
if (value  !strcasecmp(value, auto)) {
config_cover_letter = COVER_AUTO;
@@ -1235,6 +1238,8 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
PARSE_OPT_OPTARG, thread_callback },
OPT_STRING(0, signature, signature, N_(signature),
N_(add a signature)),
+   OPT_FILENAME(0, signature-file, signature_file,
+   N_(add a signature from a file)),
OPT__QUIET(quiet, N_(don't print the patch filenames)),
OPT_END()
};
@@ -1452,6 +1457,17 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
cover_letter = (config_cover_letter == COVER_ON);
}
 
+   if (signature_file) {
+   struct strbuf buf = STRBUF_INIT;
+
+   if (signature  signature != git_version_string)
+   die(_(cannot specify both signature and 
signature-file));
+
+   if (strbuf_read_file(buf, signature_file, 128)  0)
+   die_errno(_(unable to read signature file '%s'), 
signature_file);
+   signature = strbuf_detach(buf, NULL);
+   }
+
if (in_reply_to || thread || cover_letter)
rev.ref_message_ids = xcalloc(1, sizeof(struct string_list));
if (in_reply_to) {
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 9c80633..37d25c4 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -762,6 +762,47 @@ test_expect_success 'format-patch --signature= 
suppresses signatures' '
! grep ^-- \$ output
 '
 
+test_expect_success 'prepare mail-signature input' '
+   cat mail-signature -\EOF
+
+   Test User test.em...@kernel.org
+   http://git.kernel.org/cgit/git/git.git
+
+   

Re: [ANNOUNCE] Git v2.0.0-rc4

[Felipe Contreras]

Junio C Hamano wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:
 
  Junio C Hamano wrote:
 
   * The remote-helper interface to fast-import/fast-export via the
 transport-helper has been tightened to avoid leaving the import
 marks file from a failed/crashed run, as such a file that is out-of-
 sync with reality confuses a later invocation of itself.
 
  Really? Where are the patches for that?
 
  I think it's fair to say the way the remote-helpers and transport-helper
  has been handled for v2.0 has been a total disaster.
 
 Thanks for noticing.  The last-minute change of plans in the morning
 on the -rc release day did not help.  Will remove.

But this changed before that.

 Anything else I missed?

Not as far as I can see.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[no subject]

[Christian Organization]

Good day,

We are Christian organization, we give loan to those who are dedicated
Christians, contact us at mercantilefinanceloanserv...@yahoo.com

Regard

Mercantile



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/9] strbuf: add strbuf_tolower function

[Jeff King]

On Wed, May 21, 2014 at 05:07:36PM -0700, Kyle J. McKay wrote:

 +void strbuf_tolower(struct strbuf *sb)
 +{
 +size_t i;
 +for (i = 0; i  sb-len; i++)
 +sb-buf[i] = tolower(sb-buf[i]);
 +}
 +
 
 Wouldn't a direct transfer of the lowercase function be something more like:
 
 
 void strbuf_tolower(struct strbuf *sb)
 {
   char *p = sb-buf;
   for (; *p; p++)
   *p = tolower(*p);
 }
 
 That seems to me to be a bit more efficient.  According to the comments in
 strbuf.c, people can always assume buf is non NULL and -buf is NUL
 terminated even for a freshly initialized strbuf.

Yes, and that would be fine with me (I actually wrote strbuf_tolower for
my own use, and _then_ realized that we already had such a thing that
could be replaced).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html