Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
On Mon, Jun 24, 2013 at 6:33 AM, Junio C Hamano gits...@pobox.com wrote:
From: Ramkumar Ramachandra artag...@gmail.com
When remote.pushdefault or branch.name.pushremote is set (a triangular
workflow feature), master@{u} != origin, and push.default is set to
`upstream` or `simple`:
$ git push
fatal: You are pushing to remote 'origin', which is not the upstream of
your current branch 'master', without telling me what to push
to update which remote branch.
The very name of upstream indicates that it is only suitable for
use in central workflows; let us not even attempt to give it a new
meaning in triangular workflows, and error out as usual.
However, the `simple` does not have this problem: it is poised to be
the default for Git 2.0, and we would definitely like it to do
something sensible in triangular workflows.
Redefine simple as safer upstream for centralized workflow as
before, but work as current for triangular workflow.
An earlier round of this change by mistake broke the safety for
simple mode we have had since day 1 of that mode to make sure that
the branch in the repository we update is set to be the one we fetch
and integrate with, but it has been fixed.
Shouldn't there be an acompanying test to demonstrate this mistake being fixed?
Reported-by: Leandro Lucarella leandro.lucare...@sociomantic.com
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/config.txt | 10 +++---
builtin/push.c | 43 +++
2 files changed, 38 insertions(+), 15 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5d8ff1a..cae6870 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1848,9 +1848,13 @@ push.default::
pushing to the same repository you would normally pull from
(i.e. central workflow).
-* `simple` - like `upstream`, but refuses to push if the upstream
- branch's name is different from the local one. This is the safest
- option and is well-suited for beginners.
+* `simple` - in centralized workflow, work like `upstream` with an
+ added safety to refuse to push if the upstream branch's name is
+ different from the local one.
++
+When pushing to a remote that is different from the remote you normally
+pull from, work as `current`. This is the safest option and is suited
+for beginners.
+
This mode will become the default in Git 2.0.
diff --git a/builtin/push.c b/builtin/push.c
index 2d84d10..f6c8047 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -120,10 +120,11 @@ static const char message_detached_head_die[] =
\n
git push %s HEAD:name-of-remote-branch\n);
-static void setup_push_upstream(struct remote *remote, int simple)
+static void setup_push_upstream(struct remote *remote, struct branch *branch,
+ int triangular)
{
struct strbuf refspec = STRBUF_INIT;
- struct branch *branch = branch_get(NULL);
+
if (!branch)
die(_(message_detached_head_die), remote-name);
if (!branch-merge_nr || !branch-merge || !branch-remote_name)
@@ -137,18 +138,29 @@ static void setup_push_upstream(struct remote *remote,
int simple)
if (branch-merge_nr != 1)
die(_(The current branch %s has multiple upstream branches,
refusing to push.), branch-name);
- if (strcmp(branch-remote_name, remote-name))
+ if (triangular)
die(_(You are pushing to remote '%s', which is not the
upstream of\n
your current branch '%s', without telling me what to
push\n
to update which remote branch.),
remote-name, branch-name);
- if (simple strcmp(branch-refname, branch-merge[0]-src))
- die_push_simple(branch, remote);
+
+ if (push_default == PUSH_DEFAULT_SIMPLE) {
+ /* Additional safety */
+ if (strcmp(branch-refname, branch-merge[0]-src))
+ die_push_simple(branch, remote);
+ }
strbuf_addf(refspec, %s:%s, branch-name, branch-merge[0]-src);
add_refspec(refspec.buf);
}
+static void setup_push_current(struct remote *remote, struct branch *branch)
+{
+ if (!branch)
+ die(_(message_detached_head_die), remote-name);
+ add_refspec(branch-name);
Here (and above) we add a refspec to tell Git exactly what to push
from the local end, and into what on the remote end. Is it possible to
end up with multiple simultaneous refspecs matching the same local
ref, but mapping to different remote refs? If so, which will win, and
does that make sense?
+}
+
static char warn_unspecified_push_default_msg[] =
N_(push.default is unset; its implicit value is changing in\n
Git 2.0 from 'matching' to 'simple'. To squelch
Re: [PATCH 6/6] push: honor branch.*.push
On Mon, Jun 24, 2013 at 6:33 AM, Junio C Hamano gits...@pobox.com wrote:
When branch.*.push configuration variable is defined for the current
branch, a lazy-typing git push (and git push there) will push
the commit at the tip of the current branch to the destination and
update the branch named by that variable.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/push.c | 18 +-
remote.c | 5 +
remote.h | 2 ++
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/builtin/push.c b/builtin/push.c
index f6c8047..a140b8e 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -185,6 +185,15 @@ static void
warn_unspecified_push_default_configuration(void)
warning(%s\n, _(warn_unspecified_push_default_msg));
}
+static void setup_per_branch_push(struct branch *branch)
+{
+ struct strbuf refspec = STRBUF_INIT;
+
+ strbuf_addf(refspec, %s:%s,
+ branch-name, branch-push_name);
+ add_refspec(refspec.buf);
This goes back to the question I raised in 3/6: If this code path adds
refspec foo:bar, and - say - setup_push_current() has already added
refspec foo:foo (or simply foo), then do we end up pushing into
foo or bar? To me, branch.*.push feels more specific than
push.default = current, so it would make sense that foo:bar
overrides foo:foo, but that is not obvious from the refspec alone.
IMHO, this definitely needs some tests.
+}
+
static int is_workflow_triagular(struct remote *remote)
{
struct remote *fetch_remote = remote_get(NULL);
@@ -194,7 +203,14 @@ static int is_workflow_triagular(struct remote *remote)
static void setup_default_push_refspecs(struct remote *remote)
{
struct branch *branch = branch_get(NULL);
- int triangular = is_workflow_triagular(remote);
+ int triangular;
+
+ if (branch-push_name) {
+ setup_per_branch_push(branch);
+ return;
I guess this return ensures that branch.*.push overrides push.default,
but might there be other sources of add_refspec() that would
complicate things?
+ }
+
+ triangular = is_workflow_triagular(remote);
switch (push_default) {
default:
diff --git a/remote.c b/remote.c
index e71f66d..e033fef 100644
--- a/remote.c
+++ b/remote.c
@@ -372,6 +372,11 @@ static int handle_config(const char *key, const char
*value, void *cb)
if (!value)
return config_error_nonbool(key);
add_merge(branch, xstrdup(value));
+ } else if (!strcmp(subkey, .push)) {
+ if (!value)
+ return config_error_nonbool(key);
+ free(branch-push_name);
+ branch-push_name = xstrdup(value);
}
return 0;
}
diff --git a/remote.h b/remote.h
index cf56724..84e0f72 100644
--- a/remote.h
+++ b/remote.h
@@ -138,6 +138,8 @@ struct branch {
struct refspec **merge;
int merge_nr;
int merge_alloc;
+
+ char *push_name;
};
struct branch *branch_get(const char *name);
Otherwise, this patch, and the entire series looks good to me.
...Johan
--
Johan Herland, jo...@herland.net
www.herland.net
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] t/t5528-push-default: generalize test_push_*
On Mon, Jun 24, 2013 at 6:33 AM, Junio C Hamano gits...@pobox.com wrote:
From: Ramkumar Ramachandra artag...@gmail.com
The setup creates two bare repositories: repo1 and repo2, but
test_push_commit() hard-codes checking in repo1 for the actual output.
Generalize it and its caller, test_push_success(), to optionally accept
a third argument to specify the name of the repository to check for
actual output. We will use this in the next patch.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t5528-push-default.sh | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index 69ce6bf..db58e7f 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -15,17 +15,19 @@ test_expect_success 'setup bare remotes' '
# $1 = local revision
# $2 = remote revision (tested to be equal to the local one)
+# $3 = [optional] repo to check for actual output (repo1 by default)
check_pushed_commit () {
git log -1 --format='%h %s' $1 expect
- git --git-dir=repo1 log -1 --format='%h %s' $2 actual
+ git --git-dir=${3:-repo1} log -1 --format='%h %s' $2 actual
Isn't ${3:-repo1} a bashism?
test_cmp expect actual
}
# $1 = push.default value
# $2 = expected target branch for the push
+# $3 = [optional] repo to check for actual output (repo1 by default)
test_push_success () {
git -c push.default=$1 push
- check_pushed_commit HEAD $2
+ check_pushed_commit HEAD $2 $3
}
# $1 = push.default value
--
1.8.3.1-721-g0a353d3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Johan Herland, jo...@herland.net
www.herland.net
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] rebase: write better reflog messages
Junio C Hamano wrote: @@ -59,6 +63,9 @@ else return $? fi + # always reset GIT_REFLOG_ACTION before calling any external + # scripts; they have no idea about our base_reflog_action + GIT_REFLOG_ACTION=$base_reflog_action git am $git_am_opt --rebasing --resolvemsg=$resolvemsg Why does this reroll still use this base_reflog_action convention? Because it's simple and it makes sense. The original orig_reflog_action you borrowed this may have been an acceptable local solution inside git-rebase--interactive that does not call out to amyting, but the above comment a good demonstration that shows why this cannot be a good general solution that scales across scriptlets. Nonsense. How do I set a custom reflog action when a certain flag is passed to my script (like git-rebase.sh:333) without *overriding* an existing GIT_REFLOG_ACTION? How do I construct cute start/pick/reword prefixes elegantly (like in git-rebase--interactive.sh:comment_for_reflog()) without *overriding* an existing GIT_REFLOG_ACTION? In both these examples, I'm setting a GIT_REFLOG_ACTION for the rest of the code to use. I don't care about the exact command sequence, but I know that they respect GIT_REFLOG_ACTION; so I'm setting one in advance. When calling out to an external scriptlet, I want to define my own reflog message: when I call out to am from rebase -i, it should write a rebase -i: message, and ignoring its own set_reflog_message(). _That_ can be done using the subshell thing you proposed. And I have absolutely no clue why ( export GIT_REFLOG_ACTION git am ) is more scalable than GIT_REFLOG_ACTION=$base_reflog_action git am And I already explained that to you at least twice. You just gave set_reflog_action() and GIT_REFLOG_ACTION some sort of God status, and proposed to make the scripts more ugly and less extensible. ... and we're discussing absolutely trivial inconsequential rubbish once again. In any case, I've given up on arguing with you as it is clear that I can't possibly win. Do whatever you want. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] Reroll of rr/triangular-push-fix
Junio C Hamano wrote: [PATCH 3/6] push: change `simple` to accommodate triangular workflows Sqaushed in the fix to keep the semantics of simple when used in the centralized workflow the same as before. Yeah, I'm worried about this as I pointed out earlier. I don't like erroring out when no branch.$branch.merge is not explicitly set, when the 95% usecase is not naming local branches differently from remote branches (oh, and I already pointed out how difficult it is to set the damn thing). So, I'm working on a series to make branch.$branch.merge default to refs/heads/$branch. Yes, I'm aware of your argument: We already have a sane default, which is to error out. We do not need your broken default. I hope (perhaps foolishly) to persuade you nevertheless. I fear that if this series solidifies before I get there, we'll be stuck with this stupid erroring-out behavior forever. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] t/t5528-push-default: generalize test_push_*
Johan Herland jo...@herland.net writes:
+ git --git-dir=${3:-repo1} log -1 --format='%h %s' $2 actual
Isn't ${3:-repo1} a bashism?
I do not think so. But now I looked at it again, I think I would
use ${3-repo1} form in this case myself. No caller passes an empty
string to the third place.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Do not ignore merge options in interactive rebase
Junio C Hamano gits...@pobox.com writes:
Arnaud Fontaine ar...@debian.org writes:
Merge strategy and its options can be specified in `git rebase`, but
with `--interactive`, they were completely ignored.
And why is it a bad thing? If you meant s/--interactive/-m/ in the
above, then I can sort of understand the justification, though.
Sorry, it was not clear. I meant that you can do 'rebase -m --strategy
recursive'. But with 'rebase --interactive -m --strategy recursive', '-m
--strategy recursive' is ignored. To me, this is not consistent because
the behavior is different in interactive mode... Personally, I needed
to specify the strategy and its options while using interactive mode and
it seems I'm not the only one[0].
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
old mode 100644
new mode 100755
I see an unjustifiable mode change here.
Sorry about that, I fixed it.
index f953d8d..c157fdf
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -239,7 +239,16 @@ pick_one () {
test -d $rewritten
pick_one_preserving_merges $@ return
-output git cherry-pick $empty_args $ff $@
+
+if test -n $do_merge
+then
So you _did_ mean rebase -m?
I really meant 'rebase --interactive -m'. do_merge is set to true if
either '--strategy' or '-m' or '-X' is given according to git-rebase.sh.
+test -z $strategy strategy=recursive
+output git cherry-pick --strategy=$strategy \
This is a bad change.
I would understand if the above were:
git cherry-pick ${strategy+--strategy=$strategy} ...
in other words, if there is no strategy specified, do not override
the configured default that might be different from recursive
(pull.twohead may be set to resolve).
Indeed, I did not know about that. I wrongly thought it was a good idea
to do the same as both git-rebase (when -X is given) and
git-rebase--merge which do the same test ('test -z $strategy
strategy=recursive'). However after checking more carefully, I guess
that, for the former case, it is because only recursive currently takes
options, whereas, for the latter case, it is to call a default
git-rebase-$strategy.
+$(echo $strategy_opts | sed s/'--\([^']*\)'/-X\1/g) \
Is it guaranteed $startegy_opts do not have a space in it?
strategy_opts may be something like (git-rebase.sh): '--foo' '--bar',
but I'm not sure what is wrong if there is a space in it though.
There is a call to git merge that uses ${strategy+-s $strategy},
but it does not seem to propagate the strategy option. Does it need a
similar change? It seems that the first step might be to factor out
these calls to the git cherry-pick and git merge to helper
functions to make it easier to call them with -s/-X options in a
consistent way.
As far as I understand, yes. I changed it. As it is really short, I just
added an if/else inside the script itself, not sure if that's ok...
+$empty_args $ff $@
+else
+output git cherry-pick $empty_args $ff $@
+fi
It seems that there is another call to git cherry-pick in the script
(git grep for it). Does it need a similar change?
As far as I understand, yes. So I changed it as well.
I have sent the fixed patch in my next email. Many thanks for the
review!
Cheers,
--
Arnaud Fontaine
[0]
http://git.661346.n2.nabble.com/git-rebase-interactive-does-not-respect-merge-options-td7500093.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
Johan Herland jo...@herland.net writes:
+static void setup_push_current(struct remote *remote, struct branch *branch)
+{
+ if (!branch)
+ die(_(message_detached_head_die), remote-name);
+ add_refspec(branch-name);
Here (and above) we add a refspec to tell Git exactly what to push
from the local end, and into what on the remote end.
Correct.
Is it possible to end up with multiple simultaneous refspecs
matching the same local ref, but mapping to different remote refs?
Sorry, I don't follow. If you say push.default = current and you
do not give any other stronger clue (e.g. git push origin master
on the command line, or git push [origin] with remote.origin.push
configured), the above function is called and sets up your current
branch to be pushed to the same.
It is a bit more interesting for push.default = upstream, which is
for centralized workflow. If you forked frotz and nitfol branches
both from their master, e.g.
$ git checkout -t -b frotz origin/master
$ git checkout -t -b nitfol origin/master
after having worked on one of the branches, when you want to push it
back, the result of working on the topic branch goes back to master,
but I think that is what you want in the centralized workflow. If
it fast-forwards, you are fine, and if it does not, you will fetch
your upstream, i.e. their master, integrate your work with it, and
then push it back. At that point, you are playing the role of the
integrator of the shared master branch, because what you do on your
topic branch when you integrate others' work from master is exactly
that---you are not perfecting the theme you wanted to achieve on
your topic branch, but are integrating that result into shared
master to advance the overall state of the project. So pushing the
result back to 'master' makes perfect sense. After that, when you
have to restart your work on the other branch, you may first pull
--rebase before continuing, or you may just keep going with your
work based on a tad old origin/master. But when you finish working
on that topic and are about to push it out, you would be doing the
same tentatively don the central integrator's hat, and again it
makes sense to push the result to 'master'.
So in that sense, it is not which one wins. It is more like you
can push only after you become up to date, so there isn't one branch
overwriting the other one.
That is how I view it, anyway.
cf. http://git-blame.blogspot.com/2013/06/fun-with-various-workflows-1.html
+static int is_workflow_triagular(struct remote *remote)
s/triagular/triangular/
Thanks.
+{
+ struct remote *fetch_remote = remote_get(NULL);
+ return (fetch_remote fetch_remote != remote);
This changed from a strcmp() to a pointer compare. That might be safe,
depending on the sources of the two struct remote *, but I'm not sure.
Given the way remote_get() works, it should be correct, I think.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] push: honor branch.*.push
Johan Herland jo...@herland.net writes:
+static void setup_per_branch_push(struct branch *branch)
+{
+ struct strbuf refspec = STRBUF_INIT;
+
+ strbuf_addf(refspec, %s:%s,
+ branch-name, branch-push_name);
+ add_refspec(refspec.buf);
This goes back to the question I raised in 3/6: If this code path adds
refspec foo:bar, and - say - setup_push_current() has already added
refspec foo:foo (or simply foo), then do we end up pushing into
foo or bar?
I think you answered your own question below with the return.
@@ -194,7 +203,14 @@ static int is_workflow_triagular(struct remote *remote)
static void setup_default_push_refspecs(struct remote *remote)
{
struct branch *branch = branch_get(NULL);
- int triangular = is_workflow_triagular(remote);
+ int triangular;
+
+ if (branch-push_name) {
+ setup_per_branch_push(branch);
+ return;
I guess this return ensures that branch.*.push overrides push.default,
but might there be other sources of add_refspec() that would
complicate things?
The default-push-refspecs is meant to be used only when there is no
other stronger clue given by the user (e.g. refspec on the command
line, e.g. git push there master, pushing with configured refspecs
on remote.$name.push), so I think it is a bug if somebody calls this
function when there is other source.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Do not ignore merge options in interactive rebase
Fix inconsistency where `--strategy` and/or `--strategy-option` can be
specified in git rebase, but with `--interactive` argument only there
were completely ignored.
Signed-off-by: Arnaud Fontaine ar...@debian.org
---
git-rebase--interactive.sh| 13 ++---
t/t3404-rebase-interactive.sh | 11 +++
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f953d8d..e558397 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -80,6 +80,13 @@ amend=$state_dir/amend
rewritten_list=$state_dir/rewritten-list
rewritten_pending=$state_dir/rewritten-pending
+strategy_args=
+if test -n $do_merge
+then
+ strategy_args=${strategy+--strategy=$strategy}
+ $(echo $strategy_opts | sed s/'--\([^']*\)'/-X\1/g)
+fi
+
GIT_CHERRY_PICK_HELP=$resolvemsg
export GIT_CHERRY_PICK_HELP
@@ -239,7 +246,7 @@ pick_one () {
test -d $rewritten
pick_one_preserving_merges $@ return
- output git cherry-pick $empty_args $ff $@
+ output git cherry-pick $strategy_args $empty_args $ff $@
}
pick_one_preserving_merges () {
@@ -341,7 +348,7 @@ pick_one_preserving_merges () {
# No point in merging the first parent, that's HEAD
new_parents=${new_parents# $first_parent}
if ! do_with_author output \
- git merge --no-ff ${strategy:+-s $strategy} -m \
+ git merge --no-ff $strategy_args -m \
$msg_content $new_parents
then
printf %s\n $msg_content
$GIT_DIR/MERGE_MSG
@@ -350,7 +357,7 @@ pick_one_preserving_merges () {
echo $sha1 $(git rev-parse HEAD^0)
$rewritten_list
;;
*)
- output git cherry-pick $@ ||
+ output git cherry-pick $strategy_args $@ ||
die_with_patch $sha1 Could not pick $sha1
;;
esac
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 79e8d3c..8b6a36f 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -947,4 +947,15 @@ test_expect_success 'rebase -i respects core.commentchar' '
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
'
+test_expect_success 'rebase -i with --strategy and -X' '
+ git checkout -b conflict-merge-use-theirs conflict-branch
+ git reset --hard HEAD^
+ echo five conflict
+ echo Z file1
+ git commit -a -m one file conflict
+ EDITOR=true git rebase -i --strategy=recursive -Xours conflict-branch
+ test $(git show conflict-branch:conflict) = $(cat conflict)
+ test $(cat file1) = Z
+'
+
test_done
--
1.8.3.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
Johan Herland wrote:
An earlier round of this change by mistake broke the safety for
simple mode we have had since day 1 of that mode to make sure that
the branch in the repository we update is set to be the one we fetch
and integrate with, but it has been fixed.
Shouldn't there be an acompanying test to demonstrate this mistake being
fixed?
Read earlier iteration: it didn't get merged.
+static void setup_push_current(struct remote *remote, struct branch *branch)
+{
+ if (!branch)
+ die(_(message_detached_head_die), remote-name);
+ add_refspec(branch-name);
Here (and above) we add a refspec to tell Git exactly what to push
from the local end, and into what on the remote end.
Nope, we add the refspec foo, without the :destination part. The
remote end is unspecified (and defaults to foo, but that is in the
transport layer).
Is it possible to
end up with multiple simultaneous refspecs matching the same local
ref, but mapping to different remote refs? If so, which will win, and
does that make sense?
It is impossible. We either:
- Get an explicit refspec from the user and never run
setup_default_push_refspecs() to begin with.
- Run setup_push_refspecs() and add *one* refspec depending on the
push.default value.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] rebase: write better reflog messages
Ramkumar Ramachandra artag...@gmail.com writes: + # always reset GIT_REFLOG_ACTION before calling any external + # scripts; they have no idea about our base_reflog_action + GIT_REFLOG_ACTION=$base_reflog_action git am $git_am_opt --rebasing --resolvemsg=$resolvemsg Why does this reroll still use this base_reflog_action convention? Because it's simple and it makes sense. Without $base_reflog_action hack, you have to make sure GIT_REFLOG_ACTION is reasonably pristine when you call out other people. Even with $base_reflog_action, you still have to do the same keep GIT_REFLOG_ACTION pristine like this one. And in addition, you have to maintain $base_reflog_action as if it is a read-only variable [*1*]. So you are forcing people to maintain _two_ variables, instead of just _one_, without making anything simpler. What's so hard to understand why it is a wrong design? ... and we're discussing absolutely trivial inconsequential rubbish once again. In any case, I've given up on arguing with you as it is clear that I can't possibly win. Do whatever you want. It is not about winning or losing. If you truly think this is inconsequential, that unfortunately convinces me that you cannot yet be trusted enough to give you latitude to design interfaces that span multiple programs X-. [Footnote] *1* The original orig_reflog_action you borrowed from was bad enough but it had an excuse that it was confined within the leaf level of the callchain. It was merely done as a way to stash the vanilla action name (e.g. rebase -i before it is specialized into rebase -i pick etc) away, so that it can easily lose the speciailzation from GIT_REFLOG_ACTION while preparing for the next operation. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] push: honor branch.*.push
Junio C Hamano wrote:
@@ -194,7 +203,14 @@ static int is_workflow_triagular(struct remote *remote)
static void setup_default_push_refspecs(struct remote *remote)
{
struct branch *branch = branch_get(NULL);
- int triangular = is_workflow_triagular(remote);
+ int triangular;
+
+ if (branch-push_name) {
+ setup_per_branch_push(branch);
+ return;
+ }
The most obvious question comes first: what result can I expect when
this interacts with remote.name.push?
Why did you design this feature like this? Will the user _not_ want
refspec mapping except when pushing out the current branch with a
plain git push?
Also, you managed to throw out all safety out the window. What
happens when the user does:
# on branch master, derived from origin
$ git push ram
And branch.master.push is set to next? Will you let her shoot herself
in the foot like this?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Message could not be delivered
To git@vger.kernel.org - git@vger.kernel.org, Your email message Message could not be delivered did not reach your recipient. Please click to confirm your message: http://www.tiscalimail.com/ Please vissit this link to find out more information: http://www.tiscalimail.com/ Thank you. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
Johan Herland jo...@herland.net writes: An earlier round of this change by mistake broke the safety for simple mode we have had since day 1 of that mode to make sure that the branch in the repository we update is set to be the one we fetch and integrate with, but it has been fixed. Shouldn't there be an acompanying test to demonstrate this mistake being fixed? An operation that has to expect failure due to safety was disabled by the broken version. The squashed end result reverts that change to the test, to make sure we did not break the safety. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] Reroll of rr/triangular-push-fix
Ramkumar Ramachandra artag...@gmail.com writes: I hope (perhaps foolishly) to persuade you nevertheless. I fear that if this series solidifies before I get there, we'll be stuck with this stupid erroring-out behavior forever. I do not think it is stupid at all. 'simple' is supposed to be an easy and safe default to help new people. Your original patch to change its semantics for the central workflow from the current 'make sure upstream is set and set to the same name' to 'anything goes' is making the mode more dangerous than the corresponding 'upstream'. Such a mode may have its place, but labelling such a mode with rough edges as 'simple' and forcing it on new people _is_ stupid, IMHO. In any case, the good news is, if you start strict, and if it turns out to be stricter than necessary, it is easier to loosen it later, because nobody would be relying on an operation to _fail_. If you start too loose without safety, however, it is a lot harder to tighten it later when it turns out that safety helps new people. Since the beginning of this series, our working assumption for triangular-simple' has been that it can just turn into a straight 'current'. But given that 'simple' is supposed to be an easy and safe default for new people, I suspect that it should be a bit more strict. For example, if you are on a random topic branch and say git push, always pushing it out may not be a very sensible thing to do, and it might be safer if we restrict 'triangular-simple' to push out the current branch to the branch of the same name, but only when such a branch already exists at the remote end, or something. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] rebase: write better reflog messages
Junio C Hamano wrote: So you are forcing people to maintain _two_ variables, instead of just _one_, without making anything simpler. What's so hard to understand why it is a wrong design? Fine. Let's say I buy your argument about one-variable versus two-variables: how do you solve the existing problems that are solved by overriding GIT_REFLOG_ACTION that I pointed out? If you truly think this is inconsequential, that unfortunately convinces me that you cannot yet be trusted enough to give you latitude to design interfaces that span multiple programs X-. *shrug* I certainly don't think one-variable versus two-variables warrants this much discussion. I don't have anything to win or lose: I designed a solution to the problem which I think is reasonable; you don't. Fine. Show me an alternative that doesn't involve rewriting half of the rebase infrastructure in a series that fixes checkout-dash. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] push: honor branch.*.push
Ramkumar Ramachandra artag...@gmail.com writes:
Junio C Hamano wrote:
@@ -194,7 +203,14 @@ static int is_workflow_triagular(struct remote *remote)
static void setup_default_push_refspecs(struct remote *remote)
{
struct branch *branch = branch_get(NULL);
- int triangular = is_workflow_triagular(remote);
+ int triangular;
+
+ if (branch-push_name) {
+ setup_per_branch_push(branch);
+ return;
+ }
The most obvious question comes first: what result can I expect when
this interacts with remote.name.push?
Now you bring it up, the branch.*.push may want to be more specific
(when I am on _this_ branch, do this) than remote.*.push (when I am
pushing to that remote, I want this to happen in general), but this
default codepath would not be exercised when you have remote.*.push,
so the logic may need to be moved higher up in the foodchain.
Also, you managed to throw out all safety out the window. What
happens when the user does:
# on branch master, derived from origin
$ git push ram
And branch.master.push is set to next? Will you let her shoot herself
in the foot like this?
It is not shooting in the foot, if branch.master.push is explicitly
set to update next. I do not see any issue in that part.
But the relative strength betweenh branch.*.push and remote.*.push
may need to be thought out. I haven't.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] t/t5528-push-default: generalize test_push_*
On Mon, Jun 24, 2013 at 9:28 AM, Junio C Hamano gits...@pobox.com wrote:
Johan Herland jo...@herland.net writes:
+ git --git-dir=${3:-repo1} log -1 --format='%h %s' $2 actual
Isn't ${3:-repo1} a bashism?
I do not think so. But now I looked at it again, I think I would
use ${3-repo1} form in this case myself. No caller passes an empty
string to the third place.
Ok, I have to admit that I'm not at all sure where the line between sh
and bash goes when it comes to ${magic}... Is there any good
documentation on what is in sh and what is not?
...Johan
--
Johan Herland, jo...@herland.net
www.herland.net
c
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] t/t5528-push-default: generalize test_push_*
On Mon, Jun 24, 2013 at 4:33 AM, Johan Herland jo...@herland.net wrote:
On Mon, Jun 24, 2013 at 9:28 AM, Junio C Hamano gits...@pobox.com wrote:
Johan Herland jo...@herland.net writes:
+ git --git-dir=${3:-repo1} log -1 --format='%h %s' $2 actual
Isn't ${3:-repo1} a bashism?
I do not think so. But now I looked at it again, I think I would
use ${3-repo1} form in this case myself. No caller passes an empty
string to the third place.
Ok, I have to admit that I'm not at all sure where the line between sh
and bash goes when it comes to ${magic}... Is there any good
documentation on what is in sh and what is not?
POSIX:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
On Mon, Jun 24, 2013 at 9:59 AM, Junio C Hamano gits...@pobox.com wrote: Johan Herland jo...@herland.net writes: An earlier round of this change by mistake broke the safety for simple mode we have had since day 1 of that mode to make sure that the branch in the repository we update is set to be the one we fetch and integrate with, but it has been fixed. Shouldn't there be an acompanying test to demonstrate this mistake being fixed? An operation that has to expect failure due to safety was disabled by the broken version. The squashed end result reverts that change to the test, to make sure we did not break the safety. Ok, thanks. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
On Mon, Jun 24, 2013 at 9:46 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
Johan Herland wrote:
+static void setup_push_current(struct remote *remote, struct branch
*branch)
+{
+ if (!branch)
+ die(_(message_detached_head_die), remote-name);
+ add_refspec(branch-name);
Here (and above) we add a refspec to tell Git exactly what to push
from the local end, and into what on the remote end.
Nope, we add the refspec foo, without the :destination part. The
remote end is unspecified (and defaults to foo, but that is in the
transport layer).
Ok, so foo is not always semantically equivalent to foo:foo, and
when adding foo:bar it is always considered more specific than (and
superior to) foo. I think that makes sense.
Is it possible to
end up with multiple simultaneous refspecs matching the same local
ref, but mapping to different remote refs? If so, which will win, and
does that make sense?
It is impossible. We either:
- Get an explicit refspec from the user and never run
setup_default_push_refspecs() to begin with.
- Run setup_push_refspecs() and add *one* refspec depending on the
push.default value.
Thanks, that's what I wanted to hear. But then, does it make sense to
say that we will only ever have exactly _one_ push refspec in the
current context, and we should therefore replace the static const
char **refspec; string array with a single static const char
*refspec; string? That would make it obvious that there is no room
for ambiguity with overlapping refspecs.
...Johan
--
Johan Herland, jo...@herland.net
www.herland.net
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] t/t5528-push-default: generalize test_push_*
On Mon, Jun 24, 2013 at 10:44 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Mon, Jun 24, 2013 at 4:33 AM, Johan Herland jo...@herland.net wrote:
On Mon, Jun 24, 2013 at 9:28 AM, Junio C Hamano gits...@pobox.com wrote:
Johan Herland jo...@herland.net writes:
+ git --git-dir=${3:-repo1} log -1 --format='%h %s' $2 actual
Isn't ${3:-repo1} a bashism?
I do not think so. But now I looked at it again, I think I would
use ${3-repo1} form in this case myself. No caller passes an empty
string to the third place.
Ok, I have to admit that I'm not at all sure where the line between sh
and bash goes when it comes to ${magic}... Is there any good
documentation on what is in sh and what is not?
POSIX:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
Thanks! Learn something new every day, I guess. :)
...Johan
--
Johan Herland, jo...@herland.net
www.herland.net
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Splitting a rev list into 2 sets
Francis Moreau francis.m...@gmail.com writes:
On Thu, Jun 20, 2013 at 3:20 PM, Thomas Rast tr...@inf.ethz.ch wrote:
positive=$(git rev-parse $@ | grep -v '^\^')
negative=$(git rev-parse $@ | grep '^\^')
boundary=$(git rev-list --boundary $positive ^master | sed -n 's/^-//p')
# the intersection is
git rev-list $boundary $negative
I think there's a minor issue here, when boundary is empty. Please
correct me if I'm wrong but I think it can only happen if positive is
simply master or a subset of master. In that case I think the solution
is just make boundary equal to positive:
# the intersection is
git rev-list ${boundary:-$positive} $negative
Now I'm going to see if that solution is faster than the initial one.
Jan jast Krüger pointed out on #git that
git log $(git merge-base --all A B)
is exactly the set of commits reachable from both A and B; so there's
your intersection operator :-)
So it would seem that a much simpler approach is
git rev-list $(git merge-base --all master $positive) --not $negative
avoiding the boundary handling and special-case. It relies on the
(weird?) property that $(git merge-base --all A B1 B2 ...) shows the
merge bases of A with a hypothetical merge of B1, B2, ..., which is just
what you need here.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: detached HEAD before root commit - possible?
SZEDER Gábor sze...@ira.uka.de writes: I suspect that detaching HEAD before a root commit is not possible by design. What would HEAD contain then!? 'git checkout' seems to corroborate: $ git init Initialized empty Git repository in /tmp/test/.git/ $ git checkout --detach fatal: You are on a branch yet to be born Is git checkout --orphan what you're looking for? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] documentation: add git transport security notice
The fact that the git transport has no end-to-end security is easily
overlooked. Add a brief security notice to the GIT URLS section
of the documentation stating that the git transport should be used
with caution on unsecured networks.
Signed-off-by: Fraser Tweedale fr...@frase.id.au
---
Documentation/urls.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index 3ca122f..c218af5 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -11,6 +11,9 @@ and ftps can be used for fetching and rsync can be used for
fetching
and pushing, but these are inefficient and deprecated; do not use
them).
+The git protocol provides no end-to-end security and should be used
+with caution on unsecured networks.
+
The following syntaxes may be used with them:
- ssh://{startsb}user@{endsb}host.xz{startsb}:port{endsb}/path/to/repo.git/
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] diff-options: document default similarity index
The default similarity index of 50% is documented in gitdiffcore(7) but it is worth also mentioning it in the description of the -M/--find-renames option. Signed-off-by: Fraser Tweedale fr...@frase.id.au --- Documentation/diff-options.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index b8a9b86..69bb3a6 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -333,7 +333,7 @@ endif::git-log[] a fraction, with a decimal point before it. I.e., `-M5` becomes 0.5, and is thus the same as `-M50%`. Similarly, `-M05` is the same as `-M5%`. To limit detection to exact renames, use - `-M100%`. + `-M100%`. The default similarity index is 50%. -C[n]:: --find-copies[=n]:: -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] documentation: fix asterisks in fetch-options
Fix refspec asterisks in the -t/--tags section of the fetch-options
documentation.
Signed-off-by: Fraser Tweedale fr...@frase.id.au
---
Documentation/fetch-options.txt | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 9cb6496..f2ac3bc 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -61,11 +61,12 @@ endif::git-pull[]
ifndef::git-pull[]
-t::
--tags::
- This is a short-hand for giving refs/tags/*:refs/tags/*
- refspec from the command line, to ask all tags to be fetched
- and stored locally. Because this acts as an explicit
- refspec, the default refspecs (configured with the
- remote.$name.fetch variable) are overridden and not used.
+ This is a short-hand for giving
+ refs/tags/{asterisk}:refs/tags/{asterisk} refspec from the
+ command line, to ask all tags to be fetched and stored
+ locally. Because this acts as an explicit refspec, the
+ default refspecs (configured with the remote.$name.fetch
+ variable) are overridden and not used.
--recurse-submodules[=yes|on-demand|no]::
This option controls if and under what conditions new commits of
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] documentation: fix asterisks in fetch-options
Fraser Tweedale fr...@frase.id.au writes:
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 9cb6496..f2ac3bc 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -61,11 +61,12 @@ endif::git-pull[]
ifndef::git-pull[]
-t::
--tags::
- This is a short-hand for giving refs/tags/*:refs/tags/*
- refspec from the command line, to ask all tags to be fetched
- and stored locally. Because this acts as an explicit
- refspec, the default refspecs (configured with the
- remote.$name.fetch variable) are overridden and not used.
+ This is a short-hand for giving
+ refs/tags/{asterisk}:refs/tags/{asterisk} refspec from the
+ command line, to ask all tags to be fetched and stored
+ locally. Because this acts as an explicit refspec, the
+ default refspecs (configured with the remote.$name.fetch
+ variable) are overridden and not used.
Wasn't this already fixed by 9eb4754 (fetch-options.txt: prevent a
wildcard refspec from getting misformatted, 2013-06-07), currently in
master?
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] documentation: fix asterisks in fetch-options
Right you are. I missed that; apologies for the noise.
Fraser
On Mon, Jun 24, 2013 at 01:35:13PM +0200, Thomas Rast wrote:
Fraser Tweedale fr...@frase.id.au writes:
diff --git a/Documentation/fetch-options.txt
b/Documentation/fetch-options.txt
index 9cb6496..f2ac3bc 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -61,11 +61,12 @@ endif::git-pull[]
ifndef::git-pull[]
-t::
--tags::
- This is a short-hand for giving refs/tags/*:refs/tags/*
- refspec from the command line, to ask all tags to be fetched
- and stored locally. Because this acts as an explicit
- refspec, the default refspecs (configured with the
- remote.$name.fetch variable) are overridden and not used.
+ This is a short-hand for giving
+ refs/tags/{asterisk}:refs/tags/{asterisk} refspec from the
+ command line, to ask all tags to be fetched and stored
+ locally. Because this acts as an explicit refspec, the
+ default refspecs (configured with the remote.$name.fetch
+ variable) are overridden and not used.
Wasn't this already fixed by 9eb4754 (fetch-options.txt: prevent a
wildcard refspec from getting misformatted, 2013-06-07), currently in
master?
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff returns fatal error with core.safecrlf is set to true.
Hi, Le 21.06.2013 23:57, Junio C Hamano a écrit : Junio C Hamano gits...@pobox.com writes: The helper may want to learn a way to be told to demote that error to a warning. Perhaps something like this? Thanks for the patch. I run my test again, eg. run git diff after a rebase failure (see my other mail about core.safecrlf), I'm able to run git diff a get a meaningful output: # git version 1.8.1.4 fatal: CRLF would be replaced by LF in test. # git version 1.8.3.1.741.g635527f.dirty (eg. next + your patch) warning: CRLF will be replaced by LF in test. The file will have its original line endings in your working directory. diff --git a/test b/test index b043836..63ba10f 100644 --- a/test +++ b/test @@ -1,4 +1,4 @@ -Hello World 1 -Hello World 2 -Hello World 3 +Hello World 1 +Hello World 2 +Hello World 3 Hello World 4 \ No newline at end of file It seems better. The removed lines have CRLF EOL while the added line have LF line ending characters. Tested-By: Yann Droneaud ydrone...@opteya.com Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH (!) 0/2] Fix serious regressions in latest master
Hi, It turns out that status.short and status.branch introduce two very serious regressions. This series fixes them. Thanks. Ramkumar Ramachandra (2): status: really ignore config with --porcelain commit: make it work with status.short builtin/commit.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) -- 1.8.3.1.550.gd96f26e.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] commit: make it work with status.short
50e4f75 (status: introduce status.short to enable --short by default, 2013-06-11) introduced a regression in git commit; it is now impossible to commit with status.short set. This happens because commit internally runs run_status() to set s-commitable and determine whether or not there is something to commit. The problem arises from the fact that only STATUS_FORMAT_NONE (or STATUS_FORMAT_LONG) is equipped to set s-commitable. 7c9f7038 (commit: support alternate status formats, 2009-09-05) clearly states that --short and --porcelain imply --dry-run and are therefore only intended for display purposes. The bigger problem is that it is impossible to differentiate between a status_format set by the command-line option parser versus that set by the config parser. So these two are exactly equivalent: $ git -c status.short=true commit $ git commit --short To alleviate this problem, clear status_format as soon as the config parser has finished its work to nullify the effect of parsing status.short, and get commit to work again. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- builtin/commit.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/commit.c b/builtin/commit.c index 896f002..dc5ed7d 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1448,6 +1448,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) wt_status_prepare(s); gitmodules_config(); git_config(git_commit_config, s); + status_format = STATUS_FORMAT_NONE; /* Ignore status.short */ determine_whence(s); s.colopts = 0; -- 1.8.3.1.550.gd96f26e.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] status: really ignore config with --porcelain
1a22bd3 (Merge branch 'jg/status-config', 2013-06-23) introduced a serious regression in --porcelain by introducing the configuration variables status.short and status.branch. Contrary to its description, the output of $ git status --porcelain now depends on the configuration variables status.short and status.branch. As a result, callers that expect parsable output to be returned are broken. For instance, in a repository with submodules with status.branch and status.short set, $ git status always reports all submodules as containing modified content, even if they are clean. One solution to the problem is to turn off s-show_branch in wt_porcelain_print() just like we turn off other s-* variables, but that would break callers of --porcelain --branch (in fact, there is such a caller in t/t7508-status.sh). Besides, we never said that --porcelain cannot be combined with other options. The larger problem is that the config parser and command-line option parser set the same variables, making it impossible to determine who set them. The correct solution is therefore to skip the config parser completely when --porcelain is given. Unfortunately, to determine that --porcelain is given, we have to run the command-line option parser. Running the command-line option parser before the config parser is undesirable, as configuration variables would override options on the command-line. As a fair compromise, check that argv[1] is equal to the string --porcelain and skip the config parser in this case. It is a compromise, because we expect --porcelain to be specified as the first argument to status. On a related note, the command-line parser is not very robust. $ git status --short --long $ git status --long --long $ git status --porcelain --long $ git status --long --porcelain $ git status --porcelain --short $ git status --short --porcelain all return different outputs. This bug is left as an exercise for future contributors to fix. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- builtin/commit.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/builtin/commit.c b/builtin/commit.c index b589ce0..896f002 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1193,7 +1193,12 @@ int cmd_status(int argc, const char **argv, const char *prefix) wt_status_prepare(s); gitmodules_config(); - git_config(git_status_config, s); + + if (argc 1 !strcmp(argv[1], --porcelain)) + ; /* Do not read user configuration */ + else + git_config(git_status_config, s); + determine_whence(s); argc = parse_options(argc, argv, prefix, builtin_status_options, -- 1.8.3.1.550.gd96f26e.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] status: really ignore config with --porcelain
Ramkumar Ramachandra artag...@gmail.com writes:
Contrary to its description, the output of
$ git status --porcelain
now depends on the configuration variables status.short and
status.branch.
Ouch, indeed :-(.
The correct solution is therefore to skip the config parser completely
when --porcelain is given. Unfortunately, to determine that --porcelain
is given, we have to run the command-line option parser. Running the
command-line option parser before the config parser is undesirable, as
configuration variables would override options on the command-line. As
a fair compromise, check that argv[1] is equal to the string
--porcelain and skip the config parser in this case.
I really don't like this. If we go for a solution looking explicitely at
argv[], we should at least iterate over it (also not satisfactory
because --porcelain could be the argument of another switch).
I think it's possible to have an actually robust solution, either
* running the CLI parser after, if --porcelain is given, reset the
effect of the variables. Not very clean because we'd have to reset all
the variables to their default, and there is a risk of forgetting one.
* Or, running the CLI parser before, but with different variables to
specify what the command-line says and what will actually be done,
with something like
actual_short = 0;
switch (command_line_short) {
case yes:
actual_short = 1;
break;
case no:
actual_short = 0;
break;
case unset: /* nothing */
}
switch (config_short) {
// same
}
---
builtin/commit.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
No time to contribute one now myself, but this would really deserve a
test.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] Reroll of rr/triangular-push-fix
Junio C Hamano wrote: 'simple' is supposed to be an easy and safe default to help new people. Your original patch to change its semantics for the central workflow from the current 'make sure upstream is set and set to the same name' to 'anything goes' is making the mode more dangerous than the corresponding 'upstream'. Such a mode may have its place, but labelling such a mode with rough edges as 'simple' and forcing it on new people _is_ stupid, IMHO. Oh, I agree that anything goes was the wrong approach. However, I think a sane default for branch.$branch.merge is a good way forward. In any case, the good news is, if you start strict, and if it turns out to be stricter than necessary, it is easier to loosen it later, because nobody would be relying on an operation to _fail_. Okay, I will quote you if you raise issues about preserving how it has historically been functioning when I complete the upstream-fix topic. There's no need to stall this series then: [1/6] to [5/6] largely look good-to-merge; drop [6/6], as it needs more thought. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] status: really ignore config with --porcelain
Matthieu Moy wrote: [...] Before we begin, let me say that this series is written with the emergency-fix mindset, and targets maint. I didn't spend time thinking about how to do it best or even add tests. The correct solution is therefore to skip the config parser completely when --porcelain is given. Unfortunately, to determine that --porcelain is given, we have to run the command-line option parser. Running the command-line option parser before the config parser is undesirable, as configuration variables would override options on the command-line. As a fair compromise, check that argv[1] is equal to the string --porcelain and skip the config parser in this case. I really don't like this. If we go for a solution looking explicitely at argv[], we should at least iterate over it (also not satisfactory because --porcelain could be the argument of another switch). Yep, that's the compromise. I think it's possible to have an actually robust solution, either * running the CLI parser after, if --porcelain is given, reset the effect of the variables. Not very clean because we'd have to reset all the variables to their default, and there is a risk of forgetting one. Since it's impossible to determine what effect the CLI parser had on various variables (some of which are static global), I'm against this approach. * Or, running the CLI parser before, but with different variables to specify what the command-line says and what will actually be done, with something like Basically, having the CLI parser and the config parser flip two different sets of variables, so we can discriminate who set what. What annoys me is that this is the first instance of such a requirement. The approach I'm currently tilting towards is extending the parse-options API to allow parsing one special option early. I would argue that this is a good feature that we should have asked for when we saw 6758af89e (Merge branch 'jn/git-cmd-h-bypass-setup', 2010-12-10). What do you think? builtin/commit.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) No time to contribute one now myself, but this would really deserve a test. Yeah, will do as a follow-up. I'm interested in guarding against such breakages too :) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows
Johan Herland wrote: But then, does it make sense to say that we will only ever have exactly _one_ push refspec in the current context, and we should therefore replace the static const char **refspec; string array with a single static const char *refspec; string? That would make it obvious that there is no room for ambiguity with overlapping refspecs. Multiple refspecs can be specified on the command-line; set_refspecs() is responsible for calling add_refspec() multiple times for each refspec, and _that_ is the primary use of the refspec variable. The single add_refspec() invocation in the push.default switch is a special case that reuses the variable. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Do not ignore merge options in interactive rebase
Arnaud Fontaine ar...@debian.org writes: + $(echo $strategy_opts | sed s/'--\([^']*\)'/-X\1/g) \ Is it guaranteed $startegy_opts do not have a space in it? strategy_opts may be something like (git-rebase.sh): '--foo' '--bar', but I'm not sure what is wrong if there is a space in it though. I was primarily worried about '--frotz=nitfol xyzzy', where you need to pass -X='frotz=nitfol xyzzy' so that 'xyzzy' part does not become a separate argument directly given to 'git merge' and friends. And adding '' around \1 is not sufficient, because the value given to the --frotz may have to be nitfol 'n xyzzy. A comment next to that sed script that says Currently there is no strategy option that needs quoting (if it is the case; I didn't check), together with a guard to protect us against unexpected strategy-opts, might be a workable band-aid, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] push: honor branch.*.push
Junio C Hamano wrote: # on branch master, derived from origin $ git push ram And branch.master.push is set to next? Will you let her shoot herself in the foot like this? It is not shooting in the foot, if branch.master.push is explicitly set to update next. I do not see any issue in that part. The question does not pertain to master being mapped to next; it pertains to central-workflow versus triangular-workflow: origin versus ram. If the user has set push.default to upstream, she _expects_ triangular pushes to always be denied, and this is the first violation of that rule. I'm tilting towards building a dependency between branch.name.push and push.default. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
benoit.per...@ensimag.fr writes: diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl new file mode 100644 index 000..a2f0aa1 --- /dev/null +++ b/contrib/mw-to-git/git-mw.perl *.perl scripts are usually executable in Git's tree (although it's usually better to run the non-*.perl version). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] status: really ignore config with --porcelain
Matthieu Moy matthieu@grenoble-inp.fr writes: I really don't like this. If we go for a solution looking explicitely at argv[], we should at least iterate over it (also not satisfactory because --porcelain could be the argument of another switch). Ram, thanks for a report. I won't comment on what is the correct way to see if --porcelain is given by the caller before I have enough time to think about it, but we did read configurattion even when --porcelainis given before the topic was merged, and I think it was done for a good reason. Configuration related to the output format (like color=always) must be ignored under --porcelain, but if we do not read core-ish configuration variables (e.g. core.crlf) that affect the logic to list what is changed what is not, we would not give the right result, no? So checking --porcelain option and skipping configuration may not be a solution but merely trading one regression with another. For now, I'll revert the merge and see if people can come up with a reasonable way forward. My knee-jerk reaction is that, because the --porcelain output was designed to be extensible and scripts reading from it is expected to ignore what it does not understand, if the setting of status.branch is a problem, the reading side is buggy and needs to be fixed. Do we have in-core reader that does not behave well when one or both of these configuration variables are set (perhaps something related to submodule?)??? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] status: really ignore config with --porcelain
Ramkumar Ramachandra artag...@gmail.com writes: Matthieu Moy wrote: [...] Before we begin, let me say that this series is written with the emergency-fix mindset, and targets maint. maint shouldn't be necessary since the breakage hasn't been released. At worse, we can revert the bad commits now and re-implement them properly later. I didn't spend time thinking about how to do it best or even add tests. No problem. * running the CLI parser after, if --porcelain is given, reset the effect of the variables. Not very clean because we'd have to reset all the variables to their default, and there is a risk of forgetting one. Since it's impossible to determine what effect the CLI parser had on various variables (some of which are static global), I'm against this approach. I think you meant what effect the config parser had. If you meant the CLI parser, then the guilty commits did not change anything wrt that. * Or, running the CLI parser before, but with different variables to specify what the command-line says and what will actually be done, with something like Basically, having the CLI parser and the config parser flip two different sets of variables, so we can discriminate who set what. What annoys me is that this is the first instance of such a requirement. I don't think it's the first instance, but I can't remember precise examples. The approach I'm currently tilting towards is extending the parse-options API to allow parsing one special option early. I would argue that this is a good feature that we should have asked for when we saw 6758af89e (Merge branch 'jn/git-cmd-h-bypass-setup', 2010-12-10). What do you think? That's an option too, yes. But probably not easy to implement :-(. builtin/commit.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) No time to contribute one now myself, but this would really deserve a test. Yeah, will do as a follow-up. I'm interested in guarding against such breakages too :) Should look like this: diff --git a/t/t7508-status.sh b/t/t7508-status.sh index 498332c..423e8c4 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -1378,6 +1378,11 @@ test_expect_success 'status.branch=true weaker than --no-branch' ' test_cmp expected_nobranch actual ' +test_expect_success 'status.branch=true weaker than --porcelain' ' + git -c status.branch=true status --porcelain actual + test_cmp expected_nobranch actual +' + test_expect_success 'status.branch=false same as --no-branch' ' git -c status.branch=false status -s actual test_cmp expected_nobranch actual -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] status: really ignore config with --porcelain
Junio C Hamano gits...@pobox.com writes: My knee-jerk reaction is that, because the --porcelain output was designed to be extensible and scripts reading from it is expected to ignore what it does not understand, if the setting of status.branch is a problem, the reading side is buggy and needs to be fixed. It is extensible in the sense that the caller can provide more command-line options to get more output (i.e. say --branch --porcelain), but providing different results for the same call because of the configuration file is broken IMHO. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] commit: make it work with status.short
Ramkumar Ramachandra artag...@gmail.com writes: 50e4f75 (status: introduce status.short to enable --short by default, 2013-06-11) introduced a regression in git commit; it is now impossible to commit with status.short set. This happens because commit internally runs run_status() to set s-commitable and determine whether or not there is something to commit. The problem arises from the fact that only STATUS_FORMAT_NONE (or STATUS_FORMAT_LONG) is equipped to set s-commitable. 7c9f7038 (commit: support alternate status formats, 2009-09-05) clearly states that --short and --porcelain imply --dry-run and are therefore only intended for display purposes. The bigger problem is that it is impossible to differentiate between a status_format set by the command-line option parser versus that set by the config parser. So these two are exactly equivalent: $ git -c status.short=true commit $ git commit --short Thanks for a report. I think the analysis above is correct, and if we want to ignore status.short but still want to honor --short from the command line, your patch is a way to go. As I said in the other message, I'll revert the merge of the topic branch from 'master' for now, and queue this on top of the topic so that we will have the preventive fix when the topic is in a better shape for the other possible breakage on the --branch thing. Having said that, even before the merge of that status.short (e.g. v1.8.3), git commit --short did not work and that was deliberate only because git commit --dry-run has long been an equivalent for git status, --short/-z/--porcelain were options for status, and therefore these options were made to imply --dry-run. I have to wonder if that is a sane thing in the first place, now that it has been quite a while since git status has become different from git commit --dry-run. That is, git commit --short/--porcelain/-z has these three possibilities: - work (ignoring these options); - work (showing the template in some kind of short format); or - error out (clearly indicating that we did *not* make a commit). and what we currently do is closest to the last (but we do not say we did not create a commit). In the longer term for Git 2.0, we may want to change it to do one of the former two. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v14 00/16] Interactive git-clean
Johannes found that some relative_path tests should be skipped on Windows. See this thread: http://thread.gmane.org/gmane.comp.version-control.git/227706 In this reroll, * I squash Johannes's patch into patch 01/16, and seven relative_path test cases are skipped by marking with POSIX. * In patch 02/16, 4 test cases can run correctly as the refactor of relative_path. * In patch 16/16, most of the previous skiped test cases can run with the help of the new utiliy test-path-utils mingw_path /abs/path/. Jiang Xin (16): test: add test cases for relative_path path.c: refactor relative_path(), not only strip prefix quote.c: remove path_relative, use relative_path instead Refactor quote_path_relative, remove unused params Refactor write_name_quoted_relative, remove unused params git-clean: refactor git-clean into two phases git-clean: add support for -i/--interactive git-clean: show items of del_list in columns git-clean: add colors to interactive git-clean git-clean: use a git-add-interactive compatible UI git-clean: add filter by pattern interactive action git-clean: add select by numbers interactive action git-clean: add ask each interactive action git-clean: add documentation for interactive git-clean test: add t7301 for git-clean--interactive test: run testcases with POSIX absolute paths on Windows Documentation/config.txt | 21 +- Documentation/git-clean.txt | 71 +++- builtin/clean.c | 778 +-- builtin/grep.c | 5 +- builtin/ls-files.c | 16 +- cache.h | 2 +- path.c | 112 +-- quote.c | 65 +--- quote.h | 7 +- setup.c | 5 +- t/t0060-path-utils.sh| 88 +++-- t/t7301-clean-interactive.sh | 439 test-path-utils.c| 32 ++ wt-status.c | 17 +- 14 files changed, 1487 insertions(+), 171 deletions(-) create mode 100755 t/t7301-clean-interactive.sh -- 1.8.3.1.756.g41beab0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v14 03/16] quote.c: remove path_relative, use relative_path instead
Since there is an enhanced version of relative_path() in path.c,
remove duplicate counterpart path_relative() in quote.c.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
quote.c | 55 ++-
1 file changed, 2 insertions(+), 53 deletions(-)
diff --git a/quote.c b/quote.c
index 91122..64ff3 100644
--- a/quote.c
+++ b/quote.c
@@ -312,75 +312,24 @@ void write_name_quotedpfx(const char *pfx, size_t pfxlen,
fputc(terminator, fp);
}
-static const char *path_relative(const char *in, int len,
-struct strbuf *sb, const char *prefix,
-int prefix_len);
-
void write_name_quoted_relative(const char *name, size_t len,
const char *prefix, size_t prefix_len,
FILE *fp, int terminator)
{
struct strbuf sb = STRBUF_INIT;
- name = path_relative(name, len, sb, prefix, prefix_len);
+ name = relative_path(name, prefix, sb);
write_name_quoted(name, fp, terminator);
strbuf_release(sb);
}
-/*
- * Give path as relative to prefix.
- *
- * The strbuf may or may not be used, so do not assume it contains the
- * returned path.
- */
-static const char *path_relative(const char *in, int len,
-struct strbuf *sb, const char *prefix,
-int prefix_len)
-{
- int off, i;
-
- if (len 0)
- len = strlen(in);
- if (prefix_len 0) {
- if (prefix)
- prefix_len = strlen(prefix);
- else
- prefix_len = 0;
- }
-
- off = 0;
- i = 0;
- while (i prefix_len i len prefix[i] == in[i]) {
- if (prefix[i] == '/')
- off = i + 1;
- i++;
- }
- in += off;
- len -= off;
-
- if (i = prefix_len)
- return in;
-
- strbuf_reset(sb);
- strbuf_grow(sb, len);
-
- while (i prefix_len) {
- if (prefix[i] == '/')
- strbuf_addstr(sb, ../);
- i++;
- }
- strbuf_add(sb, in, len);
-
- return sb-buf;
-}
-
/* quote path as relative to the given prefix */
char *quote_path_relative(const char *in, int len,
struct strbuf *out, const char *prefix)
{
struct strbuf sb = STRBUF_INIT;
- const char *rel = path_relative(in, len, sb, prefix, -1);
+ const char *rel = relative_path(in, prefix, sb);
strbuf_reset(out);
quote_c_style_counted(rel, strlen(rel), out, NULL, 0);
strbuf_release(sb);
--
1.8.3.1.756.g41beab0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v14 01/16] test: add test cases for relative_path
Add subcommand relative_path in test-path-utils, and add test cases
in t0060.
Johannes tested this commit on Windows, and found that some relative_path
tests should be skipped on Windows. This is because the bash on Windows
rewrites arguments of regular Windows programs, such as git and the
test helpers, if the arguments look like absolute POSIX paths. As a
consequence, the actual tests performed are not what the tests scripts
expect.
The tests that need *not* be skipped are those where the two paths passed
to 'test-path-utils relative_path' have the same prefix and the result is
expected to be a relative path. This is because the rewriting changes
/a/b to D:/Src/MSysGit/a/b, and when both inputs are extended the same
way, this just cancels out in the relative path computation.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Johannes Sixt j...@kdbg.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t0060-path-utils.sh | 26 ++
test-path-utils.c | 25 +
2 files changed, 51 insertions(+)
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 09a42..7e258 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -12,6 +12,11 @@ norm_path() {
test \\$(test-path-utils normalize_path_copy '$1')\ = '$2'
}
+relative_path() {
+ test_expect_success $4 relative path: $1 $2 = $3 \
+ test \\$(test-path-utils relative_path '$1' '$2')\ = '$3'
+}
+
# On Windows, we are using MSYS's bash, which mangles the paths.
# Absolute paths are anchored at the MSYS installation directory,
# which means that the path / accounts for this many characters:
@@ -183,4 +188,25 @@ test_expect_success SYMLINKS 'real path works on symlinks'
'
test $sym = $(test-path-utils real_path $dir2/syml)
'
+relative_path /a/b/c/ /a/b/ c/
+relative_path /a/b/c/ /a/bc/
+relative_path /a//b//c///a/b// c/ POSIX
+relative_path /a/b /a/b.
+relative_path /a/b//a/b.
+relative_path /a /a/b/a POSIX
+relative_path //a/b/ / POSIX
+relative_path /a/c /a/b/ /a/cPOSIX
+relative_path /a/c /a/b/a/cPOSIX
+relative_path /a/b empty /a/bPOSIX
+relative_path /a/b null/a/bPOSIX
+relative_path empty/a/b(empty)
+relative_path emptyempty (empty)
+relative_path emptynull(empty)
+relative_path null empty (null)
+relative_path null null(null)
+
+test_expect_failure 'relative path: null /a/b = segfault' '
+ test-path-utils relative_path null /a/b
+'
+
test_done
diff --git a/test-path-utils.c b/test-path-utils.c
index 0092cb..dcc530 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -28,6 +28,19 @@ static int normalize_ceiling_entry(struct string_list_item
*item, void *unused)
return 1;
}
+static void normalize_argv_string(const char **var, const char *input)
+{
+ if (!strcmp(input, null))
+ *var = NULL;
+ else if (!strcmp(input, empty))
+ *var = ;
+ else
+ *var = input;
+
+ if (*var (**var == '' || **var == '('))
+ die(Bad value: %s\n, input);
+}
+
int main(int argc, char **argv)
{
if (argc == 3 !strcmp(argv[1], normalize_path_copy)) {
@@ -103,6 +116,18 @@ int main(int argc, char **argv)
return 0;
}
+ if (argc == 4 !strcmp(argv[1], relative_path)) {
+ const char *abs, *base, *rel;
+ normalize_argv_string(abs, argv[2]);
+ normalize_argv_string(base, argv[3]);
+ rel = relative_path(abs, base);
+ if (!rel)
+ puts((null));
+ else
+ puts(strlen(rel) 0 ? rel : (empty));
+ return 0;
+ }
+
fprintf(stderr, %s: unknown function name: %s\n, argv[0],
argv[1] ? argv[1] : (there was none));
return 1;
--
1.8.3.1.756.g41beab0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v14 02/16] path.c: refactor relative_path(), not only strip prefix
Original design of relative_path() is simple, just strip the prefix
(*base) from the absolute path (*abs). In most cases, we need a real
relative path, such as: ../foo, ../../bar. That's why there is another
reimplementation (path_relative()) in quote.c.
Borrowed some codes from path_relative() in quote.c to refactor
relative_path() in path.c, so that it could return real relative path,
and user can reuse this function without reimplement his/her own.
The function path_relative() in quote.c will be substituted, and I
would use the new relative_path() function when implement the
interactive git-clean later.
Different results for relative_path() before and after this refactor:
abs path base path relative (original) relative (refactor)
= === ===
/a/b/c/ /a/b c/ c/
/a/b//c/ //a///b/ c/ c/
/a/b /a/b ../
/a/b/ /a/b ../
/a/a/b/ /a ../
/ /a/b/ /../../
/a/c /a/b/ /a/c ../c
/a/b (empty)/a/b /a/b
/a/b (null) /a/b /a/b
(empty) /a/b (empty) ./
(null)(empty)(null) ./
(null)/a/b (segfault) ./
You may notice that return value . has been changed to ./.
It is because:
* Function quote_path_relative() in quote.c will show the relative
path as ./ if abs(in) and base(prefix) are the same.
* Function relative_path() is called only once (in setup.c), and
it will be OK for the return value as ./ instead of ..
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
cache.h | 2 +-
path.c| 112 +++---
setup.c | 5 ++-
t/t0060-path-utils.sh | 27 ++--
test-path-utils.c | 4 +-
5 files changed, 107 insertions(+), 43 deletions(-)
diff --git a/cache.h b/cache.h
index dd0fb..2f10c 100644
--- a/cache.h
+++ b/cache.h
@@ -758,7 +758,7 @@ int is_directory(const char *);
const char *real_path(const char *path);
const char *real_path_if_valid(const char *path);
const char *absolute_path(const char *path);
-const char *relative_path(const char *abs, const char *base);
+const char *relative_path(const char *abs, const char *base, struct strbuf
*sb);
int normalize_path_copy(char *dst, const char *src);
int longest_ancestor_length(const char *path, struct string_list *prefixes);
char *strip_path_suffix(const char *path, const char *suffix);
diff --git a/path.c b/path.c
index 04ff..8ff16 100644
--- a/path.c
+++ b/path.c
@@ -441,42 +441,104 @@ int adjust_shared_perm(const char *path)
return 0;
}
-const char *relative_path(const char *abs, const char *base)
+/*
+ * Give relative path for abs to base.
+ *
+ * The strbuf may or may not be used, so do not assume it contains the
+ * returned path.
+ */
+const char *relative_path(const char *abs, const char *base,
+ struct strbuf *sb)
{
- static char buf[PATH_MAX + 1];
- int i = 0, j = 0;
-
- if (!base || !base[0])
+ int abs_off, base_off, i, j;
+ int abs_len, base_len;
+
+ abs_len = abs ? strlen(abs) : 0;
+ base_len = base ? strlen(base) : 0;
+ abs_off = 0;
+ base_off = 0;
+ i = 0;
+ j = 0;
+
+ if (!abs_len)
+ return ./;
+ else if (!base_len)
return abs;
- while (base[i]) {
+
+ while (i base_len j abs_len base[i] == abs[j]) {
if (is_dir_sep(base[i])) {
- if (!is_dir_sep(abs[j]))
- return abs;
while (is_dir_sep(base[i]))
i++;
while (is_dir_sep(abs[j]))
j++;
- continue;
- } else if (abs[j] != base[i]) {
+ base_off = i;
+ abs_off = j;
+ } else {
+ i++;
+ j++;
+ }
+ }
+
+ if (
+ /* base seems like prefix of abs */
+ i = base_len
+ /*
+* but /foo is not a prefix of /foobar
+* (i.e. base not end with '/')
+*/
+ base_off base_len) {
+ if (j = abs_len) {
+ /* abs=/a/b, base=/a/b */
+ abs_off = abs_len;
+ } else if (is_dir_sep(abs[j])) {
+ /* abs=/a/b/c, base=/a/b */
+ while (is_dir_sep(abs[j]))
+ j++;
+ abs_off = j;
+ } else {
+ /* abs=/a/bbb/c,
[PATCH v14 04/16] Refactor quote_path_relative, remove unused params
After substitute path_relative() in quote.c with relative_path() from
path.c, parameters (such as len and prefix_len) are obsolete in function
quote_path_relative(). Remove unused parameters and change the order of
parameters for quote_path_relative() function.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clean.c| 18 +-
builtin/grep.c | 5 ++---
builtin/ls-files.c | 2 +-
quote.c| 7 ++-
quote.h| 4 ++--
wt-status.c| 17 -
6 files changed, 24 insertions(+), 29 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 04e39..f77f95 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -56,7 +56,7 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
if ((force_flag REMOVE_DIR_KEEP_NESTED_GIT)
!resolve_gitlink_ref(path-buf, HEAD,
submodule_head)) {
if (!quiet) {
- quote_path_relative(path-buf, strlen(path-buf),
quoted, prefix);
+ quote_path_relative(path-buf, prefix, quoted);
printf(dry_run ? _(msg_would_skip_git_dir) :
_(msg_skip_git_dir),
quoted.buf);
}
@@ -70,7 +70,7 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
/* an empty dir could be removed even if it is unreadble */
res = dry_run ? 0 : rmdir(path-buf);
if (res) {
- quote_path_relative(path-buf, strlen(path-buf),
quoted, prefix);
+ quote_path_relative(path-buf, prefix, quoted);
warning(_(msg_warn_remove_failed), quoted.buf);
*dir_gone = 0;
}
@@ -94,7 +94,7 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
if (remove_dirs(path, prefix, force_flag, dry_run,
quiet, gone))
ret = 1;
if (gone) {
- quote_path_relative(path-buf,
strlen(path-buf), quoted, prefix);
+ quote_path_relative(path-buf, prefix, quoted);
string_list_append(dels, quoted.buf);
} else
*dir_gone = 0;
@@ -102,10 +102,10 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
} else {
res = dry_run ? 0 : unlink(path-buf);
if (!res) {
- quote_path_relative(path-buf,
strlen(path-buf), quoted, prefix);
+ quote_path_relative(path-buf, prefix, quoted);
string_list_append(dels, quoted.buf);
} else {
- quote_path_relative(path-buf,
strlen(path-buf), quoted, prefix);
+ quote_path_relative(path-buf, prefix, quoted);
warning(_(msg_warn_remove_failed), quoted.buf);
*dir_gone = 0;
ret = 1;
@@ -127,7 +127,7 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
if (!res)
*dir_gone = 1;
else {
- quote_path_relative(path-buf, strlen(path-buf),
quoted, prefix);
+ quote_path_relative(path-buf, prefix, quoted);
warning(_(msg_warn_remove_failed), quoted.buf);
*dir_gone = 0;
ret = 1;
@@ -262,7 +262,7 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (remove_dirs(directory, prefix, rm_flags,
dry_run, quiet, gone))
errors++;
if (gone !quiet) {
- qname =
quote_path_relative(directory.buf, directory.len, buf, prefix);
+ qname =
quote_path_relative(directory.buf, prefix, buf);
printf(dry_run ? _(msg_would_remove) :
_(msg_remove), qname);
}
}
@@ -272,11 +272,11 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
continue;
res = dry_run ? 0 : unlink(ent-name);
if (res) {
- qname = quote_path_relative(ent-name, -1,
buf, prefix);
+ qname = quote_path_relative(ent-name, prefix,
buf);
warning(_(msg_warn_remove_failed), qname);
errors++;
[PATCH v14 16/16] test: run testcases with POSIX absolute paths on Windows
Add new subcommand mingw_path in test-path-utils, so that we can get
the expected absolute paths on Windows. For example:
COMMAND LINELinux Windows
== = ===
test-path-utils mingw_path // C:/msysgit
test-path-utils mingw_path /a/b//a/b/ C:/msysgit/a/b/
With this utility, most skipped test cases in t0060 can be runcorrectly
on Windows.
Signed-off-by: Jiang Xin worldhello@gmail.com
---
t/t0060-path-utils.sh | 73 ++-
test-path-utils.c | 5
2 files changed, 54 insertions(+), 24 deletions(-)
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 4deec..dac84 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -7,14 +7,39 @@ test_description='Test various path utilities'
. ./test-lib.sh
+mingw_path() {
+ case $2 in
+ NO_MINGW)
+ echo $1
+ ;;
+ *)
+ test-path-utils mingw_path $1
+ ;;
+ esac
+}
+
+get_prereq_flag() {
+ case $1 in
+ POSIX)
+ echo $1
+ ;;
+ *)
+ ;;
+ esac
+}
+
norm_path() {
- test_expect_success $3 normalize path: $1 = $2 \
- test \\$(test-path-utils normalize_path_copy '$1')\ = '$2'
+ expected=$(mingw_path $2 $3)
+ prereq=$(get_prereq_flag $3)
+ test_expect_success $prereq normalize path: $1 = $2 \
+ test \\$(test-path-utils normalize_path_copy '$1')\ = '$expected'
}
relative_path() {
- test_expect_success $4 relative path: $1 $2 = $3 \
- test \\$(test-path-utils relative_path '$1' '$2')\ = '$3'
+ expected=$(mingw_path $3 $4)
+ prereq=$(get_prereq_flag $4)
+ test_expect_success $prereq relative path: $1 $2 = $3 \
+ test \\$(test-path-utils relative_path '$1' '$2')\ = '$expected'
}
# On Windows, we are using MSYS's bash, which mangles the paths.
@@ -39,8 +64,8 @@ ancestor() {
test \\$actual\ = '$expected'
}
-# Absolute path tests must be skipped on Windows because due to path mangling
-# the test program never sees a POSIX-style absolute path
+# Some absolute path tests should be skipped on Windows due to path mangling
+# on POSIX-style absolute paths
case $(uname -s) in
*MINGW*)
;;
@@ -73,10 +98,10 @@ norm_path d1/s1//../s2/../../d2 d2
norm_path d1/.../d2 d1/.../d2
norm_path d1/..././../d2 d1/d2
-norm_path / / POSIX
-norm_path // / POSIX
-norm_path /// / POSIX
-norm_path /. / POSIX
+norm_path / /
+norm_path // / NO_MINGW
+norm_path /// / NO_MINGW
+norm_path /. /
norm_path /./ / POSIX
norm_path /./.. ++failed++ POSIX
norm_path /../. ++failed++ POSIX
@@ -84,19 +109,19 @@ norm_path /./../.// ++failed++ POSIX
norm_path /dir/.. / POSIX
norm_path /dir/sub/../.. / POSIX
norm_path /dir/sub/../../.. ++failed++ POSIX
-norm_path /dir /dir POSIX
-norm_path /dir// /dir/ POSIX
-norm_path /./dir /dir POSIX
-norm_path /dir/. /dir/ POSIX
-norm_path /dir///./ /dir/ POSIX
-norm_path /dir//sub/.. /dir/ POSIX
-norm_path /dir/sub/../ /dir/ POSIX
+norm_path /dir /dir
+norm_path /dir// /dir/
+norm_path /./dir /dir
+norm_path /dir/. /dir/
+norm_path /dir///./ /dir/
+norm_path /dir//sub/.. /dir/
+norm_path /dir/sub/../ /dir/
norm_path //dir/sub/../. /dir/ POSIX
-norm_path /dir/s1/../s2/ /dir/s2/ POSIX
-norm_path /d1/s1///s2/..//../s3/ /d1/s3/ POSIX
-norm_path /d1/s1//../s2/../../d2 /d2 POSIX
-norm_path /d1/.../d2 /d1/.../d2 POSIX
-norm_path /d1/..././../d2 /d1/d2 POSIX
+norm_path /dir/s1/../s2/ /dir/s2/
+norm_path /d1/s1///s2/..//../s3/ /d1/s3/
+norm_path /d1/s1//../s2/../../d2 /d2
+norm_path /d1/.../d2 /d1/.../d2
+norm_path /d1/..././../d2 /d1/d2
ancestor / / -1
ancestor /foo / 0
@@ -197,8 +222,8 @@ relative_path /a/a/b../
relative_path //a/b/ ../../
relative_path /a/c /a/b/ ../c
relative_path /a/c /a/b../c
-relative_path /a/b empty /a/bPOSIX
-relative_path /a/b null/a/bPOSIX
+relative_path /a/b empty /a/b
+relative_path /a/b null/a/b
relative_path empty/a/b./
relative_path emptyempty ./
relative_path emptynull./
diff --git a/test-path-utils.c b/test-path-utils.c
index 95ef4..699ef 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -116,6 +116,11 @@ int main(int argc, char **argv)
return 0;
}
+ if (argc == 3 !strcmp(argv[1], mingw_path)) {
+ puts(argv[2]);
+ return 0;
+ }
+
if (argc == 4 !strcmp(argv[1], relative_path)) {
struct strbuf sb = STRBUF_INIT;
const char *abs, *base, *rel;
--
1.8.3.1.756.g41beab0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at
[PATCH v14 12/16] git-clean: add select by numbers interactive action
Draw a multiple choice menu using `list_and_choose` to select items
to be deleted by numbers.
User can input:
* 1,5-7 : select 1,5,6,7 items to be deleted
* * : select all items to be deleted
* -*: unselect all, nothing will be deleted
*: (empty) finish selecting, and return back to main menu
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clean.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/builtin/clean.c b/builtin/clean.c
index 36369..643a5e 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -680,6 +680,43 @@ static int filter_by_patterns_cmd(void)
return 0;
}
+static int select_by_numbers_cmd(void)
+{
+ struct menu_opts menu_opts;
+ struct menu_stuff menu_stuff;
+ struct string_list_item *items;
+ int *chosen;
+ int i, j;
+
+ menu_opts.header = NULL;
+ menu_opts.prompt = N_(Select items to delete);
+ menu_opts.flags = 0;
+
+ menu_stuff.type = MENU_STUFF_TYPE_STRING_LIST;
+ menu_stuff.stuff = del_list;
+ menu_stuff.nr = del_list.nr;
+
+ chosen = list_and_choose(menu_opts, menu_stuff);
+ items = del_list.items;
+ for (i = 0, j = 0; i del_list.nr; i++) {
+ if (i chosen[j]) {
+ *(items[i].string) = '\0';
+ } else if (i == chosen[j]) {
+ /* delete selected item */
+ j++;
+ continue;
+ } else {
+ /* end of chosen (chosen[j] == EOF), won't delete */
+ *(items[i].string) = '\0';
+ }
+ }
+
+ string_list_remove_empty_items(del_list, 0);
+
+ free(chosen);
+ return 0;
+}
+
static int quit_cmd(void)
{
string_list_clear(del_list, 0);
@@ -693,6 +730,7 @@ static int help_cmd(void)
printf_ln(_(
clean - start cleaning\n
filter by pattern - exclude items from deletion\n
+ select by numbers - select items to be deleted by
numbers\n
quit- stop cleaning\n
help- this screen\n
? - help for prompt selection
@@ -709,6 +747,7 @@ static void interactive_main_loop(void)
struct menu_item menus[] = {
{'c', clean, 0, clean_cmd},
{'f', filter by pattern, 0,
filter_by_patterns_cmd},
+ {'s', select by numbers, 0,
select_by_numbers_cmd},
{'q', quit, 0, quit_cmd},
{'h', help, 0, help_cmd},
};
--
1.8.3.1.756.g41beab0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v14 11/16] git-clean: add filter by pattern interactive action
Add a new action for interactive git-clean: filter by pattern. When the
user chooses this action, user can input space-separated patterns (the
same syntax as gitignore), and each clean candidate that matches with
one of the patterns will be excluded from cleaning. When the user feels
it's OK, presses ENTER and backs to the confirmation dialog.
Signed-off-by: Jiang Xin worldhello@gmail.com
Suggested-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clean.c | 68 +
1 file changed, 68 insertions(+)
diff --git a/builtin/clean.c b/builtin/clean.c
index df887..36369 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -614,6 +614,72 @@ static int clean_cmd(void)
return MENU_RETURN_NO_LOOP;
}
+static int filter_by_patterns_cmd(void)
+{
+ struct dir_struct dir;
+ struct strbuf confirm = STRBUF_INIT;
+ struct strbuf **ignore_list;
+ struct string_list_item *item;
+ struct exclude_list *el;
+ int changed = -1, i;
+
+ for (;;) {
+ if (!del_list.nr)
+ break;
+
+ if (changed)
+ pretty_print_dels();
+
+ clean_print_color(CLEAN_COLOR_PROMPT);
+ printf(_(Input ignore patterns ));
+ clean_print_color(CLEAN_COLOR_RESET);
+ if (strbuf_getline(confirm, stdin, '\n') != EOF)
+ strbuf_trim(confirm);
+ else
+ putchar('\n');
+
+ /* quit filter_by_pattern mode if press ENTER or Ctrl-D */
+ if (!confirm.len)
+ break;
+
+ memset(dir, 0, sizeof(dir));
+ el = add_exclude_list(dir, EXC_CMDL, manual exclude);
+ ignore_list = strbuf_split_max(confirm, ' ', 0);
+
+ for (i = 0; ignore_list[i]; i++) {
+ strbuf_trim(ignore_list[i]);
+ if (!ignore_list[i]-len)
+ continue;
+
+ add_exclude(ignore_list[i]-buf, , 0, el, -(i+1));
+ }
+
+ changed = 0;
+ for_each_string_list_item(item, del_list) {
+ int dtype = DT_UNKNOWN;
+
+ if (is_excluded(dir, item-string, dtype)) {
+ *item-string = '\0';
+ changed++;
+ }
+ }
+
+ if (changed) {
+ string_list_remove_empty_items(del_list, 0);
+ } else {
+ clean_print_color(CLEAN_COLOR_ERROR);
+ printf_ln(_(WARNING: Cannot find items matched by:
%s), confirm.buf);
+ clean_print_color(CLEAN_COLOR_RESET);
+ }
+
+ strbuf_list_free(ignore_list);
+ clear_directory(dir);
+ }
+
+ strbuf_release(confirm);
+ return 0;
+}
+
static int quit_cmd(void)
{
string_list_clear(del_list, 0);
@@ -626,6 +692,7 @@ static int help_cmd(void)
clean_print_color(CLEAN_COLOR_HELP);
printf_ln(_(
clean - start cleaning\n
+ filter by pattern - exclude items from deletion\n
quit- stop cleaning\n
help- this screen\n
? - help for prompt selection
@@ -641,6 +708,7 @@ static void interactive_main_loop(void)
struct menu_stuff menu_stuff;
struct menu_item menus[] = {
{'c', clean, 0, clean_cmd},
+ {'f', filter by pattern, 0,
filter_by_patterns_cmd},
{'q', quit, 0, quit_cmd},
{'h', help, 0, help_cmd},
};
--
1.8.3.1.756.g41beab0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v14 14/16] git-clean: add documentation for interactive git-clean
Add new section Interactive mode for documentation of interactive git-clean. Signed-off-by: Jiang Xin worldhello@gmail.com Helped-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-clean.txt | 65 +++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt index 186e34..5bf76 100644 --- a/Documentation/git-clean.txt +++ b/Documentation/git-clean.txt @@ -39,8 +39,8 @@ OPTIONS -i:: --interactive:: - Show what would be done and the user must confirm before actually - cleaning. + Show what would be done and clean files interactively. See + ``Interactive mode'' for details. -n:: --dry-run:: @@ -69,6 +69,67 @@ OPTIONS Remove only files ignored by Git. This may be useful to rebuild everything from scratch, but keep manually created files. +Interactive mode + +When the command enters the interactive mode, it shows the +files and directories to be cleaned, and goes into its +interactive command loop. + +The command loop shows the list of subcommands available, and +gives a prompt What now . In general, when the prompt ends +with a single '', you can pick only one of the choices given +and type return, like this: + + +*** Commands *** +1: clean2: filter by pattern3: select by numbers +4: ask each 5: quit 6: help +What now 1 + + +You also could say `c` or `clean` above as long as the choice is unique. + +The main command loop has 6 subcommands. + +clean:: + + Start cleaning files and directories, and then quit. + +filter by pattern:: + + This shows the files and directories to be deleted and issues an + Input ignore patterns prompt. You can input space-seperated + patterns to exclude files and directories from deletion. + E.g. *.c *.h will excludes files end with .c and .h from + deletion. When you are satisfied with the filtered result, press + ENTER (empty) back to the main menu. + +select by numbers:: + + This shows the files and directories to be deleted and issues an + Select items to delete prompt. When the prompt ends with double + '' like this, you can make more than one selection, concatenated + with whitespace or comma. Also you can say ranges. E.g. 2-5 7,9 + to choose 2,3,4,5,7,9 from the list. If the second number in a + range is omitted, all remaining patches are taken. E.g. 7- to + choose 7,8,9 from the list. You can say '*' to choose everything. + Also when you are satisfied with the filtered result, press ENTER + (empty) back to the main menu. + +ask each:: + + This will start to clean, and you must confirm one by one in order + to delete items. Please note that this action is not as efficient + as the above two actions. + +quit:: + + This lets you quit without do cleaning. + +help:: + + Show brief usage of interactive git-clean. + SEE ALSO linkgit:gitignore[5] -- 1.8.3.1.756.g41beab0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v14 05/16] Refactor write_name_quoted_relative, remove unused params
After substitute path_relative() in quote.c with relative_path() from
path.c, parameters (such as len and prefix_len) are obsolete in function
write_name_quoted_relative(). Remove unused parameters from
write_name_quoted_relative() and related functions.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/ls-files.c | 14 --
quote.c| 3 +--
quote.h| 3 +--
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 16d4f..df83f9 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -46,10 +46,12 @@ static const char *tag_modified = ;
static const char *tag_skip_worktree = ;
static const char *tag_resolve_undo = ;
-static void write_name(const char* name, size_t len)
+static void write_name(const char *name)
{
- write_name_quoted_relative(name, len, prefix, prefix_len, stdout,
- line_terminator);
+
+ /* turn off prefix, if run with --full-name */
+ write_name_quoted_relative(name, prefix_len ? prefix : NULL,
+ stdout, line_terminator);
}
static void show_dir_entry(const char *tag, struct dir_entry *ent)
@@ -63,7 +65,7 @@ static void show_dir_entry(const char *tag, struct dir_entry
*ent)
return;
fputs(tag, stdout);
- write_name(ent-name, ent-len);
+ write_name(ent-name);
}
static void show_other_files(struct dir_struct *dir)
@@ -163,7 +165,7 @@ static void show_ce_entry(const char *tag, struct
cache_entry *ce)
find_unique_abbrev(ce-sha1,abbrev),
ce_stage(ce));
}
- write_name(ce-name, ce_namelen(ce));
+ write_name(ce-name);
if (debug_mode) {
struct stat_data *sd = ce-ce_stat_data;
@@ -198,7 +200,7 @@ static void show_ru_info(void)
printf(%s%06o %s %d\t, tag_resolve_undo, ui-mode[i],
find_unique_abbrev(ui-sha1[i], abbrev),
i + 1);
- write_name(path, len);
+ write_name(path);
}
}
}
diff --git a/quote.c b/quote.c
index ebb8..5c880 100644
--- a/quote.c
+++ b/quote.c
@@ -312,8 +312,7 @@ void write_name_quotedpfx(const char *pfx, size_t pfxlen,
fputc(terminator, fp);
}
-void write_name_quoted_relative(const char *name, size_t len,
- const char *prefix, size_t prefix_len,
+void write_name_quoted_relative(const char *name, const char *prefix,
FILE *fp, int terminator)
{
struct strbuf sb = STRBUF_INIT;
diff --git a/quote.h b/quote.h
index 5610159..ed110 100644
--- a/quote.h
+++ b/quote.h
@@ -60,8 +60,7 @@ extern void quote_two_c_style(struct strbuf *, const char *,
const char *, int);
extern void write_name_quoted(const char *name, FILE *, int terminator);
extern void write_name_quotedpfx(const char *pfx, size_t pfxlen,
const char *name, FILE *, int terminator);
-extern void write_name_quoted_relative(const char *name, size_t len,
- const char *prefix, size_t prefix_len,
+extern void write_name_quoted_relative(const char *name, const char *prefix,
FILE *fp, int terminator);
/* quote path as relative to the given prefix */
--
1.8.3.1.756.g41beab0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v14 08/16] git-clean: show items of del_list in columns
When there are lots of items to be cleaned, it is hard to see them all
in one screen. Show them in columns will solve this problem.
Signed-off-by: Jiang Xin worldhello@gmail.com
Comments-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/config.txt | 4
builtin/clean.c | 49 +++-
2 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index e203..c415f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -959,6 +959,10 @@ column.branch::
Specify whether to output branch listing in `git branch` in columns.
See `column.ui` for details.
+column.clean::
+ Specify the layout when list items in `git clean -i`, which always
+ shows files and directories in columns. See `column.ui` for details.
+
column.status::
Specify whether to output untracked files in `git status` in columns.
See `column.ui` for details.
diff --git a/builtin/clean.c b/builtin/clean.c
index 698fb..75cc6 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -13,10 +13,12 @@
#include refs.h
#include string-list.h
#include quote.h
+#include column.h
static int force = -1; /* unset */
static int interactive;
static struct string_list del_list = STRING_LIST_INIT_DUP;
+static unsigned int colopts;
static const char *const builtin_clean_usage[] = {
N_(git clean [-d] [-f] [-i] [-n] [-q] [-e pattern] [-x | -X] [--]
paths...),
@@ -31,8 +33,13 @@ static const char *msg_warn_remove_failed = N_(failed to
remove %s);
static int git_clean_config(const char *var, const char *value, void *cb)
{
- if (!strcmp(var, clean.requireforce))
+ if (!prefixcmp(var, column.))
+ return git_column_config(var, value, clean, colopts);
+
+ if (!strcmp(var, clean.requireforce)) {
force = !git_config_bool(var, value);
+ return 0;
+ }
return git_default_config(var, value, cb);
}
@@ -144,21 +151,46 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
return ret;
}
-static void interactive_main_loop(void)
+static void pretty_print_dels(void)
{
- struct strbuf confirm = STRBUF_INIT;
- struct strbuf buf = STRBUF_INIT;
+ struct string_list list = STRING_LIST_INIT_DUP;
struct string_list_item *item;
+ struct strbuf buf = STRBUF_INIT;
const char *qname;
+ struct column_options copts;
+
+ for_each_string_list_item(item, del_list) {
+ qname = quote_path_relative(item-string, NULL, buf);
+ string_list_append(list, qname);
+ }
+
+ /*
+* always enable column display, we only consult column.*
+* about layout strategy and stuff
+*/
+ colopts = (colopts ~COL_ENABLE_MASK) | COL_ENABLED;
+ memset(copts, 0, sizeof(copts));
+ copts.indent = ;
+ copts.padding = 2;
+ print_columns(list, colopts, copts);
+ putchar('\n');
+ strbuf_release(buf);
+ string_list_clear(list, 0);
+}
+
+static void interactive_main_loop(void)
+{
+ struct strbuf confirm = STRBUF_INIT;
while (del_list.nr) {
putchar('\n');
- for_each_string_list_item(item, del_list) {
- qname = quote_path_relative(item-string, NULL, buf);
- printf(_(msg_would_remove), qname);
- }
+ printf_ln(Q_(Would remove the following item:,
+Would remove the following items:,
+del_list.nr));
putchar('\n');
+ pretty_print_dels();
+
printf(_(Remove [y/n]? ));
if (strbuf_getline(confirm, stdin, '\n') != EOF) {
strbuf_trim(confirm);
@@ -184,7 +216,6 @@ static void interactive_main_loop(void)
}
}
- strbuf_release(buf);
strbuf_release(confirm);
}
--
1.8.3.1.756.g41beab0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v14 09/16] git-clean: add colors to interactive git-clean
Show header, help, error messages, and prompt in colors for interactive
git-clean. Re-use config variables, such as color.interactive and
color.interactive.slot for command `git-add--interactive`.
Signed-off-by: Jiang Xin worldhello@gmail.com
Comments-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/config.txt | 17 +--
builtin/clean.c | 73 +++-
2 files changed, 81 insertions(+), 9 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index c415f..1b31f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -879,16 +879,17 @@ The values of these variables may be specified as in
color.branch.slot.
color.interactive::
When set to `always`, always use colors for interactive prompts
- and displays (such as those used by git-add --interactive).
- When false (or `never`), never. When set to `true` or `auto`, use
- colors only when the output is to the terminal. Defaults to false.
+ and displays (such as those used by git-add --interactive and
+ git-clean --interactive). When false (or `never`), never.
+ When set to `true` or `auto`, use colors only when the output is
+ to the terminal. Defaults to false.
color.interactive.slot::
- Use customized color for 'git add --interactive'
- output. `slot` may be `prompt`, `header`, `help` or `error`, for
- four distinct types of normal output from interactive
- commands. The values of these variables may be specified as
- in color.branch.slot.
+ Use customized color for 'git add --interactive' and 'git clean
+ --interactive' output. `slot` may be `prompt`, `header`, `help`
+ or `error`, for four distinct types of normal output from
+ interactive commands. The values of these variables may be
+ specified as in color.branch.slot.
color.pager::
A boolean to enable/disable colored output when the pager is in
diff --git a/builtin/clean.c b/builtin/clean.c
index 75cc6..dfa99b 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -14,6 +14,7 @@
#include string-list.h
#include quote.h
#include column.h
+#include color.h
static int force = -1; /* unset */
static int interactive;
@@ -31,16 +32,82 @@ static const char *msg_skip_git_dir = N_(Skipping
repository %s\n);
static const char *msg_would_skip_git_dir = N_(Would skip repository %s\n);
static const char *msg_warn_remove_failed = N_(failed to remove %s);
+static int clean_use_color = -1;
+static char clean_colors[][COLOR_MAXLEN] = {
+ GIT_COLOR_RESET,
+ GIT_COLOR_NORMAL, /* PLAIN */
+ GIT_COLOR_BOLD_BLUE,/* PROMPT */
+ GIT_COLOR_BOLD, /* HEADER */
+ GIT_COLOR_BOLD_RED, /* HELP */
+ GIT_COLOR_BOLD_RED, /* ERROR */
+};
+enum color_clean {
+ CLEAN_COLOR_RESET = 0,
+ CLEAN_COLOR_PLAIN = 1,
+ CLEAN_COLOR_PROMPT = 2,
+ CLEAN_COLOR_HEADER = 3,
+ CLEAN_COLOR_HELP = 4,
+ CLEAN_COLOR_ERROR = 5,
+};
+
+static int parse_clean_color_slot(const char *var)
+{
+ if (!strcasecmp(var, reset))
+ return CLEAN_COLOR_RESET;
+ if (!strcasecmp(var, plain))
+ return CLEAN_COLOR_PLAIN;
+ if (!strcasecmp(var, prompt))
+ return CLEAN_COLOR_PROMPT;
+ if (!strcasecmp(var, header))
+ return CLEAN_COLOR_HEADER;
+ if (!strcasecmp(var, help))
+ return CLEAN_COLOR_HELP;
+ if (!strcasecmp(var, error))
+ return CLEAN_COLOR_ERROR;
+ return -1;
+}
+
static int git_clean_config(const char *var, const char *value, void *cb)
{
if (!prefixcmp(var, column.))
return git_column_config(var, value, clean, colopts);
+ /* honors the color.interactive* config variables which also
+ applied in git-add--interactive and git-stash */
+ if (!strcmp(var, color.interactive)) {
+ clean_use_color = git_config_colorbool(var, value);
+ return 0;
+ }
+ if (!prefixcmp(var, color.interactive.)) {
+ int slot = parse_clean_color_slot(var +
+ strlen(color.interactive.));
+ if (slot 0)
+ return 0;
+ if (!value)
+ return config_error_nonbool(var);
+ color_parse(value, var, clean_colors[slot]);
+ return 0;
+ }
+
if (!strcmp(var, clean.requireforce)) {
force = !git_config_bool(var, value);
return 0;
}
- return git_default_config(var, value, cb);
+
+ /* inspect the color.ui config variable and others */
+ return git_color_default_config(var, value, cb);
+}
+
+static const char *clean_get_color(enum color_clean ix)
+{
+ if (want_color(clean_use_color))
+
[PATCH v14 06/16] git-clean: refactor git-clean into two phases
Before introducing interactive git-clean, refactor git-clean operations
into two phases:
* hold cleaning items in del_list,
* and remove them in a separate loop at the end.
We will introduce interactive git-clean between the two phases. The
interactive git-clean will show what would be done and must confirm
before do real cleaning.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clean.c | 64 -
1 file changed, 45 insertions(+), 19 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index f77f95..77ec1 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -15,6 +15,7 @@
#include quote.h
static int force = -1; /* unset */
+static struct string_list del_list = STRING_LIST_INIT_DUP;
static const char *const builtin_clean_usage[] = {
N_(git clean [-d] [-f] [-n] [-q] [-e pattern] [-x | -X] [--]
paths...),
@@ -148,12 +149,13 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
- struct strbuf directory = STRBUF_INIT;
+ struct strbuf abs_path = STRBUF_INIT;
struct dir_struct dir;
static const char **pathspec;
struct strbuf buf = STRBUF_INIT;
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
struct exclude_list *el;
+ struct string_list_item *item;
const char *qname;
char *seen = NULL;
struct option options[] = {
@@ -223,6 +225,7 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
int matches = 0;
struct cache_entry *ce;
struct stat st;
+ const char *rel;
/*
* Remove the '/' at the end that directory
@@ -242,13 +245,8 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
continue; /* Yup, this one exists unmerged */
}
- /*
-* we might have removed this as part of earlier
-* recursive directory removal, so lstat() here could
-* fail with ENOENT.
-*/
if (lstat(ent-name, st))
- continue;
+ die_errno(Cannot lstat '%s', ent-name);
if (pathspec) {
memset(seen, 0, argc 0 ? argc : 1);
@@ -257,33 +255,61 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
}
if (S_ISDIR(st.st_mode)) {
- strbuf_addstr(directory, ent-name);
if (remove_directories || (matches == MATCHED_EXACTLY))
{
- if (remove_dirs(directory, prefix, rm_flags,
dry_run, quiet, gone))
- errors++;
- if (gone !quiet) {
- qname =
quote_path_relative(directory.buf, prefix, buf);
- printf(dry_run ? _(msg_would_remove) :
_(msg_remove), qname);
- }
+ rel = relative_path(ent-name, prefix, buf);
+ string_list_append(del_list, rel);
}
- strbuf_reset(directory);
} else {
if (pathspec !matches)
continue;
- res = dry_run ? 0 : unlink(ent-name);
+ rel = relative_path(ent-name, prefix, buf);
+ string_list_append(del_list, rel);
+ }
+ }
+
+ /* TODO: do interactive git-clean here, which will modify del_list */
+
+ for_each_string_list_item(item, del_list) {
+ struct stat st;
+
+ if (prefix)
+ strbuf_addstr(abs_path, prefix);
+
+ strbuf_addstr(abs_path, item-string);
+
+ /*
+* we might have removed this as part of earlier
+* recursive directory removal, so lstat() here could
+* fail with ENOENT.
+*/
+ if (lstat(abs_path.buf, st))
+ continue;
+
+ if (S_ISDIR(st.st_mode)) {
+ if (remove_dirs(abs_path, prefix, rm_flags, dry_run,
quiet, gone))
+ errors++;
+ if (gone !quiet) {
+ qname = quote_path_relative(item-string, NULL,
buf);
+ printf(dry_run ? _(msg_would_remove) :
_(msg_remove), qname);
+ }
+ } else {
+ res = dry_run ? 0 :
[PATCH v14 10/16] git-clean: use a git-add-interactive compatible UI
Rewrite menu using a new method `list_and_choose`, which is borrowed
from `git-add--interactive.perl`. We will use this framework to add
new actions for interactive git-clean later.
Please NOTE:
* Method `list_and_choose` return an array of integers, and
* it is up to you to free the allocated memory of the array.
* The array ends with EOF.
* If user pressed CTRL-D (i.e. EOF), no selection returned.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clean.c | 456
1 file changed, 427 insertions(+), 29 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index dfa99b..df887 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -50,6 +50,36 @@ enum color_clean {
CLEAN_COLOR_ERROR = 5,
};
+#define MENU_OPTS_SINGLETON01
+#define MENU_OPTS_IMMEDIATE02
+#define MENU_OPTS_LIST_ONLY04
+
+struct menu_opts {
+ const char *header;
+ const char *prompt;
+ int flags;
+};
+
+#define MENU_RETURN_NO_LOOP10
+
+struct menu_item {
+ char hotkey;
+ const char *title;
+ int selected;
+ int (*fn)();
+};
+
+enum menu_stuff_type {
+ MENU_STUFF_TYPE_STRING_LIST = 1,
+ MENU_STUFF_TYPE_MENU_ITEM
+};
+
+struct menu_stuff {
+ enum menu_stuff_type type;
+ int nr;
+ void *stuff;
+};
+
static int parse_clean_color_slot(const char *var)
{
if (!strcasecmp(var, reset))
@@ -240,54 +270,422 @@ static void pretty_print_dels(void)
copts.indent = ;
copts.padding = 2;
print_columns(list, colopts, copts);
- putchar('\n');
strbuf_release(buf);
string_list_clear(list, 0);
}
-static void interactive_main_loop(void)
+static void pretty_print_menus(struct string_list *menu_list)
+{
+ unsigned int local_colopts = 0;
+ struct column_options copts;
+
+ local_colopts = COL_ENABLED | COL_ROW;
+ memset(copts, 0, sizeof(copts));
+ copts.indent = ;
+ copts.padding = 2;
+ print_columns(menu_list, local_colopts, copts);
+}
+
+static void prompt_help_cmd(int singleton)
+{
+ clean_print_color(CLEAN_COLOR_HELP);
+ printf_ln(singleton ?
+ _(Prompt help:\n
+ 1 - select a numbered item\n
+ foo- select item based on unique prefix\n
+ - (empty) select nothing) :
+ _(Prompt help:\n
+ 1 - select a single item\n
+ 3-5- select a range of items\n
+ 2-3,6-9- select multiple ranges\n
+ foo- select item based on unique prefix\n
+ -... - unselect specified items\n
+ * - choose all items\n
+ - (empty) finish selecting));
+ clean_print_color(CLEAN_COLOR_RESET);
+}
+
+/*
+ * display menu stuff with number prefix and hotkey highlight
+ */
+static void print_highlight_menu_stuff(struct menu_stuff *stuff, int **chosen)
+{
+ struct string_list menu_list = STRING_LIST_INIT_DUP;
+ struct strbuf menu = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ struct menu_item *menu_item;
+ struct string_list_item *string_list_item;
+ int i;
+
+ switch (stuff-type) {
+ default:
+ die(Bad type of menu_staff when print menu);
+ case MENU_STUFF_TYPE_MENU_ITEM:
+ menu_item = (struct menu_item *)stuff-stuff;
+ for (i = 0; i stuff-nr; i++, menu_item++) {
+ const char *p;
+ int highlighted = 0;
+
+ p = menu_item-title;
+ if ((*chosen)[i] 0)
+ (*chosen)[i] = menu_item-selected ? 1 : 0;
+ strbuf_addf(menu, %s%2d: , (*chosen)[i] ? * : ,
i+1);
+ for (; *p; p++) {
+ if (!highlighted *p == menu_item-hotkey) {
+ strbuf_addstr(menu,
clean_get_color(CLEAN_COLOR_PROMPT));
+ strbuf_addch(menu, *p);
+ strbuf_addstr(menu,
clean_get_color(CLEAN_COLOR_RESET));
+ highlighted = 1;
+ } else {
+ strbuf_addch(menu, *p);
+ }
+ }
+ string_list_append(menu_list, menu.buf);
+ strbuf_reset(menu);
+ }
+ break;
+ case MENU_STUFF_TYPE_STRING_LIST:
+ i = 0;
+ for_each_string_list_item(string_list_item, (struct string_list
*)stuff-stuff) {
+ if ((*chosen)[i] 0)
+
[PATCH v14 07/16] git-clean: add support for -i/--interactive
Show what would be done and the user must confirm before actually
cleaning.
Would remove ...
Would remove ...
Would remove ...
Remove [y/n]?
Press y to start cleaning, and press n if you want to abort.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/git-clean.txt | 10 ++--
builtin/clean.c | 57 +
2 files changed, 60 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index bdc3a..186e34 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree
SYNOPSIS
[verse]
-'git clean' [-d] [-f] [-n] [-q] [-e pattern] [-x | -X] [--] path...
+'git clean' [-d] [-f] [-i] [-n] [-q] [-e pattern] [-x | -X] [--] path...
DESCRIPTION
---
@@ -34,7 +34,13 @@ OPTIONS
-f::
--force::
If the Git configuration variable clean.requireForce is not set
- to false, 'git clean' will refuse to run unless given -f or -n.
+ to false, 'git clean' will refuse to run unless given -f, -n or
+ -i.
+
+-i::
+--interactive::
+ Show what would be done and the user must confirm before actually
+ cleaning.
-n::
--dry-run::
diff --git a/builtin/clean.c b/builtin/clean.c
index 77ec1..698fb 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -15,10 +15,11 @@
#include quote.h
static int force = -1; /* unset */
+static int interactive;
static struct string_list del_list = STRING_LIST_INIT_DUP;
static const char *const builtin_clean_usage[] = {
- N_(git clean [-d] [-f] [-n] [-q] [-e pattern] [-x | -X] [--]
paths...),
+ N_(git clean [-d] [-f] [-i] [-n] [-q] [-e pattern] [-x | -X] [--]
paths...),
NULL
};
@@ -143,6 +144,50 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
return ret;
}
+static void interactive_main_loop(void)
+{
+ struct strbuf confirm = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ struct string_list_item *item;
+ const char *qname;
+
+ while (del_list.nr) {
+ putchar('\n');
+ for_each_string_list_item(item, del_list) {
+ qname = quote_path_relative(item-string, NULL, buf);
+ printf(_(msg_would_remove), qname);
+ }
+ putchar('\n');
+
+ printf(_(Remove [y/n]? ));
+ if (strbuf_getline(confirm, stdin, '\n') != EOF) {
+ strbuf_trim(confirm);
+ } else {
+ /* Ctrl-D is the same as quit */
+ string_list_clear(del_list, 0);
+ putchar('\n');
+ printf_ln(Bye.);
+ break;
+ }
+
+ if (confirm.len) {
+ if (!strncasecmp(confirm.buf, yes, confirm.len)) {
+ break;
+ } else if (!strncasecmp(confirm.buf, no, confirm.len)
||
+ !strncasecmp(confirm.buf, quit,
confirm.len)) {
+ string_list_clear(del_list, 0);
+ printf_ln(Bye.);
+ break;
+ } else {
+ continue;
+ }
+ }
+ }
+
+ strbuf_release(buf);
+ strbuf_release(confirm);
+}
+
int cmd_clean(int argc, const char **argv, const char *prefix)
{
int i, res;
@@ -162,6 +207,7 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
OPT__QUIET(quiet, N_(do not print names of files removed)),
OPT__DRY_RUN(dry_run, N_(dry run)),
OPT__FORCE(force, N_(force)),
+ OPT_BOOL('i', interactive, interactive, N_(interactive
cleaning)),
OPT_BOOLEAN('d', NULL, remove_directories,
N_(remove whole directories)),
{ OPTION_CALLBACK, 'e', exclude, exclude_list, N_(pattern),
@@ -188,12 +234,12 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (ignored ignored_only)
die(_(-x and -X cannot be used together));
- if (!dry_run !force) {
+ if (!interactive !dry_run !force) {
if (config_set)
- die(_(clean.requireForce set to true and neither -n
nor -f given;
+ die(_(clean.requireForce set to true and neither -i,
-n nor -f given;
refusing to clean));
else
- die(_(clean.requireForce defaults to true and neither
-n nor -f given;
+ die(_(clean.requireForce defaults to true and neither
-i, -n nor -f given;
[PATCH v14 13/16] git-clean: add ask each interactive action
Add a new action for interactive git-clean: ask each. It's just like
the rm -i command, that the user must confirm one by one for each
file or directory to be cleaned.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clean.c | 36
1 file changed, 36 insertions(+)
diff --git a/builtin/clean.c b/builtin/clean.c
index 643a5e..bf03a 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -717,6 +717,40 @@ static int select_by_numbers_cmd(void)
return 0;
}
+static int ask_each_cmd(void)
+{
+ struct strbuf confirm = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ struct string_list_item *item;
+ const char *qname;
+ int changed = 0, eof = 0;
+
+ for_each_string_list_item(item, del_list) {
+ /* Ctrl-D should stop removing files */
+ if (!eof) {
+ qname = quote_path_relative(item-string, NULL, buf);
+ printf(_(remove %s? ), qname);
+ if (strbuf_getline(confirm, stdin, '\n') != EOF) {
+ strbuf_trim(confirm);
+ } else {
+ putchar('\n');
+ eof = 1;
+ }
+ }
+ if (!confirm.len || strncasecmp(confirm.buf, yes,
confirm.len)) {
+ *item-string = '\0';
+ changed++;
+ }
+ }
+
+ if (changed)
+ string_list_remove_empty_items(del_list, 0);
+
+ strbuf_release(buf);
+ strbuf_release(confirm);
+ return MENU_RETURN_NO_LOOP;
+}
+
static int quit_cmd(void)
{
string_list_clear(del_list, 0);
@@ -731,6 +765,7 @@ static int help_cmd(void)
clean - start cleaning\n
filter by pattern - exclude items from deletion\n
select by numbers - select items to be deleted by
numbers\n
+ ask each- confirm each deletion (like \rm
-i\)\n
quit- stop cleaning\n
help- this screen\n
? - help for prompt selection
@@ -748,6 +783,7 @@ static void interactive_main_loop(void)
{'c', clean, 0, clean_cmd},
{'f', filter by pattern, 0,
filter_by_patterns_cmd},
{'s', select by numbers, 0,
select_by_numbers_cmd},
+ {'a', ask each, 0, ask_each_cmd},
{'q', quit, 0, quit_cmd},
{'h', help, 0, help_cmd},
};
--
1.8.3.1.756.g41beab0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] push: honor branch.*.push
On Mon, Jun 24, 2013 at 4:19 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Junio C Hamano wrote: # on branch master, derived from origin $ git push ram And branch.master.push is set to next? Will you let her shoot herself in the foot like this? It is not shooting in the foot, if branch.master.push is explicitly set to update next. I do not see any issue in that part. The question does not pertain to master being mapped to next; it pertains to central-workflow versus triangular-workflow: origin versus ram. If the user has set push.default to upstream, she _expects_ triangular pushes to always be denied, and this is the first violation of that rule. I'm tilting towards building a dependency between branch.name.push and push.default. I haven't followed this topic closely, and the concern described below is probably a consequence of that. Please ignore if my worries are unfounded. It seems to me like we're adding (or have already added) quite a bit of config variables and command-line options for specifying (at varying levels of specificity and overridability) either the remote to push to ($remote), or what ref to push into on the remote ($remote_ref). It worries me that we allow $remote and $remote_ref to be set _separately_ and at separate levels of specificity. I wonder if this too easily allows users to shoot themselves in the foot by specifying $remote in one place (e.g. on the command line), then (in their mind - unrelated) specifying $remote_ref in another place (e.g. branch.foo.push), and then being negatively surprised when the two conspire to push into the wrong $remote and/or $remote_ref. I haven't yet dug deep enough to figure out an obvious failure mode (and I probably should not have sent this email until I'd found one), but I wonder if we'd be better off forcing the $remote and $remote_ref configuration for a given branch to appear as more of a single unit. What if, when setting up tracking for a given branch, we immediately specified its complete pull and push targets? For example, when in a centralized workflow (e.g. push.default = upstream) and we're checking out local branch foo from origin's foo, we could set up the following configuration [1]: [branch foo] pull = origin/foo push = origin/foo In a triangular workflow (assuming we had configuration to specify such, and also a default push remote), we could then instead set up the following config: [branch foo] pull = origin/foo push = my_public/foo This leaves no ambiguity for even the most novice user as to the pull and push targets for a given branch, and it's also easy to change it, either by editing the config file directly, or by using hypothetical commands: git branch foo --pulls-from=origin/bar git branch foo --pushes-to=other_repo/foo Any git pull without arguments will fail if branch.current.pull is unset or invalid. Likewise any git push without arguments will fail if branch.current.push is unset or invalid. Obviously, specifying the remote and/or refspec on the command-line would still override, as it does today, but for the argument-less forms of git pull and git push, the hierarchy of options and defaults being consulted to figure out $remote and $remote_ref would be small and easily understandable. ...Johan [1]: I do realize that I'm abusing the foo/bar notation to mean $remote/$ref, and that this does not work in the general case where both remote names and ref names may contain slashes, or when remote names don't correspond to eponymous ref namespaces... -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v14 15/16] test: add t7301 for git-clean--interactive
Add test cases for git-clean--interactive. Signed-off-by: Jiang Xin worldhello@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7301-clean-interactive.sh | 439 +++ 1 file changed, 439 insertions(+) create mode 100755 t/t7301-clean-interactive.sh diff --git a/t/t7301-clean-interactive.sh b/t/t7301-clean-interactive.sh new file mode 100755 index 0..4e605 --- /dev/null +++ b/t/t7301-clean-interactive.sh @@ -0,0 +1,439 @@ +#!/bin/sh + +test_description='git clean -i basic tests' + +. ./test-lib.sh + +test_expect_success 'setup' ' + + mkdir -p src + touch src/part1.c Makefile + echo build .gitignore + echo \*.o .gitignore + git add . + git commit -m setup + touch src/part2.c README + git add . + +' + +test_expect_success 'git clean -i (clean)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + echo c | git clean -i + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test ! -f a.out + test -f docs/manual.txt + test ! -f src/part3.c + test ! -f src/part3.h + test ! -f src/part4.c + test ! -f src/part4.h + test -f obj.o + test -f build/lib.so + +' + +test_expect_success 'git clean -i (quit)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + echo q | git clean -i + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test -f a.out + test -f docs/manual.txt + test -f src/part3.c + test -f src/part3.h + test -f src/part4.c + test -f src/part4.h + test -f obj.o + test -f build/lib.so + +' + +test_expect_success 'git clean -i (Ctrl+D)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + echo \04 | git clean -i + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test -f a.out + test -f docs/manual.txt + test -f src/part3.c + test -f src/part3.h + test -f src/part4.c + test -f src/part4.h + test -f obj.o + test -f build/lib.so + +' + +test_expect_success 'git clean -id (filter all)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + (echo f; echo *; echo; echo c) | \ + git clean -id + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test -f a.out + test -f docs/manual.txt + test -f src/part3.c + test -f src/part3.h + test -f src/part4.c + test -f src/part4.h + test -f obj.o + test -f build/lib.so + +' + +test_expect_success 'git clean -id (filter patterns)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + (echo f; echo part3.* *.out; echo; echo c) | \ + git clean -id + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test -f a.out + test ! -f docs/manual.txt + test -f src/part3.c + test -f src/part3.h + test ! -f src/part4.c + test ! -f src/part4.h + test -f obj.o + test -f build/lib.so + +' + +test_expect_success 'git clean -id (filter patterns 2)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + (echo f; echo * !*.out; echo; echo c) | \ + git clean -id + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test ! -f a.out + test -f docs/manual.txt + test -f src/part3.c + test -f src/part3.h + test -f src/part4.c + test -f src/part4.h + test -f obj.o + test -f build/lib.so + +' + +test_expect_success 'git clean -id (select - all)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + (echo s; echo *; echo; echo c) | \ + git clean -id + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test ! -f a.out + test ! -f docs/manual.txt + test ! -f src/part3.c + test ! -f src/part3.h + test ! -f src/part4.c + test ! -f src/part4.h + test -f obj.o + test -f build/lib.so + +' +
Re: [PATCH 2/2] commit: make it work with status.short
Junio C Hamano wrote: As I said in the other message, I'll revert the merge of the topic branch from 'master' for now, and queue this on top of the topic so that we will have the preventive fix when the topic is in a better shape for the other possible breakage on the --branch thing. Thanks, a quick revert is probably the best way to go. That is, git commit --short/--porcelain/-z has these three possibilities: - work (ignoring these options); - work (showing the template in some kind of short format); or - error out (clearly indicating that we did *not* make a commit). Actually, I had a different idea: to make short (and porcelain, by extension) set s-commitable correctly. From a quick analysis, it shouldn't be a hard problem. Then, we can make $ git commit display short-form status when there is nothing to commit. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] push: honor branch.*.push
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: # on branch master, derived from origin $ git push ram And branch.master.push is set to next? Will you let her shoot herself in the foot like this? It is not shooting in the foot, if branch.master.push is explicitly set to update next. I do not see any issue in that part. The question does not pertain to master being mapped to next; it pertains to central-workflow versus triangular-workflow: origin versus ram. If the user has set push.default to upstream, she _expects_ triangular pushes to always be denied,... If the user said git push without an explicit request to push to ram, and if branch.master.pushremote was not set to ram, and still the command git push pushed the branch to ram, then I would understand what you are worried about, but otherwise I do not see how what you are saying makes sense. A safety valve is different from a straight-jacket. If you are working largely with a central repository and have push.default set to upstream, are you now disallowed to push out things to other places to ask help from your colleague to check your wip? Why? Or are you saying that with push.default set to upstream, only these two forms should be allowed? $ git push ;# no destination, no refspec $ git push there ref:spec ;# both explicitly specified -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] status: really ignore config with --porcelain
Junio C Hamano wrote: Configuration related to the output format (like color=always) must be ignored under --porcelain, but if we do not read core-ish configuration variables (e.g. core.crlf) that affect the logic to list what is changed what is not, we would not give the right result, no? Ah, ofcourse. My stupidity. My knee-jerk reaction is that, because the --porcelain output was designed to be extensible and scripts reading from it is expected to ignore what it does not understand, if the setting of status.branch is a problem, the reading side is buggy and needs to be fixed. Are you 100% sure about this? --porcelain:: Give the output in an easy-to-parse format for scripts. This is similar to the short output, but will remain stable across Git versions and regardless of user configuration. See below for details. status.branch is an exception, no doubt: it just adds one line clearly prefixed with ## to the beginning, and any script can ignore it easily. Do we have in-core reader that does not behave well when one or both of these configuration variables are set (perhaps something related to submodule?)??? Yeah, the submodule.c parser is very naively written (see submodule.c:737), and I can fix it. But the bigger question still remains: if we have such a non-robust parser in git.git itself, wouldn't you expect external parsers to be even worse? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] push: honor branch.*.push
Johan Herland jo...@herland.net writes: I haven't yet dug deep enough to figure out an obvious failure mode (and I probably should not have sent this email until I'd found one), but I wonder if we'd be better off forcing the $remote and $remote_ref configuration for a given branch to appear as more of a single unit. That sounds sensible. I could see perhaps we would want to require, for branch.*.push to be effective, branch.*.pushremote must be set (honestly speaking, branch.*.push is not my itch and I'd probably be happier if we didn't add it in the first place, though ;-). What if, when setting up tracking for a given branch, we immediately specified its complete pull and push targets? For example, when in a centralized workflow (e.g. push.default = upstream) and we're checking out local branch foo from origin's foo, we could set up the following configuration [1]: [branch foo] pull = origin/foo push = origin/foo They should both be refs/heads/foo, as these are meant to name the name in _their_ repository. I see what you are saying, but the behaviour you want happens without branch.foo.push, and the addition may be redundant. I do not immediately see what it is buying us. Other than that when the user stops being centralized and starts to push to his own publishing repository, branch.*.push needs to be removed in addition to flipping push.default from upstream to current, that is. In a triangular workflow (assuming we had configuration to specify such, and also a default push remote), we could then instead set up the following config: [branch foo] pull = origin/foo push = my_public/foo Again, these are both refs/heads/foo. This leaves no ambiguity for even the most novice user as to the pull and push targets for a given branch, and it's also easy to change it, either by editing the config file directly, or by using hypothetical commands: git branch foo --pulls-from=origin/bar git branch foo --pushes-to=other_repo/foo But you need to do that for _all_ branches when you acquire your own publishing point; isn't that a rather cumbersome usability glitch? Obviously, specifying the remote and/or refspec on the command-line would still override, as it does today, but for the argument-less forms of git pull and git push, the hierarchy of options and defaults being consulted to figure out $remote and $remote_ref would be small and easily understandable. Not really. In addition to you need to run around and change configuration for all branches issue, you can never do push.default=matching, if you always set branch.foo.push and made it stronger than push.default, no? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] push: honor branch.*.push
Junio C Hamano wrote: If the user said git push without an explicit request to push to ram, and if branch.master.pushremote was not set to ram, and still the command git push pushed the branch to ram, then I would understand what you are worried about, but otherwise I do not see how what you are saying makes sense. We currently have no system to differentiate between those two cases. A safety valve is different from a straight-jacket. If you are working largely with a central repository and have push.default set to upstream, are you now disallowed to push out things to other places to ask help from your colleague to check your wip? Why? I've been harping about how this safety valve is poorly done, and I'm the first person interested in loosening it. But I do not think this is the way to do it: drop in branch.name.push magic, and make upstream suddenly work with triangular workflows? Or are you saying that with push.default set to upstream, only these two forms should be allowed? $ git push ;# no destination, no refspec $ git push there ref:spec ;# both explicitly specified No, no. What I meant is: From the documentation of push.default, I _expect_ upstream to kick in only in the first case. In the second case, I _know_ that my push.default is inconsequential. With branch.name.push magic, you've sneaked in a push refspec under the carpet, and the first form suddenly doesn't care about push.default anymore. remote.name.push is also an under-the-carpet implementation that I don't like: it's done in push_with_options() which can completely bypass setup_default_push_refspecs(), shooting the user in the face. I want properly defined precedence and well-defined overall behavior. Not sneaky stuff. I don't have an answer, but will start a discussion with some code soon. In the meantime, since we've already figured out this series, merge it without 6/6. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] t9903: add tests for git-prompt pcmode
On Sat, Jun 22, 2013 at 01:32:38PM -0300, Eduardo D'Avila wrote: These tests where important to make sure that I wouldn't break anything during the refactorization. Having them pass before *and* after refactorization guarantees that nothing was broken (except for some subtle case that might have not be included in the tests). However, I agree that they became redundant. Would it make sense to include a patch that only removes the zsh tests, after the refactorization? That would be fine with me. Thanks, Gábor -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] documentation: add git transport security notice
Fraser Tweedale fr...@frase.id.au writes: The fact that the git transport has no end-to-end security is easily overlooked. Add a brief security notice to the GIT URLS section of the documentation stating that the git transport should be used with caution on unsecured networks. Signed-off-by: Fraser Tweedale fr...@frase.id.au --- Documentation/urls.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/urls.txt b/Documentation/urls.txt index 3ca122f..c218af5 100644 --- a/Documentation/urls.txt +++ b/Documentation/urls.txt @@ -11,6 +11,9 @@ and ftps can be used for fetching and rsync can be used for fetching and pushing, but these are inefficient and deprecated; do not use them). +The git protocol provides no end-to-end security and should be used +with caution on unsecured networks. Is this necessary? I thought we already say the git protocol does not even authenticate elsewhere in the document, and if not, I think it is a sensible thing to say here. And once it is done, I doubt it is necessary to bring up a narrower concept such as end-to-end security which requires a lot more than authentication. The only thing git protocol ensures is that the receiving end validates that what is fetched from an unknown server, and what is pushed by an unknown pusher, is internally consistent. If you allowed a push over the git protocol by enabling the receive-pack service in git daemon (not recommended), you may allow anonymous users to delete branches and to do other funky things unless you protect your repository with pre-receive hook, but that won't corrupt the repository (of course, deleting all the refs may make the repository an empty but not corrupt one, which is just as unusable as a corrupt one, so there may not be a huge practical difference). If you fetched from an unauthenticated server, possibly with MITM, over the git protocol, you may end up getting something you did not ask for, but the resulting history in your repository would still be internally consistent (the commits may be malicious ones, of course, but that is what signed tags are there to protect you against). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-options: document default similarity index
Fraser Tweedale wrote: The default similarity index of 50% is documented in gitdiffcore(7) but it is worth also mentioning it in the description of the -M/--find-renames option. Looks good from a quick look at diffcore.h: #define DEFAULT_RENAME_SCORE 3 /* rename/copy similarity minimum (50%) */ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] status: really ignore config with --porcelain
Matthieu Moy matthieu@grenoble-inp.fr writes: Basically, having the CLI parser and the config parser flip two different sets of variables, so we can discriminate who set what. What annoys me is that this is the first instance of such a requirement. I don't think it's the first instance, but I can't remember precise examples. First read config, override with command line is what we always do. One recent workaround with selective exception I can think of offhand is in diff config parser 6c374008 (diff_opt: track whether flags have been set explicitly, 2013-05-10), but I am fairly sure there are others. An older example is how show_notes_given is used in the revision traversal machinery to conditionally set show_notes. The approach I'm currently tilting towards is extending the parse-options API to allow parsing one special option early. I would argue that this is a good feature that we should have asked for when we saw 6758af89e (Merge branch 'jn/git-cmd-h-bypass-setup', 2010-12-10). What do you think? That's an option too, yes. But probably not easy to implement :-(. Isn't it essentially your second option (running the CLI parser before once, then read config, and then run the CLI parser for real)? In any case, I am still not convinced yet that status.short is a real problem if --porcelain readers trip with ## branchname output. Isn't it that the readers are broken and need fixing? They should pick out what they care about and ignore the rest. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Another core.safecrlf behavor with git diff/git status
Hi, I'm still trying to use .gitattributes text flag with CRLF line ending files under Linux. I'm surprised about the interaction between the index and the working directory, more specificaly about the interaction between git diff and git status: $ git init Initialized empty Git repository in /home/ydroneaud/tmp/.git/ $ echo test text .gitattributes $ git add .gitattributes $ git commit -m .gitattributes [master (root-commit) 67c2a06] attrib 1 file changed, 1 insertion(+) create mode 100644 .gitattributes $ printf One\r\nLine\r\n test $ git add test warning: CRLF will be replaced by LF in test. The file will have its original line endings in your working directory. $ git commit -m test [master 8b06aed] test warning: CRLF will be replaced by LF in test. The file will have its original line endings in your working directory. 1 file changed, 2 insertions(+) create mode 100644 test $ git diff # git diff report nothing $ touch test $ git diff warning: CRLF will be replaced by LF in test. The file will have its original line endings in your working directory. $ git diff# = twice warning: CRLF will be replaced by LF in test. The file will have its original line endings in your working directory. $ git status # On branch master nothing to commit, working directory clean $ git diff # git diff report nothing - Why git diff does not always report the CRLF/LF mismatch ? - Why git status does not report about the CRLF/LF mismatch before updating the index: it silently hide the CRLF/LF warning. git add, git commit report the warning. git status should probably do the same. Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
Matthieu Moy matthieu@grenoble-inp.fr writes: benoit.per...@ensimag.fr writes: diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl new file mode 100644 index 000..a2f0aa1 --- /dev/null +++ b/contrib/mw-to-git/git-mw.perl *.perl scripts are usually executable in Git's tree (although it's usually better to run the non-*.perl version). Good eyes. But if we encourage people to run non-*.perl version, perhaps we should drop the executable bit from the source, no? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 01/16] bash prompt: fix redirection coding style in tests
Use 'file' instead of ' file', in accordance with the coding guidelines. Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- t/t9903-bash-prompt.sh | 232 - 1 file changed, 116 insertions(+), 116 deletions(-) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index 15521cc4..7c7f8b97 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -14,98 +14,98 @@ actual=$TRASH_DIRECTORY/actual test_expect_success 'setup for prompt tests' ' mkdir -p subdir/subsubdir git init otherrepo - echo 1 file + echo 1 file git add file test_tick git commit -m initial git tag -a -m msg1 t1 git checkout -b b1 - echo 2 file + echo 2 file git commit -m second b1 file - echo 3 file + echo 3 file git commit -m third b1 file git tag -a -m msg2 t2 git checkout -b b2 master - echo 0 file + echo 0 file git commit -m second b2 file - echo 00 file + echo 00 file git commit -m another b2 file - echo 000 file + echo 000 file git commit -m yet another b2 file git checkout master ' test_expect_success 'gitdir - from command line (through $__git_dir)' ' - echo $TRASH_DIRECTORY/otherrepo/.git expected + echo $TRASH_DIRECTORY/otherrepo/.git expected ( __git_dir=$TRASH_DIRECTORY/otherrepo/.git - __gitdir $actual + __gitdir $actual ) test_cmp expected $actual ' test_expect_success 'gitdir - repo as argument' ' - echo otherrepo/.git expected - __gitdir otherrepo $actual + echo otherrepo/.git expected + __gitdir otherrepo $actual test_cmp expected $actual ' test_expect_success 'gitdir - remote as argument' ' - echo remote expected - __gitdir remote $actual + echo remote expected + __gitdir remote $actual test_cmp expected $actual ' test_expect_success 'gitdir - .git directory in cwd' ' - echo .git expected - __gitdir $actual + echo .git expected + __gitdir $actual test_cmp expected $actual ' test_expect_success 'gitdir - .git directory in parent' ' - echo $(pwd -P)/.git expected + echo $(pwd -P)/.git expected ( cd subdir/subsubdir - __gitdir $actual + __gitdir $actual ) test_cmp expected $actual ' test_expect_success 'gitdir - cwd is a .git directory' ' - echo . expected + echo . expected ( cd .git - __gitdir $actual + __gitdir $actual ) test_cmp expected $actual ' test_expect_success 'gitdir - parent is a .git directory' ' - echo $(pwd -P)/.git expected + echo $(pwd -P)/.git expected ( cd .git/refs/heads - __gitdir $actual + __gitdir $actual ) test_cmp expected $actual ' test_expect_success 'gitdir - $GIT_DIR set while .git directory in cwd' ' - echo $TRASH_DIRECTORY/otherrepo/.git expected + echo $TRASH_DIRECTORY/otherrepo/.git expected ( GIT_DIR=$TRASH_DIRECTORY/otherrepo/.git export GIT_DIR - __gitdir $actual + __gitdir $actual ) test_cmp expected $actual ' test_expect_success 'gitdir - $GIT_DIR set while .git directory in parent' ' - echo $TRASH_DIRECTORY/otherrepo/.git expected + echo $TRASH_DIRECTORY/otherrepo/.git expected ( GIT_DIR=$TRASH_DIRECTORY/otherrepo/.git export GIT_DIR cd subdir - __gitdir $actual + __gitdir $actual ) test_cmp expected $actual ' @@ -119,36 +119,36 @@ test_expect_success 'gitdir - non-existing $GIT_DIR' ' ' test_expect_success 'gitdir - gitfile in cwd' ' - echo $(pwd -P)/otherrepo/.git expected - echo gitdir: $TRASH_DIRECTORY/otherrepo/.git subdir/.git + echo $(pwd -P)/otherrepo/.git expected + echo gitdir: $TRASH_DIRECTORY/otherrepo/.git subdir/.git test_when_finished rm -f subdir/.git ( cd subdir - __gitdir $actual + __gitdir $actual ) test_cmp expected $actual ' test_expect_success 'gitdir - gitfile in parent' ' - echo $(pwd -P)/otherrepo/.git expected - echo gitdir: $TRASH_DIRECTORY/otherrepo/.git subdir/.git + echo $(pwd -P)/otherrepo/.git expected + echo gitdir: $TRASH_DIRECTORY/otherrepo/.git subdir/.git test_when_finished rm -f subdir/.git ( cd subdir/subsubdir - __gitdir $actual + __gitdir $actual
[PATCHv3 00/16] bash prompt speedup
Hi, displaying the git-specific bash prompt on Windows/MinGW takes quite long, long enough to be noticeable. This is mainly caused by the numerous fork()s and exec()s to create subshells and run git or other commands, which are rather expensive on Windows. This patch series eliminates many command substitutions and command executions in __git_ps1() from top to bottom by replacing them with bash builtins or consolidating them. A few timing results are shown in the log message of the last patch. Changes since v2 [1]: - The detached HEAD abbreviated object name is now unique and respects core.abbrev; see patches 5 and 11, replacing v2's patch 9. (This is why I asked the detached HEAD before root commit thing yesterday.) - Patches 12 and 16 are new. - Incorporated Peff's suggestion about using the 'write_script' helper into patch 2. - Incorporated Eric's typofix. - Rephrased a few commit messages. It applies on top of current master; 2847cae8 (prompt: squelch error output from cat, 2013-06-14) graduated recently. This patch series will conflict with Eduardo's work on refactoring the colorizing function, and the conflict is not trivial. Although there are still some open questions left with that series (using tput, zsh tests), those won't affect the conflicts between the two patch series. So, for the convenience of our maintainer, I picked up Eduardo's series, took the liberty to apply a fixup commit on top with my suggestions from [2], merged the two series, and published the result at: https://github.com/szeder/git.git bash-prompt-speedup-and-color-refactorization Eduardo, could you please also check that my conflict resolution is correct? Thanks. Best, Gábor [1] - http://thread.gmane.org/gmane.comp.version-control.git/228132 [2] - http://article.gmane.org/gmane.comp.version-control.git/228707 SZEDER Gábor (16): bash prompt: fix redirection coding style in tests bash prompt: use 'write_script' helper in interactive rebase test completion, bash prompt: move __gitdir() tests to completion test suite bash prompt: add a test for symbolic link symbolic refs bash prompt: print unique detached HEAD abbreviated object name bash prompt: return early from __git_ps1() when not in a git repository bash prompt: run 'git rev-parse --git-dir' directly instead of __gitdir() bash prompt: use bash builtins to find out rebase state bash prompt: use bash builtins to find out current branch bash prompt: combine 'git rev-parse' executions in the main code path bash prompt: combine 'git rev-parse' for detached head bash prompt: use bash builtins to check for unborn branch for dirty state bash prompt: use bash builtins to check stash state bash prompt: avoid command substitution when checking for untracked files bash prompt: avoid command substitution when finalizing gitstring bash prompt: mention that PROMPT_COMMAND mode is faster contrib/completion/git-completion.bash | 2 - contrib/completion/git-prompt.sh | 241 t/t9902-completion.sh | 134 ++ t/t9903-bash-prompt.sh | 323 +++-- 4 files changed, 367 insertions(+), 333 deletions(-) -- 1.8.3.1.599.g4459181 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 03/16] completion, bash prompt: move __gitdir() tests to completion test suite
Currently __gitdir() is duplicated in the git completion and prompt
scripts, while its tests are in the prompt test suite. This patch
series is about to change __git_ps1() in a way that it won't need
__gitdir() anymore and __gitdir() will be removed from the prompt
script.
So move all __gitdir() tests from the prompt test suite over to the
completion test suite. Update the setup tests so that they perform
only those steps that are necessary for each test suite.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
t/t9902-completion.sh | 134 +
t/t9903-bash-prompt.sh | 128 --
2 files changed, 134 insertions(+), 128 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 81a1657e..5469dee8 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -122,6 +122,140 @@ test_gitcomp_nl ()
invalid_variable_name='${foo.bar}'
+actual=$TRASH_DIRECTORY/actual
+
+test_expect_success 'setup for __gitdir tests' '
+ mkdir -p subdir/subsubdir
+ git init otherrepo
+'
+
+test_expect_success '__gitdir - from command line (through $__git_dir)' '
+ echo $TRASH_DIRECTORY/otherrepo/.git expected
+ (
+ __git_dir=$TRASH_DIRECTORY/otherrepo/.git
+ __gitdir $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success '__gitdir - repo as argument' '
+ echo otherrepo/.git expected
+ __gitdir otherrepo $actual
+ test_cmp expected $actual
+'
+
+test_expect_success '__gitdir - remote as argument' '
+ echo remote expected
+ __gitdir remote $actual
+ test_cmp expected $actual
+'
+
+test_expect_success '__gitdir - .git directory in cwd' '
+ echo .git expected
+ __gitdir $actual
+ test_cmp expected $actual
+'
+
+test_expect_success '__gitdir - .git directory in parent' '
+ echo $(pwd -P)/.git expected
+ (
+ cd subdir/subsubdir
+ __gitdir $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success '__gitdir - cwd is a .git directory' '
+ echo . expected
+ (
+ cd .git
+ __gitdir $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success '__gitdir - parent is a .git directory' '
+ echo $(pwd -P)/.git expected
+ (
+ cd .git/refs/heads
+ __gitdir $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success '__gitdir - $GIT_DIR set while .git directory in cwd' '
+ echo $TRASH_DIRECTORY/otherrepo/.git expected
+ (
+ GIT_DIR=$TRASH_DIRECTORY/otherrepo/.git
+ export GIT_DIR
+ __gitdir $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success '__gitdir - $GIT_DIR set while .git directory in parent' '
+ echo $TRASH_DIRECTORY/otherrepo/.git expected
+ (
+ GIT_DIR=$TRASH_DIRECTORY/otherrepo/.git
+ export GIT_DIR
+ cd subdir
+ __gitdir $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success '__gitdir - non-existing $GIT_DIR' '
+ (
+ GIT_DIR=$TRASH_DIRECTORY/non-existing
+ export GIT_DIR
+ test_must_fail __gitdir
+ )
+'
+
+test_expect_success '__gitdir - gitfile in cwd' '
+ echo $(pwd -P)/otherrepo/.git expected
+ echo gitdir: $TRASH_DIRECTORY/otherrepo/.git subdir/.git
+ test_when_finished rm -f subdir/.git
+ (
+ cd subdir
+ __gitdir $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success '__gitdir - gitfile in parent' '
+ echo $(pwd -P)/otherrepo/.git expected
+ echo gitdir: $TRASH_DIRECTORY/otherrepo/.git subdir/.git
+ test_when_finished rm -f subdir/.git
+ (
+ cd subdir/subsubdir
+ __gitdir $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success SYMLINKS '__gitdir - resulting path avoids symlinks' '
+ echo $(pwd -P)/otherrepo/.git expected
+ mkdir otherrepo/dir
+ test_when_finished rm -rf otherrepo/dir
+ ln -s otherrepo/dir link
+ test_when_finished rm -f link
+ (
+ cd link
+ __gitdir $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success '__gitdir - not a git repository' '
+ (
+ cd subdir/subsubdir
+ GIT_CEILING_DIRECTORIES=$TRASH_DIRECTORY
+ export GIT_CEILING_DIRECTORIES
+ test_must_fail __gitdir
+ )
+'
+
test_expect_success '__gitcomp - trailing space - options' '
test_gitcomp --re --dry-run --reuse-message= --reedit-message=
--reset-author -EOF
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 442b9a20..df36239a 100755
---
[PATCHv3 04/16] bash prompt: add a test for symbolic link symbolic refs
Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- t/t9903-bash-prompt.sh | 9 + 1 file changed, 9 insertions(+) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index df36239a..416e6219 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -40,6 +40,15 @@ test_expect_success 'prompt - branch name' ' test_cmp expected $actual ' +test_expect_success SYMLINKS 'prompt - branch name - symlink symref' ' + printf (master) expected + test_when_finished git checkout master + test_config core.preferSymlinkRefs true + git checkout master + __git_ps1 $actual + test_cmp expected $actual +' + test_expect_success 'prompt - detached head' ' printf ((%s...)) $(git log -1 --format=%h b1^) expected git checkout b1^ -- 1.8.3.1.599.g4459181 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 05/16] bash prompt: print unique detached HEAD abbreviated object name
When describing a detached HEAD according to the $GIT_PS1_DESCRIBE environment variable fails, __git_ps1() runs 'cut -c1-7 .git/HEAD' to show the 7 hexdigits abbreviated commit object name in the prompt. Obviously, this neither respects core.abbrev nor produces a unique object name. Fix this by using 'git rev-parse --short HEAD' instead and adjust the corresponding test to use non-standard number of hexdigits. Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- contrib/completion/git-prompt.sh | 2 +- t/t9903-bash-prompt.sh | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 07a6218d..3c5e62bb 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -392,7 +392,7 @@ __git_ps1 () git describe --tags --exact-match HEAD ;; esac 2/dev/null) || - b=$(cut -c1-7 $g/HEAD 2/dev/null)... || + b=$(git rev-parse --short HEAD 2/dev/null)... || b=unknown b=($b) } diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index 416e6219..0d53aa6d 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -50,7 +50,8 @@ test_expect_success SYMLINKS 'prompt - branch name - symlink symref' ' ' test_expect_success 'prompt - detached head' ' - printf ((%s...)) $(git log -1 --format=%h b1^) expected + printf ((%s...)) $(git log -1 --format=%h --abbrev=13 b1^) expected + test_config core.abbrev 13 git checkout b1^ test_when_finished git checkout master __git_ps1 $actual -- 1.8.3.1.599.g4459181 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 02/16] bash prompt: use 'write_script' helper in interactive rebase test
Helped-by: Jeff King p...@peff.net Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- t/t9903-bash-prompt.sh | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index 7c7f8b97..442b9a20 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -248,14 +248,12 @@ test_expect_success 'prompt - inside bare repository' ' test_expect_success 'prompt - interactive rebase' ' printf (b1|REBASE-i 2/3) expected - echo #!$SHELL_PATH fake_editor.sh - cat fake_editor.sh \EOF -echo exec echo $1 -echo edit $(git log -1 --format=%h) $1 -echo exec echo $1 -EOF + write_script fake_editor.sh -\EOF + echo exec echo $1 + echo edit $(git log -1 --format=%h) $1 + echo exec echo $1 + EOF test_when_finished rm -f fake_editor.sh - chmod a+x fake_editor.sh test_set_editor $TRASH_DIRECTORY/fake_editor.sh git checkout b1 test_when_finished git checkout master -- 1.8.3.1.599.g4459181 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Another core.safecrlf behavor with git diff/git status
Le 24.06.2013 18:37, Yann Droneaud a écrit : I'm still trying to use .gitattributes text flag with CRLF line ending files under Linux. I'm surprised about the interaction between the index and the working directory, more specificaly about the interaction between git diff and git status: [...] - Why git diff does not always report the CRLF/LF mismatch ? - Why git status does not report about the CRLF/LF mismatch before updating the index: it silently hide the CRLF/LF warning. git add, git commit report the warning. git status should probably do the same. Can this problem be related to the rebase failure I've described in thread git rebase fail with CRLF conversion [1][2][3] ? 1. fb20a7d711fdd218f58f1f2090b1c...@meuh.org 2. http://thread.gmane.org/gmane.comp.version-control.git/228613 3. http://marc.info/?l=gitm=137182211414404w=2 Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 07/16] bash prompt: run 'git rev-parse --git-dir' directly instead of __gitdir()
__git_ps1() finds out the path to the repository by using the
__gitdir() helper function. __gitdir() is basically just a wrapper
around 'git rev-parse --git-dir', extended with support for
recognizing a remote repository given as argument, to use the path
given on the command line, and with a few shortcuts to recognize a git
repository in cwd or at $GIT_DIR quickly without actually running 'git
rev-parse'. However, the former two is only necessary for the
completion script but makes no sense for the bash prompt, while the
latter shortcuts are performance optimizations __git_ps1() can do
without (they just avoid the overhead of fork()+exec()ing a git
process).
Run 'git rev-parse --git-dir' directly in __git_ps1(), because it will
allow this patch series to combine several $(git rev-parse ...)
command substitutions in the main code path, and the overall
performance benefit will far outweigh the loss of those few shortcuts
in __gitdir(). Furthermore, since __gitdir() is not needed anymore
for the prompt, remove it from the prompt script finally eliminating
its duplication between the prompt and completion scripts. Also
remove the comment from the completion script warning about this code
duplication.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-completion.bash | 2 --
contrib/completion/git-prompt.sh | 26 +-
2 files changed, 1 insertion(+), 27 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6c3bafee..ebc40d48 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -33,8 +33,6 @@ esac
# returns location of .git repo
__gitdir ()
{
- # Note: this function is duplicated in git-prompt.sh
- # When updating it, make sure you update the other one to match.
if [ -z ${1-} ]; then
if [ -n ${__git_dir-} ]; then
echo $__git_dir
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index a915b04c..0563dea8 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -80,30 +80,6 @@
# GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on
# the colored output of git status -sb.
-# __gitdir accepts 0 or 1 arguments (i.e., location)
-# returns location of .git repo
-__gitdir ()
-{
- # Note: this function is duplicated in git-completion.bash
- # When updating it, make sure you update the other one to match.
- if [ -z ${1-} ]; then
- if [ -n ${__git_dir-} ]; then
- echo $__git_dir
- elif [ -n ${GIT_DIR-} ]; then
- test -d ${GIT_DIR-} || return 1
- echo $GIT_DIR
- elif [ -d .git ]; then
- echo .git
- else
- git rev-parse --git-dir 2/dev/null
- fi
- elif [ -d $1/.git ]; then
- echo $1/.git
- else
- echo $1
- fi
-}
-
# stores the divergence from upstream in $p
# used by GIT_PS1_SHOWUPSTREAM
__git_ps1_show_upstream ()
@@ -335,7 +311,7 @@ __git_ps1 ()
;;
esac
- local g=$(__gitdir)
+ local g=$(git rev-parse --git-dir 2/dev/null)
if [ -z $g ]; then
if [ $pcmode = yes ]; then
#In PC mode PS1 always needs to be set
--
1.8.3.1.599.g4459181
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 06/16] bash prompt: return early from __git_ps1() when not in a git repository
... to gain one level of indentation for the bulk of the function.
(The patch looks quite unreadable, you'd better check it with 'git
diff -w'.)
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-prompt.sh | 201 ---
1 file changed, 101 insertions(+), 100 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 3c5e62bb..a915b04c 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -341,121 +341,122 @@ __git_ps1 ()
#In PC mode PS1 always needs to be set
PS1=$ps1pc_start$ps1pc_end
fi
+ return
+ fi
+
+ local r=
+ local b=
+ local step=
+ local total=
+ if [ -d $g/rebase-merge ]; then
+ b=$(cat $g/rebase-merge/head-name 2/dev/null)
+ step=$(cat $g/rebase-merge/msgnum 2/dev/null)
+ total=$(cat $g/rebase-merge/end 2/dev/null)
+ if [ -f $g/rebase-merge/interactive ]; then
+ r=|REBASE-i
+ else
+ r=|REBASE-m
+ fi
else
- local r=
- local b=
- local step=
- local total=
- if [ -d $g/rebase-merge ]; then
- b=$(cat $g/rebase-merge/head-name 2/dev/null)
- step=$(cat $g/rebase-merge/msgnum 2/dev/null)
- total=$(cat $g/rebase-merge/end 2/dev/null)
- if [ -f $g/rebase-merge/interactive ]; then
- r=|REBASE-i
+ if [ -d $g/rebase-apply ]; then
+ step=$(cat $g/rebase-apply/next 2/dev/null)
+ total=$(cat $g/rebase-apply/last 2/dev/null)
+ if [ -f $g/rebase-apply/rebasing ]; then
+ b=$(cat $g/rebase-apply/head-name
2/dev/null)
+ r=|REBASE
+ elif [ -f $g/rebase-apply/applying ]; then
+ r=|AM
else
- r=|REBASE-m
- fi
- else
- if [ -d $g/rebase-apply ]; then
- step=$(cat $g/rebase-apply/next 2/dev/null)
- total=$(cat $g/rebase-apply/last 2/dev/null)
- if [ -f $g/rebase-apply/rebasing ]; then
- b=$(cat $g/rebase-apply/head-name
2/dev/null)
- r=|REBASE
- elif [ -f $g/rebase-apply/applying ]; then
- r=|AM
- else
- r=|AM/REBASE
- fi
- elif [ -f $g/MERGE_HEAD ]; then
- r=|MERGING
- elif [ -f $g/CHERRY_PICK_HEAD ]; then
- r=|CHERRY-PICKING
- elif [ -f $g/REVERT_HEAD ]; then
- r=|REVERTING
- elif [ -f $g/BISECT_LOG ]; then
- r=|BISECTING
+ r=|AM/REBASE
fi
+ elif [ -f $g/MERGE_HEAD ]; then
+ r=|MERGING
+ elif [ -f $g/CHERRY_PICK_HEAD ]; then
+ r=|CHERRY-PICKING
+ elif [ -f $g/REVERT_HEAD ]; then
+ r=|REVERTING
+ elif [ -f $g/BISECT_LOG ]; then
+ r=|BISECTING
+ fi
- test -n $b ||
- b=$(git symbolic-ref HEAD 2/dev/null) || {
- detached=yes
- b=$(
- case ${GIT_PS1_DESCRIBE_STYLE-} in
- (contains)
- git describe --contains HEAD ;;
- (branch)
- git describe --contains --all HEAD ;;
- (describe)
- git describe HEAD ;;
- (* | default)
- git describe --tags --exact-match HEAD
;;
- esac 2/dev/null) ||
+ test -n $b ||
+ b=$(git symbolic-ref HEAD 2/dev/null) || {
+ detached=yes
+ b=$(
+ case ${GIT_PS1_DESCRIBE_STYLE-} in
+ (contains)
+ git describe --contains HEAD ;;
+ (branch)
+ git describe --contains --all HEAD ;;
+
[PATCHv3 09/16] bash prompt: use bash builtins to find out current branch
__git_ps1() runs the '$(git symbolic-ref HEAD)' command substitution
to find out whether we are on a branch and to find out the name of
that branch. This imposes the overhead of fork()ing a subshell and
fork()+exec()ing a git process.
Since HEAD is in most cases a single-line file and the symbolic ref
format is quite simple to recognize and parse, read and parse it using
only bash builtins, thereby sparing all that fork()+exec() overhead.
Don't display the git prompt if reading HEAD fails, because a readable
HEAD is required for a git repository. HEAD can also be a symlink
symbolic ref (due to 'core.preferSymlinkRefs'), so use bash builtins
for reading HEAD only when HEAD is not a symlink.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-prompt.sh | 51 ++--
1 file changed, 33 insertions(+), 18 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index bc402f56..c2050b69 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -355,25 +355,40 @@ __git_ps1 ()
r=|BISECTING
fi
- test -n $b ||
- b=$(git symbolic-ref HEAD 2/dev/null) || {
- detached=yes
- b=$(
- case ${GIT_PS1_DESCRIBE_STYLE-} in
- (contains)
- git describe --contains HEAD ;;
- (branch)
- git describe --contains --all HEAD ;;
- (describe)
- git describe HEAD ;;
- (* | default)
- git describe --tags --exact-match HEAD ;;
- esac 2/dev/null) ||
+ if [ -n $b ]; then
+ :
+ elif [ -h $g/HEAD ]; then
+ # symlink symbolic ref
+ b=$(git symbolic-ref HEAD 2/dev/null)
+ else
+ local head=
+ if ! read head 2/dev/null $g/HEAD; then
+ if [ $pcmode = yes ]; then
+ PS1=$ps1pc_start$ps1pc_end
+ fi
+ return
+ fi
+ # is it a symbolic ref?
+ b=${head#ref: }
+ if [ $head = $b ]; then
+ detached=yes
+ b=$(
+ case ${GIT_PS1_DESCRIBE_STYLE-} in
+ (contains)
+ git describe --contains HEAD ;;
+ (branch)
+ git describe --contains --all HEAD ;;
+ (describe)
+ git describe HEAD ;;
+ (* | default)
+ git describe --tags --exact-match HEAD
;;
+ esac 2/dev/null) ||
- b=$(git rev-parse --short HEAD 2/dev/null)... ||
- b=unknown
- b=($b)
- }
+ b=$(git rev-parse --short HEAD
2/dev/null)... ||
+ b=unknown
+ b=($b)
+ fi
+ fi
fi
if [ -n $step ] [ -n $total ]; then
--
1.8.3.1.599.g4459181
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 08/16] bash prompt: use bash builtins to find out rebase state
During an ongoing interactive rebase __git_ps1() finds out the name of the rebased branch, the total number of patches and the number of the current patch by executing a '$(cat .git/rebase-merge/FILE)' command substitution for each. That is not quite the most efficient way to read single line single word files, because it imposes the overhead of fork()ing a subshell and fork()+exec()ing 'cat' several times. Use the 'read' bash builtin instead to avoid those overheads. Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- contrib/completion/git-prompt.sh | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 0563dea8..bc402f56 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -325,9 +325,9 @@ __git_ps1 () local step= local total= if [ -d $g/rebase-merge ]; then - b=$(cat $g/rebase-merge/head-name 2/dev/null) - step=$(cat $g/rebase-merge/msgnum 2/dev/null) - total=$(cat $g/rebase-merge/end 2/dev/null) + read b 2/dev/null $g/rebase-merge/head-name + read step 2/dev/null $g/rebase-merge/msgnum + read total 2/dev/null $g/rebase-merge/end if [ -f $g/rebase-merge/interactive ]; then r=|REBASE-i else @@ -335,10 +335,10 @@ __git_ps1 () fi else if [ -d $g/rebase-apply ]; then - step=$(cat $g/rebase-apply/next 2/dev/null) - total=$(cat $g/rebase-apply/last 2/dev/null) + read step 2/dev/null $g/rebase-apply/next + read total 2/dev/null $g/rebase-apply/last if [ -f $g/rebase-apply/rebasing ]; then - b=$(cat $g/rebase-apply/head-name 2/dev/null) + read b 2/dev/null $g/rebase-apply/head-name r=|REBASE elif [ -f $g/rebase-apply/applying ]; then r=|AM -- 1.8.3.1.599.g4459181 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 10/16] bash prompt: combine 'git rev-parse' executions in the main code path
There are a couple of '$(git rev-parse --opt)' command substitutions
in __git_ps1() and three of them are executed in the main code path:
- the first to get the path to the .git directory ('--git-dir'),
- the second to check whether we're inside the .git directory
('--is-inside-git-dir'),
- and the last, depending on the results of the second, either
* to check whether it's a bare repo ('--is-bare-repository'), or
* to check whether inside a work tree ('--is-inside-work-tree').
Naturally, this imposes the overhead of fork()ing three subshells and
fork()+exec()ing three git commands.
Combine these four 'git rev-parse' queries into a single one and use
bash parameter expansions to parse the combined output, i.e. to
separate the path to the .git directory from the true/false of
'--is-inside-git-dir', etc. This way we can eliminate two of the
three subshells and git commands.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-prompt.sh | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index c2050b69..7d226251 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -311,8 +311,9 @@ __git_ps1 ()
;;
esac
- local g=$(git rev-parse --git-dir 2/dev/null)
- if [ -z $g ]; then
+ local repo_info=$(git rev-parse --git-dir --is-inside-git-dir \
+ --is-bare-repository --is-inside-work-tree 2/dev/null)
+ if [ -z $repo_info ]; then
if [ $pcmode = yes ]; then
#In PC mode PS1 always needs to be set
PS1=$ps1pc_start$ps1pc_end
@@ -320,6 +321,13 @@ __git_ps1 ()
return
fi
+ local inside_worktree=${repo_info##*$'\n'}
+ repo_info=${repo_info%$'\n'*}
+ local bare_repo=${repo_info##*$'\n'}
+ repo_info=${repo_info%$'\n'*}
+ local inside_gitdir=${repo_info##*$'\n'}
+ local g=${repo_info%$'\n'*}
+
local r=
local b=
local step=
@@ -402,13 +410,13 @@ __git_ps1 ()
local c=
local p=
- if [ true = $(git rev-parse --is-inside-git-dir 2/dev/null) ]; then
- if [ true = $(git rev-parse --is-bare-repository
2/dev/null) ]; then
+ if [ true = $inside_gitdir ]; then
+ if [ true = $bare_repo ]; then
c=BARE:
else
b=GIT_DIR!
fi
- elif [ true = $(git rev-parse --is-inside-work-tree 2/dev/null) ];
then
+ elif [ true = $inside_worktree ]; then
if [ -n ${GIT_PS1_SHOWDIRTYSTATE-} ]
[ $(git config --bool bash.showDirtyState) != false ]
then
--
1.8.3.1.599.g4459181
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 12/16] bash prompt: use bash builtins to check for unborn branch for dirty state
When the dirty work tree and index status indicator is enabled, __git_ps1() checks for changes in the index by running 'git diff-index --cached --quiet HEAD --' and looking at its exit code. However, that makes sense only when HEAD points to a valid commit: on an unborn branch the failure of said command would be caused by the invalid HEAD, not by changes in the index. Therefore, __git_ps1() first checks for a valid HEAD by running 'git rev-parse --quiet --verify HEAD'. Since the previous patch we implicitly check HEAD's validity by running 'git rev-parse ... --short HEAD', making the dirty status indicator's 'git rev-parse' check redundant. It's sufficient to check for non-emptyness of the variable holding the abbreviated commit object name, thereby sparing the overhead of fork()+exec()ing a git process. Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- contrib/completion/git-prompt.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 88d6121d..6e8f486e 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -429,7 +429,7 @@ __git_ps1 () [ $(git config --bool bash.showDirtyState) != false ] then git diff --no-ext-diff --quiet --exit-code || w=* - if git rev-parse --quiet --verify HEAD /dev/null; then + if [ -n $short_sha ]; then git diff-index --cached --quiet HEAD -- || i=+ else i=# -- 1.8.3.1.599.g4459181 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 11/16] bash prompt: combine 'git rev-parse' for detached head
When describing a detached HEAD according to the $GIT_PS1_DESCRIBE
environment variable fails, __git_ps1() now runs the '$(git rev-parse
--short HEAD)' command substitution to get the abbreviated detached
HEAD commit object name. This imposes the overhead of fork()ing a
subshell and fork()+exec()ing a git process.
Avoid this overhead by combining this command substitution with the
main 'git rev-parse' execution for getting the path to the .git
directory co. This means that we'll look for the abbreviated commit
object name even when it's not necessary, because we're on a branch or
the detached HEAD can be described. It doesn't matter, however,
because once 'git rev-parse' is up and running to fulfill all those
other queries, the additional overhead of looking for the abbreviated
commit object name is not measurable because it's lost in the noise.
There is a caveat, however, when we are on an unborn branch, because
in that case HEAD doesn't point to a valid commit, hence the query for
the abbreviated commit object name fails. Therefore, '--short HEAD'
must be the last options to 'git rev-parse' in order to get all the
other necessary information for the prompt even on an unborn branch.
Furthermore, in that case, and in that case only, 'git rev-parse'
doesn't output the last line containing the abbreviated commit object
name, obviously, so we have to take care to only parse it if 'git
rev-parse' exited without any error.
Although there are tests already excercising __git_ps1() on unborn
branches, they all do so implicitly. Add a test that checks this
explicitly.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-prompt.sh | 16
t/t9903-bash-prompt.sh | 8
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 7d226251..88d6121d 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -311,8 +311,12 @@ __git_ps1 ()
;;
esac
- local repo_info=$(git rev-parse --git-dir --is-inside-git-dir \
- --is-bare-repository --is-inside-work-tree 2/dev/null)
+ local repo_info rev_parse_exit_code
+ repo_info=$(git rev-parse --git-dir --is-inside-git-dir \
+ --is-bare-repository --is-inside-work-tree \
+ --short HEAD 2/dev/null)
+ rev_parse_exit_code=$?
+
if [ -z $repo_info ]; then
if [ $pcmode = yes ]; then
#In PC mode PS1 always needs to be set
@@ -321,6 +325,11 @@ __git_ps1 ()
return
fi
+ local short_sha
+ if [ $rev_parse_exit_code = 0 ]; then
+ short_sha=${repo_info##*$'\n'}
+ repo_info=${repo_info%$'\n'*}
+ fi
local inside_worktree=${repo_info##*$'\n'}
repo_info=${repo_info%$'\n'*}
local bare_repo=${repo_info##*$'\n'}
@@ -392,8 +401,7 @@ __git_ps1 ()
git describe --tags --exact-match HEAD
;;
esac 2/dev/null) ||
- b=$(git rev-parse --short HEAD
2/dev/null)... ||
- b=unknown
+ b=$short_sha...
b=($b)
fi
fi
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 0d53aa6d..b9895c79 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -49,6 +49,14 @@ test_expect_success SYMLINKS 'prompt - branch name - symlink
symref' '
test_cmp expected $actual
'
+test_expect_success 'prompt - unborn branch' '
+ printf (unborn) expected
+ git checkout --orphan unborn
+ test_when_finished git checkout master
+ __git_ps1 $actual
+ test_cmp expected $actual
+'
+
test_expect_success 'prompt - detached head' '
printf ((%s...)) $(git log -1 --format=%h --abbrev=13 b1^)
expected
test_config core.abbrev 13
--
1.8.3.1.599.g4459181
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 14/16] bash prompt: avoid command substitution when checking for untracked files
When enabled, the bash prompt can indicate the presence of untracked
files with a '%' sign. __git_ps1() checks for untracked files by running the
'$(git ls-files --others --exclude-standard)' command substitution,
and displays the indicator when there is no output.
Avoid this command substitution by additionally passing
'--error-unmatch *', and checking the command's return value.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-prompt.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index afa86703..5ea6a68b 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -442,7 +442,7 @@ __git_ps1 ()
if [ -n ${GIT_PS1_SHOWUNTRACKEDFILES-} ]
[ $(git config --bool bash.showUntrackedFiles) != false
]
- [ -n $(git ls-files --others --exclude-standard) ]
+ git ls-files --others --exclude-standard --error-unmatch --
'*' /dev/null 2/dev/null
then
u=%${ZSH_VERSION+%}
fi
--
1.8.3.1.599.g4459181
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 13/16] bash prompt: use bash builtins to check stash state
When the environment variable $GIT_PS1_SHOWSTASHSTATE is set
__git_ps1() checks the presence of stashes by running 'git rev-parse
--verify refs/stash'. This command not only checks that the
'refs/stash' ref exists but also, well, verifies that it's a valid
ref.
However, we don't need to be that thorough for the bash prompt. We
can omit that verification and only check whether 'refs/stash' exists
or not. Since 'git pack-refs' never packs 'refs/stash', it's a matter
of checking the existence of a ref file. Perform this check using
only bash builtins to spare the overhead of fork()+exec()ing a git
process.
Also run 'git pack-refs --all' in the corresponding test to document
that the prompt script depends on 'git pack-refs' not packing
'refs/stash' and to catch possible breakages should this behavior ever
change.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-prompt.sh | 5 +++--
t/t9903-bash-prompt.sh | 1 +
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 6e8f486e..afa86703 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -435,8 +435,9 @@ __git_ps1 ()
i=#
fi
fi
- if [ -n ${GIT_PS1_SHOWSTASHSTATE-} ]; then
- git rev-parse --verify refs/stash /dev/null 21
s=$
+ if [ -n ${GIT_PS1_SHOWSTASHSTATE-} ]
+ [ -r $g/refs/stash ]; then
+ s=$
fi
if [ -n ${GIT_PS1_SHOWUNTRACKEDFILES-} ]
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index b9895c79..c05458cb 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -328,6 +328,7 @@ test_expect_success 'prompt - stash status indicator -
stash' '
echo 2 file
git stash
test_when_finished git stash drop
+ git pack-refs --all
(
GIT_PS1_SHOWSTASHSTATE=y
__git_ps1 $actual
--
1.8.3.1.599.g4459181
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 15/16] bash prompt: avoid command substitution when finalizing gitstring
Before setting $PS1, __git_ps1() uses a command substitution to
redirect the output from a printf into a variable. Spare the overhead
of fork()ing a subshell by using 'printf -v var' to directly assign
the output to that variable.
zsh's printf doesn't support the '-v var' option, so stick with the
command substitution when under zsh.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-prompt.sh | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 5ea6a68b..7152ae49 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -461,7 +461,11 @@ __git_ps1 ()
else
gitstring=$c${b##refs/heads/}${f:+$z$f}$r$p
fi
- gitstring=$(printf -- $printf_format $gitstring)
+ if [[ -n ${ZSH_VERSION-} ]]; then
+ gitstring=$(printf -- $printf_format $gitstring)
+ else
+ printf -v gitstring -- $printf_format $gitstring
+ fi
PS1=$ps1pc_start$gitstring$ps1pc_end
else
# NO color option unless in PROMPT_COMMAND mode
--
1.8.3.1.599.g4459181
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 16/16] bash prompt: mention that PROMPT_COMMAND mode is faster
__git_ps1() is usually added to the prompt inside a command
substitution, imposing the overhead of fork()ing a subshell. Using
__git_ps1() for $PROMPT_COMMAND is slightly faster, because it avoids
that command substitution.
Mention this in the comments about setting up the git prompt.
The whole series speeds up the bash prompt on Windows/MSysGit
considerably. Here are some timing results in three scenarios, each
repeated 10 times:
At the top of the work tree, before:
$ time for i in {0..9} ; do prompt=$(__git_ps1) ; done
real0m1.716s
user0m0.301s
sys 0m0.772s
After:
real0m0.687s
user0m0.075s
sys 0m0.396s
After, from $PROMPT_COMMAND:
$ time for i in {0..9} ; do __git_ps1 '\h:\w' '$ ' ; done
real0m0.546s
user0m0.075s
sys 0m0.181s
At the top of the work tree, detached head, before:
real0m2.574s
user0m0.376s
sys 0m1.207s
After:
real0m1.139s
user0m0.151s
sys 0m0.500s
After, from $PROMPT_COMMAND:
real0m1.030s
user0m0.245s
sys 0m0.336s
In a subdirectory, during rebase, stash status indicator enabled,
before:
real0m3.557s
user0m0.495s
sys 0m1.767s
After:
real0m0.717s
user0m0.120s
sys 0m0.300s
After, from $PROMPT_COMMAND:
real0m0.577s
user0m0.047s
sys 0m0.258s
On Linux the speedup ratio is comparable to Windows, but overall it
was about an order of magnitude faster to begin with. The last case
from above, repeated 100 times, before:
$ time for i in {0..99} ; do prompt=$(__git_ps1) ; done
real0m2.806s
user0m0.180s
sys 0m0.264s
After:
real0m0.857s
user0m0.020s
sys 0m0.028s
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-prompt.sh | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 7152ae49..daed6a1d 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -15,11 +15,11 @@
#Bash: PS1='[\u@\h \W$(__git_ps1 (%s))]\$ '
#ZSH: PS1='[%n@%m %c$(__git_ps1 (%s))]\$ '
#the optional argument will be used as format string.
-#3b) Alternatively, if you are using bash, __git_ps1 can be
-#used for PROMPT_COMMAND with two parameters, pre and
-#post, which are strings you would put in $PS1 before
-#and after the status string generated by the git-prompt
-#machinery. e.g.
+#3b) Alternatively, for a slighly faster prompt, if you are
+#using bash, __git_ps1 can be used for PROMPT_COMMAND
+#with two parameters, pre and post, which are strings
+#you would put in $PS1 before and after the status string
+#generated by the git-prompt machinery. e.g.
#Bash: PROMPT_COMMAND='__git_ps1 \u@\h:\w \\\$ '
#ZSH: precmd () { __git_ps1 %n :%~$ |%s }
#will show username, at-sign, host, colon, cwd, then
--
1.8.3.1.599.g4459181
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] status: really ignore config with --porcelain
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Basically, having the CLI parser and the config parser flip two different sets of variables, so we can discriminate who set what. What annoys me is that this is the first instance of such a requirement. I don't think it's the first instance, but I can't remember precise examples. First read config, override with command line is what we always do. One recent workaround with selective exception I can think of offhand is in diff config parser 6c374008 (diff_opt: track whether flags have been set explicitly, 2013-05-10), but I am fairly sure there are others. That was the one I had in mind. The approach I'm currently tilting towards is extending the parse-options API to allow parsing one special option early. I would argue that this is a good feature that we should have asked for when we saw 6758af89e (Merge branch 'jn/git-cmd-h-bypass-setup', 2010-12-10). What do you think? That's an option too, yes. But probably not easy to implement :-(. Isn't it essentially your second option (running the CLI parser before once, then read config, and then run the CLI parser for real)? Not really. The first run should be a kind of dry-run, except for the --porcelain part. In any case, I am still not convinced yet that status.short is a real problem if --porcelain readers trip with ## branchname output. Isn't it that the readers are broken and need fixing? Before introducing status.short, scripts could call git status --porcelain and get some output. They had no way to know whether something would be added in the future. Now, they can run the same command and get a different output. To me, that's exactly what we're trying to avoid in plumbing. The configuration file here is really meant for the user, not for scripts. Scripts that want the branch information can use --branch. Scripts that do not have absolutely nothing to gain in getting this extra output (only extra parser complexity). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] push: honor branch.*.push
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: If the user said git push without an explicit request to push to ram, and if branch.master.pushremote was not set to ram, and still the command git push pushed the branch to ram, then I would understand what you are worried about, but otherwise I do not see how what you are saying makes sense. We currently have no system to differentiate between those two cases. I am not sure what two cases you are talking about? If you do not set anywhere (like branch.master.pushremote or remote.pushdefault) to push to ram, and if you did not say git push ram but just said git push, we will not push to ram (otherwise it is broken). So if the push is going to ram, the user must have asked us to push there, either via the command line, or these explicit configuration variables. And why do you need to differenciate between the command line ram and configured ram? After all, isn't the configuration merely a typesaver? If you know often push to ram, you configure to push there. If you don't, you don't. Or are you saying that with push.default set to upstream, only these two forms should be allowed? $ git push ;# no destination, no refspec $ git push there ref:spec ;# both explicitly specified No, no. What I meant is: From the documentation of push.default, I _expect_ upstream to kick in only in the first case. In the second case, I _know_ that my push.default is inconsequential. The push.default is meant to be in effect only when there is no other stronger clue from the user for what to update (e.g. command line refspec, remote.*.push). Because branch.*.push weatherbaloon patch did not have any documentation update, it did not say it is a yet another way to explicitly configure the push destination branch. Perhaps that led to your expectation for upstream to kick in. Of course, that requires, as you earlier pointed out, that the logic to read from branch.*.push need to be moved out of the push.default logic (in the weatherbaloon patch) and hosted to do_push() where it checks if there is remote-pushrefspec[]. If branch.*.push wants to defeat remote.*.push, then it should be checked before deciding to use that configured remote.*.push. I want properly defined precedence and well-defined overall behavior. Of course. There is no sneakiness. As I already said, I personally do not know what branch.*.push buys us, and will happy to drop 6/6. It is merely a weatherbaloon patch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] status: really ignore config with --porcelain
Junio C Hamano wrote: In any case, I am still not convinced yet that status.short is a real problem if --porcelain readers trip with ## branchname output. Isn't it that the readers are broken and need fixing? If you're going to read the configuration and then scramble to reset it in wt_porcelain_print(), why did you read the configuration in the first place? Should column.branch, status.submodulesummary, status.showuntrackedfiles affect the --porcelain output too? Isn't your expectation of my parser unreasonable? I will argue that --porcelain should skip the branch config, and drop down one level lower directly: to the diff-ui configuration. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Another core.safecrlf behavor with git diff/git status
Yann Droneaud ydrone...@opteya.com writes: - Why git diff does not always report the CRLF/LF mismatch ? Most likely because you are telling safecrlf not to error out but just warn, and then you are not fixing the cause of the warning? So diff would say Ok, you must know what you are doing, so I trust what is in the index, perhaps? - Why git status does not report about the CRLF/LF mismatch before updating the index: My suspicion is the same as diff. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: benoit.per...@ensimag.fr writes: diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl new file mode 100644 index 000..a2f0aa1 --- /dev/null +++ b/contrib/mw-to-git/git-mw.perl *.perl scripts are usually executable in Git's tree (although it's usually better to run the non-*.perl version). Good eyes. But if we encourage people to run non-*.perl version, perhaps we should drop the executable bit from the source, no? But by default, I'd say consistency is most important so if other *.perl are executable, we should do the same (otherwise my ls shows different colors and it's ugly ;-) ). But it may make sense to change the convention, i.e. run a chmod -x *.perl in Git's tree (in any case, people can still run perl foo.perl). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
On Mon, Jun 24, 2013 at 06:56:17PM +0200, Matthieu Moy wrote: Good eyes. But if we encourage people to run non-*.perl version, perhaps we should drop the executable bit from the source, no? But by default, I'd say consistency is most important so if other *.perl are executable, we should do the same (otherwise my ls shows different colors and it's ugly ;-) ). But it may make sense to change the convention, i.e. run a chmod -x *.perl in Git's tree (in any case, people can still run perl foo.perl). You'd probably want to also change the shell scripts, too, which are marked executable in the repo (but the source-able shell libraries are not). I don't remember the details, but I think there may be some magic with the --valgrind test option that depends on the executable bit to distinguish those two. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Another core.safecrlf behavor with git diff/git status
Le 24.06.2013 18:37, Yann Droneaud a écrit :
I'm still trying to use .gitattributes text flag with CRLF line
ending files
under Linux.
I'm surprised about the interaction between the index and the working
directory,
more specificaly about the interaction between git diff and git status:
[...]
- Why git diff does not always report the CRLF/LF mismatch ?
- Why git status does not report about the CRLF/LF mismatch before
updating the index:
it silently hide the CRLF/LF warning.
git add, git commit report the warning. git status should probably
do the same.
One last try for today, still under Linux, with git 1.8.1.4:
$ git init
Initialized empty Git repository in
/home/ydroneaud/src/tmp/onemore/work1/.git/
$ git commit --allow-empty -m root
[master (root-commit) 89c2ff9] root
$ CRLF=\r\n
$ printf Hello World 1${CRLF}Hello World 2${CRLF}Hello World
3${CRLF}Hello World 4 test
$ git add test
$ git commit -m test
[master 36d4628] test
1 file changed, 4 insertions(+)
create mode 100644 test
$ echo test text .gitattributes
$ git add .gitattributes
$ git commit -m .gitattributes
[master 3b9f3cc] .gitattributes
1 file changed, 1 insertion(+)
create mode 100644 .gitattributes
$ git diff # print nothing
$ git status
# On branch master
nothing to commit, working directory clean
$ touch test
$ git diff
warning: CRLF will be replaced by LF in test.
The file will have its original line endings in your working
directory.
diff --git a/test b/test
index b043836..63ba10f 100644
--- a/test
+++ b/test
@@ -1,4 +1,4 @@
-Hello World 1
-Hello World 2
-Hello World 3
+Hello World 1
+Hello World 2
+Hello World 3
Hello World 4
\ No newline at end of file
- commiting .gitattributes should have trigger a warning about CRLF
conversion
- doing git diff after commiting .gitattributes should have reported the
warning about CRLF/LF.
I dislike complaining all the time about such unusual corner cases, I
apologize not having patches
to provides to Git to try to fix/improve this.
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
Le 2013-06-24 18:56, Matthieu Moy a écrit : Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: benoit.per...@ensimag.fr writes: diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl new file mode 100644 index 000..a2f0aa1 --- /dev/null +++ b/contrib/mw-to-git/git-mw.perl *.perl scripts are usually executable in Git's tree (although it's usually better to run the non-*.perl version). Good eyes. But if we encourage people to run non-*.perl version, perhaps we should drop the executable bit from the source, no? But by default, I'd say consistency is most important so if other *.perl are executable, we should do the same (otherwise my ls shows different colors and it's ugly ;-) ). So it's really a *nice* catch then :) . But it may make sense to change the convention, i.e. run a chmod -x *.perl in Git's tree (in any case, people can still run perl foo.perl). For what I've seen so far of git.git, the best way would be to make it executable in this patch serie and send another patch applying that 'chmod -x'-thingy ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
