Git development
 help / color / mirror / Atom feed
* [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: [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

* 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 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 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 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: 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 v4.1 5/5] push: update remote tags only with force
From: Chris Rorvick @ 2012-11-20  4:43 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras
In-Reply-To: <7va9udxryf.fsf@alter.siamese.dyndns.org>

On Mon, Nov 19, 2012 at 2:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Chris Rorvick <chris@rorvick.com> writes:
>>  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?".

Sure, will fix.

>> 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?

Yeah, this was one of a few stupid mistakes.  Previously I used
'forwardable' throughout, but that is awkward except in the last
commit since until then everything is allowed to fast-forward and the
flag is only used to output tag-specific advice.  But inverting the
meaning of the flag is dumb, and I didn't even do it right.

But, as I think you're suggesting, it probably makes more sense to use
a flag that prevents fast-forwarding when set.  So maybe
"not_forwardable", or "is_a_tag" => "not_forwardable" if you think the
renaming is a good idea.  Or maybe just "is_a_tag"?  I think the
rename is good because its meaning is more general in the last commit.

>> 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?

Yes, something like that is much better.

>> -             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???

I think the comment is accurate, although inverting the meaning of the
flag probably doesn't help the clarity of this change.

What do you mean by "there"?  Annotated tags normally are referenced
via something under refs/tags/, but they could be elsewhere, right?
So what I meant to say was: if the reference is not under refs/tags/
then the starting point has to be a commit for it to be forwardable.

I interpreted parse_object(ref->old_sha1) == NULL as the old thing not
existing (rule 1) but I'm pretty sure that was a mistake.  I should
first test it with is_null_sha1, if true then it doesn't exist.
Otherwise parse_object() returning NULL means we just don't have the
object and therefore should follow rule 2.

> 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.

Nothing under refs/tags/ is updateable and only commits are updateable
outside of that due to the new logic.  The ref_newer() call will
reject updating to blobs and trees, although that probably isn't the
right thing to do.  Previously I was ensuring both old and new objects
were commits if the ref was not under refs/tags/, but it occurred to
me that fast-forwarding from a commit to a tag might be a reasonable
thing to want to do.  But a rejected update to a blob should get the
already-exists advice, not a message suggesting a merge (which is what
you get by relying on ref_newer() to fail.)

> 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.

Isn't it already doing this?  I think I've either confounded things
with my mistakes or I'm missing this point.

>> +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?

Yes, that is clearly a good addition.

Thanks for all the feedback.  Sorry I mucked up this series a bit,
tried to get an update out too quickly I guess.

Chris

^ permalink raw reply

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

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

> On Mon, Nov 19, 2012 at 04:49:09PM -0800, Junio C Hamano wrote:
>> "W. Trevor King" <wking@tremily.us> writes:
>> ...
>> > 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?

He may not be updating the gitlink with "git add foo" at the
top-level superproject level.  He is just using that submodule as
part of the larger whole as he is working on either the top-level or
some other submodule.  And checkout of 'foo' is necessary in the
working tree for him to work in the larger context of the project,
and 'foo' is set to float at the tip of its 'bar' branch.  And that
checkout results in a commit that is different from the commit the
gitlink suggests, perhaps because somebody worked in 'foo' submodule
and advanced the tip of branch 'bar'.

So:

 - at the top-level superproject level, entry 'foo' in the HEAD tree
   points at an older commit;

 - 'foo/.git/HEAD' points at refs/heads/bar, which matches the
   working tree of 'foo' and the index foo/.git/index..

I am not sure what should happen to the entry 'foo' in the index of
the top-level superproject after such a 'submodule floats at the
tip' checkout, but I imagine that it must match the contents of
foo/.git/HEAD's tree.  Otherwise, "git diff" at the top-level would
report local changes.

When committing his work at the top-level, he will see that 'foo'
gitlink is updated in that commit; after all that combination is the
context in which his work was done.

Or are you envisioning that such a check-out will and should show a
local difference at the submodule 'foo' by leaving the index of the
top-level superproject unchanged, and the user should refrain from
using "git commit -a" to avoid having to describe the changes made
on the 'bar' branch in the meantime in his top-level commit?  That
is certainly fine by me (I am no a heavy submodule user to begin
with), but I am not sure if that is useful and helpful to the
submodule users.

^ permalink raw reply

* Re: [PATCH v4.1 5/5] push: update remote tags only with force
From: Junio C Hamano @ 2012-11-20  5:44 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: <CAEUsAPa9fiF9cuMqRHNsQSz_CsbbvdO-eGDS9HWW3MrAYZPA8w@mail.gmail.com>

Chris Rorvick <chris@rorvick.com> writes:

> On Mon, Nov 19, 2012 at 2:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Yeah, this was one of a few stupid mistakes.  Previously I used
> 'forwardable' throughout, but that is awkward except in the last
> commit since until then everything is allowed to fast-forward and the
> flag is only used to output tag-specific advice.  But inverting the
> meaning of the flag is dumb, and I didn't even do it right.
>
> But, as I think you're suggesting, it probably makes more sense to use
> a flag that prevents fast-forwarding when set.  So maybe
> "not_forwardable", or "is_a_tag" => "not_forwardable" if you think the
> renaming is a good idea.

Yeah, calling it not-forwardable from the beginning would be a
sensible approach, I would think.

Thanks.

^ permalink raw reply

* [PATCH 14/13] test-wildmatch: avoid Windows path mangling
From: Johannes Sixt @ 2012-11-20  7:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <7vvcdco1pf.fsf@alter.siamese.dyndns.org>

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

The MSYS bash mangles arguments that begin with a forward slash
when they are passed to test-wildmatch. This causes tests to fail.
Avoid mangling by prepending "XXX", which is removed by
test-wildmatch before further processing.

[J6t: reworded commit message]

Reported-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 Works well, and I'm fine with this work-around.

 -- Hannes

 t/t3070-wildmatch.sh | 10 +++++-----
 test-wildmatch.c     |  8 ++++++++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
index e6ad6f4..3155eab 100755
--- a/t/t3070-wildmatch.sh
+++ b/t/t3070-wildmatch.sh
@@ -74,7 +74,7 @@ match 0 0 'foo/bar' 'foo[/]bar'
 match 0 0 'foo/bar' 'f[^eiu][^eiu][^eiu][^eiu][^eiu]r'
 match 1 1 'foo-bar' 'f[^eiu][^eiu][^eiu][^eiu][^eiu]r'
 match 1 0 'foo' '**/foo'
-match 1 x '/foo' '**/foo'
+match 1 x 'XXX/foo' '**/foo'
 match 1 0 'bar/baz/foo' '**/foo'
 match 0 0 'bar/baz/foo' '*/foo'
 match 0 0 'foo/bar/baz' '**/bar*'
@@ -95,8 +95,8 @@ match 0 0 ']' '[!]-]'
 match 1 x 'a' '[!]-]'
 match 0 0 '' '\'
 match 0 x '\' '\'
-match 0 x '/\' '*/\'
-match 1 x '/\' '*/\\'
+match 0 x 'XXX/\' '*/\'
+match 1 x 'XXX/\' '*/\\'
 match 1 1 'foo' 'foo'
 match 1 1 '@foo' '@foo'
 match 0 0 'foo' '@foo'
@@ -187,8 +187,8 @@ match 0 0 '-' '[[-\]]'
 match 1 1 '-adobe-courier-bold-o-normal--12-120-75-75-m-70-iso8859-1' '-*-*-*-*-*-*-12-*-*-*-m-*-*-*'
 match 0 0 '-adobe-courier-bold-o-normal--12-120-75-75-X-70-iso8859-1' '-*-*-*-*-*-*-12-*-*-*-m-*-*-*'
 match 0 0 '-adobe-courier-bold-o-normal--12-120-75-75-/-70-iso8859-1' '-*-*-*-*-*-*-12-*-*-*-m-*-*-*'
-match 1 1 '/adobe/courier/bold/o/normal//12/120/75/75/m/70/iso8859/1' '/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
-match 0 0 '/adobe/courier/bold/o/normal//12/120/75/75/X/70/iso8859/1' '/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
+match 1 1 'XXX/adobe/courier/bold/o/normal//12/120/75/75/m/70/iso8859/1' 'XXX/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
+match 0 0 'XXX/adobe/courier/bold/o/normal//12/120/75/75/X/70/iso8859/1' 'XXX/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
 match 1 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txt' '**/*a*b*g*n*t'
 match 0 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txtz' '**/*a*b*g*n*t'
 diff --git a/test-wildmatch.c b/test-wildmatch.c
index 74c0864..e384c8e 100644
--- a/test-wildmatch.c
+++ b/test-wildmatch.c
@@ -3,6 +3,14 @@
  int main(int argc, char **argv)
 {
+	int i;
+	for (i = 2; i < argc; i++) {
+		if (argv[i][0] == '/')
+			die("Forward slash is not allowed at the beginning of the\n"
+			    "pattern because Windows does not like it. Use `XXX/' instead.");
+		else if (!strncmp(argv[i], "XXX/", 4))
+			argv[i] += 3;
+	}
 	if (!strcmp(argv[1], "wildmatch"))
 		return !!wildmatch(argv[3], argv[2], 0);
 	else if (!strcmp(argv[1], "iwildmatch"))

^ permalink raw reply related

* Re: [PATCH 15/13] compat/fnmatch: fix off-by-one character class's length check
From: Johannes Sixt @ 2012-11-20  7:06 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <1352628837-5784-2-git-send-email-pclouds@gmail.com>

Am 11/11/2012 11:13, schrieb Nguyễn Thái Ngọc Duy:
> Character class "xdigit" is the only one that hits 6 character limit
> defined by CHAR_CLASS_MAX_LENGTH. All other character classes are 5
> character long and therefore never caught by this.
> 
> This should make xdigit tests in t3070 pass on Windows.
> 
> Reported-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  I tested with Linux (removing the ifdef __LIBC in fnmatch.c) but I
>  think this should get an ACK from someone who actually uses it on
>  Windows.

Works well here on Windows.

This does not affect Windows alone, but all platforms that fall back to
compat/fnmatch. It's perhaps worth its own topic branch.

>  We may want to tell upstream (who?) about this if they haven't fixed
>  it already.
> 
>  compat/fnmatch/fnmatch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch.c
> index 9473aed..0ff1d27 100644
> --- a/compat/fnmatch/fnmatch.c
> +++ b/compat/fnmatch/fnmatch.c
> @@ -345,7 +345,7 @@ internal_fnmatch (pattern, string, no_leading_period, flags)
>  
>  		    for (;;)
>  		      {
> -			if (c1 == CHAR_CLASS_MAX_LENGTH)
> +			if (c1 > CHAR_CLASS_MAX_LENGTH)
>  			  /* The name is too long and therefore the pattern
>  			     is ill-formed.  */
>  			  return FNM_NOMATCH;
> 

-- Hannes

^ permalink raw reply

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

On Mon, Nov 19, 2012 at 04:00:09PM -0800, Junio C Hamano wrote:
> 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

Before sending that patch, I checked that and tested with and without
Email::Valid.

> 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.

I will try to check that.

Krzysiek

^ permalink raw reply

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

On Mon, Nov 19, 2012 at 03:57:36PM -0800, Junio C Hamano wrote:
> 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?

I also thought about removing everything after first ">", but I will
not work for addresses like:

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

What about:

	$recipient =~ s/(.*<[^@]*@[^]]*>).*$/$1/;

or even

diff --git a/git-send-email.perl b/git-send-email.perl
index 9840d0a..b988c57 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -925,8 +925,11 @@ sub quote_subject {
 sub sanitize_address {
 	my ($recipient) = @_;
 
+	my $local_part_regexp = qr/[^<>"\s@]+/;
+	my $domain_regexp = qr/[^.<>"\s@]+(?:\.[^.<>"\s@]+)+/;
+
 	# remove garbage after email address
-	$recipient =~ s/(.*>).*$/$1/;
+	$recipient =~ s/(.*<$local_part_regexp\@$domain_regexp>).*$/$1/;
 
 	my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/);

which uses regex used by 99% accurate version of extract_valid_address().

Krzysiek

^ permalink raw reply related

* git-svn: What is --follow-parent / --no-follow-parent for?
From: Sebastian Leske @ 2012-11-20  7:31 UTC (permalink / raw)
  To: git

Hi,

on reading the docs of "git-svn", I stumbled across this paragraph:

> --follow-parent
> This is especially helpful when we’re tracking a directory that has been
> moved around within the repository, or if we started tracking a branch
> and never tracked the trunk it was descended from.  This feature is
> enabled by default, use --no-follow-parent to disable it.

However, this does not make sense to me: This sounds like there is no
good reason *not* to enable this option.  So why is it there? And in
what situation might I want to use "--no-follow-parent"?

As a matter of fact, I'm not even sure what "--no-follow-parent" does
(and the docs don't really say). 

I tried it out with a small test repo with a single branch (produced by
copying the trunk, then later deleted). With --follow-parent git-svn
correctly detected the branch point, and modeled the branch deletion as
a merge. With --no-follow-parent it just acted as if branch and trunk
were completely unrelated.

Commit graph of git-svn result:

--follow-parent:               --no-follow-parent:


       |                               |
      /|                             | |
     / |                             | |
     | |                             | |
     | |                             | |
     | |                             | |
     \ |                             | |
      \|                             | |
       |                               | 


(please excuse cheap ASCII art)

Is that the only effect of --no-follow-parent? And again, why would I
want that?

I'd be grateful for any clarifications. If I manage to understand the
explanation, I'll volunteer to summarize it into doc patch (if there are
no objections).

^ permalink raw reply

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

On Tue, Nov 20, 2012 at 08:31:00AM +0100, Krzysztof Mazur wrote:
> On Mon, Nov 19, 2012 at 03:57:36PM -0800, Junio C Hamano wrote:
> > 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?
> 
> I also thought about removing everything after first ">", but I will
> not work for addresses like:
> 
> Cc: "foo >" <stable@vger.kernel.org> #v3.4 v3.5 v3.6
> 
> What about:
> 
> 	$recipient =~ s/(.*<[^@]*@[^]]*>).*$/$1/;
> 
> or even
> 
> which uses regex used by 99% accurate version of extract_valid_address().
> 

Of course, as you suggested earier, only the first email address should
be used, so in both cases the first ".*" should be changed to ".*?".
The second version becomes:

diff --git a/git-send-email.perl b/git-send-email.perl
index 9840d0a..dbe520c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -925,8 +925,11 @@ sub quote_subject {
 sub sanitize_address {
 	my ($recipient) = @_;
 
+	my $local_part_regexp = qr/[^<>"\s@]+/;
+	my $domain_regexp = qr/[^.<>"\s@]+(?:\.[^.<>"\s@]+)+/;
+
 	# remove garbage after email address
-	$recipient =~ s/(.*>).*$/$1/;
+	$recipient =~ s/^(.*?<$local_part_regexp\@$domain_regexp>).*/$1/;
 
 	my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/);
 
Krzysiek

^ permalink raw reply related

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

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

On Mon, Nov 19, 2012 at 11:58:38PM +0100, Krzysztof Mazur wrote:
> 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>

Tested-by: Felipe Balbi <balbi@ti.com>

> ---
>  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
> 

-- 
balbi

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

^ permalink raw reply

* gitk: Portuguese "Ignore space change" translation
From: Joao Vitor P. Moraes @ 2012-11-20 10:03 UTC (permalink / raw)
  To: git

Inside gitk there's a checkbox which says:
Ignore space change

It was translated to portuguese (pt-br) as:
Ignorar mudanças de caixa

But that message in portuguese means:
Ignore case changes

that checkbox does not ignore case changes, but instead it ignores space
changes, a better translation would be

Ignorar mudanças de espaço
or
Ignorar espaçamentos
or
Ignorar espaços

^ permalink raw reply

* [PATCH] Fix typo in 1.8.1 release notes
From: Alexey Molchanov @ 2012-11-20 10:03 UTC (permalink / raw)
  To: git

Fix typo in documentation

Signed-off-by: Alexey Molchanov <alexey.molchanov@portaone.com>
---
 Documentation/RelNotes/1.8.1.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/RelNotes/1.8.1.txt b/Documentation/RelNotes/1.8.1.txt
index 107e5f3..559cd05 100644
--- a/Documentation/RelNotes/1.8.1.txt
+++ b/Documentation/RelNotes/1.8.1.txt
@@ -36,7 +36,7 @@ UI, Workflows & Features
    give the default number of context lines in the patch output, to
    override the hardcoded default of 3 lines.

- * "git format-patch" leraned the "--notes=<ref>" option to give
+ * "git format-patch" learned the "--notes=<ref>" option to give
    notes for the commit after the three-dash lines in its output.

  * "git log --grep=<pcre>" learned to honor the "grep.patterntype"
-- 
1.7.11.2

^ permalink raw reply related

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

On Tue, Nov 20, 2012 at 8:56 AM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:

> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -925,8 +925,11 @@ sub quote_subject {
>  sub sanitize_address {
>         my ($recipient) = @_;
>
> +       my $local_part_regexp = qr/[^<>"\s@]+/;
> +       my $domain_regexp = qr/[^.<>"\s@]+(?:\.[^.<>"\s@]+)+/;
> +
>         # remove garbage after email address
> -       $recipient =~ s/(.*>).*$/$1/;
> +       $recipient =~ s/^(.*?<$local_part_regexp\@$domain_regexp>).*/$1/;

I don't think all that extra complexity is warranted, to me
s/(.*?>)(.*)$/$1/ is just fine.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: Failure to extra stable@vger.kernel.org addresses
From: Andreas Ericsson @ 2012-11-20 11:08 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Krzysztof Mazur, Junio C Hamano, Felipe Balbi, git,
	Tomi Valkeinen
In-Reply-To: <CAMP44s38gTB_3Ao1rFZgMo2EAuiNb+h88-qRFcQPRMJNxo3CAQ@mail.gmail.com>

On 11/20/2012 11:28 AM, Felipe Contreras wrote:
> On Tue, Nov 20, 2012 at 8:56 AM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:
> 
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -925,8 +925,11 @@ sub quote_subject {
>>   sub sanitize_address {
>>          my ($recipient) = @_;
>>
>> +       my $local_part_regexp = qr/[^<>"\s@]+/;
>> +       my $domain_regexp = qr/[^.<>"\s@]+(?:\.[^.<>"\s@]+)+/;
>> +
>>          # remove garbage after email address
>> -       $recipient =~ s/(.*>).*$/$1/;
>> +       $recipient =~ s/^(.*?<$local_part_regexp\@$domain_regexp>).*/$1/;
> 
> I don't think all that extra complexity is warranted, to me
> s/(.*?>)(.*)$/$1/ is just fine.
> 

It's intentionally left without the at-sign so one can send mail to a
local account as well as remote ones. Very nifty when debugging, and
when one wants to preview outgoing emails.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

^ permalink raw reply

* Re: RFC: Making submodules "track" branches
From: nottrobin @ 2012-11-20 11:16 UTC (permalink / raw)
  To: git
In-Reply-To: <AANLkTiknGcteTjrHWM02H2KOMMDPZKHY1w0ZOIswddFn@mail.gmail.com>

Did any of this ever find its way into the submodule core? I'd like to have a
submodule that tracks a branch.



--
View this message in context: http://git.661346.n2.nabble.com/RFC-Making-submodules-track-branches-tp5151566p7571610.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: Crash when pushing large binary file
From: Nguyen Thai Ngoc Duy @ 2012-11-20 11:37 UTC (permalink / raw)
  To: Thomas Gay; +Cc: Git Mailing List
In-Reply-To: <CA+AoP-n=P3-yTshfQRwdyUcb7eFM12-+bAtS_FUAxXtwuFxjzg@mail.gmail.com>

On Tue, Nov 20, 2012 at 8:50 AM, Thomas Gay <tgay@tokyois.com> wrote:
> Sorry for the late reply; I've been out of the office for the past few days.
>
> On Fri, Nov 16, 2012 at 7:11 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>> If it still says "unpack-objects died of signal 11" then it was not
>> done the right way.
>
> I upgraded Git on both ends and tried setting it again. index-pack was
> used this time, but it still failed:
>
> Counting objects: 38, done.
> Delta compression using up to 4 threads.
> Compressing objects: 100% (26/26), done.
> error: index-pack died of signal 11 GiB | 62.44 MiB/s
> error: pack-objects died of signal 13
> error: failed to push some refs to '...'
>
> Would you still like me to try your other method?

Interesting. No, if index-pack fails too the culprit is something
else. Does the stack trace look the same (i.e. crashed in
write_sha1_file_prepare)? I still don't know how it crashes there.
It'd be great if you could run it under valgrind (not sure if valgrind
runs in OS X). Try building with NO_OPENSSL too to avoid external sha1
issues.
-- 
Duy

^ permalink raw reply

* Re: Auto-repo-repair
From: Drew Northup @ 2012-11-20 11:56 UTC (permalink / raw)
  To: Enrico Weigelt; +Cc: Jeff King, git
In-Reply-To: <fd162e10-46a8-433c-80b2-c1c4185c2032@zcs>

On Mon, Nov 19, 2012 at 5:35 PM, Enrico Weigelt <enrico.weigelt@vnc.biz> wrote:
>
>> >> 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.

I still think that it would make the most sense to do the following
(if you insist on some sort of automated repair):
(1) Fetch a "good" clone (or clones) into a temporary directory;
(2) Cannibalize the objects from it (them);
(3) Re-run git fsck and check for still-missing / unreachable items;
(4) IF THE RESULT OF (3) IS ACCEPTABLE, run git gc to clean up the
mess, discard / "merge" duplicate objects, and fix up the packfiles.

It is step (4) that requires the most user interaction. I could see
building up a shell script that does all but (4) nearly automatically.
None of this requires modifying Git itself.

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2012, #06; Mon, 19)
From: Paul Fox @ 2012-11-20 11:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpq39up0m.fsf@alter.siamese.dyndns.org>

junio c hamano wrote:
 > [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.

i don't have strong feelings on any of the later review comments
(though i guess the check on finish_command()'s return code needed
attention), and wasn't the last submitter, but would certainly like to
see this move forward.

paul
=---------------------
 paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 26.1 degrees)

^ 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