[dpdk-dev] [PATCH v1 1/2] devtools: add internal ABI version check

[dpdk-dev] [PATCH v1 1/2] devtools: add internal ABI version check

[Haiyue Wang]

INTERNAL is new introduced version, update the shell script that checks
whether built libraries are versioned with expected ABI (current ABI,
current ABI + 1, EXPERIMENTAL, or INTERNAL).

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 devtools/check-abi-version.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/devtools/check-abi-version.sh b/devtools/check-abi-version.sh
index 9a3d13546..f0cca42a9 100755
--- a/devtools/check-abi-version.sh
+++ b/devtools/check-abi-version.sh
@@ -4,7 +4,7 @@
 
 # Check whether library symbols have correct
 # version (provided ABI number or provided ABI
-# number + 1 or EXPERIMENTAL).
+# number + 1 or EXPERIMENTAL or INTERNAL).
 # Args:
 #   $1: path of the library .so file
 #   $2: ABI major version number to check
@@ -12,7 +12,7 @@
 
 if [ -z "$1" ]; then
     echo "Script checks whether library symbols have"
-    echo "correct version (ABI_VER/ABI_VER+1/EXPERIMENTAL)"
+    echo "correct version (ABI_VER/ABI_VER+1/EXPERIMENTAL/INTERNAL)"
     echo "Usage:"
     echo "  $0 SO_FILE_PATH [ABI_VER]"
     exit 1
@@ -41,11 +41,11 @@ for SYM in $(echo "${OBJ_DUMP_OUTPUT}" | awk '{print $(NF-1) "-" $NF}')
 do
     version=$(echo $SYM | cut -d'-' -f 1)
     symbol=$(echo $SYM | cut -d'-' -f 2)
-    case $version in (*"$ABIVER"*|*"$NEXT_ABIVER"*|"EXPERIMENTAL")
+    case $version in (*"$ABIVER"*|*"$NEXT_ABIVER"*|"EXPERIMENTAL"|"INTERNAL")
         ;;
     (*)
         echo "Warning: symbol $symbol ($version) should be annotated " \
-             "as ABI version $ABIVER / $NEXT_ABIVER, or EXPERIMENTAL."
+             "as ABI version $ABIVER / $NEXT_ABIVER, EXPERIMENTAL, or INTERNAL."
         ret=1
     ;;
     esac
-- 
2.26.2

[dpdk-dev] [PATCH v1 2/2] devtools: updating internal symbols ABI version

[Haiyue Wang]

INTERNAL is new introduced version, update the script that automatically
leaving internal section exactly as it is.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 devtools/update_version_map_abi.py | 37 +++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/devtools/update_version_map_abi.py b/devtools/update_version_map_abi.py
index 616412a1c..e2104e61e 100755
--- a/devtools/update_version_map_abi.py
+++ b/devtools/update_version_map_abi.py
@@ -50,7 +50,10 @@ def __parse_map_file(f_in):
     stable_lines = set()
     # copy experimental section as is
     experimental_lines = []
+    # copy internal section as is
+    internal_lines = []
     in_experimental = False
+    in_internal = False
     has_stable = False
 
     # gather all functions
@@ -63,6 +66,7 @@ def __parse_map_file(f_in):
         if match:
             # whatever section this was, it's not active any more
             in_experimental = False
+            in_internal = False
             continue
 
         # if we're in the middle of experimental section, we need to copy
@@ -71,6 +75,12 @@ def __parse_map_file(f_in):
             experimental_lines += [line]
             continue
 
+        # if we're in the middle of internal section, we need to copy
+        # the section verbatim, so just add the line
+        if in_internal:
+            internal_lines += [line]
+            continue
+
         # skip empty lines
         if not line:
             continue
@@ -81,7 +91,9 @@ def __parse_map_file(f_in):
             cur_section = match.group("version")
             # is it experimental?
             in_experimental = cur_section == "EXPERIMENTAL"
-            if not in_experimental:
+            # is it internal?
+            in_internal = cur_section == "INTERNAL"
+            if not in_experimental and not in_internal:
                 has_stable = True
             continue
 
@@ -90,7 +102,7 @@ def __parse_map_file(f_in):
         if match:
             stable_lines.add(match.group("func"))
 
-    return has_stable, stable_lines, experimental_lines
+    return has_stable, stable_lines, experimental_lines, internal_lines
 
 
 def __generate_stable_abi(f_out, abi_version, lines):
@@ -132,6 +144,20 @@ def __generate_experimental_abi(f_out, lines):
     # end section
     print("};", file=f_out)
 
+def __generate_internal_abi(f_out, lines):
+    # start internal section
+    print("INTERNAL {", file=f_out)
+
+    # print all internal lines as they were
+    for line in lines:
+        # don't print empty whitespace
+        if not line:
+            print("", file=f_out)
+        else:
+            print("\t{}".format(line), file=f_out)
+
+    # end section
+    print("};", file=f_out)
 
 def __main():
     arg_parser = argparse.ArgumentParser(
@@ -158,7 +184,7 @@ def __main():
         sys.exit(1)
 
     with open(parsed.map_file) as f_in:
-        has_stable, stable_lines, experimental_lines = __parse_map_file(f_in)
+        has_stable, stable_lines, experimental_lines, internal_lines = __parse_map_file(f_in)
 
     with open(parsed.map_file, 'w') as f_out:
         need_newline = has_stable and experimental_lines
@@ -169,6 +195,11 @@ def __main():
             print(file=f_out)
         if experimental_lines:
             __generate_experimental_abi(f_out, experimental_lines)
+        if internal_lines:
+            if has_stable or experimental_lines:
+              # separate sections with a newline
+              print(file=f_out)
+            __generate_internal_abi(f_out, internal_lines)
 
 
 if __name__ == "__main__":
-- 
2.26.2

Re: [dpdk-dev] [PATCH v1 2/2] devtools: updating internal symbols ABI version

[David Marchand]

On Thu, Apr 30, 2020 at 7:54 AM Haiyue Wang <haiyue.wang@intel.com> wrote:
>
> INTERNAL is new introduced version, update the script that automatically
> leaving internal section exactly as it is.
>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
>  devtools/update_version_map_abi.py | 37 +++++++++++++++++++++++++++---
>  1 file changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/devtools/update_version_map_abi.py b/devtools/update_version_map_abi.py
> index 616412a1c..e2104e61e 100755
> --- a/devtools/update_version_map_abi.py
> +++ b/devtools/update_version_map_abi.py
> @@ -50,7 +50,10 @@ def __parse_map_file(f_in):
>      stable_lines = set()
>      # copy experimental section as is
>      experimental_lines = []
> +    # copy internal section as is
> +    internal_lines = []
>      in_experimental = False
> +    in_internal = False
>      has_stable = False
>
>      # gather all functions
> @@ -63,6 +66,7 @@ def __parse_map_file(f_in):
>          if match:
>              # whatever section this was, it's not active any more
>              in_experimental = False
> +            in_internal = False
>              continue
>
>          # if we're in the middle of experimental section, we need to copy
> @@ -71,6 +75,12 @@ def __parse_map_file(f_in):
>              experimental_lines += [line]
>              continue
>
> +        # if we're in the middle of internal section, we need to copy
> +        # the section verbatim, so just add the line
> +        if in_internal:
> +            internal_lines += [line]
> +            continue
> +
>          # skip empty lines
>          if not line:
>              continue
> @@ -81,7 +91,9 @@ def __parse_map_file(f_in):
>              cur_section = match.group("version")
>              # is it experimental?
>              in_experimental = cur_section == "EXPERIMENTAL"
> -            if not in_experimental:
> +            # is it internal?
> +            in_internal = cur_section == "INTERNAL"
> +            if not in_experimental and not in_internal:
>                  has_stable = True
>              continue
>
> @@ -90,7 +102,7 @@ def __parse_map_file(f_in):
>          if match:
>              stable_lines.add(match.group("func"))
>
> -    return has_stable, stable_lines, experimental_lines
> +    return has_stable, stable_lines, experimental_lines, internal_lines
>
>
>  def __generate_stable_abi(f_out, abi_version, lines):
> @@ -132,6 +144,20 @@ def __generate_experimental_abi(f_out, lines):
>      # end section
>      print("};", file=f_out)
>
> +def __generate_internal_abi(f_out, lines):
> +    # start internal section
> +    print("INTERNAL {", file=f_out)
> +
> +    # print all internal lines as they were
> +    for line in lines:
> +        # don't print empty whitespace
> +        if not line:
> +            print("", file=f_out)
> +        else:
> +            print("\t{}".format(line), file=f_out)
> +
> +    # end section
> +    print("};", file=f_out)
>
>  def __main():
>      arg_parser = argparse.ArgumentParser(
> @@ -158,7 +184,7 @@ def __main():
>          sys.exit(1)
>
>      with open(parsed.map_file) as f_in:
> -        has_stable, stable_lines, experimental_lines = __parse_map_file(f_in)
> +        has_stable, stable_lines, experimental_lines, internal_lines = __parse_map_file(f_in)
>
>      with open(parsed.map_file, 'w') as f_out:
>          need_newline = has_stable and experimental_lines
> @@ -169,6 +195,11 @@ def __main():
>              print(file=f_out)
>          if experimental_lines:
>              __generate_experimental_abi(f_out, experimental_lines)
> +        if internal_lines:
> +            if has_stable or experimental_lines:
> +              # separate sections with a newline
> +              print(file=f_out)
> +            __generate_internal_abi(f_out, internal_lines)
>
>
>  if __name__ == "__main__":
> --
> 2.26.2
>

LGTM.
Acked-by: David Marchand <david.marchand@redhat.com>


One comment, trying to update to ABI 21, the script refuses and
expects a 21.X format:
$ ./devtools/update-abi.sh 21
ABI version must be formatted as MAJOR.MINOR version

And passing 21.0 then generates DPDK_21.0 blocks which I understand
are incorrect.
$ git diff drivers/common/iavf/rte_common_iavf_version.map
diff --git a/drivers/common/iavf/rte_common_iavf_version.map
b/drivers/common/iavf/rte_common_iavf_version.map
index 92ceac108d..9a1ef076aa 100644
--- a/drivers/common/iavf/rte_common_iavf_version.map
+++ b/drivers/common/iavf/rte_common_iavf_version.map
@@ -1,11 +1,11 @@
-DPDK_21 {
+DPDK_21.0 {
        global:


-- 
David Marchand

Re: [dpdk-dev] [PATCH v1 1/2] devtools: add internal ABI version check

[David Marchand]

On Thu, Apr 30, 2020 at 7:54 AM Haiyue Wang <haiyue.wang@intel.com> wrote:
>
> INTERNAL is new introduced version, update the shell script that checks
> whether built libraries are versioned with expected ABI (current ABI,
> current ABI + 1, EXPERIMENTAL, or INTERNAL).
>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
>  devtools/check-abi-version.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/devtools/check-abi-version.sh b/devtools/check-abi-version.sh
> index 9a3d13546..f0cca42a9 100755
> --- a/devtools/check-abi-version.sh
> +++ b/devtools/check-abi-version.sh
> @@ -4,7 +4,7 @@
>
>  # Check whether library symbols have correct
>  # version (provided ABI number or provided ABI
> -# number + 1 or EXPERIMENTAL).
> +# number + 1 or EXPERIMENTAL or INTERNAL).
>  # Args:
>  #   $1: path of the library .so file
>  #   $2: ABI major version number to check
> @@ -12,7 +12,7 @@
>
>  if [ -z "$1" ]; then
>      echo "Script checks whether library symbols have"
> -    echo "correct version (ABI_VER/ABI_VER+1/EXPERIMENTAL)"
> +    echo "correct version (ABI_VER/ABI_VER+1/EXPERIMENTAL/INTERNAL)"
>      echo "Usage:"
>      echo "  $0 SO_FILE_PATH [ABI_VER]"
>      exit 1
> @@ -41,11 +41,11 @@ for SYM in $(echo "${OBJ_DUMP_OUTPUT}" | awk '{print $(NF-1) "-" $NF}')
>  do
>      version=$(echo $SYM | cut -d'-' -f 1)
>      symbol=$(echo $SYM | cut -d'-' -f 2)
> -    case $version in (*"$ABIVER"*|*"$NEXT_ABIVER"*|"EXPERIMENTAL")
> +    case $version in (*"$ABIVER"*|*"$NEXT_ABIVER"*|"EXPERIMENTAL"|"INTERNAL")
>          ;;
>      (*)
>          echo "Warning: symbol $symbol ($version) should be annotated " \
> -             "as ABI version $ABIVER / $NEXT_ABIVER, or EXPERIMENTAL."
> +             "as ABI version $ABIVER / $NEXT_ABIVER, EXPERIMENTAL, or INTERNAL."
>          ret=1
>      ;;
>      esac
> --
> 2.26.2
>

LGTM + tested current master before and after the patch.

Acked-by: David Marchand <david.marchand@redhat.com>

-- 
David Marchand

Re: [dpdk-dev] [PATCH v1 1/2] devtools: add internal ABI version check

[David Marchand]

On Tue, May 19, 2020 at 5:35 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Thu, Apr 30, 2020 at 7:54 AM Haiyue Wang <haiyue.wang@intel.com> wrote:
> >
> > INTERNAL is new introduced version, update the shell script that checks
> > whether built libraries are versioned with expected ABI (current ABI,
> > current ABI + 1, EXPERIMENTAL, or INTERNAL).
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> Acked-by: David Marchand <david.marchand@redhat.com>

Series applied, thanks.


-- 
David Marchand