Page 3 of 5
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Sun Jan 16, 2011 2:04 pm
by Timbalu
Hi onli
Dashboard 0.6.1 with PHP 5.3.3
Deprecated: Call-time pass-by-reference has been deprecated in .../serendipity_event_dashboard/serendipity_event_dashboard.php on line 468 with depending 'Cannot modify header information - headers already sent' warnings in functions_config.inc.php and serendipity_admin.php
change
Code: Select all
serendipity_plugin_api::hook_event('plugin_dashboard_updater', $nv, &$addData);
to
Code: Select all
serendipity_plugin_api::hook_event('plugin_dashboard_updater', $nv, $addData);
Ian
Edit:
I downloaded dashboard and installed it, then downloaded autoupdate plugin - but there is still the same link in dashboard which leeds to sf.net - no other link or configure autoupdate available... this is not working as expected -descending order is dashboard and last autoupdate. Any ideas?
Edit2:
adding back the & to $addData shows the button lastly - but this has to be made compat to PHP 5.3.3 and +.
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Sun Jan 16, 2011 4:18 pm
by onli
Well, it is necessary to have the &$nv because the other plugin, autoupdater, changes $nv and thus adds the link. I am not a php-expert and not familiar with another method to achieve this. We could probably change &addData to &$addData in the function-definition in the plugin-Api, but that is nothing what my plugin could do.
PS: This method of chaning variables elsewhere is used in other places in the core of serendipity as well. I hope the & there wasn't simply removed tu suppress the warning? That would break quite some plugins, not only some of mine.
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Sun Jan 16, 2011 6:04 pm
by Timbalu
Jeah, but I was pointing to the &$addData, not the $nv.
To pass method calls by reference the only way to this in php 5.3up ist to put them into the function eg function foo(&$x) {}.
To change the API is not possible, since there are hooks like this
Code: Select all
serendipity_plugin_api::hook_event('backend_login', $is_authenticated, NULL);
If we can't find a cleaner way to be future compat for your
plugin, we could use
at plugin start, or use a
php.ini with
Code: Select all
allow_call_time_pass_reference = On
or add to serendipity
.htaccess
Code: Select all
php_value allow_call_time_pass_reference 1
All these possibilities are just dirty workarounds in my opinion, but will work until there is something better.
Ian
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Sun Jan 16, 2011 6:18 pm
by onli
Jeah, but I was pointing to the &$addData, not the $nv.
True, sorry, i mixed it up and meant $addData. $nv isn't changed later on nor should a change to $nv go back to the dashboard-plugin ;)
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Mon Jan 17, 2011 10:47 am
by Timbalu
Hi onli
I tried some things to get around the &$addData deprecated prob, but it seems the simplest solution is to change it this way.
in dashboard:
Code: Select all
if($this->compareVersion($nv, $serendipity['version'])){
// fake call to autoload event plugin to hook in generally
serendipity_plugin_api::hook_event('plugin_dashboard_updater', $nv);
print $this->showUpdateHeader();
$update_text = $this->get_config('update_text');
print $update_text . $addData;
print '</div>';
print '
<div class="dashboard dashboard_download">
<form action="?serendipity[adminModule]=event_display&serendipity[adminAction]=update" method="post">
<input type="hidden" name="serendipity[newVersion]" value="'.$nv.'" />
<input type="submit" value="Update now automatically" />
</form>
</div>';
}
return;
in autoload:
Code: Select all
case 'plugin_dashboard_updater':
// void
return true;
break;
PS: This method of chaning variables elsewhere is used in other places in the core of serendipity as well. I hope the & there wasn't simply removed tu suppress the warning? That would break quite some plugins, not only some of mine.
Maybe this kind of solution is practical for your other plugin making use of &$addData too.
I can't find any other reference warnings in core with PHP5.3.3.
Ian
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Mon Jan 17, 2011 12:29 pm
by onli
Hm? I don't get that code. Won't it always show the button no matter what the autoupdater-plugin does? I separated the updater-plugin and the dashboard-plugin because something like an updater don't belong into the dashboard-plugin, neither does the download-button itself.
There is an easy way around this :) Changes to $eventData go back. Simply exchanging eventData and addData, adding the button via eventData and submitting the version via addData, works.
That's probably why there were no other warnings: that method was used, but with eventData and thus without the leading &.
Thanks for reporting this bug.
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Mon Jan 17, 2011 1:22 pm
by Timbalu
onli wrote:Hm? I don't get that code. Won't it always show the button no matter what the autoupdater-plugin does?
Oh thats true, maybe...
I could not test going forward since this is my smarty3 testblog and I ran into errors with write/read perms.
onli wrote:There is an easy way around this
Changes to $eventData go back. Simply exchanging eventData and addData, adding the button via eventData and submitting the version via addData, works.
Yes, that is the way serendipity is handling event hooks.
How do you do it exactly, now? As I remember, I tried this too, but it didn't work as I expected to be.
Ian
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Mon Jan 17, 2011 2:23 pm
by onli
I really just switched the position:
Code: Select all
$nv = $this->get_config('last_version');
if($this->compareVersion($nv, $serendipity['version'])){
$eventData = '';
serendipity_plugin_api::hook_event('plugin_dashboard_updater', $eventData, $nv);
print $this->showUpdateHeader();
$update_text = $this->get_config('update_text');
print $update_text . $eventData;
print '</div>';
}
Code: Select all
case 'plugin_dashboard_updater':
$eventData = '<form action="?serendipity[adminModule]=event_display&serendipity[adminAction]=update" method="POST">
<input type="hidden" name="serendipity[newVersion]" value="'.$addData.'" />
<input type="submit" value="'.PLUGIN_EVENT_AUTOUPDATE_UPDATEBUTTON.'" />
</form>';
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Mon Jan 17, 2011 2:41 pm
by Timbalu
hmmm very curious
I have tested that and now did it again
but - no result (no button)
Ian
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Mon Jan 17, 2011 10:13 pm
by onli
Hopefully that's not an issue or a change of behavior in php 5.3 we are running into. Could you please test it again with the attached versions? Working at my system.
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Tue Jan 18, 2011 10:10 am
by Timbalu
Oh shit what a mess, I had them twice, so the autoupdate I edited, was the one in my download folder the whole time. I hate when that happens...!
Thats why all my test before with eventData went wrong...., naturally.
I have to tidy up my editor (got about 100+ tabs opened...) .... one day!
Sorry for causing inconvenience. Its allright now.
Ian
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Fri Jan 21, 2011 1:01 pm
by onli
Just commited the latest version to spartacus. The updater now compares the checksum of the archive with the one written at the downloadpage, and does an integrity-test also using checksums after unpacking the archive.
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Wed Jan 26, 2011 5:20 pm
by Timbalu
Which is a good idea!
Malte, did you read about this little problem with dependencies not auto installing, if the plugin isn't part of $classes and not delivered by core?
This has to get proved against the concept of seperating autoupdater from dashboard. Even Garvin seemed to think of making dashboard a core plugin including all notifiers and these upgrade functions.
Ian
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Wed Jan 26, 2011 5:39 pm
by onli
I dislike the idea of creating one giant plugin :/
In the end, it is garvins call. But i'd rather fix the dependency-system and add the dasboard-plugin+autoupdater+x as enabled plugins shipped with serendipity. Without fixed release-dates, it would be hard otherwise to update those plugins if they are core-pluging.
Re: [2.0] Auto upgrade manager [developer needed]
Posted: Wed Jan 26, 2011 6:55 pm
by garvinhicking
Hi!
Merging autoupdate and dashboard and putting it into s9y core is a longterm goal, if a goal at all.
In the short run, it would be great to work on the dependency system and check if it can be improved to utilize spartacus to download plugins that are not yet locally existing...
Regards,
Garvin