Re: git log --no-walk --tags produces strange result for certain user
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
(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
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
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
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
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
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
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
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
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
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
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
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
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
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/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
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
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
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
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
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
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
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
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
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
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