-
Notifications
You must be signed in to change notification settings - Fork 8
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
Speed up CI jobs with parallelism and caching #10
Draft
gregmedd
wants to merge
5
commits into
eclipse-uprotocol:main
Choose a base branch
from
gregmedd:feature/ci/must-go-faster
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Speed up CI jobs with parallelism and caching #10
gregmedd
wants to merge
5
commits into
eclipse-uprotocol:main
from
gregmedd:feature/ci/must-go-faster
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
* aligning * adding full build support * fixiing compilation * Update ci.yml * Update conanfile.py * Update ci.yml (x11) * Update main_rpc_client.cpp * Update main_sub.cpp * Update ci.yml (x6) * Update conanfile.py * Update ci.yml * Update conanfile.py (x2) * Update CMakeLists.txt (x2) * Update ci.yml * Update CMakeLists.txt (x5) * Update conanfile.py * Update CMakeLists.txt (x3) * Update ci.yml (x2) * Update CMakeLists.txt (x2) * Update ci.yml * cleaning * Update ci.yml * Update CMakeLists.txt * Update ci.yml * Update CMakeLists.txt (x3) * compilation fix * part eclipse-uprotocol#1 * continuing * continuing refactoring * fixing * aligning to changes * aligning tests * cleanup * aligning * Update CMakeLists.txt * Update ci.yml * Update conanfile.py * examples: Updated to use upZenohClient * examples: Use shared_ptr for reference counting * Use a shared_ptr for reference counting and remove the explicit calls to init/term. * fixing compilation * fixing samples * cleanup Co-authored-by: Bill Pittman <[email protected]>
Update CMakeLists.txt (x3) Update ci.yml (x19)
Address failing CI build (missing zenoh-c symbols) by switching to no-as-needed linking for zenoh-c and using a different library search technique in cmake. Adding -Wl,--no-as-needed is necessary when linking an application against a static library where *another* library is dependent on that static library's symbols. In this case, up-client-zenoh-cpp depends on zenoh-c, but does not incorporate those symbols directly. At link time, the linker will look at the symbols provided by zenoh-c and the symbols needed for the application binary, determine none are needed, and discard them. The -Wl,--no-as-needed flag tells the linker to embed those symbols into the application binary anyway. It was also necessary to explicitly pass a search directory and library name to the linker instead of providing cmake a path to a static library file. It is unclear why providing a library path was not working - the documentation indicates it should. However, using standard -L<path> and -l<library> flags did resolve the issue. I tested with each of the above solutions independently and neither one worked on its own. Both were required to fix the CI build. This change also re-enables build for examples that were previously disabled.
Builds on the pattern used in up-client-zenoh-cpp #33. eclipse-uprotocol/up-transport-zenoh-cpp#33 Diagnosing CI issues can take forever, thanks to long build times for dependencies that don't change very often. This change runs jobs in parallel whenever possible, and caches results for prerequesites of the example applications (based on hash of HEAD for their respective repos) This can trim over 4 minutes out of CI build times when iterating on CI changes.
Adds environment variable so that the target revision of dependency repos can be set to something other than HEAD if needed. Helps prevent churn / dependency failures with those repos are under active development. Note that the rev must be named, such as a branch or label. If it wouldn't work with `git rev-parse`, then it won't work here.
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.
Iterating on fixes for CI jobs can be time consuming, particularly when the issue exists at the end of a pipeline where some components take several minutes to build.
I used these CI changes during the investigations leading to the CI fix in #9. By caching results for build dependencies, test cycles were reduced by around four minutes. The main time savings come from:
The caches are tied to each repo's HEAD position, so if new commits are added the caches for that build stage are invalidated. Subsequent build stages will also re-run. Configuration parameters are provided for selecting a different named revision, if needed.
This uses the same pattern as eclipse-uprotocol/up-transport-zenoh-cpp#33, with only a small adjustment to caching structure and an additional stage for the examples from this repo.
This PR must wait until #9 is merged before it can be committed.
Use ➡️ this link ⬅️ to review the changes that are added by this PR.