Build cleanup #139
Merged
Build cleanup #139
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is likely more common action. Also updating submodules usually involves updating our patches which can't be automated.
So that all git-* targets are grouped together.
Possibly it did something in older versions, in current ones only the compiler set __EMSCRIPTEN__ macro is used in checks.
ThaUnknown
reviewed
May 29, 2022
The last released version of brotli we use, blocks the install target when building with emscripten, which is why we currently need to manually copy files of interest to their installed location. This adds complexity and is prone to failure with future changes. Upstream no longer blocks installations with emscripten since google/brotli@ce222e3 from 23.06.2021. We could just bump the brotli version to latest master, but then our already existing patch would need updating and from a quick look it appears as if other things changed as well which might not all be compatible with our baseline target out of the box. AN eventual new release with release notes will hopefully detail the changes in a more accessible form. As a safe option, just backport the upstream commit as another patch. We still need to normalise the static library names though.
emscripten already sets PKG_CONFIG_LIBDIR preventing non-wasm libraries installed in the system to be detected (and it doesn't provide a way for us to set _LIBDIR ourselves).
autoconf and cmake will pick up the variable from the environment.
To be more concise and to keep config flags from drifting apart (they already varied slightly between submodules).
Previously some builds used a hardcoded number of 8 and others didn't set it at all falling back to a single threaded build. This still isn't ideal though. What we actually want is to fix up or Makefile to allow builders to pass the number of jobs to the toplevel and then a buildserver can distribute the available jobs across the targets allowing inter-target parallelism. For now, this already is a clear improvement. The fribidi build appears to have a borked dependency on generated sources when running from a subdir, which resulted in parallel builds sometimes failing. To fix this the relevant header is now generated explicitly before other build steps.
As an intermediate measure improving build times with multiple jobs until the toplevel Makefile can properly handle parallelism.
They appear to have been commented out since the first public release and it's not clear what they were intended for or if they'd even still work with current emscripten.
The latter is required as it's picked up by buildsystems, the former is only used once by us and can easily be replaced by the latter.
Closed
A new version (0.16.0) was released.
This was referenced Dec 26, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up to the “patch cleanup” series and comments in #129.
From previous discussion:
.PHONYAdditionally this also simplifies the brotli build by backporting one commit from upstream (not yet found in a release) and as an intermediate measure to improve buildtimes until the Makefile can properly handle intra-target parallelism we now use “number of logical cores” for each build and the license extraction is split off into a separate file called running multiple jobs in parallel.
Judging from the runs so far this reduces the GHA times (2 logical cores available) from previously 21-18 minutes to now 15-13 minutes.