Git development
 help / color / mirror / Atom feed
* Re: What's cooking in git.git (Nov 2012, #06; Mon, 19)
From: Felipe Contreras @ 2012-11-20  1:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Sverre Rabbelier
In-Reply-To: <7vpq39up0m.fsf@alter.siamese.dyndns.org>

On Tue, Nov 20, 2012 at 12:55 AM, Junio C Hamano <gitster@pobox.com> wrote:

> * fc/remote-testgit-feature-done (2012-10-29) 1 commit
>  - remote-testgit: properly check for errors

Pinging Sverre again.

I now think that we need a better solution with waitpid() to check the
status of the process, so that both import and export get proper error
checks. I found that out the hard way with a buggy remote helper.

I still think this patch is good regardless, but not so big of a
priority. I'm not going to purse it more, if it gets in, good, if not,
a waitpid() patch would take care of it.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: W. Trevor King @ 2012-11-20  1:16 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Heiko Voigt, Git, Jeff King, Phil Hord, Shawn Pearce,
	Jens Lehmann, Nahor
In-Reply-To: <7vd2z9t7y2.fsf@alter.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 2806 bytes --]

On Mon, Nov 19, 2012 at 04:49:09PM -0800, Junio C Hamano wrote:
> "W. Trevor King" <wking@tremily.us> writes:
> 
> >> From what I have heard of projects using this: They usually still have
> >> something that records the SHA1s on a regular basis. Thinking further,
> >> why not record them in git? We could add an option to update which
> >> creates such a commit.
> >
> > I think it's best to have users craft their own commit messages
> > explaining why the branch was updated.  That said, an auto-generated
> > hint (a la "git merge") would probably be a useful extra feature.
> 
> I am not quite sure I agree.  When the project says "Use the tip of
> 'bar' branch for the submodule 'foo'" at the top-level, does an
> individual user who is not working on the submodule 'foo' but merely
> is using it have any clue as to why the submodule's 'foo' branch
> 'foo' moved, or does he necessarily even care?

If he doesn't care, why is he updating the submodule gitlink?

> Now, since somebody created the top-level commit you have just
> pulled and checked out, other people may have worked on submodule
> 'foo' [*1*].  What should happen on "git submodule update foo"?

If the 'foo' checkout is not the one listed in the superproject's
.gitmodules, the update should bail with an appropriate error message,
and let the user sort things out.

  $ git submodule update --pull foo
  error: Your local changes to the following submodule would be
  overwritten by update:…

This is similar to how Git currently bails on dirty-tree branch
switches:

  $ git checkout my-branch
  error: Your local changes to the following files would be
  overwritten by checkout:…

Without "--pull", the update command is intended to checkout the hash
specified in .gitmodules.  If you've committed some local work in foo
and then explicitly ask for an update, I suppose you get clobbered.

> What should appear in "git diff"?  The working tree taken as a whole
> is different from what the superproject's commit describes (which is
> the state the person who created the superproject wanted to record)
> even though this user does not have anything to do with the change
> at 'foo' from the recorded commit to the current tip of 'bar'.  What
> would his description for the reason why the branch was updated?

The submodule content is not part of the superproject.  All the
superproject has is a gitlink.  If the gitlink hasn't changed, "git
diff" in the superproject shouldn't say anything.

I'll probably have time to write up v4 over the weekend.  Maybe having
a more explicit example will clear things up.

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: Junio C Hamano @ 2012-11-20  0:49 UTC (permalink / raw)
  To: W. Trevor King
  Cc: Heiko Voigt, Git, Jeff King, Phil Hord, Shawn Pearce,
	Jens Lehmann, Nahor
In-Reply-To: <20121117192026.GI22234@odin.tremily.us>

"W. Trevor King" <wking@tremily.us> writes:

>> From what I have heard of projects using this: They usually still have
>> something that records the SHA1s on a regular basis. Thinking further,
>> why not record them in git? We could add an option to update which
>> creates such a commit.
>
> I think it's best to have users craft their own commit messages
> explaining why the branch was updated.  That said, an auto-generated
> hint (a la "git merge") would probably be a useful extra feature.

I am not quite sure I agree.  When the project says "Use the tip of
'bar' branch for the submodule 'foo'" at the top-level, does an
individual user who is not working on the submodule 'foo' but merely
is using it have any clue as to why the submodule's 'foo' branch
'foo' moved, or does he necessarily even care?

For such a user working at the top-level superproject, or working on
one part of the project, possibly on a submodule other than 'foo',
wouldn't the natural thing to do would be to run "git pull" at the
top-level, maybe with "--recursive" to update the top-level and all
the submodules to start the day.

Now, since somebody created the top-level commit you have just
pulled and checked out, other people may have worked on submodule
'foo' [*1*].  What should happen on "git submodule update foo"?  It
would notice that the submodule 'foo' is set to float, and would
check out the tip of the branch 'bar', not the commit recorded in
the top-level superproject, in the working tree for 'foo', no?

What should appear in "git diff"?  The working tree taken as a whole
is different from what the superproject's commit describes (which is
the state the person who created the superproject wanted to record)
even though this user does not have anything to do with the change
at 'foo' from the recorded commit to the current tip of 'bar'.  What
would his description for the reason why the branch was updated?

I think I would agree that "git diff" should not hide such changes
(after all, when this user records his change to the overall project
in the top-level supermodule, he will be recording the state with
the commit at the tip of 'bar' checked out in the working tree of
the submodule 'foo'), but I am not sure if the user can say anything
sensible, other than "tip of 'bar' branch in submodule 'foo' was
changed by others", in the resulting commit.


[Footnote]

*1* This may look like a non-issue if you assume that the person who
updates the 'bar' branch of submodule 'foo' always updates the
gitlink in the superproject's commit to point at that updated
commit, but that assumption is flawed; the submodule project is a
project on its own and can be worked on without what other projects
bind it as their submodules.

^ permalink raw reply

* Re: [PATCH v4 2/4] diff: introduce diff.submodule configuration variable
From: Junio C Hamano @ 2012-11-20  0:48 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <1352821367-3611-3-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> index 6c01d0c..e401814 100755
> --- a/t/t4041-diff-submodule-option.sh
> +++ b/t/t4041-diff-submodule-option.sh
> @@ -33,6 +33,7 @@ test_create_repo sm1 &&
>  add_file . foo >/dev/null
>  
>  head1=$(add_file sm1 foo1 foo2)
> +fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)

That looks like quite a roundabout way to say

	$(cd sm1; git rev-parse --verify HEAD)

doesn't it?  I know this is just moved code from the original, so I
am not saying this should be fixed in this commit.

Existing code in t7401 may want to be cleaned up, perhaps after this
topic settles.  The add_file() function, for example, has at least
these points:

 - do we know 7 hexdigits is always the right precision?
 - what happens when it fails to create a commit in one of the steps
   while looping over "$@" in its loop?
 - the function uses the "cd there and then come back", not
   "go there in a subshell and do whatever it needs to" pattern.

> +test_expect_success 'added submodule, set diff.submodule' "

s/added/add/;

Shouldn't it test the base case where no diff.submodule is set?  For
those people, the patch should not change the output when they do or
do not pass --submodule option, right?

> +	git config diff.submodule log &&
> +	git add sm1 &&
> +	git diff --cached >actual &&
> +	cat >expected <<-EOF &&
> +Submodule sm1 0000000...$head1 (new submodule)
> +EOF
> +	git config --unset diff.submodule &&
> +	test_cmp expected actual
> +"
> +
> +test_expect_success '--submodule=short overrides diff.submodule' "
> +	git config diff.submodule log &&
> +	git add sm1 &&
> +	git diff --submodule=short --cached >actual &&
> +	cat >expected <<-EOF &&
> +diff --git a/sm1 b/sm1
> +new file mode 160000
> +index 0000000..a2c4dab
> +--- /dev/null
> ++++ b/sm1
> +@@ -0,0 +1 @@
> ++Subproject commit $fullhead1
> +EOF
> +	git config --unset diff.submodule &&
> +	test_cmp expected actual
> +"
> +
>  commit_file sm1 &&
>  head2=$(add_file sm1 foo3)
>  
> @@ -73,7 +102,6 @@ EOF
>  	test_cmp expected actual
>  "
>  
> -fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)
>  fullhead2=$(cd sm1; git rev-list --max-count=1 $head2)
>  test_expect_success 'modified submodule(forward) --submodule=short' "
>  	git diff --submodule=short >actual &&

^ permalink raw reply

* Re: [PATCH 2/2] pickaxe: use textconv for -S counting
From: Junio C Hamano @ 2012-11-20  0:48 UTC (permalink / raw)
  To: Jeff King; +Cc: Peter Oberndorfer, git
In-Reply-To: <7v3905uncf.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

>> Exact renames are the obvious one, but they are not handled here.
>
> That is half true.  Before this change, we will find the same number
> of needles and this function would have said "no differences" in a
> very inefficient way.  After this change, we may apply different
> textconv filters and this function will say "there is a difference",
> even though we wouldn't see such a difference at the content level
> if there wasn't any rename.

... but I think that is a good thing anyway.

If you renamed foo.c to foo.cc with different conversions from C
code to the text that explain what the code does, if we special case
only the exact rename case but let pickaxe examine the converted
result in a case where blobs are modified only by one byte, we would
get drastically different results between the two cases.

Corollary to this is what should happen when you update the attributes
between two trees so that textconv for a path that did not change
between preimage and postimage are different.  Ideally, we should
notice that the two converted result are different, perhaps, but I
do not like the performance implications very much.

^ permalink raw reply

* Re: [PATCH 2/2] pickaxe: use textconv for -S counting
From: Junio C Hamano @ 2012-11-20  0:31 UTC (permalink / raw)
  To: Jeff King; +Cc: Peter Oberndorfer, git
In-Reply-To: <20121115012131.GA17894@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Nov 13, 2012 at 03:13:19PM -0800, Junio C Hamano wrote:
>
>> >  static int has_changes(struct diff_filepair *p, struct diff_options *o,
>> >  		       regex_t *regexp, kwset_t kws)
>> >  {
>> > +	struct userdiff_driver *textconv_one = get_textconv(p->one);
>> > +	struct userdiff_driver *textconv_two = get_textconv(p->two);
>> > +	mmfile_t mf1, mf2;
>> > +	int ret;
>> > +
>> >  	if (!o->pickaxe[0])
>> >  		return 0;
>> >  
>> > -	if (!DIFF_FILE_VALID(p->one)) {
>> > -		if (!DIFF_FILE_VALID(p->two))
>> > -			return 0; /* ignore unmerged */
>> 
>> What happened to this part that avoids showing nonsense for unmerged
>> paths?
>
> It's moved down. fill_one will return an empty mmfile if
> !DIFF_FILE_VALID, so we end up here:
>
>         fill_one(p->one, &mf1, &textconv_one);
>         fill_one(p->two, &mf2, &textconv_two);
>
>         if (!mf1.ptr) {
>                 if (!mf2.ptr)
>                         ret = 0; /* ignore unmerged */
>
> Prior to this change, we didn't use fill_one, so we had to check manually.
>
>> > +	/*
>> > +	 * If we have an unmodified pair, we know that the count will be the
>> > +	 * same and don't even have to load the blobs. Unless textconv is in
>> > +	 * play, _and_ we are using two different textconv filters (e.g.,
>> > +	 * because a pair is an exact rename with different textconv attributes
>> > +	 * for each side, which might generate different content).
>> > +	 */
>> > +	if (textconv_one == textconv_two && diff_unmodified_pair(p))
>> > +		return 0;
>> 
>> I am not sure about this part that cares about the textconv.
>> 
>> Wouldn't the normal "git diff A B" skip the filepair that are
>> unmodified in the first place at the object name level without even
>> looking at the contents (see e.g. diff_flush_patch())?
>
> Hmph. The point was to find the case when the paths are different (e.g.,
> in a rename), and therefore the textconvs might be different. But I
> think I missed the fact that diff_unmodified_pair will note the
> difference in paths. So just calling diff_unmodified_pair would be
> sufficient, as the code prior to my patch does.
>
> I thought the point was an optimization to avoid comparing contains() on
> the same data (which we can know will match without looking at it).

Yes.

> Exact renames are the obvious one, but they are not handled here.

That is half true.  Before this change, we will find the same number
of needles and this function would have said "no differences" in a
very inefficient way.  After this change, we may apply different
textconv filters and this function will say "there is a difference",
even though we wouldn't see such a difference at the content level
if there wasn't any rename.

^ permalink raw reply

* [PATCH] config --get-all: it is an error not to see any value
From: Junio C Hamano @ 2012-11-20  0:24 UTC (permalink / raw)
  To: git; +Cc: Timur Tabi
In-Reply-To: <7vd2z9uobn.fsf@alter.siamese.dyndns.org>

The wording "... is not exactly one." incorrectly hinted as if
having no value is not an error, but that was not what was
intended.  Clarify what similarity it wants to mention by
elaborating the "Like get" part of the description.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-config.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git i/Documentation/git-config.txt w/Documentation/git-config.txt
index eaea079..273239d 100644
--- i/Documentation/git-config.txt
+++ w/Documentation/git-config.txt
@@ -85,8 +85,9 @@ OPTIONS
 	found and error code 2 if multiple key values were found.
 
 --get-all::
-	Like get, but does not fail if the number of values for the key
-	is not exactly one.
+	Like `--get`, shows the value(s) given to the key, and signals an
+	error if there is no such key.  Unlike `--get`, does not
+	fail if there are multiple values defined for the key.
 
 --get-regexp::
 	Like --get-all, but interprets the name as a regular expression and

^ permalink raw reply related

* Re: git config --git-all can return non-zero error code
From: Junio C Hamano @ 2012-11-20  0:10 UTC (permalink / raw)
  To: Timur Tabi; +Cc: git
In-Reply-To: <50AABA9B.6090007@freescale.com>

Timur Tabi <timur@freescale.com> writes:

> According to the man page for git-config, the --git-all option is not
> supposed to return an error code:
>
> --get-all
> 	Like get, but does not fail if the number of values for the key is
> 	not exactly one.
>
> IMHO, zero is also "not exactly one",...

It should have stated "like get, but unlike get, it does not fail
when there are multiple values for the key", but the documentation
was written with that specific knowledge that "--get" has a logic to
make it fail when there are ambiguities, and wanted to stress that
difference.  It forgot to mention their similarity explicitly and
relied on "Like get" part to mean (1) shows the value(s) given to
the key, and (2) it is an error if there is no such key defined.

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Junio C Hamano @ 2012-11-20  0:00 UTC (permalink / raw)
  To: Krzysztof Mazur; +Cc: Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <20121119225838.GA23412@shrek.podlesie.net>

Krzysztof Mazur <krzysiek@podlesie.net> writes:

> On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:
>> Given that the problematic line
>> 
>> 	Stable Kernel Maintainance Track <stable@vger.kernel.org> # vX.Y
>> 
>> is not even a valid e-mail address, doesn't this new logic belong to
>> sanitize_address() conceptually?
>
> Yes, it's much better to do it in the sanitize_address().

Note that I did not check that all the addresses that are handled by
extract-valid-address came through sanitize-address function, so
unlike your original patch, this change alone may still pass some
garbage to Email::Valid->address().  I tend to think that is a
progress; we should make sure all the addresses are sanitized before
using them for sending messages out.

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Junio C Hamano @ 2012-11-19 23:57 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Krzysztof Mazur, Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <CAMP44s0f0zYa1FVf9RhNuwYJbkQ7zPwgJ6=ty3c5knjo5a2TNw@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:
>
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -924,6 +924,10 @@ sub quote_subject {
>>  # use the simplest quoting being able to handle the recipient
>>  sub sanitize_address {
>>         my ($recipient) = @_;
>> +
>> +       # remove garbage after email address
>> +       $recipient =~ s/(.*>).*$/$1/;
>> +
>
> Looks fine, but I would do s/(.*?>)(.*)$/$1/, so that 'test
> <foo@bar.com> <#comment>' gets the second comment removed.

Yeah, but do you need to capture the second group?  IOW, like
"s/(.*?>).*$/$1/" perhaps?

^ permalink raw reply

* What's cooking in git.git (Nov 2012, #06; Mon, 19)
From: Junio C Hamano @ 2012-11-19 23:55 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

Bunch of topics have been merged to 'next'.

We are at the beginning of the 5th week of this release cycle
(cf. http://tinyurl.com/gitcal), and I've moved many topics to the
Stalled category, which will be discarded without prejudice soonish
unless there are some updates.  I am still a bit behind on some
topics and already posted rerolls may have to be pulled in.

You can find the changes described here in the integration branches of the
repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

--------------------------------------------------
[New Topics]

* nd/pathspec-wildcard (2012-11-19) 4 commits
 - tree_entry_interesting: do basedir compare on wildcard patterns when possible
 - pathspec: apply "*.c" optimization from exclude
 - pathspec: do exact comparison on the leading non-wildcard part
 - pathspec: save the non-wildcard length part

--------------------------------------------------
[Stalled]

* pf/editor-ignore-sigint (2012-11-11) 5 commits
 - launch_editor: propagate SIGINT from editor to git
 - run-command: do not warn about child death by SIGINT
 - run-command: drop silent_exec_failure arg from wait_or_whine
 - launch_editor: ignore SIGINT while the editor has control
 - launch_editor: refactor to use start/finish_command

 Avoid confusing cases where the user hits Ctrl-C while in the editor
 session, not realizing git will receive the signal. Since most editors
 will take over the terminal and will block SIGINT, this is not likely
 to confuse anyone.

 Some people raised issues with emacsclient, which are addressed by this
 re-roll. It should probably also handle SIGQUIT, and there were a
 handful of other review comments.

 Expecting a re-roll.


* pp/gitweb-config-underscore (2012-11-08) 1 commit
 - gitweb: make remote_heads config setting work

 The key "gitweb.remote_heads" is not legal git config; this maps it to
 "gitweb.remoteheads".

 Junio raised a good point about the implementation for three-level
 variables.

 Expecting a re-roll.


* mo/cvs-server-updates (2012-10-16) 10 commits
 - cvsserver Documentation: new cvs ... -r support
 - cvsserver: add t9402 to test branch and tag refs
 - cvsserver: support -r and sticky tags for most operations
 - cvsserver: Add version awareness to argsfromdir
 - cvsserver: generalize getmeta() to recognize commit refs
 - cvsserver: implement req_Sticky and related utilities
 - cvsserver: add misc commit lookup, file meta data, and file listing functions
 - cvsserver: define a tag name character escape mechanism
 - cvsserver: cleanup extra slashes in filename arguments
 - cvsserver: factor out git-log parsing logic

 Needs review by folks interested in cvsserver.


* jc/apply-trailing-blank-removal (2012-10-12) 1 commit
 - apply.c:update_pre_post_images(): the preimage can be truncated

 Fix to update_pre_post_images() that did not take into account the
 possibility that whitespace fix could shrink the preimage and
 change the number of lines in it.

 Extra set of eyeballs appreciated.


* jn/warn-on-inaccessible-loosen (2012-10-14) 4 commits
 - config: exit on error accessing any config file
 - doc: advertise GIT_CONFIG_NOSYSTEM
 - config: treat user and xdg config permission problems as errors
 - config, gitignore: failure to access with ENOTDIR is ok

 An RFC to deal with a situation where .config/git is a file and we
 notice .config/git/config is not readable due to ENOTDIR, not
 ENOENT; I think a bit more refactored approach to consistently
 address permission errors across config, exclude and attrs is
 desirable.  Don't we also need a check for an opposite situation
 where we open .config/git/config or .gitattributes for reading but
 they turn out to be directories?


* as/check-ignore (2012-11-08) 14 commits
 - t0007: fix tests on Windows
 - Documentation/check-ignore: we show the deciding match, not the first
 - Add git-check-ignore sub-command
 - dir.c: provide free_directory() for reclaiming dir_struct memory
 - pathspec.c: move reusable code from builtin/add.c
 - dir.c: refactor treat_gitlinks()
 - dir.c: keep track of where patterns came from
 - dir.c: refactor is_path_excluded()
 - dir.c: refactor is_excluded()
 - dir.c: refactor is_excluded_from_list()
 - dir.c: rename excluded() to is_excluded()
 - dir.c: rename excluded_from_list() to is_excluded_from_list()
 - dir.c: rename path_excluded() to is_path_excluded()
 - dir.c: rename cryptic 'which' variable to more consistent name

 Duy helped to reroll this.

 Expecting a re-roll.


* aw/rebase-am-failure-detection (2012-10-11) 1 commit
 - rebase: Handle cases where format-patch fails

 I am unhappy a bit about the possible performance implications of
 having to store the output in a temporary file only for a rare case
 of format-patch aborting.


* jk/lua-hackery (2012-10-07) 6 commits
 - pretty: fix up one-off format_commit_message calls
 - Minimum compilation fixup
 - Makefile: make "lua" a bit more configurable
 - add a "lua" pretty format
 - add basic lua infrastructure
 - pretty: make some commit-parsing helpers more public

 Interesting exercise. When we do this for real, we probably would want
 to wrap a commit to make it more like an "object" with methods like
 "parents", etc.


* fc/remote-testgit-feature-done (2012-10-29) 1 commit
 - remote-testgit: properly check for errors

 Is this still in "Needs review" state?  Are poeple involved in the
 remote interface happy with this change?


* jk/send-email-sender-prompt (2012-11-15) 8 commits
 - send-email: do not prompt for explicit repo ident
 - Git.pm: teach "ident" to query explicitness
 - var: provide explicit/implicit ident information
 - var: accept multiple variables on the command line
 - ident: keep separate "explicit" flags for author and committer
 - ident: make user_ident_explicitly_given static
 - t7502: factor out autoident prerequisite
 - test-lib: allow negation of prerequisites

 Avoid annoying sender prompt in git-send-email, but only when it is
 safe to do so.

 Perhaps keep only the first three patches, and replace the rest
 with the one from Felipe that takes a much simpler approach (the
 rationale of that patch needs to be cleaned up first, along the
 lines Jeff outlined, though).  Frozen until that happens.


* nd/unify-appending-of-s-o-b (2012-11-15) 1 commit
 - Unify appending signoff in format-patch, commit and sequencer

 I am not sure if the logic to refrain from adding a sign-off based
 on the existing run of sign-offs is done correctly in this change.


* nd/pretty-placeholder-with-color-option (2012-09-30) 9 commits
 . pretty: support %>> that steal trailing spaces
 . pretty: support truncating in %>, %< and %><
 . pretty: support padding placeholders, %< %> and %><
 . pretty: two phase conversion for non utf-8 commits
 . utf8.c: add utf8_strnwidth() with the ability to skip ansi sequences
 . utf8.c: move display_mode_esc_sequence_len() for use by other functions
 . pretty: support %C(auto[,N]) to turn on coloring on next placeholder(s)
 . pretty: split parsing %C into a separate function
 . pretty: share code between format_decoration and show_decorations

 This causes warnings with -Wuninitialized, so I've ejected it from pu
 for the time being.


* rc/maint-complete-git-p4 (2012-09-24) 1 commit
  (merged to 'next' on 2012-10-29 at af52cef)
 + Teach git-completion about git p4

 Comment from Pete will need to be addressed in a follow-up patch.


* as/test-tweaks (2012-09-20) 7 commits
 - tests: paint unexpectedly fixed known breakages in bold red
 - tests: test the test framework more thoroughly
 - [SQUASH] t/t0000-basic.sh: quoting of TEST_DIRECTORY is screwed up
 - tests: refactor mechanics of testing in a sub test-lib
 - tests: paint skipped tests in bold blue
 - tests: test number comes first in 'not ok $count - $message'
 - tests: paint known breakages in bold yellow

 Various minor tweaks to the test framework to paint its output
 lines in colors that match what they mean better.

 Has the "is this really blue?" issue Peff raised resolved???


* jc/maint-name-rev (2012-09-17) 7 commits
 - describe --contains: use "name-rev --algorithm=weight"
 - name-rev --algorithm=weight: tests and documentation
 - name-rev --algorithm=weight: cache the computed weight in notes
 - name-rev --algorithm=weight: trivial optimization
 - name-rev: --algorithm option
 - name_rev: clarify the logic to assign a new tip-name to a commit
 - name-rev: lose unnecessary typedef

 "git name-rev" names the given revision based on a ref that can be
 reached in the smallest number of steps from the rev, but that is
 not useful when the caller wants to know which tag is the oldest one
 that contains the rev.  This teaches a new mode to the command that
 uses the oldest ref among those which contain the rev.

 I am not sure if this is worth it; for one thing, even with the help
 from notes-cache, it seems to make the "describe --contains" even
 slower. Also the command will be unusably slow for a user who does
 not have a write access (hence unable to create or update the
 notes-cache).

 Stalled mostly due to lack of responses.


* jc/xprm-generation (2012-09-14) 1 commit
 - test-generation: compute generation numbers and clock skews

 A toy to analyze how bad the clock skews are in histories of real
 world projects.

 Stalled mostly due to lack of responses.


* jc/blame-no-follow (2012-09-21) 2 commits
 - blame: pay attention to --no-follow
 - diff: accept --no-follow option

 Teaches "--no-follow" option to "git blame" to disable its
 whole-file rename detection.

 Stalled mostly due to lack of responses.


* jc/doc-default-format (2012-10-07) 2 commits
 - [SQAUSH] allow "cd Doc* && make DEFAULT_DOC_TARGET=..."
 - Allow generating a non-default set of documentation

 Need to address the installation half if this is to be any useful.


* mk/maint-graph-infinity-loop (2012-09-25) 1 commit
 - graph.c: infinite loop in git whatchanged --graph -m

 The --graph code fell into infinite loop when asked to do what the
 code did not expect ;-)

 Anybody who worked on "--graph" wants to comment?
 Stalled mostly due to lack of responses.


* jc/add-delete-default (2012-08-13) 1 commit
 - git add: notice removal of tracked paths by default

 "git add dir/" updated modified files and added new files, but does
 not notice removed files, which may be "Huh?" to some users.  They
 can of course use "git add -A dir/", but why should they?

 Resurrected from graveyard, as I thought it was a worthwhile thing
 to do in the longer term.

 Waiting for comments.


* mb/remote-default-nn-origin (2012-07-11) 6 commits
 - Teach get_default_remote to respect remote.default.
 - Test that plain "git fetch" uses remote.default when on a detached HEAD.
 - Teach clone to set remote.default.
 - Teach "git remote" about remote.default.
 - Teach remote.c about the remote.default configuration setting.
 - Rename remote.c's default_remote_name static variables.

 When the user does not specify what remote to interact with, we
 often attempt to use 'origin'.  This can now be customized via a
 configuration variable.

 Expecting a re-roll.

 "The first remote becomes the default" bit is better done as a
 separate step.


* mh/ceiling (2012-10-29) 8 commits
 - string_list_longest_prefix(): remove function
 - setup_git_directory_gently_1(): resolve symlinks in ceiling paths
 - longest_ancestor_length(): require prefix list entries to be normalized
 - longest_ancestor_length(): take a string_list argument for prefixes
 - longest_ancestor_length(): use string_list_split()
 - Introduce new function real_path_if_valid()
 - real_path_internal(): add comment explaining use of cwd
 - Introduce new static function real_path_internal()

 Elements of GIT_CEILING_DIRECTORIES list may not match the real
 pathname we obtain from getcwd(), leading the GIT_DIR discovery
 logic to escape the ceilings the user thought to have specified.

--------------------------------------------------
[Cooking]

* jl/submodule-rm (2012-11-14) 1 commit
  (merged to 'next' on 2012-11-18 at bf4525d)
 + docs: move submodule section

 Documentation correction for d21240f (Merge branch
 'jl/submodule-rm', 2012-10-29) that needs to be fast-tracked.

 Will merge to 'master' in the sixth batch.


* sg/complete-help-undup (2012-11-14) 1 commit
  (merged to 'next' on 2012-11-18 at eadd0f3)
 + completion: remove 'help' duplicate from porcelain commands

 Will merge to 'master' in the seventh batch.


* bc/do-not-recurse-in-die (2012-11-15) 1 commit
  (merged to 'next' on 2012-11-18 at 79d62a8)
 + usage.c: detect recursion in die routines and bail out immediately

 Will merge to 'master' in the seventh batch.


* cn/config-missing-path (2012-11-15) 1 commit
  (merged to 'next' on 2012-11-18 at c08b73c)
 + config: don't segfault when given --path with a missing value

 Will merge to 'master' in the sixth batch.


* jk/checkout-out-of-unborn (2012-11-15) 1 commit
  (merged to 'next' on 2012-11-18 at 7d2aa24)
 + checkout: print a message when switching unborn branches

 Will merge to 'master' in the sixth batch.


* mk/complete-tcsh (2012-11-16) 1 commit
  (merged to 'next' on 2012-11-19 at 8309029)
 + tcsh-completion re-using git-completion.bash

 Will merge to 'master' in the seventh batch.


* mm/status-push-pull-advise (2012-11-16) 1 commit
 - status: add advice on how to push/pull to tracking branch

 Will merge to 'next'.


* jk/maint-gitweb-xss (2012-11-12) 1 commit
  (merged to 'next' on 2012-11-14 at 7a667bc)
 + gitweb: escape html in rss title

 Fixes an XSS vulnerability in gitweb.

 Will merge to 'master' in the sixth batch.


* mg/replace-resolve-delete (2012-11-13) 1 commit
  (merged to 'next' on 2012-11-14 at fa785ae)
 + replace: parse revision argument for -d

 Be more user friendly to people using "git replace -d".

 Will merge to 'master' in the sixth batch.


* ml/cygwin-mingw-headers (2012-11-18) 2 commits
  (merged to 'next' on 2012-11-19 at f9964da)
 + USE CGYWIN_V15_WIN32API as macro to select api for cygwin
  (merged to 'next' on 2012-11-15 at 22e11b3)
 + Update cygwin.c for new mingw-64 win32 api headers

 Make git work on newer cygwin.

 Will merge to 'master' in the sixth batch.


* ta/doc-cleanup (2012-10-25) 6 commits
  (merged to 'next' on 2012-11-13 at e11fafd)
 + Documentation: build html for all files in technical and howto
 + Documentation/howto: convert plain text files to asciidoc
 + Documentation/technical: convert plain text files to asciidoc
 + Change headline of technical/send-pack-pipeline.txt to not confuse its content with content from git-send-pack.txt
 + Shorten two over-long lines in git-bisect-lk2009.txt by abbreviating some sha1
 + Split over-long synopsis in git-fetch-pack.txt into several lines

 Will merge to 'master' in the sixth batch.


* lt/diff-stat-show-0-lines (2012-10-17) 1 commit
  (merged to 'next' on 2012-11-19 at 0037290)
 + Fix "git diff --stat" for interesting - but empty - file changes

 We failed to mention a file without any content change but whose
 permission bit was modified, or (worse yet) a new file without any
 content in the "git diff --stat" output.

 Will merge to 'master' in the seventh batch.


* fc/zsh-completion (2012-11-19) 2 commits
 - completion: start moving to the new zsh completion
 - completion: add new zsh completion

 Replaced by shedding large changes to other independent topics.
 Any comments from zsh users?


* so/prompt-command (2012-10-17) 4 commits
  (merged to 'next' on 2012-10-25 at 79565a1)
 + coloured git-prompt: paint detached HEAD marker in red
 + Fix up colored git-prompt
 + show color hints based on state of the git tree
 + Allow __git_ps1 to be used in PROMPT_COMMAND

 Updates __git_ps1 so that it can be used as $PROMPT_COMMAND,
 instead of being used for command substitution in $PS1, to embed
 color escape sequences in its output.

 Will cook in 'next'.


* nd/wildmatch (2012-10-15) 13 commits
  (merged to 'next' on 2012-10-25 at 510e8df)
 + Support "**" wildcard in .gitignore and .gitattributes
 + wildmatch: make /**/ match zero or more directories
 + wildmatch: adjust "**" behavior
 + wildmatch: fix case-insensitive matching
 + wildmatch: remove static variable force_lower_case
 + wildmatch: make wildmatch's return value compatible with fnmatch
 + t3070: disable unreliable fnmatch tests
 + Integrate wildmatch to git
 + wildmatch: follow Git's coding convention
 + wildmatch: remove unnecessary functions
 + Import wildmatch from rsync
 + ctype: support iscntrl, ispunct, isxdigit and isprint
 + ctype: make sane_ctype[] const array

 Allows pathname patterns in .gitignore and .gitattributes files
 with double-asterisks "foo/**/bar" to match any number of directory
 hierarchies.

 I suspect that this needs to be plugged to pathspec matching code;
 otherwise "git log -- 'Docum*/**/*.txt'" would not show the log for
 commits that touch Documentation/git.txt, which would be confusing
 to the users.

 Will cook in 'next'.


* jh/update-ref-d-through-symref (2012-10-21) 2 commits
  (merged to 'next' on 2012-11-19 at 6bcca4c)
 + Fix failure to delete a packed ref through a symref
 + t1400-update-ref: Add test verifying bug with symrefs in delete_ref()

 "update-ref -d --deref SYM" to delete a ref through a symbolic ref
 that points to it did not remove it correctly.

 Will merge to 'master' in the seventh batch.


* jk/config-ignore-duplicates (2012-10-29) 9 commits
  (merged to 'next' on 2012-10-29 at 67fa0a2)
 + builtin/config.c: Fix a sparse warning
  (merged to 'next' on 2012-10-25 at 233df08)
 + git-config: use git_config_with_options
 + git-config: do not complain about duplicate entries
 + git-config: collect values instead of immediately printing
 + git-config: fix regexp memory leaks on error conditions
 + git-config: remove memory leak of key regexp
 + t1300: test "git config --get-all" more thoroughly
 + t1300: remove redundant test
 + t1300: style updates

 Drop duplicate detection from git-config; this lets it
 better match the internal config callbacks, which clears up
 some corner cases with includes.

 Will merge to 'master' in the sixth batch.


* fc/completion-test-simplification (2012-11-16) 6 commits
 - completion: simplify __gitcomp() test helper
 - completion: refactor __gitcomp related tests
 - completion: consolidate test_completion*() tests
 - completion: simplify tests using test_completion_long()
 - completion: standardize final space marker in tests
 - completion: add comment for test_completion()

 Clean up completion tests.  Use of conslidated helper may make
 instrumenting one particular test during debugging of the test
 itself, but I think that issue should be addressed in some other
 way (e.g. making sure individual tests in 9902 can be skipped).


* jk/pickaxe-textconv (2012-10-28) 2 commits
 - pickaxe: use textconv for -S counting
 - pickaxe: hoist empty needle check

 Use textconv filters when searching with "log -S".

 It probably should lose "are the textconv on the two sides the
 same?" check.


* fc/remote-bzr (2012-11-08) 5 commits
  (merged to 'next' on 2012-11-18 at 86add07)
 + remote-bzr: update working tree
 + remote-bzr: add support for remote repositories
 + remote-bzr: add support for pushing
 + remote-bzr: add simple tests
 + Add new remote-bzr transport helper

 New remote helper for bzr.

 Will merge to 'master' in the seventh batch.


* fc/remote-hg (2012-11-12) 20 commits
  (merged to 'next' on 2012-11-18 at 4a4f2e4)
 + remote-hg: avoid bad refs
 + remote-hg: try the 'tip' if no checkout present
 + remote-hg: fix compatibility with older versions of hg
 + remote-hg: add missing config for basic tests
 + remote-hg: the author email can be null
 + remote-hg: add option to not track branches
 + remote-hg: add extra author test
 + remote-hg: add tests to compare with hg-git
 + remote-hg: add bidirectional tests
 + test-lib: avoid full path to store test results
 + remote-hg: add basic tests
 + remote-hg: fake bookmark when there's none
 + remote-hg: add compat for hg-git author fixes
 + remote-hg: add support for hg-git compat mode
 + remote-hg: match hg merge behavior
 + remote-hg: make sure the encoding is correct
 + remote-hg: add support to push URLs
 + remote-hg: add support for remote pushing
 + remote-hg: add support for pushing
 + Add new remote-hg transport helper

 New remote helper for hg.

 Will merge to 'master' in the seventh batch.


* jk/maint-http-half-auth-fetch (2012-10-31) 2 commits
  (merged to 'next' on 2012-11-09 at af69926)
 + remote-curl: retry failed requests for auth even with gzip
 + remote-curl: hoist gzip buffer size to top of post_rpc

 Fixes fetch from servers that ask for auth only during the actual
 packing phase. This is not really a recommended configuration, but it
 cleans up the code at the same time.

 Will merge to 'master' in the sixth batch.


* kb/preload-index-more (2012-11-02) 1 commit
  (merged to 'next' on 2012-11-09 at a750ebd)
 + update-index/diff-index: use core.preloadindex to improve performance

 Use preloadindex in more places, which has a nice speedup on systems
 with slow stat calls (and even on Linux).

 Will merge to 'master' in the sixth batch.


* cr/push-force-tag-update (2012-11-19) 5 commits
 - push: update remote tags only with force
 - push: flag updates that require force
 - push: keep track of "update" state separately
 - push: add advice for rejected tag reference
 - push: return reject reasons via a mask

 Require "-f" for push to update a tag, even if it is a fast-forward.


* fc/fast-export-fixes (2012-11-08) 14 commits
 - fast-export: don't handle uninteresting refs
 - fast-export: make sure updated refs get updated
 - fast-export: fix comparison in tests
 - fast-export: trivial cleanup
 - remote-testgit: make clear the 'done' feature
 - remote-testgit: report success after an import
 - remote-testgit: exercise more features
 - remote-testgit: cleanup tests
 - remote-testgit: remove irrelevant test
 - remote-testgit: get rid of non-local functionality
 - Add new simplified git-remote-testgit
 - Rename git-remote-testgit to git-remote-testpy
 - remote-testgit: fix direction of marks
 - fast-export: avoid importing blob marks

 Improvements to fix fast-export bugs, including how refs pointing to
 already-seen commits are handled. An earlier 4-commit version of this
 series looked good to me, but this much-expanded version has not seen
 any comments.

 Looks like it has been re-rolled, but I haven't checked it out yet.

 Needs review.


* mh/alt-odb-string-list-cleanup (2012-11-08) 2 commits
  (merged to 'next' on 2012-11-13 at 2bf41d9)
 + link_alt_odb_entries(): take (char *, len) rather than two pointers
 + link_alt_odb_entries(): use string_list_split_in_place()

 Cleanups in the alternates code. Fixes a potential bug and makes the
 code much cleaner.

 Will merge to 'master' in the sixth batch.


* pw/maint-p4-rcs-expansion-newline (2012-11-08) 1 commit
  (merged to 'next' on 2012-11-13 at e90cc7c)
 + git p4: RCS expansion should not span newlines

 I do not have p4 to play with, but looks obviously correct to me.

 Will merge to 'master' in the sixth batch.


* rh/maint-gitweb-highlight-ext (2012-11-08) 1 commit
  (merged to 'next' on 2012-11-13 at c57d856)
 + gitweb.perl: fix %highlight_ext mappings

 Fixes a clever misuse of perl's list interpretation.

 Will merge to 'master' in the sixth batch.


* rr/submodule-diff-config (2012-11-18) 4 commits
  (merged to 'next' on 2012-11-19 at 355319e)
 + submodule: display summary header in bold
 + diff: rename "set" variable
 + diff: introduce diff.submodule configuration variable
 + Documentation: move diff.wordRegex from config.txt to diff-config.txt

 Lets "git diff --submodule=log" become the default via configuration.

 Will merge to 'master' in the seventh batch.

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Felipe Contreras @ 2012-11-19 23:12 UTC (permalink / raw)
  To: Krzysztof Mazur; +Cc: Junio C Hamano, Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <20121119225838.GA23412@shrek.podlesie.net>

On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:

> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -924,6 +924,10 @@ sub quote_subject {
>  # use the simplest quoting being able to handle the recipient
>  sub sanitize_address {
>         my ($recipient) = @_;
> +
> +       # remove garbage after email address
> +       $recipient =~ s/(.*>).*$/$1/;
> +

Looks fine, but I would do s/(.*?>)(.*)$/$1/, so that 'test
<foo@bar.com> <#comment>' gets the second comment removed.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* git config --git-all can return non-zero error code
From: Timur Tabi @ 2012-11-19 23:02 UTC (permalink / raw)
  To: git

According to the man page for git-config, the --git-all option is not
supposed to return an error code:

--get-all
	Like get, but does not fail if the number of values for the key is
	not exactly one.

IMHO, zero is also "not exactly one", but I still get a 1 exit code if the
key does not exist.

My .git/config says this:

[user "cq.branch"]
	mastr = upstream/master

git-config --get-all does this:

$ git config --get-all user.cq.branch.master
$ echo $?
1
$ git config --get-all user.cq.branch.mastr
upstream/master
$ echo $?
0

I just want git to return nothing if the key doesn't exist.  I don't want
it to return an exit code.  Is there a way to do this?  I think either the
code is broken or the documentation needs to be changed.

I'm running git version 1.7.3.4

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Krzysztof Mazur @ 2012-11-19 22:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <7vk3thxuj2.fsf@alter.siamese.dyndns.org>

On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:
> Given that the problematic line
> 
> 	Stable Kernel Maintainance Track <stable@vger.kernel.org> # vX.Y
> 
> is not even a valid e-mail address, doesn't this new logic belong to
> sanitize_address() conceptually?

Yes, it's much better to do it in the sanitize_address().

Felipe, may you check it?

Krzysiek
-- >8 --
Subject: [PATCH] git-send-email: remove garbage after email address

In some cases it's very useful to add some additional information
after email in Cc-list, for instance:

"Cc: Stable kernel <stable@vger.kernel.org> #v3.4 v3.5 v3.6"

Currently the git refuses to add such invalid email to Cc-list,
when the Email::Valid perl module is available or just uses whole line
as the email address.

Now in sanitize_address() everything after the email address is
removed, so the resulting line is correct email address and Email::Valid
validates it correctly.

Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
 git-send-email.perl | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..9840d0a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -924,6 +924,10 @@ sub quote_subject {
 # use the simplest quoting being able to handle the recipient
 sub sanitize_address {
 	my ($recipient) = @_;
+
+	# remove garbage after email address
+	$recipient =~ s/(.*>).*$/$1/;
+
 	my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/);
 
 	if (not $recipient_name) {
-- 
1.8.0.283.gc57d856

^ permalink raw reply related

* Re: [PATCH v3 0/6] completion: test consolidations
From: Felipe Contreras @ 2012-11-19 22:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: SZEDER Gábor, git
In-Reply-To: <7v6251xrfb.fsf@alter.siamese.dyndns.org>

On Mon, Nov 19, 2012 at 9:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> These started from a discussion with SZEDER, but then I realized there were
>> many improvements possible.
>
> Thanks; I'd loop him in and wait for his acks/reviews.

I thought my series-cc-cmd would add him. Maybe I'm using a version of
'git send-email' that doesn't have that.

Either way, we already know what he will say, to the previous series
he said he was against consolidation of test scripts. So I don't see
how the situation would change.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: Auto-repo-repair
From: Enrico Weigelt @ 2012-11-19 22:35 UTC (permalink / raw)
  To: Drew Northup; +Cc: Jeff King, git
In-Reply-To: <CAM9Z-nmu2MiE9vF9T6Aw8vFTR8mTkuR3akHgZX6+=n3uA4fmpA@mail.gmail.com>


> >> How would the broken repository be sure of what it is missing to
> >> request it from the other side?
> >
> > fsck will find missing objects.
> 
> And what about the objects referred to by objects that are missing?

Will be fetched after multiple iterations.
We could even introduce some 'fsck --autorepair' mode, which triggers
it to fetch any missing object from its remotes.

Maybe even introduce a concept of peer object stores, which (a bit like
alternates) are asked for objects that arent locally availabe - that
could be even a plain venti store.


cu
-- 
Mit freundlichen Grüßen / Kind regards 

Enrico Weigelt 
VNC - Virtual Network Consult GmbH 
Head Of Development 

Pariser Platz 4a, D-10117 Berlin
Tel.: +49 (30) 3464615-20
Fax: +49 (30) 3464615-59

enrico.weigelt@vnc.biz; www.vnc.de 

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Felipe Contreras @ 2012-11-19 21:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Krzysztof Mazur, Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <7vk3thxuj2.fsf@alter.siamese.dyndns.org>

On Mon, Nov 19, 2012 at 8:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Krzysztof Mazur <krzysiek@podlesie.net> writes:
>
>> On Mon, Nov 19, 2012 at 11:57:47AM +0200, Felipe Balbi wrote:
>>> Hi guys,
>>>
>>> for whatever reason my git has started acting up with
>>> stable@vger.kernel.org addresses. It doesn't manage to extract a valid
>>> adress from the string:
>>>
>>>  Cc: <stable@vger.kernel.org> # v3.4 v3.5 v3.6
>>>
>>> Removing the comment at the end of the line makes things work again. I
>>> do remember, however, seeing this working since few weeks back I sent a
>>> mail to stable (in fact the same one I'm using to test), so this could
>>> be related to some perl updates, who knows ?!?
>>
>> You probably just installed Email::Valid package.
>>
>> The current git-send-email works a little better and just prints an error:
>>
>> W: unable to extract a valid address from: <stable@vger.kernel.org> #v3.4 v3.5 v3.6
>>
>>
>> This patch should fix the problem, now after <email> any garbage is
>> removed while extracting address.
>>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 5a7c29d..bb659da 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -828,7 +828,7 @@ sub extract_valid_address {
>>       # check for a local address:
>>       return $address if ($address =~ /^($local_part_regexp)$/);
>>
>> -     $address =~ s/^\s*<(.*)>\s*$/$1/;
>> +     $address =~ s/^\s*<(.*)>.*$/$1/;
>>       if ($have_email_valid) {
>>               return scalar Email::Valid->address($address);
>>       } else {
>
> Given that the problematic line
>
>         Stable Kernel Maintainance Track <stable@vger.kernel.org> # vX.Y
>
> is not even a valid e-mail address, doesn't this new logic belong to
> sanitize_address() conceptually?

That would be great, it would also help the cc-cmd stuff. The
get_maintainer.pl patch from the Linux kernel outputs something like:

David Airlie <airlied@linux.ie> (maintainer:DRM DRIVERS)
Ben Skeggs <bskeggs@redhat.com>
(commit_signer:17/19=89%,commit_signer:43/46=93%)
Maxim Levitsky <maximlevitsky@gmail.com> (commit_signer:3/19=16%)
Greg Kroah-Hartman <gregkh@linuxfoundation.org> (commit_signer:2/19=11%)
Dave Airlie <airlied@redhat.com> (commit_signer:2/19=11%,commit_signer:3/46=7%)
Alex Deucher <alexander.deucher@amd.com> (commit_signer:1/19=5%)
dri-devel@lists.freedesktop.org (open list:DRM DRIVERS)
linux-kernel@vger.kernel.org (open list)

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH v6 0/2] New zsh wrapper
From: Felipe Contreras @ 2012-11-19 21:56 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Marc Khouzam, Marius Storm-Olsen, Marius Storm-Olsen,
	Jonathan Nieder, Peter van der Does, SZEDER Gábor,
	Mark Lodato
In-Reply-To: <7vpq39xw01.fsf@alter.siamese.dyndns.org>

On Mon, Nov 19, 2012 at 7:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> The second patch is new, in order for users to get the same features when
>> sourcing the bash script (they don't need to change anything). They'll get a
>> warning suggesting to check the new script git-completion.zsh. Eventually, that
>> support would be dropped from the bash script.
>>
>> Some people were suggesting something like this, so here it is.
>>
>> Can we merge the zsh wrapper now?
>>
>> Felipe Contreras (2):
>>   completion: add new zsh completion
>>   completion: start moving to the new zsh completion
>>
>>  contrib/completion/git-completion.bash | 104 +++++++++++++++++++--------------
>>  contrib/completion/git-completion.zsh  |  78 +++++++++++++++++++++++++
>>  2 files changed, 139 insertions(+), 43 deletions(-)
>>  create mode 100644 contrib/completion/git-completion.zsh
>
> Thanks; I am a bit puzzled as to the progression of this series, as
> it spanned many months.  I *think* the following are the previous
> ones, but I may be mixing up v$n patches for other series, so just
> to make sure (please correct if I am mistaken):
>
>  * (v1) http://thread.gmane.org/gmane.comp.version-control.git/189310
>    with only git-completion.zsh without any changes to the bash
>    side;

Yes, and with a lot of code that is not strictly needed.

>  * (v2) http://thread.gmane.org/gmane.comp.version-control.git/189381
>    without bash side changes;

Yes, also with more code.

>  * (v3) http://thread.gmane.org/gmane.comp.version-control.git/196720
>    without bash side changes;

Yes, now it's simpler due to changes in bash side already in.

>  * (v6) http://thread.gmane.org/gmane.comp.version-control.git/208170
>    with COMPREPLY changes;

Yeap.

>  * This one, with removal of zsh specific workarounds from bash
>    completion script.

Yes, although that is a separate patch.

> I do not care too much about how v4 and v5 looked like; I primarily
> am interested in knowing if I can discard 208170 from my inbox
> safely ;-).

Yes you can. The rest of the patches I sent in a different series.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH 3/4] pathspec: apply "*.c" optimization from exclude
From: Junio C Hamano @ 2012-11-19 21:20 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1353229989-13075-4-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> -O2 build on linux-2.6, without the patch:

Before the result, can you briefly explain what '"*.c" optimization
from exclude' the title talks about is?

    When a pattern contains only a single asterisk, e.g. "foo*bar",
    after literally comparing the leading part "foo" with the
    string, we can compare the tail of the string and make sure it
    matches "bar", instead of running fnmatch() on "*bar" against
    the remainder of the string.

The funny thing was that trying to explain what the logic should do
makes one realize potential issues in the implementation of that
logic ;-)  See below.

> $ time git rev-list --quiet HEAD -- '*.c'
>
> real    0m40.770s
> user    0m40.290s
> sys     0m0.256s
>
> With the patch
>
> $ time ~/w/git/git rev-list --quiet HEAD -- '*.c'
>
> real    0m34.288s
> user    0m33.997s
> sys     0m0.205s
>
> The above command is not supposed to be widely popular.

Hrm, perhaps.  I use "git grep <pattern> -- \*.c" quite a lot, but
haven't seen use case for \*.c in the context of the "log" family.

> diff --git a/cache.h b/cache.h
> index bf031f1..d18f584 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -473,6 +473,8 @@ extern int index_name_is_other(const struct index_state *, const char *, int);
>  extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
>  extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
>  
> +#define PSF_ONESTAR 1

Together with the GF_ prefix in the previous, PSF_ prefix needs a
bit of in-code explanation.  Is it just an RC3L (random combination
of 3 letters?)

> @@ -46,6 +46,12 @@ inline int git_fnmatch(const char *pattern, const char *string,
>  		pattern += prefix;
>  		string += prefix;
>  	}
> +	if (flags & GF_ONESTAR) {
> +		int pattern_len = strlen(++pattern);
> +		int string_len = strlen(string);
> +		return strcmp(pattern,
> +			      string + string_len - pattern_len);
> +	}

What happens when pattern="foo*oob" and string="foob"?

The prefix match before this code determines that the literal prefix
in the pattern matches with the early part of the string, and makes
pattern="*oob" and string="b".

When you come to strcmp(), you see that string_len is 1, pattern_len
is 3, and pattern is "oob".  string+string_len-pattern_len = "oob",
one past the beginning of the original string "foob".  They match.

Oops?

	return (string_len < pattern_len) ||
        	strcmp(pattern, string + string_len - pattern_len);

perhaps?

^ permalink raw reply

* Re: [PATCH 2/4] pathspec: do exact comparison on the leading non-wildcard part
From: Junio C Hamano @ 2012-11-19 20:54 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1353229989-13075-3-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  dir.c       | 18 +++++++++++++++++-
>  dir.h       |  8 ++++++++
>  tree-walk.c |  6 ++++--
>  3 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index c391d46..e4e6ca1 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -34,6 +34,21 @@ int fnmatch_icase(const char *pattern, const char *string, int flags)
>  	return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD : 0));
>  }
>  
> +inline int git_fnmatch(const char *pattern, const char *string,
> +		       int flags, int prefix)
> +{
> +	int fnm_flags = 0;
> +	if (flags & GF_PATHNAME)
> +		fnm_flags |= FNM_PATHNAME;
> +	if (prefix > 0) {
> +		if (strncmp(pattern, string, prefix))
> +			return FNM_NOMATCH;
> +		pattern += prefix;
> +		string += prefix;
> +	}
> +	return fnmatch(pattern, string, fnm_flags);

How would we protect this optimization from future breakages?

Once we start using FNM_PERIOD, this becomes unsafe, as the simple
part in "foo/bar*baz" would be "foo/bar" with remainder "*baz".

The pattern "foo/bar*baz" should match "foo/bar.baz" with FNM_PERIOD
set.  But with this optimization, fnmatch is fed pattern="*baz",
string=".baz" and they would not match.

^ permalink raw reply

* Re: [PATCH 1/4] pathspec: save the non-wildcard length part
From: Junio C Hamano @ 2012-11-19 20:48 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1353229989-13075-2-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> We marks pathspec with wildcards with the field use_wildcard. We could

s/marks/mark;

> do better by saving the length of the non-wildcard part, which can be
> for optimizations such as f9f6e2c (exclude: do strcmp as much as

s/for /used &/;

> possible before fnmatch - 2012-06-07)
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

The code looks straightforward and correct.  Thanks.

> ---
>  builtin/ls-files.c | 2 +-
>  builtin/ls-tree.c  | 2 +-
>  cache.h            | 2 +-
>  dir.c              | 6 +++---
>  tree-walk.c        | 4 ++--
>  5 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index b5434af..4a9ee69 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -337,7 +337,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)
>  		matchbuf[0] = prefix;
>  		matchbuf[1] = NULL;
>  		init_pathspec(&pathspec, matchbuf);
> -		pathspec.items[0].use_wildcard = 0;
> +		pathspec.items[0].nowildcard_len = pathspec.items[0].len;
>  	} else
>  		init_pathspec(&pathspec, NULL);
>  	if (read_tree(tree, 1, &pathspec))
> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
> index 235c17c..fb76e38 100644
> --- a/builtin/ls-tree.c
> +++ b/builtin/ls-tree.c
> @@ -168,7 +168,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
>  
>  	init_pathspec(&pathspec, get_pathspec(prefix, argv + 1));
>  	for (i = 0; i < pathspec.nr; i++)
> -		pathspec.items[i].use_wildcard = 0;
> +		pathspec.items[i].nowildcard_len = pathspec.items[i].len;
>  	pathspec.has_wildcard = 0;
>  	tree = parse_tree_indirect(sha1);
>  	if (!tree)
> diff --git a/cache.h b/cache.h
> index dbd8018..bf031f1 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -482,7 +482,7 @@ struct pathspec {
>  	struct pathspec_item {
>  		const char *match;
>  		int len;
> -		unsigned int use_wildcard:1;
> +		int nowildcard_len;
>  	} *items;
>  };
>  
> diff --git a/dir.c b/dir.c
> index 5a83aa7..c391d46 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -230,7 +230,7 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
>  			return MATCHED_RECURSIVELY;
>  	}
>  
> -	if (item->use_wildcard && !fnmatch(match, name, 0))
> +	if (item->nowildcard_len < item->len && !fnmatch(match, name, 0))
>  		return MATCHED_FNMATCH;
>  
>  	return 0;
> @@ -1429,8 +1429,8 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
>  
>  		item->match = path;
>  		item->len = strlen(path);
> -		item->use_wildcard = !no_wildcard(path);
> -		if (item->use_wildcard)
> +		item->nowildcard_len = simple_length(path);
> +		if (item->nowildcard_len < item->len)
>  			pathspec->has_wildcard = 1;
>  	}
>  
> diff --git a/tree-walk.c b/tree-walk.c
> index 3f54c02..af871c5 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -626,7 +626,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
>  					&never_interesting))
>  				return entry_interesting;
>  
> -			if (item->use_wildcard) {
> +			if (item->nowildcard_len < item->len) {
>  				if (!fnmatch(match + baselen, entry->path, 0))
>  					return entry_interesting;
>  
> @@ -642,7 +642,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
>  		}
>  
>  match_wildcards:
> -		if (!item->use_wildcard)
> +		if (item->nowildcard_len == item->len)
>  			continue;
>  
>  		/*

^ permalink raw reply

* Re: `git mv` has ambiguous error message for non-existing target
From: Patrick Lehner @ 2012-11-19 21:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpq3cja4y.fsf@alter.siamese.dyndns.org>

On Sa 17 Nov 2012 20:35:09 CET, Junio C Hamano wrote:
> Patrick Lehner <lehner.patrick@gmx.de> writes:
>
>> But just because mv's error essage isnt very good, does that mean git
>> mv's error message mustn't be better?
>
> Did I say the error message from 'mv' was not very good in the
> message you are responding to (by the way, this is why you should
> never top-post when you are responding to a message on this list)?
>
> I meant to say that the message from 'mv' is good enough, so is the
> one given by 'git mv'.
>
> I wouldn't reject a patch that updates our message to something more
> informative without looking at it, though.

I apologize for top-posting -- I don't usually use mailing lists and am 
not aware of the usual netiquette.

And yes, I did interpret a bit more into your reply than was there.

I wouldn't call the 'mv' error message "good enough" in this case, but 
very well, opinions may very well differ. Unfortunately, I have no time 
to get into the git code and contribution guidelines, so I cannot 
submit a patch myself. I would appreciate if someone else who shares my 
sentiment and knows their way around the git source a bit could find 
the time to add this :)

Regards,
Patrick

^ permalink raw reply

* Re: git-describe fails with "--dirty is incompatible with committishes" if passing HEAD as argument
From: Francis Moreau @ 2012-11-19 20:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfw45xu4e.fsf@alter.siamese.dyndns.org>

On Mon, Nov 19, 2012 at 8:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Francis Moreau <francis.moro@gmail.com> writes:
>
>> Inside the kernel repository, I tried this:
>>
>> $ git describe --dirty --match 'v[0-9]*' --abbrev=4 HEAD
>> fatal: --dirty is incompatible with committishes
>>
>> If 'HEAD' is removed then git-describe works as expected.
>>
>> Is that expected ?
>
> I would say so, at least in modern codebase.
>
> "git describe" without any commit object name used to mean "describe
> the HEAD commit using the better known points" before the --dirty
> option was introduced.
>
> But "--dirty" makes it describe the current checkout.  For example,
> output from "git describe --dirty" v1.8.0-211-gd8b4531-dirty means
> "your working tree contains work-in-progress based on d8b4531, which
> is 211 commits ahead of the v1.8.0 tag".  So conceptually, it should
> not take any commit, even if it were spelled HEAD.
>
> "git describe --dirty HEAD^^" would be an utter nonsense for the
> same reason.

Thanks for explaining, that makes sense.

--
Francis

^ permalink raw reply

* Re: [PATCH v3 0/6] completion: test consolidations
From: Junio C Hamano @ 2012-11-19 20:34 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: git, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> These started from a discussion with SZEDER, but then I realized there were
> many improvements possible.

Thanks; I'd loop him in and wait for his acks/reviews.

>
> Changes since v2:
>
>  * Updated comments and commit messages
>
> Changes since v1:
>
>  * A lot more cleanups
>
> Felipe Contreras (6):
>   completion: add comment for test_completion()
>   completion: standardize final space marker in tests
>   completion: simplify tests using test_completion_long()
>   completion: consolidate test_completion*() tests
>   completion: refactor __gitcomp related tests
>   completion: simplify __gitcomp() test helper
>
>  t/t9902-completion.sh | 133 +++++++++++++++++++-------------------------------
>  1 file changed, 51 insertions(+), 82 deletions(-)

^ permalink raw reply

* Re: [PATCH v4.1 5/5] push: update remote tags only with force
From: Junio C Hamano @ 2012-11-19 20:23 UTC (permalink / raw)
  To: Chris Rorvick
  Cc: git, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras
In-Reply-To: <1353189237-19491-1-git-send-email-chris@rorvick.com>

Chris Rorvick <chris@rorvick.com> writes:

> References are allowed to update from one commit-ish to another if the
> former is a ancestor of the latter.  This behavior is oriented to
> branches which are expected to move with commits.  Tag references are
> expected to be static in a repository, though, thus an update to a
> tag (lightweight and annotated) should be rejected unless the update is
> forced.
>
> To enable this functionality, the following checks have been added to
> set_ref_status_for_push() for updating refs (i.e, not new or deletion)
> to restrict fast-forwarding in pushes:
>
>   1) The old and new references must be commits.  If this fails,
>      it is not a valid update for a branch.
>
>   2) The reference name cannot start with "refs/tags/".  This
>      catches lightweight tags which (usually) point to commits
>      and therefore would not be caught by (1).
>
> If either of these checks fails, then it is flagged (by default) with a
> status indicating the update is being rejected due to the reference
> already existing in the remote.  This can be overridden by passing
> --force to git push.

This, if it were implemented back when we first added "git push",
would have been a nice safety, but added after 1.8.0 it would be a
regression, so we would need backward compatibility notes in the
release notes.

Not an objection; just making a mental note.

> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index fe46c42..479e25f 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -51,11 +51,11 @@ be named. If `:`<dst> is omitted, the same ref as <src> will be
>  updated.
>  +
>  The object referenced by <src> is used to update the <dst> reference
> -on the remote side, but by default this is only allowed if the
> -update can fast-forward <dst>.  By having the optional leading `+`,
> -you can tell git to update the <dst> ref even when the update is not a
> -fast-forward.  This does *not* attempt to merge <src> into <dst>.  See
> -EXAMPLES below for details.
> +on the remote side.  By default this is only allowed if the update is
> +a branch, and then only if it can fast-forward <dst>.  By having the

I can already see the next person asking "I can update refs/notes
the same way.  The doc says it applies only to the branches.  Is
this a bug?".

> diff --git a/cache.h b/cache.h
> index f410d94..127e504 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1004,13 +1004,14 @@ struct ref {
>  		requires_force:1,
>  		merge:1,
>  		nonfastforward:1,
> -		is_a_tag:1,
> +		forwardable:1,

This is somewhat an unfortunate churn.  Perhaps is_a_tag could have
started its life under its final name in the series?

I am assuming that the struct members will be initialized to 0 (false),
so everything by default is now not forwardable if somebody forgets
to set this flag?

>  	enum {
>  		REF_STATUS_NONE = 0,
>  		REF_STATUS_OK,
>  		REF_STATUS_REJECT_NONFASTFORWARD,
> +		REF_STATUS_REJECT_ALREADY_EXISTS,
>  		REF_STATUS_REJECT_NODELETE,
>  		REF_STATUS_UPTODATE,
>  		REF_STATUS_REMOTE_REJECT,
> diff --git a/remote.c b/remote.c
> index 44e72d6..a723f7a 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1311,14 +1311,24 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
>  		 *     to overwrite it; you would not know what you are losing
>  		 *     otherwise.
>  		 *
> -		 * (3) if both new and old are commit-ish, and new is a
> -		 *     descendant of old, it is OK.
> +		 * (3) if new is commit-ish and old is a commit, new is a
> +		 *     descendant of old, and the reference is not of the
> +		 *     refs/tags/ hierarchy it is OK.
>  		 *
>  		 * (4) regardless of all of the above, removing :B is
>  		 *     always allowed.
>  		 */

I think this is unreadable.  Isn't this more like

    (1.5) if the destination is inside refs/tags/ and already
          exists, you are not allowed to overwrite it without
          --force or +A:B notation.

early in the rule set?

> -		ref->is_a_tag = !prefixcmp(ref->name, "refs/tags/");
> +		if (prefixcmp(ref->name, "refs/tags/")) {
> +			/* Additionally, disallow fast-forwarding if
> +			 * the old object is not a commit.  This kicks
> +			 * out annotated tags that might pass the
> +			 * is_newer() test but dangle if the reference
> +			 * is updated.
> +			 */

Huh?  prefixcmp() excludes refs/tags/ hierarchy. What is an
annotated tag doing there?  Is this comment outdated???

	/*
         * Also please end the first line of a multi-line
         * comment with '/', '*', and nothing else.
         */

Regarding the other arm of this "if (not in refs/tags/ hierarchy)",
what happens when refs/tags/foo is a reference to a blob or a tree?
This series declares that the point of tag is not to let people to
move it willy-nilly, and I think it is in line with its spirit if
you just rejected non-creation events.

Also, I suspect that you do not even need to have old object locally
when looking at refs/tags/ hierarchy.  "Do not overwrite tags" can
be enforced by only noticing that they already have something; you
do not know what that something actually is to prevent yourself from
overwriting it.  You may have to update the rule (2) in remote.c
around l.1311 for this.

> +test_expect_success 'push requires --force to update lightweight tag' '
> +	mk_test heads/master &&
> +	mk_child child1 &&
> +	mk_child child2 &&
> +	(
> +		cd child1 &&
> +		git tag Tag &&
> +		git push ../child2 Tag &&

Don't you want to make sure that another "git push ../child2 Tag" at
this point, i.e. attempt to update Tag with the same, should succeed
without --force?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox