Re: [PATCH v3] Add core.mode configuration
On Tue, Oct 15, 2013 at 11:03:26PM -0500, Felipe Contreras wrote: 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? Yes, I think we both agreed on adding core.addremove. I'm just not convinced if we should also add core.mode. 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. Yes, if there are people that will use core.mode it will be worth adding. I'm just not one of them. 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. Yes, that's just a hypothesis. 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 1/2] doc: command line interface (cli) dot-repository dwimmery
From: Junio C Hamano gits...@pobox.com 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. Thank you, that looks good. -- 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] rev-parse --parseopt: fix handling of optional arguments
Am 10/16/2013 1:57, schrieb Jonathan Nieder: 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 Aren't you people trying to solve something that can't besolved? What does git commit -S LICENSE mean? Commit the index and sign with the key-id LICENSE or commit just the file LICENSE with the default key-id? -- Hannes -- To unsubscribe from this list: send the line 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:55:07PM -0500, Felipe Contreras wrote: John Szakmeister wrote: 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. If we add core.addremove=true the same applies to it - we cannot remove it later, the only we can do is to disable it by default in future versions after testing (core.addremove=true or core.mode=next). 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. BTW, I'm still using pre-1.6 git-foo, I have /usr/libexec/git-core in my PATH. So I would like to always have an option to disable some new incompatible improvements. So I say we shouldn't just provide warnings, but also have an option to allow users (probably a minority) to start testing this. and an option to keep the old behavior, like we did with push.default. 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
[PATCH] status: allow branch info color customization
From: Alexander Hirsch 1ze...@gmail.com git status -bs (--branch --short) does not seem to allow customization of the colors for the local and remote branch. This patch adds these via the color.status.local and color.status.remote config variables. Given the trivial nature of this patch I did not write a test for it. I did a small check that it's working so, to be on the safe side. Signed-off-by: Alexander Hirsch 1ze...@gmail.com --- Documentation/config.txt | 7 +-- builtin/commit.c | 4 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index d4d93c9..261fc99 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -904,9 +904,12 @@ color.status.slot:: `added` or `updated` (files which are added but not committed), `changed` (files which are changed but not added in the index), `untracked` (files which are not tracked by Git), - `branch` (the current branch), or + `branch` (the current branch), `nobranch` (the color the 'no branch' warning is shown in, defaulting - to red). The values of these variables may be specified as in + to red), + `local` (the local branch when showing branch info), or + `remote` (the remote-tracked branch when showing branch info). + The values of these variables may be specified as in color.branch.slot. color.ui:: diff --git a/builtin/commit.c b/builtin/commit.c index 6ab4605..43365b4 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1165,6 +1165,10 @@ static int parse_status_slot(const char *var, int offset) return WT_STATUS_HEADER; if (!strcasecmp(var+offset, branch)) return WT_STATUS_ONBRANCH; + if (!strcasecmp(var+offset, local)) + return WT_STATUS_LOCAL_BRANCH; + if (!strcasecmp(var+offset, remote)) + return WT_STATUS_REMOTE_BRANCH; if (!strcasecmp(var+offset, updated) || !strcasecmp(var+offset, added)) return WT_STATUS_UPDATED; -- 1.8.4.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
pack corruption post-mortem
I was recently presented with a repository with a corrupted packfile,
and was asked if the data was recoverable. This post-mortem describes
the steps I took to investigate and fix the problem. I thought others
might find the process interesting, and it might help somebody in the
same situation.
I started with an fsck, which found a problem with exactly one object
(I've used $pack and $obj below to keep the output readable, and also
because I'll refer to them later):
$ git fsck
error: $pack SHA1 checksum mismatch
error: index CRC mismatch for object $obj from $pack at offset 51653873
error: inflate: data stream error (incorrect data check)
error: cannot unpack $obj from $pack at offset 51653873
The pack checksum failing means a byte is munged somewhere, and it is
presumably in the object mentioned (since both the index checksum and
zlib were failing).
Reading the zlib source code, I found that incorrect data check means
that the adler-32 checksum at the end of the zlib data did not match the
inflated data. So stepping the data through zlib would not help, as it
did not fail until the very end, when we realize the crc does not match.
The problematic bytes could be anywhere in the object data.
The first thing I did was pull the broken data out of the packfile. I
needed to know how big the object was, which I found out with:
$ git show-index $idx | cut -d' ' -f1 | sort -n | grep -A1 51653873
51653873
51664736
Show-index gives us the list of objects and their offsets. We throw away
everything but the offsets, and then sort them so that our interesting
offset (which we got from the fsck output above) is followed immediately
by the offset of the next object. Now we know that the object data is
10863 bytes long, and we can grab it with:
dd if=$pack of=object bs=1 skip=51653873 count=10863
I inspected a hexdump of the data, looking for any obvious bogosity
(e.g., a 4K run of zeroes would be a good sign of filesystem
corruption). But everything looked pretty reasonable.
Note that the object file isn't fit for feeding straight to zlib; it
has the git packed object header, which is variable-length. We want to
strip that off so we can start playing with the zlib data directly. You
can either work your way through it manually (the format is described in
Documentation/technical/pack-format.txt), or you can walk through it in
a debugger. I did the latter, creating a valid pack like:
# pack magic and version
printf 'PACK\0\0\0\2' tmp.pack
# pack has one object
printf '\0\0\0\1' tmp.pack
# now add our object data
cat object tmp.pack
# and then append the pack trailer
/path/to/git.git/test-sha1 -b tmp.pack trailer
cat trailer tmp.pack
and then running git index-pack tmp.pack in the debugger (stop at
unpack_raw_entry). Doing this, I found that there were 3 bytes of header
(and the header itself had a sane type and size). So I stripped those
off with:
dd if=object of=zlib bs=1 skip=3
I ran the result through zlib's inflate using a custom C program. And
while it did report the error, I did get the right number of output
bytes (i.e., it matched git's size header that we decoded above). But
feeding the result back to git hash-object didn't produce the same
sha1. So there were some wrong bytes, but I didn't know which. The file
happened to be C source code, so I hoped I could notice something
obviously wrong with it, but I didn't. I even got it to compile!
I also tried comparing it to other versions of the same path in the
repository, hoping that there would be some part of the diff that didn't
make sense. Unfortunately, this happened to be the only revision of this
particular file in the repository, so I had nothing to compare against.
So I took a different approach. Working under the guess that the
corruption was limited to a single byte, I wrote a program to munge each
byte individually, and try inflating the result. Since the object was
only 10K compressed, that worked out to about 2.5M attempts, which took
a few minutes.
The program I used is here:
-- 8 --
#include stdio.h
#include unistd.h
#include string.h
#include signal.h
#include zlib.h
static int try_zlib(unsigned char *buf, int len)
{
/* make this absurdly large so we don't have to loop */
static unsigned char out[1024*1024];
z_stream z;
int ret;
memset(z, 0, sizeof(z));
inflateInit(z);
z.next_in = buf;
z.avail_in = len;
z.next_out = out;
z.avail_out = sizeof(out);
ret = inflate(z, 0);
inflateEnd(z);
return ret = 0;
}
/* eye candy */
static int counter = 0;
static void progress(int sig)
{
fprintf(stderr, \r%d, counter);
alarm(1);
}
int main(void)
{
/* oversized so we can read the whole buffer in */
unsigned char buf[1024*1024];
int len;
unsigned i, j;
signal(SIGALRM, progress);
alarm(1);
len = read(0, buf, sizeof(buf));
for (i = 0; i len; i++) {
unsigned char c = buf[i];
for (j = 0; j = 0xff; j++) {
buf[i] = j;
counter++;
Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments
On Wed, Oct 16, 2013 at 09:04:32AM +0200, Johannes Sixt wrote: Yes, another possibility in that vein would be to teach rev-parse --parseopt an OPTIONS_LONG_STICKED output format, and then parse with Aren't you people trying to solve something that can't besolved? What does git commit -S LICENSE mean? Commit the index and sign with the key-id LICENSE or commit just the file LICENSE with the default key-id? It means the latter. Because the argument is optional, you must use the sticked form: git commit -SLICENSE If the caller does not know whether the argument is optional or not, they should use the sticked form to be on the safe side. The problem, as I understand it, arises from the fact that shell scripts can use git rev-parse --parseopt to check and normalize their arguments. So for example: # pretend we got -bs on our command line set -- -bs git rev-parse --parseopt -- $@ \EOF usage... -- b!the b option s!the s option EOF would produce: set -- -b -s -- The latter is much easier to parse in the shell, because options are split, it ends with --, etc. But what is the normalized form for an optional argument? It either needs to be consistently sticked or unsticked, either: set -- -S '' -- ;# default set -- -S 'foo' -- ;# not default or set -- -S --;# default set -- -Sfoo -- ;# not default so that reading the normalized form is unambiguous. -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: pack corruption post-mortem
On Wed, Oct 16, 2013 at 3:34 PM, Jeff King p...@peff.net wrote: I was recently presented with a repository with a corrupted packfile, and was asked if the data was recoverable. This post-mortem describes the steps I took to investigate and fix the problem. I thought others might find the process interesting, and it might help somebody in the same situation. It's like reading an LWN article. Thank you. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.
Hello Junio, Keshav
Thank you very much for your detailed reviewing.
I just tried to update the patch and posted as [PATCH v6].
---
Tsuneo Yoshioka (吉岡 恒夫)
yoshiokatsu...@gmail.com
On Oct 16, 2013, at 1:54 AM, Junio C Hamano gits...@pobox.com wrote:
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 +
[PATCH v6] 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.
Before this commit, this output is shortened always by omitting left most
part like ...foo = barbarbar. So, if the destination filename is too long,
source filename putting left or arrow can be totally omitted like
...barbarbar, without including any of foofoofoo =.
In such a case where arrow symbol is omitted, there is no way to know
whether the file is renamed or existed in the original.
Make sure there is always an arrow, like ...foo = ...bar.
The output can contain curly braces('{','}') for grouping.
So, in general, the output 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 equally important.
Signed-off-by: Tsuneo Yoshioka yoshiokatsu...@gmail.com
Test-added-by: Thomas Rast tr...@inf.ethz.ch
---
diff.c | 180 +++--
t/t4001-diff-rename.sh | 12
2 files changed, 170 insertions(+), 22 deletions(-)
diff --git a/diff.c b/diff.c
index a04a34d..afe6a36 100644
--- a/diff.c
+++ b/diff.c
@@ -1258,11 +1258,13 @@ static void fn_out_consume(void *priv, char *line,
unsigned long len)
}
}
-static char *pprint_rename(const char *a, const char *b)
+static void 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 +1274,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 */
@@ -1322,17 +1323,142 @@ static char *pprint_rename(const char *a, const char
*b)
if (b_midlen 0)
b_midlen = 0;
- strbuf_grow(name, pfx_length + a_midlen + b_midlen + sfx_length + 7);
- if (pfx_length + sfx_length) {
- strbuf_add(name, a, pfx_length);
+ 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);
+}
+
+static int compare_size_t_descending_order(const void *left, const void *right)
+{
+ size_t left_val = *(size_t *)left;
+ size_t right_val = *(size_t *)right;
+ return (int)(right_val - left_val);
+}
+
+/*
+ * 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 rename_omit(struct strbuf *pfx,
+ struct strbuf *a_mid, struct strbuf *b_mid,
+ struct strbuf *sfx,
+ int name_width)
+{
+ static const char arrow[] = = ;
+ static const char dots[] = ...;
+ int use_curly_braces = (pfx-len 0) || (sfx-len 0);
+ size_t name_len;
+ size_t len;
+ size_t part_length[3];
+ size_t max_part_len = 0;
+ size_t remainder_part_len = 0;
+
+ 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(dots) + (name_len - pfx-len) = name_width) {
+ /*
+* Just omitting left of '{' is enough
+* Ex: ...aaa{foofoofoo = bar}file
+*/
+ strbuf_splice(pfx, 0, name_len - name_width +
strlen(dots), dots, strlen(dots));
+ return;
+ } else {
+ if (pfx-len strlen(dots)) {
+ /*
+* Just omitting left of '{' is not
Re: [PATCH v3] Add core.mode configuration
On Tue, Oct 15, 2013 at 11:55 PM, Felipe Contreras felipe.contre...@gmail.com wrote: [snip] We cannot change the behavior of push.default = simple already, so at least that option is not in question. True. Presumably you are worried about the other options that can't be enabled in any way. Yes. 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. My concern is that people don't treat it for what it is--a way to experiment with the new behaviors--and then they get upset if we discover that some behavior was not well thought out and it disappears unexpectedly when we correct the matter. We have a balance to strike: annoying users and getting some miles on the new behaviors. I see the technical end of this--your proposal to have a 'core.mode'--but where is the non-technical end of this argument? What message are we proposing to send to the users? What's our promise to them surrounding core.mode and the new behaviors it offers? Perhaps we don't have much today that this affects, but what about tomorrow? Are we saying that behaviors enabled by core.mode=next are concrete (they're going in as-is, and we won't alter their behavior before 2.0)? As I said, the only real drawback is that I see this as the latter, because any other choice means users will get annoyed when something changes out from under them. 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. You're right, I wasn't around for that. And on the whole, I absolutely agree: it's nice to get miles on these new behaviors/features/etc. I just worry that having an option like this means we've committed to it, and I'm not sure that we want to give up the ability to change them without having to go through some sort of deprecation cycle. Or worse, we have to wait until 3.0 and 2.0 hasn't even come out yet. I hope others chime in here. And don't mistake me as dissenting; I'm not. And, I'm not assenting either. I just want to know if you've thought about what this means to users, and what we're prepared to deal with. Right now, I feel like half the argument around the option is missing. So I say we shouldn't just provide warnings, but also have an option to allow users (probably a minority) to start testing this. probably a minority -- I guess that's the part I disagree with. I'm not sure what a minority means here, but I don't think it'll be a handful of people. How big does that number get before we get concerned about backlash from users if we decide to change course? Or, is that simply not an issue? Why or why not? I have to be honest, if the option was available, I'd have my developers turn it on. I'm sure a great deal of others would do so too. Is there some other way we can solve this? Having an experimental branch with all the 2.0 features merged and those concerned can just build that version? I see the downside of that too: it's not as easy for people to try, and there is nothing preventing folks from posting binaries with the new behaviors enabled. It leads me to feeling that we're stuck in some regard. But maybe I'm being overly pessimistic here, and it's really all a non-issue. As I said earlier, it'd be nice if others chimed in here. -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] rev-parse --parseopt: fix handling of optional arguments
On Wed, 16 Oct 2013, Johannes Sixt wrote: Am 10/16/2013 1:57, schrieb Jonathan Nieder: 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 Aren't you people trying to solve something that can't besolved? What does git commit -S LICENSE mean? Commit the index and sign with the key-id LICENSE or commit just the file LICENSE with the default key-id? The later, as optional arguments needs to be sticked, but I think this is not related to the problems with --parseopt. Here is a summary the problems I think we have with --parseopt and proposed solutions. This command makes it possible to parse arguments with a loop like this : while test $# != 0 do case $1 in -q) GIT_QUIET=t ;; -S) # do something with $2 # shift if you think $2 is an optional arg ;; --) shift; break ;; esac shift done # do something with the other arguments And I think the problems with the '?' flag when using this kind of loop are : - You don't know if $2 is the optional argument for -S, or the next option - You can't use '--' as an optional argument to -S, because you don't know if '--' is the optional argument to -S, or the separator between options and non options. To fix this, solution 1) is to always include the optional argument in $2, and set it to '' if it is not set. However this brings the problem that you can't distinguish between unset argument and empty argument. The following two become the same : ./opt.sh --gpg-id ./opt.sh --gpg-id= Solution 2) is to use a flag like ?default as suggested by Jonathan. Now you can distinguish between unset and empty args, but you can't between default and unset, but this is probably not a big problem as you can select default so that it is a value nobody would want to use. Solution 3) is the OPTIONS_LONG_STICKED output format suggested by Jonathan. I can't see any problem with this one. -- To unsubscribe from this list: send the line 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] t4254: modernize tests
- Don't start tests with 'test $? = 0' ;) - Move writing the bogus patch and the expected output into the appropriate test_expect_success blocks. - Use the test_must_fail helper instead of manually checking for non-zero exit code. - Use the debug-friendly test_path_is_file helper instead of 'test -f'. - No space after ''. Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- Is the exact match of 'git am's exit code important? I think it doesn't matter as long as it fails, hence test_must_fail instead of test_expect_code 1. t/t4254-am-corrupt.sh | 36 +++- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh index b7da95fac5..85716dd6ec 100755 --- a/t/t4254-am-corrupt.sh +++ b/t/t4254-am-corrupt.sh @@ -3,20 +3,19 @@ test_description='git am with corrupt input' . ./test-lib.sh -# Note the missing +++ line: -cat bad-patch.diff 'EOF' -From: A U Thor au.t...@example.com -diff --git a/f b/f -index 7898192..6178079 100644 a/f -@@ -1 +1 @@ --a -+b -EOF - test_expect_success setup ' - test $? = 0 - echo a f + # Note the missing +++ line: + cat bad-patch.diff -\EOF + From: A U Thor au.t...@example.com + diff --git a/f b/f + index 7898192..6178079 100644 + --- a/f + @@ -1 +1 @@ + -a + +b + EOF + + echo a f git add f test_tick git commit -m initial @@ -26,17 +25,12 @@ test_expect_success setup ' # fatal: unable to write file '(null)' mode 100644: Bad address # Also, it had the unwanted side-effect of deleting f. test_expect_success 'try to apply corrupted patch' ' - git am bad-patch.diff 2 actual - test $? = 1 + test_must_fail git am bad-patch.diff 2actual ' -cat expected EOF -fatal: git diff header lacks filename information (line 4) -EOF - test_expect_success 'compare diagnostic; ensure file is still here' ' - test $? = 0 - test -f f + echo fatal: git diff header lacks filename information (line 4) expected + test_path_is_file f test_cmp expected actual ' -- 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] rev-parse --parseopt: fix handling of optional arguments
On Tue, 15 Oct 2013, Jonathan Nieder wrote:
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.
I'm thinking about a patch to add the following two options to rev-parse :
--sticked-opt-args::
Only meaningful in --parseopt mode. Tells the options parser to
output options with optional arguments in sticked form. The
default is to output them in non-sticked mode, which can be
difficult to parse unambiguously.
--long-options::
Only meaningful in --parseopt mode. Tells the options parser to
output long option names, when available. The default is to use
short option names when available.
When you want to handle optional args unambiguously, you use the
--sticked-opt-args option. And if you think an empty value can be
a meaningful value, you add the --long-options option to be able to
distinguish them.
Would it make sense ?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Add core.mode configuration
On Wed, Oct 16, 2013 at 6:54 AM, John Szakmeister j...@szakmeister.net wrote: [snip] probably a minority -- I guess that's the part I disagree with. I'm not sure what a minority means here, but I don't think it'll be a handful of people. How big does that number get before we get concerned about backlash from users if we decide to change course? Or, is that simply not an issue? Why or why not? I have to be honest, if the option was available, I'd have my developers turn it on. I'm sure a great deal of others would do so too. Is there some other way we can solve this? Having an experimental branch with all the 2.0 features merged and those concerned can just build that version? I see the downside of that too: it's not as easy for people to try, and there is nothing preventing folks from posting binaries with the new behaviors enabled. It leads me to feeling that we're stuck in some regard. But maybe I'm being overly pessimistic here, and it's really all a non-issue. As I said earlier, it'd be nice if others chimed in here. Thinking about this a little more, we do have a proving ground. That's what the whole pu/next/master construct is for. So maybe this is a non-issue. By the time it lands on master, we should have decided whether the feature is worth keeping or not. -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: pack corruption post-mortem
On Wednesday, October 16, 2013 02:34:01 am Jeff King wrote: I was recently presented with a repository with a corrupted packfile, and was asked if the data was recoverable. This post-mortem describes the steps I took to investigate and fix the problem. I thought others might find the process interesting, and it might help somebody in the same situation. This is awesome Peff, thanks for the great writeup! I have nightmares about this sort of thing every now and then, and we even experience some corruption here and there that needs to be fixed (mainly missing objects when we toy with different git repack arguments). I cannot help but wonder, how we can improve git further to either help diagnose or even fix some of these problems? More inline below... The first thing I did was pull the broken data out of the packfile. I needed to know how big the object was, which I found out with: $ git show-index $idx | cut -d' ' -f1 | sort -n | grep -A1 51653873 51653873 51664736 Show-index gives us the list of objects and their offsets. We throw away everything but the offsets, and then sort them so that our interesting offset (which we got from the fsck output above) is followed immediately by the offset of the next object. Now we know that the object data is 10863 bytes long, and we can grab it with: dd if=$pack of=object bs=1 skip=51653873 count=10863 Is there a current plumbing command that should be enhanced to be able to do the 2 steps above directly for people debugging (maybe with some new switch)? If not, should we create one, git show --zlib, or git cat-file --zlib? Note that the object file isn't fit for feeding straight to zlib; it has the git packed object header, which is variable-length. We want to strip that off so we can start playing with the zlib data directly. You can either work your way through it manually (the format is described in Documentation/technical/pack-format.txt), or you can walk through it in a debugger. I did the latter, creating a valid pack like: # pack magic and version printf 'PACK\0\0\0\2' tmp.pack # pack has one object printf '\0\0\0\1' tmp.pack # now add our object data cat object tmp.pack # and then append the pack trailer /path/to/git.git/test-sha1 -b tmp.pack trailer cat trailer tmp.pack and then running git index-pack tmp.pack in the debugger (stop at unpack_raw_entry). Doing this, I found that there were 3 bytes of header (and the header itself had a sane type and size). So I stripped those off with: dd if=object of=zlib bs=1 skip=3 This too feels like something we should be able to do with a plumbing command eventually? git zlib-extract So I took a different approach. Working under the guess that the corruption was limited to a single byte, I wrote a program to munge each byte individually, and try inflating the result. Since the object was only 10K compressed, that worked out to about 2.5M attempts, which took a few minutes. Awesome! Would this make a good new plumbing command, git zlib-fix? I fixed the packfile itself with: chmod +w $pack printf '\xc7' | dd of=$pack bs=1 seek=51659518 conv=notrunc chmod -w $pack The '\xc7' comes from the replacement byte our munge program found. The offset 51659518 is derived by taking the original object offset (51653873), adding the replacement offset found by munge (5642), and then adding back in the 3 bytes of git header we stripped. Another plumbing command needed? git pack-put --zlib? I am not saying my command suggestions are good, but maybe they will inspire the right answer? -Martin -- To unsubscribe from this list: send the line 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 22:05, schrieb Jonathan Nieder: 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. Ok, that makes sense. 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. :) But we don't want to wait for 3.0, no? ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git tag output order is incorrect (IMHO)
On Sun, Sep 08, 2013 at 04:03:11AM -0400, Jeff King wrote: I have some free time to come, and would like to work on that feature. Does the offer still hold ? If it does, I would be interested in your patches. I'm sorry I have taken so long to get back to you on this. I was hoping to revisit the topic and make sure the patches were in a sensible state for showing to somebody. But it took me some time to get around to it, and now that I have, they're really not looking very good. Hi Antoine, Since I haven't heard anything, I assume you haven't been working on this. But in case you have, I wanted to let you know I found some time and moved the topic forward a bit. It's not quite ready to share with the list, but I wanted to notify you so we didn't duplicate effort. -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: Git tag output order is incorrect (IMHO)
On Wed, Oct 16, 2013 at 7:56 PM, Jeff King p...@peff.net wrote: On Sun, Sep 08, 2013 at 04:03:11AM -0400, Jeff King wrote: I have some free time to come, and would like to work on that feature. Does the offer still hold ? If it does, I would be interested in your patches. I'm sorry I have taken so long to get back to you on this. I was hoping to revisit the topic and make sure the patches were in a sensible state for showing to somebody. But it took me some time to get around to it, and now that I have, they're really not looking very good. Hi Antoine, Since I haven't heard anything, I assume you haven't been working on this. But in case you have, I wanted to let you know I found some time and moved the topic forward a bit. It's not quite ready to share with the list, but I wanted to notify you so we didn't duplicate effort. Unfortunately, I didn't have as much time as expected to work on this topic. I'm glad to hear that it's moving forward and will definitely have a look when you send the patches to the list. Thank you for letting me know, Cheers, Antoine -- To unsubscribe from this list: send the line 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] rebase: use reflog to find common base with upstream
Commit 15a147e (rebase: use @{upstream} if no upstream specified,
2011-02-09) says:
Make it default to 'git rebase @{upstream}'. That is also what
'git pull [--rebase]' defaults to, so it only makes sense that
'git rebase' defaults to the same thing.
but that isn't actually the case. Since commit d44e712 (pull: support
rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually
chosen the most recent reflog entry which is an ancestor of the current
branch if it can find one.
Change rebase so that it uses the same logic.
Signed-off-by: John Keeping j...@keeping.me.uk
---
git-rebase.sh | 8
t/t3400-rebase.sh | 6 --
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index 226752f..fd36cf7 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -437,6 +437,14 @@ then
error_on_missing_default_upstream rebase rebase \
against git rebase branch
fi
+ for reflog in $(git rev-list -g $upstream_name 2/dev/null)
+ do
+ if test $reflog = $(git merge-base $reflog HEAD)
+ then
+ upstream_name=$reflog
+ break
+ fi
+ done
;;
*) upstream_name=$1
shift
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index ebf93b0..4f45f7e 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -134,12 +134,14 @@ test_expect_success 'fail when upstream arg is missing
and not configured' '
test_must_fail git rebase
'
-test_expect_success 'default to @{upstream} when upstream arg is missing' '
+test_expect_success 'default to common base in @{upstream}s reflog if no
upstream arg' '
git checkout -b default topic
git config branch.default.remote .
git config branch.default.merge refs/heads/master
git rebase
- test $(git rev-parse default~1) = $(git rev-parse master)
+ git merge-base topic master expect
+ git rev-parse default~1 actual
+ test_cmp expect actual
'
test_expect_success 'rebase -q is quiet' '
--
1.8.4.566.g73d370b
--
To unsubscribe from this list: send the line 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] rebase: use reflog to find common base with upstream
Hi, John Keeping wrote: Since commit d44e712 (pull: support rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually chosen the most recent reflog entry which is an ancestor of the current branch if it can find one. Change rebase so that it uses the same logic. Nice idea. Could pull be made to rely on rebase for this as a follow-up? [...] --- a/git-rebase.sh +++ b/git-rebase.sh @@ -437,6 +437,14 @@ then error_on_missing_default_upstream rebase rebase \ against git rebase branch fi + for reflog in $(git rev-list -g $upstream_name 2/dev/null) + do + if test $reflog = $(git merge-base $reflog HEAD) git merge-base --is-ancestor is faster. What should happen if HEAD is not a valid commit? (Tested with: $ git checkout --orphan foo $ cat .git/config EOF [branch foo] remote = origin merge = refs/heads/master EOF $ bin-wrappers/git rebase 21 | wc -l 83 ). diff --git i/git-rebase.sh w/git-rebase.sh index fd36cf7..d2e2c2e 100755 --- i/git-rebase.sh +++ w/git-rebase.sh @@ -439,7 +439,7 @@ then fi for reflog in $(git rev-list -g $upstream_name 2/dev/null) do - if test $reflog = $(git merge-base $reflog HEAD) + if git merge-base --is-ancestor $reflog HEAD then upstream_name=$reflog break -- To unsubscribe from this list: send the line 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 11:03:26PM -0500, Felipe Contreras wrote: 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? Yes, I think we both agreed on adding core.addremove. I'm just not convinced if we should also add core.mode. If we add core.addremove, all the issues you mentioned are solved. If we do that, now the question is, how exactly does core.mode = next affect anybody genatively? If you don't like it, you don't set it, that's why it's a configuration. I don't see the problem. 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. Yes, if there are people that will use core.mode it will be worth adding. I'm just not one of them. I am already using it. -- 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 10:55:07PM -0500, Felipe Contreras wrote: John Szakmeister wrote: 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. If we add core.addremove=true the same applies to it - we cannot remove it later, the only we can do is to disable it by default in future versions after testing (core.addremove=true or core.mode=next). That is true, but adding core.addremove = true would probably imply there's the option of adding core.addremove = false. 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. BTW, I'm still using pre-1.6 git-foo, I have /usr/libexec/git-core in my PATH. So I would like to always have an option to disable some new incompatible improvements. That's what core.addremove = false would do, wouldn't it? So I say we shouldn't just provide warnings, but also have an option to allow users (probably a minority) to start testing this. and an option to keep the old behavior, like we did with push.default. Ditto. -- 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] rebase: use reflog to find common base with upstream
On Wed, Oct 16, 2013 at 12:24:13PM -0700, Jonathan Nieder wrote: John Keeping wrote: Since commit d44e712 (pull: support rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually chosen the most recent reflog entry which is an ancestor of the current branch if it can find one. Change rebase so that it uses the same logic. Nice idea. Could pull be made to rely on rebase for this as a follow-up? It's not an obvious change to do that (at least to me) - pull does the different steps of figuring out the base and then rebasing at different points in the script. [...] --- a/git-rebase.sh +++ b/git-rebase.sh @@ -437,6 +437,14 @@ then error_on_missing_default_upstream rebase rebase \ against git rebase branch fi + for reflog in $(git rev-list -g $upstream_name 2/dev/null) + do + if test $reflog = $(git merge-base $reflog HEAD) git merge-base --is-ancestor is faster. We should probably change git-pull to use that as well. What should happen if HEAD is not a valid commit? (Tested with: $ git checkout --orphan foo $ cat .git/config EOF [branch foo] remote = origin merge = refs/heads/master EOF $ bin-wrappers/git rebase 21 | wc -l 83 ). Adding 2/dev/null to the merge-base line silences most of that, all we're left with is fatal: Needed a single revision which is the same as I get from master's git-rebase. I think silencing stderr is the right thing to do here - in the worst case we just use the merge-base of the two branches instead of the appropriate reflog entry. diff --git i/git-rebase.sh w/git-rebase.sh index fd36cf7..d2e2c2e 100755 --- i/git-rebase.sh +++ w/git-rebase.sh @@ -439,7 +439,7 @@ then fi for reflog in $(git rev-list -g $upstream_name 2/dev/null) do - if test $reflog = $(git merge-base $reflog HEAD) + if git merge-base --is-ancestor $reflog HEAD then upstream_name=$reflog break -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pull and fetch don't honor `--progress` flag
When I use the `--progress` flag with the push command, I get transfer-speed statistics like this: $ git push -progress origin master 21 | tee /tmp/push Counting objects: 30, done. Compressing objects: 100% (20/20), done. Writing objects: 100% (30/30), 9.02 MiB | 206.00 KiB/s, done. Total 30 (delta 0), reused 0 (delta 0) This also works similarly with clone: $ git clone --progress $url foo.git 21 | tee /tmp/clone Cloning into 'foo.git'... remote: Counting objects: 61, done. remote: Compressing objects: 100% (43/43), done. remote: Total 61 (delta 3), reused 0 (delta 0) Receiving objects: 100% (61/61), 15.22 MiB | 473.00 KiB/s, done. Resolving deltas: 100% (3/3), done. Checking connectivity... done However, even though pull and fetch also have the same flag documented, git never reports any network statistics at all. For example: $ git pull --progress origin master 21 | tee /tmp/pull remote: Counting objects: 5, done. remote: Compressing objects: 100% (3/3), done. remote: Total 3 (delta 1), reused 0 (delta 0) This is repeatable with both Git 1.7.9 and Git 1.8.4.1 running under Cygwin. Is this a bug? If not, how can I make fetch and pull cough up throughput statistics? -- To unsubscribe from this list: send the line 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 11:55 PM, Felipe Contreras felipe.contre...@gmail.com wrote: [snip] We cannot change the behavior of push.default = simple already, so at least that option is not in question. True. Presumably you are worried about the other options that can't be enabled in any way. Yes. 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. My concern is that people don't treat it for what it is--a way to experiment with the new behaviors--and then they get upset if we discover that some behavior was not well thought out and it disappears unexpectedly when we correct the matter. Yes, but that's like removing the --force option in rm, because the user might unexpectedly remove files (s)he didn't intend to. If the user uses rm --force, the user must know what (s)he is doing, that's just the way it is. Similarly, if a user does core.mode = next, the user is expecting to enable all future behaviors, because that's what core.mode = next does, if he doesn't want to do that, then why would he use that option? Particularily because there's push.default, and there would be core.addremove, and everything that core.mode = next does would be possible to do in other ways that are not going to introduce new behavior as the version of Git advances. We have a balance to strike: annoying users and getting some miles on the new behaviors. I see the technical end of this--your proposal to have a 'core.mode'--but where is the non-technical end of this argument? What message are we proposing to send to the users? What's our promise to them surrounding core.mode and the new behaviors it offers? Perhaps we don't have much today that this affects, but what about tomorrow? Are we saying that behaviors enabled by core.mode=next are concrete (they're going in as-is, and we won't alter their behavior before 2.0)? I'd say we make it explicit that if you turn on core.mode = next, the behavior you experience is going to change from version to version. In other words, there is no backwards compatibility promise for the behaviors core.mode = next enables. In a way it's kind of experimental, except that it's very likely the new behavior won't be reverted back, just not 100% sure. As I said, the only real drawback is that I see this as the latter, because any other choice means users will get annoyed when something changes out from under them. If the user doesn't want things to change dramatically, the user shouldn't use core.mode = next. 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. You're right, I wasn't around for that. And on the whole, I absolutely agree: it's nice to get miles on these new behaviors/features/etc. I just worry that having an option like this means we've committed to it, It doesn't meant we are committed to it, because the behaviors in core.mode = next have no promise to stay. Either way, these behaviors have been announce in each Git release for several releases, and we are already warning the users that things will change in v2.0. I'd say that already means we've committed to it. and I'm not sure that we want to give up the ability to change them without having to go through some sort of deprecation cycle. Or worse, we have to wait until 3.0 and 2.0 hasn't even come out yet. I don't see why we would be giving up that ability. I hope others chime in here. And don't mistake me as dissenting; I'm not. And, I'm not assenting either. I just want to know if you've thought about what this means to users, and what we're prepared to deal with. Right now, I feel like half the argument around the option is missing. Of course I've thought about that, otherwise I wouldn't have sent the patch. But I have no hope of others chiming in. So I say we shouldn't just provide warnings, but also have an option to allow users (probably a minority) to start testing this. probably a minority -- I guess that's the part I disagree with. How many people do you think want to start testing v2.0 behaviors? How many people do you think will enable core.mode = next? I'd say the people that test release candidates are the minority, and I'd say the wants that would turn on core.mode = next would be even less. I'm not sure what a minority means here, but I don't think
Re: [PATCH v3] Add core.mode configuration
John Szakmeister wrote: On Wed, Oct 16, 2013 at 6:54 AM, John Szakmeister j...@szakmeister.net wrote: [snip] probably a minority -- I guess that's the part I disagree with. I'm not sure what a minority means here, but I don't think it'll be a handful of people. How big does that number get before we get concerned about backlash from users if we decide to change course? Or, is that simply not an issue? Why or why not? I have to be honest, if the option was available, I'd have my developers turn it on. I'm sure a great deal of others would do so too. Is there some other way we can solve this? Having an experimental branch with all the 2.0 features merged and those concerned can just build that version? I see the downside of that too: it's not as easy for people to try, and there is nothing preventing folks from posting binaries with the new behaviors enabled. It leads me to feeling that we're stuck in some regard. But maybe I'm being overly pessimistic here, and it's really all a non-issue. As I said earlier, it'd be nice if others chimed in here. Thinking about this a little more, we do have a proving ground. That's what the whole pu/next/master construct is for. No, that's not true. 'next' doesn't contain experimental patches, it contains potentially dangerous one that might benefit from some testing before going to master, but they are certainly not experimental. 'pu' doesn't contain experimental code either, the code in 'pu' has to be feature complete. It might require a few more tunning patches, but it's not experimental, and those branches are not long lived. For example the pack-v4 patches could be merged today, and the people involved could keep working on top of that merge point, but that doesn't happen, because 'pu' is not for experimental stuff. There is no place in the Git repository for pack-v4, because there's no place for experimental patches. So maybe this is a non-issue. By the time it lands on master, we should have decided whether the feature is worth keeping or not. I believe without an experimental branch, many branches would never mature to go into master, or next, or even pu. -- 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
[git-users] Problem using detached worktrees with commands implemented in scripts
In Git, one can set up a repository with a detached worktree, where
the .git directory is not a subdirectory of the top directory of the
work tree.
In general, Git commands on a repository with a detached worktree can
be executed by cd'ing into the directory containing the .git
directory, and executing the Git command there. E.g., git add and
git commit execute as one would expect. (I think they can also be
executed by cd'ing to the worktree and setting GIT_DIR.)
However, this approach does not work with git filter-branch, which
objects with You need to run this command from the toplevel of the
working tree.
I suspect that it does not work with other Git commands that are
implemented with shell scripts. The problem appears to be in the
git-sh-setup script, which is called by the Git shell scripts to set
up the environment and do preliminary tests.
It seems to me that this inconsistency between the script commands and
the binary commands can be fixed by updating git-sh-setup in this way:
--- git-sh-setup.Custom.orig2013-06-20 12:59:45.0 -0400
+++ git-sh-setup2013-10-07 22:34:06.719946134 -0400
@@ -297,14 +297,18 @@
# if we require to be in a git repository.
if test -z $NONGIT_OK
then
- GIT_DIR=$(git rev-parse --git-dir) || exit
+ export GIT_DIR=$(git rev-parse --git-dir) || exit
if [ -z $SUBDIRECTORY_OK ]
then
- test -z $(git rev-parse --show-cdup) || {
- exit=$?
- echo 2 You need to run this command from the
toplevel of the working tree.
- exit $exit
- }
+ cdup=$(git rev-parse --show-cdup)
+ if [ -n $cdup ]
+ then
+ # Current directory is not the toplevel.
+ # Set GIT_DIR to the absolute path of the repository.
+ GIT_DIR=$(cd $GIT_DIR pwd)
+ # cd to the toplevel.
+ cd $cdup
+ fi
fi
test -n $GIT_DIR GIT_DIR=$(cd $GIT_DIR pwd) || {
echo 2 Unable to determine absolute path of git directory
What this change does is, when a command is invoked from a directory
containing a repository with a detached worktree, is to set GIT_DIR to
the directory of the repository, then cd to the top of the worktree.
After that, the script command should work as expected.
I am far from being an expert in Git internals, so I don't know
whether this is the correct approach to take to this problem or not.
Does anyone have any feedback on this?
Dale
--
To unsubscribe from this list: send the line 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: Pull and fetch don't honor `--progress` flag
On Wed, Oct 16, 2013 at 03:50:51PM -0400, Jacobs, Todd wrote: When I use the `--progress` flag with the push command, I get transfer-speed statistics like this: $ git push -progress origin master 21 | tee /tmp/push Counting objects: 30, done. Compressing objects: 100% (20/20), done. Writing objects: 100% (30/30), 9.02 MiB | 206.00 KiB/s, done. Total 30 (delta 0), reused 0 (delta 0) This also works similarly with clone: $ git clone --progress $url foo.git 21 | tee /tmp/clone Cloning into 'foo.git'... remote: Counting objects: 61, done. remote: Compressing objects: 100% (43/43), done. remote: Total 61 (delta 3), reused 0 (delta 0) Receiving objects: 100% (61/61), 15.22 MiB | 473.00 KiB/s, done. Resolving deltas: 100% (3/3), done. Checking connectivity... done However, even though pull and fetch also have the same flag documented, git never reports any network statistics at all. For example: $ git pull --progress origin master 21 | tee /tmp/pull remote: Counting objects: 5, done. remote: Compressing objects: 100% (3/3), done. remote: Total 3 (delta 1), reused 0 (delta 0) This is repeatable with both Git 1.7.9 and Git 1.8.4.1 running under Cygwin. Is this a bug? If not, how can I make fetch and pull cough up throughput statistics? Does it make a difference how you invoke git fetch? From a quick look at the code, git fetch with no remote or refspec should display progress data, but if you specify --all or a remote and refspec then it won't. The following patch (untested) will fix it if that is the case: -- 8 -- diff --git a/builtin/fetch.c b/builtin/fetch.c index bd7a101..487381e 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -952,6 +952,10 @@ static void add_options_to_argv(struct argv_array *argv) argv_array_push(argv, -v); else if (verbosity 0) argv_array_push(argv, -q); + if (progress 0) + argv_array_push(argv, --progress); + else if (progress == 0) + argv_array_push(argv, --no-progress); } -- To unsubscribe from this list: send the line 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: Pull and fetch don't honor `--progress` flag
Does it make a difference how you invoke git fetch? From a quick look at the code, git fetch with no remote or refspec should display progress data, but if you specify --all or a remote and refspec then it won't. Thanks for your prompt response. However, it doesn't make any difference. The behavior remains the same even without specifying a refspec. For example: $ git pull remote: Counting objects: 5, done. remote: Compressing objects: 100% (3/3), done. remote: Total 3 (delta 1), reused 0 (delta 0) Unpacking objects: 100% (3/3), done. $ git fetch remote: Counting objects: 5, done. remote: Compressing objects: 100% (3/3), done. remote: Total 3 (delta 1), reused 0 (delta 0) Unpacking objects: 100% (3/3), done. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] doc: git-foo was obsoleted several years ago
Felipe Contreras felipe.contre...@gmail.com writes: So replace 'git-foo' with 'git foo'. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/git-checkout.txt | 4 ++-- Documentation/git-commit.txt | 4 ++-- Documentation/git-rebase.txt | 4 ++-- Documentation/git-status.txt | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) Running $ head Documentaiton/git-c*.txt shows that others e.g. git-cat-file.txt also begin with the same pattern, i.e. git-dashed-command(section) as the first header, and name section also lists the command in the dashed form, which makes one wonder why only these four are singled-out. It could have been This is merely an RFC to show the kind fo things the submitter thinks we need to do; the real version, if list agrees that it is a good thing to do, would cover a lot more files., but then it would have saved wasted time guessing if the message said so. I recall that I wanted to see this change happen myself long time ago, and suspect that there may have been some reason that prevented us from doing so. I might have found that AsciiDoc back then did not like the input if the headline name git-checkout(1) did not match the filename git-checkout.txt and the command in the NAME section git-checkout, or links linkgit:git-checkout[1] from other pages couldn't have SP there, or something silly like that. Also doesn't our build infrastructure slurp the one-line description git-checkout - Checkout a branch ... from these files and expect a dash to be there? In short, I personally do prefer to see dashless form at the top of the manpage, if it does not break other things, and there may need changes necessary to avoid breaking other things may to files other than these documentation-proper source files. diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index ca118ac..8d98383 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -1,9 +1,9 @@ -git-checkout(1) +git checkout(1) === NAME -git-checkout - Checkout a branch or paths to the working tree +git checkout - Checkout a branch or paths to the working tree SYNOPSIS ... -- To unsubscribe from this list: send the line 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: 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. OK, will reword. 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] gc: remove gc.pid file at end of execution
Jonathan Nieder jrnie...@gmail.com writes: Matthieu Moy wrote: This file isn't really harmful, but isn't useful either, and can create minor annoyance for the user: Would something like the following make sense, to ensure the gc.pid file is always removed on normal exit? Has anything further happened to this discussion? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] checkout: allow dwim for branch creation for git checkout $branch --
Matthieu Moy matthieu@grenoble-inp.fr writes: Duy Nguyen pclo...@gmail.com writes: has_dash_dash is calculated as (argc 1) !strcmp(argv[1], --), so when argc == 1, the has_dash_dash must be zero, the !has_dash_dash is redundant. Yes, but I'd rather not have to read the detailed definition of has_dash_dash to understand the code. With my version, the name of the variable is actually sufficient to understand. But it makes me wonder what we do with git checkout abc def -- xyz. Ouch. Both old and new say ... I'll resend, together with tweaks to the first patch. Did anything further happen to this discussion? Is v3 the version with agreement among the list members I just should pick up? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Feature request: Config option for --no-ignore-removal/--ignore-removal
Matthew Cline m...@nightrealms.com writes: When I try to a plain old git add . when files have been deleted/moved, I get the warning You ran 'git add' with neither '-A (--all)' or '--ignore-removal' There should be some way to put something in ~/.gitconfig to tell git to always choose one or another. The lack of configurability is very much deliberate. Adding such knobs that make basic behaviour of Git different depending on the per-user setting will make it unnecessarily harder to run to help your coworker when she is having problems. git add directory you type in her terminal during the session to help her could work differently from the way you are used to, if we added such a knob. We will not be making that mistake. As the advice message says, git add directory ignores removed files in the directory in the current version, and in Git 2.0, the removals are recorded in the index with such a command, so that add directory records the state of the directory as a whole to the index, which is more consistent. And that works for everybody the same way; the only way to prepare for you not to be negatively affected by the switchover is to train your fingers to say --all or --ignore-removal when the difference in behaviour in the current and future versions matters, hence this advise. -- To unsubscribe from this list: send the line 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
Brandon Casey bca...@nvidia.com writes: 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. Thanks; will requeue. -- To unsubscribe from this list: send the line 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
Jeff King p...@peff.net writes: ... But what is the normalized form for an optional argument? It either needs to be consistently sticked or unsticked, either: set -- -S '' -- ;# default set -- -S 'foo' -- ;# not default or set -- -S --;# default set -- -Sfoo -- ;# not default so that reading the normalized form is unambiguous. The analysis makes sense. Either form do not let you distinguish between the case where the end user wanted to explicitly pass as the optional parameter to -S and the case where she gave -S without any optional parameter, though. Which pretty much agrees with j6t's (and my earlier) comment that there is no way to solve this issue completely, I think. It is an acceptable compromise to use your suggestion as a solution that works for cases other than passing an explicit empty string as an optional parameter, I would say, if the limitation is clearly documented. 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] warn about http server document being too old
Sitaram Chamarty sitar...@gmail.com writes: - describe when it is still applicable - tell people where to go for most normal cases Signed-off-by: Sitaram Chamarty sita...@atc.tcs.com --- ref: http://thread.gmane.org/gmane.comp.version-control.git/159633. Yes it's very old but better late than never. Documentation/howto/setup-git-server-over-http.txt | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/howto/setup-git-server-over-http.txt b/Documentation/howto/setup-git-server-over-http.txt index 7f4943e..90b19a0 100644 --- a/Documentation/howto/setup-git-server-over-http.txt +++ b/Documentation/howto/setup-git-server-over-http.txt @@ -3,6 +3,11 @@ Subject: Setting up a Git repository which can be pushed into and pulled from ov Date: Thu, 10 Aug 2006 22:00:26 +0200 Content-type: text/asciidoc +NOTE: This document is from 2006. A lot has happened since then, and this +document is now relevant mainly if your web host is not CGI capable. + +Almost everyone else should instead look at linkgit:git-http-backend[1]. + How to setup Git server over http = I tend to agree that it is a good idea to have this kind of phrase somewhere in the document, but you cannot place the material above the top-level title. AsciiDoc does not seem to like 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] status: allow branch info color customization
Alexander 'z33ky' Hirsch 1ze...@gmail.com writes:
From: Alexander Hirsch 1ze...@gmail.com
git status -bs (--branch --short) does not seem to allow customization of the
colors for the local and remote branch.
wt-status can use the following colors:
WT_STATUS_CHANGED
WT_STATUS_HEADER
- WT_STATUS_LOCAL_BRANCH
WT_STATUS_NOBRANCH
WT_STATUS_ONBRANCH
- WT_STATUS_REMOTE_BRANCH
WT_STATUS_UNMERGED
WT_STATUS_UNTRACKED
WT_STATUS_UPDATED
but parse_status_slot() lets you set only these
WT_STATUS_CHANGED
WT_STATUS_HEADER
WT_STATUS_NOBRANCH
WT_STATUS_ONBRANCH
WT_STATUS_UNMERGED
WT_STATUS_UNTRACKED
WT_STATUS_UPDATED
and this patch makes the configuration mechanism cover all of the
slots by adding parsing code for the missing two.
While I think the intent of the patch makes sense, I have to wonder
if local and remote without having the branch anywhere is
painting us into an unpleasant corner we cannot later get out
of. For example, we might want to show the remote-tracking branch
information (that is where REMOTE_BRANCH color matters in the
current code) in a more detailed way later, which may include where
the source repository resides, either locally on the same host or
remotely over the network, and WT_STATUS_{LOCAL,REMOTE}_REPOSITORY
may be the natural names for the colors to paint the repository
paths/URL in.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git-users] Problem using detached worktrees with commands implemented in scripts
wor...@alum.mit.edu (Dale R. Worley) writes: In general, Git commands on a repository with a detached worktree can be executed by cd'ing into the directory containing the .git directory, ... Eh? News to me; it might happened to have appeared to work by accident, but that is not by design. IIRC, the intended use pattern (i.e. the change that introduced GIT_DIR and GIT_WORK_TREE environment variables was designed to support) for such a working tree is to: - export GIT_DIR that points at the correct .git directory; - export GIT_WORK_TREE that points at the correct top-level of such a working tree; and then - run the commands anywhere in the working tree, as if you did not export these two environment variables and instead had the .git directory at the usual place in the working tree. It _is_ possible that we may have broken this canonical use pattern over time with more recent updates; I do not think we have extensive test coverage for detached worktree use case in the first place. Does anyone have any feedback on this? Not exporting GIT_DIR variable in sh-setup was done not by accident but as a very deliberate design choice, IIRC. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Oct 2013, #03; Wed, 16)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. I think I correctly inherited all the topics Jonathan kept track of during my absence (big thanks to Jonathan); if a topic that has been in his tree is missing, please holler. I am chewing through the list backlog but still have a long way to go. Please be patient for the rest of the week. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * fc/styles (2013-10-16) 7 commits - block-sha1/sha1.c: have SP around arithmetic operators - base85.c: have SP around arithmetic operators - archive.c: have SP around arithmetic operators - alloc.c: have SP around arithmetic operators - abspath.c: have SP around arithmetic operators - alias: have SP around arithmetic operators - C: have space around and || operators C coding style fixes. The ones near the tip have not been sent to the list yet (they cover the same kind of style violation as the second one) and I may either send them or drop some of them if they turn out to conflict with other work in flight---I still haven't caught up with the backlog and do not know. * jk/remote-literal-string-leakfix (2013-10-15) 1 commit - remote: do not copy origin string literal Will merge to 'next'. * jk/split-broken-ident (2013-10-15) 2 commits - SQUASH??? remove reverse scan to simplify the logic - split_ident: parse timestamp from end of line Make the fall-back parsing of commit objects with broken author or committer lines more robust to pick up the timestamps. Will merge to 'next', perhaps after dropping the top one. * sg/prompt-svn-remote-fix (2013-10-15) 1 commit - bash prompt: don't use '+=' operator in show upstream code path Bash portability fix. Will merge to 'next'. * sc/doc-howto-dumb-http (2013-10-16) 1 commit - doc/howto: warn about (dumb)http server document being too old Will merge to 'next'. * sg/t3600-nul-sha1-fix (2013-10-16) 1 commit - t3600: fix broken choking git rm test Will merge to 'next'. -- [Stalled] * tr/merge-recursive-index-only (2013-07-07) 3 commits - merge-recursive: -Xindex-only to leave worktree unchanged - merge-recursive: untangle double meaning of o-call_depth - merge-recursive: remove dead conditional in update_stages() Holding until there is a caller to learn from. * jc/ref-excludes (2013-09-03) 2 commits - document --exclude option - revision: introduce --exclude=glob to tame wildcards People often wished a way to tell git log --branches (and git log --remotes --not --branches) to exclude some local branches from the expansion of --branches (similarly for --tags, --all and --glob=pattern). Now they have one. Needs a matching change to rev-parse. * rv/send-email-cache-generated-mid (2013-08-21) 2 commits - git-send-email: Cache generated message-ids, use them when prompting - git-send-email: add optional 'choices' parameter to the ask sub * rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits - ### DONTMERGE: needs better explanation on what config they need - pack-refs.c: Add missing call to git_config() - show-ref.c: Add missing call to git_config() The changes themselves are probably good, but it is unclear what basic setting needs to be read for which exact operation. Waiting for clarification. $gmane/228294 * jh/shorten-refname (2013-05-07) 4 commits - t1514: refname shortening is done after dereferencing symbolic refs - shorten_unambiguous_ref(): Fix shortening refs/remotes/origin/HEAD to origin - t1514: Demonstrate failure to correctly shorten refs/remotes/origin/HEAD - t1514: Add tests of shortening refnames in strict/loose mode When remotes/origin/HEAD is not a symbolic ref, rev-parse --abbrev-ref remotes/origin/HEAD ought to show origin, not origin/HEAD, which is fixed with this series (if it is a symbolic ref that points at remotes/origin/something, then it should show origin/something and it already does). Expecting a reroll, as an early part of a larger series. $gmane/225137 * jc/format-patch (2013-04-22) 2 commits - format-patch: --inline-single - format-patch: rename no_inline field A new option to send a single patch to the standard output to be appended at the bottom of a message. I personally have no need for this, but it was easy enough to cobble together. Tests, docs and stripping out more MIMEy stuff are left as exercises to interested parties. * jk/gitweb-utf8 (2013-04-08) 4 commits - gitweb: Fix broken blob action parameters on blob/commitdiff pages - gitweb: Don't append ';js=(0|1)' to external links - gitweb: Make feed title valid utf8 - gitweb: Fix utf8 encoding for blob_plain, blobdiff_plain, commitdiff_plain,
Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments
On Wed, Oct 16, 2013 at 02:40:07PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: ... But what is the normalized form for an optional argument? It either needs to be consistently sticked or unsticked, either: set -- -S '' -- ;# default set -- -S 'foo' -- ;# not default or set -- -S --;# default set -- -Sfoo -- ;# not default so that reading the normalized form is unambiguous. The analysis makes sense. Either form do not let you distinguish between the case where the end user wanted to explicitly pass as the optional parameter to -S and the case where she gave -S without any optional parameter, though. I almost mentioned that, but I am not sure that it matters. Keep in mind that: git my-script -S foo already does not involve foo with S, because it is not sticked. So there is no way for the _user_ to distinguish between I want the default and I passed you an empty string; because the argument must be sticked they both look like -S. And that distinction is already impossible in the definition of optional arguments, and is not a problem with going through the git rev-parse --parseopt channel at all. So the only bug is the ambiguity in the current normalized form. Of the two forms above, I think I prefer: set -- -S '' -- because it more closely matches the non-optional form we produce today, and because it is slightly less work to parse (you can check that $1 is -S, and then store or check $2, rather than having to match -S* and parse off the beginning). Which pretty much agrees with j6t's (and my earlier) comment that there is no way to solve this issue completely, I think. I guess it depends on what the issue is. :) No, I do not think you can ever fix the options to let those two cases be distinguishable. But I do not think anybody is really asking for that; the real concern is that the rev-parse --parseopt normalization is ambiguous, and that is easily fixable. -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
Has Git 2.0 started to be integrated?
There has been plenty of comments lately about how certain features will be released in 2.0 Have these features been tied together anywhere yet? If not, when might such an integration branch be created? Would be very interested in seeing how Git 2.0 plays, even in these early days. Regards, Andrew Ardill -- To unsubscribe from this list: send the line 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
From: Felipe Contreras felipe.contre...@gmail.com John Szakmeister wrote: On Tue, Oct 15, 2013 at 11:55 PM, Felipe Contreras felipe.contre...@gmail.com wrote: [snip] Similarly, if a user does core.mode = next, the user is expecting to enable all future behaviors, because that's what core.mode = next does, if he doesn't want to do that, then why would he use that option? Would this be a good time to suggest a specific wording should be proposed (or a reminder of what was proposed repeated) for the documentation of this option. It will be the documentation that users will refer to when they need to know, rather than the list discussions. The too and fro discussion suggested that it would be important to present the chosen viewpoint well, so there would be no misunderstanding, such that 'users' of the mode realise that they are acting as testers, and there are no promises for the posterity of any trial behaviour, and they (the tester) have a 'caveat emptor' responsibility. And that they need to keep up with developments (list release notes) so that at any update they know what will disappear and appear without warning. Philip -- 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: Has Git 2.0 started to be integrated?
Hi Andrew, Andrew Ardill wrote: There has been plenty of comments lately about how certain features will be released in 2.0 Have these features been tied together anywhere yet? They're in Junio's jch branch: https://github.com/gitster/git/commits/jch If not, when might such an integration branch be created? Would be very interested in seeing how Git 2.0 plays, even in these early days. I wonder if it would make sense to keep these topics in next even though they will probably not be part of the next release, to encourage people who test that branch to try them out. (Just thinking out loud.) 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: Has Git 2.0 started to be integrated?
On 16 October 2013 15:11, Jonathan Nieder jrnie...@gmail.com wrote: There has been plenty of comments lately about how certain features will be released in 2.0 Have these features been tied together anywhere yet? They're in Junio's jch branch: https://github.com/gitster/git/commits/jch Thanks! I'll build it and have a play tonight. If not, when might such an integration branch be created? Would be very interested in seeing how Git 2.0 plays, even in these early days. I wonder if it would make sense to keep these topics in next even though they will probably not be part of the next release, to encourage people who test that branch to try them out. (Just thinking out loud.) I guess that's my real question; we haven't had a major version release for a long time and I don't know what the cycle will look like. Will be interesting to see how it progresses. Regards, Andrew Ardill -- To unsubscribe from this list: send the line 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: I'm thinking about a patch to add the following two options to rev-parse : --sticked-opt-args:: Only meaningful in --parseopt mode. Tells the options parser to output options with optional arguments in sticked form. The default is to output them in non-sticked mode, which can be difficult to parse unambiguously. --long-options:: Only meaningful in --parseopt mode. Tells the options parser to output long option names, when available. The default is to use short option names when available. When you want to handle optional args unambiguously, you use the --sticked-opt-args option. And if you think an empty value can be a meaningful value, you add the --long-options option to be able to distinguish them. Would it make sense ? That would make four distinct output formats: --sticked --long: Doesn't lose any information that normal use of C parse_options would have kept, as long as every short option with optional argument has a corresponding long option. --sticked --no-long: Loses the distinction between --gpg-sign and --gpg-sign= --no-sticked --long: Semantically equivalent to the existing output, just noisier. --no-sticked --no-long: The existing output. Are all of them needed? Is it worth tempting people to use --sticked --no-long when we know its pitfalls? I would think that only the current normalized form and --sticked --long would need to be supported. The fix you originally proposed seems tolerable to me, too --- it is not very invasive, and while it doesn't distinguish the empty-argument form --gpg-sign=, that's a bit of an edge case. The main reason I slightly prefer the solution that makes the output use long, sticked options on request is that the normalized commandline would start being an actual equivalent command line the command expects, instead of a weird, subtly different syntax. (That problem already exists with or without your patch --- the patch just draws attention to 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: [git-users] Problem using detached worktrees with commands implemented in scripts
From: Junio C Hamano gits...@pobox.com
wor...@alum.mit.edu (Dale R. Worley) writes:
In general, Git commands on a repository with a detached worktree can
be executed by cd'ing into the directory containing the .git
directory, ...
Eh? News to me; it might happened to have appeared to work by
accident, but that is not by design.
I think it is this part in Dale's original email
However, this approach does not work with git filter-branch, which
objects with You need to run this command from the toplevel of the
working tree.
that is the problem Dale has seen. IIRC there are a few commands that do
require to be run from the toplevel ('git bisect' I think is another),
and the detection process for 'toplevel' may not work properly when in a
separated work-tree environment.
Perhaps something to consider.
Philip
IIRC, the intended use pattern (i.e. the change that introduced
GIT_DIR and GIT_WORK_TREE environment variables was designed to
support) for such a working tree is to:
- export GIT_DIR that points at the correct .git directory;
- export GIT_WORK_TREE that points at the correct top-level of such
a working tree; and then
- run the commands anywhere in the working tree, as if you did not
export these two environment variables and instead had the .git
directory at the usual place in the working tree.
It _is_ possible that we may have broken this canonical use pattern
over time with more recent updates; I do not think we have extensive
test coverage for detached worktree use case in the first place.
Does anyone have any feedback on this?
Not exporting GIT_DIR variable in sh-setup was done not by accident
but as a very deliberate design choice, IIRC.
--
--
To unsubscribe from this list: send the line 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
Philip Oakley wrote: Would this be a good time to suggest a specific wording should be proposed (or a reminder of what was proposed repeated) for the documentation of this option. It will be the documentation that users will refer to when they need to know, rather than the list discussions. It's not clear to me that this config item is a good idea. What is the intended use? If someone wants to test that their scripts will continue to work with git 2.0, wouldn't testing a 2.0 release candidate (or the current state of the 'jch' branch until one exists) be the simplest way to do that? If someone just likes the proposed behavior changes and wants to start using them right away, maybe we can help them by releasing 2.0 sooner ;-), or by advertising the fairly simple changes in commandline usage to get the new behaviors: Instead of git add, use git add -A. When using git add -u or git add -A from a subdirectory of the toplevel, specify git add -u . explicitly unless you want it to apply to the whole tree (in which case use git add -u :/). Instead of letting git push guess, name the branch you want to push: git push origin master. Or set '[push] default = simple' in your configuration. Pass --prefix to git svn clone. The downside of configuration like the proposed core.next is that it is hard to explain (What do you mean that I can't roll back to the pre-2.0 behavior in Git 2.0 by setting this configuration setting to an appropriate value?), users or scripts can rely on it, and configuration variables tend to accumulate and never be removed. If we really want a run-time switch for this, I suspect an appropriately named environment variable would work better, since we have a history of being able to remove those without alarming people. My two cents, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git-users] Problem using detached worktrees with commands implemented in scripts
Philip Oakley philipoak...@iee.org writes: ... and the detection process for 'toplevel' may not work properly when in a separated work-tree environment. Without GIT_WORK_TREE exported to point at the top-level, there is nothing that lets us detect it, as the working tree does not have .git directory to tell us to stop, no? -- To unsubscribe from this list: send the line 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 v2] gc: remove gc.pid file at end of execution
This file isn't really harmful, but isn't useful either, and can create
minor annoyance for the user:
* It's confusing, as the presence of a *.pid file often implies that a
process is currently running. A user running ls .git/ and finding
this file may incorrectly guess that a git gc is currently running.
* Leaving this file means that a git gc in an already gc-ed repo is
no-longer a no-op. A user running git gc in a set of repositories,
and then synchronizing this set (e.g. rsync -av, unison, ...) will see
all the gc.pid files as changed, which creates useless noise.
This patch unlinks the file after the garbage collection is done, so that
gc.pid is actually present only during execution.
Future versions of Git may want to use the information left in the gc.pid
file (e.g. for policies like don't attempt to run a gc if one has
already been ran less than X hours ago). If so, this patch can safely be
reverted. For now, let's not bother the users.
Explained-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
Improved-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Junio C Hamano wrote:
Has anything further happened to this discussion?
Here's a patch implementing Duy's suggestion.
builtin/gc.c | 24
t/t6500-gc.sh | 5 +
2 files changed, 29 insertions(+)
diff --git a/builtin/gc.c b/builtin/gc.c
index 891a2c2..c14190f 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -14,6 +14,7 @@
#include cache.h
#include parse-options.h
#include run-command.h
+#include sigchain.h
#include argv-array.h
#define FAILED_RUN failed to run %s
@@ -35,6 +36,21 @@ static struct argv_array repack = ARGV_ARRAY_INIT;
static struct argv_array prune = ARGV_ARRAY_INIT;
static struct argv_array rerere = ARGV_ARRAY_INIT;
+static char *pidfile;
+
+static void remove_pidfile(void)
+{
+ if (pidfile)
+ unlink(pidfile);
+}
+
+static void remove_pidfile_on_signal(int signo)
+{
+ remove_pidfile();
+ sigchain_pop(signo);
+ raise(signo);
+}
+
static int gc_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, gc.packrefs)) {
@@ -179,6 +195,10 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
FILE *fp;
int fd, should_exit;
+ if (pidfile)
+ /* already locked */
+ return NULL;
+
if (gethostname(my_host, sizeof(my_host)))
strcpy(my_host, unknown);
@@ -219,6 +239,10 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
strbuf_release(sb);
commit_lock_file(lock);
+ pidfile = git_pathdup(gc.pid);
+ sigchain_push_common(remove_pidfile_on_signal);
+ atexit(remove_pidfile);
+
return NULL;
}
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index b1a6365..63194d8 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -9,6 +9,11 @@ test_expect_success 'gc empty repository' '
git gc
'
+test_expect_success 'gc does not leave behind pid file' '
+ git gc
+ test_path_is_missing .git/gc.pid
+'
+
test_expect_success 'gc --gobbledegook' '
test_expect_code 129 git gc --nonsense 2err
test_i18ngrep [Uu]sage: git gc err
--
1.8.4-50-g437ce60
--
To unsubscribe from this list: send the line 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: pack corruption post-mortem
On Wed, Oct 16, 2013 at 09:41:16AM -0600, Martin Fick wrote: I have nightmares about this sort of thing every now and then, and we even experience some corruption here and there that needs to be fixed (mainly missing objects when we toy with different git repack arguments). I cannot help but wonder, how we can improve git further to either help diagnose or even fix some of these problems? More inline below... In general, I don't think we know enough about patterns of recovery corruption to say which commands would definitely be worth implementing. Part of the reason I wrote this up is to document this one case. But this is the first time in 7 years of git usage that I've had to do this. So I'd feel a little bit better about sinking time into it after seeing a few more cases and realizing where the patterns are. One of the major hassles is that the assumptions you can and can't make depend on what data you have that _isn't_ corrupted. Do you have a pack index, or a bare pack? Do you have zlib data that fails the crc, or zlib data that cannot be parsed? In this case there was no other copy of the repository. But if you know the broken object (which we did here), and you can copy it from elsewhere, then git already will try to find other sources of the object (loose, or in another pack). dd if=$pack of=object bs=1 skip=51653873 count=10863 Is there a current plumbing command that should be enhanced to be able to do the 2 steps above directly for people debugging (maybe with some new switch)? If not, should we create one, git show --zlib, or git cat-file --zlib? Most of the git plumbing commands deal with data at the object layer. This is really about going a step below and saying Give me the on-disk representation of the object. We recently introduced an %(objectsize:disk) formatter for cat-file. The logical extension would be to ask for %(contents:disk) or something. Though what you get would depend on how the object is stored, so you would need to figure that out to do anything useful with it. Note that this implies you actually have a packfile index that says object XXX is at offset YYY. In some corruption cases, you might have only a packfile. That is generally enough to generate the index, but if there is corruption, you cannot actually parse the pack to find out the sha1 of the objects. So in the worst case, what you really want is something like dump the object in packfile X at offset Y. But even then, you don't know the length of the object. The packfile is a stream, and the length we calculated is from the index, which depends on the zlib data parsing in some sane way. and then running git index-pack tmp.pack in the debugger (stop at unpack_raw_entry). Doing this, I found that there were 3 bytes of header (and the header itself had a sane type and size). So I stripped those off with: dd if=object of=zlib bs=1 skip=3 This too feels like something we should be able to do with a plumbing command eventually? git zlib-extract Perhaps. I think if you had some extract object at offset X from the packfile command, it would be optional to give you the whole thing, or just the zlib data. So I took a different approach. Working under the guess that the corruption was limited to a single byte, I wrote a program to munge each byte individually, and try inflating the result. Since the object was only 10K compressed, that worked out to about 2.5M attempts, which took a few minutes. Awesome! Would this make a good new plumbing command, git zlib-fix? I'd like to see it actually work more than once first. This relies on there being single-byte corruption. Even double-byte corruption starts to get expensive to brute-force like this. SHA1, by its nature, requires brute-forcing. But it's possible that the crc, not being cryptographically secure, could be reverse-engineered to find likely spots of corruption. I don't know enough about it to say. I fixed the packfile itself with: chmod +w $pack printf '\xc7' | dd of=$pack bs=1 seek=51659518 conv=notrunc chmod -w $pack The '\xc7' comes from the replacement byte our munge program found. The offset 51659518 is derived by taking the original object offset (51653873), adding the replacement offset found by munge (5642), and then adding back in the 3 bytes of git header we stripped. Another plumbing command needed? git pack-put --zlib? I think in this case that dd does a nice job of solving the problem. Some of the stuff I did was very git-specific and required knowledge of the formats. But this one is really just replace byte X at offset Y, and I don't see any need to avoid a general-purpose tool (except that dd is itself reasonably arcane :) ). -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: Feature request: Config option for --no-ignore-removal/--ignore-removal
Junio C Hamano wrote: Matthew Cline m...@nightrealms.com writes: When I try to a plain old git add . when files have been deleted/moved, I get the warning You ran 'git add' with neither '-A (--all)' or '--ignore-removal' There should be some way to put something in ~/.gitconfig to tell git to always choose one or another. The lack of configurability is very much deliberate. Adding such knobs that make basic behaviour of Git different depending on the per-user setting will make it unnecessarily harder to run to help your coworker when she is having problems. git add directory you type in her terminal during the session to help her could work differently from the way you are used to, if we added such a knob. That's going to happen regardless. You are assuming she is running Git v2.0, which she might not. -- 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: pack corruption post-mortem
On Wed, Oct 16, 2013 at 10:41 PM, Martin Fick mf...@codeaurora.org wrote: and then running git index-pack tmp.pack in the debugger (stop at unpack_raw_entry). Doing this, I found that there were 3 bytes of header (and the header itself had a sane type and size). So I stripped those off with: dd if=object of=zlib bs=1 skip=3 This too feels like something we should be able to do with a plumbing command eventually? git zlib-extract Not an official plumbing, but I faced similar problems with pack v4. I needed to verify that the output is correct and low level decoding like this is generally a good thing to start with. So I wrote test-dump [1] that can take an offset, a format and try to decode it. It does not support zlib inflation yet, but adding one should be easy. And because this is just a test program we don't really need to think hard before adding something. [1] http://article.gmane.org/gmane.comp.version-control.git/235388 -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] doc: git-foo was obsoleted several years ago
On Wed, Oct 16, 2013 at 02:38:27PM -0700, Junio C Hamano wrote: Documentation/git-checkout.txt | 4 ++-- [...] I recall that I wanted to see this change happen myself long time ago, and suspect that there may have been some reason that prevented us from doing so. I might have found that AsciiDoc back then did not like the input if the headline name git-checkout(1) did not match the filename git-checkout.txt and the command in the NAME section git-checkout, or links linkgit:git-checkout[1] from other pages couldn't have SP there, or something silly like that. Yes, I think it is still broken. After applying Felipe's patch: $ cd Documentation $ make git-checkout.1 GEN cmd-list.made SUBDIR ../ make[1]: `GIT-VERSION-FILE' is up to date. ASCIIDOC git-checkout.xml XMLTO git-checkout.1 So far, so good... $ man -l git-checkout.1 man: git-checkout.1: No such file or directory Huh? $ ls git?checkout.1 git_checkout.1 Oh. There is similar asciidoc (actually, I think it is docbook) cleverness with: $ make gitignore.1 GEN cmd-list.made SUBDIR ../ make[1]: `GIT-VERSION-FILE' is up to date. XMLTO gitignore.1 $ ls gitignore.[0-9] gitignore.5 Now obviously what I asked for is wrong (we do not actually know how to make gitignore.1), but I have certainly been confused by this in the past when working on pages outside of section 1. In both cases, it would be nice if we could tell xmlto no, really, put the output in this file. I'm not sure that is an option, though, because in theory docbook output may be multiple files (the -o option actually specifies an output directory). Since we know our manpages are a much simpler case, we could probably work around it by teaching the Makefile that building git-foo.txt generates git_foo.1, and then moving that to git-foo.1. -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
