Repository: opentofu/opentofu
File: internal/states/statemgr/filesystem.go (Go)

Code before:
}

// Open the state file, creating the directories and file as needed.
func (s *Filesystem) createStateFiles() error {
	log.Printf("[TRACE] statemgr.Filesystem: preparing to manage state snapshots at %s", s.path)

	// This could race, but we only use it to clean up empty files
	if _, err := os.Stat(s.path); os.IsNotExist(err) {
		s.created = true
	}

	// Create all the directories
	if err := os.MkdirAll(filepath.Dir(s.path), 0755); err != nil {
		return err
	}

	f, err := os.OpenFile(s.path, os.O_RDWR|os.O_CREATE, 0666)
	if err != nil {
		return err
	}

	s.stateFileOut = f
	return nil
}

func (s *Filesystem) writeBackupFile() error {
	// If the file already existed with content then that'll be the content
	// of our backup file if we write a change later.
	var err error
	s.backupFile, err = statefile.Read(s.stateFileOut, s.encryption)
	if err != nil {
		if err != statefile.ErrNoState {
			return err
		}
		log.Printf("[TRACE] statemgr.Filesystem: no previously-stored snapshot exists")
	} else {
		log.Printf("[TRACE] statemgr.Filesystem: existing snapshot has lineage %q serial %d", s.backupFile.Lineage, s.backupFile.Serial)
	}

	// Refresh now, to load in the snapshot if the file already existed
	return nil
}

// return the path for the lockInfo metadata.
func (s *Filesystem) lockInfoPath() string {
	stateDir, stateName := filepath.Split(s.path)
	if stateName == "" {
		panic("empty state file path")
	}

	if stateName[0] == '.' {

Review comment (suggestion):
I think I broadly understand why this change fixed the test -- it means that the `Lock` method can acquire a lock on the file before we try to read anything from it, which is important on Windows because it uses mandatory rather than advisory locks.

However, it took me a while to understand this change because I was initially confused by the name of this function. I assume that by "write backup file" you meant to write to _the `s.backupFile` field_, but unfortunately the naming is ambiguous because "backup file" could also refer to the the on disk that [this `s.backupFile` object is eventually saved into](https://github.com/opentofu/opentofu/blob/b203bc0dd7d09a8c549aa6081729531a3b54d100/internal/states/statemgr/filesystem.go#L202-L207) (whose path is in `s.backupPath`), so I expected that this function would actually write to that file on disk.

What do you think about calling this `prepareBackupFile` instead? For me, that means "make preparations to write the backup file later" rather than "write the backup file _now_".


Code after:
}

// Open the state file, creating the directories and file as needed.
func (s *Filesystem) createStateFiles() error {
	log.Printf("[TRACE] statemgr.Filesystem: preparing to manage state snapshots at %s", s.path)

	// This could race, but we only use it to clean up empty files
	if _, err := os.Stat(s.path); os.IsNotExist(err) {
		s.created = true
	}

	// Create all the directories
	if err := os.MkdirAll(filepath.Dir(s.path), 0755); err != nil {
		return err
	}

	f, err := os.OpenFile(s.path, os.O_RDWR|os.O_CREATE, 0666)
	if err != nil {
		return err
	}

	s.stateFileOut = f
	return nil
}

func (s *Filesystem) prepareBackupFile() error {
	// If the file already existed with content then that'll be the content
	// of our backup file if we write a change later.
	var err error
	s.backupFile, err = statefile.Read(s.stateFileOut, s.encryption)
	if err != nil {
		if err != statefile.ErrNoState {
			return err
		}
		log.Printf("[TRACE] statemgr.Filesystem: no previously-stored snapshot exists")
	} else {
		log.Printf("[TRACE] statemgr.Filesystem: existing snapshot has lineage %q serial %d", s.backupFile.Lineage, s.backupFile.Serial)
	}

	// Refresh now, to load in the snapshot if the file already existed
	return nil
}

// return the path for the lockInfo metadata.
func (s *Filesystem) lockInfoPath() string {
	stateDir, stateName := filepath.Split(s.path)
	if stateName == "" {
		panic("empty state file path")
	}

	if stateName[0] == '.' {