Re: [Duplicity-talk] gpg key password asked for backup after verify

[edgar . soldin]

nice.. let me beautify it a bit still _and_ add the patchdir.py change ;) .. 
the line numbers are from 0.7.12 btw.

'duplicity/librsync.py' line 159+
"
class PatchedFile(LikeFile):
    """File-like object which applies a librsync delta incrementally"""
    def __init__(self, basis_file, delta_file):
        """PatchedFile initializer - call with basis delta

        Here basis_file must be a true Python file, because we may
        need to seek() around in it a lot, and this is done in C.
        delta_file only needs read() and close() methods.

        """
        LikeFile.__init__(self, delta_file)
        if not isinstance(basis_file, types.FileType):
            """ tempfile.TemporaryFile() only guarantees a true file
            object on posix platforms. on cygwin/windows a file-like 
            object whose file attribute is the underlying true file 
            object is returned.
            """
            if hasattr(basis_file, 'file') and isinstance(basis_file.file, 
types.FileType):
                basis_file = basis_file.file
            else:
                raise TypeError("basis_file must be a (true) file or an object 
whose file attribute is the underlying true file object")
        try:
            self.maker = _librsync.new_patchmaker(basis_file)
        except _librsync.librsyncError as e:
            raise librsyncError(str(e))
"

'duplicity/patchdir.py' line 489+
"
    for delta_ropath in patch_seq[1:]:
        assert delta_ropath.difftype == "diff", delta_ropath.difftype
        if not isinstance(current_file, file):
            """
            librsync insists on a real file object, which we create manually
            by using the duplicity.tempdir to tell us where.

            See https://bugs.launchpad.net/duplicity/+bug/670891 for discussion
            of os.tmpfile() vs tempfile.TemporaryFile() w.r.t. Windows / Posix,
            which is worked around in librsync.PatchedFile() now.
            """
            tempfp = tempfile.TemporaryFile(dir=tempdir.default().dir())
            util.copyfileobj(current_file, tempfp)
            assert not current_file.close()
            tempfp.seek(0)
            current_file = tempfp
        current_file = librsync.PatchedFile(current_file,
                                            delta_ropath.open("rb"))
    result = patch_seq[-1].get_ropath()
    result.setfileobj(current_file)
    return result
"

sunny fluffy cloudy greetings.. ede

On 31.05.2017 17:40, Kenneth Loafman wrote:
> Thanks ede!
> 
> No, I do not need a patch.  Will get it in ASAP.
> 
> ...Ken
> 
> 
> On Wed, May 31, 2017 at 10:32 AM, edgar.soldin--- via Duplicity-talk 
> <address@hidden <mailto:address@hidden>> wrote:
> 
>     hey Ken,
> 
>     i tested the following on a local cygwin installation and it patched away 
> fine :)
> 
>     'duplicity/librsync.py' line 159+
>     "
>     class PatchedFile(LikeFile):
>         """File-like object which applies a librsync delta incrementally"""
>         def __init__(self, basis_file, delta_file):
>             """PatchedFile initializer - call with basis delta
> 
>             Here basis_file must be a true Python file, because we may
>             need to seek() around in it a lot, and this is done in C.
>             delta_file only needs read() and close() methods.
> 
>             """
>             LikeFile.__init__(self, delta_file)
>             if not isinstance(basis_file, types.FileType):
>                 if hasattr(basis_file, 'file') and 
> isinstance(basis_file.file, types.FileType):
>                     basis_file = basis_file.file
>                 else:
>                     raise TypeError("basis_file must be a (true) file or an 
> object whose file attribute is the underlying true file object")
>             try:
>                 self.maker = _librsync.new_patchmaker(basis_file)
>             except _librsync.librsyncError as e:
>                 raise librsyncError(str(e))
>     "
> 
>     is that ok w/ you? do you need a branch?  ..ede
> 
>     On 29.05.2017 16:47, Kenneth Loafman wrote:
>     > I don't know what the underlying code looks like for non-POSIX systems, 
> and I don't have a Windows/Cygwin system to test against, so my answer would 
> be that I would /prefer/ it be done in /_librsyncmodule.c/ since that seems 
> to be the only place that needs it.  Failing that, /librsync.py/ 
> <http://librsync.py/> is my second preference since it's only one level up.
>     >
>     > We would still leave the error check  on line #305 since we could have 
> made a different error.  We would need to set the error message accordingly.
>     >
>     > ...Ken
>     >
>     >
>     > On Mon, May 29, 2017 at 8:47 AM, edgar.soldin--- via Duplicity-talk 
> <address@hidden <mailto:address@hidden> <mailto:address@hidden 
> <mailto:address@hidden>>> wrote:
>     >
>     >     Ken,
>     >
>     >     On 29.05.2017 14:27, edgar.soldin--- via Duplicity-talk wrote:
>     >     > On 29.05.2017 13:26, Kenneth Loafman wrote:
>     >     >> 1) This was from the README for 0.7.05:
>     >     >>
>     >     >>         * Fix bug #1494228 CygWin: TypeError: basis_file must be 
> a (true) file
>     >     >>
>     >     >>           - The problem that caused the change to 
> tempfile.TemporaryFile was due
>     >     >>
>     >     >>             to the fact that os.tmpfile always creates its file 
> in the system
>     >     >>
>     >     >>             temp directory, not in the directory specified.  The 
> fix applied was
>     >     >>
>     >     >>             to use os.tmpfile in cygwin/windows and 
> tempfile.TemporaryFile in all
>     >     >>
>     >     >>             the rest.  This means that cygwin is now broken with 
> respect to temp
>     >     >>
>     >     >>             file placement of this one file (deleted 
> automatically on close).
>     >     >
>     >     > wrt. os.tmpfile() .. afaics. does
>     >     >
>     >     >    https://hg.python.org/cpython/file/2.7/Lib/tempfile.py#l484 
> <https://hg.python.org/cpython/file/2.7/Lib/tempfile.py#l484> 
> <https://hg.python.org/cpython/file/2.7/Lib/tempfile.py#l484 
> <https://hg.python.org/cpython/file/2.7/Lib/tempfile.py#l484>>
>     >     >
>     >     > use the os module ( os.open, os.fdopen ) all around to create 
> temp files. aren't these supposed to return a (true) file? the docs say 
> nothing in this regard. 
> https://docs.python.org/2/library/os.html#file-object-creation 
> <https://docs.python.org/2/library/os.html#file-object-creation> 
> <https://docs.python.org/2/library/os.html#file-object-creation 
> <https://docs.python.org/2/library/os.html#file-object-creation>>
>     >     >
>     >     > ..ede
>     >     >
>     >
>     >     ok answering myself here, after reading the original bug report 
> thoroughly, which says
>     >     "
>     >     The problem is that, as explained in the Python documentation 
> (http://docs.python.org/library/tempfile.html 
> <http://docs.python.org/library/tempfile.html> 
> <http://docs.python.org/library/tempfile.html 
> <http://docs.python.org/library/tempfile.html>>), tempfile.TemporaryFile 
> returns a *file-like* object, not a file object. Under Linux, it effectively 
> returns a file object but under windows it does not. Hence, the type of the 
> object returned is *not* a types.FileType, causing the exception to be raised.
>     >     "
>     >       https://bugs.launchpad.net/duplicity/+bug/670891 
> <https://bugs.launchpad.net/duplicity/+bug/670891> 
> <https://bugs.launchpad.net/duplicity/+bug/670891 
> <https://bugs.launchpad.net/duplicity/+bug/670891>>
>     >
>     >     further reading shows that the error stems from 
> 'duplicity/librsync.py'
>     >     "
>     >       File "/usr/lib/python2.6/site-packages/duplicity/librsync.py", 
> line 167, in __init__
>     >         raise TypeError("basis_file must be a (true) file")
>     >     "
>     >
>     >
>     >     so obviously something is done differently in the 
> 'duplicity/_librsyncmodule.c', that needs a delta patch file to be a plain 
> File object. let's see 'duplicity/librsync.py' says
>     >     "
>     >     class PatchedFile(LikeFile):
>     >         """File-like object which applies a librsync delta 
> incrementally"""
>     >         def __init__(self, basis_file, delta_file):
>     >             """PatchedFile initializer - call with basis delta
>     >
>     >             Here basis_file must be a true Python file, because we may
>     >             need to seek() around in it a lot, and this is done in C.
>     >             delta_file only needs read() and close() methods.
>     >
>     >             """
>     >             LikeFile.__init__(self, delta_file)
>     >             if not isinstance(basis_file, types.FileType):
>     >                 raise TypeError("basis_file must be a (true) file")
>     >             try:
>     >                 self.maker = _librsync.new_patchmaker(basis_file)
>     >             except _librsync.librsyncError as e:
>     >                 raise librsyncError(str(e))
>     >     "
>     >     
> http://bazaar.launchpad.net/~duplicity-team/duplicity/0.8-series/view/head:/duplicity/librsync.py#L163
>  
> <http://bazaar.launchpad.net/%7Eduplicity-team/duplicity/0.8-series/view/head:/duplicity/librsync.py#L163>
>  
> <http://bazaar.launchpad.net/%7Eduplicity-team/duplicity/0.8-series/view/head:/duplicity/librsync.py#L163
>  
> <http://bazaar.launchpad.net/%7Eduplicity-team/duplicity/0.8-series/view/head:/duplicity/librsync.py#L163>>
>     >
>     >
>     >     and in 'duplicity/_librsyncmodule.c' it tries to utilize 
> 'PyObject_AsFileDescriptor(python_file)'
>     >     "
>     >     /* Call with the basis file */
>     >     static PyObject*
>     >     _librsync_new_patchmaker(PyObject* self, PyObject* args)
>     >     {
>     >       _librsync_PatchMakerObject* pm;
>     >       PyObject *python_file;
>     >       int fd;
>     >
>     >       if (!PyArg_ParseTuple(args, "O:new_patchmaker", &python_file))
>     >         return NULL;
>     >       fd = PyObject_AsFileDescriptor(python_file);
>     >     "
>     >     
> http://bazaar.launchpad.net/~duplicity-team/duplicity/0.8-series/view/head:/duplicity/_librsyncmodule.c#L294
>  
> <http://bazaar.launchpad.net/%7Eduplicity-team/duplicity/0.8-series/view/head:/duplicity/_librsyncmodule.c#L294>
>  
> <http://bazaar.launchpad.net/%7Eduplicity-team/duplicity/0.8-series/view/head:/duplicity/_librsyncmodule.c#L294
>  
> <http://bazaar.launchpad.net/%7Eduplicity-team/duplicity/0.8-series/view/head:/duplicity/_librsyncmodule.c#L294>>
>     >
>     >
>     >     so, if i understand correctly, we just have to implement the method 
> 'fileno()' as described here
>     >       
> https://docs.python.org/2/c-api/object.html#c.PyObject_AsFileDescriptor 
> <https://docs.python.org/2/c-api/object.html#c.PyObject_AsFileDescriptor> 
> <https://docs.python.org/2/c-api/object.html#c.PyObject_AsFileDescriptor 
> <https://docs.python.org/2/c-api/object.html#c.PyObject_AsFileDescriptor>>
>     >     in another wrapper around the tempfile.TemporaryFile() return value
>     >       
> https://docs.python.org/2/library/tempfile.html#tempfile.TemporaryFile 
> <https://docs.python.org/2/library/tempfile.html#tempfile.TemporaryFile> 
> <https://docs.python.org/2/library/tempfile.html#tempfile.TemporaryFile 
> <https://docs.python.org/2/library/tempfile.html#tempfile.TemporaryFile>>
>     >     , utilizing (if necessary) the file-like object's file attribute as 
> mentioned in the python doc and return
>     >     "
>     >     The returned object is a true file object on POSIX platforms. On 
> other platforms, it is a file-like object whose file attribute is the 
> underlying true file object.
>     >     "
>     >
>     >     implementing that should render the failing TypeError("basis_file 
> must be a (true) file") in 'duplicity/librsync.py' meaningless, so it could 
> be removed or?
>     >
>     >     alternatively, we could simply extend 'duplicity/librsync.py', to 
> test for a file attribute, in case the "file" object given is just a wrapper. 
> on success we then simply deliver that (pseudo code)
>     >     "
>     >     self.maker = _librsync.new_patchmaker(basis_file.file)
>     >     "
>     >
>     >     what do you think Ken? would that work? which approach would you 
> prefer? ..ede
>     >
>     >     _______________________________________________
>     >     Duplicity-talk mailing list
>     >     address@hidden <mailto:address@hidden> <mailto:address@hidden 
> <mailto:address@hidden>>
>     >     https://lists.nongnu.org/mailman/listinfo/duplicity-talk 
> <https://lists.nongnu.org/mailman/listinfo/duplicity-talk> 
> <https://lists.nongnu.org/mailman/listinfo/duplicity-talk 
> <https://lists.nongnu.org/mailman/listinfo/duplicity-talk>>
>     >
>     >
> 
> 
>     _______________________________________________
>     Duplicity-talk mailing list
>     address@hidden <mailto:address@hidden>
>     https://lists.nongnu.org/mailman/listinfo/duplicity-talk 
> <https://lists.nongnu.org/mailman/listinfo/duplicity-talk>
> 
>