diff --git a/pathutil/pathutil.go b/pathutil/pathutil.go index 0be15d3c..c357938e 100644 --- a/pathutil/pathutil.go +++ b/pathutil/pathutil.go @@ -193,9 +193,9 @@ func ConfigTOML() string { return filepath.Join(VoltPath(), "config.toml") } -// TrxLock returns fullpath of "$HOME/volt/trx.lock". -func TrxLock() string { - return filepath.Join(VoltPath(), "trx.lock") +// TrxDir returns fullpath of "$HOME/volt/trx". +func TrxDir() string { + return filepath.Join(VoltPath(), "trx") } // TempDir returns fullpath of "$HOME/tmp". diff --git a/subcmd/build.go b/subcmd/build.go index 0b871d60..3d26417f 100644 --- a/subcmd/build.go +++ b/subcmd/build.go @@ -5,7 +5,6 @@ import ( "fmt" "os" - "github.com/vim-volt/volt/logger" "github.com/vim-volt/volt/subcmd/builder" "github.com/vim-volt/volt/transaction" ) @@ -53,7 +52,7 @@ Description return fs } -func (cmd *buildCmd) Run(args []string) *Error { +func (cmd *buildCmd) Run(args []string) (result *Error) { // Parse args fs := cmd.FlagSet() fs.Parse(args) @@ -62,18 +61,22 @@ func (cmd *buildCmd) Run(args []string) *Error { } // Begin transaction - err := transaction.Create() + trx, err := transaction.Start() if err != nil { - logger.Error() - return &Error{Code: 11, Msg: "Failed to begin transaction: " + err.Error()} + result = &Error{Code: 11, Msg: "Failed to begin transaction: " + err.Error()} + return } - defer transaction.Remove() + defer func() { + if err := trx.Done(); err != nil { + result = &Error{Code: 13, Msg: "Failed to end transaction: " + err.Error()} + } + }() err = builder.Build(cmd.full) if err != nil { - logger.Error() - return &Error{Code: 12, Msg: "Failed to build: " + err.Error()} + result = &Error{Code: 12, Msg: "Failed to build: " + err.Error()} + return } - return nil + return } diff --git a/subcmd/get.go b/subcmd/get.go index 54dc41cf..5a485101 100644 --- a/subcmd/get.go +++ b/subcmd/get.go @@ -192,26 +192,31 @@ func (cmd *getCmd) getReposPathList(args []string, lockJSON *lockjson.LockJSON) return reposPathList, nil } -func (cmd *getCmd) doGet(reposPathList []pathutil.ReposPath, lockJSON *lockjson.LockJSON) error { +func (cmd *getCmd) doGet(reposPathList []pathutil.ReposPath, lockJSON *lockjson.LockJSON) (err error) { // Find matching profile profile, err := lockJSON.Profiles.FindByName(lockJSON.CurrentProfileName) if err != nil { // this must not be occurred because lockjson.Read() // validates if the matching profile exists - return err + return } // Begin transaction - err = transaction.Create() + trx, err := transaction.Start() if err != nil { - return err + return } - defer transaction.Remove() + defer func() { + if e := trx.Done(); e != nil { + err = e + } + }() // Read config.toml cfg, err := config.Read() if err != nil { - return errors.Wrap(err, "could not read config.toml") + err = errors.Wrap(err, "could not read config.toml") + return } done := make(chan getParallelResult, len(reposPathList)) @@ -252,14 +257,16 @@ func (cmd *getCmd) doGet(reposPathList []pathutil.ReposPath, lockJSON *lockjson. // Write to lock.json err = lockJSON.Write() if err != nil { - return errors.Wrap(err, "could not write to lock.json") + err = errors.Wrap(err, "could not write to lock.json") + return } } // Build ~/.vim/pack/volt dir err = builder.Build(false) if err != nil { - return errors.Wrap(err, "could not build "+pathutil.VimVoltDir()) + err = errors.Wrap(err, "could not build "+pathutil.VimVoltDir()) + return } // Show results @@ -267,9 +274,10 @@ func (cmd *getCmd) doGet(reposPathList []pathutil.ReposPath, lockJSON *lockjson. fmt.Println(statusList[i]) } if failed { - return errors.New("failed to install some plugins") + err = errors.New("failed to install some plugins") + return } - return nil + return } func (*getCmd) formatStatus(r *getParallelResult) string { diff --git a/subcmd/migrate/lockjson.go b/subcmd/migrate/lockjson.go index fb3e963b..fec178fa 100644 --- a/subcmd/migrate/lockjson.go +++ b/subcmd/migrate/lockjson.go @@ -31,7 +31,7 @@ Description To suppress this, running this command simply reads and writes migrated structure to lock.json.` } -func (*lockjsonMigrater) Migrate() error { +func (*lockjsonMigrater) Migrate() (err error) { // Read lock.json lockJSON, err := lockjson.ReadNoMigrationMsg() if err != nil { @@ -39,16 +39,20 @@ func (*lockjsonMigrater) Migrate() error { } // Begin transaction - err = transaction.Create() + trx, err := transaction.Start() if err != nil { - return err + return } - defer transaction.Remove() + defer func() { + if e := trx.Done(); e != nil { + err = e + } + }() // Write to lock.json err = lockJSON.Write() if err != nil { return errors.Wrap(err, "could not write to lock.json") } - return nil + return } diff --git a/subcmd/migrate/plugconf-config-func.go b/subcmd/migrate/plugconf-config-func.go index 73d44ab8..c0b9e5c4 100644 --- a/subcmd/migrate/plugconf-config-func.go +++ b/subcmd/migrate/plugconf-config-func.go @@ -39,22 +39,24 @@ Description All plugconf files are replaced with new contents.` } -func (*plugconfConfigMigrater) Migrate() error { +func (*plugconfConfigMigrater) Migrate() (err error) { // Read lock.json lockJSON, err := lockjson.ReadNoMigrationMsg() if err != nil { - return errors.Wrap(err, "could not read lock.json") + err = errors.Wrap(err, "could not read lock.json") + return } results, parseErr := plugconf.ParseMultiPlugconf(lockJSON.Repos) if parseErr.HasErrs() { logger.Error("Please fix the following errors before migration:") - for _, err := range parseErr.Errors().Errors { - for _, line := range strings.Split(err.Error(), "\n") { + for _, e := range parseErr.Errors().Errors { + for _, line := range strings.Split(e.Error(), "\n") { logger.Errorf(" %s", line) } } - return nil + err = nil + return } type plugInfo struct { @@ -84,22 +86,27 @@ func (*plugconfConfigMigrater) Migrate() error { os.MkdirAll(filepath.Dir(info.path), 0755) err = ioutil.WriteFile(info.path, info.content, 0644) if err != nil { - return err + return } } // Begin transaction - err = transaction.Create() + trx, err := transaction.Start() if err != nil { - return err + return } - defer transaction.Remove() + defer func() { + if e := trx.Done(); e != nil { + err = e + } + }() // Build ~/.vim/pack/volt dir err = builder.Build(false) if err != nil { - return errors.Wrap(err, "could not build "+pathutil.VimVoltDir()) + err = errors.Wrap(err, "could not build "+pathutil.VimVoltDir()) + return } - return nil + return } diff --git a/subcmd/profile.go b/subcmd/profile.go index 7a2cebfc..7d1561cf 100644 --- a/subcmd/profile.go +++ b/subcmd/profile.go @@ -161,7 +161,7 @@ func (*profileCmd) getCurrentProfile() (string, error) { return lockJSON.CurrentProfileName, nil } -func (cmd *profileCmd) doSet(args []string) error { +func (cmd *profileCmd) doSet(args []string) (err error) { // Parse args createProfile := false if len(args) > 0 && args[0] == "-n" { @@ -171,45 +171,52 @@ func (cmd *profileCmd) doSet(args []string) error { if len(args) == 0 { cmd.FlagSet().Usage() logger.Error("'volt profile set' receives profile name.") - return nil + return } profileName := args[0] // Read lock.json lockJSON, err := lockjson.Read() if err != nil { - return errors.Wrap(err, "failed to read lock.json") + err = errors.Wrap(err, "failed to read lock.json") + return } // Exit if current profile is same as profileName if lockJSON.CurrentProfileName == profileName { - return errors.Errorf("'%s' is current profile", profileName) + err = errors.Errorf("'%s' is current profile", profileName) + return } // Create given profile unless the profile exists if _, err = lockJSON.Profiles.FindByName(profileName); err != nil { if !createProfile { - return err + return } if err = cmd.doNew([]string{profileName}); err != nil { - return err + return } // Read lock.json again lockJSON, err = lockjson.Read() if err != nil { - return errors.Wrap(err, "failed to read lock.json") + err = errors.Wrap(err, "failed to read lock.json") + return } if _, err = lockJSON.Profiles.FindByName(profileName); err != nil { - return err + return } } // Begin transaction - err = transaction.Create() + trx, err := transaction.Start() if err != nil { - return err + return } - defer transaction.Remove() + defer func() { + if e := trx.Done(); e != nil { + err = e + } + }() // Set profile name lockJSON.CurrentProfileName = profileName @@ -217,7 +224,7 @@ func (cmd *profileCmd) doSet(args []string) error { // Write to lock.json err = lockJSON.Write() if err != nil { - return err + return } logger.Info("Changed current profile: " + profileName) @@ -225,10 +232,11 @@ func (cmd *profileCmd) doSet(args []string) error { // Build ~/.vim/pack/volt dir err = builder.Build(false) if err != nil { - return errors.Wrap(err, "could not build "+pathutil.VimVoltDir()) + err = errors.Wrap(err, "could not build "+pathutil.VimVoltDir()) + return } - return nil + return } func (cmd *profileCmd) doShow(args []string) error { @@ -272,32 +280,38 @@ func (cmd *profileCmd) doList(args []string) error { `) } -func (cmd *profileCmd) doNew(args []string) error { +func (cmd *profileCmd) doNew(args []string) (err error) { if len(args) == 0 { cmd.FlagSet().Usage() logger.Error("'volt profile new' receives profile name.") - return nil + return } profileName := args[0] // Read lock.json lockJSON, err := lockjson.Read() if err != nil { - return errors.Wrap(err, "failed to read lock.json") + err = errors.Wrap(err, "failed to read lock.json") + return } // Return error if profiles[]/name matches profileName _, err = lockJSON.Profiles.FindByName(profileName) if err == nil { - return errors.New("profile '" + profileName + "' already exists") + err = errors.New("profile '" + profileName + "' already exists") + return } // Begin transaction - err = transaction.Create() + trx, err := transaction.Start() if err != nil { - return err + return } - defer transaction.Remove() + defer func() { + if e := trx.Done(); e != nil { + err = e + } + }() // Add profile lockJSON.Profiles = append(lockJSON.Profiles, lockjson.Profile{ @@ -308,33 +322,38 @@ func (cmd *profileCmd) doNew(args []string) error { // Write to lock.json err = lockJSON.Write() if err != nil { - return err + return } logger.Info("Created new profile '" + profileName + "'") - return nil + return } -func (cmd *profileCmd) doDestroy(args []string) error { +func (cmd *profileCmd) doDestroy(args []string) (err error) { if len(args) == 0 { cmd.FlagSet().Usage() logger.Error("'volt profile destroy' receives profile name.") - return nil + return } // Read lock.json lockJSON, err := lockjson.Read() if err != nil { - return errors.Wrap(err, "failed to read lock.json") + err = errors.Wrap(err, "failed to read lock.json") + return } // Begin transaction - err = transaction.Create() + trx, err := transaction.Start() if err != nil { - return err + return } - defer transaction.Remove() + defer func() { + if e := trx.Done(); e != nil { + err = e + } + }() var merr *multierror.Error for i := range args { @@ -359,7 +378,8 @@ func (cmd *profileCmd) doDestroy(args []string) error { rcDir := pathutil.RCDir(profileName) os.RemoveAll(rcDir) if pathutil.Exists(rcDir) { - return errors.New("failed to remove " + rcDir) + err = errors.New("failed to remove " + rcDir) + return } logger.Info("Deleted profile '" + profileName + "'") @@ -368,17 +388,18 @@ func (cmd *profileCmd) doDestroy(args []string) error { // Write to lock.json err = lockJSON.Write() if err != nil { - return err + return } - return merr.ErrorOrNil() + err = merr.ErrorOrNil() + return } -func (cmd *profileCmd) doRename(args []string) error { +func (cmd *profileCmd) doRename(args []string) (err error) { if len(args) != 2 { cmd.FlagSet().Usage() logger.Error("'volt profile rename' receives profile name.") - return nil + return } oldName := args[0] newName := args[1] @@ -386,26 +407,33 @@ func (cmd *profileCmd) doRename(args []string) error { // Read lock.json lockJSON, err := lockjson.Read() if err != nil { - return errors.Wrap(err, "failed to read lock.json") + err = errors.Wrap(err, "failed to read lock.json") + return } // Return error if profiles[]/name does not match oldName index := lockJSON.Profiles.FindIndexByName(oldName) if index < 0 { - return errors.New("profile '" + oldName + "' does not exist") + err = errors.New("profile '" + oldName + "' does not exist") + return } // Return error if profiles[]/name does not match newName if lockJSON.Profiles.FindIndexByName(newName) >= 0 { - return errors.New("profile '" + newName + "' already exists") + err = errors.New("profile '" + newName + "' already exists") + return } // Begin transaction - err = transaction.Create() + trx, err := transaction.Start() if err != nil { - return err + return } - defer transaction.Remove() + defer func() { + if e := trx.Done(); e != nil { + err = e + } + }() // Rename profile names lockJSON.Profiles[index].Name = newName @@ -418,19 +446,19 @@ func (cmd *profileCmd) doRename(args []string) error { if pathutil.Exists(oldRCDir) { newRCDir := pathutil.RCDir(newName) if err = os.Rename(oldRCDir, newRCDir); err != nil { - return errors.Errorf("could not rename %s to %s", oldRCDir, newRCDir) + return } } // Write to lock.json err = lockJSON.Write() if err != nil { - return err + return } logger.Infof("Renamed profile '%s' to '%s'", oldName, newName) - return nil + return } func (cmd *profileCmd) doAdd(args []string) error { @@ -451,7 +479,7 @@ func (cmd *profileCmd) doAdd(args []string) error { } // Read modified profile and write to lock.json - lockJSON, err = cmd.transactProfile(lockJSON, profileName, func(profile *lockjson.Profile) { + err = cmd.transactProfile(lockJSON, profileName, func(profile *lockjson.Profile) { // Add repositories to profile if the repository does not exist for _, reposPath := range reposPathList { if profile.ReposPath.Contains(reposPath) { @@ -493,7 +521,7 @@ func (cmd *profileCmd) doRm(args []string) error { } // Read modified profile and write to lock.json - lockJSON, err = cmd.transactProfile(lockJSON, profileName, func(profile *lockjson.Profile) { + err = cmd.transactProfile(lockJSON, profileName, func(profile *lockjson.Profile) { // Remove repositories from profile if the repository does not exist for _, reposPath := range reposPathList { index := profile.ReposPath.IndexOf(reposPath) @@ -547,26 +575,30 @@ func (cmd *profileCmd) parseAddArgs(lockJSON *lockjson.LockJSON, subCmd string, } // Run modifyProfile and write modified structure to lock.json -func (*profileCmd) transactProfile(lockJSON *lockjson.LockJSON, profileName string, modifyProfile func(*lockjson.Profile)) (*lockjson.LockJSON, error) { +func (*profileCmd) transactProfile(lockJSON *lockjson.LockJSON, profileName string, modifyProfile func(*lockjson.Profile)) (err error) { // Return error if profiles[]/name does not match profileName profile, err := lockJSON.Profiles.FindByName(profileName) if err != nil { - return nil, err + return } // Begin transaction - err = transaction.Create() + trx, err := transaction.Start() if err != nil { - return nil, err + return } - defer transaction.Remove() + defer func() { + if e := trx.Done(); e != nil { + err = e + } + }() modifyProfile(profile) // Write to lock.json err = lockJSON.Write() if err != nil { - return nil, err + return } - return lockJSON, nil + return } diff --git a/subcmd/rm.go b/subcmd/rm.go index ee732a47..902efa97 100644 --- a/subcmd/rm.go +++ b/subcmd/rm.go @@ -109,19 +109,23 @@ func (cmd *rmCmd) parseArgs(args []string) ([]pathutil.ReposPath, error) { return reposPathList, nil } -func (cmd *rmCmd) doRemove(reposPathList []pathutil.ReposPath) error { +func (cmd *rmCmd) doRemove(reposPathList []pathutil.ReposPath) (err error) { // Read lock.json lockJSON, err := lockjson.Read() if err != nil { - return err + return } // Begin transaction - err = transaction.Create() + trx, err := transaction.Start() if err != nil { - return err + return } - defer transaction.Remove() + defer func() { + if e := trx.Done(); e != nil { + err = e + } + }() // Get the existing entries if already have it // (e.g. github.com/tyru/CaW.vim -> github.com/tyru/caw.vim) @@ -136,13 +140,15 @@ func (cmd *rmCmd) doRemove(reposPathList []pathutil.ReposPath) error { // Check if specified plugins are depended by some plugins for _, reposPath := range reposPathList { - rdeps, err := plugconf.RdepsOf(reposPath, lockJSON.Repos) + var rdeps pathutil.ReposPathList + rdeps, err = plugconf.RdepsOf(reposPath, lockJSON.Repos) if err != nil { - return err + return } if len(rdeps) > 0 { - return errors.Errorf("cannot remove '%s' because it's depended by '%s'", + err = errors.Errorf("cannot remove '%s' because it's depended by '%s'", reposPath, strings.Join(rdeps.Strings(), "', '")) + return } } @@ -153,7 +159,7 @@ func (cmd *rmCmd) doRemove(reposPathList []pathutil.ReposPath) error { fullReposPath := reposPath.FullPath() if pathutil.Exists(fullReposPath) { if err = cmd.removeRepos(fullReposPath); err != nil { - return err + return } removeCount++ } else { @@ -166,7 +172,7 @@ func (cmd *rmCmd) doRemove(reposPathList []pathutil.ReposPath) error { plugconfPath := reposPath.Plugconf() if pathutil.Exists(plugconfPath) { if err = cmd.removePlugconf(plugconfPath); err != nil { - return err + return } removeCount++ } else { @@ -182,14 +188,13 @@ func (cmd *rmCmd) doRemove(reposPathList []pathutil.ReposPath) error { } } if removeCount == 0 { - return errors.New("no plugins are removed") + err = errors.New("no plugins are removed") + return } // Write to lock.json - if err = lockJSON.Write(); err != nil { - return err - } - return nil + err = lockJSON.Write() + return } // Remove repository directory diff --git a/transaction/transaction.go b/transaction/transaction.go index c5f4ac4d..fc76feaf 100644 --- a/transaction/transaction.go +++ b/transaction/transaction.go @@ -1,68 +1,114 @@ package transaction import ( - "github.com/pkg/errors" - "io/ioutil" "os" "path/filepath" "strconv" + "strings" + "unicode" - "github.com/vim-volt/volt/logger" + "github.com/pkg/errors" "github.com/vim-volt/volt/pathutil" ) -// Create creates $VOLTPATH/trx.lock file -func Create() error { - ownPid := []byte(strconv.Itoa(os.Getpid())) - trxLockFile := pathutil.TrxLock() - - // Create trx.lock parent directories - err := os.MkdirAll(filepath.Dir(trxLockFile), 0755) +// Start creates $VOLTPATH/trx/lock directory. +func Start() (Transaction, error) { + os.MkdirAll(pathutil.TrxDir(), 0755) + lockDir := filepath.Join(pathutil.TrxDir(), "lock") + if err := os.Mkdir(lockDir, 0755); err != nil { + return nil, errors.Wrap(err, "failed to begin transaction: "+lockDir+" exists: if no other volt process is currently running, this probably means a volt process crashed earlier. Make sure no other volt process is running and remove the file manually to continue") + } + trxID, err := genNewTrxID() if err != nil { - return errors.Wrap(err, "failed to begin transaction") + return nil, errors.Wrap(err, "could not allocate a new transaction ID") } + return &transaction{id: trxID}, nil +} - // Return error if the file exists - if pathutil.Exists(trxLockFile) { - return errors.New("failed to begin transaction: " + pathutil.TrxLock() + " exists: if no other volt process is currently running, this probably means a volt process crashed earlier. Make sure no other volt process is running and remove the file manually to continue") - } +// Transaction provides transaction methods. +type Transaction interface { + // Done renames "lock" directory to "{trxid}" directory + Done() error + + // ID returns transaction ID + ID() TrxID +} + +type transaction struct { + id TrxID +} + +func (trx *transaction) ID() TrxID { + return trx.id +} - // Write pid to trx.lock file - err = ioutil.WriteFile(trxLockFile, ownPid, 0644) +// Done removes $VOLTPATH/trx/lock directory. +func (trx *transaction) Done() error { + lockDir := filepath.Join(pathutil.TrxDir(), "lock") + return os.Remove(lockDir) +} + +// genNewTrxID gets unallocated transaction ID looking $VOLTPATH/trx/ directory. +func genNewTrxID() (_ TrxID, result error) { + trxDir, err := os.Open(pathutil.TrxDir()) if err != nil { - return errors.Wrap(err, "failed to begin transaction") + return nil, errors.Wrap(err, "could not open $VOLTPATH/trx directory") } - - // Read pid from trx.lock file - pid, err := ioutil.ReadFile(trxLockFile) + defer func() { result = trxDir.Close() }() + names, err := trxDir.Readdirnames(0) if err != nil { - return errors.Wrap(err, "failed to begin transaction") + return nil, errors.Wrap(err, "could not readdir of $VOLTPATH/trx directory") } - - if string(pid) != string(ownPid) { - return errors.New("transaction lock was taken by PID " + string(pid)) + var maxID TrxID + for i := range names { + if !isTrxDirName(names[i]) { + continue + } + if maxID == nil { + maxID = TrxID(names[i]) + continue + } + if greaterThan(names[i], string(maxID)) { + maxID = TrxID(names[i]) + } + } + if maxID == nil { + return TrxID("1"), nil // no transaction ID directory } - return nil + return maxID.inc() } -// Remove removes $VOLTPATH/trx.lock file -func Remove() { - // Read pid from trx.lock file - trxLockFile := pathutil.TrxLock() - pid, err := ioutil.ReadFile(trxLockFile) - if err != nil { - logger.Error("trx.lock was already removed") - return +func greaterThan(a, b string) bool { + d := len(a) - len(b) + if d > 0 { + b = strings.Repeat("0", d) + b + } else if d < 0 { + a = strings.Repeat("0", -d) + a } + return strings.Compare(a, b) > 0 +} - // Remove trx.lock if pid is same - if string(pid) != strconv.Itoa(os.Getpid()) { - logger.Error("Cannot remove another process's trx.lock") - return +func isTrxDirName(name string) bool { + for _, r := range name { + if !unicode.IsDigit(r) { + return false + } } - err = os.Remove(trxLockFile) + return true +} + +// TrxID is a transaction ID, which is a serial number and directory name of +// transaction log file. +type TrxID []byte + +func (tid *TrxID) inc() (TrxID, error) { + newID, err := strconv.ParseUint(string(*tid), 10, 32) if err != nil { - logger.Error("Cannot remove trx.lock: " + err.Error()) - return + return nil, err + } + if newID+uint64(1) < newID { + // TODO: compute in string? + return nil, errors.Errorf("%d + %d causes overflow", newID, 1) } + return TrxID(strconv.FormatUint(newID+uint64(1), 10)), nil }