Skip to content
This repository has been archived by the owner on Jan 2, 2020. It is now read-only.

Web-Installer installiert in einen Unterordner #6

Closed
pmmueller opened this issue Aug 21, 2012 · 19 comments
Closed

Web-Installer installiert in einen Unterordner #6

pmmueller opened this issue Aug 21, 2012 · 19 comments
Labels

Comments

@pmmueller
Copy link

Wenn ich den Ordner /check/ auf meinen Webspace hochlade, dann erzeugt der "Web-Installer" einen Unterordner mit dem Namen der Contao-Version, zum Beispiel

/check/contao-3.0.RC1/

Die Pfadangaben auf dem Server werden dadurch ein bisschen konfus, besonders wenn ich in der neuen Installation wieder einen Check-Ordner anlege, um die Installation zu überprüfen...

Klar könnte ich die Dateien aus dem /check/-Ordner in den Root-Ordner kopieren, aber dann hätte ich immer noch einen Unterordner namens /contao-3.0.RC1/.

Wäre es möglich, den Ordner-Namen selbst eingeben zu können?
Eventuell sogar eine Ebene höher, also als Brüderchen von /check/?

@Aybee
Copy link

Aybee commented Aug 21, 2012

+1

@leo-unglaub
Copy link
Contributor

@pmmueller ich überarbeite diesen Teil gerade. Ich bin auch der Meinung, dass das nicht optimal ist. Zumal ich dem User eine Auswahlder zu installierenden Contao-Version geben möchte. Ein Zwang für die aktuellste Version macht keinen sinn.

Mein Pull-Request kommt in ein paar Minuten, ich muss mich nur etwas durch den Code kämpfen, da dieser einem schrecklichen "spagetticode" Entspricht und auf Objekte und strukturierung total verzichtet. :(

@leo-unglaub
Copy link
Contributor

I added the support for the "custom directory" in the pull request #8. Feedback is more than welcome.

@leofeyer
Copy link
Member

Bei Dateien, die im Schnitt nur aus 15 bis 20 Zeilen PHP-Code bestehen, kann man wohl kaum von "schrecklichem Spaghetticode" sprechen :)

@leo-unglaub
Copy link
Contributor

Jein, funktionaler Code ist nicht immer schlecht, jedoch in diesem Beispiel hier versagt der Code teilweise. Ein kleines Beispiel: Hier setzt du die Variable $cli auf true. https://github.com/contao/check/blob/master/check/install.php#L29 Daran erkennt man später, ob man nun die cli installation, oder die PHP Installation verwenden soll. Jedoch ist diese Variable im Scope der Funktion cli_installation selbst nicht mehr verfügbar. Um in dieser Funktion nun zu testen ob wget oder curl verwendet werden sollen, muss ich den gleichen test von oben nun wieder via Copy und Paste herunter kopieren und erneut ausführen.

Wenn ich nach curl, wget, ... noch eine methode hinzufüge, blickt bei dem code irgend wann kein Mensch mehr durch. Der Contao-Check ist das erste, was sich neue User anschauen, sollte nicht gerade dieser Qualitativ gut geschrieben sein?

Ich will ehrlich sein, wenn ich ein neues System teste und so einen Code im Jahr 2012 sehe, dann lösche ich den Ordner schneller von meiner Festplatte, als ich foobar sagen kann.

@leofeyer
Copy link
Member

Das letzte Argument finde ich das überzeugendste. In den anderen Punkten hast Du auch nicht unrecht.

Ich wollte den Check ursprünglich mit Silex und Twig umsetzen, nur bräuchte man dann schon wieder einen Check, um die Systemvoraussetzungen des Check zu prüfen ;) Deswegen habe ich die einfachste aller Varianten gewählt.

@leo-unglaub
Copy link
Contributor

Wie gesagt, ich habe nichts gegen deine PlainPHP Lösung. Nur man muss das halt etwas mehr umstrukturieren.

Wie machen wir hier nun weiter? Soll ich meine Arbeit an dem Teil fortsetzen oder hast du daran eh kein Interesse?

@leofeyer
Copy link
Member

Ich hab auf jeden Fall Interesse und werde auch die meisten (hab noch nicht alles angeschaut) Deiner Änderungen übernehmen.

@leo-unglaub
Copy link
Contributor

Gut, dann werde ich mir die restlichen Files auch noch vornehmen.

@leofeyer
Copy link
Member

Im aktuellen "release/2.0"-Zweig geändert. Bitte ausführlich testen.

@leo-unglaub
Copy link
Contributor

Okay, ich habe mir deine aktuelle Version angeschaut. Finde ich schon sehr viel besser als die erste Version vom check. Über diese sprechen wir am besten einfach gar nicht mehr. :)

Den Kompromiss mit der Versionsauswahl finde ich gut. Ich denke damit sollten wir alle gut leben können.

Generell funktioniert der Test, jedoch sind mir ein paar Dinge aufgefallen:

  • Warum muss detect_unicode deaktiviert sein? Welcher Teil von Contao stört sich dadran?
  • Die Installation funktioniert nicht. Es wird bei mir kein Contao Installiert.

@leo-unglaub
Copy link
Contributor

Sorry, vergiss das mit der fehlerhaften Installation. Hatte dein git repository ja via ln -s eingebunden, da werden die Dateien natürlich wo anders hin kopiert, und nicht den gelinkten Ordner :)

Ich habe zwei kleine Dinge gefunden:

  • Die README.md vom check wird nach der Installation von Contao überschrieben. (sollte aber egal sein, da man ja meistens eh nur den check ornder selbst hoch läd)
  • Du hast eine falsche Verwendung von filter_var drin. Siehe codeblock weiter unten.
// falsch
$version = filter_var($_POST['version'], FILTER_SANITIZE_STRING);

// richtig
$version = filter_input(INPUT_POST, 'version', FILTER_SANITIZE_STRING);

Ansonsten sollte der Check fertig zur Veröffentlichung sein.

@leofeyer
Copy link
Member

Das detect_unicode-Problem betrifft nur Phar-Dateien, also das Live Update. Die falsche Verwendung von filter_var() sehe ich nicht. Ich kenne filter_input(), aber zum einen ist das mehr Schreibarbeit für dasselbe Ergebnis und zum anderen können die Werte, die filter_input() zurück gibt, nicht überschrieben werden.

Beispiel:

$version = filter_input(INPUT_POST, 'version', FILTER_SANITIZE_STRING);
echo $version; // 2.11.5

$_POST['version'] = '3.0.0';

$version = filter_input(INPUT_POST, 'version', FILTER_SANITIZE_STRING);
echo $version; // 2.11.5

Dagegen:

$version = filter_var($_POST['version'], FILTER_SANITIZE_STRING);
echo $version; // 2.11.5

$_POST['version'] = '3.0.0';

$version = filter_var($_POST['version'], FILTER_SANITIZE_STRING);
echo $version; // 3.0.0

Einen echten Bedarf für filter_var() habe ich nie gesehen.

@pmmueller
Copy link
Author

Ich habe den Check 2.1 gerade mal getestet, und wenn ich nichts übersehen habe, wird Contao jetzt immer in den Ordner oberhalb von /check/ installiert. Eine richtige "Ordnerauswahl" gibt es nicht mehr.

Ich finde das völlig okay so. Ich wollte nur sichergehen, dass ich nichts übersehen habe.

Und noch eine Frage: Contao 3.0.RC1 kann ich mit dem Check nicht installieren, oder? Die Liste im Webinstaller wird dann erst mit der "3.0.0 Final Version" ergänzt? Ich frage das, weil ich ja im Buch die Installation beschreibe, und so lange 3.0 nicht in der Liste auftaucht, kann ich das schlecht beschreiben ;-)

@leofeyer
Copy link
Member

leofeyer commented Sep 4, 2012

Ja, es wird immer in den Ordner oberhalb von "check" installiert. Dann ist der Check nämlich auch gleich an der richtigen Stelle für die Prüfung der Installation und wird – dank robots.txt von Contao selbst – auch nicht indiziert.

Bezüglich der Versionen haben wir uns darauf geeinigt, dass nur stabile Versionen installiert werden können. Also wird die 3.0.0 final die erste aus der 3er-Serie.

@leo-unglaub
Copy link
Contributor

@pmmueller Ja, @leofeyer hat sich gegen eine freie Ordnerwahl ausgesprochen. Ich war ja dafür und meine Pull Request konnte das auch, aber das Thema werden wir in Zukunft noch mal aufgreifen. :)

Du kannst aktuall nur die stabilen Versionen auswählen, für die auch ein .json file vorliegt. Auch in diesem Fall hat sich LF gegen eine freie und dynamische Wahl der Versionen entschieden. Aber auch hier kann man in zukunft vielleicht nich mal handeln :)

@xchs
Copy link
Contributor

xchs commented Sep 4, 2012

Ja, es wird immer in den Ordner oberhalb von "check" installiert.

Bedeutet das, dass eine bestehende Installation, die es dort ja geben könnte, überschrieben wird?

@leo-unglaub
Copy link
Contributor

Ja, leider. Deshalb hatte ich die dynamische Ordnerwahl ja eingebaut. Das wollte @leofeyer jedoch nicht.

@leofeyer
Copy link
Member

leofeyer commented Sep 4, 2012

Bedeutet das, dass eine bestehende Installation, die es dort ja geben könnte, überschrieben wird?

Siehe dazu #15.

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

No branches or pull requests

5 participants