[no subject]

From: Jialu Xu
Date: Mon Dec 04 2023 - 21:16:37 EST


>On Mon, Dec 04, 2023 at 06:41:42PM +0800, Jialu Xu wrote:
>> When symbolic links are involved in the path, os.path.abspath might not
>> resolve the symlinks and instead return the absolute path with the
>> symlinks intact.
>
>Can you provide an example or more detailed description of how this
>behavior is currently broken? I can't really understand how having
>symlinks in the path after normalization would break anything but I am
>potentially missing something :)

Glad to show my situation:

Say "drivers/hdf/" has some symlinks:

# ls -l drivers/hdf/
total 364
drwxrwxr-x 2 ... 4096 ... evdev
lrwxrwxrwx 1 ... 44 ... framework -> ../../../../../../drivers/hdf_core/framework
-rw-rw-r-- 1 ... 359010 ... hdf_macro_test.h
lrwxrwxrwx 1 ... 55 ... inner_api -> ../../../../../../drivers/hdf_core/interfaces/inner_api
lrwxrwxrwx 1 ... 53 ... khdf -> ../../../../../../drivers/hdf_core/adapter/khdf/linux
-rw-r--r-- 1 ... 74 ... Makefile
drwxrwxr-x 3 ... 4096 ... wifi

One .cmd file records that:

# head -1 ./framework/core/manager/src/.devmgr_service.o.cmd
cmd_drivers/hdf/khdf/manager/../../../../framework/core/manager/src/devmgr_service.o := ... \
/path/to/out/drivers/hdf/khdf/manager/../../../../framework/core/manager/src/devmgr_service.c

os.path.abspath returns "/path/to/out/framework/core/manager/src/devmgr_service.c", not correct:

# ./scripts/clang-tools/gen_compile_commands.py
INFO: Could not add line from ./framework/core/manager/src/.devmgr_service.o.cmd: File \
/path/to/out/framework/core/manager/src/devmgr_service.c not found

Use pathlib.Path resolve() instead, got the correct path

# cat compile_commands.json
...
{
"command": ...
"directory": ...
"file": "/path/to/blabla/drivers/hdf_core/framework/core/manager/src/devmgr_service.c"
},
...

>> - # Use os.path.abspath() to normalize the path resolving '.' and '..' .
>> - abs_path = os.path.abspath(os.path.join(root_directory, file_path))
>> + # Make the path absolute, resolving all symlinks on the way and also normalizing it.
>> + abs_path = str(Path(os.path.join(root_directory, file_path)).resolve())
>
>I think this can be more simply:
>
> abs_path = str(Path(root_directory, file_path).resolve())
>
>I think there should be a comment around why we are creating a Path
>object then creating a string from it, rather than using the Path object
>directly, namely that PosixPath is not JSON serializable.

Nice, update as yours, thanks.

>> if not os.path.exists(abs_path):
>> raise ValueError('File %s not found' % abs_path)
>> return {
>> --
>> 2.39.2
>>