Re: [RFCv3 2/4] Add Python support library for CVS remote helper

10 messages, 4 authors, 2016-06-15 · open the first message on its own page

Re: [RFCv3 2/4] Add Python support library for CVS remote helper

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:15

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.

[PATCH 2/2] git_remote_cvs: Use $(shell) in the Makefile

From: David Aguilar <hidden>
Date: 2016-06-15 22:47:15

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

[PATCH 1/2] git_remote_cvs: Honor DESTDIR in the Makefile

From: David Aguilar <hidden>
Date: 2016-06-15 22:47:15

This adds the --root=<path> flag to setup.py so that the
user-provided DESTDIR is honored.

Signed-off-by: David Aguilar <redacted>
---
 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

Re: [PATCH 2/2] git_remote_cvs: Use $(shell) in the Makefile

From: David Aguilar <hidden>
Date: 2016-06-15 22:47:15

On Sun, Aug 16, 2009 at 01:38:08PM -0700, David Aguilar wrote:
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 <redacted>
---
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
$(shell ...) was not the right thing to do.



quoted hunk
 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
-- 
		David

Re: [PATCH 1/2] git_remote_cvs: Honor DESTDIR in the Makefile

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:47:15

Hi,

On Sun, 16 Aug 2009, David Aguilar wrote:
quoted hunk
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 = /
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

Re: [PATCH 1/2] git_remote_cvs: Honor DESTDIR in the Makefile

From: David Aguilar <hidden>
Date: 2016-06-15 22:47:15

On Sun, Aug 16, 2009 at 10:55:29PM +0200, Johannes Schindelin wrote:
Hi,

On Sun, 16 Aug 2009, David Aguilar wrote:
quoted
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 = /
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.

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

Re: [PATCH 1/2] git_remote_cvs: Honor DESTDIR in the Makefile

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:47:15

Hi,

On Sun, 16 Aug 2009, David Aguilar wrote:
On Sun, Aug 16, 2009 at 10:55:29PM +0200, Johannes Schindelin wrote:
quoted
On Sun, 16 Aug 2009, David Aguilar wrote:
quoted
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 = /
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.

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

[PATCH v2 1/2] git_remote_cvs: Honor DESTDIR in the Makefile

From: David Aguilar <hidden>
Date: 2016-06-15 22:47:15

This modifies the setup.py invocation so that user-defined
DESTDIRs are taken into account.

Signed-off-by: David Aguilar <redacted>
---

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

[PATCH v2 2/2] git_remote_cvs: Use $(shell) in the Makefile

From: David Aguilar <hidden>
Date: 2016-06-15 22:47:15

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

Re: [PATCH 1/2] git_remote_cvs: Honor DESTDIR in the Makefile

From: Johan Herland <hidden>
Date: 2016-06-15 22:47:15

On Sunday 16 August 2009, David Aguilar wrote:
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.
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 
earlier (including _lots_ of PEP8 fixes).
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.
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, [off-list ref]
www.herland.net
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help