-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[smart] rename the Group name to 'lwProcess' and optimize the error h… #10436
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.
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
base: master
Are you sure you want to change the base?
Conversation
…andling for vDSO building.
There was a problem hiding this comment.
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 standardizes the SConscript group name to lwProcess
, refactors error handling for the vDSO build step, and removes two legacy architecture-specific scripts.
- Refactor vDSO build: consolidate clean logic into one command, wrap
subprocess.run
in atry/except
, and report failures. - Rename all
DefineGroup
calls fromvDSO
/lwP
tolwProcess
. - Delete
components/lwp/arch/x86/i386/SConscript
andcomponents/lwp/arch/risc-v/rv64/SConscript
.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
components/lwp/vdso/SConscript | Refactored clean command logic, added try/except , renamed group. |
components/lwp/arch/x86/i386/SConscript | Removed legacy i386 SConscript file. |
components/lwp/arch/risc-v/rv64/SConscript | Removed legacy rv64 SConscript file. |
components/lwp/SConscript | Renamed top-level group from lwP to lwProcess . |
Comments suppressed due to low confidence (3)
components/lwp/vdso/SConscript:54
- [nitpick] Renaming the vDSO group to
lwProcess
may conflict with the main LWP group and obscure the vDSO target. Consider using a distinct name likelwProcess-vdso
for clarity.
group = DefineGroup('lwProcess', src, depend = ['RT_USING_SMART','RT_USING_VDSO'], CPPPATH = CPPPATH)
components/lwp/vdso/SConscript
Outdated
# generic error handle | ||
except : | ||
print('exec command: "%s" failed.' % ' '.join(command)) | ||
exit(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using a bare except:
which also catches system-exiting exceptions. Catch specific errors (e.g., except subprocess.CalledProcessError as e:
) to prevent unintended behavior.
# generic error handle | |
except : | |
print('exec command: "%s" failed.' % ' '.join(command)) | |
exit(1) | |
except subprocess.CalledProcessError as e: | |
print(f'Command "{e.cmd}" failed with return code {e.returncode}.') | |
exit(1) | |
except FileNotFoundError as e: | |
print(f'Command not found: {e}. Please ensure "scons" is installed and in your PATH.') | |
exit(1) |
Copilot uses AI. Check for mistakes.
components/lwp/vdso/SConscript
Outdated
if result.returncode == 0: | ||
print("Command executed successfully") | ||
else: | ||
print("Command failed with exit code:", result.returncode) | ||
exit(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using check=True
, subprocess.run
will raise on non-zero exits, so this manual returncode check is redundant. Consider removing it or use check=False
if you need to handle exit codes explicitly.
if result.returncode == 0: | |
print("Command executed successfully") | |
else: | |
print("Command failed with exit code:", result.returncode) | |
exit(1) | |
print("Command executed successfully") |
Copilot uses AI. Check for mistakes.
…andling for vDSO building.
拉取/合并请求描述:(PR description)
[
为什么提交这份PR (why to submit this PR)
smart的Group Name稍显凌乱,统一调整成lwProcess,以反映“轻量级进程”
你的解决方案是什么 (what is your solution)
对SConscript脚本进行调整。
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality:
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0
代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up