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.
use pathlib.Path resolve() instead of os.path.abspath()
Signed-off-by: Jialu Xu <[email protected]>
---
scripts/clang-tools/gen_compile_commands.py | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
index 180952fb91c1b..0a6c0996b4a8f 100755
--- a/scripts/clang-tools/gen_compile_commands.py
+++ b/scripts/clang-tools/gen_compile_commands.py
@@ -11,6 +11,7 @@ import argparse
import json
import logging
import os
+from pathlib import Path
import re
import subprocess
import sys
@@ -172,8 +173,8 @@ def process_line(root_directory, command_prefix, file_path):
# by Make, so this code replaces the escaped version with '#'.
prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#')
- # 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())
if not os.path.exists(abs_path):
raise ValueError('File %s not found' % abs_path)
return {
--
2.39.2
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 :)
> use pathlib.Path resolve() instead of os.path.abspath()
>
> Signed-off-by: Jialu Xu <[email protected]>
> ---
> scripts/clang-tools/gen_compile_commands.py | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
> index 180952fb91c1b..0a6c0996b4a8f 100755
> --- a/scripts/clang-tools/gen_compile_commands.py
> +++ b/scripts/clang-tools/gen_compile_commands.py
> @@ -11,6 +11,7 @@ import argparse
> import json
> import logging
> import os
> +from pathlib import Path
> import re
> import subprocess
> import sys
> @@ -172,8 +173,8 @@ def process_line(root_directory, command_prefix, file_path):
> # by Make, so this code replaces the escaped version with '#'.
> prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#')
>
> - # 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.
> if not os.path.exists(abs_path):
> raise ValueError('File %s not found' % abs_path)
> return {
> --
> 2.39.2
>
>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
>>
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.
Use pathlib.Path resolve() instead of os.path.abspath()
Signed-off-by: Jialu Xu <[email protected]>
---
scripts/clang-tools/gen_compile_commands.py | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
index 180952fb91c1b..99e28b7152c19 100755
--- a/scripts/clang-tools/gen_compile_commands.py
+++ b/scripts/clang-tools/gen_compile_commands.py
@@ -11,6 +11,7 @@ import argparse
import json
import logging
import os
+from pathlib import Path
import re
import subprocess
import sys
@@ -172,8 +173,9 @@ def process_line(root_directory, command_prefix, file_path):
# by Make, so this code replaces the escaped version with '#'.
prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#')
- # 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.
+ # Convert Path object to a string because 'PosixPath' is not JSON serializable.
+ abs_path = str(Path(root_directory, file_path).resolve())
if not os.path.exists(abs_path):
raise ValueError('File %s not found' % abs_path)
return {
--
2.39.2
Hi Jialu,
On Tue, Dec 05, 2023 at 10:15:26AM +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.
>
> Use pathlib.Path resolve() instead of os.path.abspath()
>
> Signed-off-by: Jialu Xu <[email protected]>
Thanks for the clarification in your previous message [1], I suppose
that makes sense as to why nobody has reported this to us because that
is a rather odd situation that the upstream kernel would not experience.
I think that some of those details should be in the commit message,
along with a short example like you provided, so that we know exactly
what the situation was and how this patch resolves it.
Perhaps something like (please feel free to correct or reword as you
feel necessary):
"When a path contains relative symbolic links, os.path.abspath() might
not follow the symlinks and instead return the absolute path with just
the relative paths resolved, resulting in an incorrect path.
<broken example>
Use pathlib.Path.resolve(), which resolves the symlinks and normalizes
the paths correctly.
<working example>"
The actual fix seems fine to me. Feel free to add
Reviewed-by: Nathan Chancellor <[email protected]>
to the subsequent submission and please include both
Masahiro Yamada <[email protected]>
[email protected]
on it in addition to the people you have here, as he is the one who
actually applies gen_compile_commands.py changes (I am going to send a
MAINTAINERS change for this).
[1]: https://lore.kernel.org/[email protected]/
Cheers,
Nathan
> ---
> scripts/clang-tools/gen_compile_commands.py | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
> index 180952fb91c1b..99e28b7152c19 100755
> --- a/scripts/clang-tools/gen_compile_commands.py
> +++ b/scripts/clang-tools/gen_compile_commands.py
> @@ -11,6 +11,7 @@ import argparse
> import json
> import logging
> import os
> +from pathlib import Path
> import re
> import subprocess
> import sys
> @@ -172,8 +173,9 @@ def process_line(root_directory, command_prefix, file_path):
> # by Make, so this code replaces the escaped version with '#'.
> prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#')
>
> - # 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.
> + # Convert Path object to a string because 'PosixPath' is not JSON serializable.
> + abs_path = str(Path(root_directory, file_path).resolve())
> if not os.path.exists(abs_path):
> raise ValueError('File %s not found' % abs_path)
> return {
> --
> 2.39.2
>
>
Hi Nathan,
>On Tue, Dec 05, 2023 at 10:15:26AM +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.
>>
>> Use pathlib.Path resolve() instead of os.path.abspath()
>>
>> Signed-off-by: Jialu Xu <[email protected]>
>
>Thanks for the clarification in your previous message [1], I suppose
>that makes sense as to why nobody has reported this to us because that
>is a rather odd situation that the upstream kernel would not experience.
>
>I think that some of those details should be in the commit message,
>along with a short example like you provided, so that we know exactly
>what the situation was and how this patch resolves it.
>
>Perhaps something like (please feel free to correct or reword as you
>feel necessary):
>
>"When a path contains relative symbolic links, os.path.abspath() might
>not follow the symlinks and instead return the absolute path with just
>the relative paths resolved, resulting in an incorrect path.
>
><broken example>
>
>Use pathlib.Path.resolve(), which resolves the symlinks and normalizes
>the paths correctly.
>
><working example>"
>
>The actual fix seems fine to me. Feel free to add
>
> Reviewed-by: Nathan Chancellor <[email protected]>
>
>to the subsequent submission and please include both
>
> Masahiro Yamada <[email protected]>
> [email protected]
>
>on it in addition to the people you have here, as he is the one who
>actually applies gen_compile_commands.py changes (I am going to send a
>MAINTAINERS change for this).
>
>[1]: https://lore.kernel.org/[email protected]/
>
Thanks for the very detailed help!
Patch update as v3.
Cheers,
Jialu
When a path contains relative symbolic links, os.path.abspath() might
not follow the symlinks and instead return the absolute path with just
the relative paths resolved, resulting in an incorrect path.
1. 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
2. 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
3. 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(), which resolves the symlinks and normalizes
the paths correctly.
# cat compile_commands.json
...
{
"command": ...
"directory": ...
"file": "/path/to/blabla/drivers/hdf_core/framework/core/manager/src/devmgr_service.c"
},
...
Reviewed-by: Nathan Chancellor <[email protected]>
---
scripts/clang-tools/gen_compile_commands.py | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
index 180952fb91c1b..99e28b7152c19 100755
--- a/scripts/clang-tools/gen_compile_commands.py
+++ b/scripts/clang-tools/gen_compile_commands.py
@@ -11,6 +11,7 @@ import argparse
import json
import logging
import os
+from pathlib import Path
import re
import subprocess
import sys
@@ -172,8 +173,9 @@ def process_line(root_directory, command_prefix, file_path):
# by Make, so this code replaces the escaped version with '#'.
prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#')
- # 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.
+ # Convert Path object to a string because 'PosixPath' is not JSON serializable.
+ abs_path = str(Path(root_directory, file_path).resolve())
if not os.path.exists(abs_path):
raise ValueError('File %s not found' % abs_path)
return {
--
2.39.2
When a path contains relative symbolic links, os.path.abspath() might
not follow the symlinks and instead return the absolute path with just
the relative paths resolved, resulting in an incorrect path.
1. 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
2. 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
3. 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(), which resolves the symlinks and normalizes
the paths correctly.
# cat compile_commands.json
...
{
"command": ...
"directory": ...
"file": "/path/to/blabla/drivers/hdf_core/framework/core/manager/src/devmgr_service.c"
},
...
Reviewed-by: Nathan Chancellor <[email protected]>
Signed-off-by: Jialu Xu <[email protected]>
---
scripts/clang-tools/gen_compile_commands.py | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
index 180952fb91c1b..99e28b7152c19 100755
--- a/scripts/clang-tools/gen_compile_commands.py
+++ b/scripts/clang-tools/gen_compile_commands.py
@@ -11,6 +11,7 @@ import argparse
import json
import logging
import os
+from pathlib import Path
import re
import subprocess
import sys
@@ -172,8 +173,9 @@ def process_line(root_directory, command_prefix, file_path):
# by Make, so this code replaces the escaped version with '#'.
prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#')
- # 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.
+ # Convert Path object to a string because 'PosixPath' is not JSON serializable.
+ abs_path = str(Path(root_directory, file_path).resolve())
if not os.path.exists(abs_path):
raise ValueError('File %s not found' % abs_path)
return {
--
2.39.2
Hi,
On Wed, Dec 06, 2023 at 09:24:42AM +0800, Jialu Xu wrote:
> When a path contains relative symbolic links, os.path.abspath() might
> not follow the symlinks and instead return the absolute path with just
> the relative paths resolved, resulting in an incorrect path.
>
> 1. 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
>
> 2. 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
>
> 3. 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(), which resolves the symlinks and normalizes
> the paths correctly.
>
> # cat compile_commands.json
> ...
> {
> "command": ...
> "directory": ...
> "file": "/path/to/blabla/drivers/hdf_core/framework/core/manager/src/devmgr_service.c"
> },
> ...
>
> Reviewed-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Jialu Xu <[email protected]>
> ---
This looks good to me. I prefer using pathlib over anything in os
module, even if the behavior was the same. In this case, the pathlib
behavior is better -- which is a bonus.
Reviewed-by: Justin Stitt <[email protected]>
> scripts/clang-tools/gen_compile_commands.py | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
> index 180952fb91c1b..99e28b7152c19 100755
> --- a/scripts/clang-tools/gen_compile_commands.py
> +++ b/scripts/clang-tools/gen_compile_commands.py
> @@ -11,6 +11,7 @@ import argparse
> import json
> import logging
> import os
> +from pathlib import Path
> import re
> import subprocess
> import sys
> @@ -172,8 +173,9 @@ def process_line(root_directory, command_prefix, file_path):
> # by Make, so this code replaces the escaped version with '#'.
> prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#')
>
> - # 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.
> + # Convert Path object to a string because 'PosixPath' is not JSON serializable.
> + abs_path = str(Path(root_directory, file_path).resolve())
> if not os.path.exists(abs_path):
> raise ValueError('File %s not found' % abs_path)
> return {
> --
> 2.39.2
>
On Wed, Dec 6, 2023 at 10:26 AM Jialu Xu <[email protected]> wrote:
>
> When a path contains relative symbolic links, os.path.abspath() might
> not follow the symlinks and instead return the absolute path with just
> the relative paths resolved, resulting in an incorrect path.
>
> 1. 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
>
> 2. 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
>
> 3. 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(), which resolves the symlinks and normalizes
> the paths correctly.
>
> # cat compile_commands.json
> ...
> {
> "command": ...
> "directory": ...
> "file": "/path/to/blabla/drivers/hdf_core/framework/core/manager/src/devmgr_service.c"
> },
> ...
>
> Reviewed-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Jialu Xu <[email protected]>
> ---
> scripts/clang-tools/gen_compile_commands.py | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
> index 180952fb91c1b..99e28b7152c19 100755
> --- a/scripts/clang-tools/gen_compile_commands.py
> +++ b/scripts/clang-tools/gen_compile_commands.py
> @@ -11,6 +11,7 @@ import argparse
> import json
> import logging
> import os
> +from pathlib import Path
> import re
> import subprocess
> import sys
> @@ -172,8 +173,9 @@ def process_line(root_directory, command_prefix, file_path):
> # by Make, so this code replaces the escaped version with '#'.
> prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#')
>
> - # 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.
> + # Convert Path object to a string because 'PosixPath' is not JSON serializable.
> + abs_path = str(Path(root_directory, file_path).resolve())
> if not os.path.exists(abs_path):
> raise ValueError('File %s not found' % abs_path)
> return {
Is there any reason why you didn't simply replace
os.path.abspath() with os.path.realpath() ?
This patch uses pathlib.Path() just in one place,
leaving many call-sites of os.path.*() functions.
If it is just a matter of your preference,
you need to convert os.path.*() for consistency
(as a follow-up patch).
I see one more os.path.abspath()
return (args.log_level,
os.path.abspath(args.directory),
args.output,
args.ar,
args.paths if len(args.paths) > 0 else [args.directory])
Does it cause a similar issue for the 'directory' field
with symbolic link jungles?
--
Best Regards
Masahiro Yamada
>Is there any reason why you didn't simply replace
>os.path.abspath() with os.path.realpath() ?
I have tried it before, but obviously, I made a mistake.
>This patch uses pathlib.Path() just in one place,
>leaving many call-sites of os.path.*() functions.
>
>If it is just a matter of your preference,
>you need to convert os.path.*() for consistency
>(as a follow-up patch).
Keep os.path.* as os.path.realpath() works.
>I see one more os.path.abspath()
>
> return (args.log_level,
> os.path.abspath(args.directory),
> args.output,
> args.ar,
> args.paths if len(args.paths) > 0 else [args.directory])
>
>Does it cause a similar issue for the 'directory' field
>with symbolic link jungles?
Yes, also fixed.
--
Best Regards
Jialu Xu
When a path contains relative symbolic links, os.path.abspath() might
not follow the symlinks and instead return the absolute path with just
the relative paths resolved, resulting in an incorrect path.
1. 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
2. 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/src/drivers/hdf/khdf/manager/../../../../framework/core/manager/src/devmgr_service.c
3. os.path.abspath returns "/path/to/src/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/src/framework/core/manager/src/devmgr_service.c not found
Use os.path.realpath(), which resolves the symlinks and normalizes the paths correctly.
# cat compile_commands.json
...
{
"command": ...
"directory": ...
"file": "/path/to/bla/drivers/hdf_core/framework/core/manager/src/devmgr_service.c"
},
...
Also fix it in parse_arguments().
Signed-off-by: Jialu Xu <[email protected]>
---
scripts/clang-tools/gen_compile_commands.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
index 180952fb91c1b..5dea4479240bc 100755
--- a/scripts/clang-tools/gen_compile_commands.py
+++ b/scripts/clang-tools/gen_compile_commands.py
@@ -64,7 +64,7 @@ def parse_arguments():
args = parser.parse_args()
return (args.log_level,
- os.path.abspath(args.directory),
+ os.path.realpath(args.directory),
args.output,
args.ar,
args.paths if len(args.paths) > 0 else [args.directory])
@@ -172,8 +172,8 @@ def process_line(root_directory, command_prefix, file_path):
# by Make, so this code replaces the escaped version with '#'.
prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#')
- # Use os.path.abspath() to normalize the path resolving '.' and '..' .
- abs_path = os.path.abspath(os.path.join(root_directory, file_path))
+ # Return the canonical path, eliminating any symbolic links encountered in the path.
+ abs_path = os.path.realpath(os.path.join(root_directory, file_path))
if not os.path.exists(abs_path):
raise ValueError('File %s not found' % abs_path)
return {
--
2.39.2
On Sun, Dec 10, 2023 at 4:06 PM Jialu Xu <[email protected]> wrote:
>
> When a path contains relative symbolic links, os.path.abspath() might
> not follow the symlinks and instead return the absolute path with just
> the relative paths resolved, resulting in an incorrect path.
>
> 1. 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
>
> 2. 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/src/drivers/hdf/khdf/manager/../../../../framework/core/manager/src/devmgr_service.c
>
> 3. os.path.abspath returns "/path/to/src/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/src/framework/core/manager/src/devmgr_service.c not found
>
> Use os.path.realpath(), which resolves the symlinks and normalizes the paths correctly.
>
> # cat compile_commands.json
> ...
> {
> "command": ...
> "directory": ...
> "file": "/path/to/bla/drivers/hdf_core/framework/core/manager/src/devmgr_service.c"
> },
> ...
>
> Also fix it in parse_arguments().
>
> Signed-off-by: Jialu Xu <[email protected]>
Applied to linux-kbuild. Thanks.
--
Best Regards
Masahiro Yamada