Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)
Junio C Hamano gits...@pobox.com writes: * Introduce git add --ignore-removal option in the release after the current cycle (a new feature is too late for this cycle): Too late in the cycle even if the option is simply ignored ... ? [To extend the range of git versions where it's not an error] -miles -- Kilt, n. A costume sometimes worn by Scotchmen [sic] in America and Americans in Scotland. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-commit: populate the edit buffer with 2 blank lines before s-o-b
Before commit 33f2f9ab, 'commit -s' would populate the edit buffer with a blank line before the Signed-off-by line. This provided a nice hint to the user that something should be filled in. Let's restore that behavior, but now let's ensure that the Signed-off-by line is preceded by two blank lines to hint that something should be filled in, and that a blank line should separate it from the Signed-off-by line. Plus, add a test for this behavior. Reported-by: John Keeping j...@keeping.me.uk Signed-off-by: Brandon Casey draf...@gmail.com --- Ok. Here's a patch on top of 959a2623 bc/append-signed-off-by. It implements the 2 blank lines preceding sob behavior. -Brandon sequencer.c | 5 +++-- t/t7502-commit.sh | 12 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/sequencer.c b/sequencer.c index 53ee49a..2dac106 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1127,9 +1127,10 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag) const char *append_newlines = NULL; size_t len = msgbuf-len - ignore_footer; - if (len msgbuf-buf[len - 1] != '\n') + /* ensure a blank line precedes our signoff */ + if (!len || msgbuf-buf[len - 1] != '\n') append_newlines = \n\n; - else if (len 1 msgbuf-buf[len - 2] != '\n') + else if (len == 1 || msgbuf-buf[len - 2] != '\n') append_newlines = \n; if (append_newlines) diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index deb187e..a53a1e0 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -349,6 +349,18 @@ test_expect_success 'A single-liner subject with a token plus colon is not a foo ' +test_expect_success 'commit -s places sob on third line after two empty lines' ' + git commit -s --allow-empty --allow-empty-message + cat -EOF expect + + + Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL + + EOF + egrep -v '^#' .git/COMMIT_EDITMSG actual + test_cmp expect actual +' + write_script .git/FAKE_EDITOR \EOF mv $1 $1.orig ( -- 1.8.0.1.253.gfcb57d5.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add: allow users to silence Git 2.0 warnings about add -u
David Aguilar dav...@gmail.com writes: Please enlighten me. Are we really getting rid of it and replacing it with :/? That syntax looks like a meh face.. just sayin' The current behavior is indeed replaced by git add -u ., not :/. Unlike push.default, whose warning can be silenced with configuration, git 1.x does not have a way to silence this warning without retraining existing users. Yes, but push.default is really different: there is a config variable, and we want the behavior to be configurable. In the case of git add, I don't think adding a configuration option would be the right thing. That would mean typing git add -u on an account which isn't yours will be unpredictable *forever*. OTOH, git add -u :/ and git add -u . will behave predictibly on any version of Git that accepts them, past present or future (:/ was added in 1.7.6, a year and a half ago). Another example... $ git grep 'stuff' :/ would it be too much to teach it to do: $ git grep -u 'stuff' git grep is out of the scope of this change. Yes, it is inconsistant with the rest of Git, but doesn't seem to surprise users as much as git add -u (for which the inconsistancy appears within the add command). I don't understand what you mean by git grep -u. git add -u is a shortcut for git add --update, and git grep --update wouldn't make sense to me. Do you mean we should add a --full-tree to git grep? That seems really overkill to me since we already have the :/ pathspec. but in 2.0 that -u would be a no-op because grep will be full tree, no? No it won't. I need to read the old discussions. Can someone tell me the magic google search syntax they use to dig them up? See the discussion here: http://thread.gmane.org/gmane.comp.version-control.git/213988/focus=214106 (recursively, there's a pointer to an older discussion) Would a better way be a method to make git add -u behave like 2.0 today? As I said, I think adding a configuration option that would remain after 2.0 would do more harm than good. But after thinking about it, I'm not against an option like a boolean add.use2dot0Behavior that would: * Right now, adopt the future behavior and kill the warning * From 2.0, kill the warning without changing the bevavior * When we stop warning, disapear. This, or the add.silence-pathless-warnings (which BTW should be spelled add.silencePathlessWarnings) would not harm as long as they are not advertized in the warning. What we don't want is dumb users reading half the message and apply the quickest receipe they find to kill the warning without thinking about the consequences. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: -B option of git log
On 02/22/2013 01:57 AM, Jeff King wrote: I think the problem is that your test file is so tiny that it falls afoul of git's MINIMUM_BREAK_SIZE heuristic of 400 bytes (which prevents false positives on tiny files). Try replacing your Lorem ipsum echo with something like seq 1 150, and I think you will find the result you are expecting. Thanks. Two points, though: With bigger files, I do get something like: | M100 d | R100 d e The rename is fine. But I am a bit puzzled mit the M - I, somehow, would have expected an A for add and not a M. Secondly, would it be a good idea to enhance the documentation on that point to clarify this minimum size? SEcki -- 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 15/19] pkt-line: share buffer/descriptor reading implementation
On Wed, Feb 20, 2013 at 3:04 PM, Jeff King p...@peff.net wrote: diff --git a/pkt-line.h b/pkt-line.h index fa93e32..47361f5 100644 --- a/pkt-line.h +++ b/pkt-line.h @@ -25,9 +25,16 @@ void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((f void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3))); /* - * Read a packetized line from the descriptor into the buffer, which must be at - * least size bytes long. The return value specifies the number of bytes read - * into the buffer. + * Read a packetized line into the buffer, which must be at least size bytes + * long. The return value specifies the number of bytes read into the buffer. + * + * If src_buffer is not NULL (and nor is *src_buffer), it should point to a + * buffer containing the packet data to parse, of at least *src_len bytes. + * After the function returns, src_buf will be increments and src_len s/increments/incremented/ + * decremented by the number of bytes consumed. -- 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] branch: segfault fixes and validation
branch_get() can return NULL (so far on detached HEAD only) but some
code paths in builtin/branch.c cannot deal with that and cause
segfaults. While at there, make sure we bail out when the user gives 2
or more arguments, but we only process the first one and silently
ignore the rest.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
On Fri, Feb 22, 2013 at 12:47 AM, Junio C Hamano gits...@pobox.com wrote:
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
branch_get() can return NULL (so far on detached HEAD only)...
Do you anticipate any other cases where the API call should validly
return NULL?
No. But I do not see any guarantee that it may never do that in
future either. Which is why I was deliberately vague with could not
figure out But you also correctly observed my laziness there. So
how about this? It makes a special case for HEAD but not insist on
detached HEAD as the only cause.
builtin/branch.c | 24
t/t3200-branch.sh | 21 +
2 files changed, 45 insertions(+)
diff --git a/builtin/branch.c b/builtin/branch.c
index 6371bf9..82ed337 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -889,6 +889,15 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
} else if (new_upstream) {
struct branch *branch = branch_get(argv[0]);
+ if (argc 1)
+ die(_(too many branches to set new upstream));
+
+ if (!branch) {
+ if (!argc || !strcmp(argv[0], HEAD))
+ die(_(HEAD does not point to any branch. Is it
detached?));
+ die(_(no such branch '%s'), argv[0]);
+ }
+
if (!ref_exists(branch-refname))
die(_(branch '%s' does not exist), branch-name);
@@ -901,6 +910,15 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
struct branch *branch = branch_get(argv[0]);
struct strbuf buf = STRBUF_INIT;
+ if (argc 1)
+ die(_(too many branches to unset upstream));
+
+ if (!branch) {
+ if (!argc || !strcmp(argv[0], HEAD))
+ die(_(HEAD does not point to any branch. Is it
detached?));
+ die(_(no such branch '%s'), argv[0]);
+ }
+
if (!branch_has_merge_config(branch)) {
die(_(Branch '%s' has no upstream information),
branch-name);
}
@@ -916,6 +934,12 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
int branch_existed = 0, remote_tracking = 0;
struct strbuf buf = STRBUF_INIT;
+ if (!strcmp(argv[0], HEAD))
+ die(_(it does not make sense to create 'HEAD'
manually));
+
+ if (!branch)
+ die(_(no such branch '%s'), argv[0]);
+
if (kinds != REF_LOCAL_BRANCH)
die(_(-a and -r options to 'git branch' do not make
sense with a branch name));
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index f3e0e4a..12f1e4a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -42,6 +42,10 @@ test_expect_success \
'git branch a/b/c should create a branch' \
'git branch a/b/c test_path_is_file .git/refs/heads/a/b/c'
+test_expect_success \
+'git branch HEAD should fail' \
+'test_must_fail git branch HEAD'
+
cat expect EOF
$_z40 $HEAD $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL 1117150200 +
branch: Created from master
EOF
@@ -388,6 +392,14 @@ test_expect_success \
'git tag foobar
test_must_fail git branch --track my11 foobar'
+test_expect_success '--set-upstream-to fails on multiple branches' \
+'test_must_fail git branch --set-upstream-to master a b c'
+
+test_expect_success '--set-upstream-to fails on detached HEAD' \
+'git checkout HEAD^{}
+ test_must_fail git branch --set-upstream-to master
+ git checkout -'
+
test_expect_success 'use --set-upstream-to modify HEAD' \
'test_config branch.master.remote foo
test_config branch.master.merge foo
@@ -417,6 +429,15 @@ test_expect_success 'test --unset-upstream on HEAD' \
test_must_fail git branch --unset-upstream
'
+test_expect_success '--unset-upstream should fail on multiple branches' \
+'test_must_fail git branch --unset-upstream a b c'
+
+test_expect_success '--unset-upstream should fail on detached HEAD' \
+'git checkout HEAD^{}
+ test_must_fail git branch --unset-upstream
+ git checkout -
+'
+
test_expect_success 'test --unset-upstream on a particular branch' \
'git branch my15
git branch --set-upstream-to master my14
--
1.8.1.2.536.gf441e6d
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info
[PATCH 1/2] index-format.txt: mention of v4 is missing in some places
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/technical/index-format.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/technical/index-format.txt
b/Documentation/technical/index-format.txt
index 27c716b..0810251 100644
--- a/Documentation/technical/index-format.txt
+++ b/Documentation/technical/index-format.txt
@@ -12,7 +12,7 @@ Git index format
The signature is { 'D', 'I', 'R', 'C' } (stands for dircache)
4-byte version number:
- The current supported versions are 2 and 3.
+ The current supported versions are 2, 3 and 4.
32-bit number of index entries.
@@ -93,8 +93,8 @@ Git index format
12-bit name length if the length is less than 0xFFF; otherwise 0xFFF
is stored in this field.
- (Version 3) A 16-bit field, only applicable if the extended flag
- above is 1, split into (high to low bits).
+ (Version 3 or later) A 16-bit field, only applicable if the
+ extended flag above is 1, split into (high to low bits).
1-bit reserved for future
--
1.8.1.2.536.gf441e6d
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] read-cache.c: use INDEX_FORMAT_{LB,UB} in verify_hdr()
9d22778 (read-cache.c: write prefix-compressed names in the index - 2012-04-04) defined these. Interestingly, they were not used by read-cache.c, or anywhere in that patch. They were used in builtin/update-index.c later for checking supported index versions. Use them here too. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- read-cache.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/read-cache.c b/read-cache.c index 827ae55..298161f 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1260,7 +1260,8 @@ static int verify_hdr(struct cache_header *hdr, unsigned long size) if (hdr-hdr_signature != htonl(CACHE_SIGNATURE)) return error(bad signature); hdr_version = ntohl(hdr-hdr_version); - if (hdr_version 2 || 4 hdr_version) + if (hdr_version INDEX_FORMAT_LB || + hdr_version INDEX_FORMAT_UB) return error(bad index version %d, hdr_version); git_SHA1_Init(c); git_SHA1_Update(c, hdr, size - 20); -- 1.8.1.2.536.gf441e6d -- 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] Fix in Git.pm cat_blob crashes on large files
running git svn fetch on a remote repository (yes I know there are a lot of possible outside variables, including network latency) Code with 1024 reads and 64k writes: real75m19.906s user16m43.919s sys 29m16.326s Code with 1024 reads and 1024 writes: real71m21.006s user12m36.275s sys 24m26.112s ...so the simpler code wins the trivial test. I would say go with it. Should I resubmit? On Thu, Feb 21, 2013 at 3:24 PM, Jeff King p...@peff.net wrote: On Thu, Feb 21, 2013 at 03:18:40PM -0800, Joshua Clayton wrote: By having the read and flush size be the same, it's much simpler. My original bugfix did just read 1024, and write 1024. That works fine and, yes, is simpler. I changed it to be more similar to the original code in case there were performance reasons for doing it that way. That was the only reason I could think of for the design, and adding the $flushSize variable means that some motivated person could easily optimize it. So far I have been too lazy to profile the two versions I guess I'll try a trivial git svn init; git svn fetch and check back in. Its running now. I doubt it will make much of a difference; we are already writing to a perl filehandle, so it will be buffered there (which I assume is 4K, but I haven't checked). And your version retains the 1024-byte read. I do think 1024 is quite low for this sort of descriptor-to-descriptor copying. I'd be tempted to just bump that 1024 to 64K. In git svn fetch (which is how I discovered it) the file being passed to cat_blob is a temporary file, which is checksummed before putting it into place. Great. There may be other callers outside of our tree, of course, but I think it's pretty clear that the responsibility is on the caller to make sure the function succeeded. We are changing what gets put on the output stream for various error conditions, but ultimately that is an implementation detail that the caller should not be depending on. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: --inline-single
On Thu, Feb 21, 2013 at 06:13:28PM -0500, Jeff King wrote: On Thu, Feb 21, 2013 at 12:26:22PM -0800, Junio C Hamano wrote: Some people may find it convenient to append a simple patch at the bottom of a discussion e-mail separated by a scissors mark, ready to be applied with git am -c. Introduce --inline-single option to format-patch to do so. A typical usage example might be to start 'F'ollow-up to a discussion, write your message, conclude with a patch to do so may look like this., and \C-u M-! git format-patch --inline-single -1 HEAD ENTER if you are an Emacs user. Users of other MUA's may want to consult their manuals to find equivalent command to append output from an external command to the message being composed. Interesting. I usually just do this by hand, but this could save a few keystrokes in my workflow. Same here. This is great; thanks a lot both for working on 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] Fix in Git.pm cat_blob crashes on large files
On Fri, Feb 22, 2013 at 07:11:54AM -0800, Joshua Clayton wrote: running git svn fetch on a remote repository (yes I know there are a lot of possible outside variables, including network latency) Code with 1024 reads and 64k writes: real75m19.906s user16m43.919s sys 29m16.326s Code with 1024 reads and 1024 writes: real71m21.006s user12m36.275s sys 24m26.112s ...so the simpler code wins the trivial test. Interesting; I'd have expected no change or a slight win for your version, which makes me wonder if the outside variables are dominating. I wonder what 64K/64K would look like. I would say go with it. Should I resubmit? Yes, please. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: --inline-single
Jeff King p...@peff.net writes: ... helper function to see if the user is the author ... +} Nice, I'm glad you handled this case properly. I've wondered if we should have an option to do a similar test when writing out the real message format. I.e., to put the extra From line in the body of the message when !is_current_user(). Traditionally we have just said that is the responsibility of the MUA you use, and let send-email handle it. But it means people who do not use send-email have to reimplement the feature themselves. I am not sure if I follow. Do you mean that you have to remove fewer lines if you omit Date/From when it is from you in the first place? People who do not use send-email (like me) slurp the output 0001-have-gostak-distim-doshes.patch into their MUA editor, tell the MUA to use the contents on the Subject: line as the subject, and remove what is redundant, including the Subject. Because the output cannot be used as-is anyway, I do not think it is such a big deal. And those who have a custom mechanism to stuff our output in their MUA's outbox, similar to what imap-send does, would already have to have a trivial parser to read the first part of our output up to the first blank line (i.e. parsing out the header part) and formatting the information it finds into a form that is understood by their MUA. Omitting From: or Date: lines would not help those people who already have established the procedure to handle the Oh, this one is from me case, or to send the output always with the Sender: and keeping the From: intact. So,... -- 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 (Feb 2013, #05; Tue, 12)
Miles Bader mi...@gnu.org writes: Junio C Hamano gits...@pobox.com writes: * Introduce git add --ignore-removal option in the release after the current cycle (a new feature is too late for this cycle): Too late in the cycle even if the option is simply ignored ... ? [To extend the range of git versions where it's not an error] I'd feel safer to have enough time to cook the alleged no-op before merging it to 'master' and include it in a release. Possible implementation mistakes aside, --ignore-removal is probably too long to type, we haven't even discussed if it deserves a short-and-sweet single letter option, the obvious -i is not available, etc. etc. I do not think we have a concensus that the transition plan outlined is a good way to go in the first place. So, I do think it is a bit too late for this cycle, especially when we still have doubts about the design. Actually it is *I* who have doubts; I do not even know if other people share the doubts or they support the direction wholeheartedly. -- 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] Fix in Git.pm cat_blob crashes on large files (reviewed by Jeff King)
Read and write each 1024 byte buffer, rather than trying to buffer
the entire content of the file.
Previous code would crash on all files 2 Gib, when the offset variable
became negative (perhaps below the level of perl), resulting in a crash.
On a 32 bit system, or a system with low memory it might crash before
reaching 2 GiB due to memory exhaustion.
Signed-off-by: Joshua Clayton stillcompil...@gmail.com
---
perl/Git.pm | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..cc91288 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -949,13 +949,16 @@ sub cat_blob {
last unless $bytesLeft;
my $bytesToRead = $bytesLeft 1024 ? $bytesLeft : 1024;
- my $read = read($in, $blob, $bytesToRead, $bytesRead);
+ my $read = read($in, $blob, $bytesToRead);
unless (defined($read)) {
$self-_close_cat_blob();
throw Error::Simple(in pipe went bad);
}
-
$bytesRead += $read;
+ unless (print $fh $blob) {
+ $self-_close_cat_blob();
+ throw Error::Simple(couldn't write to passed in
filehandle);
+ }
}
# Skip past the trailing newline.
@@ -970,11 +973,6 @@ sub cat_blob {
throw Error::Simple(didn't find newline after blob);
}
- unless (print $fh $blob) {
- $self-_close_cat_blob();
- throw Error::Simple(couldn't write to passed in filehandle);
- }
-
return $size;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix in Git.pm cat_blob crashes on large files (reviewed by Jeff King)
On Fri, Feb 22, 2013 at 6:03 PM, Joshua Clayton stillcompil...@gmail.com wrote: Read and write each 1024 byte buffer, rather than trying to buffer the entire content of the file. Previous code would crash on all files 2 Gib, when the offset variable became negative (perhaps below the level of perl), resulting in a crash. On a 32 bit system, or a system with low memory it might crash before reaching 2 GiB due to memory exhaustion. Signed-off-by: Joshua Clayton stillcompil...@gmail.com We usually put Reviewed-by: Name email right below the SoB-line, rather than in the subject. And then we CC the people who were involved in the previous round :) -- 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] archive: let remote clients get reachable commits
Sergey Sergeev gurug...@yandex.ru writes: [jc: please do not top-post] You are right, I'll rethink this patch and write some test for this cases. Thanks. Note that this is harder to implement than one would naïvely think, if one aims for a very generic solution, without walking the whole history. I personally think that it is OK to limit the scope to expressions that start from the tip of ref and expressions that start with the SHA-1 at the tip of ref, e.g. master~12:Documentation v2.6.11:arch/alpha 5dc01c595e6c6ec9ccda# tag v2.6.11 26791a8bcf0e6d33f43a:arch # tag v2.6.12 26791a8bcf0~12:arch # starting at 26791a8b and dig down are OK, while forbidding the following: c39ae07f393806ccf406# tree of tag v2.6.11 9ee1c939d1cb936b1f98# commit v2.6.12^0 9ee1c939d1cb936b1f98: # tree of commit v2.6.12^0 9ee1c939d1cb936b1f98:arch # subtree of commit v2.6.12^0 which will make it significantly easier to implement the necessary validation in a robust way. -- 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] add: allow users to silence Git 2.0 warnings about add -u
David Aguilar dav...@gmail.com writes: Please enlighten me. As you lack the knowledge of previous discussion, I think you will be the best person to proofread the paragraph on this issue in the backward compatibilty notes section of the draft release notes to v1.8.2 to see if that is understandable to the end users and point out what are missing or confusing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: --inline-single
On Fri, Feb 22, 2013 at 08:47:39AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: ... helper function to see if the user is the author ... +} Nice, I'm glad you handled this case properly. I've wondered if we should have an option to do a similar test when writing out the real message format. I.e., to put the extra From line in the body of the message when !is_current_user(). Traditionally we have just said that is the responsibility of the MUA you use, and let send-email handle it. But it means people who do not use send-email have to reimplement the feature themselves. I am not sure if I follow. Do you mean that you have to remove fewer lines if you omit Date/From when it is from you in the first place? Sorry, I think I confused you by going off on a tangent. The rest of my email was about dropping unnecessary lines from the inline view. But here I was talking about another possible use of the is user the author function. For the existing view, we show: From: A U Thor aut...@example.com Date: ... Subject: [PATCH] whatever body and if committer != author, we expect the MUA to convert that to: From: C O Mitter commit...@example.com Date: ... Subject: [PATCH] whatever From: A U Thor aut...@example.com body That logic happens in git-send-email right now, but given that your patch adds the are we the author? function, it would be trivial to add a --sender-is-committer option to format-patch to have it do it automatically. That saves the MUA from having to worry about it. People who do not use send-email (like me) slurp the output 0001-have-gostak-distim-doshes.patch into their MUA editor, tell the MUA to use the contents on the Subject: line as the subject, and remove what is redundant, including the Subject. Because the output cannot be used as-is anyway, I do not think it is such a big deal. That is one way to do it. Another way is to hand the output of format-patch to your MUA as a template, making it a starting point for a message we are about to send. No manual editing is necessary in that case, unless the From header does not match the sender identity. And those who have a custom mechanism to stuff our output in their MUA's outbox, similar to what imap-send does, would already have to have a trivial parser to read the first part of our output up to the first blank line (i.e. parsing out the header part) and formatting the information it finds into a form that is understood by their MUA. Not necessarily. The existing format is an rfc822 message, which mailers understand already. It's perfectly cromulent to do: git format-patch --stdout $@ mbox mutt -f mbox and use mutt's resend-message as a starting point for sending each message. No editing is necessary except for adding recipients (which you can also do on the command-line to format-patch). Omitting From: or Date: lines would not help those people who already have established the procedure to handle the Oh, this one is from me case, or to send the output always with the Sender: and keeping the From: intact. So,... Right, my point was to help people who _should_ have implemented the oh, this one is from me case, but were too lazy to do so (and it's actually a little tricky to get right, because you might have to adjust the mime headers to account for encoded author names). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] archive: let remote clients get reachable commits
On Fri, Feb 22, 2013 at 09:10:46AM -0800, Junio C Hamano wrote: I personally think that it is OK to limit the scope to expressions that start from the tip of ref and expressions that start with the SHA-1 at the tip of ref, e.g. master~12:Documentation v2.6.11:arch/alpha 5dc01c595e6c6ec9ccda # tag v2.6.11 26791a8bcf0e6d33f43a:arch # tag v2.6.12 26791a8bcf0~12:arch # starting at 26791a8b and dig down are OK, while forbidding the following: c39ae07f393806ccf406# tree of tag v2.6.11 9ee1c939d1cb936b1f98 # commit v2.6.12^0 9ee1c939d1cb936b1f98: # tree of commit v2.6.12^0 9ee1c939d1cb936b1f98:arch # subtree of commit v2.6.12^0 which will make it significantly easier to implement the necessary validation in a robust way. How are you proposing to verify master~12 in that example? Because during parsing, it starts with master, and we remember that? Or because you are doing a reachability traversal on all refs after we parse? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add: allow users to silence Git 2.0 warnings about add -u
Matthieu Moy matthieu@grenoble-inp.fr writes: Yes, but push.default is really different: there is a config variable, and we want the behavior to be configurable. In the case of git add, I don't think adding a configuration option would be the right thing. That would mean typing git add -u on an account which isn't yours will be unpredictable *forever*. Exactly. $ git grep 'stuff' :/ would it be too much to teach it to do: $ git grep -u 'stuff' git grep is out of the scope of this change. Yes, it is inconsistant with the rest of Git, but doesn't seem to surprise users as much as git add -u (for which the inconsistancy appears within the add command). It is consistent with grep, and the reason git grep behaves that way is because consistency with grep matters more. I do not think it is going to change. Another is clean, which I do not personally care too deeply about; it being a destructive operation that is only used interactively and occasionally), the current behaviour to limit it to the cwd is much more sensible than making it go and touch parts of the tree that is unrelated to cwd. I don't understand what you mean by git grep -u. I think he meant to add git grep --full-tree or something, and I do not think it is going to happen when we have :/ magic pathspec. As I said, I think adding a configuration option that would remain after 2.0 would do more harm than good. But after thinking about it, I'm not against an option like a boolean add.use2dot0Behavior that would: * Right now, adopt the future behavior and kill the warning * From 2.0, kill the warning without changing the bevavior * When we stop warning, disapear. It is marginally better than David's set once without thinking because I read it on stackoverflow without fully understanding the ramifications, and forget about it only to suffer when Git 2.0 happens configuration variable, but by not much. You can easily imagine Q. Help, Git 1.8.2 is giving me this warning. What to do? A. Set this configuration variable. period. and many users setting it without realizing that they are making the operation tree-wide at the same time. We'd want to see this answer instead: A. Say git add -u .; you want to add changed paths in the current directory. Another problem with use2dot0 configuration is that it would invite people to imagine that setting it to false will keep the old behaviour forever, which is against what you set out to do with the patch under 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
[PATCH] Fix in Git.pm cat_blob crashes on large files (resubmit with reviewed-by)
Read and write each 1024 byte buffer, rather than trying to buffer
the entire content of the file.
Previous code would crash on all files 2 Gib, when the offset variable
became negative (perhaps below the level of perl), resulting in a crash.
On a 32 bit system, or a system with low memory it might crash before
reaching 2 GiB due to memory exhaustion.
Signed-off-by: Joshua Clayton stillcompil...@gmail.com
Reviewed-by: Jeff King p...@peff.net
---
perl/Git.pm | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..cc91288 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -949,13 +949,16 @@ sub cat_blob {
last unless $bytesLeft;
my $bytesToRead = $bytesLeft 1024 ? $bytesLeft : 1024;
- my $read = read($in, $blob, $bytesToRead, $bytesRead);
+ my $read = read($in, $blob, $bytesToRead);
unless (defined($read)) {
$self-_close_cat_blob();
throw Error::Simple(in pipe went bad);
}
-
$bytesRead += $read;
+ unless (print $fh $blob) {
+ $self-_close_cat_blob();
+ throw Error::Simple(couldn't write to passed
in filehandle);
+ }
}
# Skip past the trailing newline.
@@ -970,11 +973,6 @@ sub cat_blob {
throw Error::Simple(didn't find newline after blob);
}
- unless (print $fh $blob) {
- $self-_close_cat_blob();
- throw Error::Simple(couldn't write to passed in filehandle);
- }
-
return $size;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] archive: let remote clients get reachable commits
Jeff King p...@peff.net writes: How are you proposing to verify master~12 in that example? Because during parsing, it starts with master, and we remember that? By not cheating (i.e. using get_sha1()), but making sure you can parse master and the adornment on it ~12 is something sane. That is why I said this is harder than one would naively think, but limiting will make it significantly easier. I didn't say that it would become trivial, did I? -- 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] archive: let remote clients get reachable commits
On Fri, Feb 22, 2013 at 10:06:56AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: How are you proposing to verify master~12 in that example? Because during parsing, it starts with master, and we remember that? By not cheating (i.e. using get_sha1()), but making sure you can parse master and the adornment on it ~12 is something sane. So, like these patches: http://article.gmane.org/gmane.comp.version-control.git/188386 http://article.gmane.org/gmane.comp.version-control.git/188387 ? They do not allow arbitrary sha1s that happen to point to branch tips, but I am not sure whether that is something people care about or not. That is why I said this is harder than one would naively think, but limiting will make it significantly easier. I didn't say that it would become trivial, did I? I'm not implying it would be trivial. It was an honest question, since you did not seem to want to do the pass-more-information-out-of-get-sha1 approach last time this came up. Even though those patches above are from me, I've come to the conclusion that the best thing to do is to harmonize with upload-pack. Then you never have the well, but I could fetch it, so why won't upload-archive let me get it argument. Something like: 1. split name at first colon (like we already do) 2. make sure the left-hand side is reachable according to the same rules that upload-pack uses. Right we just say is it a ref. It should be: 2a. if it is a commit-ish, is it reachable from a ref? 2b. otherwise, is it pointed to directly by a ref? 3. Abort if it's not reachable. Abort if it's not a tree-ish. No checks necessary on the right-hand side, because a path lookup in a tree-ish is always reachable from the tree-ish. I.e., the same rule we have now. I did not check if upload-pack will respect a want line for an object accessible only by peeling a tag. But an obvious 2c could be is it accessible by peeling the refs? That leaves the only inaccessible thing as direct-sha1s of trees and blobs that are reachable from commits. But you also cannot ask for those directly via upload-pack, and I do not think it's worth it to do the much more expensive reachability check to verify those (OTOH, it is no more expensive than the current counting objects for a clone, and we could do it only as a fallback when cheaper checks do not work). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix in Git.pm cat_blob crashes on large files (resubmit with reviewed-by)
On Fri, Feb 22, 2013 at 09:30:57AM -0800, Joshua Clayton wrote: Read and write each 1024 byte buffer, rather than trying to buffer the entire content of the file. OK. Did you ever repeat your timing with a larger symmetric buffer? That should probably be a separate patch on top, but it might be worth doing while we are thinking about it. Previous code would crash on all files 2 Gib, when the offset variable became negative (perhaps below the level of perl), resulting in a crash. I'm still slightly dubious of this, just because it doesn't match my knowledge of perl (which is admittedly imperfect). I'm curious how you diagnosed it? On a 32 bit system, or a system with low memory it might crash before reaching 2 GiB due to memory exhaustion. Signed-off-by: Joshua Clayton stillcompil...@gmail.com Reviewed-by: Jeff King p...@peff.net The commit message is a good place to mention any side effects, and why they are not a problem. Something like: The previous code buffered the whole blob before writing, so any error reading from cat-file would result in zero bytes being written to the output stream. After this change, the output may be left in a partially written state (or even fully written, if we fail when parsing the final newline from cat-file). However, it's not reasonable for callers to expect anything about the state of the output when we return an error (after all, even with full buffering, we might fail during the writing process). So any caller which cares about this is broken already, and we do not have to worry about them. --- perl/Git.pm | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) The patch itself looks fine to me. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-commit: populate the edit buffer with 2 blank lines before s-o-b
Brandon Casey draf...@gmail.com writes: Before commit 33f2f9ab, 'commit -s' would populate the edit buffer with a blank line before the Signed-off-by line. This provided a nice hint to the user that something should be filled in. Let's restore that behavior, but now let's ensure that the Signed-off-by line is preceded by two blank lines to hint that something should be filled in, and that a blank line should separate it from the Signed-off-by line. Plus, add a test for this behavior. Reported-by: John Keeping j...@keeping.me.uk Signed-off-by: Brandon Casey draf...@gmail.com --- Ok. Here's a patch on top of 959a2623 bc/append-signed-off-by. It implements the 2 blank lines preceding sob behavior. -Brandon sequencer.c | 5 +++-- t/t7502-commit.sh | 12 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/sequencer.c b/sequencer.c index 53ee49a..2dac106 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1127,9 +1127,10 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag) const char *append_newlines = NULL; size_t len = msgbuf-len - ignore_footer; - if (len msgbuf-buf[len - 1] != '\n') + /* ensure a blank line precedes our signoff */ + if (!len || msgbuf-buf[len - 1] != '\n') append_newlines = \n\n; - else if (len 1 msgbuf-buf[len - 2] != '\n') + else if (len == 1 || msgbuf-buf[len - 2] != '\n') append_newlines = \n; Maybe I am getting slower with age, but it took me 5 minutes of staring the above to convince me that it is doing the right thing. The if/elseif cascade is dealing with three separate things and the logic is a bit dense: * Is the buffer completely empty? We need to add two LFs to give a room for the title and body; * Otherwise: - Is the final line incomplete? We need to add one LF to make it a complete line whatever we do. - Is the final line an empty line? We need to add one more LF to make sure we have a blank line before we add S-o-b. I wondered if we can rewrite it to make the logic clearer (that is where I spent most of the 5 minutes), but I did not think of a better way; probably the above is the best we could do. Thanks. By the way, I think we would want to introduce a symbolic constants for the possible return values from has_conforming_footer(). The check that appears after this hunk if (has_footer != 3 (!no_dup_sob || has_footer != 2)) strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0, sob.buf, sob.len); is hard to grok without them. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix in Git.pm cat_blob crashes on large files (resubmit
On Fri, Feb 22, 2013 at 09:30:57AM -0800, Joshua Clayton wrote: Subject: Re: [PATCH] Fix in Git.pm cat_blob crashes on large files (resubmit with reviewed-by) One thing I forgot to note in my other response: the subject is part of the commit message, so information like resubmit with... should go with other cover letter material after the ---. It's also customary to say something like PATCHv2 in the brackets (which does get stripped off), and mention what is changed since v1 after the ---. That can help reviewers remember what happened, and it can help the maintainer understand where in the process of review the patch is. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix in Git.pm cat_blob crashes on large files (resubmit with reviewed-by)
Joshua Clayton stillcompil...@gmail.com writes:
Read and write each 1024 byte buffer, rather than trying to buffer
the entire content of the file.
Previous code would crash on all files 2 Gib, when the offset variable
became negative (perhaps below the level of perl), resulting in a crash.
On a 32 bit system, or a system with low memory it might crash before
reaching 2 GiB due to memory exhaustion.
Signed-off-by: Joshua Clayton stillcompil...@gmail.com
Reviewed-by: Jeff King p...@peff.net
---
Thanks.
Subject: Re: [PATCH] Fix in Git.pm cat_blob crashes on large files (resubmit
with reviewed-by)
Please drop the () part. A rule of thumb is to make git show
output understandable by people who read it 6 months from now. They
do not care if the commit is a re-submission.
It seems that this issue was with us since the very beginning of
this sub since it was introduced at 7182530d8cad (Git.pm: Add
hash_and_insert_object and cat_blob, 2008-05-23).
perl/Git.pm | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..cc91288 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -949,13 +949,16 @@ sub cat_blob {
last unless $bytesLeft;
my $bytesToRead = $bytesLeft 1024 ? $bytesLeft : 1024;
- my $read = read($in, $blob, $bytesToRead, $bytesRead);
+ my $read = read($in, $blob, $bytesToRead);
unless (defined($read)) {
$self-_close_cat_blob();
throw Error::Simple(in pipe went bad);
}
-
$bytesRead += $read;
+ unless (print $fh $blob) {
+ $self-_close_cat_blob();
+ throw Error::Simple(couldn't write to passed
in filehandle);
+ }
Corrupt patch, line-wrapped by your MUA.
I wonder if we still need $bytesRead variable. You have $size that
is the size of the whole blob, so
my $bytesLeft = $size;
while (1) {
my $bytesToRead = $bytesLeft 1024 ? $bytesLeft : 1024;
my $bytesRead = read($in, $blob, $bytesToRead);
... check errors and use the $blob ...
$bytesLeft -= $bytesRead;
}
may be simpler and easier to read, 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: [PATCH] archive: let remote clients get reachable commits
Jeff King p...@peff.net writes: On Fri, Feb 22, 2013 at 10:06:56AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: How are you proposing to verify master~12 in that example? Because during parsing, it starts with master, and we remember that? By not cheating (i.e. using get_sha1()), but making sure you can parse master and the adornment on it ~12 is something sane. So, like these patches: http://article.gmane.org/gmane.comp.version-control.git/188386 http://article.gmane.org/gmane.comp.version-control.git/188387 ? They do not allow arbitrary sha1s that happen to point to branch tips, but I am not sure whether that is something people care about or not. That is why I said this is harder than one would naively think, but limiting will make it significantly easier. I didn't say that it would become trivial, did I? I'm not implying it would be trivial. It was an honest question, since you did not seem to want to do the pass-more-information-out-of-get-sha1 approach last time this came up. That does not match my recollection ($gmane/188427). In fact, I had those two patches you quoted earlier in mind when I wrote the limit it and it will become easier response. Even though those patches above are from me, I've come to the conclusion that the best thing to do is to harmonize with upload-pack. Then you never have the well, but I could fetch it, so why won't upload-archive let me get it argument. That sounds like a sensible yardstick. 1. split name at first colon (like we already do) 2. make sure the left-hand side is reachable according to the same rules that upload-pack uses. Well, upload-pack under the hood operates on a bare 40-hex. If you mean to do split name at first colon, run get_sha1() on the LHS in step 1., I would agree this is a good direction to go. Right we just say is it a ref. It should be: With s/should/could optionally/, I would agree. That leaves the only inaccessible thing as direct-sha1s of trees and blobs that are reachable from commits. Yes (with s/are reachable/are only reachable/). -- 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] Fix in Git.pm cat_blob crashes on large files (resubmit with reviewed-by)
Thanks for all the input and patience. On Fri, Feb 22, 2013 at 10:34 AM, Jeff King p...@peff.net wrote: On Fri, Feb 22, 2013 at 09:30:57AM -0800, Joshua Clayton wrote: Read and write each 1024 byte buffer, rather than trying to buffer the entire content of the file. OK. Did you ever repeat your timing with a larger symmetric buffer? That should probably be a separate patch on top, but it might be worth doing while we are thinking about it. Previous code would crash on all files 2 Gib, when the offset variable became negative (perhaps below the level of perl), resulting in a crash. I'm still slightly dubious of this, just because it doesn't match my knowledge of perl (which is admittedly imperfect). I'm curious how you diagnosed it? I first had the memory exhaustion problem running my git repo on a 32 vm. After bumping the memory from 512 to 4 GiB, and that failing to fix it I moved to my workstation with 16 GiB ...reproduced After the initial crash, I added print $size, , $bytesToRead, , $bytesRead, \n; right before the read command, and it does indeed crash right after the $bytesRead variable crosses LONG_MAX ... 2567089913 1024 2147482624 2567089913 1024 2147483648 2567089913 1024 2147484672 Offset outside string at /usr/share/perl5/Git.pm line 901, GEN36 line 2604. Note that $bytesRead is still positive. I know very little perl, but that symptom seems pretty clear On a 32 bit system, or a system with low memory it might crash before reaching 2 GiB due to memory exhaustion. Signed-off-by: Joshua Clayton stillcompil...@gmail.com Reviewed-by: Jeff King p...@peff.net The commit message is a good place to mention any side effects, and why they are not a problem. Something like: The previous code buffered the whole blob before writing, so any error reading from cat-file would result in zero bytes being written to the output stream. After this change, the output may be left in a partially written state (or even fully written, if we fail when parsing the final newline from cat-file). However, it's not reasonable for callers to expect anything about the state of the output when we return an error (after all, even with full buffering, we might fail during the writing process). So any caller which cares about this is broken already, and we do not have to worry about them. --- perl/Git.pm | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) The patch itself looks fine to me. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule update: when using recursion, show full path
Thanks. Your code changes are looking good and the commit message explains what you did and why you did it. A few comments below. Am 22.02.2013 05:25, schrieb Will Entriken: From d3fe2c76e6fa53e4cfa6f81600685c21bdadd4e3 Mon Sep 17 00:00:00 2001 From: William Entriken github@phor.net Date: Thu, 21 Feb 2013 23:10:07 -0500 Subject: [PATCH] submodule update: when using recursion, show full path The lines above aren't necessary as they are taken from the mail header. Previously when using update with recursion, only the path for the inner-most module was printed. Now the path is printed from GIT_DIR. You should replace from GIT_DIR with something like relative to the directory the recursion started in here. This now matches the behavior of submodule foreach Please add a '.' at the end of the sentence. --- First patch. Several tests failed, but they were failing before I started. This is against maint, I would consider this a (low priority) bug. Strange that you have failing tests, for me everything runs fine (With or without your patch). But I agree that this is a low priority bug. How does it look? Please let me know next steps. This patch does not apply due to removed leading whitespaces and a few wrapped lines. Please see Documentation/git-format-patch.txt on how to convince the mailer of your choice to send the patch out unmangled. Signed-off-by: William Entriken github@phor.net The Signed-off-by belongs just before the --- line above, as everything between --- and the line below won't make it into the commit message. git-submodule.sh | 31 ++- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 2365149..f2c53c9 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -588,7 +588,7 @@ cmd_update() die_if_unmatched $mode if test $stage = U then - echo 2 Skipping unmerged submodule $sm_path + echo 2 Skipping unmerged submodule $prefix$sm_path continue fi name=$(module_name $sm_path) || exit @@ -602,7 +602,7 @@ cmd_update() if test $update_module = none then - echo Skipping submodule '$sm_path' + echo Skipping submodule '$prefix$sm_path' continue fi @@ -611,7 +611,7 @@ cmd_update() # Only mention uninitialized submodules when its # path have been specified test $# != 0 - say $(eval_gettext Submodule path '\$sm_path' not initialized + say $(eval_gettext Submodule path '\$prefix\$sm_path' not initialized Maybe you want to use 'update --init'?) continue fi @@ -624,7 +624,7 @@ Maybe you want to use 'update --init'?) else subsha1=$(clear_local_git_env; cd $sm_path git rev-parse --verify HEAD) || - die $(eval_gettext Unable to find current revision in submodule path '\$sm_path') + die $(eval_gettext Unable to find current revision in submodule path '\$prefix\$sm_path') fi if test $subsha1 != $sha1 -o -n $force @@ -643,7 +643,7 @@ Maybe you want to use 'update --init'?) (clear_local_git_env; cd $sm_path ( (rev=$(git rev-list -n 1 $sha1 --not --all 2/dev/null) test -z $rev) || git-fetch)) || - die $(eval_gettext Unable to fetch in submodule path '\$sm_path') + die $(eval_gettext Unable to fetch in submodule path '\$prefix\$sm_path') fi # Is this something we just cloned? @@ -657,20 +657,20 @@ Maybe you want to use 'update --init'?) case $update_module in rebase) command=git rebase - die_msg=$(eval_gettext Unable to rebase '\$sha1' in submodule path '\$sm_path') - say_msg=$(eval_gettext Submodule path '\$sm_path': rebased into '\$sha1') + die_msg=$(eval_gettext Unable to rebase '\$sha1' in submodule path '\$prefix\$sm_path') + say_msg=$(eval_gettext Submodule path '\$prefix\$sm_path': rebased into '\$sha1') must_die_on_failure=yes ;; merge) command=git merge - die_msg=$(eval_gettext Unable to merge '\$sha1' in submodule path '\$sm_path') - say_msg=$(eval_gettext Submodule path '\$sm_path': merged in '\$sha1') + die_msg=$(eval_gettext Unable to merge '\$sha1' in submodule path '\$prefix\$sm_path') + say_msg=$(eval_gettext Submodule path '\$prefix\$sm_path': merged in '\$sha1') must_die_on_failure=yes ;; *) command=git checkout $subforce -q - die_msg=$(eval_gettext Unable to checkout '\$sha1' in submodule path '\$sm_path') - say_msg=$(eval_gettext Submodule path '\$sm_path': checked out '\$sha1') + die_msg=$(eval_gettext Unable to checkout '\$sha1' in submodule path '\$prefix\$sm_path') + say_msg=$(eval_gettext Submodule path '\$prefix\$sm_path': checked out '\$sha1') ;; esac @@ -688,11 +688,16 @@ Maybe you want to use 'update --init'?) if test -n $recursive then - (clear_local_git_env; cd $sm_path eval cmd_update $orig_flags) + ( + prefix=$prefix$sm_path/ + clear_local_git_env + cd $sm_path + eval cmd_update $orig_flags + ) res=$? if test $res -gt 0 then - die_msg=$(eval_gettext Failed to recurse
Re: Is this a bug?
On Tue, Feb 19, 2013 at 6:02 AM, Duy Nguyen pclo...@gmail.com wrote: On Tue, Feb 19, 2013 at 4:47 PM, Erik Faye-Lund kusmab...@gmail.com wrote: On Tue, Feb 19, 2013 at 10:32 AM, David Wade da...@statoil.com wrote: Hi, I wrote a commit message beginning with a hash (#) character, like this: 'git commit -m #ifdef ' Everything went okay when committing, but then I tried 'git commit -amend' and without editing the commit message I was told I had an empty commit message. Is this a problem with my text editor (vim 7.2) or git itself? (git version 1.7.2.2 under RedHat 5.8) Or something I'm not supposed to do ;-) ? The problem is that when doing interactive editing of messages (like 'git commit --amend' does), git considers '#' as a comment-character. You can disable this by using the --cleanup=verbatim switch (or some other suiting cleanup-setting, see 'git help commit'). Nobody is always conscious about the leading # in commit message to do that. I once edited a commit message and the auto-wrap feature put '#' at the beginning of the line. I saved and went on without noticing one line was lost until much later :( Perhaps we should change the comment signature a bit to reduce accidents, like only recognize '#' lines as comments after a special line like # this is not a comment ### START OF COMMENT ### # this is a comment Or maybe --amend should imply --cleanup=whitespace. -- Phil -- 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] branch: segfault fixes and validation
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
branch_get() can return NULL (so far on detached HEAD only) but some
code paths in builtin/branch.c cannot deal with that and cause
segfaults. While at there, make sure we bail out when the user gives 2
or more arguments, but we only process the first one and silently
ignore the rest.
Explain 2 or more arguments in what context, perhaps? Otherwise
it makes it sound as if git branch foo bar baz is covered with
this patch, no?
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
On Fri, Feb 22, 2013 at 12:47 AM, Junio C Hamano gits...@pobox.com wrote:
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
branch_get() can return NULL (so far on detached HEAD only)...
Do you anticipate any other cases where the API call should validly
return NULL?
No. But I do not see any guarantee that it may never do that in
future either. Which is why I was deliberately vague with could not
figure out But you also correctly observed my laziness there. So
how about this? It makes a special case for HEAD but not insist on
detached HEAD as the only cause.
Sure. It looks better.
What you can do is to have a single helper function that can explain
why branch_get() returned NULL (or extend branch_get() to serve that
purpose as well); then you do not have to duplicate the logic twice
on the caller's side (and there may be other callers that want to do
the same).
diff --git a/builtin/branch.c b/builtin/branch.c
index 6371bf9..82ed337 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -889,6 +889,15 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
} else if (new_upstream) {
struct branch *branch = branch_get(argv[0]);
+ if (argc 1)
+ die(_(too many branches to set new upstream));
+
+ if (!branch) {
+ if (!argc || !strcmp(argv[0], HEAD))
+ die(_(HEAD does not point to any branch. Is it
detached?));
+ die(_(no such branch '%s'), argv[0]);
+ }
+
if (!ref_exists(branch-refname))
die(_(branch '%s' does not exist), branch-name);
The latter part of the new code triggers when branch_get() returns
NULL while doing git branch --set-upstream-to=X [Y]. When Y is
string HEAD or missing, the first die() is triggered and says a
funny thing. If HEAD does not point to any branch, by definition it
is detached. The user may say Yes, I know it is detached. but the
message does not help the user to figure out what to do next.
Instead of asking Is it detached?, perhaps we can say something
like You told me to set the upstream of HEAD to branch X, but in
front? At least, that will be a better explanation for the reason
why the operation is failing.
The existing test might be wrong, by the way. Your HEAD may point
at a branch Y but you may not have any commit on it yet, and you may
want to allow setting the upstream of that to-be-born branch to
another branch X with branch --set-upstream-to=X [Y|HEAD].
While I think it is insane to do anything before creating the first
commit on your current branch (or using checkout --orphan in
general) and it may not be worth our time to babysit users who do
so, but the following sequence may feel natural to them:
git checkout --orphan X
git branch --set-upstream-to=master
... perhaps create an initial commit, perhaps not ...
git merge @{upstream}
For that to work sanely, perhaps the pattern
branch = branch_get();
if (!branch)
die due to no branch;
if (!ref_exists(branch-refname))
die due to typo in branch name
may need to be fixed globally, replacing ref_exists(branch-refname)
with branch_exists(branch) that returns true if branch-refname is
an existing ref, or the branch in question was obtained by checking
with current_branch (in remote.c), or something like that.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] index-format.txt: mention of v4 is missing in some places
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/technical/index-format.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/technical/index-format.txt
b/Documentation/technical/index-format.txt
index 27c716b..0810251 100644
--- a/Documentation/technical/index-format.txt
+++ b/Documentation/technical/index-format.txt
@@ -12,7 +12,7 @@ Git index format
The signature is { 'D', 'I', 'R', 'C' } (stands for dircache)
4-byte version number:
- The current supported versions are 2 and 3.
+ The current supported versions are 2, 3 and 4.
32-bit number of index entries.
@@ -93,8 +93,8 @@ Git index format
12-bit name length if the length is less than 0xFFF; otherwise 0xFFF
is stored in this field.
- (Version 3) A 16-bit field, only applicable if the extended flag
- above is 1, split into (high to low bits).
+ (Version 3 or later) A 16-bit field, only applicable if the
+ extended flag above is 1, split into (high to low bits).
1-bit reserved for future
Depending on how the first later version decides to encode the
additional flag information, or later may need to be changed to
and 4 when it happens. As we cannot predict the future, I think
or later is just as good as add 4 for now.
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 2/2] read-cache.c: use INDEX_FORMAT_{LB,UB} in verify_hdr()
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: 9d22778 (read-cache.c: write prefix-compressed names in the index - 2012-04-04) defined these. Interestingly, they were not used by read-cache.c, or anywhere in that patch. They were used in builtin/update-index.c later for checking supported index versions. Use them here too. Thanks. - if (hdr_version 2 || 4 hdr_version) + if (hdr_version INDEX_FORMAT_LB || + hdr_version INDEX_FORMAT_UB) return error(bad index version %d, hdr_version); -- 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
[PATCHv3] Fix in Git.pm cat_blob crashes on large files
Read and write each 1024 byte buffer, rather than trying to buffer
the entire content of the file.
Previous code would crash on all files 2 Gib, when the offset variable
became negative (perhaps below the level of perl), resulting in a crash.
On a 32 bit system, or a system with low memory it might crash before
reaching 2 GiB due to memory exhaustion.
This code may leave a partial file behind in case of failure, where the
old code would leave a completely empty file.
Neither version verifies the correctness of the content.
Calling code must take care of verification and cleanup.
Signed-off-by: Joshua Clayton stillcompil...@gmail.com
Reviewed-by: Jeff King p...@peff.net
Reviewed-by: Junio C Hamano gits...@pobox.com
---
perl/Git.pm | 17 +++--
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..db6e0a8 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -942,20 +942,22 @@ sub cat_blob {
my $size = $1;
my $blob;
- my $bytesRead = 0;
+ my $bytesLeft = $size;
while (1) {
- my $bytesLeft = $size - $bytesRead;
last unless $bytesLeft;
my $bytesToRead = $bytesLeft 1024 ? $bytesLeft : 1024;
- my $read = read($in, $blob, $bytesToRead, $bytesRead);
+ my $read = read($in, $blob, $bytesToRead);
unless (defined($read)) {
$self-_close_cat_blob();
throw Error::Simple(in pipe went bad);
}
-
- $bytesRead += $read;
+ unless (print $fh $blob) {
+ $self-_close_cat_blob();
+ throw Error::Simple(couldn't write to passed in
filehandle);
+ }
+ $bytesLeft -= $read;
}
# Skip past the trailing newline.
@@ -970,11 +972,6 @@ sub cat_blob {
throw Error::Simple(didn't find newline after blob);
}
- unless (print $fh $blob) {
- $self-_close_cat_blob();
- throw Error::Simple(couldn't write to passed in filehandle);
- }
-
return $size;
}
--
1.7.10.4
I'm trying to send this with git-send-email this time around to beat
the linewrapping problem.
--
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
post-receive-email hook with custom_showrev
Hi I'm trying to setup the post-receive-email hook, from contrib, using a custom_showrev, from the script I do this by setting hooks.showrev # hooks.showrev # The shell command used to format each revision in the email, with # %s replaced with the commit id. Defaults to git rev-list -1 # --pretty %s, displaying the commit id, author, date and log # message. To list full patches separated by a blank line, you # could set this to git show -C %s; echo. # To list a gitweb/cgit URL *and* a full patch for each change set, use this: # t=%s; printf 'http://.../?id=%%s' \$t; echo;echo; git show -C \$t; echo # Be careful if ... contains things that will be expanded by shell eval # or printf. in my repositories config I have showrev set to: [hooks] showrev = t=%s; printf 'http://server/cgit/repository/commit/?id=%%s' \$t; echo;echo; git show -C \$t; echo But in the emails from the post-receive-email hook I get something like: This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project LALSuite. The branch, master has been updated via 10f97dd6db3861e35e517301f6c1dec30be90012 (commit) from 8c7dfa89cec5ac0a5b9520967b92a927734611f0 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log - --- Summary of changes: lal/README |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) So it seems as if showrev is being ignored? Can anyone see what I'm doing wrong? Cheers Adam -- 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: [PATCHv3] Fix in Git.pm cat_blob crashes on large files
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: Is this a bug?
Phil Hord phil.h...@gmail.com writes: Or maybe --amend should imply --cleanup=whitespace. I am fairly negative on that. Such a hidden linkage, even if it is documented, is just one more thing people need to learn. It _might_ be interesting (note: I did not say worthwhile here) to think what happens if we scanned the message (either coming from the commit being amended, -F file option, or -m message option), picked a punctuation character that does not appear at the beginning of the lines in it, and automatically adjusted core.commentchar if '#' appears at the beginning of one of the lines, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: https_proxy and libcurl
Phil Hord phil.h...@gmail.com writes: I have been unable to clone via http proxy because of a wrongly configured proxy setup in my lab. I had this env: http_proxy=http://proxy.myco.com https_proxy=https://proxy.myco.com The problem is that libcurl ignores the protocol part of the proxy url, and it defaults to port 1080. wget honors the protocol specifier, but it defaults to port 80 if none is given. IIRC, the historical norm is to set these to host:port. So many people mistakenly write them with method:// that some tools over time learned to strip and ignore that prefix, though. The fix was to specify the port explicitly, like this: http_proxy=proxy.myco.com:80 https_proxy=proxy.myco.com:443 Yeah, that is the correct syntax to use. Is there anything you want Git to do to be more helpful? -- 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] git-commit: populate the edit buffer with 2 blank lines before s-o-b
From: Brandon Casey draf...@gmail.com
Before commit 33f2f9ab, 'commit -s' would populate the edit buffer with
a blank line before the Signed-off-by line. This provided a nice
hint to the user that something should be filled in. Let's restore that
behavior, but now let's ensure that the Signed-off-by line is preceded
by two blank lines to hint that something should be filled in, and that
a blank line should separate it from the Signed-off-by line.
Plus, add a test for this behavior.
Reported-by: John Keeping j...@keeping.me.uk
Signed-off-by: Brandon Casey draf...@gmail.com
---
How about something like this?
-Brandon
sequencer.c | 27 +--
t/t7502-commit.sh | 12
2 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 53ee49a..a07d2d0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1127,10 +1127,33 @@ void append_signoff(struct strbuf *msgbuf, int
ignore_footer, unsigned flag)
const char *append_newlines = NULL;
size_t len = msgbuf-len - ignore_footer;
- if (len msgbuf-buf[len - 1] != '\n')
+ if (!len) {
+ /*
+* The buffer is completely empty. Leave foom for
+* the title and body to be filled in by the user.
+*/
append_newlines = \n\n;
- else if (len 1 msgbuf-buf[len - 2] != '\n')
+ } else if (msgbuf-buf[len - 1] != '\n') {
+ /*
+* Incomplete line. Complete the line and add a
+* blank one so that there is an empty line between
+* the message body and the sob.
+*/
+ append_newlines = \n\n;
+ } else if (len == 1) {
+ /*
+* Buffer contains a single newline. Add another
+* so that we leave room for the title and body.
+*/
+ append_newlines = \n;
+ } else if (msgbuf-buf[len - 2] != '\n') {
+ /*
+* Buffer ends with a single newline. Add another
+* so that there is an empty line between the message
+* body and the sob.
+*/
append_newlines = \n;
+ } /* else, the buffer already ends with two newlines. */
if (append_newlines)
strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0,
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index deb187e..a53a1e0 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -349,6 +349,18 @@ test_expect_success 'A single-liner subject with a token
plus colon is not a foo
'
+test_expect_success 'commit -s places sob on third line after two empty lines'
'
+ git commit -s --allow-empty --allow-empty-message
+ cat -EOF expect
+
+
+ Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
+
+ EOF
+ egrep -v '^#' .git/COMMIT_EDITMSG actual
+ test_cmp expect actual
+'
+
write_script .git/FAKE_EDITOR \EOF
mv $1 $1.orig
(
--
1.8.1.3.566.gaa39828
--
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: https_proxy and libcurl
On Fri, 22 Feb 2013, Junio C Hamano wrote: http_proxy=http://proxy.myco.com https_proxy=https://proxy.myco.com The problem is that libcurl ignores the protocol part of the proxy url, and it defaults to port 1080. wget honors the protocol specifier, but it defaults to port 80 if none is given. IIRC, the historical norm is to set these to host:port. So many people mistakenly write them with method:// that some tools over time learned to strip and ignore that prefix, though. You're right, but also what exactly is the https:// _to_ the proxy supposed to mean? The standard procedure to connect to a proxy when communicating with either HTTP or HTTPS is using plain HTTP. If you would want port 443 for a HTTPS connection to a proxy, you'd use: https_proxy=http://proxy.myco.com:443 Or without the ignore protocol prefix: https_proxy=proxy.myco.com:443 -- / daniel.haxx.se -- 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
Crashes while trying to show tag objects with bad timestamps
When messing around with various repositories, I noticed that git 1.8
(currently using 1.8.2.rc0.22.gb3600c3) has problems parsing tag objects
that have invalid timestamps.
Times in tag objects appear to be kept as Unix timestamps, but I didn't
realize this at first, and ran something roughly equivalent to:
git cat-file -p $tagname | git hash-object -w -t tag --stdin
creating a tag object the tagger line containing formatted time
instead of a Unix timestamp.
Git doesn't handle the resulting tag objects nicely at all. For example,
running `git cat-file -p` on the new object outputs a really odd
timestamp Thu Jun Thu Jan 1 00:16:09 1970 +0016 (I'm guessing it
parses the year as Unix time), and `git show` outright crashes
(backtrace included below.)
I would have expected both commands to print a tag object corrupt
message, or maybe even a more specific bad timestamp in tagger line...
To reproduce:
printf '%s\n' \
'object 4b825dc642cb6eb9a060e54bf8d69288fbee4904' 'type tree' \
'tag test' 'tagger User user@none Thu Jun 9 16:44:04 2005 +' \
'' 'Test tag' | git hash-object -w -t tag --stdin | xargs git show
#0 0x7f42560bb5f3 in strtoull_l_internal () from /usr/lib/libc.so.6
No symbol table info available.
#1 0x004b4c81 in pp_user_info (pp=pp@entry=0x7fff3c30f1a0,
what=what@entry=0x50c13b Tagger, sb=sb@entry=0x7fff3c30f140,
line=0xc267c7 Jilles Tjoelker jil...@stack.nl Thu Jun 9 16:44:04 2005
+\n\nTag 1.0rc1.\n\n, encoding=0x507e20 UTF-8) at pretty.c:431
name = {alloc = 24, len = 15, buf = 0xc24690 Jilles Tjoelker}
mail = {alloc = 24, len = 15, buf = 0xc24750 jil...@stack.nl}
ident = {
name_begin = 0xc267c7 Jilles Tjoelker jil...@stack.nl Thu Jun 9
16:44:04 2005 +\n\nTag 1.0rc1.\n\n,
name_end = 0xc267d6 jil...@stack.nl Thu Jun 9 16:44:04 2005
+\n\nTag 1.0rc1.\n\n,
mail_begin = 0xc267d8 jil...@stack.nl Thu Jun 9 16:44:04 2005
+\n\nTag 1.0rc1.\n\n, mail_end = 0xc267e7 Thu Jun 9 16:44:04 2005
+\n\nTag 1.0rc1.\n\n,
date_begin = 0x0, date_end = 0x0, tz_begin = 0x0, tz_end = 0x0}
linelen = optimized out
line_end = optimized out
date = optimized out
mailbuf = 0xc267d8 jil...@stack.nl Thu Jun 9 16:44:04 2005
+\n\nTag 1.0rc1.\n\n
namebuf = 0xc267c7 Jilles Tjoelker jil...@stack.nl Thu Jun 9
16:44:04 2005 +\n\nTag 1.0rc1.\n\n
namelen = 33
maillen = 15
max_length = 78
time = optimized out
tz = optimized out
#2 0x00439af5 in show_tagger (buf=optimized out, len=optimized
out,
rev=optimized out) at builtin/log.c:400
pp = {fmt = CMIT_FMT_MEDIUM, abbrev = 0, subject = 0x0, after_subject
= 0x0,
preserve_subject = 0, date_mode = DATE_NORMAL, date_mode_explicit =
0,
need_8bit_cte = 0, notes_message = 0x0, reflog_info = 0x0,
output_encoding = 0x0, mailmap = 0x0, color = 0}
out = {alloc = 0, len = 0, buf = 0x7a8188 strbuf_slopbuf }
#3 show_tag_object (rev=0x7fff3c30f1f0,
sha1=0xc2be44
\230\211\275\331\365Q\306z\017\071d\331\035\062\247a\347~M8P, incomplete
sequence \303) at builtin/log.c:427
new_offset = 151
type = OBJ_TAG
buf = 0xc26770 object ffa28d13e40e03bd367d0219c7eb516be0f180d2\ntype
commit\ntag hyperion-1.0rc1\ntagger Jilles Tjoelker jil...@stack.nl Thu Jun
9 16:44:04 2005 +\n\nTag 1.0rc1.\n\n
size = 165
offset = optimized out
--
Mantas Mikulėnas graw...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-commit: populate the edit buffer with 2 blank lines before s-o-b
On Fri, Feb 22, 2013 at 02:05:27PM -0800, Brandon Casey wrote: From: Brandon Casey draf...@gmail.com Before commit 33f2f9ab, 'commit -s' would populate the edit buffer with a blank line before the Signed-off-by line. This provided a nice hint to the user that something should be filled in. Let's restore that behavior, but now let's ensure that the Signed-off-by line is preceded by two blank lines to hint that something should be filled in, and that a blank line should separate it from the Signed-off-by line. Plus, add a test for this behavior. Reported-by: John Keeping j...@keeping.me.uk Signed-off-by: Brandon Casey draf...@gmail.com --- How about something like this? FWIW, as a casual reader of this series, I find this to be way easier to follow than the previous round. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-commit: populate the edit buffer with 2 blank lines before s-o-b
Jeff King p...@peff.net writes: FWIW, as a casual reader of this series, I find this to be way easier to follow than the previous round. It is assuring to know that I am not the only one getting slow with age ;-) 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: Crashes while trying to show tag objects with bad timestamps
On Sat, Feb 23, 2013 at 12:30:28AM +0200, Mantas Mikulėnas wrote:
When messing around with various repositories, I noticed that git 1.8
(currently using 1.8.2.rc0.22.gb3600c3) has problems parsing tag objects
that have invalid timestamps.
Times in tag objects appear to be kept as Unix timestamps, but I didn't
realize this at first, and ran something roughly equivalent to:
git cat-file -p $tagname | git hash-object -w -t tag --stdin
creating a tag object the tagger line containing formatted time
instead of a Unix timestamp.
Thanks, that makes it easy to replicate. It looks like it is not just
tags, but rather the pp_user_info function does not realize that
split_ident may return NULL for the date field if it is unparseable.
Something like this stops the crash and just gives a bogus date:
diff --git a/pretty.c b/pretty.c
index eae57ad..9688857 100644
--- a/pretty.c
+++ b/pretty.c
@@ -428,8 +428,16 @@ void pp_user_info(const struct pretty_print_context *pp,
strbuf_add(name, namebuf, namelen);
namelen = name.len + mail.len + 3; /* ' ' + '' + '' */
- time = strtoul(ident.date_begin, date, 10);
- tz = strtol(date, NULL, 10);
+
+ if (ident.date_begin) {
+ time = strtoul(ident.date_begin, date, 10);
+ tz = strtol(date, NULL, 10);
+ }
+ else {
+ /* ident line had malformed date */
+ time = 0;
+ tz = 0;
+ }
if (pp-fmt == CMIT_FMT_EMAIL) {
strbuf_addstr(sb, From: );
I guess we should probably issue a warning, too. Also disappointingly,
git-fsck does not seem to detect this breakage at all.
Git doesn't handle the resulting tag objects nicely at all. For example,
running `git cat-file -p` on the new object outputs a really odd
timestamp Thu Jun Thu Jan 1 00:16:09 1970 +0016 (I'm guessing it
parses the year as Unix time), and `git show` outright crashes
(backtrace included below.)
If cat-file -p is not using the usual pretty-print routines, it
probably should. I'll take a look.
-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: Crashes while trying to show tag objects with bad timestamps
Jeff King p...@peff.net writes: I guess we should probably issue a warning, too. Also disappointingly, git-fsck does not seem to detect this breakage at all. Yes for the warning, and no for disappointing. IIRC, in the very early implementations allowed tag object without dates. I _think_ we can start tightening fsck, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] hash-object doc: git hash-object -w can write invalid objects
When using hash-object -w to create non-blob objects, it is generally a good policy to run git fsck afterward to make sure the resulting object is valid. Add a warning to the manpage. While it at, gently nudge the user of hash-object -w toward higher-level interfaces for creating or modifying trees, commits, and tags. Reported-by: Mantas Mikulėnas graw...@gmail.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Hi Mantas, Mantas Mikulėnas wrote: When messing around with various repositories, I noticed that git 1.8 (currently using 1.8.2.rc0.22.gb3600c3) has problems parsing tag objects that have invalid timestamps. [...] Git doesn't handle the resulting tag objects nicely at all. For example, running `git cat-file -p` on the new object outputs a really odd timestamp Thu Jun Thu Jan 1 00:16:09 1970 +0016 (I'm guessing it parses the year as Unix time), The usual rule is that with invalid objects (e.g. as detected by git fsck), any non-crash result is acceptable. Garbage in, garbage out. and `git show` outright crashes (backtrace included below.) Probably worth fixing. I notice that git-hash-object(1) doesn't contain any reference to git-fsck(1). How about something like this, to start? Perhaps by default hash-object should automatically fsck the objects it is asked to create. Thanks, Jonathan Documentation/git-hash-object.txt | 10 ++ 1 file changed, 10 insertions(+) diff --git a/Documentation/git-hash-object.txt b/Documentation/git-hash-object.txt index 02c1f12..8ed8c6e 100644 --- a/Documentation/git-hash-object.txt +++ b/Documentation/git-hash-object.txt @@ -30,6 +30,8 @@ OPTIONS -w:: Actually write the object into the object database. + This does not check that the resulting object is valid; + for that, see linkgit:git-fsck[1]. --stdin:: Read the object from standard input instead of from a file. @@ -53,6 +55,14 @@ OPTIONS conversion. If the file is read from standard input then this is always implied, unless the --path option is given. +SEE ALSO + +linkgit:git-mktree[1], +linkgit:git-commit-tree[1], +linkgit:git-tag[1], +linkgit:git-filter-branch[1], +sha1sum(1) + GIT --- Part of the linkgit:git[1] suite -- 1.8.1.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] hash-object doc: git hash-object -w can write invalid objects
Jonathan Nieder jrnie...@gmail.com writes: Perhaps by default hash-object should automatically fsck the objects it is asked to create. Yes, and let the experimentors to override when they are trying to invent a new object type, finished a reader but not a writer (that is why they are exprimenting with hash-object) nor updated fsck, with an explicit command line option to hash-objects. Then we do not have to say -w by default can create an invalid object in its documentation. In a sense, allowing to create any garbage (by the definition of then-current fsck and the rest of the Git) is the raison d'etre of the command. 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: [RFC/PATCH] hash-object doc: git hash-object -w can write invalid objects
On Fri, Feb 22, 2013 at 03:01:32PM -0800, Jonathan Nieder wrote: Git doesn't handle the resulting tag objects nicely at all. For example, running `git cat-file -p` on the new object outputs a really odd timestamp Thu Jun Thu Jan 1 00:16:09 1970 +0016 (I'm guessing it parses the year as Unix time), The usual rule is that with invalid objects (e.g. as detected by git fsck), any non-crash result is acceptable. Garbage in, garbage out. Agreed, though I think a more consistent garbage would be good (e.g., time=0, tz=0). I notice that git-hash-object(1) doesn't contain any reference to git-fsck(1). How about something like this, to start? I think it's a good change. Though note that this problem is not discovered by fsck (which I think we should also change). Perhaps by default hash-object should automatically fsck the objects it is asked to create. Not unreasonable. In this case, we also have git-mktag. It would be nice if we could simply run the input through a type-specific sanity checker (optional, I hope; I use hash-object often to craft test cases like this :) ). The same need came up a month or two ago in a discussion of how to use git replace safely. But I guess fsck after-the-fact is just another form of the same solution. -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
[PATCH] t7502: perform commits using alternate editor in a subshell
From: Brandon Casey draf...@gmail.com These tests call test_set_editor to set an alternate editor script, but they appear to presume that the assignment is of a temporary nature and will not have any effect outside of each individual test. That is not the case. All of the test functions within a test script share a single environment, so any variables modified in one, are visible in the ones that follow. So, let's protect the test functions that follow these, which set an alternate editor, by performing the test_set_editor and 'git commit' in a subshell. Signed-off-by: Brandon Casey draf...@gmail.com --- Before git-commit: populate the edit buffer with 2 blank lines before s-o-b is merged, this is needed on top of rt/commit-cleanup-config 51fb3a3d so that the default EDITOR remains in effect for the new test. -Brandon t/t7502-commit.sh | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index b1c7648..520a5cd 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -255,32 +255,40 @@ test_expect_success 'cleanup commit message (fail on invalid cleanup mode config test_expect_success 'cleanup commit message (no config and no option uses default)' ' echo content file git add file - test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment - git commit --no-status + ( + test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment + git commit --no-status + ) commit_msg_is commit message ' test_expect_success 'cleanup commit message (option overrides default)' ' echo content file git add file - test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment - git commit --cleanup=whitespace --no-status + ( + test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment + git commit --cleanup=whitespace --no-status + ) commit_msg_is commit message # comment ' test_expect_success 'cleanup commit message (config overrides default)' ' echo content file git add file - test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment - git -c commit.cleanup=whitespace commit --no-status + ( + test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment + git -c commit.cleanup=whitespace commit --no-status + ) commit_msg_is commit message # comment ' test_expect_success 'cleanup commit message (option overrides config)' ' echo content file git add file - test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment - git -c commit.cleanup=whitespace commit --cleanup=default + ( + test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment + git -c commit.cleanup=whitespace commit --cleanup=default + ) commit_msg_is commit message ' -- 1.8.1.3.566.gaa39828 -- 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: Crashes while trying to show tag objects with bad timestamps
On Sat, Feb 23, 2013 at 1:04 AM, Jeff King p...@peff.net wrote: On Fri, Feb 22, 2013 at 02:53:48PM -0800, Junio C Hamano wrote: and no for disappointing. IIRC, in the very early implementations allowed tag object without dates. I _think_ we can start tightening fsck, though. Then I think it would make sense to allow the very specific no-date tag, but not allow arbitrary crud. I wonder if there's an example in the kernel or in git.git. I couldn't find any such examples. However, I did find several tags with no tagger line at all: git.git has v0.99 and linux.git has many such tags starting with v2.6.11 ending with v2.6.13-rc3. It seems that `git cat-file -p` doesn't like such tags too – if there is no tagger, it doesn't display *any* header lines. More bugs? -- Mantas Mikulėnas -- 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: Crashes while trying to show tag objects with bad timestamps
Jeff King p...@peff.net writes: On Fri, Feb 22, 2013 at 02:53:48PM -0800, Junio C Hamano wrote: I guess we should probably issue a warning, too. Also disappointingly, git-fsck does not seem to detect this breakage at all. Yes for the warning, Unfortunately, a good warning is harder than I had hoped. At the point where we notice the problem, pp_user_info, we have very little context. We can say only something like: warning: malformed date in ident 'Jeff King p...@peff.net BOGUS' but we cannot say in which object, or even that it was a tagger line (and in some cases we do not even have an object, as in make_cover_letter). As pp_user_info() is called from very few places, I do not think it is unreasonable to add an output parameter (i.e. unsigned *) to let the caller know that we made a best guess given malformed input and handle the error in the caller. The make_cover_letter() caller may look like: pp_user_info(pp, NULL, sb, committer, encoding, errors); if (errors PP_CORRUPT_DATE) warning(unparsable datestamp in '%s', committer); although it is unlikely to see this error in practice, given that committer is coming from git_committer_info(0) and would have the current timestamp. I also took a look at parsing routine of cat-file -p. It's totally hand-rolled, separate from what git show does, and is not build on the pretty-print code at all. I wonder, though, if it actually makes sense to munge the date there. The commit-object pretty-printer for cat-file just shows the object intact. It seems weirdly inconsistent that we would munge tags just to rewrite the date. If you want a real pretty-printer, you should be using porcelain like show. The whole cat-file -p is a historical wart, aka poor-man's show. I do not even consider it a part of the plumbing. It is a fair game for Porcelainisque improvement ;-) -- 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] t7502: perform commits using alternate editor in a subshell
Brandon Casey bca...@nvidia.com writes: From: Brandon Casey draf...@gmail.com These tests call test_set_editor to set an alternate editor script, but they appear to presume that the assignment is of a temporary nature and will not have any effect outside of each individual test. That is not the case. All of the test functions within a test script share a single environment, so any variables modified in one, are visible in the ones that follow. So, let's protect the test functions that follow these, which set an alternate editor, by performing the test_set_editor and 'git commit' in a subshell. Signed-off-by: Brandon Casey draf...@gmail.com --- Before git-commit: populate the edit buffer with 2 blank lines before s-o-b is merged, this is needed on top of rt/commit-cleanup-config 51fb3a3d so that the default EDITOR remains in effect for the new test. Yeah, what I already pushed out forces EDITOR=: for your test for the same effect, but this patch clearly takes us in the right (and better) direction. -Brandon t/t7502-commit.sh | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index b1c7648..520a5cd 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -255,32 +255,40 @@ test_expect_success 'cleanup commit message (fail on invalid cleanup mode config test_expect_success 'cleanup commit message (no config and no option uses default)' ' echo content file git add file - test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment - git commit --no-status + ( + test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment + git commit --no-status + ) commit_msg_is commit message ' test_expect_success 'cleanup commit message (option overrides default)' ' echo content file git add file - test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment - git commit --cleanup=whitespace --no-status + ( + test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment + git commit --cleanup=whitespace --no-status + ) commit_msg_is commit message # comment ' test_expect_success 'cleanup commit message (config overrides default)' ' echo content file git add file - test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment - git -c commit.cleanup=whitespace commit --no-status + ( + test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment + git -c commit.cleanup=whitespace commit --no-status + ) commit_msg_is commit message # comment ' test_expect_success 'cleanup commit message (option overrides config)' ' echo content file git add file - test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment - git -c commit.cleanup=whitespace commit --cleanup=default + ( + test_set_editor $TEST_DIRECTORY/t7500/add-content-and-comment + git -c commit.cleanup=whitespace commit --cleanup=default + ) commit_msg_is commit message ' -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What will be in 1.8.2-rc1
Just a quick update. I am planning to merge the following to 'master': * regression fixes and finishing touches to a new feature ct/autoconf-htmldir jn/less-reconfigure as/check-ignore * documentation updates wk/man-deny-current-branch-is-default-these-days wk/user-manual I'd also want to have this before the final as a regression fix. I haven't decided if I should do so before -rc1: mh/maint-ceil-absolute 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
[RFD] subtree status - comparing subtree with a remote
Hi, I'm looking at adding a git subtree status command that will tell if a subtree is up-to-date, ahead of, behind, divergant with or unrelated to a remote repo. I just wanted to check that I'm working this out correctly before writing the code. 1) perform a synthetic subtree split mine=$(git subtree split --prefix=subtree/path) This outputs the SHA1 for this subtree in isolation to the superproject. 2) fetch latest branch HEAD from remote repository we're comparing with git fetch $repo $branch theirs=$(git rev-parse FETCH_HEAD) 3) Find common ancestor base=$(git merge-base $mine $theirs) Where: * $base == $mine $base == $theirs : up-to-date * $base == $mine $base != $theirs : behind remote - can pull * $base != $mine $base == $theirs : ahead of remote - can push * $base != $mine $base != $theirs : divergent * $base == null : no common ancestor - wrong repo? Comments? -- Paul [W] Campbell -- 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 checkout problem
I was on my master branch, I checked out a branch (origin/somebranch), did nothing, made no updates but did a few git pulls over about a week; made a small change to one file comitted pushed. Now am trying to go back to my master branch and get: error: The following untracked working tree files would be overwritten by checkout: lib/derbyclient.jar Please move or remove them before you can switch branches. Aborting I did not put that file there, how do I get back to my master branch? I do not want to muck up the branch that I am now one. Obviously someone put derbyclient.jar there, not sure, but it is supposed to be there so do not want to remove. any ideas? -- 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: Suggested improvements to the git-p4 documentation (branch-related)
Olivier Delalleau sh...@keba.be writes: 2013/1/5 Pete Wyckoff p...@padd.com: sh...@keba.be wrote on Thu, 03 Jan 2013 15:58 -0500: ... Please do feel welcome to to rearrange or expand the documentation so it makes more sense, if you are so inspired. I'm afraid I'm not familiar enough with git documentation to dig into it myself, but anyway that's about what I had for now. I'll send more comments to the mailing list if I have more suggestions in the future. Thanks for a great tool! :) Did anything come out of this thread? If neither of you two are inclined to conclude the discussion with a final patch, I'd appreciate anybody else who does the honors ;-) We'll be in deep pre-release freeze for a few weeks, so there is no need to rush. 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
[PATCH 3/2] update-index: list supported idx versions and their features
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- .. and the user should know (briefly) the differences between index versions too. Documentation/git-update-index.txt | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index 77a912d..e5aaba5 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -145,7 +145,15 @@ you will need to handle the situation manually. --index-version n:: Write the resulting index out in the named on-disk format version. - The current default version is 2. + Supported versions are 2, 3 and 4. The current default version is 2 + or 3, depending on whether extra features are used, such as + `git add -N`. ++ + Version 4 performs a simple pathname compression that could + reduce index size by 30%-50% on large repositories, which + results in faster load time. Version 4 is relatively young + (first released in 1.8.0 in October 2012). Other Git + implementations may not support it yet. -z:: Only meaningful with `--stdin` or `--index-info`; paths are -- 1.8.1.2.536.gf441e6d -- 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 3/2] update-index: list supported idx versions and their features
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Oops, bogus indentation in the first 3/2 Documentation/git-update-index.txt | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index 77a912d..dbb75f4 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -145,7 +145,14 @@ you will need to handle the situation manually. --index-version n:: Write the resulting index out in the named on-disk format version. - The current default version is 2. + Supported versions are 2, 3 and 4. The current default version is 2 + or 3, depending on whether extra features are used, such as + `git add -N`. ++ +Version 4 performs a simple pathname compression that could reduce +index size by 30%-50% on large repositories, which results in faster +load time. Version 4 is relatively young (first released in in 1.8.0 +in October 2012). Other Git implementations may not support it yet. -z:: Only meaningful with `--stdin` or `--index-info`; paths are -- 1.8.1.2.536.gf441e6d -- 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] add: allow users to silence Git 2.0 warnings about add -u
On Fri, Feb 22, 2013 at 9:30 AM, Junio C Hamano gits...@pobox.com wrote: Matthieu Moy matthieu@grenoble-inp.fr writes: Yes, but push.default is really different: there is a config variable, and we want the behavior to be configurable. In the case of git add, I don't think adding a configuration option would be the right thing. That would mean typing git add -u on an account which isn't yours will be unpredictable *forever*. Exactly. I completely agree. We don't want that [1]. I'm actually a big enemy of configuration, don't get me wrong. The real point of the patch I sent was to start a conversation about the thing I actually care about: After reading the draft release notes I now realize that git add -u will not die() in Git 2.0. It will operate on the full tree, as described in the note. Sweet. I was originally concerned that git add -u was going to die() and we would no longer be able to use it without pathspec. My concerns were unfounded. (If I am not understanding this correctly then it is a sign that the draft release notes can be made more clear) Another problem with use2dot0 configuration is that it would invite people to imagine that setting it to false will keep the old behaviour forever, which is against what you set out to do with the patch under discussion. I agree with both sides. There's the part of me that wants the 2.0 behavior now with a config switch for the same reasons as was discussed earlier: http://thread.gmane.org/gmane.comp.version-control.git/166223/focus=168238 In addition, mindful users would see one less warning, which is the only weight I've added to that side of the discussion. We would never want to go back to the old behavior when 2.0 roll around. Jakub's future.wholeTree suggestion makes sense in that context as the entire future.* namespace could be designated as variables with these semantics. One downside is that adding such a configuration is just more temporary code to maintain and rip out when 2.0 rolls around. OTOH a positive thing about adding configuration to get the better behavior is that the code path materializes sooner, and it will be better exercised before 2.0. This increases confidence that removing the false side of the imaginary future.fullTree configuration will be harmless. In the original-original thread Matthieu and I seemed to agree that configuration to get the new behavior (but not get the old behavior) would be nice. Peff went even farther and suggested that having a way to keep the old behavior would be good, and I agree that this is the thing [1] to avoid since it makes the command forever unpredictable. future.* means the ambiguous/unpredictable behavior does eventually go away. It's a flag day, there's no way around that. Script writers will be hurt, there is no escaping that. I guess the real question is whether it's a flag day that happens through availability of configuration, or by the inevitability of 2.0. I have one scenario where future.fullTree would be helpful to script writers: it would allow them to test their scripts with the new behavior and back it out if their scripts break. This gives them more time to make the tiny change needed to be portable across different git versions, which helps make the later default change into much less of an event. Having such a configuration would probably mean that git should probably warn for all pathless git add -u, even from the root. It will help usher users towards the new behavior. The current behavior makes the most sense since we do not have a config variable. The current behavior is certainly the simplest. I don't know what we can do about the clueless user on stackoverflow that follows the first suggestion to set the future.fullTree variable. My gut feeling is that optimizing for them is a lost cause. Providing a way for mindful users to ease themselves into the new behavior does help them, and git is certainly the tool of choice for the mindful user. ;-) I hope I haven't misrepresented anybody's opinions. If I'm the only one who thinks that future.fullTree is a good idea then I have no problem with the current behavior since the noisy warning will be gone in 2.0. Does anyone else have any weight to add to either side? -- David -- 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
