slash in branch name
Hi, After upgrade GIT from 1.7.2.5-3.1 to 1.7.10.4-1+wheezy1 following error appear: git push central versions/4.3.2 Counting objects: 45, done. Delta compression using up to 8 threads. Compressing objects: 100% (28/28), done. Writing objects: 100% (28/28), 13.01 KiB, done. Total 28 (delta 22), reused 0 (delta 0) remote: Update script started: Tue Jun 9 13:25:34 BST 2015 remote: Arguments: refs/heads/versions/4.3.2 2e5233728aecc6ac337f4d3a9f32281e7c786e27 ae25cc33f6cf745cfa1061cbdfaf445344a60d13 remote: Warning: using temporary hooks remote: error: invalid key: hooks.denypush.branch.versions/4.3.2 remote: error: invalid key: hooks.allowmerge.versions/4.3.2 remote: Packages changed by this update: remote: think_3 remote: hls_plugin_1 remote: hal_av_1 remote: rtsp_plugin_1 remote: Notifying the following package owners of this update: hooks were downloaded from: git://git.et.redhat.com/ovirt-server.git My colleague did some research about that and it seems that this commit has stopped update hook working: commit b09c53a3e331211fc0154de8ebb271e48f8c7ee5 Author: Libor Pechacek lpecha...@suse.cz Date: Sun Jan 30 20:40:41 2011 +0100 Sanity-check config variable names Sanity-check config variable names when adding and retrieving them. As a side effect code duplication between git_config_set_multivar and get_value (in builtin/config.c) was removed and the common functionality was placed in git_config_parse_key. This breaks a test in t1300 which used invalid section-less keys in the tests for git -c. However, allowing such names there was useless, since there was no way to set them via config file, and no part of git actually tried to use section-less keys. This patch updates the test to use more realistic examples as well as adding its own test. Signed-off-by: Libor Pechacek lpecha...@suse.cz Acked-by: Jeff King p...@peff.net Signed-off-by: Junio C Hamano gits...@pobox.com Could you please advise how to fix/revert this? brgds, -- 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: co-authoring commits
Tuncer Ayaz tuncer.a...@gmail.com writes: Is this something that breaks the design and would never be implemented, 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: [PATCH 2/2] test-lib.sh: fix color support when tput needs ~/.terminfo
Richard Hansen rhan...@bbn.com writes:
We could test if the variable is set first (test -n ${foo+set}), at
the cost of a bit more complexity.
I do not mind it so much as you have
it, but it does mean adding a new field needs to update two spots.
I also don't like the duplicate list of color types, and I considered
doing something similar to what you suggested, but I decided against it.
I'm a bit worried about bizarre syntax errors or code execution if
say_color() is used improperly. ('eval' with uncontrolled variables
makes me nervous.)
I originally had the same reaction to your use of `eval` (with or
without being guarded by the case to limit to known 5 ones). But
the uncontrolled-ness of this use of eval is to the same degree of
uncontrolled-ness of any test_expect_{success,failure} scriptlet,
so...
I like this save to variables instead of using tput approach very
much either way. Well done.
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 v2] git-checkout.txt: Document git checkout pathspec better
Matthieu Moy matthieu@grenoble-inp.fr writes: Torsten Bögershausen tbo...@web.de writes: My v3 will probably use the original line: git-checkout - Checkout a branch or paths to the working tree I think mentionning Switch branch was a real improvement. For someone not familiar with the version control vocabulary, checkout does not mean much (just looked in a dictionary, it talks about payment and leaving a room in a hotel ...). And someone not understanding what checkout means in this context won't be helped much reading the description and getting checkout there. Or, borrow a book from a library, which I think is the closest analogy for this operation. But you are right. It is suboptimal to explain checkout in terms of checkout ;-). But as you say, it copies into the workspace, so copy a previous version into the workspace sounds good to me. I am afraid that previous would lead to Ah, you mean HEAD~1? confusion. In any case, you cannot copy what hasn't yet been created, previous is superfluous. I think restore also by definition has to go back to _some_ existing version, not a future yet-to-be-created one, so restore to some previous state is superfluous; in that sense, I find that restore working tree files may still be the one that makes most sense, at least to me, among the phrases floated in this thread so far. Basically, I'm fine with anything starting with Switch branches or, but please do change the headline ;-). Likewise; I agree switch branches or part is good. -- 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: slash in branch name
On 17/06/2015 20:24, Jeff King wrote: On Wed, Jun 17, 2015 at 08:16:10PM +0100, KK wrote: remote: error: invalid key: hooks.denypush.branch.versions/4.3.2 remote: error: invalid key: hooks.allowmerge.versions/4.3.2 [...] Those are syntactically bogus config keys. Keys should be of the form section.subsection.key and only subsection can contain arbitrary bytes (and of course the value can, too). The hooks running on the server are using git's config system in ways that were not intended. It should rearrange its organization of the data (I cannot comment much further without seeing the hooks themselves). My colleague did some research about that and it seems that this commit has stopped update hook working: commit b09c53a3e331211fc0154de8ebb271e48f8c7ee5 Author: Libor Pechacek lpecha...@suse.cz Date: Sun Jan 30 20:40:41 2011 +0100 Sanity-check config variable names [...] Could you please advise how to fix/revert this? I guess we could add a --no-really-i-am-abusing-git-config option to git-config to let these pass, at least for lookups. I am not sure that is a good idea, though. I think your hooks are fundamentally broken for branches with odd characters (right now you are seeing complaints on the lookup side, but I suspect that you could not write a hooks.denypush.branch.versions/4.3.2 entry if you wanted to, as git would choke on reading the config file). -Peff hooks were downloaded from: git://git.et.redhat.com/ovirt-server.git -- 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 06/11] ref-filter: implement '--merged' and '--no-merged' options
On Wed, Jun 17, 2015 at 2:08 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Karthik Nayak karthik@gmail.com writes:
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -901,12 +903,19 @@ static int ref_filter_handler(const char *refname,
const struct object_id *oid,
if (!match_points_at(filter-points_at, oid-hash, refname))
return 0;
+ if (filter-merge_commit) {
+ commit = lookup_commit_reference_gently(oid-hash, 1);
+ if (!commit)
+ return 0;
+ }
I'd appreciate a comment here. If I understand correctly, the comment
could be along the lines of
/*
* A merge filter implies that we're looking at refs pointing to
* commits = discard non-commits early. The actual filtering is done
* later.
*/
(perhaps something more concise)
Will do add a comment, Thanks!
--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
On 17/06/15 19:24, Matthieu Moy wrote: Torsten Bögershausen tbo...@web.de writes: My v3 will probably use the original line: git-checkout - Checkout a branch or paths to the working tree I think mentionning Switch branch was a real improvement. For someone not familiar with the version control vocabulary, checkout does not mean much (just looked in a dictionary, it talks about payment and leaving a room in a hotel ...). And someone not understanding what checkout means in this context won't be helped much reading the description and getting checkout there. (Ironically, Junio did almost the same remark when I proposed to document git describe as Describe ..., but the word describe does not have the ambiguity problem that checkout has) 'git checkout commit -- path' will copy the version from another commit into the workspace. If commit exists, it means that the state of this path existed somewhere in path in the past (well, modulo git add -p and other ways to cheat with history). So, to me, restore a previous version does apply in this case. Perhaps restore a recorded state into the worktree (my favorite up to now I think). But as you say, it copies into the workspace, so copy a previous version into the workspace sounds good to me. Basically, I'm fine with anything starting with Switch branches or, but please do change the headline ;-). Having read all this thread, I think it's really confusing that: 1) We have this command named checkout, as Matthieu points out. 2) This command allows different distinct operations (one for when it receives a path, other for when it receives a branch, other for when it receives a commit...). So what I would propose is fix the root of the problem: split these command in several ones, and mark the checkout command as deprecated (it would still allow the same functions as before, but it would not be documented, and would be announced as deprecated when used). So then we could have a git switch branchname for switching to a different branch. Also a git discard path to discard local changes. Etcetera. Comments? -- 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: Should the --encoding argument to log/show commands make any guarantees about their output?
On Wed, Jun 17, 2015 at 07:07:48PM +0200, Jan-Philip Gehrcke wrote: The two-option scenario is totally clear. Although one must stress that the error-out option can, as discussed, be kept minimally invasive: it is sufficient (and common) to just skip those byte sequences (and replace them with a replacement symbol) that would be invalid in the requested output encoding. This would retain as much information as possible while guaranteeing a subsequent decoder to retrieve valid input. I think munge into valid UTF-8, even if it means losing data is a totally valid and useful option. I'm not completely sure that git should do that, though. E.g., you could just as easily do: git log --encoding=utf8 | drop_invalid_utf8 | your_script Or quite possibly, your_script could do the munging itself while reading the data. I do not know much about Python's input handling, but in Perl, it is easy to say the input is utf8, and replace anything bogus with a substitution character[1]. Should we * just make this more clear in the docs and/or * should we adjust the behavior of --encoding or * should we do something entirely different, like adding a new command line option or * should we just leave things as they are? I would vote for a documentation change, perhaps like: Subject: docs: clarify that --encoding can produce invalid sequences In the common case that the commit encoding matches the output encoding, we do not touch the buffer at all, which makes things much more efficient. But it might be unclear to a consumer that we will pass through bogus sequences. Signed-off-by: Jeff King p...@peff.net --- Documentation/pretty-options.txt | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt index 74aa01a..642af6e 100644 --- a/Documentation/pretty-options.txt +++ b/Documentation/pretty-options.txt @@ -37,7 +37,10 @@ people using 80-column terminals. in their encoding header; this option can be used to tell the command to re-code the commit log message in the encoding preferred by the user. For non plumbing commands this - defaults to UTF-8. + defaults to UTF-8. Note that if an object claims to be encoded + in `X` and we are outputting in `X`, we will output the object + verbatim; this means that invalid sequences in the original + commit may be copied to the output. --notes[=ref]:: Show the notes (see linkgit:git-notes[1]) that annotate the -- 2.4.4.719.g3984bc6 -- 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
co-authoring commits
Even though I don't have time to work on a feature like this, like others before me, I've been in situations where I would have liked to set more than one GIT_AUTHOR_NAME (etc.) for a single commit due to the involvement of multiple developers in authoring a change. Is this something that breaks the design and would never be implemented, or can it be integrated such that one can specify co-authors when committing a change? I'm thinking: $ git commit --add-author Tony Zwei elsegu...@example.com -- 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] test-lib.sh: fix color support when tput needs ~/.terminfo
If tput needs ~/.terminfo for the current $TERM, then tput will
succeed before HOME is changed to $TRASH_DIRECTORY (causing color to
be set to 't') but fail afterward.
One possible way to fix this is to treat HOME like TERM: back up the
original value and temporarily restore it before say_color() runs
tput.
Instead, pre-compute and save the color control sequences before
changing either TERM or HOME. Use the saved control sequences in
say_color() rather than call tput each time. This avoids the need to
back up and restore the TERM and HOME variables, and it avoids the
overhead of a subshell and two invocations of tput per call to
say_color().
Signed-off-by: Richard Hansen rhan...@bbn.com
---
t/test-lib.sh | 53 -
1 file changed, 28 insertions(+), 25 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 57212ec..4a59bfb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -15,9 +15,6 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see http://www.gnu.org/licenses/ .
-# Keep the original TERM for say_color
-ORIGINAL_TERM=$TERM
-
# Test the binaries we have just built. The tests are kept in
# t/ subdirectory and are run in 'trash directory' subdirectory.
if test -z $TEST_DIRECTORY
@@ -68,12 +65,12 @@ done,*)
esac
# For repeatability, reset the environment to known value.
+# TERM is sanitized below, after saving color control sequences.
LANG=C
LC_ALL=C
PAGER=cat
TZ=UTC
-TERM=dumb
-export LANG LC_ALL PAGER TERM TZ
+export LANG LC_ALL PAGER TZ
EDITOR=:
# A call to unset with no arguments causes at least Solaris 10
# /usr/xpg4/bin/sh and /bin/ksh to bail out. So keep the unsets
@@ -181,9 +178,7 @@ export _x05 _x40 _z40 LF u200c
# This test checks if command xyzzy does the right thing...
# '
# . ./test-lib.sh
-test x$ORIGINAL_TERM != xdumb (
- TERM=$ORIGINAL_TERM
- export TERM
+test x$TERM != xdumb (
test -t 1
tput bold /dev/null 21
tput setaf 1 /dev/null 21
@@ -263,29 +258,34 @@ fi
if test -n $color
then
+ # Save the color control sequences now rather than run tput
+ # each time say_color() is called. This is done for two
+ # reasons:
+ # * TERM will be changed to dumb
+ # * HOME will be changed to a temporary directory and tput
+ # might need to read ~/.terminfo from the original HOME
+ # directory to get the control sequences
+ # Note: This approach assumes the control sequences don't end
+ # in a newline for any terminal of interest (command
+ # substitutions strip trailing newlines). Given that most
+ # (all?) terminals in common use are related to ECMA-48, this
+ # shouldn't be a problem.
+ say_color_error=$(tput bold; tput setaf 1) # bold red
+ say_color_skip=$(tput setaf 4) # blue
+ say_color_warn=$(tput setaf 3) # brown/yellow
+ say_color_pass=$(tput setaf 2) # green
+ say_color_info=$(tput setaf 6) # cyan
+ say_color_sgr0=$(tput sgr0)
say_color () {
- (
- TERM=$ORIGINAL_TERM
- export TERM
+ say_color_color=
case $1 in
- error)
- tput bold; tput setaf 1;; # bold red
- skip)
- tput setaf 4;; # blue
- warn)
- tput setaf 3;; # brown/yellow
- pass)
- tput setaf 2;; # green
- info)
- tput setaf 6;; # cyan
+ error|skip|warn|pass|info)
+ eval say_color_color=\$say_color_$1;;
*)
test -n $quiet return;;
esac
shift
- printf %s $*
- tput sgr0
- echo
- )
+ printf %s\\n $say_color_color$*$say_color_sgr0
}
else
say_color() {
@@ -295,6 +295,9 @@ else
}
fi
+TERM=dumb
+export TERM
+
error () {
say_color error error: $*
GIT_EXIT_OK=t
--
2.4.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
[PATCH 1/2] Revert test-lib.sh: do tests for color support after changing HOME
This reverts commit 102fc80d32094ad6598b17ab9d607516ee8edc4a.
There are two issues with that commit:
* It is buggy. In pseudocode, it is doing:
color is set || TERM != dumb color works color=t
when it should be doing:
color is set || { TERM != dumb color works color=t }
* It unnecessarily disables color when tput needs to read
~/.terminfo to get the control sequences.
---
t/test-lib.sh | 90 ---
1 file changed, 43 insertions(+), 47 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 39da9c2..57212ec 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -181,8 +181,16 @@ export _x05 _x40 _z40 LF u200c
# This test checks if command xyzzy does the right thing...
# '
# . ./test-lib.sh
+test x$ORIGINAL_TERM != xdumb (
+ TERM=$ORIGINAL_TERM
+ export TERM
+ test -t 1
+ tput bold /dev/null 21
+ tput setaf 1 /dev/null 21
+ tput sgr0 /dev/null 21
+ )
+ color=t
-unset color
while test $# -ne 0
do
case $1 in
@@ -253,6 +261,40 @@ then
verbose=t
fi
+if test -n $color
+then
+ say_color () {
+ (
+ TERM=$ORIGINAL_TERM
+ export TERM
+ case $1 in
+ error)
+ tput bold; tput setaf 1;; # bold red
+ skip)
+ tput setaf 4;; # blue
+ warn)
+ tput setaf 3;; # brown/yellow
+ pass)
+ tput setaf 2;; # green
+ info)
+ tput setaf 6;; # cyan
+ *)
+ test -n $quiet return;;
+ esac
+ shift
+ printf %s $*
+ tput sgr0
+ echo
+ )
+ }
+else
+ say_color() {
+ test -z $1 test -n $quiet return
+ shift
+ printf %s\n $*
+ }
+fi
+
error () {
say_color error error: $*
GIT_EXIT_OK=t
@@ -829,52 +871,6 @@ HOME=$TRASH_DIRECTORY
GNUPGHOME=$HOME/gnupg-home-not-used
export HOME GNUPGHOME
-# run the tput tests *after* changing HOME (in case ncurses needs
-# ~/.terminfo for $TERM)
-test -n ${color+set} || test x$ORIGINAL_TERM != xdumb (
- TERM=$ORIGINAL_TERM
- export TERM
- test -t 1
- tput bold /dev/null 21
- tput setaf 1 /dev/null 21
- tput sgr0 /dev/null 21
- )
- color=t
-
-if test -n $color
-then
- say_color () {
- (
- TERM=$ORIGINAL_TERM
- export TERM
- case $1 in
- error)
- tput bold; tput setaf 1;; # bold red
- skip)
- tput setaf 4;; # blue
- warn)
- tput setaf 3;; # brown/yellow
- pass)
- tput setaf 2;; # green
- info)
- tput setaf 6;; # cyan
- *)
- test -n $quiet return;;
- esac
- shift
- printf %s $*
- tput sgr0
- echo
- )
- }
-else
- say_color() {
- test -z $1 test -n $quiet return
- shift
- printf %s\n $*
- }
-fi
-
if test -z $TEST_NO_CREATE_REPO
then
test_create_repo $TRASH_DIRECTORY
--
2.4.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 2/2] test-lib.sh: fix color support when tput needs ~/.terminfo
On Wed, Jun 17, 2015 at 03:06:26PM -0400, Richard Hansen wrote: If tput needs ~/.terminfo for the current $TERM, then tput will succeed before HOME is changed to $TRASH_DIRECTORY (causing color to be set to 't') but fail afterward. One possible way to fix this is to treat HOME like TERM: back up the original value and temporarily restore it before say_color() runs tput. Instead, pre-compute and save the color control sequences before changing either TERM or HOME. Use the saved control sequences in say_color() rather than call tput each time. This avoids the need to back up and restore the TERM and HOME variables, and it avoids the overhead of a subshell and two invocations of tput per call to say_color(). Signed-off-by: Richard Hansen rhan...@bbn.com Nice, I like it. + # Save the color control sequences now rather than run tput + # each time say_color() is called. This is done for two + # reasons: + # * TERM will be changed to dumb + # * HOME will be changed to a temporary directory and tput + # might need to read ~/.terminfo from the original HOME + # directory to get the control sequences + # Note: This approach assumes the control sequences don't end + # in a newline for any terminal of interest (command + # substitutions strip trailing newlines). Given that most + # (all?) terminals in common use are related to ECMA-48, this + # shouldn't be a problem. Yeah, that was my first thought, but I agree it probably isn't going to be a big deal in practice. + say_color_error=$(tput bold; tput setaf 1) # bold red + say_color_skip=$(tput setaf 4) # blue + say_color_warn=$(tput setaf 3) # brown/yellow + say_color_pass=$(tput setaf 2) # green + say_color_info=$(tput setaf 6) # cyan + say_color_sgr0=$(tput sgr0) [...] + error|skip|warn|pass|info) + eval say_color_color=\$say_color_$1;; *) test -n $quiet return;; I think you could dispense with this case statement entirely and do: eval say_color_color=\$say_color_$1 if test -z $say_color_color; then test -n $quiet return fi I guess that is making the assumption that all colors have non-zero sizes, but that seems reasonable. I do not mind it so much as you have it, but it does mean adding a new field needs to update two spots. -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 v2] git-checkout.txt: Document git checkout pathspec better
Torsten Bögershausen tbo...@web.de writes: On 2015-06-17 21.23, Junio C Hamano wrote: [] Basically, I'm fine with anything starting with Switch branches or, but please do change the headline ;-). Likewise; I agree switch branches or part is good. How about this: git-checkout - Switch branches or restore changes to the working tree Gahh. We are NOT restoring CHANGES. We are restoring the whole contents to a path. It is perfectly fine to do this: git reset --hard git checkout HEAD^ hello.c There is no changes in hello.c after reset --hard. This is what makes it tempting for me to say check out (an existing contents to) a working tree file. Moreover, it does not matter if the target file is changed or not in the first place, so your added text: 'git checkout' with paths or `--patch` is used to restore modified or deleted paths to their original contents from the index or replace paths with the contents from a named tree-ish (most often a commit-ish). that says restoring modified or deleted is from the index, replacing is from a tree-ish is placing a stress on a wrong spot, I would think. Checkout individual files is to replace contents with existing versions, taken either from the index or from a named tree-ish. That is done in preparation to come up with the suitable contents for specified paths. This is a tangent, but on the other hand, checkout a whole branch is to prepare the working tree to be used to modify the specified branch. And that is why the word checkout makes sense for both operations. -- 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] strbuf: stop out-of-boundary warnings from Coverity
Stefan Beller sbel...@google.com writes: Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's happy, we'll have cleaner defect list It's down 31 defects, roughly 10% of all things coverity detected as problematic. YAY! I actually think this is too ugly to live. If coverity is buggy and unusable, why aren't we raising that issue to them? -- 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 05/11] ref-filter: add parse_opt_merge_filter()
On Wed, Jun 17, 2015 at 1:57 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Err, for-each-ref already uses it before this series, no? So, you don't need any extra option to get for-each-ref, because it is already there. Having these extra options is a good side effect, though. To make sure I'm clear enough, what you're doing is - add all options to for-each-ref - port tag.c - port branch.c What I'm suggesting is to prioritize this way - add all options required for tag.c - port tag.c - add all options required for branch.c - port branch.c I meant somewhat on those lines only. Let me clear that out. The steps I plan to take are: 1. Move code from for-each-ref to ref-filter. 2. Add options to ref-filter which is available in tag.c and branch.c (--points-at, --contains, --merged) 3. Add there options to for-each-ref. 4. Add options required for functioning of tag.c alone to ref-filter 5. Port tag.c 6. Add options required for functioning of branch.c alone to ref-filter 7. Port branch.c Now why i want to complete step 2 right after 1 is so that while porting tag.c and branch.c I do not want to focus on making those common options available. Because I rather work on getting their specific options (verbose in branch.c, -n in tag.c and so on) working before porting over tag.c/branch.c. Not only the challenge, but also the way to validate your work. Think of it as a rather comprehensive set of tests that you get for free once you ported a command. BTW, talking about tests, did you do some coverage analysis on git branch and git tag? If not, I'd suggest that you do this to make sure that the pieces of code you're rewritting using ref-filter are well tested before being rewritten (a bit like Paul's work on shell - C). You don't have to actually do this before porting, but it should come befor the port in the patch series to make sure that tests pass both the old and new implementation. Yes good point. I did not do a deep coverage analysis on git tag and its tests. You are right this would be a crucial step for porting. I had a glance over the tests. Will look into it. As for git branch I'll do that after porting tag.c over to using ref-filter. -- Regards, Karthik Nayak -- 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: slash in branch name
On Wed, Jun 17, 2015 at 08:16:10PM +0100, KK wrote: remote: error: invalid key: hooks.denypush.branch.versions/4.3.2 remote: error: invalid key: hooks.allowmerge.versions/4.3.2 [...] Those are syntactically bogus config keys. Keys should be of the form section.subsection.key and only subsection can contain arbitrary bytes (and of course the value can, too). The hooks running on the server are using git's config system in ways that were not intended. It should rearrange its organization of the data (I cannot comment much further without seeing the hooks themselves). My colleague did some research about that and it seems that this commit has stopped update hook working: commit b09c53a3e331211fc0154de8ebb271e48f8c7ee5 Author: Libor Pechacek lpecha...@suse.cz Date: Sun Jan 30 20:40:41 2011 +0100 Sanity-check config variable names [...] Could you please advise how to fix/revert this? I guess we could add a --no-really-i-am-abusing-git-config option to git-config to let these pass, at least for lookups. I am not sure that is a good idea, though. I think your hooks are fundamentally broken for branches with odd characters (right now you are seeing complaints on the lookup side, but I suspect that you could not write a hooks.denypush.branch.versions/4.3.2 entry if you wanted to, as git would choke on reading the config file). -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 0/2] redo fix for test-lib.sh color support
Commit 102fc80d fixed a bug where tput was failing because it needed to read ~/.terminfo after HOME was changed. However, that commit is buggy, and it unnecessarily disables color support when tput needs to read from ~/.terminfo. This series does two things: * revert the buggy fix * fix it properly, I hope :) Richard Hansen (2): Revert test-lib.sh: do tests for color support after changing HOME test-lib.sh: fix color support when tput needs ~/.terminfo t/test-lib.sh | 103 +- 1 file changed, 51 insertions(+), 52 deletions(-) -- 2.4.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 v2] git-checkout.txt: Document git checkout pathspec better
Andres G. Aragoneses kno...@gmail.com writes: Comments? No. -- 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] mergetools: add config option to disable auto-merge
Michael Rappazzo rappa...@gmail.com writes: For some mergetools, the current invocation of git mergetool will include an auto-merge flag. By default the flag is included, however if the git config option 'merge.automerge' is set to 'false', then that flag will now be omitted. ... and why is the automerge a bad thing that user would want to avoid triggering under which condition? That description may not have to be in the proposed log message, but it would help users when they decide if they want to use the configuration to describe it in the mergetool.automerge configuration. And depending on the answer to the above question, a configuration variable may turn out be a bad mechanism to customize this (namely, set-and-forget configuration variable is a bad match for a knob that is more per invocation than user taste). Is this not about automerge but more about always-show-UI because I like GUI? Then that may be a user taste thing that is a good match for a configuration variable. I simply cannot tell from what was in the message I am responding to. -TEMPORARY FILES -`git mergetool` creates `*.orig` backup files while resolving merges. -These are safe to remove once a file has been merged and its -`git mergetool` session has completed. - +CONFIGURATION OPTIONS +- +mergetool.keepBackup:: + `git mergetool` creates `*.orig` backup files while resolving merges. + These are safe to remove once a file has been merged and its + `git mergetool` session has completed. ++ This is an unrelated change; I think it is a good change, though. I however suspect that we would not want to repeat the configuration description in this file and instead mention these in see also section referring the readers to git-config(1). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
On 2015-06-17 21.23, Junio C Hamano wrote: [] Basically, I'm fine with anything starting with Switch branches or, but please do change the headline ;-). Likewise; I agree switch branches or part is good. How about this: git-checkout - Switch branches or restore changes to the working tree -- 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] strbuf: stop out-of-boundary warnings from Coverity
On Wed, Jun 17, 2015 at 12:12 PM, Jeff King p...@peff.net wrote: On Wed, Jun 17, 2015 at 10:58:10AM -0700, Stefan Beller wrote: Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's happy, we'll have cleaner defect list It's down 31 defects, roughly 10% of all things coverity detected as problematic. YAY! That's a good thing. I do find the solution a little gross, though. I wonder if there is a way we can tell coverity more about how strbuf works. I always thought the problem was a combination of both having a custom strcmp (like skip_prefix, starts_with) and a custom data structure (strbuf, string_list). So I am not sure if it is sufficient to tell coverity I've noticed similar problems with string_list, where it complains that we are touching list-items, which was assigned to NULL (of course it was, but then after that we did string_list_append!). I know literally nothing about coverity's annotations and what's possible with them. So I may be barking up a wrong tree completely. I have searched for the exact annotations for a while, but all I found were examples in other open source projects, no official documentation with all its features. I may be missing something though (there must be some official documentation, I'd assume). -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] strbuf: stop out-of-boundary warnings from Coverity
Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's happy, we'll have cleaner defect list It's down 31 defects, roughly 10% of all things coverity detected as problematic. YAY! -- 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] strbuf: stop out-of-boundary warnings from Coverity
On Wed, Jun 17, 2015 at 10:58:10AM -0700, Stefan Beller wrote: Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's happy, we'll have cleaner defect list It's down 31 defects, roughly 10% of all things coverity detected as problematic. YAY! That's a good thing. I do find the solution a little gross, though. I wonder if there is a way we can tell coverity more about how strbuf works. I've noticed similar problems with string_list, where it complains that we are touching list-items, which was assigned to NULL (of course it was, but then after that we did string_list_append!). I know literally nothing about coverity's annotations and what's possible with them. So I may be barking up a wrong tree completely. -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: 'git status -z' missing separators on OSX
On Wed, Jun 17, 2015 at 09:07:36AM -0500, Tad Hardesty wrote: Everything looks normal using the commands you described, and it does appear to only affect status: ~/test (master)$ type git git is hashed (/usr/local/bin/git) ~/test (master)$ git config --list core.repositoryformatversion=0 core.filemode=true core.bare=false core.logallrefupdates=true core.ignorecase=true core.precomposeunicode=true ~/test (master)$ GIT_TRACE=1 git status -z 08:59:11.806197 git.c:348 trace: built-in: git 'status' '-z' ~/test (master)$ git log --oneline -1 -z | hexdump -C 35 31 35 39 30 65 30 20 49 6e 69 74 69 61 6c 20 |51590e0 Initial | 0010 63 6f 6d 6d 69 74 2e 00 |commit..| 0018 ~/test (master)$ touch c d ~/test (master)$ git status -z | hexdump -C 3f 3f 20 63 3f 3f 20 64 |?? c?? d| 0008 This is again with 2.4.3 from git-scm.com. Hmph. I don't really have any more ideas, then. I think my next step would be to walk it through a debugger (the interesting function is wt_shortstatus_status, or wt_shortstatus_other). -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] strbuf: stop out-of-boundary warnings from Coverity
On Wed, Jun 17, 2015 at 12:25 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's happy, we'll have cleaner defect list It's down 31 defects, roughly 10% of all things coverity detected as problematic. YAY! I actually think this is too ugly to live. If coverity is buggy and unusable, why aren't we raising that issue to them? We can try to do that. The last time I wanted them to take a look at Git, they were unresponsive. I presume that's what you get when not being a paying customer. :( -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] test-lib.sh: fix color support when tput needs ~/.terminfo
On Wed, Jun 17, 2015 at 05:11:21PM -0400, Richard Hansen wrote: + test -z $1 test -n $quiet return + eval say_color_color=\$say_color_$1 Thanks, this looks much simpler. In the non-quiet case, you will eval $say_color_, even though we know it to be bogus. I guess we need to make sure say_color_color is blank, though. The alternative would be: if test -z $1; then test -n $quiet return say_color_color= else eval say_color_color=\$say_color_$1 fi I dunno if that makes the intent more clear or not. I am OK with it either way. -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: co-authoring commits
On Wed, Jun 17, 2015 at 06:52:24PM -0400, Theodore Ts'o wrote: On Wed, Jun 17, 2015 at 10:26:32PM +0200, Tuncer Ayaz wrote: By allowing multiple authors, you don't have to decide who's the primary author, as in such situations usually there is no primary at all. I sometimes deliberately override the author when committing and add myself just as another co-author in the commit message, but as others have noted it would be really great if we can just specify multiple authors. Just recently, there a major thread on the IETF mailing list where IETF working group had drafts where people were listed as co-authors without their permission, and were upset that the fact that their name was added made it seem as if they agreed with the end product. (i.e., that they were endorsing the I-D). So while adding formal coauthor might solves (a few) problems, it can also introduce others. Ultimately there is one person who can decide which parts of the changes to put in the commit that gets sent to the maintainer. So there *is* someone who is the primary author; the person who takes the final pass on the patch and then hits the send key. I've worked on many patches with another person in a shared screen session, co-authoring a series of patches and commit messages in vim, and writing an email in mutt. There were, ultimately, two people deciding what to put in a commit and send to the maintainer. This is, admittedly, unusual, but pair programming is not ridiculously uncommon. In that case, perhaps you could set the from field to a mailing list address. The From field in email headers supports a list of comma-separated addresses, just like To and Cc. Speaking from experience, this more-or-less works with all the mail software we tried it with, with the occasional program only displaying the first or last entry. - Josh Triplett -- 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: co-authoring commits
j...@joshtriplett.org writes: Author and committer are used by many git tools; if they weren't part of the object header, they'd need to be part of some pseudo-header with a standardized format that git can parse. Yes, the same goes to the address on Signed-off-by: footers. There recently was a series to enhance the footer list handling (Christian Cc'ed) for the generation and maintenance side, and I do think it is reasonable to further add enhanced support for footers. That does not argue for having a new coauthor as a new commit object header at all, though. -- 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: co-authoring commits
On Wed, Jun 17, 2015 at 10:26:32PM +0200, Tuncer Ayaz wrote: By allowing multiple authors, you don't have to decide who's the primary author, as in such situations usually there is no primary at all. I sometimes deliberately override the author when committing and add myself just as another co-author in the commit message, but as others have noted it would be really great if we can just specify multiple authors. Just recently, there a major thread on the IETF mailing list where IETF working group had drafts where people were listed as co-authors without their permission, and were upset that the fact that their name was added made it seem as if they agreed with the end product. (i.e., that they were endorsing the I-D). So while adding formal coauthor might solves (a few) problems, it can also introduce others. Ultimately there is one person who can decide which parts of the changes to put in the commit that gets sent to the maintainer. So there *is* someone who is the primary author; the person who takes the final pass on the patch and then hits the send key. One could imagine some frankly, quite rare example where there is a team of people who votes on each commit before it gets sent out and where everyone is equal and there is no hierarchy. In that case, perhaps you could set the from field to a mailing list address. But honestly, how often is that *all* of the authors are completely equal[1]? In my personal practice, if I make significant changes to a patch, I will indeed simply change the submitter, and then give credit the original author. This is the case where I'm essentially saying, Bob did a lot of work, but I made a bunch of changes, so if things break horribly, blame *me*, not Bob. Alternatively, if I just need to make a few cosmetic changes to Alice's patch (i.e., fix white spaces, correct spelling, change the commit description so it's validly parsable and understandable English, etc.), I'll just add a comment in square brackets indicating what changes I made before I committed the change. This seems to work just fine, and I don't think we should try to fix something that isn't broken. - Ted [1] Gilbert and Sullivan attacked this notion is a commedic way in The Gondoliers; especially in the songs Replying we sing as one individual and There Lived a King: https://www.youtube.com/watch?v=YD0dgXTQ3K0 https://www.youtube.com/watch?v=oSaVdqcDgZc -- 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 difftool --dir-diff error in the presence of symlinks to directories
Reproduce like this (using git 2.4.3): git init mkdir foo touch foo/bar git add . git commit -m Initial commit. ln -s foo link git add . git commit -m Add link to foo. git difftool -d HEAD^ HEAD That last command outputs: fatal: Unable to hash /Users/isbadawi/test/link hash-object /Users/isbadawi/test/link: command returned error: 128 Briefly looking at the 'git difftool' source it looks like it uses the output of 'git diff --raw' and calls 'hash-object' on any object whose mode is nonzero, including symlinks. I'm not sure what the right thing to do here is -- just thought I'd report this failure. Thanks, Ismail -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 07/10] send-email: reduce dependancies impact on parse_address_line
Matthieu Moy matthieu@grenoble-inp.fr writes
+ my $commentrgx=qr/\((?:[^)]*)\)/;
+ my $quotergx=qr/(?:[^\\\]|\\.)*/;
+ my $wordrgx=qr/(?:[^][\s():;@\\,.]|\\.)+/;
Spaces around = please.
...
+ foreach my $token (@tokens) {
+ if ($token =~ /^[,;]$/) {
Here and below: you're indenting with a 4-column offset, it should be 8.
Should have spent more time on the form... Thanks
The code below is a bit hard to read (I'm neither fluent in Perl nor in
the RFC ...). A few more comments would help. A few examples below (it's
up to you to integrate them or not).
Ok, I'll add comments for the hardest parts.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 07/10] send-email: reduce dependancies impact on parse_address_line
Junio C Hamano gits...@pobox.com writes
Suffix rgx that means regular expression is a bit unusual, and
also hard to read when squashed to another word. Elsewhere in the
same script, we seem to use $re_whatever to store precompiled
regular expressions, so perhaps $re_comment, $re_quote, etc.?
Yes it's indeed a better name. I had not seen it, thanks!
+if ($str_address ne $str_phrase ne ) {
+$str_address = qq[$str_address];
+}
We see both git@vger.kernel.org and git@vger.kernel.org around
here for an address without comment or phrase; this chooses to turn
them both into git@vger.kernel.org form? Not a complaint but am
thinking aloud to see if I am reading it correctly.
If there's no phrase, this will choose the git@vger.kernel.org form,
in both cases, because it'll be recognize as an address, $str_address
will be git@vger.kernel.org and $str_phrase will be empty before the
if ($str_address ne ...)
Here are some tests:
Input: j...@example.com
Split: j...@example.com
M::A : j...@example.com
--
Input: j...@example.com
Split: j...@example.com
M::A : j...@example.com
--
Input: Jane j...@example.com
Split: Jane j...@example.com
M::A : Jane j...@example.com
--
Input: Jane Doe j...@example.com
Split: Jane Doe j...@example.com
M::A : Jane Doe j...@example.com
--
Input: Jane j...@example.com
Split: Jane j...@example.com
M::A : Jane j...@example.com
--
Input: Doe, Jane j...@example.com
Split: Doe, Jane j...@example.com
M::A : Doe, Jane j...@example.com
I've some more tests, maybe I should put them all in this post ?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] test-lib.sh: fix color support when tput needs ~/.terminfo
On Wed, Jun 17, 2015 at 03:23:49PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Wed, Jun 17, 2015 at 05:11:21PM -0400, Richard Hansen wrote: + test -z $1 test -n $quiet return + eval say_color_color=\$say_color_$1 Thanks, this looks much simpler. In the non-quiet case, you will eval $say_color_, even though we know it to be bogus. Yeah, but there is this gem in this patch: + ... + say_color_info=$(tput setaf 6) # cyan + say_color_reset=$(tput sgr0) + say_color_= # no formatting for normal text Oh, sorry, I was so focused on the later part that I totally missed that. That is rather elegant, and nicer than what I wrote. -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 v2] fetch-pack: check for shallow if depth given
When a repository is first fetched as a shallow clone, either by
git-clone or by fetching into an empty repo, the server's capabilities
are not currently consulted. The client will send shallow requests even
if the server does not understand them, and the resulting error may be
unhelpful to the user. This change pre-emptively checks so we can exit
with a helpful error if necessary.
Signed-off-by: Mike Edgar ad...@google.com
Acked-by: Jeff King p...@peff.net
---
fetch-pack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 48526aa..849a9d6 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -790,7 +790,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args
*args,
sort_ref_list(ref, ref_compare_name);
qsort(sought, nr_sought, sizeof(*sought), cmp_ref_by_name);
- if (is_repository_shallow() !server_supports(shallow))
+ if ((args-depth 0 || is_repository_shallow())
!server_supports(shallow))
die(Server does not support shallow clients);
if (server_supports(multi_ack_detailed)) {
if (args-verbose)
--
2.4.3.573.g4eafbef
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
On Thu, Jun 18, 2015 at 2:58 AM, Junio C Hamano gits...@pobox.com wrote: Torsten Bögershausen tbo...@web.de writes: On 2015-06-17 21.23, Junio C Hamano wrote: [] Basically, I'm fine with anything starting with Switch branches or, but please do change the headline ;-). Likewise; I agree switch branches or part is good. How about this: git-checkout - Switch branches or restore changes to the working tree Gahh. We are NOT restoring CHANGES. We are restoring the whole contents to a path. the whole contents is only true when --patch is not used, I think. -- 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: co-authoring commits
On Wed, Jun 17, 2015 at 01:57:12PM -0700, Junio C Hamano wrote: Tuncer Ayaz tuncer.a...@gmail.com writes: On Wed, Jun 17, 2015 at 9:58 PM, Junio C Hamano wrote: Tuncer Ayaz tuncer.a...@gmail.com writes: Is this something that breaks the design and would never be implemented, Yes. Junio, thanks for the quick response. I suppose things have changed since Jonathan Nieder's response in [1] (2010),... I do not think there is anything changed. Jonathan was being a bit more diplomatic and academic than I am. There is no reason in principle some faraway future version of Git could is _always_ true as a mental masturbation without taking reality into account, aka Sounds doable but a lot of trouble means it is doable but it is dubious that it is worth doing. What happens in old versions of git if you try to look at a signed git commit? The same level of interoperability used there would work here, with the additional property that this would be optional metadata so we might be able to make read-only access work with older versions. - Josh Triplett -- 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] progress: store throughput display in a strbuf
Coverity noticed that we strncpy() into a fixed-size buffer
without making sure that it actually ended up
NUL-terminated. This is unlikely to be a bug in practice,
since throughput strings rarely hit 32 characters, but it
would be nice to clean it up.
The most obvious way to do so is to add a NUL-terminator.
But instead, this patch switches the fixed-size buffer out
for a strbuf. At first glance this seems much less
efficient, until we realize that filling in the fixed-size
buffer is done by writing into a strbuf and copying the
result!
By writing straight to the buffer, we actually end up more
efficient:
1. We avoid an extra copy of the bytes.
2. Rather than malloc/free each time progress is shown, we
can strbuf_reset and use the same buffer each time.
Signed-off-by: Jeff King p...@peff.net
---
progress.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/progress.c b/progress.c
index 2e31bec..a3efcfd 100644
--- a/progress.c
+++ b/progress.c
@@ -25,7 +25,7 @@ struct throughput {
unsigned int last_bytes[TP_IDX_MAX];
unsigned int last_misecs[TP_IDX_MAX];
unsigned int idx;
- char display[32];
+ struct strbuf display;
};
struct progress {
@@ -98,7 +98,7 @@ static int display(struct progress *progress, unsigned n,
const char *done)
}
progress-last_value = n;
- tp = (progress-throughput) ? progress-throughput-display : ;
+ tp = (progress-throughput) ? progress-throughput-display.buf : ;
eol = done ? done :\r;
if (progress-total) {
unsigned percent = n * 100 / progress-total;
@@ -129,6 +129,7 @@ static int display(struct progress *progress, unsigned n,
const char *done)
static void throughput_string(struct strbuf *buf, off_t total,
unsigned int rate)
{
+ strbuf_reset(buf);
strbuf_addstr(buf, , );
strbuf_humanise_bytes(buf, total);
strbuf_addstr(buf, | );
@@ -141,7 +142,6 @@ void display_throughput(struct progress *progress, off_t
total)
struct throughput *tp;
uint64_t now_ns;
unsigned int misecs, count, rate;
- struct strbuf buf = STRBUF_INIT;
if (!progress)
return;
@@ -154,6 +154,7 @@ void display_throughput(struct progress *progress, off_t
total)
if (tp) {
tp-prev_total = tp-curr_total = total;
tp-prev_ns = now_ns;
+ strbuf_init(tp-display, 0);
}
return;
}
@@ -193,9 +194,7 @@ void display_throughput(struct progress *progress, off_t
total)
tp-last_misecs[tp-idx] = misecs;
tp-idx = (tp-idx + 1) % TP_IDX_MAX;
- throughput_string(buf, total, rate);
- strncpy(tp-display, buf.buf, sizeof(tp-display));
- strbuf_release(buf);
+ throughput_string(tp-display, total, rate);
if (progress-last_value != -1 progress_update)
display(progress, progress-last_value, NULL);
}
@@ -250,12 +249,9 @@ void stop_progress_msg(struct progress **p_progress, const
char *msg)
bufp = (len sizeof(buf)) ? buf : xmalloc(len + 1);
if (tp) {
- struct strbuf strbuf = STRBUF_INIT;
unsigned int rate = !tp-avg_misecs ? 0 :
tp-avg_bytes / tp-avg_misecs;
- throughput_string(strbuf, tp-curr_total, rate);
- strncpy(tp-display, strbuf.buf, sizeof(tp-display));
- strbuf_release(strbuf);
+ throughput_string(tp-display, tp-curr_total, rate);
}
progress_update = 1;
sprintf(bufp, , %s.\n, msg);
@@ -264,6 +260,8 @@ void stop_progress_msg(struct progress **p_progress, const
char *msg)
free(bufp);
}
clear_progress_signal();
+ if (progress-throughput)
+ strbuf_release(progress-throughput-display);
free(progress-throughput);
free(progress);
}
--
2.4.4.719.g3984bc6
--
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/3] trace: add GIT_TRACE_STDIN
On Wed, Jun 17, 2015 at 05:04:04PM +0700, Duy Nguyen wrote:
I wonder if we could do it a bit differently. Instead of
GIT_TRACE_STDIN, I would add GIT_TRACE_HOOK that points to a script.
Whenever a command is run via run-command interface, the actual
command line to be executed would be hook script command
arguments.
Hmm, yeah, I like that. It's even more flexible, and it is much more
obvious why it works only for run-command. If we feed the resulting
hooked command to the shell, I think you could do:
GIT_TRACE_HOOK='
f() {
case $1 $2 in
git pack-objects)
tee /tmp/foo.out | $@
;;
esac
}; f
'
That is not 100% correct (you would miss git --some-arg pack-objects),
but it is probably fine in practice for debugging sessions. It is a bit
more complicated to use, but I really like the flexibility (I can
imagine that GIT_TRACE_HOOK=gdbserver localhost:1234 would come in
handy).
Because this script is given full command line, it can decide to trace
something if the command name is matched (or arguments are matched) or
just execute the original command. It's more flexible that trace.*
config keys. We also have an opportunity to replace builtin commands,
like pack-objects, in command pipeline in fetch or push with something
else, to inject errors or whatever. It can be done manually, but it's
not easy or convenient.
My other motive for trace.* was that we could have something like
trace.prune, and have git-prune provide verbose debugging information.
We have custom patches like that on GitHub servers, which we've used to
debug occasional weirdness (e.g., you find that an object is missing
from a repo, but you have no clue why it went away; was it never there,
did somebody prune it, did it get dropped from a pack?).
I can send those upstream, but it would be nice not to introduce a
totally separate tracing facility when trace_* is so close. But it
needs:
1. To be enabled by config, not environment.
2. To support some basic output filename flexibility so the output can
be organized (we write the equivalent of GIT_TRACE_FOO to
$GIT_DIR/ghlog_foo/-MM-DD/-MM-DDTHH:MM:SS.PID).
For (1), we could just load trace.* in git_default_config; you couldn't
use it with any early tracing that happens before then, but I think in
practice it would be fine for most traces.
For (2), I think we could accomplish that with %-placeholders (like my
earlier patch), and the ability to write relative paths into $GIT_DIR
(again, you couldn't do this for early traces, but you could for other
stuff).
Or we could just do nothing. I'm not sure if anybody else is actually
interested in verbose-logging patches like these.
-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: Should the --encoding argument to log/show commands make any guarantees about their output?
* just make this more clear in the docs and/or * should we adjust the behavior of --encoding or * should we do something entirely different, like adding a new command line option or The general spirit is to keep things backwards compatible, so that users which expect the raw (and possible corrupted UTF-8) data still get the same results, when they updata their Git installation. A new command line option will allow users to get clean UTF-8. One suggestion could be --fixbroken=ISO-8859-1(a) --fixbroken=octalescape (b) --fixbroken=hexescape (c) (a) would replace 0xf6 with 0xc3 0xb6 (b) could write \366 (c) could write F6 The exact form of the syntax can be discussed of course. However, I would probably start with (a), and add other options if needed. * should we just leave things as they are? not the ideal thing. -- 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] test-lib.sh: fix color support when tput needs ~/.terminfo
On 2015-06-17 15:43, Jeff King wrote:
On Wed, Jun 17, 2015 at 03:06:26PM -0400, Richard Hansen wrote:
+say_color_error=$(tput bold; tput setaf 1) # bold red
+say_color_skip=$(tput setaf 4) # blue
+say_color_warn=$(tput setaf 3) # brown/yellow
+say_color_pass=$(tput setaf 2) # green
+say_color_info=$(tput setaf 6) # cyan
+say_color_sgr0=$(tput sgr0)
[...]
+error|skip|warn|pass|info)
+eval say_color_color=\$say_color_$1;;
*)
test -n $quiet return;;
I think you could dispense with this case statement entirely and do:
eval say_color_color=\$say_color_$1
if test -z $say_color_color; then
test -n $quiet return
fi
I guess that is making the assumption that all colors have non-zero
sizes, but that seems reasonable.
We could test if the variable is set first (test -n ${foo+set}), at
the cost of a bit more complexity.
I do not mind it so much as you have
it, but it does mean adding a new field needs to update two spots.
I also don't like the duplicate list of color types, and I considered
doing something similar to what you suggested, but I decided against it.
I'm a bit worried about bizarre syntax errors or code execution if
say_color() is used improperly. ('eval' with uncontrolled variables
makes me nervous.)
Thanks for reviewing,
Richard
--
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: Should the --encoding argument to log/show commands make any guarantees about their output?
Jeff King p...@peff.net writes: I would vote for a documentation change, perhaps like: Subject: docs: clarify that --encoding can produce invalid sequences In the common case that the commit encoding matches the output encoding, we do not touch the buffer at all, which makes things much more efficient. But it might be unclear to a consumer that we will pass through bogus sequences. Signed-off-by: Jeff King p...@peff.net --- Sounds like a sensible thing to do. Documentation/pretty-options.txt | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt index 74aa01a..642af6e 100644 --- a/Documentation/pretty-options.txt +++ b/Documentation/pretty-options.txt @@ -37,7 +37,10 @@ people using 80-column terminals. in their encoding header; this option can be used to tell the command to re-code the commit log message in the encoding preferred by the user. For non plumbing commands this - defaults to UTF-8. + defaults to UTF-8. Note that if an object claims to be encoded + in `X` and we are outputting in `X`, we will output the object + verbatim; this means that invalid sequences in the original + commit may be copied to the output. --notes[=ref]:: Show the notes (see linkgit:git-notes[1]) that annotate the -- 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] test-lib.sh: fix color support when tput needs ~/.terminfo
On Wed, Jun 17, 2015 at 03:55:05PM -0400, Richard Hansen wrote:
I do not mind it so much as you have
it, but it does mean adding a new field needs to update two spots.
I also don't like the duplicate list of color types, and I considered
doing something similar to what you suggested, but I decided against it.
I'm a bit worried about bizarre syntax errors or code execution if
say_color() is used improperly. ('eval' with uncontrolled variables
makes me nervous.)
As Junio pointed out, I think all bets are off in the test scripts. They
are running tons of arbitrary code. :)
But for the record, I am fine with your patch as-is. Thanks for looking
into it.
-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: co-authoring commits
Tuncer Ayaz tuncer.a...@gmail.com writes: On Wed, Jun 17, 2015 at 9:58 PM, Junio C Hamano wrote: Tuncer Ayaz tuncer.a...@gmail.com writes: Is this something that breaks the design and would never be implemented, Yes. Junio, thanks for the quick response. I suppose things have changed since Jonathan Nieder's response in [1] (2010),... I do not think there is anything changed. Jonathan was being a bit more diplomatic and academic than I am. There is no reason in principle some faraway future version of Git could is _always_ true as a mental masturbation without taking reality into account, aka Sounds doable but a lot of trouble means it is doable but it is dubious that it is worth doing. -- 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 status -z' missing separators on OSX
On Tue, Jun 16, 2015 at 11:32 PM, Jeff King p...@peff.net wrote: On Tue, Jun 16, 2015 at 06:21:56PM -0500, Tad Hardesty wrote: ~/test (master #)$ git status -z | hexdump -C 41 20 20 61 41 20 20 62 |A aA b| 0008 That's really weird. I don't have a Yosemite box available, but I can't reproduce on the Mavericks box I have access to. Still, I'd suspect something weird in your config (e.g., something that is inserting itself on the output pipe of git status). I also am unable to reproduce this behavior (using Yosemite). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] test-lib.sh: fix color support when tput needs ~/.terminfo
Jeff King p...@peff.net writes: On Wed, Jun 17, 2015 at 05:11:21PM -0400, Richard Hansen wrote: +test -z $1 test -n $quiet return +eval say_color_color=\$say_color_$1 Thanks, this looks much simpler. In the non-quiet case, you will eval $say_color_, even though we know it to be bogus. Yeah, but there is this gem in this patch: + ... + say_color_info=$(tput setaf 6) # cyan + say_color_reset=$(tput sgr0) + say_color_= # no formatting for normal text In other words, the patch handles these two in the same mechanism: say_color error this is my error message say_color ok this is just a regular message and treating an empy string just one of the supported colors, i.e. error, skip, warn, pass, info reset and are the colors. I guess we need to make sure say_color_color is blank, though. The alternative would be: if test -z $1; then test -n $quiet return say_color_color= else eval say_color_color=\$say_color_$1 fi I dunno if that makes the intent more clear or not. I am OK with it either way. I am OK with it either way, 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: co-authoring commits
On Wed, Jun 17, 2015 at 9:58 PM, Junio C Hamano wrote: Tuncer Ayaz tuncer.a...@gmail.com writes: Is this something that breaks the design and would never be implemented, Yes. Junio, thanks for the quick response. I suppose things have changed since Jonathan Nieder's response in [1] (2010), or I've read too much into the mini-thread between Jonathan and Josh. I was under the impression that this is generally possible without shaking up all underpinnings. For what it's worth, here's why I would use the feature: By allowing multiple authors, you don't have to decide who's the primary author, as in such situations usually there is no primary at all. I sometimes deliberately override the author when committing and add myself just as another co-author in the commit message, but as others have noted it would be really great if we can just specify multiple authors. [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=451880 -- 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: co-authoring commits
On Wed, Jun 17, 2015 at 10:26:32PM +0200, Tuncer Ayaz wrote: On Wed, Jun 17, 2015 at 9:58 PM, Junio C Hamano wrote: Tuncer Ayaz tuncer.a...@gmail.com writes: Is this something that breaks the design and would never be implemented, Yes. Junio, thanks for the quick response. I suppose things have changed since Jonathan Nieder's response in [1] (2010), or I've read too much into the mini-thread between Jonathan and Josh. I was under the impression that this is generally possible without shaking up all underpinnings. For what it's worth, here's why I would use the feature: By allowing multiple authors, you don't have to decide who's the primary author, as in such situations usually there is no primary at all. I sometimes deliberately override the author when committing and add myself just as another co-author in the commit message, but as others have noted it would be really great if we can just specify multiple authors. Having more than one author field in a commit would likely break things, but having a coauthor field seems plausible these days. Git added support for signed commits, and the world didn't end, so it's possible to extend the commit format. - Josh Triplett -- 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 0/2] redo fix for test-lib.sh color support
Changes from v1: * Eliminate the case statement and assume the user passed a sane value for $1. * Use the same test as the non-colorized version of say_color() when determining whether to suppress the output: assume that a message can only be suppresed if $1 is the empty string. This avoids the need to test whether the variable say_color_$1 is set. * Rename say_color_sgr0 to say_color_reset. * Add a new variable say_color_ (set to the empty string) as a way of documenting that $1 is expected to be the empty string for normal text. Richard Hansen (2): Revert test-lib.sh: do tests for color support after changing HOME test-lib.sh: fix color support when tput needs ~/.terminfo t/test-lib.sh | 99 --- 1 file changed, 47 insertions(+), 52 deletions(-) -- 2.4.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
[PATCH v2 1/2] Revert test-lib.sh: do tests for color support after changing HOME
This reverts commit 102fc80d32094ad6598b17ab9d607516ee8edc4a.
There are two issues with that commit:
* It is buggy. In pseudocode, it is doing:
color is set || TERM != dumb color works color=t
when it should be doing:
color is set || { TERM != dumb color works color=t }
* It unnecessarily disables color when tput needs to read
~/.terminfo to get the control sequences.
---
t/test-lib.sh | 90 ---
1 file changed, 43 insertions(+), 47 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 39da9c2..57212ec 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -181,8 +181,16 @@ export _x05 _x40 _z40 LF u200c
# This test checks if command xyzzy does the right thing...
# '
# . ./test-lib.sh
+test x$ORIGINAL_TERM != xdumb (
+ TERM=$ORIGINAL_TERM
+ export TERM
+ test -t 1
+ tput bold /dev/null 21
+ tput setaf 1 /dev/null 21
+ tput sgr0 /dev/null 21
+ )
+ color=t
-unset color
while test $# -ne 0
do
case $1 in
@@ -253,6 +261,40 @@ then
verbose=t
fi
+if test -n $color
+then
+ say_color () {
+ (
+ TERM=$ORIGINAL_TERM
+ export TERM
+ case $1 in
+ error)
+ tput bold; tput setaf 1;; # bold red
+ skip)
+ tput setaf 4;; # blue
+ warn)
+ tput setaf 3;; # brown/yellow
+ pass)
+ tput setaf 2;; # green
+ info)
+ tput setaf 6;; # cyan
+ *)
+ test -n $quiet return;;
+ esac
+ shift
+ printf %s $*
+ tput sgr0
+ echo
+ )
+ }
+else
+ say_color() {
+ test -z $1 test -n $quiet return
+ shift
+ printf %s\n $*
+ }
+fi
+
error () {
say_color error error: $*
GIT_EXIT_OK=t
@@ -829,52 +871,6 @@ HOME=$TRASH_DIRECTORY
GNUPGHOME=$HOME/gnupg-home-not-used
export HOME GNUPGHOME
-# run the tput tests *after* changing HOME (in case ncurses needs
-# ~/.terminfo for $TERM)
-test -n ${color+set} || test x$ORIGINAL_TERM != xdumb (
- TERM=$ORIGINAL_TERM
- export TERM
- test -t 1
- tput bold /dev/null 21
- tput setaf 1 /dev/null 21
- tput sgr0 /dev/null 21
- )
- color=t
-
-if test -n $color
-then
- say_color () {
- (
- TERM=$ORIGINAL_TERM
- export TERM
- case $1 in
- error)
- tput bold; tput setaf 1;; # bold red
- skip)
- tput setaf 4;; # blue
- warn)
- tput setaf 3;; # brown/yellow
- pass)
- tput setaf 2;; # green
- info)
- tput setaf 6;; # cyan
- *)
- test -n $quiet return;;
- esac
- shift
- printf %s $*
- tput sgr0
- echo
- )
- }
-else
- say_color() {
- test -z $1 test -n $quiet return
- shift
- printf %s\n $*
- }
-fi
-
if test -z $TEST_NO_CREATE_REPO
then
test_create_repo $TRASH_DIRECTORY
--
2.4.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
[PATCH v2 2/2] test-lib.sh: fix color support when tput needs ~/.terminfo
If tput needs ~/.terminfo for the current $TERM, then tput will
succeed before HOME is changed to $TRASH_DIRECTORY (causing color to
be set to 't') but fail afterward.
One possible way to fix this is to treat HOME like TERM: back up the
original value and temporarily restore it before say_color() runs
tput.
Instead, pre-compute and save the color control sequences before
changing either TERM or HOME. Use the saved control sequences in
say_color() rather than call tput each time. This avoids the need to
back up and restore the TERM and HOME variables, and it avoids the
overhead of a subshell and two invocations of tput per call to
say_color().
Signed-off-by: Richard Hansen rhan...@bbn.com
---
t/test-lib.sh | 57 -
1 file changed, 28 insertions(+), 29 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 57212ec..cea6cda 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -15,9 +15,6 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see http://www.gnu.org/licenses/ .
-# Keep the original TERM for say_color
-ORIGINAL_TERM=$TERM
-
# Test the binaries we have just built. The tests are kept in
# t/ subdirectory and are run in 'trash directory' subdirectory.
if test -z $TEST_DIRECTORY
@@ -68,12 +65,12 @@ done,*)
esac
# For repeatability, reset the environment to known value.
+# TERM is sanitized below, after saving color control sequences.
LANG=C
LC_ALL=C
PAGER=cat
TZ=UTC
-TERM=dumb
-export LANG LC_ALL PAGER TERM TZ
+export LANG LC_ALL PAGER TZ
EDITOR=:
# A call to unset with no arguments causes at least Solaris 10
# /usr/xpg4/bin/sh and /bin/ksh to bail out. So keep the unsets
@@ -181,9 +178,7 @@ export _x05 _x40 _z40 LF u200c
# This test checks if command xyzzy does the right thing...
# '
# . ./test-lib.sh
-test x$ORIGINAL_TERM != xdumb (
- TERM=$ORIGINAL_TERM
- export TERM
+test x$TERM != xdumb (
test -t 1
tput bold /dev/null 21
tput setaf 1 /dev/null 21
@@ -263,29 +258,30 @@ fi
if test -n $color
then
+ # Save the color control sequences now rather than run tput
+ # each time say_color() is called. This is done for two
+ # reasons:
+ # * TERM will be changed to dumb
+ # * HOME will be changed to a temporary directory and tput
+ # might need to read ~/.terminfo from the original HOME
+ # directory to get the control sequences
+ # Note: This approach assumes the control sequences don't end
+ # in a newline for any terminal of interest (command
+ # substitutions strip trailing newlines). Given that most
+ # (all?) terminals in common use are related to ECMA-48, this
+ # shouldn't be a problem.
+ say_color_error=$(tput bold; tput setaf 1) # bold red
+ say_color_skip=$(tput setaf 4) # blue
+ say_color_warn=$(tput setaf 3) # brown/yellow
+ say_color_pass=$(tput setaf 2) # green
+ say_color_info=$(tput setaf 6) # cyan
+ say_color_reset=$(tput sgr0)
+ say_color_= # no formatting for normal text
say_color () {
- (
- TERM=$ORIGINAL_TERM
- export TERM
- case $1 in
- error)
- tput bold; tput setaf 1;; # bold red
- skip)
- tput setaf 4;; # blue
- warn)
- tput setaf 3;; # brown/yellow
- pass)
- tput setaf 2;; # green
- info)
- tput setaf 6;; # cyan
- *)
- test -n $quiet return;;
- esac
+ test -z $1 test -n $quiet return
+ eval say_color_color=\$say_color_$1
shift
- printf %s $*
- tput sgr0
- echo
- )
+ printf %s\\n $say_color_color$*$say_color_reset
}
else
say_color() {
@@ -295,6 +291,9 @@ else
}
fi
+TERM=dumb
+export TERM
+
error () {
say_color error error: $*
GIT_EXIT_OK=t
--
2.4.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/RFC v4 07/10] send-email: reduce dependancies impact on parse_address_line
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes:
parse_address_line had not the same behavior whether the user had
Mail::Address or not. Teach parse_address_line to behave like
Mail::Address.
Sounds like a fun project ;-)
+ my $commentrgx=qr/\((?:[^)]*)\)/;
+ my $quotergx=qr/(?:[^\\\]|\\.)*/;
+ my $wordrgx=qr/(?:[^][\s():;@\\,.]|\\.)+/;
+ my $tokenrgx = qr/(?:$quotergx|$wordrgx|$commentrgx|\S)/;
Suffix rgx that means regular expression is a bit unusual, and
also hard to read when squashed to another word. Elsewhere in the
same script, we seem to use $re_whatever to store precompiled
regular expressions, so perhaps $re_comment, $re_quote, etc.?
+ my @tokens = map { $_ =~ /\s*($tokenrgx)\s*/g } @_;
+ push @tokens, ,;
+
+ my (@addr_list, @phrase, @address, @comment, @buffer) = ();
+ foreach my $token (@tokens) {
+ if ($token =~ /^[,;]$/) {
+ if (@address) {
+ push @address, @buffer;
+ } else {
+ push @phrase, @buffer;
+ }
+
+ my $str_phrase = join ' ', @phrase;
+ my $str_address = join '', @address;
+ my $str_comment = join ' ', @comment;
+
+ if ($str_phrase =~ /[][():;@\\,.\000-\037\177]/) {
+ $str_phrase =~ s/(^|[^\\])/$1/g;
+ $str_phrase = qq[$str_phrase];
+ }
+
+ if ($str_address ne $str_phrase ne ) {
+ $str_address = qq[$str_address];
+ }
We see both git@vger.kernel.org and git@vger.kernel.org around
here for an address without comment or phrase; this chooses to turn
them both into git@vger.kernel.org form? Not a complaint but am
thinking aloud to see if I am reading it correctly.
+
+ my $str_mailbox = $str_phrase $str_address $str_comment;
+ $str_mailbox =~ s/^\s*|\s*$//g;
So an empty @comment will not leave spaces after $str_address, which
makes sense (likewise for @phrase).
+ push @addr_list, $str_mailbox if ($str_mailbox);
+
+ @phrase = @address = @comment = @buffer = ();
+ } elsif ($token =~ /^\(/) {
+ push @comment, $token;
+ } elsif ($token eq ) {
+ push @phrase, (splice @address), (splice @buffer);
That is a clever use of splice (My Perl's rusty; you learn new
things every day) ;-)
+ } elsif ($token eq ) {
+ push @address, (splice @buffer);
+ } elsif ($token eq @) {
+ push @address, (splice @buffer), @;
+ } elsif ($token eq .) {
+ push @address, (splice @buffer), .;
+ } else {
+ push @buffer, $token;
+ }
+ }
+
+ return @addr_list;
}
--
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: co-authoring commits
j...@joshtriplett.org writes: Having more than one author field in a commit would likely break things, but having a coauthor field seems plausible these days. Git added support for signed commits, and the world didn't end, so it's possible to extend the commit format. Something being possible and something being sensible are two different things, though. I agree coauthor field that is not understood by anybody would unlikely break existing implementations, but it is not a useful way to add this information to commit objects. For one thing, until you teach git log or its equivalents in everybody's (re)implementation of Git, the field will not be shown, you cannot easily edit it while amending or rebasing, git log --grep= would not know about it, and you would need git cat-file commit to view it. A footer Co-authored-by: does not have any such issue. We left commit headers extensible long before we introduced commit signing, and we used it to add the encoding header. In general, we invent new headers only when structurely necessary. When you declare that the log message for this indiviaul commit is done in one encoding, that is not something you would want to _edit_ with your editor while you are editing your message. Similarly you would not want to risk touching the GPG signature of a signed commit or a signed merge while editing your message. The _only_ reason I would imagine why somebody may be tempted to think that coauthor as part of the object header makes sense is because author is already there. You can argue that author did not have to be part of the object header, and that is right. I would agree with you 100% that author did not have to be there. But that is too late to change. And being consistent with a past mistake is not a good reason to repeat that same mistake. -- 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: co-authoring commits
On Wed, Jun 17, 2015 at 11:51 PM, Junio C Hamano wrote: j...@joshtriplett.org writes: Having more than one author field in a commit would likely break things, but having a coauthor field seems plausible these days. Git added support for signed commits, and the world didn't end, so it's possible to extend the commit format. Something being possible and something being sensible are two different things, though. I agree coauthor field that is not understood by anybody would unlikely break existing implementations, but it is not a useful way to add this information to commit objects. For one thing, until you teach git log or its equivalents in everybody's (re)implementation of Git, the field will not be shown, you cannot easily edit it while amending or rebasing, git log --grep= would not know about it, and you would need git cat-file commit to view it. A footer Co-authored-by: does not have any such issue. We left commit headers extensible long before we introduced commit signing, and we used it to add the encoding header. In general, we invent new headers only when structurely necessary. When you declare that the log message for this indiviaul commit is done in one encoding, that is not something you would want to _edit_ with your editor while you are editing your message. Similarly you would not want to risk touching the GPG signature of a signed commit or a signed merge while editing your message. The _only_ reason I would imagine why somebody may be tempted to think that coauthor as part of the object header makes sense is because author is already there. You can argue that author did not have to be part of the object header, and that is right. I would agree with you 100% that author did not have to be there. But that is too late to change. And being consistent with a past mistake is not a good reason to repeat that same mistake. Makes sense. Without intimate knowledge of current internals, what about the following potentially crazy plan? 1. demote/deprecate GIT_AUTHOR_* 2. implement a new author-ship model that supports both and treats the old entries as supported-but-deprecated 3. maybe auto-migrate entries in the repo, or add a switch to do that as part of git-gc or another process 4. extend tooling to support 'commit --add-author' or similar 5. teach git.git tools to properly display additional authors as equals in commit ownership 6. let other tools catch up, but rest assured nothing was broken 7. consider other use cases and different implementations (flexibility), to not have to repeat this 5 years down the road for another field -- 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: co-authoring commits
On Wed, Jun 17, 2015 at 02:51:18PM -0700, Junio C Hamano wrote: j...@joshtriplett.org writes: Having more than one author field in a commit would likely break things, but having a coauthor field seems plausible these days. Git added support for signed commits, and the world didn't end, so it's possible to extend the commit format. Something being possible and something being sensible are two different things, though. I agree coauthor field that is not understood by anybody would unlikely break existing implementations, but it is not a useful way to add this information to commit objects. For one thing, until you teach git log or its equivalents in everybody's (re)implementation of Git, the field will not be shown, you cannot easily edit it while amending or rebasing, git log --grep= would not know about it, and you would need git cat-file commit to view it. A footer Co-authored-by: does not have any such issue. Sure it does; while it would display in raw form because it's a part of the commit message, you'd still have to teach git log --author about it (git grep is not a substitute), map it through mailmap, teach git shortlog about it, teach send-email and format-patch to use it in mail headers, teach repository statistics tools about it, and in general teach every tool that reads the author field of a commit to handle co-authors. And if it's a pseudo-field in the commit, you'd also have to have more complex parsing rules to find and parse it. Git has almost no understanding of in-band magic headers in a commit message. It has a bit of support for generating (but not parsing) Signed-off-by, and send-email has some support for adding *-by headers to Cc, but a pseudo-header that git tools actually *parse* out of the commit message would be a first. We left commit headers extensible long before we introduced commit signing, and we used it to add the encoding header. In general, we invent new headers only when structurely necessary. When you declare that the log message for this indiviaul commit is done in one encoding, that is not something you would want to _edit_ with your editor while you are editing your message. Similarly you would not want to risk touching the GPG signature of a signed commit or a signed merge while editing your message. The _only_ reason I would imagine why somebody may be tempted to think that coauthor as part of the object header makes sense is because author is already there. You can argue that author did not have to be part of the object header, and that is right. I would agree with you 100% that author did not have to be there. Author and committer are used by many git tools; if they weren't part of the object header, they'd need to be part of some pseudo-header with a standardized format that git can parse. - Josh Triplett -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
Duy Nguyen pclo...@gmail.com writes: How about this: git-checkout - Switch branches or restore changes to the working tree Gahh. We are NOT restoring CHANGES. We are restoring the whole contents to a path. the whole contents is only true when --patch is not used, I think. I've seen that people repeat this patch is not the whole and have ignored that comment; you really need to think if that nitpick adds anything of value to the description before repeating it. The patch interface of course allows you to pick and choose. You have some contents (call it W) in the working tree. You have different contents (call it X) somewhere else. Being able to do that is the whole point of the feature. But what is presented you as the choice to pick or ignore? It is the difference between W and X. If you take none from what is offered, you won't check out anything. If you take all of them, you check out the whole of X. The result is somewhere in between. An important point that everybody who repeats patch is not the whole seems to be missing is that it will never be somewhere between W and Y (the latter of which is different from X). Now, what is the X in this operation? It is either what is registered in the index, or in the tree-ish specified on the command line. So I'd say that the right mental model to understand the --patch feature is that it allows you to check out the whole contents from elsewhere; after the command line argument selects from where, i.e. either from the index or from a tree-ish, you _additionally_ have a choice to pick which part of that whole to use. The diff between W and HEAD or W and index, i.e. CHANGES, does not play any part of this selection process. -- 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] mergetools: add config option to disable auto-merge
On Wed, Jun 17, 2015 at 3:41 PM, Junio C Hamano gits...@pobox.com wrote: Michael Rappazzo rappa...@gmail.com writes: For some mergetools, the current invocation of git mergetool will include an auto-merge flag. By default the flag is included, however if the git config option 'merge.automerge' is set to 'false', then that flag will now be omitted. ... and why is the automerge a bad thing that user would want to avoid triggering under which condition? That description may not have to be in the proposed log message, but it would help users when they decide if they want to use the configuration to describe it in the mergetool.automerge configuration. And depending on the answer to the above question, a configuration variable may turn out be a bad mechanism to customize this (namely, set-and-forget configuration variable is a bad match for a knob that is more per invocation than user taste). Is this not about automerge but more about always-show-UI because I like GUI? Then that may be a user taste thing that is a good match for a configuration variable. I simply cannot tell from what was in the message I am responding to. I feel that the auto-merge takes away the context of the changes. I use araxis merge as my mergetool of choice. I almost always immediately undo the auto-merge. After taking a moment to look at each file, I will then (usually) use the keyboard shortcut for auto-merge. It sure would be nice to set-and-forget a config variable to remove the annoyance of having to undo the auto-merge. I think that this config option is reasonable. Perhaps my documentation leaves something to be desired. I can take another stab at it. -TEMPORARY FILES -`git mergetool` creates `*.orig` backup files while resolving merges. -These are safe to remove once a file has been merged and its -`git mergetool` session has completed. - +CONFIGURATION OPTIONS +- +mergetool.keepBackup:: + `git mergetool` creates `*.orig` backup files while resolving merges. + These are safe to remove once a file has been merged and its + `git mergetool` session has completed. ++ This is an unrelated change; I think it is a good change, though. I however suspect that we would not want to repeat the configuration description in this file and instead mention these in see also section referring the readers to git-config(1). I felt that adding a separate header for a different config option was more appropriate, so I went with this. Pointing to the config.txt doc is probably better. -- 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: Use get_sha1_committish instead of read_ref in init_notes()
On Wed, Jun 17, 2015 at 5:18 PM, Junio C Hamano gits...@pobox.com wrote: Mike Hommey m...@glandium.org writes: I'm tempted to make init_notes itself do the check, based on the value it is given for a read_only argument. Yeah, that would be one sensible way to go after making sure that everything goes thru this interface. Agreed. Furthermore, consider adding the read_only flag (or however you choose to encode it internally) to struct notes_tree, so that the API functions that _manipulate_ notes trees can immediately bail out when used on a read-only tree (i.e. we want them to fail as early as possible). On the other hand, some commands do their ref resolving themselves already. Again, as long as they do not bypass the read-only safety you are suggesting to add to init_notes(), that is OK. Agreed. An alternative to adding a simple read_only flag argument is to modify the const char *notes_ref argument into two separate arguments: const char *notes_treeish, and const char *update_ref, where the latter should be NULL for read-only trees. That said, currently the logic for actually updating notes ref lives outside the notes.h API (see commit_notes() in notes-utils.h/c), so there might be room for more consolidation/refactoring here... ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
Torsten Bögershausen tbo...@web.de writes: My v3 will probably use the original line: git-checkout - Checkout a branch or paths to the working tree I think mentionning Switch branch was a real improvement. For someone not familiar with the version control vocabulary, checkout does not mean much (just looked in a dictionary, it talks about payment and leaving a room in a hotel ...). And someone not understanding what checkout means in this context won't be helped much reading the description and getting checkout there. (Ironically, Junio did almost the same remark when I proposed to document git describe as Describe ..., but the word describe does not have the ambiguity problem that checkout has) 'git checkout commit -- path' will copy the version from another commit into the workspace. If commit exists, it means that the state of this path existed somewhere in path in the past (well, modulo git add -p and other ways to cheat with history). So, to me, restore a previous version does apply in this case. Perhaps restore a recorded state into the worktree (my favorite up to now I think). But as you say, it copies into the workspace, so copy a previous version into the workspace sounds good to me. Basically, I'm fine with anything starting with Switch branches or, but please do change the headline ;-). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity
On Wed, Jun 17, 2015 at 3:16 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
It usually goes like this
strbuf sb = STRBUF_INIT;
if (!strncmp(sb.buf, foo, 3))
printf(%s, sb.buf + 3);
Coverity thinks that printf() can be executed, and because initial
sb.buf only has one character (from strbuf_slopbuf), sb.buf + 3 is out
of bound. What it does not recognize is strbuf_slopbuf[0] is always (*)
zero. We always do some string comparison before jumping ahead to
sb.buf + 3 and those operations will stop out of bound accesses.
Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's
happy, we'll have cleaner defect list and better chances of spotting
true defects.
(*) It's not entirely wrong though. Somebody may do sb.buf[0] = 'f'
right after variable declaration and ruin all unused strbuf.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
There are lots of false warnings like this from Coverity. I just
wanted to kill them off so we can spot more serious problems easier.
I can't really verify that this patch shuts off those warnings
because scan.coverity.com policy does not allow forks.
Thanks a lot for looking into this!
I'll just took this patch and have run a custom build now.
(Depending on the outcome of the discussion, I may just carry
this patch around on top of the origin/pu for each scan.)
This patch is however a work around and not fixing the root problem.
(The root problem being, coverity is not good enough to understand the
implications of strncmp, skip_prefix, starts_with or such).
The trade off is we're not able to detect problems with strbuf if any arise.
I had another patch that avoids corrupting strbuf_slopbuf, by putting
it to .rodata section. The patch is more invasive though, because
this statement buf.buf[buf.len] = '\0' now has to make sure buf.buf
is not strbuf_slopbuf. It feels safer, but probably not enough to
justify the change.
strbuf.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/strbuf.c b/strbuf.c
index 0d4f4e5..0d7c3cf 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -16,7 +16,12 @@ int starts_with(const char *str, const char *prefix)
* buf is non NULL and -buf is NUL terminated even for a freshly
* initialized strbuf.
*/
+#ifndef __COVERITY__
char strbuf_slopbuf[1];
+#else
+/* Stop so many incorrect out-of-boundary warnings from Coverity */
+char strbuf_slopbuf[64];
+#endif
void strbuf_init(struct strbuf *sb, size_t hint)
{
--
2.3.0.rc1.137.g477eb31
--
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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] t3404: demonstrate CHERRY_PICK_HEAD bug
Johannes Schindelin johannes.schinde...@gmx.de writes: +test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' ' + git checkout -b commit-to-skip + for double in X 3 1 + do + seq 5 | sed s/$double// seq + git add seq + test_tick + git commit -m seq-$double + done + git tag seq-onto + git reset --hard HEAD~2 + git cherry-pick seq-onto + test_must_fail git rebase -i seq-onto Shouldn't this explicitly specify what fake editor is to be used, instead of relying on whatever the last test happened to have used? I thought this deserved to go to older maintenance track, but the fake editor that was used last are different between branches, so rebase -i fails for a wrong reason (i.e. cannot spawn the editor) when queued on say maint-2.2. + test -d .git/rebase-merge + git rebase --continue + git diff seq-onto I am puzzled with this diff; what is this about? Is it a remnant from an earlier debugging session, or is it making sure seq-onto is a valid tree-ish? + test ! -d .git/rebase-merge + test ! -f .git/CHERRY_PICK_HEAD +' + test_done 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] Documentation/i18n.txt: clarify character encoding support
Karsten Blees karsten.bl...@gmail.com writes: I do not think the removal of the text makes much sense here unless you add the equivalent to the new text below. - The contents of the blob objects are uninterpreted sequences of bytes. There is no encoding translation at the core level. - - The commit log messages are uninterpreted sequences of non-NUL - bytes. + - Pathnames are encoded in UTF-8 normalization form C. This That is true only on some systems like OSX (with HFS+) and Windows, no? BSDs in general and Linux do not do any such mangling IIRC. Modern Unices don't need any such mangling because UTF-8 NFC should be the default system encoding. I'm not sure for BSDs, but it has been the default on all major Linux distros for more than 10 years. So? All major distros do not have to worry (and do not even need to know). As I said,... I am OK with mangling described as a notable oddball to warn users, though; i.e. not as a norm as your new text suggests but as an exception. ... I am OK to describe pathnames are mangled into UTF-8 NFC on certain filesystems as a warning. I am OK if we encourage the use of UTF-8, especially if a project wants to be forward looking (i.e. it may currently be a monoculture but may become cross platform in the future). I just do not want to see us saying you *must* encode your path in UTF-8 NFC. ISO-8859-x file names may be fine if you won't ever need to: - use git-web, JGit, gitk, git-gui... - exchange repos with normal (UTF-8) Unices, Mac and Windows systems - publish your work on a git hosting service (and expect file and ref names to show up correctly in the web interface) - store the repo on Unicode-based file systems (JFS, Joliet, UDF, exFat, NTFS, HFS, CIFS...) Yes, that is exatly what I said, isn't it? Use whatever works for your project, we do not dictate. These restrictions are not that obvious when you start a new git project,... Or any project for that matter, not limited to git project, no? Perhaps that is a moot point by now, as everything in the workd seems to be a git project these days. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
Andres G. Aragoneses kno...@gmail.com writes: On 17/06/15 12:54, Matthieu Moy wrote: Duy Nguyen pclo...@gmail.com writes: On Wed, Jun 17, 2015 at 2:54 PM, Torsten Bögershausen tbo...@web.de wrote: -git-checkout - Checkout a branch or paths to the working tree +git-checkout - Switch branches or restore changes I didn't follow closely the previous discussion. (Neither did I) Forgive me if this is already discussed, but I would keep the in the working tree. Restore changes alone seems vague. Restore previous version would be better than Restore changes to me. previous version sounds ambiguous. Yes, but git checkout can do many things. It can restore an old commited state, restore from the index, ... so we need to either be vague, or use a long enumeration. How about discard local changes? To me this describes git checkout HEAD, but neither git checkout -- file nor git checkout HEAD^^^. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
On 17/06/15 12:54, Matthieu Moy wrote: Duy Nguyen pclo...@gmail.com writes: On Wed, Jun 17, 2015 at 2:54 PM, Torsten Bögershausen tbo...@web.de wrote: -git-checkout - Checkout a branch or paths to the working tree +git-checkout - Switch branches or restore changes I didn't follow closely the previous discussion. (Neither did I) Forgive me if this is already discussed, but I would keep the in the working tree. Restore changes alone seems vague. Restore previous version would be better than Restore changes to me. previous version sounds ambiguous. How about discard local changes? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
On 17/06/15 13:54, Matthieu Moy wrote: Andres G. Aragoneses kno...@gmail.com writes: On 17/06/15 12:54, Matthieu Moy wrote: Duy Nguyen pclo...@gmail.com writes: On Wed, Jun 17, 2015 at 2:54 PM, Torsten Bögershausen tbo...@web.de wrote: -git-checkout - Checkout a branch or paths to the working tree +git-checkout - Switch branches or restore changes I didn't follow closely the previous discussion. (Neither did I) Forgive me if this is already discussed, but I would keep the in the working tree. Restore changes alone seems vague. Restore previous version would be better than Restore changes to me. previous version sounds ambiguous. Yes, but git checkout can do many things. It can restore an old commited state, restore from the index, ... so we need to either be vague, or use a long enumeration. How about discard local changes? To me this describes git checkout HEAD, but neither git checkout -- file nor git checkout HEAD^^^. I didn't mean to use just discard local changes. I was proposing that as a replacement to the restore changes substring. -- 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's cooking in git.git (Jun 2015, #04; Tue, 16)
Junio C Hamano gits...@pobox.com writes: * gr/rebase-i-drop-warn (2015-06-01) 2 commits - git rebase -i: warn about removed commits - git-rebase -i: add command drop to remove a commit Add drop commit-object-name subject command as another way to skip replaying of a commit in rebase -i, and then punish those who do not use it (and instead just remove the lines) by throwing a warning. What's the status of this one? A third commit was added (static check). I have some corrections and refactoring to do after the reviewing (and less time than before), a reroll is to be expected before next week. Rémi -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] fetch-pack: check for shallow if depth given
When a repository is first fetched as a shallow clone, either by
git-clone or by fetching into an empty repo, the server's capabilities
are not currently consulted. The client will send shallow requests even
if the server does not understand them, and the resulting error may be
unhelpful to the user. This change pre-emptively checks so we can exit
with a helpful error if necessary.
Signed-off-by: Mike Edgar ad...@google.com
---
fetch-pack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index a912935..a136772 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -809,7 +809,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args
*args,
sort_ref_list(ref, ref_compare_name);
qsort(sought, nr_sought, sizeof(*sought), cmp_ref_by_name);
- if (is_repository_shallow() !server_supports(shallow))
+ if ((args-depth 0 || is_repository_shallow())
!server_supports(shallow))
die(Server does not support shallow clients);
if (server_supports(multi_ack_detailed)) {
if (args-verbose)
--
2.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
Andres G. Aragoneses kno...@gmail.com writes: On 17/06/15 13:54, Matthieu Moy wrote: Andres G. Aragoneses kno...@gmail.com writes: On 17/06/15 12:54, Matthieu Moy wrote: Duy Nguyen pclo...@gmail.com writes: On Wed, Jun 17, 2015 at 2:54 PM, Torsten Bögershausen tbo...@web.de wrote: -git-checkout - Checkout a branch or paths to the working tree +git-checkout - Switch branches or restore changes I didn't follow closely the previous discussion. (Neither did I) Forgive me if this is already discussed, but I would keep the in the working tree. Restore changes alone seems vague. Restore previous version would be better than Restore changes to me. previous version sounds ambiguous. Yes, but git checkout can do many things. It can restore an old commited state, restore from the index, ... so we need to either be vague, or use a long enumeration. How about discard local changes? To me this describes git checkout HEAD, but neither git checkout -- file nor git checkout HEAD^^^. I didn't mean to use just discard local changes. I was proposing that as a replacement to the restore changes substring. Yes, but Switch branchs or discard local changes still does not describe git checkout HEAD^^^ -- file.txt (restore to an old state, but does not switch branch) or git checkout -- file.txt (get from the index). To me, discard local changes imply that there will be no uncommited changes on the files implied in the command after the operation. -- 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 v2 7/7] bisect: allows any terms set by user
Matthieu Moy matthieu@grenoble-inp.fr writes: # terms_defined is 0 when the user did not define the terms explicitely # yet. This is the case when running 'git bisect start bad_rev good_rev' # before we see an explicit reference to a term. terms_defined=0 The thing is: 'git bisect reset git bisect new HEAD git bisect new does not exist. Did you mean git bisect start HEAD? No I meant new but it can be 'git bisect bad' aswell So ' git bisect reset git bisect bad answer yes to autostart ? ' I don't understand. The user didn't say either bad or good, so in both cases we haven't seen a term yet. Or I misunderstood what you meant by define a term. In the case I rewrited, we saw a 'bad' but terms_defined value in bisect_start (called by the autostart) is 0. 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 05/11] ref-filter: add parse_opt_merge_filter()
karthik nayak karthik@gmail.com writes: On Tue, Jun 16, 2015 at 9:48 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: This is copied from 'builtin/branch.c' which will eventually be removed when we port 'branch.c' to use ref-filter APIs. Earlier in the series you took code from tag.c. I think you should focus on either merge or tag, get a ref-filter-based replacement that passes the tests for it, and then consider the other. The fact that the test pass for a rewritten command is important to check the correctness of the these patches. I'm not asking you to remove commits from this series though. Just impatient to see one command fully replaced (actually, I see that you have more commits than you sent in your branch, so I guess it will come soon on the list) :-). The idea is to currently get ref-filter to support all options and port it over to for-each-ref which would be the first command to completely use ref-filter. Err, for-each-ref already uses it before this series, no? So, you don't need any extra option to get for-each-ref, because it is already there. Having these extra options is a good side effect, though. To make sure I'm clear enough, what you're doing is - add all options to for-each-ref - port tag.c - port branch.c What I'm suggesting is to prioritize this way - add all options required for tag.c - port tag.c - add all options required for branch.c - port branch.c And like you said, the challenge is to then ensure tag.c and branch.c to use ref-filter and make them pass all tests. Not only the challenge, but also the way to validate your work. Think of it as a rather comprehensive set of tests that you get for free once you ported a command. BTW, talking about tests, did you do some coverage analysis on git branch and git tag? If not, I'd suggest that you do this to make sure that the pieces of code you're rewritting using ref-filter are well tested before being rewritten (a bit like Paul's work on shell - C). You don't have to actually do this before porting, but it should come befor the port in the patch series to make sure that tests pass both the old and new implementation. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/7] bisect: allows any terms set by user
Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes: Matthieu Moy matthieu@grenoble-inp.fr writes: # terms_defined is 0 when the user did not define the terms explicitely # yet. This is the case when running 'git bisect start bad_rev good_rev' # before we see an explicit reference to a term. terms_defined=0 The thing is: 'git bisect reset git bisect new HEAD git bisect new does not exist. Did you mean git bisect start HEAD? No I meant new but it can be 'git bisect bad' aswell OK, answering emails before coffee doesn't suit me well, I did not remember that the series was about new ;-). (Actually your use-case is not possible yet as of PATCH 3 which introduces start_bad_good. It is possible after PATCH 4) So ' git bisect reset git bisect bad answer yes to autostart ? ' In the case I rewrited, we saw a 'bad' but terms_defined value in bisect_start (called by the autostart) is 0. As you said, it is really equivalent to git bisect start git bisect bad the autostart is just a convenience piece of code to run git bisect start for the user, but does not change the logic of the code. Write good code for the normal case (start, and then bad), and it will just work without having to worry about in in the autostart case. -- 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 v2] git-checkout.txt: Document git checkout pathspec better
git checkout pathspec can be used to reset changes in the working tree. Signed-off-by: Torsten Bögershausen tbo...@web.de --- Version 2: Try to summarize the suggestions from the mailing list Documentation/git-checkout.txt | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index d263a56..39ad36f 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -3,7 +3,7 @@ git-checkout(1) NAME -git-checkout - Checkout a branch or paths to the working tree +git-checkout - Switch branches or restore changes SYNOPSIS @@ -89,6 +89,10 @@ Omitting branch detaches HEAD at the tip of the current branch. (i.e. commit, tag or tree) to update the index for the given paths before updating the working tree. + +'git checkout' with paths or `--patch` is used to restore modified or +deleted paths to their original contents from the index or replace paths +with the contents from a named tree-ish (most often a commit-ish). ++ The index may contain unmerged entries because of a previous failed merge. By default, if you try to check out such an entry from the index, the checkout operation will fail and nothing will be checked out. -- 2.2.0.rc1.790.ge19fcd2 -- 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] rebase -i: Fix left-behind CHERRY_PICK_HEAD
Gábor's mail reminded me that this bug bites me often enough when rebasing Git for Windows. The symptom is that .git/CHERRY_PICK_HEAD is left behind after skipping an already-merged patch with `git rebase --continue` instead of `git rebase --skip`. I always prefer the former invocation because the latter would also skip legitimate patches if there were merge conflicts, while the former would not allow that. Johannes Schindelin (2): t3404: demonstrate CHERRY_PICK_HEAD bug rebase -i: do not leave a CHERRY_PICK_HEAD file behind git-rebase--interactive.sh| 6 +- t/t3404-rebase-interactive.sh | 20 2 files changed, 25 insertions(+), 1 deletion(-) -- 2.3.1.windows.1.9.g8c01ab4 -- 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] t3404: demonstrate CHERRY_PICK_HEAD bug
When rev-list's --cherry option does not detect that a patch has already been applied upstream, an interactive rebase would offer to reapply it and consequently stop at that patch with a failure, mentioning that the diff is empty. Traditionally, a `git rebase --continue` simply skips the commit in such a situation. However, as pointed out by Gábor Szeder, this leaves a CHERRY_PICK_HEAD behind, making the Git prompt believe that a cherry pick is still going on. This commit adds a test case demonstrating this bug. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- t/t3404-rebase-interactive.sh | 20 1 file changed, 20 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index ac429a0..5d52775 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1102,4 +1102,24 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' ' test $(git cat-file commit HEAD | sed -ne \$p) = I ' +test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' ' + git checkout -b commit-to-skip + for double in X 3 1 + do + seq 5 | sed s/$double// seq + git add seq + test_tick + git commit -m seq-$double + done + git tag seq-onto + git reset --hard HEAD~2 + git cherry-pick seq-onto + test_must_fail git rebase -i seq-onto + test -d .git/rebase-merge + git rebase --continue + git diff seq-onto + test ! -d .git/rebase-merge + test ! -f .git/CHERRY_PICK_HEAD +' + test_done -- 2.3.1.windows.1.9.g8c01ab4 -- 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] rebase -i: do not leave a CHERRY_PICK_HEAD file behind
When skipping commits whose changes were already applied via `git rebase --continue`, we need to clean up said file explicitly. The same is not true for `git rebase --skip` because that will execute `git reset --hard` as part of the skip handling in git-rebase.sh, even before git-rebase--interactive.sh is called. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- git-rebase--interactive.sh| 6 +- t/t3404-rebase-interactive.sh | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index dc3133f..16e0a82 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -849,7 +849,11 @@ continue) # do we have anything to commit? if git diff-index --cached --quiet HEAD -- then - : Nothing to commit -- skip this + : Nothing to commit -- skip this commit + + test ! -f $GIT_DIR/CHERRY_PICK_HEAD || + rm $GIT_DIR/CHERRY_PICK_HEAD || + die Could not remove CHERRY_PICK_HEAD else if ! test -f $author_script then diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 5d52775..241d4d1 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1102,7 +1102,7 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' ' test $(git cat-file commit HEAD | sed -ne \$p) = I ' -test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' ' +test_expect_success 'rebase --continue removes CHERRY_PICK_HEAD' ' git checkout -b commit-to-skip for double in X 3 1 do -- 2.3.1.windows.1.9.g8c01ab4 -- 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 11/11] for-each-ref: add '--contains' option
Karthik Nayak karthik@gmail.com writes: Add the '--contains' option provided by 'ref-filter'. The '--contains' option lists only refs which are contain the mentioned commit (HEAD if s/are // -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] notes: Use get_sha1_committish instead of read_ref in init_notes()
On Tue, Jun 16, 2015 at 08:17:03PM -0700, Junio C Hamano wrote:
Mike Hommey m...@glandium.org writes:
init_notes() is essentially the only point of entry to the notes API.
It is an arbitrary restriction that all it allows as input is a strict
ref name, when callers may want to give an arbitrary committish.
While it may be a good idea to allow reading from any note-shaped
tree-ish, not just what is at the tip of a ref, I suspect that the
use of read_ref() is not an arbitrary restriction, but is an
effective way to achieve safety against callers that update notes.
That is, you can feed refs/notes/commit@{4.days.ago} to the
machinery and show you notes from 4 days ago, but you cannot update
that as if it were a ref.
Hence, if you are loosening the safety at init_notes() site, you
would at least need to add a similar safety in the write codepath, I
would think.
One thing you would need to be careful about is that you would give
users a crappy experience, if an operation reads notes, does its own
thing, and then tries to write updated notes (think: rebase that
transplants notes from original to rewritten commits), and you fail
the operation only at the very last phase of updating. In order to
prevent that, in the write codepath above has to be reject any
non-ref note, e.g. --notes=refs/notes/commit@{4.days.ago} upfront,
if the operation will write updated notes.
Looking around in the code, I'm not sure how to address this.
Considering the APIs, it would seem logical to have individual callers
care about filtering the refs themselves, on the other hand, there's
room for shooting oneself in the foot when adding new code paths needing
writes.
I'm tempted to make init_notes itself do the check, based on the value
it is given for a read_only argument. On the other hand, some commands
do their ref resolving themselves already. For example, notes_merge uses
read_ref_full and check_refname_format itself (not necessarily in the
right order, btw). OTOH, I'm not even sure it ends up using init_notes
at all.
Mike
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
On Wed, Jun 17, 2015 at 2:54 PM, Torsten Bögershausen tbo...@web.de wrote: -git-checkout - Checkout a branch or paths to the working tree +git-checkout - Switch branches or restore changes I didn't follow closely the previous discussion. Forgive me if this is already discussed, but I would keep the in the working tree. Restore changes alone seems vague. -- 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: [PATCH v2] git-checkout.txt: Document git checkout pathspec better
Duy Nguyen pclo...@gmail.com writes: On Wed, Jun 17, 2015 at 2:54 PM, Torsten Bögershausen tbo...@web.de wrote: -git-checkout - Checkout a branch or paths to the working tree +git-checkout - Switch branches or restore changes I didn't follow closely the previous discussion. (Neither did I) Forgive me if this is already discussed, but I would keep the in the working tree. Restore changes alone seems vague. Restore previous version would be better than Restore changes to me. changes sounds like the diff between a commit and its parent, so it makes sense to revert a change (git revert), but not restore a change. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/11] ref-filter: implement '--merged' and '--no-merged' options
Karthik Nayak karthik@gmail.com writes:
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -901,12 +903,19 @@ static int ref_filter_handler(const char *refname,
const struct object_id *oid,
if (!match_points_at(filter-points_at, oid-hash, refname))
return 0;
+ if (filter-merge_commit) {
+ commit = lookup_commit_reference_gently(oid-hash, 1);
+ if (!commit)
+ return 0;
+ }
I'd appreciate a comment here. If I understand correctly, the comment
could be along the lines of
/*
* A merge filter implies that we're looking at refs pointing to
* commits = discard non-commits early. The actual filtering is done
* later.
*/
(perhaps something more concise)
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] trace: add GIT_TRACE_STDIN
On Wed, Jun 17, 2015 at 4:20 AM, Jeff King p...@peff.net wrote: On Tue, Jun 16, 2015 at 03:49:07PM -0400, Jeff King wrote: Another option would be to stop trying to intercept stdin in git.c, and instead make this a feature of run-command.c. That is, right before we exec a process, tee its stdin there. That means that you cannot do: GIT_TRACE_STDIN=/tmp/foo.out git foo to collect the stdin of foo. But that is not really an interesting case anyway. You can run tee yourself if you want. The interesting cases are the ones where git is spawning a sub-process, and you want to intercept the data moving between the git processes. Hmm. I guess we do not actually have to move the stdin interception there. We can just move the config-checking there, like the patch below. It basically just converts trace.foo.bar into GIT_TRACE_BAR when we are running foo as a git command. This does work, but is perhaps potentially confusing to the user, because it only kicks in when _git_ runs foo. IOW, this works: git config trace.upload-pack.foo /path/to/foo.out git daemon I wonder if we could do it a bit differently. Instead of GIT_TRACE_STDIN, I would add GIT_TRACE_HOOK that points to a script. Whenever a command is run via run-command interface, the actual command line to be executed would be hook script command arguments. Because this script is given full command line, it can decide to trace something if the command name is matched (or arguments are matched) or just execute the original command. It's more flexible that trace.* config keys. We also have an opportunity to replace builtin commands, like pack-objects, in command pipeline in fetch or push with something else, to inject errors or whatever. It can be done manually, but it's not easy or convenient. -- 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: [PATCH v2 2/2] pull: allow dirty tree when rebase.autostash enabled
On Thu, Jun 11, 2015 at 09:34:08PM +0800, Paul Tan wrote: On Sun, Jun 7, 2015 at 5:12 AM, Kevin Daudt m...@ikke.info wrote: From: Kevin Daudt compufr...@gmail.com Signed-off-by: Kevin Daudt m...@ikke.info Ehh? The sign-off does not match the author of the patch. I changed it, but aparently forgot to reset the author for that commit ' +test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' ' + test_config branch.to-rebase.rebase true Ok, though I wonder why not just a git pull --rebase... Copied that from another test, but was doubting whether to use it or not. + test_config rebase.autostash true + git checkout HEAD -- file Why not git reset --hard before-rebase? If we don't reset HEAD, then how would we know if we actually did a rebase? Good tip, thanks. + echo dirty new_file style: echo dirty new_file Fixed + git add new_file + git pull . copy + test $(git rev-parse HEAD^) = $(git rev-parse copy) Okay, although it would be better to use test_cmp_rev HEAD^ copy because it prints out the hashes if they are different. Didn't know about that, and aparently, also not documented. Thanks. + test $(cat new_file) = dirty $(cat new_file) should be quoted to prevent field splitting. Fixed New patch is coming. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] pull: allow dirty tree when rebase.autostash enabled
rebase learned to stash changes when it encounters a dirty work tree, but
git pull --rebase does not.
Only verify if the working tree is dirty when rebase.autostash is not
enabled.
Signed-off-by: Kevin Daudt m...@ikke.info
Helped-by: Paul Tan pyoka...@gmail.com
---
Changes to v2:
- Dropped the change of the existing --rebase test
- Improvements to the test.
Verified that the test fails before the change, and succeeds after the change.
git-pull.sh | 5 -
t/t5520-pull.sh | 11 +++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/git-pull.sh b/git-pull.sh
index 0917d0d..f0a3b6e 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -239,7 +239,10 @@ test true = $rebase {
die $(gettext updating an unborn branch with changes
added to the index)
fi
else
- require_clean_work_tree pull with rebase Please commit or
stash them.
+ if [ $(git config --bool --get rebase.autostash || echo false)
= false ]
+ then
+ require_clean_work_tree pull with rebase Please
commit or stash them.
+ fi
fi
oldremoteref=
test -n $curr_branch
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index af31f04..aa247ec 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -233,6 +233,17 @@ test_expect_success '--rebase fails with multiple
branches' '
test modified = $(git show HEAD:file)
'
+test_expect_success 'pull --rebase succeeds with dirty working directory and
rebase.autostash set' '
+ test_config rebase.autostash true
+ git reset --hard before-rebase
+ echo dirty new_file
+ git add new_file
+ git pull --rebase . copy
+ test_cmp_rev HEAD^ copy
+ test $(cat new_file) = dirty
+ test $(cat file) = modified again
+'
+
test_expect_success 'pull.rebase' '
git reset --hard before-rebase
test_config pull.rebase true
--
2.4.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] notes: Use get_sha1_committish instead of read_ref in init_notes()
On Tue, Jun 16, 2015 at 11:22:31PM -0400, Jeff King wrote:
On Wed, Jun 17, 2015 at 10:15:31AM +0900, Mike Hommey wrote:
init_notes() is essentially the only point of entry to the notes API.
It is an arbitrary restriction that all it allows as input is a strict
ref name, when callers may want to give an arbitrary committish.
This has the side effect of enabling the use of committish as notes refs
in commands allowing them, e.g. git log --notes=foo@{1}, although
I haven't checked whether that's the case for all of them.
What about expand_notes_ref? We call that on the argument to --notes.
I guess it is OK to expand foo@{1} into refs/notes/foo@{1}, but what
about other more arcane syntaxes, like :/?
In a sense that is weirdly broken already:
$ git log --notes=:/foo /dev/null
warning: notes ref refs/notes/:/foo is invalid
but I wonder if we should be making expand_notes_ref a little more
careful as part of the same topic.
Interestingly, now that I look, there's also this:
https://github.com/git/git/blob/master/notes-cache.c#L40
that doesn't use expand_notes_ref, but it's apparently only used in
userdiff_get_textconv, and I'm not sure why.
Mike
--
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: stop out-of-boundary warnings from Coverity
It usually goes like this
strbuf sb = STRBUF_INIT;
if (!strncmp(sb.buf, foo, 3))
printf(%s, sb.buf + 3);
Coverity thinks that printf() can be executed, and because initial
sb.buf only has one character (from strbuf_slopbuf), sb.buf + 3 is out
of bound. What it does not recognize is strbuf_slopbuf[0] is always (*)
zero. We always do some string comparison before jumping ahead to
sb.buf + 3 and those operations will stop out of bound accesses.
Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's
happy, we'll have cleaner defect list and better chances of spotting
true defects.
(*) It's not entirely wrong though. Somebody may do sb.buf[0] = 'f'
right after variable declaration and ruin all unused strbuf.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
There are lots of false warnings like this from Coverity. I just
wanted to kill them off so we can spot more serious problems easier.
I can't really verify that this patch shuts off those warnings
because scan.coverity.com policy does not allow forks.
I had another patch that avoids corrupting strbuf_slopbuf, by putting
it to .rodata section. The patch is more invasive though, because
this statement buf.buf[buf.len] = '\0' now has to make sure buf.buf
is not strbuf_slopbuf. It feels safer, but probably not enough to
justify the change.
strbuf.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/strbuf.c b/strbuf.c
index 0d4f4e5..0d7c3cf 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -16,7 +16,12 @@ int starts_with(const char *str, const char *prefix)
* buf is non NULL and -buf is NUL terminated even for a freshly
* initialized strbuf.
*/
+#ifndef __COVERITY__
char strbuf_slopbuf[1];
+#else
+/* Stop so many incorrect out-of-boundary warnings from Coverity */
+char strbuf_slopbuf[64];
+#endif
void strbuf_init(struct strbuf *sb, size_t hint)
{
--
2.3.0.rc1.137.g477eb31
--
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] pull.c: fix some sparse warnings
On Wed, Jun 17, 2015 at 7:18 AM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: Hi Paul, If you need to re-roll your patches on the 'pt/pull-builtin' branch, could you please squash this into the patch which corresponds to commit 191241e5. Thanks. I must have been half-asleep because the block of code just above uses NULL instead of 0. Regards, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] rebase -i: do not leave a CHERRY_PICK_HEAD file behind
Hi, Quoting Johannes Schindelin johannes.schinde...@gmx.de: When skipping commits whose changes were already applied via `git rebase --continue`, we need to clean up said file explicitly. The same is not true for `git rebase --skip` because that will execute `git reset --hard` as part of the skip handling in git-rebase.sh, even before git-rebase--interactive.sh is called. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de Nice quick turnaround, thanks. So, with this change the 'git reset' won't be necessary at all, right? --- git-rebase--interactive.sh| 6 +- t/t3404-rebase-interactive.sh | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index dc3133f..16e0a82 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -849,7 +849,11 @@ continue) # do we have anything to commit? if git diff-index --cached --quiet HEAD -- then - : Nothing to commit -- skip this + : Nothing to commit -- skip this commit While at it, perhaps you could turn this into a proper comment with '#. Now that this if-branch starts to actually do something, there's no reason to continue (ab)using the null command. + + test ! -f $GIT_DIR/CHERRY_PICK_HEAD || + rm $GIT_DIR/CHERRY_PICK_HEAD || + die Could not remove CHERRY_PICK_HEAD else if ! test -f $author_script then diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 5d52775..241d4d1 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1102,7 +1102,7 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' ' test $(git cat-file commit HEAD | sed -ne \$p) = I ' -test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' ' +test_expect_success 'rebase --continue removes CHERRY_PICK_HEAD' ' git checkout -b commit-to-skip for double in X 3 1 do -- 2.3.1.windows.1.9.g8c01ab4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 01/10] t9001-send-email: move script creation in a setup test
Move the creation of the scripts used in to-cmd and cc-cmd tests in a setup test to make them available for later tests. Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr --- t/t9001-send-email.sh | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index a3663da..eef12e6 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -312,13 +312,19 @@ test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email' ) ' +test_expect_success $PREREQ 'setup tocmd and cccmd scripts' ' + write_script tocmd-sed -\EOF + sed -n -e s/^tocmd--//p $1 + EOF + write_script cccmd-sed -\EOF + sed -n -e s/^cccmd--//p $1 + EOF +' + test_expect_success $PREREQ 'tocmd works' ' clean_fake_sendmail cp $patches tocmd.patch echo tocmd--to...@example.com tocmd.patch - write_script tocmd-sed -\EOF - sed -n -e s/^tocmd--//p $1 - EOF git send-email \ --from=Example nob...@example.com \ --to-cmd=./tocmd-sed \ @@ -332,9 +338,6 @@ test_expect_success $PREREQ 'cccmd works' ' clean_fake_sendmail cp $patches cccmd.patch echo cccmd-- cc...@example.com cccmd.patch - write_script cccmd-sed -\EOF - sed -n -e s/^cccmd--//p $1 - EOF git send-email \ --from=Example nob...@example.com \ --to=nob...@example.com \ -- 1.9.1 -- 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 status -z' missing separators on OSX
Everything looks normal using the commands you described, and it does appear to
only affect status:
~/test (master)$ type git
git is hashed (/usr/local/bin/git)
~/test (master)$ git config --list
core.repositoryformatversion=0
core.filemode=true
core.bare=false
core.logallrefupdates=true
core.ignorecase=true
core.precomposeunicode=true
~/test (master)$ GIT_TRACE=1 git status -z
08:59:11.806197 git.c:348 trace: built-in: git 'status' '-z'
~/test (master)$ git log --oneline -1 -z | hexdump -C
35 31 35 39 30 65 30 20 49 6e 69 74 69 61 6c 20 |51590e0 Initial |
0010 63 6f 6d 6d 69 74 2e 00 |commit..|
0018
~/test (master)$ touch c d
~/test (master)$ git status -z | hexdump -C
3f 3f 20 63 3f 3f 20 64 |?? c?? d|
0008
This is again with 2.4.3 from git-scm.com.
N�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf
[PATCH/RFC v4 05/10] send-email: Allow use of aliases in the From field of --compose mode
Aliases were expanded before checking the From field of the
--compose option. This is inconsistent with other fields
(To, Cc, ...) which already support aliases.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
git-send-email.perl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 2d5c530..f61449d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -555,8 +555,6 @@ if (@alias_files and $aliasfiletype and defined
$parse_alias{$aliasfiletype}) {
}
}
-($sender) = expand_aliases($sender) if defined $sender;
-
# is_format_patch_arg($f) returns 0 if $f names a patch, or 1 if
# $f is a revision list specification to be passed to format-patch.
sub is_format_patch_arg {
@@ -801,6 +799,8 @@ if (!$force) {
}
}
+($sender) = expand_aliases($sender) if defined $sender;
+
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 03/10] t9001-send-email: refactor header variable fields replacement
Create a function which replaces Date, Message-Id and
X-Mailer lines generated by git-send-email by a specific string:
Date:.*$ - Date: DATE-STRING
Message-Id:.*$ - Message-Id: MESSAGE-ID-STRING
X-Mailer:.*$ - X-Mailer: X-MAILER-STRING
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
t/t9001-send-email.sh | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index f7d4132..714fcae 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -522,6 +522,12 @@ Result: OK
EOF
+replace_variable_fields () {
+ sed -e s/^\(Date:\).*/\1 DATE-STRING/ \
+ -e s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/ \
+ -e s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/
+}
+
test_suppression () {
git send-email \
--dry-run \
@@ -529,10 +535,7 @@ test_suppression () {
--from=Example f...@example.com \
--to=t...@example.com \
--smtp-server relay.example.com \
- $patches |
- sed -e s/^\(Date:\).*/\1 DATE-STRING/ \
- -e s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/ \
- -e s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/ \
+ $patches | replace_variable_fields \
actual-suppress-$1${2+-$2}
test_cmp expected-suppress-$1${2+-$2} actual-suppress-$1${2+-$2}
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 04/10] send-email: refactor address list process
Simplify code by creating a function which transform a list of strings
containing email addresses (separated by commas, comporting aliases)
into a clean list of valid email addresses.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
git-send-email.perl | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 8bf38ee..2d5c530 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -833,12 +833,9 @@ sub expand_one_alias {
return $aliases{$alias} ? expand_aliases(@{$aliases{$alias}}) : $alias;
}
-@initial_to = expand_aliases(@initial_to);
-@initial_to = validate_address_list(sanitize_address_list(@initial_to));
-@initial_cc = expand_aliases(@initial_cc);
-@initial_cc = validate_address_list(sanitize_address_list(@initial_cc));
-@bcclist = expand_aliases(@bcclist);
-@bcclist = validate_address_list(sanitize_address_list(@bcclist));
+@initial_to = process_address_list(@initial_to);
+@initial_cc = process_address_list(@initial_cc);
+@bcclist = process_address_list(@bcclist);
if ($thread !defined $initial_reply_to $prompting) {
$initial_reply_to = ask(
@@ -1051,6 +1048,13 @@ sub sanitize_address_list {
return (map { sanitize_address($_) } @_);
}
+sub process_address_list {
+ my @addr_list = expand_aliases(@_);
+ @addr_list = sanitize_address_list(@addr_list);
+ @addr_list = validate_address_list(@addr_list);
+ return @addr_list;
+}
+
# Returns the local Fully Qualified Domain Name (FQDN) if available.
#
# Tightly configured MTAa require that a caller sends a real DNS
@@ -1560,10 +1564,8 @@ foreach my $t (@files) {
($confirm =~ /^(?:auto|compose)$/ $compose $message_num
== 1));
$needs_confirm = inform if ($needs_confirm $confirm_unconfigured
@cc);
- @to = expand_aliases(@to);
- @to = validate_address_list(sanitize_address_list(@to));
- @cc = expand_aliases(@cc);
- @cc = validate_address_list(sanitize_address_list(@cc));
+ @to = process_address_list(@to);
+ @cc = process_address_list(@cc);
@to = (@initial_to, @to);
@cc = (@initial_cc, @cc);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 02/10] send-email: allow aliases in patch header and command script outputs
Interpret aliases in:
- Header fields of patches generated by git format-patch
(using --to, --cc, --add-header for example) or
manually modified. Example of fields in header:
To: alias1
Cc: alias2
Cc: alias3
- Outputs of command scripts specified by --cc-cmd and
--to-cmd. Example of script:
#!/bin/sh
echo alias1
echo alias2
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
git-send-email.perl | 2 ++
t/t9001-send-email.sh | 60 +++
2 files changed, 62 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index 6bedf74..8bf38ee 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1560,7 +1560,9 @@ foreach my $t (@files) {
($confirm =~ /^(?:auto|compose)$/ $compose $message_num
== 1));
$needs_confirm = inform if ($needs_confirm $confirm_unconfigured
@cc);
+ @to = expand_aliases(@to);
@to = validate_address_list(sanitize_address_list(@to));
+ @cc = expand_aliases(@cc);
@cc = validate_address_list(sanitize_address_list(@cc));
@to = (@initial_to, @to);
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index eef12e6..f7d4132 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1579,6 +1579,66 @@ test_expect_success $PREREQ
'sendemail.aliasfiletype=sendmail' '
grep ^!o@example\.com!$ commandline1
'
+test_expect_success $PREREQ 'alias support in To header' '
+ clean_fake_sendmail
+ echo alias sbd some...@example.org .mailrc
+ test_config sendemail.aliasesfile .mailrc
+ test_config sendemail.aliasfiletype mailrc
+ git format-patch --stdout -1 --to=sbd aliased.patch
+ git send-email \
+ --from=Example nob...@example.com \
+ --smtp-server=$(pwd)/fake.sendmail \
+ aliased.patch \
+ 2errors out
+ grep ^!someone@example\.org!$ commandline1
+'
+
+test_expect_success $PREREQ 'alias support in Cc header' '
+ clean_fake_sendmail
+ echo alias sbd some...@example.org .mailrc
+ test_config sendemail.aliasesfile .mailrc
+ test_config sendemail.aliasfiletype mailrc
+ git format-patch --stdout -1 --cc=sbd aliased.patch
+ git send-email \
+ --from=Example nob...@example.com \
+ --smtp-server=$(pwd)/fake.sendmail \
+ aliased.patch \
+ 2errors out
+ grep ^!someone@example\.org!$ commandline1
+'
+
+test_expect_success $PREREQ 'tocmd works with aliases' '
+ clean_fake_sendmail
+ echo alias sbd some...@example.org .mailrc
+ test_config sendemail.aliasesfile .mailrc
+ test_config sendemail.aliasfiletype mailrc
+ git format-patch --stdout -1 tocmd.patch
+ echo tocmd--sbd tocmd.patch
+ git send-email \
+ --from=Example nob...@example.com \
+ --to-cmd=./tocmd-sed \
+ --smtp-server=$(pwd)/fake.sendmail \
+ tocmd.patch \
+ 2errors out
+ grep ^!someone@example\.org!$ commandline1
+'
+
+test_expect_success $PREREQ 'cccmd works with aliases' '
+ clean_fake_sendmail
+ echo alias sbd some...@example.org .mailrc
+ test_config sendemail.aliasesfile .mailrc
+ test_config sendemail.aliasfiletype mailrc
+ git format-patch --stdout -1 cccmd.patch
+ echo cccmd--sbd cccmd.patch
+ git send-email \
+ --from=Example nob...@example.com \
+ --cc-cmd=./cccmd-sed \
+ --smtp-server=$(pwd)/fake.sendmail \
+ cccmd.patch \
+ 2errors out
+ grep ^!someone@example\.org!$ commandline1
+'
+
do_xmailer_test () {
expected=$1 params=$2
git format-patch -1
--
1.9.1
--
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] Documentation on git-checkout --ours/--theirs improved
2015-06-16 17:41 GMT+02:00 Junio C Hamano gits...@pobox.com: Simon Eugster simon...@gmail.com writes: 2015-06-15 22:10 GMT+02:00 Junio C Hamano gits...@pobox.com: Simon A. Eugster simon...@gmail.com writes: --- - Lack of explanation as to why this is a good thing. - Lack of sign-off. Why is there still 1/2, if its effect is wholly annulled by a subsequent step 2/2? Sorry for that, still trying to find out how git send-email works. I do not think git send-email is involved in that process in any way. The problem is you made the updates on top of the previous one, without squashing. You fed two commits, instead of a squashed one commit, to git send-email, and the command obliged and sent them out. Yes, I somehow expected the two commits would be added to the same email because I provided the Message-Id, and yes, I could just have squashed them. +During merging, we assume the role of the canonical history’s keeper, +which, in case of a rebase, is the remote history, and our private commits +look to the keeper as “their” commits which need to be integrated on top +of “our” work. ++ +Normal merging: + +local -abC -- canonical history + | git checkout --ours + v +MERGE -abC + ^ + | git checkout --theirs +origin/master ---Xyz + +Rebasing: + +local ---Abc + | git checkout --theirs + v +REBASE xyZ + ^ + | git checkout --ours +origin/master -xyZ-- canonical history + I can see that an arrow with canonical history points at different things between the two pictures, but other than that, I am not sure what these are trying to illustrate. Especially between abc and xyz, why does the former choose abc while the latter choooses xyz? Are these pictures meant to show what happens when the user says checkout --ours during a conflicted integration (whether it is a merge or a rebase)? I tried to create a picture which shows the difference of ours and theirs when merging vs. rebasing, but apparently it did not turn out well, and I will just leave it away. I'll wait for several days to see what other people would say, if they care to comment on this. Maybe they can come up with a more intuitive picture, or maybe they say textual description is sufficiently clear that we do not need an illustration. I dunno. A better picture would be nice. And regarding the textual description, are you going to commit your version? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 10/10] send-email: suppress meaningless whitespaces in from field
Remove leading and trailing whitespaces in from field before
interepreting it to improve consistency with other options. The
split_addrs function already take care of trailing and leading
whitespaces for to, cc and bcc fields.
The from option now:
- has the same behavior when passing arguments like
j...@example.com , \t j...@example.com or
j...@example.com.
- interprets aliases in string containing leading and trailing
whitespaces such as alias or alias\t like other options.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
git-send-email.perl | 1 +
t/t9001-send-email.sh | 24
2 files changed, 25 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index 265299e..9b28dfa 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -836,6 +836,7 @@ if (!$force) {
}
if (defined $sender) {
+ $sender =~ s/^\s+|\s$//g;
($sender) = expand_aliases($sender);
} else {
$sender = $repoauthor || $repocommitter || '';
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 3c5b853..8e21fb0 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1719,4 +1719,28 @@ test_expect_success $PREREQ 'aliases work with email
list' '
test_cmp expected-list actual-list
'
+test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '
+ echo alias to2 t...@example.com .mutt
+ echo alias cc1 Cc 1 c...@example.com .mutt
+ test_config sendemail.aliasesfile .mutt
+ test_config sendemail.aliasfiletype mutt
+ TO1=$(echo QTo 1 t...@example.com | q_to_tab)
+ TO2=$(echo QZto2 | qz_to_tab_space)
+ CC1=$(echo cc1 | append_cr)
+ BCC1=$(echo Q b...@example.com Q | q_to_nul)
+ git send-email \
+ --dry-run \
+ --from=Example f...@example.com \
+ --to=$TO1 \
+ --to=$TO2 \
+ --to= t...@example.com\
+ --cc=$CC1 \
+ --cc=Cc2 c...@example.com \
+ --bcc=$BCC1 \
+ --bcc=b...@example.com \
+ 0001-add-master.patch | replace_variable_fields \
+ actual-list
+ test_cmp expected-list actual-list
+'
+
test_done
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 09/10] send-email: allow multiple emails using --cc, --to and --bcc
Accept a list of emails separated by commas in flags --cc, --to and
--bcc. Multiple addresses can already be given by using these options
multiple times, but it is more convenient to allow cutting-and-pasting
a list of addresses from the header of an existing e-mail message,
which already lists them as comma-separated list, as a value to a
single parameter.
The following format can now be used:
$ git send-email --to='Jane j...@example.com, m...@example.com'
Remove the limitation imposed by 79ee555b (Check and document the
options to prevent mistakes, 2006-06-21) which rejected every argument
with comma in --cc, --to and --bcc.
Helped-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
Documentation/git-send-email.txt | 12 +--
git-send-email.perl | 17 ++--
t/t9001-send-email.sh| 44
3 files changed, 52 insertions(+), 21 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index b48a764..afd9569 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -49,17 +49,17 @@ Composing
of 'sendemail.annotate'. See the CONFIGURATION section for
'sendemail.multiEdit'.
---bcc=address::
+--bcc=address,...::
Specify a Bcc: value for each email. Default is the value of
'sendemail.bcc'.
+
-The --bcc option must be repeated for each user you want on the bcc list.
+This option may be specified multiple times.
---cc=address::
+--cc=address,...::
Specify a starting Cc: value for each email.
Default is the value of 'sendemail.cc'.
+
-The --cc option must be repeated for each user you want on the cc list.
+This option may be specified multiple times.
--compose::
Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
@@ -110,13 +110,13 @@ is not set, this will be prompted for.
Only necessary if --compose is also set. If --compose
is not set, this will be prompted for.
---to=address::
+--to=address,...::
Specify the primary recipient of the emails generated. Generally, this
will be the upstream maintainer of the project involved. Default is the
value of the 'sendemail.to' configuration value; if that is unspecified,
and --to-cmd is not specified, this will be prompted for.
+
-The --to option must be repeated for each user you want on the to list.
+This option may be specified multiple times.
--8bit-encoding=encoding::
When encountering a non-ASCII message or subject that does not
diff --git a/git-send-email.perl b/git-send-email.perl
index 8594ab9..265299e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -460,20 +460,6 @@ my ($repoauthor, $repocommitter);
($repoauthor) = Git::ident_person(@repo, 'author');
($repocommitter) = Git::ident_person(@repo, 'committer');
-# Verify the user input
-
-foreach my $entry (@initial_to) {
- die Comma in --to entry: $entry'\n unless $entry !~ m/,/;
-}
-
-foreach my $entry (@initial_cc) {
- die Comma in --cc entry: $entry'\n unless $entry !~ m/,/;
-}
-
-foreach my $entry (@bcclist) {
- die Comma in --bcclist entry: $entry'\n unless $entry !~ m/,/;
-}
-
sub parse_address_line {
if ($have_mail_address) {
return map { $_-format } Mail::Address-parse($_[0]);
@@ -1101,7 +1087,8 @@ sub sanitize_address_list {
}
sub process_address_list {
- my @addr_list = expand_aliases(@_);
+ my @addr_list = map { parse_address_line($_) } @_;
+ @addr_list = expand_aliases(@addr_list);
@addr_list = sanitize_address_list(@addr_list);
@addr_list = validate_address_list(@addr_list);
return @addr_list;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 714fcae..3c5b853 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1675,4 +1675,48 @@ test_expect_success $PREREQ '--[no-]xmailer with
sendemail.xmailer=false' '
do_xmailer_test 1 --xmailer
'
+test_expect_success $PREREQ 'setup expected-list' '
+ git send-email \
+ --dry-run \
+ --from=Example f...@example.com \
+ --to=To 1 t...@example.com \
+ --to=t...@example.com \
+ --to=t...@example.com \
+ --cc=Cc 1 c...@example.com \
+ --cc=Cc2 c...@example.com \
+ --bcc=b...@example.com \
+ --bcc=b...@example.com \
+ 0001-add-master.patch | replace_variable_fields \
+ expected-list
+'
+
+test_expect_success $PREREQ 'use email list in --cc --to and --bcc' '
+ git send-email \
+ --dry-run \
+ --from=Example f...@example.com \
+ --to=To 1 t...@example.com,
[PATCH/RFC v4 08/10] send-email: consider quote as delimiter instead of character
Do not consider quote inside a recipient name as character when
they are not escaped. This interprets:
Jane Doe j...@example.com
as:
Jane Doe j...@example.com
instead of:
Jane\ \Doe j...@example.com
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
I don't know if it's an argument for this change, but rfc2822 says:
Semantically, neither the optional CFWS outside of the quote
characters nor the quote characters themselves are part of the
quoted-string...
git-send-email.perl | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index a1f6c18..8594ab9 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1078,15 +1078,17 @@ sub sanitize_address {
return $recipient;
}
+ # remove non-escaped quotes
+ $recipient_name =~ s/(^|[^\\])/$1/g;
+
# rfc2047 is needed if a non-ascii char is included
if ($recipient_name =~ /[^[:ascii:]]/) {
- $recipient_name =~ s/^(.*)$/$1/;
$recipient_name = quote_rfc2047($recipient_name);
}
# double quotes are needed if specials or CTLs are included
elsif ($recipient_name =~ /[][()@,;:\\.\000-\037\177]/) {
- $recipient_name =~ s/([\\\r])/\\$1/g;
+ $recipient_name =~ s/([\\\r])/\\$1/g;
$recipient_name = qq[$recipient_name];
}
--
1.9.1
--
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
Fwd: New Defects reported by Coverity Scan for git
I think Coverity caught this correctly.
** CID 1306846: Memory - illegal accesses (USE_AFTER_FREE)
/builtin/pull.c: 287 in config_get_rebase()
*** CID 1306846: Memory - illegal accesses (USE_AFTER_FREE)
/builtin/pull.c: 287 in config_get_rebase()
281
282 if (curr_branch) {
283 char *key = xstrfmt(branch.%s.rebase,
curr_branch-name);
284
285 if (!git_config_get_value(key, value)) {
286 free(key);
CID 1306846: Memory - illegal accesses (USE_AFTER_FREE)
Passing freed pointer key as an argument to parse_config_rebase.
287 return parse_config_rebase(key, value, 1);
288 }
289
290 free(key);
291 }
292
--
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
[PATCH/RFC v4 06/10] send-email: minor code refactoring
Group expressions in a single if statement. This avoid checking
multiple time if the variable $sender is defined.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
git-send-email.perl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index f61449d..a0cd7ff 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -799,9 +799,9 @@ if (!$force) {
}
}
-($sender) = expand_aliases($sender) if defined $sender;
-
-if (!defined $sender) {
+if (defined $sender) {
+ ($sender) = expand_aliases($sender);
+} else {
$sender = $repoauthor || $repocommitter || '';
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v4 07/10] send-email: reduce dependancies impact on parse_address_line
parse_address_line had not the same behavior whether the user had
Mail::Address or not. Teach parse_address_line to behave like
Mail::Address.
When the user input is correct, this implementation behaves
exactly like Mail::Address except when there are quotes
inside the name:
Jane Doe j...@example.com
In this case the result of parse_address_line is:
With M::A : Jane Do e j...@example.com
Without : Jane Do e j...@example.com
When the user input is not correct, the behavior is also mostly
the same.
Unlike Mail::Address, this doesn't parse groups and recursive
commentaries.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
---
git-send-email.perl | 54 +++--
1 file changed, 52 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index a0cd7ff..a1f6c18 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -477,9 +477,59 @@ foreach my $entry (@bcclist) {
sub parse_address_line {
if ($have_mail_address) {
return map { $_-format } Mail::Address-parse($_[0]);
- } else {
- return split_addrs($_[0]);
}
+
+ my $commentrgx=qr/\((?:[^)]*)\)/;
+ my $quotergx=qr/(?:[^\\\]|\\.)*/;
+ my $wordrgx=qr/(?:[^][\s():;@\\,.]|\\.)+/;
+ my $tokenrgx = qr/(?:$quotergx|$wordrgx|$commentrgx|\S)/;
+
+ my @tokens = map { $_ =~ /\s*($tokenrgx)\s*/g } @_;
+ push @tokens, ,;
+
+ my (@addr_list, @phrase, @address, @comment, @buffer) = ();
+ foreach my $token (@tokens) {
+ if ($token =~ /^[,;]$/) {
+ if (@address) {
+ push @address, @buffer;
+ } else {
+ push @phrase, @buffer;
+ }
+
+ my $str_phrase = join ' ', @phrase;
+ my $str_address = join '', @address;
+ my $str_comment = join ' ', @comment;
+
+ if ($str_phrase =~ /[][():;@\\,.\000-\037\177]/) {
+ $str_phrase =~ s/(^|[^\\])/$1/g;
+ $str_phrase = qq[$str_phrase];
+ }
+
+ if ($str_address ne $str_phrase ne ) {
+ $str_address = qq[$str_address];
+ }
+
+ my $str_mailbox = $str_phrase $str_address $str_comment;
+ $str_mailbox =~ s/^\s*|\s*$//g;
+ push @addr_list, $str_mailbox if ($str_mailbox);
+
+ @phrase = @address = @comment = @buffer = ();
+ } elsif ($token =~ /^\(/) {
+ push @comment, $token;
+ } elsif ($token eq ) {
+ push @phrase, (splice @address), (splice @buffer);
+ } elsif ($token eq ) {
+ push @address, (splice @buffer);
+ } elsif ($token eq @) {
+ push @address, (splice @buffer), @;
+ } elsif ($token eq .) {
+ push @address, (splice @buffer), .;
+ } else {
+ push @buffer, $token;
+ }
+ }
+
+ return @addr_list;
}
sub split_addrs {
--
1.9.1
--
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: New Defects reported by Coverity Scan for git
On Wed, Jun 17, 2015 at 9:54 PM, Duy Nguyen pclo...@gmail.com wrote:
I think Coverity caught this correctly.
** CID 1306846: Memory - illegal accesses (USE_AFTER_FREE)
/builtin/pull.c: 287 in config_get_rebase()
*** CID 1306846: Memory - illegal accesses (USE_AFTER_FREE)
/builtin/pull.c: 287 in config_get_rebase()
281
282 if (curr_branch) {
283 char *key = xstrfmt(branch.%s.rebase,
curr_branch-name);
284
285 if (!git_config_get_value(key, value)) {
286 free(key);
CID 1306846: Memory - illegal accesses (USE_AFTER_FREE)
Passing freed pointer key as an argument to parse_config_rebase.
287 return parse_config_rebase(key, value, 1);
288 }
289
290 free(key);
291 }
292
Ugh, thanks.
Regards,
Paul
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
