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

Add automatic check for updates that nags the user when there's a new version available #1429

Merged
merged 8 commits into from Nov 14, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -19,6 +19,7 @@
"ini": "^1.3.4",
"invariant": "^2.2.0",
"is-builtin-module": "^1.0.0",
"is-ci": "^1.0.10",
"leven": "^2.0.0",
"loud-rejection": "^1.2.0",
"minimatch": "^3.0.3",
Expand Down
112 changes: 112 additions & 0 deletions src/cli/commands/install.js
Expand Up @@ -25,8 +25,15 @@ import * as crypto from '../../util/crypto.js';
import map from '../../util/map.js';

const invariant = require('invariant');
const userHome = require('user-home');
const semver = require('semver');
const emoji = require('node-emoji');
const isCI = require('is-ci');
const path = require('path');
const fs2 = require('fs');

const YARN_VERSION = require('../../../package.json').version;
const ONE_DAY = 1000 * 60 * 60 * 24;

export type InstallPrepared = {
skip: boolean,
Expand Down Expand Up @@ -69,6 +76,43 @@ type Flags = {
tilde: boolean,
};

/**
* Try and detect the installation method for Yarn and provide a command to update it with.
*/

function getUpdateCommand(): ?string {
// Tarball install
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing this inferrence, let's save a token somewhere as part of the packaging process. @cpojer said:

Can we somehow add a token to every build indicating the install method? I think that's way better than any sort of inference we might do.

My idea for that was to add an extra installationMethod field to package.json when building the packages. For example, something like this would be bundled in the Debian package:

{
  "name": "yarn",
  "version": "0.16.2",
  "installationMethod": "deb",

What do you think? #942 is the issue that covers(-ish) that. If you agree that that's a good idea, let's complete #942 before this one, and then rebase this on top of that.

The main issue I see with inference is when the user has multiple installations of Yarn. That's a bit silly, but they might have installed via the tarball and then also done npm install -g yarn for some reason. The instructions might tell them to update the wrong one.

if (fs2.existsSync(path.join(userHome, '.yarn'))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't ~/.yarn where user settings are stored?

return 'yarn self-update';
}

// OSX
if (fs2.existsSync('/usr/local/Cellar')) {
return 'brew upgrade yarn';
}

// Debian
if (fs2.existsSync('/usr/share/lintian/overrides/yarn')) {
return 'sudo apt-get install yarn';
}

// npm
if (__dirname.indexOf('node_modules') >= 0) {
return 'npm upgrade --global yarn';
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add Chocolatey: choco upgrade yarn

return null;
}

function getUpdateInstaller(): ?string {
// Windows
if (fs2.existsSync('C:/Program Files/Yarn') || fs2.existsSync('C:/Program Files (x86)/Yarn')) {
Copy link
Member

@Daniel15 Daniel15 Oct 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work as the user can select any directory during installation. Using the environment variable (%ProgramFiles%\Yarn) is better, but still not 100% accurate. They might decide to install it to c:\apps\Yarn for example. There's a registry key we can read to get the path, but modifying package.json to have "installationMethod": "msi" as part of the build process would be the easiest thing to do, then we can just require(__dirname + 'package.json').installationMethod

return 'https://yarnpkg.com/latest.msi';
}

return null;
}

function normalizeFlags(config: Config, rawFlags: Object): Flags {
const flags = {
// install
Expand Down Expand Up @@ -273,6 +317,8 @@ export class Install {
*/

async init(): Promise<Array<string>> {
this.checkUpdate();

let [depRequests, rawPatterns] = await this.fetchRequestFromCwd();
const match = await this.matchesIntegrityHash(rawPatterns);

Expand Down Expand Up @@ -355,6 +401,7 @@ export class Install {

// fin!
await this.saveLockfileAndIntegrity(rawPatterns);
this.maybeOutputUpdate();
this.config.requestManager.clearCache();
return patterns;
}
Expand Down Expand Up @@ -640,6 +687,71 @@ export class Install {

return request;
}

/**
* Check for updates every day and output a nag message if there's a newer version.
*/

checkUpdate() {
if (!process.stdout.isTTY || isCI) {
// don't show upgrade dialog on CI or non-TTY terminals
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is still valuable for non-TTY environments. Why should we avoid showing a notice about Yarn being outdated just because the user is piping the output to a file, for example?

return;
}

// only check for updates once a day
const lastUpdateCheck = Number(this.config.getOption('lastUpdateCheck')) || 0;
if (lastUpdateCheck && Date.now() - lastUpdateCheck < ONE_DAY) {
return;
}

// don't bug for updates on tagged releases
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I wonder if we should bug on nightly builds?

if (YARN_VERSION.indexOf('-') >= 0) {
return;
}

this._checkUpdate().catch(() => {
// swallow errors
});
}

async _checkUpdate(): Promise<void> {
let latestVersion = await this.config.requestManager.request({
url: 'https://yarnpkg.com/latest-version',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to better automate this getting updated

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm going to have a webhook that runs when releases are updated, or just a cronjob. The tricky thing is that we can't bump the version number until we verify that all files are attached to the release, and the Windows installer is attached separately from everything else (as it's built on AppVeyor).

});
invariant(typeof latestVersion === 'string', 'expected string');
latestVersion = latestVersion.trim();
if (!semver.valid(latestVersion)) {
return;
}

// ensure we only check for updates periodically
this.config.registries.yarn.saveHomeConfig({
lastUpdateCheck: Date.now(),
});

if (semver.gt(latestVersion, YARN_VERSION)) {
this.maybeOutputUpdate = () => {
this.reporter.warn(this.reporter.lang('yarnOutdated', latestVersion, YARN_VERSION));

const command = getUpdateCommand();
if (command) {
this.reporter.info(this.reporter.lang('yarnOutdatedCommand', command));
} else {
const installer = getUpdateInstaller();
if (installer) {
this.reporter.info(this.reporter.lang('yarnOutdatedInstaller', installer));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional, but can we use something like boxen (or just render the ANSI codes ourself) to render a fancy box around it, like update-notifier does?

}
}
};
}
}

/**
* Method to override with a possible upgrade message.
*/

maybeOutputUpdate() {}
maybeOutputUpdate: any;
}

export function _setFlags(commander: Object) {
Expand Down
2 changes: 2 additions & 0 deletions src/config.js
Expand Up @@ -376,6 +376,8 @@ export default class Config {
return file;
}
}

return null;
});
}

Expand Down
4 changes: 4 additions & 0 deletions src/reporters/lang/en.js
Expand Up @@ -72,6 +72,10 @@ const messages = {
unexpectedError: 'An unexpected error occurred, please open a bug report with the information provided in $0.',
jsonError: 'Error parsing JSON at $0, $1.',

yarnOutdated: "Your current version of Yarn is out of date. The latest version is $0 while you're on $1.",
yarnOutdatedInstaller: 'To upgrade, download the latest installer at $0.',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is totally optional, but we could download the installer and run it for the user. Maybe we'll hold off on that for now and do it as an enhancement!

yarnOutdatedCommand: 'To upgrade, run $0.',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put the command on its own separate line. @sindresorhus said:

Having the command on a separate line makes it easier to copy-paste for users.
#1138 (comment)


tooManyArguments: 'Too many arguments, maximum of $0.',
tooFewArguments: 'Not enough arguments, expected at least $0.',
noArguments: "This command doesn't require any arguments.",
Expand Down
13 changes: 12 additions & 1 deletion yarn.lock
@@ -1,5 +1,7 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


abab@^1.0.0:
version "1.0.3"
resolved "https://registry.yarnpkg.com/abab/-/abab-1.0.3.tgz#b81de5f7274ec4e756d797cd834f303642724e5d"
Expand Down Expand Up @@ -1329,6 +1331,10 @@ chokidar@^1.4.3, chokidar@^1.5.2:
optionalDependencies:
fsevents "^1.0.0"

ci-info@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/ci-info/-/ci-info-1.0.0.tgz#dc5285f2b4e251821683681c381c3388f46ec534"

cipher-base@^1.0.0, cipher-base@^1.0.1:
version "1.0.3"
resolved "https://registry.yarnpkg.com/cipher-base/-/cipher-base-1.0.3.tgz#eeabf194419ce900da3018c207d212f2a6df0a07"
Expand Down Expand Up @@ -2671,6 +2677,12 @@ is-builtin-module@^1.0.0:
dependencies:
builtin-modules "^1.0.0"

is-ci:
version "1.0.10"
resolved "https://registry.yarnpkg.com/is-ci/-/is-ci-1.0.10.tgz#f739336b2632365061a9d48270cd56ae3369318e"
dependencies:
ci-info "^1.0.0"

is-ci@^1.0.9:
version "1.0.9"
resolved "https://registry.yarnpkg.com/is-ci/-/is-ci-1.0.9.tgz#de2c5ffe49ab3237fda38c47c8a3bbfd55bbcca7"
Expand Down Expand Up @@ -5192,4 +5204,3 @@ yargs@~3.27.0:
os-locale "^1.4.0"
window-size "^0.1.2"
y18n "^3.2.0"