cloudmatrix / esky

an auto-update framework for frozen python apps
BSD 3-Clause "New" or "Revised" License
362 stars 74 forks source link

Fix #113. Can now apply local and remote patches. #128

Closed JPFrancoia closed 8 years ago

JPFrancoia commented 8 years ago

A EOFError exception is raised when trying to apply a local patch, but an IndexError is raised when the patch is downloaded from a remote server.

Solves the issue for both remote and local server.

timeyyy commented 8 years ago

How can I go about testing this?

JPFrancoia commented 8 years ago

Use the following zip (simply the stage 3 of the tuto):

esky_test_patch.zip

First, build the esky app with:

python setup.py bdist_esky_patch

Then change the version number in setup.py, to a higher one, and build the app again. It will generate a patch for the new version.

To test the local patch, use this command in the dist folder created by esky:

python -m http.server 8000

And then run the app with the lowest version. To test the remote patch, upload the patch in the dist folder onto a remote server. I used an http server provided by OVH (it's an apache one, shared. That's where I'm going to run my apps, once in production). You will have to modify the example.py script to use the correct URL.

timeyyy commented 8 years ago

where is the eoferror getting raised? is the fact that an eoferror is not being raised also an issue? i.e i don't want to simply be masking a deeper issue here

JPFrancoia commented 8 years ago

Yes sorry, my bad, I was not precise engouh.

The fact that an oeferror is not being raised is the issue in this situation. For local patches, eoferror is raised, which is the handled case, and leads to the proper behaviour.

When the patch is remote however, an InderError is raised instead of the eoferror, and leads to an unhandled case, and makes the update crash.

Sorry if I wasn't clear enough.

timeyyy commented 8 years ago

So by simply catching the IndexError, isn't this just hiding a deeper bug?

JPFrancoia commented 8 years ago

I have absolutely no idea.

while True:
   cmd = self._read_command()
   getattr(self,"_do_" + _COMMANDS[cmd])()

Just 2 lines above, there is a while loop trying to read. So at one moment, it might try to read an unexisting index. Why it happens for a remote patch and not a local one, I have no idea.

timeyyy commented 8 years ago

Is this a complete fix for the issue?

JPFrancoia commented 8 years ago

It does fix the original issue #128 , which was reported for Linux (but was also present on Windows).

Patches still don't work for OSX, but for another reason, I'm working on it -> It follows the issue #129 .

timeyyy commented 8 years ago

@JPFrancoia Sorry man, i messed around with the git history. Would you be able to run git pull --rebase? If it's still wonky you might have to do a cherry-pick and repush the patch. I'm going to merge this and just make a note of it to test a bit more later.

JPFrancoia commented 8 years ago

What do you mean ? What should I rebase ?

timeyyy commented 8 years ago
git checkout master
git pull --rebase upstream  (where upstream is cloudmatrix repo)
git checkout "the branch you are making the pull request from"
git rebase master

If you get collisions while doing the last rebase it might be easier to just

git checkout -b fixing_remote_patches
git cherry-pick 615bd8e85356b0bb67192c22501897f8d3a5a72d

in this case i will close this pull request and you can open a new one

JPFrancoia commented 8 years ago

Clearly, too complicated. I don't have time to do that. Can't I simply close this PR and make a new one ?

timeyyy commented 8 years ago

yes