Git development
 help / color / mirror / Atom feed
* Re: rev-list --cherry-pick and context lines
From: Michael J Gruber @ 2011-09-02 15:32 UTC (permalink / raw)
  To: Stefan Haller; +Cc: git
In-Reply-To: <1k6yux4.x1kexb19bkrqpM%lists@haller-berlin.de>

Stefan Haller venit, vidit, dixit 02.09.2011 12:35:
> Consider two commits on different branches, one with this patch:
> 
>     diff --git a/file.txt b/file.txt
>     index 704fa27..2f7e74c 100644
>     --- a/file.txt
>     +++ b/file.txt
>     @@ -1,3 +1,3 @@
>      old_context
>      
>     -foo
>     +bar
> 
> and the other with this patch:
> 
>     diff --git a/file.txt b/file.txt
>     index f35051b..8c7de32 100644
>     --- a/file.txt
>     +++ b/file.txt
>     @@ -1,3 +1,3 @@
>      new_context
>      
>     -foo
>     +bar
> 
> If I run "git rev-list --cherry-pick --left-right branch1...branch2", it
> reports both commits as being genuine commits on their respective
> branch, even though I consider their patches to be the same.
> 
> I guess for my purpose I would like to have patch-ids that ignore
> context (or that use only one line of context, I'm not sure which).
> 
> In fact, if I do "git show <commit> -U1 | git patch-id", both commits
> show the same id.
> 
> So, would it make sense to have a parameter for git-rev-list (and
> git-cherry) that lets you specify how much context to be used for the
> patch ids?

It would be a bit like the patch below. "git log" accepts diff options already.
But:

- Do we want the patch id generation and the patch display (-p) to use the
  same options?

- -U1 implies -p/--patch and there is no --no-patch.

- Which other diff options do we want to pass to the patch id
  generation: --histogram, --patience, ...?

Cheers,
Michael

----

diff --git i/diff.c w/diff.c
index fcc0078..4e82912 100644
--- i/diff.c
+++ w/diff.c
@@ -4103,7 +4103,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
 		}
 
 		xpp.flags = 0;
-		xecfg.ctxlen = 3;
+		xecfg.ctxlen = options->context;
 		xecfg.flags = 0;
 		xdi_diff_outf(&mf1, &mf2, patch_id_consume, &data,
 			      &xpp, &xecfg);
diff --git i/revision.c w/revision.c
index 072ddac..5a98ed9 100644
--- i/revision.c
+++ w/revision.c
@@ -601,6 +601,7 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)
 	left_first = left_count < right_count;
 	init_patch_ids(&ids);
 	ids.diffopts.pathspec = revs->diffopt.pathspec;
+	ids.diffopts.context = revs->diffopt.context;
 
 	/* Compute patch-ids for one side */
 	for (p = list; p; p = p->next) {

^ permalink raw reply related

* Re: Dropping '+' from fetch = +refs/heads/*:refs/remotes/origin/*?
From: Jeff King @ 2011-09-02 15:29 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Junio C Hamano, git
In-Reply-To: <4E6088F9.5070102@drmicha.warpmail.net>

On Fri, Sep 02, 2011 at 09:42:49AM +0200, Michael J Gruber wrote:

> >     It would be considerably nicer if the server had some way of saying
> >     "I expect this branch to be rewound". Which has been discussed off
> >     and on over the years, as I recall.
> 
> Hmm, that sounds like the often requested feature to have part of the
> config distributed by "clone" as well, possibly after displaying it and
> getting user confirmation.

Yeah, if distributed config existed, that would be a good place to put
this information.

And I personally thing a limited form[1] of distributed config is a
sensible idea, but I'm not sure everybody else agrees.

-Peff

[1] My idea of "limited" would be an allow-known-good list of harmless
config keys which we would respect when they came from the remote, with
the option for the user to whitelist or blacklist more keys if they
wanted.

^ permalink raw reply

* Re: Dropping '+' from fetch = +refs/heads/*:refs/remotes/origin/*?
From: Jeff King @ 2011-09-02 15:26 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <4E607F27.2000405@viscovery.net>

On Fri, Sep 02, 2011 at 09:00:55AM +0200, Johannes Sixt wrote:

> >     It would be considerably nicer if the server had some way of saying
> >     "I expect this branch to be rewound". Which has been discussed off
> >     and on over the years, as I recall.
> 
> So, if such a feature were available, wouldn't it be nicer if the initial
> clone set up the refspec like this:
> 
>   [remote "origin"]
>         url = git://git.kernel.org/pub/scm/git/git.git
>         fetch = +refs/heads/*:refs/remotes/origin/*
>         fetch = refs/heads/maint:refs/remotes/origin/maint
>         fetch = refs/heads/master:refs/remotes/origin/master
> 
> i.e., the non-wildcard refspec are about which branches are *not* expected
> to be rewound rather than the other way around.

I don't see the advantage one way or the other. Doesn't it just amount
to what the default will be? And isn't "not rewind" generally the more
common, and hence a better default?

Or are you saying that for backwards compatibility, it would be better
to end up with a refspec more like what we have now? That I can see the
advantage of.

-Peff

^ permalink raw reply

* RE: IBM Rational Team Concert and Git
From: David Bainbridge @ 2011-09-02 14:22 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: git@vger.kernel.org
In-Reply-To: <CAGdFq_jNTgx=+GBzFJrvez30Y+p_C30jzrYB5QCTha+bM9Nvpw@mail.gmail.com>

Hi Sverre,

Sorry. I should have removed the statement about the mail being confidential. It's not confidential. It's definitely public!

You will need to register and create a Jazz.net account to be able to access the page. https://jazz.net/pub/user/register.jsp 
Note: I am not trying to promote RTC or Jazz in any way.

I posted this information to this list because there may be others with an interest in adding functionality this to RTC, in which case we may have some common ground. I am not sure that the Git development community would need to be involved in this at all. So you can treat it as being for information only, if you wish ...

Hope this explains it.

Regards

David


Ericsson

DAVID BAINBRIDGE
CM Area Manager

Ericsson AB
Product Line Engineering Environments
Torshamnsgatan 23
SE-164 80 Stockholm, Sweden
david.bainbridge@ericsson.com
www.ericsson.com



We only send and receive email on the basis of the terms set out at www.ericsson.com/email_disclaimer
-----Original Message-----
From: Sverre Rabbelier [mailto:srabbelier@gmail.com] 
Sent: den 2 september 2011 16:08
To: David Bainbridge
Cc: git@vger.kernel.org
Subject: Re: IBM Rational Team Concert and Git

Heya,

On Fri, Sep 2, 2011 at 11:26, David Bainbridge <david.bainbridge@ericsson.com> wrote:
> Ericsson has requested that IBM Rational provide a supported bridge to Git from IBM Rational Team Concert.
>
> A work item has been opened at Jazz.net:
> https://jazz.net/jazz/web/projects/Rational%20Team%20Concert#action=co
> m.ibm.team.workitem.viewWorkItem&id=159549

What does this mean? Is this something the git community can help with? I can't even access that page...

> This Communication is Confidential. We only send and receive email on 
> the basis of the terms set out at www.ericsson.com/email_disclaimer

Why are you sending a Confidential email to a public list?

--
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: GitTogether 2011
From: Patrick Renaud @ 2011-09-02 14:09 UTC (permalink / raw)
  To: git@vger.kernel.org
In-Reply-To: <CAC+LNESrs5gcP-JR3LriVznCtHgPz5EUGb51iHMH-2vbTGigFw@mail.gmail.com>

Hi there.

> Any news on the GitTogether front?

I'd also be interested in hearing the latest developments on this
front. We're getting near the end of the year and if this event does
not take place I would use that budget for another event instead. I'm
sure several other people share a similar concern, especially those
who need to travel from abroad in order to attend the conference.

Cheers!

--
-Patrick

^ permalink raw reply

* Re: IBM Rational Team Concert and Git
From: Sverre Rabbelier @ 2011-09-02 14:08 UTC (permalink / raw)
  To: David Bainbridge; +Cc: git@vger.kernel.org
In-Reply-To: <3A92A63EBFD41F4196707AF266E1CDA517BAE4EBBB@ESESSCMS0361.eemea.ericsson.se>

Heya,

On Fri, Sep 2, 2011 at 11:26, David Bainbridge
<david.bainbridge@ericsson.com> wrote:
> Ericsson has requested that IBM Rational provide a supported bridge to Git from IBM Rational Team Concert.
>
> A work item has been opened at Jazz.net:
> https://jazz.net/jazz/web/projects/Rational%20Team%20Concert#action=com.ibm.team.workitem.viewWorkItem&id=159549

What does this mean? Is this something the git community can help
with? I can't even access that page...

> This Communication is Confidential. We only send and receive email
> on the basis of the terms set out at www.ericsson.com/email_disclaimer

Why are you sending a Confidential email to a public list?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] test-lib: save test counts across invocations
From: Thomas Rast @ 2011-09-02 12:39 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20110901163846.GD15018@sigill.intra.peff.net>

Jeff King wrote:
> Anyway, this whole thing is a cute idea, and I do love eye candy, but I
> wonder if it's worth the complexity. All this is telling us is how far
> into each of the scripts it is. But we have literally hundreds of test
> scripts, all with varying numbers of tests of varying speeds, and you're
> probably running 16 or more at one time. So it doesn't tell you what you
> really want to know, which is: how soon will the test suite probably be
> done running.

I guess you're right.  Let's drop this, then.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: Special characters in file name
From: Alexey Shumkin @ 2011-09-02 12:38 UTC (permalink / raw)
  To: git
In-Reply-To: <4E60C6D0.7090609@intland.com>

Tajti Ákos <akos.tajti <at> intland.com> writes:

> 
> Dear List,
> 
> I have a file named "árvíz.txt" in my repository. When modify that file 
> and execute git diff, the first line looks like this:
> diff --git "a/\303\241rv\303\255z.txt" "b/\303\241rv\303\255z.txt"
> 
> Is there an option that (if specified) will get git to print "árvíz.txt" 
> instead of this escaped string?
> 
> Thanks in advance,
> Ákos Tajti
> 

please, refresh your memory )))
http://comments.gmane.org/gmane.comp.version-control.git/177849

see my comment
http://permalink.gmane.org/gmane.comp.version-control.git/177857

^ permalink raw reply

* Special characters in file name
From: Tajti Ákos @ 2011-09-02 12:06 UTC (permalink / raw)
  To: git@vger.kernel.org

Dear List,

I have a file named "árvíz.txt" in my repository. When modify that file 
and execute git diff, the first line looks like this:
diff --git "a/\303\241rv\303\255z.txt" "b/\303\241rv\303\255z.txt"

Is there an option that (if specified) will get git to print "árvíz.txt" 
instead of this escaped string?

Thanks in advance,
Ákos Tajti

^ permalink raw reply

* rev-list --cherry-pick and context lines
From: Stefan Haller @ 2011-09-02 10:35 UTC (permalink / raw)
  To: git

Consider two commits on different branches, one with this patch:

    diff --git a/file.txt b/file.txt
    index 704fa27..2f7e74c 100644
    --- a/file.txt
    +++ b/file.txt
    @@ -1,3 +1,3 @@
     old_context
     
    -foo
    +bar

and the other with this patch:

    diff --git a/file.txt b/file.txt
    index f35051b..8c7de32 100644
    --- a/file.txt
    +++ b/file.txt
    @@ -1,3 +1,3 @@
     new_context
     
    -foo
    +bar

If I run "git rev-list --cherry-pick --left-right branch1...branch2", it
reports both commits as being genuine commits on their respective
branch, even though I consider their patches to be the same.

I guess for my purpose I would like to have patch-ids that ignore
context (or that use only one line of context, I'm not sure which).

In fact, if I do "git show <commit> -U1 | git patch-id", both commits
show the same id.

So, would it make sense to have a parameter for git-rev-list (and
git-cherry) that lets you specify how much context to be used for the
patch ids?


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

^ permalink raw reply

* Re: [PATCH] sha1_file: Remove relative entries limitation
From: Hui Wang @ 2011-09-02  9:49 UTC (permalink / raw)
  To: Hui Wang; +Cc: gitster, git
In-Reply-To: <1314772896-22631-1-git-send-email-jason77.wang@gmail.com>

Hi Junio,

Does this patch make sense?

Regards,
Hui Wang.

Hui Wang wrote:
> link_alt_odb_entries() will be called recursively if alternates has
> valid object store paths, and to avoid nesting too deep, the
> recursive depth is limited to 5, this limitation is reasonable and
> safe for dead-loop reference situation.
>
> There is another limitation in this function to only permit the 1st
> level alternates has relative paths, but there is no foreseeable
> greater risk using relative paths in 2nd/3rd... level alternates than
> using absolute paths, in addition to we already have max depth 5
> limitation, we can safely remove this limitation.
>
> Moreover removing this limitation will make below two usage workable.
>
> usage1: base-repos has relative path in the alternates
>         %>git clone --reference base-repos src dest
> usage2: src2 has relative path to point src1, src1 has relative path
>     	to point src
>         %>git clone src2 dest
>
> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
> ---
>  sha1_file.c |   13 ++++---------
>  1 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index f7c3408..4130ca0 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -332,15 +332,10 @@ static void link_alt_odb_entries(const char *alt, const char *ep, int sep,
>  		}
>  		while (cp < ep && *cp != sep)
>  			cp++;
> -		if (last != cp) {
> -			if (!is_absolute_path(last) && depth) {
> -				error("%s: ignoring relative alternate object store %s",
> -						relative_base, last);
> -			} else {
> -				link_alt_odb_entry(last, cp - last,
> -						relative_base, depth);
> -			}
> -		}
> +		if (last != cp)
> +			link_alt_odb_entry(last, cp - last,
> +					relative_base, depth);
> +
>  		while (cp < ep && *cp == sep)
>  			cp++;
>  		last = cp;
>   

^ permalink raw reply

* IBM Rational Team Concert and Git
From: David Bainbridge @ 2011-09-02  9:26 UTC (permalink / raw)
  To: git@vger.kernel.org


Hi everyone,

Ericsson has requested that IBM Rational provide a supported bridge to Git from IBM Rational Team Concert.

A work item has been opened at Jazz.net:
https://jazz.net/jazz/web/projects/Rational%20Team%20Concert#action=com.ibm.team.workitem.viewWorkItem&id=159549

Regards

David Bainbridge

DAVID BAINBRIDGE  
CM Area Manager 

Ericsson AB
Product Line Engineering Environments
Torshamnsgatan 23
SE-164 80 Stockholm, Sweden
david.bainbridge@ericsson.com
www.ericsson.com  

This Communication is Confidential. We only send and receive email on the basis of the terms set out at www.ericsson.com/email_disclaimer  

^ permalink raw reply

* Re: Rebase & Trailing Whitespace
From: Jeff King @ 2011-09-02  8:28 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Philip Oakley, Hilco Wijbenga, Git Users
In-Reply-To: <4E6086AA.40806@drmicha.warpmail.net>

On Fri, Sep 02, 2011 at 09:32:58AM +0200, Michael J Gruber wrote:

> > Thanks. By itself, I think many readers would ask "why would I want the
> > empty tree, so I threw in a few examples of use on the wiki, too.
> 
> Ugh. I mean: Thanks for the wiki entries, they're nice, but have you
> checked git.git for whitespace errors? Time for another war on
> whitespace? Many false positives, of course, but also true positives.

Yeah, there are a lot. Because of the potential disruption to patches in
progress, though, we tend not to do big bulk updates of style changes.
But it may be worth using the

  rm foo.c
  git diff -R | git apply --whitespace=fix

trick if you are going to be working on foo.c. And if there are a lot,
you can "add -p" whatever fixes are appropriate for the area you're
working in.

-Peff

^ permalink raw reply

* Re: Dropping '+' from fetch = +refs/heads/*:refs/remotes/origin/*?
From: Michael J Gruber @ 2011-09-02  7:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20110902000039.GB9339@sigill.intra.peff.net>

Jeff King venit, vidit, dixit 02.09.2011 02:00:
> On Thu, Sep 01, 2011 at 11:25:31AM -0700, Junio C Hamano wrote:
> 
>> Suggested reading:
>>
>>   http://git-blame.blogspot.com/2011/08/how-to-inject-malicious-commit-to-git.html
>>
>> I am wondering if we are better off applying something along the lines of
>> this patch, so that with the default configuration, users can notice if
>> their upstream unexpectedly rewound their branches.
> 
> Hmm. This feels like it's subtly changing the meaning of
> refs/remotes/$remote/*.
> 
> Right now, I think of it as a local cache for whatever the remote side
> has. In other words, a way of separating the network-fetching parts of
> the workflow from the local parts. And in that sense, it is perfectly
> reasonable to overwrite with what the other side has, whether they
> rewind or not, because we are just representing what they have. And
> since we keep a reflog, it's not as if the previous state is lost to us
> locally.
> 
> But with this change, we are making a policy judgement about what to
> fetch. And as you noticed, it means that users need to start telling git
> about their policy (e.g., mentioning in the refspecs that pu can rewind)
> in order to keep fetch working.
> 
> So I consider that a downside, because it's extra work for the user[1].
> What are the upsides?
> 
> Is this about preventing workflow-related mistakes where people
> accidentally merge in rebased commits, creating annoying shadow
> histories?
> 
> Is it about preventing malicious rewinds from infecting downstream
> repositories?
> 
> If the former, then I suspect we need to give more guidance to the user
> than saying "reject, non-fast-forward". What then? Should they "fetch
> -f"?  Or "pull --rebase" (actually, how do they even fetch the branch
> now for such a pull --rebase)? Or talk out-of-band to the repo owner?
> 
> If the latter, then I think we should be specific about the attack
> scenarios, and what happens with and without this config. And if it's a
> security precaution, what cases doesn't it cover (e.g., initial clone is
> still vulnerable, as is a one-off pull. As are are malicious insertion
> attacks that don't involve rewinding).
> 
> And then we can weigh the upsides and the downsides.
> 
> -Peff
> 
> [1] What I really don't like is that cloning git.git is no longer:
> 
>       git clone git://git.kernel.org/pub/scm/git/git.git
> 
>     which is a minimal as it can be, but becomes:
> 
>       git clone git://git.kernel.org/pub/scm/git/git.git
>       cd git
>       git config --add remote.origin.fetch +refs/heads/pu:refs/remotes/origin/pu
> 
>     It's not that my fingers are too tired to do all that typing, but
>     rather that the first set of instructions is very easy to explain,
>     and the second one is full of magic and head-scratching about why
>     git isn't handling this magic itself.
> 
>     It would be considerably nicer if the server had some way of saying
>     "I expect this branch to be rewound". Which has been discussed off
>     and on over the years, as I recall.

Hmm, that sounds like the often requested feature to have part of the
config distributed by "clone" as well, possibly after displaying it and
getting user confirmation.

Michael

^ permalink raw reply

* Re: Rebase & Trailing Whitespace
From: Michael J Gruber @ 2011-09-02  7:32 UTC (permalink / raw)
  To: Jeff King; +Cc: Philip Oakley, Hilco Wijbenga, Git Users
In-Reply-To: <20110901212618.GB16308@sigill.intra.peff.net>

Jeff King venit, vidit, dixit 01.09.2011 23:26:
> On Thu, Sep 01, 2011 at 10:00:38PM +0100, Philip Oakley wrote:
> 
>>> [1] If you don't remember the empty tree sha1, you can always derive it
>>>   with:
>>>
>>>       git hash-object -t tree /dev/null
>>>
>>
>> I've added this tip to the
>> https://git.wiki.kernel.org/index.php/Aliases page
> 
> Thanks. By itself, I think many readers would ask "why would I want the
> empty tree, so I threw in a few examples of use on the wiki, too.

Ugh. I mean: Thanks for the wiki entries, they're nice, but have you
checked git.git for whitespace errors? Time for another war on
whitespace? Many false positives, of course, but also true positives.

Michael

^ permalink raw reply

* [PATCH] xdiff/xprepare: initialise xdlclassifier_t cf in xdl_prepare_env()
From: Tay Ray Chuan @ 2011-09-02  7:09 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano
In-Reply-To: <7vaaapzf6f.fsf@alter.siamese.dyndns.org>

Ensure that the xdl_free_classifier() call on xdlclassifier_t cf is safe
even if xdl_init_classifier() isn't called, which is the case when diff
is run with --histogram.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>

---

On Thu, Sep 1, 2011 at 1:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Tay Ray Chuan <rctay89@gmail.com> writes:
>
>> Ensure that the xdl_free_classifier() call on xdlclassifier_t cf is safe
>> even if xdl_init_classifier() isn't called. This may occur in the case
>> where diff is run with --histogram and a call to, say, xdl_prepare_ctx()
>> fails.
>>
>> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
>
> Thanks. Did you find this by code inspection?

If by "code inspection" you meant eye-balling, then yes. :)

On Thu, Sep 1, 2011 at 1:03 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> [snip]
>>> @@ -239,6 +239,9 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
>>>      long enl1, enl2, sample;
>>>      xdlclassifier_t cf;
>>>
>>> +    cf.rchash = NULL;
>>> +    cf.ncha.head = NULL;
>>
>> Would it be more appropriate to use memcpy(&cf, 0, sizeof(cf)) instead, so
>
> Oops, I meant memset(), obviously.

Right, thanks.
---
 xdiff/xprepare.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index 620fc9a..5b676b7 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -239,6 +239,8 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
 	long enl1, enl2, sample;
 	xdlclassifier_t cf;
 
+	memset(&cf, 0, sizeof(cf));
+
 	/*
 	 * For histogram diff, we can afford a smaller sample size and
 	 * thus a poorer estimate of the number of lines, as the hash
-- 
1.7.6.1.706.gaa5cf

^ permalink raw reply related

* Re: Dropping '+' from fetch = +refs/heads/*:refs/remotes/origin/*?
From: Johannes Sixt @ 2011-09-02  7:00 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20110902000039.GB9339@sigill.intra.peff.net>

Am 9/2/2011 2:00, schrieb Jeff King:
> Right now, I think of it as a local cache for whatever the remote side
> has. In other words, a way of separating the network-fetching parts of
> the workflow from the local parts.

This is also my interpretation. For this reason, I don't think a change is
necessary.

> So I consider that a downside, because it's extra work for the user[1].
> What are the upsides?
> 
> Is this about preventing workflow-related mistakes where people
> accidentally merge in rebased commits, creating annoying shadow
> histories?
> 
> Is it about preventing malicious rewinds from infecting downstream
> repositories?

All good questions to ask.

> [1] What I really don't like is that cloning git.git is no longer:
> 
>       git clone git://git.kernel.org/pub/scm/git/git.git
> 
>     which is a minimal as it can be, but becomes:
> 
>       git clone git://git.kernel.org/pub/scm/git/git.git
>       cd git
>       git config --add remote.origin.fetch +refs/heads/pu:refs/remotes/origin/pu
> 
>     It's not that my fingers are too tired to do all that typing, but
>     rather that the first set of instructions is very easy to explain,
>     and the second one is full of magic and head-scratching about why
>     git isn't handling this magic itself.

Absolutely.

>     It would be considerably nicer if the server had some way of saying
>     "I expect this branch to be rewound". Which has been discussed off
>     and on over the years, as I recall.

So, if such a feature were available, wouldn't it be nicer if the initial
clone set up the refspec like this:

  [remote "origin"]
        url = git://git.kernel.org/pub/scm/git/git.git
        fetch = +refs/heads/*:refs/remotes/origin/*
        fetch = refs/heads/maint:refs/remotes/origin/maint
        fetch = refs/heads/master:refs/remotes/origin/master

i.e., the non-wildcard refspec are about which branches are *not* expected
to be rewound rather than the other way around.

-- Hannes

^ permalink raw reply

* Re: Dropping '+' from fetch = +refs/heads/*:refs/remotes/origin/*?
From: Matthieu Moy @ 2011-09-02  5:55 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Michael J Gruber, Junio C Hamano, git
In-Reply-To: <CAJo=hJv4CkmaJuVvCA2VdO68zn4Xb9EQsdP8p1W-7B9zbvXSaQ@mail.gmail.com>

Shawn Pearce <spearce@spearce.org> writes:

> On Thu, Sep 1, 2011 at 12:35, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
>> By asking users to explicitely say "yes, I know, this branch can be
>> rewond", we also ask them to think about it before making a mistake.
>>
>> That said, enabling the check by default may also become painful. I'd
>> vote for a configuration option, defaulting to the current behavior for
>> now. Then we can try living with it for a while and see how painful it
>> is.
>
> I suspect the vast majority of branches in the wild do not rewind
> under normal conditions. Users who work against branches that rewind
> (e.g. those of us basing on a topic in pu)

Err, I don't think it's about people basing their work on pu, but rather
about anybody fetching from pu, i.e. everybody calling "git fetch" or
"git pull" in their clone.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: Cannot rewrite branch(es) with a dirty working directory
From: Matthieu Moy @ 2011-09-02  5:52 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, James Blackburn, git
In-Reply-To: <20110901215306.GD16308@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -108,9 +108,7 @@ OPTIONS_SPEC=
>  . git-sh-setup
>  
>  if [ "$(is_bare_repository)" = false ]; then
> -	git diff-files --ignore-submodules --quiet &&
> -	git diff-index --cached --quiet HEAD -- ||
> -	die "Cannot rewrite branch(es) with a dirty working directory."
> +	require_clean_work_tree 'rewrite branches'
>  fi
>  
>  tempdir=.git-rewrite

Sounds good, yes.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: Funnies with "git fetch"
From: Jeff King @ 2011-09-02  5:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vippbmygy.fsf@alter.siamese.dyndns.org>

On Thu, Sep 01, 2011 at 08:09:49PM -0700, Junio C Hamano wrote:

> You may be slightly misunderstanding the series.
> [...]
> The current code does not try to make sure we really have the objects
> necessary to connect the updated tips to our original refs at all.  Not
> just blobs but neither commits nor trees are traversed. The new check in
> store_updated_refs() is about that. So in that sense, the series is not
> about "just blobs".

Ah, OK, I see. I was too focused on pulling the bits out of quickfetch
into check_everything_connected, and missed the important new call in
store_updated_refs.

So what you are doing makes sense to me. I am curious, though, what the
performance impact is like. In particular, it seems that we will pull
each blob into memory via parse_object. Until now, we were mostly
streaming the blobs straight into packs. That makes me a little nervous
given the discussions recently about large blobs, and not accessing them
unnecessarily. But maybe that is a silly concern, as we will have just
reconstructed and hashed such an object anyway to get its name.

-Peff

^ permalink raw reply

* Re: [PATCH 3/3] fetch: verify we have everything we need before updating our ref
From: Junio C Hamano @ 2011-09-02  4:25 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8C4qX=p6d1v+i7TJanACOqL9R6-pv9AOaU_CM6DNjJfyQ@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> What about receive-pack? Does it have the same breakage?

I didn't look---you tell me ;-)

^ permalink raw reply

* Re: [PATCH 3/3] fetch: verify we have everything we need before updating our ref
From: Nguyen Thai Ngoc Duy @ 2011-09-02  3:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1314917015-3587-4-git-send-email-gitster@pobox.com>

On Fri, Sep 2, 2011 at 5:43 AM, Junio C Hamano <gitster@pobox.com> wrote:
> The "git fetch" command works in two phases. The remote side tells us what
> objects are at the tip of the refs we are fetching from, and transfers the
> objects missing from our side. After storing the objects in our repository,
> we update our remote tracking branches to point at the updated tips of the
> refs.
>
> A broken or malicious remote side could send a perfectly well-formed pack
> data during the object transfer phase, but there is no guarantee that the
> given data actually fill the gap between the objects we originally had and
> the refs we are updating to.

What about receive-pack? Does it have the same breakage?
-- 
Duy

^ permalink raw reply

* Re: Funnies with "git fetch"
From: Junio C Hamano @ 2011-09-02  3:28 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <7vippbmygy.fsf@alter.siamese.dyndns.org>

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

> Jeff King <peff@peff.net> writes:
>
>> If I understand correctly, your series is just about checking that we
>> have newly-referenced blobs. We were already checking commits and trees,
>> and we should already be hashing individual objects when we index the
>> pack. Right?
>
> You may be slightly misunderstanding the series.
>
> We let unpack-objects or index-pack consume the pack stream, either by
> exploding them into loose objects, or computing the object name for each
> object to create the mapping from object name to the offset. During this
> process, we deflate to read the contents and resolve the delta to come up
> with the object name for individual objects, so we would notice corruption
> at the individual object level. As pack stream does not say what name each
> object is (the recipient is expected to compute it), there is no "stream
> says it is object X but the data is actually for object Y" problem. The
> recipient does not even see "X"---all it sees is Y.

One thing I forgot to mention.

If you have receive.fsckobjects set, unpack-objects and index-pack are
called with the --strict option and causes fsck_objects() to run, so we
will also catch malformed commits and trees that way. But even then,
calling fsck_object() on a blob object always succeeds, so it is not a
real check.

We probably would want to flip the default for receive_fsck_objects to
true one of these days.

> The current code does not try to make sure we really have the objects
> necessary to connect the updated tips to our original refs at all.  Not
> just blobs but neither commits nor trees are traversed. The new check in
> store_updated_refs() is about that. So in that sense, the series is not
> about "just blobs".
>
> The "rev-list --verify-objects" patch is about "blob vs everything else".
> It is used in the existing quickfetch() check, and also the additional
> check in store_updated_refs(). The existing check we run with "--objects"
> is capable of detecting corruptions of commits and trees (as we had to be
> able to read them to discover objects they refer to), but that is not a
> sufficient check if we worry about missing blobs.

I am debating myself if we want to also add calls to fsck_object() to the
new codepath. An additional patch on top of [2/3] would look like this
(totally untested).

It would make "rev-list --verify-objects" useful independently from its
use in the context of "git fetch", and more importantly, what it checks in
the context of "git fetch", while it is related, is more or less
orthogonal to what receive.fsckobjects checks. The check done during the
transfer is to check the set of objects the other side threw at us. The
check done in the check_everything_connected() by calling "rev-list
--verify-objects" is to check the set of objects we are actually going to
use. The former _should_ be superset of the latter, but [3/3] is about
making sure that the former indeed is the superset of the latter.

 builtin/rev-list.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index ab3be7c..bd49615 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -7,6 +7,7 @@
 #include "log-tree.h"
 #include "graph.h"
 #include "bisect.h"
+#include "fsck.h"
 
 static const char rev_list_usage[] =
 "git rev-list [OPTION] <commit-id>... [ -- paths... ]\n"
@@ -53,6 +54,11 @@ static void show_commit(struct commit *commit, void *data)
 	struct rev_info *revs = info->revs;
 
 	graph_show_commit(revs->graph);
+	if (revs->verify_objects) {
+		if (!commit->object.parsed)
+			parse_object(commit->object.sha1);
+		fsck_object(&commit->object, 1, fsck_error_function);
+	}
 
 	if (revs->count) {
 		if (commit->object.flags & PATCHSAME)
@@ -183,8 +189,11 @@ static void show_object(struct object *obj,
 	struct rev_info *info = cb_data;
 
 	finish_object(obj, path, component, cb_data);
-	if (info->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)
-		parse_object(obj->sha1);
+	if (info->verify_objects && obj->type != OBJ_COMMIT) {
+		if (!obj->parsed)
+			parse_object(obj->sha1);
+		fsck_object(obj, 1, fsck_error_function);
+	}
 	show_object_with_name(stdout, obj, path, component);
 }
 

^ permalink raw reply related

* Re: Funnies with "git fetch"
From: Junio C Hamano @ 2011-09-02  3:09 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20110901233108.GA9339@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> If I understand correctly, your series is just about checking that we
> have newly-referenced blobs. We were already checking commits and trees,
> and we should already be hashing individual objects when we index the
> pack. Right?

You may be slightly misunderstanding the series.

We let unpack-objects or index-pack consume the pack stream, either by
exploding them into loose objects, or computing the object name for each
object to create the mapping from object name to the offset. During this
process, we deflate to read the contents and resolve the delta to come up
with the object name for individual objects, so we would notice corruption
at the individual object level. As pack stream does not say what name each
object is (the recipient is expected to compute it), there is no "stream
says it is object X but the data is actually for object Y" problem. The
recipient does not even see "X"---all it sees is Y.

The current code does not try to make sure we really have the objects
necessary to connect the updated tips to our original refs at all.  Not
just blobs but neither commits nor trees are traversed. The new check in
store_updated_refs() is about that. So in that sense, the series is not
about "just blobs".

The "rev-list --verify-objects" patch is about "blob vs everything else".
It is used in the existing quickfetch() check, and also the additional
check in store_updated_refs(). The existing check we run with "--objects"
is capable of detecting corruptions of commits and trees (as we had to be
able to read them to discover objects they refer to), but that is not a
sufficient check if we worry about missing blobs.

^ permalink raw reply

* Re: git-checkout silently throws away the dirty status of index without a warning?
From: Andrew Ardill @ 2011-09-02  1:52 UTC (permalink / raw)
  To: Tzu-Jung Lee; +Cc: Junio C Hamano, git
In-Reply-To: <CAEvN+1hA1mUjDc0cQidEBOf5LhJ9LrVbTjyV5qPkUKNM6f7urQ@mail.gmail.com>

> I accidentally switch to the switched-to branch without committing the
> intermediate status.
> The 'work' was gone, and I had no idea how to bring it back.

Forgive me if I am missing something here, but can't you use reflog to
go back to the point just before you checked the switched-to branch
out?

I guess the question is in general, how do you revert to a previous
instance of the working index AND previous checked out branch? I don't
know the 'switch branch with modified working index' mapping well
enough to be able to determine if there is a bijection possible, a
concise description of it would probably show us. If not, the only way
to achieve this is to keep the history available.

Regards,

Andrew Ardill

^ 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