Package management system

Question

A short time ago, I discovered the LinuxFromScratch project. After getting a system up and working (after much struggling), I realized that if I wanted to continue using LFS, some sort of package management would be quite nice. Of course I could have installed Pacman, apt-get, rpm, etc. like any sane person, perhaps. Instead, I was interested in creating my own simple 'package management' system that would keep track of files that belonged to a certain package, etcetera.

I have attached several files, two of which I think are particularly in need of review:

1. package.py – a class that describes information about a package such as its name, its version, what its dependencies are, etcetera
2. fakeroot.py – this file is in charge of installing all of a package's files to the filesystem from a fakeroot, adding records of the installed files to a table in a database called Files, etcetera

package.py:

import io_crate, os.path, sqlite3, core_regex, datetime, io_output

class Package:
    crate_extension = '.crate'
    database_location = 'proto.db'

    def __init__(self, name, verbosity = 0, fp = '/home/duncan/Documents/fakeroot', rp = '/home/duncan/Documents/install', ap = '/usr/src/archive/', cp = '/home/duncan/Documents/package/'):
        # Setup database stuff
        self.connection = sqlite3.connect(self.database_location)
        self.connection.text_factory = str
        self.db_cursor = self.connection.cursor()

        # Setup path and name
        self.name = name
        self.fakeroot_path = os.path.join(fp, self.name)
        self.root = rp
        self.archive_path = ap
        self.crate_path = os.path.join(cp, self.name) + self.crate_extension

        # Setup description taken from .crate file
        crate_contents = io_crate.read_crate(self.crate_path)
        self.description = crate_contents[0][1]
        self.homepage = crate_contents[1][1]
        self.optional_deps = crate_contents[2][1]
        self.recommended_deps = crate_contents[3][1]
        self.required_deps = crate_contents[4][1]

        self.verbosity = verbosity

    def add_to_db(self):
        """Adds self.name to the package database."""
        if self.is_in_db():
            return 0
        else:
            # no need to try..except this because is_in_db.
            self.db_cursor.execute('INSERT INTO Packages VALUES(?, ?);', (self.name, datetime.datetime.today()))
            io_output.vbprint(self.verbosity, '{} added to the databased'.format(self.name))
            return 1

    def remove_from_db(self):
        """Removes self from the database of packages."""
        if self.is_in_db():
            self.db_cursor.execute('DELETE FROM Packages WHERE Package=?;', (self.name,))
            io_output.vbprint(self.verbosity, '{} removed from database'.format(self.name))
            return 1
        return 0

    def is_in_db(self):
        """Checks if the name of self is contained in the packages database."""
        try:
            self.db_cursor.execute('SELECT * FROM Packages WHERE Package=?;', (self.name,))
        except:
            print 'Couldn\'t read the database at {}'.format(self.database_location)

        if not self.db_cursor.fetchone():
            return 0
        return 1

    def __del__(self):
        self.connection.commit()
        self.connection.close()

fakeroot.py:

import os, md5_gen, shutil, io_output

class Fakeroot():
    def __init__(self, package):
        self.dirs = [] # A list based on the files in the fakeroot.
        self.files = [] # A list of all the directories to be created in package.root.
        self.links = [] # A list of links from the fakeroot
        self.package = package

        for root, dirs, files in os.walk(package.fakeroot_path):
            for f in files:
                new_dir = os.path.normpath(os.path.join(package.root, root[len(package.fakeroot_path) + 1:len(root)]))
                src = os.path.join(root, f)
                dest = os.path.join(new_dir, f) 

                if (os.path.islink(src)):
                    self.links.append([root, new_dir, f])
                else:
                    self.files.append([src, dest])
            for d in dirs:
                self.dirs.append(os.path.join(package.root, root[len(package.fakeroot_path) + 1: len(root)], d))

    def create_dirs(self):
    # Go through self.dirs and check to see if a directory exists. If it does not, create it.
        for d in self.dirs:
            if not os.path.exists(d): # If the directory does not exist, run the equivalent of
                os.makedirs(d) # mkdir -p on it.
                io_output.vbprint(self.package.verbosity, 'DD {}'.format(d))
            else:
                io_output.vbprint(self.package.verbosity, 'UU {}'.format(d))
                continue

    def remove_dirs(self):
        for d in reversed(self.dirs): # remove the directories that are the highest in the tree first.
            if os.path.isdir(d): # If the directory exists
                if not os.listdir(d): # and is empty...
                    os.rmdir(d) # remove it.
                    io_output.vbprint(self.package.verbosity, '-- {}'.format(d))
                else: # If it is not empty.
                    io_output.vbprint(self.package.verbosity, 'UU {}'.format(d))
            else: # If it does not exist.
                io_output.vbprint(self.package.verbosity, '?? {}'.format(d))

    def copy_files(self):
        for f in self.files:
            if os.path.exists(f[1]): # If the file exists, show that it is being used.
                print 'Overwrite {}???'.format(f[1])
                # TODO
                # TODO: "Code" for overwiting stuff goes here.
                # TODO: If yes, copy the file and add it to the DB.
                # TODO: Perhaps an option to overwrite all files could be useful?
                # TODO
                continue
            else: # If it does not exist,
                try: # try...
                    shutil.copy2(f[0], f[1]) # copying it!
                    self.add_to_db(f)
                    io_output.vbprint(self.package.verbosity, '++ {}'.format(f[1]))
                except:
                    io_output.vbprint(self.package.verbosity, 'Failed to copy a file...rolling back changes.')
                    #self.remove_files()
                    break

    def remove_files(self):
        for f in self.files:
            if os.path.exists(f[1]):
                os.remove(f[1])
                self.remove_from_db(f)
                io_output.vbprint(self.package.verbosity, '-- {}'.format(f[1]))
            else:
                io_output.vbprint(self.package.verbosity, '?? {}'.format(f[1]))

    def create_links(self):
        for l in self.links:
            try:
                if not os.path.exists(os.path.join(l[1], l[2])):
                    linkto = os.path.join(l[1], os.readlink(os.path.join(l[0], l[2])))
                    os.symlink(linkto, os.path.join(l[1], l[2]))
                    io_output.vbprint(self.package.verbosity, 'LL {}'.format(l[2]))
                else:
                    print 'Overwrite existing link {}??'.format(l[2])
                    # TODO: See above todo for more info. ^^^^
            except:
                print 'Couldn\'t find the specified fakeroot!'
                break

    def remove_links(self):
        for l in self.links:
            try:
                os.remove(os.path.join(l[1], l[2]))
                io_output.vbprint(self.package.verbosity, '-- {}.'.format(l))
            except:
                raise OSError('\nFailed to remove the link `{}`'.format(os.path.join(l[1], l[2])))

    def add_to_db(self, f):
        """Returns 0 if the db can't be read, 1 if the file is added."""    
        try:
            self.package.db_cursor.execute('INSERT INTO Files VALUES(?, ?, ?, ?);', (f[1], md5_gen.generate(f[1]), 
                                                    os.path.getsize(f[1]),
                                                    self.package.name))
            return 1
        except:
            return 0

    def remove_from_db(self, f):
        self.package.db_cursor.execute('DELETE FROM Files WHERE Path=?;', (f[1],))

In regard to these two files, I feel that my design is somewhat crappy. In particular, the design of the Package and Fakeroot objects seems bad. Additionally, the many similar methods in fakeroot.py for copying files / removing files seem somewhat redundant. What do you think?

io_output.py:

def vbprint(verb_l, message):
    """Take a level of verboseness, `verb_l`, and a message to be printed. If verb_l > 0, the message is printed."""
    if verb_l != 0:
        print message

md5_gen.py (the idea for this code was found by searching Google for something like "md5 python"):

import hashlib

def generate(filename):
    """Return the md5 hash of filename.

    Keyword arguments:
    filename -- The path and name of the file to calculate the SHA1 hash of
    """
    # Returns the md5 hash of the given file.
    md5 = hashlib.md5()

    with open(filename, 'rb') as file:
        while True:
            block = file.read(2**10)
            if not block:
                break
            md5.update(block)
        return md5.hexdigest()

io_crate.py -- used to get information from files about packages:

from re import split

def read_crate(package):
    """Opens a crate file, reads the variables, and returns them as a sorted list of tuples.

    Keyword arguments:
    package -- A Package object, as defined in package.py
    """
    # Returns 0 if the crate file cannot be read, 1 on success.
    try:
        crate_text = open(package).read()
    except IOError:
        print 'Failed to read the crate file {}.'.format(package.crate_path)
        return 0

    processed = split("=.*?\"|\".*?\n", crate_text)

    lastline = ""
    final = []

    for line in processed:
        if lastline.isupper():
            if '_DEP' in lastline:
                final.append((lastline, split(' ', line)))
            else:
                final.append((lastline, line))
        lastline = line
    return sorted(final, key=lambda pos: pos[0])

Sample .crate file:

DESCRIPTION="The most foo of all bars."
HOMEPAGE="http://www.foo.it/"   
REQUIRED_DEP="foobar-11-2"
RECOMMENDED_DEP="foobus-1.7"    
OPTIONAL_DEP="foon-7.6a"

Answer 1

Possible Problems

1. Don't use exact paths. I do not have a user called 'duncan', nor do I wish to. Instead use ~/Documents/fakeroot.

   However, I would argue that this is the point of /tmp, as you should want to purge the files from the system afterwards.

2. My Arch install doesn't come with SQL preinstalled, so I highly doubt LFS would (Sure you can install it but that's what your package manager is meant to do). This is poor design for a package manager IMO.

3. verbosity and io_output can be replaced with logging.

4. io_crate should be added to package.py rather than be an import.

5. except should always have what you are guarding against! If you ^C out of the program, while it's in the try, it will continue execution. And print 'Couldn't read the database at ...'.

   And it will prevent sys.exit.

6. read_crate shouldn't suppress the IOError. All that achieves is an index error when you expect an array as output. And a confusing trace-back.

7. You should always close a file. A simple way to do this is with with.

   with open(package) as f:
       f.read()
   

8. You should read *.crate line by line to make processing easier.

   with open(package) as f:
       for line in f:
           ...
   

9. You should use the correct apostrophes for the correct content. '=.*?"|".*?\n' is much easier to read.

10. You should change how for line in processed works. It makes no sense, as you don't go line by line, you go keyword, value, keyword, value, ...

11. You should return a dictionary from read_crate, as then it is more reliable and extendable.

12. md5_sum.py should be added to fakeroot.py. It's also debatable if package.py should too. It's a small program...

13. md5_sum.py seems pretty good. But md5 is not sha1.

14. I dislike your Fakeroot It might just be me, but self.dirs is horrible memory hungry. Each index will have package.root + root[...] and then it's name.

    I would just use a generator.

15. Let's not do os.path.join(package.root, root[...]) on every file. Just store it in a variable.

16. If you make a generator for self.dirs, you won't need to use reversed. Instead pass topdown=False to os.walk.

17. As you don't say how you use the program, I'm assuming that you do something like:

    root = Fakeroot(...)
    root.create_dirs()
    root.copy_files()
    root.create_links()
    ...
    root.remove_links()
    root.remove_files()
    root.remove_files()
    

    Instead I would recommend that you make a purge function. And do the 'create' stuff in the __init__, or another single function.

18. Your comments are mostly pointless. # If it does not exist, and try: # try... are just bad. If you asked any programmer, and quite a few non-programmers, they would understand if not os.path.exists() means the path does indeed not exist.

19. It was hard to read you code, limit your lines to 79 characters, for best support. As shown, you need to scroll here on CR, and some code went out of my editor...

---

Extra points

Simplify read_crate

def read_crate(path):
    with open(path) as f:
        return {
            key: val
            for key, val in (
                line.split('=', 1)
                for line in f
            )
        }

This leads to the simple usage in Package.

self.crate = io_crate.read_crate(self.crate_path)
self.description = self.crate['DESCRIPTION']

Making purge.

This should be a simple os.walk in reverse. You shouldn't need to make more functions, as they'll be 1/2 liners. And would be more read-able in the algorithm.

def purge(self):
    for root, dirs, files in os.walk(package.root, topdown=False):
        for name in files:
            path = os.path.join(root, name)
            try:
                os.remove(path)
            except ...:
                logging.warn('-- ' + path)
            else:
                logging.info('-- ' + path)
                if not os.path.islink(src):
                    self.remove_from_db(path)
        for name in dirs:
            path = os.path.join(root, name)
            try:
                os.rmdir(path)
            except ...:
                logging.warn('-- ' + path)
            else:
                logging.info('-- ' + path)

Note the use of the python's logger! It's a nice tool. You can even use it to save the logs to files!

Copying the tree

This is the opposite of purge, and so, I'd recommend you just do that. But the loop should be have package.root + root[...] in the first for. Like:

fakeroot_len = len(package.fakeroot_path) + 1
for root, dirs, files in os.walk(package.fakeroot_path):
    fakeroot = os.path.join(package.root, root[fakeroot_len:len(root)])
    for name in files:
        ...
    for name in dirs:
        ...