Page MenuHomeFreeBSD

llvm-strings(1): Install as strings when WITH_LLVM_BINUTILS=YES
ClosedPublic

Authored by me_cameronkatri.com on Sep 23 2021, 12:25 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 7, 9:11 AM
Unknown Object (File)
Sat, Apr 6, 9:50 PM
Unknown Object (File)
Feb 3 2024, 3:22 PM
Unknown Object (File)
Feb 1 2024, 12:37 AM
Unknown Object (File)
Jan 23 2024, 10:06 AM
Unknown Object (File)
Dec 31 2023, 10:31 AM
Unknown Object (File)
Dec 20 2023, 6:17 AM
Unknown Object (File)
Dec 13 2023, 2:42 PM
Subscribers

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

me_cameronkatri.com created this revision.

Other than the minor issue I noted this seems reasonable to me. @emaste what do you think?

usr.bin/Makefile
260

This should probably be unconditional. @emaste pointed out that strings is sometimes used when people don't expect a full toolchain.

Install strings if despite WITHOUT_TOOLCHAIN

My argument for sticking with ELF Tool Chain strings is that we install it unconditionally and it gets used outside of traditional toolchain applications (e.g., it may be installed on more constrained embedded systems). In contrast to tools like nm, objcopy or even readelf where we need support for different formats (LLVM IR or different symbol tables, say) I think ELF Tool Chain strings is fully functional. Have you noticed anything missing from it, or is this just for consistency with the other tools?

On my laptop ELF Tool Chain strings is 16KB and depends on 103K libelf.so.2 while llvm-strings12 (from pkgs) is 30K and depends on 87MB libLLVM-12.so.

I updated this so it will install ELF Tool Chain strings, unless WITH_LLVM_BINUTILS is set, in which case llvm-strings will be installed as strings. It shouldn't change the current behavior of installing strings regardless of whether the toolchain was built (which I completely understand).

If WITHOUT_TOOLCHAIN is not set then llvm-strings will be built and installed anyways so I see no issue with installing it as strings too.

This revision is now accepted and ready to land.Apr 5 2023, 7:54 PM