diff mbox series

[ovs-dev,v2] python: Update build system to ensure dirs.py is created

Message ID 20201117180212.433352-1-mark.d.gray@redhat.com
State Changes Requested
Headers show
Series [ovs-dev,v2] python: Update build system to ensure dirs.py is created | expand

Commit Message

Mark Gray Nov. 17, 2020, 6:02 p.m. UTC
Update build system to ensure dirs.py is created when it is a
dependency for a build target. Also, update setup.py to
check for that dependency.

Signed-off-by: Mark Gray <mark.d.gray@redhat.com>
---

v2: Added v2 tag

 lib/automake.mk       |  2 +-
 python/automake.mk    | 24 ++++++++++++------------
 python/ovs/.gitignore |  2 +-
 python/setup.py       |  9 +++++++++
 tests/automake.mk     |  2 +-
 vtep/automake.mk      |  2 +-
 6 files changed, 25 insertions(+), 16 deletions(-)

Comments

Ilya Maximets Nov. 18, 2020, 3:11 p.m. UTC | #1
On 11/17/20 7:02 PM, Mark Gray wrote:
> Update build system to ensure dirs.py is created when it is a
> dependency for a build target. Also, update setup.py to
> check for that dependency.
> 
> Signed-off-by: Mark Gray <mark.d.gray@redhat.com>
> ---
> 
> v2: Added v2 tag
> 
>  lib/automake.mk       |  2 +-
>  python/automake.mk    | 24 ++++++++++++------------
>  python/ovs/.gitignore |  2 +-
>  python/setup.py       |  9 +++++++++
>  tests/automake.mk     |  2 +-
>  vtep/automake.mk      |  2 +-
>  6 files changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 8eeb6c3f676c..fad6ffb40cef 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -629,7 +629,7 @@ EXTRA_DIST += build-aux/extract-ofp-msgs
>  # _server IDL
>  OVSIDL_BUILT += lib/ovsdb-server-idl.c lib/ovsdb-server-idl.h lib/ovsdb-server-idl.ovsidl
>  EXTRA_DIST += lib/ovsdb-server-idl.ann
> -lib/ovsdb-server-idl.ovsidl: ovsdb/_server.ovsschema lib/ovsdb-server-idl.ann
> +lib/ovsdb-server-idl.ovsidl: ovsdb/_server.ovsschema lib/ovsdb-server-idl.ann python/ovs/dirs.py

What about dependency for lib/vswitch-idl.ovsidl and tests/idltest.ovsidl?
I think it's better to add dependency directly for ovsdb/ovsdb-idlc.in or
at least to $(OVSIDL_BUILT) target and not fix every single case.

>  	$(AM_V_GEN)$(OVSDB_IDLC) annotate $(srcdir)/ovsdb/_server.ovsschema $(srcdir)/lib/ovsdb-server-idl.ann > $@.tmp && mv $@.tmp $@
>  
>  INSTALL_DATA_LOCAL += lib-install-data-local
> diff --git a/python/automake.mk b/python/automake.mk
> index c4382ec60928..767512f1757f 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -74,12 +74,12 @@ ovs-install-data-local:
>  	$(MKDIR_P) python/ovs
>  	sed \
>  		-e '/^##/d' \
> -                -e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
> -                -e 's,[@]RUNDIR[@],$(RUNDIR),g' \
> -                -e 's,[@]LOGDIR[@],$(LOGDIR),g' \
> -                -e 's,[@]bindir[@],$(bindir),g' \
> -                -e 's,[@]sysconfdir[@],$(sysconfdir),g' \
> -                -e 's,[@]DBDIR[@],$(DBDIR),g' \
> +		-e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
> +		-e 's,[@]RUNDIR[@],$(RUNDIR),g' \
> +		-e 's,[@]LOGDIR[@],$(LOGDIR),g' \
> +		-e 's,[@]bindir[@],$(bindir),g' \
> +		-e 's,[@]sysconfdir[@],$(sysconfdir),g' \
> +		-e 's,[@]DBDIR[@],$(DBDIR),g' \
>  		< $(srcdir)/python/ovs/dirs.py.template \
>  		> python/ovs/dirs.py.tmp
>  	$(MKDIR_P) $(DESTDIR)$(pkgdatadir)/python/ovs
> @@ -107,12 +107,12 @@ ALL_LOCAL += $(srcdir)/python/ovs/dirs.py
>  $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
>  	$(AM_V_GEN)sed \
>  		-e '/^##/d' \
> -                -e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
> -                -e 's,[@]RUNDIR[@],$(RUNDIR),g' \
> -                -e 's,[@]LOGDIR[@],$(LOGDIR),g' \
> -                -e 's,[@]bindir[@],$(bindir),g' \
> -                -e 's,[@]sysconfdir[@],$(sysconfdir),g' \
> -                -e 's,[@]DBDIR[@],$(sysconfdir)/openvswitch,g' \
> +		-e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
> +		-e 's,[@]RUNDIR[@],$(RUNDIR),g' \
> +		-e 's,[@]LOGDIR[@],$(LOGDIR),g' \
> +		-e 's,[@]bindir[@],$(bindir),g' \
> +		-e 's,[@]sysconfdir[@],$(sysconfdir),g' \
> +		-e 's,[@]DBDIR[@],$(sysconfdir)/openvswitch,g' \
>  		< $? > $@.tmp && \
>  	mv $@.tmp $@
>  EXTRA_DIST += python/ovs/dirs.py.template
> diff --git a/python/ovs/.gitignore b/python/ovs/.gitignore
> index 51030beca437..8bbcd824f472 100644
> --- a/python/ovs/.gitignore
> +++ b/python/ovs/.gitignore
> @@ -1,2 +1,2 @@
>  version.py
> -dir.py
> +dirs.py
> diff --git a/python/setup.py b/python/setup.py
> index b7252800c1c1..d385d8372239 100644
> --- a/python/setup.py
> +++ b/python/setup.py
> @@ -30,6 +30,15 @@ except IOError:
>            file=sys.stderr)
>      sys.exit(-1)
>  
> +try:
> +    # Try to open generated ovs/dirs.py. However, in this case we
> +    # don't need to exec()
> +    open("ovs/dirs.py")
> +except IOError:
> +    print("Ensure dirs.py is created by running make python/ovs/dirs.py",
> +          file=sys.stderr)
> +    sys.exit(-1)
> +
>  ext_errors = (CCompilerError, DistutilsExecError, DistutilsPlatformError)
>  if sys.platform == 'win32':
>      ext_errors += (IOError, ValueError)
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 677b99a6b487..3faa7395a157 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -431,7 +431,7 @@ tests_test_lib_LDADD = lib/libopenvswitch.la
>  OVSIDL_BUILT += tests/idltest.c tests/idltest.h tests/idltest.ovsidl
>  IDLTEST_IDL_FILES = tests/idltest.ovsschema tests/idltest.ann
>  EXTRA_DIST += $(IDLTEST_IDL_FILES) tests/idltest2.ovsschema
> -tests/idltest.ovsidl: $(IDLTEST_IDL_FILES)
> +tests/idltest.ovsidl: $(IDLTEST_IDL_FILES) python/ovs/dirs.py
>  	$(AM_V_GEN)$(OVSDB_IDLC) -C $(srcdir) annotate $(IDLTEST_IDL_FILES) > $@.tmp && \
>  	mv $@.tmp $@
>  
> diff --git a/vtep/automake.mk b/vtep/automake.mk
> index e549922d915f..8632fb5b5b07 100644
> --- a/vtep/automake.mk
> +++ b/vtep/automake.mk
> @@ -7,7 +7,7 @@ EXTRA_DIST += vtep/vtep-idl.ann
>  VTEP_IDL_FILES = \
>  	$(srcdir)/vtep/vtep.ovsschema \
>  	$(srcdir)/vtep/vtep-idl.ann
> -vtep/vtep-idl.ovsidl: $(VTEP_IDL_FILES)
> +vtep/vtep-idl.ovsidl: $(VTEP_IDL_FILES) python/ovs/dirs.py
>  	$(AM_V_GEN)$(OVSDB_IDLC) annotate $(VTEP_IDL_FILES) > $@.tmp && \
>  	mv $@.tmp $@
>  
>
Mark Gray Nov. 19, 2020, 8:46 a.m. UTC | #2
On 18/11/2020 15:11, Ilya Maximets wrote:
> On 11/17/20 7:02 PM, Mark Gray wrote:
>> Update build system to ensure dirs.py is created when it is a
>> dependency for a build target. Also, update setup.py to
>> check for that dependency.
>>
>> Signed-off-by: Mark Gray <mark.d.gray@redhat.com>
>> ---
>>
>> v2: Added v2 tag
>>
>>  lib/automake.mk       |  2 +-
>>  python/automake.mk    | 24 ++++++++++++------------
>>  python/ovs/.gitignore |  2 +-
>>  python/setup.py       |  9 +++++++++
>>  tests/automake.mk     |  2 +-
>>  vtep/automake.mk      |  2 +-
>>  6 files changed, 25 insertions(+), 16 deletions(-)
>>
>> diff --git a/lib/automake.mk b/lib/automake.mk
>> index 8eeb6c3f676c..fad6ffb40cef 100644
>> --- a/lib/automake.mk
>> +++ b/lib/automake.mk
>> @@ -629,7 +629,7 @@ EXTRA_DIST += build-aux/extract-ofp-msgs
>>  # _server IDL
>>  OVSIDL_BUILT += lib/ovsdb-server-idl.c lib/ovsdb-server-idl.h lib/ovsdb-server-idl.ovsidl
>>  EXTRA_DIST += lib/ovsdb-server-idl.ann
>> -lib/ovsdb-server-idl.ovsidl: ovsdb/_server.ovsschema lib/ovsdb-server-idl.ann
>> +lib/ovsdb-server-idl.ovsidl: ovsdb/_server.ovsschema lib/ovsdb-server-idl.ann python/ovs/dirs.py

Thanks for the quick review Ilya
> 
> What about dependency for lib/vswitch-idl.ovsidl and tests/idltest.ovsidl?

lib/vswitch-idl.ovsidl this was actually covered in the previous patch.
The tests one is below.

> I think it's better to add dependency directly for ovsdb/ovsdb-idlc.in or
> at least to $(OVSIDL_BUILT) target and not fix every single case.

Absolutely. Much cleaner - dont know why I didn't do that from the
start. See follow-up:

https://patchwork.ozlabs.org/project/openvswitch/patch/20201119084434.461714-1-mark.d.gray@redhat.com/

> 
>>  	$(AM_V_GEN)$(OVSDB_IDLC) annotate $(srcdir)/ovsdb/_server.ovsschema $(srcdir)/lib/ovsdb-server-idl.ann > $@.tmp && mv $@.tmp $@
>>  
>>  INSTALL_DATA_LOCAL += lib-install-data-local
>> diff --git a/python/automake.mk b/python/automake.mk
>> index c4382ec60928..767512f1757f 100644
>> --- a/python/automake.mk
>> +++ b/python/automake.mk
>> @@ -74,12 +74,12 @@ ovs-install-data-local:
>>  	$(MKDIR_P) python/ovs
>>  	sed \
>>  		-e '/^##/d' \
>> -                -e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
>> -                -e 's,[@]RUNDIR[@],$(RUNDIR),g' \
>> -                -e 's,[@]LOGDIR[@],$(LOGDIR),g' \
>> -                -e 's,[@]bindir[@],$(bindir),g' \
>> -                -e 's,[@]sysconfdir[@],$(sysconfdir),g' \
>> -                -e 's,[@]DBDIR[@],$(DBDIR),g' \
>> +		-e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
>> +		-e 's,[@]RUNDIR[@],$(RUNDIR),g' \
>> +		-e 's,[@]LOGDIR[@],$(LOGDIR),g' \
>> +		-e 's,[@]bindir[@],$(bindir),g' \
>> +		-e 's,[@]sysconfdir[@],$(sysconfdir),g' \
>> +		-e 's,[@]DBDIR[@],$(DBDIR),g' \
>>  		< $(srcdir)/python/ovs/dirs.py.template \
>>  		> python/ovs/dirs.py.tmp
>>  	$(MKDIR_P) $(DESTDIR)$(pkgdatadir)/python/ovs
>> @@ -107,12 +107,12 @@ ALL_LOCAL += $(srcdir)/python/ovs/dirs.py
>>  $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
>>  	$(AM_V_GEN)sed \
>>  		-e '/^##/d' \
>> -                -e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
>> -                -e 's,[@]RUNDIR[@],$(RUNDIR),g' \
>> -                -e 's,[@]LOGDIR[@],$(LOGDIR),g' \
>> -                -e 's,[@]bindir[@],$(bindir),g' \
>> -                -e 's,[@]sysconfdir[@],$(sysconfdir),g' \
>> -                -e 's,[@]DBDIR[@],$(sysconfdir)/openvswitch,g' \
>> +		-e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
>> +		-e 's,[@]RUNDIR[@],$(RUNDIR),g' \
>> +		-e 's,[@]LOGDIR[@],$(LOGDIR),g' \
>> +		-e 's,[@]bindir[@],$(bindir),g' \
>> +		-e 's,[@]sysconfdir[@],$(sysconfdir),g' \
>> +		-e 's,[@]DBDIR[@],$(sysconfdir)/openvswitch,g' \
>>  		< $? > $@.tmp && \
>>  	mv $@.tmp $@
>>  EXTRA_DIST += python/ovs/dirs.py.template
>> diff --git a/python/ovs/.gitignore b/python/ovs/.gitignore
>> index 51030beca437..8bbcd824f472 100644
>> --- a/python/ovs/.gitignore
>> +++ b/python/ovs/.gitignore
>> @@ -1,2 +1,2 @@
>>  version.py
>> -dir.py
>> +dirs.py
>> diff --git a/python/setup.py b/python/setup.py
>> index b7252800c1c1..d385d8372239 100644
>> --- a/python/setup.py
>> +++ b/python/setup.py
>> @@ -30,6 +30,15 @@ except IOError:
>>            file=sys.stderr)
>>      sys.exit(-1)
>>  
>> +try:
>> +    # Try to open generated ovs/dirs.py. However, in this case we
>> +    # don't need to exec()
>> +    open("ovs/dirs.py")
>> +except IOError:
>> +    print("Ensure dirs.py is created by running make python/ovs/dirs.py",
>> +          file=sys.stderr)
>> +    sys.exit(-1)
>> +
>>  ext_errors = (CCompilerError, DistutilsExecError, DistutilsPlatformError)
>>  if sys.platform == 'win32':
>>      ext_errors += (IOError, ValueError)
>> diff --git a/tests/automake.mk b/tests/automake.mk
>> index 677b99a6b487..3faa7395a157 100644
>> --- a/tests/automake.mk
>> +++ b/tests/automake.mk
>> @@ -431,7 +431,7 @@ tests_test_lib_LDADD = lib/libopenvswitch.la
>>  OVSIDL_BUILT += tests/idltest.c tests/idltest.h tests/idltest.ovsidl
>>  IDLTEST_IDL_FILES = tests/idltest.ovsschema tests/idltest.ann
>>  EXTRA_DIST += $(IDLTEST_IDL_FILES) tests/idltest2.ovsschema
>> -tests/idltest.ovsidl: $(IDLTEST_IDL_FILES)
>> +tests/idltest.ovsidl: $(IDLTEST_IDL_FILES) python/ovs/dirs.py
>>  	$(AM_V_GEN)$(OVSDB_IDLC) -C $(srcdir) annotate $(IDLTEST_IDL_FILES) > $@.tmp && \
>>  	mv $@.tmp $@
>>  
>> diff --git a/vtep/automake.mk b/vtep/automake.mk
>> index e549922d915f..8632fb5b5b07 100644
>> --- a/vtep/automake.mk
>> +++ b/vtep/automake.mk
>> @@ -7,7 +7,7 @@ EXTRA_DIST += vtep/vtep-idl.ann
>>  VTEP_IDL_FILES = \
>>  	$(srcdir)/vtep/vtep.ovsschema \
>>  	$(srcdir)/vtep/vtep-idl.ann
>> -vtep/vtep-idl.ovsidl: $(VTEP_IDL_FILES)
>> +vtep/vtep-idl.ovsidl: $(VTEP_IDL_FILES) python/ovs/dirs.py
>>  	$(AM_V_GEN)$(OVSDB_IDLC) annotate $(VTEP_IDL_FILES) > $@.tmp && \
>>  	mv $@.tmp $@
>>  
>>
>
diff mbox series

Patch

diff --git a/lib/automake.mk b/lib/automake.mk
index 8eeb6c3f676c..fad6ffb40cef 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -629,7 +629,7 @@  EXTRA_DIST += build-aux/extract-ofp-msgs
 # _server IDL
 OVSIDL_BUILT += lib/ovsdb-server-idl.c lib/ovsdb-server-idl.h lib/ovsdb-server-idl.ovsidl
 EXTRA_DIST += lib/ovsdb-server-idl.ann
-lib/ovsdb-server-idl.ovsidl: ovsdb/_server.ovsschema lib/ovsdb-server-idl.ann
+lib/ovsdb-server-idl.ovsidl: ovsdb/_server.ovsschema lib/ovsdb-server-idl.ann python/ovs/dirs.py
 	$(AM_V_GEN)$(OVSDB_IDLC) annotate $(srcdir)/ovsdb/_server.ovsschema $(srcdir)/lib/ovsdb-server-idl.ann > $@.tmp && mv $@.tmp $@
 
 INSTALL_DATA_LOCAL += lib-install-data-local
diff --git a/python/automake.mk b/python/automake.mk
index c4382ec60928..767512f1757f 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -74,12 +74,12 @@  ovs-install-data-local:
 	$(MKDIR_P) python/ovs
 	sed \
 		-e '/^##/d' \
-                -e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
-                -e 's,[@]RUNDIR[@],$(RUNDIR),g' \
-                -e 's,[@]LOGDIR[@],$(LOGDIR),g' \
-                -e 's,[@]bindir[@],$(bindir),g' \
-                -e 's,[@]sysconfdir[@],$(sysconfdir),g' \
-                -e 's,[@]DBDIR[@],$(DBDIR),g' \
+		-e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
+		-e 's,[@]RUNDIR[@],$(RUNDIR),g' \
+		-e 's,[@]LOGDIR[@],$(LOGDIR),g' \
+		-e 's,[@]bindir[@],$(bindir),g' \
+		-e 's,[@]sysconfdir[@],$(sysconfdir),g' \
+		-e 's,[@]DBDIR[@],$(DBDIR),g' \
 		< $(srcdir)/python/ovs/dirs.py.template \
 		> python/ovs/dirs.py.tmp
 	$(MKDIR_P) $(DESTDIR)$(pkgdatadir)/python/ovs
@@ -107,12 +107,12 @@  ALL_LOCAL += $(srcdir)/python/ovs/dirs.py
 $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
 	$(AM_V_GEN)sed \
 		-e '/^##/d' \
-                -e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
-                -e 's,[@]RUNDIR[@],$(RUNDIR),g' \
-                -e 's,[@]LOGDIR[@],$(LOGDIR),g' \
-                -e 's,[@]bindir[@],$(bindir),g' \
-                -e 's,[@]sysconfdir[@],$(sysconfdir),g' \
-                -e 's,[@]DBDIR[@],$(sysconfdir)/openvswitch,g' \
+		-e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
+		-e 's,[@]RUNDIR[@],$(RUNDIR),g' \
+		-e 's,[@]LOGDIR[@],$(LOGDIR),g' \
+		-e 's,[@]bindir[@],$(bindir),g' \
+		-e 's,[@]sysconfdir[@],$(sysconfdir),g' \
+		-e 's,[@]DBDIR[@],$(sysconfdir)/openvswitch,g' \
 		< $? > $@.tmp && \
 	mv $@.tmp $@
 EXTRA_DIST += python/ovs/dirs.py.template
diff --git a/python/ovs/.gitignore b/python/ovs/.gitignore
index 51030beca437..8bbcd824f472 100644
--- a/python/ovs/.gitignore
+++ b/python/ovs/.gitignore
@@ -1,2 +1,2 @@ 
 version.py
-dir.py
+dirs.py
diff --git a/python/setup.py b/python/setup.py
index b7252800c1c1..d385d8372239 100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -30,6 +30,15 @@  except IOError:
           file=sys.stderr)
     sys.exit(-1)
 
+try:
+    # Try to open generated ovs/dirs.py. However, in this case we
+    # don't need to exec()
+    open("ovs/dirs.py")
+except IOError:
+    print("Ensure dirs.py is created by running make python/ovs/dirs.py",
+          file=sys.stderr)
+    sys.exit(-1)
+
 ext_errors = (CCompilerError, DistutilsExecError, DistutilsPlatformError)
 if sys.platform == 'win32':
     ext_errors += (IOError, ValueError)
diff --git a/tests/automake.mk b/tests/automake.mk
index 677b99a6b487..3faa7395a157 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -431,7 +431,7 @@  tests_test_lib_LDADD = lib/libopenvswitch.la
 OVSIDL_BUILT += tests/idltest.c tests/idltest.h tests/idltest.ovsidl
 IDLTEST_IDL_FILES = tests/idltest.ovsschema tests/idltest.ann
 EXTRA_DIST += $(IDLTEST_IDL_FILES) tests/idltest2.ovsschema
-tests/idltest.ovsidl: $(IDLTEST_IDL_FILES)
+tests/idltest.ovsidl: $(IDLTEST_IDL_FILES) python/ovs/dirs.py
 	$(AM_V_GEN)$(OVSDB_IDLC) -C $(srcdir) annotate $(IDLTEST_IDL_FILES) > $@.tmp && \
 	mv $@.tmp $@
 
diff --git a/vtep/automake.mk b/vtep/automake.mk
index e549922d915f..8632fb5b5b07 100644
--- a/vtep/automake.mk
+++ b/vtep/automake.mk
@@ -7,7 +7,7 @@  EXTRA_DIST += vtep/vtep-idl.ann
 VTEP_IDL_FILES = \
 	$(srcdir)/vtep/vtep.ovsschema \
 	$(srcdir)/vtep/vtep-idl.ann
-vtep/vtep-idl.ovsidl: $(VTEP_IDL_FILES)
+vtep/vtep-idl.ovsidl: $(VTEP_IDL_FILES) python/ovs/dirs.py
 	$(AM_V_GEN)$(OVSDB_IDLC) annotate $(VTEP_IDL_FILES) > $@.tmp && \
 	mv $@.tmp $@