builtin-fetch code with messy history

builtin-fetch code with messy history

[Daniel Barkalow]

In my branch at: git://iabervon.org/~barkalow/git builtin-fetch

I have a bunch of not-for-merging history leading up to a C version of 
fetch which passes all of the tests except that:

 * it might be fetching too much with --depth.
 * bundle isn't implemented.
 * when a branch config file section refers to a branches/* remote, the 
   merge setting is used (if one is given), even though this isn't useful 
   either way.
 * branch.<name>.merge is treated like the source side of a refspec 
   (although it cannot be a wildcard), and it fetched refs are marked for 
   merging if they name the same ref, even if they are not character-wise 
   identical to the source side of the refspec used to fetch them.

I've got more work to do on it (such as actually using my parser for 
parsing the refspecs and making the logic less convoluted), but I thought 
I'd report my progress.

I'll give a sign-off to anyone who extracts patches that can be applied, 
if anybody wants to.

	-Daniel
*This .sig left intentionally blank*

Re: builtin-fetch code with messy history

[Johannes Schindelin]

Hi,

On Tue, 19 Jun 2007, Daniel Barkalow wrote:

> In my branch at: git://iabervon.org/~barkalow/git builtin-fetch
> 
> I have a bunch of not-for-merging history leading up to a C version of 
> fetch which passes all of the tests except that:
> 
>  * it might be fetching too much with --depth.

That should be fixable. (If I get more time this week than I expect, I'll 
do it myself.)

>  * bundle isn't implemented.

That's an easy one.

>  * when a branch config file section refers to a branches/* remote, the 
>    merge setting is used (if one is given), even though this isn't useful 
>    either way.

Maybe this is the right time to cut off branches/* and remotes/*?

>  * branch.<name>.merge is treated like the source side of a refspec 
>    (although it cannot be a wildcard), and it fetched refs are marked for 
>    merging if they name the same ref, even if they are not character-wise 
>    identical to the source side of the refspec used to fetch them.
> 
> I've got more work to do on it (such as actually using my parser for 
> parsing the refspecs and making the logic less convoluted), but I 
> thought I'd report my progress.

Thank you very much!
Dscho

Re: builtin-fetch code with messy history

[Alex Riesen]

On 6/19/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >  * when a branch config file section refers to a branches/* remote, the
> >    merge setting is used (if one is given), even though this isn't useful
> >    either way.
>
> Maybe this is the right time to cut off branches/* and remotes/*?
>

Seconded. Don't use the remotes/ since some months now.
But... isn't a git package with code of something like 1.4.4 is still
in some major distributions?

Re: builtin-fetch code with messy history

[Johannes Schindelin]

Hi,

On Tue, 19 Jun 2007, Alex Riesen wrote:

> On 6/19/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > >  * when a branch config file section refers to a branches/* remote, the
> > >    merge setting is used (if one is given), even though this isn't useful
> > >    either way.
> > 
> > Maybe this is the right time to cut off branches/* and remotes/*?
> 
> Seconded. Don't use the remotes/ since some months now.
> But... isn't a git package with code of something like 1.4.4 is still
> in some major distributions?

Yes, AFAICT it is Ubuntu "the most up-to-date distro there is". At least 
many questions on the list and in IRC suggest that.

So, how about checking (at least for a year) in builtin-fetch, if 
"branches/" or "remotes/" exist, and fail, with a nice message how to move 
to config-based remotes?

Ciao,
Dscho

Re: builtin-fetch code with messy history

[Alex Riesen]

On 6/19/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > >
> > > Maybe this is the right time to cut off branches/* and remotes/*?
> >
> > Seconded. Don't use the remotes/ since some months now.
> > But... isn't a git package with code of something like 1.4.4 is still
> > in some major distributions?
>
> Yes, AFAICT it is Ubuntu "the most up-to-date distro there is". At least
> many questions on the list and in IRC suggest that.
>
> So, how about checking (at least for a year) in builtin-fetch, if
> "branches/" or "remotes/" exist, and fail, with a nice message how to move
> to config-based remotes?

...by suggesting to use a nice conversion script which we don't
have.

BTW, as far as I can see, git-remote reads old configuration just fine,
so it probably will a very simple script: read all and write all. Well,
the "all" part of it can be a bit complicated, but aside from that...

Re: builtin-fetch code with messy history

[Johannes Schindelin]

Hi,

On Tue, 19 Jun 2007, Alex Riesen wrote:

> On 6/19/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > >
> > > > Maybe this is the right time to cut off branches/* and remotes/*?
> > >
> > > Seconded. Don't use the remotes/ since some months now.
> > > But... isn't a git package with code of something like 1.4.4 is still
> > > in some major distributions?
> > 
> > Yes, AFAICT it is Ubuntu "the most up-to-date distro there is". At least
> > many questions on the list and in IRC suggest that.
> > 
> > So, how about checking (at least for a year) in builtin-fetch, if
> > "branches/" or "remotes/" exist, and fail, with a nice message how to move
> > to config-based remotes?
> 
> ...by suggesting to use a nice conversion script which we don't
> have.

git.git/contrib/remotes2config.sh

IIRC it was explicitely asked to leave branches/* out of that script, but 
the original version had it, and it should be easy to include it again.

> BTW, as far as I can see, git-remote reads old configuration just fine,
> so it probably will a very simple script: read all and write all. Well,
> the "all" part of it can be a bit complicated, but aside from that...

Ciao,
Dscho

Re: builtin-fetch code with messy history

[Alex Riesen]

On 6/19/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > On 6/19/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > > >
> > > > > Maybe this is the right time to cut off branches/* and remotes/*?
> > > >
> > > > Seconded. Don't use the remotes/ since some months now.
> > > > But... isn't a git package with code of something like 1.4.4 is still
> > > > in some major distributions?
> > >
> > > Yes, AFAICT it is Ubuntu "the most up-to-date distro there is". At least
> > > many questions on the list and in IRC suggest that.
> > >
> > > So, how about checking (at least for a year) in builtin-fetch, if
> > > "branches/" or "remotes/" exist, and fail, with a nice message how to move
> > > to config-based remotes?
> >
> > ...by suggesting to use a nice conversion script which we don't
> > have.
>
> git.git/contrib/remotes2config.sh
>
> IIRC it was explicitely asked to leave branches/* out of that script, but
> the original version had it, and it should be easy to include it again.

Right, missed it. Sounds like we have a plan

Re: builtin-fetch code with messy history

[Jakub Narebski]

Johannes Schindelin wrote:

>>  * when a branch config file section refers to a branches/* remote, the 
>>    merge setting is used (if one is given), even though this isn't useful 
>>    either way.
> 
> Maybe this is the right time to cut off branches/* and remotes/*?

We should read branches/* and remotes/* for a long time; even if people
are using new version of git, their repositories are not necessarily
converted (and IIRC there is only script for remotes -> config, in contrib,
not in core git).

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: builtin-fetch code with messy history

[Daniel Barkalow]

On Tue, 19 Jun 2007, Johannes Schindelin wrote:

> Hi,
> 
> On Tue, 19 Jun 2007, Daniel Barkalow wrote:
> 
> > In my branch at: git://iabervon.org/~barkalow/git builtin-fetch
> > 
> > I have a bunch of not-for-merging history leading up to a C version of 
> > fetch which passes all of the tests except that:
> > 
> >  * it might be fetching too much with --depth.
> 
> That should be fixable. (If I get more time this week than I expect, I'll 
> do it myself.)

I just haven't taken the time to look at what it's supposed to do exactly, 
since I wasn't paying attention to the discussions there.

> >  * bundle isn't implemented.
> 
> That's an easy one.

Yeah, just a section in transport.c about it, but the functions I need 
aren't available directly and I got distracted until I was looking at my 
list of what tests I'd disabled.

> >  * when a branch config file section refers to a branches/* remote, the 
> >    merge setting is used (if one is given), even though this isn't useful 
> >    either way.
> 
> Maybe this is the right time to cut off branches/* and remotes/*?

It's not actually too difficult to support them, except for some weird 
combination cases that nobody would do anyway. I just made the remote.c 
config file parser generate the corresponding configurations from them, 
and the rest of the code doesn't have to care. The only oddity is that I 
had to support having a remote always auto-follow tags, even without 
tracking branches, because that's what branches/* did. But this is 
probably a reasonable thing to support as an option anyway.

	-Daniel
*This .sig left intentionally blank*

Re: builtin-fetch code with messy history

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

>> >  * when a branch config file section refers to a branches/* remote, the 
>> >    merge setting is used (if one is given), even though this isn't useful 
>> >    either way.
>> 
>> Maybe this is the right time to cut off branches/* and remotes/*?
>
> It's not actually too difficult to support them, except for some weird 
> combination cases that nobody would do anyway. I just made the remote.c 
> config file parser generate the corresponding configurations from them, 
> and the rest of the code doesn't have to care. The only oddity is that I 
> had to support having a remote always auto-follow tags, even without 
> tracking branches, because that's what branches/* did. But this is 
> probably a reasonable thing to support as an option anyway.

We should support repositories with older layouts for an
eternity in git timescale (that is usually 6mo to a year) after
announcing that they are deprecated (which hasn't happened yet,
but I think everybody agrees that deprecating branches/ and
remotes/ is a sane thing to do in 1.6.0 or so).  Even if we give
clear migration path and script, having to convert them all at
once is a nuisance for the users.

Re: builtin-fetch code with messy history

[Johannes Schindelin]

Hi,

On Tue, 19 Jun 2007, Daniel Barkalow wrote:

> On Tue, 19 Jun 2007, Johannes Schindelin wrote:
> 
> > 
> > On Tue, 19 Jun 2007, Daniel Barkalow wrote:
> > 
> > > In my branch at: git://iabervon.org/~barkalow/git builtin-fetch
> > > 
> > > I have a bunch of not-for-merging history leading up to a C version 
> > > of fetch which passes all of the tests except that:
> > > 
> > >  * it might be fetching too much with --depth.
> > 
> > That should be fixable. (If I get more time this week than I expect, 
> > I'll do it myself.)
> 
> I just haven't taken the time to look at what it's supposed to do 
> exactly, since I wasn't paying attention to the discussions there.

Come to think of it, I am not sure that it does the right thing in 
existing code either. There are still a bunch of emails regarding shallow 
clone in my inbox, awaiting calmer weather.

> > >  * bundle isn't implemented.
> > 
> > That's an easy one.
> 
> Yeah, just a section in transport.c about it, but the functions I need 
> aren't available directly and I got distracted until I was looking at my 
> list of what tests I'd disabled.

What I meant is not that it is so easy that you should have done it. I 
meant that this is so easy you should not bother with it, since I'll 
gladly step in once builtin-fetch is otherwise feature complete.

> > >  * when a branch config file section refers to a branches/* remote, the 
> > >    merge setting is used (if one is given), even though this isn't useful 
> > >    either way.
> > 
> > Maybe this is the right time to cut off branches/* and remotes/*?
> 
> It's not actually too difficult to support them, except for some weird 
> combination cases that nobody would do anyway. I just made the remote.c 
> config file parser generate the corresponding configurations from them, 
> and the rest of the code doesn't have to care. The only oddity is that I 
> had to support having a remote always auto-follow tags, even without 
> tracking branches, because that's what branches/* did. But this is 
> probably a reasonable thing to support as an option anyway.

As Junio said, I think in the interest of clean code we should  
deprecate that, and eventually get rid of it. We could do that even before 
builtin-fetch reaches 'next'...

Ciao,
Dscho