Cleanup

Added variables to Makefiles to specify custom exports/ directory;
Split exception classes in Java & C#;
Added more comments;
Renamed library and Go package;
Removed real (pure) tests;
Added generate_lib.ps1 to generate import .lib for Windows (Swift);
Moved built Go libraries to exports/lib/;
Switch to hopefully faster Swift GitHub Action.

5 files changed, 73 insertions, 40 deletions

diff --git a/wrappers/python/Makefile b/wrappers/python/Makefile
index be4beaa..ba4cf5f 100644
--- a/wrappers/python/Makefile
+++ b/wrappers/python/Makefile
@@ -1,15 +1,27 @@
 .PHONY: pack test clean
 
+EXPORTS_PATH ?= ../../exports
+EXPORTS_LIB_PATH ?= $(EXPORTS_PATH)/lib
+
+ifneq ($(MAKECMDGOALS),clean)
+include $(EXPORTS_PATH)/platform.mk
+endif
+
 ifdef PLAT_NAME
 SETUP_ARGS += --plat-name=$(PLAT_NAME)
 endif
 
 # Build for current platform only
 pack:
-	./setup.py bdist_wheel $(SETUP_ARGS)
+	./setup.py bdist_wheel $(SETUP_ARGS) --exports-lib-path="$(EXPORTS_LIB_PATH)"
 
 test:
-	$(MAKE) -C ../../exports copy-to COPY_TARGET=../wrappers/python/eduvpncommon/lib
+ifneq ($(EXPORTS_PATH),)
+ifneq ($(wildcard $(EXPORTS_PATH)/Makefile),)
+	$(MAKE) -C "$(EXPORTS_PATH)"
+endif
+endif
+	install "$(EXPORTS_LIB_PATH)/$(GOOS)/$(GOARCH)/$(LIB_FILE)" -Dt "eduvpncommon/lib"
 	python3 -m unittest test_discovery
 	rm eduvpncommon/lib/*
 
diff --git a/wrappers/python/README.md b/wrappers/python/README.md
index 6175910..bce492e 100644
--- a/wrappers/python/README.md
+++ b/wrappers/python/README.md
@@ -2,9 +2,7 @@
 
 ## Requirements
 
-Python 3.6+ is assumed, but it may work with older versions.
-
-TODO Build
+Python 3.6+ is assumed, but it may work with older versions. To build, `setuptools` and `wheel` are required.
 
 ## Build & test
 
@@ -16,7 +14,11 @@ Build wheel using library for current platform:
 make pack
 ```
 
-Build wheel using library for specified platform (passed to setuptools `--plat-name`):
+(This does not build the shared Go library.)
+
+Build wheel using library for specified platform (passed to setuptools `--plat-name`,
+see [`get_build_platform`](https://setuptools.pypa.io/en/latest/pkg_resources.html?highlight=get_build_platform#platform-utilities)
+for more):
 
 ```shell
 make pack PLAT_NAME=win32
@@ -28,9 +30,12 @@ To install the wheel, run:
 pip install dist/eduvpncommon-[version]-py3-none-[platform].whl
 ```
 
-You could also reference the discovery module directly and copy the library for the platform to the `eduvpncommon/lib`
+You could also reference the discovery module directly and copy the library for the platform to the `eduvpncommon/lib/`
 folder.
 
+If you do not build this as part of the full repository, specify `EXPORTS_PATH="path/to/exports-folder"` when calling
+make. This folder must contain `platform.mk` and the `lib/` folder with built libraries.
+
 Test:
 
 ```shell
diff --git a/wrappers/python/eduvpncommon/discovery.py b/wrappers/python/eduvpncommon/discovery.py
index f7a312e..f22df58 100644
--- a/wrappers/python/eduvpncommon/discovery.py
+++ b/wrappers/python/eduvpncommon/discovery.py
@@ -15,28 +15,30 @@ _lib_suffixes = defaultdict(lambda: ".so", {
 
 _os = platform.system().lower()
 
-_libname = f"{_lib_prefixes[_os]}eduvpn_verify{_lib_suffixes[_os]}"
-_lib = cdll.LoadLibrary(str(pathlib.Path(__file__).parent / "lib" / _libname))
+_libname = "eduvpn_common"
+_libfile = f"{_lib_prefixes[_os]}{_libname}{_lib_suffixes[_os]}"
+# Library should have been copied to the lib/ folder
+_lib = cdll.LoadLibrary(str(pathlib.Path(__file__).parent / "lib" / _libfile))
 
 
-class GoSlice(Structure):
+class _GoSlice(Structure):
     _fields_ = [("data", POINTER(c_char)), ("len", c_int64), ("cap", c_int64)]
 
     @staticmethod
-    def make(bs: bytes) -> "GoSlice":
-        return GoSlice((c_char * len(bs))(*bs), len(bs), len(bs))
+    def make(bs: bytes) -> "_GoSlice":
+        return _GoSlice((c_char * len(bs))(*bs), len(bs), len(bs))
 
 
-_lib.Verify.argtypes, _lib.Verify.restype = [GoSlice, GoSlice, GoSlice, c_uint64], c_int64
-_lib.InsecureTestingSetExtraKey.argtypes, _lib.InsecureTestingSetExtraKey.restype = [GoSlice], None
+_lib.Verify.argtypes, _lib.Verify.restype = [_GoSlice, _GoSlice, _GoSlice, c_uint64], c_int64
+_lib.InsecureTestingSetExtraKey.argtypes, _lib.InsecureTestingSetExtraKey.restype = [_GoSlice], None
 
 
 class VerifyErrorCode(Enum):
-    ErrUnknownExpectedFileName = 1  # Expected file name is not one of the recognized values.
+    ErrUnknownExpectedFileName = 1  # Unknown expected file name specified. The signature has not been verified.
     ErrInvalidSignature = 2  # Signature is invalid (for the expected file type).
     ErrInvalidSignatureUnknownKey = 3  # Signature was created with an unknown key and has not been verified.
-    ErrTooOld = 4  # Signature has a timestamp lower than the specified minimum signing time.
-    Unknown = -1  # Other unknown error
+    ErrTooOld = 4  # Signature timestamp smaller than specified minimum signing time (rollback).
+    Unknown = -1  # Other unknown error.
 
 
 class VerifyError(Exception):
@@ -44,6 +46,7 @@ class VerifyError(Exception):
     code_int: int  # Original error code also for VerifyErrorCode.Unknown
 
     def __init__(self, err: int):
+        assert err
         try:
             self.code = VerifyErrorCode(err)
         except ValueError:
@@ -68,13 +71,13 @@ def verify(signature: bytes, signed_json: bytes, expected_file_name: str, min_si
     :param signature: .minisig signature file contents.
     :param signed_json: Signed .json file contents.
     :param expected_file_name: The file type to be verified, one of "server_list.json" or "organization_list.json".
-    :param min_sign_time: Minimum time for signature. Should be set to at least the time in a previously retrieved file.
+    :param min_sign_time: Minimum time for signature. Should be set to at least the time of the previous signature.
 
     :raises VerifyException: If signature verification fails or expectedFileName is not one of the allowed values.
     """
 
-    err = _lib.Verify(GoSlice.make(signature), GoSlice.make(signed_json),
-                      GoSlice.make(expected_file_name.encode()), min_sign_time)
+    err = _lib.Verify(_GoSlice.make(signature), _GoSlice.make(signed_json),
+                      _GoSlice.make(expected_file_name.encode()), min_sign_time)
     if err:
         raise VerifyError(err)
 
@@ -82,4 +85,4 @@ def verify(signature: bytes, signed_json: bytes, expected_file_name: str, min_si
 def _insecure_testing_set_extra_key(key_string: str) -> None:
     """Use for testing only, see Go documentation."""
 
-    _lib.InsecureTestingSetExtraKey(GoSlice.make(key_string.encode()))
+    _lib.InsecureTestingSetExtraKey(_GoSlice.make(key_string.encode()))
diff --git a/wrappers/python/setup.py b/wrappers/python/setup.py
index db254aa..9e7bde4 100755
--- a/wrappers/python/setup.py
+++ b/wrappers/python/setup.py
@@ -1,17 +1,20 @@
 #!/usr/bin/env python3
 
 import os
-import pathlib
 import shutil
+import sys
 import typing
 from collections import defaultdict
 
-import sys
 from setuptools import setup
 from wheel.bdist_wheel import bdist_wheel as _bdist_wheel
 
+_libname = "eduvpn_common"
+
 
 def getlibpath(plat_name: str) -> typing.Union[str, None]:
+    """Get library path for plat_name relative to exports/lib/ folder."""
+
     _plat_map = defaultdict(lambda: plat_name, {
         "win32": "win-x86",
     })
@@ -47,12 +50,21 @@ def getlibpath(plat_name: str) -> typing.Union[str, None]:
 
     processed_os = _os_map[plat_os]
     return f"{processed_os}/{_arch_map[plat_arch]}/" \
-           f"{_lib_prefixes[processed_os]}eduvpn_verify{_lib_suffixes[processed_os]}"
+           f"{_lib_prefixes[processed_os]}{_libname}{_lib_suffixes[processed_os]}"
 
 
+# Adapted from https://stackoverflow.com/a/51794740
 # You would say there would be a better way to do all of this, but I couldn't find it
 
 class bdist_wheel(_bdist_wheel):
+    user_options = _bdist_wheel.user_options + [
+        ("exports-lib-path=", None, "path to exports/lib directory"),
+    ]
+
+    def initialize_options(self):
+        super().initialize_options()
+        self.exports_lib_path = "../../exports/lib"  # default
+
     def run(self):
         self.plat_name_supplied = True  # Force use platform
 
@@ -63,9 +75,10 @@ class bdist_wheel(_bdist_wheel):
 
         print(f"Building wheel for platform {self.plat_name}")
 
-        shutil.copy2(f"../../exports/{libpath}", "eduvpncommon/lib/")
+        # setuptools will only use paths inside the package for package_data, so we copy the library
+        tmp_lib = shutil.copy2(f"{self.exports_lib_path}/{libpath}", "eduvpncommon/lib/")
         _bdist_wheel.run(self)
-        os.remove(f"eduvpncommon/lib/{pathlib.Path(libpath).name}")
+        os.remove(tmp_lib)
 
 
 setup(
@@ -73,6 +86,6 @@ setup(
     version="0.1.0",
     packages=["eduvpncommon"],
     python_requires=">=3.6",
-    package_data={"eduvpncommon": ["lib/*eduvpn_verify*"]},
+    package_data={"eduvpncommon": [f"lib/*{_libname}*"]},
     cmdclass={"bdist_wheel": bdist_wheel},
 )
diff --git a/wrappers/python/test_discovery.py b/wrappers/python/test_discovery.py
index 1282a3e..73c51c4 100755
--- a/wrappers/python/test_discovery.py
+++ b/wrappers/python/test_discovery.py
@@ -14,21 +14,21 @@ def read_bytes(path: str) -> bytes:
 class VerifyTests(unittest.TestCase):
     @classmethod
     def setUpClass(cls) -> None:
-        with open(f"{test_data_dir}/dummy/public.key") as f:
+        with open(f"{test_data_dir}/public.key") as f:
             discovery._insecure_testing_set_extra_key(f.readlines()[-1][:-1])
 
     def testValid(self):
         discovery.verify(
-            read_bytes(f"{test_data_dir}/dummy/server_list.json.minisig"),
-            read_bytes(f"{test_data_dir}/dummy/server_list.json"),
+            read_bytes(f"{test_data_dir}/server_list.json.minisig"),
+            read_bytes(f"{test_data_dir}/server_list.json"),
             "server_list.json",
             0
         )
 
     def testValidMemoryView(self):
         discovery.verify(
-            read_bytes(f"{test_data_dir}/dummy/server_list.json.minisig"),
-            memoryview(b"abc" + read_bytes(f"{test_data_dir}/dummy/server_list.json") + b"abc")[3:-3],
+            read_bytes(f"{test_data_dir}/server_list.json.minisig"),
+            memoryview(b"abc" + read_bytes(f"{test_data_dir}/server_list.json") + b"abc")[3:-3],
             "server_list.json",
             0
         )
@@ -36,8 +36,8 @@ class VerifyTests(unittest.TestCase):
     def testInvalidSignature(self):
         with self.assertRaises(discovery.VerifyError) as ctx:
             discovery.verify(
-                read_bytes(f"{test_data_dir}/dummy/random.txt"),
-                read_bytes(f"{test_data_dir}/dummy/server_list.json"),
+                read_bytes(f"{test_data_dir}/random.txt"),
+                read_bytes(f"{test_data_dir}/server_list.json"),
                 "server_list.json",
                 0
             )
@@ -46,8 +46,8 @@ class VerifyTests(unittest.TestCase):
     def testWrongKey(self):
         with self.assertRaises(discovery.VerifyError) as ctx:
             discovery.verify(
-                read_bytes(f"{test_data_dir}/dummy/server_list.json.wrong_key.minisig"),
-                read_bytes(f"{test_data_dir}/dummy/server_list.json"),
+                read_bytes(f"{test_data_dir}/server_list.json.wrong_key.minisig"),
+                read_bytes(f"{test_data_dir}/server_list.json"),
                 "server_list.json",
                 0
             )
@@ -56,8 +56,8 @@ class VerifyTests(unittest.TestCase):
     def testOldSignature(self):
         with self.assertRaises(discovery.VerifyError) as ctx:
             discovery.verify(
-                read_bytes(f"{test_data_dir}/dummy/server_list.json.minisig"),
-                read_bytes(f"{test_data_dir}/dummy/server_list.json"),
+                read_bytes(f"{test_data_dir}/server_list.json.minisig"),
+                read_bytes(f"{test_data_dir}/server_list.json"),
                 "server_list.json",
                 1 << 31
             )
@@ -66,8 +66,8 @@ class VerifyTests(unittest.TestCase):
     def TestUnknownExpectedFile(self):
         with self.assertRaises(discovery.VerifyError) as ctx:
             discovery.verify(
-                read_bytes(f"{test_data_dir}/dummy/other_list.json.minisig"),
-                read_bytes(f"{test_data_dir}/dummy/other_list.json"),
+                read_bytes(f"{test_data_dir}/other_list.json.minisig"),
+                read_bytes(f"{test_data_dir}/other_list.json"),
                 "other_list.json",
                 0
             )