Skip to content

ci: Add trigger for Android and iOS CI jobs on source/common changes#38733

Merged
phlax merged 2 commits intoenvoyproxy:mainfrom
abeyad:mobile-ci-trigger
Mar 14, 2025
Merged

ci: Add trigger for Android and iOS CI jobs on source/common changes#38733
phlax merged 2 commits intoenvoyproxy:mainfrom
abeyad:mobile-ci-trigger

Conversation

@abeyad
Copy link
Copy Markdown
Contributor

@abeyad abeyad commented Mar 13, 2025

There have been several instances that changes, particularily to source/* directories, have introduced C++ features not supported by the Android NDK or iOS XCode version for Envoy Mobile. Without triggering the mobile Android and iOS jobs on those changes, we don't catch them until after the fact.

The latest example is the std::aligned_alloc PR: #38707.

Which introduced build errors on Android that CI didn't catch at the time of the PR and only in a subsequent PR that triggered mobile CI: https://github.com/envoyproxy/envoy/actions/runs/13818727995/job/38658598098.

This change causes any changes to source/common/ to trigger Android and iOS CI. This is a start in allowing us to catch such changes in the PR making the changes.

There have been several instances that changes, particularily to
source/* directories, have introduced C++ features not supported by the
Android NDK or iOS XCode version for Envoy Mobile.  Without triggering
the mobile Android and iOS jobs on those changes, we don't catch them
until after the fact.

The latest example is the std::aligned_alloc PR:
envoyproxy#38707.

Which introduced build errors on Android that CI didn't catch at the
time of the PR and only in a subsequent PR that triggered mobile CI:
https://github.com/envoyproxy/envoy/actions/runs/13818727995/job/38658598098.

This change causes any changes to `source/common/` to trigger Android
and iOS CI.  This is a start in allowing us to catch such changes in the
PR making the changes.

Signed-off-by: Ali Beyad <abeyad@google.com>
@abeyad
Copy link
Copy Markdown
Contributor Author

abeyad commented Mar 13, 2025

/assign @phlax

@abeyad
Copy link
Copy Markdown
Contributor Author

abeyad commented Mar 13, 2025

@phlax what do you think?

@abeyad abeyad changed the title ci: Add trigger for Android and iOS CI jobs on source/common/* changes ci: Add trigger for Android and iOS CI jobs on source/common changes Mar 13, 2025
@phlax
Copy link
Copy Markdown
Member

phlax commented Mar 13, 2025

probably a good idea - iirc we held off doing this previously for fear of the ci cost - but the issue comes up from time to time

@abeyad
Copy link
Copy Markdown
Contributor Author

abeyad commented Mar 13, 2025

probably a good idea - iirc we held off doing this previously for fear of the ci cost - but the issue comes up from time to time

We can start off with just doing this for source/common/** and see what the CI costs end up being? If we're still under budget, great, if not, we can address at that time?

Copy link
Copy Markdown
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks @abeyad

my main concern is the macos ci as that is very expensive to run (and uber flakey)

with the linux/rbe stuff its more about capacity - i think we should be good there

@abeyad
Copy link
Copy Markdown
Contributor Author

abeyad commented Mar 14, 2025

/retest

@abeyad
Copy link
Copy Markdown
Contributor Author

abeyad commented Mar 14, 2025

@phlax is this good to merge?

@phlax phlax merged commit 7cb808b into envoyproxy:main Mar 14, 2025
24 checks passed
@abeyad abeyad deleted the mobile-ci-trigger branch March 14, 2025 16:35
jewertow pushed a commit to jewertow/envoy that referenced this pull request Apr 2, 2025
…nvoyproxy#38733)

There have been several instances that changes, particularily to
source/* directories, have introduced C++ features not supported by the
Android NDK or iOS XCode version for Envoy Mobile. Without triggering
the mobile Android and iOS jobs on those changes, we don't catch them
until after the fact.

The latest example is the std::aligned_alloc PR:
envoyproxy#38707.

Which introduced build errors on Android that CI didn't catch at the
time of the PR and only in a subsequent PR that triggered mobile CI:
https://github.com/envoyproxy/envoy/actions/runs/13818727995/job/38658598098.

This change causes any changes to `source/common/` to trigger Android
and iOS CI. This is a start in allowing us to catch such changes in the
PR making the changes.

Signed-off-by: Ali Beyad <abeyad@google.com>
agrawroh pushed a commit to agrawroh/envoy that referenced this pull request Apr 9, 2025
…nvoyproxy#38733)

There have been several instances that changes, particularily to
source/* directories, have introduced C++ features not supported by the
Android NDK or iOS XCode version for Envoy Mobile. Without triggering
the mobile Android and iOS jobs on those changes, we don't catch them
until after the fact.

The latest example is the std::aligned_alloc PR:
envoyproxy#38707.

Which introduced build errors on Android that CI didn't catch at the
time of the PR and only in a subsequent PR that triggered mobile CI:
https://github.com/envoyproxy/envoy/actions/runs/13818727995/job/38658598098.

This change causes any changes to `source/common/` to trigger Android
and iOS CI. This is a start in allowing us to catch such changes in the
PR making the changes.

Signed-off-by: Ali Beyad <abeyad@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants