All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH rdma-core 0/3] cbuild tweaks
@ 2017-08-04 16:57 Nicolas Morey-Chaisemartin
       [not found] ` <27366048-d80f-3711-c6cd-93c87809650e-l3A5Bk7waGM@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Nicolas Morey-Chaisemartin @ 2017-08-04 16:57 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA

Few minor cbuild tweaks needed to upstream SUSE spec file
The first one is purely cosmetic to qllow alignment in the spec file

The two other could be worked around by changing the SUSE spec but it means it won't be 100% match with the upstream one.

Nicolas Morey-Chaisemartin (3):
  cbuild: allow indentation in RPM version
  cbuild: support .tar.(gz|bz|>...) extensions
  cbuild: use rpmspec for macro replacement

 .gitignore      |  3 +++
 buildlib/cbuild | 14 +++++++++-----
 2 files changed, 12 insertions(+), 5 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH rdma-core 1/3] cbuild: allow indentation in RPM version
       [not found] ` <27366048-d80f-3711-c6cd-93c87809650e-l3A5Bk7waGM@public.gmane.org>
@ 2017-08-04 16:58   ` Nicolas Morey-Chaisemartin
  2017-08-04 16:59   ` [PATCH rdma-core 2/3] cbuild: support .tar.(gz|bz|>...) extensions Nicolas Morey-Chaisemartin
  2017-08-04 16:59   ` [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement Nicolas Morey-Chaisemartin
  2 siblings, 0 replies; 14+ messages in thread
From: Nicolas Morey-Chaisemartin @ 2017-08-04 16:58 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA

Fix support for:
Version:      <id>
with multiple spaces

Signed-off-by: Nicolas Morey-Chaisemartin <NMoreyChaisemartin-IBi9RG/b67k@public.gmane.org>
---
 buildlib/cbuild | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/buildlib/cbuild b/buildlib/cbuild
index 888fae6c..efc039bf 100755
--- a/buildlib/cbuild
+++ b/buildlib/cbuild
@@ -407,11 +407,11 @@ def run_rpm_build(args,spec_file,env):
     with open(spec_file,"r") as F:
         for ln in F:
             if ln.startswith("Version:"):
-                ver = ln.strip().partition(' ')[2];
+                ver = ln.strip().partition(' ')[2].strip();
                 assert(ver == get_version());
 
             if ln.startswith("Source:"):
-                tarfn = ln.strip().partition(' ')[2];
+                tarfn = ln.strip().partition(' ')[2].strip();
     tarfn = tarfn.replace("%{version}",version);
 
     image_id = get_image_id(args,env.image_name());
-- 
2.14.0.rc1.4.gab083fa77.dirty


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH rdma-core 2/3] cbuild: support .tar.(gz|bz|>...) extensions
       [not found] ` <27366048-d80f-3711-c6cd-93c87809650e-l3A5Bk7waGM@public.gmane.org>
  2017-08-04 16:58   ` [PATCH rdma-core 1/3] cbuild: allow indentation in RPM version Nicolas Morey-Chaisemartin
@ 2017-08-04 16:59   ` Nicolas Morey-Chaisemartin
  2017-08-04 16:59   ` [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement Nicolas Morey-Chaisemartin
  2 siblings, 0 replies; 14+ messages in thread
From: Nicolas Morey-Chaisemartin @ 2017-08-04 16:59 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA

Signed-off-by: Nicolas Morey-Chaisemartin <NMoreyChaisemartin-IBi9RG/b67k@public.gmane.org>
---
 buildlib/cbuild | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/buildlib/cbuild b/buildlib/cbuild
index efc039bf..5228ec1b 100755
--- a/buildlib/cbuild
+++ b/buildlib/cbuild
@@ -420,7 +420,7 @@ def run_rpm_build(args,spec_file,env):
         os.mkdir(os.path.join(tmpdir,"tmp"));
 
         subprocess.check_call(["git","archive",
-                               "--prefix","%s/"%(os.path.splitext(tarfn)[0]),
+                               "--prefix","%s/"%(os.path.splitext(os.path.splitext(tarfn)[0])[0]),
                                "--output",os.path.join(tmpdir,"SOURCES",tarfn),
                                "HEAD"]);
 
-- 
2.14.0.rc1.4.gab083fa77.dirty


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement
       [not found] ` <27366048-d80f-3711-c6cd-93c87809650e-l3A5Bk7waGM@public.gmane.org>
  2017-08-04 16:58   ` [PATCH rdma-core 1/3] cbuild: allow indentation in RPM version Nicolas Morey-Chaisemartin
  2017-08-04 16:59   ` [PATCH rdma-core 2/3] cbuild: support .tar.(gz|bz|>...) extensions Nicolas Morey-Chaisemartin
@ 2017-08-04 16:59   ` Nicolas Morey-Chaisemartin
       [not found]     ` <8ad0438f-b4a5-aec9-1e44-a9c9dc1ca17b-l3A5Bk7waGM@public.gmane.org>
  2 siblings, 1 reply; 14+ messages in thread
From: Nicolas Morey-Chaisemartin @ 2017-08-04 16:59 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA

Generate a temporary file with the spec file parsed and all its macro replaced.
Avoid manually replacing Version and handle extra macros

Signed-off-by: Nicolas Morey-Chaisemartin <NMoreyChaisemartin-IBi9RG/b67k@public.gmane.org>
---
 .gitignore      | 3 +++
 buildlib/cbuild | 8 ++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index f31a0617..52c7af2f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -69,3 +69,6 @@ tramp
 Session.vim
 .netrwhist
 *~
+
+# rpm
+*/*.spec.processed
\ No newline at end of file
diff --git a/buildlib/cbuild b/buildlib/cbuild
index 5228ec1b..cf7828eb 100755
--- a/buildlib/cbuild
+++ b/buildlib/cbuild
@@ -404,7 +404,12 @@ def get_image_id(args,image_name):
 
 def run_rpm_build(args,spec_file,env):
     version = get_version();
-    with open(spec_file,"r") as F:
+
+    # Pre-process file with rpm spec to avoid manually replacing macros
+    f = open(spec_file + ".processed", "w");
+    subprocess.check_call(["rpmspec", "-P", spec_file], stdout=f);
+
+    with open(spec_file + ".processed" ,"r") as F:
         for ln in F:
             if ln.startswith("Version:"):
                 ver = ln.strip().partition(' ')[2].strip();
@@ -412,7 +417,6 @@ def run_rpm_build(args,spec_file,env):
 
             if ln.startswith("Source:"):
                 tarfn = ln.strip().partition(' ')[2].strip();
-    tarfn = tarfn.replace("%{version}",version);
 
     image_id = get_image_id(args,env.image_name());
     with private_tmp(args) as tmpdir:
-- 
2.14.0.rc1.4.gab083fa77.dirty

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement
       [not found]     ` <8ad0438f-b4a5-aec9-1e44-a9c9dc1ca17b-l3A5Bk7waGM@public.gmane.org>
@ 2017-08-08 23:04       ` Jason Gunthorpe
       [not found]         ` <20170808230459.GF29372-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Gunthorpe @ 2017-08-08 23:04 UTC (permalink / raw)
  To: Nicolas Morey-Chaisemartin; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Fri, Aug 04, 2017 at 06:59:10PM +0200, Nicolas Morey-Chaisemartin wrote:
> Generate a temporary file with the spec file parsed and all its macro replaced.
> Avoid manually replacing Version and handle extra macros
> 
> Signed-off-by: Nicolas Morey-Chaisemartin <NMoreyChaisemartin-IBi9RG/b67k@public.gmane.org>
>  .gitignore      | 3 +++
>  buildlib/cbuild | 8 ++++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index f31a0617..52c7af2f 100644
> +++ b/.gitignore
> @@ -69,3 +69,6 @@ tramp
>  Session.vim
>  .netrwhist
>  *~
> +
> +# rpm
> +*/*.spec.processed
> \ No newline at end of file
> diff --git a/buildlib/cbuild b/buildlib/cbuild
> index 5228ec1b..cf7828eb 100755
> +++ b/buildlib/cbuild
> @@ -404,7 +404,12 @@ def get_image_id(args,image_name):
>  
>  def run_rpm_build(args,spec_file,env):
>      version = get_version();
> -    with open(spec_file,"r") as F:
> +
> +    # Pre-process file with rpm spec to avoid manually replacing macros
> +    f = open(spec_file + ".processed", "w");
> +    subprocess.check_call(["rpmspec", "-P", spec_file], stdout=f);

This isn't going to work on any host system that does not have
rpmspec, including my Debian systems..

How about something like this instead for the two patches?

>From 13e239b5fc39fff9e56760969b99eab462db3d89 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Date: Tue, 8 Aug 2017 17:04:06 -0600
Subject: [PATCH] cbuild: Be more robust when computing path names

Always use the same format for the git archvie prefix, this matches
the format that github-release uses, and is what a .spec file must work
with.

Use rpmspec -P to determine what the expected tar file name is instead of
trying to do substitution internally.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 buildlib/cbuild | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/buildlib/cbuild b/buildlib/cbuild
index 9b49fa5343c8b4..a3cf26e85bc722 100755
--- a/buildlib/cbuild
+++ b/buildlib/cbuild
@@ -409,16 +409,10 @@ def get_image_id(args,image_name):
 # -------------------------------------------------------------------------
 
 def run_rpm_build(args,spec_file,env):
-    version = get_version();
     with open(spec_file,"r") as F:
         for ln in F:
-            if ln.startswith("Version:"):
-                ver = ln.strip().partition(' ')[2];
-                assert(ver == get_version());
-
             if ln.startswith("Source:"):
                 tarfn = ln.strip().partition(' ')[2];
-    tarfn = tarfn.replace("%{version}",version);
 
     image_id = get_image_id(args,env.image_name());
     with private_tmp(args) as tmpdir:
@@ -426,7 +420,8 @@ def run_rpm_build(args,spec_file,env):
         os.mkdir(os.path.join(tmpdir,"tmp"));
 
         subprocess.check_call(["git","archive",
-                               "--prefix","%s/"%(os.path.splitext(tarfn)[0]),
+                               # This must match the prefix generated buildlib/github-release
+                               "--prefix","%s-%s/"%(project,get_version()),
                                "--output",os.path.join(tmpdir,"SOURCES",tarfn),
                                "HEAD"]);
 
@@ -453,17 +448,25 @@ def run_rpm_build(args,spec_file,env):
         # for the user we are going to use to do the build.
         with open(os.path.join(tmpdir,"go.py"),"w") as F:
             print >> F,"""
-import os;
+import os,subprocess;
 with open("/etc/passwd","a") as F:
    print >> F, {passwd!r};
 with open("/etc/group","a") as F:
    print >> F, {group!r};
 os.setgid({gid:d});
 os.setuid({uid:d});
+
+# Get RPM to tell us the expected tar filename.
+for ln in subprocess.check_output(["rpmspec","-P",{tspec_file!r}]).splitlines():
+   if ln.startswith("Source:"):
+      tarfn = ln.strip().partition(' ')[2];
+os.symlink({tarfn!r},os.path.join("SOURCES",tarfn));
 """.format(passwd=":".join(str(I) for I in pwd.getpwuid(os.getuid())),
            group=":".join(str(I) for I in grp.getgrgid(os.getgid())),
            uid=os.getuid(),
-           gid=os.getgid());
+           gid=os.getgid(),
+           tarfn=tarfn,
+           tspec_file=tspec_file);
 
             bopts = ["-bb",tspec_file];
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement
       [not found]         ` <20170808230459.GF29372-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
@ 2017-08-09  9:56           ` Nicolas Morey-Chaisemartin
       [not found]             ` <b3993b0c-0b9d-a917-cbf8-22f68c1d2a97-l3A5Bk7waGM@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Nicolas Morey-Chaisemartin @ 2017-08-09  9:56 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA



Le 09/08/2017 à 01:04, Jason Gunthorpe a écrit :
> On Fri, Aug 04, 2017 at 06:59:10PM +0200, Nicolas Morey-Chaisemartin wrote:
>> Generate a temporary file with the spec file parsed and all its macro replaced.
>> Avoid manually replacing Version and handle extra macros
>>
>> Signed-off-by: Nicolas Morey-Chaisemartin <NMoreyChaisemartin-IBi9RG/b67k@public.gmane.org>
>>  .gitignore      | 3 +++
>>  buildlib/cbuild | 8 ++++++--
>>  2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/.gitignore b/.gitignore
>> index f31a0617..52c7af2f 100644
>> +++ b/.gitignore
>> @@ -69,3 +69,6 @@ tramp
>>  Session.vim
>>  .netrwhist
>>  *~
>> +
>> +# rpm
>> +*/*.spec.processed
>> \ No newline at end of file
>> diff --git a/buildlib/cbuild b/buildlib/cbuild
>> index 5228ec1b..cf7828eb 100755
>> +++ b/buildlib/cbuild
>> @@ -404,7 +404,12 @@ def get_image_id(args,image_name):
>>  
>>  def run_rpm_build(args,spec_file,env):
>>      version = get_version();
>> -    with open(spec_file,"r") as F:
>> +
>> +    # Pre-process file with rpm spec to avoid manually replacing macros
>> +    f = open(spec_file + ".processed", "w");
>> +    subprocess.check_call(["rpmspec", "-P", spec_file], stdout=f);
> This isn't going to work on any host system that does not have
> rpmspec, including my Debian systems..
Are you building RPM outside of docker on your Debian ?
I ran the pkg suite for all targets and they all work.
rpmspec is with RPM since v4.9.0 which came out in early 2011 so it should work with all current RPM -based distro

> How about something like this instead for the two patches?
>
> From 13e239b5fc39fff9e56760969b99eab462db3d89 Mon Sep 17 00:00:00 2001
> From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Date: Tue, 8 Aug 2017 17:04:06 -0600
> Subject: [PATCH] cbuild: Be more robust when computing path names
>
> Always use the same format for the git archvie prefix, this matches
> the format that github-release uses, and is what a .spec file must work
> with.
>
> Use rpmspec -P to determine what the expected tar file name is instead of
> trying to do substitution internally.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  buildlib/cbuild | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/buildlib/cbuild b/buildlib/cbuild
> index 9b49fa5343c8b4..a3cf26e85bc722 100755
> --- a/buildlib/cbuild
> +++ b/buildlib/cbuild
> @@ -409,16 +409,10 @@ def get_image_id(args,image_name):
>  # -------------------------------------------------------------------------
>  
>  def run_rpm_build(args,spec_file,env):
> -    version = get_version();
>      with open(spec_file,"r") as F:
>          for ln in F:
> -            if ln.startswith("Version:"):
> -                ver = ln.strip().partition(' ')[2];
> -                assert(ver == get_version());
> -
You are losing this last check which seems important. It make sure the ti tag is in sync with the RPM

>              if ln.startswith("Source:"):
>                  tarfn = ln.strip().partition(' ')[2];
> -    tarfn = tarfn.replace("%{version}",version);
>  
>      image_id = get_image_id(args,env.image_name());
>      with private_tmp(args) as tmpdir:
> @@ -426,7 +420,8 @@ def run_rpm_build(args,spec_file,env):
>          os.mkdir(os.path.join(tmpdir,"tmp"));
>  
>          subprocess.check_call(["git","archive",
> -                               "--prefix","%s/"%(os.path.splitext(tarfn)[0]),
> +                               # This must match the prefix generated buildlib/github-release
> +                               "--prefix","%s-%s/"%(project,get_version()),
>                                 "--output",os.path.join(tmpdir,"SOURCES",tarfn),
You have an issue here because tarfn is not set yet.
I'm confused on why moving the call to rpmspec further down would solve your Debian issue ?

>                                 "HEAD"]);
>  
> @@ -453,17 +448,25 @@ def run_rpm_build(args,spec_file,env):
>          # for the user we are going to use to do the build.
>          with open(os.path.join(tmpdir,"go.py"),"w") as F:
>              print >> F,"""
> -import os;
> +import os,subprocess;
>  with open("/etc/passwd","a") as F:
>     print >> F, {passwd!r};
>  with open("/etc/group","a") as F:
>     print >> F, {group!r};
>  os.setgid({gid:d});
>  os.setuid({uid:d});
> +
> +# Get RPM to tell us the expected tar filename.
> +for ln in subprocess.check_output(["rpmspec","-P",{tspec_file!r}]).splitlines():
> +   if ln.startswith("Source:"):
> +      tarfn = ln.strip().partition(' ')[2];
> +os.symlink({tarfn!r},os.path.join("SOURCES",tarfn));
>  """.format(passwd=":".join(str(I) for I in pwd.getpwuid(os.getuid())),
>             group=":".join(str(I) for I in grp.getgrgid(os.getgid())),
>             uid=os.getuid(),
> -           gid=os.getgid());
> +           gid=os.getgid(),
> +           tarfn=tarfn,
> +           tspec_file=tspec_file);
>  
>              bopts = ["-bb",tspec_file];
>  


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement
       [not found]             ` <b3993b0c-0b9d-a917-cbf8-22f68c1d2a97-l3A5Bk7waGM@public.gmane.org>
@ 2017-08-09 15:55               ` Jason Gunthorpe
       [not found]                 ` <20170809155532.GC15586-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Gunthorpe @ 2017-08-09 15:55 UTC (permalink / raw)
  To: Nicolas Morey-Chaisemartin; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Wed, Aug 09, 2017 at 11:56:47AM +0200, Nicolas Morey-Chaisemartin wrote:

> >>  def run_rpm_build(args,spec_file,env):
> >>      version = get_version();
> >> -    with open(spec_file,"r") as F:
> >> +
> >> +    # Pre-process file with rpm spec to avoid manually replacing macros
> >> +    f = open(spec_file + ".processed", "w");
> >> +    subprocess.check_call(["rpmspec", "-P", spec_file], stdout=f);

> > This isn't going to work on any host system that does not have
> > rpmspec, including my Debian systems..

> Are you building RPM outside of docker on your Debian ?

No..

The issue is the above subprocess.check_call is run on the host, while
the call I added below inside go.py is run inside the container.

The host may not have rpmspec, while we guarentee that the container
does.

> I ran the pkg suite for all targets and they all work.

Your host has rpmspec :)

> >  def run_rpm_build(args,spec_file,env):
> > -    version = get_version();
> >      with open(spec_file,"r") as F:
> >          for ln in F:
> > -            if ln.startswith("Version:"):
> > -                ver = ln.strip().partition(' ')[2];
> > -                assert(ver == get_version());
> > -
> You are losing this last check which seems important. It make sure the ti tag is in sync with the RPM

If you think it is important it can be left in..

> >              if ln.startswith("Source:"):
> >                  tarfn = ln.strip().partition(' ')[2];
> > -    tarfn = tarfn.replace("%{version}",version);
> >  
> >      image_id = get_image_id(args,env.image_name());
> >      with private_tmp(args) as tmpdir:
> > @@ -426,7 +420,8 @@ def run_rpm_build(args,spec_file,env):
> >          os.mkdir(os.path.join(tmpdir,"tmp"));
> >  
> >          subprocess.check_call(["git","archive",
> > -                               "--prefix","%s/"%(os.path.splitext(tarfn)[0]),
> > +                               # This must match the prefix generated buildlib/github-release
> > +                               "--prefix","%s-%s/"%(project,get_version()),
> >                                 "--output",os.path.join(tmpdir,"SOURCES",tarfn),
> You have an issue here because tarfn is not set yet.

No, it was still set above, see the first hunk.

> I'm confused on why moving the call to rpmspec further down would solve your Debian issue ?

Further down the code is run in the container where rpmspec is
guarenteed to exist. Everything in cbuild is run on the host except
for the snippet in go.py

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement
       [not found]                 ` <20170809155532.GC15586-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
@ 2017-08-09 16:39                   ` Nicolas Morey-Chaisemartin
       [not found]                     ` <f42a6eaa-79f9-1063-b58a-08da363d09e7-l3A5Bk7waGM@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Nicolas Morey-Chaisemartin @ 2017-08-09 16:39 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA



Le 09/08/2017 à 17:55, Jason Gunthorpe a écrit :
> On Wed, Aug 09, 2017 at 11:56:47AM +0200, Nicolas Morey-Chaisemartin wrote:
>
>>>>  def run_rpm_build(args,spec_file,env):
>>>>      version = get_version();
>>>> -    with open(spec_file,"r") as F:
>>>> +
>>>> +    # Pre-process file with rpm spec to avoid manually replacing macros
>>>> +    f = open(spec_file + ".processed", "w");
>>>> +    subprocess.check_call(["rpmspec", "-P", spec_file], stdout=f);
>>> This isn't going to work on any host system that does not have
>>> rpmspec, including my Debian systems..
>> Are you building RPM outside of docker on your Debian ?
> No..
>
> The issue is the above subprocess.check_call is run on the host, while
> the call I added below inside go.py is run inside the container.
>
> The host may not have rpmspec, while we guarentee that the container
> does.

Oh OK. I dived into those specific bits of code and assume it was all running in the container.

>
>> I ran the pkg suite for all targets and they all work.
> Your host has rpmspec :)

Yes :)

>
>>>  def run_rpm_build(args,spec_file,env):
>>> -    version = get_version();
>>>      with open(spec_file,"r") as F:
>>>          for ln in F:
>>> -            if ln.startswith("Version:"):
>>> -                ver = ln.strip().partition(' ')[2];
>>> -                assert(ver == get_version());
>>> -
>> You are losing this last check which seems important. It make sure the ti tag is in sync with the RPM
> If you think it is important it can be left in..

Well as we will have 2 spec file, it makes sure no ones forgot a version bump when releasing late at night just before a deadline ;)

>
>>>              if ln.startswith("Source:"):
>>>                  tarfn = ln.strip().partition(' ')[2];
>>> -    tarfn = tarfn.replace("%{version}",version);
>>>  
>>>      image_id = get_image_id(args,env.image_name());
>>>      with private_tmp(args) as tmpdir:
>>> @@ -426,7 +420,8 @@ def run_rpm_build(args,spec_file,env):
>>>          os.mkdir(os.path.join(tmpdir,"tmp"));
>>>  
>>>          subprocess.check_call(["git","archive",
>>> -                               "--prefix","%s/"%(os.path.splitext(tarfn)[0]),
>>> +                               # This must match the prefix generated buildlib/github-release
>>> +                               "--prefix","%s-%s/"%(project,get_version()),
>>>                                 "--output",os.path.join(tmpdir,"SOURCES",tarfn),
>> You have an issue here because tarfn is not set yet.
> No, it was still set above, see the first hunk.

Yes. But the value it has is "wrong"
If I take the current spec file used in OBS:
Source0:        %{name}-%{version}%{git_ver}.tar.xz

Omitting the Source0 that should be changed to Source, this wille set tarfn to rdma-core-15%{git_ver}.tar.xz

>
>> I'm confused on why moving the call to rpmspec further down would solve your Debian issue ?
> Further down the code is run in the container where rpmspec is
> guarenteed to exist. Everything in cbuild is run on the host except
> for the snippet in go.py
>
> Jason

This means that we would need to detar/rename/retar the package in the go.py.
It feels overly complicated :/
What we use this for is mainly for RC and non-release packages. We set the git_ver to -rc1, -<NN>-g<SHA1>  so we know at a glance that this is not an official release.
I can either try to get this working in the go.py, or I'll simply drop this from our spec and bring it back when needed for non release packages.

Nicolas

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement
       [not found]                     ` <f42a6eaa-79f9-1063-b58a-08da363d09e7-l3A5Bk7waGM@public.gmane.org>
@ 2017-08-09 20:19                       ` Jason Gunthorpe
       [not found]                         ` <20170809201932.GA7363-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Gunthorpe @ 2017-08-09 20:19 UTC (permalink / raw)
  To: Nicolas Morey-Chaisemartin; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Wed, Aug 09, 2017 at 06:39:23PM +0200, Nicolas Morey-Chaisemartin wrote:
> >>>          subprocess.check_call(["git","archive",
> >>> -                               "--prefix","%s/"%(os.path.splitext(tarfn)[0]),
> >>> +                               # This must match the prefix generated buildlib/github-release
> >>> +                               "--prefix","%s-%s/"%(project,get_version()),
> >>>                                 "--output",os.path.join(tmpdir,"SOURCES",tarfn),
> >> You have an issue here because tarfn is not set yet.
> > No, it was still set above, see the first hunk.
> 
> Yes. But the value it has is "wrong"
> If I take the current spec file used in OBS:
> Source0:        %{name}-%{version}%{git_ver}.tar.xz
> 
> Omitting the Source0 that should be changed to Source, this wille set tarfn to rdma-core-15%{git_ver}.tar.xz

Okay..

Why not use

%setup -n %{name}-%{version}

In the spec file? Eg do not include the git version in the prefix so
it matches the format produced by the github scripts.

Then, the above will make a archive with 'rdma-core-15/' as the prefix.

The archive will be called

'SOURCE/%{name}-%{version}%{git_ver}.tar.xz'

Then the go.py will parse and determine that the archive should be
called

'SOURCE/rdma-core-15-git124556.tar.xz'

And drop a symlink to fix it up.

> This means that we would need to detar/rename/retar the package in
> the go.py.  It feels overly complicated :/ What we use this for is
> mainly for RC and non-release packages. We set the git_ver to -rc1,
> -<NN>-g<SHA1>  so we know at a glance that this is not an official
> release.  I can either try to get this working in the go.py, or I'll
> simply drop this from our spec and bring it back when needed for non
> release packages.

I think if this is your usage then you would be fine to use '%setup
-n'..

We can already recover the git HEAD from a git archive output by using
'git get-tar-commit-id' so it isn't really necessary to include the
git commit id in the prefix.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement
       [not found]                         ` <20170809201932.GA7363-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
@ 2017-08-09 21:08                           ` Nicolas Morey-Chaisemartin
       [not found]                             ` <5c4de783-ce62-2b8d-78aa-ee2f181fe719-l3A5Bk7waGM@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Nicolas Morey-Chaisemartin @ 2017-08-09 21:08 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA



Le 09/08/2017 à 22:19, Jason Gunthorpe a écrit :
> On Wed, Aug 09, 2017 at 06:39:23PM +0200, Nicolas Morey-Chaisemartin wrote:
>>>>>          subprocess.check_call(["git","archive",
>>>>> -                               "--prefix","%s/"%(os.path.splitext(tarfn)[0]),
>>>>> +                               # This must match the prefix generated buildlib/github-release
>>>>> +                               "--prefix","%s-%s/"%(project,get_version()),
>>>>>                                 "--output",os.path.join(tmpdir,"SOURCES",tarfn),
>>>> You have an issue here because tarfn is not set yet.
>>> No, it was still set above, see the first hunk.
>> Yes. But the value it has is "wrong"
>> If I take the current spec file used in OBS:
>> Source0:        %{name}-%{version}%{git_ver}.tar.xz
>>
>> Omitting the Source0 that should be changed to Source, this wille set tarfn to rdma-core-15%{git_ver}.tar.xz
> Okay..
>
> Why not use
>
> %setup -n %{name}-%{version}
>
> In the spec file? Eg do not include the git version in the prefix so
> it matches the format produced by the github scripts.
>
> Then, the above will make a archive with 'rdma-core-15/' as the prefix.
>
> The archive will be called
>
> 'SOURCE/%{name}-%{version}%{git_ver}.tar.xz'
>
> Then the go.py will parse and determine that the archive should be
> called
>
> 'SOURCE/rdma-core-15-git124556.tar.xz'
>
> And drop a symlink to fix it up.
Yes this should work. It feels a little bit dirty to have a tarball name with unreplaced macro but it's a temporay state so I can live with this :)

>> This means that we would need to detar/rename/retar the package in
>> the go.py.  It feels overly complicated :/ What we use this for is
>> mainly for RC and non-release packages. We set the git_ver to -rc1,
>> -<NN>-g<SHA1>  so we know at a glance that this is not an official
>> release.  I can either try to get this working in the go.py, or I'll
>> simply drop this from our spec and bring it back when needed for non
>> release packages.
> I think if this is your usage then you would be fine to use '%setup
> -n'..
>
> We can already recover the git HEAD from a git archive output by using
> 'git get-tar-commit-id' so it isn't really necessary to include the
> git commit id in the prefix.
>
> Jason

Yes the setup part is easy enough.
There two reason for this:
- I have a lot of packages to maintain and I can see at a glance what is what.
- If someone wants to fork the package in OBS and have its own build (for whatever reason: different compiler, different flag, extra patches, etc.) it clears which exact version it is without digging the SHA1 from the _service file we have or within the tarball.


Nicolas

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement
       [not found]                             ` <5c4de783-ce62-2b8d-78aa-ee2f181fe719-l3A5Bk7waGM@public.gmane.org>
@ 2017-08-09 21:15                               ` Jason Gunthorpe
       [not found]                                 ` <20170809211536.GA10191-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Gunthorpe @ 2017-08-09 21:15 UTC (permalink / raw)
  To: Nicolas Morey-Chaisemartin; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Wed, Aug 09, 2017 at 11:08:34PM +0200, Nicolas Morey-Chaisemartin wrote:

> > And drop a symlink to fix it up.

> Yes this should work. It feels a little bit dirty to have a tarball
> name with unreplaced macro but it's a temporay state so I can live
> with this :)

It is a bit strange, but this trivially delegates parsing the .tar.xx
to git archive which understands all the formats..

Here is the patches, should I send them?

https://github.com/jgunthorpe/rdma-plumbing/tree/cbuild

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement
       [not found]                                 ` <20170809211536.GA10191-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
@ 2017-08-13  6:37                                   ` Leon Romanovsky
       [not found]                                     ` <20170813063706.GQ24282-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Leon Romanovsky @ 2017-08-13  6:37 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Nicolas Morey-Chaisemartin, linux-rdma-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

On Wed, Aug 09, 2017 at 03:15:36PM -0600, Jason Gunthorpe wrote:
> On Wed, Aug 09, 2017 at 11:08:34PM +0200, Nicolas Morey-Chaisemartin wrote:
>
> > > And drop a symlink to fix it up.
>
> > Yes this should work. It feels a little bit dirty to have a tarball
> > name with unreplaced macro but it's a temporay state so I can live
> > with this :)
>
> It is a bit strange, but this trivially delegates parsing the .tar.xx
> to git archive which understands all the formats..
>
> Here is the patches, should I send them?
>
> https://github.com/jgunthorpe/rdma-plumbing/tree/cbuild

????

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement
       [not found]                                     ` <20170813063706.GQ24282-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2017-08-21  7:52                                       ` Nicolas Morey-Chaisemartin
       [not found]                                         ` <a17d1a6e-653a-0c8e-4b1b-d5ef2bf6d9b2-l3A5Bk7waGM@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Nicolas Morey-Chaisemartin @ 2017-08-21  7:52 UTC (permalink / raw)
  To: Leon Romanovsky, Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



Le 13/08/2017 à 08:37, Leon Romanovsky a écrit :
> On Wed, Aug 09, 2017 at 03:15:36PM -0600, Jason Gunthorpe wrote:
>> On Wed, Aug 09, 2017 at 11:08:34PM +0200, Nicolas Morey-Chaisemartin wrote:
>>
>>>> And drop a symlink to fix it up.
>>
>>> Yes this should work. It feels a little bit dirty to have a tarball
>>> name with unreplaced macro but it's a temporay state so I can live
>>> with this :)
>>
>> It is a bit strange, but this trivially delegates parsing the .tar.xx
>> to git archive which understands all the formats..
>>
>> Here is the patches, should I send them?
>>
>> https://github.com/jgunthorpe/rdma-plumbing/tree/cbuild
> 
> ????
> 
> Thanks
> 

Sorry for the late reply, was out of the office.
Yes your patches look good to me.

Thanks !

Nicolas

(Resent as previous one got dropped by majordomo due to HTML)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBAgAGBQJZmpFZAAoJEIAb3bglmI9kIxYH/3hfV0ep7UxlpU7uMay2OUAh
X1V9rwikyMoTkI9qDDmVDpBL0WpUYVSSkAZio00a24I2HUVa4+ZIj6d27EYZkRru
oFloEkfH+aER/KvVkfWjE8SYOzS31ed6cgDRlqqjEncehobYR630Q3g/zfRpml+V
aoJtMakrHrZBAcnqD2XVxieaD4KOjx4IaewrKrV+hLxLuw6UjZHtbaKAD/HHajWs
I1xUQTHaIMHO6Su982+pTDyY0zlcK2MtzsPK2gcxuyiGn3oHAd9k6xpfsxXiWa5M
6LV60V/O/6QXDu/CAok/DP+E+zg0RgoHoftVnkGkS4zR3b0r1RZDUuxb8XWloL0=
=0XFR
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement
       [not found]                                         ` <a17d1a6e-653a-0c8e-4b1b-d5ef2bf6d9b2-l3A5Bk7waGM@public.gmane.org>
@ 2017-08-21 15:42                                           ` Jason Gunthorpe
  0 siblings, 0 replies; 14+ messages in thread
From: Jason Gunthorpe @ 2017-08-21 15:42 UTC (permalink / raw)
  To: Nicolas Morey-Chaisemartin
  Cc: Leon Romanovsky, linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Mon, Aug 21, 2017 at 09:52:57AM +0200, Nicolas Morey-Chaisemartin wrote:

> Sorry for the late reply, was out of the office.
> Yes your patches look good to me.

https://github.com/linux-rdma/rdma-core/pull/186

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-08-21 15:42 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-04 16:57 [PATCH rdma-core 0/3] cbuild tweaks Nicolas Morey-Chaisemartin
     [not found] ` <27366048-d80f-3711-c6cd-93c87809650e-l3A5Bk7waGM@public.gmane.org>
2017-08-04 16:58   ` [PATCH rdma-core 1/3] cbuild: allow indentation in RPM version Nicolas Morey-Chaisemartin
2017-08-04 16:59   ` [PATCH rdma-core 2/3] cbuild: support .tar.(gz|bz|>...) extensions Nicolas Morey-Chaisemartin
2017-08-04 16:59   ` [PATCH rdma-core 3/3] cbuild: use rpmspec for macro replacement Nicolas Morey-Chaisemartin
     [not found]     ` <8ad0438f-b4a5-aec9-1e44-a9c9dc1ca17b-l3A5Bk7waGM@public.gmane.org>
2017-08-08 23:04       ` Jason Gunthorpe
     [not found]         ` <20170808230459.GF29372-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2017-08-09  9:56           ` Nicolas Morey-Chaisemartin
     [not found]             ` <b3993b0c-0b9d-a917-cbf8-22f68c1d2a97-l3A5Bk7waGM@public.gmane.org>
2017-08-09 15:55               ` Jason Gunthorpe
     [not found]                 ` <20170809155532.GC15586-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2017-08-09 16:39                   ` Nicolas Morey-Chaisemartin
     [not found]                     ` <f42a6eaa-79f9-1063-b58a-08da363d09e7-l3A5Bk7waGM@public.gmane.org>
2017-08-09 20:19                       ` Jason Gunthorpe
     [not found]                         ` <20170809201932.GA7363-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2017-08-09 21:08                           ` Nicolas Morey-Chaisemartin
     [not found]                             ` <5c4de783-ce62-2b8d-78aa-ee2f181fe719-l3A5Bk7waGM@public.gmane.org>
2017-08-09 21:15                               ` Jason Gunthorpe
     [not found]                                 ` <20170809211536.GA10191-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2017-08-13  6:37                                   ` Leon Romanovsky
     [not found]                                     ` <20170813063706.GQ24282-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2017-08-21  7:52                                       ` Nicolas Morey-Chaisemartin
     [not found]                                         ` <a17d1a6e-653a-0c8e-4b1b-d5ef2bf6d9b2-l3A5Bk7waGM@public.gmane.org>
2017-08-21 15:42                                           ` Jason Gunthorpe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.