[3.0] Introduce Sapi Class

[jdarwood007]

Move some logic from both Config and Utils that fit better here. This should be considered low-level code and should handle operating with the code in a very error-safe way.

It additionally, fixed some warnings from static analysis.

[live627]

multiline comments start with /*; phpdoc starts with /**

[Sesquipedalian]

There were a bunch of things in this PR that made PHP-CS-Fixer sad. Apparently the GitHub action didn't catch them for some reason. I push a commit to fix them all. However, we clearly need to fix something in the GitHub action so that it flags these problems like it is supposed to.

[jdarwood007]

And I have a habit of not checking php-cs-fixer before sending. But I don't see any logic in our github actions that runs cs fixer. Just a linter

[Sesquipedalian]

The problem is that the linter should have flagged the formatting issues and therefore failed the check. It didn't do that.

[jdarwood007]

I don't think most things are being checked properly by it

2024-02-01T00:43:49.6317556Z ##[group]Run composer lint
2024-02-01T00:43:49.6318036Z �[36;1mcomposer lint�[0m
2024-02-01T00:43:49.6344510Z shell: /usr/bin/bash -e {0}
2024-02-01T00:43:49.6344910Z ##[endgroup]
2024-02-01T00:43:49.8334027Z > php-cs-fixer check --diff --config .php-cs-fixer.dist.php --path-mode=intersection $(git diff --name-only "*.php")
2024-02-01T00:43:50.0463406Z PHP CS Fixer 3.48.0 Small Changes by Fabien Potencier, Dariusz Ruminski and contributors.
2024-02-01T00:43:50.0464942Z PHP runtime: 8.1.2-1ubuntu2.14
2024-02-01T00:43:50.1026517Z Loaded config default from ".php-cs-fixer.dist.php".
2024-02-01T00:43:50.1027692Z     0 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░]
2024-02-01T00:43:50.1065546Z ##[group]Run php ./vendor/simplemachines/build-tools/check-signed-off.php
2024-02-01T00:43:50.1066513Z �[36;1mphp ./vendor/simplemachines/build-tools/check-signed-off.php�[0m
2024-02-01T00:43:50.1096272Z shell: /usr/bin/bash -e {0}
2024-02-01T00:43:50.1096675Z ##[endgroup]
2024-02-01T00:43:50.1752223Z fatal: bad object 5f3a3e2f0d754d0ea544a24464f8bee9b5b0aafa
2024-02-01T00:43:50.1773935Z fatal: bad object 5f3a3e2f0d754d0ea544a24464f8bee9b5b0aafa
2024-02-01T00:43:50.1796667Z fatal: bad object 5f3a3e2f0d754d0ea544a24464f8bee9b5b0aafa
2024-02-01T00:43:50.1820262Z fatal: bad object 5f3a3e2f0d754d0ea544a24464f8bee9b5b0aafa
2024-02-01T00:43:50.1841886Z fatal: bad object 5f3a3e2f0d754d0ea544a24464f8bee9b5b0aafa
2024-02-01T00:43:50.1863929Z fatal: bad object 5f3a3e2f0d754d0ea544a24464f8bee9b5b0aafa
2024-02-01T00:43:50.1885141Z fatal: bad object 5f3a3e2f0d754d0ea544a24464f8bee9b5b0aafa
2024-02-01T00:43:50.1906571Z fatal: bad object 5f3a3e2f0d754d0ea544a24464f8bee9b5b0aafa

It went through with no errors.. But the checked sign off generated errors and passed through.

[Sesquipedalian]

Hm. Those 'bad object' errors look like Git errors. If so, that's not good at all. But I don't know where they would even be coming from; it's not from our own repository, or else we'd be seeing them ourselves constantly.

[jdarwood007]

I've dug into it. I can fix the errors, but it doesn't fix the issue well. The commit errors may be fixable. But the cs-fixer isn't. It needs to produce no output when things are ok and produce output when errors exist. I don't see a way to do that. If we can do that, I think we can fix the action.

I'm looking at https://github.com/squizlabs/PHP_CodeSniffer as a replacement. A Github Action exists that does line comments. So that would be awesome. I've been playing around with creating some custom rules to offset what rules we don't have. It may take me a while to get through it and configure all the rules. I have to create custom rules for some cases where we do things differently from the standards.

[Sesquipedalian]

Before you do too much with PHP-CodeSniffer, I believe PHP-CS-Fixer has a --quiet option. I'm on my phone right now, so I can't check at the moment, but I suggest looking at that before doing anything else.

[jdarwood007]

I think we can fix it then. I was still testing but I got something to work for the work flow which was essentially

    - name: Run Script
      run: |
        php ./check-signed-off.php > check-signed-off.txt 2>&1
        echo 'var<<EOF' >> $GITHUB_OUTPUT
        echo "$(cat check-signed-off.txt)\n" >> $GITHUB_OUTPUT
        echo 'EOF' >> $GITHUB_OUTPUT
        cat check-signed-off.txt

    - name: Fail If we had a message
      if: env.GITHUB_OUTPUT != ''
      run: |
        echo "$env.GITHUB_OUTPUT"
        exit 1

That was for sign-off, but we can repeat for linter. I'm not an expert in GitHub Actions @live627 seems to know more than me and may have a better idea to correct it.

[jdarwood007]

Also looks like we may be able to make it add to the comment:

    - name: Post result to PR
      uses: mshick/add-pr-comment@v1
      if: env.GITHUB_OUTPUT != ''
      with:
        message-path: ${{ env.GITHUB_OUTPUT }}
        repo-token: ${{ secrets.GITHUB_TOKEN }}
        repo-token-user-login: 'github-actions[bot]' # The user.login for temporary GitHub tokens
        allow-repeats: true # This is the default

Needs testing

[Tyrsson]

Actually https://github.com/squizlabs/PHP_CodeSniffer has been forked into a new project.

Issue detailing it can be found here:
squizlabs/PHP_CodeSniffer#3932

I have been needing to update some of my projects as well just have not had the time lately.

[jdarwood007]

Yep, but the composer install is still the "old name". To be fair, working with it. There is stuff we cna't do I have found so far, but I agree with what its fixing isn't the best standards anyways.

[Sesquipedalian]

I think #8072 will at least solve the problem where we don't want any output from PHP-CS-Fixer unless something is wrong.

[Sesquipedalian]

I also suggest that we move further discussion of the GitHub actions and continuous integration to that PR, rather than here in the comments of a closed PR.