This is an archive of the discontinued LLVM Phabricator instance.

Rename "llvm-gsym" and "llvm-gsymutil" to "gsymutil".
ClosedPublic

Authored by clayborg on Feb 27 2020, 12:36 PM.

Details

Summary

This patch renames the llvm-gsymutil tool to "gsymutil" along with all directories and build files. Also removed the unneeded deps from the "gsymutil" tool to allow gsymutil to link against less things.

Diff Detail

Event Timeline

clayborg created this revision.Feb 27 2020, 12:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 27 2020, 12:36 PM
Herald added a subscriber: mgorny. · View Herald Transcript
clayborg edited the summary of this revision. (Show Details)Feb 27 2020, 12:40 PM
clayborg added a subscriber: MaskRay.
aprantl accepted this revision.Feb 27 2020, 3:52 PM
aprantl added inline comments.
llvm/tools/llvm-gsym/llvm-gsymutil.cpp
434

This is not a rename?

This revision is now accepted and ready to land.Feb 27 2020, 3:52 PM
MaskRay accepted this revision.Feb 27 2020, 4:05 PM
MaskRay added inline comments.
llvm/tools/llvm-gsym/llvm-gsymutil.cpp
432

Can you recheck whether the remaining are all needed?

clayborg marked 2 inline comments as done.Feb 27 2020, 4:22 PM
clayborg added inline comments.
llvm/tools/llvm-gsym/llvm-gsymutil.cpp
432

If I removed all InitializeAllTargetInfos and InitializeAllTargetMCs I got errors when trying to load object files. I didn't try them individually though, so I will try that.

434

Yeah, I modified the description after submitting to say "Also removed the AsmPrinter from the dependencies.".

clayborg updated this revision to Diff 247129.Feb 27 2020, 4:31 PM

Removed more dependencies from the "gsymutil" tool and removed uneeded function calls.

clayborg marked an inline comment as done.Feb 27 2020, 4:32 PM
clayborg added inline comments.
llvm/tools/llvm-gsym/llvm-gsymutil.cpp
431

I was able to remove more.

clayborg edited the summary of this revision. (Show Details)Feb 27 2020, 4:33 PM

Most of the other tools are llvm- any reason for the change?

Most of the other tools are llvm- any reason for the change?

I was the one to suggest this name, since I found it inconsistent to have dsymutil but llvm-gsymutil. The others are mostly called llvm- to differentiate them from preexisting tools with the same name. I don't think it's necessary for a new tool; unless we want the prefix there it for marketing reasons?

clayborg updated this revision to Diff 247168.Feb 27 2020, 11:23 PM

Renamed back to llvm-gsymutil for everything. So this patch now is renaming from llvm-gsym to llvm-gsymutil for the tool directory.

clayborg updated this revision to Diff 247169.Feb 27 2020, 11:27 PM

Remove an extra fix for triples that I will commit with a new patch.

Is everyone ok with this naming?

MaskRay accepted this revision.Mar 2 2020, 9:13 PM
This revision was automatically updated to reflect the committed changes.

Hello @clayborg ,

this patch is breaking shared libs builds (cmake path/to/llvm -DBUILD_SHARED_LIBS=ON).

Could you please fix it?

Regards,

Francesco

Hello @clayborg ,

this patch is breaking shared libs builds (cmake path/to/llvm -DBUILD_SHARED_LIBS=ON).

Could you please fix it?

Regards,

Francesco

-DBUILD_SHARED_LIBS=ON build is good.

Hello @clayborg ,

this patch is breaking shared libs builds (cmake path/to/llvm -DBUILD_SHARED_LIBS=ON).

Could you please fix it?

Regards,

Francesco

-DBUILD_SHARED_LIBS=ON build is good.

Reverting this patch locally worked for me yesterday, so I assumed it was this patch causing the problem. When I pulled upstream today everything was working, so I must have done something wrong. In any case, thank you for double checking.

Francesco