* Re: [PATCH] fix some clang warnings
From: Matthieu Moy @ 2013-01-16 18:12 UTC (permalink / raw)
To: Jeff King
Cc: Max Horn, John Keeping, Antoine Pelisse, Junio C Hamano, git,
Johannes Sixt
In-Reply-To: <20130116175057.GB27525@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> It seems a little weird to me that clang defines __GNUC__, but I
> assume there are good reasons for it.
The reason is essentially that clang targets compatibility with GCC
(implementing the same extensions & cie), in the sense "drop in
replacement that should be able to compile legacy code possibly relying
on __GNUC__ and GCC extensions."
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: Jeff King @ 2013-01-16 18:15 UTC (permalink / raw)
To: John Keeping
Cc: Max Horn, Antoine Pelisse, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <20130116181203.GB2476@farnsworth.metanate.com>
On Wed, Jan 16, 2013 at 06:12:03PM +0000, John Keeping wrote:
> On Wed, Jan 16, 2013 at 10:00:42AM -0800, Jeff King wrote:
> > It is not about the macro itself, but rather the callsites that do not
> > return error, but call it for its printing side effect. It seems that
> > clang -Wunused-value is OK with unused values from functions being
> > discarded, but not with constants. So:
> >
> > int foo();
> > void bar()
> > {
> > foo(); /* ok */
> > 1; /* not ok */
> > (foo(), 1); /* not ok */
> > }
> >
> > The first one is OK (I think it would fall under -Wunused-result under
> > either compiler). The middle one is an obvious error, and caught by both
> > compilers. The last one is OK by gcc, but clang complains.
>
> I wonder if this would be changed in clang - the change in [1] is
> superficially similar.
>
> [1] http://llvm.org/bugs/show_bug.cgi?id=13747
Yeah, I think it is exactly the same issue, and the fix they mention
there would apply to us, too.
Is it worth applying this at all, then? Or should we apply it but limit
it with a clang version macro (they mention r163034, but I do not know
if it is in a released version yet, nor what macros are available to
inspect the version)?
-Peff
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: Antoine Pelisse @ 2013-01-16 18:21 UTC (permalink / raw)
To: Jeff King; +Cc: John Keeping, Max Horn, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <20130116181558.GA4426@sigill.intra.peff.net>
> Is it worth applying this at all, then? Or should we apply it but limit
> it with a clang version macro (they mention r163034, but I do not know
> if it is in a released version yet, nor what macros are available to
> inspect the version)?
Please also note that building with clang is not warning-free (though
I think it would be nice)
^ permalink raw reply
* Re: Question re. git remote repository
From: Jeff King @ 2013-01-16 18:21 UTC (permalink / raw)
To: Konstantin Khomoutov; +Cc: Lang, David, 'git@vger.kernel.org'
In-Reply-To: <20130116220615.48c159546bccfa5b9cd9028e@domain007.com>
On Wed, Jan 16, 2013 at 10:06:15PM +0400, Konstantin Khomoutov wrote:
> > In regards to the repositories, I think I understand correctly that
> > each developer will have a local repository that they will work
> > from, and that there will also be a remote repository (origin) that
> > will hold the original version of the project.
Note that this is just one common topology for setting up repos (and it
is probably the simplest). Others are described here:
http://git-scm.com/book/en/Distributed-Git-Distributed-Workflows
> > Ideally we'd prefer to simply create our remote repository on a drive
> > of one of our local network servers. Is this possible?
>
> Yes, this is possible, but it's not advised to keep such a "reference"
> repository on an exported networked drive for a number of reasons (both
> performance and bug-free operation).
I agree that performance is not ideal (although if you are on a fast
LAN, it probably would not matter much), but I do not recall any
specific bugs in that area. Can you elaborate?
-Peff
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: John Keeping @ 2013-01-16 18:22 UTC (permalink / raw)
To: Jeff King
Cc: John Keeping, Max Horn, Antoine Pelisse, Junio C Hamano, git,
Johannes Sixt
In-Reply-To: <20130116181558.GA4426@sigill.intra.peff.net>
On Wed, Jan 16, 2013 at 10:15:58AM -0800, Jeff King wrote:
> On Wed, Jan 16, 2013 at 06:12:03PM +0000, John Keeping wrote:
>
> > On Wed, Jan 16, 2013 at 10:00:42AM -0800, Jeff King wrote:
> > > It is not about the macro itself, but rather the callsites that do not
> > > return error, but call it for its printing side effect. It seems that
> > > clang -Wunused-value is OK with unused values from functions being
> > > discarded, but not with constants. So:
> > >
> > > int foo();
> > > void bar()
> > > {
> > > foo(); /* ok */
> > > 1; /* not ok */
> > > (foo(), 1); /* not ok */
> > > }
> > >
> > > The first one is OK (I think it would fall under -Wunused-result under
> > > either compiler). The middle one is an obvious error, and caught by both
> > > compilers. The last one is OK by gcc, but clang complains.
> >
> > I wonder if this would be changed in clang - the change in [1] is
> > superficially similar.
> >
> > [1] http://llvm.org/bugs/show_bug.cgi?id=13747
>
> Yeah, I think it is exactly the same issue, and the fix they mention
> there would apply to us, too.
>
> Is it worth applying this at all, then? Or should we apply it but limit
> it with a clang version macro (they mention r163034, but I do not know
> if it is in a released version yet, nor what macros are available to
> inspect the version)?
That maps to revision 06b3a06007 in their git repository [1], which is
contained in remotes/origin/release_32 so I think that change should be
in release 3.2, where I still see the warning (although that's not using
a clang built from that source), so I don't think that the fix for that
bug removes the warning in this case.
[1] http://llvm.org/git/clang.git
^ permalink raw reply
* Re: git fetch without --recurse-submodules option
From: Jens Lehmann @ 2013-01-16 18:24 UTC (permalink / raw)
To: 乙酸鋰; +Cc: git
In-Reply-To: <CAHtLG6RE-xGYsp-Apcu3hk8OVck+HLYZdBtWvcweaBNetwtKNA@mail.gmail.com>
Am 16.01.2013 06:45, schrieb 乙酸鋰:
> With git pull or git fetch without specifying --recurse-submodules,
> what is the default action?
on-demand fetch (unless something else is configured).
> It seems git fetches submodules wtihout specifying --recurse-submodules.
> If this is not clear, please update documentation.
You are right, the documentation for pull and fetch does not state
explicitly that "on-demand" is the default here when the option is
not used.
> In git pull document --recurse-submodules option, it tells users to
> see git-config(1) and gitmodules(5), but does not tell users to refer
> to git fetch --recurse-submodules for the meaning of the switches.
> In git fetch document --recurse-submodules option, it does not tell
> users to see git-config(1) or gitmodules(5).
Thanks for pointing that out. Unless anyone else wants to improve the
documentation I'll look into that in my next git time slot.
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: Jeff King @ 2013-01-16 18:24 UTC (permalink / raw)
To: John Keeping
Cc: Max Horn, Antoine Pelisse, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <20130116182240.GC2476@farnsworth.metanate.com>
On Wed, Jan 16, 2013 at 06:22:40PM +0000, John Keeping wrote:
> > > [1] http://llvm.org/bugs/show_bug.cgi?id=13747
> >
> > Yeah, I think it is exactly the same issue, and the fix they mention
> > there would apply to us, too.
> >
> > Is it worth applying this at all, then? Or should we apply it but limit
> > it with a clang version macro (they mention r163034, but I do not know
> > if it is in a released version yet, nor what macros are available to
> > inspect the version)?
>
> That maps to revision 06b3a06007 in their git repository [1], which is
> contained in remotes/origin/release_32 so I think that change should be
> in release 3.2, where I still see the warning (although that's not using
> a clang built from that source), so I don't think that the fix for that
> bug removes the warning in this case.
>
> [1] http://llvm.org/git/clang.git
Thanks for checking. I'd rather squelch the warning completely (as in my
re-post of Max's patch from a few minutes ago), and we can loosen it
(possibly with a version check) later when a fix is widely disseminated.
I know that compiling git with clang is not warning-free yet, but it is
close, and I do not mind if somebody puts some effort into making it so.
This gets us one step closer.
-Peff
^ permalink raw reply
* Re: [PATCH v3] test-lib.sh: unfilter GIT_PERF_*
From: Jonathan Nieder @ 2013-01-16 18:54 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Thomas Rast
In-Reply-To: <1358331833-8289-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy wrote:
> These variables are user parameters to control how to run the perf
> tests. Allow users to do so.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
FWIW, of the three versions proposed, this one makes the most sense to
me. Looks good.
^ permalink raw reply
* Re: [PATCH] fix some clang warnings
From: John Keeping @ 2013-01-16 19:01 UTC (permalink / raw)
To: Jeff King
Cc: John Keeping, Max Horn, Antoine Pelisse, Junio C Hamano, git,
Johannes Sixt
In-Reply-To: <20130116182449.GA4881@sigill.intra.peff.net>
On Wed, Jan 16, 2013 at 10:24:49AM -0800, Jeff King wrote:
> On Wed, Jan 16, 2013 at 06:22:40PM +0000, John Keeping wrote:
>
> > > > [1] http://llvm.org/bugs/show_bug.cgi?id=13747
> > >
> > > Yeah, I think it is exactly the same issue, and the fix they mention
> > > there would apply to us, too.
> > >
> > > Is it worth applying this at all, then? Or should we apply it but limit
> > > it with a clang version macro (they mention r163034, but I do not know
> > > if it is in a released version yet, nor what macros are available to
> > > inspect the version)?
> >
> > That maps to revision 06b3a06007 in their git repository [1], which is
> > contained in remotes/origin/release_32 so I think that change should be
> > in release 3.2, where I still see the warning (although that's not using
> > a clang built from that source), so I don't think that the fix for that
> > bug removes the warning in this case.
> >
> > [1] http://llvm.org/git/clang.git
>
> Thanks for checking. I'd rather squelch the warning completely (as in my
> re-post of Max's patch from a few minutes ago), and we can loosen it
> (possibly with a version check) later when a fix is widely disseminated.
I checked again with a trunk build of clang and the warning's still
there, so I've created a clang bug [1] to see if they will change the
behaviour.
I agree that we should squelch the warning for now, it can be changed
into a version check if it's accepted as a bug and once we know what
version it's fixed in.
[1] http://llvm.org/bugs/show_bug.cgi?id=14968
^ permalink raw reply
* Re: [PATCH] attr: fix off-by-one directory component length calculation
From: Junio C Hamano @ 2013-01-16 19:07 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jean-Noël AVILA, git
In-Reply-To: <20130116060238.GA29523@duynguyen-vnpc.dek-tpc.internal>
Duy Nguyen <pclouds@gmail.com> writes:
> OK I get your point now. Something like this?
>
> -- 8< --
> Subject: [PATCH] attr: avoid calling find_basename() twice per path
>
> find_basename() is only used inside collect_all_attrs(), called once
> in prepare_attr_stack, then again after prepare_attr_stack()
> returns. Both calls return exact same value. Reorder the code to do
> the same task once. Also avoid strlen() because we knows the length
> after finding basename.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Yeah, I think this is a nice code reduction, readability improvement
and micro optimization rolled into one.
> attr.c | 45 ++++++++++++++++++---------------------------
> 1 file changed, 18 insertions(+), 27 deletions(-)
>
> diff --git a/attr.c b/attr.c
> index cfc6748..880f862 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -564,32 +564,12 @@ static void bootstrap_attr_stack(void)
> attr_stack = elem;
> }
>
> -static const char *find_basename(const char *path)
> -{
> - const char *cp, *last_slash = NULL;
> -
> - for (cp = path; *cp; cp++) {
> - if (*cp == '/' && cp[1])
> - last_slash = cp;
> - }
> - return last_slash ? last_slash + 1 : path;
> -}
> -
> -static void prepare_attr_stack(const char *path)
> +static void prepare_attr_stack(const char *path, int dirlen)
> {
> struct attr_stack *elem, *info;
> - int dirlen, len;
> + int len;
> const char *cp;
>
> - dirlen = find_basename(path) - path;
> -
> - /*
> - * find_basename() includes the trailing slash, but we do
> - * _not_ want it.
> - */
> - if (dirlen)
> - dirlen--;
> -
> /*
> * At the bottom of the attribute stack is the built-in
> * set of attribute definitions, followed by the contents
> @@ -769,15 +749,26 @@ static int macroexpand_one(int attr_nr, int rem)
> static void collect_all_attrs(const char *path)
> {
> struct attr_stack *stk;
> - int i, pathlen, rem;
> - const char *basename;
> + int i, pathlen, rem, dirlen;
> + const char *basename, *cp, *last_slash = NULL;
> +
> + for (cp = path; *cp; cp++) {
> + if (*cp == '/' && cp[1])
> + last_slash = cp;
> + }
> + pathlen = cp - path;
> + if (last_slash) {
> + basename = last_slash + 1;
> + dirlen = last_slash - path;
> + } else {
> + basename = path;
> + dirlen = 0;
> + }
>
> - prepare_attr_stack(path);
> + prepare_attr_stack(path, dirlen);
> for (i = 0; i < attr_nr; i++)
> check_all_attr[i].value = ATTR__UNKNOWN;
>
> - basename = find_basename(path);
> - pathlen = strlen(path);
> rem = attr_nr;
> for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
> rem = fill(path, pathlen, basename, stk, rem);
^ permalink raw reply
* [PATCH v2] Allow custom "comment char"
From: Ralf Thielow @ 2013-01-16 19:18 UTC (permalink / raw)
To: gitster; +Cc: jrnieder, git, Ralf Thielow
In-Reply-To: <1358275827-5244-1-git-send-email-ralf.thielow@gmail.com>
From: Junio C Hamano <gitster@pobox.com>
Some users do want to write a line that begin with a pound sign, #,
in their commit log message. Many tracking system recognise
a token of #<bugid> form, for example.
The support we offer these use cases is not very friendly to the end
users. They have a choice between
- Don't do it. Avoid such a line by rewrapping or indenting; and
- Use --cleanup=whitespace but remove all the hint lines we add.
Give them a way to set a custom comment char, e.g.
$ git -c core.commentchar="%" commit
so that they do not have to do either of the two workarounds.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
---
Junio, thanks for the code in your reply to the
first version. It works very well and looks nice.
I was also unhappy about this "\n%c\n" thing and
pretty unsure with the code in "git-submodule.sh".
But with this, it looks good to me. Thanks.
Changes in v2:
- extend "git stripspace" with an option to make
it's input being converted to commented lines
- teach git-submodule.sh using this
- rename strbuf_commented_addstr to strbuf_add_commented_lines
and improve it's design
Documentation/config.txt | 6 ++++
Documentation/git-stripspace.txt | 8 ++++-
Documentation/technical/api-strbuf.txt | 10 ++++++
builtin/branch.c | 10 +++---
builtin/commit.c | 12 +++----
builtin/fmt-merge-msg.c | 2 +-
builtin/merge.c | 5 ++-
builtin/notes.c | 34 +++++++++-----------
builtin/stripspace.c | 48 +++++++++++++++++++++++-----
builtin/tag.c | 34 ++++++++++----------
cache.h | 6 ++++
config.c | 8 +++++
environment.c | 6 ++++
git-submodule.sh | 8 ++---
strbuf.c | 58 ++++++++++++++++++++++++++++------
strbuf.h | 4 +++
t/t0030-stripspace.sh | 35 ++++++++++++++++++++
t/t7502-commit.sh | 7 ++++
t/t7508-status.sh | 50 +++++++++++++++++++++++++++++
wt-status.c | 10 +++---
20 files changed, 283 insertions(+), 78 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d5809e0..e99b9f2 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -528,6 +528,12 @@ core.editor::
variable when it is set, and the environment variable
`GIT_EDITOR` is not set. See linkgit:git-var[1].
+core.commentchar::
+ Commands such as `commit` and `tag` that lets you edit
+ messages consider a line that begins with this character
+ commented, and removes them after the editor returns
+ (default '#').
+
sequence.editor::
Text editor used by `git rebase -i` for editing the rebase insn file.
The value is meant to be interpreted by the shell when it is used.
diff --git a/Documentation/git-stripspace.txt b/Documentation/git-stripspace.txt
index a80d946..e6fdfcb 100644
--- a/Documentation/git-stripspace.txt
+++ b/Documentation/git-stripspace.txt
@@ -35,7 +35,13 @@ OPTIONS
-------
-s::
--strip-comments::
- Skip and remove all lines starting with '#'.
+ Skip and remove all lines starting with comment character (default '#').
+
+-c::
+--comment-lines::
+ Prepend comment character and blank to each line. Lines will automatically
+ be terminated with a newline. On empty lines, only the comment character
+ will be prepended.
EXAMPLES
--------
diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt
index 84686b5..2c59cb2 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -156,6 +156,11 @@ then they will free() it.
Remove the bytes between `pos..pos+len` and replace it with the given
data.
+`strbuf_add_commented_lines`::
+
+ Add a NUL-terminated string to the buffer. Each line will be prepended
+ by a comment character and a blank.
+
`strbuf_add`::
Add data of given length to the buffer.
@@ -229,6 +234,11 @@ which can be used by the programmer of the callback as she sees fit.
Add a formatted string to the buffer.
+`strbuf_commented_addf`::
+
+ Add a formatted string prepended by a comment character and a
+ blank to the buffer.
+
`strbuf_fread`::
Read a given size of data from a FILE* pointer to the buffer.
diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..3548271 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -706,11 +706,11 @@ static int edit_branch_description(const char *branch_name)
read_branch_desc(&buf, branch_name);
if (!buf.len || buf.buf[buf.len-1] != '\n')
strbuf_addch(&buf, '\n');
- strbuf_addf(&buf,
- "# Please edit the description for the branch\n"
- "# %s\n"
- "# Lines starting with '#' will be stripped.\n",
- branch_name);
+ strbuf_commented_addf(&buf,
+ "Please edit the description for the branch\n"
+ " %s\n"
+ "Lines starting with '%c' will be stripped.\n",
+ branch_name, comment_line_char);
fp = fopen(git_path(edit_description), "w");
if ((fwrite(buf.buf, 1, buf.len, fp) < buf.len) || fclose(fp)) {
strbuf_release(&buf);
diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..f95f64a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -733,15 +733,15 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
if (cleanup_mode == CLEANUP_ALL)
status_printf(s, GIT_COLOR_NORMAL,
_("Please enter the commit message for your changes."
- " Lines starting\nwith '#' will be ignored, and an empty"
- " message aborts the commit.\n"));
+ " Lines starting\nwith '%c' will be ignored, and an empty"
+ " message aborts the commit.\n"), comment_line_char);
else /* CLEANUP_SPACE, that is. */
status_printf(s, GIT_COLOR_NORMAL,
_("Please enter the commit message for your changes."
- " Lines starting\n"
- "with '#' will be kept; you may remove them"
- " yourself if you want to.\n"
- "An empty message aborts the commit.\n"));
+ " Lines starting\n"
+ "with '%c' will be kept; you may remove them"
+ " yourself if you want to.\n"
+ "An empty message aborts the commit.\n"), comment_line_char);
if (only_include_assumed)
status_printf_ln(s, GIT_COLOR_NORMAL,
"%s", only_include_assumed);
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index d9af43c..b49612f 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -470,7 +470,7 @@ static void fmt_tag_signature(struct strbuf *tagbuf,
strbuf_complete_line(tagbuf);
if (sig->len) {
strbuf_addch(tagbuf, '\n');
- strbuf_add_lines(tagbuf, "# ", sig->buf, sig->len);
+ strbuf_add_commented_lines(tagbuf, sig->buf, sig->len);
}
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 9307e9c..7c8922c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -788,17 +788,16 @@ static const char merge_editor_comment[] =
N_("Please enter a commit message to explain why this merge is necessary,\n"
"especially if it merges an updated upstream into a topic branch.\n"
"\n"
- "Lines starting with '#' will be ignored, and an empty message aborts\n"
+ "Lines starting with '%c' will be ignored, and an empty message aborts\n"
"the commit.\n");
static void prepare_to_commit(struct commit_list *remoteheads)
{
struct strbuf msg = STRBUF_INIT;
- const char *comment = _(merge_editor_comment);
strbuf_addbuf(&msg, &merge_msg);
strbuf_addch(&msg, '\n');
if (0 < option_edit)
- strbuf_add_lines(&msg, "# ", comment, strlen(comment));
+ strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
write_merge_msg(&msg);
if (run_hook(get_index_file(), "prepare-commit-msg",
git_path("MERGE_MSG"), "merge", NULL, NULL))
diff --git a/builtin/notes.c b/builtin/notes.c
index 453457a..57748a6 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -92,10 +92,7 @@ static const char * const git_notes_get_ref_usage[] = {
};
static const char note_template[] =
- "\n"
- "#\n"
- "# Write/edit the notes for the following object:\n"
- "#\n";
+ "\nWrite/edit the notes for the following object:\n";
struct msg_arg {
int given;
@@ -129,7 +126,7 @@ static void write_commented_object(int fd, const unsigned char *object)
{"show", "--stat", "--no-notes", sha1_to_hex(object), NULL};
struct child_process show;
struct strbuf buf = STRBUF_INIT;
- FILE *show_out;
+ struct strbuf cbuf = STRBUF_INIT;
/* Invoke "git show --stat --no-notes $object" */
memset(&show, 0, sizeof(show));
@@ -142,21 +139,14 @@ static void write_commented_object(int fd, const unsigned char *object)
die(_("unable to start 'show' for object '%s'"),
sha1_to_hex(object));
- /* Open the output as FILE* so strbuf_getline() can be used. */
- show_out = xfdopen(show.out, "r");
- if (show_out == NULL)
- die_errno(_("can't fdopen 'show' output fd"));
+ if (strbuf_read(&buf, show.out, 0) < 0)
+ die_errno(_("could not read 'show' output"));
+ strbuf_add_commented_lines(&cbuf, buf.buf, buf.len);
+ write_or_die(fd, cbuf.buf, cbuf.len);
- /* Prepend "# " to each output line and write result to 'fd' */
- while (strbuf_getline(&buf, show_out, '\n') != EOF) {
- write_or_die(fd, "# ", 2);
- write_or_die(fd, buf.buf, buf.len);
- write_or_die(fd, "\n", 1);
- }
+ strbuf_release(&cbuf);
strbuf_release(&buf);
- if (fclose(show_out))
- die_errno(_("failed to close pipe to 'show' for object '%s'"),
- sha1_to_hex(object));
+
if (finish_command(&show))
die(_("failed to finish 'show' for object '%s'"),
sha1_to_hex(object));
@@ -170,6 +160,7 @@ static void create_note(const unsigned char *object, struct msg_arg *msg,
if (msg->use_editor || !msg->given) {
int fd;
+ struct strbuf buf = STRBUF_INIT;
/* write the template message before editing: */
path = git_pathdup("NOTES_EDITMSG");
@@ -181,11 +172,16 @@ static void create_note(const unsigned char *object, struct msg_arg *msg,
write_or_die(fd, msg->buf.buf, msg->buf.len);
else if (prev && !append_only)
write_note_data(fd, prev);
- write_or_die(fd, note_template, strlen(note_template));
+
+ strbuf_addch(&buf, '\n');
+ strbuf_add_commented_lines(&buf, note_template, strlen(note_template));
+ strbuf_addch(&buf, '\n');
+ write_or_die(fd, buf.buf, buf.len);
write_commented_object(fd, object);
close(fd);
+ strbuf_release(&buf);
strbuf_reset(&(msg->buf));
if (launch_editor(path, &(msg->buf), NULL)) {
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index f16986c..234248b 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -30,7 +30,8 @@ static size_t cleanup(char *line, size_t len)
*
* If last line does not have a newline at the end, one is added.
*
- * Enable skip_comments to skip every line starting with "#".
+ * Enable skip_comments to skip every line starting with comment
+ * character.
*/
void stripspace(struct strbuf *sb, int skip_comments)
{
@@ -45,7 +46,7 @@ void stripspace(struct strbuf *sb, int skip_comments)
eol = memchr(sb->buf + i, '\n', sb->len - i);
len = eol ? eol - (sb->buf + i) + 1 : sb->len - i;
- if (skip_comments && len && sb->buf[i] == '#') {
+ if (skip_comments && len && sb->buf[i] == comment_line_char) {
newlen = 0;
continue;
}
@@ -66,21 +67,52 @@ void stripspace(struct strbuf *sb, int skip_comments)
strbuf_setlen(sb, j);
}
+static void comment_lines(struct strbuf *buf)
+{
+ char *msg;
+ size_t len;
+
+ msg = strbuf_detach(buf, &len);
+ strbuf_add_commented_lines(buf, msg, len);
+}
+
+static const char *usage_msg = "\n"
+" git stripspace [-s | --strip-comments] < input\n"
+" git stripspace [-c | --comment-lines] < input";
+
int cmd_stripspace(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
int strip_comments = 0;
+ enum { INVAL = 0, STRIP_SPACE = 1, COMMENT_LINES = 2 } mode = STRIP_SPACE;
+
+ if (argc == 2) {
+ if (!strcmp(argv[1], "-s") ||
+ !strcmp(argv[1], "--strip-comments")) {
+ strip_comments = 1;
+ } else if (!strcmp(argv[1], "-c") ||
+ !strcmp(argv[1], "--comment-lines")) {
+ mode = COMMENT_LINES;
+ } else {
+ mode = INVAL;
+ }
+ } else if (argc > 1) {
+ mode = INVAL;
+ }
+
+ if (mode == INVAL)
+ usage(usage_msg);
- if (argc == 2 && (!strcmp(argv[1], "-s") ||
- !strcmp(argv[1], "--strip-comments")))
- strip_comments = 1;
- else if (argc > 1)
- usage("git stripspace [-s | --strip-comments] < input");
+ if (strip_comments || mode == COMMENT_LINES)
+ git_config(git_default_config, NULL);
if (strbuf_read(&buf, 0, 1024) < 0)
die_errno("could not read the input");
- stripspace(&buf, strip_comments);
+ if (mode == STRIP_SPACE)
+ stripspace(&buf, strip_comments);
+ else
+ comment_lines(&buf);
write_or_die(1, buf.buf, buf.len);
strbuf_release(&buf);
diff --git a/builtin/tag.c b/builtin/tag.c
index 9c3e067..f826688 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -246,19 +246,13 @@ static int do_sign(struct strbuf *buffer)
}
static const char tag_template[] =
- N_("\n"
- "#\n"
- "# Write a tag message\n"
- "# Lines starting with '#' will be ignored.\n"
- "#\n");
+ N_("\nWrite a tag message\n"
+ "Lines starting with '%c' will be ignored.\n");
static const char tag_template_nocleanup[] =
- N_("\n"
- "#\n"
- "# Write a tag message\n"
- "# Lines starting with '#' will be kept; you may remove them"
- " yourself if you want to.\n"
- "#\n");
+ N_("\nWrite a tag message\n"
+ "Lines starting with '%c' will be kept; you may remove them"
+ " yourself if you want to.\n");
static int git_tag_config(const char *var, const char *value, void *cb)
{
@@ -346,14 +340,18 @@ static void create_tag(const unsigned char *object, const char *tag,
if (fd < 0)
die_errno(_("could not create file '%s'"), path);
- if (!is_null_sha1(prev))
+ if (!is_null_sha1(prev)) {
write_tag_body(fd, prev);
- else if (opt->cleanup_mode == CLEANUP_ALL)
- write_or_die(fd, _(tag_template),
- strlen(_(tag_template)));
- else
- write_or_die(fd, _(tag_template_nocleanup),
- strlen(_(tag_template_nocleanup)));
+ } else {
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addch(&buf, '\n');
+ if (opt->cleanup_mode == CLEANUP_ALL)
+ strbuf_commented_addf(&buf, _(tag_template), comment_line_char);
+ else
+ strbuf_commented_addf(&buf, _(tag_template_nocleanup), comment_line_char);
+ write_or_die(fd, buf.buf, buf.len);
+ strbuf_release(&buf);
+ }
close(fd);
if (launch_editor(path, buf, NULL)) {
diff --git a/cache.h b/cache.h
index c257953..0b435a4 100644
--- a/cache.h
+++ b/cache.h
@@ -562,6 +562,12 @@ extern int core_preload_index;
extern int core_apply_sparse_checkout;
extern int precomposed_unicode;
+/*
+ * The character that begins a commented line in user-editable file
+ * that is subject to stripspace.
+ */
+extern char comment_line_char;
+
enum branch_track {
BRANCH_TRACK_UNSPECIFIED = -1,
BRANCH_TRACK_NEVER = 0,
diff --git a/config.c b/config.c
index 7b444b6..d873c59 100644
--- a/config.c
+++ b/config.c
@@ -717,6 +717,14 @@ static int git_default_core_config(const char *var, const char *value)
if (!strcmp(var, "core.editor"))
return git_config_string(&editor_program, var, value);
+ if (!strcmp(var, "core.commentchar")) {
+ const char *comment;
+ int ret = git_config_string(&comment, var, value);
+ if (!ret)
+ comment_line_char = comment[0];
+ return ret;
+ }
+
if (!strcmp(var, "core.askpass"))
return git_config_string(&askpass_program, var, value);
diff --git a/environment.c b/environment.c
index 85edd7f..a40c38b 100644
--- a/environment.c
+++ b/environment.c
@@ -62,6 +62,12 @@ int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
struct startup_info *startup_info;
unsigned long pack_size_limit_cfg;
+/*
+ * The character that begins a commented line in user-editable file
+ * that is subject to stripspace.
+ */
+char comment_line_char = '#';
+
/* Parallel index stat data preload? */
int core_preload_index = 0;
diff --git a/git-submodule.sh b/git-submodule.sh
index 22ec5b6..004c034 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -976,12 +976,12 @@ cmd_summary() {
done |
if test -n "$for_status"; then
if [ -n "$files" ]; then
- gettextln "# Submodules changed but not updated:"
+ gettextln "Submodules changed but not updated:" | git stripspace -c
else
- gettextln "# Submodule changes to be committed:"
+ gettextln "Submodule changes to be committed:" | git stripspace -c
fi
- echo "#"
- sed -e 's|^|# |' -e 's|^# $|#|'
+ printf "\n" | git stripspace -c
+ git stripspace -c
else
cat
fi
diff --git a/strbuf.c b/strbuf.c
index 9a373be..48e9abb 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -204,6 +204,54 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
va_end(ap);
}
+static void add_lines(struct strbuf *out,
+ const char *prefix1,
+ const char *prefix2,
+ const char *buf, size_t size)
+{
+ while (size) {
+ const char *prefix;
+ const char *next = memchr(buf, '\n', size);
+ next = next ? (next + 1) : (buf + size);
+
+ prefix = (prefix2 && buf[0] == '\n') ? prefix2 : prefix1;
+ strbuf_addstr(out, prefix);
+ strbuf_add(out, buf, next - buf);
+ size -= next - buf;
+ buf = next;
+ }
+ strbuf_complete_line(out);
+}
+
+void strbuf_add_commented_lines(struct strbuf *out, const char *buf, size_t size)
+{
+ static char prefix1[3];
+ static char prefix2[2];
+
+ if (prefix1[0] != comment_line_char) {
+ sprintf(prefix1, "%c ", comment_line_char);
+ sprintf(prefix2, "%c", comment_line_char);
+ }
+ add_lines(out, prefix1, prefix2, buf, size);
+}
+
+void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...)
+{
+ va_list params;
+ struct strbuf buf = STRBUF_INIT;
+ int incomplete_line = sb->len && sb->buf[sb->len - 1] != '\n';
+
+ va_start(params, fmt);
+ strbuf_vaddf(&buf, fmt, params);
+ va_end(params);
+
+ strbuf_add_commented_lines(sb, buf.buf, buf.len);
+ if (incomplete_line)
+ sb->buf[--sb->len] = '\0';
+
+ strbuf_release(&buf);
+}
+
void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)
{
int len;
@@ -414,15 +462,7 @@ int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
void strbuf_add_lines(struct strbuf *out, const char *prefix,
const char *buf, size_t size)
{
- while (size) {
- const char *next = memchr(buf, '\n', size);
- next = next ? (next + 1) : (buf + size);
- strbuf_addstr(out, prefix);
- strbuf_add(out, buf, next - buf);
- size -= next - buf;
- buf = next;
- }
- strbuf_complete_line(out);
+ add_lines(out, prefix, NULL, buf, size);
}
void strbuf_addstr_xml_quoted(struct strbuf *buf, const char *s)
diff --git a/strbuf.h b/strbuf.h
index ecae4e2..958822c 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -110,6 +110,8 @@ extern void strbuf_remove(struct strbuf *, size_t pos, size_t len);
extern void strbuf_splice(struct strbuf *, size_t pos, size_t len,
const void *, size_t);
+extern void strbuf_add_commented_lines(struct strbuf *out, const char *buf, size_t size);
+
extern void strbuf_add(struct strbuf *, const void *, size_t);
static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
strbuf_add(sb, s, strlen(s));
@@ -131,6 +133,8 @@ extern void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *
__attribute__((format (printf,2,3)))
extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
+__attribute__((format (printf, 2, 3)))
+extern void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);
__attribute__((format (printf,2,0)))
extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
index ccb0a3c..a8e84d8 100755
--- a/t/t0030-stripspace.sh
+++ b/t/t0030-stripspace.sh
@@ -397,4 +397,39 @@ test_expect_success 'strip comments, too' '
test -z "$(echo "# comment" | git stripspace -s)"
'
+test_expect_success 'strip comments with changed comment char' '
+ test ! -z "$(echo "; comment" | git -c core.commentchar=";" stripspace)" &&
+ test -z "$(echo "; comment" | git -c core.commentchar=";" stripspace -s)"
+'
+
+test_expect_success '-c with single line' '
+ printf "# foo\n" >expect &&
+ printf "foo" | git stripspace -c >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '-c with single line followed by empty line' '
+ printf "# foo\n#\n" >expect &&
+ printf "foo\n\n" | git stripspace -c >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '-c with newline only' '
+ printf "#\n" >expect &&
+ printf "\n" | git stripspace -c >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--comment-lines with single line' '
+ printf "# foo\n" >expect &&
+ printf "foo" | git stripspace -c >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '-c with changed comment char' '
+ printf "; foo\n" >expect &&
+ printf "foo" | git -c core.commentchar=";" stripspace -c >actual &&
+ test_cmp expect actual
+'
+
test_done
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 1a5cb69..6a2c67c 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -447,4 +447,11 @@ use_template="-t template"
try_commit_status_combo
+test_expect_success 'commit --status with custom comment character' '
+ test_when_finished "git config --unset core.commentchar" &&
+ git config core.commentchar ";" &&
+ try_commit --status &&
+ test_i18ngrep "^; Changes to be committed:" .git/COMMIT_EDITMSG
+'
+
test_done
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index e313ef1..a79c032 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1254,6 +1254,56 @@ test_expect_success ".git/config ignore=dirty doesn't suppress submodule summary
'
cat > expect << EOF
+; On branch master
+; Changes to be committed:
+; (use "git reset HEAD <file>..." to unstage)
+;
+; modified: sm
+;
+; Changes not staged for commit:
+; (use "git add <file>..." to update what will be committed)
+; (use "git checkout -- <file>..." to discard changes in working directory)
+;
+; modified: dir1/modified
+; modified: sm (new commits)
+;
+; Submodule changes to be committed:
+;
+; * sm $head...$new_head (1):
+; > Add bar
+;
+; Submodules changed but not updated:
+;
+; * sm $new_head...$head2 (1):
+; > 2nd commit
+;
+; Untracked files:
+; (use "git add <file>..." to include in what will be committed)
+;
+; .gitmodules
+; dir1/untracked
+; dir2/modified
+; dir2/untracked
+; expect
+; output
+; untracked
+EOF
+
+test_expect_success "status (core.commentchar with submodule summary)" '
+ test_when_finished "git config --unset core.commentchar" &&
+ git config core.commentchar ";" &&
+ git status >output &&
+ test_i18ncmp expect output
+'
+
+test_expect_success "status (core.commentchar with two chars with submodule summary)" '
+ test_when_finished "git config --unset core.commentchar" &&
+ git config core.commentchar ";;" &&
+ git status >output &&
+ test_i18ncmp expect output
+'
+
+cat > expect << EOF
# On branch master
# Changes not staged for commit:
# (use "git add <file>..." to update what will be committed)
diff --git a/wt-status.c b/wt-status.c
index 2a9658b..f6f197e 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -45,7 +45,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,
strbuf_vaddf(&sb, fmt, ap);
if (!sb.len) {
- strbuf_addch(&sb, '#');
+ strbuf_addch(&sb, comment_line_char);
if (!trail)
strbuf_addch(&sb, ' ');
color_print_strbuf(s->fp, color, &sb);
@@ -59,7 +59,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,
strbuf_reset(&linebuf);
if (at_bol) {
- strbuf_addch(&linebuf, '#');
+ strbuf_addch(&linebuf, comment_line_char);
if (*line != '\n' && *line != '\t')
strbuf_addch(&linebuf, ' ');
}
@@ -760,8 +760,10 @@ static void wt_status_print_tracking(struct wt_status *s)
for (cp = sb.buf; (ep = strchr(cp, '\n')) != NULL; cp = ep + 1)
color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s),
- "# %.*s", (int)(ep - cp), cp);
- color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
+ "%c %.*s", comment_line_char,
+ (int)(ep - cp), cp);
+ color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%c",
+ comment_line_char);
}
static int has_unmerged(struct wt_status *s)
--
1.8.1.291.g056e2eb
^ permalink raw reply related
* Re: [PATCH] git-remote: distinguish between default and configured URLs
From: Junio C Hamano @ 2013-01-16 19:19 UTC (permalink / raw)
To: Michael J Gruber; +Cc: John Keeping, git, Jardel Weyrich, Sascha Cunz
In-Reply-To: <50F6A0F0.70800@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> In short, the separate listing is correct, but in this case there's no
> improvement in readability.
Yes, I think the "insteadOf" rewrite is a related but a separate
issue.
Is "remote -v" meant for diagnosing remote.origin.{url,pushurl} that
are misconfigured?
If not, the output just should just say the final outcome, i.e. what
destinations we will fetch from and push to, without cluttering the
output.
If on the other hand it is to help users debug their configuration,
the output also needs to explain exactly what made us decide those
destinations to use (e.g. to discover there was a leftover insteadof
in $HOME/.gitconfig the user forgot about).
^ permalink raw reply
* Re: [BUG] Possible bug in `remote set-url --add --push`
From: Andreas Schwab @ 2013-01-16 19:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: Michael J Gruber, Jardel Weyrich, Sascha Cunz,
git@vger.kernel.org
In-Reply-To: <7v622xyvnd.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I actually think my earlier "it shouldn't be the same (push)" is not
> needed and probably is actively wrong. Just like you can tell
> between
>
> (only one .url) (both .url and .pushurl)
>
> origin there (fetch/push) origin there (fetch)
> origin there (push)
What should happen when you have a .pushinsteadof configured that
modifies .url for pushing?
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: Question re. git remote repository
From: Konstantin Khomoutov @ 2013-01-16 19:37 UTC (permalink / raw)
To: Jeff King
Cc: Konstantin Khomoutov, Lang, David, 'git@vger.kernel.org'
In-Reply-To: <20130116182156.GB4426@sigill.intra.peff.net>
On Wed, 16 Jan 2013 10:21:56 -0800
Jeff King <peff@peff.net> wrote:
Thanks for elaborating on the "origin" -- I intended to write up on its
special status but got distracted and sent my message missing that
bit ;-)
[...]
> > > Ideally we'd prefer to simply create our remote repository on a
> > > drive of one of our local network servers. Is this possible?
> >
> > Yes, this is possible, but it's not advised to keep such a
> > "reference" repository on an exported networked drive for a number
> > of reasons (both performance and bug-free operation).
>
> I agree that performance is not ideal (although if you are on a fast
> LAN, it probably would not matter much), but I do not recall any
> specific bugs in that area. Can you elaborate?
This one [1] for instance. I also recall seing people having other
"mystical" problems with setups like this so I somehow developed an idea
than having a repository on a networked drive is asking for troubles.
Of course, if there are happy users of such setups, I would be glad to
hear as my precautions might well be unfounded for the recent versions
of Git.
1. http://code.google.com/p/msysgit/issues/detail?id=130
^ permalink raw reply
* Re: [BUG] Possible bug in `remote set-url --add --push`
From: Junio C Hamano @ 2013-01-16 20:01 UTC (permalink / raw)
To: Andreas Schwab
Cc: Michael J Gruber, Jardel Weyrich, Sascha Cunz,
git@vger.kernel.org
In-Reply-To: <m2ip6x0vtk.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> I actually think my earlier "it shouldn't be the same (push)" is not
>> needed and probably is actively wrong. Just like you can tell
>> between
>>
>> (only one .url) (both .url and .pushurl)
>>
>> origin there (fetch/push) origin there (fetch)
>> origin there (push)
>
> What should happen when you have a .pushinsteadof configured that
> modifies .url for pushing?
I think push should work like this:
* the user gives us a nickname;
* we look at remote.$nickname.pushurl (and if there isn't,
remote.$nickname.url) to decide the logical URLs to push to;
* for each logical URL we decided to push, we look at
url.*.pushInsteadOf to see if there is one that match the $URL
(and if there isn't url.*.insteadOf), and map the logical URL to
the final destination.
So that we can instruct "push" to push, when pushing into a
repository that logically resides at git://git.k.org/pub/,
to instead push into the repository via git-over-ssh, e.g.
[remote "korg"]
url = git://git.k.org/pub/scm/git/git.git/
[url "git.k.org:/pub/"]
pushInsteadOf = git://git.k.org/pub/
without affecting the fetching side.
As I said in a separate message, the above "fetch/push" vs "fetch"
and "push" distinction is not descriptive enough to express the post
rewriting that is done with insteadOf; it only helps debugging
misconfiguration between .url vs .pushurl, which may be better than
the status quo but is not ideal.
^ permalink raw reply
* Re: Question re. git remote repository
From: David Lang @ 2013-01-16 20:07 UTC (permalink / raw)
To: Lang, David; +Cc: 'git@vger.kernel.org'
In-Reply-To: <201301161749.r0GHnGV6007806@smtpb02.one-mail.on.ca>
Hi David, now we are going to have some confusion here, two David Langs on the
list :-)
On Wed, 16 Jan 2013, Lang, David wrote:
> We're just in the process of investigating a versioning tool and are very
> interesting in git. We have one question we're hoping someone can answer. In
> regards to the repositories, I think I understand correctly that each
> developer will have a local repository that they will work from, and that
> there will also be a remote repository (origin) that will hold the original
> version of the project.
>
> It appears from the limited reading I've done that the remote repository must
> be hosted at github.com. Is this the case? Ideally we'd prefer to simply
> create our remote repository on a drive of one of our local network servers.
> Is this possible?
Git is peer-to-peer, the 'origin' is just the default to pull from. It can be
hosted on any machine that you have access to.
A typical case is that you designate one person (or a small group of people)
to oversee your master repository, and that person decides when and what to pull
there from the developers.
This gives you a chance to insert code review, tests, etc between what the
developers produce in their local repository and what you then bless as the
authoritative 'released' version of the code.
However, this master repository is just a matter of convention, it is possible
to use any repository as the 'origin', changing it is just a config change away.
David Lang
^ permalink raw reply
* Re: [PATCH v2] Make git selectively and conditionally ignore certain stat fields
From: Ramsay Jones @ 2013-01-16 20:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Robin Rosenberg, git, j sixt, Shawn Pearce
In-Reply-To: <7va9sa6f0h.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Robin Rosenberg <robin.rosenberg@dewire.com> writes:
> That configurability is a slipperly slope to drag us into giving users
> more complexity that does not help them very much, I suspect.
>
> Earlier somebody mentioned "size and mtime is often enough", so I
> think a single option core.looseStatInfo (substitute "loose" with
> short, minimum or whatever adjective that is more appropriate---I am
> not good at picking phrases, it sounds to me a way to more loosely
> define stat info cleanliness than we usually do) that makes us
> ignore all fields (regardless of their zero-ness) other than those
> two fields might not be a bad way to go.
At one point, I used to build (and test) the MSVC version of git on
cygwin, which leads to exactly the same problem. So, this is not just
an EGit/JGit vs c-git issue, although there can't be many people that
will have this problem. (Mixing the MinGW and cygwin versions on the
same repo will also have this problem).
I had a patch which, essentially, did what you suggest above; ie ignore
everything other than size and mtime, *including* ignoring the zero-ness
in the index. (I just don't understand why you would think of doing
otherwise!! ;-) ). As part of that patch, I also suppressed the "empty diff"
output that used to be shown for stat-dirty files (that's been fixed now
right?), otherwise using gitk was a pain.
[BTW, given the "schizophrenic stat" functions on cygwin, you can have
this problem with the cygwin version of git - all on it's lonesome!]
I can't help with naming, BTW, since I called the config variable
"core.ramsay-stat". :-P
>
> I do not offhand know if such a loose mode is too simple and make it
> excessively risky, though.
I suspect it would be fine ... *however*, I never sent my patch because
I didn't think there would be many idiots^H^H^H^H^H^H pioneers like me! :-D
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 1/3] Move Git::SVN::get_tz to Git::get_tz_offset
From: Ben Walton @ 2013-01-16 20:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: esr, git
In-Reply-To: <7vehhlyw90.fsf@alter.siamese.dyndns.org>
On Wed, Jan 16, 2013 at 3:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ben Walton <bdwalton@gmail.com> writes:
>
>> +sub get_tz_offset {
>> + # some systmes don't handle or mishandle %z, so be creative.
>
> Hmph. I wonder if we can use %z if it is handled correctly and fall
> back to this code only on platforms that are broken?
That would be perfectly acceptable to me. The reason I set it up to
always run through this function here is that when I originally added
this function for git-svn, I'd made it conditional and Eric Wong
preferred that the function be used exclusively[1]. I opted to take
the same approach here to keep things congrous.
If it were to be conditional, I think I'd add a variable to the build
system and have the code leverage that at runtime instead of the
try/except approach I attempted in 2009.
Thanks
-Ben
[1] http://lists-archives.com/git/683572-git-svn-fix-for-systems-without-strftime-z.html
--
---------------------------------------------------------------------------------------------------------------------------
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---------------------------------------------------------------------------------------------------------------------------
^ permalink raw reply
* Re: [PATCH v2] Allow custom "comment char"
From: Junio C Hamano @ 2013-01-16 20:30 UTC (permalink / raw)
To: Ralf Thielow; +Cc: jrnieder, git
In-Reply-To: <1358363928-16729-1-git-send-email-ralf.thielow@gmail.com>
Ralf Thielow <ralf.thielow@gmail.com> writes:
> From: Junio C Hamano <gitster@pobox.com>
>
> Some users do want to write a line that begin with a pound sign, #,
> in their commit log message. Many tracking system recognise
> a token of #<bugid> form, for example.
>
> The support we offer these use cases is not very friendly to the end
> users. They have a choice between
>
> - Don't do it. Avoid such a line by rewrapping or indenting; and
>
> - Use --cleanup=whitespace but remove all the hint lines we add.
>
> Give them a way to set a custom comment char, e.g.
>
> $ git -c core.commentchar="%" commit
>
> so that they do not have to do either of the two workarounds.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
> ---
> Junio, thanks for the code in your reply to the
> first version. It works very well and looks nice.
> I was also unhappy about this "\n%c\n" thing and
> pretty unsure with the code in "git-submodule.sh".
> But with this, it looks good to me. Thanks.
>
> Changes in v2:
> - extend "git stripspace" with an option to make
> it's input being converted to commented lines
> - teach git-submodule.sh using this
> - rename strbuf_commented_addstr to strbuf_add_commented_lines
> and improve it's design
Oh, I love it when something like this happens. Throw a "perhaps
along these lines" patch and then a finished product that fills the
gaps I didn't bother to fill magically appears, even with tests and
updates to comments and documentation.
What good things did I do recently to deserve such a luck? ;-)
> @@ -66,21 +67,52 @@ void stripspace(struct strbuf *sb, int skip_comments)
> strbuf_setlen(sb, j);
> }
>
> +static void comment_lines(struct strbuf *buf)
> +{
> + char *msg;
> + size_t len;
> +
> + msg = strbuf_detach(buf, &len);
> + strbuf_add_commented_lines(buf, msg, len);
> +}
This leaks msg (inherited from my "perhaps along these lines"
patch). I think I can just add free(msg) at the end.
> + if (strip_comments || mode == COMMENT_LINES)
> + git_config(git_default_config, NULL);
Nice spotting. The "along these lines" patch broke "stripspace -s"
under custom comment line char; this fixes it.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/3] Move Git::SVN::get_tz to Git::get_tz_offset
From: Junio C Hamano @ 2013-01-16 20:36 UTC (permalink / raw)
To: Ben Walton; +Cc: esr, git
In-Reply-To: <CAP30j164UD9gNRbZ=uCQjgpDODWnGtYmHcWES2P=YPryL=FbZA@mail.gmail.com>
Ben Walton <bdwalton@gmail.com> writes:
> On Wed, Jan 16, 2013 at 3:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Ben Walton <bdwalton@gmail.com> writes:
>>
>>> +sub get_tz_offset {
>>> + # some systmes don't handle or mishandle %z, so be creative.
>>
>> Hmph. I wonder if we can use %z if it is handled correctly and fall
>> back to this code only on platforms that are broken?
>
> That would be perfectly acceptable to me. The reason I set it up to
> always run through this function here is that when I originally added
> this function for git-svn, I'd made it conditional and Eric Wong
> preferred that the function be used exclusively[1]. I opted to take
> the same approach here to keep things congrous.
>
> If it were to be conditional, I think I'd add a variable to the build
> system and have the code leverage that at runtime instead of the
> try/except approach I attempted in 2009.
If the code was originally unconditional for a reason (and I think
being bug-to-bug compatible across platforms is actually a good
thing in a tool like importers), I would not object to it. Thanks
for the back-story.
^ permalink raw reply
* Re: [PATCH v6 0/8] push: update remote tags only with force
From: Junio C Hamano @ 2013-01-16 21:02 UTC (permalink / raw)
To: Jeff King
Cc: Max Horn, Chris Rorvick, git, Angelo Borsotti, Drew Northup,
Michael Haggerty, Philip Oakley, Johannes Sixt, Kacper Kornet,
Felipe Contreras
In-Reply-To: <20130116174325.GA27525@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I see what you are saying, but I think the ship has already sailed to
> some degree. We already implement the non-fast-forward check everywhere,
> and I cannot have a "refs/tested" hierarchy that pushes arbitrary
> commits without regard to their history. If I have such a hierarchy, I
> have to use "--force" (or more likely, mark the refspec with "+").
Yeah, actually in that example, I meant refs/tested/ would have
pointers to bare tree objects. I often rebuild 'pu' and another
private integration branch for testing, reordering the series that
are still not in 'next' and also after rewriting log messages of
some commits. It is not unusual to end up the updated 'pu' having
the identical tree as 'pu' before update, and I want to skip testing
the result (tree equality matters while commit equality does not in
such a use case).
> In my mind, the object-type checking is just making that fast-forward
> check more thorough (i.e., extending it to non-commit objects).
Yes, I agree with that point of view.
Thanks.
Here is what I am planning to queue (the patch is the same, but the
message is different on the third point).
-- >8 --
Subject: [PATCH] push: fix "refs/tags/ hierarchy cannot be updated without --force"
When pushing to update a branch with a commit that is not a
descendant of the commit at the tip, a wrong message "already
exists" was given, instead of the correct "non-fast-forward", if we
do not have the object sitting in the destination repository at the
tip of the ref we are updating.
The primary cause of the bug is that the check in a new helper
function is_forwardable() assumed both old and new objects are
available and can be checked, which is not always the case.
The way the caller uses the result of this function is also wrong.
If the helper says "we do not want to let this push go through", the
caller unconditionally translates it into "we blocked it because the
destination already exists", which is not true at all in this case.
Fix this by doing these three things:
* Remove unnecessary not_forwardable from "struct ref"; it is only
used inside set_ref_status_for_push();
* Make "refs/tags/" the only hierarchy that cannot be replaced
without --force;
* Remove the misguided attempt to force that everything that
updates an existing ref has to be a commit outside "refs/tags/"
hierarchy.
The policy last one tried to implement may later be resurrected and
extended to ensure fast-forwardness (defined as "not losing
objects", extending from the traditional "not losing commits from
the resulting history") when objects that are not commit are
involved (e.g. an annotated tag in hierarchies outside refs/tags),
but such a logic belongs to "is this a fast-forward?" check that is
done by ref_newer(); is_forwardable(), which is now removed, was not
the right place to do so.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
cache.h | 1 -
remote.c | 43 +++++++------------------------------------
t/t5516-fetch-push.sh | 21 ---------------------
3 files changed, 7 insertions(+), 58 deletions(-)
diff --git a/cache.h b/cache.h
index a32a0ea..a942bbd 100644
--- a/cache.h
+++ b/cache.h
@@ -1004,7 +1004,6 @@ struct ref {
requires_force:1,
merge:1,
nonfastforward:1,
- not_forwardable:1,
update:1,
deletion:1;
enum {
diff --git a/remote.c b/remote.c
index aa6b719..d3a1ca2 100644
--- a/remote.c
+++ b/remote.c
@@ -1279,26 +1279,6 @@ int match_push_refs(struct ref *src, struct ref **dst,
return 0;
}
-static inline int is_forwardable(struct ref* ref)
-{
- struct object *o;
-
- if (!prefixcmp(ref->name, "refs/tags/"))
- return 0;
-
- /* old object must be a commit */
- o = parse_object(ref->old_sha1);
- if (!o || o->type != OBJ_COMMIT)
- return 0;
-
- /* new object must be commit-ish */
- o = deref_tag(parse_object(ref->new_sha1), NULL, 0);
- if (!o || o->type != OBJ_COMMIT)
- return 0;
-
- return 1;
-}
-
void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
int force_update)
{
@@ -1320,32 +1300,23 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
}
/*
- * The below logic determines whether an individual
- * refspec A:B can be pushed. The push will succeed
- * if any of the following are true:
+ * Decide whether an individual refspec A:B can be
+ * pushed. The push will succeed if any of the
+ * following are true:
*
* (1) the remote reference B does not exist
*
* (2) the remote reference B is being removed (i.e.,
* pushing :B where no source is specified)
*
- * (3) the update meets all fast-forwarding criteria:
- *
- * (a) the destination is not under refs/tags/
- * (b) the old is a commit
- * (c) the new is a descendant of the old
- *
- * NOTE: We must actually have the old object in
- * order to overwrite it in the remote reference,
- * and the new object must be commit-ish. These are
- * implied by (b) and (c) respectively.
+ * (3) the destination is not under refs/tags/, and
+ * if the old and new value is a commit, the new
+ * is a descendant of the old.
*
* (4) it is forced using the +A:B notation, or by
* passing the --force argument
*/
- ref->not_forwardable = !is_forwardable(ref);
-
ref->update =
!ref->deletion &&
!is_null_sha1(ref->old_sha1);
@@ -1355,7 +1326,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
!has_sha1_file(ref->old_sha1)
|| !ref_newer(ref->new_sha1, ref->old_sha1);
- if (ref->not_forwardable) {
+ if (!prefixcmp(ref->name, "refs/tags/")) {
ref->requires_force = 1;
if (!force_ref_update) {
ref->status = REF_STATUS_REJECT_ALREADY_EXISTS;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 6009372..8f024a0 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -950,27 +950,6 @@ test_expect_success 'push requires --force to update lightweight tag' '
)
'
-test_expect_success 'push requires --force to update annotated tag' '
- mk_test heads/master &&
- mk_child child1 &&
- mk_child child2 &&
- (
- cd child1 &&
- git tag -a -m "message 1" Tag &&
- git push ../child2 Tag:refs/tmp/Tag &&
- git push ../child2 Tag:refs/tmp/Tag &&
- >file1 &&
- git add file1 &&
- git commit -m "file1" &&
- git tag -f -a -m "message 2" Tag &&
- test_must_fail git push ../child2 Tag:refs/tmp/Tag &&
- git push --force ../child2 Tag:refs/tmp/Tag &&
- git tag -f -a -m "message 3" Tag HEAD~ &&
- test_must_fail git push ../child2 Tag:refs/tmp/Tag &&
- git push --force ../child2 Tag:refs/tmp/Tag
- )
-'
-
test_expect_success 'push --porcelain' '
mk_empty &&
echo >.git/foo "To testrepo" &&
--
1.8.1.1.426.g616047d
^ permalink raw reply related
* Re: [PATCH v2] Allow custom "comment char"
From: Jens Lehmann @ 2013-01-16 21:02 UTC (permalink / raw)
To: Ralf Thielow; +Cc: gitster, jrnieder, git
In-Reply-To: <1358363928-16729-1-git-send-email-ralf.thielow@gmail.com>
Am 16.01.2013 20:18, schrieb Ralf Thielow:
> From: Junio C Hamano <gitster@pobox.com>
>
> Some users do want to write a line that begin with a pound sign, #,
> in their commit log message. Many tracking system recognise
> a token of #<bugid> form, for example.
>
> The support we offer these use cases is not very friendly to the end
> users. They have a choice between
>
> - Don't do it. Avoid such a line by rewrapping or indenting; and
>
> - Use --cleanup=whitespace but remove all the hint lines we add.
>
> Give them a way to set a custom comment char, e.g.
>
> $ git -c core.commentchar="%" commit
>
> so that they do not have to do either of the two workarounds.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
> ---
> Junio, thanks for the code in your reply to the
> first version. It works very well and looks nice.
> I was also unhappy about this "\n%c\n" thing and
> pretty unsure with the code in "git-submodule.sh".
I can't see anything wrong with it (but didn't have the time to
test it). On my todo list (but *way* down) is the task to replace
the call to "git submodule summary --for-status ..." in
wt_status_print_submodule_summary() with a call to "git diff
--submodule" (and - at least in the long term - rip out the
--for-status option from the submodule script). Maybe now is a
good time for someone else to tackle that? (especially as the new
strbuf_commented_add*() functions should make that rather easy)
^ permalink raw reply
* Re: Question re. git remote repository
From: Matt Seitz (matseitz) @ 2013-01-16 21:07 UTC (permalink / raw)
To: git@vger.kernel.org
"Konstantin Khomoutov" <kostix+git@007spb.ru> wrote in message news:<20130116233744.7d0775eaec98ce154a9de180@domain007.com>...
> On Wed, 16 Jan 2013 10:21:56 -0800
> Jeff King <peff@peff.net> wrote:
> >
> > I agree that performance is not ideal (although if you are on a fast
> > LAN, it probably would not matter much), but I do not recall any
> > specific bugs in that area. Can you elaborate?
>
> Of course, if there are happy users of such setups, I would be glad to
> hear as my precautions might well be unfounded for the recent versions
> of Git.
I'm a happy user of git on network file systems (NFS and CIFS/SMB), although not a heavy user.
> 1. http://code.google.com/p/msysgit/issues/detail?id=130
I wouldn't be surprised if there are some subtle POSIX-Win32 compatibility issues here between msysgit and Samba (POSIX GIT, ported to use Win32 file system functions, sending those Win32 requests to a Samba server, and the Samba server translating those Win32 requests back into POSIX functions).
^ permalink raw reply
* Re: [PATCH v3 2/3] config: Introduce diff.algorithm variable
From: Junio C Hamano @ 2013-01-16 21:59 UTC (permalink / raw)
To: Michal Privoznik; +Cc: git, trast, peff
In-Reply-To: <7vbocpxbwp.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Will replace the one in 'pu' with this round. Looking good.
>
> Thanks.
By the way, wouldn't we want some tests to protect this feature from
future breakages?
^ permalink raw reply
* Re: [PATCH] clean.c, ls-files.c: respect encapsulation of exclude_list_groups
From: Junio C Hamano @ 2013-01-16 22:20 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1358342758-30503-1-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> Consumers of the dir.c traversal API should avoid assuming knowledge
> of the internal implementation of exclude_list_groups. Therefore
> when adding items to an exclude list, it should be accessed via the
> pointer returned from add_exclude_list(), rather than by referencing
> a location within dir.exclude_list_groups[EXC_CMDL].
>
> Signed-off-by: Adam Spiers <git@adamspiers.org>
> ---
> builtin/clean.c | 6 +++---
> builtin/ls-files.c | 15 ++++++++++-----
> 2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/clean.c b/builtin/clean.c
> index b098288..b9cb7ad 100644
> --- a/builtin/clean.c
> +++ b/builtin/clean.c
> @@ -45,6 +45,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
> static const char **pathspec;
> struct strbuf buf = STRBUF_INIT;
> struct string_list exclude_list = STRING_LIST_INIT_NODUP;
> + struct exclude_list *el;
When a type "exclude_list" exists and used in the same function,
having a local variable of the same name but of a different type
becomes a bit awkward.
builtin/ls-files.c shares the same structure. Does the file-scope
"exclude_args" variable need to be a file-scope static over there?
It seems that it is closely tied to the elements of the string list,
so it may make sense to:
* remove the file-scope static "exclude_args";
* rename "exclude_list" string list variable to "exclude_args";
and
* replace "--exclude_args" in the loop that iterates over
exclude_list (now exclude_args) with "-(i+1)" or something,
just like you do in "builtin/clean.c" below.
> - add_exclude_list(&dir, EXC_CMDL, "--exclude option");
> + el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
> for (i = 0; i < exclude_list.nr; i++)
> - add_exclude(exclude_list.items[i].string, "", 0,
> - &dir.exclude_list_group[EXC_CMDL].el[0], -(i+1));
> + add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
We may want to use for_each_string_list_item() here and in the
corresponding loop in builtin/ls-files.c, but because we do need to
give the -(i + 1) label to each element, I think the code is OK
as-is.
^ 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