* Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs
From: Jonathan Nieder @ 2012-10-30 18:59 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <1351617089-13036-4-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras wrote:
> They have been marked as UNINTERESTING for a reason, lets respect that.
This patch looks unsafe, and in the examples listed in the patch
description the changed behavior does not look like an improvement.
Worse, the description lists a few examples but gives no convincing
explanation to reassure about the lack of bad behavior for examples
not listed.
Perhaps this patch has a prerequisite and has come out of order.
Hope that helps,
Jonathan
Patch left unsnipped so we can get a copy in the list archive.
> Currently the first ref is handled properly, but not the rest, so:
>
> % git fast-export master ^master
>
> Would currently throw a reset for master (2nd ref), which is not what we
> want.
>
> % git fast-export master ^foo ^bar ^roo
> % git fast-export master salsa..tacos
>
> Even if all these refs point to the same object; foo, bar, roo, salsa,
> and tacos would all get a reset.
>
> This is most certainly not what we want. After this patch, nothing gets
> exported, because nothing was selected (everything is UNINTERESTING).
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> builtin/fast-export.c | 7 ++++---
> t/t9350-fast-export.sh | 6 ++++++
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 065f324..7fb6fe1 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -523,10 +523,11 @@ static void get_tags_and_duplicates(struct object_array *pending,
> typename(e->item->type));
> continue;
> }
> - if (commit->util)
> + if (commit->util) {
> /* more than one name for the same object */
> - string_list_append(extra_refs, full_name)->util = commit;
> - else
> + if (!(commit->object.flags & UNINTERESTING))
> + string_list_append(extra_refs, full_name)->util = commit;
> + } else
> commit->util = full_name;
> }
> }
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> index 49bdb44..6ea8f6f 100755
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -440,4 +440,10 @@ test_expect_success 'fast-export quotes pathnames' '
> )
> '
>
> +test_expect_success 'proper extra refs handling' '
> + git fast-export master ^master master..master > actual &&
> + echo -n > expected &&
> + test_cmp expected actual
> +'
> +
> test_done
> --
> 1.8.0
^ permalink raw reply
* Re: [PATCH v2 2/4] fast-export: fix comparisson in tests
From: Jonathan Nieder @ 2012-10-30 18:57 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <1351617089-13036-3-git-send-email-felipe.contreras@gmail.com>
(actually cc-ing the git list this time. Sorry for the noise, all.)
Felipe Contreras wrote:
> [Subject: [PATCH v2 2/4] fast-export: fix comparisson in tests]
>
> First the expected, then the actual, otherwise the diff would be the
> opposite of what we want.
Spelling: s/comparisson/comparison/.
Semantics: this isn't actually fixing anything --- it's a cosmetic
thing. It would be clearer to say:
fast-export test: swap arguments to test_cmp
This way if diff output is produced, it describes how the
actual output differs from what was expected rather than the
other way around.
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
For what it's worth, with amended message,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Patch left unsnipped because it hadn't hit the list.
> ---
> t/t9350-fast-export.sh | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> index 3e821f9..49bdb44 100755
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -303,7 +303,7 @@ test_expect_success 'dropping tag of filtered out object' '
> (
> cd limit-by-paths &&
> git fast-export --tag-of-filtered-object=drop mytag -- there > output &&
> - test_cmp output expected
> + test_cmp expected output
> )
> '
>
> @@ -320,7 +320,7 @@ test_expect_success 'rewriting tag of filtered out object' '
> (
> cd limit-by-paths &&
> git fast-export --tag-of-filtered-object=rewrite mytag -- there > output &&
> - test_cmp output expected
> + test_cmp expected output
> )
> '
>
> @@ -351,7 +351,7 @@ test_expect_failure 'no exact-ref revisions included' '
> (
> cd limit-by-paths &&
> git fast-export master~2..master~1 > output &&
> - test_cmp output expected
> + test_cmp expected output
> )
> '
>
> --
> 1.8.0
^ permalink raw reply
* Re: [PATCH v2 1/4] fast-export: trivial cleanup
From: Jonathan Nieder @ 2012-10-30 18:55 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <1351617089-13036-2-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras wrote:
> Setting commit to commit is a no-op.
Wrong description. This should say:
The code uses the idiom of assigning commit to itself to quench a
"may be used uninitialized" warning. Luckily at least modern
versions of gcc do not produce that warning here, so we can drop
the self-assignment.
This makes the code clearer to human beings, makes static
analyzers that do not know that idiom happier, and means that
if the code some day evolves to use this variable uninitialized
then we will catch it.
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
With that change, for what it's worth,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Patch left unsnipped since it doesn't seem to have hit the list.
> ---
> builtin/fast-export.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 12220ad..065f324 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -483,7 +483,7 @@ static void get_tags_and_duplicates(struct object_array *pending,
> for (i = 0; i < pending->nr; i++) {
> struct object_array_entry *e = pending->objects + i;
> unsigned char sha1[20];
> - struct commit *commit = commit;
> + struct commit *commit;
> char *full_name;
>
> if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)
^ permalink raw reply
* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Felipe Contreras @ 2012-10-30 18:47 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: >, Jeff King, Junio C Hamano, Jonathan Nieder,
Johannes Schindelin, Elijah Newren
In-Reply-To: <CAGdFq_j1RROOwxDi1FfJZJ6wiP9y9FWzSpc7MXVSvRmgk0sF9A@mail.gmail.com>
On Tue, Oct 30, 2012 at 7:12 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Tue, Oct 30, 2012 at 10:11 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> When an object has already been exported (and thus is in the marks) it
>> is flagged as SHOWN, so it will not be exported again, even if this time
>> it's exported through a different ref.
>>
>> We don't need the object to be exported again, but we want the ref
>> updated, which doesn't happen.
>>
>> Since we can't know if a ref was exported or not, let's just assume that
>> if the commit was marked (flags & SHOWN), the user still wants the ref
>> updated.
>>
>> So:
>>
>> % git branch test master
>> % git fast-export $mark_flags master
>> % git fast-export $mark_flags test
>>
>> Would export 'test' properly.
>>
>> Additionally, this fixes issues with remote helpers; now they can push
>> refs wich objects have already been exported.
>
> Won't this also export child (or maybe parent) branches that weren't
> mentioned? For example:
>
> $ git branch one
> $ echo foo > content
> $ git commit -m two
> $ git fast-export one
> $ git fast-export two
>
> I suspect that one of those will export both one and two. If not, this
> seems like a great solution to the fast-export problem.
Why would it? We are not changing the way objects are exported, the
only difference is what happens at the end
(handle_tags_and_duplicates()).
And if you are talking about the ref for the reset at the end, it has
to be both in the list of refs selected by the user (initially in
&revs.pending), either marked or the object already referenced by
another ref in the list selected by the user (e.g. fast-export one
two, where one^{commit} == two^{commit}, and not marked as
UNINTERESTING (e.g. ^two).
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] parse_dirstat_params(): use string_list to split comma-separated string
From: Matt Kraai @ 2012-10-30 18:43 UTC (permalink / raw)
To: git
In-Reply-To: <1351443054-10472-1-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty <mhagger <at> alum.mit.edu> writes:
...
> -static int parse_dirstat_params(struct diff_options *options, const char ...
> +static int parse_dirstat_params(struct diff_options *options, const char ...
> struct strbuf *errmsg)
> {
> - const char *p = params;
> - int p_len, ret = 0;
> + char *params_copy = xstrdup(params_string);
> + struct string_list params = STRING_LIST_INIT_NODUP;
> + int ret = 0;
> + int i;
>
> - while (*p) {
> - p_len = strchrnul(p, ',') - p;
> - if (!memcmp(p, "changes", p_len)) {
> + if (*params_copy)
params_copy is set to the value returned by xstrdup, which cannot be NULL.
This check can be removed and if params_string can be NULL, it should be
checked before being passed to xstrdup.
^ permalink raw reply
* Re: [PATCH v5 00/14] New remote-hg helper
From: Felipe Contreras @ 2012-10-30 18:29 UTC (permalink / raw)
To: Chris Webb
Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
Ilari Liusvaara, Daniel Barkalow, Jeff King, Michael J Gruber
In-Reply-To: <20121030180021.GX26850@arachsys.com>
On Tue, Oct 30, 2012 at 7:00 PM, Chris Webb <chris@arachsys.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Yes, it seems this is an API issue; repo.branchtip doesn't exist in
>> python 2.2.
>
> Hi. Presumably this is a problem with old mercurial not a problem with old
> python as mentioned in the commit?
>
>> Both issues should be fixed now :)
>
> They are indeed, and it now works nicely on all the repos I've tested it
> with, including http://selenic.com/hg: very impressive!
>
> I wonder whether it's worth ignoring heads with bookmarks pointing to them
> when it comes to considering heads of branches, or at least allowing the
> hg branch tracking to be easily disabled?
>
> A common idiom when working with hg bookmarks is to completely ignore the
> (not very useful) hg branches (i.e. all commits are on the default hg
> branch) and have a bookmark for each line of development used exactly as a
> git branch would be.
>
> On such a repository, at the moment you will always get a warning about
> multiple heads on branches/default, even though you actually don't care
> about branches/default (and would prefer it not to exist) and just want the
> branches coming from the bookmarks.
>
> I've also seen repositories with no hg branches, but with a single
> unbookmarked tip and bookmarks on some other heads to mark non-mainline
> development. It would be nice for branches/default to track the unbookmarked
> tip in this case, without warning about the other, bookmarked heads.
>
> Finally, on a simple repo with no branches and where there's no clash with a
> bookmark called master, I'd love to be able to a get a more idiomatic
> origin/master rather than origin/branches/default.
Sounds like you might want to try this:
% git config --global remote-hg.hg-git-compat true
This would match the behavior of hg-git, which seems to be what you
are looking for: branches will be ignored. But additionally there will
be other obtrusive changes:
1) Extra information will be stored in the commit message: branch,
renames, extra info.
Personally I don't like this, which is why it's only enabled with the
hg-git compat mode. Eventually there will be support for this in the
normal mode through git notes, and eventually (hopefully), hg-git will
use notes as well.
2) Authors will be fixed trying to preserve as much information as possible
The problem with this mode is what happens when there are no
bookmarks, and what I decided to do is create a fake bookmark to the
current branch (e.g. default), so you would get 'origin/default', but
you will be warned if there are multiple heads. We could try to create
a 'master' ref to the 'tip', but that might not be what the user
wants: (s)he might switch to a certain branch, and clone the
repository in git; currently the result working directory would be the
same as in hg, but if we point to 'tip', then it would not.
Maybe we should have a branch pointing to 'tip' either way.
The reason I decided on having 'branches/default' on the normal mode
is that users unfamiliar with mercurial might expect to see all the
branches somewhere. But perhaps that should happen only for other
branches, while 'default' gets a special treatment and gets promoted
to 'origin/master' but only if there's no 'master' bookmark, as you
say. Now the tricky part would be handling the case where there was no
'master' bookmark, and suddenly one gets created, but it might be
overkill to think about that right now.
Either way, try the hg-git-compat mode, should be close :)
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v4 6/8] longest_ancestor_length(): require prefix list entries to be normalized
From: Ramsay Jones @ 2012-10-30 18:23 UTC (permalink / raw)
To: Michael Haggerty
Cc: Jeff King, Junio C Hamano, Jiang Xin, Lea Wiemann, Johannes Sixt,
git
In-Reply-To: <1351440987-26636-7-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty wrote:
> Move the responsibility for normalizing prefixes from
> longest_ancestor_length() to its callers. Use slightly different
> normalizations at the two callers:
>
> In setup_git_directory_gently_1(), use the old normalization, which
> ignores paths that are not usable. In the next commit we will change
> this caller to also resolve symlinks in the paths from
> GIT_CEILING_DIRECTORIES as part of the normalization.
>
> In "test-path-utils longest_ancestor_length", use the old
> normalization, but die() if any paths are unusable. Also change t0060
> to only pass normalized paths to the test program (no empty entries or
> non-absolute paths, strip trailing slashes from the paths, and remove
> tests that thereby become redundant).
>
> The point of this change is to reduce the scope of the ancestor_length
> tests in t0060 from testing normalization+longest_prefix to testing
> only mostly longest_prefix. This is necessary because when
> setup_git_directory_gently_1() starts resolving symlinks as part of
> its normalization, it will not be reasonable to do the same in the
> test suite, because that would make the test results depend on the
> contents of the root directory of the filesystem on which the test is
> run. HOWEVER: under Windows, bash mangles arguments that look like
> absolute POSIX paths into DOS paths.
Just to be clear, this is true for the MinGW port to Windows, but *not*
the cygwin port.
:-P
> So we have to retain the level
> of normalization done by normalize_path_copy() to convert the
> bash-mangled DOS paths (which contain backslashes) into paths that use
> forward slashes.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> path.c | 26 +++++++++++---------------
> setup.c | 23 +++++++++++++++++++++++
> t/t0060-path-utils.sh | 41 +++++++++++++----------------------------
> test-path-utils.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 91 insertions(+), 44 deletions(-)
>
[snip]
> diff --git a/test-path-utils.c b/test-path-utils.c
> index acb0560..0092cbf 100644
> --- a/test-path-utils.c
> +++ b/test-path-utils.c
> @@ -1,6 +1,33 @@
> #include "cache.h"
> #include "string-list.h"
>
> +/*
> + * A "string_list_each_func_t" function that normalizes an entry from
> + * GIT_CEILING_DIRECTORIES. If the path is unusable for some reason,
> + * die with an explanation.
> + */
> +static int normalize_ceiling_entry(struct string_list_item *item, void *unused)
> +{
> + const char *ceil = item->string;
> + int len = strlen(ceil);
> + char buf[PATH_MAX+1];
> +
> + if (len == 0)
> + die("Empty path is not supported");
> + if (len > PATH_MAX)
> + die("Path \"%s\" is too long", ceil);
> + if (!is_absolute_path(ceil))
> + die("Path \"%s\" is not absolute", ceil);
> + if (normalize_path_copy(buf, ceil) < 0)
> + die("Path \"%s\" could not be normalized", ceil);
> + len = strlen(buf);
> + if (len > 1 && buf[len-1] == '/')
> + die("Normalized path \"%s\" ended with slash", buf);
> + free(item->string);
> + item->string = xstrdup(buf);
> + return 1;
> +}
> +
> int main(int argc, char **argv)
> {
> if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
> @@ -33,10 +60,26 @@ int main(int argc, char **argv)
> if (argc == 4 && !strcmp(argv[1], "longest_ancestor_length")) {
> int len;
> struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
> + char *path = xstrdup(argv[2]);
>
> + /*
> + * We have to normalize the arguments because under
> + * Windows, bash mangles arguments that look like
ditto
> + * absolute POSIX paths or colon-separate lists of
> + * absolute POSIX paths into DOS paths (e.g.,
> + * "/foo:/foo/bar" might be converted to
> + * "D:\Src\msysgit\foo;D:\Src\msysgit\foo\bar"),
> + * whereas longest_ancestor_length() requires paths
> + * that use forward slashes.
> + */
> + if (normalize_path_copy(path, path))
> + die("Path \"%s\" could not be normalized", argv[2]);
> string_list_split(&ceiling_dirs, argv[3], PATH_SEP, -1);
> - len = longest_ancestor_length(argv[2], &ceiling_dirs);
> + filter_string_list(&ceiling_dirs, 0,
> + normalize_ceiling_entry, NULL);
> + len = longest_ancestor_length(path, &ceiling_dirs);
> string_list_clear(&ceiling_dirs, 0);
> + free(path);
> printf("%d\n", len);
> return 0;
> }
HTH
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH v5 00/14] New remote-hg helper
From: Chris Webb @ 2012-10-30 18:16 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
Ilari Liusvaara, Daniel Barkalow, Jeff King, Michael J Gruber
In-Reply-To: <20121030180021.GX26850@arachsys.com>
Chris Webb <chris@arachsys.com> writes:
> A common idiom when working with hg bookmarks is to completely ignore the
> (not very useful) hg branches (i.e. all commits are on the default hg
> branch) and have a bookmark for each line of development used exactly as a
> git branch would be.
>
> On such a repository, at the moment you will always get a warning about
> multiple heads on branches/default, even though you actually don't care
> about branches/default (and would prefer it not to exist) and just want the
> branches coming from the bookmarks.
Something which you can do with hg clone is
hg clone http://my.repo/foo#master
to clone just the history behind the master bookmark from foo. This works
nicely with git-remote-hg too:
git clone hg::http://my.repo/foo#master
gives you just origin/master and origin/branches/default, not
origin/otherbookmark. This is a case where it would be particularly nice to
be able to kill origin/branches/default and just keep the identical
origin/master.
Cheers,
Chris.
^ permalink raw reply
* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Sverre Rabbelier @ 2012-10-30 18:12 UTC (permalink / raw)
To: Felipe Contreras
Cc: >, Jeff King, Junio C Hamano, Jonathan Nieder,
Johannes Schindelin, Elijah Newren
In-Reply-To: <1351617089-13036-5-git-send-email-felipe.contreras@gmail.com>
On Tue, Oct 30, 2012 at 10:11 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> When an object has already been exported (and thus is in the marks) it
> is flagged as SHOWN, so it will not be exported again, even if this time
> it's exported through a different ref.
>
> We don't need the object to be exported again, but we want the ref
> updated, which doesn't happen.
>
> Since we can't know if a ref was exported or not, let's just assume that
> if the commit was marked (flags & SHOWN), the user still wants the ref
> updated.
>
> So:
>
> % git branch test master
> % git fast-export $mark_flags master
> % git fast-export $mark_flags test
>
> Would export 'test' properly.
>
> Additionally, this fixes issues with remote helpers; now they can push
> refs wich objects have already been exported.
Won't this also export child (or maybe parent) branches that weren't
mentioned? For example:
$ git branch one
$ echo foo > content
$ git commit -m two
$ git fast-export one
$ git fast-export two
I suspect that one of those will export both one and two. If not, this
seems like a great solution to the fast-export problem.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH] Enable parallelism in git submodule update.
From: Stefan Zager @ 2012-10-30 18:11 UTC (permalink / raw)
To: git; +Cc: Jens Lehmann, Heiko Voigt, Junio C Hamano
In-Reply-To: <5090168f.5e+7ZUFKdYL2Qnw7%szager@google.com>
This is a refresh of a conversation from a couple of months ago.
I didn't try to implement all the desired features (e.g., smart logic
for passing a -j parameter to recursive submodule invocations), but I
did address the one issue that Junio insisted on: the code makes a
best effort to detect whether xargs supports parallel execution on the
host platform, and if it doesn't, then it prints a warning and falls
back to serial execution.
Stefan
On Tue, Oct 30, 2012 at 11:03 AM, <szager@google.com> wrote:
> The --jobs parameter may be used to set the degree of per-submodule
> parallel execution.
>
> Signed-off-by: Stefan Zager <szager@google.com>
> ---
> Documentation/git-submodule.txt | 8 ++++++-
> git-submodule.sh | 40 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index b4683bb..cb23ba7 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -14,7 +14,8 @@ SYNOPSIS
> 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
> 'git submodule' [--quiet] init [--] [<path>...]
> 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
> - [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
> + [--reference <repository>] [--merge] [--recursive]
> + [-j|--jobs [jobs]] [--] [<path>...]
> 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
> [commit] [--] [<path>...]
> 'git submodule' [--quiet] foreach [--recursive] <command>
> @@ -146,6 +147,11 @@ If the submodule is not yet initialized, and you just want to use the
> setting as stored in .gitmodules, you can automatically initialize the
> submodule with the `--init` option.
> +
> +By default, each submodule is treated serially. You may specify a degree of
> +parallel execution with the --jobs flag. If a parameter is provided, it is
> +the maximum number of jobs to run in parallel; without a parameter, all jobs are
> +run in parallel.
> ++
> If `--recursive` is specified, this command will recurse into the
> registered submodules, and update any nested submodules within.
> +
> diff --git a/git-submodule.sh b/git-submodule.sh
> index ab6b110..60a5f96 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
> USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
> or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
> or: $dashless [--quiet] init [--] [<path>...]
> - or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
> + or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [-j|--jobs [jobs]] [--] [<path>...]
> or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
> or: $dashless [--quiet] foreach [--recursive] <command>
> or: $dashless [--quiet] sync [--] [<path>...]"
> @@ -500,6 +500,7 @@ cmd_update()
> {
> # parse $args after "submodule ... update".
> orig_flags=
> + jobs="1"
> while test $# -ne 0
> do
> case "$1" in
> @@ -518,6 +519,20 @@ cmd_update()
> -r|--rebase)
> update="rebase"
> ;;
> + -j|--jobs)
> + case "$2" in
> + ''|-*)
> + jobs="0"
> + ;;
> + *)
> + jobs="$2"
> + shift
> + ;;
> + esac
> + # Don't preserve this arg.
> + shift
> + continue
> + ;;
> --reference)
> case "$2" in '') usage ;; esac
> reference="--reference=$2"
> @@ -551,11 +566,34 @@ cmd_update()
> shift
> done
>
> + # Correctly handle the case where '-q' came before 'update' on the command line.
> + if test -n "$GIT_QUIET"
> + then
> + orig_flags="$orig_flags -q"
> + fi
> +
> if test -n "$init"
> then
> cmd_init "--" "$@" || return
> fi
>
> + if test "$jobs" != 1
> + then
> + if ( echo test | xargs -P "$jobs" true 2>/dev/null )
> + then
> + if ( echo test | xargs --max-lines=1 true 2>/dev/null ); then
> + max_lines="--max-lines=1"
> + else
> + max_lines="-L 1"
> + fi
> + module_list "$@" | awk '{print $4}' |
> + xargs $max_lines -P "$jobs" git submodule update $orig_flags
> + return
> + else
> + echo "Warn: parallel execution is not supported on this platform."
> + fi
> + fi
> +
> cloned_modules=
> module_list "$@" | {
> err=
> --
> 1.7.7.3
>
^ permalink raw reply
* Re: [PATCH v4 00/13] New remote-hg helper
From: Felipe Contreras @ 2012-10-30 18:10 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Jeff King, git, Junio C Hamano, Sverre Rabbelier, Ilari Liusvaara,
Daniel Barkalow, Michael J Gruber
In-Reply-To: <alpine.DEB.1.00.1210301809060.7256@s15462909.onlinehome-server.info>
On Tue, Oct 30, 2012 at 6:20 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> P.S.: I would still recommend to have a detailed look at the 'devel'
> branch, in particular the commits starting with "fast-export: do not refer
> to non-existing marks" and ending with "t5801: skip without hg". My
> understanding is that it was completely ignored after a brief and maybe
> too-cursory look. In the least, it has a couple of lessons we learnt the
> hard way, and if git.git is dead set on duplicating the work, making these
> mistakes again could be avoided by learning from our lessons.
% g l --grep="t5801: skip without hg" devel
1e000d4 t5801: skip without hg
bee410c t5801: skip without hg
5cdc7d0 t5801: skip without hg
05b703f t5801: skip without hg
6bb8d90 t5801: skip without hg
c70b4d0 t5801: skip without hg
2f46371 t5801: skip without hg
39bc40f t5801: skip without hg
d0a618b t5801: skip without hg
% g l --grep="fast-export: do not refer" devel
d3ac32c fast-export: do not refer to non-existing marks
bdbb22f fast-export: do not refer to non-existing marks
5d99930 fast-export: do not refer to non-existing marks
381f276 fast-export: do not refer to non-existing marks
b4686c7 fast-export: do not refer to non-existing marks
e3dfe01 fast-export: do not refer to non-existing marks
c00fe59 fast-export: do not refer to non-existing marks
ce357ce fast-export: do not refer to non-existing marks
5c1c7a4 fast-export: do not refer to non-existing marks
9c827d1 fast-export: do not refer to non-existing marks
I'll assume you are referring the latest ones:
% g log --oneline --reverse d3ac32c^..1e000d4
* d3ac32c fast-export: do not refer to non-existing marks
Not needed at all.
* b013fe0 setup_revisions: remember whether a ref was positive or not
* fb89a2c fast-export: do not export negative refs
* 7655869 setup_revisions: remember whether a ref was positive or not
I've fixed this problem already.
The solution proposed in these patches is to convoluted:
1) Requires multiple unrelated changes
2) Proposes change in committish semantics
It's hard to test, because the test to check for this is not in this
patch series, and it's testing for something completely unrelated:
---
cat > expected << EOF
reset refs/heads/master
from $(git rev-parse master)
EOF
test_expect_success 'refs are updated even if no commits need to be exported' '
git fast-export master..master > actual &&
test_cmp expected actual
'
---
This is most certainly not what we want.
Notice that in my patch (a single patch) I added the tests at the same
time so it's clear what it's fixing, and I also added a test to the
relevant remote-helper behavior we want:
https://github.com/felipec/git/commit/76e75315bd1bd8d9d8365bb09261a745a10ceae0
* 512cb13 t5800: test pushing a new branch with old content
If this is what the patches above were trying to fix, then yes, my
patch fixes that. Also, it's tainted by changes from another patch.
* a85de2c t5800: point out that deleting branches does not work
Correct, but hardly _necessary_.
* 2412a45 transport-helper: add trailing --
No description what's the problem, or what it's trying to fix, or
tests, so it's not possible to know if this is _needed_ or not. But
probably correct.
* 026d07c remote-helper: check helper status after import/export
Again, no explanation, but the issue was already addressed:
http://article.gmane.org/gmane.comp.version-control.git/208202
The problem is minuscule, not _needed_.
* 5165e26 remote-testgit: factor out RemoteHelper class
* 049f093 git-remote-testgit: make local a function
* f835bb2 git_remote_helpers: add fastimport library
* 088ad33 git-remote-hg: add hgimport, an hg-fast-import equivalent
* 7de6ca0 git-remote-hg: add GitHg, a helper class for converting hg
commits to git
* e3cc5ed git-remote-hg: add hgexport, an hg-fast-export equivalent
* 0edc8e9 git-remote-hg: add GitExporter/GitImporter/NonLocalGit
* 5c73277 remote-hg: adjust to hg 1.9
* 1b47007 git-remote-hg: add the helper
* 4dcc671 git-remote-hg: add tests
* 48b2769 remote-hg: Postel's law dictates we should handle Author<author@mail>
* 2587cc6 remote-hg: another case of Postel's law
* 9f934c9 remote-hg: handle another funny author line from
http://scelenic.com/hg
* a799904 remote-hg: do not interfer with hg's revs() method
All these are specific to this remote-hg version.
* ac77256 Always auto-gc after calling a fast-import transport
This might be a good idea, but not _needed_.
* 1e000d4 t5801: skip without hg
Specific to this remote-hg.
So, yeah, nothing really needed there. Some patches might be nice, but
that's it.
Now, if this is really the latest and greatest remote-hg patch series,
I can try to port them to git's master and see how it fares.
But you mentioned something about cooperation, and I've yet to see how
is it that you are planning to cooperate. If you say you don't have
time to spend on this, I don't see why I should worry about testing
this series of patches.
Also, you seem to be clearly against my implementation, is there any
evidence that will convince you that my version is "good"? Maybe my
version passing more tests than msysgit's? Or is there truly nothing I
can do to change your perception?
Cheers.
--
Felipe Contreras
^ permalink raw reply
* [PATCH] Enable parallelism in git submodule update.
From: szager @ 2012-10-30 18:03 UTC (permalink / raw)
To: git; +Cc: jens.lehmann, hvoigt, gitster
The --jobs parameter may be used to set the degree of per-submodule
parallel execution.
Signed-off-by: Stefan Zager <szager@google.com>
---
Documentation/git-submodule.txt | 8 ++++++-
git-submodule.sh | 40 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b4683bb..cb23ba7 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,7 +14,8 @@ SYNOPSIS
'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
'git submodule' [--quiet] init [--] [<path>...]
'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
- [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
+ [--reference <repository>] [--merge] [--recursive]
+ [-j|--jobs [jobs]] [--] [<path>...]
'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
[commit] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
@@ -146,6 +147,11 @@ If the submodule is not yet initialized, and you just want to use the
setting as stored in .gitmodules, you can automatically initialize the
submodule with the `--init` option.
+
+By default, each submodule is treated serially. You may specify a degree of
+parallel execution with the --jobs flag. If a parameter is provided, it is
+the maximum number of jobs to run in parallel; without a parameter, all jobs are
+run in parallel.
++
If `--recursive` is specified, this command will recurse into the
registered submodules, and update any nested submodules within.
+
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..60a5f96 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
or: $dashless [--quiet] init [--] [<path>...]
- or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
+ or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [-j|--jobs [jobs]] [--] [<path>...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
or: $dashless [--quiet] foreach [--recursive] <command>
or: $dashless [--quiet] sync [--] [<path>...]"
@@ -500,6 +500,7 @@ cmd_update()
{
# parse $args after "submodule ... update".
orig_flags=
+ jobs="1"
while test $# -ne 0
do
case "$1" in
@@ -518,6 +519,20 @@ cmd_update()
-r|--rebase)
update="rebase"
;;
+ -j|--jobs)
+ case "$2" in
+ ''|-*)
+ jobs="0"
+ ;;
+ *)
+ jobs="$2"
+ shift
+ ;;
+ esac
+ # Don't preserve this arg.
+ shift
+ continue
+ ;;
--reference)
case "$2" in '') usage ;; esac
reference="--reference=$2"
@@ -551,11 +566,34 @@ cmd_update()
shift
done
+ # Correctly handle the case where '-q' came before 'update' on the command line.
+ if test -n "$GIT_QUIET"
+ then
+ orig_flags="$orig_flags -q"
+ fi
+
if test -n "$init"
then
cmd_init "--" "$@" || return
fi
+ if test "$jobs" != 1
+ then
+ if ( echo test | xargs -P "$jobs" true 2>/dev/null )
+ then
+ if ( echo test | xargs --max-lines=1 true 2>/dev/null ); then
+ max_lines="--max-lines=1"
+ else
+ max_lines="-L 1"
+ fi
+ module_list "$@" | awk '{print $4}' |
+ xargs $max_lines -P "$jobs" git submodule update $orig_flags
+ return
+ else
+ echo "Warn: parallel execution is not supported on this platform."
+ fi
+ fi
+
cloned_modules=
module_list "$@" | {
err=
--
1.7.7.3
^ permalink raw reply related
* [PATCH] Enable parallelism in git submodule update.
From: szager @ 2012-10-30 18:03 UTC (permalink / raw)
To: git, jens.lehmann, -cc, hvoigt, -cc, gitster; +Cc: c
The --jobs parameter may be used to set the degree of per-submodule
parallel execution.
Signed-off-by: Stefan Zager <szager@google.com>
---
Documentation/git-submodule.txt | 8 ++++++-
git-submodule.sh | 40 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b4683bb..cb23ba7 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,7 +14,8 @@ SYNOPSIS
'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
'git submodule' [--quiet] init [--] [<path>...]
'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
- [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
+ [--reference <repository>] [--merge] [--recursive]
+ [-j|--jobs [jobs]] [--] [<path>...]
'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
[commit] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
@@ -146,6 +147,11 @@ If the submodule is not yet initialized, and you just want to use the
setting as stored in .gitmodules, you can automatically initialize the
submodule with the `--init` option.
+
+By default, each submodule is treated serially. You may specify a degree of
+parallel execution with the --jobs flag. If a parameter is provided, it is
+the maximum number of jobs to run in parallel; without a parameter, all jobs are
+run in parallel.
++
If `--recursive` is specified, this command will recurse into the
registered submodules, and update any nested submodules within.
+
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..60a5f96 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
or: $dashless [--quiet] init [--] [<path>...]
- or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
+ or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [-j|--jobs [jobs]] [--] [<path>...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
or: $dashless [--quiet] foreach [--recursive] <command>
or: $dashless [--quiet] sync [--] [<path>...]"
@@ -500,6 +500,7 @@ cmd_update()
{
# parse $args after "submodule ... update".
orig_flags=
+ jobs="1"
while test $# -ne 0
do
case "$1" in
@@ -518,6 +519,20 @@ cmd_update()
-r|--rebase)
update="rebase"
;;
+ -j|--jobs)
+ case "$2" in
+ ''|-*)
+ jobs="0"
+ ;;
+ *)
+ jobs="$2"
+ shift
+ ;;
+ esac
+ # Don't preserve this arg.
+ shift
+ continue
+ ;;
--reference)
case "$2" in '') usage ;; esac
reference="--reference=$2"
@@ -551,11 +566,34 @@ cmd_update()
shift
done
+ # Correctly handle the case where '-q' came before 'update' on the command line.
+ if test -n "$GIT_QUIET"
+ then
+ orig_flags="$orig_flags -q"
+ fi
+
if test -n "$init"
then
cmd_init "--" "$@" || return
fi
+ if test "$jobs" != 1
+ then
+ if ( echo test | xargs -P "$jobs" true 2>/dev/null )
+ then
+ if ( echo test | xargs --max-lines=1 true 2>/dev/null ); then
+ max_lines="--max-lines=1"
+ else
+ max_lines="-L 1"
+ fi
+ module_list "$@" | awk '{print $4}' |
+ xargs $max_lines -P "$jobs" git submodule update $orig_flags
+ return
+ else
+ echo "Warn: parallel execution is not supported on this platform."
+ fi
+ fi
+
cloned_modules=
module_list "$@" | {
err=
--
1.7.7.3
^ permalink raw reply related
* Re: [PATCH v5 00/14] New remote-hg helper
From: Chris Webb @ 2012-10-30 18:00 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
Ilari Liusvaara, Daniel Barkalow, Jeff King, Michael J Gruber
In-Reply-To: <CAMP44s1g8rFGP7UOcvp9BEZ1oiSh3+-gYheciqO8Fmghipot8A@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> Yes, it seems this is an API issue; repo.branchtip doesn't exist in
> python 2.2.
Hi. Presumably this is a problem with old mercurial not a problem with old
python as mentioned in the commit?
> Both issues should be fixed now :)
They are indeed, and it now works nicely on all the repos I've tested it
with, including http://selenic.com/hg: very impressive!
I wonder whether it's worth ignoring heads with bookmarks pointing to them
when it comes to considering heads of branches, or at least allowing the
hg branch tracking to be easily disabled?
A common idiom when working with hg bookmarks is to completely ignore the
(not very useful) hg branches (i.e. all commits are on the default hg
branch) and have a bookmark for each line of development used exactly as a
git branch would be.
On such a repository, at the moment you will always get a warning about
multiple heads on branches/default, even though you actually don't care
about branches/default (and would prefer it not to exist) and just want the
branches coming from the bookmarks.
I've also seen repositories with no hg branches, but with a single
unbookmarked tip and bookmarks on some other heads to mark non-mainline
development. It would be nice for branches/default to track the unbookmarked
tip in this case, without warning about the other, bookmarked heads.
Finally, on a simple repo with no branches and where there's no clash with a
bookmark called master, I'd love to be able to a get a more idiomatic
origin/master rather than origin/branches/default.
Just some idle thoughts...
Best wishes,
Chris.
^ permalink raw reply
* Re: [PATCH v5 00/14] New remote-hg helper
From: Johannes Schindelin @ 2012-10-30 17:27 UTC (permalink / raw)
To: Chris Webb
Cc: Felipe Contreras, git, Junio C Hamano, Sverre Rabbelier,
Ilari Liusvaara, Daniel Barkalow, Jeff King, Michael J Gruber
In-Reply-To: <20121030102526.GN4891@arachsys.com>
Hi Chris,
On Tue, 30 Oct 2012, Chris Webb wrote:
> I routinely work with projects in both hg and git, so I'm really
> interested in this. Thanks for working on it! I grabbed the latest
> version from
>
> https://github.com/felipec/git/blob/fc-remote-hg/contrib/remote-hg/git-remote-hg
>
> and have been trying it out.
You might also want to try out the 'devel' branch of
https://github.com/msysgit/git. It is in production use here.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v4 00/13] New remote-hg helper
From: Johannes Schindelin @ 2012-10-30 17:20 UTC (permalink / raw)
To: Jeff King
Cc: Felipe Contreras, git, Junio C Hamano, Sverre Rabbelier,
Ilari Liusvaara, Daniel Barkalow, Michael J Gruber
In-Reply-To: <20121029215631.GF20513@sigill.intra.peff.net>
Hi all,
On Mon, 29 Oct 2012, Jeff King wrote:
> On Mon, Oct 29, 2012 at 10:47:04PM +0100, Felipe Contreras wrote:
>
> > >> Yeah, the test script is not ready for merging, it needs to check
> > >> for python, hg, and hg-git.
> > >>
> > >> Do you have hg-git installed?
> > >
> > > No. But it's important that it fail gracefully; I can't even take it
> > > in pu if I can't run the test suite in a sane way.
> >
> > The contrib part is fine for 'pu'. The tests aren't even meant to
> > exercise stuff in 'contrib', right? There might be some exceptions,
> > but either way, there's plenty of stuff in 'contrib' without any
> > tests. The tests I'm providing are simply a little sugar.
>
> Yeah, contrib is a bit of a wildcard. Most things do not have tests.
Given that the tests of remote-hg as in git://github.com/msysgit/git's
'devel' branch run just fine without additional dependencies (which
probably triggered the not-quite-constructive and unnecessarily-flaming
"bloated" comment of Felipe), and given that the code in said branch is
well-tested and exercised by daily use, and given the fact that my major
concern was not understood (and probably not addressed), and also given
the fact that Sverre indicated that he could finalize the work as a 20%
project, I decided that other projects I have to do unfortunately have a
too-high priority to take care of testing and measuring the performance of
the patch series that is discussed in this thread.
Sorry,
Johannes
P.S.: I would still recommend to have a detailed look at the 'devel'
branch, in particular the commits starting with "fast-export: do not refer
to non-existing marks" and ending with "t5801: skip without hg". My
understanding is that it was completely ignored after a brief and maybe
too-cursory look. In the least, it has a couple of lessons we learnt the
hard way, and if git.git is dead set on duplicating the work, making these
mistakes again could be avoided by learning from our lessons.
^ permalink raw reply
* Re: [PATCH v4 00/13] New remote-hg helper
From: Felipe Contreras @ 2012-10-30 17:18 UTC (permalink / raw)
To: Jeff King
Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
Ilari Liusvaara, Daniel Barkalow, Michael J Gruber
In-Reply-To: <20121029220604.GA21712@sigill.intra.peff.net>
On Mon, Oct 29, 2012 at 11:06 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 11:02:31PM +0100, Felipe Contreras wrote:
>
>> > If remote-hg is going to live in contrib, it probably makes sense to
>> > have its tests live there, too, like subtree.
>>
>> Probably, I'll check that option.
>>
>> But eventually I think it should be installed by default, unless
>> somebody can come up for a reason not to. For now contrib might be OK.
>
> I would one day like to have it as part of the main distribution, too,
> but it would be nice to prove its worth in the field for a while first.
> I especially would like to find out how it compares in practice with the
> work that is in msysgit.
Yeah, I would like to compare it with that work, if only the patches
were readily available somewhere.
--
Felipe Contreras
^ permalink raw reply
* Re: git push tags
From: Chris Rorvick @ 2012-10-30 17:09 UTC (permalink / raw)
To: Jeff King
Cc: Kacper Kornet, Drew Northup, Michael Haggerty, Angelo Borsotti,
Philip Oakley, Johannes Sixt, git
In-Reply-To: <20121029213508.GB20513@sigill.intra.peff.net>
On Mon, Oct 29, 2012 at 4:35 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 06:23:30PM +0100, Kacper Kornet wrote:
>
>> > That patch just blocks non-forced updates to refs/tags/. I think a saner
>> > start would be to disallow updating non-commit objects without a force.
>> > We already do so for blobs and trees because they are not (and cannot
>> > be) fast forwards. The fact that annotated tags are checked for
>> > fast-forward seems to me to be a case of "it happens to work that way"
>> > and not anything planned. Since such a push drops the reference to the
>> > old version of the tag, it should probably require a force.
>>
>> I'm not sure. Looking at 37fde87 ("Fix send-pack for non-commitish
>> tags.") I have an impression that Junio allowed for fast-forward pushes
>> of annotated tags on purpose.
>
> Hmm. You're right, though I'm not sure I agree with the reasoning of
> that commit. I'd certainly like to get Junio's input on the subject.
>
>> > Then on top of that we can talk about what lightweight tags should do.
>> > I'm not sure. Following the regular fast-forward rules makes some sense
>> > to me, because you are never losing objects. But there may be
>> > complications with updating tags in general because of fetch's rules,
>> > and we would be better off preventing people from accidentally doing so.
>> > I think a careful review of fetch's tag rules would be in order before
>> > making any decision there.
>>
>> The problem with the current behaviour is, that one can never be 100% sure
>> that his push will not overwrite someone else tag.
>
> Yes, although you do know that you are not throwing away history if you
> do (because it must be a fast forward). Whereas if you have to use "-f"
> to update a tag, then you have turned off all safety checks. So it is an
> improvement for one case (creating a tag), but a regression for another
> (updating an existing tag). I agree that the latter is probably less
> common, but how much? If virtually nobody is doing it because git-fetch
> makes the fetching side too difficult, then the regression is probably
> not a big deal.
>
> -Peff
This is probably a bit premature given there are still open questions,
but I was curious and decided to take a stab at this.
The change is to only allow fast-forward when both the old and new are
commits and the reference is not a lightweight tag. All other
reference updates require --force. I think this resolves the reported
issue and takes into account feedback on this thread. This change
only broke one test and it was an expected failure given the change in
behavior (i.e., I needed to add a "-f" to update a tag in the remote.)
I wasn't sure how to handle provided feedback to the user when there
are multiple refs not pushed for different reasons. But I think this
adds the plumbing for handling it correctly, whateverever that this.
It needs some work, but thought I'd throw it out for feedback to see
if it's at least in the right direction.
Chris
--- 8< ---
diff --git a/builtin/push.c b/builtin/push.c
index db9ba30..fabcea0 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -220,6 +220,10 @@ static const char message_advice_checkout_pull_push[] =
"(e.g. 'git pull') before pushing again.\n"
"See the 'Note about fast-forwards' in 'git push --help' for details.");
+static const char message_advice_ref_already_exists[] =
+ N_("Updates were rejected because a matching reference already exists in\n"
+ "the remote. Use git push -f if you really want to make this update.");
+
static void advise_pull_before_push(void)
{
if (!advice_push_non_ff_current || !advice_push_nonfastforward)
@@ -241,6 +245,11 @@ static void advise_checkout_pull_push(void)
advise(_(message_advice_checkout_pull_push));
}
+static void advise_ref_already_exists(void)
+{
+ advise(_(message_advice_ref_already_exists));
+}
+
static int push_with_options(struct transport *transport, int flags)
{
int err;
@@ -277,6 +286,9 @@ static int push_with_options(struct transport
*transport, int flags)
else
advise_checkout_pull_push();
break;
+ case ALREADY_EXISTS:
+ advise_ref_already_exists();
+ break;
}
return 1;
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 7d05064..f159ec3 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -202,6 +202,11 @@ static void print_helper_status(struct ref *ref)
msg = "non-fast forward";
break;
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
+ res = "error";
+ msg = "already exists";
+ break;
+
case REF_STATUS_REJECT_NODELETE:
case REF_STATUS_REMOTE_REJECT:
res = "error";
@@ -288,6 +293,7 @@ int send_pack(struct send_pack_args *args,
/* Check for statuses set by set_ref_status_for_push() */
switch (ref->status) {
case REF_STATUS_REJECT_NONFASTFORWARD:
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
case REF_STATUS_UPTODATE:
continue;
default:
diff --git a/cache.h b/cache.h
index a58df84..2d160a9 100644
--- a/cache.h
+++ b/cache.h
@@ -1002,11 +1002,14 @@ struct ref {
unsigned int force:1,
merge:1,
nonfastforward:1,
+ forwardable:1,
+ update:1,
deletion:1;
enum {
REF_STATUS_NONE = 0,
REF_STATUS_OK,
REF_STATUS_REJECT_NONFASTFORWARD,
+ REF_STATUS_REJECT_ALREADY_EXISTS,
REF_STATUS_REJECT_NODELETE,
REF_STATUS_UPTODATE,
REF_STATUS_REMOTE_REJECT,
diff --git a/remote.c b/remote.c
index 04fd9ea..0d94888 100644
--- a/remote.c
+++ b/remote.c
@@ -1309,22 +1309,42 @@ void set_ref_status_for_push(struct ref
*remote_refs, int send_mirror,
* to overwrite it; you would not know what you are losing
* otherwise.
*
- * (3) if both new and old are commit-ish, and new is a
- * descendant of old, it is OK.
+ * (3) if both new and old are commits, the reference is not
+ * a tag, and new is a descendant of old, it is OK.
*
* (4) regardless of all of the above, removing :B is
* always allowed.
*/
- ref->nonfastforward =
+ ref->update =
!ref->deletion &&
- !is_null_sha1(ref->old_sha1) &&
+ !is_null_sha1(ref->old_sha1);
+
+ ref->nonfastforward =
+ ref->update &&
(!has_sha1_file(ref->old_sha1)
|| !ref_newer(ref->new_sha1, ref->old_sha1));
- if (ref->nonfastforward && !ref->force && !force_update) {
- ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
- continue;
+ if (prefixcmp(ref->name, "refs/tags/")) {
+ struct object *old = parse_object(ref->old_sha1);
+ struct object *new = parse_object(ref->new_sha1);
+ ref->forwardable = (old && new &&
+ old->type == OBJ_COMMIT && new->type == OBJ_COMMIT);
+ } else
+ ref->forwardable = 0;
+
+ if (!ref->force && !force_update) {
+ if (ref->forwardable) {
+ if (ref->nonfastforward) {
+ ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+ continue;
+ }
+ } else {
+ if (ref->update) {
+ ref->status = REF_STATUS_REJECT_ALREADY_EXISTS;
+ continue;
+ }
+ }
}
}
}
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..cff559f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -368,7 +368,7 @@ test_expect_success 'push with colon-less refspec (2)' '
git branch -D frotz
fi &&
git tag -f frotz &&
- git push testrepo frotz &&
+ git push -f testrepo frotz &&
check_push_result $the_commit tags/frotz &&
check_push_result $the_first_commit heads/frotz
@@ -929,6 +929,34 @@ test_expect_success 'push into aliased refs
(inconsistent)' '
)
'
+test_expect_success 'push tag requires --force to update remote tag' '
+ mk_test heads/master &&
+ mk_child child1 &&
+ mk_child child2 &&
+ (
+ cd child1 &&
+ git tag lw_tag &&
+ git tag -a -m "message 1" ann_tag &&
+ git push ../child2 lw_tag &&
+ git push ../child2 ann_tag &&
+ >file1 &&
+ git add file1 &&
+ git commit -m "file1" &&
+ git tag -f lw_tag &&
+ git tag -f -a -m "message 2" ann_tag &&
+ ! git push ../child2 lw_tag &&
+ ! git push ../child2 ann_tag &&
+ git push --force ../child2 lw_tag &&
+ git push --force ../child2 ann_tag &&
+ git tag -f lw_tag HEAD~ &&
+ git tag -f -a -m "message 3" ann_tag &&
+ ! git push ../child2 lw_tag &&
+ ! git push ../child2 ann_tag &&
+ git push --force ../child2 lw_tag &&
+ git push --force ../child2 ann_tag
+ )
+'
+
test_expect_success 'push --porcelain' '
mk_empty &&
echo >.git/foo "To testrepo" &&
diff --git a/transport-helper.c b/transport-helper.c
index cfe0988..ef9a6f8 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -643,6 +643,11 @@ static void push_update_ref_status(struct strbuf *buf,
free(msg);
msg = NULL;
}
+ else if (!strcmp(msg, "already exists")) {
+ status = REF_STATUS_REJECT_ALREADY_EXISTS;
+ free(msg);
+ msg = NULL;
+ }
}
if (*ref)
@@ -702,6 +707,7 @@ static int push_refs_with_push(struct transport *transport,
/* Check for statuses set by set_ref_status_for_push() */
switch (ref->status) {
case REF_STATUS_REJECT_NONFASTFORWARD:
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
case REF_STATUS_UPTODATE:
continue;
default:
diff --git a/transport.c b/transport.c
index 9932f40..d218884 100644
--- a/transport.c
+++ b/transport.c
@@ -659,7 +659,7 @@ static void print_ok_ref_status(struct ref *ref,
int porcelain)
const char *msg;
strcpy(quickref, status_abbrev(ref->old_sha1));
- if (ref->nonfastforward) {
+ if (ref->nonfastforward || (!ref->forwardable && ref->update)) {
strcat(quickref, "...");
type = '+';
msg = "forced update";
@@ -695,6 +695,10 @@ static int print_one_push_status(struct ref *ref,
const char *dest, int count, i
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
"non-fast-forward", porcelain);
break;
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
+ print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+ "already exists", porcelain);
+ break;
case REF_STATUS_REMOTE_REJECT:
print_ref_status('!', "[remote rejected]", ref,
ref->deletion ? NULL : ref->peer_ref,
@@ -714,7 +718,7 @@ static int print_one_push_status(struct ref *ref,
const char *dest, int count, i
}
void transport_print_push_status(const char *dest, struct ref *refs,
- int verbose, int porcelain, int *nonfastforward)
+ int verbose, int porcelain, int *willnotupdate)
{
struct ref *ref;
int n = 0;
@@ -733,18 +737,21 @@ void transport_print_push_status(const char
*dest, struct ref *refs,
if (ref->status == REF_STATUS_OK)
n += print_one_push_status(ref, dest, n, porcelain);
- *nonfastforward = 0;
+ *willnotupdate = 0;
for (ref = refs; ref; ref = ref->next) {
if (ref->status != REF_STATUS_NONE &&
ref->status != REF_STATUS_UPTODATE &&
ref->status != REF_STATUS_OK)
n += print_one_push_status(ref, dest, n, porcelain);
if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD &&
- *nonfastforward != NON_FF_HEAD) {
+ *willnotupdate != NON_FF_HEAD) {
if (!strcmp(head, ref->name))
- *nonfastforward = NON_FF_HEAD;
+ *willnotupdate = NON_FF_HEAD;
else
- *nonfastforward = NON_FF_OTHER;
+ *willnotupdate = NON_FF_OTHER;
+ } else if (ref->status == REF_STATUS_REJECT_ALREADY_EXISTS &&
+ *willnotupdate == 0) {
+ *willnotupdate = ALREADY_EXISTS;
}
}
}
@@ -1031,9 +1038,9 @@ static void die_with_unpushed_submodules(struct
string_list *needs_pushing)
int transport_push(struct transport *transport,
int refspec_nr, const char **refspec, int flags,
- int *nonfastforward)
+ int *willnotupdate)
{
- *nonfastforward = 0;
+ *willnotupdate = 0;
transport_verify_remote_names(refspec_nr, refspec);
if (transport->push) {
@@ -1099,7 +1106,7 @@ int transport_push(struct transport *transport,
if (!quiet || err)
transport_print_push_status(transport->url, remote_refs,
verbose | porcelain, porcelain,
- nonfastforward);
+ willnotupdate);
if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
set_upstreams(transport, remote_refs, pretend);
diff --git a/transport.h b/transport.h
index 3b21c4a..326271e 100644
--- a/transport.h
+++ b/transport.h
@@ -142,6 +142,7 @@ void transport_set_verbosity(struct transport
*transport, int verbosity,
#define NON_FF_HEAD 1
#define NON_FF_OTHER 2
+#define ALREADY_EXISTS 3
int transport_push(struct transport *connection,
int refspec_nr, const char **refspec, int flags,
int * nonfastforward);
@@ -170,7 +171,7 @@ void transport_update_tracking_ref(struct remote
*remote, struct ref *ref, int v
int transport_refs_pushed(struct ref *ref);
void transport_print_push_status(const char *dest, struct ref *refs,
- int verbose, int porcelain, int *nonfastforward);
+ int verbose, int porcelain, int *willnotupdate);
typedef void alternate_ref_fn(const struct ref *, void *);
extern void for_each_alternate_ref(alternate_ref_fn, void *);
--
1.8.0
^ permalink raw reply related
* Re: [PATCH v6 0/3] completion: refactor and zsh wrapper
From: Felipe Contreras @ 2012-10-30 16:15 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, SZEDER Gábor, Matthieu Moy,
Felipe Contreras
In-Reply-To: <1350870342-22653-1-git-send-email-felipe.contreras@gmail.com>
Hi,
On Mon, Oct 22, 2012 at 3:45 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Here's a bit of reorganition. I'm introducing a new __gitcompadd helper that is
> useful to wrapp all changes to COMPREPLY, but first, lets get rid of
> unnecessary assignments as SZEDER suggested.
>
> The zsh wrapper is now very very simple.
Junio, Jeff, just to let you know, this is an updated version of the
new zsh wrapper patch series with the feedback from SZEDER. I see the
old version is in pu, and hasn't been updated.
Cheers.
> Since v5:
>
> * Get rid of unnecessary COMPREPLY assignments
>
> Felipe Contreras (3):
> completion: get rid of empty COMPREPLY assignments
> completion: add new __gitcompadd helper
> completion: add new zsh completion
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] test-lib: avoid full path to store test results
From: Felipe Contreras @ 2012-10-30 16:02 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, Junio C Hamano, Jeff King, Ævar Arnfjörð,
Johannes Sixt
In-Reply-To: <20121030044609.GA10873@elie.Belkin>
On Tue, Oct 30, 2012 at 5:46 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>> No reason to use the full path in case this is used externally.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>
> "No reason not to" is not a reason to do anything. What symptoms does
> this prevent? Could you describe the benefit of this patch in a
> paragraph starting "Now you can ..."?
./test-lib.sh: line 394:
/home/felipec/dev/git/t/test-results//home/felipec/dev/git/contrib/remote-hg/test-21865.counts:
No such file or directory
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v5 00/14] New remote-hg helper
From: Felipe Contreras @ 2012-10-30 15:51 UTC (permalink / raw)
To: Chris Webb
Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
Ilari Liusvaara, Daniel Barkalow, Jeff King, Michael J Gruber
In-Reply-To: <20121030102526.GN4891@arachsys.com>
On Tue, Oct 30, 2012 at 11:25 AM, Chris Webb <chris@arachsys.com> wrote:
> Hi. I routinely work with projects in both hg and git, so I'm really
> interested in this. Thanks for working on it! I grabbed the latest version
> from
>
> https://github.com/felipec/git/blob/fc-remote-hg/contrib/remote-hg/git-remote-hg
>
> and have been trying it out. For the most part, it seems to work very nicely
> for the hg repos I have access to and can test against. I've spotted a couple
> of issues along the way that I thought would be worth reporting.
Great!
> The first is really a symptom of a general difference between hg and git: an hg
> repository can have multiple heads, whereas a git repo has exactly one head. To
> demonstrate:
> Now if I try to convert this:
>
> $ git clone hg::$PWD/hgtest gittest
> Cloning into 'gittest'...
> WARNING: Branch 'default' has more than one head, consider merging
> Traceback (most recent call last):
> File "/home/chris/bin/git-remote-hg", line 773, in <module>
> sys.exit(main(sys.argv))
> File "/home/chris/bin/git-remote-hg", line 759, in main
> do_list(parser)
> File "/home/chris/bin/git-remote-hg", line 463, in do_list
> list_branch_head(repo, cur)
> File "/home/chris/bin/git-remote-hg", line 425, in list_branch_head
> tip = get_branch_tip(repo, cur)
> File "/home/chris/bin/git-remote-hg", line 418, in get_branch_tip
> return repo.branchtip(branch)
> AttributeError: 'mqrepo' object has no attribute 'branchtip'
Yes, it seems this is an API issue; repo.branchtip doesn't exist in
python 2.2. I've added a check for that, and it should work fine now.
We'll be picking a random head (the first one), but the user has been
warned anyway.
> The second thing I spotted is the behaviour of bookmarks on push:
> i.e. the development bookmark hasn't been updated by the push. This might be
> connected to the warning message
This is not an API issue, this is a bug; bookmarks are not updated
(only the first creation works). I've fixed this as well, and added a
test with the example you put above:
http://github.com/felipec/git/commit/d006d54fc444484707dffa24d9fad053e574918d
Both issues should be fixed now :)
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: Git clone fails with "bad pack header", how to get remote log
From: kevin molcard @ 2012-10-30 14:57 UTC (permalink / raw)
To: Konstantin Khomoutov
Cc: git-users-/JYPxA39Uh5TLH3MbocFFw, git-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20121029211854.b58c791d30a6c8d68665e574-g5ZlayWIM10NZ+ppGFcyYQ@public.gmane.org>
I tried to install git 1.8 on the remote server and get exactly the same
problem :(.
Kevin
On 10/29/12 6:18 PM, Konstantin Khomoutov wrote:
> On Mon, 29 Oct 2012 09:52:54 -0700 (PDT)
> Kevin Molcard <kev2041-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> I have a problem with my build system.
>>
>> I have a remote server with a relatively large repository (around 12
>> GB, each branch having a size of 3 GB).
>>
>> I have also 2 build servers (Mac, Windows) that are cloning the repo
>> from the remote.
>>
>> Sometimes (very often when several git clone are sent at the same
>> time), I have the following error:
>>
>> remote: internal server error
>> fatal: protocol error: bad pack header
>>
>> I know that it happens when the remote is compressing objects (thanks
>> to `--progress -v` flags) because the last line of the log before the
>> erro is:
>> remote: Compressing objects: 93% (17959/19284) [K
>>
>> * So I have 2 questions, does anybody what is the problem and what
>> should I do?
>> * Is there a way to get a more precise log from the remote to debug
>> this problem?
> This reminds me of a bug fixed in 1.7.12.1 [1]:
>
> * When "git push" triggered the automatic gc on the receiving end, a
> message from "git prune" that said it was removing cruft leaked to
> the standard output, breaking the communication protocol.
>
> In any case, bugs should be reported to the main Git list (which is
> git at vger.kernel.org), not here.
> I'm Cc'ing the main Git list so you'll get any responses from there, if
> any.
>
> Kevin, please answer to this message (keeping all the Ccs -- use "Reply
> to group" or "Reply to all" in your MUA) and describe exactly what Git
> versions on which platforms your have.
>
> 1. https://raw.github.com/git/git/master/Documentation/RelNotes/1.7.12.1.txt
>
--
You received this message because you are subscribed to the Google Groups "Git for human beings" group.
To post to this group, send email to git-users-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To unsubscribe from this group, send email to git-users+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit this group at http://groups.google.com/group/git-users?hl=en.
^ permalink raw reply
* Re: Why does git-commit --template want the template to be modified ?
From: Francis Moreau @ 2012-10-30 14:23 UTC (permalink / raw)
To: Tomas Carnecky, Johannes Sixt; +Cc: git
In-Reply-To: <1351595365-ner-1835@calvin>
Hi,
On Tue, Oct 30, 2012 at 12:09 PM, Tomas Carnecky
<tomas.carnecky@gmail.com> wrote:
> On Tue, 30 Oct 2012 11:53:08 +0100, Francis Moreau <francis.moro@gmail.com> wrote:
>> Hi,
>>
>> I'm using git-commit with the --template option. The template I'm
>> given is self sufficient for my purpose but as stated in the
>> documentation, git-commit wants the template to be edited otherwise it
>> aborts the operation.
>>
>> Is it possible to change this ?
>
> It seems you want -F instead of --template.
Yes, but I want git to still parse the log and sanitize it (remove
trailing whitespaces, comments ...)
I actually found that "-F --edit" is what I needed.
Thanks both for your answer.
--
Francis
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2012, #01; Tue, 2)
From: Florian Achleitner @ 2012-10-30 12:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Michael Barr, Dmitry Ivankov, Jonathan Nieder
In-Reply-To: <7vmx045umh.fsf@alter.siamese.dyndns.org>
Sorry for reacting so late, I didn't read the list carefully in the last weeks
and my gmail filter somehow didn't trigger on that.
On Tuesday 02 October 2012 16:20:22 Junio C Hamano wrote:
> * fa/remote-svn (2012-09-19) 16 commits
> - Add a test script for remote-svn
> - remote-svn: add marks-file regeneration
> - Add a svnrdump-simulator replaying a dump file for testing
> - remote-svn: add incremental import
> - remote-svn: Activate import/export-marks for fast-import
> - Create a note for every imported commit containing svn metadata
> - vcs-svn: add fast_export_note to create notes
> - Allow reading svn dumps from files via file:// urls
> - remote-svn, vcs-svn: Enable fetching to private refs
> - When debug==1, start fast-import with "--stats" instead of "--quiet"
> - Add documentation for the 'bidi-import' capability of remote-helpers
> - Connect fast-import to the remote-helper via pipe, adding 'bidi-import'
> capability - Add argv_array_detach and argv_array_free_detached
> - Add svndump_init_fd to allow reading dumps from arbitrary FDs
> - Add git-remote-testsvn to Makefile
> - Implement a remote helper for svn in C
> (this branch is used by fa/vcs-svn.)
>
> A GSoC project.
> Waiting for comments from mentors and stakeholders.
>From my point of view, this is rather complete. It got eight review cycles on
the list.
Note that the remote helper can only fetch, pushing is not possible at all.
>
>
> * fa/vcs-svn (2012-09-19) 4 commits
> - vcs-svn: remove repo_tree
> - vcs-svn/svndump: rewrite handle_node(), begin|end_revision()
> - vcs-svn/svndump: restructure node_ctx, rev_ctx handling
> - svndump: move struct definitions to .h
> (this branch uses fa/remote-svn.)
>
> A GSoC project.
> Waiting for comments from mentors and stakeholders.
This is the result of what I did when I wanted to start implementing branch
detection. I found that the existing code is not suitable and restructured it.
The main goal is to seperate svn revision parsing from git commit creation.
Because for creating commits, you need to know on which branch to create the
commit.
While for finding out which branch is the right one, you need to read the
complete svn revision first to see what dirs are changed and how.
It is rather invasive and it doesn't make sense without using it later on.
So I'm not surprised that you may not like it.
Anyways it passes all existing tests (that doesn't mean it's good of course
;))
Florian
^ permalink raw reply
* Re: crash on git diff-tree -Ganything <tree> for new files with textconv filter
From: Jeff King @ 2012-10-30 13:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Peter Oberndorfer, git
In-Reply-To: <da24b6ea-ac9b-46dd-b591-25fd4e8e6504@email.android.com>
On Tue, Oct 30, 2012 at 09:46:01PM +0900, Junio C Hamano wrote:
> (1) sounds attractive for more than one reason. In addition to
> avoidance of this issue, it would bring bug-to-bug compatibility
> across platforms.
Yeah. I mentioned breaking the build for people who would now need to
turn on NO_REGEX. But the only reason to do that is to let people on
glibc systems use the system version of the tools. A much saner approach
would be to just always build with our compat regex, and turn NO_REGEX
into a no-op. We already do the same thing for kwset.
> (4), if we can run grep on streaming data (tweak interface we have for
> checking out a large blob to the working tree), would let us work on
> dataset larger than fit in core. Even though it would be much more
> work, it might turn out to be a better option in the longer run.
Agreed, that would be nice. It's potentially a lot of work, but we could
probably get by with a special streaming version of diff_populate_filespec.
The tricky thing is that we have to run the regex matcher progressively
as we stream data in (since your match might fall in the middle of a
read boundary). Which is certainly going to require switching off of
regular regexec. I don't think glibc regex will handle it either,
though. It looks like pcre can report a partial match at the end of the
string, and you can either continue with the next chunk (if using
pcre_dfa) or append and re-start the pattern match (for regular
pcre_exec).
Which means we'd probably have to make streaming matches an optional
feature, and still do (1) first to fix the correctness issue.
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox