Skip to content

redex.py: fix binary autodetection #982

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

Closed
GunniBusch wants to merge 1 commit into from
+20 −9
Closed

redex.py: fix binary autodetection #982

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 20 additions & 9 deletions redex.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,22 @@ def write_debugger_command(
return script_name


def find_redex_binary(redex_binary: typing.Optional[str]) -> typing.Optional[str]:
redex_binary = redex_binary or shutil.which("redex-all")
if redex_binary is not None:
return redex_binary

# __file__ can be /path/fb-redex.pex/redex.pyc
dir_name = dirname(abspath(__file__))
while not isdir(dir_name):
dir_name = dirname(dir_name)

bundled_binary = join(dir_name, "redex-all")
if isfile(bundled_binary):
Comment thread
GunniBusch marked this conversation as resolved.
return bundled_binary
return None


def add_extra_environment_args(env: typing.Dict[str, str]) -> None:
# If we haven't set MALLOC_CONF but we have requested to profile the memory
# of a specific pass, set some reasonable defaults
Expand Down Expand Up @@ -239,20 +255,14 @@ def run_redex_binary(
exception_formatter: ExceptionMessageFormatter,
output_line_handler: typing.Optional[typing.Callable[[str], str]],
) -> None:
state.args.redex_binary = find_redex_binary(state.args.redex_binary)
if state.args.redex_binary is None:
state.args.redex_binary = shutil.which("redex-all")

if state.args.redex_binary is None:
# __file__ can be /path/fb-redex.pex/redex.pyc
dir_name = dirname(abspath(__file__))
while not isdir(dir_name):
dir_name = dirname(dir_name)
state.args.redex_binary = join(dir_name, "redex-all")
sys.exit("redex-all was not found")
if not isfile(state.args.redex_binary) or not os.access(
state.args.redex_binary, os.X_OK
):
sys.exit(
"redex-all is not found or is not executable: " + state.args.redex_binary
"redex-all was not found or is not executable: " + state.args.redex_binary
)
LOGGER.debug("Running redex binary at %s", state.args.redex_binary)

Expand Down Expand Up @@ -607,6 +617,7 @@ def validate_args(args: argparse.Namespace) -> None:
extract_signing_args(args)

if not args.unpack_only:
args.redex_binary = find_redex_binary(args.redex_binary)

@xuhdev xuhdev Apr 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's the purpose of this line?

@GunniBusch GunniBusch Apr 17, 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.

To find the redex-all file in the same directory if not given via flag. It would fail before reaching run-binary

@xuhdev xuhdev Apr 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This specifically requires --redex-binary argument to be passed, see the line right below:

raise argparse.ArgumentTypeError("Requires --redex-binary argument")

@GunniBusch GunniBusch Apr 17, 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.

Yup, and if the new function doesnt find the binary, it will set it to null. This means, it will got to the exception raising. But if it found one, it sets the arg value and bascaily dies the sea was setting the flag to a value.

why? because in your docs, you say somewhere that you need to either set the flag or have it in the same directory.

This is already the case, but because the check for that previously happens after validating the flags.

So currently (ie in the current main branch) you check for the binary in the same directory already here

redex/redex.py

Lines 237 to 244 in b342673

def run_redex_binary(
state: State,
exception_formatter: ExceptionMessageFormatter,
output_line_handler: typing.Optional[typing.Callable[[str], str]],
) -> None:
if state.args.redex_binary is None:
state.args.redex_binary = shutil.which("redex-all")

But this (run_redex) is never called if args.redex_binary is None , because raise argparse.ArgumentTypeError("Requires --redex-binary argument") is thrown before. (Because validate_args is run before run_redex, which raises that exception if state.args.redex_binary is None )

redex/redex.py

Lines 1815 to 1816 in b342673

validate_args(args)
with_temp_cleanup(lambda: run_redex(args), args.always_clean_up)

So what I did, I basically removed the check logic from run_redex, put that into find_redex_binary, and set args.redex_binary to the return value of said function, inside validate_args. I also did this in run_redex to not mix refactor with fix in this pr

@xuhdev xuhdev Apr 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where did you see "you say somewhere that you need to either set the flag or have it in the same directory."? Is this from the Windows context in the installation page?

Bundling the redex-all binary with the python scripts is not supported on Windows. Manually copy the binary into the same directory as redex.py and use redex.py that way, or ensure that redex.py is called with the --redex-binary parameter:

Because this is a behavior change, I just wanna make sure if you see it somewhere in doc, the doc is kept in sync.

@GunniBusch GunniBusch Apr 18, 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.

yes, from the windows steps.

if not args.redex_binary:
raise argparse.ArgumentTypeError("Requires --redex-binary argument")
if not args.config:
Expand Down
Loading