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

Plugin system #491

Merged
merged 26 commits into from Oct 23, 2011
Merged

Plugin system #491

merged 26 commits into from Oct 23, 2011

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Sep 27, 2011

Here it is. Believe it or not...

I've merged in Etienne's things, and added a few changes of my own.

What I suggest we do now is

a) Check over all the commits. Etienne's and mine included (@tiennou - you might like to look over mine, esp. the qs-push-plugin script ones)
b) Merge and build this
c) Play around with it to make sure you can pull the list of plugins from qsapp.com, you don't get any errors when you have no internet etc., can download new plugins, can update existing plugins (I suggest you download some old ones from http://qsapp.com/plugins/archived then try to update them), anything else you can think of
d) Play around with this on a clean install (I have not done this yet)
e) check plugin installing from the setup wizard (not tested)

A few things of note:

  • The qs-push-plugin isn't very clever when you upload an updated plugin. i.e. it just uploads the new one and leaves the old one on the server (it should check to see if the requirements are the same and if it should delete it)
  • The web interface at http://qsapp.com/plugins.php screws up when there's more than one plugin uploaded with the same ID (direct result of the point above)
  • I have not tested any version requirements things, but this will not be required for perhaps a few versions

Good luck everyone with testing. It feels so strange to actually send out a pull request on this... should it ever happen?!

tiennou and others added 16 commits Aug 14, 2011
Provide an error message if installation fails (eg. when you send text instead of a dmg file).
Additionally, allow the update to run in "silent mode". QS will restart automatically as soon as an update is installed.
This means people who have secret plugins installed can't access the plugin repository before this version.
It never worked correctly. Reachability only checks if a given host *could* be reached, not if it *can* be reached. So it's better to just try checking for updates anyway.
Reduced the wait time for a server response when fetching plugin info to as not to leave QS hanging (line 168)
Removed some commented lines (line 172ish)
Added some task statuses when downloading plugins. I'm not sure if they do anything (the blue status bar doesn't fill) (lines 240, 287)
Alerts and logs (most other lines)
…alled

Only try to delete the old (existing) if it actually exists
@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Sep 27, 2011

Hi Patrick,

great to see you being back from your vacation and kicking things into high gear. :-)

Just to be clear: This does include requirements checking on the application side, even if it is somehow screwy on the web interface? So it wont show me the old AirPort module, when I'm using QS on SL, but instead it will show me the new one?

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Sep 27, 2011

Hi there :)

From what I've gathered from Etienne's work, the only version checking that exists in this system is for plugin versions. There's nothing within the application to check if plugins are compatible with certain OSes.

This isn't ideal, but as it stands:

This system will be for QS v61+ which is 10.6+ only. If/When we get plugins that only work with 10.6 or 10.7, that's when we could start thinking about this... hopefully we'd get round to it before that.

For the time being, we'd have a working plugins system, a new QS version out the door, and hopefully a lot less questions from lost users :)

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Sep 27, 2011

hmmm...so there's no version compatibility checking. Neither for QS version nor for OS version, right? Ok, so we just have to be sure that we only publish SL+ plugins this way. And that we have a way to extend the update protocol without to much trouble, once it's needed.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Sep 28, 2011

I have tested the QS version requirements part by artificially setting QS version requirements for plugins. It works as expected - the plugins will not show up in the plugins preference pane.

Reading Etienne's first post in the dev groups:

  • This is some kind of mixture between database-driven and filesystem-driven. The database contains everything the system needs to provide latest versions of plugins, depending on various criteria (the QS version that is updating, the OS it's running on, secret plugins installed (TODO ;-))). The things that are of no use to the plugin system are moved in a qsinfo file (this is plist), a copy of the plugin's Info.plist file.

Leads me to believe he has thought about OS version, but QS never sends the OS version to the server, and there's nothing in the databases for it so I don't think it works.

ASIDE:

For those with some PHP skills:
the qs0.qsapp.com/plugins/admin/update.php script doesn't work properly.
It's basically because of line 307 in /lib/plugin.class.php - there is no $prop variable.

If somebody knows how to propogate $prop to be an array containing the fieldnames of the SQL table then step through them that'd be great.
Access to QSApp.com can be given if you don't already have it

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Sep 28, 2011

OK the database does include minHostVersion and minOSVersion (as well as max) so the server side stuff looks like it can run fine on this.

@skurfer
Copy link
Member

@skurfer skurfer commented Sep 28, 2011

the qs0.qsapp.com/plugins/admin/update.php script doesn't work properly.
It's basically because of line 307 in /lib/plugin.class.php - there is no $prop variable.

If somebody knows how to propogate $prop to be an array containing the fieldnames of the SQL table then step through them that'd be great.

So it should get a list of fields from the database instead of using the hard-coded $fields array? Or did someone already fix this?

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Sep 28, 2011

So it should get a list of fields from the database instead of using the
hard-coded $fields array? Or did someone already fix this?

I kind of already tried playing with this, and almost got it to work, but
for some reason $this->$dirtyProperties add an extra 'name' field at their
end (you see $dirtyProperties at the top of the update.php page, they're
getting printed there atm)

I was at a loss so stopped there.

On 28 September 2011 16:27, Rob McBroom <
reply@reply.github.com>
wrote:

the qs0.qsapp.com/plugins/admin/update.php script doesn't work properly.
It's basically because of line 307 in /lib/plugin.class.php - there is
no $prop variable.

If somebody knows how to propogate $prop to be an array containing the
fieldnames of the SQL table then step through them that'd be great.

So it should get a list of fields from the database instead of using the
hard-coded $fields array? Or did someone already fix this?

Reply to this email directly or view it on GitHub:
#491 (comment)

@skurfer
Copy link
Member

@skurfer skurfer commented Sep 29, 2011

I’ve tested D and E. Everything seems to work as is should. There is a possible issue with the preference NIB. In the other (clean) account, I can’t enable the Dock icon. (It’s off and the checkbox is greyed out.) It’s fine in my main account though, so I need to do some more checking.

@skurfer
Copy link
Member

@skurfer skurfer commented Sep 29, 2011

Clean install from master has the Dock icon enabled, but you still can’t change the setting in Preferences. Weird, but clearly not the fault of this change.

UPDATE: Since the Dock icon business requires modifying the application bundle, you’d need to be an admin user. That’s why the setting is disabled for the other account.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Sep 29, 2011

For some reason this email hasn't shown up here:

Only thing I can think of is that in order to change the Dock icon setting you have to change QS.app/Contents/Info.plist
Your 'clean' account (I'm assuming you mean Mac account not QS?) may not have the right permissions (write)?

I'm thinking it's greyed out because QS has already done the check to see if you can edit QS.app and seen that you can't (that's how it is if you run QS from the .dmg - no write permissions)

Good about D and E. I'll do a few more tests, and hopefully another person will.

The web side of things is pretty fragile - I wouldn't be confident with many people other people uploading plugins as it breaks quite a bit. It may be worth us fixing those first

These include:

a) Uploading a plugin to replace an older one using the rb script: The script should check to see if min/maxHostVersion and min/maxOSVersion are the same for the new and old plugins. If so, it should delete the old one.
b) When two plugins exist, qsapp.com/plugins.php messes up. I'm not sure if the list in QS does either, but I'm assuming it does. THOUGHT: need to test this in QS.
c) I'm not including the update.php page in the list of things to fix - I don't think we should actually use this. Editing the database but NOT the plugin's .plists isn't good.

Believe it or not, but some magic {(my PHP is pretty much non-existant) I've managed to fix a)
We just have b) to go now then we'll be good to go!

Please let there be someone better than me at PHP. @skurfer ? @HenningJ ?, @tiennou ? - @...anyone?! :P

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Sep 29, 2011

Only thing I can think of is that in order to change the Dock icon setting
you have to change QS.app/Contents/Info.plist
Your 'clean' account (I'm assuming you mean Mac account not QS?) may not
have the right permissions (write)?

I'm thinking it's greyed out because QS has already done the check to see if
you can edit QS.app and seen that you can't (that's how it is if you run QS
from the .dmg - no write permissions)

Good about D and E. I'll do a few more tests, and hopefully another person
will.

The web side of things is pretty fragile - I wouldn't be confident with many
people other people uploading plugins as it breaks quite a bit. It may be
worth us fixing those first

These include:

a) Uploading a plugin to replace an older one using the rb script: The
script should check to see if min/maxHostVersion and min/maxOSVersion are
the same for the new and old plugins. If so, it should delete the old one.
b) When two plugins exist, qsapp.com/plugins.php messes up. I'm not sure if
the list in QS does either, but I'm assuming it does. THOUGHT: need to test
this in QS.
c) I'm not including the update.php page in the list of things to fix - I
don't think we should actually use this. Editing the database but NOT the
plugin's .plists isn't good.

On 29 September 2011 20:56, Rob McBroom <
reply@reply.github.com>wrote:

Clean install from master has the Dock icon enabled, but you still cant
change the setting in Preferences. Weird, but clearly not the fault of this
change.

Reply to this email directly or view it on GitHub:
#491 (comment)

@tiennou
Copy link
Member

@tiennou tiennou commented Oct 3, 2011

IRC time ! ;-)

@tiennou
Copy link
Member

@tiennou tiennou commented Oct 3, 2011

For the record : plugins identifiers can appear multiple time because you have to handle multiple version of a plugin, e.g. the Airport Module vX works in 10.4 but not in 10.5/6/7, so there's another vY plugin for those. Technically, you get a Plugin with $plugin = Plugin::get(PLUGIN_IDENTIFIER, "a.plugin.identifier"); and then iterate through the $plugin->versionarray, looking for a plugin version that can be used with this QS/OS X/CPU. The plugin you get the first time is always the latest version available (that's Plugin::fetch responsibility to recreate the full Plugin stuff in one DB request).

You're right about the OS version check : this is TODO but doable, now that the User Agent contains OS X version. The harder part is fetching the current architecture and put it in the User Agent, so that the system knows which plugins can be used by the client. I have found this snippet lurking on the web that seems to do that, but I have yet to check its return values :

cpu_type_t GetProcessArchitecture(pid_t pid)
{
    cpu_type_t cputype;
    size_t cpusz = sizeof(cputype);

    // Default values
#if __i386__
    cputype = CPU_TYPE_X86;
#else
    cputype = CPU_TYPE_POWERPC;
#endif

    if (sysctlbyname_with_pid("sysctl.proc_cputype", pid, &cputype, &cpusz, NULL, 0) == -1) {
        fprintf(stderr, "proc_cputype: sysctlbyname_with_pid failed: %s\n", strerror(errno));
    }
    return cputype;
}

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 3, 2011

I've jumped on IRC. Not sure how long I can be on for: I've moved into a new
house and have no internet (except a friend's tethered smart phone)

The harder part is fetching the current architecture and put it in the
User Agent

Do we need to do this? We're not going to be building plugins with PPC, and
from what @neurolepsy and @skurfer have said about building 64 bit, a 32 bit
version of Quicksilver can run 32/64 bit compiled versions of the plugins
fine.

On 3 October 2011 10:55, Etienne Samson <
reply@reply.github.com>wrote:

For the record : plugins identifiers can appear multiple time because you
have to handle multiple version of a plugin, e.g. the Airport Module vX
works in 10.4 but not in 10.5/6/7, so there's another vY plugin for those.
Technically, you get a Plugin with $plugin = Plugin::get(PLUGIN_IDENTIFIER, "a.plugin.identifier"); and then iterate
through the $plugin->versionarray, looking for a plugin version that
can be used with this QS/OS X/CPU. The plugin you get the first time is
always the latest version available (that's Plugin::fetch responsibility to
recreate the full Plugin stuff in one DB request).

You're right about the OS version check : this is TODO but doable, now that
the User Agent contains OS X version. The harder part is fetching the
current architecture and put it in the User Agent, so that the system knows
which plugins can be used by the client. I have found this snippet lurking
on the web that seems to do that, but I have yet to check its return values
:

cpu_type_t GetProcessArchitecture(pid_t pid)
{
    cpu_type_t cputype;
    size_t cpusz = sizeof(cputype);

    // Default values
#if __i386__
    cputype = CPU_TYPE_X86;
#else
    cputype = CPU_TYPE_POWERPC;
#endif

    if (sysctlbyname_with_pid("sysctl.proc_cputype", pid, &cputype, &cpusz,
NULL, 0) == -1) {
        fprintf(stderr, "proc_cputype: sysctlbyname_with_pid failed: %s\n",
strerror(errno));
    }
    return cputype;
}

Reply to this email directly or view it on GitHub:
#491 (comment)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 14, 2011

I think I see the problem. It seems that on startup, QS must be getting mixed up with which plugin it should delete.

Previously, the old and new plugins would have the same name, but @ddl_smurf altered this to give the plugins the format: PluginID.Version.qsplugin, which I guess means that the old plugins are staying.

There must be a problem with the bit that sniffs for old plugins and deletes them. I have just run through the steps with my 1Password plugin, and QS did in fact delete the old plugin when QS relaunched, and kept the new one. Perhaps QS is deleting the wrong one with the Mail plugin.

I am not sure as to where this takes place (probably in QSPluginManager.m somewhere). I will hopefully be able to assess a bit more this evening.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 14, 2011

suggestOldPluginRemoval?

QSPluginManager.m:565

@tiennou
Copy link
Member

@tiennou tiennou commented Oct 14, 2011

Fixed the application download stuff. That's the part of the code that's the most messy, because I got distracted by the fact that QS supported various distribution formats (ditto/qspkg, dmg, zip). Right now it's fixed to qspkg for plugins, and dmg for app updates. Does that fit everybody ?

I'm on IRC btw

…vailable locally.

Now the system checks the local plugins instead of only plugins about to be loaded. The difference is that plugins that have dependencies don't get added to the about to be loaded list, and thus the first one found gets loaded (which might not be the latest).

Also, log an error in case an old plugin can't be removed.
@tiennou
Copy link
Member

@tiennou tiennou commented Oct 14, 2011

Pushed a commit to my branch that fixes the duplicate plugin problem. The system was failing to register duplicates if their dependencies weren't met, and loaded the first one unconditionally when the dependency would get resolved. Since the first one is usually the one with the lower version number, it would always end up loading the older version...

Example with Apple Mail Module (since I think my explanation is sketchy ;-)) :

  • QS starts, (I'm looking at -loadPlugInsAtLaunch) builds the list of local plugins, check each of them for their requirements, etc. Apple Mail Module vX gets discarded at that point because it depends on Email Support, but the dependency is registered. Same for the subsequent Apple Mail Module vY.
  • Then, QS loads Email Support (-plugInDidLoad:). It sees there are two plugins waiting on the dependency, it loads (-liveLoadPlugIn:) the first, but since it doesn't know anymore that there is another version (the argument to - plugInIsMostRecent:inGroup: is nil at this point), then fails to load the second one since there's already a loaded plugin with this identifier.
  • Since it never saw duplicates, they are never added to oldPlugins, and they never get deleted in -suggestOldPluginRemoval.

Hope that makes more sense ;-). Now, it has a few shortcomings : -suggestOldPluginRemoval removes stuff unconditionally (doesn't suggest anything at all, hmm ;-)). I can see a few issues with that, since plugins can come from a lot of difference places (see -allBundles for the search path), it could delete eg. the Core Module included in QS if it finds one with a higher version number somewhere else... Opinions ?

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 14, 2011

Once again, bd81fb3 is Etienne's work and not mine. I have not checked over it... yet :)

@skurfer
Copy link
Member

@skurfer skurfer commented Oct 14, 2011

I can see a few issues with that, since plugins can come from a lot of difference places (see -allBundles for the search path), it could delete eg. the Core Module included in QS if it finds one with a higher version number somewhere else

I don’t have a complete picture here, so I don’t know how complicated what I’m suggesting is, but perhaps the decision to delete should require both bundle ID and path to match?

I was going to say maybe we should never delete from /Library/Application Support/Quicksilver at all but I suppose if an admin user manually drops a new plug-in there, it makes sense to remove the old one.

I do think we should never delete from /Applications/Quicksilver.app, but if we check the path, the only way that could happen is if someone manually puts things there. In that case, they either know what they’re doing or deserve what they get (or both). :)

@tiennou
Copy link
Member

@tiennou tiennou commented Oct 14, 2011

I don’t have a complete picture here, so I don’t know how complicated what I’m suggesting is, but perhaps the decision to delete should require both bundle ID and path to match?

At this moment the plugin is deleted, we have a complete plugin object with version et al. Adding a whitelist at this point would be easy enough. My opinion was a dialog asking the user with a "Always delete silently" checkbox, but I guess it would require a nib blah blah. I think QS handles translations on the fly, so maybe...

I discussed it a little on IRC with Patrick, and we agreed that deleting wasn't really appropriate in some places, like /Applications/Quicksilver.app. It also poses problems if someone use Dropbox to sync its QS configuration with different QS versions, but that is highly doubtful.

Handling that correctly would require adding min/max checking to QS (which I thought was, but it only checks for min).

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 16, 2011

I think a dialogue would be too far and require too much user input. Plus, as you've said – QS seems to handle things pretty well at the moment already :)

I think the business with different QS versions requiring different plugin versions is also far-fetched (now I've thought about it) so I don't think we have too much to worry about on that front.

I will test the 'two mail plugins installed' bug that Etienne says he's now fixed, then I'm happy with all of this :D

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 17, 2011

OK a few notes/bugs:

  • Fresh install works great. No problems downloading plugins. Nor with concurrent downloading - great!
  • asOfDate in the info.php? URL doesn't seem to work 100%. If I change something in qs0.qsapp.com/plugins/admin it doesn't show it changed for an URL such as: http://qs0.qsapp.com/plugins/info.php?asOfDate=20111017184255&qsversion=14416&sids=com.blacktree.Quicksilver.QSEventTriggersPlugIn
  • Not sure how to upload new QS.dmg - and then what URL they will point to on GitHub. Using the qs-push-script? Has it been modified?
  • Can we disable all the verbose logging - when this system goes live that's going to get crazy!
  • The version business is still confusing me. Looking in the SQL. It says the mail module has a version no. of 280, whereas the plugin itself (in QS and the Info.plist) says 118 :/
  • /plugins/admin/index.php only shows the latest plugin (if there are multiple versions in the system) - surely it should show all of them?
  • Downloading an app update gives me "Downloading 1000k of -0k" (i.e. the filesize should display)
  • File wasn't being downloaded from GitHub
  • The install of a newer Apple Mail module works fine - the older version is deleted.
  • Clicking 'delete' in the /admin/index.php page doesn't seem to work (at least, for when there are multiple plugins)
  • Not sure if 'delete' is also deleting the files or just the database reference.

Other than that, it all looks good! :D

I'm on IRC now if you'd like to talk!

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 19, 2011

OK I have fixed:

Clicking 'delete' in the /admin/index.php page doesn't seem to work (at least, for when there are multiple plugins)
asOfDate in the info.php? URL doesn't seem to work 100%

Problems I can't sold (perhaps @tiennou ?!)

  • Not sure how to upload a new QS.dmg file, and how to mirror it on GitHub. Has the qs-push-plugin script been updated?
  • version business still confuses me
  • Downloading an app update gives me "Downloading 1000k of -0k" (i.e. the filesize should display)

Final push! :D

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 21, 2011

Seems like the functions.php (send_file function) is sending the right file size with the content-length header, but for some reason it isn't getting through to QS

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 21, 2011

OK so after much testing it seems like it's QS's fault (B60 works fine). I'm currently running a git bisect to see where the problem lies.

As for the other two problems:

  • I recall Etienne explaining the version business
  • I see I can upload .dmg from the online admin interface. The GitHub URL I see is just as defined in /lib/plugin.class.php (see global defined) with a filename "Quicksilver Bxx.dmg"

Almost done the git bisect now... :)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 21, 2011

OK.... so the bad commit IS the `Application update paths' commit.

This means it's a problem with the remote PHP end and NOT Quicksilver.

@tiennou - would you be able to shed any light on this? I've checked to see that the Content-Length is getting set correctly, but where could it be going wrong?

I've even tried a <meta redirect> but that didn't work :(

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 21, 2011

...OK finally!

So it's no show stopped. It seems to be a problem with just QSApp.com not giving out the data. If I manually change the location to point to GitHub it works beautifully. this'll be the case for all end-users, so we don't have to worry.

I've submitted a support ticket to the hosts none-the-less.

Right:
@skurfer and @tiennou (sorry for all the emails)
ARE WE READY TO GO!? XD

@skurfer
Copy link
Member

@skurfer skurfer commented Oct 21, 2011

I have to say I’ve only tested this briefly. There’s a merge conflict with the featureLevel branch, and I’d rather keep that merged in so I can make sure it’s stable. (It appears to be.) The conflict is in a XIB, so it’s nearly impossible to resolve in a text editor.

Anyway, I’ll try to test it out over the weekend, but really, my priority now is getting repos set up for the new plug-ins. (I’m currently running an iTunes plug-in that I built. Unbelievable.)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 22, 2011

my priority now is getting repos set up for the new plug-ins.

...could you make testing this and giving it the 'ok' the priority for one
day...? :)

Once we've got this merged and the next version the world is absolutely 100%
our oyster!

If not, then I'd say that I could merge it, seeing as 99% of the work is
@tiennou's, and I've just issued the pull request.

I can't wait to get this out!

On 21 October 2011 20:44, Rob McBroom <
reply@reply.github.com>wrote:

I have to say Ive only tested this briefly. Theres a merge conflict with
the featureLevel branch, and Id rather keep that merged in so I can make
sure its stable. (It appears to be.) The conflict is in a XIB, so its
nearly impossible to resolve in a text editor.

Anyway, Ill try to test it out over the weekend, but really, my priority
now is getting repos set up for the new plug-ins. (Im currently running an
iTunes plug-in that I built. Unbelievable.)

Reply to this email directly or view it on GitHub:
#491 (comment)

@skurfer
Copy link
Member

@skurfer skurfer commented Oct 22, 2011

The whole point of my last message (which I forgot to include) was “don’t wait for me” because you guys have more knowledge of what’s going on than I do and my testing would just be icing on the cake. If you feel good about it, go for it.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 22, 2011

Sorry, didn't realise that's what you meant! Sounds good though :)
I'll have a final play on it today. (I actually found another bug yesterday)
but it would be good if you could

  • Build from plugins-system, run it from wherever you usually run your QS
    app from, and click 'now' to check for updates. You should get a download
    from GH (from a private repo I've set up) that is just a bumped version of
    'master' with 'plugins-system' merged in.
  • Run with this version (it's near identical to what we'll be releasing) for
    a teeny bit
  • Have a quick look through the 'changelog' pull request and see if there's
    anything I've forgotten :) If not - merge it!

If you're too busy/don't feel like it, I'll merge and go ahead with it all!
:)

On 22 October 2011 05:26, Rob McBroom <
reply@reply.github.com>wrote:

The whole point of my last message (which I forgot to include) was dont
wait for me because you guys have more knowledge of whats going on than I
do and my testing would just be icing on the cake. If you feel good about
it, go for it.

Reply to this email directly or view it on GitHub:
#491 (comment)

@tiennou
Copy link
Member

@tiennou tiennou commented Oct 22, 2011

Gah, more and more questions ! Here are a few answers (from a little while ago, sorry). (IRC time if you want, too ;-)). :

@pjrobertson :

Hmm... I'll double-check that, but saving a plugin through the admin pages ought to update its timestamp. Will check that...

  • Not sure how to upload new QS.dmg - and then what URL they will point to on GitHub. Using the qs-push-script? Has it been modified?

qs-push-plugin hasn't been updated yet, but it works through the add page if you check Application Update and provide the dmg and Info.plist.

  • Can we disable all the verbose logging - when this system goes live that's going to get crazy!

Yes, there's a logging level for that. Right now it defaults to DEBUG, but I plan on changing it to ERROR before publicizing this.

  • The version business is still confusing me. Looking in the SQL. It says the mail module has a version no. of 280, whereas the plugin itself (in QS and the Info.plist) says 118 :/

The Info.plist are wrong. The REAL, integer base 10 number is the one in the database. Everything else lies ;-).

  • /plugins/admin/index.php only shows the latest plugin (if there are multiple versions in the system) - surely it should show all of them?

Right now you can access older versions by using the links on a plugin's update page. But a direct link ought to be nicer...

  • Downloading an app update gives me "Downloading 1000k of -0k" (i.e. the filesize should display)

I'm checking... Okay, this one is hard ;-). It seems the qsapp.com host gz-compress the downloaded plugin on the file. I'm sending the Content-Length with the correct file size set, but the host deletes it afterwards, since it recompress the qspkg file... That explains why I don't get the same filesize value from PHP and at download time.
And then IIRC, a long time ago I disabled the thing that gets the Content-Length header inside QS because it used private stuff that broke. It seemed better to keep the plugin system working at the expense of that number. (Hint, look around QSURLDownload for that part).

  • File wasn't being downloaded from GitHub

Plugins are quite small. I'm not sure about how to handle it on GitHub without a binary git repository...

  • The install of a newer Apple Mail module works fine - the older version is deleted.

Cool :p

  • Clicking 'delete' in the /admin/index.php page doesn't seem to work (at least, for when there are multiple plugins)

It works, but you won't get a direct confirmation that the deletion had taken place. Note also that an older version of the plugin will be shown if available, so it could look like nothing was deleted.

  • Not sure if 'delete' is also deleting the files or just the database reference.

It also delete uploaded files (both qspkg/dmg and qsinfo).

Note, this looks broken at the moment. I can't download a plugin from the qsapp.com/plugins.php page (the download.php page sends me the QS dmg everytime...). EDIT Found out about that ;-).

@skurfer
Copy link
Member

@skurfer skurfer commented Oct 22, 2011

OK, got it built, it saw the update. I removed a plug-in and redownloaded it. I got the correct (newer) version. The interface still showed old versions for plug-ins that weren’t installed, but a refresh fixed that. All seems to work so far. Won’t be able to check much else until later tonight (EST).

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 22, 2011

Great - all sounds good :)

I've just upgraded to Lion (finally - had to delete my bootcamp partition),
so will fiddle with XCode 4.
Once I'm happy, I'll try and build it and away we go!

On 22 October 2011 21:34, Rob McBroom <
reply@reply.github.com>wrote:

OK, got it built, it saw the update. I removed a plug-in and redownloaded
it. I got the correct (newer) version. The interface still showed old
versions for plug-ins that werent installed, but a refresh fixed that. All
seems to work so far. Wont be able to check much else until later tonight
(EST).

Reply to this email directly or view it on GitHub:
#491 (comment)

pjrobertson added a commit that referenced this issue Oct 23, 2011
@pjrobertson pjrobertson merged commit 196b1fe into quicksilver:master Oct 23, 2011
@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 1, 2011

For those whom it may concern:

I've moved the 'delete' button from the main index.php admin page to within
the 'edit' page.
I inadvertently clicked and deleted one of the plugins yesterday, so
thought this'd be a good safety measure. It also means you can delete any
version of any plugin.

On 22 October 2011 21:43, Patrick Robertson robertson.patrick@gmail.comwrote:

Great - all sounds good :)

I've just upgraded to Lion (finally - had to delete my bootcamp
partition), so will fiddle with XCode 4.
Once I'm happy, I'll try and build it and away we go!

On 22 October 2011 21:34, Rob McBroom <
reply@reply.github.com>wrote:

OK, got it built, it saw the update. I removed a plug-in and redownloaded
it. I got the correct (newer) version. The interface still showed old
versions for plug-ins that werent installed, but a refresh fixed that. All
seems to work so far. Wont be able to check much else until later tonight
(EST).

Reply to this email directly or view it on GitHub:
#491 (comment)

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

Successfully merging this pull request may close these issues.

None yet

4 participants