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

Windows Installer: reports of new content not being available #1622

Closed
blitzmann opened this issue May 31, 2018 · 13 comments
Closed

Windows Installer: reports of new content not being available #1622

blitzmann opened this issue May 31, 2018 · 13 comments
Labels
bug Confirmed to be a bug fixed This issue has been fixed! Oh joy! 🚨 HIGH PRIORITY 🚨 This issue is receiving the highest priority in getting resolved.

Comments

@blitzmann
Copy link
Collaborator

Any right click on projected area is not working

OS version: Windows-10-10.0.17134-SP0
Python version: 3.6.1 (v3.6.1:69c0db5, Mar 21 2017, 17:54:52) [MSC v.1900 32 bit (Intel)]
wxPython version: 4.0.0b2 (wxWidgets 3.0.4)
SQLAlchemy version: 1.1.10
Logbook version: 1.0.0
Requests version: 2.18.4
Dateutil version: 2.6.0

####################

Traceback (most recent call last):
 File "C:\Users\holme\Documents\Sync\Git\blitzmann\Pyfa\venv\lib\site-packages\wx\core.py", line 3189, in <lambda>
 File "C:\Users\holme\Documents\Sync\Git\blitzmann\Pyfa\gui\builtinAdditionPanes\projectedView.py", line 311, in spawnMenu
 File "C:\Users\holme\Documents\Sync\Git\blitzmann\Pyfa\gui\contextMenu.py", line 85, in getMenu
 File "C:\Users\holme\Documents\Sync\Git\blitzmann\Pyfa\gui\builtinContextMenus\whProjector.py", line 67, in getSubMenu
 File "C:\Users\holme\Documents\Sync\Git\blitzmann\Pyfa\gui\builtinContextMenus\whProjector.py", line 173, in getAbyssalWeather
 File "C:\Users\holme\Documents\Sync\Git\blitzmann\Pyfa\service\market.py", line 411, in getGroup
 File "C:\Users\holme\Documents\Sync\Git\blitzmann\Pyfa\eos\db\gamedata\queries.py", line 59, in checkAndReturn
 File "C:\Users\holme\Documents\Sync\Git\blitzmann\Pyfa\eos\db\gamedata\queries.py", line 167, in getGroup
AttributeError: 'NoneType' object has no attribute 'ID'

Used the installer. Apparently it's only for the installer folks???

@blitzmann blitzmann added bug Confirmed to be a bug 🚨 HIGH PRIORITY 🚨 This issue is receiving the highest priority in getting resolved. labels May 31, 2018
@blitzmann
Copy link
Collaborator Author

The installer is using an old eve.db... wtf

@blitzmann
Copy link
Collaborator Author

blitzmann commented May 31, 2018

No it sn't... Odd, I look at eve.db directly, and it gives me this version: 1321734 which is correct. If I open pyfa...
image

Old version. And it's using the proper path for the game database

image

None of the new stuff is available.

If I run it as ADMIN:
image

Same path

image

new version

@blitzmann blitzmann changed the title Abyssal environment not working Windows Installer: reports of new content not being available May 31, 2018
@blitzmann
Copy link
Collaborator Author

So, uninstalling doesn't work either (after uninstalling, pyfa directory remained in Program Files (x86), but it was empty...).

If I move that pyfa directory to pyfa-bak and reinstall to default location, same things happens as described above.

however, if I change the default location and install in like Program Files (x86)/pyfa-test, seems to work without admin rights.

@blitzmann
Copy link
Collaborator Author

Thinking it might be a registry thing (for whatever reason), I searched for eve.db and didn't see anything that may shed light on the issue

@blitzmann
Copy link
Collaborator Author

Found it?

image

pyfa is accessing eve.db in a directory called "VirtualStore" in app data... what the fuck this is I have no idea

@blitzmann
Copy link
Collaborator Author

Interesting.

VirtualStore is a thing that happens when a program attempts to open write access to Program Files (in this case, to eve.db). This is supported by the fact that it shows as virtualized in task manager hen running non-admin:

image

Apparently you can bypass this by including a manifest... which is something pyinstaller might not do? Still reading into it

https://answers.microsoft.com/en-us/windows/forum/windows_7-windows_programs/please-explain-virtualstore-for-non-experts/d8912f80-b275-48d7-9ff3-9e9878954227

@blitzmann
Copy link
Collaborator Author

blitzmann commented May 31, 2018

Can kinda confirm it's something to do with the way PyInstaller packages.

I manually removed everything from Program Files (x86)/pyfa and the VirtualStore directory, then manually moved the 1.37 zip contents into it (which used cx_freeze, not PyInstaller). I can run it fine, with nothing created in VirtualStore. When I extract the 2.x zip file, it's virtualized.

@blitzmann
Copy link
Collaborator Author

More digging

Looks like the previous builds (1.x) that used cx_freeze have a different manifest in the exe than the PyInstaller (2.x) builds. Above red line is 1.x, below is 2.x:

image

There is supposedly a way of giving pyinstaller a manifest file, so I'm gonna try that and maybe that might work?

@blitzmann
Copy link
Collaborator Author

I think i got it to work... more testing is required though, and I'll have to get with some folks to test before release. more details soon

@blitzmann
Copy link
Collaborator Author

k, So I'm probably going to have to clarify a few things with the PyInstaller team, but so far:

The EXE from PyInstaller does not include an embedded manifest file, whereas the cx_freeze EXE does. When there is no embedded manifest file, windows looks for a pyfa.exe.manifest file alongside pyfa.exe. A manifest can (somehow) tell windows not to virtualize the application.

I basically copied the embedded in the cx_freeze exe into our current builds' pyfa.exe.manifest (well, I actually merged them). This is somewhat unfortunate, because now I need to ensure that this file makes it's way from dist_assets/win to the build folder after pyinstaller get done.

Additionally, there is this file: https://github.com/pyfa-org/Pyfa/blob/master/dist_assets/win/Microsoft.VC90.CRT.manifest

I am not sure if it's required, but the manifest that I ripped from the cx_freeze exe references that assembly, so why not.

So that takes care of the virtualization. What about the now defunct VirtualStore files? I could leave them there, they don't hurt, but I like cleaning that crap up:

e3bec84#diff-956365660bd67affa7d436d7ed901b00

This ill delete pyfa's VirtualStore every install. I really could have gone with just this method, but I wanted a proper fix.

So all in all, seems we're good to go. I'm prepping a new release now to address.

@blitzmann blitzmann added the fixed This issue has been fixed! Oh joy! label May 31, 2018
@minlexx
Copy link
Contributor

minlexx commented May 31, 2018

So, pyfa will continue to write into Program Files directory? Ideally apllication should not write there, otherwise it requires admin rights, in newer Windows versions. Program Files should ideally be read-only. For mutable data files, ProgramData is intended, or subdirectory in user profile folder, like %APPDATA%\Pyfa.
Maybe sqlalchemy or even sqlite is opening its DB file for r+w?..
Maybe it is better to install the whole app directly into %APPDATA% folder, some new apps already do this way (WhatsApp client, Doscord, slack, ..)

@blitzmann
Copy link
Collaborator Author

So, pyfa will continue to write into Program Files directory?

Technically it's able to, but pyfa doesn't explicitly write to anything in the installation directory.

Maybe sqlalchemy or even sqlite is opening its DB file for r+w?..

This is the most likely scenario. Windows is considering it as writable, and thus freaking out. However, from what I can tell, I can't tell SQLAlchemy to open the database as read-only.

Further investigation into better locations is welcome. ProgramData is definitely a thing that we could potentially utilize, but it would require a bit of research to get right methinks. We would have to consider long-standing assumptions that eve.db lives where pyfa is installed as wrong. ProgramData is OS-specific, so we would also have to ensure it whatever method we use doesn't conflict with the OS X build and anyone using Linux / source. That, and there's still a hell of a lot of people that still unzip the zipfile into program files instead of using the installer for who knows why. If we switch to using ProgramData, this method will suddenly break.

This was more of a "OH SHIT IT'S BROKE FIX IT FIX IT" kind of fix, with no deeper search for a better alternative. 😛 That being said, it's currently working as it always has in the past, and so the motivation to refactor things and do it "right" isn't there, at least for me.

@minlexx
Copy link
Contributor

minlexx commented May 31, 2018

There is wx.StandardPaths class, which is supposed to determine standard locations in the file system and should be used by applications to find their data files in a portable way. Especially GetUserConfigDir() seems to be the right thing. May need testing what it really returns in Windows/Linux/Mac...

eve.db lives where pyfa is installed

This path can be searched as fall-back, for example, before opening DB, manually test which of eve.db exists, current dir of in %APPDATA%\Pyfa.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed to be a bug fixed This issue has been fixed! Oh joy! 🚨 HIGH PRIORITY 🚨 This issue is receiving the highest priority in getting resolved.
Projects
None yet
Development

No branches or pull requests

2 participants