[PATCH] send-pack: don't send a thin pack when the server doesn't support it
Not every server out there supports fixing thin packs, so let's send them a full pack. Signed-off-by: Carlos Martín Nieto c...@elego.de --- It's not always possible to support thin packs (sometimes there isn't even an object database to grab bases out of). And in any case git shouldn't create thin packs if the server hasn't said it knows how to fix them, as per the point of the extension. send-pack.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/send-pack.c b/send-pack.c index 7d172ef..7b88ac8 100644 --- a/send-pack.c +++ b/send-pack.c @@ -205,6 +205,8 @@ int send_pack(struct send_pack_args *args, quiet_supported = 1; if (server_supports(agent)) agent_supported = 1; + if (!server_supports(thin-pack)) + args-use_thin_pack = 0; if (!remote_refs) { fprintf(stderr, No refs in common and none specified; doing nothing.\n -- 1.8.4.561.g1c3d45d -- 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-pack: don't send a thin pack when the server doesn't support it
On Tue, Oct 8, 2013 at 3:44 PM, Carlos Martín Nieto c...@elego.de wrote: diff --git a/send-pack.c b/send-pack.c index 7d172ef..7b88ac8 100644 --- a/send-pack.c +++ b/send-pack.c @@ -205,6 +205,8 @@ int send_pack(struct send_pack_args *args, quiet_supported = 1; if (server_supports(agent)) agent_supported = 1; + if (!server_supports(thin-pack)) + args-use_thin_pack = 0; Hmm.. I think this introduces a regression in C Git because receive-pack never advertises thin-pack and so git push from now on will never send thin packs. It's too late now to add thin-pack to receive-pack, perhaps a new extension no-thin-pack for those servers? Alternatively, just run git push --no-thin (you'll need f7c815c (push: respect --no-thin - 2013-08-12) though). if (!remote_refs) { fprintf(stderr, No refs in common and none specified; doing nothing.\n -- 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
[PATCH] clone: do not segfault when specifying a nonexistent branch
Actually I only wanted to change one line to prevent a crash, when you specify a non existing branch when cloning: - if (option_branch) { + if (option_branch our_head_points_at) { However it turns out this is not a good idea as we still want to setup 'remote.*.fetch', which previously depended the string buffer 'value' being non empty. Therefore I added a local variable 'set_remote', which determines whether we want to setup 'remote.*.fetch'. While staring at the code, I also think it is a good idea to restructure the if clauses a little as previously we had if (option_mirror || !option_bare) { if (option_single_branch !option_mirror) { The 'option_mirror' is part of both ifs, but opposing each other. This is not yet done in this patch, as it still needs some thinking how to remove the nesting of the if clauses in a nice way. Reported-by: Robert Mitwicki robert.mitwi...@opensoftware.pl Signed-off-by: Stefan Beller stefanbel...@googlemail.com --- builtin/clone.c | 50 -- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 0aff974..8b9a78a 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -686,40 +686,46 @@ static void write_refspec_config(const char* src_ref_prefix, struct strbuf key = STRBUF_INIT; struct strbuf value = STRBUF_INIT; + int set_remote = 0; if (option_mirror || !option_bare) { + set_remote = 1; if (option_single_branch !option_mirror) { if (option_branch) { - if (strstr(our_head_points_at-name, refs/tags/)) - strbuf_addf(value, +%s:%s, our_head_points_at-name, - our_head_points_at-name); - else - strbuf_addf(value, +%s:%s%s, our_head_points_at-name, - branch_top-buf, option_branch); + if (our_head_points_at) { + if (strstr(our_head_points_at-name, refs/tags/)) + strbuf_addf(value, +%s:%s, our_head_points_at-name, + our_head_points_at-name); + else + strbuf_addf(value, +%s:%s%s, our_head_points_at-name, + branch_top-buf, option_branch); + } } else if (remote_head_points_at) { strbuf_addf(value, +%s:%s%s, remote_head_points_at-name, branch_top-buf, skip_prefix(remote_head_points_at-name, refs/heads/)); + } else { + /* +* otherwise, the next git fetch will +* simply fetch from HEAD without updating +* any remote-tracking branch, which is what +* we want. +*/ + set_remote = 0; } - /* -* otherwise, the next git fetch will -* simply fetch from HEAD without updating -* any remote-tracking branch, which is what -* we want. -*/ } else { strbuf_addf(value, +%s*:%s*, src_ref_prefix, branch_top-buf); } - /* Configure the remote */ - if (value.len) { - strbuf_addf(key, remote.%s.fetch, option_origin); - git_config_set_multivar(key.buf, value.buf, ^$, 0); - strbuf_reset(key); + } + /* Configure the remote */ + if (set_remote) { + strbuf_addf(key, remote.%s.fetch, option_origin); + git_config_set_multivar(key.buf, value.buf, ^$, 0); + strbuf_reset(key); - if (option_mirror) { - strbuf_addf(key, remote.%s.mirror, option_origin); - git_config_set(key.buf, true); - strbuf_reset(key); - } + if (option_mirror) { + strbuf_addf(key, remote.%s.mirror, option_origin); + git_config_set(key.buf, true); + strbuf_reset(key); } } -- 1.8.4.1.469.gb38b9db -- To unsubscribe from this list: send the line
Re: [PATCH] send-pack: don't send a thin pack when the server doesn't support it
Duy Nguyen pclo...@gmail.com writes: On Tue, 2013-10-08 at 16:44 +0700, Duy Nguyen wrote: On Tue, Oct 8, 2013 at 3:44 PM, Carlos Martín Nieto c...@elego.de wrote: diff --git a/send-pack.c b/send-pack.c index 7d172ef..7b88ac8 100644 --- a/send-pack.c +++ b/send-pack.c @@ -205,6 +205,8 @@ int send_pack(struct send_pack_args *args, quiet_supported = 1; if (server_supports(agent)) agent_supported = 1; + if (!server_supports(thin-pack)) + args-use_thin_pack = 0; Hmm.. I think this introduces a regression in C Git because receive-pack never advertises thin-pack and so git push from now on will never send thin packs. It's too late now to add thin-pack to Oh, I'd never noticed that when looking though the network traffic. This seems like something that breaks the compatibility that git otherwise tries so hard to maintain. How did it happen that it's fine for the client to assume that the server supports thin packs? receive-pack, perhaps a new extension no-thin-pack for those servers? Alternatively, just run git push --no-thin (you'll need f7c815c (push: respect --no-thin - 2013-08-12) though). Yeah, I had an older version in my PATH and was bitten by that when testing. Having --no-thin and the server's index-pack fail with missing bases is quite worrying when you're the one who wrote most of the server-side code. Having to remember to run 'git push --no-thin' when pushing to one particular project is pretty bad experience as a user and I was hoping to avoid this with newer gits. We could advertise a no-thin-pack extension if a patch to support that would be accepted into mainline git. Cheers, cmn -- 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-pack: don't send a thin pack when the server doesn't support it
On Tue, Oct 8, 2013 at 4:44 PM, Duy Nguyen pclo...@gmail.com wrote: On Tue, Oct 8, 2013 at 3:44 PM, Carlos Martín Nieto c...@elego.de wrote: diff --git a/send-pack.c b/send-pack.c index 7d172ef..7b88ac8 100644 --- a/send-pack.c +++ b/send-pack.c @@ -205,6 +205,8 @@ int send_pack(struct send_pack_args *args, quiet_supported = 1; if (server_supports(agent)) agent_supported = 1; + if (!server_supports(thin-pack)) + args-use_thin_pack = 0; Hmm.. I think this introduces a regression in C Git because receive-pack never advertises thin-pack and so git push from now on will never send thin packs. It's too late now to add thin-pack to receive-pack Or maybe it's not that late. How about you go with your patch and add thin-pack capability to receive-pack too? When new git push is used against old server, thin pack is disabled. But that's not a big deal (I hope). The difference is traffic increases a bit more, but cpu consumption on the server side is also a bit less. Pushes are usually small, unless you do initial push, which is complete anyway. So a bit more or less does not really matter in practice. We could mitigate the regression a bit by checking agent string and enable thin-pack for those versions even if thin-pack is not advertises. That goes back to v1.7.12. We may do the same for some other popular git servers. And this hack is maintained like 3-4 years, enough time for new versions to be deployed, then removed. Hmm? -- 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
[RFC] Documentation: --options in man-pages synopsys
Hi, I've noticed that man git-pack-objects describes cmdline as following SYNOPSYS 'git pack-objects' [-q | --progress | --all-progress] [--all-progress-implied] [--no-reuse-delta] [--delta-base-offset] [--non-empty] [--local] [--incremental] [--window=n] [--depth=n] [--revs [--unpacked | --all]] [--stdout | base-name] [--keep-true-parents] object-list while OPTIONS sections has even more options, --no-reuse-objects for instance. Should it be dealt with and how? - add smth like ... at the tail of options in synopsys to indicate that there are more options - add all the [--options] to synopsys - drop all the [--options] as they all are optional - pick only the most common/important ones like -q --progress, per command or per command classes (hard to maintain and/or verify?) -- 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
HALIFAX FINANCE LOANS
Please Kindly find the attached file. PDF.pdf Description: Adobe PDF document
RE: [RFC] Documentation: --options in man-pages synopsys
As a unix user I'd expect the SYNOPSIS section at the top of the man page to include all options that the command accepts. Mutually exclusive options are expected to be in the form [-q | --progress | --all-progress], such is already done. I believe that you'd be safe in following http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap12.html unless the git-* family of commands and documentation deviate from it in a way that I am not aware of. Hope this helps, --Dave -Original Message- From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On Behalf Of Dmitry Ivankov Sent: Tuesday, October 08, 2013 7:07 AM To: Git List Subject: [RFC] Documentation: --options in man-pages synopsys Hi, I've noticed that man git-pack-objects describes cmdline as following SYNOPSYS 'git pack-objects' [-q | --progress | --all-progress] [--all-progress-implied] [-- no-reuse-delta] [--delta-base-offset] [--non-empty] [--local] [--incremental] [--window=n] [--depth=n] [--revs [--unpacked | --all]] [--stdout | base- name] [--keep-true-parents] object-list while OPTIONS sections has even more options, --no-reuse-objects for instance. Should it be dealt with and how? - add smth like ... at the tail of options in synopsys to indicate that there are more options - add all the [--options] to synopsys - drop all the [--options] as they all are optional - pick only the most common/important ones like -q --progress, per command or per command classes (hard to maintain and/or verify?) -- 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 -- 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] blame: add color
Add colorized text for git blame when color.interactive is enabled. This work is based on the colorization code in builtin/clean.c. Signed-off-by: Chris J Arges christopherar...@gmail.com --- builtin/blame.c | 51 +-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 6da7233..fbff437 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -23,6 +23,7 @@ #include userdiff.h #include line-range.h #include line-log.h +#include color.h static char blame_usage[] = N_(git blame [options] [rev-opts] [rev] [--] file); @@ -51,6 +52,24 @@ static size_t blame_date_width; static struct string_list mailmap; +static int blame_use_color = -1; +static char blame_colors[][COLOR_MAXLEN] = { + GIT_COLOR_RESET, + GIT_COLOR_NORMAL, /* PLAIN */ + GIT_COLOR_YELLOW, /* COMMIT */ + GIT_COLOR_BOLD, /* NAME */ + GIT_COLOR_CYAN, /* LINE */ + GIT_COLOR_GREEN,/* TIME */ +}; +enum color_blame { + BLAME_COLOR_RESET = 0, + BLAME_COLOR_PLAIN = 1, + BLAME_COLOR_COMMIT = 2, + BLAME_COLOR_NAME = 3, + BLAME_COLOR_LINE = 4, + BLAME_COLOR_TIME = 5, +}; + #ifndef DEBUG #define DEBUG 0 #endif @@ -1575,6 +1594,18 @@ static void assign_blame(struct scoreboard *sb, int opt) } } +static const char *blame_get_color(enum color_blame ix) +{ + if (want_color(blame_use_color)) + return blame_colors[ix]; + return ; +} + +static void blame_print_color(enum color_blame ix) +{ + printf(%s, blame_get_color(ix)); +} + static const char *format_time(unsigned long time, const char *tz_str, int show_raw_time) { @@ -1680,7 +1711,9 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt) } } + blame_print_color(BLAME_COLOR_COMMIT); printf(%.*s, length, hex); + blame_print_color(BLAME_COLOR_RESET); if (opt OUTPUT_ANNOTATE_COMPAT) { const char *name; if (opt OUTPUT_SHOW_EMAIL) @@ -1711,14 +1744,22 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt) else name = ci.author.buf; pad = longest_author - utf8_strwidth(name); - printf( (%s%*s %10s, - name, pad, , + blame_print_color(BLAME_COLOR_NAME); + printf( (%s%*s , + name, pad, ); + blame_print_color(BLAME_COLOR_RESET); + blame_print_color(BLAME_COLOR_TIME); + printf( (%10s, format_time(ci.author_time, ci.author_tz.buf, show_raw_time)); + + blame_print_color(BLAME_COLOR_RESET); } + blame_print_color(BLAME_COLOR_LINE); printf( %*d) , max_digits, ent-lno + 1 + cnt); + blame_print_color(BLAME_COLOR_RESET); } do { ch = *cp++; @@ -1948,6 +1989,12 @@ static int git_blame_config(const char *var, const char *value, void *cb) blame_date_mode = parse_date_format(value); return 0; } + /* honors the color.interactive* config variables which also + applied in git-add--interactive and git-stash */ + if (!strcmp(var, color.interactive)) { + blame_use_color = git_config_colorbool(var, value); + return 0; + } if (userdiff_config(var, value) 0) return -1; -- 1.7.9.5 -- 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] git-credential-netrc: fix uninitialized warning
Simple patch to avoid unitialized warning and log what we'll do. --- contrib/credential/netrc/git-credential-netrc | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc index 6c51c43..13e537b 100755 --- a/contrib/credential/netrc/git-credential-netrc +++ b/contrib/credential/netrc/git-credential-netrc @@ -369,7 +369,10 @@ sub find_netrc_entry { { my $entry_text = join ', ', map { $_=$entry-{$_} } keys %$entry; foreach my $check (sort keys %$query) { - if (defined $query-{$check}) { + if (!defined $entry-{$check}) { + log_debug(OK: entry has no $check token, so any value satisfies check $check); + } + elsif (defined $query-{$check}) { log_debug(compare %s [%s] to [%s] (entry: %s), $check, $entry-{$check}, -- 1.8.1.5 -- 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: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string
On Tue, 3 Sep 2013 13:35:44 -0400 Jeff King p...@peff.net wrote: JK On Tue, Sep 03, 2013 at 11:23:14AM -0400, Ted Zlatanov wrote: Yes, you're right. Something like the following (untested) could work and does the wildcards, which I will make into a proper patch and test if it looks OK to you. ... JK Yeah, that makes sense to me (and is basically what the credential-cache JK and credential-store helpers do internally). Thanks for working on this. Sorry for the delay, patch posted. Ted -- 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] blame: add color
On 10/08/2013 04:18 PM, Chris J Arges wrote: Add colorized text for git blame when color.interactive is enabled. This work is based on the colorization code in builtin/clean.c. Another way to color blame, would be by commit, not by row. Try git gui blame file, to see what I mean. The colorisation would be per commit and not by line. Though I guess that's a lot of more work, but I'd personally find it better, for what it's worth. +static const char *blame_get_color(enum color_blame ix) +{ + if (want_color(blame_use_color)) + return blame_colors[ix]; + return ; +} The only occurence of blame_get_color is in blame_print_color, so why not move it in there? + +static void blame_print_color(enum color_blame ix) +{ + printf(%s, blame_get_color(ix)); +} + static const char *format_time(unsigned long time, const char *tz_str, Thanks, Stefan signature.asc Description: OpenPGP digital signature
Error in creating git over http
I'm trying to set a http git server. I do the following on the git server: cd /var/www/html/ git clone --bare /path/to/dir/ gitproject.git cd gitproject.git/ mv hooks/post-update.sample hooks/post-update chmod a+x hooks/post-update on the local machine run: git clone http://server.name/gitproject.git I got the error: Initialized empty Git repository in /local/path/gitproject/.git/ fatal: http://server.name/gitproject.git/info/refs not found: did you run git update-server-info on the server? I saw that the file does not exist, it seem that the file post-update is not execute. I run it on the git server: git update-server-info Now the info/ref is created. On local machine I run again : git clone http://server.name/gitproject.git Now I get the error: Initialized empty Git repository in /local/path/gitproject/.git/ error: The requested URL returned error: 403 (curl_result = 22, http_code = 403, sha1 = 9d83b83df9fbc75ecd754264f95793fca93ccf93) error: Unable to find 9d83b83df9fbc75ecd754264f95793fca93ccf93 under http://server.name/gitproject.git Cannot obtain needed object 9d83b83df9fbc75ecd754264f95793fca93ccf93 What I'm doing wrong? Any ideas are welcome. 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] blame: add color
Chris J Arges christopherar...@gmail.com writes: Add colorized text for git blame when color.interactive is enabled. It does not make any sense to tie this to color.interactive at all, at least to me. The check color.blame and if absent fall back to color.ui, which is the usual pattern, would be more appropriate. +static char blame_colors[][COLOR_MAXLEN] = { + GIT_COLOR_RESET, + GIT_COLOR_NORMAL, /* PLAIN */ + GIT_COLOR_YELLOW, /* COMMIT */ + GIT_COLOR_BOLD, /* NAME */ + GIT_COLOR_CYAN, /* LINE */ + GIT_COLOR_GREEN,/* TIME */ +}; Unlike git grep, where some pieces of a single line are more interesting (i.e. the exact text matching the pattern given) than others (i.e. other text on the same line), and git diff, where some lines have different meanings from others (i.e. hunk header, deleted lines, added lines, context lines), the output from git blame is already columnar and it is obvious to the reader of the output that everything on the leftmost part of all lines is commit object name, without distraction of extra colours; I do not see much point in painting the output into vertical stripes of colours. It may make more sense to assign one colour to each blame origin (i.e. commit, path pair), to make it clear that first five lines came from the same origin that is different from the two lines that follow, etc., showing horizontal stripes of colours. If we were to go that route, I suspect that it would be too distracting to paint the whole line (like git diff does for added or deleted lines). Perhaps paint only the commit object name part in different colors to show which lines form a group that came from the same origin? The way git show-branch paints its output might give an inspiration. -- 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] blame: add color
On 10/08/2013 10:16 AM, Junio C Hamano wrote: Chris J Arges christopherar...@gmail.com writes: Add colorized text for git blame when color.interactive is enabled. It does not make any sense to tie this to color.interactive at all, at least to me. The check color.blame and if absent fall back to color.ui, which is the usual pattern, would be more appropriate. Ok, I wasn't entirely sure of convention, but I could modify it to do this. +static char blame_colors[][COLOR_MAXLEN] = { +GIT_COLOR_RESET, +GIT_COLOR_NORMAL, /* PLAIN */ +GIT_COLOR_YELLOW, /* COMMIT */ +GIT_COLOR_BOLD, /* NAME */ +GIT_COLOR_CYAN, /* LINE */ +GIT_COLOR_GREEN,/* TIME */ +}; Unlike git grep, where some pieces of a single line are more interesting (i.e. the exact text matching the pattern given) than others (i.e. other text on the same line), and git diff, where some lines have different meanings from others (i.e. hunk header, deleted lines, added lines, context lines), the output from git blame is already columnar and it is obvious to the reader of the output that everything on the leftmost part of all lines is commit object name, without distraction of extra colours; I do not see much point in painting the output into vertical stripes of colours. I mainly did this because it makes it easier for me to see where the different columns start and stop. It may make more sense to assign one colour to each blame origin (i.e. commit, path pair), to make it clear that first five lines came from the same origin that is different from the two lines that follow, etc., showing horizontal stripes of colours. If we were to go that route, I suspect that it would be too distracting to paint the whole line (like git diff does for added or deleted lines). Perhaps paint only the commit object name part in different colors to show which lines form a group that came from the same origin? The way git show-branch paints its output might give an inspiration. This could provide a more useful colorization; in addition if something as simple as the line number was colorized it would provide a easy way for me to see where the code column starts. --chris j arges -- 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: gitk next/prev buttons
On 13-09-30 10:31 PM, Lucas Sandery [three am design] wrote: The next and prev buttons are lacking consistency and logic. For RTL languages previous is almost always on the left, and next on the right. The words are contradictory, next actually goes to backwards chronologically, and prev goes forward. Could the positions of the buttons be switched and the text be replaced with earlier and later? This would make the reference time not just order, and would mean the updated UI would not be confused with the previous one. I agree with you here (I like older and newer myself). In theory gitk accepts the same arguments as git rev-list, and in theory you should be able to use things like --reverse to get the commits in a different order. However, in practice I don't think anyone futzes with the ordering much with gitk. gitk even ignores the --reverse and --timestamp options, and I've had a hard time getting gitk to display commits in an order where older and newer would not make sense. Also, the arrow-only next/previous buttons (beside the commit ID) should really point up and down. Horizontal directions are ambiguous for navigation of a vertical list. Here I don't agree. These arrows are purely about history navigation. They don't have anything to do with up or down in the list. In that sense they're more like the arrows in your web browser. M. -- 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
A workflow for local patch maintenance
This is a copy of an article I published at http://fanf.livejournal.com/128282.html I'm sending a copy here because I'm interested to know what other ways there might be of handling this situation. -- We often need to patch the software that we run in order to fix bugs quickly rather than wait for an official release, or to add functionality that we need. In many cases we have to maintain a locally-developed patch for a significant length of time, across multiple upstream releases, either because it is not yet ready for incorporation into a stable upstream version, or because it is too specific to our setup so will not be suitable for passing upstream without significant extra work. I have been experimenting with a git workflow in which I have a feature branch per patch. (Usually there is only one patch for each change we make.) To move them on to a new feature release, I tag the feature branch heads (to preserve history), rebase them onto the new release version, and octopus merge them to create a new deployment version. This is rather unsatisfactory, because there is a lot of tedious per-branch work, and I would prefer to have branches recording the development of our patches rather than a series of tags. Here is a git workflow suggested by Ian Jackson which I am trying out instead. I don't yet have much experience with it; I am writing it down now as a form of documentation. There are three branches: upstream, which is where public releases live working, which is where development happens deployment, which is what we run Which branch corresponds to upstream may change over time, for instance when we move from one stable version to the next one. The working branch exists on the developer's workstation and is not normally published. There might be multiple working branches for work-in-progress. They get rebased a lot. Starting from an upstream version, a working branch will have a number of mature patches. The developer works on top of these in commit-early-commit-often mode, without worrying about order of changes or cleanliness. Every so often we use git rebase --interactive to tidy up the patch set. Often we'll use the squash command to combine new commits with the mature patches that they amend. Sometimes it will be rebased onto a new upstream version. When the working branch is ready, we use the commands below to update the deployment branch. The aim is to make it look like updates from the working branch are repeatedly merged into the deployment branch. This is so that we can push updated versions of the patch set to a server without having to use --force, and pulling updates into a checked out version is just a fast-forward. However this isn't a normal merge since the tree at the head of deployment always matches the most recent good version of working. (This is similar to what stg publish does.) Diagramatically, | 1.1 | \ | `A---B-- 1.1-patched |\ | | \ | | `C-- 1.1-revised || 2.0 | | \ | | `-C--D-- 2.0-patched || 3.1 | | \ | | `-C--E-- 3.1-patched || upstream| deployment The horizontal-ish lines are different rebased versions of the patch set. Letters represent patches and numbers represent version tags. The tags on the deployment branch are for the install scripts so I probably won't need one on every update. Ideally we would be able to do this with the following commands: $ git checkout deployment $ git merge -s theirs working However there is an ours merge strategy but not a theirs merge strategy. Johannes Sixt described how to simulate git merge -s theirs in a post to the git mailing list in 2010. http://article.gmane.org/gmane.comp.version-control.git/163631 So the commands are: $ git checkout deployment $ git merge --no-commit -s ours working $ git read-tree -m -u working $ git commit -m Update to $(git describe working) Mark Wooding suggested the following more plumbing-based version, which unlike the above does not involve switching to the deployment branch. $ d=$(git rev-parse deployment) $ w=$(git rev-parse working) $ c=$(echo Update to $(git describe working) | git commit-tree -p $d -p $w working^{tree}) $ git update-ref deployment $c $d $ unset c d w Tony. -- f.anthony.n.finch d...@dotat.at http://dotat.at/ Forties, Cromarty: East, veering southeast, 4 or 5, occasionally 6 at first. Rough, becoming slight or moderate. Showers, rain at first. Moderate or good, occasionally poor at first. -- 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: gitk next/prev buttons
Hi, Marc Branchaud wrote: On 13-09-30 10:31 PM, Lucas Sandery [three am design] wrote: The next and prev buttons are lacking consistency and logic. For LTR languages previous is almost always on the left, and next on the right. The words are contradictory, next actually goes to backwards chronologically, and prev goes forward. Could the positions of the buttons be switched and the text be replaced with earlier and later? This would make the reference time not just order, and would mean the updated UI would not be confused with the previous one. I agree with you here (I like older and newer myself). In theory gitk accepts the same arguments as git rev-list, and in theory you should be able to use things like --reverse to get the commits in a different order. However, in practice I don't think anyone futzes with the ordering much with gitk. gitk even ignores the --reverse and --timestamp options, and I've had a hard time getting gitk to display commits in an order where older and newer would not make sense. In a branchy history, it is possible for the next matching commit to actually be newer. I think the intent of the buttons is find the next result, looking down or up in the list of commits in the upper pane. Is there some other wording that would convey this better? Also, the arrow-only next/previous buttons (beside the commit ID) should really point up and down. Horizontal directions are ambiguous for navigation of a vertical list. Here I don't agree. These arrows are purely about history navigation. They don't have anything to do with up or down in the list. In that sense they're more like the arrows in your web browser. True. It might make sense to make the analogy to a web browser clearer by using thicker arrows or triangles --- care to mock up icons to replace the current ones? The icons are in the gitk source file (search for bm_left_data) in xpm format. 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: [PATCH] git-credential-netrc: fix uninitialized warning
Hi, Ted Zlatanov wrote: Simple patch to avoid unitialized warning and log what we'll do. Sign-off? [...] --- a/contrib/credential/netrc/git-credential-netrc +++ b/contrib/credential/netrc/git-credential-netrc @@ -369,7 +369,10 @@ sub find_netrc_entry { { my $entry_text = join ', ', map { $_=$entry-{$_} } keys %$entry; foreach my $check (sort keys %$query) { - if (defined $query-{$check}) { + if (!defined $entry-{$check}) { +log_debug(OK: entry has no $check token, so any value satisfies check $check); + } + elsif (defined $query-{$check}) { Style: elsewhere this file seems to use cuddled elses: } elsif (...) { Or more simply, would it make sense to wrap both 'defined' checks into a single if, like so? if (defined $entry-{$check} defined $query-{$check}) { ... } else { log_debug(...); } 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: [PATCH] git-credential-netrc: fix uninitialized warning
On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder jrnie...@gmail.com wrote: JN Ted Zlatanov wrote: Simple patch to avoid unitialized warning and log what we'll do. JN Sign-off? I didn't realize it was a requirement, must I? JN [...] --- a/contrib/credential/netrc/git-credential-netrc +++ b/contrib/credential/netrc/git-credential-netrc @@ -369,7 +369,10 @@ sub find_netrc_entry { { my $entry_text = join ', ', map { $_=$entry-{$_} } keys %$entry; foreach my $check (sort keys %$query) { -if (defined $query-{$check}) { +if (!defined $entry-{$check}) { + log_debug(OK: entry has no $check token, so any value satisfies check $check); +} +elsif (defined $query-{$check}) { JN Style: elsewhere this file seems to use cuddled elses: JN } elsif (...) { Ah, thanks, I missed that. JN Or more simply, would it make sense to wrap both 'defined' checks into JN a single if, like so? JN if (defined $entry-{$check} defined $query-{$check}) { JN ... JN } else { JN log_debug(...); JN } I prefer the explicit version because we can issue a more precise log_debug message. Ted -- 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] git-credential-netrc: fix uninitialized warning
On 10/08/2013 09:55 PM, Ted Zlatanov wrote: JN Sign-off? I didn't realize it was a requirement, must I? Yes, this is a requirement. See Documentation/SubmittingPatches to read what signing off actually means here. Stefan signature.asc Description: OpenPGP digital signature
[PATCHv2] git-credential-netrc: fix uninitialized warning
Simple patch to avoid unitialized warning and log what we'll do. Signed-off-by: Ted Zlatanov t...@lifelogs.com --- contrib/credential/netrc/git-credential-netrc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc index 6c51c43..1571a7b 100755 --- a/contrib/credential/netrc/git-credential-netrc +++ b/contrib/credential/netrc/git-credential-netrc @@ -369,7 +369,9 @@ sub find_netrc_entry { { my $entry_text = join ', ', map { $_=$entry-{$_} } keys %$entry; foreach my $check (sort keys %$query) { - if (defined $query-{$check}) { + if (!defined $entry-{$check}) { + log_debug(OK: entry has no $check token, so any value satisfies check $check); + } elsif (defined $query-{$check}) { log_debug(compare %s [%s] to [%s] (entry: %s), $check, $entry-{$check}, -- 1.8.1.5 -- 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] git-credential-netrc: fix uninitialized warning
Ted Zlatanov wrote: On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder jrnie...@gmail.com wrote: JN Ted Zlatanov wrote: Simple patch to avoid unitialized warning and log what we'll do. JN Sign-off? I didn't realize it was a requirement, must I? See Documentation/SubmittingPatches, section '(5) Sign your work' for what this means. If you just forgot to sign off, that's fine and I can forge it or go without. If you are unable to sign off because you don't have the right to submit the change under an open source license, I'd be a bit worried going forward. [...] JN Or more simply, would it make sense to wrap both 'defined' checks into JN a single if, like so? JN if (defined $entry-{$check} defined $query-{$check}) { JN ... JN } else { JN log_debug(...); JN } I prefer the explicit version because we can issue a more precise log_debug message. That's fine with me. After this patch, the code looks like if (!defined $entry-{$check}) { log_debug(...); } elsif (defined $query-{$check}) { ... } else { log_debug(...); } As a small nit, wouldn't it be more readable with the two !defined() cases together? if (!defined $entry-{$check}) { ... } elsif (!defined $query-{$check}) { ... } else { ... } Thanks again. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-credential-netrc: fix uninitialized warning
On Tue, 08 Oct 2013 21:58:41 +0200 Stefan Beller stefanbel...@googlemail.com wrote: SB On 10/08/2013 09:55 PM, Ted Zlatanov wrote: JN Sign-off? I didn't realize it was a requirement, must I? SB Yes, this is a requirement. See Documentation/SubmittingPatches SB to read what signing off actually means here. OK, I didn't realize it was. Thanks for explaining. Ted -- 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] git-credential-netrc: fix uninitialized warning
On Tue, 8 Oct 2013 13:02:35 -0700 Jonathan Nieder jrnie...@gmail.com wrote: JN Ted Zlatanov wrote: On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder jrnie...@gmail.com wrote: JN Ted Zlatanov wrote: Simple patch to avoid unitialized warning and log what we'll do. JN Sign-off? I didn't realize it was a requirement, must I? JN See Documentation/SubmittingPatches, section '(5) Sign your work' JN for what this means. JN If you just forgot to sign off, that's fine and I can forge it or go JN without. If you are unable to sign off because you don't have the JN right to submit the change under an open source license, I'd be a bit JN worried going forward. Right, I got it. Sorry, I didn't know that applied to trivial patches. JN After this patch, the code looks like JN if (!defined $entry-{$check}) { JN log_debug(...); JN } elsif (defined $query-{$check}) { JN ... JN } else { JN log_debug(...); JN } JN As a small nit, wouldn't it be more readable with the two !defined() JN cases together? JN if (!defined $entry-{$check}) { JN ... JN } elsif (!defined $query-{$check}) { JN ... JN } else { JN ... JN } Because of way the next ENTRY line comes out, I like my way slightly better, but honestly it's fine either way :) If you like, go ahead and commit the rewrite the way it works for you, I have no objections at all. Ted -- 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: gitk next/prev buttons
On 13-10-08 03:36 PM, Jonathan Nieder wrote: In a branchy history, it is possible for the next matching commit to actually be newer. Chronologically, yes. Gitk will often display a history like this (here the numbers represent commit dates, so 1 is the oldest commit, and I've rotated this 90 degrees clockwise from how gitk would display it): --3--4---5--- / 1--2 If commits 2 and 4 match, and the user is looking at commit 2, then the next matching commit from 2 is 4, which is chronologically newer. However, I still find it more intuitive to think of commit 4 as older than commit 2, at least when using gitk. This is because in gitk the commits are generally older as you go down the list. (When it comes to branches, chronology hardly matters anyway. In fact, gitk could ensure that all commits are displayed in strictly chronological order regardless of branches, and such a display would be just as correct as what it currently shows although it'd be less compact.) A gitk user comes to expect older commits to be lower down in the display. It's certainly not a hard-and-fast rule, but it's a general paradigm that works. I think the intent of the buttons is find the next result, looking down or up in the list of commits in the upper pane. Is there some other wording that would convey this better? The problem is, at least for me, that when I'm using gitk to browse the history of changes to a file, I'll often want to see things that happened before or after a certain point. Once I start thinking like that, gitk's concepts of next and prev mix me up, because I want to see the next thing that happened to the file but the next button doesn't take me there. The chronological (dis)ordering that branches introduce doesn't trip me up as much as the next and prev buttons. M. -- 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] Update documentation for http.continue option
Explain the reason for and behavior of the new http.continue option, and that it is enabled by default. Furthermore, explain that it is required for large GSS-Negotiate requests but incompatible with some proxies. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- Documentation/config.txt | 9 + 1 file changed, 9 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index fca7749..5fff2b2 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1516,6 +1516,15 @@ http.postBuffer:: massive pack file locally. Default is 1 MiB, which is sufficient for most requests. +http.continue:: + Ensure that authentication succeeds before sending the pack data when + POSTing data using the smart HTTP transport. This is done by + requesting a 100 Continue response. For requests larger than + 'http.postBuffer', this is required when using GSS-Negotiate + (Kerberos) authentication over HTTP. However, some proxies do not + handle the protocol exchange gracefully; for them, this option must be + disabled. Defaults to enabled. + http.lowSpeedLimit, http.lowSpeedTime:: If the HTTP transfer speed is less than 'http.lowSpeedLimit' for longer than 'http.lowSpeedTime' seconds, the transfer is aborted. -- 1.8.4.rc3 -- 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] http: add option to enable 100 Continue responses
When using GSS-Negotiate authentication with libcurl, the authentication provided will change every time, and so the probe that git uses to determine if authentication is needed is not sufficient to guarantee that data can be sent. If the data fits entirely in http.postBuffer bytes, the data can be rewound and resent if authentication fails; otherwise, a 100 Continue must be requested in this case. By default, curl will send an Expect: 100-continue if a certain amount of data is to be uploaded, but when using chunked data this is never triggered. Add an option http.continue, which defaults to enabled, to control whether this header is sent. The addition of an option is necessary because some proxies break badly if sent this header. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net --- http.c| 6 ++ http.h| 1 + remote-curl.c | 7 ++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/http.c b/http.c index f3e1439..941c941 100644 --- a/http.c +++ b/http.c @@ -11,6 +11,7 @@ int active_requests; int http_is_verbose; size_t http_post_buffer = 16 * LARGE_PACKET_MAX; +int http_use_100_continue = 1; #if LIBCURL_VERSION_NUM = 0x070a06 #define LIBCURL_CAN_HANDLE_AUTH_ANY @@ -213,6 +214,11 @@ static int http_options(const char *var, const char *value, void *cb) return 0; } + if (!strcmp(http.continue, var)) { + http_use_100_continue = git_config_bool(var, value); + return 0; + } + if (!strcmp(http.useragent, var)) return git_config_string(user_agent, var, value); diff --git a/http.h b/http.h index d77c1b5..e72786e 100644 --- a/http.h +++ b/http.h @@ -102,6 +102,7 @@ extern void http_cleanup(void); extern int active_requests; extern int http_is_verbose; extern size_t http_post_buffer; +extern int http_use_100_continue; extern char curl_errorstr[CURL_ERROR_SIZE]; diff --git a/remote-curl.c b/remote-curl.c index b5ebe01..3b5e160 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -470,7 +470,12 @@ static int post_rpc(struct rpc_state *rpc) headers = curl_slist_append(headers, rpc-hdr_content_type); headers = curl_slist_append(headers, rpc-hdr_accept); - headers = curl_slist_append(headers, Expect:); + + /* Force it either on or off, since curl will try to decide based on how +* much data is to be uploaded and we want consistency. +*/ + headers = curl_slist_append(headers, http_use_100_continue ? + Expect: 100-continue : Expect:); retry: slot = get_active_slot(); -- 1.8.4.rc3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] HTTP GSS-Negotiate improvements
This patch set adds an option, http.continue, to enable requests for 100 Continue responses when pushing over HTTP. This is needed for large pushes using GSS-Negotiate authentication. I decided to be optimistic and default the option to on, which means that 100 Continue responses will be requested. My rationale for this is that modern versions of Squid, Apache, and nginx seem to do the right thing, as well as most non-proxy HTTP 1.1 implementations, and that we should optimize for functional, up-to-date software. HTTP 1.1 is 16 years old, after all. brian m. carlson (2): http: add option to enable 100 Continue responses Update documentation for http.continue option Documentation/config.txt | 9 + http.c | 6 ++ http.h | 1 + remote-curl.c| 7 ++- 4 files changed, 22 insertions(+), 1 deletion(-) -- 1.8.4.rc3 -- 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] gitk: Rename next and prev buttons to older and newer.
Users often find that next and prev do the opposite of what they expect. For example, next moves to the next match down the list, but that is almost always backwards in time. Renaming next to older and prev to newer makes it clear where the buttons will take the user. --- (Apologies to Lucas and Paul -- my previous patch email didn't include the git list.) gitk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gitk b/gitk index 5cd00d8..e610010 100755 --- a/gitk +++ b/gitk @@ -2244,8 +2244,8 @@ proc makewindow {} { # build up the bottom bar of upper window ${NS}::label .tf.lbar.flabel -text [mc Find] -${NS}::button .tf.lbar.fnext -text [mc next] -command {dofind 1 1} -${NS}::button .tf.lbar.fprev -text [mc prev] -command {dofind -1 1} +${NS}::button .tf.lbar.fnext -text [mc older] -command {dofind 1 1} +${NS}::button .tf.lbar.fprev -text [mc newer] -command {dofind -1 1} ${NS}::label .tf.lbar.flab2 -text [mc commit] pack .tf.lbar.flabel .tf.lbar.fnext .tf.lbar.fprev .tf.lbar.flab2 \ -side left -fill y -- 1.8.4.dirty -- 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-pack: don't send a thin pack when the server doesn't support it
Duy Nguyen wrote: Or maybe it's not that late. How about you go with your patch and add thin-pack capability to receive-pack too? When new git push is used against old server, thin pack is disabled. But that's not a big deal (I hope). Could we have separate patches to introduce the server-side capability and then to request it in the client? That way, people with old servers can apply the patch introducing the capability if they want. The new meaning of the thin-pack capability should also be documented in Documentation/technical/protocol-capabilities.txt. Done that way and with enough time between the server advertising the capability and the client looking for it, it seems like a good idea. My two cents, 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: [RFC/PATCHv2 1/3] Documentation/git-svn: Promote the use of --prefix in docs + examples
On Sat, Oct 5, 2013 at 7:30 PM, Johan Herland jo...@herland.net wrote: diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt index 4dd3bcb..da00671 100644 --- a/Documentation/git-svn.txt +++ b/Documentation/git-svn.txt @@ -973,6 +979,15 @@ without giving any repository layout options. If the full history with branches and tags is required, the options '--trunk' / '--branches' / '--tags' must be used. +When using the options for describing the repository layout (--trunk, +--tags, --branches, --stdlayout), please also specify the --prefix +option (e.g. '--prefix=origin/') to cause your SVN-tracking refs to be +placed at refs/remotes/origin/* rather than the default refs/remotes/*. +The former is more compatible with the layout of Git's regular +remote-tracking refs (refs/remotes/$remote/*), and may potentially +prevent similarly named SVN branches and Git remotes from clobbering +eachother. s/eachother/each other/ When using multiple --branches or --tags, 'git svn' does not automatically handle name collisions (for example, if two branches from different paths have the same name, or if a branch and a tag have the same name). In these cases, @@ -1035,8 +1050,8 @@ comma-separated list of names within braces. For example: [svn-remote huge-project] url = http://server.org/svn fetch = trunk/src:refs/remotes/trunk - branches = branches/{red,green}/src:refs/remotes/branches/* - tags = tags/{1.0,2.0}/src:refs/remotes/tags/* + branches = branches/{red,green}/src:refs/remotes/project-a/branches/* + tags = tags/{1.0,2.0}/src:refs/remotes/project-a/tags/* Multiple fetch, branches, and tags keys are supported: -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCHv2 2/3] git-svn: Warn about changing default for --prefix in Git v2.0
On Sat, Oct 5, 2013 at 7:30 PM, Johan Herland jo...@herland.net wrote: diff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh index b7ef9e2..1c8d049 100755 --- a/t/t9117-git-svn-init-clone.sh +++ b/t/t9117-git-svn-init-clone.sh @@ -52,4 +52,71 @@ test_expect_success 'clone to target directory with --stdlayout' ' rm -rf target ' +test_expect_success 'clone without -s/-T/-b/-t does not warn' ' + test ! -d trunk + git svn clone $svnrepo/project/trunk 2warning + test_must_fail grep -q prefix warning + rm -rf trunk + rm -f warning + ' + +test_svn_configured_prefix () { + prefix=$1 Did you want to maintain the -chain here? + cat expect EOF And here? +project/trunk:refs/remotes/${prefix}trunk +project/branches/*:refs/remotes/${prefix}* +project/tags/*:refs/remotes/${prefix}tags/* +EOF + test ! -f actual + git --git-dir=project/.git config svn-remote.svn.fetch actual + git --git-dir=project/.git config svn-remote.svn.branches actual + git --git-dir=project/.git config svn-remote.svn.tags actual + test_cmp expect actual + rm -f expect actual +} -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-subtree: Avoid using echo -n even indirectly
Since say uses echo, this uses echo -n, which is not portable - see 19c3c5fdcb35b66b792534c5dc4e8d87a3952d2a. Without this commit, the output looks like: ... -n 1891/1936 (1883) -n 1892/1936 (1884) -n 1893/1936 (1885) ... Signed-off-by: Paolo G. Giarrusso p.giarru...@gmail.com --- Please CC me on replies, as I am not subscribed to this mailing list. I am tracking this submission via https://github.com/git/git/pull/61, which I'll duly close myself when the discussion is resolved. contrib/subtree/git-subtree.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index 7d7af03..ebfb78f 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -592,7 +592,9 @@ cmd_split() eval $grl | while read rev parents; do revcount=$(($revcount + 1)) - say -n $revcount/$revmax ($createcount) + if [ -z $quiet ]; then + printf %s $revcount/$revmax ($createcount) 2 + fi debug Processing commit: $rev exists=$(cache_get $rev) if [ -n $exists ]; then -- 1.8.4 -- 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] mergetools/diffmerge: support DiffMerge as a git mergetool
Thanks for the review David, much appreciated. I think this line was already too long in its current form. Would you mind splitting up this long line? I've updated the patch and had a look at how to avoid repeating the list of available merge/difftools. ... follow it up with a change that generalizes the list of tools thing so that it can be reused here, possibly. The show_tool_help() function, as used by git difftool --tool-help and git mergetool --tool-help, might be a good place to look for inspiration. We were able to eliminate duplication in the docs (see the handling for $(mergetools_txt) in Documentation/Makefile) so it'd be nice if we could do the same for git-completion.bash, somehow. I can think of a number of approaches and I would like to get some feedback. Firstly I think a similar solution to how the duplication is avoided in the documentation can't be easily applied to the completion script. Looking at the script itself (and/or usage docs like http://git-scm.com/book/en/Git-Basics-Tips-and-Tricks) the recommended way of using it is by copying the script as-is. That means there won't be a build step we could rely on unless I've overlooked something? That leaves a different approach (run- vs. build time) where I can think of two possible solutions. The first would be similar to what is being done at the moment by looking at the MERGE_TOOLS_DIR and in addition considering any custom merge tools configured. I'm working with the premise that it is a reasonable assumption that users of the git completion script have a git installation available even though they may have gotten the script by other means. For users to still be able to install the script by simply copying it to any location on the filesystem the list generation function(s) would either have to be sourced from the git installation or duplicated. I suppose the former would need to take into account that the completion script doesn't necessarily matches the installed version of git with some potential brittleness around relying on external files and directories. The latter doesn't buy us anything as it duplicates even more code than the current list of available mergetools. The second approach would be to do something similar to resolving the merge strategies (in __git_list_merge_strategies) by parsing the output of the `git merge tool --tools-help` option with a very similar disadvantage that it relies on the textual output of the help command and doesn't work outside of a git repository. I'm currently leaning towards the last approach as it seems less reliant on implementation details but it doesn't look ideal either and I may be missing another approach that would be better suited. It might be worth leaving the git-completion.bash bits alone in this first patch and follow it up with a change that generalizes the list of tools thing so that it can be reused here, possibly. To decouple this and adding the diffmerge merge tool option, I'd rather keep the git-completion change part of the patch. That way the patch is self contained and covers the change including the completion using the current approach and doesn't rely on the duplication change. Any concerns around that, otherwise I'll resend the patch with only the long line fixed? Cheers, Stefan On 6 October 2013 14:21, David Aguilar dav...@gmail.com wrote: On Sat, Oct 5, 2013 at 1:29 AM, Stefan Saasen ssaa...@atlassian.com wrote: DiffMerge is a non-free (but gratis) tool that supports OS X, Windows and Linux. See http://www.sourcegear.com/diffmerge/ DiffMerge includes a script `/usr/bin/diffmerge` that can be used to launch the graphical compare tool. This change adds mergetool support for DiffMerge and adds 'diffmerge' as an option to the mergetool help. Signed-off-by: Stefan Saasen ssaa...@atlassian.com --- contrib/completion/git-completion.bash | 2 +- git-mergetool--lib.sh | 2 +- mergetools/diffmerge | 15 +++ 3 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 mergetools/diffmerge diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index e1b7313..07b0ba5 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1188,7 +1188,7 @@ _git_diff () __git_complete_revlist_file } -__git_mergetools_common=diffuse ecmerge emerge kdiff3 meld opendiff +__git_mergetools_common=diffuse diffmerge ecmerge emerge kdiff3 meld opendiff tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 codecompare It's a little unfortunate that we have to keep repeating ourselves here. mergetool--lib has a list_merge_tool_candidate() that populates $tools and help us avoid having to maintain these lists in separate files. It might be worth leaving the git-completion.bash bits alone in this first patch and follow