Skip to content

Fix unbound variable in error case in build_and_install_software#4843

Merged
smoors merged 1 commit intoeasybuilders:developfrom
Flamefire:fix-unbound-var
Apr 22, 2025
Merged

Fix unbound variable in error case in build_and_install_software#4843
smoors merged 1 commit intoeasybuilders:developfrom
Flamefire:fix-unbound-var

Conversation

@Flamefire
Copy link
Copy Markdown
Contributor

@Flamefire Flamefire commented Apr 4, 2025

@dagonzalezfo Can you please double check this is what you intended in https://github.com/easybuilders/easybuild-framework/pull/4534/files#diff-7ee72a681a6748afac9e6e8e3ff33fd2b76c661eff64435388ec03895cf8a767R177

The bug results in this crash:

== cleaning up...
EasyBuild crashed! Please consider reporting a bug, this should not happen...

Traceback (most recent call last):
  File "/usr/lib64/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib64/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/Easybuild-5.0.0/lib/python3.8/site-packages/easybuild/main.py", line 804, in <module>
    main_with_hooks()
  File "/Easybuild-5.0.0/lib/python3.8/site-packages/easybuild/main.py", line 790, in main_with_hooks
    main(args=args, prepared_cfg_data=(init_session_state, eb_go, cfg_settings))
  File "/Easybuild-5.0.0/lib/python3.8/site-packages/easybuild/main.py", line 746, in main
    hooks, do_build)
  File "/Easybuild-5.0.0/lib/python3.8/site-packages/easybuild/main.py", line 570, in process_eb_args
    exit_on_failure=exit_on_failure)
  File "/Easybuild-5.0.0/lib/python3.8/site-packages/easybuild/main.py", line 178, in build_and_install_software
    raise EasyBuildError(test_msg, exit_code=err_code)
UnboundLocalError: local variable 'err_code' referenced before assignment

We have 3 cases:

  1. build_and_install_one finishes but failed -> 'err' is an EasyBuildError with err_code set
  2. build_and_install_one failed with an EasyBuildError -> 'err' is set to an EasyBuildError, err_code is not
  3. build_and_install_one failed with another exception -> 'err' is set to that, err_code is not

How I understood the intended logic:

  • When we have an EasyBuildError exit with the code of that but use test_msg as the message. I.e. just rewrite the EasyBuildError message
  • Otherwise just throw that that unknown/unexpected exception

Maybe we should rather:

  • If 'err' is an EasyBuildError, we replace error.msg = test_msg (not create a new instance which would be logged)
  • Otherwise create a new EasyBuildError with test_msg and the default exit code

@boegel boegel changed the title Fix unbound variable in error case Fix unbound variable in error case in build_and_install_software Apr 9, 2025
@boegel boegel added the bug fix label Apr 9, 2025
@boegel boegel added this to the release after 5.0.0 milestone Apr 9, 2025
smoors
smoors approved these changes Apr 22, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@smoors smoors left a comment

Choose a reason for hiding this comment

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

i was hit by the same bug, and this PR fixed it
lgtm

@smoors smoors merged commit 3a82615 into easybuilders:develop Apr 22, 2025
37 checks passed
@Flamefire Flamefire deleted the fix-unbound-var branch April 22, 2025 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smoors smoors smoors approved these changes

Assignees

No one assigned

Labels

bug fix

Projects

None yet

Milestone

5.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@Flamefire @smoors @boegel