Re: [PATCH v2 2/2] completion: checkout: complete paths when ref given
On 02/15/2017 03:26 PM, SZEDER Gábor wrote: > On Tue, Feb 14, 2017 at 10:24 PM, wrote: > >> + *) >> + __git_complete_tree_file "$ref" "$cur" >> + ;; > > There is one more caveat here. > > Both our __git_complete_index_file() and Bash's builtin filename > completion lists matching paths like this: > > $ git rm contrib/co > coccinelle/contacts/ > completion/convert-grafts-to-replace-refs.sh > > i.e. the leading path components are not redundantly repeated. > > Now, with this patch in this code path the list would look like this: > > $ git checkout completion-refs-speedup contrib/co > contrib/coccinelle/ > contrib/completion/ > contrib/contacts/ > contrib/convert-grafts-to-replace-refs.sh > > See the difference? Now that you say it.. I had never noticed it though. > I once made a feeble attempt to make completion of the : > notation (i.e. what you extracted into __git_complete_tree_file()) > look like regular filename completion, but couldn't. Can you dig up what you tried out? Maybe somebody comes up with a good idea.
Re: [PATCH v2 2/2] completion: checkout: complete paths when ref given
Although I'm not convinced that completion of modified files is unnecessary,
I'm at least persuaded that not all users would welcome such a change. Given
the hint from Gabor that Alt-/ forces filesystem completion, there is even no
big win in stopping to offer further refnames after one has already been given.
If you think that this would be desirable, find a revised version below.
Otherwise drop it.
On 02/15/2017 04:11 AM, SZEDER Gábor wrote:
> On Tue, Feb 14, 2017 at 10:24 PM, wrote:
>> From: Cornelius Weig
>> Note that one corner-case is not covered by the current implementation:
>> if a refname contains a ':' and is followed by '--' the completion would
>> not recognize the valid refname.
>
> I'm not sure what you mean here. Refnames can't contain ':'.
Yes, my bad. I was confusing it with the case where filename and ref name are
identical.
>> + while [ $c -lt $cword ]; do
>> + i="${words[c]}"
>> + case "$i" in
>> + --) seen_double_dash=1 ;;
>> + -*|?*:*) ;;
>> + *) ref="$i"; break ;;
>
> I haven't tried it, but this would trigger on e.g. 'git checkout -b
> new-feature ', wouldn't it?
Correct, good eyes.
> What about
>
> $ echo "unintentional change" >>tracked-file && git add -u
> $ git rm important-file
> $ git checkout HEAD
>
> ? It seems it will offer neither 'tracked-file' nor 'important-file',
> but I think it should offer both.
Ideally yes. The latter of the two would also not work with Alt/.
---
>From d0e0c9af8a30dec479c393ae7fe75205c9b3b229 Mon Sep 17 00:00:00 2001
From: Cornelius Weig
Date: Tue, 14 Feb 2017 21:01:45 +0100
Subject: [PATCH] completion: checkout: complete paths when ref given
Git-checkout completes words starting with '--' as options and other
words as refs. Even after specifying a ref, further words not starting
with '--' are completed as refs, which is invalid for git-checkout.
With this commit completion suppresses refname suggestion after finding
what looks like a refname. Words before a '--' not starting with a '-'
and containing no ':' are considered to be refnames.
Signed-off-by: Cornelius Weig
---
contrib/completion/git-completion.bash | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6c6e1c774d..42e6463b67 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1059,7 +1059,16 @@ _git_bundle ()
_git_checkout ()
{
- __git_has_doubledash && return
+ local c=2 seen_ref=""
+ while [ $c -lt $cword ]; do
+ case "${words[c]}" in
+ --) return ;;
+ -b|-B|--orphan|--branch) ((c++)) ;;
+ -*|*:*) ;;
+ *) seen_ref="y" ;;
+ esac
+ ((c++))
+ done
case "$cur" in
--conflict=*)
@@ -1072,13 +1081,16 @@ _git_checkout ()
"
;;
*)
- # check if --track, --no-track, or --no-guess was specified
- # if so, disable DWIM mode
- local flags="--track --no-track --no-guess" track=1
- if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
- track=''
+ if [ -z "$seen_ref" ]
+ then
+ # check for --track, --no-track, or --no-guess
+ # if so, disable DWIM mode
+ local flags="--track --no-track --no-guess" track=1
+ if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
+ track=''
+ fi
+ __gitcomp_nl "$(__git_refs '' $track)"
fi
- __gitcomp_nl "$(__git_refs '' $track)"
;;
esac
}
--
2.11.1
Re: [PATCH v2 2/2] completion: checkout: complete paths when ref given
On 02/14/2017 10:31 PM, Junio C Hamano wrote: > cornelius.w...@tngtech.com writes: > >> From: Cornelius Weig >> >> Git-checkout completes words starting with '--' as options and other >> words as refs. Even after specifying a ref, further words not starting >> with '--' are completed as refs, which is invalid for git-checkout. >> >> This commit ensures that after specifying a ref, further non-option >> words are completed as paths. Four cases are considered: >> >> - If the word contains a ':', do not treat it as reference and use >>regular revlist completion. >> - If no ref is found on the command line, complete non-options as refs >>as before. >> - If the ref is HEAD or @, complete only with modified files because >>checking out unmodified files is a noop. >>This case also applies if no ref is given, but '--' is present. > > Please at least do not do this one; a completion that is or pretends > to be more clever than the end users will confuse them at best and > annoy them. Maybe the user does not recall if she touched the path > or not, and just trying to be extra sure that it matches HEAD or > index by doing "git checkout [HEAD] path". Leave the "make it > a noop" to Git, but just allow her do so. Hmm.. I'm a bit reluctant to let go of this just yet, because it was my original motivation for the whole patch. I admit that it may be confusing to not get completion in your example. However, I think that once acquainted with the new behavior, a user who wants some files cleaned would start by having a look at the list of files from "git checkout HEAD ". That's probably faster than spelling the first few characters and hit for a file that's already clean. Let's hear if anybody else has an opinion about this. > I personally feel that "git checkout ... foo" should > just fall back to the normal "path on the filesystem" without any > cleverness, instead of opening a tree object or peek into the index. I was thinking about that as well, but it's not what happens for "git checkout topic:path". And given that "git checkout topic path" refers to the same object, consistency kind of demands that the tree objects are opened in the latter case as well. However, because the differences to filesystem path completion are somewhat corner cases, I'm fine with that as long as I'm not offered ref names any longer...
[PATCH v2 2/2] completion: checkout: complete paths when ref given
From: Cornelius Weig
Git-checkout completes words starting with '--' as options and other
words as refs. Even after specifying a ref, further words not starting
with '--' are completed as refs, which is invalid for git-checkout.
This commit ensures that after specifying a ref, further non-option
words are completed as paths. Four cases are considered:
- If the word contains a ':', do not treat it as reference and use
regular revlist completion.
- If no ref is found on the command line, complete non-options as refs
as before.
- If the ref is HEAD or @, complete only with modified files because
checking out unmodified files is a noop.
This case also applies if no ref is given, but '--' is present.
- If a ref other than HEAD or @ is found, offer only valid paths from
that revision.
Note that one corner-case is not covered by the current implementation:
if a refname contains a ':' and is followed by '--' the completion would
not recognize the valid refname.
Signed-off-by: Cornelius Weig
---
contrib/completion/git-completion.bash | 39 +++---
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 4ab119d..df46f62 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1068,7 +1068,7 @@ _git_bundle ()
_git_checkout ()
{
- __git_has_doubledash && return
+ local i c=2 ref="" seen_double_dash=""
case "$cur" in
--conflict=*)
@@ -1081,13 +1081,36 @@ _git_checkout ()
"
;;
*)
- # check if --track, --no-track, or --no-guess was specified
- # if so, disable DWIM mode
- local flags="--track --no-track --no-guess" track=1
- if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
- track=''
- fi
- __gitcomp_nl "$(__git_refs '' $track)"
+ while [ $c -lt $cword ]; do
+ i="${words[c]}"
+ case "$i" in
+ --) seen_double_dash=1 ;;
+ -*|?*:*) ;;
+ *) ref="$i"; break ;;
+ esac
+ ((c++))
+ done
+
+ case "$ref,$seen_double_dash,$cur" in
+ ,,*:*)
+ __git_complete_revlist_file
+ ;;
+ ,,*)
+ # check for --track, --no-track, or --no-guess
+ # if so, disable DWIM mode
+ local flags="--track --no-track --no-guess" track=1
+ if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
+ track=''
+ fi
+ __gitcomp_nl "$(__git_refs '' $track)"
+ ;;
+ ,1,*|@,*|HEAD,*)
+ __git_complete_index_file "--modified"
+ ;;
+ *)
+ __git_complete_tree_file "$ref" "$cur"
+ ;;
+ esac
;;
esac
}
--
2.10.2
[PATCH v2 1/2] completion: extract utility to complete paths from tree-ish
From: Cornelius Weig
The function __git_complete_revlist_file understands how to complete a
path such as 'topic:ref'. In that case, the revision (topic) and
the path component (ref) are both part of the same word. However,
some cases require that the revision is specified elsewhere than the
current word for completion, such as 'git checkout topic ref'.
In order to allow callers to specify the revision, extract a utility
function to complete paths from a tree-ish object. The utility will be
used later to implement path completion for git-checkout.
Signed-off-by: Cornelius Weig
---
contrib/completion/git-completion.bash | 73 +++---
1 file changed, 41 insertions(+), 32 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6c6e1c7..4ab119d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -442,6 +442,46 @@ __git_compute_merge_strategies ()
__git_merge_strategies=$(__git_list_merge_strategies)
}
+# __git_complete_tree_file requires 2 argument:
+# 1: the the tree-like to look at for completion
+# 2: the path component to complete
+__git_complete_tree_file ()
+{
+ local pfx ls ref="$1" cur_="$2"
+ case "$cur_" in
+ ?*/*)
+ pfx="${cur_%/*}"
+ cur_="${cur_##*/}"
+ ls="$ref:$pfx"
+ pfx="$pfx/"
+ ;;
+ *)
+ ls="$ref"
+ ;;
+ esac
+
+ case "$COMP_WORDBREAKS" in
+ *:*) : great ;;
+ *) pfx="$ref:$pfx" ;;
+ esac
+
+ __gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree "$ls" 2>/dev/null \
+ | sed '/^100... blob /{
+ s,^.*,,
+ s,$, ,
+ }
+ /^12 blob /{
+ s,^.*,,
+ s,$, ,
+ }
+ /^04 tree /{
+ s,^.*,,
+ s,$,/,
+ }
+ s/^.*//')" \
+ "$pfx" "$cur_" ""
+}
+
__git_complete_revlist_file ()
{
local pfx ls ref cur_="$cur"
@@ -452,38 +492,7 @@ __git_complete_revlist_file ()
?*:*)
ref="${cur_%%:*}"
cur_="${cur_#*:}"
- case "$cur_" in
- ?*/*)
- pfx="${cur_%/*}"
- cur_="${cur_##*/}"
- ls="$ref:$pfx"
- pfx="$pfx/"
- ;;
- *)
- ls="$ref"
- ;;
- esac
-
- case "$COMP_WORDBREAKS" in
- *:*) : great ;;
- *) pfx="$ref:$pfx" ;;
- esac
-
- __gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree "$ls"
2>/dev/null \
- | sed '/^100... blob /{
- s,^.*,,
- s,$, ,
- }
- /^12 blob /{
- s,^.*,,
- s,$, ,
- }
- /^04 tree /{
- s,^.*,,
- s,$,/,
- }
- s/^.*//')" \
- "$pfx" "$cur_" ""
+ __git_complete_tree_file "$ref" "$cur_"
;;
*...*)
pfx="${cur_%...*}..."
--
2.10.2
Re: [PATCH] completion: complete modified files for checkout with '--'
On 02/14/2017 01:50 AM, SZEDER Gábor wrote: > On Tue, Feb 14, 2017 at 12:33 AM, wrote: >> From: Cornelius Weig >> >> The command line completion for git-checkout bails out when seeing '--' >> as an isolated argument. For git-checkout this signifies the start of a >> list of files which are to be checked out. Checkout of files makes only >> sense for modified files, > > No, there is e.g. 'git checkout that-branch this-path', too. Very true. Thanks for prodding me to this palpable oversight. My error was to aim for a small improvement. I think the correct approach is to improve the overall completion of git-checkout. IMHO it is a completion bug that after giving a ref, completion will still offer refs, e.g. $ git checkout HEAD --> list of refs As far as I can see, giving two refs to checkout is always an error. The correct behavior in the example above would be to offer paths instead. I'll follow up with an improved version which considers these cases.
[PATCH] completion: complete modified files for checkout with '--'
From: Cornelius Weig
The command line completion for git-checkout bails out when seeing '--'
as an isolated argument. For git-checkout this signifies the start of a
list of files which are to be checked out. Checkout of files makes only
sense for modified files, therefore completion can be a bit smarter:
Instead of bailing out, offer modified files for completion.
Signed-off-by: Cornelius Weig
---
contrib/completion/git-completion.bash | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6c6e1c7..d6523fd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1059,7 +1059,10 @@ _git_bundle ()
_git_checkout ()
{
- __git_has_doubledash && return
+ __git_has_doubledash && {
+ __git_complete_index_file "--modified"
+ return
+ }
case "$cur" in
--conflict=*)
--
2.10.2
[PATCH] completion: teach to complete git-subtree
From: Cornelius Weig
Git-subtree is a contrib-command without completion, making it
cumbersome to use.
Teach bash completion to complete the subcommands of subtree (add,
merge, pull, push, split) and options thereof. For subcommands
supporting it (add, push, pull) also complete remote names and refspec.
Signed-off-by: Cornelius Weig
---
contrib/completion/git-completion.bash | 35 ++
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6c6e1c7..430bfed 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -535,9 +535,9 @@ __git_complete_remote_or_refspec ()
{
local cur_="$cur" cmd="${words[1]}"
local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
- if [ "$cmd" = "remote" ]; then
- ((c++))
- fi
+ case "$cmd" in
+ remote|subtree) ((c++)) ;;
+ esac
while [ $c -lt $cword ]; do
i="${words[c]}"
case "$i" in
@@ -586,7 +586,7 @@ __git_complete_remote_or_refspec ()
__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
fi
;;
- pull|remote)
+ pull|remote|subtree)
if [ $lhs = 1 ]; then
__gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_"
else
@@ -2569,6 +2569,33 @@ _git_submodule ()
fi
}
+_git_subtree ()
+{
+ local subcommands="add merge pull push split"
+ local subcommand="$(__git_find_on_cmdline "$subcommands")"
+ if [ -z "$subcommand" ]; then
+ __gitcomp "$subcommands"
+ return
+ fi
+ case "$subcommand,$cur" in
+ add,--*|merge,--*|pull,--*)
+ __gitcomp "--prefix= --squash --message="
+ ;;
+ push,--*)
+ __gitcomp "--prefix="
+ ;;
+ split,--*)
+ __gitcomp "
+ --annotate= --branch= --ignore-joins
+ --onto= --prefix= --rejoin
+ "
+ ;;
+ add,*|push,*|pull,*)
+ __git_complete_remote_or_refspec
+ ;;
+ esac
+}
+
_git_svn ()
{
local subcommands="
--
2.10.2
Re: [PATCH 6/7] completion: teach remote subcommands option completion
On 02/02/2017 02:37 AM, SZEDER Gábor wrote: > The 'set-head' subcommand has '--auto' and '--delete' options, and > 'set-branches' has '--add'. Oops. Thanks for spotting this.. >> __git_complete_remote_or_refspec >> ;; >> -update) >> +update,*) > > The 'update' subcommand has a '--prune' option. > ..and that.
Re: Request re git status
On 02/07/2017 01:45 AM, Phil Hord wrote:
> On Mon, Feb 6, 2017 at 3:36 PM Ron Pero wrote:
> Do you mean you almost pushed some changed history with "--force"
> which would have lost others' changes? Use of this option is
> discouraged on shared branches for this very reason. But if you do
> use it, the remote will tell you the hash of the old branch so you can
> undo the damage.
>
> But if you did not use --force, then you were not in danger of being
> bit. Git would have prevented the push in that case.
I totally agree with Phil. Besides, git-status should be fast. And
talking to a remote can be painfully slow. As Phil pointed out, even the
slow answer when talking to the remote can give you better guarantees
than the quick (local) answer. Therefore, I prefer the quick answer.
Since you pointed out the use of --force, I want to add the
--force-with-lease option of git-push. The idea is basically, that we
may force-push, if the remote end does indeed have the state we think it
has. This avoids those situations where somebody pushed to the remote
while you were typing 'git push --force' (which would then loose the
other contributor's work). For details have a look at 'git help push'.
>> Or change the message to tell what it really
>> did, e.g. "Your branch was up-to-date with 'origin/master' when last
>> checked at {timestamp}"? Or even just say, "Do a fetch to find out
>> whether your branch is up to date"?
>
> These are reasonable suggestions, but i don't think the extra wording
> adds anything for most users. Adding a timestamp seems generally
> useful, but it could get us into other trouble since we have to depend
> on outside sources for timestamps.
The date of the last update is actually stored in the reflogs for the
remote branches. That timestamp is "internal" and could be trusted.
However, I don't quite believe that it would avoid accidents. For that
you would have to remember the time when some other (!) contributor has
pushed to the remote AND recognize that its timestamp is after the date
printed.
I prefer being warned by git when I try to do something stupid.
[PATCH v5] tag: generate useful reflog message
From: Cornelius Weig
When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
6e3a7b3 refs/tags/test@{0}:
Now, a reflog message is generated when creating a tag, following the
pattern "tag: tagging ()". If
GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
()" instead. If the tag references a commit object, the
description is set to the subject line of the commit, followed by its
commit date. For example:
6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)
If the tag points to a tree/blob/tag objects, the following static
strings are taken as description:
- "tree object"
- "blob object"
- "other tag object"
Signed-off-by: Cornelius Weig
---
Notes:
Interdiff v4..v5
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 894959f..1a3230f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -80,9 +80,10 @@ test_expect_success 'creating a tag using default HEAD
should succeed' '
test_must_fail git reflog exists refs/tags/mytag
'
-git log -1 > expected \
- --format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
test_expect_success 'creating a tag with --create-reflog should create
reflog' '
+ git log -1 \
+ --format="format:tag: tagging %h (%s, %cd)%n" \
+ --date=format:%Y-%m-%d >expected &&
test_when_finished "git tag -d tag_with_reflog" &&
git tag --create-reflog tag_with_reflog &&
git reflog exists refs/tags/tag_with_reflog &&
@@ -90,9 +91,10 @@ test_expect_success 'creating a tag with --create-reflog
should create reflog' '
test_cmp expected actual
'
-git log -1 > expected \
- --format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
test_expect_success 'annotated tag with --create-reflog has correct
message' '
+ git log -1 \
+ --format="format:tag: tagging %h (%s, %cd)%n" \
+ --date=format:%Y-%m-%d >expected &&
test_when_finished "git tag -d tag_with_reflog" &&
git tag -m "annotated tag" --create-reflog tag_with_reflog &&
git reflog exists refs/tags/tag_with_reflog &&
builtin/tag.c | 54 +-
t/t7004-tag.sh | 18 +-
2 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..bca890f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -302,6 +302,54 @@ static void create_tag(const unsigned char *object, const
char *tag,
}
}
+static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
+{
+ enum object_type type;
+ struct commit *c;
+ char *buf;
+ unsigned long size;
+ int subject_len = 0;
+ const char *subject_start;
+
+ char *rla = getenv("GIT_REFLOG_ACTION");
+ if (rla) {
+ strbuf_addstr(sb, rla);
+ } else {
+ strbuf_addstr(sb, _("tag: tagging "));
+ strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
+ }
+
+ strbuf_addstr(sb, " (");
+ type = sha1_object_info(sha1, NULL);
+ switch (type) {
+ default:
+ strbuf_addstr(sb, _("object of unknown type"));
+ break;
+ case OBJ_COMMIT:
+ if ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {
+ subject_len = find_commit_subject(buf, &subject_start);
+ strbuf_insert(sb, sb->len, subject_start, subject_len);
+ } else {
+ strbuf_addstr(sb, _("commit object"));
+ }
+ free(buf);
+
+ if ((c = lookup_commit_reference(sha1)) != NULL)
+ strbuf_addf(sb, ", %s", show_date(c->date, 0,
DATE_MODE(SHORT)));
+ break;
+ case OBJ_TREE:
+ strbuf_addstr(sb, _("tree object"));
+ break;
+ case OBJ_BLOB:
+ strbuf_addstr(sb, _("blob object"));
+ break;
+ case OBJ_TAG:
+ strbuf_addstr(sb, _("other tag object"));
+ break;
+ }
+ strbuf_addch(sb, ')');
+}
+
struct msg_arg {
int given;
struct strbuf buf;
@@ -335,6 +383,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
struct strbuf
Re: [PATCH v4] tag: generate useful reflog message
On 02/08/2017 10:28 PM, Junio C Hamano wrote:
> cornelius.w...@tngtech.com writes:
>
>> From: Cornelius Weig
>>
>> When tags are created with `--create-reflog` or with the option
>> `core.logAllRefUpdates` set to 'always', a reflog is created for them.
>> So far, the description of reflog entries for tags was empty, making the
>> reflog hard to understand. For example:
>> 6e3a7b3 refs/tags/test@{0}:
>>
>> Now, a reflog message is generated when creating a tag, following the
>> pattern "tag: tagging ()". If
>> GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
>> ()" instead. If the tag references a commit object, the
>> description is set to the subject line of the commit, followed by its
>> commit date. For example:
>> 6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0,
>> 2017-02-03)
>>
>> If the tag points to a tree/blob/tag objects, the following static
>> strings are taken as description:
>>
>> - "tree object"
>> - "blob object"
>> - "other tag object"
>>
>> Signed-off-by: Cornelius Weig
>> Reviewed-by: Junio C Hamano
>
> This last line is inappropriate, as I didn't review _THIS_ version,
> which is different from the previous one, and I haven't checked if
> the way the comments on the previous review were addressed in this
> version is agreeable.
Sorry for that confusion. I'm still not used to when adding what
sign-off is appropriate. I thought that adding you as reviewer is also a
question of courtesy.
A version with revised tests will follow.
Re: [PATCH 1/2] pathspec magic: add '^' as alias for '!'
As Duy pointed out, the glossary needs an update too. For this one, the cange can be minimal I think: diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index 8ad29e6..f127fe9 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -386,7 +386,7 @@ Glob magic is incompatible with literal magic. exclude;; After a path matches any non-exclude pathspec, it will be run - through all exclude pathspec (magic signature: `!`). If it + through all exclude pathspec (magic signature: `!` or `^`). If it matches, the path is ignored. --
Re: [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns
Again, as Duy pointed out this should be documented. How about something like this: diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index f127fe9..781cde3 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -387,7 +387,9 @@ Glob magic is incompatible with literal magic. exclude;; After a path matches any non-exclude pathspec, it will be run through all exclude pathspec (magic signature: `!` or `^`). If it - matches, the path is ignored. + matches, the path is ignored. If only exclude pathspec are given, + the exclusion is applied to the result set as if invoked without any + pathspec. -- [[def_parent]]parent::
Re: [RFC] mailmap.blob overrides default .mailmap
On 02/07/2017 08:28 PM, Jeff King wrote: > > I think it was mostly that I had to define _some_ order. This made sense > to me as similar to things like attributes or excludes, where we prefer > clone-specific data over in-history data (so .git/info/attributes takes > precedence over .gitattributes). > > So any mailmap.* would take precedence over the in-tree .mailmap file. > And then between mailmap.file and mailmap.blob, the "blob" form is > more "in-tree" than the "file" form (especially because we turn it on by > default in bare repos, so it really is identical to the in-tree form > there). So the clone-specific data wins over in-history makes perfect sense to me. Therefore, mailmap.file should win over mailmap.blob, agreed. On the other hand, a checked-in .mailmap file and a mailmap-blob are both as in-history as the other to me. Now consider the following settings: $ git config --unset mailmap.file $ git config mailmap.blob HEAD:.mailmap $ sed -i 's:p...@peff.com:no-valid-address:' .mailmap $ git log -1 --author 'Jeff King' So with the .mailmap being dirty, which address would one expect to be printed? I would expect 'no-valid-address', but it's 'p...@peff.com'. Even though the .mailmap file is more visible and also closer to the user, the mailmap.blob wins over it. I find that somewhat counter-intuitive. Of course, setting `git config mailmap.file .mailmap`, would do the trick.
Re: ``git clean -xdf'' and ``make clean''
On 02/07/2017 03:17 PM, Hongyi Zhao wrote: > Hi all, > > In order to delete all of the last build stuff, does the following two > methods equivalent or not? > > ``git clean -xdf'' and ``make clean'' No, it is not equivalent. * `make clean` removes any build-related files (assuming that the `clean` target is properly written). To see exactly what it would do, run `make clean -n`. Judging from your question, I think this is what you want to do. * `git clean -xdf` would remove any files that git does not track. This also includes build-related files, but also any other files that happen to be in your working directory. For example, any output from `git format-patch` would be removed by this, but not `make clean`.
[RFC] mailmap.blob overrides default .mailmap
Hi, I was reading into the mailmap handling today and I'm a bit puzzled by the overriding behavior. This is what the documentation says about precedence (emphasis mine): - mailmap.file The location of an augmenting mailmap file. The default mailmap, located in the root of the repository, is loaded first, then the mailmap file pointed to by this variable. The location of the mailmap file may be in a repository subdirectory, or somewhere outside of the repository itself. See git-shortlog(1) and git-blame(1). mailmap.blob Like mailmap.file, but consider the value as a reference to a blob in the repository. If both mailmap.file and mailmap.blob are given, both are !!! parsed, with _entries from mailmap.file taking precedence_. In a bare repository, this defaults to HEAD:.mailmap. In a non-bare repository, it defaults to empty. So from the doc I would have expected that files always get precedence over the blob. IOW entries from .mailmap override entries from mailmap.blob. However, this is not the case. The code shows why (mailmap.c): err |= read_mailmap_file(map, ".mailmap", repo_abbrev); if (startup_info->have_repository) err |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev); err |= read_mailmap_file(map, git_mailmap_file, repo_abbrev); Apparently this is not an oversight, because there is an explicit test for this overriding behavior (t4203 'mailmap.blob overrides .mailmap'). So I wonder: what is the rationale behind this? I find this mixed overriding behavior hard to explain and difficult to understand.
[PATCH v4] tag: generate useful reflog message
From: Cornelius Weig
Thanks for taking a look at my last version.
> On the other hand, it's not like failing to describe the tagged
> commit in the reflog is such a grave error. If we can get away with
> being vague on a tag that points at an object of unknown type like
> the above code does, we could loosen the "oops, we thought we got a
> commit, but it turns out that we cannot read it" case below from
> die() to just stuffing generic _("commit object") in the reflog.
Good point. I agree that failing to create the message should be no reason to
die().
As you also pointed out, "internal object" is no reliable description
for unhandled object types. I changed that as well.
Changes wrt v3 (interdiff below):
- change default message for unhandled object types
- do not die if commit is not readable, but use default description instead
- test: use verbatim HT character instead of \t
Cornelius Weig (1):
tag: generate useful reflog message
builtin/tag.c | 54 +-
t/t7004-tag.sh | 16 +++-
2 files changed, 68 insertions(+), 2 deletions(-)
Interdiff v3..v4:
diff --git a/builtin/tag.c b/builtin/tag.c
index 638b68e..9b2eabd 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -323,19 +323,19 @@ static void create_reflog_msg(const unsigned char *sha1,
struct strbuf *sb)
type = sha1_object_info(sha1, NULL);
switch (type) {
default:
- strbuf_addstr(sb, _("internal object"));
+ strbuf_addstr(sb, _("object of unknown type"));
break;
case OBJ_COMMIT:
- c = lookup_commit_reference(sha1);
- buf = read_sha1_file(sha1, &type, &size);
- if (!c || !buf) {
- die(_("commit object %s could not be read"),
- sha1_to_hex(sha1));
+ if ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {
+ subject_len = find_commit_subject(buf, &subject_start);
+ strbuf_insert(sb, sb->len, subject_start, subject_len);
+ } else {
+ strbuf_addstr(sb, _("commit object"));
}
- subject_len = find_commit_subject(buf, &subject_start);
- strbuf_insert(sb, sb->len, subject_start, subject_len);
- strbuf_addf(sb, ", %s", show_date(c->date, 0,
DATE_MODE(SHORT)));
free(buf);
+
+ if ((c = lookup_commit_reference(sha1)) != NULL)
+ strbuf_addf(sb, ", %s", show_date(c->date, 0,
DATE_MODE(SHORT)));
break;
case OBJ_TREE:
strbuf_addstr(sb, _("tree object"));
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 3c4cb58..894959f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -86,7 +86,7 @@ test_expect_success 'creating a tag with --create-reflog
should create reflog' '
test_when_finished "git tag -d tag_with_reflog" &&
git tag --create-reflog tag_with_reflog &&
git reflog exists refs/tags/tag_with_reflog &&
- sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+ sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog > actual &&
test_cmp expected actual
'
@@ -96,7 +96,7 @@ test_expect_success 'annotated tag with --create-reflog has
correct message' '
test_when_finished "git tag -d tag_with_reflog" &&
git tag -m "annotated tag" --create-reflog tag_with_reflog &&
git reflog exists refs/tags/tag_with_reflog &&
- sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+ sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog > actual &&
test_cmp expected actual
'
--
2.10.2
[PATCH v4] tag: generate useful reflog message
From: Cornelius Weig
When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
6e3a7b3 refs/tags/test@{0}:
Now, a reflog message is generated when creating a tag, following the
pattern "tag: tagging ()". If
GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
()" instead. If the tag references a commit object, the
description is set to the subject line of the commit, followed by its
commit date. For example:
6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)
If the tag points to a tree/blob/tag objects, the following static
strings are taken as description:
- "tree object"
- "blob object"
- "other tag object"
Signed-off-by: Cornelius Weig
Reviewed-by: Junio C Hamano
---
builtin/tag.c | 54 +-
t/t7004-tag.sh | 16 +++-
2 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..bca890f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -302,6 +302,54 @@ static void create_tag(const unsigned char *object, const
char *tag,
}
}
+static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
+{
+ enum object_type type;
+ struct commit *c;
+ char *buf;
+ unsigned long size;
+ int subject_len = 0;
+ const char *subject_start;
+
+ char *rla = getenv("GIT_REFLOG_ACTION");
+ if (rla) {
+ strbuf_addstr(sb, rla);
+ } else {
+ strbuf_addstr(sb, _("tag: tagging "));
+ strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
+ }
+
+ strbuf_addstr(sb, " (");
+ type = sha1_object_info(sha1, NULL);
+ switch (type) {
+ default:
+ strbuf_addstr(sb, _("object of unknown type"));
+ break;
+ case OBJ_COMMIT:
+ if ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {
+ subject_len = find_commit_subject(buf, &subject_start);
+ strbuf_insert(sb, sb->len, subject_start, subject_len);
+ } else {
+ strbuf_addstr(sb, _("commit object"));
+ }
+ free(buf);
+
+ if ((c = lookup_commit_reference(sha1)) != NULL)
+ strbuf_addf(sb, ", %s", show_date(c->date, 0,
DATE_MODE(SHORT)));
+ break;
+ case OBJ_TREE:
+ strbuf_addstr(sb, _("tree object"));
+ break;
+ case OBJ_BLOB:
+ strbuf_addstr(sb, _("blob object"));
+ break;
+ case OBJ_TAG:
+ strbuf_addstr(sb, _("other tag object"));
+ break;
+ }
+ strbuf_addch(sb, ')');
+}
+
struct msg_arg {
int given;
struct strbuf buf;
@@ -335,6 +383,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
struct strbuf ref = STRBUF_INIT;
+ struct strbuf reflog_msg = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
struct create_tag_options opt;
@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
else
die(_("Invalid cleanup mode %s"), cleanup_arg);
+ create_reflog_msg(object, &reflog_msg);
+
if (create_tag_object) {
if (force_sign_annotate && !annotate)
opt.sign = 1;
@@ -504,7 +555,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (!transaction ||
ref_transaction_update(transaction, ref.buf, object, prev,
create_reflog ? REF_FORCE_CREATE_REFLOG : 0,
- NULL, &err) ||
+ reflog_msg.buf, &err) ||
ref_transaction_commit(transaction, &err))
die("%s", err.buf);
ref_transaction_free(transaction);
@@ -514,5 +565,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
strbuf_release(&err);
strbuf_release(&buf);
strbuf_release(&ref);
+ strbuf_release(&reflog_msg);
return 0;
}
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 072e6c6..894959f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD
should succeed' '
test_must_fail git reflog exists refs/tags/mytag
'
+git log -1 > expected \
+ --format="forma
Re: [PATCH] tag: generate useful reflog message
On 02/06/2017 12:25 AM, Junio C Hamano wrote:
> cornelius.w...@tngtech.com writes
> For a tag, I would imagine something like "tag: tagged 4e59582ff7
> ("Seventh batch for 2.12", 2017-01-23)" would be more appropriate.
Yes, I agree that this is much clearer. The revised version v3
implements this behavior.
>> Notes:
>> While playing around with tag reflogs I also found a bug that was present
>> before this patch. It manifests itself when the sha1-ref in the reflog
>> does not
>> point to a commit object but something else.
>
> I think the fix would involve first ripping out the "reflog walking"
> code that was bolted on and stop allowing it to inject the entries
> taken from the reflog into the "walk the commit DAG" machinery.
> Then "reflog walking" code needs to be taught to have its own "now
> we got a single object to show, show it (using the helper functions
> to show a single object that is already used by 'git show')" code,
> instead of piggy-backing on the output codepath used by "log" and
> "rev-list".
I'll start investigating how that could be done. My first glance tells
me that it won't be easy. Especially because I'm not yet familiar with
the git code.
Thanks for your advice!
[PATCH v3] tag: generate useful reflog message
From: Cornelius Weig
When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
6e3a7b3 refs/tags/test@{0}:
Now, a reflog message is generated when creating a tag, following the
pattern "tag: tagging ()". If
GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
()" instead. If the tag references a commit object, the
description is set to the subject line of the commit, followed by its
commit date. For example:
6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)
If the tag points to a tree/blob/tag objects, the following static
strings are taken as description:
- "tree object"
- "blob object"
- "other tag object"
Signed-off-by: Cornelius Weig
---
builtin/tag.c | 54 +-
t/t7004-tag.sh | 16 +++-
2 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..638b68e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -302,6 +302,54 @@ static void create_tag(const unsigned char *object, const
char *tag,
}
}
+static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
+{
+ enum object_type type;
+ struct commit *c;
+ char *buf;
+ unsigned long size;
+ int subject_len = 0;
+ const char *subject_start;
+
+ char *rla = getenv("GIT_REFLOG_ACTION");
+ if (rla) {
+ strbuf_addstr(sb, rla);
+ } else {
+ strbuf_addstr(sb, _("tag: tagging "));
+ strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
+ }
+
+ strbuf_addstr(sb, " (");
+ type = sha1_object_info(sha1, NULL);
+ switch (type) {
+ default:
+ strbuf_addstr(sb, _("internal object"));
+ break;
+ case OBJ_COMMIT:
+ c = lookup_commit_reference(sha1);
+ buf = read_sha1_file(sha1, &type, &size);
+ if (!c || !buf) {
+ die(_("commit object %s could not be read"),
+ sha1_to_hex(sha1));
+ }
+ subject_len = find_commit_subject(buf, &subject_start);
+ strbuf_insert(sb, sb->len, subject_start, subject_len);
+ strbuf_addf(sb, ", %s", show_date(c->date, 0,
DATE_MODE(SHORT)));
+ free(buf);
+ break;
+ case OBJ_TREE:
+ strbuf_addstr(sb, _("tree object"));
+ break;
+ case OBJ_BLOB:
+ strbuf_addstr(sb, _("blob object"));
+ break;
+ case OBJ_TAG:
+ strbuf_addstr(sb, _("other tag object"));
+ break;
+ }
+ strbuf_addch(sb, ')');
+}
+
struct msg_arg {
int given;
struct strbuf buf;
@@ -335,6 +383,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
struct strbuf ref = STRBUF_INIT;
+ struct strbuf reflog_msg = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
struct create_tag_options opt;
@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
else
die(_("Invalid cleanup mode %s"), cleanup_arg);
+ create_reflog_msg(object, &reflog_msg);
+
if (create_tag_object) {
if (force_sign_annotate && !annotate)
opt.sign = 1;
@@ -504,7 +555,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (!transaction ||
ref_transaction_update(transaction, ref.buf, object, prev,
create_reflog ? REF_FORCE_CREATE_REFLOG : 0,
- NULL, &err) ||
+ reflog_msg.buf, &err) ||
ref_transaction_commit(transaction, &err))
die("%s", err.buf);
ref_transaction_free(transaction);
@@ -514,5 +565,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
strbuf_release(&err);
strbuf_release(&buf);
strbuf_release(&ref);
+ strbuf_release(&reflog_msg);
return 0;
}
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 072e6c6..3c4cb58 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD
should succeed' '
test_must_fail git reflog exists refs/tags/mytag
'
+git log -1 > expected \
+ --format="format:tag: tagging %h (%s, %cd
[PATCH v2] tag: generate useful reflog message
From: Cornelius Weig
When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
"6e3a7b3 refs/tags/test@{0}:"
Now, a reflog message is generated when creating a tag, following the
pattern ": ". Here, action is the command line with
all arguments, or the value of GIT_REFLOG_ACTION if it is set. The
description is the commit subject, if the tag points to a commit. For
example:
"6e3a7b3 refs/tags/test@{0}: tag --create-reflog test: Git 2.12-rc0"
If the tag points to a tree/blob/tag object, the following static
strings are taken as description:
- "tree object"
- "blob object"
- "other tag object"
Signed-off-by: Cornelius Weig
---
Notes:
*This patch supersedes the version submitted a few hours earlier.*
Sorry for the messup, but I realized that the pattern for the reflog message
from my first draft did not comply to standard git behavior.
Please also note my remarks from v1 (repeated here):
While playing around with tag reflogs I also found a bug that was present
before this patch. It manifests itself when the sha1-ref in the reflog does
not
point to a commit object but something else.
For example,
- when the referenced sha1 is a tag object:
$ git tag --create-reflog -f -m'annotated tag' tag_with_reflog
- when the referenced sha1 is a blob object:
$ git tag --create-reflog -f tag_with_reflog HEAD:
- when the referenced sha1 is a tree object:
$ git tag --create-reflog -f tag_with_reflog HEAD^{tree}
In each case, a proper reflog entry is generated, but
$ git reflog tag_with_reflog
will sometimes segfault (if it does, it does so consistently), or only show
the
first few entries. The tree/blob cases are IMHO not so important, but the
broken reflog for annotated tags I find quite severe.
I guess it's because the reflog is funneled through the log.c code, where
every
reflog-entry is assumed to be a commit object? If this is the case, a fix
would
probably be quite involved.
builtin/tag.c | 56 ++--
t/t7004-tag.sh | 16 +++-
2 files changed, 69 insertions(+), 3 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..3d9e105 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -30,6 +30,7 @@ static const char * const git_tag_usage[] = {
static unsigned int colopts;
static int force_sign_annotate;
+static struct strbuf default_rla = STRBUF_INIT;
static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
const char *format)
{
@@ -302,6 +303,48 @@ static void create_tag(const unsigned char *object, const
char *tag,
}
}
+static void create_reflog_msg(const unsigned char *object, struct strbuf *sb)
+{
+ enum object_type type;
+ char *buf;
+ unsigned long size;
+ int subject_len = 0;
+ const char *subject_start;
+
+ char *rla = getenv("GIT_REFLOG_ACTION");
+ if (!rla)
+ rla = default_rla.buf;
+
+ strbuf_addf(sb, "%s: ", rla ? rla : default_rla.buf);
+
+ type = sha1_object_info(object, NULL);
+ switch (type) {
+ default:
+ strbuf_addstr(sb, "internal object");
+ break;
+ case OBJ_COMMIT:
+ buf = read_sha1_file(object, &type, &size);
+ if (buf) {
+ subject_len = find_commit_subject(buf, &subject_start);
+ strbuf_insert(sb, sb->len, subject_start, subject_len);
+ free(buf);
+ } else {
+ die("commit object %s could not be read",
+ sha1_to_hex(object));
+ }
+ break;
+ case OBJ_TREE:
+ strbuf_addstr(sb, "tree object");
+ break;
+ case OBJ_BLOB:
+ strbuf_addstr(sb, "blob object");
+ break;
+ case OBJ_TAG:
+ strbuf_addstr(sb, "other tag object");
+ break;
+ }
+}
+
struct msg_arg {
int given;
struct strbuf buf;
@@ -335,6 +378,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
struct strbuf ref = STRBUF_INIT;
+ struct strbuf reflog_msg = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
struct create_tag_options opt;
@@ -349,7 +393,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct ref_filter filter;
static struct
[PATCH] tag: generate useful reflog message
From: Cornelius Weig
When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
"6e3a7b3 refs/tags/tag_with_reflog@{0}:"
Now, a reflog message is generated when creating a tag. The message
follows the pattern "commit: " where the subject is taken from
the commit the tag points to. For example:
"6e3a7b3 refs/tags/tag_with_reflog@{0}: commit: Git 2.12-rc0"
If the tag points to a tree/blob/tag object, the following static
messages are used instead:
- "tree object"
- "blob object"
- "other tag object"
Signed-off-by: Cornelius Weig
---
Notes:
While playing around with tag reflogs I also found a bug that was present
before this patch. It manifests itself when the sha1-ref in the reflog does
not
point to a commit object but something else.
For example,
- when the referenced sha1 is a tag object:
$ git tag --create-reflog -f -m'annotated tag' tag_with_reflog
- when the referenced sha1 is a blob object:
$ git tag --create-reflog -f tag_with_reflog HEAD:
- when the referenced sha1 is a tree object:
$ git tag --create-reflog -f tag_with_reflog HEAD^{tree}
In each case, a proper reflog entry is generated, but
$ git reflog tag_with_reflog
will sometimes segfault (if it does, it does so consistently), or only show
the
first few entries. The tree/blob cases are IMHO not so important, but the
broken reflog for annotated tags I find quite severe.
I guess it's because the reflog is funneled through the log.c code, where
every
reflog-entry is assumed to be a commit object? If this is the case, a fix
would
probably be quite involved.
builtin/tag.c | 43 ++-
t/t7004-tag.sh | 14 +-
2 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..c0d9478 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -302,6 +302,43 @@ static void create_tag(const unsigned char *object, const
char *tag,
}
}
+static void create_reflog_msg(const unsigned char *object, struct strbuf *sb)
+{
+ enum object_type type;
+ char *buf;
+ unsigned long size;
+ int subject_len = 0;
+ const char *subject_start;
+
+ type = sha1_object_info(object, NULL);
+ switch (type) {
+ default:
+ strbuf_addstr(sb, "internal object");
+ break;
+ case OBJ_COMMIT:
+ strbuf_addstr(sb, "commit: ");
+ buf = read_sha1_file(object, &type, &size);
+ if (buf) {
+ subject_len = find_commit_subject(buf, &subject_start);
+ strbuf_insert(sb, 8, subject_start, subject_len);
+ free(buf);
+ } else {
+ die("commit object %s could not be read",
+ sha1_to_hex(repl));
+ }
+ break;
+ case OBJ_TREE:
+ strbuf_addstr(sb, "tree object");
+ break;
+ case OBJ_BLOB:
+ strbuf_addstr(sb, "blob object");
+ break;
+ case OBJ_TAG:
+ strbuf_addstr(sb, "other tag object");
+ break;
+ }
+}
+
struct msg_arg {
int given;
struct strbuf buf;
@@ -335,6 +372,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
struct strbuf ref = STRBUF_INIT;
+ struct strbuf reflog_msg = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
struct create_tag_options opt;
@@ -494,6 +532,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
else
die(_("Invalid cleanup mode %s"), cleanup_arg);
+ create_reflog_msg(object, &reflog_msg);
+
if (create_tag_object) {
if (force_sign_annotate && !annotate)
opt.sign = 1;
@@ -504,7 +544,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (!transaction ||
ref_transaction_update(transaction, ref.buf, object, prev,
create_reflog ? REF_FORCE_CREATE_REFLOG : 0,
- NULL, &err) ||
+ reflog_msg.buf, &err) ||
ref_transaction_commit(transaction, &err))
die("%s", err.buf);
ref_transaction_free(transaction);
@@ -514,5 +554,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
strbuf_releas
Re: [PATCH 1/2] doc: add doc for git-push --recurse-submodules=only
Shouldn't this be part of v2.12-rc0? I just checked but it's not there. Cheers, Cornelius
Re: BUG: "git checkout -q -b foo origin/foo" is not quiet
On 02/03/2017 10:36 PM, Kevin Layer wrote: > Note that git version 1.8.3.1 is quiet and does not print the > "tracking remote" message. So what you are saying is, that git v1.8.3.1 *is* quiet, but v1.7.1 is not? Sounds like a fixed bug to me. I also checked with the latest version and it did not print anything when used with -q. You seem to urgently need quiet branch creation. Have you thought about dumping unwanted output in the shell? E.g. in bash $ git whatever >&/dev/null
[PATCH v2 3/7] completion: improve bash completion for git-add
From: Cornelius Weig Command completion for git-add did not recognize some long-options. This commits adds completion for all long-options that are mentioned in the man-page synopsis. In addition, if the user specified `--update` or `-u`, path completion will only suggest modified tracked files. Signed-off-by: Cornelius Weig Reviewed-by: SZEDER Gábor --- contrib/completion/git-completion.bash | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 8329f09..652c7e2 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -947,13 +947,17 @@ _git_add () --*) __gitcomp " --interactive --refresh --patch --update --dry-run - --ignore-errors --intent-to-add + --ignore-errors --intent-to-add --force --edit --chmod= " return esac - # XXX should we check for --update and --all options ? - __git_complete_index_file "--others --modified --directory --no-empty-directory" + local complete_opt="--others --modified --directory --no-empty-directory" + if test -n "$(__git_find_on_cmdline "-u --update")" + then + complete_opt="--modified" + fi + __git_complete_index_file "$complete_opt" } _git_archive () -- 2.10.2
[PATCH v2 4/7] completion: teach ls-remote to complete options
From: Cornelius Weig
ls-remote needs to complete remote names and its own options. In
addition to the existing remote name completions, do also complete
the options --heads, --tags, --refs, --get-url, and --symref.
Signed-off-by: Cornelius Weig
Reviewed-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 6 ++
1 file changed, 6 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 652c7e2..a355eeb 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,6 +1449,12 @@ _git_ls_files ()
_git_ls_remote ()
{
+ case "$cur" in
+ --*)
+ __gitcomp "--heads --tags --refs --get-url --symref"
+ return
+ ;;
+ esac
__gitcomp_nl "$(__git_remotes)"
}
--
2.10.2
[PATCH v2 7/7] completion: recognize more long-options
From: Cornelius Weig Command completion only recognizes a subset of the available options for the various git commands. The set of recognized options needs to balance between having all useful options and to not clutter the terminal. This commit adds all long-options that are mentioned in the man-page synopsis of the respective git command. Possibly dangerous options are not included in this set, to avoid accidental data loss. The added options are: - apply: --recount --directory= - archive: --output - branch: --column --no-column --sort= --points-at - clone: --no-single-branch --shallow-submodules - commit: --patch --short --date --allow-empty - describe: --first-parent - fetch, pull: --unshallow --update-shallow - fsck: --name-objects - grep: --break --heading --show-function --function-context --untracked --no-index - mergetool: --prompt --no-prompt - reset: --keep - revert: --strategy= --strategy-option= - shortlog: --email - tag: --merged --no-merged --create-reflog Signed-off-by: Cornelius Weig Helped-by: Johannes Sixt Reviewed-by: SZEDER Gábor --- contrib/completion/git-completion.bash | 29 - 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index d8960cf..3545f6a 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -936,6 +936,7 @@ _git_apply () --apply --no-add --exclude= --ignore-whitespace --ignore-space-change --whitespace= --inaccurate-eof --verbose + --recount --directory= " return esac @@ -974,7 +975,7 @@ _git_archive () --*) __gitcomp " --format= --list --verbose - --prefix= --remote= --exec= + --prefix= --remote= --exec= --output " return ;; @@ -1029,6 +1030,7 @@ _git_branch () --track --no-track --contains --merged --no-merged --set-upstream-to= --edit-description --list --unset-upstream --delete --move --remotes + --column --no-column --sort= --points-at " ;; *) @@ -1142,6 +1144,8 @@ _git_clone () --single-branch --branch --recurse-submodules + --no-single-branch + --shallow-submodules " return ;; @@ -1183,6 +1187,7 @@ _git_commit () --reset-author --file= --message= --template= --cleanup= --untracked-files --untracked-files= --verbose --quiet --fixup= --squash= + --patch --short --date --allow-empty " return esac @@ -1201,7 +1206,7 @@ _git_describe () --*) __gitcomp " --all --tags --contains --abbrev= --candidates= - --exact-match --debug --long --match --always + --exact-match --debug --long --match --always --first-parent " return esac @@ -1284,6 +1289,7 @@ __git_fetch_recurse_submodules="yes on-demand no" __git_fetch_options=" --quiet --verbose --append --upload-pack --force --keep --depth= --tags --no-tags --all --prune --dry-run --recurse-submodules= + --unshallow --update-shallow " _git_fetch () @@ -1333,7 +1339,7 @@ _git_fsck () --*) __gitcomp " --tags --root --unreachable --cache --no-reflogs --full - --strict --verbose --lost-found + --strict --verbose --lost-found --name-objects " return ;; @@ -1377,6 +1383,8 @@ _git_grep () --max-depth --count --and --or --not --all-match + --break --heading --show-function --function-context + --untracked --no-index " return ;; @@ -1576,7 +1584,7 @@ _git_mergetool () return ;; --*) - __gitcomp "--tool=" + __gitcomp "--tool= --prompt --no-prompt" return ;; esac @@ -2465,7 +2473,7 @@ _git_reset () case "$cur" in --*) - __gitcomp "--merge --mixed --hard --soft --patch" +
[PATCH v2 0/7] completion bash: add more options and commands
From: Cornelius Weig
This is the re-roll of patch series
<2017015724.19360-1-cornelius.w...@tngtech.com>.
This patch series adds all long-options that are mentioned in the synopsis of
the man-page for the respective git-command. There are only a few exceptions,
as discussed in the above thread. For example, no unsafe options should be
completed.
Furthermore, the patches add subommand option completion for git-submodule and
git-remote.
Changes wrt first submission:
- improve completion for git-remote set-head & set-branches
- remove completion of unsafe options
- improve commit messages
- added sign-off :)
- rebase to current master
Cornelius Weig (7):
completion: teach submodule subcommands to complete options
completion: add subcommand completion for rerere
completion: improve bash completion for git-add
completion: teach ls-remote to complete options
completion: teach replace to complete options
completion: teach remote subcommands to complete options
completion: recognize more long-options
contrib/completion/git-completion.bash | 139 -
1 file changed, 118 insertions(+), 21 deletions(-)
Interdiff since first iteration:
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 87d3d2c..3545f6a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -936,7 +936,7 @@ _git_apply ()
--apply --no-add --exclude=
--ignore-whitespace --ignore-space-change
--whitespace= --inaccurate-eof --verbose
- --recount --directory= --unsafe-paths
+ --recount --directory=
"
return
esac
@@ -1459,7 +1459,7 @@ _git_ls_remote ()
{
case "$cur" in
--*)
- __gitcomp "--heads --tags --refs --get-url"
+ __gitcomp "--heads --tags --refs --get-url --symref"
return
;;
esac
@@ -2415,9 +2415,18 @@ _git_remote ()
;;
add,*)
;;
+ set-head,--*)
+ __gitcomp "--auto --delete"
+ ;;
+ set-branches,--*)
+ __gitcomp "--add"
+ ;;
set-head,*|set-branches,*)
__git_complete_remote_or_refspec
;;
+ update,--*)
+ __gitcomp "--prune"
+ ;;
update,*)
__gitcomp "$(__git_get_config_variables "remotes")"
;;
@@ -2494,7 +2503,7 @@ _git_rm ()
{
case "$cur" in
--*)
- __gitcomp "--cached --dry-run --ignore-unmatch --quiet --force"
+ __gitcomp "--cached --dry-run --ignore-unmatch --quiet"
return
;;
esac
--
2.10.2
[PATCH v2 5/7] completion: teach replace to complete options
From: Cornelius Weig
Git-replace needs to complete references and its own options. In
addition to the existing references completions, do also complete the
options --edit --graft --format= --list --delete.
Signed-off-by: Cornelius Weig
Reviewed-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 6 ++
1 file changed, 6 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index a355eeb..4841036 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2408,6 +2408,12 @@ _git_remote ()
_git_replace ()
{
+ case "$cur" in
+ --*)
+ __gitcomp "--edit --graft --format= --list --delete"
+ return
+ ;;
+ esac
__gitcomp_nl "$(__git_refs)"
}
--
2.10.2
[PATCH v2 6/7] completion: teach remote subcommands to complete options
From: Cornelius Weig
Git-remote needs to complete remote names, its subcommands, and options
thereof. In addition to the existing subcommand and remote name
completion, do also complete the options
- add: --track --master --fetch --tags --no-tags --mirror=
- set-url: --push --add --delete
- get-url: --push --all
- prune: --dry-run
Signed-off-by: Cornelius Weig
Reviewed-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 45 --
1 file changed, 38 insertions(+), 7 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 4841036..d8960cf 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2384,24 +2384,55 @@ _git_config ()
_git_remote ()
{
- local subcommands="add rename remove set-head set-branches set-url show
prune update"
+ local subcommands="
+ add rename remove set-head set-branches
+ get-url set-url show prune update
+ "
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
- __gitcomp "$subcommands"
+ case "$cur" in
+ --*)
+ __gitcomp "--verbose"
+ ;;
+ *)
+ __gitcomp "$subcommands"
+ ;;
+ esac
return
fi
- case "$subcommand" in
- rename|remove|set-url|show|prune)
- __gitcomp_nl "$(__git_remotes)"
+ case "$subcommand,$cur" in
+ add,--*)
+ __gitcomp "--track --master --fetch --tags --no-tags --mirror="
+ ;;
+ add,*)
+ ;;
+ set-head,--*)
+ __gitcomp "--auto --delete"
;;
- set-head|set-branches)
+ set-branches,--*)
+ __gitcomp "--add"
+ ;;
+ set-head,*|set-branches,*)
__git_complete_remote_or_refspec
;;
- update)
+ update,--*)
+ __gitcomp "--prune"
+ ;;
+ update,*)
__gitcomp "$(__git_get_config_variables "remotes")"
;;
+ set-url,--*)
+ __gitcomp "--push --add --delete"
+ ;;
+ get-url,--*)
+ __gitcomp "--push --all"
+ ;;
+ prune,--*)
+ __gitcomp "--dry-run"
+ ;;
*)
+ __gitcomp_nl "$(__git_remotes)"
;;
esac
}
--
2.10.2
[PATCH v2 1/7] completion: teach submodule subcommands to complete options
From: Cornelius Weig Each submodule subcommand has specific long-options. Therefore, teach bash completion to support option completion based on the current subcommand. All long-options that are mentioned in the man-page synopsis are added. Signed-off-by: Cornelius Weig Reviewed-by: SZEDER Gábor --- contrib/completion/git-completion.bash | 32 ++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 6721ff8..c54a557 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2556,10 +2556,11 @@ _git_submodule () __git_has_doubledash && return local subcommands="add status init deinit update summary foreach sync" - if [ -z "$(__git_find_on_cmdline "$subcommands")" ]; then + local subcommand="$(__git_find_on_cmdline "$subcommands")" + if [ -z "$subcommand" ]; then case "$cur" in --*) - __gitcomp "--quiet --cached" + __gitcomp "--quiet" ;; *) __gitcomp "$subcommands" @@ -2567,6 +2568,33 @@ _git_submodule () esac return fi + + case "$subcommand,$cur" in + add,--*) + __gitcomp "--branch --force --name --reference --depth" + ;; + status,--*) + __gitcomp "--cached --recursive" + ;; + deinit,--*) + __gitcomp "--force --all" + ;; + update,--*) + __gitcomp " + --init --remote --no-fetch + --recommend-shallow --no-recommend-shallow + --force --rebase --merge --reference --depth --recursive --jobs + " + ;; + summary,--*) + __gitcomp "--cached --files --summary-limit" + ;; + foreach,--*|sync,--*) + __gitcomp "--recursive" + ;; + *) + ;; + esac } _git_svn () -- 2.10.2
[PATCH v2 2/7] completion: add subcommand completion for rerere
From: Cornelius Weig
Managing recorded resolutions requires command-line usage of git-rerere.
Added subcommand completion for rerere and path completion for its
subcommand forget.
Signed-off-by: Cornelius Weig
Reviewed-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 11 +++
1 file changed, 11 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index c54a557..8329f09 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2401,6 +2401,17 @@ _git_replace ()
__gitcomp_nl "$(__git_refs)"
}
+_git_rerere ()
+{
+ local subcommands="clear forget diff remaining status gc"
+ local subcommand="$(__git_find_on_cmdline "$subcommands")"
+ if test -z "$subcommand"
+ then
+ __gitcomp "$subcommands"
+ return
+ fi
+}
+
_git_reset ()
{
__git_has_doubledash && return
--
2.10.2
Re: [PATCH v2 7/7] completion: recognize more long-options
On 02/02/2017 03:00 AM, SZEDER Gábor wrote: >> Personally, I agree with you that >>> Adding more long options that git commands learn along the way is >>> always an improvement. >> However, people may start complaining that their terminal becomes too >> cluttered when doing a double-Tab. In my cover letter, I go to length >> about this. My assumption was that all options that are mentioned in the >> introduction of the command man-page should be important enough to have >> them in the completion list. > > But that doesn't mean that the ones not mentioned in the synopsis > section are not worth completing. Absolutely. What I meant is that at least the options from the synopsis should be contained in the set of completable options. >> Btw, I haven't found that non-destructive options should not be eligible >> for completion. To avoid confusion about this in the future, I suggest >> to also change the documentation: >> >> index 933bb6e..96f1c7f 100644 >> --- a/contrib/completion/git-completion.bash >> +++ b/contrib/completion/git-completion.bash >> @@ -13,7 +13,7 @@ >> #*) git email aliases for git-send-email >> #*) tree paths within 'ref:path/to/file' expressions >> #*) file paths within current working directory and index >> -#*) common --long-options >> +#*) common non-destructive --long-options > > I don't mind such a change, but I don't think that list was ever meant > to be comprehensive or decisive. It is definitely not the former, as > it's missing several things that the completion script does support. > OTOH, it talks about .git/remotes, which has been considered legacy > for quite some years (though it's right, because the completion script > still supports it). Then let's not do that change, because for some commands destructive long-options have been in the list of completed options for quite a while. Given that, the above change of the documentation, might stir up more confusion than it settles.
Re: [PATCH 4/7] completion: teach ls-remote to complete options
On 02/02/2017 02:40 AM, SZEDER Gábor wrote: > >> ls-remote needs to complete remote names and its own options. > > And refnames, too. Yes, right. However, do you think it is reasonable to complete remote refnames? I don't think so, because it would mean we would have to run ls-remote during completion -- and waiting for ls-remote could be quite lengthy. >> In >> addition to the existing remote name completions, do also complete >> the options --heads, --tags, --refs, and --get-url. > > Why only these four options and not the other four? > > There are eight options in total here, so there is really no chance > for cluttered terminals, and all eight are mentioned in the synopsis. My line of thought is the following: --quiet: does not print anything and is therefore only useful for scripting. Thus, there is no need to have it on the command line completion. --exit-code: has no visible effect and is only useful for scripting. --upload-pack: is really exotic. Nobody will ever use it without digging deep in the manuals. Therefore, I think it's unnecessary to have the option completable. --symref: Should probably be added, thanks. However, if you don't find my reasoning for omitting the three options above conclusive, I have no problem including them.
Re: [PATCH 2/7] completion: add subcommand completion for rerere
On 02/02/2017 01:57 AM, SZEDER Gábor wrote: > You didn't add 'rerere' to the list of porcelain commands, i.e. it > won't be listed after 'git '. I'm not saying it should be > listed there, because I can't decide whether 'rerere' is considered > porcelain or plumbing... Just wanted to make sure that this omission > was intentional. Yes this is intentional. There is a number of plumbing commands that have command completion, but are not listed in 'git ' (e.g. ls-tree, ls-files, ls-remote, ...). Given that rerere will not be frequently invoked, I would not add it to the porcelain commands.
Re: [PATCH] doc: add note about ignoring --no-create-reflog
> The negated form `--no-create-reflog` only overrides an earlier > `--create-reflog`, but currently does not negate the setting of > `core.logallrefupdates`. Even better than Junio's version. I especially like that it mentions where the default setting comes from.
Re: [PATCH] doc: add note about ignoring --no-create-reflog
On 02/02/2017 12:11 AM, Junio C Hamano wrote:
> Jeff King writes:
>
>> This might be nitpicking, but it's _not_ ignored. It still negates an
>> earlier "--create-reflog". It is only that it does not override the
>> decision to create a reflog caused by the setting of
>> core.logallrefupdates.
This corner case is quite important. Glad you thought about it!
> -- >8 --
> From: Cornelius Weig
> Date: Wed, 1 Feb 2017 23:07:27 +0100
> Subject: [PATCH] doc: add note about ignoring '--no-create-reflog'
>
> The commands git-branch and git-tag accept the '--create-reflog'
> option, and create reflog even when core.logallrefupdates
> configuration is explicitly set not to.
>
> On the other hand, the negated form '--no-create-reflog' is accepted
> as a valid option but has no effect (other than overriding an
> earlier '--create-reflog' on the command line). This silent noop may
> puzzle users. To communicate that this is a known limitation, add a
> short note in the manuals for git-branch and git-tag.
>
> Signed-off-by: Cornelius Weig
> Signed-off-by: Junio C Hamano
> ---
> Documentation/git-branch.txt | 3 +++
> Documentation/git-tag.txt| 3 +++
> 2 files changed, 6 insertions(+)
>
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 5516a47b54..102e426fd8 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -91,6 +91,9 @@ OPTIONS
> based sha1 expressions such as "@\{yesterday}".
> Note that in non-bare repositories, reflogs are usually
> enabled by default by the `core.logallrefupdates` config option.
> + The negated form `--no-create-reflog` does not override the
> + default, even though it overrides `--create-reflog` that appears
> + earlier on the command line.
>
> -f::
> --force::
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 2ac25a9bb3..fd7eeae075 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -152,6 +152,9 @@ This option is only applicable when listing tags without
> annotation lines.
> --create-reflog::
> Create a reflog for the tag. To globally enable reflogs for tags, see
> `core.logAllRefUpdates` in linkgit:git-config[1].
> + The negated form `--no-create-reflog` does not override the
> + default, even though it overrides `--create-reflog` that appears
> + earlier on the command line.
>
> ::
> The name of the tag to create, delete, or describe.
>
Your amended version is quite concise and says everything there is to
say. Thanks
[PATCH 1/2] doc: add doc for git-push --recurse-submodules=only
From: Cornelius Weig Add documentation for the `--recurse-submodules=only` option of git-push. The feature was added in commit 225e8bf (add option to push only submodules). Signed-off-by: Cornelius Weig --- Notes: This feature is already in 'next' but was undocumented. Unless somebody reads the release notes, there is no way of knowing about it. Documentation/git-push.txt | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index 8eefabd..1624a35 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -272,7 +272,7 @@ origin +master` to force a push to the `master` branch). See the standard error stream is not directed to a terminal. --no-recurse-submodules:: ---recurse-submodules=check|on-demand|no:: +--recurse-submodules=check|on-demand|only|no:: May be used to make sure all submodule commits used by the revisions to be pushed are available on a remote-tracking branch. If 'check' is used Git will verify that all submodule commits that @@ -280,11 +280,12 @@ origin +master` to force a push to the `master` branch). See the remote of the submodule. If any commits are missing the push will be aborted and exit with non-zero status. If 'on-demand' is used all submodules that changed in the revisions to be pushed will be - pushed. If on-demand was not able to push all necessary revisions - it will also be aborted and exit with non-zero status. A value of - 'no' or using `--no-recurse-submodules` can be used to override the - push.recurseSubmodules configuration variable when no submodule - recursion is required. + pushed. If on-demand was not able to push all necessary revisions it will + also be aborted and exit with non-zero status. If 'only' is used all + submodules will be recursively pushed while the superproject is left + unpushed. A value of 'no' or using `--no-recurse-submodules` can be used + to override the push.recurseSubmodules configuration variable when no + submodule recursion is required. --[no-]verify:: Toggle the pre-push hook (see linkgit:githooks[5]). The -- 2.10.2
[PATCH 2/2] completion: add completion for --recurse-submodules=only
From: Cornelius Weig
Command completion for 'git-push --recurse-submodules' already knows to
complete some modes. However, the recently added mode 'only' is missing.
Adding 'only' to the recognized modes completes the list of non-trivial
modes.
Signed-off-by: Cornelius Weig
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index ff7072a..fe3b0f8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1675,7 +1675,7 @@ _git_pull ()
__git_complete_remote_or_refspec
}
-__git_push_recurse_submodules="check on-demand"
+__git_push_recurse_submodules="check on-demand only"
__git_complete_force_with_lease ()
{
--
2.10.2
[PATCH] doc: add note about ignoring --no-create-reflog
From: Cornelius Weig
The commands git-branch and git-tag accept a `--create-reflog` argument.
On the other hand, the negated form `--no-create-reflog` is accepted as
a valid option but has no effect. This silent noop may puzzle users.
To communicate that this behavior is intentional, add a short note in
the manuals for git-branch and git-tag.
Signed-off-by: Cornelius Weig
---
Notes:
In a previous discussion () it
was found that git-branch and git-tag accept a "--no-create-reflog"
argument,
but it has no effect, does not produce a warning, and is undocumented.
Documentation/git-branch.txt | 1 +
Documentation/git-tag.txt| 1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 1fae4ee..fca3754 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -91,6 +91,7 @@ OPTIONS
based sha1 expressions such as "@\{yesterday}".
Note that in non-bare repositories, reflogs are usually
enabled by default by the `core.logallrefupdates` config option.
+ The negated form `--no-create-reflog` is silently ignored.
-f::
--force::
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 5b2288c..b0b933e 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -152,6 +152,7 @@ This option is only applicable when listing tags without
annotation lines.
--create-reflog::
Create a reflog for the tag. To globally enable reflogs for tags, see
`core.logAllRefUpdates` in linkgit:git-config[1].
+ The negated form `--no-create-reflog` is silently ignored.
::
The name of the tag to create, delete, or describe.
--
2.10.2
Re: [PATCH v2 7/7] completion: recognize more long-options
Hi Gabor, thanks for taking a look at these commits. On 01/31/2017 11:17 PM, SZEDER Gábor wrote: > On Fri, Jan 27, 2017 at 10:17 PM, wrote: >> From: Cornelius Weig >> >> Recognize several new long-options for bash completion in the following >> commands: > > Adding more long options that git commands learn along the way is > always an improvement. However, seeing "_several_ new long options" > (or "some long options" in one of the other patches in the series) > makes the reader wonder: are these the only new long options missing > or are there more? If there are more, why only these are added? If > there aren't any more missing long options left, then please say so, > e.g. "Add all missing long options, except the potentially > desctructive ones, for the following commands: " Personally, I agree with you that > Adding more long options that git commands learn along the way is > always an improvement. However, people may start complaining that their terminal becomes too cluttered when doing a double-Tab. In my cover letter, I go to length about this. My assumption was that all options that are mentioned in the introduction of the command man-page should be important enough to have them in the completion list. I'll change my commit message accordingly. >> - rm: --force > > '--force' is a potentially destructive option, too. Thanks for spotting this. Btw, I haven't found that non-destructive options should not be eligible for completion. To avoid confusion about this in the future, I suggest to also change the documentation: index 933bb6e..96f1c7f 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -13,7 +13,7 @@ #*) git email aliases for git-send-email #*) tree paths within 'ref:path/to/file' expressions #*) file paths within current working directory and index -#*) common --long-options +#*) common non-destructive --long-options # # To use these routines: # I take it you have also looked at the code itself? Then I would gladly mention you as reviewer in my sign-off.
Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always
On 01/31/2017 06:08 PM, Junio C Hamano wrote: > I think it is probably a good idea to document the behaviour > (i.e. "--no-create" single-shot from the command line is ignored). > I am not sure we should error out, though, in order to "disallow" > it---a documented silent no-op may be sufficient. Yes, maybe abort on seeing "--no-create-reflog" is a too drastic measure. I presume that the best place to have the documentation would be to print a warning when seeing the ignored argument? Or did you just have man pages and code comment in mind? Cheers, Cornelius
Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always
On 01/31/2017 12:37 AM, Jeff King wrote: > On Mon, Jan 30, 2017 at 01:58:10PM -0800, Junio C Hamano wrote: > >>> When writing the test for git-tag, I realized that the option >>> --no-create-reflog to git-tag does not take precedence over >>> logAllRefUpdate=always. IOW the setting cannot be overridden on the >>> command >>> line. Do you think this is a defect or would it not be desirable to >>> have this >>> feature anyway? >> >> "--no-create-reflog" should override the configuration set to "true" >> or "always". Also "--create-reflog" should override the >> configuration set to "false". >> >> If the problem was inherited from the original code before your >> change (e.g. you set logAllRefUpdates to true and then did >> "update-ref --no-create-reflog refs/heads/foo". I was actually not referring to update-ref, for which the --no-create-reflog option works fine. I was referring to git-tag which also has the --create-reflog option. For git-tag, the current code does not allow to override logAllRefUpdates=always with --no-create-reflog. On the other hand logAllRefUpdates=false is overridden by "git tag --create-reflog". The reason is that the file-backend does allow to force reflog creation, but it does not allow to force reflog non-creation. I have a patch that amends this, but it's not pretty and I don't think it will be useful (see last paragraph). > I hadn't thought about that. I think "git branch --no-create-reflog" has > the same problem in the existing code. You are right, git-branch also ignores --no-create-reflog. > I suspect nobody cares much in practice. Even if you say "don't create a > reflog now", if you have core.logAllRefUpdates turned on, then it's > likely that some _other_ operation will create the reflog later > accidentally (e.g., as soon as you "git checkout foo && git commit", > you'll get a reflog). I think you're fighting an uphill battle to turn > logAllRefUpdates on and then try to disable some reflogs selectively. > > So I agree the current behavior is quietly broken, which is not good. > But I wonder if "--no-create-reflog" is really sane in the first place, > and whether we might be better off to simply disallow it. Concerning branches, I fully agree. For git-branch, the "--no-create-reflog" option does not make sense at all and should produce an error. On the other hand, for tags it may make sense to override logAllRefUpdates=always. As tag updates come exclusively from force-creating the same tag on another revision, a reflog will actually not be created by accident. Nevertheless, I don't think it is very useful to have the "--no-create-reflog" argument to any of git-branch or git-tag. It only takes effect if a user has configured logAllRefUpdates=always, and he probably has done that for a reason. Given that the overhead from a reflog is minuscule, IMHO no-one will ever bother about "--no-create-reflog".
Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always
Hi,
> The extra free(refname) is to plug the leak I pointed out, and the
> type of refname is no longer const, because "const char *" cannot be
> free()d without casting, and in this codepath I do not see a reason
> to mark it as const.
Ooops.. thanks for not yelling at me for that :-/
> When queued on top of 4e59582ff7 ("Seventh batch for 2.12",
> 2017-01-23), however, this fails t2017#9 (orphan with -l makes
> reflog when core.logAllRefUpdates = false).
And again, thanks for not yelling. I overlooked that the
"should_autocreate_reflog" return value should have been negated as
shown below. Should I resend this patch, or is it easier for you
to do the change yourself?
Interdiff v2..v3:
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 81ea2ed..1e8631a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -612,8 +612,10 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
const char *old_desc, *reflog_msg;
if (opts->new_branch) {
if (opts->new_orphan_branch) {
- const char *refname = mkpathdup("refs/heads/%s",
opts->new_orphan_branch);
- if (opts->new_branch_log &&
should_autocreate_reflog(refname)) {
+ char *refname;
+
+ refname = mkpathdup("refs/heads/%s",
opts->new_orphan_branch);
+ if (opts->new_branch_log &&
!should_autocreate_reflog(refname)) {
int ret;
struct strbuf err = STRBUF_INIT;
@@ -622,6 +624,7 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
fprintf(stderr, _("Can not do reflog
for '%s': %s\n"),
opts->new_orphan_branch,
err.buf);
strbuf_release(&err);
+ free(refname);
return;
}
strbuf_release(&err);
[PATCH v2 0/7] completion: recognize more long-options
From: Cornelius Weig This revision addresses Johannes' concerns. Changes wrt v1: - fixed the commit message: two of the "dangerous" options erroneously ended up in the commit message. These options were already in the list of auto-completable options. - removed the possibly dangerous option '--unsafe-paths' from git-apply. - added my sign-off Patches 1-6 are not resent, because they have not changed (other than my added sign-off). Also, I added further people to CC, because nobody actually has looked at the code yet. Cornelius Weig (7): completion: recognize more long-options contrib/completion/git-completion.bash | 132 +++-- 1 file changed, 110 insertions(+), 22 deletions(-) -- 2.10.2
[PATCH v2 7/7] completion: recognize more long-options
From: Cornelius Weig Recognize several new long-options for bash completion in the following commands: - apply: --recount --directory= - archive: --output - branch: --column --no-column --sort= --points-at - clone: --no-single-branch --shallow-submodules - commit: --patch --short --date --allow-empty - describe: --first-parent - fetch, pull: --unshallow --update-shallow - fsck: --name-objects - grep: --break --heading --show-function --function-context --untracked --no-index - mergetool: --prompt --no-prompt - reset: --keep - revert: --strategy= --strategy-option= - rm: --force - shortlog: --email - tag: --merged --no-merged --create-reflog Signed-off-by: Cornelius Weig Helped-by: Johannes Sixt --- contrib/completion/git-completion.bash | 31 +-- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 0e09519..933bb6e 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -936,6 +936,7 @@ _git_apply () --apply --no-add --exclude= --ignore-whitespace --ignore-space-change --whitespace= --inaccurate-eof --verbose + --recount --directory= " return esac @@ -974,7 +975,7 @@ _git_archive () --*) __gitcomp " --format= --list --verbose - --prefix= --remote= --exec= + --prefix= --remote= --exec= --output " return ;; @@ -1029,6 +1030,7 @@ _git_branch () --track --no-track --contains --merged --no-merged --set-upstream-to= --edit-description --list --unset-upstream --delete --move --remotes + --column --no-column --sort= --points-at " ;; *) @@ -1142,6 +1144,8 @@ _git_clone () --single-branch --branch --recurse-submodules + --no-single-branch + --shallow-submodules " return ;; @@ -1183,6 +1187,7 @@ _git_commit () --reset-author --file= --message= --template= --cleanup= --untracked-files --untracked-files= --verbose --quiet --fixup= --squash= + --patch --short --date --allow-empty " return esac @@ -1201,7 +1206,7 @@ _git_describe () --*) __gitcomp " --all --tags --contains --abbrev= --candidates= - --exact-match --debug --long --match --always + --exact-match --debug --long --match --always --first-parent " return esac @@ -1284,6 +1289,7 @@ __git_fetch_recurse_submodules="yes on-demand no" __git_fetch_options=" --quiet --verbose --append --upload-pack --force --keep --depth= --tags --no-tags --all --prune --dry-run --recurse-submodules= + --unshallow --update-shallow " _git_fetch () @@ -1333,7 +1339,7 @@ _git_fsck () --*) __gitcomp " --tags --root --unreachable --cache --no-reflogs --full - --strict --verbose --lost-found + --strict --verbose --lost-found --name-objects " return ;; @@ -1377,6 +1383,8 @@ _git_grep () --max-depth --count --and --or --not --all-match + --break --heading --show-function --function-context + --untracked --no-index " return ;; @@ -1576,7 +1584,7 @@ _git_mergetool () return ;; --*) - __gitcomp "--tool=" + __gitcomp "--tool= --prompt --no-prompt" return ;; esac @@ -2456,7 +2464,7 @@ _git_reset () case "$cur" in --*) - __gitcomp "--merge --mixed --hard --soft --patch" + __gitcomp "--merge --mixed --hard --soft --patch --keep" return ;; esac @@ -2472,7 +2480,10 @@ _git_revert () fi case "$cur" in --*) - __gitcomp "--edit --mainline --no-edit --no-commit --signoff" +
Re: [PATCH] doc: clarify distinction between sign-off and pgp-signing
> > So maybe s/signed-off-by/helped-by/? > This is getting real complex :-/ As I said in the notes for the patch: >> As I don't know what is appropriate, I took the liberty to add >> everybody's >> sign-off who was involved in the discussion in alphabetic order. With your explanation, I guess the most accurate sign-off chain would be: Signed-off-by: Stefan Beller (as you sent a patch) Helped-by: Philip Oakley (no patch, but helpful) Signed-off-by: Cornelius Weig (this patch) Signed-off-by: Junio C Hamano (once he decides it's good)
[PATCH] doc: clarify distinction between sign-off and pgp-signing
From: Cornelius Weig The documentation for submission discourages pgp-signing, but demands a proper sign-off by contributors. However, when skimming the headings, the wording of the section for sign-off could mistakenly be understood as concerning pgp-signing. Thus, new contributors could oversee the necessary sign-off. This commit improves the wording such that the section about sign-off cannot be misunderstood as pgp-signing. In addition, the paragraph about pgp-signing is changed such that it avoids the impression that pgp-signing could be relevant at later stages of the submission. Signed-off-by: Cornelius Weig Signed-off-by: Junio C Hamano Signed-off-by: Philip Oakley Signed-off-by: Stefan Beller --- Notes: This patch summarizes the suggested changes. As I don't know what is appropriate, I took the liberty to add everybody's sign-off who was involved in the discussion in alphabetic order. Documentation/SubmittingPatches | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 08352de..3faf7eb 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -216,12 +216,11 @@ that it will be postponed. Exception: If your mailer is mangling patches then someone may ask you to re-send them using MIME, that is OK. -Do not PGP sign your patch, at least for now. Most likely, your -maintainer or other people on the list would not have your PGP -key and would not bother obtaining it anyway. Your patch is not -judged by who you are; a good patch from an unknown origin has a -far better chance of being accepted than a patch from a known, -respected origin that is done poorly or does incorrect things. +Do not PGP sign your patch. Most likely, your maintainer or other people on the +list would not have your PGP key and would not bother obtaining it anyway. +Your patch is not judged by who you are; a good patch from an unknown origin +has a far better chance of being accepted than a patch from a known, respected +origin that is done poorly or does incorrect things. If you really really really really want to do a PGP signed patch, format it as "multipart/signed", not a text/plain message @@ -246,7 +245,7 @@ patch. *2* The mailing list: git@vger.kernel.org -(5) Sign your work +(5) Certify your work by adding your "Signed-off-by: " line To improve tracking of who did what, we've borrowed the "sign-off" procedure from the Linux kernel project on patches -- 2.10.2
Re: [PATCH] doc: clarify distinction between sign-off and pgp-signing
Sorry, I forgot to mark this patch as follow-up to message
Re: SubmittingPatches: drop temporal reference for PGP signing
On 01/26/2017 09:58 PM, Philip Oakley wrote: > From: "Junio C Hamano" >> Cornelius Weig writes: >> >>> How about something along these lines? Does the forward reference >>> break the main line of thought too severly? >> >> I find it a bit distracting for those who know PGP signing has >> nothing to do with signing off your patch, but I think that is OK >> because they are not the primary target audience of this part of the >> document. > > Agreed. I this case the target audience was those weren't aware of that. Yes, maybe the forward reference does give a wrong hint about a connection between sign-off and pgp-signing. However, I would still vote for the following change suggested by sbel...@google.com: -Do not PGP sign your patch, -at least for now-. Most likely, your (...) +Do not PGP sign your patch. Most likely, your maintainer or other (...) > > Maybe even s/by signing off/by adding your Signed-off-by:/ to be sure > that the reader knows that it is _their certification_ that is being > sought. Even if it does double up on the 'your'. > I don't think doubling on the 'your' will make the heading clearer. The main intention of this change is to avoid mixups with pgp-signing and that would IMHO not improve by that. Besides, I find the colon in the heading a bit awkward. Is the following version as expressive as with the colon? -(5) Sign your work +(5) Certify your work by adding Signed-off-by
[PATCH v3 1/3] config: add markup to core.logAllRefUpdates doc
From: Cornelius Weig Signed-off-by: Cornelius Weig --- Notes: Changes wrt v2: Remove duplicated line. Documentation/config.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index af2ae4c..c7d8a01 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -517,10 +517,10 @@ core.logAllRefUpdates:: "`$GIT_DIR/logs/`", by appending the new and old SHA-1, the date/time and the reason of the update, but only when the file exists. If this configuration - variable is set to true, missing "`$GIT_DIR/logs/`" + variable is set to `true`, missing "`$GIT_DIR/logs/`" file is automatically created for branch heads (i.e. under - refs/heads/), remote refs (i.e. under refs/remotes/), - note refs (i.e. under refs/notes/), and the symbolic ref HEAD. + `refs/heads/`), remote refs (i.e. under `refs/remotes/`), + note refs (i.e. under `refs/notes/`), and the symbolic ref `HEAD`. + This information can be used to determine what commit was the tip of a branch "2 days ago". -- 2.10.2
[PATCH v3 2/3] refs: add option core.logAllRefUpdates = always
From: Cornelius Weig
When core.logallrefupdates is true, we only create a new reflog for refs
that are under certain well-known hierarchies. The reason is that we
know that some hierarchies (like refs/tags) are not meant to change, and
that unknown hierarchies might not want reflogs at all (e.g., a
hypothetical refs/foo might be meant to change often and drop old
history immediately).
However, sometimes it is useful to override this decision and simply log
for all refs, because the safety and audit trail is more important than
the performance implications of keeping the log around.
This patch introduces a new "always" mode for the core.logallrefupdates
option which will log updates to everything under refs/, regardless
where in the hierarchy it is (we still will not log things like
ORIG_HEAD and FETCH_HEAD, which are known to be transient).
Based-on-patch-by: Jeff King
Signed-off-by: Cornelius Weig
Reviewed-by: Jeff King
---
Notes:
Changes wrt v2:
- change wording in commit message s/do not typically/are not meant to/;
- in update_refs_for_switch move refname to the enclosing block, so that
should_autocreate_reflog has access. Thanks Junio for spotting this
potential bug early :)
- add test that asserts reflogs are created for tags if
logAllRefUpdates=always. The case with logAllRefUpdates=true is IMHO
already
covered by the default case. To make that clearer, I explicitly added
logAllRefUpdates=true.
When writing the test for git-tag, I realized that the option
--no-create-reflog to git-tag does not take precedence over
logAllRefUpdate=always. IOW the setting cannot be overridden on the command
line. Do you think this is a defect or would it not be desirable to have
this
feature anyway?
Documentation/config.txt | 2 ++
Documentation/git-tag.txt | 3 ++-
branch.c | 2 +-
builtin/checkout.c| 7 +++
builtin/init-db.c | 2 +-
cache.h | 9 -
config.c | 7 ++-
environment.c | 2 +-
refs.c| 15 ++-
refs.h| 2 ++
refs/files-backend.c | 6 +++---
refs/refs-internal.h | 2 --
t/t1400-update-ref.sh | 37 +
t/t7004-tag.sh| 8
14 files changed, 84 insertions(+), 20 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index c7d8a01..d1fab67 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -521,6 +521,8 @@ core.logAllRefUpdates::
file is automatically created for branch heads (i.e. under
`refs/heads/`), remote refs (i.e. under `refs/remotes/`),
note refs (i.e. under `refs/notes/`), and the symbolic ref `HEAD`.
+ If it is set to `always`, then a missing reflog is automatically
+ created for any ref under `refs/`.
+
This information can be used to determine what commit
was the tip of a branch "2 days ago".
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 5055a96..2ac25a9 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -150,7 +150,8 @@ This option is only applicable when listing tags without
annotation lines.
'strip' removes both whitespace and commentary.
--create-reflog::
- Create a reflog for the tag.
+ Create a reflog for the tag. To globally enable reflogs for tags, see
+ `core.logAllRefUpdates` in linkgit:git-config[1].
::
The name of the tag to create, delete, or describe.
diff --git a/branch.c b/branch.c
index c431cbf..b955d4f 100644
--- a/branch.c
+++ b/branch.c
@@ -298,7 +298,7 @@ void create_branch(const char *name, const char *start_name,
start_name);
if (reflog)
- log_all_ref_updates = 1;
+ log_all_ref_updates = LOG_REFS_NORMAL;
if (!dont_change_ref) {
struct ref_transaction *transaction;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bfe685c..81ea2ed 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -612,14 +612,12 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
const char *old_desc, *reflog_msg;
if (opts->new_branch) {
if (opts->new_orphan_branch) {
- if (opts->new_branch_log && !log_all_ref_updates) {
+ const char *refname = mkpathdup("refs/heads/%s",
opts->new_orphan_branch);
+ if (opts->new_branch_log &&
should_autocreate_reflog(refname)) {
int ret;
- char *refname;
struct strbuf err = STRBUF_INIT;
- refname = mkpathdup("refs/heads/%s",
opts->new_orpha
[PATCH v3 3/3] update-ref: add test cases for bare repository
From: Cornelius Weig
The default behavior of update-ref to create reflogs differs in
repositories with worktree and bare ones. The existing tests cover only
the behavior of repositories with worktree.
This commit adds tests that assert the correct behavior in bare
repositories for update-ref. Two cases are covered:
- If core.logAllRefUpdates is not set, no reflogs should be created
- If core.logAllRefUpdates is true, reflogs should be created
Signed-off-by: Cornelius Weig
---
Notes:
Changes wrt v2:
Remove bashism 'local' from test function
t/t1400-update-ref.sh | 43 ---
1 file changed, 36 insertions(+), 7 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index b9084ca..b0ffc0b 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -8,23 +8,33 @@ test_description='Test git update-ref and basic ref logging'
Z=$_z40
-test_expect_success setup '
+m=refs/heads/master
+n_dir=refs/heads/gu
+n=$n_dir/fixes
+outside=refs/foo
+bare=bare-repo
+create_test_commits ()
+{
+ prfx="$1"
for name in A B C D E F
do
test_tick &&
T=$(git write-tree) &&
sha1=$(echo $name | git commit-tree $T) &&
- eval $name=$sha1
+ eval $prfx$name=$sha1
done
+}
+test_expect_success setup '
+ create_test_commits "" &&
+ mkdir $bare &&
+ cd $bare &&
+ git init --bare &&
+ create_test_commits "bare" &&
+ cd -
'
-m=refs/heads/master
-n_dir=refs/heads/gu
-n=$n_dir/fixes
-outside=refs/foo
-
test_expect_success \
"create $m" \
"git update-ref $m $A &&
@@ -93,6 +103,25 @@ test_expect_success 'update-ref creates reflogs with
--create-reflog' '
git reflog exists $outside
'
+test_expect_success 'creates no reflog in bare repository' '
+ git -C $bare update-ref $m $bareA &&
+ git -C $bare rev-parse $bareA >expect &&
+ git -C $bare rev-parse $m >actual &&
+ test_cmp expect actual &&
+ test_must_fail git -C $bare reflog exists $m
+'
+
+test_expect_success 'core.logAllRefUpdates=true creates reflog in bare
repository' '
+ test_when_finished "git -C $bare config --unset core.logAllRefUpdates
&& \
+ rm $bare/logs/$m" &&
+ git -C $bare config core.logAllRefUpdates true &&
+ git -C $bare update-ref $m $bareB &&
+ git -C $bare rev-parse $bareB >expect &&
+ git -C $bare rev-parse $m >actual &&
+ test_cmp expect actual &&
+ git -C $bare reflog exists $m
+'
+
test_expect_success 'core.logAllRefUpdates=true does not create reflog by
default' '
test_config core.logAllRefUpdates true &&
test_when_finished "git update-ref -d $outside" &&
--
2.10.2
[PATCH v2 2/3] refs: add option core.logAllRefUpdates = always
From: Cornelius Weig
When core.logallrefupdates is true, we only create a new reflog for refs
that are under certain well-known hierarchies. The reason is that we
know that some hierarchies (like refs/tags) do not typically change, and
that unknown hierarchies might not want reflogs at all (e.g., a
hypothetical refs/foo might be meant to change often and drop old
history immediately).
However, sometimes it is useful to override this decision and simply log
for all refs, because the safety and audit trail is more important than
the performance implications of keeping the log around.
This patch introduces a new "always" mode for the core.logallrefupdates
option which will log updates to everything under refs/, regardless
where in the hierarchy it is (we still will not log things like
ORIG_HEAD and FETCH_HEAD, which are known to be transient).
Based-on-patch-by: Jeff King
Signed-off-by: Cornelius Weig
Reviewed-by: Jeff King
---
Notes:
Changes with respect to the previous version:
- add test that checks that no reflog is created for ORIG_HEAD if
core.logAllRefUpdates=always
- remove redundant tests that check reflog creation when update-ref is
called
with --stdin
- make test description shorter
- make the function should_autocreate_reflog() public and use it in
update_refs_for_switch().
The last item addresses Peff's concern that the previous version only works
by
accident and may break in the future (see
20170126033547.7bszipvkpi2jb...@sigill.intra.peff.net). In particular, this
concerns the following change:
- if (opts->new_branch_log && !log_all_ref_updates) {
+ if (opts->new_branch_log && should_autocreate_reflog("refs/heads/")) {
The function call to `should_autocreate_reflog()` answers exactly the
question
that the original test `!log_all_ref_updates` tried to resolve in the
original
version.
Documentation/config.txt | 2 ++
Documentation/git-tag.txt | 3 ++-
branch.c | 2 +-
builtin/checkout.c| 2 +-
builtin/init-db.c | 2 +-
cache.h | 9 -
config.c | 7 ++-
environment.c | 2 +-
refs.c| 15 ++-
refs.h| 2 ++
refs/files-backend.c | 6 +++---
refs/refs-internal.h | 2 --
t/t1400-update-ref.sh | 37 +
13 files changed, 74 insertions(+), 17 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3cd8030..2117616 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -522,6 +522,8 @@ core.logAllRefUpdates::
refs/heads/), remote refs (i.e. under refs/remotes/),
`refs/heads/`), remote refs (i.e. under `refs/remotes/`),
note refs (i.e. under `refs/notes/`), and the symbolic ref `HEAD`.
+ If it is set to `always`, then a missing reflog is automatically
+ created for any ref under `refs/`.
+
This information can be used to determine what commit
was the tip of a branch "2 days ago".
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 5055a96..2ac25a9 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -150,7 +150,8 @@ This option is only applicable when listing tags without
annotation lines.
'strip' removes both whitespace and commentary.
--create-reflog::
- Create a reflog for the tag.
+ Create a reflog for the tag. To globally enable reflogs for tags, see
+ `core.logAllRefUpdates` in linkgit:git-config[1].
::
The name of the tag to create, delete, or describe.
diff --git a/branch.c b/branch.c
index c431cbf..b955d4f 100644
--- a/branch.c
+++ b/branch.c
@@ -298,7 +298,7 @@ void create_branch(const char *name, const char *start_name,
start_name);
if (reflog)
- log_all_ref_updates = 1;
+ log_all_ref_updates = LOG_REFS_NORMAL;
if (!dont_change_ref) {
struct ref_transaction *transaction;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bfe685c..1db0b44 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -612,7 +612,7 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
const char *old_desc, *reflog_msg;
if (opts->new_branch) {
if (opts->new_orphan_branch) {
- if (opts->new_branch_log && !log_all_ref_updates) {
+ if (opts->new_branch_log &&
should_autocreate_reflog("refs/heads/")) {
int ret;
char *refname;
struct strbuf err = STRBUF_INIT;
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 76d68fa..1d4d6a0 100644
--- a/builtin/init-d
[PATCH v2 1/3] config: add markup to core.logAllRefUpdates doc
From: Cornelius Weig Signed-off-by: Cornelius Weig --- Notes: As suggested, I moved the modification of the markup to its own commit. Documentation/config.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index af2ae4c..3cd8030 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -517,10 +517,11 @@ core.logAllRefUpdates:: "`$GIT_DIR/logs/`", by appending the new and old SHA-1, the date/time and the reason of the update, but only when the file exists. If this configuration - variable is set to true, missing "`$GIT_DIR/logs/`" + variable is set to `true`, missing "`$GIT_DIR/logs/`" file is automatically created for branch heads (i.e. under refs/heads/), remote refs (i.e. under refs/remotes/), - note refs (i.e. under refs/notes/), and the symbolic ref HEAD. + `refs/heads/`), remote refs (i.e. under `refs/remotes/`), + note refs (i.e. under `refs/notes/`), and the symbolic ref `HEAD`. + This information can be used to determine what commit was the tip of a branch "2 days ago". -- 2.10.2
[PATCH v2 3/3] update-ref: add test cases for bare repository
From: Cornelius Weig
The default behavior of update-ref to create reflogs differs in
repositories with worktree and bare ones. The existing tests cover only
the behavior of repositories with worktree.
This commit adds tests that assert the correct behavior in bare
repositories for update-ref. Two cases are covered:
- If core.logAllRefUpdates is not set, no reflogs should be created
- If core.logAllRefUpdates is true, reflogs should be created
Signed-off-by: Cornelius Weig
---
t/t1400-update-ref.sh | 43 ---
1 file changed, 36 insertions(+), 7 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index b9084ca..bad88c8 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -8,23 +8,33 @@ test_description='Test git update-ref and basic ref logging'
Z=$_z40
-test_expect_success setup '
+m=refs/heads/master
+n_dir=refs/heads/gu
+n=$n_dir/fixes
+outside=refs/foo
+bare=bare-repo
+create_test_objects ()
+{
+ local T, sha1, prfx="$1"
for name in A B C D E F
do
test_tick &&
T=$(git write-tree) &&
sha1=$(echo $name | git commit-tree $T) &&
- eval $name=$sha1
+ eval $prfx$name=$sha1
done
+}
+test_expect_success setup '
+ create_test_objects "" &&
+ mkdir $bare &&
+ cd $bare &&
+ git init --bare &&
+ create_test_objects "bare" &&
+ cd -
'
-m=refs/heads/master
-n_dir=refs/heads/gu
-n=$n_dir/fixes
-outside=refs/foo
-
test_expect_success \
"create $m" \
"git update-ref $m $A &&
@@ -93,6 +103,25 @@ test_expect_success 'update-ref creates reflogs with
--create-reflog' '
git reflog exists $outside
'
+test_expect_success 'creates no reflog in bare repository' '
+ git -C $bare update-ref $m $bareA &&
+ git -C $bare rev-parse $bareA >expect &&
+ git -C $bare rev-parse $m >actual &&
+ test_cmp expect actual &&
+ test_must_fail git -C $bare reflog exists $m
+'
+
+test_expect_success 'core.logAllRefUpdates=true creates reflog in bare
repository' '
+ test_when_finished "git -C $bare config --unset core.logAllRefUpdates
&& \
+ rm $bare/logs/$m" &&
+ git -C $bare config core.logAllRefUpdates true &&
+ git -C $bare update-ref $m $bareB &&
+ git -C $bare rev-parse $bareB >expect &&
+ git -C $bare rev-parse $m >actual &&
+ test_cmp expect actual &&
+ git -C $bare reflog exists $m
+'
+
test_expect_success 'core.logAllRefUpdates=true does not create reflog by
default' '
test_config core.logAllRefUpdates true &&
test_when_finished "git update-ref -d $outside" &&
--
2.10.2
Re: [PATCH] refs: add option core.logAllRefUpdates = always
Hi Peff,
thanks for your thoughts.
> I tried to read this patch with fresh eyes. But given the history, you
> may take my review with a grain of salt. :)
Does it mean another reviewer is needed?
> I don't think my original had tests for this, but it might be worth
> adding a test for this last bit (i.e., that an update of ORIG_HEAD does
> not write a reflog when logallrefupdates is set to "always").
Good point. I blindly copied your commit message without thinking too
much about it.
> I guess the backtick fixups came from my original. It might be easier to
> see the change if they were pulled into their own patch, but it's
> probably not that big a deal.
If it's best practice to break out such changes, I'll revise it.
>> @@ -2835,8 +2835,8 @@ static int log_ref_write_1(const char *refname, const
>> unsigned char *old_sha1,
>> {
>> int logfd, result, oflags = O_APPEND | O_WRONLY;
>>
>> -if (log_all_ref_updates < 0)
>> -log_all_ref_updates = !is_bare_repository();
>> +if (log_all_ref_updates == LOG_REFS_UNSET)
>> +log_all_ref_updates = is_bare_repository() ? LOG_REFS_NONE :
>> LOG_REFS_NORMAL;
>
> This hunk is new, I think. The enum values are set in such a way that
> the original code would have continued to work, but I think using the
> symbolic names is an improvement.
Yes it's new.
> I assume you grepped for log_all_ref_updates to find this. I see only
> one spot that now doesn't use the symbolic names. In builtin/checkout.c,
> update_refs_for_switch() checks:
>
> if (opts->new_branch_log && !log_all_ref_updates)
>
> That looks buggy, as it would treat LOG_REFS_NORMAL and LOG_REFS_UNSET
> the same, and I do not see us resolving the UNSET case to a true/false
> value. But I don't think the bug is new in your patch; the default value
> was "-1" already.
>
> I doubt it can be triggered in practice, because either:
>
> - the config value is set in the config file, and we pick up that
> value, whether it's "true" or "false"
>
> - it's unset, in which case our default would be to enable reflogs in
> a non-bare repo. And since git-checkout would refuse to run in a
> bare repo, we must be non-bare, and thus enabling reflogs does the
> right thing.
That far I can follow.
> But it works quite by accident. I wonder if we should this
> "is_bare_repository" check into a function that can be called instead of
> accessing log_all_ref_updates() directly.
Are you saying that we should move the `!log_all_ref_updates` check into
its own function where we should also check `is_bare_repository`? I
don't see that this would win much, because as you said: checkouts in a
bare repo are forbidden anyway.
Other than that, I guess it should better read `log_all_ref_update !=
LOG_REFS_NONE` instead of `!log_all_ref_updates`.
>> +test_expect_success 'update-ref does not create reflog with
>> --no-create-reflog if core.logAllRefUpdates=always' '
>
> This test title is _really_ long, and will wrap in the output on
> reasonable-sized terminals. Maybe '--no-create-reflog overrides
> core.logAllRefUpdates=always' would be shorter?
Yes, I agree.
>> +test_expect_success 'stdin does not create reflog when
>> core.logAllRefUpdates=true' '
>
> I don't mind these extra stdin tests, but IMHO they are just redundant.
> The "--stdin --create-reflog" one makes sure the option is propagated
> down via the --stdin machinery. But we know the config option is handled
> at a low level anyway.
>
> I guess it depends on how black-box we want the testing to be. It just
> seems unlikely for a regression to be found here and not in the tests
> above.
Since these other stdin tests were around, I added this variant. But
you're right: this test breaks along with the other and doesn't add add
more safety. I'll remove it.
However, I realized that I have not written tests about ref updates in a
bare repository. Do you think it's worthwile?
Cheers,
Cornelius
Re: SubmittingPatches: drop temporal reference for PGP signing
> > Yeah I agree. My patch was not the best shot by far. > How about something along these lines? Does the forward reference break the main line of thought too severly? diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 08352de..c2b0cbe 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -216,12 +216,12 @@ that it will be postponed. Exception: If your mailer is mangling patches then someone may ask you to re-send them using MIME, that is OK. -Do not PGP sign your patch, at least for now. Most likely, your -maintainer or other people on the list would not have your PGP -key and would not bother obtaining it anyway. Your patch is not -judged by who you are; a good patch from an unknown origin has a -far better chance of being accepted than a patch from a known, -respected origin that is done poorly or does incorrect things. +Do not PGP sign your patch, but do sign-off your work as explained in (5). +Most likely, your maintainer or other people on the list would not have your +PGP key and would not bother obtaining it anyway. Your patch is not judged by +who you are; a good patch from an unknown origin has a far better chance of +being accepted than a patch from a known, respected origin that is done poorly +or does incorrect things. If you really really really really want to do a PGP signed patch, format it as "multipart/signed", not a text/plain message @@ -246,7 +246,7 @@ patch. *2* The mailing list: git@vger.kernel.org -(5) Sign your work +(5) Certify your work by signing off To improve tracking of who did what, we've borrowed the "sign-off" procedure from the Linux kernel project on patches
[PATCH] refs: add option core.logAllRefUpdates = always
Hi peff, you made it easy for me. Most of your patch still applied, only the tests didn't quite fit. Maybe you can have a look if I've overlooked something, since you know the changes best? Thanks for supporting this with your patch!
[PATCH] refs: add option core.logAllRefUpdates = always
From: Cornelius Weig
When core.logallrefupdates is true, we only create a new reflog for refs
that are under certain well-known hierarchies. The reason is that we
know that some hierarchies (like refs/tags) do not typically change, and
that unknown hierarchies might not want reflogs at all (e.g., a
hypothetical refs/foo might be meant to change often and drop old
history immediately).
However, sometimes it is useful to override this decision and simply log
for all refs, because the safety and audit trail is more important than
the performance implications of keeping the log around.
This patch introduces a new "always" mode for the core.logallrefupdates
option which will log updates to everything under refs/, regardless
where in the hierarchy it is (we still will not log things like
ORIG_HEAD and FETCH_HEAD, which are known to be transient).
Based-on-patch-by: Jeff King
Signed-off-by: Cornelius Weig
---
Documentation/config.txt | 7 +--
Documentation/git-tag.txt | 3 ++-
branch.c | 2 +-
builtin/init-db.c | 2 +-
cache.h | 9 +++-
config.c | 7 ++-
environment.c | 2 +-
refs.c| 15 +-
refs/files-backend.c | 6 +++---
t/t1400-update-ref.sh | 53 +++
10 files changed, 90 insertions(+), 16 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index af2ae4c..2117616 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -517,10 +517,13 @@ core.logAllRefUpdates::
"`$GIT_DIR/logs/`", by appending the new and old
SHA-1, the date/time and the reason of the update, but
only when the file exists. If this configuration
- variable is set to true, missing "`$GIT_DIR/logs/`"
+ variable is set to `true`, missing "`$GIT_DIR/logs/`"
file is automatically created for branch heads (i.e. under
refs/heads/), remote refs (i.e. under refs/remotes/),
- note refs (i.e. under refs/notes/), and the symbolic ref HEAD.
+ `refs/heads/`), remote refs (i.e. under `refs/remotes/`),
+ note refs (i.e. under `refs/notes/`), and the symbolic ref `HEAD`.
+ If it is set to `always`, then a missing reflog is automatically
+ created for any ref under `refs/`.
+
This information can be used to determine what commit
was the tip of a branch "2 days ago".
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 5055a96..2ac25a9 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -150,7 +150,8 @@ This option is only applicable when listing tags without
annotation lines.
'strip' removes both whitespace and commentary.
--create-reflog::
- Create a reflog for the tag.
+ Create a reflog for the tag. To globally enable reflogs for tags, see
+ `core.logAllRefUpdates` in linkgit:git-config[1].
::
The name of the tag to create, delete, or describe.
diff --git a/branch.c b/branch.c
index c431cbf..b955d4f 100644
--- a/branch.c
+++ b/branch.c
@@ -298,7 +298,7 @@ void create_branch(const char *name, const char *start_name,
start_name);
if (reflog)
- log_all_ref_updates = 1;
+ log_all_ref_updates = LOG_REFS_NORMAL;
if (!dont_change_ref) {
struct ref_transaction *transaction;
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 76d68fa..1d4d6a0 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -262,7 +262,7 @@ static int create_default_files(const char *template_path,
const char *work_tree = get_git_work_tree();
git_config_set("core.bare", "false");
/* allow template config file to override the default */
- if (log_all_ref_updates == -1)
+ if (log_all_ref_updates == LOG_REFS_UNSET)
git_config_set("core.logallrefupdates", "true");
if (needs_work_tree_config(original_git_dir, work_tree))
git_config_set("core.worktree", work_tree);
diff --git a/cache.h b/cache.h
index 00a029a..96eeaaf 100644
--- a/cache.h
+++ b/cache.h
@@ -660,7 +660,6 @@ extern int minimum_abbrev, default_abbrev;
extern int ignore_case;
extern int assume_unchanged;
extern int prefer_symlink_refs;
-extern int log_all_ref_updates;
extern int warn_ambiguous_refs;
extern int warn_on_object_refname_ambiguity;
extern const char *apply_default_whitespace;
@@ -728,6 +727,14 @@ enum hide_dotfiles_type {
};
extern enum hide_dotfiles_type hide_dotfiles;
+enum log_refs_config {
+ LOG_REFS_UNSET = -1,
+ LOG_REFS_NONE = 0,
+ LOG_REFS_NORMAL,
+ LOG_REFS_ALWAYS
+};
+extern enum log_refs_config log_all_ref_updates;
+
enum branch_track {
BRANCH_T
Re: [PATCH] tag: add tag.createReflog option
> > It may have been obvious, but to be explicit for somebody new, > !prefixcmp() corresponds to starts_with(). IOW, we changed the > meaning of the return value when moving from cmp-lookalike (where 0 > means "equal") to "does it start with this string?" bool (where 1 > means "yes"). > I see. It reads much better that way! I re-did all the changes from Jeff's patch, but some tests are breaking now. I will have to mend that tomorrow, because it's already too late in my timezone. Thanks a lot for your support m(_ _)m
Re: [PATCH] tag: add tag.createReflog option
On 01/25/2017 07:00 PM, Jeff King wrote: > - Is that the end of it, or is the desire really "I want reflogs for > _everything_"? That seems like a sane thing to want. > > If so, then the update to core.logallrefupdates should turn it into > a tri-state: > > - false; no reflogs > > - true; reflogs for branches, remotes, notes, as now > > - always; reflogs for all refs under "refs/" > I think you nailed it. This is much more useful than what I suggested. I'll see if I can code it up.
Re:
On 01/25/2017 01:21 AM, Stefan Beller wrote: >> >>> Do not PGP sign your patch, at least *for now*. (...) >> > > And maybe these 2 small words are the bug in the documentation? > Shall we drop the "at least for now" part, like so: > > ---8<--- > From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001 > From: Stefan Beller > Date: Tue, 24 Jan 2017 16:19:13 -0800 > Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP signing > > Signed-off-by: Stefan Beller > --- > Documentation/SubmittingPatches | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches > index 08352deaae..28da4ad2d4 100644 > --- a/Documentation/SubmittingPatches > +++ b/Documentation/SubmittingPatches > @@ -216,12 +216,12 @@ that it will be postponed. > Exception: If your mailer is mangling patches then someone may ask > you to re-send them using MIME, that is OK. > > -Do not PGP sign your patch, at least for now. Most likely, your > -maintainer or other people on the list would not have your PGP > -key and would not bother obtaining it anyway. Your patch is not > -judged by who you are; a good patch from an unknown origin has a > -far better chance of being accepted than a patch from a known, > -respected origin that is done poorly or does incorrect things. > +Do not PGP sign your patch. Most likely, your maintainer or other > +people on the list would not have your PGP key and would not bother > +obtaining it anyway. Your patch is not judged by who you are; a good > +patch from an unknown origin has a far better chance of being accepted > +than a patch from a known, respected origin that is done poorly or > +does incorrect things. > > If you really really really really want to do a PGP signed > patch, format it as "multipart/signed", not a text/plain message > It definitely is an improvement. Though it would still leave me puzzled when finding a section about signing just below. Is changing heading (5) too big a change? Like so: diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 08352de..71898dc 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -246,7 +246,7 @@ patch. *2* The mailing list: git@vger.kernel.org -(5) Sign your work +(5) Certify your work by signing off. To improve tracking of who did what, we've borrowed the "sign-off" procedure from the Linux kernel project on patches
Re: [PATCH 7/7] completion: recognize more long-options
On 01/25/2017 12:43 AM, Stefan Beller wrote: > On Tue, Jan 24, 2017 at 3:33 PM, Cornelius Weig > wrote: >> On 01/25/2017 12:24 AM, Junio C Hamano wrote: >>> Cornelius Weig writes: >>> >>>>> Please study item (5) "Sign your work" in >>>>> Documentation/SubmittingPatches and sign off your work. >>>> >>>> I followed the recommendations to submitting work, and in the first >>>> round signing is discouraged. >>> >>> Just this point. You found a bug in our documentation if that is >>> the case; it should not be giving that impression to you. >>> >> >> Well, I am referring to par. (4) of Documentation/SubmittingPatches >> (emphasis mine): >> >> <<<<<<<<<<<<<< >> *Do not PGP sign your patch, at least for now*. Most likely, your >> maintainer or other people on the list would not have your PGP >> key and would not bother obtaining it anyway. Your patch is not >> judged by who you are; a good patch from an unknown origin has a >> far better chance of being accepted than a patch from a known, >> respected origin that is done poorly or does incorrect things. >> <<<<<<<<<<<<<< >> >> If first submissions should be signed as well, then I find this quite >> misleading. >> > > Please read on; While this part addresses PGP signing, > which is discouraged at any round, > later on we talk about another type of signing. > (not cryptographic strong signing, but signing the intent;) > the DCO in the commit message. > > So no PGP signing (in any round of the patch). > > But DCO signed (in anything that you deem useful for the > project and that you are allowed to contribute) > Right, it's crystal clear now. What confused me was the combination of > Do not PGP sign your patch, at least *for now*. (...) and then the section with heading > (5) *Sign* your work So I didn't even bother to read (5) because I deemed it irrelevant. I think if it had said `(5) *Certify* your work` this would not have happened.
[PATCH] tag: add tag.createReflog option
From: Cornelius Weig
Git does not create a history for tags, in contrast to common
expectation to simply version everything. This can be changed by using
the `--create-reflog` flag when creating the tag. However, a config
option to enable this behavior by default is missing.
This commit adds the configuration variable `tag.createReflog` which
enables reflogs for new tags by default.
Signed-off-by: Cornelius Weig
---
Documentation/config.txt | 5 +
Documentation/git-tag.txt | 8 +---
builtin/tag.c | 6 +-
t/t7004-tag.sh| 14 ++
4 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index af2ae4c..9e5f6f6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2945,6 +2945,11 @@ submodule.alternateErrorStrategy
as computed via `submodule.alternateLocation`. Possible values are
`ignore`, `info`, `die`. Default is `die`.
+tag.createReflog::
+ A boolean to specify whether newly created tags should have a reflog.
+ If `--[no-]create-reflog` is specified on the command line, it takes
+ precedence. Defaults to `false`.
+
tag.forceSignAnnotated::
A boolean to specify whether annotated tags created should be GPG
signed.
If `--annotate` is specified on the command line, it takes
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 5055a96..f2ed370 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,7 @@ SYNOPSIS
[ | ]
'git tag' -d ...
'git tag' [-n[]] -l [--contains ] [--points-at ]
- [--column[=] | --no-column] [--create-reflog] [--sort=]
+ [--column[=] | --no-column] [--[no-]create-reflog]
[--sort=]
[--format=] [--[no-]merged []] [...]
'git tag' -v ...
@@ -149,8 +149,10 @@ This option is only applicable when listing tags without
annotation lines.
all, 'whitespace' removes just leading/trailing whitespace lines and
'strip' removes both whitespace and commentary.
---create-reflog::
- Create a reflog for the tag.
+--[no-]create-reflog::
+ Force to create a reflog for the tag, or no reflog if
`--no-create-reflog`
+ is used. Unless the `tag.createReflog` config variable is set to true,
no
+ reflog is created by default. See linkgit:git-config[1].
::
The name of the tag to create, delete, or describe.
diff --git a/builtin/tag.c b/builtin/tag.c
index 73df728..1f13e4d 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -30,6 +30,7 @@ static const char * const git_tag_usage[] = {
static unsigned int colopts;
static int force_sign_annotate;
+static int create_reflog;
static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
const char *format)
{
@@ -165,6 +166,10 @@ static int git_tag_config(const char *var, const char
*value, void *cb)
force_sign_annotate = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, "tag.createreflog")) {
+ create_reflog = git_config_bool(var, value);
+ return 0;
+ }
if (starts_with(var, "column."))
return git_column_config(var, value, "tag", &colopts);
@@ -325,7 +330,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
const char *object_ref, *tag;
struct create_tag_options opt;
char *cleanup_arg = NULL;
- int create_reflog = 0;
int annotate = 0, force = 0;
int cmdmode = 0, create_tag_object = 0;
const char *msgfile = NULL, *keyid = NULL;
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 1cfa8a2..67b39ec 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -90,6 +90,20 @@ test_expect_success '--create-reflog does not create reflog
on failure' '
test_must_fail git reflog exists refs/tags/mytag
'
+test_expect_success 'option tag.createreflog creates reflog by default' '
+ test_when_finished "git tag -d tag_with_reflog" &&
+ git config tag.createReflog true &&
+ git tag tag_with_reflog &&
+ git reflog exists refs/tags/tag_with_reflog
+'
+
+test_expect_success 'option tag.createreflog overridden by command line' '
+ test_when_finished "git tag -d tag_without_reflog" &&
+ git config tag.createReflog true &&
+ git tag --no-create-reflog tag_without_reflog &&
+ test_must_fail git reflog exists refs/tags/tag_without_reflog
+'
+
test_expect_success 'listing all tags if one exists should succeed' '
git tag -l &&
git tag
--
2.10.2
Re: [PATCH 7/7] completion: recognize more long-options
On 01/25/2017 12:24 AM, Junio C Hamano wrote: > Cornelius Weig writes: > >>> Please study item (5) "Sign your work" in >>> Documentation/SubmittingPatches and sign off your work. >> >> I followed the recommendations to submitting work, and in the first >> round signing is discouraged. > > Just this point. You found a bug in our documentation if that is > the case; it should not be giving that impression to you. > Well, I am referring to par. (4) of Documentation/SubmittingPatches (emphasis mine): <<<<<<<<<<<<<< *Do not PGP sign your patch, at least for now*. Most likely, your maintainer or other people on the list would not have your PGP key and would not bother obtaining it anyway. Your patch is not judged by who you are; a good patch from an unknown origin has a far better chance of being accepted than a patch from a known, respected origin that is done poorly or does incorrect things. >>>>>>>>>>>>>> If first submissions should be signed as well, then I find this quite misleading.
Re: [PATCH 7/7] completion: recognize more long-options
On 01/24/2017 08:15 AM, Johannes Sixt wrote: > If at all possible, please use your real email address as the From > address. It is pointless to hide behind a fake address because as Git > contributor you will have to reveal your identity anyway. These are both real addresses, but for send-mail I would not want to use my work account. I hope this is not a problem. > Please study item (5) "Sign your work" in > Documentation/SubmittingPatches and sign off your work. I followed the recommendations to submitting work, and in the first round signing is discouraged. > AFAIR, it was a deliberate decision that potentially destructive command > line options are not included in command completions. In the list given, > I find these: > >> - reset: --merge --mixed --hard --soft --patch --keep My bad, I only added --keep, which should be fine. As to these options >> - apply: --unsafe-paths >> - rm: --force let's wait for further comments, but I won't cling to it. > Additionally, these options are only for internal purposes, but I may be > wrong: > >> - archive: --remote= --exec= These may in fact be too exotic and just clutter the command line. Best they are removed. -- Cornelius
