List Archive

Thread

Thread Index

Message

From: Thomas Klausner <tk%giga.or.at@localhost>
To: Michał Janiszewski <janisozaur%gmail.com@localhost>
Subject: Re: UWP builds
Date: Mon, 4 Dec 2017 11:18:08 +0100

On Thu, Nov 30, 2017 at 11:48:50PM +0100, Michał Janiszewski wrote:
> I have just noticed I missent this email and it never reached the mailing 
> list.
> It only strengthens my point about mail reviews vs pull requests and
> true to my words I have submitted a pull request:
> https://github.com/nih-at/libzip/pull/16

Thanks. I've added review comments.

> It already contains your suggestions. I can come up with a sample CI
> job proving UWP builds with those changes, should you need that.

If you could get appveyor working on github, that'd be really
appreciated. (Or if there is other automation that can be
automatically run from github.)

> Please consider merging it and if you do, can I ask for creating
> another patch release? That way we could drop some of the patches
> needed in vcpkg.

Once we have handled the remaining issues we can surely make a patch
release.

Cheers,
 Thomas

> Cheers,
> Michał.
> 
> On Tue, Nov 28, 2017 at 5:36 PM, Michał Janiszewski
> <janisozaur%gmail.com@localhost> wrote:
> > On Tue, Nov 28, 2017 at 5:19 PM, Thomas Klausner <tk%giga.or.at@localhost> 
> > wrote:
> >>
> >> On Fri, Nov 24, 2017 at 12:30:02PM +0100, Michał Janiszewski wrote:
> >> > Please consider applying this patch which allows building for UWP.
> >>
> >> Is this the complete patch, i.e. I can forget about the one that moved
> >> around zipint.h etc?
> >
> > Yes, the previous patch was prepared for libzip 1.2.0, the
> > then-current vcpkg version.
> > Since then I have updated vcpkg to 1.3.2 and based my work on that.
> >
> >> > For your convenience, the patch is also available at
> >> > https://hastebin.com/diwavuqubo.diff
> >> > This only addresses building the library itself and won't support
> >> > encryption, as seeding entropy is turned off, until someone who knows the
> >> > platform better can implement it.
> >>
> >> Hm, that would be nice to have before the merge.
> >
> > While I agree, I will leave this exercise to someone who will know
> > where to seed the entropy from.
> >
> >> > +if(CMAKE_SYSTEM_NAME MATCHES WindowsPhone OR CMAKE_SYSTEM_NAME
> >> > MATCHES WindowsStore)
> >> > +  ADD_DEFINITIONS(-DWINRT)
> >>
> >> What does WINRT stand for?
> >
> > I believe it stands for Windows RunTime, the limited API that used to
> > form the base of "Windows on ARM" when that previously tried to be a
> > thing.
> > This acronym is still in use across some products and is once again
> > being used for "Windows on ARM (v2)"
> >
> >> > diff --git a/lib/zip_source_win32a.c b/lib/zip_source_win32a.c
> >> > index e343a70..625784f 100644
> >> > --- a/lib/zip_source_win32a.c
> >> > +++ b/lib/zip_source_win32a.c
> >> > @@ -31,7 +31,7 @@ OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
> >> > SOFTWARE, EVEN
> >> >  IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >> >  */
> >> >
> >> > -
> >> > +#ifndef WINRT
> >> >  #include <stdio.h>
> >> >
> >> >  #include "zipint.h"
> >> > @@ -122,3 +122,5 @@ _win32_remove_a(const void *fname)
> >> >      DeleteFileA((const char *)fname);
> >> >      return 0;
> >> >  }
> >> > +
> >> > +#endif //WINRT
> >>
> >> Is there no ANSI interface on UWP?
> >>
> >> Instead of commenting out the whole file, it would be better to
> >> disable building it in lib/CMakeLists.txt.
> >
> > Ok, I can try amending the patch to exclude this file.
> >
> >> > diff --git a/lib/zip_source_win32handle.c b/lib/zip_source_win32handle.c
> >> > index 7fe003d..f7ac12e 100644
> >> > --- a/lib/zip_source_win32handle.c
> >> > +++ b/lib/zip_source_win32handle.c
> >> > @@ -420,12 +420,15 @@
> >> > _win32_create_temp_file(_zip_source_win32_read_file_t *ctx)
> >> >      int i;
> >> >      HANDLE th = INVALID_HANDLE_VALUE;
> >> >      void *temp = NULL;
> >> > -    SECURITY_INFORMATION si;
> >> > -    SECURITY_ATTRIBUTES sa;
> >> >      PSECURITY_DESCRIPTOR psd = NULL;
> >> >      PSECURITY_ATTRIBUTES psa = NULL;
> >> > +
> >> > +
> >> > +#ifndef WINRT
> >> >      DWORD len;
> >> >      BOOL success;
> >> > +    SECURITY_ATTRIBUTES sa;
> >> > +    SECURITY_INFORMATION si;
> >> >
> >> >      /*
> >> >      Read the DACL from the original file, so we can copy it to the temp 
> >> > file.
> >> > @@ -452,6 +455,10 @@ 
> >> > _win32_create_temp_file(_zip_source_win32_read_file_t *ctx)
> >> >      }
> >> >
> >> >      value = GetTickCount();
> >> > +#else
> >> > +    value = (zip_uint32_t)GetTickCount64();
> >> > +#endif
> >> > +
> >> >      for (i = 0; i < 1024 && th == INVALID_HANDLE_VALUE; i++) {
> >> >       th = ctx->ops->op_create_temp(ctx, &temp, value + i, psa);
> >> >       if (th == INVALID_HANDLE_VALUE && GetLastError() != 
> >> > ERROR_FILE_EXISTS)
> >>
> >> This code is just a workaround anyway... I can imagine that there is a
> >> better interface for temporary files on UWP -- do you know one?
> >
> > Sorry, I don't know. Files are generally frowned upon, based on how
> > convoluted the UWP API is.
> > All the UWP examples try forcing some odd and proprietary C++
> > extension on users.
> >
> >> > diff --git a/lib/zip_source_win32w.c b/lib/zip_source_win32w.c
> >> > index d8b8f86..092c044 100644
> >> > --- a/lib/zip_source_win32w.c
> >> > +++ b/lib/zip_source_win32w.c
> >> > @@ -83,7 +83,19 @@ _win32_strdup_w(const void *str)
> >> >  static HANDLE
> >> >  _win32_open_w(_zip_source_win32_read_file_t *ctx)
> >> >  {
> >> > +#ifdef WINRT
> >> > +    CREATEFILE2_EXTENDED_PARAMETERS extParams = { 0 };
> >> > +    extParams.dwFileAttributes = FILE_ATTRIBUTE_NORMAL;
> >> > +    extParams.dwFileFlags = FILE_FLAG_RANDOM_ACCESS;
> >> > +    extParams.dwSecurityQosFlags = SECURITY_ANONYMOUS;
> >> > +    extParams.dwSize = sizeof(extParams);
> >> > +    extParams.hTemplateFile = NULL;
> >> > +    extParams.lpSecurityAttributes = NULL;
> >> > +
> >> > +    return CreateFile2(ctx->fname, GENERIC_READ, FILE_SHARE_READ |
> >> > FILE_SHARE_WRITE, OPEN_EXISTING, &extParams);
> >> > +#else
> >> >      return CreateFileW(ctx->fname, GENERIC_READ, FILE_SHARE_READ |
> >> > FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> >> > +#endif
> >> >  }
> >> >
> >> >
> >> > @@ -103,7 +115,19 @@
> >> > _win32_create_temp_w(_zip_source_win32_read_file_t *ctx, void **temp,
> >> > zip_uint32
> >> >       return INVALID_HANDLE_VALUE;
> >> >      }
> >> >
> >> > +#ifdef WINRT
> >> > +    CREATEFILE2_EXTENDED_PARAMETERS extParams = { 0 };
> >> > +    extParams.dwFileAttributes = FILE_ATTRIBUTE_NORMAL |
> >> > FILE_ATTRIBUTE_TEMPORARY;
> >> > +    extParams.dwFileFlags = FILE_FLAG_RANDOM_ACCESS;
> >> > +    extParams.dwSecurityQosFlags = SECURITY_ANONYMOUS;
> >> > +    extParams.dwSize = sizeof(extParams);
> >> > +    extParams.hTemplateFile = NULL;
> >> > +    extParams.lpSecurityAttributes = NULL;
> >> > +
> >> > +    return CreateFile2((const wchar_t *)*temp, GENERIC_READ |
> >> > GENERIC_WRITE, FILE_SHARE_READ, CREATE_NEW, &extParams);
> >> > +#else
> >> >      return CreateFileW((const wchar_t *)*temp, GENERIC_READ |
> >> > GENERIC_WRITE, FILE_SHARE_READ, sa, CREATE_NEW, FILE_ATTRIBUTE_NORMAL
> >> > | FILE_ATTRIBUTE_TEMPORARY, NULL);
> >> > +#endif
> >> >  }
> >>
> >> The file is so short I wonder if a separate zip_source_win32uwp.c (or
> >> so) instead would be better.
> >
> > I'll see what I can do about it.
> >
> >> > diff --git a/lib/zipwin32.h b/lib/zipwin32.h
> >> > index 60f8d0c..701a34c 100644
> >> > --- a/lib/zipwin32.h
> >> > +++ b/lib/zipwin32.h
> >> > @@ -35,7 +35,10 @@
> >> >  */
> >> >
> >> >  /* 0x0501 => Windows XP; needs to be at least this value because of
> >> > GetFileSizeEx */
> >> > +#ifndef WINRT
> >> >  #define _WIN32_WINNT 0x0501
> >> > +#endif
> >> > +
> >> >  #include <windows.h>
> >> >
> >> >  /* context for Win32 source */
> >>
> >> Why is that needed?
> >
> > I can only imagine it clashes with some UWP defines.
> >
> >> Cheers,
> >>  Thomas
> >
> > One final note - would you accept this change to be submitted as a
> > pull request to https://github.com/nih-at/libzip ? The mail review
> > process is very high maintenance for me. You can still download raw
> > patches from github that you can later apply in your repo.
> >
> > Cheers,
> > Michał.
> >
> >
> > --
> > Michal Janiszewski
> 
> 
> 
> -- 
> Michal Janiszewski
> 

Made by MHonArc.