Parsing error in /etc/inc/system.inc

bmeeks

It would be pretty easy to write a simple script to syntax check the PHP files. This is the command to run:

php -l <filename>

That's a lowercase L (so "l") in the command. It checks the given file for correct PHP syntax, and prints out any errors.

I use this command pretty much everytime I make an edit to a PHP file in the Snort or Suricata packages if I'm not immediately running the new PHP file on one my test VMs.

bimmerdriver

@nrgia said in Parsing error in /etc/inc/system.inc:

@bimmerdriver

Yes there is.
You must edit the file with vi, vim or nano.
Jump to the line in the error.
There you will notice that after the last word in the code, before the closing } there is no semicolon ; and it must be. Please add one.
After that save the file and exit.
You can reboot the system by typing reboot.
After the restart you will notice that another line is missing the semicolon, do the same for that line also, reboot and all will be well.

I had the same issue, and solved it this way.

I would like to point out that this issue happened a few weeks back, but for another file. I think a syntax check can be implemented in the pipeline, this is getting frustrating when you are away from the pfSense appliance.

Thank you for the help. I had to fix two missing semi-colons. There really is no excuse for this happening. I hope the development process will be improved to prevent this from happening.

bimmerdriver

@sergei_shablovsky said in Parsing error in /etc/inc/system.inc:

@nrgia said in Parsing error in /etc/system.inc:

@bimmerdriver

Yes there is.
You must edit the file with vi, vim or nano.
For someone who newbies in pfSense:

logging as “admin” / “your-psw-to-web-GUI” bring You to pfSense Console Menu

Choose “Shell” item from Console Menu

Strat vi by type “vi” in Command Line (You have no vim/nano on pure pfSense installation, this link may help a You if You have no experience with vi https://staff.washington.edu/rells/R110/)

Jump to the line in the error.
There you will notice that after the last word in the code, before the closing } there is no semicolon ; and it must be. Please add one.
After that save the file and exit.

This “command” would be “return false” and You need to correct it to “return false;”

You can reboot the system by typing reboot.
After the restart you will notice that another line is missing the semicolon, do the same for that line also, reboot and all will be well.

I had the same issue, and solved it this way.

I would like to point out that this issue happened a few weeks back, but for another file. I think a syntax check can be implemented in the pipeline, this is getting frustrating when you are away from the pfSense appliance.

All modern code editors and IDE system already have this checking, on macOS that are small and very useful app Espresso (have a lot plugins for different OS and Languages), Coda 2, and some “heavy” IDE app from a Jet Brain Systems like PhpStorm, AppCode, WebStorm, Apple Xcode.
And of coarse, better to using GIT (on macOS that would be perfect Tower GIT with Kaleidoscope companion) to avoid any mistakes and mare development easy, predictable and error free.

May be coding pipeline in DevTeam are outdated .... :)

Thank you for the link to the vi web page. I had forgotten how much I dislike vi. Now I remember it again.

Gertjan

@bimmerdriver said in Parsing error in /etc/inc/system.inc:

forgotten how much I dislike vi.

pkg install nano

and you will not be remembered any more that you have forgotten it.

Sergei_Shablovsky

@nrgia said in Parsing error in /etc/inc/system.inc:

@sergei_shablovsky said in Parsing error in /etc/inc/system.inc:

All modern code editors and IDE system already have this checking, on macOS that are small and very useful app Espresso (have a lot plugins for different OS and Languages), Coda 2, and some “heavy” IDE app from a Jet Brain Systems like PhpStorm, AppCode, WebStorm, Apple Xcode.
And of coarse, better to using GIT (on macOS that would be perfect Tower GIT with Kaleidoscope companion) to avoid any mistakes and mare development easy, predictable and error free.

May be coding pipeline in DevTeam are outdated .... :)

It is pretty obvious that not using a proper IDE is the culprit here. But then again just suggesting to a DevTeam a list of proper ones, will not change their ways of working. They can have restrictions on what they use, or a licensing policy..., so they must work with what they are allowed or have access to. That's why I suggested some unit tests to run in the pipeline for the syntax check, outdated as it is.

Agree.

Sergei_Shablovsky

@gertjan said in Parsing error in /etc/inc/system.inc:

@bimmerdriver said in Parsing error in /etc/inc/system.inc:

forgotten how much I dislike vi.
pkg install nano
and you will not be remembered any more that you have forgotten it.

This is another one example (previous are about the set different but both high resolution to local-connected VGA and COM console) of what I am write: WHY to be so sticky to VERY OLD standards, when most of all already changed?

Sergei_Shablovsky

@nrgia said in Parsing error in /etc/inc/system.inc:

@sergei_shablovsky said in Parsing error in /etc/inc/system.inc:

All modern code editors and IDE system already have this checking, on macOS that are small and very useful app Espresso (have a lot plugins for different OS and Languages), Coda 2, and some “heavy” IDE app from a Jet Brain Systems like PhpStorm, AppCode, WebStorm, Apple Xcode.
And of coarse, better to using GIT (on macOS that would be perfect Tower GIT with Kaleidoscope companion) to avoid any mistakes and mare development easy, predictable and error free.

May be coding pipeline in DevTeam are outdated .... :)

It is pretty obvious that not using a proper IDE is the culprit here. But then again just suggesting to a DevTeam a list of proper ones, will not change their ways of working. They can have restrictions on what they use, or a licensing policy...,

If this needed to 70-80%+ of users that use the pfSense WHY NOT incorporate it and let this millions of peoples wasting the time and (and worst of it) making mistakes and have the problem and bad User Experience? (rhetoric question)

bimmerdriver

@sergei_shablovsky said in Parsing error in /etc/inc/system.inc:

@nrgia said in Parsing error in /etc/inc/system.inc:

@sergei_shablovsky said in Parsing error in /etc/inc/system.inc:

All modern code editors and IDE system already have this checking, on macOS that are small and very useful app Espresso (have a lot plugins for different OS and Languages), Coda 2, and some “heavy” IDE app from a Jet Brain Systems like PhpStorm, AppCode, WebStorm, Apple Xcode.
And of coarse, better to using GIT (on macOS that would be perfect Tower GIT with Kaleidoscope companion) to avoid any mistakes and mare development easy, predictable and error free.

May be coding pipeline in DevTeam are outdated .... :)

It is pretty obvious that not using a proper IDE is the culprit here. But then again just suggesting to a DevTeam a list of proper ones, will not change their ways of working. They can have restrictions on what they use, or a licensing policy..., so they must work with what they are allowed or have access to. That's why I suggested some unit tests to run in the pipeline for the syntax check, outdated as it is.

Agree.

Regardless about what IDE is used, ensuring the system boots up properly is a fundamental test that must be passed before a new build is made available. This is not the first time that the system has been broken at a low level, requiring reinstallation or manual editing of code. This should not happen.

NRgia

@bimmerdriver @sergei_shablovsky

All that all of you pointed out is true, if someone commits a code with an incorrect syntax (skipping the IDE checks) the build must fail.
All of us reached the same conclusion I think, there are no controls in place that must prevent a build process to complete if a part of the code contains incorrect syntax. (and much more).

Gertjan

I do not want to argue with all that has been said above.
I even tend to say "yeah, all true".

Still, I like to throw in some more words.

Snapshots are made public for a reason.
Those who made the snapshots public, wrote this : https://www.pfsense.org/snapshots/ and the message over there was written to the ones that made the snapshots available.
Everybody is free to interpret that message as he wants, but what the author wrote should be taken in consideration.

Or should I be old fashioned and state : all what counts is what the author said, what we thinks is ok, but has no meaning.

I do very well understand that a snapshot could contain a solution to a problem that exist in released version.
Still, that reasoning comes in second.
First is ans stays "These builds are for testing purposes only.".

So, sorry for the easy joke, but "thanks for finding a missing semi colon ". That's what using (== testing in this case, remember) snapshots is all about.
Very often, the issues found a a snapshot are far worse.

Still, snapshots can be used.
In that case : before upgrading, make sure you have a "image" copy of the entire installed disk / partitions. If a newer snapshots fails, you press the button to go back to the version the day before.
Btw : you won't find this button in pfSense, the most easy way to use this button : use a VM that can snapshot your snapshot.

Again : peace, guys, I fully agree : there should be no coding errors like this.
( but as long as the development is partially driven by humans we will run into issues like this )

I didn't even know this existed :
php -l <filename>

Keeping mind that PHP files are not compiled or build or something like that.
The files on github are exact copies of what devs have on their dev system.
Or, maybe possible ( ?) the merging of "pull requests" went wrong on Github ?
Anyway, most of the snapshots issues are not syntax related, but more classic coding errors that execute just fine, and produce 'none sense' output.

Btw : as usual : I'm a user like all of you. And all of this is my personal opinion.

edit : Snapshots were being called "the bleading edge" in the past.
I guess that still stands.

bimmerdriver

@gertjan I hear what you are saying, but I still think if a build is made available as an update, it should pass a basic sniff test. It should boot and it should start up far enough that it can be updated. That way, even if it's broken, it's still possible to salvage it. If someone is testing individual commits that haven't been merged into a build, then all bets are off. It should be exceedingly rare that someone updates using the GUI and they break the system so badly that it doesn't boot or start up enough to be updated. At least in this recent case it was possible to hack it into working, but it should not happen that a system has to be reinstalled from scratch because of coding errors.

Sergei_Shablovsky

@gertjan This “syntax error” are smallest (but annoying) issue, because of “this build are for test purpose only”.

But mostly important the fact that “if the mistakes of that level are happened so frequently, how may be errors on algorithm level, memory leakage etc, may be hides from our view”?

Gertjan

@sergei_shablovsky said in Parsing error in /etc/inc/system.inc:

“if the mistakes of that level are happened so frequently, how may be errors on algorithm level, memory leakage etc, may be hides from our view”?

The one who answers that question will also find this one "What is the meaning of live ?".
You really want to know why people make mistakes ?

NRgia

@gertjan said in Parsing error in /etc/inc/system.inc:

I do not want to argue with all that has been said above.
I even tend to say "yeah, all true".

Still, I like to throw in some more words.

Snapshots are made public for a reason.
Those who made the snapshots public, wrote this : https://www.pfsense.org/snapshots/ and the message over there was written to the ones that made the snapshots available.
Everybody is free to interpret that message as he wants, but what the author wrote should be taken in consideration.

If you are referring to the following text: "These builds are for testing purposes only" then you don't know what testing really is. Did you heard of Definition of Ready or Definition of Done in a Software Development Lifecycle? I don't think you've heard about it.
First of all, before anyone starts testing a build, the one responsible releasing that build, must make sure that the build is TESTABLE, if it's not it should not reach the QA team in the first place. A short Smoke Test like:

Install the Snapshot see if it boots
Is the pfSense GUI displayed to the user
Are all the default services(packages) loaded without errors after the boot?
Is there any error messages in the logs after booting?

should be executed

If any of that fails you should not pass that Build further. You recheck the code and rebuild.

We are all pfSense fans here, with our comments we are hopping that the process will improve, and this will not happen again. Inventing excuses that this is ok to happen is not productive, and proves to me that you don't know what you're talking about.

Also from my perspective I don't have anything to add to this issue, and it will be my last comment for this particular issue.

Hope the process will change though.

Sergei_Shablovsky

@nrgia said in Parsing error in /etc/inc/system.inc:

@gertjan said in Parsing error in /etc/inc/system.inc:

I do not want to argue with all that has been said above.
I even tend to say "yeah, all true".

Still, I like to throw in some more words.

Snapshots are made public for a reason.
Those who made the snapshots public, wrote this : https://www.pfsense.org/snapshots/ and the message over there was written to the ones that made the snapshots available.
Everybody is free to interpret that message as he wants, but what the author wrote should be taken in consideration.

If you are referring to the following text: "These builds are for testing purposes only" then you don't know what testing really is. Did you heard of Definition of Ready or Definition of Done in a Software Development Lifecycle? I don't think you've heard about it.
First of all, before anyone starts testing a build, the one responsible releasing that build, must make sure that the build is TESTABLE, if it's not it should not reach the QA team in the first place. A short Smoke Test like:

Install the Snapshot see if it boots

Is the pfSense GUI displayed to the user

Are all the default services(packages) loaded without errors after the boot?

Is there any error messages in the logs after booting?

should be executed

If any of that fails you should not pass that Build further. You recheck the code and rebuild.

We are all pfSense fans here, with our comments we are hopping that the process will improve, and this will not happen again. Inventing excuses that this is ok to happen is not productive, and proves to me that you don't know what you're talking about.

Also from my perspective I don't have anything to add to this issue, and it will be my last comment for this particular issue.

Hope the process will change though.

TOTALLY AGREE!