[2.0] Auto upgrade manager [developer needed]

Mark threads with "[2.0]" for discussions about features in the longer-term future, "[1.6]" is for short-term. This is not the place for general discussions or plugin or template requests. Only features that are approved to happen by the core team should be listed here for better structuring.
User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: [2.0] Auto upgrade manager [developer needed]

Post by Timbalu » Sun Jan 16, 2011 3:04 pm

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 +.

User avatar
onli
Regular
Posts: 2230
Joined: Tue Sep 09, 2008 10:04 pm
Contact:

Re: [2.0] Auto upgrade manager [developer needed]

Post by onli » Sun Jan 16, 2011 5:18 pm

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.

User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: [2.0] Auto upgrade manager [developer needed]

Post by Timbalu » Sun Jan 16, 2011 7:04 pm

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

Code: Select all

      error_reporting(0);
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

User avatar
onli
Regular
Posts: 2230
Joined: Tue Sep 09, 2008 10:04 pm
Contact:

Re: [2.0] Auto upgrade manager [developer needed]

Post by onli » Sun Jan 16, 2011 7:18 pm

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 ;)

User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: [2.0] Auto upgrade manager [developer needed]

Post by Timbalu » Mon Jan 17, 2011 11:47 am

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

User avatar
onli
Regular
Posts: 2230
Joined: Tue Sep 09, 2008 10:04 pm
Contact:

Re: [2.0] Auto upgrade manager [developer needed]

Post by onli » Mon Jan 17, 2011 1:29 pm

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.

User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: [2.0] Auto upgrade manager [developer needed]

Post by Timbalu » Mon Jan 17, 2011 2:22 pm

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

User avatar
onli
Regular
Posts: 2230
Joined: Tue Sep 09, 2008 10:04 pm
Contact:

Re: [2.0] Auto upgrade manager [developer needed]

Post by onli » Mon Jan 17, 2011 3:23 pm

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>';

User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: [2.0] Auto upgrade manager [developer needed]

Post by Timbalu » Mon Jan 17, 2011 3:41 pm

hmmm very curious
I have tested that and now did it again
but - no result (no button)

Ian

User avatar
onli
Regular
Posts: 2230
Joined: Tue Sep 09, 2008 10:04 pm
Contact:

Re: [2.0] Auto upgrade manager [developer needed]

Post by onli » Mon Jan 17, 2011 11:13 pm

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.
Attachments
serendipity_event_dashboard.tar.gz
(11.21 KiB) Downloaded 295 times
serendipity_event_autoupdate.tar.gz
(2.53 KiB) Downloaded 293 times

User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: [2.0] Auto upgrade manager [developer needed]

Post by Timbalu » Tue Jan 18, 2011 11:10 am

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

User avatar
onli
Regular
Posts: 2230
Joined: Tue Sep 09, 2008 10:04 pm
Contact:

Re: [2.0] Auto upgrade manager [developer needed]

Post by onli » Fri Jan 21, 2011 2:01 pm

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.

User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: [2.0] Auto upgrade manager [developer needed]

Post by Timbalu » Wed Jan 26, 2011 6:20 pm

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

User avatar
onli
Regular
Posts: 2230
Joined: Tue Sep 09, 2008 10:04 pm
Contact:

Re: [2.0] Auto upgrade manager [developer needed]

Post by onli » Wed Jan 26, 2011 6:39 pm

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.

User avatar
garvinhicking
Core Developer
Posts: 30014
Joined: Tue Sep 16, 2003 9:45 pm
Location: Cologne, Germany
Contact:

Re: [2.0] Auto upgrade manager [developer needed]

Post by garvinhicking » Wed Jan 26, 2011 7:55 pm

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
# Garvin Hicking (s9y Developer)
# Did I help you? Consider making me happy: http://wishes.garv.in/
# or use my PayPal account "paypal {at} supergarv (dot) de"
# My "other" hobby: http://flickr.garv.in/

Locked