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

APP-6696 include os_version tag on GOOS=darwin #4536

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

abe-winter
Copy link
Member

@abe-winter abe-winter commented Nov 6, 2024

What changed

  • add readDarwinTags which supplies os_version platform tag. (on linux systems we already get this from /etc/os-release)
  • incidental: make the 5-second timer apply to the entire read tags function, rather than just the readGPUTags function

Why

Macos version targeting for binaries.

Manual testing

On a device with:

$ sw_vers --productVersion
14.6.1

RDK logged:

{"tags":"os_version:14"}

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Nov 6, 2024
Copy link
Member

@cheukt cheukt left a comment

Choose a reason for hiding this comment

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

LGTM, any reason why we don't just return the full version string?

@abe-winter
Copy link
Member Author

abe-winter commented Nov 13, 2024

why we don't just return the full version string

use case for this is to deploy modules based on compatibility; and today we only have string matching, not semver

(i.e. a module targeting os_version:14 won't match os_version:14.6.1)

if we returned full 14.6.1, module authors would have to build their module for each patch version

@abe-winter abe-winter merged commit 3439e38 into viamrobotics:main Nov 13, 2024
18 checks passed
@abe-winter abe-winter deleted the darwin-tags branch November 13, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants