Skip to content

Add support to build cobalt/starboard using an external RDK toolchain - #5166

Merged
niranjanyardi merged 1 commit into
youtube:25.lts.1+from
andrunko:external-rdk-toolchain-support-cobalt-only
Apr 11, 2025
Merged

niranjanyardi merged 1 commit into
youtube:25.lts.1+from
andrunko:external-rdk-toolchain-support-cobalt-only

Conversation

@andrunko

@andrunko andrunko commented Mar 24, 2025 •

Copy link
Copy Markdown
Collaborator

b/402053134

This is a work in progress to add support for building cobalt using an external RDK toolchain. The actual integration expects the RDK starboard implementation to be located under starboard/contrib/rdk and won't work without it - this will be done (likely as a git subtree) once the RDK starboard changes are integrated into https://cobalt.googlesource.com/ (proposed changes currently under review) where they live.

@andrunko

Copy link
Copy Markdown
Collaborator Author

Note that these changes currently disable .rsp files as it breaks the build when using the external RDK toolchain - looking into it.

@andrunko andrunko changed the title External rdk toolchain support cobalt only Add support to build cobalt/starboard using an external RDK toolchain Mar 24, 2025
@andrunko
andrunko force-pushed the external-rdk-toolchain-support-cobalt-only branch 6 times, most recently from 39a41dc to a223136 Compare April 3, 2025 19:54
Comment thread docker-compose.yml Resolved
Comment thread docker-compose.yml Resolved
@niranjanyardi
niranjanyardi force-pushed the external-rdk-toolchain-support-cobalt-only branch from a223136 to 6bbf9b6 Compare April 7, 2025 22:09
Comment thread starboard/build/config/BUILDCONFIG.gn Resolved
@niranjanyardi

Copy link
Copy Markdown
Member

apart from the the open comments lgtm!

@andrunko
andrunko force-pushed the external-rdk-toolchain-support-cobalt-only branch from 6bbf9b6 to 33c50d3 Compare April 9, 2025 01:27
@andrunko andrunko mentioned this pull request Apr 9, 2025
Comment thread docker/linux/rdk/Dockerfile Resolved
@niranjanyardi
niranjanyardi requested a review from kaidokert April 10, 2025 18:23
@niranjanyardi

Copy link
Copy Markdown
Member

@andrunko can you see why the required checks are failing . maybe rerun the failing checks to ensure there's no flakes.

@niranjanyardi niranjanyardi added the runtest Trigger presubmit workflows label Apr 10, 2025
@andrunko

Copy link
Copy Markdown
Collaborator Author

@andrunko can you see why the required checks are failing . maybe rerun the failing checks to ensure there's no flakes.

The failures seem to be caused by this line on docker-compose.yml:

    - ${RDK_SDK:-.}:${RDK_SDK}

This was added to support users extracting the SDK anywhere, as the SDK executables expect their path to be the same as the one where they got extracted to, so we need to mount the SDK in the same place inside the container where it was extracted outside the container.

I was initially mounting $HOME:$HOME here, but this would only work if the SDK is extracted somewhere inside $HOME outside the container as well. I changed it after #5166 (comment), which would solve the issue when mounting outside $HOME, but would also require all containers to have RDK_SDK set.

Note we only need that until we get the sysroot uploaded somewhere public, then we can extract it directly from the docker container anywhere and set the env vars accordingly.

For now, the options I can see:

  • mount $HOME and ask any user trying to build for RDK to extract the sysroot inside their $HOME as well
    • this considers CI has $HOME set, which is likely
  • always set RDK_SDK for all CI jobs

Thoughts?

@niranjanyardi

Copy link
Copy Markdown
Member

@andrunko can you see why the required checks are failing . maybe rerun the failing checks to ensure there's no flakes.

The failures seem to be caused by this line on docker-compose.yml:

    - ${RDK_SDK:-.}:${RDK_SDK}

This was added to support users extracting the SDK anywhere, as the SDK executables expect their path to be the same as the one where they got extracted to, so we need to mount the SDK in the same place inside the container where it was extracted outside the container.

I was initially mounting $HOME:$HOME here, but this would only work if the SDK is extracted somewhere inside $HOME outside the container as well. I changed it after #5166 (comment), which would solve the issue when mounting outside $HOME, but would also require all containers to have RDK_SDK set.

Note we only need that until we get the sysroot uploaded somewhere public, then we can extract it directly from the docker container anywhere and set the env vars accordingly.

For now, the options I can see:

  • mount $HOME and ask any user trying to build for RDK to extract the sysroot inside their $HOME as well

    • this considers CI has $HOME set, which is likely
  • always set RDK_SDK for all CI jobs

Thoughts?

@oxve thoughts? the HOME option sounds like what we would use currently

@jonastsai

Copy link
Copy Markdown
Contributor

if possible, try not to ask the user to install the SDK in $HOME. On some partner environment, they have very limited disc quota at $HOME because it is system disc. but they have large shared work space mounted on other paths. And engineers from the same partner can share the installed SDK if it is installed in system folder. Having a symbolic link in $HOME might not solve the issue but it is not straightforward.

Can we set the RDK_SDK using ENV in Dockerfile which point to the default value we expected to be installed? And partner can overwrite it if they specifically set the value in bash env?

@jonastsai

Copy link
Copy Markdown
Contributor

the rest of changes look good to me

@andrunko

andrunko commented Apr 11, 2025 •

Copy link
Copy Markdown
Collaborator Author

if possible, try not to ask the user to install the SDK in $HOME. On some partner environment, they have very limited disc quota at $HOME because it is system disc. but they have large shared work space mounted on other paths. And engineers from the same partner can share the installed SDK if it is installed in system folder. Having a symbolic link in $HOME might not solve the issue but it is not straightforward.

Can we set the RDK_SDK using ENV in Dockerfile which point to the default value we expected to be installed? And partner can overwrite it if they specifically set the value in bash env?

We cant use ENV as the sysroot needs to be mounted inside the container exactly in the same location it was extracted outside the container. I've updated the PR again to use HOME for now (which in theory should solve the checks here), but another option would be to have a fixed location where the sysroot needs to be extracted (outside the container), and then we mount this hardcoded location in the same place inside the container - e.g. /opt/rdk-sysroot or something like that.

Please let me know what you think and if you decide on a hardcoded path, which one we should use here.

@andrunko
andrunko force-pushed the external-rdk-toolchain-support-cobalt-only branch from 33c50d3 to dfce61f Compare April 11, 2025 02:14
@andrunko

andrunko commented Apr 11, 2025 •

Copy link
Copy Markdown
Collaborator Author

if possible, try not to ask the user to install the SDK in $HOME. On some partner environment, they have very limited disc quota at $HOME because it is system disc. but they have large shared work space mounted on other paths. And engineers from the same partner can share the installed SDK if it is installed in system folder. Having a symbolic link in $HOME might not solve the issue but it is not straightforward.
Can we set the RDK_SDK using ENV in Dockerfile which point to the default value we expected to be installed? And partner can overwrite it if they specifically set the value in bash env?

We cant use ENV as the sysroot needs to be mounted inside the container exactly in the same location it was extracted outside the container. I've updated the PR again to use HOME for now (which in theory should solve the checks here), but another option would be to have a fixed location where the sysroot needs to be extracted (outside the container), and then we mount this hardcoded location in the same place inside the container - e.g. /opt/rdk-sysroot or something like that.

Please let me know what you think and if you decide on a hardcoded path, which one we should use here.

Using a hardcoded path could also make the checks here fail, e.g. if we try to mount /opt/rdk-sysroot:/opt/rdk-sysroot but this dir doesnt exist outside the container (on CI), the checks will likely fail.

Tbh the best I can think atm is to use $HOME until we get the public sysroot available, then we get rid of all this.

@andrunko

Copy link
Copy Markdown
Collaborator Author

The new CI failures seem unrelated?

Post job cleanup.
/usr/local/bin/docker exec  5a6448ea877[2](https://github.com/youtube/cobalt/actions/runs/14394116159/job/40367256156?pr=5166#step:30:2)49e4e732fbba93873e66026e230ab1b9b8c4691aec1452b4f5df sh -c "cat /etc/*release | grep ^ID"
/__e/node20/bin/node: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.28' not found (required by /__e/node20/bin/node)

This defines the `rdk_starboard_root` gn arg that can be used to set the location
of the RDK starboard code (default to `//starboard/contrib/rdk`).

It also forwards cflags/cxxflags when using the clang toolchain, needed to
to pass the rdk sources' include dir to compilers and thus avoid touching
the sources which include "third_party/...".

b/402053134
@andrunko
andrunko force-pushed the external-rdk-toolchain-support-cobalt-only branch from dfce61f to ca07fbf Compare April 11, 2025 02:45
@niranjanyardi
niranjanyardi merged commit d07900e into youtube:25.lts.1+ Apr 11, 2025
niranjanyardi pushed a commit that referenced this pull request May 29, 2025
Add github actions to build RDK evergreen platform
Depends on #5166.

b/406991008
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

runtest Trigger presubmit workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants