Skip to content

Deduplicate RunEnvironmentInfo env-merging logic between appimage.bzl and mkapprun.bzl - #436

Merged
lalten merged 2 commits into
mainfrom
copilot/refactor-duplicate-logic
Aug 2, 2026
Merged

Deduplicate RunEnvironmentInfo env-merging logic between appimage.bzl and mkapprun.bzl#436
lalten merged 2 commits into
mainfrom
copilot/refactor-duplicate-logic

Conversation

Copilot AI commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

The RunEnvironmentInfo env-merging block (merging the binary target's runtime env with the appimage/apprun target's own env attribute) was duplicated verbatim in both appimage/appimage.bzl and appimage/private/mkapprun.bzl.

Changes

  • Extracted the merging logic into a new get_merged_env(ctx) helper in appimage/private/runfiles.bzl, which both files already depend on.
  • Updated _appimage_impl in appimage.bzl and _make_env_sh in mkapprun.bzl to call the shared helper instead of inlining the logic.
  • Docstring kept to a single line, consistent with the other helpers in runfiles.bzl.
def get_merged_env(ctx):
    """Take the `binary` env and add the appimage target's env on top of it."""
    env = {}
    if RunEnvironmentInfo in ctx.attr.binary:
        env.update(ctx.attr.binary[RunEnvironmentInfo].environment)
    env.update(ctx.attr.env)
    return env

No behavioral change — both call sites operate on the same ctx shape as before.

@lalten
lalten marked this pull request as ready for review August 2, 2026 09:00
Copilot AI review requested due to automatic review settings August 2, 2026 09:00
@lalten
lalten enabled auto-merge (squash) August 2, 2026 09:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR removes duplicated RunEnvironmentInfo environment-merging logic by centralizing it into a shared helper, reducing repetition across the AppImage rule implementation and the AppRun script generation.

Changes:

  • Added get_merged_env(ctx) to appimage/private/runfiles.bzl to merge binary’s RunEnvironmentInfo.environment with the rule’s env attribute (rule env overrides binary env).
  • Updated appimage/appimage.bzl to call get_merged_env(ctx) instead of inlining the merge logic.
  • Updated appimage/private/mkapprun.bzl to call get_merged_env(ctx) and adjusted loads accordingly.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
appimage/private/runfiles.bzl Introduces get_merged_env(ctx) helper to centralize env merge behavior.
appimage/private/mkapprun.bzl Replaces duplicated env merge block with a call to get_merged_env(ctx).
appimage/appimage.bzl Replaces duplicated env merge block with a call to get_merged_env(ctx).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lalten
lalten merged commit 538d23c into main Aug 2, 2026
20 checks passed
@lalten
lalten deleted the copilot/refactor-duplicate-logic branch August 2, 2026 09:24
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