Skip to content

Add a android log implementation #10938

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

Merged
merged 1 commit into from
May 20, 2025
Merged

Add a android log implementation #10938

merged 1 commit into from
May 20, 2025

Conversation

kirklandsign
Copy link
Contributor

No description provided.

Copy link

pytorch-bot bot commented May 16, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/10938

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit bc62fa4 with merge base 9aedbeb (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 16, 2025
@facebook-github-bot
Copy link
Contributor

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Comment on lines +46 to +48
"DEFAULT": _select_pal({
"minimal": ["default/minimal.cpp"],
"posix": ["default/posix.cpp"],
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if this should be another "android" entry

Copy link
Contributor

Choose a reason for hiding this comment

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

@larryliu0820 that would require users to manually set -c executorch.pal_default=android. I think what @kirklandsign is doing here is the best way forward because it's automatic. However, it might also cause some confusion for users since executorch.pal_default is now ignored

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However, it might also cause some confusion for users since executorch.pal_default is now ignored

That makes sense, but ovr_config//os:android is automatic.

@facebook-github-bot
Copy link
Contributor

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@kirklandsign kirklandsign marked this pull request as ready for review May 17, 2025 00:25
@facebook-github-bot
Copy link
Contributor

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

deps = [
":pal_interface",
"fbsource//third-party/toolchains:log",
Copy link
Contributor

Choose a reason for hiding this comment

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

This will break in OSS buck right? Can you also hide it under ovr_config android?

facebook-github-bot pushed a commit that referenced this pull request May 19, 2025
Summary: Pull Request resolved: #10938

Differential Revision: D74865527

Pulled By: kirklandsign
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D74865527

facebook-github-bot pushed a commit that referenced this pull request May 19, 2025
Summary: Pull Request resolved: #10938

Differential Revision: D74865527

Pulled By: kirklandsign
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D74865527

facebook-github-bot pushed a commit that referenced this pull request May 19, 2025
Summary: Pull Request resolved: #10938

Differential Revision: D74865527

Pulled By: kirklandsign
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D74865527

facebook-github-bot pushed a commit that referenced this pull request May 19, 2025
Summary: Pull Request resolved: #10938

Differential Revision: D74865527

Pulled By: kirklandsign
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D74865527

@kirklandsign kirklandsign added release notes: none Do not include this in the release notes release notes: api Changes to public facing apis (any interfaces, pybinded runtime methods, etc.) and removed release notes: none Do not include this in the release notes labels May 19, 2025
Summary: Pull Request resolved: #10938

Differential Revision: D74865527

Pulled By: kirklandsign
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D74865527

Copy link
Contributor

@jathu jathu left a comment

Choose a reason for hiding this comment

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

build changes lgtm

@facebook-github-bot facebook-github-bot merged commit 7d194cf into main May 20, 2025
88 of 90 checks passed
@facebook-github-bot facebook-github-bot deleted the android-log branch May 20, 2025 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported release notes: api Changes to public facing apis (any interfaces, pybinded runtime methods, etc.) topic: not user facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants