HaxeFoundation / haxelib

The Haxe library manager
https://lib.haxe.org/
MIT License
173 stars 78 forks source link

is the install counter on lib.haxe.org working? #253

Closed nadako closed 8 years ago

nadako commented 8 years ago

Subj. Because e.g. for hxnodejs it shows 0 installs, though I downloaded it myself at least once.

markknol commented 8 years ago

@jasononeil @ncannasse bump.

What is needed to fix this? I can imaging it's very discouraging for library authors to see the download count stay the same. See http://lib.haxe.org/search/?v=hxnodejs

The popularity list also does not change because of this.

markknol commented 8 years ago

+1

nadako commented 8 years ago

I'll look into it.

ibilon commented 8 years ago

I launched my local haxelib server and the download counter is not 0, strange.

ibilon commented 8 years ago

So not all version of the library can have their counter increase, and I get a invalid field access : index if I trace the error, tacked it to https://github.com/HaxeFoundation/haxelib/blob/development/src/haxelib/server/Repo.hx#L319-L326

markknol commented 8 years ago

You will never notice because the function seems to be try/catched https://github.com/HaxeFoundation/haxelib/blob/development/src/haxelib/client/Main.hx#L769

nadako commented 8 years ago

This seems to be related to the preview/previewNum being null and passed to the SPOD query. Gotta dig into SPOD for that.

ibilon commented 8 years ago

after removing the try/catch ;)

For a lib without preview and previewNum: it's the preview field, commenting it prevent the crash but doesn't find the version, commenting also previewNum "works", but not on the right version.

ibilon commented 8 years ago

Well if you're ok with hacks you can do:

var version = SemVer.ofString(version);
var vl = Version.manager.search(
    $project == p.id &&
    $major == version.major &&
    $minor == version.minor &&
    $patch == version.patch
);

var v = null;
for (ve in vl)
    if (ve.preview == version.preview && ve.previewNum == version.previewNum)
        v = ve;

if( v == null )
    throw "No such Version : " + version;
v.downloads++;
v.update();
p.downloads++;
p.update();

it works, but that's sad to do part of a select manually.

nadako commented 8 years ago

It generates wrong query expression for the enum, it doesn't respect that it's nullable and tries to get Type.enumIndex(preview) which fails with the invalid field access. This may be related to 6d41c431e4d3056d4a8269c4aedc46acb408cafe

nadako commented 8 years ago

Well if you're ok with hacks you can do

another version is to construct the SQL manually, but I'd like to look into fixing RecordMacros first

nadako commented 8 years ago

SPOD is a real mess, see https://github.com/HaxeFoundation/haxe/issues/4932 and https://github.com/HaxeFoundation/haxe/issues/4931

I don't feel like messing with RecordMacros at the moment, so I'll just hand-craft the SQL here...

nadako commented 8 years ago

I cherry-picked the fix to the master branch which should be auto-deplyed from Travis, let's see if it works...

nadako commented 8 years ago

It works \o/

markknol commented 8 years ago

This is great! Is it online yet?

nadako commented 8 years ago

Yeah it's online!

markknol commented 8 years ago

It works, I tried with a lib that had no downloads yet, now it has 1.

markknol commented 8 years ago

Are installs from CI also count as download count? Can we exclude them? Otherwise it might be a bit unfair.

nadako commented 8 years ago

We could check for travis/appveyor/whatever env vars and skip invocation of postInstall