Re: GSoC 2014: Summary so far, discussion starter: how to improve?
Thomas Rast writes: > * Does libgit2 want to remain under the Git umbrella, or participate > on its own? > > * Figure out the wiki situation. In previous years the project > proposals and other important information were hosted at k.org [5] and > github wikis [6]. Other options were floated, such as an official > wiki hosted by github. (This is somewhat of a contentious issue that > spans beyond GSoC.) > > * Find an org admin and backup. In previous years Shawn and Peff did > this. Would you do it again? Any opinions on these points? I would actually favor a move to a wiki of the style that Peff proposed, hosted by github and backed by git with either a very mild ACL or none ("bots don't know git push"). k.org wiki had a grand total of three edits in the last 30 days (plus some bot edits for user creation). And of course without an org admin we don't really need to go any further... > [5] https://git.wiki.kernel.org/index.php/SoC2011Projects > similarly for previous years > [6] https://github.com/peff/git/wiki/SoC-2012-Ideas > https://github.com/trast/git/wiki/SoC-2013-Ideas -- Thomas Rast t...@thomasrast.ch -- 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: GSoC 2014: Summary so far, discussion starter: how to improve?
On Thu, Nov 21, 2013 at 9:36 AM, Thomas Rast wrote: > Thomas Rast writes: >> >> * Find an org admin and backup. In previous years Shawn and Peff did >> this. Would you do it again? If Shawn and Peff don't answer, it probably means that you should volunteer :-) > Any opinions on these points? > > I would actually favor a move to a wiki of the style that Peff proposed, > hosted by github and backed by git with either a very mild ACL or none > ("bots don't know git push"). k.org wiki had a grand total of three > edits in the last 30 days (plus some bot edits for user creation). If we are provided a wiki of the style Peff proposed, then I am ok using it. But until we have it, let's use the k.org wiki. Thanks, Christian. -- 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: GSoC 2014: Summary so far, discussion starter: how to improve?
On Thu, Nov 21, 2013 at 09:36:37AM +0100, Thomas Rast wrote: > Thomas Rast writes: > > > * Does libgit2 want to remain under the Git umbrella, or participate > > on its own? > > > > * Figure out the wiki situation. In previous years the project > > proposals and other important information were hosted at k.org [5] and > > github wikis [6]. Other options were floated, such as an official > > wiki hosted by github. (This is somewhat of a contentious issue that > > spans beyond GSoC.) > > > > * Find an org admin and backup. In previous years Shawn and Peff did > > this. Would you do it again? > > Any opinions on these points? I'll answer them in reverse order. :) I'm happy to be org admin again (or happy to give up the mantle if somebody else is interested). Regarding the wiki, my main complaints about the k.org wiki have been: 1. It was down last time we tried to use it for GSoC. :) This has since been fixed. 2. I had the impression that spam was a problem and it needed a crew of scourers to find and remove it. That's a thankless job that I don't personally want to do. It looks like there is not much spam these days (perhaps due to k.org's login restrictions?), and people do remove it. So if there are sufficient volunteers, it may be a non-issue. 3. You can't use git to edit it or view the history. These days we have the git-remote-mediawiki helper. I haven't done any real work with it, but I have played around and I think it may be sufficient. 4. Mediawiki syntax sucks compared to Markdown. :) That's my personal feeling, but I recognize others may disagree. A syntax change might be seen as a disadvantage of moving by some. I can live with it (but I want to throw it out there in case everybody feels the same way). The disadvantages / complications of moving to a different wiki (e.g., a GitHub wiki) are: 1. It's non-zero work to set up, especially if we do not just use an out-of-the-box solution. I'd be in favor of some kind of static site generator, but then we have to pick one, style it, set up auto-build-on-push, etc. GitHub Pages would do most of the dirty work there, if we want to use it. 2. Some solutions involve using non-free software or services, which some people may not like. 3. What happens to the old wiki? Other sites link there, and it has content. Do we migrate the content? Do they both exist simultaneously? Is one just for GSoC stuff (which seems unnecessarily confusing to me)? For libgit2, I think it makes sense to leave them under the Git umbrella and give them a slot or two as appropriate, as it saves administrative effort. But it is up to them if they want to split off into their own GSoC org. If that is the case, they will need an admin and to do a bunch of application paperwork. I believe we can give them a recommendation to help ease the process along. -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] git-svn: Support svn:global-ignores property
--- Documentation/git-svn.txt | 12 ++-- git-svn.perl | 46 -- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt index 30c5ee2..0c1cd46 100644 --- a/Documentation/git-svn.txt +++ b/Documentation/git-svn.txt @@ -405,14 +405,14 @@ Any other arguments are passed directly to 'git log' independently of 'git svn' functions. 'create-ignore':: -Recursively finds the svn:ignore property on directories and -creates matching .gitignore files. The resulting files are staged to -be committed, but are not committed. Use -r/--revision to refer to a -specific revision. +Recursively finds svn:ignore and svn:global-ignores properties on +directories and creates matching .gitignore files. The resulting +files are staged to be committed, but are not committed. +Use -r/--revision to refer to a specific revision. 'show-ignore':: -Recursively finds and lists the svn:ignore property on -directories. The output is suitable for appending to +Recursively finds and lists svn:ignore and svn:global-ignores +properties on directories. The output is suitable for appending to the $GIT_DIR/info/exclude file. 'mkdirs':: diff --git a/git-svn.perl b/git-svn.perl index 7349ffe..a2565e1 100755 --- a/git-svn.perl +++ b/git-svn.perl @@ -1261,6 +1261,15 @@ sub cmd_rebase { } } +sub get_svn_ignore { +my ($props, $prop) = @_; +my $s = $props->{$prop} or return; +$s =~ s/[\r\n]+/\n/g; +$s =~ s/^\n+//; +chomp $s; +return $s; +} + sub cmd_show_ignore { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; @@ -1268,12 +1277,17 @@ sub cmd_show_ignore { $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; print STDOUT "\n# $path\n"; -my $s = $props->{'svn:ignore'} or return; -$s =~ s/[\r\n]+/\n/g; -$s =~ s/^\n+//; -chomp $s; -$s =~ s#^#$path#gm; -print STDOUT "$s\n"; +my $s = &get_svn_ignore($props, 'svn:ignore'); +my $s_global = &get_svn_ignore($props, 'svn:global-ignores'); +$s or $s_global or return; +if ($s) { +$s =~ s#^#$path#gm; +print STDOUT "$s\n"; +} +if ($s_global) { +$s_global =~ s#^#$path**/#gm; +print STDOUT "$s_global\n"; +} }); } @@ -1304,16 +1318,20 @@ sub cmd_create_ignore { # which git won't track mkpath([$path]) unless -d $path; my $ignore = $path . '.gitignore'; -my $s = $props->{'svn:ignore'} or return; +my $s = &get_svn_ignore($props, 'svn:ignore'); +my $s_global = &get_svn_ignore($props, 'svn:global-ignores'); +$s or $s_global or return; open(GITIGNORE, '>', $ignore) or fatal("Failed to open `$ignore' for writing: $!"); -$s =~ s/[\r\n]+/\n/g; -$s =~ s/^\n+//; -chomp $s; -# Prefix all patterns so that the ignore doesn't apply -# to sub-directories. -$s =~ s#^#/#gm; -print GITIGNORE "$s\n"; +if ($s) { +# Prefix all patterns so that the ignore doesn't apply +# to sub-directories. +$s =~ s#^#/#gm; +print GITIGNORE "$s\n"; +} +if ($s_global) { +print GITIGNORE "$s_global\n"; +} close(GITIGNORE) or fatal("Failed to close `$ignore': $!"); command_noisy('add', '-f', $ignore); -- 1.8.3.msysgit.0 -- 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] Revamp git-cherry(1)
git-cherry(1)'s "description" section has never really managed to explain to me what the command does. It contains too much explanation of the algorithm instead of simply saying what goals it achieves, and too much terminology that we otherwise do not use (fork-point instead of merge-base). Try a much more concise approach: state what it finds out, why this is neat, and how the output is formatted, in a few short paragraphs. In return, provide a longer example of how it fits into a format-patch/am based workflow. Also carefully avoid using "merge" in a context where it does not mean something that comes from git-merge(1). Instead, say "apply" in an attempt to further link to patch workflow concepts. While there, also omit the language about _which_ upstream branch we treat as the default. I literally just learned that we support having several, so let's not confuse new users here, especially considering that git-config(1) does _not_ document this. Prompted-by: a.hue...@commend.com on #git Signed-off-by: Thomas Rast --- Documentation/git-cherry.txt | 73 +--- 1 file changed, 41 insertions(+), 32 deletions(-) diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt index 2d0daae..78ffddf 100644 --- a/Documentation/git-cherry.txt +++ b/Documentation/git-cherry.txt @@ -3,7 +3,7 @@ git-cherry(1) NAME -git-cherry - Find commits not merged upstream +git-cherry - Find commits not applied in upstream SYNOPSIS @@ -12,46 +12,27 @@ SYNOPSIS DESCRIPTION --- -The changeset (or "diff") of each commit between the fork-point and -is compared against each commit between the fork-point and . -The diffs are compared after removing any whitespace and line numbers. +Determine whether there are commits in `..` that are +equivalent to those in the range `..`. -Every commit that doesn't exist in the branch -has its id (sha1) reported, prefixed by a symbol. The ones that have -equivalent change already -in the branch are prefixed with a minus (-) sign, and those -that only exist in the branch are prefixed with a plus (+) symbol: - - __*__*__*__*__> - / -fork-point - \__+__+__-__+__+__-__+__> - - -If a has been given then the commits along the branch up -to and including are not reported: - - __*__*__*__*__> - / -fork-point - \__*__*-__+__> - - -Because 'git cherry' compares the changeset rather than the commit id -(sha1), you can use 'git cherry' to find out if a commit you made locally -has been applied under a different commit id. For example, -this will happen if you're feeding patches via email rather -than pushing or pulling commits directly. +The equivalence test is based on the diff, after removing whitespace +and line numbers. git-cherry therefore detects when commits have been +"copied" by means of linkgit:git-cherry-pick[1], linkgit:git-am[1] or +linkgit:git-rebase[1]. +Outputs the SHA1 of every commit in `..`, prefixed with +`-` for commits that have an equivalent in , and `+` for +commits that do not. OPTIONS --- -v:: - Verbose. + Verbose. Currently shows the commit subjects next to their + SHA1. :: Upstream branch to compare against. - Defaults to the first tracked remote branch, if available. + Defaults to the upstream branch of HEAD. :: Working branch; defaults to HEAD. @@ -59,6 +40,34 @@ OPTIONS :: Do not report commits up to (and including) limit. +EXAMPLES + + +git-cherry is frequently used in patch-based workflows (see +linkgit:gitworkflows[7]) to determine if a series of patches has been +applied by the upstream maintainer. In such a workflow you might +create and send a topic branch like this (fill in appropriate +arguments for `...`): ++ + +git checkout -b topic origin/master +# work and create some commits +git format-patch origin/master +git send-email ... 00* + ++ +Later, you can whether your changes have been applied by saying (still +on `topic`): ++ + +git fetch # update your notion of origin/master +git cherry -v + ++ +Note that this uses , and assumes that +`core.autosetupmerge` is enabled (the default). + + SEE ALSO linkgit:git-patch-id[1] -- 1.8.5.rc2.355.g6969a19 -- 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] drop support for "experimental" loose objects
On Wed, Nov 20, 2013 at 06:28:06PM -0400, Joey Hess wrote: > Jeff King wrote: > > As for your specific corruption, I can't make heads or tails of it. It > > is not a single-bit error. > > Oh, I should have mentioned that I am generating corrupt git > repositories mechanically for testing. I think in this case it prepended > some garbage to an object. Ah. That explains a lot (yes, dropping the first 19 bytes of your corrupted file recovers the object). I still think we should probably do this, though: -- >8 -- Subject: drop support for "experimental" loose objects In git v1.4.3, we introduced a new loose object format that encoded some object information outside of the zlib stream. Ultimately the format was dropped in v1.5.3, but we kept the reading side around to help people migrate objects. Each time we open a loose object, we use a heuristic to check whether it is in the normal loose format or the experimental one. This heuristic is robust in the face of valid data, but it tends to treat corrupted or garbage data as an experimental object. With the regular format, we would notice quickly that zlib's crc does not check out and complain. With the experimental object, we are likely to extract a nonsensical object size and try to allocate a huge buffer, resulting in xmalloc calling "die". This latter behavior is much worse for two reasons. One, git reports an allocation error when the real error is corruption. And two, the program dies unconditionally, so you cannot even run fsck (which would otherwise ignore the broken object and keep going). We could try to improve the heuristic to err on the side of normal objects in the face of corruption, but there is really little point. The experimental format is long-dead, and was never enabled by default to begin with. We can instead simply remove it. The only affected repository would be one that explicitly set core.legacyheaders in 2007, and then never repacked in the intervening 6 years. Signed-off-by: Jeff King --- The test objects removed are all binary. Git seems to guess a few as non-binary, though, because they don't contain any NULs, and includes gross binary bytes in the patch below. In theory the mail's transfer encoding will take care of this. We'll see, I guess. :) sha1_file.c| 74 - t/t1013-loose-object-format.sh | 66 -- .../14/9cedb5c46929d18e0f118e9fa31927487af3b6 | Bin 117 -> 0 bytes .../16/56f9233d999f61ef23ef390b9c71d75399f435 | Bin 17 -> 0 bytes .../1e/72a6b2c4a577ab0338860fa9fe87f761fc9bbd | Bin 18 -> 0 bytes .../25/7cc5642cb1a054f08cc83f2d943e56fd3ebe99 | Bin 19 -> 0 bytes .../2e/65efe2a145dda7ee51d1741299f848e5bf752e | Bin 10 -> 0 bytes .../6b/aee0540ea990d9761a3eb9ab183003a71c3696 | Bin 181 -> 0 bytes .../70/e6a83d8dcb26fc8bc0cf702e2ddeb6adca18fd | Bin 26 -> 0 bytes .../76/e7fa9941f4d5f97f64fea65a2cba436bc79cbb | 2 - .../78/75c6237d3fcdd0ac2f0decc7d3fa6a50b66c09 | Bin 139 -> 0 bytes .../7a/37b887a73791d12d26c0d3e39568a8fb0fa6e8 | Bin 54 -> 0 bytes .../85/df50785d62d3b05ab03d9cbf7e4a0b49449730 | Bin 13 -> 0 bytes .../8d/4e360d6c70fbd72411991c02a09c442cf7a9fa | Bin 156 -> 0 bytes .../95/b1625de3ba8b2214d1e0d0591138aea733f64f | Bin 252 -> 0 bytes .../9a/e9e86b7bd6cb1472d9373702d8249973da0832 | Bin 11 -> 0 bytes .../bd/15045f6ce8ff75747562173640456a394412c8 | Bin 34 -> 0 bytes .../e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 | Bin 9 -> 0 bytes .../f8/16d5255855ac160652ee5253b06cd8ee14165a | 1 - 19 files changed, 143 deletions(-) delete mode 100755 t/t1013-loose-object-format.sh delete mode 100644 t/t1013/objects/14/9cedb5c46929d18e0f118e9fa31927487af3b6 delete mode 100644 t/t1013/objects/16/56f9233d999f61ef23ef390b9c71d75399f435 delete mode 100644 t/t1013/objects/1e/72a6b2c4a577ab0338860fa9fe87f761fc9bbd delete mode 100644 t/t1013/objects/25/7cc5642cb1a054f08cc83f2d943e56fd3ebe99 delete mode 100644 t/t1013/objects/2e/65efe2a145dda7ee51d1741299f848e5bf752e delete mode 100644 t/t1013/objects/6b/aee0540ea990d9761a3eb9ab183003a71c3696 delete mode 100644 t/t1013/objects/70/e6a83d8dcb26fc8bc0cf702e2ddeb6adca18fd delete mode 100644 t/t1013/objects/76/e7fa9941f4d5f97f64fea65a2cba436bc79cbb delete mode 100644 t/t1013/objects/78/75c6237d3fcdd0ac2f0decc7d3fa6a50b66c09 delete mode 100644 t/t1013/objects/7a/37b887a73791d12d26c0d3e39568a8fb0fa6e8 delete mode 100644 t/t1013/objects/85/df50785d62d3b05ab03d9cbf7e4a0b49449730 delete mode 100644 t/t1013/objects/8d/4e360d6c70fbd72411991c02a09c442cf7a9fa delete mode 100644 t/t1013/objects/95/b1625de3ba8b2214d1e0d0591138aea733f64f delete mode 100644 t/t1013/objects/9a/e9e86b7bd6cb1472d9373702d8249973da0832 delete mode 100644 t/t1013/objects/bd/15045f6ce8ff75747562173640456a394412c8 delete mode 100644 t/t1013/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 delete mode 100644 t/t1013/objec
Re: [PATCH] drop support for "experimental" loose objects
On Thu, Nov 21, 2013 at 06:41:58AM -0500, Jeff King wrote: > The test objects removed are all binary. Git seems to guess a few as > non-binary, though, because they don't contain any NULs, and includes > gross binary bytes in the patch below. In theory the mail's transfer > encoding will take care of this. We'll see, I guess. :) Nope; something munged it along the way (I sent it as C-T-E: 8bit, but vger rewrites to QP before re-mailing, so that is the likely spot). Here's the same patch, but with this stuck into .git/info/attributes: /t/t1013/objects/*/* binary which should work. -- >8 -- Subject: drop support for "experimental" loose objects In git v1.4.3, we introduced a new loose object format that encoded some object information outside of the zlib stream. Ultimately the format was dropped in v1.5.3, but we kept the reading side around to help people migrate objects. Each time we open a loose object, we use a heuristic to check whether it is in the normal loose format, or the experimental one. This heuristic is robust in the face of valid data, but it tends to treat corrupted or garbage data as an experimental object. With the regular format, we would notice quickly that zlib's crc does not check out and complain. With the experimental object, we are likely to extract a nonsensical object size and try to allocate a huge buffer, resulting in xmalloc calling "die". This latter behavior is much worse, for two reasons. One, git reports an allocation error when the real error is corruption. And two, the program dies unconditionally, so you cannot even run fsck (which would otherwise ignore the broken object and keep going). We could try to improve the heuristic to err on the side of normal objects in the face of corruption, but there is really little point. The experimental format is long-dead, and was never enabled by default to begin with. We can instead simply remove it. The only affected repository would be one that explicitly set core.legacyheaders in 2007, and then never repacked in the intervening 6 years. Signed-off-by: Jeff King --- sha1_file.c| 74 - t/t1013-loose-object-format.sh | 66 -- .../14/9cedb5c46929d18e0f118e9fa31927487af3b6 | Bin 117 -> 0 bytes .../16/56f9233d999f61ef23ef390b9c71d75399f435 | Bin 17 -> 0 bytes .../1e/72a6b2c4a577ab0338860fa9fe87f761fc9bbd | Bin 18 -> 0 bytes .../25/7cc5642cb1a054f08cc83f2d943e56fd3ebe99 | Bin 19 -> 0 bytes .../2e/65efe2a145dda7ee51d1741299f848e5bf752e | Bin 10 -> 0 bytes .../6b/aee0540ea990d9761a3eb9ab183003a71c3696 | Bin 181 -> 0 bytes .../70/e6a83d8dcb26fc8bc0cf702e2ddeb6adca18fd | Bin 26 -> 0 bytes .../76/e7fa9941f4d5f97f64fea65a2cba436bc79cbb | Bin 155 -> 0 bytes .../78/75c6237d3fcdd0ac2f0decc7d3fa6a50b66c09 | Bin 139 -> 0 bytes .../7a/37b887a73791d12d26c0d3e39568a8fb0fa6e8 | Bin 54 -> 0 bytes .../85/df50785d62d3b05ab03d9cbf7e4a0b49449730 | Bin 13 -> 0 bytes .../8d/4e360d6c70fbd72411991c02a09c442cf7a9fa | Bin 156 -> 0 bytes .../95/b1625de3ba8b2214d1e0d0591138aea733f64f | Bin 252 -> 0 bytes .../9a/e9e86b7bd6cb1472d9373702d8249973da0832 | Bin 11 -> 0 bytes .../bd/15045f6ce8ff75747562173640456a394412c8 | Bin 34 -> 0 bytes .../e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 | Bin 9 -> 0 bytes .../f8/16d5255855ac160652ee5253b06cd8ee14165a | Bin 116 -> 0 bytes 19 files changed, 140 deletions(-) delete mode 100755 t/t1013-loose-object-format.sh delete mode 100644 t/t1013/objects/14/9cedb5c46929d18e0f118e9fa31927487af3b6 delete mode 100644 t/t1013/objects/16/56f9233d999f61ef23ef390b9c71d75399f435 delete mode 100644 t/t1013/objects/1e/72a6b2c4a577ab0338860fa9fe87f761fc9bbd delete mode 100644 t/t1013/objects/25/7cc5642cb1a054f08cc83f2d943e56fd3ebe99 delete mode 100644 t/t1013/objects/2e/65efe2a145dda7ee51d1741299f848e5bf752e delete mode 100644 t/t1013/objects/6b/aee0540ea990d9761a3eb9ab183003a71c3696 delete mode 100644 t/t1013/objects/70/e6a83d8dcb26fc8bc0cf702e2ddeb6adca18fd delete mode 100644 t/t1013/objects/76/e7fa9941f4d5f97f64fea65a2cba436bc79cbb delete mode 100644 t/t1013/objects/78/75c6237d3fcdd0ac2f0decc7d3fa6a50b66c09 delete mode 100644 t/t1013/objects/7a/37b887a73791d12d26c0d3e39568a8fb0fa6e8 delete mode 100644 t/t1013/objects/85/df50785d62d3b05ab03d9cbf7e4a0b49449730 delete mode 100644 t/t1013/objects/8d/4e360d6c70fbd72411991c02a09c442cf7a9fa delete mode 100644 t/t1013/objects/95/b1625de3ba8b2214d1e0d0591138aea733f64f delete mode 100644 t/t1013/objects/9a/e9e86b7bd6cb1472d9373702d8249973da0832 delete mode 100644 t/t1013/objects/bd/15045f6ce8ff75747562173640456a394412c8 delete mode 100644 t/t1013/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 delete mode 100644 t/t1013/objects/f8/16d5255855ac160652ee5253b06cd8ee14165a diff --git a/sha1_file.c b/sha1_file.c index 7dadd04..a72fcb6 100644 --- a/sha1_file.c +++ b/sha1_file.c @@
Re: [RFC PATCH] Revamp git-cherry(1)
On Thu, Nov 21, 2013 at 12:30:56PM +0100, Thomas Rast wrote: > git-cherry(1)'s "description" section has never really managed to > explain to me what the command does. It contains too much explanation > of the algorithm instead of simply saying what goals it achieves, and > too much terminology that we otherwise do not use (fork-point instead > of merge-base). > > Try a much more concise approach: state what it finds out, why this is > neat, and how the output is formatted, in a few short paragraphs. In > return, provide a longer example of how it fits into a format-patch/am > based workflow. FWIW, I find your concise explanation much friendlier. > +Later, you can whether your changes have been applied by saying (still > +on `topic`): s/can/& see/ ? > + > +git fetch # update your notion of origin/master > +git cherry -v > + > ++ > +Note that this uses , and assumes that > +`core.autosetupmerge` is enabled (the default). I couldn't quite parse this. Is there a word missing before the comma, or is it "uses and assumes that..."? Given that it is the default, I wonder if it is worth mentioning at all. Even I, who knows what autosetupmerge does, took a minute to figure out why it is relevant here. I suspect it may just confuse most readers. -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
Git issues with submodules
Hey everyone from Blender developers! As you might already know, we've recently switched from SVN to Git to host Blender sources. In general it works really awesome, but we've got some issues with submodules. in SVN we had separate repositories for addons and translations which were attached to main tree as svn:external. The reason for this was: 1. Separate commit access between core sources and addons so nobody accidentally breaks anything in the core. 2. Separate commit history to help tracking issues down. For the most developers and all artists (yes, we've got loads of artists who builds blender on their own) it makes sense to always checkout latest versions of addons and translations when updating working tree. We used Git submodules as a replacement for svn:external, with some tweaks and specific of update procedure. Namely, we always do `git submodule update --remote` to pull all the latest changes from submodules. This will mark checkout as modified because submodule hash changes. To avoid infinite commits of submodule hash we've added ignore=all to their configuration. In most cases it works fine, but there're some circumstances when it gives weirdo issues. Namely, `git ls-files -m` will show addons as modified, regardless ignore=all configuration. In the same time `git diff-index --name-only HEAD --` will show no changes at all. This leads to issues with Arcanist (which is a Phabricator's tool) who considers addons as uncommited changes and either complains on this or just adds this to commits. This issue i might easily reproduce on my laptop with latest Git 1.8.4.3. There're also some more issues which happens to our developers and which i can not quite reproduce. Sometimes it happens so git checkout to another branch yields about uncommited changes to addons and doesn't checkout to another branch. My guess here is that submodule hash in master and branch was different and having hash modified in master somehow prevented changes from another branch to be checked out. In this case question would be: what would be the proper way to checkout branches when having submodules configured this way? Second issue is that some developers still manages to commit changes to submodule hash, which i have totally no idea why Git allows to include such a changes. I could not do such a commits on purpose even. Here're some links to help understanding what's going on: - Blender repository browser: http://developer.blender.org/diffusion/B/ - Task in our tracker about issues we've got with Git: http://developer.blender.org/T37528 - History of changes to addons hash: http://developer.blender.org/diffusion/B/history/master/release/scripts/addons We're totally new to git submodules and clarification (and maybe even confirmed bug with ls-files -m :) would really be appreciated. We're also open for suggestions about re-configuring our submodules so they works in a way we'd expect this. Thanks in advance! -- With best regards, Sergey Sharybin -- 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] Revamp git-cherry(1)
Thomas Rast writes: > Junio C Hamano writes: > >>> OPTIONS >>> --- >>> -v:: >>> - Verbose. >>> + Verbose. Currently shows the commit subjects next to their >>> + SHA1. >> >> Whenever I see "Currently", it makes me wonder "why does it need to >> say that? Is there a plan to change it soon, and if so where is the >> plan described?". > > I wanted to avoid documenting exactly what it does, so that in the > future it could do more than that. Is that overly paranoid? I would have to say so. After all, the documentation is supposed to describe the current state of affairs, and we would update it when "the current state" changes. In places, we may express our plan to forewarn readers of planned upcoming changes, but still... -- 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] commit -v: strip diffs and submodule shortlogs from the commit message
Jens Lehmann writes: > But what about this: > > struct strbuf cut_line = STRBUF_INIT; > strbuf_addf(&cut_line, "%c %s", comment_line_char, wt_status_cut_line); > p = strstr(buf->buf, cut_line.buf); > if (p && (p == buf->buf || p[-1] == '\n')) > strbuf_setlen(buf, p - buf->buf); > strbuf_release(&cut_line); > > That is shorter can easily be adapted to a comment line string later. Sure, that would work fine. 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 1/2] glossary-content.txt: remove an obsolete paragrah
Nguyễn Thái Ngọc Duy writes: > With the introduction of :(literal), :(glob) and :(icase), :(top) is > no longer the only recognized magic signature. You need to re-read the message you took hints that led to this patch from (or, rather, the existing document around the area you are patching, which led to the wording in that message from me). The above is a list of magic words, but until you add the magic signature ":!not_in_this_directory" for the ":(exclude)" magic word, ":/" still is the only magic signature. Another moderately related tangent is this phrase in the existing document: In the short form, the leading colon `:` is followed by zero or more "magic signature" letters (which optionally is terminated by another colon `:`), and the remainder is the pattern to match against the path. The optional colon that terminates the "magic signature" can be omitted if the pattern begins with a character that cannot be a "magic signature" and is not a colon. While it is technically correct, the phrase "a character that cannot be" is somewhat misleading, and I think it needs to be clarified by rephrasing. As we can see in a later paragaph: The "magic signature" consists of an ASCII symbol that is not alphanumeric. Currently only the slash `/` is recognized as a "magic signature"... the correct way to read that "a character that cannot be a magic signature" is "a character that is not 'an ASCII symbol that is not alphanumeric'", which means you can do: :!/foo - with magic signatures ! and /, pattern begins at 'f' :/#abc - with magic signatures # and /, pattern begins at 'a', even if in a particular version of Git, '#' magic signature may be invalid and produce an error :/:#abc - with magic signature /, pattern is "#abc". but because the definition of "magic signature" syntax comes later than "cannot be", it is prone to be misinterpreted as "anything that is not currently defined as a magic signature (namely, '/')". > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > .. on top of nd/magic-pathspec.. > > Documentation/glossary-content.txt | 4 > 1 file changed, 4 deletions(-) > > diff --git a/Documentation/glossary-content.txt > b/Documentation/glossary-content.txt > index e470661..e22b524 100644 > --- a/Documentation/glossary-content.txt > +++ b/Documentation/glossary-content.txt > @@ -379,10 +379,6 @@ full pathname may have special meaning: > Glob magic is incompatible with literal magic. > -- > + > -Currently only the slash `/` is recognized as the "magic signature", > -but it is envisioned that we will support more types of magic in later > -versions of Git. > -+ > A pathspec with only a colon means "there is no pathspec". This form > should not be combined with other pathspec. -- 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] commit -v: strip diffs and submodule shortlogs from the commit message
Am 21.11.2013 00:04, schrieb Junio C Hamano: > Jens Lehmann writes: >> diff --git a/wt-status.c b/wt-status.c >> index b4e44ba..734f94b 100644 >> --- a/wt-status.c >> +++ b/wt-status.c >> @@ -16,6 +16,9 @@ >> #include "column.h" >> #include "strbuf.h" >> >> +static char wt_status_cut_line[] = /* 'X' is replaced with >> comment_line_char */ >> +"X >8 \n"; >> + >> static char default_wt_status_colors[][COLOR_MAXLEN] = { >> GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */ >> GIT_COLOR_GREEN, /* WT_STATUS_UPDATED */ >> @@ -767,6 +770,15 @@ conclude: >> status_printf_ln(s, GIT_COLOR_NORMAL, ""); >> } >> >> +void wt_status_truncate_message_at_cut_line(struct strbuf *buf) >> +{ >> +const char *p; >> + >> +p = strstr(buf->buf, wt_status_cut_line); >> +if (p && (p == buf->buf || p[-1] == '\n')) >> +strbuf_setlen(buf, p - buf->buf); >> +} > > Perhaps it may happen that all the current callers have called > wt_status_print_verbose() to cause wt_status_cut_line[0] to hold > comment_line_char, but relying on that calling sequence somehow > makes me feel uneasy. I initialized the place to be occupied by the comment_line_char in wt_status_cut_line with 'X' on purpose to notice such a problem. But I'd be also fine with setting wt_status_cut_line[0] again here just to be sure. But please also see below ... > Perhaps cut_line[] should only have "--- >8 ---" part and both > printing side (below) and finding side (this one) should check these > separately? ... ok ... > That is: > > p = buf->buf; > while (p && *p) { > p = strchr(p, comment_line_char); > if (!p) > break; > if (strstr(p + 1, cut_line) == p + 1) > break; > p++; > continue; > } > if (p && *p && (p == buf->buf || p[-1] == '\n')) > strbuf_setlen(buf, p - buf->buf); > > or something (the above is deliberately less-efficient-than-ideal, > because I want to keep the code structure in such a way that we can > later turn comment_line_char to a string[] that can hold "//" to > allow a multi-char comment introducer more easily)? Hmm, I'm a bit reluctant to go that far to optimize this patch for another one that might materialize later. But what about this: struct strbuf cut_line = STRBUF_INIT; strbuf_addf(&cut_line, "%c %s", comment_line_char, wt_status_cut_line); p = strstr(buf->buf, cut_line.buf); if (p && (p == buf->buf || p[-1] == '\n')) strbuf_setlen(buf, p - buf->buf); strbuf_release(&cut_line); That is shorter can easily be adapted to a comment line string later. And even though it's slightly less performant should not be a problem here as this happens only once after invoking an editor for user input. >> static void wt_status_print_verbose(struct wt_status *s) >> { >> struct rev_info rev; >> @@ -787,10 +799,17 @@ static void wt_status_print_verbose(struct wt_status >> *s) >> * If we're not going to stdout, then we definitely don't >> * want color, since we are going to the commit message >> * file (and even the "auto" setting won't work, since it >> - * will have checked isatty on stdout). >> + * will have checked isatty on stdout). But we then do want >> + * to insert the scissor line here to reliably remove the >> + * diff before committing. >> */ >> -if (s->fp != stdout) >> +if (s->fp != stdout) { >> rev.diffopt.use_color = 0; >> +wt_status_cut_line[0] = comment_line_char; >> +fprintf(s->fp, wt_status_cut_line); >> +fprintf(s->fp, _("%c Do not touch the line above.\n"), >> comment_line_char); >> +fprintf(s->fp, _("%c Everything below will be removed.\n"), >> comment_line_char); >> +} > > I didn't bother with my "how about this" version, but we may want to > use strbuf_add_commented_lines() to help i18n/l10n folks. Depending > on the l10n, this message may want to become more or less than 2 > lines. Makes sense, will change that (maybe using strbuf_commented_addf() instead) for v4. -- 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] Revamp git-cherry(1)
Thomas Rast writes: > NAME > > -git-cherry - Find commits not merged upstream > +git-cherry - Find commits not applied in upstream Good. > +Determine whether there are commits in `..` that are > +equivalent to those in the range `..`. > > +The equivalence test is based on the diff, after removing whitespace > +and line numbers. git-cherry therefore detects when commits have been > +"copied" by means of linkgit:git-cherry-pick[1], linkgit:git-am[1] or > +linkgit:git-rebase[1]. > > +Outputs the SHA1 of every commit in `..`, prefixed with > +`-` for commits that have an equivalent in , and `+` for > +commits that do not. Yeah, short-sweet-and-sufficient. > OPTIONS > --- > -v:: > - Verbose. > + Verbose. Currently shows the commit subjects next to their > + SHA1. Whenever I see "Currently", it makes me wonder "why does it need to say that? Is there a plan to change it soon, and if so where is the plan described?". > +EXAMPLES > + > + > +git-cherry is frequently used in patch-based workflows (see > +linkgit:gitworkflows[7]) to determine if a series of patches has been > +applied by the upstream maintainer. In such a workflow you might > +create and send a topic branch like this (fill in appropriate > +arguments for `...`): I think the ASCII art commit graph that shows topology which we lost by this patch gave a more intiutive sense of what "a topic branch like this" looked like than an incomplete skeleton of a command sequence that would be understood by those who already know how to work with multiple branches. Perhaps we want both? Thanks. > ++ > + > +git checkout -b topic origin/master > +# work and create some commits > +git format-patch origin/master > +git send-email ... 00* > + > +Later, you can whether your changes have been applied by saying (still > +on `topic`): > ++ > + > +git fetch # update your notion of origin/master > +git cherry -v > + > ++ > +Note that this uses , and assumes that > +`core.autosetupmerge` is enabled (the default). > + > + > SEE ALSO > > linkgit:git-patch-id[1] -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drop support for "experimental" loose objects
On Thu, Nov 21, 2013 at 07:43:03PM +0700, Duy Nguyen wrote: > > - if (experimental_loose_object(map)) { > > Perhaps keep this.. > > > - /* > > -* The old experimental format we no longer produce; > > -* we can still read it. > > -*/ > > - used = unpack_object_header_buffer(map, mapsize, &type, > > &size); > > - if (!used || !valid_loose_object_type[type]) > > - return -1; > > - map += used; > > - mapsize -= used; > > - > > - /* Set up the stream for the rest.. */ > > - stream->next_in = map; > > - stream->avail_in = mapsize; > > - git_inflate_init(stream); > > - > > - /* And generate the fake traditional header */ > > - stream->total_out = 1 + snprintf(buffer, bufsiz, "%s %lu", > > -typename(type), size); > > - return 0; > > and replace all this with > > die("detected an object in obsolete format, please repack the > repository using a version before XXX"); That would eliminate the second part of my purpose, which is to not die() on a corrupted object because we incorrectly guess that it is experimental. If we think these objects are in the wild, the right thing to do would be to warn() and continue. But I really find it hard to believe any such objects exist at this point. -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 v6 05/10] git fetch-pack: Add --diag-url
Torsten Bögershausen writes: >> Subject: Re: [PATCH v6 05/10] git fetch-pack: Add --diag-url s/Add/add/ please. > The main purpose is to trace the URL parser called by git_connect() in > connect.c > > The main features of the parser can be listed as this: > - parse out host and path for URLs with a scheme (git:// file:// ssh://) > - parse host names embedded by [] correctly > - extract the port number, if present > - seperate URLs like "file" (which are local) > from URLs like "host:repo" which should use ssh > > Add the new parameter "--diag-url" to "git fetch-pack", > which prints the value for protocol, host and path to stderr and exits. > --- Sign-off? > builtin/fetch-pack.c | 14 ++--- > connect.c | 27 > connect.h | 1 + > fetch-pack.h | 1 + > t/t5500-fetch-pack.sh | 57 > +++ > 5 files changed, 97 insertions(+), 3 deletions(-) > > diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c > index c8e8582..758b5ac 100644 > --- a/builtin/fetch-pack.c > +++ b/builtin/fetch-pack.c > @@ -7,7 +7,7 @@ > static const char fetch_pack_usage[] = > "git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] " > "[--include-tag] [--upload-pack=] [--depth=] " > -"[--no-progress] [-v] [:] [...]"; > +"[--no-progress] [--diag-url] [-v] [:] [...]"; > > static void add_sought_entry_mem(struct ref ***sought, int *nr, int *alloc, >const char *name, int namelen) > @@ -81,6 +81,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char > *prefix) > args.stdin_refs = 1; > continue; > } > + if (!strcmp("--diag-url", arg)) { > + args.diag_url = 1; > + continue; > + } > if (!strcmp("-v", arg)) { > args.verbose = 1; > continue; > @@ -146,10 +150,14 @@ int cmd_fetch_pack(int argc, const char **argv, const > char *prefix) > fd[0] = 0; > fd[1] = 1; > } else { > + int flags = args.verbose ? CONNECT_VERBOSE : 0; > + if (args.diag_url) > + flags |= CONNECT_DIAG_URL; > conn = git_connect(fd, dest, args.uploadpack, > -args.verbose ? CONNECT_VERBOSE : 0); > +flags); > + if (!conn) > + return args.diag_url ? 0 : 1; > } > - > get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL); > > ref = fetch_pack(&args, fd, conn, ref, dest, > diff --git a/connect.c b/connect.c > index a6cf345..1b93b4d 100644 > --- a/connect.c > +++ b/connect.c > @@ -236,6 +236,19 @@ enum protocol { > PROTO_GIT > }; > > +static const char *prot_name(enum protocol protocol) { Style: please move that "{" to the beginning of the next line (see the beginning of existing functions e.g. get_protocol()). > diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh > index d87ddf7..9136f2a 100755 > --- a/t/t5500-fetch-pack.sh > +++ b/t/t5500-fetch-pack.sh > @@ -531,5 +531,62 @@ test_expect_success 'shallow fetch with tags does not > break the repository' ' > git fsck > ) > ' > +check_prot_path() { > + > actual && Style: no SP between the redirection operator and its target, i.e. >actual && > + (git fetch-pack --diag-url "$1" 2>&1 1>stdout) | grep -v host= >actual > && Do we use "stdout" in this test? Otherwise "1>/dev/null" would make it clearer what is going on. > + echo "Diag: url=$1" >expected && > + echo "Diag: protocol=$2" >>expected && > + echo "Diag: path=$3" >>expected && Perhaps this is a good place to use here-doc, i.e. cat >expected <<-EOF && Diag: ... ... EOF > + test_cmp expected actual > +} > + > +check_prot_host_path() { > + > actual && > + git fetch-pack --diag-url "$1" 2>actual && > + echo "Diag: url=$1" >expected && > + echo "Diag: protocol=$2" >>expected && > + echo "Diag: host=$3" >>expected && > + echo "Diag: path=$4" >>expected && > + test_cmp expected actual > +} > + > +for r in repo re:po re/po > +do > + # git or ssh with scheme > + for p in "ssh+git" "git+ssh" git ssh > + do > + for h in host host:12 [::1] [::1]:23 > + do > + if $(echo $p | grep ssh >/dev/null 2>/dev/null); then Style: "; then" should be spelled as "LF" followed by "then" on the next line by itself. But more ipmportantly, the above tries to do if "some computed string"; then which is very iffy. I think you meant: case "$p" in *ssh*) do ssh thing ;; *) do other thing esac -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a
[PATCH v6 07/10] connect.c: Corner case for IPv6
Commit faea9ccbadf75128 introduced "user-relative paths": "ssh://host.xz/~junio/repo" is the same as "host.xz:~junio/repo". Commit 356bece0a27258181 "Support [address] in URLs" introduced a regression: When an URL like "[::1]:/~repo" is used, the replacement of "/~" with "~" was enabled for IPv6 host names, and "[::1]:/~repo" is converted into "[::1]:~repo". Solution: Don't use "if (url != hostname)", but look at the separator instead. Rename the variable "c" into separator. --- connect.c | 14 +++--- t/t5500-fetch-pack.sh | 16 t/t5601-clone.sh | 12 ++-- 3 files changed, 13 insertions(+), 29 deletions(-) diff --git a/connect.c b/connect.c index 1b93b4d..0cb88b8 100644 --- a/connect.c +++ b/connect.c @@ -566,7 +566,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, char *url; char *host, *path; char *end; - int c; + int separator; enum protocol protocol = PROTO_LOCAL; int free_path = 0; char *port = NULL; @@ -581,10 +581,10 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, *host = '\0'; protocol = get_protocol(url); host += 3; - c = '/'; + separator = '/'; } else { host = url; - c = ':'; + separator = ':'; } /* @@ -604,9 +604,9 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, } else end = host; - path = strchr(end, c); + path = strchr(end, separator); if (path && !has_dos_drive_prefix(end)) { - if (c == ':') { + if (separator == ':') { if (host != url || path < strchrnul(host, '/')) { protocol = PROTO_SSH; *path++ = '\0'; @@ -623,7 +623,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, * null-terminate hostname and point path to ~ for URL's like this: *ssh://host.xz/~user/repo */ - if (protocol != PROTO_LOCAL && host != url) { + if (protocol != PROTO_LOCAL && separator == '/') { char *ptr = path; if (path[1] == '~') path++; @@ -638,7 +638,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, /* * Add support for ssh port: ssh://host.xy:/... */ - if (protocol == PROTO_SSH && host != url) + if (protocol == PROTO_SSH && separator == '/') port = get_port(end); *ret_host = xstrdup(host); diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 7f55b95..ac5b08b 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -604,19 +604,11 @@ do test_expect_success "fetch-pack --diag-url $h:$r" ' check_prot_path $h:$r $p "$r" ' + # No "/~" -> "~" conversion + test_expect_success "fetch-pack --diag-url $h:/~$r" ' + check_prot_host_path $h:/~$r $p "$hh" "/~$r" + ' done - h=host - hh=host - # "/~" -> "~" conversion - test_expect_failure "fetch-pack --diag-url $h:/~$r" ' - check_prot_host_path $h:/~$r $p "$hh" "~$r" - ' - h=[::1] - hh=$(echo $h | tr -d "[]") - # "/~" -> "~" conversion - test_expect_success "fetch-pack --diag-url $h:/~$r" ' - check_prot_host_path $h:/~$r $p "$hh" "~$r" - ' done test_expect_success MINGW 'fetch-pack --diag-url file://c:/repo' ' diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index ba99972..57234c0 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -356,15 +356,7 @@ do done #ipv6 -# failing -for repo in /~proj -do - test_expect_failure "clone [::1]:$repo" ' - test_clone_url [::1]:$repo ::1 $repo - ' -done - -for repo in rep rep/home/project 123 +for repo in rep rep/home/project 123 /~proj do test_expect_success "clone [::1]:$repo" ' test_clone_url [::1]:$repo ::1 $repo @@ -373,7 +365,7 @@ done # Corner cases # failing -for repo in [foo]bar/baz:qux [foo/bar]:baz +for url in [foo]bar/baz:qux [foo/bar]:baz do test_expect_failure "clone $url is not ssh" ' test_clone_url $url none -- 1.8.4.457.g424cb08 -- 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] transport: Catch non positive --depth option value
"Andrés G. Aragoneses" writes: > From 99e387151594572dc136bf1fae45593ee710e817 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Andr=C3=A9s=20G=2E=20Aragoneses?= > Date: Wed, 13 Nov 2013 16:55:08 +0100 > Subject: [PATCH] transport: Catch non positive --depth option value > > Instead of simply ignoring the value passed to --depth > option when it is zero or negative, now it is caught > and reported. > > This will let people know that they were using the > option incorrectly (as depth<0 should be simply invalid, > and under the hood depth==0 didn't have any effect). > > Signed-off-by: Andres G. Aragoneses > Reviewed-by: Duy Nguyen > Reviewed-by: Junio C Hamano I didn't exactly "review" this. Have you run the tests with this patch? It seems that it breaks quite a lot of them, including t5500, t5503, t5510, among others. > --- > transport.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/transport.c b/transport.c > index 7202b77..edd63eb 100644 > --- a/transport.c > +++ b/transport.c > @@ -483,6 +483,8 @@ static int set_git_option(struct git_transport_options > *opts, > opts->depth = strtol(value, &end, 0); > if (*end) > die("transport: invalid depth option '%s'", > value); > + if (opts->depth < 1) > + die("transport: invalid depth option '%s' (must > be positive)", value); > } > return 0; > } -- 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 v6 02/10] t5601: Add tests for ssh
Add more tests testing all the combinations: -IPv4 or IPv6 -path starting with "/" or with "/~" -with and without the ssh:// scheme Some test fail, they need updates in connect.c --- t/t5601-clone.sh | 102 +-- 1 file changed, 100 insertions(+), 2 deletions(-) diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 83b21f5..ba99972 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -298,7 +298,7 @@ setup_ssh_wrapper () { expect_ssh () { test_when_finished ' - (cd "$TRASH_DIRECTORY" && rm -f ssh-expect && >ssh-output) + (cd "$TRASH_DIRECTORY" && rm -f ssh-expect && >ssh-output) ' && { case "$1" in @@ -313,7 +313,7 @@ expect_ssh () { setup_ssh_wrapper -test_expect_success 'cloning myhost:src uses ssh' ' +test_expect_success 'clone myhost:src uses ssh' ' git clone myhost:src ssh-clone && expect_ssh myhost src ' @@ -329,6 +329,104 @@ test_expect_success 'bracketed hostnames are still ssh' ' expect_ssh myhost:123 src ' +counter=0 +# $1 url +# $2 none|host +# $3 path +test_clone_url () { + counter=$(($counter + 1)) + test_might_fail git clone "$1" tmp$counter && + expect_ssh "$2" "$3" +} + +test_expect_success NOT_MINGW 'clone c:temp is ssl' ' + test_clone_url c:temp c temp +' + +test_expect_success MINGW 'clone c:temp is dos drive' ' + test_clone_url c:temp none +' + +#ip v4 +for repo in rep rep/home/project /~proj 123 +do + test_expect_success "clone host:$repo" ' + test_clone_url host:$repo host $repo + ' +done + +#ipv6 +# failing +for repo in /~proj +do + test_expect_failure "clone [::1]:$repo" ' + test_clone_url [::1]:$repo ::1 $repo + ' +done + +for repo in rep rep/home/project 123 +do + test_expect_success "clone [::1]:$repo" ' + test_clone_url [::1]:$repo ::1 $repo + ' +done + +# Corner cases +# failing +for repo in [foo]bar/baz:qux [foo/bar]:baz +do + test_expect_failure "clone $url is not ssh" ' + test_clone_url $url none + ' +done + +for url in foo/bar:baz +do + test_expect_success "clone $url is not ssh" ' + test_clone_url $url none + ' +done + +#with ssh:// scheme +test_expect_success 'clone ssh://host.xz/home/user/repo' ' + test_clone_url "ssh://host.xz/home/user/repo" host.xz "/home/user/repo" +' + +# from home directory +test_expect_success 'clone ssh://host.xz/~repo' ' + test_clone_url "ssh://host.xz/~repo" host.xz "~repo" +' + +# with port number +test_expect_success 'clone ssh://host.xz:22/home/user/repo' ' + test_clone_url "ssh://host.xz:22/home/user/repo" "-p 22 host.xz" "/home/user/repo" +' + +# from home directory with port number +test_expect_success 'clone ssh://host.xz:22/~repo' ' + test_clone_url "ssh://host.xz:22/~repo" "-p 22 host.xz" "~repo" +' + +#IPv6 +test_expect_success 'clone ssh://[::1]/home/user/repo' ' + test_clone_url "ssh://[::1]/home/user/repo" "::1" "/home/user/repo" +' + +#IPv6 from home directory +test_expect_success 'clone ssh://[::1]/~repo' ' + test_clone_url "ssh://[::1]/~repo" "::1" "~repo" +' + +#IPv6 with port number +test_expect_success 'clone ssh://[::1]:22/home/user/repo' ' + test_clone_url "ssh://[::1]:22/home/user/repo" "-p 22 ::1" "/home/user/repo" +' + +#IPv6 from home directory with port number +test_expect_success 'clone ssh://[::1]:22/~repo' ' + test_clone_url "ssh://[::1]:22/~repo" "-p 22 ::1" "~repo" +' + test_expect_success 'clone from a repository with two identical branches' ' ( -- 1.8.4.457.g424cb08 -- 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] drop support for "experimental" loose objects
Jeff King writes: > We could try to improve the heuristic to err on the side of > normal objects in the face of corruption, but there is > really little point. The experimental format is long-dead, > and was never enabled by default to begin with. We can > instead simply remove it. The only affected repository would > be one that explicitly set core.legacyheaders in 2007, and > then never repacked in the intervening 6 years. Sounds sensible. 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 v6 04/10] git_connect: factor out discovery of the protocol and its parts
Torsten Bögershausen writes: > git_connect has grown large due to the many different protocols syntaxes > that are supported. Move the part of the function that parses the URL to > connect to into a separate function for readability. > > Signed-off-by: Johannes Sixt > --- I lost track, but was this authored by j6t? If so please: (1) begin the body of the message like so: From: Johannes Sixt git_connect has grown ... so that the resulting commit will have him as the author; and (2) have your own sign-off after his at the end, i.e. connect to into a separate function for readability. Signed-off-by: Johannes Sixt Signed-off-by: Torsten Bögershausen to record the flow of the patch. The resulting code looks fine from a cursory view. Thanks. > connect.c | 80 > ++- > 1 file changed, 53 insertions(+), 27 deletions(-) > > diff --git a/connect.c b/connect.c > index 6cc1f8d..a6cf345 100644 > --- a/connect.c > +++ b/connect.c > @@ -543,37 +543,20 @@ static char *get_port(char *host) > return NULL; > } > > -static struct child_process no_fork; > - > /* > - * This returns a dummy child_process if the transport protocol does not > - * need fork(2), or a struct child_process object if it does. Once done, > - * finish the connection with finish_connect() with the value returned from > - * this function (it is safe to call finish_connect() with NULL to support > - * the former case). > - * > - * If it returns, the connect is successful; it just dies on errors (this > - * will hopefully be changed in a libification effort, to return NULL when > - * the connection failed). > + * Extract protocol and relevant parts from the specified connection URL. > + * The caller must free() the returned strings. > */ > -struct child_process *git_connect(int fd[2], const char *url_orig, > - const char *prog, int flags) > +static enum protocol parse_connect_url(const char *url_orig, char **ret_host, > +char **ret_port, char **ret_path) > { > char *url; > char *host, *path; > char *end; > int c; > - struct child_process *conn = &no_fork; > enum protocol protocol = PROTO_LOCAL; > int free_path = 0; > char *port = NULL; > - const char **arg; > - struct strbuf cmd = STRBUF_INIT; > - > - /* Without this we cannot rely on waitpid() to tell > - * what happened to our children. > - */ > - signal(SIGCHLD, SIG_DFL); > > if (is_url(url_orig)) > url = url_decode(url_orig); > @@ -645,6 +628,49 @@ struct child_process *git_connect(int fd[2], const char > *url_orig, > if (protocol == PROTO_SSH && host != url) > port = get_port(end); > > + *ret_host = xstrdup(host); > + if (port) > + *ret_port = xstrdup(port); > + else > + *ret_port = NULL; > + if (free_path) > + *ret_path = path; > + else > + *ret_path = xstrdup(path); > + free(url); > + return protocol; > +} > + > +static struct child_process no_fork; > + > +/* > + * This returns a dummy child_process if the transport protocol does not > + * need fork(2), or a struct child_process object if it does. Once done, > + * finish the connection with finish_connect() with the value returned from > + * this function (it is safe to call finish_connect() with NULL to support > + * the former case). > + * > + * If it returns, the connect is successful; it just dies on errors (this > + * will hopefully be changed in a libification effort, to return NULL when > + * the connection failed). > + */ > +struct child_process *git_connect(int fd[2], const char *url, > + const char *prog, int flags) > +{ > + char *host, *path; > + struct child_process *conn = &no_fork; > + enum protocol protocol; > + char *port; > + const char **arg; > + struct strbuf cmd = STRBUF_INIT; > + > + /* Without this we cannot rely on waitpid() to tell > + * what happened to our children. > + */ > + signal(SIGCHLD, SIG_DFL); > + > + protocol = parse_connect_url(url, &host, &port, &path); > + > if (protocol == PROTO_GIT) { > /* These underlying connection commands die() if they >* cannot connect. > @@ -666,9 +692,9 @@ struct child_process *git_connect(int fd[2], const char > *url_orig, >prog, path, 0, >target_host, 0); > free(target_host); > - free(url); > - if (free_path) > - free(path); > + free(host); > + free(port); > + free(path); > return conn; > } > > @@ -709,9 +735,9 @@ struct child_process *git_connect(int fd[2], const char > *url_orig, > fd[
Re: [PATCH v6 06/10] t5500: Test case for diag-url
Torsten Bögershausen writes: > Add test cases using git fetch-pack --diag-url: > > - parse out host and path for URLs with a scheme (git:// file:// ssh://) > - parse host names embedded by [] correctly > - extract the port number, if present > - seperate URLs like "file" (which are local) > from URLs like "host:repo" which should use ssh > --- > t/t5500-fetch-pack.sh | 72 > +++ > 1 file changed, 56 insertions(+), 16 deletions(-) > > diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh > index 9136f2a..7f55b95 100755 > --- a/t/t5500-fetch-pack.sh > +++ b/t/t5500-fetch-pack.sh > @@ -533,7 +533,7 @@ test_expect_success 'shallow fetch with tags does not > break the repository' ' > ' > check_prot_path() { > > actual && > - (git fetch-pack --diag-url "$1" 2>&1 1>stdout) | grep -v host= >actual > && > + (git fetch-pack --diag-url "$1" 2>&1 1>stdout) | grep -v host= >actual > && What is this change about??? > echo "Diag: url=$1" >expected && > echo "Diag: protocol=$2" >>expected && > echo "Diag: path=$3" >>expected && > @@ -542,7 +542,7 @@ check_prot_path() { > > check_prot_host_path() { > > actual && > - git fetch-pack --diag-url "$1" 2>actual && > + git fetch-pack --diag-url "$1" 2>actual && Likewise... > @@ -564,29 +564,69 @@ do > hh=$h > pp=$p > fi > - test_expect_success "fetch-pack $p://$h/$r" ' > + test_expect_success "fetch-pack --diag-url $p://$h/$r" ' > check_prot_host_path $p://$h/$r $pp "$hh" "/$r" > ' > - # "/~" -> "~" conversion for git > - test_expect_success "fetch-pack $p://$h/~$r" ' > + # "/~" -> "~" conversion > + test_expect_success "fetch-pack --diag-url $p://$h/~$r" > ' Is this part, especially the "s/ for git//" bit, an "oops, the earlier one was wrong" fix to [05/10]? If so, please don't. Instead, please get it right in that patch before sending the series out. -- 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
Can a git push over ssh trigger a gc/repack? Diagnosing pack explosion
Hi git list, I am trying to diagnose a strange problem in a VM running as a 'git over ssh server', with one repo which periodically grows very quickly. The complete dataset packs to a single pack+index of ~650MB. Growth is slow, these are ASCII text reports that use a template -- highly compressible. Reports come from a few dozen machines that log in every hour. However, something is happening that explodes the efficient pack into an ungodly mess. Do client pushes over git+ssh ever trigger a repack on the server? If so, these repacking processes are racing with each other and taking 650MB to 7GB at which point we hit ENOSPC, sometimes pom killer joins the party, etc. pack dir looks like this, ordered by timestamp: http://fpaste.org/55730/04636313/ cheers, m -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- 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 v6 07/10] connect.c: Corner case for IPv6
Torsten Bögershausen writes: > Commit faea9ccbadf75128 introduced "user-relative paths": > "ssh://host.xz/~junio/repo" is the same as "host.xz:~junio/repo". > > Commit 356bece0a27258181 "Support [address] in URLs" introduced a regression: > When an URL like "[::1]:/~repo" is used, the replacement of "/~" > with "~" was enabled for IPv6 host names, and "[::1]:/~repo" is > converted into "[::1]:~repo". > > Solution: > Don't use "if (url != hostname)", but look at the separator instead. > Rename the variable "c" into separator. > --- Sign-off? The above explanation sounds sensible. Thanks. > connect.c | 14 +++--- > t/t5500-fetch-pack.sh | 16 > t/t5601-clone.sh | 12 ++-- > 3 files changed, 13 insertions(+), 29 deletions(-) > > diff --git a/connect.c b/connect.c > index 1b93b4d..0cb88b8 100644 > --- a/connect.c > +++ b/connect.c > @@ -566,7 +566,7 @@ static enum protocol parse_connect_url(const char > *url_orig, char **ret_host, > char *url; > char *host, *path; > char *end; > - int c; > + int separator; > enum protocol protocol = PROTO_LOCAL; > int free_path = 0; > char *port = NULL; > @@ -581,10 +581,10 @@ static enum protocol parse_connect_url(const char > *url_orig, char **ret_host, > *host = '\0'; > protocol = get_protocol(url); > host += 3; > - c = '/'; > + separator = '/'; > } else { > host = url; > - c = ':'; > + separator = ':'; > } > > /* > @@ -604,9 +604,9 @@ static enum protocol parse_connect_url(const char > *url_orig, char **ret_host, > } else > end = host; > > - path = strchr(end, c); > + path = strchr(end, separator); > if (path && !has_dos_drive_prefix(end)) { > - if (c == ':') { > + if (separator == ':') { > if (host != url || path < strchrnul(host, '/')) { > protocol = PROTO_SSH; > *path++ = '\0'; > @@ -623,7 +623,7 @@ static enum protocol parse_connect_url(const char > *url_orig, char **ret_host, >* null-terminate hostname and point path to ~ for URL's like this: >*ssh://host.xz/~user/repo >*/ > - if (protocol != PROTO_LOCAL && host != url) { > + if (protocol != PROTO_LOCAL && separator == '/') { > char *ptr = path; > if (path[1] == '~') > path++; > @@ -638,7 +638,7 @@ static enum protocol parse_connect_url(const char > *url_orig, char **ret_host, > /* >* Add support for ssh port: ssh://host.xy:/... >*/ > - if (protocol == PROTO_SSH && host != url) > + if (protocol == PROTO_SSH && separator == '/') > port = get_port(end); > > *ret_host = xstrdup(host); > diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh > index 7f55b95..ac5b08b 100755 > --- a/t/t5500-fetch-pack.sh > +++ b/t/t5500-fetch-pack.sh > @@ -604,19 +604,11 @@ do > test_expect_success "fetch-pack --diag-url $h:$r" ' > check_prot_path $h:$r $p "$r" > ' > + # No "/~" -> "~" conversion > + test_expect_success "fetch-pack --diag-url $h:/~$r" ' > + check_prot_host_path $h:/~$r $p "$hh" "/~$r" > + ' > done > - h=host > - hh=host > - # "/~" -> "~" conversion > - test_expect_failure "fetch-pack --diag-url $h:/~$r" ' > - check_prot_host_path $h:/~$r $p "$hh" "~$r" > - ' > - h=[::1] > - hh=$(echo $h | tr -d "[]") > - # "/~" -> "~" conversion > - test_expect_success "fetch-pack --diag-url $h:/~$r" ' > - check_prot_host_path $h:/~$r $p "$hh" "~$r" > - ' > done > > test_expect_success MINGW 'fetch-pack --diag-url file://c:/repo' ' > diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh > index ba99972..57234c0 100755 > --- a/t/t5601-clone.sh > +++ b/t/t5601-clone.sh > @@ -356,15 +356,7 @@ do > done > > #ipv6 > -# failing > -for repo in /~proj > -do > - test_expect_failure "clone [::1]:$repo" ' > - test_clone_url [::1]:$repo ::1 $repo > - ' > -done > - > -for repo in rep rep/home/project 123 > +for repo in rep rep/home/project 123 /~proj > do > test_expect_success "clone [::1]:$repo" ' > test_clone_url [::1]:$repo ::1 $repo > @@ -373,7 +365,7 @@ done > > # Corner cases > # failing > -for repo in [foo]bar/baz:qux [foo/bar]:baz > +for url in [foo]bar/baz:qux [foo/bar]:baz > do > test_expect_failure "clone $url is not ssh" ' > test_clone_url $url none -- 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 v6 09/10] connect.c: Refactor url parsing
Torsten Bögershausen writes: > Make the function is_local() from tramsport.c public and use it s/ams/ans/, perhaps? > in both transport.c and connect.c > Use a protocol "local" for URLs for the local file system. > --- > connect.c| 58 > ++-- > connect.h| 1 + > t/t5601-clone.sh | 10 +- > transport.c | 8 > 4 files changed, 33 insertions(+), 44 deletions(-) > > diff --git a/connect.c b/connect.c > index 3d174c8..95568ac 100644 > --- a/connect.c > +++ b/connect.c > @@ -232,13 +232,23 @@ int server_supports(const char *feature) > > enum protocol { > PROTO_LOCAL = 1, > + PROTO_FILE, > PROTO_SSH, > PROTO_GIT > }; > > +int is_local(const char *url) > +{ > + const char *colon = strchr(url, ':'); > + const char *slash = strchr(url, '/'); > + return !colon || (slash && slash < colon) || > + has_dos_drive_prefix(url); > +} > + > static const char *prot_name(enum protocol protocol) { > switch (protocol) { > case PROTO_LOCAL: > + case PROTO_FILE: > return "file"; > case PROTO_SSH: > return "ssh"; > @@ -260,7 +270,7 @@ static enum protocol get_protocol(const char *name) > if (!strcmp(name, "ssh+git")) > return PROTO_SSH; > if (!strcmp(name, "file")) > - return PROTO_LOCAL; > + return PROTO_FILE; > die("I don't handle protocol '%s'", name); > } > > @@ -563,9 +573,8 @@ static enum protocol parse_connect_url(const char > *url_orig, char **ret_host, > char *url; > char *host, *path; > char *end; > - int separator; > + int separator = '/'; Hmph. Doesn't this belong to the earlier patch that did s/c/separator/? > enum protocol protocol = PROTO_LOCAL; > - int free_path = 0; > > if (is_url(url_orig)) > url = url_decode(url_orig); > @@ -577,10 +586,12 @@ static enum protocol parse_connect_url(const char > *url_orig, char **ret_host, > *host = '\0'; > protocol = get_protocol(url); > host += 3; > - separator = '/'; > } else { > host = url; > - separator = ':'; > + if (!is_local(url)) { > + protocol = PROTO_SSH; > + separator = ':'; > + } > } > > /* > @@ -596,17 +607,12 @@ static enum protocol parse_connect_url(const char > *url_orig, char **ret_host, > } else > end = host; > > - path = strchr(end, separator); > - if (path && !has_dos_drive_prefix(end)) { > - if (separator == ':') { > - if (host != url || path < strchrnul(host, '/')) { > - protocol = PROTO_SSH; > - *path++ = '\0'; > - } else /* '/' in the host part, assume local path */ > - path = end; > - } > - } else > + if (protocol == PROTO_LOCAL) > + path = end; > + else if (protocol == PROTO_FILE && has_dos_drive_prefix(end)) Hmm, this is for things like... "file:///Z:/Documents/repo.git"??? > diff --git a/connect.h b/connect.h > index 527d58a..ce657b3 100644 > --- a/connect.h > +++ b/connect.h > @@ -9,5 +9,6 @@ extern int git_connection_is_socket(struct child_process > *conn); > extern int server_supports(const char *feature); > extern int parse_feature_request(const char *features, const char *feature); > extern const char *server_feature_value(const char *feature, int *len_ret); > +int is_local(const char *url); In .h files, we very strongly prefer "extern" in front. I also have to wonder if "is_local()" too generic a name for a global API function. It was a perfectly fine name for a static function within the context of transport.c, though. > #endif > diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh > index 57234c0..bd1bfd3 100755 > --- a/t/t5601-clone.sh > +++ b/t/t5601-clone.sh > @@ -364,15 +364,7 @@ do > done > > # Corner cases > -# failing > -for url in [foo]bar/baz:qux [foo/bar]:baz > -do > - test_expect_failure "clone $url is not ssh" ' > - test_clone_url $url none > - ' > -done > - > -for url in foo/bar:baz > +for url in foo/bar:baz [foo]bar/baz:qux [foo/bar]:baz > do > test_expect_success "clone $url is not ssh" ' > test_clone_url $url none > diff --git a/transport.c b/transport.c > index 7202b77..a09ba95 100644 > --- a/transport.c > +++ b/transport.c > @@ -885,14 +885,6 @@ void transport_take_over(struct transport *transport, > transport->cannot_reuse = 1; > } > > -static int is_local(const char *url) > -{ > - const char *colon = strchr(url, ':'); > - const char *slash = strchr(url, '/'); > - return !colon || (slash && slash < colon) || > - has_dos_drive_p
Re:Answer back
I would like to discuss a very important crude oil project with you,kindly revert back to me if this is your valid email address for further information. Regards, Lee -- 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 v6 10/10] git fetch: support host:/~repo
Torsten Bögershausen writes: > Since commit faea9ccb URLs host:~repo or ssh://host:/~repo > reach the home directory. > In 356bece0 support for [] was introduced, and as a side > effect, [host]:/~repo was the same as [host]:~repo. > The side effect was removed in c01049ae, "connect.c: Corner case for IPv6". > > Re-reading the documentation (in urls.txt) we find that > "ssh://host:/~repo", > "host:/~repo" or > "host:~repo" > specifiy the repository "repo" in the home directory at "host". > > So make the handling of "host:/~repo" (or "[host]:/~repo") a feature, > and revert the possible regression introduced in c01049ae. Isn't c01049ae a private copy in your repository, a part of this series? Perhaps the series needs to be (re)structured to avoid introducing a "possible regression" in the first place? Confused... > --- > connect.c | 3 +-- > t/t5500-fetch-pack.sh | 4 ++-- > t/t5601-clone.sh | 12 ++-- > 3 files changed, 13 insertions(+), 6 deletions(-) > > diff --git a/connect.c b/connect.c > index 95568ac..2cad296 100644 > --- a/connect.c > +++ b/connect.c > @@ -625,8 +625,7 @@ static enum protocol parse_connect_url(const char > *url_orig, char **ret_host, > end = path; /* Need to \0 terminate host here */ > if (separator == ':') > path++; /* path starts after ':' */ > - if ((protocol == PROTO_GIT) || > - (protocol == PROTO_SSH && separator == '/')) { > + if (protocol == PROTO_GIT || protocol == PROTO_SSH) { > if (path[1] == '~') > path++; > } > diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh > index 69a2110..7d9f18c 100755 > --- a/t/t5500-fetch-pack.sh > +++ b/t/t5500-fetch-pack.sh > @@ -601,9 +601,9 @@ do > test_expect_success "fetch-pack --diag-url $h:$r" ' > check_prot_path $h:$r $p "$r" > ' > - # No "/~" -> "~" conversion > + # Do the "/~" -> "~" conversion > test_expect_success "fetch-pack --diag-url $h:/~$r" ' > - check_prot_host_path $h:/~$r $p "$h" "/~$r" > + check_prot_host_path $h:/~$r $p "$h" "~$r" > ' > done > done > diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh > index bd1bfd3..62fbd7e 100755 > --- a/t/t5601-clone.sh > +++ b/t/t5601-clone.sh > @@ -348,7 +348,7 @@ test_expect_success MINGW 'clone c:temp is dos drive' ' > ' > > #ip v4 > -for repo in rep rep/home/project /~proj 123 > +for repo in rep rep/home/project 123 > do > test_expect_success "clone host:$repo" ' > test_clone_url host:$repo host $repo > @@ -356,12 +356,20 @@ do > done > > #ipv6 > -for repo in rep rep/home/project 123 /~proj > +for repo in rep rep/home/project 123 > do > test_expect_success "clone [::1]:$repo" ' > test_clone_url [::1]:$repo ::1 $repo > ' > done > +#home directory > +test_expect_success "clone host:/~repo" ' > + test_clone_url host:/~repo host "~repo" > +' > + > +test_expect_success "clone [::1]:/~repo" ' > + test_clone_url [::1]:/~repo ::1 "~repo" > +' > > # Corner cases > for url in foo/bar:baz [foo]bar/baz:qux [foo/bar]:baz -- 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:Answer back
I would like to discuss a very important crude oil project with you,kindly revert back to me if this is your valid email address for further information. Regards, Lee -- 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 v6 01/10] t5601: remove clear_ssh, refactor setup_ssh_wrapper
Commit 8d3d28f5 added test cases for URLs which should be ssh. Remove the function clear_ssh, use test_when_finished to clean up. Introduce the function setup_ssh_wrapper, which could be factored out together with expect_ssh. Tighten one test and use "foo:bar" instead of "./foo:bar", Helped-by: Jeff King Signed-off-by: Torsten Bögershausen --- Changes since last version: - updated t5601, thanks Peff - Split up the patch into 10 commits - Hannes suggested 2 patches - Add tests for git fetch-pack, which verifies the parsing - Added lots of test cases in t5500 via git fetch-pack --diag-url t/t5601-clone.sh | 40 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 1d1c875..83b21f5 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -280,25 +280,26 @@ test_expect_success 'clone checking out a tag' ' test_cmp fetch.expected fetch.actual ' -test_expect_success 'setup ssh wrapper' ' - write_script "$TRASH_DIRECTORY/ssh-wrapper" <<-\EOF && - echo >>"$TRASH_DIRECTORY/ssh-output" "ssh: $*" && - # throw away all but the last argument, which should be the - # command - while test $# -gt 1; do shift; done - eval "$1" - EOF - - GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" && - export GIT_SSH && - export TRASH_DIRECTORY -' - -clear_ssh () { - >"$TRASH_DIRECTORY/ssh-output" +setup_ssh_wrapper () { + test_expect_success 'setup ssh wrapper' ' + write_script "$TRASH_DIRECTORY/ssh-wrapper" <<-\EOF && + echo >>"$TRASH_DIRECTORY/ssh-output" "ssh: $*" && + # throw away all but the last argument, which should be the + # command + while test $# -gt 1; do shift; done + eval "$1" + EOF + GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" && + export GIT_SSH && + export TRASH_DIRECTORY && + >"$TRASH_DIRECTORY"/ssh-output + ' } expect_ssh () { + test_when_finished ' + (cd "$TRASH_DIRECTORY" && rm -f ssh-expect && >ssh-output) + ' && { case "$1" in none) @@ -310,21 +311,20 @@ expect_ssh () { (cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output) } +setup_ssh_wrapper + test_expect_success 'cloning myhost:src uses ssh' ' - clear_ssh && git clone myhost:src ssh-clone && expect_ssh myhost src ' test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' ' - clear_ssh && cp -R src "foo:bar" && - git clone "./foo:bar" foobar && + git clone "foo:bar" foobar && expect_ssh none ' test_expect_success 'bracketed hostnames are still ssh' ' - clear_ssh && git clone "[myhost:123]:src" ssh-bracket-clone && expect_ssh myhost:123 src ' -- 1.8.4.457.g424cb08 -- 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] glossary-content.txt: remove an obsolete paragrah
Junio C Hamano writes: An addendum. > As we can see in a later paragaph: > > The "magic signature" consists of an ASCII symbol that is not > alphanumeric. Currently only the slash `/` is recognized as a > "magic signature"... > > the correct way to read that "a character that cannot be a magic > signature" is "a character that is not 'an ASCII symbol that is not > alphanumeric'", which means you can do: > > :!/foo - with magic signatures ! and /, pattern begins at 'f' > > :/#abc - with magic signatures # and /, pattern begins at 'a', > even if in a particular version of Git, '#' magic > signature may be invalid and produce an error > > :/:#abc - with magic signature /, pattern is "#abc". > > but because the definition of "magic signature" syntax comes later > than "cannot be", it is prone to be misinterpreted as "anything that > is not currently defined as a magic signature (namely, '/')". ... and that misinterpretation may give a false impression that ":/#abc" is with the magic signature '/' (i.e. match from top) for a pattern "#abc". Doing so would mean that a version of Git that does not support a magic signature '#' will allow people and scripts to use ":/#abc" with such a meaning, and will prevent us from using '#' as a new magic signature with some useful meaning in the future. Rather, we need to force them to always spell it as ":/:#abc" even in the version of Git before the magic signature '#'. And the phrasing 'if the pattern begins with a character that cannot be a "magic signature" and is not a colon' needs to be clarified to avoid that misinterpretation for that reason. -- 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 v6 03/10] git_connect: remove artificial limit of a remote command
Since day one, function git_connect() had a limit on the command line of the command that is invoked to make a connection. 7a33bcbe converted the code that constructs the command to strbuf. This would have been the right time to remove the limit, but it did not happen. Remove it now. git_connect() uses start_command() to invoke the command; consequently, the limits of the system still apply, but are diagnosed only at execve() time. But these limits are more lenient than the 1K that git_connect() imposed. Signed-off-by: Johannes Sixt --- connect.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/connect.c b/connect.c index 06e88b0..6cc1f8d 100644 --- a/connect.c +++ b/connect.c @@ -527,8 +527,6 @@ static struct child_process *git_proxy_connect(int fd[2], char *host) return proxy; } -#define MAX_CMD_LEN 1024 - static char *get_port(char *host) { char *end; @@ -570,7 +568,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig, int free_path = 0; char *port = NULL; const char **arg; - struct strbuf cmd; + struct strbuf cmd = STRBUF_INIT; /* Without this we cannot rely on waitpid() to tell * what happened to our children. @@ -676,12 +674,9 @@ struct child_process *git_connect(int fd[2], const char *url_orig, conn = xcalloc(1, sizeof(*conn)); - strbuf_init(&cmd, MAX_CMD_LEN); strbuf_addstr(&cmd, prog); strbuf_addch(&cmd, ' '); sq_quote_buf(&cmd, path); - if (cmd.len >= MAX_CMD_LEN) - die("command line too long"); conn->in = conn->out = -1; conn->argv = arg = xcalloc(7, sizeof(*arg)); -- 1.8.4.457.g424cb08 -- 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] Revamp git-cherry(1)
Junio C Hamano writes: >> OPTIONS >> --- >> -v:: >> -Verbose. >> +Verbose. Currently shows the commit subjects next to their >> +SHA1. > > Whenever I see "Currently", it makes me wonder "why does it need to > say that? Is there a plan to change it soon, and if so where is the > plan described?". I wanted to avoid documenting exactly what it does, so that in the future it could do more than that. Is that overly paranoid? >> +EXAMPLES >> + >> + >> +git-cherry is frequently used in patch-based workflows (see >> +linkgit:gitworkflows[7]) to determine if a series of patches has been >> +applied by the upstream maintainer. In such a workflow you might >> +create and send a topic branch like this (fill in appropriate >> +arguments for `...`): > > I think the ASCII art commit graph that shows topology which we lost > by this patch gave a more intiutive sense of what "a topic branch > like this" looked like than an incomplete skeleton of a command > sequence that would be understood by those who already know how to > work with multiple branches. Perhaps we want both? Hmm. I'll ponder for a moment and try to cook something up for v2. I can't say exactly what, but after initially trying to keep it, something felt wrong to me about the ascii art. Perhaps it's that it is only vaguely related to the actual output format. -- Thomas Rast t...@thomasrast.ch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Nov 2013, #05; Thu, 21)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. Hopefully 1.8.5-rc3 that was tagged on Wednesday will be the final release candidate for this cycle. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * nd/liteal-pathspecs (2013-10-28) 1 commit (merged to 'next' on 2013-11-01 at 1a91775) + pathspec: stop --*-pathspecs impact on internal parse_pathspec() uses Fixes a regression on 'master' since v1.8.4. -- [New Topics] * jj/doc-markup-hints-in-coding-guidelines (2013-11-18) 1 commit (merged to 'next' on 2013-11-21 at 9c638a6) + State correct usage of literal examples in man pages in the coding standards Can wait in 'next'. * jn/perl-lib-extra (2013-11-18) 2 commits (merged to 'next' on 2013-11-20 at 8c90afae) + Makefile: add PERLLIB_EXTRA variable that adds to default perl path + Makefile: rebuild perl scripts when perl paths change * jj/doc-markup-gitcli (2013-11-20) 1 commit (merged to 'next' on 2013-11-21 at 5e49fa8) + Documentation/gitcli.txt: fix double quotes Can wait in 'next'. * jk/remove-experimental-loose-object-support (2013-11-21) 1 commit (merged to 'next' on 2013-11-21 at d37bab7) + drop support for "experimental" loose objects Can wait in 'next'. * jl/commit-v-strip-marker (2013-11-19) 1 commit - commit -v: strip diffs and submodule shortlogs from the commit message Perhaps another reroll for core.commentChar coming? * nd/glossary-content-pathspec-markup (2013-11-21) 1 commit (merged to 'next' on 2013-11-21 at 6072636) + glossary-content.txt: fix documentation of "**" patterns Can wait in 'next'. * nd/magic-pathspec (2013-11-20) 1 commit (merged to 'next' on 2013-11-21 at f914a30) + diff: restrict pathspec limitations to diff b/f case only Can wait in 'next'. -- [Stalled] * fc/transport-helper-fixes (2013-11-13) 12 commits - remote-bzr: support the new 'force' option - transport-helper: add support to delete branches - fast-export: add support to delete refs - fast-import: add support to delete refs - transport-helper: add support for old:new refspec - fast-export: add new --refspec option - fast-export: improve argument parsing - test-hg.sh: tests are now expected to pass - transport-helper: check for 'forced update' message - transport-helper: add 'force' to 'export' helpers - transport-helper: don't update refs in dry-run - transport-helper: mismerge fix Updates transport-helper, fast-import and fast-export to allow the ref mapping and ref deletion in a way similar to the natively supported transports. The option name "--refspec" needs to be rethought. It does not mean what refspec usually means, even though it shares the same syntax with refspec; calling it --refspec only because it shares the same syntax is like calling it --asciistring and does not make sense. * nv/commit-gpgsign-config (2013-11-06) 1 commit - Add the commit.gpgsign option to sign all commits Introduce commit.gpgsign configuration variable to force every commit to be GPG signed. Needs tests, perhaps? * tb/clone-ssh-with-colon-for-port (2013-11-04) 1 commit . git clone: is an URL local or ssh Still being reworked. * cn/thin-push-capability (2013-11-06) 2 commits - send-pack: only send a thin pack if the server supports it - receive-pack: advertise thin-pack Peff had a good suggestion to control this by expressing what the receiving end wants in a more direct way, namely to advertise a 'no-thin' trait in the capability list, which seems to be favored by Shawn, too. * jt/commit-fixes-footer (2013-10-30) 1 commit - commit: Add -f, --fixes option to add Fixes: line There is an ongoing discussion around this topic; in general I am fairly negative on a new feature that is too narrow and prefer a more generic solution that can be tailored for specific needs, as many people stated in the thread. It appears that the discussion stalled. * np/pack-v4 (2013-09-18) 90 commits . packv4-parse.c: add tree offset caching . t1050: replace one instance of show-index with verify-pack . index-pack, pack-objects: allow creating .idx v2 with .pack v4 . unpack-objects: decode v4 trees . unpack-objects: allow to save processed bytes to a buffer - ... Nico and Duy advancing the eternal vaporware pack-v4. This is here primarily for wider distribution of the preview edition. Temporarily ejected from 'pu', to try out jk/pack-bitmap, which this topic conflicts with. * jk/pack-bitmap (2013-11-18) 22 commits - compat/mingw.h: Fix the MinGW and msvc builds - pack-bitmap: implement optional name_hash cache - t/perf: add tests for pack bitmaps
[PATCH v6 06/10] t5500: Test case for diag-url
Add test cases using git fetch-pack --diag-url: - parse out host and path for URLs with a scheme (git:// file:// ssh://) - parse host names embedded by [] correctly - extract the port number, if present - seperate URLs like "file" (which are local) from URLs like "host:repo" which should use ssh --- t/t5500-fetch-pack.sh | 72 +++ 1 file changed, 56 insertions(+), 16 deletions(-) diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 9136f2a..7f55b95 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -533,7 +533,7 @@ test_expect_success 'shallow fetch with tags does not break the repository' ' ' check_prot_path() { > actual && - (git fetch-pack --diag-url "$1" 2>&1 1>stdout) | grep -v host= >actual && + (git fetch-pack --diag-url "$1" 2>&1 1>stdout) | grep -v host= >actual && echo "Diag: url=$1" >expected && echo "Diag: protocol=$2" >>expected && echo "Diag: path=$3" >>expected && @@ -542,7 +542,7 @@ check_prot_path() { check_prot_host_path() { > actual && - git fetch-pack --diag-url "$1" 2>actual && + git fetch-pack --diag-url "$1" 2>actual && echo "Diag: url=$1" >expected && echo "Diag: protocol=$2" >>expected && echo "Diag: host=$3" >>expected && @@ -564,29 +564,69 @@ do hh=$h pp=$p fi - test_expect_success "fetch-pack $p://$h/$r" ' + test_expect_success "fetch-pack --diag-url $p://$h/$r" ' check_prot_host_path $p://$h/$r $pp "$hh" "/$r" ' - # "/~" -> "~" conversion for git - test_expect_success "fetch-pack $p://$h/~$r" ' + # "/~" -> "~" conversion + test_expect_success "fetch-pack --diag-url $p://$h/~$r" ' check_prot_host_path $p://$h/~$r $pp "$hh" "~$r" ' done done + p=file # file with scheme - for p in file + for h in "" host host:12 [::1] [::1]:23 do - for h in "" host host:12 [::1] [::1]:23 - do - test_expect_success "fetch-pack $p://$h/$r" ' - check_prot_path $p://$h/$r $p "/$r" - ' - # No "/~" -> "~" conversion for file - test_expect_success "fetch-pack $p://$h/~$r" ' - check_prot_path $p://$h/~$r $p "/~$r" - ' - done + test_expect_success "fetch-pack --diag-url $p://$h/$r" ' + check_prot_path $p://$h/$r $p "/$r" + ' + # No "/~" -> "~" conversion for file + test_expect_success "fetch-pack --diag-url $p://$h/~$r" ' + check_prot_path $p://$h/~$r $p "/~$r" + ' + done + # file without scheme + for h in nohost nohost:12 [::1] [::1]:23 [ [:aa + do + test_expect_success "fetch-pack --diag-url ./$h:$r" ' + check_prot_path ./$h:$r $p "./$h:$r" + ' + # No "/~" -> "~" conversion for file + test_expect_success "fetch-pack --diag-url ./$p:$h/~$r" ' + check_prot_path ./$p:$h/~$r $p "./$p:$h/~$r" + ' + done + #ssh without scheme + p=ssh + for h in host [::1] + do + hh=$(echo $h | tr -d "[]") + test_expect_success "fetch-pack --diag-url $h:$r" ' + check_prot_path $h:$r $p "$r" + ' done + h=host + hh=host + # "/~" -> "~" conversion + test_expect_failure "fetch-pack --diag-url $h:/~$r" ' + check_prot_host_path $h:/~$r $p "$hh" "~$r" + ' + h=[::1] + hh=$(echo $h | tr -d "[]") + # "/~" -> "~" conversion + test_expect_success "fetch-pack --diag-url $h:/~$r" ' + check_prot_host_path $h:/~$r $p "$hh" "~$r" + ' done +test_expect_success MINGW 'fetch-pack --diag-url file://c:/repo' ' + check_prot_path file://c:/repo file c:/repo +' +test_expect_success MINGW 'fetch-pack --diag-url file:///c:/repo' ' + check_prot_path file://c:/repo file c:/repo +' +test_expect_success MINGW 'fetch-pack --diag-url c:repo' ' + check_prot_path c:repo file c:repo +' + test_done -- 1.8.4.457.g424cb08 -- 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 v6 04/10] git_connect: factor out discovery of the protocol and its parts
git_connect has grown large due to the many different protocols syntaxes that are supported. Move the part of the function that parses the URL to connect to into a separate function for readability. Signed-off-by: Johannes Sixt --- connect.c | 80 ++- 1 file changed, 53 insertions(+), 27 deletions(-) diff --git a/connect.c b/connect.c index 6cc1f8d..a6cf345 100644 --- a/connect.c +++ b/connect.c @@ -543,37 +543,20 @@ static char *get_port(char *host) return NULL; } -static struct child_process no_fork; - /* - * This returns a dummy child_process if the transport protocol does not - * need fork(2), or a struct child_process object if it does. Once done, - * finish the connection with finish_connect() with the value returned from - * this function (it is safe to call finish_connect() with NULL to support - * the former case). - * - * If it returns, the connect is successful; it just dies on errors (this - * will hopefully be changed in a libification effort, to return NULL when - * the connection failed). + * Extract protocol and relevant parts from the specified connection URL. + * The caller must free() the returned strings. */ -struct child_process *git_connect(int fd[2], const char *url_orig, - const char *prog, int flags) +static enum protocol parse_connect_url(const char *url_orig, char **ret_host, + char **ret_port, char **ret_path) { char *url; char *host, *path; char *end; int c; - struct child_process *conn = &no_fork; enum protocol protocol = PROTO_LOCAL; int free_path = 0; char *port = NULL; - const char **arg; - struct strbuf cmd = STRBUF_INIT; - - /* Without this we cannot rely on waitpid() to tell -* what happened to our children. -*/ - signal(SIGCHLD, SIG_DFL); if (is_url(url_orig)) url = url_decode(url_orig); @@ -645,6 +628,49 @@ struct child_process *git_connect(int fd[2], const char *url_orig, if (protocol == PROTO_SSH && host != url) port = get_port(end); + *ret_host = xstrdup(host); + if (port) + *ret_port = xstrdup(port); + else + *ret_port = NULL; + if (free_path) + *ret_path = path; + else + *ret_path = xstrdup(path); + free(url); + return protocol; +} + +static struct child_process no_fork; + +/* + * This returns a dummy child_process if the transport protocol does not + * need fork(2), or a struct child_process object if it does. Once done, + * finish the connection with finish_connect() with the value returned from + * this function (it is safe to call finish_connect() with NULL to support + * the former case). + * + * If it returns, the connect is successful; it just dies on errors (this + * will hopefully be changed in a libification effort, to return NULL when + * the connection failed). + */ +struct child_process *git_connect(int fd[2], const char *url, + const char *prog, int flags) +{ + char *host, *path; + struct child_process *conn = &no_fork; + enum protocol protocol; + char *port; + const char **arg; + struct strbuf cmd = STRBUF_INIT; + + /* Without this we cannot rely on waitpid() to tell +* what happened to our children. +*/ + signal(SIGCHLD, SIG_DFL); + + protocol = parse_connect_url(url, &host, &port, &path); + if (protocol == PROTO_GIT) { /* These underlying connection commands die() if they * cannot connect. @@ -666,9 +692,9 @@ struct child_process *git_connect(int fd[2], const char *url_orig, prog, path, 0, target_host, 0); free(target_host); - free(url); - if (free_path) - free(path); + free(host); + free(port); + free(path); return conn; } @@ -709,9 +735,9 @@ struct child_process *git_connect(int fd[2], const char *url_orig, fd[0] = conn->out; /* read from child's stdout */ fd[1] = conn->in; /* write to child's stdin */ strbuf_release(&cmd); - free(url); - if (free_path) - free(path); + free(host); + free(port); + free(path); return conn; } -- 1.8.4.457.g424cb08 -- 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 v6 01/10] t5601: remove clear_ssh, refactor setup_ssh_wrapper
Commit 8d3d28f5 added test cases for URLs which should be ssh. Remove the function clear_ssh, use test_when_finished to clean up. Introduce the function setup_ssh_wrapper, which could be factored out together with expect_ssh. Tighten one test and use "foo:bar" instead of "./foo:bar", Helped-by: Jeff King Signed-off-by: Torsten Bögershausen --- Changes since last version: - updated t5601, thanks Peff - Split up the patch into 10 commits - Hannes suggested 2 patches - Add tests for git fetch-pack, which verifies the parsing - Added lots of test cases in t5500 via git fetch-pack --diag-url t/t5601-clone.sh | 40 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 1d1c875..83b21f5 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -280,25 +280,26 @@ test_expect_success 'clone checking out a tag' ' test_cmp fetch.expected fetch.actual ' -test_expect_success 'setup ssh wrapper' ' - write_script "$TRASH_DIRECTORY/ssh-wrapper" <<-\EOF && - echo >>"$TRASH_DIRECTORY/ssh-output" "ssh: $*" && - # throw away all but the last argument, which should be the - # command - while test $# -gt 1; do shift; done - eval "$1" - EOF - - GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" && - export GIT_SSH && - export TRASH_DIRECTORY -' - -clear_ssh () { - >"$TRASH_DIRECTORY/ssh-output" +setup_ssh_wrapper () { + test_expect_success 'setup ssh wrapper' ' + write_script "$TRASH_DIRECTORY/ssh-wrapper" <<-\EOF && + echo >>"$TRASH_DIRECTORY/ssh-output" "ssh: $*" && + # throw away all but the last argument, which should be the + # command + while test $# -gt 1; do shift; done + eval "$1" + EOF + GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" && + export GIT_SSH && + export TRASH_DIRECTORY && + >"$TRASH_DIRECTORY"/ssh-output + ' } expect_ssh () { + test_when_finished ' + (cd "$TRASH_DIRECTORY" && rm -f ssh-expect && >ssh-output) + ' && { case "$1" in none) @@ -310,21 +311,20 @@ expect_ssh () { (cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output) } +setup_ssh_wrapper + test_expect_success 'cloning myhost:src uses ssh' ' - clear_ssh && git clone myhost:src ssh-clone && expect_ssh myhost src ' test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' ' - clear_ssh && cp -R src "foo:bar" && - git clone "./foo:bar" foobar && + git clone "foo:bar" foobar && expect_ssh none ' test_expect_success 'bracketed hostnames are still ssh' ' - clear_ssh && git clone "[myhost:123]:src" ssh-bracket-clone && expect_ssh myhost:123 src ' -- 1.8.4.457.g424cb08 -- 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] Revamp git-cherry(1)
Jeff King writes: > On Thu, Nov 21, 2013 at 12:30:56PM +0100, Thomas Rast wrote: > >> +Later, you can whether your changes have been applied by saying (still >> +on `topic`): > > s/can/& see/ ? > >> +Note that this uses , and assumes that >> +`core.autosetupmerge` is enabled (the default). > > I couldn't quite parse this. Is there a word missing before the comma, > or is it "uses and assumes that..."? I will just let this stand as evidence that I had a bad day. Two sentences ruined by botched editing in all of five paragraphs. Sheesh. Thanks for reading carefully. > Given that it is the default, I wonder if it is worth mentioning at all. > Even I, who knows what autosetupmerge does, took a minute to figure out > why it is relevant here. I suspect it may just confuse most readers. Ok, then let's remove it. -- Thomas Rast t...@thomasrast.ch -- 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] transport: Catch non positive --depth option value
On Fri, Nov 22, 2013 at 3:18 AM, Junio C Hamano wrote: > "Andrés G. Aragoneses" writes: > >> From 99e387151594572dc136bf1fae45593ee710e817 Mon Sep 17 00:00:00 2001 >> From: =?UTF-8?q?Andr=C3=A9s=20G=2E=20Aragoneses?= >> Date: Wed, 13 Nov 2013 16:55:08 +0100 >> Subject: [PATCH] transport: Catch non positive --depth option value >> >> Instead of simply ignoring the value passed to --depth >> option when it is zero or negative, now it is caught >> and reported. >> >> This will let people know that they were using the >> option incorrectly (as depth<0 should be simply invalid, >> and under the hood depth==0 didn't have any effect). >> >> Signed-off-by: Andres G. Aragoneses >> Reviewed-by: Duy Nguyen >> Reviewed-by: Junio C Hamano > > I didn't exactly "review" this. > > Have you run the tests with this patch? It seems that it breaks > quite a lot of them, including t5500, t5503, t5510, among others. I guess it's caused by builtin/fetch.c:backfill_tags(). And the call could be replaced with transport_set_option(transport, TRANS_OPT_DEPTH, NULL); > >> --- >> transport.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/transport.c b/transport.c >> index 7202b77..edd63eb 100644 >> --- a/transport.c >> +++ b/transport.c >> @@ -483,6 +483,8 @@ static int set_git_option(struct git_transport_options >> *opts, >> opts->depth = strtol(value, &end, 0); >> if (*end) >> die("transport: invalid depth option '%s'", >> value); >> + if (opts->depth < 1) >> + die("transport: invalid depth option '%s' >> (must be positive)", value); >> } >> return 0; >> } > -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3] transport: Catch non positive --depth option value
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] drop support for "experimental" loose objects
On Thu, Nov 21, 2013 at 5:04 PM, Joey Hess wrote: > > BTW, I've also seen git cat-file --batch report wrong sizes for objects, > sometimes without crashing. This is particularly problimatic because if > the object size is wrong, it's very hard to detect the actual end of the > object output in the batch mode stream. Yeah, I think it might report wrong size in case of replaced objects for example. I looked at that following Junio's comment about the sha1_object_info() API, which, unlike read_sha1_file() API, does not interact with the "replace" mechanism: http://thread.gmane.org/gmane.comp.version-control.git/234023/ I started to work on a patch about this but didn't take the time to finish and post it. Thanks, Christian. -- 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-svn: Support svn:global-ignores property
--- Documentation/git-svn.txt | 12 ++-- git-svn.perl | 46 -- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt index 30c5ee2..0c1cd46 100644 --- a/Documentation/git-svn.txt +++ b/Documentation/git-svn.txt @@ -405,14 +405,14 @@ Any other arguments are passed directly to 'git log' independently of 'git svn' functions. 'create-ignore':: - Recursively finds the svn:ignore property on directories and - creates matching .gitignore files. The resulting files are staged to - be committed, but are not committed. Use -r/--revision to refer to a - specific revision. + Recursively finds svn:ignore and svn:global-ignores properties on + directories and creates matching .gitignore files. The resulting + files are staged to be committed, but are not committed. + Use -r/--revision to refer to a specific revision. 'show-ignore':: - Recursively finds and lists the svn:ignore property on - directories. The output is suitable for appending to + Recursively finds and lists svn:ignore and svn:global-ignores + properties on directories. The output is suitable for appending to the $GIT_DIR/info/exclude file. 'mkdirs':: diff --git a/git-svn.perl b/git-svn.perl index 7349ffe..a2565e1 100755 --- a/git-svn.perl +++ b/git-svn.perl @@ -1261,6 +1261,15 @@ sub cmd_rebase { } } +sub get_svn_ignore { + my ($props, $prop) = @_; + my $s = $props->{$prop} or return; + $s =~ s/[\r\n]+/\n/g; + $s =~ s/^\n+//; + chomp $s; + return $s; +} + sub cmd_show_ignore { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; @@ -1268,12 +1277,17 @@ sub cmd_show_ignore { $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; print STDOUT "\n# $path\n"; - my $s = $props->{'svn:ignore'} or return; - $s =~ s/[\r\n]+/\n/g; - $s =~ s/^\n+//; - chomp $s; - $s =~ s#^#$path#gm; - print STDOUT "$s\n"; + my $s = &get_svn_ignore($props, 'svn:ignore'); + my $s_global = &get_svn_ignore($props, 'svn:global-ignores'); + $s or $s_global or return; + if ($s) { + $s =~ s#^#$path#gm; + print STDOUT "$s\n"; + } + if ($s_global) { + $s_global =~ s#^#$path**/#gm; + print STDOUT "$s_global\n"; + } }); } @@ -1304,16 +1318,20 @@ sub cmd_create_ignore { # which git won't track mkpath([$path]) unless -d $path; my $ignore = $path . '.gitignore'; - my $s = $props->{'svn:ignore'} or return; + my $s = &get_svn_ignore($props, 'svn:ignore'); + my $s_global = &get_svn_ignore($props, 'svn:global-ignores'); + $s or $s_global or return; open(GITIGNORE, '>', $ignore) or fatal("Failed to open `$ignore' for writing: $!"); - $s =~ s/[\r\n]+/\n/g; - $s =~ s/^\n+//; - chomp $s; - # Prefix all patterns so that the ignore doesn't apply - # to sub-directories. - $s =~ s#^#/#gm; - print GITIGNORE "$s\n"; + if ($s) { + # Prefix all patterns so that the ignore doesn't apply + # to sub-directories. + $s =~ s#^#/#gm; + print GITIGNORE "$s\n"; + } + if ($s_global) { + print GITIGNORE "$s_global\n"; + } close(GITIGNORE) or fatal("Failed to close `$ignore': $!"); command_noisy('add', '-f', $ignore); -- 1.8.3.msysgit.0 -- 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] Support pathspec magic :(exclude) and its short form :-
Duy Nguyen writes: > Btw I'm thinking of extending pathspec magic syntax a bit to allow > path completion. Right now the user has to write > > git log -- :-Documentation > > which does not play well with path completion. I'm thinking of accepting > > git log -- :- Documentation Please don't. That does not help our users, but actively harm them. They have to stop and wonder why a single pathspec is spelled as two tokens on the command line of some other people. Doing that stupidity only to help those who polish the tool (namely, "bash completion") to be lazy is doubly wrong (in the meantime, the users can type your second variant and then edit the result). For the same reason why I do not think rewriting echo "hello, world!" to echo "hello, world-" only to work around a pitfall of a particular tool (namely "bash") makes any sense, I do not think it makes sense to make _our_ tool inconsistent by using "!excluded" in the files (and --exclude) and "-not this pattern" only here. >>> + if (nr_exclude == n) >>> + die(_("There is nothing to exclude from by :(exclude) >>> patterns.\n" >>> + "Perhaps you forgot to add either ':/' or '.' ?")); >> >> ;-). > > Hey it was originally not there,... I am not objecting. I noticed it and was commending on it as "a nice touch" ;-) >>> + /* >>> + * # | positive | negative | result >>> + * -+--+--+--- >>> + * 1..4 | -1 |* | -1 >>> + * 5..8 |0 |* | 0 >>> + * 9 |1 | -1 | 1 >>> + * 10 |1 |0 | 1 >>> + * 11 |1 |1 | 0 >>> + * 12 |1 |2 | 0 >>> + * 13 |2 | -1 | 2 >>> + * 14 |2 |0 | 2 >>> + * 15 |2 |1 | 0 >>> + * 16 |2 |2 | -1 >>> + */ >> >> Not sure what this case-table means... > > Sorry, because tree_entry_interesting_1() returns more than "match > or not", we need to combine the result from positive pathspec with > the negative one to correctly handle all_not_interesting and > all_interesting. This table sums it up. I'll add more explanation > in the next patch. I managed to have guessed what the three columns on the right meant; I was wondering about the meaning of the "#" column and where it is defined/explained. -- 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] glossary-content.txt: remove an obsolete paragrah
On Fri, Nov 22, 2013 at 6:55 AM, Junio C Hamano wrote: > Junio C Hamano writes: > > An addendum. > >> As we can see in a later paragaph: >> >> The "magic signature" consists of an ASCII symbol that is not >> alphanumeric. Currently only the slash `/` is recognized as a >> "magic signature"... >> >> the correct way to read that "a character that cannot be a magic >> signature" is "a character that is not 'an ASCII symbol that is not >> alphanumeric'", which means you can do: >> >> :!/foo - with magic signatures ! and /, pattern begins at 'f' >> >> :/#abc - with magic signatures # and /, pattern begins at 'a', >> even if in a particular version of Git, '#' magic >> signature may be invalid and produce an error >> >> :/:#abc - with magic signature /, pattern is "#abc". >> >> but because the definition of "magic signature" syntax comes later >> than "cannot be", it is prone to be misinterpreted as "anything that >> is not currently defined as a magic signature (namely, '/')". > > ... and that misinterpretation may give a false impression that > > ":/#abc" > > is with the magic signature '/' (i.e. match from top) for a pattern > "#abc". > > Doing so would mean that a version of Git that does not support a > magic signature '#' will allow people and scripts to use ":/#abc" > with such a meaning, and will prevent us from using '#' as a new > magic signature with some useful meaning in the future. Rather, we > need to force them to always spell it as ":/:#abc" even in the > version of Git before the magic signature '#'. Current version does force that. $ git log -- ':/#aa' fatal: Unimplemented pathspec magic '#' in ':/#aa' Not sure if there are tests for it though. I'll add an advice to do ":/:#aa". > And the phrasing 'if the pattern begins with a character that cannot > be a "magic signature" and is not a colon' needs to be clarified to > avoid that misinterpretation for that reason. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Can a git push over ssh trigger a gc/repack? Diagnosing pack explosion
On Thu, Nov 21, 2013 at 2:52 PM, Junio C Hamano wrote: >> - if it's receiving from many pushers, it races with itself; needs >> some lock or back-off mechanism > > Surely. > > I think these should help: > > 64a99eb4 (gc: reject if another gc is running, unless --force is given, > 2013-08-08) > 4c5baf02 (gc: remove gc.pid file at end of execution, 2013-10-16) > > They should be in the upcoming v1.8.5. Ah, great to hear. For the record, this hit me on git 1.7.1, current on RHEL6. thanks! m -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- 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] drop support for "experimental" loose objects
On Thu, Nov 21, 2013 at 6:48 PM, Jeff King wrote: > @@ -1514,14 +1469,6 @@ unsigned long unpack_object_header_buffer(const > unsigned char *buf, > > int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned > long mapsize, void *buffer, unsigned long bufsiz) > { > - unsigned long size, used; > - static const char valid_loose_object_type[8] = { > - 0, /* OBJ_EXT */ > - 1, 1, 1, 1, /* "commit", "tree", "blob", "tag" */ > - 0, /* "delta" and others are invalid in a loose object */ > - }; > - enum object_type type; > - > /* Get the data stream */ > memset(stream, 0, sizeof(*stream)); > stream->next_in = map; > @@ -1529,27 +1476,6 @@ int unpack_sha1_header(git_zstream *stream, unsigned > char *map, unsigned long ma > stream->next_out = buffer; > stream->avail_out = bufsiz; > > - if (experimental_loose_object(map)) { Perhaps keep this.. > - /* > -* The old experimental format we no longer produce; > -* we can still read it. > -*/ > - used = unpack_object_header_buffer(map, mapsize, &type, > &size); > - if (!used || !valid_loose_object_type[type]) > - return -1; > - map += used; > - mapsize -= used; > - > - /* Set up the stream for the rest.. */ > - stream->next_in = map; > - stream->avail_in = mapsize; > - git_inflate_init(stream); > - > - /* And generate the fake traditional header */ > - stream->total_out = 1 + snprintf(buffer, bufsiz, "%s %lu", > -typename(type), size); > - return 0; and replace all this with die("detected an object in obsolete format, please repack the repository using a version before XXX"); ? > - } > git_inflate_init(stream); > return git_inflate(stream, 0); > } -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 10/10] git fetch: support host:/~repo
Since commit faea9ccb URLs host:~repo or ssh://host:/~repo reach the home directory. In 356bece0 support for [] was introduced, and as a side effect, [host]:/~repo was the same as [host]:~repo. The side effect was removed in c01049ae, "connect.c: Corner case for IPv6". Re-reading the documentation (in urls.txt) we find that "ssh://host:/~repo", "host:/~repo" or "host:~repo" specifiy the repository "repo" in the home directory at "host". So make the handling of "host:/~repo" (or "[host]:/~repo") a feature, and revert the possible regression introduced in c01049ae. --- connect.c | 3 +-- t/t5500-fetch-pack.sh | 4 ++-- t/t5601-clone.sh | 12 ++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/connect.c b/connect.c index 95568ac..2cad296 100644 --- a/connect.c +++ b/connect.c @@ -625,8 +625,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, end = path; /* Need to \0 terminate host here */ if (separator == ':') path++; /* path starts after ':' */ - if ((protocol == PROTO_GIT) || - (protocol == PROTO_SSH && separator == '/')) { + if (protocol == PROTO_GIT || protocol == PROTO_SSH) { if (path[1] == '~') path++; } diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 69a2110..7d9f18c 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -601,9 +601,9 @@ do test_expect_success "fetch-pack --diag-url $h:$r" ' check_prot_path $h:$r $p "$r" ' - # No "/~" -> "~" conversion + # Do the "/~" -> "~" conversion test_expect_success "fetch-pack --diag-url $h:/~$r" ' - check_prot_host_path $h:/~$r $p "$h" "/~$r" + check_prot_host_path $h:/~$r $p "$h" "~$r" ' done done diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index bd1bfd3..62fbd7e 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -348,7 +348,7 @@ test_expect_success MINGW 'clone c:temp is dos drive' ' ' #ip v4 -for repo in rep rep/home/project /~proj 123 +for repo in rep rep/home/project 123 do test_expect_success "clone host:$repo" ' test_clone_url host:$repo host $repo @@ -356,12 +356,20 @@ do done #ipv6 -for repo in rep rep/home/project 123 /~proj +for repo in rep rep/home/project 123 do test_expect_success "clone [::1]:$repo" ' test_clone_url [::1]:$repo ::1 $repo ' done +#home directory +test_expect_success "clone host:/~repo" ' + test_clone_url host:/~repo host "~repo" +' + +test_expect_success "clone [::1]:/~repo" ' + test_clone_url [::1]:/~repo ::1 "~repo" +' # Corner cases for url in foo/bar:baz [foo]bar/baz:qux [foo/bar]:baz -- 1.8.4.457.g424cb08 -- 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] transport: Catch non positive --depth option value
>From 99e387151594572dc136bf1fae45593ee710e817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20G=2E=20Aragoneses?= Date: Wed, 13 Nov 2013 16:55:08 +0100 Subject: [PATCH] transport: Catch non positive --depth option value Instead of simply ignoring the value passed to --depth option when it is zero or negative, now it is caught and reported. This will let people know that they were using the option incorrectly (as depth<0 should be simply invalid, and under the hood depth==0 didn't have any effect). Signed-off-by: Andres G. Aragoneses Reviewed-by: Duy Nguyen Reviewed-by: Junio C Hamano --- transport.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/transport.c b/transport.c index 7202b77..edd63eb 100644 --- a/transport.c +++ b/transport.c @@ -483,6 +483,8 @@ static int set_git_option(struct git_transport_options *opts, opts->depth = strtol(value, &end, 0); if (*end) die("transport: invalid depth option '%s'", value); + if (opts->depth < 1) + die("transport: invalid depth option '%s' (must be positive)", value); } return 0; } -- 1.8.1.2 -- 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: Can a git push over ssh trigger a gc/repack? Diagnosing pack explosion
On Thu, Nov 21, 2013 at 10:21 AM, Martin Langhoff wrote: > Do client pushes over git+ssh ever trigger a repack on the server? man git-config [snip] receive.autogc By default, git-receive-pack will run "git-gc --auto" after receiving data from git-push and updating refs. You can stop it by setting this variable to false. Oops! Ok, couple problems here: - if it's receiving from many pushers, it races with itself; needs some lock or back-off mechanism - alternatively, an splay mechanism. We have a "hard" threshold... given many "pushers" acting in parallel, they'll all hit the threshold at the same time. There is no need for this, we could randomize the threshold by 20%; that would radically reduce the racy-ness - auto repack in this scenario has a reasonable likelihood if being visited by the OOM killer -- therefore it needs to fail more gracefully, for example with tmpfile cleanup. Perhaps by having the tmpfiles places in a tmpdir named with the pid of the child would make this easier... Naturally, I'll move quickly to disable this evil-spawn-automagic setting and setup a cronjob. But I think it is possible to have defaults that work more reliably and with lower risk of explosion. thoughts? m -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- 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] drop support for "experimental" loose objects
Duy Nguyen writes: > On Thu, Nov 21, 2013 at 6:48 PM, Jeff King wrote: >> @@ -1514,14 +1469,6 @@ unsigned long unpack_object_header_buffer(const >> unsigned char *buf, >> >> int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned >> long mapsize, void *buffer, unsigned long bufsiz) >> { >> - unsigned long size, used; >> - static const char valid_loose_object_type[8] = { >> - 0, /* OBJ_EXT */ >> - 1, 1, 1, 1, /* "commit", "tree", "blob", "tag" */ >> - 0, /* "delta" and others are invalid in a loose object */ >> - }; >> - enum object_type type; >> - >> /* Get the data stream */ >> memset(stream, 0, sizeof(*stream)); >> stream->next_in = map; >> @@ -1529,27 +1476,6 @@ int unpack_sha1_header(git_zstream *stream, unsigned >> char *map, unsigned long ma >> stream->next_out = buffer; >> stream->avail_out = bufsiz; >> >> - if (experimental_loose_object(map)) { > > Perhaps keep this.. > >> - /* >> -* The old experimental format we no longer produce; >> -* we can still read it. >> -*/ >> - used = unpack_object_header_buffer(map, mapsize, &type, >> &size); >> - if (!used || !valid_loose_object_type[type]) >> - return -1; >> - map += used; >> - mapsize -= used; >> - >> - /* Set up the stream for the rest.. */ >> - stream->next_in = map; >> - stream->avail_in = mapsize; >> - git_inflate_init(stream); >> - >> - /* And generate the fake traditional header */ >> - stream->total_out = 1 + snprintf(buffer, bufsiz, "%s %lu", >> -typename(type), size); >> - return 0; > > and replace all this with > > die("detected an object in obsolete format, please repack the > repository using a version before XXX"); > > ? Wouldn't that fail to solve the issue of `git fsck` dying on corrupt data? experimental_loose_object() would need to be rewritten to be more conservative in deciding that an object was in the experimental loose object format. -Keshav -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drop support for "experimental" loose objects
On Thu, Nov 21, 2013 at 12:04:26PM -0400, Joey Hess wrote: > Jeff King wrote: > > This latter behavior is much worse for two reasons. One, > > git reports an allocation error when the real error is > > corruption. And two, the program dies unconditionally, so > > you cannot even run fsck (which would otherwise ignore the > > broken object and keep going). > > BTW, I've also seen git cat-file --batch report wrong sizes for objects, > sometimes without crashing. This is particularly problimatic because if > the object size is wrong, it's very hard to detect the actual end of the > object output in the batch mode stream. Hrm. For --batch, I'd think we would open the whole object and notice the corruption, even with the current code. But for --batch-check, we use sha1_object_info, and for an "experimental" object, we do not need to de-zlib the object at all. So we end up reporting whatever crap we decipher from the garbage bytes. My patch would fix that, as we would not incorrectly guess an object is experimental anymore. If you have specific cases that trigger even after my patch, I'd be interested to see them. -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: Can a git push over ssh trigger a gc/repack? Diagnosing pack explosion
Martin Langhoff writes: > On Thu, Nov 21, 2013 at 10:21 AM, Martin Langhoff > wrote: >> Do client pushes over git+ssh ever trigger a repack on the server? > > man git-config > [snip] > >receive.autogc >By default, git-receive-pack will run "git-gc --auto" after >receiving data from git-push and updating refs. You can stop it by >setting this variable to false. > > Oops! > > Ok, couple problems here: > > - if it's receiving from many pushers, it races with itself; needs > some lock or back-off mechanism Surely. I think these should help: 64a99eb4 (gc: reject if another gc is running, unless --force is given, 2013-08-08) 4c5baf02 (gc: remove gc.pid file at end of execution, 2013-10-16) They should be in the upcoming v1.8.5. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 08/10] git_connect(): Refactor the port handling
Use get_host_and_port() even for ssh. Remove the variable port git_connect(), and simplify parse_connect_url() Use only one return point in git_connect(), doing the free() and return conn. t5601 had 2 corner test cases which now pass. --- connect.c | 47 +-- t/t5500-fetch-pack.sh | 9 +++-- 2 files changed, 16 insertions(+), 40 deletions(-) diff --git a/connect.c b/connect.c index 0cb88b8..3d174c8 100644 --- a/connect.c +++ b/connect.c @@ -540,16 +540,13 @@ static struct child_process *git_proxy_connect(int fd[2], char *host) return proxy; } -static char *get_port(char *host) +static const char *get_port_numeric(const char *p) { char *end; - char *p = strchr(host, ':'); - if (p) { long port = strtol(p + 1, &end, 10); if (end != p + 1 && *end == '\0' && 0 <= port && port < 65536) { - *p = '\0'; - return p+1; + return p; } } @@ -561,7 +558,7 @@ static char *get_port(char *host) * The caller must free() the returned strings. */ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, - char **ret_port, char **ret_path) + char **ret_path) { char *url; char *host, *path; @@ -569,7 +566,6 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, int separator; enum protocol protocol = PROTO_LOCAL; int free_path = 0; - char *port = NULL; if (is_url(url_orig)) url = url_decode(url_orig); @@ -588,16 +584,12 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, } /* -* Don't do destructive transforms with git:// as that -* protocol code does '[]' unwrapping of its own. +* Don't do destructive transforms as protocol code does +* '[]' unwrapping in get_host_and_port() */ if (host[0] == '[') { end = strchr(host + 1, ']'); if (end) { - if (protocol != PROTO_GIT) { - *end = 0; - host++; - } end++; } else end = host; @@ -635,17 +627,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, *ptr = '\0'; } - /* -* Add support for ssh port: ssh://host.xy:/... -*/ - if (protocol == PROTO_SSH && separator == '/') - port = get_port(end); - *ret_host = xstrdup(host); - if (port) - *ret_port = xstrdup(port); - else - *ret_port = NULL; if (free_path) *ret_path = path; else @@ -673,7 +655,6 @@ struct child_process *git_connect(int fd[2], const char *url, char *host, *path; struct child_process *conn = &no_fork; enum protocol protocol; - char *port; const char **arg; struct strbuf cmd = STRBUF_INIT; @@ -682,18 +663,13 @@ struct child_process *git_connect(int fd[2], const char *url, */ signal(SIGCHLD, SIG_DFL); - protocol = parse_connect_url(url, &host, &port, &path); + protocol = parse_connect_url(url, &host, &path); if (flags & CONNECT_DIAG_URL) { fprintf(stderr, "Diag: url=%s\n", url ? url : "NULL"); fprintf(stderr, "Diag: protocol=%s\n", prot_name(protocol)); - fprintf(stderr, "Diag: host=%s", host ? host : "NULL"); - if (port) - fprintf(stderr, ":%s\n", port); - else - fprintf(stderr, "\n"); + fprintf(stderr, "Diag: host=%s\n", host ? host : "NULL"); fprintf(stderr, "Diag: path=%s\n", path ? path : "NULL"); free(host); - free(port); free(path); return NULL; } @@ -720,7 +696,6 @@ struct child_process *git_connect(int fd[2], const char *url, target_host, 0); free(target_host); free(host); - free(port); free(path); return conn; } @@ -736,6 +711,11 @@ struct child_process *git_connect(int fd[2], const char *url, if (protocol == PROTO_SSH) { const char *ssh = getenv("GIT_SSH"); int putty = ssh && strcasestr(ssh, "plink"); + char *ssh_host = host; /* keep host for the free() below */ + const char *port = NULL; + get_host_and_port(&ssh_host, &port); + port = get_port_numeric(port); + if (!ssh) ssh = "ssh"; *arg++ = ssh;
[PATCH v6 05/10] git fetch-pack: Add --diag-url
The main purpose is to trace the URL parser called by git_connect() in connect.c The main features of the parser can be listed as this: - parse out host and path for URLs with a scheme (git:// file:// ssh://) - parse host names embedded by [] correctly - extract the port number, if present - seperate URLs like "file" (which are local) from URLs like "host:repo" which should use ssh Add the new parameter "--diag-url" to "git fetch-pack", which prints the value for protocol, host and path to stderr and exits. --- builtin/fetch-pack.c | 14 ++--- connect.c | 27 connect.h | 1 + fetch-pack.h | 1 + t/t5500-fetch-pack.sh | 57 +++ 5 files changed, 97 insertions(+), 3 deletions(-) diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index c8e8582..758b5ac 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -7,7 +7,7 @@ static const char fetch_pack_usage[] = "git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] " "[--include-tag] [--upload-pack=] [--depth=] " -"[--no-progress] [-v] [:] [...]"; +"[--no-progress] [--diag-url] [-v] [:] [...]"; static void add_sought_entry_mem(struct ref ***sought, int *nr, int *alloc, const char *name, int namelen) @@ -81,6 +81,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) args.stdin_refs = 1; continue; } + if (!strcmp("--diag-url", arg)) { + args.diag_url = 1; + continue; + } if (!strcmp("-v", arg)) { args.verbose = 1; continue; @@ -146,10 +150,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) fd[0] = 0; fd[1] = 1; } else { + int flags = args.verbose ? CONNECT_VERBOSE : 0; + if (args.diag_url) + flags |= CONNECT_DIAG_URL; conn = git_connect(fd, dest, args.uploadpack, - args.verbose ? CONNECT_VERBOSE : 0); + flags); + if (!conn) + return args.diag_url ? 0 : 1; } - get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL); ref = fetch_pack(&args, fd, conn, ref, dest, diff --git a/connect.c b/connect.c index a6cf345..1b93b4d 100644 --- a/connect.c +++ b/connect.c @@ -236,6 +236,19 @@ enum protocol { PROTO_GIT }; +static const char *prot_name(enum protocol protocol) { + switch (protocol) { + case PROTO_LOCAL: + return "file"; + case PROTO_SSH: + return "ssh"; + case PROTO_GIT: + return "git"; + default: + return "unkown protocol"; + } +} + static enum protocol get_protocol(const char *name) { if (!strcmp(name, "ssh")) @@ -670,6 +683,20 @@ struct child_process *git_connect(int fd[2], const char *url, signal(SIGCHLD, SIG_DFL); protocol = parse_connect_url(url, &host, &port, &path); + if (flags & CONNECT_DIAG_URL) { + fprintf(stderr, "Diag: url=%s\n", url ? url : "NULL"); + fprintf(stderr, "Diag: protocol=%s\n", prot_name(protocol)); + fprintf(stderr, "Diag: host=%s", host ? host : "NULL"); + if (port) + fprintf(stderr, ":%s\n", port); + else + fprintf(stderr, "\n"); + fprintf(stderr, "Diag: path=%s\n", path ? path : "NULL"); + free(host); + free(port); + free(path); + return NULL; + } if (protocol == PROTO_GIT) { /* These underlying connection commands die() if they diff --git a/connect.h b/connect.h index 64fb7db..527d58a 100644 --- a/connect.h +++ b/connect.h @@ -2,6 +2,7 @@ #define CONNECT_H #define CONNECT_VERBOSE (1u << 0) +#define CONNECT_DIAG_URL (1u << 1) extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags); extern int finish_connect(struct child_process *conn); extern int git_connection_is_socket(struct child_process *conn); diff --git a/fetch-pack.h b/fetch-pack.h index 461cbf3..20ccc12 100644 --- a/fetch-pack.h +++ b/fetch-pack.h @@ -14,6 +14,7 @@ struct fetch_pack_args { use_thin_pack:1, fetch_all:1, stdin_refs:1, + diag_url:1, verbose:1, no_progress:1, include_tag:1, diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index d87ddf7..9136f2a 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -531,5 +531,62 @@ test_e
[PATCH] git p4: Use git diff-tree instead of format-patch
The output of git format-patch can vary with user preferences. In particular setting diff.noprefix will break the "git apply" that is done as part of "git p4 submit". Signed-off-by: Crestez Dan Leonard --- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index 31e71ff..fe988ce 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1308,7 +1308,7 @@ class P4Submit(Command, P4UserMap): else: die("unknown modifier %s for %s" % (modifier, path)) -diffcmd = "git format-patch -k --stdout \"%s^\"..\"%s\"" % (id, id) +diffcmd = "git diff-tree -p \"%s\"" % (id) patchcmd = diffcmd + " | git apply " tryPatchCmd = patchcmd + "--check -" applyPatchCmd = patchcmd + "--check --apply -"
Re:Answer back
I would like to discuss a very important crude oil project with you,kindly revert back to me if this is your valid email address for further information. Regards, Lee -- 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 v6 09/10] connect.c: Refactor url parsing
Make the function is_local() from tramsport.c public and use it in both transport.c and connect.c Use a protocol "local" for URLs for the local file system. --- connect.c| 58 ++-- connect.h| 1 + t/t5601-clone.sh | 10 +- transport.c | 8 4 files changed, 33 insertions(+), 44 deletions(-) diff --git a/connect.c b/connect.c index 3d174c8..95568ac 100644 --- a/connect.c +++ b/connect.c @@ -232,13 +232,23 @@ int server_supports(const char *feature) enum protocol { PROTO_LOCAL = 1, + PROTO_FILE, PROTO_SSH, PROTO_GIT }; +int is_local(const char *url) +{ + const char *colon = strchr(url, ':'); + const char *slash = strchr(url, '/'); + return !colon || (slash && slash < colon) || + has_dos_drive_prefix(url); +} + static const char *prot_name(enum protocol protocol) { switch (protocol) { case PROTO_LOCAL: + case PROTO_FILE: return "file"; case PROTO_SSH: return "ssh"; @@ -260,7 +270,7 @@ static enum protocol get_protocol(const char *name) if (!strcmp(name, "ssh+git")) return PROTO_SSH; if (!strcmp(name, "file")) - return PROTO_LOCAL; + return PROTO_FILE; die("I don't handle protocol '%s'", name); } @@ -563,9 +573,8 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, char *url; char *host, *path; char *end; - int separator; + int separator = '/'; enum protocol protocol = PROTO_LOCAL; - int free_path = 0; if (is_url(url_orig)) url = url_decode(url_orig); @@ -577,10 +586,12 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, *host = '\0'; protocol = get_protocol(url); host += 3; - separator = '/'; } else { host = url; - separator = ':'; + if (!is_local(url)) { + protocol = PROTO_SSH; + separator = ':'; + } } /* @@ -596,17 +607,12 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, } else end = host; - path = strchr(end, separator); - if (path && !has_dos_drive_prefix(end)) { - if (separator == ':') { - if (host != url || path < strchrnul(host, '/')) { - protocol = PROTO_SSH; - *path++ = '\0'; - } else /* '/' in the host part, assume local path */ - path = end; - } - } else + if (protocol == PROTO_LOCAL) + path = end; + else if (protocol == PROTO_FILE && has_dos_drive_prefix(end)) path = end; + else + path = strchr(end, separator); if (!path || !*path) die("No path specified. See 'man git-pull' for valid url syntax"); @@ -615,23 +621,21 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, * null-terminate hostname and point path to ~ for URL's like this: *ssh://host.xz/~user/repo */ - if (protocol != PROTO_LOCAL && separator == '/') { - char *ptr = path; + + end = path; /* Need to \0 terminate host here */ + if (separator == ':') + path++; /* path starts after ':' */ + if ((protocol == PROTO_GIT) || + (protocol == PROTO_SSH && separator == '/')) { if (path[1] == '~') path++; - else { - path = xstrdup(ptr); - free_path = 1; - } - - *ptr = '\0'; } + path = xstrdup(path); + *end = '\0'; + *ret_host = xstrdup(host); - if (free_path) - *ret_path = path; - else - *ret_path = xstrdup(path); + *ret_path = path; free(url); return protocol; } diff --git a/connect.h b/connect.h index 527d58a..ce657b3 100644 --- a/connect.h +++ b/connect.h @@ -9,5 +9,6 @@ extern int git_connection_is_socket(struct child_process *conn); extern int server_supports(const char *feature); extern int parse_feature_request(const char *features, const char *feature); extern const char *server_feature_value(const char *feature, int *len_ret); +int is_local(const char *url); #endif diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 57234c0..bd1bfd3 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -364,15 +364,7 @@ do done # Corner cases -# failing -for url in [foo]bar/baz:qux [foo/bar]:baz -do - test_expect_failure
Re: [PATCH] drop support for "experimental" loose objects
Jeff King wrote: > This latter behavior is much worse for two reasons. One, > git reports an allocation error when the real error is > corruption. And two, the program dies unconditionally, so > you cannot even run fsck (which would otherwise ignore the > broken object and keep going). BTW, I've also seen git cat-file --batch report wrong sizes for objects, sometimes without crashing. This is particularly problimatic because if the object size is wrong, it's very hard to detect the actual end of the object output in the batch mode stream. -- see shy jo signature.asc Description: Digital signature
Re: [PATCH] git p4: Use git diff-tree instead of format-patch
Crestez Dan Leonard writes: > The output of git format-patch can vary with user preferences. In > particular setting diff.noprefix will break the "git apply" that > is done as part of "git p4 submit". > > Signed-off-by: Crestez Dan Leonard > --- > git-p4.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/git-p4.py b/git-p4.py > index 31e71ff..fe988ce 100755 > --- a/git-p4.py > +++ b/git-p4.py > @@ -1308,7 +1308,7 @@ class P4Submit(Command, P4UserMap): > else: > die("unknown modifier %s for %s" % (modifier, path)) > > -diffcmd = "git format-patch -k --stdout \"%s^\"..\"%s\"" % (id, id) > +diffcmd = "git diff-tree -p \"%s\"" % (id) > patchcmd = diffcmd + " | git apply " > tryPatchCmd = patchcmd + "--check -" > applyPatchCmd = patchcmd + "--check --apply -" I do not do p4 myself, but from a cursory reading it looks like the right thing to do. Thanks. The output of "git shortlog --no-merges --since=1.year git-p4.py" tells me that Pete should be the person much more familiar with the code than myself, so I'll Cc him just in case... -- 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 v6 01/10] t5601: remove clear_ssh, refactor setup_ssh_wrapper
Commit 8d3d28f5 added test cases for URLs which should be ssh. Remove the function clear_ssh, use test_when_finished to clean up. Introduce the function setup_ssh_wrapper, which could be factored out together with expect_ssh. Tighten one test and use "foo:bar" instead of "./foo:bar", Helped-by: Jeff King Signed-off-by: Torsten Bögershausen --- Changes since last version: - updated t5601, thanks Peff - Split up the patch into 10 commits - Hannes suggested 2 patches - Add tests for git fetch-pack, which verifies the parsing - Added lots of test cases in t5500 via git fetch-pack --diag-url t/t5601-clone.sh | 40 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 1d1c875..83b21f5 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -280,25 +280,26 @@ test_expect_success 'clone checking out a tag' ' test_cmp fetch.expected fetch.actual ' -test_expect_success 'setup ssh wrapper' ' - write_script "$TRASH_DIRECTORY/ssh-wrapper" <<-\EOF && - echo >>"$TRASH_DIRECTORY/ssh-output" "ssh: $*" && - # throw away all but the last argument, which should be the - # command - while test $# -gt 1; do shift; done - eval "$1" - EOF - - GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" && - export GIT_SSH && - export TRASH_DIRECTORY -' - -clear_ssh () { - >"$TRASH_DIRECTORY/ssh-output" +setup_ssh_wrapper () { + test_expect_success 'setup ssh wrapper' ' + write_script "$TRASH_DIRECTORY/ssh-wrapper" <<-\EOF && + echo >>"$TRASH_DIRECTORY/ssh-output" "ssh: $*" && + # throw away all but the last argument, which should be the + # command + while test $# -gt 1; do shift; done + eval "$1" + EOF + GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" && + export GIT_SSH && + export TRASH_DIRECTORY && + >"$TRASH_DIRECTORY"/ssh-output + ' } expect_ssh () { + test_when_finished ' + (cd "$TRASH_DIRECTORY" && rm -f ssh-expect && >ssh-output) + ' && { case "$1" in none) @@ -310,21 +311,20 @@ expect_ssh () { (cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output) } +setup_ssh_wrapper + test_expect_success 'cloning myhost:src uses ssh' ' - clear_ssh && git clone myhost:src ssh-clone && expect_ssh myhost src ' test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' ' - clear_ssh && cp -R src "foo:bar" && - git clone "./foo:bar" foobar && + git clone "foo:bar" foobar && expect_ssh none ' test_expect_success 'bracketed hostnames are still ssh' ' - clear_ssh && git clone "[myhost:123]:src" ssh-bracket-clone && expect_ssh myhost:123 src ' -- 1.8.4.457.g424cb08 -- 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] glossary-content.txt: remove an obsolete paragrah
Duy Nguyen writes: > Current version does force that. > > $ git log -- ':/#aa' > fatal: Unimplemented pathspec magic '#' in ':/#aa' > > Not sure if there are tests for it though. I'll add an advice to do ":/:#aa". It is good that we already do the right thing, but actually I was not questioning the implementation. I was talking about the fact that the documentation is open to misinterpretation that contradicts the correct behaviour, as if it were buggy. -- 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