Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?
Jeff King p...@peff.net writes: On Thu, Oct 18, 2012 at 07:31:35AM +0200, Johannes Sixt wrote: Right. But we should really be doing something like this instead to save a few subprocesses. [...] -eval $(set_ident AUTHOR ../commit) || +eval $(set_ident AUTHOR author ../commit) || I cringe a little at losing DRY-ness to avoid processes. Well, the header field token author and the middle word of the variable GIT_AUTHOR_NAME _happen_ to be the same modulo case, but they did not have to be, so you could argue the updated set_ident implementation is more generally useful (you could even argue that we should spell the first parameter out as GIT_AUTHOR_NAME and GIT_AUTHOR_EMAIL, two separate parameters). Speaking of repetition, this seems like almost the exact same parsing that happens in git-sh-setup's get_author_ident_from_commit. Maybe it's worth merging them. I suspect you could also avoid another process by parsing out both author and committer information in the same sed invocation. Yes, yes and yes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?
On Wed, Oct 17, 2012 at 11:06:11PM -0700, Junio C Hamano wrote: - eval $(set_ident AUTHOR ../commit) || + eval $(set_ident AUTHOR author ../commit) || I cringe a little at losing DRY-ness to avoid processes. Well, the header field token author and the middle word of the variable GIT_AUTHOR_NAME _happen_ to be the same modulo case, but they did not have to be, so you could argue the updated set_ident implementation is more generally useful (you could even argue that we should spell the first parameter out as GIT_AUTHOR_NAME and GIT_AUTHOR_EMAIL, two separate parameters). True, though that is even more work for the caller (and *_DATE, too). We could make it GIT_AUTHOR, but I don't think there is much point in being that level of half-way general. The caller can always pick it out of the variables if they really want to do something tricky. Speaking of repetition, this seems like almost the exact same parsing that happens in git-sh-setup's get_author_ident_from_commit. Maybe it's worth merging them. I suspect you could also avoid another process by parsing out both author and committer information in the same sed invocation. Yes, yes and yes. Working on it now. git-sh-setup works, but chasing an annoying bug in filter-branch. I'm sure it's something silly and stupid. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add new git-remote-hd helper
On Wed, Oct 17, 2012 at 10:18 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Right now I've just added an error when using remote repositories. But it seems there's no way around it; if we want to have support for remote repos, we need to make a local clone. My git-remote-hg does the local clone into .git/ using a hash of the url (although you could just as well use urlencode, basically any way to safely use a url as a directory name). Have a look if you want. -- Cheers, Sverre Rabbelier -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] clean up filter-branch ident parsing
On Thu, Oct 18, 2012 at 02:08:47AM -0400, Jeff King wrote: Working on it now. git-sh-setup works, but chasing an annoying bug in filter-branch. I'm sure it's something silly and stupid. Ugh. I was being caught by crazy dash-versus-bash stuff. Try this: $ bash -c 'echo ' \\ $ dash -c 'echo ' \ It's the echo will automatically do backslash escaping magic we have so often enjoyed. I solved it with printf. Patches to follow. My primary motivation was cleanup, but it also has a net reduction of 5 fork+execs for each commit that filter-branch processes. This dropped the run-time of git filter-branch HEAD -1000 on my linux box from 62s to 47s. A real filter-branch would do more work in the filters, of course, but that translates to 7.5 minutes of time saved if you are filtering all 30,000 commits of git.git. [1/2]: git-sh-setup: refactor ident-parsing functions [2/2]: filter-branch: use git-sh-setup's ident parsing functions -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] git-sh-setup: refactor ident-parsing functions
The only ident-parsing function we currently provide is
get_author_ident_from_commit. This is not very
flexible for two reasons:
1. It takes a commit as an argument, and can't read from
commit headers saved on disk.
2. It will only parse authors, not committers.
This patch provides a more flexible interface which will
parse multiple idents from a commit provide on stdin. We can
easily use it as a building block for the current function
to retain compatibility.
Signed-off-by: Jeff King p...@peff.net
---
Since we are counting processes in this series, I should note that this
actually adds a subshell invocation for each call, since it went from:
script='...'
sed $script
to:
sed $(make_script)
For filter-branch, which is really the only high-performance caller we
have, this is negated by the fact that it will do author and committer
at the same time, saving us an extra subshell (in addition to an extra
sed invocation).
git-sh-setup.sh | 62 +++--
1 file changed, 43 insertions(+), 19 deletions(-)
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index ee0e0bc..22f0aed 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -191,28 +191,52 @@ get_author_ident_from_commit () {
fi
}
+# Generate a sed script to parse identities from a commit.
+#
+# Reads the commit from stdin, which should be in raw format (e.g., from
+# cat-file or --pretty=raw).
+#
+# The first argument specifies the ident line to parse (e.g., author), and
+# the second specifies the environment variable to put it in (e.g., AUTHOR
+# for GIT_AUTHOR_*). Multiple pairs can be given to parse author and
+# committer.
+pick_ident_script () {
+ while test $# -gt 0
+ do
+ lid=$1; shift
+ uid=$1; shift
+ printf '%s'
+ /^$lid /{
+ s/'/'''/g
+ h
+ s/^$lid '\([^]*\) [^]* .*$/\1/'
+ s/.*/GIT_${uid}_NAME=''/p
+
+ g
+ s/^$lid '[^]* \([^]*\) .*$/\1/'
+ s/.*/GIT_${uid}_EMAIL=''/p
+
+ g
+ s/^$lid '[^]* [^]* \(.*\)$/@\1/'
+ s/.*/GIT_${uid}_DATE=''/p
+ }
+
+ done
+ echo '/^$/q'
+}
+
+# Create a pick-script as above and feed it to sed. Stdout is suitable for
+# feeding to eval.
+parse_ident_from_commit () {
+ LANG=C LC_ALL=C sed -ne $(pick_ident_script $@)
+}
+
+# Parse the author from a commit given as an argument. Stdout is suitable for
+# feeding to eval to set the usual GIT_* ident variables.
get_author_ident_from_commit () {
- pick_author_script='
- /^author /{
- s/'\''/'\''\\'\'\''/g
- h
- s/^author \([^]*\) [^]* .*$/\1/
- s/.*/GIT_AUTHOR_NAME='\'''\''/p
-
- g
- s/^author [^]* \([^]*\) .*$/\1/
- s/.*/GIT_AUTHOR_EMAIL='\'''\''/p
-
- g
- s/^author [^]* [^]* \(.*\)$/@\1/
- s/.*/GIT_AUTHOR_DATE='\'''\''/p
-
- q
- }
- '
encoding=$(git config i18n.commitencoding || echo UTF-8)
git show -s --pretty=raw --encoding=$encoding $1 -- |
- LANG=C LC_ALL=C sed -ne $pick_author_script
+ parse_ident_from_commit author AUTHOR
}
# Clear repo-local GIT_* environment variables. Useful when switching to
--
1.8.0.rc3.3.gba630e1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
This saves us some code, but it also reduces the number of
processes we start for each filtered commit. Since we can
parse both author and committer in the same sed invocation,
we save one process. And since the new interface avoids tr,
we save 4 processes.
It also avoids using tr, which has had some odd
portability problems reported with from Solaris's xpg6
version.
Signed-off-by: Jeff King p...@peff.net
---
git-filter-branch.sh | 42 +-
1 file changed, 9 insertions(+), 33 deletions(-)
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 178e453..69406ae 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -64,37 +64,15 @@ set_ident () {
eval $functions
-# When piped a commit, output a script to set the ident of either
-# author or committer
+# Ensure non-empty id name.
+fallback_name() {
+ echo case \\$GIT_$1_NAME\ in \\)
GIT_$1_NAME=\\${GIT_$1_EMAIL%%@*}\ export GIT_$1_NAME;; esac
+}
set_ident () {
- lid=$(echo $1 | tr [A-Z] [a-z])
- uid=$(echo $1 | tr [a-z] [A-Z])
- pick_id_script='
- /^'$lid' /{
- s/'\''/'\''\\'\'\''/g
- h
- s/^'$lid' \([^]*\) [^]* .*$/\1/
- s/'\''/'\''\'\'\''/g
- s/.*/GIT_'$uid'_NAME='\'''\''; export GIT_'$uid'_NAME/p
-
- g
- s/^'$lid' [^]* \([^]*\) .*$/\1/
- s/'\''/'\''\'\'\''/g
- s/.*/GIT_'$uid'_EMAIL='\'''\''; export
GIT_'$uid'_EMAIL/p
-
- g
- s/^'$lid' [^]* [^]* \(.*\)$/@\1/
- s/'\''/'\''\'\'\''/g
- s/.*/GIT_'$uid'_DATE='\'''\''; export GIT_'$uid'_DATE/p
-
- q
- }
- '
-
- LANG=C LC_ALL=C sed -ne $pick_id_script
- # Ensure non-empty id name.
- echo case \\$GIT_${uid}_NAME\ in \\)
GIT_${uid}_NAME=\\${GIT_${uid}_EMAIL%%@*}\ export GIT_${uid}_NAME;; esac
+ parse_ident_from_commit author AUTHOR committer COMMITTER
+ fallback_name AUTHOR
+ fallback_name COMMITTER
}
USAGE=[--env-filter command] [--tree-filter command]
@@ -320,10 +298,8 @@ while read commit parents; do
git cat-file commit $commit ../commit ||
die Cannot read commit $commit
- eval $(set_ident AUTHOR ../commit) ||
- die setting author failed for commit $commit
- eval $(set_ident COMMITTER ../commit) ||
- die setting committer failed for commit $commit
+ eval $(set_ident ../commit) ||
+ die setting author/committer failed for commit $commit
eval $filter_env /dev/null ||
die env filter failed: $filter_env
--
1.8.0.rc3.3.gba630e1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] transport-helper: call git fast-import properly
On Thu, Oct 18, 2012 at 7:13 AM, Sverre Rabbelier srabbel...@gmail.com wrote:
On Wed, Oct 17, 2012 at 1:27 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
The marks options are being ignored right now.
It seems unlikely to me that this never worked, surely no reviewer
would accept a patch that doesn't actually implement the feature?
What's the history here?
Now I see, the {import,export}-marks options are only meant for
fast-export, for fast-import one should use the 'feature' commands. It
took me a while because the git_remote_helper code for python is very
confusing: it uses testgit.marks for the marks that git generates, and
git.marks for the marks that testgit generates.
It's not very convenient for remote-helpers that can export single
branches as opposed to the whole repo:
http://github.com/felipec/git/commit/0961fdf8231a4ac057eec8a306a708e66f7b6ae9
But it works, so this patch is not needed.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
Am 10/18/2012 9:25, schrieb Jeff King:
-# When piped a commit, output a script to set the ident of either
-# author or committer
+# Ensure non-empty id name.
+fallback_name() {
+ echo case \\$GIT_$1_NAME\ in \\)
GIT_$1_NAME=\\${GIT_$1_EMAIL%%@*}\ export GIT_$1_NAME;; esac
+}
set_ident () {
- lid=$(echo $1 | tr [A-Z] [a-z])
- uid=$(echo $1 | tr [a-z] [A-Z])
- pick_id_script='
- /^'$lid' /{
- s/'\''/'\''\\'\'\''/g
- h
- s/^'$lid' \([^]*\) [^]* .*$/\1/
- s/'\''/'\''\'\'\''/g
- s/.*/GIT_'$uid'_NAME='\'''\''; export GIT_'$uid'_NAME/p
-
- g
- s/^'$lid' [^]* \([^]*\) .*$/\1/
- s/'\''/'\''\'\'\''/g
- s/.*/GIT_'$uid'_EMAIL='\'''\''; export
GIT_'$uid'_EMAIL/p
-
- g
- s/^'$lid' [^]* [^]* \(.*\)$/@\1/
- s/'\''/'\''\'\'\''/g
- s/.*/GIT_'$uid'_DATE='\'''\''; export GIT_'$uid'_DATE/p
-
- q
- }
- '
-
- LANG=C LC_ALL=C sed -ne $pick_id_script
- # Ensure non-empty id name.
- echo case \\$GIT_${uid}_NAME\ in \\)
GIT_${uid}_NAME=\\${GIT_${uid}_EMAIL%%@*}\ export GIT_${uid}_NAME;; esac
+ parse_ident_from_commit author AUTHOR committer COMMITTER
+ fallback_name AUTHOR
+ fallback_name COMMITTER
}
Didn't you lose the export GIT_$uid_{NAME,EMAIL,DATE} parts somewhere on
the way?
Thanks for working on this!
-- Hannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] pretty: prepare notes message at a centralized place
On Wed, Oct 17, 2012 at 10:45:25PM -0700, Junio C Hamano wrote:
+ if (opt-show_notes) {
+ int raw;
+ struct strbuf notebuf = STRBUF_INIT;
+
+ raw = (opt-commit_format == CMIT_FMT_USERFORMAT);
+ format_display_notes(commit-object.sha1, notebuf,
+ get_log_output_encoding(), raw);
+ ctx.notes_message = notebuf.len
+ ? strbuf_detach(notebuf, NULL)
+ : xcalloc(1, 1);
+ }
This last line seems like it is caused by a bug in the strbuf API.
Detaching an empty string will sometimes get you NULL and sometimes not.
For example, this:
struct strbuf foo = STRBUF_INIT;
strbuf_detach(foo, NULL);
will return NULL. But this:
struct strbuf foo = STRBUF_INIT;
strbuf_addstr(foo, bar);
strbuf_reset(foo);
strbuf_detach(foo, NULL);
will get you a zero-length string. Which just seems insane to me. The
whole point of strbuf_detach is that you do not have to care about the
internal representation. It should probably always return a newly
allocated zero-length string.
Looking through a few uses of strbuf_detach, it looks like callers
assume they will always get a pointer from strbuf_detach, and we are
saved by implementation details. For example, sha1_file_to_archive might
have an empty file, but the fact that strbuf_attach always allocates a
byte means that the detach will never return NULL. Similarly,
argv_array_pushf would never want to turn an empty string into an
accidental NULL; it is saved by the fact that strbuf_vaddf will always
preemptively allocate 64 bytes.
It's possible that switching it would create bugs elsewhere (there are
over 100 uses of strbuf_detach, so maybe somebody really does want this
NULL behavior), but I tend to think it is just as likely to be fixing
undiscovered bugs.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
On Thu, Oct 18, 2012 at 09:49:04AM +0200, Johannes Sixt wrote:
- s/.*/GIT_'$uid'_NAME='\'''\''; export GIT_'$uid'_NAME/p
Didn't you lose the export GIT_$uid_{NAME,EMAIL,DATE} parts somewhere on
the way?
Yikes, you're right. I didn't even notice, as the test suite still
passes. I can see how the env filter would still be able to see the
variables, but the commit-tree call wouldn't. I guess it happens to work
because we do not test alternate idents in our filter branch tests (IOW,
we are silently rewriting each commit during the filter-branch, but it
happens to have the same identities).
I'll investigate.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add new git-remote-hd helper
Hi Felipe, On Wed, 17 Oct 2012, Felipe Contreras wrote: On Wed, Oct 17, 2012 at 8:18 PM, Sverre Rabbelier srabbel...@gmail.com wrote: On Wed, Oct 17, 2012 at 11:12 AM, Felipe Contreras felipe.contre...@gmail.com wrote: But fine, lets remove the tests out of the equation (150 lines), the number of lines of code still exceeds 3000. I don't think it's fair to just look at LOC, git-remote-hg when it was just parsing was fairly simple. Most of the current code is our copy of the python fast-import library which is only used to support pushing to mercurial. Well, as a rule of thumb more code means more places for bugs to hide. Everybody on this list knows that. But it is equally true that more functionality requires more code. Besides, we are talking about concrete code, so there is no need at all for handwaving arguments. GitHub makes it easy to point at exact line numbers in exact file names in exact revisions, as you know, and we should use that to discuss code. It is also quite frankly rather difficult to navigate; very spaghetti-like. I have the feeling [...] Yours truly always welcomes constructive criticism. Other types of criticism, not so much. As to the functionality you seek: git-remote-hg found in git://github.com/msysgit/git works. It has the following advantages over every other solution, including the one proposed in this thread: - it works - no really, it works - it supports pushes, too - it matured over a long time - there are tests - whenever we fixed bugs, we also added tests for the bug fixes - it is rock solid - it is in constant use Without push support, remote-hg is useless to me. Without regression tests proving that it is rock solid, I will not use remote-hg. And I will not indulge in efforts to duplicate work. If there are concerns about code style or unnecessary code (insofar it is really unnecessary, testgit for example is not, unless you want to avoid robust regression tests), I will discuss issues and collaborate. If the idea was not to collaborate, but to show off how much shorter code can be when it lacks functionality and proof of robustness I require for my everyday use of the program, dismissing existing code and concepts, less so. Hth, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add new git-remote-hd helper
On Thu, Oct 18, 2012 at 5:44 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Thu, Oct 18, 2012 at 12:59 AM, Jeff King p...@peff.net wrote: The first thing I tried was: $ git clone hg::https://code.google.com/p/dactyl/ Right, doesn't look like it works for remote repositories. I think that's the next feature I want to implement, but to be honest, I don't think it's a big issue. To replace this: Done, now you should be able to clone and fetch remote repositories :) https://github.com/felipec/git/commit/783e4b380ab4fabb4e2fb200722c92afc8494a83 -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add new git-remote-hd helper
On Thu, Oct 18, 2012 at 10:47 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi Felipe, On Wed, 17 Oct 2012, Felipe Contreras wrote: On Wed, Oct 17, 2012 at 8:18 PM, Sverre Rabbelier srabbel...@gmail.com wrote: On Wed, Oct 17, 2012 at 11:12 AM, Felipe Contreras felipe.contre...@gmail.com wrote: But fine, lets remove the tests out of the equation (150 lines), the number of lines of code still exceeds 3000. I don't think it's fair to just look at LOC, git-remote-hg when it was just parsing was fairly simple. Most of the current code is our copy of the python fast-import library which is only used to support pushing to mercurial. Well, as a rule of thumb more code means more places for bugs to hide. Everybody on this list knows that. But it is equally true that more functionality requires more code. Not necessarily. There's projects with more code and less functionality than their alternatives. It is also quite frankly rather difficult to navigate; very spaghetti-like. I have the feeling [...] Yours truly always welcomes constructive criticism. Other types of criticism, not so much. As to the functionality you seek: git-remote-hg found in git://github.com/msysgit/git works. It has the following advantages over every other solution, including the one proposed in this thread: - it works - no really, it works Not for me. - it supports pushes, too I don't care. When I need that I'll implement that with probably much less code. - it matured over a long time So has CVS. - there are tests Only a dozen of them. If I write the same tests for my solution would you be happier? I didn't think so. - whenever we fixed bugs, we also added tests for the bug fixes Like this one? https://github.com/msysgit/git/commit/9f934c9987981cbecf4ebaf8eb4a8e9f1d002caf I don't see any tests for that. - it is in constant use So you say, my impression is different. Without push support, remote-hg is useless to me. Different people have different needs. Without an easy way to setup, remote-hg is useless to me. If there are concerns about code style or unnecessary code (insofar it is really unnecessary, testgit for example is not, unless you want to avoid robust regression tests), I will discuss issues and collaborate. If the idea was not to collaborate, but to show off how much shorter code can be when it lacks functionality and proof of robustness I require for my everyday use of the program, dismissing existing code and concepts, less so. So your idea of collaboration is accept that your code is awesome, and my code sucks, and that I should fix your code, and throw my code to the trash, while you do absolutely nothing but complain about the whole situation. I have at least looked at your code. Have you even looked at mine? I've done my part in making my code easily available and ready for review. I will not reply to you anymore until you show your willingness to collaborate that you seem to demand for me, and: 1) Point to a remote-hg branch that is independent of msysgit stuff, or any other irrelevant stuff 2) Is based on top of a recent version of git Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add new git-remote-hd helper
On Thu, Oct 18, 2012 at 8:12 AM, Sverre Rabbelier srabbel...@gmail.com wrote: On Wed, Oct 17, 2012 at 10:18 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Right now I've just added an error when using remote repositories. But it seems there's no way around it; if we want to have support for remote repos, we need to make a local clone. My git-remote-hg does the local clone into .git/ using a hash of the url (although you could just as well use urlencode, basically any way to safely use a url as a directory name). Have a look if you want. Can you point to the version you are talking about? I've been checking the remote-hg branch of fingolfin. https://github.com/fingolfin/git/tree/remote-hg/ -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] pretty: prepare notes message at a centralized place
Jeff King p...@peff.net writes: It's possible that switching it would create bugs elsewhere (there are over 100 uses of strbuf_detach, so maybe somebody really does want this NULL behavior), but I tend to think it is just as likely to be fixing undiscovered bugs. Yeah, I tend to agree. This format-patch --notes is obviously a post 1.8.0 topic, and so is the strbuf_detach() clean-up. Let me bookmark this thread in case it hasn't been resolved when I came back from my vacation, so that I won't forget ;-). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add new git-remote-hd helper
Hi Felipe, On Thu, 18 Oct 2012, Felipe Contreras wrote: On Thu, Oct 18, 2012 at 10:47 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: So your idea of collaboration is accept that your code is awesome, and my code sucks, and that I should fix your code, and throw my code to the trash, while you do absolutely nothing but complain about the whole situation. I said no such thing. I like to keep things professional and keep emotions out. Hth, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] pretty: prepare notes message at a centralized place
On Thu, Oct 18, 2012 at 02:17:01AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: It's possible that switching it would create bugs elsewhere (there are over 100 uses of strbuf_detach, so maybe somebody really does want this NULL behavior), but I tend to think it is just as likely to be fixing undiscovered bugs. Yeah, I tend to agree. This format-patch --notes is obviously a post 1.8.0 topic, and so is the strbuf_detach() clean-up. Let me bookmark this thread in case it hasn't been resolved when I came back from my vacation, so that I won't forget ;-). Actually, I have found a few segfaults, one of them remotely triggerable in http-backend. I think it can probably wait until post-1.8.0 as it does not have any security implications, though. Details in a moment. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add new git-remote-hd helper
Hi, On Thu, 18 Oct 2012, Felipe Contreras wrote: On Thu, Oct 18, 2012 at 8:12 AM, Sverre Rabbelier srabbel...@gmail.com wrote: On Wed, Oct 17, 2012 at 10:18 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Right now I've just added an error when using remote repositories. But it seems there's no way around it; if we want to have support for remote repos, we need to make a local clone. My git-remote-hg does the local clone into .git/ using a hash of the url (although you could just as well use urlencode, basically any way to safely use a url as a directory name). Have a look if you want. Can you point to the version you are talking about? I've been checking the remote-hg branch of fingolfin. https://github.com/fingolfin/git/tree/remote-hg/ The code is in https://github.com/msysgit/git/ (Sverre does not have enough time to work on remote-hg, and was okay with me hosting it in Git for Windows, for all the reasons I mentioned earlier in this thread). Hth, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add new git-remote-hd helper
On Thu, Oct 18, 2012 at 11:13 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi, On Thu, 18 Oct 2012, Felipe Contreras wrote: On Thu, Oct 18, 2012 at 8:12 AM, Sverre Rabbelier srabbel...@gmail.com wrote: On Wed, Oct 17, 2012 at 10:18 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Right now I've just added an error when using remote repositories. But it seems there's no way around it; if we want to have support for remote repos, we need to make a local clone. My git-remote-hg does the local clone into .git/ using a hash of the url (although you could just as well use urlencode, basically any way to safely use a url as a directory name). Have a look if you want. Can you point to the version you are talking about? I've been checking the remote-hg branch of fingolfin. https://github.com/fingolfin/git/tree/remote-hg/ The code is in https://github.com/msysgit/git/ (Sverre does not have enough time to work on remote-hg, and was okay with me hosting it in Git for Windows, for all the reasons I mentioned earlier in this thread). Which branch? I don't see any remote-hg branch. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add new git-remote-hd helper
Felipe Contreras felipe.contre...@gmail.com writes: As to the functionality you seek: git-remote-hg found in git://github.com/msysgit/git works. It has the following advantages over every other solution, including the one proposed in this thread: - it works - no really, it works Not for me. Felipe, an argument along this line would not help very much. Please elaborate a bit to describe what does not work and where you are having problems more concretely. Even for people who may want to see if they agree with your It does not work, Not for me alone is too little for them to try out. Others who may want to and are capable of helping you won't know where to start. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add new git-remote-hd helper
On Thu, Oct 18, 2012 at 11:26 AM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: As to the functionality you seek: git-remote-hg found in git://github.com/msysgit/git works. It has the following advantages over every other solution, including the one proposed in this thread: - it works - no really, it works Not for me. Felipe, an argument along this line would not help very much. Please elaborate a bit to describe what does not work and where you are having problems more concretely. Even for people who may want to see if they agree with your It does not work, Not for me alone is too little for them to try out. Others who may want to and are capable of helping you won't know where to start. Basically what I already described: 1) You need a non-vanilla version of git 2) It's not easy to set up 3) It's not even clear which branch you should be using (in case you are not using msysgit) -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add new git-remote-hd helper
Felipe Contreras felipe.contre...@gmail.com writes: Basically what I already described: 1) You need a non-vanilla version of git 2) It's not easy to set up 3) It's not even clear which branch you should be using (in case you are not using msysgit) I do not read that as it does not work, but instead as no one took the time to push this code into git.git (although someone did in msysgit). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] strbuf: always return a non-NULL value from strbuf_detach
On Thu, Oct 18, 2012 at 03:49:21AM -0400, Jeff King wrote:
This last line seems like it is caused by a bug in the strbuf API.
Detaching an empty string will sometimes get you NULL and sometimes not.
For example, this:
struct strbuf foo = STRBUF_INIT;
strbuf_detach(foo, NULL);
will return NULL. But this:
struct strbuf foo = STRBUF_INIT;
strbuf_addstr(foo, bar);
strbuf_reset(foo);
strbuf_detach(foo, NULL);
will get you a zero-length string. Which just seems insane to me. The
whole point of strbuf_detach is that you do not have to care about the
internal representation. It should probably always return a newly
allocated zero-length string.
[...]
It's possible that switching it would create bugs elsewhere (there are
over 100 uses of strbuf_detach, so maybe somebody really does want this
NULL behavior), but I tend to think it is just as likely to be fixing
undiscovered bugs.
So I just read through all 108 grep hits for strbuf_detach. In almost
every case, the NULL return is not triggerable, because we do _some_
strbuf operation. And even if it is empty, like strbuf_read from an
empty source, or strbuf_addstr on an empty string, we still end up
calling `strbuf_grow(sb, 0)`, which will allocate.
There are a few cases where there are code paths where we really might
not add anything to the strbuf, and changing strbuf_detach would have an
impact:
In log.c:cmd_format_patch, creating the rev.extra_headers string from
the individual headers currently ends up NULL when you have no such
headers. But it ends up not mattering if we have NULL or an empty
string, since all code paths just end up appending it to our headers
anyway, and the empty string is a noop.
In commit.c:read_commit_extra_header_lines, a commit without a value
will get a NULL value instead of the empty string. But we end up not
dereferencing the NULL, because it just gets fed to add_extra_header
later, which will only look at the value if its length parameter is
non-zero. So it is built to expect the current behavior, but would be
fine with a switch.
In http-push.c:xml_entities, we will return NULL if fed an empty
string. You can dereference NULL by doing git http-push '', although
on glibc systems it will not segfault, because we feed the NULL to
printf, which converts it to (null).
In http-backend.c:get_parameters, we call url_decode_parameter_* to
look at the contents of $QUERY_STRING. These functions can return
NULL via strbuf_detach if fed an empty string. We are ready to handle
a NULL value like empty=other=value. But not an empty name, like
one=1two=2 (note the bogus double- which yields an empty
parameter). You can get a segfault by sending that to a smart-http
server.
Probably more detail than you wanted, but I'm pretty confident now that
we should switch it, and that it won't cause any regressions. Patch is
below.
-- 8 --
Subject: strbuf: always return a non-NULL value from strbuf_detach
The current behavior is to return NULL when strbuf did not
actually allocate a string. This can be quite surprising to
callers, though, who may feed the strbuf from arbitrary data
and expect to always get a valid value.
In most cases, it does not make a difference because calling
any strbuf function will cause an allocation (even if the
function ends up not inserting any data). But if the code is
structured like:
struct strbuf buf = STRBUF_INIT;
if (some_condition)
strbuf_addstr(buf, some_string);
return strbuf_detach(buf, NULL);
then you may or may not return NULL, depending on the
condition. This can cause us to segfault in http-push
(when fed an empty URL) and in http-backend (when an empty
parameter like foo=bar is in the $QUERY_STRING).
This patch forces strbuf_detach to allocate an empty
NUL-terminated string when it is called on a strbuf that has
not been allocated.
I investigated all call-sites of strbuf_detach. The majority
are either not affected by the change (because they call a
strbuf_* function unconditionally), or can handle the empty
string just as easily as NULL.
Signed-off-by: Jeff King p...@peff.net
---
strbuf.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/strbuf.c b/strbuf.c
index 0510f76..4b9e30c 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -44,7 +44,9 @@ char *strbuf_detach(struct strbuf *sb, size_t *sz)
char *strbuf_detach(struct strbuf *sb, size_t *sz)
{
- char *res = sb-alloc ? sb-buf : NULL;
+ char *res;
+ strbuf_grow(sb, 0);
+ res = sb-buf;
if (sz)
*sz = sb-len;
strbuf_init(sb, 0);
--
1.8.0.rc3.3.gba630e1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] Bring format-patch --notes closer to a real feature
On Wed, Oct 17, 2012 at 10:45:22PM -0700, Junio C Hamano wrote: We never advertised the --notes option to format-patch (or anything related to the pretty format options for that matter) because the behaviour of these options was whatever they happened to do, not what they were designed to do. It had a few obvious glitches: * The notes section was appended immediately after the log message, and then the three-dash line was added. Such a supplimental material should come after the three-dash line. * The logic to append a new sign-off with format-patch --signoff worked on the message after the note was added, which made the detection of existing sign-off lines incorrect. This updates the handling of --notes option to correct these, in an attempt to bring it closer to a real feature. I just read through the whole series. Aside from the tangent about strbuf_detach, I didn't see anything wrong. Thanks for moving this forward. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
On Thu, Oct 18, 2012 at 03:54:29AM -0400, Jeff King wrote:
On Thu, Oct 18, 2012 at 09:49:04AM +0200, Johannes Sixt wrote:
- s/.*/GIT_'$uid'_NAME='\'''\''; export GIT_'$uid'_NAME/p
Didn't you lose the export GIT_$uid_{NAME,EMAIL,DATE} parts somewhere on
the way?
Yikes, you're right. I didn't even notice, as the test suite still
passes. I can see how the env filter would still be able to see the
variables, but the commit-tree call wouldn't. I guess it happens to work
because we do not test alternate idents in our filter branch tests (IOW,
we are silently rewriting each commit during the filter-branch, but it
happens to have the same identities).
Hrm. We _do_ test this in t7003. Weirder, if I instrument filter-branch
like this:
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 69406ae..1b504ce 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -298,8 +298,13 @@ while read commit parents; do
git cat-file commit $commit ../commit ||
die Cannot read commit $commit
+ echo 2 pre: $GIT_AUTHOR_NAME
+ sh -c 'echo 2 pre, subshell: $GIT_AUTHOR_NAME'
+
eval $(set_ident ../commit) ||
die setting author/committer failed for commit $commit
+ echo 2 post: $GIT_AUTHOR_NAME
+ sh -c 'echo 2 post, subshell: $GIT_AUTHOR_NAME'
eval $filter_env /dev/null ||
die env filter failed: $filter_env
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 4d13e10..ce57fc5 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -174,6 +174,8 @@ test_expect_success 'author information is preserved' '
test 1 = $(git rev-list --author=B V Uips preserved-author | wc -l)
'
+test_done
+
test_expect_success remove a certain author's commits '
echo i i
test_tick
and run t7003, it shows that the variable is properly exported to the
sub-process! But I can't seem to figure out why. Confused...
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
On Thu, Oct 18, 2012 at 06:22:17AM -0400, Jeff King wrote: Hrm. We _do_ test this in t7003. Weirder, if I instrument filter-branch like this: [...] and run t7003, it shows that the variable is properly exported to the sub-process! But I can't seem to figure out why. Confused... Oh, I see. The variables are already exported by test-lib.sh. You can see the breakage with: diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh index 4d13e10..1e7a209 100755 --- a/t/t7003-filter-branch.sh +++ b/t/t7003-filter-branch.sh @@ -167,10 +167,11 @@ test_expect_success 'author information is preserved' ' test_tick GIT_AUTHOR_NAME=B V Uips git commit -m bvuips git branch preserved-author - git filter-branch -f --msg-filter cat; \ + (sane_unset GIT_AUTHOR_NAME +git filter-branch -f --msg-filter cat; \ test \$GIT_COMMIT != $(git rev-parse master) || \ echo Hallo \ - preserved-author + preserved-author) test 1 = $(git rev-list --author=B V Uips preserved-author | wc -l) ' -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 2/2] filter-branch: use git-sh-setup's ident parsing functions
This saves us some code, but it also reduces the number of
processes we start for each filtered commit. Since we can
parse both author and committer in the same sed invocation,
we save one process. And since the new interface avoids tr,
we save 4 processes.
It also avoids using tr, which has had some odd
portability problems reported with from Solaris's xpg6
version.
We also tweak one of the tests in t7003 to double-check that
we are properly exporting the variables (because test-lib.sh
exports GIT_AUTHOR_NAME, it will be automatically exported
in subprograms. We override this to make sure that
filter-branch handles it properly itself).
Signed-off-by: Jeff King p...@peff.net
---
This fixes the missing exports from v1. There's no changes needed to
patch 1.
git-filter-branch.sh | 46 +-
t/t7003-filter-branch.sh | 5 +++--
2 files changed, 16 insertions(+), 35 deletions(-)
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 178e453..5314249 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -64,37 +64,19 @@ set_ident () {
eval $functions
-# When piped a commit, output a script to set the ident of either
-# author or committer
+finish_ident() {
+ # Ensure non-empty id name.
+ echo case \\$GIT_$1_NAME\ in \\)
GIT_$1_NAME=\\${GIT_$1_EMAIL%%@*}\ export GIT_$1_NAME;; esac
+ # And make sure everything is exported.
+ echo export GIT_$1_NAME
+ echo export GIT_$1_EMAIL
+ echo export GIT_$1_DATE
+}
set_ident () {
- lid=$(echo $1 | tr [A-Z] [a-z])
- uid=$(echo $1 | tr [a-z] [A-Z])
- pick_id_script='
- /^'$lid' /{
- s/'\''/'\''\\'\'\''/g
- h
- s/^'$lid' \([^]*\) [^]* .*$/\1/
- s/'\''/'\''\'\'\''/g
- s/.*/GIT_'$uid'_NAME='\'''\''; export GIT_'$uid'_NAME/p
-
- g
- s/^'$lid' [^]* \([^]*\) .*$/\1/
- s/'\''/'\''\'\'\''/g
- s/.*/GIT_'$uid'_EMAIL='\'''\''; export
GIT_'$uid'_EMAIL/p
-
- g
- s/^'$lid' [^]* [^]* \(.*\)$/@\1/
- s/'\''/'\''\'\'\''/g
- s/.*/GIT_'$uid'_DATE='\'''\''; export GIT_'$uid'_DATE/p
-
- q
- }
- '
-
- LANG=C LC_ALL=C sed -ne $pick_id_script
- # Ensure non-empty id name.
- echo case \\$GIT_${uid}_NAME\ in \\)
GIT_${uid}_NAME=\\${GIT_${uid}_EMAIL%%@*}\ export GIT_${uid}_NAME;; esac
+ parse_ident_from_commit author AUTHOR committer COMMITTER
+ finish_ident AUTHOR
+ finish_ident COMMITTER
}
USAGE=[--env-filter command] [--tree-filter command]
@@ -320,10 +302,8 @@ while read commit parents; do
git cat-file commit $commit ../commit ||
die Cannot read commit $commit
- eval $(set_ident AUTHOR ../commit) ||
- die setting author failed for commit $commit
- eval $(set_ident COMMITTER ../commit) ||
- die setting committer failed for commit $commit
+ eval $(set_ident ../commit) ||
+ die setting author/committer failed for commit $commit
eval $filter_env /dev/null ||
die env filter failed: $filter_env
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 4d13e10..1e7a209 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -167,10 +167,11 @@ test_expect_success 'author information is preserved' '
test_tick
GIT_AUTHOR_NAME=B V Uips git commit -m bvuips
git branch preserved-author
- git filter-branch -f --msg-filter cat; \
+ (sane_unset GIT_AUTHOR_NAME
+git filter-branch -f --msg-filter cat; \
test \$GIT_COMMIT != $(git rev-parse master) || \
echo Hallo \
- preserved-author
+ preserved-author)
test 1 = $(git rev-list --author=B V Uips preserved-author | wc -l)
'
--
1.8.0.rc3.3.gba630e1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] Bring format-patch --notes closer to a real feature
On Thu, Oct 18, 2012 at 12:45 PM, Junio C Hamano gits...@pobox.com wrote: This replaces the earlier wip with a real thing. We never advertised the --notes option to format-patch (or anything related to the pretty format options for that matter) because the behaviour of these options was whatever they happened to do, not what they were designed to do. Stupid question: does git am recreate notes from format-patch --notes output? If it does not, should it? I think it could be a nice way of copying notes from one machine to another, but not enabled by default (iow am --notes). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG when trying to delete symbolic refs
Am 16.10.2012 18:09, schrieb Junio C Hamano: Having said all that, I think your patch is going in the right direction. If somebody had a symbolic ref in refs/heads/, the removal should remove it, not the pointee, which may not even exist. Does branch -d sym work correctly with your patch when refs/heads/sym is pointing at something that does not exist? No, it doesn't, neither with nor without the patch. But we can make it work, and also address a UI issue. This series starts with two patches that only move code around, then follows the patch you commented on, a patch addressing dangling symrefs and finally a change to the way deleted symrefs are reported by git branch. branch: factor out check_branch_commit() branch: factor out delete_branch_config() branch: delete symref branch, not its target branch: skip commit checks when deleting symref branches branch: show targets of deleted symrefs, not sha1s builtin/branch.c | 75 --- t/t3200-branch.sh | 19 ++ 2 files changed, 68 insertions(+), 26 deletions(-) -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] branch: factor out check_branch_commit()
Move the code to perform checks on the tip commit of a branch
to its own function.
Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx
---
builtin/branch.c | 33 +
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index ffd2684..852019e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -154,10 +154,28 @@ static int branch_merged(int kind, const char *name,
return merged;
}
+static int check_branch_commit(const char *branchname, const char *refname,
+ unsigned char *sha1, struct commit *head_rev,
+ int kinds, int force)
+{
+ struct commit *rev = lookup_commit_reference(sha1);
+ if (!rev) {
+ error(_(Couldn't look up commit object for '%s'), refname);
+ return -1;
+ }
+ if (!force !branch_merged(kinds, branchname, rev, head_rev)) {
+ error(_(The branch '%s' is not fully merged.\n
+ If you are sure you want to delete it,
+ run 'git branch -D %s'.), branchname, branchname);
+ return -1;
+ }
+ return 0;
+}
+
static int delete_branches(int argc, const char **argv, int force, int kinds,
int quiet)
{
- struct commit *rev, *head_rev = NULL;
+ struct commit *head_rev = NULL;
unsigned char sha1[20];
char *name = NULL;
const char *fmt;
@@ -206,17 +224,8 @@ static int delete_branches(int argc, const char **argv,
int force, int kinds,
continue;
}
- rev = lookup_commit_reference(sha1);
- if (!rev) {
- error(_(Couldn't look up commit object for '%s'),
name);
- ret = 1;
- continue;
- }
-
- if (!force !branch_merged(kinds, bname.buf, rev, head_rev)) {
- error(_(The branch '%s' is not fully merged.\n
- If you are sure you want to delete it,
- run 'git branch -D %s'.), bname.buf, bname.buf);
+ if (check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
+ force)) {
ret = 1;
continue;
}
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] branch: factor out delete_branch_config()
Provide a small helper function for deleting branch config sections.
Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx
---
builtin/branch.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 852019e..97c7361 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -172,6 +172,15 @@ static int check_branch_commit(const char *branchname,
const char *refname,
return 0;
}
+static void delete_branch_config(const char *branchname)
+{
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(buf, branch.%s, branchname);
+ if (git_config_rename_section(buf.buf, NULL) 0)
+ warning(_(Update of config-file failed));
+ strbuf_release(buf);
+}
+
static int delete_branches(int argc, const char **argv, int force, int kinds,
int quiet)
{
@@ -237,17 +246,13 @@ static int delete_branches(int argc, const char **argv,
int force, int kinds,
bname.buf);
ret = 1;
} else {
- struct strbuf buf = STRBUF_INIT;
if (!quiet)
printf(remote_branch
? _(Deleted remote branch %s (was
%s).\n)
: _(Deleted branch %s (was %s).\n),
bname.buf,
find_unique_abbrev(sha1,
DEFAULT_ABBREV));
- strbuf_addf(buf, branch.%s, bname.buf);
- if (git_config_rename_section(buf.buf, NULL) 0)
- warning(_(Update of config-file failed));
- strbuf_release(buf);
+ delete_branch_config(bname.buf);
}
}
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] branch: delete symref branch, not its target
If a branch that is to be deleted happens to be a symref to another
branch, the current code removes the targeted branch instead of the
one it was called for.
Change this surprising behaviour and delete the symref branch
instead.
Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx
---
builtin/branch.c | 2 +-
t/t3200-branch.sh | 11 +++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 97c7361..5e1e5b4 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -239,7 +239,7 @@ static int delete_branches(int argc, const char **argv, int
force, int kinds,
continue;
}
- if (delete_ref(name, sha1, 0)) {
+ if (delete_ref(name, sha1, REF_NODEREF)) {
error(remote_branch
? _(Error deleting remote branch '%s')
: _(Error deleting branch '%s'),
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 79c8d01..ec5f70e 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -262,6 +262,17 @@ test_expect_success 'config information was renamed, too' \
test $(git config branch.s.dummy) = Hello
test_must_fail git config branch.s/s/dummy
+test_expect_success 'deleting a symref' '
+ git branch target
+ git symbolic-ref refs/heads/symref refs/heads/target
+ sha1=$(git rev-parse symref | cut -c 1-7)
+ echo Deleted branch symref (was $sha1). expect
+ git branch -d symref actual
+ test_path_is_file .git/refs/heads/target
+ test_path_is_missing .git/refs/heads/symref
+ test_i18ncmp expect actual
+'
+
test_expect_success 'renaming a symref is not allowed' \
'
git symbolic-ref refs/heads/master2 refs/heads/master
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] branch: skip commit checks when deleting symref branches
Before a branch is deleted, we check that it points to a valid
commit. With -d we also check that the commit is a merged; this
check is not done with -D.
The reason for that is that commits pointed to by branches should
never go missing; if they do then something broke and it's better
to stop instead of adding to the mess. And a non-merged commit
may contain changes that are worth preserving, so we require the
stronger option -D instead of -d to get rid of them.
If a branch consists of a symref, these concerns don't apply.
Deleting such a branch can't make a commit become unreferenced,
so we don't need to check if it is merged, or even if it is
actually a valid commit. Skip them in that case. This allows
us to delete dangling symref branches.
Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx
---
builtin/branch.c | 10 --
t/t3200-branch.sh | 9 +
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 5e1e5b4..d87035a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -214,6 +214,9 @@ static int delete_branches(int argc, const char **argv, int
force, int kinds,
die(_(Couldn't look up commit object for HEAD));
}
for (i = 0; i argc; i++, strbuf_release(bname)) {
+ const char *target;
+ int flags = 0;
+
strbuf_branchname(bname, argv[i]);
if (kinds == REF_LOCAL_BRANCH !strcmp(head, bname.buf)) {
error(_(Cannot delete the branch '%s'
@@ -225,7 +228,9 @@ static int delete_branches(int argc, const char **argv, int
force, int kinds,
free(name);
name = mkpathdup(fmt, bname.buf);
- if (read_ref(name, sha1)) {
+ target = resolve_ref_unsafe(name, sha1, 0, flags);
+ if (!target ||
+ (!(flags REF_ISSYMREF) is_null_sha1(sha1))) {
error(remote_branch
? _(remote branch '%s' not found.)
: _(branch '%s' not found.), bname.buf);
@@ -233,7 +238,8 @@ static int delete_branches(int argc, const char **argv, int
force, int kinds,
continue;
}
- if (check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
+ if (!(flags REF_ISSYMREF)
+ check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
force)) {
ret = 1;
continue;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index ec5f70e..1323f6f 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -273,6 +273,15 @@ test_expect_success 'deleting a symref' '
test_i18ncmp expect actual
'
+test_expect_success 'deleting a dangling symref' '
+ git symbolic-ref refs/heads/dangling-symref nowhere
+ test_path_is_file .git/refs/heads/dangling-symref
+ echo Deleted branch dangling-symref (was 000). expect
+ git branch -d dangling-symref actual
+ test_path_is_missing .git/refs/heads/dangling-symref
+ test_i18ncmp expect actual
+'
+
test_expect_success 'renaming a symref is not allowed' \
'
git symbolic-ref refs/heads/master2 refs/heads/master
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] branch: show targets of deleted symrefs, not sha1s
git branch reports the abbreviated hash of the head commit of
a deleted branch to make it easier for a user to undo the
operation. For symref branches this doesn't help. Print the
symref target instead for them.
Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx
---
builtin/branch.c | 19 +++
t/t3200-branch.sh | 5 ++---
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index d87035a..1ec9c02 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -251,15 +251,18 @@ static int delete_branches(int argc, const char **argv,
int force, int kinds,
: _(Error deleting branch '%s'),
bname.buf);
ret = 1;
- } else {
- if (!quiet)
- printf(remote_branch
- ? _(Deleted remote branch %s (was
%s).\n)
- : _(Deleted branch %s (was %s).\n),
- bname.buf,
- find_unique_abbrev(sha1,
DEFAULT_ABBREV));
- delete_branch_config(bname.buf);
+ continue;
+ }
+ if (!quiet) {
+ printf(remote_branch
+ ? _(Deleted remote branch %s (was %s).\n)
+ : _(Deleted branch %s (was %s).\n),
+ bname.buf,
+ (flags REF_ISSYMREF)
+ ? target
+ : find_unique_abbrev(sha1, DEFAULT_ABBREV));
}
+ delete_branch_config(bname.buf);
}
free(name);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 1323f6f..80e6be3 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -265,8 +265,7 @@ test_expect_success 'config information was renamed, too' \
test_expect_success 'deleting a symref' '
git branch target
git symbolic-ref refs/heads/symref refs/heads/target
- sha1=$(git rev-parse symref | cut -c 1-7)
- echo Deleted branch symref (was $sha1). expect
+ echo Deleted branch symref (was refs/heads/target). expect
git branch -d symref actual
test_path_is_file .git/refs/heads/target
test_path_is_missing .git/refs/heads/symref
@@ -276,7 +275,7 @@ test_expect_success 'deleting a symref' '
test_expect_success 'deleting a dangling symref' '
git symbolic-ref refs/heads/dangling-symref nowhere
test_path_is_file .git/refs/heads/dangling-symref
- echo Deleted branch dangling-symref (was 000). expect
+ echo Deleted branch dangling-symref (was nowhere). expect
git branch -d dangling-symref actual
test_path_is_missing .git/refs/heads/dangling-symref
test_i18ncmp expect actual
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] notes: mention --notes in more places
Jeff King venit, vidit, dixit 17.10.2012 21:05: On Wed, Oct 17, 2012 at 07:30:56AM -0600, Eric Blake wrote: We've talked about it several times, but it's never happened (probably because most people don't actually use notes). And people (like me) don't use notes because they aren't documented. Catch-22, so we have to start somewhere. Oh, I definitely agree your patch is the right direction. I was just explaining why it hasn't happened, even though people think it's a good idea. I'll submit a v2 with the non-controversial edits, and spend some time trying to figure out how to isolate the portion of pretty-options.txt that is relevant to format-patch. If it's easy enough, I can also consider using --- instead of Notes: as the separator when using format-patch. Hmm. After digging in the archive, it seems we (including both you and me!) have discussed this several times, and there are even some patches floating around. Maybe one of them would be a good starting point for your submission (I did not read carefully over all of the arguments for each): Patch from Thomas, Feb 2010: http://thread.gmane.org/gmane.comp.version-control.git/139919/focus=140818 Discussion between us, Dec 2010: http://thread.gmane.org/gmane.comp.version-control.git/163141 Patch from Michael, Apr 2011: http://thread.gmane.org/gmane.comp.version-control.git/172079 That one used to work for about one more year or so (it went through a few rebases) but stopped working during some rework involving the signature (signed-off-by), i.e. it puts the notes before the signed-off now. I didn't update it because nobody seemed interested anyway (and because branch-notes got implemented in a different, non-note way, so I dumped that part of my workflow also). Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] Bring format-patch --notes closer to a real feature
Nguyen Thai Ngoc Duy venit, vidit, dixit 18.10.2012 13:06: On Thu, Oct 18, 2012 at 12:45 PM, Junio C Hamano gits...@pobox.com wrote: This replaces the earlier wip with a real thing. We never advertised the --notes option to format-patch (or anything related to the pretty format options for that matter) because the behaviour of these options was whatever they happened to do, not what they were designed to do. Stupid question: does git am recreate notes from format-patch --notes output? If it does not, should it? I think it could be a nice way of copying notes from one machine to another, but not enabled by default (iow am --notes). Not yet, but you can already start basing your am patches on top of Junio's series ;) am should allow a notes ref (--notes=hereyougo) like other similar instances. Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add new git-remote-hd helper
Felipe Contreras venit, vidit, dixit 17.10.2012 14:58:
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
I've looked at many hg-git tools and none satisfy me. Too complicated, or
too
slow, or to difficult to setup, etc.
It's in an unsatisfying state, I agree. We have a great remote helper
infrastructure, but neither git-svn nor git-cvsimport/export use it, and
remote-hg is not in git.git.
git-svn used to be our killer feature! (It's becoming hard to maintain,
though.)
git-hg (in the shape of a remote helper) could be our next killer
feature, finally leading to our world domination ;)
The only one I've liked so far is hg-fast-export[1], which is indeed fast,
relatively simple, and relatively easy to use. But it's not properly
maintained
any more.
So, I decided to write my own from scratch, using hg-fast-export as
inspiration, and voila.
This one doesn't have any dependencies, just put it into your $PATH, and you
can clone and fetch hg repositories. More importantly to me; the code is
simple, and easy to maintain.
Well, it still has hg as a dependency. All our remote
integration/helpers suffer from that. At least, every hg install comes
with the modules, whereas svn is a beast (apr and such) that often comes
without the language bindings.
[1] http://repo.or.cz/w/fast-export.git
contrib/remote-hd/git-remote-hg | 231
1 file changed, 231 insertions(+)
create mode 100755 contrib/remote-hd/git-remote-hg
That diffstat looks great (sans tests, of course; it's contrib), but
there's no push functionality, and that is usually the most difficult
part all helpers have to implement: Not only the push interaction, but
also making sure that commits don't get duped in a roundtrip (git fetch
from vcs, push to vcs, git fetch from vcs).
Just cloning and fetching can be done most easily with a shared worktree
and scripting around hg up and git commit -A in some flavour.
diff --git a/contrib/remote-hd/git-remote-hg b/contrib/remote-hd/git-remote-hg
new file mode 100755
index 000..9157b30
--- /dev/null
+++ b/contrib/remote-hd/git-remote-hg
@@ -0,0 +1,231 @@
+#!/usr/bin/python2
+
+# Inspired by Rocco Rutte's hg-fast-export
+
+# Just copy to your ~/bin, or anywhere in your $PATH.
+# Then you can clone with:
+# hg::file:///path/to/mercurial/repo/
+
+from mercurial import hg, ui
+
+import re
+import sys
+import os
+import json
+
+def die(msg, *args):
+print sys.stderr, 'ERROR:', msg % args
+sys.exit(1)
While we don't have to code for py3, avoiding '' will help us later.
(It got removed in py3.) sys.sdterr.write() should be most portable.
+def gitmode(flags):
+return 'l' in flags and '12' or 'x' in flags and '100755' or '100644'
+
+def export_file(fc):
+if fc.path() == '.hgtags':
Is this always relative? Just wondering, dunno.
+return
+d = fc.data()
+print M %s inline %s % (gitmode(fc.flags()), fc.path())
+print data %d % len(d)
+print d
+
+def get_filechanges(repo, ctx, parents):
+l = [repo.status(p, ctx)[:3] for p in parents]
+changed, added, removed = [sum(e, []) for e in zip(*l)]
+return added + changed, removed
+
+author_re = re.compile('^((.+?) )?(.+?)$')
+
+def fixup_user(user):
+user = user.replace('', '')
+m = author_re.match(user)
+if m:
+name = m.group(1)
+mail = m.group(3)
+else:
+name = user
+mail = None
+
+if not name:
+name = 'Unknown'
+if not mail:
+mail = 'unknown'
+
+return '%s %s' % (name, mail)
+
+def get_repo(path, alias):
+myui = ui.ui()
+myui.setconfig('ui', 'interactive', 'off')
+repo = hg.repository(myui, path)
+return repo
Is there a reason to spell this out, e.g.: Why not
return hg.repository(myui, path)
+
+def hg_branch(b):
+if b == 'master':
+return 'default'
+return b
+
+def git_branch(b):
+if b == 'default':
+return 'master'
+return b
+
+def export_tag(repo, tag):
+global prefix
+print reset %s/tags/%s % (prefix, tag)
+print from :%s % (repo[tag].rev() + 1)
+print
+
+def export_branch(repo, branch):
+global prefix, marks, cache, branches
+
+heads = branches[hg_branch(branch)]
+
+# verify there's only one head
+if (len(heads) 1):
+die(Branch '%s' has more than one head % hg_branch(branch))
We have to deal with this at some point... Do you support hg
bookmarks? They'd be an option, or we implement better detached head
handling in git...
+
+head = repo[heads[0]]
+tip = marks.get(branch, 0)
+# mercurial takes too much time checking this
+if tip == head.rev():
+# nothing to do
+return
+revs = repo.revs('%u:%u' % (tip, head))
+count = 0
+
+revs = [rev for rev in revs if not cache.get(rev, False)]
+
+for rev in revs:
Those lines set up
[PATCH v3] status: refactor output format to represent default and add --long
From: Jeff King p...@peff.net
When deciding which output format to use, we default an internal enum
to STATUS_FORMAT_LONG and modify it if --porcelain or --short is
given. If this enum is set to LONG, then we know the user has not
specified any format, and we can kick in default behaviors. This works
because there is no --long which they could use to explicitly
specify LONG.
Let's expand the enum to have an explicit STATUS_FORMAT_NONE, in
preparation for adding --long, which can be used to override --short
or --porcelain. Then we can distinguish between LONG and NONE when
setting other defaults. There are two such cases:
1. The user has asked for NUL termination. With NONE, we
currently default to turning on the porcelain mode.
With an explicit --long, we would in theory use NUL
termination with the long mode, but it does not support
it. So we can just complain and die.
2. When an output format is given to git commit, we
default to --dry-run. This behavior would now kick in
when --long is given, too.
Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
On Thu, Oct 18, 2012 at 9:03 AM, Jeff King p...@peff.net wrote:
I think that is fine to split it like this, but you would want to update
the commit message above. Probably just remove those two cases and say
something like:
Note that you cannot actually trigger STATUS_FORMAT_LONG, as we do
not yet have --long; that will come in a follow-on patch.
And then move the reasoning for how --long works with each case into
the commit message of the next patch.
Nope, it's hard to split the explanation in two (at least to me),
which may mean that the split does not make sense.
Documentation/git-commit.txt | 4
Documentation/git-status.txt | 3 +++
builtin/commit.c | 29 +++--
3 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 9594ac8..3acf2e7 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -109,6 +109,10 @@ OPTIONS
format. See linkgit:git-status[1] for details. Implies
`--dry-run`.
+--long::
+ When doing a dry-run, give the output in a the long-format.
+ Implies `--dry-run`.
+
-z::
--null::
When showing `short` or `porcelain` status output, terminate
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 67e5f53..9f1ef9a 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -38,6 +38,9 @@ OPTIONS
across git versions and regardless of user configuration. See
below for details.
+--long::
+ Give the output in the long-format. This is the default.
+
-u[mode]::
--untracked-files[=mode]::
Show untracked files.
diff --git a/builtin/commit.c b/builtin/commit.c
index a17a5df..1dd2ec5 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -112,10 +112,11 @@ static const char *only_include_assumed;
static struct strbuf message = STRBUF_INIT;
static enum {
+ STATUS_FORMAT_NONE = 0,
STATUS_FORMAT_LONG,
STATUS_FORMAT_SHORT,
STATUS_FORMAT_PORCELAIN
-} status_format = STATUS_FORMAT_LONG;
+} status_format;
static int opt_parse_m(const struct option *opt, const char *arg, int unset)
{
@@ -454,6 +455,7 @@ static int run_status(FILE *fp, const char *index_file,
const char *prefix, int
case STATUS_FORMAT_PORCELAIN:
wt_porcelain_print(s);
break;
+ case STATUS_FORMAT_NONE:
case STATUS_FORMAT_LONG:
wt_status_print(s);
break;
@@ -1058,9 +1060,13 @@ static int parse_and_validate_options(int argc, const
char *argv[],
if (all argc 0)
die(_(Paths with -a does not make sense.));
- if (s-null_termination status_format == STATUS_FORMAT_LONG)
- status_format = STATUS_FORMAT_PORCELAIN;
- if (status_format != STATUS_FORMAT_LONG)
+ if (s-null_termination) {
+ if (status_format == STATUS_FORMAT_NONE)
+ status_format = STATUS_FORMAT_PORCELAIN;
+ else if (status_format == STATUS_FORMAT_LONG)
+ die(_(--long and -z are incompatible));
+ }
+ if (status_format != STATUS_FORMAT_NONE)
dry_run = 1;
return argc;
@@ -1159,6 +1165,9 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
OPT_SET_INT(0, porcelain, status_format,
N_(machine-readable output),
STATUS_FORMAT_PORCELAIN),
+ OPT_SET_INT(0, long, status_format,
+ N_(show status in long format (default)),
+ STATUS_FORMAT_LONG),
OPT_BOOLEAN('z', null, s.null_termination,
N_(terminate entries with
Re: [PATCH] Add new git-remote-hd helper
On Thu, Oct 18, 2012 at 3:18 PM, Michael J Gruber
g...@drmicha.warpmail.net wrote:
Felipe Contreras venit, vidit, dixit 17.10.2012 14:58:
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
I've looked at many hg-git tools and none satisfy me. Too complicated, or
too
slow, or to difficult to setup, etc.
It's in an unsatisfying state, I agree. We have a great remote helper
infrastructure, but neither git-svn nor git-cvsimport/export use it, and
remote-hg is not in git.git.
git-svn used to be our killer feature! (It's becoming hard to maintain,
though.)
git-hg (in the shape of a remote helper) could be our next killer
feature, finally leading to our world domination ;)
The only one I've liked so far is hg-fast-export[1], which is indeed fast,
relatively simple, and relatively easy to use. But it's not properly
maintained
any more.
So, I decided to write my own from scratch, using hg-fast-export as
inspiration, and voila.
This one doesn't have any dependencies, just put it into your $PATH, and you
can clone and fetch hg repositories. More importantly to me; the code is
simple, and easy to maintain.
Well, it still has hg as a dependency. All our remote
integration/helpers suffer from that. At least, every hg install comes
with the modules, whereas svn is a beast (apr and such) that often comes
without the language bindings.
Yeah, but there's no way around that, even if we write some code to
access the repository, it's quite likely that it will change. Unlike
git, mercurial developers expect people to access the repository
through mercurial API, not directly, and that's probably what we
should do if we want to avoid problems.
contrib/remote-hd/git-remote-hg | 231
1 file changed, 231 insertions(+)
create mode 100755 contrib/remote-hd/git-remote-hg
That diffstat looks great (sans tests, of course; it's contrib), but
there's no push functionality, and that is usually the most difficult
part all helpers have to implement: Not only the push interaction, but
also making sure that commits don't get duped in a roundtrip (git fetch
from vcs, push to vcs, git fetch from vcs).
Yeah, it will probably require much more code, but I think by far the
most important feature is fetching.
Just cloning and fetching can be done most easily with a shared worktree
and scripting around hg up and git commit -A in some flavour.
Yea, but that will be dead slow.
diff --git a/contrib/remote-hd/git-remote-hg
b/contrib/remote-hd/git-remote-hg
new file mode 100755
index 000..9157b30
--- /dev/null
+++ b/contrib/remote-hd/git-remote-hg
@@ -0,0 +1,231 @@
+#!/usr/bin/python2
+
+# Inspired by Rocco Rutte's hg-fast-export
+
+# Just copy to your ~/bin, or anywhere in your $PATH.
+# Then you can clone with:
+# hg::file:///path/to/mercurial/repo/
+
+from mercurial import hg, ui
+
+import re
+import sys
+import os
+import json
+
+def die(msg, *args):
+print sys.stderr, 'ERROR:', msg % args
+sys.exit(1)
While we don't have to code for py3, avoiding '' will help us later.
(It got removed in py3.) sys.sdterr.write() should be most portable.
All right.
+def gitmode(flags):
+return 'l' in flags and '12' or 'x' in flags and '100755' or
'100644'
+
+def export_file(fc):
+if fc.path() == '.hgtags':
Is this always relative? Just wondering, dunno.
AFAIK, why wouldn't it?
+def get_repo(path, alias):
+myui = ui.ui()
+myui.setconfig('ui', 'interactive', 'off')
+repo = hg.repository(myui, path)
+return repo
Is there a reason to spell this out, e.g.: Why not
return hg.repository(myui, path)
No reason, but I already have patches on top of this.
+def hg_branch(b):
+if b == 'master':
+return 'default'
+return b
+
+def git_branch(b):
+if b == 'default':
+return 'master'
+return b
+
+def export_tag(repo, tag):
+global prefix
+print reset %s/tags/%s % (prefix, tag)
+print from :%s % (repo[tag].rev() + 1)
+print
+
+def export_branch(repo, branch):
+global prefix, marks, cache, branches
+
+heads = branches[hg_branch(branch)]
+
+# verify there's only one head
+if (len(heads) 1):
+die(Branch '%s' has more than one head % hg_branch(branch))
We have to deal with this at some point... Do you support hg
bookmarks? They'd be an option, or we implement better detached head
handling in git...
I already updated this, I converted it to a warning and just picked a
random head.
Adding support for bookmarks should be easy, it's just a matter of
deciding how to differentiate branches from bookmarks. Perhaps
'refs/heads/bookmarks/foo'.
+revs = [rev for rev in revs if not cache.get(rev, False)]
+
+for rev in revs:
Those lines set up the list just so that you iterate over it later.
Please don't do this (unless you know that revs is very small).
for rev in revs:
if
[PATCH v2 0/7] Cure some format-patch wrapping and encoding issues
Hi all. [This is the second version of this series. If you still remember the first version, you might want to jump directly to the summary of changes below.] The main point of this series is to teach git to encode my name correctly, see patches 5+6, so that the decoded version is actually my name, so that send-email does not insist on adding a wrong superfluous From: line to the mail body. The other patches more mostly by-products that fix other issues I came across. Patch 1 fixes an old off-by-one error, so that wrapped text may now use all available columns. Patches 2 and 3 make the wrapping of header lines more correct, i. e., neither too early nor too late. Patch 4 does some refactoring, which is too unrelated to be included in one of the later patches. Patch 5 improves RFC 2047 encoding; patch 6 removes an old non-RFC conform workaround. Patch 7 is more an RFC, which seems to be a good idea from my point of view. Indeed, I thought the current implementation is erroneous, until Junio C Hamano pointed out, that this might be desired behavior. Thus, make up your mind about this one. The series is currently based on the maint branch, but it applies to master as well. It does also apply to next, but then my implementation of isprint() has to be dropped from patch 5. Changes in v2: - patch 1 is new and is a result of the v1 discussion - patch 5+6 split the old patch 4 into two patches - use of constants for maximum line lengths - even better adherence to RFC 2047 than v1 - updated commit messages/comments Regards Jan Jan H. Schönherr (7): utf8: fix off-by-one wrapping of text format-patch: do not wrap non-rfc2047 headers too early format-patch: do not wrap rfc2047 encoded headers too late format-patch: introduce helper function last_line_length() format-patch: make rfc2047 encoding more strict format-patch: fix rfc2047 address encoding with respect to rfc822 specials format-patch tests: check quoting/encoding in To: and Cc: headers git-compat-util.h | 2 + pretty.c| 149 +++ t/t4014-format-patch.sh | 231 ++-- t/t4202-log.sh | 4 +- utf8.c | 2 +- 5 Dateien geändert, 262 Zeilen hinzugefügt(+), 126 Zeilen entfernt(-) -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/7] utf8: fix off-by-one wrapping of text
From: Jan H. Schönherr schn...@cs.tu-berlin.de
The wrapping logic in strbuf_add_wrapped_text() does currently not allow
lines that entirely fill the allowed width, instead it wraps the line one
character too early.
For example, the text This is the sixth commit. formatted via
%w(11,1,2) (wrap at 11 characters, 1 char indent of first line, 2 char
indent of following lines) results in four lines: This is, the,
sixth, commit. This is wrong, because the sixth is exactly
11 characters long, and thus allowed.
Fix this by allowing the (width+1) character of a line to be a valid
wrapping point if it is a whitespace character.
Signed-off-by: Jan H. Schönherr schn...@cs.tu-berlin.de
---
v2: new patch, result of v1 discussion
---
t/t4202-log.sh | 4 ++--
utf8.c | 2 +-
2 Dateien geändert, 3 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index b3ac6be..584e3d8 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -72,9 +72,9 @@ cat expect EOF
commit.
EOF
-test_expect_success 'format %w(12,1,2)' '
+test_expect_success 'format %w(11,1,2)' '
- git log -2 --format=%w(12,1,2)This is the %s commit. actual
+ git log -2 --format=%w(11,1,2)This is the %s commit. actual
test_cmp expect actual
'
diff --git a/utf8.c b/utf8.c
index a544f15..28791a7 100644
--- a/utf8.c
+++ b/utf8.c
@@ -353,7 +353,7 @@ retry:
c = *text;
if (!c || isspace(c)) {
- if (w width || !space) {
+ if (w = width || !space) {
const char *start = bol;
if (!c text == start)
return w;
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/7] format-patch: do not wrap non-rfc2047 headers too early
From: Jan H. Schönherr schn...@cs.tu-berlin.de
Do not wrap the second and later lines of non-rfc2047-encoded headers
substantially before the 78 character limit.
Instead of passing the remaining length of the first line as wrapping
width, use the correct maximum length and tell strbuf_add_wrapped_bytes()
how many characters of the first line are already used.
Signed-off-by: Jan H. Schönherr schn...@cs.tu-berlin.de
---
v2:
- removed off-by-one correction now handled by first patch
- commit message clarifications
---
pretty.c| 2 +-
t/t4014-format-patch.sh | 60 -
2 Dateien geändert, 35 Zeilen hinzugefügt(+), 27 Zeilen entfernt(-)
diff --git a/pretty.c b/pretty.c
index 8b1ea9f..71e4024 100644
--- a/pretty.c
+++ b/pretty.c
@@ -286,7 +286,7 @@ static void add_rfc2047(struct strbuf *sb, const char
*line, int len,
if ((i + 1 len) (ch == '=' line[i+1] == '?'))
goto needquote;
}
- strbuf_add_wrapped_bytes(sb, line, len, 0, 1, max_length - line_len);
+ strbuf_add_wrapped_bytes(sb, line, len, -line_len, 1, max_length);
return;
needquote:
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 959aa26..d66e358 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -752,16 +752,14 @@ M64=$M8$M8$M8$M8$M8$M8$M8$M8
M512=$M64$M64$M64$M64$M64$M64$M64$M64
cat expect 'EOF'
Subject: [PATCH] foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo
- bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar
- foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo
- bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar
- foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo
- bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar
- foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo
- bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar
- foo bar foo bar foo bar foo bar
+ bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar
+ foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo
+ bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar
+ foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo
+ bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar
+ foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar
EOF
-test_expect_success 'format-patch wraps extremely long headers (ascii)' '
+test_expect_success 'format-patch wraps extremely long subject (ascii)' '
echo content file
git add file
git commit -m $M512
@@ -807,28 +805,12 @@ test_expect_success 'format-patch wraps extremely long
headers (rfc2047)' '
test_cmp expect subject
'
-M8=foo_bar_
-M64=$M8$M8$M8$M8$M8$M8$M8$M8
-cat expect EOF
-From: $M64
- foo...@foo.bar
-EOF
-test_expect_success 'format-patch wraps non-quotable headers' '
- rm -rf patches/
- echo content file
- git add file
- git commit -mfoo --author $M64 foo...@foo.bar
- git format-patch --stdout -1 patch
- sed -n /^From: /p; /^ /p; /^$/q patch from
- test_cmp expect from
-'
-
check_author() {
echo content file
git add file
GIT_AUTHOR_NAME=$1 git commit -m author-check
git format-patch --stdout -1 patch
- grep ^From: patch actual
+ sed -n /^From: /p; /^ /p; /^$/q patch actual
test_cmp expect actual
}
@@ -853,6 +835,32 @@ test_expect_success 'rfc2047-encoded headers also
double-quote 822 specials' '
check_author Föo B. Bar
'
+cat expect EOF
+From: foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_
+ aut...@example.com
+EOF
+test_expect_success 'format-patch wraps moderately long from-header (ascii)' '
+ check_author
foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_
+'
+
+cat expect 'EOF'
+From: Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar
+ Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo
+ Bar Foo Bar Foo Bar Foo Bar aut...@example.com
+EOF
+test_expect_success 'format-patch wraps extremely long from-header (ascii)' '
+ check_author Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar
Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar
Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar
+'
+
+cat expect 'EOF'
+From: Foo.Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar
+ Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo
+ Bar Foo Bar Foo Bar Foo Bar aut...@example.com
+EOF
+test_expect_success 'format-patch wraps extremely long from-header (rfc822)' '
+ check_author Foo.Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar
Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar
Foo Bar Foo Bar Foo Bar Foo Bar Foo Bar
+'
+
cat expect
[PATCH v2 3/7] format-patch: do not wrap rfc2047 encoded headers too late
From: Jan H. Schönherr schn...@cs.tu-berlin.de
Encoded characters add more than one character at once to an encoded
header. Include all characters that are about to be added in the length
calculation for wrapping.
Additionally, RFC 2047 imposes a maximum line length of 76 characters
if that line contains an rfc2047 encoded word.
Signed-off-by: Jan H. Schönherr schn...@cs.tu-berlin.de
---
v2:
- use constants for both, the 76 and 78 char limit
- rephrase comment
---
pretty.c| 26 +-
t/t4014-format-patch.sh | 58 +
2 Dateien geändert, 51 Zeilen hinzugefügt(+), 33 Zeilen entfernt(-)
diff --git a/pretty.c b/pretty.c
index 71e4024..da75879 100644
--- a/pretty.c
+++ b/pretty.c
@@ -263,6 +263,9 @@ static void add_rfc822_quoted(struct strbuf *out, const
char *s, int len)
static int is_rfc2047_special(char ch)
{
+ if (ch == ' ' || ch == '\n')
+ return 1;
+
return (non_ascii(ch) || (ch == '=') || (ch == '?') || (ch == '_'));
}
@@ -270,6 +273,7 @@ static void add_rfc2047(struct strbuf *sb, const char
*line, int len,
const char *encoding)
{
static const int max_length = 78; /* per rfc2822 */
+ static const int max_encoded_length = 76; /* per rfc2047 */
int i;
int line_len;
@@ -295,23 +299,25 @@ needquote:
line_len += strlen(encoding) + 5; /* 5 for =??q? */
for (i = 0; i len; i++) {
unsigned ch = line[i] 0xFF;
+ int is_special = is_rfc2047_special(ch);
+
+ /*
+* According to RFC 2047, we could encode the special character
+* ' ' (space) with '_' (underscore) for readability. But many
+* programs do not understand this and just leave the
+* underscore in place. Thus, we do nothing special here, which
+* causes ' ' to be encoded as '=20', avoiding this problem.
+*/
- if (line_len = max_length - 2) {
+ if (line_len + 2 + (is_special ? 3 : 1) max_encoded_length) {
strbuf_addf(sb, ?=\n =?%s?q?, encoding);
line_len = strlen(encoding) + 5 + 1; /* =??q? plus SP */
}
- /*
-* We encode ' ' using '=20' even though rfc2047
-* allows using '_' for readability. Unfortunately,
-* many programs do not understand this and just
-* leave the underscore in place.
-*/
- if (is_rfc2047_special(ch) || ch == ' ' || ch == '\n') {
+ if (is_special) {
strbuf_addf(sb, =%02X, ch);
line_len += 3;
- }
- else {
+ } else {
strbuf_addch(sb, ch);
line_len++;
}
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index d66e358..1d5636d 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -772,30 +772,31 @@ M8=föö bar
M64=$M8$M8$M8$M8$M8$M8$M8$M8
M512=$M64$M64$M64$M64$M64$M64$M64$M64
cat expect 'EOF'
-Subject: [PATCH]
=?UTF-8?q?f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
[PATCH v2 4/7] format-patch: introduce helper function last_line_length()
From: Jan H. Schönherr schn...@cs.tu-berlin.de
Currently, an open-coded loop to calculate the length of the last
line of a string buffer is used in multiple places.
Move that code into a function of its own.
Signed-off-by: Jan H. Schönherr schn...@cs.tu-berlin.de
---
pretty.c | 25 +
1 Datei geändert, 13 Zeilen hinzugefügt(+), 12 Zeilen entfernt(-)
diff --git a/pretty.c b/pretty.c
index da75879..482402d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -240,6 +240,17 @@ static int has_rfc822_specials(const char *s, int len)
return 0;
}
+static int last_line_length(struct strbuf *sb)
+{
+ int i;
+
+ /* How many bytes are already used on the last line? */
+ for (i = sb-len - 1; i = 0; i--)
+ if (sb-buf[i] == '\n')
+ break;
+ return sb-len - (i + 1);
+}
+
static void add_rfc822_quoted(struct strbuf *out, const char *s, int len)
{
int i;
@@ -275,13 +286,7 @@ static void add_rfc2047(struct strbuf *sb, const char
*line, int len,
static const int max_length = 78; /* per rfc2822 */
static const int max_encoded_length = 76; /* per rfc2047 */
int i;
- int line_len;
-
- /* How many bytes are already used on the current line? */
- for (i = sb-len - 1; i = 0; i--)
- if (sb-buf[i] == '\n')
- break;
- line_len = sb-len - (i+1);
+ int line_len = last_line_length(sb);
for (i = 0; i len; i++) {
int ch = line[i];
@@ -346,7 +351,6 @@ void pp_user_info(const struct pretty_print_context *pp,
if (pp-fmt == CMIT_FMT_EMAIL) {
char *name_tail = strchr(line, '');
int display_name_length;
- int final_line;
if (!name_tail)
return;
while (line name_tail isspace(name_tail[-1]))
@@ -361,10 +365,7 @@ void pp_user_info(const struct pretty_print_context *pp,
add_rfc2047(sb, quoted.buf, quoted.len, encoding);
strbuf_release(quoted);
}
- for (final_line = 0; final_line sb-len; final_line++)
- if (sb-buf[sb-len - final_line - 1] == '\n')
- break;
- if (namelen - display_name_length + final_line 78) {
+ if (namelen - display_name_length + last_line_length(sb) 78) {
strbuf_addch(sb, '\n');
if (!isspace(name_tail[0]))
strbuf_addch(sb, ' ');
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/7] format-patch: make rfc2047 encoding more strict
From: Jan H. Schönherr schn...@cs.tu-berlin.de
RFC 2047 requires more characters to be encoded than it is currently done.
Especially, RFC 2047 distinguishes between allowed remaining characters
in encoded words in addresses (From, To, etc.) and other headers, such
as Subject.
Make add_rfc2047() and is_rfc2047_special() location dependent and include
all non-allowed characters to hopefully be RFC 2047 conform.
This especially fixes a problem, where RFC 822 specials (e. g. .) were
left unencoded in addresses, which was solved with a non-standard-conform
workaround in the past (which is going to be removed in a follow-up patch).
Signed-off-by: Jan H. Schönherr schn...@cs.tu-berlin.de
---
v2:
- part of restructured patch 4 of v1
- disallow even more characters in is_rfc2047_special()
The implementation of isprint() should later probably be substituted by
the one from Nguyen:
http://article.gmane.org/gmane.comp.version-control.git/207666
---
git-compat-util.h | 2 ++
pretty.c| 67 +++--
t/t4014-format-patch.sh | 15 ---
3 Dateien geändert, 72 Zeilen hinzugefügt(+), 12 Zeilen entfernt(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 42d..d4ea446 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -475,6 +475,7 @@ extern const char tolower_trans_tbl[256];
#undef isdigit
#undef isalpha
#undef isalnum
+#undef isprint
#undef islower
#undef isupper
#undef tolower
@@ -492,6 +493,7 @@ extern unsigned char sane_ctype[256];
#define isdigit(x) sane_istest(x,GIT_DIGIT)
#define isalpha(x) sane_istest(x,GIT_ALPHA)
#define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT)
+#define isprint(x) ((x) = 0x20 (x) = 0x7e)
#define islower(x) sane_iscase(x, 1)
#define isupper(x) sane_iscase(x, 0)
#define is_glob_special(x) sane_istest(x,GIT_GLOB_SPECIAL)
diff --git a/pretty.c b/pretty.c
index 482402d..613e4ea 100644
--- a/pretty.c
+++ b/pretty.c
@@ -272,16 +272,65 @@ static void add_rfc822_quoted(struct strbuf *out, const
char *s, int len)
strbuf_addch(out, '');
}
-static int is_rfc2047_special(char ch)
+enum rfc2047_type {
+ RFC2047_SUBJECT,
+ RFC2047_ADDRESS,
+};
+
+static int is_rfc2047_special(char ch, enum rfc2047_type type)
{
- if (ch == ' ' || ch == '\n')
+ /*
+* rfc2047, section 4.2:
+*
+*8-bit values which correspond to printable ASCII characters other
+*than =, ?, and _ (underscore), MAY be represented as those
+*characters. (But see section 5 for restrictions.) In
+*particular, SPACE and TAB MUST NOT be represented as themselves
+*within encoded words.
+*/
+
+ /*
+* rule out non-ASCII characters and non-printable characters (the
+* non-ASCII check should be redundant as isprint() is not localized
+* and only knows about ASCII, but be defensive about that)
+*/
+ if (non_ascii(ch) || !isprint(ch))
+ return 1;
+
+ /*
+* rule out special printable characters (' ' should be the only
+* whitespace character considered printable, but be defensive and use
+* isspace())
+*/
+ if (isspace(ch) || ch == '=' || ch == '?' || ch == '_')
return 1;
- return (non_ascii(ch) || (ch == '=') || (ch == '?') || (ch == '_'));
+ /*
+* rfc2047, section 5.3:
+*
+*As a replacement for a 'word' entity within a 'phrase', for
example,
+*one that precedes an address in a From, To, or Cc header. The
ABNF
+*definition for 'phrase' from RFC 822 thus becomes:
+*
+*phrase = 1*( encoded-word / word )
+*
+*In this case the set of characters that may be used in a
Q-encoded
+*'encoded-word' is restricted to: upper and lower case ASCII
+*letters, decimal digits, !, *, +, -, /, =, and _
+*(underscore, ASCII 95.). An 'encoded-word' that appears within a
+*'phrase' MUST be separated from any adjacent 'word', 'text' or
+*'special' by 'linear-white-space'.
+*/
+
+ if (type != RFC2047_ADDRESS)
+ return 0;
+
+ /* '=' and '_' are special cases and have been checked above */
+ return !(isalnum(ch) || ch == '!' || ch == '*' || ch == '+' || ch ==
'-' || ch == '/');
}
static void add_rfc2047(struct strbuf *sb, const char *line, int len,
- const char *encoding)
+ const char *encoding, enum rfc2047_type type)
{
static const int max_length = 78; /* per rfc2822 */
static const int max_encoded_length = 76; /* per rfc2047 */
@@ -304,7 +353,7 @@ needquote:
line_len += strlen(encoding) + 5; /* 5 for =??q? */
for (i = 0; i len; i++) {
unsigned ch = line[i] 0xFF;
- int is_special = is_rfc2047_special(ch);
+
[PATCH v2 6/7] format-patch: fix rfc2047 address encoding with respect to rfc822 specials
From: Jan H. Schönherr schn...@cs.tu-berlin.de
According to RFC 2047 and RFC 822, rfc2047 encoded words and and rfc822
quoted strings do not mix. Since add_rfc2047() no longer leaves RFC 822
specials behind, the quoting is also no longer necessary to create a
standard-conform mail.
Remove the quoting, when RFC 2047 encoding takes place. This actually
requires to refactor add_rfc2047() a bit, so that the different cases
can be distinguished.
With this patch, my own name gets correctly decoded as Jan H. Schönherr
(without quotes) and not as Jan H. Schönherr (with quotes).
Signed-off-by: Jan H. Schönherr schn...@cs.tu-berlin.de
---
v2:
- part of restructured patch 4 of v1
- use constants for both, the 76 and 78 char limit
- select correct maximum length for possible final folding
- removed off-by-one correction now handled by first patch
---
pretty.c| 49 -
t/t4014-format-patch.sh | 2 +-
2 Dateien geändert, 33 Zeilen hinzugefügt(+), 18 Zeilen entfernt(-)
diff --git a/pretty.c b/pretty.c
index 613e4ea..413e758 100644
--- a/pretty.c
+++ b/pretty.c
@@ -231,7 +231,7 @@ static int is_rfc822_special(char ch)
}
}
-static int has_rfc822_specials(const char *s, int len)
+static int needs_rfc822_quoting(const char *s, int len)
{
int i;
for (i = 0; i len; i++)
@@ -329,25 +329,29 @@ static int is_rfc2047_special(char ch, enum rfc2047_type
type)
return !(isalnum(ch) || ch == '!' || ch == '*' || ch == '+' || ch ==
'-' || ch == '/');
}
-static void add_rfc2047(struct strbuf *sb, const char *line, int len,
- const char *encoding, enum rfc2047_type type)
+static int needs_rfc2047_encoding(const char *line, int len,
+ enum rfc2047_type type)
{
- static const int max_length = 78; /* per rfc2822 */
- static const int max_encoded_length = 76; /* per rfc2047 */
int i;
- int line_len = last_line_length(sb);
for (i = 0; i len; i++) {
int ch = line[i];
if (non_ascii(ch) || ch == '\n')
- goto needquote;
+ return 1;
if ((i + 1 len) (ch == '=' line[i+1] == '?'))
- goto needquote;
+ return 1;
}
- strbuf_add_wrapped_bytes(sb, line, len, -line_len, 1, max_length);
- return;
-needquote:
+ return 0;
+}
+
+static void add_rfc2047(struct strbuf *sb, const char *line, int len,
+ const char *encoding, enum rfc2047_type type)
+{
+ static const int max_encoded_length = 76; /* per rfc2047 */
+ int i;
+ int line_len = last_line_length(sb);
+
strbuf_grow(sb, len * 3 + strlen(encoding) + 100);
strbuf_addf(sb, =?%s?q?, encoding);
line_len += strlen(encoding) + 5; /* 5 for =??q? */
@@ -383,6 +387,7 @@ void pp_user_info(const struct pretty_print_context *pp,
const char *what, struct strbuf *sb,
const char *line, const char *encoding)
{
+ int max_length = 78; /* per rfc2822 */
char *date;
int namelen;
unsigned long time;
@@ -406,17 +411,21 @@ void pp_user_info(const struct pretty_print_context *pp,
name_tail--;
display_name_length = name_tail - line;
strbuf_addstr(sb, From: );
- if (!has_rfc822_specials(line, display_name_length)) {
+ if (needs_rfc2047_encoding(line, display_name_length,
RFC2047_ADDRESS)) {
add_rfc2047(sb, line, display_name_length,
encoding, RFC2047_ADDRESS);
- } else {
+ max_length = 76; /* per rfc2047 */
+ } else if (needs_rfc822_quoting(line, display_name_length)) {
struct strbuf quoted = STRBUF_INIT;
add_rfc822_quoted(quoted, line, display_name_length);
- add_rfc2047(sb, quoted.buf, quoted.len,
- encoding, RFC2047_ADDRESS);
+ strbuf_add_wrapped_bytes(sb, quoted.buf, quoted.len,
+ -6, 1, max_length);
strbuf_release(quoted);
+ } else {
+ strbuf_add_wrapped_bytes(sb, line, display_name_length,
+ -6, 1, max_length);
}
- if (namelen - display_name_length + last_line_length(sb) 78) {
+ if (namelen - display_name_length + last_line_length(sb)
max_length) {
strbuf_addch(sb, '\n');
if (!isspace(name_tail[0]))
strbuf_addch(sb, ' ');
@@ -1336,6 +1345,7 @@ void pp_title_line(const struct pretty_print_context *pp,
[PATCH v2 7/7] format-patch tests: check quoting/encoding in To: and Cc: headers
From: Jan H. Schönherr schn...@cs.tu-berlin.de git-format-patch does currently not parse user supplied extra header values (e. g., --cc, --add-header) and just replays them. That forces users to add them RFC 2822/2047 conform in encoded form, e. g. --cc '=?UTF-8?q?Jan=20H=2E=20Sch=C3=B6nherr?= ...' which is inconvenient. We would want to update git-format-patch to accept human-readable input --cc 'Jan H. Schönherr ...' and handle the encoding, wrapping and quoting internally in the future, similar to what is already done in git-send-email. The necessary code should mostly exist in the code paths that handle the From: and Subject: headers. Whether we want to do this only for the git-format-patch options --to and --cc (and the corresponding config options) or also for user supplied headers via --add-header, is open for discussion. For now, add test_expect_failure tests for To: and Cc: headers as a reminder and fix tests that would otherwise fail should this get implemented. Signed-off-by: Jan H. Schönherr schn...@cs.tu-berlin.de --- This patch is RFC material. There are a few reasons, why this is a good idea and also a few, why it is bad: Pro: - current git-format-patch behavior differs from git-send-email - we should be able to use the address format that git uses elsewhere (e. g., author and committer info) - necessary code mostly exists Con: - changes current behavior - make code more complex (Feel free to add more.) The first drawback can be mitigated by checking whether the input is already properly encoded, so that we do not accidentally double-encode things. git-send-email does that, but that's written in Perl, so we would need even more code. For now, this is only about _addresses_ supplied to git-format-patch, not _headers_. We could also validate/encode/wrap user supplied headers. RFC 2822/2047 is specific enough to allow that. But there is no point thinking about that without the intention of encoding addresses. v2: - updated commit message as suggested by Junio C Hamano --- t/t4014-format-patch.sh | 98 + 1 Datei geändert, 66 Zeilen hinzugefügt(+), 32 Zeilen entfernt(-) diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index e024eb8..ad9f69e 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -110,73 +110,107 @@ test_expect_success 'replay did not screw up the log message' ' test_expect_success 'extra headers' ' - git config format.headers To: R. E. Cipient rcipi...@example.com + git config format.headers To: R E Cipient rcipi...@example.com - git config --add format.headers Cc: S. E. Cipient scipi...@example.com + git config --add format.headers Cc: S E Cipient scipi...@example.com git format-patch --stdout master..side patch2 sed -e /^\$/q patch2 hdrs2 - grep ^To: R. E. Cipient rcipi...@example.com\$ hdrs2 - grep ^Cc: S. E. Cipient scipi...@example.com\$ hdrs2 + grep ^To: R E Cipient rcipi...@example.com\$ hdrs2 + grep ^Cc: S E Cipient scipi...@example.com\$ hdrs2 ' test_expect_success 'extra headers without newlines' ' - git config --replace-all format.headers To: R. E. Cipient rcipi...@example.com - git config --add format.headers Cc: S. E. Cipient scipi...@example.com + git config --replace-all format.headers To: R E Cipient rcipi...@example.com + git config --add format.headers Cc: S E Cipient scipi...@example.com git format-patch --stdout master..side patch3 sed -e /^\$/q patch3 hdrs3 - grep ^To: R. E. Cipient rcipi...@example.com\$ hdrs3 - grep ^Cc: S. E. Cipient scipi...@example.com\$ hdrs3 + grep ^To: R E Cipient rcipi...@example.com\$ hdrs3 + grep ^Cc: S E Cipient scipi...@example.com\$ hdrs3 ' test_expect_success 'extra headers with multiple To:s' ' - git config --replace-all format.headers To: R. E. Cipient rcipi...@example.com - git config --add format.headers To: S. E. Cipient scipi...@example.com + git config --replace-all format.headers To: R E Cipient rcipi...@example.com + git config --add format.headers To: S E Cipient scipi...@example.com git format-patch --stdout master..side patch4 sed -e /^\$/q patch4 hdrs4 - grep ^To: R. E. Cipient rcipi...@example.com,\$ hdrs4 - grep ^ *S. E. Cipient scipi...@example.com\$ hdrs4 + grep ^To: R E Cipient rcipi...@example.com,\$ hdrs4 + grep ^ *S E Cipient scipi...@example.com\$ hdrs4 ' -test_expect_success 'additional command line cc' ' +test_expect_success 'additional command line cc (ascii)' ' - git config --replace-all format.headers Cc: R. E. Cipient rcipi...@example.com + git config --replace-all format.headers Cc: R E Cipient rcipi...@example.com + git format-patch --cc=S E Cipient scipi...@example.com --stdout master..side | sed -e /^\$/q patch5 + grep
Re: [PATCH 0/6] Bring format-patch --notes closer to a real feature
Nguyen Thai Ngoc Duy pclo...@gmail.com writes: On Thu, Oct 18, 2012 at 12:45 PM, Junio C Hamano gits...@pobox.com wrote: This replaces the earlier wip with a real thing. We never advertised the --notes option to format-patch (or anything related to the pretty format options for that matter) because the behaviour of these options was whatever they happened to do, not what they were designed to do. Stupid question: does git am recreate notes from format-patch --notes output? If it does not, should it? I think it could be a nice way of copying notes from one machine to another, but not enabled by default (iow am --notes). Thinking about what the notes are, I do not think it should, at least by default; the model is broken at the conceptual level. The notes are comments on a commit you make after the fact, because you do not want to (or cannot afford to) amend the commit to include the comment. When you are sending it out over e-mailto be applied with am, as opposed to asking the other to pull, you are by definition willing to see the commit replayed with modification. I think it is sensible for format-patch/am pipeline when asked to use --notes to add the notes section after --- as additional material to help the recipient understand the context of the patch better, which is done with this series. If the submitter (or the recipient) wants to incorporate the description from the notes to update the proposed log message, it can easily be done by editing the output of format-patch --notes before sending it out (or before applying it with am). That does not mean that the recipient should not use notes for his own purpose on the resulting commit, by the way. It would be a convenient feature to prime the contents of such a new note the recipient creates on the resulting commit from the comment after --- before the diffstat or diff --git line. Note that (no pun intended) that additional comment does not have to originate from any notes in the submitter's repository. If saving the additional comments the submitter attached from the notes to the patch is useful, it would equally be useful to save typed-in comments on the patch that came from the submitter's fingers, not from the notes. It is something you can do by inspecting $dotest/patch file in your post-applypatch hook with today's git. If many people use and find such a feature desirable, we could add inbuilt support for it, but I do not think we are there yet. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Aw: Re: Aw: Re: [Patch v3 0/8] Create single PDF for all HTML files
That means that for the patch [6/8], which adds content-type to the
text files, to be complete, it needs to update Makefile to produce
html files from them.
IMHO also for the new files in ./technical html should be created because
now as we have asciidoc markup why not also use it.
BTW2: The 'pretty-print shell script in update-hook-example.txt'
part of my changes was left out from the merge to pu ...
Do you mean e2399e9 (Documentation/howto: convert plain text files
to asciidoc, 2012-10-16), or something else?
Yes; in e2399e9 the following hunks where left out from the patch to
update-hook-example.txt:
@@ -111,12 +114,12 @@ then
info Found matching head pattern: '$head_pattern'
for user_pattern in $user_patterns; do
- info Checking user: '$username' against pattern: '$user_pattern'
- matchlen=$(expr $username : $user_pattern)
- if test $matchlen = ${#username}
- then
- grant Allowing user: '$username' with pattern: '$user_pattern'
- fi
+info Checking user: '$username' against pattern: '$user_pattern'
+matchlen=$(expr $username : $user_pattern)
+if test $matchlen = ${#username}
+then
+ grant Allowing user: '$username' with pattern: '$user_pattern'
+fi
done
deny The user is not in the access list for this branch
done
@@ -149,13 +152,13 @@ then
info Found matching head pattern: '$head_pattern'
for group_pattern in $group_patterns; do
- for groupname in $groups; do
- info Checking group: '$groupname' against pattern: '$group_pattern'
- matchlen=$(expr $groupname : $group_pattern)
- if test $matchlen = ${#groupname}
- then
- grant Allowing group: '$groupname' with pattern: '$group_pattern'
- fi
+for groupname in $groups; do
+ info Checking group: '$groupname' against pattern: '$group_pattern'
+ matchlen=$(expr $groupname : $group_pattern)
+ if test $matchlen = ${#groupname}
+ then
+grant Allowing group: '$groupname' with pattern: '$group_pattern'
+ fi
done
done
deny None of the user's groups are in the access list for this branch
---
Thomas
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] status: refactor output format to represent default and add --long
On Thu, Oct 18, 2012 at 09:15:50PM +0700, Nguyen Thai Ngoc Duy wrote: On Thu, Oct 18, 2012 at 9:03 AM, Jeff King p...@peff.net wrote: I think that is fine to split it like this, but you would want to update the commit message above. Probably just remove those two cases and say something like: Note that you cannot actually trigger STATUS_FORMAT_LONG, as we do not yet have --long; that will come in a follow-on patch. And then move the reasoning for how --long works with each case into the commit message of the next patch. Nope, it's hard to split the explanation in two (at least to me), which may mean that the split does not make sense. Yeah, that was the same place that I had difficulty when writing the original. This version looks OK to me. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Aw: Re: Aw: Re: [Patch v3 0/8] Create single PDF for all HTML files
Thomas Ackermann th.acke...@arcor.de writes:
Yes; in e2399e9 the following hunks where left out from the patch to
update-hook-example.txt:
@@ -111,12 +114,12 @@ then
info Found matching head pattern: '$head_pattern'
for user_pattern in $user_patterns; do
- info Checking user: '$username' against pattern: '$user_pattern'
- matchlen=$(expr $username : $user_pattern)
- if test $matchlen = ${#username}
- then
- grant Allowing user: '$username' with pattern: '$user_pattern'
- fi
+info Checking user: '$username' against pattern: '$user_pattern'
+matchlen=$(expr $username : $user_pattern)
+if test $matchlen = ${#username}
+then
+ grant Allowing user: '$username' with pattern: '$user_pattern'
+fi
done
deny The user is not in the access list for this branch
done
@@ -149,13 +152,13 @@ then
info Found matching head pattern: '$head_pattern'
for group_pattern in $group_patterns; do
- for groupname in $groups; do
- info Checking group: '$groupname' against pattern: '$group_pattern'
- matchlen=$(expr $groupname : $group_pattern)
- if test $matchlen = ${#groupname}
- then
- grant Allowing group: '$groupname' with pattern: '$group_pattern'
- fi
+for groupname in $groups; do
+ info Checking group: '$groupname' against pattern:
'$group_pattern'
+ matchlen=$(expr $groupname : $group_pattern)
+ if test $matchlen = ${#groupname}
+ then
+grant Allowing group: '$groupname' with pattern:
'$group_pattern'
+ fi
done
done
deny None of the user's groups are in the access list for this branch
Yuck. That is because I almost always apply patches with whitespace
breakage fix. The above two hunks, if taken as patches to shell
script, does nothing but adding whitespace breakages, turning tab
indent into expanded runs of spaces, and that was why the tool
dropped them.
Resurrected; will queue the result later.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] status: refactor output format to represent default and add --long
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: From: Jeff King p...@peff.net When deciding which output format to use, we default an internal enum to STATUS_FORMAT_LONG and modify it if --porcelain or --short is given. If this enum is set to LONG, then we know the user has not specified any format, and we can kick in default behaviors. This works because there is no --long which they could use to explicitly specify LONG. Let's expand the enum to have an explicit STATUS_FORMAT_NONE, in preparation for adding --long, which can be used to override --short or --porcelain. Then we can distinguish between LONG and NONE when setting other defaults. There are two such cases: 1. The user has asked for NUL termination. With NONE, we currently default to turning on the porcelain mode. With an explicit --long, we would in theory use NUL termination with the long mode, but it does not support it. So we can just complain and die. 2. When an output format is given to git commit, we default to --dry-run. This behavior would now kick in when --long is given, too. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- On Thu, Oct 18, 2012 at 9:03 AM, Jeff King p...@peff.net wrote: I think that is fine to split it like this, but you would want to update the commit message above. Probably just remove those two cases and say something like: Note that you cannot actually trigger STATUS_FORMAT_LONG, as we do not yet have --long; that will come in a follow-on patch. And then move the reasoning for how --long works with each case into the commit message of the next patch. Nope, it's hard to split the explanation in two (at least to me), which may mean that the split does not make sense. I guess combining both is fine, but then the commit is no longer in preparation for adding the option, but it already adds --long, I would think. Will queue. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] status: refactor output format to represent default and add --long
On Thu, Oct 18, 2012 at 02:16:11PM -0700, Junio C Hamano wrote: I guess combining both is fine, but then the commit is no longer in preparation for adding the option, but it already adds --long, I would think. Maybe a better commit message is: -- 8 -- Subject: status: add --long output format option You can currently set the output format to --short or --porcelain. There is no --long, because we default to it already. However, you may want to override an alias that uses --short to get back to the default. This requires a little bit of refactoring, because currently we use STATUS_FORMAT_LONG internally to mean the same as the user did not specify anything. By expanding the enum to include STATUS_FORMAT_NONE, we can distinguish between the implicit and explicit cases. This effects these conditions: 1. The user has asked for NUL termination. With NONE, we currently default to turning on the porcelain mode. With an explicit --long, we would in theory use NUL termination with the long mode, but it does not support it. So we can just complain and die. 2. When an output format is given to git commit, we default to --dry-run. This behavior would now kick in when --long is given, too. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] branch: skip commit checks when deleting symref branches
René Scharfe rene.scha...@lsrfire.ath.cx writes: Before a branch is deleted, we check that it points to a valid commit. With -d we also check that the commit is a merged; this check is not done with -D. The reason for that is that commits pointed to by branches should never go missing; if they do then something broke and it's better to stop instead of adding to the mess. And a non-merged commit may contain changes that are worth preserving, so we require the stronger option -D instead of -d to get rid of them. If a branch consists of a symref, these concerns don't apply. Deleting such a branch can't make a commit become unreferenced, so we don't need to check if it is merged, or even if it is actually a valid commit. Skip them in that case. This allows us to delete dangling symref branches. Purist in me tells me that we should be using symbolic-ref -d to correct from such a misconfiguration, but ignoring that, the above logic makes perfect sense to me, especially together with the next change to tell what branch the symref was pointing at, which can be used by the user when the user removes such a symbolic ref by mistake. Thanks. Will queue all five. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] format-patch --notes: show notes after three-dashes
From: Junio C Hamano gits...@pobox.com
When inserting the note after the commit log message to format-patch
output, add three dashes before the note. Record the fact that we
did so in the rev_info and omit showing duplicated three dashes in
the usual codepath that is used when notes are not being shown.
Signed-off-by: Junio C Hamano gits...@pobox.com
Should this also include a documentation update to make this substantive
benefit visible, whether that be in the format-patch man pages, the
SubmittingPatches guide, in the git-notes description of 'A typical
use...', or even in the user-manual?
Do you have a preference?
---
log-tree.c | 15 +++
revision.h | 1 +
t/t4014-format-patch.sh | 7 +--
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 4390b11..712a22b 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -677,8 +677,13 @@ void show_log(struct rev_info *opt)
append_signoff(msgbuf, opt-add_signoff);
if ((ctx.fmt != CMIT_FMT_USERFORMAT)
- ctx.notes_message *ctx.notes_message)
+ ctx.notes_message *ctx.notes_message) {
+ if (ctx.fmt == CMIT_FMT_EMAIL) {
+ strbuf_addstr(msgbuf, ---\n);
+ opt-shown_dashes = 1;
+ }
strbuf_addstr(msgbuf, ctx.notes_message);
+ }
if (opt-show_log_size) {
printf(log size %i\n, (int)msgbuf.len);
@@ -710,6 +715,7 @@ void show_log(struct rev_info *opt)
int log_tree_diff_flush(struct rev_info *opt)
{
+ opt-shown_dashes = 0;
diffcore_std(opt-diffopt);
if (diff_queue_is_empty()) {
@@ -737,10 +743,11 @@ int log_tree_diff_flush(struct rev_info *opt)
opt-diffopt.output_prefix_data);
fwrite(msg-buf, msg-len, 1, stdout);
}
- if ((pch opt-diffopt.output_format) == pch) {
- printf(---);
+ if (!opt-shown_dashes) {
+ if ((pch opt-diffopt.output_format) == pch)
+ printf(---);
+ putchar('\n');
}
- putchar('\n');
}
}
diff_flush(opt-diffopt);
diff --git a/revision.h b/revision.h
index a95bd0b..059bfff 100644
--- a/revision.h
+++ b/revision.h
@@ -111,6 +111,7 @@ struct rev_info {
/* Format info */
unsigned int shown_one:1,
+ shown_dashes:1,
show_merge:1,
show_notes:1,
show_notes_given:1,
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index bea6381..9750ba6 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -623,9 +623,12 @@ test_expect_success 'format-patch --signoff' '
test_expect_success 'format-patch --notes --signoff' '
git notes --ref test add -m test message HEAD
git format-patch -1 --signoff --stdout --notes=test out
- # Notes message must come after S-o-b
+ # Three dashes must come after S-o-b
! sed /^Signed-off-by: /q out | grep test message
- sed 1,/^Signed-off-by: /d out | grep test message
+ sed 1,/^Signed-off-by: /d out | grep test message
+ # Notes message must come after three dashes
+ ! sed /^---$/q out | grep test message
+ sed 1,/^---$/d out | grep test message
'
echo fatal: --name-only does not make sense expect.name-only
--
1.8.0.rc3.112.gdb88a5e
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
-
No virus found in this message.
Checked by AVG - www.avg.com
Version: 2013.0.2741 / Virus Database: 2614/5837 - Release Date:
10/17/12
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Fix potential hang in https handshake.
From 700b8075c578941c8f951711825c390ac68b190f Mon Sep 17 00:00:00 2001
From: Stefan Zager sza...@google.com
Date: Thu, 18 Oct 2012 14:03:59 -0700
Subject: [PATCH] Fix potential hang in https handshake.
It will sometimes happen that curl_multi_fdset() doesn't
return any file descriptors. In that case, it's recommended
that the application sleep for a short time before running
curl_multi_perform() again.
http://curl.haxx.se/libcurl/c/curl_multi_fdset.html
Signed-off-by: Stefan Zager sza...@google.com
---
http.c | 40 ++--
1 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/http.c b/http.c
index df9bb71..a6f66c0 100644
--- a/http.c
+++ b/http.c
@@ -602,35 +602,47 @@ void run_active_slot(struct active_request_slot *slot)
int max_fd;
struct timeval select_timeout;
int finished = 0;
+ long curl_timeout;
slot-finished = finished;
while (!finished) {
step_active_slots();
if (slot-in_use) {
+ max_fd = -1;
+ FD_ZERO(readfds);
+ FD_ZERO(writefds);
+ FD_ZERO(excfds);
+ curl_multi_fdset(curlm, readfds, writefds, excfds,
max_fd);
+
#if LIBCURL_VERSION_NUM = 0x070f04
- long curl_timeout;
- curl_multi_timeout(curlm, curl_timeout);
- if (curl_timeout == 0) {
- continue;
- } else if (curl_timeout == -1) {
+ /* It will sometimes happen that curl_multi_fdset()
doesn't
+ return any file descriptors. In that case, it's
recommended
+ that the application sleep for a short time before
running
+ curl_multi_perform() again.
+
+ http://curl.haxx.se/libcurl/c/curl_multi_fdset.html
+ */
+ if (max_fd == -1) {
select_timeout.tv_sec = 0;
select_timeout.tv_usec = 5;
} else {
- select_timeout.tv_sec = curl_timeout / 1000;
- select_timeout.tv_usec = (curl_timeout % 1000)
* 1000;
+ curl_timeout = 0;
+ curl_multi_timeout(curlm, curl_timeout);
+ if (curl_timeout == 0) {
+ continue;
+ } else if (curl_timeout == -1) {
+ select_timeout.tv_sec = 0;
+ select_timeout.tv_usec = 5;
+ } else {
+ select_timeout.tv_sec = curl_timeout
/ 1000;
+ select_timeout.tv_usec = (curl_timeout
% 1000) * 1000;
+ }
}
#else
select_timeout.tv_sec = 0;
select_timeout.tv_usec = 5;
#endif
-
- max_fd = -1;
- FD_ZERO(readfds);
- FD_ZERO(writefds);
- FD_ZERO(excfds);
- curl_multi_fdset(curlm, readfds, writefds, excfds,
max_fd);
-
select(max_fd+1, readfds, writefds, excfds,
select_timeout);
}
}
--
1.7.7.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] format-patch --notes: show notes after three-dashes
Philip Oakley philipoak...@iee.org writes: From: Junio C Hamano gits...@pobox.com When inserting the note after the commit log message to format-patch output, add three dashes before the note. Record the fact that we did so in the rev_info and omit showing duplicated three dashes in the usual codepath that is used when notes are not being shown. Signed-off-by: Junio C Hamano gits...@pobox.com Should this also include a documentation update to make this substantive benefit visible, whether that be in the format-patch man pages, the SubmittingPatches guide, in the git-notes description of 'A typical use...', or even in the user-manual? Eric Blake (http://mid.gmane.org/507eb310.8020...@redhat.com) was already working on a documentation updates already, I thought. As long as what it does is explained in format-patch, that is fine. I do not think this deserves to be in the SubmittingPatches. We do tell people to hide here is the context of the change additional explanation after three dashes, but how the submitters prepare that text is entirely up to them (and I personally do not think notes is not necessarily the right tool to do so). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git for Windows and line endings
On Fri, Oct 19, 2012 at 12:13 AM, Chris B chris.blaszczyn...@gmail.com wrote: Hi.. it is such a crime to have that default option of MSysGit mess around with the line endings. No it's not. There is no thought to the fact that it's possible the Git users are not using Git the exact way the authors thought it would be used. Suggesting this is just insulting. A lot of thought was laid to ground for the decision. We have both Windows and Linux systems that have parts of their files stored in Git repositories. And in addition to that, some Linux and Windows Git clients. If everyone leaves the line endings alone, everything works out just fine! No, it will not. Notepad, which is the default text editor on Windows, barfs on LF line-endings, and many vi installations does the same thing on Unix-systems bards on CRLF line-endings. And so does a huge amount of custom, less tested tools. Thinking that this works for me, so it must work for everyone is exactly the reason why this whole situation is a big mess. You are only making it worse by not realizing the issues. But messing with the line endings broke some things in production. I'm not even sure what you're trying to say with this e-mail other than to blame us for your own problems. Stop making broken commits. Clean up after you when you did by accident. And for the love of God, stop blaming other people when you messed up. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation/fetch-options.txt: Clarify --prune with option --tags
From: Jari Aalto jari.aa...@cante.net Signed-off-by: Jari Aalto jari.aa...@cante.net --- Documentation/fetch-options.txt |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index b4d6476..90d50fb 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -38,7 +38,8 @@ ifndef::git-pull[] -p:: --prune:: After fetching, remove any remote-tracking branches which - no longer exist on the remote. + no longer exist on the remote. If used with option `--tags`, + remove tags in a similar manner. endif::git-pull[] ifdef::git-pull[] -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fix potential hang in https handshake.
sza...@google.com writes:
From 700b8075c578941c8f951711825c390ac68b190f Mon Sep 17 00:00:00 2001
From: Stefan Zager sza...@google.com
Date: Thu, 18 Oct 2012 14:03:59 -0700
Subject: [PATCH] Fix potential hang in https handshake.
It will sometimes happen that curl_multi_fdset() doesn't
return any file descriptors. In that case, it's recommended
that the application sleep for a short time before running
curl_multi_perform() again.
http://curl.haxx.se/libcurl/c/curl_multi_fdset.html
Signed-off-by: Stefan Zager sza...@google.com
---
Thanks. Would it be a better idea to patch up in problematic
case, instead of making this logic too deeply nested, like this
instead, I have to wonder...
... all the existing code above unchanged ...
curl_multi_fdset(..., max_fd);
+ if (max_fd 0) {
+ /* nothing actionable??? */
+ select_timeout.tv_sec = 0;
+ select_timeout.tv_usec = 5;
+ }
select(max_fd+1, ..., select_timeout);
http.c | 40 ++--
1 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/http.c b/http.c
index df9bb71..a6f66c0 100644
--- a/http.c
+++ b/http.c
@@ -602,35 +602,47 @@ void run_active_slot(struct active_request_slot *slot)
int max_fd;
struct timeval select_timeout;
int finished = 0;
+ long curl_timeout;
slot-finished = finished;
while (!finished) {
step_active_slots();
if (slot-in_use) {
+ max_fd = -1;
+ FD_ZERO(readfds);
+ FD_ZERO(writefds);
+ FD_ZERO(excfds);
+ curl_multi_fdset(curlm, readfds, writefds, excfds,
max_fd);
+
#if LIBCURL_VERSION_NUM = 0x070f04
- long curl_timeout;
- curl_multi_timeout(curlm, curl_timeout);
- if (curl_timeout == 0) {
- continue;
- } else if (curl_timeout == -1) {
+ /* It will sometimes happen that curl_multi_fdset()
doesn't
+return any file descriptors. In that case, it's
recommended
+that the application sleep for a short time before
running
+curl_multi_perform() again.
+
+http://curl.haxx.se/libcurl/c/curl_multi_fdset.html
+ */
+ if (max_fd == -1) {
select_timeout.tv_sec = 0;
select_timeout.tv_usec = 5;
} else {
- select_timeout.tv_sec = curl_timeout / 1000;
- select_timeout.tv_usec = (curl_timeout % 1000)
* 1000;
+ curl_timeout = 0;
+ curl_multi_timeout(curlm, curl_timeout);
+ if (curl_timeout == 0) {
+ continue;
+ } else if (curl_timeout == -1) {
+ select_timeout.tv_sec = 0;
+ select_timeout.tv_usec = 5;
+ } else {
+ select_timeout.tv_sec = curl_timeout
/ 1000;
+ select_timeout.tv_usec = (curl_timeout
% 1000) * 1000;
+ }
}
#else
select_timeout.tv_sec = 0;
select_timeout.tv_usec = 5;
#endif
-
- max_fd = -1;
- FD_ZERO(readfds);
- FD_ZERO(writefds);
- FD_ZERO(excfds);
- curl_multi_fdset(curlm, readfds, writefds, excfds,
max_fd);
-
select(max_fd+1, readfds, writefds, excfds,
select_timeout);
}
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
git subtree error (just how do you expect me to merge 0 trees?)
I noticed today that if you leave off the branch name from git subtree like so: $ git subtree add --prefix somewhere -m adding CDH as subtree path/to/repo warning: read-tree: emptying the index with no arguments is deprecated; use --empty fatal: just how do you expect me to merge 0 trees? The error message is not particularly helpful (and seems to actually be in read-subtree?) The solution in my case was to add the branch name on the end of the command. Ideally it would be better to emit an error-message from a script higher up the calling chain that would be more descriptive about the problem (such as suggesting no branch is specified).-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Unexpected directories from read-tree
I'm testing out the sparse checkout feature of Git on my large (14GB) repository and am running into a problem. When I add dir1/ to sparse-checkout and then run git read-tree -mu HEAD I see dir1 as expected. But when I add dir2/ to sparse-checkout and read-tree again I see dir2 and dir3 appear and they're not nested. If I replace dir2/ with dir3/ in the sparse-checkout file, then I see dir1 and dir3 but not dir2 as expected again. How can I debug this problem? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fix potential hang in https handshake (v2).
From aa77ab3dd5b98a5786ac158528f45355fc0ddbc3 Mon Sep 17 00:00:00 2001
From: Stefan Zager sza...@google.com
Date: Thu, 18 Oct 2012 16:23:53 -0700
Subject: [PATCH] Fix potential hang in https handshake.
It will sometimes happen that curl_multi_fdset() doesn't
return any file descriptors. In that case, it's recommended
that the application sleep for a short time before running
curl_multi_perform() again.
http://curl.haxx.se/libcurl/c/curl_multi_fdset.html
Signed-off-by: Stefan Zager sza...@google.com
---
http.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/http.c b/http.c
index df9bb71..e8aba7f 100644
--- a/http.c
+++ b/http.c
@@ -630,6 +630,10 @@ void run_active_slot(struct active_request_slot *slot)
FD_ZERO(writefds);
FD_ZERO(excfds);
curl_multi_fdset(curlm, readfds, writefds, excfds,
max_fd);
+ if (max_fd 0) {
+ select_timeout.tv_sec = 0;
+ select_timeout.tv_usec = 5;
+ }
select(max_fd+1, readfds, writefds, excfds,
select_timeout);
}
--
1.7.7.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
A design for distributed submodules
I think I finally agree that it's best to develop submodules further rather than introduce a new tool for the functionality I require. Here are some explicit proposals for submodules so we can at least establish agreement on what should be done. These are in order of decreasing importance (to me). * Upstreamless submodules If there is no 'url' key defined for a submodule in .gitconfig, there is no authoritative upstream for it. When a recursive fetch/pull/clone/push is performed on a remote superproject, its upstreamless submodules are also fetched/pulled/cloned/pushed directly from/to the submodule repositories under the superproject .git/modules. If this is the first time that remote's submodules are accessed, that remote is initialized for the local submodules: the submodule of the remote superproject becomes a remote of the local submodule, and is given the same name as the remote of the superproject. So, suppose we have a superproject with .gitmodules: [submodule sub] path = sub which is hosted at repositories at URL1 and URL2. Then we do: git clone --recursive URL1 super cd super git remote add other URL2 git fetch --recursive URL2 Now .git/modules/sub/config has: [remote origin] url = URL1/.git/modules/sub [remote other] url = URL2/.git/modules/sub So the effect is similar to just setting the submodule's url as .git/modules/sub, except that: - it hides the implementation detail of the exact location of the submodule repository from the publicly visible configuration file - it also works with bare remotes (where the actual remote submodule location would be URL/modules/sub) - it allows multiple simultaneous superproject remotes (where git-submodule currently always resolves relative urls against branch.$branch.remote with no option to fetch from a different remote). * Submodule discovery across all refs This is what Jens already mentioned. If we fetch multiple refs of a remote superproject, we also need to fetch _all_ of the submodules referenced by _any_ of the refs, not just the ones in the currently active branch. Finding the complete list of submodules probably has to be implemented by reading .gitmodules in all of the (updated) refs, which is a bit ugly, but not too bad. * Recording the active branch of a submodule When a submodule is added, its active branch should be stored in .gitmodules as submodule.$sub.branch. Then, when the submodule is checked out, and the head of that branch is the same as the commit in the gitlink (i.e. the superproject tree is current), then that branch is set as the active branch in the checked-out submodule working tree. Otherwise, a detached head is used. * Multiple working trees for a submodule A superproject may have multiple paths for the same submodule, presumably for different commits. This is for cases where the superproject is a snapshot of a developer's directory hierarchy, and the developer is simultaneously working on multiple branches of a submodule and it is convenient to have separate working trees for each of them. This is a bit hard to express with the current .gitconfig format, since paths are attributes of repository ids instead of vice versa. I'd introduce an alternative section format where you can say: [mount path1] module = sub branch = master [mount path2] module = sub branch = topic Implementing this is a bit intricate, since we need to use the git-new-workdir method to create multiple working directories that share the same refs, config, and object store, but have separate HEAD and index. I think this is a problem with the repository layout: the non-workdir-specific bits should all be in a single directory so that a single symlink would be enough. Obviously, I'm willing to implement the above functionalities since I need them. However, I think I'm going to work in Dulwich (which doesn't currently have any submodule support): a Python API is currently more important to me than a command-line tool, and the git.git codebase doesn't look like a very attractive place to contribute anyway. No offense, it's just not to my tastes. So the main reason I'd like to reach some tentative agreement about the details of the proposal is to ensure that _once_ someone finally implements this kind of functionality in git.git, it will use the same configuration format and same conventions, so that it will be compatible with my code. The compatibility between different tools is after all the main reason for doing this stuff as an extension to submodules instead of something completely different. Lauri -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fix potential hang in https handshake (v2).
sza...@google.com writes:
From aa77ab3dd5b98a5786ac158528f45355fc0ddbc3 Mon Sep 17 00:00:00 2001
From: Stefan Zager sza...@google.com
Date: Thu, 18 Oct 2012 16:23:53 -0700
Subject: [PATCH] Fix potential hang in https handshake.
Please don't do the above (as usual ;-)
It will sometimes happen that curl_multi_fdset() doesn't
return any file descriptors. In that case, it's recommended
that the application sleep for a short time before running
curl_multi_perform() again.
http://curl.haxx.se/libcurl/c/curl_multi_fdset.html
Signed-off-by: Stefan Zager sza...@google.com
---
The above sounds like the code is doing something against a
recommended practice, but is there a user-visible symptom due to
this?
We end up calling select() without any bit set in fds, so it would
become micro-sleep of select_timeout in such a case, but as far as I
can see, the existing code either
* does not select() and keeps polling step_active_slots() without
delay, when curl_timeout gives a 0 return value; or
* sets 50ms timeout or whatever negative value derived from
curl_timeout when the returned value is not 0 nor -1.
Is the symptom that select(), when given a negative timeout and no
fd to wake it, sleeps forever (or lng time, taking that negative
value as if it is a large unsigned long) or something?
Thanks.
http.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/http.c b/http.c
index df9bb71..e8aba7f 100644
--- a/http.c
+++ b/http.c
@@ -630,6 +630,10 @@ void run_active_slot(struct active_request_slot *slot)
FD_ZERO(writefds);
FD_ZERO(excfds);
curl_multi_fdset(curlm, readfds, writefds, excfds,
max_fd);
+ if (max_fd 0) {
+ select_timeout.tv_sec = 0;
+ select_timeout.tv_usec = 5;
+ }
select(max_fd+1, readfds, writefds, excfds,
select_timeout);
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fix potential hang in https handshake (v2).
Junio C Hamano gits...@pobox.com writes: We end up calling select() without any bit set in fds, so it would become micro-sleep of select_timeout in such a case, but as far as I can see, the existing code either * does not select() and keeps polling step_active_slots() without delay, when curl_timeout gives a 0 return value; or * sets 50ms timeout or whatever negative value derived from curl_timeout when the returned value is not 0 nor -1. Is the symptom that select(), when given a negative timeout and no fd to wake it, sleeps forever (or lng time, taking that negative value as if it is a large unsigned long) or something? Addendum. What I am trying to get at are (1) three line description I can put in the release notes for this fix when it is merged to the maintenance track, and (2) a feel of how often this happens and how bad the damage is. The latter helps us judge if this do the normal thing, but if in a rare case where we do not find any fds, patch it up to proceed is a better approach over your original. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
fatal: cannot convert from utf8 to UTF-8
This error: fatal: cannot convert from utf8 to UTF-8 occured in two distinct situations in our work group with git binaries older or equal to 1.7.7. Once during a commit, the other time during a rebase. Both occurences are 100% reproductible. But the commit that gives the error during a rebase doesn't do so in a cherry-pick. This is in part our fault: during the standardisation of our git environment, we (re)enforced UTF-8 encodings by setting i18n.commitenconding and i18n.logOutputEncoding to utf8. It is the i18n.logOutputEncoding = utf8 that *sometimes* triggers the error above. I know utf8 is not an accepted denomination (UTF-8 or utf-8 should be used, according to IANA standards), but we have attenuating circumstances in the fact that most things dealing with encoding accept the erroneous name. That includes at least iconv(1) and python(1). Thus we ignored that a distinction existed and, as self-respecting lazy typers, we preferred the (one touch) shorter version. I wonder if it should be expected that git accepts these name variants (utf8 and UTF8) as valid and equivalent to the standard ones. Of course it is very easy for us to work around the error, since setting i18n.logOutputEncoding = utf-8 or removing it altogether from the git config file chases the error away. It's only that these kinds of things are bound to happen and for a good proportion of git users it might be well opaque, difficult to fix and, in drastic (user ignorance-induced) cases, a showstopper. Thanks for listening. -- Cristian Tibirna(418) 656-2131 / 4340 Laval University - Québec, CAN ... http://www.giref.ulaval.ca/~ctibirna Research professional - GIREF ... ctibi...@giref.ulaval.ca -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unexpected directories from read-tree
On Fri, Oct 19, 2012 at 6:10 AM, Uri Moszkowicz u...@4refs.com wrote: I'm testing out the sparse checkout feature of Git on my large (14GB) repository and am running into a problem. When I add dir1/ to sparse-checkout and then run git read-tree -mu HEAD I see dir1 as expected. But when I add dir2/ to sparse-checkout and read-tree again I see dir2 and dir3 appear and they're not nested. If I replace dir2/ with dir3/ in the sparse-checkout file, then I see dir1 and dir3 but not dir2 as expected again. How can I debug this problem? Posting here is step 1. What version are you using? You can look at unpack-trees.c The function that does the check is excluded_from_list. You should check ls-files -t, see if CE_SKIP_WORKTREE is set correctly for all dir1/*, dir2/* and dir3/*. Can you recreate a minimal test case for the problem? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fatal: cannot convert from utf8 to UTF-8
Cristian Tibirna ctibi...@giref.ulaval.ca writes:
This error:
fatal: cannot convert from utf8 to UTF-8
...
This is in part our fault: during the standardisation of our git environment,
we (re)enforced UTF-8 encodings by setting i18n.commitenconding and
i18n.logOutputEncoding to utf8.
...
I know utf8 is not an accepted denomination (UTF-8 or utf-8 should be
used, according to IANA standards),...
Perhaps like this.
-- 8 --
Subject: [PATCH] reencode_string(): introduce and use same_encoding()
Callers of reencode_string() that re-encodes a string from one
encoding to another all used ad-hoc way to bypass the case where the
input and the output encodings are the same. Some did strcmp(),
some did strcasecmp(), yet some others when converting to UTF-8 used
is_encoding_utf8().
Introduce same_encoding() helper function to make these callers
use the same logic. Notably, is_encoding_utf8() has a work-around
for common misconfiguration to use utf8 to name UTF-8 encoding,
which does not match UTF-8 hence strcasecmp() would not consider
the same. Make use of it in this helper function.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/mailinfo.c | 2 +-
notes.c| 2 +-
pretty.c | 2 +-
sequencer.c| 2 +-
utf8.c | 7 +++
utf8.h | 1 +
6 files changed, 12 insertions(+), 4 deletions(-)
diff --git c/builtin/mailinfo.c w/builtin/mailinfo.c
index da23140..e4e39d6 100644
--- c/builtin/mailinfo.c
+++ w/builtin/mailinfo.c
@@ -483,7 +483,7 @@ static void convert_to_utf8(struct strbuf *line, const char
*charset)
if (!charset || !*charset)
return;
- if (!strcasecmp(metainfo_charset, charset))
+ if (same_encoding(metainfo_charset, charset))
return;
out = reencode_string(line-buf, metainfo_charset, charset);
if (!out)
diff --git c/notes.c w/notes.c
index bc454e1..ee8f01f 100644
--- c/notes.c
+++ w/notes.c
@@ -1231,7 +1231,7 @@ static void format_note(struct notes_tree *t, const
unsigned char *object_sha1,
}
if (output_encoding *output_encoding
- strcmp(utf8, output_encoding)) {
+ !is_encoding_utf8(output_encoding)) {
char *reencoded = reencode_string(msg, output_encoding, utf8);
if (reencoded) {
free(msg);
diff --git c/pretty.c w/pretty.c
index 8b1ea9f..e87fe9f 100644
--- c/pretty.c
+++ w/pretty.c
@@ -504,7 +504,7 @@ char *logmsg_reencode(const struct commit *commit,
return NULL;
encoding = get_header(commit, encoding);
use_encoding = encoding ? encoding : utf8;
- if (!strcmp(use_encoding, output_encoding))
+ if (same_encoding(use_encoding, output_encoding))
if (encoding) /* we'll strip encoding header later */
out = xstrdup(commit-buffer);
else
diff --git c/sequencer.c w/sequencer.c
index e3723d2..73c396b 100644
--- c/sequencer.c
+++ w/sequencer.c
@@ -60,7 +60,7 @@ static int get_message(struct commit *commit, struct
commit_message *out)
out-reencoded_message = NULL;
out-message = commit-buffer;
- if (strcmp(encoding, git_commit_encoding))
+ if (same_encoding(encoding, git_commit_encoding))
out-reencoded_message = reencode_string(commit-buffer,
git_commit_encoding, encoding);
if (out-reencoded_message)
diff --git c/utf8.c w/utf8.c
index a544f15..6a52834 100644
--- c/utf8.c
+++ w/utf8.c
@@ -423,6 +423,13 @@ int is_encoding_utf8(const char *name)
return 0;
}
+int same_encoding(const char *src, const char *dst)
+{
+ if (is_encoding_utf8(src) is_encoding_utf8(dst))
+ return 1;
+ return !strcasecmp(src, dst);
+}
+
/*
* Given a buffer and its encoding, return it re-encoded
* with iconv. If the conversion fails, returns NULL.
diff --git c/utf8.h w/utf8.h
index 3c0ae76..93ef600 100644
--- c/utf8.h
+++ w/utf8.h
@@ -7,6 +7,7 @@ int utf8_width(const char **start, size_t *remainder_p);
int utf8_strwidth(const char *string);
int is_utf8(const char *text);
int is_encoding_utf8(const char *name);
+int same_encoding(const char *, const char *);
int strbuf_add_wrapped_text(struct strbuf *buf,
const char *text, int indent, int indent2, int width);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
