threads / discuss / 8648

builtin-fetch code with messy history

Subject: builtin-fetch code with messy history

## tl;dr

11 messages between Jun 19, 2007 and Jun 19, 2007.

replies: 10people: 5as markdown or json

Daniel Barkalow· Jun 19, 2007, 07:12 UTC · lore
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*
Johannes Schindelin· Jun 19, 2007, 09:40 UTC · re: Daniel Barkalow · lore

Re: builtin-fetch code with messy history

Hi,
On Tue, 19 Jun 2007, Daniel Barkalow wrote:
Show 6 quoted lines
> 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/*?
Show 8 quoted lines
>  * 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

Alex Riesen· Jun 19, 2007, 10:13 UTC · re: Johannes Schindelin · lore

Re: builtin-fetch code with messy history

On 6/19/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 6 quoted lines
> >  * 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?

Johannes Schindelin· Jun 19, 2007, 11:11 UTC · re: Alex Riesen · lore

Re: builtin-fetch code with messy history

Hi,
On Tue, 19 Jun 2007, Alex Riesen wrote:
Show 10 quoted lines
> 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

Alex Riesen· Jun 19, 2007, 11:47 UTC · re: Johannes Schindelin · lore

Re: builtin-fetch code with messy history

On 6/19/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 13 quoted lines
> > >
> > > 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...

Johannes Schindelin· Jun 19, 2007, 11:51 UTC · re: Alex Riesen · lore

Re: builtin-fetch code with messy history

Hi,
On Tue, 19 Jun 2007, Alex Riesen wrote:
Show 17 quoted lines
> 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

Alex Riesen· Jun 19, 2007, 11:57 UTC · re: Johannes Schindelin · lore

Re: builtin-fetch code with messy history

On 6/19/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 22 quoted lines
> > 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
Jakub Narebski· Jun 19, 2007, 16:46 UTC · re: Johannes Schindelin · lore

Re: builtin-fetch code with messy history

Johannes Schindelin wrote:
Show 5 quoted lines
>>  * 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
Daniel Barkalow· Jun 19, 2007, 16:49 UTC · re: Johannes Schindelin · lore

Re: builtin-fetch code with messy history

On Tue, 19 Jun 2007, Johannes Schindelin wrote:
Show 13 quoted lines
> 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.

Show 5 quoted lines
> >  * 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*
Junio C Hamano· Jun 19, 2007, 17:38 UTC · re: Daniel Barkalow · lore

Re: builtin-fetch code with messy history

Daniel Barkalow <barkalow@iabervon.org> writes:
Show 13 quoted lines
>> >  * 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.

Johannes Schindelin· Jun 19, 2007, 19:47 UTC · re: Daniel Barkalow · lore

Re: builtin-fetch code with messy history

Hi,
On Tue, 19 Jun 2007, Daniel Barkalow wrote:
Show 17 quoted lines
> 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.

Show 7 quoted lines
> > >  * 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.

Show 13 quoted lines
> > >  * 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

← back to recent threads