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

Error in detection of highest installed package version #34

Closed
cemrich opened this issue Feb 13, 2013 · 9 comments
Closed

Error in detection of highest installed package version #34

cemrich opened this issue Feb 13, 2013 · 9 comments
Labels
Bug Issues where something has happened which was not expected or intended
Milestone

Comments

@cemrich
Copy link
Contributor

cemrich commented Feb 13, 2013

Installed packages are read from the chocolatey lib dir. It looks like directories of old package versions are not removed (not sure if that's a bug or a feature). So ChocolateyLibDirHelper tries to guess which version is the highest one by string comparison of the version string. This works fine in following case:

  • git.install.1.8.0
  • git.install.1.8.1.2 <-- highest and reported as highest

And it fails for example in following case:

  • freefilesync.5.9 <-- reported as highest
  • freefilesync.5.11 <-- highest

cver freefilesync reports the correct version. But using this command for detecting installed versions would worsen performance a lot.

@chrissie1
Copy link
Contributor

there is a cver all -localonly but it seems slow. But does that do a better job?

I have an IComparer at work that could serve this purpose. Then it would become versionQuery.ToList().Sort(New VersionComparer).Last();

I'll if I can adapt the comparer to do what we need. And write the needed unittests.

@cemrich
Copy link
Contributor Author

cemrich commented Feb 13, 2013

cver all -localonly works fine. But it's really not very fast. Yes, a VersionComparer would work fine I think.

@chrissie1
Copy link
Contributor

added failing tests for this.

@tarwn
Copy link
Contributor

tarwn commented Feb 15, 2013

@chrissie1: Did you purposefully remove the file system service from the PackageManager during this commit? It looks like it was still there in your prior commit, but then it went away in this one and the dialog boxes during tests came back. I'd add it back in, but it looked like you might have done it on purpose and didn't want to get in a commit fight :)

@chrissie1
Copy link
Contributor

Nope thats was accidental, git messed up the merge, i guess, yeah it was git's fault

@gep13
Copy link
Member

gep13 commented Feb 15, 2013

FIGHT, FIGHT, FIGHT!!! :-)

@chrissie1
Copy link
Contributor

@cemrich Here is the gist to the comparer. Needs some more tests but I think it should work. Based on the full packagename. https://gist.github.com/chrissie1/4959638

tarwn added a commit that referenced this issue Feb 15, 2013
FileStorageService fell out during an earlier commit, added it back in per comments in #34
@tarwn
Copy link
Contributor

tarwn commented Feb 15, 2013

All fixed.

@chrissie1 are those 4 package name parsing tests added for this issue or a different one? I can fix them to properly parse out name, version, and pre but wouldn't want to step on any work being done to manage the sorting.

Also, should there have a been a link to the gist you just mentioned somewhere?

@chrissie1
Copy link
Contributor

The link is there. You need to look better.

I think they are unrelated. This one is about the fact that The .Last() call did not give the correct version because it does stringsorting. So I made the sorter to sort them correctly so that you can find the last version more easily. My sorter is based on the directoryname but it easily adapted to work on a Package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues where something has happened which was not expected or intended
Projects
None yet
Development

No branches or pull requests

4 participants