Skip to content

fix: move canary from cwd instead of build#3985

Open
andreoam wants to merge 2 commits into
opentibiabr:mainfrom
andreoam:fix-canary-executable-in-recompile.sh
Open

fix: move canary from cwd instead of build#3985
andreoam wants to merge 2 commits into
opentibiabr:mainfrom
andreoam:fix-canary-executable-in-recompile.sh

Conversation

@andreoam

@andreoam andreoam commented May 28, 2026

Copy link
Copy Markdown
Contributor

Description

eng:
Create a backup of the executable before recompiling using recompile.sh.
The current script is looking for the canary executable in the build folder. But in reality, it should be looking in the root folder.

pt-br:
O script estava procurando o executavel do canary na pasta build, mas na realidade o executavel esta na pasta raiz, entao o backup nunca era feito.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested

Test Configuration:

  • Server Version: Main
  • Client: 15.11
  • Operating System: Ubuntu

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I checked the PR checks reports
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

Summary by CodeRabbit

  • Chores
    • Updated build script to operate on the top-level build artifact instead of a subdirectory, improving artifact handling.
    • Script now logs the previous artifact location and creates a local ".old" backup of the previous executable to preserve prior builds.

Review Change Stack

Change move_executable to look for the canary executable in the current working directory and rename it to canary.old there. Previously the script checked and moved build/canary; the checks and mv command were updated to use $executable_name (cwd) instead of build/ to match where the artifact is produced.
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 840f8c83-2251-4dba-86e3-91ab480f9a85

📥 Commits

Reviewing files that changed from the base of the PR and between a4a8733 and ba63bcb.

📒 Files selected for processing (1)
  • recompile.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • recompile.sh

📝 Walkthrough

Walkthrough

The recompile.sh script updates move_executable to check for a top-level ./canary and rename it to ./canary.old (using mv -f) instead of operating on build/canary and build/canary.old.

Changes

Build executable path convention

Layer / File(s) Summary
Update executable path convention
recompile.sh
move_executable now locates ./canary and backs it up as ./canary.old, and uses mv -f for the rename instead of operating on build/canary.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A little script in the meadow hops,
It finds the canary where the worktree stops,
Renames with care to canary.old,
No more digging in build-folds untold,
A tidy hop, brisk and bold.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: moving the canary executable lookup from the build directory to the current working directory (cwd).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the move_executable function in recompile.sh to look for and move the canary executable in the current directory instead of the build/ directory. The review feedback suggests improving this change by using -f instead of -e to ensure the target is a regular file, and adding the -f flag to the mv command to prevent interactive prompts in non-interactive environments.

Comment thread recompile.sh Outdated
@andreoam andreoam changed the title Move canary from cwd instead of build/ fix: move canary from cwd instead of build May 28, 2026
@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

This PR is stale because it has been open 45 days with no activity.

@github-actions github-actions Bot added the Stale No activity label Jun 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Stale No activity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant