Skip to content
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

Initial Update of Windows Packaging #438

Merged
12 commits merged into from
Aug 28, 2017
Merged

Initial Update of Windows Packaging #438

12 commits merged into from
Aug 28, 2017

Conversation

arsenetar
Copy link
Owner

@arsenetar arsenetar commented Aug 23, 2017

I tested the generated installers and executable files on a couple windows 10 machines and they seem to work fine. There are probably a few areas for improvement, namely:

  • Installing both x86 and x64 versions is not completely supported as the installer script is written right now, I think I know how to get this working without being overly complicated.
  • path to makensis is currently hard-coded in package.py, adding the ability to pass the path would be better

Right now I think as long as the program itself works and the packaging works well it would probably be good to get it out to see if there are additional issues.

Ref #393

arsenetar added 11 commits July 21, 2017 21:41
- Update run.py to execute on windows as SIGQUIT is not available.
- Update .gitignore to ignore the generate .pyd files
Ref #300, #393
Add package_windows back into package.py
- Using cx_freeze for freezing installation
- Will be using nsis for actual installer
Tested with python 3.5 64bit on windows 10
Ref #393
- Update the makefile to support windows
- Use different bin path in virtualenv
- Use pyd instead of so files
- Tested with Msys2
- Add *.exe to .gitignore
- Fix minor format error in package.py
Ref #393
Add the requirements-windows.txt
- contains cx-Freeze for bundling
Ref #393
Initial Version of a NSIS installer script
- Multi-user install (install for just one or all)
- Registers uninstaller (more values need to finish up)
- Tested both single and all install / uninstall and works
- Still need to add parameters instead of hardcoded values in some spots
- Need to clean up vendor folders / keys if empty on uninstall
- Need to add the other dupeGuru languages to the language list
- Minor cleanup of script needed as well
Ref #393
Updates to setup.nsi including:
- Defines from CLI
- Version information (MAJOR, MINOR, PATCH)
- Bits (64 / 32)
- SourcePath (if we wanted something other than build)
- Added extra defines to move application specifics to one location
- Added extra defines for uninstall information
- Added calculation of install size
- Added switching between 64 and 32 bit contexts (need to verify
functionality)
- Updated output file naming
- Added NSIS supported languages which are also supported by dupeGuru
- Added rest of registry keys for uninstall information
- Added missing registry key for installType
- Added removeing Vendor folder and registry key if empty on uninstall

Should be very close to having this installer script ready to integrate
into the package.py script if desired.
Ref #393
Minor update to README to indicate windows is supported.  Add PyQt5 to
requirements-windows.txt to make installation easier.
- Update package.py to integrate NSIS for windows
- Update makefile to deal with a few additional windows issues
- Add Windows.md to contain specific windows instructions, if we want
this can be merged with README.md
- Minor formatting update to setup.nsi
Ref #393
- Update the README to include a reference to the Windows instructions.
-  Add some additional notes into Windows Instructions and remove one
incorrect command.
- Update .gitignore to ignore all permutations of env* to allow for
multiple side by side virtual environments (used to build different
versions for windows)
Ref:  #393
Fix broken python link and move nsis link for consistency.
# Conflicts:
#	Makefile
- Update to use makefile changes from hsoft/master
- Remove unneeded PIP_OPTIONS
@ghost ghost self-assigned this Aug 24, 2017
@ghost
Copy link

ghost commented Aug 24, 2017

@arsenetar Thanks! I'll set myself up with a windows 7 VM and try it. As for x86 and x64, you mean that we would need to supply one package per arch? I'm fine with that.

I hope to find the time to do that before next week.

@arsenetar
Copy link
Owner Author

@hsoft You would need one package per arch, but what I had meant was that if you installed both x86 and x64 versions they would conflict over some registry keys and if installed for just one user the default installation directory. I need to look over my plans for negating this to make sure they make sense before I implement them. The solution is basically to use different registry keys (for installation information), different default install location when not installed machine wide, and a different uninstall entry. I am not sure when I will get to it and wanted to keep this moving forward as the application is working and most people probably do not install both versions on the same machine.

@ghost
Copy link

ghost commented Aug 25, 2017

What do you use to provide make to your windows environment? I know that there's tons of programs like cygwin, msys32 and others but I'm not sure of which is needed to build something that works. Could this be added to the Dependencies section?

@arsenetar
Copy link
Owner Author

I was using msys2 when using make msys and other versions should work as well but I did not test them. I did mention it in the Windows.md file. However, I found it easier to just forego using make and run the respective commands instead.

@ghost
Copy link

ghost commented Aug 25, 2017

Oops, I did not even read the whole thing. I only saw make and the lack of prerequisite that provide it. Sorry.

@ghost
Copy link

ghost commented Aug 26, 2017

I get an error during build:

Building PE Modules
running build_ext
building '_block' extension
creating build\temp.win-amd64-3.6
creating build\temp.win-amd64-3.6\Release
creating build\temp.win-amd64-3.6\Release\core
creating build\temp.win-amd64-3.6\Release\core\pe
creating build\temp.win-amd64-3.6\Release\core\pe\modules
C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\x86_amd64\cl.exe /c /
nologo /Ox /W3 /GL /DNDEBUG /MD -IC:\Users\win7\src\dupeguru\env\include -IC:\Us
ers\win7\AppData\Local\Programs\Python\Python36\include -IC:\Users\win7\AppData\
Local\Programs\Python\Python36\include "-IC:\Program Files (x86)\Microsoft Visua
l Studio 14.0\VC\INCLUDE" "-IC:\Program Files (x86)\Windows Kits\8.1\include\sha
red" "-IC:\Program Files (x86)\Windows Kits\8.1\include\um" "-IC:\Program Files
(x86)\Windows Kits\8.1\include\winrt" /Tccore\pe\modules\block.c /Fobuild\temp.w
in-amd64-3.6\Release\core\pe\modules\block.obj
block.c
c:\users\win7\appdata\local\programs\python\python36\include\pyconfig.h(59): fat
al error C1083: Cannot open include file: 'io.h': No such file or directory
error: command 'C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\BIN\\
x86_amd64\\cl.exe' failed with exit status 2

I've installed Visual Studio 2015 build tools and python 3.6, amd64.

Also, shouldn't Visual Studio be listed as a dependency? Python is particularly picky about VS versions. There should be better instructions about which one to choose, where to download it, etc..

@arsenetar
Copy link
Owner Author

Yeah, I will get some updates on that, I have had that part setup for quite some time and had not thought about it. I was building with 3.5 as well on my machines, not sure if that ends up making a difference. I will look at that. It looks like they might be recommending Visual Studio 2017 build tools now, but I had used 2015 just fine. Since I have both the windows 10 sdk and windows 8.1 sdk it seems like my builds of those modules are using the windows 10 sdk.

@arsenetar
Copy link
Owner Author

Just tested 3.6 x64 bit and it seems to work fine on my system for the build again, I know we are not using exactly the same versions but from what I can tell either sdk should work. I also finally had the chance to run the installer and program on a windows 7 computer and it seems to be working fine there as well. It did have the Visual C++ Redistributables already installed, which I wonder if those would cause issues if absent even though the packaging bundles the dll.

Update Windows.md including:
- Information on compilier requirements for windows
- Notes about the windows 10 sdk
- Some clarification around some of the steps
- Addition of msys2 links

Going to review this a bit more to polish it up.

Ref #393.
@ghost
Copy link

ghost commented Aug 26, 2017

I haven't tried it again, but could it simply be that I ran from cmd.exe instead of the command line from VS ( https://stackoverflow.com/a/43098806 )? If yes, it might be worth mentioning in the docs.

@arsenetar
Copy link
Owner Author

Possible, mine works without needing that, I think python's setuptools should call the respective visual studio bits from my understanding. I've had something like that happen before outside of python but I don't remember the solution... I might have reinstalled the visual studio tools and sdks.

@ghost
Copy link

ghost commented Aug 28, 2017

I can't believe how long it is to install all updates on a win7 machine. And then comes the VS build tools which also takes forever!

@ghost
Copy link

ghost commented Aug 28, 2017

And now I have cryptic linking error. grrr! I give up. I already spent hours on this and I hate myself for having wasted this time in Windows.

Would you mind supplying builds yourself? There's still a trust issue due to the fact that I don't know you, but what the hell, I'll host it anyway with a "at your own risk" notice.

@arsenetar
Copy link
Owner Author

I have no problem supplying builds. Yeah, I had issues at work with getting windows 7 all setup for building software. Luckily windows 10 seems to have less issues for me, still doesn't beat Linux though.

@ghost
Copy link

ghost commented Aug 28, 2017

Thanks! Then I'd be ready to merge and wait for your builds for v4.0.3 (I think my email server will handle this size). Just a couple of questions:

  1. I was planning on eventually phasing out build.py entirely and only have a Makefile approach. I see that you consider the make approach an alternative rather than the main way to build. Would that be a big problem for you if we got rid of build.py?
  2. Considering that I prefer to avoid Windows, would you mind being assigned all windows-related issues?

Also: you mentioned earlier problems with x86 and amd64 running together. Considering that, IIRC, Windows installs in 64-bit by default since v7, I wouldn't even bother building a x86 version. If you want to, fine, but if that was me, I wouldn't bother.

@arsenetar
Copy link
Owner Author

  1. That would probably work out, would you be keeping package.py? I have found build.py to be easier just since it does not need msys2 and just seemed to work more consistently for me when I was initially getting everything working (it might have just been how I was doing stuff though). Just have to keep in mind depending on what changes are needed to phase out to just the Makefile, there might be additional things in the makefile to handle some windows differences.
  2. That should be fine, I should probably take a look at them to see what is all there.

I can build a x86 version as it is pretty quick to do so no big deal there, but yeah most people are probably running x64 by now.

@ghost ghost merged commit 8cd0ef4 into arsenetar:master Aug 28, 2017
@ghost
Copy link

ghost commented Aug 28, 2017

Yes, package.py will stay.

As for issues, they're not very well labeled. I've labeled "Windows" anything that had a windows traceback, but most of them are probably not windows-specific.

I don't think that there's many windows-specific tasks in the backlog. My question was more in reference of possible future windows-specific issues. I'll assign them to you if they arise.

@ghost ghost mentioned this pull request Sep 1, 2017
@arsenetar
Copy link
Owner Author

Just commenting I cleaned up my master to fix how I was doing PRs. Changes which made this PR up are now in https://github.com/arsenetar/dupeguru/tree/393. This is probably not needed as they are preserved here. But just in case I have moved them there.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant