Differential D162

Custom output directory of MashDirective
ClosedPublic
Actions

Authored by lbrabec on Jun 26 2014, 10:22 AM.

Details

Reviewers
tflink
mkrizek
jskladan
Maniphest Tasks
T162: Specify repodata output for mash directive
Summary

Method do_mash() of MashDirective now accepts outdir as its argument -- path to directory where repo is created, if not specified rpmdir is used as before.

Test Plan

$ py.test -F (with the new tests)

Diff Detail

Repository
rLTRN libtaskotron
Branch
feature/T162
Lint
No Linters Available
Unit
No Unit Test Coverage
lbrabec retitled this revision from to Custom output direcotry of MashDirective.Jun 26 2014, 10:22 AM
lbrabec updated this object.
lbrabec edited the test plan for this revision. (Show Details)
lbrabec added a reviewer: mkrizek.
lbrabec retitled this revision from Custom output direcotry of MashDirective to Custom output directory of MashDirective.Jun 26 2014, 11:33 AM
jskladan requested changes to this revision.Jun 26 2014, 12:45 PM

Other than the comments, it looks OK. I'd try to clean-up after the tests, but I'm not sure whether it is really necessary.

libtaskotron/directives/mash_directive.py
83

Please change the logic here, so the rpm files are not copied, but create symlinks instead. We already strive with disk space on the test slaves, so having two copies of the same files does not really make sense. I'm not sure, though, whether mash will work on just symlinks.

96

It might make more sense to return the "actuall" outdir ( (rpmdir if not outdir else outdir) ) here. And also then return it as a result of the process() method.

111

I know that dict.get() returns None on non-existing key, but I'd rather have it explicit: outdir = input_data.get('outdir', None)

This revision now requires changes to proceed.Jun 26 2014, 12:45 PM
lbrabec updated this revision to Diff 477.Jun 26 2014, 1:00 PM
  • custom output directory of mash directive (polished)
This revision is now accepted and ready to land.Jun 26 2014, 1:25 PM
lbrabec closed this revision.Jun 27 2014, 8:44 AM

Closed by commit 65b8128 (authored by @lbrabec).

Revision Contents

Diff 477

View Options

libtaskotron/directives/mash_directive.py

1# -*- coding: utf-8 -*- 1​# -*- coding: utf-8 -*-
2# Copyright 2009-2014, Red Hat, Inc. 2​# Copyright 2009-2014, Red Hat, Inc.
3# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+> 3​# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+>
4# See the LICENSE file for more details on Licensing 4​# See the LICENSE file for more details on Licensing
5 5​
6from __future__ import absolute_import 6​from __future__ import absolute_import
7import subprocess as sub 7​import subprocess as sub
8​import os
8import mash 9​import mash
9import mash.config 10​import mash.config
10import StringIO 11​import StringIO
11from ConfigParser import RawConfigParser 12​from ConfigParser import RawConfigParser
12 13​
13from libtaskotron.directives import BaseDirective 14​from libtaskotron.directives import BaseDirective
14from libtaskotron.logger import log 15​from libtaskotron.logger import log
15from libtaskotron.exceptions import TaskotronDirectiveError 16​from libtaskotron.exceptions import TaskotronDirectiveError
16 17​
17directive_class = 'MashDirective' 18​directive_class = 'MashDirective'
18 19​
19class MashDirective(BaseDirective): 20​class MashDirective(BaseDirective):
20 """This will create a YUM repository in ``rpmdir`` from all RPM packages 21​ """This will create a YUM repository in ``outdir`` from from all RPM
21 inside the same directory. It will call ``mash`` tool to perform this task. 22​ packages in ``rpmdir``. If ``outdir`` is not specified, repo is created
23​ in ``rpmdir``. It will call ``mash`` tool to perform this task.
22 24​
23 The directive returns None. 25​ The directive returns actual path where repo was created.
24 26​
25 For the mash directive, we're expecting a yaml declaration of the form: 27​ For the mash directive, we're expecting a yaml declaration of the form:
26 28​
27 mash: 29​ mash:
28 rpmdir=/path/to/some/dir 30​ rpmdir=/path/to/some/dir
29 dodelta=[True, False] 31​ dodelta=[True, False]
30 arch=["i386", "x86_64", "armhfp", "noarch"] 32​ arch=["i386", "x86_64", "armhfp", "noarch"]
31 """ 33​ """
32 34​
33 def do_mash(self, rpmdir, dodelta, arch): 35​ def do_mash(self, rpmdir, dodelta, arch, outdir=None):
34 """Set up a mash object with an ad-hoc config 36​ """Set up a mash object with an ad-hoc config
35 37​
36 :param str rpmdir: directory containing rpms to mash 38​ :param str rpmdir: directory containing rpms to mash
37 :param bool dodelta: create drpms during mash 39​ :param bool dodelta: create drpms during mash
38 :param str arch: arch of the rpms 40​ :param str arch: arch of the rpms
39 :returns: return code of doMultilib, 0 if it went fine, 1 othwerise 41​ :param str outdir: output directory where repo is created,
42​ if not specified or None, rpmdir is used
43​ :rtype str
44​ :returns: path to directory where repo was created
40 """ 45​ """
41 46​
42 # copied over from the old depcheck 47​ # copied over from the old depcheck
43 distname = 'depcheck' 48​ distname = 'depcheck'
44 ​
45 mash_conf_items = ["[%s]" % distname, 49​ mash_conf_items = ["[%s]" % distname,
46 "output_dir = ", 50​ "output_dir = ",
47 "output_subdir = ", 51​ "output_subdir = ",
48 "rpm_path = %s" % rpmdir, 52​ "rpm_path = %s" % (rpmdir if not outdir else outdir),
49 "strict_keys = False", 53​ "strict_keys = False",
50 "multilib = True", 54​ "multilib = True",
51 "multilib_method = devel", 55​ "multilib_method = devel",
52 "tag = this-is-a-dummy-tag", 56​ "tag = this-is-a-dummy-tag",
53 "inherit = False", 57​ "inherit = False",
54 "debuginfo = False", 58​ "debuginfo = False",
55 "delta = %s" % dodelta, 59​ "delta = %s" % dodelta,
56 "source = False", 60​ "source = False",
Show All 10 Lines
67 conf.distros.append(dist) 71​ conf.distros.append(dist)
68 dist.repodata_path = dist.rpm_path 72​ dist.repodata_path = dist.rpm_path
69 dist.name = distname 73​ dist.name = distname
70 74​
71 themash = mash.Mash(dist) 75​ themash = mash.Mash(dist)
72 themash.logger.handlers = [themash.logger.handlers[-1]] 76​ themash.logger.handlers = [themash.logger.handlers[-1]]
73 77​
74 log.info('running createrepo on %s', rpmdir) 78​ log.info('running createrepo on %s', rpmdir)
75 p = sub.Popen(['createrepo', rpmdir], stdout=sub.PIPE, stderr=sub.PIPE) 79​ if outdir:
80​ p = sub.Popen(['createrepo', '-o', outdir, rpmdir], stdout=sub.PIPE,
81​ stderr=sub.PIPE)
82​ for rpm in os.listdir(rpmdir):
83​ os.symlink(os.path.join(rpmdir, rpm), os.path.join(outdir, rpm))
jskladanUnsubmitted
Not Done

Please change the logic here, so the rpm files are not copied, but create symlinks instead. We already strive with disk space on the test slaves, so having two copies of the same files does not really make sense. I'm not sure, though, whether mash will work on just symlinks.

84​ else:
85​ p = sub.Popen(['createrepo', rpmdir], stdout=sub.PIPE, stderr=sub.PIPE)
76 86​
77 output, errors = p.communicate() 87​ output, errors = p.communicate()
78 88​
79 if p.returncode: 89​ if p.returncode:
80 raise TaskotronDirectiveError(errors) 90​ raise TaskotronDirectiveError(errors)
81 91​
82 log.info('running mash multilib on %s', rpmdir) 92​ log.info('running mash multilib on %s', rpmdir)
83 rc = themash.doMultilib() 93​ themash.doMultilib()
84 conf.distros.pop() 94​ conf.distros.pop()
85 95​
86 return rc 96​ return rpmdir if not outdir else outdir
jskladanUnsubmitted
Not Done

It might make more sense to return the "actuall" outdir ( (rpmdir if not outdir else outdir) ) here. And also then return it as a result of the process() method.

87 97​
88 def process(self, input_data, env_data): 98​ def process(self, input_data, env_data):
89 if 'rpmdir' not in input_data or 'arch' not in input_data: 99​ if 'rpmdir' not in input_data or 'arch' not in input_data:
90 detected_args = ', '.join(input_data.keys()) 100​ detected_args = ', '.join(input_data.keys())
91 raise TaskotronDirectiveError( 101​ raise TaskotronDirectiveError(
92 "The mash directive requires 'rpmdir' and 'arch' " 102​ "The mash directive requires 'rpmdir' and 'arch' "
93 "arguments. Detected arguments: %s" % detected_args) 103​ "arguments. Detected arguments: %s" % detected_args)
94 104​
95 if 'dodelta' not in input_data: 105​ if 'dodelta' not in input_data:
96 dodelta = False 106​ dodelta = False
97 else: 107​ else:
98 dodelta = input_data['dodelta'] 108​ dodelta = input_data['dodelta']
99 109​
100 rpmdir = input_data['rpmdir'] 110​ rpmdir = input_data['rpmdir']
111​ outdir = input_data.get('outdir', None)
jskladanUnsubmitted
Not Done

I know that dict.get() returns None on non-existing key, but I'd rather have it explicit: outdir = input_data.get('outdir', None)

101 arch = input_data['arch'] 112​ arch = input_data['arch']
102 113​
103 self.do_mash(rpmdir, dodelta, arch) 114​ return self.do_mash(rpmdir, dodelta, arch, outdir)
View Options

testing/test_mash_directive.py

Show First 20 Lines • Show All 67 Lines • ▼ Show 20 Line(s)63 def test_mash_arch_x86_64(self):
68 directive = mash_directive.MashDirective() 68​ directive = mash_directive.MashDirective()
69 before = os.listdir(self.rpmdir) 69​ before = os.listdir(self.rpmdir)
70 directive.process(input_data, {}) 70​ directive.process(input_data, {})
71 after = os.listdir(self.rpmdir) 71​ after = os.listdir(self.rpmdir)
72 72​
73 assert self.rpms[0] in before and self.rpms[0] in after and \ 73​ assert self.rpms[0] in before and self.rpms[0] in after and \
74 self.rpms[1] in before and self.rpms[1] not in after 74​ self.rpms[1] in before and self.rpms[1] not in after
75 75​
76​ def test_repodata_in_rpmdir(self):
77​ input_data = {
78​ 'rpmdir': self.rpmdir,
79​ 'arch': ['x86_64']
80​ }
81​ directive = mash_directive.MashDirective()
82​ directive.process(input_data, {})
83​ after = os.listdir(self.rpmdir)
84​
85​ assert 'repodata' in after
86​
87​ def test_repodata_in_outdir(self):
88​ outdir = mkdtemp()
89​ input_data = {
90​ 'rpmdir': self.rpmdir,
91​ 'outdir': outdir,
92​ 'arch': ['x86_64']
93​ }
94​ directive = mash_directive.MashDirective()
95​ directive.process(input_data, {})
96​ after = os.listdir(self.rpmdir)
97​
98​ assert 'repodata' not in after and 'repodata' in os.listdir(outdir)
99​