Closed leo-unglaub closed 12 years ago
BugBuster1701
commented
12 years ago Ergänzungen:
--- Originally created on November 7th, 2010, at 04:17pm
aschempp
commented
12 years ago Hier als "meine Unterstützung" ein Proof-Of-Concept der aus meiner Sicht funktionieren müsste (ungetestet!)
--- Originally created on November 8th, 2010, at 12:22pm
ghost
commented
12 years ago Hi Andreas, ich habe es nicht getestet, aber ich glaube dein Patch wird nicht funktionieren. Konkret beziehe ich mich auf die Zeile
if (`include(TL_ROOT . '/system/modules/' . $strModule . '/config/runonce.php')
Laut php Dokumentation wird, da include ein Sprachkonstrukt ist, der Code als folgendes Statement interpretiert werden:
if (`include((TL_ROOT . '/system/modules/' . $strModule . '/config/runonce.php') )== false))Was in logischer Konsequenz dann folgendes ergibt:
if (@include(true))Mein Vorschlag waere eher, den aktuellen Ansatz beizubehalten und lediglich zu adaptieren. Also weiterhin file_exists() zu verwenden. Als Beispiel auch von mir ein ungetesteter Patch.
Edit: in meinem Patch ist am Anfang des strings ein "/" zuviel beim loeschen der runonce der einzelnen Extensions, das wird die Files Klasse zwar abfangen, aber dennoch ist es eine Unschoenheit. Jedoch lediglich um diesen "/" zu entfernen nun ein neues patchfile hochzuladen empfinde ich als overkill.
--- Originally created by xtra on November 8th, 2010, at 03:49pm
aschempp
commented
12 years ago Ich habe versucht möglichst wenig unnötige Funktionen (wie eben den file_exists) zu benutzen. Folgendes habe ich in der PHP-Doku gefunden, wonach es funktionieren müsste:
If the file can't be included, FALSE is returned and E_WARNING is issued.Wirklich wissen können wir's aber erst wenn es jemand testet ;-)
--- Originally created on November 8th, 2010, at 03:58pm
ghost
commented
12 years ago Oder man macht einfach:
if(false !== @include(...))--- Originally created by backbone on November 9th, 2010, at 08:22am
aschempp
commented
12 years ago das ist doch dasselbe was ich gemacht habe??
--- Originally created on November 9th, 2010, at 11:11am
ghost
commented
12 years ago Ja natürlich, es ging darum, das xtra aufgrund dessen das "include" ein "Language Construct" ist seine Parameter anders assoziiert. Deswegen damit aus:
if (`include(TL_ROOT . '/system/modules/' . $strModule . '/config/runonce.php')
nicht
if (`include((TL_ROOT . '/system/modules/' . $strModule . '/config/runonce.php') )== false))wird, die Schreibweise
if(false !== @include(...))nutzen.
--- Originally created by backbone on November 9th, 2010, at 12:41pm
ghost
commented
12 years ago Nicht ich assoziere die Parameter anders, sondern include tut dies. Umgangen werden kann dies durch "(include('foo.php'))
![](== false" bzw. "(include 'foo.php') )== false". Was beides dasselbe bedeutet.
Daryberhinaus sehe ich die "file_exists" nicht als sinnlos an sondern finde sie in der Tat besser als mittels "" die warnings dass eine Datei nicht existiert ins Nirvana zu geleiten. IMO sollte man "" allgemein vermeiden wo es geht, um etwaigen Problemen bei der spaeteren Fehlersuche aus dem Wege zu gehen.
--- Originally created by xtra on November 10th, 2010, at 03:09pm
aschempp
commented
12 years ago Ich habe eine Lösung gefunden, welche mit bisherigen Contao-Versionen bereits funktioniert, vorausgesetzt alle Entwickler halten sich daran... Im Anhang meine system/runonce.php. Diese führt alle runonce.php in den Modulverzeichnissen aus. Alle Erweiterungen sollten diese Datei mitliefern und die "echte" runonce.php in's Modulverzeichnis verschieben.
--- Originally created on March 21st, 2011, at 12:35pm
leo-unglaub
commented
12 years ago Hallo ! Ich kann deiner Idee leider nichts abgewinnen. Das ist zu Riskannt, da ein einzelner Entwickler reicht der das hier nicht mitbekommt und schon funktioniert das ganze nicht mehr. Wir wollen das hier ja ändern um genau dieses Risiko nicht mehr zu haben. Daher finde ich sollten wir jetzt nicht mit so was anfangen, denn das geht ziehmlich sicher nach hinten los.
Viele GRüße Leo
PS: Wäre es nicht besser in deiner runonce.php $this->getActiveModules() zu verwenden? Das ist zum einen gecached und zum anderen können deaktivierte Extensions hier nicht dazwischen funken.
--- Originally created on March 22nd, 2011, at 01:52pm
aschempp
commented
12 years ago Ich verstehe dein Problem, es fehlt eine Prüfung < 2.10. Im Anhang eine angepasste Variante. Natürlich wäre es besser wenn es im Core gelöst wird, aber es dauert ja wohl noch 3 Monate und die Lösung funktioniert aus meiner Sicht. Das heisst natürlich nicht dass das Ticket nicht umgesetzt werden sollte!
Bezüglich der getActiveModules(), ich kann diese nicht verwenden weil sonst ggf. die runonce.php einer deaktivierten Erweiterung nie ausgeführt wird. Die system/runonce.php wird ja beim ersten Aufruf gelöscht... In deiner Umsetzung sollte das ggf. aber schon so sein.
--- Originally created on March 22nd, 2011, at 03:13pm
leofeyer
commented
12 years ago Das Problem an allen Vorschlägen - egal ob mit include oder file_exists - ist dass das komplette Modulverzeichnis durchlaufen werden müsste. Und zwar bei jedem einzelnen Aufruf! Was bringt es, wenn wir für __autoload(), getTemplate(), loadDataContainer() etc. extra einen File-Cache entwickeln, um die Schleifen durch das Dateisystem zu verringern, und auf der anderen Seite dann solche Performance-Bremsen einbauen?
Hier gibt es bestimmt eine bessere Lösung. Spontan fiele mir ein Ordner system/runonce ein, in dem einfach alle vorhandenen Dateien ausgeführt und dann gelöscht werden. Alternativ wäre auch eine Tabelle in der Datenbank möglich, in die eine auszuführende Datei eingetragen und nach Beendigung wieder ausgetragen wird.
Die bisherigen Vorschläge sind jedenfalls aus meiner Sicht nicht optimal.
--- Originally created on May 30th, 2011, at 06:12pm
aschempp
commented
12 years ago Die Modulordner werden doch bereits beim laden der config.php durchlaufen, könnte man da nicht auch gleich eine Art Cache anlegen, ob die runonce.php da ist?
--- Originally created on May 30th, 2011, at 06:14pm
leo-unglaub
commented
12 years ago @Leo: Dein Vorschlag eines system/runonce.d/ wurde bereits diskutiert. Ganz zufrieden werden da wohl nie alle sein. Aber ich habe da eh eine generelle Frage. Muss die runonce.php überhaupt bei jedem Seitenaufruf ausgeführt werden? Wenn man ganz ehrlich ist würde es auch reichen das einfach nach dem Installieren und Updaten zu machen? Wo anders braucht man das sonst? Ich persöhnlich hatte den fall jedenfalls noch nicht.
--- Originally created on May 30th, 2011, at 06:39pm
aschempp
commented
12 years ago ich find die idee gar nicht so schlecht. runonce.php ausführen bei:
--- Originally created on May 30th, 2011, at 06:41pm
leofeyer
commented
12 years ago Die Modulordner werden doch bereits beim laden der config.php durchlaufen, könnte man da nicht auch gleich eine Art Cache anlegen, ob die runonce.php da ist?
Gute Idee. Hab ich nicht dran gedacht :)
--- Originally created on May 30th, 2011, at 07:39pm
leo-unglaub
commented
12 years ago Nur damit es nicht in Vegessenheit gerät, Leo Feyer hat auf der Konferenz angemerkt, dass es dies pro seitenaufruf mindestens 10 aufrufe von file_exists bedeuten würde die in 99,99999 %Seitenaufrufe im Live betrieb nicht benötigt würden. Daher möchte ich noch mal auf die Idee verweisen die runonce.php nur bei bestimmten Triggern auszuführen. (siehe Liste von Andreas oben) Dadurch wäre auch das Problem gelöst, dass die runconce.php vorm DB Update getriggert wird.
--- Originally created on June 5th, 2011, at 03:45pm
leofeyer
commented
12 years ago Implementiert in 8f30d837a5c339d27cb45ce4daca2d9b. Nach Abwägen aller Beiträge und der Diskussion auf der Konferenz habe ich mich nun für die DB-Variante entschieden. Es gibt also zukünftig eine Tabelle tl_runonce, in die die Pfade zu den einzelnen runonce.php-Dateien eingetragen werden müssen. Im Backend wird dann geprüft, ob diese Tabelle Einträge enthält, und nur dann wird die Runonce-Logik ausgeführt. Das bringt folgende Vorteile:
runonce.php-Dateien (z.B. pro Extension).--- Originally created on June 23rd, 2011, at 04:27pm
aschempp
commented
12 years ago Wie bitte soll eine Erweiterung bei Installation diesen Eintrag machen??
--- Originally created on June 23rd, 2011, at 04:51pm
ghost
commented
12 years ago Nur eine kleine MiniAnmerkung : Wenn eine Erweiterung deinstalliert wird, die eine runonce.php enthaelt, kommt natuerlich eine Meldung, das jene Datei nicht mehr gefunden wurde.
Ist banal, sollte aber erwaehnt werden.
Jedoch : Wann und wo soll sich ein Modul denn eintragen ?
Das kann ja nur durch das Modul selbst geschehen, da DB Inserts vom ER Client nicht gemacht werden.
--- Originally created by lindesbs on June 23rd, 2011, at 04:52pm
leofeyer
commented
12 years ago Über die .sql-Dateien im INSTALL-Verzeichnis hatte ich gedacht. Ist das nicht praktikabel?
--- Originally created on June 23rd, 2011, at 05:32pm
aschempp
commented
12 years ago Ich glaube die verwendet kein Mensch. Ausserdem spreche ich auch von manuellen Installationen. Wenn die Install.sql fürs repo genutzt werden sollte, dann ist das ja auch wieder ein Filetugriff. Da könnte gleich im Modulverzeichnis eine runonce ausgeführt werden. Aber eben, das löst nicht das "manuell"-Problem.
--- Originally created on June 23rd, 2011, at 05:41pm
leofeyer
commented
12 years ago Hm, da hast Du leider Recht. Ich würde trotzdem gerne den RC heute auf den Weg bringen und dieses Problem am Wochenende bzw. nächste Woche lösen. Da Beta-Versionen sowieso nicht für den produktiven Betrieb gedacht sind, sollte das kein Problem sein, oder?
--- Originally created on June 23rd, 2011, at 05:45pm
ghost
commented
12 years ago im config Verzeichniss der Module die runonce.php lassen. Und nur kontrollieren, wenn install.php oder der ER Client aufgerufen wird. Sollte doch einfacher machbar sein.
Bzw. dann, wenn die Validitaet der DB kontrolliert wird. Dies wird bei allen Durchgaengen erledigt. Dann koennte man auch gleich, durch die Modulverzeicnisse crawlern und ausfuehren
--- Originally created by lindesbs on June 23rd, 2011, at 05:52pm
leofeyer
commented
12 years ago So einfach ist es leider nicht, denn beim Live Update wird beispielsweise keine install.php aufgerufen. Aber wir werden schon noch eine Lösung finden :)
Wie kommt es denn, dass niemand von der INSTALL/UNINSTALL-Möglichkeit im ER Gebrauch macht?
--- Originally created on June 23rd, 2011, at 06:33pm
ghost
commented
12 years ago von welcher INSTALL/UNINSTALL-Möglichkeit redest Du ?
Und das LiveUpdate muss doch sicherlich auch die DB Updates machen, wenn z.B. neue Spalten dazu koemmen. Dorthinein die runonce Abhandlung.
--- Originally created by lindesbs on June 23rd, 2011, at 06:35pm
leofeyer
commented
12 years ago Die aus dem ER. Es gibt eine generelle install.sql und uninstall.sql sowie die Möglichkeit, weitere .sql-Dateien pro Version anzulegen. Dazu ein Zitat aus Peter's Manual:
Below is the content of a package ZIP-file for a fictional hotel room reservation extension:
INSTALL/install.sql
INSTALL/uninstall.sql
INSTALL/010010009.sql
INSTALL/010010019.sql
INSTALL/010020009.sql
TL_FILES/hotelres/floorplan.jpg
TL_ROOT/system/modules/hotelres/LICENSE.txt
TL_ROOT/system/modules/hotelres/HotelController.php
TL_ROOT/system/modules/hotelres/HotelInstaller.php
TL_ROOT/system/modules/hotelres/config/config.php
TL_ROOT/system/modules/hotelres/config/database.sql
TL_ROOT/system/modules/hotelres/dca/hotel.php
TL_ROOT/system/modules/hotelres/languages/en/hotel.php
TL_ROOT/system/modules/hotelres/languages/de/hotel.php
TL_ROOT/templates/hotelres_fe.tpl
package.xml
package.jpgThe archive naming scheme as Contao_package_version_build.zip, for the example application this could therefore be Contao_hotel_10000129_102.zip.
--- Originally created on June 23rd, 2011, at 06:39pm
ghost
commented
12 years ago Das ist mir neu ;-) Ich find aktuell auch nicht die Stell eim ER Client, wo die ausgefuehrt werden sollten.
Er loescht beim uninstall einfach brav alle Dateien : http://dev.contao.org/projects/typolight/repository/entry/tags/contao-2.9.5/system/modules/rep_client/RepositoryManager.php#L811
--- Originally created by lindesbs on June 23rd, 2011, at 06:53pm
ghost
commented
12 years ago Wie wäre es mit einer anderen Kombination:
Eine Erweiterung, welche ein Runonce benötigt, hat diese im Modulverzeichnis. (Wie schon oben erwähnt) Zusätzlich wird im Verzeichnis system eine Dummy-Datei runonce.txt angelegt. Nur wenn diese vorhanden ist, werden die Modulverzeichnisse nach spezifischen Runonce durchsuche und diese werden ausgeführt. Anschließend werden die Runonce und die Dummy-Datei gelöscht. Der Inhalt der Dummy-Datei ist beliebig, es geht nur um die Existenz dieser Datei.
Vorteile:
Nachteil: Löst das Uninstall-Problem nicht.
PS: Hab das mit INSTALL/install.sql und INSTALL/uninstall.sql auch noch nicht kapiert.
--- Originally created by styx7 on June 23rd, 2011, at 07:29pm
BugBuster1701
commented
12 years ago Das INSTALL/ Verzeichnis gibt es nur in der Dokumentation. Soweit ich mich erinnern kann wollte Peter das noch später implementieren. Daraus wurde aber nichts. Genau aus dem Grund kamen dann ja immer mehr Erweiterungen die diese Funktionalität zum Teil über die runonce abbildeten, sofern es um DB Updates / Änderungen ging.
Die Funktionaliät von INSTALL/install.sql INSTALL/uninstall.sql wäre natürlich super zu haben, aber das ist dann ein Feature Request für den ER Client.
--- Originally created on June 24th, 2011, at 05:33pm
leofeyer
commented
12 years ago So, jetzt müsste es aber passen (siehe f05ce0c70980976857e308c9411198fa).
--- Originally created on June 27th, 2011, at 05:20pm
aschempp
commented
12 years ago Gefällt mir schon viel besser, aber wie wird das runonce jetzt ausgeführt, wenn ein Modul per FTP installiert wurde?
--- Originally created on June 27th, 2011, at 05:52pm
leofeyer
commented
12 years ago Hm … Gar nicht? Brauchen wir das? Vermutlich schon wenn Du fragst :)
--- Originally created on June 27th, 2011, at 07:20pm
ghost
commented
12 years ago Es reicht doch aus, wenn die Modulverzeichnisse beim TabellenCheck auf die runonce.php kontrolliert werden. Diesen Vorgang muss man auch bei manueller Installation machen. Zwei Stellen, an denen einfach eine Funktion aufgerufen wird, die die Eintraege automatisch macht :
http://dev.contao.org/projects/typolight/repository/entry/tags/contao-2.9.5/system/modules/rep_client/DatabaseInstaller.php#L133 http://dev.contao.org/projects/typolight/repository/entry/tags/contao-2.9.5/contao/install.php#L917
--- Originally created by lindesbs on June 27th, 2011, at 07:45pm
BugBuster1701
commented
12 years ago Es sollte dabei aber weiterhin versucht werden, das auch bei einer manuellen Installation die runonce.php erst nach dem DB Update (install.php Aufruf) gestartet wird. Ich kann zwar auch davor mit leben, würde aber die runonce vereinfachen.
--- Originally created on June 27th, 2011, at 08:22pm
aschempp
commented
12 years ago Oh nein, UNBEDINGT vor dem DB Aufruf. Denn ich konvertiere z.B. bei Isotope viele Datenbankfelder, welche sonst beim Update fehlerhaft angelegt werden würden (z.B. umbenennen, Inhalte konvertieren usw.). Falls es beim DB-Update aufgerufen wird, müssen wir sicherstellen dass die DB-Konfiguration erst danach geladen wird, oder z.B. nach ausführen der runonce ein reload gemacht wird. Was spricht denn dagegen die runonce im Backend immer zu machen wie in der ersten Implementation? Im Frontend braucht es die sicherlich nicht, da bin ich einverstanden.
--- Originally created on June 28th, 2011, at 08:16am
ghost
commented
12 years ago M.E. bitte vor dem DB Update!
Wenn die runonce "nur" beim TabellenCheck aufgerufen wird, dann wäre es auch sinnvoll die "neue" Erweiterung ggf. automatisch als Deatkiviert zu setzen, bis der TabellenCheck ausgeführt wird. Das wäre dann aber eine größere Änderung. Die Idee von styx7 finde ich auch nicht schlecht, auch wenn ich die Semaphorendatei nicht runonce.txt nennen würde.
@leo: Ja bitte FTP-Upload unterstützen, sonst ist Contao immer auf Soap oder das Vorhandensein einer Erweiterung als Erstatz angewiesen.
Was ich mir an dieser Stelle auch wünsche ist der Aufruf einer config/uninstall.php am Anfang der RepositoryManager::uninstallExtension, dann können auch spezielle Änderungen, wie z.B. bei Isotope, beim Uninstall rückgängig gemacht werden.
--- Originally created by N.Parker on June 28th, 2011, at 08:57am
fbender
commented
12 years ago Solang in der runonce.php zwischen vor und nach DB-Update unterschieden werden kann, kann man doch beide Varianten haben? Einmal vor, und einmal nach dem Update. Die runonce's, die ihr Programm abgespult haben, löschen sich dann selbtständig und es sollte keine Konflikte geben.
--- Originally created on June 28th, 2011, at 09:30am
BugBuster1701
commented
12 years ago Hmm, ich habe die f05ce0c70980976857e308c9411198fa wohl falsch verstanden, sah für mich so aus als wenn es danach wäre. OK, dann ebend vor dem DB Update, derzeit gehe ich in meinen von beiden Fällen aus.
--- Originally created on June 28th, 2011, at 11:14am
leofeyer
commented
12 years ago Der Aufruf bleibt auf jeden Fall vor dem DB-Update.
--- Originally created on June 28th, 2011, at 11:21am
leofeyer
commented
12 years ago Neuer Versuch in der 22d5b9e5e4e51dde5459c17ec4e9d4de: Die Routine wird jetzt immer vor dem DB-Update getriggert.
--- Originally created on June 28th, 2011, at 01:48pm
Toflar
commented
12 years ago Seh ich das richtig, dass sich der Installationsprozess jetzt aus Anwendersicht dahingehend verändert, dass er aus Rücksicht auf die runonce.php immer die install.php aufrufen muss (bei manueller Installation). Vorher war das ja nicht zwingend nötig, falls eine Extension keine database.sql benötigt.
Was spricht eigentlich gegen eine Modulinstallations-Routine wie man es von anderen Systemen auch kennt? Oftmals kann man da einfach Module hochladen und muss sie nachher im Backend quasi importieren und aktivieren. Weit davon entfernt sind wir doch nicht? Was wenn die User nicht das zip-File lokal entpacken und die Dateien hochladen müssen, sondern einfach das zip-File as-is nach...kA...system/irgendwas hochladen und dann ins Backend gehen und dort den Module-Importer alles machen lassen? Ich meine, die Routine muss ja schon existieren weil das ER ja nichts anderes als das macht? Wäre das nicht benutzerfreundlicher?
--- Originally created on June 28th, 2011, at 01:58pm
BugBuster1701
commented
12 years ago ??dass er aus Rücksicht auf die runonce.php immer die install.php aufrufen muss (bei manueller Installation).?? Da sehe ich gar kein Problem. Das erwähne ich immer bei meinen Modulen, egal ob nötig oder nicht. Die Nutzer sollen sich dran gewöhnen es immer zu tun, nicht erst nachlesen ob oder ob nicht. (was die eh nicht tun, sein wir ehrlich)
--- Originally created on June 28th, 2011, at 11:11pm
leofeyer
commented
12 years ago --- Originally completed on June 27th, 2011, at 05:20pm
Hallo Leo, wir haben auf der contao Devcon gerade über die runonce.php geredet. Du kennst ja sicher das Problem, dass wenn man mehrere Extensions auf einmal aktualisiert und die alle eine runonce.php mitbringen, alle bis auf eine überschrieben werden. Wir hätten nun folgende Idee dazu gehabt:
Es wäre eine einfache Anpassung, die runonce.php aus /system/ nach system/modulname/config/runonce.php zu verschieben. Dort hätte jede Extension seine eigene runonce.php. Du müsstest dann nur alle Verzeichnisse durchlaufen und so wie beim config.php File die runonce.php sourcen.
Was hällst du von dieser anpassung? Wäre das in deinen Augen sinnvoll? Könnte man das noch in 2.9.2 einbauen? Viele Grüße von allen auf der devcon leo
Download the attachments
--- Originally created on November 7th, 2010, at 04:02pm (ID 2664)