-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[scons] code cleanup for scons script. #10429
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
Conversation
…nction in hello.c and hello.h
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 refactors the SCons scripts and target generators for RT-Thread by cleaning up build tool scripts, updating copyright dates, and reorganizing module dependencies. Key changes include removing the legacy implementations in tools/xmake.lua and win32spawn.py, refactoring the SCons preprocessor patch into an independent class, and updating import paths in many target generator scripts.
Reviewed Changes
Copilot reviewed 29 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/xmake.lua | Entire file removed to clean up unused build script functionality. |
| tools/win32spawn.py | Replaces the old thread‐based spawn approach with a subprocess call. |
| tools/targets/* | Updates to import paths and minor refactoring in target scripts. |
| tools/package.py | Adds robustness in handling package.json and minor improvements. |
| tools/building.py | Refactors SCons preprocessor patch integration using the new helper. |
| print('Exception: ' + os.strerror(e.errno)) | ||
| if (os.strerror(e.errno) == "No such file or directory"): | ||
| print ("\nPlease check Toolchains PATH setting.\n") |
Copilot
AI
Jun 23, 2025
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 catching exceptions from subprocess.Popen, e may not always have an 'errno' attribute. It is safer to use str(e) or check for the attribute before calling os.strerror(e.errno).
| print('Exception: ' + os.strerror(e.errno)) | |
| if (os.strerror(e.errno) == "No such file or directory"): | |
| print ("\nPlease check Toolchains PATH setting.\n") | |
| if hasattr(e, 'errno'): | |
| print('Exception: ' + os.strerror(e.errno)) | |
| if os.strerror(e.errno) == "No such file or directory": | |
| print("\nPlease check Toolchains PATH setting.\n") | |
| else: | |
| print('Exception: ' + str(e)) |
| for item in group['LIBPATH']: | ||
| full_path = os.path.join(item, group['name'] + '.lib') | ||
| if os.path.isfile(full_path): # has this library | ||
| if group_tree != None: | ||
| MDK4AddLibToGroup(ProjectFiles, group_tree, group['name'], lib_path, project_path) | ||
| MDK4AddLibToGroup(ProjectFiles, group_tree, group['name'], full_path, project_path) | ||
| else: | ||
| group_tree = MDK4AddGroupForFN(ProjectFiles, groups, group['name'], lib_path, project_path) | ||
| group_tree = MDK4AddGroupForFN(ProjectFiles, groups, group['name'], full_path, project_path) |
Copilot
AI
Jun 23, 2025
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.
The change in the library lookup loop now iterates over 'LIBPATH' and constructs the library name using the group's name. Verify that this change reflects the intended logic, as previously the iteration was over 'LIBS'.
拉取/合并请求描述:(PR description)
[
为什么提交这份PR (why to submit this PR)
tools下的构建脚本显得比较乱,需要进行整理调整;
你的解决方案是什么 (what is your solution)
使用AI工具根据需求情况,结构划分情况进行重构。
请提供验证的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