python: libsmb_samba_internal vs. libsmb
Tim Beale
timbeale at catalyst.net.nz
Wed Jan 9 04:08:25 UTC 2019
How about the attached patch as a compromise? It renames the Python
bindings back to libsmb_samba_internal, which should make it obvious to
external consumers that it's not a stable API. And we just change how
the python module is imported, so that we still end up with nice-looking
samba-tool code.
CI link: https://gitlab.com/catalyst-samba/samba/pipelines/42620501
On 9/01/19 10:01 AM, Tim Beale via samba-technical wrote:
> OK, I agree it doesn't make sense to expose this API to external
> consumers. I'll make a change to fix this up. Sorry for the
> misunderstanding.
>
> On 9/01/19 8:58 AM, Volker Lendecke wrote:
>> On Tue, Jan 08, 2019 at 01:04:48PM +0100, Stefan Metzmacher wrote:
>>> Hi Tim,
>>>
>>>> I wanted to rename the python module because the naming didn't seem
>>>> consistent with any of the other python bindings Samba has. I think
>>>> 'libsmb_samba_internal' made sense when it was unused by any of the
>>>> samba python code (except some test code). But now (well, soon) these
>>>> Python bindings will be used by the samba-tool code. It looks a bit
>>>> strange in samba-tool to have:
>>>>
>>>> conn = libsmb_samba_internal.Conn(server)
>>>>
>>>> E.g. my latest patches start to use the new bindings in more places:
>>>> https://gitlab.com/catalyst-samba/samba/commits/timb-pysmb-latest
>>>>
>>>> But maybe I've misunderstood something. Why do you want to keep the
>>>> libsmb_samba_internal name?
>>> The idea was that we make it clear that we don't expose a stable api
>>> for external consumers. And we're free to change it without
>>> consequences, as you did with the directory listing.
>>>
>>> samba-tool and other tools are not external consumers and would be
>>> changed at the same time as we change the api.
>>>
>>> Volker, you invented the name, do you have any comments on this?
>> I don't really remember that, but the argument that this is an
>> internal API that has no stability guarantees is pretty convincing. We
>> need a clear distinction between things we publish to our users and
>> things that we can freely mess with. On the C side of things we have
>> the (admittedly limited) libsmbclient and all the internal stuff that
>> smbclient and winbind use. We can't restrict ourselves to an ABI/API
>> for some things.
>>
>> Volker
>>
-------------- next part --------------
From 2caa8191e38fde0c9678baaeb64a7bb7119979e3 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 9 Jan 2019 10:15:49 +1300
Subject: [PATCH] s3:libsmb: Revert SMB Py bindings name back to
libsmb_samba_internal
In order to make it clear that the APIs in these Python bindings are
unstable and should not be used by external consumers, this patch
changes the name of the Python bindings back to libsmb_samba_internal.
To make the Python code that uses these bindings (i.e. samba-tool, etc)
look a little cleaner, we can just change the module name as we import
it, e.g.
from samba.samba3 import libsmb_samba_internal as libsmb
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/ntacls.py | 2 +-
python/samba/tests/dcerpc/raw_testcase.py | 2 +-
python/samba/tests/libsmb.py | 2 +-
python/samba/tests/smb.py | 2 +-
source3/libsmb/pylibsmb.c | 13 ++++++++-----
source3/wscript_build | 2 +-
6 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/python/samba/ntacls.py b/python/samba/ntacls.py
index 1ab5fd3..9924573 100644
--- a/python/samba/ntacls.py
+++ b/python/samba/ntacls.py
@@ -32,7 +32,7 @@ from samba.samba3 import param as s3param
from samba.dcerpc import security, xattr, idmap
from samba.ndr import ndr_pack, ndr_unpack
from samba.samba3 import smbd
-from samba.samba3 import libsmb
+from samba.samba3 import libsmb_samba_internal as libsmb
# don't include volumes
SMB_FILE_ATTRIBUTE_FLAGS = libsmb.FILE_ATTRIBUTE_SYSTEM | \
diff --git a/python/samba/tests/dcerpc/raw_testcase.py b/python/samba/tests/dcerpc/raw_testcase.py
index 4f3f999..fc1c86a 100644
--- a/python/samba/tests/dcerpc/raw_testcase.py
+++ b/python/samba/tests/dcerpc/raw_testcase.py
@@ -35,7 +35,7 @@ from samba.ntstatus import (
)
from samba import NTSTATUSError
from samba.samba3 import param as s3param
-from samba.samba3 import libsmb
+from samba.samba3 import libsmb_samba_internal as libsmb
class smb_pipe_socket(object):
diff --git a/python/samba/tests/libsmb.py b/python/samba/tests/libsmb.py
index 5a37b09..e8f8e7f 100644
--- a/python/samba/tests/libsmb.py
+++ b/python/samba/tests/libsmb.py
@@ -17,7 +17,7 @@
"""Tests for samba.samba3.libsmb."""
-from samba.samba3 import libsmb
+from samba.samba3 import libsmb_samba_internal as libsmb
from samba.dcerpc import security
from samba.samba3 import param as s3param
from samba import credentials
diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index fcb0b7f..5a52885 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -22,7 +22,7 @@ import sys
from samba import NTSTATUSError
from samba.ntstatus import (NT_STATUS_OBJECT_NAME_NOT_FOUND,
NT_STATUS_OBJECT_PATH_NOT_FOUND)
-from samba.samba3 import libsmb
+from samba.samba3 import libsmb_samba_internal as libsmb
from samba.samba3 import param as s3param
PY3 = sys.version_info[0] == 3
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 4322c2b..9acfbb2 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -1,6 +1,9 @@
/*
* Unix SMB/CIFS implementation.
- * Samba-internal work in progress Python binding for libsmbclient
+ *
+ * SMB client Python bindings used internally by Samba (for things like
+ * samba-tool). These Python bindings may change without warning, and so
+ * should not be used outside of the Samba codebase.
*
* Copyright (C) Volker Lendecke 2012
*
@@ -1526,7 +1529,7 @@ static PyMethodDef py_cli_state_methods[] = {
static PyTypeObject py_cli_state_type = {
PyVarObject_HEAD_INIT(NULL, 0)
- .tp_name = "libsmb.Conn",
+ .tp_name = "libsmb_samba_internal.Conn",
.tp_basicsize = sizeof(struct py_cli_state),
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
.tp_doc = "libsmb connection",
@@ -1540,17 +1543,17 @@ static PyMethodDef py_libsmb_methods[] = {
{ NULL },
};
-void initlibsmb(void);
+void initlibsmb_samba_internal(void);
static struct PyModuleDef moduledef = {
PyModuleDef_HEAD_INIT,
- .m_name = "libsmb",
+ .m_name = "libsmb_samba_internal",
.m_doc = "libsmb wrapper",
.m_size = -1,
.m_methods = py_libsmb_methods,
};
-MODULE_INIT_FUNC(libsmb)
+MODULE_INIT_FUNC(libsmb_samba_internal)
{
PyObject *m = NULL;
diff --git a/source3/wscript_build b/source3/wscript_build
index 9d188a8..a8ea8e5 100644
--- a/source3/wscript_build
+++ b/source3/wscript_build
@@ -1323,7 +1323,7 @@ for env in bld.gen_python_environments():
bld.SAMBA3_PYTHON('pylibsmb',
source='libsmb/pylibsmb.c',
deps='smbclient samba-credentials %s' % pycredentials,
- realname='samba/samba3/libsmb.so'
+ realname='samba/samba3/libsmb_samba_internal.so'
)
bld.SAMBA3_BINARY('spotlight2sparql',
--
2.7.4
More information about the samba-technical
mailing list