Re: git log --no-walk --tags produces strange result for certain user

2014-01-17 Thread Michael Haggerty
On 01/16/2014 11:31 AM, Michael Haggerty wrote:
 On 12/16/2013 12:52 PM, Kirill Likhodedov wrote:
 I received one more complaint for this issue, and now it appears in a public 
 repository https://github.com/spray/spray 

 To reproduce:

 # git clone https://github.com/spray/spray 
 # cd spray
 # git log --no-walk --tags --pretty=%H %d --decorate=full | tail -3
 3273edafcd9f9701d62e061c5257c0a09e2e1fb7  (tag: refs/tags/v0.8.0-RC1)
 ff3a2946bc54da76ddb47e82c81419cc7ae3db6b  (tag: refs/tags/v0.7.0)
 8b4043428b90b7f45b7241b3c2c032cf785479ce 

 So here the last hash doesn't have a decoration.
 
 The problem is that reference refs/tags/v0.5.0 points at a tag object
 8f6ca98087 which itself points at another tag object 2eddbcbff4 which
 finally points at commit 8b4043428b.  Probably we should handle
 recursive tag objects like this, but OTOH I can't think of a reason why
 one would want to create them in the first place.

Junio just pointed out to me that this bug has been fixed already, by
Brian Carlson, in 5e1361cc, which is already in master.  Sorry for the
noise.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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: with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream

2014-01-17 Thread John Keeping
On Thu, Jan 16, 2014 at 06:47:38PM -0800, Siddharth Agarwal wrote:
 On 01/16/2014 06:21 PM, Jeff King wrote:
  On Thu, Jan 16, 2014 at 05:08:14PM -0800, Siddharth Agarwal wrote:
 
  With git-next, where git pull --rebase can print out fatal: No such
  ref: '' if git pull --rebase is run on branches without an upstream.
  This is already fixed in bb3f458 (rebase: fix fork-point with zero
  arguments, 2014-01-09), I think.
 
 If I'm reading the patch correctly, that only fixes it for git rebase, 
 not for git pull --rebase. git-pull.sh contains a separate invocation of 
 git merge-base --fork-point.

I'm pretty sure the invocation in git-pull.sh is OK.  The error then
comes out of git-rebase.sh when git-pull invokes it.

Are you running a version of git-next that includes bb3f458?
--
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 0/3] improved out-of-the-box color settings

2014-01-17 Thread Yuri

On 01/16/2014 20:14, Jeff King wrote:

The second patch turns on MORE=R only for FreeBSD. Yuri, if you can
confirm that my patch works for you, that would be excellent. And if you
(or anyone) has access to NetBSD or OpenBSD to test the second patch on,
I'd welcome updates to config.mak.uname for those systems.


I applied 3 patches, and it fixed the issue for me.

Thank you for such a fast response! This is very impressive!
I saw this issue for a long while, and only last night took time to 
report it.


Unfortunately, I don't have any other BSDs sitting around. The easiest 
way to try this is to install them in VM.


Yuri
--
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 quietly fails on https:// URL, https errors are never reported to user

2014-01-17 Thread Yuri

On 01/16/2014 10:03, Jeff King wrote:

We used to print Reading from helper 'git-remote-https' failed in this
instance. But in the majority of cases, remote-https has printed a
useful message already to stderr, and the extra line just confused
people. The downside, as you noticed, is that when the helper dies
without printing an error, the user is left with no message.


I would like to suggest to return this printout, see patch below. This 
would be a revert of this commit:

commit 266f1fdfa99f5d29ca7ce455966e7960c00a82e4
Author: Jeff King p...@peff.net
Date:   Fri Jun 21 03:05:39 2013 -0400

I think that in a rare case of error this extra-printout wouldn't hurt. 
I also made this message more user friendly, without mentioning the term 
helper.


Yuri

diff --git a/transport-helper.c b/transport-helper.c
index 2010674..5ea2831 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -58,7 +58,7 @@ static int recvline_fh(FILE *helper, struct strbuf 
*buffer, const char *name)

if (strbuf_getline(buffer, helper, '\n') == EOF) {
if (debug)
fprintf(stderr, Debug: Remote helper quit.\n);
-   exit(128);
+   die(Failure in '%s' protocol reader, name);
}

if (debug)

--
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/WIP v2 00/14] inotify support

2014-01-17 Thread Nguyễn Thái Ngọc Duy
This is getting in better shape. Still wondering if the design is
right, so documentation, tests and some error cases are still
neglected. I have not addressed Jonathan's and Jeff's comments in this
reroll, but I haven't forgotten them yet. The test suite seems to be
fine when file-watcher is forced on with GIT_TEST_FORCE_WATCHER set..

Thomas, you were a proponent of per-user daemon last time. I agree
that is a better solution when you need to support submodules. So if
you have time, have a look and see if anything I did may prevent
per-user daemon changes later (hint, I have a few unfriendly exit() in
file-watcher.c). You also worked with inotify before maybe you can
help spot some mishandling too as I'm totally new to inotify.

Nguyễn Thái Ngọc Duy (14):
  read-cache: save trailing sha-1
  read-cache: new extension to mark what file is watched
  read-cache: connect to file watcher
  read-cache: ask file watcher to watch files
  read-cache: put some limits on file watching
  read-cache: get modified file list from file watcher
  read-cache: add config to start file watcher automatically
  read-cache: add GIT_TEST_FORCE_WATCHER for testing
  file-watcher: add --shutdown and --log options
  file-watcher: automatically quit
  file-watcher: support inotify
  file-watcher: exit when cwd is gone
  pkt-line.c: increase buffer size to 8192
  t1301: exclude sockets from file permission check

 .gitignore   |   1 +
 Documentation/config.txt |  14 ++
 Makefile |   2 +
 cache.h  |   8 +
 config.mak.uname |   1 +
 file-watcher-lib.c (new) | 109 +++
 file-watcher-lib.h (new) |   9 +
 file-watcher.c (new) | 483 +++
 git-compat-util.h|   3 +
 pkt-line.c   |   4 +-
 pkt-line.h   |   2 +
 read-cache.c | 338 -
 t/t1301-shared-repo.sh   |   3 +-
 trace.c  |   3 +-
 14 files changed, 969 insertions(+), 11 deletions(-)
 create mode 100644 file-watcher-lib.c
 create mode 100644 file-watcher-lib.h
 create mode 100644 file-watcher.c

-- 
1.8.5.1.208.g05b12ea

--
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/WIP v2 02/14] read-cache: new extension to mark what file is watched

2014-01-17 Thread Nguyễn Thái Ngọc Duy
If an entry is watched, git lets an external program decide if the
entry is modified or not. It's more like --assume-unchanged, but
designed to be controlled by machine.

We are running out of on-disk ce_flags, so instead of extending
on-disk entry format again, watched flags are in-core only and
stored as extension instead.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 cache.h  |  2 ++
 read-cache.c | 41 -
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index a09d622..069dce7 100644
--- a/cache.h
+++ b/cache.h
@@ -168,6 +168,8 @@ struct cache_entry {
 /* used to temporarily mark paths matched by pathspecs */
 #define CE_MATCHED   (1  26)
 
+#define CE_WATCHED   (1  27)
+
 /*
  * Extended on-disk flags
  */
diff --git a/read-cache.c b/read-cache.c
index fe1d153..6f21e3f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -33,6 +33,7 @@ static struct cache_entry *refresh_cache_entry(struct 
cache_entry *ce, int reall
 #define CACHE_EXT(s) ( (s[0]24)|(s[1]16)|(s[2]8)|(s[3]) )
 #define CACHE_EXT_TREE 0x54524545  /* TREE */
 #define CACHE_EXT_RESOLVE_UNDO 0x52455543 /* REUC */
+#define CACHE_EXT_WATCH 0x57415443   /* WATC */
 
 struct index_state the_index;
 
@@ -1293,6 +1294,19 @@ static int verify_hdr(struct cache_header *hdr,
return 0;
 }
 
+static void read_watch_extension(struct index_state *istate, uint8_t *data,
+unsigned long sz)
+{
+   int i;
+   if ((istate-cache_nr + 7) / 8 != sz) {
+   error(invalid 'WATC' extension);
+   return;
+   }
+   for (i = 0; i  istate-cache_nr; i++)
+   if (data[i / 8]  (1  (i % 8)))
+   istate-cache[i]-ce_flags |= CE_WATCHED;
+}
+
 static int read_index_extension(struct index_state *istate,
const char *ext, void *data, unsigned long sz)
 {
@@ -1303,6 +1317,9 @@ static int read_index_extension(struct index_state 
*istate,
case CACHE_EXT_RESOLVE_UNDO:
istate-resolve_undo = resolve_undo_read(data, sz);
break;
+   case CACHE_EXT_WATCH:
+   read_watch_extension(istate, data, sz);
+   break;
default:
if (*ext  'A' || 'Z'  *ext)
return error(index uses %.4s extension, which we do 
not understand,
@@ -1781,7 +1798,7 @@ int write_index(struct index_state *istate, int newfd)
 {
git_SHA_CTX c;
struct cache_header hdr;
-   int i, err, removed, extended, hdr_version;
+   int i, err, removed, extended, hdr_version, has_watches = 0;
struct cache_entry **cache = istate-cache;
int entries = istate-cache_nr;
struct stat st;
@@ -1790,6 +1807,8 @@ int write_index(struct index_state *istate, int newfd)
for (i = removed = extended = 0; i  entries; i++) {
if (cache[i]-ce_flags  CE_REMOVE)
removed++;
+   else if (cache[i]-ce_flags  CE_WATCHED)
+   has_watches++;
 
/* reduce extended entries if possible */
cache[i]-ce_flags = ~CE_EXTENDED;
@@ -1861,6 +1880,26 @@ int write_index(struct index_state *istate, int newfd)
if (err)
return -1;
}
+   if (has_watches) {
+   int id, sz = (entries - removed + 7) / 8;
+   uint8_t *data = xmalloc(sz);
+   memset(data, 0, sz);
+   for (i = 0, id = 0; i  entries  has_watches; i++) {
+   struct cache_entry *ce = cache[i];
+   if (ce-ce_flags  CE_REMOVE)
+   continue;
+   if (ce-ce_flags  CE_WATCHED) {
+   data[id / 8] |= 1  (id % 8);
+   has_watches--;
+   }
+   id++;
+   }
+   err = write_index_ext_header(c, newfd, CACHE_EXT_WATCH, sz)  0
+   || ce_write(c, newfd, data, sz)  0;
+   free(data);
+   if (err)
+   return -1;
+   }
 
if (ce_flush(c, newfd) || fstat(newfd, st))
return -1;
-- 
1.8.5.1.208.g05b12ea

--
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/WIP v2 01/14] read-cache: save trailing sha-1

2014-01-17 Thread Nguyễn Thái Ngọc Duy
This will be used as signature to know if the index has changed.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 cache.h  | 1 +
 read-cache.c | 7 ---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index 323481c..a09d622 100644
--- a/cache.h
+++ b/cache.h
@@ -279,6 +279,7 @@ struct index_state {
 initialized : 1;
struct hashmap name_hash;
struct hashmap dir_hash;
+   unsigned char sha1[20];
 };
 
 extern struct index_state the_index;
diff --git a/read-cache.c b/read-cache.c
index 3f735f3..fe1d153 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1273,10 +1273,11 @@ struct ondisk_cache_entry_extended {
ondisk_cache_entry_extended_size(ce_namelen(ce)) : \
ondisk_cache_entry_size(ce_namelen(ce)))
 
-static int verify_hdr(struct cache_header *hdr, unsigned long size)
+static int verify_hdr(struct cache_header *hdr,
+ unsigned long size,
+ unsigned char *sha1)
 {
git_SHA_CTX c;
-   unsigned char sha1[20];
int hdr_version;
 
if (hdr-hdr_signature != htonl(CACHE_SIGNATURE))
@@ -1465,7 +1466,7 @@ int read_index_from(struct index_state *istate, const 
char *path)
close(fd);
 
hdr = mmap;
-   if (verify_hdr(hdr, mmap_size)  0)
+   if (verify_hdr(hdr, mmap_size, istate-sha1)  0)
goto unmap;
 
istate-version = ntohl(hdr-hdr_version);
-- 
1.8.5.1.208.g05b12ea

--
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/WIP v2 07/14] read-cache: add config to start file watcher automatically

2014-01-17 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/config.txt |  5 +
 file-watcher-lib.c   | 18 +++---
 file-watcher-lib.h   |  2 +-
 file-watcher.c   |  8 ++--
 read-cache.c | 17 +++--
 5 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e394399..3316b69 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1038,6 +1038,11 @@ difftool.tool.cmd::
 difftool.prompt::
Prompt before each invocation of the diff tool.
 
+filewatcher.autorun::
+   Run `git file-watcher` automatically if the number of cached
+   entries is greater than this limit. Zero means no running
+   file-watcher automatically. Default value is zero.
+
 filewatcher.minfiles::
Start watching files if the number of watchable files are
above this limit. Default value is 65536.
diff --git a/file-watcher-lib.c b/file-watcher-lib.c
index ed14ef9..71c8545 100644
--- a/file-watcher-lib.c
+++ b/file-watcher-lib.c
@@ -1,16 +1,28 @@
 #include cache.h
+#include run-command.h
 
 #define WAIT_TIME 20   /* in ms */
 #define TRACE_KEY GIT_TRACE_WATCHER
 
-int connect_watcher(const char *path)
+int connect_watcher(const char *path, int autorun)
 {
struct strbuf sb = STRBUF_INIT;
struct stat st;
-   int fd = -1;
+   int fd = -1, ret;
 
strbuf_addf(sb, %s.watcher, path);
-   if (!stat(sb.buf, st)  S_ISSOCK(st.st_mode)) {
+   ret = stat(sb.buf, st);
+   if (autorun  ret  errno == ENOENT) {
+   const char *av[] = { file-watcher, --daemon, --quiet, 
NULL };
+   struct child_process cp;
+   memset(cp, 0, sizeof(cp));
+   cp.git_cmd = 1;
+   cp.argv = av;
+   if (run_command(cp))
+   return -1;
+   ret = stat(sb.buf, st);
+   }
+   if (!ret  S_ISSOCK(st.st_mode)) {
struct sockaddr_un sun;
fd = socket(AF_UNIX, SOCK_DGRAM, 0);
sun.sun_family = AF_UNIX;
diff --git a/file-watcher-lib.h b/file-watcher-lib.h
index 0fe9399..ef3d196 100644
--- a/file-watcher-lib.h
+++ b/file-watcher-lib.h
@@ -1,7 +1,7 @@
 #ifndef __FILE_WATCHER_LIB__
 #define __FILE_WATCHER_LIB__
 
-int connect_watcher(const char *path);
+int connect_watcher(const char *path, int autorun);
 ssize_t send_watcher(int sockfd, struct sockaddr_un *dest,
 const char *fmt, ...)
__attribute__((format (printf, 3, 4)));
diff --git a/file-watcher.c b/file-watcher.c
index 369af37..1b4ac0a 100644
--- a/file-watcher.c
+++ b/file-watcher.c
@@ -168,8 +168,9 @@ int main(int argc, const char **argv)
struct pollfd pfd[2];
int fd, err, nr;
const char *prefix;
-   int daemon = 0;
+   int daemon = 0, quiet = 0;
struct option options[] = {
+   OPT__QUIET(quiet, N_(be quiet)),
OPT_BOOL(0, daemon, daemon,
 N_(run in background)),
OPT_END()
@@ -189,8 +190,11 @@ int main(int argc, const char **argv)
fd = socket(AF_UNIX, SOCK_DGRAM, 0);
sun.sun_family = AF_UNIX;
strlcpy(sun.sun_path, socket_path, sizeof(sun.sun_path));
-   if (bind(fd, (struct sockaddr *)sun, sizeof(sun)))
+   if (bind(fd, (struct sockaddr *)sun, sizeof(sun))) {
+   if (quiet)
+   exit(128);
die_errno(unable to bind to %s, socket_path);
+   }
atexit(cleanup);
sigchain_push_common(cleanup_on_signal);
 
diff --git a/read-cache.c b/read-cache.c
index 3aa541d..5dae9eb 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -40,6 +40,7 @@ static struct cache_entry *refresh_cache_entry(struct 
cache_entry *ce, int reall
 struct index_state the_index;
 static int watch_lowerlimit = 65536;
 static int recent_limit = 1800;
+static int autorun_watcher = -1;
 
 static void set_index_entry(struct index_state *istate, int nr, struct 
cache_entry *ce)
 {
@@ -1518,6 +1519,10 @@ failed:
 
 static int watcher_config(const char *var, const char *value, void *data)
 {
+   if (!strcmp(var, filewatcher.autorun)) {
+   autorun_watcher = git_config_int(var, value);
+   return 0;
+   }
if (!strcmp(var, filewatcher.minfiles)) {
watch_lowerlimit = git_config_int(var, value);
return 0;
@@ -1538,8 +1543,16 @@ static void validate_watcher(struct index_state *istate, 
const char *path)
return;
}
 
-   git_config(watcher_config, NULL);
-   istate-watcher = connect_watcher(path);
+   if (autorun_watcher == -1) {
+   git_config(watcher_config, NULL);
+   if (autorun_watcher == -1)
+   autorun_watcher = 0;
+   }
+
+   istate-watcher = connect_watcher(path,
+ 

[PATCH/WIP v2 06/14] read-cache: get modified file list from file watcher

2014-01-17 Thread Nguyễn Thái Ngọc Duy
A new command is added to file watcher to send back the list of
updated files to git. These entries will have CE_WATCHED removed. The
remaining CE_WATCHED entries will have CE_VALID set (i.e. no changes
and no lstat either).

The file watcher does not cache stat info and send back to git. Its
main purpose is to reduce lstat on most untouched files, not to
completely eliminate lstat.

The file watcher keeps reporting the same updated list until it
receives forget commands, which should only be issued after the
updated index is written down. This ensures that if git crashes half
way before it could update the index (or multiple processes is reading
the same index), updated info is not lost.

After the index is updated (e.g. in this case because of toggling
CE_WATCHED bits), git sends the new index signature to the file
watcher.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 cache.h|   1 +
 file-watcher.c |  63 +---
 read-cache.c   | 100 +++--
 3 files changed, 157 insertions(+), 7 deletions(-)

diff --git a/cache.h b/cache.h
index bcec29b..8f065ed 100644
--- a/cache.h
+++ b/cache.h
@@ -284,6 +284,7 @@ struct index_state {
struct hashmap dir_hash;
unsigned char sha1[20];
int watcher;
+   struct string_list *updated_entries;
 };
 
 extern struct index_state the_index;
diff --git a/file-watcher.c b/file-watcher.c
index 3a54168..369af37 100644
--- a/file-watcher.c
+++ b/file-watcher.c
@@ -3,6 +3,7 @@
 #include parse-options.h
 #include exec_cmd.h
 #include file-watcher-lib.h
+#include string-list.h
 #include pkt-line.h
 
 static const char *const file_watcher_usage[] = {
@@ -11,6 +12,8 @@ static const char *const file_watcher_usage[] = {
 };
 
 static char index_signature[41];
+static struct string_list updated = STRING_LIST_INIT_DUP;
+static int updated_sorted;
 
 static int watch_path(char *path)
 {
@@ -23,6 +26,37 @@ static int watch_path(char *path)
return -1;
 }
 
+static void reset(void)
+{
+   string_list_clear(updated, 0);
+   index_signature[0] = '\0';
+}
+
+static void send_status(int fd, struct sockaddr_un *sun)
+{
+   struct strbuf sb = STRBUF_INIT;
+   int i, size;
+   socklen_t vallen = sizeof(size);
+   if (getsockopt(fd, SOL_SOCKET, SO_SNDBUF, size, vallen))
+   die_errno(could not get SO_SNDBUF from socket %d, fd);
+
+   strbuf_grow(sb, size);
+   strbuf_addstr(sb, new );
+
+   for (i = 0; i  updated.nr; i++) {
+   int len = strlen(updated.items[i].string) + 4;
+   if (sb.len + len = size) {
+   send_watcher(fd, sun, %s, sb.buf);
+   strbuf_reset(sb);
+   strbuf_addstr(sb, new );
+   }
+   packet_buf_write(sb, %s, updated.items[i].string);
+   }
+   strbuf_addstr(sb, );
+   send_watcher(fd, sun, %s, sb.buf);
+   strbuf_release(sb);
+}
+
 static void watch_paths(char *buf, int maxlen,
int fd, struct sockaddr_un *sock)
 {
@@ -40,6 +74,19 @@ static void watch_paths(char *buf, int maxlen,
send_watcher(fd, sock, fine %d, n);
 }
 
+static void remove_updated(const char *path)
+{
+   struct string_list_item *item;
+   if (!updated_sorted) {
+   sort_string_list(updated);
+   updated_sorted = 1;
+   }
+   item = string_list_lookup(updated, path);
+   if (!item)
+   return;
+   unsorted_string_list_delete_item(updated, item - updated.items, 0);
+}
+
 static int handle_command(int fd)
 {
struct sockaddr_un sun;
@@ -53,11 +100,17 @@ static int handle_command(int fd)
if ((arg = skip_prefix(msg, hello ))) {
send_watcher(fd, sun, hello %s, index_signature);
if (strcmp(arg, index_signature))
-   /*
-* Index SHA-1 mismatch, something has gone
-* wrong. Clean up and start over.
-*/
-   index_signature[0] = '\0';
+   reset();
+   } else if ((arg = skip_prefix(msg, clear))) {
+   reset();
+   } else if (!strcmp(msg, status)) {
+   send_status(fd, sun);
+   } else if ((arg = skip_prefix(msg, bye ))) {
+   strlcpy(index_signature, arg, sizeof(index_signature));
+   } else if ((arg = skip_prefix(msg, forget ))) {
+   int len = strlen(index_signature);
+   if (!strncmp(arg, index_signature, len)  arg[len] == ' ')
+   remove_updated(arg + len + 1);
} else if (starts_with(msg, watch )) {
watch_paths(msg + 6, len - 6, fd, sun);
} else if (!strcmp(msg, die)) {
diff --git a/read-cache.c b/read-cache.c
index 406834a..3aa541d 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1453,6 +1453,69 @@ static struct cache_entry 

[PATCH/WIP v2 08/14] read-cache: add GIT_TEST_FORCE_WATCHER for testing

2014-01-17 Thread Nguyễn Thái Ngọc Duy
This can be used to force watcher on when running the test
suite.

git-file-watcher processes are not automatically cleaned up after each
test. So after running the test suite you'll be left with plenty
git-file-watcher processes that should all end after about a minute.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 read-cache.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/read-cache.c b/read-cache.c
index 5dae9eb..a1245d4 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1544,7 +1544,12 @@ static void validate_watcher(struct index_state *istate, 
const char *path)
}
 
if (autorun_watcher == -1) {
-   git_config(watcher_config, NULL);
+   if (getenv(GIT_TEST_FORCE_WATCHER)) {
+   watch_lowerlimit = 0;
+   recent_limit = 0;
+   autorun_watcher = 1;
+   } else
+   git_config(watcher_config, NULL);
if (autorun_watcher == -1)
autorun_watcher = 0;
}
-- 
1.8.5.1.208.g05b12ea

--
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/WIP v2 10/14] file-watcher: automatically quit

2014-01-17 Thread Nguyễn Thái Ngọc Duy
If $GIT_DIR/index.watcher or $GIT_DIR/index is gone, exit. We could
watch this path too, but we'll waste precious resources (at least with
inotify). And with inotify, it seems to miss the case when $GIT_DIR is
moved. Just check if the socket path still exists every minute.

As the last resort, if we do not receive any commands in the last 6
hours, exit. The code is structured this way because later on inotify
is also polled. On an busy watched directory, the timeout may never
happen for us to kil the watcher, even if index.watcher is already
gone.

For mass cleanup, killall -USR1 git-file-watcher asks every watcher
process to question the purpose of its existence.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 file-watcher.c | 38 +++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/file-watcher.c b/file-watcher.c
index df06529..f334e23 100644
--- a/file-watcher.c
+++ b/file-watcher.c
@@ -163,6 +163,12 @@ static void daemonize(void)
 #endif
 }
 
+static int check_exit_please;
+static void check_exit_signal(int signo)
+{
+   check_exit_please = 1;
+}
+
 int main(int argc, const char **argv)
 {
struct strbuf sb = STRBUF_INIT;
@@ -172,6 +178,8 @@ int main(int argc, const char **argv)
const char *prefix;
int daemon = 0, quiet = 0, shutdown = 0;
const char *log_string = NULL;
+   struct stat socket_st;
+   struct timeval tv_last_command;
struct option options[] = {
OPT__QUIET(quiet, N_(be quiet)),
OPT_BOOL(0, daemon, daemon,
@@ -217,6 +225,10 @@ int main(int argc, const char **argv)
 
atexit(cleanup);
sigchain_push_common(cleanup_on_signal);
+   sigchain_push(SIGUSR1, check_exit_signal);
+
+   if (stat(socket_path, socket_st))
+   die_errno(failed to stat %s, socket_path);
 
if (daemon) {
strbuf_addf(sb, %s.log, socket_path);
@@ -234,17 +246,37 @@ int main(int argc, const char **argv)
pfd[nr].fd = fd;
pfd[nr++].events = POLLIN;
 
+   gettimeofday(tv_last_command, NULL);
for (;;) {
-   if (poll(pfd, nr, -1)  0) {
+   int check_exit = check_exit_please;
+   int ret = poll(pfd, nr, check_exit ? 0 : 60 * 1000);
+   if (ret  0) {
if (errno != EINTR) {
error(Poll failed, resuming: %s, 
strerror(errno));
sleep(1);
}
continue;
+   } else if (!ret)
+   check_exit = 1;
+
+   if ((pfd[0].revents  POLLIN)) {
+   if (handle_command(fd))
+   break;
+   gettimeofday(tv_last_command, NULL);
}
 
-   if ((pfd[0].revents  POLLIN)  handle_command(fd))
-   break;
+   if (check_exit) {
+   struct stat st;
+   struct timeval now;
+   gettimeofday(now, NULL);
+   if (tv_last_command.tv_sec + 6 * 60  now.tv_sec)
+   break;
+   if (stat(socket_path, st) ||
+   st.st_ino != socket_st.st_ino ||
+   stat(get_index_file(), st))
+   break;
+   check_exit_please = 0;
+   }
}
return 0;
 }
-- 
1.8.5.1.208.g05b12ea

--
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/WIP v2 11/14] file-watcher: support inotify

2014-01-17 Thread Nguyễn Thái Ngọc Duy
git diff on webkit:

no file watcher  1st run   subsequent runs
real0m1.361s0m1.445s  0m0.691s
user0m0.889s0m0.940s  0m0.649s
sys 0m0.469s0m0.495s  0m0.040s

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 config.mak.uname  |   1 +
 file-watcher.c| 194 ++
 git-compat-util.h |   3 +
 3 files changed, 198 insertions(+)

diff --git a/config.mak.uname b/config.mak.uname
index 7d31fad..ee548f5 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -33,6 +33,7 @@ ifeq ($(uname_S),Linux)
HAVE_PATHS_H = YesPlease
LIBC_CONTAINS_LIBINTL = YesPlease
HAVE_DEV_TTY = YesPlease
+   BASIC_CFLAGS += -DHAVE_INOTIFY
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
NO_STRLCPY = YesPlease
diff --git a/file-watcher.c b/file-watcher.c
index f334e23..356b58a 100644
--- a/file-watcher.c
+++ b/file-watcher.c
@@ -15,6 +15,166 @@ static char index_signature[41];
 static struct string_list updated = STRING_LIST_INIT_DUP;
 static int updated_sorted;
 
+#ifdef HAVE_INOTIFY
+
+static struct string_list watched_dirs = STRING_LIST_INIT_DUP;
+static int watched_dirs_sorted;
+static int inotify_fd;
+
+struct dir_info {
+   int wd;
+   struct string_list names;
+   int names_sorted;
+};
+
+static void reset_watches(void)
+{
+   int i;
+   for (i = 0; i  watched_dirs.nr; i++) {
+   struct dir_info *dir = watched_dirs.items[i].util;
+   inotify_rm_watch(inotify_fd, dir-wd);
+   string_list_clear(dir-names, 0);
+   }
+   string_list_clear(watched_dirs, 1);
+}
+
+static void update(const char *base, const char *name)
+{
+   if (!strcmp(base, .))
+   string_list_append(updated, name);
+   else {
+   static struct strbuf sb = STRBUF_INIT;
+   strbuf_addf(sb, %s/%s, base, name);
+   string_list_append(updated, sb.buf);
+   strbuf_reset(sb);
+   }
+   updated_sorted = 0;
+}
+
+static int do_handle_inotify(const struct inotify_event *event)
+{
+   struct dir_info *dir;
+   struct string_list_item *item;
+   int i;
+
+   if (event-mask  (IN_Q_OVERFLOW | IN_UNMOUNT)) {
+   /*
+* The connectionless nature of file watcher means we
+* can never tell we are reset in the middle of a
+* session because there are no sessions. Close
+* the socket so all clients can react on it.
+*/
+   exit(0);
+   }
+
+   /* Should have indexed them for faster access like trast's watch */
+   for (i = 0; i  watched_dirs.nr; i++) {
+   struct dir_info *dir = watched_dirs.items[i].util;
+   if (dir-wd == event-wd)
+   break;
+   }
+   if (i == watched_dirs.nr)
+   return 0;
+   dir = watched_dirs.items[i].util;
+
+   /*
+* If something happened to the watched directory, consider
+* everything inside modified
+*/
+   if (event-mask  (IN_DELETE_SELF | IN_MOVE_SELF)) {
+   int dir_idx = i;
+   for (i = 0; i  dir-names.nr; i++)
+   update(watched_dirs.items[dir_idx].string,
+  dir-names.items[i].string);
+   inotify_rm_watch(inotify_fd, dir-wd);
+   unsorted_string_list_delete_item(watched_dirs, dir_idx, 1);
+   return 0;
+   }
+
+   if (!dir-names_sorted) {
+   sort_string_list(dir-names);
+   dir-names_sorted = 1;
+   }
+   item = string_list_lookup(dir-names, event-name);
+   if (item) {
+   update(watched_dirs.items[i].string, item-string);
+   unsorted_string_list_delete_item(dir-names,
+item - dir-names.items, 0);
+   if (dir-names.nr == 0) {
+   inotify_rm_watch(inotify_fd, dir-wd);
+   unsorted_string_list_delete_item(watched_dirs, i, 1);
+   }
+   }
+   return 0;
+}
+
+static int handle_inotify(int fd)
+{
+   static char buf[10 * (sizeof(struct inotify_event) + NAME_MAX + 1)];
+   struct inotify_event *event;
+   int offset = 0;
+   int len = read(fd, buf, sizeof(buf));
+   if (len = 0)
+   return -1;
+   for (event = (struct inotify_event *)(buf + offset);
+offset  len;
+offset += sizeof(struct inotify_event) + event-len) {
+   if (do_handle_inotify(event))
+   return -1;
+   }
+   return 0;
+}
+
+static int watch_path(char *path)
+{
+   struct string_list_item *item;
+   char *sep = strrchr(path, '/');
+   struct dir_info *dir;
+   const char *dirname = .;
+
+   if (sep) {
+   *sep = '\0';
+   dirname = path;
+   }
+
+  

[PATCH/WIP v2 03/14] read-cache: connect to file watcher

2014-01-17 Thread Nguyễn Thái Ngọc Duy
This patch establishes a connection between a new file watcher daemon
and git. Each index file may have at most one file watcher attached to
it. The file watcher maintains a UNIX socket at
$GIT_DIR/index.watcher. Any process that has write access to $GIT_DIR
can talk to the file watcher.

A validation is performed after git connects to the file watcher to
make sure both sides have the same view. This is done by exchanging
the index signature (*) The file watcher keeps a copy of the signature
locally while git computes the signature from the index. If the
signatures do not match, something has gone wrong so both sides
reinitialize wrt. to file watching: the file watcher clears all
watches while git clears CE_WATCHED flags.

If the signatures match, we can trust the file watcher and git can
start asking questions that are not important to this patch.

This file watching thing is all about speed. So if the daemon is not
responding within 20ms (or even hanging), git moves on without it.

A note about per-repo vs global (or per-user) daemon approach. While I
implement per-repo daemon, this is actually implementation
details. Nothing can stop you from writing a global daemon that opens
unix sockets to many repos, e.g. to avoid hitting inotify's 128 user
instances limit.

If env variable GIT_NO_FILE_WATCHER is set, the file watcher is
ignored. 'WATC' extension is kept, but if the index is updated
(likely), it'll become invalid at the next connection.

(*) for current index versions, the signature is the index SHA-1
trailer. But it could be something else (e.g. v5 does not have SHA-1
trailer)

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 .gitignore   |   1 +
 Makefile |   2 +
 cache.h  |   3 +
 file-watcher-lib.c (new) |  97 
 file-watcher-lib.h (new) |   9 +++
 file-watcher.c (new) | 142 +++
 read-cache.c |  37 
 trace.c  |   3 +-
 8 files changed, 292 insertions(+), 2 deletions(-)
 create mode 100644 file-watcher-lib.c
 create mode 100644 file-watcher-lib.h
 create mode 100644 file-watcher.c

diff --git a/.gitignore b/.gitignore
index dc600f9..dc870cc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -56,6 +56,7 @@
 /git-fast-import
 /git-fetch
 /git-fetch-pack
+/git-file-watcher
 /git-filter-branch
 /git-fmt-merge-msg
 /git-for-each-ref
diff --git a/Makefile b/Makefile
index 287e6f8..4369b3b 100644
--- a/Makefile
+++ b/Makefile
@@ -536,6 +536,7 @@ PROGRAMS += $(EXTRA_PROGRAMS)
 PROGRAM_OBJS += credential-store.o
 PROGRAM_OBJS += daemon.o
 PROGRAM_OBJS += fast-import.o
+PROGRAM_OBJS += file-watcher.o
 PROGRAM_OBJS += http-backend.o
 PROGRAM_OBJS += imap-send.o
 PROGRAM_OBJS += sh-i18n--envsubst.o
@@ -798,6 +799,7 @@ LIB_OBJS += entry.o
 LIB_OBJS += environment.o
 LIB_OBJS += exec_cmd.o
 LIB_OBJS += fetch-pack.o
+LIB_OBJS += file-watcher-lib.o
 LIB_OBJS += fsck.o
 LIB_OBJS += gettext.o
 LIB_OBJS += gpg-interface.o
diff --git a/cache.h b/cache.h
index 069dce7..0d1 100644
--- a/cache.h
+++ b/cache.h
@@ -282,6 +282,7 @@ struct index_state {
struct hashmap name_hash;
struct hashmap dir_hash;
unsigned char sha1[20];
+   int watcher;
 };
 
 extern struct index_state the_index;
@@ -1241,6 +1242,8 @@ extern void alloc_report(void);
 __attribute__((format (printf, 1, 2)))
 extern void trace_printf(const char *format, ...);
 __attribute__((format (printf, 2, 3)))
+extern void trace_printf_key(const char *key, const char *fmt, ...);
+__attribute__((format (printf, 2, 3)))
 extern void trace_argv_printf(const char **argv, const char *format, ...);
 extern void trace_repo_setup(const char *prefix);
 extern int trace_want(const char *key);
diff --git a/file-watcher-lib.c b/file-watcher-lib.c
new file mode 100644
index 000..ed14ef9
--- /dev/null
+++ b/file-watcher-lib.c
@@ -0,0 +1,97 @@
+#include cache.h
+
+#define WAIT_TIME 20   /* in ms */
+#define TRACE_KEY GIT_TRACE_WATCHER
+
+int connect_watcher(const char *path)
+{
+   struct strbuf sb = STRBUF_INIT;
+   struct stat st;
+   int fd = -1;
+
+   strbuf_addf(sb, %s.watcher, path);
+   if (!stat(sb.buf, st)  S_ISSOCK(st.st_mode)) {
+   struct sockaddr_un sun;
+   fd = socket(AF_UNIX, SOCK_DGRAM, 0);
+   sun.sun_family = AF_UNIX;
+   strlcpy(sun.sun_path, sb.buf, sizeof(sun.sun_path));
+   if (connect(fd, (struct sockaddr *)sun, sizeof(sun))) {
+   error(_(unable to connect to file watcher: %s),
+ strerror(errno));
+   close(fd);
+   fd = -1;
+   } else {
+   sprintf(sun.sun_path, %c%PRIuMAX, 0, 
(uintmax_t)getpid());
+   if (bind(fd, (struct sockaddr *)sun, sizeof(sun))) {
+   error(_(unable to bind socket: %s),
+

[PATCH/WIP v2 04/14] read-cache: ask file watcher to watch files

2014-01-17 Thread Nguyễn Thái Ngọc Duy
We want to watch files that are never changed because lstat() on those
files is a wasted effort. So we sort unwatched files by date and start
adding them to the file watcher until it barfs (e.g. hits inotify
limit). Recently updated entries are also excluded from watch list.
CE_VALID is used in combination with CE_WATCHED. Those entries that
have CE_VALID already set will never be watched.

We send as many paths as possible in one packet in pkt-line format to
reduce roundtrips. For small projects like git, all entries can be
packed in one packet. For large projects like webkit (182k entries) it
takes two packets. We may do prefix compression as well to send more
in fewer packets..

The file watcher replies how many entries it can watch (because at
least inotify has system limits).

Note that we still do lstat() on these new watched files because they
could have changed before the file watcher could watch them. Watched
files may only skip lstat() at the next git run.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 file-watcher.c |  31 
 pkt-line.c |   2 +-
 pkt-line.h |   2 ++
 read-cache.c   | 111 +++--
 4 files changed, 143 insertions(+), 3 deletions(-)

diff --git a/file-watcher.c b/file-watcher.c
index 36a9a8d..3a54168 100644
--- a/file-watcher.c
+++ b/file-watcher.c
@@ -3,6 +3,7 @@
 #include parse-options.h
 #include exec_cmd.h
 #include file-watcher-lib.h
+#include pkt-line.h
 
 static const char *const file_watcher_usage[] = {
N_(git file-watcher [options]),
@@ -11,6 +12,34 @@ static const char *const file_watcher_usage[] = {
 
 static char index_signature[41];
 
+static int watch_path(char *path)
+{
+   /*
+* Consider send wait every 10ms or so, in case there are
+* many paths to process that takes more than 20ms or the
+* sender won't keep waiting. This is usually one-time cost,
+* waiting a bit is ok.
+*/
+   return -1;
+}
+
+static void watch_paths(char *buf, int maxlen,
+   int fd, struct sockaddr_un *sock)
+{
+   char *end = buf + maxlen;
+   int n, ret, len;
+   for (n = ret = 0; buf  end  !ret; buf += len) {
+   char ch;
+   len = packet_length(buf);
+   ch = buf[len];
+   buf[len] = '\0';
+   if (!(ret = watch_path(buf + 4)))
+   n++;
+   buf[len] = ch;
+   }
+   send_watcher(fd, sock, fine %d, n);
+}
+
 static int handle_command(int fd)
 {
struct sockaddr_un sun;
@@ -29,6 +58,8 @@ static int handle_command(int fd)
 * wrong. Clean up and start over.
 */
index_signature[0] = '\0';
+   } else if (starts_with(msg, watch )) {
+   watch_paths(msg + 6, len - 6, fd, sun);
} else if (!strcmp(msg, die)) {
exit(0);
} else {
diff --git a/pkt-line.c b/pkt-line.c
index bc63b3b..b5af84e 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -135,7 +135,7 @@ static int get_packet_data(int fd, char **src_buf, size_t 
*src_size,
return ret;
 }
 
-static int packet_length(const char *linelen)
+int packet_length(const char *linelen)
 {
int n;
int len = 0;
diff --git a/pkt-line.h b/pkt-line.h
index 0a838d1..40470b9 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -75,6 +75,8 @@ char *packet_read_line(int fd, int *size);
  */
 char *packet_read_line_buf(char **src_buf, size_t *src_len, int *size);
 
+int packet_length(const char *linelen);
+
 #define DEFAULT_PACKET_MAX 1000
 #define LARGE_PACKET_MAX 65520
 extern char packet_buffer[LARGE_PACKET_MAX];
diff --git a/read-cache.c b/read-cache.c
index 76cf0e3..21c3207 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -15,6 +15,7 @@
 #include strbuf.h
 #include varint.h
 #include file-watcher-lib.h
+#include pkt-line.h
 
 static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int 
really);
 
@@ -1479,6 +1480,98 @@ static void validate_watcher(struct index_state *istate, 
const char *path)
}
 }
 
+static int sort_by_date(const void *a_, const void *b_)
+{
+   const struct cache_entry *a = *(const struct cache_entry **)a_;
+   const struct cache_entry *b = *(const struct cache_entry **)b_;
+   uint32_t seca = a-ce_stat_data.sd_mtime.sec;
+   uint32_t secb = b-ce_stat_data.sd_mtime.sec;
+   return seca - secb;
+}
+
+static int do_watch_entries(struct index_state *istate,
+   struct cache_entry **cache,
+   struct strbuf *sb, int start, int now)
+{
+   char *line;
+   int i;
+   ssize_t len;
+
+   send_watcher(istate-watcher, NULL, %s, sb-buf);
+   line = read_watcher(istate-watcher, len, NULL);
+   if (!line) {
+   if (!len) {
+   close(istate-watcher);
+   istate-watcher = -1;
+   }

[PATCH/WIP v2 05/14] read-cache: put some limits on file watching

2014-01-17 Thread Nguyễn Thái Ngọc Duy
watch_entries() is a lot of computation and could trigger a lot more
lookups in file-watcher. Normally after the first set of watches are
in place, we do not need to update often. Moreover if the number of
entries is small, the overhead of file watcher may actually slow git
down.

This patch only allows to update watches if the number of watchable
files is over a limit (and there are new files added if this is not
the first time). Measurements on Core i5-2520M and Linux 3.7.6, about
920 lstat() take 1ms. Somewhere between 2^16 and 2^17 lstat calls that
it starts to take longer than 100ms. 2^16 is chosen at the minimum
limit to start using file watcher.

Recently updated files are not considered watchable because they are
likely to be updated again soon, not worth the ping-pong game with
file watcher. The default limit 30min is just a random value.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/config.txt |  9 +
 cache.h  |  1 +
 read-cache.c | 44 
 3 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a405806..e394399 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1038,6 +1038,15 @@ difftool.tool.cmd::
 difftool.prompt::
Prompt before each invocation of the diff tool.
 
+filewatcher.minfiles::
+   Start watching files if the number of watchable files are
+   above this limit. Default value is 65536.
+
+filewatcher.recentlimit::
+   Files that are last updated within filewatcher.recentlimit
+   seconds from now are not considered watchable. Default value
+   is 1800 (30 minutes).
+
 fetch.recurseSubmodules::
This option can be either set to a boolean value or to 'on-demand'.
Setting it to a boolean changes the behavior of fetch and pull to
diff --git a/cache.h b/cache.h
index 0d1..bcec29b 100644
--- a/cache.h
+++ b/cache.h
@@ -278,6 +278,7 @@ struct index_state {
struct cache_tree *cache_tree;
struct cache_time timestamp;
unsigned name_hash_initialized : 1,
+update_watches : 1,
 initialized : 1;
struct hashmap name_hash;
struct hashmap dir_hash;
diff --git a/read-cache.c b/read-cache.c
index 21c3207..406834a 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -38,6 +38,8 @@ static struct cache_entry *refresh_cache_entry(struct 
cache_entry *ce, int reall
 #define CACHE_EXT_WATCH 0x57415443   /* WATC */
 
 struct index_state the_index;
+static int watch_lowerlimit = 65536;
+static int recent_limit = 1800;
 
 static void set_index_entry(struct index_state *istate, int nr, struct 
cache_entry *ce)
 {
@@ -1014,6 +1016,7 @@ int add_index_entry(struct index_state *istate, struct 
cache_entry *ce, int opti
(istate-cache_nr - pos - 1) * sizeof(ce));
set_index_entry(istate, pos, ce);
istate-cache_changed = 1;
+   istate-update_watches = 1;
return 0;
 }
 
@@ -1300,13 +1303,14 @@ static void read_watch_extension(struct index_state 
*istate, uint8_t *data,
 unsigned long sz)
 {
int i;
-   if ((istate-cache_nr + 7) / 8 != sz) {
+   if ((istate-cache_nr + 7) / 8 + 1 != sz) {
error(invalid 'WATC' extension);
return;
}
for (i = 0; i  istate-cache_nr; i++)
if (data[i / 8]  (1  (i % 8)))
istate-cache[i]-ce_flags |= CE_WATCHED;
+   istate-update_watches = data[sz - 1];
 }
 
 static int read_index_extension(struct index_state *istate,
@@ -1449,6 +1453,19 @@ static struct cache_entry *create_from_disk(struct 
ondisk_cache_entry *ondisk,
return ce;
 }
 
+static int watcher_config(const char *var, const char *value, void *data)
+{
+   if (!strcmp(var, filewatcher.minfiles)) {
+   watch_lowerlimit = git_config_int(var, value);
+   return 0;
+   }
+   if (!strcmp(var, filewatcher.recentlimit)) {
+   recent_limit = git_config_int(var, value);
+   return 0;
+   }
+   return 0;
+}
+
 static void validate_watcher(struct index_state *istate, const char *path)
 {
int i;
@@ -1458,6 +1475,7 @@ static void validate_watcher(struct index_state *istate, 
const char *path)
return;
}
 
+   git_config(watcher_config, NULL);
istate-watcher = connect_watcher(path);
if (istate-watcher != -1) {
struct strbuf sb = STRBUF_INIT;
@@ -1478,6 +1496,7 @@ static void validate_watcher(struct index_state *istate, 
const char *path)
istate-cache[i]-ce_flags = ~CE_WATCHED;
istate-cache_changed = 1;
}
+   istate-update_watches = 1;
 }
 
 static int sort_by_date(const void *a_, const void *b_)
@@ -1524,7 +1543,7 @@ static int do_watch_entries(struct 

[PATCH/WIP v2 09/14] file-watcher: add --shutdown and --log options

2014-01-17 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 file-watcher.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/file-watcher.c b/file-watcher.c
index 1b4ac0a..df06529 100644
--- a/file-watcher.c
+++ b/file-watcher.c
@@ -113,6 +113,8 @@ static int handle_command(int fd)
remove_updated(arg + len + 1);
} else if (starts_with(msg, watch )) {
watch_paths(msg + 6, len - 6, fd, sun);
+   } else if ((arg = skip_prefix(msg, log ))) {
+   fprintf(stderr, log %s\n, arg);
} else if (!strcmp(msg, die)) {
exit(0);
} else {
@@ -168,11 +170,16 @@ int main(int argc, const char **argv)
struct pollfd pfd[2];
int fd, err, nr;
const char *prefix;
-   int daemon = 0, quiet = 0;
+   int daemon = 0, quiet = 0, shutdown = 0;
+   const char *log_string = NULL;
struct option options[] = {
OPT__QUIET(quiet, N_(be quiet)),
OPT_BOOL(0, daemon, daemon,
-N_(run in background)),
+N_(run in background (default))),
+   OPT_BOOL(0, shutdown, shutdown,
+N_(shut down running file-watcher daemon)),
+   OPT_STRING(0, log, log_string, string,
+  N_(string to log to index.watcher.log)),
OPT_END()
};
 
@@ -190,11 +197,24 @@ int main(int argc, const char **argv)
fd = socket(AF_UNIX, SOCK_DGRAM, 0);
sun.sun_family = AF_UNIX;
strlcpy(sun.sun_path, socket_path, sizeof(sun.sun_path));
+
+   if (shutdown || log_string) {
+   struct stat st;
+   if (stat(socket_path, st) || !S_ISSOCK(st.st_mode))
+   return 0;
+   if (log_string  send_watcher(fd, sun, log %s, log_string) 
 0)
+   die_errno(failed to shut file-watcher down);
+   if (shutdown  send_watcher(fd, sun, die)  0)
+   die_errno(failed to shut file-watcher down);
+   return 0;
+   }
+
if (bind(fd, (struct sockaddr *)sun, sizeof(sun))) {
if (quiet)
exit(128);
die_errno(unable to bind to %s, socket_path);
}
+
atexit(cleanup);
sigchain_push_common(cleanup_on_signal);
 
-- 
1.8.5.1.208.g05b12ea

--
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/WIP v2 02/14] read-cache: new extension to mark what file is watched

2014-01-17 Thread Thomas Gummerer
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:

 If an entry is watched, git lets an external program decide if the
 entry is modified or not. It's more like --assume-unchanged, but
 designed to be controlled by machine.

 We are running out of on-disk ce_flags, so instead of extending
 on-disk entry format again, watched flags are in-core only and
 stored as extension instead.

As you said yourself in 
http://thread.gmane.org/gmane.comp.version-control.git/240339/focus=240385
this is not quite true.  As for your explanation there,

 Anyway using extended flags means 2 extra bytes per entry for
 almost every entry in this case (and for index v5 it means redoing
 crc32 for almost every entry too when the bit is updated) so it may
 still be a good idea to keep the new flag separate.

I don't think adding 2 extra bytes would be too bad, since we are
already using 62 bytes plus the bytes for the filename for each index
entry, so it would be a less than 3% increase in the index file size.
(And the extended flags may be used anyway in some cases)

As for index-v5 (if that's ever going to happen), it depends mostly on
how often the CE_WATCHED is going to be updated, to decide whether it
makes sense to store this as extension.

That said, I don't care too deeply if it's stored one way or another,
but I think it would be good to update the commit message with a better
rationale for the choice.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  cache.h  |  2 ++
  read-cache.c | 41 -
  2 files changed, 42 insertions(+), 1 deletion(-)

 diff --git a/cache.h b/cache.h
 index a09d622..069dce7 100644
 --- a/cache.h
 +++ b/cache.h
 @@ -168,6 +168,8 @@ struct cache_entry {
  /* used to temporarily mark paths matched by pathspecs */
  #define CE_MATCHED   (1  26)

 +#define CE_WATCHED   (1  27)
 +
  /*
   * Extended on-disk flags
   */
 diff --git a/read-cache.c b/read-cache.c
 index fe1d153..6f21e3f 100644
 --- a/read-cache.c
 +++ b/read-cache.c
 @@ -33,6 +33,7 @@ static struct cache_entry *refresh_cache_entry(struct 
 cache_entry *ce, int reall
  #define CACHE_EXT(s) ( (s[0]24)|(s[1]16)|(s[2]8)|(s[3]) )
  #define CACHE_EXT_TREE 0x54524545/* TREE */
  #define CACHE_EXT_RESOLVE_UNDO 0x52455543 /* REUC */
 +#define CACHE_EXT_WATCH 0x57415443 /* WATC */

  struct index_state the_index;

 @@ -1293,6 +1294,19 @@ static int verify_hdr(struct cache_header *hdr,
   return 0;
  }

 +static void read_watch_extension(struct index_state *istate, uint8_t *data,
 +  unsigned long sz)
 +{
 + int i;
 + if ((istate-cache_nr + 7) / 8 != sz) {
 + error(invalid 'WATC' extension);
 + return;
 + }
 + for (i = 0; i  istate-cache_nr; i++)
 + if (data[i / 8]  (1  (i % 8)))
 + istate-cache[i]-ce_flags |= CE_WATCHED;
 +}
 +
  static int read_index_extension(struct index_state *istate,
   const char *ext, void *data, unsigned long sz)
  {
 @@ -1303,6 +1317,9 @@ static int read_index_extension(struct index_state 
 *istate,
   case CACHE_EXT_RESOLVE_UNDO:
   istate-resolve_undo = resolve_undo_read(data, sz);
   break;
 + case CACHE_EXT_WATCH:
 + read_watch_extension(istate, data, sz);
 + break;
   default:
   if (*ext  'A' || 'Z'  *ext)
   return error(index uses %.4s extension, which we do 
 not understand,
 @@ -1781,7 +1798,7 @@ int write_index(struct index_state *istate, int newfd)
  {
   git_SHA_CTX c;
   struct cache_header hdr;
 - int i, err, removed, extended, hdr_version;
 + int i, err, removed, extended, hdr_version, has_watches = 0;
   struct cache_entry **cache = istate-cache;
   int entries = istate-cache_nr;
   struct stat st;
 @@ -1790,6 +1807,8 @@ int write_index(struct index_state *istate, int newfd)
   for (i = removed = extended = 0; i  entries; i++) {
   if (cache[i]-ce_flags  CE_REMOVE)
   removed++;
 + else if (cache[i]-ce_flags  CE_WATCHED)
 + has_watches++;

   /* reduce extended entries if possible */
   cache[i]-ce_flags = ~CE_EXTENDED;
 @@ -1861,6 +1880,26 @@ int write_index(struct index_state *istate, int newfd)
   if (err)
   return -1;
   }
 + if (has_watches) {
 + int id, sz = (entries - removed + 7) / 8;
 + uint8_t *data = xmalloc(sz);
 + memset(data, 0, sz);
 + for (i = 0, id = 0; i  entries  has_watches; i++) {
 + struct cache_entry *ce = cache[i];
 + if (ce-ce_flags  CE_REMOVE)
 + continue;
 + if (ce-ce_flags  CE_WATCHED) {
 + data[id / 8] |= 1  (id % 8);
 + has_watches--;
 

Re: [PATCH v2] git-svn: workaround for a bug in svn serf backend

2014-01-17 Thread Roman Kagan
2013/12/31 Roman Kagan rka...@mail.ru:
 2013/12/30 Junio C Hamano gits...@pobox.com:
 Roman Kagan rka...@mail.ru writes:
 I'd like to note that it's IMO worth including in the 'maint' branch
 as it's a crasher.  Especially so since the real fix has been merged
 in the subversion upstream and nominated for 1.8 branch, so the
 workaround may soon lose its relevance.

 I do not quite get this part, though.

 If they refused to fix it for real, it would make it likely that
 this workaround will stay relevant for a long time, in which case it
 would be worth cherry-picking to an older maintenance track.  But if
 this workaround is expected to lose its relevance shortly, I see it
 as one less reason to cherry-pick it to an older maintenance track.

 Confused...

 I thought it was exactly the other way around.  By the time the next
 feature release reaches users, chances are they'd already have
 subversion with the fix.  OTOH the workaround would benefit those who
 get their maintenance release of git (e.g. through their Linux distro
 update) before they get their maintenance release of subversion.

So this actually happened: 1.8.5.3 is out, and some distributions are
shipping it (Arch, Debian), but the workaround didn't make it there.

Could you please consider including it in 'maint', so that 1.8.5.4
brings them a working combination of git and subversion?

Roman.
--
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 0/6] Make 'git help everyday' work - relnotes

2014-01-17 Thread Stefan Näwe
Am 16.01.2014 22:14, schrieb Philip Oakley:
 From: Stefan Näwe stefan.na...@atlas-elektronik.com
 [...]

 I'd really like to see 'git help relnotes' working as well...

 Stefan
 
 Stefan,
 
 Were you thinking that all the release notes would be quoted verbatim in 
 the one long man page?
 
 Or that it would be a set of links to each of the individual text files 
 (see the ifdef::stalenotes[] in git/Documentation/git.txt)?
 
 The latter allows individual release notes to be checked, but still 
 leaves folks with a difficult search problem if they want to find when 
 some command was 'tweaked'.
 
 Obviously, any method would need to be easy to maintain. And the 
 RelNotes symlink would need handling.

'git help relnotes' should show the current release note with
a link to the previous.

And 'git help git' should link to the current release note.


Stefan
-- 

/dev/random says: We now return you to your regularly scheduled flame-throwing
python -c print 
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')
--
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


Workflow on git with 2 branch with specifc code

2014-01-17 Thread Gordon Freeman
Hello guys, im Gordon.

I have a question about workflow with git that i dont know if im doing
it right.
I have 1 repo with 2 branchs the first is the master of the project.
the second is a branch copy of the master but he need to have some
specifc code because is code for a client.
so, every time that i updade master i need to merge master with client
branch and it give me conflicts of course that will hapen.
Well if was just me who work on this 2 branchs it will be easy to fix
the conflicts and let all work and shine.
But whe have here, 10 people woking on master branch and some times code
are lost on merge and we need to look on commits to search whats goin
on. 
What i just asking here is if its correct the workflow that i do. If for
some problem like this, the community have a standard resolution. Or if
what im doing here is all wrong.

Any help here will be apreciated.
Thx for all.

~   

~   

~   

~   
   

--
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/2] prefer xwrite instead of write

2014-01-17 Thread Erik Faye-Lund
Our xwrite wrapper already deals with a few potential hazards, and
are as such more robust. Prefer it instead of write to get the
robustness benefits everywhere.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
With this patch, we don't call write directly any more; all calls
goes via the xwrite-wrapper.

 builtin/merge.c| 2 +-
 streaming.c| 2 +-
 transport-helper.c | 5 ++---
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 4941a6c..9383c31 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -367,7 +367,7 @@ static void squash_message(struct commit *commit, struct 
commit_list *remotehead
sha1_to_hex(commit-object.sha1));
pretty_print_commit(ctx, commit, out);
}
-   if (write(fd, out.buf, out.len)  0)
+   if (xwrite(fd, out.buf, out.len)  0)
die_errno(_(Writing SQUASH_MSG));
if (close(fd))
die_errno(_(Finishing SQUASH_MSG));
diff --git a/streaming.c b/streaming.c
index 9659f18..d7c9f32 100644
--- a/streaming.c
+++ b/streaming.c
@@ -538,7 +538,7 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1, 
struct stream_filter *f
goto close_and_exit;
}
if (kept  (lseek(fd, kept - 1, SEEK_CUR) == (off_t) -1 ||
-write(fd, , 1) != 1))
+xwrite(fd, , 1) != 1))
goto close_and_exit;
result = 0;
 
diff --git a/transport-helper.c b/transport-helper.c
index 2010674..bf64ad7 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1129,9 +1129,8 @@ static int udt_do_write(struct unidirectional_transfer *t)
return 0;   /* Nothing to write. */
 
transfer_debug(%s is writable, t-dest_name);
-   bytes = write(t-dest, t-buf, t-bufuse);
-   if (bytes  0  errno != EWOULDBLOCK  errno != EAGAIN 
-   errno != EINTR) {
+   bytes = xwrite(t-dest, t-buf, t-bufuse);
+   if (bytes  0  errno != EWOULDBLOCK) {
error(write(%s) failed: %s, t-dest_name, strerror(errno));
return -1;
} else if (bytes  0) {
-- 
1.8.4.msysgit.0

--
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/2] mingw: remove mingw_write

2014-01-17 Thread Erik Faye-Lund
Since 0b6806b9 (xread, xwrite: limit size of IO to 8MB), this
wrapper is no longer needed, as read and write are already split
into small chunks.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
 compat/mingw.c | 17 -
 compat/mingw.h |  3 ---
 2 files changed, 20 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index fecb98b..e9892f8 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -304,23 +304,6 @@ int mingw_open (const char *filename, int oflags, ...)
return fd;
 }
 
-#undef write
-ssize_t mingw_write(int fd, const void *buf, size_t count)
-{
-   /*
-* While write() calls to a file on a local disk are translated
-* into WriteFile() calls with a maximum size of 64KB on Windows
-* XP and 256KB on Vista, no such cap is placed on writes to
-* files over the network on Windows XP.  Unfortunately, there
-* seems to be a limit of 32MB-28KB on X64 and 64MB-32KB on x86;
-* bigger writes fail on Windows XP.
-* So we cap to a nice 31MB here to avoid write failures over
-* the net without changing the number of WriteFile() calls in
-* the local case.
-*/
-   return write(fd, buf, min(count, 31 * 1024 * 1024));
-}
-
 static BOOL WINAPI ctrl_ignore(DWORD type)
 {
return TRUE;
diff --git a/compat/mingw.h b/compat/mingw.h
index 92cd728..e033e72 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -180,9 +180,6 @@ int mingw_rmdir(const char *path);
 int mingw_open (const char *filename, int oflags, ...);
 #define open mingw_open
 
-ssize_t mingw_write(int fd, const void *buf, size_t count);
-#define write mingw_write
-
 int mingw_fgetc(FILE *stream);
 #define fgetc mingw_fgetc
 
-- 
1.8.4.msysgit.0

--
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/WIP v2 03/14] read-cache: connect to file watcher

2014-01-17 Thread Torsten Bögershausen
On 2014-01-17 10.47, Nguyễn Thái Ngọc Duy wrote:
[snip[
 diff --git a/file-watcher-lib.c b/file-watcher-lib.c


 +int connect_watcher(const char *path)
Could it be worth to check if we can use some code from unix-socket.c ?

Especially important could be that unix_sockaddr_init() wotks around a problem
when long path names are used. 

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


[Question] Usercase about git clone

2014-01-17 Thread Wang Shilong
Hello everyone,

I have a question about command 'git clone'
If i clone a repo from remote, and if i run command:

# git remote show origin

It will output origin's url, however, this is what i want,  i just want to clone
codes, but keep everything else unchanged, for example branches and
they url….

How can i implement such functions by 'git clone'….I think this is really
helpful because i really don't want to reset my branches' url every one…

Really thanks for your time and response!

Thanks,
Wang

  --
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/WIP v2 03/14] read-cache: connect to file watcher

2014-01-17 Thread Duy Nguyen
On Fri, Jan 17, 2014 at 10:24 PM, Torsten Bögershausen tbo...@web.de wrote:
 On 2014-01-17 10.47, Nguyễn Thái Ngọc Duy wrote:
 [snip[
 diff --git a/file-watcher-lib.c b/file-watcher-lib.c


 +int connect_watcher(const char *path)
 Could it be worth to check if we can use some code from unix-socket.c ?

 Especially important could be that unix_sockaddr_init() wotks around a problem
 when long path names are used.


Thanks! I did not even know about unix-socket.c. Well, I never paid
attention to credential-cache.c :(
-- 
Duy
--
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: file permissions in Git repo

2014-01-17 Thread Torsten Bögershausen
On 01/17/2014 03:26 AM, Jeff King wrote:
 On Thu, Jan 16, 2014 at 03:58:57PM -0800, SH wrote:

 We have a repository which holds lots of shell and perl scripts. We add the
 files to repository (from windows client) with executable permissions (using
 cygwin) but when we pull that repository on another machine (windows or 
 linux),
 files dont have executable permission. Can you please provide a solutions for
 this?

 Git does not preserve file permissions _except_ for the executable bit.
 So this should be working.

 However, I suspect that `core.fileMode` is set to `false` in your
 repository, which causes git to ignore the executable bit. When a
 repository is initialized, we check whether the filesystem simply
 creates everything with the executable bit. If so, we turn off
 core.fileMode for the repository (since otherwise every file would have
 it set).

 -Peff
Cygwin has been a little bit special (and mingw still is).
Until this commit:
Author: Junio C Hamano gits...@pobox.com
Date:   Wed Jul 24 19:22:49 2013 -0700

Merge branch 'ml/cygwin-updates'

The tip one does _not_ revert c869753e (Force core.filemode to
false on Cygwin., 2006-12-30) on purpose, so that people can
still retain the old behaviour if they wanted to.

* ml/cygwin-updates:
  cygwin: stop forcing core.filemode=false
  Cygwin 1.7 supports mmap
  Cygwin 1.7 has thread-safe pread
  Cygwin 1.7 needs compat/regex
the repositories created by cygwin had always core.filemode=false.

You can easily check your configuration by running
git config -l
on the cygwin machine, as Peff suggested.

The next step is to check how the files had been recored in git, using
git ls-files -s | less
on any machine.

If I do this on git.git, we find lines like this, where
100755 means an executable file,
100644 means non-executable file.

100755 9c3f4131b8586408acd81d1e60912b51688575ed 0 
Documentation/technical/api-index.sh
100644 dd894043ae8b04269b3aa2108f96cb935217181d 0 
Documentation/technical/api-lockfile.txt


The 3rd step is to check how file are shown in cygwin, run
ls -l
(Do they have the executable bit set ?)

Side note:
And I think the way the auto-probing of the file system works is
like this:
When a git repo is initialized, the .git/config file is created.
After that, we try to toggle the executable bit, and if lstat reports
it as toggled, we set core.filemode=true.
(See builtin/init-db.c, search for core.filemode)

I tested to create a repo on a network share exported by SAMBA.
The Server was configured so that all new created files had the
executable bit set by default.
Git detected that the executable bit could be switched off,
and configured core.filemode=true
Nice.

/Torsten
--
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: with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream

2014-01-17 Thread Siddharth Agarwal

On 01/17/2014 12:40 AM, John Keeping wrote:

On Thu, Jan 16, 2014 at 06:47:38PM -0800, Siddharth Agarwal wrote:

On 01/16/2014 06:21 PM, Jeff King wrote:

On Thu, Jan 16, 2014 at 05:08:14PM -0800, Siddharth Agarwal wrote:


With git-next, where git pull --rebase can print out fatal: No such
ref: '' if git pull --rebase is run on branches without an upstream.

This is already fixed in bb3f458 (rebase: fix fork-point with zero
arguments, 2014-01-09), I think.

If I'm reading the patch correctly, that only fixes it for git rebase,
not for git pull --rebase. git-pull.sh contains a separate invocation of
git merge-base --fork-point.

I'm pretty sure the invocation in git-pull.sh is OK.  The error then
comes out of git-rebase.sh when git-pull invokes it.


That doesn't square with 48059e4 being the culprit commit.


Are you running a version of git-next that includes bb3f458?


Yes, I am.
--
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] send-email: If the ca path is not specified, use the defaults

2014-01-17 Thread Junio C Hamano
Kyle J. McKay mack...@gmail.com writes:

 On my OS X platform depending on which version of OpenSSL I'm using,
 the OPENSSLDIR path would be one of these:

   /System/Library/OpenSSL
   /opt/local/etc/openssl

 And neither of those uses a certs directory, they both use a
 cert.pem bundle instead:

   $ ls -l /System/Library/OpenSSL
   total 32
   lrwxrwxrwx  1 root  wheel42 cert.pem - ../../../usr/share/curl/
 curl-ca-bundle.crt
   drwxr-xr-x  2 root  wheel68 certs
   drwxr-xr-x  8 root  wheel   272 misc
   -rw-r--r--  1 root  wheel  9381 openssl.cnf
   drwxr-xr-x  2 root  wheel68 private
   # the certs directory is empty

   $ ls -l /opt/local/etc/openssl
   total 32
   lrwxrwxrwx   1 root  admin35 cert.pem@ - ../../share/curl/curl-
 ca-bundle.crt
   drwxr-xr-x   9 root  admin   306 misc/
   -rw-r--r--   1 root  admin 10835 openssl.cnf

 Notice neither of those refers to /etc/ssl/certs at all.

 So the short answer is, yes, hard-coding /etc/ssl/certs as the path on
 OS X is incorrect and if setting /etc/ssl/certs as the path has the
 effect of replacing the default locations the verification will fail.

The current code says if nothing is specified, let's pretend
/etc/ssl/certs was specified.  Then if it is a directory, use it
with SSL_ca_path, if it is a file, use it with SSL_ca_file, if it
does not exist, do not even attempt verification.

And that let's pretend breaks Fedora, where /etc/ssl/certs is a
directory but is not meant to be used with SSL_ca_path---we try to
use /etc/ssl/certs with SSL_ca_path and verification fails miserably.

If I am reading the code correctly, if /etc/ssl/certs does not exist
on the filesystem at all, it wouldn't even attempt verification, so
I take your the verification will fail to mean that you forgot to
also mention And on OS X, /etc/ssl/certs directory still exists,
even though OpenSSL does not use it.  If that is the case, then our
current code indeed is broken in exactly the same way for OS X as
for Fedora.

The proposed change in this thread would stop the defaulting
altogether, and still ask verification to the library using its own
default, so I can see how that would make the setting you described
used on OS X work properly.

In short, I agree with you on both counts (the current code is wrong
for OS X, and the proposed change will fix it).  I just want to make
sure that my understanding of the current breakage is in line with
the reality ;-)

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 1/2] prefer xwrite instead of write

2014-01-17 Thread Jonathan Nieder
Hi,

Erik Faye-Lund wrote:

 --- a/builtin/merge.c
 +++ b/builtin/merge.c
 @@ -367,7 +367,7 @@ static void squash_message(struct commit *commit, struct 
 commit_list *remotehead
   sha1_to_hex(commit-object.sha1));
   pretty_print_commit(ctx, commit, out);
   }
 - if (write(fd, out.buf, out.len)  0)
 + if (xwrite(fd, out.buf, out.len)  0)
   die_errno(_(Writing SQUASH_MSG));

Shouldn't this use write_in_full() to avoid a silently truncated result? (*)

[...]
 --- a/streaming.c
 +++ b/streaming.c
 @@ -538,7 +538,7 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1, 
 struct stream_filter *f
   goto close_and_exit;
   }
   if (kept  (lseek(fd, kept - 1, SEEK_CUR) == (off_t) -1 ||
 -  write(fd, , 1) != 1))
 +  xwrite(fd, , 1) != 1))

Yeah, if we get EINTR then it's worth retrying.

[...]
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -1129,9 +1129,8 @@ static int udt_do_write(struct unidirectional_transfer 
 *t)
   return 0;   /* Nothing to write. */
  
   transfer_debug(%s is writable, t-dest_name);
 - bytes = write(t-dest, t-buf, t-bufuse);
 - if (bytes  0  errno != EWOULDBLOCK  errno != EAGAIN 
 - errno != EINTR) {
 + bytes = xwrite(t-dest, t-buf, t-bufuse);
 + if (bytes  0  errno != EWOULDBLOCK) {

Here the write is limited by BUFFERSIZE, and returning to the outer
loop to try another read when the write returns EAGAIN, like the
original code does, seems philosophically like the right thing to do.

Luckily we don't use O_NONBLOCK anywhere, so the change shouldn't
matter in practice.  So although it doesn't do any good, using xwrite
here for consistency should be fine.

So my only worry is the (*) above.  With that change,
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 1/5] diff_filespec: reorder dirty_submodule macro definitions

2014-01-17 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 diff_filespec has a 2-bit dirty_submodule field and
 defines two flags as macros. Originally these were right
 next to each other, but a new field was accidentally added
 in between in commit 4682d85.

Interesting.

 - 4682d852 (diff-index.c: git diff has no need to read blob from
   the standard input, 2012-06-27) wants to use this rule: all the
   bitfield definitions first, and then whatever macro constants
   next.

 - 25e5e2bf (combine-diff: support format_callback, 2011-08-19),
   wants to use a different rule: a run of (one bitfield definition
   and zero-or-more macro constants to be used in that bitfield).

When they were merged together at d7afe648 (Merge branch
'jc/refactor-diff-stdin', 2012-07-13), these two conflicting
philosophies crashed.

That is the commit to be blamed for this mess ;-)

I am of course fine with the end result this patch gives us.

Thanks.

 This patch puts the field and
 its flags back together.

 Using an enum like:

   enum {
 DIRTY_SUBMODULE_UNTRACKED = 1,
 DIRTY_SUBMODULE_MODIFIED = 2
   } dirty_submodule;

 would be more obvious, but it bloats the structure. Limiting
 the enum size like:

   } dirty_submodule : 2;

 might work, but it is not portable.

 Signed-off-by: Jeff King p...@peff.net
 ---
  diffcore.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/diffcore.h b/diffcore.h
 index 1c16c85..f822f9e 100644
 --- a/diffcore.h
 +++ b/diffcore.h
 @@ -43,9 +43,9 @@ struct diff_filespec {
   unsigned should_free : 1; /* data should be free()'ed */
   unsigned should_munmap : 1; /* data should be munmap()'ed */
   unsigned dirty_submodule : 2;  /* For submodules: its work tree is 
 dirty */
 - unsigned is_stdin : 1;
  #define DIRTY_SUBMODULE_UNTRACKED 1
  #define DIRTY_SUBMODULE_MODIFIED  2
 + unsigned is_stdin : 1;
   unsigned has_more_entries : 1; /* only appear in combined diff */
   struct userdiff_driver *driver;
   /* data should be considered binary; -1 means don't know yet */
--
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 0/5] diff_filespec cleanups and optimizations

2014-01-17 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 But while looking at it, I noticed a bunch of cleanups for
 diff_filespec.  With the patches below, sizeof(struct diff_filespec) on
 my 64-bit machine goes from 96 bytes down to 80. Compiling with -m32
 goes from 64 bytes down to 52.

 The first few patches have cleanup value aside from the struct size
 improvement. The last two are pure optimization. I doubt the
 optimization is noticeable for any real-life cases, so I don't mind if
 they get dropped. But they're quite trivial and obvious.

Thanks for a pleasant read.
--
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: with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream

2014-01-17 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Thu, Jan 16, 2014 at 05:08:14PM -0800, Siddharth Agarwal wrote:

 With git-next, where git pull --rebase can print out fatal: No such
 ref: '' if git pull --rebase is run on branches without an upstream.

 This is already fixed in bb3f458 (rebase: fix fork-point with zero
 arguments, 2014-01-09), I think.

Doesn't the call to get_remote_merge_branch in this part

test -n $curr_branch 
. git-parse-remote 
remoteref=$(get_remote_merge_branch $@ 2/dev/null) 
oldremoteref=$(git merge-base --fork-point $remoteref $curr_branch)

yield an empty string, feeding it to merge-base --fork-point as
its first parameter?

Perhaps something like this is needed?

 git-pull.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-pull.sh b/git-pull.sh
index 605e957..467c66c 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -229,6 +229,7 @@ test true = $rebase  {
test -n $curr_branch 
. git-parse-remote 
remoteref=$(get_remote_merge_branch $@ 2/dev/null) 
+   test -n $remoteref 
oldremoteref=$(git merge-base --fork-point $remoteref $curr_branch)
 }
 orig_head=$(git rev-parse -q --verify HEAD)
--
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 1/2] prefer xwrite instead of write

2014-01-17 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 Hi,

 Erik Faye-Lund wrote:

 --- a/builtin/merge.c
 +++ b/builtin/merge.c
 @@ -367,7 +367,7 @@ static void squash_message(struct commit *commit, struct 
 commit_list *remotehead
  sha1_to_hex(commit-object.sha1));
  pretty_print_commit(ctx, commit, out);
  }
 -if (write(fd, out.buf, out.len)  0)
 +if (xwrite(fd, out.buf, out.len)  0)
  die_errno(_(Writing SQUASH_MSG));

 Shouldn't this use write_in_full() to avoid a silently truncated result? (*)

Meaning this?  If so, I think it makes sense.

 builtin/merge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 6e108d2..a6a38ee 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -367,7 +367,7 @@ static void squash_message(struct commit *commit, struct 
commit_list *remotehead
sha1_to_hex(commit-object.sha1));
pretty_print_commit(ctx, commit, out);
}
-   if (xwrite(fd, out.buf, out.len)  0)
+   if (write_in_full(fd, out.buf, out.len) != out.len)
die_errno(_(Writing SQUASH_MSG));
if (close(fd))
die_errno(_(Finishing SQUASH_MSG));




 [...]
 --- a/streaming.c
 +++ b/streaming.c
 @@ -538,7 +538,7 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1, 
 struct stream_filter *f
  goto close_and_exit;
  }
  if (kept  (lseek(fd, kept - 1, SEEK_CUR) == (off_t) -1 ||
 - write(fd, , 1) != 1))
 + xwrite(fd, , 1) != 1))

 Yeah, if we get EINTR then it's worth retrying.

 [...]
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -1129,9 +1129,8 @@ static int udt_do_write(struct unidirectional_transfer 
 *t)
  return 0;   /* Nothing to write. */
  
  transfer_debug(%s is writable, t-dest_name);
 -bytes = write(t-dest, t-buf, t-bufuse);
 -if (bytes  0  errno != EWOULDBLOCK  errno != EAGAIN 
 -errno != EINTR) {
 +bytes = xwrite(t-dest, t-buf, t-bufuse);
 +if (bytes  0  errno != EWOULDBLOCK) {

 Here the write is limited by BUFFERSIZE, and returning to the outer
 loop to try another read when the write returns EAGAIN, like the
 original code does, seems philosophically like the right thing to do.

 Luckily we don't use O_NONBLOCK anywhere, so the change shouldn't
 matter in practice.  So although it doesn't do any good, using xwrite
 here for consistency should be fine.

 So my only worry is the (*) above.  With that change,
 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 1/2] prefer xwrite instead of write

2014-01-17 Thread Erik Faye-Lund
On Fri, Jan 17, 2014 at 8:07 PM, Junio C Hamano gits...@pobox.com wrote:
 Jonathan Nieder jrnie...@gmail.com writes:

 Hi,

 Erik Faye-Lund wrote:

 --- a/builtin/merge.c
 +++ b/builtin/merge.c
 @@ -367,7 +367,7 @@ static void squash_message(struct commit *commit, 
 struct commit_list *remotehead
  sha1_to_hex(commit-object.sha1));
  pretty_print_commit(ctx, commit, out);
  }
 -if (write(fd, out.buf, out.len)  0)
 +if (xwrite(fd, out.buf, out.len)  0)
  die_errno(_(Writing SQUASH_MSG));

 Shouldn't this use write_in_full() to avoid a silently truncated result? (*)

 Meaning this?  If so, I think it makes sense.


Yeah, I think that's better. Thanks, both!
--
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/3] setup_pager: set MORE=R

2014-01-17 Thread Junio C Hamano
Kyle J. McKay mack...@gmail.com writes:

 On Jan 16, 2014, at 20:21, Jeff King wrote:
 When we run the pager, we always set LESS=R to tell the
 pager to pass through ANSI colors. On modern versions of
 FreeBSD, the system more can do the same trick.
 [snip]
 diff --git a/pager.c b/pager.c
 index 90d237e..2303164 100644
 --- a/pager.c
 +++ b/pager.c
 @@ -87,6 +87,10 @@ void setup_pager(void)
  argv_array_push(env, LESS=FRSX);
  if (!getenv(LV))
  argv_array_push(env, LV=-c);
 +#ifdef PAGER_MORE_UNDERSTANDS_R
 +if (!getenv(MORE))
 +argv_array_push(env, MORE=R);
 +#endif

 How about adding a leading - to both the LESS and MORE settings?
 Since you're in there patching... :)

The discussion we had when LV=-c was added, namely $gmane/240124,
agrees.  I however am perfectly fine to see it done as a separate
clean-up patch.
--
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/3] setup_pager: set MORE=R

2014-01-17 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 diff --git a/pager.c b/pager.c
 index 90d237e..2303164 100644
 --- a/pager.c
 +++ b/pager.c
 @@ -87,6 +87,10 @@ void setup_pager(void)
   argv_array_push(env, LESS=FRSX);
   if (!getenv(LV))
   argv_array_push(env, LV=-c);
 +#ifdef PAGER_MORE_UNDERSTANDS_R
 + if (!getenv(MORE))
 + argv_array_push(env, MORE=R);
 +#endif
   pager_process.env = argv_array_detach(env, NULL);
  
   if (start_command(pager_process))

Let me repeat from $gmane/240110:

 - Can we generalize this a bit so that a builder can pass a list
   of var=val pairs and demote the existing LESS=FRSX to just a
   canned setting of such a mechanism?

As we need to maintain this set these environments when unset here
and also in git-sh-setup.sh, I think it is about time to do that
clean-up.  Duplicating two settings was borderline OK, but seeing
the third added fairly soon after the second was added tells me that
the clean-up must come before adding the third.

--
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: file permissions in Git repo

2014-01-17 Thread SH
Thanks guys.  Sorry but one more question, like I mentioned we have hosted 
repositories so how do I make some configuration changes are server based so 
whether the client have those changes or not, it wouldn't matter. Also I have 
one client on linux and another one on windows (for my testing purpose) and I 
see that .git/config on both machines are little different. Is that normal?

Thanks Again.





On Friday, January 17, 2014 10:08 AM, Torsten Bögershausen tbo...@web.de 
wrote:
On 01/17/2014 03:26 AM, Jeff King wrote:

 On Thu, Jan 16, 2014 at 03:58:57PM -0800, SH wrote:

 We have a repository which holds lots of shell and perl scripts. We add the
 files to repository (from windows client) with executable permissions (using
 cygwin) but when we pull that repository on another machine (windows or 
 linux),
 files dont have executable permission. Can you please provide a solutions for
 this?

 Git does not preserve file permissions _except_ for the executable bit.
 So this should be working.

 However, I suspect that `core.fileMode` is set to `false` in your
 repository, which causes git to ignore the executable bit. When a
 repository is initialized, we check whether the filesystem simply
 creates everything with the executable bit. If so, we turn off
 core.fileMode for the repository (since otherwise every file would have
 it set).

 -Peff
Cygwin has been a little bit special (and mingw still is).
Until this commit:
Author: Junio C Hamano gits...@pobox.com
Date:   Wed Jul 24 19:22:49 2013 -0700

    Merge branch 'ml/cygwin-updates'

    The tip one does _not_ revert c869753e (Force core.filemode to
    false on Cygwin., 2006-12-30) on purpose, so that people can
    still retain the old behaviour if they wanted to.

    * ml/cygwin-updates:
      cygwin: stop forcing core.filemode=false
      Cygwin 1.7 supports mmap
      Cygwin 1.7 has thread-safe pread
      Cygwin 1.7 needs compat/regex
the repositories created by cygwin had always core.filemode=false.

You can easily check your configuration by running
git config -l
on the cygwin machine, as Peff suggested.

The next step is to check how the files had been recored in git, using
git ls-files -s | less
on any machine.

If I do this on git.git, we find lines like this, where
100755 means an executable file,
100644 means non-executable file.

100755 9c3f4131b8586408acd81d1e60912b51688575ed 0 
Documentation/technical/api-index.sh
100644 dd894043ae8b04269b3aa2108f96cb935217181d 0 
Documentation/technical/api-lockfile.txt


The 3rd step is to check how file are shown in cygwin, run
ls -l
(Do they have the executable bit set ?)

Side note:
And I think the way the auto-probing of the file system works is
like this:
When a git repo is initialized, the .git/config file is created.
After that, we try to toggle the executable bit, and if lstat reports
it as toggled, we set core.filemode=true.
(See builtin/init-db.c, search for core.filemode)

I tested to create a repo on a network share exported by SAMBA.
The Server was configured so that all new created files had the
executable bit set by default.
Git detected that the executable bit could be switched off,
and configured core.filemode=true
Nice.

/Torsten

--
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 v2] git-svn: workaround for a bug in svn serf backend

2014-01-17 Thread Junio C Hamano
Roman Kagan rka...@mail.ru writes:

 2013/12/31 Roman Kagan rka...@mail.ru:
 2013/12/30 Junio C Hamano gits...@pobox.com:
 Roman Kagan rka...@mail.ru writes:
 I'd like to note that it's IMO worth including in the 'maint' branch
 as it's a crasher.  Especially so since the real fix has been merged
 in the subversion upstream and nominated for 1.8 branch, so the
 workaround may soon lose its relevance.

 I do not quite get this part, though.

 If they refused to fix it for real, it would make it likely that
 this workaround will stay relevant for a long time, in which case it
 would be worth cherry-picking to an older maintenance track.  But if
 this workaround is expected to lose its relevance shortly, I see it
 as one less reason to cherry-pick it to an older maintenance track.

 Confused...

 I thought it was exactly the other way around.  By the time the next
 feature release reaches users, chances are they'd already have
 subversion with the fix.  OTOH the workaround would benefit those who
 get their maintenance release of git (e.g. through their Linux distro
 update) before they get their maintenance release of subversion.

 So this actually happened: 1.8.5.3 is out, and some distributions are
 shipping it (Arch, Debian), but the workaround didn't make it there.

The way I read your message was that the fix on the subversion side
is already there and this patch to work it around on our end is of
no importance.

But actually you wanted to say quite the opposite.  They are slow
and it is likely that we need to work their bug around for a while.

If so, then I think it might make sense to cherry-pick it to the
maint branch, even though we usually apply only fixes to our own
bugs to the maintenance track.
--
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: with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream

2014-01-17 Thread John Keeping
On Fri, Jan 17, 2014 at 10:57:56AM -0800, Junio C Hamano wrote:
 Jeff King p...@peff.net writes:
 
  On Thu, Jan 16, 2014 at 05:08:14PM -0800, Siddharth Agarwal wrote:
 
  With git-next, where git pull --rebase can print out fatal: No such
  ref: '' if git pull --rebase is run on branches without an upstream.
 
  This is already fixed in bb3f458 (rebase: fix fork-point with zero
  arguments, 2014-01-09), I think.
 
 Doesn't the call to get_remote_merge_branch in this part
 
 test -n $curr_branch 
 . git-parse-remote 
 remoteref=$(get_remote_merge_branch $@ 2/dev/null) 
 oldremoteref=$(git merge-base --fork-point $remoteref $curr_branch)
 
 yield an empty string, feeding it to merge-base --fork-point as
 its first parameter?

For some reason I assumed that get_remote_merge_branch would either
yield a non-empty string or return failure, meaning that the -chain
makes everything OK.

Before the change to use merge-base --fork-point, the code was:

oldremoteref=$(git rev-parse -q --verify $remoteref) 
for reflog in $(git rev-list -g $remoteref 2/dev/null)
do
if test $reflog = $(git merge-base $reflog $curr_branch)
then
oldremoteref=$reflog
break
fi
done

which has a similar failure - rev-list requires a revision argument and
prints its usage if not given one.

 Perhaps something like this is needed?
 
  git-pull.sh | 1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/git-pull.sh b/git-pull.sh
 index 605e957..467c66c 100755
 --- a/git-pull.sh
 +++ b/git-pull.sh
 @@ -229,6 +229,7 @@ test true = $rebase  {
   test -n $curr_branch 
   . git-parse-remote 
   remoteref=$(get_remote_merge_branch $@ 2/dev/null) 
 + test -n $remoteref 
   oldremoteref=$(git merge-base --fork-point $remoteref $curr_branch)
  }
  orig_head=$(git rev-parse -q --verify HEAD)

Either that or 2/dev/null like in the original, yes.
--
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 quietly fails on https:// URL, https errors are never reported to user

2014-01-17 Thread Junio C Hamano
Yuri y...@rawbw.com writes:

 I think that in a rare case of error this extra-printout wouldn't
 hurt.

If the error is rare, extra verbiage does not hurt were a valid
attitude, error is rare, non-zero exit is enough would be equally
valid ;-)

Also that statement contradicts with the rationale given by 266f1fdf
(transport-helper: be quiet on read errors from helpers,
2013-06-21), no?

However, this makes a much more common case worse: when a helper
does die with a useful message, we print the extra Reading from
'git-remote-foo failed message. This can also end up confusing
users, as they may not even know what remote helpers are (e.g.,
the fact that http support comes through git-remote-https is
purely an implementation detail that most users do not know or
care about).

Your change is not an exact revert and rewords the message to read

Failure in 'http' protocol reader.

instead of

Reading from helper 'git-remote-http' failed.

which avoids the helper word and replacing it with protocol
reader [*1*] in an attempt to make it less likely to end up
confusing users, but I am not sure if protocol reader is good
enough for those who get confused with helper in the first place.
They will ask their resident guru or favourite search engine about
the message and will be told that your http connection did not go
well either way, but not many people have seen this new message.

If we were to reinstate the extra final line in the error message, I
think we would be off doing a straight revert without any rewording
that introduces yet another word protocol reader that is not found
anywhere in our glossary.

I think I am OK with adding one more line after Reading from
... failed that explains a more detailed error message might be
there above that line, but I am not sure what the good phrasing
would be.


[Footnote]

*1* It may introduce a new confusion was it 'read' that failed, or
'write'?, though ;-)
--
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 1/5] diff_filespec: reorder dirty_submodule macro definitions

2014-01-17 Thread Jeff King
On Fri, Jan 17, 2014 at 10:46:59AM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  diff_filespec has a 2-bit dirty_submodule field and
  defines two flags as macros. Originally these were right
  next to each other, but a new field was accidentally added
  in between in commit 4682d85.
 
 Interesting.
 
  - 4682d852 (diff-index.c: git diff has no need to read blob from
the standard input, 2012-06-27) wants to use this rule: all the
bitfield definitions first, and then whatever macro constants
next.
 
  - 25e5e2bf (combine-diff: support format_callback, 2011-08-19),
wants to use a different rule: a run of (one bitfield definition
and zero-or-more macro constants to be used in that bitfield).
 
 When they were merged together at d7afe648 (Merge branch
 'jc/refactor-diff-stdin', 2012-07-13), these two conflicting
 philosophies crashed.
 
 That is the commit to be blamed for this mess ;-)

That makes sense. I had assumed it was a mis-merge initially, so was
surprised to find 4682d85. It just looked like an error to me, but the
rule you gave above does at least make sense.

I'm happy with it either way. I almost just pulled the macro
definitions, including DIFF_FILE_VALID, out of the struct definition
completely. I see the value in having the flags near their bitfield, but
it makes the definition a bit harder to read.

 I am of course fine with the end result this patch gives us.

Me too, though if you feel like doing it the other way, I'm fine with
that, too.

-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: file permissions in Git repo

2014-01-17 Thread Torsten Bögershausen
(Please no top posting next time)
On 2014-01-17 20.20, SH wrote:
 On Friday, January 17, 2014 10:08 AM, Torsten Bögershausen tbo...@web.de 
 wrote:
 On 01/17/2014 03:26 AM, Jeff King wrote:
 
 On Thu, Jan 16, 2014 at 03:58:57PM -0800, SH wrote:

 We have a repository which holds lots of shell and perl scripts. We add the
 files to repository (from windows client) with executable permissions (using
 cygwin) but when we pull that repository on another machine (windows or 
 linux),
 files dont have executable permission. Can you please provide a solutions 
 for
 this?

 Git does not preserve file permissions _except_ for the executable bit.
 So this should be working.

 However, I suspect that `core.fileMode` is set to `false` in your
 repository, which causes git to ignore the executable bit. When a
 repository is initialized, we check whether the filesystem simply
 creates everything with the executable bit. If so, we turn off
 core.fileMode for the repository (since otherwise every file would have
 it set).

 -Peff
 Cygwin has been a little bit special (and mingw still is).
 Until this commit:
 Author: Junio C Hamano gits...@pobox.com
 Date:   Wed Jul 24 19:22:49 2013 -0700
 
 Merge branch 'ml/cygwin-updates'
 
 The tip one does _not_ revert c869753e (Force core.filemode to
 false on Cygwin., 2006-12-30) on purpose, so that people can
 still retain the old behaviour if they wanted to.
 
 * ml/cygwin-updates:
   cygwin: stop forcing core.filemode=false
   Cygwin 1.7 supports mmap
   Cygwin 1.7 has thread-safe pread
   Cygwin 1.7 needs compat/regex
 the repositories created by cygwin had always core.filemode=false.
 
 You can easily check your configuration by running
 git config -l
 on the cygwin machine, as Peff suggested.
 
 The next step is to check how the files had been recored in git, using
 git ls-files -s | less
 on any machine.
 
 If I do this on git.git, we find lines like this, where
 100755 means an executable file,
 100644 means non-executable file.
 
 100755 9c3f4131b8586408acd81d1e60912b51688575ed 0 
 Documentation/technical/api-index.sh
 100644 dd894043ae8b04269b3aa2108f96cb935217181d 0 
 Documentation/technical/api-lockfile.txt
 
 
 The 3rd step is to check how file are shown in cygwin, run
 ls -l
 (Do they have the executable bit set ?)
 
 Side note:
 And I think the way the auto-probing of the file system works is
 like this:
 When a git repo is initialized, the .git/config file is created.
 After that, we try to toggle the executable bit, and if lstat reports
 it as toggled, we set core.filemode=true.
 (See builtin/init-db.c, search for core.filemode)
 
 I tested to create a repo on a network share exported by SAMBA.
 The Server was configured so that all new created files had the
 executable bit set by default.
 Git detected that the executable bit could be switched off,
 and configured core.filemode=true
 Nice.
 
 /Torsten
 Thanks guys.  Sorry but one more question, like I mentioned we have hosted 
 repositories so how do I make some configuration changes are server based so 
 whether the client have those changes or not, it wouldn't matter. Also I have 
 one client on linux and another one on windows (for my testing purpose) and I 
 see that .git/config on both machines are little different. Is that normal?
 
 Thanks Again.
That a config file has small differences could be normal:
the server has typically core.bare true.

About other differences, please don't heasitate to consult
http://git-htmldocs.googlecode.com/git/git-config.html

And if there are still questions, they are there to be answered here.
/Torsten



--
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/3] setup_pager: set MORE=R

2014-01-17 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

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

 diff --git a/pager.c b/pager.c
 index 90d237e..2303164 100644
 --- a/pager.c
 +++ b/pager.c
 @@ -87,6 +87,10 @@ void setup_pager(void)
  argv_array_push(env, LESS=FRSX);
  if (!getenv(LV))
  argv_array_push(env, LV=-c);
 +#ifdef PAGER_MORE_UNDERSTANDS_R
 +if (!getenv(MORE))
 +argv_array_push(env, MORE=R);
 +#endif
  pager_process.env = argv_array_detach(env, NULL);
  
  if (start_command(pager_process))

 Let me repeat from $gmane/240110:

  - Can we generalize this a bit so that a builder can pass a list
of var=val pairs and demote the existing LESS=FRSX to just a
canned setting of such a mechanism?

 As we need to maintain this set these environments when unset here
 and also in git-sh-setup.sh, I think it is about time to do that
 clean-up.  Duplicating two settings was borderline OK, but seeing
 the third added fairly soon after the second was added tells me that
 the clean-up must come before adding the third.

Perhaps we can start it like this.  Then pager.c can iterate over
the PAGER_ENV string, parse out LESS=, LV=, etc., and do its thing.

We would also need to muck with git-sh-setup.sh but that file is
already preprocessed in the Makefile, so we should be able to do
something similar to # @@BROKEN_PATH_FIX@@ there.

 Makefile | 15 ++-
 config.mak.uname |  1 +
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index b4af1e2..c9e7847 100644
--- a/Makefile
+++ b/Makefile
@@ -342,6 +342,14 @@ all::
 # Define DEFAULT_HELP_FORMAT to man, info or html
 # (defaults to man) if you want to have a different default when
 # git help is called without a parameter specifying the format.
+#
+# Define PAGER_ENV to a SP separated VAR=VAL pairs to define
+# default environment variables to be passed when a pager is spawned, e.g.
+#
+#PAGER_ENV = LESS=-FRSX LV=-c
+#
+# to say export LESS=-FRSX (and LV=-c) if the environment variable
+# LESS (and LV) is not set, respectively.
 
 GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1506,6 +1514,10 @@ ifeq ($(PYTHON_PATH),)
 NO_PYTHON = NoThanks
 endif
 
+ifndef PAGER_ENV
+PAGER_ENV = LESS=-FRSX LV=-c
+endif
+
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -1585,11 +1597,12 @@ PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 DIFF_SQ = $(subst ','\'',$(DIFF))
 PERLLIB_EXTRA_SQ = $(subst ','\'',$(PERLLIB_EXTRA))
+PAGER_ENV_SQ = $(subst ','\'',$(PAGER_ENV))
 
 LIBS = $(GITLIBS) $(EXTLIBS)
 
 BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
-   $(COMPAT_CFLAGS)
+   $(COMPAT_CFLAGS) -DPAGER_ENV='$(PAGER_ENV_SQ)'
 LIB_OBJS += $(COMPAT_OBJS)
 
 # Quote for C
diff --git a/config.mak.uname b/config.mak.uname
index 7d31fad..8aea8a6 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -188,6 +188,7 @@ ifeq ($(uname_S),FreeBSD)
endif
PYTHON_PATH = /usr/local/bin/python
HAVE_PATHS_H = YesPlease
+   PAGER_ENV = LESS=-FRSX LV=-c MORE=-R
 endif
 ifeq ($(uname_S),OpenBSD)
NO_STRCASESTR = YesPlease
--
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] pull: suppress error when no remoteref is found

2014-01-17 Thread John Keeping
Commit 48059e4 (pull: use merge-base --fork-point when appropriate,
2013-12-08) incorrectly assumes that get_remote_merge_branch will either
yield a non-empty string or return an error, but there are circumstances
where it will yield an empty string.

The previous code then invoked git-rev-list with no arguments, which
results in an error suppressed by redirecting stderr to /dev/null.  Now
we invoke git-merge-base with an empty branch name, which also results
in an error.  Suppress this in the same way.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 git-pull.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-pull.sh b/git-pull.sh
index f210d0a..0a5aa2c 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -229,7 +229,7 @@ test true = $rebase  {
test -n $curr_branch 
. git-parse-remote 
remoteref=$(get_remote_merge_branch $@ 2/dev/null) 
-   oldremoteref=$(git merge-base --fork-point $remoteref $curr_branch)
+   oldremoteref=$(git merge-base --fork-point $remoteref $curr_branch 
2/dev/null)
 }
 orig_head=$(git rev-parse -q --verify HEAD)
 git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok 
$@ || exit 1
-- 
1.8.5.226.g0d60d77

--
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: with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream

2014-01-17 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes:

 Perhaps something like this is needed?
  ...
 Either that or 2/dev/null like in the original, yes.

Ah, that makes sense.  I see you already followed-up with a patch,
so I'll pick it up.

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 1/2] prefer xwrite instead of write

2014-01-17 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 Junio C Hamano wrote:
 Jonathan Nieder jrnie...@gmail.com writes:

 Shouldn't this use write_in_full() to avoid a silently truncated result? (*)

 Meaning this?  If so, I think it makes sense.
 [...]
 -if (xwrite(fd, out.buf, out.len)  0)
 +if (write_in_full(fd, out.buf, out.len) != out.len)

 Yes.  Either ' 0' or '!= out.len' would work fine here, since
 write_in_full is defined to always either write the full 'count'
 bytes or return an error.

An unrelated tangent but we may want to fix majority of callers that
do not seem to know that ;-)

--
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 1/2] prefer xwrite instead of write

2014-01-17 Thread Jonathan Nieder
Junio C Hamano wrote:
 Jonathan Nieder jrnie...@gmail.com writes:

 Shouldn't this use write_in_full() to avoid a silently truncated result? (*)

 Meaning this?  If so, I think it makes sense.
[...]
 - if (xwrite(fd, out.buf, out.len)  0)
 + if (write_in_full(fd, out.buf, out.len) != out.len)

Yes.  Either ' 0' or '!= out.len' would work fine here, since
write_in_full is defined to always either write the full 'count'
bytes or return an error.

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: file permissions in Git repo

2014-01-17 Thread SH
Thanks again.





On Friday, January 17, 2014 11:55 AM, Torsten Bögershausen tbo...@web.de 
wrote:
(Please no top posting next time)
On 2014-01-17 20.20, SH wrote:
 On Friday, January 17, 2014 10:08 AM, Torsten Bögershausen tbo...@web.de 
 wrote:
 On 01/17/2014 03:26 AM, Jeff King wrote:
 
 On Thu, Jan 16, 2014 at 03:58:57PM -0800, SH wrote:

 We have a repository which holds lots of shell and perl scripts. We add the
 files to repository (from windows client) with executable permissions (using
 cygwin) but when we pull that repository on another machine (windows or 
 linux),
 files dont have executable permission. Can you please provide a solutions 
 for
 this?

 Git does not preserve file permissions _except_ for the executable bit.
 So this should be working.

 However, I suspect that `core.fileMode` is set to `false` in your
 repository, which causes git to ignore the executable bit. When a
 repository is initialized, we check whether the filesystem simply
 creates everything with the executable bit. If so, we turn off
 core.fileMode for the repository (since otherwise every file would have
 it set).

 -Peff
 Cygwin has been a little bit special (and mingw still is).
 Until this commit:
 Author: Junio C Hamano gits...@pobox.com
 Date:   Wed Jul 24 19:22:49 2013 -0700
 
     Merge branch 'ml/cygwin-updates'
 
     The tip one does _not_ revert c869753e (Force core.filemode to
     false on Cygwin., 2006-12-30) on purpose, so that people can
     still retain the old behaviour if they wanted to.
 
     * ml/cygwin-updates:
       cygwin: stop forcing core.filemode=false
       Cygwin 1.7 supports mmap
       Cygwin 1.7 has thread-safe pread
       Cygwin 1.7 needs compat/regex
 the repositories created by cygwin had always core.filemode=false.
 
 You can easily check your configuration by running
 git config -l
 on the cygwin machine, as Peff suggested.
 
 The next step is to check how the files had been recored in git, using
 git ls-files -s | less
 on any machine.
 
 If I do this on git.git, we find lines like this, where
 100755 means an executable file,
 100644 means non-executable file.
 
 100755 9c3f4131b8586408acd81d1e60912b51688575ed 0 
 Documentation/technical/api-index.sh
 100644 dd894043ae8b04269b3aa2108f96cb935217181d 0 
 Documentation/technical/api-lockfile.txt
 
 
 The 3rd step is to check how file are shown in cygwin, run
 ls -l
 (Do they have the executable bit set ?)
 
 Side note:
 And I think the way the auto-probing of the file system works is
 like this:
 When a git repo is initialized, the .git/config file is created.
 After that, we try to toggle the executable bit, and if lstat reports
 it as toggled, we set core.filemode=true.
 (See builtin/init-db.c, search for core.filemode)
 
 I tested to create a repo on a network share exported by SAMBA.
 The Server was configured so that all new created files had the
 executable bit set by default.
 Git detected that the executable bit could be switched off,
 and configured core.filemode=true
 Nice.
 
 /Torsten
 Thanks guys.  Sorry but one more question, like I mentioned we have hosted 
 repositories so how do I make some configuration changes are server based so 
 whether the client have those changes or not, it wouldn't matter. Also I have 
 one client on linux and another one on windows (for my testing purpose) and I 
 see that .git/config on both machines are little different. Is that normal?
 
 Thanks Again.
That a config file has small differences could be normal:
the server has typically core.bare true.

About other differences, please don't heasitate to consult
http://git-htmldocs.googlecode.com/git/git-config.html

And if there are still questions, they are there to be answered here.

/Torsten
--
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 quietly fails on https:// URL, https errors are never reported to user

2014-01-17 Thread Jeff King
On Fri, Jan 17, 2014 at 11:43:35AM -0800, Junio C Hamano wrote:

 Yuri y...@rawbw.com writes:
 
  I think that in a rare case of error this extra-printout wouldn't
  hurt.
 
 If the error is rare, extra verbiage does not hurt were a valid
 attitude, error is rare, non-zero exit is enough would be equally
 valid ;-)

I think the problem is that error is _not_ rare. For years, we did not
print the extra verbiage, and nobody complained. Then, within days of us
making a release that included the extra line, somebody complained[1].

The real issue is that the remote-helper hanging up _should_ be an
exceptional condition, but it's not. The remote-helper protocol sucks,
and has no way to signal failure of an operation besides just hanging
up. So you end up with junk like:

  $ git clone https://google.com/foo.git
  Cloning into 'foo'...
  fatal: repository 'https://google.com/foo.git/' not found
  fatal: Reading from helper 'git-remote-https' failed

That second line is not adding anything, and IMHO is making things
uglier and more confusing. We _expected_ the helper to hang up; that's
how it signals an error to us. It is not an unexpected condition at all.
The exit(128) we do is simply propagating the error report of the
helper.

That's the common error case: the message is redundant and annoying. The
_uncommon_ case is the one Yuri hit: some library misconfiguration that
causes the helper not to run at all.  Adding back any message is hurting
the common case to help the uncommon one.

 I think I am OK with adding one more line after Reading from
 ... failed that explains a more detailed error message might be
 there above that line, but I am not sure what the good phrasing
 would be.

I'd really rather not. Hopefully the explanation above makes it clear
why not.

The most right solution is to fix the helper protocol to allow error
reporting. That may be somewhat complicated to retain backward
compatibility (we have to introduce a capability, probe for it, handle
old helpers, etc).

We _may_ be able to do better by annotating certain commands with
whether we expect them to hangup. The big one, I think, would be the
initial capabilities command. Something like the patch below would
detect any startup problems in the helper. From Yuri's description, that
would catch his case or any similar ones.

Anything after startup should be the responsibility of the helper to
report. If it doesn't, that's a bug in the helper. The one exception may
be signals. We could waitpid() on the helper and report any signal
death (e.g., it obviously cannot report its own SIGKILL death, and I'd
guess that most do not report their own SIGPIPE death).

diff --git a/transport-helper.c b/transport-helper.c
index 2010674..af03f1a 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -50,7 +50,8 @@ static void sendline(struct helper_data *helper, struct 
strbuf *buffer)
die_errno(Full write to remote helper failed);
 }
 
-static int recvline_fh(FILE *helper, struct strbuf *buffer, const char *name)
+static int recvline_fh(FILE *helper, struct strbuf *buffer, const char *name,
+  int die_on_failure)
 {
strbuf_reset(buffer);
if (debug)
@@ -58,7 +59,9 @@ static int recvline_fh(FILE *helper, struct strbuf *buffer, 
const char *name)
if (strbuf_getline(buffer, helper, '\n') == EOF) {
if (debug)
fprintf(stderr, Debug: Remote helper quit.\n);
-   exit(128);
+   if (die_on_failure)
+   exit(128);
+   return -1;
}
 
if (debug)
@@ -68,7 +71,7 @@ static int recvline_fh(FILE *helper, struct strbuf *buffer, 
const char *name)
 
 static int recvline(struct helper_data *helper, struct strbuf *buffer)
 {
-   return recvline_fh(helper-out, buffer, helper-name);
+   return recvline_fh(helper-out, buffer, helper-name, 1);
 }
 
 static void xchgline(struct helper_data *helper, struct strbuf *buffer)
@@ -163,7 +166,9 @@ static struct child_process *get_helper(struct transport 
*transport)
while (1) {
const char *capname;
int mandatory = 0;
-   recvline(data, buf);
+
+   if (recvline_fh(data-out, buf, data-name, 0)  0)
+   die(remote helper '%s' aborted session, data-name);
 
if (!*buf.buf)
break;
@@ -557,7 +562,7 @@ static int process_connect_service(struct transport 
*transport,
goto exit;
 
sendline(data, cmdbuf);
-   recvline_fh(input, cmdbuf, name);
+   recvline_fh(input, cmdbuf, name, 1);
if (!strcmp(cmdbuf.buf, )) {
data-no_disconnect_req = 1;
if (debug)
--
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 log' escape symbols shown as ESC[33 and ESC[m

2014-01-17 Thread Yuri
One other idea to handle this is at configuration phase. You can test 
more and less with every option used on every platform for each of them 
respectively. Test could run the command with the option, and check if 
it passes the given escape sequence. This would be reflected in config.h 
defines like this: MORE_DASH_R_WORKS This would be very accurate.


Not sure if this is an overkill for this issue.

Yuri
--
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 quietly fails on https:// URL, https errors are never reported to user

2014-01-17 Thread Jeff King
On Fri, Jan 17, 2014 at 03:13:25PM -0500, Jeff King wrote:

 On Fri, Jan 17, 2014 at 11:43:35AM -0800, Junio C Hamano wrote:
 
  Yuri y...@rawbw.com writes:
  
   I think that in a rare case of error this extra-printout wouldn't
   hurt.
  
  If the error is rare, extra verbiage does not hurt were a valid
  attitude, error is rare, non-zero exit is enough would be equally
  valid ;-)
 
 I think the problem is that error is _not_ rare. For years, we did not
 print the extra verbiage, and nobody complained. Then, within days of us
 making a release that included the extra line, somebody complained[1].

Forgot my footnote here, but it was:

  http://article.gmane.org/gmane.comp.version-control.git/228498

which led to 266f1fd (transport-helper: be quiet on read errors from
helpers, 2013-06-21).

-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


BUG: [Cosmetic] Commiting a gerrit ChangeId before the commit hook was installed

2014-01-17 Thread Strainu
I was trying to send a new version of a patch to a gerrit server from
a new computer, so I made a change with a ChangeId in the description
and tried to review it:

strainu@emily:~/core git branch archivebot
strainu@emily:~/core git checkout archivebot
M   pywikibot/page.py
Switched to branch 'archivebot'
strainu@emily:~/core git diff
strainu@emily:~/core git add .
strainu@emily:~/core git commit
[archivebot 282ad24] Update getFileVersionHistoryTable.
 1 file changed, 3 insertions(+), 4 deletions(-)
strainu@emily:~/core git review -f
Creating a git remote called gerrit that maps to:
ssh://stra...@gerrit.wikimedia.org:29418/pywikibot/core.git
Your change was committed before the commit hook was installed.
Amending the commit to add a gerrit change id.


At this point I ended the transaction, as I was confused by the last
message: I was afraid the ChangeId would have changed, causing the
patch to be attached to another review.

I think git should not show this message if the change description
already has a change id, or at least add another message that
clarifies the fact that the change id has not changed.

Thanks,
   Strainu
--
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 quietly fails on https:// URL, https errors are never reported to user

2014-01-17 Thread Yuri

On 01/17/2014 12:13, Jeff King wrote:

   $ git clonehttps://google.com/foo.git
   Cloning into 'foo'...
   fatal: repository 'https://google.com/foo.git/' not found
   fatal: Reading from helper 'git-remote-https' failed

That second line is not adding anything, and IMHO is making things
uglier and more confusing. We_expected_  the helper to hang up; that's
how it signals an error to us. It is not an unexpected condition at all.
The exit(128) we do is simply propagating the error report of the
helper.

That's the common error case: the message is redundant and annoying. The
_uncommon_  case is the one Yuri hit: some library misconfiguration that
causes the helper not to run at all.  Adding back any message is hurting
the common case to help the uncommon one.


But you can use the error code value to convey the cause of the failure 
to git, and avoid an unnecessary message in git itself. Based on the 
error code value git could tell if the error has already been reported 
to user.


Yuri
--
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: BUG: [Cosmetic] Commiting a gerrit ChangeId before the commit hook was installed

2014-01-17 Thread Jonathan Nieder
Hi,

Strainu wrote:

 strainu@emily:~/core git review -f
 Creating a git remote called gerrit that maps to:
 ssh://stra...@gerrit.wikimedia.org:29418/pywikibot/core.git
 Your change was committed before the commit hook was installed.
 Amending the commit to add a gerrit change id.

 At this point I ended the transaction, as I was confused by the last
 message: I was afraid the ChangeId would have changed, causing the
 patch to be attached to another review.

 I think git should not show this message if the change description
 already has a change id

This message doesn't come from git.  It comes from the git-review
tool (in git_review/cmd.py), so cc-ing the authors in case they
have thoughts on that.

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: git quietly fails on https:// URL, https errors are never reported to user

2014-01-17 Thread Jeff King
On Fri, Jan 17, 2014 at 12:39:39PM -0800, Yuri wrote:

 That second line is not adding anything, and IMHO is making things
 uglier and more confusing. We_expected_  the helper to hang up; that's
 how it signals an error to us. It is not an unexpected condition at all.
 The exit(128) we do is simply propagating the error report of the
 helper.
 
 That's the common error case: the message is redundant and annoying. The
 _uncommon_  case is the one Yuri hit: some library misconfiguration that
 causes the helper not to run at all.  Adding back any message is hurting
 the common case to help the uncommon one.
 
 But you can use the error code value to convey the cause of the
 failure to git, and avoid an unnecessary message in git itself. Based
 on the error code value git could tell if the error has already been
 reported to user.

Yes, we can, but that is in the same boat as a protocol change: you have
to teach every remote helper (some of which are written by third
parties) to communicate over this sideband channel.

It's should be slightly easier than a change to the protocol text,
because it's mostly backwards compatible (helpers should already be
returning a non-zero error code). I think there is some complication
with exit codes and remote-helpers, where you cannot expect just check
the exit code at any time. I _think_ from previous discussions that it
is safe to waitpid() on the helper after we have gotten EOF on the
reading pipe, though.

-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: BUG: [Cosmetic] Commiting a gerrit ChangeId before the commit hook was installed

2014-01-17 Thread Strainu
2014/1/17 Jonathan Nieder jrnie...@gmail.com:
 Hi,

 Strainu wrote:

 strainu@emily:~/core git review -f
 Creating a git remote called gerrit that maps to:
 ssh://stra...@gerrit.wikimedia.org:29418/pywikibot/core.git
 Your change was committed before the commit hook was installed.
 Amending the commit to add a gerrit change id.

 At this point I ended the transaction, as I was confused by the last
 message: I was afraid the ChangeId would have changed, causing the
 patch to be attached to another review.

 I think git should not show this message if the change description
 already has a change id

 This message doesn't come from git.  It comes from the git-review
 tool (in git_review/cmd.py), so cc-ing the authors in case they
 have thoughts on that.

Thanks for clarifying that. I'll log a bug on launchpad then.

Strainu
--
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: Workflow on git with 2 branch with specifc code

2014-01-17 Thread brian m. carlson
On Fri, Jan 17, 2014 at 10:14:28AM -0200, Gordon Freeman wrote:
 Hello guys, im Gordon.

 I have a question about workflow with git that i dont know if im doing
 it right.
 I have 1 repo with 2 branchs the first is the master of the project.
 the second is a branch copy of the master but he need to have some
 specifc code because is code for a client.
 so, every time that i updade master i need to merge master with client
 branch and it give me conflicts of course that will hapen.
 Well if was just me who work on this 2 branchs it will be easy to fix
 the conflicts and let all work and shine.
 But whe have here, 10 people woking on master branch and some times code
 are lost on merge and we need to look on commits to search whats goin
 on.
 What i just asking here is if its correct the workflow that i do. If for
 some problem like this, the community have a standard resolution. Or if
 what im doing here is all wrong.

There are many correct workflows.  I personally use the workflow you've
mentioned for the exact same reason (customizations for a client), but
I'm the only developer on that repository.

What you might try instead is a slightly different workflow.  Have each
developer create a feature branch to add a feature or fix a bug.  Merge
these into master as they become ready.  Have a specific person or group
of people be integrators, and have them merge master into the client
branch as necessary, fixing up any conflicts.  When conflicts are
non-trivial, use pair programming or a review process to ensure that the
result is good.

We use a similar workflow at my regular employer, and it is generally
very successful for a department with 45 employees.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] send-email: If the ca path is not specified, use the defaults

2014-01-17 Thread Kyle J. McKay

On Jan 17, 2014, at 10:14, Junio C Hamano wrote:

If I am reading the code correctly, if /etc/ssl/certs does not exist
on the filesystem at all, it wouldn't even attempt verification, so
I take your the verification will fail to mean that you forgot to
also mention And on OS X, /etc/ssl/certs directory still exists,
even though OpenSSL does not use it.  If that is the case, then our
current code indeed is broken in exactly the same way for OS X as
for Fedora.


My bad.  That directory does not normally exist, but, if some errant  
installer were to create an empty one...



In short, I agree with you on both counts (the current code is wrong
for OS X, and the proposed change will fix it).  I just want to make
sure that my understanding of the current breakage is in line with
the reality ;-)


Yes, you're right.  :)
--
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: [OpenStack-Infra] BUG: [Cosmetic] Commiting a gerrit ChangeId before the commit hook was installed

2014-01-17 Thread Strainu
Perhaps I haven't been clear enough: the commit already had a change
ID, added manually, so with or without the hook it would have been
attached to the correct review.

In this case, the hook will actually do nothing, making the current
wording of the message confusing IMO. My suggestion was [1] to change
the mesage to Amending the commit to add a gerrit change id if none
is available. or something similar.

Strainu

[1] https://bugs.launchpad.net/git-review/+bug/1270301

2014/1/18 Jerry Xinyu Zhao xyzje...@gmail.com:
 I think if you hadn't installed the commit hook for generating change ID,
 the commit indeed wouldn't have included a change ID, which is necessary for
 referencing the change when you submit a patch over it. There is nothing
 wrong with the message.  git review tool will install the hook and add a
 change ID for you automatically(a new feature of recent git-review release).



 On Fri, Jan 17, 2014 at 1:10 PM, Strainu strain...@gmail.com wrote:

 2014/1/17 Jonathan Nieder jrnie...@gmail.com:
  Hi,
 
  Strainu wrote:
 
  strainu@emily:~/core git review -f
  Creating a git remote called gerrit that maps to:
  ssh://stra...@gerrit.wikimedia.org:29418/pywikibot/core.git
  Your change was committed before the commit hook was installed.
  Amending the commit to add a gerrit change id.
 
  At this point I ended the transaction, as I was confused by the last
  message: I was afraid the ChangeId would have changed, causing the
  patch to be attached to another review.
 
  I think git should not show this message if the change description
  already has a change id
 
  This message doesn't come from git.  It comes from the git-review
  tool (in git_review/cmd.py), so cc-ing the authors in case they
  have thoughts on that.

 Thanks for clarifying that. I'll log a bug on launchpad then.

 Strainu

 ___
 OpenStack-Infra mailing list
 openstack-in...@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra


--
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/3] setup_pager: set MORE=R

2014-01-17 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Perhaps we can start it like this.  Then pager.c can iterate over
 the PAGER_ENV string, parse out LESS=, LV=, etc., and do its thing.

 We would also need to muck with git-sh-setup.sh but that file is
 already preprocessed in the Makefile, so we should be able to do
 something similar to # @@BROKEN_PATH_FIX@@ there.

And here is such an attempt.  There may be some missing dependencies
that need to be covered with a mechanism like GIT-BUILD-OPTIONS,
though.

 Makefile | 18 --
 config.mak.uname |  1 +
 git-sh-setup.sh  |  9 +
 pager.c  | 44 +---
 4 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/Makefile b/Makefile
index b4af1e2..690f4c6 100644
--- a/Makefile
+++ b/Makefile
@@ -342,6 +342,14 @@ all::
 # Define DEFAULT_HELP_FORMAT to man, info or html
 # (defaults to man) if you want to have a different default when
 # git help is called without a parameter specifying the format.
+#
+# Define PAGER_ENV to a SP separated VAR=VAL pairs to define
+# default environment variables to be passed when a pager is spawned, e.g.
+#
+#PAGER_ENV = LESS=-FRSX LV=-c
+#
+# to say export LESS=-FRSX (and LV=-c) if the environment variable
+# LESS (and LV) is not set, respectively.
 
 GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1506,6 +1514,10 @@ ifeq ($(PYTHON_PATH),)
 NO_PYTHON = NoThanks
 endif
 
+ifndef PAGER_ENV
+PAGER_ENV = LESS=-FRSX LV=-c
+endif
+
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -1585,11 +1597,12 @@ PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 DIFF_SQ = $(subst ','\'',$(DIFF))
 PERLLIB_EXTRA_SQ = $(subst ','\'',$(PERLLIB_EXTRA))
+PAGER_ENV_SQ = $(subst ','\'',$(PAGER_ENV))
 
 LIBS = $(GITLIBS) $(EXTLIBS)
 
 BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
-   $(COMPAT_CFLAGS)
+   $(COMPAT_CFLAGS) -DPAGER_ENV='$(PAGER_ENV)'
 LIB_OBJS += $(COMPAT_OBJS)
 
 # Quote for C
@@ -1739,7 +1752,7 @@ common-cmds.h: $(wildcard Documentation/git-*.txt)
 
 SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
-   $(gitwebdir_SQ):$(PERL_PATH_SQ)
+   $(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV)
 define cmd_munge_script
 $(RM) $@ $@+  \
 sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
@@ -1751,6 +1764,7 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 -e $(BROKEN_PATH_FIX) \
 -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \
 -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
+-e 's|@@PAGER_ENV@@|$(PAGER_ENV_SQ)|g' \
 $@.sh $@+
 endef
 
diff --git a/config.mak.uname b/config.mak.uname
index 7d31fad..8aea8a6 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -188,6 +188,7 @@ ifeq ($(uname_S),FreeBSD)
endif
PYTHON_PATH = /usr/local/bin/python
HAVE_PATHS_H = YesPlease
+   PAGER_ENV = LESS=-FRSX LV=-c MORE=-R
 endif
 ifeq ($(uname_S),OpenBSD)
NO_STRCASESTR = YesPlease
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index fffa3c7..8fc1bbd 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -158,10 +158,11 @@ git_pager() {
else
GIT_PAGER=cat
fi
-   : ${LESS=-FRSX}
-   : ${LV=-c}
-   export LESS LV
-
+   for vardef in @@PAGER_ENV@@
+   do
+   var=${vardef%%=*}
+   eval : \${$vardef}  export $var
+   done
eval $GIT_PAGER '$@'
 }
 
diff --git a/pager.c b/pager.c
index 0cc75a8..81a8af7 100644
--- a/pager.c
+++ b/pager.c
@@ -1,6 +1,7 @@
 #include cache.h
 #include run-command.h
 #include sigchain.h
+#include argv-array.h
 
 #ifndef DEFAULT_PAGER
 #define DEFAULT_PAGER less
@@ -60,9 +61,37 @@ const char *git_pager(int stdout_is_tty)
return pager;
 }
 
+#define stringify_(x) #x
+#define stringify(x) stringify_(x)
+
+static void setup_pager_env(struct argv_array *env)
+{
+   const char *pager_env = stringify(PAGER_ENV);
+
+   while (*pager_env) {
+   struct strbuf buf = STRBUF_INIT;
+   const char *cp = strchrnul(pager_env, '=');
+
+   if (!*cp)
+   die(malformed build-time PAGER_ENV);
+   strbuf_add(buf, pager_env, cp - pager_env);
+   cp = strchrnul(pager_env, ' ');
+   if (!getenv(buf.buf)) {
+   strbuf_reset(buf);
+   strbuf_add(buf, pager_env, cp - pager_env);
+   argv_array_push(env, strbuf_detach(buf, NULL));
+   }
+   strbuf_reset(buf);
+   while (*cp  *cp == ' ')
+   cp++;
+   pager_env = cp;
+   }
+}
+
 void setup_pager(void)
 {
const char *pager = git_pager(isatty(1));
+   struct argv_array env = ARGV_ARRAY_INIT;
 
if (!pager || pager_in_use())
return;
@@ -80,17 +109,10 @@ void 

Re: [PATCH 1/5] diff_filespec: reorder dirty_submodule macro definitions

2014-01-17 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 I'm happy with it either way. I almost just pulled the macro
 definitions, including DIFF_FILE_VALID, out of the struct definition
 completely. I see the value in having the flags near their bitfield, but
 it makes the definition a bit harder to read.

Yeah, my thoughts exactly when I did those two conflicting changes.
I have a slight preference Constants go with the fields they are
used in over fields and macros mixed together is harder to read,
so let's use your patch as-is.
--
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


Date format in 'git log' should be in local timezone

2014-01-17 Thread Yuri
Currently git log mixes timezones in the date records in the same log, 
so the following dates wold appear in one log:

Date:   Thu Jan 16 17:11:28 2014 -0800
Date:   Thu Jan 16 20:10:04 2014 -0500

Timezone here doesn't help the log reader at all. It doesn't even 
reflect the actual location of the submitter. Instead, it should be 
converted to the local TZ of the client. This will make it easier to 
read and understand the time.


Even further, timezone shouldn't even be stored by the git server. It 
should just store the UTC time, following the approach how time is 
managed in most UNIX-like systems.


Yuri
--
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: Date format in 'git log' should be in local timezone

2014-01-17 Thread Jonathan Nieder
Hi,

Yuri wrote:

 Timezone here doesn't help the log reader at all. It doesn't even
 reflect the actual location of the submitter. Instead, it should be
 converted to the local TZ of the client. This will make it easier to
 read and understand the time.

Does git log --date=local or git log --date=relative do what
you're looking for?

If so, you can set that permanently by setting 'date = local' or
'date = relative' in the [log] section of your ~/.gitconfig.  See
log.date in the git-config(1) manpage for details.

I wonder if 'date = relative' would make a better default.

 Even further, timezone shouldn't even be stored by the git server.

I've found it very useful and would consider that a regression, at
least.

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


Re: Consistency question

2014-01-17 Thread Mike Hommey
On Wed, Jan 15, 2014 at 06:13:30AM -0500, Jeff King wrote:
 On Wed, Jan 15, 2014 at 11:37:08AM +0100, David Kastrup wrote:
 
  The question is what guarantees I have with regard to the commit date of
  a commit in relation to that of its parent commits:
  
  a) none
  b) commitdate(child) = commitdate(parent)
  c) commitdate(child)  commitdate(parent)
 
 a) none
 
  Obviously, I can rely on c) being true almost always:
 
 Actually, b) is quite often the case in automated processes (e.g., git
 am or git rebase). The author dates are different, but the committer
 dates may be in the same second.
 
 And of course a) is the result of clock skew and software bugs.

... or importing non-git repositories that don't have commit info
separated from author info like git does. In such cases, it's usual to
duplicate the author info as commit info so that clones of the same
non-git repo end up with the same git sha1s. Mercurial easily allows
author dates to be in a non topological order.

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


Want to do some cleanups in this round of l10n

2014-01-17 Thread Jiang Xin
Hi,

I want to do some cleaning in this round of l10n:

Removing two po files (da.po and nl.po) without a single
translation for almost 2 years.

Statistics for other languages:

 * There are five full translations for German, French,
Swedish, Vietnamese and Simplified Chinese;

 * 2 partial translations for for Italian and Portuguese;

 * and one for l10n test (is.po is used in testcases).

Statistics for Git l10n:

da.po : 0 translated messages, 724 untranslated messages.
de.po : 2192 translated messages, 2 untranslated messages.
fr.po : 2194 translated messages.
is.po : 14 translated messages.
it.po : 716 translated messages, 350 untranslated messages.
nl.po : 0 translated messages, 722 untranslated messages.
pt_PT.po  : 306 translated messages, 687 untranslated messages.
sv.po : 2194 translated messages.
vi.po : 2194 translated messages.
zh_CN.po  : 2194 translated messages.

Any suggestions?

2014/1/18 Jiang Xin worldhello@gmail.com:
 Hi All,

 Since Git v1.9-rc0 had already been released, it's time to start new round
 of git l10n. This time there are 27 new messages need to be translated.
 The new git.pot is generated in commit v1.9-rc0-1-gdf49095:

 l10n: git.pot: v1.9 round 1 (27 new, 11 removed)

 Generate po/git.pot from v1.9-rc0 for git v1.9 l10n round 1.

 Signed-off-by: Jiang Xin worldhello@gmail.com

 You can get it from the usual place:

 https://github.com/git-l10n/git-po/

 As how to update your XX.po and help to translate Git, please see
 Updating a XX.po file and other sections in “po/README file.

 --
 Jiang Xin


-- 
Jiang Xin
--
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: Workflow on git with 2 branch with specifc code

2014-01-17 Thread Jon Seymour
On Sat, Jan 18, 2014 at 10:05 AM, brian m. carlson
sand...@crustytoothpaste.net wrote:
 On Fri, Jan 17, 2014 at 10:14:28AM -0200, Gordon Freeman wrote:
 Hello guys, im Gordon.

 I have a question about workflow with git that i dont know if im doing
 it right.
 I have 1 repo with 2 branchs the first is the master of the project.
 the second is a branch copy of the master but he need to have some
 specifc code because is code for a client.
 so, every time that i updade master i need to merge master with client
 branch and it give me conflicts of course that will hapen.
 Well if was just me who work on this 2 branchs it will be easy to fix
 the conflicts and let all work and shine.
 But whe have here, 10 people woking on master branch and some times code
 are lost on merge and we need to look on commits to search whats goin
 on.
 What i just asking here is if its correct the workflow that i do. If for
 some problem like this, the community have a standard resolution. Or if
 what im doing here is all wrong.

 There are many correct workflows.  I personally use the workflow you've
 mentioned for the exact same reason (customizations for a client), but
 I'm the only developer on that repository.


I agree with Brian that there are many correct workflows and which one
you choose does depend on details of the branches you are trying to
manage.

Myself, I would tend to avoid a workflow in which you continually
merge from master into the client branch. The reason is that once you
have done this 20 times or so it will become quite difficult to
understand how and why the client branch diverged from the master
branch. Yes, it is in the history, but reasoning about diffs that
cross merge points is just hard.

Assuming that there is not much actual development on the client
branch, but rather a relatively small set of customizations to
configuration and things of that kind, then I would tend to maintain
the client changes as topic branch, then maintain a client integration
branch which represents the merge between master and the client topic
branch. Changes that represent divergence of the client from the
master branch would be committed to the client topic branch and then
merged into the client integration branch.  Refreshes from master
would be merged into the integration branch. Commits directly to the
integration branch would be avoided where possible.

Once master has diverged from client enough that there start to be
frequent conflicts when merging into the integration branch, then
consider rebasing the client topic branch onto the tip of master
branch and then repeat the cycle again. There is some risk of history
loss with this approach - a later release of the client branch may not
be a direct descendent of an earlier release of the client branch, but
even this problem can be solved with judicious use of merge -s ours
after you have successfully rebased the client topic branch. I can
expand on how you do this, if there is interest.

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