* Re: git grep performance regression
From: Duy Nguyen @ 2013-01-15 4:38 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: git, Jean-Noël AVILA, Junio C Hamano
In-Reply-To: <CACsJy8A7FLYqdY2Mt5pUq0nH3N8mbZ4crkYJYFfepp19c0aWhg@mail.gmail.com>
On Tue, Jan 15, 2013 at 9:46 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> I don't have time to look into details now, but by enabling
> DEBUG_ATTR, it looks like this commit makes it push and pop patterns a
> lot more than without the commit.
I think the culprit is at this chunk:
static void prepare_attr_stack(const char *path)
{
struct attr_stack *elem, *info;
int dirlen, len;
const char *cp;
- cp = strrchr(path, '/');
- if (!cp)
- dirlen = 0;
- else
- dirlen = cp - path;
+ dirlen = find_basename(path) - path;
dirlen is not expected to include the trailing slash, but
find_basename() does that. It messes up with the path filters for
push/pop in the next code. This brings grep performance closely back
to before for me. Ross, can you check (patch could be whitespace
damaged by gmail)?
-- 8< --
diff --git a/attr.c b/attr.c
index b05110d..1e96e26 100644
--- a/attr.c
+++ b/attr.c
@@ -583,6 +583,9 @@ static void prepare_attr_stack(const char *path)
dirlen = find_basename(path) - path;
+ if (dirlen)
+ dirlen--;
+
/*
* At the bottom of the attribute stack is the built-in
* set of attribute definitions, followed by the contents
-- 8< --
^ permalink raw reply related
* Re: [BUG] Possible bug in `remote set-url --add --push`
From: Jardel Weyrich @ 2013-01-15 5:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, Sascha Cunz, git@vger.kernel.org
In-Reply-To: <7v1udnbmyz.fsf@alter.siamese.dyndns.org>
On 14/01/2013, at 17:09, Junio C Hamano <gitster@pobox.com> wrote:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> It seems to me that everything works as designed, and that the man page
>> talk about "push URLs" can be read in two ways,...
>
> Hmph, but I had an impression that Jardel's original report was that
> one of the --add --pushurl was not adding but was replacing. If
> that was a false alarm, everything you said makes sense to me.
>
> Thanks.
I failed to explain my reasoning. But I learned quite a bit from this discussion. I understood that the defaul push url is not used by git-push when there's at least one pushurl for a given remote.
If that's by design, I still fail to comprehend the exact reason.
If you allow me, I'd like you to forget about the concepts for a minute, and focus on the user experience.
Imagine a simple hypothetical scenario in which the user wants to push to 2 distinct repositories. He already has cloned the repo from the 1st repository, thus (theoretically) all he needs to do, is to add a new repository for push. He then uses `remote set-url --add --push <2nd-repo>` (which I personally thought would suffice). However, if he tries to push a new commit to this remote, it would be pushed _only_ to the 2nd-repo.
This is exactly what I thought to be a bug. If it's intended to work the way I described in the previous scenario, I'll have to ask and/or research to understand the reason behind this -- Why does having a pushurl make git-push _not_ to push to the default push location (the 1st repo in my scenario) as well? Could you describe a scenario in which that behavior is useful and/or better than the behavior I expected?
Please, pardon me for not being as clear as needed. I appreciate your time on this. Thank you all.
Sent from my mobile.
^ permalink raw reply
* [PATCH v2 01/19] reset $pathspec: no need to discard index
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
Since 34110cd (Make 'unpack_trees()' have a separate source and
destination index, 2008-03-06), the index no longer gets clobbered by
do_diff_cache() and we can remove the code for discarding and
re-reading it.
There are two paths to update_index_refresh() from cmd_reset(), but on
both paths, either read_cache() or read_cache_unmerged() will have
been called, so the call to read_cache() in this method is redundant
(although practically free).
This speeds up "git reset -- ." a little on the linux-2.6 repo (best
of five, warm cache):
Before After
real 0m0.093s 0m0.080s
user 0m0.040s 0m0.020s
sys 0m0.050s 0m0.050s
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 16 +---------------
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 915cc9f..8cc7c72 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -126,9 +126,6 @@ static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)
fd = hold_locked_index(index_lock, 1);
}
- if (read_cache() < 0)
- return error(_("Could not read index"));
-
result = refresh_index(&the_index, (flags), NULL, NULL,
_("Unstaged changes after reset:")) ? 1 : 0;
if (write_cache(fd, active_cache, active_nr) ||
@@ -141,12 +138,6 @@ static void update_index_from_diff(struct diff_queue_struct *q,
struct diff_options *opt, void *data)
{
int i;
- int *discard_flag = data;
-
- /* do_diff_cache() mangled the index */
- discard_cache();
- *discard_flag = 1;
- read_cache();
for (i = 0; i < q->nr; i++) {
struct diff_filespec *one = q->queue[i]->one;
@@ -179,17 +170,15 @@ static int read_from_tree(const char *prefix, const char **argv,
unsigned char *tree_sha1, int refresh_flags)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd, index_was_discarded = 0;
+ int index_fd;
struct diff_options opt;
memset(&opt, 0, sizeof(opt));
diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- opt.format_callback_data = &index_was_discarded;
index_fd = hold_locked_index(lock, 1);
- index_was_discarded = 0;
read_cache();
if (do_diff_cache(tree_sha1, &opt))
return 1;
@@ -197,9 +186,6 @@ static int read_from_tree(const char *prefix, const char **argv,
diff_flush(&opt);
diff_tree_release_paths(&opt);
- if (!index_was_discarded)
- /* The index is still clobbered from do_diff_cache() */
- discard_cache();
return update_index_refresh(index_fd, lock, refresh_flags);
}
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
We use the path arguments in two places in reset.c: in
interactive_reset() and read_from_tree(). Both of these call
get_pathspec(), so we pass the (prefix, argv) pair to both
functions. Move the call to get_pathspec() out of these methods, for
two reasons: 1) One argument is simpler than two. 2) It lets us use
the (arguably clearer) "if (pathspec)" in place of "if (i < argc)".
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 65413d0..045c960 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -153,26 +153,15 @@ static void update_index_from_diff(struct diff_queue_struct *q,
}
}
-static int interactive_reset(const char *revision, const char **argv,
- const char *prefix)
-{
- const char **pathspec = NULL;
-
- if (*argv)
- pathspec = get_pathspec(prefix, argv);
-
- return run_add_interactive(revision, "--patch=reset", pathspec);
-}
-
-static int read_from_tree(const char *prefix, const char **argv,
- unsigned char *tree_sha1, int refresh_flags)
+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
+ int refresh_flags)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int index_fd;
struct diff_options opt;
memset(&opt, 0, sizeof(opt));
- diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);
+ diff_tree_setup_paths(pathspec, &opt);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
@@ -216,6 +205,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
const char *rev = "HEAD";
unsigned char sha1[20], *orig = NULL, sha1_orig[20],
*old_orig = NULL, sha1_old_orig[20];
+ const char **pathspec = NULL;
struct commit *commit;
struct strbuf msg = STRBUF_INIT;
const struct option options[] = {
@@ -287,22 +277,25 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die(_("Could not parse object '%s'."), rev);
hashcpy(sha1, commit->object.sha1);
+ if (i < argc)
+ pathspec = get_pathspec(prefix, argv + i);
+
if (patch_mode) {
if (reset_type != NONE)
die(_("--patch is incompatible with --{hard,mixed,soft}"));
- return interactive_reset(rev, argv + i, prefix);
+ return run_add_interactive(rev, "--patch=reset", pathspec);
}
/* git reset tree [--] paths... can be used to
* load chosen paths from the tree into the index without
* affecting the working tree nor HEAD. */
- if (i < argc) {
+ if (pathspec) {
if (reset_type == MIXED)
warning(_("--mixed with paths is deprecated; use 'git reset -- <paths>' instead."));
else if (reset_type != NONE)
die(_("Cannot do %s reset with paths."),
_(reset_type_names[reset_type]));
- return read_from_tree(prefix, argv + i, sha1,
+ return read_from_tree(pathspec, sha1,
quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
}
if (reset_type == NONE)
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 04/19] reset: don't allow "git reset -- $pathspec" in bare repo
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
Running e.g. "git reset ." in a bare repo results in an index file
being created from the HEAD commit. The differences compared to the
index are then printed as usual, but since there is no worktree, it
will appear as if all files are deleted. For example, in a bare clone
of git.git:
Unstaged changes after reset:
D .gitattributes
D .gitignore
D .mailmap
...
This happens because the check for is_bare_repository() happens after
we branch off into read_from_tree() to reset with paths. Fix by moving
the branching point after the check.
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 045c960..664fad9 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -295,8 +295,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
else if (reset_type != NONE)
die(_("Cannot do %s reset with paths."),
_(reset_type_names[reset_type]));
- return read_from_tree(pathspec, sha1,
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
}
if (reset_type == NONE)
reset_type = MIXED; /* by default */
@@ -308,6 +306,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die(_("%s reset is not allowed in a bare repository"),
_(reset_type_names[reset_type]));
+ if (pathspec)
+ return read_from_tree(pathspec, sha1,
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+
/* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order. Other resets reset
* the index file to the tree object we are switching to. */
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 17/19] reset $sha1 $pathspec: require $sha1 only to be treeish
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
Resetting with paths does not update HEAD and there is nothing else
that a commit should be needed for. Relax the argument parsing so only
a tree is required.
The sha1 is only passed to read_from_tree(), which already only
requires a tree.
The "rev" variable we pass to run_add_interactive() will resolve to a
tree. This is fine since interactive_reset only needs the parameter to
be a treeish and doesn't use it for display purposes.
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 48 +++++++++++++++++++++++++++---------------------
t/t7102-reset.sh | 8 ++++++++
2 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 520c1a5..b776867 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -178,9 +178,10 @@ static const char **parse_args(const char **argv, const char *prefix, const char
/*
* Possible arguments are:
*
- * git reset [-opts] <rev> <paths>...
- * git reset [-opts] <rev> -- <paths>...
- * git reset [-opts] -- <paths>...
+ * git reset [-opts] [<rev>]
+ * git reset [-opts] <tree> [<paths>...]
+ * git reset [-opts] <tree> -- [<paths>...]
+ * git reset [-opts] -- [<paths>...]
* git reset [-opts] <paths>...
*
* At this point, argv points immediately after [-opts].
@@ -195,11 +196,13 @@ static const char **parse_args(const char **argv, const char *prefix, const char
}
/*
* Otherwise, argv[0] could be either <rev> or <paths> and
- * has to be unambiguous.
+ * has to be unambiguous. If there is a single argument, it
+ * can not be a tree
*/
- else if (!get_sha1_committish(argv[0], unused)) {
+ else if ((!argv[1] && !get_sha1_committish(argv[0], unused)) ||
+ (argv[1] && !get_sha1_treeish(argv[0], unused))) {
/*
- * Ok, argv[0] looks like a rev; it should not
+ * Ok, argv[0] looks like a commit/tree; it should not
* be a filename.
*/
verify_non_filename(prefix, argv[0]);
@@ -241,7 +244,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
const char *rev;
unsigned char sha1[20];
const char **pathspec = NULL;
- struct commit *commit;
const struct option options[] = {
OPT__QUIET(&quiet, N_("be quiet, only report errors")),
OPT_SET_INT(0, "mixed", &reset_type,
@@ -263,19 +265,23 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
PARSE_OPT_KEEP_DASHDASH);
pathspec = parse_args(argv, prefix, &rev);
- if (get_sha1_committish(rev, sha1))
- die(_("Failed to resolve '%s' as a valid ref."), rev);
-
- /*
- * NOTE: As "git reset $treeish -- $path" should be usable on
- * any tree-ish, this is not strictly correct. We are not
- * moving the HEAD to any commit; we are merely resetting the
- * entries in the index to that of a treeish.
- */
- commit = lookup_commit_reference(sha1);
- if (!commit)
- die(_("Could not parse object '%s'."), rev);
- hashcpy(sha1, commit->object.sha1);
+ if (!pathspec) {
+ struct commit *commit;
+ if (get_sha1_committish(rev, sha1))
+ die(_("Failed to resolve '%s' as a valid revision."), rev);
+ commit = lookup_commit_reference(sha1);
+ if (!commit)
+ die(_("Could not parse object '%s'."), rev);
+ hashcpy(sha1, commit->object.sha1);
+ } else {
+ struct tree *tree;
+ if (get_sha1_treeish(rev, sha1))
+ die(_("Failed to resolve '%s' as a valid tree."), rev);
+ tree = parse_tree_indirect(sha1);
+ if (!tree)
+ die(_("Could not parse object '%s'."), rev);
+ hashcpy(sha1, tree->object.sha1);
+ }
if (patch_mode) {
if (reset_type != NONE)
@@ -340,7 +346,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
update_ref_status = update_refs(rev, sha1);
if (reset_type == HARD && !update_ref_status && !quiet)
- print_new_head_line(commit);
+ print_new_head_line(lookup_commit_reference(sha1));
remove_branch_state();
}
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 81b2570..1fa2a5f 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -497,4 +497,12 @@ test_expect_success 'disambiguation (4)' '
test ! -f secondfile
'
+test_expect_success 'reset with paths accepts tree' '
+ # for simpler tests, drop last commit containing added files
+ git reset --hard HEAD^ &&
+ git reset HEAD^^{tree} -- . &&
+ git diff --cached HEAD^ --exit-code &&
+ git diff HEAD --exit-code
+'
+
test_done
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 19/19] reset [--mixed]: use diff-based reset whether or not pathspec was given
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
Thanks to b65982b (Optimize "diff-index --cached" using cache-tree,
2009-05-20), resetting with paths is much faster than resetting
without paths. Some timings for the linux-2.6 repo to illustrate this
(best of five, warm cache):
reset reset .
real 0m0.219s 0m0.080s
user 0m0.140s 0m0.040s
sys 0m0.070s 0m0.030s
These two commands should do the same thing, so instead of having the
user type the trailing " ." to get the faster do_diff_cache()-based
implementation, always use it when doing a mixed reset, with or
without paths (so "git reset $rev" would also be faster).
Timing "git reset" shows that it indeed becomes as fast as
"git reset ." after this patch.
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
It seems like a better solution would be for unpack_trees() learn the
same tricks as do_diff_cache(). I'm leaving that a challange for the
reader :-). I did have a look a unpack_trees(), but it looked rather
overwhelming.
builtin/reset.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 45b01eb..921afbe 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -322,7 +322,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (reset_type != SOFT) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
- if (pathspec) {
+ if (reset_type == MIXED) {
if (read_from_tree(pathspec, sha1))
return 1;
} else {
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 02/19] reset $pathspec: exit with code 0 if successful
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
"git reset $pathspec" currently exits with a non-zero exit code if the
worktree is dirty after resetting, which is inconsistent with reset
without pathspec, and it makes it harder to know whether the command
really failed. Change it to exit with code 0 regardless of whether the
worktree is dirty so that non-zero indicates an error.
This makes the 4 "disambiguation" test cases in t7102 clearer since
they all used to "fail", 3 of which "failed" due to changes in the
work tree. Now only the ambiguous one fails.
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 8 +++-----
t/t2013-checkout-submodule.sh | 2 +-
t/t7102-reset.sh | 18 ++++++++++++------
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 8cc7c72..65413d0 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -119,19 +119,17 @@ static void print_new_head_line(struct commit *commit)
static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)
{
- int result;
-
if (!index_lock) {
index_lock = xcalloc(1, sizeof(struct lock_file));
fd = hold_locked_index(index_lock, 1);
}
- result = refresh_index(&the_index, (flags), NULL, NULL,
- _("Unstaged changes after reset:")) ? 1 : 0;
+ refresh_index(&the_index, (flags), NULL, NULL,
+ _("Unstaged changes after reset:"));
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(index_lock))
return error ("Could not refresh index");
- return result;
+ return 0;
}
static void update_index_from_diff(struct diff_queue_struct *q,
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 70edbb3..06b18f8 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -23,7 +23,7 @@ test_expect_success '"reset <submodule>" updates the index' '
git update-index --refresh &&
git diff-files --quiet &&
git diff-index --quiet --cached HEAD &&
- test_must_fail git reset HEAD^ submodule &&
+ git reset HEAD^ submodule &&
test_must_fail git diff-files --quiet &&
git reset submodule &&
git diff-files --quiet
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index b096dc8..81b2570 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -388,7 +388,8 @@ test_expect_success 'test --mixed <paths>' '
echo 4 > file4 &&
echo 5 > file1 &&
git add file1 file3 file4 &&
- test_must_fail git reset HEAD -- file1 file2 file3 &&
+ git reset HEAD -- file1 file2 file3 &&
+ test_must_fail git diff --quiet &&
git diff > output &&
test_cmp output expect &&
git diff --cached > output &&
@@ -402,7 +403,8 @@ test_expect_success 'test resetting the index at give paths' '
>sub/file2 &&
git update-index --add sub/file1 sub/file2 &&
T=$(git write-tree) &&
- test_must_fail git reset HEAD sub/file2 &&
+ git reset HEAD sub/file2 &&
+ test_must_fail git diff --quiet &&
U=$(git write-tree) &&
echo "$T" &&
echo "$U" &&
@@ -440,7 +442,8 @@ test_expect_success 'resetting specific path that is unmerged' '
echo "100644 $F3 3 file2"
} | git update-index --index-info &&
git ls-files -u &&
- test_must_fail git reset HEAD file2 &&
+ git reset HEAD file2 &&
+ test_must_fail git diff --quiet &&
git diff-index --exit-code --cached HEAD
'
@@ -449,7 +452,8 @@ test_expect_success 'disambiguation (1)' '
git reset --hard &&
>secondfile &&
git add secondfile &&
- test_must_fail git reset secondfile &&
+ git reset secondfile &&
+ test_must_fail git diff --quiet -- secondfile &&
test -z "$(git diff --cached --name-only)" &&
test -f secondfile &&
test ! -s secondfile
@@ -474,7 +478,8 @@ test_expect_success 'disambiguation (3)' '
>secondfile &&
git add secondfile &&
rm -f secondfile &&
- test_must_fail git reset HEAD secondfile &&
+ git reset HEAD secondfile &&
+ test_must_fail git diff --quiet &&
test -z "$(git diff --cached --name-only)" &&
test ! -f secondfile
@@ -486,7 +491,8 @@ test_expect_success 'disambiguation (4)' '
>secondfile &&
git add secondfile &&
rm -f secondfile &&
- test_must_fail git reset -- secondfile &&
+ git reset -- secondfile &&
+ test_must_fail git diff --quiet &&
test -z "$(git diff --cached --name-only)" &&
test ! -f secondfile
'
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
By not returning from inside the "if (pathspec)" block, we can let the
pathspec-aware and pathspec-less code share a bit more, making it
easier to make future changes that should affect both cases. This also
highlights the similarity between read_from_tree() and reset_index().
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 42 ++++++++++++++++++------------------------
1 file changed, 18 insertions(+), 24 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index e8a3e41..c316d9b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -309,19 +309,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die(_("%s reset is not allowed in a bare repository"),
_(reset_type_names[reset_type]));
- if (pathspec) {
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd = hold_locked_index(lock, 1);
- if (read_from_tree(pathspec, sha1))
- return 1;
- update_index_refresh(
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- if (write_cache(index_fd, active_cache, active_nr) ||
- commit_locked_index(lock))
- return error("Could not write new index file.");
- return 0;
- }
-
/* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order. Other resets reset
* the index file to the tree object we are switching to. */
@@ -331,11 +318,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (reset_type != SOFT) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
- int err = reset_index(sha1, reset_type, quiet);
- if (reset_type == KEEP && !err)
- err = reset_index(sha1, MIXED, quiet);
- if (err)
- die(_("Could not reset index file to revision '%s'."), rev);
+ if (pathspec) {
+ if (read_from_tree(pathspec, sha1))
+ return 1;
+ } else {
+ int err = reset_index(sha1, reset_type, quiet);
+ if (reset_type == KEEP && !err)
+ err = reset_index(sha1, MIXED, quiet);
+ if (err)
+ die(_("Could not reset index file to revision '%s'."), rev);
+ }
if (reset_type == MIXED) /* Report what has not been updated. */
update_index_refresh(
@@ -346,14 +338,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die(_("Could not write new index file."));
}
- /* Any resets update HEAD to the head being switched to,
- * saving the previous head in ORIG_HEAD before. */
- update_ref_status = update_refs(rev, sha1);
+ if (!pathspec) {
+ /* Any resets without paths update HEAD to the head being
+ * switched to, saving the previous head in ORIG_HEAD before. */
+ update_ref_status = update_refs(rev, sha1);
- if (reset_type == HARD && !update_ref_status && !quiet)
- print_new_head_line(commit);
+ if (reset_type == HARD && !update_ref_status && !quiet)
+ print_new_head_line(commit);
- remove_branch_state();
+ remove_branch_state();
+ }
return update_ref_status;
}
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 18/19] reset: allow reset on unborn branch
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
Some users seem to think, knowingly or not, that being on an unborn
branch is like having a commit with an empty tree checked out, but
when run on an unborn branch, "git reset" currently fails with:
fatal: Failed to resolve 'HEAD' as a valid ref.
Instead of making users figure out that they should run
git rm --cached -r .
, let's teach "git reset" without a revision argument, when on an
unborn branch, to behave as if the user asked to reset to an empty
tree. Don't take the analogy with an empty commit too far, though, but
still disallow explictly referring to HEAD in "git reset HEAD".
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 16 ++++++++-----
t/t7106-reset-unborn-branch.sh | 52 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 6 deletions(-)
create mode 100755 t/t7106-reset-unborn-branch.sh
diff --git a/builtin/reset.c b/builtin/reset.c
index b776867..45b01eb 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -240,7 +240,7 @@ static int update_refs(const char *rev, const unsigned char *sha1)
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
- int patch_mode = 0;
+ int patch_mode = 0, unborn;
const char *rev;
unsigned char sha1[20];
const char **pathspec = NULL;
@@ -265,7 +265,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
PARSE_OPT_KEEP_DASHDASH);
pathspec = parse_args(argv, prefix, &rev);
- if (!pathspec) {
+ unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", sha1);
+ if (unborn) {
+ /* reset on unborn branch: treat as reset to empty tree */
+ hashcpy(sha1, EMPTY_TREE_SHA1_BIN);
+ } else if (!pathspec) {
struct commit *commit;
if (get_sha1_committish(rev, sha1))
die(_("Failed to resolve '%s' as a valid revision."), rev);
@@ -286,7 +290,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (patch_mode) {
if (reset_type != NONE)
die(_("--patch is incompatible with --{hard,mixed,soft}"));
- return run_add_interactive(rev, "--patch=reset", pathspec);
+ return run_add_interactive(sha1_to_hex(sha1), "--patch=reset", pathspec);
}
/* git reset tree [--] paths... can be used to
@@ -340,16 +344,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die(_("Could not write new index file."));
}
- if (!pathspec) {
+ if (!pathspec && !unborn) {
/* Any resets without paths update HEAD to the head being
* switched to, saving the previous head in ORIG_HEAD before. */
update_ref_status = update_refs(rev, sha1);
if (reset_type == HARD && !update_ref_status && !quiet)
print_new_head_line(lookup_commit_reference(sha1));
-
- remove_branch_state();
}
+ if (!pathspec)
+ remove_branch_state();
return update_ref_status;
}
diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh
new file mode 100755
index 0000000..8062cf5
--- /dev/null
+++ b/t/t7106-reset-unborn-branch.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='git reset should work on unborn branch'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo a >a &&
+ echo b >b
+'
+
+test_expect_success 'reset' '
+ git add a b &&
+ git reset &&
+ test "$(git ls-files)" = ""
+'
+
+test_expect_success 'reset HEAD' '
+ rm .git/index &&
+ git add a b &&
+ test_must_fail git reset HEAD
+'
+
+test_expect_success 'reset $file' '
+ rm .git/index &&
+ git add a b &&
+ git reset a &&
+ test "$(git ls-files)" = "b"
+'
+
+test_expect_success 'reset -p' '
+ rm .git/index &&
+ git add a &&
+ echo y | git reset -p &&
+ test "$(git ls-files)" = ""
+'
+
+test_expect_success 'reset --soft is a no-op' '
+ rm .git/index &&
+ git add a &&
+ git reset --soft
+ test "$(git ls-files)" = "a"
+'
+
+test_expect_success 'reset --hard' '
+ rm .git/index &&
+ git add a &&
+ git reset --hard &&
+ test "$(git ls-files)" = "" &&
+ test_path_is_missing a
+'
+
+test_done
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 14/19] reset [--mixed]: only write index file once
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
When doing a mixed reset without paths, the index is locked, read,
reset, and written back as part of the actual reset operation (in
reset_index()). Then, when showing the list of worktree modifications,
we lock the index again, refresh it, and write it.
Change this so we only write the index once, making "git reset" a
little faster. It does mean that the index lock will be held a little
longer, but the difference is small compared to the time spent
refreshing the index.
There is one minor functional difference: We used to say "Could not
write new index file." if the first write failed, and "Could not
refresh index" if the second write failed. Now, we will only use the
first message.
This speeds up "git reset" a little on the linux-2.6 repo (best of
five, warm cache):
Before After
real 0m0.239s 0m0.214s
user 0m0.160s 0m0.130s
sys 0m0.070s 0m0.080s
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index c1d6ef2..e8a3e41 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -336,6 +336,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
err = reset_index(sha1, MIXED, quiet);
if (err)
die(_("Could not reset index file to revision '%s'."), rev);
+
+ if (reset_type == MIXED) /* Report what has not been updated. */
+ update_index_refresh(
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
die(_("Could not write new index file."));
@@ -347,15 +352,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (reset_type == HARD && !update_ref_status && !quiet)
print_new_head_line(commit);
- else if (reset_type == MIXED) { /* Report what has not been updated. */
- struct lock_file *index_lock = xcalloc(1, sizeof(struct lock_file));
- int fd = hold_locked_index(index_lock, 1);
- update_index_refresh(
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- if (write_cache(fd, active_cache, active_nr) ||
- commit_locked_index(index_lock))
- error("Could not refresh index");
- }
remove_branch_state();
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 10/19] reset: avoid redundant error message
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
If writing or committing the new index file fails, we print "Could not
write new index file." followed by "Could not reset index file to
revision $rev.". The first message seems to imply the second, so print
only the first message.
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 7c440ad..97fa9f7 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -338,13 +338,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
int err = reset_index(sha1, reset_type, quiet);
if (reset_type == KEEP && !err)
err = reset_index(sha1, MIXED, quiet);
- if (!err &&
- (write_cache(newfd, active_cache, active_nr) ||
- commit_locked_index(lock))) {
- err = error(_("Could not write new index file."));
- }
if (err)
die(_("Could not reset index file to revision '%s'."), rev);
+ if (write_cache(newfd, active_cache, active_nr) ||
+ commit_locked_index(lock))
+ die(_("Could not write new index file."));
}
/* Any resets update HEAD to the head being switched to,
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 05/19] reset.c: extract function for parsing arguments
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
Declutter cmd_reset() a bit by moving out the argument parsing to its
own function.
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 70 +++++++++++++++++++++++++++++++--------------------------
1 file changed, 38 insertions(+), 32 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 664fad9..58f0f61 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -198,36 +198,11 @@ static void die_if_unmerged_cache(int reset_type)
}
-int cmd_reset(int argc, const char **argv, const char *prefix)
+static const char **parse_args(int argc, const char **argv, const char *prefix, const char **rev_ret)
{
- int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
- int patch_mode = 0;
+ int i = 0;
const char *rev = "HEAD";
- unsigned char sha1[20], *orig = NULL, sha1_orig[20],
- *old_orig = NULL, sha1_old_orig[20];
- const char **pathspec = NULL;
- struct commit *commit;
- struct strbuf msg = STRBUF_INIT;
- const struct option options[] = {
- OPT__QUIET(&quiet, N_("be quiet, only report errors")),
- OPT_SET_INT(0, "mixed", &reset_type,
- N_("reset HEAD and index"), MIXED),
- OPT_SET_INT(0, "soft", &reset_type, N_("reset only HEAD"), SOFT),
- OPT_SET_INT(0, "hard", &reset_type,
- N_("reset HEAD, index and working tree"), HARD),
- OPT_SET_INT(0, "merge", &reset_type,
- N_("reset HEAD, index and working tree"), MERGE),
- OPT_SET_INT(0, "keep", &reset_type,
- N_("reset HEAD but keep local changes"), KEEP),
- OPT_BOOLEAN('p', "patch", &patch_mode, N_("select hunks interactively")),
- OPT_END()
- };
-
- git_config(git_default_config, NULL);
-
- argc = parse_options(argc, argv, prefix, options, git_reset_usage,
- PARSE_OPT_KEEP_DASHDASH);
-
+ unsigned char unused[20];
/*
* Possible arguments are:
*
@@ -250,7 +225,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
* Otherwise, argv[i] could be either <rev> or <paths> and
* has to be unambiguous.
*/
- else if (!get_sha1_committish(argv[i], sha1)) {
+ else if (!get_sha1_committish(argv[i], unused)) {
/*
* Ok, argv[i] looks like a rev; it should not
* be a filename.
@@ -262,6 +237,40 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
verify_filename(prefix, argv[i], 1);
}
}
+ *rev_ret = rev;
+ return i < argc ? get_pathspec(prefix, argv + i) : NULL;
+}
+
+int cmd_reset(int argc, const char **argv, const char *prefix)
+{
+ int reset_type = NONE, update_ref_status = 0, quiet = 0;
+ int patch_mode = 0;
+ const char *rev;
+ unsigned char sha1[20], *orig = NULL, sha1_orig[20],
+ *old_orig = NULL, sha1_old_orig[20];
+ const char **pathspec = NULL;
+ struct commit *commit;
+ struct strbuf msg = STRBUF_INIT;
+ const struct option options[] = {
+ OPT__QUIET(&quiet, N_("be quiet, only report errors")),
+ OPT_SET_INT(0, "mixed", &reset_type,
+ N_("reset HEAD and index"), MIXED),
+ OPT_SET_INT(0, "soft", &reset_type, N_("reset only HEAD"), SOFT),
+ OPT_SET_INT(0, "hard", &reset_type,
+ N_("reset HEAD, index and working tree"), HARD),
+ OPT_SET_INT(0, "merge", &reset_type,
+ N_("reset HEAD, index and working tree"), MERGE),
+ OPT_SET_INT(0, "keep", &reset_type,
+ N_("reset HEAD but keep local changes"), KEEP),
+ OPT_BOOLEAN('p', "patch", &patch_mode, N_("select hunks interactively")),
+ OPT_END()
+ };
+
+ git_config(git_default_config, NULL);
+
+ argc = parse_options(argc, argv, prefix, options, git_reset_usage,
+ PARSE_OPT_KEEP_DASHDASH);
+ pathspec = parse_args(argc, argv, prefix, &rev);
if (get_sha1_committish(rev, sha1))
die(_("Failed to resolve '%s' as a valid ref."), rev);
@@ -277,9 +286,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die(_("Could not parse object '%s'."), rev);
hashcpy(sha1, commit->object.sha1);
- if (i < argc)
- pathspec = get_pathspec(prefix, argv + i);
-
if (patch_mode) {
if (reset_type != NONE)
die(_("--patch is incompatible with --{hard,mixed,soft}"));
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 13/19] reset.c: move lock, write and commit out of update_index_refresh()
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
In preparation for the/a following patch, move the locking, writing
and committing of the index file out of update_index_refresh(). The
code duplication caused will soon be taken care of. What remains of
update_index_refresh() is just one line, but it is still called from
two places, so let's leave it for now.
In the process, we expose and fix the minor UI bug that makes us print
"Could not refresh index" when we fail to write the index file when
invoked with a pathspec. Copy the error message from the pathspec-less
codepath ("Could not write new index file.").
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 70733c2..c1d6ef2 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,19 +109,10 @@ static void print_new_head_line(struct commit *commit)
printf("\n");
}
-static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)
+static void update_index_refresh(int flags)
{
- if (!index_lock) {
- index_lock = xcalloc(1, sizeof(struct lock_file));
- fd = hold_locked_index(index_lock, 1);
- }
-
refresh_index(&the_index, (flags), NULL, NULL,
_("Unstaged changes after reset:"));
- if (write_cache(fd, active_cache, active_nr) ||
- commit_locked_index(index_lock))
- return error ("Could not refresh index");
- return 0;
}
static void update_index_from_diff(struct diff_queue_struct *q,
@@ -321,9 +312,14 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (pathspec) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int index_fd = hold_locked_index(lock, 1);
- return read_from_tree(pathspec, sha1) ||
- update_index_refresh(index_fd, lock,
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (read_from_tree(pathspec, sha1))
+ return 1;
+ update_index_refresh(
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (write_cache(index_fd, active_cache, active_nr) ||
+ commit_locked_index(lock))
+ return error("Could not write new index file.");
+ return 0;
}
/* Soft reset does not touch the index file nor the working tree
@@ -351,9 +347,15 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (reset_type == HARD && !update_ref_status && !quiet)
print_new_head_line(commit);
- else if (reset_type == MIXED) /* Report what has not been updated. */
- update_index_refresh(0, NULL,
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ else if (reset_type == MIXED) { /* Report what has not been updated. */
+ struct lock_file *index_lock = xcalloc(1, sizeof(struct lock_file));
+ int fd = hold_locked_index(index_lock, 1);
+ update_index_refresh(
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (write_cache(fd, active_cache, active_nr) ||
+ commit_locked_index(index_lock))
+ error("Could not refresh index");
+ }
remove_branch_state();
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 11/19] reset.c: replace switch by if-else
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
The switch statement towards the end of reset.c is missing case arms
for KEEP and MERGE for no obvious reason, and soon the only non-empty
case arm will be the one for HARD. So let's proactively replace it by
if-else, which will let us move one if statement out without leaving
funny-looking left-overs.
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 97fa9f7..c3eb2eb 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -349,18 +349,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
* saving the previous head in ORIG_HEAD before. */
update_ref_status = update_refs(rev, sha1);
- switch (reset_type) {
- case HARD:
- if (!update_ref_status && !quiet)
- print_new_head_line(commit);
- break;
- case SOFT: /* Nothing else to do. */
- break;
- case MIXED: /* Report what has not been updated. */
+ if (reset_type == HARD && !update_ref_status && !quiet)
+ print_new_head_line(commit);
+ else if (reset_type == MIXED) /* Report what has not been updated. */
update_index_refresh(0, NULL,
quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- break;
- }
remove_branch_state();
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 16/19] reset.c: inline update_index_refresh()
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
Now that there is only one caller left to the single-line method
update_index_refresh(), inline it.
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index c316d9b..520c1a5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,12 +109,6 @@ static void print_new_head_line(struct commit *commit)
printf("\n");
}
-static void update_index_refresh(int flags)
-{
- refresh_index(&the_index, (flags), NULL, NULL,
- _("Unstaged changes after reset:"));
-}
-
static void update_index_from_diff(struct diff_queue_struct *q,
struct diff_options *opt, void *data)
{
@@ -329,9 +323,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die(_("Could not reset index file to revision '%s'."), rev);
}
- if (reset_type == MIXED) /* Report what has not been updated. */
- update_index_refresh(
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (reset_type == MIXED) { /* Report what has not been updated. */
+ int flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;
+ refresh_index(&the_index, flags, NULL, NULL,
+ _("Unstaged changes after reset:"));
+ }
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 07/19] reset.c: extract function for updating {ORIG_,}HEAD
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
By extracting the code for updating the HEAD and ORIG_HEAD symbolic
references to a separate function, we declutter cmd_reset() a bit and
we make it clear that e.g. the four variables {,sha1_}{,old_}orig are
only used by this code.
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index d89cf4d..2187d64 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -240,16 +240,35 @@ static const char **parse_args(const char **argv, const char *prefix, const char
return argv[0] ? get_pathspec(prefix, argv) : NULL;
}
+static int update_refs(const char *rev, const unsigned char *sha1)
+{
+ int update_ref_status;
+ struct strbuf msg = STRBUF_INIT;
+ unsigned char *orig = NULL, sha1_orig[20],
+ *old_orig = NULL, sha1_old_orig[20];
+
+ if (!get_sha1("ORIG_HEAD", sha1_old_orig))
+ old_orig = sha1_old_orig;
+ if (!get_sha1("HEAD", sha1_orig)) {
+ orig = sha1_orig;
+ set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
+ update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
+ } else if (old_orig)
+ delete_ref("ORIG_HEAD", old_orig, 0);
+ set_reflog_message(&msg, "updating HEAD", rev);
+ update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR);
+ strbuf_release(&msg);
+ return update_ref_status;
+}
+
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
int patch_mode = 0;
const char *rev;
- unsigned char sha1[20], *orig = NULL, sha1_orig[20],
- *old_orig = NULL, sha1_old_orig[20];
+ unsigned char sha1[20];
const char **pathspec = NULL;
struct commit *commit;
- struct strbuf msg = STRBUF_INIT;
const struct option options[] = {
OPT__QUIET(&quiet, N_("be quiet, only report errors")),
OPT_SET_INT(0, "mixed", &reset_type,
@@ -333,17 +352,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
/* Any resets update HEAD to the head being switched to,
* saving the previous head in ORIG_HEAD before. */
- if (!get_sha1("ORIG_HEAD", sha1_old_orig))
- old_orig = sha1_old_orig;
- if (!get_sha1("HEAD", sha1_orig)) {
- orig = sha1_orig;
- set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
- update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
- }
- else if (old_orig)
- delete_ref("ORIG_HEAD", old_orig, 0);
- set_reflog_message(&msg, "updating HEAD", rev);
- update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR);
+ update_ref_status = update_refs(rev, sha1);
switch (reset_type) {
case HARD:
@@ -360,7 +369,5 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
remove_branch_state();
- strbuf_release(&msg);
-
return update_ref_status;
}
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 09/19] reset --keep: only write index file once
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
"git reset --keep" calls reset_index_file() twice, first doing a
two-way merge to the target revision, updating the index and worktree,
and then resetting the index. After each call, we write the index
file.
In the unlikely event that the second call to reset_index_file()
fails, the index will have been merged to the target revision, but
HEAD will not be updated, leaving the user with a dirty index.
By moving the locking, writing and committing out of
reset_index_file() and into the caller, we can avoid writing the index
twice, thereby making the sure we don't end up in the half-way reset
state. As a bonus, we speed up "git reset --keep" a little on the
linux-2.6 repo (best of five, warm cache):
Before After
real 0m0.315s 0m0.296s
user 0m0.290s 0m0.280s
sys 0m0.020s 0m0.010s
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4e34195..7c440ad 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -38,14 +38,12 @@ static inline int is_merge(void)
return !access(git_path("MERGE_HEAD"), F_OK);
}
-static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet)
+static int reset_index(const unsigned char *sha1, int reset_type, int quiet)
{
int nr = 1;
- int newfd;
struct tree_desc desc[2];
struct tree *tree;
struct unpack_trees_options opts;
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
memset(&opts, 0, sizeof(opts));
opts.head_idx = 1;
@@ -67,8 +65,6 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
opts.reset = 1;
}
- newfd = hold_locked_index(lock, 1);
-
read_cache_unmerged();
if (reset_type == KEEP) {
@@ -91,10 +87,6 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
prime_cache_tree(&active_cache_tree, tree);
}
- if (write_cache(newfd, active_cache, active_nr) ||
- commit_locked_index(lock))
- return error(_("Could not write new index file."));
-
return 0;
}
@@ -341,9 +333,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die_if_unmerged_cache(reset_type);
if (reset_type != SOFT) {
- int err = reset_index_file(sha1, reset_type, quiet);
+ struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+ int newfd = hold_locked_index(lock, 1);
+ int err = reset_index(sha1, reset_type, quiet);
if (reset_type == KEEP && !err)
- err = reset_index_file(sha1, MIXED, quiet);
+ err = reset_index(sha1, MIXED, quiet);
+ if (!err &&
+ (write_cache(newfd, active_cache, active_nr) ||
+ commit_locked_index(lock))) {
+ err = error(_("Could not write new index file."));
+ }
if (err)
die(_("Could not reset index file to revision '%s'."), rev);
}
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 00/19] reset improvements
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1357719376-16406-1-git-send-email-martinvonz@gmail.com>
Changes since v1:
- Spelling fixes.
- Explained how "git reset -- $pathspec" in bare repo is broken.
- Provided motivation for replacement of switch by if-else
- Fixed argv/argc handling by removing use of argc.
- Replaced "don't refresh index on --quiet" patch by one that just
inlines update_index_refresh()
- Incorporated fixes from Junio's repo
- Provided some motivation for "replace switch by if-else" amd moved
the patch later in the series.
Thanks for reviewing!
Martin von Zweigbergk (19):
reset $pathspec: no need to discard index
reset $pathspec: exit with code 0 if successful
reset.c: pass pathspec around instead of (prefix, argv) pair
reset: don't allow "git reset -- $pathspec" in bare repo
reset.c: extract function for parsing arguments
reset.c: remove unnecessary variable 'i'
reset.c: extract function for updating {ORIG_,}HEAD
reset.c: share call to die_if_unmerged_cache()
reset --keep: only write index file once
reset: avoid redundant error message
reset.c: replace switch by if-else
reset.c: move update_index_refresh() call out of read_from_tree()
reset.c: move lock, write and commit out of update_index_refresh()
reset [--mixed]: only write index file once
reset.c: finish entire cmd_reset() whether or not pathspec is given
reset.c: inline update_index_refresh()
reset $sha1 $pathspec: require $sha1 only to be treeish
reset: allow reset on unborn branch
reset [--mixed]: use diff-based reset whether or not pathspec was
given
builtin/reset.c | 283 +++++++++++++++++++----------------------
t/t2013-checkout-submodule.sh | 2 +-
t/t7102-reset.sh | 26 +++-
t/t7106-reset-unborn-branch.sh | 52 ++++++++
4 files changed, 203 insertions(+), 160 deletions(-)
create mode 100755 t/t7106-reset-unborn-branch.sh
--
1.8.1.1.454.gce43f05
^ permalink raw reply
* [PATCH v2 12/19] reset.c: move update_index_refresh() call out of read_from_tree()
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
The final part of cmd_reset() essentially looks like:
if (pathspec) {
...
read_from_tree(...);
} else {
...
reset_index(...);
update_index_refresh(...);
...
}
where read_from_tree() internally also calls
update_index_refresh(). Move the call to update_index_refresh() out of
read_from_tree for symmetry with the 'else' block, making
read_from_tree() and reset_index() closer in functionality.
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index c3eb2eb..70733c2 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -145,11 +145,8 @@ static void update_index_from_diff(struct diff_queue_struct *q,
}
}
-static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
- int refresh_flags)
+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1)
{
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd;
struct diff_options opt;
memset(&opt, 0, sizeof(opt));
@@ -157,7 +154,6 @@ static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- index_fd = hold_locked_index(lock, 1);
read_cache();
if (do_diff_cache(tree_sha1, &opt))
return 1;
@@ -165,7 +161,7 @@ static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
diff_flush(&opt);
diff_tree_release_paths(&opt);
- return update_index_refresh(index_fd, lock, refresh_flags);
+ return 0;
}
static void set_reflog_message(struct strbuf *sb, const char *action,
@@ -322,9 +318,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die(_("%s reset is not allowed in a bare repository"),
_(reset_type_names[reset_type]));
- if (pathspec)
- return read_from_tree(pathspec, sha1,
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (pathspec) {
+ struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+ int index_fd = hold_locked_index(lock, 1);
+ return read_from_tree(pathspec, sha1) ||
+ update_index_refresh(index_fd, lock,
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ }
/* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order. Other resets reset
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* [PATCH v2 08/19] reset.c: share call to die_if_unmerged_cache()
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
Use a single condition to guard the call to die_if_unmerged_cache for
both --soft and --keep. This avoids the small distraction of the
precondition check from the logic following it.
Also change an instance of
if (e)
err = err || f();
to the almost as short, but clearer
if (e && !err)
err = f();
(which is equivalent since we only care whether exit code is 0)
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
builtin/reset.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 2187d64..4e34195 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -337,15 +337,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
/* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order. Other resets reset
* the index file to the tree object we are switching to. */
- if (reset_type == SOFT)
+ if (reset_type == SOFT || reset_type == KEEP)
die_if_unmerged_cache(reset_type);
- else {
- int err;
- if (reset_type == KEEP)
- die_if_unmerged_cache(reset_type);
- err = reset_index_file(sha1, reset_type, quiet);
- if (reset_type == KEEP)
- err = err || reset_index_file(sha1, MIXED, quiet);
+
+ if (reset_type != SOFT) {
+ int err = reset_index_file(sha1, reset_type, quiet);
+ if (reset_type == KEEP && !err)
+ err = reset_index_file(sha1, MIXED, quiet);
if (err)
die(_("Could not reset index file to revision '%s'."), rev);
}
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* Re: What's cooking in git.git (Jan 2013, #06; Mon, 14)
From: Chris Rorvick @ 2013-01-15 6:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eric S. Raymond, git, Michael Haggerty, Jonathan Nieder
In-Reply-To: <7v1udn6tdg.fsf@alter.siamese.dyndns.org>
On Mon, Jan 14, 2013 at 9:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I converted one of Chris's follow-up test tweaks to this to
> illustrate how it can be done without breaking tests for the
> original cvsimport, but didn't do all of them. Chris, is this a
> foundation we can work together on top?
Sure, looks straightforward and makes things easier.
Thanks,
Chris
^ permalink raw reply
* [PATCH v2 06/19] reset.c: remove unnecessary variable 'i'
From: Martin von Zweigbergk @ 2013-01-15 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen, Jeff King,
Martin von Zweigbergk
In-Reply-To: <1358228871-7142-1-git-send-email-martinvonz@gmail.com>
Throughout most of parse_args(), the variable 'i' remains at 0. Many
references are still made to the variable even when it could only have
the value 0. This made at least me, who has relatively little
experience with C programming styles, think that parts of the function
was meant to be part of a loop. To avoid such confusion, remove the
variable and also the 'argc' parameter and check for NULL trailing
argv instead.
Signed-off-by: Martin von Zweigbergk <martinvonz@gmail.com>
---
I explained a bit more why I was confused by the current style, but
I'm also perfectly happy if you just drop the patch (there would be
some minor conflicts in a later patch, though).
builtin/reset.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 58f0f61..d89cf4d 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -198,9 +198,8 @@ static void die_if_unmerged_cache(int reset_type)
}
-static const char **parse_args(int argc, const char **argv, const char *prefix, const char **rev_ret)
+static const char **parse_args(const char **argv, const char *prefix, const char **rev_ret)
{
- int i = 0;
const char *rev = "HEAD";
unsigned char unused[20];
/*
@@ -211,34 +210,34 @@ static const char **parse_args(int argc, const char **argv, const char *prefix,
* git reset [-opts] -- <paths>...
* git reset [-opts] <paths>...
*
- * At this point, argv[i] points immediately after [-opts].
+ * At this point, argv points immediately after [-opts].
*/
- if (i < argc) {
- if (!strcmp(argv[i], "--")) {
- i++; /* reset to HEAD, possibly with paths */
- } else if (i + 1 < argc && !strcmp(argv[i+1], "--")) {
- rev = argv[i];
- i += 2;
+ if (argv[0]) {
+ if (!strcmp(argv[0], "--")) {
+ argv++; /* reset to HEAD, possibly with paths */
+ } else if (argv[1] && !strcmp(argv[1], "--")) {
+ rev = argv[0];
+ argv += 2;
}
/*
- * Otherwise, argv[i] could be either <rev> or <paths> and
+ * Otherwise, argv[0] could be either <rev> or <paths> and
* has to be unambiguous.
*/
- else if (!get_sha1_committish(argv[i], unused)) {
+ else if (!get_sha1_committish(argv[0], unused)) {
/*
- * Ok, argv[i] looks like a rev; it should not
+ * Ok, argv[0] looks like a rev; it should not
* be a filename.
*/
- verify_non_filename(prefix, argv[i]);
- rev = argv[i++];
+ verify_non_filename(prefix, argv[0]);
+ rev = *argv++;
} else {
/* Otherwise we treat this as a filename */
- verify_filename(prefix, argv[i], 1);
+ verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
- return i < argc ? get_pathspec(prefix, argv + i) : NULL;
+ return argv[0] ? get_pathspec(prefix, argv) : NULL;
}
int cmd_reset(int argc, const char **argv, const char *prefix)
@@ -270,7 +269,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, options, git_reset_usage,
PARSE_OPT_KEEP_DASHDASH);
- pathspec = parse_args(argc, argv, prefix, &rev);
+ pathspec = parse_args(argv, prefix, &rev);
if (get_sha1_committish(rev, sha1))
die(_("Failed to resolve '%s' as a valid ref."), rev);
--
1.8.1.1.454.gce43f05
^ permalink raw reply related
* Re: [PATCH 3/3] cvsimport: start adding cvsps 3.x support
From: Chris Rorvick @ 2013-01-15 6:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, jrnieder, mhagger, esr
In-Reply-To: <1358127629-7500-4-git-send-email-gitster@pobox.com>
On Sun, Jan 13, 2013 at 7:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
> The new cvsps 3.x series lacks support of some options cvsps 2.x
> series had and used by cvsimport-2; add a replacement program from
> the author of cvsps 3.x and allow users to choose it by setting the
> GIT_CVSPS_VERSION environment variable to 3. We would later add
> support to auto-detect the version of installed cvsps to this code
> when the environment variable is not set.
>
> Note that in this step, cvsimport-3 that relies on cvsps 3.x does
> not have any test coverage. As cvsimport-3 supports most of the
> command line options cvsimport-2, we should be able to tweak some of
> t96xx tests and run them with GIT_CVSPS_VERSION set to both 2 and 3,
> but that is a topic of later patches that should come on top.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Makefile | 18 ++-
> git-cvsimport-3.py | 344 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> git-cvsimport.sh | 4 +-
> 3 files changed, 359 insertions(+), 7 deletions(-)
> create mode 100755 git-cvsimport-3.py
>
> diff --git a/Makefile b/Makefile
> index b022db2..060cdc2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -470,8 +470,9 @@ SCRIPT_PERL += git-relink.perl
> SCRIPT_PERL += git-send-email.perl
> SCRIPT_PERL += git-svn.perl
>
> -SCRIPT_PYTHON += git-remote-testpy.py
> +SCRIPT_PYTHON += git-cvsimport-3.py
> SCRIPT_PYTHON += git-p4.py
> +SCRIPT_PYTHON += git-remote-testpy.py
>
> SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
> $(patsubst %.perl,%,$(SCRIPT_PERL)) \
> @@ -575,8 +576,11 @@ endif
> ifndef CVSPS2_PATH
> CVSPS2_PATH = cvsps
> endif
> +ifndef CVSPS3_PATH
> + CVSPS3_PATH = cvsps
> +endif
>
> -export PERL_PATH PYTHON_PATH CVSPS2_PATH
> +export PERL_PATH PYTHON_PATH CVSPS2_PATH CVSPS3_PATH
>
> LIB_FILE = libgit.a
> XDIFF_LIB = xdiff/lib.a
> @@ -1515,6 +1519,7 @@ PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
> PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
> TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
> CVSPS2_PATH_SQ = $(subst ','\'',$(CVSPS2_PATH))
> +CVSPS3_PATH_SQ = $(subst ','\'',$(CVSPS3_PATH))
> DIFF_SQ = $(subst ','\'',$(DIFF))
>
> LIBS = $(GITLIBS) $(EXTLIBS)
> @@ -1757,12 +1762,15 @@ ifndef NO_PYTHON
> $(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
> $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py
> $(QUIET_GEN)$(RM) $@ $@+ && \
> - INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \
> + INSTLIBDIR_SQ=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \
> --no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' \
> - instlibdir` && \
> + instlibdir | \
> + sed -e "s/'/'\''/g"` && \
> + echo "InstLibDir is <$$INSTLIBDIR_SQ>" && \
> sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \
> -e 's|\(os\.getenv("GITPYTHONLIB"\)[^)]*)|\1,"@@INSTLIBDIR@@")|' \
> - -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR"'|g' \
> + -e 's|@@CVSPS3_PATH@@|$(CVSPS3_PATH_SQ)|g' \
> + -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR_SQ"'|g' \
> $@.py >$@+ && \
> chmod +x $@+ && \
> mv $@+ $@
> diff --git a/git-cvsimport-3.py b/git-cvsimport-3.py
> new file mode 100755
> index 0000000..57f13b7
> --- /dev/null
> +++ b/git-cvsimport-3.py
> @@ -0,0 +1,344 @@
> +#!/usr/bin/env python
> +#
> +# Import CVS history into git
> +#
> +# Intended to be a near-workalike of Matthias Urlichs's Perl implementation.
> +#
> +# By Eric S. Raymond <esr@thyrsus.com>, December 2012
> +# May be redistributed under the license of the git project.
> +
> +import sys
> +
> +if sys.hexversion < 0x02060000:
> + sys.stderr.write("git cvsimport: requires Python 2.6 or later.\n")
> + sys.exit(1)
> +
> +import os, getopt, subprocess, tempfile, shutil
> +
> +DEBUG_COMMANDS = 1
> +
> +class Fatal(Exception):
> + "Unrecoverable error."
> + def __init__(self, msg):
> + Exception.__init__(self)
> + self.msg = msg
> +
> +def do_or_die(dcmd, legend=""):
> + "Either execute a command or raise a fatal exception."
> + if legend:
> + legend = " " + legend
> + if verbose >= DEBUG_COMMANDS:
> + sys.stdout.write("git cvsimport: executing '%s'%s\n" % (dcmd, legend))
> + try:
> + retcode = subprocess.call(dcmd, shell=True)
> + if retcode < 0:
> + raise Fatal("git cvsimport: child was terminated by signal %d." % -retcode)
> + elif retcode != 0:
> + raise Fatal("git cvsimport: child returned %d." % retcode)
> + except (OSError, IOError) as e:
> + raise Fatal("git cvsimport: execution of %s%s failed: %s" % (dcmd, legend, e))
> +
> +def capture_or_die(dcmd, legend=""):
> + "Either execute a command and capture its output or die."
> + if legend:
> + legend = " " + legend
> + if verbose >= DEBUG_COMMANDS:
> + sys.stdout.write("git cvsimport: executing '%s'%s\n" % (dcmd, legend))
> + try:
> + return subprocess.check_output(dcmd, shell=True)
> + except subprocess.CalledProcessError as e:
> + if e.returncode < 0:
> + sys.stderr.write("git cvsimport: child was terminated by signal %d." % -e.returncode)
> + elif e.returncode != 0:
> + sys.stderr.write("git cvsimport: child returned %d." % e.returncode)
> + sys.exit(1)
> +
> +class cvsps:
> + "Method class for cvsps back end."
> +
> + cvsps = "@@CVSPS3_PATH@@"
> + def __init__(self):
> + self.opts = ""
> + self.revmap = None
> + def set_repo(self, val):
> + "Set the repository root option."
> + if not val.startswith(":"):
> + if not val.startswith(os.sep):
> + val = os.path.abspath(val)
> + val = ":local:" + val
> + self.opts += " --root '%s'" % val
> + def set_authormap(self, val):
> + "Set the author-map file."
> + self.opts += " -A '%s'" % val
> + def set_fuzz(self, val):
> + "Set the commit-similarity window."
> + self.opts += " -z %s" % val
> + def set_nokeywords(self):
> + "Suppress CVS keyword expansion."
> + self.opts += " -k"
> + def add_opts(self, val):
> + "Add options to the engine command line."
> + self.opts += " " + val
> + def set_exclusion(self, val):
> + "Set a file exclusion regexp."
> + self.opts += " -n -f '%s'" % val
> + def set_after(self, val):
> + "Set a date threshold for incremental import."
> + self.opts += " -d '%s'" % val
> + def set_revmap(self, val):
> + "Set the file to which the engine should dump a reference map."
> + self.revmap = val
> + self.opts += " -R '%s'" % self.revmap
> + def set_module(self, val):
> + "Set the module to query."
> + self.opts += " " + val
> + def command(self):
> + "Emit the command implied by all previous options."
> + return self.cvsps + "--fast-export " + self.opts
"--fast-export" string is missing a leading space. With this fix and
the latest cvsps build I'm seeing 6 of 15 failures for t9650 which is
what I was getting out of the patched t9600.
Chris
^ permalink raw reply
* Re: [BUG] Possible bug in `remote set-url --add --push`
From: Junio C Hamano @ 2013-01-15 6:22 UTC (permalink / raw)
To: Jardel Weyrich; +Cc: Michael J Gruber, Sascha Cunz, git@vger.kernel.org
In-Reply-To: <1D472234-A0A5-4F02-878D-D05DEE995FCD@gmail.com>
Jardel Weyrich <jweyrich@gmail.com> writes:
> If you allow me, I'd like you to forget about the concepts for a minute, and focus on the user experience.
> Imagine a simple hypothetical scenario in which the user wants to push to 2 distinct repositories. He already has cloned the repo from the 1st repository, thus (theoretically) all he needs to do, is to add a new repository for push. He then uses `remote set-url --add --push <2nd-repo>` (which I personally thought would suffice). However, if he tries to push a new commit to this remote, it would be pushed _only_ to the 2nd-repo.
The primary reason behind push-url was that
(1) usually you push to and fetch from the same, so no pushUrl is
ever needed, just a single Url will do (this is often true for
cvs/svn style shared repository workflow); and
(2) sometimes you want to fetch from one place and push to another
(this is often true for "fetch from upstream, push to my own
and ask upstream to pull from it" workflow), and in that case
you want pushUrl in addition to Url. Most importantly, in this
case, you do *NOT* want to push to Url. You only push to
pushUrl.
Setting *one* pushURL is a way to say "That URL I fetch from is
*not* the place I want to push (I may not even be able to push
there); when I say 'push', push there instead". Your proposed
semantics will make it impossible to arrange such an asymmetric
setting.
^ 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