-
Notifications
You must be signed in to change notification settings - Fork 714
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
Support building this plugin in Visual Studio [2017 RC] out of the box #580
Comments
I have a couple questions:
In case we cannot get rid of the Thanks! |
Unfortunately this wouldn't work - and even if it worked it would have a significan drawback - one app compiled from two different versions of VS would have different binaries because of the different compilers, which IMO is wrong.
I'd say that it the user have cordova-windows <4.4.1 then he must have VS 2015 (with toolset v140) or less because these versions of cordova-windows are missing some critical fixes, required to work with VS15. |
Thanks @vladimir-kotikov. I will probably make a new "vsnext" version next week to support testing with VS15. It will not support PhoneGap Build. (I can support PhoneGap Build based on the legacy version if you need it for any reason.) I also do not expect to publish it on nom unless you need it for some reason. As I write this I wonder if there could be a way to make this |
Hey @brodybits. I'm the dev lead for the Cordova tools in Visual Studio. I think it would help if I gave you context. Can you email me at mbraude@microsoft.com so we can set up a time to talk? Thanks. |
Hey @vladimir-kotikov @mbraude I tried installing VS 15 preview 5 and using it to build and run https://github.com/brodybits/Cordova-sqlite-bootstrap-test with and without applying the first proposed fix (replace Unfortunately I cannot tell whether the problem lies within this plugin, VS15 preview, or both. I wish there were a sample Cordova C++ UWP plugin that can be supported without the SQLite3 part. I would be happy to submit a sample if necessary. |
My bad. If I build and run the Cordova-sqlite-bootstrap-test in VS 15 preview 5 with the plugins installed properly it works fine. I think the problem now is that this plugin will not build if someone has only VS 15 installed. Can you confirm that this is correct? |
@vladimir-kotikov I took the liberty to reach out to you in phonegap/build#562 (comment) in case you can help us with PhoneGap Build. This is a blocking issue for an existing customer. @vladimir-kotikov @mbraude I would like to propose the following to prepare for the upcoming VS 15 release:
I think this should solve the problem with VS 15 previews while keeping VS 2015 users up to date. Please report if this sounds OK or not, thanks! |
@brodybits, I will take a look at that phonegap-build problem |
The migration plan looks good! Just a couple of questions/suggestions:
@brodybits, @mbraude, what do you think? I will be happy to help with any issues related to this migration and could send a PR with all necessary changes. |
Yeah I think this should be considered a breaking change. For suggestion 1, I can think of the following alternatives for handling plugin updates with the proposed
For suggestion 2: I must admit I have not used dist-tag(s) before and just read a nice article at http://jbavari.github.io/blog/2015/10/16/using-npm-tags/. This might be good in cases where a someone uses an updated version of Cordova CLI with VS 2015. I will probably need a little time to decide which would work best for me. I will keep you posted and let you know if I need any help with the changes needed. I wish there was a way to configure the
Thanks for your efforts here. |
Yeah, this might be a good idea to reduce maintenance cost once 2.x became stable
This is very close to what I proposed - you can have 1.4.x and 2.x branches with the same
Yeah, I agree, It would be helpful addition, especially looking at the android platform which already has a way to provide custom arguments to build system ( |
@brodybits this plan sounds good to me. Thanks! |
Progress and questions so farMy goals
ProgressVS 15 preview 5I was able to replace VS 2017 RCI then tried replacing VS 15 preview 5 with VS 2017 RC. During the installation of VS 2017 RC I selected the Universal Windows Platform development and Desktop development with C++ Windows "Workloads". I did not select .NET desktop development since I did not think it would be relevant here. When I tried to open a newly generated project with the modified plugin in VS RC I got a message that some Universal Windows component was missing. I went through some dialogs to deal with the missing component then stopped it. (I discovered with preview 5 that it was much better to reopen the installer GUI and install the missing components from there.) Looking through the installer GUI I found a "Visual C++ runtime for UWP" component that was not already selected. I tried selecting this component and installing it. Then I made another newly generated project with the modified plugin, tried opening it with VS RC and got the same missing component dialog again. At this point I tried generating a project with no sqlite plugin and it worked on both desktop and mobile device. Ideas for next stepsSome other ideas I can think of to try installing:
NOTE: If I click the Mobile development with C++ workload it does not seem to automatically select "Visual C++ compilers and libraries for ARM". Questions@vladimir-kotikov can you give me some guidance which components I should try installing to get the build working on the desktop? Is there any way to get this working without the full .NET desktop development or Mobile development with C++ workloads? I suspect that I will have to explicitly add the "Visual C++ compilers and libraries for ARM" to get it working on my ARM mobile device. Does this sound right? Thanks! |
@brodybits, sadly I haven't tried VS 2017 RC yet, going to install it now and test your changes. I will let you know once i have some results. |
Thanks @vladimir-kotikov. In principle my change was just to replace |
Okay, I have just installed VS 2017 with "Universal Windows Platform development" workload, and two optional components for this workload - "C++ UWP Support" and "Windows 10 SDK (10.0.10586.0)" and tried the following:
Build succeeded and I can see built packages in So in theory you wouldn't need to install workloads other than "Universal Windows Platform development" with some additional components. If you're still having build issues, could you share an error output from 'cordova build windows' Upd. I'm also going ot install "Mobile development with Javascript" workload to test how this works in TACO |
Thanks @vladimir-kotikov. I will try this early next week and report. |
Trying without Setting:
Build command:
Build output:
|
Hmm.. I think this is because you're setting |
I got it running on desktop (x86) and mobile ARM with some trial and error and expect to release the "edge" trial within the next 1-2 days or so. The following commands worked for me (desktop build, got it running with the test suite): set VSINSTALLDIR=c:\Program Files (x86)\Microsoft Visual Studio\2017\Community
cordova build windows -- --release --archs="x86 x64" --appx uap # probably not needed
cordova run windows -- --release --archs="x86" --appx uap Cordova did not seem to see my When I tried the following command:
I got the following output (x86 part omitted):
I tried adding the "Visual C++ compilers and libraries for ARM" individual component in the installer but it didn't seem to help. Then I tried on a clean project with the test suite (with the modified plugin and windows platform installed):
and it installed on my mobile. I had to start it manually. |
I just published cordova-sqlite-storage@2.0.0-edge01 with |
Hey, Chris. Sorry for annnoyance but this still needs some tweaking to work as it expected to be. Specifying exact version (or dist tag) for
So i'd like to ask you to add this to package.json "engines": {
"cordovaDependencies": {
"2.0.0": { "cordova-windows": ">4.4.1" }
}
}, to both 1.5.x and 2.x branches, so cordova would be able to fetch proper version based on installed cordova-windows. This will also require to republish both 1.5x (will became 1.5.1) and 2.0.0-edge1 (will became just 2.0.0) |
I will fix this within the next 1-2 days or so. Also do you have any suggestions that can help me with the problems in brodycj/cordova-sqlite-legacy#10 (Windows 10 SQLite3 C++ library on PhoneGap Build)? |
Thanks. Chris! |
@vladimir-kotikov if I publish newer versions such as 2.0.1, 2.1.0, ... would I have to update the cordovaDependencies part as well? |
No, you wouldn't. Having |
Should be |
Hmm... 4.4.1 had a bug that was preventing it to work w/ VS 15/2017 properly, which has been fixed in 4.4.2 (see CB-11458 in [RELEASENOTES](https://github.com/apache/cordova-windows/blob/master/RELEASENOTES.md#442-jul-25-2016) ), so I'd better stick to 4.4.2
|
@vladimir-kotikov I just published 1.5.1 and 2.0.0 with the tweaks you requested. I asked a couple more questions on brodycj/cordova-sqlite-legacy#10. Can you or someone else from cordova-windows give me a quick answer? |
Thanks Chris, I just verified it works perfectly, e.g. for windows@4.3.2:
|
Thanks for the confirmation! |
When installing the latest version of Visual Studio 2017 RC: in addition to the big Universal Windows Platform development item I had to check "C++ Universal Windows Platform tools" on the right hand side. I had also checked the big Desktop development with C++ item but unchecked it since I do not think it is necessary for this plugin. |
From the original description:
I will probably drop support for VS 2015 from the next release, and direct VS 2015 users to Cordova-sqlite-legacy-build-support. Please give a shout ASAP if this will be a problem, thanks. |
Support for Visual Studio 2017 is now part of the default |
@brodybits @vladimir-kotikov Everything works just fine! +1 |
Something strange is that if I would install Visual Studio 2015 Update 3 on a new system it seems to include a version of Windows 10 SDK with platform toolset v141 by default. (I had to install some other components to get it to work with cordova-sqlite-legacy which supports old platform toolset v140.) But if I would try running a project with this plugin on the Visual Studio 2015 Update 3 installation it would still complain that I need to install a certain Windows 10 SDK component version. It would be very interesting if VS 2015 (Update 3) would actually work with this plugin if I would install the needed Windows 10 SDK component version. Another side note is that in brodycj/cordova-sqlite-ext#60 (comment) @tuanbs contributed the following pointer to a workaround for using the old platform toolset v140 on VS 2017, which I still have not tried myself: https://developercommunity.visualstudio.com/content/problem/48806/cant-find-v140-in-visual-studio-2017.html |
Visual Studio '15' Preview is now publicitly available and we'd like to make sure that building apps using this plugin will work with VS15 just out of the box. However as of now the plugin requires VS 2015 to be installed as it specifies
<PlatformToolset>v140</PlatformToolset>
in SQLite3.UWP.vcxproj and will not build from VS15 which ships with toolset v141So, to support VS15 I propose to:
"cordova-windows": ">=4.4.1"
inpackage.json
(see more about plugin requirements here: Specifying Cordova dependencies).The support for build with VS 2015 then might be moved to Cordova-sqlite-legacy-build-support (just as in #579) so users would still be able to build the plugin with previous version of VS.
The text was updated successfully, but these errors were encountered: