Re: [PATCH] xen/build: Fix `make cscope` rune

[Andrew Cooper <amc96@xxxxxxxx>]

On 16/12/2021 14:00, Jan Beulich wrote:
> On 16.12.2021 10:20, Andrew Cooper wrote:
>> Second, and this way for a long time:
>>
>>   $ make cscope
>>   ( find arch/x86/include -name '*.h' -print; find include -name '*.h' 
>> -print;
>>   find xsm arch/x86 common drivers lib test -name '*.[chS]' -print ) >
>>   cscope.files
>>   cscope -k -b -q
>>   cscope: cannot find file arch/x86/efi/efi.h
>>   cscope: cannot find file arch/x86/efi/ebmalloc.c
>>   cscope: cannot find file arch/x86/efi/compat.c
>>   cscope: cannot find file arch/x86/efi/pe.c
>>   cscope: cannot find file arch/x86/efi/boot.c
>>   cscope: cannot find file arch/x86/efi/runtime.c
>>
>> This is caused by these being symlinks to common/efi.  Restrict all find 
>> runes
>> to `-type f` to skip symlinks, because common/efi/*.c are already listed.
> I have reservations here, albeit of theoretical nature as long as only
> the csope target is affected (simply because I don't use it): Make
> rules should really be independent of a dir entry being a real file or
> a symlink. I did run into problems with that already years ago when
> the shim was introduced. My arrangements heavily use symlinking, and
> any assumption on files being "real" ones will break this. At the very
> least symlink checks should be restricted to cover only relative ones;
> ideally one would distinguish ones staying within the tree vs ones
> reaching to the "outside".

all_sources is used exclusively for "tags" purposes; the
TAGS/tags/gtags/cscope targets.

Personally, I'd prefer there to not be symlinks in the first place.  The
EFI logic is unnecessarily complicated to navigate.

But the reality is that inter-xen/ symlinks for source files are also a
duplication as far as these `find` runes are concerned.

Apparently tags et al will follow symlinks, while there's no obviously
help online about cscope, other than "resolve your symlinks first".

In either case, you don't want to end up with both the regular path, and
the symlink, ending up in the file list.


I don't anticipate the usecase for all_source changing, nor the way we
symlink things, so I think sticking with `-type f` is the appropriate
action.

Furthermore, you really don't want a directory (e.g. include/foo.d )
getting into the file list either.

>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -468,9 +468,8 @@ arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: 
>> asm-offsets.s
>>  
>>  SUBDIRS = xsm arch/$(TARGET_ARCH) common drivers lib test
>>  define all_sources
>> -    ( find arch/$(TARGET_ARCH)/include -name '*.h' -print; \
>> -      find include -name '*.h' -print; \
>> -      find $(SUBDIRS) -name '*.[chS]' -print )
>> +    ( find include -type f -name '*.h' -print; \
>> +      find $(SUBDIRS) -type f -name '*.[chS]' -print )
>>  endef
> I further wonder how use of $(TARGET_ARCH) can be correct here. Why
> would the enumeration of items here be limited to a particular arch?
> When you edit files in the source tree, everything should be covered.
> Restriction to a particular arch only makes sense in a build tree.

I'd say that's a matter of opinion.  There are times when I'd quite like
it to be all arch's, and there are other times when I'm very glad it's not.

Either way, I don't intend to address that rats nest in this patch.

~Andrew