* Re: [PATCH] am: invoke perl's strftime in C locale
From: Dmitry V. Levin @ 2013-01-15 17:40 UTC (permalink / raw)
To: Jeff King; +Cc: Antoine Pelisse, Junio C Hamano, git
In-Reply-To: <20130115165058.GA29301@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]
On Tue, Jan 15, 2013 at 08:50:59AM -0800, Jeff King wrote:
> On Tue, Jan 15, 2013 at 05:42:12PM +0100, Antoine Pelisse wrote:
>
> > > This puts all of perl into the C locale, which would mean error messages
> > > from perl would be in English rather than the user's language. It
> > > probably isn't a big deal, because that snippet of perl is short and not
> > > likely to produce problems, but I wonder how hard it would be to set the
> > > locale just for the strftime call.
> >
> > Maybe just setting LC_TIME to C would do ...
>
> Yeah, that is a nice simple solution. Dmitry, does just setting LC_TIME
> fix the problem for you?
Just setting LC_TIME environment variable instead of LC_ALL would end up
with unreliable solution because LC_ALL has the highest priority.
If keeping error messages from perl has the utmost importance, it could be
achieved by
- perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+ perl -M'POSIX qw(strftime :locale_h)' -ne '
+ BEGIN { setlocale(LC_TIME, "C"); $subject = 0 }
but the little perl helper script we are talking about hardly worths so
much efforts.
--
ldv
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH] remote-hg: store converted URL
From: Max Horn @ 2013-01-15 17:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Felipe Contreras
In-Reply-To: <7vzk0a4ekj.fsf@alter.siamese.dyndns.org>
On 15.01.2013, at 17:05, Junio C Hamano wrote:
> Max Horn <max@quendi.de> writes:
>
>> On 14.01.2013, at 19:14, Junio C Hamano wrote:
>>
>>> What is lacking from this description is why it even needs to work
>>> from a different working directory....
>
> In your rewrite below, this is still lacking, I think.
Hm, I thought I made it clear: It has to change because relative paths only make sense when you know the reference point they are relative with.
Typically. This is the pwd. But when I
git clone repo newrepo
cd newrepo
I just changed the PWD. The clone command was given the relative path "repo". If git were to use that, it would suddenly refer to a directory inside newrepo, not next to it. Bang. Hence, git expands the relative path to an absolute one in the above example.
But git cannot do that for URLs in the form HELPER::PATH, because such a string is necessarily opaque to git.
<snip>
>> Thus when latter attempting to, say, "git pull"
>> from inside gitrepo, remote-hg cannot resolve the relative path correctly,
>> and the user sees an unexpected error.
>
> ... "cannot resolve the relative path correctly" above sound like a
> bug in remote-hg. Something like:
>
> Cloning a local hg repository using a relative path, e.g.
>
> git clone hg::hgrepo gitrepo
>
> stores "hg::hgrepo" in gitrepo/.git/config as its URL. When
> remote-hg is invoked by "git fetch", it chdirs to X (which is
> different from the "gitrepo" directory) and uses the URL (which
> is not correct, as it is a relative path but the cwd is
> different when it is used) to interact with the original
> "hgrepo", which will fail.
>
> is needed, but you didn't explain what that X is. Perhaps it is a
> temporary directory. Perhaps it is a hidden Hg repository somewhere
> in gitrepo/.git directory. Or something else.
None of the above. Nor does the remote helper chdir anywhere. It is the user who has done the chdir: Away from the location he invoked "git clone" at, and into the new repository directory that previously did not even exist.
Cheers,
Max
^ permalink raw reply
* Re: [PATCH] remote-hg: store converted URL
From: Max Horn @ 2013-01-15 18:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Felipe Contreras
In-Reply-To: <7vr4lm4cez.fsf@alter.siamese.dyndns.org>
On 15.01.2013, at 17:51, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Max Horn <max@quendi.de> writes:
>> ...
>>> See also the discussion (yeah, this time a real one ;-) leading to this:
>>> https://github.com/felipec/git/issues/2
>>> ...
>
> If I understand correctly, the $backend::$opaqueToken is a contract
> between the remote-helper and the remote-$backend
Just to clarify: What is the "remote-helper" ? So far, I thought of this being a contract between git (or some part of git), and remote-$backend (i.e. remote-hg in this case).
> that says "When
> user wants to interact with the same (foreign) repository, we agreed
> to let her use 'origin' nickname. The remote-helper looks up this
> opaque token that corresponds to 'origin' and gives it to the
> remote-$backend, and whatever is in the opaque token should be
> sufficient for the remote-$backend to figure out how to proceed from
> there".
Supposing that I interpret "remote-helper" correctly, that sounds about right to me.
>
> But in this hg::../over/there case, it seems that string is not
> sufficient for remote-hg to do so and the contract is broken.
One could put it that way.
>
> When "git clone $backend::$opaqueToken repo" is run in /dir/ecto/ry,
> and then subsequent "git fetch origin" will be run in (a
> subdirectory of) /dir/ecto/ry/repo, but anything relative to
> /dir/ecto/ry will not work once you go inside /dir/ecto/ry/repo.
> The "create a new repository here" argument could even be an
> absolute path to a totally different place, so if the
> remote-$backend wants to use $opaqueToken as anything relative to
> the $(cwd) when "git clone" was invoked, that original location
> needs to be available somehow.
That would be one option. Another is to do what the proposed patch does, and what git itself does: Change the relative path into an absolute one. This requires remote-$backend to be able to modify the opaque token supplied by the user.
Yet another would be to be more strict in remote-$backend as to which opaque tokens to accept: When it contains a relative path, simply always refuse to work, even if the PWD happens to be set the right way. Of course this would be quite undesirable from a user's perspective.
>
> Would a new helper protocol message be necessary, so that the
> backend can rewrite the $opaqueToken at "clone" time and tell the
> helper what to store as URL instead of the original? I do not think
> that is much different from remote-$backend updating the value of the
> remote.origin.URL using "git config".
>
> An alternative approach may be for somebody (either the "git clone"
> or the remote-$backend) to store a "base directory" when "git clone"
> was invoked in remote.origin.dirAtCloneTime variable, so that the
> next time remote-$backend runs, it can read that directory and
> interpret the $opaqueToken as a relative path to that directory if
> it wants to. That way, nobody needs to rewrite $opaqueToken.
As I said above, this would be an option. However, I would prefer rewriting the $opaqueToken, as that would be closer to what git does for "native" tokens passed to "git clone" Specifically, I am talking about get_repo_path() in builtin/clone.c which is called by cmd_clone. What this does is in my eyes essentially the equivalent of what the patch discussed here is doing.
Anyway, at the end of the day, I mainly care about relative paths working, somehow :-). But I think it would be important to make the issue easy to resolve for all remote-$backend authors, as many of them are affected (see below).
>
> How do other remote helpers solve this, I have to wonder, though.
> By not allowing relative paths to a directory?
So far, all I look at do not deal with this at all. Any attempts to deal with it should be pretty easy to recognize: The remote-$backend would have to store something into the git config, or else, verify the opaque token and refuse to work with it under certain conditions (e.g. when it contains a relative path). But they don't. E.g. git-remote-testgit has the exact same problem. Doing
git init repo && cd repo && echo a > a && git add a && git ci -m a a
git clone testgit::repo clone
cd clone
results in a .git/config file containing
[remote "origin"]
url = testgit::repo
fetch = +refs/heads/*:refs/remotes/origin/*
Trying to do a "git push" from within the clone then gives this:
fatal: 'repo/.git' does not appear to be a git repository
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
Traceback (most recent call last):
File "/usr/local/libexec/git-core/git-remote-testgit", line 272, in <module>
sys.exit(main(sys.argv))
File "/usr/local/libexec/git-core/git-remote-testgit", line 261, in main
repo = get_repo(alias, url)
File "/usr/local/libexec/git-core/git-remote-testgit", line 39, in get_repo
repo.get_revs()
File "/usr/local/lib/python2.7/site-packages/git_remote_helpers/git/repo.py", line 59, in get_revs
check_call(args, stdout=ofile)
File "/usr/local/lib/python2.7/site-packages/git_remote_helpers/util.py", line 174, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'ls-remote', 'repo/.git']' returned non-zero exit status 128
Cheers,
Max
^ permalink raw reply
* Re: [PATCH v2 06/19] reset.c: remove unnecessary variable 'i'
From: Martin von Zweigbergk @ 2013-01-15 18:36 UTC (permalink / raw)
To: Holding, Lawrence (NZ)
Cc: git, Junio C Hamano, Matt Kraai, Ramsay Jones, Duy Nguyen,
Jeff King
In-Reply-To: <A5E8E180685CEF45AB9E737A010799805E00DD@cdnz-ex1.corp.cubic.cub>
I suppose this was meant for everyone. Adding back the others.
On Tue, Jan 15, 2013 at 10:27 AM, Holding, Lawrence (NZ)
<Lawrence.Holding@cubic.com> wrote:
> Maybe use *argv instead of argv[0]?
Sure. Everywhere? Also in the lines added in patch 17/19 that refer to
both argv[0] and argv[1], such as "argv[1] &&
!get_sha1_treeish(argv[0], unused)"? Or is this just a sign that I'm
making the code _more_ confusing to those who are more familiar with
C?
^ permalink raw reply
* [PATCH] Allow custom "comment char"
From: Ralf Thielow @ 2013-01-15 18:50 UTC (permalink / raw)
To: gitster; +Cc: jrnieder, git, Ralf Thielow
In-Reply-To: <7vk3rk25iw.fsf_-_@alter.siamese.dyndns.org>
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>
---
Documentation/config.txt | 6 ++++
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 | 2 +-
builtin/tag.c | 35 ++++++++++++------------
cache.h | 6 ++++
config.c | 8 ++++++
environment.c | 6 ++++
git-submodule.sh | 14 +++++++---
strbuf.c | 38 ++++++++++++++++++++++++++
strbuf.h | 4 +++
t/t7502-commit.sh | 7 +++++
t/t7508-status.sh | 50 ++++++++++++++++++++++++++++++++++
wt-status.c | 10 ++++---
18 files changed, 198 insertions(+), 61 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/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt
index 84686b5..6767102 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_commented_addstr`::
+
+ Add a NUL-terminated string prepended by a comment character and a blank
+ to the buffer.
+
`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..d0fcd34 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_commented_addstr(tagbuf, sig->buf);
}
}
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..5e84e35 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";
+ "Write/edit the notes for the following object:";
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_commented_addstr(&cbuf, buf.buf);
+ 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_addf(&buf, "\n%c\n", comment_line_char);
+ strbuf_commented_addstr(&buf, note_template);
+ strbuf_addf(&buf, "\n%c\n", comment_line_char);
+ 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..600ca66 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -45,7 +45,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;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 9c3e067..e1b72be 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_("Write a tag message\n"
+ "Lines starting with '%c' will be ignored.");
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_("Write a tag message\n"
+ "Lines starting with '%c' will be kept; you may remove them"
+ " yourself if you want to.");
static int git_tag_config(const char *var, const char *value, void *cb)
{
@@ -346,14 +340,19 @@ 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_addf(&buf, "\n%c\n", comment_line_char);
+ 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);
+ strbuf_addf(&buf, "\n%c\n", 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..1b8d95f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -975,13 +975,19 @@ cmd_summary() {
echo
done |
if test -n "$for_status"; then
+ comment_char=`git config core.commentchar`
+ if [ ! -n "$comment_char" ]; then
+ comment_char='#'
+ elif [ ${#comment_char} -gt 1 ]; then
+ comment_char=`expr substr $comment_char 1 1`
+ fi
if [ -n "$files" ]; then
- gettextln "# Submodules changed but not updated:"
+ eval_gettextln "\$comment_char Submodules changed but not updated:"
else
- gettextln "# Submodule changes to be committed:"
+ eval_gettextln "\$comment_char Submodule changes to be committed:"
fi
- echo "#"
- sed -e 's|^|# |' -e 's|^# $|#|'
+ echo "$comment_char"
+ sed -e "s|^|$comment_char |" -e "s|^$comment_char $|$comment_char|"
else
cat
fi
diff --git a/strbuf.c b/strbuf.c
index 9a373be..8af4b4f 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -204,6 +204,44 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
va_end(ap);
}
+void strbuf_commented_addstr(struct strbuf *sb, const char *s)
+{
+ struct strbuf buf = STRBUF_INIT;
+ struct strbuf prefix = STRBUF_INIT;
+
+ strbuf_addf(&prefix, "%c ", comment_line_char);
+ strbuf_addstr(&buf, s);
+ strbuf_add_lines(sb, prefix.buf, buf.buf, buf.len);
+
+ // remove additional '\n' added by strbuf_add_lines()
+ if (sb->len && sb->buf[sb->len - 1] != buf.buf[buf.len - 1])
+ strbuf_remove(sb, sb->len - 1, 1);
+
+ strbuf_release(&prefix);
+ strbuf_release(&buf);
+}
+
+void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...)
+{
+ va_list params;
+ struct strbuf buf = STRBUF_INIT;
+ struct strbuf prefix = STRBUF_INIT;
+
+ va_start(params, fmt);
+ strbuf_vaddf(&buf, fmt, params);
+ va_end(params);
+
+ strbuf_addf(&prefix, "%c ", comment_line_char);
+ strbuf_add_lines(sb, prefix.buf, buf.buf, buf.len);
+
+ // remove additional '\n' added by strbuf_add_lines()
+ if (sb->len && sb->buf[sb->len - 1] != buf.buf[buf.len - 1])
+ strbuf_remove(sb, sb->len - 1, 1);
+
+ strbuf_release(&prefix);
+ strbuf_release(&buf);
+}
+
void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)
{
int len;
diff --git a/strbuf.h b/strbuf.h
index ecae4e2..8d5afd5 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_commented_addstr(struct strbuf *sb, const char *s);
+
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/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.g185e8f6
^ permalink raw reply related
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Ramsay Jones @ 2013-01-15 18:47 UTC (permalink / raw)
To: Mark Levedahl
Cc: Alex Riesen, Junio C Hamano, Jason Pyeron, git, Jonathan Nieder,
Torsten Bögershausen, Stephen & Linda Smith, Eric Blake
In-Reply-To: <50F303D8.20709@gmail.com>
Mark Levedahl wrote:
> On 01/11/2013 03:17 PM, Alex Riesen wrote:
>> On Fri, Jan 11, 2013 at 9:08 PM, Alex Riesen <raa.lkml@gmail.com> wrote:
>>> This short discussion on GitHub (file git-compat-util.h) might be relevant:
>>>
>>> https://github.com/msysgit/git/commit/435bdf8c7ffa493f8f6f2e8f329f8cc22db16ce6#commitcomment-2407194
>>>
>>> The change suggested there (to remove an inclusion of windows.h in
>>> git-compat-util.h) might simplify the solution a little. Might even
>>> remove the need for auto-configuration in Makefile (worked for me).
>> Just to be clear, the change is this:
>>
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index 4a1979f..780a919 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -85,12 +85,6 @@
>> #define _NETBSD_SOURCE 1
>> #define _SGI_SOURCE 1
>>
>> -#ifdef WIN32 /* Both MinGW and MSVC */
>> -#define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */
>> -#include <winsock2.h>
>> -#include <windows.h>
>> -#endif
>> -
>> #include <unistd.h>
>> #include <stdio.h>
>> #include <sys/stat.h>
>>
> That change alone seems fine, no apparent change building on current
> cygwin. However, with that change the build still fails if
> CYGWIN_V15_WIN32API is defined, so unless someone can show the
> compilation works on cygwin1.5 WITHOUT defining CYGWIN_V15_WIN32API this
> change does not help. I do not have an older installation available, so
> cannot test. Frankly, assuming you can compile with that macro defined,
> I would suggest leaving well enough alone - an unsupported configuration
> is unsupported :^)
I haven't been following this thread too closely, so I may have misunderstood
what you would like to test but, since I use cygwin 1.5, I tried the patch
given below.
I only had time to compile test this patch (ie I have *not* run any of the
tests - it takes over 3 hours for me), but it seems to work to that extent.
(I also tried a few simple commands: status, diff, branch; seems to work OK.)
If you would like me to test something else, just let me know.
HTH
ATB,
Ramsay Jones
-- >8 --
diff --git a/Makefile b/Makefile
index 1b30d7b..1c84f68 100644
--- a/Makefile
+++ b/Makefile
@@ -281,10 +281,6 @@ all::
#
# Define NO_REGEX if you have no or inferior regex support in your C library.
#
-# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x but are not
-# using the current w32api packages. The recommended approach, however,
-# is to update your installation if compilation errors occur.
-#
# Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
# user.
#
@@ -1402,9 +1398,6 @@ ifdef NO_REGEX
COMPAT_CFLAGS += -Icompat/regex
COMPAT_OBJS += compat/regex/regex.o
endif
-ifdef CYGWIN_V15_WIN32API
- COMPAT_CFLAGS += -DCYGWIN_V15_WIN32API
-endif
ifdef USE_NED_ALLOCATOR
COMPAT_CFLAGS += -Icompat/nedmalloc
diff --git a/compat/cygwin.c b/compat/cygwin.c
index 5428858..0a9aa6d 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -1,13 +1,8 @@
#define WIN32_LEAN_AND_MEAN
-#ifdef CYGWIN_V15_WIN32API
-#include "../git-compat-util.h"
-#include "win32.h"
-#else
#include <sys/stat.h>
#include <sys/errno.h>
#include "win32.h"
#include "../git-compat-util.h"
-#endif
#include "../cache.h" /* to read configuration */
static inline void filetime_to_timespec(const FILETIME *ft, struct timespec *ts)
diff --git a/config.mak.uname b/config.mak.uname
index bea34f0..5e493c9 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -158,7 +158,6 @@ ifeq ($(uname_O),Cygwin)
NO_SYMLINK_HEAD = YesPlease
NO_IPV6 = YesPlease
OLD_ICONV = UnfortunatelyYes
- CYGWIN_V15_WIN32API = YesPlease
endif
NO_THREAD_SAFE_PREAD = YesPlease
NEEDS_LIBICONV = YesPlease
diff --git a/git-compat-util.h b/git-compat-util.h
index e5a4b74..3186e55 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -85,12 +85,6 @@
#define _NETBSD_SOURCE 1
#define _SGI_SOURCE 1
-#ifdef WIN32 /* Both MinGW and MSVC */
-#define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */
-#include <winsock2.h>
-#include <windows.h>
-#endif
-
#include <unistd.h>
#include <stdio.h>
#include <sys/stat.h>
^ permalink raw reply related
* [PATCH v3] am: invoke perl's strftime in C locale
From: Dmitry V. Levin @ 2013-01-15 19:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Antoine Pelisse, git
In-Reply-To: <20130115174015.GA7471@altlinux.org>
This fixes "hg" patch format support for locales other than C and en_*.
Before the change, git-am was making "Date:" line from hg changeset
metadata according to the current locale, and this line was rejected
later with "invalid date format" diagnostics because localized date
strings are not supported.
Reported-by: Gleb Fotengauer-Malinovskiy <glebfm@altlinux.org>
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
v3: alternative implementation using setlocale(LC_TIME, "C")
git-am.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-am.sh b/git-am.sh
index c682d34..8677d8c 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -334,7 +334,8 @@ split_patches () {
# Since we cannot guarantee that the commit message is in
# git-friendly format, we put no Subject: line and just consume
# all of the message as the body
- perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+ perl -M'POSIX qw(strftime :locale_h)' -ne '
+ BEGIN { setlocale(LC_TIME, "C"); $subject = 0 }
if ($subject) { print ; }
elsif (/^\# User /) { s/\# User/From:/ ; print ; }
elsif (/^\# Date /) {
--
ldv
^ permalink raw reply related
* Re: [PATCH v2 07/14] imap-send.c: inline imap_parse_list() in imap_list()
From: Matt Kraai @ 2013-01-15 18:51 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git
In-Reply-To: <1358237193-8887-8-git-send-email-mhagger@alum.mit.edu>
On Tue, Jan 15, 2013 at 09:06:25AM +0100, Michael Haggerty wrote:
> -static struct imap_list *parse_imap_list(struct imap *imap, char **sp)
> +static struct imap_list *parse_list(char **sp)
The commit subject refers to imap_parse_list and imap_list whereas the
code refers to parse_imap_list and parse_list.
^ permalink raw reply
* Re: [PATCH] Allow custom "comment char"
From: Junio C Hamano @ 2013-01-15 19:12 UTC (permalink / raw)
To: Ralf Thielow; +Cc: jrnieder, git
In-Reply-To: <1358275827-5244-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>
> ---
It would have helped if you said you finished the NEEDSWORK: in
builtin/branch.c in the earlier draft with strbuf_commented_*
functions ;-)
Looks like a good progress overall, except for nits here and there.
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 453457a..5e84e35 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";
> + "Write/edit the notes for the following object:";
I think this (and its use site that manually adds "\n#\n") is a
symptom of strbuf_commented_add*() function not designed right.
When it iterates over lines and adds each of them in a commented out
form, it could check if the line is an empty one and refrain from
adding a trailing SP if that is the case. Then this can become
"\nWrite/edit the notes...\n\n";
You have to create the "\n" blank line at the beginning manually,
but that is logically outside the commented out block, so it is not
a problem.
> @@ -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_addf(&buf, "\n%c\n", comment_line_char);
> + strbuf_commented_addstr(&buf, note_template);
> + strbuf_addf(&buf, "\n%c\n", comment_line_char);
> + write_or_die(fd, buf.buf, buf.len);
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 9c3e067..e1b72be 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_("Write a tag message\n"
> + "Lines starting with '%c' will be ignored.");
> ...
> + else
> + strbuf_commented_addf(&buf, _(tag_template_nocleanup), comment_line_char);
> + strbuf_addf(&buf, "\n%c\n", comment_line_char);
> + write_or_die(fd, buf.buf, buf.len);
Same here.
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 22ec5b6..1b8d95f 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -975,13 +975,19 @@ cmd_summary() {
> echo
> done |
> if test -n "$for_status"; then
> + comment_char=`git config core.commentchar`
> + if [ ! -n "$comment_char" ]; then
> + comment_char='#'
> + elif [ ${#comment_char} -gt 1 ]; then
Not portable, I think.
> + echo "$comment_char"
> + sed -e "s|^|$comment_char |" -e "s|^$comment_char $|$comment_char|"
Can $comment_char be a '|'?
> diff --git a/strbuf.c b/strbuf.c
> index 9a373be..8af4b4f 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -204,6 +204,44 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
> va_end(ap);
> }
>
> +void strbuf_commented_addstr(struct strbuf *sb, const char *s)
> +{
> + struct strbuf buf = STRBUF_INIT;
> + struct strbuf prefix = STRBUF_INIT;
> +
> + strbuf_addf(&prefix, "%c ", comment_line_char);
> + strbuf_addstr(&buf, s);
> + strbuf_add_lines(sb, prefix.buf, buf.buf, buf.len);
> +
> + // remove additional '\n' added by strbuf_add_lines()
No C++ comments.
^ permalink raw reply
* Re: [PATCH] am: invoke perl's strftime in C locale
From: Junio C Hamano @ 2013-01-15 19:14 UTC (permalink / raw)
To: Dmitry V. Levin; +Cc: Jeff King, Antoine Pelisse, git
In-Reply-To: <20130115174015.GA7471@altlinux.org>
"Dmitry V. Levin" <ldv@altlinux.org> writes:
> On Tue, Jan 15, 2013 at 08:50:59AM -0800, Jeff King wrote:
>> On Tue, Jan 15, 2013 at 05:42:12PM +0100, Antoine Pelisse wrote:
>>
>> > > This puts all of perl into the C locale, which would mean error messages
>> > > from perl would be in English rather than the user's language. It
>> > > probably isn't a big deal, because that snippet of perl is short and not
>> > > likely to produce problems, but I wonder how hard it would be to set the
>> > > locale just for the strftime call.
>> >
>> > Maybe just setting LC_TIME to C would do ...
>>
>> Yeah, that is a nice simple solution. Dmitry, does just setting LC_TIME
>> fix the problem for you?
>
> Just setting LC_TIME environment variable instead of LC_ALL would end up
> with unreliable solution because LC_ALL has the highest priority.
>
> If keeping error messages from perl has the utmost importance, it could be
> achieved by
> - perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
> + perl -M'POSIX qw(strftime :locale_h)' -ne '
> + BEGIN { setlocale(LC_TIME, "C"); $subject = 0 }
> but the little perl helper script we are talking about hardly worths so
> much efforts.
Yeah I agree that this is not worth it, I would think.
^ permalink raw reply
* Re: [PATCH] attr: fix off-by-one directory component length calculation
From: Jean-Noël AVILA @ 2013-01-15 19:14 UTC (permalink / raw)
To: git
In-Reply-To: <1358256924-31578-1-git-send-email-pclouds@gmail.com>
Thank you for the explanation.
I did not monitor the system calls when writing that patch.
Where is the perf framework?
As the mistake is located in the "find_basename" function, I would propose a
fix directly into it so that the output fits what the other functions expect.
Something in the line of:
diff --git a/attr.c b/attr.c
index d6d7190..b6e72f3 100644
--- a/attr.c
+++ b/attr.c
@@ -572,7 +572,7 @@ static const char *find_basename(const char *path)
if (*cp == '/' && cp[1])
last_slash = cp;
}
- return last_slash ? last_slash + 1 : path;
+ return last_slash ? last_slash : path;
}
static void prepare_attr_stack(const char *path)
@@ -770,6 +770,10 @@ static void collect_all_attrs(const char *path)
check_all_attr[i].value = ATTR__UNKNOWN;
basename = find_basename(path);
+ /* the slash is included in the basename
+ so that it can be matched by a directory pattern */
+ if (basename != path)
+ basename++;
pathlen = strlen(path);
rem = attr_nr;
for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
^ permalink raw reply related
* Re: [PATCH] attr: fix off-by-one directory component length calculation
From: Junio C Hamano @ 2013-01-15 19:29 UTC (permalink / raw)
To: Jean-Noël AVILA; +Cc: git
In-Reply-To: <201301152014.28433.avila.jn@gmail.com>
"Jean-Noël AVILA" <avila.jn@gmail.com> writes:
> Thank you for the explanation.
>
> I did not monitor the system calls when writing that patch.
> Where is the perf framework?
>
> As the mistake is located in the "find_basename" function, I would propose a
> fix directly into it so that the output fits what the other functions expect.
Isn't that a crazy semantics for the function, though? I would
expect find_basename("/a/path/to/file") to return "file", not
"/file".
>
> Something in the line of:
>
> diff --git a/attr.c b/attr.c
> index d6d7190..b6e72f3 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -572,7 +572,7 @@ static const char *find_basename(const char *path)
> if (*cp == '/' && cp[1])
> last_slash = cp;
> }
> - return last_slash ? last_slash + 1 : path;
> + return last_slash ? last_slash : path;
> }
>
> static void prepare_attr_stack(const char *path)
> @@ -770,6 +770,10 @@ static void collect_all_attrs(const char *path)
> check_all_attr[i].value = ATTR__UNKNOWN;
>
> basename = find_basename(path);
> + /* the slash is included in the basename
> + so that it can be matched by a directory pattern */
> + if (basename != path)
> + basename++;
> pathlen = strlen(path);
> rem = attr_nr;
> for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
^ permalink raw reply
* Re: [PATCH v2] test-lib.sh: unfilter GIT_PERF_*
From: Junio C Hamano @ 2013-01-15 19:31 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Thomas Rast
In-Reply-To: <1358257856-3274-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> 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>
> ---
I think the Subject makes more sense, so I'd suggest replacing the
current one with "PERF_", not with "PERF".
> t/test-lib.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index f50f834..e1c8c85 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -85,7 +85,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
> .*_TEST
> PROVE
> VALGRIND
> - PERF_AGGREGATING_LATER
> + PERF
> ));
> my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
> print join("\n", @vars);
^ permalink raw reply
* [RFC/PATCH 2/8 v2] git_remote_helpers: fix input when running under Python 3
From: John Keeping @ 2013-01-15 19:48 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <20130114094721.GQ4574@serenity.lan>
Although 2to3 will fix most issues in Python 2 code to make it run under
Python 3, it does not handle the new strict separation between byte
strings and unicode strings. There is one instance in
git_remote_helpers where we are caught by this, which is when reading
refs from "git for-each-ref".
While we could fix this by explicitly handling refs as byte strings,
this is merely punting the problem to users of the library since the
same problem will be encountered as soon you want to display the ref
name to a user.
Instead of doing this, explicit decode the incoming byte string into a
unicode string. Following the lead of pygit2 (the Python bindings for
libgit2 - see [1] and [2]), use the filesystem encoding by default,
providing a way for callers to override this if necessary.
[1] https://github.com/libgit2/pygit2/blob/e34911b63e5d2266f9f72a4e3f32e27b13190feb/src/pygit2/reference.c#L261
[2] https://github.com/libgit2/pygit2/blob/e34911b63e5d2266f9f72a4e3f32e27b13190feb/include/pygit2/utils.h#L55
Signed-off-by: John Keeping <john@keeping.me.uk>
---
I think this is in fact the best way to handle this, and I hope the
above description clarified why I don't think we want to treat refs as
byte strings in Python 3.
My only remaining question is whether it would be better to set the
error mode when decoding to "replace" instead of "strict" (the default).
"strict" will cause a UnicodeError if the string cannot be decoded
whereas "replace" will use U+FFFD (the replacement character). [3]
I think it's better to use "strict" and let the user know that
something has gone wrong rather than silently change the string, but I'd
welcome other opinions.
[3] http://docs.python.org/2/library/codecs.html#codec-base-classes
git_remote_helpers/git/importer.py | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py
index e28cc8f..5bc16a4 100644
--- a/git_remote_helpers/git/importer.py
+++ b/git_remote_helpers/git/importer.py
@@ -1,5 +1,6 @@
import os
import subprocess
+import sys
from git_remote_helpers.util import check_call, check_output
@@ -10,17 +11,26 @@ class GitImporter(object):
This importer simply delegates to git fast-import.
"""
- def __init__(self, repo):
+ def __init__(self, repo, ref_encoding=None):
"""Creates a new importer for the specified repo.
+
+ If ref_encoding is specified that refs are decoded using that
+ encoding. Otherwise the system filesystem encoding is used.
"""
self.repo = repo
+ self.ref_encoding = ref_encoding
def get_refs(self, gitdir):
"""Returns a dictionary with refs.
"""
args = ["git", "--git-dir=" + gitdir, "for-each-ref", "refs/heads"]
- lines = check_output(args).strip().split('\n')
+ encoding = self.ref_encoding
+ if encoding is None:
+ encoding = sys.getfilesystemencoding()
+ if encoding is None:
+ encoding = sys.getdefaultencoding()
+ lines = check_output(args).decode(encoding).strip().split('\n')
refs = {}
for line in lines:
value, name = line.split(' ')
--
1.8.1
^ permalink raw reply related
* Re: [PATCH] attr: fix off-by-one directory component length calculation
From: Jean-Noël AVILA @ 2013-01-15 19:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwqve2qk3.fsf@alter.siamese.dyndns.org>
Le mardi 15 janvier 2013 20:29:16, Junio C Hamano a écrit :
> "Jean-Noël AVILA" <avila.jn@gmail.com> writes:
> > Thank you for the explanation.
> >
> > I did not monitor the system calls when writing that patch.
> > Where is the perf framework?
> >
> > As the mistake is located in the "find_basename" function, I would
> > propose a fix directly into it so that the output fits what the other
> > functions expect.
>
> Isn't that a crazy semantics for the function, though? I would
> expect find_basename("/a/path/to/file") to return "file", not
> "/file".
>
Yes, it is. I was wrong on the meaning of basename.
Btw, the test 10 to t9902 is failing on my Debian testing. Is it a known
issue?
^ permalink raw reply
* Re: [PATCH] remote-hg: store converted URL
From: Junio C Hamano @ 2013-01-15 20:10 UTC (permalink / raw)
To: Max Horn; +Cc: git, Felipe Contreras
In-Reply-To: <B35B3EA6-F01B-46D8-AC3D-0F7C8A45A06B@quendi.de>
Max Horn <max@quendi.de> writes:
> So far, all I look at do not deal with this at all. Any attempts
> to deal with it should be pretty easy to recognize: The
> remote-$backend would have to store something into the git config,
> or else, verify the opaque token and refuse to work with it under
> certain conditions (e.g. when it contains a relative path). But
> they don't. E.g. git-remote-testgit has the exact same problem.
Thanks for confirming what I suspected. I think the way Felipe's
patch makes remote-hg take responsibility of how $opaqueToken should
look like for future invocations is the simplest and makes the most
sense. We could try to go fancier and end up over-engineering, but
I'd rather have a simple fix in the tree first.
Thanks.
^ permalink raw reply
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Torsten Bögershausen @ 2013-01-15 20:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Torsten Bögershausen, git, kraai
In-Reply-To: <7v4nikiu81.fsf@alter.siamese.dyndns.org>
On 13.01.13 23:38, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Hi,
>>
>> Torsten Bögershausen wrote:
>>
>>> - /^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
>>> + /^\s*[^#]\s*which\s+[-a-zA-Z0-9]+$/ and err 'which is not portable (please use type)';
>>
>> Hmm. Neither the old version nor the new one matches what seem to
>> be typical uses of 'which', based on a quick code search:
>>
>> if which sl >/dev/null 2>&1
>> then
>> sl -l
>> ...
>> fi
>>
>> or
>>
>> if test -x "$(which sl 2>/dev/null)"
>> then
>> sl -l
>> ...
>> fi
>
> Yes, these two misuses are what we want it to trigger on, so the
> test is very easy to trigger and produce a false positive, but does
> not trigger on what we really want to catch.
>
> That does not sound like a good benefit/cost ratio to me.
>
Thanks for comments, I think writing a regexp for which is difficult.
What do we think about something like this for fishing for which:
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -644,6 +644,10 @@ yes () {
:
done
}
+which () {
+ echo >&2 "which is not portable (please use type)"
+ exit 1
+}
This will happen in runtime, which might be good enough ?
@Matt:
>The "[^#]" appears to ensure that there's at least one character
>before the which and that it's not a pound sign. Why is this done?
This is simply wrong.
^ permalink raw reply
* Re: [RFC/PATCH] ignore memcmp() overreading in bsearch() callback
From: Andreas Schwab @ 2013-01-15 20:27 UTC (permalink / raw)
To: René Scharfe
Cc: Junio C Hamano, git, Jeff King, Carlos Martín Nieto,
Johannes Schindelin
In-Reply-To: <50F57BDF.1050400@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> + return '\0' - ent->name[key->len];
You need to cast to unsigned char first to make it consistent with
memcmp and strcmp.
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: [PATCH v2 06/14] imap-send.c: remove some unused fields from struct store
From: Jonathan Nieder @ 2013-01-15 20:32 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <1358237193-8887-7-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty wrote:
> - else if ((arg1 = next_arg(&cmd))) {
> - if (!strcmp("EXISTS", arg1))
> - ctx->gen.count = atoi(arg);
> - else if (!strcmp("RECENT", arg1))
> - ctx->gen.recent = atoi(arg);
> + } else if ((arg1 = next_arg(&cmd))) {
> + /* unused */
The above is just the right thing to do to ensure no behavior change.
Let's take a look at the resulting code, though:
if (... various reasonable things ...) {
...
} else if ((arg1 = next_arg(&cmd))) {
/* unused */
} else {
fprintf(stderr, "IMAP error: unable to parse untagged response\n");
return RESP_BAD;
Anyone forced by some bug to examine this "/* unused */" case is going
to have no clue what's going on. In that respect, the old code was
much better, since it at least made it clear that one case where this
code gets hit is handling "<num> EXISTS" and "<num> RECENT" untagged
responses.
I suspect that original code did not have an implicit and intended
missing
else
; /* negligible response; ignore it */
but the intent was rather
else {
fprintf(stderr, "IMAP error: I can't parse this\n");
return RESP_BAD;
}
Since actually fixing that is probably too aggressive for this patch,
how about a FIXME comment like the following?
/*
* Unhandled response-data with at least two words.
* Ignore it.
*
* NEEDSWORK: Previously this case handled '<num> EXISTS'
* and '<num> RECENT' but as a probably-unintended side
* effect it ignores other unrecognized two-word
* responses. imap-send doesn't ever try to read
* messages or mailboxes these days, so consider
* eliminating this case.
*/
^ permalink raw reply
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Junio C Hamano @ 2013-01-15 20:38 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Jonathan Nieder, git, kraai
In-Reply-To: <50F5B83E.9060800@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> What do we think about something like this for fishing for which:
>
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -644,6 +644,10 @@ yes () {
> :
> done
> }
> +which () {
> + echo >&2 "which is not portable (please use type)"
> + exit 1
> +}
>
>
> This will happen in runtime, which might be good enough ?
if (
which frotz &&
test $(frobonitz --version" -le 2.0
)
then
test_set_prereq FROTZ_FROBONITZ
else
echo >&2 "suitable Frotz/Frobonitz combo not available;"
echo >&2 "some tests may be skipped"
fi
I somehow think this is a lost cause.
^ permalink raw reply
* Re: [PATCH] attr: fix off-by-one directory component length calculation
From: Junio C Hamano @ 2013-01-15 20:49 UTC (permalink / raw)
To: Jean-Noël AVILA; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <201301152053.58561.avila.jn@gmail.com>
"Jean-Noël AVILA" <avila.jn@gmail.com> writes:
> Btw, the test 10 to t9902 is failing on my Debian testing. Is it a known
> issue?
Which branch?
If you mean "'master' with the patch in this discussion applied", I
didn't even have a chance to start today's integration cycle, so I
don't know (it is not known to me).
^ permalink raw reply
* Re: [PATCH v2 00/14] Remove unused code from imap-send.c
From: Jonathan Nieder @ 2013-01-15 20:49 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <1358237193-8887-1-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty wrote:
> imap-send.c | 308 +++++++++++-------------------------------------------------
> 1 file changed, 55 insertions(+), 253 deletions(-)
Patch 14 is lovely. Except for patch 6, for what it's worth these are
all
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Nicely done.
^ permalink raw reply
* Re: [RFC/PATCH 2/8 v2] git_remote_helpers: fix input when running under Python 3
From: Junio C Hamano @ 2013-01-15 20:51 UTC (permalink / raw)
To: John Keeping
Cc: Michael Haggerty, git, Eric S. Raymond, Felipe Contreras,
Sverre Rabbelier
In-Reply-To: <20130115194809.GU4574@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> Although 2to3 will fix most issues in Python 2 code to make it run under
> Python 3, it does not handle the new strict separation between byte
> strings and unicode strings. There is one instance in
> git_remote_helpers where we are caught by this, which is when reading
> refs from "git for-each-ref".
>
> While we could fix this by explicitly handling refs as byte strings,
> this is merely punting the problem to users of the library since the
> same problem will be encountered as soon you want to display the ref
> name to a user.
>
> Instead of doing this, explicit decode the incoming byte string into a
> unicode string.
That really feels wrong. Displaying is a separate issue and it is
the _right_ thing to punt the problem at the lower-level machinery
level.
> Following the lead of pygit2 (the Python bindings for
> libgit2 - see [1] and [2]),...
I do not think other people getting it wrong is not an excuse to
repeat the same mistake.
Is it really so cumbersome to handle byte strings as byte strings in
Python?
^ permalink raw reply
* [PATCH] config.txt: Document help.htmlpath config parameter
From: Sebastian Staudt @ 2013-01-15 20:56 UTC (permalink / raw)
To: git
Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
---
Documentation/config.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bf8f911..e452ff8 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1351,6 +1351,12 @@ help.autocorrect::
value is 0 - the command will be just shown but not executed.
This is the default.
+help.htmlpath::
+ Specify the path where the HTML documentation resides. File system paths
+ and URLs are supported. HTML pages will be prefixed with this path when
+ help is displayed in the 'web' format. This defaults to the documentation
+ path of your Git installation.
+
http.proxy::
Override the HTTP proxy, normally configured using the 'http_proxy',
'https_proxy', and 'all_proxy' environment variables (see
--
1.8.1.1
^ permalink raw reply related
* Re: [PATCH] config.txt: Document help.htmlpath config parameter
From: Junio C Hamano @ 2013-01-15 21:51 UTC (permalink / raw)
To: Sebastian Staudt; +Cc: git
In-Reply-To: <20130115205621.GB49671@mormegil.local>
Thanks.
This will eventually go to the maintenance track as well.
^ 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