vincelwt / harmony

:musical_note: Sleek music player for Spotify, SoundCloud, Google Play Music and your local files
http://getharmony.xyz
859 stars 59 forks source link

Rebuild native module (ll-keyboard-hook-win) on install #99

Closed elizabwth closed 8 years ago

elizabwth commented 8 years ago

My fix in version 0.4.2 for Windows keyboard shortcuts didn't work correctly as the build system did not run an electron-rebuild. This PR fixes problem with ll-keyboard-hook-win by rebuilding the module on dev installs and when distributing. I tested building on my Windows 10 desktop extensively and it works well. Hopefully this fixes my previous pull request's issue. (#97)

referenced from: https://github.com/todbot/electron-hid-toy/blob/master/package.json

elizabwth commented 8 years ago

I have dist copies of Harmony if you'd like to test them yourself, @vincelwt . I'm also working on a keyboard rebinding system. It may be worth looking at GPMDP's solution for Linux key bindings.

vincelwt commented 8 years ago

So I just tried on a Windows 10 64bit laptop and it doesn't works :/ Module is still not found after rebuild. (tried with harmony-media-keys-windows-build-errors branch)

See the log failed-log.txt

There seems to be problems on AppVeyor too: https://ci.appveyor.com/project/vincelwt/harmony/build/1.0.82

You can send me dist copies already compiled to see if it works here.

elizabwth commented 8 years ago

This is probably caused because I am using windows-build-tools to build ll-keyboard-hook-win. I'd try adding that to the dev dependencies and see if it passes.

Here is my compiled copy (64bit) from my machine. https://puu.sh/s8lss/e8b51d71a1.zip

elizabwth commented 8 years ago

That seems to the issue here. I am able to build Harmony with windows-build-tools installed via Power Shell before attempting to build ll-keyboard-hook-win and run electron-rebuild.

I have windows-build-tools installed globally in my environment. $ npm install --global windows-build-tools

vincelwt commented 8 years ago

I'll try with that, and add it to AppVeyor if it works, thanks.

vincelwt commented 8 years ago

Still no luck after installing windows-build-tools to my env :/ failed-log.txt

elizabwth commented 8 years ago

That's strange. I've been able to successfully install on two separate machines. My desktop and my new laptop (which had a clean install of Windows 10 x64). It also worked for the user in #98.

elizabwth commented 8 years ago

I'll give it a shot on building it on AppVeyor and send you the results.

elizabwth commented 8 years ago

I was able to install on AppVeyor without a hitch.

appveyor.yml:

environment:
  matrix:
    - nodejs_version: "6"

platform:
  - x86
  - x64

install:
  - ps: Install-Product node $env:nodejs_version
  - ps: npm install --global windows-build-tools
  - npm install

test_script:
  - node --version
  - npm --version

build: off

AppVeyor PowerShell output of ll-keyboard-hook-win:

> ll-keyboard-hook-win@3.0.0 install C:\projects\harmony\node_modules\ll-keyboard-hook-win
> node-gyp rebuild
C:\projects\harmony\node_modules\ll-keyboard-hook-win>if not defined npm_config_node_gyp (node "C:\Program Files (x86)\nodejs\node_modules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild )  else (node "" rebuild ) 
Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.
  ll_keyboard_hooks.cc
  win_delay_load_hook.cc
..\ll_keyboard_hooks.cc(7): warning C4005: '_WIN32_WINNT': macro redefinition [C:\projects\harmony\node_modules\ll-keyboard-hook-win\build\ll_keyboard_hooks.vcxproj]
  c:\users\appveyor\.node-gyp\6.9.1\include\node\uv-win.h(23): note: see previous definition of '_WIN32_WINNT' (compiling source file ..\ll_keyboard_hooks.cc)
..\ll_keyboard_hooks.cc(79): warning C4101: 'req': unreferenced local variable [C:\projects\harmony\node_modules\ll-keyboard-hook-win\build\ll_keyboard_hooks.vcxproj]
C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\xstring(2195): warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc (compiling source file ..\ll_keyboard_hooks.cc) [C:\projects\harmony\node_modules\ll-keyboard-hook-win\build\ll_keyboard_hooks.vcxproj]
  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\xstring(2182): note: while compiling class template member function 'void std::basic_string<char32_t,std::char_traits<char32_t>,std::allocator<char32_t>>::_Copy(unsigned int,unsigned int)' (compiling source file ..\ll_keyboard_hooks.cc)
  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\xstring(2228): note: see reference to function template instantiation 'void std::basic_string<char32_t,std::char_traits<char32_t>,std::allocator<char32_t>>::_Copy(unsigned int,unsigned int)' being compiled (compiling source file ..\ll_keyboard_hooks.cc)
  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\string(689): note: see reference to class template instantiation 'std::basic_string<char32_t,std::char_traits<char32_t>,std::allocator<char32_t>>' being compiled (compiling source file ..\ll_keyboard_hooks.cc)
C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\iosfwd(343): warning C4577: 'noexcept' used with no exception handling mode specified; termination on exception is not guaranteed. Specify /EHsc (compiling source file ..\ll_keyboard_hooks.cc) [C:\projects\harmony\node_modules\ll-keyboard-hook-win\build\ll_keyboard_hooks.vcxproj]
     Creating library C:\projects\harmony\node_modules\ll-keyboard-hook-win\build\Release\ll_keyboard_hooks.lib and object C:\projects\harmony\node_modules\ll-keyboard-hook-win\build\Release\ll_keyboard_hooks.exp
  Generating code
  Finished generating code
  ll_keyboard_hooks.vcxproj -> C:\projects\harmony\node_modules\ll-keyboard-hook-win\build\Release\\ll_keyboard_hooks.node
  ll_keyboard_hooks.vcxproj -> C:\projects\harmony\node_modules\ll-keyboard-hook-win\build\Release\ll_keyboard_hooks.pdb (Full PDB)
vincelwt commented 8 years ago

Great! I'm adding it to Appveyor.