[PATCH] D66613: [support][llvm-objcopy] Add support for shell wildcards

Jordan Rupprecht via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 23 07:43:30 PDT 2019


rupprecht added a comment.

(Will get to the code comments later, just wanted to justify the patch)

In D66613#1642461 <https://reviews.llvm.org/D66613#1642461>, @jhenderson wrote:

> I've not fully reviewed this yet, and won't get around to it until next week, but I wanted to highlight your own comment from D57517 <https://reviews.llvm.org/D57517> where you said the following:
>
> > Also a question, for others as well: is this a feature we actually want to carry forward? Are there any users of this feature/is it useful? This isn't something I've come across as actually being used, though I'm happy to review it if is.
>
> We added --regex instead of --wildcard precisely to address the specific use-case @evgeny777 had. I'm not opposed to adding --wildcard, if there are actual users of it who don't want to try to migrate to --regex


IMO --regex is nicer for interactive uses, but when using objcopy in a build script that may be used by different toolchains, with either GNU objcopy or llvm-objcopy, a common denominator is needed. Apparently clang+kernel people need it for some versions of the kernel, and we've had some internal users need this too. (For now, they use llvm-objcopy w/ --regex, but it would be a problem if someone using GNU objcopy in their toolchain needs to depend on their package). I also think it will generally help with the long tail of users that want to try it out.

> but I do wonder about the additional complexity this is introducing, and how likely it is to get the rules wrong, e.g. to do with escaping etc.

My concern as well. Please review diligently, I won't take comments about regex bugs personally :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66613/new/

https://reviews.llvm.org/D66613





More information about the llvm-commits mailing list