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

update PHP Fat-Free Framework #1734

Merged
merged 13 commits into from
Mar 16, 2016
Merged

Conversation

ikkez
Copy link
Contributor

@ikkez ikkez commented Aug 30, 2015

fixed an issue found in the logs and updated to v3.5.0.

@blee-techempower
Copy link
Contributor

All test faild.

@msmith-techempower
Copy link
Member

Ah, the folder names all got changed for round 11.

Basically, I would suggest closing this pull request and opening a new one off of master.

@ikkez
Copy link
Contributor Author

ikkez commented Oct 19, 2015

Alright. I've updated the branch. It should be able to merge now.

@ikkez
Copy link
Contributor Author

ikkez commented Oct 20, 2015

hm. Is this CI check related to this Framework test? What's the problem?

@blee-techempower
Copy link
Contributor

Localtest failed. Here is log. ( http://sprunge.us/SOQY )
Please resolve this issue, so this branch can be merged.

@ikkez
Copy link
Contributor Author

ikkez commented Oct 21, 2015

Well actually I have no idea why it's not working. It worked in Round 10 before. What was changed?
I've adjusted a path in the nginx config now. hopefully that was the issue.
In the log you showed me, I noticed this notice:
Setup fat-free: [20-Oct-2015 15:29:49] NOTICE: PHP message: PHP Warning: PHP Startup: Unable to load dynamic library '/home/blee/FrameworkBenchmarks/installs/php-5.5.17/lib/php/extensions/no-debug-non-zts-20121212/yaf.so' - /home/blee/FrameworkBenchmarks/installs/php-5.5.17/lib/php/extensions/no-debug-non-zts-20121212/yaf.so: cannot open shared object file: No such file or directory in Unknown on line 0
is that relevant? It does not seem to belong to our framework. why was this tried to be loaded?

@hamiltont
Copy link
Contributor

@msmith-techempower Is there a readme we can point to that helps explain all the reasons why we don't like PRs with the source code for the entire framework included?

@ikkez Some context: all other frameworks (besides the 5-10 that have yet been updated, incl. fatfree) do not put their entire framework source code directly in the TFB repo, that is downloaded by the setup.sh script. Is this something you could add to this PR?

@msmith-techempower
Copy link
Member

@hamiltont Not to my knowledge... but there isn't much of that anyway.

@hamiltont
Copy link
Contributor

not anymore ;-)

@ikkez
Copy link
Contributor Author

ikkez commented Oct 30, 2015

@hamiltont really? As far as I can see others have included the source files as well, and fat-free is one of those with a relative small code base (~150kb, hand-picked for this tests). I can remove the changelog file to save additional 30kb if you want. I am currently more interested in getting the test-suite running instead of setup optimizations. I guess I'm lost with both tasks.

@Rayne
Copy link
Contributor

Rayne commented Nov 18, 2015

My Vagrant box returns the following result with f3-factory@4c03423:

toolset/run-tests.py --install server --mode verify --test fat-free

[…]

================================================================================
  Verification Summary

| Test: fat-free
|       plaintext   : WARN
|       query       : PASS
|       json        : PASS
|       db          : WARN
|       update      : PASS
================================================================================

The Travis CI runner instead fails because of ./toolset/run-ci.py prereq "$TESTDIR" terminating with exit code 1 (run-tests.py#L176-L186, see also the caller terminating at run-ci.py#L323-L326).

I haven't found a solution in the documentation or a similar problem in Google Groups. Do you have an idea how to fix this problem?

Also I would like to reset our master branch to create a new pull request (as suggested by @msmith-techempower) on a dedicated branch (probably this branch) after resolving the mentioned problems.

@ikkez
Copy link
Contributor Author

ikkez commented Nov 19, 2015

Also I would like to reset our master branch... as suggested by msmith-techempower

@Rayne I did that already and force-pushed it into the current master that is used in this pull-request. There should be no merge conflicts anymore.

@Rayne
Copy link
Contributor

Rayne commented Nov 19, 2015

Java/sabina has the same problem.

@ikkez You are right that there aren't merge conflicts, but our master branch isn't deployable and diverges from TechEmpower's master branch.

There's only one rule: anything in the master branch is always deployable.

Because of this, it's extremely important that your new branch is created off of master when working on a feature or a fix.

Source: https://guides.github.com/introduction/flow/index.html

@msmith-techempower
Copy link
Member

@ikkez @Rayne We have recently made a number of fixes for Travis-CI on our master; can you please rebase and push your branch so that Travis-CI will automatically test for us?

@ikkez
Copy link
Contributor Author

ikkez commented Dec 10, 2015

it's rebased now.

@msmith-techempower
Copy link
Member

Awesome!

@ikkez
Copy link
Contributor Author

ikkez commented Dec 11, 2015

Can I find the complete travis ci error logs somewhere? I'm looking for the full response of this test: https://travis-ci.org/F3Community/FrameworkBenchmarks/jobs/96214489#L1701
seems like it's the every last one to fix, the others are green now :)

@ikkez
Copy link
Contributor Author

ikkez commented Dec 11, 2015

Fat-Free uses a strict error_reporting setting by default. Because Php yaf seems to be installed globally as a php module #1521 and randomly fails to load, our error handler is triggered and shouts things like this:

Server fat-free: 2015/12/11 15:04:49 [error] 28739#0: *2 FastCGI sent in stderr: "PHP message: Fatal error: PHP Startup: Unable to load dynamic library '/home/vagrant/FrameworkBenchmarks/installs/php-5.5.17/lib/php/extensions/no-debug-non-zts-20121212/yaf.so' - /home/vagrant/FrameworkBenchmarks/installs/php-5.5.17/lib/php/extensions/no-debug-non-zts-20121212/yaf.so: cannot open shared object file: No such file or directory" while reading response header from upstream, client: 127.0.0.1, server: localhost, request: "GET /db-orm HTTP/1.1", upstream: "fastcgi://127.0.0.1:9001", host: "127.0.0.1:8080"

Because of this, your test verification randomly fails... sometimes. Of course we don't need yaf, and we could probably adjust the error reporting level to get around this, but I don't see that our framework should clean up install requirements or ignore errors from others participants.

@msmith-techempower
Copy link
Member

Hmm, I'm not convinced that yaf is the problem.

@ssmith-techempower Could you pull this and test locally; at least this will output error logs that can be attached to the task to better understand the issue.

@ikkez
Copy link
Contributor Author

ikkez commented Dec 11, 2015

Fat-Free scans for already occurred system errors on its initialization. We already have a similar issue here bcosca/fatfree#851 which reveals a server issue before F3 starts. A way to clear system errors is to use the error_clear_last(); function, but it's only available with PHP7.

@@ -43,6 +43,49 @@
"display_name": "fat-free",
"notes": "",
"versus": "php"
},
"php5": {
"setup_file": "setup",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be "setup_file": "setup_php5"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yes, indeed. thanks.

@msmith-techempower
Copy link
Member

I'm not quite sure what happened on Travis; it got into the fat-free-php5 test, successfully installed php5 and nginx, then just stopped doing work entirely so the job timed out.

I see the line nginx is installed! in the Travis log, so it definitely finished its fw_depends calls, but the next call should have been sed, and it never got to that... curious.

@ikkez
Copy link
Contributor Author

ikkez commented Jan 18, 2016

hm, should I just try to drop the php5 test for now, to see if it works, or will it be out of comparison with other frameworks, which are not using php7 already?

@msmith-techempower
Copy link
Member

Hmm, according to the log, it seems that there is still this timeout issue.

I'm not really sure what is going on here, but it's hanging up in Travis-CI.

@msmith-techempower
Copy link
Member

Seems that there is STILL a hang-up in Travis and I am not sure why.

@ssmith-techempower Can you test all the fat-free variants locally and report back?

@ssmith-techempower
Copy link
Contributor

It's timing out my local install as well. Gets to "nginx is installed!" and then hangs.

@msmith-techempower
Copy link
Member

Thanks - it's odd that of all the PHP tests, this is the only one hanging in this way.


F3DIR="$TROOT/src"

[[ ! -e "$F3DIR" ]] || rm -r "$F3DIR"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rm command here needs the f flag to force the rm to happen without bringing up a prompt, which was hanging Travis and my local install due to prompting about write-protected files.

@ikkez
Copy link
Contributor Author

ikkez commented Mar 16, 2016

looks good here https://travis-ci.org/F3Community/FrameworkBenchmarks/jobs/116216563
thank you all for the support :)

@ssmith-techempower
Copy link
Contributor

Looks good on my local install, and I trust your Travis job means it would succeed for us. Merging in.

ssmith-techempower added a commit that referenced this pull request Mar 16, 2016
update PHP Fat-Free Framework
@ssmith-techempower ssmith-techempower merged commit b2aa669 into TechEmpower:master Mar 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants