New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic support for Universal Windows Platform apps #820

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@m-deckel
Contributor

m-deckel commented May 20, 2016

Basic support for Universal Windows Platform apps

@bagder

This comment has been minimized.

Member

bagder commented May 20, 2016

The build failure shows the code uses too long source code lines, which is against our code style guidelines.

@bagder bagder added the build label May 20, 2016

@m-deckel

This comment has been minimized.

Contributor

m-deckel commented May 20, 2016

Sorry for the inconvenience, I just fouond the code style check tool. Pushed the fix. :)

@gvanem

This comment has been minimized.

Member

gvanem commented May 20, 2016

I just tried your patches and they kinda works.

But I have little experience with UWP (Universal Windows Platform Apps).
Which _UWP_s is this for? Phone-Apps, Desktop-Apps, or both?

IMHO your patches for UWP should check the rather limited Win-API there. E.g. these functions seems not present:

LoadLibrary
GetStdHandle
PeekNamedPipe

which are used in telnet.c. Similar problems for USE_WINDOWS_SSPI and USE_SCHANNEL.

@m-deckel

This comment has been minimized.

Contributor

m-deckel commented May 20, 2016

It's the new Windows 10 apps. The are universal and only have to be
compiled for ARM and x86 but will run on both phones, desktops, tablets,
etc etc.

As for now it's not a complete patch for all modules. For my configuration
it works but I will probably have a look and try to fix it for all
configurations in the future.

Best
Am 20.05.2016 11:49 vorm. schrieb "Gisle Vanem" notifications@github.com:

I just tried your patches and they kinda works.

But I have little experience with UWA (Windows Universal Platform Apps).
Which UWAs is this for? Phone-Apps, Desktop App, or both?

IMHO your patches for UWA should check the rather limited Win-API there.
E.g. these functions seems not present:

LoadLibrary
GetStdHandle
PeekNamedPipe

which are used in telnet.c. Similar problems for WIN_USE_SSPI and
WIN_SCHANNEL.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#820 (comment)

@MarcelRaad

This comment has been minimized.

Member

MarcelRaad commented May 20, 2016

Works for me with CURL_DISABLE_NTLM, CURL_DISABLE_TELNET, CURL_DISABLE_LDAP, and CURL_DISABLE_CRYPTO_AUTH.
@m-deckel If you change md5.c line 127 from #elif defined(_WIN32) to #elif defined(_WIN32) && !defined(CURL_WINDOWS_APP), CURL_DISABLE_CRYPTO_AUTH is not necessary.

@m-deckel

This comment has been minimized.

Contributor

m-deckel commented May 31, 2016

Thanks @MarcelRaad

@bagder

This comment has been minimized.

Member

bagder commented Jun 27, 2016

I'd like a +1 from another windows person on this PR before I'd merge it.

@m-deckel

This comment has been minimized.

Contributor

m-deckel commented Jun 27, 2016

That's fine, as it works for me, I'm not depending on a quick release of
this patch. I hope we can find some more WinRT developers.

Mit freundlichen Grüßen,
Marco Deckel
Am 27.06.2016 5:22 nachm. schrieb "Daniel Stenberg" <
notifications@github.com>:

I'd like a +1 from another windows person on this PR before I'd merge it.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#820 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/APiTiN6d9qvZPBqFQgnXZfE4ZHJ3ibeJks5qP-rQgaJpZM4Ii_th
.

@gvanem

This comment has been minimized.

Member

gvanem commented Jul 12, 2016

Just a comment on OpenSSL regarding this pull request. AFAICS, all code in an UWP application should be written in such Universal Windows style, no?

Just a FYI, Microsoft has forked the OpenSSL repo and seems to have made OpenSSL UWP compatible. I've not tested it. But it's here. And more specifically, the UWP change is here.

@captain-caveman2k

This comment has been minimized.

Member

captain-caveman2k commented Aug 20, 2016

@bagder The changes seem relatively simple and it would be good to add UWP support so a +1 from me.

@bagder bagder closed this in 7f3df80 Aug 21, 2016

@bagder

This comment has been minimized.

Member

bagder commented Aug 21, 2016

Thanks!

So will you continue this effort to make it more than "basic support" ? Is there something to document for future users who are interested using this or wanting to help expanding the support?

@m-deckel

This comment has been minimized.

Contributor

m-deckel commented Aug 21, 2016

@bagder You're welcome. I'm sorry to say though that I am currently not finding time to extend this. I had to do the fix for my employer and thought it would be beneficial to contribute at least the first steps. Generally there is not much change needed besides fixing the usage of prohibited APIs. OpenSSL indeed has a Microsoft fork and I am already using it successfully together with curl.
Best regards!

@joycepg

This comment has been minimized.

joycepg commented Oct 1, 2016

@m-deckel @MarcelRaad how did you build this?
I have tried to build with these changes as merged into the curl master branch. The command I ran was
nmake /f Makefile.vc mode=static debug=no machine=x86 rtlibcfg=dll enable_winssl=no enable_sspi=no enable_ipv6=yes
Out of the box, this build does not pick up the changes, since _WIN32_WINNT is defined but empty, so CURL_WINDOWS_APP never gets defined in curl_setup.h

By manually hacking the winbuild/MakefileBuild.vc, adding at line 67
CFLAGS = $(CFLAGS) /DCURL_DISABLE_NTLM /DCURL_DISABLE_TELNET /DCURL_DISABLE_LDAP /DCURL_WINDOWS_APP
I was able to get further. However, it still borks on linkinf my UWA with
1>libcurl_a.lib(system_win32.obj) : error LNK2001: unresolved external symbol __imp__VerSetConditionMask@16
1>libcurl_a.lib(system_win32.obj) : error LNK2001: unresolved external symbol __imp__VerifyVersionInfoA@16
This is because the Curl_verify_windows_version() function has two implementations with pre-processor choosing one, but neither are valid code for UWP. GetVersionEx is old/deprecated/desktop only, and VerifyVersionInfo is desktop only.

Before I hack, am I doing something wrong?

@MarcelRaad

This comment has been minimized.

Member

MarcelRaad commented Oct 1, 2016

@joycepg You're right, this patch broke with commit 332e8d6 (curl version 7.50.0) and then the change in connect.c was merged to the wrong place. I'm successfully using this patch on top of libcurl 7.49.1 at home. I'll create a pull request to fix the merge error, after which I can successfully build 7.50.3 again.

Note that you should always set _WIN32_WINNT explicitly (to 0x0A00 for Windows 10). Otherwise, libcurl's config-win32.h will choose Windows Vista for you. You don't have to define CURL_WINDOWS_APP yourself.

MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Oct 1, 2016

win: fix Universal Windows Platform build
This fixes a merge error in commit 7f3df80 caused by commit 332e8d6.

Additionally, this disables Curl_verify_windows_version for Windows App
builds. There seems to be no way to determine the Windows version from
a UWP app. Neither GetVersion(Ex), nor VerifyVersionInfo, nor the
Version Helper functions are supported.

Bug: curl#820 (comment)
Reported-by: Paul Joyce

MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Oct 2, 2016

win: fix Universal Windows Platform build
This fixes a merge error in commit 7f3df80 caused by commit 332e8d6.

Additionally, this changes Curl_verify_windows_version for Windows App
builds to assume to always be running on the target Windows version.
There seems to be no way to determine the Windows version from a
UWP app. Neither GetVersion(Ex), nor VerifyVersionInfo, nor the
Version Helper functions are supported.

Bug: curl#820 (comment)
Reported-by: Paul Joyce

MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Oct 16, 2016

win: fix Universal Windows Platform build
This fixes a merge error in commit 7f3df80 caused by commit 332e8d6.

Additionally, this changes Curl_verify_windows_version for Windows App
builds to assume to always be running on the target Windows version.
There seems to be no way to determine the Windows version from a
UWP app. Neither GetVersion(Ex), nor VerifyVersionInfo, nor the
Version Helper functions are supported.

Bug: curl#820 (comment)
Reported-by: Paul Joyce

MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Oct 16, 2016

win: fix Universal Windows Platform build
This fixes a merge error in commit 7f3df80 caused by commit 332e8d6.

Additionally, this changes Curl_verify_windows_version for Windows App
builds to assume to always be running on the target Windows version.
There seems to be no way to determine the Windows version from a
UWP app. Neither GetVersion(Ex), nor VerifyVersionInfo, nor the
Version Helper functions are supported.

Bug: curl#820 (comment)
Reported-by: Paul Joyce

MarcelRaad added a commit that referenced this pull request Oct 16, 2016

win: fix Universal Windows Platform build
This fixes a merge error in commit 7f3df80 caused by commit 332e8d6.

Additionally, this changes Curl_verify_windows_version for Windows App
builds to assume to always be running on the target Windows version.
There seems to be no way to determine the Windows version from a
UWP app. Neither GetVersion(Ex), nor VerifyVersionInfo, nor the
Version Helper functions are supported.

Bug: #820 (comment)
Reported-by: Paul Joyce

Closes #1048
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment