List Archive
Thread
- Re: UWP builds, (continued)
-
Re: UWP builds,
Thomas Klausner
(2017/11/28 16:19:02)
- Message not available
- Re: UWP builds, Michał Janiszewski (2017/11/30 22:49:14)
- Re: UWP builds, Thomas Klausner (2017/12/04 10:18:10)
- Re: UWP builds, Michał Janiszewski (2017/12/04 10:24:47)
- Re: UWP builds, Michał Janiszewski (2017/12/04 16:33:56)
-
Re: UWP builds,
Thomas Klausner
(2017/11/28 16:19:02)
Message
On Mon, Dec 4, 2017 at 11:18 AM, Thomas Klausner <tk%giga.or.at@localhost> wrote: > 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. Are you sure you have posted the review? I see no comments yet. >> 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.) Sure, I'll work on getting vcpkg proof. I'll let you know when I will have done it. >> 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. Sounds great, thanks. > 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 >> -- Michal Janiszewski
Made by MHonArc.