login
Search
Hi, Another iteration of the patch series implementing a CVS remote helper, as promised a couple of days ago. Changes from the previous iteration: - Rebased on top of Daniel Barkalow's latest foreign VCS helpers work (aka. 'db/vcs-helper' (early part)). This means that all the "generic" foreign-scm patches that were part of the previous iteration are no longer needed, and only the CVS remote helper patches remain. Also, this series applies cleanly to current 'pu'. - Replaced the "git-vcs-cvs" naming with "git-remote-cvs" throughout the code and documentation. - Minor updates to the git-remote-cvs implementation in order to more closely follow the current remote helper API. - Split up the patch series into somewhat smaller patches to (hopefully) be allowed onto git@vger.kernel.org. Have fun! :) ...Johan Johan Herland (4): Basic build infrastructure for Python scripts Add Python support library for CVS remote helper Third draft of CVS remote helper program Add simple selftests of git-remote-cvs functionality Documentation/git-remote-cvs.txt | 85 ++++ Makefile | 46 ++ configure.ac | 3 + git-remote-cvs.py | 697 ++++++++++++++++++++++++++++ git_remote_cvs/.gitignore | 2 + git_remote_cvs/Makefile | 27 ++ git_remote_cvs/changeset.py | 114 +++++ git_remote_cvs/commit_states.py | 52 +++ git_remote_cvs/cvs.py | 884 ++++++++++++++++++++++++++++++++++++ git_remote_cvs/cvs_revision_map.py | 362 +++++++++++++++ git_remote_cvs/cvs_symbol_cache.py | 283 ++++++++++++ git_remote_cvs/git.py | 586 ++++++++++++++++++++++++ git_remote_cvs/setup.py | 12 + git_remote_cvs/util.py | 147 ++++++ t/t9800-remote-cvs-basic.sh | 524 +++++++++++++++++++++ t/t9801-remote-cvs-fetch.sh | 291 ++++++++++++ t/test-lib.sh | 1 + 17 files changed, ...
This patch adds basic boilerplate support (based on corresponding Perl sections) for enabling the building and installation Python scripts. There are currently no Python scripts being built, and when Python scripts are added in future patches, their building and installation can be disabled by defining NO_PYTHON. Signed-off-by: Johan Herland <johan@herland.net> --- Makefile | 13 +++++++++++++ configure.ac | 3 +++ t/test-lib.sh | 1 + 3 files changed, 17 insertions(+), 0 deletions(-) diff --git a/Makefile b/Makefile index 189aee5..969cef5 100644 --- a/Makefile +++ b/Makefile @@ -166,6 +166,8 @@ all:: # # Define NO_PERL if you do not want Perl scripts or libraries at all. # +# Define NO_PYTHON if you do not want Python scripts or libraries at all. +# # Define NO_TCLTK if you do not want Tcl/Tk GUI. # # The TCL_PATH variable governs the location of the Tcl interpreter @@ -311,6 +313,7 @@ LIB_H = LIB_OBJS = PROGRAMS = SCRIPT_PERL = +SCRIPT_PYTHON = SCRIPT_SH = TEST_PROGRAMS = @@ -349,6 +352,7 @@ SCRIPT_PERL += git-svn.perl SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \ $(patsubst %.perl,%,$(SCRIPT_PERL)) \ + $(patsubst %.py,%,$(SCRIPT_PYTHON)) \ git-instaweb # Empty... @@ -404,8 +408,12 @@ endif ifndef PERL_PATH PERL_PATH = /usr/bin/perl endif +ifndef PYTHON_PATH + PYTHON_PATH = /usr/bin/python +endif export PERL_PATH +export PYTHON_PATH LIB_FILE=libgit.a XDIFF_LIB=xdiff/lib.a @@ -1259,6 +1267,10 @@ ifeq ($(PERL_PATH),) NO_PERL=NoThanks endif +ifeq ($(PYTHON_PATH),) +NO_PYTHON=NoThanks +endif + QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir QUIET_SUBDIR1 = @@ -1306,6 +1318,7 @@ prefix_SQ = $(subst ','\'',$(prefix)) SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH)) +PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH)) TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH)) LIBS = $(GITLIBS) $(EXTLIBS) diff --git a/configure.ac ...
Add two new selftests: - t9800-remote-cvs-basic: Test the git-remote-cvs implementation of the remote helper API, by verifying the expected output of the git-remote-cvs program when invoking remote helper API commands while doing some simple CVS operations. - t9801-remote-cvs-fetch: A more high-level test of the fetch/import-side of the git-remote-cvs implementation, by verifying the expected repository state after doing "git fetch" from a CVS remote where a variety of CVS operations are being performed. Signed-off-by: Johan Herland <johan@herland.net> --- t/t9800-remote-cvs-basic.sh | 524 +++++++++++++++++++++++++++++++++++++++++++ t/t9801-remote-cvs-fetch.sh | 291 ++++++++++++++++++++++++ 2 files changed, 815 insertions(+), 0 deletions(-) create mode 100755 t/t9800-remote-cvs-basic.sh create mode 100755 t/t9801-remote-cvs-fetch.sh diff --git a/t/t9800-remote-cvs-basic.sh b/t/t9800-remote-cvs-basic.sh new file mode 100755 index 0000000..6ddec17 --- /dev/null +++ b/t/t9800-remote-cvs-basic.sh @@ -0,0 +1,524 @@ +#!/bin/sh + +test_description='git remote-cvs basic tests' +. ./test-lib.sh + +if ! test_have_prereq PYTHON; then + say 'skipping CVS foreign-vcs helper tests, python not available' + test_done +fi + +CVS_EXEC=cvs +CVS_OPTS="-f -q" +CVS="$CVS_EXEC $CVS_OPTS" + +CVSROOT=$(pwd)/cvsroot +export CVSROOT +unset CVS_SERVER + +CVSMODULE=cvsmodule +GITREMOTE=cvsremote + +if ! type $CVS_EXEC >/dev/null 2>&1 +then + say 'skipping remote-cvs tests, $CVS_EXEC not found' + test_done +fi + +test_expect_success 'setup cvsroot' '$CVS init' + +test_expect_success '#1: setup a cvs module' ' + + mkdir "$CVSROOT/$CVSMODULE" && + $CVS co -d module-cvs $CVSMODULE && + ( + cd module-cvs && + cat <<EOF >o_fortuna && +O Fortuna +velut luna +statu variabilis, + +semper crescis +aut decrescis; +vita detestabilis + +nunc obdurat +et tunc curat +ludo mentis aciem, + +egestatem, +potestatem +dissolvit ut glaciem. +EOF + $CVS add ...
Implements the import of objects from a local or remote CVS repository. This helper program uses the "git_remote_cvs" Python package introduced earlier, and provides a working draft implementation of the remote helper API, as described in Documentation/git-remote-helpers.txt. Further details about this specific helper are described in the new Documentation/git-remote-cvs.txt. This patch has been improved by the following contributions: - Daniel Barkalow: Updates reflecting changes in remote helper API Signed-off-by: Johan Herland <johan@herland.net> --- Documentation/git-remote-cvs.txt | 85 +++++ Makefile | 24 ++ git-remote-cvs.py | 697 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 806 insertions(+), 0 deletions(-) create mode 100644 Documentation/git-remote-cvs.txt create mode 100755 git-remote-cvs.py diff --git a/Documentation/git-remote-cvs.txt b/Documentation/git-remote-cvs.txt new file mode 100644 index 0000000..783d542 --- /dev/null +++ b/Documentation/git-remote-cvs.txt @@ -0,0 +1,85 @@ +git-remote-cvs(1) +============== + +NAME +---- +git-remote-cvs - Helper program for interoperation with CVS repositories + +SYNOPSIS +-------- +'git remote-cvs' <remote> + +DESCRIPTION +----------- + +Please see the linkgit:git-remote-helpers[1] documentation for general +information about remote helper programs. + +CONFIGURATION +------------- + +remote.*.cvsRoot:: + The URL of the CVS repository (as found in a `CVSROOT` variable, or + in a `CVS/Root` file). + Example: "`:pserver:user@server/var/cvs/cvsroot`". + +remote.*.cvsModule:: + The path of the CVS module (as found in a `CVS/Repository` file) + within the CVS repository specified in `remote.*.cvsRoot`. + Example: "`foo/bar`" + +remote.*.cachedSymbolsOnly:: + When 'true', a cache of CVS symbols is used instead of querying the + CVS server for all existing symbols (potentially expensive). In this + mode, git-remote-cvs will not discover new ...
This patch introduces a Python package called "git_remote_cvs" containing the building blocks of the CVS remote helper. The CVS remote helper itself is NOT part of this patch. The patch includes the necessary Makefile additions to build and install the git_remote_cvs Python package along with the rest of Git. Signed-off-by: Johan Herland <johan@herland.net> --- Makefile | 9 + git_remote_cvs/.gitignore | 2 + git_remote_cvs/Makefile | 27 ++ git_remote_cvs/changeset.py | 114 +++++ git_remote_cvs/commit_states.py | 52 +++ git_remote_cvs/cvs.py | 884 ++++++++++++++++++++++++++++++++++++ git_remote_cvs/cvs_revision_map.py | 362 +++++++++++++++ git_remote_cvs/cvs_symbol_cache.py | 283 ++++++++++++ git_remote_cvs/git.py | 586 ++++++++++++++++++++++++ git_remote_cvs/setup.py | 12 + git_remote_cvs/util.py | 147 ++++++ 11 files changed, 2478 insertions(+), 0 deletions(-) create mode 100644 git_remote_cvs/.gitignore create mode 100644 git_remote_cvs/Makefile create mode 100644 git_remote_cvs/__init__.py create mode 100644 git_remote_cvs/changeset.py create mode 100644 git_remote_cvs/commit_states.py create mode 100644 git_remote_cvs/cvs.py create mode 100644 git_remote_cvs/cvs_revision_map.py create mode 100644 git_remote_cvs/cvs_symbol_cache.py create mode 100644 git_remote_cvs/git.py create mode 100644 git_remote_cvs/setup.py create mode 100644 git_remote_cvs/util.py diff --git a/Makefile b/Makefile index 969cef5..bb5cea2 100644 --- a/Makefile +++ b/Makefile @@ -1350,6 +1350,9 @@ endif ifndef NO_PERL $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all endif +ifndef NO_PYTHON + $(QUIET_SUBDIR0)git_remote_cvs $(QUIET_SUBDIR1) PYTHON_PATH='$(PYTHON_PATH_SQ)' prefix='$(prefix_SQ)' all +endif $(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) please_set_SHELL_PATH_to_a_more_modern_shell: @@ ...
Importing * is frowned upon in Python. It's much easier to see where things are coming from if you 'import util' and use the namespaced util.foo() way of accessing the functions. Furthermore, you're going to want to use absolute imports. Anyone can create 'util.py' and blindly importing 'util' is asking for trouble. Instead use: I think it reads better as Changeset(object) (drop the spaces before the parens). That applies to the rest of this patch as well. This also had me wondering about the following: git uses tabs for indentation BUT, the python convention is to use 4-space indents ala PEP-8 http://www.python.org/dev/peps/pep-0008/ It might be appealing to when-in-Rome (Rome being Python) here and do things the python way when we code in Python. Consistency with pep8 is good if we expect to get python hackers __slots__ is pretty esoteric in Python-land. But, if your justification is to minimize memory usage, then pep8 and other parts of the git codebase recommend against lining up the equals signs like that. Ya, sorry for the nits Similar to the git coding style, this might be better written: ... if len(msg) > 25: msg = msg[:22] + '...' # Max 25 chars long ... (aka avoid single-line ifs) enumerate has special meaning in Python. items = (1, 2, 3, 4) for idx, item in enumerate(items): print idx, item asserts go away when running with PYTHONOPTIMIZE. If this is really an error then we should we raise an exception Hmm.. Does it make more sense to use the unittest module? We definitely need absolute imports here. 'import git' could find the git-python project's git module. Nonetheless, interesting stuff. -- David --
First, thank you very much for the review. It is very helpful, and I really appreciate it. I'd rather do "from util import X Y Z", as the util stuff is used all over I thought the python import rules specified that the current package was consulted first, and therefore the 'util' package would always come from the current package. However, I must confess that I don't know these rules very I see your point, but I believe that since git_remote_cvs is not an independent project (but very much coupled to git), its allegiance is with Git, and it should therefore follow the Git coding style. In other words, I Yes, I only use __slots__ for classes that potentially have a large number I can't find a good rationale for this rule in PEP8 (other than Guido's personal style), and I personally find the above much more readable (otherwise I wouldn't go through the trouble of lining them all up...). Can I use asserts to verify pre/post-conditions and other invariants. I believe that if this assert fails, it is indicative of something horribly wrong with the code itself. However, I now see that one can also trigger this case with bad input (e.g. CvsNum("1.2.3.0").parent()). I will keep the assert here, Thanks for the review! Have fun! :) ...Johan -- Johan Herland, <johan@herland.net> www.herland.net --
Heya, Working with python a lot myself, if you want to claim readability _to python hackers_, then you should follow PEP8. If you want to follow your own personal style which is easily readable to you, then by all means; just know that people that read python a lot will have a hard time reading your code ;). -- Cheers, Sverre Rabbelier --
I think you are missing the point. It may be true that the rules in PEP8 were *originally* written according to the unjustified whims of the BDFL, but now that they are established the reason for following them is not that Guido likes them but rather to be consistent with the bulk of other Python code on the planet. With respect to the rule to use 4-space indents, there are serious practical problems with using tabs *in addition to* the consistency argument. Michael --
There are? What arguments? Assuming I don't mix spaces and tabs (which I certainly don't), I can't see any "practical problems" with using tabs (except for the PEP8/consistency issue). ...Johan -- Johan Herland, <johan@herland.net> www.herland.net --
Not relevant. That is a rule for our "C" source code. We also use it in our Perl scripts and shell scripts because there is no single "one right way" that is strongly defined and everybody adheres to, like the 4-space So is this one. If experienced Python folks also frown on single-line conditionals, then by all means please update this. But if this suggestion is solely because we don't do a single-line conditional in our C source code, then please do not insist on it too strongly. The code should look familiar to Pythonistas with good tastes (if such a class of people exist, that is ;-)). --
Ok. Thanks. I will follow PEP8 as closely as possible, including the 4-space indent. ...Johan -- Johan Herland, <johan@herland.net> www.herland.net --
It appears that the "make install" step with this patch is broken, trying to write into /usr/lib/python2.6/ without honoring DESTDIR. It needs to be resolved before the series nears 'master', preferrably before it hits 'next', as "make rpm" step is one of the things that is broken by this. I am sure people who are more savvy on Python can offer help. Thanks. --
This adds the --root=<path> flag to setup.py so that the user-provided DESTDIR is honored. Signed-off-by: David Aguilar <davvid@gmail.com> --- git_remote_cvs/Makefile | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/git_remote_cvs/Makefile b/git_remote_cvs/Makefile index 8dbf3fa..f52c096 100644 --- a/git_remote_cvs/Makefile +++ b/git_remote_cvs/Makefile @@ -3,6 +3,15 @@ # pysetupfile:=setup.py +# Setup the DESTDIR for Python. +ifeq ($(DESTDIR),) +PYTHON_DESTDIR = / +else +PYTHON_DESTDIR = $(DESTDIR) +endif +# Shell quote (do not use $(call) to accommodate ancient setups); +PYTHON_DESTDIR_SQ = $(subst ','\'',$(PYTHON_DESTDIR)) + ifndef PYTHON_PATH PYTHON_PATH = /usr/bin/python endif @@ -19,7 +28,10 @@ PYLIBDIR=`$(PYTHON_PATH) -c "import sys; print 'lib/python%i.%i/site-packages' % all: $(pysetupfile) $(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build install: $(pysetupfile) - $(PYTHON_PATH) $(pysetupfile) install --prefix $(prefix) + $(PYTHON_PATH) $(pysetupfile) install \ + --prefix $(prefix) \ + --root $(PYTHON_DESTDIR_SQ) + instlibdir: $(pysetupfile) @echo "$(prefix)/$(PYLIBDIR)" clean: -- 1.6.4.169.g64d5 --
This updates the git_remote_cvs Makefile to use the same $(shell <cmd>) style used by the top-level git Makefile. Signed-off-by: David Aguilar <davvid@gmail.com> --- git_remote_cvs/Makefile | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/git_remote_cvs/Makefile b/git_remote_cvs/Makefile index f52c096..d281d48 100644 --- a/git_remote_cvs/Makefile +++ b/git_remote_cvs/Makefile @@ -23,10 +23,13 @@ ifndef V QUIETSETUP = --quiet endif -PYLIBDIR=`$(PYTHON_PATH) -c "import sys; print 'lib/python%i.%i/site-packages' % sys.version_info[:2]"` +PYLIBDIR=$(shell $(PYTHON_PATH) -c \ + "import sys; \ + print 'lib/python%i.%i/site-packages' % sys.version_info[:2]") all: $(pysetupfile) $(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build + install: $(pysetupfile) $(PYTHON_PATH) $(pysetupfile) install \ --prefix $(prefix) \ @@ -34,6 +37,7 @@ install: $(pysetupfile) instlibdir: $(pysetupfile) @echo "$(prefix)/$(PYLIBDIR)" + clean: $(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) clean -a $(RM) *.pyo *.pyc -- 1.6.4.169.g64d5 --
I should have mentioned here that I also spaced stuff out and chopped the long line so that it fits within 78 chars. I intentionally broke this out as a 2nd patch in case using -- David --
Hi, Hmm. I think this would break on msysGit. Not that anybody worked on getting Python to compile on msysGit. (Just to make sure you understand the issue: on msysGit, we set prefix to "" (and I think DESTDIR somehow ends up taking on the same value). Now, when DESTDIR is set to "/" and something wants to be copied to $(DESTDIR)/something, the latter expands to //something, which tells MSys not to expand //something to the correct Windows path. Ciao, Dscho --
I see. Hmm.. setup.py is a real pain. I'll see if we rework this so that we end up passing "" to --root instead of /. I'm going to be gone for a few hours so probably won't be able to try it out until tonight. Another thing to consider -- Debian once submitted a bug against another Python app asking that we not place modules in site-packages unless we plan on having other applications importing those modules. The more appropriate place for them if we don't plan on that is $(prefix)/share/git-core/git_remote_cvs or something like that. I guess that's another thing to think about. -- David --
Hi, Thinking about it a bit more: you might not need to do anything. I think that msysGit should move to prefix = /mingw anyway. I hesitated to do that earlier, as all .perl and .sh scripts need MSys. But the Git .exe files are MinGW programs, so technically they belong into /mingw/bin/ anyway. But the issue would only really become relevant if anybody supported Python on msysGit (and thereby git-remote-cvs, before that, we cannot make use of it). Just a thing to keep in mind. Ciao, Dscho --
Thanks a lot for your work! I will send an updated series shortly which will include v2 of your DESTDIR/Makefile fixes, and also the fixes you suggested Yes, Debian raises a valid point. I haven't thought much about making the git_remote_cvs package into something that would be useful for other applications. (I just assumed that the Python convention was to install it into site-packages regardless...) For now, I'll concentrate on git-remote- cvs, and leave it to others to figure out if anything in the git_remote_cvs package is useful for other programs. Note that there's a small chicken-and-egg problem here as well: If Debian refuses us to install into site-packages, it will be harder for other Python programs to discover (and import) the git_remote_cvs package. BTW, when we're on the subject of packaging: There are some variables in git_remote_cvs/setup.py where I'm not sure what the correct value should be: - version - should this follow Git's version number, or is it independent? - author (and author_email + url) - For now, I'm referring to the Git community. Should this be more specific/ Feedback welcome. Have fun! :) ...Johan -- Johan Herland, <johan@herland.net> www.herland.net --
This modifies the setup.py invocation so that user-defined DESTDIRs are taken into account. Signed-off-by: David Aguilar <davvid@gmail.com> --- setup.py gets confused if we use --root like in v1 of this patch. I think this is simpler. Thoughts? git_remote_cvs/Makefile | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/git_remote_cvs/Makefile b/git_remote_cvs/Makefile index 8dbf3fa..2e26dbe 100644 --- a/git_remote_cvs/Makefile +++ b/git_remote_cvs/Makefile @@ -3,6 +3,9 @@ # pysetupfile:=setup.py +# Shell quote (do not use $(call) to accommodate ancient setups); +DESTDIR_SQ = $(subst ','\'',$(DESTDIR)) + ifndef PYTHON_PATH PYTHON_PATH = /usr/bin/python endif @@ -19,7 +22,8 @@ PYLIBDIR=`$(PYTHON_PATH) -c "import sys; print 'lib/python%i.%i/site-packages' % all: $(pysetupfile) $(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build install: $(pysetupfile) - $(PYTHON_PATH) $(pysetupfile) install --prefix $(prefix) + $(PYTHON_PATH) $(pysetupfile) install --prefix $(DESTDIR_SQ)$(prefix) + instlibdir: $(pysetupfile) @echo "$(prefix)/$(PYLIBDIR)" clean: -- 1.6.4.314.g034e1 --
This updates the git_remote_cvs Makefile to use the same $(shell <cmd>) style used by the top-level git Makefile. Signed-off-by: David Aguilar <davvid@gmail.com> --- git_remote_cvs/Makefile | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/git_remote_cvs/Makefile b/git_remote_cvs/Makefile index 2e26dbe..061c247 100644 --- a/git_remote_cvs/Makefile +++ b/git_remote_cvs/Makefile @@ -17,15 +17,19 @@ ifndef V QUIETSETUP = --quiet endif -PYLIBDIR=`$(PYTHON_PATH) -c "import sys; print 'lib/python%i.%i/site-packages' % sys.version_info[:2]"` +PYLIBDIR=$(shell $(PYTHON_PATH) -c \ + "import sys; \ + print 'lib/python%i.%i/site-packages' % sys.version_info[:2]") all: $(pysetupfile) $(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build + install: $(pysetupfile) $(PYTHON_PATH) $(pysetupfile) install --prefix $(DESTDIR_SQ)$(prefix) instlibdir: $(pysetupfile) @echo "$(prefix)/$(PYLIBDIR)" + clean: $(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) clean -a $(RM) *.pyo *.pyc -- 1.6.4.314.g034e1 --
