From f2fb1c0433f0ac26c502cea4726b3d6d9e02aa46 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Tue, 22 Nov 2022 15:28:42 +0800 Subject: [PATCH 1/8] Use SWIGPYTHON to replace SWIG for python wrapper. Add unit test for SonicDBConfig init. --- .azure-pipelines/build-template.yml | 4 ++++ common/configdb.h | 4 ++-- common/countertable.h | 2 +- common/dbconnector.h | 10 +++++----- common/producerstatetable.h | 2 +- common/producertable.h | 2 +- common/sonicv2connector.h | 2 +- common/table.h | 4 ++-- goext/Makefile | 23 +++++++++++++++++++++++ goext/swsscommon.i | 13 +++++++++++++ goext/swsscommon_test.go | 13 +++++++++++++ 11 files changed, 66 insertions(+), 13 deletions(-) create mode 100644 goext/Makefile create mode 100644 goext/swsscommon.i create mode 100644 goext/swsscommon_test.go diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index 794e67977..2a0f290eb 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -131,6 +131,7 @@ jobs: sudo mkdir /usr/local/yang-models sudo dpkg -i libswsscommon_*.deb + sudo dpkg -i libswsscommon-dev_*.deb sudo dpkg -i python-swsscommon_*.deb ./tests/tests @@ -138,6 +139,9 @@ jobs: pytest --cov=. --cov-report=xml mv coverage.xml tests/coverage.xml gcovr -r ./ -e ".*/swsscommon_wrap.cpp" --exclude-unreachable-branches --exclude-throw-branches -x --xml-pretty -o coverage.xml + redis-cli FLUSHALL + make -C goext + make -C goext check rm -rf $(Build.ArtifactStagingDirectory)/download displayName: "Run swss common unit tests" diff --git a/common/configdb.h b/common/configdb.h index 38084f019..e6513c2ed 100644 --- a/common/configdb.h +++ b/common/configdb.h @@ -37,7 +37,7 @@ class ConfigDBConnector_Native : public SonicV2Connector_Native std::string m_db_name; }; -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) %pythoncode %{ ## Note: diamond inheritance, reusing functions in both classes class ConfigDBConnector(SonicV2Connector, ConfigDBConnector_Native): @@ -307,7 +307,7 @@ class ConfigDBPipeConnector_Native: public ConfigDBConnector_Native int _get_config(DBConnector& client, RedisTransactioner& pipe, std::map>>& data, int cursor); }; -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) %pythoncode %{ class ConfigDBPipeConnector(ConfigDBConnector, ConfigDBPipeConnector_Native): diff --git a/common/countertable.h b/common/countertable.h index 4b906732d..44d58a0e2 100644 --- a/common/countertable.h +++ b/common/countertable.h @@ -68,7 +68,7 @@ class KeyCache { return m_keyMap.at(name); } -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) %pythoncode %{ def __getitem__(self, name): return self.at(name) diff --git a/common/dbconnector.h b/common/dbconnector.h index 8a70a2cfb..42c50531f 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -41,7 +41,7 @@ class SonicDBConfig static constexpr const char *DEFAULT_SONIC_DB_CONFIG_FILE = "/var/run/redis/sonic-db/database_config.json"; static constexpr const char *DEFAULT_SONIC_DB_GLOBAL_CONFIG_FILE = "/var/run/redis/sonic-db/database_global.json"; static void initialize(const std::string &file = DEFAULT_SONIC_DB_CONFIG_FILE); -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) %pythoncode %{ ## TODO: the python function and C++ one is not on-par @staticmethod @@ -51,7 +51,7 @@ class SonicDBConfig #endif static void initializeGlobalConfig(const std::string &file = DEFAULT_SONIC_DB_GLOBAL_CONFIG_FILE); -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) %pythoncode %{ ## TODO: the python function and C++ one is not on-par @staticmethod @@ -70,7 +70,7 @@ class SonicDBConfig static std::string getDbHostname(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE); static int getDbPort(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE); static std::vector getNamespaces(); -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) %pythoncode %{ ## TODO: the python function and C++ one is not on-par @staticmethod @@ -163,7 +163,7 @@ class DBConnector : public RedisContext std::string getDbName() const; std::string getNamespace() const; -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) %pythoncode %{ namespace = property(getNamespace) %} @@ -181,7 +181,7 @@ class DBConnector : public RedisContext int64_t del(const std::string &key); -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) // SWIG interface file (.i) globally rename map C++ `del` to python `delete`, // but applications already followed the old behavior of auto renamed `_del`. // So we implemented old behavior for backward compatibility diff --git a/common/producerstatetable.h b/common/producerstatetable.h index 95061bb51..a5c4b2adc 100644 --- a/common/producerstatetable.h +++ b/common/producerstatetable.h @@ -25,7 +25,7 @@ class ProducerStateTable : public TableBase, public TableName_KeySet const std::string &op = DEL_COMMAND, const std::string &prefix = EMPTY_PREFIX); -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) // SWIG interface file (.i) globally rename map C++ `del` to python `delete`, // but applications already followed the old behavior of auto renamed `_del`. // So we implemented old behavior for backward compatibility diff --git a/common/producertable.h b/common/producertable.h index ff0c4cb75..953b3d332 100644 --- a/common/producertable.h +++ b/common/producertable.h @@ -35,7 +35,7 @@ class ProducerTable : public TableBase, public TableName_KeyValueOpQueues const std::string &op = DEL_COMMAND, const std::string &prefix = EMPTY_PREFIX); -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) // SWIG interface file (.i) globally rename map C++ `del` to python `delete`, // but applications already followed the old behavior of auto renamed `_del`. // So we implemented old behavior for backward compatibility diff --git a/common/sonicv2connector.h b/common/sonicv2connector.h index d700d926f..2bfc937ef 100644 --- a/common/sonicv2connector.h +++ b/common/sonicv2connector.h @@ -62,7 +62,7 @@ class SonicV2Connector_Native std::string m_netns; }; -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) %pythoncode %{ class SonicV2Connector(SonicV2Connector_Native): diff --git a/common/table.h b/common/table.h index 2ba765f7d..a575897db 100644 --- a/common/table.h +++ b/common/table.h @@ -146,7 +146,7 @@ class TableEntryPoppable { } }; -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) %pythoncode %{ def transpose_pops(m): return [tuple(m[j][i] for j in range(len(m))) for i in range(len(m[0]))] @@ -207,7 +207,7 @@ class Table : public TableBase, public TableEntryEnumerable { /* Get the configured ttl value for key */ bool ttl(const std::string &key, int64_t &reply_value); -#ifdef SWIG +#if defined(SWIG) && defined(SWIGPYTHON) // SWIG interface file (.i) globally rename map C++ `del` to python `delete`, // but applications already followed the old behavior of auto renamed `_del`. // So we implemented old behavior for backward compatibility diff --git a/goext/Makefile b/goext/Makefile new file mode 100644 index 000000000..9e01988b1 --- /dev/null +++ b/goext/Makefile @@ -0,0 +1,23 @@ +export CGO_LDFLAGS := -lswsscommon +export CGO_CXXFLAGS := -I/usr/include/swss/ -w -Wall -fpermissive + +GO ?= /usr/local/go/bin/go +SWIG ?= /usr/bin/swig +RM=rm -f + +SWIG_FLAG = -go -cgo -c++ -intgosize 64 +ifeq ($(CONFIGURED_ARCH),arm64) +SWIG_FLAG += -DSWIGWORDSIZE64 +endif + +.PHONY: all check clean + +all: + $(SWIG) $(SWIG_FLAG) -I/usr/include/swss/ swsscommon.i + +check: + sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test + +clean: + $(RM) swsscommon.go *.cxx + diff --git a/goext/swsscommon.i b/goext/swsscommon.i new file mode 100644 index 000000000..b38cc033a --- /dev/null +++ b/goext/swsscommon.i @@ -0,0 +1,13 @@ +%module swsscommon + +%{ +#include "dbconnector.h" +#include "producerstatetable.h" +using namespace swss; +%} + +%include "std_string.i" + +%include "dbconnector.h" +%include "producerstatetable.h" + diff --git a/goext/swsscommon_test.go b/goext/swsscommon_test.go new file mode 100644 index 000000000..356a9f339 --- /dev/null +++ b/goext/swsscommon_test.go @@ -0,0 +1,13 @@ +package swsscommon + +import ( + "testing" + "os" + "path/filepath" +) + +func TestSonicDBConfig(t *testing.T) { + pwd, _ := os.Getwd() + SonicDBConfigInitialize(filepath.Dir(pwd) + "/tests/redis_multi_db_ut_config/database_config.json") +} + From 1a147ceeebb353537b3aaf239a0bc2c2ebd9a441 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Wed, 23 Nov 2022 17:02:38 +0800 Subject: [PATCH 2/8] Use single swsscommon.i --- .azure-pipelines/build-template.yml | 1 - goext/Makefile | 6 ++++-- goext/swsscommon.i | 13 ------------- pyext/swsscommon.i | 4 ++++ 4 files changed, 8 insertions(+), 16 deletions(-) delete mode 100644 goext/swsscommon.i diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index 2a0f290eb..965b2fd02 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -131,7 +131,6 @@ jobs: sudo mkdir /usr/local/yang-models sudo dpkg -i libswsscommon_*.deb - sudo dpkg -i libswsscommon-dev_*.deb sudo dpkg -i python-swsscommon_*.deb ./tests/tests diff --git a/goext/Makefile b/goext/Makefile index 9e01988b1..3476294f8 100644 --- a/goext/Makefile +++ b/goext/Makefile @@ -1,9 +1,10 @@ -export CGO_LDFLAGS := -lswsscommon -export CGO_CXXFLAGS := -I/usr/include/swss/ -w -Wall -fpermissive +export CGO_LDFLAGS := -lswsscommon -lhiredis +export CGO_CXXFLAGS := -I../ -I../common/ -w -Wall -fpermissive GO ?= /usr/local/go/bin/go SWIG ?= /usr/bin/swig RM=rm -f +LN=ln SWIG_FLAG = -go -cgo -c++ -intgosize 64 ifeq ($(CONFIGURED_ARCH),arm64) @@ -13,6 +14,7 @@ endif .PHONY: all check clean all: + -$(LN) -s ../pyext/swsscommon.i swsscommon.i $(SWIG) $(SWIG_FLAG) -I/usr/include/swss/ swsscommon.i check: diff --git a/goext/swsscommon.i b/goext/swsscommon.i deleted file mode 100644 index b38cc033a..000000000 --- a/goext/swsscommon.i +++ /dev/null @@ -1,13 +0,0 @@ -%module swsscommon - -%{ -#include "dbconnector.h" -#include "producerstatetable.h" -using namespace swss; -%} - -%include "std_string.i" - -%include "dbconnector.h" -%include "producerstatetable.h" - diff --git a/pyext/swsscommon.i b/pyext/swsscommon.i index daca2dbf3..266f296dc 100644 --- a/pyext/swsscommon.i +++ b/pyext/swsscommon.i @@ -49,7 +49,9 @@ %include %include %include +#ifdef SWIGPYTHON %include +#endif %include %include %include @@ -64,6 +66,7 @@ %template(GetConfigResult) std::map>>; %template(GetInstanceListResult) std::map; +#ifdef SWIGPYTHON %exception { try { @@ -136,6 +139,7 @@ temp = SWIG_NewPointerObj(*$1, SWIGTYPE_p_swss__Selectable, 0); SWIG_Python_AppendOutput($result, temp); } +#endif %inline %{ template From ca822e81d40a325960f4034f03abaed779087c68 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Wed, 23 Nov 2022 11:41:42 +0000 Subject: [PATCH 3/8] Address comment and fix build parameter. --- .azure-pipelines/build-template.yml | 2 +- goext/Makefile | 3 +-- goext/swsscommon.i | 1 + 3 files changed, 3 insertions(+), 3 deletions(-) create mode 120000 goext/swsscommon.i diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index 965b2fd02..bb15f49ea 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -138,8 +138,8 @@ jobs: pytest --cov=. --cov-report=xml mv coverage.xml tests/coverage.xml gcovr -r ./ -e ".*/swsscommon_wrap.cpp" --exclude-unreachable-branches --exclude-throw-branches -x --xml-pretty -o coverage.xml - redis-cli FLUSHALL make -C goext + redis-cli FLUSHALL make -C goext check rm -rf $(Build.ArtifactStagingDirectory)/download diff --git a/goext/Makefile b/goext/Makefile index 3476294f8..e85025635 100644 --- a/goext/Makefile +++ b/goext/Makefile @@ -14,8 +14,7 @@ endif .PHONY: all check clean all: - -$(LN) -s ../pyext/swsscommon.i swsscommon.i - $(SWIG) $(SWIG_FLAG) -I/usr/include/swss/ swsscommon.i + $(SWIG) $(SWIG_FLAG) -I../common/ swsscommon.i check: sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test diff --git a/goext/swsscommon.i b/goext/swsscommon.i new file mode 120000 index 000000000..5e46e6ab5 --- /dev/null +++ b/goext/swsscommon.i @@ -0,0 +1 @@ +../pyext/swsscommon.i \ No newline at end of file From 13f0c37820614f0350b5bdf569754f5932862d86 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Wed, 23 Nov 2022 12:12:06 +0000 Subject: [PATCH 4/8] Where is libswsscommon.so? --- goext/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/goext/Makefile b/goext/Makefile index e85025635..50fd72016 100644 --- a/goext/Makefile +++ b/goext/Makefile @@ -17,6 +17,7 @@ all: $(SWIG) $(SWIG_FLAG) -I../common/ swsscommon.i check: + find / -name libswsscommon.so sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test clean: From 22a917ce48e30f49a17cb2626c7a5449f3aa6cb4 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Wed, 23 Nov 2022 12:34:05 +0000 Subject: [PATCH 5/8] Add lib path --- goext/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/goext/Makefile b/goext/Makefile index 50fd72016..d23f320ba 100644 --- a/goext/Makefile +++ b/goext/Makefile @@ -1,4 +1,4 @@ -export CGO_LDFLAGS := -lswsscommon -lhiredis +export CGO_LDFLAGS := -lswsscommon -lhiredis -L../common/.libs/ export CGO_CXXFLAGS := -I../ -I../common/ -w -Wall -fpermissive GO ?= /usr/local/go/bin/go @@ -17,7 +17,6 @@ all: $(SWIG) $(SWIG_FLAG) -I../common/ swsscommon.i check: - find / -name libswsscommon.so sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test clean: From 6bafd6b3cab48b6dc68130c49a9df4741dacf59b Mon Sep 17 00:00:00 2001 From: ganglyu Date: Wed, 23 Nov 2022 13:08:42 +0000 Subject: [PATCH 6/8] Update lib path --- goext/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/goext/Makefile b/goext/Makefile index d23f320ba..3c33c6afc 100644 --- a/goext/Makefile +++ b/goext/Makefile @@ -1,4 +1,4 @@ -export CGO_LDFLAGS := -lswsscommon -lhiredis -L../common/.libs/ +export CGO_LDFLAGS := -lswsscommon -lhiredis -L../debian/tmp/usr/lib/x86_64-linux-gnu/ export CGO_CXXFLAGS := -I../ -I../common/ -w -Wall -fpermissive GO ?= /usr/local/go/bin/go From d4599a4e6e21172e097261fd844ee0e76da37fb5 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Wed, 23 Nov 2022 14:04:15 +0000 Subject: [PATCH 7/8] Need dev package --- .azure-pipelines/build-template.yml | 1 + goext/Makefile | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index bb15f49ea..0f67b8287 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -131,6 +131,7 @@ jobs: sudo mkdir /usr/local/yang-models sudo dpkg -i libswsscommon_*.deb + sudo dpkg -i libswsscommon-dev_*.deb sudo dpkg -i python-swsscommon_*.deb ./tests/tests diff --git a/goext/Makefile b/goext/Makefile index 3c33c6afc..7f59ca6e5 100644 --- a/goext/Makefile +++ b/goext/Makefile @@ -1,4 +1,4 @@ -export CGO_LDFLAGS := -lswsscommon -lhiredis -L../debian/tmp/usr/lib/x86_64-linux-gnu/ +export CGO_LDFLAGS := -lswsscommon -lhiredis export CGO_CXXFLAGS := -I../ -I../common/ -w -Wall -fpermissive GO ?= /usr/local/go/bin/go @@ -14,7 +14,7 @@ endif .PHONY: all check clean all: - $(SWIG) $(SWIG_FLAG) -I../common/ swsscommon.i + $(SWIG) $(SWIG_FLAG) -I/usr/include/swss/ swsscommon.i check: sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test From 2c0ff4a279f5478b05fab40f0cfea45b030d4d87 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Thu, 24 Nov 2022 03:07:25 +0000 Subject: [PATCH 8/8] Add UT for producer state table --- goext/swsscommon_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/goext/swsscommon_test.go b/goext/swsscommon_test.go index 356a9f339..cbefd96f8 100644 --- a/goext/swsscommon_test.go +++ b/goext/swsscommon_test.go @@ -11,3 +11,26 @@ func TestSonicDBConfig(t *testing.T) { SonicDBConfigInitialize(filepath.Dir(pwd) + "/tests/redis_multi_db_ut_config/database_config.json") } +func TestProducerStateTable(t *testing.T) { + db := NewDBConnector("APPL_DB", uint(0), true) + pt := NewProducerStateTable(db, "TEST_TABLE") + tbl := NewTable(db, "_TEST_TABLE") + vec := NewFieldValuePairs() + key := "aaa" + pair := NewFieldValuePair("a", "b") + vec.Add(pair) + pt.Set(key, vec, "SET", "") + fvs := NewFieldValuePairs() + ret := tbl.Get("aaa", fvs) + if ret != true { + t.Errorf("Get table failed") + } + fv := fvs.Get(0) + if fv.GetFirst() != "a" { + t.Errorf("Wronge fv first %v", fv.GetFirst()) + } + if fv.GetSecond() != "b" { + t.Errorf("Wronge fv second: %v", fv.GetSecond()) + } +} +