git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] remote-helpers: graduate
@ 2014-04-21 20:37 Felipe Contreras
  2014-04-21 20:37 ` [PATCH v2 1/3] remote-helpers: squelch python import exceptions Felipe Contreras
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Felipe Contreras @ 2014-04-21 20:37 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras

There doesn't seem to be any reason to keep these remote-helpers in the contrib
area.


Felipe Contreras (3):
  remote-helpers: squelch python import exceptions
  remote-helpers: move out of contrib
  remote-helpers: move tests out of contrib

 .gitignore                                                 |  2 ++
 Makefile                                                   |  2 ++
 contrib/remote-helpers/Makefile                            | 14 --------------
 contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py |  0
 contrib/remote-helpers/git-remote-hg => git-remote-hg.py   |  0
 contrib/remote-helpers/test-hg.sh => t/t5810-remote-hg.sh  |  5 ++---
 .../test-hg-bidi.sh => t/t5811-remote-hg-bidi.sh           |  5 ++---
 .../test-hg-hg-git.sh => t/t5812-remote-hg-hg-git.sh       |  7 +++----
 .../remote-helpers/test-bzr.sh => t/t5820-remote-bzr.sh    |  5 ++---
 9 files changed, 13 insertions(+), 27 deletions(-)
 delete mode 100644 contrib/remote-helpers/Makefile
 rename contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py (100%)
 rename contrib/remote-helpers/git-remote-hg => git-remote-hg.py (100%)
 rename contrib/remote-helpers/test-hg.sh => t/t5810-remote-hg.sh (99%)
 rename contrib/remote-helpers/test-hg-bidi.sh => t/t5811-remote-hg-bidi.sh (97%)
 rename contrib/remote-helpers/test-hg-hg-git.sh => t/t5812-remote-hg-hg-git.sh (98%)
 rename contrib/remote-helpers/test-bzr.sh => t/t5820-remote-bzr.sh (98%)

-- 
1.9.2+fc1.1.g5c924db

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 1/3] remote-helpers: squelch python import exceptions
  2014-04-21 20:37 [PATCH v2 0/3] remote-helpers: graduate Felipe Contreras
@ 2014-04-21 20:37 ` Felipe Contreras
  2014-04-21 20:37 ` [PATCH v2 2/3] remote-helpers: move out of contrib Felipe Contreras
  2014-04-21 20:37 ` [PATCH v2 3/3] remote-helpers: move tests " Felipe Contreras
  2 siblings, 0 replies; 17+ messages in thread
From: Felipe Contreras @ 2014-04-21 20:37 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras

When the python modules are not present we get an unwanted message:

    *** prove ***
    test-bzr.sh .. Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/usr/local/buildtools/current/sitecustomize/sitecu...
        return real_import(name, globals, locals, fromlist, level)
    ImportError: No module named bzrlib
    test-bzr.sh .. skipped: skipping remote-bzr tests; bzr not available
    Files=1, Tests=0,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.02 ...

Squelch that output.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-bzr.sh       | 2 +-
 contrib/remote-helpers/test-hg-bidi.sh   | 2 +-
 contrib/remote-helpers/test-hg-hg-git.sh | 4 ++--
 contrib/remote-helpers/test-hg.sh        | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index a4656ce..932fe8a 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -14,7 +14,7 @@ then
 	test_done
 fi
 
-if ! python -c 'import bzrlib'
+if ! python -c 'import bzrlib' > /dev/null 2>&1
 then
 	skip_all='skipping remote-bzr tests; bzr not available'
 	test_done
diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh
index d86e147..2b5aa9d 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -17,7 +17,7 @@ then
 	test_done
 fi
 
-if ! python -c 'import mercurial'
+if ! python -c 'import mercurial' > /dev/null 2>&1
 then
 	skip_all='skipping remote-hg tests; mercurial not available'
 	test_done
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
index b23909a..456a07f 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -17,13 +17,13 @@ then
 	test_done
 fi
 
-if ! python -c 'import mercurial'
+if ! python -c 'import mercurial' > /dev/null 2>&1
 then
 	skip_all='skipping remote-hg tests; mercurial not available'
 	test_done
 fi
 
-if ! python -c 'import hggit'
+if ! python -c 'import hggit' > /dev/null 2>&1
 then
 	skip_all='skipping remote-hg tests; hg-git not available'
 	test_done
diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 7d90056..056940a 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -17,7 +17,7 @@ then
 	test_done
 fi
 
-if ! python -c 'import mercurial'
+if ! python -c 'import mercurial' > /dev/null 2>&1
 then
 	skip_all='skipping remote-hg tests; mercurial not available'
 	test_done
-- 
1.9.2+fc1.1.g5c924db

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-21 20:37 [PATCH v2 0/3] remote-helpers: graduate Felipe Contreras
  2014-04-21 20:37 ` [PATCH v2 1/3] remote-helpers: squelch python import exceptions Felipe Contreras
@ 2014-04-21 20:37 ` Felipe Contreras
  2014-04-21 21:22   ` Junio C Hamano
  2014-04-23 13:10   ` Max Horn
  2014-04-21 20:37 ` [PATCH v2 3/3] remote-helpers: move tests " Felipe Contreras
  2 siblings, 2 replies; 17+ messages in thread
From: Felipe Contreras @ 2014-04-21 20:37 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras

The remote-helpers in contrib/remote-helpers have proved to work, be
reliable, and stable. It's time to move them out of contrib, and be
distributed by default.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 .gitignore                                                 | 2 ++
 Makefile                                                   | 2 ++
 contrib/remote-helpers/test-bzr.sh                         | 2 +-
 contrib/remote-helpers/test-hg-bidi.sh                     | 2 +-
 contrib/remote-helpers/test-hg-hg-git.sh                   | 4 ++--
 contrib/remote-helpers/test-hg.sh                          | 2 +-
 contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py | 0
 contrib/remote-helpers/git-remote-hg => git-remote-hg.py   | 0
 8 files changed, 9 insertions(+), 5 deletions(-)
 rename contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py (100%)
 rename contrib/remote-helpers/git-remote-hg => git-remote-hg.py (100%)

diff --git a/.gitignore b/.gitignore
index dc600f9..db5f15e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -119,11 +119,13 @@
 /git-reflog
 /git-relink
 /git-remote
+/git-remote-bzr
 /git-remote-http
 /git-remote-https
 /git-remote-ftp
 /git-remote-ftps
 /git-remote-fd
+/git-remote-hg
 /git-remote-ext
 /git-remote-testgit
 /git-remote-testpy
diff --git a/Makefile b/Makefile
index 2128ce3..fda4567 100644
--- a/Makefile
+++ b/Makefile
@@ -483,6 +483,8 @@ SCRIPT_PERL += git-send-email.perl
 SCRIPT_PERL += git-svn.perl
 
 SCRIPT_PYTHON += git-p4.py
+SCRIPT_PYTHON += git-remote-hg.py
+SCRIPT_PYTHON += git-remote-bzr.py
 
 NO_INSTALL += git-remote-testgit
 
diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 932fe8a..a656571 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -14,7 +14,7 @@ then
 	test_done
 fi
 
-if ! python -c 'import bzrlib' > /dev/null 2>&1
+if ! "$PYTHON_PATH" -c 'import bzrlib' > /dev/null 2>&1
 then
 	skip_all='skipping remote-bzr tests; bzr not available'
 	test_done
diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh
index 2b5aa9d..d44ec92 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -17,7 +17,7 @@ then
 	test_done
 fi
 
-if ! python -c 'import mercurial' > /dev/null 2>&1
+if ! "$PYTHON_PATH" -c 'import mercurial' > /dev/null 2>&1
 then
 	skip_all='skipping remote-hg tests; mercurial not available'
 	test_done
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
index 456a07f..d0d186c 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -17,13 +17,13 @@ then
 	test_done
 fi
 
-if ! python -c 'import mercurial' > /dev/null 2>&1
+if ! "$PYTHON_PATH" -c 'import mercurial' > /dev/null 2>&1
 then
 	skip_all='skipping remote-hg tests; mercurial not available'
 	test_done
 fi
 
-if ! python -c 'import hggit' > /dev/null 2>&1
+if ! "$PYTHON_PATH" -c 'import hggit' > /dev/null 2>&1
 then
 	skip_all='skipping remote-hg tests; hg-git not available'
 	test_done
diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 056940a..214c548 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -17,7 +17,7 @@ then
 	test_done
 fi
 
-if ! python -c 'import mercurial' > /dev/null 2>&1
+if ! "$PYTHON_PATH" -c 'import mercurial' > /dev/null 2>&1
 then
 	skip_all='skipping remote-hg tests; mercurial not available'
 	test_done
diff --git a/contrib/remote-helpers/git-remote-bzr b/git-remote-bzr.py
similarity index 100%
rename from contrib/remote-helpers/git-remote-bzr
rename to git-remote-bzr.py
diff --git a/contrib/remote-helpers/git-remote-hg b/git-remote-hg.py
similarity index 100%
rename from contrib/remote-helpers/git-remote-hg
rename to git-remote-hg.py
-- 
1.9.2+fc1.1.g5c924db

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 3/3] remote-helpers: move tests out of contrib
  2014-04-21 20:37 [PATCH v2 0/3] remote-helpers: graduate Felipe Contreras
  2014-04-21 20:37 ` [PATCH v2 1/3] remote-helpers: squelch python import exceptions Felipe Contreras
  2014-04-21 20:37 ` [PATCH v2 2/3] remote-helpers: move out of contrib Felipe Contreras
@ 2014-04-21 20:37 ` Felipe Contreras
  2 siblings, 0 replies; 17+ messages in thread
From: Felipe Contreras @ 2014-04-21 20:37 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras

They should be tested by default.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/Makefile                            | 14 --------------
 contrib/remote-helpers/test-hg.sh => t/t5810-remote-hg.sh  |  3 +--
 .../test-hg-bidi.sh => t/t5811-remote-hg-bidi.sh           |  3 +--
 .../test-hg-hg-git.sh => t/t5812-remote-hg-hg-git.sh       |  3 +--
 .../remote-helpers/test-bzr.sh => t/t5820-remote-bzr.sh    |  3 +--
 5 files changed, 4 insertions(+), 22 deletions(-)
 delete mode 100644 contrib/remote-helpers/Makefile
 rename contrib/remote-helpers/test-hg.sh => t/t5810-remote-hg.sh (99%)
 rename contrib/remote-helpers/test-hg-bidi.sh => t/t5811-remote-hg-bidi.sh (98%)
 rename contrib/remote-helpers/test-hg-hg-git.sh => t/t5812-remote-hg-hg-git.sh (99%)
 rename contrib/remote-helpers/test-bzr.sh => t/t5820-remote-bzr.sh (98%)

diff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile
deleted file mode 100644
index 239161d..0000000
--- a/contrib/remote-helpers/Makefile
+++ /dev/null
@@ -1,14 +0,0 @@
-TESTS := $(wildcard test*.sh)
-
-export T := $(addprefix $(CURDIR)/,$(TESTS))
-export MAKE := $(MAKE) -e
-export PATH := $(CURDIR):$(PATH)
-export TEST_LINT := test-lint-executable test-lint-shell-syntax
-
-test:
-	$(MAKE) -C ../../t $@
-
-$(TESTS):
-	$(MAKE) -C ../../t $(CURDIR)/$@
-
-.PHONY: $(TESTS)
diff --git a/contrib/remote-helpers/test-hg.sh b/t/t5810-remote-hg.sh
similarity index 99%
rename from contrib/remote-helpers/test-hg.sh
rename to t/t5810-remote-hg.sh
index 214c548..594a0a1 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/t/t5810-remote-hg.sh
@@ -8,8 +8,7 @@
 
 test_description='Test remote-hg'
 
-test -n "$TEST_DIRECTORY" || TEST_DIRECTORY=${0%/*}/../../t
-. "$TEST_DIRECTORY"/test-lib.sh
+. ./test-lib.sh
 
 if ! test_have_prereq PYTHON
 then
diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/t/t5811-remote-hg-bidi.sh
similarity index 98%
rename from contrib/remote-helpers/test-hg-bidi.sh
rename to t/t5811-remote-hg-bidi.sh
index d44ec92..d738ed4 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/t/t5811-remote-hg-bidi.sh
@@ -8,8 +8,7 @@
 
 test_description='Test bidirectionality of remote-hg'
 
-test -n "$TEST_DIRECTORY" || TEST_DIRECTORY=${0%/*}/../../t
-. "$TEST_DIRECTORY"/test-lib.sh
+. ./test-lib.sh
 
 if ! test_have_prereq PYTHON
 then
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/t/t5812-remote-hg-hg-git.sh
similarity index 99%
rename from contrib/remote-helpers/test-hg-hg-git.sh
rename to t/t5812-remote-hg-hg-git.sh
index d0d186c..50c216f 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/t/t5812-remote-hg-hg-git.sh
@@ -8,8 +8,7 @@
 
 test_description='Test remote-hg output compared to hg-git'
 
-test -n "$TEST_DIRECTORY" || TEST_DIRECTORY=${0%/*}/../../t
-. "$TEST_DIRECTORY"/test-lib.sh
+. ./test-lib.sh
 
 if ! test_have_prereq PYTHON
 then
diff --git a/contrib/remote-helpers/test-bzr.sh b/t/t5820-remote-bzr.sh
similarity index 98%
rename from contrib/remote-helpers/test-bzr.sh
rename to t/t5820-remote-bzr.sh
index a656571..8f0719a 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/t/t5820-remote-bzr.sh
@@ -5,8 +5,7 @@
 
 test_description='Test remote-bzr'
 
-test -n "$TEST_DIRECTORY" || TEST_DIRECTORY=${0%/*}/../../t
-. "$TEST_DIRECTORY"/test-lib.sh
+. ./test-lib.sh
 
 if ! test_have_prereq PYTHON
 then
-- 
1.9.2+fc1.1.g5c924db

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-21 20:37 ` [PATCH v2 2/3] remote-helpers: move out of contrib Felipe Contreras
@ 2014-04-21 21:22   ` Junio C Hamano
  2014-04-21 21:24     ` Felipe Contreras
  2014-04-23 13:10   ` Max Horn
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2014-04-21 21:22 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git

Felipe Contreras <felipe.contreras@gmail.com> writes:

>  contrib/remote-helpers/test-bzr.sh                         | 2 +-
>  contrib/remote-helpers/test-hg-bidi.sh                     | 2 +-
>  contrib/remote-helpers/test-hg-hg-git.sh                   | 4 ++--
>  contrib/remote-helpers/test-hg.sh                          | 2 +-
>  contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py | 0
>  contrib/remote-helpers/git-remote-hg => git-remote-hg.py   | 0
>  8 files changed, 9 insertions(+), 5 deletions(-)
>  rename contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py (100%)
>  rename contrib/remote-helpers/git-remote-hg => git-remote-hg.py (100%)
> ...
> diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh
> index 2b5aa9d..d44ec92 100755
> --- a/contrib/remote-helpers/test-hg-bidi.sh
> +++ b/contrib/remote-helpers/test-hg-bidi.sh
> @@ -17,7 +17,7 @@ then
>  	test_done
>  fi
>  
> -if ! python -c 'import mercurial' > /dev/null 2>&1
> +if ! "$PYTHON_PATH" -c 'import mercurial' > /dev/null 2>&1

Does this change relate to the moving of main scripts, and if so
how?

Ditto for other test-*.sh scripts in the same directory.

>  then
>  	skip_all='skipping remote-hg tests; mercurial not available'
>  	test_done
> diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
> index 456a07f..d0d186c 100755
> --- a/contrib/remote-helpers/test-hg-hg-git.sh
> +++ b/contrib/remote-helpers/test-hg-hg-git.sh
> @@ -17,13 +17,13 @@ then
>  	test_done
>  fi
>  
> -if ! python -c 'import mercurial' > /dev/null 2>&1
> +if ! "$PYTHON_PATH" -c 'import mercurial' > /dev/null 2>&1
>  then
>  	skip_all='skipping remote-hg tests; mercurial not available'
>  	test_done
>  fi
>  
> -if ! python -c 'import hggit' > /dev/null 2>&1
> +if ! "$PYTHON_PATH" -c 'import hggit' > /dev/null 2>&1
>  then
>  	skip_all='skipping remote-hg tests; hg-git not available'
>  	test_done
> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
> index 056940a..214c548 100755
> --- a/contrib/remote-helpers/test-hg.sh
> +++ b/contrib/remote-helpers/test-hg.sh
> @@ -17,7 +17,7 @@ then
>  	test_done
>  fi
>  
> -if ! python -c 'import mercurial' > /dev/null 2>&1
> +if ! "$PYTHON_PATH" -c 'import mercurial' > /dev/null 2>&1
>  then
>  	skip_all='skipping remote-hg tests; mercurial not available'
>  	test_done
> diff --git a/contrib/remote-helpers/git-remote-bzr b/git-remote-bzr.py
> similarity index 100%
> rename from contrib/remote-helpers/git-remote-bzr
> rename to git-remote-bzr.py
> diff --git a/contrib/remote-helpers/git-remote-hg b/git-remote-hg.py
> similarity index 100%
> rename from contrib/remote-helpers/git-remote-hg
> rename to git-remote-hg.py

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-21 21:22   ` Junio C Hamano
@ 2014-04-21 21:24     ` Felipe Contreras
  2014-04-21 21:42       ` Junio C Hamano
  0 siblings, 1 reply; 17+ messages in thread
From: Felipe Contreras @ 2014-04-21 21:24 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras; +Cc: git

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> >  contrib/remote-helpers/test-bzr.sh                         | 2 +-
> >  contrib/remote-helpers/test-hg-bidi.sh                     | 2 +-
> >  contrib/remote-helpers/test-hg-hg-git.sh                   | 4 ++--
> >  contrib/remote-helpers/test-hg.sh                          | 2 +-
> >  contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py | 0
> >  contrib/remote-helpers/git-remote-hg => git-remote-hg.py   | 0
> >  8 files changed, 9 insertions(+), 5 deletions(-)
> >  rename contrib/remote-helpers/git-remote-bzr => git-remote-bzr.py (100%)
> >  rename contrib/remote-helpers/git-remote-hg => git-remote-hg.py (100%)
> > ...
> > diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh
> > index 2b5aa9d..d44ec92 100755
> > --- a/contrib/remote-helpers/test-hg-bidi.sh
> > +++ b/contrib/remote-helpers/test-hg-bidi.sh
> > @@ -17,7 +17,7 @@ then
> >  	test_done
> >  fi
> >  
> > -if ! python -c 'import mercurial' > /dev/null 2>&1
> > +if ! "$PYTHON_PATH" -c 'import mercurial' > /dev/null 2>&1
> 
> Does this change relate to the moving of main scripts, and if so
> how?

Yes.

Before the scripts were not generated, the shebang was '/usr/bin/env python',
that means if the user doesn't have 'python' but 'python2' git-remote-hg would
fail, even if the user did PYTHON_PATH=python2, therefore the test scripts
should fail too, and that's the reason 'python' is used in the test scripts.

Now that the scripts are generated the build system would replace the shebang,
and PYTHON_PATH will be used correctly, and we should use that in the tests.

I actually implemented this script generation inside contrib/remote-helpers,
indepedently of the move, but you didn't apply those patches.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-21 21:24     ` Felipe Contreras
@ 2014-04-21 21:42       ` Junio C Hamano
  0 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2014-04-21 21:42 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> Does this change relate to the moving of main scripts, and if so
>> how?
>
> Yes.
>
> Before the scripts were not generated, the shebang was '/usr/bin/env python',
> that means if the user doesn't have 'python' but 'python2' git-remote-hg would
> fail, even if the user did PYTHON_PATH=python2, therefore the test scripts
> should fail too, and that's the reason 'python' is used in the test scripts.

Ahh, OK, I agree that is a sensible change, then.  I would think
that deserves to be explained in the proposed log message, though.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-21 20:37 ` [PATCH v2 2/3] remote-helpers: move out of contrib Felipe Contreras
  2014-04-21 21:22   ` Junio C Hamano
@ 2014-04-23 13:10   ` Max Horn
  2014-04-23 19:20     ` Junio C Hamano
                       ` (2 more replies)
  1 sibling, 3 replies; 17+ messages in thread
From: Max Horn @ 2014-04-23 13:10 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 1989 bytes --]


On 21.04.2014, at 22:37, Felipe Contreras <felipe.contreras@gmail.com> wrote:

> The remote-helpers in contrib/remote-helpers have proved to work, be
> reliable, and stable. It's time to move them out of contrib, and be
> distributed by default.

Really? While I agree that git-remote-hg by now works quite well for basic usage in simple situation, there are still unresolved bugs and fundamental issues with it.

E.g. I recently showed you a reproducible use case involving git-remote-hg that puts the helper into a broken state from which it is difficult for a normal user to recover. Namely when a hg branch has multiple heads, then git-remote-hg exports all of those to git, but only adds a git ref for one of them; after pruning unreferenced commits, the fast-import marks file references git commits that now are missing, prompting git fast-import to crash and trash the marks file. Afterwards, attempts to push or pull from the remote hg repository are answered with an error.

There are other ways to trigger variants of this, and these are not artificial, they occur in real life (this is how I run into the issue). There are more issues related to unresolved clashes between the git and hg ways of naming things. E.g. I am collaborating on a hg repository that has branches "foo" and "foo/bar" which git-remote-hg cannot handle because it translates them to git branch names, and, well, git cannot handle that.

It may be hard to deal with some of them, and admittedly I wouldn't necessarily expect that all of these are handled from the outset, i.e. "in version 1.0". But I think at the very least, users should be warned about these things.

More broadly speaking, there is currently no documentation at all in git.git for those remote helpers, which I find worrisome.

That said, I don't know what the criteria are for moving something out of contrib. Perhaps it is OK to move an undocumented remote-helper with known bugs out of contrib.


Cheers,
Max

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-23 13:10   ` Max Horn
@ 2014-04-23 19:20     ` Junio C Hamano
  2014-04-23 21:00       ` Felipe Contreras
  2014-04-23 20:12     ` Junio C Hamano
  2014-04-23 20:54     ` Felipe Contreras
  2 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2014-04-23 19:20 UTC (permalink / raw)
  To: Max Horn; +Cc: Felipe Contreras, git

Max Horn <max@quendi.de> writes:

> That said, I don't know what the criteria are for moving something out
> of contrib.

Because we accept stuff to contrib/, with an assumption that it is
to stay there without contaminating the main part of the system, the
quality of stuff in contrib/ can be sub-par and it is very possible
that a lot of clean-ups and fixes are necessary before moving them
out.

> Perhaps it is OK to move an undocumented remote-helper
> with known bugs out of contrib.

We should strive to apply the same criteria as new submission to the
main part of the system.  And inputs from people like you who have
more experiences than others on the list in dealing with hg-to-git
bridging are very much welcomed to identify and document what kind
of breakages there are, and to come up with the solution to them.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-23 13:10   ` Max Horn
  2014-04-23 19:20     ` Junio C Hamano
@ 2014-04-23 20:12     ` Junio C Hamano
  2014-04-23 20:54     ` Felipe Contreras
  2 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2014-04-23 20:12 UTC (permalink / raw)
  To: Max Horn; +Cc: Felipe Contreras, git

Max Horn <max@quendi.de> writes:

[Administrivia: please wrap your lines to reasonable length like 70-75].

> On 21.04.2014, at 22:37, Felipe Contreras <felipe.contreras@gmail.com>
> wrote:
>
>> The remote-helpers in contrib/remote-helpers have proved to work, be
>> reliable, and stable. It's time to move them out of contrib, and be
>> distributed by default.
>
> Really? While I agree that git-remote-hg by now works quite well for
> basic usage in simple situation, there are still unresolved bugs and
> fundamental issues with it.

As you mentioned later in your message, I agree that remaining bugs
are expected in an initial release.  I found that the above phrase
is exaggerating, but I think you are over-reacting [*1*].

> E.g. I recently showed you a reproducible use case involving
> git-remote-hg that puts the helper into a broken state from which it
> is difficult for a normal user to recover. Namely when ...
> ... prompting git fast-import to crash and trash the marks
> file. Afterwards, attempts to push or pull from the remote hg
> repository are answered with an error.
> There are other ways to trigger variants of this,...

Isn't the recent fc/transport-helper-sync-error-fix a reasonable
workaround for this issue?  The split-head in Hg fundamentally
cannot be expressed as a single ref on the Git side, and the series
attempts not to trash the marks file when the importer quits
abnormally for whatever reason to avoid rendering the resulting
repository unusable for future operations, which I thought was the
best you could do.

> It may be hard to deal with some of them, and admittedly I wouldn't
> necessarily expect that all of these are handled from the outset,
> i.e. "in version 1.0". But I think at the very least, users should be
> warned about these things.
>
> More broadly speaking, there is currently no documentation at all in
> git.git for those remote helpers, which I find worrisome.

I think your points regarding certain Hg features that do not map
well to Git data model are good ones; it would be good to have them
at least documented.


[Footnote]

*1* Personally I think it would have been better if it stopped at
somewhere around "some people in the field are using and reporting
success, let's give it wider exposure", without using words like
"proven", "reliable", or "stable" to make it sound like some
objective goodness has already been achieved.  People will call the
result of the project's work as "proven to be reliable and stable"
if it is so; the project does not have to claim and advertise its
ware by using such words---the code will prove itself given time,
and it is better to let others use these words, not ourselves.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-23 13:10   ` Max Horn
  2014-04-23 19:20     ` Junio C Hamano
  2014-04-23 20:12     ` Junio C Hamano
@ 2014-04-23 20:54     ` Felipe Contreras
  2014-04-23 22:41       ` Max Horn
  2 siblings, 1 reply; 17+ messages in thread
From: Felipe Contreras @ 2014-04-23 20:54 UTC (permalink / raw)
  To: Max Horn, Felipe Contreras; +Cc: git

Max Horn wrote:
> On 21.04.2014, at 22:37, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> 
> > The remote-helpers in contrib/remote-helpers have proved to work, be
> > reliable, and stable. It's time to move them out of contrib, and be
> > distributed by default.
> 
> Really? While I agree that git-remote-hg by now works quite well for basic
> usage in simple situation, there are still unresolved bugs and fundamental
> issues with it.

s/basic usage in simple situation/complex usage in the vast majority of situations/

> E.g. I recently showed you a reproducible use case involving git-remote-hg
> that puts the helper into a broken state from which it is difficult for a
> normal user to recover. Namely when a hg branch has multiple heads, then
> git-remote-hg exports all of those to git, but only adds a git ref for one of
> them; after pruning unreferenced commits, the fast-import marks file
> references git commits that now are missing, prompting git fast-import to
> crash and trash the marks file. Afterwards, attempts to push or pull from the
> remote hg repository are answered with an error.

Yes, and how often does that happen? A normal user would only see this if a
branch remains with multiple heads in Mercurial for more than one month or so.

In practice that's very unlikely, and proof of that is that nobody has reported
such issues.

Either way, I just fixed it [1].

> There are more issues related to unresolved clashes between the git and hg
> ways of naming things. E.g. I am collaborating on a hg repository that has
> branches "foo" and "foo/bar" which git-remote-hg cannot handle because it
> translates them to git branch names, and, well, git cannot handle that.

I don't see this as a limitation of git-remote-hg, ideally Git remote-helpers
should have a standardized way to let users map external branch names.

> It may be hard to deal with some of them, and admittedly I wouldn't
> necessarily expect that all of these are handled from the outset, i.e. "in
> version 1.0". But I think at the very least, users should be warned about
> these things.
> 
> More broadly speaking, there is currently no documentation at all in git.git
> for those remote helpers, which I find worrisome.

Here is the documentation:
https://github.com/felipec/git/wiki/git-remote-hg
https://github.com/felipec/git/wiki/git-remote-hg

> That said, I don't know what the criteria are for moving something out of
> contrib. Perhaps it is OK to move an undocumented remote-helper with known
> bugs out of contrib.

There are no known bugs. This is the list of open bugs:

https://github.com/felipec/git/issues

Now if you want to label the limitation of Git that you can't have both 'foo'
and 'foo/bar' as a bug of git-remote-hg, that's up to you, but it's something
nobody had reported before, so it definitely can't be labeled as a "known bug".

[1] https://github.com/felipec/git/commit/fbaae8caa51804a655fd6bc5727763b64e3c2e9f

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-23 19:20     ` Junio C Hamano
@ 2014-04-23 21:00       ` Felipe Contreras
  2014-04-23 21:30         ` Junio C Hamano
  0 siblings, 1 reply; 17+ messages in thread
From: Felipe Contreras @ 2014-04-23 21:00 UTC (permalink / raw)
  To: Junio C Hamano, Max Horn; +Cc: Felipe Contreras, git

Junio C Hamano wrote:
> Max Horn <max@quendi.de> writes:
> 
> > Perhaps it is OK to move an undocumented remote-helper
> > with known bugs out of contrib.
> 
> We should strive to apply the same criteria as new submission to the
> main part of the system.  And inputs from people like you who have
> more experiences than others on the list in dealing with hg-to-git
> bridging are very much welcomed to identify and document what kind
> of breakages there are, and to come up with the solution to them.

FTR. There are no "known brakages", at least to me. Max Horn tends to hoard
these issues and never report them (unless an opportunity to criticize
git-remote-hg comes, apparently).

The very unlikely issue that nobody has reported about hg multiple heads and gc
I just fixed, and the issue he just reported about 'foo' and 'foo/bar' is newly
reported, and there's no easy way to fix this. My proposal would be to have
some sort of branch mapping mechanism in fast-import, but hardly something that
should prevent the move out of contrib.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-23 21:00       ` Felipe Contreras
@ 2014-04-23 21:30         ` Junio C Hamano
  0 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2014-04-23 21:30 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Max Horn, git

Felipe Contreras <felipe.contreras@gmail.com> writes:

> The very unlikely issue that nobody has reported about hg multiple heads and gc
> I just fixed, and the issue he just reported about 'foo' and 'foo/bar' is newly
> reported, and there's no easy way to fix this.

I would not judge on likelyhood, but I would say that 'foo' vs
'foo/bar' is something that falls into the same category as "if you
want your project to be cross-platform, don't have paths Foo and foo
both tracked, as some people have to work on case insensitive
systems".

In other words, I think it is perfectly fine as long as the users
are made aware of these issues.  One way to deal with such a project
may be to allow users to map 'foo' to 'foo_' to make room for
'foo/bar', but lack of such a feature is not grave enough to block
it from being used---it would be unreasonable to demand the ref
mapping to be implemented before the command is given to the end
users.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-23 20:54     ` Felipe Contreras
@ 2014-04-23 22:41       ` Max Horn
  2014-04-24  0:23         ` Felipe Contreras
  0 siblings, 1 reply; 17+ messages in thread
From: Max Horn @ 2014-04-23 22:41 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git


On 23.04.2014, at 22:54, Felipe Contreras <felipe.contreras@gmail.com> wrote:

> Max Horn wrote:
>> On 21.04.2014, at 22:37, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> 
>>> The remote-helpers in contrib/remote-helpers have proved to work, be
>>> reliable, and stable. It's time to move them out of contrib, and be
>>> distributed by default.
>> 
>> Really? While I agree that git-remote-hg by now works quite well for basic
>> usage in simple situation, there are still unresolved bugs and fundamental
>> issues with it.
> 
> s/basic usage in simple situation/complex usage in the vast majority of situations/

Yeah, hm, no. We can agree to disagree, I guess. It might also depend on
what you call "basic" or "complex" usage...

For example, whenever I need to
- close a branch
- fix a branch with multiple heads
- deal with phases
I need to switch to hg. I am pretty sure there are more things that make
that necessary, but luckily they don't happen to me.

What does work, though (and what I count as basic usage, although I'd say it
probably is enough for 95% of people out there) is making a clone of "safe"
repository (i.e. no "bad" branch names like 'foo' together with 'foo/bar'),
push and pull with it; and if you are careful, you can even get branch
attribution right. This is great, but in day-to-day usage, I still regularly
need to work with a hg clone for some tasks. But I am not really complaining
about that: For most of my regular "developer" work, I can stay in git, and
that makes me happy.

Out of curiosity: How do you yourself use git-remote-hg in your daily work?
Many people I encountered are happy enough with the ability to quickly
clone a hg repository, prepare a fix / feature branch and then submit
it back to upstream. For this, git-remote-hg *usually* is good enough.
But I am worried about people hitting the edge cases where it does not
quite work -- and then people are lost. This is what concerns me -- and
this concern would be alleviate if there was a list of known things that
do not work (and perhaps cannot work, at least for now, due to fundamental
differences between hg and git which need major work to bridge over).

Anyway, despite my criticism, I'd like to emphasis that I am actually quite
happy and grateful that your git-remote-hg exists and that you continue
improving it and the surrounding infrastructure. I just wish you could do it
while not acting like an asshole most of the time, but I'll survive that,
too *shrug*.



> 
>> E.g. I recently showed you a reproducible use case involving git-remote-hg
>> that puts the helper into a broken state from which it is difficult for a
>> normal user to recover. Namely when a hg branch has multiple heads, then
>> git-remote-hg exports all of those to git, but only adds a git ref for one of
>> them; after pruning unreferenced commits, the fast-import marks file
>> references git commits that now are missing, prompting git fast-import to
>> crash and trash the marks file. Afterwards, attempts to push or pull from the
>> remote hg repository are answered with an error.
> 
> Yes, and how often does that happen? A normal user would only see this if a
> branch remains with multiple heads in Mercurial for more than one month or so.

There are projects who do exactly that, although I believe most of them use
bookmarks, so the issue should indeed not affect those. Anyway, they do the
wrong thing ;-). Still, if you are forced with such a repository, it's not
very helpful to be told that this is your own fault...

But this kind of issue also happens in any other scenario were heads are not
mapped to a git reference. At the very least, it also happens for closed hg
branches. These are quite common, and I also run into that in real life.

[And to reply to a claim you made in another mail: No, I am not deliberately
"hoarding" issues to make you look bad. But analyzing a breakage you run into
and then properly writing it up takes time; and when you know you will likely
be insulted when reporting it doesn't really help to motivate me to sit
down and do that...]


> 
> In practice that's very unlikely, and proof of that is that nobody has reported
> such issues.

No, that logic is flawed. For example, It could also mean that not many
people are using the tool, and of those not many bother to report issues via
your github bug tracker.

> 
> Either way, I just fixed it [1].

That's great to hear, thanks!

> 
>> There are more issues related to unresolved clashes between the git and hg
>> ways of naming things. E.g. I am collaborating on a hg repository that has
>> branches "foo" and "foo/bar" which git-remote-hg cannot handle because it
>> translates them to git branch names, and, well, git cannot handle that.
> 
> I don't see this as a limitation of git-remote-hg, ideally Git remote-helpers
> should have a standardized way to let users map external branch names.

Agreed. But in the meantime, I think users should still be warned about it.
Or perhaps git-remote-hg could detect this and print a more helpful message
that tells the user what is wrong...?

I did not actively know about this limitation of git and learned about it
when a hg repository I cloned caused git-remote-hg to print strange messages
I did not understand at first... ;-). For me that's OK as I am used to
debugging remote-helper issue by now, but "regular" users might not
fare quite so well...

> 
>> It may be hard to deal with some of them, and admittedly I wouldn't
>> necessarily expect that all of these are handled from the outset, i.e. "in
>> version 1.0". But I think at the very least, users should be warned about
>> these things.
>> 
>> More broadly speaking, there is currently no documentation at all in git.git
>> for those remote helpers, which I find worrisome.
> 
> Here is the documentation:
> https://github.com/felipec/git/wiki/git-remote-hg
> https://github.com/felipec/git/wiki/git-remote-hg

[I assume you meant one of the links to be for -bzr; but your point is clear in any case].

Great! But my point was not that there is no documentation, rather that
there is none in the git repository. Since you already have some
documentation, this should be easy enough to resolve; e.g. for starters,
those links could be included in the repository. Though I think that if the
helpers are moved out of contrib, they'd deserve a proper help page.

Reading through the git-remote-hg page, here are some suggestions:
- you already warn about octopus merges -- perhaps this would be a good
  place to also warn about e.g. "foo" vs "foo/bar", branches with multiple heads,
  closed branches, etc...
- you talk about "branch/next", shouldn't that be "branches/next"?
- indeed, branches deserve some extra attention. At the beginning of my usage
  of your git-remote-hg and of the gitifyhg fork, I run into the problem
  that commits I pushed to hg ended up on the wrong branch. This was because
  I added commits to one branch, say "branches/foo", then merged those
  into "branches/bar", then did a push -- and all commits I made ended
  up on the hg branch "bar". Now I know that I need to first push "foo",
  then afterwards push "foo".
  I am not asking for this to be "fixed" because it seems hard to "fix
  in the general case; just that users are warned about such pitfalls
  that do occur in basic usage in real life (and I know of other users
  who run into it, simply by the fact that I helped multiple people to
  get git-remote-hg or gitifyhg working and they come to me with their
  questions).



> 
>> That said, I don't know what the criteria are for moving something out of
>> contrib. Perhaps it is OK to move an undocumented remote-helper with known
>> bugs out of contrib.
> 
> There are no known bugs. This is the list of open bugs:
> 
> https://github.com/felipec/git/issues

At the time of writing, I was not aware that you had fixed the bug with the
marks file getting trashed. I did not yet have a chance to test your fix,
but I trust it works, so yeah, that resolve that. Excellent, thanks.

So, for now, there are just some "known limitations", but that's fine, I
guess.

Note that there are more limitations than those I listed above; not because
I want to "hoard" them, but because it seems rather trivial to come up with
those if one thinks about it for 5 minutes, so I simply assume(d?) you are
aware of most of them. Here's what I can think of right now; this still may
not be complete; nor do I mean to say those should hold up anything; I merely
want to get them out to prevent any future accusations about "hoarding"
stuff)

- close hg branches cannot be referenced from git, and thus not easily
  reopened (at least not from their old tip)
- multiple tips/heads of a hg branch cannot be referenced; only a single
  "random" head (in the sense that it is not documented how it is chosen)
  is visible
- more name clashes, e.g. the somewhat hypothetical case of having a hg
  branch "foo" and a hg bookmark "branches/foo"
- renaming remotes is a bit problematic (this is IMHO a shortcoming of the
  remote helpers interface, not of git-remote-{hg,bzr}). I.e. it
  can result in commits being re-exported, and hence in diverging history.
- Not sure if this is still a problem, but I was seeing some issues when
  a hg remote pruned some commits. Of course this is a bad thing to do in
  the first place, but sadly it sometimes happens. IIRC this resulted in a
  truncated marks file (which, as I understand, should be fixed already),
  but I am a bit fuzzy on the details (it was quite some time ago, and since
  something bad and exceptional was done on the hg side, I didn't bother to
  look closer at it.


Max

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-23 22:41       ` Max Horn
@ 2014-04-24  0:23         ` Felipe Contreras
  2014-04-25 22:26           ` Max Horn
  0 siblings, 1 reply; 17+ messages in thread
From: Felipe Contreras @ 2014-04-24  0:23 UTC (permalink / raw)
  To: Max Horn, Felipe Contreras; +Cc: git

Max Horn wrote:
> On 23.04.2014, at 22:54, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> > Max Horn wrote:
> >> On 21.04.2014, at 22:37, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> >> 
> >>> The remote-helpers in contrib/remote-helpers have proved to work, be
> >>> reliable, and stable. It's time to move them out of contrib, and be
> >>> distributed by default.
> >> 
> >> Really? While I agree that git-remote-hg by now works quite well for basic
> >> usage in simple situation, there are still unresolved bugs and fundamental
> >> issues with it.
> > 
> > s/basic usage in simple situation/complex usage in the vast majority of situations/
> 
> Yeah, hm, no. We can agree to disagree, I guess.

Right, except one of us has evidence to back this up:

* There hasn't been a single person coming to the Git mailing (or git-fc's
  bugtracker) reporting that they haven't been able to do something. This means
  if there's any situation where git-remote-hg doesn't work well, it hasn't
  been reported, and therefore the reasonable thing to assume is that if there
  are any, they are marginal at best.

> It might also depend on what you call "basic" or "complex" usage...
> 
> For example, whenever I need to
> - close a branch
> - fix a branch with multiple heads
> - deal with phases
> I need to switch to hg. I am pretty sure there are more things that make
> that necessary, but luckily they don't happen to me.

The fact that there are complex things you can't do, doesn't mean there isn't
any complex usage supported at all.

> Out of curiosity: How do you yourself use git-remote-hg in your daily work?

I don't.

> But I am worried about people hitting the edge cases where it does not quite
> work -- and then people are lost.

And which people are those? Are they imaginary by any chance?

> This is what concerns me -- and this concern would be alleviate if there was
> a list of known things that do not work (and perhaps cannot work, at least
> for now, due to fundamental differences between hg and git which need major
> work to bridge over).

That list is empty at the moment, because again, nobody has come reporting
anything they cannot do.

The things that have been reported, I've fixed.

> Anyway, despite my criticism, I'd like to emphasis that I am actually quite
> happy and grateful that your git-remote-hg exists and that you continue
> improving it and the surrounding infrastructure. I just wish you could do it
> while not acting like an asshole most of the time, but I'll survive that,
> too *shrug*.

I am being straight-forward, not an asshole.

If *you* have issues, why don't *you* list them? The people that have reported
anything back about git-remote-hg/bzr, have reported success, or their issue
was resolved. Even better would be if *you* fixed the issues you have.

> >> E.g. I recently showed you a reproducible use case involving git-remote-hg
> >> that puts the helper into a broken state from which it is difficult for a
> >> normal user to recover. Namely when a hg branch has multiple heads, then
> >> git-remote-hg exports all of those to git, but only adds a git ref for one of
> >> them; after pruning unreferenced commits, the fast-import marks file
> >> references git commits that now are missing, prompting git fast-import to
> >> crash and trash the marks file. Afterwards, attempts to push or pull from the
> >> remote hg repository are answered with an error.
> > 
> > Yes, and how often does that happen? A normal user would only see this if a
> > branch remains with multiple heads in Mercurial for more than one month or so.
> 
> There are projects who do exactly that, although I believe most of them use
> bookmarks, so the issue should indeed not affect those. Anyway, they do the
> wrong thing ;-). Still, if you are forced with such a repository, it's not
> very helpful to be told that this is your own fault...

Nobody said it's their fault, what I'm saying is that those people don't exist,
or haven't bothered to report such issues.
 
> But this kind of issue also happens in any other scenario were heads are not
> mapped to a git reference.

And where is that?

> At the very least, it also happens for closed hg branches. These are quite
> common, and I also run into that in real life.

If a branch is closed, it's ignored by git-remote-hg, so that doesn't cause a
problem. If there's any scenario where it would be a problem, it's certain more
complex than having a closed branch.

> [And to reply to a claim you made in another mail: No, I am not deliberately
> "hoarding" issues to make you look bad. But analyzing a breakage you run into
> and then properly writing it up takes time; and when you know you will likely
> be insulted when reporting it doesn't really help to motivate me to sit
> down and do that...]

Show me exactly where anybody did insult you for reporting an issue.

> > In practice that's very unlikely, and proof of that is that nobody has reported
> > such issues.
> 
> No, that logic is flawed. For example, It could also mean that not many
> people are using the tool, and of those not many bother to report issues via
> your github bug tracker.

Yes, it could mean that as well, if that's your contention then it doesn't
matter either; most people are not being affected.

> >> There are more issues related to unresolved clashes between the git and hg
> >> ways of naming things. E.g. I am collaborating on a hg repository that has
> >> branches "foo" and "foo/bar" which git-remote-hg cannot handle because it
> >> translates them to git branch names, and, well, git cannot handle that.
> > 
> > I don't see this as a limitation of git-remote-hg, ideally Git remote-helpers
> > should have a standardized way to let users map external branch names.
> 
> Agreed. But in the meantime, I think users should still be warned about it.
> Or perhaps git-remote-hg could detect this and print a more helpful message
> that tells the user what is wrong...?

Yes, the tool could do that, but that is not something that should prevent the
move out of contrib, so it's not relevant for $subject.

> >> It may be hard to deal with some of them, and admittedly I wouldn't
> >> necessarily expect that all of these are handled from the outset, i.e. "in
> >> version 1.0". But I think at the very least, users should be warned about
> >> these things.
> >> 
> >> More broadly speaking, there is currently no documentation at all in git.git
> >> for those remote helpers, which I find worrisome.
> > 
> > Here is the documentation:
> > https://github.com/felipec/git/wiki/git-remote-hg
> > https://github.com/felipec/git/wiki/git-remote-hg
> 
> [I assume you meant one of the links to be for -bzr; but your point is clear in any case].
> 
> Great! But my point was not that there is no documentation, rather that
> there is none in the git repository. Since you already have some
> documentation, this should be easy enough to resolve; e.g. for starters,
> those links could be included in the repository. Though I think that if the
> helpers are moved out of contrib, they'd deserve a proper help page.

Yes, it should be trivial to copy that documentation. This is not a
make-or-break point against the move.

> >> That said, I don't know what the criteria are for moving something out of
> >> contrib. Perhaps it is OK to move an undocumented remote-helper with known
> >> bugs out of contrib.
> > 
> > There are no known bugs. This is the list of open bugs:
> > 
> > https://github.com/felipec/git/issues
> 
> At the time of writing, I was not aware that you had fixed the bug with the
> marks file getting trashed. I did not yet have a chance to test your fix,
> but I trust it works, so yeah, that resolve that. Excellent, thanks.
> 
> So, for now, there are just some "known limitations", but that's fine, I
> guess.
> 
> Note that there are more limitations than those I listed above; not because
> I want to "hoard" them, but because it seems rather trivial to come up with
> those if one thinks about it for 5 minutes, so I simply assume(d?) you are
> aware of most of them. Here's what I can think of right now; this still may
> not be complete; nor do I mean to say those should hold up anything; I merely
> want to get them out to prevent any future accusations about "hoarding"
> stuff)
> 
> - close hg branches cannot be referenced from git, and thus not easily
>   reopened (at least not from their old tip)

Easy to fix: closed-branches/$name, or only
refs/hg/$remote/closed-branches/$name.

Hardly an issue. Nobody has reported this.

> - multiple tips/heads of a hg branch cannot be referenced; only a single
>   "random" head (in the sense that it is not documented how it is chosen)
>   is visible

Not random, the tip of the branch.

And who cares?

> - more name clashes, e.g. the somewhat hypothetical case of having a hg
>   branch "foo" and a hg bookmark "branches/foo"

Very important (not).

> - renaming remotes is a bit problematic (this is IMHO a shortcoming of the
>   remote helpers interface, not of git-remote-{hg,bzr}). I.e. it
>   can result in commits being re-exported, and hence in diverging history.

That's not true. That's an issue of gitifyhg, not git-remote-hg.

> - Not sure if this is still a problem, but I was seeing some issues when
>   a hg remote pruned some commits. Of course this is a bad thing to do in
>   the first place, but sadly it sometimes happens. IIRC this resulted in a
>   truncated marks file (which, as I understand, should be fixed already),
>   but I am a bit fuzzy on the details (it was quite some time ago, and since
>   something bad and exceptional was done on the hg side, I didn't bother to
>   look closer at it.

I don't see how that could be of any issue issue since the mark files have been
updated.

I don't see how any of these should prevent the move out of contrib, so how is
that relevant to the $subject? Most of these are so marginal that I wouldn't
even bother documenting them. Maybe making a list as a to-consider, or
to-think-about-fixing-later would make sense, but with the exception of closed
branches and foo vs foo/bar, I don't see much point in worrying about these
myself. Of course patches are welcome.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-24  0:23         ` Felipe Contreras
@ 2014-04-25 22:26           ` Max Horn
  2014-04-26  1:28             ` Felipe Contreras
  0 siblings, 1 reply; 17+ messages in thread
From: Max Horn @ 2014-04-25 22:26 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 5988 bytes --]

Hi,

I am going to step away from this painful discussion and also this mailing
list, but I owed it to send one last reply with two of the problems I am
still seeing, simply in the hope that this will benefit some future
git-remote-hg users, but also to dispel any claims I was "hoarding" bugs to
somehow hurt Felipe.

Beyond that, I refuse to further "discuss" with Felipe on anything. It leads
nowhere, and he is so full of himself that it seems impossible to reason
with him. I will also refrain from countering everything he said, even
though I am sure he'll construe this as me admitting that he is right. I
don't care enough to try to keep up the flames *shrug*.  In the end,
everybody here can interpret this in whichever way they like.

Ah well, OK, I can't resist one last retort to one point Felipe wrote:

On 24.04.2014, at 02:23, Felipe Contreras <felipe.contreras@gmail.com> wrote:

> Max Horn wrote:
[...]
> 
>> Out of curiosity: How do you yourself use git-remote-hg in your daily work?
> 
> I don't.

So you don't eat your own dog-food (at most you sometimes snack on it)?
That would explain a lot...



Now to the issues: The following results are based on git "next" at revision
e8486314780a4. In addition, I cobbled together fixes from Felipe's
repository, namely the commit [1] he claimed to have fixed the multi head
issue I mentioned, as well as the changes from the fc/remote/fixes branch on
his github repository [2].

It is very well possible that this is still not the latest and greatest, and
that I missed some important patch that changes everything -- it's hard to
tell given the multitude of branches in Felipe's repository. Anyway, to
avoid confusion, I put my merged version of the script into a Gist [3], so
if I made a mistake there, it should be easy to discover.

Given that, the following script (which Felipe already knows from his
issue tracker [4]) still causes a fast-import crash after which "git pull"
from the remote hg repository is impossible, and the user has no idea
how to recover.

-- 8< --
#!/bin/sh -ex
rm -rf hgrepo gitrepo

# Create a multi-head repository
hg init hgrepo
cd hgrepo
echo a > a && hg add a && hg commit -m a
echo b > b && hg add b && hg commit -m b
hg update 0
echo c > c && hg add c && hg commit -m c
cd ..

# Clone it via remote-hg
git clone "hg::hgrepo" gitrepo

cd gitrepo
git gc --prune=now
git pull  # error
-- 8< --

Which results in this:

WARNING: Branch 'default' has more than one head, consider merging
fatal: object not found: 61780798228d17af2d34fce4cfbdf35556832472
fast-import: dumping crash report to .git/fast_import_crash_78219
fatal: Error while running fast-import

Any subsequent pulls then give something like this:

WARNING: Branch 'default' has more than one head, consider merging
fatal: mark :6 not declared
fast-import: dumping crash report to .git/fast_import_crash_78834
fatal: Error while running fast-import


What happens here is that a hg branch with two heads is created; this
repository is cloned via git-remote-hg. Both heads of the hg branch are
imported to git, but only one is referenced by a git ref. We then prune, and
end up with a "missing" commit that is still referenced by the marks file.

Note that the "next" version I used has fc/transport-helper-sync-error-fix,
but this did not stop "git fast-import" from trashing the marks file. :-(



The second script is similar but uses a closed branch to trigger essentially
the same issue. Background: closed branches are ignored by git-remote-hg,
meaning that no git ref is created for them, which can again lead to a
commit without a git ref but referenced by the marks file(s). However,
reproducing the bug in this case is a bit more complicated, because the
problem is obscured by another bug (?): Namely, if a hg branch is closed,
then git-remote-hg starts ignoring it, but does not seem to remove the ref
created for that branch.  So, the git user will not see that the remote
branch was closed (which is a bug, I'd say); on the upside, since the ref is
still around, no dangling commit is produced.

But we can "work around" this by re-opening the hg branch at a different
position, i.e. as child of an unrelated commit, which does *not* have the
commits of the original branch as parents. If we do that, git-remote-hg
moves the git ref to point to the new branch tip, and again we end up with a
dangling commit (the git commit corresponding to the former hg branch tip).

Again, this is something me and also colleagues "discovered" in real life
usage. So don't be fooled by the somewhat convoluted test script. This *does*
happen.

-- 8< ---
#!/bin/sh -ex

rm -rf hgrepo gitrepo

# Create a repository with two branches
hg init hgrepo
cd hgrepo
echo a > a && hg add a && hg commit -m a
hg update 0
hg branch foobar
echo b > b && hg add b && hg commit -m b
hg update default
cd ..

# Clone it via remote-hg
git clone "hg::hgrepo" gitrepo
cd gitrepo
git gc --prune=now
git pull
cd ..

# close the branch
cd hgrepo
hg update foobar
hg commit --close-branch -m "close branch"
hg update default
cd ..

cd gitrepo
git gc --prune=now
git pull
# This pull should trigger the issue, but for some reason, the ref
# origin/branches/foobar is still around, and so nothing happens.
cd ..

cd hgrepo
hg update default
hg branch -f foobar
echo c > c && hg add c && hg commit -m c
hg update default
cd ..

cd gitrepo
git gc --prune=now
git pull
# at this point, origin/branches/foobar is gone
git gc --prune=now
git pull      # now we get the error
-- 8< --


Goodbye,
Max


[1] https://github.com/felipec/git/commit/fbaae8caa51804a655fd6bc5727763b64e3c2e9f
[2] https://github.com/felipec/git/commit/1bf5fc892ebaa4a07dcd71ef96f8a8f5c876cb5f
[3] https://gist.github.com/fingolfin/11296352
[4] https://github.com/felipec/git/issues/56#issuecomment-40305442


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/3] remote-helpers: move out of contrib
  2014-04-25 22:26           ` Max Horn
@ 2014-04-26  1:28             ` Felipe Contreras
  0 siblings, 0 replies; 17+ messages in thread
From: Felipe Contreras @ 2014-04-26  1:28 UTC (permalink / raw)
  To: Max Horn, Felipe Contreras; +Cc: git

Max Horn wrote:
> I am going to step away from this painful discussion and also this mailing
> list, but I owed it to send one last reply with two of the problems I am
> still seeing, simply in the hope that this will benefit some future
> git-remote-hg users, but also to dispel any claims I was "hoarding" bugs to
> somehow hurt Felipe.
> 
> Beyond that, I refuse to further "discuss" with Felipe on anything. It leads
> nowhere, and he is so full of himself that it seems impossible to reason
> with him. I will also refrain from countering everything he said, even
> though I am sure he'll construe this as me admitting that he is right. I
> don't care enough to try to keep up the flames *shrug*.  In the end,
> everybody here can interpret this in whichever way they like.

I don't construe anything as an admission that I'm right. The $subject is that
git-remote-hg/bzr are ready to be moved out of contrib, that's my contention
and I don't see why would anyone think that it's not right.

> Ah well, OK, I can't resist one last retort to one point Felipe wrote:
> 
> On 24.04.2014, at 02:23, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> 
> > Max Horn wrote:
> [...]
> > 
> >> Out of curiosity: How do you yourself use git-remote-hg in your daily work?
> > 
> > I don't.
> 
> So you don't eat your own dog-food (at most you sometimes snack on it)?
> That would explain a lot...

Fortunately I don't use Mercurial in my daily work, so I can't use
git-remote-hg on my daily work. It's not a matter of will.

> Now to the issues: The following results are based on git "next" at revision
> e8486314780a4. In addition, I cobbled together fixes from Felipe's
> repository, namely the commit [1] he claimed to have fixed the multi head
> issue I mentioned, as well as the changes from the fc/remote/fixes branch on
> his github repository [2].
> 
> It is very well possible that this is still not the latest and greatest, and
> that I missed some important patch that changes everything -- it's hard to
> tell given the multitude of branches in Felipe's repository. Anyway, to
> avoid confusion, I put my merged version of the script into a Gist [3], so
> if I made a mistake there, it should be easy to discover.
> 
> Given that, the following script (which Felipe already knows from his
> issue tracker [4]) still causes a fast-import crash after which "git pull"
> from the remote hg repository is impossible, and the user has no idea
> how to recover.
> 
> -- 8< --
> #!/bin/sh -ex
> rm -rf hgrepo gitrepo
> 
> # Create a multi-head repository
> hg init hgrepo
> cd hgrepo
> echo a > a && hg add a && hg commit -m a
> echo b > b && hg add b && hg commit -m b
> hg update 0
> echo c > c && hg add c && hg commit -m c
> cd ..
> 
> # Clone it via remote-hg
> git clone "hg::hgrepo" gitrepo
> 
> cd gitrepo
> git gc --prune=now
> git pull  # error
> -- 8< --
> 
> Which results in this:
> 
> WARNING: Branch 'default' has more than one head, consider merging
> fatal: object not found: 61780798228d17af2d34fce4cfbdf35556832472
> fast-import: dumping crash report to .git/fast_import_crash_78219
> fatal: Error while running fast-import
> 
> Any subsequent pulls then give something like this:
> 
> WARNING: Branch 'default' has more than one head, consider merging
> fatal: mark :6 not declared
> fast-import: dumping crash report to .git/fast_import_crash_78834
> fatal: Error while running fast-import

Clearly you are doing something wrong, because 'next' with the git-remote-hg
you put in gist[1] doesn't trigger this issue. I tested.

> What happens here is that a hg branch with two heads is created; this
> repository is cloned via git-remote-hg. Both heads of the hg branch are
> imported to git,

No, they are not. You must be using a version of git-remote-hg other than the
one you said you did.

> The second script is similar but uses a closed branch to trigger essentially
> the same issue. Background: closed branches are ignored by git-remote-hg,
> meaning that no git ref is created for them, which can again lead to a
> commit without a git ref but referenced by the marks file(s). However,
> reproducing the bug in this case is a bit more complicated, because the
> problem is obscured by another bug (?): Namely, if a hg branch is closed,
> then git-remote-hg starts ignoring it, but does not seem to remove the ref
> created for that branch.  So, the git user will not see that the remote
> branch was closed (which is a bug, I'd say); on the upside, since the ref is
> still around, no dangling commit is produced.
> 
> But we can "work around" this by re-opening the hg branch at a different
> position, i.e. as child of an unrelated commit, which does *not* have the
> commits of the original branch as parents. If we do that, git-remote-hg
> moves the git ref to point to the new branch tip, and again we end up with a
> dangling commit (the git commit corresponding to the former hg branch tip).
> 
> Again, this is something me and also colleagues "discovered" in real life
> usage. So don't be fooled by the somewhat convoluted test script. This *does*
> happen.
> 
> -- 8< ---
> #!/bin/sh -ex
> 
> rm -rf hgrepo gitrepo
> 
> # Create a repository with two branches
> hg init hgrepo
> cd hgrepo
> echo a > a && hg add a && hg commit -m a
> hg update 0
> hg branch foobar
> echo b > b && hg add b && hg commit -m b
> hg update default
> cd ..
> 
> # Clone it via remote-hg
> git clone "hg::hgrepo" gitrepo
> cd gitrepo
> git gc --prune=now
> git pull
> cd ..
> 
> # close the branch
> cd hgrepo
> hg update foobar
> hg commit --close-branch -m "close branch"
> hg update default
> cd ..
> 
> cd gitrepo
> git gc --prune=now
> git pull
> # This pull should trigger the issue, but for some reason, the ref
> # origin/branches/foobar is still around, and so nothing happens.
> cd ..
> 
> cd hgrepo
> hg update default
> hg branch -f foobar
> echo c > c && hg add c && hg commit -m c
> hg update default
> cd ..
> 
> cd gitrepo
> git gc --prune=now
> git pull
> # at this point, origin/branches/foobar is gone
> git gc --prune=now
> git pull      # now we get the error
> -- 8< --

Fixed.
http://pastie.org/9113797

[1] https://gist.github.com/fingolfin/11296352

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-04-26  1:39 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-21 20:37 [PATCH v2 0/3] remote-helpers: graduate Felipe Contreras
2014-04-21 20:37 ` [PATCH v2 1/3] remote-helpers: squelch python import exceptions Felipe Contreras
2014-04-21 20:37 ` [PATCH v2 2/3] remote-helpers: move out of contrib Felipe Contreras
2014-04-21 21:22   ` Junio C Hamano
2014-04-21 21:24     ` Felipe Contreras
2014-04-21 21:42       ` Junio C Hamano
2014-04-23 13:10   ` Max Horn
2014-04-23 19:20     ` Junio C Hamano
2014-04-23 21:00       ` Felipe Contreras
2014-04-23 21:30         ` Junio C Hamano
2014-04-23 20:12     ` Junio C Hamano
2014-04-23 20:54     ` Felipe Contreras
2014-04-23 22:41       ` Max Horn
2014-04-24  0:23         ` Felipe Contreras
2014-04-25 22:26           ` Max Horn
2014-04-26  1:28             ` Felipe Contreras
2014-04-21 20:37 ` [PATCH v2 3/3] remote-helpers: move tests " Felipe Contreras

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).