10
\$\begingroup\$

I just finished writing a simple version control. You register files using "add", and then "com" will save them inside a directory, with an ID attached to it, same ID for all files. With "rev" it will copy back the content of the files, but won't delete any file other than the one in the "commitdir".

package main

import (
    "encoding/gob"
    "errors"
    "fmt"
    //"github.com/davecgh/go-spew/spew"
    //"io"
    "io/ioutil"
    "log"
    "os"
    "strconv"
)

type DB struct {
    filename   string
    dirname    string
    CommitList map[string]bool
    ID         int
    file       *os.File
}

func LoadDB(filename, dirname string) (*DB, error) {
    er := ger("Loading DB")
    file, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE, 0666)
    if err != nil {
        return nil, er("Opening file", err)
    }
    dec := gob.NewDecoder(file)
    db := &DB{}
    stats, err := file.Stat()
    if err != nil {
        return nil, er("Getting file stats", err)
    }

    if stats.Size() > 0 {
        err = dec.Decode(db)
        if err != nil {
            return nil, er("Decoding non empty db", err)
        }

    } else {
        db = &DB{}
        db.CommitList = make(map[string]bool)
    }
    db.filename = filename
    db.dirname = dirname
    db.file = file
    return db, nil
}

func (db *DB) Write() error {
    er := ger("Writing DB")
    _, err := db.file.Seek(0, 0)
    if err != nil {
        return er("File seek", err)
    }
    enc := gob.NewEncoder(db.file)
    err = enc.Encode(db)
    if err != nil {
        return er("Encoding", err)
    }
    err = db.file.Close()
    if err != nil {
        return er("Closing file", err)
    }
    return nil
}

func (db *DB) Add(name string) error {
    if _, ok := db.CommitList[name]; ok {
        return fmt.Errorf("%s already in the list", name)
    }
    db.CommitList[name] = true
    return nil
}

func (db *DB) Rem(name string) error {
    if _, ok := db.CommitList[name]; !ok {
        return fmt.Errorf("%s wasn't in the list", name)
    }
    delete(db.CommitList, name)
    return nil
}
func (db *DB) Com() error {
    er := ger("")
    err := os.MkdirAll(db.dirname, os.ModeDir|0777)
    db.ID++
    if err != nil {
        return er("Mkdir", err)
    }
    for name := range db.CommitList {
        filea, err := os.Open(name)
        if err != nil {
            return er("Opening committed file", err)
        }
        defer filea.Close()

        content, err := ioutil.ReadAll(filea)
        if err != nil {
            return er("Reading committed file", err)
        }

        filename := db.name(name, db.ID)
        fileb, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666)
        if err != nil {
            return err
        }
        defer fileb.Close()
        _, err = fileb.Write(content)
        if err != nil {
            return err
        }
        filea.Close()
        fileb.Close()
    }
    return nil
}

func (db *DB) name(filename string, id int) string {
    return fmt.Sprintf("%s/%s_%d.min", db.dirname, filename, id)
}

func (db *DB) Rev(id int) error {
    er := ger("")
    if len(db.CommitList) == 0 {
        return er("Nothing yet", nil)
    }
    for name := range db.CommitList {
        filea, err := os.Open(db.name(name, id))
        if err != nil {
            continue
        }
        defer filea.Close()
        for i := id + 1; i <= db.ID; i++ {
            fmt.Println("deleting", db.name(name, i))
            err := os.Remove(db.name(name, i))
            if err != nil {
                fmt.Println(err)
            }
        }

        content, err := ioutil.ReadAll(filea)
        if err != nil {
            return er("Reading committed file", err)
        }

        fileb, err := os.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
        if err != nil {
            return err
        }
        defer fileb.Close()
        _, err = fileb.Write(content)
        if err != nil {
            return err
        }
    }
    db.ID -= id
    return nil
}

func (db *DB) List() error {
    i := 0
    fmt.Println("Last rev :", db.ID)
    for name := range db.CommitList {
        fmt.Printf("%d - %s\n", i, name)
        i++
    }
    return nil
}

var missArg error = errors.New("Wrong number of argument")
var wrongCmd error = errors.New("Wrong command")

func HandleCmd(db *DB) error {
    er := ger("Handling cmd")
    numArgs := len(os.Args) - 1
    cmd := os.Args[1]
    var name string
    if numArgs == 2 {
        name = os.Args[2]
    }

    switch cmd {
    case "add":
        if numArgs != 2 {
            return missArg
        }
        err := db.Add(name)
        if err != nil {
            return er("Add", err)
        }
        return db.Write()
    case "rem":
        if numArgs != 2 {
            return missArg

        }
        err := db.Rem(name)
        if err != nil {
            return er("Rem", err)
        }

        return db.Write()
    case "com":

        err := db.Com()
        if err != nil {
            return er("Com", err)
        }

        return db.Write()
    case "rev":
        id, err := strconv.Atoi(name)
        if err != nil {
            return er("Rev", err)
        }
        return db.Rev(id)
    case "list":
        return db.List()
    default:
        return wrongCmd
    }
    return nil
}

func main() {
    numArgs := len(os.Args) - 1
    if numArgs < 1 || numArgs > 2 {
        printHelp()
        os.Exit(0)
    }
    filename := "commitfile"
    dirname := "commitdir"
    db, err := LoadDB(filename, dirname)
    if err != nil {
        log.Fatal(err)
    }
    defer db.file.Close()

    err = HandleCmd(db)
    if err == missArg || err == wrongCmd {
        fmt.Println(err)
        printHelp()
    } else if err != nil {
        log.Println(err)
    }
}

func printHelp() {
    fmt.Printf("MVCS - help - Commands :\n" +
        "\tadd [filename]\n" +
        "\trem [filename]\n" +
        "\tcom\n" +
        "\trev [rev numb]\n" +
        "\tlist\n")
}

func ger(ctxt string) func(string, error) error {
    return func(msg string, err error) error {
        return fmt.Errorf("%s : %s : %v", ctxt, msg, err)
    }
}

First time doing a sort-of usable CLI tool in Go, I didn't really know how git worked (still don't) so I'm probably mis-using words in there.

Things I'd like to improve on error handling, and maybe discover a better way to do this kind of program. Maybe some bits aren't idiomatic, I'd like to know.

\$\endgroup\$
5
+100
\$\begingroup\$

Version control system principle

Your "version control system" principle is a little strange for me.

You don't store which files belong to a revision (ID) and so for example if you add a file and you remove it later, the db file will have no knowledge of it. And so if you perform a revert operation, it will not delete previously committed instances of that file.

Also since every commit makes a copy of all the files that are under version control, it seems redundant and unneccessary to add the id to each of the file name. It would be easier and cleaner to just create a subfolder named after the revision or ID in the commit folder, and copy the files under that without renaming.

This ID-subfolder implementation gives you more than the db file: it gives you the file list for every single revision, while the db file can be used to provide the "staged", to-be-committed file list. A folder listing of any ID subfolder gives you the file list, the folder name itself gives you the ID, the biggest ID gives you the latest commit. Of course if you would want to improve the application to handle comments, you could use a db or meta file in every ID subfolder.

When you do a revert ("rev"), your code deletes files from the commit folder that have higher revision. This is a very deviant and weird implementation, it should just reproduce the revision in the local folder and leave the commit folder untouched.

If you would want to go serious about your version control tool, I would recommend to first get to know existing, popular version control systems to get ideas and to know what works and what not.

Now on to your code

It's hard to follow when and if the file stored in your DB struct is closed. The LoadDB function does not close it. If you create a resource which is returned and not closed, you should provide a Close() method on it which makes it clear to the users (which may only be you) that it has resources which should be closed properly.

For example if opening the file succeeds in LoadDB() but reading and decoding its content fails, LoadDB() returns an error and does not close it. And your main() function which called LoadDB() in this calse will do a log.Fatal() call and not close it, because defer db.file.Close() is only called after this. Although this won't cause any problems, it's not nice to sometimes properly close a resource and other times leave it hang and put it in the hands of the OS to release.

To address this, you could just remove the DB.file field (for which I don't see much purpose - both performance wise and otherwise), and open it when/where you need it (and use defer Close()). Or - as mentioned earlier - provide a DB.Close() method and use defer db.Close() in the main() function.

Your DB.Com() method:

Whenever I would need to do something using defer, I would warp in in a separate function, so it is clear when the defer needs to run and what it does. And also so that the deferred function runs as soon as possible and not hold on to resources when they are not needed anymore. In your Com() method you are using defer inside a loop, and also doing it manually at the end of each iteration. Even if you don't want to separate code (to a distinct function), the least I would do is use an anonymous function which would designate the point were defers used in that are called, and so you can also avoid duplication, for example:

for name := range something {
    func() {
        res, err := ... // Open some resource
        if err != nil {
            // Handle error
        }
        defer res.Close()
        // Do something with res
    }()
}

But of course it looks nicer this way:

func handleRes(name string) error {
    res, err := ... // Open some resource
    if err != nil {
        return err
    }
    defer res.Close()
    // Do something with res...
    return nil
}()

for name := range something {
    handleRes(name)
}

Also note that your code does not work properly on Windows systems as you append file paths to the commit dir, and if the file path contains a drive letter (e.g. C:\my\file.txt), the concatenation will result in an invalid path. A (not neccessarily the best) solution to this problem could be to introduce a "drive" level inside the commit dir, so for example C:\my\file.txt would go to "commitdir/c/my/file.txt". This is very easily implemented by first converting filepaths to absolute, and removing the ':' character, and then proceeding to concatenate as before.

Also note that your DB.Rev() method does not create subdirectories when trying to copy back the files, if subfolders are not present, will stop with an error. You should also use os.MkdirAll() before copying back.

Copying files:

In your DB.Com() method you copy files by reading their full content into memory (with ioutil.ReadAll()), and then you write the content from memory to the destination file. This is inefficient and becomes quite unfeasible in case of large files. Since *os.File implements both io.Reader and io.Writer interfaces, you can simply use the io.Copy() function to copy the contents of a file to another, e.g.:

// filea and fileb are opened:
if _, err := io.Copy(fileb, filea); err != nil {
    // Failed to copy
}

Reading from and writing to files:

To open a file for reading (only) purposes, you can just use the shorter and clearer os.Open() function. For writing (only) purposes (and truncating if exists), you can use the shorter and clearer os.Create() function.

Error handling

I think your error handling is fine and in some ways creative (with the ger() function returning another function with a context).

For other creative error handling see this answer and answers to this question.

\$\endgroup\$
  • \$\begingroup\$ I really appreciate the effort to understand my code, the advice given here is great. \$\endgroup\$ – Simpfally Jul 5 '16 at 10:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.