[PATCH 0/7] pylibfdt: Add installation support

[PATCH 0/7] pylibfdt: Add installation support

[Simon Glass]

This series adds support for installation of the Pylib module. It adjusts
the setup.py file to provide this functionality and the Makefile rules
to call it correctly.

It also adds a way to disable building the Python module. This is useful
since some build systems want to use setup.py to do both the build and
the install step. In this case the correct build commands would be:

   make NO_PYTHON=1
   make install_pylibfdt SETUP_PREFIX=/path/to/install_prefix


Simon Glass (7):
  pylibfdt: Allow pkg-config to be supplied in the environment
  pylibfdt: Allow building to be disabled
  pylibfdt: Use environment to pass C flags and files
  pylibfdt: Use package_dir to set the package directory
  pylibfdt: Enable installation of Python module
  pylibfdt: Use the correct libfdt version in the module
  pylibfdt: Use the call function to simplify the Makefile

 Makefile                   |  6 ++++--
 README                     | 13 +++++++++++++
 pylibfdt/Makefile.pylibfdt | 19 +++++++++++++++++--
 pylibfdt/setup.py          | 22 +++++++++++-----------
 4 files changed, 45 insertions(+), 15 deletions(-)

-- 
2.12.1.578.ge9c3154ca4-goog

[PATCH 1/7] pylibfdt: Allow pkg-config to be supplied in the environment

[Simon Glass]

Some build systems have their own version of the pkg-config tool.
Use a variable for this instead of hard-coding it, to allow for this.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Suggested-by: Mike Frysinger <vapier-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 1d08ec1..e6d8251 100644
--- a/Makefile
+++ b/Makefile
@@ -23,6 +23,7 @@ CFLAGS = -g -Os -fPIC -Werror $(WARNINGS)
 BISON = bison
 LEX = flex
 SWIG = swig
+PKG_CONFIG ?= pkg-config
 
 INSTALL = /usr/bin/install
 DESTDIR =
@@ -119,7 +120,7 @@ SCRIPTS = dtdiff
 # We need both Python and swig to build pylibfdt.
 .PHONY: maybe_pylibfdt
 maybe_pylibfdt: FORCE
-	if pkg-config --cflags python >/dev/null 2>&1; then \
+	if $(PKG_CONFIG) --cflags python >/dev/null 2>&1; then \
 		if which swig >/dev/null 2>&1; then \
 			can_build=yes; \
 		fi; \
-- 
2.12.1.578.ge9c3154ca4-goog

[PATCH 2/7] pylibfdt: Allow building to be disabled

[Simon Glass]

Some build systems want to build python libraries separately from the
rest of the build.

Add a NO_PYTHON option to enable this.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

 Makefile | 1 +
 README   | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/Makefile b/Makefile
index e6d8251..5cf4aee 100644
--- a/Makefile
+++ b/Makefile
@@ -120,6 +120,7 @@ SCRIPTS = dtdiff
 # We need both Python and swig to build pylibfdt.
 .PHONY: maybe_pylibfdt
 maybe_pylibfdt: FORCE
+	if [ -n "${NO_PYTHON}" ]; then exit; fi; \
 	if $(PKG_CONFIG) --cflags python >/dev/null 2>&1; then \
 		if which swig >/dev/null 2>&1; then \
 			can_build=yes; \
diff --git a/README b/README
index 96d8486..d2323fd 100644
--- a/README
+++ b/README
@@ -50,6 +50,12 @@ If you add new features, please check code coverage:
     # Open 'htmlcov/index.html' in your browser
 
 
+To disable building the python library, even if swig and Python are available,
+use:
+
+    make NO_PYTHON=1
+
+
 More work remains to support all of libfdt, including access to numeric
 values.
 
-- 
2.12.1.578.ge9c3154ca4-goog

[PATCH 3/7] pylibfdt: Use environment to pass C flags and files

[Simon Glass]

At present setup.py adjusts its command line when running, so that the
C flags and file list can be passed as arguments. Pass them in environment
variables instead, so we can avoid this messiness. It also allows us to
support the 'install' command.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

 pylibfdt/Makefile.pylibfdt |  3 ++-
 pylibfdt/setup.py          | 16 ++++++----------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
index 0d8c010..3d99fd4 100644
--- a/pylibfdt/Makefile.pylibfdt
+++ b/pylibfdt/Makefile.pylibfdt
@@ -7,7 +7,8 @@ PYMODULE = $(PYLIBFDT_objdir)/_libfdt.so
 
 $(PYMODULE): $(PYLIBFDT_srcs) $(WRAP)
 	@$(VECHO) PYMOD $@
-	python $(PYLIBFDT_objdir)/setup.py "$(CPPFLAGS)" $^
+	SOURCES="$^" CPPFLAGS="$(CPPFLAGS)" \
+	python $(PYLIBFDT_objdir)/setup.py --quiet build_ext --inplace
 	mv _libfdt.so $(PYMODULE)
 
 $(WRAP): $(PYLIBFDT_srcdir)/libfdt.swig
diff --git a/pylibfdt/setup.py b/pylibfdt/setup.py
index 0ff160c..e45f110 100644
--- a/pylibfdt/setup.py
+++ b/pylibfdt/setup.py
@@ -2,6 +2,9 @@
 
 """
 setup.py file for SWIG libfdt
+
+Files to be built into the extension are provided in SOURCES
+C flags to use are provided in CPPFLAGS
 """
 
 from distutils.core import setup, Extension
@@ -9,22 +12,15 @@ import os
 import sys
 
 progname = sys.argv[0]
-cflags = sys.argv[1]
-files = sys.argv[2:]
-
-if cflags:
-    cflags = [flag for flag in cflags.split(' ') if flag]
-else:
-    cflags = None
+files = os.environ['SOURCES'].split()
+cflags = os.environ['CPPFLAGS'].split()
 
 libfdt_module = Extension(
     '_libfdt',
     sources = files,
-    extra_compile_args =  cflags
+    extra_compile_args = cflags
 )
 
-sys.argv = [progname, '--quiet', 'build_ext', '--inplace']
-
 setup (name = 'libfdt',
        version = '0.1',
        author      = "Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>",
-- 
2.12.1.578.ge9c3154ca4-goog

[PATCH 4/7] pylibfdt: Use package_dir to set the package directory

[Simon Glass]

At present we manually move _libfdt.so into the correct place. Provide a
package directory so we can avoid needing to do this.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

 pylibfdt/Makefile.pylibfdt | 3 +--
 pylibfdt/setup.py          | 3 +++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
index 3d99fd4..861e67c 100644
--- a/pylibfdt/Makefile.pylibfdt
+++ b/pylibfdt/Makefile.pylibfdt
@@ -7,9 +7,8 @@ PYMODULE = $(PYLIBFDT_objdir)/_libfdt.so
 
 $(PYMODULE): $(PYLIBFDT_srcs) $(WRAP)
 	@$(VECHO) PYMOD $@
-	SOURCES="$^" CPPFLAGS="$(CPPFLAGS)" \
+	SOURCES="$^" CPPFLAGS="$(CPPFLAGS)" OBJDIR="$(PYLIBFDT_objdir)" \
 	python $(PYLIBFDT_objdir)/setup.py --quiet build_ext --inplace
-	mv _libfdt.so $(PYMODULE)
 
 $(WRAP): $(PYLIBFDT_srcdir)/libfdt.swig
 	@$(VECHO) SWIG $@
diff --git a/pylibfdt/setup.py b/pylibfdt/setup.py
index e45f110..ef6e2c0 100644
--- a/pylibfdt/setup.py
+++ b/pylibfdt/setup.py
@@ -5,6 +5,7 @@ setup.py file for SWIG libfdt
 
 Files to be built into the extension are provided in SOURCES
 C flags to use are provided in CPPFLAGS
+Object file directory is provided in OBJDIR
 """
 
 from distutils.core import setup, Extension
@@ -14,6 +15,7 @@ import sys
 progname = sys.argv[0]
 files = os.environ['SOURCES'].split()
 cflags = os.environ['CPPFLAGS'].split()
+objdir = os.environ['OBJDIR']
 
 libfdt_module = Extension(
     '_libfdt',
@@ -26,5 +28,6 @@ setup (name = 'libfdt',
        author      = "Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>",
        description = """Python binding for libfdt""",
        ext_modules = [libfdt_module],
+       package_dir = {'': objdir},
        py_modules = ["libfdt"],
        )
-- 
2.12.1.578.ge9c3154ca4-goog

[PATCH 5/7] pylibfdt: Enable installation of Python module

[Simon Glass]

Adjust the setup script to support installation, and call it from the
Makefile if enabled. It will be disabled if we were unable to build the
module (e.g. due to swig being missing), or the NO_PYTHON environment
variable is set.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

 Makefile                   |  2 +-
 README                     |  7 +++++++
 pylibfdt/Makefile.pylibfdt | 14 ++++++++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 5cf4aee..52ff72c 100644
--- a/Makefile
+++ b/Makefile
@@ -195,7 +195,7 @@ install-includes:
 	$(INSTALL) -d $(DESTDIR)$(INCLUDEDIR)
 	$(INSTALL) -m 644 $(LIBFDT_include) $(DESTDIR)$(INCLUDEDIR)
 
-install: install-bin install-lib install-includes
+install: install-bin install-lib install-includes maybe_install_pylibfdt
 
 $(VERSION_FILE): Makefile FORCE
 	$(call filechk,version)
diff --git a/README b/README
index d2323fd..5add557 100644
--- a/README
+++ b/README
@@ -50,6 +50,13 @@ If you add new features, please check code coverage:
     # Open 'htmlcov/index.html' in your browser
 
 
+To install the library use:
+
+    make install_pylibfdt SETUP_PREFIX=/path/to/install_dir
+
+If SETUP_PREFIX is not provided, the default prefix is used, typically '/usr'
+or '/usr/local'. See Python's distutils documentation for details.
+
 To disable building the python library, even if swig and Python are available,
 use:
 
diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
index 861e67c..a0271da 100644
--- a/pylibfdt/Makefile.pylibfdt
+++ b/pylibfdt/Makefile.pylibfdt
@@ -14,4 +14,18 @@ $(WRAP): $(PYLIBFDT_srcdir)/libfdt.swig
 	@$(VECHO) SWIG $@
 	$(SWIG) -python -o $@ $<
 
+install_pylibfdt: $(WRAP) $(PYMODULE)
+	$(VECHO) INSTALL-PYLIB; \
+	SOURCES="$(PYLIBFDT_srcs) $(WRAP)" CPPFLAGS="$(CPPFLAGS)" \
+	OBJDIR="$(PYLIBFDT_objdir)" \
+	python $(PYLIBFDT_objdir)/setup.py --quiet install \
+		$(if $(SETUP_PREFIX),--prefix=$(SETUP_PREFIX))
+
+maybe_install_pylibfdt:
+	if [ -e $(PYMODULE) ]; then \
+		if [ -z "$(NO_PYTHON)" ]; then \
+			$(MAKE) install_pylibfdt; \
+		fi; \
+	fi
+
 PYLIBFDT_cleanfiles = libfdt_wrap.c libfdt.py libfdt.pyc _libfdt.so
-- 
2.12.1.578.ge9c3154ca4-goog

[PATCH 6/7] pylibfdt: Use the correct libfdt version in the module

[Simon Glass]

Use the same version number in the module as with the rest of libfdt. This
can be examined with:

   import pkg_resources
   print pkg_resources.require('libfdt')[0].version

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

 pylibfdt/Makefile.pylibfdt | 3 ++-
 pylibfdt/setup.py          | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
index a0271da..a74cd30 100644
--- a/pylibfdt/Makefile.pylibfdt
+++ b/pylibfdt/Makefile.pylibfdt
@@ -8,6 +8,7 @@ PYMODULE = $(PYLIBFDT_objdir)/_libfdt.so
 $(PYMODULE): $(PYLIBFDT_srcs) $(WRAP)
 	@$(VECHO) PYMOD $@
 	SOURCES="$^" CPPFLAGS="$(CPPFLAGS)" OBJDIR="$(PYLIBFDT_objdir)" \
+	VERSION="$(dtc_version)" \
 	python $(PYLIBFDT_objdir)/setup.py --quiet build_ext --inplace
 
 $(WRAP): $(PYLIBFDT_srcdir)/libfdt.swig
@@ -17,7 +18,7 @@ $(WRAP): $(PYLIBFDT_srcdir)/libfdt.swig
 install_pylibfdt: $(WRAP) $(PYMODULE)
 	$(VECHO) INSTALL-PYLIB; \
 	SOURCES="$(PYLIBFDT_srcs) $(WRAP)" CPPFLAGS="$(CPPFLAGS)" \
-	OBJDIR="$(PYLIBFDT_objdir)" \
+	OBJDIR="$(PYLIBFDT_objdir)" VERSION="$(dtc_version)" \
 	python $(PYLIBFDT_objdir)/setup.py --quiet install \
 		$(if $(SETUP_PREFIX),--prefix=$(SETUP_PREFIX))
 
diff --git a/pylibfdt/setup.py b/pylibfdt/setup.py
index ef6e2c0..3bafe30 100644
--- a/pylibfdt/setup.py
+++ b/pylibfdt/setup.py
@@ -16,6 +16,7 @@ progname = sys.argv[0]
 files = os.environ['SOURCES'].split()
 cflags = os.environ['CPPFLAGS'].split()
 objdir = os.environ['OBJDIR']
+version = os.environ['VERSION']
 
 libfdt_module = Extension(
     '_libfdt',
@@ -24,7 +25,7 @@ libfdt_module = Extension(
 )
 
 setup (name = 'libfdt',
-       version = '0.1',
+       version = version,
        author      = "Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>",
        description = """Python binding for libfdt""",
        ext_modules = [libfdt_module],
-- 
2.12.1.578.ge9c3154ca4-goog

[PATCH 7/7] pylibfdt: Use the call function to simplify the Makefile

[Simon Glass]

This is in a separate patch since I not sure if GNU make features
are permitted in the Makefile.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

 pylibfdt/Makefile.pylibfdt | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
index a74cd30..0d95c11 100644
--- a/pylibfdt/Makefile.pylibfdt
+++ b/pylibfdt/Makefile.pylibfdt
@@ -5,11 +5,13 @@ PYLIBFDT_srcs = $(addprefix $(LIBFDT_srcdir)/,$(LIBFDT_SRCS))
 WRAP = $(PYLIBFDT_objdir)/libfdt_wrap.c
 PYMODULE = $(PYLIBFDT_objdir)/_libfdt.so
 
+run_setup = SOURCES="$(1)" CPPFLAGS="$(CPPFLAGS)" OBJDIR="$(PYLIBFDT_objdir)" \
+	VERSION="$(dtc_version)" \
+	python $(PYLIBFDT_objdir)/setup.py --quiet $(2)
+
 $(PYMODULE): $(PYLIBFDT_srcs) $(WRAP)
 	@$(VECHO) PYMOD $@
-	SOURCES="$^" CPPFLAGS="$(CPPFLAGS)" OBJDIR="$(PYLIBFDT_objdir)" \
-	VERSION="$(dtc_version)" \
-	python $(PYLIBFDT_objdir)/setup.py --quiet build_ext --inplace
+	$(call run_setup, $^, build_ext --inplace)
 
 $(WRAP): $(PYLIBFDT_srcdir)/libfdt.swig
 	@$(VECHO) SWIG $@
@@ -17,10 +19,8 @@ $(WRAP): $(PYLIBFDT_srcdir)/libfdt.swig
 
 install_pylibfdt: $(WRAP) $(PYMODULE)
 	$(VECHO) INSTALL-PYLIB; \
-	SOURCES="$(PYLIBFDT_srcs) $(WRAP)" CPPFLAGS="$(CPPFLAGS)" \
-	OBJDIR="$(PYLIBFDT_objdir)" VERSION="$(dtc_version)" \
-	python $(PYLIBFDT_objdir)/setup.py --quiet install \
-		$(if $(SETUP_PREFIX),--prefix=$(SETUP_PREFIX))
+	$(call run_setup, $(PYLIBFDT_srcs) $(WRAP), \
+		install $(if $(SETUP_PREFIX),--prefix=$(SETUP_PREFIX)))
 
 maybe_install_pylibfdt:
 	if [ -e $(PYMODULE) ]; then \
-- 
2.12.1.578.ge9c3154ca4-goog

Re: [PATCH 1/7] pylibfdt: Allow pkg-config to be supplied in the environment

[David Gibson]

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

On Sun, Mar 26, 2017 at 01:06:17PM -0600, Simon Glass wrote:
> Some build systems have their own version of the pkg-config tool.
> Use a variable for this instead of hard-coding it, to allow for this.
> 
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Suggested-by: Mike Frysinger <vapier-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

This is good Makefile practice, regardless of anything else, so I've
merged this.

> ---
> 
>  Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 1d08ec1..e6d8251 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -23,6 +23,7 @@ CFLAGS = -g -Os -fPIC -Werror $(WARNINGS)
>  BISON = bison
>  LEX = flex
>  SWIG = swig
> +PKG_CONFIG ?= pkg-config
>  
>  INSTALL = /usr/bin/install
>  DESTDIR =
> @@ -119,7 +120,7 @@ SCRIPTS = dtdiff
>  # We need both Python and swig to build pylibfdt.
>  .PHONY: maybe_pylibfdt
>  maybe_pylibfdt: FORCE
> -	if pkg-config --cflags python >/dev/null 2>&1; then \
> +	if $(PKG_CONFIG) --cflags python >/dev/null 2>&1; then \
>  		if which swig >/dev/null 2>&1; then \
>  			can_build=yes; \
>  		fi; \

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/7] pylibfdt: Allow building to be disabled

[Mike Frysinger]

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

On 26 Mar 2017 13:06, Simon Glass wrote:
> --- a/Makefile
> +++ b/Makefile
> @@ -120,6 +120,7 @@ SCRIPTS = dtdiff
>  # We need both Python and swig to build pylibfdt.
>  .PHONY: maybe_pylibfdt
>  maybe_pylibfdt: FORCE
> +	if [ -n "${NO_PYTHON}" ]; then exit; fi; \

normally this is handled at the Makefile level, not inline in the script.
also, negative variables are messy.

what about:
PYTHON ?= y
ifeq ($(PYTHON),y)
all: maybe_pylibfdt
endif
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/7] pylibfdt: Use environment to pass C flags and files

[Mike Frysinger]

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

On 26 Mar 2017 13:06, Simon Glass wrote:
> --- a/pylibfdt/Makefile.pylibfdt
> +++ b/pylibfdt/Makefile.pylibfdt
> @@ -7,7 +7,8 @@ PYMODULE = $(PYLIBFDT_objdir)/_libfdt.so
>  
>  $(PYMODULE): $(PYLIBFDT_srcs) $(WRAP)
>  	@$(VECHO) PYMOD $@
> -	python $(PYLIBFDT_objdir)/setup.py "$(CPPFLAGS)" $^
> +	SOURCES="$^" CPPFLAGS="$(CPPFLAGS)" \
> +	python $(PYLIBFDT_objdir)/setup.py --quiet build_ext --inplace

you should make setup.py executable and drop the `python` call here

> +files = os.environ['SOURCES'].split()

you could parse this from the Makefile

> +cflags = os.environ['CPPFLAGS'].split()

this should be:
cflags = os.environ.get('CPPFLAGS', '').split()

>  libfdt_module = Extension(
>      '_libfdt',
>      sources = files,

python style says to not put spaces around = when it comes to args
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/7] pylibfdt: Enable installation of Python module

[Mike Frysinger]

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

On 26 Mar 2017 13:06, Simon Glass wrote:
> --- a/README
> +++ b/README
> @@ -50,6 +50,13 @@ If you add new features, please check code coverage:
>      # Open 'htmlcov/index.html' in your browser
>  
>  
> +To install the library use:
> +
> +    make install_pylibfdt SETUP_PREFIX=/path/to/install_dir

seems like you should just tell people to run `./setup.py` like normal ?
then you don't have to invent all this project-specific wrappers.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 7/7] pylibfdt: Use the call function to simplify the Makefile

[Mike Frysinger]

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

On 26 Mar 2017 13:06, Simon Glass wrote:
> +run_setup = SOURCES="$(1)" CPPFLAGS="$(CPPFLAGS)" OBJDIR="$(PYLIBFDT_objdir)" \
> +	VERSION="$(dtc_version)" \
> +	python $(PYLIBFDT_objdir)/setup.py --quiet $(2)

would be cleaner to use `define` here imo.
define run_setup
 ....
endef
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/7] pylibfdt: Enable installation of Python module

[Simon Glass]

Hi Mike,

On 3 April 2017 at 12:43, Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org> wrote:
> On 26 Mar 2017 13:06, Simon Glass wrote:
>> --- a/README
>> +++ b/README
>> @@ -50,6 +50,13 @@ If you add new features, please check code coverage:
>>      # Open 'htmlcov/index.html' in your browser
>>
>>
>> +To install the library use:
>> +
>> +    make install_pylibfdt SETUP_PREFIX=/path/to/install_dir
>
> seems like you should just tell people to run `./setup.py` like normal ?
> then you don't have to invent all this project-specific wrappers.
> -mike

OK I will make that the default approach. The SETUP_PREFIX is optional.

Regards,
Simon

Re: [PATCH 2/7] pylibfdt: Allow building to be disabled

[Simon Glass]

Hi Mike,

On 3 April 2017 at 12:41, Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org> wrote:
> On 26 Mar 2017 13:06, Simon Glass wrote:
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -120,6 +120,7 @@ SCRIPTS = dtdiff
>>  # We need both Python and swig to build pylibfdt.
>>  .PHONY: maybe_pylibfdt
>>  maybe_pylibfdt: FORCE
>> +     if [ -n "${NO_PYTHON}" ]; then exit; fi; \
>
> normally this is handled at the Makefile level, not inline in the script.

Yes it's pretty funny that I didn't just do that.

> also, negative variables are messy.

But I want to build Python by default!

>
> what about:
> PYTHON ?= y
> ifeq ($(PYTHON),y)
> all: maybe_pylibfdt
> endif
> -mike

Regards,
Simon

Re: [PATCH 3/7] pylibfdt: Use environment to pass C flags and files

[Simon Glass]

Hi Mike,

On 3 April 2017 at 12:42, Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org> wrote:
> On 26 Mar 2017 13:06, Simon Glass wrote:
>> --- a/pylibfdt/Makefile.pylibfdt
>> +++ b/pylibfdt/Makefile.pylibfdt
>> @@ -7,7 +7,8 @@ PYMODULE = $(PYLIBFDT_objdir)/_libfdt.so
>>
>>  $(PYMODULE): $(PYLIBFDT_srcs) $(WRAP)
>>       @$(VECHO) PYMOD $@
>> -     python $(PYLIBFDT_objdir)/setup.py "$(CPPFLAGS)" $^
>> +     SOURCES="$^" CPPFLAGS="$(CPPFLAGS)" \
>> +     python $(PYLIBFDT_objdir)/setup.py --quiet build_ext --inplace
>
> you should make setup.py executable and drop the `python` call here
>
>> +files = os.environ['SOURCES'].split()
>
> you could parse this from the Makefile
>
>> +cflags = os.environ['CPPFLAGS'].split()
>
> this should be:
> cflags = os.environ.get('CPPFLAGS', '').split()
>

I did these in a separate patch as Makefile parsing is a separate feature IMO.

>>  libfdt_module = Extension(
>>      '_libfdt',
>>      sources = files,
>
> python style says to not put spaces around = when it comes to args

Yes I was following a crappy example - will change it.

> -mike

Regards,
Simon