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

feat!: Rewrite build system and third-party dependencies #12

Merged
merged 91 commits into from
Oct 20, 2023

Conversation

joeyparrish
Copy link
Owner

@joeyparrish joeyparrish commented Oct 20, 2023

BEGIN_COMMIT_OVERRIDE
feat!: Replace gyp build system with CMake
feat!: Update all dependencies
fix: Fix Python 3.10+ compatibility in scripts
feat!: Drop Python 2 support in all scripts
feat!: Replace glog with absl::log, tweak log output and flags
feat: Add install target to build system
feat: Respect the file mode for HttpFiles (shaka-project#1081)
feat: Move all third-party deps into git submodules (shaka-project#1083)
fix: Fix local files with UTF8 names (shaka-project#1246)
fix: Fix build on FreeBSD (shaka-project#1287)
fix: Fix clang build (shaka-project#1288)
fix: Fix various build issues on macOS
fix: Fix various build issues on Windows
END_COMMIT_OVERRIDE

joeyparrish and others added 30 commits August 16, 2022 11:34
…#1072)

There are a lot of changes in this first phase, because there was a
lot of infrastructure required to get some meaningful amount of
porting done.  Future PRs should be simpler.

<b>Summary of changes:</b><details>

 - Remove old deps:
   - boringssl (replaced with mbedtls, lighter, easier to build)
   - gflags (replaced with absl::flags)
   - Chromium build tools
 - New deps to replace parts of Chromium base:
   - abseil-cpp
   - glog
   - nlohmann::json (for tests only)
 - Submodules, updates, and CMake build rules for third-party
   libraries:
   - curl
   - gmock/gtest
 - Ported internal libraries and their tests by removing Chromium deps
   and adding CMake build rules:
   - file (now using C++17 filesystem APIs)
   - license_notice
   - status
   - version
 - Test improvements
   - Removed file tests that can never be re-enabled
   - Re-enabled all other disabled file tests
   - Debug JSON values when HTTP tests fail
   - Fixed chunked-encoding issues in HTTP tests
 - Updated and refactored Dockerfiles testing
   - All docker files working, with OS versions updated to meet the
     new tool requirements
   - Local docker builds no longer write files to your working
     directory as root
   - Local docker builds can now be run in parallel without clobbering
     each others' build outputs
   - DEBUG=1 can drop you into an interactive shell when a docker
     build fails
 - Updated and heavily refactored workflows and Dockerfiles
   - All docker files now tested in parallel on GitHub, speeding up CI
   - All common workflow components broken out and using workflow_call
     instead of custom actions
   - Self-hosted runners now optional, to make testing easier on forks
   - CMake porting works-in-process can now be fully tested on GitHub
   - Building ported libraries and passing ported tests on all three
     platforms!
 - CI hacks for macOS removed, now testing on macos-latest!
 - Python2 no longer required!  (Only Python3)
 - Using strict build flags, treating all warnings as errors.

</details>

<b>Required to build:</b>

 - CMake >= 3.16
 - Python 3
 - A compiler supporting C++ >= 17
   - g++ >= 9 if using GCC (Clang also fine)
   - MSVC for Windows

<b>Still needs work:</b><details>

 - Moving other dependencies into submodules (if we keep them):
   - apple_apsl
   - icu
   - libevent
   - libpng
   - libwebm
   - libxml
   - modp_b64
   - protobuf
   - zlib
 - Port remaining internal libraries:
   - app
   - hls
   - media/base
   - media/chunking
   - media/codecs
   - media/crypto
   - media/demuxer
   - media/event
   - media/formats/dvb
   - media/formats/mp2t
   - media/formats/mp4
   - media/formats/packed_audio
   - media/formats/ttml
   - media/formats/webm
   - media/formats/webvtt
   - media/formats/wvm
   - media/origin
   - media/public
   - media/replicator
   - media/trick_play
   - mpd
 - Port main application
   - Add logging flags in absl and connect them to glog (which expects
     gflags)
 - Port pssh-box.py
 - Port main test targets (packager_test.py and packager_app.py)
 - Updating all requirement and build documentation
 - Remove any remaining refs to gclient, depot_tools, ninja
 - Update and complete release workflows using release-please
</details>

Issue shaka-project#346 (Switch to abseil)
Issue shaka-project#1047 (New build system)
This removes apple_apsl and libevent (which are no longer used) and
libxml and protobuf (which will be replaced in a follow-up with
submodules).

This change only contains deletions, to make it easier to review
separately from later additions for submodules and cmake integration.

Issue shaka-project#1047 (New build system)
For a read mode, make a GET request.  Otherwise, make a PUT request.
This also normalizes the structure of all submodule folders so that
there is a clear place to put configurations, outside the submodule
source, but limited in scope to that folder.

Issue shaka-project#1047 (New build system)
Note:
* An xHE-AAC capable encoder will auto adjust the user-specified SAP/RAP
  value to the allowed grid where SAP/RAPs can occur.
e.g.: `-rapInterval 5000` (5s) may result in actual SAPs/RAPs every
4.984s.
* To ensure SAP/RAP starts a new segment, Shaka needs to executed with a
  "--segment_duration" is less than or equal to that adjusted value.
* If every SAP/RAP should trigger a new segment, just set the segment
  length to a very low value e.g.: `--segment_duration 0.1`
The docs build in GitHub Actions broke with the release of jinja 3.1,
which introduced a breaking change that broke the cloud_sptheme sphinx
extension in use by our docs. That extension is not maintained any more,
so there will be no upstream fix. See also
https://foss.heptapod.net/doc-utils/cloud_sptheme/-/issues/47

That extension adds support for table styling (see
https://cloud-sptheme.readthedocs.io/en/latest/lib/cloud_sptheme.ext.table_styling.html),
but we only appear to have one table in the docs currently
(docs/source/options/segment_template_formatting.rst) and it does not
use the options added by the cloud_sptheme extension.

Therefore the best fix for GitHub Actions and any other system running
jinja 3.1 is to remove that extension from our config.
Build in parallel with however many cores are available on the system.
This might not affect build times in every environment (for example, if
GitHub Actions VMs are single core).

From the initial test run of this PR, we're seeing roughly 1x build
speed on Linux, 3x on macOS, and 1.5x on Windows, compared to the same
build step on a contemporaneous PR.
If another instance of the PR workflow is started for the same PR,
cancel the old one.  If a PR is updated and a new test run is started,
the old test run will be cancelled automatically to conserve
resources.
It is not necessary to change directory before invoking ctest.

Issue shaka-project#1047 (CMake porting)
For some reason, the status util unittest was in media/base/ instead of
status/

The broken headers in the status library were not obviously broken until
media/base/ porting started, and they were used for the first time.

Issue shaka-project#1047 (CMake porting)
…oject#1110)

The hack of using "secrets" to store per-repo settings was not working.
The main reason is that pull_request workflows don't have access to
secrets no matter what you do. So it was impossible to make this work
for settings like "ENABLE_SELF_HOSTED" for PR tests.

This change replaces that old hack with a new one. Now a repo owner must
create a "GitHub Environment" with the name of the setting they want to
enable. Currently supported values are "self_hosted", to add self-hosted
runners to the build/test matrix, and "debug", to start an SSH server
for debugging when a workflow fails.

Issue shaka-project#1047 (CMake porting)
Rewrite test_data_util.cc to locate files relative to the source file
itself, rather than using a service from chromium `base::`.

Issue shaka-project#1047 (CMake porting)
Issue shaka-project#346 (absl porting)
mbedtls works very differently from BoringSSL, and many changes had to
be made in the details of AES decryption to accomodate this.

Beyond the basic changes required for mbedtls, part of the CTS padding
implementation had to be rewritten. I believe this is because of an
assumption that held for BoringSSL, but not for mbedtls. I was unable to
determine what it was, so I rewrote the CTS decryption using reference
materials. After this, tests passed.

The deterministc PRNG I used with mbedtls in the RSA tests differs
somewhat from the old one, so the expected vectors had to be
regenerated. The old determinstic tests were also disabled, and are now
re-enabled.

Since cryptography is sensitive code, and because there were far more
changes needed here than just updating some headers and utility function
calls, this has been split into its own PR for separate review from the
rest of the media/base porting work.

Issue shaka-project#1047 (CMake porting)
Issue shaka-project#346 (absl porting)
This seems to bring current docker build times in the cmake branch from
4-6 minutes down to 3-4 minutes.
This does not depend on absl, which frees macros.h from any library deps
that would make the CMake dependency tree more complicated. This also
fixes build errors in some environments.
This appears to fix some issues with crashing compilers on arm64 and
docker builds. This may be caused by resource constraints in those
environments.
See also:
 - protocolbuffers/protobuf#10899
 - protocolbuffers/protobuf#10900

Also updates flags to deal with various compile-time warnings/errors for
protobuf libs on all platforms.
This removes all chromium dependencies from media/base/ and completes
the build system in CMake.

The ClosureThread class and its classes were removed, as they were
specific to chromium base. ClosureThread has been replaced by
std::thread.

The byte-swapping utilities in network_util.cc have been removed and
replaced with absl.

generate_unique_temp_path() was split out of file_unittest.cc into
file_test_util.cc, where other test suites could make use of it.

WARN_UNUSED_RESULT was replaced with the C++ standard attribute
[[nodiscard]].

The base::Clock interface was replaced with a typedef for a function
pointer that returns the current time.

This re-enables the tests in http_key_fetcher_unittest.cc by using
httpbin.org.

Issue shaka-project#1047 (CMake porting)
Issue shaka-project#346 (absl porting)
This fixes a libxml2 build failure in release mode and a build failure
in generated proto code.
cosmin and others added 28 commits August 21, 2023 17:22
Related to issue shaka-project#1047

After this it should be possible to build a working `packager` application from the `cmake` branch. Some further logging improvements may be needed to get full parity with the `main` branch in terms of ability to do verbose debug logging, but other than that everything is expected to work.
…ct#1264)

build in CMake branch is broken at least locally on OS X after shaka-project#1210 was
merged which introduced some new warnings and reintroduced
base::nullopt.
This feeds into efforts to create a working install target.

The order of headers is still funky, and was "fixed" by clang-format,
but in a way that doesn't exactly align with the style guide. Further
cleanup of header order is coming in a follow-up PR.
Reorder headers to follow the Google C++ Style Guide:

> In dir/foo.cc or dir/foo_test.cc:
>
> 1. dir2/foo2.h.
> 2. A blank line
> 3. C system headers (more precisely: headers in angle brackets with
the .h extension), e.g., <unistd.h>, <stdlib.h>.
> 4. A blank line
> 5. C++ standard library headers (without file extension), e.g.,
<algorithm>, <cstddef>.
> 6. A blank line
> 7. Other libraries' .h files.
> 8. A blank line
> 9. Your project's .h files.


https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
This deprecates --vmodule (not available in absl, mapping it to --v for
a very minimal sort of compatibility) and adds the absl::log flags
--minloglevel, --stderrthreshold, --log_backtrace_at, and --log_prefix.
This organizes all public headers for the library into
include/  and makes sure then don't rely on any headers from
other folders.

To accomplish this, this change also refactors macros.h,
media/base/macros.h, and status/status_macros.h into macros/classes.h,
macros/compiler.h, macros/crypto.h, macros/logging.h, macros/status.h,
and public/export.h.  Now the export macros from macros.h live in
include/ to keep include/ from requiring anything else.

This refactor enables an install target that includes public headers
only.

---------

Co-authored-by: Cosmin Stejerean <[email protected]>
this adds an install target which will install binaries, libraries, pkg-config and headers, along with a simple link-test program to verify the installed library and headers work.

---------

Co-authored-by: Joey Parrish <[email protected]>
Updated the build docs for Linux, Mac and Windows.

Updated the Linux profiling docs to remove references to things that don't work anymore. 


---------

Co-authored-by: Joey Parrish <[email protected]>
The link-test targets rebuilt every time you built the project because
that is the behavior of commands attached to add_custom_target. This is
fixed by combining add_custom_command and add_custom_target.

Fixing that led to a situation where the link-test targets built on the
first build, but didn't properly rebuild when their dependencies
changed. This is fixed by using set_source_file_properties to treat
test.cc as dirty when the test-install command runs.
…#1279)

pssh-box.py needs Python protos, which we were not building. It also
needs to have those protos installed properly.

This fixes generation of Python proto interfaces and fixes the install
targets to put those protos where they are needed. This also updates the
documentation to match.
…t#1283)

This caused issues with some shared library link tests when I tried to
split the library target into shared and static targets.
This is used to run a fairly large number of integrations tests (which are exposing some failures on the CMake branch). Let's copy these over first to enable the integration tests workflow.

Actually running integration tests in CTest is off by default (gated by `SKIP_INTEGRATION_TESTS` while we fix the tests).
c-ares (used on Linux only) was an exception to the rule of only linking
against internally-built libraries. This fixes that, to support a truly
static build of packager.
The upstream repo at gnome.org sometimes fails in CI
Use the standard BUILD_SHARED_LIBS variable instead of the custom
LIBPACKAGER_SHARED variable
This splits the target so that we always build static libraries, but
optionally also build shared libraries.  This also tweaks the
installation so that the static library is never installed, because it
is not usable without the other internal static library deps.  The
headers and pkgconfig will only be installed if we have a shared
library to use them with.
This updates our workflows to collect build artifacts.
This also updates deprecated docker commands and the paths to
artifacts in Dockerfile.
This adopts release-please to manage releases and changelogs, similar
to other shaka-project repos.
@joeyparrish joeyparrish merged commit 98ae717 into main Oct 20, 2023
10 checks passed
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.

6 participants