Package management system

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:

• package.py – a class that describes information about a package such as its name, its version, what its dependencies are, etcetera

• 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]

Possible Problems

-
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.

-
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.

-
verbosity and io_output can be replaced with logging.

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

-
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.

-
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.

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

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

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

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

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

-
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, ...

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

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

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

-
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.

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

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

-
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.

-
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.

-
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:
        ...

StackExchange Code Review Q#111335, answer score: 4