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

Rewrite the install.php #8

Closed
wants to merge 4 commits into from
Closed

Rewrite the install.php #8

wants to merge 4 commits into from

Conversation

leo-unglaub
Copy link
Contributor

I startet to rewrite the install.php so it's based on classes
instead of the "spagetti-code" witch functions and no
structure/logic behind it.

I used the rewrite to add some new features:

  • The shell now supports wget and curl.
  • The user can choose witch version should be installed. The tags
    are dynamicly fetched from the github api, so there is no need
    to update the contao-check on every new release.
  • The user can specify in witch directory the new contao should
    be installed.

The html part is currently not complete rewritten, but sience i hate
css thats a job for some of the frontend developers here. I am
focusing on rewriting the other scripts.

I startet to rewrite the install.php so it's based on classes
instead of the "spagetti-code" witch functions and no
structure/logic behind it.

I used the rewrite to add some new features:
  * The shell now supports wget and curl.
  * The user can choose witch version should be installed. The tags
    are dynamicly fetched from the github api, so there is no need
    to update the contao-check on every new release.
  * The user can specify in witch directory the new contao should
    be installed.

The html part is currently not complete rewritten, but sience i hate
css thats a job for some of the frontend developers here. I am
focusing on rewriting the other scripts.
@BugBuster1701
Copy link

👍 Nice, it is now much clearer, in my opinion.

I have rewritten the bootstrap.php so now everything is seperated
into logical methods witch can be easily extented if needed.

I have fixed the folowing bugs:
  * Works now with asian browsers to
  * Shows errors on all setups, so it's easier to debug
  * Instead of die(); the script now throws Exceptions
  * Fix an possible XSS attack point
I sadly forgott some debug calls in the previous commit, but
now there are removed.
@leo-unglaub
Copy link
Contributor Author

I continued my rewrite of the entire contao check. I have to check all the other classes to, but this one is ready to use.

I have rewritten the repository.php. This file now uses methods
for all tests. So it's much easier to implement new tests. You simply
have to add a new method with the name check... and add your status
messages.

I also implemented the following tickets:
  * #2 from BugBuster
@leofeyer
Copy link
Member

Hm, you don't have to use classes just because you can :) Some of the files consist of only two lines of PHP code, which is less than the class variant.

@BugBuster1701
Copy link

you don't have to use classes just because you can

If it offers the clarity and easier way of expanding, then yes.

@leofeyer
Copy link
Member

Unfortunately, your code style did not at all match the one I used, so I had to write everything from scratch based on your pull request (even the phpDoc comments were mentioning params and return values where there were none).

I've started with the web installer (see 970587f). However:

  • I don't like the idea that the target path is freely editable. The installer will now install into the parent directory of the check.
  • I don't like the choosable version numbers, either. I have left them in for now, however I actually don't want people to install outdated versions.

How about only offering "latest stable" or "latest long term"?

@leo-unglaub
Copy link
Contributor Author

I have used my own coding style witch is very similar to the one you used in the contao core. (In the beginning your coding style was one of the reasons i started using contao, because it was similar to mine). I also think that my coding style is very readable and very well documented. (Besides from my spelling errors :) )

But you can choose any coding style you want. I am not picky as long as it is well documented, clean and good readable. It's your choice.

  • I don't see a problem in a free path, because it gives the user freedom to do what they need. And there is also NO security risk in that, because the webserver must limit the website in it's own directories anyway. You can set a useful default, but i don't see any reason why it should be static. You can store the choosen path in the session, so you don't have to look for it after the unzip.
  • Well, it's again the matter of user freedom. If a user needs for example the 3.1.2 because thats the latest version avisota and the catalog will work with, why should he be forced to use the 3.2.0? Again, it's your decision, but maybe it has something to do with me being a Linux user. I simply hate it if a system is blocking me from doing what i want to do. And it's simply done in 20 lines of code. So if there is no security reasion for removing functions, what kind of reasion is there?

Greetings
Leo

@leofeyer
Copy link
Member

There is a very strong reason: security! I don't want people to be able to easily install Contao 2.6 or any other version from which we know there are security holes in it.

@leo-unglaub
Copy link
Contributor Author

Maybe thats a little bit of a meta discussion. But isn't that the users choice to make? In Debian i am able to apt-pin every package version i want, event if it contains security bugs. But i can choice if i want so

@leofeyer
Copy link
Member

You can still chose to do so: git checkout 2.6.0 :)

@leo-unglaub
Copy link
Contributor Author

You can still chose to do so: git checkout 2.6.0 :)

;)

I think we just have different oppinions about how software should work. So i leave this up to you.

@leofeyer
Copy link
Member

There is something else I only recognized now: I was downloading the files from sourceforge.net so the download statistic won't get messed up. Does GitHub have a statistic, too?

@leo-unglaub
Copy link
Contributor Author

I have never cared about download stats or something like that. So i have no idea if github counts the downloads. In the web interface there is no counter and also on the API there are no stats. So i think it's save to say, that there are no stats.

@leofeyer Are you really someone how cares about huge download counters? I always thought quality is the only thing counting for you.

@leo-unglaub
Copy link
Contributor Author

I stand corrected, there are download stats on github. But not for tags, only for directly added downloads.

leo@voyager:~$ curl https://api.github.com/repos/contao/core/downloads
[

]

@leofeyer
Copy link
Member

I have implemented a compromise in c94dcf6: only the versions which are available for validating an installion can be installed. And sourceforge.net will be used for download :)

@leofeyer
Copy link
Member

Just pushed the "release/2.0" branch. Please check the code.

@leofeyer leofeyer closed this Aug 31, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants