miscellaneous patches for rpkg

[Lars R. Damerow]

Hi all,

I wrote a wrapper for rpkg to assist in our internal RPM builds, and while doing so I
fixed a few bugs and added some minor features. Might someone review them and consider
them for inclusion upstream?

thanks!
-lars

[PATCH 1/5] Make UPLOADEXTS a class variable that can be extended

[Lars R. Damerow]

From: "Lars R. Damerow" <lars(a)pixar.com&gt;

If sites want to accept other file formats into their src rpms, they can
subclass pyrpkg.Commands and add more values to Commands.UPLOADEXTS.
---
 src/pyrpkg/__init__.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
index 00bcfab..9260894 100644
--- a/src/pyrpkg/__init__.py
+++ b/src/pyrpkg/__init__.py
@@ -62,6 +62,11 @@ class Commands(object):
     by clients
     """
 
+    # This shouldn't change... often
+    UPLOADEXTS = ['tar', 'gz', 'bz2', 'lzma',
'xz', 'Z', 'zip', 'tff',
+                  'bin', 'tbz', 'tbz2', 'tgz',
'tlz', 'txz', 'pdf', 'rpm',
+                  'jar', 'war', 'db', 'cpio',
'jisp', 'egg', 'gem', 'spkg']
+
     def __init__(self, path, lookaside, lookasidehash, lookaside_cgi,
                  gitbaseurl, anongiturl, branchre, kojiconfig,
                  build_client, user=None, dist=None, target=None,
@@ -914,11 +919,6 @@ class Commands(object):
     def _srpmdetails(self, srpm):
         """Return a tuple of package name, package files, and upload
files."""
 
-        # This shouldn't change... often
-        UPLOADEXTS = ['tar', 'gz', 'bz2', 'lzma',
'xz', 'Z', 'zip', 'tff',
-                      'bin', 'tbz', 'tbz2', 'tgz',
'tlz', 'txz', 'pdf', 'rpm',
-                      'jar', 'war', 'db', 'cpio',
'jisp', 'egg', 'gem', 'spkg']
-
         # get the name
         cmd = ['rpm', '-qp', '--nosignature', '--qf',
'%{NAME}', srpm]
                 # Run the command
@@ -957,7 +957,7 @@ class Commands(object):
         contents = output.strip().split('\n')
         # Cycle through the stuff and sort correctly by its extension
         for file in contents:
-            if file.rsplit('.')[-1] in UPLOADEXTS:
+            if file.rsplit('.')[-1] in self.UPLOADEXTS:
                 uploadfiles.append(file)
             else:
                 files.append(file)
-- 
1.8.3.1

[PATCH 2/5] Fix file listing when running import in empty repo

[Lars R. Damerow]

From: "Lars R. Damerow" <lars(a)pixar.com&gt;

Before this change, the list of files generated for an empty repo would be
[''] instead of []. Now '' gets removed from the list and importing srpms
into
an empty repo works properly.
---
 src/pyrpkg/__init__.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
index 9260894..2908c22 100644
--- a/src/pyrpkg/__init__.py
+++ b/src/pyrpkg/__init__.py
@@ -1295,6 +1295,8 @@ class Commands(object):
 
         # Get a list of files we're currently tracking
         ourfiles = self.repo.git.ls_files().split('\n')
+        if ourfiles == ['']:
+            ourfiles = []
         # Trim out sources and .gitignore
         for file in ('.gitignore', 'sources', ''):
             try:
-- 
1.8.3.1

[PATCH 3/5] Properly remove possible .py when creating man pages

[Lars R. Damerow]

From: "Lars R. Damerow" <lars(a)pixar.com&gt;

Before this change, man page scripts that had 'p' or 'y' at the beginning
of
their names would have those trimmed off too (along with the .py suffix),
leading to breakage and sadness.
---
 src/pyrpkg/cli.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/pyrpkg/cli.py b/src/pyrpkg/cli.py
index cbe6ef2..9733a6b 100755
--- a/src/pyrpkg/cli.py
+++ b/src/pyrpkg/cli.py
@@ -1359,8 +1359,10 @@ Tasks still running. You can continue to watch with the '%s
watch-task' command.
 
         if  manpage:
             # Generate the man page
-            man_page = __import__('%s' %
-                                  self.name.strip('.py'))
+            man_name = self.name
+            if man_name.endswith('.py'):
+                man_name = man_name[:-3]
+            man_page = __import__('%s' % man_name)
             man_page.generate(self.parser, self.subparsers)
             sys.exit(0)
             # no return possible
-- 
1.8.3.1

[PATCH 4/5] Add skip-diffs option for import_srpms

[Lars R. Damerow]

From: "Lars R. Damerow" <lars(a)pixar.com&gt;

This makes it easier to do unattended imports of a large number of srpms.
---
 src/pyrpkg/cli.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/pyrpkg/cli.py b/src/pyrpkg/cli.py
index 9733a6b..80812e6 100755
--- a/src/pyrpkg/cli.py
+++ b/src/pyrpkg/cli.py
@@ -539,6 +539,10 @@ defined, packages will be built sequentially.""" %
         #import_srpm_parser.add_argument('--create', '-c',
         #                                help = 'Create a new local repo',
         #                                action = 'store_true')
+        import_srpm_parser.add_argument('--skip-diffs',
+                                        help = "Don't show diffs when \
+                                        import srpms",
+                                        action = 'store_true')
         import_srpm_parser.add_argument('srpm',
                                         help = 'Source rpm to import')
         import_srpm_parser.set_defaults(command = self.import_srpm)
@@ -1064,7 +1068,8 @@ defined, packages will be built sequentially.""" %
         uploadfiles = self.cmd.import_srpm(self.args.srpm)
         if uploadfiles:
             self.cmd.upload(uploadfiles, replace=True)
-        self.cmd.diff(cached=True)
+        if not self.args.skip_diffs:
+            self.cmd.diff(cached=True)
         self.log.info('--------------------------------------------')
         self.log.info("New content staged and new sources uploaded.")
         self.log.info("Commit if happy or revert with: git reset --hard HEAD")
-- 
1.8.3.1

[PATCH 5/5] put disttag in mockbuild results directory name

[Lars R. Damerow]

From: "Lars R. Damerow" <lars(a)pixar.com&gt;

This patch stops multiple mockbuild runs for different distributions
from clobbering each other.
---
 src/pyrpkg/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
index 2908c22..4bd5b86 100644
--- a/src/pyrpkg/__init__.py
+++ b/src/pyrpkg/__init__.py
@@ -2149,7 +2149,7 @@ class Commands(object):
                 cmd.extend(['--configdir', config_dir])
 
         cmd.extend(['-r', root, '--resultdir',
-                    os.path.join(self.path, "results_%s" % self.module_name,
+                    os.path.join(self.path, "results_%s_%s" %
(self.module_name, self.disttag),
                                  self.ver, self.rel),
                     '--rebuild', self.srpmname])
         # Run the command
-- 
1.8.3.1

[Stephen John Smoogen]

Hey Lars

Thanks for sending this. Two of the guys who are on the buildsys are out
with the flu so I don't know when they would be able to get to these, but I
didn't want to have this sit out in the ether til then.


On 6 September 2014 21:25, Lars R. Damerow <lars(a)pixar.com&gt; wrote:

>  Hi all,
>
>  I wrote a wrapper for rpkg to assist in our internal RPM builds, and while
>  doing so I fixed a few bugs and added some minor features. Might someone
>  review them and consider them for inclusion upstream?
>
>  thanks!
>  -lars
>
>  --
>  buildsys mailing list
>  buildsys(a)lists.fedoraproject.org
>  https://admin.fedoraproject.org/mailman/listinfo/buildsys 



-- 
Stephen J Smoogen.

[PATCH 5/5] put disttag in mockbuild results directory name

[Mathieu Bridon]

Note: I'm not an rpkg maintainer, just an occasional contributor. ;)


On Sat, 2014-09-06 at 20:25 -0700, Lars R. Damerow wrote:
>  From: "Lars R. Damerow" <lars(a)pixar.com&gt;
>  
>  This patch stops multiple mockbuild runs for different distributions
>  from clobbering each other.
>  ---
>   src/pyrpkg/__init__.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>  
>  diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
>  index 2908c22..4bd5b86 100644
>  --- a/src/pyrpkg/__init__.py
>  +++ b/src/pyrpkg/__init__.py
>  @@ -2149,7 +2149,7 @@ class Commands(object):
>                   cmd.extend(['--configdir', config_dir])
>   
>           cmd.extend(['-r', root, '--resultdir',
>  -                    os.path.join(self.path, "results_%s" % self.module_name,
>  +                    os.path.join(self.path, "results_%s_%s" %
> (self.module_name, self.disttag), 
Isn't the disttag already part of the results dir tree?

    results_%(name)s/%(version)s/%(release)s/

And the release includes the dist (well, unless it hasn't been set in
the spec file...)

This changes where the results are though (they aren't all in the same
results_%(name)s folder any more), which might break some expectations?

Maybe this specific change should really be in your rpkg-based tool?


-- 
Mathieu

[PATCH 5/5] put disttag in mockbuild results directory name

[Lars R. Damerow]

> From Mathieu Bridon <bochecha(a)fedoraproject.org&gt;, Mon, Sep 08,
> 2014 at 09:57:22AM +0200:
>  On Sat, 2014-09-06 at 20:25 -0700, Lars R. Damerow wrote:
>  > From: "Lars R. Damerow" <lars(a)pixar.com&gt;
>  > 
>  > This patch stops multiple mockbuild runs for different distributions
>  > from clobbering each other.
>  
>  Isn't the disttag already part of the results dir tree?
>  
>      results_%(name)s/%(version)s/%(release)s/
>  
>  And the release includes the dist (well, unless it hasn't been set in
>  the spec file...) 
We have lots of old packages that don't include the dist in the release.

>  This changes where the results are though (they aren't all in the
> same
>  results_%(name)s folder any more), which might break some expectations?
>  
>  Maybe this specific change should really be in your rpkg-based tool? 
That's fine with me. The only complication is that this line is in the middle of a
fair bit of code, so I'd have to replicate the whole function in my derived class. How
about I factor it into its own function call (Commands._set_result_dir()?) that can be
easily overridden?

thanks,
-lars

--
lars r. damerow :: button pusher :: pixar animation studios

[PATCH 1/5] Make UPLOADEXTS a class variable that can be extended

[Pavol Babincak]

Patch applied:
https://git.fedorahosted.org/cgit/rpkg.git/commit/?id=028fd4565a0cf33a4ad...

Thanks.


On 09/07/2014 05:25 AM, Lars R. Damerow wrote:
>  From: "Lars R. Damerow" <lars(a)pixar.com&gt;
>  
>  If sites want to accept other file formats into their src rpms, they can
>  subclass pyrpkg.Commands and add more values to Commands.UPLOADEXTS.
>  ---
>   src/pyrpkg/__init__.py | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>  
>  diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
>  index 00bcfab..9260894 100644
>  --- a/src/pyrpkg/__init__.py
>  +++ b/src/pyrpkg/__init__.py
>  @@ -62,6 +62,11 @@ class Commands(object):
>       by clients
>       """
>   
>  +    # This shouldn't change... often
>  +    UPLOADEXTS = ['tar', 'gz', 'bz2', 'lzma',
> 'xz', 'Z', 'zip', 'tff',
>  +                  'bin', 'tbz', 'tbz2', 'tgz',
> 'tlz', 'txz', 'pdf', 'rpm',
>  +                  'jar', 'war', 'db', 'cpio',
> 'jisp', 'egg', 'gem', 'spkg']
>  +
>       def __init__(self, path, lookaside, lookasidehash, lookaside_cgi,
>                    gitbaseurl, anongiturl, branchre, kojiconfig,
>                    build_client, user=None, dist=None, target=None,
>  @@ -914,11 +919,6 @@ class Commands(object):
>       def _srpmdetails(self, srpm):
>           """Return a tuple of package name, package files, and upload
> files."""
>   
>  -        # This shouldn't change... often
>  -        UPLOADEXTS = ['tar', 'gz', 'bz2', 'lzma',
> 'xz', 'Z', 'zip', 'tff',
>  -                      'bin', 'tbz', 'tbz2', 'tgz',
> 'tlz', 'txz', 'pdf', 'rpm',
>  -                      'jar', 'war', 'db', 'cpio',
> 'jisp', 'egg', 'gem', 'spkg']
>  -
>           # get the name
>           cmd = ['rpm', '-qp', '--nosignature', '--qf',
> '%{NAME}', srpm]
>                   # Run the command
>  @@ -957,7 +957,7 @@ class Commands(object):
>           contents = output.strip().split('\n')
>           # Cycle through the stuff and sort correctly by its extension
>           for file in contents:
>  -            if file.rsplit('.')[-1] in UPLOADEXTS:
>  +            if file.rsplit('.')[-1] in self.UPLOADEXTS:
>                   uploadfiles.append(file)
>               else:
>                   files.append(file)
>   

-- 
Pavol Babincak
Release Engineering, Red Hat

[PATCH 2/5] Fix file listing when running import in empty repo

[Pavol Babincak]

This fix was already there:
https://git.fedorahosted.org/cgit/rpkg.git/commit/?id=0bc5050ae5644ddbf87...

There wasn't any release with this fix which might be the reason why you
haven't noticed that. I haven't applied your patch but made that fix
more explicit:
https://git.fedorahosted.org/cgit/rpkg.git/commit/?id=2477ebd674301b8e604...

On 09/07/2014 05:25 AM, Lars R. Damerow wrote:
>  From: "Lars R. Damerow" <lars(a)pixar.com&gt;
>  
>  Before this change, the list of files generated for an empty repo would be
>  [''] instead of []. Now '' gets removed from the list and importing srpms
> into
>  an empty repo works properly.
>  ---
>   src/pyrpkg/__init__.py | 2 ++
>   1 file changed, 2 insertions(+)
>  
>  diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
>  index 9260894..2908c22 100644
>  --- a/src/pyrpkg/__init__.py
>  +++ b/src/pyrpkg/__init__.py
>  @@ -1295,6 +1295,8 @@ class Commands(object):
>   
>           # Get a list of files we're currently tracking
>           ourfiles = self.repo.git.ls_files().split('\n')
>  +        if ourfiles == ['']:
>  +            ourfiles = []
>           # Trim out sources and .gitignore
>           for file in ('.gitignore', 'sources', ''):
>               try:
>   

-- 
Pavol Babincak
Release Engineering, Red Hat

[PATCH 3/5] Properly remove possible .py when creating man pages

[Pavol Babincak]

Applied:
https://git.fedorahosted.org/cgit/rpkg.git/commit/?id=00eff15f1fddd0a516f...

Thanks.

On 09/07/2014 05:25 AM, Lars R. Damerow wrote:
>  From: "Lars R. Damerow" <lars(a)pixar.com&gt;
>  
>  Before this change, man page scripts that had 'p' or 'y' at the beginning
> of
>  their names would have those trimmed off too (along with the .py suffix),
>  leading to breakage and sadness. 
-- 
Pavol Babincak
Release Engineering, Red Hat

[PATCH 4/5] Add skip-diffs option for import_srpms

[Pavol Babincak]

Applied:
https://git.fedorahosted.org/cgit/rpkg.git/commit/?id=c0a4dd5fc303175755b...

Thanks.

On 09/07/2014 05:25 AM, Lars R. Damerow wrote:
>  From: "Lars R. Damerow" <lars(a)pixar.com&gt;
>  
>  This makes it easier to do unattended imports of a large number of srpms. 
-- 
Pavol Babincak
Release Engineering, Red Hat

[PATCH 5/5] put disttag in mockbuild results directory name

[Pavol Babincak]

On 09/08/2014 08:21 PM, Lars R. Damerow wrote:
>  From Mathieu Bridon <bochecha(a)fedoraproject.org&gt;, Mon, Sep 08,
> 2014
>  at 09:57:22AM +0200:
> > On Sat, 2014-09-06 at 20:25 -0700, Lars R. Damerow wrote:
> >> From: "Lars R. Damerow" <lars(a)pixar.com&gt;
> >> 
> >> This patch stops multiple mockbuild runs for different
> >> distributions from clobbering each other.
> > 
> > Isn't the disttag already part of the results dir tree?
> > 
> > results_%(name)s/%(version)s/%(release)s/
> > 
> > And the release includes the dist (well, unless it hasn't been set
> > in the spec file...)
>  
>  We have lots of old packages that don't include the dist in the
>  release.
>  
> > This changes where the results are though (they aren't all in the
> > same results_%(name)s folder any more), which might break some
> > expectations?
> > 
> > Maybe this specific change should really be in your rpkg-based
> > tool?
>  
>  That's fine with me. The only complication is that this line is in
>  the middle of a fair bit of code, so I'd have to replicate the whole
>  function in my derived class. How about I factor it into its own
>  function call (Commands._set_result_dir()?) that can be easily
>  overridden? 
I don't want to change this directory structure for every
pyrpkg's
clients (including fedpkg). I went ahead and refactored this directory
name to separate @property which you can easily override in your code:
https://git.fedorahosted.org/cgit/rpkg.git/commit/?id=a629482569d46d62441...
-- 
Pavol Babincak
Release Engineering, Red Hat