[LTP] [PATCH v1] Move cpuset_lib code to libs/ directory

[Petr Vorel]

Hi Wei,


First, this does not work [1].

1) LTP would depend on fts.h on musl:
Musl has missing fts.h dependency for HAVE_FTS_H:
2024-11-05T10:52:38.2449988Z CC libs/cpuset_lib/cpuinfo.o
2024-11-05T10:52:38.2681767Z libcpuset.c:32:10: fatal error: fts.h: No such file or directory

libcpuset is compiled on Musl only when fts.h is installed (not part of official
musl):
ifneq ($(HAVE_FTS_H),1)
	FILTER_OUT_DIRS += cpuset
endif
The new library would need to be compiled conditionally and
e.g. libcpuset being mandatory will break musl without

And anything which depends on this will need to have the same check (e.g. the
original cpuset02 patch [3]).

2) There are still some problems with the patchset, e.g. on Fedora, old Leap 42.2 and Ubuntu bionic

2024-11-05T10:53:18.7290696Z /usr/bin/ld: ../../lib/libltp.a: error adding symbols: file format not recognized
2024-11-05T10:53:18.7295116Z clang: error: linker command failed with exit code 1 (use -v to see invocation)

I haven't found what causes it.

3) Debian when cross compiling seqfaults
2024-11-05T10:54:35.2460053Z collect2: fatal error: ld terminated with signal 11 [Segmentation fault], core dumped
2024-11-05T10:54:35.2461039Z compilation terminated.
2024-11-05T10:54:35.2461847Z collect2: fatal error: ld terminated with signal 11 [Segmentation fault], core dumped

I suppose the same root cause as 2).

Some general tips for the patches:

1) Always push to your fork
It would save reviewers time if you push to your fork before sending a patch.
That would save me to do that and reviewers to look at the broken patch.

2) Describe the reason of the change in the commit message ("what" vs. "why")
I.e. *why* the change is needed is sometimes more important than to describe
what was *changed* [2].

3) Put common things together
I'm not sure if I move code to libs/ on it's own (without the patch which that
requires it [3]). And even if the change itself makes sense (maybe apply here as
a cleanup), it would be nice to post here the motivation (e.g. link of the
original patch). That's actually 2 (*why*). This people looking at your patches
to get the context). Sometimes it's worth to have it in the commit message,
here would be enough just to put link [3] below --- (note which will not be part
of the commit message).

4) very nit: it's nice to respect the credit (for new ideas) :)
Suggested-by: Petr Vorel <pvorel@suse.cz>

> ---
>  .../cpuset/cpuset_lib => include}/cpuset.h    |  0
>  .../bitmask.h => include/cpuset_bitmask.h     |  0
>  .../common.h => include/cpuset_common.h       |  0
>  .../cpuinfo.h => include/cpuset_cpuinfo.h     |  0
>  .../meminfo.h => include/cpuset_meminfo.h     |  0

You added 'cpuset_' prefix for headers. Good, but IMHO it should be 'libcpuset_' prefix.

We usually have 'tst_' or 'safe_' or 'lib' prefix for new code in include/*.
When I filter also config.h and vendored ujson library (which we obviously did
not want to change just to add the 'lib' prefix) there is not much left:

$ ls include/*.h |grep -v -e /tst_ -e /safe_ -e /lib -e /ujson -e /config.h
include/ipcmsg.h
include/ipcsem.h
include/parse_vdso.h
include/time64_variants.h

I wonder if we need whole cpuset_lib or just part. Moving just what is needed
into libs/ might have also solve fts.h dependency on musl.

Kind regards,
Petr

[1] https://github.com/pevik/ltp/actions/runs/11682645742
[2] https://cbea.ms/git-commit/
[3] https://patchwork.ozlabs.org/project/ltp/patch/20240930135809.9300-1-wegao@suse.com/