Re: [PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation
I checked the documentation of different commands. From what I've seen, such indications either does not appear or are right after the text. I agree that it's a good idea, but for the sake of consistency, I'd rather use one of these two format as long as it's ok for you. After re-checking: OK for me, sorry for the noise -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits
Junio C Hamano writes: > The anticipation is to use another feature introducer after "/!" to > enhance the matching, so that we can keep enhancing the syntax. > > cf. http://thread.gmane.org/gmane.comp.version-control.git/40460/focus=40477 > > Using "/!Message" to match commits that do not match Message > directly goes against that extensivility design. > > We need to always remind ourselves that our latest shiny new toy > will not be the final new feature. There always will be need to add > yet another new thing, and we need to keep the door open for them. > > Perhaps > > /!-string -> find commit without "string" > > or something? Of course, as I do not think it is something people would do regularly to look for a non-match, I do not necessarily think we need a short-hand "/!-string". Perhaps following the long-hand syntax suggested in that old article, it may be sensible to start with something more descriptive like /!(negative)string to look for a commit that does not say "string", without the short-hand form. Only after we see that people find the feature useful and find the need to use it frequently (if it ever happens, that is), we can introduce "/!-string" as a short-hand form as a follow-up patch. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits
On Wed, Jun 3, 2015 at 10:52 PM, Junio C Hamano wrote:
> The /! sequence being reserved does not mean it was planned to be
> used only for a single thing in the future, though.
>
> (snip)
>
> cf. http://thread.gmane.org/gmane.comp.version-control.git/40460/focus=40477
>
Thank you for that additional context, which I didn't see previously.
> Using "/!Message" to match commits that do not match Message
> directly goes against that extensivility design.
>
> We need to always remind ourselves that our latest shiny new toy
> will not be the final new feature. There always will be need to add
> yet another new thing, and we need to keep the door open for them.
>
> Perhaps
>
> /!-string -> find commit without "string"
>
> or something?
>
What I'm thinking now is that "@^{/foo}" can be thought of as a
potential "shorthand-form" of what could be "@^{/!(m=foo)}", in which
case "@^{/!-foo}" could similarly be thought of as a potential
shorthand-form of what could be "@^{/!(m-foo)}".
So with that in mind, I agree that a syntax of "@^{/!-foo}" could indeed give
me the results I'm looking for, while leaving room for the previously
mentioned forms of future extension.
I don't know if I consider those potential extensions to be commendable
as a unified (and chain-able) syntax for finding revisions in the graph,
or to be needless clutter which would only add "yet another way to specify
the same thing". I mean, I like the idea of being able to specify that
I want "The third parent of the first commit authored by Fred which is
also an ancestor of a commit which touched a file in the libraries
subdirectory", it sounds like maybe it would be good to be able to do
that sort of thing without bringing xargs and shell expansion into the
picture... but I certainly don't have a clue what it might be good for!
In any case, it sounds like we have a good way forward for this smaller
change, at least. I'll re-submit with the suggested syntax.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Urgent Notification
You are required to click on the link to verify your email account because we are upgrading our webmail.http://bethanychildrenhome.com/121/ Webmail Technical Support Copyright 2012. All Rights Reserved -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] status: give more information during rebase -i
git status gives more information during rebase -i, about the list of command that are done during the rebase. It displays the two last commands executed and the two next lines to be executed. It also gives hints to find the whole files in .git directory. --- t/t7512-status-help.sh | 111 + wt-status.c| 90 +++ 2 files changed, 201 insertions(+) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 190656d..4dd201a 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -134,9 +134,13 @@ test_expect_success 'prepare for rebase_i_conflicts' ' test_expect_success 'status during rebase -i when conflicts unresolved' ' test_when_finished "git rebase --abort" && ONTO=$(git rev-parse --short rebase_i_conflicts) && + LAST_COMMIT=$(git rev-parse rebase_i_conflicts_second) && test_must_fail git rebase -i rebase_i_conflicts && cat >expected1? 0 : 1; + for (i = begin; i < 2; i++) { + status_printf_ln(s,color," %s",lines[i]); + } + if (numlines > 2 && s->hints) + status_printf_ln(s,color, + _(" (see more at .git/rebase-merge/done)")); + } + numlines = 0; + get_two_first_lines("rebase-merge/git-rebase-todo", +&numlines, lines); + if (numlines == 0) + status_printf_ln(s, color, +_("No command remaining.")); + else{ + + status_printf_ln(s, color, + _("Next command(s) to do (%d remaining command(s)):"), + numlines); + begin = numlines > 1? 0 : 1; + for (i = 0; (i < 2 && i < numlines); i++) { + status_printf(s, color, " %s", lines[i]); + } + if (s->hints) + status_printf_ln(s, color, + _(" (use git rebase --edit-todo to view and edit)")); + } + } +} + static void show_rebase_in_progress(struct wt_status *s, struct wt_status_state *state, const char *color) { struct stat st; + show_rebase_information(s, state, color); if (has_unmerged(s) || state->rebase_in_progress || !stat(git_path("MERGE_MSG"), &st)) { if (state->branch) status_printf_ln(s, color, -- 2.4.2.342.g3cebd9b -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] status: add new tests for status during rebase -i
--- t/t7512-status-help.sh | 88 ++ 1 file changed, 88 insertions(+) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 4dd201a..dff912b 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -856,4 +856,92 @@ EOF test_i18ncmp expected actual ' +test_expect_success 'prepare for different number of commits rebased' ' + git reset --hard master && + git checkout -b several_commits && + test_commit one_commit main.txt one && + test_commit two_commit main.txt two && + test_commit three_commit main.txt three && + test_commit four_commit main.txt four +' + + +test_expect_success 'status: one command done nothing remaining' ' + FAKE_LINES=" exec_exit_15" && + export FAKE_LINES && + test_when_finished "git rebase --abort" && + ONTO=$(git rev-parse --short HEAD~3) && + (git rebase -i HEAD~3 || true)&& + cat >expectedexpected expected
[PATCH 1/4] status: factor two rebase-related messages together
---
wt-status.c | 30 +++---
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 33452f1..fec6e85 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1032,7 +1032,7 @@ static void show_rebase_in_progress(struct wt_status *s,
{
struct stat st;
- if (has_unmerged(s)) {
+ if (has_unmerged(s) || state->rebase_in_progress ||
!stat(git_path("MERGE_MSG"), &st)) {
if (state->branch)
status_printf_ln(s, color,
_("You are currently rebasing branch
'%s' on '%s'."),
@@ -1042,25 +1042,17 @@ static void show_rebase_in_progress(struct wt_status *s,
status_printf_ln(s, color,
_("You are currently rebasing."));
if (s->hints) {
- status_printf_ln(s, color,
- _(" (fix conflicts and then run \"git rebase
--continue\")"));
- status_printf_ln(s, color,
- _(" (use \"git rebase --skip\" to skip this
patch)"));
- status_printf_ln(s, color,
- _(" (use \"git rebase --abort\" to check out
the original branch)"));
+ if (has_unmerged(s)) {
+ status_printf_ln(s, color,
+ _(" (fix conflicts and then run \"git
rebase --continue\")"));
+ status_printf_ln(s, color,
+ _(" (use \"git rebase --skip\" to skip
this patch)"));
+ status_printf_ln(s, color,
+ _(" (use \"git rebase --abort\" to
check out the original branch)"));
+ } else
+ status_printf_ln(s, color,
+ _(" (all conflicts fixed: run \"git
rebase --continue\")"));
}
- } else if (state->rebase_in_progress || !stat(git_path("MERGE_MSG"),
&st)) {
- if (state->branch)
- status_printf_ln(s, color,
-_("You are currently rebasing branch
'%s' on '%s'."),
-state->branch,
-state->onto);
- else
- status_printf_ln(s, color,
-_("You are currently rebasing."));
- if (s->hints)
- status_printf_ln(s, color,
- _(" (all conflicts fixed: run \"git rebase
--continue\")"));
} else if (split_commit_in_progress(s)) {
if (state->branch)
status_printf_ln(s, color,
--
2.4.2.342.g3cebd9b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] status: differentiate interactive from non-interactive rebases
--- t/t7512-status-help.sh | 28 ++-- wt-status.c| 5 - 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 68ad2d7..190656d 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -136,7 +136,7 @@ test_expect_success 'status during rebase -i when conflicts unresolved' ' ONTO=$(git rev-parse --short rebase_i_conflicts) && test_must_fail git rebase -i rebase_i_conflicts && cat >expected
Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits
Will Palmer writes:
> Specifically, as documented: '^{/!Message}' should fail, as this syntax
> is currently reserved; while '^{!!Message}' should search for a commit
> whose message contains the string "!Message".
The /! sequence being reserved does not mean it was planned to be
used only for a single thing in the future, though.
Think of it as a syntax to introduce extended features, the first
use of which was this:
/!!string -> find commit with "!string"
The above is just one "feature" that the reserved syntax allows,
namely, "to find a string that begins with an exclamation mark".
The anticipation is to use another feature introducer after "/!" to
enhance the matching, so that we can keep enhancing the syntax.
cf. http://thread.gmane.org/gmane.comp.version-control.git/40460/focus=40477
Using "/!Message" to match commits that do not match Message
directly goes against that extensivility design.
We need to always remind ourselves that our latest shiny new toy
will not be the final new feature. There always will be need to add
yet another new thing, and we need to keep the door open for them.
Perhaps
/!-string -> find commit without "string"
or something?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
Jeff King writes: > On Wed, Jun 03, 2015 at 10:32:40AM -0700, Junio C Hamano wrote: > > Yeah, I'd say "cp -i" is the closest thing. I don't have a problem with > adding that, but I'd really hate for it to be the default (just as I > find distros which "alias rm='rm -i" annoying). Oh, no question about it. I think a typical user cease to be a newbie before having to type "-i" every time starts to annoy her, and instead will learn to use the tool more effectively and efficiently [*1*], so making "-i" default is not good not just for you but for everybody. [Footnote] *1* In the context of this discussion, after screwing up the change in hello.c, instead of expressing the wish to recover and to start from scratch in two separate commands, i.e. rm hello.c && update-from-scm they will learn to use a single command that is designed for that purpose, i.e. checkout-from-scm hello.c without the "rm" step, which _is_ an artificial workaround for their other SCMs that do not update from the repository unless they remove the files. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] read_loose_refs(): treat NULL_SHA1 loose references as broken
Jeff King writes: > On Wed, Jun 03, 2015 at 11:51:43AM -0700, Junio C Hamano wrote: > >> Jeff King writes: >> >> > On Wed, Jun 03, 2015 at 03:51:59PM +0200, Michael Haggerty wrote: >> > >> >> NULL_SHA1 is used to indicate of "invalid SHA-1" throughout our code >> > >> > s/of/an/ ? >> >> Also possibly s/invalid SHA-1/invalid ref/? > > I thought it was trying to express that we use the null sha1 as a > sentinel value throughout the code, no matter if the value came from a > ref or otherwise. Yeah, an invalid object name, not limited to refs, is correct. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSoC] Week 1: Unification of 'for-each-ref', 'tag -l' and 'branch -l'
On Wed, Jun 03, 2015 at 06:08:50PM +0200, Matthieu Moy wrote: > Karthik Nayak writes: > > > Matthieu Moy suggested that I work on the unification of these > > commands let both the implementations stay where the user can select > > the implementation to be used > > Just to be clear: my advice is the above with "user" = "caller of the > API", not "human being using Git". In other words, "git branch > --contains" and "git tag --contains" would still be using two different > algorithms, but the user wouldn't notice (except for performance). Yeah, I think that is sensible. It should be a "feature" of the ref-filter that can hopefully go away one day (when we have a sensible implementation that works for both; this is something I've been meaning to push forward, but Karthik should not have to wait on me). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] test for '!' handling in rev-parse's named commits
In anticipation of modifying this behaviour, add a test verifying the
handling of exclamation marks when looking up a commit "by name".
Specifically, as documented: '^{/!Message}' should fail, as this syntax
is currently reserved; while '^{!!Message}' should search for a commit
whose message contains the string "!Message".
Signed-off-by: Will Palmer
---
t/t1511-rev-parse-caret.sh | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh
index 15973f2..0c46e5c 100755
--- a/t/t1511-rev-parse-caret.sh
+++ b/t/t1511-rev-parse-caret.sh
@@ -18,7 +18,14 @@ test_expect_success 'setup' '
git checkout master &&
echo modified >>a-blob &&
git add -u &&
- git commit -m Modified
+ git commit -m Modified &&
+ echo changed! >>a-blob &&
+ git add -u &&
+ git commit -m !Exp &&
+ git branch expref &&
+ echo changed >>a-blob &&
+ git add -u &&
+ git commit -m Changed
'
test_expect_success 'ref^{non-existent}' '
@@ -77,4 +84,14 @@ test_expect_success 'ref^{/Initial}' '
test_cmp expected actual
'
+test_expect_success 'ref^{/!Exp}' '
+ test_must_fail git rev-parse master^{/!Exp}
+'
+
+test_expect_success 'ref^{/!!Exp}' '
+ git rev-parse expref >expected &&
+ git rev-parse master^{/!!Exp} >actual &&
+ test_cmp expected actual
+'
+
test_done
--
2.3.0.rc1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] specify commit by negative pattern
add support for negative pattern matching in @^{/} style
revision specifiers. So now you can find the first commit whose message
doesn't match a pattern, complementing the existing positive matching.
e.g.:
$ git rebase -i @^{/!^WIP}
My use-case is in having a "work, work, work, rebase, push"-style
workflow, which generates a lot of "WIP foo" commits. While rebasing is
usually handled via "git rebase -i origin/master", occasionally I will
already have several "good, but not yet ready to push" commits hanging
around while I finish work on related commits. In these situations, the
ability to quickly "git diff @^{/!^WIP}" to get an overview of all
changes "since the last one I was happy with", can be useful.
Reading through the history of this type of revision specifier, it feels
like a negative match was always thought of as potentially useful
someday, but didn't fit well with the original patch's limitations
(namely: always searching across all refs).
Will Palmer (2):
test for '!' handling in rev-parse's named commits
object name: introduce '^{/!}' notation
Documentation/revisions.txt | 7 ---
sha1_name.c | 22 --
t/t1511-rev-parse-caret.sh | 45 -
3 files changed, 64 insertions(+), 10 deletions(-)
--
2.3.0.rc1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] object name: introduce '^{/!}' notation
To name a commit, you can now say
$ git rev-parse HEAD^{/!foo}
and it will return the hash of the first commit reachable from HEAD,
whose commit message does not contain "foo".
Since the ability to reference a commit by "name" was introduced (way
back in 1.5, in 364d3e6), with the across-all-refs syntax of ':/foo',
there has been a note in the documentation indicating that a leading
exclamation mark was "reserved for now" (unless followed immediately be
another exclamation mark.)
At the time, this was sensible: we didn't get the '^{/foo}' flavour
until sometime around 1.7.4 (41cd797) , so while a "negative search" was
a foreseeable feature, it wouldn't have made much sense to apply one
across all refs, as the result would have been essentially random.
These days, a negative pattern can make sense. In particular, if you tend
to use a rebase-heavy workflow with many "work in progress" commits, it
may be useful to diff or rebase against the latest "not work-in-progress"
commit. That sort of thing now possible, via commands such as:
$ git rebase -i @^{/!^WIP}
Perhaps notably, the "special case" for the empty pattern has been
extended to handle the empty negative pattern - which never matches, to
continue to ensure that an empty pattern never reaches the real regexp
code, as per notes in 4322842 "get_sha1: handle special case $commit^{/}"
Signed-off-by: Will Palmer
---
Documentation/revisions.txt | 7 ---
sha1_name.c | 22 --
t/t1511-rev-parse-caret.sh | 32 +---
3 files changed, 49 insertions(+), 12 deletions(-)
diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 0796118..6a6b8b9 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -151,9 +151,10 @@ existing tag object.
A colon, followed by a slash, followed by a text, names
a commit whose commit message matches the specified regular expression.
This name returns the youngest matching commit which is
- reachable from any ref. If the commit message starts with a
- '!' you have to repeat that; the special sequence ':/!',
- followed by something else than '!', is reserved for now.
+ reachable from any ref. To name a commit whose commit message does not
+ match the specified regular expression, begin the pattern-part with a
+ '!', e.g. ':/!foo'. If the commit message you wish to match starts with
+ a '!' you have to repeat that.
The regular expression can match any part of the commit message. To
match messages starting with a string, one can use e.g. ':/^foo'.
diff --git a/sha1_name.c b/sha1_name.c
index 46218ba..3d50dc9 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -737,11 +737,15 @@ static int peel_onion(const char *name, int len, unsigned
char *sha1)
/*
* $commit^{/}. Some regex implementation may reject.
-* We don't need regex anyway. '' pattern always matches.
+* We don't need regex anyway. '' pattern always matches,
+* and '!' pattern never matches.
*/
if (sp[1] == '}')
return 0;
+ if (sp[1] == '!' && sp[2] == '}')
+ return -1;
+
prefix = xstrndup(sp + 1, name + len - 1 - (sp + 1));
commit_list_insert((struct commit *)o, &list);
ret = get_sha1_oneline(prefix, sha1, list);
@@ -825,8 +829,9 @@ static int get_sha1_1(const char *name, int len, unsigned
char *sha1, unsigned l
* through history and returning the first commit whose message starts
* the given regular expression.
*
- * For future extension, ':/!' is reserved. If you want to match a message
- * beginning with a '!', you have to repeat the exclamation mark.
+ * For negative-matching, prefix the pattern-part with a '!', like:
+ * ':/!WIP'. If you want to match a message beginning with a literal
+ * '!', you heave to repeat the exlamation mark.
*/
/* Remember to update object flag allocation in object.h */
@@ -855,11 +860,16 @@ static int get_sha1_oneline(const char *prefix, unsigned
char *sha1,
{
struct commit_list *backup = NULL, *l;
int found = 0;
+ int negative = 0;
regex_t regex;
if (prefix[0] == '!') {
- if (prefix[1] != '!')
- die ("Invalid search pattern: %s", prefix);
+ if (prefix[1] != '!') {
+ negative = 1;
+ } else if (prefix[1] == '!' && prefix[2] == '!') {
+ negative = 1;
+ prefix++;
+ }
prefix++;
}
@@ -880,7 +890,7 @@ static int get_sha1_oneline(const char *prefix, unsigned
char *sha1,
continue;
buf = get_commit_buffer(commit, NULL);
p = strstr(buf, "\n\n");
- matches = p && !regexec(®ex, p + 2, 0, NULL, 0);
+
Re: [PATCH v3 4/4] read_loose_refs(): treat NULL_SHA1 loose references as broken
On Wed, Jun 03, 2015 at 11:51:43AM -0700, Junio C Hamano wrote: > Jeff King writes: > > > On Wed, Jun 03, 2015 at 03:51:59PM +0200, Michael Haggerty wrote: > > > >> NULL_SHA1 is used to indicate of "invalid SHA-1" throughout our code > > > > s/of/an/ ? > > Also possibly s/invalid SHA-1/invalid ref/? I thought it was trying to express that we use the null sha1 as a sentinel value throughout the code, no matter if the value came from a ref or otherwise. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
From: "Ed Avis" Sent: Wednesday, June 03, 2015 10:55 AM Jeff King peff.net> writes: I would say the more "usual" way to use checkout like this is to give specific paths. I.e., run "git status", say "oh, I need to restore the contents of 'foo', but not 'bar'", and run "git checkout foo". That works regardless of the type of change to "foo" and "bar". That seems fine - a specific file is named and you clearly want to alter the contents of that file. By analogy, 'rm foo' will silently delete it, but if you specify a directory to delete recursively you need the -r flag. OK, it's not a perfect analogy because the purpose of rm is to delete data and nothing else ;-). If my personal experience is anything to go by, newcomers may fall into the habit of running 'git checkout .' to restore missing files. In the old days I would often delete a file and then run 'cvs update' or 'svn update' to restore it. That would fetch a fresh copy from the repository, and while it might do some kind of diff/patch operation on modified files, it would not simply throw away local changes. 'git checkout .' seems like the analogous command, but it has much sharper edges. I still think it should be safer by default, but if you decide against that then perhaps you need to create some way to restore missing files and not overwrite others. 'git checkout --no-overwrite'? Then it could even be added to .gitconfig as the default for those who like it. I have to say that as a newcomer to git I do not like the idea of creating a special undo log for git. It would just be yet another concept to learn and another thing to add to the list of 'where is git hiding my data this time?'. And the time when it would be useful - after some bungled operation that lost data - is just the time when the user is already confused and adding another semi-hidden stash of objects to the mix would befuddle them further. If there is to be a backup made of local changes that get lost, and I agree it is a good idea, then it should be something stupid and completely obvious, such as saving the old file as 'foo.before_checkout.1'. -- Ed Avis To me, when I saw the 'git checkout .', I was reminded of the 'git push . ' special case where '.' is the repo, so in my mind the first thought was that Ed wanted to checkout the head of the current repo, and that should have barfed from that viewpoint. The [is it equivalent? (rhet)] 'git checkout -- .' would clearly indicate that the '.' refers to the files of the current directory (wouldn't it?) So it's about how '.' is perceived by the code in different circumstances, and whether, perhaps, the optional discriminating '--' should be required in this (special) case. Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFCv2 2/2] git rebase -i: warn about removed commits
Eric Sunshine writes: > > +test_expect_success 'rebase -i respects rebase.missingCommitsCheck=ignore' > > ' > > + test_config rebase.missingCommitsCheck ignore && > > + test_when_finished "git checkout master && > > + git branch -D tmp2" && > > Strange indentation. Considering that 'git branch -D tmp2' is a part of test_when_finished, I wasn't sure of how it was supposed to be indented, so I did it this way to show that it was still within test_when_finished and not a separate command. > test_when_finished "git checkout master && > git branch -D tmp2" && Doesn't seem as clear, especially if you quickly read the lines. For now, I have removed the tab. Also the other points have been corrected. Thank you, Rémi -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/2] status: fix tests to handle new verbose git status during rebase
Thanks for reviewing, I take everything into account and release a v2 ASAP. Guillaume -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
On Wed, Jun 03, 2015 at 09:55:05AM +, Ed Avis wrote: > Jeff King peff.net> writes: > > If my personal experience is anything to go by, newcomers may fall into the > habit of running 'git checkout .' to restore missing files. In the old days > I would often delete a file and then run 'cvs update' or 'svn update' to > restore it. That would fetch a fresh copy from the repository, and while > it might do some kind of diff/patch operation on modified files, it would > not simply throw away local changes. > The problem with these kinds of habbits is that they easily extend to the --force variant. If people execute git checkout . as a habbit without thinking, they will soon train to do git checkout -f . without thinking, and then you still have the same problem. I do share your sentiment that it's easy to loose uncomitted changes to git checkout , but like Jeff said, the entire goal of this command is to reset specific files from the index or commits. Introducing a way to undo this would be a much better option to me then adding an extra switch with no way to undo. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] implement submodule config cache for lookup of submodule names
On Tue, Jun 02, 2015 at 12:57:08PM -0700, Junio C Hamano wrote:
> Heiko Voigt writes:
>
> > This submodule configuration cache allows us to lazily read .gitmodules
> > configurations by commit into a runtime cache which can then be used to
> > easily lookup values from it. Currently only the values for path or name
> > are stored but it can be extended for any value needed.
> >
> > It is expected that .gitmodules files do not change often between
> > commits. Thats why we lookup the .gitmodules sha1 from a commit and then
> > either lookup an already parsed configuration or parse and cache an
> > unknown one for each sha1. The cache is lazily build on demand for each
> > requested commit.
> >
> > This cache can be used for all purposes which need knowledge about
> > submodule configurations. Example use cases are:
> >
> > * Recursive submodule checkout needs lookup a submodule name from its
> >path when a submodule first appears. This needs be done before this
> >configuration exists in the worktree.
> >
> > * The implementation of submodule support for 'git archive' needs to
> >lookup the submodule name to generate the archive when given a
> >revision that is not checked out.
> >
> > * 'git fetch' when given the --recurse-submodules=on-demand option (or
> >configuration) needs to lookup submodule names by path from the
> >database rather than reading from the worktree. For new submodule it
> >needs to lookup the name from its path to allow cloning new
> >submodules into the .git folder so they can be checked out without
> >any network interaction when the user does a checkout of that
> >revision.
>
> What is unclear to me after reading the above twice is what this
> thing is meant to achieve. Is it efficiency by doing lazy lookups
> and caching to avoid asking the same thing more than once from
> either the filesystem or read_sha1_file()? Is it expected that
> reading through this "cache" will be the _only_ way callers would
> interact with the .gitmodules data, or is it an opt-in feature that
> some callers that do not see the benefit (why they may want to
> ignore is totally unclear, because what the "cache" system wants to
> achieve is) can safely ignore and bypass?
Maybe I should add a paragraph like this on top:
In a superproject some commands need to interact with submodules. They
need to query values from the .gitmodules file either from the
worktree of from certain revisions. At the moment this is quite hard
since a caller would need to read the .gitmodules file from the
history and then parse the values. We want to provide an API for this
so we have one place to get values from .gitmodules from any revision
(including the worktree).
So yes it will be the only way callers would read .gitmodules data.
Since you abstractly wrote "interact" I realize that I have not thought
about writing values.
> Because the above talks about looking up ".gitmodules from a
> commit", I am guessing that the "commit" used as one of the lookup
> keys throughout the system is a commit in the superproject, not from
> submodules, but you may want to state that more explicitly.
Yes. Well it is a commit in the current project which should be the
superproject to other submodules. Assuming we have the additional
paragraph above. Does that make it more clear?
> What, if anything, should be done for .gitmodules that are not yet
> committed? Are there cases that the callers that usually interact
> with .gitmodules via this "cache" system need to use .gitmodules
> immediately after adding a new submodule but before committing that
> change to the superproject? Do they code something like this?
>
> if (cached)
> read .gitmodules from the index and fabricate
> struct submodule;
> else if (worktree)
> read .gitmodules from the working tree and fabricate
> struct submodule;
> else
> call submodule_from_name("HEAD", ...) and receive
> struct submodule;
>
> use the struct submodule to learn from the module;
>
> Yes, I am wondering if submodule_from_name() should be extended to
> allow the former two cases, so that the caller can make a single
> call above and then use resulting "struct submodule" throughout its
> code after doing so.
Reading from the worktree is supported by passing in null_sha1 (as
documented). That said it may also contain values from the local
configuration (overlaid in the typical priority).
The index case is the only one that is missing. I am not sure whether we
will need that. The use cases I have described above do not require this
(AFAICS). But you are right maybe it makes sense to keep this open so we
can at least allow it. Since we already use the NULL pointer and the
null_sha1 as special values we could either add another function pair
and a separate internal cache for them or define some special sha1 value
that is very unlike
Re: Suggestion: make git checkout safer
On 2015-06-03 11.55, Ed Avis wrote: > Jeff King peff.net> writes: > >> I would say the more "usual" way to use checkout like this >> is to give specific paths. I.e., run "git status", say "oh, I need to >> restore the contents of 'foo', but not 'bar'", and run "git checkout >> foo". That works regardless of the type of change to "foo" and "bar". > > That seems fine - a specific file is named and you clearly want to alter > the contents of that file. By analogy, 'rm foo' will silently delete it, > but if you specify a directory to delete recursively you need the -r flag. > OK, it's not a perfect analogy because the purpose of rm is to delete data > and nothing else ;-). > > If my personal experience is anything to go by, newcomers may fall into the > habit of running 'git checkout .' to restore missing files. In the old days > I would often delete a file and then run 'cvs update' or 'svn update' to > restore it. That would fetch a fresh copy from the repository, and while > it might do some kind of diff/patch operation on modified files, it would > not simply throw away local changes. > > 'git checkout .' seems like the analogous command, but it has much sharper > edges. I still think it should be safer by default, but if you decide > against that then perhaps you need to create some way to restore missing > files and not overwrite others. 'git checkout --no-overwrite'? Then it > could even be added to .gitconfig as the default for those who like it. > > I have to say that as a newcomer to git I do not like the idea of creating > a special undo log for git. It would just be yet another concept to learn > and another thing to add to the list of 'where is git hiding my data this > time?'. And the time when it would be useful - after some bungled operation > that lost data - is just the time when the user is already confused and > adding another semi-hidden stash of objects to the mix would befuddle them > further. If there is to be a backup made of local changes that get lost, > and I agree it is a good idea, then it should be something stupid and > completely obvious, such as saving the old file as 'foo.before_checkout.1'. > This is what my Git says: git status On branch master Changes not staged for commit: (use "git add/rm ..." to update what will be committed) (use "git checkout -- ..." to discard changes in working directory) modified: A deleted:B (So it should be somewhat self-documenting) I try to avoid things like "git reset --hard", and "git checkout .", and often use "git stash" instead. It may be that there is a chance to improve the documentation. Just for curiosity: >From where did you got the information to run "git checkout ." ? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Suggestion: make git checkout safer
On June 3, 2015 3:06 PM Jeff King wrote: > On Wed, Jun 03, 2015 at 10:32:40AM -0700, Junio C Hamano wrote: > > "git checkout $paths" (and you can give "." for $paths to mean > > "everything") is akin to "cp -R $elsewhere/$path ." to restore the > > working tree copies from somewhere else. > > > > "Ouch, 'git checkout .' overwrote what was in my working tree" is > > exactly the same kind of confusion as "I ran 'cp -r ../saved .' and it > > overwrote everything". As you said in your initial response, that is > > what the command is meant for. > > > > What does that similar command outside world, "cp", have for "more > > safety"? 'cp -i' asks if the user wants to overwrite a file for each > > path; perhaps a behaviour similar to that was the original poster > > wanted to see? > > Yeah, I'd say "cp -i" is the closest thing. I don't have a problem with > adding that, > but I'd really hate for it to be the default (just as I find distros which > "alias > rm='rm -i" annoying). Brainstorming a few compromises: or some such config option to turn on behaviour like this: core.checkout=-i or some such thing where if there are strictly more than m files being touched and strictly less than n files to act accordingly - a threshold concept: core.checkout_warn_upperlimit=n # default to 0 core.checkout_warn_lowerlimit=m # default to 0 or in a more gross fashion provide a pre-checkout hook to do all the work of prompting/control of the situation. Personally I'm happy with the defaults as they are (and was not a fan of defaulting rm -i or cp -i either) but I can see the point and have had diffuse whines from my team on the checkout subject, which is why I'm commenting. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFCv4 1/2] git-rebase -i: add command "drop" to remove a commit
Junio C Hamano writes: > But what if you got this back after the user edits? > > drop > pick 2c9c1c5 gostak: distim doshes > drop e3b601d pull: use git-rev-parse... > edit eb2a8d9 pull: handle git-fetch'... > > [...] > Did the user tried to drop something else but the > object name has gone missing by mistake? If the user tried to drop something but the SHA-1 has gone missing, it would be picked up by 2/2 if rebase.missingCommitsCheck is set to warn or error. > Did the user wanted to drop the first one but made mistake while > editing 'pick' away into 'drop'? I see your point here. > Noticing and flagging malformed 'drop' lines (or line with any > command, for that matter) as such is part of that process to make > sure you collected the object names from the "after" image > correctly, which is the job of 2/2 in your series (if I am reading > the description of your series right). I agree on the fact that noticing and flagging malformed lines for the various commands is a part of the process to make sure the collected object names are correct. However, the original job of 2/2 was to avoid the silent loss of information caused by deleting a line by mistake, not to check the correctness of the whole todo-file (though I think that it is a good idea, to develop in another commit of this patch/in another patch). On the opposite, in some way it could be seen as some loss of information, as your previous example suggests: > Did the user wanted to drop the first one but made mistake while > editing 'pick' away into 'drop'? We lose the information that the user wanted to drop the line and not pick it. Aside from that, checking specifically the 'drop' command beforehand (that's what happens in 2/2) while not doing any checking on the other commands (or checking on the fly) doesn't really makes sense, I think the commands should be treated equally on the matter of checking. In doing so, checking that the various commands exist, that they are followed by the correct argument and that their argument is of the right type (SHA-1 representing a commit for example), in top of checking that no commit is removed, without forgetting tests to make sure everything has the right behaviour, I am afraid that it would make this part of the patch far too long thus why I think it should be in another commit/patch. By the way > In the new world order to punish those who simply remove lines to > signal that they want the commits omitted from replaying Aside from liking the wording here, I don't think it can really be considered as a punishment since it is the user that chooses if he wants to be "punished" or not; I guess it can be considered BDSM though. Junio C Hamano writes: > > +drop|d) > > +if test -z $sha1 > > +then > > +warn "Missing SHA-1 in 'drop' command." > > +die "Please fix this using 'git rebase > > --edit-todo'." > > Is this a sensible piece of advice, though? The user edited the > line away and it does not have the commit object name anymore. > How does she "fix" it in the editor? The same puzzlement applies > to the other message. For a missing SHA-1, if it is a 'drop' that he forgot to remove after changing his mind, then it can be fixed. For an incorrect SHA-1, maybe he can find it back using git log or others commands of the kind, though I don't think this way of fixing things is user-friendly at all. However, as you point out, in most cases it will be a line edited away, I agree that the "fix it" doesn't really help. The only alternative I see right now (in 1/2) is aborting the rebase (die_abort) though it seems overkill, as the user I wouldn't want to lose all of the modifications I have done on the todo-list. Through this I see your point about checking in 2/2 since we would have more information (for example if he has an error because a drop has no SHA-1 or the SHA-1 is incorrect and at the same time an error because a SHA-1 has disappeared, it would make more sense to give him "fix it" since he would have most of the informations he needs to do so). However, right now, I think that my previous points about 2/2 still stand. Thanks, Rémi -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFCv2 2/2] git rebase -i: warn about removed commits
On Wednesday, June 3, 2015, Galan Rémi wrote: > Check if commits were removed (i.e. a line was deleted) and print > warnings or abort git rebase depending on the value of the > configuration variable rebase.missingCommits. A few comments below in addition to those already made by Matthieu... > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh > index 8960083..f369d2c 100755 > --- a/t/t3404-rebase-interactive.sh > +++ b/t/t3404-rebase-interactive.sh > @@ -1112,4 +1112,67 @@ test_expect_success 'drop' ' > test A = $(git cat-file commit HEAD^^ | sed -ne \$p) > ' > > +cat >expect < +Successfully rebased and updated refs/heads/tmp2. > +EOF > + > +test_expect_success 'rebase -i respects rebase.missingCommitsCheck=ignore' ' > + test_config rebase.missingCommitsCheck ignore && > + test_when_finished "git checkout master && > + git branch -D tmp2" && Strange indentation. > + git checkout -b tmp2 master && > + set_fake_editor && > + FAKE_LINES="1 2 3 4" \ > + git rebase -i --root 2>warning && The file containing the actual output is usually spelled "actual". > + test D = $(git cat-file commit HEAD | sed -ne \$p) && > + test_cmp warning expect The arguments to test_cmp are usually reversed so that 'expect' comes before 'actual', which results in a more natural-feeling diff when test_cmp detects that the files differ. These comments apply to remaining new tests, as well. > +' > + > +cat >expect < +Warning: some commits may have been dropped accidentally. > +Dropped commits (newer to older): > + - $(git rev-list --pretty=oneline --abbrev-commit -1 master) > + > +To avoid this message, use "drop" to explicitly remove a commit. > +Use git --config rebase.missingCommitsCheck to change the level of warnings > (ignore, warn, error). > + > +Successfully rebased and updated refs/heads/tmp2. > +EOF > + > +test_expect_success 'rebase -i respects rebase.missingCommitsCheck=warn' ' > + test_config rebase.missingCommitsCheck warn && > + test_when_finished "git checkout master && > + git branch -D tmp2" && > + git checkout -b tmp2 master && > + set_fake_editor && > + FAKE_LINES="1 2 3 4" \ > + git rebase -i --root 2>warning && > + test D = $(git cat-file commit HEAD | sed -ne \$p) && > + test_cmp warning expect > +' > + > +cat >expect < +Warning: some commits may have been dropped accidentally. > +Dropped commits (newer to older): > + - $(git rev-list --pretty=oneline --abbrev-commit -1 master) > + - $(git rev-list --pretty=oneline --abbrev-commit -1 master~2) > + > +To avoid this message, use "drop" to explicitly remove a commit. > +Use git --config rebase.missingCommitsCheck to change the level of warnings > (ignore, warn, error). > + > +Rebase aborted due to dropped commits. > +EOF > + > +test_expect_success 'rebase -i respects rebase.missingCommitsCheck=error' ' > + test_config rebase.missingCommitsCheck error && > + test_when_finished "git checkout master && > + git branch -D tmp2" && > + git checkout -b tmp2 master && > + set_fake_editor && > + test_must_fail env FAKE_LINES="1 2 4" \ > + git rebase -i --root 2>warning && > + test E = $(git cat-file commit HEAD | sed -ne \$p) && > + test_cmp warning expect > +' > + > test_done > -- > 2.4.2.389.geaf7ccf -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
On Wed, Jun 03, 2015 at 10:32:40AM -0700, Junio C Hamano wrote: > "git checkout $paths" (and you can give "." for $paths to mean > "everything") is akin to "cp -R $elsewhere/$path ." to restore the > working tree copies from somewhere else. > > "Ouch, 'git checkout .' overwrote what was in my working tree" is > exactly the same kind of confusion as "I ran 'cp -r ../saved .' and > it overwrote everything". As you said in your initial response, > that is what the command is meant for. > > What does that similar command outside world, "cp", have for "more > safety"? 'cp -i' asks if the user wants to overwrite a file for > each path; perhaps a behaviour similar to that was the original > poster wanted to see? Yeah, I'd say "cp -i" is the closest thing. I don't have a problem with adding that, but I'd really hate for it to be the default (just as I find distros which "alias rm='rm -i" annoying). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Stash Feature
> "Konstantin" == Konstantin Khomoutov writes:
Konstantin> On Wed, 3 Jun 2015 17:22:57 +0200
Konstantin> Fabrizio Mancin wrote:
>> I've a little request for you.
>> What about saving date-time on git stash save command and show it on
>> git stash show stash@{xxx}?
>> I think it is a useful poperty to save.
>>
>> What do you think about it?
Konstantin> This information is already there as a stash entry is
Konstantin> internally represented as a commit (with one or more
Konstantin> parents), and being a commit, it possess all the standard
Konstantin> attributes of a commit, including the creation timestamp.
git stash list can take a --format as well. Here's a couple of my
aliases:
sl = stash list --pretty='%gd: %h [%ar] %s'
stashed = stash list --pretty='%gd: %Cred%h%Creset %Cgreen[%ar]%Creset %s'
--
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
http://www.stonehenge.com/merlyn/>
Perl/Unix consulting, Technical writing, Comedy, etc. etc.
Still trying to think of something clever for the fourth line of this .sig
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] read_loose_refs(): treat NULL_SHA1 loose references as broken
Jeff King writes: > On Wed, Jun 03, 2015 at 03:51:59PM +0200, Michael Haggerty wrote: > >> NULL_SHA1 is used to indicate of "invalid SHA-1" throughout our code > > s/of/an/ ? Also possibly s/invalid SHA-1/invalid ref/? >> (and the code of other git implementations), so it is vastly more >> likely that a reference was set to this value due to a software bug >> than that NULL_SHA1 is the legitimate SHA-1 of an actual object. >> Therefore, if a loose reference has the value NULL_SHA1, consider it >> to be broken. >> >> Amusingly, each of the other 2^160-1 possible SHA-1 values is exactly >> as unlikely as NULL_SHA1 to be the SHA-1 of an actual object. The >> difference is that most of those other values are also very unlikely >> to be written to a loose reference file by accident, whereas >> accidentally writing NULL_SHA1 to a loose reference file would be an >> easy mistake to make. > > FWIW, I think this justification (and the comment below) reads better > than what you had before. I agree, and further I think the second paragraph is redundant and unnecessary. If you update "... likely that a reference was set to this value" to clarify that the "reference" it talks about is an on-disk entity, not the in-core representation (which can legitimately have NULL_SHA1 to signal "invalid ref", it would be sufficient. I.e. ... so it is vastly more likely that an on-disk reference was set to this value due to a bug ... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/2] status: fix tests to handle new verbose git status during rebase
Matthieu Moy writes: > Guillaume Pagès writes: > >> Signed-off-by: Guillaume Pagès >> --- >> t/t7512-status-help.sh | 227 >> ++--- > > Your history is not bisectable if you sparate tests and code changes in > two patches. Yes. And by squashing the two, you do not have to label 2/2 incorrectly as "fix tests"; it is merely fixing what 1/2 broke ;-) > Plus, as a reviewer, I like seeing changes to the tests next to changes > to the code, to show me what is the impact of the code change on the > output of the program. That too. is a very important consideration, maybe even more so than bisectability. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug "$message" literal in commit message
Yauheni Zablotski writes: > Hello, > > I think I found a bug(or strange behavior) in the git. > If commit message contains literal "$message" than that literal > disappears from commit message. > > For example: > - > user@comp ~/cc $ git commit -am "1$message1" Not a Git issue, but a user-error that Git cannot recover. Your shell is doing the variable expansion before calling git, and $message1 is considered as a shell variable here. Git does not know that you used $message1. Solution: git commit -am '1$message1' or git commit -am "1\$message1" -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Suggestion: make git checkout safer
On June 3, 2015 2:11 PM Junio C Hamano wrote: > "Randall S. Becker" writes: > > On June 3, 2015 1:35 PM Junio C Hamano wrote: > >> Is that really true? It all depends on why you came to a situation > >> to have "missing files" in the first place, I would think, but "git > >> checkout $path" is "I messed up the version in the working tree at > >> $path, and want to restore them". One particular kind of "I messed > >> up" may be "I deleted by mistake" > >> (hence making them "missing"), but is it so common to delete things > >> by mistake, as opposed to editing, making a mess and realizing that > >> the work so far was not improving things and wanting to restart from > >> scratch? > > > > When working in an IDE like ECLIPSE or MonoDevelop, accidentally > > hitting the DEL button or a drag-drop move is a fairly common trigger > > for the "Wait-No-Stop-Oh-Drats" process which includes running git > > checkout to recover. > > That is an interesting tangent. If you are lucky then the deleted file may be > unedited one, but I presume that you are not always lucky. So perhaps "git > checkout" is not a solution to that particular IDE issue in the first place? Agreed. That's why I like knowing what's in my sausages and commit often. Only lost a minor change once from this. I wonder what else is afoot. Ed, can you expand on the issue? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug "$message" literal in commit message
Yauheni Zablotski writes: > Hello, > > I think I found a bug(or strange behavior) in the git. > If commit message contains literal "$message" than that literal > disappears from commit message. > > For example: > - > user@comp ~/cc $ git commit -am "1$message1" > [master (root-commit) d36a841] 1 > 1 file changed, 0 insertions(+), 0 deletions(-) > create mode 100644 1 > > user@comp ~/cc $ git log > commit d36a841ae25510ada80246a78225446083fcb3e1 > Author: user > Date: Wed Jun 3 18:21:45 2015 +0200 > > file > > > Sorry for having disturbed you Learn shell ;-) Instead of "git commit -am", try "echo" and repeat your exercise, and you would see: $ echo "1$message1" 1 If you prepare a shell variable message1 beforehand, e.g. $ message1='This is the contents of message1 variable' $ echo "1$message1" 1This is the contents of message1 variable Your shell interpolates the value of message1 variable if you write "$message1" on your command line, way before individual commands (e.g. echo and git above) that receive the string as its parameter sees them. Contrast the above with this invocation after understanding the above. $ git commit -a -m '1$message1' $ git log -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
Maybe the expectation comes from the existing warnings when checking out branches? $ mkdir tmp && cd tmp $ git init $ echo Hello >foo $ git add foo $ git commit -am "Hello" $ git branch next $ echo "world" >bar $ git add bar $ git commit -a -m "World" $ git checkout test # no problem so far, just going back one commit on anther branch $ echo Kitty >bar $ git checkout master # now we get it: error: The following untracked working tree files would be overwritten by checkout: bar Please move or remove them before you can switch branches. Aborting So in one mode, we do actually warn about contents going missing, and the other mode is designed to actually make things go missing without any warning. So maybe the checkout command is *too powerful* ? Looking at the man page: Updates files in the working tree to match the version in the index or the specified tree. If no paths are given, git checkout will also update HEAD to set the specified branch as the current branch. we're mixing two different tasks here anyway. "Updating files in the work tree" can be understood as "throwing away all changes until you're back at a specified safe point". If I were to come up with a name for such an action it's maybe "reset" or "reset-file(s)". Though git reset is taken already and does different things. "reset" sounds as if stuff may go missing, so anyone who types "reset", (even without exactly understanding what it does, would assume it is as safe as typing "rm" probably. And "also update HEAD" can be understood as "switch to another branch", so if I were to invent a new porcelain command for such functionality it may be called "git switch-branch". And typing switch-branch would be expected to carry all the warnings (no updating files in the work tree, when in danger of losing its content) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
"Randall S. Becker" writes: > On June 3, 2015 1:35 PM Junio C Hamano wrote: >> Is that really true? It all depends on why you came to a >> situation to have "missing files" in the first place, I would >> think, but "git checkout $path" is "I messed up the version in >> the working tree at $path, and want to restore them". One >> particular kind of "I messed up" may be "I deleted by mistake" >> (hence making them "missing"), but is it so common to delete >> things by mistake, as opposed to editing, making a mess and >> realizing that the work so far was not improving things and >> wanting to restart from scratch? > > When working in an IDE like ECLIPSE or MonoDevelop, accidentally > hitting the DEL button or a drag-drop move is a fairly common > trigger for the "Wait-No-Stop-Oh-Drats" process which includes > running git checkout to recover. That is an interesting tangent. If you are lucky then the deleted file may be unedited one, but I presume that you are not always lucky. So perhaps "git checkout" is not a solution to that particular IDE issue in the first place? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFCv4 2/2] git rebase -i: warn about removed commits
Remi Galan Alfonso writes: > Matthieu Moy writes: >> You're using $1 and $2 only to redirect input and output. I would find >> it more elegant to write todo_list_to_sha_list as a filter, and do the >> redirection in the call site (to keep the option of using >> todo_list_to_sha_list in a pipe). > > If I understood correctly, then the calling line would look like: >> todo_list_to_sha_list <"$todo".backup >"$todo".oldsha1 > ? Yes. > Should I do the same for the function warn_file ? > Meaning >> warn_file <"$todo".miss > instead of >> warn_file "$todo".miss If you do so, then warn_file won't be a good name anymore since you'd be able to read from a pipe too. Anyway, it's really a matter of personnal preference. I would keep warn_file as-is and change todo_list_to_sha_list, but feel free to ignore this point if you disagree. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFCv4 1/2] git-rebase -i: add command "drop" to remove a commit
Galan Rémi writes:
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index dc3133f..869cc60 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -152,6 +152,7 @@ Commands:
> s, squash = use commit, but meld into previous commit
> f, fixup = like "squash", but discard this commit's log message
> x, exec = run command (the rest of the line) using shell
> + d, drop = remove commit
>
> These lines can be re-ordered; they are executed from top to bottom.
>
> @@ -508,6 +509,23 @@ do_next () {
> "$comment_char"*|''|noop)
> mark_action_done
> ;;
> + drop|d)
> + if test -z $sha1
> + then
> + warn "Missing SHA-1 in 'drop' command."
> + die "Please fix this using 'git rebase --edit-todo'."
Is this a sensible piece of advice, though? The user edited the
line away and it does not have the commit object name anymore.
How does she "fix" it in the editor? The same puzzlement applies
to the other message.
> + sha1_verif="$(git rev-parse --verify --quiet $sha1^{commit})"
> + if test -z $sha1_verif
As you are not using the returned string at all, instead of wasting
a shell variable, it would be better to decide to come into this
block by using the exit status from rev-parse (it exits with
non-zero status upon failure).
> + then
> + warn "'$sha1' is not a SHA-1 or does not represent" \
> + "a commit in 'drop' command."
> + die "Please fix this using 'git rebase --edit-todo'."
> + fi
> +
> + mark_action_done
> + ;;
> pick|p)
> comment_for_reflog pick
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFCv4 2/2] git rebase -i: warn about removed commits
Matthieu Moy writes: > You're using $1 and $2 only to redirect input and output. I would find > it more elegant to write todo_list_to_sha_list as a filter, and do the > redirection in the call site (to keep the option of using > todo_list_to_sha_list in a pipe). If I understood correctly, then the calling line would look like: > todo_list_to_sha_list <"$todo".backup >"$todo".oldsha1 ? Should I do the same for the function warn_file ? Meaning > warn_file <"$todo".miss instead of > warn_file "$todo".miss The other points mentioned have been corrected. Thanks, Rémi -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFCv2 1/2] git-rebase -i: add command "drop" to remove a commit
Remi Galan Alfonso writes: > Junio C Hamano writes: >> As long as what is given to 'drop' >> is checked when it matters (e.g. when the code in patch 2/2 tries >> see if some commits in the original list are no longer there in >> order to warn sees "drop foo bar" where "foo" is obviously not an >> object name in the original list, that should be checked), it is >> fine. And I agree 1/2 is not the place to do so, even though it may >> be easier from the implementation point of view (which is why I >> mentioned the possibility in the review of that patch). > > I disagree, I think that that either the checking for the 'drop' > command should either be in the 1/2 where it is introduced or in the > function check_commits introduced by 2/2 but in a separate > commit/patch. > The 2/2 checks if there are removed commits to have the possibility to > avoid silent loss of information. It is not its role to check if the > SHA-1 following 'drop' are correct. Suppose you started from this insn sheet: pick 2c9c1c5 gostak: distim doshes pick e3b601d pull: use git-rev-parse... pick eb2a8d9 pull: handle git-fetch'... and then after letting the user edit, you got this back: pick 2c9c1c5 gostak: distim doshes drop e3b601d pull: use git-rev-parse... edit eb2a8d9 pull: handle git-fetch'... In the new world order to punish those who simply remove lines to signal that they want the commits omitted from replaying, you would want to see all commit object names that was in the original insn sheet appear in the post-edit insn sheet. I'd presume that the way to do so is to collect all the object names from each insn sheet and compute the set difference. The first one has three commit object names, the same three commit object names appear in the second one, and all is well. But what if you got this back after the user edits? drop pick 2c9c1c5 gostak: distim doshes drop e3b601d pull: use git-rev-parse... edit eb2a8d9 pull: handle git-fetch'... As a part of "collecting object names from the list before and after editing into two separate sets, and computing the set difference in order to notice potential mistakes", you would need to make sure that you got these two sets collected _correctly_, but you do not know from the above sample input what the user wanted to do with the first line. Did the user tried to drop something else but the object name has gone missing by mistake? Did the user wanted to drop the first one but made mistake while editing 'pick' away into 'drop'? Noticing and flagging malformed 'drop' lines (or line with any command, for that matter) as such is part of that process to make sure you collected the object names from the "after" image correctly, which is the job of 2/2 in your series (if I am reading the description of your series right). So logically I would think 2/2 is where the verification should happen, but doing it as a part of 1/2 may be easier to do. The end result would not make a difference, and that is why I said it would be OK either way. I am puzzled as to what you are disagreeing with, and why. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Suggestion: make git checkout safer
On June 3, 2015 1:35 PM Junio C Hamano wrote: > Ed Avis writes: > > If my personal experience is anything to go by, newcomers may fall > > into the habit of running 'git checkout .' to restore missing files. > Is that really true? It all depends on why you came to a situation to have > "missing files" in the first place, I would think, but "git checkout $path" is "I > messed up the version in the working tree at $path, and want to restore them". > One particular kind of "I messed up" may be "I deleted by mistake" (hence > making them "missing"), but is it so common to delete things by mistake, as > opposed to editing, making a mess and realizing that the work so far was not > improving things and wanting to restart from scratch? When working in an IDE like ECLIPSE or MonoDevelop, accidentally hitting the DEL button or a drag-drop move is a fairly common trigger for the "Wait-No-Stop-Oh-Drats" process which includes running git checkout to recover. My keyboard is excessively sensitive static, so this happens more often than I will admit (shamelessly blaming hardware when it really is a user problem). Git checkout is a life-saver in this case as is frequently committing. :) Cheers, Randall -- Brief whoami: NonStop&UNIX developer since approximately UNIX(421664400)/NonStop(2112884442) -- In my real life, I talk too much. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 19/19] pull: remove redirection to git-pull.sh
On Tue, Jun 2, 2015 at 11:49 PM, Paul Tan wrote:
> At the beginning of the rewrite of git-pull.sh to C, we introduced a
> redirection to git-pull.sh if the environment variable
> _GIT_USE_BUILTIN_PULL was not defined in order to not break test scripts
> that relied on a functional git-pull.
>
> Now that all of git-pull's functionality has been re-implemented in
> builtin/pull.c, remove this redirection, and retire the old git-pull.sh
> into contrib/examples/.
>
> Signed-off-by: Paul Tan
The whole series was a pleasant read.
Thanks,
Stefan
> ---
> Makefile| 1 -
> builtin/pull.c | 7 ---
> git-pull.sh => contrib/examples/git-pull.sh | 0
> 3 files changed, 8 deletions(-)
> rename git-pull.sh => contrib/examples/git-pull.sh (100%)
>
> diff --git a/Makefile b/Makefile
> index 2057a9d..67cef1c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -474,7 +474,6 @@ SCRIPT_SH += git-merge-octopus.sh
> SCRIPT_SH += git-merge-one-file.sh
> SCRIPT_SH += git-merge-resolve.sh
> SCRIPT_SH += git-mergetool.sh
> -SCRIPT_SH += git-pull.sh
> SCRIPT_SH += git-quiltimport.sh
> SCRIPT_SH += git-rebase.sh
> SCRIPT_SH += git-remote-testgit.sh
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 4e1ab5b..dad49cf 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -805,13 +805,6 @@ int cmd_pull(int argc, const char **argv, const char
> *prefix)
> unsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ];
> unsigned char rebase_fork_point[GIT_SHA1_RAWSZ];
>
> - if (!getenv("_GIT_USE_BUILTIN_PULL")) {
> - const char *path = mkpath("%s/git-pull", git_exec_path());
> -
> - if (sane_execvp(path, (char**) argv) < 0)
> - die_errno("could not exec %s", path);
> - }
> -
> if (!getenv("GIT_REFLOG_ACTION"))
> set_reflog_message(argc, argv);
>
> diff --git a/git-pull.sh b/contrib/examples/git-pull.sh
> similarity index 100%
> rename from git-pull.sh
> rename to contrib/examples/git-pull.sh
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/2] status: fix tests to handle new verbose git status during rebase
Guillaume Pagès writes: > Signed-off-by: Guillaume Pagès > --- > t/t7512-status-help.sh | 227 > ++--- Your history is not bisectable if you sparate tests and code changes in two patches. Plus, as a reviewer, I like seeing changes to the tests next to changes to the code, to show me what is the impact of the code change on the output of the program. In this case, this patch really shows what your code is doing, and reviewing PATCH 2 means checking that we agree on the specification, which should not be done _after_ checking the code. + (use git rebase --edit-todo to view and edit) All other status hints use double-quotes around commands. I think this one should, too. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFCv4 2/2] git rebase -i: warn about removed commits
Galan Rémi writes:
> Check if commits were removed (i.e. a line was deleted) and print
> warnings or abort git rebase depending on the value of the
> configuration variable rebase.missingCommits.
Spelled missingCommitsCheck everywhere else.
> +rebase.missingCommitsCheck::
> + If set to "warn" print warnings about removed commits in
If set to warn, print (comma). Here and below
> + interactive mode. If set to "error" print the warnings and
> + abort the rebase. If set to "ignore" no checking is
> + done. "ignore" by default.
> +# Print the list of the SHA-1 of the commits
> +# from a todo list in a file.
> +# $1: todo-file, $2: outfile
> +todo_list_to_sha_list () {
> + git stripspace --strip-comments <"$1" | while read -r command sha1 rest
I'd split the line in two:
git stripspace --strip-comments <"$1" |
while read -r command sha1 rest
> + do
> + case $command in
> + x|"exec")
> + ;;
> + *)
> + printf "%s\n" "$sha1"
> + ;;
> + esac
> + done >"$2"
> +}
You're using $1 and $2 only to redirect input and output. I would find
it more elegant to write todo_list_to_sha_list as a filter, and do the
redirection in the call site (to keep the option of using
todo_list_to_sha_list in a pipe).
> +check_commits
You're not really checking commits, but rather the todo-list.
> +Use git --config rebase.missingCommitsCheck to change the level of warnings
> (ignore, warn, error).
Long line. Please, wrap the message to <80 columns.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 18/19] pull --rebase: error on no merge candidate cases
On Tue, Jun 2, 2015 at 11:49 PM, Paul Tan wrote:
> Tweak the error messages printed by die_no_merge_candidates() to take
> into account that we may be "rebasing against" rather than "merging
> with".
>
> Signed-off-by: Paul Tan
> ---
>
> Notes:
> v2
>
> * Decided to use fprintf_ln() for the sake of code consistency, and for
> the added trailing newline.
>
> builtin/pull.c | 15 ---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index f5d437a..4e1ab5b 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -439,7 +439,10 @@ static void NORETURN die_no_merge_candidates(const char
> *repo, const char **refs
> const char *remote = curr_branch ? curr_branch->remote_name : NULL;
>
> if (*refspecs) {
> - fprintf_ln(stderr, _("There are no candidates for merging
> among the refs that you just fetched."));
> + if (opt_rebase)
> + fprintf_ln(stderr, _("There is no candidate for
> rebasing against among the refs that you just fetched."));
> + else
> + fprintf_ln(stderr, _("There are no candidates for
> merging among the refs that you just fetched."));
> fprintf_ln(stderr, _("Generally this means that you provided
> a wildcard refspec which had no\n"
> "matches on the remote end."));
> } else if (repo && curr_branch && (!remote || strcmp(repo, remote))) {
> @@ -449,7 +452,10 @@ static void NORETURN die_no_merge_candidates(const char
> *repo, const char **refs
> repo);
> } else if (!curr_branch) {
> fprintf_ln(stderr, _("You are not currently on a branch."));
> - fprintf_ln(stderr, _("Please specify which branch you want to
> merge with."));
> + if (opt_rebase)
> + fprintf_ln(stderr, _("Please specify which branch you
> want to rebase against."));
> + else
> + fprintf_ln(stderr, _("Please specify which branch you
> want to merge with."));
Now that you're using fprintf you could make use of its formatting
capabilities, but then it occurred to me
it's translated strings, so it's most likely better to not make it
concise but rather easier for the translators
by having each sentence written out in full.
> fprintf_ln(stderr, _("See git-pull(1) for details."));
> fprintf(stderr, "\n");
> fprintf_ln(stderr, "git pull ");
> @@ -461,7 +467,10 @@ static void NORETURN die_no_merge_candidates(const char
> *repo, const char **refs
> remote_name = "";
>
> fprintf_ln(stderr, _("There is no tracking information for
> the current branch."));
> - fprintf_ln(stderr, _("Please specify which branch you want to
> merge with."));
> + if (opt_rebase)
> + fprintf_ln(stderr, _("Please specify which branch you
> want to rebase against."));
> + else
> + fprintf_ln(stderr, _("Please specify which branch you
> want to merge with."));
> fprintf_ln(stderr, _("See git-pull(1) for details."));
> fprintf(stderr, "\n");
> fprintf_ln(stderr, "git pull ");
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
Ed Avis writes: > If my personal experience is anything to go by, newcomers may fall into the > habit of running 'git checkout .' to restore missing files. Is that really true? It all depends on why you came to a situation to have "missing files" in the first place, I would think, but "git checkout $path" is "I messed up the version in the working tree at $path, and want to restore them". One particular kind of "I messed up" may be "I deleted by mistake" (hence making them "missing"), but is it so common to delete things by mistake, as opposed to editing, making a mess and realizing that the work so far was not improving things and wanting to restart from scratch? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation
Matthieu Moy writes > Most git-*.txt do not have this CONFIGURATION section. > > In an ideal world, we would have such section automatically generated > (i.e. the description for each variable would exist in one place, and we > would make sure that both "man git-config" and "man git-" show > it). In a really ideal world, it would also be propagated to the code > and we would have a "git config --describe am.keepcr" or so that would > return the doc. > > I'm a bit worried to see documentation cut-and-pasted from config.txt to > git-*.txt for maintainability: if the text on one side is modified, > we're likely to forget the other and the text will diverge with time. > > Not a strong objection, but I have the feeling that the more we do this > kind of patches, the harder it will be if ever we decide to do the above. I've seen occurences of this (mainly git-rebase.txt and git-grep), but I agree, I think I'll remove the configuration section. > > +CONFIGURATION > > +- > > + > > +am.keepcr:: > > +If true, git-am will call git-mailsplit for patches in mbox format > > `git am` > `git mailsplit` > > > +with parameter '--keep-cr'. In this case git-mailsplit will > > Likewise > > > +not remove `\r` from lines ending with `\r\n`. Can be overridden > > +by giving '--no-keep-cr' from the command line. > > That should be backquote, not forward-quote, right? > > I know it's not your code since it's a cut-and-paste from config.txt, > but that illustrates my point above: we used to have one place with > wrong quotes, and we'd have two after the patch. Ok I'll correct it in a minor patch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
Jeff King writes: > If we want to introduce more safety here, I'd be inclined to perform the > operation by default, but give a better escape hatch. For example, by > creating a loose object for any file we're about to overwrite, and > possibly writing an entry into a log. Can we borrow the ideas from other tools that have similar characteristics, I wonder. "git checkout $paths" (and you can give "." for $paths to mean "everything") is akin to "cp -R $elsewhere/$path ." to restore the working tree copies from somewhere else. "Ouch, 'git checkout .' overwrote what was in my working tree" is exactly the same kind of confusion as "I ran 'cp -r ../saved .' and it overwrote everything". As you said in your initial response, that is what the command is meant for. What does that similar command outside world, "cp", have for "more safety"? 'cp -i' asks if the user wants to overwrite a file for each path; perhaps a behaviour similar to that was the original poster wanted to see? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] log: diagnose empty HEAD more clearly
Jeff King writes:
> My concern there would be risk of regression. I.e., that we would take
> some case which used to error out and turn it into a silent noop. So I'd
> prefer to keep the behavior the same, and just modify the error code
> path. The end result is pretty similar to the user, because we obviously
> cannot produce any actual output either way.
Okay.
> Something like:
>
> -- >8 --
> Subject: log: diagnose empty HEAD more clearly
>
> If you init or clone an empty repository, the initial
> message from running "git log" is not very friendly:
>
> $ git init
> Initialized empty Git repository in /home/peff/foo/.git/
> $ git log
> fatal: bad default revision 'HEAD'
>
> Let's detect this situation and write a more friendly
> message:
>
> $ git log
> fatal: default revision 'HEAD' points to an unborn branch 'master'
>
> We also detect the case that 'HEAD' points to a broken ref;
> this should be even less common, but is easy to see. Note
> that we do not diagnose all possible cases. We rely on
> resolve_ref, which means we do not get information about
> complex cases. E.g., "--default master" would use dwim_ref
> to find "refs/heads/master", but we notice only that
> "master" does not exist. Similarly, a complex sha1
> expression like "--default HEAD^2" will not resolve as a
> ref.
>
> But that's OK. We fall back to the existing error message in
> those cases, and they are unlikely to be used anyway.
> Catching an empty or broken "HEAD" improves the common case,
> and the other cases are not regressed.
>
> Signed-off-by: Jeff King
> ---
> I'm not a big fan of the new error message, either, just because the
> term "unborn" is also likely to be confusing to new users.
>
> We can also probably get rid of mentioning "HEAD" completely. It is
> always the default unless the user has overridden it explicitly.
I think that still mentioning "HEAD" goes directly against the
reasoning you made in an earlier part of your message:
> I think it is one of those cases where the message makes sense when you
> know how git works. But a new user who does not even know what "HEAD" or
> a ref actually is may find it a bit confusing. Saying "we can't run
> git-log, your repository empty!" may seem redundantly obvious even to
> such a new user. But saying "bad revision 'HEAD'" may leave them saying
> "eh, what is HEAD and why is it bad?".
and I agree with that reasoning: the user didn't say anything about
"HEAD", so why complain about it?
Which is what led me to say "Why are we defaulting to HEAD before
checking if it even exists? Isn't that the root cause of this
confusion? What happens if we stopped doing it?"
And I think the "diagnose after finding that pending is empty and we
were given 'def'" approach almost gives us the equivalent, which is
why I said "Okay" above.
If we can make it possible to tell if that "def" was given by the
user (i.e. --default parameter) or by the machinery (e.g. "git log"
and friends), then we can remove "almost" from the above sentence.
> So we
> could go with something like:
>
> fatal: your default branch 'master' does not contain any commits
>
> or something. I dunno. Bikeshed colors welcome. Adding:
>
> advise("try running 'git commit'");
>
> is probably too pedantic. ;)
;-)
I am wondering if the attached is better, if only because it is of
less impact. I have die() there to avoid the behaviour change, but
if we are going to have another future version where we are willing
to take incompatiblity for better end-user experience like we did at
2.0, I suspect turning it to warning() or even removing it might be
a candidate for such a version. If you have one commit and ask
"log", you get one commit back. If you have no commit and ask "log",
I think it is OK to say that you should get nothing back without
fuss.
builtin/log.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 4c4e6be..3b568a1 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -148,6 +148,9 @@ static void cmd_log_init_finish(int argc, const char
**argv, const char *prefix,
rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
argc = setup_revisions(argc, argv, rev, opt);
+ if (!rev->pending.nr && !opt->def)
+ die("you do not have a commit yet on your branch");
+
/* Any arguments at this point are not recognized */
if (argc > 1)
die(_("unrecognized argument: %s"), argv[1]);
@@ -404,6 +407,14 @@ static int git_log_config(const char *var, const char
*value, void *cb)
return git_diff_ui_config(var, value, cb);
}
+static void default_to_head_if_exists(struct setup_revision_opt *opt)
+{
+ unsigned char unused[20];
+
+ if (resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, unused, NULL))
+ opt->def = "HEAD";
+}
+
int cmd_whatchanged(int argc, const char **argv, const char *prefix)
{
struct rev_info rev;
@@ -4
Re: [PATCH v2 08/19] pull: pass git-fetch's options to git-fetch
On Tue, Jun 2, 2015 at 11:48 PM, Paul Tan wrote:
> Since eb2a8d9 (pull: handle git-fetch's options as well, 2015-06-02),
> git-pull knows about and handles git-fetch's options, passing them to
> git-fetch. Re-implement this behavior.
>
> Since 29609e6 (pull: do nothing on --dry-run, 2010-05-25) git-pull
> supported the --dry-run option, exiting after git-fetch if --dry-run is
> set. Re-implement this behavior.
>
> Signed-off-by: Paul Tan
> ---
>
> Notes:
> v2
>
> * Use parse_opt_parse_strbuf()
>
> builtin/pull.c | 95
> ++
> 1 file changed, 95 insertions(+)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 5f08634..0b66b43 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -32,6 +32,21 @@ static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
> static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
> static struct strbuf opt_gpg_sign = STRBUF_INIT;
>
> +/* Options passed to git-fetch */
> +static struct strbuf opt_all = STRBUF_INIT;
> +static struct strbuf opt_append = STRBUF_INIT;
> +static struct strbuf opt_upload_pack = STRBUF_INIT;
> +static int opt_force;
> +static struct strbuf opt_tags = STRBUF_INIT;
> +static struct strbuf opt_prune = STRBUF_INIT;
> +static struct strbuf opt_recurse_submodules = STRBUF_INIT;
> +static int opt_dry_run;
> +static struct strbuf opt_keep = STRBUF_INIT;
> +static struct strbuf opt_depth = STRBUF_INIT;
> +static struct strbuf opt_unshallow = STRBUF_INIT;
> +static struct strbuf opt_update_shallow = STRBUF_INIT;
> +static struct strbuf opt_refmap = STRBUF_INIT;
> +
> static struct option pull_options[] = {
> /* Shared options */
> OPT__VERBOSITY(&opt_verbosity),
> @@ -82,6 +97,46 @@ static struct option pull_options[] = {
> N_("GPG sign commit"),
> PARSE_OPT_OPTARG, parse_opt_pass_strbuf },
>
> + /* Options passed to git-fetch */
> + OPT_GROUP(N_("Options related to fetching")),
> + { OPTION_CALLBACK, 0, "all", &opt_all, 0,
> + N_("fetch from all remotes"),
> + PARSE_OPT_NOARG, parse_opt_pass_strbuf },
> + { OPTION_CALLBACK, 'a', "append", &opt_append, 0,
> + N_("append to .git/FETCH_HEAD instead of overwriting"),
> + PARSE_OPT_NOARG, parse_opt_pass_strbuf },
> + { OPTION_CALLBACK, 0, "upload-pack", &opt_upload_pack, N_("path"),
> + N_("path to upload pack on remote end"),
> + 0, parse_opt_pass_strbuf },
> + OPT__FORCE(&opt_force, N_("force overwrite of local branch")),
> + { OPTION_CALLBACK, 't', "tags", &opt_tags, 0,
> + N_("fetch all tags and associated objects"),
> + PARSE_OPT_NOARG, parse_opt_pass_strbuf },
> + { OPTION_CALLBACK, 'p', "prune", &opt_prune, 0,
> + N_("prune remote-tracking branches no longer on remote"),
> + PARSE_OPT_NOARG, parse_opt_pass_strbuf },
> + { OPTION_CALLBACK, 0, "recurse-submodules", &opt_recurse_submodules,
> + N_("on-demand"),
> + N_("control recursive fetching of submodules"),
> + PARSE_OPT_OPTARG, parse_opt_pass_strbuf },
> + OPT_BOOL(0, "dry-run", &opt_dry_run,
> + N_("dry run")),
> + { OPTION_CALLBACK, 'k', "keep", &opt_keep, 0,
> + N_("keep downloaded pack"),
> + PARSE_OPT_NOARG, parse_opt_pass_strbuf },
> + { OPTION_CALLBACK, 0, "depth", &opt_depth, N_("depth"),
> + N_("deepen history of shallow clone"),
> + 0, parse_opt_pass_strbuf },
> + { OPTION_CALLBACK, 0, "unshallow", &opt_unshallow, 0,
> + N_("convert to a complete repository"),
> + PARSE_OPT_NONEG | PARSE_OPT_NOARG, parse_opt_pass_strbuf },
> + { OPTION_CALLBACK, 0, "update-shallow", &opt_update_shallow, 0,
> + N_("accept refs that update .git/shallow"),
> + PARSE_OPT_NOARG, parse_opt_pass_strbuf },
> + { OPTION_CALLBACK, 0, "refmap", &opt_refmap, N_("refmap"),
> + N_("specify fetch refmap"),
> + PARSE_OPT_NONEG, parse_opt_pass_strbuf },
> +
> OPT_END()
> };
>
> @@ -100,6 +155,16 @@ static void argv_push_verbosity(struct argv_array *arr)
> }
>
> /**
> + * Pushes "-f" switches into arr to match the opt_force level.
> + */
> +static void argv_push_force(struct argv_array *arr)
> +{
> + int force = opt_force;
> + while (force-- > 0)
This made me chuckle despite the formatting,
as we referred to it as the limes operator in school for fun
#define limes while
limes (n --> 0) { // n goes towards zero
...
}
A quick
grep -r "--" -- *.c |grep while
shows we actually use this quite a lot, though the "> 0" is
omitted quite often.
> + argv_array_push(arr, "-f");
> +}
> +
> +/**
> * Parses argv into [ [...]], returning their values in
> `repo`
> * as a string and `refspecs` as a null-terminated array of strings. If
> `repo`
> * is not provided in argv, it is set to NULL.
> @@ -131,6 +196,33 @@ static int
Re: [RFC/PATCH 1/2] status: give more information during rebase -i
Guillaume Pagès writes: > + if (!fclose(fp)) > + strbuf_detach(&buf, NULL); > + else > + strbuf_release(&buf); Why these two cases? Aren't you leaking the buffer when calling strbuf_detach and ignoring its return value? (In general, there's not much to do when fclose fails actually) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/2] status: give more information during rebase -i
Hi,
Your code is not C90:
wt-status.c: In function ‘get_two_last_lines’:
wt-status.c:1030:2: error: ISO C90 forbids mixed declarations and code
[-Werror=declaration-after-statement]
struct strbuf buf = STRBUF_INIT;
^
wt-status.c: In function ‘get_two_first_lines’:
wt-status.c:1050:2: error: ISO C90 forbids mixed declarations and code
[-Werror=declaration-after-statement]
struct strbuf buf = STRBUF_INIT;
^
I have this to notice it:
$ cat config.mak
CFLAGS += -Wdeclaration-after-statement -Wall -Werror #-O0 -g
Guillaume Pagès writes:
> +void get_two_last_lines(char *filename,int * numlines, char ** lines)
Space after ,
> +{
> + *numlines=0;
Spaces around =
> + struct strbuf buf = STRBUF_INIT;
> + FILE *fp = fopen(git_path("%s", filename), "r");
> + if (!fp) {
> + strbuf_release(&buf);
> + return;
> + }
> + while (strbuf_getline(&buf, fp, '\n')!=EOF){
Spaces around !=
There are many other space issues, I won't list them all.
> + (*numlines)++;
> + lines[0]=lines[1];
> + lines[1]=strbuf_detach(&buf, NULL);
> + }
> + if (!fclose(fp))
> + strbuf_detach(&buf, NULL);
> + else
> + strbuf_release(&buf);
> +}
> +
> +void get_two_first_lines(char *filename,int * numlines, char ** lines)
Stick the * to the corresponding variable.
Do: int *numlines
Don't: int * numlines, int* numlines
One rationale is that when you write
int *i, j;
it reads "*i is an int, and so is j" (which is right), while
int * i, j;
may read as "both i and j are int *" (which is not right).
> +{
> + *numlines=0;
> + struct strbuf buf = STRBUF_INIT;
> + char *line;
> + FILE *fp = fopen(git_path("%s", filename), "r");
> + if (!fp) {
> + strbuf_release(&buf);
> + return;
> + }
> + while (strbuf_getline(&buf, fp, '\n')!=EOF){
> + stripspace(&buf, 1);
> + line = strbuf_detach(&buf, NULL);
> + if (strcmp(line,"")==0)
> + continue;
> + if (*numlines<2)
> + lines[*numlines] = line;
> + (*numlines)++;
> + }
> + if (!fclose(fp))
> + strbuf_detach(&buf, NULL);
> + else
> + strbuf_release(&buf);
> +}
> +
> +void show_rebase_information(struct wt_status *s,
> + struct wt_status_state *state,
> + const char *color)
static?
> +{
> + if (state->rebase_interactive_in_progress){
> + int i, begin;
> + int numlines =0;
> + char* lines[2];
> + get_two_last_lines("rebase-merge/done", &numlines, lines);
> + if (numlines==0)
> + status_printf_ln(s,color,"No command done.");
> + else{
Space before {. Several instances too.
> + status_printf_ln(s,color,
> + "Last command(s) done (%d command(s) done):",
> + numlines);
> + begin = numlines > 1? 0 : 1;
> + for (i = begin; i < 2; i++){
> + status_printf_ln(s,color," %s",lines[i]);
> + }
> + if (numlines>2 && s->hints )
> +status_printf_ln(s,color,
> + " (see more at .git/rebase-merge/done)");
> + }
> + numlines =0;
> + get_two_first_lines("rebase-merge/git-rebase-todo",
> + &numlines, lines);
> + if (numlines==0)
> + status_printf_ln(s,color,
Space after ,.
> + "No command remaining.");
> + else{
> +
> + status_printf_ln(s,color,
> + "Next command(s) to do (%d remaining
> command(s)):",
> + numlines);
> + begin = numlines > 1? 0 : 1;
> + for (i = 0; (i < 2 && i < numlines); i++){
> + status_printf(s,color," %s",lines[i]);
> + }
> + if (s->hints )
No space before ).
> +status_printf_ln(s,color,
> + " (use git rebase --edit-todo to view and
> edit)");
Mark for translation -> _("..."). Many instances above.
> - if (has_unmerged(s)) {
> + show_rebase_information(s,state,color);
> + if (has_unmerged(s) || state->rebase_in_progress ||
> !stat(git_path("MERGE_MSG"), &st)) {
The show_rebase_information() line does belong to this patch, but the
other change do not IHMO. It would be _much_ easier to review in its own
patch with an explicit title like "status: factor two rebase-related
messages together" or so.
> @@ -1327,7 +1410,10 @@ void wt_status_print(struct wt_status *s)
> else if (!
Re: [PATCH v2 02/19] parse-options-cb: implement parse_opt_pass_argv_array()
On Tue, Jun 2, 2015 at 11:48 PM, Paul Tan wrote:
> Certain git commands, such as git-pull, are simply wrappers around other
> git commands like git-fetch, git-merge and git-rebase. As such, these
> wrapper commands will typically need to "pass through" command-line
> options of the commands they wrap.
>
> Implement the parse_opt_pass_argv_array() parse-options callback, which
> will reconstruct all the provided command-line options into an
> argv_array, such that it can be passed to another git command. This is
> useful for passing command-line options that can be specified multiple
> times.
>
> Signed-off-by: Paul Tan
> ---
>
> Notes:
> v2
>
> * This function is a requirement for the rewrite of git-am to handle
> passing git-apply's options to git-apply. Since it would be
> implemented anyway I thought it would be good if git-pull could take
> advantage of it as well to handle --strategy and --strategy-option.
>
> parse-options-cb.c | 32
> parse-options.h| 1 +
> 2 files changed, 33 insertions(+)
>
> diff --git a/parse-options-cb.c b/parse-options-cb.c
> index 5b1dbcf..7330506 100644
> --- a/parse-options-cb.c
> +++ b/parse-options-cb.c
> @@ -4,6 +4,7 @@
> #include "commit.h"
> #include "color.h"
> #include "string-list.h"
> +#include "argv-array.h"
>
> /*- some often used options -*/
>
> @@ -163,3 +164,34 @@ int parse_opt_pass_strbuf(const struct option *opt,
> const char *arg, int unset)
>
> return 0;
> }
> +
> +/**
> + * For an option opt, recreate the command-line option, appending it to
> + * opt->value which must be a argv_array. This is useful when we need to pass
> + * the command-line option, which can be specified multiple times, to another
> + * command.
> + */
> +int parse_opt_pass_argv_array(const struct option *opt, const char *arg, int
> unset)
> +{
> + struct strbuf sb = STRBUF_INIT;
> + struct argv_array *opt_value = opt->value;
> +
> + if (opt->long_name) {
> + strbuf_addstr(&sb, unset ? "--no-" : "--");
> + strbuf_addstr(&sb, opt->long_name);
> + if (arg) {
> + strbuf_addch(&sb, '=');
> + strbuf_addstr(&sb, arg);
> + }
> + } else if (opt->short_name && !unset) {
> + strbuf_addch(&sb, '-');
> + strbuf_addch(&sb, opt->short_name);
> + if (arg)
> + strbuf_addstr(&sb, arg);
> + } else
> + return -1;
> +
> + argv_array_push(opt_value, sb.buf);
> + strbuf_release(&sb);
> + return 0;
> +}
> diff --git a/parse-options.h b/parse-options.h
> index 1d21398..b663f87 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -225,6 +225,7 @@ extern int parse_opt_tertiary(const struct option *,
> const char *, int);
> extern int parse_opt_string_list(const struct option *, const char *, int);
> extern int parse_opt_noop_cb(const struct option *, const char *, int);
> extern int parse_opt_pass_strbuf(const struct option *, const char *, int);
> +extern int parse_opt_pass_argv_array(const struct option *, const char *,
> int);
>
> #define OPT__VERBOSE(var, h) OPT_COUNTUP('v', "verbose", (var), (h))
> #define OPT__QUIET(var, h)OPT_COUNTUP('q', "quiet", (var), (h))
> --
> 2.1.4
>
This patch looks very similar to the first one, so I wonder if the
combination of these
could be written as:
static int parse_opt_pass_strbuf_internal(const struct option *opt,
const char *arg, int unset, struct strbuf *sb)
{
if (opt->long_name) {
strbuf_addstr(sb, unset ? "--no-" : "--");
strbuf_addstr(sb, opt->long_name);
if (arg) {
strbuf_addch(sb, '=');
strbuf_addstr(sb, arg);
}
} else if (opt->short_name && !unset) {
strbuf_addch(sb, '-');
strbuf_addch(sb, opt->short_name);
if (arg)
strbuf_addstr(sb, arg);
} else
return -1;
return 0;
}
/**
* For an option opt, recreates the command-line option in opt->value which
* must be an strbuf. This is useful when we need to pass the command-line
* option to another command.
*/
int parse_opt_pass_strbuf(const struct option *opt, const char *arg, int unset)
{
struct strbuf *sb = opt->value;
strbuf_reset(sb);
return parse_opt_pass_strbuf_internal(...);
}
/**
* For an option opt, recreate the command-line option, appending it to
* opt->value which must be a argv_array. This is useful when we need to pass
* the command-line option, which can be specified multiple times, to another
* command.
*/
int parse_opt_pass_argv_array(const struct option *opt, const char
*arg, int unset)
{
struct strbuf sb = STRBUF_INIT;
struct argv_array *opt_value = opt->value;
int ret = parse_opt_pass_strbuf_internal(...
bug "$message" literal in commit message
Hello, I think I found a bug(or strange behavior) in the git. If commit message contains literal "$message" than that literal disappears from commit message. For example: - user@comp ~/cc $ git commit -am "1$message1" [master (root-commit) d36a841] 1 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 1 user@comp ~/cc $ git log commit d36a841ae25510ada80246a78225446083fcb3e1 Author: user Date: Wed Jun 3 18:21:45 2015 +0200 file Sorry for having disturbed you -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v3 4/4] git-am: add am.threeWay config variable
Matthieu Moy writes > Remi Lespinet writes: > > > +am.threeWay:: > > +By default, git-am will fail if the patch does not apply cleanly. > > http://article.gmane.org/gmane.comp.version-control.git/270538 Ok sorry. Thanks -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation
Matthieu Moy writes: >> OPTIONS >> --- >> (|)...:: >> @@ -43,11 +53,11 @@ OPTIONS >> --keep-non-patch:: >> Pass `-b` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]). >> >> ---[no-]keep-cr:: >> +--keep-cr:: >> +--no-keep-cr:: >> With `--keep-cr`, call 'git mailsplit' (see linkgit:git-mailsplit[1]) >> with the same option, to prevent it from stripping CR at the end of >> -lines. `am.keepcr` configuration variable can be used to specify the >> -default behaviour. > > I don't think it's a good idea to remove this part. If I look for a way > to make --keep-cr the default, the first place I'd look would be the doc > for --keep-cr, and I'd appreciate a link to am.keepcr. Yup, very sensible suggestion. Configuration gives default, command line options override the default. People come two different ends; having to describe the same thing twice is wasteful, but will help them one indirection to find relevant facts in the documentation. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSoC] Week 1: Unification of 'for-each-ref', 'tag -l' and 'branch -l'
On 06/03/2015 09:38 PM, Matthieu Moy wrote: Karthik Nayak writes: Matthieu Moy suggested that I work on the unification of these commands let both the implementations stay where the user can select the implementation to be used Just to be clear: my advice is the above with "user" = "caller of the API", not "human being using Git". In other words, "git branch --contains" and "git tag --contains" would still be using two different algorithms, but the user wouldn't notice (except for performance). Thanks for clearing that out, I should have said "caller of the API." -- Regards, Karthik -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 4/4] git-am: add am.threeWay config variable
Remi Lespinet writes: > +am.threeWay:: > + By default, git-am will fail if the patch does not apply cleanly. http://article.gmane.org/gmane.comp.version-control.git/270538 -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation
Remi Lespinet writes: > Prepare a configuration section for the git am documentation. Most git-*.txt do not have this CONFIGURATION section. In an ideal world, we would have such section automatically generated (i.e. the description for each variable would exist in one place, and we would make sure that both "man git-config" and "man git-" show it). In a really ideal world, it would also be propagated to the code and we would have a "git config --describe am.keepcr" or so that would return the doc. I'm a bit worried to see documentation cut-and-pasted from config.txt to git-*.txt for maintainability: if the text on one side is modified, we're likely to forget the other and the text will diverge with time. Not a strong objection, but I have the feeling that the more we do this kind of patches, the harder it will be if ever we decide to do the above. > +CONFIGURATION > +- > + > +am.keepcr:: > + If true, git-am will call git-mailsplit for patches in mbox format `git am` `git mailsplit` > + with parameter '--keep-cr'. In this case git-mailsplit will Likewise > + not remove `\r` from lines ending with `\r\n`. Can be overridden > + by giving '--no-keep-cr' from the command line. That should be backquote, not forward-quote, right? I know it's not your code since it's a cut-and-paste from config.txt, but that illustrates my point above: we used to have one place with wrong quotes, and we'd have two after the patch. > OPTIONS > --- > (|)...:: > @@ -43,11 +53,11 @@ OPTIONS > --keep-non-patch:: > Pass `-b` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]). > > ---[no-]keep-cr:: > +--keep-cr:: > +--no-keep-cr:: > With `--keep-cr`, call 'git mailsplit' (see linkgit:git-mailsplit[1]) > with the same option, to prevent it from stripping CR at the end of > - lines. `am.keepcr` configuration variable can be used to specify the > - default behaviour. I don't think it's a good idea to remove this part. If I look for a way to make --keep-cr the default, the first place I'd look would be the doc for --keep-cr, and I'd appreciate a link to am.keepcr. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSoC] Week 1: Unification of 'for-each-ref', 'tag -l' and 'branch -l'
Karthik Nayak writes: > Matthieu Moy suggested that I work on the unification of these > commands let both the implementations stay where the user can select > the implementation to be used Just to be clear: my advice is the above with "user" = "caller of the API", not "human being using Git". In other words, "git branch --contains" and "git tag --contains" would still be using two different algorithms, but the user wouldn't notice (except for performance). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Stash Feature
On Wed, 3 Jun 2015 17:22:57 +0200
Fabrizio Mancin wrote:
> I've a little request for you.
> What about saving date-time on git stash save command and show it on
> git stash show stash@{xxx}?
> I think it is a useful poperty to save.
>
> What do you think about it?
This information is already there as a stash entry is internally
represented as a commit (with one or more parents), and being a commit,
it possess all the standard attributes of a commit, including the
creation timestamp.
To get "normal" view of this commit just run
git show stash@{0}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Minor bug report
On di, 2015-06-02 at 23:48 -0700, Junio C Hamano wrote: > > I am kind of surprised after reading these two threads that my > take on this issue has changed over time, as my knee-jerk > reaction before reading them was the opposite, something > along the lines of "This is only immediately after 'git init'; why > bother?". Or depending on my mood, that "How stupid do you > have to be..." sounds exactly like a message I may advocate > for us to send. Perhaps I grew more bitter with age. The "fatal: Failed to resolve 'HEAD' as a valid ref." message, closely related to the "fatal: bad default revision 'HEAD'" message that started this thread just came by in #git with the following situation: $ git init $ git add . # Oops, didn't want to add foo.xyz $ git reset foo.xyz fatal: Failed to resolve 'HEAD' as a valid ref. The solution there is simple, git rm --cached, but I think git could produce more helpful messages when a repo is empty. I think you are growing bitter with age ;) -- Dennis Kaarsemaker www.kaarsemaker.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Stash Feature
Hi Guys,
I've a little request for you.
What about saving date-time on git stash save command and show it on
git stash show stash@{xxx}?
I think it is a useful poperty to save.
What do you think about it?
Thank you
Fabrizio
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] read_loose_refs(): treat NULL_SHA1 loose references as broken
On Wed, Jun 03, 2015 at 03:51:59PM +0200, Michael Haggerty wrote:
> NULL_SHA1 is used to indicate of "invalid SHA-1" throughout our code
s/of/an/ ?
> (and the code of other git implementations), so it is vastly more
> likely that a reference was set to this value due to a software bug
> than that NULL_SHA1 is the legitimate SHA-1 of an actual object.
> Therefore, if a loose reference has the value NULL_SHA1, consider it
> to be broken.
>
> Amusingly, each of the other 2^160-1 possible SHA-1 values is exactly
> as unlikely as NULL_SHA1 to be the SHA-1 of an actual object. The
> difference is that most of those other values are also very unlikely
> to be written to a loose reference file by accident, whereas
> accidentally writing NULL_SHA1 to a loose reference file would be an
> easy mistake to make.
FWIW, I think this justification (and the comment below) reads better
than what you had before.
> diff --git a/refs.c b/refs.c
> index 6736424..83af13d 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1324,6 +1324,16 @@ static void read_loose_refs(const char *dirname,
> struct ref_dir *dir)
> if (!read_ok) {
> hashclr(sha1);
> flag |= REF_ISBROKEN;
> + } else if (is_null_sha1(sha1)) {
> + /*
> + * It is so astronomically unlikely
> + * that NULL_SHA1 is the SHA-1 of an
> + * actual object that we consider its
> + * appearance in a loose reference
> + * file to be repo corruption
> + * (probably due to a software bug).
> + */
> + flag |= REF_ISBROKEN;
Nice. After reading the other thread, I did not think we needed to
bother with more refactoring here, but I agree this end result flows
nicely.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/4] read_loose_refs(): treat NULL_SHA1 loose references as broken
NULL_SHA1 is used to indicate of "invalid SHA-1" throughout our code
(and the code of other git implementations), so it is vastly more
likely that a reference was set to this value due to a software bug
than that NULL_SHA1 is the legitimate SHA-1 of an actual object.
Therefore, if a loose reference has the value NULL_SHA1, consider it
to be broken.
Amusingly, each of the other 2^160-1 possible SHA-1 values is exactly
as unlikely as NULL_SHA1 to be the SHA-1 of an actual object. The
difference is that most of those other values are also very unlikely
to be written to a loose reference file by accident, whereas
accidentally writing NULL_SHA1 to a loose reference file would be an
easy mistake to make.
Signed-off-by: Michael Haggerty
---
refs.c | 10 ++
t/t6301-for-each-ref-errors.sh | 2 +-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 6736424..83af13d 100644
--- a/refs.c
+++ b/refs.c
@@ -1324,6 +1324,16 @@ static void read_loose_refs(const char *dirname, struct
ref_dir *dir)
if (!read_ok) {
hashclr(sha1);
flag |= REF_ISBROKEN;
+ } else if (is_null_sha1(sha1)) {
+ /*
+* It is so astronomically unlikely
+* that NULL_SHA1 is the SHA-1 of an
+* actual object that we consider its
+* appearance in a loose reference
+* file to be repo corruption
+* (probably due to a software bug).
+*/
+ flag |= REF_ISBROKEN;
}
if (check_refname_format(refname.buf,
diff --git a/t/t6301-for-each-ref-errors.sh b/t/t6301-for-each-ref-errors.sh
index 72d2397..cdb67a0 100755
--- a/t/t6301-for-each-ref-errors.sh
+++ b/t/t6301-for-each-ref-errors.sh
@@ -24,7 +24,7 @@ test_expect_success 'Broken refs are reported correctly' '
test_cmp broken-err err
'
-test_expect_failure 'NULL_SHA1 refs are reported correctly' '
+test_expect_success 'NULL_SHA1 refs are reported correctly' '
r=refs/heads/zeros &&
echo $ZEROS >.git/$r &&
test_when_finished "rm -f .git/$r" &&
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/4] for-each-ref: report broken references correctly
If there is a loose reference file with invalid contents, "git
for-each-ref" incorrectly reports the problem as being a missing
object with name NULL_SHA1:
$ echo '12345678' >.git/refs/heads/nonsense
$ git for-each-ref
fatal: missing object for
refs/heads/nonsense
With an explicit "--format" string, it can even report that the
reference validly points at NULL_SHA1:
$ git for-each-ref --format='%(objectname) %(refname)'
refs/heads/nonsense
$ echo $?
0
This has been broken since
b7dd2d2 for-each-ref: Do not lookup objects when they will not be used
(2009-05-27)
, which changed for-each-ref from using for_each_ref() to using
git_for_each_rawref() in order to avoid looking up the referred-to
objects unnecessarily. (When "git for-each-ref" is given a "--format"
string that doesn't include information about the pointed-to object,
it does not look up the object at all, which makes it considerably
faster. Iterating with DO_FOR_EACH_INCLUDE_BROKEN is essential to this
optimization because otherwise for_each_ref() would itself need to
check whether the object exists as part of its brokenness test.)
But for_each_rawref() includes broken references in the iteration, and
"git for-each-ref" doesn't itself reject references with REF_ISBROKEN.
The result is that broken references are processed *as if* they had
the value NULL_SHA1, which is the value stored in entries for broken
references.
Change "git for-each-ref" to emit warnings for references that are
REF_ISBROKEN but to otherwise skip them.
Signed-off-by: Michael Haggerty
---
builtin/for-each-ref.c | 5 +
t/t6301-for-each-ref-errors.sh | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 83f9cf9..13d2172 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -851,6 +851,11 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
return 0;
}
+ if (flag & REF_ISBROKEN) {
+ warning("ignoring broken ref %s", refname);
+ return 0;
+ }
+
if (*cb->grab_pattern) {
const char **pattern;
int namelen = strlen(refname);
diff --git a/t/t6301-for-each-ref-errors.sh b/t/t6301-for-each-ref-errors.sh
index cf25244..72d2397 100755
--- a/t/t6301-for-each-ref-errors.sh
+++ b/t/t6301-for-each-ref-errors.sh
@@ -14,7 +14,7 @@ test_expect_success setup '
git for-each-ref --format="%(objectname) %(refname)" >brief-list
'
-test_expect_failure 'Broken refs are reported correctly' '
+test_expect_success 'Broken refs are reported correctly' '
r=refs/heads/bogus &&
: >.git/$r &&
test_when_finished "rm -f .git/$r" &&
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/4] read_loose_refs(): simplify function logic
Make it clearer that there are two possible ways to read the
reference, but that we handle read errors uniformly regardless of
which way it was read.
This refactoring also makes the following change easier to implement.
Signed-off-by: Michael Haggerty
---
refs.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 47e4e53..6736424 100644
--- a/refs.c
+++ b/refs.c
@@ -1308,19 +1308,24 @@ static void read_loose_refs(const char *dirname, struct
ref_dir *dir)
create_dir_entry(refs, refname.buf,
refname.len, 1));
} else {
+ int read_ok;
+
if (*refs->name) {
hashclr(sha1);
flag = 0;
- if (resolve_gitlink_ref(refs->name,
refname.buf, sha1) < 0) {
- hashclr(sha1);
- flag |= REF_ISBROKEN;
- }
- } else if (read_ref_full(refname.buf,
-RESOLVE_REF_READING,
-sha1, &flag)) {
+ read_ok = !resolve_gitlink_ref(refs->name,
+ refname.buf,
sha1);
+ } else {
+ read_ok = !read_ref_full(refname.buf,
+RESOLVE_REF_READING,
+sha1, &flag);
+ }
+
+ if (!read_ok) {
hashclr(sha1);
flag |= REF_ISBROKEN;
}
+
if (check_refname_format(refname.buf,
REFNAME_ALLOW_ONELEVEL)) {
hashclr(sha1);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/4] Fix how for-each-ref handles broken loose references
This reroll addresses all of the comments about v1 [1] and v2 [2]. Thanks to Stefan, Junio, and Peff for their comments about v2. Changes since v2: * Simplify logic flow in read_loose_refs(). * Remove unnecessary call to hashclr() in read_loose_refs(). * Improve a comment and commit message. This patch series is also available from my GitHub account [3] as branch for-each-ref-errors. [1] http://thread.gmane.org/gmane.comp.version-control.git/270429 [2] http://thread.gmane.org/gmane.comp.version-control.git/270556 [3] https://github.com/mhagger/git Michael Haggerty (4): t6301: new tests of for-each-ref error handling for-each-ref: report broken references correctly read_loose_refs(): simplify function logic read_loose_refs(): treat NULL_SHA1 loose references as broken builtin/for-each-ref.c | 5 refs.c | 29 -- t/t6301-for-each-ref-errors.sh | 56 ++ 3 files changed, 83 insertions(+), 7 deletions(-) create mode 100755 t/t6301-for-each-ref-errors.sh -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/4] t6301: new tests of for-each-ref error handling
Add tests that for-each-ref correctly reports broken loose reference files and references that point at missing objects. In fact, two of these tests fail, because (1) NULL_SHA1 is not recognized as an invalid reference value, and (2) for-each-ref doesn't respect REF_ISBROKEN. Fixes to come. Note that when for-each-ref is run with a --format option that doesn't require the object to be looked up, then we should still notice if a loose reference file is corrupt or contains NULL_SHA1, but we don't notice if it points at a missing object because we don't do an object lookup. This is OK. Signed-off-by: Michael Haggerty --- t/t6301-for-each-ref-errors.sh | 56 ++ 1 file changed, 56 insertions(+) create mode 100755 t/t6301-for-each-ref-errors.sh diff --git a/t/t6301-for-each-ref-errors.sh b/t/t6301-for-each-ref-errors.sh new file mode 100755 index 000..cf25244 --- /dev/null +++ b/t/t6301-for-each-ref-errors.sh @@ -0,0 +1,56 @@ +#!/bin/sh + +test_description='for-each-ref errors for broken refs' + +. ./test-lib.sh + +ZEROS=$_z40 +MISSING=abababababababababababababababababababab + +test_expect_success setup ' + git commit --allow-empty -m "Initial" && + git tag testtag && + git for-each-ref >full-list && + git for-each-ref --format="%(objectname) %(refname)" >brief-list +' + +test_expect_failure 'Broken refs are reported correctly' ' + r=refs/heads/bogus && + : >.git/$r && + test_when_finished "rm -f .git/$r" && + echo "warning: ignoring broken ref $r" >broken-err && + git for-each-ref >out 2>err && + test_cmp full-list out && + test_cmp broken-err err +' + +test_expect_failure 'NULL_SHA1 refs are reported correctly' ' + r=refs/heads/zeros && + echo $ZEROS >.git/$r && + test_when_finished "rm -f .git/$r" && + echo "warning: ignoring broken ref $r" >zeros-err && + git for-each-ref >out 2>err && + test_cmp full-list out && + test_cmp zeros-err err && + git for-each-ref --format="%(objectname) %(refname)" >brief-out 2>brief-err && + test_cmp brief-list brief-out && + test_cmp zeros-err brief-err +' + +test_expect_success 'Missing objects are reported correctly' ' + r=refs/heads/missing && + echo $MISSING >.git/$r && + test_when_finished "rm -f .git/$r" && + echo "fatal: missing object $MISSING for $r" >missing-err && + test_must_fail git for-each-ref 2>err && + test_cmp missing-err err && + ( + cat brief-list && + echo "$MISSING $r" + ) | sort -k 2 >missing-brief-expected && + git for-each-ref --format="%(objectname) %(refname)" >brief-out 2>brief-err && + test_cmp missing-brief-expected brief-out && + test_must_be_empty brief-err +' + +test_done -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposal for git stash : add --staged option
Hi again,
just wanted to tell that I have created a solution by doing a few lines
of scripting:
git-cstash
```
#/bin/sh
git commit -m 'temporary, will be stashed soon'
git stash --include-untracked
git reset HEAD^1
git stash
git stash pop stash@{1}
```
Le 2015-04-22 11:25, Johannes Schindelin a écrit :
Hi Edgar,
On 2015-04-22 10:30, edgar.h...@netapsys.fr wrote:
When you have a lot of unstaged files, and would like to test what
happens if you undo some of the changes that you think are unecessary,
you would rather keep a copy of those changes somewhere.
For example
Changed but not updated:
M config_test.xml
M config_real.xml
I have changed both config_test.xml and config_real.xml, but I think
the changes made in config_test.xml are unnecessary. However, I would
still like to keep them somewhere in case it breaks something.
In this case for example, I would like to be able to stash only the
file config_test.xml
Eg:
git add config_test.xml
git stash --staged
So that after this, my git looks like this:
Changed but not updated:
M config_real.xml
and my stash contains only the changes introduced in config_test.xml
`git stash --keep-index` doesn't give the necessary control, because
it will still stash everything (and create unnecessary merge
complications if I change the files and apply the stash)
I often have the same problem. How about doing this:
```sh
git add config_real.xml
git stash -k
git reset
```
The difference between our approaches is that I keep thinking of the
staging area as the place to put changes I want to *keep*, not that I
want to forget for a moment.
Having said that, I am sympathetic to your cause, although I would
rather have `git stash [--patch] -- [...]` that would be used
like `git add -p` except that the selected changes are *not* staged,
but stashed instead.
Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFCv4 2/2] git rebase -i: warn about removed commits
Galan Rémi writes: > +comm -2 -3 "$todo".oldsha1 "$todo".newsha1 >"$todo".miss > + > +# Make the list user-friendly > +opt="--no-walk=sorted --format=oneline --abbrev-commit > --stdin" > +git rev-list $opt <"$todo".miss >"$todo".miss+ > +mv "$todo".miss+ "$todo".miss > + > +# Check missing commits Found a bug here, got an error message from git rev-list if "$todo".miss is empty. Now it looks like: > # Check missing commits > if test -s "$todo".miss > then > # Make the list user-friendly > opt="--no-walk=sorted --format=oneline --abbrev-commit > --stdin" > git rev-list $opt <"$todo".miss >"$todo".miss+ > mv "$todo".miss+ "$todo".miss > > warn "Warning: some commits may have been dropped" \ Thus the empty case is tested by the test -s of the warnings. By the way, should I add --quiet to the options of the call to git rev-list? Rémi -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Spontaneous "changes" upon checking out a commit
When I checkout a particular commit in my repo and immediately do a git status, there is a file that show up as changed. If I look at the SHAs for my working copy and the copy in the index/repo, they are indeed different. The working copy has carriage returns, and while I *suspect* the copy in the index/repo does not have them, the output from git cat-file on the repo SHA does have them. In fact, if I redirect that output to a file and do a git hash-object of that file, I get back the the same SHA as the working copy. Doing a git checkout on the file changes nothing, i.e. it is still marked as changed in git status. Trying to checkout another commit fails, as my "changes" to the file would be overwritten by the checkout. Doing a git diff on the file returns nothing. The repo has a .gitattributes file, which starts out with "* text=auto !eol". Every file in the repo is explicitly listed in .gitattributes as -text, *except* the file that has the spontaneous "changes". The file's suffix is .cs, which I thought was automatically considered text anyway. On my machine, core.autocrlf is false. This is running on Windows. Git version is 1.9.5.msysgit.1. Does anyone know what might be going on here? It is almost as if git is adding the CRs to the file at checkout, but not "realizing" that when it comes to deciding if there are local changes. Thanks for any insight. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFCv4 2/2] git rebase -i: warn about removed commits
Check if commits were removed (i.e. a line was deleted) and print
warnings or abort git rebase depending on the value of the
configuration variable rebase.missingCommits.
This patch gives the user the possibility to avoid silent loss of
information (losing a commit through deleting the line in this case)
if he wants to.
Add the configuration variable rebase.missingCommitsCheck.
- When unset or set to "ignore", no checking is done.
- When set to "warn", the commits are checked, warnings are
displayed but git rebase still proceeds.
- When set to "error", the commits are checked, warnings are
displayed and the rebase is aborted.
rebase.missingCommitsCheck defaults to "ignore".
Signed-off-by: Galan Rémi
---
Documentation/config.txt | 10 ++
Documentation/git-rebase.txt | 6
git-rebase--interactive.sh| 82 +++
t/t3404-rebase-interactive.sh | 63 +
4 files changed, 161 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4d21ce1..b29cd8d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2160,6 +2160,16 @@ rebase.autoStash::
successful rebase might result in non-trivial conflicts.
Defaults to false.
+rebase.missingCommitsCheck::
+ If set to "warn", git rebase -i will print a warning if some
+ commits are removed (e.g. a line was deleted), however the
+ rebase will still proceed. If set to "error", it will print
+ the previous warning and abort the rebase. If set to
+ "ignore", no checking is done.
+ To drop a commit without warning or error, use the `drop`
+ command in the todo-list.
+ Defaults to "ignore".
+
receive.advertiseAtomic::
By default, git-receive-pack will advertise the atomic push
capability to its clients. If you don't want to this capability
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 9cf3760..6d413a1 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -213,6 +213,12 @@ rebase.autoSquash::
rebase.autoStash::
If set to true enable '--autostash' option by default.
+rebase.missingCommitsCheck::
+ If set to "warn" print warnings about removed commits in
+ interactive mode. If set to "error" print the warnings and
+ abort the rebase. If set to "ignore" no checking is
+ done. "ignore" by default.
+
OPTIONS
---
--onto ::
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 869cc60..26804dd 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -851,6 +851,86 @@ add_exec_commands () {
mv "$1.new" "$1"
}
+# Print the list of the SHA-1 of the commits
+# from a todo list in a file.
+# $1: todo-file, $2: outfile
+todo_list_to_sha_list () {
+ git stripspace --strip-comments <"$1" | while read -r command sha1 rest
+ do
+ case $command in
+ x|"exec")
+ ;;
+ *)
+ printf "%s\n" "$sha1"
+ ;;
+ esac
+ done >"$2"
+}
+
+# Use warn for each line of a file
+# $1: file
+warn_file () {
+ while read -r line
+ do
+ warn " - $line"
+ done <"$1"
+}
+
+# Check if the user dropped some commits by mistake
+# Behaviour determined by rebase.missingCommitsCheck.
+check_commits () {
+ checkLevel=$(git config --get rebase.missingCommitsCheck)
+ checkLevel=${checkLevel:-ignore}
+ # Don't be case sensitive
+ checkLevel=$(echo "$checkLevel" | tr 'A-Z' 'a-z')
+
+ case "$checkLevel" in
+ warn|error)
+ # Get the SHA-1 of the commits
+ todo_list_to_sha_list "$todo".backup "$todo".oldsha1
+ todo_list_to_sha_list "$todo" "$todo".newsha1
+
+ # Sort the SHA-1 and compare them
+ sort -u "$todo".oldsha1 >"$todo".oldsha1+
+ mv "$todo".oldsha1+ "$todo".oldsha1
+ sort -u "$todo".newsha1 >"$todo".newsha1+
+ mv "$todo".newsha1+ "$todo".newsha1
+ comm -2 -3 "$todo".oldsha1 "$todo".newsha1 >"$todo".miss
+
+ # Make the list user-friendly
+ opt="--no-walk=sorted --format=oneline --abbrev-commit --stdin"
+ git rev-list $opt <"$todo".miss >"$todo".miss+
+ mv "$todo".miss+ "$todo".miss
+
+ # Check missing commits
+ if test -s "$todo".miss
+ then
+ warn "Warning: some commits may have been dropped" \
+ "accidentally."
+ warn "Dropped commits (newer to older):"
+ warn_file "$todo".miss
+ warn ""
+ warn "To avoid this message, use \"drop\" to" \
+ "explicitly
[PATCH/RFCv4 1/2] git-rebase -i: add command "drop" to remove a commit
Instead of removing a line to remove the commit, you can use the
command "drop" (just like "pick" or "edit"). It has the same effect as
deleting the line (removing the commit) except that you keep a visual
trace of your actions, allowing a better control and reducing the
possibility of removing a commit by mistake.
Signed-off-by: Galan Rémi
---
Documentation/git-rebase.txt | 3 +++
git-rebase--interactive.sh| 18 ++
t/lib-rebase.sh | 4 ++--
t/t3404-rebase-interactive.sh | 10 ++
4 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 1d01baa..9cf3760 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -514,6 +514,9 @@ rebasing.
If you just want to edit the commit message for a commit, replace the
command "pick" with the command "reword".
+To drop a commit, replace the command "pick" with "drop", or just
+delete its line.
+
If you want to fold two or more commits into one, replace the command
"pick" for the second and subsequent commits with "squash" or "fixup".
If the commits had different authors, the folded commit will be
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..869cc60 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -152,6 +152,7 @@ Commands:
s, squash = use commit, but meld into previous commit
f, fixup = like "squash", but discard this commit's log message
x, exec = run command (the rest of the line) using shell
+ d, drop = remove commit
These lines can be re-ordered; they are executed from top to bottom.
@@ -508,6 +509,23 @@ do_next () {
"$comment_char"*|''|noop)
mark_action_done
;;
+ drop|d)
+ if test -z $sha1
+ then
+ warn "Missing SHA-1 in 'drop' command."
+ die "Please fix this using 'git rebase --edit-todo'."
+ fi
+
+ sha1_verif="$(git rev-parse --verify --quiet $sha1^{commit})"
+ if test -z $sha1_verif
+ then
+ warn "'$sha1' is not a SHA-1 or does not represent" \
+ "a commit in 'drop' command."
+ die "Please fix this using 'git rebase --edit-todo'."
+ fi
+
+ mark_action_done
+ ;;
pick|p)
comment_for_reflog pick
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6bd2522..fdbc900 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -14,7 +14,7 @@
# specified line.
#
# " " -- add a line with the specified command
-# ("squash", "fixup", "edit", or "reword") and the SHA1 taken
+# ("squash", "fixup", "edit", "reword" or "drop") and the SHA1 taken
# from the specified line.
#
# "exec_cmd_with_args" -- add an "exec cmd with args" line.
@@ -46,7 +46,7 @@ set_fake_editor () {
action=pick
for line in $FAKE_LINES; do
case $line in
- squash|fixup|edit|reword)
+ squash|fixup|edit|reword|drop)
action="$line";;
exec*)
echo "$line" | sed 's/_/ /g' >> "$1";;
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ac429a0..8960083 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1102,4 +1102,14 @@ test_expect_success 'rebase -i commits that overwrite
untracked files (no ff)' '
test $(git cat-file commit HEAD | sed -ne \$p) = I
'
+test_expect_success 'drop' '
+ test_when_finished "git checkout master" &&
+ git checkout -b dropBranchTest master &&
+ set_fake_editor &&
+ FAKE_LINES="1 drop 2 3 drop 4 5" git rebase -i --root &&
+ test E = $(git cat-file commit HEAD | sed -ne \$p) &&
+ test C = $(git cat-file commit HEAD^ | sed -ne \$p) &&
+ test A = $(git cat-file commit HEAD^^ | sed -ne \$p)
+'
+
test_done
--
2.4.2.389.geaf7ccf
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 17/19] pull --rebase: exit early when the working directory is dirty
On Wed, Jun 03, 2015 at 02:49:01PM +0800, Paul Tan wrote: > Re-implement the behavior introduced by f9189cf (pull --rebase: exit > early when the working directory is dirty, 2008-05-21). When the option rebase.autoStash is set, it should not be necessary to die in this case. See also this[1] patch I'm working on. [1]: http://article.gmane.org/gmane.comp.version-control.git/270612 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git Rev News edition 4
Hi, Git Rev News edition 4 is now available: https://git.github.io/rev_news/2015/06/03/edition-4/ Thanks a lot to all the helpers! Enjoy, Christian, Thomas and Nicola. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
Jeff King peff.net> writes: >I would say the more "usual" way to use checkout like this >is to give specific paths. I.e., run "git status", say "oh, I need to >restore the contents of 'foo', but not 'bar'", and run "git checkout >foo". That works regardless of the type of change to "foo" and "bar". That seems fine - a specific file is named and you clearly want to alter the contents of that file. By analogy, 'rm foo' will silently delete it, but if you specify a directory to delete recursively you need the -r flag. OK, it's not a perfect analogy because the purpose of rm is to delete data and nothing else ;-). If my personal experience is anything to go by, newcomers may fall into the habit of running 'git checkout .' to restore missing files. In the old days I would often delete a file and then run 'cvs update' or 'svn update' to restore it. That would fetch a fresh copy from the repository, and while it might do some kind of diff/patch operation on modified files, it would not simply throw away local changes. 'git checkout .' seems like the analogous command, but it has much sharper edges. I still think it should be safer by default, but if you decide against that then perhaps you need to create some way to restore missing files and not overwrite others. 'git checkout --no-overwrite'? Then it could even be added to .gitconfig as the default for those who like it. I have to say that as a newcomer to git I do not like the idea of creating a special undo log for git. It would just be yet another concept to learn and another thing to add to the list of 'where is git hiding my data this time?'. And the time when it would be useful - after some bungled operation that lost data - is just the time when the user is already confused and adding another semi-hidden stash of objects to the mix would befuddle them further. If there is to be a backup made of local changes that get lost, and I agree it is a good idea, then it should be something stupid and completely obvious, such as saving the old file as 'foo.before_checkout.1'. -- Ed Avis -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
On Wed, Jun 03, 2015 at 09:21:59AM +, Ed Avis wrote: > I had expected that 'git checkout .' would fix up my working tree to make it > match the repository (in this case, the current revision of the master > branch). It did. :) > The user interface might be something like: > > % git checkout . > error: Your local changes to the following files would be overwritten: > foo > You may want to commit or stash these changes, or delete the files if you > don't want them. Use 'git checkout --force' to proceed, throwing away > local changes. > Aborting > > If the checkout operation would only involve creating some files on disk > which aren't currently there, then it would proceed without prompting. Thanks for explaining. I see where you are coming from, though I'm still a bit lukewarm on the idea, if only because the vast majority of invocations would involve "--force". It also seems a bit special-cased to treat restoring deletions specially. I would say the more "usual" way to use checkout like this is to give specific paths. I.e., run "git status", say "oh, I need to restore the contents of 'foo', but not 'bar'", and run "git checkout foo". That works regardless of the type of change to "foo" and "bar". If we want to introduce more safety here, I'd be inclined to perform the operation by default, but give a better escape hatch. For example, by creating a loose object for any file we're about to overwrite, and possibly writing an entry into a log. That's a lot more work, but has a few advantages: 1. It helps even when you just ran with "--force" followed by an "oops, why did I do that?" moment. 2. It can help other commands like "git clean". 3. That log could form a basis for a "git undo" program to help with "oops" moments in general (e.g., if you use "git reset ." to overwrite what is in the index, we have all of the old file content in objects, but it can sometimes be a pain to figure out _which_ objects went where. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
I had expected that 'git checkout .' would fix up my working tree to make it match the repository (in this case, the current revision of the master branch). When I originally ran it I had deleted a couple of files from the working tree and wanted to restore them. However, I expected that if doing the checkout operation would lose data currently on disk then git would prompt me first. To compare, 'git pull' will not silently overwrite local changes; it will prompt you to commit or stash them first. 'git checkout .' is a fairly innocuous-looking command; it doesn't contain any --force or --overwrite or other things that would make you think twice before typing it. So I suggest it should be equally safe to run. The user interface might be something like: % git checkout . error: Your local changes to the following files would be overwritten: foo You may want to commit or stash these changes, or delete the files if you don't want them. Use 'git checkout --force' to proceed, throwing away local changes. Aborting If the checkout operation would only involve creating some files on disk which aren't currently there, then it would proceed without prompting. -- Ed Avis -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFCv2 1/2] git-rebase -i: add command "drop" to remove a commit
Junio C Hamano writes: > As long as what is given to 'drop' > is checked when it matters (e.g. when the code in patch 2/2 tries > see if some commits in the original list are no longer there in > order to warn sees "drop foo bar" where "foo" is obviously not an > object name in the original list, that should be checked), it is > fine. And I agree 1/2 is not the place to do so, even though it may > be easier from the implementation point of view (which is why I > mentioned the possibility in the review of that patch). I disagree, I think that that either the checking for the 'drop' command should either be in the 1/2 where it is introduced or in the function check_commits introduced by 2/2 but in a separate commit/patch. The 2/2 checks if there are removed commits to have the possibility to avoid silent loss of information. It is not its role to check if the SHA-1 following 'drop' are correct. What I think should be the best for now is checking the SHA-1 following 'drop' in 1/2 (or not checking at all) and change the whole checking system of rebase in a later patch (e.g. check beforehand (probably in check_commits) if the commands exist, if the SHA-1 are correct and possibly other things). Rémi -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] read_loose_refs(): treat NULL_SHA1 loose references as broken
On Tue, Jun 02, 2015 at 11:10:22PM +0200, Michael Haggerty wrote:
> >> +
> >> + if (!(flag & REF_ISBROKEN) && is_null_sha1(sha1)) {
> >
> > Why do we do the extra check for !(flag & REF_ISBROKEN) here?
>
> That was an attempt to avoid calling is_null_sha1() unnecessarily. I
> think I can make this go away and make the code clearer in general by
> restructuring the logic a little bit. I will do that in the next round.
If you get rid of the useless hashclr(), then this just becomes:
if (!(flag & REF_ISBROKEN) && is_null_sha1(sha1))
flag |= REF_ISBROKEN;
The reason for the initial check seems pretty obvious then (but it would
also be OK without it; is_null_sha1 is not that expensive).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: make git checkout safer
On Wed, Jun 03, 2015 at 08:50:44AM +, Ed Avis wrote: > Currently a plain 'git checkout .' will revert any local changes, e.g. > > % mkdir test > % cd test > % git init > Initialized empty Git repository in /home/eda/test/.git/ > % echo hello >foo > % git add foo > % git commit -m. > [master (root-commit) 34f6694] . > 1 file changed, 1 insertion(+) > create mode 100644 foo > % echo goodbye >foo > % git checkout . > % cat foo > hello > > I suggest this is dangerous and by default 'git checkout' should only alter > files which do not have local changes (as would be reported by 'git diff'). > Only if --force is given should working tree differences be thrown away. > > % git --version > git version 2.4.0 That's what "git checkout " is designed for. I'm not clear on what you expect "git checkout ." to do in this example, if not overwrite "foo". Can you elaborate? -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Suggestion: make git checkout safer
Currently a plain 'git checkout .' will revert any local changes, e.g. % mkdir test % cd test % git init Initialized empty Git repository in /home/eda/test/.git/ % echo hello >foo % git add foo % git commit -m. [master (root-commit) 34f6694] . 1 file changed, 1 insertion(+) create mode 100644 foo % echo goodbye >foo % git checkout . % cat foo hello I suggest this is dangerous and by default 'git checkout' should only alter files which do not have local changes (as would be reported by 'git diff'). Only if --force is given should working tree differences be thrown away. % git --version git version 2.4.0 -- Ed Avis -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 1/2] status: give more information during rebase -i
git status gives more information during rebase -i, about the list of
command that are done during the rebase. It displays the two last
commands executed and the two next lines to be executed. It also gives
hints to find the whole files in .git directory.
Signed-off-by: Guillaume Pagès
---
At the moment it only gives information during an interactive rebase. It
displays full sha1 identifiers, changing to short sha1 would be better.
wt-status.c | 126 ++--
1 file changed, 106 insertions(+), 20 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 33452f1..8a32aea 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1026,13 +1026,104 @@ static int split_commit_in_progress(struct wt_status
*s)
return split_in_progress;
}
+void get_two_last_lines(char *filename,int * numlines, char ** lines)
+{
+ *numlines=0;
+ struct strbuf buf = STRBUF_INIT;
+ FILE *fp = fopen(git_path("%s", filename), "r");
+ if (!fp) {
+ strbuf_release(&buf);
+ return;
+ }
+ while (strbuf_getline(&buf, fp, '\n')!=EOF){
+ (*numlines)++;
+ lines[0]=lines[1];
+ lines[1]=strbuf_detach(&buf, NULL);
+ }
+ if (!fclose(fp))
+ strbuf_detach(&buf, NULL);
+ else
+ strbuf_release(&buf);
+}
+
+void get_two_first_lines(char *filename,int * numlines, char ** lines)
+{
+ *numlines=0;
+ struct strbuf buf = STRBUF_INIT;
+ char *line;
+ FILE *fp = fopen(git_path("%s", filename), "r");
+ if (!fp) {
+ strbuf_release(&buf);
+ return;
+ }
+ while (strbuf_getline(&buf, fp, '\n')!=EOF){
+ stripspace(&buf, 1);
+ line = strbuf_detach(&buf, NULL);
+ if (strcmp(line,"")==0)
+ continue;
+ if (*numlines<2)
+ lines[*numlines] = line;
+ (*numlines)++;
+ }
+ if (!fclose(fp))
+ strbuf_detach(&buf, NULL);
+ else
+ strbuf_release(&buf);
+}
+
+void show_rebase_information(struct wt_status *s,
+ struct wt_status_state *state,
+ const char *color)
+{
+ if (state->rebase_interactive_in_progress){
+ int i, begin;
+ int numlines =0;
+ char* lines[2];
+ get_two_last_lines("rebase-merge/done", &numlines, lines);
+ if (numlines==0)
+ status_printf_ln(s,color,"No command done.");
+ else{
+ status_printf_ln(s,color,
+ "Last command(s) done (%d command(s) done):",
+ numlines);
+ begin = numlines > 1? 0 : 1;
+ for (i = begin; i < 2; i++){
+ status_printf_ln(s,color," %s",lines[i]);
+ }
+ if (numlines>2 && s->hints )
+ status_printf_ln(s,color,
+ " (see more at .git/rebase-merge/done)");
+ }
+ numlines =0;
+ get_two_first_lines("rebase-merge/git-rebase-todo",
+&numlines, lines);
+ if (numlines==0)
+ status_printf_ln(s,color,
+"No command remaining.");
+ else{
+
+ status_printf_ln(s,color,
+ "Next command(s) to do (%d remaining
command(s)):",
+ numlines);
+ begin = numlines > 1? 0 : 1;
+ for (i = 0; (i < 2 && i < numlines); i++){
+ status_printf(s,color," %s",lines[i]);
+ }
+ if (s->hints )
+ status_printf_ln(s,color,
+ " (use git rebase --edit-todo to view and
edit)");
+ }
+ }
+}
+
static void show_rebase_in_progress(struct wt_status *s,
struct wt_status_state *state,
const char *color)
{
struct stat st;
- if (has_unmerged(s)) {
+ show_rebase_information(s,state,color);
+ if (has_unmerged(s) || state->rebase_in_progress ||
!stat(git_path("MERGE_MSG"), &st)) {
if (state->branch)
status_printf_ln(s, color,
_("You are currently rebasing branch
'%s' on '%s'."),
@@ -1042,25 +1133,17 @@ static void show_rebase_in_progress(struct wt_status *s,
status_printf_ln(s, color,
_("You are currently rebasing."));
if (s->hints) {
-
[RFC/PATCH 2/2] status: fix tests to handle new verbose git status during rebase
Signed-off-by: Guillaume Pagès --- t/t7512-status-help.sh | 227 ++--- 1 file changed, 213 insertions(+), 14 deletions(-) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 68ad2d7..242124f 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -134,9 +134,13 @@ test_expect_success 'prepare for rebase_i_conflicts' ' test_expect_success 'status during rebase -i when conflicts unresolved' ' test_when_finished "git rebase --abort" && ONTO=$(git rev-parse --short rebase_i_conflicts) && + LAST_COMMIT=$(git rev-parse rebase_i_conflicts_second) && test_must_fail git rebase -i rebase_i_conflicts && cat >expectedexpected expected
[PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation
> On 06/03/2015 07:50 AM Torsten Bögershausen wrote > > > +CONFIGURATION > > +- > > + > > +am.keepcr:: > > +If true, git-am will call git-mailsplit for patches in mbox format > > +with parameter '--keep-cr'. In this case git-mailsplit will > > +not remove `\r` from lines ending with `\r\n`. Can be overridden > > +by giving '--no-keep-cr' from the command line. > (This documentation assumes that am.keepcr is true) > Would it be clearer to put the "overridden" into one line and write like > this: > > Can be overridden by giving '--no-keep-cr' or '--keep-cr' from the command > line. Yes I agree, or maybe: '--keep-cr' and '--no-keep-cr' take precedence over this variable. Actually, I don't know if we need to write it (as Paul Tan suggested in the previous version of this patch for the threeway option http://article.gmane.org/gmane.comp.version-control.git/270150) I checked the documentation of different commands. From what I've seen, such indications either does not appear or are right after the text. I agree that it's a good idea, but for the sake of consistency, I'd rather use one of these two format as long as it's ok for you. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFCv3 2/2] git rebase -i: warn about removed commits
> Matthieu Moy writes: > > Ideally, you would check the list of commits displayed too. If the > > commits sha1 are stable, this should be easy to do. If it's too hard to > > test, I'd say its not worth the trouble, but others may disagree. > > Originally I chose not to check if the SHA-1 were corrects since > check_commits was called right after expand_todo_ids and I thought > that expand_todo_ids checked them, but from what I understand, it > doesn't seem to check if the SHA-1 are commits, I could be wrong > though. Ignore this email, I completely misunderstood the email I was responding to. (Mailer that doesn't show the quotes by default) Rémi -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP 1/8] wrapper: implement xopen()
On Thu, May 28, 2015 at 5:53 AM, Jeff King wrote:
> On Wed, May 27, 2015 at 09:03:47PM +0200, Torsten Bögershausen wrote:
>
>> The original open can take 2 or 3 parameters, how about this:
>> int xopen(const char *path, int oflag, ... )
>> {
>> va_list params;
>> int mode;
>> int fd;
>>
>> va_start(params, oflag);
>> mode = va_arg(params, int);
>> va_end(params);
>>
>> fd = open(path, oflag, mode);
>
> Don't you need a conditional on pulling the mode arg off the stack
> (i.e., if O_CREAT is in the flags)?
Yeah, we do, as va_arg()'s behavior is undefined if we do not have the
next argument.
The POSIX spec[1] only mentions O_CREAT as requiring the extra
argument, so I guess we'll only need to check for that.
[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
Thanks,
Paul
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] log: diagnose empty HEAD more clearly
On Tue, Jun 02, 2015 at 11:48:30PM -0700, Junio C Hamano wrote:
> I am kind of surprised after reading these two threads that my
> take on this issue has changed over time, as my knee-jerk
> reaction before reading them was the opposite, something
> along the lines of "This is only immediately after 'git init'; why
> bother?". Or depending on my mood, that "How stupid do you
> have to be..." sounds exactly like a message I may advocate
> for us to send. Perhaps I grew more bitter with age.
I think it is one of those cases where the message makes sense when you
know how git works. But a new user who does not even know what "HEAD" or
a ref actually is may find it a bit confusing. Saying "we can't run
git-log, your repository empty!" may seem redundantly obvious even to
such a new user. But saying "bad revision 'HEAD'" may leave them saying
"eh, what is HEAD and why is it bad?".
> Jokes aside, I wonder why we did not pursue that "default to
> HEAD only when HEAD points at a branch that exists"
> approach, with the definition of "exists" being "either a file
> exists under the $GIT_DIR/refs/heads directory with any
> (including corrupt) contents, or an entry in the packed refs
> file exists with any (including corrupt) contents". With or
> without the error exit and warning, that sounds like a very
> sensible way to set the default, at least to me.
My concern there would be risk of regression. I.e., that we would take
some case which used to error out and turn it into a silent noop. So I'd
prefer to keep the behavior the same, and just modify the error code
path. The end result is pretty similar to the user, because we obviously
cannot produce any actual output either way.
Something like:
-- >8 --
Subject: log: diagnose empty HEAD more clearly
If you init or clone an empty repository, the initial
message from running "git log" is not very friendly:
$ git init
Initialized empty Git repository in /home/peff/foo/.git/
$ git log
fatal: bad default revision 'HEAD'
Let's detect this situation and write a more friendly
message:
$ git log
fatal: default revision 'HEAD' points to an unborn branch 'master'
We also detect the case that 'HEAD' points to a broken ref;
this should be even less common, but is easy to see. Note
that we do not diagnose all possible cases. We rely on
resolve_ref, which means we do not get information about
complex cases. E.g., "--default master" would use dwim_ref
to find "refs/heads/master", but we notice only that
"master" does not exist. Similarly, a complex sha1
expression like "--default HEAD^2" will not resolve as a
ref.
But that's OK. We fall back to the existing error message in
those cases, and they are unlikely to be used anyway.
Catching an empty or broken "HEAD" improves the common case,
and the other cases are not regressed.
Signed-off-by: Jeff King
---
I'm not a big fan of the new error message, either, just because the
term "unborn" is also likely to be confusing to new users.
We can also probably get rid of mentioning "HEAD" completely. It is
always the default unless the user has overridden it explicitly. So we
could go with something like:
fatal: your default branch 'master' does not contain any commits
or something. I dunno. Bikeshed colors welcome. Adding:
advise("try running 'git commit'");
is probably too pedantic. ;)
revision.c | 19 ++-
t/t4202-log.sh | 11 +++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/revision.c b/revision.c
index 7ddbaa0..775df8b 100644
--- a/revision.c
+++ b/revision.c
@@ -2170,6 +2170,23 @@ static int handle_revision_pseudo_opt(const char
*submodule,
return 1;
}
+static void NORETURN diagnose_missing_default(const char *def)
+{
+ unsigned char sha1[20];
+ int flags;
+ const char *refname;
+
+ refname = resolve_ref_unsafe(def, 0, sha1, &flags);
+ if (!(flags & REF_ISSYMREF))
+ die(_("bad default revision '%s'"), def);
+ if (flags & REF_ISBROKEN)
+ die(_("default revision '%s' points to a broken branch"), def);
+
+ skip_prefix(refname, "refs/heads/", &refname);
+ die(_("default revision '%s' points to an unborn branch '%s'"),
+ def, refname);
+}
+
/*
* Parse revision information, filling in the "rev_info" structure,
* and removing the used arguments from the argument list.
@@ -2299,7 +2316,7 @@ int setup_revisions(int argc, const char **argv, struct
rev_info *revs, struct s
struct object *object;
struct object_context oc;
if (get_sha1_with_context(revs->def, 0, sha1, &oc))
- die("bad default revision '%s'", revs->def);
+ diagnose_missing_default(revs->def);
object = get_reference(revs, revs->def, sha1, 0);
add_pending_object_with_mode(revs, object, revs->def, oc.mode);
}
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 1b2e981..71d832
Re: [PATCH/WIP 6/8] am: extract patch, message and authorship with git-mailinfo
On Thu, May 28, 2015 at 4:44 AM, Junio C Hamano wrote:
> Paul Tan writes:
>
>> @@ -17,6 +34,10 @@ struct am_state {
>> struct strbuf dir;/* state directory path */
>> int cur; /* current patch number */
>> int last; /* last patch number */
>> + struct strbuf msg;/* commit message */
>> + struct strbuf author_name;/* commit author's name */
>> + struct strbuf author_email; /* commit author's email */
>> + struct strbuf author_date;/* commit author's date */
>> int prec; /* number of digits in patch filename */
>> };
>
> I always get suspicious when structure fields are overly commented,
> wondering if it is a sign of naming fields poorly. All of the above
> fields look quite self-explanatory and I am not sure if it is worth
> having these comments, spending efforts to type SP many times to
> align them and all.
Okay, I'll take Jeff's suggestion to organize them into blocks.
>> +static int read_author_script(struct am_state *state)
>> +{
>> + char *value;
>> + struct strbuf sb = STRBUF_INIT;
>> + const char *filename = am_path(state, "author-script");
>> + FILE *fp = fopen(filename, "r");
>> + if (!fp) {
>> + if (errno == ENOENT)
>> + return 0;
>> + die(_("could not open '%s' for reading"), filename);
>
> Hmph, do we want to report with die_errno()?
Yes, we do.
>> + }
>> +
>> + if (strbuf_getline(&sb, fp, '\n'))
>> + return -1;
>> + if (!skip_prefix(sb.buf, "GIT_AUTHOR_NAME=", (const char**) &value))
>
> This cast is unfortunate; can't "value" be of "const char *" type?
We can't, because sq_dequote() modifies the string directly. I would
think casting from non-const to const is safer than the other way
around.
Thanks,
Paul
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP 6/8] am: extract patch, message and authorship with git-mailinfo
On Thu, May 28, 2015 at 6:13 AM, Junio C Hamano wrote:
> Paul Tan writes:
>
>> +static const char *msgnum(const struct am_state *state)
>> +{
>> + static struct strbuf fmt = STRBUF_INIT;
>> + static struct strbuf sb = STRBUF_INIT;
>> +
>> + strbuf_reset(&fmt);
>> + strbuf_addf(&fmt, "%%0%dd", state->prec);
>> +
>> + strbuf_reset(&sb);
>> + strbuf_addf(&sb, fmt.buf, state->cur);
>
> Hmph, wouldn't ("%*d", state->prec, state->cur) work or am I missing
> something?
Yeah, I think it would. I justs didn't know about the existence of '*'.
Thanks,
Paul
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFCv3 2/2] git rebase -i: warn about removed commits
Matthieu Moy writes: > Ideally, you would check the list of commits displayed too. If the > commits sha1 are stable, this should be easy to do. If it's too hard to > test, I'd say its not worth the trouble, but others may disagree. Originally I chose not to check if the SHA-1 were corrects since check_commits was called right after expand_todo_ids and I thought that expand_todo_ids checked them, but from what I understand, it doesn't seem to check if the SHA-1 are commits, I could be wrong though. Rémi -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
