-
Notifications
You must be signed in to change notification settings - Fork 10.6k
Rework lit.cfg regex to avoid basename subshell in substitution #84583
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
Rework lit.cfg regex to avoid basename subshell in substitution #84583
Conversation
Changed %target-build-swift-dylib\(([^)]+)\) to %target-build-swift-dylib\(([^)]+?)([^/()]+)\) Group 1 now captures the "prefix" and group 2 captures the final word Captured the basename directly in the regex instead of using a subshell since Lit's internal shell does not support
@swift-ci please smoke test |
test/lit.cfg
Outdated
config.substitutions.append(('%target-run', config.target_run)) | ||
config.substitutions.append(('%target-jit-run', subst_target_jit_run)) | ||
config.substitutions.append(('%target-build-swift-dylib\(([^)]+)\)', config.target_build_swift_dylib)) | ||
config.substitutions.append(('%target-build-swift-dylib\(([^)]+?)([^/()]+)\)', config.target_build_swift_dylib)) |
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.
Could you add a comment describing that the file name is captured by \2
and the rest of the path by \1
? Regexes are notoriously hard to read.
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.
Oh and also you'll likely need to adjust this to handle Windows paths as well. But if you want you can wait for the Windows test results to arrive to know for sure.
Do I need to update all the other |
Ah yup, those also need to be updated |
0f72ce1
to
3a21c45
Compare
@swift-ci please smoke test |
🙌 It passed! |
Changed %target-build-swift-dylib(([^)]+)) to %target-build-swift-dylib(([^)]+?)([^/()]+))
Group 1 now captures the pat and group 2 captures the file name
Captured the basename directly in the regex instead of using a subshell since Lit's internal shell does not support
Partially address #84407