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

Automated Release Builds #4114

Merged
merged 35 commits into from
Mar 28, 2020
Merged

Automated Release Builds #4114

merged 35 commits into from
Mar 28, 2020

Conversation

Deltik
Copy link
Member

@Deltik Deltik commented Mar 28, 2020

Motivation and Context

e107 release builds are currently done manually in an unreproducible closed-source process.

To reduce the effort of building releases and to allow anyone to generate consistent builds, automated release builds are needed.

Description

Release Builds

This pull request configures this GitHub repository to run a release build on every push and pull request. Builds can be tagged or untagged.

Tagged releases are built from a specific Git tag, which is pushed to the GitHub repository at the same time as the revision. The Git tag is converted into a PHP version, which will be assigned to that e107 release. For example, the tag v2.3.0 will build e107 2.3.0. Tagged releases are expected to be published on the releases page.

Untagged releases are practically "nightly" builds. They are created on every commit as snapshots leading up to the next tagged release. The version number is the latest tag version or the version in ./e107_admin/ver.php, whichever is higher, plus dev to indicate a prerelease, plus a hyphen (-), plus the number of commits since the latest tag version, plus (-g), plus the first 9 characters of the Git commit hash. For example, e107 2.3.0dev-881-g5eb116142 is the v2.3.0 prerelease built from commit 5eb116142, which was probably 881 commits after the last tag, whatever that was.

All release builds remove certain files that are not used in production (such as the ./e107_tests/ folder) and generate a checksum map (./e107_admin/core_image.php) so that the e107 administrator can use File Inspector to check the integrity of their e107 installation.

Core Image

The core image format has been rewritten. Previously, $core_image and $deprecated_image were stored as PHP arrays and used by File Inspector to verify the e107 installation.

Now, a phar gzip-compressed JSON object is created containing the following information:

  • Default relative path – The path to each file of an uncustomized e107 installation
  • Version – The version corresponding to the checksum
  • Checksum – The checksum of the path and the version

All core e107 files in the release are checksummed and put into the JSON object using the release version in the version field.

The build script then looks back into all past stable e107 release tags to generate a list of core e107 files that have been removed or renamed by the current release. These old files are also checksummed and inserted into the JSON object. This information allows the e107 administrator to see what files they should delete after upgrading their e107 installation:

image

For the core image rewrite, this pull request also includes:

  • a base JSON implementation (e_file_inspector_json) that stores a pretty-printed JSON string in $core_image, but it is not used because the compressed child implementation (e_file_inspector_json_phar) has a space savings of over ⅔, and
  • an SQLite phar implementation (e_file_inspector_sqlphar) that stores the same data in SQLite format, but it is not used because the data retrieval performance was unacceptably poor.

e_file_inspector

A new abstract class e_file_inspector has been introduced to read integrity database files. The current API is read-only, but it can be expanded later to:

  • allow plugin authors to generate integrity images for their own plugins, and
  • heal damaged files by downloading them from a mirror.

In the future, a new implementation can be added with low effort to download a detailed integrity database from an official e107 mirror to do integrity checks from an authoritative online source.

File Inspector

File Inspector has also been rewritten to support the new e_file_inspector interface. The output behavior of File Inspector differs in some ways:

  • Files are now shown with folders instead of in an overlay view.
  • JavaScript-expanded help has been fixed.
  • In the tree view, the root directory is expanded by default, all sub-folders are contracted.
  • If a file checksum matches a previous version of e107, that version is shown.
  • Filters that were not clearly defined have been removed.
  • The option to perform a scan without an integrity check has been removed.

How Has This Been Tested?

Only e_file_inspector has a few basic unit tests.

The release builds have been verified manually by installing e107 from those builds and then running File Inspector to ensure the new installation is intact.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist

App root files introduced in #4099
Format is different and currently incompatible.  The checksum has been
replaced with an associated array where the keys are the e107 version
and the values are the checksum from that e107 version.
Looks like SQLite scales better when there are more files to put in the
database.
Much faster checksumming of deleted files by using git-checkout instead
of git-show
This has two benefits:

1. Protects the SQLite database from being accessed on the Internet by
   having the e107_INIT constant check.
2. Compresses the SQLite database, saving over ⅔ of the space!
Now uses bit flags, as the previous approach of "overriding" the
validation code may hide information.  Previously,
e_file_inspector::VALIDATION_FAIL could be overridden by
e_file_inspector::VALIDATION_OLD.  Now, e_file_inspector::VALIDATED_HASH
and e_file_inspector::VALIDATED_UPTODATE provides information about
both.
- MOD: e_file_inspector::validate() will no longer bother with
       checksumming a file if the database hash is not present for the
       requested version.
- MOD: Better check for e_file_inspector::VALIDATED_DETERMINABLE
- MOD: e_file_inspector::VALIDATED_UPTODATE will now pass if there is no
       checksum in the database and no calculable checksum in the real
       file.
- FIX: Better documentation for
       e_file_inspector_interface::VALIDATED_DETERMINABLE
- FIX: Use the same bit for e_file_inspector::VALIDATED for a full
       validation check in e_file_inspector::validate()
Optimized performance of e_file_inspector_sqlphar::pathToDefaultPath()
by eliminating expensive method calls
PDO SQLite was performing unacceptably poorly.
Let's just load the entire integrity image into RAM…
Squeezed out more performance at the cost of memory usage
e_file_inspector_json_phar is an extended implementation of
e_file_inspector_json that encapsulates a JSON core image into a
gzip-compressed phar.

This results in space savings of over 60% compared to plain JSON.
Now File Inspector detects old files regardless of their hash value.
Now supports:

* "Show Core Files"
* "Show Missing Core Files"
* "Show Non Core Files"
* "Show Old Core Files"

Removed support for:

* "Exclude Language-Files"
List mode is supported again

Also added file size support
Bugfixes related to this change:

* e_file_inspector: During validation, the checksum is now calculated,
  even if the file is old.
* e_file_inspector_json: Strip the "v" from the beginning of version
  numbers in the database to make them proper PHP-standardized versions.
Bugs fixed:

* Security failure status is now prioritized in
  file_inspector::getStatusForValidationCode()
* File Inspector list view now supports filters
@Deltik Deltik requested a review from CaMer0n March 28, 2020 00:28
@Deltik Deltik added the type: enhancement An improvement or new feature request label Mar 28, 2020
Low-hanging resolutions for Code Climate
@codeclimate
Copy link

codeclimate bot commented Mar 28, 2020

Code Climate has analyzed commit 07f5beb and detected 28 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 26
Duplication 2

The test coverage on the diff in this pull request is 21.7% (80% is the threshold).

This pull request will bring the total coverage in the repository to 7.9% (0.3% change).

View more on Code Climate.

@CaMer0n CaMer0n added this to the e107 2.3.0 milestone Mar 28, 2020
@CaMer0n CaMer0n merged commit 4e78b5e into master Mar 28, 2020
@Deltik Deltik deleted the build-release branch March 29, 2020 00:46
@Moc
Copy link
Member

Moc commented Apr 29, 2020

@Deltik Can we add the instruction to upload in binary mode within the warning message? I forsee that more users will experience this issue, and having the direct instruction there on how to solve it might prevent (some) new issue reports.

Deltik added a commit that referenced this pull request Apr 29, 2020
Now advises admins to reupload using binary mode if the admin uploads
files with FTP
@Deltik
Copy link
Member Author

Deltik commented Apr 29, 2020

@Moc: Done in 7291ebc

I'm not sure how to localize the message, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement An improvement or new feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants