Re: [PATCH] http: use curl's tcp keepalive if available
Jeff King p...@peff.net wrote: I am more concerned with Windows, which may not compile your original patch at all. The code I introduced in e47a8583a20256851e7fc882233e3bd5bf33dc6e (enable SO_KEEPALIVE for connected TCP sockets Dec 2011) didn't trigger the addition of any new #ifdef guards. I think we're fine (but I'll let others confirm it). -- 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 v3] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.
Hello Duy
Thank you very much your suggestion.
As you suggested, I tried to reuse intermediate result of pprint_rename(),
instead of
parsing the output again.
I just posted the new patch as PATCH v4
Thanks !
---
Tsuneo Yoshioka (吉岡 恒夫)
yoshiokatsu...@gmail.com
On Oct 14, 2013, at 10:04 PM, Duy Nguyen pclo...@gmail.com wrote:
On Sun, Oct 13, 2013 at 3:48 AM, Yoshioka Tsuneo
yoshiokatsu...@gmail.com wrote:
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar, but if destination filename is long the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
This commit makes it visible like ...foo = ...bar.
Signed-off-by: Tsuneo Yoshioka yoshiokatsu...@gmail.com
---
diff.c | 58 +++---
1 file changed, 51 insertions(+), 7 deletions(-)
diff --git a/diff.c b/diff.c
index a04a34d..3aeaf3e 100644
--- a/diff.c
+++ b/diff.c
@@ -1643,13 +1643,57 @@ static void show_stats(struct diffstat_t *data,
struct diff_options *options)
len = name_width;
name_len = strlen(name);
if (name_width name_len) {
- char *slash;
- prefix = ...;
- len -= 3;
- name += name_len - len;
- slash = strchr(name, '/');
- if (slash)
- name = slash;
+ char *arrow = strstr(name, = );
+ if (arrow) {
This looks iffy. What if = is part of the path name?
file-is_renamed would be a more reliable sign. In that case I think
you just need an ellipsis version of pprint_rename() (i.e. drop the
result of previous pprint_rename() on the floor and create a new
string with ... and = in your pprint_ellipsis_rename or
something)
+ int prefix_len = (name_width - 4) / 2;
+ int f_omit;
+ int f_brace = 0;
+ char *pre_arrow = alloca(name_width + 10);
+ char *post_arrow = arrow + 4;
+ char *prefix_buf = alloca(name_width + 10);
+ char *pre_arrow_slash = NULL;
+
+ if (arrow - name prefix_len) {
+ prefix_len = (int)(arrow - name);
+ f_omit = 0;
+ } else {
+ prefix_len -= 3;
+ f_omit = 1;
+ if (name[0] == '{') {
+ prefix_len -= 1;
+ f_brace = 1;
+ }
+ }
+ prefix_len = ((prefix_len = 0) ? prefix_len
: 0);
+ strncpy(pre_arrow, arrow - prefix_len,
prefix_len);
+ pre_arrow[prefix_len] = '\0';
+ pre_arrow_slash = strchr(pre_arrow, '/');
+ if (f_omit pre_arrow_slash)
+ pre_arrow = pre_arrow_slash;
+ sprintf(prefix_buf, %s%s%s = , (f_brace ?
{ : ), (f_omit ? ... : ), pre_arrow);
+ prefix = prefix_buf;
+
+ if (strlen(post_arrow) name_width -
strlen(prefix)) {
+ char *post_arrow_slash = NULL;
+
+ post_arrow += strlen(post_arrow) -
(name_width - strlen(prefix) - 3);
+ strcat(prefix_buf, ...);
+ post_arrow_slash =
strchr(post_arrow, '/');
+ if (post_arrow_slash)
+ post_arrow =
post_arrow_slash;
+ name = post_arrow;
+ name_len = (int) (name_width -
strlen(prefix));
+ }
+ len -= strlen(prefix);
+ } else {
+ char *slash = NULL;
+ prefix = ...;
+ len -= 3;
+ name += name_len - len;
+ slash = strchr(name, '/');
+ if (slash)
+ name = slash;
+ }
}
if (file-is_binary) {
--
1.8.4.475.g867697c
--
To unsubscribe from this list:
Re: [PATCH v4] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.
On Tue, Oct 15, 2013 at 4:45 AM, Yoshioka Tsuneo
yoshiokatsu...@gmail.com wrote:
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar. But if destination filename is long, the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
Make sure there is always an arrow, like ...foo = ...bar.
The output can contains curly braces('{','}') for grouping.
So, in general, the outpu format is pfx{mid_a = mid_b}sfx
To keep arrow(=), try to omit pfx as long as possible at first
because later part or changing part will be the more important part.
If it is not enough, shorten mid_a, mid_b, and sfx trying to
have the maximum length the same because those will be equaly important.
This has similar style issues as v1.
-static char *pprint_rename(const char *a, const char *b)
+static void pprint_rename_find_common_prefix_suffix(const char *a, const
char *b, struct strbuf *pfx, struct strbuf *a_mid, struct strbuf *b_mid,
struct strbuf *sfx)
{
const char *old = a;
const char *new = b;
- struct strbuf name = STRBUF_INIT;
int pfx_length, sfx_length;
int pfx_adjust_for_slash;
int len_a = strlen(a);
@@ -1272,10 +1271,9 @@ static char *pprint_rename(const char *a, const char
*b)
int qlen_b = quote_c_style(b, NULL, NULL, 0);
if (qlen_a || qlen_b) {
- quote_c_style(a, name, NULL, 0);
- strbuf_addstr(name, = );
- quote_c_style(b, name, NULL, 0);
- return strbuf_detach(name, NULL);
+ quote_c_style(a, a_mid, NULL, 0);
+ quote_c_style(b, b_mid, NULL, 0);
+ return;
}
/* Find common prefix */
@@ -1321,18 +1319,149 @@ static char *pprint_rename(const char *a, const char
*b)
a_midlen = 0;
if (b_midlen 0)
b_midlen = 0;
+
+ strbuf_add(pfx, a, pfx_length);
+ strbuf_add(a_mid, a + pfx_length, a_midlen);
+ strbuf_add(b_mid, b + pfx_length, b_midlen);
+ strbuf_add(sfx, a + len_a - sfx_length, sfx_length);
+}
- strbuf_grow(name, pfx_length + a_midlen + b_midlen + sfx_length + 7);
- if (pfx_length + sfx_length) {
- strbuf_add(name, a, pfx_length);
+/*
+ * Omit each parts to fix in name_width.
+ * Formatted string is pfx{a_mid = b_mid}sfx.
+ * At first, omit pfx as long as possible.
+ * If it is not enough, omit a_mid, b_mid, sfx by tring to set the
length of
+ * those 3 parts(including ...) to the same.
+ * Ex:
+ * foofoofoo = barbarbar
+ * will be like
+ * ...foo = ...bar.
+ * long_parent{foofoofoo = barbarbar}longfilename
+ * will be like
+ * ...parent{...foofoo = ...barbar}...lename
+ */
+static void pprint_rename_omit(struct strbuf *pfx, struct strbuf *a_mid,
struct strbuf *b_mid, struct strbuf *sfx, int name_width)
Seems like this line needs to be broken.
+{
+
+#define ARROW =
+#define ELLIPSIS ...
+#define swap(a,b) myswap((a),(b),sizeof(a))
I'm not entirely sure, but I think this should be:
#define swap(a, b) myswap((a), (b), sizeof(a))
+
+#define myswap(a, b, size) do {\
+unsigned char mytmp[size]; \
+memcpy(mytmp, a, size); \
+memcpy(a, b, size); \
+memcpy(b, mytmp, size); \
+} while (0)
+
+ int use_curly_braces = (pfx-len 0) || (sfx-len 0);
+ size_t name_len;
+ size_t len;
+ size_t part_lengths[4];
+ size_t max_part_len = 0;
+ size_t remainder_part_len = 0;
+ int i, j;
+
+ name_len = pfx-len + a_mid-len + b_mid-len + sfx-len +
strlen(ARROW) + (use_curly_braces?2:0);
+
+ if (name_len = name_width){
if () {
+ /* Everthing fits in name_width */
+ return;
+ }
+
+ if(use_curly_braces){
Ditto.
+ if(strlen(ELLIPSIS) + (name_len - pfx-len) = name_width){
Ditto.
+ /*
+Just omitting left of '{' is enough
+Ex: ...aaa{foofoofoo = bar}file
+*/
+ strbuf_splice(pfx, name_len - pfx-len, name_width -
(name_len - pfx-len), ELLIPSIS, strlen(ELLIPSIS));
+ return;
+ }else{
} else {
+ if (pfx-len strlen(ELLIPSIS)) {
+ /*
+Just omitting left of '{' is not enough
+name will be ...{SOMETHING}SOMETHING
+*/
+ strbuf_reset(pfx);
+ strbuf_addstr(pfx, ELLIPSIS);
+ }
+ }
+ }
+
+ /* available length for a_mid, b_mid and sfx */
+ len = name_width - strlen(ARROW) -
[PATCH v5] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar. But if destination filename is long, the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
Make sure there is always an arrow, like ...foo = ...bar.
The output can contains curly braces('{','}') for grouping.
So, in general, the outpu format is pfx{mid_a = mid_b}sfx
To keep arrow(=), try to omit pfx as long as possible at first
because later part or changing part will be the more important part.
If it is not enough, shorten mid_a, mid_b, and sfx trying to
have the maximum length the same because those will be equaly important.
Signed-off-by: Tsuneo Yoshioka yoshiokatsu...@gmail.com
Test-added-by: Thomas Rast tr...@inf.ethz.ch
---
diff.c | 187 +++--
t/t4001-diff-rename.sh | 12
2 files changed, 177 insertions(+), 22 deletions(-)
diff --git a/diff.c b/diff.c
index a04a34d..cf50807 100644
--- a/diff.c
+++ b/diff.c
@@ -1258,11 +1258,12 @@ static void fn_out_consume(void *priv, char *line,
unsigned long len)
}
}
-static char *pprint_rename(const char *a, const char *b)
+static void pprint_rename_find_common_prefix_suffix(const char *a, const char
*b
+
, struct strbuf *pfx, struct strbuf *a_mid
+
, struct strbuf *b_mid, struct strbuf *sfx)
{
const char *old = a;
const char *new = b;
- struct strbuf name = STRBUF_INIT;
int pfx_length, sfx_length;
int pfx_adjust_for_slash;
int len_a = strlen(a);
@@ -1272,10 +1273,9 @@ static char *pprint_rename(const char *a, const char *b)
int qlen_b = quote_c_style(b, NULL, NULL, 0);
if (qlen_a || qlen_b) {
- quote_c_style(a, name, NULL, 0);
- strbuf_addstr(name, = );
- quote_c_style(b, name, NULL, 0);
- return strbuf_detach(name, NULL);
+ quote_c_style(a, a_mid, NULL, 0);
+ quote_c_style(b, b_mid, NULL, 0);
+ return;
}
/* Find common prefix */
@@ -1321,18 +1321,151 @@ static char *pprint_rename(const char *a, const char
*b)
a_midlen = 0;
if (b_midlen 0)
b_midlen = 0;
+
+ strbuf_add(pfx, a, pfx_length);
+ strbuf_add(a_mid, a + pfx_length, a_midlen);
+ strbuf_add(b_mid, b + pfx_length, b_midlen);
+ strbuf_add(sfx, a + len_a - sfx_length, sfx_length);
+}
+
+/*
+ * Omit each parts to fix in name_width.
+ * Formatted string is pfx{a_mid = b_mid}sfx.
+ * At first, omit pfx as long as possible.
+ * If it is not enough, omit a_mid, b_mid, sfx by tring to set the
length of
+ * those 3 parts(including ...) to the same.
+ * Ex:
+ * foofoofoo = barbarbar
+ * will be like
+ * ...foo = ...bar.
+ * long_parent{foofoofoo = barbarbar}longfilename
+ * will be like
+ * ...parent{...foofoo = ...barbar}...lename
+ */
+static void pprint_rename_omit(struct strbuf *pfx, struct strbuf *a_mid,
struct strbuf *b_mid
+ , struct strbuf
*sfx, int name_width)
+{
+
+#define ARROW =
+#define ELLIPSIS ...
+#define swap(a, b) myswap((a), (b), sizeof(a))
+
+#define myswap(a, b, size) do {\
+unsigned char mytmp[size]; \
+memcpy(mytmp, a, size); \
+memcpy(a, b, size); \
+memcpy(b, mytmp, size); \
+} while (0)
+
+ int use_curly_braces = (pfx-len 0) || (sfx-len 0);
+ size_t name_len;
+ size_t len;
+ size_t part_lengths[4];
+ size_t max_part_len = 0;
+ size_t remainder_part_len = 0;
+ int i, j;
+
+ name_len = pfx-len + a_mid-len + b_mid-len + sfx-len + strlen(ARROW)
+ + (use_curly_braces ? 2 : 0);
+
+ if (name_len = name_width) {
+ /* Everthing fits in name_width */
+ return;
+ }
+
+ if (use_curly_braces) {
+ if (strlen(ELLIPSIS) + (name_len - pfx-len) = name_width) {
+ /*
+Just omitting left of '{' is enough
+Ex: ...aaa{foofoofoo = bar}file
+*/
+ strbuf_splice(pfx, name_len - pfx-len, name_width -
(name_len - pfx-len), ELLIPSIS, strlen(ELLIPSIS));
+ return;
+ } else {
+ if (pfx-len strlen(ELLIPSIS)) {
+ /*
+Just omitting left of '{' is not enough
+name will be ...{SOMETHING}SOMETHING
+*/
+ strbuf_reset(pfx);
+
Re: [PATCH v4] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.
Hello Felipe
Thank you for pointing out the style issue again.
I just fixed it and posted as [PATCH v5].
Thanks!
---
Tsuneo Yoshioka (吉岡 恒夫)
yoshiokatsu...@gmail.com
On Oct 15, 2013, at 1:07 PM, Felipe Contreras felipe.contre...@gmail.com
wrote:
On Tue, Oct 15, 2013 at 4:45 AM, Yoshioka Tsuneo
yoshiokatsu...@gmail.com wrote:
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar. But if destination filename is long, the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
Make sure there is always an arrow, like ...foo = ...bar.
The output can contains curly braces('{','}') for grouping.
So, in general, the outpu format is pfx{mid_a = mid_b}sfx
To keep arrow(=), try to omit pfx as long as possible at first
because later part or changing part will be the more important part.
If it is not enough, shorten mid_a, mid_b, and sfx trying to
have the maximum length the same because those will be equaly important.
This has similar style issues as v1.
-static char *pprint_rename(const char *a, const char *b)
+static void pprint_rename_find_common_prefix_suffix(const char *a, const
char *b, struct strbuf *pfx, struct strbuf *a_mid, struct strbuf *b_mid,
struct strbuf *sfx)
{
const char *old = a;
const char *new = b;
- struct strbuf name = STRBUF_INIT;
int pfx_length, sfx_length;
int pfx_adjust_for_slash;
int len_a = strlen(a);
@@ -1272,10 +1271,9 @@ static char *pprint_rename(const char *a, const char
*b)
int qlen_b = quote_c_style(b, NULL, NULL, 0);
if (qlen_a || qlen_b) {
- quote_c_style(a, name, NULL, 0);
- strbuf_addstr(name, = );
- quote_c_style(b, name, NULL, 0);
- return strbuf_detach(name, NULL);
+ quote_c_style(a, a_mid, NULL, 0);
+ quote_c_style(b, b_mid, NULL, 0);
+ return;
}
/* Find common prefix */
@@ -1321,18 +1319,149 @@ static char *pprint_rename(const char *a, const
char *b)
a_midlen = 0;
if (b_midlen 0)
b_midlen = 0;
+
+ strbuf_add(pfx, a, pfx_length);
+ strbuf_add(a_mid, a + pfx_length, a_midlen);
+ strbuf_add(b_mid, b + pfx_length, b_midlen);
+ strbuf_add(sfx, a + len_a - sfx_length, sfx_length);
+}
- strbuf_grow(name, pfx_length + a_midlen + b_midlen + sfx_length +
7);
- if (pfx_length + sfx_length) {
- strbuf_add(name, a, pfx_length);
+/*
+ * Omit each parts to fix in name_width.
+ * Formatted string is pfx{a_mid = b_mid}sfx.
+ * At first, omit pfx as long as possible.
+ * If it is not enough, omit a_mid, b_mid, sfx by tring to set the
length of
+ * those 3 parts(including ...) to the same.
+ * Ex:
+ * foofoofoo = barbarbar
+ * will be like
+ * ...foo = ...bar.
+ * long_parent{foofoofoo = barbarbar}longfilename
+ * will be like
+ * ...parent{...foofoo = ...barbar}...lename
+ */
+static void pprint_rename_omit(struct strbuf *pfx, struct strbuf *a_mid,
struct strbuf *b_mid, struct strbuf *sfx, int name_width)
Seems like this line needs to be broken.
+{
+
+#define ARROW =
+#define ELLIPSIS ...
+#define swap(a,b) myswap((a),(b),sizeof(a))
I'm not entirely sure, but I think this should be:
#define swap(a, b) myswap((a), (b), sizeof(a))
+
+#define myswap(a, b, size) do {\
+unsigned char mytmp[size]; \
+memcpy(mytmp, a, size); \
+memcpy(a, b, size); \
+memcpy(b, mytmp, size); \
+} while (0)
+
+ int use_curly_braces = (pfx-len 0) || (sfx-len 0);
+ size_t name_len;
+ size_t len;
+ size_t part_lengths[4];
+ size_t max_part_len = 0;
+ size_t remainder_part_len = 0;
+ int i, j;
+
+ name_len = pfx-len + a_mid-len + b_mid-len + sfx-len +
strlen(ARROW) + (use_curly_braces?2:0);
+
+ if (name_len = name_width){
if () {
+ /* Everthing fits in name_width */
+ return;
+ }
+
+ if(use_curly_braces){
Ditto.
+ if(strlen(ELLIPSIS) + (name_len - pfx-len) = name_width){
Ditto.
+ /*
+Just omitting left of '{' is enough
+Ex: ...aaa{foofoofoo = bar}file
+*/
+ strbuf_splice(pfx, name_len - pfx-len, name_width -
(name_len - pfx-len), ELLIPSIS, strlen(ELLIPSIS));
+ return;
+ }else{
} else {
+ if (pfx-len strlen(ELLIPSIS)) {
+ /*
+Just omitting left of '{' is not enough
+name will be ...{SOMETHING}SOMETHING
+*/
+
Re
Greetings, How are you and the family? My contacting you again is based on neglecting my previous email of investment establishment in your country. Be aware that I am in a desire of any investments establishment that will guaranty a safe and secured profitable returns in terms of energy renewals, transportation, agriculture, aviation, oil and gas, real estates, hotel resorts, casinos etc. Or any other business or investment interest of your choice that you believe will be encouraging enough for us to established in your home town and I will be very ready to cooperate and partner with you. Please contact me through this e-mail address (dr.dar@gmail.com) to enable me give you more details about the investment establishment plans and how much is the total amount that I am intending to invest in your country. Say me well to the family as I wait to read from you soon.for more details. Regards. Dr.Dar Rot -- 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] rev-parse doc: clarify use of optional / required arguments
On Mon, 14 Oct 2013, brian m. carlson wrote: On Mon, Oct 14, 2013 at 05:25:29PM +0200, Nicolas Vigier wrote: The reason that I looked at this documentation in the first place was that I was looking at adding an option '-S[keyid], --gpg-sign[=keyid]' to git-rebase, similar to the option in git-commit, so that rebased commits can be signed. In git-commit this option takes an optional argument, so I think it would make sense to make it optional in git-rebase too. It's funny you say that, because I literally started on that yesterday. I have cherry-pick and revert working, but I haven't gotten to anything else yet. Feel free to work on it if you're interested, as I probably won't get around to finishing it for some time. I have a patch for git-am working, but with a small problem : it has to assume that the optional argument to -S does not start with a dash. This is because git-rev-parse --parseopt does not allow us to see the difference between -S-q and -S -q, we don't know if -q is the next option or the argument to -S. Maybe that's the reason why the use of optional argument options with git rev-parse --parseopt is discouraged ? I'm going to send a patch proposal so that rev-parse --parseopt gives an empty argument for an unset optional argument. pgpfkhsC1L4vp.pgp Description: PGP signature
[PATCH] rev-parse --parseopt: fix handling of optional arguments
git rev-parse --parseopt does not allow us to see the difference
between an option with an optional argument starting with a dash, and an
option with an unset optional argument followed by an other option.
If I use this script :
$ cat /tmp/opt.sh
#!/bin/sh
OPTIONS_SPEC=\
git [options]
--
q,quiet be quiet
S,gpg-sign? GPG-sign commit
echo $OPTIONS_SPEC | git rev-parse --parseopt $parseopt_extra -- $@
Then the following two commands give us the same result :
$ /tmp/opt.sh -S -q
set -- -S -q --
$ /tmp/opt.sh -S-q
set -- -S '-q' --
We cannot know if '-q' is an argument to '-S' or a new option.
With this patch, rev-parse --parseopt will always give an argument to
optional options, as an empty string if the argument is unset.
The same two commands now give us :
$ /tmp/opt.sh -S -q
set -- -S '' -q --
$ /tmp/opt.sh -S-q
set -- -S '-q' --
We can now see if '-q' is an argument to '-S' or an other option.
Also adding two tests in t1502.
There does not seem to be any shell script git command included in git
sources tree that is currently using optional arguments and could be
affected by this change.
Signed-off-by: Nicolas Vigier bo...@mars-attacks.org
---
builtin/rev-parse.c | 3 +++
t/t1502-rev-parse-parseopt.sh | 18 ++
2 files changed, 21 insertions(+)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index de894c7..25e8c74 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -327,6 +327,9 @@ static int parseopt_dump(const struct option *o, const char
*arg, int unset)
if (arg) {
strbuf_addch(parsed, ' ');
sq_quote_buf(parsed, arg);
+ } else if (o-flags PARSE_OPT_OPTARG) {
+ const char empty_arg[] = '';
+ strbuf_add(parsed, empty_arg, strlen(empty_arg));
}
return 0;
}
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 13c88c9..abe7c2f 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -99,4 +99,22 @@ test_expect_success 'test --parseopt --keep-dashdash
--stop-at-non-option withou
test_cmp expect output
'
+cat expect EOF
+set -- -C '' --foo --
+EOF
+
+test_expect_success 'test --parseopt -C --foo' '
+ git rev-parse --parseopt -- -C --foo optionspec output
+ test_cmp expect output
+'
+
+cat expect EOF
+set -- -C '--foo' --
+EOF
+
+test_expect_success 'test --parseopt -C--foo' '
+ git rev-parse --parseopt -- -C--foo optionspec output
+ test_cmp expect output
+'
+
test_done
--
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] git-prompt.sh: show the upstream abbrev name
On Thu, Oct 10, 2013 at 04:43:22PM +0200, Julien Carsique wrote:
It's fixed.
Thanks, the updated patch looks good to me.
Note '+=' was already used line 114:
svn_url_pattern+=\\|$value
I guess noone has tried to use the upstream status indicator with an
SVN upstream and an ancient Bash version yet, thanks for pointing it
out.
-- 8 --
Subject: [PATCH] bash prompt: don't use '+=' operator in show upstream code path
The '+=' operator is not supported by old Bash versions (3.0) we still
care about.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-prompt.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 202e2e520f..7b732d2aeb 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -111,7 +111,7 @@ __git_ps1_show_upstream ()
;;
svn-remote.*.url)
svn_remote[$((${#svn_remote[@]} + 1))]=$value
- svn_url_pattern+=\\|$value
+ svn_url_pattern=$svn_url_pattern\\|$value
upstream=svn+git # default upstream is SVN if
available, else git
;;
esac
--
1.8.4.1.495.gd8d272e
--
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 v3] Add core.mode configuration
On Mon, Oct 14, 2013 at 04:35:50PM -0500, Felipe Contreras wrote: Krzysztof Mazur wrote: On Sat, Oct 12, 2013 at 02:04:45AM -0500, Felipe Contreras wrote: So that we can specify general modes of operation, specifically, add the 'next' mode, which makes Git pre v2.0 behave as Git v2.0. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- I don't think that single option it's a good idea. From the user's point of view I think that the way push.default was introduced and will be changed is much better. So maybe it's better to just add core.addremove option instead? Maybe, but what happens when we start doing changes for v3.0? As a user, I don't and to figure out which are the new configurations that will turn v3.0 behavior on, I just want to be testing that mode, even if I'm not following Git development closely. If I find something annoying with core.mode = next, I report the problem to the mailing list, which is good, we want to know problems with the backward-incompatible changes that will be introduced before it's too late, don't we? But with core.mode = next after upgrade you may experience incompatible change without any warning. I think it's better to keep the old behavior by default and warn the user if with new behavior the result might be different. So the user: a) knows about the change b) may set appropriate option to enable the new default or keep the old behavior and disable the warning c) may report that he does not like that change I'd be fine with having *both* a fine-tuned option to trigger each specific behavior, and another one that turns all those fine-tuned options on that are meant for v2.0. Unfortunately, I don't see much interest from Git developers in either. I think that most users have already set the push.default, so git add is the only problem. If Junio really wants to change git add he should be interested in allowing user to use it now. I don't see the change in git add as an improvement, because removing files with git add IMHO is more confusing than ignoring such files. Maybe introducing new command - git update for instance - which is equivalent to new git add and teaching new users to use it instead of git add is better. Krzysiek -- 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 v3] Add core.mode configuration
Krzysztof Mazur wrote: On Mon, Oct 14, 2013 at 04:35:50PM -0500, Felipe Contreras wrote: Krzysztof Mazur wrote: On Sat, Oct 12, 2013 at 02:04:45AM -0500, Felipe Contreras wrote: So that we can specify general modes of operation, specifically, add the 'next' mode, which makes Git pre v2.0 behave as Git v2.0. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- I don't think that single option it's a good idea. From the user's point of view I think that the way push.default was introduced and will be changed is much better. So maybe it's better to just add core.addremove option instead? Maybe, but what happens when we start doing changes for v3.0? As a user, I don't and to figure out which are the new configurations that will turn v3.0 behavior on, I just want to be testing that mode, even if I'm not following Git development closely. If I find something annoying with core.mode = next, I report the problem to the mailing list, which is good, we want to know problems with the backward-incompatible changes that will be introduced before it's too late, don't we? But with core.mode = next after upgrade you may experience incompatible change without any warning. Yes, and that is actually what the user wants. I mean, why would the user set core.mode=next, if the user doesn't want to experencie incompatible changes? A user that sets this mode is expecting incompatible changes, and will be willing to test them, and report back if there's any problem with them. I think it's better to keep the old behavior by default and warn the user if with new behavior the result might be different. So the user: a) knows about the change b) may set appropriate option to enable the new default or keep the old behavior and disable the warning c) may report that he does not like that change But that's what we are doing already. Look at the test I wrote, it's testing the warnings for the current version of Git. I'd be fine with having *both* a fine-tuned option to trigger each specific behavior, and another one that turns all those fine-tuned options on that are meant for v2.0. Unfortunately, I don't see much interest from Git developers in either. I think that most users have already set the push.default, so git add is the only problem. If Junio really wants to change git add he should be interested in allowing user to use it now. I agree, but he really wants the change, and proof of that is that the warning is already there, and every Git release since then has an annoying message about that at the top. I don't see the change in git add as an improvement, because removing files with git add IMHO is more confusing than ignoring such files. Maybe introducing new command - git update for instance - which is equivalent to new git add and teaching new users to use it instead of git add is better. I agree. At first I simply ignored the changes because I didn't have the patience to figure out what exactly did they mean. Now I was forced to understand them to write this patch, and I'm also forcing myself to use this behavior. 'git add' removing files is counter-intutive, 'git stage' (currently an alias to 'git add') might make more sense. But even better would be to use my proposed changes to 'git stage', which add subcommands, for example: * git stage all (git add --all) * git stage update (git add --update) But it doesn't seem that patch is going to be applied by Junio, so most likely we would have to deal with yet anotyer counter-intuitive behavior in Git. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Add core.mode configuration
On Tue, Oct 15, 2013 at 07:32:39AM -0500, Felipe Contreras wrote: Krzysztof Mazur wrote: But with core.mode = next after upgrade you may experience incompatible change without any warning. Yes, and that is actually what the user wants. I mean, why would the user set core.mode=next, if the user doesn't want to experencie incompatible changes? A user that sets this mode is expecting incompatible changes, and will be willing to test them, and report back if there's any problem with them. With your patch, because it's the only way to have 'git add' v2.0. But if another git v2.0 incompatible change will be added it will not be warned, because with core.mode=next he decided to enable also future changes and that's why I would never set that. I think it's better to keep the old behavior by default and warn the user if with new behavior the result might be different. So the user: a) knows about the change b) may set appropriate option to enable the new default or keep the old behavior and disable the warning c) may report that he does not like that change But that's what we are doing already. Look at the test I wrote, it's testing the warnings for the current version of Git. With pull.default we did that, but with git add v2.0 now we only warn the user. With your patch he can enable new git add (and disable warning), but he also enables future incompatible changes and disables warnings for such changes. He also cannot keep the old behaviour and disable the warning. I don't see the change in git add as an improvement, because removing files with git add IMHO is more confusing than ignoring such files. Maybe introducing new command - git update for instance - which is equivalent to new git add and teaching new users to use it instead of git add is better. I agree. At first I simply ignored the changes because I didn't have the patience to figure out what exactly did they mean. Now I was forced to understand them to write this patch, and I'm also forcing myself to use this behavior. 'git add' removing files is counter-intutive, 'git stage' (currently an alias to 'git add') might make more sense. Yeah, 'git stage' as an alias to 'git add -A' is much more intuitive. Krzysiek -- 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
on broken command chains in tests (was: Re: [PATCH] status: show commit sha1 in You are currently)
Hi,
On Fri, Oct 11, 2013 at 10:42:10AM -0700, Jonathan Nieder wrote:
-- 8 --
Subject: status test: add missing to EOF blocks
When a test forgets to include after each command, it is possible
for an early command to succeed but the test to fail, which can hide
bugs.
Surely you meant succeed and fail the other way around :)
Checked using the following patch to the test harness:
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -425,7 +425,17 @@ test_eval_ () {
eval /dev/null 3 24 $*
}
+check_command_chaining_ () {
+ eval 3 24 (exit 189) $*
+ eval_chain_ret=$?
+ if test $eval_chain_ret != 189
+ then
+ error 'bug in test script: missing in test
commands'
+ fi
+}
+
test_run_ () {
+ check_command_chaining_ $1
test_cleanup=:
expecting_failure=$2
setup_malloc_check
Clever.
If I do a
- error 'bug in test script: missing in test commands'
+ say_color error 'error: bug in test script: missing in
test commands'
to avoid erroring out and skipping the rest of the test script on the
first broken command chain, then we can see that we have a lot of
broken command chains in the test suite:
$ for t in t[0-9][0-9][0-9][0-9]*.sh ; do ./$t ; done |grep -c
'^error:.*missing in test commands$'
345
After a cursory look most of them seem to be the simple missing
type, but there are some funny ones, too.
Gábor
--
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 v3] Add core.mode configuration
Krzysztof Mazur wrote: On Tue, Oct 15, 2013 at 07:32:39AM -0500, Felipe Contreras wrote: Krzysztof Mazur wrote: But with core.mode = next after upgrade you may experience incompatible change without any warning. Yes, and that is actually what the user wants. I mean, why would the user set core.mode=next, if the user doesn't want to experencie incompatible changes? A user that sets this mode is expecting incompatible changes, and will be willing to test them, and report back if there's any problem with them. With your patch, because it's the only way to have 'git add' v2.0. Yeah, but that's not what I'm suggesting. I suggested to have *both* a fined-tunned way to have this behavior, say core.addremove = true, and a way to enable *all* v2.0 behaviors (core.mode = next). If we have both, and the user sets core.mode = next, that means the user wants *all* the incompatible changes. But if another git v2.0 incompatible change will be added it will not be warned, because with core.mode=next he decided to enable also future changes and that's why I would never set that. That's fine, you wouldn't set that, but I would. That's why it's a configuration. I think it's better to keep the old behavior by default and warn the user if with new behavior the result might be different. So the user: a) knows about the change b) may set appropriate option to enable the new default or keep the old behavior and disable the warning c) may report that he does not like that change But that's what we are doing already. Look at the test I wrote, it's testing the warnings for the current version of Git. With pull.default we did that, but with git add v2.0 now we only warn the user. With your patch he can enable new git add (and disable warning), but he also enables future incompatible changes and disables warnings for such changes. Yeah, but I suggested to have *both* a fine-tunned option and a general one, didn't I? He also cannot keep the old behaviour and disable the warning. He cannot do that regardless if my patch is merged or not. I don't see the change in git add as an improvement, because removing files with git add IMHO is more confusing than ignoring such files. Maybe introducing new command - git update for instance - which is equivalent to new git add and teaching new users to use it instead of git add is better. I agree. At first I simply ignored the changes because I didn't have the patience to figure out what exactly did they mean. Now I was forced to understand them to write this patch, and I'm also forcing myself to use this behavior. 'git add' removing files is counter-intutive, 'git stage' (currently an alias to 'git add') might make more sense. Yeah, 'git stage' as an alias to 'git add -A' is much more intuitive. Agreed. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] miscellaneous patches
On 15/10/13 00:25, Jonathan Nieder wrote: Ramsay Jones wrote: These patches don't have too much in common, hence the subject line, except perhaps that 4 of them fix sparse warnings. Thanks. These look good. I tweaked the descriptions a bit to focus on what sparse was warning about instead of our having quieted sparse. :) Yes, the subject lines are much better. Thanks! ATB, Ramsay Jones -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Add core.mode configuration
On Tue, Oct 15, 2013 at 08:29:56AM -0500, Felipe Contreras wrote: Krzysztof Mazur wrote: On Tue, Oct 15, 2013 at 07:32:39AM -0500, Felipe Contreras wrote: Krzysztof Mazur wrote: But with core.mode = next after upgrade you may experience incompatible change without any warning. Yes, and that is actually what the user wants. I mean, why would the user set core.mode=next, if the user doesn't want to experencie incompatible changes? A user that sets this mode is expecting incompatible changes, and will be willing to test them, and report back if there's any problem with them. With your patch, because it's the only way to have 'git add' v2.0. Yeah, but that's not what I'm suggesting. I suggested to have *both* a fined-tunned way to have this behavior, say core.addremove = true, and a way to enable *all* v2.0 behaviors (core.mode = next). I'm just not sure if a lot of users would use core.mode=next, because of possible different behavior without any warning. Maybe we should also add core.mode=next-warn that changes defaults like next but keeps warnings enabled until the user accepts that change by setting appropriate config option? That's safer than next (at least for interactive use) and maybe more users would use that, but I don't think that's worth adding. For me, old behavior by default and warnings with information how to enable new incompatible features, is sufficient. So I don't need core.mode option, but as long it will be useful for other users I have nothing against it. Krzysiek -- 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] Add password parameter to git svn commands and use it when provided instead of defaulting to end-user prompt
Le 15 oct. 2013 à 01:35, Eric Wong normalper...@yhbt.net a écrit : Jeff King p...@peff.net wrote: On Mon, Oct 14, 2013 at 06:40:05PM +, Eric Wong wrote: arnaud.brej...@gmail.com wrote: Signed-off-by: Arnaud Brejeon arnaud.brejeon at gmail.com Thanks. Can you say a little more about the context? Do you run a script that wants to pass a password to 'git svn', do you type it each time on the command line, or something else? Is it ok that the password would show up in ps output? Would the platform's keyring or netrc be usable here, or is there something in the context that avoids that? I think using keyring or netrc is more appropriate. Having a password on the command-line and visible to all via ps doesn't seem like something git should support. Agreed. We have ready-made git-credential helpers to handle this exact problem. We would need to convert SVN::Prompt to use git-credential rather than prompting itself, though. One of the things that held me back from writing such a patch is that I thought libsvn already handled things like keychain integration, and it was better for git-svn to be more svn-like than git-like in its access of SVN repos. Are those already supported out of the box by libsvn? If git's credential helpers are significantly more featureful, it might be worth converting, but if not, I think it makes sense to stay with svn's existing code. I looks like this patch was forgotten once again: http://mid.gmane.org/1371573490-21973-1-git-send-email-matth...@stdin.nl Matthijs: can you add a Signed-off-by for your patch? I'm inclined to push it to Junio as-is since it looks reasonable. I admit I don't know SVN callbacks anymore well enough and don't have time to test with GNOME. I wanted to provide some contexts, I should have done before. I want to use git svn in some scripts that are launched un-attended. As my SVN server requires a password, I need to provide it but it can not be at user prompt. This is why I wanted to add the password parameter that is available in svn CLI. I understand the concern regarding the fact that the password can be retrieved through ps. You are right, it would be better to be able to use git-credential or libsvn solution for this purpose. Arnaud -- 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 v3] Add core.mode configuration
On Tue, Oct 15, 2013 at 10:51 AM, Krzysztof Mazur krzys...@podlesie.net wrote: On Tue, Oct 15, 2013 at 08:29:56AM -0500, Felipe Contreras wrote: Krzysztof Mazur wrote: On Tue, Oct 15, 2013 at 07:32:39AM -0500, Felipe Contreras wrote: Krzysztof Mazur wrote: But with core.mode = next after upgrade you may experience incompatible change without any warning. Yes, and that is actually what the user wants. I mean, why would the user set core.mode=next, if the user doesn't want to experencie incompatible changes? A user that sets this mode is expecting incompatible changes, and will be willing to test them, and report back if there's any problem with them. With your patch, because it's the only way to have 'git add' v2.0. Yeah, but that's not what I'm suggesting. I suggested to have *both* a fined-tunned way to have this behavior, say core.addremove = true, and a way to enable *all* v2.0 behaviors (core.mode = next). I'm just not sure if a lot of users would use core.mode=next, because of possible different behavior without any warning. Maybe we should also add core.mode=next-warn that changes defaults like next but keeps warnings enabled until the user accepts that change by setting appropriate config option? That's safer than next (at least for interactive use) and maybe more users would use that, but I don't think that's worth adding. I like the idea that we could kick git into a mode that applies the behaviors we're talking about having in 2.0, but I'm concerned about one aspect of it. Not having these behaviors until 2.0 hits means we're free to renege on our decisions in favor of something better, or to pull out a bad idea. But once we insert this knob, I don't know that we have the same ability. Once people realize it's there and start using it, it gets harder to back out. I guess we could maintain the stance that the features are not concrete yet, or something like that, but I think people would still get upset if something changes out from under them. So, at the end of the day, I'm just not sure it's worthwhile to have. -John -- 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] split_ident: parse timestamp from end of line
Jeff King p...@peff.net writes:
Yeah, unrolling the loop is probably better. You may even be able
to do so in a single pass with an extra last seen pointer
variable without too much additional code complexity, I would think.
I'm not sure what you mean here.
If you mean doing a single pass to find the final , that is easy,
because we know the length of the line already and can jump past and
start from the back.
I meant a single forward pass, like this.
ident.c | 29 +++--
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/ident.c b/ident.c
index 7d1c79c..ff29779 100644
--- a/ident.c
+++ b/ident.c
@@ -200,7 +200,7 @@ static void strbuf_addstr_without_crud(struct strbuf *sb,
const char *src)
*/
int split_ident_line(struct ident_split *split, const char *line, int len)
{
- const char *cp;
+ const char *cp, *last_ket;
size_t span;
int status = -1;
@@ -225,29 +225,22 @@ int split_ident_line(struct ident_split *split, const
char *line, int len)
split-name_end = split-name_begin;
}
- for (cp = split-mail_begin; cp line + len; cp++)
- if (*cp == '') {
+ for (cp = split-mail_begin, last_ket = NULL; cp line + len; cp++) {
+ if (*cp != '')
+ continue;
+ if (!last_ket)
split-mail_end = cp;
- break;
- }
+ last_ket = cp;
+ }
if (!split-mail_end)
return status;
/*
-* Look from the end-of-line to find the trailing of the mail
-* address, even though we should already know it as split-mail_end.
-* This can help in cases of broken idents with an extra somewhere
-* in the email address. Note that we are assuming the timestamp will
-* never have a in it.
-*
-* Note that we will always find some before going off the front of
-* the string, because will always hit the split-mail_end closing
-* bracket.
+* Typically, last_ket is the same as split_mail_end, but with
+* a broken identity line, there may be multiple closing ket '';
+* read the timestamp after the last one.
*/
- for (cp = line + len - 1; *cp != ''; cp--)
- ;
-
- for (cp = cp + 1; cp line + len isspace(*cp); cp++)
+ for (cp = last_ket + 1; cp line + len isspace(*cp); cp++)
;
if (line + len = cp)
goto person_only;
--
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] split_ident: parse timestamp from end of line
On Tue, Oct 15, 2013 at 10:52:55AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Yeah, unrolling the loop is probably better. You may even be able to do so in a single pass with an extra last seen pointer variable without too much additional code complexity, I would think. I'm not sure what you mean here. If you mean doing a single pass to find the final , that is easy, because we know the length of the line already and can jump past and start from the back. I meant a single forward pass, like this. Ah, I see. You are combining with the pass before, not the pass after. I do not think this is any more (nor less) efficient than what I posted. We still pass over the space after split-mail_end one additional time searching for the closing bracket. Mine is _slightly_ more efficient in that by going backwards we can stop when we see the first '', avoiding looking at the space between mail_end and last_ket. But that space is 0 in the normal case, and even if it is not, we are talking about tens of bytes at most. So I doubt it would ever matter. My version seems a little clearer to me, but that is probably because I wrote it. If you strongly prefer the other, feel free to mark up my patch. -Peff PS I learned a new term, ket. I always called it closing angle bracket. -- 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: What's cooking in git.git (Oct 2013, #02; Mon, 14)
Jonathan Nieder jrnie...@gmail.com writes: What's cooking in git.git (Oct 2013, #02; Mon, 14) Tying up loose ends before the hand-off. I'll try: - slurping your integration branches, - teasing the topics apart out of your 'pu', - populating my rerere database to match your confict resolution, - reconstructing the Meta/Reintegrate insn for 'pu', and - rebuilding 'pu' to make sure the end result matches yours and then push the result out to the usual places listed at http://git-blame.blogspot.com/p/git-public-repositories.html I haven't had enough time to go through the new/updated patches in tree to look at them carefully (let alone peeking into patches that are not in 'pu'), but it appears to me that the list has been busy adding quality fixes and enhancements while I was away. Thanks. -- [Stalled] ... * mg/more-textconv (2013-05-10) 7 commits (merged to 'next' on 2013-10-14 at 8a12490) + grep: honor --textconv for the case rev:path + grep: allow to use textconv filters + t7008: demonstrate behavior of grep with textconv + cat-file: do not die on --textconv without textconv filters + show: honor --textconv for blobs + diff_opt: track whether flags have been set explicitly + t4030: demonstrate behavior of show with textconv Make git grep and git show pay attention to --textconv when dealing with blob objects. There was a question about how defaulting to 'git show --textconv' would interact with the git show HEAD:file.c file.c habit. $gmane/221833 I'll move this to the Cooking category (caught it by looking at the output of Meta/cook -w). [Cooking] * ak/submodule-foreach-quoting (2013-09-27) 1 commit (merged to 'next' on 2013-10-14 at d77c5f1) + submodule foreach: skip eval for more than one argument A behavior change, but a worthwhile one: git submodule foreach was treating its arguments as part of a single command to be concatenated and passed to a shell, making writing buggy scripts too easy. This patch preserves the old just pass it to the shell behavior when a single argument is passed to 'git submodule foreach' and moves to a new skip the shell and use the arguments passed unmolested behavior when more than one argument is passed. When scripts give 'echo' and '$path' (two args), does this change allow the 'echo' command to see the value of $path (coming from $sm_path), or just the not-useful-because-not-exported variable name '$path'? The old behavior (always concatenating and passing to the shell) was similar to the 'ssh' command, while the new behavior (switching on the number of arguments) is what 'xterm -e' does. May need more thought to make sure this change is advertised well so that scripts that used multiple arguments but added their own extra layer of quoting are not broken. I suspect no amount of thinking is enough to avoid fallout from this change. People will not read advertisement and will complain. * ew/keepalive (2013-10-14) 1 commit (merged to 'next' on 2013-10-14 at 24d786f) + http: enable keepalive on TCP sockets $gmane/236154 has a follow-up to do more magic with recent curl. Thanks for leaving a good note here. Will take a look at that follow-up in tomorrow's integration cycle. * jc/revision-range-unpeel (2013-09-20) 2 commits - (possible fixup) jc/revision-range-unpeel - peel only when necessary - revision: do not peel tags used in range notation git rev-list --objects ^v1.0^ v1.0 gave v1.0 tag itself in the output, but git rev-list --objects v1.0^..v1.0 did not. Need to decide either squashing the top fixup in, or dropping it and then merge to 'next'. Funny that you did not decide as the interim maintainer ;-) I'll squash these two, per $gmane/235339, and have it advance, perhaps in tomorrow's integration cycle. * jk/format-patch-from (2013-09-20) 1 commit (merged to 'next' on 2013-09-20 at 0506530) + format-patch: print in-body From only when needed format-patch --from=whom forgot to omit unnecessary in-body from line, i.e. when whom is the same as the real author. Will merge to 'master'. There seem to be many topics marked for 'master' but have been cooking in 'next' for very long. Will start moving them later this week (i.e. not today, not tomorrow). * jc/upload-pack-send-symref (2013-09-17) 7 commits - clone: test the new HEAD detection logic ... Will merge to 'next'. Likewise for the ones slated for 'next'. 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] split_ident: parse timestamp from end of line
Jeff King p...@peff.net writes: My version seems a little clearer to me, but that is probably because I wrote it. If you strongly prefer the other, feel free to mark up my patch. I do not have strong preference either way. Just that I thought two loops would be shorter and easier to understand than three, that's 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
Re: [PATCH v3] Add core.mode configuration
Krzysztof Mazur wrote: On Tue, Oct 15, 2013 at 08:29:56AM -0500, Felipe Contreras wrote: Krzysztof Mazur wrote: On Tue, Oct 15, 2013 at 07:32:39AM -0500, Felipe Contreras wrote: Krzysztof Mazur wrote: But with core.mode = next after upgrade you may experience incompatible change without any warning. Yes, and that is actually what the user wants. I mean, why would the user set core.mode=next, if the user doesn't want to experencie incompatible changes? A user that sets this mode is expecting incompatible changes, and will be willing to test them, and report back if there's any problem with them. With your patch, because it's the only way to have 'git add' v2.0. Yeah, but that's not what I'm suggesting. I suggested to have *both* a fined-tunned way to have this behavior, say core.addremove = true, and a way to enable *all* v2.0 behaviors (core.mode = next). I'm just not sure if a lot of users would use core.mode=next, I'm not sure if a lot of urser would even notice the difference. because of possible different behavior without any warning. I don't see what is the problem. We haven't had the need for push.default = simplewarning, have we? If you want the warning, you don't change anything, if you want to specify something, you already know what you are doing. Maybe we should also add core.mode=next-warn that changes defaults like next but keeps warnings enabled until the user accepts that change by setting appropriate config option? Maybe, but would you actually use that option? That's safer than next (at least for interactive use) and maybe more users would use that, but I don't think that's worth adding. Maybe, but I don't think many users would use either mode, and that's good. For me, old behavior by default and warnings with information how to enable new incompatible features, is sufficient. So I don't need core.mode option, but as long it will be useful for other users I have nothing against it. OK, but that seems to mean you don't need core.mode = next-warn either. I'm not against adding such a mode, but I would like to hear about _somebody_ that would like to actually use it. I don't like to program for ghosts. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Oct 2013, #02; Mon, 14)
Junio C Hamano wrote: I'll try: - slurping your integration branches, - teasing the topics apart out of your 'pu', - populating my rerere database to match your confict resolution, - reconstructing the Meta/Reintegrate insn for 'pu', and - rebuilding 'pu' to make sure the end result matches yours and then push the result out to the usual places Sounds good. The teased-apart topics can also be found at https://github.com/jrn/git [...] This patch preserves the old just pass it to the shell behavior when a single argument is passed to 'git submodule foreach' and moves to a new skip the shell and use the arguments passed unmolested behavior when more than one argument is passed. When scripts give 'echo' and '$path' (two args), does this change allow the 'echo' command to see the value of $path (coming from $sm_path), or just the not-useful-because-not-exported variable name '$path'? The latter. A quick search (web search + codesearch.debian.net) reveals that most callers to submodule foreach either pass a script as a single quoted argument (e.g., http://stackoverflow.com/questions/8364738/bash-git-submodule-foreach: git submodule foreach '[ $path = Libraries/JSONKit ] \ branch=experimental \ || branch=master; git co $branch' or http://sources.debian.net/src/jquery/1.7.2+dfsg-3/Makefile?hl=131#L131: git submodule foreach git pull \$(git config remote.origin.url) ) or pass a one-off command with no arguments intended for the shell (e.g., http://sources.debian.net/src/libreoffice/1:4.1.1-1/g?hl=352#L352, http://sources.debian.net/src/swi-prolog/6.4.1-3/scripts/newversion?hl=81#L81): git submodule foreach git push $@ git submodule foreach git tag -s -f -F $tmp $gittag So I suspect this will fix more scripts than it breaks, though it may still break some. :/ It might make sense to warn when passed multiple arguments and some include shell metacharacters, since that's probably rare, too, except it's punishing people who use multiple arguments as a way to avoid quoting issues. Probably there's no replacement for just advertising the change loudly and seeking out scripts it could break. 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: What's cooking in git.git (Oct 2013, #02; Mon, 14)
Am 15.10.2013 02:12, schrieb Jonathan Nieder: * jl/submodule-mv (2013-10-13) 1 commit - mv: Fix spurious warning when moving a file in presence of submodules Moving a regular file in a repository with a .gitmodules file was producing a warning 'Could not find section in .gitmodules where path=filename'. The test can use a little cleanup. Otherwise looks good. What cleanups do you have in mind? The new test is basically a copy from another one from the same file, so maybe some other tests there need some cleanups too? ;-) -- 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: What's cooking in git.git (Oct 2013, #02; Mon, 14)
Am 15.10.2013 21:16, schrieb Jonathan Nieder: So I suspect this will fix more scripts than it breaks, though it may still break some. :/ Hmm, I'm really not sure if we should do this or not. It might make sense to warn when passed multiple arguments and some include shell metacharacters, since that's probably rare, too, except it's punishing people who use multiple arguments as a way to avoid quoting issues. Probably there's no replacement for just advertising the change loudly and seeking out scripts it could break. And maybe only change that on a major version bump where people should not be terribly surprised about such a change in behavior and are more likely to read release notes? I've thought about issuing a warning on certain quoting patterns too, but dismissed that for not helping much in the scripting case. E.g. at $dayjob we have foreach commands running in the shell execution for quite some jobs on our Jenkins server; nobody would see any warnings there until we'd have the reason to dig deeper int the logs because something breaks. -- 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: What's cooking in git.git (Oct 2013, #02; Mon, 14)
Jens Lehmann wrote: Am 15.10.2013 21:16, schrieb Jonathan Nieder: So I suspect this will fix more scripts than it breaks, though it may still break some. :/ Hmm, I'm really not sure if we should do this or not. What convinced me was Anders's observation that the current behavior can have very bad consequences if a script is passing untrusted input in multiple arguments to git submodule foreach. I still haven't found any examples that pass input intended for the shell to git submodule foreach in multiple arguments. I suspect no one will notice, except that some scripts (like libreoffice's g) start to work better when passed arguments containing shell metacharacters. And maybe only change that on a major version bump where people should not be terribly surprised about such a change in behavior and are more likely to read release notes? Ok with me, but please don't make it 2.0. :) [...] E.g. at $dayjob we have foreach commands running in the shell execution for quite some jobs on our Jenkins server; nobody would see any warnings there until we'd have the reason to dig deeper int the logs because something breaks. Yes, I think that's typical. Warning to stderr probably wouldn't help. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/17] np/pack-v4 updates
Nicolas Pitre n...@fluxnic.net writes: On Sat, 21 Sep 2013, Nguyễn Thái Ngọc Duy wrote: This contains many bug fixes or cleanups. Also you can now run the test suite with v4 by setting GIT_TEST_OPTS=--packv4. The test suite passes now. pack size limit is not officially not supported with v4. index-pack also learns to convert appended trees to v4 for completing thin packs (still need to convert commits though) PS. Nico do you still take patches and then send pull requests to Junio occasionally, or should I start to CC Junio? I'm still willing to act as the middle man if that suits everybody. That gives me the opportunity to review those patches and stay minimally involved. Your reviews in this area are greatly appreciated. -- 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/2] remote: fix trivial memory leak
Jeff King p...@peff.net writes: I wondered if we might also leak when seeing duplicate config options (i.e., leaking the old one when replacing it with the new). But we don't actually strdup() the configured remote names, but instead just point into the struct branch, which owns the data. In addition, we do not copy this string to remote-name in make_remote(), so even if we start allowing destruction of existing remote[], the resulting code will stay safe. So I think an even better fix would be: -- 8 -- Subject: remote: do not copy origin string literal Our default_remote_name starts at origin, but may be overridden by the config file. In the former case, we allocate a new string, but in the latter case, we point to the remote name in an existing struct branch. This gives the variable inconsistent free() semantics (we are sometimes responsible for freeing the string and sometimes pointing to somebody else's storage), and causes a small leak when the allocated string is overridden by config. We can fix both by simply dropping the extra copy and pointing to the string literal. Noticed-by: Felipe Contreras felipe.contre...@gmail.com Signed-off-by: Jeff King p...@peff.net Sounds sensible. Thanks. --- remote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remote.c b/remote.c index e9fedfa..9f1a8aa 100644 --- a/remote.c +++ b/remote.c @@ -483,7 +483,7 @@ static void read_config(void) int flag; if (default_remote_name) /* did this already */ return; - default_remote_name = xstrdup(origin); + default_remote_name = origin; current_branch = NULL; head_ref = resolve_ref_unsafe(HEAD, sha1, 0, flag); if (head_ref (flag REF_ISSYMREF) -- 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] http.c: Spell the null pointer as NULL
Commit 1bbcc224 (http: refactor options to http_get_*, 28-09-2013)
changed the type of final 'options' argument of the http_get_file()
function from an int to an 'struct http_get_options' pointer.
However, it neglected to update the (single) call site. Since this
call was passing '0' to that argument, it was (correctly) being
interpreted as a null pointer. Change to argument to NULL.
Noticed by sparse. (Using plain integer as NULL pointer)
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
Hi Jonathan, Junio,
I'm a little puzzled by not having noticed this until this evening! ;-)
Also, I note that ma...@kernel.org != ma...@repo.or.cz/jrn
ATB,
Ramsay Jones
http.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/http.c b/http.c
index 96d7578..b133ffd 100644
--- a/http.c
+++ b/http.c
@@ -1045,7 +1045,7 @@ static char *fetch_pack_index(unsigned char *sha1, const
char *base_url)
strbuf_addf(buf, %s.temp, sha1_pack_index_name(sha1));
tmp = strbuf_detach(buf, NULL);
- if (http_get_file(url, tmp, 0) != HTTP_OK) {
+ if (http_get_file(url, tmp, NULL) != HTTP_OK) {
error(Unable to get pack index %s, url);
free(tmp);
tmp = NULL;
--
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 V3 1/2] doc: command line interface (cli) dot-repository dwimmery
Philip Oakley philipoak...@iee.org writes: The Git cli will accept dot '.' (period) as the relative path, and thus the current repository. Explain this action. Signed-off-by: Philip Oakley philipoak...@iee.org --- This updates 431260cc8dd It appears that the original has already been merged to 'next', so we need to make this incremental on top. I'll queue this on top. -- 8 -- From: Philip Oakley philipoak...@iee.org Subject: doc/cli: make dot repository an independent bullet point The way to spell the current repository with a '.' dot is independent from how the pathspec allows globs expanded by Git. Make them two separate bullet items in the enumeration. Signed-off-by: Philip Oakley philipoak...@iee.org Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/gitcli.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt index 1672842..24e1784 100644 --- a/Documentation/gitcli.txt +++ b/Documentation/gitcli.txt @@ -58,10 +58,10 @@ the paths in the index that match the pattern to be checked out to your working tree. After running `git add hello.c; rm hello.c`, you will _not_ see `hello.c` in your working tree with the former, but with the latter you will. -+ -Just as the filesystem '.' (period) refers to the current directory, -using a '.' as a repository name in Git (a dot-repository) is a relative -path for your current repository. + + * Just as the filesystem '.' (period) refers to the current directory, + using a '.' as a repository name in Git (a dot-repository) is a relative + path and means your current repository. Here are the rules regarding the flags that you should follow when you are scripting Git: -- 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] http.c: Spell the null pointer as NULL
On Tue, Oct 15, 2013 at 10:55:02PM +0100, Ramsay Jones wrote: Commit 1bbcc224 (http: refactor options to http_get_*, 28-09-2013) changed the type of final 'options' argument of the http_get_file() function from an int to an 'struct http_get_options' pointer. However, it neglected to update the (single) call site. Since this call was passing '0' to that argument, it was (correctly) being interpreted as a null pointer. Change to argument to NULL. Thanks, patch is obviously correct (and the cause was just oversight on my part). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] symbolic-ref: trivial style fix
Felipe Contreras felipe.contre...@gmail.com writes:
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
Let's do something like this instead.
- We usually refrain from making such a tree-wide change in order
to avoid unnecessary conflicts with other real work patches,
but the end result does not have a potentially cumbersome
tree-wide impact.
- We also tend to frown upon an I fixed it here only because I
happened to notice this one, there may be others but it is up to
the readers to see if there are other instances half-assed code
churn.
The point of the proposed log message is to tell readers that this
is a tree-wide clean-up that is worth applying.
-- 8 --
From: Felipe Contreras felipe.contre...@gmail.com
Subject: C: have space around and || operators
Correct all hits from
git grep -e '\(\|||\)[^ ]' -e '[^ ]\(\|||\)' -- '*.c'
i.e. or || operators that are followed by anything but a SP,
or that follow something other than a SP or a HT, so that these
operators have a SP around it when necessary.
We usually refrain from making this kind of a tree-wide change in
order to avoid unnecessary conflicts with other real work patches,
but the end result does not have a potentially cumbersome tree-wide
impact.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/read-tree.c| 2 +-
builtin/rev-list.c | 2 +-
builtin/symbolic-ref.c | 2 +-
compat/regex/regcomp.c | 2 +-
xdiff/xemit.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 0f5d7fe..0d7ef84 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -178,7 +178,7 @@ int cmd_read_tree(int argc, const char **argv, const char
*unused_prefix)
if (1 opts.index_only + opts.update)
die(-u and -i at the same time makes no sense);
- if ((opts.update||opts.index_only) !opts.merge)
+ if ((opts.update || opts.index_only) !opts.merge)
die(%s is meaningless without -m, --reset, or --prefix,
opts.update ? -u : -i);
if ((opts.dir !opts.update))
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 4fc1616..0745e2d 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -322,7 +322,7 @@ int cmd_rev_list(int argc, const char **argv, const char
*prefix)
revs.commit_format = CMIT_FMT_RAW;
if ((!revs.commits
-(!(revs.tag_objects||revs.tree_objects||revs.blob_objects)
+(!(revs.tag_objects || revs.tree_objects || revs.blob_objects)
!revs.pending.nr)) ||
revs.diff)
usage(rev_list_usage);
diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index f481959..71286b4 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -47,7 +47,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char
*prefix)
git_config(git_default_config, NULL);
argc = parse_options(argc, argv, prefix, options,
git_symbolic_ref_usage, 0);
- if (msg !*msg)
+ if (msg !*msg)
die(Refusing to perform update with empty message);
if (delete) {
diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c
index b2c5d46..06f3088 100644
--- a/compat/regex/regcomp.c
+++ b/compat/regex/regcomp.c
@@ -339,7 +339,7 @@ re_compile_fastmap_iter (regex_t *bufp, const re_dfastate_t
*init_state,
p = buf;
*p++ = dfa-nodes[node].opr.c;
while (++node dfa-nodes_len
- dfa-nodes[node].type == CHARACTER
+ dfa-nodes[node].type == CHARACTER
dfa-nodes[node].mb_partial)
*p++ = dfa-nodes[node].opr.c;
memset (state, '\0', sizeof (state));
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 4d86458..4266ada 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -108,7 +108,7 @@ static long def_ff(const char *rec, long len, char *buf,
long sz, void *priv)
{
if (len 0
(isalpha((unsigned char)*rec) || /* identifier? */
-*rec == '_' || /* also identifier? */
+*rec == '_' || /* also identifier? */
*rec == '$')) { /* identifiers from VMS and other
esoterico */
if (len sz)
len = sz;
--
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 v3] build: add default aliases
Jeff King p...@peff.net writes: It seems[1] that some people define ci as commit -a, and some people define st as status -s or even status -sb. These option variants aside. Just like thinking that committing must be the same as publishing, it is a cvs/svn induced braindamage to think that checking in must be the same as committing. The former is a sign of not understanding the distributed, the latter the index. In a world with both check-in and commit as two words usable to denote possibly different concepts, it may make sense to say you check-in the current status of the working tree files into the index, in order to make commits out of it later. -- 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?] inconsistent `git reflog show` output, possibly `git fsck` output
Roberto Tyley roberto.ty...@gmail.com writes: On 21/09/2013 23:16, Keshav Kini wrote: [SNIP] This situation came about because the BFG Repo-Cleaner doesn't write new reflog entries after creating its new objects and moving refs around. True enough - I don't think the BFG does write new entires to the reflog when it does the final ref-update, and it would be nicer if it did. I'll get that fixed. (sorry for replying late) So this can be closed as BFG not writing reflog in a consistent way, and 'git reflog show' is acting GIGO way? Or was there something the core side needs to do? -- 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 00/16] Make Gnome Credential helper more Gnome-y and support ancient distros
This seems to post-date what Jonathan has kept in his 'pu'; is this the latest (I have a huge backlog to wade through, so I'd rather skip it if another reroll is coming and move on to other topics). 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: [BUG?] inconsistent `git reflog show` output, possibly `git fsck` output
Junio C Hamano gits...@pobox.com writes: Roberto Tyley roberto.ty...@gmail.com writes: On 21/09/2013 23:16, Keshav Kini wrote: [SNIP] This situation came about because the BFG Repo-Cleaner doesn't write new reflog entries after creating its new objects and moving refs around. True enough - I don't think the BFG does write new entires to the reflog when it does the final ref-update, and it would be nicer if it did. I'll get that fixed. (sorry for replying late) So this can be closed as BFG not writing reflog in a consistent way, and 'git reflog show' is acting GIGO way? Or was there something the core side needs to do? Hi Junio, Thanks for your reply. In my original mail, immediately after the snippet Roberto quoted above, I said, But that aside, I think how git handles the situation might be a bug. To wit: It seems to me that one of two things should be the case. Either 1) it should be considered impossible to have a reflog for a ref X which doesn't contain a chain of commits leading up to the current location of X; or 2) if reflogs are allowed not to form an unbroken chain of commits leading to X, then `git reflog show` should at least make sure to actually display a commit ID corresponding to the second field of each reflog entry it reads, and not some other commit ID. In the first case, the bug is that `git fsck` doesn't catch the supposedly impossible situation that exists in the repository I've described in this email. In the second case, the bug is that `git reflog show` has bad output. Before this is closed, I would appreciate it if I could get some feedback from git developers on the above two paragraphs. Thanks, Keshav -- 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.txt: Fix asciidoc syntax of --*-pathspecs
Steffen Prohaska proha...@zib.de writes: Labeled lists require a double colon. Signed-off-by: Steffen Prohaska proha...@zib.de --- Looks sensible; it would have been nicer if the log message said something like I eyeballed the output from git grep '[^:]:$' Documentation/\*.txt and these are the only breakages of this kind (which I did just now). Documentation/git.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index 5d68d33..4c2757e 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -475,19 +475,19 @@ example the following invocations are equivalent: This is equivalent to setting the `GIT_LITERAL_PATHSPECS` environment variable to `1`. ---glob-pathspecs: +--glob-pathspecs:: Add glob magic to all pathspec. This is equivalent to setting the `GIT_GLOB_PATHSPECS` environment variable to `1`. Disabling globbing on individual pathspecs can be done using pathspec magic :(literal) ---noglob-pathspecs: +--noglob-pathspecs:: Add literal magic to all pathspec. This is equivalent to setting the `GIT_NOGLOB_PATHSPECS` environment variable to `1`. Enabling globbing on individual pathspecs can be done using pathspec magic :(glob) ---icase-pathspecs: +--icase-pathspecs:: Add icase magic to all pathspec. This is equivalent to setting the `GIT_ICASE_PATHSPECS` environment variable to `1`. -- 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 00/16] Make Gnome Credential helper more Gnome-y and support ancient distros
On 10/15/2013 3:40 PM, Junio C Hamano wrote: This seems to post-date what Jonathan has kept in his 'pu'; is this the latest (I have a huge backlog to wade through, so I'd rather skip it if another reroll is coming and move on to other topics). Thanks. This is the latest. I didn't have anything else planned. I think Philipp planned to submit some style cleanups on top for areas of the code I didn't touch. -Brandon --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.
Yoshioka Tsuneo yoshiokatsu...@gmail.com writes:
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar. But if destination filename is long, the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
Is destination filename more special than the source filename?
Perhaps s/if destination filename is/if filenames are/?
Note: I do not want you to reroll using the suggested
wording without explanation; it may be possible that I am
missing something obvious and do not understand why you
singled out destination, in which case I'd rather see it
explained better in the log message than the potentially
suboptimal suggestion I made in the review without
understanding the issue. Of course, it is possible that you
want to do the same when source is overlong, in which case
you can just say Yeah, you're right; will reroll.
The above applies to all the other comments in this message.
Also s/source commit/original/. You may not be comparing two
commits after all.
Make sure there is always an arrow, like ...foo = ...bar.
The output can contains curly braces('{','}') for grouping.
s/contains/contain/;
So, in general, the outpu format is pfx{mid_a = mid_b}sfx
s/outpu/t/;
To keep arrow(=), try to omit pfx as long as possible at first
because later part or changing part will be the more important part.
If it is not enough, shorten mid_a, mid_b, and sfx trying to
have the maximum length the same because those will be equaly important.
A sound reasoning.
Signed-off-by: Tsuneo Yoshioka yoshiokatsu...@gmail.com
Test-added-by: Thomas Rast tr...@inf.ethz.ch
---
diff.c | 187
+++--
t/t4001-diff-rename.sh | 12
2 files changed, 177 insertions(+), 22 deletions(-)
diff --git a/diff.c b/diff.c
index a04a34d..cf50807 100644
--- a/diff.c
+++ b/diff.c
@@ -1258,11 +1258,12 @@ static void fn_out_consume(void *priv, char *line,
unsigned long len)
}
}
-static char *pprint_rename(const char *a, const char *b)
+static void pprint_rename_find_common_prefix_suffix(const char *a, const
char *b
+
, struct strbuf *pfx, struct strbuf *a_mid
+
, struct strbuf *b_mid, struct strbuf *sfx)
What kind of line splitting is this?
I think the real issue is that the function name is overly long, but
aside from that,
- comma comes at the end of the line, not at the beginning of the
next line;
- the second and subsequent lines are indented, but not more than
the usual line width (align with the first letter inside the
opening parenthesis of the first line);
- a_mid and b_mid are more alike than pfx and a_mid.
so I would expect to see it more like:
static void abbrev_rename(const char *a, const char *b,
struct strbuf *pfx,
struct strbuf *a_mid, struct strbuf *b_mid,
struct strbuf *sfx)
Note that the suggested name does not say pprint, because in your
version of this file, the code around here is no longer doing any
printing. The caller does so after using this function to decide
how to abbreviate renames, so naming the helper function after what
it does (e.g. abbreviate renames) is more appropriate.
{
const char *old = a;
const char *new = b;
- struct strbuf name = STRBUF_INIT;
int pfx_length, sfx_length;
int pfx_adjust_for_slash;
int len_a = strlen(a);
@@ -1272,10 +1273,9 @@ static char *pprint_rename(const char *a, const char
*b)
int qlen_b = quote_c_style(b, NULL, NULL, 0);
if (qlen_a || qlen_b) {
- quote_c_style(a, name, NULL, 0);
- strbuf_addstr(name, = );
- quote_c_style(b, name, NULL, 0);
- return strbuf_detach(name, NULL);
+ quote_c_style(a, a_mid, NULL, 0);
+ quote_c_style(b, b_mid, NULL, 0);
+ return;
}
/* Find common prefix */
@@ -1321,18 +1321,151 @@ static char *pprint_rename(const char *a, const char
*b)
a_midlen = 0;
if (b_midlen 0)
b_midlen = 0;
+
Trailing whitespace (there are many others you added to this file; I
won't bother to point out all of them).
+ strbuf_add(pfx, a, pfx_length);
+ strbuf_add(a_mid, a + pfx_length, a_midlen);
+ strbuf_add(b_mid, b + pfx_length, b_midlen);
+ strbuf_add(sfx, a + len_a - sfx_length, sfx_length);
+}
+
+/*
+ * Omit each parts to fix in name_width.
+ * Formatted string is pfx{a_mid = b_mid}sfx.
+ * At first, omit pfx as long as possible.
+ * If it is not enough, omit
Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments
Nicolas Vigier bo...@mars-attacks.org writes: git rev-parse --parseopt does not allow us to see the difference between an option with an optional argument starting with a dash, and an option with an unset optional argument followed by an other option. If I use this script : $ cat /tmp/opt.sh #!/bin/sh OPTIONS_SPEC=\ git [options] -- q,quiet be quiet S,gpg-sign? GPG-sign commit echo $OPTIONS_SPEC | git rev-parse --parseopt $parseopt_extra -- $@ Then the following two commands give us the same result : $ /tmp/opt.sh -S -q set -- -S -q -- $ /tmp/opt.sh -S-q set -- -S '-q' -- We cannot know if '-q' is an argument to '-S' or a new option. With this patch, rev-parse --parseopt will always give an argument to optional options, as an empty string if the argument is unset. The same two commands now give us : $ /tmp/opt.sh -S -q set -- -S '' -q -- $ /tmp/opt.sh -S-q set -- -S '-q' -- Two are different, but the former set -- -S '' -q -- is not what you want, either, no? -S with an explicit empty argument and -S alone without argument may mean two totally different things, which is the whole point of option with an optional parameter. If some code that have been using rev-parse --parseopt was happy with $ /tmp/opt.sh -S set -- -S -- and then your updated version gave it this instead: $ /tmp/opt.sh -S set -- -S '' -- wouldn't it be a regression to them? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.
Junio C Hamano gits...@pobox.com writes:
Yoshioka Tsuneo yoshiokatsu...@gmail.com writes:
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar. But if destination filename is long, the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
Is destination filename more special than the source filename?
Perhaps s/if destination filename is/if filenames are/?
Note: I do not want you to reroll using the suggested
wording without explanation; it may be possible that I am
missing something obvious and do not understand why you
singled out destination, in which case I'd rather see it
explained better in the log message than the potentially
suboptimal suggestion I made in the review without
understanding the issue. Of course, it is possible that you
want to do the same when source is overlong, in which case
you can just say Yeah, you're right; will reroll.
The above applies to all the other comments in this message.
Also s/source commit/original/. You may not be comparing two
commits after all.
Make sure there is always an arrow, like ...foo = ...bar.
The output can contains curly braces('{','}') for grouping.
s/contains/contain/;
So, in general, the outpu format is pfx{mid_a = mid_b}sfx
s/outpu/t/;
To keep arrow(=), try to omit pfx as long as possible at first
because later part or changing part will be the more important part.
If it is not enough, shorten mid_a, mid_b, and sfx trying to
have the maximum length the same because those will be equaly important.
A sound reasoning.
Also s/equaly/equally/;
-Keshav
--
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] http.c: Spell the null pointer as NULL
Ramsay Jones ram...@ramsay1.demon.co.uk writes: Also, I note that ma...@kernel.org != ma...@repo.or.cz/jrn 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] rev-parse --parseopt: fix handling of optional arguments
Nicolas Vigier wrote: $ cat /tmp/opt.sh #!/bin/sh OPTIONS_SPEC=\ git [options] -- q,quiet be quiet S,gpg-sign? GPG-sign commit echo $OPTIONS_SPEC | git rev-parse --parseopt $parseopt_extra -- $@ Then the following two commands give us the same result : $ /tmp/opt.sh -S -q set -- -S -q -- $ /tmp/opt.sh -S-q set -- -S '-q' -- We cannot know if '-q' is an argument to '-S' or a new option. Hmph. As Junio mentioned, inserting '' would be a backward-incompatible change. I don't think it's worth breaking existing scripts. Probably what is needed is a new parseopt special character with the new semantics (e.g., Use ?? to mean the option has an optional argument. If the option is supplied without its argument, the argument is taken to be ''. or something like Use ?default to mean the option has an optional argument. If the option is supplied without its argument and default is nonempty, the argument is taken to be default. ). Sensible? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] symbolic-ref: trivial style fix
Junio C Hamano wrote: From: Felipe Contreras felipe.contre...@gmail.com Subject: C: have space around and || operators [...] builtin/read-tree.c| 2 +- builtin/rev-list.c | 2 +- builtin/symbolic-ref.c | 2 +- compat/regex/regcomp.c | 2 +- xdiff/xemit.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) For what it's worth, Reviewed-by: Jonathan Nieder jrnie...@gmail.com [...] --- a/compat/regex/regcomp.c +++ b/compat/regex/regcomp.c @@ -339,7 +339,7 @@ re_compile_fastmap_iter (regex_t *bufp, const re_dfastate_t *init_state, p = buf; *p++ = dfa-nodes[node].opr.c; while (++node dfa-nodes_len - dfa-nodes[node].type == CHARACTER + dfa-nodes[node].type == CHARACTER It took a little staring to see what changed here. The preimage has a tab, probably from an autoformatter gone wild. I don't think fixing it should interfere with importing new versions of compat/regex, so the change seems fine. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments
Jonathan Nieder jrnie...@gmail.com writes: Nicolas Vigier wrote: $ cat /tmp/opt.sh #!/bin/sh OPTIONS_SPEC=\ git [options] -- q,quiet be quiet S,gpg-sign? GPG-sign commit echo $OPTIONS_SPEC | git rev-parse --parseopt $parseopt_extra -- $@ Then the following two commands give us the same result : $ /tmp/opt.sh -S -q set -- -S -q -- $ /tmp/opt.sh -S-q set -- -S '-q' -- We cannot know if '-q' is an argument to '-S' or a new option. Hmph. As Junio mentioned, inserting '' would be a backward-incompatible change. I don't think it's worth breaking existing scripts. Probably what is needed is a new parseopt special character with the new semantics (e.g., Use ?? to mean the option has an optional argument. If the option is supplied without its argument, the argument is taken to be ''. or something like Use ?default to mean the option has an optional argument. If the option is supplied without its argument and default is nonempty, the argument is taken to be default. ). Sensible? You just made these two that the user clearly meant to express two different things indistinguishable. opt.sh -S opt.sh -S '' So I do not think it is sensible. In fact, I do not think there is any sensible way to handle a shortopt with optional parameter that is not at the end of the command line. And that is exactly why gitcli.txt tells users to use the 'sticked' form, and ends the bullet point with: An option that takes optional option-argument must be written in the 'sticked' form. That still does not give the command line a way to express an option that could take an optional argument without the optional argument in the middle of the command line, 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] symbolic-ref: trivial style fix
Jonathan Nieder jrnie...@gmail.com writes: - dfa-nodes[node].type == CHARACTER + dfa-nodes[node].type == CHARACTER It took a little staring to see what changed here. The preimage has a tab, probably from an autoformatter gone wild. I don't think fixing it should interfere with importing new versions of compat/regex, so the change seems fine. xdiff/xemit.c had the breakage of the same nature. Perhaps the commit log message should mention these two. -- 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] rev-parse --parseopt: fix handling of optional arguments
On Tue, 15 Oct 2013, Junio C Hamano wrote: Nicolas Vigier bo...@mars-attacks.org writes: git rev-parse --parseopt does not allow us to see the difference between an option with an optional argument starting with a dash, and an option with an unset optional argument followed by an other option. If I use this script : $ cat /tmp/opt.sh #!/bin/sh OPTIONS_SPEC=\ git [options] -- q,quiet be quiet S,gpg-sign? GPG-sign commit echo $OPTIONS_SPEC | git rev-parse --parseopt $parseopt_extra -- $@ Then the following two commands give us the same result : $ /tmp/opt.sh -S -q set -- -S -q -- $ /tmp/opt.sh -S-q set -- -S '-q' -- We cannot know if '-q' is an argument to '-S' or a new option. With this patch, rev-parse --parseopt will always give an argument to optional options, as an empty string if the argument is unset. The same two commands now give us : $ /tmp/opt.sh -S -q set -- -S '' -q -- $ /tmp/opt.sh -S-q set -- -S '-q' -- Two are different, but the former set -- -S '' -q -- is not what you want, either, no? -S with an explicit empty argument and -S alone without argument may mean two totally different things, which is the whole point of option with an optional parameter. If some code that have been using rev-parse --parseopt was happy with $ /tmp/opt.sh -S set -- -S -- and then your updated version gave it this instead: $ /tmp/opt.sh -S set -- -S '' -- wouldn't it be a regression to them? Indeed, this could be a regression to them. I couldn't find any script using rev-parse --parseopt with an option with an optional argument, but yes, it doesn't mean that nobody uses 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] rev-parse --parseopt: fix handling of optional arguments
On Tue, 15 Oct 2013, Jonathan Nieder wrote: Nicolas Vigier wrote: $ cat /tmp/opt.sh #!/bin/sh OPTIONS_SPEC=\ git [options] -- q,quiet be quiet S,gpg-sign? GPG-sign commit echo $OPTIONS_SPEC | git rev-parse --parseopt $parseopt_extra -- $@ Then the following two commands give us the same result : $ /tmp/opt.sh -S -q set -- -S -q -- $ /tmp/opt.sh -S-q set -- -S '-q' -- We cannot know if '-q' is an argument to '-S' or a new option. Hmph. As Junio mentioned, inserting '' would be a backward-incompatible change. I don't think it's worth breaking existing scripts. Probably what is needed is a new parseopt special character with the new semantics (e.g., Use ?? to mean the option has an optional argument. If the option is supplied without its argument, the argument is taken to be ''. or something like Use ?default to mean the option has an optional argument. If the option is supplied without its argument and default is nonempty, the argument is taken to be default. ). Sensible? Yes, I think it's sensible. I will look at this and propose an other patch. 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] rev-parse --parseopt: fix handling of optional arguments
Junio C Hamano wrote:
You just made these two that the user clearly meant to express two
different things indistinguishable.
opt.sh -S
opt.sh -S ''
[...]
And that is exactly why gitcli.txt tells users to use the 'sticked'
form, and ends the bullet point with:
An option that takes optional option-argument must be written in
the 'sticked' form.
Yes, another possibility in that vein would be to teach rev-parse
--parseopt an OPTIONS_LONG_STICKED output format, and then parse with
while :
do
case $1 in
--gpg-sign)
... no keyid ...
;;
--gpg-sign=*)
keyid=${1#--gpg-sign=}
...
;;
esac
shift
done
This still leaves
opt.sh -S
and
opt.sh -S''
indistinguishable. Given what the shell passes to execve, I think
that's ok.
The analagous method without preferring long options could work almost
as well:
while :
do
case $1 in
-S)
... no keyid ...
;;
-S?*)
keyid=${1#-S}
...
;;
esac
shift
done
but it mishandles --gpg-sign= with empty argument.
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] t3600: fix broken choking git rm test
The test 'choking git rm should not let it die with cruft' is supposed to check 'git rm's behavior when interrupted by provoking a SIGPIPE while 'git rm' is busily deleting files from a specially crafted index. This test is silently broken for the following reasons: - The test crafts a special index by feeding a large number of index entries with null shas to 'git update-index --index-info'. It was OK back then when this test was introduced in commit 0693f9ddad (Make sure lockfiles are unlocked when dying on SIGPIPE, 2008-12-18), but since commit 4337b5856f (do not write null sha1s to on-disk index, 2012-07-28) null shas are not allowed in the on-disk index causing 'git update-index' to error out. - The barfing 'git update-index --index-info' should fail the test, but it remains unnoticed because of the severely broken chain: the test's result depends solely on whether there is a stale lock file left behind, but after 'git update-index' errors out 'git rm' won't be executed at all. To fix this test feed only non-null shas to 'git update-index' and restore the chain (partly by adding a missing and by using the test_when_finished helper instead of manual cleanup). Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- A particularly funny one from the fallout of gmane/236183 t/t3600-rm.sh | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 85854f338f..4dd0130dd9 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -240,18 +240,15 @@ test_expect_success 'refresh index before checking if it is up-to-date' ' test_expect_success 'choking git rm should not let it die with cruft' ' git reset -q --hard + test_when_finished rm -f .git/index.lock ; git reset -q --hard i=0 while test $i -lt 12000 do - echo 100644 $_z40 0some-file-$i + echo 100644 1234567890123456789012345678901234567890 0 some-file-$i i=$(( $i + 1 )) done | git update-index --index-info - git rm -n some-file-* | :; - test -f .git/index.lock - status=$? - rm -f .git/index.lock - git reset -q --hard - test $status != 0 + git rm -n some-file-* | : + test ! -f .git/index.lock ' test_expect_success 'rm removes subdirectories recursively' ' -- 1.8.4.1.495.gd8d272e -- 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] t3600: fix broken choking git rm test
SZEDER Gábor wrote: The test 'choking git rm should not let it die with cruft' is supposed to check 'git rm's behavior when interrupted by provoking a SIGPIPE while 'git rm' is busily deleting files from a specially crafted index. This test is silently broken for the following reasons: [...] Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- A particularly funny one from the fallout of gmane/236183 Fun. :) Makes sense. --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -240,18 +240,15 @@ test_expect_success 'refresh index before checking if it is up-to-date' ' test_expect_success 'choking git rm should not let it die with cruft' ' git reset -q --hard + test_when_finished rm -f .git/index.lock ; git reset -q --hard I'd use here --- the test_cleanup checks the exit status from this scriptlet, so it's a good habit. [...] - test -f .git/index.lock - status=$? - rm -f .git/index.lock - git reset -q --hard - test $status != 0 + test ! -f .git/index.lock Gah. Thanks for cleaning it up. Maybe test_path_is_missing would make sense here? (It would notice a .git/index.lock directory, which is not very likely :), but more importantly, it says why it is failing the test when it fails.) With or without the changes mentioned above, Reviewed-by: Jonathan Nieder jrnie...@gmail.com Thanks. diff --git i/t/t3600-rm.sh w/t/t3600-rm.sh index 8386b54..540c49b 100755 --- i/t/t3600-rm.sh +++ w/t/t3600-rm.sh @@ -240,7 +240,7 @@ test_expect_success 'refresh index before checking if it is up-to-date' ' test_expect_success 'choking git rm should not let it die with cruft' ' git reset -q --hard - test_when_finished rm -f .git/index.lock ; git reset -q --hard + test_when_finished rm -f .git/index.lock git reset -q --hard i=0 while test $i -lt 12000 do @@ -248,7 +248,7 @@ test_expect_success 'choking git rm should not let it die with cruft' ' i=$(( $i + 1 )) done | git update-index --index-info git rm -n some-file-* | : - test ! -f .git/index.lock + test_path_is_missing .git/index.lock ' test_expect_success 'rm removes subdirectories recursively' ' -- 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] t3600: fix broken choking git rm test
On Tue, Oct 15, 2013 at 05:18:04PM -0700, Jonathan Nieder wrote: SZEDER Gábor wrote: --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -240,18 +240,15 @@ test_expect_success 'refresh index before checking if it is up-to-date' ' test_expect_success 'choking git rm should not let it die with cruft' ' git reset -q --hard + test_when_finished rm -f .git/index.lock ; git reset -q --hard I'd use here --- the test_cleanup checks the exit status from this scriptlet, so it's a good habit. OK. My motivation for the ';' was that we should make sure that both steps of the cleanup are executed. However, now thinking about it if regular 'rm -f' can't remove the lock file then all bets are off anyway. [...] - test -f .git/index.lock - status=$? - rm -f .git/index.lock - git reset -q --hard - test $status != 0 + test ! -f .git/index.lock Gah. Thanks for cleaning it up. Maybe test_path_is_missing would make sense here? (It would notice a .git/index.lock directory, which is not very likely :), but more importantly, it says why it is failing the test when it fails.) I was not aware of the test_path_is_missing helper. I don't think it matters whether it's a file or a directory, because a stale .git/index.lock directory would be just as bad. -- 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] t3600: fix broken choking git rm test
The test 'choking git rm should not let it die with cruft' is supposed to check 'git rm's behavior when interrupted by provoking a SIGPIPE while 'git rm' is busily deleting files from a specially crafted index. This test is silently broken for the following reasons: - The test crafts a special index by feeding a large number of index entries with null shas to 'git update-index --index-info'. It was OK back then when this test was introduced in commit 0693f9ddad (Make sure lockfiles are unlocked when dying on SIGPIPE, 2008-12-18), but since commit 4337b5856f (do not write null sha1s to on-disk index, 2012-07-28) null shas are not allowed in the on-disk index causing 'git update-index' to error out. - The barfing 'git update-index --index-info' should fail the test, but it remains unnoticed because of the severely broken chain: the test's result depends solely on whether there is a stale lock file left behind, but after 'git update-index' errors out 'git rm' won't be executed at all. To fix this test feed only non-null shas to 'git update-index' and restore the chain (partly by adding a missing and by using the test_when_finished helper instead of manual cleanup). Signed-off-by: SZEDER Gábor sze...@ira.uka.de Reviewed-by: Jonathan Nieder jrnie...@gmail.com --- t/t3600-rm.sh | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 824342f413..ad30a61f9e 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -230,18 +230,15 @@ test_expect_success 'refresh index before checking if it is up-to-date' ' test_expect_success 'choking git rm should not let it die with cruft' ' git reset -q --hard + test_when_finished rm -f .git/index.lock git reset -q --hard i=0 while test $i -lt 12000 do - echo 100644 $_z40 0some-file-$i + echo 100644 1234567890123456789012345678901234567890 0 some-file-$i i=$(( $i + 1 )) done | git update-index --index-info - git rm -n some-file-* | :; - test -f .git/index.lock - status=$? - rm -f .git/index.lock - git reset -q --hard - test $status != 0 + git rm -n some-file-* | : + test_path_is_missing .git/index.lock ' test_expect_success 'rm removes subdirectories recursively' ' -- 1.8.4.1.495.gd8d272e -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation/config.txt: denyDeleteCurrent applies to bare repos too
From: Brandon Casey draf...@gmail.com The setting of denyDeleteCurrent applies to both bare and non-bare repositories. Correct the description on this point, and expand it to provide some background justification for the current behavior and describe the full suite of settings. Signed-off-by: Brandon Casey draf...@gmail.com --- Documentation/config.txt | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index c3f7002..3d416ec 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1993,8 +1993,15 @@ receive.denyDeletes:: the ref. Use this to prevent such a ref deletion via a push. receive.denyDeleteCurrent:: - If set to true, git-receive-pack will deny a ref update that - deletes the currently checked out branch of a non-bare repository. + If set to true or refuse, git-receive-pack will deny a ref update + that deletes the currently checked out branch of a non-bare repository, + or the default branch in a bare repository. i.e. the branch + that HEAD refers to. Deleting the current branch from a remote will + cause the HEAD symbolic ref to become dangling and will result in the + next clone from it to not check out anything. If set to warn, + then a warning will be printed to stderr and the deletion will be + performed. If set to false or ignore, then the deletion will be + performed with no warning message. Defaults to refuse. receive.denyCurrentBranch:: If set to true or refuse, git-receive-pack will deny a ref update -- 1.8.4.rc4.6.gd19 --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] symbolic-ref: trivial style fix
Junio C Hamano wrote: Felipe Contreras felipe.contre...@gmail.com writes: Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Let's do something like this instead. - We usually refrain from making such a tree-wide change in order to avoid unnecessary conflicts with other real work patches, but the end result does not have a potentially cumbersome tree-wide impact. You might want to change that phrasing to we usually give low priority. Sure, this is low priority, but this is something has to be done sooner or later, if we don't the code-style will be forever inconsistent. If there is a conflict it would be sensible to delay the change for another release perhaps, it's perfectly sensible to ask the submitter to set another version, and hopefully there would be no conflict. Usually resolving the conflict is trivial, but there's a lot of options. Refraining from making a code-style fix is not ideal. We want those. It's only a matter of how to implement them. - We also tend to frown upon an I fixed it here only because I happened to notice this one, there may be others but it is up to the readers to see if there are other instances half-assed code churn. If you have more general patch, sure, but would you really reject a patch that fixes one thing, because it doesn't fix all similar things at the same time? The point of the proposed log message is to tell readers that this is a tree-wide clean-up that is worth applying. -- 8 -- From: Felipe Contreras felipe.contre...@gmail.com Subject: C: have space around and || operators Saying this patch is from me is not really accurate, it's based on a patch by me, or it was reported by me, but it's not really from me. I don't have a problem taking the credit though, as probably half the change is finding out there's a problem, just saying. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] build: add default aliases
Junio C Hamano wrote: Jeff King p...@peff.net writes: It seems[1] that some people define ci as commit -a, and some people define st as status -s or even status -sb. These option variants aside. Just like thinking that committing must be the same as publishing, it is a cvs/svn induced braindamage to think that checking in must be the same as committing. The former is a sign of not understanding the distributed, the latter the index. In a world with both check-in and commit as two words usable to denote possibly different concepts, it may make sense to say you check-in the current status of the working tree files into the index, in order to make commits out of it later. Yet a wide amount of users do use 'ci' to mean 'commit', so basically they are just wrong. So you are saying they are just ignorant. Personally I don't care if it's 'ci', or 'co', or 'cm', or 'ct'. I just want/need a shortcut, then I can train my fingers to type that. If you have a better alias than 'ci', then by all means, throw away your suggestion. Now, if you are commenting on the aliases, that would mean you are not against the idea of aliaes per se, but more about values of those aliases. So if we agreed on the right values, you would welcome this patch. Is that correct? -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Add core.mode configuration
John Szakmeister wrote: On Tue, Oct 15, 2013 at 10:51 AM, Krzysztof Mazur krzys...@podlesie.net wrote: On Tue, Oct 15, 2013 at 08:29:56AM -0500, Felipe Contreras wrote: Krzysztof Mazur wrote: On Tue, Oct 15, 2013 at 07:32:39AM -0500, Felipe Contreras wrote: Krzysztof Mazur wrote: But with core.mode = next after upgrade you may experience incompatible change without any warning. Yes, and that is actually what the user wants. I mean, why would the user set core.mode=next, if the user doesn't want to experencie incompatible changes? A user that sets this mode is expecting incompatible changes, and will be willing to test them, and report back if there's any problem with them. With your patch, because it's the only way to have 'git add' v2.0. Yeah, but that's not what I'm suggesting. I suggested to have *both* a fined-tunned way to have this behavior, say core.addremove = true, and a way to enable *all* v2.0 behaviors (core.mode = next). I'm just not sure if a lot of users would use core.mode=next, because of possible different behavior without any warning. Maybe we should also add core.mode=next-warn that changes defaults like next but keeps warnings enabled until the user accepts that change by setting appropriate config option? That's safer than next (at least for interactive use) and maybe more users would use that, but I don't think that's worth adding. I like the idea that we could kick git into a mode that applies the behaviors we're talking about having in 2.0, but I'm concerned about one aspect of it. Not having these behaviors until 2.0 hits means we're free to renege on our decisions in favor of something better, or to pull out a bad idea. But once we insert this knob, I don't know that we have the same ability. Once people realize it's there and start using it, it gets harder to back out. I guess we could maintain the stance that the features are not concrete yet, or something like that, but I think people would still get upset if something changes out from under them. We cannot change the behavior of push.default = simple already, so at least that option is not in question. Presumably you are worried about the other options that can't be enabled in any way. But think about this; you are worried that if we add an *option* to enable this new behaviors, then we would be kind of forced to keep these behaviors. That seems to imply that you are proposing the current default; we wait until 2.0 and not make it an *option*, but make it *default*. I think waiting until 2.0 to make it a default without evern having an option, and thus nobody actuallly testing this, is way worst than what I'm proposing; to add an option to start testing. So, at the end of the day, I'm just not sure it's worthwhile to have. This is exactly what happened on 1.6; nobody really tested the 'git foo' behavior, so we just switched from one version to the next. If you are not familiar with the outcome; it wasn't good. So I say we shouldn't just provide warnings, but also have an option to allow users (probably a minority) to start testing this. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Add core.mode configuration
Krzysztof Mazur wrote: On Tue, Oct 15, 2013 at 01:51:41PM -0500, Felipe Contreras wrote: I don't see what is the problem. We haven't had the need for push.default = simplewarning, have we? If you want the warning, you don't change anything, if simplewarning makes no sense, because push.default=simple sets exact behavior, Exactly. not some next behavior that may change in future. But I'm suggesting to add a core.addremove option as well, like you suggested, am I not? That option wouldn't change in the future. you want to specify something, you already know what you are doing. Maybe we should also add core.mode=next-warn that changes defaults like next but keeps warnings enabled until the user accepts that change by setting appropriate config option? Maybe, but would you actually use that option? No. So you would be happy if we had core.addremove = true *and* core.mode = next, right? You would use one, different people with different needs would use the other. That's safer than next (at least for interactive use) and maybe more users would use that, but I don't think that's worth adding. Maybe, but I don't think many users would use either mode, and that's good. For me, old behavior by default and warnings with information how to enable new incompatible features, is sufficient. So I don't need core.mode option, but as long it will be useful for other users I have nothing against it. OK, but that seems to mean you don't need core.mode = next-warn either. I'm not against adding such a mode, but I would like to hear about _somebody_ that would like to actually use it. I don't like to program for ghosts. As I said earlier, I don't think that next-warn it's worth adding, but such option might increase the number of people interested in the core.mode. Well that's a hypothesis, and I would be interested in finding out if that's true, but until I see somebody that says I want core.mode = next-war, I'm going to assume they are hypothetical. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
