Skip to content
This repository has been archived by the owner on Oct 21, 2022. It is now read-only.

Upgrading isn't working #2138

Open
kenny-evitt opened this issue Jan 31, 2016 · 7 comments
Open

Upgrading isn't working #2138

kenny-evitt opened this issue Jan 31, 2016 · 7 comments

Comments

@kenny-evitt
Copy link
Contributor

There are several recent bug reports for users that upgraded in LT, for 0.8.1 in particular, that are reported as being fixed by downloading the relevant release package directly. Some of those issues:

I was able to reproduce one of those bugs on a Windows 10 computer [L] after upgrading in LT from 0.8.0 to 0.8.1. I downloaded the release package for 0.8.1 directly and compared the directories where the 0.8.0 and 0.8.1 versions were installed; a lot (all?) of the 0.8.0 version files seem to not have been upgraded.

As explained previously, too much has changed between versions 0.7.2 and 0.8.0 so this should be restricted to upgrades between 0.8.0 and 0.8.1 (or newer verions).

@kenny-evitt
Copy link
Contributor Author

I was able to reproduce #2131 just now after downloading and install 0.8.0 and then supposedly upgrading it to 0.8.1.

@kenny-evitt
Copy link
Contributor Author

So, to clarify, I was able to reproduce this issue just now on a Windows 7 computer [T].

@kenny-evitt kenny-evitt self-assigned this Feb 5, 2016
@kenny-evitt
Copy link
Contributor Author

@cldwalker @rundis I narrowed this down somewhat. I'm using #2131 as my example issue – that seemed to have been caused by resources\app\core\node_modules\lighttable\cljs\lt\objs\docs.js not being copied during the upgrade. And it's not being copied because it doesn't exist in the tarball that's downloaded.

Here's the relevant code that generates the tarball URLs:

(defn tar-path [v]
  (if (cache/fetch :edge)
    (str "https://api.github.com/repos/LightTable/LightTable/tarball/master")
    (str "https://api.github.com/repos/LightTable/LightTable/tarball/" v)))

Those URLs correspond to the GitHub API 'get archive link' method:

This method will return a 302 to a URL to download a tarball or zipball archive for a repository. Please make sure your HTTP framework is configured to follow redirects or you will need to use the Location header to make a second GET request.

It seems like we broke this during the Atom→Electron migration. I strongly suspect that the previous tar-path URLs were for 'release tarballs' not 'source tarballs'. I imagine we never really ran into this because we've been running locally-built-from-source builds for a long while now.

I'm going to work on using the GitHub releases instead of the source archive tarballs.

@cldwalker
Copy link
Member

@kenny-evitt Thanks for taking a look. That sounds like a good place to dig in

@rundis
Copy link
Contributor

rundis commented Mar 16, 2016

Yeah I've used an upgraded 0.8.1 and started getting all kinds of weird errors. Ended up downloading a package from lighttable.com. What's more I can't build from source on OS/X El Capitan, but that's a separate issue.

@kenny-evitt
Copy link
Contributor Author

@cldwalker @rundis I created a PR for this – #2176. The two commits are very-much WIP.

The major change is that we want to download a release file (with the compiled ClojureScript) instead of the source tarball (as the code does now). That's mildly tricky for a few reasons:

  1. There's a separate release for each platform. In the interest of potentially being able to also upgrade Electron itself, I figured we might as well download the relevant release file for the current user.
  2. I'm retrieving the release info from GitHub to retrieve the URLs for the release files so there's a new callback now that propagates thru a few functions just to get that URL. Previously, the URL was very simple to generate.
  3. The Windows release file is a ZIP file so I needed to add a Node module to extract the files. I'm not sure how much we should attempt to avoid this. We could publish .tar.gz release files for Windows because archive utilities, e.g. 7-Zip, can handle them just fine.
  4. Electron patches fs to make .asar files act as directories – this breaks both of the ZIP libraries I tried, probably breaks all ZIP libraries, and it also apparently breaks tar files (tho I haven't confirmed this myself yet). I couldn't get any of the seeming workarounds documented by Electron for forcing Electron to treat an ASAR archive as a normal file to work for me. Others have mentioned needing to use libraries that allow an alternate fs module to be supplied. We can avoid this indefinitely because we don't need to extract the entirety of any of the release archive files – we seem to only need the lighttable-version-platform/resources/app/ directory and the .asar file is in the lighttable-version-platform/resources/ directory. But then we can't really do Electron upgrades per [1].

Related to [2], there's an existing 'edge' setting that seems to control whether the user is running the latest source version. I'm going to defer implementing that for now, but we could potentially use the Travis CI builds to support that.

As-is, I'm going to continue with my current 'solution', i.e. by modifying fetch-and-deploy to call unzip if the platform of the current computer is Windows.

@kenny-evitt
Copy link
Contributor Author

Please un-assign me.

@prertik prertik moved this from Upgrading/Versioning to In Progress in Technical Debt Dec 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Technical Debt
  
In Progress
Development

No branches or pull requests

6 participants