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

Update loop in windows #359

Closed
philipgiuliani opened this issue Apr 26, 2016 · 39 comments
Closed

Update loop in windows #359

philipgiuliani opened this issue Apr 26, 2016 · 39 comments

Comments

@philipgiuliani
Copy link

philipgiuliani commented Apr 26, 2016

Hi! I have no idea where i should report it, so i start here with the hope that somebody can help.

I have packaged my windows app with electron builder and i'm using Squirrel.Windows for auto-updates. I've put everything from the release to the FTP and set the URL properly. But now when I start the windows app its in an update loop. It downloads the 1.0.0 every time again.

Does someone has an idea how it happened?

859E5121E95B411FA2107DF3A59E3880D4E9EA0B myapp-1.0.0-ia32-full.nupkg 45302165

Main app index.js (i ommited unreleated code)

if (require("electron-squirrel-startup")) {
    return;
}

app.on('ready', function() {
    // create new browserwindow

    // search for updates
    const updateFeed = os == "darwin" ? "https://HOST/updates/latest" : "https://HOST/releases/win32";

    const autoUpdater = require('auto-updater');
    autoUpdater.on("update-downloaded", function() {
        autoUpdater.quitAndInstall();
    });

    autoUpdater.setFeedURL(updateFeed + '?v=' + app.getVersion());
    autoUpdater.checkForUpdates();
});

I am building the windows distribution with OSX (Vine) with the following command: build --platform win32 --arch ia32

OSX is working fine.

@jbleuzen
Copy link

You should show your code that handle auto-update...
Without that, it's pretty hard to help.

@philipgiuliani
Copy link
Author

philipgiuliani commented Apr 26, 2016

Sorry, i was currently editing my post, pressed publish accidently.

@develar
Copy link
Member

develar commented Apr 26, 2016

Side note: Don't call autoUpdater.quitAndInstall on "update-downloaded". You should quit only if user click "Ok, quit and install. But better solution is — just show notification — https://github.com/develar/onshape-desktop-shell/blob/master/src/AppUpdater.ts#L24

Please ensure that your updated app has greater version number.

@philipgiuliani
Copy link
Author

@develar Thanks, yes i've not implemented it correctly yet, just want to get it running first :/ And thanks for the code snippet!

NGINX access.log after installing and starting the application

[26/Apr/2016:08:50:33 +0200] "GET /downloader/releases/win32/RELEASES?v=1.0.0&id=youtv&localVersion=1.0.0&arch=amd64 HTTP/1.1" 200 91
[26/Apr/2016:08:50:49 +0200] "GET /downloader/releases/win32/youtv-downloader-1.0.0-ia32-full.nupkg HTTP/1.1" 200 45302165
[26/Apr/2016:08:50:49 +0200] "GET /downloader/releases/win32/RELEASES?v=1.0.0&id=youtv&localVersion=1.0.0&arch=amd64 HTTP/1.1" 200 91
[26/Apr/2016:08:51:27 +0200] "GET /downloader/releases/win32/youtv-downloader-1.0.0-ia32-full.nupkg HTTP/1.1" 200 19496705

@develar
Copy link
Member

develar commented Apr 26, 2016

Please see electron/electron#5057

Do you code sign your app?

@philipgiuliani
Copy link
Author

Thanks for the link.
No, im not code signing the windows app at the moment. Could it prevent the issue?

@develar
Copy link
Member

develar commented Apr 26, 2016

Could it prevent the issue?

No, it can be a reason of issue.

@philipgiuliani
Copy link
Author

Code signing didn't help.

What's happening:

  1. App starts
  2. Server receives requests to RELEASES and downloads the -full.nupkg two times (auto-updater-win.js downloads updates twice when available. electron/electron#5057)
  3. After download the client restarts (autoUpdater.quitAndInstall())
  4. It starts again from 1

@philipgiuliani
Copy link
Author

I found the issue. I think its a bug in Squirrel.Windows.

I have always uploaded the ia32 version and not the 64bit version. I installed the 32 bit version on a 64bit PC and got into this loop issue. After uploading the 64bit RELEASES + nupkg it worked fine.

@develar
Copy link
Member

develar commented Apr 26, 2016

@philipgiuliani It is your issue due to lack of documentation. Please see #190 (comment) Your update server must serve RELEASES-ia32 as RELEASES on 32-bit update request.

@develar develar reopened this Apr 26, 2016
@develar
Copy link
Member

develar commented Apr 26, 2016

Reopened — must be documented.

@philipgiuliani
Copy link
Author

philipgiuliani commented Apr 26, 2016

When i generated 32 bit version it generates a RELEASES + RELEASES-ia32. I uploaded both and the content seemed the same. The app fetched the RELEASES file (from logs) and downloaded the app-ia32.nupkg. But looping. So the file was actually present. Thanks for your help.

@develar
Copy link
Member

develar commented Apr 26, 2016

The app fetched the RELEASES file

It is an issue in all known update servers. You must send RELEASES-ia32 if RELEASES requested for 32-bit version. See, for example, issue GitbookIO/nuts#32 — nuts server just doesn't support 32-bit releases.

I suggest to distribute only 64-bit versions (forget 32-bit). Or you must fix your update server to support both 32 and 64 bit releases.

But looping.

I see &arch=amd64 in your URL. Are your sure that proper file is returned (i.e. 64-bit version)?

And please, as an easy solution — "I suggest to distribute only 64-bit versions (forget 32-bit)." :)

@develar
Copy link
Member

develar commented Apr 26, 2016

I uploaded both and the content seemed the same

If you build only 32-bit, RELEASES file is generated as well and identical to RELEASES-ia32. Yes — it is a bug — we must not generate RELEASES file in this case. I will check.

@philipgiuliani
Copy link
Author

Yes we're now only distributing 64-bit versions. My update server has currently no logic (just for Squirrel.Mac) because they said Squirrel.Windows doesn't need that. Feel free to close when its fixed :) I'm happy with 64 bit.

@develar
Copy link
Member

develar commented Apr 27, 2016

I'm happy with 64 bit.

Good, because real fix is not possible on our side. RELEASES file cannot contain versions for several archs, so, it is update server responsibility to handle this issue. And reality is — no one update server implements it correctly because... damn windows! forget ia32 and use only 64-bit as OS X does for several years.

@develar develar closed this as completed Apr 27, 2016
@develar
Copy link
Member

develar commented Apr 27, 2016

On other hand if you distribute only ia32, we should still generate RELEASES file. So, it will be not changed.

@develar develar reopened this Apr 27, 2016
@philipgiuliani
Copy link
Author

It took me about 5 minutes to get OS X updates working... For Windows i spent a day to find out that it is because i only uploaded the x86 version... Yes i think its ok to generate it because its required.

@ghost
Copy link

ghost commented May 12, 2016

I ran into this issue on a project and found a solution. Due to certain restrictions, our application is only available as a 32-bit program, so we had to find a fix for this.

During the build process, electron-builder would output the following files:

app-0.1.0-ia32-full.nupkg
AppSetup-0.1.0-ia32.exe
RELEASES
RELEASES-ia32

Our fix was to:

  1. Delete RELEASES-ia32
  2. Remove all -ia32 strings from the filenames
  3. Update RELEASES with the new filenames

Below is our build script that does this:

builder.build(opts)
    .then(() => {
        const dir = jetpack.cwd('dist', 'win');

        // This is a fix for Squirrel's infinite update loop.
        return dir.removeAsync('RELEASES-ia32')
            .then(() => {
                return Promise.each(dir.listAsync(), (filename) => {
                    if (filename.indexOf('-ia32') !== -1) {
                        const newFilename = filename.replace('-ia32', '');
                        return dir.renameAsync(filename, newFilename);
                    }
                });
            })
            .then(() => {
                return dir.readAsync('RELEASES');
            })
            .then((txt) => {
                txt = txt.replace('-ia32', '');
                return dir.writeAsync('RELEASES', txt);
            });
    })
    .catch((err) => {
        console.log(err);
        process.exit(1);
    });

@develar
Copy link
Member

develar commented May 13, 2016

@bontibon I will revert #190 (comment) if @mcaoun will confirm that fix doesn't work in any case.

cc @demetris-manikas

So

  • warn that ia32 for windows is not recommended: "For windows consider only distributing 64-bit"
  • don't change RELEASES file name and contents.

If @mcaoun will not say that "don't revert, it works for me!", I will publish fix in the 3.x version, not major 4.

@develar
Copy link
Member

develar commented May 13, 2016

@bontibon Enjoy. 3.22.0 will not modify RELEASES for ia32 anymore.

@friksa
Copy link

friksa commented May 18, 2016

How can you just break all existing build processes? There are many developers depending on these projects. Yes, we would love to live in a perfect world where we do not care about customers that do not upgrade.

@develar
Copy link
Member

develar commented May 18, 2016

@friksa What do you mean?

@friksa
Copy link

friksa commented May 18, 2016

Our build process was building win32 versions of electron happily. After these changes, it does not.

@develar
Copy link
Member

develar commented May 18, 2016

@friksa Please specify error, or what's wrong?

@friksa
Copy link

friksa commented May 18, 2016

Got to the bottom of this and it turns out to be easily solvable in the morning. The build process always yielded a folder named "win". Now it is "win-ia32". This caused the build process to break. Adjusting the build process to use the new folder name resolves the issue. Thanks!

@develar
Copy link
Member

develar commented May 18, 2016

@friksa Sorry for that. Could you please specify why "the build process to break"? Do you publish manually instead of github publisher (or creating issue "support my product_name (amazon s3?) to publish!!!")?

@friksa
Copy link

friksa commented May 18, 2016

We build it with

npm run clean:win && ./node_modules/.bin/build --platform win32 --arch ia32 && node tasks/sign

Then we scp the files to our server and use a home-grown Java server to serve updates.

Since the files were not found in the expected place, the build process reported a broken build.

@develar
Copy link
Member

develar commented May 18, 2016

node tasks/sign

BTW, code sign is supported (and it is not possible for you to sign correctly outside of build process).

@friksa Thanks for explanation.

@Vj3k0
Copy link

Vj3k0 commented May 20, 2016

I wrote a blog post about building and deploying app, and it supports both 32 and 64 versions of Win:
http://electron.rocks/building-and-deploying-application/

Hope it helps, best,
Vjeko

@develar
Copy link
Member

develar commented May 20, 2016

@Vj3k0 Thanks a lot for your articles. Please see and comment if possible #413

@ccnokes
Copy link

ccnokes commented Jun 3, 2016

@develar for future reference, in what version was this issue introduced and in what issue is it fixed (or has it even been fixed)? Thanks!

@develar
Copy link
Member

develar commented Jun 4, 2016

@ccnokes It not our bug, it is Squirrel.windows&nuget design mistake and as unpreventable result — user configuration error. There is no fix — we just revert our feature, because we don't want to fix broken windows. Please see #439

@AdrienFery
Copy link

Hi,

I have both a 32-bit and 64-bit release of my app and the auto-updater work well with both arch.

When the app starts, I make a request on my server with the following parameters: current app version, platform, arch to check if an update is available. If the current version is already up to date nothing happened... otherwise I call autoUpdater.setFeedURL(feedUrl); with the feedUrl returned by the server (that depends of the version, platform and arch).

@weiway
Copy link

weiway commented Nov 21, 2016

I think I just met the same looping issue, I reported details to electron project and here is the link.
electron/electron#8040

@develar
Copy link
Member

develar commented Nov 22, 2016

Squirrel.Windows is deprecated by electron-builder — consider to use nsis target.

@dougludlow
Copy link

@develar can you still use the electron autoupdater api with nsis? I thought it relied on squirrel.windows.

@dougludlow
Copy link

Nevermind, I see #529 now.

@Neustradamus
Copy link

This bug has been solved in Squirrel?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants