Re: [PATCH v2 11/13] remote-hg: force remote push
Jed Brown wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Thu, Apr 4, 2013 at 2:48 PM, Jed Brown j...@59a2.org wrote: ... We have to assume that every Git (remote-hg) User is dealing with Hg Team No, we don't. Really? If there is no Hg Team, why bother with an Hg upstream? Huh? the counterpart of every user wpuld be some users and not no user or no HG team, isn't it? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
Ramkumar Ramachandra wrote: After some discussion, I hope to be able to finalize a list of fields that will suffice for (nearly) everything. The task is actually much easier than this. All we have to do is finalize the list of fields that will mandatorily be written to the link object. As I might have indicated in my series, this is: upstream_url, checkout_rev, and ref_name. Really, the user only needs to supply a valid upstream_url: after a clone, everything else can be inferred (with the exception of a ref_name conflict; I don't like auto-mangling). Other fields are like .git/config fields. We can add new key/value pairs in the future, without worrying about migration. A problem arises only if we want to add a new mandatory field, change the default value of a key, or deprecate an existing key/value pair. -- 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 7/7] sha1_file: write ref_name to link object
Ramkumar Ramachandra wrote: Great. Now, we just have to write refs/modules/branch/* at commit-time. Actually, we have to update things in refs/modules/ everytime we update things in refs/heads/. In the case of a 'git branch -M' for example, refs/heads/oldname is rewritten to refs/heads/newname: similarly, refs/modules/oldname/ needs to be moved to refs/modules/newname/. There is one special case worth mentioning. Let's say I'm committing changes to link objects to a detached HEAD, b8bb3f. Then, I write refs/modules/b8bb3f/ at commit-time. A subsequent 'checkout -b' that updates refs/heads/newbranch will just have to move refs/modules/b8bb3f/ to refs/modules/newbranch/. The caveat is that I might commit to the detached HEAD and leave it hanging around until the next gc. So, gc will need to remove refs/modules/b8bb3f/ too, but that's not a pressing issue at the moment. -- 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 6/7] clone: introduce clone.submodulegitdir
Ramkumar Ramachandra wrote:
diff --git a/builtin/clone.c b/builtin/clone.c
index e0aaf13..1b798e6 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -658,11 +659,22 @@ static void write_refspec_config(const char*
src_ref_prefix,
strbuf_release(value);
}
+static int git_clone_config(const char *var, const char *value, void *cb)
+{
+ if (!strcmp(var, clone.submodulegitdir)) {
+ git_config_string(submodule_gitdir, var, value);
+ return 0;
+ }
+ return git_default_config(var, value, cb);
+}
submodule_gitdir can be a human-path, and we will need real_path() to
turn it into a concrete path. Why doesn't real_path() expand ~ yet?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] sha1_file, link: write link objects to the database
Ramkumar Ramachandra wrote:
diff --git a/link.c b/link.c
index bb20a51..349646d 100644
--- a/link.c
+++ b/link.c
@@ -20,8 +20,30 @@ struct link *lookup_link(const unsigned char *sha1)
int parse_link_buffer(struct link *item, void *buffer, unsigned long size)
{
+ char *bufptr = buffer;
+ char *tail = buffer + size;
+ char *eol;
+
if (item-object.parsed)
return 0;
item-object.parsed = 1;
+ while (bufptr tail) {
+ eol = strchr(bufptr, '\n');
+ *eol = '\0';
+ if (!prefixcmp(bufptr, upstream_url = ))
+ item-upstream_url = xstrdup(bufptr + 15);
+ else if (!prefixcmp(bufptr, checkout_rev = ))
+ item-checkout_rev = xstrdup(bufptr + 15);
+ else if (!prefixcmp(bufptr, ref_name = ))
+ item-ref_name = xstrdup(bufptr + 11);
+ else if (!prefixcmp(bufptr, floating = ))
+ item-floating = atoi(bufptr + 11);
+ else if (!prefixcmp(bufptr, statthrough = ))
+ item-statthrough = atoi(bufptr + 14);
+ else
+ return error(Parse error in link buffer);
+
+ bufptr = eol + 1;
+ }
return 0;
}
This needs to be replaced by a .git/config parser. However, I can't
use the parser from config.c as-it-is, because it expects a section
like [core] to be present. So, we have to refactor it to optionally
parse section-less configs.
--
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: Composing git repositories
Am 05.04.2013 07:27, schrieb Duy Nguyen: On Fri, Apr 5, 2013 at 3:53 PM, Junio C Hamano gits...@pobox.com wrote: A brief summary or outcome from these links in the comment would be nice. A summary of what to consider in Documentation/technical/ somewhere may be a very welcome addition. Thanks for volunteering ;-). No thanks :-) I did not really follow this thread to make such contribution. Still have some work to do with other topics. I'll do that (unless someone else steps up). I had these links in my todo list with the intent of adding them to my github page, but I agree they make more sense in Documentation/technical. Will see when I find a time slot to read through the whole threads to give them meaningful captions. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] diffcore-pickaxe: respect --no-textconv
Simon Ruderich si...@ruderich.org writes: --- a/t/t4209-log-pickaxe.sh +++ b/t/t4209-log-pickaxe.sh @@ -116,4 +116,18 @@ test_expect_success 'log -S -i (nomatch)' ' test_cmp expect actual ' +test_expect_success 'log -S --textconv (missing textconv tool)' ' + echo * diff=test .gitattributes + test_must_fail git -c diff.test.textconv=missing log -Sfoo + rm .gitattributes +' + +test_expect_success 'log -S --no-textconv (missing textconv tool)' ' + echo * diff=test .gitattributes + git -c diff.test.textconv=missing log -Sfoo --no-textconv actual + expect + test_cmp expect actual + rm .gitattributes +' While we're there, we could test -G --no-textconv too. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/12] pretty: share code between format_decoration and show_decorations
Junio C Hamano wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: diff --git a/log-tree.h b/log-tree.h index 9140f48..e6a2da5 100644 --- a/log-tree.h +++ b/log-tree.h @@ -13,6 +13,9 @@ int log_tree_diff_flush(struct rev_info *); int log_tree_commit(struct rev_info *, struct commit *); int log_tree_opt_parse(struct rev_info *, const char **, int); void show_log(struct rev_info *opt); +void format_decoration(struct strbuf *sb, + const struct commit *commit, + int use_color); I think you can fit these on a single line, especially if you drop the unused variable names (they help when there are more than one parameter of the same type to document the order of the arguments, but that does not apply here). That would help people who run grep on the header files without using CTAGS/ETAGS. Well, I think int use_color should be left with variable name, don't you think? -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] sha1_file, link: write link objects to the database
Ramkumar Ramachandra wrote: This needs to be replaced by a .git/config parser. However, I can't use the parser from config.c as-it-is, because it expects a section like [core] to be present. So, we have to refactor it to optionally parse section-less configs. Er, sorry about the thinko: I meant that edit-link should use the .git/config parser. This one is just fine. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/13] remote-hg: general updates
Antoine Pelisse wrote: * internally, the marks are using the hg sha1s instead of the hg rev ids. The latter are not necessarily invariant, and using the sha1s makes it much easier to recover from semi-broken states. I doubt this makes any difference (except for more wasted space). I think this is definitely wrong. If you happen to strip a changeset from the mercurial repository, and redo a completely different commit on top of it, the new commit will never be seen on git end (because it will have the same rev id and will thus be identified as identical from git point of view). That is true. I've written the code to use SHA-1 nodeids inststead andd this particular scenario works correctly, I just need to figure a way to discard the old ones. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/git-commit: reword the --amend explanation
On Thu, 2013-04-04 at 10:04 -0700, Junio C Hamano wrote: Carlos Martín Nieto c...@elego.de writes: On Wed, 2013-04-03 at 23:25 +0100, Philip Oakley wrote: + Replace the tip of the current branch with a fresh commit. [or updated commit, or new commit, or ...] Ack, we should lead with the goal, I'd go for the Replace the tip of the current branch with a new commit wording. We would want to be careful to make sure that the reader understands that the new commit is created by running this command (i.e. it is not like git branch -f $current_branch $new_commit), but other than that, sounds sensible. Perhaps like this? Documentation/git-commit.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index bc919ac..61266d8 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -190,8 +190,8 @@ OPTIONS without changing its commit message. --amend:: - Create a new commit and replace the tip of the current - branch. The recorded tree is prepared as usual (including + Replace the tip of the current branch by creating a new + commit. The recorded tree is prepared as usual (including the effect of the `-i` and `-o` options and explicit pathspec), and the message from the original commit is used as the starting point, instead of an empty message, when no Looks good, yeah. This should stop anybody thinking that they can replace the tip with an arbitrary commit. cmn -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 13/13] remote-hg: push to the appropriate branch
On Thu, Apr 4, 2013 at 10:50 AM, Junio C Hamano gits...@pobox.com wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
From: Dusty Phillips du...@linux.ca
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-hg | 4
1 file changed, 4 insertions(+)
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 56b3641..d82eb2d 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -625,6 +625,10 @@ def parse_commit(parser):
if merge_mark:
get_merge_files(repo, p1, p2, files)
+# Check if the ref is supposed to be a named branch
+if ref.startswith('refs/heads/branches/'):
+extra['branch'] = ref.rpartition('/')[2]
+
Is this meant to cut everything after refs/heads/branches/, or cut
at the last slash? I know rpartition does the latter, but I was
wondering if we see refs/heads/branches/foo/bar as its input here.
Good catch, it should be the former.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] count-objects: output KiB instead of kilobytes
On Thu, Apr 4, 2013 at 6:27 PM, Junio C Hamano gits...@pobox.com wrote: Mihai Capotă mi...@mihaic.ro writes: The git manual contains an explicit warning about the output of a porcelain command changing: The interface to Porcelain commands on the other hand are subject to change in order to improve the end user experience. Yeah, I know that, as I wrote it ;-) Aside from count-object being not exactly a Porcelain, the statement does not give us a blank check to make random changes as we see fit. There needs to be a clear improvement. I am just having a hard time weighing the benefit of using more accurate kibibytes over kilobytes and the possible downside of breaking other peoples' tools. Perhaps it would be alright if the change was accompanied by a warning in the Release Notes to say something like: If you have scripts that decide when to run git repack by parsing the output from git count-objects, this release may break them. Sorry about that. One of the scripts shipped by git-core itself also had to be adjusted. The command reports the total diskspace used to store loose objects in kibibytes, but it was labelled as kilobytes. The number now is shown with KiB, e.g. 6750 objects, 50928 KiB. You may want to consider updating such scripts to always call git gc --auto to let it decide when to repack for you. Also, I suspect that for the purpose of this exact output field, nobody cares the difference between kibibytes and kilobytes. Depending on the system, we add up either st.st_blocks or st.st_size and the result is not that exact as how much diskspace is consumed. I agree completely. I think the release notes warning is a good plan. Just in case you decide against it, I'm also sending a completely different patch to document the issue. Mihai -- 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] count-objects doc: document use of kibibytes
Document the use of kibibytes, not kilobytes, in the output of count-objects
and the reason for not correcting the output.
Also, make cvsimport comment and variable name reflect unit actually used.
Signed-off-by: Mihai Capotă mi...@mihaic.ro
---
Documentation/git-count-objects.txt |7 +++
git-cvsimport.perl |6 +++---
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-count-objects.txt
b/Documentation/git-count-objects.txt
index 23c80ce..d562dad 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -26,6 +26,13 @@ OPTIONS
and number of objects that can be removed by running
`git prune-packed`.
+
+BUGS
+
+Consumed space is actually expressed in kibibytes, not kilobytes as stated in
+the output. The output is kept as it is for compatibility reasons.
+
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 73d367c..6803f04 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -1126,12 +1126,12 @@ unless ($opt_P) {
}
# The heuristic of repacking every 1024 commits can leave a
-# lot of unpacked data. If there is more than 1MB worth of
+# lot of unpacked data. If there is more than 1MiB worth of
# not-packed objects, repack once more.
my $line = `git count-objects`;
if ($line =~ /^(\d+) objects, (\d+) kilobytes$/) {
- my ($n_objects, $kb) = ($1, $2);
- 1024 $kb
+ my ($n_objects, $kib) = ($1, $2);
+ 1024 $kib
and system(qw(git repack -a -d));
}
--
1.7.9.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
git diff --quiet on dirty tree sometimes erroneously exits with status 0
I'm seeing a strange problem where git diff --quiet sometimes returns an
exit code of zero even though the tree is dirty and other invocations of
git diff --quiet in the same directory return an exit code of 1. I'm
using git v1.8.2 from Debian unstable but I've also seen the problem when
running v1.8.2 built from source and using older versions from Debian. The
repository in question is a submodule.
Unfortunately this bad behaviour only happens deep inside a variable
expansion under OpenEmbedded's bitbake so investigation is not
straightforward. It's possible that when the bad behaviour occurs other
git diff and similar informational commands are being run on the same
repository in parallel. I have been unable to provoke the problem by more
conventional means. :( I can reproduce it on a separate repository on the
same machine though.
Using git diff --exit-code --no-ext-diff instead seems to avoid the
problem but unfortunately so does adding a sleep before calling git diff
--quiet so that result isn't necessarily significant.
I have been able to strace the problematic case. Diffing the straces and
ignoring unrelated differences such as addresses yields (+ =
diff returns 0, - = diff returns 1):
[...lots of omitted stuff...]
open(/home/mac/src/oe/sources/linux/.gitmodules, O_RDONLY) = -1 ENOENT
(No such file or directory)
access(/etc/gitconfig, R_OK) = -1 ENOENT (No such file or
directory)
access(/home/mac/.config/git/config, R_OK) = -1 ENOENT (No such file or
directory)
access(/home/mac/.gitconfig, R_OK)= 0
open(/home/mac/.gitconfig, O_RDONLY) = 20
fstat(20, {st_mode=S_IFREG|0644, st_size=424, ...}) = 0
0x7f22bfa18000
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7f79414f1000
read(20, [color]\n\tui = true\n[core]\n#\tpage..., 4096) = 424
read(20, , 4096) = 0
close(20) = 0
munmap(0x7f79414f1000, 4096)= 0
access(/home/mac/src/oe/.git/modules/sources/linux/config, R_OK) = 0
open(/home/mac/src/oe/.git/modules/sources/linux/config, O_RDONLY) = 20
fstat(20, {st_mode=S_IFREG|0644, st_size=1467, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7f79414f1000
read(20, [core]\n\trepositoryformatversion ..., 4096) = 1467
read(20, , 4096) = 0
close(20) = 0
munmap(0x7f79414f1000, 4096)= 0
chdir(/home/mac/src/oe/sources/linux) = 0
lstat(.gitignore, {st_mode=S_IFREG|0644, st_size=955, ...}) = 0
At this point the diff that erroneously returned zero went off and did:
+lstat(.gitignore, {st_mode=S_IFREG|0644, st_size=955, ...}) = 0
+open(/home/mac/src/oe/.git/modules/sources/linux/objects/pack,
O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 20
+getdents(20, /* 4 entries */, 32768)= 192
+access(/home/mac/src/oe/.git/modules/sources/linux/objects/pack/pack-2711d98bc06e8caa9c8d8e10ee40e645e6d05f18.keep,
F_OK) = -1 ENOENT (No such file or directory)
+stat(/home/mac/src/oe/.git/modules/sources/linux/objects/pack/pack-2711d98bc06e8caa9c8d8e10ee40e645e6d05f18.pack,
{st_mode=S_IFREG|0444, st_size=148768254, ...}) = 0
+getdents(20, /* 0 entries */, 32768)= 0
+close(20) = 0
..and spent a while re-reading config files before continuing to recurse
over all the files but whilst the diff that provides the correct result
merely lstats each file this one reads them too:
lstat(COPYING, {st_mode=S_IFREG|0644, st_size=18693, ...}) = 0
+open(COPYING, O_RDONLY) = 21
+read(21, \n NOTE! This copyright does *n..., 18693) = 18693
+close(21) = 0
When it comes across the repository file that has been changed the
behaviour of the invocation that provides the correct result is rather
different:
lstat(drivers/net/bcmgenet/bcmgenet.c, {st_mode=S_IFREG|0644,
st_size=114290, ...}) = 0
-lstat(drivers/net/bcmgenet/bcmgenet.c, {st_mode=S_IFREG|0644,
st_size=114290, ...}) = 0
-open(/home/mac/src/oe/.git/modules/sources/linux/objects/pack,
O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 20
-getdents(20, /* 4 entries */, 32768)= 192
-access(/home/mac/src/oe/.git/modules/sources/linux/objects/pack/pack-2711d98bc06e8caa9c8d8e10ee40e645e6d05f18.keep,
F_OK) = -1 ENOENT (No such file or directory)
-stat(/home/mac/src/oe/.git/modules/sources/linux/objects/pack/pack-2711d98bc06e8caa9c8d8e10ee40e645e6d05f18.pack,
{st_mode=S_IFREG|0444, st_size=148768254, ...}) = 0
-getdents(20, /* 0 entries */, 32768)= 0
-close(20) = 0
-open(/home/mac/src/oe/.git/modules/sources/linux/objects/info/alternates,
O_RDONLY|O_NOATIME) = -1 ENOENT (No such file or directory)
-open(/home/mac/src/oe/.git/modules/sources/linux/objects/pack/pack-2711d98bc06e8caa9c8d8e10ee40e645e6d05f18.idx,
O_RDONLY|O_NOATIME) = 20
-fstat(20, {st_mode=S_IFREG|0444, st_size=1945504, ...}) = 0
-mmap(NULL, 1945504, PROT_READ, MAP_PRIVATE, 20, 0) = 0x7f22bea11000
[PATCH] diff: allow unstuck arguments with --diff-algorithm
The argument to --diff-algorithm is mandatory, so there is no reason to
require the argument to be stuck to the option with '='. Change this
for consistency with other Git commands.
Note that this doesi not change the handling of diff-algorithm in
merge-recursive.c since the primary interface to that is via the -X
option to 'git merge' where the unstuck form does not make sense.
Signed-off-by: John Keeping j...@keeping.me.uk
---
diff.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index db952a5..e0152f8 100644
--- a/diff.c
+++ b/diff.c
@@ -3596,8 +3596,8 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
options-xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
else if (!strcmp(arg, --histogram))
options-xdl_opts = DIFF_WITH_ALG(options, HISTOGRAM_DIFF);
- else if (!prefixcmp(arg, --diff-algorithm=)) {
- long value = parse_algorithm_value(arg+17);
+ else if ((argcount = parse_long_opt(diff-algorithm, av, optarg))) {
+ long value = parse_algorithm_value(optarg);
if (value 0)
return error(option diff-algorithm accepts \myers\,
\minimal\, \patience\ and
\histogram\);
@@ -3605,6 +3605,7 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
DIFF_XDL_CLR(options, NEED_MINIMAL);
options-xdl_opts = ~XDF_DIFF_ALGORITHM_MASK;
options-xdl_opts |= value;
+ return argcount;
}
/* flags options */
--
1.8.2.540.gf023cfe
--
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/PATH 0/4] remoe-hg: switch to SHA-1 ids
Hi, As some people suggested this is necessary for some use-cases, because revision id numbers can change and point to different revisions. Seems to work fine, but I wouldn't merge it just yet. Felipe Contreras (4): remote-hg: shuffle some code remote-hg: improve node traversing remote-hg: add version checks to the marks remote-hg: switch from revisions to SHA-1 noteids contrib/remote-helpers/git-remote-hg | 77 ++-- 1 file changed, 48 insertions(+), 29 deletions(-) -- 1.8.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
[RFC/PATH 1/4] remote-hg: shuffle some code
In preparation to shift to SHA-1's.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-hg | 27 +++
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index d82eb2d..a8591a2 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -63,6 +63,9 @@ def hgmode(mode):
def hghex(node):
return hg.node.hex(node)
+def hgbin(node):
+return hg.node.bin(node)
+
def get_config(config):
cmd = ['git', 'config', '--get', config]
process = subprocess.Popen(cmd, stdout=subprocess.PIPE)
@@ -207,7 +210,7 @@ def get_filechanges(repo, ctx, parent):
removed = set()
cur = ctx.manifest()
-prev = repo[parent].manifest().copy()
+prev = parent.manifest().copy()
for fn in cur:
if fn in prev:
@@ -326,7 +329,7 @@ def export_ref(repo, name, kind, head):
else:
committer = author
-parents = [p for p in repo.changelog.parentrevs(rev) if p = 0]
+parents = [repo[p] for p in repo.changelog.parentrevs(rev) if p = 0]
if len(parents) == 0:
modified = c.manifest().keys()
@@ -372,9 +375,9 @@ def export_ref(repo, name, kind, head):
print desc
if len(parents) 0:
-print from :%s % (rev_to_mark(parents[0]))
+print from :%s % (rev_to_mark(parents[0].rev()))
if len(parents) 1:
-print merge :%s % (rev_to_mark(parents[1]))
+print merge :%s % (rev_to_mark(parents[1].rev()))
for f in modified:
export_file(c.filectx(f))
@@ -389,10 +392,10 @@ def export_ref(repo, name, kind, head):
# make sure the ref is updated
print reset %s/%s % (prefix, ename)
-print from :%u % rev_to_mark(rev)
+print from :%u % rev_to_mark(head.rev())
print
-marks.set_tip(ename, rev)
+marks.set_tip(ename, head.rev())
def export_tag(repo, tag):
export_ref(repo, tag, 'tags', repo[tag])
@@ -651,7 +654,7 @@ def parse_commit(parser):
tmp = encoding.encoding
encoding.encoding = 'utf-8'
-node = repo.commitctx(ctx)
+node = hghex(repo.commitctx(ctx))
encoding.encoding = tmp
@@ -675,7 +678,7 @@ def parse_reset(parser):
parser.next()
node = parser.repo.changelog.node(mark_to_rev(from_mark))
-parsed_refs[ref] = node
+parsed_refs[ref] = hghex(node)
def parse_tag(parser):
name = parser[1]
@@ -720,10 +723,10 @@ def do_export(parser):
elif ref.startswith('refs/tags/'):
tag = ref[len('refs/tags/'):]
if mode == 'git':
-msg = 'Added tag %s for changeset %s' % (tag, hghex(node[:6]));
-parser.repo.tag([tag], node, msg, False, None, {})
+msg = 'Added tag %s for changeset %s' % (tag, node[:6]);
+parser.repo.tag([tag], hgbin(node), msg, False, None, {})
else:
-parser.repo.tag([tag], node, None, True, None, {})
+parser.repo.tag([tag], hgbin(node), None, True, None, {})
print ok %s % ref
else:
# transport-helper/fast-export bugs
@@ -735,7 +738,7 @@ def do_export(parser):
# handle bookmarks
for bmark, node in p_bmarks:
ref = 'refs/heads' + bmark
-new = hghex(node)
+new = node
if bmark in bmarks:
old = bmarks[bmark].hex()
--
1.8.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
[RFC/PATH 2/4] remote-hg: improve node traversing
We won't be able to count the unmarked commits, but we are not going to be able to do that anyway when we switch to SHA-1 ids. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index a8591a2..02fda2d 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -314,12 +314,16 @@ def export_ref(repo, name, kind, head): revs = xrange(tip, head.rev() + 1) count = 0 -revs = [rev for rev in revs if not marks.is_marked(rev)] - for rev in revs: c = repo[rev] -(manifest, user, (time, tz), files, desc, extra) = repo.changelog.read(c.node()) +node = c.node() + +if marks.is_marked(c.hex()): +count += 1 +continue + +(manifest, user, (time, tz), files, desc, extra) = repo.changelog.read(node) rev_branch = extra['branch'] author = %s %d %s % (fixup_user(user), time, gittz(tz)) -- 1.8.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
[RFC/PATH 3/4] remote-hg: add version checks to the marks
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-hg | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 02fda2d..9e124e1 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -43,6 +43,8 @@ AUTHOR_RE = re.compile('^([^]+?)? ?([^]*)$')
AUTHOR_HG_RE = re.compile('^(.*?) ?(.*?)(?:(.+)?)?$')
RAW_AUTHOR_RE = re.compile('^(\w+) (?:(.+)? )?(.*) (\d+) ([+-]\d+)')
+VERSION = 1
+
def die(msg, *args):
sys.stderr.write('ERROR: %s\n' % (msg % args))
sys.exit(1)
@@ -76,12 +78,19 @@ class Marks:
def __init__(self, path):
self.path = path
+self.clear()
+self.load()
+
+if self.version VERSION:
+self.clear()
+self.version = VERSION
+
+def clear(self):
self.tips = {}
self.marks = {}
self.rev_marks = {}
self.last_mark = 0
-
-self.load()
+self.version = 0
def load(self):
if not os.path.exists(self.path):
@@ -92,12 +101,13 @@ class Marks:
self.tips = tmp['tips']
self.marks = tmp['marks']
self.last_mark = tmp['last-mark']
+self.version = tmp.get('version', 1)
for rev, mark in self.marks.iteritems():
self.rev_marks[mark] = int(rev)
def dict(self):
-return { 'tips': self.tips, 'marks': self.marks, 'last-mark' :
self.last_mark }
+return { 'tips': self.tips, 'marks': self.marks, 'last-mark' :
self.last_mark, 'version' : self.version }
def store(self):
json.dump(self.dict(), open(self.path, 'w'))
--
1.8.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
[RFC/PATH 4/4] remote-hg: switch from revisions to SHA-1 noteids
Otherwise we won't know if revisions are replaced.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-hg | 40 +++-
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 9e124e1..162dabc 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -43,7 +43,7 @@ AUTHOR_RE = re.compile('^([^]+?)? ?([^]*)$')
AUTHOR_HG_RE = re.compile('^(.*?) ?(.*?)(?:(.+)?)?$')
RAW_AUTHOR_RE = re.compile('^(\w+) (?:(.+)? )?(.*) (\d+) ([+-]\d+)')
-VERSION = 1
+VERSION = 2
def die(msg, *args):
sys.stderr.write('ERROR: %s\n' % (msg % args))
@@ -104,7 +104,7 @@ class Marks:
self.version = tmp.get('version', 1)
for rev, mark in self.marks.iteritems():
-self.rev_marks[mark] = int(rev)
+self.rev_marks[mark] = rev
def dict(self):
return { 'tips': self.tips, 'marks': self.marks, 'last-mark' :
self.last_mark, 'version' : self.version }
@@ -116,23 +116,23 @@ class Marks:
return str(self.dict())
def from_rev(self, rev):
-return self.marks[str(rev)]
+return self.marks[rev]
def to_rev(self, mark):
return self.rev_marks[mark]
def get_mark(self, rev):
self.last_mark += 1
-self.marks[str(rev)] = self.last_mark
+self.marks[rev] = self.last_mark
return self.last_mark
def new_mark(self, rev, mark):
-self.marks[str(rev)] = mark
+self.marks[rev] = mark
self.rev_marks[mark] = rev
self.last_mark = mark
def is_marked(self, rev):
-return self.marks.has_key(str(rev))
+return self.marks.has_key(rev)
def get_tip(self, branch):
return self.tips.get(branch, 0)
@@ -305,7 +305,7 @@ def get_repo(url, alias):
def rev_to_mark(rev):
global marks
-return marks.from_rev(rev)
+return marks.from_rev(rev.hex())
def mark_to_rev(mark):
global marks
@@ -316,6 +316,10 @@ def export_ref(repo, name, kind, head):
ename = '%s/%s' % (kind, name)
tip = marks.get_tip(ename)
+if tip in repo:
+tip = repo[tip].rev()
+else:
+tip = 0
# mercurial takes too much time checking this
if tip and tip == head.rev():
@@ -382,16 +386,16 @@ def export_ref(repo, name, kind, head):
print 'reset %s/%s' % (prefix, ename)
print commit %s/%s % (prefix, ename)
-print mark :%d % (marks.get_mark(rev))
+print mark :%d % (marks.get_mark(c.hex()))
print author %s % (author)
print committer %s % (committer)
print data %d % (len(desc))
print desc
if len(parents) 0:
-print from :%s % (rev_to_mark(parents[0].rev()))
+print from :%s % (rev_to_mark(parents[0]))
if len(parents) 1:
-print merge :%s % (rev_to_mark(parents[1].rev()))
+print merge :%s % (rev_to_mark(parents[1]))
for f in modified:
export_file(c.filectx(f))
@@ -406,10 +410,10 @@ def export_ref(repo, name, kind, head):
# make sure the ref is updated
print reset %s/%s % (prefix, ename)
-print from :%u % rev_to_mark(head.rev())
+print from :%u % rev_to_mark(head)
print
-marks.set_tip(ename, head.rev())
+marks.set_tip(ename, head.hex())
def export_tag(repo, tag):
export_ref(repo, tag, 'tags', repo[tag])
@@ -626,12 +630,12 @@ def parse_commit(parser):
extra['committer'] = %s %u %u % committer
if from_mark:
-p1 = repo.changelog.node(mark_to_rev(from_mark))
+p1 = mark_to_rev(from_mark)
else:
p1 = '\0' * 20
if merge_mark:
-p2 = repo.changelog.node(mark_to_rev(merge_mark))
+p2 = mark_to_rev(merge_mark)
else:
p2 = '\0' * 20
@@ -672,10 +676,8 @@ def parse_commit(parser):
encoding.encoding = tmp
-rev = repo[node].rev()
-
parsed_refs[ref] = node
-marks.new_mark(rev, commit_mark)
+marks.new_mark(node, commit_mark)
def parse_reset(parser):
global parsed_refs
@@ -691,8 +693,8 @@ def parse_reset(parser):
from_mark = parser.get_mark()
parser.next()
-node = parser.repo.changelog.node(mark_to_rev(from_mark))
-parsed_refs[ref] = hghex(node)
+rev = mark_to_rev(from_mark)
+parsed_refs[ref] = rev
def parse_tag(parser):
name = parser[1]
--
1.8.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
[PATCH] git-merge(1): document diff-algorithm option to merge-recursive
Commit 07924d4 (diff: Introduce --diff-algorithm command line option 2013-01-16) added diff-algorithm as a parameter to the recursive merge strategy but did not document it. Do so. Signed-off-by: John Keeping j...@keeping.me.uk --- Documentation/merge-strategies.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 66db802..49a9a7d 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -48,6 +48,12 @@ patience;; this when the branches to be merged have diverged wildly. See also linkgit:git-diff[1] `--patience`. +diff-algorithm=[patience|minimal|histogram|myers];; + Tells 'merge-recursive' to use a different diff algorithm, which + can help avoid mismerges that occur due to unimportant matching + lines (such as braces from distinct functions). See also + linkgit:git-diff[1] `--diff-algorithm`. + ignore-space-change;; ignore-all-space;; ignore-space-at-eol;; -- 1.8.2.540.gf023cfe -- 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] diff: allow unstuck arguments with --diff-algorithm
John Keeping j...@keeping.me.uk writes:
The argument to --diff-algorithm is mandatory, so there is no reason to
require the argument to be stuck to the option with '='. Change this
for consistency with other Git commands.
Note that this doesi not change the handling of diff-algorithm in
^
strayi ;-)
merge-recursive.c since the primary interface to that is via the -X
option to 'git merge' where the unstuck form does not make sense.
Signed-off-by: John Keeping j...@keeping.me.uk
---
diff.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index db952a5..e0152f8 100644
--- a/diff.c
+++ b/diff.c
@@ -3596,8 +3596,8 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
options-xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
else if (!strcmp(arg, --histogram))
options-xdl_opts = DIFF_WITH_ALG(options, HISTOGRAM_DIFF);
- else if (!prefixcmp(arg, --diff-algorithm=)) {
- long value = parse_algorithm_value(arg+17);
+ else if ((argcount = parse_long_opt(diff-algorithm, av, optarg))) {
+ long value = parse_algorithm_value(optarg);
if (value 0)
return error(option diff-algorithm accepts \myers\,
\minimal\, \patience\ and
\histogram\);
@@ -3605,6 +3605,7 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
DIFF_XDL_CLR(options, NEED_MINIMAL);
options-xdl_opts = ~XDF_DIFF_ALGORITHM_MASK;
options-xdl_opts |= value;
+ return argcount;
}
/* flags options */
--
Thomas Rast
trast@{inf,student}.ethz.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: [PATCH 00/13] remote-hg: general updates
On Thu, Apr 4, 2013 at 10:14 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Tue, Apr 2, 2013 at 4:23 PM, Max Horn m...@quendi.de wrote: * internally, the marks are using the hg sha1s instead of the hg rev ids. The latter are not necessarily invariant, and using the sha1s makes it much easier to recover from semi-broken states. I will investigate the pros and cons of this, but either way it's not something people are going to immediately need (I doubt the semi-broken states will happen again). And another one bites the dust: https://github.com/felipec/git/commits/fc/remote/hg-noteids -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/13] remote-hg: force remote push
Joachim Schmitz j...@schmitz-digital.de writes: Jed Brown wrote: Really? If there is no Hg Team, why bother with an Hg upstream? Huh? the counterpart of every user wpuld be some users and not no user or no HG team, isn't it? I'm not sure what you're getting at here, but the whole premise of a two-way git-remote-X is that the users of git-remote-X have less influence in the project than the users of X have in the project. Usually this means that the project workflow is whatever the X users find comfortable rather than whatever git-remote-X users prefer. If you are the sole publisher to a remote repository, sending pull requests to upstream, and if they are comfortable with pulling bookmarks (much more likely if they use a pull-request model rather than a shared repo), then force-pushing by default is more reasonable. An imperfect analogy is Git's push.default=simple, which is more friendly to beginners and to those sharing a remote. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] diff: allow unstuck arguments with --diff-algorithm
The argument to --diff-algorithm is mandatory, so there is no reason to
require the argument to be stuck to the option with '='. Change this
for consistency with other Git commands.
Note that this does not change the handling of diff-algorithm in
merge-recursive.c since the primary interface to that is via the -X
option to 'git merge' where the unstuck form does not make sense.
Signed-off-by: John Keeping j...@keeping.me.uk
---
On Fri, Apr 05, 2013 at 01:43:16PM +0200, Thomas Rast wrote:
John Keeping j...@keeping.me.uk writes:
The argument to --diff-algorithm is mandatory, so there is no reason to
require the argument to be stuck to the option with '='. Change this
for consistency with other Git commands.
Note that this doesi not change the handling of diff-algorithm in
^
strayi ;-)
Thanks.
diff.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index db952a5..e0152f8 100644
--- a/diff.c
+++ b/diff.c
@@ -3596,8 +3596,8 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
options-xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
else if (!strcmp(arg, --histogram))
options-xdl_opts = DIFF_WITH_ALG(options, HISTOGRAM_DIFF);
- else if (!prefixcmp(arg, --diff-algorithm=)) {
- long value = parse_algorithm_value(arg+17);
+ else if ((argcount = parse_long_opt(diff-algorithm, av, optarg))) {
+ long value = parse_algorithm_value(optarg);
if (value 0)
return error(option diff-algorithm accepts \myers\,
\minimal\, \patience\ and
\histogram\);
@@ -3605,6 +3605,7 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
DIFF_XDL_CLR(options, NEED_MINIMAL);
options-xdl_opts = ~XDF_DIFF_ALGORITHM_MASK;
options-xdl_opts |= value;
+ return argcount;
}
/* flags options */
--
1.8.2.540.gf023cfe
--
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: Collective wisdom about repos on NFS accessed by concurrent clients (== corruption!?)
Hi Basically, I'm at a place where I'm considering giving up getting this to work reliably. In general, my setup work really fine, except for the itty-bitty detail that when I put pressure on things I tend to get into various kinds of trouble with the central repo being corrupted. Can anyone authoritatively state anything either way? TIA, ken1 On 2013-03-28 11:22, Kenneth Ölwing wrote: Hi, I'm hoping to hear some wisdom on the subject so I can decide if I'm chasing a pipe dream or if it should be expected to work and I just need to work out the kinks. Finding things like this makes it sound possible: http://permalink.gmane.org/gmane.comp.version-control.git/122670 but then again, in threads like this: http://kerneltrap.org/mailarchive/git/2010/11/14/44799 opinions are that it's just not reliable to trust. My experience so far is that I eventually get repo corruption when I stress it with concurrent read/write access from multiple hosts (beyond any sort of likely levels, but still). Maybe I'm doing something wrong, missing a configuration setting somewhere, put an unfair stress on the system, there's a bona fide bug - or, given the inherent difficulty in achieving perfect coherency between machines on what's visible on the mount, it's just impossible (?) to truly get it working under all situations. My eventual usecase is to set up a bunch of (gitolite) hosts that all are effectively identical and all seeing the same storage and clients can then be directed to any of them to get served. For the purpose of this query I assume it can be boiled down to be the same as n users working on their own workstations and accessing a central repo on an NFS share they all have mounted, using regular file paths. Correct? I have a number of load-generating test scripts (that from humble beginnings have grown to beasts), but basically, they fork a number of code pieces that proceed to hammer the repo with concurrent reading and writing. If necessary, the scripts can be started on different hosts, too. It's all about the central repo so clients will retry in various modes whenever they get an error, up to re-cloning and starting over. All in all, they can yield quite a load. On a local filesystem everything seems to be holding up fine which is expected. In the scenario with concurrency on top of shared NFS storage however, the scripts eventually fails with various problems (when the timing finally finds a hole, I guess) - possible to clone but checkouts fails, errors about refs pointing wrong and errors where the original repo is actually corrupted and can't be cloned from. Depending on test strategy, I've also had everything going fine (ops looks fine in my logs), but afterwards the repo has lost a branch or two. I've experimented with various NFS settings (e.g. turning off the attribute cache), but haven't reached a conclusion. I do suspect that it just is a fact of life with a remote filesystem to have coherency problems with high concurrency like this but I'd be happily proven wrong - I'm not an expert in either NFS or git. So, any opionions either way would be valuable, e.g. 'it should work' or 'it can never work 100%' is fine, as well as any suggestions. Obviously, the concurrency needed to make it probable to hit this seems so unlikely that maybe I just shouldn't worry... I'd be happy to share scripts and whatever if someone would like to try it out themselves. Thanks for your time, ken1 -- 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 -- 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: trouble on windows network share
[strace attachment has been removed, email being resent]
It looks like there is a race condition going on, especially since the location
and message changes.
Could it be the file creation, file read, apply file security is happening when
it should be file create, apply security, file read?
Looking at trace line: 1088001 (full trace attached)
1181 1087049 [main] git 9320 symlink_info::check: 0x0 = NtCreateFile
(\??\UNC\server\share\dir1\dir2\test\.git\objects\68)
84 1087133 [main] git 9320 symlink_info::check: not a symlink
733 1087866 [main] git 9320 symlink_info::check: 0 =
symlink.check(\\server\share\dir1\dir2\test\.git\objects\68, 0x288280) (0x4022)
62 1087928 [main] git 9320 path_conv::check:
this-path(\\server\share\dir1\dir2\test\.git\objects\68\38761d549cf76033d2e9faf5954e62839eb25d),
has_acls(1)
31 1087959 [main] git 9320 build_fh_pc: fh 0x61274CA0, dev 0xC3
21 1087980 [main] git 9320 __set_errno: int fhandler_base::fhaccess(int,
bool):367 setting errno 2
21 1088001 [main] git 9320 fhandler_base::fhaccess: returning -1
19 1088020 [main] git 9320 access: returning -1
164 1088184 [main] git 9320 fhandler_pty_slave::write: pty0, write(50A474, 7)
18 1088202 [main] git 9320 fhandler_pty_slave::write: (657): pty
output_mutex (0xC0): waiting -1 ms
20 1088222 [main] git 9320 fhandler_pty_slave::write: (657): pty
output_mutex: acquired
error:
21 1088243 [main] git 9320 fhandler_pty_slave::write: (672): pty
output_mutex(0xC0) released
18 1088261 [main] git 9320 write: 7 = write(2, 0x50A474, 7)
36 1088297 [main] git 9320 fhandler_pty_slave::write: pty0, write(288680,
102)
18 1088315 [main] git 9320 fhandler_pty_slave::write: (657): pty
output_mutex (0xC0): waiting -1 ms
18 1088333 [main] git 9320 fhandler_pty_slave::write: (657): pty
output_mutex: acquired
Trying to write ref refs/heads/master with nonexistent object
6838761d549cf76033d2e9faf5954e62839eb25d
-Original Message-
From: Jeff King
Sent: Thursday, April 04, 2013 11:51 AM
On Thu, Apr 04, 2013 at 03:01:36PM +, Pyeron, Jason J CTR (US)
wrote:
I am having trouble when the .git folder is on a network share, given
the below where should I start on my debugging?
[...]
jason.pyeron@localhost //server/share/dir/subdir/test
$ git add test.txt
jason.pyeron@localhost //server/share/dir/subdir/test
$ git commit -m test
error: unable to find 8b7323820a21ebd1360e27262b3c61283c266c23
fatal: 8b7323820a21ebd1360e27262b3c61283c266c23 is not a valid object
Hmm. That message probably comes from:
static int sha1_loose_object_info(const unsigned char *sha1, unsigned
long *sizep)
{
[...]
map = map_sha1_file(sha1, mapsize);
if (!map)
return error(unable to find %s, sha1_to_hex(sha1));
So we have found the object and know that it is loose, but then mmap-
ing
it fails. My guess is that your system does not support mmap across
network shares (whether this is an OS issue or a cygwin limitation, I
don't know). You could confirm it by running your git commit under
strace, which I expect would show mmap returning -ENODEV or similar.
You can work around it by compiling git with NO_MMAP=1.
You might also try msysgit rather than cygwin, which seems to have its
own win32 mmap compatibility layer.
Finally, I suspect we could include our emulate-mmap-with-pread
compatibility wrapper all the time, and drop back to it automatically
at
run-time when we see ENODEV or a similar error return from mmap.
-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
smime.p7s
Description: S/MIME cryptographic signature
Re: [RFC/PATH 3/4] remote-hg: add version checks to the marks
Felipe Contreras felipe.contre...@gmail.com writes: @@ -76,12 +78,19 @@ class Marks: def __init__(self, path): self.path = path +self.clear() +self.load() + +if self.version VERSION: +self.clear() It's friendlier to just upgrade the marks in-place. This takes less than one second to run on repositories where full re-import would take half an hour: def upgrade_marks(self, hgrepo): if self.marks_version == 1: # Convert from integer reversions to hgsha1 warn(Upgrading marks-hg from hg sequence number to SHA1) self.marks_to_revisions = dict( (mark, hghex(hgrepo.changelog.node(int(rev for mark, rev in self.marks_to_revisions.iteritems()) self.revisions_to_marks = dict( (hghex(hgrepo.changelog.node(int(rev))), mark) for rev, mark in self.revisions_to_marks.iteritems()) self.marks_version = 2 warn(Upgrade complete) https://github.com/buchuki/gitifyhg/commit/23a6709efd14f3e058e3a846624b7677d1e8b497#L0R195 -- 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 v3 3/3] diffcore-pickaxe: respect --no-textconv
git log -S doesn't respect --no-textconv:
$ echo '*.txt diff=wrong' .gitattributes
$ git -c diff.wrong.textconv='xxx' log --no-textconv -Sfoo
error: cannot run xxx: No such file or directory
fatal: unable to read files to diff
Reported-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Simon Ruderich si...@ruderich.org
---
On Fri, Apr 05, 2013 at 09:40:21AM +0200, Matthieu Moy wrote:
While we're there, we could test -G --no-textconv too.
Hello Matthieu,
Good idea, I've added it.
Regards
Simon
diffcore-pickaxe.c | 12
t/t4209-log-pickaxe.sh | 28
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 3124f49..26ddf00 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -86,8 +86,10 @@ static int diff_grep(struct diff_filepair *p, struct
diff_options *o,
if (diff_unmodified_pair(p))
return 0;
- textconv_one = get_textconv(p-one);
- textconv_two = get_textconv(p-two);
+ if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
+ textconv_one = get_textconv(p-one);
+ textconv_two = get_textconv(p-two);
+ }
mf1.size = fill_textconv(textconv_one, p-one, mf1.ptr);
mf2.size = fill_textconv(textconv_two, p-two, mf2.ptr);
@@ -201,8 +203,10 @@ static int has_changes(struct diff_filepair *p, struct
diff_options *o,
if (!o-pickaxe[0])
return 0;
- textconv_one = get_textconv(p-one);
- textconv_two = get_textconv(p-two);
+ if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
+ textconv_one = get_textconv(p-one);
+ textconv_two = get_textconv(p-two);
+ }
/*
* If we have an unmodified pair, we know that the count will be the
diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
index eed7273..38fb80f 100755
--- a/t/t4209-log-pickaxe.sh
+++ b/t/t4209-log-pickaxe.sh
@@ -80,6 +80,20 @@ test_expect_success 'log -G -i (match)' '
test_cmp expect actual
'
+test_expect_success 'log -G --textconv (missing textconv tool)' '
+ echo * diff=test .gitattributes
+ test_must_fail git -c diff.test.textconv=missing log -Gfoo
+ rm .gitattributes
+'
+
+test_expect_success 'log -G --no-textconv (missing textconv tool)' '
+ echo * diff=test .gitattributes
+ git -c diff.test.textconv=missing log -Gfoo --no-textconv actual
+ expect
+ test_cmp expect actual
+ rm .gitattributes
+'
+
test_expect_success 'log -S (nomatch)' '
git log -Spicked --format=%H actual
expect
@@ -116,4 +130,18 @@ test_expect_success 'log -S -i (nomatch)' '
test_cmp expect actual
'
+test_expect_success 'log -S --textconv (missing textconv tool)' '
+ echo * diff=test .gitattributes
+ test_must_fail git -c diff.test.textconv=missing log -Sfoo
+ rm .gitattributes
+'
+
+test_expect_success 'log -S --no-textconv (missing textconv tool)' '
+ echo * diff=test .gitattributes
+ git -c diff.test.textconv=missing log -Sfoo --no-textconv actual
+ expect
+ test_cmp expect actual
+ rm .gitattributes
+'
+
test_done
--
1.8.2
--
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] diffcore-pickaxe: remove unnecessary call to get_textconv()
On Thu, Apr 04, 2013 at 01:48:52PM -0700, Junio C Hamano wrote: If I am reading the code correctly, it is has_changes(), which is used for log -S (not log -G that uses diff_grep()), that does the unnecessary get_textconv() unconditionally. The way diff_grep() divides the work to make fill_one() responsible for filling the textconv as necessary is internally consistent, and there is no unnecessary call. Yes, of course. I meant has_changes() which has the unnecessary call. Perhaps... The fill_one() function is responsible for finding and filling the textconv filter as necessary, and is called by diff_grep() function that implements git log -Gpattern. The has_changes() function calls get_textconv() for two sides being compared, before it checks to see if it was asked to perform the pickaxe limiting with the -S option. Move the code around to avoid this wastage. After that, fill_one() is called to use the textconv. By adding get_textconv() to diff_grep() and relieving fill_one() of responsibility to find the textconv filter, we can avoid calling get_textconv() twice. Sounds good to me. Regards Simon -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9 -- 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: Collective wisdom about repos on NFS accessed by concurrent clients (== corruption!?)
Kenneth Ölwing kenn...@olwing.se writes:
Basically, I'm at a place where I'm considering giving up getting this
to work reliably. In general, my setup work really fine, except for
the itty-bitty detail that when I put pressure on things I tend to get
into various kinds of trouble with the central repo being corrupted.
Can anyone authoritatively state anything either way?
My non-authoritative impression was that it's supposed to work
concurrently. Obviously something breaks:
My experience so far is that I eventually get repo corruption when I
stress it with concurrent read/write access from multiple hosts
(beyond any sort of likely levels, but still). Maybe I'm doing
something wrong, missing a configuration setting somewhere, put an
unfair stress on the system, there's a bona fide bug - or, given the
inherent difficulty in achieving perfect coherency between machines
on what's visible on the mount, it's just impossible (?) to truly
get it working under all situations.
Can you run the same tests under strace or similar, and gather the
relevant outputs? Otherwise it's probably very hard to say what is
going wrong.
In particular we've had some reports on lustre that boiled down to
impossible returns from libc functions, not git issues. It's hard to
say without some evidence.
--
Thomas Rast
trast@{inf,student}.ethz.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
[PATCH 0/2] log -L overlapping ranges
I noticed that it doesn't like getting multiple overlapping ranges from the user. This fixes it. Thomas Rast (2): log -L: check range set invariants when we look it up log -L: fix overlapping input ranges line-log.c | 43 +++-- t/t4211-line-log.sh | 6 ++ t/t4211/expect.multiple | 104 t/t4211/expect.multiple-overlapping | 187 t/t4211/expect.multiple-superset| 59 5 files changed, 392 insertions(+), 7 deletions(-) create mode 100644 t/t4211/expect.multiple create mode 100644 t/t4211/expect.multiple-overlapping create mode 100644 t/t4211/expect.multiple-superset -- 1.8.2.662.g6b31d33 -- 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 1/2] log -L: check range set invariants when we look it up
lookup_line_range() is a good place to check that the range sets
satisfy the invariants: they have been computed and set in earlier
iterations, and we now start working with them.
Signed-off-by: Thomas Rast tr...@inf.ethz.ch
---
line-log.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/line-log.c b/line-log.c
index 30edef4..5a12ceb 100644
--- a/line-log.c
+++ b/line-log.c
@@ -80,6 +80,25 @@ static int range_cmp(const void *_r, const void *_s)
}
/*
+ * Check that the ranges are non-empty, sorted and non-overlapping
+ */
+static void range_set_check_invariants(struct range_set *rs)
+{
+ int i;
+
+ if (!rs)
+ return;
+
+ if (rs-nr)
+ assert(rs-ranges[0].start rs-ranges[0].end);
+
+ for (i = 1; i rs-nr; i++) {
+ assert(rs-ranges[i-1].end rs-ranges[i].start);
+ assert(rs-ranges[i].start rs-ranges[i].end);
+ }
+}
+
+/*
* Helper: In-place pass of sorting and merging the ranges in the
* range set, to re-establish the invariants after another operation
*
@@ -103,6 +122,8 @@ static void sort_and_merge_range_set(struct range_set *rs)
}
assert(o = rs-nr);
rs-nr = o;
+
+ range_set_check_invariants(rs);
}
/*
@@ -687,8 +708,13 @@ static struct line_log_data *lookup_line_range(struct
rev_info *revs,
struct commit *commit)
{
struct line_log_data *ret = NULL;
+ struct line_log_data *d;
ret = lookup_decoration(revs-line_log_data, commit-object);
+
+ for (d = ret; d; d = d-next)
+ range_set_check_invariants(d-ranges);
+
return ret;
}
--
1.8.2.662.g6b31d33
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] log -L: fix overlapping input ranges
The existing code was too defensive, and would trigger the assert in
range_set_append() if the user gave overlapping ranges.
The intent was always to define overlapping ranges as just the union
of all of them, as evidenced by the call to sort_and_merge_range_set().
(Which was already used, unlike what the comment said.)
Fix by splitting out the meat of range_set_append() to a new _unsafe()
function that lacks the paranoia. sort_and_merge_range_set will fix
up the ranges, so we don't need the checks there.
Signed-off-by: Thomas Rast tr...@inf.ethz.ch
---
line-log.c | 17 ++--
t/t4211-line-log.sh | 6 ++
t/t4211/expect.multiple | 104
t/t4211/expect.multiple-overlapping | 187
t/t4211/expect.multiple-superset| 59
5 files changed, 366 insertions(+), 7 deletions(-)
create mode 100644 t/t4211/expect.multiple
create mode 100644 t/t4211/expect.multiple-overlapping
create mode 100644 t/t4211/expect.multiple-superset
diff --git a/line-log.c b/line-log.c
index 5a12ceb..85c7c24 100644
--- a/line-log.c
+++ b/line-log.c
@@ -56,16 +56,21 @@ static void range_set_move(struct range_set *dst, struct
range_set *src)
}
/* tack on a _new_ range _at the end_ */
-static void range_set_append(struct range_set *rs, long a, long b)
+static void range_set_append_unsafe(struct range_set *rs, long a, long b)
{
assert(a = b);
- assert(rs-nr == 0 || rs-ranges[rs-nr-1].end = a);
range_set_grow(rs, 1);
rs-ranges[rs-nr].start = a;
rs-ranges[rs-nr].end = b;
rs-nr++;
}
+static void range_set_append(struct range_set *rs, long a, long b)
+{
+ assert(rs-nr == 0 || rs-ranges[rs-nr-1].end = a);
+ range_set_append_unsafe(rs, a, b);
+}
+
static int range_cmp(const void *_r, const void *_s)
{
const struct range *r = _r;
@@ -99,10 +104,8 @@ static void range_set_check_invariants(struct range_set *rs)
}
/*
- * Helper: In-place pass of sorting and merging the ranges in the
- * range set, to re-establish the invariants after another operation
- *
- * NEEDSWORK currently not needed
+ * In-place pass of sorting and merging the ranges in the range set,
+ * to establish the invariants when we get the ranges from the user
*/
static void sort_and_merge_range_set(struct range_set *rs)
{
@@ -280,7 +283,7 @@ static void line_log_data_insert(struct line_log_data
**list,
struct line_log_data *p = search_line_log_data(*list, spec-path, ip);
if (p) {
- range_set_append(p-ranges, begin, end);
+ range_set_append_unsafe(p-ranges, begin, end);
sort_and_merge_range_set(p-ranges);
free_filespec(spec);
return;
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 426a828..2341351 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -39,6 +39,12 @@ canned_test -L 24,+1:a.c simple vanishes-early
canned_test -L '/long f/,/^}/:b.c' move-support move-support-f
+canned_test -L 4,12:a.c -L :main:a.c simple multiple
+canned_test -L 4,18:a.c -L :main:a.c simple multiple-overlapping
+canned_test -L :main:a.c -L 4,18:a.c simple multiple-overlapping
+canned_test -L 4:a.c -L 8,12:a.c simple multiple-superset
+canned_test -L 8,12:a.c -L 4:a.c simple multiple-superset
+
test_bad_opts -L switch.*requires a value
test_bad_opts -L b.c argument.*not of the form
test_bad_opts -L 1: argument.*not of the form
diff --git a/t/t4211/expect.multiple b/t/t4211/expect.multiple
new file mode 100644
index 000..76ad5b5
--- /dev/null
+++ b/t/t4211/expect.multiple
@@ -0,0 +1,104 @@
+commit 4659538844daa2849b1a9e7d6fadb96fcd26fc83
+Author: Thomas Rast tr...@student.ethz.ch
+Date: Thu Feb 28 10:48:43 2013 +0100
+
+change back to complete line
+
+diff --git a/a.c b/a.c
+--- a/a.c
b/a.c
+@@ -18,5 +18,7 @@
+ int main ()
+ {
+ printf(%ld\n, f(15));
+ return 0;
+-}
+\ No newline at end of file
++}
++
++/* incomplete lines are bad! */
+
+commit 100b61a6f2f720f812620a9d10afb3a960ccb73c
+Author: Thomas Rast tr...@student.ethz.ch
+Date: Thu Feb 28 10:48:10 2013 +0100
+
+change to an incomplete line at end
+
+diff --git a/a.c b/a.c
+--- a/a.c
b/a.c
+@@ -18,5 +18,5 @@
+ int main ()
+ {
+ printf(%ld\n, f(15));
+ return 0;
+-}
++}
+\ No newline at end of file
+
+commit a6eb82647d5d67f893da442f8f9375fd89a3b1e2
+Author: Thomas Rast tr...@student.ethz.ch
+Date: Thu Feb 28 10:45:16 2013 +0100
+
+touch both functions
+
+diff --git a/a.c b/a.c
+--- a/a.c
b/a.c
+@@ -3,9 +3,9 @@
+-int f(int x)
++long f(long x)
+ {
+ int s = 0;
+ while (x) {
+ x = 1;
+ s++;
+ }
+ return s;
+ }
+@@ -17,5 +17,5 @@
+ int main ()
+ {
+- printf(%d\n, f(15));
++ printf(%ld\n, f(15));
+ return 0;
+ }
+
+commit f04fb20f2c77850996cba739709acc6faecc58f7
+Author: Thomas Rast
Re: Collective wisdom about repos on NFS accessed by concurrent clients (== corruption!?)
On 2013-04-05 15:42, Thomas Rast wrote: Can you run the same tests under strace or similar, and gather the relevant outputs? Otherwise it's probably very hard to say what is going wrong. In particular we've had some reports on lustre that boiled down to impossible returns from libc functions, not git issues. It's hard to say without some evidence. Thomas, thanks for your reply. I'm assuming I should strace the git commands as they're issued? I'm already collecting regular stdout/err output in a log as I go. Is there any debugging things I can turn on to make the calls issue internal tracing of some sort? The main issue I see is that I suspect it will generate so much data that it'll overflow my disk ;-). Consider that my hammer consists of a Perl script that forks a number of tasks (e.g. 15) that each loops doing clone/commit/push/pull, with retrying on a few levels as errors occur (usually expected ones due to the concurrency, i.e. someone else pushed so a pull is necessary first, but occasionally the central repo is broken enough that it can't be cloned from, or at least not checked out master from...sometimes with printed errors that still give me a zero exit code...). That is then also run on several machines to the same repo to hopefully cause a breakage by sheer pounding...it's going to generate huge collections of strace output I expect... I have some variations of this (e.g. all tasks are working on different branches, improving concurrency in some respects, but effects there have been that at the end I was missing a branch or so...). The likelihood of problems seems to increase when I actually use ssh in my ultimate setup, so a loadbalancer roundrobins each call to any of several hosts. In that case I must admit I don't know how to get in on the action since I guess I would need to strace the git-upload/receive-pack processes on the server side...? Lastly, I don't know how much this will impact timings etc, or load. To get a broken result I have sometimes needed to run for many hours, others fairly quickly. Well...I will try, it'll probably be a blast :-) BTW, this is mostly done on Centos 6.3 and 6.4, locally built git-1.8.2. ken1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] perl: redirect stderr to /dev/null instead of closing
Hi!
On Thu, Apr 04, 2013 at 10:41:41PM +0200, Thomas Rast wrote:
As pointed out by Eric Wong (thanks), the initial close needs to go:
die() would again write nowhere if we close STDERR beforehand.
Perhaps we should also do the following:
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1489,9 +1489,6 @@ sub _command_common_pipe {
if (not defined $pid) {
throw Error::Simple(open failed: $!);
} elsif ($pid == 0) {
- if (defined $opts{STDERR}) {
- close STDERR;
- }
if ($opts{STDERR}) {
open (STDERR, '', $opts{STDERR})
or die dup failed: $!;
Indeed. Thanks for pointing that out.
I'm sorry, I don't follow. Doesn't this just break the STDERR option
altogether as we will try to dup2() over an already open file
descriptor? We do need to close STDERR if we are going to reopen it,
I think.
Kind regards,
Petr Pasky Baudis
--
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] glossary: extend detached HEAD description
When we introduced the concept of detached HEAD, we made sure that commands that operate on the history of the current branch just work in that state. They update the HEAD to point at the new history without affecting any branch when the HEAD is detached, just like they update the tip of the current branch to point at the new history when HEAD points at a specific branch. As this is done as the natural extension for these commands, we did not, we still do not, and we do not want to repeat A detached HEAD is updated without affecting any branch when describing what each and every one of these commands that operate on the current branch do. Add a blanket description to the glossary to cover them instead. The general principle is that operations to update the branch work and affect on the HEAD, while operations to update the information about a branch do not. Signed-off-by: Junio C Hamano gits...@pobox.com --- * Noticed that we describe commit --amend as updating the current branch while reviewing the recent update by CMN, and it is shared by all the manual pages of commands that work on the current branch. git grep -i detached Documentation/ shows very small number of hits, indicating that we do not repeat if detached, only the HEAD is updated. I am not absolutely sure if glossary is the best place, but something like this may be necessary to help people who are new to this state. Documentation/glossary-content.txt | 19 --- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index f928b57..69c90d1 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -100,9 +100,22 @@ to point at the new commit. [[def_detached_HEAD]]detached HEAD:: Normally the def_HEAD,HEAD stores the name of a - def_branch,branch. However, git also allows you to def_checkout,check out - an arbitrary def_commit,commit that isn't necessarily the tip of any - particular branch. In this case HEAD is said to be detached. + def_branch,branch, and commands that operate on the + history HEAD represents operate on the history leading to the + tip of the branch the HEAD points at. However, Git also + allows you to def_checkout,check out an arbitrary + def_commit,commit that isn't necessarily the tip of any + particular branch. The HEAD in such a state is called + detached. ++ +Note that commands that operate on the history of the current branch +(e.g. `git commit` to build a new history on top of it) still work +while the HEAD is detached. They update the HEAD to point at the tip +of the updated history without affecting any branch. Commands that +update or inquire information _about_ the current branch (e.g. `git +branch --set-upstream-to` that sets what remote tracking branch the +current branch integrates with) obviously do not work, as there is no +(real) current branch to ask about in this state. [[def_dircache]]dircache:: You are *way* behind. See def_index,index. -- 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 0/7] Rework git core for native submodules
On Thu, Apr 4, 2013 at 1:04 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Linus Torvalds wrote: Or you could also just edit and carry a dirty .gitmodules around for your personal use-case. I'm sorry, but a dirty worktree is unnecessarily painful to work with. Bzzt. Wrong. A dirty worktree is not only easy to work with (I do it all the time, having random test-patches in my tree that I never even intend to commit), it's a *requirement*. One thing that git does really really well is merging. And one of the reasons why git does merging well (apart from the obvious meta-issue: it's what I care about) is that it not only has the stable information in the object database, it also has the staging information in the index, *and* it has dirty data in the working tree. You absolutely need all three. Having an edit command to edit stable data (or staging data) is broken. Trust me, I've been there, done that, got the T-shirt and know it is wrong. The whole stable objects + index + dirty worktree is FUNDAMENTALLY the right way to work, and it *has* to work that way for merges to work well. The only things that we don't have dirty data for in the worktree is creating commits and tags, but those aren't relevant for the merging process anyway, in the sense that you never change them for merging, you create them *after* merging (and this is fundamental, and not just a git implementation issue). So you absolutely need a dirty worktree. You need it for testing, and you need it for merging. Having a model where you don't have a in-progress entity that works as a temporary is absolutely and entirely wrong. Linus -- 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] submodule: print graph output next to submodule log
From: John Keeping j...@metanate.com
When running git log -p --submodule=log, the submodule log is not
indented by the graph output, although all other lines are. Fix this by
prepending the current line prefix to each line of the submodule log.
Signed-off-by: John Keeping j...@keeping.me.uk
---
diff.c | 1 +
submodule.c | 13 +
submodule.h | 1 +
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/diff.c b/diff.c
index db952a5..28a742c 100644
--- a/diff.c
+++ b/diff.c
@@ -2255,6 +2255,7 @@ static void builtin_diff(const char *name_a,
const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
show_submodule_summary(o-file, one ? one-path : two-path,
+ line_prefix,
one-sha1, two-sha1, two-dirty_submodule,
meta, del, add, reset);
return;
diff --git a/submodule.c b/submodule.c
index 975bc87..e728025 100644
--- a/submodule.c
+++ b/submodule.c
@@ -216,6 +216,7 @@ static int prepare_submodule_summary(struct rev_info *rev,
const char *path,
}
static void print_submodule_summary(struct rev_info *rev, FILE *f,
+ const char *line_prefix,
const char *del, const char *add, const char *reset)
{
static const char format[] = %m %s;
@@ -226,6 +227,7 @@ static void print_submodule_summary(struct rev_info *rev,
FILE *f,
struct pretty_print_context ctx = {0};
ctx.date_mode = rev-date_mode;
strbuf_setlen(sb, 0);
+ strbuf_addstr(sb, line_prefix);
if (commit-object.flags SYMMETRIC_LEFT) {
if (del)
strbuf_addstr(sb, del);
@@ -256,6 +258,7 @@ int parse_fetch_recurse_submodules_arg(const char *opt,
const char *arg)
}
void show_submodule_summary(FILE *f, const char *path,
+ const char *line_prefix,
unsigned char one[20], unsigned char two[20],
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset)
@@ -280,16 +283,18 @@ void show_submodule_summary(FILE *f, const char *path,
message = (revision walker failed);
if (dirty_submodule DIRTY_SUBMODULE_UNTRACKED)
- fprintf(f, Submodule %s contains untracked content\n, path);
+ fprintf(f, %sSubmodule %s contains untracked content\n,
+ line_prefix, path);
if (dirty_submodule DIRTY_SUBMODULE_MODIFIED)
- fprintf(f, Submodule %s contains modified content\n, path);
+ fprintf(f, %sSubmodule %s contains modified content\n,
+ line_prefix, path);
if (!hashcmp(one, two)) {
strbuf_release(sb);
return;
}
- strbuf_addf(sb, %sSubmodule %s %s.., meta, path,
+ strbuf_addf(sb, %s%sSubmodule %s %s.., line_prefix, meta, path,
find_unique_abbrev(one, DEFAULT_ABBREV));
if (!fast_backward !fast_forward)
strbuf_addch(sb, '.');
@@ -301,7 +306,7 @@ void show_submodule_summary(FILE *f, const char *path,
fwrite(sb.buf, sb.len, 1, f);
if (!message) /* only NULL if we succeeded in setting up the walk */
- print_submodule_summary(rev, f, del, add, reset);
+ print_submodule_summary(rev, f, line_prefix, del, add, reset);
if (left)
clear_commit_marks(left, ~0);
if (right)
diff --git a/submodule.h b/submodule.h
index 3dc1b3f..c7ffc7c 100644
--- a/submodule.h
+++ b/submodule.h
@@ -19,6 +19,7 @@ int parse_submodule_config_option(const char *var, const char
*value);
void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
void show_submodule_summary(FILE *f, const char *path,
+ const char *line_prefix,
unsigned char one[20], unsigned char two[20],
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset);
--
1.8.2.452.gb520e27
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] submodule: print graph output next to submodule log
When running git log -p --submodule=log, the submodule log is not
indented by the graph output, although all other lines are. Fix this by
prepending the current line prefix to each line of the submodule log.
Signed-off-by: John Keeping j...@keeping.me.uk
---
On Fri, Apr 05, 2013 at 05:06:30PM +0100, John Keeping wrote:
From: John Keeping j...@metanate.com
That shouldn't have escaped here :-( The email in the S-O-B is the
right one.
No other change from v1.
diff.c | 1 +
submodule.c | 13 +
submodule.h | 1 +
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/diff.c b/diff.c
index db952a5..28a742c 100644
--- a/diff.c
+++ b/diff.c
@@ -2255,6 +2255,7 @@ static void builtin_diff(const char *name_a,
const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
show_submodule_summary(o-file, one ? one-path : two-path,
+ line_prefix,
one-sha1, two-sha1, two-dirty_submodule,
meta, del, add, reset);
return;
diff --git a/submodule.c b/submodule.c
index 975bc87..e728025 100644
--- a/submodule.c
+++ b/submodule.c
@@ -216,6 +216,7 @@ static int prepare_submodule_summary(struct rev_info *rev,
const char *path,
}
static void print_submodule_summary(struct rev_info *rev, FILE *f,
+ const char *line_prefix,
const char *del, const char *add, const char *reset)
{
static const char format[] = %m %s;
@@ -226,6 +227,7 @@ static void print_submodule_summary(struct rev_info *rev,
FILE *f,
struct pretty_print_context ctx = {0};
ctx.date_mode = rev-date_mode;
strbuf_setlen(sb, 0);
+ strbuf_addstr(sb, line_prefix);
if (commit-object.flags SYMMETRIC_LEFT) {
if (del)
strbuf_addstr(sb, del);
@@ -256,6 +258,7 @@ int parse_fetch_recurse_submodules_arg(const char *opt,
const char *arg)
}
void show_submodule_summary(FILE *f, const char *path,
+ const char *line_prefix,
unsigned char one[20], unsigned char two[20],
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset)
@@ -280,16 +283,18 @@ void show_submodule_summary(FILE *f, const char *path,
message = (revision walker failed);
if (dirty_submodule DIRTY_SUBMODULE_UNTRACKED)
- fprintf(f, Submodule %s contains untracked content\n, path);
+ fprintf(f, %sSubmodule %s contains untracked content\n,
+ line_prefix, path);
if (dirty_submodule DIRTY_SUBMODULE_MODIFIED)
- fprintf(f, Submodule %s contains modified content\n, path);
+ fprintf(f, %sSubmodule %s contains modified content\n,
+ line_prefix, path);
if (!hashcmp(one, two)) {
strbuf_release(sb);
return;
}
- strbuf_addf(sb, %sSubmodule %s %s.., meta, path,
+ strbuf_addf(sb, %s%sSubmodule %s %s.., line_prefix, meta, path,
find_unique_abbrev(one, DEFAULT_ABBREV));
if (!fast_backward !fast_forward)
strbuf_addch(sb, '.');
@@ -301,7 +306,7 @@ void show_submodule_summary(FILE *f, const char *path,
fwrite(sb.buf, sb.len, 1, f);
if (!message) /* only NULL if we succeeded in setting up the walk */
- print_submodule_summary(rev, f, del, add, reset);
+ print_submodule_summary(rev, f, line_prefix, del, add, reset);
if (left)
clear_commit_marks(left, ~0);
if (right)
diff --git a/submodule.h b/submodule.h
index 3dc1b3f..c7ffc7c 100644
--- a/submodule.h
+++ b/submodule.h
@@ -19,6 +19,7 @@ int parse_submodule_config_option(const char *var, const char
*value);
void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
void show_submodule_summary(FILE *f, const char *path,
+ const char *line_prefix,
unsigned char one[20], unsigned char two[20],
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset);
--
1.8.2.452.gb520e27
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] submodule: print graph output next to submodule log
Hi John, On Fri, 5 Apr 2013, John Keeping wrote: When running git log -p --submodule=log, the submodule log is not indented by the graph output, although all other lines are. Fix this by prepending the current line prefix to each line of the submodule log. Nice! I agree that submodules are an ugly child of Git; not even Git itself uses them ;-) Thanks for that fix! Ciao, Dscho -- 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 0/7] Rework git core for native submodules
Linus Torvalds wrote:
So you absolutely need a dirty worktree. You need it for testing, and
you need it for merging. Having a model where you don't have a
in-progress entity that works as a temporary is absolutely and
entirely wrong.
I agree entirely. My comment was just a by the way, and specific to
how people work with .gitmodules: I didn't imply any strong notions of
Right or Wrong with respect to dirty worktrees in general. So, yes:
links stage and unstage, just like blobs do.
Oh, and I'm currently writing infrastructure to work with links like
blobs. Here's a WIP: git cat-link link is exactly the same as cat
file, to the end user.
-- 8 --
From d8a1de6f9075771dde6f1fde9ffa193dce386a17 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra artag...@gmail.com
Date: Fri, 5 Apr 2013 19:42:56 +0530
Subject: [PATCH] builtin/cat-link: implement new builtin
This is a simple program that calls unpack_trees() with a custom
callback that just prints the contents of whatever objects were
matched using revs.prune_data. Blobs can be cat'ed directly from the
filesystem, so this program is primarily useful for links; git
cat-link link shows it up like a blob.
We will use this program to build edit-link.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
Makefile | 3 +-
builtin.h | 1 +
builtin/cat-link.c | 83 ++
diff-lib.c | 10 +++
diff.h | 6
git.c | 1 +
6 files changed, 98 insertions(+), 6 deletions(-)
create mode 100644 builtin/cat-link.c
diff --git a/Makefile b/Makefile
index cd4b6f9..28194d7 100644
--- a/Makefile
+++ b/Makefile
@@ -349,7 +349,7 @@ GIT-VERSION-FILE: FORCE
# CFLAGS and LDFLAGS are for the users to override from the command line.
-CFLAGS = -g -O2 -Wall
+CFLAGS = -g -O0 -Wall
LDFLAGS =
ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
ALL_LDFLAGS = $(LDFLAGS)
@@ -893,6 +893,7 @@ BUILTIN_OBJS += builtin/blame.o
BUILTIN_OBJS += builtin/branch.o
BUILTIN_OBJS += builtin/bundle.o
BUILTIN_OBJS += builtin/cat-file.o
+BUILTIN_OBJS += builtin/cat-link.o
BUILTIN_OBJS += builtin/check-attr.o
BUILTIN_OBJS += builtin/check-ignore.o
BUILTIN_OBJS += builtin/check-ref-format.o
diff --git a/builtin.h b/builtin.h
index faef559..be0160d 100644
--- a/builtin.h
+++ b/builtin.h
@@ -49,6 +49,7 @@ extern int cmd_blame(int argc, const char **argv, const char
*prefix);
extern int cmd_branch(int argc, const char **argv, const char *prefix);
extern int cmd_bundle(int argc, const char **argv, const char *prefix);
extern int cmd_cat_file(int argc, const char **argv, const char *prefix);
+extern int cmd_cat_link(int argc, const char **argv, const char *prefix);
extern int cmd_checkout(int argc, const char **argv, const char *prefix);
extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);
extern int cmd_check_attr(int argc, const char **argv, const char *prefix);
diff --git a/builtin/cat-link.c b/builtin/cat-link.c
new file mode 100644
index 000..14dd92b
--- /dev/null
+++ b/builtin/cat-link.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) 2013 Ramkumar Ramachandra
+ */
+#include cache.h
+#include tree.h
+#include cache-tree.h
+#include unpack-trees.h
+#include commit.h
+#include diff.h
+#include revision.h
+
+static int cat_file(struct cache_entry **src, struct unpack_trees_options *o) {
+ int cached, match_missing = 1;
+ unsigned dirty_submodule = 0;
+ unsigned int mode;
+ const unsigned char *sha1;
+ struct cache_entry *idx = src[0];
+ struct cache_entry *tree = src[1];
+ struct rev_info *revs = o-unpack_data;
+ enum object_type type;
+ unsigned long size;
+ char *buf;
+
+ cached = o-index_only;
+ if (ce_path_match(idx ? idx : tree, revs-prune_data)) {
+ if (get_stat_data(idx, sha1, mode, cached, match_missing,
+ dirty_submodule, NULL) 0)
+ die(Something went wrong!);
+ buf = read_sha1_file(sha1, type, size);
+ printf(%s, buf);
+ }
+ return 0;
+}
+
+int cmd_cat_link(int argc, const char **argv, const char *prefix)
+{
+ struct unpack_trees_options opts;
+ int cached = 1;
+ struct rev_info revs;
+ struct tree *tree;
+ struct tree_desc tree_desc;
+ struct object_array_entry *ent;
+
+ if (argc 2)
+ die(Usage: git cat-link link);
+
+ init_revisions(revs, prefix);
+ setup_revisions(argc, argv, revs, NULL); /* For revs.prune_data */
+ add_head_to_pending(revs);
+
+ /* Hack to diff against index; we create a dummy tree for the
+ index information */
+ if (!revs.pending.nr) {
+ struct tree *tree;
+ tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
+ add_pending_object(revs, tree-object, HEAD);
+ }
+
+ if (read_cache() 0)
+
Re: [PATCH 3/3] diffcore-pickaxe: unify setup and teardown code between log -S/-G
Jeff King p...@peff.net writes:
I didn't actually test that patch beyond compilation (but it's
_obviously_ correct, right?), and I'm about to go to bed. Do you want to
take care of adapting your commit message to it?
The code looks obviously correct, yes.
We might have to revisit the Is unmerged? thing, Cf. d7c9bf22351e
(diffcore-rename: don't consider unmerged path as source,
2011-03-23), but that is an unlikely corner case, especially for
these options (-S/-G).
-- 8 --
From: Jeff King p...@peff.net
Date: Fri, 5 Apr 2013 01:28:10 -0400
Subject: [PATCH] diffcore-pickaxe: unify code for log -S/-G
The logic flow of has_changes() used for log -S and diff_grep()
used for log -G are essentially the same. See if we have both
sides that could be different in any interesting way, slurp the
contents in core, possibly after applying textconv, inspect the
contents, clean-up and report the result. The only difference
between the two is how inspect step works.
Unify this codeflow in a helper, pickaxe_match(), which takes a
callback function that implements the specific inspect step.
After removing the common scaffolding code from the existing
has_changes() and diff_grep(), they each becomes such a callback
function suitable for passing to pickaxe_match().
Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
---
diffcore-pickaxe.c | 118 ++---
1 file changed, 49 insertions(+), 69 deletions(-)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index cadb071..63722f8 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -8,7 +8,12 @@
#include xdiff-interface.h
#include kwset.h
-typedef int (*pickaxe_fn)(struct diff_filepair *p, struct diff_options *o,
regex_t *regexp, kwset_t kws);
+typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,
+ struct diff_options *o,
+ regex_t *regexp, kwset_t kws);
+
+static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
+regex_t *regexp, kwset_t kws, pickaxe_fn fn);
static void pickaxe(struct diff_queue_struct *q, struct diff_options *o,
regex_t *regexp, kwset_t kws, pickaxe_fn fn)
@@ -22,7 +27,7 @@ static void pickaxe(struct diff_queue_struct *q, struct
diff_options *o,
/* Showing the whole changeset if needle exists */
for (i = 0; i q-nr; i++) {
struct diff_filepair *p = q-queue[i];
- if (fn(p, o, regexp, kws))
+ if (pickaxe_match(p, o, regexp, kws, fn))
return; /* do not munge the queue */
}
@@ -37,7 +42,7 @@ static void pickaxe(struct diff_queue_struct *q, struct
diff_options *o,
/* Showing only the filepairs that has the needle */
for (i = 0; i q-nr; i++) {
struct diff_filepair *p = q-queue[i];
- if (fn(p, o, regexp, kws))
+ if (pickaxe_match(p, o, regexp, kws, fn))
diff_q(outq, p);
else
diff_free_filepair(p);
@@ -74,64 +79,33 @@ static void diffgrep_consume(void *priv, char *line,
unsigned long len)
line[len] = hold;
}
-static int diff_grep(struct diff_filepair *p, struct diff_options *o,
+static int diff_grep(mmfile_t *one, mmfile_t *two,
+struct diff_options *o,
regex_t *regexp, kwset_t kws)
{
regmatch_t regmatch;
- struct userdiff_driver *textconv_one = NULL;
- struct userdiff_driver *textconv_two = NULL;
- mmfile_t mf1, mf2;
- int hit;
+ struct diffgrep_cb ecbdata;
+ xpparam_t xpp;
+ xdemitconf_t xecfg;
- if (!o-pickaxe[0])
- return 0;
+ if (!one)
+ return !regexec(regexp, two-ptr, 1, regmatch, 0);
+ if (!two)
+ return !regexec(regexp, one-ptr, 1, regmatch, 0);
- if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
- textconv_one = get_textconv(p-one);
- textconv_two = get_textconv(p-two);
- }
-
- if (textconv_one == textconv_two diff_unmodified_pair(p))
- return 0;
-
- mf1.size = fill_textconv(textconv_one, p-one, mf1.ptr);
- mf2.size = fill_textconv(textconv_two, p-two, mf2.ptr);
-
- if (!DIFF_FILE_VALID(p-one)) {
- if (!DIFF_FILE_VALID(p-two))
- hit = 0; /* ignore unmerged */
- else
- /* created two -- does it have what we are looking
for? */
- hit = !regexec(regexp, mf2.ptr, 1, regmatch, 0);
- } else if (!DIFF_FILE_VALID(p-two)) {
- /* removed one -- did it have what we are looking for? */
- hit = !regexec(regexp, mf1.ptr, 1, regmatch, 0);
- } else {
-
Re: Possible bug: Git submodules can get into broken state
Chris Wilson chris+git...@aptivate.org writes: They fixed it by checking in a rectified .gitmodules file. In the mean time, I ran git submodule init, and now I've ended up in a situation where git submodule update doesn't work, and there's no submodule command to fix it, so I have to remove the broken submodules from .git/config. and the configuration mechanism is the way to use custom URL in place of it. I don't want to use a custom URL, I want to use the URL which is now in .gitmodules. Then don't call it a custom URL ;-). Isn't your problem that the old, broken one is now in your config, and you want to update that with a corrected one? How you learned which one is correct does not really matter---you may learned it out of band by looking at upstream's gitweb, or by running git fetch in the superproject again and looking at the updated .gitmodules file. The configuration mechanism has a way to update that entry. Could submodule init at least change the URLs of submodules which are not checked out? (e.g. because they couldn't be)? Perhaps submodule deinit might be what you are looking for, but I dunno. -- 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: Beyond inotify recursive watches
Jan Kara wrote: Hum, I have somewhat hard time to understand what do you mean by 'magically optimized syscalls'. What should happen in VFS to speedup your load? In retrospect, I think this is a terrible hack to begin with. Tuning the filesystem specifically for git repositories is inelegant on so many levels, I can't recall why I ever thought it would be a good idea. Like all software, Git has scaling issues with ultra-large repositories. Too many stat() calls is just one of the problems: there will be too many objects to do any operation at reasonable speed, and the overall UX would just suck. Instead of growing to a huge monolithic beast that spawns off worker threads for everything and ultimately dying off, I've decided that git should take a different direction: it should work with well with many small easily-composable repositories. I've started work on this already, and it looks very promising. Let the filesystem people do what they do best: optimizing for all applications uniformly. -- 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 0/7] Rework git core for native submodules
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: git log -p .gitmodules would be a way to review what changed in the information about submodules. Don't you need git log-link for exactly the same reason why you need git diff-link in the first place? So you may not have suggested it, but I suspect that was only because you haven't had enough time to think things through. What is this git log -p .gitmodules doing? It's walking down the commit history, and picking out the commits in which that blob changed. Then it's diffing the blobs in those commits with each other. Why is git log -p link any different? We already know how to diff blobs, and we just need a way to diff links. You already forget what you invented git diff-link as a solution for, perhaps? By recording the submodules themselve and information _about_ the submodules separately (the latter is in .gitmodules), git diff A can show the difference in submodule A, while git diff .gitmodules can show a change, which is a possibly in-working-tree-only proposed change, in information about submoudules. Once you start recording the latter also at path A, it becomes unclear what git diff A should show. That is what I said in the message, to which you invented diff-link as a solution to the unclear-ness. Am I misremembering the flow of discussion in this thread? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 09/12] pretty: add %C(auto) for auto-coloring on the next placeholder
Duy Nguyen pclo...@gmail.com writes: The above should have written will turn on auto coloring on the following _textual_ placeholder. I didn't intend %C(auto) to be followed by %C(color) as it's already covered by %C(auto,red). But of course we could make it work too. You are right that there is no need to say %C(auto)%C(red), it is %C(auto,red), but that misses the point. If %C(auto) applies to some %placeholder but not to some others, the user needs to learn which %placeholder will eat the auto (so it no longer applies to the next one) and which one will not even look at auto (so the next %placeholder is affected by the auto, i.e. making the effect of auto skip a %placeholder). If the rule were %C(auto) applies to -next- placeholder, then the user does not have to worry about which ones are what you call textual and which ones are not (and there is no textual placeholder defined in the glossary). That would make it harder to learn. It would be much easier to explain if you said %C(auto) affects the next %-placeholder and then resets. I wonder if Everything after %C(auto) will not be coloured if the output is not going to the terminal., i.e. not resetting once colouring decision is made, makes more sense, though... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] Rework git core for native submodules
Junio C Hamano wrote: Once you start recording the latter also at path A, it becomes unclear what git diff A should show. That is what I said in the message, to which you invented diff-link as a solution to the unclear-ness. I just thought it would be a stopgap until we get diff to support links natively. Obviously, when we get native diff support, 'log -p' will be able to show differences as well. As it turns out from my little experiment with cat-link, it's really easy to get native diff support, and I'm targeting that directly instead of a scripted solution. As for the unclearness issue, it's a little more complicated than that: a non-floating submodule could've previously been a floating one, or vice-versa. As of this moment, I'm only planning to show differences between link buffers. In the case when two consecutive commits change a link checkout_rev (where floating is set to 0), we can come up with something like the current diff.submodule = log. I see no cause to worry about the interface of that now. -- 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 3/3] diffcore-pickaxe: respect --no-textconv
Thanks; will replace the one in 'pu' with this. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] log -L overlapping ranges
Thomas Rast tr...@inf.ethz.ch writes: I noticed that it doesn't like getting multiple overlapping ranges from the user. This fixes it. Both look sensible. Will queue on top of tr/line-log. 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 v2 1/2] perl: redirect stderr to /dev/null instead of closing
Petr Baudis pa...@ucw.cz writes:
} elsif ($pid == 0) {
- if (defined $opts{STDERR}) {
- close STDERR;
- }
if ($opts{STDERR}) {
open (STDERR, '', $opts{STDERR})
or die dup failed: $!;
Indeed. Thanks for pointing that out.
I'm sorry, I don't follow. Doesn't this just break the STDERR option
altogether as we will try to dup2() over an already open file
descriptor? We do need to close STDERR if we are going to reopen it,
I think.
When $opts{STDERR} is 2, what the three lines the proposed patch
removes did is actively wrong, because you dup2 the fd you just
closed.
When $opts{STDERR} is 1, it seems to do the right thing with or
without the close STDERR in front. Isn't this because the usual
open($fd, anything) closes $fd as necessary applies to this
case as well?
So, I am not sure what you are viewing as a problem. Puzzled...
--
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: [RFD] annnotating a pair of commit objects?
On Thu, Jan 3, 2013 at 10:59 AM, Michael Haggerty mhag...@alum.mit.edu wrote: On 01/03/2013 08:03 AM, Junio C Hamano wrote: I'd like a datastore that maps a pair of commit object names to another object name, such that: * When looking at two commits A and B, efficiently query all data associated with a pair of commits X,Y where X is contained in the range A..B and not in B..A, and Y is contained in the range B..A and not in A..B. * When X,Y is registered in the datastore, and X is rewritten to X' and/or Y is rewritten to Y', the mapping is updated so that it can be queried with X',Y' as a new key, similar to the way a notes tree that maps object X can be updated to map object X' when such a rewrite happens. The intended use case is to go beyond rerere. Given a history of this shape: o---o---o---I mainline / O---o---X---o---Atopic A \ o---Y---o---o---B topic B I would indeed also be interested in such a feature for my day-to-day work where we use a workflow similar to git.git's. When doing this merge, I think your goal is equivalent to discovering that M includes part of the merge of J and B, and adding M as an (implicit or explicit) third parent to the merge: o---o---o---I---J---K mainline / /. / O---o---X---o---A. /topic A \ \ . / \ M. / \ / / o---Y---o---o---Btopic B How could M be stored? Assuming that these type of premerge merges are sparse, then Jeff's analysis seems good. Concretely, one could simply store pointers to M from both X and Y; e.g., * Add a note to X with the information when merging this commit with Y, use premerge M * Add a note to Y with the information when merging this commit with X, use premerge M Then, when merging, create the set J..B, scan all of the commits on B..J for these premerge notes (O(|B..J|)), and for each one, look in the set J..B to see if it is present. The effort should scale like O( |J..B| + |B..J| * lg(|J..B|) ) where, of course J and B could be exchanged for either aesthetic or performance reasons. (One would also need a mechanism for preventing M from being garbage-collected.) I like the idea of using notes and a kind of pre-merge. The proposal seems decent to me. Michael, have you started implementing such a thing ? If you did, I would love to help as much as I can. If you didn't, I would like to start implementing this feature (I think I now have some time to do so). Maybe that would require some kind of mentoring though. It could be a nice opportunity for the community to improve that too as a fake gsoc (no google, no summer, no student) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] count-objects: output KiB instead of kilobytes
Should we use that opportunity to implement an option like -h (for humanize) similar to what ls(1), df(1), du(1) does ? Of course -h is already used for help, so we could use -H or any other sensible choice. It can become tough to read the size when it gets big enough. On Thu, Apr 4, 2013 at 6:27 PM, Junio C Hamano gits...@pobox.com wrote: Mihai Capotă mi...@mihaic.ro writes: The git manual contains an explicit warning about the output of a porcelain command changing: The interface to Porcelain commands on the other hand are subject to change in order to improve the end user experience. Yeah, I know that, as I wrote it ;-) Aside from count-object being not exactly a Porcelain, the statement does not give us a blank check to make random changes as we see fit. There needs to be a clear improvement. I am just having a hard time weighing the benefit of using more accurate kibibytes over kilobytes and the possible downside of breaking other peoples' tools. Perhaps it would be alright if the change was accompanied by a warning in the Release Notes to say something like: If you have scripts that decide when to run git repack by parsing the output from git count-objects, this release may break them. Sorry about that. One of the scripts shipped by git-core itself also had to be adjusted. The command reports the total diskspace used to store loose objects in kibibytes, but it was labelled as kilobytes. The number now is shown with KiB, e.g. 6750 objects, 50928 KiB. You may want to consider updating such scripts to always call git gc --auto to let it decide when to repack for you. Also, I suspect that for the purpose of this exact output field, nobody cares the difference between kibibytes and kilobytes. Depending on the system, we add up either st.st_blocks or st.st_size and the result is not that exact as how much diskspace is consumed. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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] show-branch: use strbuf instead of static buffer
When we generate relative names (e.g., master~20^2), we
format the name into a static buffer, then xstrdup the
result to attach it to the commit. Since the first thing we
add into the static buffer is the already-computed name of
the child commit, the names may get longer and longer as
the traversal gets deeper, and we may eventually overflow
the fixed-size buffer.
Fix this by converting the fixed-size buffer into a dynamic
strbuf. The performance implications should be minimal, as
we end up allocating a heap copy of the name anyway (and now
we can just detach the heap copy from the strbuf).
Reported-by: Eric Roman ero...@chromium.org
Signed-off-by: Jeff King p...@peff.net
---
This is a fix for a bug report that came to me off-list. A real-world
example can be seen by running git show-branch --all on a fresh clone
of:
https://chromium.googlesource.com/chromium/src.git
(but that repo is 1.7G, so I don't recommend cloning it unless you're
really interested). Its master branch consists of a strange sequence of
merges that results in naming commits like master^2^2^2^2... and so on
(it's unclear to me why, but it looks like maybe syncing up separate svn
and git repositories?). Which is odd, but looking at graph, I think the
names show-branch is generating are correct; they're just really long.
And of course odd history is no excuse to overflow a buffer.
Though this is a stack overflow, I don't know that it's exploitable for
anything interesting; an attacker does not get to write arbitrary data,
but rather only a sequence of ^%d and ~%d relative history markers.
Perhaps in theory one could devise a history such that the sequence
markers spelled out some malicious code, but it would be quite a
challenge (and given that you have only ascii [^~0-9] to work with,
probably impossible).
I prepared this on master, but it should be suitable for maint; the
code dates all the way back to git v0.99.
builtin/show-branch.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index d208fd6..90fc6b1 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -162,29 +162,28 @@ static void name_commits(struct commit_list *list,
nth = 0;
while (parents) {
struct commit *p = parents-item;
- char newname[1000], *en;
+ struct strbuf newname = STRBUF_INIT;
parents = parents-next;
nth++;
if (p-util)
continue;
- en = newname;
switch (n-generation) {
case 0:
- en += sprintf(en, %s, n-head_name);
+ strbuf_addstr(newname, n-head_name);
break;
case 1:
- en += sprintf(en, %s^, n-head_name);
+ strbuf_addf(newname, %s^,
n-head_name);
break;
default:
- en += sprintf(en, %s~%d,
- n-head_name, n-generation);
+ strbuf_addf(newname, %s~%d,
+ n-head_name,
n-generation);
break;
}
if (nth == 1)
- en += sprintf(en, ^);
+ strbuf_addch(newname, '^');
else
- en += sprintf(en, ^%d, nth);
- name_commit(p, xstrdup(newname), 0);
+ strbuf_addf(newname, ^%d, nth);
+ name_commit(p, strbuf_detach(newname, NULL),
0);
i++;
name_first_parent_chain(p);
}
--
1.8.2.rc0.33.gd915649
--
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 0/9] friendlier http error messages
The error messages that git generates for routine http problems can sometimes be a bit verbose or confusing. They also provide no opportunity for the server to communicate any free-form text, even though the server knows much better than the git client the reason for the error or what the next step to suggest to the user might be. This series provides a channel for those messages, and does some general cleanup and reformatting of the error messages that git itself produces. Here are some before-and-after examples with this series (apologies for the long lines, but they are part of the ugliness I want to address, so I am leaving them in). The remote: bits are simulated in the output below, as I haven't yet updated GitHub's server side to produce more useful messages. So you can repeat the tests, but note that the text you get from the server will not be the same (e.g., our 404 currently just says Repository not found which is not all that helpful). [before] $ git ls-remote https://github.com/non/existent fatal: https://github.com/non/existent/info/refs?service=git-upload-pack not found: did you run git update-server-info on the server? [after] $ git ls-remote https://github.com/non/existent remote: The remote repository was not found, or you do not have remote: permission to access it. fatal: repository 'https://github.com/non/existent/' not found [before] $ GIT_SMART_HTTP=0 git ls-remote https://github.com/git/git.git error: The requested URL returned error: 403 Forbidden while accessing https://github.com/git/git.git/info/refs fatal: HTTP request failed [after] $ GIT_SMART_HTTP=0 git ls-remote https://github.com/git/git.git remote: Sorry, fetching via dumb http is forbidden. remote: Please upgrade your git client to v1.6.6 or greater remote: and make sure that smart-http is enabled. fatal: unable to access 'https://github.com/git/git.git/': The requested URL returned error: 403 I still really hate the length of the generic http message (which you can see in the final 403 example). The text The requested URL returned error: comes from curl, though there is actually an opportunity to munge it, as you will see in the patches. However, I was unable to come up with a shorter text that sounded any better. Another option would be to just split it across lines with some indentation, like: fatal: The requested URL returned error: 403 while accessing https://github.com/git/git.git I'm open to suggestions. There are a lot of little patches, as I tried to explain the rationale for each individual change (and it makes it easy to take or reject individual patches). If we do take them all, it may make sense to just squash patches 3-5. [1/9]: http: add HTTP_KEEP_ERROR option [2/9]: remote-curl: show server content on http errors [3/9]: remote-curl: let servers override http 404 advice [4/9]: remote-curl: always show friendlier 404 message [5/9]: remote-curl: consistently report repo url for http errors [6/9]: http: simplify http_error helper function [7/9]: http: re-word http error message [8/9]: remote-curl: die directly with http error messages [9/9]: http: drop http_error function -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 1/9] http: add HTTP_KEEP_ERROR option
We currently set curl's FAILONERROR option, which means that
any http failures are reported as curl errors, and the
http body content from the server is thrown away.
This patch introduces a new option to http_get_strbuf which
specifies that the body content from a failed http response
should be placed in the destination strbuf, where it can be
accessed by the caller.
Signed-off-by: Jeff King p...@peff.net
---
http.c | 37 +
http.h | 1 +
2 files changed, 38 insertions(+)
diff --git a/http.c b/http.c
index 8803c70..45cc7c7 100644
--- a/http.c
+++ b/http.c
@@ -761,6 +761,25 @@ int handle_curl_result(struct slot_results *results)
int handle_curl_result(struct slot_results *results)
{
+ /*
+* If we see a failing http code with CURLE_OK, we have turned off
+* FAILONERROR (to keep the server's custom error response), and should
+* translate the code into failure here.
+*/
+ if (results-curl_result == CURLE_OK
+ results-http_code = 400) {
+ results-curl_result = CURLE_HTTP_RETURNED_ERROR;
+ /*
+* Normally curl will already have put the reason phrase
+* from the server into curl_errorstr; unfortunately without
+* FAILONERROR it is lost, so we can give only the numeric
+* status code.
+*/
+ snprintf(curl_errorstr, sizeof(curl_errorstr),
+The requested URL returned error: %ld,
+results-http_code);
+ }
+
if (results-curl_result == CURLE_OK) {
credential_approve(http_auth);
return HTTP_OK;
@@ -825,6 +844,8 @@ static int http_request(const char *url, struct strbuf
*type,
strbuf_addstr(buf, Pragma:);
if (options HTTP_NO_CACHE)
strbuf_addstr(buf, no-cache);
+ if (options HTTP_KEEP_ERROR)
+ curl_easy_setopt(slot-curl, CURLOPT_FAILONERROR, 0);
headers = curl_slist_append(headers, buf.buf);
@@ -862,6 +883,22 @@ static int http_request_reauth(const char *url,
int ret = http_request(url, type, result, target, options);
if (ret != HTTP_REAUTH)
return ret;
+
+ /*
+* If we are using KEEP_ERROR, the previous request may have
+* put cruft into our output stream; we should clear it out before
+* making our next request. We only know how to do this for
+* the strbuf case, but that is enough to satisfy current callers.
+*/
+ if (options HTTP_KEEP_ERROR) {
+ switch (target) {
+ case HTTP_REQUEST_STRBUF:
+ strbuf_reset(result);
+ break;
+ default:
+ die(BUG: HTTP_KEEP_ERROR is only supported with
strbufs);
+ }
+ }
return http_request(url, type, result, target, options);
}
diff --git a/http.h b/http.h
index 25d1931..0fe54f4 100644
--- a/http.h
+++ b/http.h
@@ -118,6 +118,7 @@ extern char *get_remote_object_url(const char *url, const
char *hex,
/* Options for http_request_*() */
#define HTTP_NO_CACHE 1
+#define HTTP_KEEP_ERROR2
/* Return values for http_request_*() */
#define HTTP_OK0
--
1.8.2.rc0.33.gd915649
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/9] remote-curl: show server content on http errors
If an http request to a remote git server fails, we show
only the http response code, or sometimes a custom message
for particular codes. This gives the server no opportunity
to offer a more detailed explanation of the reason for the
failure, or to give extra advice.
This patch teaches remote-curl to record and display the
body content of a failed http response. We only display such
responses when the content-type is advertised as text/plain,
as it is the most likely to look presentable on the user's
terminal (and it is hoped to be a good indication that the
message is intended for git clients, and not for a web
browser).
Each line of the new output is prepended with remote:.
Example output may look like this (assuming the server is
configured to display such a helpful message):
$ GIT_SMART_HTTP=0 git clone https://example.com/some/repo.git
Cloning into 'repo'...
remote: Sorry, fetching via dumb http is forbidden.
remote: Please upgrade your git client to v1.6.6 or greater
remote: and make sure that smart-http is enabled.
error: The requested URL returned error: 403 while accessing
http://localhost:5001/some/repo.git/info/refs
fatal: HTTP request failed
For the sake of simplicity, we only record and display these
errors during the initial fetch of the ref list, as that is
the initial contact with the server and where the most
common, interesting errors happen (and there is already
precedent, as that is the only place we currently massage
http error codes into more helpful messages).
Signed-off-by: Jeff King p...@peff.net
---
I took Jonathan's suggestion of just respecting text/plain to signify a
message that git can show on the terminal. It is the most sensible
content-type to use, I think, but I'm a little worried that
random servers may produce totally uninteresting text/plain (e.g., many
servers ship with 404 pages that just say 404 again with more useless
text). But such pages tend to be text/html, so we may be able to get
away with assuming text/plain will be useful.
remote-curl.c | 33 -
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/remote-curl.c b/remote-curl.c
index 93a09a6..4fea94f 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -151,6 +151,33 @@ static void free_discovery(struct discovery *d)
}
}
+static int show_http_message(struct strbuf *type, struct strbuf *msg)
+{
+ const char *p, *eol;
+
+ /*
+* We only show text/plain parts, as other types are likely
+* to be ugly to look at on the user's terminal.
+*
+* TODO should handle ; charset=XXX, and re-encode into
+* logoutputencoding
+*/
+ if (strcasecmp(type-buf, text/plain))
+ return -1;
+
+ strbuf_trim(msg);
+ if (!msg-len)
+ return -1;
+
+ p = msg-buf;
+ do {
+ eol = strchrnul(p, '\n');
+ fprintf(stderr, remote: %.*s\n, (int)(eol - p), p);
+ p = eol + 1;
+ } while(*eol);
+ return 0;
+}
+
static struct discovery* discover_refs(const char *service, int for_push)
{
struct strbuf exp = STRBUF_INIT;
@@ -176,16 +203,20 @@ static struct discovery* discover_refs(const char
*service, int for_push)
}
refs_url = strbuf_detach(buffer, NULL);
- http_ret = http_get_strbuf(refs_url, type, buffer, HTTP_NO_CACHE);
+ http_ret = http_get_strbuf(refs_url, type, buffer,
+ HTTP_NO_CACHE | HTTP_KEEP_ERROR);
switch (http_ret) {
case HTTP_OK:
break;
case HTTP_MISSING_TARGET:
+ show_http_message(type, buffer);
die(%s not found: did you run git update-server-info on the
server?, refs_url);
case HTTP_NOAUTH:
+ show_http_message(type, buffer);
die(Authentication failed);
default:
+ show_http_message(type, buffer);
http_error(refs_url, http_ret);
die(HTTP request failed);
}
--
1.8.2.rc0.33.gd915649
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/9] remote-curl: let servers override http 404 advice
When we get an http 404 trying to get the initial list of refs from the server, we try to be helpful and remind the user that update-server-info may need to be run. This looks like: $ git clone https://github.com/non/existent Cloning into 'existent'... fatal: https://github.com/non/existent/info/refs?service=git-upload-pack not found: did you run git update-server-info on the server? Suggesting update-server-info may be a good suggestion for users who are in control of the server repo and who are planning to set up dumb http. But for users of smart http, and especially users who are not in control of the server repo, the advice is useless and confusing. The previous patch taught remote-curl to show custom advice from the server when it is available. When we have shown messages from the server, we can also drop our custom advice; what the server has to say is likely to be more accurate and helpful. We not only drop the mention of update-server-info, but also show only the main repo URL, not the full info/refs and service parameter. These elements may be useful for debugging a broken server configuration, but again, anything the server has provided is likely to be more useful (and one can still use GIT_CURL_VERBOSE to get much more complete debugging information). Signed-off-by: Jeff King p...@peff.net --- remote-curl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/remote-curl.c b/remote-curl.c index 4fea94f..083efdf 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -209,7 +209,8 @@ static struct discovery* discover_refs(const char *service, int for_push) case HTTP_OK: break; case HTTP_MISSING_TARGET: - show_http_message(type, buffer); + if (!show_http_message(type, buffer)) + die(repository '%s' not found, url); die(%s not found: did you run git update-server-info on the server?, refs_url); case HTTP_NOAUTH: -- 1.8.2.rc0.33.gd915649 -- 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 4/9] remote-curl: always show friendlier 404 message
When we get an http 404 trying to get the initial list of refs from the server, we try to be helpful and remind the user that update-server-info may need to be run. This looks like: $ git clone https://github.com/non/existent Cloning into 'existent'... fatal: https://github.com/non/existent/info/refs?service=git-upload-pack not found: did you run git update-server-info on the server? Suggesting update-server-info may be a good suggestion for users who are in control of the server repo and who are planning to set up dumb http. But for users of smart http, and especially users who are not in control of the server repo, the advice is useless and confusing. Since most people are expected to use smart http these days, it does not make sense to keep the update-server-info hint. We not only drop the mention of update-server-info, but also show only the main repo URL, not the full info/refs and service parameter. These elements may be useful for debugging a broken server configuration, but in the majority of cases, users are not fetching from their own repositories, but rather from other people's repositories; they have neither the power nor interest to fix a broken configuration, and the extra components just make the message more confusing. Users who do want to debug can and should use GIT_CURL_VERBOSE to get more complete information on the actual URLs visited. Signed-off-by: Jeff King p...@peff.net --- This is obviously a more stringent version of the last patch, and could just replace it. I think the last one is a no-brainer, because it lets well-configured sites replace these messages. This one is less obvious, because we may be hitting some random poorly configured server that somebody is trying to set up. Still, if you think about the number of people fetching (against any server) versus the number of people configuring new servers, I think the advice to run update-server-info has a vanishingly small chance of being useful (and IMHO is misleading if you do not know how dumb-http works, as it makes you think the server is misconfigured, when it is much more likely to be a user error). remote-curl.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/remote-curl.c b/remote-curl.c index 083efdf..5d9f961 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -209,10 +209,8 @@ static struct discovery* discover_refs(const char *service, int for_push) case HTTP_OK: break; case HTTP_MISSING_TARGET: - if (!show_http_message(type, buffer)) - die(repository '%s' not found, url); - die(%s not found: did you run git update-server-info on the -server?, refs_url); + show_http_message(type, buffer); + die(repository '%s' not found, url); case HTTP_NOAUTH: show_http_message(type, buffer); die(Authentication failed); -- 1.8.2.rc0.33.gd915649 -- 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 5/9] remote-curl: consistently report repo url for http errors
When we report http errors in fetching the initial ref advertisement, we show the full URL we attempted to use, including info/refs?service=git-upload-pack. While this may be useful for debugging a broken server, it is unnecessarily verbose and confusing for most cases, in which the client user is not even the same person as the owner of the repository. Let's just show the repository URL; debugging can happen with GIT_CURL_VERBOSE, which shows way more useful information, anyway. At the same time, let's also make sure to mention the repository URL when we report failed authentication (previously we said only Authentication failed). Knowing the URL can help the user realize why authentication failed (e.g., they meant to push to remote A, not remote B). Signed-off-by: Jeff King p...@peff.net --- This is the same rationale as the latter half of the last patch; if we take them all, I'd be happy to see them squashed together. remote-curl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/remote-curl.c b/remote-curl.c index 5d9f961..6c6714b 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -213,10 +213,10 @@ static struct discovery* discover_refs(const char *service, int for_push) die(repository '%s' not found, url); case HTTP_NOAUTH: show_http_message(type, buffer); - die(Authentication failed); + die(Authentication failed for '%s', url); default: show_http_message(type, buffer); - http_error(refs_url, http_ret); + http_error(url, http_ret); die(HTTP request failed); } -- 1.8.2.rc0.33.gd915649 -- 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 6/9] http: simplify http_error helper function
This helper function should really be a one-liner that
prints an error message, but it has ended up unnecessarily
complicated:
1. We call error() directly when we fail to start the curl
request, so we must later avoid printing a duplicate
error in http_error().
It would be much simpler in this case to just stuff the
error message into our usual curl_errorstr buffer
rather than printing it ourselves. This means that
http_error does not even have to care about curl's exit
value (the interesting part is in the errorstr buffer
already).
2. We return the ret value passed in to us, but none of
the callers actually cares about our return value. We
can just drop this entirely.
Signed-off-by: Jeff King p...@peff.net
---
http-push.c | 2 +-
http.c| 11 ---
http.h| 5 ++---
remote-curl.c | 2 +-
4 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/http-push.c b/http-push.c
index bd66f6a..439a555 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1551,7 +1551,7 @@ static int remote_exists(const char *path)
ret = 0;
break;
case HTTP_ERROR:
- http_error(url, HTTP_ERROR);
+ http_error(url);
default:
ret = -1;
}
diff --git a/http.c b/http.c
index 45cc7c7..5e6f67d 100644
--- a/http.c
+++ b/http.c
@@ -857,7 +857,8 @@ static int http_request(const char *url, struct strbuf
*type,
run_active_slot(slot);
ret = handle_curl_result(results);
} else {
- error(Unable to start HTTP request for %s, url);
+ snprintf(curl_errorstr, sizeof(curl_errorstr),
+failed to start HTTP request);
ret = HTTP_START_FAILED;
}
@@ -940,13 +941,9 @@ int http_error(const char *url, int ret)
return ret;
}
-int http_error(const char *url, int ret)
+void http_error(const char *url)
{
- /* http_request has already handled HTTP_START_FAILED. */
- if (ret != HTTP_START_FAILED)
- error(%s while accessing %s, curl_errorstr, url);
-
- return ret;
+ error(%s while accessing %s, curl_errorstr, url);
}
int http_fetch_ref(const char *base, struct ref *ref)
diff --git a/http.h b/http.h
index 0fe54f4..fa65128 100644
--- a/http.h
+++ b/http.h
@@ -136,10 +136,9 @@ int http_get_strbuf(const char *url, struct strbuf
*content_type, struct strbuf
int http_get_strbuf(const char *url, struct strbuf *content_type, struct
strbuf *result, int options);
/*
- * Prints an error message using error() containing url and curl_errorstr,
- * and returns ret.
+ * Prints an error message using error() containing url and curl_errorstr.
*/
-int http_error(const char *url, int ret);
+void http_error(const char *url);
extern int http_fetch_ref(const char *base, struct ref *ref);
diff --git a/remote-curl.c b/remote-curl.c
index 6c6714b..9abe4b7 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -216,7 +216,7 @@ static struct discovery* discover_refs(const char *service,
int for_push)
die(Authentication failed for '%s', url);
default:
show_http_message(type, buffer);
- http_error(url, http_ret);
+ http_error(url);
die(HTTP request failed);
}
--
1.8.2.rc0.33.gd915649
--
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 7/9] http: re-word http error message
When we report an http error code, we say something like:
error: The requested URL reported failure: 403 Forbidden while accessing
http://example.com/repo.git
Everything between error: and while is written by curl,
and the resulting sentence is hard to read (especially
because there is no punctuation between curl's sentence and
the remainder of ours). Instead, let's re-order this to give
better flow:
error: unable to access 'http://example.com/repo.git: The requested URL
reported failure: 403 Forbidden
This is still annoyingly long, but at least reads more
clearly left to right.
Signed-off-by: Jeff King p...@peff.net
---
As I mentioned in the cover letter, I'm still not that excited about the
after result here. Suggestions welcome.
http.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/http.c b/http.c
index 5e6f67d..64068a2 100644
--- a/http.c
+++ b/http.c
@@ -943,7 +943,7 @@ void http_error(const char *url)
void http_error(const char *url)
{
- error(%s while accessing %s, curl_errorstr, url);
+ error(unable to access '%s': %s, url, curl_errorstr);
}
int http_fetch_ref(const char *base, struct ref *ref)
--
1.8.2.rc0.33.gd915649
--
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 8/9] remote-curl: die directly with http error messages
When we encounter an unknown http error (e.g., a 403), we hand the error code to http_error, which then prints it with error(). After that we die with the redundant message HTTP request failed. Instead, let's just drop http_error entirely, which does nothing but pass arguments to error(), and instead die directly with a useful message. So before: $ git clone https://example.com/repo.git Cloning into 'repo'... error: unable to access 'https://example.com/repo.git': The requested URL returned error: 403 Forbidden fatal: HTTP request failed and after: $ git clone https://example.com/repo.git Cloning into 'repo'... fatal: unable to access 'https://example.com/repo.git': The requested URL returned error: 403 Forbidden Signed-off-by: Jeff King p...@peff.net --- remote-curl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/remote-curl.c b/remote-curl.c index 9abe4b7..60eda63 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -216,8 +216,7 @@ static struct discovery* discover_refs(const char *service, int for_push) die(Authentication failed for '%s', url); default: show_http_message(type, buffer); - http_error(url); - die(HTTP request failed); + die(unable to access '%s': %s, url, curl_errorstr); } last= xcalloc(1, sizeof(*last_discovery)); -- 1.8.2.rc0.33.gd915649 -- 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 9/9] http: drop http_error function
This function is a single-liner and is only called from one
place. Just inline it, which makes the code more obvious.
Signed-off-by: Jeff King p...@peff.net
---
This is a cleanup made possible by the previous patch.
http-push.c | 2 +-
http.c | 5 -
http.h | 5 -
3 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/http-push.c b/http-push.c
index 439a555..395a8cf 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1551,7 +1551,7 @@ static int remote_exists(const char *path)
ret = 0;
break;
case HTTP_ERROR:
- http_error(url);
+ error(unable to access '%s': %s, url, curl_errorstr);
default:
ret = -1;
}
diff --git a/http.c b/http.c
index 64068a2..58c063c 100644
--- a/http.c
+++ b/http.c
@@ -941,11 +941,6 @@ cleanup:
return ret;
}
-void http_error(const char *url)
-{
- error(unable to access '%s': %s, url, curl_errorstr);
-}
-
int http_fetch_ref(const char *base, struct ref *ref)
{
char *url;
diff --git a/http.h b/http.h
index fa65128..4dc5353 100644
--- a/http.h
+++ b/http.h
@@ -135,11 +135,6 @@ int http_get_strbuf(const char *url, struct strbuf
*content_type, struct strbuf
*/
int http_get_strbuf(const char *url, struct strbuf *content_type, struct
strbuf *result, int options);
-/*
- * Prints an error message using error() containing url and curl_errorstr.
- */
-void http_error(const char *url);
-
extern int http_fetch_ref(const char *base, struct ref *ref);
/* Helpers for fetching packs */
--
1.8.2.rc0.33.gd915649
--
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 (Apr 2013, #02; Fri, 5)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
A handful of topics that have been stalled for quite a while have
been discarded; for those that are not superseded by something else,
interested parties can still resubmit a reroll, but without any
advances, we do not get any benefit from carrying them in my tree.
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]
* bk/document-commit-tree-S (2013-03-25) 1 commit
(merged to 'next' on 2013-03-26 at 8ee205f)
+ commit-tree: document -S option consistently
* jc/apply-ws-fix-tab-in-indent (2013-03-29) 2 commits
(merged to 'next' on 2013-03-29 at 26eb6e9)
+ test: resurrect q_to_tab
(merged to 'next' on 2013-03-26 at 46c6bda)
+ apply --whitespace=fix: avoid running over the postimage buffer
git apply --whitespace=fix was not prepared to see a line getting
longer after fixing whitespaces (e.g. tab-in-indent aka Python).
* jc/directory-attrs-regression-fix (2013-03-28) 6 commits
(merged to 'next' on 2013-03-29 at a3dce2b)
+ t: check that a pattern without trailing slash matches a directory
+ dir.c::match_pathname(): pay attention to the length of string parameters
+ dir.c::match_pathname(): adjust patternlen when shifting pattern
+ dir.c::match_basename(): pay attention to the length of string parameters
+ attr.c::path_matches(): special case paths that end with a slash
+ attr.c::path_matches(): the basename is part of the pathname
Fix 1.8.1.x regression that stopped matching dir (without
trailing slash) to a directory dir.
* jc/merge-tag-object (2013-04-01) 3 commits
(merged to 'next' on 2013-04-03 at 94b5c7d)
+ t6200: test message for merging of an annotated tag
+ t6200: use test_config/test_unconfig
(merged to 'next' on 2013-03-29 at aeec39c)
+ merge: a random object may not necssarily be a commit
git merge $(git rev-parse v1.8.2) behaved quite differently from
git merge v1.8.2 as if v1.8.2 were written as v1.8.2^0 and did
not pay much attention to the annotated tag payload.
This makes the code notice the type of the tag object, in addition
to the dwim_ref() based classification the current code uses
(i.e. the name appears in refs/tags/) to decide when to special
case merging of tags.
* jc/sha1-name-object-peeler (2013-03-31) 2 commits
(merged to 'next' on 2013-04-01 at cdb4a18)
+ peel_onion(): teach $foo^{object} peeler
+ peel_onion: disambiguate to favor tree-ish when we know we want a tree-ish
(this branch is used by mh/rev-parse-verify-doc.)
There was no good way to ask I have a random string that came from
outside world. I want to turn it into a 40-hex object name while
making sure such an object exists. A new peeling suffix ^{object}
can be used for that purpose, together with rev-parse --verify.
* jc/t5516-pushInsteadOf-vs-pushURL (2013-03-28) 1 commit
(merged to 'next' on 2013-04-01 at bed2879)
+ t5516: test interaction between pushURL and pushInsteadOf correctly
Update a test to match the documented interaction between pushURL
and pushInsteadOf.
* jk/check-corrupt-objects-carefully (2013-03-29) 10 commits
(merged to 'next' on 2013-03-29 at b6a04a7)
+ clone: leave repo in place after checkout errors
+ clone: run check_everything_connected
+ clone: die on errors from unpack_trees
+ add tests for cloning corrupted repositories
+ streaming_write_entry: propagate streaming errors
+ add test for streaming corrupt blobs
+ avoid infinite loop in read_istream_loose
+ read_istream_filtered: propagate read error from upstream
+ check_sha1_signature: check return value from read_istream
+ stream_blob_to_fd: detect errors reading from stream
Have the streaming interface and other codepaths more carefully
examine for corrupt objects.
* jk/config-with-empty-section (2013-03-29) 1 commit
(merged to 'next' on 2013-04-01 at 7972aa9)
+ t1300: document some aesthetic failures of the config editor
Document that git config --unset does not remove an empty section
head after removing the last variable in a section, and adding a
new variable does not try to reuse a leftover empty section head.
* jk/difftool-no-overwrite-on-copyback (2013-03-29) 5 commits
(merged to 'next' on 2013-03-29 at 9f42d34)
+ t7800: run --dir-diff tests with and without symlinks
+ t7800: fix tests when difftool uses --no-symlinks
+ t7800: don't hide grep output
+ difftool: don't overwrite modified files
+ t7800: move '--symlinks' specific test to the end
Try to be careful when difftool backend allows the user to write
into the temporary files being shown *and* the user makes changes
to the working tree at the same time. One of the changes has to be
lost in such a case, but at least tell the user what he did.
Re: [RFC/PATH 3/4] remote-hg: add version checks to the marks
On Fri, Apr 5, 2013 at 6:46 AM, Jed Brown j...@59a2.org wrote: Felipe Contreras felipe.contre...@gmail.com writes: @@ -76,12 +78,19 @@ class Marks: def __init__(self, path): self.path = path +self.clear() +self.load() + +if self.version VERSION: +self.clear() It's friendlier to just upgrade the marks in-place. This takes less than one second to run on repositories where full re-import would take half an hour: Yeah, but that's riskier, and only works for this particular version. Besides, it only happens one time per repository at best. Also, it would mess up the current organization of the code. I'm not sure it's worth worrying about it at this point, and we could always add a patch on top. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/13] remote-hg: general updates
RANT
While I am not really interested in exchanging any further emails or any other
form of communication with Felipe, as I find his vitriolic style of
communication unbearable, I feel compelled to reply to a few points. I'll
probably regret this... anyway, I promise this will be my last mail in this
thread. Even though I fully expect to receive a barrage of hostile and
aggressive replies by Felipe. So be it, /dev/null has plenty space left.
/RANT
OK, I'll try to keep a professional tone from now on :-).
On 04.04.2013, at 08:42, Felipe Contreras wrote:
[...]
* The gitifyhg test suite is run after each push on Travis CI against
several git / mercurial combinations [4].
In particular, unlike all other remote-hg implementations I know, we
explicitly promise (and test) compatibility with a specific range of
Mercurial versions (not just the one the dev happens to have installed
right now). This has been a frequent issue for me with the msysgit
remote-hg
I've personally checked against multiple versions of Mercurial. It's
possible that some error might slip by, but it would get quickly
noticed.
Really? This sounds close to some people who say things like I don't need a
test suite, I personally run some tests every now and then on my machine.
Do you see any compatibility issues reported in the git mailing list,
or my github[1]? No? KTHXBYE. There _were_ compatibility issues, and
those got reported, and fixed, not any more.
Please consider that the willingness of people to collaborate with you in any
way is directly related to how you treat them. That includes bug reports. The
way you acted towards Jed, who was very calmly and matter-of-factly explaining
things, was IMHO completely inappropriate and unacceptable. Indeed, I should
augment my list of reasons why people might not want to contribute to remote-hg
by one major bullet point: You. And please, don't feel to compelled to tell us
that Junio is really the maintainer of remote-hg and not you: Whether this is
true or not doesn't matter for this point.
remote-hg certainly works on versions older than 1.9, again
That's plain wrong. Indeed, remote-hg is using hg interfaces that were only
introduced in 1.9. As such, I would be quite surprised if remote-hg worked with
older hg versions, but I don't see why I should bother to test... Hmm, wait, I
see a reason:
, I find it
annoying that you claim to know what is important for users, as if
somehow knowing that gitifyhg doesn't work with the user's version of
mercurial (e.g. 1.8) is better than remote-hg's situation; where it
*actually works*, but it's not mentioned. Yeah, mentioning that it
doesn't work is better than working, right.
I'll leave it to everybody to read what you wrote there, and contrast it with
the following, and draw their own conclusions...
The reason why I did not write what exactly is wrong with remote-hg in hg 1.8
and older is that it is so obvious that I didn't think anybody would need
handholding to verify it or find out the details :-). But since you asked for
it:
$ hg --version
Mercurial Distributed SCM (version 1.8.4)
(see http://mercurial.selenic.com for more information)
Copyright (C) 2005-2011 Matt Mackall and others
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ git clone hg::foobar/ foobar.git
Cloning into 'foobar.git'...
Traceback (most recent call last):
File /Users/mhorn/bin/git-remote-hg, line 846, in module
sys.exit(main(sys.argv))
File /Users/mhorn/bin/git-remote-hg, line 819, in main
fix_path(alias, peer or repo, url)
File /Users/mhorn/bin/git-remote-hg, line 765, in fix_path
repo_url = util.url(repo.url())
AttributeError: 'module' object has no attribute 'url'
Indeed, util.url was only added in 1.9.3. OK, let's work around that. Then
local clones work. But of course in reality, most users will want to interact
with a remote repository. But that's still broken:
$ git clone hg::ssh://h...@bitbucket.org/fingolfin/test-gitifyhg
Cloning into 'test-gitifyhg'...
Traceback (most recent call last):
File /Users/mhorn/bin/git-remote-hg, line 1138, in module
sys.exit(main(sys.argv))
File /Users/mhorn/bin/git-remote-hg, line 1107, in main
repo = get_repo(url, alias)
File /Users/mhorn/bin/git-remote-hg, line 284, in get_repo
peer, dstpeer = hg.clone(myui, {}, url, local_path, update=False, pull=True)
TypeError: clone() got multiple values for keyword argument 'pull'
Right, clone() changed. And some more stuff. See
https://github.com/fingolfin/gitifyhg/commit/d3d37a7a853f3c8a1907bdaf933844128d5e7d81.
There also was a good reason why I stopped at that point, but I don't remember
the details right now. And quite frankly, I have zero incentive to even try to
remember. But anyway, I don't think it's that useful to add support for 1.8,
too, since one can't get back much further
Re: [PATCH] glossary: extend detached HEAD description
On Fri, Apr 5, 2013 at 11:19 AM, Junio C Hamano gits...@pobox.com wrote: Add a blanket description to the glossary to cover them instead. The general principle is that operations to update the branch work and affect on the HEAD, while operations to update the information s/work and affect on the/work on and affect the/ about a branch do not. Signed-off-by: Junio C Hamano gits...@pobox.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] show-branch: use strbuf instead of static buffer
Jeff King wrote: When we generate relative names (e.g., master~20^2), we format the name into a static buffer, then xstrdup the result to attach it to the commit. Since the first thing we add into the static buffer is the already-computed name of the child commit, the names may get longer and longer as the traversal gets deeper, and we may eventually overflow the fixed-size buffer. Good catch. [...] Though this is a stack overflow, I don't know that it's exploitable for anything interesting; an attacker does not get to write arbitrary data, but rather only a sequence of ^%d and ~%d relative history markers. Perhaps in theory one could devise a history such that the sequence markers spelled out some malicious code, but it would be quite a challenge Overwrite the return address and return-to-libc? [...] --- a/builtin/show-branch.c +++ b/builtin/show-branch.c Very clean and obviously correct. Thanks. Reviewed-by: Jonathan Nieder jrnie...@gmail.com A test would be nice, though. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/13] remote-hg: general updates
On Fri, Apr 5, 2013 at 4:30 PM, Max Horn m...@quendi.de wrote: On 04.04.2013, at 08:42, Felipe Contreras wrote: Please consider that the willingness of people to collaborate with you in any way is directly related to how you treat them. That includes bug reports. The way you acted towards Jed, who was very calmly and matter-of-factly explaining things, was IMHO completely inappropriate and unacceptable He did not provide any bug report at all, also, he was not willing to collaborate, as he clearly stated; he won't work git-hg bridges no more. Indeed, I should augment my list of reasons why people might not want to contribute to remote-hg by one major bullet point: You. And please, don't feel to compelled to tell us that Junio is really the maintainer of remote-hg and not you: Whether this is true or not doesn't matter for this point. Oh but it does. The only reason my remote-hg managed to land in the mainline is because I showed it was superior to the other remote-hg[1], and that there really was no point in trying to salvage the other code base. Maybe this turns out to be a mistake, and the other remote-hg manages to improve and surpass my remote-hg, but it seems rather unlikely at this point. Similarly, you could *show* that gitifyhg is superior than remote-hg, why it makes sense to use that instead of trying to improve this code base, but you don't, because you can't, because it doesn't make sense. Ultimately this is not about people, this is about the code. A sensible person that is not emotionally attached to any code, would simply look at the code, and if what you claim is true (that remote-hg has many issues), this sensible person would start cherry picking patches from gitifyhg, send them to the git mailing list explaining why they make sense. Eventually, remote-hg in git.git would become essentially gitifyhg (albeit better because it would have received more review from other git developers). Codewise this would make sense. And this sensible person would replace me as the go-to person for remote-hg. And he would probably have a friendly and wonderful personality, and you can accept him as your messiah and what not. But this wouldn't change the fact that codewise this is the best way to proceed. But the fact of the matter is that either a) remote-hg is perfectly fine, or b) this sensible person doesn't exist. , I find it annoying that you claim to know what is important for users, as if somehow knowing that gitifyhg doesn't work with the user's version of mercurial (e.g. 1.8) is better than remote-hg's situation; where it *actually works*, but it's not mentioned. Yeah, mentioning that it doesn't work is better than working, right. I'll leave it to everybody to read what you wrote there, and contrast it with the following, and draw their own conclusions... Keep in mind that it's not me the one that claims remote-hg is superior because it supports 1.8, as you seem try to portray. Rather, it's _you_ the one that claims superiority because gitifyhg *mentions* support until 1.9 (which remote-hg also supports). It's not important who supports the most ancient version of mercurial that nobody uses anyway, what is important is that *mentioning* or not mentioning which ancient version of mercurial is supported doesn't make one project superior to the other. But perhaps this point is too subtle for you to understand, so there: https://github.com/felipec/git/wiki/Git-remote-hg Now it's mentioned that remote-hg too, supports up to version 1.9. Can we agree now, that nobody has any advantage, either on version support, or the mentioning of version support? Indeed, util.url was only added in 1.9.3. OK, let's work around that. Then local clones work. But of course in reality, most users will want to interact with a remote repository. But that's still broken: Fixed: https://github.com/felipec/git/commit/df0ed732b56c6c7910cc76f3e930229816e27117 And it was _you_ the one that sent the commit that broke it to the git mailing list to be pushed. Right, clone() changed. And some more stuff. See https://github.com/fingolfin/gitifyhg/commit/d3d37a7a853f3c8a1907bdaf933844128d5e7d81. Indeed, that might be useful, but that doesn't mean remote-hg doesn't work with 1.8; it works perfectly fine for local repositories, and all the tests pass. There also was a good reason why I stopped at that point, but I don't remember the details right now. And quite frankly, I have zero incentive to even try to remember. But anyway, I don't think it's that useful to add support for 1.8, too, since one can't get back much further anyway. And upgrading to a newer Mercurial is (a) quite easy (even if you don't have root, just install it into $HOME), and (b) using a newer Mercurial version is a good idea for other reasons, too. If supporting 1.8 is not useful because most people have newer versions, then mentioning support for 1.9 doesn't give any advantage. Thanks for wasting our
Re: [PATCH] http-backend: respect GIT_NAMESPACE with dumb clients
On Thu, Apr 4, 2013 at 10:43 PM, Jeff King p...@peff.net wrote:
On Thu, Apr 04, 2013 at 10:34:49PM -0700, Junio C Hamano wrote:
+static void get_head(char *arg)
+{
+ struct strbuf buf = STRBUF_INIT;
+ head_ref_namespaced(show_text_ref, buf);
+ send_strbuf(text/plain, buf);
+ strbuf_release(buf);
+}
You identified the right place to patch, but I think we need a bit
more than this.
The show_text_ref() function gives SHA-1 TAB refname. It is
likely that the dumb client will ignore the trailing part of that
output, but let's avoid a hack that we would not want see other
implementations imitate.
Oh, right. I was thinking too much about normal clients which see HEAD
in the ref advertisement; of course the dumb client is expecting to see
the actual HEAD file.
One advantage dumb clients has over smart ones is that they can read
HEAD that is a textual symref from a dumb server and learn which
branch is the default one (remote.c::guess_remote_head()) without
guessing. I think this function should:
- Turn HEAD into a namespaced equivalent;
- Run resolve_ref() on the result of the above;
- Is it a symbolic ref?
. If it is, then format ref: target\n into a strbuf and send
it (make sure target is without the namespace prefix);
. Otherwise, HEAD is detached. Prepare %s\n % sha1_to_hex(sha1),
and send it.
Yes, that sounds right; it is basically just reconstructing a HEAD
file. What do the HEADs inside namespaces look like? Do they refer to
full global refs, or do they refer to refs within the namespace?
If the latter, we could just send the HEAD file directly. But I suspect
it is the former, so that they can function when non-namespaced commands
are used.
Here's a quick cut at this. Seems to work ok in local testing, I
haven't updated the test suite yet. If the namespaced HEAD is a
symbolic ref, its target must have the namespace prefix applied, or
the resolved ref will be from outside the namespace (eg
refs/heads/master vs refs/namespace/ns/refs/heads/master). This seems
to be handled at write time, not sure if we need to do more
verification here or not.
diff --git a/http-backend.c b/http-backend.c
index d32128f..da4482c 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -404,13 +404,40 @@ static void get_info_refs(char *arg)
} else {
select_getanyfile();
- head_ref_namespaced(show_text_ref, buf);
for_each_namespaced_ref(show_text_ref, buf);
send_strbuf(text/plain, buf);
}
strbuf_release(buf);
}
+static int show_head_ref(const char *name, const unsigned char *sha1,
+ int flag, void *cb_data)
+{
+ struct strbuf *buf = cb_data;
+
+ if (flag REF_ISSYMREF) {
+ unsigned char sha1[20];
+ const char *target = resolve_ref_unsafe(name, sha1, 1, NULL);
+ const char *target_nons = strip_namespace(target);
+
+ strbuf_addf(buf, ref: %s\n, target_nons);
+ } else {
+ strbuf_addf(buf, %s\n, sha1_to_hex(sha1));
+ }
+
+ return 0;
+}
+
+static void get_head(char *arg)
+{
+ struct strbuf buf = STRBUF_INIT;
+
+ select_getanyfile();
+ head_ref_namespaced(show_head_ref, buf);
+ send_strbuf(text/plain, buf);
+ strbuf_release(buf);
+}
+
static void get_info_packs(char *arg)
{
size_t objdirlen = strlen(get_object_directory());
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/13] remote-hg: general updates
Max Horn m...@quendi.de writes: OK, I'll try to keep a professional tone from now on :-). Please consider that the willingness of people to collaborate with you in any way is directly related to how you treat them. That includes bug reports. The way you acted towards Jed, who was very calmly and matter-of-factly explaining things, was IMHO completely inappropriate and unacceptable. Indeed, I should augment my list of reasons why people might not want to contribute to remote-hg by one major bullet point: You. And please, don't feel to compelled to tell us that Junio is really the maintainer of remote-hg and not you: Whether this is true or not doesn't matter for this point. Only on this point, as the top-level maintainer. I do not have any opinion on technical merits between the two Hg gateways myself. A tool that is in contrib/ follows the contrib/README rule. I do not maintain it. Maintenance is up to the person who asked to include it there. I do ask the people who propose to add something in contrib/ to promise that they arrange it to be maintained. I do not even guarantee that they are the best in the breed in their respective category. When something is added to contrib/, others can raise objections by proposing alternatives, by arguing that tools of the nature are better kept out of my tree, etc. When remote-hg was added, I didn't see specific objections against it. There is one generic objection to adding anything new in contrib/ I have myself, though. In early days of Git, almost all users, who might be interested in improving their Git experience by helping to polish third-party tools, had clones of my tree and did not hesitate to come to this list. Back then, having a copy of an emerging third-party tool in my tree in contrib/ was a good way to give more exposure to it, and to give those interested in it a place to meet and join forces to improve it. Because Git population was small, almost everybody was here, and it was an efficient distribution mechanism. Git is now reasonably well known and has big enough user base, and many users, even those who are inclined to help improving their Git experience by contributing to third-party tools, do not necessarily have a clone of my tree. A third-party tool around Git, if it is any good, is likely to have much much better chance to thrive as a free-standing project with its own community, compared to those early days. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/13] remote-hg: general updates
On Fri, Apr 5, 2013 at 7:28 PM, Junio C Hamano gits...@pobox.com wrote: A tool that is in contrib/ follows the contrib/README rule. I do not maintain it. Maintenance is up to the person who asked to include it there. I do ask the people who propose to add something in contrib/ to promise that they arrange it to be maintained. That's true, but I meant that you are the gatekeeper. Ultimately you decide which patches go in. If Max, or anybody else, wants a patch into contrib, you can get it in, even if I disagree with it. You can also decide that somebody from gitifyhg might be a more suitable person as a maintainer of remote-hg. Either way, I think if things go well, remote-hg will prove it's worth and move out of contrib and into git's core. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] remote-bzr: generic updates
Hi, Here are a couple fixes for remote-bzr, some of these can be really annoying to certain users. I believe all of them should be safe. Christophe Simonis (2): remote-bzr: fix directory renaming remote-bzr: remove files before modifications David Engster (1): remote-bzr: set author if available Felipe Contreras (3): remote-bzr: only update workingtree on local repos remote-bzr: avoid unreferred tags remote-bzr: add utf-8 support for pushing Timotheus Pokorra (1): remote-bzr: add utf-8 support for fetching contrib/remote-helpers/git-remote-bzr | 36 ++--- contrib/remote-helpers/test-bzr.sh| 95 +++ 2 files changed, 123 insertions(+), 8 deletions(-) -- 1.8.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
[PATCH 2/7] remote-bzr: remove files before modifications
From: Christophe Simonis christo...@kn.gl Allow re-add of a deleted file in the same commit. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-bzr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index a7d041b..f818e93 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -303,10 +303,10 @@ def export_branch(branch, name): else: print merge :%s % m +for f in removed: +print D %s % (f,) for f in modified_final: print M %s :%u %s % f -for f in removed: -print D %s % (f) print count += 1 -- 1.8.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
[PATCH 3/7] remote-bzr: set author if available
From: David Engster d...@randomsample.de
[fc: added tests]
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-bzr | 7 ++-
contrib/remote-helpers/test-bzr.sh| 15 +++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index f818e93..a99a924 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -266,7 +266,12 @@ def export_branch(branch, name):
tz = rev.timezone
committer = rev.committer.encode('utf-8')
committer = %s %u %s % (fixup_user(committer), time, gittz(tz))
-author = committer
+authors = rev.get_apparent_authors()
+if authors:
+author = authors[0].encode('utf-8')
+author = %s %u %s % (fixup_user(author), time, gittz(tz))
+else:
+author = committer
msg = rev.message.encode('utf-8')
msg += '\n'
diff --git a/contrib/remote-helpers/test-bzr.sh
b/contrib/remote-helpers/test-bzr.sh
index d26e5c7..68105fc 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -150,4 +150,19 @@ test_expect_success 'moving directory' '
test_cmp expected actual
'
+test_expect_success 'different authors' '
+ (cd bzrrepo
+ echo john content
+ bzr commit -m john \
+--author Jane Rey j...@example.com \
+--author John Doe j...@example.com)
+
+ (cd gitrepo
+ git pull
+ git show --format=%an %ae, %cn %ce --quiet ../actual)
+
+ echo Jane Rey j...@example.com, A U Thor aut...@example.com expected
+ test_cmp expected actual
+'
+
test_done
--
1.8.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
[PATCH 4/7] remote-bzr: only update workingtree on local repos
Apparently, that's the only way it's possible. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-bzr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index a99a924..9466cb9 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -630,10 +630,9 @@ def do_export(parser): peer.import_last_revision_info_and_tags(repo, revno, revid) else: peer.import_last_revision_info(repo.repository, revno, revid) -wt = peer.bzrdir.open_workingtree() else: wt = repo.bzrdir.open_workingtree() -wt.update() +wt.update() print ok %s % ref print -- 1.8.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
[PATCH 5/7] remote-bzr: avoid unreferred tags
They have no content, there's nothing we can do with them. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-bzr | 4 1 file changed, 4 insertions(+) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index 9466cb9..0bcf8c5 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -654,7 +654,11 @@ def do_capabilities(parser): def do_list(parser): global tags print ? refs/heads/%s % 'master' + +history = parser.repo.revision_history() for tag, revid in parser.repo.tags.get_tag_dict().items(): +if revid not in history: +continue print ? refs/tags/%s % tag tags[tag] = revid print @refs/heads/%s HEAD % 'master' -- 1.8.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
[PATCH 6/7] remote-bzr: add utf-8 support for fetching
From: Timotheus Pokorra timotheus.poko...@gmail.com
[fc: added tests]
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-bzr | 4 ++--
contrib/remote-helpers/test-bzr.sh| 25 +
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index 0bcf8c5..0bd0759 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -223,7 +223,7 @@ def export_files(tree, files):
# is the blog already exported?
if h in filenodes:
mark = filenodes[h]
-final.append((mode, mark, path))
+final.append((mode, mark, path.encode('utf-8')))
continue
d = tree.get_file_text(fid)
@@ -240,7 +240,7 @@ def export_files(tree, files):
print data %d % len(d)
print d
-final.append((mode, mark, path))
+final.append((mode, mark, path.encode('utf-8')))
return final
diff --git a/contrib/remote-helpers/test-bzr.sh
b/contrib/remote-helpers/test-bzr.sh
index 68105fc..e468079 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -165,4 +165,29 @@ test_expect_success 'different authors' '
test_cmp expected actual
'
+test_expect_success 'fetch utf-8 filenames' '
+ mkdir -p tmp cd tmp
+ test_when_finished cd .. rm -rf tmp LC_ALL=C
+
+ export LC_ALL=en_US.UTF-8
+
+ (
+ bzr init bzrrepo
+ cd bzrrepo
+
+ echo test áéíóú
+ bzr add áéíóú
+ bzr commit -m utf-8
+ )
+
+ (
+ git clone bzr::$PWD/bzrrepo gitrepo
+ cd gitrepo
+ git ls-files ../actual
+ )
+
+ echo \\\303\\241\\303\\251\\303\\255\\303\\263\\303\\272\ expected
+ test_cmp expected actual
+'
+
test_done
--
1.8.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
[PATCH 7/7] remote-bzr: add utf-8 support for pushing
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-bzr | 6 ++
contrib/remote-helpers/test-bzr.sh| 31 +++
2 files changed, 37 insertions(+)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index 0bd0759..fad4a48 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -512,6 +512,11 @@ class CustomTree():
def get_symlink_target(self, file_id):
return self.updates[file_id]['data']
+def c_style_unescape(string):
+if string[0] == string[-1] == '':
+return string.decode('string-escape')[1:-1]
+return string
+
def parse_commit(parser):
global marks, blob_marks, bmarks, parsed_refs
global mode
@@ -551,6 +556,7 @@ def parse_commit(parser):
f = { 'deleted' : True }
else:
die('Unknown file command: %s' % line)
+path = c_style_unescape(path).decode('utf-8')
files[path] = f
repo = parser.repo
diff --git a/contrib/remote-helpers/test-bzr.sh
b/contrib/remote-helpers/test-bzr.sh
index e468079..f4c7768 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -190,4 +190,35 @@ test_expect_success 'fetch utf-8 filenames' '
test_cmp expected actual
'
+test_expect_success 'push utf-8 filenames' '
+ mkdir -p tmp cd tmp
+ test_when_finished cd .. rm -rf tmp LC_ALL=C
+
+ export LC_ALL=en_US.UTF-8
+
+ (
+ bzr init bzrrepo
+ cd bzrrepo
+
+ echo one content
+ bzr add content
+ bzr commit -m one
+ )
+
+ (
+ git clone bzr::$PWD/bzrrepo gitrepo
+ cd gitrepo
+
+ echo test áéíóú
+ git add áéíóú
+ git commit -m utf-8
+
+ git push
+ )
+
+ (cd bzrrepo bzr ls ../actual)
+ echo -e content\náéíóú expected
+ test_cmp expected actual
+'
+
test_done
--
1.8.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
Fwd: Delivery Status Notification (Failure)
Hello, I'm am trying to patch git to refuse to commit if there are both staged and unstaged changes, and I pass the -a flag. I expected this simple addition to parse_and_validate_options() in commit.c to accomplish most of what I want: if (all s-workdir_dirty) die(_(Cannot commit with -a if there are staged and unstaged changes)); But when compiled, this will commit anyway even with staged and unstaged files. I think I misunderstanding the workdir_dirty flag. Can someone help me? Drew Gross -- 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: Fwd: Delivery Status Notification (Failure)
On Sat, Apr 06, 2013 at 12:15:33AM -0400, Drew Gross wrote: I'm am trying to patch git to refuse to commit if there are both staged and unstaged changes, and I pass the -a flag. I expected this simple addition to parse_and_validate_options() in commit.c to accomplish most of what I want: if (all s-workdir_dirty) die(_(Cannot commit with -a if there are staged and unstaged changes)); But when compiled, this will commit anyway even with staged and unstaged files. I think I misunderstanding the workdir_dirty flag. Can someone help me? I am not sure if that is a good safety measure in general. But ignoring that question for a moment, there are a few issues with your proposed implementation: 1. workdir_dirty only talks about unstaged changes; it sounds like you want to check for both staged and unstaged. 2. no flags in in the wt_status struct are set until wt_status_collect is called; you need to put your check later. But of course by the time we call it, we have already updated the index, so you would not know anymore which changes were already in the index and which were added by -a. 3. we do not always run wt_collect_status (e.g., if you are not going to run an editor, we do not bother going to the effort to create the commit template). So you'd probably need something more like this: diff --git a/builtin/commit.c b/builtin/commit.c index 4620437..ebb5480 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1061,6 +1061,10 @@ static int parse_and_validate_options(int argc, const char *argv[], if (status_format != STATUS_FORMAT_NONE) dry_run = 1; + wt_status_collect(s); + if (all s-change.nr s-workdir_dirty) + die(Cannot use '-a' with staged and unstaged changes); + return argc; } Note that this may run the diff-index and diff-files procedures twice (once here, and once later if we actually call run_status). If were doing this for real (and I do not think it is something we want to take upstream anyway), you would want to make sure that information was cached. -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] show-branch: use strbuf instead of static buffer
On Fri, Apr 05, 2013 at 04:49:15PM -0700, Jonathan Nieder wrote: Though this is a stack overflow, I don't know that it's exploitable for anything interesting; an attacker does not get to write arbitrary data, but rather only a sequence of ^%d and ~%d relative history markers. Perhaps in theory one could devise a history such that the sequence markers spelled out some malicious code, but it would be quite a challenge Overwrite the return address and return-to-libc? Still hard, since you need to construct a usable address (and arguments) out of sequences of ^[0-9]+ and ~[0-9]+. But I'd love to see a working exploit if somebody thinks they can do it. :) Very clean and obviously correct. Thanks. Reviewed-by: Jonathan Nieder jrnie...@gmail.com Thanks. A test would be nice, though. What should it be testing? That a giant chain of second-parent merges that exceeds 1000 bytes doesn't segfault? Tests like that are not all that interesting, because they do not catch real-world regressions. We have closed this barn door; it is not impossible that it will be re-opened, but it is not likely. A test that checks only for a very specific type of failure is only ever going to see that failure. If you want to design a suite of tests that check that show-branch gives correct output for particular brands of large repo, that would be generic and potentially useful. But I don't think it's actually worth spending a lot of time on (reviewing the code for more static buffers and sprintfs would probably be a much more fruitful use of time). -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 apply --index --reject does not add new files
Hi, I believe this to be a bug. git apply --index does add new files but git apply --index --reject does not even those that apply without errors. Regards Karoly Negyesi -- 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
