Skip to content

Improve build-te for build-jax step in CI - #2245

Open
Steboss wants to merge 17 commits into
mainfrom
sbosisio/test-te-build
Open

Improve build-te for build-jax step in CI#2245
Steboss wants to merge 17 commits into
mainfrom
sbosisio/test-te-build

Conversation

@Steboss

@Steboss Steboss commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@copy-pr-bot

copy-pr-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@Steboss

Steboss commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 3448da0

@github-actions

Copy link
Copy Markdown

📖 Docs preview

@Steboss

Steboss commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 3448da0

@Steboss

Steboss commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 6285bfc

@github-actions

Copy link
Copy Markdown

📖 Docs preview

@Steboss

Steboss commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test cb1317d

@github-actions

Copy link
Copy Markdown

📖 Docs preview

@@ -0,0 +1,160 @@
#!/bin/bash

@Steboss Steboss Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

0.17.0 is released, can this all be dropped?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not yet, I tried to get this script in a better shape though.
The reason is that sccache 0.17.0 was released before the CUDA 13.3 fix, so there is no prebuilt sccache binary that contains the CUDA 13.3 fix yet

# Install ccache if not present (needs >= 4.1 for Redis remote storage support)
if ! command -v ccache &> /dev/null; then
apt-get update && apt-get install -y --no-install-recommends ccache
NVTE_CCACHE_BIN="${NVTE_CCACHE_BIN:-ccache}"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If we want to use sscache, then we need the install script

export NVTE_USE_CCACHE=1
ccache --zero-stats
export NVTE_CCACHE_BIN
case "${CACHE_PROGRAM}" in

@Steboss Steboss Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here we're initializing the compiler-cache backend.

For sccache, we explicitly start the daemon, disable its idle shutdown for the
duration of the long CUDA build (line 216)

@@ -0,0 +1,44 @@
#!/usr/bin/env bash

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

build-te-with-sccache.sh is a thin BuildKit-specific wrapper around
build-te.sh. It keeps Docker secret handling separate from the normal
Transformer Engine build logic.

When ENABLE_NVTE_SCCACHE=1, the wrapper:

  • Verifies that the temporary AWS and S3 BuildKit secret files are present.
  • Reads them without command tracing and exports them only to the current
    process tree.
  • Configures sccache to use TLS and an architecture-specific S3 prefix.
  • Adds --ccache to the build-te.sh invocation.

@Steboss
Steboss requested a review from olupton July 31, 2026 20:13

@olupton olupton left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't see a CI run from the latest diff? It seems like a lot of code and branches. Do we really need to support both ccache and sccache?

needs: build-base
runs-on: ${{ inputs.ARCHITECTURE == 'amd64' && 'linux-amd64-cpu32m' || 'linux-arm64-cpu32m' }}
env:
CACHE_AWS_REGION: ${{ vars.AWS_REGION || secrets.AWS_REGION }}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Old test, going to remove this bit

id: build-jax
uses: ./.github/actions/build-container
env:
SCCACHE_BUCKET: ${{ secrets.BAZEL_REMOTE_BUCKET }}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why 4 entries in mealkit-secret-envs and 1 here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The env creates and env variable for build-jax, and it does not need the AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN, as the OIDC action that we have is already automatically providing these variables.
The mealkit-secret-envs is an explicit list of which existing runner environment variables we have that may cross into the Docker build. Since then we have Docker running, we do nee dto pass thes evariable somehow,and this is the only way I found.

@@ -0,0 +1,160 @@
#!/bin/bash

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

0.17.0 is released, can this all be dropped?

@Steboss

Steboss commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 95ab9f9

@github-actions

Copy link
Copy Markdown

📖 Docs preview

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants