Page MenuHomePhabricator

[objcopy][NFC] Add rules to cmake to put files under specific folders.
AcceptedPublic

Authored by avl on Nov 26 2021, 11:57 PM.

Details

Summary
This patch adds rules to cmake to put files under specific folders.

It allows to have files for different formats(which are located in different
subdirectories) be displayed in different subfolders of VS IDE solution.

Depends on D114429

Diff Detail

Event Timeline

avl created this revision.Nov 26 2021, 11:57 PM
avl requested review of this revision.Nov 26 2021, 11:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 26 2021, 11:57 PM
jhenderson added inline comments.Nov 29 2021, 12:34 AM
llvm/lib/Object/CMakeLists.txt
2

Nit: delete this blank line.

3

The backslashes are surprising to me. Can this be forward slashes, like a cross-platform file path?

4

I think you want to use the REGULAR_EXPRESSION rather than FILES, so that you don't need to explicitly specify each source file in the list.

This should allow you to avoid the need for the macros below too.

93–96

Do you need these explicitly?

avl added inline comments.Nov 29 2021, 3:10 AM
llvm/lib/Object/CMakeLists.txt
3

If forward slashes are used then Visual Studio shows "ObjCopy" not as subfolder of "Header Files" but as another folder on the same level as "Header Files".

avl updated this revision to Diff 390454.Nov 29 2021, 1:46 PM

addressed comments.

avl added inline comments.Nov 29 2021, 1:49 PM
llvm/lib/Object/CMakeLists.txt
93–96

yes, we need these. If we do not specify include paths here then corresponding .h files would not be shown in the "Header Files" folder.

jhenderson accepted this revision.Nov 30 2021, 12:16 AM

LGTM.

llvm/lib/Object/CMakeLists.txt
3

That's bizarre, but okay.

93–96

Interesting. That suggests the add_llvm_component_library function doesn't recurse down the tree, which it intuitively should (possibly auto-adding the source groups along the way). Anyway, not an issue with this patch.

This revision is now accepted and ready to land.Nov 30 2021, 12:16 AM