-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Support target names containing dots in all utilities #65812
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: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,11 +5,14 @@ | |
# RUN: mkdir %t | ||
# RUN: ln -s llvm-ranlib %t/llvm-ranlib-9 | ||
# RUN: ln -s llvm-ranlib %t/ranlib.exe | ||
# RUN: ln -s llvm-ranlib %t/x86_64-unknown-freebsd13.2-llvm-ranlib | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm struggling to understand the logic in the llvm-ar test case gaining two new test cases, but the ranlib one only gaining one. (On further investigation, I'm struggling to understand why the llvm-ranlib tool-name test even needs to exist, given that the code is the same code as llvm-ar at the relevant point). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, your second point is why I didn't bother making the second case for llvm-ranlib. On my first pass I just updated the existing testcases I found, but for ranlib I realised the same thing you did. I may just drop the ranlib test entirely. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's put the new test files and deletion of this old test in a different PR. The old code was untested, so we're not making things worse, but it also helps keep the PRs focused. Aside: if we're deleting this old file, I think it would be a good idea to add one or two cases to the llvm-ar test showing the "llvm-ranlib" name. |
||
|
||
# RUN: llvm-ranlib -h | FileCheck %s --check-prefix=DEFAULT | ||
# RUN: %t/llvm-ranlib-9 -h | FileCheck %s --check-prefix=VERSION | ||
# RUN: %t/ranlib.exe -h | FileCheck %s --check-prefix=SUFFIX | ||
# RUN: %t/x86_64-unknown-freebsd13.2-llvm-ranlib -h | FileCheck %s --check-prefix=TRIPLE | ||
|
||
# DEFAULT: USAGE: llvm-ranlib{{ }} | ||
# VERSION: USAGE: llvm-ranlib-9{{ }} | ||
# SUFFIX: USAGE: ranlib{{ }} | ||
# TRIPLE: USAGE: x86_64-unknown-freebsd13.2-llvm-ranlib{{ }} |
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.
Hm. I'm happy with this file, but I also discovered a bunch of other
tool-name
tests that can be updated. I'll add those too.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.
Okay, added tool-name tests for the utilities for ELF and such. Not sure if I should add similar tests for Windows targets; they never have dots in their names.