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

Solves 9326: clean beta 4 install without pre-created database doesn't work #9328

Merged
merged 1 commit into from
Mar 7, 2016

Conversation

andrepereiradasilva
Copy link
Contributor

Pull Request for Issue #9326 .

Summary of Changes

Solves #9326

Testing Instructions

  1. Use beta 4 or latest staging and proceed with clean joomla install
  2. Do step 1 of install
  3. Check code difference in this PR and change that file (/libraries/joomla/database/driver.php) before executing step 2 (database) of install.
  4. On step 2 of install enter a new database (don't use one that already exists). before this change it wouldn't work, after this change it will work.

@andrepereiradasilva andrepereiradasilva changed the title Aolves https://github.com/joomla/joomla-cms/issues/9326 Solves https://github.com/joomla/joomla-cms/issues/9326 Mar 7, 2016
@andrepereiradasilva andrepereiradasilva changed the title Solves https://github.com/joomla/joomla-cms/issues/9326 Solves 9326 Mar 7, 2016
@andrepereiradasilva
Copy link
Contributor Author

@richard67 can you test this?

@andrepereiradasilva andrepereiradasilva changed the title Solves 9326 Solves 9326: clean beta 4 install without pre-created database doesn't work Mar 7, 2016
@Bakual
Copy link
Contributor

Bakual commented Mar 7, 2016

I have tested this item ✅ successfully on 7cacd55


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9328.

@richard67
Copy link
Member

@andrepereiradasilva I just was testing new install for several db and php versions but could not confirm problems reported in issue #9326 yet. But in 1 case my server got extremely slow. Because I was dealing with this piece of code, too (also wanted to change it to support utf8mb4), I would say by code review that your change with this PR here corrects a bad mistake.

@wilsonge Should be merged by code review, it is obviosly correcting a bad mistake, no need to test that.

@andrepereiradasilva
Copy link
Contributor Author

@richard67 it only happens when you create a new database on install on a utf8 capable server. If you use a database that already exists it works before this PR.

@Bakual
Copy link
Contributor

Bakual commented Mar 7, 2016

I could replicate the issue and the proposed fix solves it.

Also current code is obviously wrong as there is an undefined variable ($collation)

@richard67
Copy link
Member

Yes, just got that after reading comments to the issue. I can test but would be nice if I could save the work because it gets merged before.

@andrepereiradasilva
Copy link
Contributor Author

yeah sure

@richard67
Copy link
Member

Well I start preparing the test ... is a bit uncomfortable because I test on remote shared host, not on local PC.

@richard67
Copy link
Member

ahhh ... i cannot test this i am afraid ... will not have privileges maybe to create new db with Joomla! installation.

@richard67
Copy link
Member

Maybe @stellainformatica can test?

@AlexRed
Copy link
Contributor

AlexRed commented Mar 7, 2016

I have tested this item ✅ successfully on 7cacd55

Ok, now I can install Joomla 3.5 beta4


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9328.

@stellainformatica
Copy link
Contributor

I have tested this item ✅ successfully on 7cacd55

Tested in localhost, it solves the issue I reported here https://issues.joomla.org/tracker/joomla-cms/9326


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9328.

@richard67
Copy link
Member

@wilsonge Could be worth to make a Beta 5 just with this Patch included.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9328.

wilsonge added a commit that referenced this pull request Mar 7, 2016
Solves 9326: clean beta 4 install without pre-created database doesn't work
@wilsonge wilsonge merged commit 51a8fde into joomla:staging Mar 7, 2016
@wilsonge
Copy link
Contributor

wilsonge commented Mar 7, 2016

I don't have the resources to release on laptop :(

@andrepereiradasilva andrepereiradasilva deleted the patch-2 branch March 7, 2016 13:21
@mbabker
Copy link
Contributor

mbabker commented Mar 7, 2016

@wilsonge Do you need a b5 package posted? Begrudgingly I still have access to everything to do it.

@Kubik-Rubik
Copy link
Member

@wilsonge @mbabker Beta 5 should be released to fix this issue. It would be great if you could create it, Michael. Thank you!

@mbabker
Copy link
Contributor

mbabker commented Mar 7, 2016

On it. Expect everything to be published in next half hour.

@Kubik-Rubik
Copy link
Member

Welcome back, my friend! :-)

Thank you!

@mbabker
Copy link
Contributor

mbabker commented Mar 7, 2016

@Kubik-Rubik
Copy link
Member

Thank you @mbabker!

@wilsonge
Copy link
Contributor

wilsonge commented Mar 7, 2016

Thanks so much michael!

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

Successfully merging this pull request may close these issues.

None yet

9 participants