Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-26 Thread Johannes Schindelin
Hi Junio,

On Sun, 25 Nov 2012, Junio C Hamano wrote:

 From: Johannes Schindelin johannes.schinde...@gmx.de
 Subject: Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly
 Date: Fri, 2 Nov 2012 16:17:14 +0100 (CET)
 Message-ID: 
 alpine.deb.1.00.1211021612320.7...@s15462909.onlinehome-server.info
 
 (which is $gmane/208946) that says:
 
   Note that
 
   $ git branch foo master~1
   $ git fast-export foo master~1..master
 
   still does not update the foo ref, but a partial fix is better
   than no fix.

If you changed your stance on the patch Sverre and I sent to fix this, we
could get a non-partial fix for this. You wanted a fix for a bigger
problem, though, which I am unwilling to fix because it is not my itch to
scratch and I have to balance my time.

Ciao,
Johannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-26 Thread Junio C Hamano
Johannes Schindelin johannes.schinde...@gmx.de writes:

 If you changed your stance on the patch Sverre and I sent to fix this, we
 could get a non-partial fix for this.

This is long time ago so I may be misremembering the details, but I
thought the original patch was (ab)using object flags to mark this
was explicitly asked for, even though some other range operation may
have marked it uninteresting.  Because it predated the introduction
of the rev_cmdline_info mechanism to record what was mentioned on
the command line separately from what objects are uninteresting
(i.e. object flags), it may have been one convenient way to record
this information, but it still looked unnecessarily ugly hack to me,
in that it allocated scarce object flag bits to represent a narrow
special case (iirc, only a freestanding A on the command line but
not A spelled in B..A, or something), making it more expensive
to record other kinds of command line information in a way
consistent with the approach chosen (we do not want to waste object
flag bits in order to record this was right hand side tip of the
symmetric difference range and such).

If you are calling do not waste object flags to represent one
special case among endless number of possibilities, as it will make
it impossible to extend it my stance, that hasn't changed.

We added rev_cmdline_info since then so that we can tell what refs
were given from the command line in what way, and I thought that we
applied a patch from Sverre that uses it instead of the object
flags.  Am I misremembering things?

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-26 Thread Johannes Schindelin
Hi Junio,

On Mon, 26 Nov 2012, Junio C Hamano wrote:

 Johannes Schindelin johannes.schinde...@gmx.de writes:
 
  If you changed your stance on the patch Sverre and I sent to fix this,
  we could get a non-partial fix for this.
 
 This is long time ago so I may be misremembering the details, but I
 thought the original patch was (ab)using object flags to mark this was
 explicitly asked for, even though some other range operation may have
 marked it uninteresting.  Because it predated the introduction of the
 rev_cmdline_info mechanism to record what was mentioned on the command
 line separately from what objects are uninteresting (i.e. object flags),
 it may have been one convenient way to record this information, but it
 still looked unnecessarily ugly hack to me, in that it allocated scarce
 object flag bits to represent a narrow special case (iirc, only a
 freestanding A on the command line but not A spelled in B..A, or
 something), making it more expensive to record other kinds of command
 line information in a way consistent with the approach chosen (we do not
 want to waste object flag bits in order to record this was right hand
 side tip of the symmetric difference range and such).

Good to know. I will find some time to look at rev_cmdline_info and patch
my patch.

 If you are calling do not waste object flags to represent one
 special case among endless number of possibilities, as it will make
 it impossible to extend it my stance, that hasn't changed.
 
 We added rev_cmdline_info since then so that we can tell what refs
 were given from the command line in what way, and I thought that we
 applied a patch from Sverre that uses it instead of the object
 flags.  Am I misremembering things?

It does sound so familiar that I am intended to claim that you remember
things correctly.

Ciao,
Johannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-26 Thread Sverre Rabbelier
On Mon, Nov 26, 2012 at 11:26 AM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 We added rev_cmdline_info since then so that we can tell what refs
 were given from the command line in what way, and I thought that we
 applied a patch from Sverre that uses it instead of the object
 flags.  Am I misremembering things?

 It does sound so familiar that I am intended to claim that you remember
 things correctly.

FWIW, I implemented that in
http://thread.gmane.org/gmane.comp.version-control.git/184874 but
didn't do the work to get it merged.

-- 
Cheers,

Sverre Rabbelier
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-26 Thread Junio C Hamano
Sverre Rabbelier srabbel...@gmail.com writes:

 On Mon, Nov 26, 2012 at 11:26 AM, Johannes Schindelin
 johannes.schinde...@gmx.de wrote:
 We added rev_cmdline_info since then so that we can tell what refs
 were given from the command line in what way, and I thought that we
 applied a patch from Sverre that uses it instead of the object
 flags.  Am I misremembering things?

 It does sound so familiar that I am intended to claim that you remember
 things correctly.

 FWIW, I implemented that in
 http://thread.gmane.org/gmane.comp.version-control.git/184874 but
 didn't do the work to get it merged.

Ah, OK.  Should I expect an updated series then?  How would it
interact with the recent work by Felipe?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-25 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes:

 On Wed, Nov 21, 2012 at 8:48 PM, Jeff King p...@peff.net wrote:
 ...
 I would like to understand that that even means. What behavior is
 currently broken?

I do not know if this is the same as what Peff was referring to, but
I found this message in the discussion thread during my absense.

From: Johannes Schindelin johannes.schinde...@gmx.de
Subject: Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly
Date: Fri, 2 Nov 2012 16:17:14 +0100 (CET)
Message-ID: 
alpine.deb.1.00.1211021612320.7...@s15462909.onlinehome-server.info

(which is $gmane/208946) that says:

Note that

$ git branch foo master~1
$ git fast-export foo master~1..master

still does not update the foo ref, but a partial fix is better
than no fix.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-22 Thread Max Horn
On 21.11.2012, at 06:08, Junio C Hamano wrote:

 Jonathan Nieder jrnie...@gmail.com writes:
 
 Never mind that others have said that that's not the current interface
 (I don't yet see why it would be a good interface after a transition,
 but maybe it would be).  Still, hopefully that clarifies the intended
 meaning.
 
 Care to explain how the current interface is supposed to work, how
 fast-export and transport-helper should interact with remote helpers
 that adhere to the current interface, and how well/correctly the
 current implementation of these pieces work?

Yes, please!


 
 What I am trying to get at is to see where the problem lies.  Felipe
 sees bugs in the aggregated whole.  Is the root cause of the problems
 he sees some breakages in the current interface?  Is the interface
 designed right but the problem is that the implementation of the
 transport-helper is buggy and driving fast-export incorrectly?  Or is
 the implementation of the fast-export buggy and emitting wrong results,
 even though the transport-helper is driving fast-export correctly?
 Something else?
 
 I see Felipe keeps repeating that there are bugs, and keeps posting
 patches to change fast-export, but I haven't seen a concrete No,
 the reason why you see these problems is because you are not using
 the interface correctly; the currrent interface is fine.  Here is
 how you can fix your program from others.

I was wondering about the same, actually... Moreover, I started to try to 
understand more about this, but found this a bit difficult. Apparently I am 
primarily supposed to learn about remote helpers by reverse engineering the 
(sparsely commented, if at all) existing ones. The fact that remote helpers can 
implement different subsets of the feature spectrum complicates this further. 

Overall, my impression is that there are two kinds of remote helpers:

1) Some are git-to-git helpers, which allow access to another git repos via 
some intermediate media / protocol (via http, ssh, ...). Those use either 
connect, or fetch+push. They do not need marks, because they can use the git 
sha1s. Examples (together with the capabilities they claim to implement):

- remote-curl: fetch, option, push
- remote-ext: connect
- remote-fd: connect


2) Some are interfaces to foreign systems (bzr, hg, mediawiki, ...). They 
cannot use sha1s and must use marks (at least that is how I understand felipe's 
explanation). These tools use import combined with either export, or push. 
Examples:

- git-remote-mediawiki: import, push, refspec
(its capabilities command also prints list, but that seems to be a bug?)
- git-remote-hg: import, export, refspec, import-marks, export-marks
(both the msysgit one and felipe's
- git-remote-bzr: import, push
(the one from https://github.com/lelutin/git-remote-bzr)
- git-remote-bzr (felipe's): import, export, refspec, *import-marks, 
*export-marks
(but why the * ?)


Does that sound about right? If so, can somebody give me a hint when a type 2 
helper would use export and when push?

And while I am at it: git-remote-helpers.txt does not mention the export, 
import-marks and export-marks capabilities. Could somebody who knows what 
they do look into fixing that? Overall, that doc helped me a bit, but it is 
more a reference to somebody who already understands in detail how remote 
helpers work, and who just wants to look up some specific detail :-(. Some 
hints on when to implement which capabilities might be useful (similar to the 
Tips and Tricks section in git-fast-import.txt).

As it is, felipe's recent explanation on why he thinks marks are essential for 
remote-helpers (I assume he was only referring to type 2 helpers, though) was 
one of the most enlightening texts I read on the whole subject so far (then 
again, I am fairly new to this list, so I may have missed lots of past 
goodness). Anyway, it would be nice if this could be augmented by somebody 
from the other camp ;).


Cheers,
Max

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-21 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes:

 They have been marked as UNINTERESTING for a reason, lets respect that.
 ...
 The current behavior is most certainly not what we want. After this
 patch, nothing gets exported, because nothing was selected (everything
 is UNINTERESTING).

The old behaviour was an incorrect workaround that has been
superseded by your 14/15 make sure updated refs get updated, no?
Mentioning that would help people realize that this patch would not
cause regression on them, I would think.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-21 Thread Jeff King
On Tue, Nov 20, 2012 at 09:08:36PM -0800, Junio C Hamano wrote:

 With such a one-sided discussion, I've been having a hard time
 convincing myself if Felipe's effort is making the interface better,
 or just breaking it even more for existing remote helpers, only to
 fit his world model better.

Felipe responded in more detail, but I will just add the consensus we
came to earlier in the discussion: the series does make things better
for users of fast-export that use marks, but does not make things any
better for users of negative refs on the command line. However, I do not
think that it makes things worse for them, either (neither by changing
the behavior negatively, nor by making the code harder for a more
complete fix later).

So while fixing everybody might be nice, there is no need to hold up
progress for the marks case. Which, as he has noted, is probably the
sanest way to implement a remote-helper[1].

-Peff

[1] There are other possible use cases for fast-export which might
benefit from negative refs working more sanely, but since they are
in the minority and are not being made worse, I think the partial
fix is OK.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-20 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes:

 Of course, transport-helper shouldn't even be specifying the negative
 (^) refs, but that's another story.

Hrm, I am not sure I understand what you mean by this.

How should it be telling the fast-export up to what commit the
receiving end should already have the history for (hence they do not
need to be sent)?  Or are you advocating to re-send the entire
history down to the root commit every time?


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-20 Thread Jonathan Nieder
Junio C Hamano wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 Of course, transport-helper shouldn't even be specifying the negative
 (^) refs, but that's another story.

 Hrm, I am not sure I understand what you mean by this.

 How should it be telling the fast-export up to what commit the
 receiving end should already have the history for (hence they do not
 need to be sent)?  Or are you advocating to re-send the entire
 history down to the root commit every time?

I think Felipe has mentioned before that he considers it the remote
helper's responsibility to keep track of what commits have already
been imported, for example using a marks file.

Never mind that others have said that that's not the current interface
(I don't yet see why it would be a good interface after a transition,
but maybe it would be).  Still, hopefully that clarifies the intended
meaning.

Hope that helps,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-20 Thread Felipe Contreras
On Wed, Nov 21, 2012 at 5:17 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 Junio C Hamano wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 Of course, transport-helper shouldn't even be specifying the negative
 (^) refs, but that's another story.

 Hrm, I am not sure I understand what you mean by this.

 How should it be telling the fast-export up to what commit the
 receiving end should already have the history for (hence they do not
 need to be sent)?  Or are you advocating to re-send the entire
 history down to the root commit every time?

 I think Felipe has mentioned before that he considers it the remote
 helper's responsibility to keep track of what commits have already
 been imported, for example using a marks file.

It's not the remote helper, fast-export does that.

 Never mind that others have said that that's not the current interface
 (I don't yet see why it would be a good interface after a transition,
 but maybe it would be).  Still, hopefully that clarifies the intended
 meaning.

The current interface is broken.

not ok 16 - pulling without marks # TODO known breakage
not ok 17 - pushing without marks # TODO known breakage

See? A remote helper without marks doesn't work.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-20 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 Never mind that others have said that that's not the current interface
 (I don't yet see why it would be a good interface after a transition,
 but maybe it would be).  Still, hopefully that clarifies the intended
 meaning.

Care to explain how the current interface is supposed to work, how
fast-export and transport-helper should interact with remote helpers
that adhere to the current interface, and how well/correctly the
current implementation of these pieces work?

What I am trying to get at is to see where the problem lies.  Felipe
sees bugs in the aggregated whole.  Is the root cause of the problems
he sees some breakages in the current interface?  Is the interface
designed right but the problem is that the implementation of the
transport-helper is buggy and driving fast-export incorrectly?  Or is
the implementation of the fast-export buggy and emitting wrong results,
even though the transport-helper is driving fast-export correctly?
Something else?

I see Felipe keeps repeating that there are bugs, and keeps posting
patches to change fast-export, but I haven't seen a concrete No,
the reason why you see these problems is because you are not using
the interface correctly; the currrent interface is fine.  Here is
how you can fix your program from others.

With such a one-sided discussion, I've been having a hard time
convincing myself if Felipe's effort is making the interface better,
or just breaking it even more for existing remote helpers, only to
fit his world model better.

Help?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs

2012-11-20 Thread Felipe Contreras
On Wed, Nov 21, 2012 at 6:08 AM, Junio C Hamano gits...@pobox.com wrote:

 I see Felipe keeps repeating that there are bugs, and keeps posting
 patches to change fast-export, but I haven't seen a concrete No,
 the reason why you see these problems is because you are not using
 the interface correctly; the currrent interface is fine.  Here is
 how you can fix your program from others.

 With such a one-sided discussion, I've been having a hard time
 convincing myself if Felipe's effort is making the interface better,
 or just breaking it even more for existing remote helpers, only to
 fit his world model better.

IIRC you mentioned something about this mailing list being focused on
*technical* merit. I've explained as much as I could, but at the end
of the talk, talk is cheap, the code speaks for itself. I added a new
very very very simple testgit remote helper, so anybody can see what's
going on, and figure out how the interface could be used wrong.

Anybody can modify the bash version of git-remote-testgit and say 'no,
the interface is not broken, here is how you push and pull without
marks'. How hard is it to hack 82 lines of bash code?

But lets assume my testgit is fatally broken, would you, Junio, accept
these patches if I show the same broken behavior with the python
git-remote-testgit?

I'm afraid I have to point out the hard truth; the reason why nobody
is doing that is because a) the interface is truly broken b) if they
try, they most likely would fail, and that would prove they were wrong
in previous discussion, or c) not enough familiarity with the code. I
don't want to point fingers, nor do I intend to offend anybody, but I
cannot find any other explanation of why this patch series, which is
obviously correct (to me), doesn't receive any feedback, even though
in theory, it should be very very very easy to show what's wrong with
the series.

The tests are there, and the remote helper is as simple as it gets.
There's nothing else but fast-export and transport-helper to blame for
the issues. It's that simple.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html