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 installation/upgrade fails with 2.1.2 #548

Closed
th-h opened this Issue Mar 28, 2018 · 9 comments

Comments

Projects
None yet
3 participants
@th-h
Copy link
Member

th-h commented Mar 28, 2018

See https://board.s9y.org/viewtopic.php?f=3&t=23821.

After upgrading to 2.1.2, attempting to install or upgrade a plugin yielded the error

The URL /additional_plugins/serendipity_event_spamblock_bayesimg/spamblock_bayes.load.gif?revision=1.9999 (IP ) could not be opened.
Maybe the Serendipity or SourceForge.net Server is down - we are sorry, you need to try again later.

Note that server and root path are missing from the URL. I do have set github.com in the Spartacus plugin as source for both, File/Mirror location (XML metadata) and File/Mirror location (files). After going to the Spartacus setup and intuitively ;-) clicking Save these informational messages were displayed:

Deleting file package_template.xml...
Deleting file package_sidebar_en.xml...
Deleting file package_event_en.xml...

So it seems that the upgrade from 2.1.1 to 2.1.2 should had cleared those cache files. Probably manually removing templates_c/package_* should work as well.

I can reproduce that (I just get Unknown Reason / Your browser sent a request that this server could not understand., but the workaround described above fixed it for me, too).

@onli: May that be due to 615591a?

Would adding an upgrader task (and probably releasing a 2.1.3 shortly) be the right fix?

@th-h th-h added bugs backend labels Mar 28, 2018

@th-h th-h added this to the Patch milestone Mar 28, 2018

@garvinhicking

This comment has been minimized.

Copy link
Member

garvinhicking commented Mar 28, 2018

@th-h

This comment has been minimized.

Copy link
Member Author

th-h commented Mar 28, 2018

@garvinhicking

This comment has been minimized.

Copy link
Member

garvinhicking commented Mar 28, 2018

@onli

This comment has been minimized.

Copy link
Member

onli commented Mar 28, 2018

Well, the bug is there. Sorry. The change seemed bulletproof: Spartacus just saved the array index, if that one were too big for the now smaller array it should have set $mirror to null by doing $mirror = $mirrors[$indextoolarge], which we catch. And that was tested and seemed to work fine :/

Debugging this now (I can reproduce the error in my blog) I missed at least one place in spartacus where the mirror is set, namely when downloading updates... I will push the patch now, we should look in a second step whether there are other places where this issue persists.

@onli

This comment has been minimized.

Copy link
Member

onli commented Mar 28, 2018

Patch in 2.1: 483868b

Master was out of sync: b0fb8d2

@th-h

This comment has been minimized.

Copy link
Member Author

th-h commented Mar 28, 2018

Well, the bug is there. Sorry.

That just happens, to all of us. :-) Thanks for your patch!

I will push the patch now, we should look in a second step whether there are other places where this issue persists.

The same code is present in lines 840/841 (tip of 2.1, after your patch), so I think that will need the same fix, too.

th-h added a commit to th-h/Serendipity that referenced this issue Mar 30, 2018

Another fix to Spartacus after dropping netmirror.
See s9y#548.

Expands b0fb8d2
by adding the necessary fix at another code point.

Fixes a whitespace issue (tabs -> spaces).

Add Changelog entries.

Needs to be backported to 2.1 after tests and review.

Signed-off-by: Thomas Hochstein <thh@inter.net>
@th-h

This comment has been minimized.

Copy link
Member Author

th-h commented Mar 30, 2018

The same code is present in lines 840/841 (tip of 2.1, after your patch), so I think that will need the same fix, too.

Added a fix in #549. Please review and possibly merge to master.

Has to be backported (cherry-picked) to 2.1 afterwards.

onli added a commit that referenced this issue Mar 30, 2018

Another fix to Spartacus after dropping netmirror.
See #548.

Expands b0fb8d2
by adding the necessary fix at another code point.

Fixes a whitespace issue (tabs -> spaces).

Add Changelog entries.

Needs to be backported to 2.1 after tests and review.

Signed-off-by: Thomas Hochstein <thh@inter.net>

onli added a commit that referenced this issue Mar 30, 2018

Another fix to Spartacus after dropping netmirror.
See #548.

Expands b0fb8d2
by adding the necessary fix at another code point.

Fixes a whitespace issue (tabs -> spaces).

Add Changelog entries.

Needs to be backported to 2.1 after tests and review.

Signed-off-by: Thomas Hochstein <thh@inter.net>
@onli

This comment has been minimized.

Copy link
Member

onli commented Mar 30, 2018

Thanks. Merged and backported, if everything worked right.

@th-h

This comment has been minimized.

Copy link
Member Author

th-h commented Sep 2, 2018

Should be fixed by the commits above.

@th-h th-h closed this Sep 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.