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

First attempt to get PHP 8.0 support #160

Open
wants to merge 274 commits into
base: devel
Choose a base branch
from

Conversation

GwynethLlewelyn
Copy link

Hello all,

This is my first pull request here — and quite a big one!

My testing environment is Ubuntu 20.04.2 LTS, Linux kernel 5.4.0-72-generic, running inside an nginx 1.19.6 vhost (no special configuration needed) with PHP 8.0.3 running as PHP-FPM, and using MariaDB 10.4.18 as the database server (I also use macOS to do simple syntax checking, but, so far, I haven't tried it out on the Mac).

There are a few glitches here and there, but I managed to do an install from scratch and get all the basics working except for the ticketing system, which is still throwing errors even on a clean database and doesn't seem to like new tickets.

It includes an upgrade to the last PEAR DB version that was released; it's been obsolete for several years now, but at least it still works with PHP 8.0.

I admit being partial to the ?? Null Coalescing Operator and tend to overuse it — before forgetting that it was just introduced in PHP 7. In other words, it's highly unlikely that this code will ever run on anything below PHP 7, but if full compatibility with PHP 5.6 is a requirement (duh!), I could theoretically do a regex on the whole code to get rid of ??...

Mind you, it'll take quite a lot of time to do a full review of all the code. I tried to document as much as possible, but feel free to ask if there are any doubts about some of my decisions.

I'm also aware that it's good GitHub etiquette to PGP-sign all commits submitted for inclusion in a pull request. Alas, for some reason, my favourite code editor seems to be unable to sign those commits, when I invoke git from inside it. Running git from the command line has no such problem. I haven't yet figured out what's wrong with my setup, but I hope to be able to fix it sooner or later.

Last but not least, I'm not really a professional programmer, and I often prefer easier-to-read code as opposed to very terse, idiomatic PHP-isms (except for the ternary operators... which I'm abusing all the time). In my experience, relying on those PHP-isms means that some things stop working after a few major versions; for example, the idiomatic if ($variable) { ... } to check if that variable exists, has been initialised, and doesn't have a value that equates to 0, null, false, "", [], and so forth, has been over-used for decades and is a relic of the time that PHP was purely interpreted. These days, with highly effective Just-In-Time compiling, the PHP team is turning PHP into a stricter language — which means that they can compile it much more efficiently — at the cost of forcing programmers to drop some of their more idiomatic PHP-isms. There is no end to this discussion, of course, and I'm not even taking sides (as said before, I've always loved strongly-typed languages, and that's why I'm doing everything in Go these days), but keep that in mind when you see quite a lot of extra non-idiomatic checks sprinkled all over the code. The goal was reaching a point where not even warnings are thrown!

…g and we need to check for existence; also, do some more checking for empty/unset vars
…tions flagged as static in the comments became *really* static (mandatory in PHP 8.0)
…database and restarting from scratch with a new install
…option to be added automagically to dprint (to-do; hardcoded for now)
…ollowup ticket table with 'modern' CSS as opposed to calculating colours in a hardwired way.
…* `.bordertable`, however, is used _once_ on the Classic style...
…l Rich Text Editor around, as much as feasible.
I mistakenly did a global search & replace for PERM_EDIT with PERM_DENY — stoopid!
I have some issues under nginx + PHP-FPM where syslog lines get truncated/buffered/filtered, so this option allows logs to be sent directly to a file on the system instead.
... and cleaned up a bug with overLib
@GwynethLlewelyn
Copy link
Author

I wonder if anyone had time to take a look at the whole PR?... I know, it's horribly long. Worse, I'm planning to add a few more changes in the probably-not-so-distant-future lol

@ipeevski
Copy link

ipeevski commented Aug 19, 2021 via email

@GwynethLlewelyn
Copy link
Author

@ipeevski thanks for that! Aye, I'm aware that there is a lot of code to review... it still has many issues here and there, broken functionality, and so forth... and I even haven't started to take a look at known vulnerabilities, some of which are around since 2012...

@tonymurray
Copy link

In calendar.class.php, line 527, I added 'static' before function getRecurrentEventforPeriod:
static function getRecurrentEventforPeriod($start_date, $end_date, $event_start_date, $event_end_date)

Similar to the fix you made on line 597. Does that make sense?- at least it fixed a Fatal error for me. Thanks for your previous work.

@GwynethLlewelyn
Copy link
Author

Thanks, @tonymurray — I'm pretty sure I'm missing a lot of static keywords here and there. I fully admit that I haven't tried out all of dotProject's functionality, so, no wonder here and there a few fatal errors are thrown...

BTW, PHP 8.1 is out, but I haven't tried to run dotProject under it. Let's see if there are many breaking changes...

@GwynethLlewelyn
Copy link
Author

Huh... I wonder if there's something else I should change on the code to get this PR merged?

Maybe testing it under PHP 8.1...?

I have no idea why...?
@fatabek
Copy link

fatabek commented Oct 19, 2022

Thanks for for this good working, we try today first time.

  1. When we click any options for example "write config" or "Install db & write config" Installlation not working but we have PHP 7.4 version installation. We use PHP 7.4 version config.php and databeses files. Can you check this ?
  2. When we try to see gantt chart, if the project owner and the login user aren't same gantt chart doesn't display. Can you check this ? (We try PHP version 8 & 8.1)

Note : We use devel branch

@GwynethLlewelyn
Copy link
Author

Ouch... I'm sorry, I haven't kept up with this lately!

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.

5 participants