-
Notifications
You must be signed in to change notification settings - Fork 26
Check for Update on Application Start (lightweight) #34
base: master
Are you sure you want to change the base?
Conversation
Thanks for taking this on! You have verified that both Windows and macOS builds work as expected, right? In particular, you're not getting certificate issues? |
Sorry, but I am not sure where so I want to ask, where can I check if there were certificate issues with the builds? We are able to make the binaries and then open them. |
@estberg can you verify that the Windows binary actually makes a successful request to Github? If it makes the request and gets the response as expected, then you don't have those issues. To be 100% sure, you can observe network traffic. |
@estberg and I followed @yanokwa s comment in the following way:
print(latest_version[1:])
print(self._current_version[1:]) With those changes, we saw the following output: C:\xlsform-offline>"dist\win\ODK XLSForm Offline.exe"
1.11.1
1.11.1 We're pretty sure this indicates that we are checking for the update correctly, but haven't explicitly monitored traffic. Should we go that step further or do you think this is sufficient evidence? |
That looks sufficient to me, thanks! |
requirements.txt
Outdated
pyinstaller==3.5.0 | ||
wxpython==4.0.6 | ||
pyxform==0.15.1 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason you are using old versions of these packages instead of the most recent ones? If so, a comment here would be great. If not, we should use recent versions.
Relatedly, in the case of pyinstaller 3.4, it has a severe vulnerability and so we have to use 3.6. See GHSA-7fcj-pq9j-wh2r for more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I will try in my environment with the most recent versions.
I tried this on a fresh macOS 10.15.2 (Catalina) install and did not have success. This is a long comment, so bear with me... I used Python 3 The steps I took to run from source are:
The build could not be packaged, probably because of Python 3
Based on https://github.com/pyinstaller/pyinstaller/wiki/FAQ#mac-os-x, my guess is that we'll have to use pyenv and build with The app did run, but had some Python 3 issues
Once I did the above, the app launched, but I had two more problems. These should probably be fixed with follow on PRs, but I don't feel strongly about where those commits go.
My guess is that somehow Python doesn't know where the certs are? I tried @estberg, I know this is a lot to digest, so maybe we can start with few questions so I can reproduce your successful build.
|
Thanks @yanokwa for taking all the time to try this out and leave this feedback for us.
|
I'm not super familiar with conda, but it seems for this use-case, it's playing the role of pip and pyenv. So yes, I would use pip and pyenv and see how that goes. |
@yanokwa @estberg and I have been working on windows computers with Python 3.7.4 installed. The way we got our end working was to change the make-win.bat file to specify which version of python to install (as well as ensure the correct packages have been installed).
I'm the one who made the decision to do it this way because my understanding was that Python2 and Python3 are separate enough that having Python3 installed does not satisfy the 2.7 requirement specified by this repos readme. Should we change the .sh and .bat files to always work regardless of Python version? If so, should the readme on the repo also be changed? I'll also note that the urllib issue you mentioned sounds familiar from our experience trying to use Python3 without the correct packages installed in 2.7. |
@PieterBenjamin The README is out of date so yes, please update the README to read "Python 3". I don't think we need to specify the point version just yet. It is true that Python 2 and 3 are very different. I think we should drop Python 2 support and focus exclusively on Python 3 for this release. As far as sequencing on the Window side, I'd try to get it running on a machine with a single Python 3 (kind of like you did above) first. Once that works, see if you can get it running with a pyenv setup. I bet pyenv will not work, but we should try because that's what most developers expect. It's not super important to solve though because we're going to be using a build machine to make these binaries. |
@yanokwa Should the installation files specify python 3 or just use whatever the client has installed? |
To be explicit here...you don't need any Python to run the app. PyInstaller bundles whatever Python the developer has installed when packaging. In this case, we are requiring that devs use Python 3, so that's what will be bundled. |
a6002b2
to
e33050d
Compare
We are still working on testing this on Windows, but it works on Mac now! |
Co-authored-by: Pieter Benjamin <[email protected]>
We just pushed a commit - we weren't copying the Let us know if this looks good and we can squash the commit. |
Thanks so much for all the hard work that's gone into this @PieterBenjamin, @estberg and so sorry about the long radio silence. The code looks good to me and I've double checked that the binary sizes are only slightly increased. I wanted to do a sanity check on the behavior and am having some problems. It looks like only branches on the main repo, not forks, get their builds uploaded, is that right? I pushed to https://github.com/getodk/xlsform-offline/compare/feature/update-checker where I added a commit setting the version to '1.0.0'. I can correctly see v1.0.0 in the footer of the about screen but I'm not getting a version update notice. Is this a reasonable way to try it out? http://travis.getodk.org/xlsform-offline/ODK-XLSForm-Offline-macOS-v2.0.0-21-g62b5a35.zip is the macOS build I tried. |
Hi folks, thanks for the feedback. I think that yeah only branches get built. As to the v1.0.0, the thing which jumps to mind for me is how we check new versions. If v1.0.0 was less than the current version, I can imagine it not working. But it sounds like @yanokwa was able to get the notification on windows? Although from his screenshot, it sounds like he used v2.0.0. |
I downloaded @lognaturel's build which should have set the version to v1.0.0. On Windows, it correctly showed that v2.0.0 is the newest version. On macOS, it did nothing. |
A Continuation of 28 aiming to resolve 13.
Essentially uses @NizamLZ's approach to:
The only change is the packages that are used to do this are much more lightweight.
The mac binary is 37.3 MB and the windows binary is 13.1 MB.
(Special thanks to @PieterBenjamin for helping with building the windows binary)