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

unknown package or version in .meteor/versions should trigger a refresh (but not otherwise cause an error) #3653

Closed
arunoda opened this Issue Feb 5, 2015 · 22 comments

Comments

Projects
None yet
4 participants
@arunoda

arunoda commented Feb 5, 2015

I've seen downgrading packages a quite often. The thing is it's pretty hard to debug and send you are reproducible build. Since, it works pretty well on local machine(OSX) and once we are trying to build it on a CI box or something similar package gets downgraded.

So, I got an issue from @dweldon today and let me share it with you. May be we could find an answer for this.


This is related to kadira package and this issue came from Ubuntu 14.04.1 LTS. Here's what meteor says.

meteorhacks:kadira downgraded from 2.17.5 to 2.17.0

But, there is no big changed between these two versions. Mainly, a few bug fixes. See: https://gist.github.com/arunoda/9fca292380dd576ee6f6

In kadira I don't publish .versions file or versions.json file to git. But, I think it has nothing to do with this issue.

@dweldon is it possible to share the package list and versions files? That might give some hint.

@arunoda

This comment has been minimized.

arunoda commented Feb 5, 2015

Here's the original issue: meteorhacks/kadira#134

@glasser

This comment has been minimized.

Member

glasser commented Feb 5, 2015

There have been several reports recently of behavior like this. But not one reproduction that we can actually test out.

@arunoda

This comment has been minimized.

arunoda commented Feb 5, 2015

I get it. I had a issue like this and it get resolved by suddenly. I'll try to build a reproducible with this.

@awatson1978

This comment has been minimized.

Contributor

awatson1978 commented Feb 5, 2015

I've seen this as well, with nightwatch being downgraded on Travis CI server. Started with Meteor 1.0.3 it seems. Specifying a version constraint in .meteor/packages got things working again.

@awatson1978

This comment has been minimized.

Contributor

awatson1978 commented Feb 6, 2015

Here's a build history. Nightwatch was being downgraded in runs 1 through 5. Run number 6 fixed the issue by specifying a version constraint.
https://travis-ci.org/awatson1978/minimongo-table/builds

Here's the log file of build 5, just before the fix was put into place.
https://travis-ci.org/awatson1978/minimongo-table/builds/49039074

Here's the source code as of build #5.
https://github.com/awatson1978/minimongo-table/tree/2a7c8930984acdc9fdeb045120a822128f170726

# clone the repository
git clone https://github.com/awatson1978/minimongo-table
cd minimongo-table

# remove the existing git repository
rm -rf .git

# add to your own repository, so you can attach it to your own travis account
git init .
git add --all .
git commit -a -m 'initial commit'
git remote add origin http://github.com/myaccount/minimongo-table.git
git push origin master

# connect repository to travis to deploy

Once everything is deploying, the fix is simply to add the version constraint.
awatson1978/minimongo-table@3ab4bbc

@arunoda

This comment has been minimized.

arunoda commented Feb 6, 2015

Okay. I've a reproducible repo here. Yeah!
Here it is: https://github.com/meteorhacks-samples/meteor-downgrade-issue

This is how to reproduce it.

  • Start a Ubuntu 14.04 (We tested on Digital Ocean)
  • uname -a: Linux abc 3.13.0-43-generic #72-Ubuntu SMP Mon Dec 8 19:35:06 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
  • Just clone this repo
  • run it with meteor

You'll get this message:

=> Started proxy.
=> Started MongoDB.

Changes to your project's package version selections:

meteorhacks:kadira  downgraded from 2.17.5 to 2.17.0

=> Started your app.

=> App running at: http://localhost:3000/
@arunoda

This comment has been minimized.

arunoda commented Feb 6, 2015

Again with testing. After I updated .meteor/packages with meteorhacks:kadira@=2.17.5 fixes this issue. But I think we should avoid @= as possible as we can.

@glasser

This comment has been minimized.

Member

glasser commented Feb 6, 2015

I can replicate this on OSX, though strangely it gives me a downgrade to 2.17.4. @dgreensp do you see this? Does your branch give better behavior?

@glasser

This comment has been minimized.

Member

glasser commented Feb 6, 2015

... and now it stopped happening, after it happening repeatedly a few times. This is definitely bizarre and a bug.

The only thing I can imagine is, maybe my local DB did not know about 2.17.5 at the time it did the original run? And this is basically a side effect of fixing #2846. I think it's definitely the case that if an unknown version is mentioned in .meteor/packages, we don't consider this a hard "we refuse to let anything work" error. But maybe it should cue a refresh and maybe it doesn't.

But surely I have synced my local DB in the past few days since you published 2.17.5?

@glasser

This comment has been minimized.

Member

glasser commented Feb 6, 2015

Though yeah, this does make sense, if you can reproduce it from a clean install of meteor (which ships with a database that is accurate as of the time the last release was published) but not otherwise.

@arunoda

This comment has been minimized.

arunoda commented Feb 6, 2015

Yes. I reproduce this with a clean meteor installation. I just created VM and installed meteor.

@glasser

This comment has been minimized.

Member

glasser commented Feb 6, 2015

Yep, this is definitely the problem.

@glasser glasser changed the title from Downgrading packages due to no reason to unknown package or version in .meteor/versions should trigger a refresh (but not otherwise cause an error) Feb 6, 2015

@arunoda

This comment has been minimized.

arunoda commented Feb 6, 2015

Good luck on the fix :)

@glasser

This comment has been minimized.

Member

glasser commented Feb 6, 2015

OK, I'm mostly focused on Mongo today, but I think the proper fix is, in _resolveConstraints in project-context.js, after reading cachedVersions, doing something like catalog.refreshIfAnyVersionsUnknown(cachedVersions).

Where this function is defined as: if catalog.triedToRefreshRecently || catalog.official.offline, return. Otherwise loop through all the cached versions and make sure they are in the catalog. If not, refresh the catalog... which unfortunately there isn't currently a function for which doesn't either expect to be called in buildmessage or to be able to warn to the console (and ProjectContext shouldn't be directly warning). (In general, there's a problem that buildmessage has no way of printing optional warnings like this.)

@arunoda

This comment has been minimized.

arunoda commented Feb 6, 2015

Okay, if I found someone into this issue, I'll ask him to use @=. Do you have any other workaround?

@glasser

This comment has been minimized.

Member

glasser commented Feb 6, 2015

Oh, probably "these things were unknown in .meteor/versions, even after refreshing, so we ignored them" or "these things were unknown in .meteor/versions, so we ignored them (without refreshing because catalog.official.offline)" should be things we can tell the PackageMapDelta constructor when you call it and which are printed by displayOnConsole.

@glasser

This comment has been minimized.

Member

glasser commented Feb 6, 2015

@arunoda @ should be good enough, but the simplest answer is "just run meteor refresh first".

I mean, to be honest though: If your project depends on a minimum version of a (non-core) package, it should include that in .meteor/packages (as a foo@1.2.3 constraint).

The semantics are:

.meteor/packages are the rules that your project must follow.

.meteor/versions are defaults to help choose between many possible correct ways to fall within the rules.

But if something really matters, it should be in the rules, not in the suggestions.

@glasser

This comment has been minimized.

Member

glasser commented Feb 6, 2015

BTW, to whoever works on fixing this issue: another possibility is that we could just consider "something unknown in .meteor/versions" as a hard error, and the version solver should just refuse to continue in this case. (This will properly trigger the "refresh and try again" because any version solver error gets tagged by project-context.js with "refreshCanHelp".)

The downside here is that now users need to be told to go in and manually edit this file and... do what? delete a line and hope for the best?

@arunoda

This comment has been minimized.

arunoda commented Feb 6, 2015

@glasser meteor refresh worked great. I'll use it. Thanks.

@dgreensp

This comment has been minimized.

Contributor

dgreensp commented Feb 9, 2015

I'm working on fixing this.

Steps I'm using to reproduce: Publish a new version of a package from a different computer. In an app on your main computer, edit .meteor/versions to refer to the new version of the package. Then run the app, without running any meteor commands in the mean time that could refresh the catalog. Meteor will downgrade the package to a version it knows about, and will not refresh the catalog.

Besides the benefit of reproducible runs, it's important for us to deliver the versions in .meteor/versions because those are the versions you've written your app against. (Concretely, if your app is using version 1.5 of a package, it may be using features that are not present in 1.4.) In theory you could maintain version constraints in your app's .meteor/packages that make .meteor/versions just a set of hints -- so that the latter could be ignored or regenerated without any chance of breaking the app -- but if that's the intended model, Meteor should just automate copying the versions for you from one file to the other. When I asked Geoff about this, he said keeping the versions in both places would be redundant. .meteor/versions really is there to be a record of what API your app is written against, and that's why it's checked into the repo alongside .meteor/packages.

I'm going to be putting out a preview release of the new version solver tonight or tomorrow morning, and I'll include at least a preliminary fix for this.

@dgreensp

This comment has been minimized.

Contributor

dgreensp commented Feb 10, 2015

Fix is included in meteor --release METEOR@new-version-solver-2.

@dgreensp

This comment has been minimized.

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