Skip to content

Conversation

@leodido
Copy link
Contributor

@leodido leodido commented Dec 12, 2025

Summary

Resolve builtin variables (${__pkg_version}, ${__git_commit}, ${__git_commit_short}) in PackageInternal fields (prep, env), not just in Config.

Part of https://linear.app/ona-team/issue/CLC-2147/fix-non-deterministic-version-across-packages

Problem

resolveBuiltinVariables() only resolved builtin variables in Config, but FindUnresolvedArguments() checks both PackageInternal and Config. This caused builtin variables in prep or env fields to be flagged as unresolved, failing the build with:

cannot build with unresolved argument "${__git_commit_short}": use -D__git_commit_short=value to set the argument

Example BUILD.yaml that would fail:

packages:
  - name: app
    type: yarn
    prep:
      - ["/bin/bash", "prepare.sh", "${__pkg_version}"]  # This was NOT resolved
    config:
      commands:
        build: ["./package.sh", "${__git_commit_short}"]  # This WAS resolved

Solution

Resolve builtin variables in PackageInternal before resolving in Config.

Testing

  • Added TestResolveBuiltinVariablesInPackageInternal test
  • All existing tests pass

The resolveBuiltinVariables function only resolved builtin variables
(${__pkg_version}, ${__git_commit}, ${__git_commit_short}) in Config,
but FindUnresolvedArguments checks both PackageInternal and Config.

This caused builtin variables in prep or env fields to be flagged as
unresolved, failing the build with errors like:
  cannot build with unresolved argument "${__git_commit_short}"

Now resolves builtin variables in PackageInternal before Config.

Co-authored-by: Ona <no-reply@ona.com>
@leodido leodido force-pushed the ldd/resolve-builtin-pkg-internal branch 3 times, most recently from 7b5ffb0 to c53da73 Compare December 13, 2025 15:54
@leodido leodido merged commit d592e02 into main Dec 15, 2025
22 checks passed
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.

3 participants