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
From: "Lars R. Damerow" lars@pixar.com
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)
Patch applied: https://git.fedorahosted.org/cgit/rpkg.git/commit/?id=028fd4565a0cf33a4adf47...
Thanks.
On 09/07/2014 05:25 AM, Lars R. Damerow wrote:
From: "Lars R. Damerow" lars@pixar.com
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)
From: "Lars R. Damerow" lars@pixar.com
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:
This fix was already there: https://git.fedorahosted.org/cgit/rpkg.git/commit/?id=0bc5050ae5644ddbf8765b...
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=2477ebd674301b8e604cd9...
On 09/07/2014 05:25 AM, Lars R. Damerow wrote:
From: "Lars R. Damerow" lars@pixar.com
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:
From: "Lars R. Damerow" lars@pixar.com
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
Applied: https://git.fedorahosted.org/cgit/rpkg.git/commit/?id=00eff15f1fddd0a516f4f6...
Thanks.
On 09/07/2014 05:25 AM, Lars R. Damerow wrote:
From: "Lars R. Damerow" lars@pixar.com
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.
From: "Lars R. Damerow" lars@pixar.com
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")
Applied: https://git.fedorahosted.org/cgit/rpkg.git/commit/?id=c0a4dd5fc303175755ba53...
Thanks.
On 09/07/2014 05:25 AM, Lars R. Damerow wrote:
From: "Lars R. Damerow" lars@pixar.com
This makes it easier to do unattended imports of a large number of srpms.
From: "Lars R. Damerow" lars@pixar.com
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
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@pixar.com
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?
From Mathieu Bridon bochecha@fedoraproject.org, 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@pixar.com
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
On 09/08/2014 08:21 PM, Lars R. Damerow wrote:
From Mathieu Bridon bochecha@fedoraproject.org, 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@pixar.com
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=a629482569d46d62441b6d...
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@pixar.com 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/buildsys
buildsys@lists.fedoraproject.org