Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs
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
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
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
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
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
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
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
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
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
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
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
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
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
