[PATCH v2] blame: CRLF in the working tree and LF in the repo
A typicall setup under Windows: core.eol is CRLF and a file is marked as "text" in .gitattributes, or core.autocrlf is true After 4d4813a5 "git blame" no longer works as expected, every line is annotated as "Not Committed Yet", even though the working directory is clean. commit 4d4813a5 removed the conversion in blame.c for all files, with or without CRLF in the repo. Having files with CRLF in the repo and core.autocrlf=input is a temporary situation, the files should be normalized in the repo. Blaming them with "Not Committed Yet" is OK. The solution is to revert commit 4d4813a5. Reported-By: Stepan Kasal Signed-off-by: Torsten Bögershausen --- What happened to the test cases ? They where improved, but it turned out that they failed from time to time, both Windows and Mac OS. "Spicing them" with test_tick or other things didn't help, if they are run in a tight loop. running with "debug=t verbose=t ./t8003*.sh" make the test pass. My suggestion is to revert 4d4813a5 and drop TC's for the moment. builtin/blame.c | 1 + t/t8003-blame-corner-cases.sh | 9 - 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 06484c2..8d70623 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2348,6 +2348,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt, if (strbuf_read(&buf, 0, 0) < 0) die_errno("failed to read from stdin"); } + convert_to_git(path, buf.buf, buf.len, &buf, 0); origin->file.ptr = buf.buf; origin->file.size = buf.len; pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1); diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh index 32895e5..a073d36 100755 --- a/t/t8003-blame-corner-cases.sh +++ b/t/t8003-blame-corner-cases.sh @@ -191,13 +191,4 @@ test_expect_success 'indent of line numbers, ten lines' ' test $(grep -c " " actual) = 9 ' -test_expect_success 'blaming files with CRLF newlines' ' - git config core.autocrlf false && - printf "testcase\r\n" >crlffile && - git add crlffile && - git commit -m testcase && - git -c core.autocrlf=input blame crlffile >actual && - grep "A U Thor" actual -' - test_done -- 2.2.0.rc1.790.ge19fcd2 -- 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 v9 1/5] sha1_file: support reading from a loose object of unknown type
On 04/30/2015 01:05 AM, Junio C Hamano wrote: karthik nayak writes: > On 04/29/2015 08:19 PM, Junio C Hamano wrote: >> Karthik Nayak writes: >> >>> Update sha1_loose_object_info() to optionally allow it to read >>> from a loose object file of unknown/bogus type; as the function >>> usually returns the type of the object it read in the form of enum >>> for known types, add an optional "typename" field to receive the >>> name of the type in textual form and a flag to indicate the reading >>> of a loose object file of unknown/bogus type. >>> >>> Add parse_sha1_header_extended() which acts as a wrapper around >>> parse_sha1_header() allowing more information to be obtained. >> >> Thanks. This mostly looks good modulo a nit. > > Sorry didn't get what you meant by "modulo a nit.". "nit" as in "Nit-pick"; a small imperfection that may need to be corrected (such as the "what if we saw failure earlier and 'status' already had a value?" issue). Thanks for clearing that out. >> It is a good trade-off between complexity and efficiency. The >> complexity is isolated as the function is file-scope-static and it >> is perfectly fine to force the callers to be extra careful. >> >> But this suggests that the patch to add tests should try at least >> two, preferably three, kinds of test input. A bogus type that needs >> a header longer than the caller's fixed buffer, a bogus type whose >> header would fit within the fixed buffer, and optionally a correct >> type whose header should always fit within the fixed buffer. > > Yes it is a tradeoff, and it is complex as in the user has to check > the strbuf provided to see if its been used. But this like you said I > guess its a good tradeoff. > About the three tests, My patch checks "a bogus type whose header > would fit within the fixed buffer" and "correct type whose header > should always fit within the fixed buffer" but will write a test to > check "A bogus type that needs a header longer than the caller's fixed > buffer" Yup. Please do so; that would make the test coverage more complete. Yup will do :) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: CURLOPT_NOBODY
On Wed, Apr 29, 2015 at 11:20:55PM -0300, Thiago Farina wrote: > Do we need to set CURLOPT_NOBODY to 0 in > https://code.googlesource.com/git/+/master/http.c#1138? Do we do this > for the sake of doing, because it doesn't hurt? > > According to the documentation in > http://curl.haxx.se/libcurl/c/CURLOPT_HTTPGET.html, if we set HTTPGET > to 1 it will automatically set NOBODY to 0, so the answer for the > above question would be no. It may have been necessary at one time... Running "git blame" on the curl repository's lib/url.c shows that the behavior started in 726b9e2, which is in curl 7.14.1, released in 2005. Grepping for LIBCURL_VERSION_NUM in git, we definitely support versions older than that. Most of those version checks are quite old, too, and we could probably stop supporting antique versions of curl. But unless there is a compelling benefit (e.g., we get to clean up some old cruft), I'd rather leave things as-is. Dropping this one line does not seem like a compelling cleanup to me, though it's possible if we said "you must have curl from the last 5 years" we could do other cleanups, and this would come along for the ride. > Also, according to http://curl.haxx.se/libcurl/c/CURLOPT_NOBODY.html, > it is 0 by default. We reuse curl handles, so we reinitialize the request-specific options for each request. -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: RFC: git cat-file --follow-symlinks?
On Wed, 2015-04-29 at 21:16 -0400, Jeff King wrote: > On Wed, Apr 29, 2015 at 06:06:23PM -0700, David Turner wrote: > 3. Ditto for out-of-tree. Note that this would be the _raw_ symlink > contents, not any kind of simplification (so if you asked for > "foo/bar/baz" and it was "../../../../out", you would the full path > with all those dots, not a simplified "../out", which I think is > what you were trying to show in earlier examples). Unfortunately, we need the simplified version, because we otherwise don't know what the ..s are relative to in the case of a link to a link: echo content >dest ;# actual blob mkdir -p foo/bar ln -s foo/bar/baz fleem # in-tree link-to-link ln -s ../../../external foo/bar/baz # out-of-tree link If echo HEAD^{resolve}:fleem were to return ../../../external (after following the first symlink to the second), we would have lost information. -- 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 (Apr 2015, #04; Mon, 27)
Koosha Khajehmoogahi writes: > Sorry for the delay. I will send a new reroll ASAP. No rush. I just wanted to make sure none of these is abandoned (and drop any that is). 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: What's cooking in git.git (Apr 2015, #04; Mon, 27)
Paul Tan writes: > Hi Junio, > > On Thu, Apr 30, 2015 at 6:42 AM, Junio C Hamano wrote: >> Junio C Hamano writes: >>> * pt/xdg-config-path (2015-04-12) 7 commits >>> - path.c: remove home_config_paths() >>> - git-config: replace use of home_config_paths() >>> - git-commit: replace use of home_config_paths() >>> - credential-store.c: replace home_config_paths() with xdg_config_home() >>> - dir.c: replace home_config_paths() with xdg_config_home() >>> - attr.c: replace home_config_paths() with xdg_config_home() >>> - path.c: implement xdg_config_home() >>> (this branch uses pt/credential-xdg.) >>> >>> Seen some discussions. >>> Waiting for a reroll ($gmane/267518). > > Only the first patch of the series needed changes, though I'm waiting > for any final reviews. Do you need me to resend the other patches? I'd prefer the final submission to be a full series to avoid mistakes on my end. Just to be sure, there is no rush. I just wanted to make sure none of these is abandoned (and drop any that is). 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: git cat-file --follow-symlinks?
Jeff King writes: > I had imagined we would stop resolution and you would just get the last > object peeled object. Combined with teaching cat-file to show more > object context, doing: > > echo content >dest ;# actual blob > ln -s dest link;# link to blob > ln -s broken foo ;# broken link > ln -s out ../foo ;# out-of-tree link > git add . && git commit -m foo > for i in link broken out; do > echo HEAD^{resolve}:$i > done | > git cat-file --batch="%(intreemode) %(size)" > > would yield: > > (1) 100644 8 >content > (2) 04 3 >foo > (3) 04 6 >../foo > > where the left-margin numbers are for reference: > > 1. We dereference a real symlink, and pretend like we actually asked > for its referent. > > 2. For a broken link, we can't dereference, so we return the link > itself. You can tell by the mode, and the content tells you what > would have been dereferenced. > > 3. Ditto for out-of-tree. Note that this would be the _raw_ symlink > contents, not any kind of simplification (so if you asked for > "foo/bar/baz" and it was "../../../../out", you would the full path > with all those dots, not a simplified "../out", which I think is > what you were trying to show in earlier examples). s/04/16/ I would think (if you really meant to expose a tree, write it as 4 instead, so that people will not get a wrong impression and reimplement a broken tree object encoding some popular Git hosting site broke their customer projects with ;-). I am not sure $treeish^{resolve} is a great syntax, but I like the concept and agree that it is a lot more sensible to handle this at the level of sha1_name.c layer than an ad-hoc solution in the cat-file layer. -- 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: git cat-file --follow-symlinks?
On Wed, 2015-04-29 at 20:37 -0400, Jeff King wrote: > On Wed, Apr 29, 2015 at 07:11:50PM -0400, Jeff King wrote: > > > Yeah, I agree if you let git punt on leaving the filesystem, most of the > > complicated problems go away. It still feels a bit more magical than I > > expect out of cat-file, and there are still corner cases (e.g., do we do > > cycle detection? Or just have a limit to the recursion depth?) > > I was pondering the "magical" above. I think what bugs me is that it > seems like a feature that is implemented as part of one random bit of > plumbing, but not available elsewhere. > > Conceptually, this is like peeling object names. You may give a tag > name, but if you ask for a tree commit we will peel the tag to a commit, > and the commit to a tree. This is sort of the same thing; you give a > path within a tree, and we will peel until we hit a "real" non-symlink > object. > > I don't know what the syntax would look like. To match "foo^{tree}" it > would be something like: > > HEAD:foo/bar^{resolve} > > or something like that. Except that it is a bad idea to allow "^{}" > syntax on the right-hand side of a colon, as it is ambiguous with > filenames that contain "^{resolve}". So it would have to look something > like: > > HEAD^{resolve}:foo/bar > > which is a _little_ weird, but actually kind of makes sense. The > "resolve" operation inherently is not just about the filename, but about > uses HEAD^{tree} as the root context. > > So I dunno. This pushes the resolving logic even _lower_ in the stack > than it would be in cat-file. So why do I like it more? Cognitive > dissonance? I guess I the appeal to me is that it: > > 1. Makes the concept available more generally (you can "rev-parse" it, > you can "git show" it, etc). It also lets you _name_ the object in > question, so you can ask for other things besides it contents (like > its name, its type, etc). > > 2. Positions it alongside other peeling name-resolution functions. Just to clarify: if you do git rev-parse, and the result is an out-of-tree symlink, you see /foo or ../foo instead of a sha? And if you "git show" it it says "symlink HEAD:../foo"? This seems totally reasonable to me, and solves my 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
Re: What's cooking in git.git (Apr 2015, #04; Mon, 27)
On Wed, Apr 29, 2015 at 03:42:57PM -0700, Junio C Hamano wrote: > > * jk/at-push-sha1 (2015-03-31) 6 commits > > - sha1_name: implement @{push} shorthand > > - sha1_name: refactor upstream_mark > > - remote.c: provide per-branch pushremote name > > - remote.c: hoist branch.*.remote lookup out of remote_get_1 > > - remote.c: drop "remote" pointer from "struct branch" > > - remote.c: drop default_remote_name variable > > > > Introduce @{push} short-hand to denote the remote-tracking > > branch that tracks the branch at the remote the would be > > pushed to. > > > > Waiting for a reroll ($gmane/266573). I re-rolled this and _almost_ sent it out last week. But I noticed that it gives us only "git rev-parse foo@{push}" and not "git for-each-ref --format=%(push)" (whereas we have "upstream" for both versions). For "upstream", computing the answer is simple enough that the tiny bit of logic is largely duplicated in the two spots. For @{push}, that would be a bad idea. So I started refactoring the final patch to use the same logic in both spots, but didn't finish. I can send the intermediate version (i.e., the re-roll with a few minor fixups based on list comments), and we can build the other on top, but I don't think there's any rush, and it can wait for the refactor (which shouldn't be _too_ bad, I don't think). -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: What's cooking in git.git (Apr 2015, #04; Mon, 27)
Hi Junio, On Thu, Apr 30, 2015 at 6:42 AM, Junio C Hamano wrote: > Junio C Hamano writes: >> * pt/xdg-config-path (2015-04-12) 7 commits >> - path.c: remove home_config_paths() >> - git-config: replace use of home_config_paths() >> - git-commit: replace use of home_config_paths() >> - credential-store.c: replace home_config_paths() with xdg_config_home() >> - dir.c: replace home_config_paths() with xdg_config_home() >> - attr.c: replace home_config_paths() with xdg_config_home() >> - path.c: implement xdg_config_home() >> (this branch uses pt/credential-xdg.) >> >> Seen some discussions. >> Waiting for a reroll ($gmane/267518). Only the first patch of the series needed changes, though I'm waiting for any final reviews. Do you need me to resend the other patches? Thanks, Paul -- 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: git cat-file --follow-symlinks?
Jeff King wrote: > 1. Git has to make a decision about what to do in corner cases. What > is our cwd for relative links? The project root? I don't follow. Isn't symlink resolution always relative to the symlink, regardless of cwd? Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/15] merge: decide if we auto-generate the message early in collect_parents()
Signed-off-by: Junio C Hamano --- builtin/merge.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index 9f98538..eb3be68 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1098,6 +1098,10 @@ static struct commit_list *collect_parents(struct commit *head_commit, int i; struct commit_list *remoteheads = NULL; struct commit_list **remotes = &remoteheads; + struct strbuf merge_names = STRBUF_INIT, *autogen = NULL; + + if (merge_msg && (!have_message || shortlog_len)) + autogen = &merge_names; if (head_commit) remotes = &commit_list_insert(head_commit, remotes)->next; @@ -,15 +1115,13 @@ static struct commit_list *collect_parents(struct commit *head_commit, remoteheads = reduce_parents(head_commit, head_subsumed, remoteheads); - if (merge_msg && - (!have_message || shortlog_len)) { - struct strbuf merge_names = STRBUF_INIT; + if (autogen) { struct commit_list *p; - for (p = remoteheads; p; p = p->next) - merge_name(merge_remote_util(p->item)->name, &merge_names); - prepare_merge_message(&merge_names, merge_msg); - strbuf_release(&merge_names); + merge_name(merge_remote_util(p->item)->name, autogen); + + prepare_merge_message(autogen, merge_msg); + strbuf_release(autogen); } return remoteheads; -- 2.4.0-rc3-300-g052d062 -- 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 00/15] Teach "git merge FETCH_HEAD" octopus merges
This series is an attempt to make these two operations truly equivalent: $ git pull . topic-a topic-b... $ git fetch . topic-a topic-b... $ git merge FETCH_HEAD Compared to the previous one ($gmane/267809), there are only a few minor changes: - The first patch is new; it adds tests to illustrate what the current code does. Accordingly, the penultimate patch is the same as the previous, but updates the tests that expect failure in this step to expect success. - One step failed to compile (the offending code was removed in a later patch and the endgame did not break the build), which has been corrected. Junio C Hamano (15): merge: test the top-level merge driver merge: simplify code flow t5520: style fixes t5520: test pulling an octopus into an unborn branch merge: clarify "pulling into void" special case merge: do not check argc to determine number of remote heads merge: small leakfix and code simplification merge: clarify collect_parents() logic merge: split reduce_parents() out of collect_parents() merge: narrow scope of merge_names merge: extract prepare_merge_message() logic out merge: make collect_parents() auto-generate the merge message merge: decide if we auto-generate the message early in collect_parents() merge: handle FETCH_HEAD internally merge: deprecate 'git merge HEAD ' syntax Documentation/git-merge.txt | 4 + builtin/merge.c | 248 +++--- git-cvsimport.perl| 2 +- git-pull.sh | 3 +- t/t3033-merge-toplevel.sh | 136 +++ t/t3402-rebase-merge.sh | 2 +- t/t5520-pull.sh | 31 +++--- t/t6020-merge-df.sh | 2 +- t/t6021-merge-criss-cross.sh | 6 +- t/t9402-git-cvsserver-refs.sh | 2 +- 10 files changed, 324 insertions(+), 112 deletions(-) create mode 100755 t/t3033-merge-toplevel.sh -- 2.4.0-rc3-300-g052d062 -- 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: git cat-file --follow-symlinks?
On Wed, 2015-04-29 at 14:16 -0700, Jonathan Nieder wrote: > Hi, > > David Turner wrote: > > > Instead, it would be cool if cat-file had a mode in which it would > > follow symlinks. > > Makes sense. > > > The major wrinkle is that symlinks can point outside the repository -- > > either because they are absolute paths, or because they are relative > > paths with enough ../ in them. For this case, I propose that > > --follow-symlinks should output [sha] "symlink" [target] instead of the > > usual [sha] "blob" [bytes]. > > What happens when the symlink payload contains a newline? Oh, right. So, how about [sha] "symlink" [bytes] "\n" [target] instead? -- 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 v9 5/5] t1006: add tests for git cat-file --allow-unkown-type
On Wed, Apr 29, 2015 at 8:54 AM, Karthik Nayak wrote: > Signed-off-by: Karthik Nayak > --- > diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh > index ab36b1e..8362019 100755 > --- a/t/t1006-cat-file.sh > +++ b/t/t1006-cat-file.sh > @@ -47,6 +47,18 @@ $content" > test_cmp expect actual > ' > > +test_expect_success "Type of $type is correct using --allow-unkown-type" > ' > + echo $type >expect && > +git cat-file -t --allow-unkown-type $sha1 >actual && Somehow the indentation got botched here and in the remaining tests you added. This issue is new since v8. > + test_cmp expect actual > +' > + > +test_expect_success "Size of $type is correct using --allow-unkown-type" > ' > + echo $size >expect && > +git cat-file -s --allow-unkown-type $sha1 >actual && > + test_cmp expect actual > +' > + > test -z "$content" || > test_expect_success "Content of $type is correct" ' > maybe_remove_timestamp "$content" $no_ts >expect && > @@ -296,4 +308,21 @@ test_expect_success '%(deltabase) reports packed delta > bases' ' > } > ' > > +bogus_type="bogus" > +bogus_content="bogus" > +bogus_size=$(strlen "$bogus_content") > +bogus_sha1=$(echo_without_newline "$bogus_content" | git hash-object -t > $bogus_type --literally -w --stdin) > + > +test_expect_success "Type of broken object is correct" ' > + echo $bogus_type >expect && > +git cat-file -t --allow-unkown-type $bogus_sha1 >actual && > + test_cmp expect actual > +' > + > +test_expect_success "Size of broken object is correct" ' > + echo $bogus_size >expect && > +git cat-file -s --allow-unkown-type $bogus_sha1 >actual && > + test_cmp expect actual > +' > + > test_done > -- > 2.4.0.rc1.250.g565e85b -- 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 v9 4/5] cat-file: add documentation for '--allow-unkown-type' option.
On Wed, Apr 29, 2015 at 8:56 AM, Karthik Nayak wrote: > cat-file: add documentation for '--allow-unkown-type' option. Drop the end-of-line period. > Signed-off-by: Karthik Nayak It's not clear why this change is done separately from patch 3/5 (cat-file: teach cat-file a '--allow-unknown-type' option). Given how small this patch is and considering how closely related it is to the actual introduction of the --allow-unknown-type option, it might make sense to fold it into 3/5. > --- > diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt > index f6a16f4..f6f6064 100644 > --- a/Documentation/git-cat-file.txt > +++ b/Documentation/git-cat-file.txt > @@ -9,7 +9,7 @@ git-cat-file - Provide content or type and size information > for repository objec > SYNOPSIS > > [verse] > -'git cat-file' (-t | -s | -e | -p | | --textconv ) > +'git cat-file' (-t [--allow-unkown-type]| -s [--allow-unkown-type]| -e | -p > | | --textconv ) > 'git cat-file' (--batch | --batch-check) < > > DESCRIPTION > @@ -69,6 +69,9 @@ OPTIONS > not be combined with any other options or arguments. See the > section `BATCH OUTPUT` below for details. > > +--allow-unkown-type:: > + Allow -s or -t to query broken/corrupt objects of unknown type. > + > OUTPUT > -- > If '-t' is specified, one of the . > -- > 2.4.0.rc1.250.g565e85b -- 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] Documentation: Fix inconsistent quotes
On Wed, Apr 29, 2015 at 12:09:46PM -0700, Jonathan Nieder wrote: > Hi, > > Jeff King wrote: > > > But IMHO, using backticks looks much better. In the roff-formatted > > manpages single quotes underline, but backticks use bold. > > Are you sure? My copy of git.1.gz has backticks converted into no > formatting at all: > > Other options are available to control how the manual page is > displayed\&. See > \fBgit-help\fR(1) > for more information, because > git \-\-help \&.\&.\&. > is converted internally into > git help \&.\&.\&.\&. It's actually optional. See 5121a6d (Documentation: option to render literal text as bold for manpages, 2009-03-27). I don't see a good reason that wasn't made the default early, except conservatism. I've had it enabled for years (though I admit I don't read the manpages that much these days :) ). -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] rebase -i: redo tasks that die during cherry-pick
Phil Hord writes: > On Wed, Apr 29, 2015 at 1:15 PM, Junio C Hamano wrote: >> >> Thanks, will queue. >> >> Aside from the "much more invasive" possibility, the patch makes me >> wonder if it would have been a better design to have a static "todo" >> with a "current" pointer as two state files. Then reschedule would >> have been just the matter of decrementing the number in "current", >> instead of "grab the last line of one file and prepend to the other >> file, and then lose the last line". > > That's an interesting idea. Changing it now would impact anyone who > now depends on the current todo/done behavior, and I imagine there > are many. Yeah, in case it wasn't clear, I was merely lamenting over water under the bridge, not seriously suggesting to break users to simplify our logic. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase -i: redo tasks that die during cherry-pick
On Wed, Apr 29, 2015 at 1:15 PM, Junio C Hamano wrote: > > Thanks, will queue. > > Aside from the "much more invasive" possibility, the patch makes me > wonder if it would have been a better design to have a static "todo" > with a "current" pointer as two state files. Then reschedule would > have been just the matter of decrementing the number in "current", > instead of "grab the last line of one file and prepend to the other > file, and then lose the last line". That's an interesting idea. Changing it now would impact anyone who now depends on the current todo/done behavior, and I imagine there are many. -- 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] Documentation: Fix inconsistent quotes
On Wed, Apr 29, 2015 at 08:08:52PM +0200, Stefan Tatschner wrote: > While reading 'man git' I realized that the highlighting of the > environment variables is not consistent. This patch adds missing single > quotes and substitutes backticks with the proper quotes as well. I think this is OK in that it makes things consistent, and chooses the style that is already in the majority. But IMHO, using backticks looks much better. In the roff-formatted manpages single quotes underline, but backticks use bold. The former is hard to read when the content has underscores. For the HTML, the single quotes produce italics, versus a typewriter font for backticks. Which is...OK, I guess, but I think I like the backtick behavior better. I think we're wildly inconsistent about this throughout the documentation, though. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/5] cat-file: teach cat-file a '--allow-unknown-type' option
On 04/29/2015 08:23 PM, Phil Hord wrote: On Wed, Apr 29, 2015 at 9:01 AM Karthik Nayak wrote: > > Currently 'git cat-file' throws an error while trying to > print the type or size of a broken/corrupt object. This is > because these objects are usually of unknown types. > > Teach git cat-file a '--allow-unkown-type' option where it prints > the type or size of a broken/corrupt object without throwing > an error. In this entire series, replace all 'unkown' with 'unknown' in both the commit messages and the code ("unknown" is misspelled most of the time). I notice the switch name itself is misspelled, but also variable names such as 'unkown_type' in this patch. Respectfully, because I know English is a challenging beast sometimes, and spelling is difficult even for many native speakers... Thanks for that, Yes it does get a bit tricky with spellings, will find an editor with spellcheck and avoid nano :D -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug report : bad filter-branch (OSX only)
On Wed, Apr 29, 2015 at 09:30:00AM -0700, Junio C Hamano wrote: > >> ( > >>while read x && test -n "$x" > >> do > >>:; > >>done > >>cat > >> ) <../commit | eval "$filter_msg" > >> > >> would not spin too much in shell loop, perhaps? > > > > Yeah, that is not too bad. Probably we want "read -r", just in case of > > weirdness in the header lines (and that's in POSIX, and we use it > > in other scripts, so it should be portable enough). And we can save a > > subshell if we don't mind the potential variable-name conflict. > > As all we care about is "have we hit an empty line", I do not think "-r" > really matters, but it would not hurt. I think something like: author ... committer ... encoding foo\ this is the real commit message would behave incorrectly without "-r". I would be shocked if that ever happens in real life, but I think it doesn't hurt to be careful. > As to s/()/{}/, please tell me what I am doing wrong. I am getting > the same process IDs from all of the $$s and the only difference > seems to be variable clobbering. $$ is always the pid of the main shell process, even in a subshell. If your shell is bash, it provides $BASHPID which can tell the difference (if you put $BASHPID in your test script, it does show that we fork for the subshell). On Linux, you can also test with "strace -fce clone". Interestingly, dash produces one fewer fork than bash on your test script, but I didn't track down the exact difference. But I can imagine a shell that is smart enough to realize a fork is not required in this instance. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug report : bad filter-branch (OSX only)
Jeff King writes: > On Tue, Apr 28, 2015 at 10:39:44PM -0700, Junio C Hamano wrote: > >> Jeff King writes: >> >> > I'm not sure of a solution short of replacing the use of sed here with >> > something else. perl would be a simple choice, but filter-branch does >> > not otherwise depend on it. We could use a shell "read" loop, but those >> > are quite slow (and filter-branch is slow enough as it is!). >> >> You need to only skip the header part, right? >> I would imagine that >> >> ( >> while read x && test -n "$x" >> do >> :; >> done >> cat >> ) <../commit | eval "$filter_msg" >> >> would not spin too much in shell loop, perhaps? > > Yeah, that is not too bad. Probably we want "read -r", just in case of > weirdness in the header lines (and that's in POSIX, and we use it > in other scripts, so it should be portable enough). And we can save a > subshell if we don't mind the potential variable-name conflict. As all we care about is "have we hit an empty line", I do not think "-r" really matters, but it would not hurt. As to s/()/{}/, please tell me what I am doing wrong. I am getting the same process IDs from all of the $$s and the only difference seems to be variable clobbering. -- >8 -- #!/bin/sh cat >/var/tmp/tester <" x=foo { echo "inside brace $$" while read x && test -n "$x" do :; done cat } " -- 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: Regular Rebase Failure
> On Mon, Apr 27, 2015 at 10:07 AM, Adam Steel wrote: >> Stefan, >> >> So I switched git versions. >> >> $ git --version >> git version 2.3.1 >> >> I'm still getting the same regular rebase failures. >> >> --- >> >> fatal: Unable to create >> '/Users/asteel/Repositories/rails-teespring/.git/index.lock': File >> exists. Is the repository located on a mounted network share, or could other users be accessing it via a network mount? We had a similar problem recently on a new Jenkins VM instance which had only NFS-mounted storage available. I don't remember if it was Git that was failing on there, and I wasn't directly involved in solving the problem. But while researching the issue I found ominous warnings about the dangers of file-locking on remote shares [1]. Which is to say, I don't know much, but I heard a rumor... :-) Perhaps this is old news and already well covered in Git. But I am curious... [1] http://0pointer.de/blog/projects/locking.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/5] cat-file: teach cat-file a '--allow-unknown-type' option
Karthik Nayak writes: > Currently 'git cat-file' throws an error while trying to > print the type or size of a broken/corrupt object. This is > because these objects are usually of unknown types. > > Teach git cat-file a '--allow-unkown-type' option where it prints > the type or size of a broken/corrupt object without throwing > an error. Drop "Currently" from the description of the problem you are solving. We know that the problem you have to solve in the code is current without being told. This comment applies to all patches. > Modify '-t' and '-s' options to call sha1_object_info_extended() > directly to support the '--allow-unkown-type' option. > > Helped-by: Junio C Hamano > Helped-by: Eric Sunshine > Signed-off-by: Karthik Nayak > --- > builtin/cat-file.c | 38 ++ > 1 file changed, 26 insertions(+), 12 deletions(-) > > diff --git a/builtin/cat-file.c b/builtin/cat-file.c > index 53b5376..299e2e5 100644 > --- a/builtin/cat-file.c > +++ b/builtin/cat-file.c > @@ -9,13 +9,20 @@ > #include "userdiff.h" > #include "streaming.h" > > -static int cat_one_file(int opt, const char *exp_type, const char *obj_name) > +static int cat_one_file(int opt, const char *exp_type, const char *obj_name, > + int unkown_type) > { > unsigned char sha1[20]; > enum object_type type; > char *buf; > unsigned long size; > struct object_context obj_context; > + struct object_info oi = {NULL}; > + struct strbuf sb = STRBUF_INIT; > + unsigned flags = LOOKUP_REPLACE_OBJECT; > + > + if (unkown_type) > + flags |= LOOKUP_UNKNOWN_OBJECT; > > if (get_sha1_with_context(obj_name, 0, sha1, &obj_context)) > die("Not a valid object name %s", obj_name); > @@ -23,20 +30,22 @@ static int cat_one_file(int opt, const char *exp_type, > const char *obj_name) > buf = NULL; > switch (opt) { > case 't': > - type = sha1_object_info(sha1, NULL); > - if (type > 0) { > - printf("%s\n", typename(type)); > + oi.typename = &sb; > + if (sha1_object_info_extended(sha1, &oi, flags) < 0) > + die("git cat-file: could not get object info"); > + if (sb.len) { > + printf("%s\n", sb.buf); > + strbuf_release(&sb); > return 0; > } > break; > > case 's': > - type = sha1_object_info(sha1, &size); > - if (type > 0) { > - printf("%lu\n", size); > - return 0; > - } > - break; > + oi.sizep = &size; > + if (sha1_object_info_extended(sha1, &oi, flags) < 0) > + die("git cat-file: could not get object info"); > + printf("%lu\n", size); > + return 0; > > case 'e': > return !has_sha1_file(sha1); > @@ -323,7 +332,7 @@ static int batch_objects(struct batch_options *opt) > } > > static const char * const cat_file_usage[] = { > - N_("git cat-file (-t | -s | -e | -p | | --textconv) "), > + N_("git cat-file (-t [--allow-unkown-type]|-s > [--allow-unkown-type]|-e|-p||--textconv) "), > N_("git cat-file (--batch | --batch-check) < "), > NULL > }; > @@ -359,6 +368,7 @@ int cmd_cat_file(int argc, const char **argv, const char > *prefix) > int opt = 0; > const char *exp_type = NULL, *obj_name = NULL; > struct batch_options batch = {0}; > + int unkown_type = 0; > > const struct option options[] = { > OPT_GROUP(N_(" can be one of: blob, tree, commit, tag")), > @@ -369,6 +379,8 @@ int cmd_cat_file(int argc, const char **argv, const char > *prefix) > OPT_CMDMODE('p', NULL, &opt, N_("pretty-print object's > content"), 'p'), > OPT_CMDMODE(0, "textconv", &opt, > N_("for blob objects, run textconv on object's > content"), 'c'), > + OPT_BOOL( 0, "allow-unkown-type", &unkown_type, > + N_("allow -s and -t to work with broken/corrupt > objects")), > { OPTION_CALLBACK, 0, "batch", &batch, "format", > N_("show info and content of objects fed from the > standard input"), > PARSE_OPT_OPTARG, batch_option_callback }, > @@ -402,5 +414,7 @@ int cmd_cat_file(int argc, const char **argv, const char > *prefix) > if (batch.enabled) > return batch_objects(&batch); > > - return cat_one_file(opt, exp_type, obj_name); > + if (unkown_type && opt != 't' && opt != 's') > + die("git cat-file --allow-unkown-type: use with -s or -t"); > + return cat_one_file(opt, exp_type, obj_name, unkown_type); > } -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More ma
Re: [PATCH v9 1/5] sha1_file: support reading from a loose object of unknown type
Karthik Nayak writes: > Update sha1_loose_object_info() to optionally allow it to read > from a loose object file of unknown/bogus type; as the function > usually returns the type of the object it read in the form of enum > for known types, add an optional "typename" field to receive the > name of the type in textual form and a flag to indicate the reading > of a loose object file of unknown/bogus type. > > Add parse_sha1_header_extended() which acts as a wrapper around > parse_sha1_header() allowing more information to be obtained. Thanks. This mostly looks good modulo a nit. > diff --git a/sha1_file.c b/sha1_file.c > index 980ce6b..9a15634 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -1564,6 +1564,33 @@ int unpack_sha1_header(git_zstream *stream, unsigned > char *map, unsigned long ma > return git_inflate(stream, 0); > } > > +static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char > *map, > + unsigned long mapsize, void *buffer, > + unsigned long bufsiz, struct strbuf > *header) This function in this round looks somewhat tricky. > +{ > + unsigned char *cp; > + int status; > + > + status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz); Here, we unpack into the caller-supplied buffer, without touching the caller-supplied header strbuf. > + /* > + * Check if entire header is unpacked in the first iteration. > + */ > + if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer)) > + return 0; And we return the object header in the buffer without ever touching the header strbuf. We expect that the caller would know that the buffer it gave us would contain the full object header line. > + strbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer); > + do { > + status = git_inflate(stream, 0); > + strbuf_add(header, buffer, stream->next_out - (unsigned char > *)buffer); > + if (memchr(buffer, '\0', stream->next_out - (unsigned char > *)buffer)) > + return 0; However, here, we return the object header in the caller-supplied header strbuf, while using the caller-supplied buffer as a scratch area. We expect that the caller would know that the header strbuf is what it has to use to find the object header. Which is good in the sense that there is no unnecessary copies, but the caller needs to be careful to do something like: if (!unpack_to_strbuf(... buffer, sizeof(buffer), &header)) { if (header.len) object_header = header.buf; else object_header = buffer; } else { error("cannot unpack the object header"); } It is a good trade-off between complexity and efficiency. The complexity is isolated as the function is file-scope-static and it is perfectly fine to force the callers to be extra careful. But this suggests that the patch to add tests should try at least two, preferably three, kinds of test input. A bogus type that needs a header longer than the caller's fixed buffer, a bogus type whose header would fit within the fixed buffer, and optionally a correct type whose header should always fit within the fixed buffer. > @@ -1614,27 +1641,38 @@ static void *unpack_sha1_rest(git_zstream *stream, > void > ... > + /* > + * Set type to 0 if its an unknown object and > + * we're obtaining the type using '--allow-unkown-type' > + * option. > + */ > + if ((flags & LOOKUP_UNKNOWN_OBJECT) && (type == -1)) > + type = 0; > + else if (type == -1) > + die("invalid object type"); Would "type == -2" or any other negative value, if existed, mean something different? I do not think so, and hence I would prefer these two checks be done with "type < 0" instead. > @@ -2522,13 +2572,15 @@ struct packed_git *find_sha1_pack(const unsigned char > *sha1, > } > > static int sha1_loose_object_info(const unsigned char *sha1, > - struct object_info *oi) > + struct object_info *oi, > + int flags) > { > - int status; > - unsigned long mapsize, size; > + int status = 0; > + unsigned long mapsize; > void *map; > git_zstream stream; > char hdr[32]; > + struct strbuf hdrbuf = STRBUF_INIT; > ... > + if ((flags & LOOKUP_UNKNOWN_OBJECT)) { > + if (unpack_sha1_header_to_strbuf(&stream, map, mapsize, hdr, > sizeof(hdr), &hdrbuf) < 0) > + status = error("unable to unpack %s header with > --allow-unknown-type", > +sha1_to_hex(sha1)); > + } else if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) > < 0) > status = error("unable to unpack %s header", > sha
[PATCH v9 3/5] cat-file: teach cat-file a '--allow-unknown-type' option
Currently 'git cat-file' throws an error while trying to print the type or size of a broken/corrupt object. This is because these objects are usually of unknown types. Teach git cat-file a '--allow-unkown-type' option where it prints the type or size of a broken/corrupt object without throwing an error. Modify '-t' and '-s' options to call sha1_object_info_extended() directly to support the '--allow-unkown-type' option. Helped-by: Junio C Hamano Helped-by: Eric Sunshine Signed-off-by: Karthik Nayak --- builtin/cat-file.c | 38 ++ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 53b5376..299e2e5 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -9,13 +9,20 @@ #include "userdiff.h" #include "streaming.h" -static int cat_one_file(int opt, const char *exp_type, const char *obj_name) +static int cat_one_file(int opt, const char *exp_type, const char *obj_name, + int unkown_type) { unsigned char sha1[20]; enum object_type type; char *buf; unsigned long size; struct object_context obj_context; + struct object_info oi = {NULL}; + struct strbuf sb = STRBUF_INIT; + unsigned flags = LOOKUP_REPLACE_OBJECT; + + if (unkown_type) + flags |= LOOKUP_UNKNOWN_OBJECT; if (get_sha1_with_context(obj_name, 0, sha1, &obj_context)) die("Not a valid object name %s", obj_name); @@ -23,20 +30,22 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name) buf = NULL; switch (opt) { case 't': - type = sha1_object_info(sha1, NULL); - if (type > 0) { - printf("%s\n", typename(type)); + oi.typename = &sb; + if (sha1_object_info_extended(sha1, &oi, flags) < 0) + die("git cat-file: could not get object info"); + if (sb.len) { + printf("%s\n", sb.buf); + strbuf_release(&sb); return 0; } break; case 's': - type = sha1_object_info(sha1, &size); - if (type > 0) { - printf("%lu\n", size); - return 0; - } - break; + oi.sizep = &size; + if (sha1_object_info_extended(sha1, &oi, flags) < 0) + die("git cat-file: could not get object info"); + printf("%lu\n", size); + return 0; case 'e': return !has_sha1_file(sha1); @@ -323,7 +332,7 @@ static int batch_objects(struct batch_options *opt) } static const char * const cat_file_usage[] = { - N_("git cat-file (-t | -s | -e | -p | | --textconv) "), + N_("git cat-file (-t [--allow-unkown-type]|-s [--allow-unkown-type]|-e|-p||--textconv) "), N_("git cat-file (--batch | --batch-check) < "), NULL }; @@ -359,6 +368,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) int opt = 0; const char *exp_type = NULL, *obj_name = NULL; struct batch_options batch = {0}; + int unkown_type = 0; const struct option options[] = { OPT_GROUP(N_(" can be one of: blob, tree, commit, tag")), @@ -369,6 +379,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) OPT_CMDMODE('p', NULL, &opt, N_("pretty-print object's content"), 'p'), OPT_CMDMODE(0, "textconv", &opt, N_("for blob objects, run textconv on object's content"), 'c'), + OPT_BOOL( 0, "allow-unkown-type", &unkown_type, + N_("allow -s and -t to work with broken/corrupt objects")), { OPTION_CALLBACK, 0, "batch", &batch, "format", N_("show info and content of objects fed from the standard input"), PARSE_OPT_OPTARG, batch_option_callback }, @@ -402,5 +414,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) if (batch.enabled) return batch_objects(&batch); - return cat_one_file(opt, exp_type, obj_name); + if (unkown_type && opt != 't' && opt != 's') + die("git cat-file --allow-unkown-type: use with -s or -t"); + return cat_one_file(opt, exp_type, obj_name, unkown_type); } -- 2.4.0.rc1.250.g565e85b -- 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 v9 0/5] cat-file: teach cat-file a '--allow-unkown-type' option
The last iteration of the patch can be seen : http://thread.gmane.org/gmane.comp.version-control.git/267213 Changes since last version: sha1_file: * eliminate "struct strbuf typename = STRBUF_INIT" in "parse_sha1_header_extended()" * make "unpack_sha1_header_to_strbuf()" work automagically. Now it unpacks first 32 bytes and checks if the header is unpacked before moving to a strbuf. * remove unnecessary if condition for "strbuf_release()" . cat-file: * change the option name from "--literally" to "--allow-unkown-type". * make the options mutually exclusive. * clean up '-s' option. t1006: * use echo_without_newline() and added quotes. -- 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-p4: add failing tests for case-folding p4d
(Adding Pete, Vitor, and Fusion in case they have any thoughts on working with P4 servers that do case-folding, or at least failing gracefully). On 29/04/15 00:01, Lex Spoon wrote: The last comment in the test took me a minute to decipher. I would suggest "no repo path called LC" instead of "no repo called LC". Also, it would have helped me to either have a little comment on the "UC" version of the test, or to make the previous comment a little more neutral so that it will apply to both test cases. OK, thanks! Otherwise, while I am not a regular maintainer of this code, the patch does LGTM. Certainly it's good to have more test coverage. For the underlying problem, I haven't thought about it very much, but it looks like a plausible first step might be to simply probe the given file name and see if it comes back the same way. If it comes back differently, then maybe the command should abort? I think the problem may be a bit trickier than that. I think what's happening when cloning is that when files come back from the server, git-p4 checks that they are contained within the directory it is cloning. This happens in p4StartsWith(), (called from extractFilesFromCommit()) which already tries to fix this problem by checking 'core.ignorecase'. However, that won't work if the local machine is case sensitive but the server isn't (e.g. Linux client, Windows server). git-p4 does this because it's fetching *commits* from Perforce, and a commit might have files that are outside the directory being cloned. I tried teaching p4StartsWith() to ask the server if it is case-folding ('p4 info') and that then means that the git-p4 clone actually succeeds. However, git-p4 submit then fails because it gets terribly confused about pathnames - it probably needs to do some lowercasing somewhere. So that might be worth pursuing. Open to other suggestions though! What a tough problem all around... Indeed! Luke -- 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