Skip to content

build-trigger: skip aliased derivations #518

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+30 −3
Open

build-trigger: skip aliased derivations #518

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f54d0b4
build-trigger: skip aliased derivations
Mic92 Sep 30, 2025
65ca5a1
handle canceled builds more gracefully
Mic92 Sep 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 30 additions & 3 deletions buildbot_nix/buildbot_nix/build_trigger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@
from buildbot.plugins import steps, util
from buildbot.process import buildstep
from buildbot.process.properties import Properties
from buildbot.process.results import ALL_RESULTS, SUCCESS, statusToString, worst_status
from buildbot.process.results import (
ALL_RESULTS,
CANCELLED,
SUCCESS,
statusToString,
worst_status,
)
from buildbot.reporters.utils import getURLForBuild, getURLForBuildrequest
from twisted.internet import defer

Expand Down Expand Up @@ -95,6 +101,7 @@ class SchedulingContext:
scheduled: list[BuildTrigger.ScheduledJob]
scheduler_log: StreamLog
schedule_now: list[NixEvalJobSuccess]
seen_drvs: dict[str, str] # Maps drvPath to attr name of first occurrence

def __init__(
self,
Expand Down Expand Up @@ -530,7 +537,18 @@ async def _schedule_ready_jobs(
) -> list[NixEvalJobSuccess]:
"""Schedule jobs that are ready to run. Returns list of skipped jobs."""
skipped_jobs = []
for job in ctx.schedule_now:
for job in sorted(ctx.schedule_now, key=lambda j: j.attr):
# Check if we've already scheduled a build for this derivation (alias detection)
if job.drvPath in ctx.seen_drvs:
original_attr = ctx.seen_drvs[job.drvPath]
ctx.scheduler_log.addStdout(
f"\t- {job.attr} (skipped, alias of {original_attr})\n"
)
skipped_jobs.append(job)
self._skipped_count += 1
continue
ctx.seen_drvs[job.drvPath] = job.attr
Comment on lines +542 to +550
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential issue: Aliased jobs don't set required output path properties.

When a job is skipped as an alias, schedule_success is never called (due to continue at line 543), so the properties normally set at lines 216-217 ({attr}-out_path, {attr}-drv_path) are never stored in build_props. This differs from the "already built" skip case (lines 560-565), where schedule_success is called first and sets these properties.

If downstream processes (e.g., gcroot registration) expect these properties for all jobs, this could cause issues.

Consider calling schedule_success for aliased jobs or manually setting the required properties before skipping:

         for job in sorted(ctx.schedule_now, key=lambda j: j.attr):
             # Check if we've already scheduled a build for this derivation (alias detection)
             if job.drvPath in ctx.seen_drvs:
                 original_attr = ctx.seen_drvs[job.drvPath]
+                # Still need to set output path properties for downstream processes
+                source = "nix-eval-nix"
+                out_path = job.outputs["out"] or None
+                build_props.setProperty(f"{job.attr}-out_path", out_path, source)
+                build_props.setProperty(f"{job.attr}-drv_path", job.drvPath, source)
+                if out_path and job.cacheStatus == CacheStatus.local:
+                    build_props.setProperty(f"{job.attr}-skipped_out_path", out_path, source)
                 ctx.scheduler_log.addStdout(
                     f"\t- {job.attr} (skipped, alias of {original_attr})\n"
                 )
                 skipped_jobs.append(job)
                 self._skipped_count += 1
                 continue
             ctx.seen_drvs[job.drvPath] = job.attr
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if job.drvPath in ctx.seen_drvs:
original_attr = ctx.seen_drvs[job.drvPath]
ctx.scheduler_log.addStdout(
f"\t- {job.attr} (skipped, alias of {original_attr})\n"
)
skipped_jobs.append(job)
self._skipped_count += 1
continue
ctx.seen_drvs[job.drvPath] = job.attr
if job.drvPath in ctx.seen_drvs:
original_attr = ctx.seen_drvs[job.drvPath]
# Still need to set output path properties for downstream processes
source = "nix-eval-nix"
out_path = job.outputs["out"] or None
build_props.setProperty(f"{job.attr}-out_path", out_path, source)
build_props.setProperty(f"{job.attr}-drv_path", job.drvPath, source)
if out_path and job.cacheStatus == CacheStatus.local:
build_props.setProperty(f"{job.attr}-skipped_out_path", out_path, source)
ctx.scheduler_log.addStdout(
f"\t- {job.attr} (skipped, alias of {original_attr})\n"
)
skipped_jobs.append(job)
self._skipped_count += 1
continue
ctx.seen_drvs[job.drvPath] = job.attr


schedule_result = self.schedule_success(build_props, job)
if schedule_result is not None:
ctx.scheduler_log.addStdout(f"\t- {job.attr}\n")
Expand Down Expand Up @@ -573,7 +591,15 @@ async def _wait_and_process_completed(
fireOnOneCallback=True,
fireOnOneErrback=True,
)
results, index = await self.wait_for_finish_deferred # type: ignore[assignment]
try:
results, index = await self.wait_for_finish_deferred # type: ignore[assignment]
except defer.FirstError as e:
# DeferredList wraps the actual error in FirstError
if isinstance(e.subFailure.value, defer.CancelledError):
ctx.scheduler_log.addStdout("Build was cancelled\n")
return CANCELLED
# Re-raise other errors
raise

# Get the completed job
scheduled_job = ctx.scheduled.pop(index)
Expand Down Expand Up @@ -715,6 +741,7 @@ async def run(self) -> int:
scheduled=scheduled,
schedule_now=[],
scheduler_log=scheduler_log,
seen_drvs={},
)

# Main scheduling loop
Expand Down
Loading