* [PATCH] documentation fix: git difftool uses diff tools, not merge tools.
From: Thomas Hochstein @ 2011-11-14 22:55 UTC (permalink / raw)
To: git; +Cc: Thomas Hochstein
Let the documentation for -t list valid *diff* tools,
not valid *merge* tools.
Signed-off-by: Thomas Hochstein <thh@inter.net>
---
Documentation/git-difftool.txt | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index a03515f..19d473c 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -31,7 +31,7 @@ OPTIONS
-t <tool>::
--tool=<tool>::
Use the diff tool specified by <tool>.
- Valid merge tools are:
+ Valid diff tools are:
araxis, bc3, diffuse, emerge, ecmerge, gvimdiff, kdiff3,
kompare, meld, opendiff, p4merge, tkdiff, vimdiff and xxdiff.
+
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH, v2] tag: implement --[no-]strip option
From: Junio C Hamano @ 2011-11-14 23:04 UTC (permalink / raw)
To: Kirill A. Shutemov; +Cc: git
In-Reply-To: <20111114223903.GA5751@shutemov.name>
"Kirill A. Shutemov" <kirill@shutemov.name> writes:
>> > @@ -356,7 +362,7 @@ static void create_tag(const unsigned char *object, const char *tag,
>> >
>> > if (!is_null_sha1(prev))
>> > write_tag_body(fd, prev);
>> > - else
>> > + else if (opt->strip)
>> > write_or_die(fd, _(tag_template), strlen(_(tag_template)));
>>
>> Why are you not writing template when no strip is done? (Not an objection
>> disguised as a rhetorical question, but a question).
>>
>> The user who typed "tag -a v1.2.3 HEAD" that spawns an editor would still
>> find it useful to have commented instructions, once we start filling the
>> template with more useful information that is customized for the
>> situation (e.g. "git show -s --oneline" output), no?
>
> Yes. But in this case commented instructions will not be stripped and they
> will go to the message. I think user will be confused.
>
> We can show show some instructions before spawning the editor. What do
> you think?
My knee-jerk reaction is that it would be worse than what your patch
does. I'd say we'd start from your patch and see how users of 'next'
reacts while the topic is cooking.
^ permalink raw reply
* [PATCH 4/4] refresh_index: notice typechanges in output
From: Jeff King @ 2011-11-14 22:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlos Martín Nieto, git
In-Reply-To: <20111114225056.GA27370@sigill.intra.peff.net>
If a file changes type and a porcelain updates the index, we
will print "M file". Instead, let's be more specific and
print "T file", which matches actual diff and status output.
The plumbing version remains "needs update" for historical
compatibility.
Signed-off-by: Jeff King <peff@peff.net>
---
The "changed" flag comes from refresh_cache_ent, which in turn gets it
from ie_modified_stat. The one hesitation I have is that intent-to-add
entries get the TYPE_CHANGED flag set, which means they will get a "T"
output. Whereas I actually think "M" is a little more sensible.
For "git reset", I'm not sure if it matters, since resetting the index
will always drop such an entry anyway. But you can see it with:
git add -N file
git add --refresh -v other
read-cache.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 83fb19c..0e17add 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1107,14 +1107,17 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;
const char *modified_fmt;
const char *deleted_fmt;
+ const char *typechange_fmt;
const char *unmerged_fmt;
modified_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
deleted_fmt = (in_porcelain ? "D\t%s\n" : "%s: needs update\n");
+ typechange_fmt = (in_porcelain ? "T\t%s\n" : "%s needs update\n");
unmerged_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");
for (i = 0; i < istate->cache_nr; i++) {
struct cache_entry *ce, *new;
int cache_errno = 0;
+ int changed = 0;
ce = istate->cache[i];
if (ignore_submodules && S_ISGITLINK(ce->ce_mode))
@@ -1135,7 +1138,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
if (pathspec && !match_pathspec(pathspec, ce->name, strlen(ce->name), 0, seen))
continue;
- new = refresh_cache_ent(istate, ce, options, &cache_errno, NULL);
+ new = refresh_cache_ent(istate, ce, options, &cache_errno, &changed);
if (new == ce)
continue;
if (!new) {
@@ -1151,6 +1154,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
if (quiet)
continue;
show_file((cache_errno == ENOENT ? deleted_fmt :
+ changed & TYPE_CHANGED ? typechange_fmt :
modified_fmt),
ce->name, in_porcelain, &first, header_msg);
has_errors = 1;
--
1.7.7.3.8.g38efa
^ permalink raw reply related
* [PATCH 3/4] read-cache: let refresh_cache_ent pass up changed flags
From: Jeff King @ 2011-11-14 22:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlos Martín Nieto, git
In-Reply-To: <20111114225056.GA27370@sigill.intra.peff.net>
This will enable refresh_cache to differentiate more cases
of modification (such as typechange) when telling the user
what isn't fresh.
Signed-off-by: Jeff King <peff@peff.net>
---
There's only a single layer of call-stack to modify, and there's only
one other caller, so it turned out to be a pretty minor change.
read-cache.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 046cf7e..83fb19c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1001,7 +1001,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
*/
static struct cache_entry *refresh_cache_ent(struct index_state *istate,
struct cache_entry *ce,
- unsigned int options, int *err)
+ unsigned int options, int *err,
+ int *changed_ret)
{
struct stat st;
struct cache_entry *updated;
@@ -1033,6 +1034,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
}
changed = ie_match_stat(istate, ce, &st, options);
+ if (changed_ret)
+ *changed_ret = changed;
if (!changed) {
/*
* The path is unchanged. If we were told to ignore
@@ -1132,7 +1135,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
if (pathspec && !match_pathspec(pathspec, ce->name, strlen(ce->name), 0, seen))
continue;
- new = refresh_cache_ent(istate, ce, options, &cache_errno);
+ new = refresh_cache_ent(istate, ce, options, &cache_errno, NULL);
if (new == ce)
continue;
if (!new) {
@@ -1161,7 +1164,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really)
{
- return refresh_cache_ent(&the_index, ce, really, NULL);
+ return refresh_cache_ent(&the_index, ce, really, NULL, NULL);
}
static int verify_hdr(struct cache_header *hdr, unsigned long size)
--
1.7.7.3.8.g38efa
^ permalink raw reply related
* [PATCH 2/4] refresh_index: mark deletions in porcelain output
From: Jeff King @ 2011-11-14 22:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlos Martín Nieto, git
In-Reply-To: <20111114225056.GA27370@sigill.intra.peff.net>
If you have a deleted file and a porcelain refreshes the
cache, we print:
Unstaged changes after reset:
M file
This is technically correct, in that the file is modified,
but it's friendlier to the user if we further differentiate
the case of a deleted file (especially because this output
looks a lot like "diff --name-status", which would also make
the distinction).
The plumbing version stays as "needs update" for historical
compatibility.
Signed-off-by: Jeff King <peff@peff.net>
---
read-cache.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index eb3aae3..046cf7e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1103,9 +1103,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
int in_porcelain = (flags & REFRESH_IN_PORCELAIN);
unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;
const char *modified_fmt;
+ const char *deleted_fmt;
const char *unmerged_fmt;
modified_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
+ deleted_fmt = (in_porcelain ? "D\t%s\n" : "%s: needs update\n");
unmerged_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");
for (i = 0; i < istate->cache_nr; i++) {
struct cache_entry *ce, *new;
@@ -1145,7 +1147,9 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
}
if (quiet)
continue;
- show_file(modified_fmt, ce->name, in_porcelain, &first, header_msg);
+ show_file((cache_errno == ENOENT ? deleted_fmt :
+ modified_fmt),
+ ce->name, in_porcelain, &first, header_msg);
has_errors = 1;
continue;
}
--
1.7.7.3.8.g38efa
^ permalink raw reply related
* [PATCH 1/4] refresh_index: rename format variables
From: Jeff King @ 2011-11-14 22:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlos Martín Nieto, git
In-Reply-To: <20111114225056.GA27370@sigill.intra.peff.net>
When refreshing the index, for modified (or unmerged) files
we will print "needs update" (or "needs merge") for plumbing,
or a "diff --name-status"-ish line for porcelain.
The variables holding which type of message to show are
named after the plumbing messages. However, as we begin to
differentiate more cases at the porcelain level (with the
plumbing message stayin the same), that naming scheme will
become awkward.
Instead, name the variables after which case we found
(modified or unmerged), not what we will output.
Signed-off-by: Jeff King <peff@peff.net>
---
read-cache.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 5790a91..eb3aae3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1102,11 +1102,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
int first = 1;
int in_porcelain = (flags & REFRESH_IN_PORCELAIN);
unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;
- const char *needs_update_fmt;
- const char *needs_merge_fmt;
+ const char *modified_fmt;
+ const char *unmerged_fmt;
- needs_update_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
- needs_merge_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");
+ modified_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
+ unmerged_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");
for (i = 0; i < istate->cache_nr; i++) {
struct cache_entry *ce, *new;
int cache_errno = 0;
@@ -1122,7 +1122,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
i--;
if (allow_unmerged)
continue;
- show_file(needs_merge_fmt, ce->name, in_porcelain, &first, header_msg);
+ show_file(unmerged_fmt, ce->name, in_porcelain, &first, header_msg);
has_errors = 1;
continue;
}
@@ -1145,7 +1145,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
}
if (quiet)
continue;
- show_file(needs_update_fmt, ce->name, in_porcelain, &first, header_msg);
+ show_file(modified_fmt, ce->name, in_porcelain, &first, header_msg);
has_errors = 1;
continue;
}
--
1.7.7.3.8.g38efa
^ permalink raw reply related
* Re: reset reports file as modified when it's in fact deleted
From: Jeff King @ 2011-11-14 22:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlos Martín Nieto, git
In-Reply-To: <7vhb2aqjwu.fsf@alter.siamese.dyndns.org>
On Fri, Nov 11, 2011 at 04:11:29PM -0800, Junio C Hamano wrote:
> > I'm happy to make that change. What do you think of the feature overall?
>
> The "feature" is more or less "Meh" to me; I do not mind differentiating M
> and D there because the necessary information is already there in the
> codepath, but if we were to really do that, then the variables must be
> renamed. We shouldn't name them after "you must do this at the plumbing
> level" like we have done so far, and instead use "the path is in this
> state". This is even more so if we were to further split a single "state"
> that plumbing layer considers the same and gives the same "needs X" to
> different states that Porcelains would give different messages in the
> future.
I admit I don't care all that much either, but it's easy to do, and I
think it is less surprising to users. Patches are below.
> > And should typechanges also be handled here (it's no extra work for git
> > to detect them; we just have to pass the TYPE_CHANGED flag back up the
> > stack)?
>
> As long as "pass ... back up the stack" does not result in too much
> ugliness in the code, I am OK with it.
It ended up pretty simple, but I split it out from the deletion case so
you can see it more clearly (and can drop the latter half of the series
if we don't want it).
[1/4]: refresh_index: rename format variables
[2/4]: refresh_index: mark deletions in porcelain output
[3/4]: read-cache: let refresh_cache_ent pass up changed flags
[4/4]: refresh_index: notice typechanges in output
(If I were sure we wanted the typechange output, I think a more
reasonable ordering would be 1, then 3, then squash 2 and 4 into a
single patch. But see my comment in 4/4).
-Peff
^ permalink raw reply
* Re: [PATCH, v2] tag: implement --[no-]strip option
From: Kirill A. Shutemov @ 2011-11-14 22:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vipmmibx4.fsf@alter.siamese.dyndns.org>
On Mon, Nov 14, 2011 at 02:20:23PM -0800, Junio C Hamano wrote:
> "Kirill A. Shutemov" <kirill@shutemov.name> writes:
>
> > From: "Kirill A. Shutemov" <kirill@shutemov.name>
> >
> > --strip::
> > Remove from tag message lines staring with '#', trailing spaces
> > from every line and empty lines from the beginning and end.
> > Enabled by default. Use --no-strip to overwrite the behaviour.
> >
> > --no-strip is useful if you want to take a tag message as-is, without
> > any stripping.
>
> That is not a commit log message ;-)
Ok, I'll fix.
> > Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
>
> > diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> > index c83cb13..dbb76a6 100644
> > --- a/Documentation/git-tag.txt
> > +++ b/Documentation/git-tag.txt
> > @@ -99,6 +99,11 @@ OPTIONS
> > Implies `-a` if none of `-a`, `-s`, or `-u <key-id>`
> > is given.
> >
> > +--strip::
> > + Remove from tag message lines staring with '#', trailing spaces
> > + from every line and empty lines from the beginning and end.
> > + Enabled by default. Use --no-strip to overwrite the behaviour.
>
> s/overwrite/override/; or replace the last sentence with "With
> `--no-strip`, the tag message given by the user is used as-is".
Ok.
> > diff --git a/builtin/tag.c b/builtin/tag.c
> > index 9b6fd95..05a1fd4 100644
> > --- a/builtin/tag.c
> > +++ b/builtin/tag.c
> > ...
> > @@ -356,7 +362,7 @@ static void create_tag(const unsigned char *object, const char *tag,
> >
> > if (!is_null_sha1(prev))
> > write_tag_body(fd, prev);
> > - else
> > + else if (opt->strip)
> > write_or_die(fd, _(tag_template), strlen(_(tag_template)));
>
> Why are you not writing template when no strip is done? (Not an objection
> disguised as a rhetorical question, but a question).
>
> The user who typed "tag -a v1.2.3 HEAD" that spawns an editor would still
> find it useful to have commented instructions, once we start filling the
> template with more useful information that is customized for the
> situation (e.g. "git show -s --oneline" output), no?
Yes. But in this case commented instructions will not be stripped and they
will go to the message. I think user will be confused.
We can show show some instructions before spawning the editor. What do
you think?
> > @@ -423,8 +430,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> > const char *object_ref, *tag;
> > struct ref_lock *lock;
> >
> > - int annotate = 0, sign = 0, force = 0, lines = -1,
> > - list = 0, delete = 0, verify = 0;
> > + struct create_tag_options opt = {
> > + .sign = 0,
> > + .strip = 1,
> > + };
>
> Avoid doing this. Even though these C99 initializers are nicer and more
> readable way to write this, we try to be gentle to people with older
> compilers that do not grok the syntax.
It's sad. Do you have a list of compilers which are important for the
project?
> Except for the above minor nits, the patch basically looks good. Please
> hold onto it and resubmit after 1.7.8 final ships.
Thanks.
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH, v2] tag: implement --[no-]strip option
From: Junio C Hamano @ 2011-11-14 22:20 UTC (permalink / raw)
To: Kirill A. Shutemov; +Cc: git
In-Reply-To: <1321307019-5557-1-git-send-email-kirill@shutemov.name>
"Kirill A. Shutemov" <kirill@shutemov.name> writes:
> From: "Kirill A. Shutemov" <kirill@shutemov.name>
>
> --strip::
> Remove from tag message lines staring with '#', trailing spaces
> from every line and empty lines from the beginning and end.
> Enabled by default. Use --no-strip to overwrite the behaviour.
>
> --no-strip is useful if you want to take a tag message as-is, without
> any stripping.
That is not a commit log message ;-)
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index c83cb13..dbb76a6 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -99,6 +99,11 @@ OPTIONS
> Implies `-a` if none of `-a`, `-s`, or `-u <key-id>`
> is given.
>
> +--strip::
> + Remove from tag message lines staring with '#', trailing spaces
> + from every line and empty lines from the beginning and end.
> + Enabled by default. Use --no-strip to overwrite the behaviour.
s/overwrite/override/; or replace the last sentence with "With
`--no-strip`, the tag message given by the user is used as-is".
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 9b6fd95..05a1fd4 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> ...
> @@ -356,7 +362,7 @@ static void create_tag(const unsigned char *object, const char *tag,
>
> if (!is_null_sha1(prev))
> write_tag_body(fd, prev);
> - else
> + else if (opt->strip)
> write_or_die(fd, _(tag_template), strlen(_(tag_template)));
Why are you not writing template when no strip is done? (Not an objection
disguised as a rhetorical question, but a question).
The user who typed "tag -a v1.2.3 HEAD" that spawns an editor would still
find it useful to have commented instructions, once we start filling the
template with more useful information that is customized for the
situation (e.g. "git show -s --oneline" output), no?
> @@ -423,8 +430,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> const char *object_ref, *tag;
> struct ref_lock *lock;
>
> - int annotate = 0, sign = 0, force = 0, lines = -1,
> - list = 0, delete = 0, verify = 0;
> + struct create_tag_options opt = {
> + .sign = 0,
> + .strip = 1,
> + };
Avoid doing this. Even though these C99 initializers are nicer and more
readable way to write this, we try to be gentle to people with older
compilers that do not grok the syntax.
Except for the above minor nits, the patch basically looks good. Please
hold onto it and resubmit after 1.7.8 final ships.
Thanks.
^ permalink raw reply
* Re: [RFC] deprecating and eventually removing "git relink"?
From: Junio C Hamano @ 2011-11-14 22:08 UTC (permalink / raw)
To: Jeff King; +Cc: Simon Brenner, git
In-Reply-To: <20111114202522.GA26269@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Nov 14, 2011 at 12:18:25PM -0800, Junio C Hamano wrote:
>
>> > Yes, I think that is sensible. I'm not sure there is even any core git
>> > code to be written. I think a wrapper that does the following would
>> > probably work:
>>
>> I agree with your outline, which I find is in line with what I had in mind
>> in the message Miles responded.
>>
>> The approach is different from what Miles alluded to, which is to have
>> "clients" create objects in the "central" place in the first place,
>> though.
>
> It seems to me that is simply an optimization that can come later.
I did not mean "it is wrong because it does not match what Miles said" by
that. In fact, I think it is a better approach to put things in clients
first and consolidating possible duplicates at the central one purely as
optimization, and I do not necessarily see "write to central from the
beginning" as a particularly good "optimization".
^ permalink raw reply
* Re: Compile warnings
From: Frans Klaver @ 2011-11-14 21:55 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git@vger.kernel.org, Junio C Hamano
In-Reply-To: <CACBZZX6d_ykc9CbN7H-ACWNUxACb9+TH4ffJ-+9T=SEv6Ai0Ug@mail.gmail.com>
On Mon, 14 Nov 2011 17:58:20 +0100, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Mon, Nov 14, 2011 at 15:55, Frans Klaver <fransklaver@gmail.com>
> wrote:
>> Every now and then I see an 'unused result' warning come by during
>> building.
>> What is the general attitude towards these warnings? Remove them (by
>> properly checking)? Or leave them be as a kind of documentation -- we
>> know
>> we're ignoring the info, but it's good to be reminded?
>
> Under what OS / version and compiler / version and what's the warning?
> Paste the full warning(s) you get verbatim.
This question was triggered by
warning: ignoring return value of ‘fwrite’, declared with attribute
warn_unused_result
appearing in diff.c, graph.c, grep.c and several others. I'm using gentoo
linux, gcc 4.5.3.
So the specific question would be, do these fwrites need to be checked?
^ permalink raw reply
* Re: [PATCH] svn: Create config options for common args
From: Eric Wong @ 2011-11-14 21:46 UTC (permalink / raw)
To: Ted Percival; +Cc: git
In-Reply-To: <1321302310-28793-1-git-send-email-ted.percival@quest.com>
Ted Percival <ted.percival@quest.com> wrote:
> Documentation/git-svn.txt | 10 ++++++++++
Thanks, documentation part is very much appreciated
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -21,6 +21,9 @@ $Git::SVN::default_repo_id = 'svn';
> $Git::SVN::default_ref_id = $ENV{GIT_SVN_ID} || 'git-svn';
> $Git::SVN::Ra::_log_window_size = 100;
> $Git::SVN::_minimize_url = 'unset';
> +$Git::SVN::_localtime = Git::config_bool('svn.localtime');
> +$Git::SVN::_add_author_from = Git::config_bool('svn.addAuthorFrom');
> +$Git::SVN::_use_log_author = Git::config_bool('svn.useLogAuthor');
I don't think these are needed. The read_git_config() function /should/
be checking all GIT_CONFIG equivalents of the Getopt::Long option specs.
Can you verify? Thanks.
^ permalink raw reply
* [PATCH, v2] tag: implement --[no-]strip option
From: Kirill A. Shutemov @ 2011-11-14 21:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Kirill A. Shutemov
From: "Kirill A. Shutemov" <kirill@shutemov.name>
--strip::
Remove from tag message lines staring with '#', trailing spaces
from every line and empty lines from the beginning and end.
Enabled by default. Use --no-strip to overwrite the behaviour.
--no-strip is useful if you want to take a tag message as-is, without
any stripping.
Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
Documentation/git-tag.txt | 5 +++++
builtin/tag.c | 41 ++++++++++++++++++++++++++++-------------
2 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index c83cb13..dbb76a6 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -99,6 +99,11 @@ OPTIONS
Implies `-a` if none of `-a`, `-s`, or `-u <key-id>`
is given.
+--strip::
+ Remove from tag message lines staring with '#', trailing spaces
+ from every line and empty lines from the beginning and end.
+ Enabled by default. Use --no-strip to overwrite the behaviour.
+
<tagname>::
The name of the tag to create, delete, or describe.
The new tag name must pass all checks defined by
diff --git a/builtin/tag.c b/builtin/tag.c
index 9b6fd95..05a1fd4 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -319,8 +319,14 @@ static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)
return 0;
}
+struct create_tag_options {
+ unsigned int message;
+ unsigned int sign;
+ unsigned int strip;
+};
+
static void create_tag(const unsigned char *object, const char *tag,
- struct strbuf *buf, int message, int sign,
+ struct strbuf *buf, struct create_tag_options *opt,
unsigned char *prev, unsigned char *result)
{
enum object_type type;
@@ -345,7 +351,7 @@ static void create_tag(const unsigned char *object, const char *tag,
if (header_len > sizeof(header_buf) - 1)
die(_("tag header too big."));
- if (!message) {
+ if (!opt->message) {
int fd;
/* write the template message before editing: */
@@ -356,7 +362,7 @@ static void create_tag(const unsigned char *object, const char *tag,
if (!is_null_sha1(prev))
write_tag_body(fd, prev);
- else
+ else if (opt->strip)
write_or_die(fd, _(tag_template), strlen(_(tag_template)));
close(fd);
@@ -367,14 +373,15 @@ static void create_tag(const unsigned char *object, const char *tag,
}
}
- stripspace(buf, 1);
+ if (opt->strip)
+ stripspace(buf, 1);
- if (!message && !buf->len)
+ if (opt->message && !buf->len)
die(_("no tag message?"));
strbuf_insert(buf, 0, header_buf, header_len);
- if (build_tag_object(buf, sign, result) < 0) {
+ if (build_tag_object(buf, opt->sign, result) < 0) {
if (path)
fprintf(stderr, _("The tag message has been left in %s\n"),
path);
@@ -423,8 +430,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
const char *object_ref, *tag;
struct ref_lock *lock;
- int annotate = 0, sign = 0, force = 0, lines = -1,
- list = 0, delete = 0, verify = 0;
+ struct create_tag_options opt = {
+ .sign = 0,
+ .strip = 1,
+ };
+
+ int annotate = 0, force = 0, lines = -1, list = 0,
+ delete = 0, verify = 0;
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
@@ -442,7 +454,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_CALLBACK('m', "message", &msg, "message",
"tag message", parse_msg_arg),
OPT_FILENAME('F', "file", &msgfile, "read message from file"),
- OPT_BOOLEAN('s', "sign", &sign, "annotated and GPG-signed tag"),
+ OPT_BOOLEAN('s', "sign", &opt.sign, "annotated and GPG-signed tag"),
+ OPT_BOOLEAN(0, "strip", &opt.strip,
+ "turn off tag message stripping"),
OPT_STRING('u', "local-user", &keyid, "key-id",
"use another key to sign the tag"),
OPT__FORCE(&force, "replace the tag if exists"),
@@ -462,10 +476,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
if (keyid) {
- sign = 1;
+ opt.sign = 1;
set_signingkey(keyid);
}
- if (sign)
+ if (opt.sign)
annotate = 1;
if (argc == 0 && !(delete || verify))
list = 1;
@@ -523,9 +537,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
else if (!force)
die(_("tag '%s' already exists"), tag);
+ opt.message = msg.given || msgfile;
+
if (annotate)
- create_tag(object, tag, &buf, msg.given || msgfile,
- sign, prev, object);
+ create_tag(object, tag, &buf, &opt, prev, object);
lock = lock_any_ref_for_update(ref.buf, prev, 0);
if (!lock)
--
1.7.7.2
^ permalink raw reply related
* Re: Git: Unexpected behaviour?
From: Jakub Narebski @ 2011-11-14 21:33 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git, Junio C Hamano, J.V.
In-Reply-To: <CAOeW2eEUbvd0eJHjNfbvi9QnDiUO=mFA9rrKsjv8Yu0_QiPgSw@mail.gmail.com>
Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
> On Sat, Nov 12, 2011 at 11:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > "J.V." <jvsrvcs@gmail.com> writes:
> >
> > > OK so "work tree" is a new term for me. I thought we were in isolated
> > > sandboxes called "branches" and changes made in a branch would stay in
> > > that branch regardless.
That would be the default and only solution if each branch was checked
out to a separate working directory.
You can do that in git using git-new-worktree script from contrib.
> > Do not think of "branches" as isolated _sandboxes_.
> >
> > Rather, "branches" are where the independent states are to be _recorded_.
Branches are lines of development, and are about _comitted_ changes.
This means that when switching branches "in place", un-committed
changes are not on any branch.
> I think I was confused about this when learning Git too. I friend of
> mine made the following argument, which I agree with and which I haven
> seen on the list before:
>
> Either you want the modifications to stay on the branch, or you want
> them to carry over to the branch you are checking out. In the former
> case, you would want Git to fail if there are modifications (that you
> might have forgotten you made). In the latter case, you would want
> "git checkout -m". The current behavior is somewhere in between. It is
> not clear to me if there is a use case where the current behavior is
> better (from the user's point of view) than either failing or
> "checkout -m".
The "checkout -m" behavior is unsafe; you can land in a state where it
would be difficult to revert, and could lose your changes. The
default behavior of switching branches is to carry over changes if it
is safe to do so.
> It is obviously too late to change this now, though.
Well, we could in theory add knob that would stash changes when
switching to branch, and unstash when switching to branch.
--
Jakub Narębski
^ permalink raw reply
* Re: Git: Unexpected behaviour?
From: Junio C Hamano @ 2011-11-14 20:55 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git, J.V.
In-Reply-To: <CAOeW2eEUbvd0eJHjNfbvi9QnDiUO=mFA9rrKsjv8Yu0_QiPgSw@mail.gmail.com>
Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
> Either you want the modifications to stay on the branch, or you want
> them to carry over to the branch you are checking out. In the former
> case, you would want Git to fail if there are modifications (that you
> might have forgotten you made). In the latter case, you would want
> "git checkout -m". The current behavior is somewhere in between. It is
> not clear to me if there is a use case where the current behavior is
> better (from the user's point of view) than either failing or
> "checkout -m".
Current behaviour is deliberately made safe because "checkout -m" may end
up forcing you to resolve a 3-way conflict you may not be prepared to do
correctly at your first attempt. Stopping and refusing to check out the
other branch gives you the choice to create a temporary stash-away commit
either on a current branch, or a temporary branch you create with "git
checkout -b temp && git commit" before switching to the target branch to
attempt to port the change over with "git cherry-pick @{-1}", which you
_can_ redo if you screw up conflict resolution and want to start over.
If you are confident that your local changes are trivial that you can
reproduce it even if you screw up your conflict resolution attempt, then
you can choose to run "checkout -m". If we made it the default, you will
lose this safety.
On the other hand, if you do *not* want to carry over the change when
checking out a different branch, you can easily stash-away the changes.
^ permalink raw reply
* Re: feature request: git annotate -w like git blame -w
From: Junio C Hamano @ 2011-11-14 20:49 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Raoul Bhatia [IPAX], git
In-Reply-To: <m3sjlun4o9.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
> "Raoul Bhatia [IPAX]" <r.bhatia@ipax.at> writes:
>
>> is it possible to add a "git annotate -w" option like git blame has?
>
> Why not use "git blame -c -w"? `-c' turns on annotate-compatibile
> output.
>
> From git-annotate(1) manpage:
>
> The only difference between this command and git-blame(1) is that they use
> slightly different output formats, and this command exists only for back-
> ward compatibility to support existing scripts, and provide a more familiar
> command name for people coming from other SCM systems.
Somebody may want to compare git-blame.txt and git-annotate.txt in
Documentation/, notice that they share blame-options.txt and that the
former has accumulated option descriptions for a few more options that
should have been placed in blame-options.txt, and then prepare a patch to
improve the documentation? Hint, hint...
I *think* '-c', '-f', and '-n' are blame-only options but all others would
work with annotate (whoever is doing the patch needs to verify this,
though).
^ permalink raw reply
* Re: Compile warnings
From: Junio C Hamano @ 2011-11-14 20:36 UTC (permalink / raw)
To: Frans Klaver; +Cc: git@vger.kernel.org
In-Reply-To: <op.v4xyekju0aolir@keputer>
"Frans Klaver" <fransklaver@gmail.com> writes:
> Every now and then I see an 'unused result' warning come by during
> building. What is the general attitude towards these warnings? Remove
> them (by properly checking)? Or leave them be as a kind of
> documentation -- we know we're ignoring the info, but it's good to be
> reminded?
A callsite of a function whose return value is better checked should be
checked (e.g. not checking return from close(2) or write(2) in a non-error
codepath), but there is no strong mechanical "General attitude".
Sprinkling (void) that casts the return values all over the place makes
our code illegible, and we do not prefer it as a solution. A function
that returns a value that is useful for some callers but can be safely
ignored by others is sometimes an indication of a poor API, and for our
own code, we tend to prefer designing the API to pass optional pointer
to return value from callers that do want to use the return value (and
others that do not care about the return value pass NULL).
^ permalink raw reply
* Re: Git shouldn't allow to push a new branch called HEAD
From: Jeff King @ 2011-11-14 20:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, Daniele Segato, Git Mailing List
In-Reply-To: <7vbosejvx8.fsf@alter.siamese.dyndns.org>
On Mon, Nov 14, 2011 at 12:22:59PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > So one solution is to block fetching of remote branches called HEAD
> > (which I would be OK with). But another is...
> > ... Obviously there's a lot more to it than just tweaking the default fetch
> > refspecs. The ref lookup rules need to be changed to take this into
> > account. There was some discussion about this over the summer (under the
> > subject of possible "1.8.0" changes), but I don't think any work has
> > been done.
>
> I would say discussing and ironing out the kinks of the design counts as
> work, but I agree nobody was seriously interested in laying out a sensible
> transition plan and discussion died out before anything concrete happened.
Yeah, I should have said "...has been done since then".
> Regardless of the layout chanage, which probably is a 2.X topic, I think a
> good first step would be to start forbidding anything that ends with _?HEAD
> as a branch or tag name, on top of Michael's "enforce the refname rules more
> vigorously when a ref is created" series.
Agreed. Changing the layout is a long-term fix, and I think disallowing
HEAD is a reasonable stop-gap measure.
-Peff
^ permalink raw reply
* Re: [PATCH] tag: implement --no-strip option
From: Junio C Hamano @ 2011-11-14 20:26 UTC (permalink / raw)
To: Kirill A. Shutemov; +Cc: git, Junio C Hamano
In-Reply-To: <1321268902-2170-1-git-send-email-kirill@shutemov.name>
"Kirill A. Shutemov" <kirill@shutemov.name> writes:
> +-S::
> +--no-strip::
> + Take tag message as-is. Do not strip any comments or empty lines.
Wrong naming convention. Just introduce a boolean --strip option which is
on by default, without adding -S; use of parse-options will allow the user
to negate it from the command line with --no-strip for free.
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 9b6fd95..427d646 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -320,7 +320,7 @@ static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)
> }
>
> static void create_tag(const unsigned char *object, const char *tag,
> - struct strbuf *buf, int message, int sign,
> + struct strbuf *buf, int message, int sign, int nostrip,
Again, wrong naming convention. "int strip" would be fine but I think at
this point "message, sign, strip" tuple should become fields of "struct
create_tag_option" that is passed to this funciton.
> unsigned char *prev, unsigned char *result)
> {
> enum object_type type;
> @@ -356,7 +356,7 @@ static void create_tag(const unsigned char *object, const char *tag,
>
> if (!is_null_sha1(prev))
> write_tag_body(fd, prev);
> - else
> + else if (!nostrip)
This double negation comes only because the argument is misnamed with negation.
^ permalink raw reply
* mergetool's test for rerere seems to be incorrect
From: Erik Carstensen @ 2011-11-14 20:26 UTC (permalink / raw)
To: git
Hi,
Occasionally when I use mergetool in a conflicted state ('git status'
says it's a conflict), nothing happens.
By bisection I found that bb0a484e985ef8d9bbbbeb172b1fcf4982634bef is
the offending commit.
I have rerere.enabled unset, but I did find an empty file MERGE_RR in
.git/, which explains my problem: mergetool chooses to use rerere
because it finds the MERGE_RR file, but rerere just exits silently
because it's not enabled.
Wouldn't it make sense to restrict mergetool's use of rerere to the
case when rerere.enabled is set?
I'm also a little confused about why the MERGE_RR file appeared there
in the first place. I can see that empty MERGE_RR files, with
different timestamps, have ended up in various other of my git
repositories as well. I never used rerere in these repos, and the only
non-git tools I have used for version control are stgit and kdiff3.
Erik
^ permalink raw reply
* [PATCH] svn: Create config options for common args
From: Ted Percival @ 2011-11-14 20:25 UTC (permalink / raw)
To: git; +Cc: Eric Wong, Ted Percival
These config options may be set to apply to all commands on the
repository in lieu of providing the command-line options each time:
svn.localtime: --localtime
svn.useLogAuthor: --use-log-author
svn.addAuthorFrom: --add-author-from
Since these flags apply to multiple operations, it's easier to set them
once rather than remembering to use them every time for every operation.
Signed-off-by: Ted Percival <ted.percival@quest.com>
---
Documentation/git-svn.txt | 10 ++++++++++
git-svn.perl | 3 +++
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 34ee785..20f8edd 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -113,6 +113,9 @@ repository you cloned from, but if you wish for your local Git
repository to be able to interoperate with someone else's local Git
repository, either don't use this option or you should both use it in
the same local timezone.
++
+[verse]
+config key: svn.localtime
--parent;;
Fetch only from the SVN parent of the current HEAD.
@@ -596,12 +599,19 @@ creating the branch or tag.
When retrieving svn commits into git (as part of 'fetch', 'rebase', or
'dcommit' operations), look for the first `From:` or `Signed-off-by:` line
in the log message and use that as the author string.
++
+[verse]
+config key: svn.useLogAuthor
+
--add-author-from::
When committing to svn from git (as part of 'commit-diff', 'set-tree' or 'dcommit'
operations), if the existing log message doesn't already have a
`From:` or `Signed-off-by:` line, append a `From:` line based on the
git commit's author string. If you use this, then `--use-log-author`
will retrieve a valid author string for all commits.
++
+[verse]
+config key: svn.addAuthorFrom
ADVANCED OPTIONS
diff --git a/git-svn.perl b/git-svn.perl
index e30df22..d69b0d7 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -21,6 +21,9 @@ $Git::SVN::default_repo_id = 'svn';
$Git::SVN::default_ref_id = $ENV{GIT_SVN_ID} || 'git-svn';
$Git::SVN::Ra::_log_window_size = 100;
$Git::SVN::_minimize_url = 'unset';
+$Git::SVN::_localtime = Git::config_bool('svn.localtime');
+$Git::SVN::_add_author_from = Git::config_bool('svn.addAuthorFrom');
+$Git::SVN::_use_log_author = Git::config_bool('svn.useLogAuthor');
if (! exists $ENV{SVN_SSH} && exists $ENV{GIT_SSH}) {
$ENV{SVN_SSH} = $ENV{GIT_SSH};
--
1.7.7.1
^ permalink raw reply related
* Re: [RFC] deprecating and eventually removing "git relink"?
From: Jeff King @ 2011-11-14 20:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Simon Brenner, git
In-Reply-To: <7vfwhqjw4u.fsf@alter.siamese.dyndns.org>
On Mon, Nov 14, 2011 at 12:18:25PM -0800, Junio C Hamano wrote:
> > Yes, I think that is sensible. I'm not sure there is even any core git
> > code to be written. I think a wrapper that does the following would
> > probably work:
>
> I agree with your outline, which I find is in line with what I had in mind
> in the message Miles responded.
>
> The approach is different from what Miles alluded to, which is to have
> "clients" create objects in the "central" place in the first place,
> though.
It seems to me that is simply an optimization that can come later. An
initial, no-C-code implementation would write to individual repos as
usual, and then occasionally migrate objects to the master shared repo
(and remove duplicates from individual repos). That's an easy to
implement low-risk experiment from which we can draw conclusions about
how well such a system works in practice.
And then if it seems like a good path, an obvious optimization[1] is to
write directly into the parent object store, skipping the migration.
This might involve git-core code, or maybe it just means setting up the
repos differently (e.g., symlinking the objects directory to the master
store).
-Peff
[1] Actually, I am slightly dubious that this optimization is worth
doing. It seems like it would save you from writing the data only to
copy it later. But in practice, we write loose objects, and you are
already rewriting the data to migrate it into packfiles. So the
migration already happens, and instead we would just be migrating to
packfiles in the central repo.
^ permalink raw reply
* Re: Git shouldn't allow to push a new branch called HEAD
From: Junio C Hamano @ 2011-11-14 20:22 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Haggerty, Daniele Segato, Git Mailing List
In-Reply-To: <20111114111659.GC10847@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So one solution is to block fetching of remote branches called HEAD
> (which I would be OK with). But another is...
> ... Obviously there's a lot more to it than just tweaking the default fetch
> refspecs. The ref lookup rules need to be changed to take this into
> account. There was some discussion about this over the summer (under the
> subject of possible "1.8.0" changes), but I don't think any work has
> been done.
I would say discussing and ironing out the kinks of the design counts as
work, but I agree nobody was seriously interested in laying out a sensible
transition plan and discussion died out before anything concrete happened.
Regardless of the layout chanage, which probably is a 2.X topic, I think a
good first step would be to start forbidding anything that ends with _?HEAD
as a branch or tag name, on top of Michael's "enforce the refname rules more
vigorously when a ref is created" series.
^ permalink raw reply
* Re: [RFC] deprecating and eventually removing "git relink"?
From: Junio C Hamano @ 2011-11-14 20:18 UTC (permalink / raw)
To: Jeff King; +Cc: Simon Brenner, git
In-Reply-To: <20111114103451.GA10847@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Nov 14, 2011 at 09:48:07AM +0100, Simon Brenner wrote:
>
>> On Mon, Nov 14, 2011 at 7:06 AM, Miles Bader <miles@gnu.org> wrote:
>> > It might be nice to have a mechanism where new objects would update
>> > the _alternate_ rather than the object-store in the tree where the
>> > command was run... then you could easily have a bunch of trees using a
>> > central object store without needing to update the central store
>> > occasionally by hand (and do gc in its "clients")...
>>
>> This sounds like a nice way forward: replace/extend the current
>> alternates system ...
>
> Yes, I think that is sensible. I'm not sure there is even any core git
> code to be written. I think a wrapper that does the following would
> probably work:
I agree with your outline, which I find is in line with what I had in mind
in the message Miles responded.
The approach is different from what Miles alluded to, which is to have
"clients" create objects in the "central" place in the first place,
though.
^ permalink raw reply
* Re: Repository data loss in fast-export with a merge of a deleted submodule
From: Jens Lehmann @ 2011-11-14 19:51 UTC (permalink / raw)
To: Joshua Jensen; +Cc: git@vger.kernel.org, Elijah Newren, Johannes Sixt
In-Reply-To: <4EC12E8B.3050909@workspacewhiz.com>
Am 14.11.2011 16:06, schrieb Joshua Jensen:
> ----- Original Message -----
> From: Joshua Jensen
> Date: 11/3/2011 10:05 AM
>> ----- Original Message -----
>> From: Joshua Jensen
>> Date: 10/27/2011 1:27 PM
>>> We had a submodule that we deleted and then added back into the repository at the same location as the former submodule. When running fast-export, the newly 'added' files for the merge commit are listed and then are followed with a:
>>>
>>> M ... path/to/submodule/file
>>> D path/to/submodule
>>>
>>> On fast-import, the resultant repository becomes corrupt due to the Delete instruction above occurring AFTER the file adds/modifications. The new repository does not match the old repository where the fast-export was performed.
>>>
>>> I am not familiar with the fast-export code. Can anyone help out?
>> Okay, I looked into this further, and I came up with a patch that works for me. Nevertheless, I do not understand exactly what is going on here, so I would like to defer to someone else's patch to fix the issue.
>>
> Hi.
>
> __This is a genuine data loss problem in Git.__
>
> I'm confused at the lack of response to this. I first posted about the issue **2-1/2 weeks ago**, and there have been no responses Does no one care?
Maybe no one cares, people didn't read the message (or forgot about it)
or they are too busy ... thanks for prodding us again.
While I'm interested in this issue because submodules are affected, I'm
very short on Git time these days and can't investigate this issue
further (and I have no clue about export/import either). I added the last
two people who touched depth_first() in builtin/fast-export.c to the CC,
maybe they can tell us more about your patch to solve this issue (found
in [2]).
> In case no one received the messages, you can find them at [1] and [2].
>
> -Josh
>
> [1] http://www.spinics.net/lists/git/msg168295.html
> [2] http://www.spinics.net/lists/git/msg168691.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox