Re: Error using git-remote-hg

2014-05-13 Thread Charles Brossollet
Le 12 mai 2014 à 21:37, Felipe Contreras felipe.contre...@gmail.com a écrit :

 Torsten Bögershausen wrote:
 I'm using git 1.9.3 on Mac OS X 10.9.2, with hg 3.0 installed with brew.
 
 It used to work before, on this same repository, since then git and hg were 
 both upgraded.
 In short: The remote helper of Git 1.9.3 is not compatible with hg 3.0
 You can eiher downgrade hg, or rebuild Git and cherry-pick this commit:
 
 No need to rebuild Git.
 
 You can also use the latest version:
 https://github.com/felipec/git-remote-hg

Thanks Felipe and Torsten, just using the HEAD version of git-remote-hg solved 
the problem.

Best regards,
Charles--
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: Error using git-remote-hg

2014-05-13 Thread Torsten Bögershausen
(Please use reply-all and don't snip the important stuff)
On 2014-05-13 09.54, Charles Brossollet wrote:
 Le 12 mai 2014 à 21:37, Felipe Contreras felipe.contre...@gmail.com a écrit 
 :
 
 Torsten Bögershausen wrote:
 I'm using git 1.9.3 on Mac OS X 10.9.2, with hg 3.0 installed with brew.

 It used to work before, on this same repository, since then git and hg 
 were both upgraded.
 In short: The remote helper of Git 1.9.3 is not compatible with hg 3.0
 You can eiher downgrade hg, or rebuild Git and cherry-pick this commit:


 No need to rebuild Git.

 You can also use the latest version:
 https://github.com/felipec/git-remote-hg
 
 Thanks Felipe and Torsten, just using the HEAD version of git-remote-hg 
 solved the problem.
 
 Best regards,
 Charles--

I did some testing with Git 2.0-rc3 + 58aee0864adeeb5363f.
The remote-helper tests for hg-git worked OK
with both hg version 2.9 and 3.0 under both Mac OS and Linux.

Should we consider 58aee086 to be included in Git 2.0 ?


--
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: Error using git-remote-hg

2014-05-13 Thread Junio C Hamano
Torsten Bögershausen tbo...@web.de writes:

 I did some testing with Git 2.0-rc3 + 58aee0864adeeb5363f.
 The remote-helper tests for hg-git worked OK
 with both hg version 2.9 and 3.0 under both Mac OS and Linux.

 Should we consider 58aee086 to be included in Git 2.0 ?

It is way too late for Git 2.0, unless we are willing to pretend
that it is before 2.0-rc1, and cut one rc release tarball or two and
delay the final release by a few weeks.  And I tested it with 2.9
and 3.0 is not the kind of test that is sufficient before the
release (e.g. my desktop seems to have 2.0.2-1ubuntu1); it is the
kind of test we want to see before the patch is sent to the list,
which I know Felipe did.

A clarification for new people might be in order, as there seem to
be some confusions in some recent threads.

Code freeze in preparation for the next release is *not* the time to
test new code.  It is time to catch regressions by asking wider
audiences who do not normally follow Git development (i.e. those who
are not the ones that follow 'master' and rebuild/install it once or
twice a week for their daily use).

These people (and I am one of them for other projects whose products
I use myself and whose development I do not follow) may be curious
what will be in the upcoming release, but what they care more about
is that the upcoming release will *not* break their established
usage.  If the version of Git they currently run allows them to do
something, and if the upcoming release stops them from doing that
thing they care, that will prevent them from using the new version
of Git.

That is what we want to catch by announcing release candidate
tarballs; in exchange for giving an early access to those who do not
always live on the edge by having a clone of git.git and following
its development, we want them to catch regressions and report to us,
so that we can fix (and the fix often is to revert when deep in
the release candidate cycles).

On the other hand, if their current Git did not allow them to do
something, and if the new version still does not, it is unfortunate
but it is the same flaw they have been living with.

My understanding is that versions of remote-hg shipped with all
versions of Git did not work with Hg 3.0, so 58aee08 is not a
regression fix at all.  Is working with Hg 3.0 at such a high
priority that it makes it worth to slip the whole release schedule
by a few weeks?  I do not think so.

Another thing to consider is that fewer and fewer people test later
release candidates, so issuing a 2.0-rc4 tarball that wasn't
scheduled and giving it a soak time of two weeks will not get as
wide a testing as we would get for 2.0-rc0/rc1.

So the answer to your question is an unambiguous no.

It is a different matter if we want a change to allow us to operate
with both older and newer version of mercurial in a release of Git
in near future.  The answer is a resounding yes, even if the
solution may not be the exact 58aee0864 commit [*2*].  I think an
update should eventurally go to the maintenance tracks of even older
releases, perhaps maint-1.8.5 and upwards, after we merge it to
'master' in preparation for the feature release that comes after Git
2.0, which probably will be called 2.1 but do not quote me on that.

Regarding the commit in question, I asked Felipe a question and
never got a straight answer.

Why do we import changegroup unconditionally, even though it
is only used in the new codepath meant only for version 3.0 or
higher, not inside the if block that decides if we need that
module?

I had a few more questions in mind that I didn't have a chance to
ask, and the commit log message did not explain.

Is the reason why this fix is needed is because repo stopped
being a way to ask .getbundle() and in the new world order
changegroup is the new way to ask .getbundle()?

If the answer is yes, then asking are we with 3.0 or
later---if so ask changegroup for .getbundle()?, which is this
patch does, may not be the right condition to trigger the new
codepath.  Does repo know how to do .getbundle()?  If not,
import changegroup and ask that module to perform .getbundle()
instead may be a way to base the decision on a more directly
relevant condition.

Answers to these questions, if they came, were meant to convince
myself that the patch 58aee0864 is the best solution to the problem,
but because I only got Of course it is not a mistake [*1*], seeing
it did not lead to a productive discussion, I gave up.  So I haven't
even managed to convince myself that that commit is the best
solution to the problem.


[References]

*1* http://thread.gmane.org/gmane.comp.version-control.git/248063/focus=248348

*2*
commit 58aee0864adeeb5363f2a06728596f9c9315811f
Author: Felipe Contreras felipe.contre...@gmail.com
Date:   Sat May 3 21:16:54 2014 -0500

remote-hg: add support for hg v3.0

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
Signed-off-by: Junio C 

Re: Error using git-remote-hg

2014-05-13 Thread Felipe Contreras
Junio C Hamano wrote:

 It is time to catch regressions by asking wider audiences who do not
 normally follow Git development (i.e. those who are not the ones that
 follow 'master' and rebuild/install it once or twice a week for their
 daily use).

And you have one of those regressions in Git v2.0.

 My understanding is that versions of remote-hg shipped with all
 versions of Git did not work with Hg 3.0, so 58aee08 is not a
 regression fix at all.  Is working with Hg 3.0 at such a high priority
 that it makes it worth to slip the whole release schedule by a few
 weeks?  I do not think so.

It is in the contrib/ area, you don't need to slip the schedule for
something that is not part of the core.

Moreover, it *CANNOT POSSIBLY INTRODUCE REGRESSIONS*. I bet my
reputation on that.

Some time ago I asked you to trust my judgement and introduce changes
late into a release, and I told you there wouldn't be any problems, and
to trust me. If any problems arise I wouldn't be asking for something
like that again.

Well, I was right, there were no problems. And I'm right this time too,
there would be no issues.

But you don't care about reality and facts. You don't care about the
intent behind the release-candidates rule, you would rather follow the
rule blindly.

 Regarding the commit in question, I asked Felipe a question and
 never got a straight answer.

Nor will you get one, because thanks to your stupid decision for which
you still haven't provided a rationale, the git-remote-hg and
git-remote-hg are no longer maintained in git.git.

If you hadn't made such a move, you would have your answer, the fix
would be properly explained, the regression fixed, and all your users
would be happy that git-remote-hg and git-remote-bzr get distributed by
default.

-- 
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: Error using git-remote-hg

2014-05-13 Thread William Giokas
On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote:
 Torsten Bögershausen tbo...@web.de writes:
 
  I did some testing with Git 2.0-rc3 + 58aee0864adeeb5363f.
  The remote-helper tests for hg-git worked OK
  with both hg version 2.9 and 3.0 under both Mac OS and Linux.
 
  Should we consider 58aee086 to be included in Git 2.0 ?
 
 So the answer to your question is an unambiguous no.
 
 It is a different matter if we want a change to allow us to operate
 with both older and newer version of mercurial in a release of Git
 in near future.  The answer is a resounding yes, even if the
 solution may not be the exact 58aee0864 commit [*2*].  I think an
 update should eventurally go to the maintenance tracks of even older
 releases, perhaps maint-1.8.5 and upwards, after we merge it to
 'master' in preparation for the feature release that comes after Git
 2.0, which probably will be called 2.1 but do not quote me on that.
 
 Regarding the commit in question, I asked Felipe a question and
 never got a straight answer.
 
 Why do we import changegroup unconditionally, even though it
 is only used in the new codepath meant only for version 3.0 or
 higher, not inside the if block that decides if we need that
 module?

It should not be, as it is not used outside of hg 3.0. In fact, what
would be even better would be to test whether changegroup.getbundle is
available, and then set a function like `getbundl()` to run either
`changegroup.getbundl()` with the correct args, or `repo.getbundle()` as
a fallback.

changegroup is prefectly /okay/ to import unconditionally, though as you
say it will never be used.

We can also be even more explicit with what we import by doing something
like::

  try:
  from mercurial.changegroup import getbundle

  except ImportError:
  def getbundle(__empty__, **kwargs):
  return repo.getbundle(**kwargs)

and then just calling::

  cg = getbundle(repo, 'push', heads=list(p_revs), common=common)

The `__empty__` arg is there because repo.getbundle uses a different
number of arguments than the changegroup.getbundle function. It's a much
cleaner solution than assuming that that will stay in changegroup, and
not get moved back to repo in the future. Seems to be what you described
in the second bit, though. If you would like I can submit a patch once
I've finished running the tests on it, and possibly after having Felipe
run it through his tests to be sure thta the 2.[7-9] series works as
well, and maybe you would want to test it on other versions of
mercurial that you are running alongside git.

Just my 2 cents.

 
 I had a few more questions in mind that I didn't have a chance to
 ask, and the commit log message did not explain.
 
 Is the reason why this fix is needed is because repo stopped
 being a way to ask .getbundle() and in the new world order
 changegroup is the new way to ask .getbundle()?
 
 If the answer is yes, then asking are we with 3.0 or
 later---if so ask changegroup for .getbundle()?, which is this
 patch does, may not be the right condition to trigger the new
 codepath.  Does repo know how to do .getbundle()?  If not,
 import changegroup and ask that module to perform .getbundle()
 instead may be a way to base the decision on a more directly
 relevant condition.
 
 Answers to these questions, if they came, were meant to convince
 myself that the patch 58aee0864 is the best solution to the problem,
 but because I only got Of course it is not a mistake [*1*], seeing
 it did not lead to a productive discussion, I gave up.  So I haven't
 even managed to convince myself that that commit is the best
 solution to the problem.

I was really sad to see that, and didn't have time to really look at it
because of work and other projects, but I hope this presents a better
solution than the current patch.

Thanks,

-- 
William Giokas | KaiSforza | http://kaictl.net/
GnuPG Key: 0x73CD09CF
Fingerprint: F73F 50EF BBE2 9846 8306  E6B8 6902 06D8 73CD 09CF


pgpC7ZLGG4hd9.pgp
Description: PGP signature


Re: Error using git-remote-hg

2014-05-13 Thread Felipe Contreras
William Giokas wrote:
 On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote:

  Why do we import changegroup unconditionally, even though it
  is only used in the new codepath meant only for version 3.0 or
  higher, not inside the if block that decides if we need that
  module? 

 changegroup is prefectly /okay/ to import unconditionally, though as you
 say it will never be used.

As you say, it's perfectly OK.

 We can also be even more explicit with what we import by doing something
 like::
 
   try:
   from mercurial.changegroup import getbundle
 
   except ImportError:
   def getbundle(__empty__, **kwargs):
   return repo.getbundle(**kwargs)

We could try that, but that would assume we want to maintain getbundle()
for the long run, and I personally don't want to do that. I would rather
contact the Mercurial developers about ways in which the push() method
can be improved so we don't need to have our own version. Hopefully
after it's improved we wouldn't have to call getbundle().

Moreover, eventually there will be a Mercurial 4.0, even 5.0, and at
some point we would want to remove the hacks for older versions. When we
do so we would want the import to remain unconditionally, and remove the
'check_version(3, 0)' which is already helping to explain what the code
is for without the need of comments.

 I was really sad to see that, and didn't have time to really look at it
 because of work and other projects, but I hope this presents a better
 solution than the current patch.

Either way Junio doesn't maintain this code, I do. And it's not
maintained in git.git, git's maintained out-of-tree (thanks to Junio's
decisions).

So please post your suggestions and patches to git...@googlegroups.com,
and use the latest code at https://github.com/felipec/git-remote-hg.

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


Re: Error using git-remote-hg

2014-05-13 Thread William Giokas
On Tue, May 13, 2014 at 02:09:55PM -0500, Felipe Contreras wrote:
 William Giokas wrote:
  On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote:
 
   Why do we import changegroup unconditionally, even though it
   is only used in the new codepath meant only for version 3.0 or
   higher, not inside the if block that decides if we need that
   module? 
 
  changegroup is prefectly /okay/ to import unconditionally, though as you
  say it will never be used.
 
 As you say, it's perfectly OK.

But wrong. Yes, it works, but it's not how it should be done when we
have a code review such as this. It should simply not be done and makes
no sense to do with an 'if check ver; else' kind of thing later in the
application.

 
  We can also be even more explicit with what we import by doing something
  like::
  
try:
from mercurial.changegroup import getbundle
  
except ImportError:
def getbundle(__empty__, **kwargs):
return repo.getbundle(**kwargs)
 
 We could try that, but that would assume we want to maintain getbundle()
 for the long run, and I personally don't want to do that. I would rather
 contact the Mercurial developers about ways in which the push() method
 can be improved so we don't need to have our own version. Hopefully
 after it's improved we wouldn't have to call getbundle().

Assuming that mercurial 3.0 will not change, then this should never
need to change. Changes in 'getbundle' upstream would require changes
either way.

 Moreover, eventually there will be a Mercurial 4.0, even 5.0, and at
 some point we would want to remove the hacks for older versions. When we
 do so we would want the import to remain unconditionally, and remove the
 'check_version(3, 0)' which is already helping to explain what the code
 is for without the need of comments.

The same exact thing can be done with this. In fact, it would probably
allow us to have better future-proofing with regards to new versions of
mercurial, there would just be different try:except statements at the
beginning.

 
  I was really sad to see that, and didn't have time to really look at it
  because of work and other projects, but I hope this presents a better
  solution than the current patch.
 
 Either way Junio doesn't maintain this code, I do. And it's not
 maintained in git.git, git's maintained out-of-tree (thanks to Junio's
 decisions).

I still see it in git.git, and I will contribute this upstream for as
long as it is in the tree. If you want to use the patch that I sent to
this list, feel free.

 So please post your suggestions and patches to git...@googlegroups.com,
 and use the latest code at https://github.com/felipec/git-remote-hg.

Thanks,

-- 
William Giokas | KaiSforza | http://kaictl.net/
GnuPG Key: 0x73CD09CF
Fingerprint: F73F 50EF BBE2 9846 8306  E6B8 6902 06D8 73CD 09CF


pgp_mLEPyhosF.pgp
Description: PGP signature


Re: Error using git-remote-hg

2014-05-13 Thread Felipe Contreras
William Giokas wrote:
 On Tue, May 13, 2014 at 02:09:55PM -0500, Felipe Contreras wrote:
  William Giokas wrote:
   On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote:
  
Why do we import changegroup unconditionally, even though it
is only used in the new codepath meant only for version 3.0 or
higher, not inside the if block that decides if we need that
module? 
  
   changegroup is prefectly /okay/ to import unconditionally, though as you
   say it will never be used.
  
  As you say, it's perfectly OK.
 
 But wrong. Yes, it works, but it's not how it should be done when we
 have a code review such as this. It should simply not be done and makes
 no sense to do with an 'if check ver; else' kind of thing later in the
 application.

That's exactly how it should be done. Maybe this visualization helps

  from mercurial import hg, ui, bookmarks, ...# for hg = 3.0
  from mercurial import node, error, extensions, ...  # for hg = 3.0
  from mercurial import changegroup   # for hg = 3.0

  if check_version(3, 0):
  cg = changegroup.getbundle(repo, 'push', ...# for hg = 3.0
  else:
  cg = repo.getbundle('push', heads=list(p_revs)  # for hg  3.0

Eventually the code would become:

  from mercurial import hg, ui, bookmarks, ...# for hg = 3.0
  from mercurial import node, error, extensions, ...  # for hg = 3.0
  from mercurial import changegroup   # for hg = 3.0

  cg = changegroup.getbundle(repo, 'push', ...# for hg = 3.0

The fact that at some point 'import changegroup' was not needed is
irrelevant.

Primarily we want to support the current version of Mercurial.
Secondarily we want to support older versions. That's why we add the
repo.getbundle() code (ass an addendum to the core part).

   We can also be even more explicit with what we import by doing something
   like::
   
 try:
 from mercurial.changegroup import getbundle
   
 except ImportError:
 def getbundle(__empty__, **kwargs):
 return repo.getbundle(**kwargs)
  
  We could try that, but that would assume we want to maintain getbundle()
  for the long run, and I personally don't want to do that. I would rather
  contact the Mercurial developers about ways in which the push() method
  can be improved so we don't need to have our own version. Hopefully
  after it's improved we wouldn't have to call getbundle().
 
 Assuming that mercurial 3.0 will not change, then this should never
 need to change.

But it should. Otherwise the code will keep having more and more hacks
and it will become less and less maintainable.

That's why we don't have code for hg 1.0; because it would require too
many hacks, and nobody cared anyway.

Just like nobody cares about hg 1.0, eventually nobody will care about
hg 2.0.

 Changes in 'getbundle' upstream would require changes either way.

I doubt we will see any more changes in getbundle, at least not until
4.0, and hopefully by then we wouldn't be using it anyway. I am willing
 to bet we won't see those changes.

  Moreover, eventually there will be a Mercurial 4.0, even 5.0, and at
  some point we would want to remove the hacks for older versions. When we
  do so we would want the import to remain unconditionally, and remove the
  'check_version(3, 0)' which is already helping to explain what the code
  is for without the need of comments.
 
 The same exact thing can be done with this. In fact, it would probably
 allow us to have better future-proofing with regards to new versions of
 mercurial, there would just be different try:except statements at the
 beginning.

No, your code doesn't show that this is for versiosn = 3.0,
'check_version(3, 0)' does.

Plus, when we remove this code my version makes it straight forward:

-if check_version(3, 0):
-cg = changegroup.getbundle(repo, 'push', ...
-else:
-cg = repo.getbundle('push', heads=list(p_revs), ...
+cg = changegroup.getbundle(repo, 'push', ...

Not so with your code:

-
-try:
-from mercurial.changegroup import getbundle
-
-except ImportError:
-def getbundle(__empty__, **kwargs):
-return repo.getbundle(**kwargs)
+from mercurial import getbundle
 
 import re
 import sys
@@ -1036,7 +1030,7 @@ def push_unsafe(repo, remote, ...
 if not checkheads(repo, remote, p_revs):
 return None
 
-cg = getbundle(repo, 'push', heads=list(p_revs), ...
+cg = changegroup.getbundle(repo, 'push', ...


   I was really sad to see that, and didn't have time to really look at it
   because of work and other projects, but I hope this presents a better
   solution than the current patch.
  
  Either way Junio doesn't maintain this code, I do. And it's not
  maintained in git.git, git's maintained out-of-tree (thanks to Junio's
  decisions).
 
 I still see it in git.git, and I will contribute this upstream for as
 long as it is in the tree.

And what happens when it's eventually removed?

 If you 

Re: Error using git-remote-hg

2014-05-13 Thread William Giokas
On Tue, May 13, 2014 at 03:24:51PM -0500, Felipe Contreras wrote:
 William Giokas wrote:
  On Tue, May 13, 2014 at 02:09:55PM -0500, Felipe Contreras wrote:
   As you say, it's perfectly OK.
  
  But wrong. Yes, it works, but it's not how it should be done when we
  have a code review such as this. It should simply not be done and makes
  no sense to do with an 'if check ver; else' kind of thing later in the
  application.
 
 That's exactly how it should be done. Maybe this visualization helps
 
   from mercurial import hg, ui, bookmarks, ...# for hg = 3.0
   from mercurial import node, error, extensions, ...  # for hg = 3.0
   from mercurial import changegroup   # for hg = 3.0
 
   if check_version(3, 0):
   cg = changegroup.getbundle(repo, 'push', ...# for hg = 3.0
   else:
   cg = repo.getbundle('push', heads=list(p_revs)  # for hg  3.0
 
 Eventually the code would become:
 
   from mercurial import hg, ui, bookmarks, ...# for hg = 3.0
   from mercurial import node, error, extensions, ...  # for hg = 3.0
   from mercurial import changegroup   # for hg = 3.0
 
   cg = changegroup.getbundle(repo, 'push', ...# for hg = 3.0

No, the way that it's going to change is that the import statements will
change, not the 'if:else' things. It would work exactly the same with
this, however the things that are used only in a specific version for
this are stated up front. Maybe this visualization helps for what I have
set up::

  from mercurial import ...# for all hg
  
  try:
  from mercurial.changegroup import getbundle  # for hg with
   # changegroup.getbundle,
   # regardless of version

  except ImportError:  # for hg from before the
  def getbundle(__empty__, **kwargs):  # move to changegroup
  return repo.getbundle(**kwargs)
  ...
  cg = getbundle(...)

When we eventually remove support for mercurial that uses
repo.getbundle:

  from mercurial import changegroup, ...   # for all hg
  ...
  cg = changegroup.getbundle(...)
  

That should make sense. You could even just remove the try: and except:
bits and just to 'from mercurial.changegroup import getbundle' and not
mess with the meat of the code at all.

 The fact that at some point 'import changegroup' was not needed is
 irrelevant.
 
 Primarily we want to support the current version of Mercurial.
 Secondarily we want to support older versions. That's why we add the
 repo.getbundle() code (as an addendum to the core part).

So I use arch myself, and I am very used to the 'rolling release' model
that it employs. I do agree that we should concentrate support for the
latest release, but for a project like git the other versions of hg
cannot be ignored, as this project is used on so many different systems.

   We could try that, but that would assume we want to maintain getbundle()
   for the long run, and I personally don't want to do that. I would rather
   contact the Mercurial developers about ways in which the push() method
   can be improved so we don't need to have our own version. Hopefully
   after it's improved we wouldn't have to call getbundle().
  
  Assuming that mercurial 3.0 will not change, then this should never
  need to change.
 
 But it should. Otherwise the code will keep having more and more hacks
 and it will become less and less maintainable.
 
 That's why we don't have code for hg 1.0; because it would require too
 many hacks, and nobody cared anyway.
 
 Just like nobody cares about hg 1.0, eventually nobody will care about
 hg 2.0.

Yes, it can change. Eventually hg 2.0 will be defunct and no one will
use it, but what happens if they go back to the old 2.0 style for
getbundle in hg 4.0? We're already good. What if they switch it back in
4.1? This works for all of those conditions, and doesn't do anything if
we're able to get mercurial.changegroup.getbundle.

  Changes in 'getbundle' upstream would require changes either way.
 
 I doubt we will see any more changes in getbundle, at least not until
 4.0, and hopefully by then we wouldn't be using it anyway. I am willing
  to bet we won't see those changes.
 
   Moreover, eventually there will be a Mercurial 4.0, even 5.0, and at
   some point we would want to remove the hacks for older versions. When we
   do so we would want the import to remain unconditionally, and remove the
   'check_version(3, 0)' which is already helping to explain what the code
   is for without the need of comments.
  
  The same exact thing can be done with this. In fact, it would probably
  allow us to have better future-proofing with regards to new versions of
  mercurial, there would just be different try:except statements at the
  beginning.
 
 No, your code doesn't show that this is for versiosn = 3.0,
 'check_version(3, 0)' does.

That's the whole point. This did 

Re: Error using git-remote-hg

2014-05-13 Thread Felipe Contreras
William Giokas wrote:
 On Tue, May 13, 2014 at 03:24:51PM -0500, Felipe Contreras wrote:
  William Giokas wrote:
   On Tue, May 13, 2014 at 02:09:55PM -0500, Felipe Contreras wrote:
As you say, it's perfectly OK.
   
   But wrong. Yes, it works, but it's not how it should be done when we
   have a code review such as this. It should simply not be done and makes
   no sense to do with an 'if check ver; else' kind of thing later in the
   application.
  
  That's exactly how it should be done. Maybe this visualization helps
  
from mercurial import hg, ui, bookmarks, ...# for hg = 3.0
from mercurial import node, error, extensions, ...  # for hg = 3.0
from mercurial import changegroup   # for hg = 3.0
  
if check_version(3, 0):
cg = changegroup.getbundle(repo, 'push', ...# for hg = 3.0
else:
cg = repo.getbundle('push', heads=list(p_revs)  # for hg  3.0
  
  Eventually the code would become:
  
from mercurial import hg, ui, bookmarks, ...# for hg = 3.0
from mercurial import node, error, extensions, ...  # for hg = 3.0
from mercurial import changegroup   # for hg = 3.0
  
cg = changegroup.getbundle(repo, 'push', ...# for hg = 3.0
 
 No, the way that it's going to change is that the import statements will
 change, not the 'if:else' things. It would work exactly the same with
 this, however the things that are used only in a specific version for
 this are stated up front. Maybe this visualization helps for what I have
 set up::
 
   from mercurial import ...# for all hg
   
   try:
   from mercurial.changegroup import getbundle  # for hg with
# changegroup.getbundle,
# regardless of version

This would make sense if in our eyse all versions of Mercurial were
the same, and we would want the code to work optimally for all of them
forever.

But that's not the case. The current version of Mercurial is more
important, the fact that we have one unnecessary import in older
versions is not of consequence because eventually the won't be
supported.

 When we eventually remove support for mercurial that uses
 repo.getbundle:
 
   from mercurial import changegroup, ...   # for all hg
   ...
   cg = changegroup.getbundle(...)

And the diff from my version to the final version is smaller.

  The fact that at some point 'import changegroup' was not needed is
  irrelevant.
  
  Primarily we want to support the current version of Mercurial.
  Secondarily we want to support older versions. That's why we add the
  repo.getbundle() code (as an addendum to the core part).
 
 So I use arch myself, and I am very used to the 'rolling release' model
 that it employs. I do agree that we should concentrate support for the
 latest release, but for a project like git the other versions of hg
 cannot be ignored, as this project is used on so many different systems.

Older versions are not ignored, they are supported. It's just not worth
tainting the code to avoid an 'import'.

We could try that, but that would assume we want to maintain getbundle()
for the long run, and I personally don't want to do that. I would rather
contact the Mercurial developers about ways in which the push() method
can be improved so we don't need to have our own version. Hopefully
after it's improved we wouldn't have to call getbundle().
   
   Assuming that mercurial 3.0 will not change, then this should never
   need to change.
  
  But it should. Otherwise the code will keep having more and more hacks
  and it will become less and less maintainable.
  
  That's why we don't have code for hg 1.0; because it would require too
  many hacks, and nobody cared anyway.
  
  Just like nobody cares about hg 1.0, eventually nobody will care about
  hg 2.0.
 
 Yes, it can change. Eventually hg 2.0 will be defunct and no one will
 use it, but what happens if they go back to the old 2.0 style for
 getbundle in hg 4.0?

Then you can tell me I was wrong and we go back to your version. But
that's not going to happen.

And even if it does, we still would need to fix the code.

 We're already good. What if they switch it back in
 4.1? This works for all of those conditions, and doesn't do anything if
 we're able to get mercurial.changegroup.getbundle.

Every method can change, we can't have wrappers for all of them.

In reality few of them do. And we deal with them on a case by case
basis.

There's no need to worry about the unlikely, especially since there
isn't much difference between the likely and unlikely; we are still
going to need to fix the code.
 
   Changes in 'getbundle' upstream would require changes either way.
  
  I doubt we will see any more changes in getbundle, at least not until
  4.0, and hopefully by then we wouldn't be using it anyway. I am willing
   to bet we won't see those changes.
  

Re: Error using git-remote-hg

2014-05-12 Thread Torsten Bögershausen
 I'm using git 1.9.3 on Mac OS X 10.9.2, with hg 3.0 installed with brew.
 
 It used to work before, on this same repository, since then git and hg were 
 both upgraded.
In short: The remote helper of Git 1.9.3 is not compatible with hg 3.0
You can eiher downgrade hg, or rebuild Git and cherry-pick this commit:

commit 58aee0864adeeb5363f2a06728596f9c9315811f
Author: Felipe Contreras felipe.contre...@gmail.com
Date:   Sat May 3 21:16:54 2014 -0500

remote-hg: add support for hg v3.0

HTH
/Torsten





--
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: Error using git-remote-hg

2014-05-12 Thread Felipe Contreras
Torsten Bögershausen wrote:
  I'm using git 1.9.3 on Mac OS X 10.9.2, with hg 3.0 installed with brew.
  
  It used to work before, on this same repository, since then git and hg were 
  both upgraded.
 In short: The remote helper of Git 1.9.3 is not compatible with hg 3.0
 You can eiher downgrade hg, or rebuild Git and cherry-pick this commit:

No need to rebuild Git.

You can also use the latest version:
https://github.com/felipec/git-remote-hg

-- 
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