diff mbox series

[v4,2/4] buildtools: use macro to embed information in binaries

Message ID 1744839898-15745-3-git-send-email-andremue@linux.microsoft.com (mailing list archive)
State Superseded
Delegated to: David Marchand
Headers
Series allow pmdinfo to be inserted and parsed using MSVC |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Andre Muezerie April 16, 2025, 9:44 p.m. UTC 
The archiver tool from the MSVC toolset is lib.exe. It has different
parameters then it's GNU counterpart "ar".

buildtools\meson.build was updated to use lib.exe when MSVC compiler is
used. This is to allow the code to be built without requiring GNU "ar"
to be installed in that scenario.

Script gen-pmdinfo-cfile.py was updated to use the correct parameters
when using lib.exe.

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 buildtools/gen-pmdinfo-cfile.py | 24 +++++++++++++++++++-----
 buildtools/meson.build          |  4 +++-
 buildtools/pmdinfogen.py        | 11 ++++++-----
 3 files changed, 28 insertions(+), 11 deletions(-)

Comments

David Marchand June 4, 2025, 11:04 a.m. UTC | #1 
Hello Andre,

On Wed, Apr 16, 2025 at 11:45 PM Andre Muezerie
<andremue@linux.microsoft.com> wrote:
>
> The archiver tool from the MSVC toolset is lib.exe. It has different
> parameters then it's GNU counterpart "ar".
>
> buildtools\meson.build was updated to use lib.exe when MSVC compiler is
> used. This is to allow the code to be built without requiring GNU "ar"
> to be installed in that scenario.
>
> Script gen-pmdinfo-cfile.py was updated to use the correct parameters
> when using lib.exe.
>
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> ---
>  buildtools/gen-pmdinfo-cfile.py | 24 +++++++++++++++++++-----
>  buildtools/meson.build          |  4 +++-
>  buildtools/pmdinfogen.py        | 11 ++++++-----
>  3 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/buildtools/gen-pmdinfo-cfile.py b/buildtools/gen-pmdinfo-cfile.py
> index 5fbd51658a..3a078ab6d1 100644
> --- a/buildtools/gen-pmdinfo-cfile.py
> +++ b/buildtools/gen-pmdinfo-cfile.py
> @@ -7,15 +7,29 @@
>  import sys
>  import tempfile
>
> -_, tmp_root, ar, archive, output, *pmdinfogen = sys.argv
> +_, tmp_root, archiver, archive, output, *pmdinfogen = sys.argv
>  with tempfile.TemporaryDirectory(dir=tmp_root) as temp:
>      paths = []
> -    for name in subprocess.run([ar, "t", archive], stdout=subprocess.PIPE,
> -                               check=True).stdout.decode().splitlines():
> +    if archiver == "lib":
> +        archiver_options = ["/LIST", "/NOLOGO"]
> +    else:
> +        archiver_options = ["t"]
> +    for name in (
> +        subprocess.run(
> +            [archiver] + archiver_options + [archive],
> +            stdout=subprocess.PIPE,
> +            check=True,
> +        )
> +        .stdout.decode()
> +        .splitlines()
> +    ):
>          if os.path.exists(name):
>              paths.append(name)

Indent after the change seems strange.

And we need a rebase of this series in any case.
Please send a new revision.
Andre Muezerie June 4, 2025, 1:59 p.m. UTC | #2 
On Wed, Jun 04, 2025 at 01:04:29PM +0200, David Marchand wrote:
> Hello Andre,
> 
> On Wed, Apr 16, 2025 at 11:45 PM Andre Muezerie
> <andremue@linux.microsoft.com> wrote:
> >
> > The archiver tool from the MSVC toolset is lib.exe. It has different
> > parameters then it's GNU counterpart "ar".
> >
> > buildtools\meson.build was updated to use lib.exe when MSVC compiler is
> > used. This is to allow the code to be built without requiring GNU "ar"
> > to be installed in that scenario.
> >
> > Script gen-pmdinfo-cfile.py was updated to use the correct parameters
> > when using lib.exe.
> >
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > ---
> >  buildtools/gen-pmdinfo-cfile.py | 24 +++++++++++++++++++-----
> >  buildtools/meson.build          |  4 +++-
> >  buildtools/pmdinfogen.py        | 11 ++++++-----
> >  3 files changed, 28 insertions(+), 11 deletions(-)
> >
> > diff --git a/buildtools/gen-pmdinfo-cfile.py b/buildtools/gen-pmdinfo-cfile.py
> > index 5fbd51658a..3a078ab6d1 100644
> > --- a/buildtools/gen-pmdinfo-cfile.py
> > +++ b/buildtools/gen-pmdinfo-cfile.py
> > @@ -7,15 +7,29 @@
> >  import sys
> >  import tempfile
> >
> > -_, tmp_root, ar, archive, output, *pmdinfogen = sys.argv
> > +_, tmp_root, archiver, archive, output, *pmdinfogen = sys.argv
> >  with tempfile.TemporaryDirectory(dir=tmp_root) as temp:
> >      paths = []
> > -    for name in subprocess.run([ar, "t", archive], stdout=subprocess.PIPE,
> > -                               check=True).stdout.decode().splitlines():
> > +    if archiver == "lib":
> > +        archiver_options = ["/LIST", "/NOLOGO"]
> > +    else:
> > +        archiver_options = ["t"]
> > +    for name in (
> > +        subprocess.run(
> > +            [archiver] + archiver_options + [archive],
> > +            stdout=subprocess.PIPE,
> > +            check=True,
> > +        )
> > +        .stdout.decode()
> > +        .splitlines()
> > +    ):
> >          if os.path.exists(name):
> >              paths.append(name)
> 
> Indent after the change seems strange.
> 
> And we need a rebase of this series in any case.
> Please send a new revision.
> 

I rebased the code and formatted it using Ruff. I agree that the formatting
looks substantially different than the original code and it takes a while to
get used to it, but an argument could be made that the new formatting looks
better.

> 
> 
> -- 
> David Marchand
diff mbox series

Patch

diff --git a/buildtools/gen-pmdinfo-cfile.py b/buildtools/gen-pmdinfo-cfile.py
index 5fbd51658a..3a078ab6d1 100644
--- a/buildtools/gen-pmdinfo-cfile.py
+++ b/buildtools/gen-pmdinfo-cfile.py
@@ -7,15 +7,29 @@ 
 import sys
 import tempfile
 
-_, tmp_root, ar, archive, output, *pmdinfogen = sys.argv
+_, tmp_root, archiver, archive, output, *pmdinfogen = sys.argv
 with tempfile.TemporaryDirectory(dir=tmp_root) as temp:
     paths = []
-    for name in subprocess.run([ar, "t", archive], stdout=subprocess.PIPE,
-                               check=True).stdout.decode().splitlines():
+    if archiver == "lib":
+        archiver_options = ["/LIST", "/NOLOGO"]
+    else:
+        archiver_options = ["t"]
+    for name in (
+        subprocess.run(
+            [archiver] + archiver_options + [archive],
+            stdout=subprocess.PIPE,
+            check=True,
+        )
+        .stdout.decode()
+        .splitlines()
+    ):
         if os.path.exists(name):
             paths.append(name)
         else:
-            subprocess.run([ar, "x", os.path.abspath(archive), name],
-                           check=True, cwd=temp)
+            if archiver == "lib":
+                run_args = [archiver, f"/EXTRACT:{name}", os.path.abspath(archive)]
+            else:
+                run_args = [archiver, "x", os.path.abspath(archive), name]
+            subprocess.run(run_args, check=True, cwd=temp)
             paths.append(os.path.join(temp, name))
     subprocess.run(pmdinfogen + paths + [output], check=True)
diff --git a/buildtools/meson.build b/buildtools/meson.build
index 1cd1ce02fd..11221f02ea 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -37,7 +37,9 @@  install_data([
 pmdinfo = py3 + files('gen-pmdinfo-cfile.py') + [meson.current_build_dir()]
 pmdinfogen = py3 + files('pmdinfogen.py')
 if host_machine.system() == 'windows'
-    if cc.get_id() == 'gcc'
+    if cc.get_id() == 'msvc'
+        pmdinfo += 'lib'
+    elif cc.get_id() == 'gcc'
         pmdinfo += 'ar'
     else
         pmdinfo += 'llvm-ar'
diff --git a/buildtools/pmdinfogen.py b/buildtools/pmdinfogen.py
index dfb89500c0..710913083f 100755
--- a/buildtools/pmdinfogen.py
+++ b/buildtools/pmdinfogen.py
@@ -87,7 +87,7 @@  def get_value(self, offset, size):
     @property
     def string_value(self):
         value = self._symbol.get_value(0)
-        return coff.decode_asciiz(value) if value else ''
+        return coff.decode_asciiz(value) if value else ""
 
 
 class COFFImage:
@@ -192,7 +192,7 @@  def dump(self, file):
         dumped = json.dumps(self.__dict__)
         escaped = dumped.replace('"', '\\"')
         print(
-            'const char %s_pmd_info[] __attribute__((used)) = "PMD_INFO_STRING= %s";'
+            'RTE_INCLUDE(const char, %s_pmd_info)[] = "PMD_INFO_STRING= %s";'
             % (self.name, escaped),
             file=file,
         )
@@ -215,7 +215,7 @@  def parse_args():
     parser = argparse.ArgumentParser()
     parser.add_argument("format", help="object file format, 'elf' or 'coff'")
     parser.add_argument(
-        "input", nargs='+', help="input object file path or '-' for stdin"
+        "input", nargs="+", help="input object file path or '-' for stdin"
     )
     parser.add_argument("output", help="output C file path or '-' for stdout")
     return parser.parse_args()
@@ -252,13 +252,14 @@  def open_output(path):
 
 def write_header(output):
     output.write(
-        "static __attribute__((unused)) const char *generator = \"%s\";\n" % sys.argv[0]
+        "#include <rte_common.h>\n"
+        'static __rte_unused const char *generator = "%s";\n' % sys.argv[0]
     )
 
 
 def main():
     args = parse_args()
-    if args.input.count('-') > 1:
+    if args.input.count("-") > 1:
         raise Exception("'-' input cannot be used multiple times")
     if args.format == "elf" and "ELFFile" not in globals():
         raise Exception("elftools module not found")