Merge lp:~elopio/snapcraft/fix1474022-lower_case into lp:~snappy-dev/snapcraft/core

[Leo Arias]

Refactored all methods to be lowercase, following pep8.

Still missing: lowercase attributes and arguments.

[Michael Terry (mterry)]

Missed one internal function I noticed, but otherwise seems fine, yeah. Thanks :)

[lp:~elopio/snapcraft/fix1474022-lower_case]

81. By Leo Arias on 2015-07-13

Refactored the internal wrap_bins.

[Leo Arias (elopio)]

> Missed one internal function I noticed, but otherwise seems fine, yeah.
> Thanks :)

done.

[Michael Terry (mterry)]

LGTM

review: Approve

=== modified file 'snapcraft/__init__.py'
--- snapcraft/__init__.py	2015-07-09 20:08:38 +0000
+++ snapcraft/__init__.py	2015-07-13 16:27:03 +0000
@@ -36,7 +36,7 @@
     def build(self):
         return True
 
-    def snapFiles(self):
+    def snap_files(self):
         """Returns two iteratables of globs:
             - the first is the set of files/dirs to include
             - the second is the set of files/dirs to exclude
@@ -57,20 +57,20 @@
                 print(cmd)
         return snapcraft.common.run(cmd, cwd=cwd, **kwargs)
 
-    def pullBzr(self, url):
+    def pull_bzr(self, url):
         if os.path.exists(os.path.join(self.sourcedir, ".bzr")):
             return self.run(['bzr', 'pull', url], cwd=self.sourcedir)
         else:
             os.rmdir(self.sourcedir)
             return self.run(['bzr', 'branch', url, self.sourcedir])
 
-    def pullGit(self, url):
+    def pull_git(self, url):
         if os.path.exists(os.path.join(self.sourcedir, ".git")):
             return self.run(['git', 'pull'], cwd=self.sourcedir)
         else:
             return self.run(['git', 'clone', url, '.'], cwd=self.sourcedir)
 
-    def pullBranch(self, url):
+    def pull_branch(self, url):
         branchType = None
         if url.startswith("bzr:") or url.startswith("lp:"):
             branchType = 'bzr'
@@ -87,12 +87,12 @@
             url = os.path.abspath(url)
 
         if branchType == 'bzr':
-            if not self.pullBzr(url):
+            if not self.pull_bzr(url):
                 return False
             if not self.run(['cp', '-Trfa', self.sourcedir, self.builddir]):
                 return False
         elif branchType == "git":
-            if not self.pullGit(url):
+            if not self.pull_git(url):
                 return False
             if not self.run(['cp', '-Trfa', self.sourcedir, self.builddir]):
                 return False

=== modified file 'snapcraft/cmds.py'
--- snapcraft/cmds.py	2015-07-08 20:45:52 +0000
+++ snapcraft/cmds.py	2015-07-13 16:27:03 +0000
@@ -31,7 +31,7 @@
         sys.exit(1)
     yaml = 'parts:\n'
     for partName in args.part:
-        part = snapcraft.plugin.loadPlugin(partName, partName, loadCode=False)
+        part = snapcraft.plugin.load_plugin(partName, partName, loadCode=False)
         yaml += '    ' + part.names()[0] + ':\n'
         for opt in part.config.get('options', []):
             if part.config['options'][opt].get('required', False):
@@ -47,7 +47,7 @@
 
 def shell(args):
     config = snapcraft.yaml.Config()
-    snapcraft.common.env = config.stageEnv()
+    snapcraft.common.env = config.stage_env()
     userCommand = args.userCommand
     if not userCommand:
         userCommand = "/usr/bin/env PS1='\[\e[1;32m\]snapcraft:\w\$\[\e[0m\] ' /bin/bash --norc"
@@ -64,10 +64,10 @@
         "cp -arv %s %s" % (config.data["snap"]["meta"], snapcraft.common.snapdir))
 
     # wrap all included commands
-    snapcraft.common.env = config.snapEnv()
-    script = "#!/bin/sh\n%s\nexec %%s $*" % snapcraft.common.assembleEnv().replace(snapcraft.common.snapdir, "$SNAP_APP_PATH")
+    snapcraft.common.env = config.snap_env()
+    script = "#!/bin/sh\n%s\nexec %%s $*" % snapcraft.common.assemble_env().replace(snapcraft.common.snapdir, "$SNAP_APP_PATH")
 
-    def wrapBins(bindir):
+    def wrap_bins(bindir):
         absbindir = os.path.join(snapcraft.common.snapdir, bindir)
         if not os.path.exists(absbindir):
             return
@@ -83,8 +83,8 @@
             with open(exePath, 'w+') as f:
                 f.write(script % ('"$SNAP_APP_PATH/' + bindir + '/' + exe + '.real"'))
             os.chmod(exePath, 0o755)
-    wrapBins('bin')
-    wrapBins('usr/bin')
+    wrap_bins('bin')
+    wrap_bins('usr/bin')
 
     snapcraft.common.run("snappy build " + snapcraft.common.snapdir)
 
@@ -134,7 +134,7 @@
     qemu.kill()
 
 
-def checkForCollisions(parts):
+def check_for_collisions(parts):
     partsFiles = {}
     for part in parts:
         # Gather our own files up
@@ -190,10 +190,10 @@
                 # allParts.  But then we need to make sure we continue to handle
                 # cases like go, where you want go built before trying to pull
                 # a go project.
-                if not checkForCollisions(config.allParts):
+                if not check_for_collisions(config.allParts):
                     sys.exit(1)
 
-            snapcraft.common.env = config.buildEnvForPart(part)
+            snapcraft.common.env = config.build_env_for_part(part)
             force = forceAll or cmd == forceCommand
             if not getattr(part, cmd)(force=force):
                 snapcraft.common.log("Failed doing %s for %s!" % (cmd, part.names()[0]))

=== modified file 'snapcraft/common.py'
--- snapcraft/common.py	2015-07-06 16:48:22 +0000
+++ snapcraft/common.py	2015-07-13 16:27:03 +0000
@@ -23,7 +23,7 @@
 env = []
 
 
-def assembleEnv():
+def assemble_env():
     return '\n'.join(['export ' + e for e in env])
 
 
@@ -32,7 +32,7 @@
     if isinstance(cmd, list):
         cmd = ' '.join(cmd)
     with tempfile.NamedTemporaryFile(mode='w+') as f:
-        f.write(assembleEnv())
+        f.write(assemble_env())
         f.write('\n')
         f.write('exec ' + cmd)
         f.flush()

=== modified file 'snapcraft/plugin.py'
--- snapcraft/plugin.py	2015-07-10 18:35:43 +0000
+++ snapcraft/plugin.py	2015-07-13 16:27:03 +0000
@@ -122,16 +122,16 @@
         except FileExistsError:
             pass
 
-    def isValid(self):
+    def is_valid(self):
         return self.valid
 
     def names(self):
         return self.partNames
 
-    def notifyStage(self, stage, hint=''):
+    def notify_stage(self, stage, hint=''):
         snapcraft.common.log(stage + " " + self.partNames[0] + hint)
 
-    def isDirty(self, stage):
+    def is_dirty(self, stage):
         try:
             with open(self.statefile, 'r') as f:
                 lastStep = f.read()
@@ -139,70 +139,70 @@
         except Exception:
             return True
 
-    def shouldStageRun(self, stage, force):
-        if not force and not self.isDirty(stage):
-            self.notifyStage('Skipping ' + stage, ' (already ran)')
+    def should_stage_run(self, stage, force):
+        if not force and not self.is_dirty(stage):
+            self.notify_stage('Skipping ' + stage, ' (already ran)')
             return False
         return True
 
-    def markDone(self, stage):
+    def mark_done(self, stage):
         with open(self.statefile, 'w+') as f:
             f.write(stage)
 
     def pull(self, force=False):
-        if not self.shouldStageRun('pull', force):
+        if not self.should_stage_run('pull', force):
             return True
         self.makedirs()
         if self.code and hasattr(self.code, 'pull'):
-            self.notifyStage("Pulling")
+            self.notify_stage("Pulling")
             if not getattr(self.code, 'pull')():
                 return False
-            self.markDone('pull')
+            self.mark_done('pull')
         return True
 
     def build(self, force=False):
-        if not self.shouldStageRun('build', force):
+        if not self.should_stage_run('build', force):
             return True
         self.makedirs()
         if self.code and hasattr(self.code, 'build'):
-            self.notifyStage("Building")
+            self.notify_stage("Building")
             if not getattr(self.code, 'build')():
                 return False
-            self.markDone('build')
+            self.mark_done('build')
         return True
 
     def stage(self, force=False):
-        if not self.shouldStageRun('stage', force):
+        if not self.should_stage_run('stage', force):
             return True
         self.makedirs()
         if not self.code:
             return True
 
-        self.notifyStage("Staging")
+        self.notify_stage("Staging")
         snapcraft.common.run(['cp', '-arT', self.installdir, self.stagedir])
-        self.markDone('stage')
+        self.mark_done('stage')
         return True
 
     def snap(self, force=False):
-        if not self.shouldStageRun('snap', force):
+        if not self.should_stage_run('snap', force):
             return True
         self.makedirs()
 
-        if self.code and hasattr(self.code, 'snapFiles'):
-            self.notifyStage("Snapping")
+        if self.code and hasattr(self.code, 'snap_files'):
+            self.notify_stage("Snapping")
 
-            includes, excludes = getattr(self.code, 'snapFiles')()
-            snapDirs, snapFiles = self.collectSnapFiles(includes, excludes)
+            includes, excludes = getattr(self.code, 'snap_files')()
+            snapDirs, snap_files = self.collect_snap_files(includes, excludes)
 
             if snapDirs:
                 snapcraft.common.run(['mkdir', '-p'] + list(snapDirs), cwd=self.stagedir)
-            if snapFiles:
-                snapcraft.common.run(['cp', '-a', '--parent'] + list(snapFiles) + [self.snapdir], cwd=self.stagedir)
+            if snap_files:
+                snapcraft.common.run(['cp', '-a', '--parent'] + list(snap_files) + [self.snapdir], cwd=self.stagedir)
 
-            self.markDone('snap')
+            self.mark_done('snap')
         return True
 
-    def collectSnapFiles(self, includes, excludes):
+    def collect_snap_files(self, includes, excludes):
         sourceFiles = set()
         for root, dirs, files in os.walk(self.installdir):
             sourceFiles |= set([os.path.join(root, d) for d in dirs])
@@ -232,15 +232,15 @@
         excludeFiles = set([os.path.relpath(x, self.stagedir) for x in excludeFiles])
 
         # And chop files, including whole trees if any dirs are mentioned
-        snapFiles = (includeFiles & sourceFiles) - excludeFiles
+        snap_files = (includeFiles & sourceFiles) - excludeFiles
         for excludeDir in excludeDirs:
-            snapFiles = set([x for x in snapFiles if not x.startswith(excludeDir + '/')])
+            snap_files = set([x for x in snap_files if not x.startswith(excludeDir + '/')])
 
         # Separate dirs from files
-        snapDirs = set([x for x in snapFiles if os.path.isdir(os.path.join(self.stagedir, x))])
-        snapFiles = snapFiles - snapDirs
+        snapDirs = set([x for x in snap_files if os.path.isdir(os.path.join(self.stagedir, x))])
+        snap_files = snap_files - snapDirs
 
-        return snapDirs, snapFiles
+        return snapDirs, snap_files
 
     def env(self, root):
         if self.code and hasattr(self.code, 'env'):
@@ -248,9 +248,9 @@
         return []
 
 
-def loadPlugin(partName, pluginName, properties={}, loadCode=True):
+def load_plugin(partName, pluginName, properties={}, loadCode=True):
     part = Plugin(pluginName, partName, properties, loadCode=loadCode)
-    if not part.isValid():
+    if not part.is_valid():
         snapcraft.common.log("Could not load part %s" % pluginName, file=sys.stderr)
         sys.exit(1)
     return part

=== modified file 'snapcraft/plugins/autotools_project.py'
--- snapcraft/plugins/autotools_project.py	2015-07-08 14:03:15 +0000
+++ snapcraft/plugins/autotools_project.py	2015-07-13 16:27:03 +0000
@@ -25,7 +25,7 @@
             self.options.configflags = ''
 
     def pull(self):
-        return self.pullBranch(self.options.source)
+        return self.pull_branch(self.options.source)
 
     def build(self):
         if not os.path.exists(os.path.join(self.builddir, "configure")):
@@ -35,5 +35,5 @@
             return False
         return self.run("make all") and self.run("make install DESTDIR=" + self.installdir)
 
-    def snapFiles(self):
+    def snap_files(self):
         return (['*'], ['include'])

=== modified file 'snapcraft/plugins/cmake_project.py'
--- snapcraft/plugins/cmake_project.py	2015-07-09 13:32:43 +0000
+++ snapcraft/plugins/cmake_project.py	2015-07-13 16:27:03 +0000
@@ -24,7 +24,7 @@
             self.options.configflags = ''
 
     def pull(self):
-        return self.pullBranch(self.options.source)
+        return self.pull_branch(self.options.source)
 
     def build(self):
         return self.run('cmake . -DCMAKE_INSTALL_PREFIX= %s' % self.options.configflags) and \

=== modified file 'snapcraft/plugins/go14.py'
--- snapcraft/plugins/go14.py	2015-07-08 14:03:15 +0000
+++ snapcraft/plugins/go14.py	2015-07-13 16:27:03 +0000
@@ -45,5 +45,5 @@
         self.makedirs(targetdir)
         return self.run(['tar', 'xf', tar_file], cwd=targetdir)
 
-    def snapFiles(self):
+    def snap_files(self):
         return ([], [])

=== modified file 'snapcraft/plugins/make_project.py'
--- snapcraft/plugins/make_project.py	2015-07-08 14:03:15 +0000
+++ snapcraft/plugins/make_project.py	2015-07-13 16:27:03 +0000
@@ -20,7 +20,7 @@
 class MakePlugin(snapcraft.BasePlugin):
 
     def pull(self):
-        return self.pullBranch(self.options.source)
+        return self.pull_branch(self.options.source)
 
     def build(self):
         return self.run("make") and self.run("make install DESTDIR=" + self.installdir)

=== modified file 'snapcraft/plugins/ubuntu.py'
--- snapcraft/plugins/ubuntu.py	2015-07-08 14:03:15 +0000
+++ snapcraft/plugins/ubuntu.py	2015-07-13 16:27:03 +0000
@@ -37,18 +37,18 @@
             self.includedPackages.append(name)
 
     def pull(self):
-        self.downloadablePackages = self.getAllDepPackages(self.includedPackages)
-        return self.downloadDebs(self.downloadablePackages)
+        self.downloadablePackages = self.get_all_dep_packages(self.includedPackages)
+        return self.download_debs(self.downloadablePackages)
 
     def build(self):
         if not self.downloadablePackages:
-            self.downloadablePackages = self.getAllDepPackages(self.includedPackages)
-        return self.unpackDebs(self.downloadablePackages, self.installdir)
+            self.downloadablePackages = self.get_all_dep_packages(self.includedPackages)
+        return self.unpack_debs(self.downloadablePackages, self.installdir)
 
-    def snapFiles(self):
+    def snap_files(self):
         return (['*'], ['/usr/include', '/lib/*/*.a', '/usr/lib/*/*.a'])
 
-    def getAllDepPackages(self, packages):
+    def get_all_dep_packages(self, packages):
         cache = apt.Cache()
         alldeps = set()
         manifestdeps = set()
@@ -60,7 +60,7 @@
                 if pkg in cache:
                     manifestdeps.add(pkg)
 
-        def addDeps(pkgs):
+        def add_deps(pkgs):
             for p in pkgs:
                 if p in alldeps:
                     continue
@@ -72,11 +72,11 @@
                     candidatePkg = cache[p].candidate
                     deps = candidatePkg.dependencies + candidatePkg.recommends
                     alldeps.add(p)
-                    addDeps([x[0].name for x in deps])
+                    add_deps([x[0].name for x in deps])
                 except:
                     pass
 
-        addDeps(packages)
+        add_deps(packages)
 
         exit = False
         for p in packages:
@@ -88,14 +88,14 @@
 
         return sorted(alldeps)
 
-    def downloadDebs(self, pkgs, debdir=None):
+    def download_debs(self, pkgs, debdir=None):
         debdir = debdir or self.builddir
         if pkgs:
             return self.run(['dget'] + pkgs, cwd=debdir, stdout=subprocess.DEVNULL)
         else:
             return True
 
-    def unpackDebs(self, pkgs, targetDir, debdir=None):
+    def unpack_debs(self, pkgs, targetDir, debdir=None):
         debdir = debdir or self.builddir
         for p in pkgs:
             if not self.run(['dpkg-deb', '--extract', p + '_*.deb', targetDir], cwd=debdir):

=== modified file 'snapcraft/yaml.py'
--- snapcraft/yaml.py	2015-07-10 18:35:43 +0000
+++ snapcraft/yaml.py	2015-07-13 16:27:03 +0000
@@ -47,7 +47,7 @@
 
             # TODO: support 'filter' or 'blacklist' field to filter what gets put in snap/
 
-            self.loadPlugin(partName, pluginName, properties)
+            self.load_plugin(partName, pluginName, properties)
 
         localPlugins = set()
         for part in self.allParts:
@@ -68,7 +68,7 @@
                         alreadyPresent = True
                         break
                 if not alreadyPresent:
-                    newParts.append(self.loadPlugin(requiredPart, requiredPart, {}))
+                    newParts.append(self.load_plugin(requiredPart, requiredPart, {}))
 
         # Gather lists of dependencies
         for part in self.allParts:
@@ -104,55 +104,55 @@
             self.allParts.remove(topPart)
         self.allParts = sortedParts
 
-    def loadPlugin(self, partName, pluginName, properties, loadCode=True):
-        part = snapcraft.plugin.loadPlugin(partName, pluginName, properties, loadCode=loadCode)
+    def load_plugin(self, partName, pluginName, properties, loadCode=True):
+        part = snapcraft.plugin.load_plugin(partName, pluginName, properties, loadCode=loadCode)
 
         self.systemPackages += part.config.get('systemPackages', [])
         self.allParts.append(part)
         return part
 
-    def runtimeEnv(self, root):
+    def runtime_env(self, root):
         env = []
         env.append("PATH=\"%s/bin:%s/usr/bin:$PATH\"" % (root, root))
         env.append("LD_LIBRARY_PATH=\"%s/lib:%s/usr/lib:$LD_LIBRARY_PATH\"" % (root, root))
         return env
 
-    def buildEnv(self, root):
+    def build_env(self, root):
         env = []
         env.append("CFLAGS=\"-I%s/include $CFLAGS\"" % root)
         env.append("LDFLAGS=\"-L%s/lib -L%s/usr/lib $LDFLAGS\"" % (root, root))
         return env
 
-    def buildEnvForPart(self, part):
+    def build_env_for_part(self, part):
         # Grab build env of all part's dependencies
 
         env = []
 
         for dep in part.deps:
             root = dep.installdir
-            env += self.runtimeEnv(root)
-            env += self.buildEnv(root)
+            env += self.runtime_env(root)
+            env += self.build_env(root)
             env += dep.env(root)
-            env += self.buildEnvForPart(dep)
+            env += self.build_env_for_part(dep)
 
         return env
 
-    def stageEnv(self):
+    def stage_env(self):
         root = snapcraft.common.stagedir
         env = []
 
-        env += self.runtimeEnv(root)
-        env += self.buildEnv(root)
+        env += self.runtime_env(root)
+        env += self.build_env(root)
         for part in self.allParts:
             env += part.env(root)
 
         return env
 
-    def snapEnv(self):
+    def snap_env(self):
         root = snapcraft.common.snapdir
         env = []
 
-        env += self.runtimeEnv(root)
+        env += self.runtime_env(root)
         for part in self.allParts:
             env += part.env(root)
 

=== modified file 'tests/unit/test_cmds.py'
--- tests/unit/test_cmds.py	2015-07-08 18:21:52 +0000
+++ tests/unit/test_cmds.py	2015-07-13 16:27:03 +0000
@@ -19,13 +19,13 @@
 import unittest
 from unittest import mock
 
-from snapcraft.cmds import checkForCollisions
+from snapcraft.cmds import check_for_collisions
 
 
 class TestCommands(unittest.TestCase):
 
     @mock.patch('snapcraft.common.log')
-    def test_checkForCollisions(self, logmock):
+    def test_check_for_collisions(self, logmock):
         tmpdirObject = tempfile.TemporaryDirectory()
         self.addCleanup(tmpdirObject.cleanup)
         tmpdir = tmpdirObject.name
@@ -52,8 +52,8 @@
         open(part3.installdir + '/1', mode='w').close()
         open(part3.installdir + '/a/2', mode='w').close()
 
-        self.assertTrue(checkForCollisions([part1, part2]))
+        self.assertTrue(check_for_collisions([part1, part2]))
         self.assertFalse(logmock.called)
 
-        self.assertFalse(checkForCollisions([part1, part2, part3]))
+        self.assertFalse(check_for_collisions([part1, part2, part3]))
         logmock.assert_called_with("Error: parts part2 and part3 have the following files in common:\n  1\n  a/2")

=== modified file 'tests/unit/test_plugin.py'
--- tests/unit/test_plugin.py	2015-07-08 14:03:15 +0000
+++ tests/unit/test_plugin.py	2015-07-13 16:27:03 +0000
@@ -24,7 +24,7 @@
 
 class TestPlugin(unittest.TestCase):
 
-    def test_isDirty(self):
+    def test_is_dirty(self):
         p = Plugin("mock", "mock-part", {}, loadConfig=False)
         p.statefile = tempfile.NamedTemporaryFile().name
         self.addCleanup(os.remove, p.statefile)
@@ -37,7 +37,7 @@
         p.pull()
         self.assertFalse(p.code.pull.called)
 
-    def test_collectSnapFiles(self):
+    def test_collect_snap_files(self):
         p = Plugin("mock", "mock-part", {}, loadConfig=False)
 
         tmpdirObject = tempfile.TemporaryDirectory()
@@ -65,22 +65,22 @@
         open(tmpdir + '/stage/2/2b/a', mode='w').close()
         open(tmpdir + '/stage/3/a', mode='w').close()
 
-        self.assertEqual(p.collectSnapFiles([], []), (set(), set()))
+        self.assertEqual(p.collect_snap_files([], []), (set(), set()))
 
-        self.assertEqual(p.collectSnapFiles(['*'], []), (
+        self.assertEqual(p.collect_snap_files(['*'], []), (
             set(['1', '1/1a', '1/1a/1b', '2', '2/2a', '3']),
             set(['a', 'b', '1/a', '3/a'])))
 
-        self.assertEqual(p.collectSnapFiles(['*'], ['1']), (
+        self.assertEqual(p.collect_snap_files(['*'], ['1']), (
             set(['2', '2/2a', '3']),
             set(['a', 'b', '3/a'])))
 
-        self.assertEqual(p.collectSnapFiles(['a'], ['*']), (set(), set()))
+        self.assertEqual(p.collect_snap_files(['a'], ['*']), (set(), set()))
 
-        self.assertEqual(p.collectSnapFiles(['*'], ['*/*']), (
+        self.assertEqual(p.collect_snap_files(['*'], ['*/*']), (
             set(['1', '2', '3']),
             set(['a', 'b'])))
 
-        self.assertEqual(p.collectSnapFiles(['1', '2'], ['*/a']), (
+        self.assertEqual(p.collect_snap_files(['1', '2'], ['*/a']), (
             set(['1', '1/1a', '1/1a/1b', '2', '2/2a']),
             set()))